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

Optional relation forced #3077

Open
2 tasks done
Lisciowsky opened this issue Dec 14, 2024 · 8 comments
Open
2 tasks done

Optional relation forced #3077

Lisciowsky opened this issue Dec 14, 2024 · 8 comments
Assignees
Labels
graphql-transformer-v2 pending-maintainer-response Issue is pending a response from the Amplify team. to-be-reproduced Pending reproduction transferred

Comments

@Lisciowsky
Copy link

Lisciowsky commented Dec 14, 2024

How did you install the Amplify CLI?

npm install -g @aws-amplify/cli

If applicable, what version of Node.js are you using?

v18.20.3

Amplify CLI Version

12.13.1

What operating system are you using?

ubuntu 22.04

Did you make any manual changes to the cloud resources managed by Amplify? Please describe the changes made.

no manual changes

Describe the bug

type File
  @model
{
  id: ID!
  ownerEmail: String!
  createdAt: AWSDateTime
  updatedAt: AWSDateTime
  fileName: String!
  filePath: String!
  storeID: ID
  store: Store @belongsTo(fields: ["storeID"])
}
createFile = """
  mutation CreateFile(
    $input: CreateFileInput!
    $condition: ModelFileConditionInput
  ) {
    createFile(input: $input, condition: $condition) {
      id
      ownerEmail
      createdAt
      updatedAt
      fileName
      filePath
      storeID
      store {
        id
        ownerEmail
        createdAt
        updatedAt
        name
        __typename
      }
      __typename
    }
  }
"""

createFile mutation variables:

"fileName": "text_file",
"filePath": "/random_user/text_file",
"ownerEmail": "[email protected]",
"storeID": "3afa26ba-b30f-405e-aee6-804a9a9c19a6"

With the newest version of amplify it is not possible to resolve the Store, I had to dump the version of the amplify cli so that the created resolvers would understand that store can be associated with a file but doesn't have to and if the store id is given during the file creation mutation it then can resolve the actual store.

You have a serious problem with hasMany and belongs to directive, no doubt about that.

Expected behavior

I expected to have successful graphql call during file creation mutation with or without providing the existing associated store id with the newest version of amplify cli. This isn't the case

Reproduction steps

If you would like to reproduce the bug just install the newest aws cli and then create two models

one model will have
storeID: ID
store: Store @belongsTo(fields: ["storeID"])

another model will have:
files: [File] @hasmany(fields: ["id"])

And try to create the file without specifying the store in this example, or omitting it completely.

I am forced to use the older version 12.1.0 which created the "reasonable" resolvers.

Project Identifier

No response

Log output

# Put your logs below this line


Additional information

N/A

Before submitting, please confirm:

  • I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.
  • I have removed any sensitive information from my code snippets and submission.
@ykethan
Copy link
Member

ykethan commented Dec 16, 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-cli Dec 16, 2024
@Siqi-Shan
Copy link
Member

Hi @Lisciowsky, thanks for raising the issue! We'll investigate and see what's going on, and get back to you once we have a solution.

@Siqi-Shan Siqi-Shan added to-be-reproduced Pending reproduction pending-maintainer-response Issue is pending a response from the Amplify team. labels Dec 16, 2024
@chrisbonifacio
Copy link
Member

chrisbonifacio commented Dec 17, 2024

Hi @Lisciowsky 👋 A few questions to better understand the issue described.

  1. How is the relationship being forced? Can you provide any error messages or logs you've received related to this issue?
  2. The createFile mutation returns store and possibly required fields on store so if one doesn't exist, those fields being included in the selection set would likely cause an error.
  3. Can you confirm that there's a difference in the resolver logic or in the mutation selection set between the working and non-working CLI versions?
  4. Can you share the schema for both File and Storage so we can reproduce the issue as closely to your setup as possible? It might help identify any issues related to non-obvious things like auth rules as well.

in the meantime, as a potential workaround, you could try omitting the entire store, or at least any required fields on it from the selection set.

i'm not sure what the non-nullable fields on store are, but usually id, createdAt, updatedAt are non-nullable so I removed them from the mutation selection set below.

createFile(input: $input, condition: $condition) {
      id
      ownerEmail
      createdAt
      updatedAt
      fileName
      filePath
      storeID
      store {
        ownerEmail
        name
        __typename
      }
      __typename
    }
  }

@chrisbonifacio chrisbonifacio added pending-community-response Issue is pending a response from the author or community. graphql-transformer-v2 and removed pending-maintainer-response Issue is pending a response from the Amplify team. pending-triage labels Dec 18, 2024
@Lisciowsky
Copy link
Author

