Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle Enums correctly on Typescript data schema generation #2870

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
3e4a3a2
added enums via ref for single models
tejas2008 Sep 12, 2024
7bc5b64
extended implementation to enums for multiple models and added filte…
tejas2008 Sep 12, 2024
6161264
added naming in npascal case
tejas2008 Sep 13, 2024
07535fa
added enum support for postgresql multiple models same enum
tejas2008 Sep 16, 2024
3436237
added unit tests to test working enum handling
tejas2008 Sep 17, 2024
6aed951
Merge branch 'aws-amplify:main' into Handle-enums-sql
tejas2008 Sep 17, 2024
4030828
added error handling for unsupported engines removed unused code and …
tejas2008 Sep 19, 2024
c4f9b4d
variable naming updated for enums
tejas2008 Sep 20, 2024
c3d3ee1
fix linting issues
tejas2008 Sep 20, 2024
8227f72
refactored code for readbility and updated unit tests
tejas2008 Sep 24, 2024
bb0a235
fixed linting issues
tejas2008 Sep 24, 2024
5ab00c2
updated one comment and fixed lint issue
tejas2008 Sep 24, 2024
b064a2d
fixed nit
tejas2008 Sep 26, 2024
4e52a85
fixed nit naming
tejas2008 Sep 26, 2024
2a88fb9
fixed nit naming
tejas2008 Sep 26, 2024
3734e8f
lint errors fixed
tejas2008 Oct 2, 2024
94b0767
extracted api
tejas2008 Oct 2, 2024
c383ec5
Merge branch 'main' into Handle-enums-sql
tejas2008 Oct 2, 2024
47697f4
extracted api transformer common
tejas2008 Oct 2, 2024
1b80f3b
Merge branch 'Handle-enums-sql' of https://github.com/tejas2008/ampli…
tejas2008 Oct 2, 2024
f11e13c
helper lint
tejas2008 Oct 2, 2024
9b04649
lint fix
tejas2008 Oct 3, 2024
19273c7
Merge branch 'main' of https://github.com/aws-amplify/amplify-categor…
tejas2008 Oct 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,120 @@ export const schema = a.schema({
\\"User\\": a.model({
id: a.string().required(),
name: a.string(),
status: a.enum([
\\"ACTIVE\\",
\\"INACTIVE\\"
])
status: a.ref(\\"UserStatus\\")
}).identifier([
\\"id\\"
]),
UserStatus: a.enum([
\\"ACTIVE\\",
\\"INACTIVE\\"
])
});
"
`;

exports[`Type name conversions generates enums for models in mysql 1`] = `
"/* eslint-disable */
/* THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. */
import { a } from \\"@aws-amplify/data-schema\\";

export const schema = a.schema({
\\"User\\": a.model({
id: a.string().required(),
name: a.string(),
status: a.ref(\\"UserStatus\\").required()
}).identifier([
\\"id\\"
]),
\\"Test\\": a.model({
id: a.string().required(),
name: a.string(),
age: a.ref(\\"TestAge\\").required()
}).identifier([
\\"id\\"
]),
UserStatus: a.enum([
tejas2008 marked this conversation as resolved.
Show resolved Hide resolved
\\"ACTIVE\\",
\\"INACTIVE\\"
]),
TestAge: a.enum([
\\"Above18\\",
\\"Below18\\"
])
});
"
`;

exports[`Type name conversions generates required and null enums correctly in the same model 1`] = `
"/* eslint-disable */
/* THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. */
import { a } from \\"@aws-amplify/data-schema\\";

export const schema = a.schema({
\\"User\\": a.model({
id: a.string().required(),
name: a.string(),
statsu1: a.ref(\\"UserStatus1\\"),
status: a.ref(\\"UserStatus\\").required()
}).identifier([
\\"id\\"
]),
UserStatus1: a.enum([
\\"ACTIVE\\",
\\"INACTIVE\\"
]),
UserStatus: a.enum([
\\"ACTIVE\\",
\\"INACTIVE\\"
])
});
"
`;

exports[`Type name conversions generates required enums correctly 1`] = `
"/* eslint-disable */
/* THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. */
import { a } from \\"@aws-amplify/data-schema\\";

export const schema = a.schema({
\\"User\\": a.model({
id: a.string().required(),
name: a.string(),
status: a.ref(\\"UserStatus\\").required()
}).identifier([
\\"id\\"
]),
UserStatus: a.enum([
\\"ACTIVE\\",
\\"INACTIVE\\"
])
});
"
`;

exports[`Type name conversions generates single enum referenced for two different models 1`] = `
"/* eslint-disable */
/* THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. */
import { a } from \\"@aws-amplify/data-schema\\";

