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

Make it possible to overwrite client id prefix; also enables catching duplicate ids & cids #320

Conversation

doublerebel
Copy link

Currently, uid can never detect id or cid collisions because it has no idea of the prefix set for cid.

I believe because uid is only called in one place, the prefix should be set at uid. Then uid can be overridden to provide a custom prefix and also to detect collisions. Therefore it will be a truly "unique id".

I discovered this problem when extending the model to use local storage.

For example:

Using Spine.Model.Local
say Local has ids c-1
fetch() then will end up assigning cid c-0 to item with id c-1

Then in create...

...create new id, will have cid c-1, which is then set as id...

@id          = @cid unless @id

...and then overwrites the old record...

@constructor.records[@id]   = record
@constructor.crecords[@cid] = record

...this is further finalized when saving.

@doublerebel
Copy link
Author

After this patch is applied, catching duplicates is as easy as this commit:
doublerebel@759ef73

@maccman
Copy link
Member

maccman commented Jun 4, 2012

Sorry, I'm still struggling to understand the issue. Can you provide a simple test case demonstrating it?

@maccman
Copy link
Member

maccman commented Jun 4, 2012

Does my latest push fix this?

@maccman
Copy link
Member

maccman commented Jun 5, 2012

@doublerebel ping

@doublerebel
Copy link
Author

Yes, commit e06166 to detect duplicates does solve the duplicate id issue. For reference, I've uploaded the test case. The meat of it is here:
https://github.com/doublerebel/spinebugexample/blob/master/app/index.coffee

The example is running with Spine 1.0.7 from npm at http://lager.doublerebel.com:9001/
The same example with Spine from Github master is running at http://lager.doublerebel.com:9002/

As can be seen in either example, uid is called at least twice for every create -- possibly more in the latest version as uid is now recursive. The patch in pull req 319 will prevent the extra calls.

@doublerebel
Copy link
Author

Separately, I had moved 'c-' into the uid fn for the case when a custom clientside id scheme is required. Having 'c-' in the constructor is difficult to overwrite. If it is instead hardcoded in find or uid then those methods can be easily be extended and overwritten.

@aeischeid
Copy link
Member

think some of what you were going for in this was handled by #451

@aeischeid aeischeid closed this Feb 10, 2014
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

Successfully merging this pull request may close these issues.

None yet

3 participants