Eliminate duplicate keys in pref xmls.

- Remove additional_system_update pref device_info page, we don't need
  it.
- Update keys in xml and Preference controller, and search index
  provider.
- Clean up in ScreenZoomPreference, it's anti-pattern to set fragment in
  constructor.
- Whitelist 2 that are super hard to remove.

Change-Id: Ibab6e2cb074513042a2ae007d9085aa64046eec8
Fixes: 67852637
Test: uniquePreferenceTest
This commit is contained in:
Fan Zhang
2017-10-27 13:33:48 -07:00
parent 34f7b5af59
commit 57ef92a810
20 changed files with 108 additions and 223 deletions

View File

@@ -49,7 +49,8 @@
android:title="@string/title_font_size" />
<com.android.settings.display.ScreenZoomPreference
android:key="screen_zoom"
android:key="accessibility_settings_screen_zoom"
android:fragment="com.android.settings.display.ScreenZoomSettings"
android:title="@string/screen_zoom_title" />
<Preference

View File

@@ -18,14 +18,6 @@
android:key="device_info_pref_screen"
android:title="@string/about_settings">
<!-- System update settings - launches activity -->
<Preference android:key="additional_system_update_settings"
android:title="@string/additional_system_update_settings_list_item_title">
<intent android:action="android.intent.action.MAIN"
android:targetPackage="@string/additional_system_update"
android:targetClass="@string/additional_system_update_menu" />
</Preference>
<!-- Device status - launches activity -->
<Preference android:key="status_info"
android:title="@string/device_status"

View File

@@ -78,8 +78,9 @@
settings:keywords="@string/keywords_display_font_size" />
<com.android.settings.display.ScreenZoomPreference
android:key="screen_zoom"
android:key="display_settings_screen_zoom"
android:title="@string/screen_zoom_title"
android:fragment="com.android.settings.display.ScreenZoomSettings"
settings:keywords="@string/screen_zoom_keywords" />
<Preference

View File

@@ -25,7 +25,7 @@
android:summary="@string/summary_placeholder" />
<com.android.settingslib.RestrictedSwitchPreference
android:key="add_users_when_locked"
android:key="security_lockscreen_add_users_when_locked"
android:title="@string/user_add_on_lockscreen_menu" />
<com.android.settingslib.RestrictedPreference

View File

@@ -77,7 +77,7 @@
android:summary="@string/switch_off_text"
android:fragment="com.android.settings.ScreenPinningSettings"/>
<Preference android:key="usage_access"
<Preference android:key="security_misc_usage_access"
android:title="@string/usage_access_title"
android:fragment="com.android.settings.applications.manageapplications.ManageApplications">
<extra

View File

@@ -92,7 +92,7 @@
</Preference>
<Preference
android:key="usage_access"
android:key="special_app_usage_access"
android:title="@string/usage_access"
android:fragment="com.android.settings.applications.manageapplications.ManageApplications"
settings:keywords="@string/keywords_write_settings">

View File

@@ -15,5 +15,5 @@
-->
<PreferenceScreen xmlns:android="http://schemas.android.com/apk/res/android"
android:key="usage_access"
android:key="usage_access_screen"
android:title="@string/usage_access_title"/>

View File

@@ -47,7 +47,7 @@
android:order="104"/>
<com.android.settingslib.RestrictedSwitchPreference
android:key="add_users_when_locked"
android:key="user_settings_add_users_when_locked"
android:title="@string/user_add_on_lockscreen_menu"
android:singleLineTitle="false"
android:order="105"/>

View File

