Fix ResourceTypeSchema for schemas with oneOf and conditionals (aws-c…

…loudformation#70)

src/main/java/software/amazon/cloudformation/resource/ResourceTypeSchema.java

@@ -25,16 +25,16 @@
 
 import org.everit.json.schema.JSONPointer;
 import org.everit.json.schema.JSONPointerException;
-import org.everit.json.schema.ObjectSchema;
+import org.everit.json.schema.Schema;
 import org.everit.json.schema.PublicJSONPointer;
-import org.everit.json.schema.loader.SchemaLoader;
 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<>();
 
@@ -47,11 +47,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()
@@ -102,19 +103,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 {
@@ -143,7 +139,6 @@ 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);
     }

src/main/java/software/amazon/cloudformation/resource/Validator.java

@@ -24,6 +24,7 @@
 import org.everit.json.schema.loader.SchemaLoader;
 import org.everit.json.schema.loader.SchemaLoader.SchemaLoaderBuilder;
 import org.everit.json.schema.loader.internal.DefaultSchemaClient;
+import org.json.JSONException;
 import org.json.JSONObject;
 import org.json.JSONTokener;
 
@@ -50,7 +51,6 @@ public class Validator implements SchemaValidator {
      * against it
      */
     private final JSONObject jsonSchemaObject;
-
     /**
      * this is what SchemaLoader uses to download remote $refs. Not necessarily an
      * HTTP client, see the docs for details. We override the default SchemaClient
@@ -103,10 +103,9 @@ public void validateResourceDefinition(final JSONObject definition) throws Valid
         // 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);
     }
 
-    public Schema loadResourceSchema(final JSONObject resourceDefinition) {
+        validateObject(resourceDefinition, definitionSchemaJsonObject);
         return getResourceSchemaBuilder(resourceDefinition).build();
     }
 
@@ -120,7 +119,7 @@ public Schema loadResourceSchema(final JSONObject resourceDefinition) {
      */
     public Schema.Builder<?> getResourceSchemaBuilder(final JSONObject resourceDefinition) {
         final SchemaLoaderBuilder loaderBuilder = getSchemaLoader(resourceDefinition);
-        loaderBuilder.registerSchemaByURI(RESOURCE_DEFINITION_SCHEMA_URI, definitionSchemaJsonObject);
+        registerMetaSchema(loaderBuilder, definitionSchemaJsonObject);
 
         final SchemaLoader loader = loaderBuilder.build();
         try {
@@ -147,6 +146,7 @@ private SchemaLoaderBuilder getSchemaLoader(JSONObject schemaObject) {
         // registered twice because we've seen some confusion around this in the past
         builder.registerSchemaByURI(JSON_SCHEMA_URI_HTTP, jsonSchemaObject);
         builder.registerSchemaByURI(JSON_SCHEMA_URI_HTTPS, jsonSchemaObject);
+        registerMetaSchema(builder, jsonSchemaObject);
 
         return builder;
     }
@@ -163,4 +163,33 @@ static URI newURI(final String uri) {
             throw new RuntimeException(uri);
         }
     }
+    /**
+     * Register a meta-schema with the SchemaLoaderBuilder. The meta-schema $id is used to generate schema URI
+     * This has the effect of caching the meta-schema. When SchemaLoaderBuilder is used to build the Schema object,
+     * the cached version will be used. No calls to remote URLs will be made.
+     * Validator caches JSON schema (/resources/schema) and Resource Definition Schema
+     * (/resources/provider.definition.schema.v1.json)
+     *
+     * @param loaderBuilder
+     * @param schema meta-schema JSONObject to be cached. Must have a valid $id property
+     */
+    void registerMetaSchema(final SchemaLoaderBuilder loaderBuilder, JSONObject schema) {
+        try {
+            String id = schema.getString(ID_KEY);
+            if (id.isEmpty()) {
+                throw new ValidationException("Invalid $id value", "$id", "[empty string]");
+            }
+            final URI uri = new URI(id);
+            loaderBuilder.registerSchemaByURI(uri, schema);
+        } catch (URISyntaxException e) {
+            throw new ValidationException("Invalid $id value", "$id", e);
+        } catch (JSONException e) {
+            // $id is missing or not a string
+            throw new ValidationException("Invalid $id value", "$id", e);
+        }
+    }
+
+    private static JSONObject loadResourceAsJSON(String path) {
+        return new JSONObject(new JSONTokener(Validator.class.getResourceAsStream(path)));
+    }
 }

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;
 
@@ -151,4 +152,10 @@ public void removeWriteOnlyProperties_hasWriteOnlyProperties_shouldRemove() {
         // ensure that other non writeOnlyProperty is not removed
         assertThat(resourceModel.has("propertyB")).isTrue();
     }
+
+    @Test
+    public void validSchema_withOneOf_shouldSucceed() {
+        JSONObject resource = loadJSON("/valid-with-oneof.json");
+        final ResourceTypeSchema schema = ResourceTypeSchema.load(resource);
+    }
 }

