From 1ae1e171993bfc521636b9a664d1aee5c0e5abe9 Mon Sep 17 00:00:00 2001 From: Fan Zhang Date: Tue, 28 Nov 2017 16:52:53 -0800 Subject: [PATCH] Fix indexing in cast and zen mode settings - Fragment should either not implement serach provider or return a valid xml. Search provider with 0 resource is invalid. Change-Id: Ie87c739bf72c926cecf48d271c6c2d72459787c4 Fixes: 69864274 Test: robotests --- res/xml/wifi_display_settings.xml | 4 +- .../ZenModeAutomationSettings.java | 38 +++++++++++------ .../notification/ZenModeBehaviorSettings.java | 13 ++++++ .../notification/ZenModeRuleSettingsBase.java | 31 -------------- .../search/SearchIndexableResources.java | 7 +--- .../settings/wfd/WifiDisplaySettings.java | 23 +++++++++- ...randfather_not_implementing_index_provider | 4 +- .../grandfather_not_implementing_indexable | 1 - .../SearchIndexProviderCodeInspector.java | 42 ++++++++++++++++++- 9 files changed, 108 insertions(+), 55 deletions(-) diff --git a/res/xml/wifi_display_settings.xml b/res/xml/wifi_display_settings.xml index 2be31d2f7a5..7ad214e917f 100644 --- a/res/xml/wifi_display_settings.xml +++ b/res/xml/wifi_display_settings.xml @@ -14,7 +14,9 @@ limitations under the License. --> - diff --git a/src/com/android/settings/notification/ZenModeAutomationSettings.java b/src/com/android/settings/notification/ZenModeAutomationSettings.java index d096e8c7a15..582fb03b470 100644 --- a/src/com/android/settings/notification/ZenModeAutomationSettings.java +++ b/src/com/android/settings/notification/ZenModeAutomationSettings.java @@ -18,6 +18,7 @@ package com.android.settings.notification; import android.app.Fragment; import android.content.Context; +import android.provider.SearchIndexableResource; import android.service.notification.ConditionProviderService; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; @@ -78,17 +79,30 @@ public class ZenModeAutomationSettings extends ZenModeSettingsBase { */ public static final Indexable.SearchIndexProvider SEARCH_INDEX_DATA_PROVIDER = new BaseSearchIndexProvider() { - @Override - public List getNonIndexableKeys(Context context) { - final List keys = super.getNonIndexableKeys(context); - keys.add(KEY_ADD_RULE); - keys.add(KEY_AUTOMATIC_RULES); - return keys; - } - @Override - public List getPreferenceControllers(Context context) { - return buildPreferenceControllers(context, null, null); - } - }; + @Override + public List getXmlResourcesToIndex(Context context, + boolean enabled) { + final ArrayList result = new ArrayList<>(); + + final SearchIndexableResource sir = new SearchIndexableResource(context); + sir.xmlResId = R.xml.zen_mode_automation_settings; + result.add(sir); + return result; + } + + @Override + public List getNonIndexableKeys(Context context) { + final List keys = super.getNonIndexableKeys(context); + keys.add(KEY_ADD_RULE); + keys.add(KEY_AUTOMATIC_RULES); + return keys; + } + + @Override + public List getPreferenceControllers( + Context context) { + return buildPreferenceControllers(context, null, null); + } + }; } diff --git a/src/com/android/settings/notification/ZenModeBehaviorSettings.java b/src/com/android/settings/notification/ZenModeBehaviorSettings.java index dfe6786a071..b58ef86a547 100644 --- a/src/com/android/settings/notification/ZenModeBehaviorSettings.java +++ b/src/com/android/settings/notification/ZenModeBehaviorSettings.java @@ -17,6 +17,7 @@ package com.android.settings.notification; import android.content.Context; +import android.provider.SearchIndexableResource; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.settings.R; @@ -65,6 +66,18 @@ public class ZenModeBehaviorSettings extends ZenModeSettingsBase implements Inde */ public static final Indexable.SearchIndexProvider SEARCH_INDEX_DATA_PROVIDER = new BaseSearchIndexProvider() { + + @Override + public List getXmlResourcesToIndex(Context context, + boolean enabled) { + final ArrayList result = new ArrayList<>(); + + final SearchIndexableResource sir = new SearchIndexableResource(context); + sir.xmlResId = R.xml.zen_mode_behavior_settings; + result.add(sir); + return result; + } + @Override public List getNonIndexableKeys(Context context) { final List keys = super.getNonIndexableKeys(context); diff --git a/src/com/android/settings/notification/ZenModeRuleSettingsBase.java b/src/com/android/settings/notification/ZenModeRuleSettingsBase.java index 9eccfdc9990..069d38b8dd9 100644 --- a/src/com/android/settings/notification/ZenModeRuleSettingsBase.java +++ b/src/com/android/settings/notification/ZenModeRuleSettingsBase.java @@ -26,7 +26,6 @@ import android.content.DialogInterface.OnClickListener; import android.content.Intent; import android.net.Uri; import android.os.Bundle; -import android.provider.SearchIndexableResource; import android.service.notification.ConditionProviderService; import android.support.v7.preference.DropDownPreference; import android.support.v7.preference.Preference; @@ -44,15 +43,9 @@ import android.widget.Toast; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.settings.R; import com.android.settings.SettingsActivity; -import com.android.settings.search.BaseSearchIndexProvider; -import com.android.settings.search.Indexable; import com.android.settings.widget.SwitchBar; import com.android.settingslib.core.AbstractPreferenceController; -import java.util.Arrays; -import java.util.List; - -import java.util.Arrays; import java.util.List; public abstract class ZenModeRuleSettingsBase extends ZenModeSettingsBase @@ -62,8 +55,6 @@ public abstract class ZenModeRuleSettingsBase extends ZenModeSettingsBase private static final String KEY_RULE_NAME = "rule_name"; private static final String KEY_ZEN_MODE = "zen_mode"; - private static final String KEY_EVENT_RULE_SETTINGS = "zen_mode_event_rule_settings"; - private static final String KEY_SCHEDULE_RULE_SETTINGS = "zen_mode_schedule_rule_settings"; protected Context mContext; protected boolean mDisableListeners; @@ -306,26 +297,4 @@ public abstract class ZenModeRuleSettingsBase extends ZenModeSettingsBase } mDisableListeners = false; } - - /** - * For Search. - */ - public static final Indexable.SearchIndexProvider SEARCH_INDEX_DATA_PROVIDER = - new BaseSearchIndexProvider() { - @Override - public List getXmlResourcesToIndex( - Context context, boolean enabled) { - final SearchIndexableResource sir = new SearchIndexableResource(context); - // not indexable - return Arrays.asList(sir); - } - - @Override - public List getNonIndexableKeys(Context context) { - final List keys = super.getNonIndexableKeys(context); - keys.add(KEY_SCHEDULE_RULE_SETTINGS); - keys.add(KEY_EVENT_RULE_SETTINGS); - return keys; - } - }; } diff --git a/src/com/android/settings/search/SearchIndexableResources.java b/src/com/android/settings/search/SearchIndexableResources.java index 7512c1311ab..82ab0205800 100644 --- a/src/com/android/settings/search/SearchIndexableResources.java +++ b/src/com/android/settings/search/SearchIndexableResources.java @@ -39,7 +39,6 @@ import com.android.settings.datausage.DataUsageMeteredSettings; import com.android.settings.datausage.DataUsageSummary; import com.android.settings.deletionhelper.AutomaticStorageManagerSettings; import com.android.settings.development.DevelopmentSettingsDashboardFragment; -import com.android.settings.deviceinfo.Status; import com.android.settings.deviceinfo.StorageDashboardFragment; import com.android.settings.deviceinfo.StorageSettings; import com.android.settings.display.AmbientDisplaySettings; @@ -69,8 +68,6 @@ import com.android.settings.notification.ConfigureNotificationSettings; import com.android.settings.notification.SoundSettings; import com.android.settings.notification.ZenModeAutomationSettings; import com.android.settings.notification.ZenModeBehaviorSettings; -import com.android.settings.notification.ZenModeEventRuleSettings; -import com.android.settings.notification.ZenModeScheduleRuleSettings; import com.android.settings.notification.ZenModeSettings; import com.android.settings.print.PrintSettingsFragment; import com.android.settings.security.EncryptionAndCredential; @@ -86,6 +83,7 @@ import com.android.settings.tts.TextToSpeechSettings; import com.android.settings.tts.TtsEnginePreferenceFragment; import com.android.settings.users.UserSettings; import com.android.settings.wallpaper.WallpaperTypeSettings; +import com.android.settings.wfd.WifiDisplaySettings; import com.android.settings.wifi.ConfigureWifiSettings; import com.android.settings.wifi.WifiSettings; @@ -169,10 +167,9 @@ public final class SearchIndexableResources { addIndex(PowerUsageSummary.class); addIndex(BatterySaverSettings.class); addIndex(LockscreenDashboardFragment.class); + addIndex(WifiDisplaySettings.class); addIndex(ZenModeBehaviorSettings.class); addIndex(ZenModeAutomationSettings.class); - addIndex(ZenModeEventRuleSettings.class); - addIndex(ZenModeScheduleRuleSettings.class); } private SearchIndexableResources() { diff --git a/src/com/android/settings/wfd/WifiDisplaySettings.java b/src/com/android/settings/wfd/WifiDisplaySettings.java index 3fe438fda1d..5707e3e576b 100755 --- a/src/com/android/settings/wfd/WifiDisplaySettings.java +++ b/src/com/android/settings/wfd/WifiDisplaySettings.java @@ -36,7 +36,7 @@ import android.net.wifi.p2p.WifiP2pManager.Channel; import android.os.Bundle; import android.os.Handler; import android.os.Looper; -import android.os.ServiceManager; +import android.provider.SearchIndexableResource; import android.provider.Settings; import android.support.v14.preference.SwitchPreference; import android.support.v7.preference.ListPreference; @@ -63,6 +63,11 @@ import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.settings.R; import com.android.settings.SettingsPreferenceFragment; import com.android.settings.dashboard.SummaryLoader; +import com.android.settings.search.BaseSearchIndexProvider; +import com.android.settings.search.Indexable; + +import java.util.ArrayList; +import java.util.List; /** * The Settings screen for WifiDisplay configuration and connection management. @@ -72,7 +77,7 @@ import com.android.settings.dashboard.SummaryLoader; * on the system. In that case, the enable option will not be shown but other * remote display routes will continue to be made available. */ -public final class WifiDisplaySettings extends SettingsPreferenceFragment { +public final class WifiDisplaySettings extends SettingsPreferenceFragment implements Indexable { private static final String TAG = "WifiDisplaySettings"; private static final boolean DEBUG = false; @@ -823,4 +828,18 @@ public final class WifiDisplaySettings extends SettingsPreferenceFragment { public static final SummaryLoader.SummaryProviderFactory SUMMARY_PROVIDER_FACTORY = (activity, summaryLoader) -> new SummaryProvider(activity, summaryLoader); + + public static final Indexable.SearchIndexProvider SEARCH_INDEX_DATA_PROVIDER = + new BaseSearchIndexProvider() { + @Override + public List getXmlResourcesToIndex(Context context, + boolean enabled) { + final ArrayList result = new ArrayList<>(); + + final SearchIndexableResource sir = new SearchIndexableResource(context); + sir.xmlResId = R.xml.wifi_display_settings; + result.add(sir); + return result; + } + }; } diff --git a/tests/robotests/assets/grandfather_not_implementing_index_provider b/tests/robotests/assets/grandfather_not_implementing_index_provider index 6704fc3fb0e..ab1a2af2f32 100644 --- a/tests/robotests/assets/grandfather_not_implementing_index_provider +++ b/tests/robotests/assets/grandfather_not_implementing_index_provider @@ -19,4 +19,6 @@ com.android.settings.enterprise.ApplicationListFragment$AdminGrantedPermissionMi com.android.settings.enterprise.ApplicationListFragment$EnterpriseInstalledPackages com.android.settings.enterprise.EnterpriseSetDefaultAppsListFragment com.android.settings.wifi.tether.WifiTetherSettings -com.android.settings.wifi.SavedAccessPointsWifiSettings \ No newline at end of file +com.android.settings.wifi.SavedAccessPointsWifiSettings +com.android.settings.notification.ZenModeEventRuleSettings +com.android.settings.notification.ZenModeScheduleRuleSettings diff --git a/tests/robotests/assets/grandfather_not_implementing_indexable b/tests/robotests/assets/grandfather_not_implementing_indexable index 500a582839c..681e8f66ba0 100644 --- a/tests/robotests/assets/grandfather_not_implementing_indexable +++ b/tests/robotests/assets/grandfather_not_implementing_indexable @@ -50,7 +50,6 @@ com.android.settings.applications.InstalledAppDetails com.android.settings.applications.AppInfoDashboardFragment com.android.settings.accessibility.ToggleAccessibilityServicePreferenceFragment com.android.settings.print.PrintServiceSettingsFragment -com.android.settings.wfd.WifiDisplaySettings com.android.settings.deviceinfo.PrivateVolumeSettings com.android.settings.users.AppRestrictionsFragment com.android.settings.deviceinfo.PrivateVolumeUnmount diff --git a/tests/robotests/src/com/android/settings/search/SearchIndexProviderCodeInspector.java b/tests/robotests/src/com/android/settings/search/SearchIndexProviderCodeInspector.java index 3c51a90a51f..a8372d93d26 100644 --- a/tests/robotests/src/com/android/settings/search/SearchIndexProviderCodeInspector.java +++ b/tests/robotests/src/com/android/settings/search/SearchIndexProviderCodeInspector.java @@ -16,6 +16,9 @@ package com.android.settings.search; +import static com.google.common.truth.Truth.assertWithMessage; + +import android.provider.SearchIndexableResource; import android.util.ArraySet; import android.util.Log; @@ -23,13 +26,13 @@ import com.android.settings.SettingsPreferenceFragment; import com.android.settings.core.codeinspection.CodeInspector; import com.android.settings.dashboard.DashboardFragmentSearchIndexProviderInspector; +import org.robolectric.RuntimeEnvironment; + import java.lang.reflect.Field; import java.util.ArrayList; import java.util.List; import java.util.Set; -import static com.google.common.truth.Truth.assertWithMessage; - /** * {@link CodeInspector} to ensure fragments implement search components correctly. */ @@ -49,6 +52,9 @@ public class SearchIndexProviderCodeInspector extends CodeInspector { "Class containing " + DatabaseIndexingManager.FIELD_NAME_SEARCH_INDEX_DATA_PROVIDER + " must be added to " + SearchIndexableResources.class.getName() + " but these are not: \n"; + private static final String NOT_PROVIDING_VALID_RESOURCE_ERROR = + "SearchIndexableProvider must either provide no resource to index, or valid ones. " + + "But the followings contain resource with xml id = 0\n"; private final List notImplementingIndexableGrandfatherList; private final List notImplementingIndexProviderGrandfatherList; @@ -77,6 +83,7 @@ public class SearchIndexProviderCodeInspector extends CodeInspector { final Set notImplementingIndexProvider = new ArraySet<>(); final Set notInSearchProviderRegistry = new ArraySet<>(); final Set notSharingPreferenceControllers = new ArraySet<>(); + final Set notProvidingValidResource = new ArraySet<>(); for (Class clazz : mClasses) { if (!isConcreteSettingsClass(clazz)) { @@ -119,6 +126,10 @@ public class SearchIndexProviderCodeInspector extends CodeInspector { notInSearchProviderRegistry.add(className); } } + // Search provider must either don't provider resource xml, or provide valid ones. + if (!hasValidResourceFromProvider(clazz)) { + notProvidingValidResource.add(className); + } } // Build error messages @@ -131,6 +142,8 @@ public class SearchIndexProviderCodeInspector extends CodeInspector { notSharingPreferenceControllers); final String notInProviderRegistryError = buildErrorMessage(NOT_IN_INDEXABLE_PROVIDER_REGISTRY, notInSearchProviderRegistry); + final String notProvidingValidResourceError = buildErrorMessage( + NOT_PROVIDING_VALID_RESOURCE_ERROR, notProvidingValidResource); assertWithMessage(indexableError) .that(notImplementingIndexable) .isEmpty(); @@ -143,6 +156,9 @@ public class SearchIndexProviderCodeInspector extends CodeInspector { assertWithMessage(notInProviderRegistryError) .that(notInSearchProviderRegistry) .isEmpty(); + assertWithMessage(notProvidingValidResourceError) + .that(notProvidingValidResource) + .isEmpty(); assertNoObsoleteInGrandfatherList("grandfather_not_implementing_indexable", notImplementingIndexableGrandfatherList); assertNoObsoleteInGrandfatherList("grandfather_not_implementing_index_provider", @@ -168,6 +184,28 @@ public class SearchIndexProviderCodeInspector extends CodeInspector { } } + private boolean hasValidResourceFromProvider(Class clazz) { + try { + final Indexable.SearchIndexProvider provider = + DatabaseIndexingUtils.getSearchIndexProvider(clazz); + final List resources = provider.getXmlResourcesToIndex( + RuntimeEnvironment.application, true /* enabled */); + if (resources == null) { + // No resource, that's fine. + return true; + } + for (SearchIndexableResource res : resources) { + if (res.xmlResId == 0) { + // Invalid resource + return false; + } + } + } catch (Exception e) { + // Ignore. + } + return true; + } + private String buildErrorMessage(String errorSummary, Set errorClasses) { final StringBuilder error = new StringBuilder(errorSummary); for (String c : errorClasses) {