Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Sync] Update locally created records with server ids #216

Open
davideweaver opened this issue Jan 25, 2019 · 27 comments
Open

[Sync] Update locally created records with server ids #216

davideweaver opened this issue Jan 25, 2019 · 27 comments

Comments

@davideweaver
Copy link

Using PushChanges with a locally created record uses the id generated on the client. My server ignores this id and generates it's own id. What's the recommended way to update the local record with the id returned from the server? If I don't update the local id of the record, it will never receive updates from the server. Thanks

@radex
Copy link
Collaborator

radex commented Jan 25, 2019

@davideweaver is there no way for your server to just accept client's ID?

@davideweaver
Copy link
Author

davideweaver commented Jan 25, 2019 via email

@davideweaver
Copy link
Author

davideweaver commented Jan 25, 2019

@radex I've tried updating the original model immediately after pushing a new record to the server (during the sync process) using...

let origModel = await database.collections.get('appointments').find(id);
const update = origModel.prepareUpdate((m) => {
    m._raw.id = newId; // based on some other posts i've read
});
await database.batch(update);

But this errors out with record.prepareUpdate was called on appointments#appt_cjrbjw29701761un2eqculfsa but wasn't sent to batch() synchronously -- this is bad!

@esbenp
Copy link
Contributor

esbenp commented Jan 28, 2019

In our API (we have sequential IDs) we have DB table that maps all database IDs to tablet IDs. So everytime we send data to the tablet we replace all internal IDs with the right tablet IDs. We also replace IDs of related entities etc. We also use this table to track deletes (so if you delete something in our DB we set a deleted_at timestamp on the tablet_id table).

@davideweaver
Copy link
Author

@esbenp Did you create the tablet id mapping mechanism to work around the fact that there's no way to update ids for records created on your tablets? With zero knowledge of your stack, it seems like it would be easier to make the ids correct on the tablets.

@esbenp
Copy link
Contributor

esbenp commented Jan 29, 2019

Well yes, but I am not sure if it would be possible to update IDs on the tablet. I could imagine problems could arise from mixing string IDs and sequence IDs, so therefore we just decided to convert it all to support Watermelon's datamodel

@davideweaver
Copy link
Author

davideweaver commented Jan 29, 2019

@radex Any additional thoughts on this? Is it feasible to update the local record with the new id from the server after it is pushed?

@davideweaver
Copy link
Author

@radex Been trying to make this work. In sync/impl/helpers.js I have this code.

export function prepareMarkAsSynced<T: Model>(record: T): T {
  const newRaw = { ...record._raw, _status: 'synced', _changed: '' }

  // intercept a record with a new remote id
  if (record._raw.idRemote) {
    record._raw.id = record._raw.idRemote;
  }

  return record.prepareUpdate(() => {
    replaceRaw(record, newRaw)
  })
}

This appears to update the record (the new id shows in the UI), but when I refresh the app the id reverts to the original value. What am I missing here?

@davideweaver
Copy link
Author

@radex Following up on the previous code, it fails because the update path in the adapter does not allow updates to the id as it's currently implemented. encodeUpdate uses the updated id to identify the record to be updated, so if id is changed the sql command doesn't find the record to update. Makes sense.

So I can fork this repo and get it to work for my purposes. Ideally this change could make it's way into your repo, if it's something you want. Let me know if that's something you'd like to talk about.

@radex
Copy link
Collaborator

radex commented Feb 1, 2019

So I can fork this repo and get it to work for my purposes. Ideally this change could make it's way into your repo, if it's something you want. Let me know if that's something you'd like to talk about.

@davideweaver As I think you can understand, I'm hesitant towards accepting such changes into Watermelon. The problem is that "the ID of a record never changes" is an assumption that's probably hardcoded into a lot of places, and breaking this assumption could cause a lot of Watermelon stuff to break (perhaps in non-trivial ways). If you want to make a fork and work on this, I very much encourage you to do this — and if it turns out it's not so bad, I'll be very happy to pull your changes 🎉

But perhaps this isn't the best way to approach it? @fletling and his team, for example, chose to use a system to convert between local and remote IDs during synchronization. This has its downsides too, but this way, the trouble with local and remote IDs is only concentrated on the interface between local and remote database. So it's easier to figure out all ways it could go wrong and protect against them — and requires no (or few) changes to Watermelon.

Of course, we think it's best to have consistent IDs between local and remote databases, but understandably, this might not always be possible to people — and you probably noticed a lot of similar issues dealing with ID issues. So it would be great to figure out a Watermelon best practice for this.

