Bug: onCreating Hook Not Executed When Creating Document Through Instance.save

[CatGuardian]

I have found a bug where the onCreating hook of an Instance/Model is not getting triggered when creating a document through the Instance.save method. Below is an example of what I mean:

 const userDoc: UserDocument = {
    name: seed,
    age,
  };

  const user = new db.Users.Instance(userDoc);

  // Doesn't trigger corresponding Model's onCreating hook.
  await user.save();

[notheotherben]

Well spotted and thanks so much for the PR, I've gone and gotten that merged and released 8.0.0-alpha.11 with your changes included.

[CatGuardian]

Awesome, thanks.

The only thing I wasn't sure of is that in the beginning of that save method, validation is performed when there are no changes object specified. And then later by me adding the handlers.creatingDocuments() call validations are ran again. That is because creatingDocuments run validations again after the onCreating hook gets a chance to modify the document. Not sure if that is a concern to you or not (I think validating the document again after the onCreating hook has a chance to change it is not necessarily a bad idea, in fact it sounds like a good idea? I'm just not sure what performance concerns exist by validating twice). The other way I debated doing it was calling hooks.onCreating directly but since you had used handlers I figured that was the right way to do it.

Thoughts? @spartan563

[notheotherben]

I think you did the right thing, there's a number of reasons why using the handlers is always the right approach (especially since it manages tasks like transforms as well as hooks and validation). The secondary advantage of validating after the hooks get executed is a nice to have and probably won't have too significant impact on performance.

As a side note, my view on performance is "If you're making changes to a single object, performance is probably not a significant concern, while if you're making changes to a lot of objects then it is" - you'll see that reflected in the way that many of the model-level methods are optimized to work well with large numbers of objects while the instance-level stuff is all optimized for a nice and reliable dev experience at the potential cost of some performance if you're trying to do bulk updates with it.

[CatGuardian]

Awesome. That is good insight that you have set up Model methods to be concerned with many objects where as the Instance methods concern themselves with object.

[CatGuardian]

I tested this out last night on my code where I discovered the bug and this fix did it.

Can probably close this issue.

[notheotherben]

Great stuff, I'll keep this open until 8.0.0-alpha.* is actually completely released, in case anybody else runs into this issue on 7.x.