From a9507dba585d2149e6104e88483406839a301ff0 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Thu, 17 Oct 2019 16:44:16 +0200 Subject: [PATCH] inline writeErrors into createOrUpgradeConfig to simplify testing --- .../create_or_upgrade_saved_config.test.ts | 108 ++++++++---------- .../create_or_upgrade_saved_config.ts | 22 ++-- .../create_or_upgrade.test.ts | 5 + .../ui_settings/ui_settings_client.test.ts | 12 ++ .../server/ui_settings/ui_settings_client.ts | 29 +---- 5 files changed, 82 insertions(+), 94 deletions(-) diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts index 5f7e915365873..65b8792532acf 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts @@ -20,6 +20,8 @@ import sinon from 'sinon'; import Chance from 'chance'; +import { SavedObjectsErrorHelpers } from '../../saved_objects'; + import { loggingServiceMock } from '../../logging/logging_service.mock'; import * as getUpgradeableConfigNS from './get_upgradeable_config'; import { createOrUpgradeSavedConfig } from './create_or_upgrade_saved_config'; @@ -50,6 +52,7 @@ describe('uiSettings/createOrUpgradeSavedConfig', function() { version, buildNum, log: logger.get(), + handleWriteErrors: false, ...options, }); @@ -173,85 +176,64 @@ describe('uiSettings/createOrUpgradeSavedConfig', function() { }); }); - describe('onWriteError()', () => { - it('is called with error and attributes when savedObjectsClient.create rejects', async () => { - const { run, savedObjectsClient } = setup(); + describe('handleWriteErrors', () => { + describe('handleWriteErrors: false', () => { + it('throws write errors', async () => { + const { run, savedObjectsClient } = setup(); + const error = new Error('foo'); + savedObjectsClient.create.callsFake(async () => { + throw error; + }); - const error = new Error('foo'); - savedObjectsClient.create.callsFake(async () => { - throw error; - }); - - const onWriteError = sinon.stub(); - await run({ onWriteError }); - sinon.assert.calledOnce(onWriteError); - sinon.assert.calledWithExactly(onWriteError, error, { - buildNum, + await expect(run({ handleWriteErrors: false })).rejects.toThrowError(error); }); }); + describe('handleWriteErrors:true', () => { + it('returns undefined for ConflictError', async () => { + const { run, savedObjectsClient } = setup(); + const error = new Error('foo'); + savedObjectsClient.create.throws(SavedObjectsErrorHelpers.decorateConflictError(error)); - it('resolves with the return value of onWriteError()', async () => { - const { run, savedObjectsClient } = setup(); - - savedObjectsClient.create.callsFake(async () => { - throw new Error('foo'); + expect(await run({ handleWriteErrors: true })).toBe(undefined); }); - const result = await run({ onWriteError: () => 123 }); - expect(result).toBe(123); - }); - - it('rejects with the error from onWriteError() if it rejects', async () => { - const { run, savedObjectsClient } = setup(); + it('returns config attributes for NotAuthorizedError', async () => { + const { run, savedObjectsClient } = setup(); + const error = new Error('foo'); + savedObjectsClient.create.throws( + SavedObjectsErrorHelpers.decorateNotAuthorizedError(error) + ); - savedObjectsClient.create.callsFake(async () => { - throw new Error('foo'); + expect(await run({ handleWriteErrors: true })).toEqual({ + buildNum, + }); }); - try { - await run({ - onWriteError: (error: Error) => Promise.reject(new Error(`${error.message} bar`)), + it('returns config attributes for ForbiddenError', async () => { + const { run, savedObjectsClient } = setup(); + const error = new Error('foo'); + savedObjectsClient.create.throws(SavedObjectsErrorHelpers.decorateForbiddenError(error)); + + expect(await run({ handleWriteErrors: true })).toEqual({ + buildNum, }); - throw new Error('expected run() to reject'); - } catch (error) { - expect(error.message).toBe('foo bar'); - } - }); + }); - it('rejects with the error from onWriteError() if it throws sync', async () => { - const { run, savedObjectsClient } = setup(); + it('throws error for other SavedObjects exceptions', async () => { + const { run, savedObjectsClient } = setup(); + const error = new Error('foo'); + savedObjectsClient.create.throws(SavedObjectsErrorHelpers.decorateGeneralError(error)); - savedObjectsClient.create.callsFake(async () => { - throw new Error('foo'); + await expect(run({ handleWriteErrors: true })).rejects.toThrowError(error); }); - try { - await run({ - onWriteError: (error: Error) => { - throw new Error(`${error.message} bar`); - }, - }); - throw new Error('expected run() to reject'); - } catch (error) { - expect(error.message).toBe('foo bar'); - } - }); + it('throws error for all other exceptions', async () => { + const { run, savedObjectsClient } = setup(); + const error = new Error('foo'); + savedObjectsClient.create.throws(error); - it('rejects with the writeError if onWriteError() is undefined', async () => { - const { run, savedObjectsClient } = setup(); - - savedObjectsClient.create.callsFake(async () => { - throw new Error('foo'); + await expect(run({ handleWriteErrors: true })).rejects.toThrowError(error); }); - - try { - await run({ - onWriteError: undefined, - }); - throw new Error('expected run() to reject'); - } catch (error) { - expect(error.message).toBe('foo'); - } }); }); }); diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts index 1655297adb6c9..809e15248b5b0 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts @@ -20,6 +20,7 @@ import { defaults } from 'lodash'; import { SavedObjectsClientContract, SavedObjectAttribute } from '../../saved_objects/types'; +import { SavedObjectsErrorHelpers } from '../../saved_objects/'; import { Logger } from '../../logging'; import { getUpgradeableConfig } from './get_upgradeable_config'; @@ -29,15 +30,13 @@ interface Options { version: string; buildNum: number; log: Logger; - onWriteError?: ( - error: Error, - attributes: Record - ) => Record | undefined; + handleWriteErrors: boolean; } + export async function createOrUpgradeSavedConfig( options: Options ): Promise | undefined> { - const { savedObjectsClient, version, buildNum, log, onWriteError } = options; + const { savedObjectsClient, version, buildNum, log, handleWriteErrors } = options; // try to find an older config we can upgrade const upgradeableConfig = await getUpgradeableConfig({ @@ -52,8 +51,17 @@ export async function createOrUpgradeSavedConfig { version: '5.4.0', buildNum: 54099, log: logger, + handleWriteErrors: false, }); const config540 = await savedObjectsClient.get('config', '5.4.0'); @@ -119,6 +120,7 @@ describe('createOrUpgradeSavedConfig()', () => { version: '5.4.1', buildNum: 54199, log: logger, + handleWriteErrors: false, }); const config541 = await savedObjectsClient.get('config', '5.4.1'); @@ -145,6 +147,7 @@ describe('createOrUpgradeSavedConfig()', () => { version: '7.0.0-rc1', buildNum: 70010, log: logger, + handleWriteErrors: false, }); const config700rc1 = await savedObjectsClient.get('config', '7.0.0-rc1'); @@ -172,6 +175,7 @@ describe('createOrUpgradeSavedConfig()', () => { version: '7.0.0', buildNum: 70099, log: logger, + handleWriteErrors: false, }); const config700 = await savedObjectsClient.get('config', '7.0.0'); @@ -200,6 +204,7 @@ describe('createOrUpgradeSavedConfig()', () => { version: '6.2.3-rc1', buildNum: 62310, log: logger, + handleWriteErrors: false, }); const config623rc1 = await savedObjectsClient.get('config', '6.2.3-rc1'); diff --git a/src/core/server/ui_settings/ui_settings_client.test.ts b/src/core/server/ui_settings/ui_settings_client.test.ts index 2403f90317c30..1108bb3f1e6df 100644 --- a/src/core/server/ui_settings/ui_settings_client.test.ts +++ b/src/core/server/ui_settings/ui_settings_client.test.ts @@ -120,7 +120,12 @@ describe('ui settings', () => { await uiSettings.setMany({ foo: 'bar' }); sinon.assert.calledTwice(savedObjectsClient.update); + sinon.assert.calledOnce(createOrUpgradeSavedConfig); + sinon.assert.calledWith( + createOrUpgradeSavedConfig, + sinon.match({ handleWriteErrors: false }) + ); }); it('only tried to auto create once and throws NotFound', async () => { @@ -136,6 +141,11 @@ describe('ui settings', () => { sinon.assert.calledTwice(savedObjectsClient.update); sinon.assert.calledOnce(createOrUpgradeSavedConfig); + + sinon.assert.calledWith( + createOrUpgradeSavedConfig, + sinon.match({ handleWriteErrors: false }) + ); }); it('throws CannotOverrideError if the key is overridden', async () => { @@ -299,7 +309,9 @@ describe('ui settings', () => { expect(await uiSettings.getUserProvided()).to.eql({}); sinon.assert.calledTwice(savedObjectsClient.get); + sinon.assert.calledOnce(createOrUpgradeSavedConfig); + sinon.assert.calledWith(createOrUpgradeSavedConfig, sinon.match({ handleWriteErrors: true })); }); it('returns result of savedConfig creation in case of notFound error', async () => { diff --git a/src/core/server/ui_settings/ui_settings_client.ts b/src/core/server/ui_settings/ui_settings_client.ts index af4665fcc80a9..039ddec91a033 100644 --- a/src/core/server/ui_settings/ui_settings_client.ts +++ b/src/core/server/ui_settings/ui_settings_client.ts @@ -18,6 +18,7 @@ */ import { defaultsDeep } from 'lodash'; +import { SavedObjectsErrorHelpers } from '../saved_objects'; import { SavedObjectsClientContract, SavedObjectAttribute } from '../saved_objects/types'; import { Logger } from '../logging'; import { createOrUpgradeSavedConfig } from './create_or_upgrade_saved_config'; @@ -163,8 +164,7 @@ export class UiSettingsClient implements IUiSettingsClient { try { await this.savedObjectsClient.update(this.type, this.id, changes); } catch (error) { - const { isNotFoundError } = this.savedObjectsClient.errors; - if (!isNotFoundError(error) || !autoCreateOrUpgradeIfMissing) { + if (!SavedObjectsErrorHelpers.isNotFoundError(error) || !autoCreateOrUpgradeIfMissing) { throw error; } @@ -173,6 +173,7 @@ export class UiSettingsClient implements IUiSettingsClient { version: this.id, buildNum: this.buildNum, log: this.log, + handleWriteErrors: false, }); await this.write({ @@ -186,37 +187,17 @@ export class UiSettingsClient implements IUiSettingsClient { ignore401Errors = false, autoCreateOrUpgradeIfMissing = true, }: ReadOptions = {}): Promise> { - const { - isConflictError, - isNotFoundError, - isForbiddenError, - isNotAuthorizedError, - } = this.savedObjectsClient.errors; - try { const resp = await this.savedObjectsClient.get(this.type, this.id); return resp.attributes; } catch (error) { - if (isNotFoundError(error) && autoCreateOrUpgradeIfMissing) { + if (SavedObjectsErrorHelpers.isNotFoundError(error) && autoCreateOrUpgradeIfMissing) { const failedUpgradeAttributes = await createOrUpgradeSavedConfig({ savedObjectsClient: this.savedObjectsClient, version: this.id, buildNum: this.buildNum, log: this.log, - onWriteError(writeError, attributes) { - if (isConflictError(writeError)) { - // trigger `!failedUpgradeAttributes` check below, since another - // request caused the uiSettings object to be created so we can - // just re-read - return; - } - - if (isNotAuthorizedError(writeError) || isForbiddenError(writeError)) { - return attributes; - } - - throw writeError; - }, + handleWriteErrors: true, }); if (!failedUpgradeAttributes) {