Allow savedObjects types registration from NP #57430

pgayvallet · 2020-02-12T09:14:01Z

Summary

This PR officially makes the registerType API introduced in #55857 a public one, and allow plugins to register savedObjects types from the new platform.

PR does the following:

move the registerType SO setup API from internal to public
expose a new getTypeRegistry API to the SO start contract to allow plugin to get informations on the registered types (this was a need for a least security)
change the SavedObjectMigrationFn signature to have a context object as second parameter instead of only a logger. This context only exposes the logger atm, but makes any futur addition now way easier (thanks to @restrry for the suggestion). Legacy migration methods are wrapped to continue passing the logger as the second parameter to ensure BWC.
improve documentation on some SO types

Requires #56971 to be merged.

Fix #50313, #50311, #50309

Checklist

Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

Dev Docs

A new registerType API has been added to the core savedObjects setup API, allowing to register savedObject types from new platform plugins

// src/plugins/my_plugin/server/saved_objects/types.ts
import { SavedObjectsType } from 'src/core/server';
import * as migrations from './migrations';

export const myType: SavedObjectsType = {
  name: 'MyType',
  hidden: false,
  namespaceAgnostic: true,
  mappings: {
    properties: {
      textField: {
        type: 'text',
      },
      boolField: {
        type: 'boolean',
      },
    },
  },
  migrations: {
    '2.0.0': migrations.migrateToV2,
    '2.1.0': migrations.migrateToV2_1
  },
};

// src/plugins/my_plugin/server/plugin.ts
import { SavedObjectsClient, CoreSetup } from 'src/core/server';
import { myType } from './saved_objects';

export class Plugin() {
  setup: (core: CoreSetup) => {
    core.savedObjects.registerType(myType);
  }
}

Please check the migration guide for more complete examples and migration procedure.

…register-type-so-api

elasticmachine · 2020-02-12T10:22:10Z

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

pgayvallet · 2020-02-12T10:26:19Z

src/core/server/saved_objects/saved_objects_service.ts

