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

Major release breaks GQL queries with composite identifiers #2526

Closed
JJK801 opened this issue May 3, 2024 · 8 comments
Closed

Major release breaks GQL queries with composite identifiers #2526

JJK801 opened this issue May 3, 2024 · 8 comments
Labels

Comments

@JJK801
Copy link

JJK801 commented May 3, 2024

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
Copy link
Author

JJK801 commented May 3, 2024

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
Copy link
Author

JJK801 commented May 3, 2024

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'));

@ykethan
Copy link

ykethan commented May 3, 2024

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

@ykethan ykethan transferred this issue from aws-amplify/amplify-backend May 3, 2024
@iartemiev
Copy link
Contributor

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

@JJK801
Copy link
Author

JJK801 commented May 4, 2024

@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)

@iartemiev
Copy link
Contributor

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

@iartemiev
Copy link
Contributor

@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/backend@1.0 and then run npm update @aws-amplify/data-schema to consume the patch.

Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants