Updated function declarations and diverted manual parsing to use obje…

…ctMapper
linkedin · Nov 6, 2023 · f747adc · f747adc
1 parent cec72b6
commit f747adc
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 30 deletions.
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
@@ -992,7 +992,7 @@ private static void stringMapParam(
       Arg param,
       Consumer<Map<String, String>> setter,
       Set<Arg> 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<Set<String>> setter, Set<Arg> argSet) {

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
@@ -2,7 +2,7 @@
 
 import static com.linkedin.venice.HttpConstants.LOCALHOST;
 
-import com.fasterxml.jackson.databind.JsonNode;
+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;
@@ -308,35 +308,21 @@ 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<String, String> parseCommaSeparatedStringMapFromString(String value, String fieldName) {
+  public static Map<String, String> parseJsonMapFromString(String value, String fieldName) {
     try {
       Map<String, String> map = new HashMap<>();
       if (!value.isEmpty()) {
-        ObjectMapper objectMapper = new ObjectMapper();
-        JsonNode rootNode = objectMapper.readTree(value);
-
-        rootNode.fieldNames().forEachRemaining(key -> {
-          JsonNode childNode = rootNode.get(key);
-          if (childNode.isObject()) {
-            map.put(key, childNode.toString());
-          } else {
-            map.put(key, childNode.asText());
-          }
+        ObjectMapper objectMapper = ObjectMapperFactory.getInstance();
+        return objectMapper.readValue(value, new TypeReference<Map<String, String>>() {
         });
       }
       return map;
     } catch (IOException jsonException) {
-      try {
-        Map<String, String> map = new HashMap<>();
-        Arrays.stream(value.split(",")).map(s -> s.split("=")).forEach(strings -> map.put(strings[0], strings[1]));
-        return map;
-      } catch (Exception kvException) {
-        throw new VeniceException(
-            fieldName + " must be a valid JSON object or key-value pairs separated by comma, but value: " + value);
-      }
+      throw new VeniceException(
+          fieldName + " must be a valid JSON object or key-value pairs separated by comma, 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
@@ -167,15 +167,14 @@ public void testIterateOnMapOfLists() throws Exception {
 
   @Test
   public void testParseMap() {
-    assertEquals(Utils.parseCommaSeparatedStringMapFromString("", "test_field").size(), 0);
-    Map nonEmptyMap = Utils.parseCommaSeparatedStringMapFromString("a=b", "test_field");
+    assertEquals(Utils.parseJsonMapFromString("", "test_field").size(), 0);
+    Map nonEmptyMap = Utils.parseJsonMapFromString("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"));
+    VeniceException e =
+        expectThrows(VeniceException.class, () -> Utils.parseJsonMapFromString("invalid_value", "test_field"));
     assertTrue(e.getMessage().contains("must be key value pairs separated by comma"));
   }
 
@@ -185,9 +184,9 @@ public void testSanitizingStringForLogger() {
   }
 
   @Test
-  public void testParseCommaSeparatedStringMapFromString() {
+  public void testParseJsonMapFromString() {
     try {
-      Utils.parseCommaSeparatedStringMapFromString(
+      Utils.parseJsonMapFromString(
           "{\"changeCaptureView\": {\"viewClassName\": \"com.linkedin.venice.views.ChangeCaptureView\",\"params\": {}}}",
           "someField"); // Method that should not throw an exception
     } catch (Exception e) {
@@ -196,7 +195,7 @@ public void testParseCommaSeparatedStringMapFromString() {
 
     Assert.assertThrows(
         VeniceException.class,
-        () -> Utils.parseCommaSeparatedStringMapFromString(
+        () -> Utils.parseJsonMapFromString(
             "{\"changeCaptureView\": {\"viewClassName\": \"com.linkedin.venice.views.ChangeCaptureView\",\"params\":",
             "someField"));
   }