Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Allow clients to specify/override local id generator for new objects #26

Open
wtrocki opened this issue Jul 25, 2017 · 6 comments
Open

Comments

@wtrocki
Copy link
Member

wtrocki commented Jul 25, 2017

Background

When creating new object in sync result function returns auto generated id that may be used to reference and obtain that specific object in the future. Once this object is synced actual value will be replaced with the server side id. Fact that object id is auto generated and that we replacing it later is causing a lots of issues and making entire sync really hard to understand.

Proposition

As solution sync users may just generate identifiers themselves (or use already existing fields that they know they are unique. Then sync framework should have option to specify function or field that will be used as id. Thanks to that users can use this ID's in other objects without worrying about swapping id or holding localy generated identifiers as separate fields.

How this will work

var obj = {
  id:"1234325234",
  name:"Wojtek"
}

var idGenerator = function(object){
      return object.id;
};

// Suggestion to override method
sync.manage("test",{idGenerator: idGenerator},{},{});

// We can reference ID at any time and assign that to different dataset.
var ref={ testRef:object.id };

// We save object and do not care about what's being returned
sync.doSave("test", obj);

// Saving reference that has object ID. 
sync.doSave("testReference", ref);

// This should work not matter if we are in sync with server or offfline
sync.doRead("test", ref.testRef); 

Additional requirements

In order for this to fully work we will also need to provide custom data handler that
will use the same obj.id as id instead of the generated mongo ObjectId.
I see this as the best pattern for the moment to work with the sync.

This will ensure that ID stays the same no matter if user is offline/online, data was synced etc.

@wtrocki
Copy link
Member Author

wtrocki commented Jul 25, 2017

ping @wei-lee @paolobueno comments.

@paolobueno
Copy link

👍 I think a safe recommendation (or even default) would be generating UUIDs on the client for stable ids.

Your example with a timestamp is subject to race conditions i.e. two offline clients creating an object at the same time.

@wtrocki
Copy link
Member Author

wtrocki commented Jul 25, 2017

I did not mean to use some UUID generator in example, but this implementation will need to use it. or even sync can provide that to be consistent.

@evanshortiss
Copy link
Member

evanshortiss commented Jul 25, 2017

Fact that object id is auto generated and that we replacing it later is causing a lots of issues and making entire sync really hard to understand - This makes sense though, it's a temporary local ID until it synchronises with the remote data store which assigns a safe unique ID. Local UUID is also very very safe of course.

I think for MongoDB datastore this idea works well, but for custom sync handlers talking to systems such as Sharepoint and SQL databases could be an issue. Those systems might not have a field to store the UUID a client uploads and instead return an ID of their own. So for those systems this change won't work without redesign on the customer system side.

Example with current sync:

sync.handleCreate('stuff', (dataset, data, meta, cb) => {
  sqldb.create(data, (err, record) => {
    if (err) {
      cb(err)
    } else {
      cb(null, {
        uid: record.id, // db generated id
        data: record // entire db json
      })
    }
  })
})

Possible new sync requirement to return client ID?

sync.handleCreate('stuff', (dataset, data, meta, cb) => {
  sqldb.create(data, (err, record) => {
    if (err) {
      cb(err)
    } else {
      cb(null, {
        uid: data.id, // client generated id or record.clientID
        data: record // entire db json (including db row ID)
      })
    }
  })
})

This new implementation could be an issue for handleRead if the customer DB cannot store the client UUID since read needs an ID shared by both client and backend data stores. This could also affect DB performance due to new indexes being needed for the client ID column?

This is an edge case, but worth thinking about. Anyway it's a nice idea for Dev UX, and I assume this would be optional as a feature?

@MikeyBurkman
Copy link
Contributor

What would be the ramifications for collisions on the new ID? I assume right now that isn't an issue because the server (which is the source of record) is providing the ID, but now the client is going to be required to provide a globally unique ID. Presumably that's an ID that is already present and unique in the database (idGenerator = (o) => o.projectID or something), but as @paolobueno pointed out, you could easily use the timestamp, thinking it to be unique, and then have collisions. (It's obviously not a good idea, but I can guarantee someone will do it.)

These collisions could occur in two places:

  • On the client, when the record is created, and the result of idGenerator() collides with an existing value in the collection.
  • On the server, if the client only sees a subset of the data, there is now a collision on the server with data that was not visible to the client.

We already have handlers for collisions now, but this seems to me like a different use case. This isn't just colliding changes to the same object--it's basically a schema validation error and the record shouldn't be created in the first place. (Like if you tried to insert a record into a database with a duplicate primary key.)

@wtrocki
Copy link
Member Author

wtrocki commented Aug 7, 2017

I think for MongoDB datastore this idea works well, but for custom sync handlers talking to systems such as Sharepoint and SQL databases could be an issue.

This will be reference sync implementation for mongo only. Please note that we not going to change anything internally - just going to add option to include localid generator.

The best option will be to enable sync client to connect and work with different data stores (like sqllite etc.) This change will allow us to do it as id generated by sql lite (id sequence) can be then used and synced back to mongo database.

Possible new sync requirement to return client ID?

Nothing changes in that area. Users can still return different id from backend, but that's going to be swapped. If they do not wish id to be swapped same id as in client may be returned.

These collisions could occur in two places:

We will model this process so no collisions will happen. Overall goal is to simplify interaction with our sync library and provide best patterns. If any of this will cause collisions (I hope not) we should not recommend this (or use in our products like RainCatcher)

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

No branches or pull requests

4 participants