aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.44k stars 2.13k forks source link

DataStore - update mutation complaining about missing fields that aren't missing #11271

Open caridavidson opened 1 year ago

caridavidson commented 1 year ago

Before opening, please confirm:

Note that this might be related to https://github.com/aws-amplify/amplify-js/issues/10487

JavaScript Framework

Vue

Amplify APIs

DataStore

Amplify Categories

api

Environment information

``` # Put output below this line ╰─➤ npx envinfo --system --binaries --browsers --npmPackages --duplicates --npmGlobalPackages System: OS: Linux 5.15 Ubuntu 20.04.5 LTS (Focal Fossa) CPU: (20) x64 12th Gen Intel(R) Core(TM) i7-12700H Memory: 2.68 GB / 7.60 GB Container: Yes Shell: 5.8 - /usr/bin/zsh Binaries: Node: 19.1.0 - /usr/local/bin/node Yarn: 1.22.19 - /usr/local/bin/yarn npm: 8.19.3 - /usr/local/bin/npm npmPackages: @babel/core: ^7.12.16 => 7.21.4 @babel/eslint-parser: ^7.12.16 => 7.21.3 @mdi/js: ^7.1.96 => 7.2.96 @quasar/extras: ^1.0.0 => 1.16.2 @tiptap/extension-code-block-lowlight: ^2.0.0-beta.220 => 2.0.3 @tiptap/extension-floating-menu: ^2.0.0-beta.220 => 2.0.3 @tiptap/extension-gapcursor: ^2.0.0-beta.220 => 2.0.3 @tiptap/extension-link: ^2.0.0-beta.220 => 2.0.3 @tiptap/extension-placeholder: ^2.0.0-beta.220 => 2.0.3 @tiptap/extension-task-item: ^2.0.0-beta.220 => 2.0.3 @tiptap/extension-task-list: ^2.0.0-beta.220 => 2.0.3 @tiptap/pm: ^2.0.0-beta.220 => 2.0.3 @tiptap/starter-kit: ^2.0.0-beta.220 => 2.0.3 @tiptap/vue-3: ^2.0.0-beta.220 => 2.0.3 @types/aws-sdk: ^2.7.0 => 2.7.0 @types/debounce: ^1.2.1 => 1.2.1 @types/gapi: ^0.0.44 => 0.0.44 @types/lodash: ^4.14.191 => 4.14.194 @types/lodash.debounce: ^4.0.7 => 4.0.7 @typescript-eslint/eslint-plugin: ^5.4.0 => 5.58.0 @typescript-eslint/parser: ^5.4.0 => 5.58.0 @vue/cli-plugin-babel: ^5.0.8 => 5.0.8 @vue/cli-plugin-eslint: ^5.0.8 => 5.0.8 @vue/cli-plugin-router: ^5.0.8 => 5.0.8 @vue/cli-plugin-typescript: ^5.0.8 => 5.0.8 @vue/cli-service: ^5.0.8 => 5.0.8 @vue/eslint-config-typescript: ^9.1.0 => 9.1.0 @vueuse/core: ^9.10.0 => 9.13.0 @yeger/vue-masonry-wall: ^3.4.4 => 3.4.4 application-name: 0.0.1 aws-amplify: 5.0.24 => 5.0.24 aws-lambda: ^1.0.7 => 1.0.7 aws-sdk: ^2.1351.0 => 2.1358.0 chart.js: ^4.2.1 => 4.2.1 chart.js-auto: undefined () chart.js-helpers: undefined () core-js: ^3.8.3 => 3.30.1 countries-list: ^2.6.1 => 2.6.1 csvtojson: ^2.0.10 => 2.0.10 eslint: ^7.32.0 => 7.32.0 eslint-plugin-vue: ^9.9.0 => 9.10.0 fuse: ^0.4.0 => 0.4.0 fuse.js: ^6.6.2 => 6.6.2 gapi: ^0.0.3 => 0.0.3 lodash: ^4.17.21 => 4.17.21 lowlight: ^2.8.1 => 2.8.1 (1.20.0) pinia: ^2.0.29 => 2.0.34 quasar: ^2.11.7 => 2.11.10 sass: 1.32.12 => 1.32.12 sass-loader: ^12.0.0 => 12.6.0 throttle-typescript: ^1.1.0 => 1.1.0 tiptap-extensions: ^1.35.2 => 1.35.2 typescript: ~4.5.5 => 4.5.5 uuid: ^9.0.0 => 9.0.0 (3.4.0, 8.3.2, 8.0.0) v-network-graph: ^0.8.1 => 0.8.2 vue: ^3.2.13 => 3.2.47 vue-chartjs: ^5.2.0 => 5.2.0 vue-class-component: ^8.0.0-0 => 8.0.0-rc.1 vue-cli-plugin-quasar: ~5.0.1 => 5.0.2 vue-draggable-next: ^2.1.1 => 2.1.1 vue-eslint-parser: ^9.1.0 => 9.1.1 (8.3.0) vue-plugin-load-script: ^2.1.1 => 2.1.1 vue-router: ^4.1.6 => 4.1.6 yargs: ^17.7.1 => 17.7.1 (16.2.0) npmGlobalPackages: @aws-amplify/cli: 11.0.3 amplify: 0.0.11 corepack: 0.15.1 n: 9.0.1 npm: 8.19.3 yarn: 1.22.19 ```