export const schema = a.schema({
\\"User\\": a.model({
id: a.string().required(),
name: a.string(),
status: a.ref(\\"UserStatus\\").required()
}).identifier([
\\"id\\"
]),
\\"Test\\": a.model({
id: a.string().required(),
name: a.string(),
status: a.ref(\\"UserStatus\\").required()
}).identifier([
\\"id\\"
]),
UserStatus: a.enum([
\\"ACTIVE\\",
\\"INACTIVE\\"
])
});
"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,77 @@ describe('Type name conversions', () => {
expect(graphqlSchema).toMatchSnapshot();
});

it('generates required enums correctly', () => {
const dbschema = new Schema(new Engine('Postgres'));
const model = new Model('User');
model.addField(new Field('id', { kind: 'NonNull', type: { kind: 'Scalar', name: 'String' } }));
model.addField(new Field('name', { kind: 'Scalar', name: 'String' }));
model.addField(new Field('status', { kind: 'NonNull', type: { kind: 'Enum', name: 'UserStatus', values: ['ACTIVE', 'INACTIVE'] } }));
model.setPrimaryKey(['id']);
dbschema.addModel(model);

const graphqlSchema = generateTypescriptDataSchema(dbschema);
expect(graphqlSchema).toMatchSnapshot();
});

it('generates required and null enums correctly in the same model', () => {
const dbschema = new Schema(new Engine('Postgres'));
const model = new Model('User');
model.addField(new Field('id', { kind: 'NonNull', type: { kind: 'Scalar', name: 'String' } }));
model.addField(new Field('name', { kind: 'Scalar', name: 'String' }));
model.addField(new Field('statsu1', { kind: 'Enum', name: 'UserStatus1', values: ['ACTIVE', 'INACTIVE'] }));
model.addField(new Field('status', { kind: 'NonNull', type: { kind: 'Enum', name: 'UserStatus', values: ['ACTIVE', 'INACTIVE'] } }));
model.setPrimaryKey(['id']);
dbschema.addModel(model);

const graphqlSchema = generateTypescriptDataSchema(dbschema);
expect(graphqlSchema).toMatchSnapshot();
});

it('generates single enum referenced for two different models', () => {
const dbschema = new Schema(new Engine('Postgres'));
const modelUser = new Model('User');
const modelTest = new Model('Test');
modelUser.addField(new Field('id', { kind: 'NonNull', type: { kind: 'Scalar', name: 'String' } }));
modelUser.addField(new Field('name', { kind: 'Scalar', name: 'String' }));
modelUser.addField(
new Field('status', { kind: 'NonNull', type: { kind: 'Enum', name: 'UserStatus', values: ['ACTIVE', 'INACTIVE'] } }),
);
modelUser.setPrimaryKey(['id']);
dbschema.addModel(modelUser);
modelTest.addField(new Field('id', { kind: 'NonNull', type: { kind: 'Scalar', name: 'String' } }));
modelTest.addField(new Field('name', { kind: 'Scalar', name: 'String' }));
modelTest.addField(
new Field('status', { kind: 'NonNull', type: { kind: 'Enum', name: 'UserStatus', values: ['ACTIVE', 'INACTIVE'] } }),
);
modelTest.setPrimaryKey(['id']);
dbschema.addModel(modelTest);

const graphqlSchema = generateTypescriptDataSchema(dbschema);
expect(graphqlSchema).toMatchSnapshot();
});

it('generates enums for models in mysql', () => {
const dbschema = new Schema(new Engine('MySQL'));
const modelUser = new Model('User');
const modelTest = new Model('Test');
modelUser.addField(new Field('id', { kind: 'NonNull', type: { kind: 'Scalar', name: 'String' } }));
modelUser.addField(new Field('name', { kind: 'Scalar', name: 'String' }));
modelUser.addField(
new Field('status', { kind: 'NonNull', type: { kind: 'Enum', name: 'userStatus', values: ['ACTIVE', 'INACTIVE'] } }),
);
modelUser.setPrimaryKey(['id']);
dbschema.addModel(modelUser);
modelTest.addField(new Field('id', { kind: 'NonNull', type: { kind: 'Scalar', name: 'String' } }));
modelTest.addField(new Field('name', { kind: 'Scalar', name: 'String' }));
modelTest.addField(new Field('age', { kind: 'NonNull', type: { kind: 'Enum', name: 'TestAge', values: ['Above18', 'Below18'] } }));
modelTest.setPrimaryKey(['id']);
dbschema.addModel(modelTest);

const graphqlSchema = generateTypescriptDataSchema(dbschema);
expect(graphqlSchema).toMatchSnapshot();
});

