alessandrobenedetti commented on code in PR #2030:
URL: https://github.com/apache/solr/pull/2030#discussion_r1394113617


##########
solr/core/src/test/org/apache/solr/util/TestUtils.java:
##########
@@ -294,4 +294,23 @@ public void testMergeJson() {
     assertEquals(
         2L, Utils.getObjectByPath(sink, true, List.of(DEFAULTS, 
COLLECTION_PROP, NRT_REPLICAS)));
   }
+
+  @SuppressWarnings({"unchecked"})
+  public void testToJson() {
+    Map<String, Object> object =
+        (Map<String, Object>) Utils.fromJSONString("{k2:v2, k1: {a:b, p:r, 
k21:{xx:yy}}}");
+
+    assertEquals(
+        
"{\"k2\":\"v2\",\"k1\":{\"a\":\"b\",\"p\":\"r\",\"k21\":{\"xx\":\"yy\"}}}",
+        new String(Utils.toJSON(object, -1), UTF_8));
+    String formatedJson =

Review Comment:
   formatted?



##########
solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java:
##########
@@ -421,8 +421,20 @@ public String getInfo() {
   /** Default storage implementation that uses JSON as the storage format for 
managed data. */
   public static class JsonStorage extends ManagedResourceStorage {
 
+    private final int indentSize;
+
+    /** Uses 2 space characters as an indent. */
     public JsonStorage(StorageIO storageIO, SolrResourceLoader loader) {
+      this(storageIO, loader, 2);

Review Comment:
   if this '2' is a default parameter I would prefer to see it as a constant, 
it would be more readable



##########
solr/core/src/test/org/apache/solr/util/TestUtils.java:
##########
@@ -294,4 +294,23 @@ public void testMergeJson() {
     assertEquals(
         2L, Utils.getObjectByPath(sink, true, List.of(DEFAULTS, 
COLLECTION_PROP, NRT_REPLICAS)));
   }
+
+  @SuppressWarnings({"unchecked"})
+  public void testToJson() {

Review Comment:
   I would prefer 3 tests here:
   edge case 0
   edge case -1
   standard case
   
   I've never been a fan of mixing test cases within a single one (even if it's 
simple)



##########
solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java:
##########
@@ -421,8 +421,20 @@ public String getInfo() {
   /** Default storage implementation that uses JSON as the storage format for 
managed data. */
   public static class JsonStorage extends ManagedResourceStorage {
 
+    private final int indentSize;
+
+    /** Uses 2 space characters as an indent. */
     public JsonStorage(StorageIO storageIO, SolrResourceLoader loader) {
+      this(storageIO, loader, 2);

Review Comment:
   Furthermore, who's calling this default?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to