amplify-category-api icon indicating copy to clipboard operation
amplify-category-api copied to clipboard

Major release breaks GQL queries with composite identifiers

Open JJK801 opened this issue 1 year ago • 5 comments

Environment information

System:
  OS: macOS 13.3.1
  CPU: (10) arm64 Apple M2 Pro
  Memory: 185.75 MB / 16.00 GB
  Shell: /bin/zsh
Binaries:
  Node: 20.0.0 - ~/.nvm/versions/node/v20.0.0/bin/node
  Yarn: 4.1.0 - ~/.nvm/versions/node/v20.0.0/bin/yarn
  npm: 9.6.4 - ~/.nvm/versions/node/v20.0.0/bin/npm
  pnpm: undefined - undefined
NPM Packages:
  @aws-amplify/backend: Not Found
  @aws-amplify/backend-cli: Not Found
  aws-amplify: Not Found
  aws-cdk: Not Found
  aws-cdk-lib: Not Found
  typescript: 5.4.5
AWS environment variables:
  AWS_STS_REGIONAL_ENDPOINTS = regional
  AWS_NODEJS_CONNECTION_REUSE_ENABLED = 1
  AWS_SDK_LOAD_CONFIG = 1
No CDK environment variables

Description

After a few weeks without coding, i upgraded my app to the brand new 1.0.0 version, everything goes well on the backend side (congrats to the team for the very useful new features), but when running my app on the frontend i got errors because of:

Validation error of type UnknownArgument: Unknown field argument companyMembershipCompanyCountryCode @ 'listCompanyMemberships'

After some investigation, it seems that generateClient stuff doesn't behaves like codegen/appsync when dealing with composite identifiers.

Frontend generated document:

query(
    $companyMembershipProfileUserId: String,
    $sortDirection: ModelSortDirection,
    $companyMembershipCompanyCountryCode: ModelStringKeyConditionInput,
    $companyMembershipCompanyLegalId: ModelStringKeyConditionInput,
    $filter: ModelCompanyMembershipFilterInput,
    $limit: Int,
    $nextToken: String
) {
    listCompanyMemberships(
        companyMembershipProfileUserId: $companyMembershipProfileUserId,
        sortDirection: $sortDirection,
        companyMembershipCompanyCountryCode: $companyMembershipCompanyCountryCode,
        companyMembershipCompanyLegalId: $companyMembershipCompanyLegalId,
        filter: $filter,
        limit: $limit,
        nextToken: $nextToken
    ) {
        items {
            companyMembershipProfileUserId
            companyMembershipCompanyCountryCode
            companyMembershipCompanyLegalId
            companyMembershipSubscribedPlanId
            createdAt
            updatedAt
        }
        nextToken
        __typename
    }
}

Codegen/Appsync Document

query ListCompanyMemberships(
  $companyMembershipCompanyCountryCodeCompanyMembershipCompanyLegalId: ModelCompanyMembershipPrimaryCompositeKeyConditionInput
  $companyMembershipProfileUserId: String
  $filter: ModelCompanyMembershipFilterInput
  $limit: Int
  $nextToken: String
  $sortDirection: ModelSortDirection
) {
  listCompanyMemberships(
    companyMembershipCompanyCountryCodeCompanyMembershipCompanyLegalId: $companyMembershipCompanyCountryCodeCompanyMembershipCompanyLegalId
    companyMembershipProfileUserId: $companyMembershipProfileUserId
    filter: $filter
    limit: $limit
    nextToken: $nextToken
    sortDirection: $sortDirection
  ) {
    items {
      companyMembershipCompanyCountryCode
      companyMembershipCompanyLegalId
      companyMembershipProfileUserId
      companyMembershipSubscribedPlanId
      createdAt
      updatedAt
      __typename
    }
    nextToken
    __typename
  }
}

As you can see, my Company identifier is composite, and is handled by frontend with 2 arguments and by others with 1 argument.

I'm looking for a fix a will make a PR if i find it, any help appreciated.

JJK801 avatar May 03 '24 10:05 JJK801

here is a patch that seems solve my problem:

