hide client private methods, update tests

elastic · Oct 17, 2019 · 6564624 · 6564624
1 parent 851c48d
commit 6564624
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 96 deletions.
diff --git a/src/core/server/ui_settings/types.ts b/src/core/server/ui_settings/types.ts
@@ -26,7 +26,7 @@ export interface IUiSettingsClient {
   /**
    * Returns uiSettings default values {@link UiSettingsParams}
    */
-  getDefaults: () => Record<string, UiSettingsParams>;
+  getDefaults: () => Readonly<Record<string, UiSettingsParams>>;
   /**
    * Retrieves uiSettings values set by the user with fallbacks to default values if not specified.
    */

diff --git a/src/core/server/ui_settings/ui_settings_client.test.ts b/src/core/server/ui_settings/ui_settings_client.test.ts
@@ -138,7 +138,7 @@ describe('ui settings', () => {
       sinon.assert.calledOnce(createOrUpgradeSavedConfig);
     });
 
-    it('throws an error if any key is overridden', async () => {
+    it('throws CannotOverrideError if the key is overridden', async () => {
       const { uiSettings } = setup({
         overrides: {
           foo: 'bar',
@@ -151,6 +151,7 @@ describe('ui settings', () => {
           foo: 'baz',
         });
       } catch (error) {
+        expect(error).to.be.a(CannotOverrideError);
         expect(error.message).to.be('Unable to update "foo" because it is overridden');
       }
     });
@@ -168,7 +169,7 @@ describe('ui settings', () => {
       assertUpdateQuery({ one: 'value' });
     });
 
-    it('throws an error if the key is overridden', async () => {
+    it('throws CannotOverrideError if the key is overridden', async () => {
       const { uiSettings } = setup({
         overrides: {
           foo: 'bar',
@@ -178,6 +179,7 @@ describe('ui settings', () => {
       try {
         await uiSettings.set('foo', 'baz');
       } catch (error) {
+        expect(error).to.be.a(CannotOverrideError);
         expect(error.message).to.be('Unable to update "foo" because it is overridden');
       }
     });
@@ -195,7 +197,7 @@ describe('ui settings', () => {
       assertUpdateQuery({ one: null });
     });
 
-    it('throws an error if the key is overridden', async () => {
+    it('throws CannotOverrideError if the key is overridden', async () => {
       const { uiSettings } = setup({
         overrides: {
           foo: 'bar',
@@ -205,6 +207,7 @@ describe('ui settings', () => {
       try {
         await uiSettings.remove('foo');
       } catch (error) {
+        expect(error).to.be.a(CannotOverrideError);
         expect(error.message).to.be('Unable to update "foo" because it is overridden');
       }
     });
@@ -228,7 +231,7 @@ describe('ui settings', () => {
       assertUpdateQuery({ one: null, two: null, three: null });
     });
 
-    it('throws an error if any key is overridden', async () => {
+    it('throws CannotOverrideError if any key is overridden', async () => {
       const { uiSettings } = setup({
         overrides: {
           foo: 'bar',
@@ -238,6 +241,7 @@ describe('ui settings', () => {
       try {
         await uiSettings.setMany({ baz: 'baz', foo: 'foo' });
       } catch (error) {
+        expect(error).to.be.a(CannotOverrideError);
         expect(error.message).to.be('Unable to update "foo" because it is overridden');
       }
     });
@@ -246,10 +250,9 @@ describe('ui settings', () => {
   describe('#getDefaults()', () => {
     it('returns the defaults passed to the constructor', () => {
       const value = chance.word();
-      const { uiSettings } = setup({ defaults: { key: { value } } });
-      expect(uiSettings.getDefaults()).to.eql({
-        key: { value },
-      });
+      const defaults = { key: { value } };
+      const { uiSettings } = setup({ defaults });
+      expect(uiSettings.getDefaults()).to.be(defaults);
     });
   });
 
@@ -285,31 +288,46 @@ describe('ui settings', () => {
       });
     });
 
-    it.skip('returns an empty object on NotFound responses', async () => {
-      const { uiSettings, savedObjectsClient } = setup();
+    it('automatically creates the savedConfig if it is missing and returns empty object', async () => {
+      const { uiSettings, savedObjectsClient, createOrUpgradeSavedConfig } = setup();
+      savedObjectsClient.get
+        .onFirstCall()
+        .throws(savedObjectsClientErrors.createGenericNotFoundError())
+        .onSecondCall()
+        .returns({ attributes: {} });
 
-      const error = savedObjectsClientErrors.createGenericNotFoundError();
-      savedObjectsClient.get.throws(error);
+      expect(await uiSettings.getUserProvided()).to.eql({});
 
-      expect(await uiSettings.getUserProvided({})).to.eql({});
+      sinon.assert.calledTwice(savedObjectsClient.get);
+      sinon.assert.calledOnce(createOrUpgradeSavedConfig);
+    });
+
+    it('returns result of savedConfig creation in case of notFound error', async () => {
+      const { uiSettings, savedObjectsClient, createOrUpgradeSavedConfig } = setup();
+      createOrUpgradeSavedConfig.resolves({ foo: 'bar ' });
+      savedObjectsClient.get.throws(savedObjectsClientErrors.createGenericNotFoundError());
+
+      expect(await uiSettings.getUserProvided()).to.eql({ foo: { userValue: 'bar ' } });
     });
 
     it('returns an empty object on Forbidden responses', async () => {
-      const { uiSettings, savedObjectsClient } = setup();
+      const { uiSettings, savedObjectsClient, createOrUpgradeSavedConfig } = setup();
 
       const error = savedObjectsClientErrors.decorateForbiddenError(new Error());
       savedObjectsClient.get.throws(error);
 
       expect(await uiSettings.getUserProvided()).to.eql({});
+      sinon.assert.notCalled(createOrUpgradeSavedConfig);
     });
 
     it('returns an empty object on EsUnavailable responses', async () => {
-      const { uiSettings, savedObjectsClient } = setup();
+      const { uiSettings, savedObjectsClient, createOrUpgradeSavedConfig } = setup();
 
       const error = savedObjectsClientErrors.decorateEsUnavailableError(new Error());
       savedObjectsClient.get.throws(error);
 
       expect(await uiSettings.getUserProvided()).to.eql({});
+      sinon.assert.notCalled(createOrUpgradeSavedConfig);
     });
 
     it('throws Unauthorized errors', async () => {
@@ -347,6 +365,7 @@ describe('ui settings', () => {
 
       const overrides = {
         foo: 'bar',
+        baz: null,
       };
 
       const { uiSettings } = setup({ esDocSource, overrides });
@@ -358,57 +377,7 @@ describe('ui settings', () => {
           userValue: 'bar',
           isOverridden: true,
         },
-      });
-    });
-  });
-
-  describe('#getRaw()', () => {
-    it('pulls user configuration from ES', async () => {
-      const esDocSource = {};
-      const { uiSettings, assertGetQuery } = setup({ esDocSource });
-      await uiSettings.getRaw();
-      assertGetQuery();
-    });
-
-    it(`without user configuration it's equal to the defaults`, async () => {
-      const esDocSource = {};
-      const defaults = { key: { value: chance.word() } };
-      const { uiSettings } = setup({ esDocSource, defaults });
-      const result = await uiSettings.getRaw();
-      expect(result).to.eql(defaults);
-    });
-
-    it(`user configuration gets merged with defaults`, async () => {
-      const esDocSource = { foo: 'bar' };
-      const defaults = { key: { value: chance.word() } };
-      const { uiSettings } = setup({ esDocSource, defaults });
-      const result = await uiSettings.getRaw();
-
-      expect(result).to.eql({
-        foo: {
-          userValue: 'bar',
-        },
-        key: {
-          value: defaults.key.value,
-        },
-      });
-    });
-
-    it('includes the values for overridden keys', async () => {
-      const esDocSource = { foo: 'bar' };
-      const defaults = { key: { value: chance.word() } };
-      const overrides = { foo: true };
-      const { uiSettings } = setup({ esDocSource, defaults, overrides });
-      const result = await uiSettings.getRaw();
-
-      expect(result).to.eql({
-        foo: {
-          userValue: true,
-          isOverridden: true,
-        },
-        key: {
-          value: defaults.key.value,
-        },
+        baz: { isOverridden: true },
       });
     });
   });
@@ -546,21 +515,4 @@ describe('ui settings', () => {
       expect(uiSettings.isOverridden('bar')).to.be(true);
     });
   });
-
-  describe('#assertUpdateAllowed()', () => {
-    it('returns false if no overrides defined', () => {
-      const { uiSettings } = setup();
-      expect(uiSettings.assertUpdateAllowed('foo')).to.be(undefined);
-    });
-    it('throws CannotOverrideError when key is overridden', () => {
-      const { uiSettings } = setup({ overrides: { foo: true } });
-      expect(() => uiSettings.assertUpdateAllowed('foo')).to.throwError(error => {
-        expect(error).to.be.a(CannotOverrideError);
-        expect(error).to.have.property(
-          'message',
-          'Unable to update "foo" because it is overridden'
-        );
-      });
-    });
-  });
 });
diff --git a/src/core/server/ui_settings/ui_settings_client.ts b/src/core/server/ui_settings/ui_settings_client.ts
@@ -92,19 +92,11 @@ export class UiSettingsClient implements IUiSettingsClient {
     );
   }
 
-  // NOTE: should be a private method
-  async getRaw(): Promise<UiSettingsRaw> {
-    const userProvided = await this.getUserProvided();
-    return defaultsDeep(userProvided, this.defaults);
-  }
-
-  async getUserProvided<T extends SavedObjectAttribute = any>(
-    options: ReadOptions = {}
-  ): Promise<UserProvided<T>> {
+  async getUserProvided<T extends SavedObjectAttribute = any>(): Promise<UserProvided<T>> {
     const userProvided: UserProvided = {};
 
     // write the userValue for each key stored in the saved object that is not overridden
-    for (const [key, userValue] of Object.entries(await this.read(options))) {
+    for (const [key, userValue] of Object.entries(await this.read())) {
       if (userValue !== null && !this.isOverridden(key)) {
         userProvided[key] = {
           userValue,
@@ -146,13 +138,17 @@ export class UiSettingsClient implements IUiSettingsClient {
     return this.overrides.hasOwnProperty(key);
   }
 
-  // NOTE: should be private method
-  assertUpdateAllowed(key: string) {
+  private assertUpdateAllowed(key: string) {
     if (this.isOverridden(key)) {
       throw new CannotOverrideError(`Unable to update "${key}" because it is overridden`);
     }
   }
 
+  private async getRaw(): Promise<UiSettingsRaw> {
+    const userProvided = await this.getUserProvided();
+    return defaultsDeep(userProvided, this.defaults);
+  }
+
   private async write<T extends SavedObjectAttribute = any>({
     changes,
     autoCreateOrUpgradeIfMissing = true,

diff --git a/src/core/server/ui_settings/ui_settings_service.test.ts b/src/core/server/ui_settings/ui_settings_service.test.ts
@@ -52,6 +52,14 @@ afterEach(() => {
 describe('uiSettings', () => {
   describe('#setup', () => {
     describe('#asScopedToClient', () => {
+      it('passes saved object type "config" to UiSettingsClient', async () => {
+        const service = new UiSettingsService(coreContext);
+        const setup = await service.setup(setupDeps);
+        setup.asScopedToClient(savedObjectsClient);
+        expect(MockUiSettingsClientConstructor).toBeCalledTimes(1);
+        expect(MockUiSettingsClientConstructor.mock.calls[0][0].type).toBe('config');
+      });
+
       it('passes overrides to UiSettingsClient', async () => {
         const service = new UiSettingsService(coreContext);
         const setup = await service.setup(setupDeps);