Merge "Handle case that settings string is JSON" into qt-r1-dev

am: 6060068d5a

Change-Id: I3386b08801beb7356edb37c5485b29188d8a7c3f

core/java/android/provider/Settings.java

@@ -6321,13 +6321,15 @@ public final class Settings {
                 "lock_screen_allow_remote_input";
 
         /**
-         * Indicates which clock face to show on lock screen and AOD.
+         * Indicates which clock face to show on lock screen and AOD formatted as a serialized
+         * {@link org.json.JSONObject} with the format:
+         *     {"clock": id, "_applied_timestamp": timestamp}
          * @hide
          */
         public static final String LOCK_SCREEN_CUSTOM_CLOCK_FACE = "lock_screen_custom_clock_face";
 
         private static final Validator LOCK_SCREEN_CUSTOM_CLOCK_FACE_VALIDATOR =
-                ANY_STRING_VALIDATOR;
+                SettingsValidators.JSON_OBJECT_VALIDATOR;
 
         /**
          * Indicates which clock face to show on lock screen and AOD while docked.

packages/SystemUI/src/com/android/keyguard/clock/SettingsWrapper.java

@@ -15,21 +15,37 @@
  */
 package com.android.keyguard.clock;
 
+import android.annotation.Nullable;
 import android.content.ContentResolver;
 import android.provider.Settings;
+import android.util.Log;
+
+import com.android.internal.annotations.VisibleForTesting;
+
+import org.json.JSONException;
+import org.json.JSONObject;
 
 /**
  * Wrapper around Settings used for testing.
  */
 public class SettingsWrapper {
 
+    private static final String TAG = "ClockFaceSettings";
     private static final String CUSTOM_CLOCK_FACE = Settings.Secure.LOCK_SCREEN_CUSTOM_CLOCK_FACE;
     private static final String DOCKED_CLOCK_FACE = Settings.Secure.DOCKED_CLOCK_FACE;
+    private static final String CLOCK_FIELD = "clock";
 
-    private ContentResolver mContentResolver;
+    private final ContentResolver mContentResolver;
+    private final Migration mMigration;
 
-    public SettingsWrapper(ContentResolver contentResolver) {
+    SettingsWrapper(ContentResolver contentResolver) {
+        this(contentResolver, new Migrator(contentResolver));
+    }
+
+    @VisibleForTesting
+    SettingsWrapper(ContentResolver contentResolver, Migration migration) {
         mContentResolver = contentResolver;
+        mMigration = migration;
     }
 
     /**
@@ -37,8 +53,10 @@ public class SettingsWrapper {
      *
      * @param userId ID of the user.
      */
-    public String getLockScreenCustomClockFace(int userId) {
+    String getLockScreenCustomClockFace(int userId) {
-        return Settings.Secure.getStringForUser(mContentResolver, CUSTOM_CLOCK_FACE, userId);
+        return decode(
+                Settings.Secure.getStringForUser(mContentResolver, CUSTOM_CLOCK_FACE, userId),
+                userId);
     }
 
     /**
@@ -46,7 +64,74 @@ public class SettingsWrapper {
      *
      * @param userId ID of the user.
      */
-    public String getDockedClockFace(int userId) {
+    String getDockedClockFace(int userId) {
         return Settings.Secure.getStringForUser(mContentResolver, DOCKED_CLOCK_FACE, userId);
     }
+
+    /**
+     * Decodes the string stored in settings, which should be formatted as JSON.
+     * @param value String stored in settings. If value is not JSON, then the settings is
+     *              overwritten with JSON containing the prior value.
+     * @return ID of the clock face to show on AOD and lock screen. If value is not JSON, the value
+     *         is returned.
+     */
+    @VisibleForTesting
+    String decode(@Nullable String value, int userId) {
+        if (value == null) {
+            return value;
+        }
+        JSONObject json;
+        try {
+            json = new JSONObject(value);
+        } catch (JSONException ex) {
+            Log.e(TAG, "Settings value is not valid JSON", ex);
+            // The settings value isn't JSON since it didn't parse so migrate the value to JSON.
+            // TODO(b/135674383): Remove this migration path in the following release.
+            mMigration.migrate(value, userId);
+            return value;
+        }
+        try {
+            return json.getString(CLOCK_FIELD);
+        } catch (JSONException ex) {
+            Log.e(TAG, "JSON object does not contain clock field.", ex);
+            return null;
+        }
+    }
+
+    interface Migration {
+        void migrate(String value, int userId);
+    }
+
+    /**
+     * Implementation of {@link Migration} that writes valid JSON back to Settings.
+     */
+    private static final class Migrator implements Migration {
+
+        private final ContentResolver mContentResolver;
+
+        Migrator(ContentResolver contentResolver) {
+            mContentResolver = contentResolver;
+        }
+
+        /**
+         * Migrate settings values that don't parse by converting to JSON format.
+         *
+         * Values in settings must be JSON to be backed up and restored. To help users maintain
+         * their current settings, convert existing values into the JSON format.
+         *
+         * TODO(b/135674383): Remove this migration code in the following release.
+         */
+        @Override
+        public void migrate(String value, int userId) {
+            try {
+                JSONObject json = new JSONObject();
+                json.put(CLOCK_FIELD, value);
+                Settings.Secure.putStringForUser(mContentResolver, CUSTOM_CLOCK_FACE,
+                        json.toString(),
+                        userId);
+            } catch (JSONException ex) {
+                Log.e(TAG, "Failed migrating settings value to JSON format", ex);
+            }
+        }
+    }
 }

packages/SystemUI/tests/src/com/android/keyguard/clock/SettingsWrapperTest.kt

@@ -0,0 +1,93 @@
+/*
+ * Copyright (C) 2019 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.keyguard.clock
+
+import android.testing.AndroidTestingRunner
+import androidx.test.filters.SmallTest
+import com.android.systemui.SysuiTestCase
+import com.google.common.truth.Truth.assertThat
+import org.json.JSONObject
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.Mockito.mock
+import org.mockito.Mockito.never
+import org.mockito.Mockito.verify
+
+private const val PACKAGE = "com.android.keyguard.clock.Clock"
+private const val CLOCK_FIELD = "clock"
+private const val TIMESTAMP_FIELD = "_applied_timestamp"
+private const val USER_ID = 0
+
+@RunWith(AndroidTestingRunner::class)
+@SmallTest
+class SettingsWrapperTest : SysuiTestCase() {
+
+    private lateinit var wrapper: SettingsWrapper
+    private lateinit var migration: SettingsWrapper.Migration
+
+    @Before
+    fun setUp() {
+        migration = mock(SettingsWrapper.Migration::class.java)
+        wrapper = SettingsWrapper(getContext().contentResolver, migration)
+    }
+
+    @Test
+    fun testDecodeUnnecessary() {
+        // GIVEN a settings value that doesn't need to be decoded
+        val value = PACKAGE
+        // WHEN the value is decoded
+        val decoded = wrapper.decode(value, USER_ID)
+        // THEN the same value is returned, because decoding isn't necessary.
+        // TODO(b/135674383): Null should be returned when the migration code in removed.
+        assertThat(decoded).isEqualTo(value)
+        // AND the value is migrated to JSON format
+        verify(migration).migrate(value, USER_ID)
+    }
+
+    @Test
+    fun testDecodeJSON() {
+        // GIVEN a settings value that is encoded in JSON
+        val json: JSONObject = JSONObject()
+        json.put(CLOCK_FIELD, PACKAGE)
+        json.put(TIMESTAMP_FIELD, System.currentTimeMillis())
+        val value = json.toString()
+        // WHEN the value is decoded
+        val decoded = wrapper.decode(value, USER_ID)
+        // THEN the clock field should have been extracted
+        assertThat(decoded).isEqualTo(PACKAGE)
+    }
+
+    @Test
+    fun testDecodeJSONWithoutClockField() {
+        // GIVEN a settings value that doesn't contain the CLOCK_FIELD
+        val json: JSONObject = JSONObject()
+        json.put(TIMESTAMP_FIELD, System.currentTimeMillis())
+        val value = json.toString()
+        // WHEN the value is decoded
+        val decoded = wrapper.decode(value, USER_ID)
+        // THEN null is returned
+        assertThat(decoded).isNull()
+        // AND the value is not migrated to JSON format
+        verify(migration, never()).migrate(value, USER_ID)
+    }
+
+    @Test
+    fun testDecodeNullJSON() {
+        assertThat(wrapper.decode(null, USER_ID)).isNull()
+    }
+}