feat: auto increment support #2883

p5quared · 2024-09-18T01:25:53Z

Description of changes

Transformer changes to support auto increment (serial) fields on Postgres datasources. Accomplished by making the 'create' input types optional when @default is applied.

Definition of @default has changed, its value: String parameter is now optional.
default-value-transformer allows no value on Int fields if the datasource is Postgres
Unit tests for the above

CDK / CloudFormation Parameters Changed

n/a

Description of how you validated changes

Unit tests, existing e2e tests, testTransform. Will write e2e test after implementing changes to Lambda layer.

Checklist

PR description included
yarn test passes
Tests are changed or added
Relevant documentation is changed or added (and PR referenced)

Codebuild Tests

Ran against beta via yarn split-e2e-tests-beta.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

...l-default-value-transformer/src/__tests__/amplify-grapphql-default-value-transformer.test.ts

packages/amplify-graphql-api-construct-tests/src/__tests__/sql-pg-auto-increment.test.ts

packages/amplify-graphql-api-construct-tests/src/sql-tests-common/sql-models-auto-increment.ts

palpatim · 2024-10-01T19:01:08Z

packages/amplify-graphql-api-construct-tests/src/sql-tests-common/sql-models-auto-increment.ts

+        coffeeQueueTableCRUDLHelper.checkGenericError(error?.message);
+      }
+
+      const invalidOrderNumber = 99999999;


I assume this is invalid because it exceeds the maximum value of a SERIAL field? If so, worth noting that in a comment, in case we start supporting PG versions that have a higher limit.

Actually it is invalid because it is greater than any existing record's orderNumber. That purpose is definitely not clear since even the number 4 should be invalid. I wrote some comments but then opted to use a more descriptive variable name:

const biggerThanAnyExistingOrderNumber = 99999999;

palpatim · 2024-10-01T19:18:54Z

...l-default-value-transformer/src/__tests__/amplify-grapphql-default-value-transformer.test.ts

+          transformers: [new ModelTransformer(), new DefaultValueTransformer(), new PrimaryKeyTransformer()],
+          dataSourceStrategies: constructDataSourceStrategies(schema, strategy),
+        });
+      }).toThrow('The @default directive requires a value property on non Postgres datasources.');


Can you confirm that a @default(value: ...) works as expected on a mysql data source? (That is, it passes specified value in the input if the mutation input doesn't include it; and it allows overrides of the default if it IS specified). Best way to confirm that would be a test (possibly an already existing one!)

Do we have a test (possibly an already existing one!) to assert that @default(value:...) works for postgres?

The only e2e usage of @default I could find only used DDB. I modified the general sql models test to utilize @default(value) which should verify behavior on both sources.

packages/amplify-graphql-default-value-transformer/src/graphql-default-value-transformer.ts

packages/amplify-graphql-transformer-core/src/index.ts

packages/amplify-graphql-transformer-core/src/utils/model-datasource-strategy-utils.ts

...l-default-value-transformer/src/__tests__/amplify-grapphql-default-value-transformer.test.ts

…ement support

Co-authored-by: Tim Schmelter <[email protected]>

… values

p5quared · 2024-10-07T21:22:45Z

Changes since approval:

Tiny update/refactor to E2E since chore: migrate pg array objects e2e test in gen2 cdk #2906.
Ignore types.ts from @default test coverage and add a test case it('cannot use null as a default value') (on DynamoDB) to hit coverage requirements.

p5quared · 2024-10-07T22:01:29Z

packages/amplify-graphql-api-construct-tests/src/sql-tests-common/sql-models-auto-increment.ts

+        },
+      });
+
+      coffeeQueueTableCRUDLHelper = new CRUDLHelper(appSyncClient, 'CoffeeQueue', 'CoffeeQueues', coffeeQueueFieldMap);


Since previous approval, we now use a FieldMap instead of a list of fields.

p5quared · 2024-10-07T22:02:03Z

