Canonicalization : sorting json property values by key if they are ob…

…ject (#530)
linkedin · Jan 11, 2024 · dfd1900 · dfd1900
1 parent 26ae2c0
commit dfd1900
Show file tree

Hide file tree

Showing 13 changed files with 308 additions and 5 deletions.
diff --git a/helper/helper-common/src/main/java/com/linkedin/avroutil1/compatibility/Jackson1Utils.java b/helper/helper-common/src/main/java/com/linkedin/avroutil1/compatibility/Jackson1Utils.java
@@ -12,6 +12,7 @@
 import java.util.Collection;
 import java.util.Map;
 import java.util.Objects;
+import java.util.TreeMap;
 import org.apache.avro.AvroRuntimeException;
 import org.apache.avro.Schema;
 import org.apache.avro.generic.GenericData;
@@ -23,6 +24,7 @@
 import org.codehaus.jackson.JsonParser;
 import org.codehaus.jackson.JsonToken;
 import org.codehaus.jackson.map.ObjectMapper;
+import org.codehaus.jackson.node.ArrayNode;
 import org.codehaus.jackson.node.DoubleNode;
 import org.codehaus.jackson.node.IntNode;
 import org.codehaus.jackson.node.JsonNodeFactory;
@@ -300,6 +302,30 @@ public static void assertNoTrailingContent(String json) {
     }
   }
 
+  /**
+   * Sorts a JsonNode alphabetically by field name. This is useful for comparing JsonNodes for equality.
+   * @param jsonNode the JsonNode to sort
+   * @return the sorted JsonNode
+   */
+  public static JsonNode sortJsonNode(JsonNode jsonNode) {
+    if (jsonNode.isObject()) {
+      ObjectNode objectNode = JsonNodeFactory.instance.objectNode();
+      Map<String, JsonNode> sortedMap = new TreeMap<>();
+
+      // create map of field names to field values of json node
+      jsonNode.getFieldNames()
+          .forEachRemaining(fieldName -> sortedMap.put(fieldName, sortJsonNode(jsonNode.get(fieldName))));
+
+      sortedMap.forEach(objectNode::put);
+      return objectNode;
+    } else if (jsonNode.isArray()) {
+      for (int i = 0; i < jsonNode.size(); i++) {
+        ((ArrayNode) jsonNode).set(i, sortJsonNode(jsonNode.get(i)));
+      }
+    }
+    return jsonNode;
+  }
+
   private static boolean isAMathematicalInteger(BigDecimal bigDecimal) {
     return bigDecimal.stripTrailingZeros().scale() <= 0;
   }

diff --git a/helper/helper-common/src/main/java/com/linkedin/avroutil1/compatibility/Jackson2Utils.java b/helper/helper-common/src/main/java/com/linkedin/avroutil1/compatibility/Jackson2Utils.java
@@ -12,15 +12,20 @@
 import com.fasterxml.jackson.core.JsonToken;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.DoubleNode;
 import com.fasterxml.jackson.databind.node.FloatNode;
 import com.fasterxml.jackson.databind.node.IntNode;
+import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 import com.fasterxml.jackson.databind.node.LongNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.fasterxml.jackson.databind.node.TextNode;
 import java.io.StringReader;
 import java.math.BigDecimal;
+import java.util.Iterator;
 import java.util.Map;
 import java.util.Objects;
+import java.util.TreeMap;
 import org.apache.avro.Schema;
 import org.apache.avro.SchemaParseException;
 import org.slf4j.Logger;
@@ -235,6 +240,32 @@ public static void assertNoTrailingContent(String json) {
     }
   }
 
