Merge "Add code inspector to ensure fragments implements search."

src/com/android/settings/search/Index.java

@@ -110,6 +110,9 @@ public class Index {
 
     public static final String ENTRIES_SEPARATOR = "|";
 
+    static final String FIELD_NAME_SEARCH_INDEX_DATA_PROVIDER =
+            "SEARCH_INDEX_DATA_PROVIDER";
+
     // If you change the order of columns here, you SHOULD change the COLUMN_INDEX_XXX values
     private static final String[] SELECT_COLUMNS = new String[] {
             IndexColumns.DATA_RANK,               // 0
@@ -155,9 +158,6 @@ public class Index {
     private static final String HYPHEN = "-";
     private static final String SPACE = " ";
 
-    private static final String FIELD_NAME_SEARCH_INDEX_DATA_PROVIDER =
-            "SEARCH_INDEX_DATA_PROVIDER";
-
     private static final String NODE_NAME_PREFERENCE_SCREEN = "PreferenceScreen";
     private static final String NODE_NAME_CHECK_BOX_PREFERENCE = "CheckBoxPreference";
     private static final String NODE_NAME_LIST_PREFERENCE = "ListPreference";

tests/robotests/assets/grandfather_not_implementing_index_provider

@@ -0,0 +1,13 @@
+com.android.settings.connecteddevice.ConnectedDeviceDashboardFragment
+com.android.settings.gestures.PickupGestureSettings
+com.android.settings.notification.ConfigureNotificationSettings
+com.android.settings.language.LanguageAndRegionSettings
+com.android.settings.accounts.UserAndAccountDashboardFragment
+com.android.settings.gestures.DoubleTapScreenSettings
+com.android.settings.network.NetworkDashboardFragment
+com.android.settings.applications.AppAndNotificationDashboardFragment
+com.android.settings.gestures.SwipeToNotificationSettings
+com.android.settings.notification.ZenModePrioritySettings
+com.android.settings.gestures.DoubleTapPowerSettings
+com.android.settings.inputmethod.InputAndGestureSettings
+com.android.settings.gestures.DoubleTwistGestureSettings

tests/robotests/assets/grandfather_not_implementing_indexable

@@ -0,0 +1,90 @@
+com.android.settings.location.LocationMode
+com.android.settings.notification.ZenModeVisualInterruptionSettings
+com.android.settings.accessibility.ToggleScreenMagnificationPreferenceFragment
+com.android.settings.deviceinfo.SimStatus
+com.android.settings.deviceinfo.PrivateVolumeForget
+com.android.settings.inputmethod.SpellCheckersSettings
+com.android.settings.inputmethod.KeyboardLayoutPickerFragment
+com.android.settings.notification.ZenModeEventRuleSettings
+com.android.settings.fuelgauge.InactiveApps
+com.android.settings.accessibility.CaptionPropertiesFragment
+com.android.settings.accounts.ManageAccountsSettings
+com.android.settings.accessibility.AccessibilitySettingsForSetupWizard
+com.android.settings.deviceinfo.ImeiInformation
+com.android.settings.datausage.DataUsageList
+com.android.settings.vpn2.AppManagementFragment
+com.android.settings.display.NightDisplaySettings
+com.android.settings.vpn2.VpnSettings
+com.android.settings.fingerprint.FingerprintSettings$FingerprintSettingsFragment
+com.android.settings.applications.ProcessStatsDetail
+com.android.settings.wifi.WifiInfo
+com.android.settings.applications.VrListenerSettings
+com.android.settings.nfc.PaymentSettings
+com.android.settings.inputmethod.VirtualKeyboardFragment
+com.android.settings.bluetooth.DevicePickerFragment
+com.android.settings.inputmethod.UserDictionaryList
+com.android.settings.deviceinfo.Status
+com.android.settings.datausage.DataSaverSummary
+com.android.settings.notification.ChannelNotificationSettings
+com.android.settings.datausage.AppDataUsage
+com.android.settings.accessibility.FontSizePreferenceFragmentForSetupWizard
+com.android.settings.inputmethod.PhysicalKeyboardFragment
+com.android.settings.applications.ManageDomainUrls
+com.android.settings.applications.WriteSettingsDetails
+com.android.settings.location.LocationSettings
+com.android.settings.applications.ProcessStatsSummary
+com.android.settings.users.RestrictedProfileSettings
+com.android.settings.accounts.ChooseAccountActivity
+com.android.settings.accounts.ManagedProfileSettings
+com.android.settings.notification.ZenModeAutomationSettings
+com.android.settings.accessibility.ToggleAutoclickPreferenceFragment
+com.android.settings.applications.AppLaunchSettings
+com.android.settings.fuelgauge.BatterySaverSettings
+com.android.settings.location.ScanningSettings
+com.android.settings.tts.TextToSpeechSettings
+com.android.settings.applications.ProcessStatsUi
+com.android.settings.fuelgauge.PowerUsageDetail
+com.android.settings.notification.ZenModeScheduleRuleSettings
+com.android.settings.datausage.BillingCycleSettings
+com.android.settings.notification.NotificationStation
+com.android.settings.print.PrintJobSettingsFragment
+com.android.settings.applications.SpecialAccessSettings
+com.android.settings.accessibility.ToggleScreenReaderPreferenceFragmentForSetupWizard
+com.android.settings.accounts.AccountSyncSettings
+com.android.settings.notification.RedactionInterstitial$RedactionInterstitialFragment
+com.android.settings.inputmethod.InputMethodAndSubtypeEnabler
+com.android.settings.inputmethod.AvailableVirtualKeyboardFragment
+com.android.settings.applications.DrawOverlayDetails
+com.android.settings.tts.TtsEngineSettingsFragment
+com.android.settings.backup.ToggleBackupSettingFragment
+com.android.settings.users.UserDetailsSettings
+com.android.settings.datausage.UnrestrictedDataAccess
+com.android.settings.accessibility.ToggleScreenMagnificationPreferenceFragmentForSetupWizard
+com.android.settings.fuelgauge.BatteryHistoryDetail
+com.android.settings.fuelgauge.PowerUsageSummary
+com.android.settings.applications.RunningServices
+com.android.settings.wifi.p2p.WifiP2pSettings
+com.android.settings.applications.ManageAssist
+com.android.settings.applications.ConfirmConvertToFbe
+com.android.settings.deviceinfo.PublicVolumeSettings
+com.android.settings.applications.InstalledAppDetails
+com.android.settings.accessibility.ToggleAccessibilityServicePreferenceFragment
+com.android.settings.print.PrintServiceSettingsFragment
+com.android.settings.wfd.WifiDisplaySettings
+com.android.settings.notification.AppNotificationSettings
+com.android.settings.deviceinfo.PrivateVolumeSettings
+com.android.settings.users.AppRestrictionsFragment
+com.android.settings.deviceinfo.PrivateVolumeUnmount
+com.android.settings.deletionhelper.AutomaticStorageManagerSettings
+com.android.settings.notification.ZenAccessSettings
+com.android.settings.accessibility.ToggleFontSizePreferenceFragment
+com.android.settings.accessibility.ToggleGlobalGesturePreferenceFragment
+com.android.settings.wifi.ConfigureWifiSettings
+com.android.settings.wifi.AdvancedWifiSettings
+com.android.settings.applications.PremiumSmsAccess
+com.android.settings.applications.UsageAccessDetails
+com.android.settings.applications.AppStorageSettings
+com.android.settings.notification.NotificationAccessSettings
+com.android.settings.notification.ZenModeSettings
+com.android.settings.accessibility.ToggleDaltonizerPreferenceFragment
+com.android.settings.applications.ConvertToFbe

tests/robotests/assets/grandfather_not_implementing_instrumentable

@@ -0,0 +1,4 @@
+com.android.settings.deletionhelper.ActivationWarningFragment
+com.android.settings.core.lifecycle.ObservableDialogFragment
+com.android.settings.applications.AppOpsCategory
+com.android.settings.inputmethod.UserDictionaryLocalePicker

tests/robotests/src/com/android/settings/SettingsRobolectricTestRunner.java

@@ -15,7 +15,6 @@
  */
 package com.android.settings;
 
-import java.util.List;
 import org.junit.runners.model.InitializationError;
 import org.robolectric.RobolectricTestRunner;
 import org.robolectric.annotation.Config;
@@ -23,6 +22,8 @@ import org.robolectric.manifest.AndroidManifest;
 import org.robolectric.res.Fs;
 import org.robolectric.res.ResourcePath;
 
+import java.util.List;
+
 /**
  * Custom test runner for the testing of BluetoothPairingDialogs. This is needed because the
  * default behavior for robolectric is just to grab the resource directory in the target package.
@@ -47,7 +48,7 @@ public class SettingsRobolectricTestRunner extends RobolectricTestRunner {
         final String appRoot = "packages/apps/Settings";
         final String manifestPath = appRoot + "/AndroidManifest.xml";
         final String resDir = appRoot + "/res";
-        final String assetsDir = appRoot + "/assets";
+        final String assetsDir = appRoot + config.assetDir();
 
         // By adding any resources from libraries we need to the AndroidManifest, we can access
         // them from within the parallel universe's resource loader.

tests/robotests/src/com/android/settings/core/codeinspection/CodeInspectionTest.java

@@ -19,6 +19,7 @@ package com.android.settings.core.codeinspection;
 import com.android.settings.SettingsRobolectricTestRunner;
 import com.android.settings.TestConfig;
 import com.android.settings.core.instrumentation.InstrumentableFragmentCodeInspector;
+import com.android.settings.search.SearchIndexProviderCodeInspector;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -32,7 +33,8 @@ import java.util.List;
  * for conformance.
  */
 @RunWith(SettingsRobolectricTestRunner.class)
-@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION)
+@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION,
+        assetDir = "/tests/robotests/assets")
 public class CodeInspectionTest {
 
     private List<Class<?>> mClasses;
@@ -45,5 +47,6 @@ public class CodeInspectionTest {
     @Test
     public void runCodeInspections() {
         new InstrumentableFragmentCodeInspector(mClasses).run();
+        new SearchIndexProviderCodeInspector(mClasses).run();
     }
 }

tests/robotests/src/com/android/settings/core/codeinspection/CodeInspector.java

@@ -16,6 +16,15 @@
 
 package com.android.settings.core.codeinspection;
 
+import com.google.common.truth.Truth;
+
+import org.junit.Assert;
+import org.robolectric.shadows.ShadowApplication;
+
+import java.io.BufferedReader;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.lang.reflect.Modifier;
 import java.util.List;
 
 /**
@@ -24,7 +33,10 @@ import java.util.List;
  */
 public abstract class CodeInspector {
 
-    public static final String PACKAGE_NAME = "com.android.settings";
+    protected static final String PACKAGE_NAME = "com.android.settings";
+
+    protected static final String TEST_CLASS_SUFFIX = "Test";
+    private static final String TEST_INNER_CLASS_SIGNATURE = "Test$";
 
     protected final List<Class<?>> mClasses;
 
@@ -36,4 +48,41 @@ public abstract class CodeInspector {
      * Code inspection runner method.
      */
     public abstract void run();
+
+    protected boolean isConcreteSettingsClass(Class clazz) {
+        // Abstract classes
+        if (Modifier.isAbstract(clazz.getModifiers())) {
+            return false;
+        }
+        final String packageName = clazz.getPackage().getName();
+        // Classes that are not in Settings
+        if (!packageName.contains(PACKAGE_NAME + ".")) {
+            return false;
+        }
+        final String className = clazz.getName();
+        // Classes from tests
+        if (className.endsWith(TEST_CLASS_SUFFIX)) {
+            return false;
+        }
+        if (className.contains(TEST_INNER_CLASS_SIGNATURE)) {
+            return false;
+        }
+        return true;
+    }
+
+    protected void initializeGrandfatherList(List<String> grandfather, String filename) {
+        try {
+            final InputStream in = ShadowApplication.getInstance().getApplicationContext()
+                    .getAssets()
+                    .open(filename);
+            BufferedReader reader = new BufferedReader(new InputStreamReader(in));
+            String line;
+            while ((line = reader.readLine()) != null) {
+                grandfather.add(line);
+            }
+        } catch (Exception e) {
+            throw new IllegalArgumentException("Error initializing grandfather " + filename, e);
+        }
+
+    }
 }

tests/robotests/src/com/android/settings/core/instrumentation/InstrumentableFragmentCodeInspector.java

@@ -19,20 +19,8 @@ package com.android.settings.core.instrumentation;
 import android.app.Fragment;
 import android.util.ArraySet;
 
-import com.android.settings.ChooseLockPassword;
-import com.android.settings.ChooseLockPattern;
-import com.android.settings.CredentialCheckResultTracker;
-import com.android.settings.CustomDialogPreference;
-import com.android.settings.CustomEditTextPreference;
-import com.android.settings.CustomListPreference;
-import com.android.settings.RestrictedListPreference;
-import com.android.settings.applications.AppOpsCategory;
 import com.android.settings.core.codeinspection.CodeInspector;
-import com.android.settings.core.lifecycle.ObservableDialogFragment;
-import com.android.settings.deletionhelper.ActivationWarningFragment;
-import com.android.settings.inputmethod.UserDictionaryLocalePicker;
 
-import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
@@ -44,30 +32,13 @@ import static com.google.common.truth.Truth.assertWithMessage;
  */
 public class InstrumentableFragmentCodeInspector extends CodeInspector {
 
-    private static final String TEST_CLASS_SUFFIX = "Test";
+    private final List<String> grandfather_notImplmentingInstrumentable;
-
-    private static final List<String> whitelist;
-
-    static {
-        whitelist = new ArrayList<>();
-        whitelist.add(
-                CustomEditTextPreference.CustomPreferenceDialogFragment.class.getName());
-        whitelist.add(
-                CustomListPreference.CustomListPreferenceDialogFragment.class.getName());
-        whitelist.add(
-                RestrictedListPreference.RestrictedListPreferenceDialogFragment.class.getName());
-        whitelist.add(ChooseLockPassword.SaveAndFinishWorker.class.getName());
-        whitelist.add(ChooseLockPattern.SaveAndFinishWorker.class.getName());
-        whitelist.add(ActivationWarningFragment.class.getName());
-        whitelist.add(ObservableDialogFragment.class.getName());
-        whitelist.add(CustomDialogPreference.CustomPreferenceDialogFragment.class.getName());
-        whitelist.add(AppOpsCategory.class.getName());
-        whitelist.add(UserDictionaryLocalePicker.class.getName());
-        whitelist.add(CredentialCheckResultTracker.class.getName());
-    }
 
     public InstrumentableFragmentCodeInspector(List<Class<?>> classes) {
         super(classes);
+        grandfather_notImplmentingInstrumentable = new ArrayList<>();
+        initializeGrandfatherList(grandfather_notImplmentingInstrumentable,
+                "grandfather_not_implementing_instrumentable");
     }
 
     @Override
@@ -75,24 +46,14 @@ public class InstrumentableFragmentCodeInspector extends CodeInspector {
         final Set<String> broken = new ArraySet<>();
 
         for (Class clazz : mClasses) {
-            // Skip abstract classes.
+            if (!isConcreteSettingsClass(clazz)) {
-            if (Modifier.isAbstract(clazz.getModifiers())) {
-                continue;
-            }
-            final String packageName = clazz.getPackage().getName();
-            // Skip classes that are not in Settings.
-            if (!packageName.contains(PACKAGE_NAME + ".")) {
                 continue;
             }
             final String className = clazz.getName();
-            // Skip classes from tests.
-            if (className.endsWith(TEST_CLASS_SUFFIX)) {
-                continue;
-            }
             // If it's a fragment, it must also be instrumentable.
             if (Fragment.class.isAssignableFrom(clazz)
                     && !Instrumentable.class.isAssignableFrom(clazz)
-                    && !whitelist.contains(className)) {
+                    && !grandfather_notImplmentingInstrumentable.contains(className)) {
                 broken.add(className);
             }
         }

tests/robotests/src/com/android/settings/search/SearchIndexProviderCodeInspector.java

@@ -0,0 +1,107 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.settings.search;
+
+import android.util.ArraySet;
+import android.util.Log;
+
+import com.android.settings.SettingsPreferenceFragment;
+import com.android.settings.core.codeinspection.CodeInspector;
+
+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.
+ */
+public class SearchIndexProviderCodeInspector extends CodeInspector {
+    private static final String TAG = "SearchCodeInspector";
+
+    private final List<String> notImplementingIndexableWhitelist;
+    private final List<String> notImplementingIndexProviderWhitelist;
+
+    public SearchIndexProviderCodeInspector(List<Class<?>> classes) {
+        super(classes);
+        notImplementingIndexableWhitelist = new ArrayList<>();
+        notImplementingIndexProviderWhitelist = new ArrayList<>();
+        initializeGrandfatherList(notImplementingIndexableWhitelist,
+                "grandfather_not_implementing_indexable");
+        initializeGrandfatherList(notImplementingIndexProviderWhitelist,
+                "grandfather_not_implementing_index_provider");
+    }
+
+    @Override
+    public void run() {
+        final Set<String> notImplementingIndexable = new ArraySet<>();
+        final Set<String> notImplementingIndexProvider = new ArraySet<>();
+
+        for (Class clazz : mClasses) {
+            if (!isConcreteSettingsClass(clazz)) {
+                continue;
+            }
+            final String className = clazz.getName();
+            // Skip fragments if it's not SettingsPreferenceFragment.
+            if (!SettingsPreferenceFragment.class.isAssignableFrom(clazz)) {
+                continue;
+            }
+            // If it's a SettingsPreferenceFragment, it must also be Indexable.
+            final boolean implementsIndexable = Indexable.class.isAssignableFrom(clazz);
+            if (!implementsIndexable && !notImplementingIndexableWhitelist.contains(className)) {
+                notImplementingIndexable.add(className);
+            }
+            // If it implements Indexable, it must also implement the index provider field.
+            if (implementsIndexable && !hasSearchIndexProvider(clazz)
+                    && !notImplementingIndexProviderWhitelist.contains(className)) {
+                notImplementingIndexProvider.add(className);
+            }
+        }
+
+        // Build error messages
+        final StringBuilder indexableError = new StringBuilder(
+                "SettingsPreferenceFragment should implement Indexable, but these are not:\n");
+        for (String c : notImplementingIndexable) {
+            indexableError.append(c).append("\n");
+        }
+        final StringBuilder indexProviderError = new StringBuilder(
+                "Indexable should have public field " + Index.FIELD_NAME_SEARCH_INDEX_DATA_PROVIDER
+                        + " but these are not:\n");
+        for (String c : notImplementingIndexProvider) {
+            indexProviderError.append(c).append("\n");
+        }
+
+        assertWithMessage(indexableError.toString())
+                .that(notImplementingIndexable.isEmpty())
+                .isTrue();
+        assertWithMessage(indexProviderError.toString())
+                .that(notImplementingIndexProvider.isEmpty())
+                .isTrue();
+    }
+
+    private boolean hasSearchIndexProvider(Class clazz) {
+        try {
+            final Field f = clazz.getField(Index.FIELD_NAME_SEARCH_INDEX_DATA_PROVIDER);
+            return f != null;
+        } catch (NoSuchFieldException e) {
+            Log.e(TAG, "error fetching search provider from class " + clazz.getName());
+            return false;
+        }
+    }
+}