fix(graphql-transformer-common): improve generated graphql pluralization

This commit adds the `pluralize` npm library instead of blindly appending an "s" to acheive pluralization. For example, a model named "Match" should pluralize to "Matches", but before this commit Amplify would pluralize it to "Matchs" fix #4224
aws-amplify · May 21, 2021 · 0843e8d · 0843e8d
1 parent 3dbb3bf
commit 0843e8d
Show file tree

Hide file tree

Showing 36 changed files with 543 additions and 81 deletions.
diff --git a/packages/amplify-category-api/src/commands/api/add-graphql-datasource.js b/packages/amplify-category-api/src/commands/api/add-graphql-datasource.js
@@ -5,8 +5,7 @@ const path = require('path');
 const { RelationalDBSchemaTransformer } = require('graphql-relational-schema-transformer');
 const { RelationalDBTemplateGenerator, AuroraServerlessMySQLDatabaseReader } = require('graphql-relational-schema-transformer');
 const { mergeTypeDefs } = require('@graphql-tools/merge');
-const { ResourceDoesNotExistError, exitOnNextTick } = require('amplify-cli-core');
-
+const { FeatureFlags, ResourceDoesNotExistError, exitOnNextTick } = require('amplify-cli-core');
 const subcommand = 'add-graphql-datasource';
 const categories = 'categories';
 const category = 'api';
