aws-amplify / amplify-cli

The AWS Amplify CLI is a toolchain for simplifying serverless web and mobile development.
Apache License 2.0
2.81k stars 821 forks source link

AppSync provisioned with Amplify CLI contains Create Mutation with condition input #3146

Closed lawmicha closed 4 years ago

lawmicha commented 4 years ago

Describe the bug This is more of a question and possibly a bug. Why does AppSync provisioned API contain an input for the "condition" for create mutations? Is there a use case for this? Or is "condition" only useful in update and delete mutations?

For example:

mutation createTodo($input: CreateTodoInput!, $condition: ModelTodoConditionInput) {
  createTodo(input: $input, condition: $condition){
    id
    name
    description
  }
}

I can pass in a condition even though I don't see a use case for this.

Amplify CLI Version 4.6.0

To Reproduce

  1. amplify init
  2. amplify add api
    ? Please select from one of the below mentioned services: GraphQL
    ? Provide API name: apicondition
    ? Choose the default authorization type for the API API key
    ? Enter a description for the API key:
    ? After how many days from now the API key should expire (1-365): 30
    ? Do you want to configure advanced settings for the GraphQL API No, I am done.
    ? Do you have an annotated GraphQL schema? No
    ? Do you want a guided schema creation? Yes
    ? What best describes your project: Single object with fields (e.g., “Todo” with ID, name, description)
    ? Do you want to edit the schema now? No
  3. amplify push
  4. amplify console api
  5. Go to AppSync Queries tab to see createTodo(input: CreateTodoInput!, condition: ModelTodoConditionInput): Todo is created with condition

Expected behavior The transformer from the user generated graphQL schema to appsync's graphQL schema (plus data resolvers?) should not produce a create mutation with condition if there is no use case for this.

Should produce a mutation like createTodo(input: CreateTodoInput!): Todo

Screenshots

image

Additional context https://github.com/aws-amplify/amplify-ios/pull/272#discussion_r360930506 https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Expressions.ConditionExpressions.html

ammarkarachi commented 4 years ago

@lawmicha thanks for the question. It's an optional input provided by default. This is utilized as support for datastore on other operations. Closing the issue for now let me know if you have any other questions.

lawmicha commented 4 years ago

To provide additional context, we are exposing ways to call amplify provisioned services over on the amplify iOS and amplify android frameworks in API Category and is confusing to allow the developer to pass a condition on create mutations. To clarify, it should never be used on create mutations, then is it technically feasible to update the transformer to only add the condition input to update and delete mutations?

gaochenyue commented 4 years ago

ConditionInput for creation is useful because currently amplify treats both create and update as update. If there is no condition for create, you have the risk to overwrite existing data.

lawmicha commented 4 years ago

thanks for your input @gaochenyue, can you clarify what you mean by "amplify treats both create and update as update"? what do I pass in the conditionInput for creation to avoid overwriting existing data?

gaochenyue commented 4 years ago

@lawmicha the autogenerated appsync resolver uses PutItem to create new item. If you read the dynamodb doc, you will find that PutItem will overwriting existing item if primary key matches. The only way to avoid overwriting is to use a condition expression to check if the item already exists. The conditionInput you asked to remove is to specify the condition. Here is a buildin example of autogenerated vtl file. $condition = { "expression": "attribute_not_exists(#id)", "expressionNames": { "#id": "id" } BTW, if you use id, deduplication is included by the code above, but if you don't use id (as I do), condition becomes the only way to deduplicate.

attilah commented 4 years ago

@lawmicha the design decision was to expose the same condition possibilities as it is available for update and delete. As @gaochenyue points out attribute_not_exists is one thing that it is useful for and if you're going with custom resolvers or if your higher level needs to decide on to update or insert an item based on some business rules, without the exposed condition parameter you'd have no way of doing it.

kldeb commented 4 years ago

I didn't find any mention of this condition in the docs btw.

github-actions[bot] commented 3 years ago

This issue 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.