fix(apigateway): invalid schema generated due to un-mapped ref (#3258)

* fix(api-gateway): invalid schema generated due to `ref`

When using a `ref` (which in JSON Schema really is `$ref$`) for the type
of some object property, the `$ref` replacement did not happen because
the arbitrary property names were not white-listed for transformation.

* fixed lint violations

packages/@aws-cdk/aws-apigateway/lib/json-schema.ts

@@ -65,7 +65,7 @@ export interface JsonSchema {
   readonly properties?: { [name: string]: JsonSchema };
   readonly additionalProperties?: JsonSchema;
   readonly patternProperties?: { [name: string]: JsonSchema };
-  readonly dependencies?: { [name: string]: JsonSchema | string[] };
+  readonly dependencies?: { [name: string]: JsonSchema | string[] };
   readonly propertyNames?: JsonSchema;
 
   // Conditional

packages/@aws-cdk/aws-apigateway/lib/util.ts

@@ -95,36 +95,27 @@ export class JsonSchemaMapper {
     ref: '$ref',
     id: '$id'
   };
-  private static readonly SubSchemaProps: { [key: string]: boolean } = {
+  // The value indicates whether direct children should be key-mapped.
+  private static readonly SchemaPropsWithUserDefinedChildren: { [key: string]: boolean } = {
     definitions: true,
-    items: true,
-    additionalItems: true,
-    contains: true,
     properties: true,
-    additionalProperties: true,
     patternProperties: true,
     dependencies: true,
-    propertyNames: true
   };
 
-  private static _toCfnJsonSchema(schema: any): any {
-    if (schema === null || schema === undefined) {
-      return schema;
-    }
-    if ((typeof(schema) === "string") || (typeof(schema) === "boolean") || (typeof(schema) === "number")) {
+  private static _toCfnJsonSchema(schema: any, preserveKeys = false): any {
+    if (schema == null || typeof schema !== 'object') {
       return schema;
     }
     if (Array.isArray(schema)) {
-      return schema.map((entry) => JsonSchemaMapper._toCfnJsonSchema(entry));
-    }
-    if (typeof(schema) === "object") {
-      return Object.assign({}, ...Object.entries(schema).map((entry) => {
-        const key = entry[0];
-        const newKey = (key in JsonSchemaMapper.SchemaPropsWithPrefix) ? JsonSchemaMapper.SchemaPropsWithPrefix[key] : key;
-        const value = (key in JsonSchemaMapper.SubSchemaProps) ? JsonSchemaMapper._toCfnJsonSchema(entry[1]) : entry[1];
-        return { [newKey]: value };
-      }));
+      return schema.map(entry => JsonSchemaMapper._toCfnJsonSchema(entry));
     }
-    return schema;
+    return Object.assign({}, ...Object.entries(schema).map(([key, value]) => {
+      const mapKey = !preserveKeys && (key in JsonSchemaMapper.SchemaPropsWithPrefix);
+      const newKey = mapKey ? JsonSchemaMapper.SchemaPropsWithPrefix[key] : key;
+      // If keys were preserved, don't consider SchemaPropsWithUserDefinedChildren for those keys (they are user-defined!)
+      const newValue = JsonSchemaMapper._toCfnJsonSchema(value, !preserveKeys && JsonSchemaMapper.SchemaPropsWithUserDefinedChildren[key]);
+      return { [newKey]: newValue };
+    }));
   }
 }

packages/@aws-cdk/aws-apigateway/test/test.util.ts

@@ -1,8 +1,9 @@
-import { Test } from 'nodeunit';
-import { parseAwsApiCall, parseMethodOptionsPath } from '../lib/util';
+import { Test, testCase } from 'nodeunit';
+import { JsonSchema, JsonSchemaType } from '../lib';
+import { JsonSchemaMapper, parseAwsApiCall, parseMethodOptionsPath } from '../lib/util';
 
-export = {
-  parseMethodResourcePath: {
+export = testCase({
+  'parseMethodResourcePath': {
     'fails if path does not start with a /'(test: Test) {
       test.throws(() => parseMethodOptionsPath('foo'), /Method options path must start with \'\/\'/);
       test.done();
@@ -31,7 +32,7 @@ export = {
     }
   },
 
-  parseAwsApiCall: {
+  'parseAwsApiCall': {
     'fails if "actionParams" is set but "action" is undefined'(test: Test) {
       test.throws(() => parseAwsApiCall(undefined, undefined, { foo: '123' }), /"actionParams" requires that "action" will be set/);
       test.done();
@@ -64,5 +65,73 @@ export = {
       });
       test.done();
     }
-  }
-};
+  },
+
+  'JsonSchemaMapper.toCfnJsonSchema': {
+    'maps "ref" found under properties'(test: Test) {
+      const schema: JsonSchema = {
+        type: JsonSchemaType.OBJECT,
+        properties: {
+          collection: {
+            type: JsonSchemaType.ARRAY,
+            items: {
+              ref: '#/some/reference',
+            },
+            uniqueItems: true,
+          },
+        },
+        required: ['collection'],
+      };
+
+      const actual = JsonSchemaMapper.toCfnJsonSchema(schema);
+      test.deepEqual(actual, {
+        $schema: 'http://json-schema.org/draft-04/schema#',
+        type: 'object',
+        properties: {
+          collection: {
+            type: 'array',
+            items: {
+              $ref: '#/some/reference',
+            },
+            uniqueItems: true,
+          },
+        },
+        required: ['collection'],
+      });
+      test.done();
+    },
+
+    'does not map a "ref" property name'(test: Test) {
+      const schema: JsonSchema = {
+        type: JsonSchemaType.OBJECT,
+        properties: {
+          ref: {
+            type: JsonSchemaType.ARRAY,
+            items: {
+              ref: '#/some/reference',
+            },
+            uniqueItems: true,
+          },
+        },
+        required: ['ref'],
+      };
+
+      const actual = JsonSchemaMapper.toCfnJsonSchema(schema);
+      test.deepEqual(actual, {
+        $schema: 'http://json-schema.org/draft-04/schema#',
+        type: 'object',
+        properties: {
+          ref: {
+            type: 'array',
+            items: {
+              $ref: '#/some/reference',
+            },
+            uniqueItems: true,
+          },
+        },
+        required: ['ref'],
+      });
+      test.done();
+    },
+  },
+});