diff --git a/node_modules/@aws-amplify/data-schema/dist/esm/runtime/internals/APIClient.mjs b/node_modules/@aws-amplify/data-schema/dist/esm/runtime/internals/APIClient.mjs
index 7887bba..9fafebd 100644
--- a/node_modules/@aws-amplify/data-schema/dist/esm/runtime/internals/APIClient.mjs
+++ b/node_modules/@aws-amplify/data-schema/dist/esm/runtime/internals/APIClient.mjs
@@ -469,16 +469,35 @@ function generateGraphQLDocument(modelIntrospection, modelName, modelOperation,
     };
     const listPkArgs = {};
     const generateSkArgs = (op) => {
-        return sortKeyFieldNames.reduce((acc, fieldName) => {
-            const fieldType = fields[fieldName].type;
-            if (op === 'get') {
-                acc[fieldName] = `${fieldType}!`;
-            }
-            else if (op === 'list') {
-                acc[fieldName] = `Model${fieldType}KeyConditionInput`;
-            }
-            return acc;
-        }, {});
+        let filteredSortKeyFieldNames = [...sortKeyFieldNames]
+        const associationKeys = {}
+
+        if (op === 'list') {
+            const skAssociations = Object.values(fields).filter(({ association }) => {
+                return !!association && association.connectionType == 'BELONGS_TO' && association.targetNames.every((targetName) => sortKeyFieldNames.includes(targetName))
+            })
+
+            skAssociations.forEach(({ association: { targetNames } }) => {
+                filteredSortKeyFieldNames = filteredSortKeyFieldNames.filter((sortKeyFieldName) => !targetNames.includes(sortKeyFieldName));
+                const argName = targetNames.map((targetName, i) => i > 0 ? `${targetName[0].toUpperCase()}${targetName.slice(1)}`: targetName).join('')
+
+                associationKeys[argName] = `Model${modelName}PrimaryCompositeKeyConditionInput`
+            })
+        }
+
+        return {
+            ...associationKeys,
+            ...filteredSortKeyFieldNames.reduce((acc, fieldName) => {
+                const fieldType = fields[fieldName].type;
+                if (op === 'get') {
+                    acc[fieldName] = `${fieldType}!`;
+                }
+                else if (op === 'list') {
+                    acc[fieldName] = `Model${fieldType}KeyConditionInput`;
+                }
+                return acc;
+            }, {})
+        };
     };
     if (isCustomPrimaryKey) {
         Object.assign(getPkArgs, generateSkArgs('get'));

JJK801 avatar May 03 '24 14:05 JJK801

Another bug found:

When using a composite key and calling a model association getter, only the primary key (the first part of composite key) is used, which makes the request fails.

Here is the updated patch:

diff --git a/node_modules/@aws-amplify/data-schema/dist/cjs/runtime/internals/APIClient.js b/node_modules/@aws-amplify/data-schema/dist/cjs/runtime/internals/APIClient.js
index 0d8b294..d9d2a2f 100644
--- a/node_modules/@aws-amplify/data-schema/dist/cjs/runtime/internals/APIClient.js
+++ b/node_modules/@aws-amplify/data-schema/dist/cjs/runtime/internals/APIClient.js
@@ -59,21 +59,21 @@ function initializeModel(client, modelName, result, modelIntrospection, authMode
             switch (relationType) {
                 case connectionType.HAS_ONE:
                 case connectionType.BELONGS_TO: {
-                    const sortKeyValues = relatedModelSKFieldNames.reduce(
+                    const variables = [relatedModelPKFieldName, ...relatedModelSKFieldNames].reduce(
                     // TODO(Eslint): is this implementation correct?
                     // eslint-disable-next-line array-callback-return
-                    (acc, curVal) => {
-                        if (record[curVal]) {
-                            return (acc[curVal] = record[curVal]);
+                    (acc, curVal, idx) => {
+                        if (targetNames[idx] && record[targetNames[idx]]) {
+                            return {
+                                ...acc,
+                                [curVal]: record[targetNames[idx]]
+                            };
                         }
                     }, {});
                     if (context) {
                         initializedRelationalFields[fieldName] = (contextSpec, options) => {
                             if (record[targetNames[0]]) {
-                                return client.models[relatedModelName].get(contextSpec, {
-                                    [relatedModelPKFieldName]: record[targetNames[0]],
-                                    ...sortKeyValues,
-                                }, {
+                                return client.models[relatedModelName].get(contextSpec, variables, {
                                     authMode: options?.authMode || authMode,
                                     authToken: options?.authToken || authToken,
                                 });
@@ -84,10 +84,7 @@ function initializeModel(client, modelName, result, modelIntrospection, authMode
                     else {
                         initializedRelationalFields[fieldName] = (options) => {
                             if (record[targetNames[0]]) {
-                                return client.models[relatedModelName].get({
-                                    [relatedModelPKFieldName]: record[targetNames[0]],
-                                    ...sortKeyValues,
-                                }, {
+                                return client.models[relatedModelName].get(variables, {
                                     authMode: options?.authMode || authMode,
                                     authToken: options?.authToken || authToken,
                                 });
@@ -478,16 +475,35 @@ function generateGraphQLDocument(modelIntrospection, modelName, modelOperation,
     };
     const listPkArgs = {};
     const generateSkArgs = (op) => {
-        return sortKeyFieldNames.reduce((acc, fieldName) => {
-            const fieldType = fields[fieldName].type;
-            if (op === 'get') {
-                acc[fieldName] = `${fieldType}!`;
-            }
-            else if (op === 'list') {
-                acc[fieldName] = `Model${fieldType}KeyConditionInput`;
-            }
-            return acc;
-        }, {});
+        let filteredSortKeyFieldNames = [...sortKeyFieldNames]
+        const associationKeys = {}
+
+        if (op === 'list') {
+            const skAssociations = Object.values(fields).filter(({ association }) => {
+                return !!association && association.connectionType == 'BELONGS_TO' && association.targetNames.every((targetName) => sortKeyFieldNames.includes(targetName))
+            })
+
+            skAssociations.forEach(({ association: { targetNames } }) => {
+                filteredSortKeyFieldNames = filteredSortKeyFieldNames.filter((sortKeyFieldName) => !targetNames.includes(sortKeyFieldName));
+                const argName = targetNames.map((targetName, i) => i > 0 ? `${targetName[0].toUpperCase()}${targetName.slice(1)}`: targetName).join('')
+
+                associationKeys[argName] = `Model${modelName}PrimaryCompositeKeyConditionInput`
+            })
+        }
+
+        return {
+            ...associationKeys,
+            ...filteredSortKeyFieldNames.reduce((acc, fieldName) => {
+                const fieldType = fields[fieldName].type;
+                if (op === 'get') {
+                    acc[fieldName] = `${fieldType}!`;
+                }
+                else if (op === 'list') {
+                    acc[fieldName] = `Model${fieldType}KeyConditionInput`;
+                }
+                return acc;
+            }, {})
+        };
     };
     if (isCustomPrimaryKey) {
         Object.assign(getPkArgs, generateSkArgs('get'));
diff --git a/node_modules/@aws-amplify/data-schema/dist/esm/runtime/internals/APIClient.mjs b/node_modules/@aws-amplify/data-schema/dist/esm/runtime/internals/APIClient.mjs
index 7887bba..60384a3 100644
--- a/node_modules/@aws-amplify/data-schema/dist/esm/runtime/internals/APIClient.mjs
+++ b/node_modules/@aws-amplify/data-schema/dist/esm/runtime/internals/APIClient.mjs
@@ -55,21 +55,21 @@ function initializeModel(client, modelName, result, modelIntrospection, authMode
             switch (relationType) {
                 case connectionType.HAS_ONE:
                 case connectionType.BELONGS_TO: {
-                    const sortKeyValues = relatedModelSKFieldNames.reduce(
+                    const variables = [relatedModelPKFieldName, ...relatedModelSKFieldNames].reduce(
                     // TODO(Eslint): is this implementation correct?
                     // eslint-disable-next-line array-callback-return
-                    (acc, curVal) => {
-                        if (record[curVal]) {
-                            return (acc[curVal] = record[curVal]);
+                    (acc, curVal, idx) => {
+                        if (targetNames[idx] && record[targetNames[idx]]) {
+                            return {
+                                ...acc,
+                                [curVal]: record[targetNames[idx]]
+                            };
                         }
                     }, {});
                     if (context) {
                         initializedRelationalFields[fieldName] = (contextSpec, options) => {
                             if (record[targetNames[0]]) {
-                                return client.models[relatedModelName].get(contextSpec, {
-                                    [relatedModelPKFieldName]: record[targetNames[0]],
-                                    ...sortKeyValues,
-                                }, {
+                                return client.models[relatedModelName].get(contextSpec, variables, {
                                     authMode: options?.authMode || authMode,
                                     authToken: options?.authToken || authToken,
                                 });
@@ -80,10 +80,7 @@ function initializeModel(client, modelName, result, modelIntrospection, authMode
                     else {
                         initializedRelationalFields[fieldName] = (options) => {
                             if (record[targetNames[0]]) {
-                                return client.models[relatedModelName].get({
-                                    [relatedModelPKFieldName]: record[targetNames[0]],
-                                    ...sortKeyValues,
-                                }, {
+                                return client.models[relatedModelName].get(variables, {
                                     authMode: options?.authMode || authMode,
                                     authToken: options?.authToken || authToken,
                                 });
@@ -469,16 +466,35 @@ function generateGraphQLDocument(modelIntrospection, modelName, modelOperation,
     };
     const listPkArgs = {};
     const generateSkArgs = (op) => {
-        return sortKeyFieldNames.reduce((acc, fieldName) => {
-            const fieldType = fields[fieldName].type;
-            if (op === 'get') {
-                acc[fieldName] = `${fieldType}!`;
-            }
-            else if (op === 'list') {
-                acc[fieldName] = `Model${fieldType}KeyConditionInput`;
-            }
-            return acc;
-        }, {});
+        let filteredSortKeyFieldNames = [...sortKeyFieldNames]
+        const associationKeys = {}
+
+        if (op === 'list') {
+            const skAssociations = Object.values(fields).filter(({ association }) => {
+                return !!association && association.connectionType == 'BELONGS_TO' && association.targetNames.every((targetName) => sortKeyFieldNames.includes(targetName))
+            })
+
+            skAssociations.forEach(({ association: { targetNames } }) => {
+                filteredSortKeyFieldNames = filteredSortKeyFieldNames.filter((sortKeyFieldName) => !targetNames.includes(sortKeyFieldName));
+                const argName = targetNames.map((targetName, i) => i > 0 ? `${targetName[0].toUpperCase()}${targetName.slice(1)}`: targetName).join('')
+
+                associationKeys[argName] = `Model${modelName}PrimaryCompositeKeyConditionInput`
+            })
+        }
+
+        return {
+            ...associationKeys,
+            ...filteredSortKeyFieldNames.reduce((acc, fieldName) => {
+                const fieldType = fields[fieldName].type;
+                if (op === 'get') {
+                    acc[fieldName] = `${fieldType}!`;
+                }
+                else if (op === 'list') {
+                    acc[fieldName] = `Model${fieldType}KeyConditionInput`;
+                }
+                return acc;
+            }, {})
+        };
     };
     if (isCustomPrimaryKey) {
         Object.assign(getPkArgs, generateSkArgs('get'));

JJK801 avatar May 03 '24 15:05 JJK801

Hey, 👋 thanks for raising this! I'm going to transfer this over to our API repository for better assistance 🙂

ykethan avatar May 03 '24 15:05 ykethan

@JJK801 which version of Amplify JS (aws-amplify) are you using?

iartemiev avatar May 03 '24 17:05 iartemiev

@JJK801 which version of Amplify JS (aws-amplify) are you using?

Hi @iartemiev , i'm on 6.2.0 version

├─ @boostrade/amplify-backend@workspace:packages/amplify-backend
│  └─ aws-amplify@npm:6.2.0 (via npm:^6.2.0)
│
└─ @boostrade/core-amplify-adapter@workspace:packages/core-amplify-adapter
   └─ aws-amplify@npm:6.2.0 (via npm:^6.2.0)

JJK801 avatar May 04 '24 19:05 JJK801

Thanks @JJK801. Confirming that this is a bug. We're working on fixing it

iartemiev avatar May 07 '24 18:05 iartemiev

@JJK801 we've patched this bug in the latest release of @aws-amplify/data-schema. Please make sure you're using at least @aws-amplify/[email protected] and then run npm update @aws-amplify/data-schema to consume the patch.

iartemiev avatar May 14 '24 11:05 iartemiev

This issue is now closed. Comments on closed issues are hard for our team to see. If you need more assistance, please open a new issue that references this one.

github-actions[bot] avatar May 14 '24 11:05 github-actions[bot]