From 329275eef8575652294ddac665a88a2c86589d09 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Fri, 4 Sep 2020 10:12:10 +0530 Subject: [PATCH 01/30] Issue101:relocating one package Signed-off-by: Atharva Joshi --- build.gradle | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/build.gradle b/build.gradle index dc34b181b..77322956d 100644 --- a/build.gradle +++ b/build.gradle @@ -270,7 +270,7 @@ project('serializers:avro') { classifier ='all' mergeServiceFiles() } - + tasks.build.dependsOn tasks.shadowJar artifacts { archives shadowJar } javadoc { @@ -391,7 +391,7 @@ project('serializers') { testCompile group: 'io.pravega', name: 'pravega-test-testcommon', version: pravegaVersion testCompile group: 'io.pravega', name: 'pravega-client', version: pravegaVersion } - + shadowJar { zip64 true relocate 'org.xerial.snappy' , 'io.pravega.schemaregistry.shaded.org.xerial.snappy' @@ -409,7 +409,7 @@ project('serializers') { classifier ='all' mergeServiceFiles() } - + tasks.build.dependsOn tasks.shadowJar artifacts { archives shadowJar } javadoc { @@ -508,7 +508,7 @@ project('test') { compile project(':common') compile project(':contract') compile project(':client') - compile project(':server') + compile project(':server') compile project(':serializers') compile project(':serializers:protobuf') compile project(':serializers:avro') @@ -733,5 +733,4 @@ class DockerPushTask extends Exec { return tag } } -} - +} \ No newline at end of file From d59ea5436ed88ce528bffca804c5f3c182291596 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Fri, 4 Sep 2020 20:18:30 +0530 Subject: [PATCH 02/30] build with fat jar & thin jar Signed-off-by: Atharva Joshi --- build.gradle | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/build.gradle b/build.gradle index 77322956d..a8f4905da 100644 --- a/build.gradle +++ b/build.gradle @@ -391,6 +391,18 @@ project('serializers') { testCompile group: 'io.pravega', name: 'pravega-test-testcommon', version: pravegaVersion testCompile group: 'io.pravega', name: 'pravega-client', version: pravegaVersion } + + shadowJar { + //classifier = 'tests' + //from sourceSets.test.output + //configurations = [project.configurations.testRuntime] + zip64 true + relocate 'org.apache.avro', 'io.pravega.schemaregistry.shaded.org.apache.avro' + classifier ='all' + mergeServiceFiles() + } + + tasks.build.dependsOn tasks.shadowJar shadowJar { zip64 true From 969d1139805a3ffc7ae38272a79b83a003c5295f Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Tue, 6 Oct 2020 20:05:53 +0530 Subject: [PATCH 03/30] json compatibility checker first commit Signed-off-by: Atharva Joshi --- .../JsonCompatibilityChecker.java | 269 ++++++++++++++++++ .../JsonCompatibilityCheckerTest.java | 236 +++++++++++++++ 2 files changed, 505 insertions(+) create mode 100644 server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java create mode 100644 server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java new file mode 100644 index 000000000..c5cabf82c --- /dev/null +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java @@ -0,0 +1,269 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.google.common.collect.Iterators; +import com.google.common.collect.Sets; +import io.pravega.schemaregistry.contract.data.SchemaInfo; +import io.pravega.schemaregistry.rules.CompatibilityChecker; + +import java.io.IOException; +import java.util.*; +import java.util.stream.Collectors; + +public class JsonCompatibilityChecker implements CompatibilityChecker { + private static List characteristics = Arrays.asList("properties", "dependencies", "required", "additionalProperties"); + + @Override + public boolean canRead(SchemaInfo readUsing, List writtenUsing) { + return false; + } + + @Override + public boolean canBeRead(SchemaInfo writtenUsing, List readUsing) { + return false; + } + + @Override + public boolean canMutuallyRead(SchemaInfo schema, List schemaList) { + return false; + } + + private enum PossibleDifferences { + VARIABLE_TYPE_CHANGED, + VARIABLE_ADDED, + VARIABLE_DELETED, + DEPENDENCIES_ADDED, + DEPENDENCIES_REMOVED, + DEPENDENCY_VALUE_ADDED, + DEPENDENCY_VALUE_REMOVED, + DEPENDENCY_KEY_ADDED, + DEPENDENCY_KEY_REMOVED, + ARRAY_ELEMENT_ADDED, + ARRAY_ELEMENT_REMOVED, + ARRAY_ELEMENT_CHANGED, + PROPERTIES_ADDED, + PROPERTIES_REMOVED, + PROPERTY_ADDED, + PROPERTY_REMOVED, + PROPERTY_CHANGED, + REQUIREMENTS_ADDED, + REQUIREMENTS_REMOVED, + REQUIRED_VALUE_ADDED, + REQUIRED_VALUE_REMOVED + } + Queue path = new ArrayDeque<>(); + + public void relay(SchemaInfo toValidate, SchemaInfo toValidateAgainst) throws IOException { + ObjectMapper objectMapper = new ObjectMapper(); + JsonNode toCheck = objectMapper.readTree(toValidate.getSchemaData().array()); + JsonNode toCheckAgainst = objectMapper.readTree(toValidateAgainst.getSchemaData().array()); + ArrayList ans = getDifferences(toCheck, toCheckAgainst); + if(!ans.isEmpty()){ + System.out.println("Invalid"); + //temp print statement + System.out.println(ans); + }else{ + System.out.println("Valid"); + } + // toCheckAgainst + + // compute compatibility + + } + + private ArrayList getDifferences(JsonNode toCheck, JsonNode toCheckAgainst){ + ArrayList differences = new ArrayList<>(); + //cover cases when either are null + if(checkOrdinaryObject(toCheck, toCheckAgainst)){ + differences.addAll(getOrdinaryObjectProperties(toCheck,toCheckAgainst)); + return differences; + } + //check for properties + ArrayList propDifferences = compareProperties(toCheck, toCheckAgainst); + if(!propDifferences.isEmpty()) + differences.addAll(propDifferences); + //check for required + ArrayList reqDifferences = compareRequired(toCheck, toCheckAgainst); + if(!reqDifferences.isEmpty()) + differences.addAll(reqDifferences); + //check for dependencies + ArrayList dependencyDifferences = compareDependencies(toCheck, toCheckAgainst); + if(!dependencyDifferences.isEmpty()) + differences.addAll(dependencyDifferences); + //when both are values + if(toCheck.isValueNode() && toCheckAgainst.isValueNode()){ + if(!toCheck.asText().equals(toCheckAgainst.asText())){ + differences.add(PossibleDifferences.VARIABLE_TYPE_CHANGED); + } + } + //when both are arrays + if(toCheck.isArray() && toCheckAgainst.isArray()) + differences.addAll(getArrayDifferences(toCheck, toCheckAgainst)); + return differences; + } + + private ArrayList getArrayDifferences(JsonNode toCheck, JsonNode toCheckAgainst){ + HashSet toCheckCollection = new HashSet<>(); + HashSet toCheckAgainstCollection = new HashSet<>(); + ArrayNode arrayNodeToCheck = (ArrayNode) toCheck; + ArrayNode arrayNodeToCheckAgainst = (ArrayNode) toCheckAgainst; + for(int i=0;i differences = new ArrayList<>( + Sets.difference(toCheckCollection, toCheckAgainstCollection).stream().map( + node -> PossibleDifferences.ARRAY_ELEMENT_ADDED).collect( + Collectors.toCollection(ArrayList::new))); + Sets.difference(toCheckAgainstCollection, toCheckCollection).stream().map( + node -> PossibleDifferences.ARRAY_ELEMENT_REMOVED).forEach(differences::add); + return differences; + } + + private ArrayList compareProperties(JsonNode toCheck, JsonNode toCheckAgainst){ + ArrayList differences = new ArrayList<>(); + toCheck = toCheck.get("properties"); + toCheckAgainst = toCheckAgainst.get("properties"); + if(toCheck == null && toCheckAgainst!=null){ + differences.add(PossibleDifferences.PROPERTIES_REMOVED); + return differences; + } + else if(toCheck!=null && toCheckAgainst==null){ + differences.add(PossibleDifferences.PROPERTIES_ADDED); + return differences; + } + else if(toCheck==null && toCheckAgainst==null) return differences; + Iterator keys = Iterators.concat(toCheck.fieldNames(), toCheckAgainst.fieldNames()); + while(keys.hasNext()){ + String key = keys.next(); + //System.out.println(key); + if(toCheck.get(key)==null && toCheckAgainst.get(key)!=null) + differences.add(PossibleDifferences.PROPERTY_REMOVED); + else if(toCheck.get(key)!=null&&toCheckAgainst.get(key)==null) + differences.add(PossibleDifferences.PROPERTY_ADDED); + else{ + if(!toCheck.get(key).equals(toCheckAgainst.get(key))) { + differences.addAll(getDifferences(toCheck.get(key), toCheckAgainst.get(key))); + differences.add(PossibleDifferences.PROPERTY_CHANGED); + } + } + } + return differences; + } + + private ArrayList compareRequired(JsonNode toCheck, JsonNode toCheckAgainst){ + ArrayList differences = new ArrayList<>(); + toCheck = toCheck.get("required"); + toCheckAgainst = toCheckAgainst.get("required"); + if(toCheck==null && toCheckAgainst==null) return differences; + else if(toCheck!=null && toCheckAgainst==null){ + differences.add(PossibleDifferences.REQUIREMENTS_ADDED); + return differences; + } + else if(toCheck==null && toCheckAgainst!=null){ + differences.add(PossibleDifferences.REQUIREMENTS_REMOVED); + return differences; + } + ArrayList differencesReq = getArrayDifferences(toCheck, toCheckAgainst); + for(PossibleDifferences possibleDifference : differencesReq){ + switch (possibleDifference){ + case ARRAY_ELEMENT_ADDED: differences.add(PossibleDifferences.REQUIRED_VALUE_ADDED); + break; + case ARRAY_ELEMENT_REMOVED: differences.add(PossibleDifferences.REQUIRED_VALUE_REMOVED); + break; + } + } + return differences; + } + + private ArrayList compareDependencies(JsonNode toCheck, JsonNode toCheckAgainst){ + ArrayList differences = new ArrayList<>(); + toCheck = toCheck.get("dependencies"); + toCheckAgainst = toCheckAgainst.get("dependencies"); + if(toCheck==null && toCheckAgainst==null) + return differences; + else if(toCheck==null && toCheckAgainst!=null){ + differences.add(PossibleDifferences.DEPENDENCIES_REMOVED); + return differences; + } + else if(toCheck!=null && toCheck==null){ + differences.add(PossibleDifferences.DEPENDENCIES_ADDED); + return differences; + } + String testKeyToCheck = toCheck.fieldNames().next(); + String testKeyToCheckAgainst = toCheckAgainst.fieldNames().next(); + if(toCheck.get(testKeyToCheck).isArray() && toCheckAgainst.get(testKeyToCheckAgainst).isArray()) + differences.addAll(propertyDependencies(toCheck, toCheckAgainst)); + else if(toCheck.get(testKeyToCheck).isObject() && toCheckAgainst.get(testKeyToCheckAgainst).isObject()) + differences.addAll(schemaDependencies(toCheck, toCheckAgainst)); + else + System.err.println("dependency format mismatch"); + return differences; + } + + private ArrayList propertyDependencies(JsonNode toCheck, JsonNode toCheckAgainst){ + ArrayList differences = new ArrayList<>(); + Iterator keys = Iterators.concat(toCheck.fieldNames(), toCheckAgainst.fieldNames()); + while(keys.hasNext()){ + String key = keys.next(); + System.out.println(key); + if(toCheck.get(key)==null && toCheckAgainst.get(key)!=null) + differences.add(PossibleDifferences.DEPENDENCY_KEY_REMOVED); + else if(toCheck.get(key)!=null && toCheckAgainst.get(key)==null) + differences.add(PossibleDifferences.DEPENDENCY_KEY_ADDED); + else{ + if(!toCheck.get(key).equals(toCheckAgainst.get(key))){ + ArrayList elementDifferences = getArrayDifferences(toCheck.get(key), toCheckAgainst.get(key)); + for(PossibleDifferences possibleDifference: elementDifferences){ + switch (possibleDifference){ + case ARRAY_ELEMENT_ADDED: differences.add(PossibleDifferences.DEPENDENCY_VALUE_ADDED); + break; + case ARRAY_ELEMENT_REMOVED: differences.add(PossibleDifferences.DEPENDENCY_VALUE_REMOVED); + break; + } + } + } + } + } + return differences; + } + + private ArrayList schemaDependencies(JsonNode toCheck, JsonNode toCheckAgainst){ + //treating schema dependencies and differences between them as schemas and differences between schemas respectively + ArrayList differences = new ArrayList<>(); + differences.addAll(getDifferences(toCheck, toCheckAgainst)); + return differences; + } + + private ArrayList getOrdinaryObjectProperties(JsonNode toCheck, JsonNode toCheckAgainst){ + ArrayList differences = new ArrayList<>(); + Iterator keys = Iterators.concat(toCheck.fieldNames(), toCheckAgainst.fieldNames()); + while(keys.hasNext()){ + String key = keys.next(); + if(toCheck.get(key)==null && toCheckAgainst.get(key)!=null) + differences.add(PossibleDifferences.VARIABLE_ADDED); + else if(toCheck.get(key)!=null&&toCheckAgainst.get(key)==null) + differences.add(PossibleDifferences.VARIABLE_DELETED); + else{ + if(!toCheck.get(key).equals(toCheckAgainst.get(key))) { + differences.addAll(getDifferences(toCheck.get(key), toCheckAgainst.get(key))); + } + } + } + return differences; + } + + private boolean checkOrdinaryObject(JsonNode toCheck, JsonNode toCheckAgainst){ + for(String field : characteristics) { + if (toCheck.has(field) || toCheckAgainst.has(field)) + return false; + } + if(toCheck.isValueNode() && toCheckAgainst.isValueNode()) + return false; + return true; + } +} diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java new file mode 100644 index 000000000..ebd2d2ef7 --- /dev/null +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java @@ -0,0 +1,236 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.google.common.collect.ImmutableMap; +import io.pravega.schemaregistry.contract.data.SchemaInfo; +import io.pravega.schemaregistry.contract.data.SerializationFormat; +import io.pravega.schemaregistry.rules.jsoncompatibility.JsonCompatibilityChecker; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; + +public class JsonCompatibilityCheckerTest { + + @Test + public void testCanRead(){ + JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); + String x = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": {\n" + + "\"properties\": {\n" + + "\"billing_address\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"billing_address\"]\n" + + "}\n" + + "}\n" + + "}\n"; + String y = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": {\n" + + "\"properties\": {\n" + + "\"billing_address\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"billing_address\"]\n" + + "}\n" + + "}\n" + + "}\n"; + SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x.getBytes()), ImmutableMap.of()); + List toValidateAgainst = new ArrayList<>(); + SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(y.getBytes()), ImmutableMap.of()); + toValidateAgainst.add(schemaInfo1); + Assert.assertTrue(jsonCompatibilityChecker.canMutuallyRead(toValidate, toValidateAgainst)); + } + + @Test + public void testPrintNodes() throws IOException { + JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); + String x = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": {\n" + + "\"properties\": {\n" + + "\"billing_address\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"billing_address\"]\n" + + "}\n" + + "}\n" + + "}\n"; + String y = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": {\n" + + "\"properties\": {\n" + + "\"billing_address\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"billing_address\"]\n" + + "}\n" + + "}\n" + + "}\n"; + String z = "{\n"+ + "\"type\": \"object\",\n"+ + "\"properties\": {\n"+ + "\"first_name\": { \"type\": \"string\" },\n"+ + "\"last_name\": { \"type\": \"string\" },\n"+ + "\"birthday\": { \"type\": \"string\", \"format\": \"date\" },\n"+ + "\"address\": {\n"+ + "\"type\": \"object\",\n"+ + "\"properties\": {\n"+ + "\"street_address\": { \"type\": \"string\" },\n"+ + "\"city\": { \"type\": \"string\" },\n"+ + "\"state\": { \"type\": \"string\" },\n"+ + "\"country\": { \"type\" : \"string\" }\n"+ + "},\n"+ + "\"required\": [\"city\"]\n"+ + "}\n"+ + "},\n"+ + "\"required\": [\"first_name\"]\n"+ + "}\n"; + SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x.getBytes()), ImmutableMap.of()); + SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(y.getBytes()), ImmutableMap.of()); + SchemaInfo schemaInfo11 = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(z.getBytes()), ImmutableMap.of()); + jsonCompatibilityChecker.relay(toValidate, schemaInfo11); + } + + @Test + public void testDependencies() throws IOException { + JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); + String x1 = "{\n"+ + "\"type\": \"object\",\n"+ + "\"properties\": {\n"+ + "\"name\": { \"type\": \"string\" },\n"+ + "\"credit_card\": { \"type\": \"number\" },\n"+ + "\"billing_address\": { \"type\": \"string\" }\n"+ + "},\n"+ + "\"required\": [\"name\"],\n"+ + "\"dependencies\": {\n"+ + "\"credit_card\": [\"billing_address\"],\n"+ + "\"billing_address\": [\"credit_card\"]\n"+ + "}\n"+ + "}\n"; + String x2 = "{\n"+ + "\"type\": \"object\",\n"+ + "\"properties\": {\n"+ + "\"name\": { \"type\": \"string\" },\n"+ + "\"credit_card\": { \"type\": \"number\" },\n"+ + "\"billing_address\": { \"type\": \"string\" }\n"+ + "},\n"+ + "\"required\": [\"name\"],\n"+ + "\"dependencies\": {\n"+ + "\"credit_card\": [\"billing_address\"]\n"+ + "}\n"+ + "}\n"; + SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); + SchemaInfo toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); + jsonCompatibilityChecker.relay(toValidate, toValidateAgainst); + } + + @Test + public void testBasicProperties() throws IOException { + JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); + String x1 = "{\n"+ + "\"type\": \"object\",\n"+ + "\"properties\": {\n"+ + "\"number\": { \"type\": \"number\" },\n"+ + "\"street_name\": { \"type\": \"string\" },\n"+ + "\"street_type\": { \"type\": \"string\"}\n"+ + "}\n"+ + "}\n"; + String x2 = "{\n"+ + "\"type\": \"object\",\n"+ + "\"properties\": {\n"+ + "\"number\": { \"type\": \"number\" },\n"+ + "\"street_name\": { \"type\": \"string\" },\n"+ + "\"street_type\": { \"type\": \"string\"}\n"+ + "}\n"+ + "}\n"; + + SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x1.getBytes()), ImmutableMap.of()); + SchemaInfo toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); + jsonCompatibilityChecker.relay(toValidate, toValidateAgainst); + + //different properties + x2 = "{\n"+ + "\"type\": \"object\",\n"+ + "\"properties\": {\n"+ + "\"number\": { \"type\": \"number\" },\n"+ + "\"street_name1\": { \"type\": \"string\" },\n"+ + "\"street_type\": { \"type\": \"string\"}\n"+ + "}\n"+ + "}\n"; + toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x1.getBytes()), ImmutableMap.of()); + toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); + jsonCompatibilityChecker.relay(toValidate, toValidateAgainst); + + //different property values + x2 = "{\n"+ + "\"type\": \"object\",\n"+ + "\"properties\": {\n"+ + "\"number\": { \"type\": \"number\" },\n"+ + "\"street_name\": { \"type\": \"number\" },\n"+ + "\"street_type\": { \"type\": \"string\"}\n"+ + "}\n"+ + "}\n"; + toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x1.getBytes()), ImmutableMap.of()); + toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); + jsonCompatibilityChecker.relay(toValidate, toValidateAgainst); + } + + @Test + public void testRequired() throws IOException { + JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); + // equal test + String x1 = "{\n"+ + "\"type\": \"object\",\n"+ + "\"properties\": {\n"+ + "\"name\": { \"type\": \"string\" },\n"+ + "\"email\": { \"type\": \"string\" },\n"+ + "\"address\": { \"type\": \"string\" },\n"+ + "\"telephone\": { \"type\": \"string\" }\n"+ + "},\n"+ + "\"required\": [\"name\", \"email\"]\n"+ + "}\n"; + SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x1.getBytes()), ImmutableMap.of()); + SchemaInfo toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x1.getBytes()), ImmutableMap.of()); + jsonCompatibilityChecker.relay(toValidate, toValidateAgainst); + + //remove required array element + String x2 = "{\n"+ + "\"type\": \"object\",\n"+ + "\"properties\": {\n"+ + "\"name\": { \"type\": \"string\" },\n"+ + "\"email\": { \"type\": \"string\" },\n"+ + "\"address\": { \"type\": \"string\" },\n"+ + "\"telephone\": { \"type\": \"string\" }\n"+ + "},\n"+ + "\"required\": [\"email\"]\n"+ + "}\n"; + toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); + jsonCompatibilityChecker.relay(toValidate, toValidateAgainst); + } +} + \ No newline at end of file From 31814439abb6ef2575e07c02110323087c5de9dc Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Sat, 17 Oct 2020 17:35:42 +0530 Subject: [PATCH 04/30] propertiesComparator Signed-off-by: Atharva Joshi --- .../BreakingChangesStore.java | 83 ++++++++++++++ .../DependenciesComparator.java | 13 +++ .../JsonCompatibilityChecker.java | 24 ++++ .../JsonCompatibilityCheckerUtils.java | 37 ++++++ .../ObjectTypeComparator.java | 18 +++ .../PropertiesComparator.java | 108 ++++++++++++++++++ 6 files changed, 283 insertions(+) create mode 100644 server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java create mode 100644 server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java create mode 100644 server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java create mode 100644 server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ObjectTypeComparator.java create mode 100644 server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java new file mode 100644 index 000000000..b770596f9 --- /dev/null +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java @@ -0,0 +1,83 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +public class BreakingChangesStore { + //include only those changes that lead to incompatibility + protected enum BreakingChanges { + // strings + MAX_LENGTH_ADDED, + MAX_LENGTH_DECREASED, + MIN_LENGTH_ADDED, + MIN_LENGTH_INCREASED, + PATTERN_ADDED, + PATTERN_CHANGED, + // NUMBERS + MAXIMUM_ADDED, + MAXIMUM_DECREASED, + EXCLUSIVE_MAXIMUM_ADDED, + EXCLUSIVE_MAXIMUM_DECREASED, + MINIMUM_ADDED, + MINIMUM_INCREASED, + EXCLUSIVE_MINIMUM_ADDED, + EXCLUSIVE_MINIMUM_INCREASED, + MULTIPLE_OF_ADDED, + MULTIPLE_OF_CHANGED, + MULTIPLE_OF_EXPANDED, + TYPE_NARROWED, + //ARRAYS + MAX_ITEMS_ADDED, + MAX_ITEMS_DECREASED, + MIN_ITEMS_ADDED, + MIN_ITEMS_INCREASED, + UNIQUE_ITEMS_ADDED, + ADDITIONAL_ITEMS_REMOVED, + ADDITIONAL_ITEMS_NARROWED, + ITEMS_REMOVED_NOT_COVERED_BY_PARTIALLY_OPEN_CONTENT_MODEL, + ITEM_REMOVED_NOT_COVERED_BY_PARTIALLY_OPEN_CONTENT_MODEL, + ITEM_REMOVED_FROM_CLOSED_CONTENT_MODEL, + ITEM_ADDED_TO_OPEN_CONTENT_MODEL, + ITEM_ADDED_NOT_COVERED_BY_PARTIALLY_OPEN_CONTENT_MODEL, + // PROPERTIES + PROPERTIES_SECTION_ADDED, // this would be taken care of by checking for required and additional properties since that is when incompatibility arises + PROPERTY_REMOVED_NOT_PART_OF_DYNAMIC_PROPERTY_SET_WITH_CONDITION, // check on updated schema + PROPERTY_REMOVED_FROM_STATIC_PROPERTY_SET, // check on updated schema + PROPERTY_ADDED_TO_DYNAMIC_PROPERTY_SET, // check on original schema + PROPERTY_ADDED_NOT_PART_OF_DYNAMIC_PROPERTY_SET_WITH_CONDITION, // check on original schema + REQUIRED_PROPERTY_ADDED_WITHOUT_DEFAULT, + REQUIRED_ATTRIBUTE_ADDED, // may not be needed + MAX_PROPERTIES_ADDED, + MAX_PROPERTIES_LIMIT_DECREASED, + MIN_PROPERTIES_ADDED, + MIN_PROPERTIES_LIMIT_INCREASED, + ADDITIONAL_PROPERTIES_REMOVED, + ADDITIONAL_PROPERTIES_NARROWED, + // DEPENDENCIES + DEPENDENCY_ARRAY_ADDED, + DEPENDENCY_ARRAY_EXTENDED, + DEPENDENCY_ARRAY_CHANGED, + DEPENDENCY_SCHEMA_ADDED, + // ENUM + ENUM_ARRAY_NARROWED, + ENUM_ARRAY_CHANGED, + // NOT TYPE + NOT_TYPE_EXTENDED, + // COMBINED + COMBINED_TYPE_SUBSCHEMAS_CHANGED, + COMPOSITION_METHOD_CHANGED, + PRODUCT_TYPE_EXTENDED, + SUM_TYPE_NARROWED + } + + private List breakingChangesList = new ArrayList<>(); + + private void computeBreakingChanges() { + breakingChangesList = Arrays.asList(BreakingChanges.values()); + } + + public List getBreakingChangesList() { + return breakingChangesList; + } +} diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java new file mode 100644 index 000000000..2aa5e3b7f --- /dev/null +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java @@ -0,0 +1,13 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; + +import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.*; + +public class DependenciesComparator { + + public BreakingChanges checkDependencies(JsonNode toCheck, JsonNode toCheckAgainst) { + + return null; + } +} diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java index c5cabf82c..2f605c52f 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java @@ -12,8 +12,11 @@ import java.util.*; import java.util.stream.Collectors; +import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.*; + public class JsonCompatibilityChecker implements CompatibilityChecker { private static List characteristics = Arrays.asList("properties", "dependencies", "required", "additionalProperties"); + JsonCompatibilityCheckerUtils jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); @Override public boolean canRead(SchemaInfo readUsing, List writtenUsing) { @@ -72,6 +75,27 @@ public void relay(SchemaInfo toValidate, SchemaInfo toValidateAgainst) throws IO // compute compatibility } + + protected BreakingChanges checkNodeType(JsonNode toCheck, JsonNode toCheckAgainst) { + String nodeType = jsonCompatibilityCheckerUtils.getTypeValue(toCheck).equals(jsonCompatibilityCheckerUtils.getTypeValue(toCheckAgainst)) ? jsonCompatibilityCheckerUtils.getTypeValue(toCheck) : "mismatch"; + switch (nodeType) { + case "object" : + break; + case "number" : + break; + case "string" : + break; + case "array" : + break; + case "boolean" : + break; + case "null": + break; + case "mismatch" : + break; + } + return null; + } private ArrayList getDifferences(JsonNode toCheck, JsonNode toCheckAgainst){ ArrayList differences = new ArrayList<>(); diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java new file mode 100644 index 000000000..d9ce8c99c --- /dev/null +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java @@ -0,0 +1,37 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ArrayNode; + +public class JsonCompatibilityCheckerUtils { + + public String getTypeValue(JsonNode node) { + String value = null; + while(node.fieldNames().hasNext()) { + if (node.fieldNames().next().equals("type")) + value = node.get("type").textValue(); + } + return value; + } + + public boolean hasDynamicPropertySet(JsonNode node) { + if(node.get("additionalProperties")==null && node.get("patternProperties")==null) + return true; + return false; + } + + public boolean hasStaticPropertySet(JsonNode node) { + if(node.get("patternProperties") == null && node.get("additionalProperties").asText() == "false") + return true; + return false; + } + + public boolean isInRequired(String toSearch, JsonNode toSearchIn) { + if(toSearchIn.get("required") != null) { + ArrayNode arrayToSearch = (ArrayNode) toSearchIn.get("required"); + if(arrayToSearch.has(toSearch)) + return true; + } + return false; + } +} diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ObjectTypeComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ObjectTypeComparator.java new file mode 100644 index 000000000..e5e245f9d --- /dev/null +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ObjectTypeComparator.java @@ -0,0 +1,18 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; + +import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.*; + +public class ObjectTypeComparator { + + public BreakingChanges checkAspects(JsonNode toCheck, JsonNode toCheckAgainst) { + // will check for properties,dependencies, required, additional properties by calling required classes. + PropertiesComparator propertiesComparator = new PropertiesComparator(); + BreakingChanges propertiesDifference = propertiesComparator.checkProperties(toCheck, toCheckAgainst); + if (propertiesDifference != null) + return propertiesDifference; + return null; + } + +} diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java new file mode 100644 index 000000000..f1633e690 --- /dev/null +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java @@ -0,0 +1,108 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.google.common.collect.Iterators; +import io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.BreakingChanges; +import java.util.Iterator; + +public class PropertiesComparator { + JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); + JsonCompatibilityCheckerUtils jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); + public BreakingChanges checkProperties(JsonNode toCheck, JsonNode toCheckAgainst) { + Iterator propertyFields = Iterators.concat(toCheck.get("properties").fieldNames(), toCheckAgainst.get("properties").fieldNames()); + while(propertyFields.hasNext()) { + String propertyField = propertyFields.next(); + if(toCheck.get("properties").get(propertyField) == null) { + // property has been removed from toCheck + if(jsonCompatibilityCheckerUtils.hasStaticPropertySet(toCheck)) + return BreakingChanges.PROPERTY_REMOVED_FROM_STATIC_PROPERTY_SET; + if(!jsonCompatibilityCheckerUtils.hasStaticPropertySet(toCheck) && !jsonCompatibilityCheckerUtils.hasDynamicPropertySet(toCheck)) { + // assume that pattern properties always matches + // TO DO: add regex check for pattern properties + BreakingChanges breakingChanges = jsonCompatibilityChecker.checkNodeType( + toCheck.get("properties").get("additionalProperties"), + toCheckAgainst.get("properties").get(propertyField)); + if(breakingChanges!= null) + return BreakingChanges.PROPERTY_REMOVED_NOT_PART_OF_DYNAMIC_PROPERTY_SET_WITH_CONDITION; + } + } + else if(toCheckAgainst.get("properties").get(propertyField) == null) { + // property has been added to toCheck + if(jsonCompatibilityCheckerUtils.hasDynamicPropertySet(toCheckAgainst)) + return BreakingChanges.PROPERTY_ADDED_TO_DYNAMIC_PROPERTY_SET; + //check if required property in toCheck + if(toCheck.get("required") != null) { + if (jsonCompatibilityCheckerUtils.isInRequired(propertyField, toCheck)) { + if (toCheck.get("properties").get(propertyField).get("default") == null) + return BreakingChanges.REQUIRED_PROPERTY_ADDED_WITHOUT_DEFAULT; + } + } + if(!jsonCompatibilityCheckerUtils.hasStaticPropertySet(toCheckAgainst) && !jsonCompatibilityCheckerUtils.hasDynamicPropertySet(toCheckAgainst)) { + // assume that pattern properties always matches + // TO DO: add regex check for pattern properties + BreakingChanges breakingChanges = jsonCompatibilityChecker.checkNodeType( + toCheck.get("properties").get(propertyField), + toCheckAgainst.get("properties").get("additionalProperties")); + if(breakingChanges!= null) + return BreakingChanges.PROPERTY_ADDED_NOT_PART_OF_DYNAMIC_PROPERTY_SET_WITH_CONDITION; + } + } + } + // check for min-max conditions on properties + BreakingChanges minMaxBreakingChanges = minMaxProperties(toCheck, toCheckAgainst); + if(minMaxBreakingChanges!=null) + return minMaxBreakingChanges; + // check for additional properties + BreakingChanges additionalPropsBreakingChanges = additionalProperties(toCheck, toCheckAgainst); + if (additionalPropsBreakingChanges != null) + return additionalPropsBreakingChanges; + // check for required properties + BreakingChanges requiredPropsBreakingChanges = requiredProperties(toCheck, toCheckAgainst); + if(requiredPropsBreakingChanges != null) + return requiredPropsBreakingChanges; + return null; + } + + private BreakingChanges minMaxProperties(JsonNode toCheck, JsonNode toCheckAgainst) { + // minProperties + if(toCheck.get("minProperties") != null && toCheckAgainst.get("minProperties") == null) + return BreakingChanges.MIN_PROPERTIES_ADDED; + else if (toCheck.get("minProperties") != null && toCheckAgainst.get("minProperties") != null) { + if(toCheck.get("minProperties").intValue() > toCheckAgainst.get("minProperties").intValue()) + return BreakingChanges.MIN_PROPERTIES_LIMIT_INCREASED; + } + // maxProperties + if(toCheck.get("maxProperties") != null && toCheckAgainst.get("maxProperties") != null) + return BreakingChanges.MAX_PROPERTIES_ADDED; + else if(toCheck.get("maxProperties") != null && toCheckAgainst.get("maxProperties") != null) { + if(toCheck.get("maxProperties").intValue() < toCheckAgainst.get("maxProperties").intValue()) + return BreakingChanges.MAX_PROPERTIES_LIMIT_DECREASED; + } + return null; + } + + private BreakingChanges additionalProperties(JsonNode toCheck, JsonNode toCheckAgainst) { + if(toCheck.get("additionalProperties") == null && toCheckAgainst.get("additionalProperties") != null) + return BreakingChanges.ADDITIONAL_PROPERTIES_REMOVED; + else if(toCheck.get("aaditionalProperties") != null && toCheckAgainst.get("additinalProperties") != null){ + BreakingChanges additionalPropertiesBreakingChanges = jsonCompatibilityChecker.checkNodeType(toCheck.get("additionalProperties"), toCheckAgainst.get("additionalProperties")); + if(additionalPropertiesBreakingChanges != null) + return additionalPropertiesBreakingChanges; + } + return null; + } + + private BreakingChanges requiredProperties(JsonNode toCheck, JsonNode toCheckAgainst) { + ArrayNode arrayNodeToCheck = (ArrayNode) toCheck.get("required"); + if(arrayNodeToCheck != null) { + for(int i=0;i < arrayNodeToCheck.size();i++) { + if(!jsonCompatibilityCheckerUtils.isInRequired(arrayNodeToCheck.get(i).textValue(), toCheckAgainst)) { + if(toCheck.get("properties").get(arrayNodeToCheck.get(i).textValue()).get("default") == null) + return BreakingChanges.REQUIRED_PROPERTY_ADDED_WITHOUT_DEFAULT; + } + } + } + return null; + } +} From 50ba7d7e5696b2352668d9e5f1ad73f2c3b4152a Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Mon, 19 Oct 2020 20:23:19 +0530 Subject: [PATCH 05/30] dependency comparator Signed-off-by: Atharva Joshi --- .../ArrayTypeComparator.java | 11 +++++ .../BreakingChangesStore.java | 9 ++-- .../DependenciesComparator.java | 48 ++++++++++++++++++- .../PropertiesComparator.java | 2 +- 4 files changed, 63 insertions(+), 7 deletions(-) create mode 100644 server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java new file mode 100644 index 000000000..4da2a356c --- /dev/null +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java @@ -0,0 +1,11 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; + +public class ArrayTypeComparator { + + public BreakingChangesStore.BreakingChanges compareArrays (JsonNode toCheck, JsonNode toCheckAgainst) { + + return null; + } +} diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java index b770596f9..3d92398be 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java @@ -55,10 +55,11 @@ protected enum BreakingChanges { ADDITIONAL_PROPERTIES_REMOVED, ADDITIONAL_PROPERTIES_NARROWED, // DEPENDENCIES - DEPENDENCY_ARRAY_ADDED, - DEPENDENCY_ARRAY_EXTENDED, - DEPENDENCY_ARRAY_CHANGED, - DEPENDENCY_SCHEMA_ADDED, + DEPENDENCY_SECTION_ADDED, + DEPENDENCY_ADDED_IN_ARRAY_FORM, + DEPENDENCY_ARRAY_ELEMENTS_CHANGED, + DEPENDENCY_ADDED_IN_SCHEMA_FORM, + DEPENDENCY_IN_SCHEMA_FORM_MODIFIED, // ENUM ENUM_ARRAY_NARROWED, ENUM_ARRAY_CHANGED, diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java index 2aa5e3b7f..1ee233734 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java @@ -1,13 +1,57 @@ package io.pravega.schemaregistry.rules.jsoncompatibility; import com.fasterxml.jackson.databind.JsonNode; +import com.google.common.collect.Iterators; + +import java.util.Iterator; import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.*; public class DependenciesComparator { - + ArrayTypeComparator arrayTypeComparator = new ArrayTypeComparator(); + PropertiesComparator propertiesComparator = new PropertiesComparator(); public BreakingChanges checkDependencies(JsonNode toCheck, JsonNode toCheckAgainst) { - + if(toCheck.get("dependencies") != null && toCheckAgainst.get("dependencies") == null) + return BreakingChanges.DEPENDENCY_SECTION_ADDED; + else if(toCheck.get("dependencies") == null && toCheckAgainst.get("dependencies") == null) + return null; + else if(toCheck.get("dependencies") == null && toCheckAgainst.get("dependencies") != null) + return null; + Iterator dependencyFields = Iterators.concat(toCheck.get("dependencies").fieldNames(), toCheckAgainst.get("dependencies").fieldNames()); + boolean dependencyTypeIsArray = checkDependencyTypeIsArray(toCheck.get("dependencies"), toCheckAgainst.get("dependencies")); + while(dependencyFields.hasNext()) { + String field = dependencyFields.next(); + if(toCheck.get("dependencies").get(field) != null && toCheckAgainst.get("dependencies").get(field)==null) { + if(dependencyTypeIsArray) + return BreakingChanges.DEPENDENCY_ADDED_IN_ARRAY_FORM; + else + return BreakingChanges.DEPENDENCY_ADDED_IN_SCHEMA_FORM; + } + else if (toCheck.get("dependencies").get(field) != null && toCheckAgainst.get("dependencies").get(field) != null) { + if(dependencyTypeIsArray) { + // check the value returned by the array comparator + if(arrayTypeComparator.compareArrays(toCheck.get("dependencies").get(field), + toCheckAgainst.get("dependencies").get(field)) != null) + return BreakingChanges.DEPENDENCY_ARRAY_ELEMENTS_CHANGED; + } + else { + if(propertiesComparator.checkProperties(toCheck.get("dependencies").get(field), + toCheckAgainst.get("dependencies").get(field)) != null) + return BreakingChanges.DEPENDENCY_IN_SCHEMA_FORM_MODIFIED; + } + } + } return null; } + + private boolean checkDependencyTypeIsArray(JsonNode toCheck, JsonNode toCheckAgainst) { + // it is assumed that the dependency type does not change from array to schema or vice versa. + Iterator toCheckFields = toCheck.fieldNames(); + String toCheckSample = toCheckFields.next(); + Iterator toCheckAgainstFields = toCheckAgainst.fieldNames(); + String toCheckAgainstSample = toCheckAgainstFields.next(); + if(toCheck.get(toCheckSample).isArray() && toCheckAgainst.get(toCheckAgainstSample).isArray()) + return true; + return false; + } } diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java index f1633e690..6a33ec3a2 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java @@ -73,7 +73,7 @@ else if (toCheck.get("minProperties") != null && toCheckAgainst.get("minProperti return BreakingChanges.MIN_PROPERTIES_LIMIT_INCREASED; } // maxProperties - if(toCheck.get("maxProperties") != null && toCheckAgainst.get("maxProperties") != null) + if(toCheck.get("maxProperties") != null && toCheckAgainst.get("maxProperties") == null) return BreakingChanges.MAX_PROPERTIES_ADDED; else if(toCheck.get("maxProperties") != null && toCheckAgainst.get("maxProperties") != null) { if(toCheck.get("maxProperties").intValue() < toCheckAgainst.get("maxProperties").intValue()) From 60e5767f6294076470c1b6b1659b74c785860f14 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Thu, 22 Oct 2020 20:14:00 +0530 Subject: [PATCH 06/30] ArrayCompatibility Signed-off-by: Atharva Joshi --- .../ArrayTypeComparator.java | 109 ++++++++++++- .../BreakingChangesStore.java | 19 +-- .../JsonCompatibilityChecker.java | 150 +++++++++--------- 3 files changed, 194 insertions(+), 84 deletions(-) diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java index 4da2a356c..804222c4a 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java @@ -1,11 +1,114 @@ package io.pravega.schemaregistry.rules.jsoncompatibility; import com.fasterxml.jackson.databind.JsonNode; +import com.google.common.collect.Iterators; + +import java.util.Iterator; + +import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.BreakingChanges; public class ArrayTypeComparator { - - public BreakingChangesStore.BreakingChanges compareArrays (JsonNode toCheck, JsonNode toCheckAgainst) { - + JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); + public BreakingChanges compareArrays(JsonNode toCheck, JsonNode toCheckAgainst) { + if (toCheck.isArray() && toCheckAgainst.isArray()) + return arraySimpleBodyComparision(toCheck, toCheckAgainst); + else { + if (checkUniqueItems(toCheck, toCheckAgainst) != null) + return checkUniqueItems(toCheck, toCheckAgainst); + BreakingChanges minMaxChanges = minMaxItems(toCheck, toCheckAgainst); + if (minMaxChanges != null) + return minMaxChanges; + BreakingChanges additionalItemsChanges = additionalItems(toCheck, toCheckAgainst); + if (additionalItemsChanges != null) + return additionalItemsChanges; + } + return null; + } + + private BreakingChanges arraySimpleBodyComparision(JsonNode toCheck, JsonNode toCheckAgainst) { + Iterator allNodes = Iterators.concat(toCheck.fieldNames(), toCheckAgainst.fieldNames()); + while(allNodes.hasNext()) { + String item = allNodes.next(); + if(!toCheck.has(item)) + return BreakingChanges.ARRAY_SIMPLE_BODY_CHECK_ELEMENT_REMOVED; + else if (!toCheckAgainst.has(item)) + return BreakingChanges.ARRAY_SIMPLE_BODY_CHECK_ELEMENT_ADDED; + } return null; } + + private BreakingChanges checkUniqueItems(JsonNode toCheck, JsonNode toCheckAgainst) { + if (toCheck.get("uniqueItems").isBoolean() && toCheck.get("uniqueItems").asText() == "true") { + if (toCheckAgainst.get("uniqueItems") == null) + return BreakingChanges.ARRAY_UNIQUE_ITEMS_CONDITION_ENABLED; + } + return null; + } + + private BreakingChanges minMaxItems(JsonNode toCheck, JsonNode toCheckAgainst) { + if (toCheck.get("maxItems") != null && toCheckAgainst.get("maxItems") == null) + return BreakingChanges.ARRAY_MAX_ITEMS_CONDITION_ADDED; + else if (toCheck.get("maxItems") != null && toCheckAgainst.get("maxItems") != null) { + int originalMaxLimit = toCheckAgainst.get("maxItems").asInt(); + int changedMaxLimit = toCheck.get("maxItems").asInt(); + if (changedMaxLimit < originalMaxLimit) + return BreakingChanges.ARRAY_MAX_ITEMS_VALUE_DECREASED; + } + if (toCheck.get("minItems") != null && toCheckAgainst.get("minItems") == null) + return BreakingChanges.ARRAY_MIN_ITEMS_CONDITION_ADDED; + else if (toCheck.get("minItems") != null && toCheckAgainst.get("minItems") != null) { + int originalMinLimit = toCheckAgainst.get("minItems").asInt(); + int changedMinLimit = toCheck.get("minItems").asInt(); + if (changedMinLimit > originalMinLimit) + return BreakingChanges.ARRAY_MIN_ITEMS_VALUE_INCREASED; + } + return null; + } + + private BreakingChanges additionalItems(JsonNode toCheck, JsonNode toCheckAgainst) { + if (toCheck.get("additionalItems") != null && toCheckAgainst.get("additionalItems") == null) { + if (toCheck.get("additionalItems").isBoolean() && toCheck.get("additionalItems").asText() == "false") + return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_DISABLED; + else if (toCheck.get("additionalItems").isObject()) + return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_DECREASED; + } else if (toCheck.get("additionalItems") != null && toCheckAgainst.get("additionalItems") != null) { + if (toCheck.get("additionalItems").isBoolean() && toCheck.get("additionalItems").asText() == "false") { + if(!(toCheckAgainst.get("additionalItems").asText() == "false")) + return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_DISABLED; + } + else if (toCheck.get("additionalItems").isObject()) { + if(toCheckAgainst.get("additionalItems").isObject()) { + if(jsonCompatibilityChecker.checkNodeType(toCheck.get("additionalItems"), toCheckAgainst.get("additionalItems")) != null) + return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_DECREASED; + else if (toCheckAgainst.get("additionalItems").isBoolean() && toCheckAgainst.get("additionalItems").asText() == "true") + return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_DECREASED; + } + } + } + return null; + } + + private boolean isDynamicArray(JsonNode node) { + if(node.get("additionalItems") == null) + return true; + else if(node.get("additionalItems").isBoolean()) { + if(node.get("additionalItems").asText() == "true") + return true; + } + return false; + } + + private boolean isDynamicArrayWithCondition(JsonNode node) { + if(node.get("additionalItems").isObject()) + return true; + return false; + } + + private boolean isStaticArray(JsonNode node) { + if(node.get("additionalItems").isBoolean()) { + if(node.get("additionalItems").asText() == "false") + return true; + } + return false; + } } diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java index 3d92398be..71eb532e3 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java @@ -5,7 +5,7 @@ import java.util.List; public class BreakingChangesStore { - //include only those changes that lead to incompatibility + //includes only those changes that lead to incompatibility protected enum BreakingChanges { // strings MAX_LENGTH_ADDED, @@ -28,14 +28,15 @@ protected enum BreakingChanges { MULTIPLE_OF_EXPANDED, TYPE_NARROWED, //ARRAYS - MAX_ITEMS_ADDED, - MAX_ITEMS_DECREASED, - MIN_ITEMS_ADDED, - MIN_ITEMS_INCREASED, - UNIQUE_ITEMS_ADDED, - ADDITIONAL_ITEMS_REMOVED, - ADDITIONAL_ITEMS_NARROWED, - ITEMS_REMOVED_NOT_COVERED_BY_PARTIALLY_OPEN_CONTENT_MODEL, + ARRAY_MAX_ITEMS_CONDITION_ADDED, + ARRAY_MAX_ITEMS_VALUE_DECREASED, + ARRAY_MIN_ITEMS_CONDITION_ADDED, + ARRAY_MIN_ITEMS_VALUE_INCREASED, + ARRAY_UNIQUE_ITEMS_CONDITION_ENABLED, + ARRAY_ADDITIONAL_ITEMS_DISABLED, + ARRAY_ADDITIONAL_ITEMS_SCOPE_DECREASED, + ARRAY_SIMPLE_BODY_CHECK_ELEMENT_REMOVED, + ARRAY_SIMPLE_BODY_CHECK_ELEMENT_ADDED, ITEM_REMOVED_NOT_COVERED_BY_PARTIALLY_OPEN_CONTENT_MODEL, ITEM_REMOVED_FROM_CLOSED_CONTENT_MODEL, ITEM_ADDED_TO_OPEN_CONTENT_MODEL, diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java index 2f605c52f..678f53f94 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java @@ -15,7 +15,8 @@ import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.*; public class JsonCompatibilityChecker implements CompatibilityChecker { - private static List characteristics = Arrays.asList("properties", "dependencies", "required", "additionalProperties"); + private static List characteristics = Arrays.asList("properties", "dependencies", "required", + "additionalProperties"); JsonCompatibilityCheckerUtils jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); @Override @@ -56,6 +57,7 @@ private enum PossibleDifferences { REQUIRED_VALUE_ADDED, REQUIRED_VALUE_REMOVED } + Queue path = new ArrayDeque<>(); public void relay(SchemaInfo toValidate, SchemaInfo toValidateAgainst) throws IOException { @@ -63,11 +65,11 @@ public void relay(SchemaInfo toValidate, SchemaInfo toValidateAgainst) throws IO JsonNode toCheck = objectMapper.readTree(toValidate.getSchemaData().array()); JsonNode toCheckAgainst = objectMapper.readTree(toValidateAgainst.getSchemaData().array()); ArrayList ans = getDifferences(toCheck, toCheckAgainst); - if(!ans.isEmpty()){ + if (!ans.isEmpty()) { System.out.println("Invalid"); //temp print statement System.out.println(ans); - }else{ + } else { System.out.println("Valid"); } // toCheckAgainst @@ -75,68 +77,70 @@ public void relay(SchemaInfo toValidate, SchemaInfo toValidateAgainst) throws IO // compute compatibility } - + protected BreakingChanges checkNodeType(JsonNode toCheck, JsonNode toCheckAgainst) { - String nodeType = jsonCompatibilityCheckerUtils.getTypeValue(toCheck).equals(jsonCompatibilityCheckerUtils.getTypeValue(toCheckAgainst)) ? jsonCompatibilityCheckerUtils.getTypeValue(toCheck) : "mismatch"; + String nodeType = jsonCompatibilityCheckerUtils.getTypeValue(toCheck).equals( + jsonCompatibilityCheckerUtils.getTypeValue( + toCheckAgainst)) ? jsonCompatibilityCheckerUtils.getTypeValue(toCheck) : "mismatch"; switch (nodeType) { - case "object" : + case "object": break; - case "number" : + case "number": break; - case "string" : + case "string": break; - case "array" : + case "array": break; - case "boolean" : + case "boolean": break; case "null": break; - case "mismatch" : + case "mismatch": break; } return null; } - private ArrayList getDifferences(JsonNode toCheck, JsonNode toCheckAgainst){ + private ArrayList getDifferences(JsonNode toCheck, JsonNode toCheckAgainst) { ArrayList differences = new ArrayList<>(); //cover cases when either are null - if(checkOrdinaryObject(toCheck, toCheckAgainst)){ - differences.addAll(getOrdinaryObjectProperties(toCheck,toCheckAgainst)); + if (checkOrdinaryObject(toCheck, toCheckAgainst)) { + differences.addAll(getOrdinaryObjectProperties(toCheck, toCheckAgainst)); return differences; } //check for properties ArrayList propDifferences = compareProperties(toCheck, toCheckAgainst); - if(!propDifferences.isEmpty()) + if (!propDifferences.isEmpty()) differences.addAll(propDifferences); //check for required ArrayList reqDifferences = compareRequired(toCheck, toCheckAgainst); - if(!reqDifferences.isEmpty()) + if (!reqDifferences.isEmpty()) differences.addAll(reqDifferences); //check for dependencies ArrayList dependencyDifferences = compareDependencies(toCheck, toCheckAgainst); - if(!dependencyDifferences.isEmpty()) + if (!dependencyDifferences.isEmpty()) differences.addAll(dependencyDifferences); //when both are values - if(toCheck.isValueNode() && toCheckAgainst.isValueNode()){ - if(!toCheck.asText().equals(toCheckAgainst.asText())){ + if (toCheck.isValueNode() && toCheckAgainst.isValueNode()) { + if (!toCheck.asText().equals(toCheckAgainst.asText())) { differences.add(PossibleDifferences.VARIABLE_TYPE_CHANGED); } } //when both are arrays - if(toCheck.isArray() && toCheckAgainst.isArray()) + if (toCheck.isArray() && toCheckAgainst.isArray()) differences.addAll(getArrayDifferences(toCheck, toCheckAgainst)); return differences; } - private ArrayList getArrayDifferences(JsonNode toCheck, JsonNode toCheckAgainst){ + private ArrayList getArrayDifferences(JsonNode toCheck, JsonNode toCheckAgainst) { HashSet toCheckCollection = new HashSet<>(); HashSet toCheckAgainstCollection = new HashSet<>(); ArrayNode arrayNodeToCheck = (ArrayNode) toCheck; ArrayNode arrayNodeToCheckAgainst = (ArrayNode) toCheckAgainst; - for(int i=0;i differences = new ArrayList<>( @@ -148,29 +152,27 @@ private ArrayList getArrayDifferences(JsonNode toCheck, Jso return differences; } - private ArrayList compareProperties(JsonNode toCheck, JsonNode toCheckAgainst){ + private ArrayList compareProperties(JsonNode toCheck, JsonNode toCheckAgainst) { ArrayList differences = new ArrayList<>(); toCheck = toCheck.get("properties"); toCheckAgainst = toCheckAgainst.get("properties"); - if(toCheck == null && toCheckAgainst!=null){ + if (toCheck == null && toCheckAgainst != null) { differences.add(PossibleDifferences.PROPERTIES_REMOVED); return differences; - } - else if(toCheck!=null && toCheckAgainst==null){ + } else if (toCheck != null && toCheckAgainst == null) { differences.add(PossibleDifferences.PROPERTIES_ADDED); return differences; - } - else if(toCheck==null && toCheckAgainst==null) return differences; + } else if (toCheck == null && toCheckAgainst == null) return differences; Iterator keys = Iterators.concat(toCheck.fieldNames(), toCheckAgainst.fieldNames()); - while(keys.hasNext()){ + while (keys.hasNext()) { String key = keys.next(); //System.out.println(key); - if(toCheck.get(key)==null && toCheckAgainst.get(key)!=null) + if (toCheck.get(key) == null && toCheckAgainst.get(key) != null) differences.add(PossibleDifferences.PROPERTY_REMOVED); - else if(toCheck.get(key)!=null&&toCheckAgainst.get(key)==null) + else if (toCheck.get(key) != null && toCheckAgainst.get(key) == null) differences.add(PossibleDifferences.PROPERTY_ADDED); - else{ - if(!toCheck.get(key).equals(toCheckAgainst.get(key))) { + else { + if (!toCheck.get(key).equals(toCheckAgainst.get(key))) { differences.addAll(getDifferences(toCheck.get(key), toCheckAgainst.get(key))); differences.add(PossibleDifferences.PROPERTY_CHANGED); } @@ -179,74 +181,77 @@ else if(toCheck.get(key)!=null&&toCheckAgainst.get(key)==null) return differences; } - private ArrayList compareRequired(JsonNode toCheck, JsonNode toCheckAgainst){ + private ArrayList compareRequired(JsonNode toCheck, JsonNode toCheckAgainst) { ArrayList differences = new ArrayList<>(); toCheck = toCheck.get("required"); toCheckAgainst = toCheckAgainst.get("required"); - if(toCheck==null && toCheckAgainst==null) return differences; - else if(toCheck!=null && toCheckAgainst==null){ + if (toCheck == null && toCheckAgainst == null) return differences; + else if (toCheck != null && toCheckAgainst == null) { differences.add(PossibleDifferences.REQUIREMENTS_ADDED); return differences; - } - else if(toCheck==null && toCheckAgainst!=null){ + } else if (toCheck == null && toCheckAgainst != null) { differences.add(PossibleDifferences.REQUIREMENTS_REMOVED); return differences; } ArrayList differencesReq = getArrayDifferences(toCheck, toCheckAgainst); - for(PossibleDifferences possibleDifference : differencesReq){ - switch (possibleDifference){ - case ARRAY_ELEMENT_ADDED: differences.add(PossibleDifferences.REQUIRED_VALUE_ADDED); + for (PossibleDifferences possibleDifference : differencesReq) { + switch (possibleDifference) { + case ARRAY_ELEMENT_ADDED: + differences.add(PossibleDifferences.REQUIRED_VALUE_ADDED); break; - case ARRAY_ELEMENT_REMOVED: differences.add(PossibleDifferences.REQUIRED_VALUE_REMOVED); + case ARRAY_ELEMENT_REMOVED: + differences.add(PossibleDifferences.REQUIRED_VALUE_REMOVED); break; } } return differences; } - private ArrayList compareDependencies(JsonNode toCheck, JsonNode toCheckAgainst){ + private ArrayList compareDependencies(JsonNode toCheck, JsonNode toCheckAgainst) { ArrayList differences = new ArrayList<>(); toCheck = toCheck.get("dependencies"); toCheckAgainst = toCheckAgainst.get("dependencies"); - if(toCheck==null && toCheckAgainst==null) + if (toCheck == null && toCheckAgainst == null) return differences; - else if(toCheck==null && toCheckAgainst!=null){ + else if (toCheck == null && toCheckAgainst != null) { differences.add(PossibleDifferences.DEPENDENCIES_REMOVED); return differences; - } - else if(toCheck!=null && toCheck==null){ + } else if (toCheck != null && toCheck == null) { differences.add(PossibleDifferences.DEPENDENCIES_ADDED); return differences; } String testKeyToCheck = toCheck.fieldNames().next(); String testKeyToCheckAgainst = toCheckAgainst.fieldNames().next(); - if(toCheck.get(testKeyToCheck).isArray() && toCheckAgainst.get(testKeyToCheckAgainst).isArray()) + if (toCheck.get(testKeyToCheck).isArray() && toCheckAgainst.get(testKeyToCheckAgainst).isArray()) differences.addAll(propertyDependencies(toCheck, toCheckAgainst)); - else if(toCheck.get(testKeyToCheck).isObject() && toCheckAgainst.get(testKeyToCheckAgainst).isObject()) + else if (toCheck.get(testKeyToCheck).isObject() && toCheckAgainst.get(testKeyToCheckAgainst).isObject()) differences.addAll(schemaDependencies(toCheck, toCheckAgainst)); else System.err.println("dependency format mismatch"); return differences; } - private ArrayList propertyDependencies(JsonNode toCheck, JsonNode toCheckAgainst){ + private ArrayList propertyDependencies(JsonNode toCheck, JsonNode toCheckAgainst) { ArrayList differences = new ArrayList<>(); Iterator keys = Iterators.concat(toCheck.fieldNames(), toCheckAgainst.fieldNames()); - while(keys.hasNext()){ + while (keys.hasNext()) { String key = keys.next(); System.out.println(key); - if(toCheck.get(key)==null && toCheckAgainst.get(key)!=null) + if (toCheck.get(key) == null && toCheckAgainst.get(key) != null) differences.add(PossibleDifferences.DEPENDENCY_KEY_REMOVED); - else if(toCheck.get(key)!=null && toCheckAgainst.get(key)==null) + else if (toCheck.get(key) != null && toCheckAgainst.get(key) == null) differences.add(PossibleDifferences.DEPENDENCY_KEY_ADDED); - else{ - if(!toCheck.get(key).equals(toCheckAgainst.get(key))){ - ArrayList elementDifferences = getArrayDifferences(toCheck.get(key), toCheckAgainst.get(key)); - for(PossibleDifferences possibleDifference: elementDifferences){ - switch (possibleDifference){ - case ARRAY_ELEMENT_ADDED: differences.add(PossibleDifferences.DEPENDENCY_VALUE_ADDED); + else { + if (!toCheck.get(key).equals(toCheckAgainst.get(key))) { + ArrayList elementDifferences = getArrayDifferences(toCheck.get(key), + toCheckAgainst.get(key)); + for (PossibleDifferences possibleDifference : elementDifferences) { + switch (possibleDifference) { + case ARRAY_ELEMENT_ADDED: + differences.add(PossibleDifferences.DEPENDENCY_VALUE_ADDED); break; - case ARRAY_ELEMENT_REMOVED: differences.add(PossibleDifferences.DEPENDENCY_VALUE_REMOVED); + case ARRAY_ELEMENT_REMOVED: + differences.add(PossibleDifferences.DEPENDENCY_VALUE_REMOVED); break; } } @@ -256,24 +261,25 @@ else if(toCheck.get(key)!=null && toCheckAgainst.get(key)==null) return differences; } - private ArrayList schemaDependencies(JsonNode toCheck, JsonNode toCheckAgainst){ - //treating schema dependencies and differences between them as schemas and differences between schemas respectively + private ArrayList schemaDependencies(JsonNode toCheck, JsonNode toCheckAgainst) { + //treating schema dependencies and differences between them as schemas and differences between schemas + // respectively ArrayList differences = new ArrayList<>(); differences.addAll(getDifferences(toCheck, toCheckAgainst)); return differences; } - private ArrayList getOrdinaryObjectProperties(JsonNode toCheck, JsonNode toCheckAgainst){ + private ArrayList getOrdinaryObjectProperties(JsonNode toCheck, JsonNode toCheckAgainst) { ArrayList differences = new ArrayList<>(); Iterator keys = Iterators.concat(toCheck.fieldNames(), toCheckAgainst.fieldNames()); - while(keys.hasNext()){ + while (keys.hasNext()) { String key = keys.next(); - if(toCheck.get(key)==null && toCheckAgainst.get(key)!=null) + if (toCheck.get(key) == null && toCheckAgainst.get(key) != null) differences.add(PossibleDifferences.VARIABLE_ADDED); - else if(toCheck.get(key)!=null&&toCheckAgainst.get(key)==null) + else if (toCheck.get(key) != null && toCheckAgainst.get(key) == null) differences.add(PossibleDifferences.VARIABLE_DELETED); - else{ - if(!toCheck.get(key).equals(toCheckAgainst.get(key))) { + else { + if (!toCheck.get(key).equals(toCheckAgainst.get(key))) { differences.addAll(getDifferences(toCheck.get(key), toCheckAgainst.get(key))); } } @@ -281,12 +287,12 @@ else if(toCheck.get(key)!=null&&toCheckAgainst.get(key)==null) return differences; } - private boolean checkOrdinaryObject(JsonNode toCheck, JsonNode toCheckAgainst){ - for(String field : characteristics) { + private boolean checkOrdinaryObject(JsonNode toCheck, JsonNode toCheckAgainst) { + for (String field : characteristics) { if (toCheck.has(field) || toCheckAgainst.has(field)) return false; } - if(toCheck.isValueNode() && toCheckAgainst.isValueNode()) + if (toCheck.isValueNode() && toCheckAgainst.isValueNode()) return false; return true; } From 6becb24ac4de385facf6a1574f712aa01a811332 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Fri, 23 Oct 2020 18:40:20 +0530 Subject: [PATCH 07/30] NumbersComparator Signed-off-by: Atharva Joshi --- .../ArrayTypeComparator.java | 8 ++ .../BreakingChangesStore.java | 24 ++--- .../JsonCompatibilityChecker.java | 1 + .../jsoncompatibility/NumberComparator.java | 98 +++++++++++++++++++ 4 files changed, 119 insertions(+), 12 deletions(-) create mode 100644 server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/NumberComparator.java diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java index 804222c4a..1bc203ea0 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java @@ -21,6 +21,10 @@ public BreakingChanges compareArrays(JsonNode toCheck, JsonNode toCheckAgainst) BreakingChanges additionalItemsChanges = additionalItems(toCheck, toCheckAgainst); if (additionalItemsChanges != null) return additionalItemsChanges; + BreakingChanges itemsValidationChanges = itemValidation(toCheck, toCheckAgainst); + if(itemsValidationChanges != null) + return itemsValidationChanges; + //TO DO: Add contains and tupleValidation } return null; } @@ -88,6 +92,10 @@ else if (toCheckAgainst.get("additionalItems").isBoolean() && toCheckAgainst.get return null; } + private BreakingChanges itemValidation(JsonNode toCheck, JsonNode toCheckAgainst) { + return jsonCompatibilityChecker.checkNodeType(toCheck.get("items"), toCheckAgainst.get("items")); + } + private boolean isDynamicArray(JsonNode node) { if(node.get("additionalItems") == null) return true; diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java index 71eb532e3..779c4e8fd 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java @@ -15,18 +15,18 @@ protected enum BreakingChanges { PATTERN_ADDED, PATTERN_CHANGED, // NUMBERS - MAXIMUM_ADDED, - MAXIMUM_DECREASED, - EXCLUSIVE_MAXIMUM_ADDED, - EXCLUSIVE_MAXIMUM_DECREASED, - MINIMUM_ADDED, - MINIMUM_INCREASED, - EXCLUSIVE_MINIMUM_ADDED, - EXCLUSIVE_MINIMUM_INCREASED, - MULTIPLE_OF_ADDED, - MULTIPLE_OF_CHANGED, - MULTIPLE_OF_EXPANDED, - TYPE_NARROWED, + NUMBER_TYPE_MAXIMUM_VALUE_ADDED, + NUMBER_TYPE_MAXIMUM_VALUE_DECREASED, + NUMBER_TYPE_EXCLUSIVE_MAXIMUM_VALUE_ADDED, + NUMBER_TYPE_EXCLUSIVE_MAXIMUM_VALUE_DECREASED, + NUMBER_TYPE_MINIMUM_VALUE_ADDED, + NUMBER_TYPE_MINIMUM_VALUE_INCREASED, + NUMBER_TYPE_EXCLUSIVE_MINIMUM_VALUE_ADDED, + NUMBER_TYPE_EXCLUSIVE_MINIMUM_VALUE_INCREASED, + NUMBER_TYPE_MULTIPLE_OF_ADDED, + NUMBER_TYPE_MULTIPLE_OF_NON_DIVISIBLE_CHANGE, + NUMBER_TYPE_MULTIPLE_OF_INCREASED, + NUMBER_TYPE_CHANGED_FROM_NUMBER_TO_INTEGER, //ARRAYS ARRAY_MAX_ITEMS_CONDITION_ADDED, ARRAY_MAX_ITEMS_VALUE_DECREASED, diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java index 678f53f94..21ade91e1 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java @@ -86,6 +86,7 @@ protected BreakingChanges checkNodeType(JsonNode toCheck, JsonNode toCheckAgains case "object": break; case "number": + case "integer": break; case "string": break; diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/NumberComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/NumberComparator.java new file mode 100644 index 000000000..bdb39053e --- /dev/null +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/NumberComparator.java @@ -0,0 +1,98 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; + +import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.*; + +public class NumberComparator { + public BreakingChanges compareNumbers(JsonNode toCheck, JsonNode toCheckAgainst) { + if(maximumComparator(toCheck, toCheckAgainst) != null) + return maximumComparator(toCheck, toCheckAgainst); + if(minimumComparator(toCheck, toCheckAgainst) != null) + return minimumComparator(toCheck, toCheckAgainst); + if(multipleOfComparator(toCheck, toCheckAgainst) != null) + return multipleOfComparator(toCheck, toCheckAgainst); + if(typeChanged(toCheck, toCheckAgainst) != null) + return typeChanged(toCheck, toCheckAgainst); + return null; + //TO DO - Changes from exclusive max/min to max/min & vice versa. + } + + private BreakingChanges maximumComparator(JsonNode toCheck, JsonNode toCheckAgainst) { + if(toCheck.get("maximum") != null && toCheckAgainst.get("maximum") == null) + return BreakingChanges.NUMBER_TYPE_MAXIMUM_VALUE_ADDED; + else if(toCheck.get("maximum") != null && toCheckAgainst.get("maximum") != null) { + int toCheckAgainstMaximum = toCheckAgainst.get("maximum").intValue(); + int toCheckMaximum = toCheck.get("maximum").intValue(); + if(toCheckMaximum < toCheckAgainstMaximum) { + return BreakingChanges.NUMBER_TYPE_MAXIMUM_VALUE_DECREASED; + } + } + else if(toCheck.get("exclusiveMaximum") != null && toCheckAgainst.get("exclusiveMaximum") == null) { + return BreakingChanges.NUMBER_TYPE_EXCLUSIVE_MAXIMUM_VALUE_ADDED; + } + else if(toCheck.get("exclusiveMaximum") != null && toCheckAgainst.get("exclusiveMaximum") != null) { + if(toCheckAgainst.get("exclusiveMaximum").isBoolean()) { + if(toCheckAgainst.get("exclusiveMaximum").asText() == "false" && toCheck.get("exclusiveMaximum").asText() == "true") + return BreakingChanges.NUMBER_TYPE_EXCLUSIVE_MAXIMUM_VALUE_ADDED; + } + else { + int toCheckAgainstExclusiveMaximum = toCheckAgainst.get("exclusiveMaximum").asInt(); + int toCheckExclusiveMaximum = toCheck.get("exclusiveMaximum").asInt(); + if(toCheckExclusiveMaximum < toCheckAgainstExclusiveMaximum) + return BreakingChanges.NUMBER_TYPE_EXCLUSIVE_MAXIMUM_VALUE_DECREASED; + } + } + return null; + } + + private BreakingChanges minimumComparator(JsonNode toCheck, JsonNode toCheckAgainst) { + if(toCheck.get("minimum") != null && toCheckAgainst.get("minimum") == null) + return BreakingChanges.NUMBER_TYPE_MINIMUM_VALUE_ADDED; + else if(toCheck.get("minimum") != null && toCheckAgainst.get("minimum") != null) { + int toCheckAgainstMinimum = toCheckAgainst.get("minimum").intValue(); + int toCheckMinimum = toCheck.get("minimum").intValue(); + if(toCheckMinimum > toCheckAgainstMinimum) { + return BreakingChanges.NUMBER_TYPE_MINIMUM_VALUE_INCREASED; + } + } + else if(toCheck.get("exclusiveMinimum") != null && toCheckAgainst.get("exclusiveMinimum") == null) { + return BreakingChanges.NUMBER_TYPE_EXCLUSIVE_MINIMUM_VALUE_ADDED; + } + else if(toCheck.get("exclusiveMinimum") != null && toCheckAgainst.get("exclusiveMinimum") != null) { + if(toCheckAgainst.get("exclusiveMinimum").isBoolean()) { + if(toCheckAgainst.get("exclusiveMinimum").asText() == "false" && toCheck.get("exclusiveMinimum").asText() == "true") + return BreakingChanges.NUMBER_TYPE_EXCLUSIVE_MINIMUM_VALUE_ADDED; + } + else { + int toCheckAgainstExclusiveMinimum = toCheckAgainst.get("exclusiveMinimum").asInt(); + int toCheckExclusiveMinimum = toCheck.get("exclusiveMinimum").asInt(); + if(toCheckExclusiveMinimum > toCheckAgainstExclusiveMinimum) + return BreakingChanges.NUMBER_TYPE_EXCLUSIVE_MINIMUM_VALUE_INCREASED; + } + } + return null; + } + + private BreakingChanges multipleOfComparator(JsonNode toCheck, JsonNode toCheckAgainst) { + if(toCheck.get("multipleOf") != null && toCheckAgainst.get("multipleOf") == null) + return BreakingChanges.NUMBER_TYPE_MULTIPLE_OF_ADDED; + else if(toCheck.get("multipleOf") != null && toCheckAgainst.get("multipleOf") != null) { + int toCheckMultipleOf = toCheck.get("multipleOf").asInt(); + int toCheckAgainstMultipleOf = toCheckAgainst.get("multipleOf").asInt(); + if(toCheckAgainstMultipleOf != toCheckMultipleOf) { + if(toCheckMultipleOf%toCheckAgainstMultipleOf == 0) + return BreakingChanges.NUMBER_TYPE_MULTIPLE_OF_INCREASED; + else if (toCheckAgainstMultipleOf%toCheckMultipleOf != 0) + return BreakingChanges.NUMBER_TYPE_MULTIPLE_OF_NON_DIVISIBLE_CHANGE; + } + } + return null; + } + + private BreakingChanges typeChanged(JsonNode toCheck, JsonNode toCheckAgainst) { + if(toCheck.get("type").asText() == "integer" && toCheckAgainst.get("type").asText() == "number") + return BreakingChanges.NUMBER_TYPE_CHANGED_FROM_NUMBER_TO_INTEGER; + return null; + } +} From fea569dcdeee56e0baabe73e70cac5548004ade5 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Fri, 23 Oct 2020 22:40:53 +0530 Subject: [PATCH 08/30] StringComparator Signed-off-by: Atharva Joshi --- .../BreakingChangesStore.java | 12 ++--- .../jsoncompatibility/StringComparator.java | 52 +++++++++++++++++++ 2 files changed, 58 insertions(+), 6 deletions(-) create mode 100644 server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/StringComparator.java diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java index 779c4e8fd..95f4d9281 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java @@ -8,12 +8,12 @@ public class BreakingChangesStore { //includes only those changes that lead to incompatibility protected enum BreakingChanges { // strings - MAX_LENGTH_ADDED, - MAX_LENGTH_DECREASED, - MIN_LENGTH_ADDED, - MIN_LENGTH_INCREASED, - PATTERN_ADDED, - PATTERN_CHANGED, + STRING_TYPE_MAX_LENGTH_ADDED, + STRING_TYPE_MAX_LENGTH_VALUE_DECREASED, + STRING_TYPE_MIN_LENGTH_ADDED, + STRING_TYPE_MIN_LENGTH_VALUE_INCREASED, + STRING_TYPE_PATTERN_ADDED, + STRING_TYPE_PATTERN_MODIFIED, // NUMBERS NUMBER_TYPE_MAXIMUM_VALUE_ADDED, NUMBER_TYPE_MAXIMUM_VALUE_DECREASED, diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/StringComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/StringComparator.java new file mode 100644 index 000000000..20fc9e03f --- /dev/null +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/StringComparator.java @@ -0,0 +1,52 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; + +import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.*; + +public class StringComparator { + + public BreakingChanges stringComparator(JsonNode toCheck, JsonNode toCheckAgainst) { + if(minLengthComparator(toCheck, toCheckAgainst) != null) + return minLengthComparator(toCheck, toCheckAgainst); + if(maxLengthComparator(toCheck, toCheckAgainst) != null) + return maxLengthComparator(toCheck, toCheckAgainst); + if(patternComparator(toCheck, toCheckAgainst) != null) + return patternComparator(toCheck, toCheckAgainst); + return null; + } + + private BreakingChanges minLengthComparator(JsonNode toCheck, JsonNode toCheckAgainst) { + if(toCheck.get("minLength") != null && toCheckAgainst.get("minLength") == null) + return BreakingChanges.STRING_TYPE_MIN_LENGTH_ADDED; + else if(toCheck.get("minLength") != null && toCheckAgainst.get("minLength") != null) { + int toCheckMinLength = toCheck.get("minLength").asInt(); + int toCheckAgainstMinLength = toCheckAgainst.get("minLength").asInt(); + if(toCheckMinLength > toCheckAgainstMinLength) + return BreakingChanges.STRING_TYPE_MIN_LENGTH_VALUE_INCREASED; + } + return null; + } + + private BreakingChanges maxLengthComparator(JsonNode toCheck, JsonNode toCheckAgainst) { + if(toCheck.get("maxLength") != null && toCheckAgainst.get("minLength") == null) + return BreakingChanges.STRING_TYPE_MAX_LENGTH_ADDED; + else if(toCheck.get("maxLength") != null && toCheckAgainst.get("maxLength") != null) { + int toCheckMaxLength = toCheck.get("maxLength").asInt(); + int toCheckAgainstMaxLength = toCheckAgainst.get("maxLength").asInt(); + if(toCheckMaxLength < toCheckAgainstMaxLength) + return BreakingChanges.STRING_TYPE_MAX_LENGTH_VALUE_DECREASED; + } + return null; + } + + private BreakingChanges patternComparator(JsonNode toCheck, JsonNode toCheckAgainst) { + if(toCheck.get("pattern") != null && toCheckAgainst.get("patternProperties") == null) + return BreakingChanges.STRING_TYPE_PATTERN_ADDED; + else if (toCheck.get("pattern") != null && toCheckAgainst.get("pattern") != null) { + if(!toCheck.get("pattern").asText().equals(toCheckAgainst.get("pattern").asText())) + return BreakingChanges.STRING_TYPE_PATTERN_MODIFIED; + } + return null; + } +} From ddeb442c2cef9388970a0b8b580de7b1d6bb411d Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Sat, 24 Oct 2020 00:19:27 +0530 Subject: [PATCH 09/30] EnumCompatibility Signed-off-by: Atharva Joshi --- .../BreakingChangesStore.java | 6 +++--- .../DependenciesComparator.java | 9 +++++---- .../jsoncompatibility/EnumComparator.java | 19 +++++++++++++++++++ .../JsonCompatibilityChecker.java | 6 ++++++ .../JsonCompatibilityCheckerUtils.java | 18 ++++++++++++++++++ 5 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/EnumComparator.java diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java index 95f4d9281..708d5fe0e 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java @@ -58,12 +58,12 @@ protected enum BreakingChanges { // DEPENDENCIES DEPENDENCY_SECTION_ADDED, DEPENDENCY_ADDED_IN_ARRAY_FORM, - DEPENDENCY_ARRAY_ELEMENTS_CHANGED, + DEPENDENCY_ARRAY_ELEMENTS_NON_REMOVAL, DEPENDENCY_ADDED_IN_SCHEMA_FORM, DEPENDENCY_IN_SCHEMA_FORM_MODIFIED, // ENUM - ENUM_ARRAY_NARROWED, - ENUM_ARRAY_CHANGED, + ENUM_TYPE_ADDED, + ENUM_TYPE_ARRAY_CONTENTS_NON_ADDITION_OF_ELEMENTS, // NOT TYPE NOT_TYPE_EXTENDED, // COMBINED diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java index 1ee233734..38711ffa3 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java @@ -1,6 +1,7 @@ package io.pravega.schemaregistry.rules.jsoncompatibility; import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ArrayNode; import com.google.common.collect.Iterators; import java.util.Iterator; @@ -8,7 +9,7 @@ import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.*; public class DependenciesComparator { - ArrayTypeComparator arrayTypeComparator = new ArrayTypeComparator(); + JsonCompatibilityCheckerUtils jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); PropertiesComparator propertiesComparator = new PropertiesComparator(); public BreakingChanges checkDependencies(JsonNode toCheck, JsonNode toCheckAgainst) { if(toCheck.get("dependencies") != null && toCheckAgainst.get("dependencies") == null) @@ -30,9 +31,9 @@ else if(toCheck.get("dependencies") == null && toCheckAgainst.get("dependencies" else if (toCheck.get("dependencies").get(field) != null && toCheckAgainst.get("dependencies").get(field) != null) { if(dependencyTypeIsArray) { // check the value returned by the array comparator - if(arrayTypeComparator.compareArrays(toCheck.get("dependencies").get(field), - toCheckAgainst.get("dependencies").get(field)) != null) - return BreakingChanges.DEPENDENCY_ARRAY_ELEMENTS_CHANGED; + if(!jsonCompatibilityCheckerUtils.arrayComparisionOnlyRemoval((ArrayNode) toCheck.get("dependencies").get(field), + (ArrayNode) toCheckAgainst.get("dependencies").get(field))) + return BreakingChanges.DEPENDENCY_ARRAY_ELEMENTS_NON_REMOVAL; } else { if(propertiesComparator.checkProperties(toCheck.get("dependencies").get(field), diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/EnumComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/EnumComparator.java new file mode 100644 index 000000000..12492aafc --- /dev/null +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/EnumComparator.java @@ -0,0 +1,19 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ArrayNode; + +import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.*; + +public class EnumComparator { + JsonCompatibilityCheckerUtils jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); + public BreakingChanges enumComparator(JsonNode toCheck, JsonNode toCheckAgainst) { + if(toCheck.has("enum") && !toCheckAgainst.has("enum")) + return BreakingChanges.ENUM_TYPE_ADDED; + else if(toCheck.has("enum") && toCheck.has("enum")) { + if(!jsonCompatibilityCheckerUtils.arrayComparisionOnlyAddition((ArrayNode)toCheck.get("enum"), (ArrayNode)toCheckAgainst.get("enum"))) + return BreakingChanges.ENUM_TYPE_ARRAY_CONTENTS_NON_ADDITION_OF_ELEMENTS; + } + return null; + } +} diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java index 21ade91e1..f85178492 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java @@ -15,6 +15,7 @@ import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.*; public class JsonCompatibilityChecker implements CompatibilityChecker { + EnumComparator enumComparator = new EnumComparator(); private static List characteristics = Arrays.asList("properties", "dependencies", "required", "additionalProperties"); JsonCompatibilityCheckerUtils jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); @@ -79,6 +80,11 @@ public void relay(SchemaInfo toValidate, SchemaInfo toValidateAgainst) throws IO } protected BreakingChanges checkNodeType(JsonNode toCheck, JsonNode toCheckAgainst) { + if(toCheck.has("enum") || toCheckAgainst.has("enum")) { + BreakingChanges enumChanges = enumComparator.enumComparator(toCheck, toCheckAgainst); + if(enumChanges != null) + return enumChanges; + } String nodeType = jsonCompatibilityCheckerUtils.getTypeValue(toCheck).equals( jsonCompatibilityCheckerUtils.getTypeValue( toCheckAgainst)) ? jsonCompatibilityCheckerUtils.getTypeValue(toCheck) : "mismatch"; diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java index d9ce8c99c..04e75d239 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java @@ -34,4 +34,22 @@ public boolean isInRequired(String toSearch, JsonNode toSearchIn) { } return false; } + + public boolean arrayComparisionOnlyRemoval(ArrayNode toCheck, ArrayNode toCheckAgainst) { + // every element in toCheck array must be in toCheckAgainst + for(int i=0;i Date: Sat, 24 Oct 2020 02:11:11 +0530 Subject: [PATCH 10/30] main method Signed-off-by: Atharva Joshi --- .../JsonCompatibilityChecker.java | 274 ++---------------- .../ObjectTypeComparator.java | 4 + 2 files changed, 32 insertions(+), 246 deletions(-) diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java index f85178492..e617b6d9f 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java @@ -2,26 +2,30 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ArrayNode; -import com.google.common.collect.Iterators; -import com.google.common.collect.Sets; import io.pravega.schemaregistry.contract.data.SchemaInfo; import io.pravega.schemaregistry.rules.CompatibilityChecker; import java.io.IOException; -import java.util.*; +import java.util.List; import java.util.stream.Collectors; -import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.*; +import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.BreakingChanges; public class JsonCompatibilityChecker implements CompatibilityChecker { EnumComparator enumComparator = new EnumComparator(); - private static List characteristics = Arrays.asList("properties", "dependencies", "required", - "additionalProperties"); JsonCompatibilityCheckerUtils jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); + ObjectTypeComparator objectTypeComparator = new ObjectTypeComparator(); + NumberComparator numberComparator = new NumberComparator(); + StringComparator stringComparator = new StringComparator(); + ArrayTypeComparator arrayTypeComparator = new ArrayTypeComparator(); @Override public boolean canRead(SchemaInfo readUsing, List writtenUsing) { + try { + canReadChecker(readUsing, writtenUsing); + } catch (IOException e) { + e.printStackTrace(); + } return false; } @@ -35,48 +39,21 @@ public boolean canMutuallyRead(SchemaInfo schema, List schemaList) { return false; } - private enum PossibleDifferences { - VARIABLE_TYPE_CHANGED, - VARIABLE_ADDED, - VARIABLE_DELETED, - DEPENDENCIES_ADDED, - DEPENDENCIES_REMOVED, - DEPENDENCY_VALUE_ADDED, - DEPENDENCY_VALUE_REMOVED, - DEPENDENCY_KEY_ADDED, - DEPENDENCY_KEY_REMOVED, - ARRAY_ELEMENT_ADDED, - ARRAY_ELEMENT_REMOVED, - ARRAY_ELEMENT_CHANGED, - PROPERTIES_ADDED, - PROPERTIES_REMOVED, - PROPERTY_ADDED, - PROPERTY_REMOVED, - PROPERTY_CHANGED, - REQUIREMENTS_ADDED, - REQUIREMENTS_REMOVED, - REQUIRED_VALUE_ADDED, - REQUIRED_VALUE_REMOVED - } - - Queue path = new ArrayDeque<>(); + - public void relay(SchemaInfo toValidate, SchemaInfo toValidateAgainst) throws IOException { + public boolean canReadChecker(SchemaInfo toValidate, List toValidateAgainst) throws IOException { ObjectMapper objectMapper = new ObjectMapper(); JsonNode toCheck = objectMapper.readTree(toValidate.getSchemaData().array()); - JsonNode toCheckAgainst = objectMapper.readTree(toValidateAgainst.getSchemaData().array()); - ArrayList ans = getDifferences(toCheck, toCheckAgainst); - if (!ans.isEmpty()) { - System.out.println("Invalid"); - //temp print statement - System.out.println(ans); - } else { - System.out.println("Valid"); - } - // toCheckAgainst - - // compute compatibility - + List toCheckAgainst = toValidateAgainst.stream().map(x -> { + try { + return objectMapper.readTree(x.getSchemaData().array()); + } catch (IOException e) { + e.printStackTrace(); + } + return null; + }).collect( + Collectors.toList()); + return toCheckAgainst.stream().map(x -> checkNodeType(toCheck, x)).anyMatch(x -> x!= null); } protected BreakingChanges checkNodeType(JsonNode toCheck, JsonNode toCheckAgainst) { @@ -90,14 +67,14 @@ protected BreakingChanges checkNodeType(JsonNode toCheck, JsonNode toCheckAgains toCheckAgainst)) ? jsonCompatibilityCheckerUtils.getTypeValue(toCheck) : "mismatch"; switch (nodeType) { case "object": - break; + return objectTypeComparator.checkAspects(toCheck, toCheckAgainst); case "number": case "integer": - break; + return numberComparator.compareNumbers(toCheck, toCheckAgainst); case "string": - break; + return stringComparator.stringComparator(toCheck, toCheckAgainst); case "array": - break; + return arrayTypeComparator.compareArrays(toCheck, toCheckAgainst); case "boolean": break; case "null": @@ -107,200 +84,5 @@ protected BreakingChanges checkNodeType(JsonNode toCheck, JsonNode toCheckAgains } return null; } - - private ArrayList getDifferences(JsonNode toCheck, JsonNode toCheckAgainst) { - ArrayList differences = new ArrayList<>(); - //cover cases when either are null - if (checkOrdinaryObject(toCheck, toCheckAgainst)) { - differences.addAll(getOrdinaryObjectProperties(toCheck, toCheckAgainst)); - return differences; - } - //check for properties - ArrayList propDifferences = compareProperties(toCheck, toCheckAgainst); - if (!propDifferences.isEmpty()) - differences.addAll(propDifferences); - //check for required - ArrayList reqDifferences = compareRequired(toCheck, toCheckAgainst); - if (!reqDifferences.isEmpty()) - differences.addAll(reqDifferences); - //check for dependencies - ArrayList dependencyDifferences = compareDependencies(toCheck, toCheckAgainst); - if (!dependencyDifferences.isEmpty()) - differences.addAll(dependencyDifferences); - //when both are values - if (toCheck.isValueNode() && toCheckAgainst.isValueNode()) { - if (!toCheck.asText().equals(toCheckAgainst.asText())) { - differences.add(PossibleDifferences.VARIABLE_TYPE_CHANGED); - } - } - //when both are arrays - if (toCheck.isArray() && toCheckAgainst.isArray()) - differences.addAll(getArrayDifferences(toCheck, toCheckAgainst)); - return differences; - } - - private ArrayList getArrayDifferences(JsonNode toCheck, JsonNode toCheckAgainst) { - HashSet toCheckCollection = new HashSet<>(); - HashSet toCheckAgainstCollection = new HashSet<>(); - ArrayNode arrayNodeToCheck = (ArrayNode) toCheck; - ArrayNode arrayNodeToCheckAgainst = (ArrayNode) toCheckAgainst; - for (int i = 0; i < arrayNodeToCheck.size(); i++) { - toCheckCollection.add(arrayNodeToCheck.get(i)); - } - for (int i = 0; i < arrayNodeToCheckAgainst.size(); i++) { - toCheckAgainstCollection.add(arrayNodeToCheckAgainst.get(i)); - } - ArrayList differences = new ArrayList<>( - Sets.difference(toCheckCollection, toCheckAgainstCollection).stream().map( - node -> PossibleDifferences.ARRAY_ELEMENT_ADDED).collect( - Collectors.toCollection(ArrayList::new))); - Sets.difference(toCheckAgainstCollection, toCheckCollection).stream().map( - node -> PossibleDifferences.ARRAY_ELEMENT_REMOVED).forEach(differences::add); - return differences; - } - - private ArrayList compareProperties(JsonNode toCheck, JsonNode toCheckAgainst) { - ArrayList differences = new ArrayList<>(); - toCheck = toCheck.get("properties"); - toCheckAgainst = toCheckAgainst.get("properties"); - if (toCheck == null && toCheckAgainst != null) { - differences.add(PossibleDifferences.PROPERTIES_REMOVED); - return differences; - } else if (toCheck != null && toCheckAgainst == null) { - differences.add(PossibleDifferences.PROPERTIES_ADDED); - return differences; - } else if (toCheck == null && toCheckAgainst == null) return differences; - Iterator keys = Iterators.concat(toCheck.fieldNames(), toCheckAgainst.fieldNames()); - while (keys.hasNext()) { - String key = keys.next(); - //System.out.println(key); - if (toCheck.get(key) == null && toCheckAgainst.get(key) != null) - differences.add(PossibleDifferences.PROPERTY_REMOVED); - else if (toCheck.get(key) != null && toCheckAgainst.get(key) == null) - differences.add(PossibleDifferences.PROPERTY_ADDED); - else { - if (!toCheck.get(key).equals(toCheckAgainst.get(key))) { - differences.addAll(getDifferences(toCheck.get(key), toCheckAgainst.get(key))); - differences.add(PossibleDifferences.PROPERTY_CHANGED); - } - } - } - return differences; - } - - private ArrayList compareRequired(JsonNode toCheck, JsonNode toCheckAgainst) { - ArrayList differences = new ArrayList<>(); - toCheck = toCheck.get("required"); - toCheckAgainst = toCheckAgainst.get("required"); - if (toCheck == null && toCheckAgainst == null) return differences; - else if (toCheck != null && toCheckAgainst == null) { - differences.add(PossibleDifferences.REQUIREMENTS_ADDED); - return differences; - } else if (toCheck == null && toCheckAgainst != null) { - differences.add(PossibleDifferences.REQUIREMENTS_REMOVED); - return differences; - } - ArrayList differencesReq = getArrayDifferences(toCheck, toCheckAgainst); - for (PossibleDifferences possibleDifference : differencesReq) { - switch (possibleDifference) { - case ARRAY_ELEMENT_ADDED: - differences.add(PossibleDifferences.REQUIRED_VALUE_ADDED); - break; - case ARRAY_ELEMENT_REMOVED: - differences.add(PossibleDifferences.REQUIRED_VALUE_REMOVED); - break; - } - } - return differences; - } - - private ArrayList compareDependencies(JsonNode toCheck, JsonNode toCheckAgainst) { - ArrayList differences = new ArrayList<>(); - toCheck = toCheck.get("dependencies"); - toCheckAgainst = toCheckAgainst.get("dependencies"); - if (toCheck == null && toCheckAgainst == null) - return differences; - else if (toCheck == null && toCheckAgainst != null) { - differences.add(PossibleDifferences.DEPENDENCIES_REMOVED); - return differences; - } else if (toCheck != null && toCheck == null) { - differences.add(PossibleDifferences.DEPENDENCIES_ADDED); - return differences; - } - String testKeyToCheck = toCheck.fieldNames().next(); - String testKeyToCheckAgainst = toCheckAgainst.fieldNames().next(); - if (toCheck.get(testKeyToCheck).isArray() && toCheckAgainst.get(testKeyToCheckAgainst).isArray()) - differences.addAll(propertyDependencies(toCheck, toCheckAgainst)); - else if (toCheck.get(testKeyToCheck).isObject() && toCheckAgainst.get(testKeyToCheckAgainst).isObject()) - differences.addAll(schemaDependencies(toCheck, toCheckAgainst)); - else - System.err.println("dependency format mismatch"); - return differences; - } - - private ArrayList propertyDependencies(JsonNode toCheck, JsonNode toCheckAgainst) { - ArrayList differences = new ArrayList<>(); - Iterator keys = Iterators.concat(toCheck.fieldNames(), toCheckAgainst.fieldNames()); - while (keys.hasNext()) { - String key = keys.next(); - System.out.println(key); - if (toCheck.get(key) == null && toCheckAgainst.get(key) != null) - differences.add(PossibleDifferences.DEPENDENCY_KEY_REMOVED); - else if (toCheck.get(key) != null && toCheckAgainst.get(key) == null) - differences.add(PossibleDifferences.DEPENDENCY_KEY_ADDED); - else { - if (!toCheck.get(key).equals(toCheckAgainst.get(key))) { - ArrayList elementDifferences = getArrayDifferences(toCheck.get(key), - toCheckAgainst.get(key)); - for (PossibleDifferences possibleDifference : elementDifferences) { - switch (possibleDifference) { - case ARRAY_ELEMENT_ADDED: - differences.add(PossibleDifferences.DEPENDENCY_VALUE_ADDED); - break; - case ARRAY_ELEMENT_REMOVED: - differences.add(PossibleDifferences.DEPENDENCY_VALUE_REMOVED); - break; - } - } - } - } - } - return differences; - } - - private ArrayList schemaDependencies(JsonNode toCheck, JsonNode toCheckAgainst) { - //treating schema dependencies and differences between them as schemas and differences between schemas - // respectively - ArrayList differences = new ArrayList<>(); - differences.addAll(getDifferences(toCheck, toCheckAgainst)); - return differences; - } - - private ArrayList getOrdinaryObjectProperties(JsonNode toCheck, JsonNode toCheckAgainst) { - ArrayList differences = new ArrayList<>(); - Iterator keys = Iterators.concat(toCheck.fieldNames(), toCheckAgainst.fieldNames()); - while (keys.hasNext()) { - String key = keys.next(); - if (toCheck.get(key) == null && toCheckAgainst.get(key) != null) - differences.add(PossibleDifferences.VARIABLE_ADDED); - else if (toCheck.get(key) != null && toCheckAgainst.get(key) == null) - differences.add(PossibleDifferences.VARIABLE_DELETED); - else { - if (!toCheck.get(key).equals(toCheckAgainst.get(key))) { - differences.addAll(getDifferences(toCheck.get(key), toCheckAgainst.get(key))); - } - } - } - return differences; - } - - private boolean checkOrdinaryObject(JsonNode toCheck, JsonNode toCheckAgainst) { - for (String field : characteristics) { - if (toCheck.has(field) || toCheckAgainst.has(field)) - return false; - } - if (toCheck.isValueNode() && toCheckAgainst.isValueNode()) - return false; - return true; - } + } diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ObjectTypeComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ObjectTypeComparator.java index e5e245f9d..b902a7180 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ObjectTypeComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ObjectTypeComparator.java @@ -9,9 +9,13 @@ public class ObjectTypeComparator { public BreakingChanges checkAspects(JsonNode toCheck, JsonNode toCheckAgainst) { // will check for properties,dependencies, required, additional properties by calling required classes. PropertiesComparator propertiesComparator = new PropertiesComparator(); + DependenciesComparator dependenciesComparator = new DependenciesComparator(); BreakingChanges propertiesDifference = propertiesComparator.checkProperties(toCheck, toCheckAgainst); if (propertiesDifference != null) return propertiesDifference; + BreakingChanges dependenciesDifference = dependenciesComparator.checkDependencies(toCheck, toCheckAgainst); + if(dependenciesDifference != null) + return dependenciesDifference; return null; } From 85d33f8a6ae45753bd8d9a271fb10e735fae6fca Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Sat, 24 Oct 2020 02:24:32 +0530 Subject: [PATCH 11/30] analyze mismatch Signed-off-by: Atharva Joshi --- .../BreakingChangesStore.java | 2 ++ .../JsonCompatibilityChecker.java | 21 +++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java index 708d5fe0e..28a4d8487 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java @@ -7,6 +7,8 @@ public class BreakingChangesStore { //includes only those changes that lead to incompatibility protected enum BreakingChanges { + // data type mismatch + DATA_TYPE_MISMATCH, // strings STRING_TYPE_MAX_LENGTH_ADDED, STRING_TYPE_MAX_LENGTH_VALUE_DECREASED, diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java index e617b6d9f..6d5def26b 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java @@ -39,7 +39,6 @@ public boolean canMutuallyRead(SchemaInfo schema, List schemaList) { return false; } - public boolean canReadChecker(SchemaInfo toValidate, List toValidateAgainst) throws IOException { ObjectMapper objectMapper = new ObjectMapper(); @@ -53,13 +52,13 @@ public boolean canReadChecker(SchemaInfo toValidate, List toValidate return null; }).collect( Collectors.toList()); - return toCheckAgainst.stream().map(x -> checkNodeType(toCheck, x)).anyMatch(x -> x!= null); + return toCheckAgainst.stream().map(x -> checkNodeType(toCheck, x)).anyMatch(x -> x != null); } protected BreakingChanges checkNodeType(JsonNode toCheck, JsonNode toCheckAgainst) { - if(toCheck.has("enum") || toCheckAgainst.has("enum")) { + if (toCheck.has("enum") || toCheckAgainst.has("enum")) { BreakingChanges enumChanges = enumComparator.enumComparator(toCheck, toCheckAgainst); - if(enumChanges != null) + if (enumChanges != null) return enumChanges; } String nodeType = jsonCompatibilityCheckerUtils.getTypeValue(toCheck).equals( @@ -80,9 +79,19 @@ protected BreakingChanges checkNodeType(JsonNode toCheck, JsonNode toCheckAgains case "null": break; case "mismatch": - break; + return analyzeMismatch(toCheck, toCheckAgainst); } return null; } - + + private BreakingChanges analyzeMismatch(JsonNode toCheck, JsonNode toCheckAgainst) { + if ((jsonCompatibilityCheckerUtils.getTypeValue(toCheck).equals( + "number") || jsonCompatibilityCheckerUtils.getTypeValue(toCheck).equals( + "integer")) && jsonCompatibilityCheckerUtils.getTypeValue(toCheckAgainst).equals( + "number") || jsonCompatibilityCheckerUtils.getTypeValue(toCheckAgainst).equals("integer")) + return numberComparator.compareNumbers(toCheck, toCheckAgainst); + else + return BreakingChanges.DATA_TYPE_MISMATCH; + } + } From 59cc890eb1ed714f6f0c21a63344b504148f4913 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Tue, 27 Oct 2020 12:07:09 +0530 Subject: [PATCH 12/30] unit test first commit Signed-off-by: Atharva Joshi --- .../JsonCompatibilityChecker.java | 2 +- .../JsonCompatibilityCheckerUtils.java | 6 +- .../JsonCompatibilityCheckerTest.java | 243 ++++++++++-------- .../JsonCompatibilityCheckerUtilsTest.java | 31 +++ 4 files changed, 167 insertions(+), 115 deletions(-) create mode 100644 server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtilsTest.java diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java index 6d5def26b..dd8c04c0d 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java @@ -40,7 +40,7 @@ public boolean canMutuallyRead(SchemaInfo schema, List schemaList) { } - public boolean canReadChecker(SchemaInfo toValidate, List toValidateAgainst) throws IOException { + private boolean canReadChecker(SchemaInfo toValidate, List toValidateAgainst) throws IOException { ObjectMapper objectMapper = new ObjectMapper(); JsonNode toCheck = objectMapper.readTree(toValidate.getSchemaData().array()); List toCheckAgainst = toValidateAgainst.stream().map(x -> { diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java index 04e75d239..fecbb60e8 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java @@ -3,13 +3,17 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; +import java.util.Iterator; + public class JsonCompatibilityCheckerUtils { public String getTypeValue(JsonNode node) { String value = null; - while(node.fieldNames().hasNext()) { + Iterator fieldNames = node.fieldNames(); + while(fieldNames.hasNext()) { if (node.fieldNames().next().equals("type")) value = node.get("type").textValue(); + break; } return value; } diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java index ebd2d2ef7..46aad4ca3 100644 --- a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java @@ -3,7 +3,6 @@ import com.google.common.collect.ImmutableMap; import io.pravega.schemaregistry.contract.data.SchemaInfo; import io.pravega.schemaregistry.contract.data.SerializationFormat; -import io.pravega.schemaregistry.rules.jsoncompatibility.JsonCompatibilityChecker; import org.junit.Assert; import org.junit.Test; @@ -15,7 +14,7 @@ public class JsonCompatibilityCheckerTest { @Test - public void testCanRead(){ + public void testCanRead() { JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); String x = "{\n" + "\"type\": \"object\",\n" + @@ -49,9 +48,11 @@ public void testCanRead(){ "}\n" + "}\n" + "}\n"; - SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x.getBytes()), ImmutableMap.of()); + SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x.getBytes()), + ImmutableMap.of()); List toValidateAgainst = new ArrayList<>(); - SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(y.getBytes()), ImmutableMap.of()); + SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(y.getBytes()), + ImmutableMap.of()); toValidateAgainst.add(schemaInfo1); Assert.assertTrue(jsonCompatibilityChecker.canMutuallyRead(toValidate, toValidateAgainst)); } @@ -91,146 +92,162 @@ public void testPrintNodes() throws IOException { "}\n" + "}\n" + "}\n"; - String z = "{\n"+ - "\"type\": \"object\",\n"+ - "\"properties\": {\n"+ - "\"first_name\": { \"type\": \"string\" },\n"+ - "\"last_name\": { \"type\": \"string\" },\n"+ - "\"birthday\": { \"type\": \"string\", \"format\": \"date\" },\n"+ - "\"address\": {\n"+ - "\"type\": \"object\",\n"+ - "\"properties\": {\n"+ - "\"street_address\": { \"type\": \"string\" },\n"+ - "\"city\": { \"type\": \"string\" },\n"+ - "\"state\": { \"type\": \"string\" },\n"+ - "\"country\": { \"type\" : \"string\" }\n"+ - "},\n"+ - "\"required\": [\"city\"]\n"+ - "}\n"+ - "},\n"+ - "\"required\": [\"first_name\"]\n"+ + String z = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"first_name\": { \"type\": \"string\" },\n" + + "\"last_name\": { \"type\": \"string\" },\n" + + "\"birthday\": { \"type\": \"string\", \"format\": \"date\" },\n" + + "\"address\": {\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"street_address\": { \"type\": \"string\" },\n" + + "\"city\": { \"type\": \"string\" },\n" + + "\"state\": { \"type\": \"string\" },\n" + + "\"country\": { \"type\" : \"string\" }\n" + + "},\n" + + "\"required\": [\"city\"]\n" + + "}\n" + + "},\n" + + "\"required\": [\"first_name\"]\n" + "}\n"; - SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x.getBytes()), ImmutableMap.of()); - SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(y.getBytes()), ImmutableMap.of()); - SchemaInfo schemaInfo11 = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(z.getBytes()), ImmutableMap.of()); - jsonCompatibilityChecker.relay(toValidate, schemaInfo11); + SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x.getBytes()), + ImmutableMap.of()); + SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(y.getBytes()), + ImmutableMap.of()); + SchemaInfo schemaInfo11 = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, + ByteBuffer.wrap(z.getBytes()), ImmutableMap.of()); + List toValidateAgainst = new ArrayList<>(); + toValidateAgainst.add(schemaInfo1); + toValidateAgainst.add(schemaInfo11); + jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainst); } @Test public void testDependencies() throws IOException { JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); - String x1 = "{\n"+ - "\"type\": \"object\",\n"+ - "\"properties\": {\n"+ - "\"name\": { \"type\": \"string\" },\n"+ - "\"credit_card\": { \"type\": \"number\" },\n"+ - "\"billing_address\": { \"type\": \"string\" }\n"+ - "},\n"+ - "\"required\": [\"name\"],\n"+ - "\"dependencies\": {\n"+ - "\"credit_card\": [\"billing_address\"],\n"+ - "\"billing_address\": [\"credit_card\"]\n"+ - "}\n"+ + String x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" },\n" + + "\"billing_address\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": [\"billing_address\"],\n" + + "\"billing_address\": [\"credit_card\"]\n" + + "}\n" + "}\n"; - String x2 = "{\n"+ - "\"type\": \"object\",\n"+ - "\"properties\": {\n"+ - "\"name\": { \"type\": \"string\" },\n"+ - "\"credit_card\": { \"type\": \"number\" },\n"+ - "\"billing_address\": { \"type\": \"string\" }\n"+ - "},\n"+ - "\"required\": [\"name\"],\n"+ - "\"dependencies\": {\n"+ - "\"credit_card\": [\"billing_address\"]\n"+ - "}\n"+ + String x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" },\n" + + "\"billing_address\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": [\"billing_address\"]\n" + + "}\n" + "}\n"; - SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); - SchemaInfo toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); - jsonCompatibilityChecker.relay(toValidate, toValidateAgainst); + SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), + ImmutableMap.of()); + SchemaInfo toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, + ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); + List toValidateAgainstList = new ArrayList<>(); + jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList); } @Test public void testBasicProperties() throws IOException { JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); - String x1 = "{\n"+ - "\"type\": \"object\",\n"+ - "\"properties\": {\n"+ - "\"number\": { \"type\": \"number\" },\n"+ - "\"street_name\": { \"type\": \"string\" },\n"+ - "\"street_type\": { \"type\": \"string\"}\n"+ - "}\n"+ + String x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "}\n" + "}\n"; - String x2 = "{\n"+ - "\"type\": \"object\",\n"+ - "\"properties\": {\n"+ - "\"number\": { \"type\": \"number\" },\n"+ - "\"street_name\": { \"type\": \"string\" },\n"+ - "\"street_type\": { \"type\": \"string\"}\n"+ - "}\n"+ + String x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "}\n" + "}\n"; - SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x1.getBytes()), ImmutableMap.of()); - SchemaInfo toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); - jsonCompatibilityChecker.relay(toValidate, toValidateAgainst); + SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x1.getBytes()), + ImmutableMap.of()); + SchemaInfo toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, + ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); + List toValidateAgainstList = new ArrayList<>(); //different properties - x2 = "{\n"+ - "\"type\": \"object\",\n"+ - "\"properties\": {\n"+ - "\"number\": { \"type\": \"number\" },\n"+ - "\"street_name1\": { \"type\": \"string\" },\n"+ - "\"street_type\": { \"type\": \"string\"}\n"+ - "}\n"+ + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name1\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "}\n" + "}\n"; - toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x1.getBytes()), ImmutableMap.of()); - toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); - jsonCompatibilityChecker.relay(toValidate, toValidateAgainst); + SchemaInfo toValidateAgainst1 = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, + ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); //different property values - x2 = "{\n"+ - "\"type\": \"object\",\n"+ - "\"properties\": {\n"+ - "\"number\": { \"type\": \"number\" },\n"+ - "\"street_name\": { \"type\": \"number\" },\n"+ - "\"street_type\": { \"type\": \"string\"}\n"+ - "}\n"+ + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"number\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "}\n" + "}\n"; - toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x1.getBytes()), ImmutableMap.of()); - toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); - jsonCompatibilityChecker.relay(toValidate, toValidateAgainst); + SchemaInfo toValidateAgainst2 = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, + ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); + toValidateAgainstList.add(toValidateAgainst); + toValidateAgainstList.add(toValidateAgainst1); + toValidateAgainstList.add(toValidateAgainst2); + jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList); } @Test public void testRequired() throws IOException { JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); // equal test - String x1 = "{\n"+ - "\"type\": \"object\",\n"+ - "\"properties\": {\n"+ - "\"name\": { \"type\": \"string\" },\n"+ - "\"email\": { \"type\": \"string\" },\n"+ - "\"address\": { \"type\": \"string\" },\n"+ - "\"telephone\": { \"type\": \"string\" }\n"+ - "},\n"+ - "\"required\": [\"name\", \"email\"]\n"+ + String x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"email\": { \"type\": \"string\" },\n" + + "\"address\": { \"type\": \"string\" },\n" + + "\"telephone\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"name\", \"email\"]\n" + "}\n"; - SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x1.getBytes()), ImmutableMap.of()); - SchemaInfo toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x1.getBytes()), ImmutableMap.of()); - jsonCompatibilityChecker.relay(toValidate, toValidateAgainst); - + SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x1.getBytes()), + ImmutableMap.of()); + SchemaInfo toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, + ByteBuffer.wrap(x1.getBytes()), ImmutableMap.of()); + List toValidateAgainstList = new ArrayList<>(); + toValidateAgainstList.add(toValidateAgainst); //remove required array element - String x2 = "{\n"+ - "\"type\": \"object\",\n"+ - "\"properties\": {\n"+ - "\"name\": { \"type\": \"string\" },\n"+ - "\"email\": { \"type\": \"string\" },\n"+ - "\"address\": { \"type\": \"string\" },\n"+ - "\"telephone\": { \"type\": \"string\" }\n"+ - "},\n"+ - "\"required\": [\"email\"]\n"+ + String x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"email\": { \"type\": \"string\" },\n" + + "\"address\": { \"type\": \"string\" },\n" + + "\"telephone\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"email\"]\n" + "}\n"; - toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); - jsonCompatibilityChecker.relay(toValidate, toValidateAgainst); + SchemaInfo toValidateAgainst1 = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, + ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); + toValidateAgainstList.add(toValidateAgainst1); } } \ No newline at end of file diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtilsTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtilsTest.java new file mode 100644 index 000000000..397d04a5a --- /dev/null +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtilsTest.java @@ -0,0 +1,31 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; +import java.nio.ByteBuffer; + +public class JsonCompatibilityCheckerUtilsTest { + ObjectMapper objectMapper = new ObjectMapper(); + JsonCompatibilityCheckerUtils jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); + @Test + public void testGetTypeValue() throws IOException { + String x = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" },\n" + + "\"billing_address\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": [\"billing_address\"]\n" + + "}\n" + + "}\n"; + JsonNode node = objectMapper.readTree(ByteBuffer.wrap(x.getBytes()).array()); + Assert.assertEquals("object", jsonCompatibilityCheckerUtils.getTypeValue(node)); + } +} From 7e346146ba847e5b383d0b503d71d33cac2385f3 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Tue, 27 Oct 2020 14:00:34 +0530 Subject: [PATCH 13/30] jsonCompatibilityCheckerUtilsTest Signed-off-by: Atharva Joshi --- .../JsonCompatibilityCheckerUtils.java | 31 +++++- .../JsonCompatibilityCheckerUtilsTest.java | 94 +++++++++++++++++++ 2 files changed, 121 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java index fecbb60e8..7eea8db3a 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java @@ -21,6 +21,11 @@ public String getTypeValue(JsonNode node) { public boolean hasDynamicPropertySet(JsonNode node) { if(node.get("additionalProperties")==null && node.get("patternProperties")==null) return true; + else if(node.get("additionalProperties").isBoolean() && node.get("patternProperties") == null) { + if(node.get("additionalProperties").asText() == "true") { + return true; + } + } return false; } @@ -33,8 +38,10 @@ public boolean hasStaticPropertySet(JsonNode node) { public boolean isInRequired(String toSearch, JsonNode toSearchIn) { if(toSearchIn.get("required") != null) { ArrayNode arrayToSearch = (ArrayNode) toSearchIn.get("required"); - if(arrayToSearch.has(toSearch)) - return true; + for(int i=0;i Date: Wed, 28 Oct 2020 22:41:57 +0530 Subject: [PATCH 14/30] testDependenciesComparator Signed-off-by: Atharva Joshi --- .../ArrayTypeComparator.java | 6 +- .../JsonCompatibilityChecker.java | 21 +++- .../JsonCompatibilityCheckerUtils.java | 2 +- .../JsonCompatibilityCheckerTest.java | 106 ++++++++++++++++-- 4 files changed, 118 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java index 1bc203ea0..3d37fe950 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java @@ -2,13 +2,17 @@ import com.fasterxml.jackson.databind.JsonNode; import com.google.common.collect.Iterators; +import io.pravega.schemaregistry.rules.CompatibilityChecker; import java.util.Iterator; import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.BreakingChanges; public class ArrayTypeComparator { - JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); + JsonCompatibilityChecker jsonCompatibilityChecker; + public void setJsonTypeComparator() { + this.jsonCompatibilityChecker = new JsonCompatibilityChecker(); + } public BreakingChanges compareArrays(JsonNode toCheck, JsonNode toCheckAgainst) { if (toCheck.isArray() && toCheckAgainst.isArray()) return arraySimpleBodyComparision(toCheck, toCheckAgainst); diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java index dd8c04c0d..f0dc28be3 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java @@ -12,12 +12,20 @@ import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.BreakingChanges; public class JsonCompatibilityChecker implements CompatibilityChecker { - EnumComparator enumComparator = new EnumComparator(); - JsonCompatibilityCheckerUtils jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); - ObjectTypeComparator objectTypeComparator = new ObjectTypeComparator(); - NumberComparator numberComparator = new NumberComparator(); - StringComparator stringComparator = new StringComparator(); - ArrayTypeComparator arrayTypeComparator = new ArrayTypeComparator(); + EnumComparator enumComparator; + JsonCompatibilityCheckerUtils jsonCompatibilityCheckerUtils; + ObjectTypeComparator objectTypeComparator; + NumberComparator numberComparator; + StringComparator stringComparator; + ArrayTypeComparator arrayTypeComparator; + public JsonCompatibilityChecker() { + this.enumComparator = new EnumComparator(); + this.jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); + this.objectTypeComparator = new ObjectTypeComparator(); + this.numberComparator = new NumberComparator(); + this.stringComparator = new StringComparator(); + this.arrayTypeComparator = new ArrayTypeComparator(); + } @Override public boolean canRead(SchemaInfo readUsing, List writtenUsing) { @@ -73,6 +81,7 @@ protected BreakingChanges checkNodeType(JsonNode toCheck, JsonNode toCheckAgains case "string": return stringComparator.stringComparator(toCheck, toCheckAgainst); case "array": + arrayTypeComparator.setJsonTypeComparator(); return arrayTypeComparator.compareArrays(toCheck, toCheckAgainst); case "boolean": break; diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java index 7eea8db3a..dc0781489 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java @@ -52,7 +52,7 @@ public boolean arrayComparisionOnlyRemoval(ArrayNode toCheck, ArrayNode toCheckA int flag =0; String toSearch = toCheck.get(i).asText(); for(int j=0;j toValidateAgainstList = new ArrayList<>(); - jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList); + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_ADDED_IN_ARRAY_FORM, + dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" },\n" + + "\"billing_address\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": [\"billing_address\", \"name\"]\n" + + "}\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_ARRAY_ELEMENTS_NON_REMOVAL, + dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "}\n" + + "}\n"; + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_SECTION_ADDED, + dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": {\n" + + "\"properties\": {\n" + + "\"billing_address\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"billing_address\"]\n" + + "}\n" + + "}\n" + + "}\n"; + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": {\n" + + "\"properties\": {\n" + + "\"card_number\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"card_number\"]\n" + + "}\n" + + "}\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_IN_SCHEMA_FORM_MODIFIED, + dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": {\n" + + "\"properties\": {\n" + + "\"card_number\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"card_number\"]\n" + + "},\n" + + "\"name\": {\n" + + "\"properties\": {\n" + + "\"salutation\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"salutation\"]\n" + + "}\n" + + "}\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_ADDED_IN_SCHEMA_FORM, + dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); } @Test From ca95106cfc6a0ff02a0daa8cbf018ff74f389cf0 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Wed, 28 Oct 2020 23:43:19 +0530 Subject: [PATCH 15/30] seperate test classes Signed-off-by: Atharva Joshi --- .../ObjectTypeComparator.java | 1 + .../PropertiesComparator.java | 5 +- .../DependenciesComparatorTest.java | 134 ++++++++++++++++++ .../JsonCompatibilityCheckerTest.java | 124 ---------------- 4 files changed, 139 insertions(+), 125 deletions(-) create mode 100644 server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparatorTest.java diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ObjectTypeComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ObjectTypeComparator.java index b902a7180..b3e4dee6e 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ObjectTypeComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ObjectTypeComparator.java @@ -9,6 +9,7 @@ public class ObjectTypeComparator { public BreakingChanges checkAspects(JsonNode toCheck, JsonNode toCheckAgainst) { // will check for properties,dependencies, required, additional properties by calling required classes. PropertiesComparator propertiesComparator = new PropertiesComparator(); + propertiesComparator.setJsonCompatibilityChecker(); DependenciesComparator dependenciesComparator = new DependenciesComparator(); BreakingChanges propertiesDifference = propertiesComparator.checkProperties(toCheck, toCheckAgainst); if (propertiesDifference != null) diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java index 6a33ec3a2..0bfccae25 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java @@ -7,8 +7,11 @@ import java.util.Iterator; public class PropertiesComparator { - JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); + JsonCompatibilityChecker jsonCompatibilityChecker; JsonCompatibilityCheckerUtils jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); + public void setJsonCompatibilityChecker() { + this.jsonCompatibilityChecker = new JsonCompatibilityChecker(); + } public BreakingChanges checkProperties(JsonNode toCheck, JsonNode toCheckAgainst) { Iterator propertyFields = Iterators.concat(toCheck.get("properties").fieldNames(), toCheckAgainst.get("properties").fieldNames()); while(propertyFields.hasNext()) { diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparatorTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparatorTest.java new file mode 100644 index 000000000..2da562a62 --- /dev/null +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparatorTest.java @@ -0,0 +1,134 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; +import java.nio.ByteBuffer; + +public class DependenciesComparatorTest { + + @Test + public void testCheckDependencies() throws IOException { + DependenciesComparator dependenciesComparator = new DependenciesComparator(); + ObjectMapper objectMapper = new ObjectMapper(); + String x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" },\n" + + "\"billing_address\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": [\"billing_address\"],\n" + + "\"billing_address\": [\"credit_card\"]\n" + + "}\n" + + "}\n"; + String x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" },\n" + + "\"billing_address\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": [\"billing_address\"]\n" + + "}\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_ADDED_IN_ARRAY_FORM, + dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" },\n" + + "\"billing_address\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": [\"billing_address\", \"name\"]\n" + + "}\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_ARRAY_ELEMENTS_NON_REMOVAL, + dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "}\n" + + "}\n"; + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_SECTION_ADDED, + dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": {\n" + + "\"properties\": {\n" + + "\"billing_address\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"billing_address\"]\n" + + "}\n" + + "}\n" + + "}\n"; + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": {\n" + + "\"properties\": {\n" + + "\"card_number\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"card_number\"]\n" + + "}\n" + + "}\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_IN_SCHEMA_FORM_MODIFIED, + dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": {\n" + + "\"properties\": {\n" + + "\"card_number\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"card_number\"]\n" + + "},\n" + + "\"name\": {\n" + + "\"properties\": {\n" + + "\"salutation\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"salutation\"]\n" + + "}\n" + + "}\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_ADDED_IN_SCHEMA_FORM, + dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); + } +} diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java index c13d6fd36..1668c9dd9 100644 --- a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java @@ -1,7 +1,5 @@ package io.pravega.schemaregistry.rules.jsoncompatibility; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; import io.pravega.schemaregistry.contract.data.SchemaInfo; import io.pravega.schemaregistry.contract.data.SerializationFormat; @@ -125,128 +123,6 @@ public void testPrintNodes() throws IOException { jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainst); } - @Test - public void testCheckDependencies() throws IOException { - DependenciesComparator dependenciesComparator = new DependenciesComparator(); - ObjectMapper objectMapper = new ObjectMapper(); - String x1 = "{\n" + - "\"type\": \"object\",\n" + - "\"properties\": {\n" + - "\"name\": { \"type\": \"string\" },\n" + - "\"credit_card\": { \"type\": \"number\" },\n" + - "\"billing_address\": { \"type\": \"string\" }\n" + - "},\n" + - "\"required\": [\"name\"],\n" + - "\"dependencies\": {\n" + - "\"credit_card\": [\"billing_address\"],\n" + - "\"billing_address\": [\"credit_card\"]\n" + - "}\n" + - "}\n"; - String x2 = "{\n" + - "\"type\": \"object\",\n" + - "\"properties\": {\n" + - "\"name\": { \"type\": \"string\" },\n" + - "\"credit_card\": { \"type\": \"number\" },\n" + - "\"billing_address\": { \"type\": \"string\" }\n" + - "},\n" + - "\"required\": [\"name\"],\n" + - "\"dependencies\": {\n" + - "\"credit_card\": [\"billing_address\"]\n" + - "}\n" + - "}\n"; - JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); - JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); - Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_ADDED_IN_ARRAY_FORM, - dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); - x1 = "{\n" + - "\"type\": \"object\",\n" + - "\"properties\": {\n" + - "\"name\": { \"type\": \"string\" },\n" + - "\"credit_card\": { \"type\": \"number\" },\n" + - "\"billing_address\": { \"type\": \"string\" }\n" + - "},\n" + - "\"required\": [\"name\"],\n" + - "\"dependencies\": {\n" + - "\"credit_card\": [\"billing_address\", \"name\"]\n" + - "}\n" + - "}\n"; - toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); - Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_ARRAY_ELEMENTS_NON_REMOVAL, - dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); - x2 = "{\n" + - "\"type\": \"object\",\n" + - "\"properties\": {\n" + - "\"number\": { \"type\": \"number\" },\n" + - "\"street_name\": { \"type\": \"string\" },\n" + - "\"street_type\": { \"type\": \"string\"}\n" + - "}\n" + - "}\n"; - toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); - Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_SECTION_ADDED, - dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); - x1 = "{\n" + - "\"type\": \"object\",\n" + - "\"properties\": {\n" + - "\"name\": { \"type\": \"string\" },\n" + - "\"credit_card\": { \"type\": \"number\" }\n" + - "},\n" + - "\"required\": [\"name\"],\n" + - "\"dependencies\": {\n" + - "\"credit_card\": {\n" + - "\"properties\": {\n" + - "\"billing_address\": { \"type\": \"string\" }\n" + - "},\n" + - "\"required\": [\"billing_address\"]\n" + - "}\n" + - "}\n" + - "}\n"; - x2 = "{\n" + - "\"type\": \"object\",\n" + - "\"properties\": {\n" + - "\"name\": { \"type\": \"string\" },\n" + - "\"credit_card\": { \"type\": \"number\" }\n" + - "},\n" + - "\"required\": [\"name\"],\n" + - "\"dependencies\": {\n" + - "\"credit_card\": {\n" + - "\"properties\": {\n" + - "\"card_number\": { \"type\": \"number\" }\n" + - "},\n" + - "\"required\": [\"card_number\"]\n" + - "}\n" + - "}\n" + - "}\n"; - toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); - toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); - Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_IN_SCHEMA_FORM_MODIFIED, - dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); - x1 = "{\n" + - "\"type\": \"object\",\n" + - "\"properties\": {\n" + - "\"name\": { \"type\": \"string\" },\n" + - "\"credit_card\": { \"type\": \"number\" }\n" + - "},\n" + - "\"required\": [\"name\"],\n" + - "\"dependencies\": {\n" + - "\"credit_card\": {\n" + - "\"properties\": {\n" + - "\"card_number\": { \"type\": \"number\" }\n" + - "},\n" + - "\"required\": [\"card_number\"]\n" + - "},\n" + - "\"name\": {\n" + - "\"properties\": {\n" + - "\"salutation\": { \"type\": \"string\" }\n" + - "},\n" + - "\"required\": [\"salutation\"]\n" + - "}\n" + - "}\n" + - "}\n"; - toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); - Assert.assertEquals(BreakingChangesStore.BreakingChanges.DEPENDENCY_ADDED_IN_SCHEMA_FORM, - dependenciesComparator.checkDependencies(toCheck, toCheckAgainst)); - } - @Test public void testBasicProperties() throws IOException { JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); From 2f50d99363e72d6c02b5aa49697a75d15a0f16d3 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Thu, 29 Oct 2020 16:02:19 +0530 Subject: [PATCH 16/30] propertiesComparatorTest - basic properties Signed-off-by: Atharva Joshi --- .../PropertiesComparator.java | 76 +++++----- .../PropertiesComparatorTest.java | 131 ++++++++++++++++++ 2 files changed, 175 insertions(+), 32 deletions(-) create mode 100644 server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparatorTest.java diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java index 0bfccae25..39d63cc23 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java @@ -4,57 +4,68 @@ import com.fasterxml.jackson.databind.node.ArrayNode; import com.google.common.collect.Iterators; import io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.BreakingChanges; + import java.util.Iterator; public class PropertiesComparator { JsonCompatibilityChecker jsonCompatibilityChecker; JsonCompatibilityCheckerUtils jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); + public void setJsonCompatibilityChecker() { this.jsonCompatibilityChecker = new JsonCompatibilityChecker(); } + public BreakingChanges checkProperties(JsonNode toCheck, JsonNode toCheckAgainst) { - Iterator propertyFields = Iterators.concat(toCheck.get("properties").fieldNames(), toCheckAgainst.get("properties").fieldNames()); - while(propertyFields.hasNext()) { + Iterator propertyFields = Iterators.concat(toCheck.get("properties").fieldNames(), + toCheckAgainst.get("properties").fieldNames()); + while (propertyFields.hasNext()) { String propertyField = propertyFields.next(); - if(toCheck.get("properties").get(propertyField) == null) { + if (toCheck.get("properties").get(propertyField) == null) { // property has been removed from toCheck - if(jsonCompatibilityCheckerUtils.hasStaticPropertySet(toCheck)) + if (jsonCompatibilityCheckerUtils.hasStaticPropertySet(toCheck)) return BreakingChanges.PROPERTY_REMOVED_FROM_STATIC_PROPERTY_SET; - if(!jsonCompatibilityCheckerUtils.hasStaticPropertySet(toCheck) && !jsonCompatibilityCheckerUtils.hasDynamicPropertySet(toCheck)) { + if (!jsonCompatibilityCheckerUtils.hasStaticPropertySet( + toCheck) && !jsonCompatibilityCheckerUtils.hasDynamicPropertySet(toCheck)) { // assume that pattern properties always matches // TO DO: add regex check for pattern properties BreakingChanges breakingChanges = jsonCompatibilityChecker.checkNodeType( - toCheck.get("properties").get("additionalProperties"), + toCheck.get("additionalProperties"), toCheckAgainst.get("properties").get(propertyField)); - if(breakingChanges!= null) + if (breakingChanges != null) return BreakingChanges.PROPERTY_REMOVED_NOT_PART_OF_DYNAMIC_PROPERTY_SET_WITH_CONDITION; } - } - else if(toCheckAgainst.get("properties").get(propertyField) == null) { + } else if (toCheckAgainst.get("properties").get(propertyField) == null) { // property has been added to toCheck - if(jsonCompatibilityCheckerUtils.hasDynamicPropertySet(toCheckAgainst)) + if (jsonCompatibilityCheckerUtils.hasDynamicPropertySet(toCheckAgainst)) return BreakingChanges.PROPERTY_ADDED_TO_DYNAMIC_PROPERTY_SET; //check if required property in toCheck - if(toCheck.get("required") != null) { + if (toCheck.get("required") != null) { if (jsonCompatibilityCheckerUtils.isInRequired(propertyField, toCheck)) { if (toCheck.get("properties").get(propertyField).get("default") == null) return BreakingChanges.REQUIRED_PROPERTY_ADDED_WITHOUT_DEFAULT; } } - if(!jsonCompatibilityCheckerUtils.hasStaticPropertySet(toCheckAgainst) && !jsonCompatibilityCheckerUtils.hasDynamicPropertySet(toCheckAgainst)) { + if (!jsonCompatibilityCheckerUtils.hasStaticPropertySet( + toCheckAgainst) && !jsonCompatibilityCheckerUtils.hasDynamicPropertySet(toCheckAgainst)) { // assume that pattern properties always matches // TO DO: add regex check for pattern properties BreakingChanges breakingChanges = jsonCompatibilityChecker.checkNodeType( toCheck.get("properties").get(propertyField), - toCheckAgainst.get("properties").get("additionalProperties")); - if(breakingChanges!= null) + toCheckAgainst.get("additionalProperties")); + if (breakingChanges != null) return BreakingChanges.PROPERTY_ADDED_NOT_PART_OF_DYNAMIC_PROPERTY_SET_WITH_CONDITION; } + } else if (toCheckAgainst.get("properties").get(propertyField) != null && toCheck.get("properties").get( + propertyField) != null) { + BreakingChanges singlePropertyChanges = jsonCompatibilityChecker.checkNodeType(toCheck.get("properties").get(propertyField), + toCheckAgainst.get("properties").get(propertyField)); + if(singlePropertyChanges != null) + return singlePropertyChanges; } } // check for min-max conditions on properties BreakingChanges minMaxBreakingChanges = minMaxProperties(toCheck, toCheckAgainst); - if(minMaxBreakingChanges!=null) + if (minMaxBreakingChanges != null) return minMaxBreakingChanges; // check for additional properties BreakingChanges additionalPropsBreakingChanges = additionalProperties(toCheck, toCheckAgainst); @@ -62,46 +73,47 @@ else if(toCheckAgainst.get("properties").get(propertyField) == null) { return additionalPropsBreakingChanges; // check for required properties BreakingChanges requiredPropsBreakingChanges = requiredProperties(toCheck, toCheckAgainst); - if(requiredPropsBreakingChanges != null) + if (requiredPropsBreakingChanges != null) return requiredPropsBreakingChanges; return null; } - + private BreakingChanges minMaxProperties(JsonNode toCheck, JsonNode toCheckAgainst) { // minProperties - if(toCheck.get("minProperties") != null && toCheckAgainst.get("minProperties") == null) + if (toCheck.get("minProperties") != null && toCheckAgainst.get("minProperties") == null) return BreakingChanges.MIN_PROPERTIES_ADDED; else if (toCheck.get("minProperties") != null && toCheckAgainst.get("minProperties") != null) { - if(toCheck.get("minProperties").intValue() > toCheckAgainst.get("minProperties").intValue()) + if (toCheck.get("minProperties").intValue() > toCheckAgainst.get("minProperties").intValue()) return BreakingChanges.MIN_PROPERTIES_LIMIT_INCREASED; } // maxProperties - if(toCheck.get("maxProperties") != null && toCheckAgainst.get("maxProperties") == null) + if (toCheck.get("maxProperties") != null && toCheckAgainst.get("maxProperties") == null) return BreakingChanges.MAX_PROPERTIES_ADDED; - else if(toCheck.get("maxProperties") != null && toCheckAgainst.get("maxProperties") != null) { - if(toCheck.get("maxProperties").intValue() < toCheckAgainst.get("maxProperties").intValue()) + else if (toCheck.get("maxProperties") != null && toCheckAgainst.get("maxProperties") != null) { + if (toCheck.get("maxProperties").intValue() < toCheckAgainst.get("maxProperties").intValue()) return BreakingChanges.MAX_PROPERTIES_LIMIT_DECREASED; } return null; } - + private BreakingChanges additionalProperties(JsonNode toCheck, JsonNode toCheckAgainst) { - if(toCheck.get("additionalProperties") == null && toCheckAgainst.get("additionalProperties") != null) + if (toCheck.get("additionalProperties") == null && toCheckAgainst.get("additionalProperties") != null) return BreakingChanges.ADDITIONAL_PROPERTIES_REMOVED; - else if(toCheck.get("aaditionalProperties") != null && toCheckAgainst.get("additinalProperties") != null){ - BreakingChanges additionalPropertiesBreakingChanges = jsonCompatibilityChecker.checkNodeType(toCheck.get("additionalProperties"), toCheckAgainst.get("additionalProperties")); - if(additionalPropertiesBreakingChanges != null) + else if (toCheck.get("additionalProperties") != null && toCheckAgainst.get("additionalProperties") != null) { + BreakingChanges additionalPropertiesBreakingChanges = jsonCompatibilityChecker.checkNodeType( + toCheck.get("additionalProperties"), toCheckAgainst.get("additionalProperties")); + if (additionalPropertiesBreakingChanges != null) return additionalPropertiesBreakingChanges; } return null; } - + private BreakingChanges requiredProperties(JsonNode toCheck, JsonNode toCheckAgainst) { ArrayNode arrayNodeToCheck = (ArrayNode) toCheck.get("required"); - if(arrayNodeToCheck != null) { - for(int i=0;i < arrayNodeToCheck.size();i++) { - if(!jsonCompatibilityCheckerUtils.isInRequired(arrayNodeToCheck.get(i).textValue(), toCheckAgainst)) { - if(toCheck.get("properties").get(arrayNodeToCheck.get(i).textValue()).get("default") == null) + if (arrayNodeToCheck != null) { + for (int i = 0; i < arrayNodeToCheck.size(); i++) { + if (!jsonCompatibilityCheckerUtils.isInRequired(arrayNodeToCheck.get(i).textValue(), toCheckAgainst)) { + if (toCheck.get("properties").get(arrayNodeToCheck.get(i).textValue()).get("default") == null) return BreakingChanges.REQUIRED_PROPERTY_ADDED_WITHOUT_DEFAULT; } } diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparatorTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparatorTest.java new file mode 100644 index 000000000..722621757 --- /dev/null +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparatorTest.java @@ -0,0 +1,131 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; +import java.nio.ByteBuffer; + +public class PropertiesComparatorTest { + @Test + public void testBasicProperties() throws IOException { + PropertiesComparator propertiesComparator = new PropertiesComparator(); + propertiesComparator.setJsonCompatibilityChecker(); + ObjectMapper objectMapper = new ObjectMapper(); + String x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "}\n" + + "}\n"; + String x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"},\n" + + "\"city\": { \"type\": \"string\"}\n" + + "}\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.PROPERTY_ADDED_TO_DYNAMIC_PROPERTY_SET, + propertiesComparator.checkProperties(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "},\n" + + "\"additionalProperties\": false\n" + + "}\n"; + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"},\n" + + "\"city\": { \"type\": \"string\"}\n" + + "},\n" + + "\"required\": [\"city\"]\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.REQUIRED_PROPERTY_ADDED_WITHOUT_DEFAULT, + propertiesComparator.checkProperties(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "},\n" + + "\"additionalProperties\": {\"type\": \"number\"}\n" + + "}\n"; + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"},\n" + + "\"city\": { \"type\": \"string\"}\n" + + "},\n" + + "\"additionalProperties\": {\"type\": \"number\"}\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.PROPERTY_ADDED_NOT_PART_OF_DYNAMIC_PROPERTY_SET_WITH_CONDITION, + propertiesComparator.checkProperties(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "},\n" + + "\"additionalProperties\": false\n" + + "}\n"; + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"},\n" + + "\"city\": { \"type\": \"string\"}\n" + + "}\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.PROPERTY_REMOVED_FROM_STATIC_PROPERTY_SET, + propertiesComparator.checkProperties(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"},\n" + + "\"city\": { \"type\": \"string\"}\n" + + "},\n" + + "\"additionalProperties\": {\"type\": \"number\"}\n" + + "}\n"; + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "},\n" + + "\"additionalProperties\": {\"type\": \"number\"}\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.PROPERTY_REMOVED_NOT_PART_OF_DYNAMIC_PROPERTY_SET_WITH_CONDITION, + propertiesComparator.checkProperties(toCheck, toCheckAgainst)); + } + +} From f0b4b93cdb15aaeccae35024f1669891f9797e90 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Thu, 29 Oct 2020 17:07:08 +0530 Subject: [PATCH 17/30] propertiesComparatorTest Signed-off-by: Atharva Joshi --- .../BreakingChangesStore.java | 2 +- .../PropertiesComparator.java | 2 +- .../PropertiesComparatorTest.java | 198 +++++++++++++++++- 3 files changed, 199 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java index 28a4d8487..298cb92fe 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java @@ -56,7 +56,7 @@ protected enum BreakingChanges { MIN_PROPERTIES_ADDED, MIN_PROPERTIES_LIMIT_INCREASED, ADDITIONAL_PROPERTIES_REMOVED, - ADDITIONAL_PROPERTIES_NARROWED, + ADDITIONAL_PROPERTIES_NON_COMPATIBLE_CHANGE, // DEPENDENCIES DEPENDENCY_SECTION_ADDED, DEPENDENCY_ADDED_IN_ARRAY_FORM, diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java index 39d63cc23..4ceec2c19 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java @@ -103,7 +103,7 @@ else if (toCheck.get("additionalProperties") != null && toCheckAgainst.get("addi BreakingChanges additionalPropertiesBreakingChanges = jsonCompatibilityChecker.checkNodeType( toCheck.get("additionalProperties"), toCheckAgainst.get("additionalProperties")); if (additionalPropertiesBreakingChanges != null) - return additionalPropertiesBreakingChanges; + return BreakingChanges.ADDITIONAL_PROPERTIES_NON_COMPATIBLE_CHANGE; } return null; } diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparatorTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparatorTest.java index 722621757..57a29ff3a 100644 --- a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparatorTest.java +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparatorTest.java @@ -127,5 +127,201 @@ public void testBasicProperties() throws IOException { Assert.assertEquals(BreakingChangesStore.BreakingChanges.PROPERTY_REMOVED_NOT_PART_OF_DYNAMIC_PROPERTY_SET_WITH_CONDITION, propertiesComparator.checkProperties(toCheck, toCheckAgainst)); } - + + @Test + public void testMinMaxProperties() throws IOException { + ObjectMapper objectMapper = new ObjectMapper(); + PropertiesComparator propertiesComparator = new PropertiesComparator(); + propertiesComparator.setJsonCompatibilityChecker(); + String x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "},\n" + + "\"minProperties\": 1\n" + + "}\n"; + String x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "}\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.MIN_PROPERTIES_ADDED, + propertiesComparator.checkProperties(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "},\n" + + "\"minProperties\": 1\n" + + "}\n"; + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "},\n" + + "\"minProperties\": 2\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.MIN_PROPERTIES_LIMIT_INCREASED, + propertiesComparator.checkProperties(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "},\n" + + "\"maxProperties\": 3\n" + + "}\n"; + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "}\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.MAX_PROPERTIES_ADDED, + propertiesComparator.checkProperties(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "},\n" + + "\"maxProperties\": 4\n" + + "}\n"; + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "},\n" + + "\"maxProperties\": 3\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.MAX_PROPERTIES_LIMIT_DECREASED, + propertiesComparator.checkProperties(toCheck, toCheckAgainst)); + } + + @Test + public void testAdditionalProperties() throws IOException { + ObjectMapper objectMapper = new ObjectMapper(); + PropertiesComparator propertiesComparator = new PropertiesComparator(); + propertiesComparator.setJsonCompatibilityChecker(); + String x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "},\n" + + "\"additionalProperties\": false\n" + + "}\n"; + String x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "}\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ADDITIONAL_PROPERTIES_REMOVED, + propertiesComparator.checkProperties(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "},\n" + + "\"additionalProperties\": {\"type\": \"string\"}\n" + + "}\n"; + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"}\n" + + "},\n" + + "\"additionalProperties\": {\"type\": \"number\"}\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ADDITIONAL_PROPERTIES_NON_COMPATIBLE_CHANGE, + propertiesComparator.checkProperties(toCheck, toCheckAgainst)); + } + + @Test + public void testRequiredProperties() throws IOException { + ObjectMapper objectMapper = new ObjectMapper(); + PropertiesComparator propertiesComparator = new PropertiesComparator(); + propertiesComparator.setJsonCompatibilityChecker(); + String x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"},\n" + + "\"city\": { \"type\": \"string\"}\n" + + "},\n" + + "\"required\": [\"city\"]\n" + + "}\n"; + String x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"},\n" + + "\"city\": { \"type\": \"string\"}\n" + + "}\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.REQUIRED_PROPERTY_ADDED_WITHOUT_DEFAULT, + propertiesComparator.checkProperties(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"},\n" + + "\"city\": { \"type\": \"string\"}\n" + + "},\n" + + "\"required\": [\"city\"]\n" + + "}\n"; + x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"number\": { \"type\": \"number\" },\n" + + "\"street_name\": { \"type\": \"string\" },\n" + + "\"street_type\": { \"type\": \"string\"},\n" + + "\"city\": { \"type\": \"string\"}\n" + + "},\n" + + "\"required\": [\"city\", \"number\"]\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.REQUIRED_PROPERTY_ADDED_WITHOUT_DEFAULT, + propertiesComparator.checkProperties(toCheck, toCheckAgainst)); + } } From 4aa47e339a9f81ece20131aa8cfc787cb0997545 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Thu, 29 Oct 2020 19:09:21 +0530 Subject: [PATCH 18/30] ArrayTypeComparator - first half Signed-off-by: Atharva Joshi --- .../ArrayTypeComparator.java | 11 +++- .../ArrayTypeComparatorTest.java | 56 +++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparatorTest.java diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java index 3d37fe950..63313e6ae 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java @@ -46,9 +46,14 @@ else if (!toCheckAgainst.has(item)) } private BreakingChanges checkUniqueItems(JsonNode toCheck, JsonNode toCheckAgainst) { - if (toCheck.get("uniqueItems").isBoolean() && toCheck.get("uniqueItems").asText() == "true") { - if (toCheckAgainst.get("uniqueItems") == null) - return BreakingChanges.ARRAY_UNIQUE_ITEMS_CONDITION_ENABLED; + if(toCheck.has("uniqueItems")) { + if (toCheck.get("uniqueItems").isBoolean() && toCheck.get("uniqueItems").asText() == "true") { + if (toCheckAgainst.get("uniqueItems") == null) + return BreakingChanges.ARRAY_UNIQUE_ITEMS_CONDITION_ENABLED; + else if (toCheckAgainst.get("uniqueItems").isBoolean() && toCheckAgainst.get( + "uniqueItems").asText().equals("false")) + return BreakingChanges.ARRAY_UNIQUE_ITEMS_CONDITION_ENABLED; + } } return null; } diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparatorTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparatorTest.java new file mode 100644 index 000000000..955f161ec --- /dev/null +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparatorTest.java @@ -0,0 +1,56 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; +import java.nio.ByteBuffer; + +public class ArrayTypeComparatorTest { + + @Test + public void testCheckUniqueItems() throws IOException { + ObjectMapper objectMapper = new ObjectMapper(); + ArrayTypeComparator arrayTypeComparator = new ArrayTypeComparator(); + arrayTypeComparator.setJsonTypeComparator(); + String x1 = "{\n" + + "\"type\": \"array\" ,\n" + + "\"uniqueItems\": true\n" + + "}\n"; + String x2 = "{\n" + + "\"type\": \"array\" ,\n" + + "\"uniqueItems\": false\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ARRAY_UNIQUE_ITEMS_CONDITION_ENABLED, + arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"array\"\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ARRAY_UNIQUE_ITEMS_CONDITION_ENABLED, + arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); + } + + @Test + public void testMinMaxItems() throws IOException { + ObjectMapper objectMapper = new ObjectMapper(); + ArrayTypeComparator arrayTypeComparator = new ArrayTypeComparator(); + arrayTypeComparator.setJsonTypeComparator(); + String x1 = "{\n" + + "\"type\": \"array\" ,\n" + + "\"maxItems\": 3\n" + + "}\n"; + String x2 = "{\n" + + "\"type\": \"array\"\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ARRAY_MAX_ITEMS_CONDITION_ADDED, + arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); + } +} From 8a17d38473c9c867ac13af10b5eafca19affe4ef Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Fri, 30 Oct 2020 00:27:53 +0530 Subject: [PATCH 19/30] arrayTypeComparatorTest Signed-off-by: Atharva Joshi --- .../ArrayTypeComparator.java | 8 +- .../BreakingChangesStore.java | 1 + .../ArrayTypeComparatorTest.java | 105 +++++++++++++++++- 3 files changed, 110 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java index 63313e6ae..8ba089901 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java @@ -92,16 +92,18 @@ else if (toCheck.get("additionalItems").isObject()) else if (toCheck.get("additionalItems").isObject()) { if(toCheckAgainst.get("additionalItems").isObject()) { if(jsonCompatibilityChecker.checkNodeType(toCheck.get("additionalItems"), toCheckAgainst.get("additionalItems")) != null) - return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_DECREASED; - else if (toCheckAgainst.get("additionalItems").isBoolean() && toCheckAgainst.get("additionalItems").asText() == "true") - return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_DECREASED; + return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_INCOMPATIBLE_CHANGE; } + else if(toCheckAgainst.get("additionalItems").isBoolean() && toCheckAgainst.get("additionalItems").asText() == "true") + return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_DECREASED; } } return null; } private BreakingChanges itemValidation(JsonNode toCheck, JsonNode toCheckAgainst) { + if(!toCheck.has("items") && !toCheckAgainst.has("items")) + return null; return jsonCompatibilityChecker.checkNodeType(toCheck.get("items"), toCheckAgainst.get("items")); } diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java index 298cb92fe..301083827 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java @@ -37,6 +37,7 @@ protected enum BreakingChanges { ARRAY_UNIQUE_ITEMS_CONDITION_ENABLED, ARRAY_ADDITIONAL_ITEMS_DISABLED, ARRAY_ADDITIONAL_ITEMS_SCOPE_DECREASED, + ARRAY_ADDITIONAL_ITEMS_SCOPE_INCOMPATIBLE_CHANGE, ARRAY_SIMPLE_BODY_CHECK_ELEMENT_REMOVED, ARRAY_SIMPLE_BODY_CHECK_ELEMENT_ADDED, ITEM_REMOVED_NOT_COVERED_BY_PARTIALLY_OPEN_CONTENT_MODEL, diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparatorTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparatorTest.java index 955f161ec..cd3d5df00 100644 --- a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparatorTest.java +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparatorTest.java @@ -35,7 +35,7 @@ public void testCheckUniqueItems() throws IOException { Assert.assertEquals(BreakingChangesStore.BreakingChanges.ARRAY_UNIQUE_ITEMS_CONDITION_ENABLED, arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); } - + @Test public void testMinMaxItems() throws IOException { ObjectMapper objectMapper = new ObjectMapper(); @@ -52,5 +52,108 @@ public void testMinMaxItems() throws IOException { JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); Assert.assertEquals(BreakingChangesStore.BreakingChanges.ARRAY_MAX_ITEMS_CONDITION_ADDED, arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"array\" ,\n" + + "\"maxItems\": 4\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ARRAY_MAX_ITEMS_VALUE_DECREASED, + arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"array\" ,\n" + + "\"minItems\": 3\n" + + "}\n"; + x2 = "{\n" + + "\"type\": \"array\"\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ARRAY_MIN_ITEMS_CONDITION_ADDED, + arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"array\" ,\n" + + "\"minItems\": 2\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ARRAY_MIN_ITEMS_VALUE_INCREASED, + arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); + } + + @Test + public void testAdditionalItems() throws IOException { + ObjectMapper objectMapper = new ObjectMapper(); + ArrayTypeComparator arrayTypeComparator = new ArrayTypeComparator(); + arrayTypeComparator.setJsonTypeComparator(); + String x1 = "{\n" + + "\"type\": \"array\" ,\n" + + "\"additionalItems\": false\n" + + "}\n"; + String x2 = "{\n" + + "\"type\": \"array\"\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ARRAY_ADDITIONAL_ITEMS_DISABLED, + arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"array\" ,\n" + + "\"additionalItems\": { \"type\": \"string\" }\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_DECREASED, + arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"array\" ,\n" + + "\"additionalItems\": true\n" + + "}\n"; + x1 = "{\n" + + "\"type\": \"array\" ,\n" + + "\"additionalItems\": false\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ARRAY_ADDITIONAL_ITEMS_DISABLED, + arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"array\" ,\n" + + "\"additionalItems\": { \"type\": \"string\" }\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_DECREASED, + arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"array\" ,\n" + + "\"additionalItems\": { \"type\": \"number\" }\n" + + "}\n"; + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_INCOMPATIBLE_CHANGE, + arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); + } + + @Test + public void testItemValidation() throws IOException { + // node.get(items) is an object + ObjectMapper objectMapper = new ObjectMapper(); + ArrayTypeComparator arrayTypeComparator = new ArrayTypeComparator(); + arrayTypeComparator.setJsonTypeComparator(); + String x1 = "{\n" + + "\"type\": \"array\" ,\n" + + "\"items\": {\n" + + "\"type\": \"number\"\n" + + "}\n" + + "}\n"; + String x2 = "{\n" + + "\"type\": \"array\" ,\n" + + "\"items\": {\n" + + "\"type\": \"string\"\n" + + "}\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertNotNull(arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.DATA_TYPE_MISMATCH, + arrayTypeComparator.compareArrays(toCheck, toCheckAgainst)); } } From a995f40bf9df75568bb4ab4e57a55a1605f0ff34 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Fri, 30 Oct 2020 08:55:31 +0530 Subject: [PATCH 20/30] NumberTypeComparatorTest Signed-off-by: Atharva Joshi --- .../jsoncompatibility/NumberComparator.java | 2 +- .../NumberComparatorTest.java | 171 ++++++++++++++++++ 2 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/NumberComparatorTest.java diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/NumberComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/NumberComparator.java index bdb39053e..fbd6c2a2c 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/NumberComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/NumberComparator.java @@ -91,7 +91,7 @@ else if (toCheckAgainstMultipleOf%toCheckMultipleOf != 0) } private BreakingChanges typeChanged(JsonNode toCheck, JsonNode toCheckAgainst) { - if(toCheck.get("type").asText() == "integer" && toCheckAgainst.get("type").asText() == "number") + if(toCheck.get("type").asText().equals("integer") && toCheckAgainst.get("type").asText().equals("number")) return BreakingChanges.NUMBER_TYPE_CHANGED_FROM_NUMBER_TO_INTEGER; return null; } diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/NumberComparatorTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/NumberComparatorTest.java new file mode 100644 index 000000000..7259cee3f --- /dev/null +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/NumberComparatorTest.java @@ -0,0 +1,171 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; +import java.nio.ByteBuffer; + +public class NumberComparatorTest { + + @Test + public void testMaximumComparator() throws IOException { + NumberComparator numberComparator = new NumberComparator(); + ObjectMapper objectMapper = new ObjectMapper(); + String x2 = "{\n" + + "\"type\": \"number\"\n" + + "}\n"; + String x1 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"maximum\": 3\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.NUMBER_TYPE_MAXIMUM_VALUE_ADDED, + numberComparator.compareNumbers(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"exclusiveMaximum\": 3\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.NUMBER_TYPE_EXCLUSIVE_MAXIMUM_VALUE_ADDED, + numberComparator.compareNumbers(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"exclusiveMaximum\": 4\n" + + "}\n"; + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.NUMBER_TYPE_EXCLUSIVE_MAXIMUM_VALUE_DECREASED, + numberComparator.compareNumbers(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"maximum\": 3\n" + + "}\n"; + x2 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"maximum\": 4\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.NUMBER_TYPE_MAXIMUM_VALUE_DECREASED, + numberComparator.compareNumbers(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"exclusiveMaximum\": true\n" + + "}\n"; + x2 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"exclusiveMaximum\": false\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.NUMBER_TYPE_EXCLUSIVE_MAXIMUM_VALUE_ADDED, + numberComparator.compareNumbers(toCheck, toCheckAgainst)); + } + + @Test + public void testMinimumComparator() throws IOException { + NumberComparator numberComparator = new NumberComparator(); + ObjectMapper objectMapper = new ObjectMapper(); + String x2 = "{\n" + + "\"type\": \"number\"\n" + + "}\n"; + String x1 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"minimum\": 3\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.NUMBER_TYPE_MINIMUM_VALUE_ADDED, + numberComparator.compareNumbers(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"exclusiveMinimum\": 3\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.NUMBER_TYPE_EXCLUSIVE_MINIMUM_VALUE_ADDED, + numberComparator.compareNumbers(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"exclusiveMinimum\": 2\n" + + "}\n"; + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.NUMBER_TYPE_EXCLUSIVE_MINIMUM_VALUE_INCREASED, + numberComparator.compareNumbers(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"minimum\": 3\n" + + "}\n"; + x2 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"minimum\": 2\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.NUMBER_TYPE_MINIMUM_VALUE_INCREASED, + numberComparator.compareNumbers(toCheck, toCheckAgainst)); + x1 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"exclusiveMinimum\": true\n" + + "}\n"; + x2 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"exclusiveMinimum\": false\n" + + "}\n"; + toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.NUMBER_TYPE_EXCLUSIVE_MINIMUM_VALUE_ADDED, + numberComparator.compareNumbers(toCheck, toCheckAgainst)); + } + + @Test + public void testMultipleOfComparator() throws IOException { + NumberComparator numberComparator = new NumberComparator(); + ObjectMapper objectMapper = new ObjectMapper(); + String x2 = "{\n" + + "\"type\": \"number\"\n" + + "}\n"; + String x1 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"multipleOf\": 10\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.NUMBER_TYPE_MULTIPLE_OF_ADDED, + numberComparator.compareNumbers(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"multipleOf\": 5\n" + + "}\n"; + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.NUMBER_TYPE_MULTIPLE_OF_INCREASED, + numberComparator.compareNumbers(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"number\" ,\n" + + "\"multipleOf\": 3\n" + + "}\n"; + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.NUMBER_TYPE_MULTIPLE_OF_NON_DIVISIBLE_CHANGE, + numberComparator.compareNumbers(toCheck, toCheckAgainst)); + } + + @Test + public void testTypeChanged() throws IOException { + NumberComparator numberComparator = new NumberComparator(); + ObjectMapper objectMapper = new ObjectMapper(); + String x2 = "{\n" + + "\"type\": \"number\"\n" + + "}\n"; + String x1 = "{\n" + + "\"type\": \"integer\"\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.NUMBER_TYPE_CHANGED_FROM_NUMBER_TO_INTEGER, + numberComparator.compareNumbers(toCheck, toCheckAgainst)); + } + + +} From f815c3a8f346167746ae881123d801b47a00078c Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Fri, 30 Oct 2020 09:25:07 +0530 Subject: [PATCH 21/30] StringTypeComparatorTest Signed-off-by: Atharva Joshi --- .../jsoncompatibility/StringComparator.java | 4 +- .../StringComparatorTest.java | 84 +++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/StringComparatorTest.java diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/StringComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/StringComparator.java index 20fc9e03f..de514efd3 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/StringComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/StringComparator.java @@ -29,7 +29,7 @@ else if(toCheck.get("minLength") != null && toCheckAgainst.get("minLength") != n } private BreakingChanges maxLengthComparator(JsonNode toCheck, JsonNode toCheckAgainst) { - if(toCheck.get("maxLength") != null && toCheckAgainst.get("minLength") == null) + if(toCheck.get("maxLength") != null && toCheckAgainst.get("maxLength") == null) return BreakingChanges.STRING_TYPE_MAX_LENGTH_ADDED; else if(toCheck.get("maxLength") != null && toCheckAgainst.get("maxLength") != null) { int toCheckMaxLength = toCheck.get("maxLength").asInt(); @@ -41,7 +41,7 @@ else if(toCheck.get("maxLength") != null && toCheckAgainst.get("maxLength") != n } private BreakingChanges patternComparator(JsonNode toCheck, JsonNode toCheckAgainst) { - if(toCheck.get("pattern") != null && toCheckAgainst.get("patternProperties") == null) + if(toCheck.get("pattern") != null && toCheckAgainst.get("pattern") == null) return BreakingChanges.STRING_TYPE_PATTERN_ADDED; else if (toCheck.get("pattern") != null && toCheckAgainst.get("pattern") != null) { if(!toCheck.get("pattern").asText().equals(toCheckAgainst.get("pattern").asText())) diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/StringComparatorTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/StringComparatorTest.java new file mode 100644 index 000000000..4023f0d92 --- /dev/null +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/StringComparatorTest.java @@ -0,0 +1,84 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; +import java.nio.ByteBuffer; + +public class StringComparatorTest { + + @Test + public void testMinLengthComparator() throws IOException { + StringComparator stringComparator = new StringComparator(); + ObjectMapper objectMapper = new ObjectMapper(); + String x2 = "{\n" + + "\"type\": \"string\"\n" + + "}\n"; + String x1 = "{\n" + + "\"type\": \"string\" ,\n" + + "\"minLength\": 3\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.STRING_TYPE_MIN_LENGTH_ADDED, + stringComparator.stringComparator(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"string\" ,\n" + + "\"minLength\": 2\n" + + "}\n"; + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.STRING_TYPE_MIN_LENGTH_VALUE_INCREASED, + stringComparator.stringComparator(toCheck, toCheckAgainst)); + } + + @Test + public void testMaxLengthComparator() throws IOException { + StringComparator stringComparator = new StringComparator(); + ObjectMapper objectMapper = new ObjectMapper(); + String x2 = "{\n" + + "\"type\": \"string\"\n" + + "}\n"; + String x1 = "{\n" + + "\"type\": \"string\" ,\n" + + "\"maxLength\": 3\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.STRING_TYPE_MAX_LENGTH_ADDED, + stringComparator.stringComparator(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"string\" ,\n" + + "\"maxLength\": 4\n" + + "}\n"; + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.STRING_TYPE_MAX_LENGTH_VALUE_DECREASED, + stringComparator.stringComparator(toCheck, toCheckAgainst)); + } + + @Test + public void testPatternComparator() throws IOException { + StringComparator stringComparator = new StringComparator(); + ObjectMapper objectMapper = new ObjectMapper(); + String x2 = "{\n" + + "\"type\": \"string\"\n" + + "}\n"; + String x1 = "{\n" + + "\"type\": \"string\" ,\n" + + "\"pattern\": \"^(\\\\([0-9]{3}\\\\))?[0-9]{3}-[0-9]{4}$\"\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.STRING_TYPE_PATTERN_ADDED, + stringComparator.stringComparator(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"string\" ,\n" + + "\"pattern\": \"^(\\\\([0-9]{3}\\\\))?[0-9]{4}-[0-9]{5}$\"\n" + + "}\n"; + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.STRING_TYPE_PATTERN_MODIFIED, + stringComparator.stringComparator(toCheck, toCheckAgainst)); + } +} From c633470f1be382bf25eebe54aa365125ea7875ea Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Fri, 30 Oct 2020 09:53:31 +0530 Subject: [PATCH 22/30] StringTypeComparatorTest Signed-off-by: Atharva Joshi --- .../jsoncompatibility/EnumComparatorTest.java | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/EnumComparatorTest.java diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/EnumComparatorTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/EnumComparatorTest.java new file mode 100644 index 000000000..53b1bba37 --- /dev/null +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/EnumComparatorTest.java @@ -0,0 +1,36 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; +import java.nio.ByteBuffer; + +public class EnumComparatorTest { + + @Test + public void testEnumComparator() throws IOException { + EnumComparator enumComparator = new EnumComparator(); + ObjectMapper objectMapper = new ObjectMapper(); + String x1 = "{\n" + + "\"type\": \"string\" ,\n" + + "\"enum\": [\"red\", \"amber\", \"green\"]\n" + + "}\n"; + String x2 = "{\n" + + "\"type\": \"string\"\n" + + "}\n"; + JsonNode toCheck = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ENUM_TYPE_ADDED, + enumComparator.enumComparator(toCheck, toCheckAgainst)); + x2 = "{\n" + + "\"type\": \"string\" ,\n" + + "\"enum\": [\"red\", \"amber\", \"yellow\"]\n" + + "}\n"; + toCheckAgainst = objectMapper.readTree(ByteBuffer.wrap(x2.getBytes()).array()); + Assert.assertEquals(BreakingChangesStore.BreakingChanges.ENUM_TYPE_ARRAY_CONTENTS_NON_ADDITION_OF_ELEMENTS, + enumComparator.enumComparator(toCheck, toCheckAgainst)); + } +} From 489c18cb2a0e95e26e47672bcae79c76a75a253a Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Fri, 30 Oct 2020 12:25:32 +0530 Subject: [PATCH 23/30] IT part 1 Signed-off-by: Atharva Joshi --- .../DependenciesComparator.java | 1 + .../JsonCompatibilityChecker.java | 2 +- .../JsonCompatibilityCheckerTest.java | 21 ++++++++++++++++--- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java index 38711ffa3..62dbf13e4 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/DependenciesComparator.java @@ -12,6 +12,7 @@ public class DependenciesComparator { JsonCompatibilityCheckerUtils jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); PropertiesComparator propertiesComparator = new PropertiesComparator(); public BreakingChanges checkDependencies(JsonNode toCheck, JsonNode toCheckAgainst) { + propertiesComparator.setJsonCompatibilityChecker(); if(toCheck.get("dependencies") != null && toCheckAgainst.get("dependencies") == null) return BreakingChanges.DEPENDENCY_SECTION_ADDED; else if(toCheck.get("dependencies") == null && toCheckAgainst.get("dependencies") == null) diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java index f0dc28be3..1ec0517e2 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java @@ -30,7 +30,7 @@ public JsonCompatibilityChecker() { @Override public boolean canRead(SchemaInfo readUsing, List writtenUsing) { try { - canReadChecker(readUsing, writtenUsing); + return canReadChecker(readUsing, writtenUsing); } catch (IOException e) { e.printStackTrace(); } diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java index 1668c9dd9..e7839678a 100644 --- a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java @@ -54,11 +54,12 @@ public void testCanRead() { SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(y.getBytes()), ImmutableMap.of()); toValidateAgainst.add(schemaInfo1); - Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst)); + //Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst)); + System.out.println(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainst)); } @Test - public void testPrintNodes() throws IOException { + public void testPrintNodes() { JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); String x = "{\n" + "\"type\": \"object\",\n" + @@ -175,7 +176,7 @@ public void testBasicProperties() throws IOException { toValidateAgainstList.add(toValidateAgainst); toValidateAgainstList.add(toValidateAgainst1); toValidateAgainstList.add(toValidateAgainst2); - jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList); + Assert.assertFalse(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList)); } @Test @@ -212,6 +213,20 @@ public void testRequired() throws IOException { SchemaInfo toValidateAgainst1 = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); toValidateAgainstList.add(toValidateAgainst1); + Assert.assertFalse(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList)); + x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"email\": { \"type\": \"string\" },\n" + + "\"address\": { \"type\": \"string\" },\n" + + "\"telephone\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"email\", \"email\", \"address\"]\n" + + "}\n"; + SchemaInfo toValidateAgainst2 = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, + ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); + } } \ No newline at end of file From dd93162bcab03767f5799bab3e8763069a9e38c1 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Sat, 31 Oct 2020 00:58:43 +0530 Subject: [PATCH 24/30] integration test Signed-off-by: Atharva Joshi --- .../JsonCompatibilityChecker.java | 2 +- .../JsonCompatibilityCheckerTest.java | 70 +++++++++++-------- 2 files changed, 40 insertions(+), 32 deletions(-) diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java index 1ec0517e2..4b74585ac 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java @@ -60,7 +60,7 @@ private boolean canReadChecker(SchemaInfo toValidate, List toValidat return null; }).collect( Collectors.toList()); - return toCheckAgainst.stream().map(x -> checkNodeType(toCheck, x)).anyMatch(x -> x != null); + return !toCheckAgainst.stream().map(x -> checkNodeType(toCheck, x)).anyMatch(x -> x != null); } protected BreakingChanges checkNodeType(JsonNode toCheck, JsonNode toCheckAgainst) { diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java index e7839678a..16d060375 100644 --- a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java @@ -54,12 +54,11 @@ public void testCanRead() { SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(y.getBytes()), ImmutableMap.of()); toValidateAgainst.add(schemaInfo1); - //Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst)); - System.out.println(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainst)); + Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst)); } @Test - public void testPrintNodes() { + public void testDependencies() { JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); String x = "{\n" + "\"type\": \"object\",\n" + @@ -90,38 +89,44 @@ public void testPrintNodes() { "\"billing_address\": { \"type\": \"string\" }\n" + "},\n" + "\"required\": [\"billing_address\"]\n" + + "},\n" + + "\"name\": {\n" + + "\"properties\": {\n" + + "\"salutation\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"salutation\"]\n" + "}\n" + "}\n" + "}\n"; + SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x.getBytes()), + ImmutableMap.of()); + SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(y.getBytes()), + ImmutableMap.of()); + List toValidateAgainst = new ArrayList<>(); + toValidateAgainst.add(toValidate); + Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst)); + toValidateAgainst.add(schemaInfo1); + Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst)); String z = "{\n" + "\"type\": \"object\",\n" + "\"properties\": {\n" + - "\"first_name\": { \"type\": \"string\" },\n" + - "\"last_name\": { \"type\": \"string\" },\n" + - "\"birthday\": { \"type\": \"string\", \"format\": \"date\" },\n" + - "\"address\": {\n" + - "\"type\": \"object\",\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": {\n" + "\"properties\": {\n" + - "\"street_address\": { \"type\": \"string\" },\n" + - "\"city\": { \"type\": \"string\" },\n" + - "\"state\": { \"type\": \"string\" },\n" + - "\"country\": { \"type\" : \"string\" }\n" + + "\"billing_address\": { \"type\": \"number\" }\n" + "},\n" + - "\"required\": [\"city\"]\n" + + "\"required\": [\"billing_address\"]\n" + + "}\n" + "}\n" + - "},\n" + - "\"required\": [\"first_name\"]\n" + "}\n"; - SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x.getBytes()), - ImmutableMap.of()); - SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(y.getBytes()), - ImmutableMap.of()); SchemaInfo schemaInfo11 = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(z.getBytes()), ImmutableMap.of()); - List toValidateAgainst = new ArrayList<>(); - toValidateAgainst.add(schemaInfo1); toValidateAgainst.add(schemaInfo11); - jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainst); + Assert.assertFalse(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainst)); } @Test @@ -149,7 +154,8 @@ public void testBasicProperties() throws IOException { SchemaInfo toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); List toValidateAgainstList = new ArrayList<>(); - + toValidateAgainstList.add(toValidateAgainst); + Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainstList)); //different properties x2 = "{\n" + "\"type\": \"object\",\n" + @@ -161,7 +167,8 @@ public void testBasicProperties() throws IOException { "}\n"; SchemaInfo toValidateAgainst1 = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); - + toValidateAgainstList.add(toValidateAgainst1); + Assert.assertFalse(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainstList)); //different property values x2 = "{\n" + "\"type\": \"object\",\n" + @@ -173,10 +180,8 @@ public void testBasicProperties() throws IOException { "}\n"; SchemaInfo toValidateAgainst2 = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); - toValidateAgainstList.add(toValidateAgainst); - toValidateAgainstList.add(toValidateAgainst1); toValidateAgainstList.add(toValidateAgainst2); - Assert.assertFalse(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList)); + Assert.assertFalse(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainstList)); } @Test @@ -199,6 +204,7 @@ public void testRequired() throws IOException { ByteBuffer.wrap(x1.getBytes()), ImmutableMap.of()); List toValidateAgainstList = new ArrayList<>(); toValidateAgainstList.add(toValidateAgainst); + Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainstList)); //remove required array element String x2 = "{\n" + "\"type\": \"object\",\n" + @@ -208,12 +214,12 @@ public void testRequired() throws IOException { "\"address\": { \"type\": \"string\" },\n" + "\"telephone\": { \"type\": \"string\" }\n" + "},\n" + - "\"required\": [\"email\"]\n" + + "\"required\": [\"name\", \"email\", \"address\"]\n" + "}\n"; SchemaInfo toValidateAgainst1 = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); toValidateAgainstList.add(toValidateAgainst1); - Assert.assertFalse(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList)); + Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainstList)); x2 = "{\n" + "\"type\": \"object\",\n" + "\"properties\": {\n" + @@ -222,11 +228,13 @@ public void testRequired() throws IOException { "\"address\": { \"type\": \"string\" },\n" + "\"telephone\": { \"type\": \"string\" }\n" + "},\n" + - "\"required\": [\"email\", \"email\", \"address\"]\n" + + "\"required\": [\"address\"]\n" + "}\n"; SchemaInfo toValidateAgainst2 = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); - + toValidateAgainstList.clear(); + toValidateAgainstList.add(toValidateAgainst2); + Assert.assertFalse(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList)); } } \ No newline at end of file From c4df98569bcf5cc277a2887eae27e52ab868471c Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Tue, 3 Nov 2020 16:40:20 +0530 Subject: [PATCH 25/30] changes based on PR comments 1 Signed-off-by: Atharva Joshi --- .../ArrayTypeComparator.java | 64 ++++++++++--------- .../JsonCompatibilityChecker.java | 9 ++- .../PropertiesComparator.java | 4 +- .../JsonCompatibilityCheckerTest.java | 12 +++- 4 files changed, 53 insertions(+), 36 deletions(-) diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java index 8ba089901..037a63c80 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java @@ -2,17 +2,24 @@ import com.fasterxml.jackson.databind.JsonNode; import com.google.common.collect.Iterators; -import io.pravega.schemaregistry.rules.CompatibilityChecker; import java.util.Iterator; import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.BreakingChanges; +/** + * this class provides methods for comparing array types in 2 schemas. + * the simple body check is used for instances when an array type node is found. Not when there is an array type. + * being defined + */ public class ArrayTypeComparator { + JsonCompatibilityChecker jsonCompatibilityChecker; + public void setJsonTypeComparator() { this.jsonCompatibilityChecker = new JsonCompatibilityChecker(); } + public BreakingChanges compareArrays(JsonNode toCheck, JsonNode toCheckAgainst) { if (toCheck.isArray() && toCheckAgainst.isArray()) return arraySimpleBodyComparision(toCheck, toCheckAgainst); @@ -26,18 +33,18 @@ public BreakingChanges compareArrays(JsonNode toCheck, JsonNode toCheckAgainst) if (additionalItemsChanges != null) return additionalItemsChanges; BreakingChanges itemsValidationChanges = itemValidation(toCheck, toCheckAgainst); - if(itemsValidationChanges != null) + if (itemsValidationChanges != null) return itemsValidationChanges; - //TO DO: Add contains and tupleValidation + //TODO: Add contains and tupleValidation } return null; } private BreakingChanges arraySimpleBodyComparision(JsonNode toCheck, JsonNode toCheckAgainst) { Iterator allNodes = Iterators.concat(toCheck.fieldNames(), toCheckAgainst.fieldNames()); - while(allNodes.hasNext()) { + while (allNodes.hasNext()) { String item = allNodes.next(); - if(!toCheck.has(item)) + if (!toCheck.has(item)) return BreakingChanges.ARRAY_SIMPLE_BODY_CHECK_ELEMENT_REMOVED; else if (!toCheckAgainst.has(item)) return BreakingChanges.ARRAY_SIMPLE_BODY_CHECK_ELEMENT_ADDED; @@ -46,12 +53,11 @@ else if (!toCheckAgainst.has(item)) } private BreakingChanges checkUniqueItems(JsonNode toCheck, JsonNode toCheckAgainst) { - if(toCheck.has("uniqueItems")) { + if (toCheck.has("uniqueItems")) { if (toCheck.get("uniqueItems").isBoolean() && toCheck.get("uniqueItems").asText() == "true") { - if (toCheckAgainst.get("uniqueItems") == null) - return BreakingChanges.ARRAY_UNIQUE_ITEMS_CONDITION_ENABLED; - else if (toCheckAgainst.get("uniqueItems").isBoolean() && toCheckAgainst.get( - "uniqueItems").asText().equals("false")) + if (toCheckAgainst.get("uniqueItems") == null || (toCheckAgainst.get( + "uniqueItems").isBoolean() && toCheckAgainst.get( + "uniqueItems").asText().equals("false"))) return BreakingChanges.ARRAY_UNIQUE_ITEMS_CONDITION_ENABLED; } } @@ -86,46 +92,46 @@ else if (toCheck.get("additionalItems").isObject()) return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_DECREASED; } else if (toCheck.get("additionalItems") != null && toCheckAgainst.get("additionalItems") != null) { if (toCheck.get("additionalItems").isBoolean() && toCheck.get("additionalItems").asText() == "false") { - if(!(toCheckAgainst.get("additionalItems").asText() == "false")) + if (!(toCheckAgainst.get("additionalItems").asText() == "false")) return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_DISABLED; - } - else if (toCheck.get("additionalItems").isObject()) { - if(toCheckAgainst.get("additionalItems").isObject()) { - if(jsonCompatibilityChecker.checkNodeType(toCheck.get("additionalItems"), toCheckAgainst.get("additionalItems")) != null) + } else if (toCheck.get("additionalItems").isObject()) { + if (toCheckAgainst.get("additionalItems").isObject()) { + if (jsonCompatibilityChecker.checkNodeType(toCheck.get("additionalItems"), + toCheckAgainst.get("additionalItems")) != null) return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_INCOMPATIBLE_CHANGE; - } - else if(toCheckAgainst.get("additionalItems").isBoolean() && toCheckAgainst.get("additionalItems").asText() == "true") + } else if (toCheckAgainst.get("additionalItems").isBoolean() && toCheckAgainst.get( + "additionalItems").asText() == "true") return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_DECREASED; } } return null; } - + private BreakingChanges itemValidation(JsonNode toCheck, JsonNode toCheckAgainst) { - if(!toCheck.has("items") && !toCheckAgainst.has("items")) + if (!toCheck.has("items") && !toCheckAgainst.has("items")) return null; return jsonCompatibilityChecker.checkNodeType(toCheck.get("items"), toCheckAgainst.get("items")); } - + private boolean isDynamicArray(JsonNode node) { - if(node.get("additionalItems") == null) + if (node.get("additionalItems") == null) return true; - else if(node.get("additionalItems").isBoolean()) { - if(node.get("additionalItems").asText() == "true") + else if (node.get("additionalItems").isBoolean()) { + if (node.get("additionalItems").asText() == "true") return true; } return false; } - - private boolean isDynamicArrayWithCondition(JsonNode node) { - if(node.get("additionalItems").isObject()) + + private boolean isDynamicArrayWithCondition(JsonNode node) { + if (node.get("additionalItems").isObject()) return true; return false; } - + private boolean isStaticArray(JsonNode node) { - if(node.get("additionalItems").isBoolean()) { - if(node.get("additionalItems").asText() == "false") + if (node.get("additionalItems").isBoolean()) { + if (node.get("additionalItems").asText() == "false") return true; } return false; diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java index 4b74585ac..fc1b85c7a 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java @@ -6,6 +6,7 @@ import io.pravega.schemaregistry.rules.CompatibilityChecker; import java.io.IOException; +import java.util.Collections; import java.util.List; import java.util.stream.Collectors; @@ -18,6 +19,7 @@ public class JsonCompatibilityChecker implements CompatibilityChecker { NumberComparator numberComparator; StringComparator stringComparator; ArrayTypeComparator arrayTypeComparator; + public JsonCompatibilityChecker() { this.enumComparator = new EnumComparator(); this.jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); @@ -39,12 +41,13 @@ public boolean canRead(SchemaInfo readUsing, List writtenUsing) { @Override public boolean canBeRead(SchemaInfo writtenUsing, List readUsing) { - return false; + return readUsing.stream().map(x -> canRead(x, Collections.singletonList(writtenUsing))).allMatch(x -> x.equals(true)); } @Override public boolean canMutuallyRead(SchemaInfo schema, List schemaList) { - return false; + return schemaList.stream().map(x -> canRead(schema, Collections.singletonList(x)) && canBeRead(x, + Collections.singletonList(schema))).allMatch(x -> x.equals(true)); } @@ -99,7 +102,7 @@ private BreakingChanges analyzeMismatch(JsonNode toCheck, JsonNode toCheckAgains "integer")) && jsonCompatibilityCheckerUtils.getTypeValue(toCheckAgainst).equals( "number") || jsonCompatibilityCheckerUtils.getTypeValue(toCheckAgainst).equals("integer")) return numberComparator.compareNumbers(toCheck, toCheckAgainst); - else + else return BreakingChanges.DATA_TYPE_MISMATCH; } diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java index 4ceec2c19..627ae8868 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java @@ -27,7 +27,7 @@ public BreakingChanges checkProperties(JsonNode toCheck, JsonNode toCheckAgainst if (!jsonCompatibilityCheckerUtils.hasStaticPropertySet( toCheck) && !jsonCompatibilityCheckerUtils.hasDynamicPropertySet(toCheck)) { // assume that pattern properties always matches - // TO DO: add regex check for pattern properties + // TODO: add regex check for pattern properties BreakingChanges breakingChanges = jsonCompatibilityChecker.checkNodeType( toCheck.get("additionalProperties"), toCheckAgainst.get("properties").get(propertyField)); @@ -48,7 +48,7 @@ public BreakingChanges checkProperties(JsonNode toCheck, JsonNode toCheckAgainst if (!jsonCompatibilityCheckerUtils.hasStaticPropertySet( toCheckAgainst) && !jsonCompatibilityCheckerUtils.hasDynamicPropertySet(toCheckAgainst)) { // assume that pattern properties always matches - // TO DO: add regex check for pattern properties + // TODO: add regex check for pattern properties BreakingChanges breakingChanges = jsonCompatibilityChecker.checkNodeType( toCheck.get("properties").get(propertyField), toCheckAgainst.get("additionalProperties")); diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java index 16d060375..dad2095a4 100644 --- a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java @@ -9,12 +9,13 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.util.ArrayList; +import java.util.Collections; import java.util.List; public class JsonCompatibilityCheckerTest { @Test - public void testCanRead() { + public void testEqualCase() { JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); String x = "{\n" + "\"type\": \"object\",\n" + @@ -54,7 +55,14 @@ public void testCanRead() { SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(y.getBytes()), ImmutableMap.of()); toValidateAgainst.add(schemaInfo1); - Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst)); + boolean canRead = jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst); + Assert.assertTrue(canRead); + //CanBeRead + boolean canBeRead = toValidateAgainst.stream().map(p -> jsonCompatibilityChecker.canBeRead(p, + Collections.singletonList(toValidate))).allMatch(p -> p.equals(true)); + Assert.assertTrue(canBeRead); + // canMutuallyRead + Assert.assertTrue(canRead && canBeRead); } @Test From f7254b251cc82cc2196abbdf3ac668f695db76f8 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Tue, 3 Nov 2020 20:10:11 +0530 Subject: [PATCH 26/30] test cases for canBeRead Signed-off-by: Atharva Joshi --- .../JsonCompatibilityCheckerTest.java | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java index dad2095a4..1c0ccd5d2 100644 --- a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java @@ -55,14 +55,9 @@ public void testEqualCase() { SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(y.getBytes()), ImmutableMap.of()); toValidateAgainst.add(schemaInfo1); - boolean canRead = jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst); - Assert.assertTrue(canRead); + Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst)); //CanBeRead - boolean canBeRead = toValidateAgainst.stream().map(p -> jsonCompatibilityChecker.canBeRead(p, - Collections.singletonList(toValidate))).allMatch(p -> p.equals(true)); - Assert.assertTrue(canBeRead); - // canMutuallyRead - Assert.assertTrue(canRead && canBeRead); + Assert.assertTrue(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainst)); } @Test @@ -113,8 +108,11 @@ public void testDependencies() { List toValidateAgainst = new ArrayList<>(); toValidateAgainst.add(toValidate); Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst)); + // canBeRead + Assert.assertTrue(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainst)); toValidateAgainst.add(schemaInfo1); Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst)); + Assert.assertFalse(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainst)); String z = "{\n" + "\"type\": \"object\",\n" + "\"properties\": {\n" + @@ -135,6 +133,7 @@ public void testDependencies() { ByteBuffer.wrap(z.getBytes()), ImmutableMap.of()); toValidateAgainst.add(schemaInfo11); Assert.assertFalse(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainst)); + Assert.assertFalse(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainst)); } @Test @@ -164,6 +163,7 @@ public void testBasicProperties() throws IOException { List toValidateAgainstList = new ArrayList<>(); toValidateAgainstList.add(toValidateAgainst); Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainstList)); + Assert.assertTrue(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList)); //different properties x2 = "{\n" + "\"type\": \"object\",\n" + @@ -177,6 +177,7 @@ public void testBasicProperties() throws IOException { ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); toValidateAgainstList.add(toValidateAgainst1); Assert.assertFalse(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainstList)); + Assert.assertFalse(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList)); //different property values x2 = "{\n" + "\"type\": \"object\",\n" + @@ -190,6 +191,7 @@ public void testBasicProperties() throws IOException { ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); toValidateAgainstList.add(toValidateAgainst2); Assert.assertFalse(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainstList)); + Assert.assertFalse(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList)); } @Test @@ -213,6 +215,7 @@ public void testRequired() throws IOException { List toValidateAgainstList = new ArrayList<>(); toValidateAgainstList.add(toValidateAgainst); Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainstList)); + Assert.assertTrue(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList)); //remove required array element String x2 = "{\n" + "\"type\": \"object\",\n" + @@ -228,6 +231,7 @@ public void testRequired() throws IOException { ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); toValidateAgainstList.add(toValidateAgainst1); Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainstList)); + Assert.assertFalse(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList)); x2 = "{\n" + "\"type\": \"object\",\n" + "\"properties\": {\n" + @@ -236,13 +240,14 @@ public void testRequired() throws IOException { "\"address\": { \"type\": \"string\" },\n" + "\"telephone\": { \"type\": \"string\" }\n" + "},\n" + - "\"required\": [\"address\"]\n" + + "\"required\": [\"name\"]\n" + "}\n"; SchemaInfo toValidateAgainst2 = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); toValidateAgainstList.clear(); toValidateAgainstList.add(toValidateAgainst2); - Assert.assertFalse(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList)); + Assert.assertFalse(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainstList)); + Assert.assertTrue(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList)); } } \ No newline at end of file From f7f52ce54a66f28f0afebfa2882279474950209b Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Thu, 5 Nov 2020 13:24:52 +0530 Subject: [PATCH 27/30] integration test - dynamic properties Signed-off-by: Atharva Joshi --- .../JsonCompatibilityCheckerTest.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java index 1c0ccd5d2..15cbd378e 100644 --- a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java @@ -249,5 +249,39 @@ public void testRequired() throws IOException { Assert.assertFalse(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainstList)); Assert.assertTrue(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList)); } + + @Test + public void testDynamicProperties() { + JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); + String x1 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"email\": { \"type\": \"string\" },\n" + + "\"address\": { \"type\": \"string\" },\n" + + "\"telephone\": { \"type\": \"string\" }\n" + + "},\n" + + "\"additionalProperties\": { \"type\": \"string\" }\n" + + "}\n"; + String x2 = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"email\": { \"type\": \"string\" },\n" + + "\"address\": { \"type\": \"string\" },\n" + + "\"telephone\": { \"type\": \"string\" },\n" + + "\"SSN\": { \"type\": \"number\" }\n" + + "},\n" + + "\"additionalProperties\": { \"type\": \"string\" }\n" + + "}\n"; + SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x1.getBytes()), + ImmutableMap.of()); + SchemaInfo toValidateAgainst = new SchemaInfo("toValidateAgainst", SerializationFormat.Json, + ByteBuffer.wrap(x2.getBytes()), ImmutableMap.of()); + List toValidateAgainstList = new ArrayList<>(); + toValidateAgainstList.add(toValidateAgainst); + Assert.assertFalse(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainstList)); + Assert.assertFalse(jsonCompatibilityChecker.canBeRead(toValidate, toValidateAgainstList)); + } } \ No newline at end of file From 83c6e97c4457b7a05d9bf6e6d7f64443798c4111 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Tue, 24 Nov 2020 14:45:14 +0530 Subject: [PATCH 28/30] subschema comparator Signed-off-by: Atharva Joshi --- .../BreakingChangesStore.java | 12 +- .../JsonCompatibilityChecker.java | 10 +- .../JsonCompatibilityCheckerUtils.java | 7 ++ .../SubSchemaComparator.java | 114 ++++++++++++++++++ 4 files changed, 138 insertions(+), 5 deletions(-) create mode 100644 server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/SubSchemaComparator.java diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java index 301083827..a09688c78 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/BreakingChangesStore.java @@ -70,10 +70,14 @@ protected enum BreakingChanges { // NOT TYPE NOT_TYPE_EXTENDED, // COMBINED - COMBINED_TYPE_SUBSCHEMAS_CHANGED, - COMPOSITION_METHOD_CHANGED, - PRODUCT_TYPE_EXTENDED, - SUM_TYPE_NARROWED + SUBSCHEMA_TYPE_ADDED, + SUBSCHEMA_TYPE_CHANGED, + SUBSCHEMA_TYPE_ALL_OF_SCHEMAS_INCREASED, + SUBSCHEMA_TYPE_ALL_OF_SCHEMAS_CHANGED, + SUBSCHEMA_TYPE_ONE_OF_SCHEMAS_DECREASED, + SUBSCHEMA_TYPE_ONE_OF_SCHEMAS_CHANGED, + SUBSCHEMA_TYPE_ANY_OF_SCHEMAS_DECREASED, + SUBSCHEMA_TYPE_ANYOF_SCHEMAS_CHANGED } private List breakingChangesList = new ArrayList<>(); diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java index fc1b85c7a..14e472785 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java @@ -19,7 +19,8 @@ public class JsonCompatibilityChecker implements CompatibilityChecker { NumberComparator numberComparator; StringComparator stringComparator; ArrayTypeComparator arrayTypeComparator; - + SubSchemaComparator subSchemaComparator; + public JsonCompatibilityChecker() { this.enumComparator = new EnumComparator(); this.jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils(); @@ -27,6 +28,7 @@ public JsonCompatibilityChecker() { this.numberComparator = new NumberComparator(); this.stringComparator = new StringComparator(); this.arrayTypeComparator = new ArrayTypeComparator(); + this.subSchemaComparator = new SubSchemaComparator(); } @Override @@ -67,6 +69,12 @@ private boolean canReadChecker(SchemaInfo toValidate, List toValidat } protected BreakingChanges checkNodeType(JsonNode toCheck, JsonNode toCheckAgainst) { + if(jsonCompatibilityCheckerUtils.hasSubSchema(toCheck)) { + subSchemaComparator.setJsonCompatibilityChecker(); + BreakingChanges subSchemaChanges = subSchemaComparator.checkSubSchemas(toCheck, toCheckAgainst); + if(subSchemaChanges != null) + return subSchemaChanges; + } if (toCheck.has("enum") || toCheckAgainst.has("enum")) { BreakingChanges enumChanges = enumComparator.enumComparator(toCheck, toCheckAgainst); if (enumChanges != null) diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java index dc0781489..08c64de0b 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtils.java @@ -79,4 +79,11 @@ public boolean arrayComparisionOnlyAddition(ArrayNode toCheck, ArrayNode toCheck } return true; } + + public boolean hasSubSchema(JsonNode toCheck) { + if(toCheck.has("anyOf") || toCheck.has("allOf") || toCheck.has("oneOf")) { + return true; + } + return false; + } } diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/SubSchemaComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/SubSchemaComparator.java new file mode 100644 index 000000000..4c5f53437 --- /dev/null +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/SubSchemaComparator.java @@ -0,0 +1,114 @@ +package io.pravega.schemaregistry.rules.jsoncompatibility; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ArrayNode; + +import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.*; + +public class SubSchemaComparator { + JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); + + public void setJsonCompatibilityChecker() { + this.jsonCompatibilityChecker = new JsonCompatibilityChecker(); + } + + public BreakingChanges checkSubSchemas(JsonNode toCheck, JsonNode toCheckAgainst) { + if(toCheck.has("anyOf")) { + if(toCheckAgainst.has("oneOf") || toCheckAgainst.has("allOf")) + return BreakingChanges.SUBSCHEMA_TYPE_CHANGED; + else { + if(toCheckAgainst.has("anyOf")) + return nonAllOfComparator(toCheck, toCheckAgainst); + else + return BreakingChanges.SUBSCHEMA_TYPE_ADDED; + } + } + else if(toCheck.has("oneOf")) { + if(toCheckAgainst.has("anyOf") || toCheckAgainst.has("allOf")) + return BreakingChanges.SUBSCHEMA_TYPE_CHANGED; + else { + if(toCheckAgainst.has("oneOf")) + return nonAllOfComparator(toCheck, toCheckAgainst); + else + return BreakingChanges.SUBSCHEMA_TYPE_ADDED; + } + } + else if(toCheck.has("allOf")) { + if(toCheckAgainst.has("anyOf") || toCheckAgainst.has("oneOf")) + return BreakingChanges.SUBSCHEMA_TYPE_CHANGED; + else { + if(toCheckAgainst.has("allOf")) + return allOfComparator(toCheck, toCheckAgainst); + else + return BreakingChanges.SUBSCHEMA_TYPE_ADDED; + } + } + return null; + } + + private BreakingChanges nonAllOfComparator(JsonNode toCheck, JsonNode toCheckAgainst) { + if(toCheck.has("oneOf")) { + ArrayNode toCheckArray = (ArrayNode) toCheck.get("oneOf"); + ArrayNode toCheckAgainstArray = (ArrayNode) toCheckAgainst.get("oneOf"); + if(toCheckArray.size() < toCheckAgainstArray.size()) + return BreakingChanges.SUBSCHEMA_TYPE_ONE_OF_SCHEMAS_DECREASED; + else { + if (!nonAllOfCompatibilityComputation(toCheckArray, toCheckAgainstArray)) + return BreakingChanges.SUBSCHEMA_TYPE_ONE_OF_SCHEMAS_CHANGED; + } + } + else { + ArrayNode toCheckArray = (ArrayNode) toCheck.get("anyOf"); + ArrayNode toCheckAgainstArray = (ArrayNode) toCheckAgainst.get("anyOf"); + if(toCheckArray.size() < toCheckAgainstArray.size()) + return BreakingChanges.SUBSCHEMA_TYPE_ANY_OF_SCHEMAS_DECREASED; + else { + if (!nonAllOfCompatibilityComputation(toCheckArray, toCheckAgainstArray)) + return BreakingChanges.SUBSCHEMA_TYPE_ANYOF_SCHEMAS_CHANGED; + } + } + return null; + } + + private BreakingChanges allOfComparator(JsonNode toCheck, JsonNode toCheckAgainst) { + ArrayNode toCheckArray = (ArrayNode) toCheck.get("allOf"); + ArrayNode toCheckAgainstArray = (ArrayNode) toCheckAgainst.get("allOf"); + if(!(toCheckArray.size() <= toCheckAgainstArray.size())) + return BreakingChanges.SUBSCHEMA_TYPE_ALL_OF_SCHEMAS_INCREASED; + else { + if (!allOfCompatibilityComputation(toCheckArray, toCheckAgainstArray)) + return BreakingChanges.SUBSCHEMA_TYPE_ALL_OF_SCHEMAS_CHANGED; + } + return null; + } + + private boolean allOfCompatibilityComputation(ArrayNode toCheckArray, ArrayNode toCheckAgainstArray) { + for(int i=0;i Date: Tue, 24 Nov 2020 20:02:56 +0530 Subject: [PATCH 29/30] subschema test 1 Signed-off-by: Atharva Joshi --- .../JsonCompatibilityCheckerUtilsTest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtilsTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtilsTest.java index 748be5b0f..6cc75fb4a 100644 --- a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtilsTest.java +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtilsTest.java @@ -122,4 +122,16 @@ public void testArrayComparisionOnlyAddition() throws IOException { ArrayNode originalArray =(ArrayNode) originalNode; Assert.assertTrue(jsonCompatibilityCheckerUtils.arrayComparisionOnlyAddition(finalArray, originalArray)); } + + @Test + public void testHasSubSchema() throws IOException { + String x1 = "{\n" + + "\"anyOf\": [\n" + + "{ \"type\": \"string\" },\n" + + "{ \"type\": \"number\" }\n" + + "]\n" + + "}\n"; + JsonNode node = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + Assert.assertTrue(jsonCompatibilityCheckerUtils.hasSubSchema(node)); + } } From cab2df547565cfadb7246870111a2898cc127469 Mon Sep 17 00:00:00 2001 From: Atharva Joshi Date: Wed, 25 Nov 2020 00:39:23 +0530 Subject: [PATCH 30/30] subschema test 2 Signed-off-by: Atharva Joshi --- .../SubSchemaComparator.java | 2 +- .../JsonCompatibilityCheckerTest.java | 18 +--------- .../JsonCompatibilityCheckerUtilsTest.java | 34 +++++++++++++++++-- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/SubSchemaComparator.java b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/SubSchemaComparator.java index 4c5f53437..57fd365ae 100644 --- a/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/SubSchemaComparator.java +++ b/server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/SubSchemaComparator.java @@ -6,7 +6,7 @@ import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.*; public class SubSchemaComparator { - JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker(); + JsonCompatibilityChecker jsonCompatibilityChecker; public void setJsonCompatibilityChecker() { this.jsonCompatibilityChecker = new JsonCompatibilityChecker(); diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java index 15cbd378e..c16db4eb9 100644 --- a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java @@ -33,26 +33,10 @@ public void testEqualCase() { "}\n" + "}\n" + "}\n"; - String y = "{\n" + - "\"type\": \"object\",\n" + - "\"properties\": {\n" + - "\"name\": { \"type\": \"string\" },\n" + - "\"credit_card\": { \"type\": \"number\" }\n" + - "},\n" + - "\"required\": [\"name\"],\n" + - "\"dependencies\": {\n" + - "\"credit_card\": {\n" + - "\"properties\": {\n" + - "\"billing_address\": { \"type\": \"string\" }\n" + - "},\n" + - "\"required\": [\"billing_address\"]\n" + - "}\n" + - "}\n" + - "}\n"; SchemaInfo toValidate = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x.getBytes()), ImmutableMap.of()); List toValidateAgainst = new ArrayList<>(); - SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(y.getBytes()), + SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(x.getBytes()), ImmutableMap.of()); toValidateAgainst.add(schemaInfo1); Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst)); diff --git a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtilsTest.java b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtilsTest.java index 6cc75fb4a..0f0cd579b 100644 --- a/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtilsTest.java +++ b/server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerUtilsTest.java @@ -125,13 +125,43 @@ public void testArrayComparisionOnlyAddition() throws IOException { @Test public void testHasSubSchema() throws IOException { - String x1 = "{\n" + + String x = "{\n" + "\"anyOf\": [\n" + "{ \"type\": \"string\" },\n" + "{ \"type\": \"number\" }\n" + "]\n" + "}\n"; - JsonNode node = objectMapper.readTree(ByteBuffer.wrap(x1.getBytes()).array()); + JsonNode node = objectMapper.readTree(ByteBuffer.wrap(x.getBytes()).array()); + Assert.assertTrue(jsonCompatibilityCheckerUtils.hasSubSchema(node)); + x = "{\n" + + "\"oneOf\": [\n" + + "{ \"type\": \"string\" },\n" + + "{ \"type\": \"number\" }\n" + + "]\n" + + "}\n"; + node = objectMapper.readTree(ByteBuffer.wrap(x.getBytes()).array()); + Assert.assertTrue(jsonCompatibilityCheckerUtils.hasSubSchema(node)); + x = "{\n" + + "\"allOf\": [\n" + + "{ \"type\": \"string\" },\n" + + "{ \"type\": \"number\" }\n" + + "]\n" + + "}\n"; + node = objectMapper.readTree(ByteBuffer.wrap(x.getBytes()).array()); Assert.assertTrue(jsonCompatibilityCheckerUtils.hasSubSchema(node)); + x = "{\n" + + "\"type\": \"object\",\n" + + "\"properties\": {\n" + + "\"name\": { \"type\": \"string\" },\n" + + "\"credit_card\": { \"type\": \"number\" },\n" + + "\"billing_address\": { \"type\": \"string\" }\n" + + "},\n" + + "\"required\": [\"name\"],\n" + + "\"dependencies\": {\n" + + "\"credit_card\": [\"billing_address\"]\n" + + "}\n" + + "}\n"; + node = objectMapper.readTree(ByteBuffer.wrap(x.getBytes()).array()); + Assert.assertFalse(jsonCompatibilityCheckerUtils.hasSubSchema(node)); } }