Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow usage of a function with plugin hook, pipe and controller actions #1227

Merged
merged 17 commits into from Jan 22, 2019
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions lib/api/core/plugins/pluginContext.js
Expand Up @@ -95,9 +95,6 @@ class PluginContext {
}
});

/**
* @deprecated will be removed in v2, replaced by "sdk" accessor
*/
Object.defineProperty(this.accessors, 'execute', {
enumerable: true,
get: () => (...args) => execute(kuzzle, ...args)
Expand Down Expand Up @@ -184,7 +181,6 @@ class PluginContext {
}

/**
* @deprecated will be removed in v2, replaced by "sdk" accessor
* @param {Kuzzle} kuzzle
* @param {Request} request
* @param {Function} [callback]
Expand Down
47 changes: 29 additions & 18 deletions lib/api/core/plugins/pluginsManager.js
Expand Up @@ -353,7 +353,7 @@ class PluginsManager {
* @param {number} warnDelay - delay before a warning is issued
* @param {number} timeoutDelay - delay after which the function is timed out
* @param {string} event name
* @param {function} fn - function to attach
* @param {string|function} fn - function to attach
*/
registerPipe(plugin, warnDelay, timeoutDelay, event, fn) {
debug('[%s] register pipe on event "%s"', plugin.manifest.name, event);
Expand All @@ -370,7 +370,7 @@ class PluginsManager {

if (warnDelay) {
pipeWarnTimer = setTimeout(() => {
this.trigger('log:warn', `Plugin pipe ${plugin.manifest.name}:${fn} exceeded ${warnDelay}ms to execute.`);
this.trigger('log:warn', `Plugin pipe ${plugin.manifest.name}:${fn} for event '${event}' exceeded ${warnDelay}ms to execute.`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error message must be fixed: fn can now be a function. There are other occurences below to be fixed.

}, warnDelay);
}

Expand Down Expand Up @@ -401,7 +401,14 @@ class PluginsManager {
}
};

const pipeResponse = plugin.object[fn](data, cb);
const pipeResponse = (() => {
if (typeof fn === 'function') {
return fn(data, cb);
xbill82 marked this conversation as resolved.
Show resolved Hide resolved
}

return plugin.object[fn](data, cb);
})();

if (typeof pipeResponse === 'object'
&& pipeResponse !== null
&& typeof pipeResponse.then === 'function'
Expand All @@ -425,14 +432,18 @@ class PluginsManager {
*
* @param {object} plugin
* @param {string} event
* @param {string} fn - function to attach
* @param {string|function} fn - function to attach
*/
registerHook(plugin, event, fn) {
debug('[%s] register hook on event "%s"', plugin.manifest.name, event);

this.kuzzle.on(event, message => {
try {
plugin.object[fn](message, event);
if (typeof fn === 'function') {
fn(message, event);
xbill82 marked this conversation as resolved.
Show resolved Hide resolved
} else {
plugin.object[fn](message, event);
}
}
catch (error) {
throw new PluginImplementationError(error);
Expand Down Expand Up @@ -580,15 +591,15 @@ class PluginsManager {
_timeout = plugin.config.pipeTimeout;
}

_.forEach(plugin.object.pipes, (fn, pipe) => {
_.forEach(plugin.object.pipes, (fn, event) => {
const list = Array.isArray(fn) ? fn : [fn];

for (const target of list) {
if (!_.isFunction(plugin.object[target])) {
throw new PluginImplementationError(`Unable to configure pipe with method ${plugin.manifest.name}.${target}: function not found`);
if (typeof target !== 'function' && typeof plugin.object[target] !== 'function') {
throw new PluginImplementationError(`Unable to configure pipe for event '${event}' with provided method. ${target} should be a plugin method name, or a function.`);
}

this.registerPipe(plugin, _warnTime, _timeout, pipe, target);
this.registerPipe(plugin, _warnTime, _timeout, event, target);
}
});
}
Expand All @@ -601,8 +612,8 @@ class PluginsManager {
const list = Array.isArray(fn) ? fn : [fn];

for (const target of list) {
if (!_.isFunction(plugin.object[target])) {
throw new PluginImplementationError(`Unable to configure hook with method ${plugin.manifest.name}.${target}: function not found`);
if (typeof target !== 'function' && typeof plugin.object[target] !== 'function') {
throw new PluginImplementationError(`Unable to configure hook for event '${event}' with provided method. ${target} should be a plugin method name, or a function.`);
}

this.registerHook(plugin, event, target);
Expand Down Expand Up @@ -631,19 +642,19 @@ class PluginsManager {
Object.keys(description).forEach(action => {
debug('[%s][%s][%s] starting action controller registration', plugin.manifest.name, controller, action);

if (typeof description[action] !== 'string' || description[action].length === 0) {
throw new PluginImplementationError(`${errorControllerPrefix} Invalid action description (expected non-empty string, got: "${typeof action}")`);
}

if (!_.isFunction(plugin.object[description[action]])) {
throw new PluginImplementationError(`${errorControllerPrefix} Action "${plugin.manifest.name + '.' + description[action]}" is not a function`);
if (typeof description[action] !== 'function' && typeof plugin.object[description[action]] !== 'function') {
throw new PluginImplementationError(`${errorControllerPrefix} Action for '${controller}:${action}' is not a function`);
}

if (!this.controllers[`${plugin.manifest.name}/${controller}`]) {
this.controllers[`${plugin.manifest.name}/${controller}`] = {};
}

this.controllers[`${plugin.manifest.name}/${controller}`][action] = plugin.object[description[action]].bind(plugin.object);
if (typeof description[action] === 'function') {
this.controllers[`${plugin.manifest.name}/${controller}`][action] = description[action];
} else {
this.controllers[`${plugin.manifest.name}/${controller}`][action] = plugin.object[description[action]].bind(plugin.object);
}
});
});

Expand Down