diff --git a/.github/workflows/verify.yaml b/.github/workflows/verify.yaml index 98ca6aa04..01d6c11b2 100755 --- a/.github/workflows/verify.yaml +++ b/.github/workflows/verify.yaml @@ -7,14 +7,14 @@ on: - 'LICENSE' - 'README*' - 'CODE_OF_CONDUCT*' - branches: [main] + branches: [main, feature/json-schema-rewrite-clean] pull_request: paths-ignore: - '.gitignore' - 'LICENSE' - 'README*' - 'CODE_OF_CONDUCT*' - branches: [main] + branches: [main, feature/json-schema-rewrite-clean] jobs: build-verify: diff --git a/data-models/src/main/java/io/apitomy/datamodels/jsonschema/compat/CompoundSchemaDiffVisitor.java b/data-models/src/main/java/io/apitomy/datamodels/jsonschema/compat/CompoundSchemaDiffVisitor.java index ca25c7720..8f55d9b78 100644 --- a/data-models/src/main/java/io/apitomy/datamodels/jsonschema/compat/CompoundSchemaDiffVisitor.java +++ b/data-models/src/main/java/io/apitomy/datamodels/jsonschema/compat/CompoundSchemaDiffVisitor.java @@ -4,7 +4,6 @@ import io.apitomy.datamodels.jsonschema.convert.CompoundSchemaConverter; import io.apitomy.datamodels.models.ModelType; import io.apitomy.datamodels.models.jsonschema.BooleanFullSchemaFullSchemaListUnion; -import io.apitomy.datamodels.models.jsonschema.Dependency; import io.apitomy.datamodels.models.jsonschema.JFullSchema; import io.apitomy.datamodels.models.jsonschema.JsonSchema; import io.apitomy.datamodels.models.jsonschema.compound.JCFullSchema; @@ -19,6 +18,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import static io.apitomy.datamodels.jsonschema.compat.DiffType.*; import static io.apitomy.datamodels.jsonschema.compat.DiffUtil.*; @@ -233,7 +233,18 @@ private boolean isEmptyOrTrueSchema(JFullSchema schema) { public boolean visitFullSchemaPatternProperty(JsonSchema original, JsonSchema updated) { return false; } @Override - public boolean visitFullSchemaDependency(Dependency original, Dependency updated) { return false; } + public boolean visitFullSchemaDependentSchema(JsonSchema original, JsonSchema updated) { + if (original != null && updated != null + && original.isFullSchema() && updated.isFullSchema()) { + var subCtx = ctx.sub("dependentSchemas"); + if (!isSchemaCompatible(subCtx, original.asFullSchema(), + updated.asFullSchema(), true)) { + subCtx.addDifference(OBJECT_TYPE_SCHEMA_DEPENDENCIES_CHANGED, + original, updated); + } + } + return false; + } @Override public boolean visitFullSchemaAllOfItem(JsonSchema original, JsonSchema updated) { return false; } @@ -755,11 +766,31 @@ public boolean diffFullSchemaPropertyNames(JsonSchema original, JsonSchema updat return false; } + // dependencies is always empty after conversion — d4-d7 entries are split + // into dependentSchemas/dependentRequired by the converters. + + @Override + public void diffFullSchemaDependentSchemas(Map original, + Map updated, + CollectionDiff diff) { + if (original == null && updated == null) return; + + var origKeys = original != null + ? new HashSet<>(original.keySet()) : new HashSet(); + var updKeys = updated != null + ? new HashSet<>(updated.keySet()) : new HashSet(); + + diffSetChanged(ctx, origKeys, updKeys, + OBJECT_TYPE_PROPERTY_DEPENDENCIES_KEYS_ADDED, + OBJECT_TYPE_PROPERTY_DEPENDENCIES_KEYS_REMOVED, + OBJECT_TYPE_PROPERTY_DEPENDENCIES_KEYS_CHANGED, + OBJECT_TYPE_PROPERTY_DEPENDENCIES_KEYS_MEMBER_ADDED, + OBJECT_TYPE_PROPERTY_DEPENDENCIES_KEYS_MEMBER_REMOVED); + } + @Override - public void diffFullSchemaDependencies(Map original, - Map updated, - CollectionDiff diff) { - // Suppress auto-recursion for matched dependencies + public void diffFullSchemaDependentRequired(Map original, + Map updated) { if (original == null && updated == null) return; var origKeys = original != null @@ -778,40 +809,43 @@ public void diffFullSchemaDependencies(Map original, var commonKeys = new HashSet<>(origKeys); commonKeys.retainAll(updKeys); for (var key : commonKeys) { - var origValue = original.get(key); - var updValue = updated.get(key); - if (origValue.isStringList() && updValue.isStringList()) { - var origSet = new HashSet<>(origValue.asStringList()); - var updSet = new HashSet<>(updValue.asStringList()); - for (var v : origSet) { - if (!updSet.contains(v)) { - ctx.addDifference( - OBJECT_TYPE_PROPERTY_DEPENDENCIES_VALUE_MEMBER_REMOVED, - v, null); - } - } - for (var v : updSet) { - if (!origSet.contains(v)) { - ctx.addDifference( - OBJECT_TYPE_PROPERTY_DEPENDENCIES_VALUE_MEMBER_ADDED, - null, v); - } - } - if (!origSet.equals(updSet)) { + var origArray = original.get(key); + var updArray = updated.get(key); + var origSet = jsonArrayToStringSet(origArray); + var updSet = jsonArrayToStringSet(updArray); + for (var v : origSet) { + if (!updSet.contains(v)) { ctx.addDifference( - OBJECT_TYPE_PROPERTY_DEPENDENCIES_VALUE_MEMBER_CHANGED, - origValue, updValue); + OBJECT_TYPE_PROPERTY_DEPENDENCIES_VALUE_MEMBER_REMOVED, + v, null); } - } else if (origValue.isFullSchema() && updValue.isFullSchema()) { - var subCtx = ctx.sub("dependencies/" + key); - if (!isSchemaCompatible(subCtx, origValue.asFullSchema(), - updValue.asFullSchema(), true)) { - subCtx.addDifference(OBJECT_TYPE_SCHEMA_DEPENDENCIES_CHANGED, - origValue, updValue); + } + for (var v : updSet) { + if (!origSet.contains(v)) { + ctx.addDifference( + OBJECT_TYPE_PROPERTY_DEPENDENCIES_VALUE_MEMBER_ADDED, + null, v); } } + if (!origSet.equals(updSet)) { + ctx.addDifference( + OBJECT_TYPE_PROPERTY_DEPENDENCIES_VALUE_MEMBER_CHANGED, + origArray, updArray); + } + } + } + } + + private static Set jsonArrayToStringSet(JsonNode arrayNode) { + var set = new HashSet(); + if (arrayNode != null && arrayNode.isArray()) { + for (var element : arrayNode) { + if (element.isTextual()) { + set.add(element.asText()); + } } } + return set; } // ----------------------------------------------------------------------- diff --git a/data-models/src/main/java/io/apitomy/datamodels/jsonschema/compat/JsonSchemaCompatibilityChecker.java b/data-models/src/main/java/io/apitomy/datamodels/jsonschema/compat/JsonSchemaCompatibilityChecker.java index 66ad12004..e0b179f0b 100644 --- a/data-models/src/main/java/io/apitomy/datamodels/jsonschema/compat/JsonSchemaCompatibilityChecker.java +++ b/data-models/src/main/java/io/apitomy/datamodels/jsonschema/compat/JsonSchemaCompatibilityChecker.java @@ -135,10 +135,6 @@ private DiffContext doCheck(String originalSchemaJson, String updatedSchemaJson) var originalCompound = toCompoundFullSchema(originalParsed, originalModelType); var updatedCompound = toCompoundFullSchema(updatedParsed, updatedModelType); - // TODO: Modern version support — flag for now, remove when diff classes handle all keywords - flagModernVersions(ctx, originalModelType); - flagModernVersions(ctx, updatedModelType); - CompoundSchemaDiffVisitor.diffSchemas(ctx, originalCompound, updatedCompound); return ctx; } @@ -151,12 +147,6 @@ private static JFullSchema toCompoundFullSchema(JFullSchema doc, ModelType model throw new IllegalArgumentException("Failed to convert schema to compound type"); } - private static void flagModernVersions(DiffContext ctx, ModelType modelType) { - if (modelType == ModelType.JM201909 || modelType == ModelType.JM202012) { - ctx.addUnsupported("JSON Schema %s (modern version support not yet implemented)".formatted(modelType)); - } - } - private static JFullSchema parseSchema(String schemaJson) { var doc = Library.readRootFromJSONString(schemaJson); if (!(doc instanceof JFullSchema jsonSchemaDoc)) { diff --git a/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/CompoundSchemaConverter.java b/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/CompoundSchemaConverter.java index 93d762f46..ab788b369 100644 --- a/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/CompoundSchemaConverter.java +++ b/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/CompoundSchemaConverter.java @@ -1,14 +1,21 @@ package io.apitomy.datamodels.jsonschema.convert; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import io.apitomy.datamodels.models.Any; import io.apitomy.datamodels.models.ModelType; +import io.apitomy.datamodels.models.jsonschema.Dependency; import io.apitomy.datamodels.models.jsonschema.JsonSchema; +import io.apitomy.datamodels.models.jsonschema.compound.JCFullSchema; import io.apitomy.datamodels.models.jsonschema.draft.draft4.visitors.JD4ToJCConversionTraverser; import io.apitomy.datamodels.models.jsonschema.draft.draft6.visitors.JD6ToJCConversionTraverser; import io.apitomy.datamodels.models.jsonschema.draft.draft7.visitors.JD7ToJCConversionTraverser; import io.apitomy.datamodels.models.jsonschema.modern.v201909.visitors.JM201909ToJCConversionTraverser; import io.apitomy.datamodels.models.jsonschema.modern.v202012.visitors.JM202012ToJCConversionTraverser; +import java.util.LinkedHashMap; +import java.util.Map; + /** * Converts any JSON Schema version to the compound schema type. */ @@ -41,4 +48,29 @@ public static JsonSchema toCompound(JsonSchema source, ModelType modelType) { } return (JsonSchema) result; } + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + /** + * Splits a d4-d7 {@code dependencies} map into {@code dependentSchemas} + * and {@code dependentRequired} on the compound target. + */ + static void splitDependencies(Map value, JCFullSchema target) { + if (value == null) return; + Map requiredMap = null; + for (Map.Entry entry : value.entrySet()) { + var dep = entry.getValue(); + if (dep.isFullSchema()) { + target.addDependentSchema(entry.getKey(), (JsonSchema) dep.asFullSchema()); + } else if (dep.isStringList()) { + if (requiredMap == null) { + requiredMap = new LinkedHashMap<>(); + } + requiredMap.put(entry.getKey(), MAPPER.valueToTree(dep.asStringList())); + } + } + if (requiredMap != null) { + target.setDependentRequired(requiredMap); + } + } } diff --git a/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/JD4ToCompoundConverter.java b/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/JD4ToCompoundConverter.java index 0db425647..9ae0a73c3 100644 --- a/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/JD4ToCompoundConverter.java +++ b/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/JD4ToCompoundConverter.java @@ -1,10 +1,13 @@ package io.apitomy.datamodels.jsonschema.convert; +import io.apitomy.datamodels.models.jsonschema.Dependency; import io.apitomy.datamodels.models.jsonschema.compound.JCFullSchema; import io.apitomy.datamodels.models.jsonschema.compound.JCRangeValue; import io.apitomy.datamodels.models.jsonschema.compound.JCRangeValueImpl; import io.apitomy.datamodels.models.jsonschema.draft.draft4.visitors.JD4ToJCConversionVisitor; +import java.util.Map; + /** * Converts Draft 4 schemas to the compound schema type. * Handles: exclusiveMinimum/Maximum boolean + minimum/maximum number to RangeValue. @@ -43,6 +46,11 @@ public void convertFullSchemaExclusiveMaximum(Boolean value, JCFullSchema target } } + @Override + public void convertFullSchemaDependencies(Map value, JCFullSchema target) { + CompoundSchemaConverter.splitDependencies(value, target); + } + private static JCRangeValue rangeValue(Number value, boolean exclusive) { JCRangeValueImpl rv = new JCRangeValueImpl(); rv.setValue(value); diff --git a/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/JD6ToCompoundConverter.java b/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/JD6ToCompoundConverter.java index 136fd036d..c0711113a 100644 --- a/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/JD6ToCompoundConverter.java +++ b/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/JD6ToCompoundConverter.java @@ -1,11 +1,13 @@ package io.apitomy.datamodels.jsonschema.convert; +import io.apitomy.datamodels.models.jsonschema.Dependency; import io.apitomy.datamodels.models.jsonschema.compound.JCFullSchema; import io.apitomy.datamodels.models.jsonschema.compound.JCRangeValue; import io.apitomy.datamodels.models.jsonschema.compound.JCRangeValueImpl; import io.apitomy.datamodels.models.jsonschema.draft.draft6.visitors.JD6ToJCConversionVisitor; import java.math.BigDecimal; +import java.util.Map; /** * Converts Draft 6 schemas to the compound schema type. @@ -48,6 +50,11 @@ public void convertFullSchemaExclusiveMaximum(Number value, JCFullSchema target) } } + @Override + public void convertFullSchemaDependencies(Map value, JCFullSchema target) { + CompoundSchemaConverter.splitDependencies(value, target); + } + /** * For minimum (lower bound), a higher value is tighter. * At the same value, exclusive is tighter than inclusive. diff --git a/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/JD7ToCompoundConverter.java b/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/JD7ToCompoundConverter.java index 1f26c390d..a3536f1e3 100644 --- a/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/JD7ToCompoundConverter.java +++ b/data-models/src/main/java/io/apitomy/datamodels/jsonschema/convert/JD7ToCompoundConverter.java @@ -1,11 +1,13 @@ package io.apitomy.datamodels.jsonschema.convert; +import io.apitomy.datamodels.models.jsonschema.Dependency; import io.apitomy.datamodels.models.jsonschema.compound.JCFullSchema; import io.apitomy.datamodels.models.jsonschema.compound.JCRangeValue; import io.apitomy.datamodels.models.jsonschema.compound.JCRangeValueImpl; import io.apitomy.datamodels.models.jsonschema.draft.draft7.visitors.JD7ToJCConversionVisitor; import java.math.BigDecimal; +import java.util.Map; /** * Converts Draft 7 schemas to the compound schema type. @@ -48,6 +50,11 @@ public void convertFullSchemaExclusiveMaximum(Number value, JCFullSchema target) } } + @Override + public void convertFullSchemaDependencies(Map value, JCFullSchema target) { + CompoundSchemaConverter.splitDependencies(value, target); + } + private static boolean isTighterMinimum(Number newValue, boolean newExclusive, JCRangeValue existing) { int cmp = new BigDecimal(newValue.toString()).compareTo(new BigDecimal(existing.getValue().toString())); if (cmp > 0) return true; diff --git a/data-models/src/test/resources/io/apitomy/datamodels/jsonschema/compat/compatibility-test-data.json b/data-models/src/test/resources/io/apitomy/datamodels/jsonschema/compat/compatibility-test-data.json index a91f12f80..a1c228b22 100644 --- a/data-models/src/test/resources/io/apitomy/datamodels/jsonschema/compat/compatibility-test-data.json +++ b/data-models/src/test/resources/io/apitomy/datamodels/jsonschema/compat/compatibility-test-data.json @@ -3827,6 +3827,176 @@ "backward": { "compatible": true }, "forward": { "compatible": true } } + }, + { + "enabled": true, + "id": "Modern: $defs basic compatibility", + "compatibility": "both", + "original": { + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "object", + "$defs": { + "Address": { + "type": "object", + "properties": { "street": { "type": "string" } } + } + }, + "properties": { "home": { "$ref": "#/$defs/Address" } } + }, + "updated": { + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "object", + "$defs": { + "Address": { + "type": "object", + "properties": { "street": { "type": "string" } } + } + }, + "properties": { "home": { "$ref": "#/$defs/Address" } } + } + }, + { + "enabled": true, + "id": "Modern: dependentSchemas add dependency", + "compatibility": "backward", + "original": { + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "object", + "properties": { "name": { "type": "string" }, "address": { "type": "string" } }, + "dependentSchemas": { + "address": { + "properties": { "city": { "type": "string" } } + } + } + }, + "updated": { + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "object", + "properties": { "name": { "type": "string" }, "address": { "type": "string" } } + } + }, + { + "enabled": true, + "id": "Modern: dependentRequired add member", + "compatibility": "backward", + "original": { + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "object", + "properties": { "name": { "type": "string" }, "address": { "type": "string" }, "city": { "type": "string" } }, + "dependentRequired": { + "address": ["city"] + } + }, + "updated": { + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "object", + "properties": { "name": { "type": "string" }, "address": { "type": "string" }, "city": { "type": "string" } } + } + }, + { + "enabled": true, + "id": "Modern: dependentRequired added is not backward compatible", + "original": { + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "object", + "properties": { "name": { "type": "string" }, "address": { "type": "string" } }, + "required": ["name"] + }, + "updated": { + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "object", + "properties": { "name": { "type": "string" }, "address": { "type": "string" } }, + "required": ["name"], + "dependentRequired": { + "address": ["name"] + } + }, + "expected": { + "backward": { "compatible": false }, + "forward": { "compatible": true } + } + }, + { + "enabled": true, + "id": "Cross-version: d7 dependencies vs 2019-09 dependentSchemas", + "config": { "allowCrossVersionChecking": true }, + "compatibility": "both", + "original": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { "name": { "type": "string" }, "address": { "type": "string" } }, + "dependencies": { + "address": { + "properties": { "city": { "type": "string" } } + } + } + }, + "updated": { + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "object", + "properties": { "name": { "type": "string" }, "address": { "type": "string" } }, + "dependentSchemas": { + "address": { + "properties": { "city": { "type": "string" } } + } + } + } + }, + { + "enabled": true, + "id": "Cross-version: d7 dependencies (string-array) vs 2019-09 dependentRequired", + "config": { "allowCrossVersionChecking": true }, + "compatibility": "both", + "original": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { "name": { "type": "string" }, "address": { "type": "string" }, "city": { "type": "string" } }, + "dependencies": { + "address": ["city"] + } + }, + "updated": { + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "object", + "properties": { "name": { "type": "string" }, "address": { "type": "string" }, "city": { "type": "string" } }, + "dependentRequired": { + "address": ["city"] + } + } + }, + { + "enabled": true, + "id": "Modern: 2019-09 basic schema compatibility", + "compatibility": "both", + "original": { + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "object", + "properties": { "name": { "type": "string" }, "age": { "type": "integer" } }, + "required": ["name"] + }, + "updated": { + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "object", + "properties": { "name": { "type": "string" }, "age": { "type": "integer" } }, + "required": ["name"] + } + }, + { + "enabled": true, + "id": "Modern: 2020-12 basic schema compatibility", + "compatibility": "both", + "original": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { "name": { "type": "string" } }, + "required": ["name"] + }, + "updated": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { "name": { "type": "string" } }, + "required": ["name"] + } } ] }