Feature/@key #1463

mikeparisstuff · 2019-05-16T02:28:12Z

Issue #, if available:

Description of changes:

Still a work in progress. This is the initial work for custom indexes. I have a few more things to finish so please do not merge this yet. These features require substantial changes so I want to start the PR process earlier rather than later.

Complete:

Implement @key and handle updating DynamoDB table primary and secondary index structures.
Advanced @key validation.
Update auto-generated get, create, update, and delete operations for custom primary keys.
Update GraphQL schema to work with custom primary keys.
Auto-generate composite keys when indexing on 3 or more attributes for get, create, update, delete.
Tests for schema structural changes
Tests for get, create, update, delete operation with composite and simple keys.
Update list operation to work with custom primary keys.
Tests for list operations
Implement query resolver when instructed for secondary indexes.
Tests for query operations
Rework composite key condition input shape to be more logically consistent.
Write docs.
Handle unique partial composite key update in update operations using one of two strategies. 1. Pre-fetch key and allow partial updates. 2. Force update mutation to include all parts of composite key if any are included.

Work remaining:

?

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

ranguard · 2019-05-20T12:29:04Z

excited (sorry can't be technically constructive but wanted to show thanks)

…tructures. Still need to update list as well as implement the new query resolver using existing parts. The last change is to handle the partial key update scenario in update operations.

janhesters · 2019-05-25T12:55:03Z

Will there be a way to dynamically specify the keys client-side like so

API.graphql(graphqlOperation(listContacts, { limit: 100, nextToken, sortBy: { firstName: 'gte', lastName: 'gte' }));

? Or if that's not possible, will we be able to add multiple query fields like

type Contact 
  @model 
  @key(name: "ByOwnerLastNameFirstName", fields: ["owner", "lastName", "firstName"], queryField: "contactsByOwnerAndName")
  @key(name: "ByOwnerCityLastNameFirstName", fields: ["owner", "city", "lastName", "firstName"], queryField: "contactsByOnwerAndCity")
  @auth(rules: [{ allow: owner }]) 
{
  id: ID!
  firstName: String
  lastName: String
  city: String
  email: String
  telephone: String
  owner: String
}

?

…ite sort key integrity

mikeparisstuff · 2019-05-28T18:15:32Z

@janhesters Unfortunately there is no such thing as a dynamic sort by argument in DynamoDB but yes you are able to add multiple query fields exactly like you mentioned.

edit: I should mention that you can dictate the direction that an index is scanned/queried dynamically.

yuth · 2019-05-29T02:02:43Z

packages/graphql-key-transformer/src/KeyTransformer.ts

+            if (deleteResolver) {
+                deleteResolver.Properties.RequestMappingTemplate = joinSnippets([
+                    this.setKeySnippet(directive, true), 
+                    ensureCompositeKeySnippet(directive),


Do we need this? Since we don't support batch delete do we need to ensure composite key

Nope we do not. Removed.

yuth · 2019-05-29T02:02:43Z

packages/graphql-key-transformer/src/KeyTransformer.ts

+            const finalSortKeyField = definition.fields.find(f => f.name.value === finalSortKeyFieldName);
+            // All composite keys (length > 2) are casted to strings.
+            const sortKeyConditionInput = args.fields.length > 2 ? 
+                makeScalarKeyConditionForType(makeNamedType('String')) :


As discussed can we make this simpler as we know the fields.length is 2

kaustavghosh06

LGTM.

yuth · 2019-05-29T18:06:00Z

packages/graphql-transformers-e2e-tests/src/__tests__/KeyTransformer.e2e.test.ts

+    GRAPHQL_CLIENT = new GraphQLClient(endpoint, { 'x-api-key': apiKey })
+});
+
+// afterAll(async () => {


Can we have the cleanup back

yuth · 2019-05-29T18:06:00Z

testing-custom-indexes.md

@@ -0,0 +1,163 @@
+# Testing Custom Indexes


We don't need this anymore right

yuth · 2019-05-29T18:06:00Z

packages/graphql-transformers-e2e-tests/src/__tests__/KeyTransformerLocal.e2e.test.ts

+import ModelTransformer from 'graphql-dynamodb-transformer';
+import KeyTransformer from 'graphql-key-transformer';
+import { parse, FieldDefinitionNode, ObjectTypeDefinitionNode } from 'graphql';
+import { expectArguments, expectFields, expectNonNullFields, expectNullableFields } from '../testUtil';


Are we using expectFields?

yuth · 2019-05-29T18:06:00Z

packages/graphql-transformers-e2e-tests/src/testUtil.ts

+} from 'graphql';
+import { isNonNullType } from 'graphql-transformer-common';
+
+export function expectFields(type: ObjectTypeDefinitionNode, fields: string[]) {


Extending the matcher would be a better approach. More on this at jest docs

janhesters · 2019-06-08T20:35:32Z

@mikeparisstuff

I should mention that you can dictate the direction that an index is scanned/queried dynamically.

How? 😊

github-actions · 2021-05-25T13:16:27Z

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels for those types of questions.

ghost assigned mikeparisstuff May 16, 2019

ghost added the review label May 16, 2019

wizawuza mentioned this pull request May 16, 2019

Filtering on weeknumbers greater then 10 doesn't return results #1458

Closed

kaustavghosh06 mentioned this pull request May 21, 2019

Cannot perform more than one GSI creation or deletion in a single update; ConnectionStack failed to update #922

Closed

mikeparisstuff force-pushed the feature/@key branch from 8ec1aac to bf44796 Compare May 22, 2019

Paris and others added 11 commits May 23, 2019

Initial @key implementation

69235b1

Adding support for GSI/LSIs with @key.

913dc5e

Adding support for create, update, delete, & get with updated index s…

ec845e4

…tructures. Still need to update list as well as implement the new query resolver using existing parts. The last change is to handle the partial key update scenario in update operations.

Adding support for listX.

09659ef

Adding custom-index test steps and fixing lint issue

6282990

Updating testing steps

b4a933c

Update testing-custom-indexes.md

874e4c3

Updating instructions

ad832bc

Adding @auth support for @key queryFields and adding more tests

d6c6c65

pulling testing instruction updates

3a75db6

Rearranging for logical consistency

76d3574

mikeparisstuff force-pushed the feature/@key branch from e0aed5f to 76d3574 Compare May 23, 2019

Paris added 8 commits May 23, 2019

Fixing compilation errors

af7e793

fixing compilation errors

05926d1

Fixing TSC errors

3eb6e74

Moving to new sort key structure and fixing tests

7d2b128

Fixing circle ci issue

9c8ea2a

Fixing test

05deecc

Fixing circleci build error

02f7360

Adding updated auth snapshots

d06c677

mikeparisstuff mentioned this pull request May 24, 2019

Using an exisiting AppSync API w/ Amplify #1516

Closed

mikeparisstuff mentioned this pull request May 24, 2019

Sort single field API #1502

Closed

Adding update mutation validation to secondary keys to protect compos…

7820a1b

…ite sort key integrity

Paris added 2 commits May 28, 2019

Fixing list with filter operations

7f08229

Updating snapshots

5fcb2ac

yuth reviewed May 29, 2019

View changes

Addressing PR comments

9096677

kaustavghosh06 approved these changes May 29, 2019

View changes

kaustavghosh06 merged commit 00ed819 into aws-amplify:master May 29, 2019
2 checks passed

yuth reviewed May 29, 2019

View changes

github-actions bot locked as resolved and limited conversation to collaborators May 25, 2021

Mar	APR	May
	24
2021	2022	2023

aws-amplify / amplify-cli Public

Feature/@key #1463

Feature/@key #1463

mikeparisstuff commented May 16, 2019 •

edited

ranguard commented May 20, 2019

janhesters commented May 25, 2019

mikeparisstuff commented May 28, 2019 •

edited

yuth May 29, 2019

mikeparisstuff May 29, 2019

yuth May 29, 2019

mikeparisstuff May 29, 2019

kaustavghosh06 left a comment

yuth May 29, 2019

yuth May 29, 2019

yuth May 29, 2019

yuth May 29, 2019

janhesters commented Jun 8, 2019

github-actions bot commented May 25, 2021

aws-amplify / amplify-cli Public

Feature/@key #1463

Feature/@key #1463

Conversation

mikeparisstuff commented May 16, 2019 • edited

ranguard commented May 20, 2019

janhesters commented May 25, 2019

mikeparisstuff commented May 28, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaustavghosh06 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janhesters commented Jun 8, 2019

github-actions bot commented May 25, 2021

mikeparisstuff commented May 16, 2019 •

edited

mikeparisstuff commented May 28, 2019 •

edited