From dfd19006c07af592abcca0e62774740eb667350a Mon Sep 17 00:00:00 2001 From: Uttam Kumar Date: Thu, 11 Jan 2024 12:03:35 -0800 Subject: [PATCH] Canonicalization : sorting json property values by key if they are object (#530) --- .../compatibility/Jackson1Utils.java | 26 +++++++++ .../compatibility/Jackson2Utils.java | 31 +++++++++++ .../avro110/Avro110AvscWriter.java | 2 +- .../avro111/Avro111AvscWriter.java | 2 +- .../avro17/Avro17AvscWriter.java | 2 +- .../avro18/Avro18AvscWriter.java | 2 +- .../avro19/Avro19AvscWriter.java | 2 +- .../AvroUtilSchemaNormalizationTest17.java | 13 +++++ .../src/test/resources/PropRecord1.avsc | 55 +++++++++++++++++++ .../src/test/resources/PropRecord2.avsc | 55 +++++++++++++++++++ .../AvroUtilSchemaNormalizationTest19.java | 13 +++++ .../src/test/resources/PropRecord1.avsc | 55 +++++++++++++++++++ .../src/test/resources/PropRecord2.avsc | 55 +++++++++++++++++++ 13 files changed, 308 insertions(+), 5 deletions(-) create mode 100644 helper/tests/helper-tests-17/src/test/resources/PropRecord1.avsc create mode 100644 helper/tests/helper-tests-17/src/test/resources/PropRecord2.avsc create mode 100644 helper/tests/helper-tests-19/src/test/resources/PropRecord1.avsc create mode 100644 helper/tests/helper-tests-19/src/test/resources/PropRecord2.avsc 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 index 74027d541..19a70e61a 100644 --- 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 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 index 07cd08e29..44fa66c0f 100644 --- 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 sortedMap = new TreeMap<>(); + + Iterator> fields = jsonNode.fields(); + while (fields.hasNext()) { + Map.Entry 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/helper/impls/helper-impl-110/src/main/java/com/linkedin/avroutil1/compatibility/avro110/Avro110AvscWriter.java b/helper/impls/helper-impl-110/src/main/java/com/linkedin/avroutil1/compatibility/avro110/Avro110AvscWriter.java index 3cf09bbbd..e27e332c9 100644 --- a/helper/impls/helper-impl-110/src/main/java/com/linkedin/avroutil1/compatibility/avro110/Avro110AvscWriter.java +++ b/helper/impls/helper-impl-110/src/main/java/com/linkedin/avroutil1/compatibility/avro110/Avro110AvscWriter.java @@ -125,7 +125,7 @@ private void writeProps(Map props, Jackson2JsonGeneratorWrapper JsonGenerator delegate = gen.getDelegate(); for (Map.Entry 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/helper/impls/helper-impl-111/src/main/java/com/linkedin/avroutil1/compatibility/avro111/Avro111AvscWriter.java b/helper/impls/helper-impl-111/src/main/java/com/linkedin/avroutil1/compatibility/avro111/Avro111AvscWriter.java index 655586b6f..63313c21d 100644 --- a/helper/impls/helper-impl-111/src/main/java/com/linkedin/avroutil1/compatibility/avro111/Avro111AvscWriter.java +++ b/helper/impls/helper-impl-111/src/main/java/com/linkedin/avroutil1/compatibility/avro111/Avro111AvscWriter.java @@ -124,7 +124,7 @@ private void writeProps(Map props, Jackson2JsonGeneratorWrapper JsonGenerator delegate = gen.getDelegate(); for (Map.Entry 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/helper/impls/helper-impl-17/src/main/java/com/linkedin/avroutil1/compatibility/avro17/Avro17AvscWriter.java b/helper/impls/helper-impl-17/src/main/java/com/linkedin/avroutil1/compatibility/avro17/Avro17AvscWriter.java index d215307c1..0fac9018a 100644 --- a/helper/impls/helper-impl-17/src/main/java/com/linkedin/avroutil1/compatibility/avro17/Avro17AvscWriter.java +++ b/helper/impls/helper-impl-17/src/main/java/com/linkedin/avroutil1/compatibility/avro17/Avro17AvscWriter.java @@ -143,7 +143,7 @@ private void writeProps( for (Map.Entry 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/helper/impls/helper-impl-18/src/main/java/com/linkedin/avroutil1/compatibility/avro18/Avro18AvscWriter.java b/helper/impls/helper-impl-18/src/main/java/com/linkedin/avroutil1/compatibility/avro18/Avro18AvscWriter.java index c9e2d05d3..a07f44a06 100644 --- a/helper/impls/helper-impl-18/src/main/java/com/linkedin/avroutil1/compatibility/avro18/Avro18AvscWriter.java +++ b/helper/impls/helper-impl-18/src/main/java/com/linkedin/avroutil1/compatibility/avro18/Avro18AvscWriter.java @@ -143,7 +143,7 @@ private void writeProps( for (Map.Entry 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/helper/impls/helper-impl-19/src/main/java/com/linkedin/avroutil1/compatibility/avro19/Avro19AvscWriter.java b/helper/impls/helper-impl-19/src/main/java/com/linkedin/avroutil1/compatibility/avro19/Avro19AvscWriter.java index fdc40ecaa..03710c5ee 100644 --- a/helper/impls/helper-impl-19/src/main/java/com/linkedin/avroutil1/compatibility/avro19/Avro19AvscWriter.java +++ b/helper/impls/helper-impl-19/src/main/java/com/linkedin/avroutil1/compatibility/avro19/Avro19AvscWriter.java @@ -125,7 +125,7 @@ private void writeProps(Map props, Jackson2JsonGeneratorWrapper JsonGenerator delegate = gen.getDelegate(); for (Map.Entry 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/helper/tests/helper-tests-17/src/test/java/com/linkedin/avroutil1/compatibility/avro17/AvroUtilSchemaNormalizationTest17.java b/helper/tests/helper-tests-17/src/test/java/com/linkedin/avroutil1/compatibility/avro17/AvroUtilSchemaNormalizationTest17.java index 24eb366f2..3efc907bd 100644 --- a/helper/tests/helper-tests-17/src/test/java/com/linkedin/avroutil1/compatibility/avro17/AvroUtilSchemaNormalizationTest17.java +++ b/helper/tests/helper-tests-17/src/test/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 new file mode 100644 index 000000000..ed2a3a0b5 --- /dev/null +++ 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 new file mode 100644 index 000000000..7b42dc49a --- /dev/null +++ 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/helper/tests/helper-tests-19/src/test/java/com/linkedin/avroutil1/compatibility/avro19/AvroUtilSchemaNormalizationTest19.java b/helper/tests/helper-tests-19/src/test/java/com/linkedin/avroutil1/compatibility/avro19/AvroUtilSchemaNormalizationTest19.java index fc6871b8b..c19dd7934 100644 --- a/helper/tests/helper-tests-19/src/test/java/com/linkedin/avroutil1/compatibility/avro19/AvroUtilSchemaNormalizationTest19.java +++ b/helper/tests/helper-tests-19/src/test/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 new file mode 100644 index 000000000..ed2a3a0b5 --- /dev/null +++ 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" + } + } + } + ] + } + } + ] +} diff --git a/helper/tests/helper-tests-19/src/test/resources/PropRecord2.avsc b/helper/tests/helper-tests-19/src/test/resources/PropRecord2.avsc new file mode 100644 index 000000000..7b42dc49a --- /dev/null +++ b/helper/tests/helper-tests-19/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" + } + ] + } + } + ] +}