Nozbe / WatermelonDB

🍉 Reactive & asynchronous database for powerful React and React Native apps ⚡️
https://watermelondb.dev
MIT License
10.49k stars 589 forks source link

Fix sync limitation: "If a record being pushed changes between pull and push, push will just fail!" #1374

Open mfbx9da4 opened 2 years ago

mfbx9da4 commented 2 years ago

Clarification of Limitation

"If a record being pushed changes between pull and push, push will just fail!"

My interpretation of this limitation is the following:

In between the local changes being fetched from the local DB for sync and being returned as synced from the server, it's possible some of those records have been updated locally. Whether this scenario occurred for any records is detected here:

https://github.com/Nozbe/WatermelonDB/blob/44d89925985aca3fa72eef1df78f89356b1d9b6f/src/sync/impl/markAsSynced.js#L30

If this scenario did occur, the local record is not marked as synced. Leaving the record marked as unsynced makes sense, however the _changed and _status fields are no longer accurate.

Is that a correct interpretation?

I don't see how it will fail, or what error it will throw so I feel like I may be off base?

Proposal

Since you are already diffing the raw records before marking them as synced, why not repair the _changed column so that it is left only with the columns which have not been synced yet?

Something like this code:

function resolveSyncedStatus(localRecord, syncedRaw, schemaColumns) {
  const unsyncedColumns = schemaColumns.filter(x => localRecord._raw[x] !== syncedRaw[x])
  if (unsyncedColumns.length === 0) return { _status: 'synced', _changed: '' }
  return { _status: 'updated', _changed: unsyncedColumns.join(',') }
}

An even faster approach, provided the update tracking is enabled for this table, would be to use the magic updated_at column, then we don't even need to diff the objects.

function resolveSyncedStatusFromUpdatedAt(localRecord, syncedRaw) {
  if (syncedRaw.updated_at === localRecord._raw.updated_at) return { _status: 'synced', _changed: '' }
  // Diffing still needed for created records
  if (localRecord._raw.status === 'created') return resolveSyncedStatus(localRecord, syncedRaw)
  const localChanged = new Set(local._raw._changed.split(','))
  const syncedChanged = new Set(syncedRaw._changed.split(','))
  const unsyncedColumns = leftJoin(localChanged, syncedChanged)
  return return { _status: 'updated', _changed: [...unsyncedColumns].join(',') }
}

function leftJoin(setA, setB) {
  const ret = new Set(setA)
  for (const x of setB.values()) ret.delete(x)
  return ret
}
mfbx9da4 commented 2 years ago

@radex What do you think of this proposal? 🙂

josephbuchma commented 1 year ago

According to docs, the client is responsible for fixing conflicts, so I think "If a record being pushed changes between pull and push, push will just fail!" means that if a record has been updated on the server between pull and push, the push will fail. In other words, the client is allowed to push only if it has the latest changes from the server.

PS: I'm new to watermelon, currently evaluating if it'll work for my app. So maybe I'm wrong.

GitMurf commented 2 weeks ago

I am also new to watermelon but isn’t it more like this:

Server = foo