@davideweaver
Copy link
Author

davideweaver commented Feb 5, 2019

@radex Thanks for the follow-up. I understand that what I'm trying to do may not fit with your vision. At the same time, I am unable to control the ids of the service I'm syncing with and don't especially like the idea of maintaining a map between local and remote ids. So here's how I managed to make it work for now...

In markLocalChangesAsSynced I replace the record having the local id with a cloned record using the remote id. I'm sure there's a better way to clone the original record, but it seems to be working. The components observing the collection are behaving correctly as well.

export default function markLocalChangesAsSynced(
  db: Database,
  syncedLocalChanges: SyncLocalChanges,
  pushResults: ?SyncPushResult
): Promise<void> {
  ensureActionsEnabled(db)
  return db.action(async () => {
    // update and destroy records concurrently
    await Promise.all([
      db.batch(...map(prepareMarkAsSynced, recordsToMarkAsSynced(syncedLocalChanges))),
      destroyDeletedRecords(db, syncedLocalChanges),
    ])

    // pushResults is an optional return value from pushChanges
    // that contains the mapping from remote to local ids per collection
    if (pushResults && pushResults.updatedCreateTables) {
      for (const tableKey: TableName<any> in pushResults.updatedCreateTables) {
        if (tableKey) {
          const table = pushResults.updatedCreateTables[tableKey]
          for (const id in table) {
            const collection = db.collections.get(tableKey)
            const model = await collection.find(id)
            if (model) {

              // remove the original record
              await collection._destroyPermanently(model)

              // replace it with a synced clone of the original record having the remote id
              await collection.create(record => {
                const raw = {}
                for (const key in model._raw) {
                  if (key && key !== 'id' && key[0] !== '_') {
                    raw[key] = model._raw[key]
                  }
                }
                for (const key in model) {
                  if (key && key !== 'id' && key !== 'collection' && key !== 'syncStatus' && key[0] !== '_') {
                    record[key] = model[key]
                  }
                }
                raw.id = table[id]
                raw._status = 'synced'
                record._raw = sanitizedRaw(raw, collection.schema)
              })
            }
          }
        }
      }
    }

  }, 'sync-markLocalChangesAsSynced')
}

@cesargdm
Copy link
Contributor

cesargdm commented Feb 5, 2019

I would like to set my custom MongoDB IDs as the ids in the local DB so i can query easier a object based on the MongoDB ID I already retrieve by my backend service.

[UPDATE] Found a way at #7

@stale
Copy link

stale bot commented May 6, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label May 6, 2019
@radex radex removed the wontfix This will not be worked on label May 8, 2019
@stale
Copy link

stale bot commented Nov 4, 2019

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix This will not be worked on label Nov 4, 2019
@Bessonov
Copy link

Bessonov commented Nov 5, 2019

Yea, still relevant.

@stale stale bot removed the wontfix This will not be worked on label Nov 5, 2019
@Skullcan
Copy link

Skullcan commented Nov 18, 2019

I would like to set my custom MongoDB IDs as the ids in the local DB so i can query easier a object based on the MongoDB ID I already retrieve by my backend service.

[UPDATE] Found a way at #7

Foud this thread trying to achiev same you did. Looked the #7 thread but haven't found how you change the ID. Can someone help me plz?

@diegolmello
Copy link

Take a look at this file on our repo: https://github.com/RocketChat/Rocket.Chat.ReactNative/blob/develop/app/lib/methods/getRoles.js

It's a simple insert/update function (no delete) of roles requested from the server.
Note that we fetch all roles from watermelon and compare against the response to create or update.
It's verbose, but it works.

cc @radex just to check if there's a better idea on this case 😛

@Skullcan
Copy link

@diegolmello thank you! That´s what I was looking for.

@stale
Copy link

stale bot commented May 16, 2020

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix This will not be worked on label May 16, 2020
@Bessonov
Copy link

Yeah, still relevant.

@stale stale bot removed the wontfix This will not be worked on label May 16, 2020
@whalemare
Copy link

I have my own backend, that IDs I can't track.
Why you prefer client side ids instead backend, if data that you get, received from backend 🧐

@radex
Copy link
Collaborator

radex commented Aug 17, 2020

Why you prefer client side ids instead backend

No, we prefer client and backend IDs being equivalent, because it makes things a lot simpler. Explained here: https://github.com/Nozbe/WatermelonDB/blob/master/docs-master/Advanced/Sync.md#local-vs-remote-ids

@stale
Copy link

