search

geddy / model

Datastore-agnostic ORM in JavaScript
265 stars 55 forks source link

Allow "id" property to be set manually #225

Closed mshick closed 9 years ago

mshick commented 10 years ago

It would be nice if the id property could be set manually, rather than requiring a system generated value.

My use-case is wanting to use a custom hashing routine to create locally unique keys, but also have a primary-key based, fast uniqueness test. Imagine, I require a unique username and am using the LevelDB adapter. So, I hash the entered username string, do a Model.first({id: 'userHash'}) test, and know my answer much more efficiently than iterating all the keys.

The current behavior is that when I provide a custom id during Model.create() it is stripped from the data sent to the adapter, because it's not in the schema.

When I then define an id property in my schema, I receive the error:

You cannot define the property "id" on a model, as it's a reserved system-property name.

The cleanest resolution, to my mind, would be to add support for a force flag on property definitions. This maintains the current practice, which is wise in most cases, but allows a way around it for situations like mine.

A PR is forthcoming.

mshick commented 10 years ago

Well shucks, after writing all that I found the magical isSystem flag... Does exactly what I needed.

Since I didn't see that documented, can somebody confirm that I'm not doing a Bad Thing by using that in my app?

mde commented 10 years ago

That depends on how you're using the flag. Can you give us an example of what you're doing?

Model itself wants to own the id field so that it can use either string UUIDs or auto-incrementing numbers. And I believe you can still set a custom id on instances -- you just don't define the id property in your schema.

mshick commented 9 years ago

Closing this out since isSystem works for me right now.

mde commented 9 years ago

Glad you were able to get it sorted.