Skip to content

Commit d47154f

Browse files
ricardo-devis-agulloricardo-devis-agullo
authored
Merge pull request #1269 from opencomponents/plugins-initialiser-promise
[CB-INTERNAL] move plugins initialiser to promises
2 parents 7cb407f + 1c15773 commit d47154f

File tree

5 files changed

+93
-91
lines changed

5 files changed

+93
-91
lines changed

‎src/registry/domain/plugins-initialiser.ts

Lines changed: 45 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
1-
import async from 'async';
1+
import pLimit from 'p-limit';
22
import _ from 'lodash';
33
import { DepGraph } from 'dependency-graph';
4+
import { promisify } from 'util';
45

56
import strings from '../../resources';
67
import { Plugin } from '../../types';
78

89
function validatePlugins(plugins: unknown[]): asserts plugins is Plugin[] {
9-
let c = 0;
10-
11-
plugins.forEach(plugin => {
12-
c++;
10+
for (let idx = 0; idx < plugins.length; idx++) {
11+
const plugin = plugins[idx];
1312
if (
1413
!_.isObject((plugin as Plugin).register) ||
1514
typeof (plugin as Plugin).register.register !== 'function' ||
@@ -18,11 +17,11 @@ function validatePlugins(plugins: unknown[]): asserts plugins is Plugin[] {
1817
) {
1918
throw new Error(
2019
strings.errors.registry.PLUGIN_NOT_VALID(
21-
(plugin as Plugin).name || String(c)
20+
(plugin as Plugin).name || String(idx + 1)
2221
)
2322
);
2423
}
25-
});
24+
}
2625
}
2726

2827
function checkDependencies(plugins: Plugin[]) {
@@ -48,23 +47,16 @@ function checkDependencies(plugins: Plugin[]) {
4847
}
4948

5049
let deferredLoads: Plugin[] = [];
51-
const defer = function (plugin: Plugin, cb: (err?: Error) => void) {
52-
deferredLoads.push(plugin);
53-
return cb();
54-
};
55-
56-
export function init(
57-
pluginsToRegister: unknown[],
58-
callback: Callback<Dictionary<(...args: unknown[]) => void>, unknown>
59-
): void {
60-
const registered: Dictionary<(...args: unknown[]) => void> = {};
61-
62-
try {
63-
validatePlugins(pluginsToRegister);
64-
checkDependencies(pluginsToRegister);
65-
} catch (err) {
66-
return callback(err, undefined as any);
67-
}
50+
51+
type PluginFunctions = Dictionary<(...args: unknown[]) => void>;
52+
53+
export async function init(
54+
pluginsToRegister: unknown[]
55+
): Promise<PluginFunctions> {
56+
const registered: PluginFunctions = {};
57+
58+
validatePlugins(pluginsToRegister);
59+
checkDependencies(pluginsToRegister);
6860

6961
const dependenciesRegistered = (dependencies: string[]) => {
7062
if (dependencies.length === 0) {
@@ -81,48 +73,55 @@ export function init(
8173
return present;
8274
};
8375

84-
const loadPlugin = (plugin: Plugin, cb: (err?: Error) => void) => {
85-
const done = _.once(cb);
86-
76+
const loadPlugin = async (plugin: Plugin): Promise<void> => {
8777
if (registered[plugin.name]) {
88-
return done();
78+
return;
8979
}
9080

9181
if (!plugin.register.dependencies) {
9282
plugin.register.dependencies = [];
9383
}
9484

9585
if (!dependenciesRegistered(plugin.register.dependencies)) {
96-
return defer(plugin, done);
86+
deferredLoads.push(plugin);
87+
return;
9788
}
9889

9990
const dependencies = _.pick(registered, plugin.register.dependencies);
10091

101-
plugin.register.register(
102-
plugin.options || {},
103-
dependencies,
104-
(err?: Error) => {
105-
const pluginCallback = plugin.callback || _.noop;
106-
pluginCallback(err);
107-
// Overriding toString so implementation details of plugins do not
108-
// leak to OC consumers
109-
plugin.register.execute.toString = () => plugin.description || '';
110-
registered[plugin.name] = plugin.register.execute;
111-
done(err);
112-
}
113-
);
92+
const register = promisify(plugin.register.register);
93+
94+
const pluginCallback = plugin.callback || _.noop;
95+
await register(plugin.options || {}, dependencies).catch(err => {
96+
pluginCallback(err);
97+
throw err;
98+
});
99+
// Overriding toString so implementation details of plugins do not
100+
// leak to OC consumers
101+
plugin.register.execute.toString = () => plugin.description || '';
102+
registered[plugin.name] = plugin.register.execute;
103+
pluginCallback();
114104
};
115105

116-
const terminator = function (err: Error) {
106+
const terminator = async (): Promise<PluginFunctions> => {
117107
if (deferredLoads.length > 0) {
118108
const deferredPlugins = _.clone(deferredLoads);
119109
deferredLoads = [];
120110

121-
return async.mapSeries(deferredPlugins, loadPlugin, terminator as any);
111+
await Promise.all(
112+
deferredPlugins.map(plugin => onSeries(() => loadPlugin(plugin)))
113+
);
114+
return terminator();
122115
}
123116

124-
callback(err, registered);
117+
return registered;
125118
};
126119

127-
async.mapSeries(pluginsToRegister, loadPlugin, terminator as any);
120+
const onSeries = pLimit(1);
121+
122+
await Promise.all(
123+
pluginsToRegister.map(plugin => onSeries(() => loadPlugin(plugin)))
124+
);
125+
126+
return terminator();
128127
}

‎src/registry/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export default function registry(inputOptions: Input) {
5959
async.waterfall(
6060
[
6161
(cb: Callback<Dictionary<(...args: unknown[]) => unknown>, unknown>) =>
62-
pluginsInitialiser.init(plugins, cb),
62+
fromPromise(pluginsInitialiser.init)(plugins, cb),
6363

6464
(
6565
plugins: Dictionary<(...args: unknown[]) => void>,

‎src/types.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,15 +198,15 @@ export interface Template {
198198
}
199199

200200
export interface Plugin {
201-
callback?: (...args: unknown[]) => void;
201+
callback?: (error: unknown) => void;
202202
description?: string;
203203
name: string;
204204
options?: any;
205205
register: {
206206
register: (
207207
options: unknown,
208208
dependencies: unknown,
209-
next: () => void
209+
next: (error?: unknown) => void
210210
) => void;
211211
execute: (...args: unknown[]) => unknown;
212212
dependencies?: string[];

‎test/unit/registry-domain-plugins-initialiser.js

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ describe('registry : domain : plugins-initialiser', () => {
1515
}
1616
];
1717

18-
pluginsInitialiser.init(plugins, err => {
19-
error = err;
20-
done();
21-
});
18+
pluginsInitialiser
19+
.init(plugins)
20+
.catch(err => (error = err))
21+
.finally(done);
2222
});
2323

2424
it('should error', () => {
@@ -40,10 +40,10 @@ describe('registry : domain : plugins-initialiser', () => {
4040
}
4141
];
4242

43-
pluginsInitialiser.init(plugins, err => {
44-
error = err;
45-
done();
46-
});
43+
pluginsInitialiser
44+
.init(plugins)
45+
.catch(err => (error = err))
46+
.finally(done);
4747
});
4848

4949
it('should error', () => {
@@ -61,10 +61,10 @@ describe('registry : domain : plugins-initialiser', () => {
6161
}
6262
];
6363

64-
pluginsInitialiser.init(plugins, err => {
65-
error = err;
66-
done();
67-
});
64+
pluginsInitialiser
65+
.init(plugins)
66+
.catch(err => (error = err))
67+
.finally(done);
6868
});
6969

7070
it('should error', () => {
@@ -84,10 +84,10 @@ describe('registry : domain : plugins-initialiser', () => {
8484
}
8585
];
8686

87-
pluginsInitialiser.init(plugins, err => {
88-
error = err;
89-
done();
90-
});
87+
pluginsInitialiser
88+
.init(plugins)
89+
.catch(err => (error = err))
90+
.finally(done);
9191
});
9292

9393
it('should error', () => {
@@ -130,10 +130,10 @@ describe('registry : domain : plugins-initialiser', () => {
130130
}
131131
];
132132

133-
pluginsInitialiser.init(plugins, (err, res) => {
134-
result = res;
135-
done();
136-
});
133+
pluginsInitialiser
134+
.init(plugins)
135+
.then(res => (result = res))
136+
.finally(done);
137137
});
138138

139139
it('should register plugin with passed options', () => {
@@ -190,9 +190,10 @@ describe('registry : domain : plugins-initialiser', () => {
190190
}
191191
];
192192

193-
pluginsInitialiser.init(plugins, () => {
194-
done();
195-
});
193+
pluginsInitialiser
194+
.init(plugins)
195+
.catch(() => {})
196+
.finally(done);
196197
});
197198

198199
it('should provide the getValue register method with the required dependent plugins', () => {
@@ -231,10 +232,10 @@ describe('registry : domain : plugins-initialiser', () => {
231232
}
232233
];
233234

234-
pluginsInitialiser.init(plugins, err => {
235-
error = err;
236-
done();
237-
});
235+
pluginsInitialiser
236+
.init(plugins)
237+
.catch(err => (error = err))
238+
.finally(done);
238239
});
239240

240241
it('should throw an error', () => {
@@ -261,10 +262,10 @@ describe('registry : domain : plugins-initialiser', () => {
261262
}
262263
];
263264

264-
pluginsInitialiser.init(plugins, err => {
265-
error = err;
266-
done();
267-
});
265+
pluginsInitialiser
266+
.init(plugins)
267+
.catch(err => (error = err))
268+
.finally(done);
268269
});
269270

270271
it('should throw an error', () => {
@@ -317,10 +318,10 @@ describe('registry : domain : plugins-initialiser', () => {
317318
}
318319
];
319320

320-
pluginsInitialiser.init(plugins, (err, res) => {
321-
result = res;
322-
done();
323-
});
321+
pluginsInitialiser
322+
.init(plugins)
323+
.then(res => (result = res))
324+
.finally(done);
324325
});
325326

326327
it('should defer the initalisation of the plugin until all dependencies have bee registered', () => {

‎test/unit/registry.js

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,23 +76,25 @@ describe('registry', () => {
7676
describe('when plugins initialiser fails', () => {
7777
let error;
7878
beforeEach(done => {
79-
deps['./domain/plugins-initialiser'].init.yields('error!');
79+
deps['./domain/plugins-initialiser'].init.rejects(
80+
new Error('error!')
81+
);
8082
registry.start(err => {
8183
error = err;
8284
done();
8385
});
8486
});
8587

8688
it('should fail with error', () => {
87-
expect(error).to.equal('error!');
89+
expect(error.message).to.equal('error!');
8890
});
8991
});
9092

9193
describe('when plugins initialiser succeeds', () => {
9294
describe('when repository initialisation fails', () => {
9395
let error;
9496
beforeEach(done => {
95-
deps['./domain/plugins-initialiser'].init.yields(null, 'ok');
97+
deps['./domain/plugins-initialiser'].init.resolves('ok');
9698
repositoryInitStub.rejects(new Error('nope'));
9799

98100
registry.start(err => {
@@ -110,7 +112,7 @@ describe('registry', () => {
110112
describe('when app fails to start', () => {
111113
let error;
112114
beforeEach(done => {
113-
deps['./domain/plugins-initialiser'].init.yields(null, 'ok');
115+
deps['./domain/plugins-initialiser'].init.resolves('ok');
114116
repositoryInitStub.resolves('ok');
115117
deps['./app-start'].rejects({ msg: 'I got a problem' });
116118

@@ -129,7 +131,7 @@ describe('registry', () => {
129131
describe('when http listener errors', () => {
130132
let error;
131133
beforeEach(done => {
132-
deps['./domain/plugins-initialiser'].init.yields(null, 'ok');
134+
deps['./domain/plugins-initialiser'].init.resolves('ok');
133135
repositoryInitStub.resolves('ok');
134136
deps['./app-start'].resolves('ok');
135137

@@ -153,7 +155,7 @@ describe('registry', () => {
153155
let error;
154156
let result;
155157
beforeEach(done => {
156-
deps['./domain/plugins-initialiser'].init.yields(null, 'ok');
158+
deps['./domain/plugins-initialiser'].init.resolves('ok');
157159
repositoryInitStub.resolves('ok');
158160
deps['./app-start'].resolves('ok');
159161
deps['./domain/events-handler'].fire = sinon.stub();
@@ -190,7 +192,7 @@ describe('registry', () => {
190192
describe('when http listener emits an error before the listener to start', () => {
191193
let error;
192194
beforeEach(done => {
193-
deps['./domain/plugins-initialiser'].init.yields(null, 'ok');
195+
deps['./domain/plugins-initialiser'].init.resolves('ok');
194196
repositoryInitStub.resolves('ok');
195197
deps['./app-start'].resolves('ok');
196198
deps['./domain/events-handler'].fire = sinon.stub();

0 commit comments

Comments
 (0)