From 38ead1cd8f3ffb66dd0ec71b7cbb2d880ce54396 Mon Sep 17 00:00:00 2001 From: Marcelo Boveto Shima Date: Wed, 29 Jan 2020 12:31:56 -0300 Subject: [PATCH] Improvements to custom queues (#1158) - Create queueTask, createTaskGroup methods. - Add options to custom priorities. Allows to create custom priorities that executes an task only once (based on the method name). Useful for inheritance and composability. --- lib/index.js | 188 +++++++++++++++++++++++++++++++++++++-------------- test/base.js | 185 +++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 308 insertions(+), 65 deletions(-) diff --git a/lib/index.js b/lib/index.js index df2eaea3..2791a9f5 100644 --- a/lib/index.js +++ b/lib/index.js @@ -28,6 +28,37 @@ const methodIsValid = function(name) { return name.charAt(0) !== '_' && name !== 'constructor'; }; +/** + * Queue options. + * @typedef {Object} QueueOptions + * @property {string} [queueName] - Name of the queue. + * @property {boolean} [once] - Execute only once by namespace and taskName. + * @property {boolean} [run] - Run the queue if not running yet. + */ + +/** + * Task options. + * @typedef {Object} TaskOptions + * @extends QueueOptions + * @property {Function} [reject] - Reject callback. + */ + +/** + * Priority object. + * @typedef {Object} Priority + * @extends QueueOptions + * @property {string} priorityName - Name of the priority. + * @property {string} [before] - The queue which this priority should be added before. + */ + +/** + * Complete Task object. + * @typedef {Object} Task + * @extends TaskOptions + * @property {Function} method - Function to be queued. + * @property {string} taskName - Name of the task. + */ + class Generator extends EventEmitter { // If for some reason environment adds more queues, we should use or own for stability. static get queues() { @@ -190,24 +221,29 @@ class Generator extends EventEmitter { // Add original queues. Generator.queues.forEach(queue => { - this._queues[queue] = queue; + this._queues[queue] = { priorityName: queue, queueName: queue }; }); // Add custom queues if (Array.isArray(this.options.customPriorities)) { - const customPriorities = this.options.customPriorities; + const customPriorities = this.options.customPriorities.map(customPriority => { + // Keep backward compatibility with name + const newPriority = { priorityName: customPriority.name, ...customPriority }; + delete newPriority.name; + return newPriority; + }); // Sort customPriorities, a referenced custom queue must be added before the one that reference it. customPriorities.sort((a, b) => { - if (a.name === b.name) { + if (a.priorityName === b.priorityName) { throw new Error(`Duplicate custom queue ${a.name}`); } - if (a.name === b.before) { + if (a.priorityName === b.before) { return -1; } - if (b.name === a.before) { + if (b.priorityName === a.before) { return 1; } @@ -216,11 +252,13 @@ class Generator extends EventEmitter { // Add queue to runLoop customPriorities.forEach(customQueue => { - const queueName = `${this.options.namespace}#${customQueue.name}`; - debug(`Registering custom queue ${queueName}`); - this._queues[customQueue.name] = queueName; + customQueue.queueName = + customQueue.queueName || + `${this.options.namespace}#${customQueue.priorityName}`; + debug(`Registering custom queue ${customQueue.queueName}`); + this._queues[customQueue.priorityName] = customQueue; - if (this.env.runLoop.queueNames.includes(queueName)) { + if (this.env.runLoop.queueNames.includes(customQueue.queueName)) { return; } @@ -260,7 +298,10 @@ class Generator extends EventEmitter { }; } - this.env.runLoop.addSubQueue(queueName, this._queues[customQueue.before]); + let beforeQueue = customQueue.before + ? this._queues[customQueue.before].queueName + : undefined; + this.env.runLoop.addSubQueue(customQueue.queueName, beforeQueue); }); } } @@ -501,9 +542,9 @@ class Generator extends EventEmitter { * Schedule methods on a run queue. * * @param {Function|Object} method: Method to be scheduled or object with function properties. - * @param {String} [methodName]: Name of the method to be scheduled. + * @param {String} [methodName]: Name of the method (task) to be scheduled. * @param {String} [queueName]: Name of the queue to be scheduled on. - * @param {String} [reject]: Reject callback. + * @param {Function} [reject]: Reject callback. */ queueMethod(method, methodName, queueName, reject = () => {}) { if (typeof queueName === 'function') { @@ -513,54 +554,95 @@ class Generator extends EventEmitter { queueName = queueName || 'default'; } - const self = this; if (!_.isFunction(method)) { if (typeof methodName === 'function') { reject = methodName; methodName = undefined; } - queueName = methodName || queueName; - // Run each queue items - _.each(method, (newMethod, newMethodName) => { - if (!_.isFunction(newMethod) || !methodIsValid(newMethodName)) return; - - self.queueMethod(newMethod, newMethodName, queueName, reject); + this.queueTaskGroup(method, { + queueName: methodName, + reject }); return; } + this.queueTask({ + method, + taskName: methodName, + queueName, + reject + }); + } + + /** + * Schedule methods on a run queue. + * + * @param {Object} taskGroup: Name of the method (task) to be scheduled or taskOptions. + * @param {TaskOptions} [taskOptions]: Name of the method (task) to be scheduled or taskOptions. + */ + queueTaskGroup(taskGroup, taskOptions) { + const self = this; + // Run each queue items + _.each(taskGroup, (newMethod, newMethodName) => { + if (!_.isFunction(newMethod) || !methodIsValid(newMethodName)) return; + + self.queueTask({ + ...taskOptions, + method: newMethod, + taskName: newMethodName + }); + }); + } + + /** + * Schedule tasks on a run queue. + * + * @param {Task} task: Task to be queued. + */ + queueTask(task) { + const reject = task.reject || (() => {}); + const queueName = task.queueName || 'default'; + const methodName = task.taskName; + const method = task.method; + const once = task.once ? methodName : undefined; + + const self = this; let namespace = ''; if (self.options && self.options.namespace) { namespace = self.options.namespace; } - debug(`Queueing ${namespace}#${methodName} in ${queueName}`); - self.env.runLoop.add(queueName, completed => { - debug(`Running ${namespace}#${methodName}`); - self.emit(`method:${methodName}`); - - runAsync(function() { - self.async = () => this.async(); - self.runningState = { namespace, queueName, methodName }; - return method.apply(self, self.args); - })() - .then(function() { - delete self.runningState; - completed(); - }) - .catch(err => { - debug(`An error occured while running ${namespace}#${methodName}`, err); - delete self.runningState; - - // Ensure we emit the error event outside the promise context so it won't be - // swallowed when there's no listeners. - setImmediate(() => { - self.emit('error', err); - reject(err); + debug(`Queueing ${namespace}#${methodName} with options %o`, task); + self.env.runLoop.add( + queueName, + completed => { + debug(`Running ${namespace}#${methodName}`); + self.emit(`method:${methodName}`); + + runAsync(function() { + self.async = () => this.async(); + self.runningState = { namespace, queueName, methodName }; + return method.apply(self, self.args); + })() + .then(function() { + delete self.runningState; + completed(); + }) + .catch(err => { + debug(`An error occured while running ${namespace}#${methodName}`, err); + delete self.runningState; + + // Ensure we emit the error event outside the promise context so it won't be + // swallowed when there's no listeners. + setImmediate(() => { + self.emit('error', err); + reject(err); + }); }); - }); - }); + }, + { once, run: task.run } + ); } /** @@ -606,19 +688,22 @@ class Generator extends EventEmitter { ); const item = property.value ? property.value : property.get.call(self); - const queueName = self._queues[name]; + const priority = self._queues[name]; + let taskOptions = { ...priority, reject }; // Name points to a function; run it! if (typeof item === 'function') { - return self.queueMethod(item, name, queueName, reject); + taskOptions.taskName = name; + taskOptions.method = item; + return self.queueTask(taskOptions); } // Not a queue hash; stop - if (!queueName) { + if (!priority) { return; } - self.queueMethod(item, queueName, reject); + self.queueTaskGroup(item, taskOptions); } validMethods.forEach(addInQueue); @@ -684,7 +769,12 @@ class Generator extends EventEmitter { } const instantiate = (Generator, path) => { - Generator.resolved = require.resolve(path); + if (path === 'unknown') { + Generator.resolved = path; + } else { + Generator.resolved = require.resolve(path); + } + Generator.namespace = this.env.namespace(path); return this.env.instantiate(Generator, { diff --git a/test/base.js b/test/base.js index 2379dc34..ec5e20ac 100644 --- a/test/base.js +++ b/test/base.js @@ -1244,6 +1244,97 @@ describe('Base', () => { done(); }); }); + + it('queued method with function, methodName and reject', function() { + const env = yeoman.createEnv([], { 'skip-install': true }, new TestAdapter()); + const gen = new this.Generator({ + resolved: resolveddir, + namespace: 'dummy', + env, + testQueue: 'This value' + }); + + sinon.spy(env.runLoop, 'add'); + const noop = () => {}; + gen.queueMethod(noop, 'configuring', noop); + + assert(env.runLoop.add.calledOnce); + assert.equal('default', env.runLoop.add.getCall(0).args[0]); + }); + + it('queued method with object, queueName and reject', function() { + const env = yeoman.createEnv([], { 'skip-install': true }, new TestAdapter()); + const gen = new this.Generator({ + resolved: resolveddir, + namespace: 'dummy', + env, + testQueue: 'This value' + }); + + sinon.spy(env.runLoop, 'add'); + const noop = () => {}; + const queueName = 'configuring'; + const tasks = { + foo() {} + }; + gen.queueMethod(tasks, queueName, noop); + + assert(env.runLoop.add.calledOnce); + assert.equal(queueName, env.runLoop.add.getCall(0).args[0]); + }); + }); + + describe('#queueTask()', () => { + beforeEach(function() { + this.Generator = class extends Base { + constructor(args, opts) { + super(args, opts); + + this.queueMethod( + { + testQueue: function() { + this.queue = this.options.testQueue; + } + }, + () => {} + ); + } + + exec() {} + }; + }); + + it('queued method with function and options', function() { + const env = yeoman.createEnv([], { 'skip-install': true }, new TestAdapter()); + const gen = new this.Generator({ + resolved: resolveddir, + namespace: 'dummy', + env, + testQueue: 'This value' + }); + + sinon.spy(env.runLoop, 'add'); + const method = () => {}; + const taskName = 'foo'; + const queueName = 'configuring'; + gen.queueTask({ + method, + queueName, + taskName, + once: true, + run: false + }); + + assert(env.runLoop.add.calledOnce); + assert.equal(queueName, env.runLoop.add.getCall(0).args[0]); + assert.deepStrictEqual( + { + once: taskName, + run: false + }, + env.runLoop.add.getCall(0).args[2] + ); + }); }); describe('Custom priorities', () => { @@ -1253,6 +1344,7 @@ describe('Base', () => { super(args, { ...options, customPriorities: [ + ...(options.customPriorities || []), { // Change priority prompting to be queue before writing for this generator. // If we change defaults priorities in the future, the order of custom priorities will keep the same. @@ -1265,14 +1357,16 @@ describe('Base', () => { }, { name: 'preConfiguring1', - before: 'preConfiguring2' + before: 'preConfiguring2', + queueName: 'common#preConfiguring1', + once: true }, { - name: 'preConfiguring2', + priorityName: 'preConfiguring2', before: 'configuring' }, { - name: 'afterEnd' + priorityName: 'afterEnd' } ] }); @@ -1284,23 +1378,40 @@ describe('Base', () => { _.extend(this.TestGenerator.prototype, { assert: function() { assert.deepStrictEqual(this._queues, { - initializing: 'initializing', - preConfiguring1: 'dummy#preConfiguring1', - preConfiguring2: 'dummy#preConfiguring2', - configuring: 'configuring', - default: 'default', - prePrompting1: 'dummy#prePrompting1', - prompting: 'dummy#prompting', - writing: 'writing', - conflicts: 'conflicts', - install: 'install', - end: 'end', - afterEnd: 'dummy#afterEnd' + initializing: { priorityName: 'initializing', queueName: 'initializing' }, + preConfiguring1: { + priorityName: 'preConfiguring1', + queueName: 'common#preConfiguring1', + before: 'preConfiguring2', + once: true + }, + preConfiguring2: { + priorityName: 'preConfiguring2', + queueName: 'dummy#preConfiguring2', + before: 'configuring' + }, + configuring: { priorityName: 'configuring', queueName: 'configuring' }, + default: { priorityName: 'default', queueName: 'default' }, + prePrompting1: { + priorityName: 'prePrompting1', + queueName: 'dummy#prePrompting1', + before: 'prompting' + }, + prompting: { + priorityName: 'prompting', + queueName: 'dummy#prompting', + before: 'writing' + }, + writing: { priorityName: 'writing', queueName: 'writing' }, + conflicts: { priorityName: 'conflicts', queueName: 'conflicts' }, + install: { priorityName: 'install', queueName: 'install' }, + end: { priorityName: 'end', queueName: 'end' }, + afterEnd: { priorityName: 'afterEnd', queueName: 'dummy#afterEnd' } }); assert.deepStrictEqual(this.env.runLoop.queueNames, [ 'initializing', 'prompting', - 'dummy#preConfiguring1', + 'common#preConfiguring1', 'dummy#preConfiguring2', 'configuring', 'default', @@ -1364,6 +1475,48 @@ describe('Base', () => { assert(end.calledBefore(afterEnd)); }); }); + + it('correctly run custom priority with once option', function() { + const commonPreConfiguring = sinon.spy(); + const customPreConfiguring1 = sinon.spy(); + const customPreConfiguring2 = sinon.spy(); + + _.extend(this.TestGenerator.prototype, { + get preConfiguring1() { + return { commonPreConfiguring }; + } + }); + + this.TestGenerator2 = class extends this.TestGenerator { + get preConfiguring1() { + return { ...super.preConfiguring1, customPreConfiguring1 }; + } + }; + + this.TestGenerator3 = class extends this.TestGenerator { + get preConfiguring1() { + return { ...super.preConfiguring1, customPreConfiguring2 }; + } + }; + + this.testGen = new this.TestGenerator2([], { + resolved: 'unknown', + namespace: 'dummy', + env: this.env, + 'skip-install': true + }); + + this.testGen.composeWith({ + Generator: this.TestGenerator3, + path: 'unknown' + }); + + return this.testGen.run().then(() => { + sinon.assert.calledOnce(commonPreConfiguring); + sinon.assert.calledOnce(customPreConfiguring1); + sinon.assert.calledOnce(customPreConfiguring2); + }); + }); }); describe('Custom priorities errors', () => {