Simplify key-value mapping representation.

Intsead of an array of objects with keys "key" and "value", just use a
simple JSON object which encode key-value mappings more simply and
compactly.

Test: atest SignedConfigTest
Bug: 110509075
Change-Id: Ib81b6869d680de7b0e2c6b6b202dda2c7e854449

2 files changed, 46 insertions, 58 deletions

diff --git a/services/core/java/com/android/server/signedconfig/SignedConfig.java b/services/core/java/com/android/server/signedconfig/SignedConfig.java
index e6bb800045c8..560a1e1cfe6c 100644
--- a/services/core/java/com/android/server/signedconfig/SignedConfig.java
+++ b/services/core/java/com/android/server/signedconfig/SignedConfig.java
@@ -33,19 +33,34 @@ import java.util.Set;
  * Represents signed configuration.
  *
  * <p>This configuration should only be used if the signature has already been verified.
+ *
+ * This class also parses signed config from JSON. The format expected is:
+ * <pre>
+ * {
+ *   "version": 1
+ *   "config": [
+ *     {
+ *       "min_sdk": 28,
+ *       "max_sdk": 29,
+ *       "values": {
+ *         "key": "value",
+ *         "key2": "value2"
+ *         ...
+ *       }
+ *     },
+ *     ...
+ *   ],
+ * }
+ * </pre>
  */
 public class SignedConfig {
 
     private static final String KEY_VERSION = "version";
     private static final String KEY_CONFIG = "config";
 
-    private static final String CONFIG_KEY_MIN_SDK = "minSdk";
-    private static final String CONFIG_KEY_MAX_SDK = "maxSdk";
+    private static final String CONFIG_KEY_MIN_SDK = "min_sdk";
+    private static final String CONFIG_KEY_MAX_SDK = "max_sdk";
     private static final String CONFIG_KEY_VALUES = "values";
-    // TODO it may be better to use regular key/value pairs in a JSON object, rather than an array
-    // of objects with the 2 keys below.
-    private static final String CONFIG_KEY_KEY = "key";
-    private static final String CONFIG_KEY_VALUE = "value";
 
     /**
      * Represents config values targeting an SDK range.
@@ -141,14 +156,10 @@ public class SignedConfig {
             throws JSONException, InvalidConfigException {
         int minSdk = json.getInt(CONFIG_KEY_MIN_SDK);
         int maxSdk = json.getInt(CONFIG_KEY_MAX_SDK);
-        JSONArray valueArray = json.getJSONArray(CONFIG_KEY_VALUES);
+        JSONObject valuesJson = json.getJSONObject(CONFIG_KEY_VALUES);
         Map<String, String> values = new HashMap<>();
-        for (int i = 0; i < valueArray.length(); ++i) {
-            JSONObject keyValuePair = valueArray.getJSONObject(i);
-            String key = keyValuePair.getString(CONFIG_KEY_KEY);
-            Object valueObject = keyValuePair.has(CONFIG_KEY_VALUE)
-                    ? keyValuePair.get(CONFIG_KEY_VALUE)
-                    : null;
+        for (String key : valuesJson.keySet()) {
+            Object valueObject = valuesJson.get(key);
             String value = valueObject == JSONObject.NULL || valueObject == null
                             ? null
                             : valueObject.toString();
diff --git a/services/tests/servicestests/src/com/android/server/signedconfig/SignedConfigTest.java b/services/tests/servicestests/src/com/android/server/signedconfig/SignedConfigTest.java
index 70fadd101a91..00a62a300487 100644
--- a/services/tests/servicestests/src/com/android/server/signedconfig/SignedConfigTest.java
+++ b/services/tests/servicestests/src/com/android/server/signedconfig/SignedConfigTest.java
@@ -66,7 +66,7 @@ public class SignedConfigTest {
 
     @Test
     public void testParsePerSdkConfigSdkMinMax() throws JSONException, InvalidConfigException {
-        JSONObject json = new JSONObject("{\"minSdk\":2, \"maxSdk\": 3, \"values\": []}");
+        JSONObject json = new JSONObject("{\"min_sdk\":2, \"max_sdk\": 3, \"values\": {}}");
         SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, emptySet(),
                 emptyMap());
         assertThat(config.minSdk).isEqualTo(2);
@@ -75,7 +75,7 @@ public class SignedConfigTest {
 
     @Test
     public void testParsePerSdkConfigNoMinSdk() throws JSONException {
-        JSONObject json = new JSONObject("{\"maxSdk\": 3, \"values\": []}");
+        JSONObject json = new JSONObject("{\"max_sdk\": 3, \"values\": {}}");
         try {
             SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap());
             fail("Expected InvalidConfigException or JSONException");
@@ -86,7 +86,7 @@ public class SignedConfigTest {
 
     @Test
     public void testParsePerSdkConfigNoMaxSdk() throws JSONException {
-        JSONObject json = new JSONObject("{\"minSdk\": 1, \"values\": []}");
+        JSONObject json = new JSONObject("{\"min_sdk\": 1, \"values\": {}}");
         try {
             SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap());
             fail("Expected InvalidConfigException or JSONException");
@@ -97,7 +97,7 @@ public class SignedConfigTest {
 
     @Test
     public void testParsePerSdkConfigNoValues() throws JSONException {
-        JSONObject json = new JSONObject("{\"minSdk\": 1, \"maxSdk\": 3}");
+        JSONObject json = new JSONObject("{\"min_sdk\": 1, \"max_sdk\": 3}");
         try {
             SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap());
             fail("Expected InvalidConfigException or JSONException");
@@ -108,7 +108,7 @@ public class SignedConfigTest {
 
     @Test
     public void testParsePerSdkConfigSdkNullMinSdk() throws JSONException {
-        JSONObject json = new JSONObject("{\"minSdk\":null, \"maxSdk\": 3, \"values\": []}");
+        JSONObject json = new JSONObject("{\"min_sdk\":null, \"max_sdk\": 3, \"values\": {}}");
         try {
             SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap());
             fail("Expected InvalidConfigException or JSONException");
@@ -119,7 +119,7 @@ public class SignedConfigTest {
 
     @Test
     public void testParsePerSdkConfigSdkNullMaxSdk() throws JSONException {
-        JSONObject json = new JSONObject("{\"minSdk\":1, \"maxSdk\": null, \"values\": []}");
+        JSONObject json = new JSONObject("{\"min_sdk\":1, \"max_sdk\": null, \"values\": {}}");
         try {
             SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap());
             fail("Expected InvalidConfigException or JSONException");
@@ -130,7 +130,7 @@ public class SignedConfigTest {
 
     @Test
     public void testParsePerSdkConfigNullValues() throws JSONException {
-        JSONObject json = new JSONObject("{\"minSdk\": 1, \"maxSdk\": 3, \"values\": null}");
+        JSONObject json = new JSONObject("{\"min_sdk\": 1, \"max_sdk\": 3, \"values\": null}");
         try {
             SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap());
             fail("Expected InvalidConfigException or JSONException");
@@ -142,7 +142,7 @@ public class SignedConfigTest {
     @Test
     public void testParsePerSdkConfigZeroValues()
             throws JSONException, InvalidConfigException {
-        JSONObject json = new JSONObject("{\"minSdk\": 1, \"maxSdk\": 3, \"values\": []}");
+        JSONObject json = new JSONObject("{\"min_sdk\": 1, \"max_sdk\": 3, \"values\": {}}");
         SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a", "b"),
                 emptyMap());
         assertThat(config.values).hasSize(0);
@@ -152,7 +152,7 @@ public class SignedConfigTest {
     public void testParsePerSdkConfigSingleKey()
             throws JSONException, InvalidConfigException {
         JSONObject json = new JSONObject(
-                "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": \"1\"}]}");
+                "{\"min_sdk\": 1, \"max_sdk\": 1, \"values\": {\"a\": \"1\"}}");
         SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a", "b"),
                 emptyMap());
         assertThat(config.values).containsExactly("a", "1");
@@ -162,7 +162,7 @@ public class SignedConfigTest {
     public void testParsePerSdkConfigSingleKeyNullValue()
             throws JSONException, InvalidConfigException {
         JSONObject json = new JSONObject(
-                "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": null}]}");
+                "{\"min_sdk\": 1, \"max_sdk\": 1, \"values\": {\"a\": null}}");
         SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a", "b"),
                 emptyMap());
         assertThat(config.values.keySet()).containsExactly("a");
@@ -173,8 +173,7 @@ public class SignedConfigTest {
     public void testParsePerSdkConfigMultiKeys()
             throws JSONException, InvalidConfigException {
         JSONObject json = new JSONObject(
-                "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": \"1\"}, "
-                        + "{\"key\":\"c\", \"value\": \"2\"}]}");
+                "{\"min_sdk\": 1, \"max_sdk\": 1, \"values\": {\"a\": \"1\", \"c\": \"2\"}}");
         SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(
                 json, setOf("a", "b", "c"), emptyMap());
         assertThat(config.values).containsExactly("a", "1", "c", "2");
@@ -183,7 +182,7 @@ public class SignedConfigTest {
     @Test
     public void testParsePerSdkConfigSingleKeyNotAllowed() throws JSONException {
         JSONObject json = new JSONObject(
-                "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": \"1\"}]}");
+                "{\"min_sdk\": 1, \"max_sdk\": 1, \"values\": {\"a\": \"1\"}}");
         try {
             SignedConfig.parsePerSdkConfig(json, setOf("b"), emptyMap());
             fail("Expected InvalidConfigException or JSONException");
@@ -196,7 +195,7 @@ public class SignedConfigTest {
     public void testParsePerSdkConfigSingleKeyWithMap()
             throws JSONException, InvalidConfigException {
         JSONObject json = new JSONObject(
-                "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": \"1\"}]}");
+                "{\"min_sdk\": 1, \"max_sdk\": 1, \"values\": {\"a\": \"1\"}}");
         SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a"),
                 mapOf("a", mapOf("1", "one")));
         assertThat(config.values).containsExactly("a", "one");
@@ -205,7 +204,7 @@ public class SignedConfigTest {
     @Test
     public void testParsePerSdkConfigSingleKeyWithMapInvalidValue() throws JSONException {
         JSONObject json = new JSONObject(
-                "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": \"2\"}]}");
+                "{\"min_sdk\": 1, \"max_sdk\": 1, \"values\": {\"a\": \"2\"}}");
         try {
             SignedConfig.parsePerSdkConfig(json, setOf("b"), mapOf("a", mapOf("1", "one")));
             fail("Expected InvalidConfigException");
@@ -218,8 +217,7 @@ public class SignedConfigTest {
     public void testParsePerSdkConfigMultiKeysWithMap()
             throws JSONException, InvalidConfigException {
         JSONObject json = new JSONObject(
-                "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": \"1\"},"
-                + "{\"key\":\"b\", \"value\": \"1\"}]}");
+                "{\"min_sdk\": 1, \"max_sdk\": 1, \"values\": {\"a\": \"1\", \"b\": \"1\"}}");
         SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a", "b"),
                 mapOf("a", mapOf("1", "one")));
         assertThat(config.values).containsExactly("a", "one", "b", "1");
@@ -229,7 +227,7 @@ public class SignedConfigTest {
     public void testParsePerSdkConfigSingleKeyWithMapToNull()
             throws JSONException, InvalidConfigException {
         JSONObject json = new JSONObject(
-                "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": \"1\"}]}");
+                "{\"min_sdk\": 1, \"max_sdk\": 1, \"values\": {\"a\": \"1\"}}");
         SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a"),
                 mapOf("a", mapOf("1", null)));
         assertThat(config.values).containsExactly("a", null);
@@ -241,25 +239,15 @@ public class SignedConfigTest {
         assertThat(mapOf(null, "allitnil")).containsExactly(null, "allitnil");
         assertThat(mapOf(null, "allitnil").containsKey(null)).isTrue();
         JSONObject json = new JSONObject(
-                "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": null}]}");
+                "{\"min_sdk\": 1, \"max_sdk\": 1, \"values\": {\"a\": null}}");
         SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a"),
                 mapOf("a", mapOf(null, "allitnil")));
         assertThat(config.values).containsExactly("a", "allitnil");
     }
 
     @Test
-    public void testParsePerSdkConfigSingleKeyNoValue()
-            throws JSONException, InvalidConfigException {
-        JSONObject json = new JSONObject(
-                "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\"}]}");
-        SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a", "b"),
-                emptyMap());
-        assertThat(config.values).containsExactly("a", null);
-    }
-
-    @Test
     public void testParsePerSdkConfigValuesInvalid() throws JSONException  {
-        JSONObject json = new JSONObject("{\"minSdk\": 1, \"maxSdk\": 1,  \"values\": \"foo\"}");
+        JSONObject json = new JSONObject("{\"min_sdk\": 1, \"max_sdk\": 1,  \"values\": \"foo\"}");
         try {
             SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap());
             fail("Expected InvalidConfigException or JSONException");
@@ -270,18 +258,7 @@ public class SignedConfigTest {
 
     @Test
     public void testParsePerSdkConfigConfigEntryInvalid() throws JSONException {
-        JSONObject json = new JSONObject("{\"minSdk\": 1, \"maxSdk\": 1,  \"values\": [1, 2]}");
-        try {
-            SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap());
-            fail("Expected InvalidConfigException or JSONException");
-        } catch (JSONException | InvalidConfigException e) {
-            // expected
-        }
-    }
-
-    @Test
-    public void testParsePerSdkConfigConfigEntryNull() throws JSONException {
-        JSONObject json = new JSONObject("{\"minSdk\": 1, \"maxSdk\": 1,  \"values\": [null]}");
+        JSONObject json = new JSONObject("{\"min_sdk\": 1, \"max_sdk\": 1,  \"values\": [1, 2]}");
         try {
             SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap());
             fail("Expected InvalidConfigException or JSONException");
@@ -361,7 +338,7 @@ public class SignedConfigTest {
     @Test
     public void testParseSdkConfigSingle() throws InvalidConfigException {
         SignedConfig config = SignedConfig.parse(
-                "{\"version\": 1, \"config\":[{\"minSdk\": 1, \"maxSdk\": 1, \"values\": []}]}",
+                "{\"version\": 1, \"config\":[{\"min_sdk\": 1, \"max_sdk\": 1, \"values\": {}}]}",
                 emptySet(), emptyMap());
         assertThat(config.perSdkConfig).hasSize(1);
     }
@@ -369,8 +346,8 @@ public class SignedConfigTest {
     @Test
     public void testParseSdkConfigMultiple() throws InvalidConfigException {
         SignedConfig config = SignedConfig.parse(
-                "{\"version\": 1, \"config\":[{\"minSdk\": 1, \"maxSdk\": 1, \"values\": []}, "
-                        + "{\"minSdk\": 2, \"maxSdk\": 2, \"values\": []}]}", emptySet(),
+                "{\"version\": 1, \"config\":[{\"min_sdk\": 1, \"max_sdk\": 1, \"values\": {}}, "
+                        + "{\"min_sdk\": 2, \"max_sdk\": 2, \"values\": {}}]}", emptySet(),
                 emptyMap());
         assertThat(config.perSdkConfig).hasSize(2);
     }