OSDN Git Service

Catch exception when failing to create slice
authorFan Zhang <zhfan@google.com>
Fri, 20 Apr 2018 19:53:25 +0000 (12:53 -0700)
committerFan Zhang <zhfan@google.com>
Fri, 20 Apr 2018 23:55:39 +0000 (23:55 +0000)
- Created a non-generic exception type when failing SliceData.build()
- Catch exception and log error instead of crash.
- Added a presubmit test

Fixes: 78347031
Test: robotests, atest
Change-Id: I06e528cb5e1cd328f82f9561553f3c4b5b0bed26
Merged-In: I06e528cb5e1cd328f82f9561553f3c4b5b0bed26

src/com/android/settings/slices/SliceData.java
src/com/android/settings/slices/SliceDataConverter.java
tests/robotests/src/com/android/settings/slices/SliceDataTest.java
tests/unit/src/com/android/settings/slices/SliceDataContractTest.java [new file with mode: 0644]

index 2caf6e6..1d81c9a 100644 (file)
@@ -212,19 +212,19 @@ public class SliceData {
 
         public SliceData build() {
             if (TextUtils.isEmpty(mKey)) {
-                throw new IllegalStateException("Key cannot be empty");
+                throw new InvalidSliceDataException("Key cannot be empty");
             }
 
             if (TextUtils.isEmpty(mTitle)) {
-                throw new IllegalStateException("Title cannot be empty");
+                throw new InvalidSliceDataException("Title cannot be empty");
             }
 
             if (TextUtils.isEmpty(mFragmentClassName)) {
-                throw new IllegalStateException("Fragment Name cannot be empty");
+                throw new InvalidSliceDataException("Fragment Name cannot be empty");
             }
 
             if (TextUtils.isEmpty(mPrefControllerClassName)) {
-                throw new IllegalStateException("Preference Controller cannot be empty");
+                throw new InvalidSliceDataException("Preference Controller cannot be empty");
             }
 
             return new SliceData(this);
@@ -234,4 +234,11 @@ public class SliceData {
             return mKey;
         }
     }
+
+    public static class InvalidSliceDataException extends RuntimeException {
+
+        public InvalidSliceDataException(String message) {
+            super(message);
+        }
+    }
 }
\ No newline at end of file
index 27724bf..c6378d5 100644 (file)
@@ -39,12 +39,12 @@ import android.util.Log;
 import android.util.Xml;
 import android.view.accessibility.AccessibilityManager;
 
-import com.android.settings.accessibility.AccessibilitySlicePreferenceController;
-import com.android.settings.core.PreferenceXmlParserUtils;
-import com.android.settings.core.PreferenceXmlParserUtils.MetadataFlag;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.settings.R;
 import com.android.settings.accessibility.AccessibilitySettings;
+import com.android.settings.accessibility.AccessibilitySlicePreferenceController;
+import com.android.settings.core.PreferenceXmlParserUtils;
+import com.android.settings.core.PreferenceXmlParserUtils.MetadataFlag;
 import com.android.settings.dashboard.DashboardFragment;
 import com.android.settings.overlay.FeatureFactory;
 import com.android.settings.search.DatabaseIndexingUtils;
@@ -215,6 +215,8 @@ class SliceDataConverter {
 
                 xmlSliceData.add(xmlSlice);
             }
