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

Add pipes & hooks wildcard event #1305

Merged
merged 7 commits into from
May 27, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 36 additions & 15 deletions lib/api/core/plugins/pluginsManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -831,8 +831,12 @@ class PluginsManager {
* @returns {Promise}
*/
function triggerHooks(emitter, event, data) {
const wildcardEvents = getWildcardEvents(event);

emitter.emit(event, data);

wildcardEvents.forEach(wildcardEvent => emitter.emit(wildcardEvent, data));

return Bluebird.resolve(data);
}

Expand All @@ -845,15 +849,19 @@ function triggerHooks(emitter, event, data) {
* @returns {Promise}
*/
function triggerPipes(pipes, event, data) {
let preparedPipes = [];
const wildcardEvent = getWildcardEvent(event);
const preparedPipes = [];
const wildcardEvents = getWildcardEvents(event);

if (pipes && pipes[event] && pipes[event].length) {
preparedPipes = pipes[event];
preparedPipes.push(...pipes[event]);
scottinet marked this conversation as resolved.
Show resolved Hide resolved
}

if (wildcardEvent && pipes && pipes[wildcardEvent] && pipes[wildcardEvent].length) {
preparedPipes = preparedPipes.concat(pipes[wildcardEvent]);
if (wildcardEvents && pipes) {
wildcardEvents.forEach(wildcardEvent => {
if (pipes[wildcardEvent] && pipes[wildcardEvent].length) {
preparedPipes.push(...pipes[wildcardEvent]);
}
});
}

if (preparedPipes.length === 0) {
Expand All @@ -876,20 +884,33 @@ function triggerPipes(pipes, event, data) {
}

/**
* For a specific event, return the corresponding wildcard
* For a specific event, return the correspondings wildcards
* @example
* getWildcardEvent('data:create') // return 'data:*'
* @param {string} event
* @returns {String} wildcard event
* getWildcardEvents('data:create') // return ['data:*']
* getWildcardEvents('data:beforeCreate') // return ['data:*', 'data:before*']
* @param {String} event
* @returns {Array<String>} wildcard events
*/
function getWildcardEvent (event) {
const indexDelimiter = event.indexOf(':');
if (indexDelimiter !== 1) {
return event.substring(0, indexDelimiter+1) + '*';
const getWildcardEvents = _.memoize(event => {
const delimIndex = event.lastIndexOf(':');

if (delimIndex === -1) {
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

to prevent breaking code using the return value of this function without first checking that it didn't return null:

Suggested change
return null;
return [];

}

const scope = event.slice(0, delimIndex);
const name = event.slice(delimIndex + 1);
const wildcardEvents = ['*'];

['before', 'after'].forEach(prefix => {
if (!name.startsWith(prefix)) {
return;
}
wildcardEvents.push(`${prefix}*`);
});

return null;
}
return wildcardEvents.map(e => `${scope}:${e}`);
});

/**
* Test if the provided argument is a constructor or not
Expand Down
9 changes: 2 additions & 7 deletions lib/api/kuzzle.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
'use strict';

const
EventEmitter = require('eventemitter2').EventEmitter2,
EventEmitter = require('eventemitter3'),
config = require('../config'),
path = require('path'),
murmur = require('murmurhash-native').murmurHash128,
Expand Down Expand Up @@ -55,12 +55,7 @@ const
*/
class Kuzzle extends EventEmitter {
constructor() {
super({
verboseMemoryLeak: true,
wildcard: true,
maxListeners: 30,
delimiter: ':'
});
super();

/** @type {KuzzleConfiguration} */
this.config = config;
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"dumpme": "^1.0.2",
"easy-circular-list": "^1.0.13",
"elasticsearch": "^15.4.1",
"eventemitter2": "^5.0.1",
"eventemitter3": "^3.1.2",
"fs-extra": "^7.0.1",
"glob": "^7.1.3",
"ioredis": "^4.9.0",
Expand Down
24 changes: 8 additions & 16 deletions test/api/core/pluginsManager/run.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,8 @@ describe('PluginsManager.run', () => {
pluginMock.expects('bar').never();

return pluginsManager.run()
.then(() => {
kuzzle.emit('foo:bar');
pluginMock.verify();
});
.then(() => pluginsManager.trigger('foo:bar'))
.then(() => pluginMock.verify());
});

it('should attach event hook with function', () => {
Expand All @@ -78,9 +76,8 @@ describe('PluginsManager.run', () => {
};

return pluginsManager.run()
.then(() => pluginsManager.trigger('foo:bar'))
.then(() => {
kuzzle.emit('foo:bar');

should(bar).be.calledOnce();
should(foo).not.be.called();
});
Expand All @@ -101,10 +98,8 @@ describe('PluginsManager.run', () => {
pluginMock.expects('baz').never();

return pluginsManager.run()
.then(() => {
kuzzle.emit('foo:bar');
pluginMock.verify();
});
.then(() => pluginsManager.trigger('foo:bar'))
.then(() => pluginMock.verify());
});

it('should attach multi-target hook with function', () => {
Expand All @@ -119,9 +114,8 @@ describe('PluginsManager.run', () => {
};

return pluginsManager.run()
.then(() => pluginsManager.trigger('foo:bar'))
.then(() => {
kuzzle.emit('foo:bar');

should(bar).be.calledOnce();
should(foo).be.calledOnce();
should(baz).not.be.called();
Expand All @@ -142,10 +136,8 @@ describe('PluginsManager.run', () => {
pluginMock.expects('bar').never();

return pluginsManager.run()
.then(() => {
kuzzle.emit('foo:bar');
pluginMock.verify();
});
.then(() => pluginsManager.trigger('foo:bar'))
.then(() => pluginMock.verify());
});

it('should throw if a hook target is not a function and not a method name', () => {
Expand Down
117 changes: 116 additions & 1 deletion test/api/core/pluginsManager/trigger.test.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
const
mockrequire = require('mock-require'),
should = require('should'),
sinon = require('sinon'),
ElasticsearchClientMock = require('../../../mocks/services/elasticsearchClient.mock'),
KuzzleMock = require('../../../mocks/kuzzle.mock'),
{
errors: {
PluginImplementationError
}
},
Request: KuzzleRequest
} = require('kuzzle-common-objects');

describe('Test plugins manager trigger', () => {
Expand Down Expand Up @@ -44,6 +46,119 @@ describe('Test plugins manager trigger', () => {
});
});

it('should trigger hooks with before wildcard event', done => {
pluginsManager.plugins = [{
object: {
init: () => {},
hooks: {
'foo:before*': 'myFunc'
},
myFunc: done
},
config: {},
activated: true,
manifest: {
name: 'foo'
}
}];

pluginsManager.run()
.then(() => {
pluginsManager.trigger('foo:beforeBar');
});
});

it('should trigger hooks with after wildcard event', done => {
pluginsManager.plugins = [{
object: {
init: () => {},
hooks: {
'foo:after*': 'myFunc'
},
myFunc: done
},
config: {},
activated: true,
manifest: {
name: 'foo'
}
}];

pluginsManager.run()
.then(() => {
pluginsManager.trigger('foo:afterBar');
});
});

it('should trigger pipes with wildcard event', () => {
pluginsManager.plugins = [{
object: {
init: () => {},
pipes: {
'foo:*': 'myFunc'
},
myFunc: sinon.stub().callsArgWith(1, null, new KuzzleRequest({}))
},
config: {},
activated: true,
manifest: {
name: 'foo'
}
}];

pluginsManager.run()
Copy link
Contributor

Choose a reason for hiding this comment

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

that promise must be returned, otherwise the test isn't awaited and it always passes

.then(() => pluginsManager.trigger('foo:bar'))
.then(() => {
should(pluginsManager.plugins[0].object.myFunc).be.calledOnce();
});
});

it('should trigger pipes with before wildcard event', () => {
pluginsManager.plugins = [{
object: {
init: () => {},
pipes: {
'foo:before*': 'myFunc'
},
myFunc: sinon.stub().callsArgWith(1, null, new KuzzleRequest({}))
},
config: {},
activated: true,
manifest: {
name: 'foo'
}
}];

pluginsManager.run()
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

.then(() => pluginsManager.trigger('foo:beforeBar'))
.then(() => {
should(pluginsManager.plugins[0].object.myFunc).be.calledOnce();
});
});

it('should trigger pipes with after wildcard event', () => {
pluginsManager.plugins = [{
object: {
init: () => {},
pipes: {
'foo:after*': 'myFunc'
},
myFunc: sinon.stub().callsArgWith(1, null, new KuzzleRequest({}))
},
config: {},
activated: true,
manifest: {
name: 'foo'
}
}];

pluginsManager.run()
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

.then(() => pluginsManager.trigger('foo:afterBar'))
.then(() => {
should(pluginsManager.plugins[0].object.myFunc).be.calledOnce();
});
});

it('should reject a pipe returned value if it is not a Request instance', () => {
const pluginMock = {
object: {
Expand Down
6 changes: 3 additions & 3 deletions test/mocks/services/redisClient.mock.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const
EventEmitter = require('eventemitter2').EventEmitter2,
EventEmitter = require('eventemitter3'),
IORedis = require('ioredis'),
getBuiltinCommands = (new IORedis({lazyConnect: true})).getBuiltinCommands,
Bluebird = require('bluebird');
Expand All @@ -10,7 +10,7 @@ const
*/
class RedisClientMock extends EventEmitter {
constructor (err) {
super({verboseMemoryLeak: true});
super();

this.getBuiltinCommands = getBuiltinCommands;

Expand Down Expand Up @@ -46,7 +46,7 @@ class RedisClientMock extends EventEmitter {
this.emit('end', true);
}, 50);
};
Stream.prototype = new EventEmitter({verboseMemoryLeak: true});
Stream.prototype = new EventEmitter();

return new Stream();
}
Expand Down