Skip to content

Commit

Permalink
Fix ResourceTypeSchema for schemas with oneOf and conditionals (aws-c…
Browse files Browse the repository at this point in the history
  • Loading branch information
vladtsir committed Dec 16, 2019
1 parent b18ed33 commit 19d0aca
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@
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<String, Object> unprocessedProperties = new HashMap<>();

Expand All @@ -45,11 +45,12 @@ public class ResourceTypeSchema extends ObjectSchema {
private final List<List<JSONPointer>> additionalIdentifiers = new ArrayList<>();
private final List<JSONPointer> readOnlyProperties = new ArrayList<>();
private final List<JSONPointer> 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()
Expand Down Expand Up @@ -100,19 +101,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<String> getCreateOnlyPropertiesAsStrings() throws ValidationException {
Expand Down Expand Up @@ -141,8 +137,8 @@ public List<String> getWriteOnlyPropertiesAsStrings() throws ValidationException
return this.writeOnlyProperties.stream().map(JSONPointer::toString).collect(Collectors.toList());
}

@Override
public Map<String, Object> getUnprocessedProperties() {
return Collections.unmodifiableMap(this.unprocessedProperties);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand All @@ -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");
Expand Down
21 changes: 21 additions & 0 deletions src/test/resources/valid-with-oneof.json
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 19d0aca

Please sign in to comment.