+  /**
+   * Sorts the properties of a JsonNode alphabetically by key.
+   * @param jsonNode the JsonNode to sort
+   * @return the sorted JsonNode
+   */
+  public static JsonNode sortJsonNode(JsonNode jsonNode) {
+    if (jsonNode.isObject()) {
+      ObjectNode objectNode = JsonNodeFactory.instance.objectNode();
+      Map<String, JsonNode> sortedMap = new TreeMap<>();
+
+      Iterator<Map.Entry<String, JsonNode>> fields = jsonNode.fields();
+      while (fields.hasNext()) {
+        Map.Entry<String, JsonNode> field = fields.next();
+        sortedMap.put(field.getKey(), sortJsonNode(field.getValue()));
+      }
+
+      sortedMap.forEach(objectNode::set);
+      return objectNode;
+    } else if (jsonNode.isArray()) {
+      for (int i = 0; i < jsonNode.size(); i++) {
+        ((ArrayNode)jsonNode).set(i, sortJsonNode(jsonNode.get(i)));
+      }
+    }
+    return jsonNode;
+  }
+
   private static boolean isAMathematicalInteger(BigDecimal bigDecimal) {
     return bigDecimal.stripTrailingZeros().scale() <= 0;
   }

diff --git a/...mpl-110/src/main/java/com/linkedin/avroutil1/compatibility/avro110/Avro110AvscWriter.java b/...mpl-110/src/main/java/com/linkedin/avroutil1/compatibility/avro110/Avro110AvscWriter.java
@@ -125,7 +125,7 @@ private void writeProps(Map<String, Object> props, Jackson2JsonGeneratorWrapper
         JsonGenerator delegate = gen.getDelegate();
         for (Map.Entry<String, Object> entry : props.entrySet()) {
             Object o = entry.getValue();
-            delegate.writeObjectField(entry.getKey(), JacksonUtils.toJsonNode(o));
+            delegate.writeObjectField(entry.getKey(), Jackson2Utils.sortJsonNode(JacksonUtils.toJsonNode(o)));
         }
     }
 

diff --git a/...mpl-111/src/main/java/com/linkedin/avroutil1/compatibility/avro111/Avro111AvscWriter.java b/...mpl-111/src/main/java/com/linkedin/avroutil1/compatibility/avro111/Avro111AvscWriter.java
@@ -124,7 +124,7 @@ private void writeProps(Map<String, Object> props, Jackson2JsonGeneratorWrapper
         JsonGenerator delegate = gen.getDelegate();
         for (Map.Entry<String, Object> entry : props.entrySet()) {
             Object o = entry.getValue();
-            delegate.writeObjectField(entry.getKey(), JacksonUtils.toJsonNode(o));
+            delegate.writeObjectField(entry.getKey(), Jackson2Utils.sortJsonNode(JacksonUtils.toJsonNode(o)));
         }
     }
 

diff --git a/...r-impl-17/src/main/java/com/linkedin/avroutil1/compatibility/avro17/Avro17AvscWriter.java b/...r-impl-17/src/main/java/com/linkedin/avroutil1/compatibility/avro17/Avro17AvscWriter.java
@@ -143,7 +143,7 @@ private void writeProps(
         for (Map.Entry<String, JsonNode> entry : props.entrySet()) {
             String propName = entry.getKey();
             if (propNameFilter == null || propNameFilter.test(propName)) {
-                delegate.writeObjectField(entry.getKey(), entry.getValue());
+                delegate.writeObjectField(entry.getKey(), Jackson1Utils.sortJsonNode(entry.getValue()));
             }
         }
     }

diff --git a/...r-impl-18/src/main/java/com/linkedin/avroutil1/compatibility/avro18/Avro18AvscWriter.java b/...r-impl-18/src/main/java/com/linkedin/avroutil1/compatibility/avro18/Avro18AvscWriter.java
@@ -143,7 +143,7 @@ private void writeProps(
         for (Map.Entry<String, JsonNode> entry : props.entrySet()) {
             String propName = entry.getKey();
             if (propNameFilter == null || propNameFilter.test(propName)) {
-                delegate.writeObjectField(entry.getKey(), entry.getValue());
+                delegate.writeObjectField(entry.getKey(), Jackson1Utils.sortJsonNode(entry.getValue()));
             }
         }
     }