Lisciowsky commented Dec 19, 2024

Hello,

  • How is the relationship being forced? Can you provide any error messages or logs you've received related to this issue?
  • The createFile mutation returns store and possibly required fields on store so if one doesn't exist, those fields being included in the selection set would likely cause an error.

It is forced because the createFile mutation does include the [optional] store, and without providing it, it causes the error as you have mentioned.

  • Can you confirm that there's a difference in the resolver logic or in the mutation selection set between the working and non-working CLI versions?

Yes I can confirm, I had to compare on 2 separate environemnts DEV (created with newest version of amplify cli) and STAGE with (12.1.0)

DEV:

#if( $ctx.stash.deniedField )
  #return($util.toJson(null))
#end

#set( $partitionKeyValue = $util.defaultIfNull($ctx.stash.connectionAttributes.get("officeProjectsId"), $ctx.source.officeProjectsId) )

#if( $util.isNullOrEmpty($partitionKeyValue) )
  #return($util.toJson(null))
#else
  #set( $GetRequest = {
    "version": "2018-05-29",
    "operation": "Query",
    "query": {
      "expression": "#partitionKey = :partitionValue",
      "expressionNames": {
        "#partitionKey": "id"
      },
      "expressionValues": {
        ":partitionValue": $util.dynamodb.toDynamoDB($partitionKeyValue)
      }
    }
  })

  #if( !$util.isNullOrEmpty($ctx.stash.authFilter) )
    $util.qr($GetRequest.put("filter", $util.transform.toDynamoDBFilterExpression($ctx.stash.authFilter)))
  #end

  $util.toJson($GetRequest)
#end

#if( $ctx.error )
$util.error($ctx.error.message, $ctx.error.type)
#else
  #if( !$ctx.result.items.isEmpty() && $ctx.result.scannedCount == 1 )
    #set( $resultValue = $ctx.result.items[0] )
    #set( $operation = $util.defaultIfNull($ctx.source.get("__operation"), null) )
    #if( $operation == "Mutation" )
      $util.qr($resultValue.put("__operation", "Mutation"))
    #end
    $util.toJson($resultValue)
  #else
    #if( $ctx.result.items.isEmpty() && $ctx.result.scannedCount == 1 )
$util.unauthorized()
    #end
    $util.toJson(null)
  #end
#end

STAGE:

#if( $ctx.stash.deniedField )
  #return($util.toJson(null))
#end
#set( $partitionKeyValue = $util.defaultIfNull($ctx.stash.connectionAttibutes.get("officeProjectsId"), $ctx.source.officeProjectsId) )
#if( $util.isNull($partitionKeyValue) )
  #return
#else
  #set( $GetRequest = {
  "version": "2018-05-29",
  "operation": "Query"
} )
  $util.qr($GetRequest.put("query", {
  "expression": "#partitionKey = :partitionValue",
  "expressionNames": {
      "#partitionKey": "id"
  },
  "expressionValues": {
      ":partitionValue": $util.parseJson($util.dynamodb.toDynamoDBJson($util.defaultIfNullOrBlank($partitionKeyValue, "___xamznone____")))
  }
}))
  #if( !$util.isNullOrEmpty($ctx.stash.authFilter) )
    $util.qr($GetRequest.put("filter", $util.parseJson($util.transform.toDynamoDBFilterExpression($ctx.stash.authFilter))))
  #end
  $util.toJson($GetRequest)
#end

#if( $ctx.error )
$util.error($ctx.error.message, $ctx.error.type)
#else
  #if( !$ctx.result.items.isEmpty() && $ctx.result.scannedCount == 1 )
    $util.toJson($ctx.result.items[0])
  #else
    #if( $ctx.result.items.isEmpty() && $ctx.result.scannedCount == 1 )
$util.unauthorized()
    #end
    $util.toJson(null)
  #end
#end

This is CGPT comment:

Why Stage Works, but Dev Doesn’t:
Different Resolver Logic: The Stage resolver uses toDynamoDBJson($util.defaultIfNullOrBlank(...)) while the Dev resolver directly uses $util.dynamodb.toDynamoDB($partitionKeyValue). If the officeProjectsId is blank, the Stage resolver gracefully handles it with "___xamznone____", whereas the Dev resolver fails.

Error Handling Differences: The Stage resolver is more concise and handles error cases better, directly returning unauthorized() or null, while the Dev resolver introduces unnecessary complexity in logic and error handling. 

Can you share the schema for both File and Storage so we can reproduce the issue as closely to your setup as possible? It might help identify any issues related to non-obvious things like auth rules as well.

