Skip to content

Commit 2785420

Browse files
Atharva JoshiAtharva Joshi
Atharva Joshi
authored and
Atharva Joshi
committed
changes based on PR comments 1
Signed-off-by: Atharva Joshi <[email protected]>
1 parent d998053 commit 2785420

File tree

4 files changed

+53
-36
lines changed

4 files changed

+53
-36
lines changed

‎server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/ArrayTypeComparator.java‎

Lines changed: 35 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,24 @@
22

33
import com.fasterxml.jackson.databind.JsonNode;
44
import com.google.common.collect.Iterators;
5-
import io.pravega.schemaregistry.rules.CompatibilityChecker;
65

76
import java.util.Iterator;
87

98
import static io.pravega.schemaregistry.rules.jsoncompatibility.BreakingChangesStore.BreakingChanges;
109

10+
/**
11+
* this class provides methods for comparing array types in 2 schemas.
12+
* the simple body check is used for instances when an array type node is found. Not when there is an array type.
13+
* being defined
14+
*/
1115
public class ArrayTypeComparator {
16+
1217
JsonCompatibilityChecker jsonCompatibilityChecker;
18+
1319
public void setJsonTypeComparator() {
1420
this.jsonCompatibilityChecker = new JsonCompatibilityChecker();
1521
}
22+
1623
public BreakingChanges compareArrays(JsonNode toCheck, JsonNode toCheckAgainst) {
1724
if (toCheck.isArray() && toCheckAgainst.isArray())
1825
return arraySimpleBodyComparision(toCheck, toCheckAgainst);
@@ -26,18 +33,18 @@ public BreakingChanges compareArrays(JsonNode toCheck, JsonNode toCheckAgainst)
2633
if (additionalItemsChanges != null)
2734
return additionalItemsChanges;
2835
BreakingChanges itemsValidationChanges = itemValidation(toCheck, toCheckAgainst);
29-
if(itemsValidationChanges != null)
36+
if (itemsValidationChanges != null)
3037
return itemsValidationChanges;
31-
//TO DO: Add contains and tupleValidation
38+
//TODO: Add contains and tupleValidation
3239
}
3340
return null;
3441
}
3542

