Hooks #503
Hooks #503
Conversation
ShogunPanda
requested review from
mcollina,
dberesford,
mihaidma and
cianfoley-nearform
There was a problem hiding this comment.
This looks great - minor comments but overall fantastic job @ShogunPanda
packages/udaru-core/README.md
Outdated
| @@ -36,6 +36,35 @@ udaru.organizations.list({}, (err, orgs) => { | |||
|
|
|||
| ``` | |||
|
|
|||
| ## Hooks | |||
|
|
|||
| Hooks are registered using the `udaru.addHook` method and allow you to listen to specific events in udaru life. | |||
packages/udaru-core/README.md
Outdated
|
|
||
| Each udaru method exposes a namespaced hook (e.g.: the `udaru.authorize.isUserAuthorized` method exposes the `authorize:isUserAuthorized` hook). | ||
|
|
||
| The hook is a node-style callback with three arguments: the method arguments, the method result values and a callback to invoke once done |
There was a problem hiding this comment.
small nit, missing full stop
|
|
||
| udaru.addHook('authorize:isUserAuthorized', function (error, args, result, done) { | ||
| if (error) { | ||
| console.error(`Authorization errored: ${error}`) |
There was a problem hiding this comment.
so this is the contentious bit, IMO this should be:
if (error) {
return done(error)
}
There was a problem hiding this comment.
You're right. This was a legacy of first draft when the errors were swallowed. Updating it.
|
|
||
| udaru.addHook('authorize:isUserAuthorized', function (error, args, result, done) { | ||
| if (error) { | ||
| console.error(`Authorization errored: ${error}`) |
| addPolicies: hooks.wrap('organization:addPolicies', organizationOps.addOrganizationPolicies), | ||
| replacePolicies: hooks.wrap('organization:replacePolicies', organizationOps.replaceOrganizationPolicies), | ||
| deletePolicies: hooks.wrap('organization:deletePolicies', organizationOps.deleteOrganizationAttachedPolicies), | ||
| deletePolicy: hooks.wrap('organization:deletePolicy', organizationOps.deleteOrganizationAttachedPolicy) |
There was a problem hiding this comment.
so does this mean that we can have hooks on all these functions? I wouldn't of seen the need to wrap read and list commands for example, but I guess if we get them for free, why not..
There was a problem hiding this comment.
I don't see many use cases as well, but who knows. Might be useful for security monitoring.
Anyway each wrapping has small cost on bootstraping.
When the handlers list is empty each call will result in additional function invoked with iteration on a empty array. So it shouldn't impact performances at all. Especially when these methods are not really used. :)
packages/udaru-hapi-plugin/README.md
Outdated
| @@ -27,6 +28,36 @@ const server = new Hapi.server() | |||
| server.register({register: UdaruPlugin}) | |||
| ``` | |||
|
|
|||
| In order to register udaru hooks, just provide a `hooks` key in the plugin options where keys are the names and values are handler functions (or array of functions). | |||
There was a problem hiding this comment.
Please add ## Hooks heading here
Implements #502