- * Note: The Saved Object setup API's should only be used for creating and
- * registering client wrappers. Constructing a Saved Objects client or
- * repository for use within your own plugin won't have any of the registered
- * wrappers applied and is considered an anti-pattern. Use the Saved Objects
- * client from the
- * {@link SavedObjectsServiceStart | SavedObjectsServiceStart#getScopedClient }


Removed outdated comment, as it's no longer possible to create clients or repository from the setup contract

pgayvallet · 2020-02-12T10:28:06Z

src/core/server/saved_objects/saved_objects_type_registry.ts

 /**
 * Registry holding information about all the registered {@link SavedObjectsType | savedObject types}.
 *
- * @internal
+ * @public
 */
 export class SavedObjectTypeRegistry {


I was 'forced' to expose this class as public to get proper doc generation for the associated ISavedObjectTypeRegistry interface. I'm starting to wonder if we really should use Pick-based interface when exposing public interfaces from internal implementations...

…register-type-so-api

mshustov · 2020-02-24T10:19:35Z

docs/development/core/server/kibana-plugin-server.savedobjectsservicesetup.md

@@ -4,7 +4,7 @@

 ## SavedObjectsServiceSetup interface

-Saved Objects is Kibana's data persistence mechanism allowing plugins to use Elasticsearch for storing and querying state. The SavedObjectsServiceSetup API exposes methods for creating and registering Saved Object client wrappers.
+Saved Objects is Kibana's data persistence mechanism allowing plugins to use Elasticsearch for storing and querying state. The SavedObjectsServiceSetup API exposes methods for registering Saved Object types and creating and registering Saved Object client wrappers and factories


The SavedObjectsServiceSetup API exposes methods for registering Saved Object types, creating and registering Saved Object client wrappers and factories. 🤔 ?

That would be indeed better 😄

mshustov · 2020-02-24T10:21:53Z

src/core/MIGRATION_EXAMPLES.md

+
+### Changes in structure compared to legacy
+
+The NP `registerType` expected input is very close to the legacy format, However there are some minor changes:


Suggested change

The NP `registerType` expected input is very close to the legacy format, However there are some minor changes:

The NP `registerType` expected input is very close to the legacy format. However, there are some minor changes:

mshustov · 2020-02-24T11:27:23Z

src/core/server/saved_objects/saved_objects_service.ts

+   *
+   * @example
+   * ```ts
+   * // src/plugins/my_plugin/server/saved_objects/types.ts


Is there a convention to place SO definition under the saved_objects folder? I can't see it in https://github.com/elastic/kibana/blob/ec31611911c8d7630d61361f6aa22d57f86a964f/src/core/CONVENTIONS.md Probably we should add one?

We should. Added a section and updated the example folder structure

mshustov · 2020-02-24T12:18:31Z

src/core/server/saved_objects/utils.test.ts

+    expect(Object.keys(converted[1]!.migrations!)).toEqual(Object.keys(migrationsB));
+  });
+
+  it('migrates the migration to the new format', () => {


Suggested change

it('migrates the migration to the new format', () => {

it('converts the migration to the new format', () => {

…register-type-so-api

pgayvallet · 2020-02-24T22:49:40Z

retest

jportner · 2020-02-24T22:59:24Z

From an offline discussion:

It would be great if we can expose the type registry (or just the getters) in the SavedObjectsClientWrapperOptions. Then this PR would resolve #58414.

rudolf

Some documentation nits, but otherwise 👍

rudolf · 2020-02-25T08:21:59Z

docs/development/core/server/kibana-plugin-server.savedobjectmigrationfn.md

@@ -4,10 +4,26 @@

 ## SavedObjectMigrationFn type

-A migration function defined for a [saved objects type](./kibana-plugin-server.savedobjectstype.md) used to migrate it's 
+A migration function defined for a [saved objects type](./kibana-plugin-server.savedobjectstype.md) used to migrate it's  to a given version


nit:

Suggested change

A migration function defined for a [saved objects type](./kibana-plugin-server.savedobjectstype.md) used to migrate it's to a given version

A migration function for a [saved object type](./kibana-plugin-server.savedobjectstype.md) used to migrate it to a given version

rudolf · 2020-02-25T08:30:42Z

src/core/MIGRATION_EXAMPLES.md

+
+## Saved Objects types
+
+In the legacy platform, saved object types were registered using static definitions in `uiExports` part 


Suggested change

In the legacy platform, saved object types were registered using static definitions in `uiExports` part

In the legacy platform, saved object types were registered using static definitions in the `uiExports` part of

rudolf · 2020-02-25T08:33:51Z

src/core/MIGRATION_EXAMPLES.md

+- The `schema.indexPattern` was accepting either a `string` or a `(config: LegacyConfig) => string`. `SavedObjectsType.indexPattern` only accepts a string, as you can access the configuration during your plugin's setup phase.
+
+- The migration function signature has changed:
+In legacy, is was `(doc: SavedObjectUnsanitizedDoc, log: SavedObjectsMigrationLogger) => SavedObjectUnsanitizedDoc;`


Suggested change

In legacy, is was `(doc: SavedObjectUnsanitizedDoc, log: SavedObjectsMigrationLogger) => SavedObjectUnsanitizedDoc;`

In legacy, it was `(doc: SavedObjectUnsanitizedDoc, log: SavedObjectsMigrationLogger) => SavedObjectUnsanitizedDoc;`

rudolf · 2020-02-25T08:46:04Z

src/core/server/saved_objects/migrations/types.ts

+ *   try {
+ *     doc.attributes.someProp = migrateProperty(doc.attributes.someProp);
+ *   } catch(e) {
+ *     log.warn('Error migrating `someProp`');


Catching migration exceptions is a common pattern in existing migrations but I think it's something we should discourage. It effectively causes a bug in some unknown place higher up in the stack when this object is eventually used. Also when we introduce "invalid" saved objects we want the migration system to catch and handle these migration exceptions.

It was mostly to demonstrate a use of the log context property. I will remove the try/catch and add another dummy check instead

rudolf · 2020-02-25T08:46:45Z

src/core/server/saved_objects/saved_objects_service.ts

+   *
+   * @example
+   * ```ts
+   * // src/plugins/my_plugin/server/saved_objects/types.ts


Suggested change

* // src/plugins/my_plugin/server/saved_objects/types.ts

* // src/plugins/my_plugin/server/saved_objects/my_type.ts

rudolf · 2020-02-25T08:48:25Z

src/core/MIGRATION_EXAMPLES.md

+First type:
+
+```typescript
+// src/plugins/my-plugin/server/saved_objects/first_type.ts


Suggested change

// src/plugins/my-plugin/server/saved_objects/first_type.ts

// src/plugins/my_plugin/server/saved_objects/first_type.ts

rudolf · 2020-02-25T08:48:32Z

src/core/MIGRATION_EXAMPLES.md

+```
+
+```js
+// src/legacy/core_plugins/my-plugin/migrations.js


Suggested change

// src/legacy/core_plugins/my-plugin/migrations.js

// src/legacy/core_plugins/my_plugin/migrations.js

rudolf · 2020-02-25T08:48:41Z

src/core/MIGRATION_EXAMPLES.md

+```
+
+```json
+// src/legacy/core_plugins/my-plugin/mappings.json


Suggested change

// src/legacy/core_plugins/my-plugin/mappings.json

// src/legacy/core_plugins/my_plugin/mappings.json

rudolf · 2020-02-25T08:48:50Z

src/core/MIGRATION_EXAMPLES.md

+Let say we have the following in a legacy plugin:
+
+```js
+// src/legacy/core_plugins/my-plugin/index.js


Suggested change

// src/legacy/core_plugins/my-plugin/index.js

// src/legacy/core_plugins/my_plugin/index.js

rudolf · 2020-02-25T08:49:22Z

src/core/MIGRATION_EXAMPLES.md

+Second type:
+
+```typescript
+// src/plugins/my-plugin/server/saved_objects/second_type.ts


Suggested change

// src/plugins/my-plugin/server/saved_objects/second_type.ts

// src/plugins/my_plugin/server/saved_objects/second_type.ts

rudolf

more nits (unfortunately I couldn't always make suggestions on the source code)

rudolf · 2020-02-25T08:52:56Z

docs/development/core/server/kibana-plugin-server.savedobjecttyperegistry.md

+
+## SavedObjectTypeRegistry class
+
+Registry holding information about all the registered [savedObject types](./kibana-plugin-server.savedobjectstype.md)<!-- -->.


Suggested change

Registry holding information about all the registered [savedObject types](./kibana-plugin-server.savedobjectstype.md).

Registry holding information about all the registered [saved object types](./kibana-plugin-server.savedobjectstype.md).

rudolf · 2020-02-25T08:53:10Z

src/core/server/saved_objects/saved_objects_service.ts

@@ -158,6 +203,11 @@ export interface SavedObjectsServiceStart {
   * Creates a {@link SavedObjectsSerializer | serializer} that is aware of all registered types.
   */
  createSerializer: () => SavedObjectsSerializer;
+  /**
+   * Returns the {@link ISavedObjectTypeRegistry | registry} containing all registered
+   * {@link SavedObjectsType | savedObject types}


Suggested change

* {@link SavedObjectsType | savedObject types}

* {@link SavedObjectsType | saved object types}

rudolf · 2020-02-25T08:54:29Z

docs/development/core/server/kibana-plugin-server.md

@@ -27,6 +27,7 @@ The plugin integrates with the core system via lifecycle events: `setup`<!-- -->
 |  [SavedObjectsErrorHelpers](./kibana-plugin-server.savedobjectserrorhelpers.md) |  |
 |  [SavedObjectsRepository](./kibana-plugin-server.savedobjectsrepository.md) |  |
 |  [SavedObjectsSerializer](./kibana-plugin-server.savedobjectsserializer.md) | A serializer that can be used to manually convert [raw](./kibana-plugin-server.savedobjectsrawdoc.md) or [sanitized](./kibana-plugin-server.savedobjectsanitizeddoc.md) documents to the other kind. |
+|  [SavedObjectTypeRegistry](./kibana-plugin-server.savedobjecttyperegistry.md) | Registry holding information about all the registered [savedObject types](./kibana-plugin-server.savedobjectstype.md)<!-- -->. |


Suggested change

| [SavedObjectTypeRegistry](./kibana-plugin-server.savedobjecttyperegistry.md) | Registry holding information about all the registered [savedObject types](./kibana-plugin-server.savedobjectstype.md). |

| [SavedObjectTypeRegistry](./kibana-plugin-server.savedobjecttyperegistry.md) | Registry holding information about all the registered [saved object types](./kibana-plugin-server.savedobjectstype.md). |

…register-type-so-api

pgayvallet · 2020-02-25T11:14:01Z

@jportner Added in 18f5c05, tell me if that looks alright to you

jportner · 2020-02-25T14:21:08Z

@jportner Added in 18f5c05, tell me if that looks alright to you

Beautiful!

…register-type-so-api

kibanamachine · 2020-02-26T13:41:39Z

💚 Build Succeeded

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

History

💚 Build #28767 succeeded 18f5c05
💚 Build #28651 succeeded 15e7646
💔 Build #28474 failed c3efbb1
💔 Build #28443 failed e408a9c
💔 Build #28414 failed 6857054

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* expose `registerType` API * expose `getTypeRegistry` API * change SavedObjectMigrationFn signature to add context * fix exported types * update generated doc * update migration documentation * fix legacy service test * fix typings * update service setup description * add saved_objects server folder convention * fix unit test * documentation NITs * add typeRegistry to SavedObjectClientWrapperOptions

pgayvallet added 7 commits February 12, 2020 10:12

expose registerType API

af5ac5e

expose getTypeRegistry API

97f8c1e

change SavedObjectMigrationFn signature to add context

cbf73b3

fix exported types

b5ad014

update generated doc

3b8cd43

update migration documentation

9e7aa96

Merge remote-tracking branch 'upstream/master' into kbn-503xx-expose-…

af8c10d

…register-type-so-api

pgayvallet added Feature:New Platform release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.7.0 v8.0.0 labels Feb 12, 2020

pgayvallet marked this pull request as ready for review February 12, 2020 10:22

pgayvallet requested a review from a team as a code owner February 12, 2020 10:22

pgayvallet commented Feb 12, 2020

View reviewed changes

pgayvallet added 3 commits February 12, 2020 12:47

fix legacy service test

d3937c7

Merge remote-tracking branch 'upstream/master' into kbn-503xx-expose-…

76f0dd4

…register-type-so-api

Merge remote-tracking branch 'upstream/master' into kbn-503xx-expose-…

1989e89

…register-type-so-api

mshustov reviewed Feb 24, 2020

View reviewed changes

pgayvallet added 2 commits February 24, 2020 13:05

fix typings

d2f1016

update service setup description

6857054

mshustov approved these changes Feb 24, 2020

View reviewed changes

pgayvallet added 4 commits February 24, 2020 14:46

Merge remote-tracking branch 'upstream/master' into kbn-503xx-expose-…

9422f06

…register-type-so-api

add saved_objects server folder convention

e408a9c

fix unit test

c3efbb1

Merge remote-tracking branch 'upstream/master' into kbn-503xx-expose-…

15e7646

…register-type-so-api

rudolf approved these changes Feb 25, 2020

View reviewed changes

pgayvallet added 3 commits February 25, 2020 11:50

Merge remote-tracking branch 'upstream/master' into kbn-503xx-expose-…

9c93b40

…register-type-so-api

documentation NITs

6c8a015

add typeRegistry to SavedObjectClientWrapperOptions

18f5c05

jportner linked an issue Feb 25, 2020 that may be closed by this pull request

Expose getters for SavedObjectTypeRegistry type definitions #58414

Closed

Merge remote-tracking branch 'upstream/master' into kbn-503xx-expose-…

7d1475f

…register-type-so-api

pgayvallet merged commit 8524303 into elastic:master Feb 26, 2020

pgayvallet mentioned this pull request Feb 26, 2020

[7.x] Allow savedObjects types registration from NP (#57430) #58602

Merged

pgayvallet mentioned this pull request Feb 27, 2020

block SO setup API calls after startup #58718

Merged

2 tasks

joshdover mentioned this pull request Feb 27, 2020

SavedObjectsTypeMappingDefinition should support the dynamic option #58783

Closed

lukeelmers mentioned this pull request Mar 1, 2020

[App Arch] Saved object migrations from kibana plugin to new platform #55946

Closed

6 tasks

alexwizp mentioned this pull request Mar 2, 2020

[App Arch] Saved object migrations from kibana plugin to new platform #59044

Merged

11 tasks

weltenwort mentioned this pull request Mar 3, 2020

[Logs / Metrics UI] [NP followup] Remove the legacy plugin #57999

Closed

spong mentioned this pull request Mar 3, 2020

[SIEM] Migrate to new platform #45831

Closed

26 tasks

cjcenizal mentioned this pull request Apr 23, 2020

ES UI new platform cleanup #64332

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow savedObjects types registration from NP #57430

Allow savedObjects types registration from NP #57430

pgayvallet commented Feb 12, 2020 •

edited

Loading

elasticmachine commented Feb 12, 2020

pgayvallet Feb 12, 2020

pgayvallet Feb 12, 2020

mshustov Feb 24, 2020

pgayvallet Feb 24, 2020

mshustov Feb 24, 2020

mshustov Feb 24, 2020 •

edited

Loading

pgayvallet Feb 24, 2020

mshustov Feb 24, 2020

pgayvallet commented Feb 24, 2020

jportner commented Feb 24, 2020

rudolf left a comment

rudolf Feb 25, 2020

rudolf Feb 25, 2020

rudolf Feb 25, 2020

rudolf Feb 25, 2020

pgayvallet Feb 25, 2020

rudolf Feb 25, 2020

rudolf Feb 25, 2020

rudolf Feb 25, 2020

rudolf Feb 25, 2020

rudolf Feb 25, 2020

rudolf Feb 25, 2020

rudolf left a comment

rudolf Feb 25, 2020

rudolf Feb 25, 2020

rudolf Feb 25, 2020

pgayvallet commented Feb 25, 2020

jportner commented Feb 25, 2020

kibanamachine commented Feb 26, 2020


		### Changes in structure compared to legacy

		The NP `registerType` expected input is very close to the legacy format, However there are some minor changes:

	it('migrates the migration to the new format', () => {
	it('converts the migration to the new format', () => {

	A migration function defined for a [saved objects type](./kibana-plugin-server.savedobjectstype.md) used to migrate it's to a given version
	A migration function for a [saved object type](./kibana-plugin-server.savedobjectstype.md) used to migrate it to a given version


		## Saved Objects types

		In the legacy platform, saved object types were registered using static definitions in `uiExports` part

	In legacy, is was `(doc: SavedObjectUnsanitizedDoc, log: SavedObjectsMigrationLogger) => SavedObjectUnsanitizedDoc;`
	In legacy, it was `(doc: SavedObjectUnsanitizedDoc, log: SavedObjectsMigrationLogger) => SavedObjectUnsanitizedDoc;`

	* // src/plugins/my_plugin/server/saved_objects/types.ts
	* // src/plugins/my_plugin/server/saved_objects/my_type.ts

	// src/plugins/my-plugin/server/saved_objects/first_type.ts
	// src/plugins/my_plugin/server/saved_objects/first_type.ts

	// src/legacy/core_plugins/my-plugin/migrations.js
	// src/legacy/core_plugins/my_plugin/migrations.js

	// src/legacy/core_plugins/my-plugin/mappings.json
	// src/legacy/core_plugins/my_plugin/mappings.json

	// src/legacy/core_plugins/my-plugin/index.js
	// src/legacy/core_plugins/my_plugin/index.js


		## SavedObjectTypeRegistry class

		Registry holding information about all the registered [savedObject types](./kibana-plugin-server.savedobjectstype.md)<!-- -->.

	* {@link SavedObjectsType \| savedObject types}
	* {@link SavedObjectsType \| saved object types}

	\| [SavedObjectTypeRegistry](./kibana-plugin-server.savedobjecttyperegistry.md) \| Registry holding information about all the registered [savedObject types](./kibana-plugin-server.savedobjectstype.md)<!-- -->. \|
	\| [SavedObjectTypeRegistry](./kibana-plugin-server.savedobjecttyperegistry.md) \| Registry holding information about all the registered [saved object types](./kibana-plugin-server.savedobjectstype.md)<!-- -->. \|

Allow savedObjects types registration from NP #57430

Allow savedObjects types registration from NP #57430

Conversation

pgayvallet commented Feb 12, 2020 • edited Loading

Summary

Checklist

For maintainers

Dev Docs

elasticmachine commented Feb 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mshustov Feb 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgayvallet commented Feb 24, 2020

jportner commented Feb 24, 2020

rudolf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rudolf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgayvallet commented Feb 25, 2020

jportner commented Feb 25, 2020

kibanamachine commented Feb 26, 2020

💚 Build Succeeded

History

pgayvallet commented Feb 12, 2020 •

edited

Loading

mshustov Feb 24, 2020 •

edited

Loading