Add ctx. isNewInstance to "before save" and "after save" hooks

[pungoyal]

Extend before save and after save hooks and add a new property ctx.isNewInstance (or similar).

This new property will have one of the following values for Model.updateOrCreate:

• before save:
  • true when the data has not id and the method falls back to a regular create
  • true/false when a non-atomic implementation is used
  • undefined when an atomic implementation is used
• after save:
  • true when the connector reported that a record was created
  • false when the connector reported that a record was updated
  • undefined when the connector did not report anythin

A thing to figure out: how to handle atomic findOrCreate, as it may fire the before save hook even if the model already exists?

---

The original issue description

Does it make sense to have a created event on model? Currently we have changed, but that fires for any changes to the model.

[clark0x]

I have same question. Currently, I fire a created event in afterCreate hook and an updated event in afterUpdate hook in all my models.

I lookup the PR history, this loopbackio/loopback-datasource-juggler#65 introduces changed, deleted, deletedAll by @ritch .

To @ritch so, what do you think?

[ritch]

+1 for created event

[bajtos]

This should be a reasonably easy change, would you (@pungoyal or @clarkorz) mind to contribute it?

[glesage]

A state other than beforeCreate and afterCreate ? (which already exists)

[bajtos]

It is probably better to defer this enhancement until loopbackio/loopback-datasource-juggler#367 is implemented.

[bajtos]

The new Operation hooks implemented by loopbackio/loopback-datasource-juggler#367 provide after save hook which does not distinguish between created and updated. One of the reasons is that this information may not be available from updateOrCreate (a.k.a upsert and HTTP PUT) implemented by connectors as an atomic operation.

However, if you are able to come up with a way how to distinguish between CREATE and UPDATE that works in all connectors, then I am ok with adding a new property to the ctx object passed to after save hook and use this property to decide which event to fire.

Another alternative is to let updateOrCreate() trigger always updated event, even in the case where a new model instance was created.

Thoughts?

[clark0x]

@bajtos loopback-connector-mongodb implements updateOrCreate by using findAndModify. The result that findAndModify command responses could be use to distinguish FIND, CREATE, and UPDATE.

For loopback-connector-mysql, I am not familiar with mysql, but I do find a hint in mysql document:

With ON DUPLICATE KEY UPDATE, the affected-rows value per row is 1 if the row is inserted as a new row, 2 if an existing row is updated, and 0 if an existing row is set to its current values.

I don't study all others connectors, but can we tolerate that some connectors could distinguish between CREATE and UPDATE while others couldn't?

[bajtos]

@clarkorz thank you for making the research.

Based on what you have described above, I am proposing the following implementation: extend after save hook and add a new property ctx.wasCreated (or ctx.isNewInstance or similar).

This new property will have one of the following values for Model.updateOrCreate:

1. true when the connector reported that a record was created
2. false when the connector reported that a record was updated
3. undefined when the connector did not report anything

The benefit of such solution is that a simple check if (!ctx.isNewInstance) will catch both 2 & 3, while it is still possible to distinguish between 2 and 3 if needed.

Of course, methods that already know what kind of write is being performed (Model.create, Model.updateAll, etc.) will set this flag independently on the connector implementation.

Thoughts?

/cc @raymondfeng @ritch

[clark0x]

LGTM

Besides the new hooks, should we complete the event system in advance? To specific, should we replace changed event with created and updated events, or pass a similar flag (as you describe above) when emitting changed event?

[bajtos]

We need to preserve the changed event for backwards compatibility.

The choice is then changed with flag, or emit both changed and created/updated. I slightly prefer the flag as it is consistent with after save, but I don't really mind either option.