Merge "Don't populate a setting instead of defaulting to 0 when not set." into oc-dev

2 files changed, 178 insertions, 39 deletions

diff --git a/services/core/java/com/android/server/am/CoreSettingsObserver.java b/services/core/java/com/android/server/am/CoreSettingsObserver.java
index 73a17c613c4d..160c753d9540 100644
--- a/services/core/java/com/android/server/am/CoreSettingsObserver.java
+++ b/services/core/java/com/android/server/am/CoreSettingsObserver.java
@@ -21,6 +21,9 @@ import android.database.ContentObserver;
 import android.net.Uri;
 import android.os.Bundle;
 import android.provider.Settings;
+
+import com.android.internal.annotations.VisibleForTesting;
+
 import java.util.HashMap;
 import java.util.Map;
 
@@ -34,11 +37,14 @@ final class CoreSettingsObserver extends ContentObserver {
     private static final String LOG_TAG = CoreSettingsObserver.class.getSimpleName();
 
     // mapping form property name to its type
-    private static final Map<String, Class<?>> sSecureSettingToTypeMap = new HashMap<
+    @VisibleForTesting
+    static final Map<String, Class<?>> sSecureSettingToTypeMap = new HashMap<
             String, Class<?>>();
-    private static final Map<String, Class<?>> sSystemSettingToTypeMap = new HashMap<
+    @VisibleForTesting
+    static final Map<String, Class<?>> sSystemSettingToTypeMap = new HashMap<
             String, Class<?>>();
-    private static final Map<String, Class<?>> sGlobalSettingToTypeMap = new HashMap<
+    @VisibleForTesting
+    static final Map<String, Class<?>> sGlobalSettingToTypeMap = new HashMap<
             String, Class<?>>();
     static {
         sSecureSettingToTypeMap.put(Settings.Secure.LONG_PRESS_TIMEOUT, int.class);
@@ -101,51 +107,31 @@ final class CoreSettingsObserver extends ContentObserver {
         }
     }
 
-    private void populateSettings(Bundle snapshot, Map<String, Class<?>> map) {
+    @VisibleForTesting
+    void populateSettings(Bundle snapshot, Map<String, Class<?>> map) {
         Context context = mActivityManagerService.mContext;
         for (Map.Entry<String, Class<?>> entry : map.entrySet()) {
             String setting = entry.getKey();
+            final String value;
+            if (map == sSecureSettingToTypeMap) {
+                value = Settings.Secure.getString(context.getContentResolver(), setting);
+            } else if (map == sSystemSettingToTypeMap) {
+                value = Settings.System.getString(context.getContentResolver(), setting);
+            } else {
+                value = Settings.Global.getString(context.getContentResolver(), setting);
+            }
+            if (value == null) {
+                continue;
+            }
             Class<?> type = entry.getValue();
             if (type == String.class) {
-                final String value;
-                if (map == sSecureSettingToTypeMap) {
-                    value = Settings.Secure.getString(context.getContentResolver(), setting);
-                } else if (map == sSystemSettingToTypeMap) {
-                    value = Settings.System.getString(context.getContentResolver(), setting);
-                } else {
-                    value = Settings.Global.getString(context.getContentResolver(), setting);
-                }
                 snapshot.putString(setting, value);
             } else if (type == int.class) {
-                final int value;
-                if (map == sSecureSettingToTypeMap) {
-                    value = Settings.Secure.getInt(context.getContentResolver(), setting, 0);
-                } else if (map == sSystemSettingToTypeMap) {
-                    value = Settings.System.getInt(context.getContentResolver(), setting, 0);
-                } else {
-                    value = Settings.Global.getInt(context.getContentResolver(), setting, 0);
-                }
-                snapshot.putInt(setting, value);
+                snapshot.putInt(setting, Integer.parseInt(value));
             } else if (type == float.class) {
-                final float value;
-                if (map == sSecureSettingToTypeMap) {
-                    value = Settings.Secure.getFloat(context.getContentResolver(), setting, 0);
-                } else if (map == sSystemSettingToTypeMap) {
-                    value = Settings.System.getFloat(context.getContentResolver(), setting, 0);
-                } else {
-                    value = Settings.Global.getFloat(context.getContentResolver(), setting, 0);
-                }
-                snapshot.putFloat(setting, value);
+                snapshot.putFloat(setting, Float.parseFloat(value));
             } else if (type == long.class) {
-                final long value;
-                if (map == sSecureSettingToTypeMap) {
-                    value = Settings.Secure.getLong(context.getContentResolver(), setting, 0);
-                } else if (map == sSystemSettingToTypeMap) {
-                    value = Settings.System.getLong(context.getContentResolver(), setting, 0);
-                } else {
-                    value = Settings.Global.getLong(context.getContentResolver(), setting, 0);
-                }
-                snapshot.putLong(setting, value);
+                snapshot.putLong(setting, Long.parseLong(value));
             }
         }
     }
diff --git a/services/tests/servicestests/src/com/android/server/am/CoreSettingsObserverTest.java b/services/tests/servicestests/src/com/android/server/am/CoreSettingsObserverTest.java
new file mode 100644
index 000000000000..19defe158122
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/am/CoreSettingsObserverTest.java
@@ -0,0 +1,153 @@
+/*
+ * Copyright (C) 2017 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.server.am;
+
+import static com.android.server.am.ActivityManagerService.Injector;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.mockito.Mockito.when;
+
+import android.content.Context;
+import android.os.Bundle;
+import android.os.Handler;
+import android.provider.Settings;
+import android.support.test.InstrumentationRegistry;
+import android.support.test.filters.SmallTest;
+import android.support.test.runner.AndroidJUnit4;
+import android.test.mock.MockContentResolver;
+
+import com.android.internal.util.test.FakeSettingsProvider;
+import com.android.server.AppOpsService;
+
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+import java.io.File;
+
+/**
+ * Test class for {@link CoreSettingsObserver}.
+ *
+ * To run the tests, use
+ *
+ * runtest -c com.android.server.am.CoreSettingsObserverTest frameworks-services
+ *
+ * or the following steps:
+ *
+ * Build: m FrameworksServicesTests
+ * Install: adb install -r \
+ *     ${ANDROID_PRODUCT_OUT}/data/app/FrameworksServicesTests/FrameworksServicesTests.apk
+ * Run: adb shell am instrument -e class com.android.server.am.CoreSettingsObserverTest -w \
+ *     com.android.frameworks.servicestests/android.support.test.runner.AndroidJUnitRunner
+ */
+@SmallTest
+@RunWith(AndroidJUnit4.class)
+public class CoreSettingsObserverTest {
+    private static final String TEST_SETTING_SECURE_INT = "secureInt";
+    private static final String TEST_SETTING_GLOBAL_FLOAT = "globalFloat";
+    private static final String TEST_SETTING_SYSTEM_STRING = "systemString";
+
+    private static final int TEST_INT = 111;
+    private static final float TEST_FLOAT = 3.14f;
+    private static final String TEST_STRING = "testString";
+
+    private ActivityManagerService mAms;
+    @Mock private Context mContext;
+
+    private MockContentResolver mContentResolver;
+    private CoreSettingsObserver mCoreSettingsObserver;
+
+    @BeforeClass
+    public static void setupOnce() {
+        CoreSettingsObserver.sSecureSettingToTypeMap.put(TEST_SETTING_SECURE_INT, int.class);
+        CoreSettingsObserver.sGlobalSettingToTypeMap.put(TEST_SETTING_GLOBAL_FLOAT, float.class);
+        CoreSettingsObserver.sSystemSettingToTypeMap.put(TEST_SETTING_SYSTEM_STRING, String.class);
+    }
+
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+
+        final Context originalContext = InstrumentationRegistry.getContext();
+        when(mContext.getApplicationInfo()).thenReturn(originalContext.getApplicationInfo());
+        mContentResolver = new MockContentResolver(mContext);
+        mContentResolver.addProvider(Settings.AUTHORITY, new FakeSettingsProvider());
+        when(mContext.getContentResolver()).thenReturn(mContentResolver);
+
+        mAms = new ActivityManagerService(new TestInjector());
+        mCoreSettingsObserver = new CoreSettingsObserver(mAms);
+    }
+
+    @Test
+    public void testPopulateSettings() {
+        Settings.Secure.putInt(mContentResolver, TEST_SETTING_SECURE_INT, TEST_INT);
+        Settings.Global.putFloat(mContentResolver, TEST_SETTING_GLOBAL_FLOAT, TEST_FLOAT);
+        Settings.System.putString(mContentResolver, TEST_SETTING_SYSTEM_STRING, TEST_STRING);
+
+        final Bundle settingsBundle = getPopulatedBundle();
+
+        assertEquals("Unexpected value of " + TEST_SETTING_SECURE_INT,
+                TEST_INT, settingsBundle.getInt(TEST_SETTING_SECURE_INT));
+        assertEquals("Unexpected value of " + TEST_SETTING_GLOBAL_FLOAT,
+                TEST_FLOAT, settingsBundle.getFloat(TEST_SETTING_GLOBAL_FLOAT), 0);
+        assertEquals("Unexpected value of " + TEST_SETTING_SYSTEM_STRING,
+                TEST_STRING, settingsBundle.getString(TEST_SETTING_SYSTEM_STRING));
+    }
+
+    @Test
+    public void testPopulateSettings_settingNotSet() {
+        final Bundle settingsBundle = getPopulatedBundle();
+
+        assertFalse("Bundle should not contain " + TEST_SETTING_SECURE_INT,
+                settingsBundle.containsKey(TEST_SETTING_SECURE_INT));
+        assertFalse("Bundle should not contain " + TEST_SETTING_GLOBAL_FLOAT,
+                settingsBundle.containsKey(TEST_SETTING_GLOBAL_FLOAT));
+        assertFalse("Bundle should not contain " + TEST_SETTING_SYSTEM_STRING,
+                settingsBundle.containsKey(TEST_SETTING_SYSTEM_STRING));
+    }
+
+    private Bundle getPopulatedBundle() {
+        final Bundle settingsBundle = new Bundle();
+        mCoreSettingsObserver.populateSettings(settingsBundle,
+                CoreSettingsObserver.sGlobalSettingToTypeMap);
+        mCoreSettingsObserver.populateSettings(settingsBundle,
+                CoreSettingsObserver.sSecureSettingToTypeMap);
+        mCoreSettingsObserver.populateSettings(settingsBundle,
+                CoreSettingsObserver.sSystemSettingToTypeMap);
+        return settingsBundle;
+    }
+
+    private class TestInjector extends Injector {
+        @Override
+        public Context getContext() {
+            return mContext;
+        }
+
+        public AppOpsService getAppOpsService(File file, Handler handler) {
+            return null;
+        }
+
+        @Override
+        public Handler getUiHandler(ActivityManagerService service) {
+            return null;
+        }
+    }
+}