diff --git a/...r-impl-19/src/main/java/com/linkedin/avroutil1/compatibility/avro19/Avro19AvscWriter.java b/...r-impl-19/src/main/java/com/linkedin/avroutil1/compatibility/avro19/Avro19AvscWriter.java
@@ -125,7 +125,7 @@ private void writeProps(Map<String, Object> props, Jackson2JsonGeneratorWrapper
         JsonGenerator delegate = gen.getDelegate();
         for (Map.Entry<String, Object> entry : props.entrySet()) {
             Object o = entry.getValue();
-            delegate.writeObjectField(entry.getKey(), JacksonUtils.toJsonNode(o));
+            delegate.writeObjectField(entry.getKey(), Jackson2Utils.sortJsonNode(JacksonUtils.toJsonNode(o)));
         }
     }
     @Override

diff --git a/...t/java/com/linkedin/avroutil1/compatibility/avro17/AvroUtilSchemaNormalizationTest17.java b/...t/java/com/linkedin/avroutil1/compatibility/avro17/AvroUtilSchemaNormalizationTest17.java
@@ -119,6 +119,19 @@ public void testCanonicalBroadNoPlugin() throws IOException {
     Assert.assertTrue(AvroCompatibilityHelper.getFieldAliases(canonicalizedSchema.getFields().get(3)).toString().equals("[afield_alias, field_alias]"));
   }
 
