Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -245,6 +245,31 @@ export const schema = configure({
"
`;

exports[`Type name conversions should annotate scalar int fields with existing default with \`.default()\` 1`] = `
"/* eslint-disable */
/* THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. */
import { a } from \\"@aws-amplify/data-schema\\";
import { configure } from \\"@aws-amplify/data-schema/internals\\";
import { secret } from \\"@aws-amplify/backend\\";

export const schema = configure({
database: {
identifier: \\"ID1234567890\\",
engine: \\"postgresql\\",
connectionUri: secret(\\"CONN_STR\\")
}
}).schema({
\\"CoffeeQueue\\": a.model({
id: a.integer().default(),
name: a.string(),
orderNumber: a.integer().default()
Comment on lines +262 to +265
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the desired behavior of this PR.

}).identifier([
\\"id\\"
])
});
"
`;

exports[`Type name conversions ts schema generator should invoke generate schema 1`] = `
"/* eslint-disable */
/* THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,19 @@ type Story @refersTo(name: \\"stories\\") @model {
"
`;

exports[`generate graphqlSchemaFromSQLSchema creates graphql schema from "postgres" "serial" schema 1`] = `
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This snapshot is not intentional as we are not intending to support GQL generation here. It is generated automatically to test against anyway from the schemas in __utils__.

"input AMPLIFY {
engine: String = \\"postgres\\"
}


type SerialTable @refersTo(name: \\"serial_table\\") @model {
id: Int! @primaryKey
number: Int
}
"
`;

exports[`generate graphqlSchemaFromSQLSchema creates graphql schema from "postgres" "todo" schema 1`] = `
"input AMPLIFY {
engine: String = \\"postgres\\"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,52 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`testPostgresStringDataSourceAdapter annotates the model field correctly from a schema with a SERIAL field 1`] = `
Array [
Model {
"fields": Array [
Object {
"default": Object {
"kind": "DB_GENERATED",
"value": "nextval('serial_table_id_seq'::regclass)",
},
"length": "Null",
"name": "id",
"type": Object {
"kind": "NonNull",
"type": Object {
"kind": "Scalar",
"name": "Int",
},
},
},
Object {
"default": Object {
"kind": "DB_GENERATED",
"value": "nextval('serial_table_number_seq'::regclass)",
},
"length": "Null",
"name": "number",
"type": Object {
"kind": "NonNull",
"type": Object {
"kind": "Scalar",
"name": "Int",
},
},
},
],
"indexes": Array [],
"name": "serial_table",
"primaryKey": Index {
"fields": Array [
"id",
],
"name": "PRIMARY_KEY",
},
},
]
`;

exports[`testPostgresStringDataSourceAdapter sets the correct models from a news schema 1`] = `
Array [
Model {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,5 +155,15 @@ comments,comment_text,NULL,4,varchar,varchar(30),NO,30,NULL,NULL,NULL,NULL`,
"stories",NULL,NULL,"placeholder",NULL,3,"character","bpchar","NO",1,NULL,NULL,NULL
"stories",NULL,NULL,"pub_id",NULL,1,"integer","int4","NO",NULL,"stories_pkey","PRIMARY KEY","pub_id, pub_type"
"stories",NULL,NULL,"pub_type","'S'::bpchar",2,"character","bpchar","NO",1,"stories_pkey","PRIMARY KEY","pub_id, pub_type"`,
/*
* CREATE TABLE serial_table (
* id SERIAL PRIMARY KEY,
* number SERIAL,
* );
*/
serial: `"table_name","enum_name","enum_values","column_name","column_default","ordinal_position","data_type","udt_name","is_nullable","character_maximum_length","indexname","constraint_type","index_columns"
"serial_table","Null","Null","id","nextval('serial_table_id_seq'::regclass)","1","integer","int4","NO","Null","serial_table_pkey","PRIMARY KEY","id"
"serial_table","Null","Null","number","nextval('serial_table_number_seq'::regclass)","2","integer","int4","NO","Null","Null","Null",
`,
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,108 @@ describe('Type name conversions', () => {
expect(graphqlSchema).toMatchSnapshot();
});

it.each([
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These cases are not exhaustive over every Field that could exist but cover the most unique cases.

{
case: 'string',
field: () => {
const f = new Field('field', { kind: 'Scalar', name: 'String' });
f.default = { kind: 'DB_GENERATED', value: 'A squat grey building of only thirty-four stouries' };
return f;
},
},
{
case: 'Float',
field: () => {
const f = new Field('field', { kind: 'Scalar', name: 'Float' });
f.default = { kind: 'DB_GENERATED', value: 3.14 };
return f;
},
},
{
case: 'List',
field: () => {
const f = new Field('field', { kind: 'List', type: { kind: 'Scalar', name: 'String' } });
f.default = { kind: 'DB_GENERATED', value: false };
return f;
},
},
{
case: 'CustomType',
field: () => {
const f = new Field('field', { kind: 'Custom', name: 'MyCustomType' });
f.default = { kind: 'DB_GENERATED', value: 'I could make of both names nothing longer or more explicit than Pip' };
return f;
},
},
{
case: 'Transformer Generated',
field: () => {
const f = new Field('field', { kind: 'Scalar', name: 'Int' });
f.default = { kind: 'TRANSFORMER_GENERATED', value: 42 };
return f;
},
},
{
case: 'Default Integer Constant',
field: () => {
const f = new Field('field', { kind: 'Scalar', name: 'Int' });
f.default = { kind: 'DB_GENERATED', value: 42 };
return f;
},
},
{
case: 'No default',
field: () => new Field('field', { kind: 'Scalar', name: 'Int' }),
},
])('should not annotate fields with `.default()` where we do not support db generation (case: %case)', (test) => {
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.setPrimaryKey(['id']);

model.addField(test.field());

dbschema.addModel(model);
const config: DataSourceGenerateConfig = {
identifier: 'ID1234567890',
secretNames: {
connectionUri: 'CONN_STR',
},
};

const graphqlSchema = generateTypescriptDataSchema(dbschema, config);
const containsDefault = graphqlSchema.includes('default()');
expect(containsDefault).toBe(false);
});

it('should annotate scalar int fields with existing default with `.default()`', async () => {
const dbschema = new Schema(new Engine('Postgres'));

const model = new Model('CoffeeQueue');

const serialPKField = new Field('id', { kind: 'NonNull', type: { kind: 'Scalar', name: 'Int' } });
serialPKField.default = { kind: 'DB_GENERATED', value: "nextval('coffeequeue_id_seq'::regclass)" };
model.addField(serialPKField);
model.setPrimaryKey(['id']);

model.addField(new Field('name', { kind: 'Scalar', name: 'String' }));

const serialField = new Field('orderNumber', { kind: 'Scalar', name: 'Int' });
serialField.default = { kind: 'DB_GENERATED', value: "nextval('coffeequeue_ordernumber_seq'::regclass)" };
model.addField(serialField);

dbschema.addModel(model);
const config: DataSourceGenerateConfig = {
identifier: 'ID1234567890',
secretNames: {
connectionUri: 'CONN_STR',
},
};

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

it('schema with database config without 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 @@ -131,6 +131,11 @@ describe('testPostgresStringDataSourceAdapter', () => {
expect(adapter.getModels()).toMatchSnapshot();
});

it('annotates the model field correctly from a schema with a SERIAL field', () => {
const adapter = new PostgresStringDataSourceAdapter(schemas.postgres.serial);
expect(adapter.getModels()).toMatchSnapshot();
});

it('sets the correct models from a news schema', () => {
const adapter = new PostgresStringDataSourceAdapter(schemas.postgres.news);
expect(adapter.getModels()).toMatchSnapshot();
Expand Down
6 changes: 3 additions & 3 deletions packages/amplify-graphql-schema-generator/src/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import { DocumentNode } from 'graphql';
import { Schema, Engine } from './schema-representation';
import { generateGraphQLSchema } from './schema-generator';
import { constructRDSGlobalAmplifyInput } from './input';
import { MySQLStringDataSourceAdapter, PostgresStringDataSourceAdapter } from './datasource-adapter';
import { MySQLStringDataSourceAdapter, PostgresStringDataSourceAdapter, StringDataSourceAdapter } from './datasource-adapter';

const buildSchemaFromString = (stringSchema: string, engineType: ImportedRDSType): Schema => {
let schema;
let adapter;
let schema: Schema;
let adapter: StringDataSourceAdapter;
switch (engineType) {
case ImportedRDSType.MYSQL:
adapter = new MySQLStringDataSourceAdapter(stringSchema);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import ts from 'typescript';
import { TYPESCRIPT_DATA_SCHEMA_CONSTANTS } from 'graphql-transformer-common';
import { VpcConfig } from '@aws-amplify/graphql-transformer-interfaces';
import { DBEngineType, Field, FieldType, Model, Schema } from '../schema-representation';
import { DBEngineType, Field, Model, Schema } from '../schema-representation';

const GQL_TYPESCRIPT_DATA_SCHEMA_TYPE_MAP = {
string: 'string',
Expand All @@ -27,32 +27,45 @@ const GQL_TYPESCRIPT_DATA_SCHEMA_TYPE_MAP = {
* @returns Typescript data schema property in TS Node format
*/
const createProperty = (field: Field): ts.Node => {
const typeExpression = createDataType(field.type);
const typeExpression = createDataType(field);
return ts.factory.createPropertyAssignment(ts.factory.createIdentifier(field.name), typeExpression as ts.Expression);
};

/**
* Creates a typescript data schema type from internal SQL schema representation
* Example typescript data schema type output: `a.string().required()`
* @param type SQL IR field type
* @param field SQL IR field
* @returns Typescript data schema type in TS Node format
*/
const createDataType = (type: FieldType): ts.Node => {
if (type.kind === 'Scalar') {
const createDataType = (field: Field): ts.Node => {
if (isSequenceField(field)) {
const baseTypeExpression =
field.type.kind === 'NonNull'
? createDataType(new Field(field.name, field.type.type))
: createDataType(new Field(field.name, field.type));

return ts.factory.createCallExpression(
ts.factory.createPropertyAccessExpression(baseTypeExpression as ts.Expression, TYPESCRIPT_DATA_SCHEMA_CONSTANTS.DEFAULT_METHOD),
undefined,
undefined,
);
}

if (field.type.kind === 'Scalar') {
return ts.factory.createCallExpression(
ts.factory.createIdentifier(`${TYPESCRIPT_DATA_SCHEMA_CONSTANTS.REFERENCE_A}.${getTypescriptDataSchemaType(type.name)}`),
ts.factory.createIdentifier(`${TYPESCRIPT_DATA_SCHEMA_CONSTANTS.REFERENCE_A}.${getTypescriptDataSchemaType(field.type.name)}`),
undefined,
undefined,
);
}

if (type.kind === 'Enum') {
if (field.type.kind === 'Enum') {
return 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)),
field.type.values.map((value) => ts.factory.createStringLiteral(value)),
true,
),
],
Expand All @@ -61,7 +74,7 @@ const createDataType = (type: FieldType): ts.Node => {

// We do not import any Database type as 'Custom' type.
// In case if there is a custom type in the IR schema, we will import it as string.
if (type.kind === 'Custom') {
if (field.type.kind === 'Custom') {
return ts.factory.createCallExpression(
ts.factory.createIdentifier(`${TYPESCRIPT_DATA_SCHEMA_CONSTANTS.REFERENCE_A}.${TYPESCRIPT_DATA_SCHEMA_CONSTANTS.STRING_METHOD}`),
undefined,
Expand All @@ -70,14 +83,21 @@ const createDataType = (type: FieldType): ts.Node => {
}

// List or NonNull
const modifier = type.kind === 'List' ? TYPESCRIPT_DATA_SCHEMA_CONSTANTS.ARRAY_METHOD : TYPESCRIPT_DATA_SCHEMA_CONSTANTS.REQUIRED_METHOD;
const modifier =
field.type.kind === 'List' ? TYPESCRIPT_DATA_SCHEMA_CONSTANTS.ARRAY_METHOD : TYPESCRIPT_DATA_SCHEMA_CONSTANTS.REQUIRED_METHOD;
const unwrappedField = new Field(field.name, field.type.type);
return ts.factory.createCallExpression(
ts.factory.createPropertyAccessExpression(createDataType(type.type) as ts.Expression, ts.factory.createIdentifier(modifier)),
ts.factory.createPropertyAccessExpression(createDataType(unwrappedField) as ts.Expression, ts.factory.createIdentifier(modifier)),
undefined,
undefined,
);
};

const isSequenceField = (field: Field): boolean => {
const sequenceRegex = /^nextval\(.+::regclass\)$/;
return field.default?.kind === 'DB_GENERATED' && sequenceRegex.test(field.default.value.toString());
Comment on lines +97 to +98
Copy link
Member

Choose a reason for hiding this comment

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

(Copying this from main conversation comment)

Can we find documentation that this is the canonical way to determine whether a field is serial? In the past we've relied on experimentally-verified "rules" that turned out to be incomplete, so I'd love to get ahead of that this time.

Copy link
Contributor Author

@p5quared p5quared Oct 21, 2024

Choose a reason for hiding this comment

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

In summary we cannot determine if a column was declared serial, we can identify sequences though, and from this, infer serial fields.

Ultimately serial doesn't really exist as a datatype we can directly introspect. In PG it is basically a macro for "Create a column as an integer and create a sequence to back that column as a default". We end up with an ordinary non-null integer with a special default value.

We can tell if a column is backed by a sequence since its default will be nextval(_), one of the special sequence functions.

Reference.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the thorough explanation!

};

const getTypescriptDataSchemaType = (type: string): string => {
const DEFAULT_DATATYPE = TYPESCRIPT_DATA_SCHEMA_CONSTANTS.STRING_METHOD;
const tsDataSchemaType = GQL_TYPESCRIPT_DATA_SCHEMA_TYPE_MAP[type.toLowerCase()];
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 @@ -512,6 +512,7 @@ export const TYPESCRIPT_DATA_SCHEMA_CONSTANTS: {
IDENTIFIER_METHOD: string;
ARRAY_METHOD: string;
REQUIRED_METHOD: string;
DEFAULT_METHOD: string;
STRING_METHOD: string;
ENUM_METHOD: string;
REFERENCE_A: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export const TYPESCRIPT_DATA_SCHEMA_CONSTANTS = {
IDENTIFIER_METHOD: 'identifier',
ARRAY_METHOD: 'array',
REQUIRED_METHOD: 'required',
DEFAULT_METHOD: 'default',
STRING_METHOD: 'string',
ENUM_METHOD: 'enum',
REFERENCE_A: 'a',
Expand Down
Loading