Skip to content

Commit

Permalink
Revert "fix(graphql-transformer-common): improve generated graphql pl…
Browse files Browse the repository at this point in the history
…uralization (#7258)" (#7578)
  • Loading branch information
edwardfoyle authored Jun 22, 2021
1 parent d936bec commit 22b1a6a
Show file tree
Hide file tree
Showing 63 changed files with 152 additions and 908 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ 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 { FeatureFlags, ResourceDoesNotExistError, exitOnNextTick } = require('amplify-cli-core');
const { ResourceDoesNotExistError, exitOnNextTick } = require('amplify-cli-core');

const subcommand = 'add-graphql-datasource';
const categories = 'categories';
const category = 'api';
Expand Down Expand Up @@ -142,11 +143,7 @@ module.exports = {
context[rdsResourceName] = resourceName;
context[rdsDatasource] = datasource;
let template = templateGenerator.createTemplate(context);
template = templateGenerator.addRelationalResolvers(
template,
resolversDir,
FeatureFlags.getBoolean('graphqltransformer.improvePluralization'),
);
template = templateGenerator.addRelationalResolvers(template, resolversDir);
const cfn = templateGenerator.printCloudformationTemplate(template);

/**
Expand Down
8 changes: 1 addition & 7 deletions packages/amplify-cli-core/src/feature-flags/featureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -515,12 +515,6 @@ export class FeatureFlags {
defaultValueForExistingProjects: false,
defaultValueForNewProjects: true,
},
{
name: 'improvePluralization',
type: 'boolean',
defaultValueForExistingProjects: false,
defaultValueForNewProjects: true,
},
{
name: 'validateTypeNameReservedWords',
type: 'boolean',
Expand Down Expand Up @@ -625,7 +619,7 @@ export class FeatureFlags {
type: 'boolean',
defaultValueForExistingProjects: false,
defaultValueForNewProjects: true,
},
}
]);

this.registerFlag('appSync', [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ export const expected_result_query6 = {

export const query7 = `
## 7. See all employees hired recently:
#Having '@key(name: "newHire", fields: ["newHire", "id"])' on the 'Employee' model allows one to query by whether an employee has been hired recently.
#Having '@key(name: "newHire", fields: ["newHire", "id"])' on the 'Employee' model allows one to query by whether an employee has been hired recently.
query employeesNewHire {
employeesNewHire(newHire: "true") {
Expand Down Expand Up @@ -685,8 +685,8 @@ export const query16 = `
## 16. Get total product inventory:
#How this would be done depends on the use case. If one just wants a list of all inventories in all warehouses, one could just run a list inventories on the Inventory model:
query listInventories {
listInventories {
query listInventorys {
listInventorys {
items {
productID
warehouseID
Expand All @@ -696,7 +696,7 @@ query listInventories {
}`;
export const expected_result_query16 = {
data: {
listInventories: {
listInventorys: {
items: [
{
productID: 'yeezyboost',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,6 @@ 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 = `
Expand All @@ -26,7 +15,6 @@ test('Test SearchableModelTransformer validation happy case', () => {
`;
const transformer = new GraphQLTransform({
transformers: [new ModelTransformer(), new SearchableModelTransformer()],
featureFlags,
});
const out = transformer.transform(validSchema);
expect(out).toBeDefined();
Expand All @@ -45,7 +33,6 @@ test('Test SearchableModelTransformer vtl', () => {
`;
const transformer = new GraphQLTransform({
transformers: [new ModelTransformer(), new SearchableModelTransformer()],
featureFlags,
});

const out = transformer.transform(validSchema);
Expand All @@ -63,7 +50,6 @@ test('Test SearchableModelTransformer with query overrides', () => {
`;
const transformer = new GraphQLTransform({
transformers: [new ModelTransformer(), new SearchableModelTransformer()],
featureFlags,
});
const out = transformer.transform(validSchema);
expect(out).toBeDefined();
Expand All @@ -81,7 +67,6 @@ 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();
Expand All @@ -105,7 +90,6 @@ 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();
Expand All @@ -124,7 +108,6 @@ test('Test SearchableModelTransformer with sort fields', () => {
`;
const transformer = new GraphQLTransform({
transformers: [new ModelTransformer(), new SearchableModelTransformer()],
featureFlags,
});
const out = transformer.transform(validSchema);
expect(out).toBeDefined();
Expand Down Expand Up @@ -154,7 +137,6 @@ test('it generates expected resources', () => {
`;
const transformer = new GraphQLTransform({
transformers: [new ModelTransformer(), new SearchableModelTransformer()],
featureFlags,
});
const out = transformer.transform(validSchema);
expect(out).toBeDefined();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,7 @@ export class SearchableModelTransformer extends TransformerPluginBase {
searchFieldNameOverride = directiveArguments.queries.search;
}
}
const fieldName = searchFieldNameOverride
? searchFieldNameOverride
: graphqlName(`search${plurality(toUpper(definition.name.value), ctx.featureFlags.getBoolean('improvePluralization'))}`);
const fieldName = searchFieldNameOverride ? searchFieldNameOverride : graphqlName(`search${plurality(toUpper(definition.name.value))}`);
this.searchableObjectTypeDefinitions.push({
node: definition,
fieldName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export interface GraphQLTransformOptions {
readonly authConfig?: AppSyncAuthConfiguration;
readonly buildParameters?: Record<string, any>;
readonly stacks?: Record<string, Template>;
readonly featureFlags?: FeatureFlagProvider;
readonly featuerFlags?: FeatureFlagProvider;
}
export type StackMapping = { [resourceId: string]: string };
export class GraphQLTransform {
Expand Down Expand Up @@ -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.featureFlags);
const context = new TransformerContext(this.app, parsedDocument, this.stackMappingOverrides, this.options.featuerFlags);
const validDirectiveNameMap = this.transformers.reduce(
(acc: any, t: TransformerPluginProvider) => ({ ...acc, [t.directive.name.value]: true }),
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,15 @@ 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>,
featureFlags?: FeatureFlagProvider,
) {
constructor(app: App, public readonly inputDocument: DocumentNode, stackMapping: Record<string, string>, featuerFlags?: 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 = featureFlags ?? new NoopFeatureFlagProvider();
this.featureFlags = featuerFlags ?? new NoopFeatureFlagProvider();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ async function _buildProject(opts: ProjectOptions<TransformerFactoryArgs>) {
authConfig: opts.authConfig,
buildParameters: opts.buildParameters,
stacks: opts.projectConfig.stacks || {},
featureFlags: new AmplifyCLIFeatureFlagAdapter(),
featuerFlags: new AmplifyCLIFeatureFlagAdapter(),
});
return transform.transform(userProjectConfig.schema.toString());
}
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ test(`Test getSalary w/ Admin group protection not authorized`, async () => {
expect((req2.errors[0] as any).errorType).toEqual('Unauthorized');
});

test(`Test listSalaries w/ Admin group protection authorized`, async () => {
test(`Test listSalarys w/ Admin group protection authorized`, async () => {
const req = await GRAPHQL_CLIENT_1.query(`
mutation {
createSalary(input: { wage: 101 }) {
Expand All @@ -809,20 +809,20 @@ test(`Test listSalaries w/ Admin group protection authorized`, async () => {
expect(req.data.createSalary.wage).toEqual(101);
const req2 = await GRAPHQL_CLIENT_1.query(`
query {
listSalaries(filter: { wage: { eq: 101 }}) {
listSalarys(filter: { wage: { eq: 101 }}) {
items {
id
wage
}
}
}
`);
expect(req2.data.listSalaries.items.length).toEqual(1);
expect(req2.data.listSalaries.items[0].id).toEqual(req.data.createSalary.id);
expect(req2.data.listSalaries.items[0].wage).toEqual(101);
expect(req2.data.listSalarys.items.length).toEqual(1);
expect(req2.data.listSalarys.items[0].id).toEqual(req.data.createSalary.id);
expect(req2.data.listSalarys.items[0].wage).toEqual(101);
});

test(`Test listSalaries w/ Admin group protection not authorized`, async () => {
test(`Test listSalarys w/ Admin group protection not authorized`, async () => {
const req = await GRAPHQL_CLIENT_1.query(`
mutation {
createSalary(input: { wage: 102 }) {
Expand All @@ -836,15 +836,15 @@ test(`Test listSalaries w/ Admin group protection not authorized`, async () => {
expect(req.data.createSalary.wage).toEqual(102);
const req2 = await GRAPHQL_CLIENT_2.query(`
query {
listSalaries(filter: { wage: { eq: 102 }}) {
listSalarys(filter: { wage: { eq: 102 }}) {
items {
id
wage
}
}
}
`);
expect(req2.data.listSalaries.items).toEqual([]);
expect(req2.data.listSalarys.items).toEqual([]);
});

/**
Expand Down Expand Up @@ -2377,7 +2377,7 @@ test(`Test createTestIdentity as admin.`, async () => {

const listResponse = await GRAPHQL_CLIENT_3.query(
`query {
listTestIdentities(filter: { title: { eq: "Test title update" } }, limit: 100) {
listTestIdentitys(filter: { title: { eq: "Test title update" } }, limit: 100) {
items {
id
title
Expand All @@ -2387,7 +2387,7 @@ test(`Test createTestIdentity as admin.`, async () => {
}`,
{},
);
const relevantPost = listResponse.data.listTestIdentities.items.find(p => p.id === getReq.data.getTestIdentity.id);
const relevantPost = listResponse.data.listTestIdentitys.items.find(p => p.id === getReq.data.getTestIdentity.id);
logDebug(JSON.stringify(listResponse, null, 4));
expect(relevantPost).toBeTruthy();
expect(relevantPost.title).toEqual('Test title update');
Expand Down
8 changes: 2 additions & 6 deletions packages/graphql-auth-transformer/src/ModelAuthTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, ctx.featureFlags.getBoolean('improvePluralization'));
const modelConfiguration = new ModelDirectiveConfiguration(modelDirective, def);

// Get the directives we need to add to the GraphQL nodes
const directives = this.getDirectivesForRules(rules, rules.length === 0 ? this.shouldAddDefaultAuthDirective() : false);
Expand Down Expand Up @@ -516,11 +516,7 @@ 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,
ctx.featureFlags.getBoolean('improvePluralization'),
);
const modelConfiguration = new ModelDirectiveConfiguration(modelDirective, parent);
// 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,12 @@ type ModelDirectiveOperation = {
export class ModelDirectiveConfiguration {
map: Map<ModelDirectiveOperationType, ModelDirectiveOperation> = new Map();

constructor(directive: DirectiveNode, def: ObjectTypeDefinitionNode, improvePluralization: boolean) {
constructor(directive: DirectiveNode, def: ObjectTypeDefinitionNode) {
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), improvePluralization) : toUpper(typeName)));
nameOverride ? nameOverride : graphqlName(operation + (isList ? plurality(toUpper(typeName)) : toUpper(typeName)));

let shouldHaveCreate = true;
let shouldHaveUpdate = true;
Expand Down
Loading

0 comments on commit 22b1a6a

Please sign in to comment.