src/test/java/software/amazon/cloudformation/resource/ValidatorRefResolutionTests.java

@@ -27,10 +27,15 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.junit.jupiter.MockitoExtension;
+import org.mockito.verification.VerificationMode;
 
 import software.amazon.cloudformation.resource.exceptions.ValidationException;
 
+/**
+ *
+ */
 @ExtendWith(MockitoExtension.class)
 public class ValidatorRefResolutionTests {
 
@@ -111,4 +116,9 @@ public void validateTemplateAgainsResourceSchema_invalid_shoudThrow() {
     private JSONObject getSampleTemplate() {
         return new JSONObject().put("Time", "2019-12-12T10:10:22.212Z");
     }
+
+    private static VerificationMode twice() {
+        return Mockito.times(2);
+    }
+
 }

src/test/java/software/amazon/cloudformation/resource/ValidatorTest.java

@@ -24,6 +24,7 @@
 import java.util.Collections;
 import java.util.List;
 
+import org.everit.json.schema.loader.SchemaLoader;
 import org.json.JSONObject;
 import org.json.JSONTokener;
 import org.junit.jupiter.api.BeforeEach;
@@ -35,6 +36,7 @@
 import software.amazon.cloudformation.resource.exceptions.ValidationException;
 
 public class ValidatorTest {
+    private static final String RESOURCE_DEFINITION_SCHEMA_PATH = "/schema/provider.definition.schema.v1.json";
     private static final String TEST_SCHEMA_PATH = "/test-schema.json";
     private static final String TEST_VALUE_SCHEMA_PATH = "/scrubbed-values-schema.json";
     private static final String TYPE_NAME_KEY = "typeName";
@@ -480,6 +482,32 @@ public void validateExample_exampleResource_shouldBeValid() throws IOException {
         }
     }
 
+    /**
+     * trivial coverage test: cannot cache a schema if it has an invalid $id
+     */
+    @ParameterizedTest
+    @ValueSource(strings = { ":invalid/uri", "" })
+    public void registerMetaSchema_invalidRelativeRef_shouldThrow(String uri) {
+
+        JSONObject badSchema = loadJSON(RESOURCE_DEFINITION_SCHEMA_PATH);
+        badSchema.put("$id", uri);
+        assertThatExceptionOfType(ValidationException.class).isThrownBy(() -> {
+            validator.registerMetaSchema(SchemaLoader.builder(), badSchema);
+        });
+    }
+
+    /**
+     * trivial coverage test: cannot cache a schema if it has no $id
+     */
+    @Test
+    public void registerMetaSchema_nullId_shouldThrow() {
+        JSONObject badSchema = loadJSON(RESOURCE_DEFINITION_SCHEMA_PATH);
+        badSchema.remove("$id");
+        assertThatExceptionOfType(ValidationException.class).isThrownBy(() -> {
+            validator.registerMetaSchema(SchemaLoader.builder(), badSchema);
+        });
+    }
+
     static JSONObject loadJSON(String path) {
         try {
             return new JSONObject(new JSONTokener(ValidatorTest.getResourceAsStream(path)));

src/test/resources/valid-with-oneof.json

@@ -0,0 +1,29 @@
+{
+    "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
+}