Handle a missed case in query the settings provider

bug:19361521

Change-Id: Ibf4731b5d665563bb87ef93a4cf63e4c4d2e46a4

4 files changed, 62 insertions, 22 deletions

diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
index ff2c004..5aac06d 100644
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
@@ -1228,6 +1228,7 @@ public class SettingsProvider extends ContentProvider {
                             && whereArgs.length == 1) {
                         name = whereArgs[0];
                         table = computeTableForSetting(uri, name);
+                        return;
                     } else if (where != null
                             && (WHERE_PATTERN_NO_PARAM_NO_BRACKETS.matcher(where).matches()
                                 || WHERE_PATTERN_NO_PARAM_IN_BRACKETS.matcher(where).matches())) {
@@ -1237,30 +1238,35 @@ public class SettingsProvider extends ContentProvider {
                                 where.lastIndexOf("\""));
                         name = where.substring(startIndex, endIndex);
                         table = computeTableForSetting(uri, name);
+                        return;
                     } else if (supportAll && where == null && whereArgs == null) {
                         name = null;
                         table = computeTableForSetting(uri, null);
-                    } else if (uri.getPathSegments().size() == 2
-                            && where == null && whereArgs == null) {
+                        return;
+                    }
+                } break;
+
+                case 2: {
+                    if (where == null && whereArgs == null) {
                         name = uri.getPathSegments().get(1);
                         table = computeTableForSetting(uri, name);
-                    } else {
-                        EventLogTags.writeUnsupportedSettingsQuery(
-                                uri.toSafeString(), where, Arrays.toString(whereArgs));
-                        throw new IllegalArgumentException("Only null where and args"
-                                + " or name=? where and a single arg or name='SOME_SETTING' "
-                                + "are supported uri: " + uri + " where: " + where + " args: "
-                                + Arrays.toString(whereArgs));
+                        return;
                     }
                 } break;
-
-                default: {
-                    throw new IllegalArgumentException("Invalid URI: " + uri);
-                }
             }
+
+            EventLogTags.writeUnsupportedSettingsQuery(
+                    uri.toSafeString(), where, Arrays.toString(whereArgs));
+            String message = String.format( "Supported SQL:\n"
+                    + "  uri content://some_table/some_property with null where and where args\n"
+                    + "  uri content://some_table with query name=? and single name as arg\n"
+                    + "  uri content://some_table with query name=some_name and null args\n"
+                    + "  but got - uri:%1s, where:%2s whereArgs:%3s", uri, where,
+                    Arrays.toString(whereArgs));
+            throw new IllegalArgumentException(message);
         }
 
-        public static String computeTableForSetting(Uri uri, String name) {
+        private static String computeTableForSetting(Uri uri, String name) {
             String table = getValidTableOrThrow(uri);
 
             if (name != null) {
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
index e63d220..833638c 100644
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
@@ -275,7 +275,8 @@ final class SettingsState {
 
         if (newSize > mMaxBytesPerAppPackage) {
             throw new IllegalStateException("You are adding too many system settings. "
-                    + "You should stop using system settings for app specific data.");
+                    + "You should stop using system settings for app specific data"
+                    + " package: " + packageName);
         }
 
         if (DEBUG) {
diff --git a/packages/SettingsProvider/test/src/com/android/providers/settings/BaseSettingsProviderTest.java b/packages/SettingsProvider/test/src/com/android/providers/settings/BaseSettingsProviderTest.java
index f713c33..8473db4 100644
--- a/packages/SettingsProvider/test/src/com/android/providers/settings/BaseSettingsProviderTest.java
+++ b/packages/SettingsProvider/test/src/com/android/providers/settings/BaseSettingsProviderTest.java
@@ -136,16 +136,27 @@ abstract class BaseSettingsProviderTest extends AndroidTestCase {
     }
 
     protected String queryStringViaProviderApi(int type, String name) {
-        return queryStringViaProviderApi(type, name, false);
+        return queryStringViaProviderApi(type, name, false, false);
     }
 
-    protected String queryStringViaProviderApi(int type, String name, boolean queryStringInQuotes) {
-        Uri uri = getBaseUriForType(type);
-
-        String queryString = queryStringInQuotes ? "(name=?)" : "name=?";
+    protected String queryStringViaProviderApi(int type, String name, boolean queryStringInQuotes,
+            boolean appendNameToUri) {
+        final Uri uri;
+        final String queryString;
+        final String[] queryArgs;
+
+        if (appendNameToUri) {
+            uri = Uri.withAppendedPath(getBaseUriForType(type), name);
+            queryString = null;
+            queryArgs = null;
+        } else {
+            uri = getBaseUriForType(type);
+            queryString = queryStringInQuotes ? "(name=?)" : "name=?";
+            queryArgs = new String[]{name};
+        }
 
         Cursor cursor = getContext().getContentResolver().query(uri, NAME_VALUE_COLUMNS,
-                queryString, new String[]{name}, null);
+                queryString, queryArgs, null);
 
         if (cursor == null) {
             return null;
diff --git a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsProviderTest.java b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsProviderTest.java
index cbfcbf5..b89fb10 100644
--- a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsProviderTest.java
+++ b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsProviderTest.java
@@ -184,6 +184,28 @@ public class SettingsProviderTest extends BaseSettingsProviderTest {
         doTestQueryStringInBracketsViaProviderApiForType(SETTING_TYPE_SYSTEM);
     }
 
+    public void testQueryStringWithAppendedNameToUriViaProviderApi() throws Exception {
+        // Make sure we have a clean slate.
+        deleteStringViaProviderApi(SETTING_TYPE_SYSTEM, FAKE_SETTING_NAME);
+
+        try {
+            // Insert the setting.
+            final Uri uri = insertStringViaProviderApi(SETTING_TYPE_SYSTEM, FAKE_SETTING_NAME,
+                    FAKE_SETTING_VALUE, false);
+            Uri expectUri = Uri.withAppendedPath(getBaseUriForType(SETTING_TYPE_SYSTEM),
+                    FAKE_SETTING_NAME);
+            assertEquals("Did not get expected Uri.", expectUri, uri);
+
+            // Make sure the first setting is there.
+            String firstValue = queryStringViaProviderApi(SETTING_TYPE_SYSTEM, FAKE_SETTING_NAME,
+                    false, true);
+            assertEquals("Setting must be present", FAKE_SETTING_VALUE, firstValue);
+        } finally {
+            // Clean up.
+            deleteStringViaProviderApi(SETTING_TYPE_SYSTEM, FAKE_SETTING_NAME);
+        }
+    }
+
     private void doTestQueryStringInBracketsViaProviderApiForType(int type) {
         // Make sure we have a clean slate.
         deleteStringViaProviderApi(type, FAKE_SETTING_NAME);
@@ -196,7 +218,7 @@ public class SettingsProviderTest extends BaseSettingsProviderTest {
             assertEquals("Did not get expected Uri.", expectUri, uri);
 
             // Make sure the first setting is there.
-            String firstValue = queryStringViaProviderApi(type, FAKE_SETTING_NAME, true);
+            String firstValue = queryStringViaProviderApi(type, FAKE_SETTING_NAME, true, false);
             assertEquals("Setting must be present", FAKE_SETTING_VALUE, firstValue);
         } finally {
             // Clean up.