Added virtual properties for all fields defined in the schema

[jfbenckhuijsen]

I've added some logic to create virtual properties for each of the fields in the schema. This way I can access properties directly via MyModel.myProperty instead of MyModel.entityData.myProperty, same as can be done in Mongoose

[sebelga]

It's interesting. Although I don't think we could use it just like this in gstore because properties from the Schema could override internal properties of the Model. I just made a test and I could override "constructor", "gstore", and also a custom method that I had on the Schema (ex: userSchema.method('verifyPassword', verifyPassword)).

Do you know if Mongoose define a set of reserved property names that can not be used for the Models definition?

[jfbenckhuijsen]

As for keywords override internal fields, looks like Mongoose also has this issue and has defined some reserved keywords. See http://mongoosejs.com/docs/api.html#schema_Schema.reserved

As for properties overriding custom methods: from a design point of view, it would be very strange to define an object (looking at Models in an object oriented way) having a property and a function with the same name. So I think that should be the developers responsibility to keep them separated (i.e. having both a birthday property and a birthday method om the same object would be strange).

Concerning this patch: I can imagine adding a test to verify whether property overrides one of the reserved words for GStore-node and throw an error in that case. To preserve backwards compatibility we can add an option to the schema or model to enable this functionality an the reserved word checking. Not sure if preserving this kind of backwards compatibility is a hot issue for you.

[sebelga]

Actually I forgot but the same behaviour of reserved was copied from Mongoose (https://github.com/sebelga/gstore-node/blob/master/lib/schema.js#L175). :) So yeah we could add a few more properties on that hashtable ("gstore", "entityData", "constructor" and any other we might think of).

Also could you add some tests and update the README.md? I think the documentation could go under Entity -> Properties. Mention that virtual properties are automatically added and mapped from entityData. (they can both be accessed and set) + a short code sample?

I think we can safely add this new feature and not worry too much about those few new reserved properties that might break some projects. I think it shouldn't affect almost anyone (I hope!). I am in the middle of adding #11 (Promises) where there will be more concern about some breaking change. ;)

[jfbenckhuijsen]

Will do, I'll be at a conference in the coming days, so that'll be next week before I can add them.

[sebelga]

Great, no hurry at all. thank you!

[sebelga]

Great stuff! I think you can safely delete the test you added at line 106 as it is covered by the one on L302. Also could you squash this in 1 commit. I know I haven't been doing this much (at all) but from now on I'd like to squash features/fixes from branches in 1 commit before merging to master. :) thanks!

[jfbenckhuijsen]

Squash -> NP, was still working on it a bit.
Test -> Your call, I personally like my test small and focussed on testing individual stuff, but thats just a matter of taste ;-)

[jfbenckhuijsen]

Test is removed. Squashing will be an issue: I had a merge of your master in between commits, messing up the squashing. Will take that into account for other changes, but won't work for now.

[sebelga]

Great thank you. As for the tests yes you're right question of taste. I am now reviewing all the tests 1 by 1 for the Promise support and that's why my brain is probably thinking 0 & 1 in tests related stuff :). Your merge should hopefully be released early next week with Promises support. thanks again!