Describe the bug

I'm using AWS Amplify DataStore to update a model. First, I create the model with some basic data, and that saves fine. Then I update some fields. Say, first_name, and then DataStore generates a mutation (below). It returns an error:

DataStore - conflict trycatch Error: Field tenant is required is a WARN to the console, and the following is the payload returned. Note that tenant is not sent in the update. . Should it be an error? Is this a DataStore bug, or mine? How can I bypass this problem? Maybe it's just missing _version? I think DataStore should be handling that though, right?

// Mutation:
// Mutation
mutation operation($input: UpdatePersonInput!, $condition: ModelPersonConditionInput) {
  updatePerson(input: $input, condition: $condition) {
    id
    tenant  // this is the missing one
    owner
    username
    first_name
    last_name
    ...
    _version
    _lastChangedAt
    _deleted
  }
}
// Variables  (note that tenant is missing.  This is what DataStore generated)
{
  "input": {
    "id": "fd3d6333-3217-416b-8f89-2083826d97e3",
    "first_name": "1"
  },
  "condition": null
}
// Response
{
  "data": {
    "updatePerson": null
  },
  "errors": [
    {
      "path": [
        "updatePerson"
      ],
      "data": {
        "id": "fd3d6333-3217-416b-8f89-2083826d97e3",
        "tenant": "2faa7d6a-0e81-442d-9714-0c6bf1ff1761",   // <--- It's there!
        "owner": null,
        "username": null,
        "first_name": "",
        "last_name": "",
        // ...
        "createdAt": "2023-04-19T15:55:59.998Z",
        "updatedAt": "2023-04-19T15:55:59.998Z",
        "_version": 1,
        "_lastChangedAt": 1681919760022,
        "_deleted": null
      },
      "errorType": "ConflictUnhandled",
      "errorInfo": null,
      "locations": [
        {
          "line": 2,
          "column": 3,
          "sourceName": null
        }
      ],
      "message": "Conflict resolver rejects mutation."
    }
  ]
}

Expected behavior

When calling DataStore.save() with a valid model, I expect it to save, regardless of what is happening in the background, and without having to fiddle with versioning. I also expect that if there are required fields on the model, and they are required in a mutation, then they are sent with that mutation.

Reproduction steps

see above...

Code Snippet

// Put your code below this line.
const saveMe= new Person({
    "first_name": "1",
    "last_name": "23",
    "id": "fd3d6333-3217-416b-8f89-2083826d97e3",
    "tenant": "2faa7d6a-0e81-442d-9714-0c6bf1ff1761",
    "owner": null,
    "username": null,
    // ...
    "createdAt": null,
    "updatedAt": null,
})
DataStore.save(saveMe)

Log output

``` // Put your logs below this line [WARN] 15:36.534 DataStore - conflict trycatch Error: Field tenant is required at datastore.js:386:1 at datastore.js:531:1 at Array.forEach () at initializeInstance (datastore.js:528:1) at datastore.js:576:1 at produce (immer.esm.js:1:16032) at new Person (datastore.js:575:35) at modelInstanceCreator (datastore.js:373:1) at MutationProcessor.defaultConflictHandler [as conflictHandler] (datastore.js:963:1) at MutationProcessor. (mutation.js:318:1) at step (tslib.es6.js:100:1) at Object.throw (tslib.es6.js:81:46) at rejected (tslib.es6.js:72:42) ```

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

ryancircelli commented 1 year ago

I have this issue as well using Datastore with Expo React Native.

WARN [WARN] 52:05.128 DataStore - conflict trycatch [Error: Field userID is required]

It happens to my code when using Optimistic Concurrency and sending updates. I am correctly importing the models generated in ./src/models. It might be linked to sending updates in rapid succession while using Optimistic Concurrency?

Use Case:

const original = await DataStore.query(Exercise, updatedExercise.id);
await DataStore.save(
  Exercise.copyOf(original, (updated) => {
    updated.variationID = updatedExercise.variationID;
    updated.sets = JSON.stringify(updatedExercise.sets);
    updated.userID = user.username; //The error occurs when this line is here and when it is not here
  })
);

