From 5e698c62a19d65975e3807a1c58551c9a2d92640 Mon Sep 17 00:00:00 2001 From: Vladimir Tsirushkin Date: Sun, 15 Dec 2019 17:13:02 -0800 Subject: [PATCH] Fix ResourceTypeSchema for schemas with oneOf and conditionals (#70) --- .../resource/ResourceTypeSchema.java | 33 +++++++++---------- .../cloudformation/resource/Validator.java | 6 ++-- .../resource/ResourceTypeSchemaTest.java | 7 ++++ .../resource/ValidatorRefResolutionTests.java | 4 +-- src/test/resources/valid-with-oneof.json | 21 ++++++++++++ 5 files changed, 48 insertions(+), 23 deletions(-) create mode 100644 src/test/resources/valid-with-oneof.json diff --git a/src/main/java/software/amazon/cloudformation/resource/ResourceTypeSchema.java b/src/main/java/software/amazon/cloudformation/resource/ResourceTypeSchema.java index fb7c53f..5ffda79 100644 --- a/src/main/java/software/amazon/cloudformation/resource/ResourceTypeSchema.java +++ b/src/main/java/software/amazon/cloudformation/resource/ResourceTypeSchema.java @@ -24,15 +24,16 @@ import lombok.Getter; import org.everit.json.schema.JSONPointer; -import org.everit.json.schema.ObjectSchema; -import org.everit.json.schema.loader.SchemaLoader; +import org.everit.json.schema.Schema; import org.json.JSONObject; import org.json.JSONTokener; import software.amazon.cloudformation.resource.exceptions.ValidationException; @Getter -public class ResourceTypeSchema extends ObjectSchema { +public class ResourceTypeSchema { + + private static final Validator VALIDATOR = new Validator(); private final Map unprocessedProperties = new HashMap<>(); @@ -45,11 +46,12 @@ public class ResourceTypeSchema extends ObjectSchema { private final List> additionalIdentifiers = new ArrayList<>(); private final List readOnlyProperties = new ArrayList<>(); private final List writeOnlyProperties = new ArrayList<>(); + private final Schema schema; - public ResourceTypeSchema(final ObjectSchema.Builder builder) { - super(builder); + public ResourceTypeSchema(Schema schema) { - super.getUnprocessedProperties().forEach(this.unprocessedProperties::put); + this.schema = schema; + schema.getUnprocessedProperties().forEach(this.unprocessedProperties::put); this.sourceUrl = this.unprocessedProperties.containsKey("sourceUrl") ? this.unprocessedProperties.get("sourceUrl").toString() @@ -100,19 +102,14 @@ public ResourceTypeSchema(final ObjectSchema.Builder builder) { }); } - public static ResourceTypeSchema load(final JSONObject schemaJson) { - // first validate incoming resource schema against definition schema - Validator.builder().build().validateObject(schemaJson, new JSONObject(new JSONTokener(ResourceTypeSchema.class - .getResourceAsStream(SchemaValidator.DEFINITION_SCHEMA_PATH)))); - - // now extract identifiers from resource schema - final SchemaLoader loader = SchemaLoader.builder().schemaJson(schemaJson) - // registers the local schema with the draft-07 url - .draftV7Support().build(); + public static ResourceTypeSchema load(final JSONObject resourceDefinition) { - final ObjectSchema.Builder builder = (ObjectSchema.Builder) loader.load(); + Schema schema = VALIDATOR.loadResourceDefinitionSchema(resourceDefinition); + return new ResourceTypeSchema(schema); + } - return new ResourceTypeSchema(builder); + public String getDescription() { + return schema.getDescription(); } public List getCreateOnlyPropertiesAsStrings() throws ValidationException { @@ -141,8 +138,8 @@ public List getWriteOnlyPropertiesAsStrings() throws ValidationException return this.writeOnlyProperties.stream().map(JSONPointer::toString).collect(Collectors.toList()); } - @Override public Map getUnprocessedProperties() { return Collections.unmodifiableMap(this.unprocessedProperties); } + } diff --git a/src/main/java/software/amazon/cloudformation/resource/Validator.java b/src/main/java/software/amazon/cloudformation/resource/Validator.java index 3ee761d..3afb4a7 100644 --- a/src/main/java/software/amazon/cloudformation/resource/Validator.java +++ b/src/main/java/software/amazon/cloudformation/resource/Validator.java @@ -89,16 +89,16 @@ public void validateObject(final JSONObject modelObject, final JSONObject defini * @throws ValidationException Thrown for any schema validation errors */ public void validateResourceDefinition(final JSONObject definition) throws ValidationException { - validateObject(definition, definitionSchemaJsonObject); // validateObject cannot validate schema-specific attributes. For example if definition // contains "propertyA": { "$ref":"./some-non-existent-location.json#definitions/PropertyX"} // validateObject will succeed, because all it cares about is that "$ref" is a URI // In order to validate that $ref points at an existing location in an existing document // we have to "load" the schema - loadResourceSchema(definition); + loadResourceDefinitionSchema(definition); } - public Schema loadResourceSchema(final JSONObject resourceDefinition) { + public Schema loadResourceDefinitionSchema(final JSONObject resourceDefinition) { + validateObject(resourceDefinition, definitionSchemaJsonObject); return getResourceSchemaBuilder(resourceDefinition).build(); } diff --git a/src/test/java/software/amazon/cloudformation/resource/ResourceTypeSchemaTest.java b/src/test/java/software/amazon/cloudformation/resource/ResourceTypeSchemaTest.java index 1575b92..bb4cd33 100644 --- a/src/test/java/software/amazon/cloudformation/resource/ResourceTypeSchemaTest.java +++ b/src/test/java/software/amazon/cloudformation/resource/ResourceTypeSchemaTest.java @@ -16,6 +16,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static software.amazon.cloudformation.resource.ValidatorTest.loadJSON; import java.util.List; @@ -131,4 +132,10 @@ public void minimalSchema_hasNoSemantics() { assertThat(schema.getReadOnlyPropertiesAsStrings()).isEmpty(); assertThat(schema.getWriteOnlyPropertiesAsStrings()).isEmpty(); } + + @Test + public void validSchema_withOneOf_shouldSucceed() { + JSONObject resource = loadJSON("/valid-with-oneof.json"); + final ResourceTypeSchema schema = ResourceTypeSchema.load(resource); + } } diff --git a/src/test/java/software/amazon/cloudformation/resource/ValidatorRefResolutionTests.java b/src/test/java/software/amazon/cloudformation/resource/ValidatorRefResolutionTests.java index 9a8147f..1822297 100644 --- a/src/test/java/software/amazon/cloudformation/resource/ValidatorRefResolutionTests.java +++ b/src/test/java/software/amazon/cloudformation/resource/ValidatorRefResolutionTests.java @@ -86,7 +86,7 @@ public void loadResourceSchema_invalidRelativeRef_shouldThrow() { public void validateTemplateAgainstResourceSchema_valid_shouldSucceed() { JSONObject resourceDefinition = loadJSON(RESOURCE_DEFINITION_PATH); - Schema schema = validator.loadResourceSchema(resourceDefinition); + Schema schema = validator.loadResourceDefinitionSchema(resourceDefinition); schema.validate(getSampleTemplate()); } @@ -98,7 +98,7 @@ public void validateTemplateAgainstResourceSchema_valid_shouldSucceed() { @Test public void validateTemplateAgainsResourceSchema_invalid_shoudThrow() { JSONObject resourceDefinition = loadJSON(RESOURCE_DEFINITION_PATH); - Schema schema = validator.loadResourceSchema(resourceDefinition); + Schema schema = validator.loadResourceDefinitionSchema(resourceDefinition); final JSONObject template = getSampleTemplate(); template.put("propertyB", "not.an.IP.address"); diff --git a/src/test/resources/valid-with-oneof.json b/src/test/resources/valid-with-oneof.json new file mode 100644 index 0000000..5e8c8b3 --- /dev/null +++ b/src/test/resources/valid-with-oneof.json @@ -0,0 +1,21 @@ +{ + "typeName": "AWS::Test::TestModel", + "description": "A test schema for unit tests.", + "sourceUrl": "https://mycorp.com/my-repo.git", + "properties": { + "propertyA": { + "type": "string" + }, + "propertyB": { + "type": "string" + } + }, + "oneOf": [ + { "required": ["propertyA"]}, + { "required" : ["propertyB"]} + ], + "primaryIdentifier": [ + "/properties/propertyA" + ], + "additionalProperties": false +}