From 8c5bd755602e2b8b0e0d4c6292645c8b92ede5ac Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Wed, 20 Sep 2023 00:20:50 +0800 Subject: [PATCH] Use BillingCycleRepository in DataUsageList Unify the enable logic for the "Data warning & limit" page. Bug: 290856342 Test: manual - on DataUsageList Test: m RunSettingsRoboTests Change-Id: I3014461ef21768b5d0eb6d91873a4ba52d20f6bf --- .../datausage/BillingCyclePreference.kt | 1 + .../datausage/DataUsageBaseFragment.java | 45 ------------------- .../settings/datausage/DataUsageList.java | 21 +++++++-- .../datausage/DataUsageListAppsController.kt | 6 ++- .../settings/datausage/DataUsageSummary.java | 2 +- .../datausage/TemplatePreference.java | 8 ---- .../datausage/lib/BillingCycleRepository.kt | 6 ++- .../settings/datausage/DataUsageListTest.java | 43 +++++++++++++----- 8 files changed, 58 insertions(+), 74 deletions(-) diff --git a/src/com/android/settings/datausage/BillingCyclePreference.kt b/src/com/android/settings/datausage/BillingCyclePreference.kt index 05066be7541..619f7e96286 100644 --- a/src/com/android/settings/datausage/BillingCyclePreference.kt +++ b/src/com/android/settings/datausage/BillingCyclePreference.kt @@ -49,6 +49,7 @@ class BillingCyclePreference @JvmOverloads constructor( this.subId = subId summary = null updateEnabled() + intent = intent } override fun onAttached() { diff --git a/src/com/android/settings/datausage/DataUsageBaseFragment.java b/src/com/android/settings/datausage/DataUsageBaseFragment.java index eee3228d13d..5ddbab88428 100644 --- a/src/com/android/settings/datausage/DataUsageBaseFragment.java +++ b/src/com/android/settings/datausage/DataUsageBaseFragment.java @@ -15,23 +15,13 @@ package com.android.settings.datausage; import android.content.Context; -import android.net.NetworkPolicy; import android.net.NetworkPolicyManager; import android.os.Bundle; -import android.os.INetworkManagementService; -import android.os.RemoteException; -import android.os.ServiceManager; -import android.os.UserManager; -import android.telephony.SubscriptionManager; -import android.telephony.TelephonyManager; -import android.util.Log; import com.android.settings.dashboard.DashboardFragment; import com.android.settingslib.NetworkPolicyEditor; public abstract class DataUsageBaseFragment extends DashboardFragment { - private static final String TAG = "DataUsageBase"; - private static final String ETHERNET = "ethernet"; protected final TemplatePreference.NetworkServices services = new TemplatePreference.NetworkServices(); @@ -41,16 +31,10 @@ public abstract class DataUsageBaseFragment extends DashboardFragment { super.onCreate(icicle); Context context = getContext(); - services.mNetworkService = INetworkManagementService.Stub.asInterface( - ServiceManager.getService(Context.NETWORKMANAGEMENT_SERVICE)); services.mPolicyManager = (NetworkPolicyManager) context .getSystemService(Context.NETWORK_POLICY_SERVICE); services.mPolicyEditor = new NetworkPolicyEditor(services.mPolicyManager); - - services.mTelephonyManager = context.getSystemService(TelephonyManager.class); - services.mSubscriptionManager = SubscriptionManager.from(context); - services.mUserManager = UserManager.get(context); } @Override @@ -58,33 +42,4 @@ public abstract class DataUsageBaseFragment extends DashboardFragment { super.onResume(); services.mPolicyEditor.read(); } - - protected boolean isAdmin() { - return services.mUserManager.isAdminUser(); - } - - protected boolean isMobileDataAvailable(int subId) { - return services.mSubscriptionManager.getActiveSubscriptionInfo(subId) != null; - } - - protected boolean isNetworkPolicyModifiable(NetworkPolicy policy, int subId) { - return policy != null && isBandwidthControlEnabled() && services.mUserManager.isAdminUser() - && isDataEnabled(subId); - } - - private boolean isDataEnabled(int subId) { - if (subId == SubscriptionManager.INVALID_SUBSCRIPTION_ID) { - return true; - } - return services.mTelephonyManager.getDataEnabled(subId); - } - - protected boolean isBandwidthControlEnabled() { - try { - return services.mNetworkService.isBandwidthControlEnabled(); - } catch (RemoteException e) { - Log.w(TAG, "problem talking with INetworkManagementService: ", e); - return false; - } - } } diff --git a/src/com/android/settings/datausage/DataUsageList.java b/src/com/android/settings/datausage/DataUsageList.java index 39287c19471..15a56039ed9 100644 --- a/src/com/android/settings/datausage/DataUsageList.java +++ b/src/com/android/settings/datausage/DataUsageList.java @@ -32,7 +32,6 @@ import android.view.View.AccessibilityDelegate; import android.view.accessibility.AccessibilityEvent; import android.widget.AdapterView; import android.widget.AdapterView.OnItemSelectedListener; -import android.widget.ImageView; import android.widget.Spinner; import androidx.annotation.NonNull; @@ -46,6 +45,7 @@ import androidx.preference.Preference; import com.android.settings.R; import com.android.settings.core.SubSettingLauncher; import com.android.settings.datausage.CycleAdapter.SpinnerInterface; +import com.android.settings.datausage.lib.BillingCycleRepository; import com.android.settings.network.MobileDataEnabledListener; import com.android.settings.network.MobileNetworkRepository; import com.android.settings.widget.LoadingViewController; @@ -108,6 +108,7 @@ public class DataUsageList extends DataUsageBaseFragment private MobileNetworkRepository mMobileNetworkRepository; private SubscriptionInfoEntity mSubscriptionInfoEntity; private DataUsageListAppsController mDataUsageListAppsController; + private BillingCycleRepository mBillingCycleRepository; @Override public int getMetricsCategory() { @@ -125,7 +126,8 @@ public class DataUsageList extends DataUsageBaseFragment } final Activity activity = getActivity(); - if (!isBandwidthControlEnabled()) { + mBillingCycleRepository = createBillingCycleRepository(); + if (!mBillingCycleRepository.isBandwidthControlEnabled()) { Log.w(TAG, "No bandwidth control; leaving"); activity.finish(); return; @@ -146,6 +148,12 @@ public class DataUsageList extends DataUsageBaseFragment mDataUsageListAppsController.init(mTemplate); } + @VisibleForTesting + @NonNull + BillingCycleRepository createBillingCycleRepository() { + return new BillingCycleRepository(requireContext()); + } + @Override public void onViewCreated(@NonNull View v, Bundle savedInstanceState) { super.onViewCreated(v, savedInstanceState); @@ -286,10 +294,9 @@ public class DataUsageList extends DataUsageBaseFragment final NetworkPolicy policy = services.mPolicyEditor.getPolicy(mTemplate); final View configureButton = mHeader.findViewById(R.id.filter_settings); //SUB SELECT - if (isNetworkPolicyModifiable(policy, mSubId) && isMobileDataAvailable(mSubId)) { + if (policy != null && isMobileDataAvailable()) { mChart.setNetworkPolicy(policy); configureButton.setVisibility(View.VISIBLE); - ((ImageView) configureButton).setColorFilter(android.R.color.white); } else { // controls are disabled; don't bind warning/limit sweeps mChart.setNetworkPolicy(null); @@ -304,6 +311,12 @@ public class DataUsageList extends DataUsageBaseFragment updateSelectedCycle(); } + private boolean isMobileDataAvailable() { + return mBillingCycleRepository.isModifiable(mSubId) + && SubscriptionManager.from(requireContext()) + .getActiveSubscriptionInfo(mSubId) != null; + } + /** * Updates the chart and detail data when initial loaded or selected cycle changed. */ diff --git a/src/com/android/settings/datausage/DataUsageListAppsController.kt b/src/com/android/settings/datausage/DataUsageListAppsController.kt index cc55e1ad1fc..c324407655a 100644 --- a/src/com/android/settings/datausage/DataUsageListAppsController.kt +++ b/src/com/android/settings/datausage/DataUsageListAppsController.kt @@ -20,6 +20,7 @@ import android.app.ActivityManager import android.content.Context import android.net.NetworkTemplate import android.os.Bundle +import androidx.annotation.OpenForTesting import androidx.annotation.VisibleForTesting import androidx.lifecycle.LifecycleCoroutineScope import androidx.lifecycle.LifecycleOwner @@ -37,7 +38,8 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import kotlinx.coroutines.withContext -class DataUsageListAppsController(context: Context, preferenceKey: String) : +@OpenForTesting +open class DataUsageListAppsController(context: Context, preferenceKey: String) : BasePreferenceController(context, preferenceKey) { private val uidDetailProvider = UidDetailProvider(context) @@ -48,7 +50,7 @@ class DataUsageListAppsController(context: Context, preferenceKey: String) : private var cycleData: List? = null - fun init(template: NetworkTemplate) { + open fun init(template: NetworkTemplate) { this.template = template repository = AppDataUsageRepository( context = mContext, diff --git a/src/com/android/settings/datausage/DataUsageSummary.java b/src/com/android/settings/datausage/DataUsageSummary.java index 6966611ed5f..4f876abab8d 100644 --- a/src/com/android/settings/datausage/DataUsageSummary.java +++ b/src/com/android/settings/datausage/DataUsageSummary.java @@ -103,7 +103,7 @@ public class DataUsageSummary extends DataUsageBaseFragment implements DataUsage mDefaultTemplate = DataUsageUtils.getDefaultTemplate(context, defaultSubId); mSummaryPreference = findPreference(KEY_STATUS_HEADER); - if (!hasMobileData || !isAdmin()) { + if (!hasMobileData || !UserManager.get(context).isAdminUser()) { removePreference(KEY_RESTRICT_BACKGROUND); } boolean hasWifiRadio = DataUsageUtils.hasWifiRadio(context); diff --git a/src/com/android/settings/datausage/TemplatePreference.java b/src/com/android/settings/datausage/TemplatePreference.java index 61822299f99..8e780dbf94e 100644 --- a/src/com/android/settings/datausage/TemplatePreference.java +++ b/src/com/android/settings/datausage/TemplatePreference.java @@ -16,10 +16,6 @@ package com.android.settings.datausage; import android.net.NetworkPolicyManager; import android.net.NetworkTemplate; -import android.os.INetworkManagementService; -import android.os.UserManager; -import android.telephony.SubscriptionManager; -import android.telephony.TelephonyManager; import com.android.settingslib.NetworkPolicyEditor; @@ -29,11 +25,7 @@ public interface TemplatePreference { void setTemplate(NetworkTemplate template, int subId); class NetworkServices { - INetworkManagementService mNetworkService; NetworkPolicyManager mPolicyManager; - TelephonyManager mTelephonyManager; - SubscriptionManager mSubscriptionManager; - UserManager mUserManager; NetworkPolicyEditor mPolicyEditor; } diff --git a/src/com/android/settings/datausage/lib/BillingCycleRepository.kt b/src/com/android/settings/datausage/lib/BillingCycleRepository.kt index 779ae4a3a4b..bd6aa273ad2 100644 --- a/src/com/android/settings/datausage/lib/BillingCycleRepository.kt +++ b/src/com/android/settings/datausage/lib/BillingCycleRepository.kt @@ -21,9 +21,11 @@ import android.os.INetworkManagementService import android.os.ServiceManager import android.telephony.TelephonyManager import android.util.Log +import androidx.annotation.OpenForTesting import com.android.settingslib.spaprivileged.framework.common.userManager -class BillingCycleRepository( +@OpenForTesting +open class BillingCycleRepository @JvmOverloads constructor( context: Context, private val networkService: INetworkManagementService = INetworkManagementService.Stub.asInterface( @@ -36,7 +38,7 @@ class BillingCycleRepository( fun isModifiable(subId: Int): Boolean = isBandwidthControlEnabled() && userManager.isAdminUser && isDataEnabled(subId) - fun isBandwidthControlEnabled(): Boolean = try { + open fun isBandwidthControlEnabled(): Boolean = try { networkService.isBandwidthControlEnabled } catch (e: Exception) { Log.w(TAG, "problem talking with INetworkManagementService: ", e) diff --git a/tests/robotests/src/com/android/settings/datausage/DataUsageListTest.java b/tests/robotests/src/com/android/settings/datausage/DataUsageListTest.java index b16d336804f..5eee615a408 100644 --- a/tests/robotests/src/com/android/settings/datausage/DataUsageListTest.java +++ b/tests/robotests/src/com/android/settings/datausage/DataUsageListTest.java @@ -17,6 +17,7 @@ package com.android.settings.datausage; import static com.google.common.truth.Truth.assertThat; + import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; @@ -38,22 +39,28 @@ import android.view.View; import android.widget.FrameLayout; import android.widget.Spinner; +import androidx.annotation.NonNull; import androidx.fragment.app.FragmentActivity; import androidx.loader.app.LoaderManager; import androidx.preference.PreferenceManager; import com.android.settings.R; +import com.android.settings.datausage.lib.BillingCycleRepository; import com.android.settings.network.MobileDataEnabledListener; import com.android.settings.testutils.FakeFeatureFactory; import com.android.settings.widget.LoadingViewController; import com.android.settingslib.NetworkPolicyEditor; +import com.android.settingslib.core.AbstractPreferenceController; import com.android.settingslib.core.instrumentation.VisibilityLoggerMixin; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.MockitoAnnotations; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; import org.robolectric.Robolectric; import org.robolectric.RobolectricTestRunner; import org.robolectric.android.controller.ActivityController; @@ -64,6 +71,8 @@ import org.robolectric.util.ReflectionHelpers; @RunWith(RobolectricTestRunner.class) public class DataUsageListTest { + @Rule + public MockitoRule mMockitoRule = MockitoJUnit.rule(); @Mock private MobileDataEnabledListener mMobileDataEnabledListener; @@ -73,20 +82,23 @@ public class DataUsageListTest { private LoaderManager mLoaderManager; @Mock private UserManager mUserManager; + @Mock + private BillingCycleRepository mBillingCycleRepository; private Activity mActivity; - private DataUsageList mDataUsageList; + + @Spy + private TestDataUsageList mDataUsageList; @Before public void setUp() { - MockitoAnnotations.initMocks(this); FakeFeatureFactory.setupForTest(); final ActivityController mActivityController = Robolectric.buildActivity(Activity.class); mActivity = spy(mActivityController.get()); mNetworkServices.mPolicyEditor = mock(NetworkPolicyEditor.class); - mDataUsageList = spy(DataUsageList.class); mDataUsageList.mDataStateListener = mMobileDataEnabledListener; + mDataUsageList.mTemplate = mock(NetworkTemplate.class); doReturn(mActivity).when(mDataUsageList).getContext(); doReturn(mUserManager).when(mActivity).getSystemService(UserManager.class); @@ -97,6 +109,7 @@ public class DataUsageListTest { doReturn(mLoaderManager).when(mDataUsageList).getLoaderManager(); mDataUsageList.mLoadingViewController = mock(LoadingViewController.class); doNothing().when(mDataUsageList).updateSubscriptionInfoEntity(); + when(mBillingCycleRepository.isBandwidthControlEnabled()).thenReturn(true); } @Test @@ -225,15 +238,13 @@ public class DataUsageListTest { final View rootView = LayoutInflater.from(mActivity) .inflate(R.layout.preference_list_fragment, null, false); final FrameLayout pinnedHeader = rootView.findViewById(R.id.pinned_header); - final View header = mActivity.getLayoutInflater() - .inflate(R.layout.apps_filter_spinner, pinnedHeader, false); - return header; + return mActivity.getLayoutInflater() + .inflate(R.layout.apps_filter_spinner, pinnedHeader, false); } private Spinner getSpinner(View header) { - final Spinner spinner = header.findViewById(R.id.filter_spinner); - return spinner; + return header.findViewById(R.id.filter_spinner); } @Implements(DataUsageBaseFragment.class) @@ -242,10 +253,18 @@ public class DataUsageListTest { public void onCreate(Bundle icicle) { // do nothing } + } - @Implementation - protected boolean isBandwidthControlEnabled() { - return true; + public class TestDataUsageList extends DataUsageList { + @Override + protected T use(Class clazz) { + return mock(clazz); + } + + @NonNull + @Override + BillingCycleRepository createBillingCycleRepository() { + return mBillingCycleRepository; } } }