3643
private BreakingChanges arraySimpleBodyComparision(JsonNode toCheck, JsonNode toCheckAgainst) {
3744
Iterator<String> allNodes = Iterators.concat(toCheck.fieldNames(), toCheckAgainst.fieldNames());
38-
while(allNodes.hasNext()) {
45+
while (allNodes.hasNext()) {
3946
String item = allNodes.next();
40-
if(!toCheck.has(item))
47+
if (!toCheck.has(item))
4148
return BreakingChanges.ARRAY_SIMPLE_BODY_CHECK_ELEMENT_REMOVED;
4249
else if (!toCheckAgainst.has(item))
4350
return BreakingChanges.ARRAY_SIMPLE_BODY_CHECK_ELEMENT_ADDED;
@@ -46,12 +53,11 @@ else if (!toCheckAgainst.has(item))
4653
}
4754

4855
private BreakingChanges checkUniqueItems(JsonNode toCheck, JsonNode toCheckAgainst) {
49-
if(toCheck.has("uniqueItems")) {
56+
if (toCheck.has("uniqueItems")) {
5057
if (toCheck.get("uniqueItems").isBoolean() && toCheck.get("uniqueItems").asText() == "true") {
51-
if (toCheckAgainst.get("uniqueItems") == null)
52-
return BreakingChanges.ARRAY_UNIQUE_ITEMS_CONDITION_ENABLED;
53-
else if (toCheckAgainst.get("uniqueItems").isBoolean() && toCheckAgainst.get(
54-
"uniqueItems").asText().equals("false"))
58+
if (toCheckAgainst.get("uniqueItems") == null || (toCheckAgainst.get(
59+
"uniqueItems").isBoolean() && toCheckAgainst.get(
60+
"uniqueItems").asText().equals("false")))
5561
return BreakingChanges.ARRAY_UNIQUE_ITEMS_CONDITION_ENABLED;
5662
}
5763
}
@@ -86,46 +92,46 @@ else if (toCheck.get("additionalItems").isObject())
8692
return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_DECREASED;
8793
} else if (toCheck.get("additionalItems") != null && toCheckAgainst.get("additionalItems") != null) {
8894
if (toCheck.get("additionalItems").isBoolean() && toCheck.get("additionalItems").asText() == "false") {
89-
if(!(toCheckAgainst.get("additionalItems").asText() == "false"))
95+
if (!(toCheckAgainst.get("additionalItems").asText() == "false"))
9096
return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_DISABLED;
91-
}
92-
else if (toCheck.get("additionalItems").isObject()) {
93-
if(toCheckAgainst.get("additionalItems").isObject()) {
94-
if(jsonCompatibilityChecker.checkNodeType(toCheck.get("additionalItems"), toCheckAgainst.get("additionalItems")) != null)
97+
} else if (toCheck.get("additionalItems").isObject()) {
98+
if (toCheckAgainst.get("additionalItems").isObject()) {
99+
if (jsonCompatibilityChecker.checkNodeType(toCheck.get("additionalItems"),
100+
toCheckAgainst.get("additionalItems")) != null)
95101
return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_INCOMPATIBLE_CHANGE;
96-
}
97-
else if(toCheckAgainst.get("additionalItems").isBoolean() && toCheckAgainst.get("additionalItems").asText() == "true")
102+
} else if (toCheckAgainst.get("additionalItems").isBoolean() && toCheckAgainst.get(
103+
"additionalItems").asText() == "true")
98104
return BreakingChanges.ARRAY_ADDITIONAL_ITEMS_SCOPE_DECREASED;
99105
}
100106
}
101107
return null;
102108
}
103-
109+
104110
private BreakingChanges itemValidation(JsonNode toCheck, JsonNode toCheckAgainst) {
105-
if(!toCheck.has("items") && !toCheckAgainst.has("items"))
111+
if (!toCheck.has("items") && !toCheckAgainst.has("items"))
106112
return null;
107113
return jsonCompatibilityChecker.checkNodeType(toCheck.get("items"), toCheckAgainst.get("items"));
108114
}
109-
115+
110116
private boolean isDynamicArray(JsonNode node) {
111-
if(node.get("additionalItems") == null)
117+
if (node.get("additionalItems") == null)
112118
return true;
113-
else if(node.get("additionalItems").isBoolean()) {
114-
if(node.get("additionalItems").asText() == "true")
119+
else if (node.get("additionalItems").isBoolean()) {
120+
if (node.get("additionalItems").asText() == "true")
115121
return true;
116122
}
117123
return false;
118124
}
119-
120-
private boolean isDynamicArrayWithCondition(JsonNode node) {
121-
if(node.get("additionalItems").isObject())
125+
126+
private boolean isDynamicArrayWithCondition(JsonNode node) {
127+
if (node.get("additionalItems").isObject())
122128
return true;
123129
return false;
124130
}
125-
131+
126132
private boolean isStaticArray(JsonNode node) {
127-
if(node.get("additionalItems").isBoolean()) {
128-
if(node.get("additionalItems").asText() == "false")
133+
if (node.get("additionalItems").isBoolean()) {
134+
if (node.get("additionalItems").asText() == "false")
129135
return true;
130136
}
131137
return false;

‎server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityChecker.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import io.pravega.schemaregistry.rules.CompatibilityChecker;
77

88
import java.io.IOException;
9+
import java.util.Collections;
910
import java.util.List;
1011
import java.util.stream.Collectors;
1112

@@ -18,6 +19,7 @@ public class JsonCompatibilityChecker implements CompatibilityChecker {
1819
NumberComparator numberComparator;
1920
StringComparator stringComparator;
2021
ArrayTypeComparator arrayTypeComparator;
22+
2123
public JsonCompatibilityChecker() {
2224
this.enumComparator = new EnumComparator();
2325
this.jsonCompatibilityCheckerUtils = new JsonCompatibilityCheckerUtils();
@@ -39,12 +41,13 @@ public boolean canRead(SchemaInfo readUsing, List<SchemaInfo> writtenUsing) {
3941

4042
@Override
4143
public boolean canBeRead(SchemaInfo writtenUsing, List<SchemaInfo> readUsing) {
42-
return false;
44+
return readUsing.stream().map(x -> canRead(x, Collections.singletonList(writtenUsing))).allMatch(x -> x.equals(true));
4345
}
4446

4547
@Override
4648
public boolean canMutuallyRead(SchemaInfo schema, List<SchemaInfo> schemaList) {
47-
return false;
49+
return schemaList.stream().map(x -> canRead(schema, Collections.singletonList(x)) && canBeRead(x,
50+
Collections.singletonList(schema))).allMatch(x -> x.equals(true));
4851
}
4952

5053

@@ -99,7 +102,7 @@ private BreakingChanges analyzeMismatch(JsonNode toCheck, JsonNode toCheckAgains
99102
"integer")) && jsonCompatibilityCheckerUtils.getTypeValue(toCheckAgainst).equals(
100103
"number") || jsonCompatibilityCheckerUtils.getTypeValue(toCheckAgainst).equals("integer"))
101104
return numberComparator.compareNumbers(toCheck, toCheckAgainst);
102-
else
105+
else
103106
return BreakingChanges.DATA_TYPE_MISMATCH;
104107
}
105108

‎server/src/main/java/io/pravega/schemaregistry/rules/jsoncompatibility/PropertiesComparator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public BreakingChanges checkProperties(JsonNode toCheck, JsonNode toCheckAgainst
2727
if (!jsonCompatibilityCheckerUtils.hasStaticPropertySet(
2828
toCheck) && !jsonCompatibilityCheckerUtils.hasDynamicPropertySet(toCheck)) {
2929
// assume that pattern properties always matches
30-
// TO DO: add regex check for pattern properties
30+
// TODO: add regex check for pattern properties
3131
BreakingChanges breakingChanges = jsonCompatibilityChecker.checkNodeType(
3232
toCheck.get("additionalProperties"),
3333
toCheckAgainst.get("properties").get(propertyField));
@@ -48,7 +48,7 @@ public BreakingChanges checkProperties(JsonNode toCheck, JsonNode toCheckAgainst
4848
if (!jsonCompatibilityCheckerUtils.hasStaticPropertySet(
4949
toCheckAgainst) && !jsonCompatibilityCheckerUtils.hasDynamicPropertySet(toCheckAgainst)) {
5050
// assume that pattern properties always matches
51-
// TO DO: add regex check for pattern properties
51+
// TODO: add regex check for pattern properties
5252
BreakingChanges breakingChanges = jsonCompatibilityChecker.checkNodeType(
5353
toCheck.get("properties").get(propertyField),
5454
toCheckAgainst.get("additionalProperties"));

‎server/src/test/java/io/pravega/schemaregistry/rules/jsoncompatibility/JsonCompatibilityCheckerTest.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@
99
import java.io.IOException;
1010
import java.nio.ByteBuffer;
1111
import java.util.ArrayList;
12+
import java.util.Collections;
1213
import java.util.List;
1314

1415
public class JsonCompatibilityCheckerTest {
1516

1617
@Test
17-
public void testCanRead() {
18+
public void testEqualCase() {
1819
JsonCompatibilityChecker jsonCompatibilityChecker = new JsonCompatibilityChecker();
1920
String x = "{\n" +
2021
"\"type\": \"object\",\n" +
@@ -54,7 +55,14 @@ public void testCanRead() {
5455
SchemaInfo schemaInfo1 = new SchemaInfo("toValidate", SerializationFormat.Json, ByteBuffer.wrap(y.getBytes()),
5556
ImmutableMap.of());
5657
toValidateAgainst.add(schemaInfo1);
57-
Assert.assertTrue(jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst));
58+
boolean canRead = jsonCompatibilityChecker.canRead(toValidate, toValidateAgainst);
59+
Assert.assertTrue(canRead);
60+
//CanBeRead
61+
boolean canBeRead = toValidateAgainst.stream().map(p -> jsonCompatibilityChecker.canBeRead(p,
62+
Collections.singletonList(toValidate))).allMatch(p -> p.equals(true));
63+
Assert.assertTrue(canBeRead);
64+
// canMutuallyRead
65+
Assert.assertTrue(canRead && canBeRead);
5866
}
5967

6068
@Test

0 commit comments

Comments
 (0)