fix: display value json input

Computing the display value should retain the original value type of the value. For example if the value is of type string but the contents of the string resemble a primitive value the some information may get lost. Given an value list of strings `["2024.20", "2024.0"]` the json string passed to `parseDisplayValue` would be the the values `2024.20` and `2024.0`, which when re-parsed as json values will be interpreted as numbers rather than strings. This has the end effect of the display values `2024.2` and `2024` while the associated values are `2024.20` and `2024.0`. To fix this behavior each value is "json stringified" before sent to `parseDisplayValue`. For the example above the values would be `"2024.20"` and `"2024.0"`.
jenkinsci · Dec 4, 2024 · 792f9d7 · 792f9d7
1 parent a88b356
commit 792f9d7
Show file tree

Hide file tree

Showing 3 changed files with 96 additions and 3 deletions.
diff --git a/src/main/java/io/jenkins/plugins/restlistparam/logic/ValueResolver.java b/src/main/java/io/jenkins/plugins/restlistparam/logic/ValueResolver.java
@@ -5,6 +5,7 @@
 import io.jenkins.plugins.restlistparam.model.MimeType;
 import io.jenkins.plugins.restlistparam.model.ResultContainer;
 import io.jenkins.plugins.restlistparam.model.ValueItem;
+import org.json.JSONStringer;
 import org.w3c.dom.Document;
 import org.w3c.dom.NodeList;
 import org.xml.sax.InputSource;
@@ -125,8 +126,7 @@ public static ResultContainer<List<ValueItem>> resolveJsonPath(final String json
           resolved.stream()
                   .map(JsonPath::parse)
                   .map(context -> context.read("$"))
-                  .map(ValueResolver::convertToString)
-                  .map(value -> new ValueItem(value, parseDisplayValue(value, displayExpression)))
+                  .map(value -> new ValueItem(convertToString(value), parseDisplayValue(convertToJson(value), displayExpression)))
                   .collect(Collectors.toList())
         );
       }
@@ -155,8 +155,12 @@ public static ResultContainer<List<ValueItem>> resolveJsonPath(final String json
     return container;
   }
 