Respective Schema:

type Exercise @model @auth(rules: [{ allow: owner }]) {
  id: ID! @primaryKey
  userID: ID!
  workoutID: ID!
  variationID: ID!
  sets: AWSJSON
}
chrisbonifacio commented 1 year ago

@rinorocks8 it seems that the warning is a bit of a red herring. During reproduction of this issue, it seems that the first update succeeds without the userID field but other mutations are sending a version number lower than the server version that was incremented on the first successful mutation.

We will investigate this further now that we can consistently reproduce it. Thank you!

Jackson3195 commented 1 year ago

Can confirm an similar issue happening but it would seem with the primary key.

Given the following model:

type Pulse @model @auth(rules: [{allow: public}]) {
    id: ID!
    hashKey: ID! @index(sortKeyFields: ["rangeKey"])
    rangeKey: ID!
    geoJson: String!
    geoHash: String!
}

And I attempt to do

const pulse = new Pulse({
   hasKey: "ID-X",
   rangeKey: "ID-Y",
   geoJson: "A",
   geoHash: "B"
})

Datastore.save(pulse)

It results in:

 [Error: Field id is required]

It's weird as it happened pre-upgrade to v5.1.4 prior version was set to use v5.0.18.

Manually setting the id attribute resolves the issue however! This works:

const pulse = new Pulse({
   id: "some-uuid-v4",
   hasKey: "ID-X",
   rangeKey: "ID-Y",
   geoJson: "A",
   geoHash: "B"
})

Datastore.save(pulse)
charlieforward9 commented 1 year ago

I am having this a similar issue. DataStore is throwing:

Error: Field id is required
    at datastore.ts:580:11
    at datastore.ts:891:7
    at Array.forEach (<anonymous>)
    at datastore.ts:888:28
    at produce (immerClass.ts:94:14)
    at Model.copyOf (datastore.ts:870:18)
    at traverseModel (util.ts:220:39)
    at StorageAdapterBase2.saveMetadata (StorageAdapterBase.ts:186:27)
    at IndexedDBAdapter2.<anonymous> (IndexedDBAdapter.ts:218:9)
    at step (tslib.es6.js:147:23)

when I run the DataStore.save() operation with a complete (new, not updated) model.

charlieforward9 commented 1 year ago

Manually setting the id attribute resolves the issue however!

@Jackson3195 I tried setting the ID with:

id: randomUUID().toString()

but got:

Object literal may only specify known properties, and 'id' does not exist in type 'ModelInit<modelName>'.

How do you work around this?

danrivett commented 1 year ago

I'd love an update on this bug if possible as it's one we're actively tracking because we can't update our DataStore package version until it's fixed.

svidgen commented 1 year ago

@Jackson3195 @charlieforward9

With respect to the Field: id is required error, there's an open bug in the codegen side (used by CLI) today that prevents DataStore for recognizing id is the primary key on models with @index directives that do not provide a name attribute. You can see the details in amplify-codegen/issues/561.

Using @Jackson3195's Pulse model as an example:

type Pulse @model @auth(rules: [{allow: public}]) {
    id: ID!
    hashKey: ID! @index(sortKeyFields: ["rangeKey"])
    rangeKey: ID!
    geoJson: String!
    geoHash: String!
}

DataStore doesn't know that id is the PK, so it won't generate a value for it. But, it's a required field, so DataStore demands that you supply the value.

The workaround is simply to add a name attribute. E.g.,

type Pulse @model @auth(rules: [{allow: public}]) {
    id: ID!
    hashKey: ID! @index(name: 'byHashKeyAndRangekey', sortKeyFields: ["rangeKey"])
    rangeKey: ID!
    geoJson: String!
    geoHash: String!
}

This will cause the metadata for hashKey supplied to DataStore to also include a name property, which tells DataStore that it's not the PK. When DataStore tries to determine the PK, it will then default to id and generate the id.

svidgen commented 1 year ago

@caridavidson The original issue may be different than the Field: id is required error that others are seeing. Can you post the graphql schema you're using? (Or at least the relevant model and any related models?)

caridavidson commented 1 year ago

I posted this 2 months ago. I posted what existed at the time and I have since moved on and dropped using data store completely.

Please use the information provided or find another way to reproduce this.

Thanks.

ryancircelli commented 1 year ago

I also gave up on Datastore because of this error and Datastore’s many limitations. I ended up creating my own version of it using dynamodb and vercel edge functions by creating a similar implementation of watermelondb’s push and pull server functions and implementing my own database sync on the front end.