From 85a9e53c80720d3061c5d15c86e5f83f7e989616 Mon Sep 17 00:00:00 2001 From: Robert Yokota Date: Fri, 22 Dec 2023 11:38:54 -0800 Subject: [PATCH] DGS-9520 Also support allOf to singleton; add tests Also support changing a alleOf to a singleton. Add additional tests --- checkstyle/suppressions.xml | 2 +- .../json/diff/CombinedSchemaDiff.java | 27 ++--- .../diff-combined-schema-examples.json | 107 +++++++++++++++++- 3 files changed, 115 insertions(+), 21 deletions(-) diff --git a/checkstyle/suppressions.xml b/checkstyle/suppressions.xml index 30b147df6ee..ba6b376d745 100644 --- a/checkstyle/suppressions.xml +++ b/checkstyle/suppressions.xml @@ -40,7 +40,7 @@ files="(KafkaSchemaRegistry|SchemaRegistryConfig|ProtobufData|JsonSchemaData).java"/> + files="(AvroData|JsonSchema|KafkaSchemaRegistry|ProtobufData|ProtobufSchema|CelExecutor|FieldRuleExecutor|MetadataEncoderService|MockDekRegistryClient|DataEncryptionKey|KeyEncryptionKey|Rule|CombinedSchemaDiff).java"/> diff --git a/json-schema-provider/src/main/java/io/confluent/kafka/schemaregistry/json/diff/CombinedSchemaDiff.java b/json-schema-provider/src/main/java/io/confluent/kafka/schemaregistry/json/diff/CombinedSchemaDiff.java index 3026dd8ca62..7f56d6ac2b0 100644 --- a/json-schema-provider/src/main/java/io/confluent/kafka/schemaregistry/json/diff/CombinedSchemaDiff.java +++ b/json-schema-provider/src/main/java/io/confluent/kafka/schemaregistry/json/diff/CombinedSchemaDiff.java @@ -56,10 +56,11 @@ static void compare( ctx.addDifference(SUM_TYPE_EXTENDED); } } else if (originalSize > updateSize) { - if (update.getCriterion() == CombinedSchema.ALL_CRITERION) { - ctx.addDifference(PRODUCT_TYPE_NARROWED); - } else { + if (original.getCriterion() == CombinedSchema.ANY_CRITERION + || original.getCriterion() == CombinedSchema.ONE_CRITERION) { ctx.addDifference(SUM_TYPE_NARROWED); + } else { + ctx.addDifference(PRODUCT_TYPE_NARROWED); } } @@ -104,23 +105,11 @@ private static Difference.Type compareCriteria( Difference.Type type; if (originalCriterion.equals(updateCriterion)) { type = null; - } else if (updateCriterion == CombinedSchema.ANY_CRITERION) { - // Allow allOf/oneOf to anyOf + } else if (updateCriterion == CombinedSchema.ANY_CRITERION + || (isSingleton(original) && isSingleton(update)) + || (isSingleton(original) && updateCriterion == CombinedSchema.ONE_CRITERION) + || (isSingleton(update) && originalCriterion == CombinedSchema.ALL_CRITERION)) { type = COMBINED_TYPE_EXTENDED; - } else if (updateCriterion == CombinedSchema.ONE_CRITERION) { - if (isSingleton(original)) { - // Allow singleton anyOf/allOf to oneOf - type = COMBINED_TYPE_EXTENDED; - } else { - type = COMBINED_TYPE_CHANGED; - } - } else if (updateCriterion == CombinedSchema.ALL_CRITERION) { - if (isSingleton(original) && isSingleton(update)) { - // Allow singleton oneOf/anyOf to singleton allOf - type = COMBINED_TYPE_EXTENDED; - } else { - type = COMBINED_TYPE_CHANGED; - } } else { type = COMBINED_TYPE_CHANGED; } diff --git a/json-schema-provider/src/test/resources/diff-combined-schema-examples.json b/json-schema-provider/src/test/resources/diff-combined-schema-examples.json index 31e59c0be31..0be81c05dd8 100644 --- a/json-schema-provider/src/test/resources/diff-combined-schema-examples.json +++ b/json-schema-provider/src/test/resources/diff-combined-schema-examples.json @@ -486,6 +486,41 @@ ], "compatible": true }, + { + "description": "Detect incompatible change from oneOf to anyOf schema with fewer properties", + "original_schema": { + "type": "object", + "properties": { + "prop1": { + "oneOf": [ + { + "type": "number" + }, + { + "type": "string" + } + ] + } + } + }, + "update_schema": { + "type": "object", + "properties": { + "prop1": { + "anyOf": [ + { + "type": "string" + } + ] + } + } + }, + "changes": [ + "COMBINED_TYPE_EXTENDED #/properties/prop1", + "SUM_TYPE_NARROWED #/properties/prop1" + ], + "compatible": false + }, { "description": "Detect compatible change from allOf to anyOf schema with more properties", "original_schema": { @@ -521,6 +556,41 @@ ], "compatible": true }, + { + "description": "Detect compatible change from allOf to anyOf schema with fewer properties", + "original_schema": { + "type": "object", + "properties": { + "prop1": { + "allOf": [ + { + "type": "number" + }, + { + "type": "string" + } + ] + } + } + }, + "update_schema": { + "type": "object", + "properties": { + "prop1": { + "anyOf": [ + { + "type": "string" + } + ] + } + } + }, + "changes": [ + "COMBINED_TYPE_EXTENDED #/properties/prop1", + "PRODUCT_TYPE_NARROWED #/properties/prop1" + ], + "compatible": true + }, { "description": "Detect compatible change from anyOf to oneOf schema with more properties", "original_schema": { @@ -557,7 +627,7 @@ "compatible": true }, { - "description": "Detect incompatible change from anyOf with more properties to oneOf schema", + "description": "Detect incompatible change from anyOf to oneOf schema with fewer properties", "original_schema": { "type": "object", "properties": { @@ -625,6 +695,41 @@ ], "compatible": true }, + { + "description": "Detect compatible change from allOf to oneOf schema with fewer properties", + "original_schema": { + "type": "object", + "properties": { + "prop1": { + "allOf": [ + { + "type": "number" + }, + { + "type": "string" + } + ] + } + } + }, + "update_schema": { + "type": "object", + "properties": { + "prop1": { + "oneOf": [ + { + "type": "string" + } + ] + } + } + }, + "changes": [ + "COMBINED_TYPE_EXTENDED #/properties/prop1", + "PRODUCT_TYPE_NARROWED #/properties/prop1" + ], + "compatible": true + }, { "description": "Detect compatible change from anyOf to allOf schema", "original_schema": {