Improve docs for Operation hooks

[bajtos]

Judging by the recent discussions in GitHub issues, the current minimal documentation does not make it clear how to correctly use the hooks and why they are designed in a seemingly unintuitive way.

The goal of this task is to write down all knowledge that's in Miroslav's head at the moment, add information from comments in the issues linked below, and beef up the doc page Operation hooks.

A list of pain-points can be found here:

• Pain-points of the new Operation hooks loopbackio/loopback-datasource-juggler#482

Related issues:

• updateAttributes operational hooks: use instance, not where loopbackio/loopback-datasource-juggler#481
• Operational hook 'after delete' should receive id(s) loopbackio/loopback-datasource-juggler#474
• Fix #481 and #474 loopbackio/loopback-datasource-juggler#483
• Shared state for hooks invoked for a single operation loopbackio/loopback-datasource-juggler#484
• https://github.com/strongloop/loopback-datasource-juggler/pull/488/files#r25886292

[chandadharap]

I'd rather that Rand sit with you and get the info to update and improve docs, rather than you write this.

[crandmck]

Miroslav, I will go through the above issues and try to extract the pertinent info. I will likely have questions as a result.... Then let's meet and you can give me a brain dump, as Chanda suggests. You could also jot down some notes if you have some specific things you know you'd like to add to or change in the docs.

[crandmck]

This may not be completed this sprint, since the feature is being significantly reworked.

Miroslav said:

there are still pull requests in progress that will improve certain areas, so this docs work may be a longer term affair.

[bajtos]

@crandmck first few things that are ready to be documented.

Document "hookState" - see loopbackio/loopback-datasource-juggler#485 and loopbackio/loopback-datasource-juggler#484

Capture the information from loopbackio/loopback-datasource-juggler#483 (comment). The first part explains why there is "instance" object instead of providing always a "data" object. The second part explains different use cases of hooks.

Explain why the "before delete" and "after delete" hooks don't receive a list of deleted ids, see the discussion in loopbackio/loopback-datasource-juggler#474. Explain that a naive implementation of hooks creates a race condition (loopbackio/loopback-datasource-juggler#474 (comment)) and describe how it can be avoided (loopbackio/loopback-datasource-juggler#474 (comment)).

NOTE: loopbackio/loopback-datasource-juggler#488 is adding ctx.instance to both "delete" hooks when they are deleting a single model, we will need to describe that once the PR is landed. Because batch delete is not exposed via REST API by default (IIRC), in most cases ctx.instance should be a good solution for getting data of the deleted model (including the id).

loopbackio/loopback-datasource-juggler#481 contains useful information too, e.g. the first part of loopbackio/loopback-datasource-juggler#481 (comment) explains why "updateAttributes" does not have ctx.instance.

NOTE: loopbackio/loopback-datasource-juggler#488 is adding ctx.currentInstance to "before save" hook triggered by "updateAttributes", this should somewhat alleviate the problem.

[bajtos]

Some material for starting a FAQ section: loopbackio/loopback-datasource-juggler#482 (comment)

[bajtos]

Re-estimating to 1 story point. I'll only gather more pointers to relevant comments and pull requests, so that @crandmck can write the content himself.

[crandmck]

@bajtos Added a note about ctx.options that Fabien added. However, we really need an example. See http://docs.strongloop.com/display/LB/Operation+hooks#Operationhooks-Operationhookcontextobject.

[bajtos]

@crandmck Can we get an example of how ctx.options might be used?

Here is a code snippet based on loopbackio/loopback-datasource-juggler#549 (comment).

var FILTERED_PROPERTIES = ['immutable', 'birthday'];
MyModel.observe('before save', function filterProperties(ctx, next) {
  if (ctx.options && ctx.options.skipPropertyFilter) return next();
  if (ctx.instance) {
    FILTERED_PROPERTIES.forEach(function(p) { ctx.instance.unsetAttribute(p); });
  } else {
    FILTERED_PROPERTIES.forEach(function(p) { delete ctx.data[p]; });
  }
  next();
});

// immutable is not updated
MyModel.updateOrCreate({ id: 1, immutable: 'new value' }, cb);

// immutable is changed
MyModel.updateOrCreate({ id: 2, immutable: 'new value' }, { skipPropertyFilter: true }, cb);

[crandmck]

Added the example code..thanks.

We still need to add all the items noted in the comment above.

@bajtos Were you planning to do this, or did you want me to do it?

[bajtos]

Were you planning to do this, or did you want me to do it?

Please do it yourself. My plan was only to collect a list of pointers for you.

[bajtos]

Actually there are no more features to add to the list in the comment above, I am reassigning this issue to you, @crandmck.

[crandmck]

OK, here is what I have for this:

• Document "hookState" - http://docs.strongloop.com/display/LB/Operation+hooks#Operationhooks-SharedhookStateproperty
• Explain why there is "instance" object instead of providing always a "data" object. etc.
  See http://docs.strongloop.com/display/LB/Operation+hooks#Operationhooks-Usinginstanceobjects
• Explain why the "before delete" and "after delete" hooks don't receive a list of deleted ids, etc
  See http://docs.strongloop.com/display/LB/Operation+hooks#Operationhooks-beforedelete
• Doc ctx.instance for both "delete" hooks when they are deleting a single model.
• updateAttributes operational hooks: use instance, not where loopbackio/loopback-datasource-juggler#481 contains useful information too

I'm going to need a bit more time to sort all this out, because it's fairly complex and there are an abundance of PRs and discussions to sort through.

[bajtos]

OK, here is what I have for this:
(...)

Sounds good!

FYI, there were two more hooks added recently, see loopbackio/loopback-datasource-juggler#599 and loopbackio/loopback-datasource-juggler#586. I am afraid the task of (fully) documenting operation hooks will be a long one.

[crandmck]

@bajtos It was difficult for me to sort out from the very lengthy discussions across multiple PRs what was actually done and needs to be documented. I gave it my best shot, but apologies if I missed something. I admit to still being somewhat confused.

Please review the updates referenced in the comment above and let me know what more needs to be added or changed. Some illustrative examples would help.

Re the two new hooks, see http://docs.strongloop.com/display/LB/Operation+hooks#Operationhooks-newhooks. Note this information is "hidden" until the new features are released to production.

[bajtos]

@crandmck thank you for updating the doc page. I am afraid I don't have enough time to review the changes now, I have created a new sprint story so that we can plan this task: https://github.com/strongloop-internal/scrum-loopback/issues/376 Sorry for the delay :(

[crandmck]

OK, np. It took me quite a while to get around to working on this myself.

[crandmck]

Closed in favor of https://github.com/strongloop-internal/scrum-loopback/issues/376.

[mike-aungsan]

Is it possible to terminate the invocation from Operation hooks?
I cannot find in doc. I try ctx.end() and it raise err.

[bajtos]

@mike-aungsan

Is it possible to terminate the invocation from Operation hooks?
I cannot find in doc. I try ctx.end() and it raise err.

You can terminate the invocation by raising an error via next(err).

In the future, please post questions to our mailing list and/or community forums, see https://github.com/strongloop/loopback/wiki/Questions.