From f96474e81bd47435c87971180a4edb9548d43bf2 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Mon, 13 Jul 2020 13:52:41 +0200 Subject: [PATCH 1/8] First stab at developer-focussed saved objects docs --- docs/developer/plugin-development.asciidoc | 5 +- .../development-plugin-saved-objects.asciidoc | 205 ++++++++++++++++++ 2 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 docs/developer/plugin/development-plugin-saved-objects.asciidoc diff --git a/docs/developer/plugin-development.asciidoc b/docs/developer/plugin-development.asciidoc index 691fdb0412fd2f..d9d6efa66352c3 100644 --- a/docs/developer/plugin-development.asciidoc +++ b/docs/developer/plugin-development.asciidoc @@ -3,7 +3,7 @@ [IMPORTANT] ============================================== -The Kibana plugin interfaces are in a state of constant development. We cannot provide backwards compatibility for plugins due to the high rate of change. Kibana enforces that the installed plugins match the version of Kibana itself. Plugin developers will have to release a new version of their plugin for each new Kibana release as a result. +The Kibana plugin interfaces are in a state of constant development. We cannot provide backwards compatibility for plugin due to the high rate of change. Kibana enforces that the installed plugins match the version of Kibana itself. Plugin developers will have to release a new version of their plugin for each new Kibana release as a result. ============================================== * <> @@ -11,6 +11,7 @@ The Kibana plugin interfaces are in a state of constant development. We cannot * <> * <> * <> +* <> include::plugin/development-plugin-resources.asciidoc[] @@ -22,3 +23,5 @@ include::plugin/development-plugin-functional-tests.asciidoc[] include::plugin/development-plugin-localization.asciidoc[] +include::plugin/development-plugin-saved-objects.asciidoc[] + diff --git a/docs/developer/plugin/development-plugin-saved-objects.asciidoc b/docs/developer/plugin/development-plugin-saved-objects.asciidoc new file mode 100644 index 00000000000000..ff1fdb6204985a --- /dev/null +++ b/docs/developer/plugin/development-plugin-saved-objects.asciidoc @@ -0,0 +1,205 @@ +[[development-plugin-saved-objects]] +=== Using Saved Objects + +Saved Objects allow Kibana plugins to use Elasticsearch like a primary +database. Think of it as an Object Document Mapper for Elasticsearch. Once a +plugin has registered one or more Saved Object types, the Saved Objects client +can be used to query or perform create, read, update and delete operations on +each type. + +By using Saved Objects your plugin can take advantage of the following +features: + +* Migrations can evolve your document's schema by transforming documents and +ensuring that the field mappings on the index are always up to date. +* An HTTP API is automatically exposed for each type (unless the type is +hidden). +* a Saved Objects client that can be used from both the server and the browser. +* Users can import or export Saved Objects using the Saved Objects management +UI or the Saved Objects import/export API. +* By declaring `references`, an object's entire reference graph will be +exported. This makes it easy for users to export e.g. a `dashboard` object and +have all the `visualization` objects required to display the dashboard +included in the export. +* When the X-Pack security and spaces plugins are enabled these transparently +provide RBAC access control and the ability to organize Saved Objects into +spaces. + +This document contains developer guidelines and best-practises for plugins +wanting to use Saved Objects. + +==== Registering a Saved Object type +Saved object type definitions should be defined in their own `my_plugin/server/saved_objects` directory. + +The folder should contain a file per type, named after the snake_case name of the type, and an `index.ts` file exporting all the types. + +src/plugins/my_plugin/server/saved_objects/dashboard_visualization.ts +[source,typescript] +---- +import { SavedObjectsType } from 'src/core/server'; + +export const dashboardVisualization: SavedObjectsType = { + name: 'dashboard_visualization', // <1> + hidden: false, + namespaceType: 'single', + mappings: { + dynamic: false, + properties: { + description: { + type: 'text', + }, + hits: { + type: 'integer', + }, + }, + }, + migrations: { + '1.0.0': migratedashboardVisualizationToV1, + '2.0.0': migratedashboardVisualizationToV2, + }, +}; +---- +<1> Since the name of a Saved Object type forms part of the url path for the +public Saved Objects HTTP API, these should follow our API URL path convention +and always be written as snake case. + +src/plugins/my_plugin/server/saved_objects/index.ts +[source,typescript] +---- +export { dashboardVisualization } from './dashboard_visualization'; +export { dashboard } from './dashboard'; +---- + +src/plugins/my_plugin/server/plugin.ts +[source,typescript] +---- +import { dashboard, dashboardVisualization } from './saved_objects'; + +export class MyPlugin implements Plugin { + setup({ savedObjects }) { + savedObjects.registerType(dashboard); + savedObjects.registerType(dashboardVisualization); + } +} +---- + +==== Mappings +Each Saved Object type can define it's own Elasticsearch field mappings. +Because multiple Saved Object types can share the same index, mappings defined +by a type will be nested under a top-level field that matches the type name. + +For example, the mappings defined by the `dashboard_visualization` Saved +Object type: + +src/plugins/my_plugin/server/saved_objects/dashboard_visualization.ts +[source,typescript] +---- +import { SavedObjectsType } from 'src/core/server'; + +export const dashboardVisualization: SavedObjectsType = { + name: 'dashboard_visualization', + ... + mappings: { + properties: { + dynamic: false, + description: { + type: 'text', + }, + hits: { + type: 'integer', + }, + }, + }, + migrations: { ... }, +}; +---- + +Will result in the following mappings being applied to the `.kibana` index: +[source,json] +---- +{ + "mappings": { + "dynamic": "strict", + "properties": { + ... + "dashboard_vizualization": { + "dynamic": false, + "properties": { + "description": { + "type": "text", + }, + "hits": { + "type": "integer", + }, + }, + } + } + } +} +---- + +Do not use field mappings like you would use data types for the columns of a +SQL database. Instead, field mappings are analogous to a SQL index. Only +specify field mappings for the fields you wish to search on or query. By +specifying `dynamic: false` in any level of your mappings, Elasticsearch will +accept and store any other fields even if they are not specified in your mappings. + +Since Elasticsearch has a default limit of 1000 fields per index, plugins +should carefully consider the fields they add to the mappings. Similarly, +Saved Object types should never use `dynamic: true` as this can cause an +arbitrary amount of fields to be added to the `.kibana` index. + +==== References +When a Saved Object declares `references` to other Saved Objects, the +Saved Objects Export API will automatically export the target object with all +of it's references. This makes it easy for users to export the entire +reference graph of an object. + +If a Saved Objects can't be used on it's own, that is, it needs other objects +to exist for a feature to function correctly, that Saved Object should declare +references to all the objects it requires. For example, a `dashboard` +object might have panels for several `visualization` objects. When these +`visualization` objects don't exist, the dashboard cannot be rendered +correctly. The `dashboard` object should declare references to all it's +visualizations. + +However, `visualization` objects can continue to be rendered or embedded into +other dashboards even if the `dashboard` it was originally embedded into +doesn't exist. As a result, `visualization` objects should not declare +references to `dashboard` objects. + +For each referenced object, an `id`, `type` and `name` are added to the +`references` array: + +[source, typescript] +---- +router.get( + { path: '/some-path', validate: false }, + async (context, req, res) => { + const object = await context.core.savedObjects.client.create( + 'dashboard', + { + title: 'my dashboard', + panels: [ + { visualization: 'vis1' }, // <1> + ], + indexPattern: 'indexPattern1' + }, + { references: [ + { id: '...', type: 'visualization', name: 'vis1' }, + { id: '...', type: 'index_pattern', name: 'indexPattern1' }, + ] + } + ) + ... + } +); +---- +<1> Note how `dashboard.panels[0].visualization` stores the `name` property of +the reference (not the `id` directly) to be able to uniquely identify this +reference. This guarantees that the id the reference points to always remains +up to date. If a visualization `id` was directly stored in +`dashboard.panels[0].visualization` there is a risk that this `id` gets +updated without updating the reference in the references array. + +// TODO: Writing migrations \ No newline at end of file From efeb2403ee6efdb9e9ac19de5dd60f17bc43be35 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Mon, 13 Jul 2020 14:10:45 +0200 Subject: [PATCH 2/8] Don't introduce spelling mistakes --- docs/developer/plugin-development.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer/plugin-development.asciidoc b/docs/developer/plugin-development.asciidoc index d9d6efa66352c3..c821563152ccc7 100644 --- a/docs/developer/plugin-development.asciidoc +++ b/docs/developer/plugin-development.asciidoc @@ -3,7 +3,7 @@ [IMPORTANT] ============================================== -The Kibana plugin interfaces are in a state of constant development. We cannot provide backwards compatibility for plugin due to the high rate of change. Kibana enforces that the installed plugins match the version of Kibana itself. Plugin developers will have to release a new version of their plugin for each new Kibana release as a result. +The Kibana plugin interfaces are in a state of constant development. We cannot provide backwards compatibility for plugins due to the high rate of change. Kibana enforces that the installed plugins match the version of Kibana itself. Plugin developers will have to release a new version of their plugin for each new Kibana release as a result. ============================================== * <> From a6fc33ff544bfe397fa302c4483a6328e97336e0 Mon Sep 17 00:00:00 2001 From: Josh Dover Date: Thu, 6 Aug 2020 11:46:48 -0600 Subject: [PATCH 3/8] Add docs for SO migrations --- .../development-plugin-saved-objects.asciidoc | 226 +++++++++++++++++- 1 file changed, 224 insertions(+), 2 deletions(-) diff --git a/docs/developer/architecture/development-plugin-saved-objects.asciidoc b/docs/developer/architecture/development-plugin-saved-objects.asciidoc index ff1fdb6204985a..5cd6aab9e81fab 100644 --- a/docs/developer/architecture/development-plugin-saved-objects.asciidoc +++ b/docs/developer/architecture/development-plugin-saved-objects.asciidoc @@ -25,7 +25,7 @@ included in the export. provide RBAC access control and the ability to organize Saved Objects into spaces. -This document contains developer guidelines and best-practises for plugins +This document contains developer guidelines and best-practices for plugins wanting to use Saved Objects. ==== Registering a Saved Object type @@ -202,4 +202,226 @@ up to date. If a visualization `id` was directly stored in `dashboard.panels[0].visualization` there is a risk that this `id` gets updated without updating the reference in the references array. -// TODO: Writing migrations \ No newline at end of file +==== Writing Migrations + +Saved Objects support schema changes between Kibana versions, which we call +migrations. Migrations are applied when a Kibana installation is upgraded from +one version to the next, when exports are imported via the Saved Objects +Management UI, or when a new object is created via the HTTP API. + +Each Saved Object type may define migrations for its schema. Migrations are +specified by the Kibana version number, receive an input document, and must +return the fully migrated document to be persisted to Elasticsearch. + +Let's say we want to define two migrations: +- In version 1.1.0, we want to drop the `subtitle` field and append it to the + title +- In version 1.4.0, we want to add a new `id` field to every panel with a newly + generated UUID. + +First, the current `mappings` should always reflect the latest or "target" +schema. Next, we should define a migration function for each step in the schema +evolution: + +src/plugins/my_plugin/server/saved_objects/dashboard_visualization.ts +[source,typescript] +---- +import { SavedObjectsType } from 'src/core/server'; + +export const dashboardVisualization: SavedObjectsType = { + name: 'dashboard_visualization', // <1> + /** ... */ + migrations: { + // Takes a pre 1.1.0 doc, and converts it to 1.1.0 + '1.1.0': (doc: DashboardVisualizationPre110): DashboardVisualization110 => { // <1> + return { + ...doc, // <2> + attributes: { + ...doc.attributes, + title: `${doc.attributes.title} - ${doc.attributes.subtitle}` + } + } + }, + + // Takes a 1.1.0 doc, and converts it to 1.4.0 + '1.4.0': (doc: DashboardVisualization110): DashboardVisualization140 => { // <3> + doc.attributes.panels = doc.attributes.panels.map(panel => { + panel.id = uuid.v4(); + return panel; + }); + return doc; + }, + }, +}; +---- +<1> It is useful to define an interface for each version of the schema. This +allows TypeScript to ensure that you are properly handling the input and output +types correctly as the schema evolves. +<2> Returning a shallow copy is necessary to avoid type errors when using +different types for the input and output shape. +<3> Migrations do not have to be defined for every version, only those in which +the schema needs to change. + +Migrations should be written defensively. If a document is encountered that is +not in the expected shape, migrations are encouraged to throw an exception to +abort the upgrade. + +===== Nested Migrations + +In some cases, objects may contain state that is functionally "owned" by other +plugins. An example is a dashboard that may contain state owned by specific +types of embeddables. In this case, the dashboard migrations should delegate +migrating this state to their functional owner, the individual embeddable types, +in order to compose a single migration function that handles all nested state. + +How the migration of the nested object is surfaced to the containing object is +up to plugin authors to define. In general, we encourage that registries that +expose state that may be persisted elsewhere include migrations as part of the +interface that is registered. This allows consumers of the registry items to +utilize these migrations on the nested pieces of state inside of the root +migration. + +To demonstrate this, let's imagine we have: +- A chart registry plugin which allows plugins to register multiple types of charts +- A bar chart plugin that implements and registers a type of chart +- A chart list plugin that contains a list of charts to display. This plugin +persists the state of the underlying charts and allows those charts to define +migrations for that state. + +The example code here is length, but necessary to demonstrate how this different +pieces fit together. + +src/plugins/dashboard/server/saved_objects/dashboard.ts +[source,typescript] +---- +type ChartMigrationFn = (chartDoc: object) => ChartState; + +interface Chart { + type: string; + render(chart: ChartState, element: HTMLElement): void; + create(): ChartState; + getMigrations(): Record>; +} + +class ChartRegistryPlugin { + private readonly charts = new Map(); + public setup() { + return { + register(chart: Chart) { + charts.set(chart.id, chart); + }, + + /** Returns migrations by version that can handle migrating a chart of any type */ + getMigrations() { + // Here we rollup the migrations from each individual chart implementation to create a single migration function + // for each version that will the chart state migration if the input chart is of the same type. + const chartMigrations = this.charts.reduce((migrations, chart) => { + for (const [version, chartMigration] of Object.entries(chart.getMigrations)) { + const existingMigration = migrations[version] ?? (input: any) => input; + migrations[version] = (chartDoc) => { + if (chartDoc.type === chart.type) { + chartDoc = chartMigration(chartDoc); + } + + return existingMigration(chartDoc); + } + } + }, {} as Record); + + return { + '1.1.0': (oldChart) => { + return { + ...oldChart, + // Apply the migrations on the chart state if and only if any charts define migrations for this version. + // It's important that the registry items can only access the `state` part of the complete chart object. + // This is the part of the document that the chart implementation functionally "owns". + state: chartMigrations['1.1.0'] ? chartMigrations['1.1.0'](oldChart.chartState) : oldChart.chartState + } + } + } + } + } + } +} + +interface BarChartState100 { + xAxis: string; + yAxis: string; + indexPattern: string; +} + +interface BarChartState110 { + xAxis: string; + yAxis: string; + dataSource: { + indexPattern: string; + } +} + +class BarChartPlugin { + public setup(core, plugins) { + plugins.charts.register({ + type: 'bar-chart', + render() { ... }, + create() { return { xAxis: 'foo', yAxis: 'bar', dataSource: { indexPattern: 'baz' } }}, + getMigrations() { + return { + '1.1.0': (oldChartState: BarChartState100): BarChartState110 => ({ + xAxis: oldChartState.xAxis, + yAxis: oldChartState.yAxis, + dataSource: { indexPattern: oldChartState.dataSource } + }) + } + } + }) + } +} + +class ChartListPlugin { + public setup(core, plugins) { + core.savedObjects.registerType({ + name: 'chart-list', + hidden: false, + mappings: { + dynamic: false, + properties: { + title: { + type: 'text', + }, + // We will store charts as an array, no need to index this field. + charts: { + index: false; + } + }, + }, + getMigrations() { + // Request the migrations for the chart state from the charts registry + const chartMigrations = plugins.charts.getMigrations(); + + return { + '1.1.0': (chartList) => { + return { + ...chartList, + // For each chart, apply the chart migration for this version if it exists + charts: chartList.map( + chart => chartMigrations['1.1.0'] ? chartMigrations['1.1.0'](chart) : chart + ) + } + } + } + } + }) + } +} +---- + +For this to all work, a few things must be true: +- Nested state that comes from pluggable sources must be isolated within the +containing object's schema. In this example, the state is isolated within the +chart's `state` field. +- Pluggable sources must provide their migrations to the registry. +- Containing objects must have a mechanism for locating these migrations. + +What about nested nested nested objects? It's turtles all the way down! These +migrations should all follow the same pattern to compose the complete migration +from migrations provided by their functional owners. From 669eb5527aa92f5e5ed41d6f4d0425feb559203b Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Tue, 15 Sep 2020 09:22:20 +0200 Subject: [PATCH 4/8] Link to HTTP API documentation --- .../architecture/development-plugin-saved-objects.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/developer/architecture/development-plugin-saved-objects.asciidoc b/docs/developer/architecture/development-plugin-saved-objects.asciidoc index 95a55e034afbc6..2e59f7874ab9f7 100644 --- a/docs/developer/architecture/development-plugin-saved-objects.asciidoc +++ b/docs/developer/architecture/development-plugin-saved-objects.asciidoc @@ -12,8 +12,8 @@ features: * Migrations can evolve your document's schema by transforming documents and ensuring that the field mappings on the index are always up to date. -* An HTTP API is automatically exposed for each type (unless the type is -hidden). +* An <> is automatically exposed for each type (unless +`hidden=true` is specified). * a Saved Objects client that can be used from both the server and the browser. * Users can import or export Saved Objects using the Saved Objects management UI or the Saved Objects import/export API. From 1923224f474d46027a9579a7fc33e4581b0f7fc2 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Tue, 15 Sep 2020 09:25:42 +0200 Subject: [PATCH 5/8] Grammar fixes --- .../architecture/development-plugin-saved-objects.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer/architecture/development-plugin-saved-objects.asciidoc b/docs/developer/architecture/development-plugin-saved-objects.asciidoc index 2e59f7874ab9f7..d0b0000d3e63f6 100644 --- a/docs/developer/architecture/development-plugin-saved-objects.asciidoc +++ b/docs/developer/architecture/development-plugin-saved-objects.asciidoc @@ -12,7 +12,7 @@ features: * Migrations can evolve your document's schema by transforming documents and ensuring that the field mappings on the index are always up to date. -* An <> is automatically exposed for each type (unless +* a <> is automatically exposed for each type (unless `hidden=true` is specified). * a Saved Objects client that can be used from both the server and the browser. * Users can import or export Saved Objects using the Saved Objects management From 915036d0406ef370ed1466bc6397d047ca026775 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Tue, 15 Sep 2020 12:02:35 +0200 Subject: [PATCH 6/8] Rendering fixes --- .../development-plugin-saved-objects.asciidoc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/developer/architecture/development-plugin-saved-objects.asciidoc b/docs/developer/architecture/development-plugin-saved-objects.asciidoc index d0b0000d3e63f6..85b1e08ded09c0 100644 --- a/docs/developer/architecture/development-plugin-saved-objects.asciidoc +++ b/docs/developer/architecture/development-plugin-saved-objects.asciidoc @@ -33,7 +33,7 @@ Saved object type definitions should be defined in their own `my_plugin/server/s The folder should contain a file per type, named after the snake_case name of the type, and an `index.ts` file exporting all the types. -src/plugins/my_plugin/server/saved_objects/dashboard_visualization.ts +.src/plugins/my_plugin/server/saved_objects/dashboard_visualization.ts [source,typescript] ---- import { SavedObjectsType } from 'src/core/server'; @@ -63,14 +63,14 @@ export const dashboardVisualization: SavedObjectsType = { public Saved Objects HTTP API, these should follow our API URL path convention and always be written as snake case. -src/plugins/my_plugin/server/saved_objects/index.ts +.src/plugins/my_plugin/server/saved_objects/index.ts [source,typescript] ---- export { dashboardVisualization } from './dashboard_visualization'; export { dashboard } from './dashboard'; ---- -src/plugins/my_plugin/server/plugin.ts +.src/plugins/my_plugin/server/plugin.ts [source,typescript] ---- import { dashboard, dashboardVisualization } from './saved_objects'; @@ -91,7 +91,7 @@ by a type will be nested under a top-level field that matches the type name. For example, the mappings defined by the `dashboard_visualization` Saved Object type: -src/plugins/my_plugin/server/saved_objects/dashboard_visualization.ts +.src/plugins/my_plugin/server/saved_objects/dashboard_visualization.ts [source,typescript] ---- import { SavedObjectsType } from 'src/core/server'; @@ -155,7 +155,7 @@ Saved Objects Export API will automatically export the target object with all of it's references. This makes it easy for users to export the entire reference graph of an object. -If a Saved Objects can't be used on it's own, that is, it needs other objects +If a Saved Object can't be used on it's own, that is, it needs other objects to exist for a feature to function correctly, that Saved Object should declare references to all the objects it requires. For example, a `dashboard` object might have panels for several `visualization` objects. When these From 513cd178635489be6be791c7573a861dc354c27f Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Wed, 23 Sep 2020 15:53:05 +0200 Subject: [PATCH 7/8] Migrations should be tested, remove nested migration docs for now --- .../development-plugin-saved-objects.asciidoc | 164 +----------------- 1 file changed, 3 insertions(+), 161 deletions(-) diff --git a/docs/developer/architecture/development-plugin-saved-objects.asciidoc b/docs/developer/architecture/development-plugin-saved-objects.asciidoc index 183dbd77796605..c3fa771e946127 100644 --- a/docs/developer/architecture/development-plugin-saved-objects.asciidoc +++ b/docs/developer/architecture/development-plugin-saved-objects.asciidoc @@ -264,164 +264,6 @@ the schema needs to change. Migrations should be written defensively. If a document is encountered that is not in the expected shape, migrations are encouraged to throw an exception to -abort the upgrade. - -===== Nested Migrations - -In some cases, objects may contain state that is functionally "owned" by other -plugins. An example is a dashboard that may contain state owned by specific -types of embeddables. In this case, the dashboard migrations should delegate -migrating this state to their functional owner, the individual embeddable types, -in order to compose a single migration function that handles all nested state. - -How the migration of the nested object is surfaced to the containing object is -up to plugin authors to define. In general, we encourage that registries that -expose state that may be persisted elsewhere include migrations as part of the -interface that is registered. This allows consumers of the registry items to -utilize these migrations on the nested pieces of state inside of the root -migration. - -To demonstrate this, let's imagine we have: -- A chart registry plugin which allows plugins to register multiple types of charts -- A bar chart plugin that implements and registers a type of chart -- A chart list plugin that contains a list of charts to display. This plugin -persists the state of the underlying charts and allows those charts to define -migrations for that state. - -The example code here is length, but necessary to demonstrate how this different -pieces fit together. - -src/plugins/dashboard/server/saved_objects/dashboard.ts -[source,typescript] ----- -type ChartMigrationFn = (chartDoc: object) => ChartState; - -interface Chart { - type: string; - render(chart: ChartState, element: HTMLElement): void; - create(): ChartState; - getMigrations(): Record>; -} - -class ChartRegistryPlugin { - private readonly charts = new Map(); - public setup() { - return { - register(chart: Chart) { - charts.set(chart.id, chart); - }, - - /** Returns migrations by version that can handle migrating a chart of any type */ - getMigrations() { - // Here we rollup the migrations from each individual chart implementation to create a single migration function - // for each version that will the chart state migration if the input chart is of the same type. - const chartMigrations = this.charts.reduce((migrations, chart) => { - for (const [version, chartMigration] of Object.entries(chart.getMigrations)) { - const existingMigration = migrations[version] ?? (input: any) => input; - migrations[version] = (chartDoc) => { - if (chartDoc.type === chart.type) { - chartDoc = chartMigration(chartDoc); - } - - return existingMigration(chartDoc); - } - } - }, {} as Record); - - return { - '1.1.0': (oldChart) => { - return { - ...oldChart, - // Apply the migrations on the chart state if and only if any charts define migrations for this version. - // It's important that the registry items can only access the `state` part of the complete chart object. - // This is the part of the document that the chart implementation functionally "owns". - state: chartMigrations['1.1.0'] ? chartMigrations['1.1.0'](oldChart.chartState) : oldChart.chartState - } - } - } - } - } - } -} - -interface BarChartState100 { - xAxis: string; - yAxis: string; - indexPattern: string; -} - -interface BarChartState110 { - xAxis: string; - yAxis: string; - dataSource: { - indexPattern: string; - } -} - -class BarChartPlugin { - public setup(core, plugins) { - plugins.charts.register({ - type: 'bar-chart', - render() { ... }, - create() { return { xAxis: 'foo', yAxis: 'bar', dataSource: { indexPattern: 'baz' } }}, - getMigrations() { - return { - '1.1.0': (oldChartState: BarChartState100): BarChartState110 => ({ - xAxis: oldChartState.xAxis, - yAxis: oldChartState.yAxis, - dataSource: { indexPattern: oldChartState.dataSource } - }) - } - } - }) - } -} - -class ChartListPlugin { - public setup(core, plugins) { - core.savedObjects.registerType({ - name: 'chart-list', - hidden: false, - mappings: { - dynamic: false, - properties: { - title: { - type: 'text', - }, - // We will store charts as an array, no need to index this field. - charts: { - index: false; - } - }, - }, - getMigrations() { - // Request the migrations for the chart state from the charts registry - const chartMigrations = plugins.charts.getMigrations(); - - return { - '1.1.0': (chartList) => { - return { - ...chartList, - // For each chart, apply the chart migration for this version if it exists - charts: chartList.map( - chart => chartMigrations['1.1.0'] ? chartMigrations['1.1.0'](chart) : chart - ) - } - } - } - } - }) - } -} ----- - -For this to all work, a few things must be true: -- Nested state that comes from pluggable sources must be isolated within the -containing object's schema. In this example, the state is isolated within the -chart's `state` field. -- Pluggable sources must provide their migrations to the registry. -- Containing objects must have a mechanism for locating these migrations. - -What about nested nested nested objects? It's turtles all the way down! These -migrations should all follow the same pattern to compose the complete migration -from migrations provided by their functional owners. +abort the upgrade. A bug in a migration will cause downtime for our users, it +is critical that you have extensive tests to ensure that migrations behave as +expected with all possible input documents. \ No newline at end of file From 77e4af999a92890d1dceb683ee2207ba18a11678 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Mon, 28 Sep 2020 13:22:28 +0200 Subject: [PATCH 8/8] Drop subtitle field in migration, add notes about migration version, behaviour for corrupt documents and emphasize testing --- .../development-plugin-saved-objects.asciidoc | 92 ++++++++++++++----- 1 file changed, 67 insertions(+), 25 deletions(-) diff --git a/docs/developer/architecture/development-plugin-saved-objects.asciidoc b/docs/developer/architecture/development-plugin-saved-objects.asciidoc index c3fa771e946127..0d31f5d90f6680 100644 --- a/docs/developer/architecture/development-plugin-saved-objects.asciidoc +++ b/docs/developer/architecture/development-plugin-saved-objects.asciidoc @@ -226,31 +226,63 @@ evolution: src/plugins/my_plugin/server/saved_objects/dashboard_visualization.ts [source,typescript] ---- -import { SavedObjectsType } from 'src/core/server'; +import { SavedObjectsType, SavedObjectMigrationFn } from 'src/core/server'; +import uuid from 'uuid'; + +interface DashboardVisualizationPre110 { + title: string; + subtitle: string; + panels: Array<{}>; +} +interface DashboardVisualization110 { + title: string; + panels: Array<{}>; +} + +interface DashboardVisualization140 { + title: string; + panels: Array<{ id: string }>; +} + +const migrateDashboardVisualization110: SavedObjectMigrationFn< + DashboardVisualizationPre110, // <1> + DashboardVisualization110 +> = (doc) => { + const { subtitle, ...attributesWithoutSubtitle } = doc.attributes; + return { + ...doc, // <2> + attributes: { + ...attributesWithoutSubtitle, + title: `${doc.attributes.title} - ${doc.attributes.subtitle}`, + }, + }; +}; + +const migrateDashboardVisualization140: SavedObjectMigrationFn< + DashboardVisualization110, + DashboardVisualization140 +> = (doc) => { + const outPanels = doc.attributes.panels?.map((panel) => { + return { ...panel, id: uuid.v4() }; + }); + return { + ...doc, + attributes: { + ...doc.attributes, + panels: outPanels, + }, + }; +}; export const dashboardVisualization: SavedObjectsType = { name: 'dashboard_visualization', // <1> /** ... */ migrations: { // Takes a pre 1.1.0 doc, and converts it to 1.1.0 - '1.1.0': (doc: DashboardVisualizationPre110): DashboardVisualization110 => { // <1> - return { - ...doc, // <2> - attributes: { - ...doc.attributes, - title: `${doc.attributes.title} - ${doc.attributes.subtitle}` - } - } - }, + '1.1.0': migrateDashboardVisualization110, // Takes a 1.1.0 doc, and converts it to 1.4.0 - '1.4.0': (doc: DashboardVisualization110): DashboardVisualization140 => { // <3> - doc.attributes.panels = doc.attributes.panels.map(panel => { - panel.id = uuid.v4(); - return panel; - }); - return doc; - }, + '1.4.0': migrateDashboardVisualization140, // <3> }, }; ---- @@ -259,11 +291,21 @@ allows TypeScript to ensure that you are properly handling the input and output types correctly as the schema evolves. <2> Returning a shallow copy is necessary to avoid type errors when using different types for the input and output shape. -<3> Migrations do not have to be defined for every version, only those in which -the schema needs to change. - -Migrations should be written defensively. If a document is encountered that is -not in the expected shape, migrations are encouraged to throw an exception to -abort the upgrade. A bug in a migration will cause downtime for our users, it -is critical that you have extensive tests to ensure that migrations behave as -expected with all possible input documents. \ No newline at end of file +<3> Migrations do not have to be defined for every version. The version number +of a migration must always be the earliest Kibana version in which this +migration was released. So if you are creating a migration which will be +part of the v7.10.0 release, but will also be backported and released as +v7.9.3, the migration version should be: 7.9.3. + +Migrations should be written defensively, an exception in a migration function +will prevent a Kibana upgrade from succeeding and will cause downtime for our +users. Having said that, if a document is encountered that is not in the +expected shape, migrations are encouraged to throw an exception to abort the +upgrade. In most scenarios, it is better to fail an upgrade than to silently +ignore a corrupt document which can cause unexpected behaviour at some future +point in time. + +It is critical that you have extensive tests to ensure that migrations behave +as expected with all possible input documents. Given how simple it is to test +all the branch conditions in a migration function and the high impact of a bug +in this code, there's really no reason not to aim for 100% test code coverage. \ No newline at end of file