it('schema with database config secret and vpc should generate typescript data schema with configure', () => {
const dbschema = new Schema(new Engine('MySQL'));
let model = new Model('User');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('testStringDataSourceAdapter', () => {
});
expect(adapter.mapDataType('enum', true, 'table', 'field', 'enum("OPEN","CLOSED")')).toEqual({
kind: 'Enum',
name: 'table_field',
name: 'TableField',
values: ['OPEN', 'CLOSED'],
});
expect(adapter.mapDataType('bool', true, 'table', 'field', 'varchar(50)')).toEqual({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { toPascalCase } from 'graphql-transformer-common';
import { EnumType, Field, FieldDataType, FieldType, Index } from '../schema-representation';
import { StringDataSourceAdapter, EmptySchemaError, InvalidSchemaError } from './string-datasource-adapter';

Expand Down Expand Up @@ -279,12 +280,12 @@ export class MySQLStringDataSourceAdapter extends StringDataSourceAdapter {
return value.match(regex).map((a) => a.slice(1, -1));
}

private generateEnumName(tableName: string, fieldName: string) {
const enumNamePrefix = [tableName, fieldName].join('_');
private generateEnumName(tableName: string, fieldName: string): string {
const enumNamePrefix = toPascalCase([tableName, fieldName]);
let enumName = enumNamePrefix;
let counter = 0;
while (this.enums.has(enumName)) {
enumName = [enumNamePrefix, counter.toString()].join('_');
enumName = toPascalCase([enumNamePrefix, counter.toString()]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you confirm this wouldn't be a breaking change if we change the naming of enums from table_field to TableField?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per the discussion @palpatim the change could affect customers only if they reimport the schema which is highly unlikely, but we still are waiting for conformation from Josef on the same

counter++;
}
return enumName;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { EnumType, Field, FieldDataType, FieldType, Index } from '../schema-representation';
import { StringDataSourceAdapter, EmptySchemaError, InvalidSchemaError } from './string-datasource-adapter';
import { toPascalCase } from 'graphql-transformer-common';

export interface PostgresIndex {
tableName: string;
Expand Down Expand Up @@ -156,7 +157,7 @@ export class PostgresStringDataSourceAdapter extends StringDataSourceAdapter {
const enumValues = row.enum_values.substring(1, row.enum_values.length - 1).split(',');
const enumType: EnumType = {
kind: 'Enum',
name: enumName,
name: toPascalCase([enumName]),
values: enumValues,
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import ts from 'typescript';
import { TYPESCRIPT_DATA_SCHEMA_CONSTANTS } from 'graphql-transformer-common';
import { TYPESCRIPT_DATA_SCHEMA_CONSTANTS, toPascalCase } from 'graphql-transformer-common';
import { VpcConfig } from '@aws-amplify/graphql-transformer-interfaces';
import { DBEngineType, Field, FieldType, Model, Schema } from '../schema-representation';
import { DBEngineType, EnumType, Field, FieldType, Model, Schema } from '../schema-representation';
import { CommentNode } from '../../../graphql-mapping-template/lib/ast';

const GQL_TYPESCRIPT_DATA_SCHEMA_TYPE_MAP = {
string: 'string',
Expand Down Expand Up @@ -46,16 +47,12 @@ const createDataType = (type: FieldType): ts.Node => {
);
}

// make it in the form a.ref('name')
if (type.kind === 'Enum') {
return ts.factory.createCallExpression(
ts.factory.createIdentifier(`${TYPESCRIPT_DATA_SCHEMA_CONSTANTS.REFERENCE_A}.${TYPESCRIPT_DATA_SCHEMA_CONSTANTS.ENUM_METHOD}`),
ts.factory.createIdentifier(`${TYPESCRIPT_DATA_SCHEMA_CONSTANTS.REFERENCE_A}.${TYPESCRIPT_DATA_SCHEMA_CONSTANTS.REF_METHOD}`),
undefined,
[
ts.factory.createArrayLiteralExpression(
type.values.map((value) => ts.factory.createStringLiteral(value)),
true,
),
],
[ts.factory.createStringLiteral(toPascalCase([type.name]))],
);
}

Expand Down Expand Up @@ -91,6 +88,26 @@ const createModelDefinition = (model: Model): ts.Node => {
return ts.factory.createObjectLiteralExpression(properties as ts.ObjectLiteralElementLike[], true);
};

/**
* Creates a typescript data schema type from internal SQL schema representation
* Example typescript data schema type output: `a.enum()`
* @param type SQL IR Enum type
* @returns Typescript data schema type in TS Node format
*/
const createEnums = (type: EnumType): ts.Node => {
const typeExpression = ts.factory.createCallExpression(
ts.factory.createIdentifier(`${TYPESCRIPT_DATA_SCHEMA_CONSTANTS.REFERENCE_A}.${TYPESCRIPT_DATA_SCHEMA_CONSTANTS.ENUM_METHOD}`),
undefined,
[
ts.factory.createArrayLiteralExpression(
type.values.map((value) => ts.factory.createStringLiteral(value)),
true,
),
],
);
return ts.factory.createPropertyAssignment(ts.factory.createIdentifier(toPascalCase([type.name])), typeExpression);
};

const createModel = (model: Model): ts.Node => {
const modelExpr = ts.factory.createCallExpression(
ts.factory.createIdentifier(`${TYPESCRIPT_DATA_SCHEMA_CONSTANTS.REFERENCE_A}.${TYPESCRIPT_DATA_SCHEMA_CONSTANTS.MODEL_METHOD}`),
Expand Down Expand Up @@ -147,19 +164,61 @@ export const createSchema = (schema: Schema, config?: DataSourceGenerateConfig):
throw new Error('No valid tables found. Make sure at least one table has a primary key.');
}

const nullableEnumFields = schema.getModels().map((model) =>
model
.getFields()
.filter((field) => field.type.kind === 'Enum')
.map((field) => {
if (field.type.kind === 'Enum') {
return createEnums(field.type);
} else {
return undefined;
}
}),
);

const requiredEnumFields = schema.getModels().map((model) =>
model
.getFields()
.filter((field) => field.type.kind === 'NonNull' && field.type.type.kind === 'Enum')
.map((field) => {
if (field.type.kind === 'NonNull' && field.type.type.kind === 'Enum') {
return createEnums(field.type.type);
} else {
return undefined;
}
}),
Comment on lines +184 to +190
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is checking field.type.kind and field.type.type.kind redundant because of the filter?

Suggested change
.map((field) => {
if (field.type.kind === 'NonNull' && field.type.type.kind === 'Enum') {
return createEnums(field.type.type);
} else {
return undefined;
}
}),
.map((field) => createEnums(field.type.type))

);

const models = schema
.getModels()
.filter((model) => model.getPrimaryKey())
.map((model) => {
return createModel(model);
});

const combinedEnums = nullableEnumFields.concat(requiredEnumFields).flat(); // making 1 D array

// to eliminate duplicate definition of enums in case where same enum is referenced in 2 differed models
const seenEnums = new Set<string>();
const uniqueEnums = combinedEnums.filter((node) => {
if (seenEnums.has(node['name'].escapedText)) {
return false;
} else {
seenEnums.add(node['name'].escapedText);
return true;
}
});

const modelsWithEnums = models.concat(uniqueEnums);

const tsSchema = ts.factory.createCallExpression(
ts.factory.createPropertyAccessExpression(
createConfigureExpression(schema, config),
ts.factory.createIdentifier(TYPESCRIPT_DATA_SCHEMA_CONSTANTS.SCHEMA_METHOD),
),
undefined,
[ts.factory.createObjectLiteralExpression(models as ts.ObjectLiteralElementLike[], true)],
[ts.factory.createObjectLiteralExpression(modelsWithEnums as ts.ObjectLiteralElementLike[], true)],
);
return ts.factory.createVariableStatement(
[exportModifier],
Expand Down
1 change: 1 addition & 0 deletions packages/graphql-transformer-common/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ export const TYPESCRIPT_DATA_SCHEMA_CONSTANTS: {
ARRAY_METHOD: string;
REQUIRED_METHOD: string;
STRING_METHOD: string;
REF_METHOD: string;
ENUM_METHOD: string;
REFERENCE_A: string;
EXPORT_VARIABLE_NAME: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const TYPESCRIPT_DATA_SCHEMA_CONSTANTS = {
ARRAY_METHOD: 'array',
REQUIRED_METHOD: 'required',
STRING_METHOD: 'string',
REF_METHOD: 'ref',
ENUM_METHOD: 'enum',
REFERENCE_A: 'a',
EXPORT_VARIABLE_NAME: 'schema',
Expand Down
Loading