feat: update model validator to throw on names and component types wi…

…th whitespace
aws-amplify · Nov 12, 2021 · 760a826 · 760a826
1 parent 049e003
commit 760a826
Show file tree

Hide file tree

Showing 7 changed files with 147 additions and 13 deletions.
diff --git a/packages/studio-ui-codegen/lib/__tests__/__snapshots__/validation-helper.test.ts.snap b/packages/studio-ui-codegen/lib/__tests__/__snapshots__/validation-helper.test.ts.snap
@@ -4,13 +4,17 @@ exports[`validation-helper validateComponentSchema child component requires comp
 
 exports[`validation-helper validateComponentSchema deeply nested child components requires properties 1`] = `"children[0].children[0].children[0].properties is a required field"`;
 
+exports[`validation-helper validateComponentSchema fails on componentType with leading number 1`] = `"Expected an alphanumeric string, starting with a character"`;
+
+exports[`validation-helper validateComponentSchema fails on componentType with whitespace 1`] = `"Expected an alphanumeric string, starting with a character"`;
+
 exports[`validation-helper validateComponentSchema top-level component requires componentType 1`] = `"componentType is a required field"`;
 
 exports[`validation-helper validateComponentSchema top-level component requires componentType to be the correct type 1`] = `"componentType must be a \`string\` type, but the final value was: \`2\`."`;
 
 exports[`validation-helper validateComponentSchema top-level component requires properties 1`] = `"properties is a required field"`;
 
-exports[`validation-helper validateThemeSchema children objects should not be empty 1`] = `"values[1].key is a required field"`;
+exports[`validation-helper validateThemeSchema children objects should not be empty 1`] = `"values[1].key is a required field, values[1].value is a required field"`;
 
 exports[`validation-helper validateThemeSchema overrides should contain the right shape 1`] = `"overrides[0].key is a required field"`;
 

diff --git a/packages/studio-ui-codegen/lib/__tests__/validation-helper.test.ts b/packages/studio-ui-codegen/lib/__tests__/validation-helper.test.ts
@@ -95,6 +95,49 @@ describe('validation-helper', () => {
         });
       }).toThrowErrorMatchingSnapshot();
     });
+
+    test('fails on componentType with whitespace', () => {
+      expect(() => {
+        validateComponentSchema({
+          name: 'CustomComponent',
+          componentType: 'View 2',
+          properties: {},
+        });
+      }).toThrowErrorMatchingSnapshot();
+    });
+
+    test('fails on componentType with leading number', () => {
+      expect(() => {
+        validateComponentSchema({
+          name: 'CustomComponent',
+          componentType: '2View',
+          properties: {},
+        });
+      }).toThrowErrorMatchingSnapshot();
+    });
+
+    test('succeeds on componentType with trailing number', () => {
+      validateComponentSchema({
+        name: 'CustomComponent',
+        componentType: 'View2',
+        properties: {},
+      });
+    });
+
+    test('child component name may contain whitespace', () => {
+      validateComponentSchema({
+        name: 'CustomComponent',
+        componentType: 'View',
+        properties: {},
+        children: [
+          {
+            name: 'I Have Spaces',
+            componentType: 'Button',
+            properties: {},
+          },
+        ],
+      });
+    });
   });
 
   describe('validateThemeSchema', () => {

diff --git a/packages/studio-ui-codegen/lib/validation-helper.ts b/packages/studio-ui-codegen/lib/validation-helper.ts
@@ -16,13 +16,23 @@
 import * as yup from 'yup';
 import { InvalidInputError } from './errors';
 
+const alphaNumString = () => {
+  return yup.string().matches(/^[a-zA-Z0-9]*$/, { message: 'Expected an alphanumeric string' });
+};
+
+const alphaNumNoLeadingNumberString = () => {
+  return yup
+    .string()
+    .matches(/^[a-zA-Z][a-zA-Z0-9]*$/, { message: 'Expected an alphanumeric string, starting with a character' });
+};
+
 /**
  * Component Schema Definitions
  */
 const studioComponentChildSchema: any = yup.object({
-  componentType: yup.string().strict().required(),
+  componentType: alphaNumNoLeadingNumberString().required(),
   // TODO: Name is required in the studio-types file, but doesn't seem to need to be. Relaxing the restriction here.
-  name: yup.string().strict(),
+  name: yup.string(),
   properties: yup.object().required(),
   // Doing lazy eval here since we reference our own type otherwise
   children: yup.lazy(() => yup.array(studioComponentChildSchema.default(undefined))),
@@ -35,10 +45,10 @@ const studioComponentChildSchema: any = yup.object({
 });
 
 const studioComponentSchema = yup.object({
-  name: yup.string().strict(),
-  id: yup.string().strict(),
-  sourceId: yup.string().strict(),
-  componentType: yup.string().strict().required(),
+  name: alphaNumString(),
+  id: yup.string(),
+  sourceId: yup.string(),
+  componentType: alphaNumNoLeadingNumberString().required(),
   properties: yup.object().required(),
   children: yup.array(studioComponentChildSchema),
   figmaMetadata: yup.object(),
@@ -53,18 +63,18 @@ const studioComponentSchema = yup.object({
  * Theme Schema Definitions
  */
 const studioThemeValuesSchema: any = yup.object({
-  key: yup.string().strict().required(),
+  key: yup.string().required(),
   value: yup
     .object({
-      value: yup.string().strict(),
+      value: yup.string(),
       children: yup.lazy(() => yup.array(studioThemeValuesSchema.default(undefined))),
     })
     .required(),
 });
 
 const studioThemeSchema = yup.object({
-  name: yup.string().strict().required(),
-  id: yup.string().strict(),
+  name: alphaNumString().required(),
+  id: yup.string(),
   values: yup.array(studioThemeValuesSchema).required(),
   overrides: yup.array(studioThemeValuesSchema),
 });
@@ -74,7 +84,7 @@ const studioThemeSchema = yup.object({
  */
 const validateSchema = (validator: yup.AnySchema, studioSchema: any) => {
   try {
-    validator.validateSync(studioSchema);
+    validator.validateSync(studioSchema, { strict: true, abortEarly: false });
   } catch (e) {
     if (e instanceof yup.ValidationError) {
       throw new InvalidInputError(e.errors.join(', '));

diff --git a/packages/test-generator/integration-test-templates/cypress/integration/generate-spec.js b/packages/test-generator/integration-test-templates/cypress/integration/generate-spec.js
@@ -1,4 +1,9 @@
-const EXPECTED_INVALID_INPUT_CASES = new Set(['ComponentMissingProperties', 'ComponentMissingType', 'InvalidTheme']);
+const EXPECTED_INVALID_INPUT_CASES = new Set([
+  'ComponentMissingProperties',
+  'ComponentMissingType',
+  'InvalidTheme',
+  'CardWithInvalidChildComponentType',
+]);
 const EXPECTED_INTERNAL_ERROR_CASES = new Set([]);
 
 const TARGET_GENERATORS = ['ES2016_TSX', 'ES2016_JSX', 'ES5_TSX', 'ES5_JSX'];

diff --git a/packages/test-generator/lib/components/custom/cardWithInvalidChildComponentType.json b/packages/test-generator/lib/components/custom/cardWithInvalidChildComponentType.json
@@ -0,0 +1,70 @@
+{
+  "name": "CardWithInvalidChildComponentType",
+  "children": [
+    {
+      "children": [
+        {
+          "children": [],
+          "name": "Frame 2",
+          "componentType": "Frame 2",
+          "properties": {
+            "width": {
+              "value": "323px"
+            },
+            "height": {
+              "value": "100px"
+            },
+            "alignItems": {
+              "value": "center"
+            }
+          },
+          "overrides": {},
+          "variants": []
+        }
+      ],
+      "name": "Card",
+      "componentType": "Card",
+      "properties": {
+        "style": {
+          "value": "plain"
+        }
+      },
+      "overrides": {},
+      "variants": []
+    }
+  ],
+  "id": "1736:4332",
+  "bindingProperties": {},
+  "componentType": "Flex",
+  "properties": {
+    "width": {
+      "value": "375px"
+    },
+    "height": {
+      "value": "152px"
+    },
+    "gap": {
+      "value": "10px"
+    },
+    "direction": {
+      "value": "row"
+    },
+    "alignItems": {
+      "value": "flex-start"
+    },
+    "overflow": {
+      "value": "hidden"
+    },
+    "position": {
+      "value": "relative"
+    },
+    "padding": {
+      "value": "10px 10px 10px 10px"
+    },
+    "backgroundColor": {
+      "value": "rgba(255,255,255,1)"
+    }
+  },
+  "overrides": {},
+  "variants": []
+}
diff --git a/packages/test-generator/lib/components/custom/index.ts b/packages/test-generator/lib/components/custom/index.ts
@@ -16,3 +16,4 @@
 export { default as CustomParent } from './customParent.json';
 export { default as CustomChildren } from './customChildren.json';
 export { default as CustomParentAndChildren } from './customParentAndChildren.json';
+export { default as CardWithInvalidChildComponentType } from './cardWithInvalidChildComponentType.json';
diff --git a/packages/test-generator/lib/generators/GenerateTestApp.ts b/packages/test-generator/lib/generators/GenerateTestApp.ts
@@ -29,6 +29,7 @@ const DISABLED_SCHEMAS = [
   'ComponentMissingProperties', // Expected failure cases
   'ComponentMissingType', // Expected failure cases
   'InvalidTheme', // Expected failure cases
+  'CardWithInvalidChildComponentType', // Expected failure cases
 ];
 
 const generator = new NodeTestGenerator({