fix(@aws-amplify/datastore): fix consecutive updates

[iartemiev]

Issue #, if available: #6363

type Todo @model {
  id: ID!
  name: String!
}

Currently, if we run the following code in DataStore:

async function changeTodoName() {
  const [todo] = await DataStore.query(Todo); // { name: A,  _version: 1 }

  for (let letter of ['B', 'C', 'D']) {
    await DataStore.save(
      Todo.copyOf(todo, (updated) => {
        updated.name = letter;
      })
    );
  }
}

...the name of our Todo will be 'B' after all AppSync requests are resolved, i.e., latter updates get ignored.

The expected behavior is for name to be 'D'.

Here's a sequence diagram detailing the current behavior (abstracted/simplified):

It happens on both fast and slow networks.

Fast (~500 Mbps):

Slow (10 Kbps):

Proposal

Reconcile the version of update mutations in the mutation queue when an AppSync mutation for the same model ID resolves.

Diagram of updated behavior:

Here's the same example running with the code in this PR:
Fast (~500 Mbps):

Slow (10 Kbps):

Note: the _version in the response will only be synced to related items in the queue when the AppSync mutation response contains the same item that was sent in the request, i.e., if the original mutation was sent by the same client.
In the event of a handled conflict, where the response payload is different from the request payload, we do not sync the _version and subsequent writes will effectively be discarded.

Sequence diagram for multi-client behavior:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[codecov]

Codecov Report

Merging #7354 (53989a5) into main (638c94d) will increase coverage by 0.56%.
The diff coverage is 90.00%.

@@            Coverage Diff             @@
##             main    #7354      +/-   ##
==========================================
+ Coverage   74.24%   74.80%   +0.56%     
==========================================
  Files         215      215              
  Lines       13471    13522      +51     
  Branches     2647     2661      +14     
==========================================
+ Hits        10001    10115     +114     
+ Misses       3272     3208      -64     
- Partials      198      199       +1     

Impacted Files	Coverage Δ
packages/datastore/src/datastore/datastore.ts	78.57% <ø> (+0.42%)	⬆️
packages/datastore/src/sync/merger.ts	82.35% <ø> (+47.05%)	⬆️
packages/datastore/src/sync/processors/mutation.ts	12.92% <0.00%> (-0.09%)	⬇️
packages/datastore/__tests__/helpers.ts	100.00% <100.00%> (ø)
packages/datastore/src/sync/outbox.ts	89.04% <100.00%> (+65.04%)	⬆️
packages/datastore/src/util.ts	83.40% <100.00%> (+1.97%)	⬆️
.../datastore/src/storage/adapter/IndexedDBAdapter.ts	81.71% <0.00%> (+3.42%)	⬆️
packages/datastore/src/sync/utils.ts	87.87% <0.00%> (+3.53%)	⬆️
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 638c94d...53989a5. Read the comment docs.

[nubpro]

Been wondering about this, how do iOS and Android library counterpart deal with such issue?
Do they share the same implementation?

tagging @jamesonwilliams perhaps he can share some insides from the Android side.

[iartemiev]

@nubpro they most likely have the same issue on iOS and Android. We're going to sync up on this across teams after the holidays.

[lgtm-com]

This pull request introduces 1 alert when merging 01129ce into 286c9e8 - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[lgtm-com]

This pull request introduces 1 alert when merging d939a1c into 651c3b2 - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[kylanry]

Hi guys! Does anybody know when this fix will be added to the new Amplify release? I am using the iOS Amplify SDK and also got the same issue. It is blocker for us. Will this fix also applied for iOS version?

[iartemiev]

Hi guys! Does anybody know when this fix will be added to the new Amplify release? I am using the iOS Amplify SDK and also got the same issue. It is blocker for us. Will this fix also applied for iOS version?

@kylanry, yes, this fix will be applied across the Amplify libraries (JS, iOS, Android, Flutter). We're making some final modifications to the solution and are planning on releasing it soon.

[iartemiev]

@manueliglesias @amhinson - this is ready for review now 😸

outbox.test.ts is very hairy because I had to do a ton of manual dependency injection to test the outbox functionality without the Sync Engine running.

[amhinson]

Looks good to me 👍 Great job on the tests!

[nubpro]

Just want to drop by to say that the included diagrams are supppper duper helpful, I wish there's more......could you guys spill more beans on the rest of datastore ins and outs?

I was gonna try to understand the solution u guys came out with and honestly, I wasn't able to totally comprehend them.
Mad props to @iartemiev and the team for working on this!!

[github-actions]

This pull request 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 or Discussions for those types of questions.