From ea603d033eb5dfe02fc25256484b2cb2b3a82bfb Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Thu, 2 Jun 2016 14:15:47 -0700 Subject: [PATCH] [uiExports] add uiExport type "injectDefaultVars" In order for plugins to inject default injected vars, they should define an injectVars uiExport that will be called once in order to build the default injected vars. This function should return an object with the injectedVars it wishes to add. Plugins are run in a non-deterministic order, so they should not be written to override each other. injectedVars should all have unique names. The injectedVars for the app will override any values provided by default injectedVars functions. [server/plugins/init] allow extendRegister() fns to be async [server/plugins/init] convert async promise to bluebird [uiExports] execute default injectedVars functions early [server/plugin] do not leak server.register() implemenation detail [test/utils/kbnServer] opt-in to plugin loading [uiExports] test injectDefaultVars() --- src/plugins/elasticsearch/index.js | 10 +++ .../elasticsearch/lib/__tests__/routes.js | 17 ++-- src/plugins/kibana/index.js | 8 +- .../dashboard/services/saved_dashboards.js | 1 + src/server/plugins/Plugin.js | 25 ++++-- src/ui/UiExports.js | 14 +++ .../fixtures/plugin_async_foo/index.js | 18 ++++ .../fixtures/plugin_async_foo/package.json | 4 + src/ui/__tests__/fixtures/plugin_bar/index.js | 14 +++ .../fixtures/plugin_bar/package.json | 4 + src/ui/__tests__/fixtures/plugin_foo/index.js | 14 +++ .../fixtures/plugin_foo/package.json | 4 + src/ui/__tests__/ui_exports.js | 89 ++++++++++++++++++- src/ui/index.js | 12 +-- test/utils/kbn_server.js | 7 +- 15 files changed, 205 insertions(+), 36 deletions(-) create mode 100644 src/ui/__tests__/fixtures/plugin_async_foo/index.js create mode 100644 src/ui/__tests__/fixtures/plugin_async_foo/package.json create mode 100644 src/ui/__tests__/fixtures/plugin_bar/index.js create mode 100644 src/ui/__tests__/fixtures/plugin_bar/package.json create mode 100644 src/ui/__tests__/fixtures/plugin_foo/index.js create mode 100644 src/ui/__tests__/fixtures/plugin_foo/package.json diff --git a/src/plugins/elasticsearch/index.js b/src/plugins/elasticsearch/index.js index ded418725a19e..e1f368d755550 100644 --- a/src/plugins/elasticsearch/index.js +++ b/src/plugins/elasticsearch/index.js @@ -33,6 +33,16 @@ module.exports = function ({ Plugin }) { }).default(); }, + uiExports: { + injectDefaultVars(server, options) { + return { + esRequestTimeout: options.requestTimeout, + esShardTimeout: options.shardTimeout, + esApiVersion: options.apiVersion, + }; + } + }, + init(server, options) { const kibanaIndex = server.config().get('kibana.index'); diff --git a/src/plugins/elasticsearch/lib/__tests__/routes.js b/src/plugins/elasticsearch/lib/__tests__/routes.js index 6b1ec512be12d..ff681438623e5 100644 --- a/src/plugins/elasticsearch/lib/__tests__/routes.js +++ b/src/plugins/elasticsearch/lib/__tests__/routes.js @@ -1,10 +1,7 @@ const expect = require('expect.js'); -const util = require('util'); -const requireFromTest = require('requirefrom')('test'); -const kbnTestServer = requireFromTest('utils/kbn_server'); - -const format = util.format; - +const { format } = require('util'); +const kbnTestServer = require('../../../../../test/utils/kbn_server'); +const fromRoot = require('../../../../utils/fromRoot'); describe('plugins/elasticsearch', function () { describe('routes', function () { @@ -14,7 +11,13 @@ describe('plugins/elasticsearch', function () { before(function () { this.timeout(15000); // sometimes waiting for server takes longer than 10 - kbnServer = kbnTestServer.createServer(); + kbnServer = kbnTestServer.createServer({ + plugins: { + scanDirs: [ + fromRoot('src/plugins') + ] + } + }); return kbnServer.ready() .then(() => kbnServer.server.plugins.elasticsearch.waitUntilReady()); }); diff --git a/src/plugins/kibana/index.js b/src/plugins/kibana/index.js index fa9504f9d339e..59c64655b95e5 100644 --- a/src/plugins/kibana/index.js +++ b/src/plugins/kibana/index.js @@ -41,7 +41,13 @@ module.exports = function (kibana) { kbnDefaultAppId: config.get('kibana.defaultAppId') }; } - } + }, + + injectDefaultVars(server, options) { + return { + kbnIndex: options.index + }; + }, } }); diff --git a/src/plugins/kibana/public/dashboard/services/saved_dashboards.js b/src/plugins/kibana/public/dashboard/services/saved_dashboards.js index 513a30b04e582..76c0f209e9a94 100644 --- a/src/plugins/kibana/public/dashboard/services/saved_dashboards.js +++ b/src/plugins/kibana/public/dashboard/services/saved_dashboards.js @@ -16,6 +16,7 @@ define(function (require) { // This is the only thing that gets injected into controllers module.service('savedDashboards', function (Promise, SavedDashboard, kbnIndex, es, kbnUrl) { + console.log('savedDashboards', kbnIndex); const scanner = new Scanner(es, { index: kbnIndex, type: 'dashboard' diff --git a/src/server/plugins/Plugin.js b/src/server/plugins/Plugin.js index 7703b21758900..0cc9292169c87 100644 --- a/src/server/plugins/Plugin.js +++ b/src/server/plugins/Plugin.js @@ -1,9 +1,11 @@ let _ = require('lodash'); let Joi = require('joi'); -let { attempt, fromNode } = require('bluebird'); +let Bluebird = require('bluebird'); let { resolve } = require('path'); let { inherits } = require('util'); +const extendInitFns = Symbol('extend plugin initialization'); + const defaultConfigSchema = Joi.object({ enabled: Joi.boolean().default(true) }).default(); @@ -23,6 +25,7 @@ module.exports = class Plugin { this.externalInit = opts.init || _.noop; this.getConfigSchema = opts.config || _.noop; this.init = _.once(this.init); + this[extendInitFns] = []; } static scoped(kbnServer, path, pkg) { @@ -51,14 +54,14 @@ module.exports = class Plugin { let { config } = kbnServer; // setup the hapi register function and get on with it - let register = (server, options, next) => { + const asyncRegister = async (server, options, next) => { this.server = server; // bind the server and options to all // apps created by this plugin - for (let app of this.apps) { - app.getInjectedVars = _.partial(app.getInjectedVars, server, options); - } + await Promise.all(this[extendInitFns].map(async fn => { + await fn.call(this, server, options); + })); server.log(['plugins', 'debug'], { tmpl: 'Initializing plugin <%= plugin.toString() %>', @@ -72,12 +75,16 @@ module.exports = class Plugin { this.status = kbnServer.status.create(this); server.expose('status', this.status); - attempt(this.externalInit, [server, options], this).nodeify(next); + return await Bluebird.attempt(this.externalInit, [server, options], this); + }; + + const register = (server, options, next) => { + Bluebird.resolve(asyncRegister(server, options)).nodeify(next); }; register.attributes = { name: id, version: version }; - await fromNode(cb => { + await Bluebird.fromNode(cb => { kbnServer.server.register({ register: register, options: config.has(id) ? config.get(id) : null @@ -91,6 +98,10 @@ module.exports = class Plugin { } } + extendInit(fn) { + this[extendInitFns].push(fn); + } + toJSON() { return this.pkg; } diff --git a/src/ui/UiExports.js b/src/ui/UiExports.js index cd0fd4f73b8c2..296f7537510e9 100644 --- a/src/ui/UiExports.js +++ b/src/ui/UiExports.js @@ -11,6 +11,7 @@ class UiExports { this.exportConsumer = _.memoize(this.exportConsumer); this.consumers = []; this.bundleProviders = []; + this.defaultInjectedVars = []; } consumePlugin(plugin) { @@ -53,6 +54,12 @@ class UiExports { id: plugin.id, urlBasePath: this.urlBasePath })); + + plugin.extendInit((server, options) => { // eslint-disable-line no-loop-func + const wrapped = app.getInjectedVars; + app.getInjectedVars = () => wrapped.call(plugin, server, options); + }); + plugin.apps.add(app); } }; @@ -76,6 +83,13 @@ class UiExports { this.aliases[adhocType] = _.union(this.aliases[adhocType] || [], spec); }); }; + + case 'injectDefaultVars': + return (plugin, injector) => { + plugin.extendInit(async (server, options) => { + _.merge(this.defaultInjectedVars, await injector.call(plugin, server, options)); + }); + }; } } diff --git a/src/ui/__tests__/fixtures/plugin_async_foo/index.js b/src/ui/__tests__/fixtures/plugin_async_foo/index.js new file mode 100644 index 0000000000000..a0cd8887bbed0 --- /dev/null +++ b/src/ui/__tests__/fixtures/plugin_async_foo/index.js @@ -0,0 +1,18 @@ +import Bluebird from 'bluebird'; + +export default kibana => new kibana.Plugin({ + config(Joi) { + return Joi.object().keys({ + enabled: Joi.boolean().default(true), + delay: Joi.number().required(), + shared: Joi.string(), + }).default(); + }, + + uiExports: { + async injectDefaultVars(server, options) { + await Bluebird.delay(options.delay); + return { shared: options.shared }; + } + } +}); diff --git a/src/ui/__tests__/fixtures/plugin_async_foo/package.json b/src/ui/__tests__/fixtures/plugin_async_foo/package.json new file mode 100644 index 0000000000000..4ad7dda995ca5 --- /dev/null +++ b/src/ui/__tests__/fixtures/plugin_async_foo/package.json @@ -0,0 +1,4 @@ +{ + "name": "plugin_async_foo", + "version": "0.0.0" +} diff --git a/src/ui/__tests__/fixtures/plugin_bar/index.js b/src/ui/__tests__/fixtures/plugin_bar/index.js new file mode 100644 index 0000000000000..59c5556444496 --- /dev/null +++ b/src/ui/__tests__/fixtures/plugin_bar/index.js @@ -0,0 +1,14 @@ +export default kibana => new kibana.Plugin({ + config(Joi) { + return Joi.object().keys({ + enabled: Joi.boolean().default(true), + shared: Joi.string() + }).default(); + }, + + uiExports: { + injectDefaultVars(server, options) { + return { shared: options.shared }; + } + } +}); diff --git a/src/ui/__tests__/fixtures/plugin_bar/package.json b/src/ui/__tests__/fixtures/plugin_bar/package.json new file mode 100644 index 0000000000000..bfc5da0182266 --- /dev/null +++ b/src/ui/__tests__/fixtures/plugin_bar/package.json @@ -0,0 +1,4 @@ +{ + "name": "plugin_bar", + "version": "0.0.0" +} diff --git a/src/ui/__tests__/fixtures/plugin_foo/index.js b/src/ui/__tests__/fixtures/plugin_foo/index.js new file mode 100644 index 0000000000000..59c5556444496 --- /dev/null +++ b/src/ui/__tests__/fixtures/plugin_foo/index.js @@ -0,0 +1,14 @@ +export default kibana => new kibana.Plugin({ + config(Joi) { + return Joi.object().keys({ + enabled: Joi.boolean().default(true), + shared: Joi.string() + }).default(); + }, + + uiExports: { + injectDefaultVars(server, options) { + return { shared: options.shared }; + } + } +}); diff --git a/src/ui/__tests__/fixtures/plugin_foo/package.json b/src/ui/__tests__/fixtures/plugin_foo/package.json new file mode 100644 index 0000000000000..6a73b89110ae9 --- /dev/null +++ b/src/ui/__tests__/fixtures/plugin_foo/package.json @@ -0,0 +1,4 @@ +{ + "name": "plugin_foo", + "version": "0.0.0" +} diff --git a/src/ui/__tests__/ui_exports.js b/src/ui/__tests__/ui_exports.js index 6ef4e6cabf158..4250c140db0c8 100644 --- a/src/ui/__tests__/ui_exports.js +++ b/src/ui/__tests__/ui_exports.js @@ -1,11 +1,13 @@ import expect from 'expect.js'; +import { resolve } from 'path'; -import UiExports from '../UiExports'; +import UiExports from '../ui_exports'; +import * as kbnTestServer from '../../../test/utils/kbn_server'; describe('UiExports', function () { describe('#find()', function () { it('finds exports based on the passed export names', function () { - let uiExports = new UiExports({}); + var uiExports = new UiExports({}); uiExports.aliases.foo = ['a', 'b', 'c']; uiExports.aliases.bar = ['d', 'e', 'f']; @@ -15,7 +17,7 @@ describe('UiExports', function () { }); it('allows query types that match nothing', function () { - let uiExports = new UiExports({}); + var uiExports = new UiExports({}); uiExports.aliases.foo = ['a', 'b', 'c']; expect(uiExports.find(['foo'])).to.eql(['a', 'b', 'c']); @@ -23,4 +25,83 @@ describe('UiExports', function () { expect(uiExports.find(['foo', 'bar'])).to.eql(['a', 'b', 'c']); }); }); -}); +// + describe('#defaultInjectedVars', function () { + context('two plugins, two sync', function () { + this.slow(10000); + this.timeout(60000); + + let kbnServer; + before(async function () { + kbnServer = kbnTestServer.createServer({ + plugins: { + paths: [ + resolve(__dirname, 'fixtures/plugin_bar'), + resolve(__dirname, 'fixtures/plugin_foo') + ] + }, + + plugin_foo: { + shared: 'foo' + }, + + plugin_bar: { + shared: 'bar' + } + }); + + await kbnServer.ready(); + }); + + after(async function () { + await kbnServer.close(); + }); + + it('merges the two plugins in the order they are loaded', function () { + expect(kbnServer.uiExports.defaultInjectedVars).to.eql({ + shared: 'foo' + }); + }); + }); + + context('two plugins, one async', function () { + this.slow(10000); + this.timeout(60000); + + let kbnServer; + before(async function () { + kbnServer = kbnTestServer.createServer({ + plugins: { + scanDirs: [], + paths: [ + resolve(__dirname, 'fixtures/plugin_async_foo'), + resolve(__dirname, 'fixtures/plugin_foo') + ] + }, + + plugin_async_foo: { + delay: 500, + shared: 'foo' + }, + + plugin_bar: { + shared: 'bar' + } + }); + + await kbnServer.ready(); + }); + + after(async function () { + await kbnServer.close(); + }); + + it('merges the two plugins in the order they are loaded', function () { + // even though plugin_async_foo loads 500ms later, it is still "first" to merge + expect(kbnServer.uiExports.defaultInjectedVars).to.eql({ + shared: 'foo' + }); + }); + }); + }); +}); \ No newline at end of file diff --git a/src/ui/index.js b/src/ui/index.js index ebefd439077ab..3a3ff6e60e7db 100644 --- a/src/ui/index.js +++ b/src/ui/index.js @@ -59,16 +59,6 @@ module.exports = async (kbnServer, server, config) => { } }); - const defaultInjectedVars = {}; - if (config.has('kibana')) { - defaultInjectedVars.kbnIndex = config.get('kibana.index'); - } - if (config.has('elasticsearch')) { - defaultInjectedVars.esRequestTimeout = config.get('elasticsearch.requestTimeout'); - defaultInjectedVars.esShardTimeout = config.get('elasticsearch.shardTimeout'); - defaultInjectedVars.esApiVersion = config.get('elasticsearch.apiVersion'); - } - server.decorate('reply', 'renderApp', function (app) { const payload = { app: app, @@ -77,7 +67,7 @@ module.exports = async (kbnServer, server, config) => { buildNum: config.get('pkg.buildNum'), buildSha: config.get('pkg.buildSha'), basePath: config.get('server.basePath'), - vars: defaults(app.getInjectedVars(), defaultInjectedVars), + vars: defaults(app.getInjectedVars() || {}, uiExports.defaultInjectedVars), }; return this.view(app.templateName, { diff --git a/test/utils/kbn_server.js b/test/utils/kbn_server.js index 35a0bc3aa1c59..4c362c5035b08 100644 --- a/test/utils/kbn_server.js +++ b/test/utils/kbn_server.js @@ -5,7 +5,6 @@ import { kibanaUser, kibanaServer } from '../shield'; const src = requirefrom('src'); const KbnServer = src('server/KbnServer'); -const fromRoot = src('utils/fromRoot'); const SERVER_DEFAULTS = { server: { @@ -17,11 +16,7 @@ const SERVER_DEFAULTS = { logging: { quiet: true }, - plugins: { - scanDirs: [ - fromRoot('src/plugins') - ] - }, + plugins: {}, optimize: { enabled: false },