Require name in defineStorage props (#989)

Author: edwardfoyle

.changeset/selfish-eels-jam.md

@@ -0,0 +1,5 @@
+---
+'@aws-amplify/backend-storage': minor
+---
+
+Require name in defineStorage props

packages/backend-storage/API.md

@@ -15,6 +15,7 @@ export type AmplifyStorageFactoryProps = Omit<AmplifyStorageProps, 'outputStorag
 
 // @public (undocumented)
 export type AmplifyStorageProps = {
+    name: string;
     versioned?: boolean;
     outputStorageStrategy?: BackendOutputStorageStrategy<StorageOutput>;
 };

packages/backend-storage/src/construct.test.ts

@@ -14,15 +14,15 @@ void describe('AmplifyStorage', () => {
   void it('creates a bucket', () => {
     const app = new App();
     const stack = new Stack(app);
-    new AmplifyStorage(stack, 'test', {});
+    new AmplifyStorage(stack, 'test', { name: 'testName' });
     const template = Template.fromStack(stack);
     template.resourceCountIs('AWS::S3::Bucket', 1);
   });
 
   void it('turns versioning on if specified', () => {
     const app = new App();
     const stack = new Stack(app);
-    new AmplifyStorage(stack, 'test', { versioned: true });
+    new AmplifyStorage(stack, 'test', { versioned: true, name: 'testName' });
     const template = Template.fromStack(stack);
     template.resourceCountIs('AWS::S3::Bucket', 1);
     template.hasResourceProperties('AWS::S3::Bucket', {
@@ -33,7 +33,7 @@ void describe('AmplifyStorage', () => {
   void it('stores attribution data in stack', () => {
     const app = new App();
     const stack = new Stack(app);
-    new AmplifyStorage(stack, 'testAuth', {});
+    new AmplifyStorage(stack, 'testAuth', { name: 'testName' });
 
     const template = Template.fromStack(stack);
     assert.equal(
@@ -54,11 +54,12 @@ void describe('AmplifyStorage', () => {
         };
 
       const storageConstruct = new AmplifyStorage(stack, 'test', {
+        name: 'testName',
         outputStorageStrategy: storageStrategy,
       });
 
       const expectedBucketName = (
-        storageConstruct.node.findChild('testBucket') as Bucket
+        storageConstruct.node.findChild('Bucket') as Bucket
       ).bucketName;
       const expectedRegion = Stack.of(storageConstruct).region;
 
@@ -80,7 +81,7 @@ void describe('AmplifyStorage', () => {
       const app = new App();
       const stack = new Stack(app);
 
-      new AmplifyStorage(stack, 'test', {});
+      new AmplifyStorage(stack, 'test', { name: 'testName' });
       const template = Template.fromStack(stack);
       template.templateMatches({
         Metadata: {

packages/backend-storage/src/construct.ts

@@ -19,6 +19,7 @@ import { fileURLToPath } from 'url';
 const storageStackType = 'storage-S3';
 
 export type AmplifyStorageProps = {
+  name: string;
   versioned?: boolean;
   outputStorageStrategy?: BackendOutputStorageStrategy<StorageOutput>;
 };
@@ -48,7 +49,7 @@ export class AmplifyStorage
     };
 
     this.resources = {
-      bucket: new Bucket(this, `${id}Bucket`, bucketProps),
+      bucket: new Bucket(this, 'Bucket', bucketProps),
     };
 
     this.storeOutput(props.outputStorageStrategy);

packages/backend-storage/src/factory.test.ts

@@ -37,7 +37,7 @@ void describe('AmplifyStorageFactory', () => {
   let getInstanceProps: ConstructFactoryGetInstanceProps;
 
   beforeEach(() => {
-    storageFactory = defineStorage({});
+    storageFactory = defineStorage({ name: 'testName' });
     const stack = createStackAndSetContext();
 
     constructContainer = new ConstructContainerStub(
@@ -110,4 +110,12 @@ void describe('AmplifyStorageFactory', () => {
       )
     );
   });
+
+  void it('throws on invalid name', () => {
+    const storageFactory = defineStorage({ name: '!$87++|' });
+    assert.throws(() => storageFactory.getInstance(getInstanceProps), {
+      message:
+        'defineStorage name can only contain alphanumeric characters, found !$87++|',
+    });
+  });
 });

packages/backend-storage/src/factory.ts

@@ -13,6 +13,7 @@ import {
   AmplifyStorageProps,
   StorageResources,
 } from './construct.js';
+import { AmplifyUserError } from '@aws-amplify/platform-core';
 
 export type AmplifyStorageFactoryProps = Omit<
   AmplifyStorageProps,
@@ -48,6 +49,7 @@ class AmplifyStorageFactory
       path.join('amplify', 'storage', 'resource'),
       'Amplify Storage must be defined in amplify/storage/resource.ts'
     );
+    this.validateName(this.props.name);
     if (!this.generator) {
       this.generator = new AmplifyStorageGenerator(
         this.props,
@@ -56,19 +58,29 @@ class AmplifyStorageFactory
     }
     return constructContainer.getOrCompute(this.generator) as AmplifyStorage;
   };
+
+  private validateName = (name: string): void => {
+    const nameIsAlphanumeric = /^[a-zA-Z0-9]+$/.test(name);
+    if (!nameIsAlphanumeric) {
+      throw new AmplifyUserError('InvalidResourceNameError', {
+        message: `defineStorage name can only contain alphanumeric characters, found ${name}`,
+        resolution:
+          'Change the name parameter of defineStorage to only use alphanumeric characters',
+      });
+    }
+  };
 }
 
 class AmplifyStorageGenerator implements ConstructContainerEntryGenerator {
   readonly resourceGroupName = 'storage';
-  private readonly defaultName = 'amplifyStorage';
 
   constructor(
     private readonly props: AmplifyStorageProps,
     private readonly outputStorageStrategy: BackendOutputStorageStrategy<BackendOutputEntry>
   ) {}
 
   generateContainerEntry = (scope: Construct) => {
-    return new AmplifyStorage(scope, this.defaultName, {
+    return new AmplifyStorage(scope, `${this.props.name}`, {
       ...this.props,
       outputStorageStrategy: this.outputStorageStrategy,
     });

packages/integration-tests/src/test-in-memory/integration_tests.test.ts

@@ -39,7 +39,7 @@ void it('data storage auth with triggers', () => {
 
   assertExpectedLogicalIds(templates.storage, 'AWS::S3::Bucket', [
     // eslint-disable-next-line spellcheck/spell-checker
-    'amplifyStorageamplifyStorageBucketC2F702CD',
+    'testNameBucketB4152AD5',
   ]);
 });
 

packages/integration-tests/src/test-projects/data-storage-auth-with-triggers-ts/amplify/storage/resource.ts

@@ -1,3 +1,3 @@
 import { defineStorage } from '@aws-amplify/backend';
 
-export const storage = defineStorage({});
+export const storage = defineStorage({name: 'testName'});

packages/integration-tests/src/test-projects/minimalist-project-with-typescript-idioms/amplify/storage/resource.ts

@@ -1,3 +1,3 @@
 import { defineStorage } from '@aws-amplify/backend';
 
-export const storage = defineStorage({});
+export const storage = defineStorage({name: 'testName'});

packages/platform-core/API.md

@@ -64,7 +64,7 @@ export class AmplifyUserError extends AmplifyError {
 }
 
 // @public
-export type AmplifyUserErrorType = 'InvalidPackageJsonError' | 'InvalidSchemaAuthError' | 'InvalidSchemaError' | 'ExpiredTokenError' | 'CloudFormationDeploymentError' | 'CFNUpdateNotSupportedError' | 'SyntaxError' | 'BackendBuildError' | 'BootstrapNotDetectedError' | 'AccessDeniedError' | 'FileConventionError';
+export type AmplifyUserErrorType = 'InvalidPackageJsonError' | 'InvalidSchemaAuthError' | 'InvalidSchemaError' | 'ExpiredTokenError' | 'CloudFormationDeploymentError' | 'CFNUpdateNotSupportedError' | 'SyntaxError' | 'BackendBuildError' | 'BootstrapNotDetectedError' | 'AccessDeniedError' | 'FileConventionError' | 'InvalidResourceNameError';
 
 // @public
 export class BackendIdentifierConversions {