Provide uiSettings service in NP #48413

mshustov · 2019-10-16T14:44:27Z

Summary

This PR:

provides UiSettings service in NP. UiSettingsClient is available via RequestHanlderContext. Plugins can access register in the setup phase.
rename getDefaults to getRegistered. Only found usage in the core plugins and updated accordingly. I decided to update them because plugins are about to start using this API and I don't want to change public API later. uiSettings internals still uses defaults term.
move ui settings routes to NP
moves integrations tests to NP
add integration tests for uiSettings service
add typings for test_utils
convert other core_plugins functional tests into TS. Sorry for the additional noize.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~[ ] This was checked for cross-browser compatibility, including a check against IE11~~
~~[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
~~[ ] Documentation was added for features that require explanation or tutorials~~
Unit or functional tests were updated or added to match the most common scenarios
~~[ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

Dev Docs

New platform plugins can register custom uiSettings via uiSettings.register method

// src/plugins/my-plugin/server/plugin.ts
setup(core: CoreSetup){
  core.uiSettings.register({
    'my-plugin:my-setting': {
      name: 'just-work',
      value: true,
      description: 'make it work',
      category: ['my-category'],
    },
  })
}

UiSettings client can be accessed:

via RequestHandlerContext on server-side:

(context, req, res) {
  const uiSettingsClient = context.core.uiSettings.client;
  const value = await uiSettings.get('myPlugin:key');
  // ...
}

via core interface in setup/start lifecycles on the client-side:

public start({ uiSettings }) {
  const value = uiSettings.get('myPlugin:key');

elasticmachine · 2019-10-16T14:44:30Z

Pinging @elastic/kibana-platform (Team:Platform)

mshustov · 2019-10-16T14:56:26Z

src/core/server/server.ts

          },
          elasticsearch: {
            adminClient: adminClient.asScoped(req),
            dataClient: dataClient.asScoped(req),
          },
+          uiSettings: {
+            client: coreSetup.uiSettings.asScopedToClient(savedObjectsClient),


Note: There are several reasons to pass savedObjectsClient here:

Consistency. All context services use the same version of the client.

There is no way for uiSettings to declare a dependency on the saved object service at the moment. The current dependency graph is uiSettings -- used by --> legacy -- used by --> savedObjects.
We can work around it exposing register via start contract as well, but it's not something we want to support in the long term.

elasticmachine · 2019-10-16T16:08:56Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 27af6eb

mshustov · 2019-10-17T14:56:14Z

src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts

@@ -52,8 +51,17 @@ export async function createOrUpgradeSavedConfig<T extends SavedObjectAttribute
    // create the new SavedConfig
    await savedObjectsClient.create('config', attributes, { id: version });
  } catch (error) {
-    if (onWriteError) {
-      return onWriteError(error, attributes);
+    if (handleWriteErrors) {


I inlined this logic to improve testability.

elasticmachine · 2019-10-17T16:05:36Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 35742d2

mshustov · 2019-10-18T09:30:49Z

src/core/server/ui_settings/ui_settings_client.test.ts

-      const { uiSettings } = setup();
-      expect(uiSettings.assertUpdateAllowed('foo')).to.be(undefined);
-    });
-    it('throws 400 Boom error when keys is overridden', () => {


covered by CannotOverrideError tests

mshustov · 2019-10-18T09:31:46Z

src/core/server/ui_settings/ui_settings_client.test.ts

-    });
-  });
-
-  describe('#getRaw()', () => {


covered by getAll tests

mshustov · 2019-10-18T09:32:37Z

src/core/server/ui_settings/ui_settings_client.ts

    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))) {


I haven't found someone utilizing them in LP.

elasticmachine · 2019-10-18T09:33:29Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 22eb9bc

elasticmachine · 2019-10-18T11:20:30Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 7ad176d

mshustov · 2019-10-21T07:08:45Z

src/core/server/server.api.md

@@ -763,7 +765,7 @@ export type IScopedClusterClient = Pick<ScopedClusterClient, 'callAsCurrentUser'
 export interface IUiSettingsClient {
    get: <T extends SavedObjectAttribute = any>(key: string) => Promise<T>;
    getAll: <T extends SavedObjectAttribute = any>() => Promise<Record<string, T>>;
-    getDefaults: () => Record<string, UiSettingsParams>;
+    getRegistered: () => Readonly<Record<string, UiSettingsParams>>;


We can still keep getDefaults for BWC with 3rd party plugins. I decided we don't want to have an additional API to maintain. Any objections?

elasticmachine · 2019-10-21T08:28:12Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: d5a0b8b

joshdover · 2019-10-21T22:16:51Z

ack: will review tomorrow, sorry would have done sooner but I didn't see it in the kibana-platform project 😅

spalger

Renovate changes LGTM

mshustov · 2019-10-22T05:59:08Z

@joshdover my bad 😅

joshdover

Can you update the uiExport table in the migration guide with a link to the docs for registering a uiSetting?