+        } catch (SliceData.InvalidSliceDataException e) {
+            Log.w(TAG, "Invalid data when building SliceData for " + fragmentName, e);
         } catch (XmlPullParserException e) {
             Log.w(TAG, "XML Error parsing PreferenceScreen: ", e);
         } catch (IOException e) {
@@ -267,8 +269,11 @@ class SliceDataConverter {
                     .setTitle(title)
                     .setIcon(iconResource)
                     .setSliceType(SliceData.SliceType.SWITCH);
-
-            sliceData.add(sliceDataBuilder.build());
+            try {
+                sliceData.add(sliceDataBuilder.build());
+            } catch (SliceData.InvalidSliceDataException e) {
+                Log.w(TAG, "Invalid data when building a11y SliceData for " + flattenedName, e);
+            }
         }
 
         return sliceData;
index c2ab0af..2e04013 100644 (file)
@@ -67,7 +67,7 @@ public class SliceDataTest {
         assertThat(data.isPlatformDefined()).isEqualTo(IS_PLATFORM_DEFINED);
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = SliceData.InvalidSliceDataException.class)
     public void testBuilder_noKey_throwsIllegalStateException() {
         new SliceData.Builder()
                 .setTitle(TITLE)
@@ -80,7 +80,7 @@ public class SliceDataTest {
                 .build();
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = SliceData.InvalidSliceDataException.class)
     public void testBuilder_noTitle_throwsIllegalStateException() {
         new SliceData.Builder()
                 .setKey(KEY)
@@ -93,7 +93,7 @@ public class SliceDataTest {
                 .build();
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = SliceData.InvalidSliceDataException.class)
     public void testBuilder_noFragment_throwsIllegalStateException() {
         new SliceData.Builder()
                 .setKey(KEY)
@@ -106,7 +106,7 @@ public class SliceDataTest {
                 .build();
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = SliceData.InvalidSliceDataException.class)
     public void testBuilder_noPrefController_throwsIllegalStateException() {
         new SliceData.Builder()
                 .setKey(KEY)
diff --git a/tests/unit/src/com/android/settings/slices/SliceDataContractTest.java b/tests/unit/src/com/android/settings/slices/SliceDataContractTest.java
new file mode 100644 (file)
index 0000000..ae8d16e
--- /dev/null
@@ -0,0 +1,122 @@
+/*
+ * Copyright (C) 2018 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.slices;
+
+import static junit.framework.Assert.fail;
+
+import android.content.Context;
+import android.os.Bundle;
+import android.platform.test.annotations.Presubmit;
+import android.provider.SearchIndexableResource;
+import android.support.test.InstrumentationRegistry;
+import android.support.test.filters.MediumTest;
+import android.support.test.runner.AndroidJUnit4;
+import android.text.TextUtils;
+import android.util.Log;
+
+import com.android.settings.core.PreferenceXmlParserUtils;
+import com.android.settings.overlay.FeatureFactory;
+import com.android.settings.search.DatabaseIndexingUtils;
+import com.android.settings.search.Indexable;
+import com.android.settings.search.SearchIndexableResources;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.xmlpull.v1.XmlPullParserException;
+
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+@RunWith(AndroidJUnit4.class)
+@MediumTest
+public class SliceDataContractTest {
+
+    private static final String TAG = "SliceDataContractTest";
+    private Context mContext;
+
+    @Before
+    public void setUp() {
+        mContext = InstrumentationRegistry.getTargetContext();
+    }
+
+    @Test
+    @Presubmit
+    public void preferenceWithControllerMustHaveNonEmptyTitle()
+            throws IOException, XmlPullParserException {
+        final Set<String> nullTitleFragments = new HashSet<>();
+
+        final SearchIndexableResources resources =
+                FeatureFactory.getFactory(mContext).getSearchFeatureProvider()
+                        .getSearchIndexableResources();
+
+        for (Class<?> clazz : resources.getProviderValues()) {
+            verifyPreferenceTitle(nullTitleFragments, clazz);
+        }
+
+        if (!nullTitleFragments.isEmpty()) {
+            final StringBuilder error = new StringBuilder(
+                    "All preferences with a controller must have a non-empty title by default, "
+                            + "found empty title in the following fragments\n");
+            for (String c : nullTitleFragments) {
+                error.append(c).append("\n");
+            }
+            fail(error.toString());
+        }
+    }
+
+    private void verifyPreferenceTitle(Set<String> nullTitleFragments, Class<?> clazz)
+            throws IOException, XmlPullParserException {
+        if (clazz == null) {
+            return;
+        }
+        final String className = clazz.getName();
+        final Indexable.SearchIndexProvider provider =
+                DatabaseIndexingUtils.getSearchIndexProvider(clazz);
+
+        final List<SearchIndexableResource> resourcesToIndex =
+                provider.getXmlResourcesToIndex(mContext, true);
+
+        if (resourcesToIndex == null) {
+            Log.d(TAG, className + "is not providing SearchIndexableResource, skipping");
+            return;
+        }
+
+        for (SearchIndexableResource sir : resourcesToIndex) {
+            final List<Bundle> metadata = PreferenceXmlParserUtils.extractMetadata(mContext,
+                    sir.xmlResId,
+                    PreferenceXmlParserUtils.MetadataFlag.FLAG_INCLUDE_PREF_SCREEN
+                            | PreferenceXmlParserUtils.MetadataFlag.FLAG_NEED_PREF_TITLE
+                            | PreferenceXmlParserUtils.MetadataFlag.FLAG_NEED_PREF_CONTROLLER);
+
+            for (Bundle bundle : metadata) {
+                final String controller = bundle.getString(
+                        PreferenceXmlParserUtils.METADATA_CONTROLLER);
+                if (TextUtils.isEmpty(controller)) {
+                    continue;
+                }
+                final String title = bundle.getString(PreferenceXmlParserUtils.METADATA_TITLE);
+                if (TextUtils.isEmpty(title)) {
+                    nullTitleFragments.add(className);
+                }
+            }
+        }
+    }
+
+}
\ No newline at end of file