diff --git a/clients/venice-admin-tool/src/main/java/com/linkedin/venice/AdminTool.java b/clients/venice-admin-tool/src/main/java/com/linkedin/venice/AdminTool.java index 82df29d470..e8774e8c1d 100644 --- a/clients/venice-admin-tool/src/main/java/com/linkedin/venice/AdminTool.java +++ b/clients/venice-admin-tool/src/main/java/com/linkedin/venice/AdminTool.java @@ -992,7 +992,7 @@ private static void stringMapParam( Arg param, Consumer> setter, Set argSet) { - genericParam(cmd, param, s -> Utils.parseCommaSeparatedStringMapFromString(s, param.toString()), setter, argSet); + genericParam(cmd, param, s -> Utils.parseJsonMapFromString(s, param.toString()), setter, argSet); } private static void stringSetParam(CommandLine cmd, Arg param, Consumer> setter, Set argSet) { diff --git a/clients/venice-admin-tool/src/test/java/com/linkedin/venice/TestAdminTool.java b/clients/venice-admin-tool/src/test/java/com/linkedin/venice/TestAdminTool.java index e7cef8c86e..d59e0ab3e0 100644 --- a/clients/venice-admin-tool/src/test/java/com/linkedin/venice/TestAdminTool.java +++ b/clients/venice-admin-tool/src/test/java/com/linkedin/venice/TestAdminTool.java @@ -75,7 +75,7 @@ public void testAdminUpdateStoreArg() throws ParseException, IOException { final String K1 = "k1", V1 = "v1", K2 = "k2", V2 = "v2", K3 = "k3", V3 = "v3"; String[] args = { "--update-store", "--url", "http://localhost:7036", "--cluster", "test-cluster", "--store", "testStore", "--rmd-chunking-enabled", "true", "--partitioner-params", - K1 + "=" + V1 + "," + K2 + "=" + V2 + "," + K3 + "=" + V3 }; + "{\"" + K1 + "\":\"" + V1 + "\",\"" + K2 + "\":\"" + V2 + "\",\"" + K3 + "\":\"" + V3 + "\"}" }; CommandLine commandLine = AdminTool.getCommandLine(args); UpdateStoreQueryParams params = AdminTool.getUpdateStoreQueryParams(commandLine); @@ -293,7 +293,7 @@ public void testAdminConfigureView() throws ParseException, IOException { // Case 1: Happy path to setup a view. String[] args = { "--configure-store-view", "--url", "http://localhost:7036", "--cluster", "test-cluster", "--store", "testStore", "--view-name", "testView", "--view-class", ChangeCaptureView.class.getCanonicalName(), - "--view-params", K1 + "=" + V1 + "," + K2 + "=" + V2 }; + "--view-params", "{\"" + K1 + "\":\"" + V1 + "\",\"" + K2 + "\":\"" + V2 + "\"}" }; CommandLine commandLine = AdminTool.getCommandLine(args); UpdateStoreQueryParams params = AdminTool.getConfigureStoreViewQueryParams(commandLine); diff --git a/internal/venice-common/src/main/java/com/linkedin/venice/utils/Utils.java b/internal/venice-common/src/main/java/com/linkedin/venice/utils/Utils.java index a1641d7817..ba6b2d8ecd 100644 --- a/internal/venice-common/src/main/java/com/linkedin/venice/utils/Utils.java +++ b/internal/venice-common/src/main/java/com/linkedin/venice/utils/Utils.java @@ -2,6 +2,8 @@ import static com.linkedin.venice.HttpConstants.LOCALHOST; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; import com.linkedin.avroutil1.compatibility.AvroCompatibilityHelper; import com.linkedin.venice.controllerapi.ControllerResponse; import com.linkedin.venice.exceptions.ConfigurationException; @@ -306,18 +308,20 @@ public static boolean parseBooleanFromString(String value, String fieldName) { } /** - * For String-String key-value map config, we expect the command-line interface users to use "key1=value1,key2=value2,..." + * For String-String key-value map config, we expect the command-line interface users to use JSON * format to represent it. This method deserialize it to String-String map. */ - public static Map parseCommaSeparatedStringMapFromString(String value, String fieldName) { + public static Map parseJsonMapFromString(String value, String fieldName) { try { Map map = new HashMap<>(); if (!value.isEmpty()) { - Arrays.stream(value.split(",")).map(s -> s.split("=")).forEach(strings -> map.put(strings[0], strings[1])); + ObjectMapper objectMapper = ObjectMapperFactory.getInstance(); + return objectMapper.readValue(value, new TypeReference>() { + }); } return map; - } catch (Exception e) { - throw new VeniceException(fieldName + " must be key value pairs separated by comma, but value: " + value); + } catch (IOException jsonException) { + throw new VeniceException(fieldName + " must be a valid JSON object, but value: " + value); } } diff --git a/internal/venice-common/src/test/java/com/linkedin/venice/utils/UtilsTest.java b/internal/venice-common/src/test/java/com/linkedin/venice/utils/UtilsTest.java index baa069c4a4..24f3cc4acf 100644 --- a/internal/venice-common/src/test/java/com/linkedin/venice/utils/UtilsTest.java +++ b/internal/venice-common/src/test/java/com/linkedin/venice/utils/UtilsTest.java @@ -167,16 +167,16 @@ public void testIterateOnMapOfLists() throws Exception { @Test public void testParseMap() { - assertEquals(Utils.parseCommaSeparatedStringMapFromString("", "test_field").size(), 0); - Map nonEmptyMap = Utils.parseCommaSeparatedStringMapFromString("a=b", "test_field"); Map expectedMap = new HashMap<>(); expectedMap.put("a", "b"); - assertEquals(nonEmptyMap, expectedMap); - VeniceException e = expectThrows( - VeniceException.class, - () -> Utils.parseCommaSeparatedStringMapFromString("invalid_value", "test_field")); - assertTrue(e.getMessage().contains("must be key value pairs separated by comma")); + assertEquals(Utils.parseJsonMapFromString("", "test_field").size(), 0); + Map validMap = Utils.parseJsonMapFromString("{\"a\":\"b\"}", "test_field"); + assertEquals(validMap, expectedMap); + + VeniceException e = expectThrows(VeniceException.class, () -> Utils.parseJsonMapFromString("a=b", "test_field")); + assertTrue(e.getMessage().contains("must be a valid JSON object")); + } @Test