Implement operation hooks for EmbedsMany methods

[bajtos]

create() triggers

• before save
• after save

updateById() triggers

• before save
• after save

destroy() triggers

• before delete
• after delete

The implementation here is intentionally left with less features
than the regular DAO methods provide, the goal is to get a partial
(but still useful!) version released soon.

Limitations:

• before save & after save hooks don't provide ctx.isNewInstance
• async validations are not supported yet
• persist and loaded hooks are not triggered at all
• before delete hook does not provide ctx.where property and
  it's not possible to change the outcome of destroy() using this
  hook. Note that regular DAO does support this.
• updating embedded instances triggers update of the parent (owning)
  model, which is correct and expected. However, the context provided
  by before save and after save hooks on the parent model is sort of
  arbitrary and may include wrong/extra data. The same probably applies
  to the scenario when deleting embedded instances triggers update of
  the parent model.

Connect to #497
See the original implementation proposed by @fabien in #516

/to @raymondfeng @Amir-61 please review

[Amir-61]

Nice 👍

Thanks Miroslav for keeping me in the loop :-)

I went through the logic and it seems although you have added some tests in embeds-many-destroy.suite.js for destroy which should trigger before delete and after delete, the actual implementation in not added... I guess you forgot to add the implementation for before delete and after delete...

Also I liked your suggestion here for breaking lib/dao.js into multiple smaller files like lib/dao/patch-by-id.js, lib/dao/create.js and ... Shouldn't this refactoring be started for relation-definition.js as well? Right now the file is huge... I guess we should start refactoring this file into multiple files, what is your thought?

Thanks :-)

[bajtos]

@Amir-61 thank you for reviewing the code.

I went through the logic and it seems although you have added some tests in embeds-many-destroy.suite.js for destroy which should trigger before delete and after delete, the actual implementation in not added... I guess you forgot to add the implementation for before delete and after delete...

Ah, good catch, makes me wonder how could I miss this 8-O. Anyways, I added a fixup commit a1f56e6 which adds missing implementation and fixes another test failure.

Also I liked your suggestion here for breaking lib/dao.js into multiple smaller files like lib/dao/patch-by-id.js, lib/dao/create.js and ... Shouldn't this refactoring be started for relation-definition.js as well? Right now the file is huge... I guess we should start refactoring this file into multiple files, what is your thought?

I agree we should eventually split lib/relation-definition.js too, that's a great idea.

However, I want to keep this pull request small and focused, I don't want to move code around here.

In my comment, I was proposing to apply the new approach only for new code. That's what I did here too: new tests for operation hooks are following the new convention, but the old tests in test/persistence-hooks.suite.js remain there unchanged. Because I am not introducing any new relation methods, I am not creating any new files in lib folder.

Now I don't mind refactoring the old code too, we should eventually do it, but right now I feel there are more important things to address compared to this refactoring.

[raymondfeng]

LGTM

[Amir-61]

LGTM as well... I just wonder why 12 tests pass out of 49 tests.

Added: I re-run tests

[Amir-61]

@slnode test please

[frankorama]

Are you planning on providing the isNewInstance property for the 'before save' and 'after save' methods in the future?