From 10bca39fbd7f9116c7f4453e6b4fe9acc7f545b1 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Tue, 26 Jun 2018 09:39:03 -0400 Subject: [PATCH 1/2] fix merge from master (rxjs6 + xpack_main license change handling) --- .../watch_status_and_license_to_initialize.js | 60 +++++++++-------- ...h_status_and_license_to_initialize.test.js | 6 +- .../server/lib/__tests__/xpack_info.js | 66 ------------------- .../xpack_main/server/lib/xpack_info.js | 16 ----- 4 files changed, 34 insertions(+), 114 deletions(-) diff --git a/x-pack/plugins/security/server/lib/watch_status_and_license_to_initialize.js b/x-pack/plugins/security/server/lib/watch_status_and_license_to_initialize.js index 6e68a4e404705..14aa3b3ed365e 100644 --- a/x-pack/plugins/security/server/lib/watch_status_and_license_to_initialize.js +++ b/x-pack/plugins/security/server/lib/watch_status_and_license_to_initialize.js @@ -3,53 +3,55 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { Observable } from 'rxjs'; +import * as Rx from 'rxjs'; +import { map, switchMap, tap } from 'rxjs/operators'; export function watchStatusAndLicenseToInitialize(xpackMainPlugin, downstreamPlugin, initialize) { const xpackInfo = xpackMainPlugin.info; const xpackInfoFeature = xpackInfo.feature(downstreamPlugin.id); const upstreamStatus = xpackMainPlugin.status; - const currentStatus$ = Observable + const currentStatus$ = Rx .of({ state: upstreamStatus.state, message: upstreamStatus.message, }); - const newStatus$ = Observable + const newStatus$ = Rx .fromEvent(upstreamStatus, 'change', null, (previousState, previousMsg, state, message) => { return { state, message, }; }); - const status$ = Observable.merge(currentStatus$, newStatus$); + const status$ = Rx.merge(currentStatus$, newStatus$); - const currentLicense$ = Observable - .of(xpackInfoFeature.getLicenseCheckResults()); - const newLicense$ = Observable - .fromEventPattern(xpackInfoFeature.registerLicenseChangeCallback) - .map(() => xpackInfoFeature.getLicenseCheckResults()); - const license$ = Observable.merge(currentLicense$, newLicense$); + const currentLicense$ = Rx.of(xpackInfoFeature.getLicenseCheckResults()); + const newLicense$ = Rx + .fromEventPattern(xpackInfo.onLicenseInfoChange.bind(xpackInfo)) + .pipe(map(() => xpackInfoFeature.getLicenseCheckResults())); + const license$ = Rx.merge(currentLicense$, newLicense$); - Observable.combineLatest(status$, license$) - .map(([status, license]) => ({ status, license })) - .switchMap(({ status, license }) => { - if (status.state !== 'green') { - return Observable.of({ state: status.state, message: status.message }); - } + Rx.combineLatest(status$, license$) + .pipe( + map(([status, license]) => ({ status, license })), + switchMap(({ status, license }) => { + if (status.state !== 'green') { + return Rx.of({ state: status.state, message: status.message }); + } - return initialize(license) - .then(() => ({ - state: 'green', - message: 'Ready', - })) - .catch((err) => ({ - state: 'red', - message: err.message - })); - }) - .do(({ state, message }) => { - downstreamPlugin.status[state](message); - }) + return initialize(license) + .then(() => ({ + state: 'green', + message: 'Ready', + })) + .catch((err) => ({ + state: 'red', + message: err.message + })); + }), + tap(({ state, message }) => { + downstreamPlugin.status[state](message); + }) + ) .subscribe(); } diff --git a/x-pack/plugins/security/server/lib/watch_status_and_license_to_initialize.test.js b/x-pack/plugins/security/server/lib/watch_status_and_license_to_initialize.test.js index 55c2d83ab48f6..7fa66cd5b3d77 100644 --- a/x-pack/plugins/security/server/lib/watch_status_and_license_to_initialize.test.js +++ b/x-pack/plugins/security/server/lib/watch_status_and_license_to_initialize.test.js @@ -12,9 +12,6 @@ const createMockXpackMainPluginAndFeature = (featureId) => { const mockFeature = { getLicenseCheckResults: jest.fn(), - registerLicenseChangeCallback: (callback) => { - licenseChangeCallbacks.push(callback); - }, mock: { triggerLicenseChange: () => { for (const callback of licenseChangeCallbacks) { @@ -29,6 +26,9 @@ const createMockXpackMainPluginAndFeature = (featureId) => { const mockXpackMainPlugin = { info: { + onLicenseInfoChange: (callback) => { + licenseChangeCallbacks.push(callback); + }, feature: (id) => { if (id === featureId) { return mockFeature; diff --git a/x-pack/plugins/xpack_main/server/lib/__tests__/xpack_info.js b/x-pack/plugins/xpack_main/server/lib/__tests__/xpack_info.js index ded2fe0e5eab6..260b86d03f17a 100644 --- a/x-pack/plugins/xpack_main/server/lib/__tests__/xpack_info.js +++ b/x-pack/plugins/xpack_main/server/lib/__tests__/xpack_info.js @@ -462,72 +462,6 @@ describe('XPackInfo', () => { }); }); - it('registerLicenseChangeCallback() does not invoke callbacks if license has not changed', async () => { - const securityFeature = xPackInfo.feature('security'); - const watcherFeature = xPackInfo.feature('watcher'); - - const securityChangeCallback = sinon.stub(); - securityFeature.registerLicenseChangeCallback(securityChangeCallback); - - const watcherChangeCallback = sinon.stub(); - watcherFeature.registerLicenseChangeCallback(watcherChangeCallback); - - mockElasticsearchCluster.callWithInternalUser.returns( - getMockXPackInfoAPIResponse({ mode: 'gold' }) - ); - - await xPackInfo.refreshNow(); - - sinon.assert.notCalled(securityChangeCallback); - sinon.assert.notCalled(watcherChangeCallback); - }); - - it('registerLicenseChangeCallback() invokes callbacks on license change', async () => { - const securityFeature = xPackInfo.feature('security'); - const watcherFeature = xPackInfo.feature('watcher'); - - const securityChangeCallback = sinon.stub(); - securityFeature.registerLicenseChangeCallback(securityChangeCallback); - - const watcherChangeCallback = sinon.stub(); - watcherFeature.registerLicenseChangeCallback(watcherChangeCallback); - - mockElasticsearchCluster.callWithInternalUser.returns( - getMockXPackInfoAPIResponse({ mode: 'platinum' }) - ); - - await xPackInfo.refreshNow(); - - sinon.assert.calledOnce(securityChangeCallback); - sinon.assert.calledOnce(watcherChangeCallback); - }); - - it('registerLicenseChangeCallback() gracefully handles callbacks that throw errors', async () => { - const securityFeature = xPackInfo.feature('security'); - const watcherFeature = xPackInfo.feature('watcher'); - - const securityChangeCallback = sinon.stub().throws(new Error(`Something happened`)); - securityFeature.registerLicenseChangeCallback(securityChangeCallback); - - const watcherChangeCallback = sinon.stub(); - watcherFeature.registerLicenseChangeCallback(watcherChangeCallback); - - mockElasticsearchCluster.callWithInternalUser.returns( - getMockXPackInfoAPIResponse({ mode: 'platinum' }) - ); - - await xPackInfo.refreshNow(); - - sinon.assert.calledOnce(securityChangeCallback); - sinon.assert.calledOnce(watcherChangeCallback); - - sinon.assert.calledWithExactly( - mockServer.log, - ['license', 'error', 'xpack'], - `Error during invocation of license change callback for security. Error: Something happened` - ); - }); - it('getLicenseCheckResults() correctly returns feature specific info.', async () => { const securityFeature = xPackInfo.feature('security'); const watcherFeature = xPackInfo.feature('watcher'); diff --git a/x-pack/plugins/xpack_main/server/lib/xpack_info.js b/x-pack/plugins/xpack_main/server/lib/xpack_info.js index 52f5c5e14d13a..39766c0ac8156 100644 --- a/x-pack/plugins/xpack_main/server/lib/xpack_info.js +++ b/x-pack/plugins/xpack_main/server/lib/xpack_info.js @@ -36,12 +36,6 @@ export class XPackInfo { */ _licenseInfoChangedListeners = new Set(); - /** - * Feature name <-> license change callback mapping. - * @type {Map} - * @private - */ - _featureLicenseChangeCallbacks = new Map(); /** * Cache that may contain last xpack info API response or error, json representation @@ -224,16 +218,6 @@ export class XPackInfo { this._cache.signature = undefined; }, - /** - * Registers a callback function that will be called whenever the XPack license changes. - * Callback will be invoked after the license change have been applied to this XPack Info instance. - * Callbacks may be asynchronous, but will not be awaited. - * @param {Function} callback Function to call whenever the XPack license changes. - */ - registerLicenseChangeCallback: (callback) => { - this._featureLicenseChangeCallbacks.set(name, callback); - }, - /** * Returns license check results that were previously produced by the `generator` function. * @returns {Object} From a7ac136d914f0499caf2a9e7357644a2fb9f88d4 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Mon, 25 Jun 2018 07:57:46 -0400 Subject: [PATCH 2/2] update tests to use new 'not found' error message --- x-pack/test/rbac_api_integration/apis/saved_objects/delete.js | 2 +- x-pack/test/rbac_api_integration/apis/saved_objects/get.js | 2 +- x-pack/test/rbac_api_integration/apis/saved_objects/update.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/test/rbac_api_integration/apis/saved_objects/delete.js b/x-pack/test/rbac_api_integration/apis/saved_objects/delete.js index f89073acca7fd..d6bd0d61dd646 100644 --- a/x-pack/test/rbac_api_integration/apis/saved_objects/delete.js +++ b/x-pack/test/rbac_api_integration/apis/saved_objects/delete.js @@ -21,7 +21,7 @@ export default function ({ getService }) { expect(resp.body).to.eql({ statusCode: 404, error: 'Not Found', - message: 'Not Found' + message: 'Saved object [dashboard/not-a-real-id] not found' }); }; diff --git a/x-pack/test/rbac_api_integration/apis/saved_objects/get.js b/x-pack/test/rbac_api_integration/apis/saved_objects/get.js index 029a44475b12e..e055d9ef75e48 100644 --- a/x-pack/test/rbac_api_integration/apis/saved_objects/get.js +++ b/x-pack/test/rbac_api_integration/apis/saved_objects/get.js @@ -34,7 +34,7 @@ export default function ({ getService }) { const expectNotFound = (resp) => { expect(resp.body).to.eql({ error: 'Not Found', - message: 'Not Found', + message: 'Saved object [visualization/foobar] not found', statusCode: 404, }); }; diff --git a/x-pack/test/rbac_api_integration/apis/saved_objects/update.js b/x-pack/test/rbac_api_integration/apis/saved_objects/update.js index ae299348847d6..e3740ede1aaaf 100644 --- a/x-pack/test/rbac_api_integration/apis/saved_objects/update.js +++ b/x-pack/test/rbac_api_integration/apis/saved_objects/update.js @@ -34,7 +34,7 @@ export default function ({ getService }) { expect(resp.body).eql({ statusCode: 404, error: 'Not Found', - message: 'Not Found' + message: 'Saved object [visualization/not an id] not found' }); };