...l-default-value-transformer/src/__tests__/amplify-grapphql-default-value-transformer.test.ts

+  it('cannot use `null` as a default value', () => {
+    const schema = `
+      type CoffeeQueue @model {
+        id: ID! @primaryKey
+        orderNumber: Int! @default(value: null)
+        name: String
+      }`;
+
+    expect(() => {
+      testTransform({
+        schema: schema,
+        transformers: [new ModelTransformer(), new DefaultValueTransformer(), new PrimaryKeyTransformer()],
+      });
+    }).toThrow('The @default directive does not support null values.');
+  });


Also added this test case to hit coverage requirements.

@default

* chore: update .jsii assembly * chore: update .jsii assembly * chore: migrate pg array objects e2e test in gen2 cdk (#2906) * chore: graphql prep for test migration * refactor: generic graphql field selection string with fieldmap * feat: add postgres array objects e2e test * test: remove bootstrap in test code * chore: schema cleanup * chore: final cleanup * chore: add explanation on FieldMap ans examples * chore: remove dup test --------- Signed-off-by: Kevin Shan <[email protected]> Co-authored-by: Tim Schmelter <[email protected]> * fix(model-transformer) IndexName -> index in query list resolver (#2912) * chore: upgrade cdk library dependency to 2.158.0 (#2876) * chore: upgrade cdk dependency to 2.158.0 * chore: install and use nvm * chore: use full version for nvm * chore: testing linux build with nvm * chore: fix version in cdk tests * chore: update jsii files * update: increase memory size * add: debug statement * update: mem size back to 8096, use ps1 file for shell script * fix: path to Setup-NodeVersion.ps1 * fix: path to codebuild_specs/Setup-NodeVersion.ps1 * add: set runtime version * update: image * add: debug statement * update: use earlier code * add: debug statements * update: clean up code * update: use the correct image * add: list installed node versions and used nodejs.install * restart: install nvm using choco * add: back mem size variable * add: nvm install and use 18.20.4 * add: env var NVM_HOME and NVM_SYMLINK * add: spawn powershell as admin * update: remove all other builds * add: debug statement * add: env var path * update: print env var * add: commands * update: env var set up * add: refresh env var * update: more debug statement * update * revamp: find nvm.exe * update: install nvm windows directly * update: launch new shell if current shell does not recognize nvm * update: install node in buildspec * add: install and use node in build spec * update: use single quote to prevent interpreting \ * add: 2 scripts, one for installing nvm, another for using nvm * fix: path error * test: which way set env var * update: set up env var in pre_build * update: use choco in pre-build * fix: syntax error * update: build_windows working, running all tests * test: remove bootstrap in test code * debug: _runGqlE2ETests * update: debug_workflow * update: debug_workflow * update: debug_workflow * update: debug_workflow * add: debug statement * add: debug test * add: debug * update: use uuid for bucket name * remove: use of uuid * add: debug statement * update: use differrent bucket name * add: mili second timestamp * add: debug statement * remove: debug statement * remove: redundant code --------- Co-authored-by: Bobby Yu <[email protected]> Co-authored-by: Tim Schmelter <[email protected]> * test: fix gen 1 init (#2924) * fix(conversation): allow changes to systemPrompt, inferenceConfig, aiModel to be hotswapped (#2923) * feat: auto increment support (#2883) * chore(graphql-default-value-transformer): tidy tests * test(graphql-default-value-transformer): add unit tests for auto increment support * feat: 🎸 utils to detect Postgres datasource * feat: 🎸 support auto increment Implements support for auto increment (serial) fields from Postgres datasources. Such fields are denoted by an empty `@default` applied to an `Int` field. * test(graphql-default-value-transformer): pk can be auto increment * test(graphql-default-value-transformer): auto-increment crud e2e * chore: describe test purpose * chore: removing logging * chore: describe why invalid cases are invalid * chore: remove unecessary e2e test case * chore: test messaging clarity * chore: type safety * chore: alphabetize list * chore: type of return value asserts against string Co-authored-by: Tim Schmelter <[email protected]> * chore: test ensures customers can insert to serial fields with custom values * chore: verify that @default(value) works on mysql * chore: remove unecessary ssm test case * chore: update branch from main * test: value cannot be null on ddb --------- Co-authored-by: Tim Schmelter <[email protected]> * fix(conversation): use functionMap for custom handler IFunction reference (#2922) * fix(generation): gracefully handle stringified tool_use responses (#2919) * feat(conversation): per message items and lambda history retrieval pattern (#2914) * fix: sql default value e2e failures (#2932) * fix(generation): remove trailing comma in inferenceConfig resolver code (#2933) * fix: add aws_iam to custom operations when enableIamAuthorization is enabled; fix graphql type utils (#2921) - test: Add additional tests to fix coverage metrics for unchanged files - test: Add implicit IAM auth support tests - Added a skipped test for custom type support, to be re-enabled once we figure out the right strategy for this. * fix: appsync ttl correct duration time unit in ms (#2928) Signed-off-by: Kevin Shan <[email protected]> --------- Signed-off-by: Kevin Shan <[email protected]> Co-authored-by: amplify-data-ci <[email protected]> Co-authored-by: Kevin Shan <[email protected]> Co-authored-by: Ian Saultz <[email protected]> Co-authored-by: Phani Srikar Edupuganti <[email protected]> Co-authored-by: Bobby Yu <[email protected]> Co-authored-by: Dane Pilcher <[email protected]> Co-authored-by: Peter V. <[email protected]>

@default

* chore(graphql-default-value-transformer): tidy tests * test(graphql-default-value-transformer): add unit tests for auto increment support * feat: 🎸 utils to detect Postgres datasource * feat: 🎸 support auto increment Implements support for auto increment (serial) fields from Postgres datasources. Such fields are denoted by an empty `@default` applied to an `Int` field. * test(graphql-default-value-transformer): pk can be auto increment * test(graphql-default-value-transformer): auto-increment crud e2e * chore: describe test purpose * chore: removing logging * chore: describe why invalid cases are invalid * chore: remove unecessary e2e test case * chore: test messaging clarity * chore: type safety * chore: alphabetize list * chore: type of return value asserts against string Co-authored-by: Tim Schmelter <[email protected]> * chore: test ensures customers can insert to serial fields with custom values * chore: verify that @default(value) works on mysql * chore: remove unecessary ssm test case * chore: update branch from main * test: value cannot be null on ddb --------- Co-authored-by: Tim Schmelter <[email protected]>

p5quared added the @default label Sep 18, 2024

p5quared force-pushed the auto-increment-support branch 2 times, most recently from 9922afd to c0bc7ff Compare September 18, 2024 18:30

p5quared changed the title ~~Auto increment support~~ feat: auto increment support Sep 18, 2024

p5quared marked this pull request as ready for review September 18, 2024 18:49

p5quared requested review from a team as code owners September 18, 2024 18:49

p5quared commented Sep 18, 2024

View reviewed changes

...l-default-value-transformer/src/__tests__/amplify-grapphql-default-value-transformer.test.ts Show resolved Hide resolved

p5quared force-pushed the auto-increment-support branch 2 times, most recently from 7ddef03 to 376fa27 Compare September 18, 2024 21:55

sundersc previously approved these changes Sep 23, 2024

View reviewed changes

p5quared force-pushed the auto-increment-support branch 2 times, most recently from 29a8c20 to 3896d20 Compare September 30, 2024 16:45

p5quared dismissed sundersc’s stale review via de142bd October 1, 2024 17:15

p5quared force-pushed the auto-increment-support branch 4 times, most recently from 5235728 to 9708662 Compare October 1, 2024 18:08

p5quared requested a review from sundersc October 1, 2024 18:12

p5quared force-pushed the auto-increment-support branch 2 times, most recently from 9d796c6 to ead5e20 Compare October 1, 2024 19:20

palpatim reviewed Oct 1, 2024

View reviewed changes

p5quared force-pushed the auto-increment-support branch 4 times, most recently from 2445213 to 6dd3b50 Compare October 2, 2024 18:50

palpatim previously approved these changes Oct 2, 2024

View reviewed changes

...l-default-value-transformer/src/__tests__/amplify-grapphql-default-value-transformer.test.ts Show resolved Hide resolved

sundersc previously approved these changes Oct 4, 2024

View reviewed changes

p5quared added 2 commits October 7, 2024 13:33

chore(graphql-default-value-transformer): tidy tests

30b3d61

test(graphql-default-value-transformer): add unit tests for auto incr…

316029f

…ement support

p5quared added 8 commits October 7, 2024 13:33

test(graphql-default-value-transformer): auto-increment crud e2e

56e93b9

chore: describe test purpose

cc39a40

chore: removing logging

634f6e7

chore: describe why invalid cases are invalid

a9cb7c4

chore: remove unecessary e2e test case

4fabed6

chore: test messaging clarity

f5d03f6

chore: type safety

dca85b4

chore: alphabetize list

d95e101

p5quared dismissed stale reviews from sundersc and palpatim via c24617c October 7, 2024 20:33

p5quared force-pushed the auto-increment-support branch from 6dd3b50 to c24617c Compare October 7, 2024 20:33

p5quared and others added 6 commits October 7, 2024 14:11

chore: type of return value asserts against string

e8ad58b

Co-authored-by: Tim Schmelter <[email protected]>

chore: test ensures customers can insert to serial fields with custom…

dacb206

… values

chore: verify that @default(value) works on mysql

a9db3a6

chore: remove unecessary ssm test case

ebdae2e

chore: update branch from main

f78fd03

test: value cannot be null on ddb

cb3156b

p5quared force-pushed the auto-increment-support branch from c24617c to cb3156b Compare October 7, 2024 21:12

p5quared self-assigned this Oct 7, 2024

sundersc approved these changes Oct 7, 2024

View reviewed changes

p5quared commented Oct 7, 2024

View reviewed changes

palpatim approved these changes Oct 7, 2024

View reviewed changes

bobbyu99 merged commit 4e66ac3 into main Oct 7, 2024
7 checks passed

bobbyu99 deleted the auto-increment-support branch October 7, 2024 22:27

p5quared mentioned this pull request Oct 8, 2024

fix: sql default value e2e failures #2932

Merged

4 tasks

p5quared mentioned this pull request Nov 4, 2024

doc(data): support for auto increment (serial) fields aws-amplify/docs#8074

Closed

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: auto increment support #2883

feat: auto increment support #2883

p5quared commented Sep 18, 2024 •

edited

Loading

palpatim Oct 1, 2024

p5quared Oct 1, 2024

palpatim Oct 1, 2024

p5quared Oct 1, 2024

p5quared commented Oct 7, 2024

p5quared Oct 7, 2024

p5quared Oct 7, 2024

feat: auto increment support #2883

feat: auto increment support #2883

Conversation

p5quared commented Sep 18, 2024 • edited Loading

Description of changes

CDK / CloudFormation Parameters Changed

Description of how you validated changes

Checklist

Codebuild Tests

palpatim Oct 1, 2024

Choose a reason for hiding this comment

p5quared Oct 1, 2024

Choose a reason for hiding this comment

palpatim Oct 1, 2024

Choose a reason for hiding this comment

p5quared Oct 1, 2024

Choose a reason for hiding this comment

p5quared commented Oct 7, 2024

p5quared Oct 7, 2024

Choose a reason for hiding this comment

p5quared Oct 7, 2024

Choose a reason for hiding this comment

p5quared commented Sep 18, 2024 •

edited

Loading