stale bot commented Jun 18, 2021

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix This will not be worked on label Jun 18, 2021
@KennanChan
Copy link

KennanChan commented Apr 13, 2022

In my project I did some trick on this to solve the id problem

The client side database generates its own random id for a local record "A".
The backend receives the record "A" and inserts a cloned record "ClonedA" to cloud database with server generated id.
The record "ClonedA" has a special field "clientId" to keep track of the id of record "A"
In the next pulling, the record "ClonedA" is pulled to the client.
I use an adapter to do some transformation to the changes.
The adapter scans the changes and finds that the created record "ClonedA" has a field "clientId". Then it adds the "clientId" value(id of record "A") to the deleted changes.
The database will then insert the record "clonedA" and delete the local record "A", which acts like a replacement.

If the id of Record "A" is a foreign key of another record. The server is responsible to replace the id of "ClonedA" to that record. The client will automatically update the relation on the next pull.

BTW, my server uses integer id, so I have to do this whole workaround like this (the adapter also handles the id type changing in both pull and push phase). If your server uses string id, you can simplify the trick by inserting both "A" and "ClonedA" into the database with the "updateAt" field carefully handled, and mark "A" as logically deleted.

@stale stale bot removed the wontfix This will not be worked on label Apr 13, 2022
@heliojunior
Copy link

@KennanChan, do you have an example of the adapter you made? I'm not finding an example adapter.

Thanks

@KennanChan
Copy link

KennanChan commented Jun 27, 2022

@heliojunior

I have an interface that defines an adapter to handle table changes

export interface TableChangesAdapter {
  // handles pulled changes
  toLocal(changes: SyncTableChangeSet, database: Database): Promise<SyncTableChangeSet>;
  // handles pushing changes
  toRemote(changes: SyncTableChangeSet, database: Database): Promise<SyncTableChangeSet>;
}

Then I have an implementation to handle local id

export class HandleLocalRecordsAdapter implements TableChangesAdapter {
  // if a server record has a localId, add the localId to deleted to delete local record
  toLocal(changes: SyncTableChangeSet): Promise<SyncTableChangeSet> {
    if (changes && changes.created) {
      changes.created.forEach((record: DirtyRaw) => {
        if (record.localId) {
          if (!changes.deleted) {
            changes.deleted = []
          }
          changes.deleted.push(record.localId)
        }
        return record
      })
    }
    return Promise.resolve(changes)
  }

  // if a local record is created, add a localId to keep track of the record on the next pulling
  toRemote(changes: SyncTableChangeSet): Promise<SyncTableChangeSet> {
    if (changes && changes.created) {
      changes = {
        ...changes,
        created: changes.created.map((record: DirtyRaw) => {
	       return {
	         ...record,
	         localId: record.id,
	         id: null // use null to be compatible with non-string server id
	       }
        })
      }
    }
    return Promise.resolve(changes)
  }
}

Assume we have created a local record:

const locallyCreatedRecord: User = {
	id: "random-id",
	name: "kennan"
}

Right before pushing, it will be passed into the HandleLocalRecordsAdapter.toRemote() method

const userTableChanges = {
	created: [locallyCreatedRecord],
	updated: [],
	deleted: []
}
const handledUserTableChanges = await handleLocalRecordsAdapter.toRemote(userTableChanges)

The record is transformed into this:

const modifiedLocallyCreatedRecord: User = {
	id: null,
	name: "kennan",
	localId: "random-id",
}

Then the modified changes will be pushed to the server.

The server just clones the record and generates server-side id for the cloned record.

On the next pulling, the client receives a record like this:

const remoteRecord: User = {
	id: "server-id",
	name: "kennan",
	localId: "random-id"
}

Right after pulling, it will be passed into the HandleLocalRecordsAdapter.toLocal() method

const remoteUserTableChanges = {
	created: [remoteRecord],
	updated: [],
	deleted: []
}
const handledRemoteUserTableChanges = await handleLocalRecordsAdapter.toLocal(remoteUserTableChanges)

The changes is transformed into this:

const handledRemoteUserTableChanges = {
	created: [remoteRecord],
	updated: [],
	deleted: ["random-id"]
}

The changes is then handled by watermelon db.

If you have multiple models that are related, it is the server to replace the relations with the newly generated server ids. The client data just hooks up automatically.

It's better to start another synchronize right after the local records are pushed to the server, which is good for consistency.

@heliojunior
Copy link

Thanks @KennanChan,

I implemented the same pattern and it worked great.

thanks for your help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants