Sync issue: Locally created records are deleted by remote changes

[mfbx9da4]

🍉 's sync ethos is the client is the source of truth, therefore if the client has created a local record it should not be deleted by the server remote changes claiming that it has been deleted.

Reproduction Steps

1. Create a record locally, the record now has _status = 'created'
2. During pull phase, remote changes sync indicate that record has been deleted
3. The record was deleted even though it shouldn't have been!

You'll need a table called 'profiles' in your local database to run this code

import applyRemoteChanges from '@nozbe/watermelondb/sync/impl/applyRemote'
import { Q } from '@nozbe/watermelondb'
async function test() {
  const id = String(Math.random())

  // 1. Create a record locally, the record now has `_status = 'created'`
  await database.write(() =>
    database.get('profiles').create(x => {
      x._raw.id = id
    })
  )

  // Sanity check that the profile was created, will throw if not
  await database.get('profiles').find(id)

  // 2. During pull phase, remote changes sync indicate that record has been deleted
  const remoteChanges = { profiles: { created: [], updated: [], deleted: [id] } }
  await database.write(() => applyRemoteChanges(database, remoteChanges))

  // 3. The record was deleted even though it shouldn't have been!
  const [profile] = await database.get('profiles').query(Q.where('id', id)).fetch()
  if (!profile) {
    console.error('Oh no, the server changes deleted the local profile')
  }
}

void test()

Expected Behaviour

Local records in the "created" or "updated" state should not be deleted by remote changes

[mfbx9da4]

@radex do you agree with the expected behaviour here?

[radex]

@mfbx9da4 I don't think I do. Can you give me more context as for why you came across this issue and why it is an issue?

If you see a record that's locally "created", but already exists remotely, I think what this means is that record WAS successfully pushed to remote, but then server/network/client failed during the time between a successful push transaction and successfully marking locally these changes as marked.

And if it was pushed, and then deleted, then it seems reasonable to me that it should, in fact, be deleted locally, no?

[mfbx9da4]

If you see a record that's locally "created", but already exists remotely, I think what this means is that record WAS successfully pushed to remote,

Not necessarily, it could have been created by a different user

This record which was deleted actually represents whether a user is part of a group. The row actually has a stable id as it is a concatenation of ${groupId}#${userId}.

You can run into this issue if there are some users concurrently adding and removing users to a group.

We specifically ran into this issue where

1. User A was invited locally to some group
2. Group was pulled returning data without the local member (therefore inferred that the member was deleted)

[radex]

This record which was deleted actually represents whether a user is part of a group. The row actually has a stable id as it is a concatenation of ${groupId}#${userId}.

I strongly recommend that you do not do that, WatermelonDB assumes that IDs are unique (preferably random, though some users have implemented a layer that renames all record IDs to a unique ID generated by backend, which could be either random or sequential).

I think you can run into more issues than this one if you choose to do it this way. If you still insist on it, perhaps it's a better idea to override sync's conflict resolver (or something similar to that that's going to work in your case) to customize sync behavior?

[mfbx9da4]

customize sync behavior?

Yes I've implemented a workaround which just checks if the local record is not yet "synced" don't delete it.

WatermelonDB assumes that IDs are unique

To be pedantic, in my application IDs are unique, random and created on device, it's just that this record is referring to the same record. I understand your point though and 🍉 's assumption.

I strongly recommend that you do not do that

I don't really see how else to do this, as the many-to-many table relation between groups and users doesn't actually exist on my server, it's stored flat as an array on the group and I don't have a practical way of updating this.

Even if it was trivial for the backend storage to support this, it really doesn't make sense to have the same relation of a user to a group stored multiple times.

I think you can run into more issues than this one if you choose to do it this way.

Anything come to mind?

[radex]

Anything come to mind?

Not off the top of my head

I don't really see how else to do this, as the many-to-many table relation between groups and users doesn't actually exist on my server, it's stored flat as an array on the group and I don't have a practical way of updating this.

Even if it was trivial for the backend storage to support this, it really doesn't make sense to have the same relation of a user to a group stored multiple times.

The standard way to do this is to just have a group_user_assignment { id: xxx, group_id: ggg, user_id: uuu } record such that you ensure that a given assignment is unique. I completely understand why you chose to do it another way, and I think it's OK as long as you're aware that while you avoided some challenges, this can bring you some other challenges, including those that WatermelonDB is not thoroughly tested to handle gracefully.

[mfbx9da4]

The standard way to do this is to just have a group_user_assignment { id: xxx, group_id: ggg, user_id: uuu } record such that you ensure that a given assignment is unique.

Yeah the many-to-many table does indeed have id, group_id, user_id fields.

It sounds like what you're saying is to do something else other than using id=group_id+user_id on the device and the server to enforce such uniqueness e.g.

1. A transaction which checks the row doesn't exist before writing
2. A unique constraint on the compound index of group_id + user_id

It's tricky though, as if I did it this way, when I sync-pull I would also have to do the same uniqueness checks as otherwise I could end up with a duplicate assignments in the local database. So this doesn't necessarily seem better.

I completely understand why you chose to do it another way, and I think it's OK as long as you're aware that while you avoided some challenges, this can bring you some other challenges, including those that WatermelonDB is not thoroughly tested to handle gracefully.

I get your point, though, there are no solutions, only trade-offs and 🍉 isn't designed this way.

I would like to make the final appeal though that this case, is indeed the same record and it is possible that two devices independently create the same record in a decentralized fashion. Another example might be, creating a DM with another user, the ID of that chat could be a hash of the participants of the chat resulting in an idempotent stable and globally unique ID.

Anyway, if you don't agree that record IDs should ever be compound-idempotent-keys then feel free to close this issue and I'll be on my way with my hacks 🙂