@@ -143,7 +142,11 @@ module.exports = {
         context[rdsResourceName] = resourceName;
         context[rdsDatasource] = datasource;
         let template = templateGenerator.createTemplate(context);
-        template = templateGenerator.addRelationalResolvers(template, resolversDir);
+        template = templateGenerator.addRelationalResolvers(
+          template,
+          resolversDir,
+          FeatureFlags.getBoolean('graphqltransformer.improvePluralization'),
+        );
         const cfn = templateGenerator.printCloudformationTemplate(template);
 
         /**

diff --git a/packages/amplify-cli-core/src/feature-flags/featureFlags.ts b/packages/amplify-cli-core/src/feature-flags/featureFlags.ts
@@ -515,6 +515,12 @@ export class FeatureFlags {
         defaultValueForExistingProjects: false,
         defaultValueForNewProjects: true,
       },
+      {
+        name: 'improvePluralization',
+        type: 'boolean',
+        defaultValueForExistingProjects: false,
+        defaultValueForNewProjects: true,
+      },
       {
         name: 'validateTypeNameReservedWords',
         type: 'boolean',
@@ -619,7 +625,7 @@ export class FeatureFlags {
         type: 'boolean',
         defaultValueForExistingProjects: false,
         defaultValueForNewProjects: true,
-      }
+      },
     ]);
 
     this.registerFlag('appSync', [

diff --git a/...phql-searchable-transformer/src/__tests__/amplify-graphql-searchable-transformer.tests.ts b/...phql-searchable-transformer/src/__tests__/amplify-graphql-searchable-transformer.tests.ts
@@ -3,6 +3,17 @@ import { SearchableModelTransformer } from '../';
 import { ModelTransformer } from '@aws-amplify/graphql-model-transformer';
 import { anything, countResources, expect as cdkExpect, haveResource } from '@aws-cdk/assert';
 import { parse } from 'graphql';
+const featureFlags = {
+  getBoolean: jest.fn().mockImplementation((name, defaultValue) => {
+    if (name === 'improvePluralization') {
+      return true;
+    }
+    return;
+  }),
+  getNumber: jest.fn(),
+  getObject: jest.fn(),
+  getString: jest.fn(),
+};
 
 test('Test SearchableModelTransformer validation happy case', () => {
   const validSchema = `
@@ -15,6 +26,7 @@ test('Test SearchableModelTransformer validation happy case', () => {
     `;
   const transformer = new GraphQLTransform({
     transformers: [new ModelTransformer(), new SearchableModelTransformer()],
+    featureFlags,
   });
   const out = transformer.transform(validSchema);
   expect(out).toBeDefined();
@@ -33,6 +45,7 @@ test('Test SearchableModelTransformer vtl', () => {
     `;
   const transformer = new GraphQLTransform({
     transformers: [new ModelTransformer(), new SearchableModelTransformer()],
+    featureFlags,
   });
 
   const out = transformer.transform(validSchema);
@@ -50,6 +63,7 @@ test('Test SearchableModelTransformer with query overrides', () => {
     `;
   const transformer = new GraphQLTransform({
     transformers: [new ModelTransformer(), new SearchableModelTransformer()],
+    featureFlags,
   });
   const out = transformer.transform(validSchema);
   expect(out).toBeDefined();
@@ -67,6 +81,7 @@ test('Test SearchableModelTransformer with only create mutations', () => {
     `;
   const transformer = new GraphQLTransform({
     transformers: [new ModelTransformer(), new SearchableModelTransformer()],
+    featureFlags,
   });
   const out = transformer.transform(validSchema);
   expect(out).toBeDefined();
@@ -90,6 +105,7 @@ test('Test SearchableModelTransformer with multiple model searchable directives'
     `;
   const transformer = new GraphQLTransform({
     transformers: [new ModelTransformer(), new SearchableModelTransformer()],
+    featureFlags,
   });
   const out = transformer.transform(validSchema);
   expect(out).toBeDefined();
@@ -108,6 +124,7 @@ test('Test SearchableModelTransformer with sort fields', () => {
     `;
   const transformer = new GraphQLTransform({
     transformers: [new ModelTransformer(), new SearchableModelTransformer()],
+    featureFlags,
   });
   const out = transformer.transform(validSchema);
   expect(out).toBeDefined();
@@ -137,6 +154,7 @@ test('it generates expected resources', () => {
  `;
   const transformer = new GraphQLTransform({
     transformers: [new ModelTransformer(), new SearchableModelTransformer()],
+    featureFlags,
   });
   const out = transformer.transform(validSchema);
   expect(out).toBeDefined();

diff --git a/packages/amplify-graphql-searchable-transformer/src/graphql-searchable-transformer.ts b/packages/amplify-graphql-searchable-transformer/src/graphql-searchable-transformer.ts
@@ -158,7 +158,9 @@ export class SearchableModelTransformer extends TransformerPluginBase {
         searchFieldNameOverride = directiveArguments.queries.search;
       }
     }
-    const fieldName = searchFieldNameOverride ? searchFieldNameOverride : graphqlName(`search${plurality(toUpper(definition.name.value))}`);
+    const fieldName = searchFieldNameOverride
+      ? searchFieldNameOverride
+      : graphqlName(`search${plurality(toUpper(definition.name.value), ctx.featureFlags.getBoolean('improvePluralization'))}`);
     this.searchableObjectTypeDefinitions.push({
       node: definition,
       fieldName,

diff --git a/packages/amplify-graphql-transformer-core/src/transformation/transform.ts b/packages/amplify-graphql-transformer-core/src/transformation/transform.ts
@@ -61,7 +61,7 @@ export interface GraphQLTransformOptions {
   readonly authConfig?: AppSyncAuthConfiguration;
   readonly buildParameters?: Record<string, any>;
   readonly stacks?: Record<string, Template>;
-  readonly featuerFlags?: FeatureFlagProvider;
+  readonly featureFlags?: FeatureFlagProvider;
 }
 export type StackMapping = { [resourceId: string]: string };
 export class GraphQLTransform {
@@ -110,7 +110,7 @@ export class GraphQLTransform {
     this.seenTransformations = {};
     const parsedDocument = parse(schema);
     this.app = new App();
-    const context = new TransformerContext(this.app, parsedDocument, this.stackMappingOverrides, this.options.featuerFlags);
+    const context = new TransformerContext(this.app, parsedDocument, this.stackMappingOverrides, this.options.featureFlags);
     const validDirectiveNameMap = this.transformers.reduce(
       (acc: any, t: TransformerPluginProvider) => ({ ...acc, [t.directive.name.value]: true }),
       {

diff --git a/packages/amplify-graphql-transformer-core/src/transformer-context/index.ts b/packages/amplify-graphql-transformer-core/src/transformer-context/index.ts
@@ -26,15 +26,20 @@ export class TransformerContext implements TransformerContextProvider {
   public readonly resourceHelper: TransformerResourceHelper;
   public readonly featureFlags: FeatureFlagProvider;
   public _api?: GraphQLAPIProvider;
-  constructor(app: App, public readonly inputDocument: DocumentNode, stackMapping: Record<string, string>, featuerFlags?: FeatureFlagProvider) {
+  constructor(
+    app: App,
+    public readonly inputDocument: DocumentNode,
+    stackMapping: Record<string, string>,
+    featureFlags?: FeatureFlagProvider,
+  ) {
     this.output = new TransformerOutput(inputDocument);
     this.resolvers = new ResolverManager();
     this.dataSources = new TransformerDataSourceManager();
     this.providerRegistry = new TransformerContextProviderRegistry();
-    const stackManager = new  StackManager(app, stackMapping);
-    this.stackManager = stackManager
+    const stackManager = new StackManager(app, stackMapping);
+    this.stackManager = stackManager;
     this.resourceHelper = new TransformerResourceHelper(stackManager);
-    this.featureFlags = featuerFlags ?? new NoopFeatureFlagProvider();
+    this.featureFlags = featureFlags ?? new NoopFeatureFlagProvider();
   }
 
   /**

diff --git a/...es/amplify-provider-awscloudformation/src/graphql-transformer/transform-graphql-schema.ts b/...es/amplify-provider-awscloudformation/src/graphql-transformer/transform-graphql-schema.ts
@@ -414,7 +414,7 @@ async function _buildProject(opts: ProjectOptions<TransformerFactoryArgs>) {
     authConfig: opts.authConfig,
     buildParameters: opts.buildParameters,
     stacks: opts.projectConfig.stacks || {},
-    featuerFlags: new AmplifyCLIFeatureFlagAdapter(),
+    featureFlags: new AmplifyCLIFeatureFlagAdapter(),
   });
   return transform.transform(userProjectConfig.schema.toString());
 }
diff --git a/packages/graphql-auth-transformer/src/ModelAuthTransformer.ts b/packages/graphql-auth-transformer/src/ModelAuthTransformer.ts
@@ -368,7 +368,7 @@ export class ModelAuthTransformer extends Transformer {
     this.propagateAuthDirectivesToNestedTypes(def, rules, ctx);
 
     // Retrieve the configuration options for the related @model directive
-    const modelConfiguration = new ModelDirectiveConfiguration(modelDirective, def);
+    const modelConfiguration = new ModelDirectiveConfiguration(modelDirective, def, ctx.featureFlags.getBoolean('improvePluralization'));
 
     // Get the directives we need to add to the GraphQL nodes
     const directives = this.getDirectivesForRules(rules, rules.length === 0 ? this.shouldAddDefaultAuthDirective() : false);
@@ -516,7 +516,11 @@ Static group authorization should perform as expected.`,
       const isDeleteRule = isOpRule('delete');
 
       // Retrieve the configuration options for the related @model directive
-      const modelConfiguration = new ModelDirectiveConfiguration(modelDirective, parent);
+      const modelConfiguration = new ModelDirectiveConfiguration(
+        modelDirective,
+        parent,
+        ctx.featureFlags.getBoolean('improvePluralization'),
+      );
       // The field handler adds the read rule on the object
       const readRules = rules.filter((rule: AuthRule) => isReadRule(rule));
       this.protectReadForField(ctx, parent, definition, readRules, modelConfiguration);

diff --git a/packages/graphql-auth-transformer/src/ModelDirectiveConfiguration.ts b/packages/graphql-auth-transformer/src/ModelDirectiveConfiguration.ts
@@ -41,12 +41,14 @@ type ModelDirectiveOperation = {
 export class ModelDirectiveConfiguration {
   map: Map<ModelDirectiveOperationType, ModelDirectiveOperation> = new Map();
 
-  constructor(directive: DirectiveNode, def: ObjectTypeDefinitionNode) {
+  constructor(directive: DirectiveNode, def: ObjectTypeDefinitionNode, improvePluralization: boolean) {
     const typeName = def.name.value;
     const directiveArguments: ModelDirectiveArgs = getDirectiveArguments(directive);
 
     const makeName = (operation: ModelDirectiveOperationType, nameOverride?: string, isList: boolean = false) =>
-      nameOverride ? nameOverride : graphqlName(operation + (isList ? plurality(toUpper(typeName)) : toUpper(typeName)));
+      nameOverride
+        ? nameOverride
+        : graphqlName(operation + (isList ? plurality(toUpper(typeName), improvePluralization) : toUpper(typeName)));
 
     let shouldHaveCreate = true;
     let shouldHaveUpdate = true;

diff --git a/packages/graphql-auth-transformer/src/__tests__/AmplifyAdminAuth.test.ts b/packages/graphql-auth-transformer/src/__tests__/AmplifyAdminAuth.test.ts
@@ -1,8 +1,18 @@
 import { GraphQLTransform } from 'graphql-transformer-core';
-import { ResourceConstants } from 'graphql-transformer-common';
 import { DynamoDBModelTransformer } from 'graphql-dynamodb-transformer';
 import { ModelAuthTransformer } from '../ModelAuthTransformer';
 import _ from 'lodash';
+const featureFlags = {
+  getBoolean: jest.fn().mockImplementation((name, defaultValue) => {
+    if (name === 'improvePluralization') {
+      return true;
+    }
+    return;
+  }),
+  getNumber: jest.fn(),
+  getObject: jest.fn(),
+  getString: jest.fn(),
+};
 
 test('Test simple model with public auth rule and amplify admin app is present', () => {
   const validSchema = `
@@ -14,6 +24,7 @@ test('Test simple model with public auth rule and amplify admin app is present',
     }
     `;
   const transformer = new GraphQLTransform({
+    featureFlags,
     transformers: [
       new DynamoDBModelTransformer(),
       new ModelAuthTransformer({
@@ -46,6 +57,7 @@ test('Test simple model with public auth rule and amplify admin app is not enabl
       }
       `;
   const transformer = new GraphQLTransform({
+    featureFlags,
     transformers: [
       new DynamoDBModelTransformer(),
       new ModelAuthTransformer({
@@ -74,6 +86,7 @@ test('Test simple model with private auth rule and amplify admin app is present'
       }
       `;
   const transformer = new GraphQLTransform({
+    featureFlags,
     transformers: [
       new DynamoDBModelTransformer(),
       new ModelAuthTransformer({
@@ -106,6 +119,7 @@ test('Test simple model with private auth rule and amplify admin app not enabled
       }
       `;
   const transformer = new GraphQLTransform({
+    featureFlags,
     transformers: [
       new DynamoDBModelTransformer(),
       new ModelAuthTransformer({
@@ -138,6 +152,7 @@ test('Test model with public auth rule without all operations and amplify admin
       }
       `;
   const transformer = new GraphQLTransform({
+    featureFlags,
     transformers: [
       new DynamoDBModelTransformer(),
       new ModelAuthTransformer({
@@ -182,6 +197,7 @@ test('Test simple model with private auth rule, few operations, and amplify admi
       }
       `;
   const transformer = new GraphQLTransform({
+    featureFlags,
     transformers: [
       new DynamoDBModelTransformer(),
       new ModelAuthTransformer({
@@ -225,6 +241,7 @@ test('Test simple model with private IAM auth rule, few operations, and amplify
       }
       `;
   const transformer = new GraphQLTransform({
+    featureFlags,
     transformers: [
       new DynamoDBModelTransformer(),
       new ModelAuthTransformer({
@@ -263,6 +280,7 @@ test('Test simple model with AdminUI enabled should add IAM policy only for fiel
       }
       `;
   const transformer = new GraphQLTransform({
+    featureFlags,
     transformers: [
       new DynamoDBModelTransformer(),
       new ModelAuthTransformer({

diff --git a/packages/graphql-auth-transformer/src/__tests__/GroupAuthTransformer.test.ts b/packages/graphql-auth-transformer/src/__tests__/GroupAuthTransformer.test.ts
@@ -2,6 +2,17 @@ import { GraphQLTransform } from 'graphql-transformer-core';
 import { ResourceConstants } from 'graphql-transformer-common';
 import { DynamoDBModelTransformer } from 'graphql-dynamodb-transformer';
 import { ModelAuthTransformer } from '../ModelAuthTransformer';
+const featureFlags = {
+  getBoolean: jest.fn().mockImplementation((name, defaultValue) => {
+    if (name === 'improvePluralization') {
+      return true;
+    }
+    return;
+  }),
+  getNumber: jest.fn(),
+  getObject: jest.fn(),
+  getString: jest.fn(),
+};
 
 test('Test ModelAuthTransformer validation happy case w/ static groups', () => {
   const validSchema = `
@@ -13,6 +24,7 @@ test('Test ModelAuthTransformer validation happy case w/ static groups', () => {
     }
     `;
   const transformer = new GraphQLTransform({
+    featureFlags,
     transformers: [
       new DynamoDBModelTransformer(),
       new ModelAuthTransformer({
@@ -28,7 +40,7 @@ test('Test ModelAuthTransformer validation happy case w/ static groups', () => {
   const out = transformer.transform(validSchema);
   expect(out).toBeDefined();
   expect(out.rootStack.Resources[ResourceConstants.RESOURCES.GraphQLAPILogicalID].Properties.AuthenticationType).toEqual(
-    'AMAZON_COGNITO_USER_POOLS'
+    'AMAZON_COGNITO_USER_POOLS',
   );
 });
 
@@ -43,6 +55,7 @@ test('Test ModelAuthTransformer validation happy case w/ dynamic groups', () =>
     }
     `;
   const transformer = new GraphQLTransform({
+    featureFlags,
     transformers: [
       new DynamoDBModelTransformer(),
       new ModelAuthTransformer({
@@ -58,7 +71,7 @@ test('Test ModelAuthTransformer validation happy case w/ dynamic groups', () =>
   const out = transformer.transform(validSchema);
   expect(out).toBeDefined();
   expect(out.rootStack.Resources[ResourceConstants.RESOURCES.GraphQLAPILogicalID].Properties.AuthenticationType).toEqual(
-    'AMAZON_COGNITO_USER_POOLS'
+    'AMAZON_COGNITO_USER_POOLS',
   );
 });
 
@@ -73,6 +86,7 @@ test('Test ModelAuthTransformer validation happy case w/ dynamic group', () => {
     }
     `;
   const transformer = new GraphQLTransform({
+    featureFlags,
     transformers: [
       new DynamoDBModelTransformer(),
       new ModelAuthTransformer({
@@ -88,7 +102,7 @@ test('Test ModelAuthTransformer validation happy case w/ dynamic group', () => {
   const out = transformer.transform(validSchema);
   expect(out).toBeDefined();
   expect(out.rootStack.Resources[ResourceConstants.RESOURCES.GraphQLAPILogicalID].Properties.AuthenticationType).toEqual(
-    'AMAZON_COGNITO_USER_POOLS'
+    'AMAZON_COGNITO_USER_POOLS',
   );
 });
 
@@ -104,6 +118,7 @@ test('Test ModelAuthTransformer validation @auth on non @model. Should fail.', (
             }
         `;
     const transformer = new GraphQLTransform({
+      featureFlags,
       transformers: [
         new DynamoDBModelTransformer(),
         new ModelAuthTransformer({