neumino / thinky

JavaScript ORM for RethinkDB
http://justonepixel.com/thinky/
Other
1.12k stars 128 forks source link

Hooks? #53

Closed mshick closed 10 years ago

mshick commented 10 years ago

I'm currently looking to replace JugglingDB with Thinky. As such, I find myself looking for a few features JDB supports that have proved very useful. Hooks is one.

In JDB there are several predefined points in the validate/update/create/save chain, where you can perform an operation on the doc to be saved. A very simple use case is, if I wanted to set a new updatedAt time on stories before they get saved to the database, I can do:

Model.beforeUpdate = function(next, data) {
  data.updatedAt = Date.now();
}

Perhaps a Thinky implementation would be useful / possible?

Here's a full list, FYI: https://github.com/1602/jugglingdb#hooks

neumino commented 10 years ago

There is an event "saving" that get emitted before the document is saved, so you can do something like

var User = thinky.createModel("user", {
    name: String,
    updatedAt: Date
})
var user = new User({name: "Marc"});
user.on("saving", function(doc) { doc.updatedAt = new Date() } )
user.save();

Does that work for you?

mshick commented 10 years ago

I saw that, but is that going to reliably modify the document before it gets saved, if, say, I need to perform an async operation (hashing algo) inside the event handler?

Forgive my ignorance here, but I’ve never quite gotten this detail of eventEmitters — do they block until all have returned? What if one caused an error, should it throw?

Beyond that, there’s a bit more info being provided through the JDB hooks; whether the doc is new or existing. I suppose an id check could suffice for that.

But one detail, that I found to be very useful about the JDB validate hook was the ability to transform an existing value to provide a schema-required value if that value wasn’t initially give.

Imagine something like:

Post.on(“validating”, function (post) {
  post.slug = slugify(post.title);
});

Where the lack of a user-provided title and slug will cause a validation error, but the lack of a slug alone will not.

This brings me to another question about defining validators in Thinky, but I’ll open another issue for that...

On Apr 30, 2014, at 4:04 PM, Michel notifications@github.com wrote:

There is an event "saving" that get emitted before the document is saved, so you can do something like

var User = thinky.createModel("user", { name: String, updatedAt: Date }) var user = new User({name: "Marc"}); user.on("saving", function(doc) { doc.updatedAt = new Date() } ) user.save(); Does that work for you?

— Reply to this email directly or view it on GitHub.

mshick commented 10 years ago

Just did a simple test with a process.nextTick() in the saving event, and the results were not incorporated into the model that got saved to the DB, so, it looks like I answered my question there.

Can you perhaps see how a the hook, with the series / next implementation could be useful?

I can obviously build around Thinky to provide this sort of stuff in custom methods, but they seem like reasonable concerns for the ORM itself.

neumino commented 10 years ago

Ah I see.

If you currently need an asynchronous operation before saving a document or to validate a document, thinky will not wait for it -- I mean the event is fired, but the operation will not wait.

I can probably add that. Just for my own curiosity, what kind of asynchronous operation are you executing before saving a document? Do you validate an email with a third party service? Or something different?

mshick commented 10 years ago

The best example of something I've actually been using is generating a bcrypted password hash before saving a user doc.

But I could imagine many scenarios where something like this would be useful. External validations, yes, maybe even queries against other Thinky models...

neumino commented 10 years ago

Generating a bcrypted password can be done in a synchronous way, so events should be enough -- https://www.npmjs.org/package/bcrypt-nodejs

I'll still add something for asynchronous events.

mshick commented 10 years ago

Yup, I realized.

Thanks for considering.

As I get more familiar with the thinky code hopefully I can offer PRs and not just issues.

On Apr 30, 2014, at 5:20 PM, Michel notifications@github.com wrote:

Generating a bcrypted password can be done in a synchronous way, so events should be enough -- https://www.npmjs.org/package/bcrypt-nodejs

I'll still add something for asynchronous events.

— Reply to this email directly or view it on GitHub.

jcspencer commented 10 years ago

A way that asynchronous events could work is that events are passed a callback, and execution continues after the callback is fired. And if the value passed is a falsey value, we throw an error.

jcspencer commented 10 years ago

This API could work like this:

user.on("saving", function(doc, callback) {
    doc.updatedAt = new Date();
    return callback(); //all okay, continue with execution
    /* or */
    return callback(new Error('something went horribly wrong')); // something died, halt execution and throw error
});

This behaviour is used in ActiveRecord in the before_save block. If an error is thrown, the execution halts and the document is not saved.

neumino commented 10 years ago

I'll probably do this one with #56 since both require to shuffle the same code (and quite a lot).

neumino commented 10 years ago

Hooks are shipped in 1.13.9 - And they can be asynchronous.