From 1ee4adf32162116f0b530da46c482b72ea1ed3e4 Mon Sep 17 00:00:00 2001 From: Arc Wang Date: Wed, 15 May 2019 17:44:03 +0800 Subject: [PATCH 1/3] Refactor WifiWakeupPreferenceController into TogglePreferenceController WifiWakeupPreferenceController is essentially reimplementing TogglePreferenceController. Add below 2 changes for test failure of CodeInspectionTest 1. Remove some arguments from constructor of the controller 2. Declare the controller in xml instead of in code Bug: 132391311 Test: manual WifiWakeupPreferenceControllerTest Change-Id: I4aa607f78d5e7de70600a410dfc7267e6bd7d387 --- res/xml/wifi_configure_settings.xml | 3 +- .../settings/wifi/ConfigureWifiSettings.java | 13 ++- .../wifi/WifiWakeupPreferenceController.java | 92 +++++++------------ .../WifiWakeupPreferenceControllerTest.java | 42 +++------ 4 files changed, 59 insertions(+), 91 deletions(-) diff --git a/res/xml/wifi_configure_settings.xml b/res/xml/wifi_configure_settings.xml index c6214662d35..a5152acb09e 100644 --- a/res/xml/wifi_configure_settings.xml +++ b/res/xml/wifi_configure_settings.xml @@ -24,7 +24,8 @@ android:key="enable_wifi_wakeup" android:title="@string/wifi_wakeup" android:icon="@drawable/ic_auto_wifi" - android:summary="@string/wifi_wakeup_summary" /> + android:summary="@string/wifi_wakeup_summary" + settings:controller="com.android.settings.wifi.WifiWakeupPreferenceController"/> createPreferenceControllers(Context context) { - mWifiWakeupPreferenceController = new WifiWakeupPreferenceController(context, this, - getSettingsLifecycle()); mUseOpenWifiPreferenceController = new UseOpenWifiPreferenceController(context, this, getSettingsLifecycle()); final WifiManager wifiManager = (WifiManager) getSystemService(WIFI_SERVICE); final List controllers = new ArrayList<>(); - controllers.add(mWifiWakeupPreferenceController); controllers.add(new NotifyOpenNetworksPreferenceController(context, getSettingsLifecycle())); controllers.add(mUseOpenWifiPreferenceController); @@ -91,9 +88,17 @@ public class ConfigureWifiSettings extends DashboardFragment { return controllers; } + @Override + public void onAttach(Context context) { + super.onAttach(context); + + mWifiWakeupPreferenceController = use(WifiWakeupPreferenceController.class); + mWifiWakeupPreferenceController.setFragment(this); + } + @Override public void onActivityResult(int requestCode, int resultCode, Intent data) { - if (requestCode == WIFI_WAKEUP_REQUEST_CODE && mWifiWakeupPreferenceController != null) { + if (requestCode == WIFI_WAKEUP_REQUEST_CODE) { mWifiWakeupPreferenceController.onActivityResult(requestCode, resultCode); return; } diff --git a/src/com/android/settings/wifi/WifiWakeupPreferenceController.java b/src/com/android/settings/wifi/WifiWakeupPreferenceController.java index 11a58af4c4a..98c6bae4b99 100644 --- a/src/com/android/settings/wifi/WifiWakeupPreferenceController.java +++ b/src/com/android/settings/wifi/WifiWakeupPreferenceController.java @@ -25,7 +25,6 @@ import android.content.Intent; import android.content.IntentFilter; import android.location.LocationManager; import android.provider.Settings; -import android.text.TextUtils; import androidx.annotation.VisibleForTesting; import androidx.fragment.app.Fragment; @@ -34,108 +33,94 @@ import androidx.preference.PreferenceScreen; import androidx.preference.SwitchPreference; import com.android.settings.R; -import com.android.settings.core.PreferenceControllerMixin; -import com.android.settings.dashboard.DashboardFragment; +import com.android.settings.core.TogglePreferenceController; import com.android.settings.utils.AnnotationSpan; -import com.android.settingslib.core.AbstractPreferenceController; -import com.android.settingslib.core.lifecycle.Lifecycle; import com.android.settingslib.core.lifecycle.LifecycleObserver; import com.android.settingslib.core.lifecycle.events.OnPause; import com.android.settingslib.core.lifecycle.events.OnResume; /** - * {@link PreferenceControllerMixin} that controls whether the Wi-Fi Wakeup feature should be + * {@link TogglePreferenceController} that controls whether the Wi-Fi Wakeup feature should be * enabled. */ -public class WifiWakeupPreferenceController extends AbstractPreferenceController implements +public class WifiWakeupPreferenceController extends TogglePreferenceController implements LifecycleObserver, OnPause, OnResume { private static final String TAG = "WifiWakeupPrefController"; private static final String KEY_ENABLE_WIFI_WAKEUP = "enable_wifi_wakeup"; - private final Fragment mFragment; + private Fragment mFragment; @VisibleForTesting SwitchPreference mPreference; + @VisibleForTesting LocationManager mLocationManager; + private final BroadcastReceiver mLocationReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { updateState(mPreference); } }; + private final IntentFilter mLocationFilter = new IntentFilter(LocationManager.MODE_CHANGED_ACTION); - public WifiWakeupPreferenceController(Context context, DashboardFragment fragment, - Lifecycle lifecycle) { - super(context); - mFragment = fragment; + public WifiWakeupPreferenceController(Context context) { + super(context, KEY_ENABLE_WIFI_WAKEUP); mLocationManager = (LocationManager) context.getSystemService(Service.LOCATION_SERVICE); - lifecycle.addObserver(this); + } + + public void setFragment(Fragment hostFragment) { + mFragment = hostFragment; } @Override public void displayPreference(PreferenceScreen screen) { super.displayPreference(screen); - mPreference = screen.findPreference(KEY_ENABLE_WIFI_WAKEUP); - updateState(mPreference); + mPreference = screen.findPreference(getPreferenceKey()); } @Override - public boolean isAvailable() { - return true; + public int getAvailabilityStatus() { + return AVAILABLE; } @Override - public boolean handlePreferenceTreeClick(Preference preference) { - if (!TextUtils.equals(preference.getKey(), KEY_ENABLE_WIFI_WAKEUP)) { - return false; - } - if (!(preference instanceof SwitchPreference)) { - return false; - } + public boolean isChecked() { + return getWifiWakeupEnabled() + && getWifiScanningEnabled() + && mLocationManager.isLocationEnabled(); + } - // TODO(b/132391311): WifiWakeupPreferenceController is essentially reimplementing - // TogglePreferenceController. Refactor it into TogglePreferenceController. + @Override + public boolean setChecked(boolean isChecked) { + if (isChecked) { + if (mFragment == null) { + throw new IllegalStateException("No fragment to start activity"); + } - // Toggle wifi-wakeup setting between 1/0 based on its current state, and some other checks. - if (isWifiWakeupAvailable()) { - setWifiWakeupEnabled(false); - } else { if (!mLocationManager.isLocationEnabled()) { final Intent intent = new Intent(Settings.ACTION_LOCATION_SOURCE_SETTINGS); mFragment.startActivityForResult(intent, WIFI_WAKEUP_REQUEST_CODE); - return true; + return false; } else if (!getWifiScanningEnabled()) { showScanningDialog(); - } else { - setWifiWakeupEnabled(true); + return false; } } - updateState(mPreference); + setWifiWakeupEnabled(isChecked); return true; } @Override - public String getPreferenceKey() { - return KEY_ENABLE_WIFI_WAKEUP; - } - - @Override - public void updateState(Preference preference) { - if (!(preference instanceof SwitchPreference)) { - return; - } - final SwitchPreference enableWifiWakeup = (SwitchPreference) preference; - - enableWifiWakeup.setChecked(isWifiWakeupAvailable()); + public CharSequence getSummary() { if (!mLocationManager.isLocationEnabled()) { - preference.setSummary(getNoLocationSummary()); + return getNoLocationSummary(); } else { - preference.setSummary(R.string.wifi_wakeup_summary); + return mContext.getText(R.string.wifi_wakeup_summary); } } @@ -152,8 +137,8 @@ public class WifiWakeupPreferenceController extends AbstractPreferenceController } if (mLocationManager.isLocationEnabled() && getWifiScanningEnabled()) { setWifiWakeupEnabled(true); + updateState(mPreference); } - updateState(mPreference); } private boolean getWifiScanningEnabled() { @@ -173,15 +158,6 @@ public class WifiWakeupPreferenceController extends AbstractPreferenceController Settings.Global.WIFI_WAKEUP_ENABLED, 0) == 1; } - /** - * Wifi wakeup is available only when both location and Wi-Fi scanning are enabled. - */ - private boolean isWifiWakeupAvailable() { - return getWifiWakeupEnabled() - && getWifiScanningEnabled() - && mLocationManager.isLocationEnabled(); - } - private void setWifiWakeupEnabled(boolean enabled) { Settings.Global.putInt(mContext.getContentResolver(), Settings.Global.WIFI_WAKEUP_ENABLED, enabled ? 1 : 0); diff --git a/tests/robotests/src/com/android/settings/wifi/WifiWakeupPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/wifi/WifiWakeupPreferenceControllerTest.java index 81ba9a23c94..4266c84365f 100644 --- a/tests/robotests/src/com/android/settings/wifi/WifiWakeupPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/wifi/WifiWakeupPreferenceControllerTest.java @@ -35,7 +35,6 @@ import androidx.preference.SwitchPreference; import com.android.settings.R; import com.android.settings.dashboard.DashboardFragment; -import com.android.settingslib.core.lifecycle.Lifecycle; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -55,14 +54,13 @@ public class WifiWakeupPreferenceControllerTest { private LocationManager mLocationManager; @Mock private SwitchPreference mPreference; - @Mock - private Lifecycle mLifecycle; @Before public void setUp() { MockitoAnnotations.initMocks(this); mContext = RuntimeEnvironment.application; - mController = new WifiWakeupPreferenceController(mContext, mFragment, mLifecycle); + mController = new WifiWakeupPreferenceController(mContext); + mController.setFragment(mFragment); mController.mLocationManager = mLocationManager; mController.mPreference = mPreference; @@ -71,42 +69,29 @@ public class WifiWakeupPreferenceControllerTest { } @Test - public void handlePreferenceTreeClick_nonMatchingKey_shouldDoNothing() { - final SwitchPreference pref = new SwitchPreference(mContext); + public void setChecked_scanEnableLocationEnable_wifiWakeupEnable() { + Settings.Global.putInt(mContext.getContentResolver(), WIFI_WAKEUP_ENABLED, 0); + Settings.Global.putInt(mContext.getContentResolver(), WIFI_SCAN_ALWAYS_AVAILABLE, 1); + doReturn(true).when(mLocationManager).isLocationEnabled(); - assertThat(mController.handlePreferenceTreeClick(pref)).isFalse(); - } + mController.setChecked(true); - @Test - public void handlePreferenceTreeClick_nonMatchingType_shouldDoNothing() { - final Preference pref = new Preference(mContext); - pref.setKey(mController.getPreferenceKey()); - - assertThat(mController.handlePreferenceTreeClick(pref)).isFalse(); - } - - @Test - public void handlePreferenceTreeClick_matchingKeyAndType_shouldUpdateSetting() { - final SwitchPreference pref = new SwitchPreference(mContext); - pref.setChecked(true); - pref.setKey(mController.getPreferenceKey()); - - assertThat(mController.handlePreferenceTreeClick(pref)).isTrue(); assertThat(Settings.Global.getInt(mContext.getContentResolver(), WIFI_WAKEUP_ENABLED, 0)) .isEqualTo(1); } @Test - public void handlePreferenceTreeClick_wifiWakeupEnableScanningDisable_wifiWakeupEnable() { + public void updateState_wifiWakeupEnableScanningDisable_wifiWakeupDisabled() { + final SwitchPreference preference = new SwitchPreference(mContext); Settings.Global.putInt(mContext.getContentResolver(), WIFI_WAKEUP_ENABLED, 1); Settings.Global.putInt(mContext.getContentResolver(), WIFI_SCAN_ALWAYS_AVAILABLE, 0); doReturn(true).when(mLocationManager).isLocationEnabled(); - mController.handlePreferenceTreeClick(mPreference); - final boolean isWifiWakeupEnabled = Settings.Global.getInt(mContext.getContentResolver(), - Settings.Global.WIFI_WAKEUP_ENABLED, 0) == 1; + mController.updateState(preference); - assertThat(isWifiWakeupEnabled).isTrue(); + assertThat(preference.isChecked()).isFalse(); + assertThat(preference.getSummary()) + .isEqualTo(mContext.getString(R.string.wifi_wakeup_summary)); } @Test @@ -114,6 +99,7 @@ public class WifiWakeupPreferenceControllerTest { final SwitchPreference preference = new SwitchPreference(mContext); Settings.Global.putInt(mContext.getContentResolver(), WIFI_WAKEUP_ENABLED, 1); Settings.Global.putInt(mContext.getContentResolver(), WIFI_SCAN_ALWAYS_AVAILABLE, 1); + doReturn(true).when(mLocationManager).isLocationEnabled(); mController.updateState(preference); From 912f3558df98ce53f9360fb9f8f9c8e80fef608c Mon Sep 17 00:00:00 2001 From: Arc Wang Date: Thu, 16 May 2019 14:05:07 +0800 Subject: [PATCH 2/3] Refactor UseOpenWifiPreferenceController into TogglePreferenceController UseOpenWifiPreferenceController is essentially reimplementing TogglePreferenceController. Add below 2 changes for test failure of CodeInspectionTest 1. Remove some arguments from constructor of the controller 2. Declare the controller in xml instead of in code Bug: 132391311 Test: manual UseOpenWifiPreferenceControllerTest Change-Id: Ie52e79fd0bc66a227bd46f09c74d1fbec614f636 --- res/xml/wifi_configure_settings.xml | 3 +- .../settings/wifi/ConfigureWifiSettings.java | 10 +- .../wifi/UseOpenWifiPreferenceController.java | 96 ++++++++----------- .../UseOpenWifiPreferenceControllerTest.java | 40 ++------ 4 files changed, 55 insertions(+), 94 deletions(-) diff --git a/res/xml/wifi_configure_settings.xml b/res/xml/wifi_configure_settings.xml index a5152acb09e..996e8798e0a 100644 --- a/res/xml/wifi_configure_settings.xml +++ b/res/xml/wifi_configure_settings.xml @@ -31,7 +31,8 @@ android:key="use_open_wifi_automatically" android:icon="@drawable/ic_open_wifi_autoconnect" android:title="@string/use_open_wifi_automatically_title" - android:summary="@string/use_open_wifi_automatically_summary" /> + android:summary="@string/use_open_wifi_automatically_summary" + settings:controller="com.android.settings.wifi.UseOpenWifiPreferenceController"/> createPreferenceControllers(Context context) { - mUseOpenWifiPreferenceController = new UseOpenWifiPreferenceController(context, this, - getSettingsLifecycle()); final WifiManager wifiManager = (WifiManager) getSystemService(WIFI_SERVICE); final List controllers = new ArrayList<>(); controllers.add(new NotifyOpenNetworksPreferenceController(context, getSettingsLifecycle())); - controllers.add(mUseOpenWifiPreferenceController); controllers.add(new WifiInfoPreferenceController(context, getSettingsLifecycle(), wifiManager)); controllers.add(new WifiP2pPreferenceController(context, getSettingsLifecycle(), @@ -92,8 +89,12 @@ public class ConfigureWifiSettings extends DashboardFragment { public void onAttach(Context context) { super.onAttach(context); + mWifiWakeupPreferenceController = use(WifiWakeupPreferenceController.class); mWifiWakeupPreferenceController.setFragment(this); + + mUseOpenWifiPreferenceController = use(UseOpenWifiPreferenceController.class); + mUseOpenWifiPreferenceController.setFragment(this); } @Override @@ -102,8 +103,7 @@ public class ConfigureWifiSettings extends DashboardFragment { mWifiWakeupPreferenceController.onActivityResult(requestCode, resultCode); return; } - if (requestCode == UseOpenWifiPreferenceController.REQUEST_CODE_OPEN_WIFI_AUTOMATICALLY - && mUseOpenWifiPreferenceController != null) { + if (requestCode == UseOpenWifiPreferenceController.REQUEST_CODE_OPEN_WIFI_AUTOMATICALLY) { mUseOpenWifiPreferenceController.onActivityResult(requestCode, resultCode); return; } diff --git a/src/com/android/settings/wifi/UseOpenWifiPreferenceController.java b/src/com/android/settings/wifi/UseOpenWifiPreferenceController.java index eef22167bb2..27cf1e2e809 100644 --- a/src/com/android/settings/wifi/UseOpenWifiPreferenceController.java +++ b/src/com/android/settings/wifi/UseOpenWifiPreferenceController.java @@ -17,12 +17,10 @@ import android.text.TextUtils; import androidx.fragment.app.Fragment; import androidx.preference.Preference; import androidx.preference.PreferenceScreen; -import androidx.preference.SwitchPreference; import com.android.settings.R; import com.android.settings.core.PreferenceControllerMixin; -import com.android.settingslib.core.AbstractPreferenceController; -import com.android.settingslib.core.lifecycle.Lifecycle; +import com.android.settings.core.TogglePreferenceController; import com.android.settingslib.core.lifecycle.LifecycleObserver; import com.android.settingslib.core.lifecycle.events.OnPause; import com.android.settingslib.core.lifecycle.events.OnResume; @@ -30,10 +28,10 @@ import com.android.settingslib.core.lifecycle.events.OnResume; import java.util.List; /** - * {@link AbstractPreferenceController} that controls whether a user wants to enable the "use open - * networks automatically" feature provider by the current network recommendation provider. + * {@link TogglePreferenceController} that controls whether a user wants to enable the "use open + * networks automatically" feature provided by the current network recommendation provider. */ -public class UseOpenWifiPreferenceController extends AbstractPreferenceController +public class UseOpenWifiPreferenceController extends TogglePreferenceController implements PreferenceControllerMixin, Preference.OnPreferenceChangeListener, LifecycleObserver, OnResume, OnPause { public static final int REQUEST_CODE_OPEN_WIFI_AUTOMATICALLY = 400; @@ -41,7 +39,7 @@ public class UseOpenWifiPreferenceController extends AbstractPreferenceControlle private static final String KEY_USE_OPEN_WIFI_AUTOMATICALLY = "use_open_wifi_automatically"; private final ContentResolver mContentResolver; - private final Fragment mFragment; + private Fragment mFragment; private final NetworkScoreManager mNetworkScoreManager; private final SettingObserver mSettingObserver; @@ -49,17 +47,18 @@ public class UseOpenWifiPreferenceController extends AbstractPreferenceControlle private ComponentName mEnableUseWifiComponentName; private boolean mDoFeatureSupportedScorersExist; - public UseOpenWifiPreferenceController(Context context, Fragment fragment, - Lifecycle lifecycle) { - super(context); + public UseOpenWifiPreferenceController(Context context) { + super(context, KEY_USE_OPEN_WIFI_AUTOMATICALLY); mContentResolver = context.getContentResolver(); - mFragment = fragment; mNetworkScoreManager = (NetworkScoreManager) context.getSystemService(Context.NETWORK_SCORE_SERVICE); mSettingObserver = new SettingObserver(); updateEnableUseWifiComponentName(); checkForFeatureSupportedScorers(); - lifecycle.addObserver(this); + } + + public void setFragment(Fragment hostFragment) { + mFragment = hostFragment; } private void updateEnableUseWifiComponentName() { @@ -86,7 +85,7 @@ public class UseOpenWifiPreferenceController extends AbstractPreferenceControlle @Override public void displayPreference(PreferenceScreen screen) { super.displayPreference(screen); - mPreference = screen.findPreference(KEY_USE_OPEN_WIFI_AUTOMATICALLY); + mPreference = screen.findPreference(getPreferenceKey()); } @Override @@ -100,67 +99,54 @@ public class UseOpenWifiPreferenceController extends AbstractPreferenceControlle } @Override - public boolean isAvailable() { - return mDoFeatureSupportedScorersExist; - } - - @Override - public String getPreferenceKey() { - return KEY_USE_OPEN_WIFI_AUTOMATICALLY; + public int getAvailabilityStatus() { + return mDoFeatureSupportedScorersExist ? AVAILABLE : CONDITIONALLY_UNAVAILABLE; } @Override public void updateState(Preference preference) { - if (!(preference instanceof SwitchPreference)) { - return; - } - final SwitchPreference useOpenWifiPreference = (SwitchPreference) preference; + super.updateState(preference); - boolean isScorerSet = mNetworkScoreManager.getActiveScorerPackage() != null; - boolean doesActiveScorerSupportFeature = mEnableUseWifiComponentName != null; - - useOpenWifiPreference.setChecked(isSettingEnabled()); - useOpenWifiPreference.setVisible(isAvailable()); - useOpenWifiPreference.setEnabled(isScorerSet && doesActiveScorerSupportFeature); + final boolean isScorerSet = mNetworkScoreManager.getActiveScorerPackage() != null; + final boolean doesActiveScorerSupportFeature = mEnableUseWifiComponentName != null; + preference.setEnabled(isScorerSet && doesActiveScorerSupportFeature); if (!isScorerSet) { - useOpenWifiPreference.setSummary( - R.string.use_open_wifi_automatically_summary_scoring_disabled); + preference.setSummary(R.string.use_open_wifi_automatically_summary_scoring_disabled); } else if (!doesActiveScorerSupportFeature) { - useOpenWifiPreference.setSummary( + preference.setSummary( R.string.use_open_wifi_automatically_summary_scorer_unsupported_disabled); } else { - useOpenWifiPreference.setSummary(R.string.use_open_wifi_automatically_summary); + preference.setSummary(R.string.use_open_wifi_automatically_summary); } } @Override - public boolean onPreferenceChange(Preference preference, Object newValue) { - if (!TextUtils.equals(preference.getKey(), KEY_USE_OPEN_WIFI_AUTOMATICALLY) - || !isAvailable()) { - return false; - } - - if (isSettingEnabled()) { - Settings.Global.putString(mContentResolver, - Settings.Global.USE_OPEN_WIFI_PACKAGE, ""); - return true; - } - - Intent intent = new Intent(NetworkScoreManager.ACTION_CUSTOM_ENABLE); - intent.setComponent(mEnableUseWifiComponentName); - mFragment.startActivityForResult(intent, REQUEST_CODE_OPEN_WIFI_AUTOMATICALLY); - return false; // Updating state is done in onActivityResult. - } - - private boolean isSettingEnabled() { - String enabledUseOpenWifiPackage = Settings.Global.getString(mContentResolver, + public boolean isChecked() { + final String enabledUseOpenWifiPackage = Settings.Global.getString(mContentResolver, Settings.Global.USE_OPEN_WIFI_PACKAGE); - String currentUseOpenWifiPackage = mEnableUseWifiComponentName == null + final String currentUseOpenWifiPackage = mEnableUseWifiComponentName == null ? null : mEnableUseWifiComponentName.getPackageName(); return TextUtils.equals(enabledUseOpenWifiPackage, currentUseOpenWifiPackage); } + @Override + public boolean setChecked(boolean isChecked) { + if (isChecked) { + if (mFragment == null) { + throw new IllegalStateException("No fragment to start activity"); + } + + final Intent intent = new Intent(NetworkScoreManager.ACTION_CUSTOM_ENABLE); + intent.setComponent(mEnableUseWifiComponentName); + mFragment.startActivityForResult(intent, REQUEST_CODE_OPEN_WIFI_AUTOMATICALLY); + return false; // Updating state is done in onActivityResult. + } else { + Settings.Global.putString(mContentResolver, Settings.Global.USE_OPEN_WIFI_PACKAGE, ""); + return true; + } + } + public boolean onActivityResult(int requestCode, int resultCode) { if (requestCode != REQUEST_CODE_OPEN_WIFI_AUTOMATICALLY) { return false; diff --git a/tests/robotests/src/com/android/settings/wifi/UseOpenWifiPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/wifi/UseOpenWifiPreferenceControllerTest.java index 47f1c262460..beaa1a6b8ca 100644 --- a/tests/robotests/src/com/android/settings/wifi/UseOpenWifiPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/wifi/UseOpenWifiPreferenceControllerTest.java @@ -43,7 +43,6 @@ import androidx.preference.Preference; import androidx.preference.SwitchPreference; import com.android.settings.R; -import com.android.settingslib.core.lifecycle.Lifecycle; import com.google.common.collect.Lists; @@ -76,8 +75,6 @@ public class UseOpenWifiPreferenceControllerTest { sAppDataNoActivity = new NetworkScorerAppData(0, null, null, null, null); } - @Mock - private Lifecycle mLifecycle; @Mock private Fragment mFragment; @Mock @@ -97,7 +94,8 @@ public class UseOpenWifiPreferenceControllerTest { } private void createController() { - mController = new UseOpenWifiPreferenceController(mContext, mFragment, mLifecycle); + mController = new UseOpenWifiPreferenceController(mContext); + mController.setFragment(mFragment); } /** @@ -146,52 +144,28 @@ public class UseOpenWifiPreferenceControllerTest { } @Test - public void onPreferenceChange_nonMatchingKey_shouldDoNothing() { - createController(); - - final SwitchPreference pref = new SwitchPreference(mContext); - - assertThat(mController.onPreferenceChange(pref, null)).isFalse(); - } - - @Test - public void onPreferenceChange_notAvailable_shouldDoNothing() { - createController(); - - final Preference pref = new Preference(mContext); - pref.setKey(mController.getPreferenceKey()); - - assertThat(mController.onPreferenceChange(pref, null)).isFalse(); - } - - @Test - public void onPreferenceChange_matchingKeyAndAvailable_enableShouldStartEnableActivity() { + public void setChecked_withTrue_enableShouldStartEnableActivity() { setupScorers(Lists.newArrayList(sAppData, sAppDataNoActivity)); createController(); - final SwitchPreference pref = new SwitchPreference(mContext); - pref.setKey(mController.getPreferenceKey()); + mController.setChecked(true); - assertThat(mController.onPreferenceChange(pref, null)).isFalse(); verify(mFragment).startActivityForResult(mIntentCaptor.capture(), eq(REQUEST_CODE_OPEN_WIFI_AUTOMATICALLY)); - Intent activityIntent = mIntentCaptor.getValue(); + final Intent activityIntent = mIntentCaptor.getValue(); assertThat(activityIntent.getComponent()).isEqualTo(sEnableActivityComponent); assertThat(activityIntent.getAction()).isEqualTo(NetworkScoreManager.ACTION_CUSTOM_ENABLE); } @Test - public void onPreferenceChange_matchingKeyAndAvailable_disableShouldUpdateSetting() { + public void setChecked_withFalse_disableShouldUpdateSetting() { setupScorers(Lists.newArrayList(sAppData, sAppDataNoActivity)); Settings.Global.putString(mContext.getContentResolver(), USE_OPEN_WIFI_PACKAGE, sEnableActivityComponent.getPackageName()); - createController(); - final SwitchPreference pref = new SwitchPreference(mContext); - pref.setKey(mController.getPreferenceKey()); + mController.setChecked(false); - assertThat(mController.onPreferenceChange(pref, null)).isTrue(); assertThat(Settings.Global.getString(mContext.getContentResolver(), USE_OPEN_WIFI_PACKAGE)) .isEqualTo(""); } From 535e72a45b4f72384c0c3cc78def7152459b9127 Mon Sep 17 00:00:00 2001 From: Arc Wang Date: Thu, 16 May 2019 15:22:39 +0800 Subject: [PATCH 3/3] Refactor NotifyOpenNetworksPreferenceControllerTest into TogglePreferenceController NotifyOpenNetworksPreferenceControllerTest is essentially reimplementing TogglePreferenceController. Add below 2 changes for test failure of CodeInspectionTest 1. Remove some arguments from constructor of the controller 2. Declare the controller in xml instead of in code Bug: 132391311 Test: manual NotifyOpenNetworksPreferenceControllerTestTest Change-Id: Icda870ef0b90aacbacfe588b23d1b28d2b60941c --- res/xml/wifi_configure_settings.xml | 3 +- .../settings/wifi/ConfigureWifiSettings.java | 2 - ...otifyOpenNetworksPreferenceController.java | 51 ++++++------------- ...OpenNetworksPreferenceControllerTest.java} | 41 +++++++-------- 4 files changed, 36 insertions(+), 61 deletions(-) rename tests/robotests/src/com/android/settings/wifi/{NotifyOpenNetworkPreferenceControllerTest.java => NotifyOpenNetworksPreferenceControllerTest.java} (73%) diff --git a/res/xml/wifi_configure_settings.xml b/res/xml/wifi_configure_settings.xml index 996e8798e0a..c7e16ef6760 100644 --- a/res/xml/wifi_configure_settings.xml +++ b/res/xml/wifi_configure_settings.xml @@ -39,7 +39,8 @@ android:title="@string/wifi_notify_open_networks" android:icon="@drawable/ic_open_wifi_notifications" android:summary="@string/wifi_notify_open_networks_summary" - settings:keywords="@string/keywords_wifi_notify_open_networks"/> + settings:keywords="@string/keywords_wifi_notify_open_networks" + settings:controller="com.android.settings.wifi.NotifyOpenNetworksPreferenceController"/> createPreferenceControllers(Context context) { final WifiManager wifiManager = (WifiManager) getSystemService(WIFI_SERVICE); final List controllers = new ArrayList<>(); - controllers.add(new NotifyOpenNetworksPreferenceController(context, - getSettingsLifecycle())); controllers.add(new WifiInfoPreferenceController(context, getSettingsLifecycle(), wifiManager)); controllers.add(new WifiP2pPreferenceController(context, getSettingsLifecycle(), diff --git a/src/com/android/settings/wifi/NotifyOpenNetworksPreferenceController.java b/src/com/android/settings/wifi/NotifyOpenNetworksPreferenceController.java index a46a82873a5..6455f5b97cc 100644 --- a/src/com/android/settings/wifi/NotifyOpenNetworksPreferenceController.java +++ b/src/com/android/settings/wifi/NotifyOpenNetworksPreferenceController.java @@ -22,38 +22,34 @@ import android.database.ContentObserver; import android.net.Uri; import android.os.Handler; import android.provider.Settings; -import android.text.TextUtils; import androidx.preference.Preference; import androidx.preference.PreferenceScreen; -import androidx.preference.SwitchPreference; import com.android.settings.core.PreferenceControllerMixin; -import com.android.settingslib.core.AbstractPreferenceController; -import com.android.settingslib.core.lifecycle.Lifecycle; +import com.android.settings.core.TogglePreferenceController; import com.android.settingslib.core.lifecycle.LifecycleObserver; import com.android.settingslib.core.lifecycle.events.OnPause; import com.android.settingslib.core.lifecycle.events.OnResume; /** - * {@link AbstractPreferenceController} that controls whether we should notify user when open + * {@link TogglePreferenceController} that controls whether we should notify user when open * network is available. */ -public class NotifyOpenNetworksPreferenceController extends AbstractPreferenceController +public class NotifyOpenNetworksPreferenceController extends TogglePreferenceController implements PreferenceControllerMixin, LifecycleObserver, OnResume, OnPause { private static final String KEY_NOTIFY_OPEN_NETWORKS = "notify_open_networks"; private SettingObserver mSettingObserver; - public NotifyOpenNetworksPreferenceController(Context context, Lifecycle lifecycle) { - super(context); - lifecycle.addObserver(this); + public NotifyOpenNetworksPreferenceController(Context context) { + super(context, KEY_NOTIFY_OPEN_NETWORKS); } @Override public void displayPreference(PreferenceScreen screen) { super.displayPreference(screen); - mSettingObserver = new SettingObserver(screen.findPreference(KEY_NOTIFY_OPEN_NETWORKS)); + mSettingObserver = new SettingObserver(screen.findPreference(getPreferenceKey())); } @Override @@ -71,39 +67,24 @@ public class NotifyOpenNetworksPreferenceController extends AbstractPreferenceCo } @Override - public boolean isAvailable() { - return true; + public int getAvailabilityStatus() { + return AVAILABLE; } @Override - public boolean handlePreferenceTreeClick(Preference preference) { - if (!TextUtils.equals(preference.getKey(), KEY_NOTIFY_OPEN_NETWORKS)) { - return false; - } - if (!(preference instanceof SwitchPreference)) { - return false; - } + public boolean isChecked() { + return Settings.Global.getInt(mContext.getContentResolver(), + Settings.Global.WIFI_NETWORKS_AVAILABLE_NOTIFICATION_ON, 0) == 1; + } + + @Override + public boolean setChecked(boolean isChecked) { Settings.Global.putInt(mContext.getContentResolver(), Settings.Global.WIFI_NETWORKS_AVAILABLE_NOTIFICATION_ON, - ((SwitchPreference) preference).isChecked() ? 1 : 0); + isChecked ? 1 : 0); return true; } - @Override - public String getPreferenceKey() { - return KEY_NOTIFY_OPEN_NETWORKS; - } - - @Override - public void updateState(Preference preference) { - if (!(preference instanceof SwitchPreference)) { - return; - } - final SwitchPreference notifyOpenNetworks = (SwitchPreference) preference; - notifyOpenNetworks.setChecked(Settings.Global.getInt(mContext.getContentResolver(), - Settings.Global.WIFI_NETWORKS_AVAILABLE_NOTIFICATION_ON, 0) == 1); - } - class SettingObserver extends ContentObserver { private final Uri NETWORKS_AVAILABLE_URI = Settings.Global.getUriFor( Settings.Global.WIFI_NETWORKS_AVAILABLE_NOTIFICATION_ON); diff --git a/tests/robotests/src/com/android/settings/wifi/NotifyOpenNetworkPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/wifi/NotifyOpenNetworksPreferenceControllerTest.java similarity index 73% rename from tests/robotests/src/com/android/settings/wifi/NotifyOpenNetworkPreferenceControllerTest.java rename to tests/robotests/src/com/android/settings/wifi/NotifyOpenNetworksPreferenceControllerTest.java index 2cf7b21569d..f9271a65e4c 100644 --- a/tests/robotests/src/com/android/settings/wifi/NotifyOpenNetworkPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/wifi/NotifyOpenNetworksPreferenceControllerTest.java @@ -29,8 +29,6 @@ import android.provider.Settings; import androidx.preference.Preference; import androidx.preference.SwitchPreference; -import com.android.settingslib.core.lifecycle.Lifecycle; - import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -39,7 +37,7 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; @RunWith(RobolectricTestRunner.class) -public class NotifyOpenNetworkPreferenceControllerTest { +public class NotifyOpenNetworksPreferenceControllerTest { private Context mContext; private NotifyOpenNetworksPreferenceController mController; @@ -48,7 +46,7 @@ public class NotifyOpenNetworkPreferenceControllerTest { public void setUp() { MockitoAnnotations.initMocks(this); mContext = RuntimeEnvironment.application; - mController = new NotifyOpenNetworksPreferenceController(mContext, mock(Lifecycle.class)); + mController = new NotifyOpenNetworksPreferenceController(mContext); } @Test @@ -57,32 +55,29 @@ public class NotifyOpenNetworkPreferenceControllerTest { } @Test - public void handlePreferenceTreeClick_nonMatchingKey_shouldDoNothing() { - final SwitchPreference pref = new SwitchPreference(mContext); + public void setChecked_withTrue_shouldUpdateSetting() { + Settings.Global.putInt(mContext.getContentResolver(), + WIFI_NETWORKS_AVAILABLE_NOTIFICATION_ON, 0); - assertThat(mController.handlePreferenceTreeClick(pref)).isFalse(); - } + mController.setChecked(true); - @Test - public void handlePreferenceTreeClick_nonMatchingType_shouldDoNothing() { - final Preference pref = new Preference(mContext); - pref.setKey(mController.getPreferenceKey()); - - assertThat(mController.handlePreferenceTreeClick(pref)).isFalse(); - } - - @Test - public void handlePreferenceTreeClick_matchingKeyAndType_shouldUpdateSetting() { - final SwitchPreference pref = new SwitchPreference(mContext); - pref.setChecked(true); - pref.setKey(mController.getPreferenceKey()); - - assertThat(mController.handlePreferenceTreeClick(pref)).isTrue(); assertThat(Settings.Global.getInt(mContext.getContentResolver(), WIFI_NETWORKS_AVAILABLE_NOTIFICATION_ON, 0)) .isEqualTo(1); } + @Test + public void setChecked_withFalse_shouldUpdateSetting() { + Settings.Global.putInt(mContext.getContentResolver(), + WIFI_NETWORKS_AVAILABLE_NOTIFICATION_ON, 1); + + mController.setChecked(false); + + assertThat(Settings.Global.getInt(mContext.getContentResolver(), + WIFI_NETWORKS_AVAILABLE_NOTIFICATION_ON, 0)) + .isEqualTo(0); + } + @Test public void updateState_preferenceSetCheckedWhenSettingsAreEnabled() { final SwitchPreference preference = mock(SwitchPreference.class);