You can replicate this one easily as long as you have one to many with @belongsTo ans @hasmany directives on the one-to-many model relation.

Here is the example grapqhl schema definition:

type Store
  @model
  @auth(
    rules: [
      { allow: owner, ownerField: "ownerEmail", identityClaim: "email", provider: oidc, operations: [create, update, delete, read] }
      { allow: groups, groups: ["Admins"], groupClaim: "groups", provider: oidc, operations: [create, update, delete, read] }
    ]
  ) {
  id: ID!
  ownerEmail: String!
  createdAt: AWSDateTime
  updatedAt: AWSDateTime
  name: String!
  files: [File] @hasMany
}

type File
  @model
  @auth(
    rules: [
      { allow: owner, ownerField: "ownerEmail", identityClaim: "email", provider: oidc, operations: [create, update, delete, read] }
      { allow: groups, groups: ["Admins"], groupClaim: "groups", provider: oidc, operations: [create, update, delete, read] }
    ]
  ) {
  id: ID!
  ownerEmail: String!
  createdAt: AWSDateTime
  updatedAt: AWSDateTime
  fileName: String!
  filePath: String!
  storeID: ID
  store: Store @belongsTo(fields: ["storeID"])
}

Just to summarize, the newest version of the aws cli does create the resolvers but they force the File (following the example schema above) to ensure it points to the store. Your tempfix would certainly work (not including the store in the grapqhl).

I do not want to modify the auto-generated amplify queries OR resolvers OR subscriptions and I believe nobody wants to every time I code gen or make some changes. Then I would end up in a scenario where a file without an associated store would have to use a different query, that's just bad news.

PS: The resolver differences on DEV & STAGE example were taken from different project but the issue is exactly the same.

I hope it helps you to fix this, or perhaps roll back the changes that impact the resolver creation

PS2: I just reminded myself that it gets even worse, I think even if you provide the storeID while creating the file it still is unable to resolve the store. At least that was the case for me with the exactly same schema generated via newest amplify-cli version vs 12.1.0 which worked correctly.

For both of the cases I have changed the amplify version to 12.1.0 recreated the environment and everything works like a charm

@github-actions github-actions bot added pending-maintainer-response Issue is pending a response from the Amplify team. and removed pending-community-response Issue is pending a response from the author or community. labels Dec 19, 2024
@AnilMaktala AnilMaktala self-assigned this Dec 23, 2024
@armanian
Copy link

armanian commented Dec 25, 2024

I think I have the same problem. My schema:

type Chat
  @model(queries: null, mutations: { create: "createChat", update: "updateChat" })
  @auth(rules: [{ allow: owner, ownerField: "owners", operations: [create, read, update] }]) {
  id: ID!
  lastMessageID: ID
  owners: [ID!]! @auth(rules: [{ allow: owner, ownerField: "owners", operations: [create, read] }])
  updatedAt: AWSDateTime!

  LastMessage: ChatMessage @hasOne(fields: ["lastMessageID"])
}

type ChatMessage
  @model(queries: null, mutations: { create: "createChatMessage" })
  @auth(rules: [{ allow: owner, ownerField: "owners", operations: [create, read] }]) {
  id: ID!
  chatID: ID
  message: String
  owners: [ID!]!
  userID: ID!
  createdAt: AWSDateTime

  Chat: Chat @hasOne(fields: ["chatID"])
}

When I use CreateChat I get the following error message:

[{"locations": null, "message": "Cannot return null for non-nullable type: 'ID' within parent 'ChatMessage' (/createChat/LastMessage/userID)", "path": [Array]}, {"locations": null, "message": "Cannot return null for non-nullable type: 'null' within parent 'ChatMessage' (/createChat/LastMessage/owners)", "path": [Array]}

Error message although the lastMessageID field is optional. It used to work like this, but no longer does.

@armanian
Copy link

@Lisciowsky I found out that if I deploy with version 12.12.1, everything works correctly. If I deploy with version 12.12.2, then I get the error message. Can you confirm this?

@AnilMaktala
Copy link
Member

Hey @armanian Thank you for providing additional details. Could you please try using the latest version of the CLI (12.13.1) and let us know if the issue persists?

@bobbyu99
Copy link
Contributor

@Lisciowsky @armanian Thanks for sharing this info. I am working on repro-ing your error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
graphql-transformer-v2 pending-maintainer-response Issue is pending a response from the Amplify team. to-be-reproduced Pending reproduction transferred
Projects
None yet
Development

No branches or pull requests

7 participants