+  private static String convertToJson(Object obj) {
+    return JSONStringer.valueToString(obj);
+  }
+
   public static String convertToString(Object obj) {
-    if (obj instanceof Map) {
+    if (obj instanceof Map || obj instanceof List) {
       return JsonPath.parse(obj).jsonString();
     }
     else if (obj instanceof Integer) {
@@ -165,6 +169,12 @@ else if (obj instanceof Integer) {
     else if (obj instanceof Float) {
       return Float.toString((Float) obj);
     }
+    else if (obj instanceof Double) {
+      return Double.toString((Double) obj);
+    }
+    else if (obj instanceof Boolean) {
+      return Boolean.toString((Boolean) obj);
+    }
     else if (obj instanceof String) {
       return (String) obj;
     }

diff --git a/src/test/java/io/jenkins/plugins/restlistparam/TestConst.java b/src/test/java/io/jenkins/plugins/restlistparam/TestConst.java
@@ -34,6 +34,48 @@ public class TestConst {
     "  }\n" +
     "]\n";
 
+  public static final String numberLikeTestJson = "[\n" +
+    "  {\n" +
+    "    \"name\": \"2024.19\",\n" +
+    "  },\n" +
+    "  {\n" +
+    "    \"name\": \"2024.20\",\n" +
+    "  },\n" +
+    "  {\n" +
+    "    \"name\": \"2024.0\",\n" +
+    "  }\n" +
+    "]\n";
+
+  public static final String numberValuesTestJson = "[\n" +
+    "  {\n" +
+    "    \"value\": 1.0\n" +
+    "  },\n" +
+    "  {\n" +
+    "    \"value\": 10\n" +
+    "  },\n" +
+    "  {\n" +
+    "    \"value\": 11.50\n" +
+    "  }\n" +
+    "]\n";
+
+  public static final String mixedTypeValuesTestJson = "[\n" +
+    "  {\n" +
+    "    \"value\": 1.0\n" +
+    "  },\n" +
+    "  {\n" +
+    "    \"value\": 10\n" +
+    "  },\n" +
+    "  {\n" +
+    "    \"value\": \"11.50\"\n" +
+    "  },\n" +
+    "  {\n" +
+    "    \"value\": true\n" +
+    "  },\n" +
+    "  {\n" +
+    "    \"value\": false\n" +
+    "  }\n" +
+    "]\n";
+
   public static final String validJsonValueItem = "{" +
     "\"name\":\"v10.6.4\"," +
     "\"zipball_url\":\"https://api.github.com/repos/jellyfin/jellyfin/zipball/v10.6.4\"," +

diff --git a/src/test/java/io/jenkins/plugins/restlistparam/ValueResolverTest.java b/src/test/java/io/jenkins/plugins/restlistparam/ValueResolverTest.java
@@ -8,6 +8,7 @@
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.util.HashMap;
 import java.util.List;
 
 public class ValueResolverTest {
@@ -20,6 +21,46 @@ public void resolveJsonPathTest() {
     Assert.assertEquals(3, res.getValue().size());
     Assert.assertArrayEquals(new String[]{"v10.6.4", "v10.6.3", "v10.6.2"},
                              res.getValue().stream().map(ValueItem::getValue).toArray());
+    Assert.assertArrayEquals(new String[]{"v10.6.4", "v10.6.3", "v10.6.2"},
+                             res.getValue().stream().map(ValueItem::getDisplayValue).toArray());
+  }
+
+  @Test
+  public void resolveJsonPathNumberLikeValuesTest() {
+    ResultContainer<List<ValueItem>> res = ValueResolver.resolveJsonPath(TestConst.numberLikeTestJson, "$.*.name", "$");
+    Assert.assertNotNull(res);
+    Assert.assertFalse(res.getErrorMsg().isPresent());
+    Assert.assertEquals(3, res.getValue().size());
+    Assert.assertArrayEquals(new String[]{"2024.19", "2024.20", "2024.0"},
+                             res.getValue().stream().map(ValueItem::getValue).toArray());
+    Assert.assertArrayEquals(new String[]{"2024.19", "2024.20", "2024.0"},
+                             res.getValue().stream().map(ValueItem::getDisplayValue).toArray());
+  }
+
+  @Test
+  public void resolveJsonPathNumberValuesTest() {
+    ResultContainer<List<ValueItem>> res = ValueResolver.resolveJsonPath(TestConst.numberValuesTestJson, "$.*.value", "$");
+    Assert.assertNotNull(res);
+    Assert.assertFalse(res.getErrorMsg().isPresent());
+    Assert.assertEquals(3, res.getValue().size());
+    Assert.assertArrayEquals(new String[]{"1.0", "10", "11.5"},
+                             res.getValue().stream().map(ValueItem::getValue).toArray());
+    // trailing 0 and decimal points are shaved off numbers by JSONStringer
+    Assert.assertArrayEquals(new String[]{"1", "10", "11.5"},
+                             res.getValue().stream().map(ValueItem::getDisplayValue).toArray());
+  }
+
+  @Test
+  public void resolveJsonPathMixedTypeValuesTest() {
+    ResultContainer<List<ValueItem>> res = ValueResolver.resolveJsonPath(TestConst.mixedTypeValuesTestJson, "$.*.value", "$");
+    Assert.assertNotNull(res);
+    Assert.assertFalse(res.getErrorMsg().isPresent());
+    Assert.assertEquals(5, res.getValue().size());
+    Assert.assertArrayEquals(new String[]{"1.0", "10", "11.50", "true", "false"},
+                             res.getValue().stream().map(ValueItem::getValue).toArray());
+    // trailing 0 and decimal points are shaved off numbers by JSONStringer
+    Assert.assertArrayEquals(new String[]{"1", "10", "11.50", "true", "false"},
+                             res.getValue().stream().map(ValueItem::getDisplayValue).toArray());
   }
 
   @Test