@@ -26,7 +26,6 @@ import android.util.FeatureFlagUtils;
import com.android.internal.logging.nano.MetricsProto.MetricsEvent;
import com.android.settings.dashboard.DashboardFragment;
import com.android.settings.dashboard.SummaryLoader;
import com.android.settings.deviceinfo.AdditionalSystemUpdatePreferenceController;
import com.android.settings.deviceinfo.BasebandVersionPreferenceController;
import com.android.settings.deviceinfo.BuildNumberPreferenceController;
import com.android.settings.deviceinfo.DeviceModelPreferenceController;
@@ -153,7 +152,6 @@ public class DeviceInfoSettings extends DashboardFragment implements Indexable {
final List<AbstractPreferenceController> controllers = new ArrayList<>();
controllers.add(
new BuildNumberPreferenceController(context, activity, fragment, lifecycle));
controllers.add(new AdditionalSystemUpdatePreferenceController(context));
controllers.add(new ManualPreferenceController(context));
controllers.add(new FeedbackPreferenceController(fragment, context));
controllers.add(new KernelVersionPreferenceController(context));

View File

@@ -50,7 +50,7 @@ public class DisplaySettings extends DashboardFragment {
private static final String TAG = "DisplaySettings";
public static final String KEY_AUTO_BRIGHTNESS = "auto_brightness";
public static final String KEY_DISPLAY_SIZE = "screen_zoom";
public static final String KEY_DISPLAY_SIZE = "display_settings_screen_zoom";
private static final String KEY_SCREEN_TIMEOUT = "screen_timeout";
private static final String KEY_AMBIENT_DISPLAY = "ambient_display";

View File

@@ -46,7 +46,6 @@ import com.android.internal.content.PackageMonitor;
import com.android.internal.logging.nano.MetricsProto.MetricsEvent;
import com.android.internal.view.RotationPolicy;
import com.android.internal.view.RotationPolicy.RotationPolicyListener;
import com.android.settings.DisplaySettings;
import com.android.settings.R;
import com.android.settings.SettingsPreferenceFragment;
import com.android.settings.Utils;
@@ -219,7 +218,6 @@ public class AccessibilitySettings extends SettingsPreferenceFragment implements
* on non-accelerated platforms due to the performance implications.
*
* @param context The current context
* @return
*/
public static boolean isColorTransformAccelerated(Context context) {
return context.getResources()
@@ -734,6 +732,8 @@ public class AccessibilitySettings extends SettingsPreferenceFragment implements
public static final SearchIndexProvider SEARCH_INDEX_DATA_PROVIDER =
new BaseSearchIndexProvider() {
public static final String KEY_DISPLAY_SIZE = "accessibility_settings_screen_zoom";
@Override
public List<SearchIndexableResource> getXmlResourcesToIndex(Context context,
boolean enabled) {
@@ -749,7 +749,7 @@ public class AccessibilitySettings extends SettingsPreferenceFragment implements
List<String> keys = super.getNonIndexableKeys(context);
// Duplicates in Display
keys.add(FONT_SIZE_PREFERENCE_SCREEN);
keys.add(DisplaySettings.KEY_DISPLAY_SIZE);
keys.add(KEY_DISPLAY_SIZE);
// Duplicates in Language & Input
keys.add(TTS_SETTINGS_PREFERENCE);

View File

@@ -31,14 +31,13 @@ public class AddUserWhenLockedPreferenceController extends AbstractPreferenceCon
implements PreferenceControllerMixin, Preference.OnPreferenceChangeListener,
LifecycleObserver, OnPause, OnResume {
private static final String KEY_ADD_USER_WHEN_LOCKED = "add_users_when_locked";
private RestrictedSwitchPreference mAddUserWhenLocked;
private UserCapabilities mUserCaps;
private final String mPrefKey;
private final UserCapabilities mUserCaps;
private boolean mShouldUpdateUserList;
public AddUserWhenLockedPreferenceController(Context context) {
public AddUserWhenLockedPreferenceController(Context context, String key) {
super(context);
mPrefKey = key;
mUserCaps = UserCapabilities.create(context);
}
@@ -80,6 +79,6 @@ public class AddUserWhenLockedPreferenceController extends AbstractPreferenceCon
@Override
public String getPreferenceKey() {
return KEY_ADD_USER_WHEN_LOCKED;
return mPrefKey;
}
}

View File

@@ -38,6 +38,7 @@ import java.util.List;
public class UserAndAccountDashboardFragment extends DashboardFragment {
private static final String TAG = "UserAndAccountDashboard";
private static final String KEY_ADD_USER_WHEN_LOCKED = "user_settings_add_users_when_locked";
@Override
public int getMetricsCategory() {
@@ -64,7 +65,8 @@ public class UserAndAccountDashboardFragment extends DashboardFragment {
final List<AbstractPreferenceController> controllers = new ArrayList<>();
controllers.add(new EmergencyInfoPreferenceController(context));
AddUserWhenLockedPreferenceController addUserWhenLockedPrefController =
new AddUserWhenLockedPreferenceController(context);
new AddUserWhenLockedPreferenceController(
context, KEY_ADD_USER_WHEN_LOCKED);
controllers.add(addUserWhenLockedPrefController);
getLifecycle().addObserver(addUserWhenLockedPrefController);
controllers.add(new AutoSyncDataPreferenceController(context, this));

View File

@@ -34,10 +34,6 @@ public class ScreenZoomPreference extends Preference {
android.support.v7.preference.R.attr.preferenceStyle,
android.R.attr.preferenceStyle));
if (TextUtils.isEmpty(getFragment())) {
setFragment("com.android.settings.display.ScreenZoomSettings");
}
final DisplayDensityUtils density = new DisplayDensityUtils(context);
final int defaultIndex = density.getCurrentIndex();
if (defaultIndex < 0) {

View File

@@ -49,6 +49,9 @@ public class LockscreenDashboardFragment extends DashboardFragment
@VisibleForTesting
static final String KEY_LOCK_SCREEN_NOTIFICATON_WORK_PROFILE =
"security_setting_lock_screen_notif_work";
@VisibleForTesting
static final String KEY_ADD_USER_FROM_LOCK_SCREEN =
"security_lockscreen_add_users_when_locked";
private OwnerInfoPreferenceController mOwnerInfoPreferenceController;
@@ -84,7 +87,7 @@ public class LockscreenDashboardFragment extends DashboardFragment
lifecycle.addObserver(notificationController);
controllers.add(notificationController);
final AddUserWhenLockedPreferenceController addUserWhenLockedController =
new AddUserWhenLockedPreferenceController(context);
new AddUserWhenLockedPreferenceController(context, KEY_ADD_USER_FROM_LOCK_SCREEN);
lifecycle.addObserver(addUserWhenLockedController);
controllers.add(addUserWhenLockedController);
mOwnerInfoPreferenceController =
@@ -111,13 +114,22 @@ public class LockscreenDashboardFragment extends DashboardFragment
}
@Override
public List<AbstractPreferenceController> getPreferenceControllers(Context context) {
public List<AbstractPreferenceController> getPreferenceControllers(
Context context) {
final List<AbstractPreferenceController> controllers = new ArrayList<>();
controllers.add(new LockScreenNotificationPreferenceController(context));
controllers.add(new AddUserWhenLockedPreferenceController(context));
controllers.add(new AddUserWhenLockedPreferenceController(context,
KEY_ADD_USER_FROM_LOCK_SCREEN));
controllers.add(new OwnerInfoPreferenceController(
context, null /* fragment */, null /* lifecycle */));
return controllers;
}
@Override
public List<String> getNonIndexableKeys(Context context) {
final List<String> niks = super.getNonIndexableKeys(context);
niks.add(KEY_ADD_USER_FROM_LOCK_SCREEN);
return niks;
}
};
}

View File

@@ -1,5 +0,0 @@
add_users_when_locked
additional_system_update_settings
dashboard_tile_placeholder
screen_zoom
usage_access

View File

@@ -15,16 +15,24 @@
*/
package com.android.settings.accounts;
import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Answers.RETURNS_DEEP_STUBS;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import android.content.Context;
import android.content.pm.UserInfo;
import android.os.UserManager;
import android.provider.Settings.Global;
import android.support.v7.preference.Preference;
import android.support.v7.preference.PreferenceScreen;
import com.android.settingslib.RestrictedSwitchPreference;
import com.android.settings.testutils.SettingsRobolectricTestRunner;
import com.android.settings.TestConfig;
import com.android.settings.testutils.SettingsRobolectricTestRunner;
import com.android.settingslib.RestrictedSwitchPreference;
import org.junit.Before;
import org.junit.Test;
@@ -34,14 +42,6 @@ import org.mockito.MockitoAnnotations;
import org.robolectric.annotation.Config;
import org.robolectric.shadows.ShadowApplication;
import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Answers.RETURNS_DEEP_STUBS;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@RunWith(SettingsRobolectricTestRunner.class)
@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION)
public class AddUserWhenLockedPreferenceControllerTest {
@@ -62,7 +62,7 @@ public class AddUserWhenLockedPreferenceControllerTest {
ShadowApplication shadowContext = ShadowApplication.getInstance();
shadowContext.setSystemService(Context.USER_SERVICE, mUserManager);
mContext = shadowContext.getApplicationContext();
mController = new AddUserWhenLockedPreferenceController(mContext);
mController = new AddUserWhenLockedPreferenceController(mContext, "fake_key");
}
@Test

View File

@@ -1,120 +0,0 @@
package com.android.settings.search;
import static com.google.common.truth.Truth.assertThat;
import android.content.Context;
import android.provider.SearchIndexableResource;
import android.util.ArraySet;
import com.android.settings.DateTimeSettings;
import com.android.settings.R;
import com.android.settings.SecuritySettings;
import com.android.settings.TestConfig;
import com.android.settings.core.codeinspection.CodeInspector;
import com.android.settings.datausage.DataPlanUsageSummary;
import com.android.settings.datausage.DataUsageSummary;
import com.android.settings.testutils.SettingsRobolectricTestRunner;
import com.android.settings.testutils.XmlTestUtils;
import com.android.settings.testutils.shadow.SettingsShadowResources;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.annotation.Config;
@RunWith(SettingsRobolectricTestRunner.class)
@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION,
assetDir = "/tests/robotests/assets")
public class DataIntegrityTest {
@Test
@Config(shadows = {
SettingsShadowResources.class,
SettingsShadowResources.SettingsShadowTheme.class,
})
public void testIndexableResources_uniqueKeys() {
final Context context = RuntimeEnvironment.application;
// Aggregation of all keys
final Set<String> masterKeys = new ArraySet<>();
// Aggregation of the incorrectly duplicate keys
final Set<String> duplicateKeys = new ArraySet<>();
// Keys for a specific page
final Set<String> pageKeys = new ArraySet<>();
// List of all Xml preferences
final Set<Integer> xmlList = new ArraySet<>();
// Duplicates we know about.
List<String> grandfatheredKeys = new ArrayList<>();
CodeInspector.initializeGrandfatherList(grandfatheredKeys,
"whitelist_duplicate_index_key");
// Get a list of all Xml.
for (SearchIndexableResource val : SearchIndexableResources.values()) {
final int xmlResId = val.xmlResId;
if (xmlResId != 0) {
xmlList.add(xmlResId);
} else {
// Take class and get all keys
final Class clazz = DatabaseIndexingUtils.getIndexableClass(val.className);
// Skip classes that are invalid or cannot be mocked. Add them as special Xml below.
if (clazz == null
|| clazz == DateTimeSettings.class
|| clazz == DataPlanUsageSummary.class
|| clazz == DataUsageSummary.class
|| clazz == SecuritySettings.class) {
continue;
}
Indexable.SearchIndexProvider provider = DatabaseIndexingUtils
.getSearchIndexProvider(clazz);
if (provider == null) {
continue;
}
List<SearchIndexableResource> subXml =
provider.getXmlResourcesToIndex(context, true);
if (subXml == null) {
continue;
}
for (SearchIndexableResource resource : subXml) {
final int subXmlResId = resource.xmlResId;
if (subXmlResId != 0) {
xmlList.add(subXmlResId);
}
}
}
}
addSpecialXml(xmlList);
// Get keys from all Xml and check for duplicates.
for (Integer xmlResId : xmlList) {
// Get all keys to be indexed
final List<String> prefKeys = XmlTestUtils.getKeysFromPreferenceXml(context, xmlResId);
pageKeys.addAll(prefKeys);
// Find all already-existing keys.
pageKeys.retainAll(masterKeys);
// Keep list of offending duplicate keys.
duplicateKeys.addAll(pageKeys);
// Add all keys to master key list.
masterKeys.addAll(prefKeys);
pageKeys.clear();
}
assertThat(duplicateKeys).containsExactlyElementsIn(grandfatheredKeys);
}
/**
* Add XML preferences from Fragments which have issues being instantiated in robolectric.
*/
private void addSpecialXml(Set<Integer> xmlList) {
xmlList.add(R.xml.date_time_prefs);
xmlList.add(R.xml.data_usage);
xmlList.add(R.xml.data_usage_cellular);
xmlList.add(R.xml.data_usage_wifi);
xmlList.add(R.xml.security_settings_misc);
}
}

View File

@@ -16,8 +16,10 @@
package com.android.settings.security;
import com.android.settings.testutils.SettingsRobolectricTestRunner;
import static com.google.common.truth.Truth.assertThat;
import com.android.settings.TestConfig;
import com.android.settings.testutils.SettingsRobolectricTestRunner;
import com.android.settings.testutils.XmlTestUtils;
import org.junit.Test;
@@ -27,8 +29,6 @@ import org.robolectric.annotation.Config;
import java.util.List;
import static com.google.common.truth.Truth.assertThat;
@RunWith(SettingsRobolectricTestRunner.class)
@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION)
public class LockscreenDashboardFragmentTest {

View File

@@ -54,10 +54,19 @@ public class UniquePreferenceTest {
private static final String TAG = "UniquePreferenceTest";
private static final List<String> SUPPORTED_PREF_TYPES = Arrays.asList(
"Preference", "PreferenceCategory", "PreferenceScreen");
private static final List<String> WHITELISTED_DUPLICATE_KEYS = Arrays.asList(
"owner_info_settings", // Lock screen message in security - multiple xml files
// contain this because security page is constructed by
// combining small xml chunks. Eventually the page
// should be formed as one single xml and this entry
// should be removed.
"dashboard_tile_placeholder" // This is the placeholder pref for injecting dynamic
// tiles.
);
private Context mContext;
@Before
public void setUp() {
mContext = InstrumentationRegistry.getTargetContext();
@@ -148,7 +157,7 @@ public class UniquePreferenceTest {
nullKeyClasses.add(page.className);
continue;
}
if (uniqueKeys.contains(key)) {
if (uniqueKeys.contains(key) && !WHITELISTED_DUPLICATE_KEYS.contains(key)) {
Log.e(TAG, "Every preference key must unique; found " + nodeName
+ " in " + page.className
+ " at " + parser.getPositionDescription());