+  @Test
+  public void testNestedObjectJsonPropertySerialization() throws IOException {
+    Schema schema1 = Schema.parse(TestUtil.load("PropRecord1.avsc"));
+    Schema schema2 = Schema.parse(TestUtil.load("PropRecord2.avsc"));
+    AvscGenerationConfig avscGenerationConfig =
+        new AvscGenerationConfig(false, false, false, Optional.of(Boolean.FALSE), false, true, false, true, true, true,
+            true, false, false);
+    String schemaStr1 = AvroUtilSchemaNormalization.getCanonicalForm(schema1, avscGenerationConfig, null);
+    String schemaStr2 = AvroUtilSchemaNormalization.getCanonicalForm(schema2, avscGenerationConfig, null);
+
+    Assert.assertEquals(schemaStr1, schemaStr2);
+  }
+
   @Test
   public void testCanonicalStrictWithNonSpecificJsonIncluded() throws IOException {
     AvscGenerationConfig config = new AvscGenerationConfig(

diff --git a/helper/tests/helper-tests-17/src/test/resources/PropRecord1.avsc b/helper/tests/helper-tests-17/src/test/resources/PropRecord1.avsc
@@ -0,0 +1,55 @@
+{
+  "type": "record",
+  "namespace": "com.acme",
+  "name": "RecordWithStuff",
+  "doc": "Here is a great overall doc",
+  "aliases": [
+    "record_alias",
+    "a_record_alias"
+  ],
+  "fields": [
+    {
+      "name": "strTest",
+      "type": {
+        "type": "string",
+        "avro.java.string": "String"
+      },
+      "some_prop": {
+        "KEY1": "VALUE1",
+        "KEY2": "VALUE2",
+        "ObjKey": {
+          "K1": "V1",
+          "K2": "V2",
+          "K3": "V3",
+          "K4": "V4",
+          "K5": {
+            "K5K1": "K5V1",
+            "K5K2": "K5V2",
+            "K5K3": "K5V3"
+          }
+        },
+        "ArObjKey": [
+          {
+            "K1": "V1",
+            "K2": "V2",
+            "K3": "V3",
+            "K4": "V4"
+          },
+          {
+            "K1": "V1",
+            "K2": "V2",
+            "K3": {
+              "K3K1": "K3V1",
+              "K3K2": "K3V2",
+              "K3K3": {
+                "K3K3K1": "K3K3V1",
+                "K3K3K2": "K3K3V2",
+                "K3K3K3": "K3K3V3"
+              }
+            }
+          }
+        ]
+      }
+    }
+  ]
+}
diff --git a/helper/tests/helper-tests-17/src/test/resources/PropRecord2.avsc b/helper/tests/helper-tests-17/src/test/resources/PropRecord2.avsc
@@ -0,0 +1,55 @@
+{
+  "type": "record",
+  "namespace": "com.acme",
+  "name": "RecordWithStuff",
+  "doc": "Here is a great overall doc",
+  "aliases": [
+    "record_alias",
+    "a_record_alias"
+  ],
+  "fields": [
+    {
+      "name": "strTest",
+      "type": {
+        "type": "string",
+        "avro.java.string": "String"
+      },
+      "some_prop": {
+        "KEY1": "VALUE1",
+        "KEY2": "VALUE2",
+        "ObjKey": {
+          "K2": "V2",
+          "K1": "V1",
+          "K5": {
+            "K5K2": "K5V2",
+            "K5K1": "K5V1",
+            "K5K3": "K5V3"
+          },
+          "K4": "V4",
+          "K3": "V3"
+        },
+        "ArObjKey": [
+          {
+            "K3": "V3",
+            "K1": "V1",
+            "K4": "V4",
+            "K2": "V2"
+          },
+          {
+            "K1": "V1",
+            "K3": {
+              "K3K2": "K3V2",
+              "K3K1": "K3V1",
+              "K3K3": {
+                "K3K3K2": "K3K3V2",
+                "K3K3K1": "K3K3V1",
+                "K3K3K3": "K3K3V3"
+              }
+            },
+            "K2": "V2"
+          }
+        ]
+      }
+    }
+  ]
+}
diff --git a/...t/java/com/linkedin/avroutil1/compatibility/avro19/AvroUtilSchemaNormalizationTest19.java b/...t/java/com/linkedin/avroutil1/compatibility/avro19/AvroUtilSchemaNormalizationTest19.java
@@ -116,6 +116,19 @@ public void testCanonicalBroadNoPlugin() throws IOException {
     Assert.assertTrue(canonicalizedSchema.getFields().get(3).aliases().toString().equals("[afield_alias, field_alias]"));
   }
 
+  @Test
+  public void testNestedObjectJsonPropertySerialization() throws IOException {
+    Schema schema1 = Schema.parse(TestUtil.load("PropRecord1.avsc"));
+    Schema schema2 = Schema.parse(TestUtil.load("PropRecord2.avsc"));
+    AvscGenerationConfig avscGenerationConfig =
+        new AvscGenerationConfig(false, false, false, Optional.of(Boolean.FALSE), false, true, true, true, true, true,
+            true, false, false);
+    String schemaStr1 = AvroUtilSchemaNormalization.getCanonicalForm(schema1, avscGenerationConfig, null);
+    String schemaStr2 = AvroUtilSchemaNormalization.getCanonicalForm(schema2, avscGenerationConfig, null);
+
+    Assert.assertEquals(schemaStr1, schemaStr2);
+  }
+
   @Test
   public void testCanonicalStrictWithNonSpecificJsonIncluded() throws IOException {
     AvscGenerationConfig config = new AvscGenerationConfig(

diff --git a/helper/tests/helper-tests-19/src/test/resources/PropRecord1.avsc b/helper/tests/helper-tests-19/src/test/resources/PropRecord1.avsc
@@ -0,0 +1,55 @@
+{
+  "type": "record",
+  "namespace": "com.acme",
+  "name": "RecordWithStuff",
+  "doc": "Here is a great overall doc",
+  "aliases": [
+    "record_alias",
+    "a_record_alias"
+  ],
+  "fields": [
+    {
+      "name": "strTest",
+      "type": {
+        "type": "string",
+        "avro.java.string": "String"
+      },
+      "some_prop": {
+        "KEY1": "VALUE1",
+        "KEY2": "VALUE2",
+        "ObjKey": {
+          "K1": "V1",
+          "K2": "V2",
+          "K3": "V3",
+          "K4": "V4",
+          "K5": {
+            "K5K1": "K5V1",
+            "K5K2": "K5V2",
+            "K5K3": "K5V3"
+          }
+        },
+        "ArObjKey": [
+          {
+            "K1": "V1",
+            "K2": "V2",
+            "K3": "V3",
+            "K4": "V4"
+          },
+          {
+            "K1": "V1",
+            "K2": "V2",
+            "K3": {
+              "K3K1": "K3V1",
+              "K3K2": "K3V2",
+              "K3K3": {
+                "K3K3K1": "K3K3V1",
+                "K3K3K2": "K3K3V2",
+                "K3K3K3": "K3K3V3"
+              }
+            }
+          }
+        ]
+      }
+    }
+  ]
+}