DGS-9520 Also support allOf to singleton; add tests

Also support changing a alleOf to a singleton. Add additional tests
confluentinc · Jan 9, 2024 · 85a9e53 · 85a9e53
1 parent 630e805
commit 85a9e53
Show file tree

Hide file tree

Showing 3 changed files with 115 additions and 21 deletions.
diff --git a/checkstyle/suppressions.xml b/checkstyle/suppressions.xml
@@ -40,7 +40,7 @@
               files="(KafkaSchemaRegistry|SchemaRegistryConfig|ProtobufData|JsonSchemaData).java"/>
 
     <suppress checks="BooleanExpressionComplexity"
-              files="(AvroData|JsonSchema|KafkaSchemaRegistry|ProtobufData|ProtobufSchema|CelExecutor|FieldRuleExecutor|MetadataEncoderService|MockDekRegistryClient|DataEncryptionKey|KeyEncryptionKey|Rule).java"/>
+              files="(AvroData|JsonSchema|KafkaSchemaRegistry|ProtobufData|ProtobufSchema|CelExecutor|FieldRuleExecutor|MetadataEncoderService|MockDekRegistryClient|DataEncryptionKey|KeyEncryptionKey|Rule|CombinedSchemaDiff).java"/>
 
     <suppress checks="MemberName"
               files="(DynamicSchema|EnumDefinition|MessageDefinition|ServiceDefinition).java"/>

diff --git a/...rovider/src/main/java/io/confluent/kafka/schemaregistry/json/diff/CombinedSchemaDiff.java b/...rovider/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
@@ -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": {