From 8c79e4fc5de72050bb043b300f59138ec9188574 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 29 Mar 2023 10:33:07 +0200 Subject: [PATCH 01/14] plug downward transform from model versions and add some tests around pipeline --- .../document_migrator/document_migrator.ts | 16 +- .../document_migrator_pipeline.test.ts | 254 ++++++++++++++++++ .../document_migrator_pipeline.ts | 38 ++- .../document_migrator/model_version.test.ts | 159 ++++++++--- .../src/document_migrator/model_version.ts | 15 +- .../src/document_migrator/types.ts | 8 +- 6 files changed, 422 insertions(+), 68 deletions(-) create mode 100644 packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts index 9da59cba4dc10..7f9b58affa067 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts @@ -145,16 +145,12 @@ export class DocumentMigrator implements VersionedTransformer { throw new Error('Migrations are not ready. Make sure prepareMigrations is called first.'); } - // Clone the document to prevent accidental mutations on the original data - // Ex: Importing sample data that is cached at import level, migrations would - // execute on mutated data the second time. - const clonedDoc = _.cloneDeep(doc); - const pipeline = new DocumentMigratorPipeline( - clonedDoc, - this.migrations, - this.documentMigratorOptions.kibanaVersion, - convertNamespaceTypes - ); + const pipeline = new DocumentMigratorPipeline({ + document: doc, + migrations: this.migrations, + kibanaVersion: this.documentMigratorOptions.kibanaVersion, + convertNamespaceTypes, + }); pipeline.run(); const { document, additionalDocs } = pipeline; diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts new file mode 100644 index 0000000000000..0f3be26caa889 --- /dev/null +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts @@ -0,0 +1,254 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import _ from 'lodash'; +import type { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server'; +import { + modelVersionVirtualMajor, + modelVersionToVirtualVersion, +} from '@kbn/core-saved-objects-base-server-internal'; +import { Transform, TransformType, TypeTransforms, TransformFn } from './types'; +import { DocumentMigratorPipeline } from './document_migrator_pipeline'; + +// snake case is way better for migration function names in this very specific scenario. +/* eslint-disable @typescript-eslint/naming-convention */ + +describe('DocumentMigratorPipeline', () => { + const defaultKibanaVersion = '8.8.0'; + + const createDoc = ( + parts: Partial> = {} + ): SavedObjectUnsanitizedDoc => ({ + id: 'test-doc', + type: 'test-type', + attributes: {}, + references: [], + coreMigrationVersion: defaultKibanaVersion, + ...parts, + }); + + const latestVersions = ( + parts: Partial> = {} + ): Record => ({ + [TransformType.Convert]: defaultKibanaVersion, + [TransformType.Migrate]: defaultKibanaVersion, + [TransformType.Core]: defaultKibanaVersion, + [TransformType.Reference]: defaultKibanaVersion, + ...parts, + }); + + const getTypeTransforms = (transforms: Transform[]): TypeTransforms => { + const versions = _.chain(transforms) + .groupBy('transformType') + .mapValues((items) => _.last(items)?.version) + .value() as Record; + + return { + transforms, + latestVersion: latestVersions(versions), + }; + }; + + const createTransformFn = (impl?: TransformFn): jest.MockedFunction => { + const defaultImpl: TransformFn = (doc) => ({ transformedDoc: doc, additionalDocs: [] }); + return jest.fn().mockImplementation(impl ?? defaultImpl); + }; + + describe('upward conversions', () => { + it('calls a single `Migrate` conversion function', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + typeMigrationVersion: '8.5.0', + }); + + const migrate8_8_0_up = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { transformType: TransformType.Migrate, version: '8.8.0', transform: migrate8_8_0_up }, + ]); + + const pipeline = new DocumentMigratorPipeline({ + document, + kibanaVersion: '8.8.0', + convertNamespaceTypes: false, + migrations: { + foo: fooTransforms, + }, + }); + + pipeline.run(); + + expect(migrate8_8_0_up).toHaveBeenCalledTimes(1); + expect(migrate8_8_0_up).toHaveBeenCalledWith(document); + + const outputDoc = pipeline.document; + expect(outputDoc.typeMigrationVersion).toEqual('8.8.0'); + }); + + it('calls multiple `Migrate` conversion function in order', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + typeMigrationVersion: '8.5.0', + }); + + const migrate8_6_0_up = createTransformFn(); + const migrate8_7_0_up = createTransformFn(); + const migrate8_8_0_up = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { transformType: TransformType.Migrate, version: '8.6.0', transform: migrate8_6_0_up }, + { transformType: TransformType.Migrate, version: '8.7.0', transform: migrate8_7_0_up }, + { transformType: TransformType.Migrate, version: '8.8.0', transform: migrate8_8_0_up }, + ]); + + const pipeline = new DocumentMigratorPipeline({ + document, + kibanaVersion: '8.8.0', + convertNamespaceTypes: false, + migrations: { + foo: fooTransforms, + }, + }); + + pipeline.run(); + + expect(migrate8_6_0_up).toHaveBeenCalledTimes(1); + expect(migrate8_6_0_up).toHaveBeenCalledWith(document); + + expect(migrate8_7_0_up).toHaveBeenCalledTimes(1); + expect(migrate8_7_0_up).toHaveBeenCalledWith({ ...document, typeMigrationVersion: '8.6.0' }); + + expect(migrate8_8_0_up).toHaveBeenCalledTimes(1); + expect(migrate8_8_0_up).toHaveBeenCalledWith({ ...document, typeMigrationVersion: '8.7.0' }); + + expect(migrate8_6_0_up.mock.invocationCallOrder[0]).toBeLessThan( + migrate8_7_0_up.mock.invocationCallOrder[0] + ); + expect(migrate8_7_0_up.mock.invocationCallOrder[0]).toBeLessThan( + migrate8_8_0_up.mock.invocationCallOrder[0] + ); + + const outputDoc = pipeline.document; + expect(outputDoc.typeMigrationVersion).toEqual('8.8.0'); + }); + + it('skips `Migrate` conversion function lower or equal to the document version', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + typeMigrationVersion: '8.7.0', + }); + + const migrate8_6_0_up = createTransformFn(); + const migrate8_7_0_up = createTransformFn(); + const migrate8_8_0_up = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { transformType: TransformType.Migrate, version: '8.6.0', transform: migrate8_6_0_up }, + { transformType: TransformType.Migrate, version: '8.7.0', transform: migrate8_7_0_up }, + { transformType: TransformType.Migrate, version: '8.8.0', transform: migrate8_8_0_up }, + ]); + + const pipeline = new DocumentMigratorPipeline({ + document, + kibanaVersion: '8.8.0', + convertNamespaceTypes: false, + migrations: { + foo: fooTransforms, + }, + }); + + pipeline.run(); + + expect(migrate8_6_0_up).not.toHaveBeenCalled(); + + expect(migrate8_7_0_up).not.toHaveBeenCalled(); + + expect(migrate8_8_0_up).toHaveBeenCalledTimes(1); + expect(migrate8_8_0_up).toHaveBeenCalledWith({ ...document, typeMigrationVersion: '8.7.0' }); + + const outputDoc = pipeline.document; + expect(outputDoc.typeMigrationVersion).toEqual('8.8.0'); + }); + + it('throws an error when trying to convert a document from a higher version', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + typeMigrationVersion: '8.9.0', + }); + + const migrate8_8_0_up = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { transformType: TransformType.Migrate, version: '8.8.0', transform: migrate8_8_0_up }, + ]); + + const pipeline = new DocumentMigratorPipeline({ + document, + kibanaVersion: '8.8.0', + convertNamespaceTypes: false, + migrations: { + foo: fooTransforms, + }, + }); + + expect(() => pipeline.run()).toThrowErrorMatchingInlineSnapshot( + `"Document \\"foo-1\\" belongs to a more recent version of Kibana [8.9.0] when the last known version is [8.8.0]."` + ); + }); + + it('do not throw when converting a document to a virtual model version', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + typeMigrationVersion: '8.6.0', + }); + + const migrate8_8_0_up = createTransformFn(); + + const virtualModelVersion_3 = modelVersionToVirtualVersion(3); + const migrate_mv_3 = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { transformType: TransformType.Migrate, version: '8.8.0', transform: migrate8_8_0_up }, + { + transformType: TransformType.Migrate, + version: virtualModelVersion_3, + transform: migrate_mv_3, + }, + ]); + + const pipeline = new DocumentMigratorPipeline({ + document, + kibanaVersion: '8.8.0', + convertNamespaceTypes: false, + migrations: { + foo: fooTransforms, + }, + }); + + pipeline.run(); + + expect(migrate8_8_0_up).toHaveBeenCalledTimes(1); + expect(migrate8_8_0_up).toHaveBeenCalledWith({ ...document }); + + expect(migrate_mv_3).toHaveBeenCalledTimes(1); + expect(migrate_mv_3).toHaveBeenCalledWith({ ...document, typeMigrationVersion: '8.8.0' }); + + expect(migrate8_8_0_up.mock.invocationCallOrder[0]).toBeLessThan( + migrate_mv_3.mock.invocationCallOrder[0] + ); + + const outputDoc = pipeline.document; + expect(outputDoc.typeMigrationVersion).toEqual(virtualModelVersion_3); + }); + }); +}); diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts index 2eaa936428b92..714fa4cf49c94 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts @@ -7,6 +7,7 @@ */ import Boom from '@hapi/boom'; +import { cloneDeep } from 'lodash'; import Semver from 'semver'; import type { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server'; import { ActiveMigrations, Transform, TransformType } from './types'; @@ -17,23 +18,40 @@ function isGreater(a?: string, b?: string) { } export class DocumentMigratorPipeline { - additionalDocs = [] as SavedObjectUnsanitizedDoc[]; - - constructor( - public document: SavedObjectUnsanitizedDoc, - private migrations: ActiveMigrations, - private kibanaVersion: string, - private convertNamespaceTypes: boolean - ) {} + public additionalDocs = [] as SavedObjectUnsanitizedDoc[]; + public document: SavedObjectUnsanitizedDoc; + + private originalDoc: SavedObjectUnsanitizedDoc; + private migrations: ActiveMigrations; + private kibanaVersion: string; + private convertNamespaceTypes: boolean; + + constructor({ + document, + migrations, + kibanaVersion, + convertNamespaceTypes, + }: { + document: SavedObjectUnsanitizedDoc; + migrations: ActiveMigrations; + kibanaVersion: string; + convertNamespaceTypes: boolean; + }) { + this.originalDoc = document; + this.document = cloneDeep(document); + this.migrations = migrations; + this.kibanaVersion = kibanaVersion; + this.convertNamespaceTypes = convertNamespaceTypes; + } protected *getPipeline(): Generator { while (this.hasPendingTransforms()) { - const { type } = this.document; + const { type: previousType } = this.document; for (const transform of this.getPendingTransforms()) { yield transform; - if (type !== this.document.type) { + if (this.document.type !== previousType) { // In the initial implementation, all the transforms for the new type should be applied. // And at the same time, documents with `undefined` in `typeMigrationVersion` are treated as the most recent ones. // This is a workaround to get into the loop again and apply all the migrations for the new type. diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.test.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.test.ts index c32d18463083c..a5a5e578a0b2e 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.test.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.test.ts @@ -23,6 +23,7 @@ describe('getModelVersionTransforms', () => { version, transformType: type, transform: expect.any(Function), + transformDown: expect.any(Function), }); const createType = (parts: Partial): SavedObjectsType => ({ @@ -123,65 +124,135 @@ describe('convertModelVersionTransformFn', () => { })); }; - it('generates a transform function calling the model transform', () => { - const upTransform = createModelTransformFn(); - const downTransform = createModelTransformFn(); + describe('up transformation', () => { + it('generates a transform function calling the model transform', () => { + const upTransform = createModelTransformFn(); + const downTransform = createModelTransformFn(); - const definition: SavedObjectsModelVersion = { - modelChange: { - type: 'expansion', - transformation: { up: upTransform, down: downTransform }, - }, - }; + const definition: SavedObjectsModelVersion = { + modelChange: { + type: 'expansion', + transformation: { up: upTransform, down: downTransform }, + }, + }; + + const transform = convertModelVersionTransformFn({ + log, + modelVersion: 1, + virtualVersion: '10.1.0', + definition, + type: 'up', + }); + + expect(upTransform).not.toHaveBeenCalled(); + expect(downTransform).not.toHaveBeenCalled(); + + const doc = createDoc(); + const context = { log, modelVersion: 1 }; + + transform(doc); - const transform = convertModelVersionTransformFn({ - log, - modelVersion: 1, - virtualVersion: '10.1.0', - definition, + expect(upTransform).toHaveBeenCalledTimes(1); + expect(downTransform).not.toHaveBeenCalled(); + expect(upTransform).toHaveBeenCalledWith(doc, context); }); - expect(upTransform).not.toHaveBeenCalled(); - expect(downTransform).not.toHaveBeenCalled(); + it('returns the document from the model transform', () => { + const upTransform = createModelTransformFn(); - const doc = createDoc(); - const context = { log, modelVersion: 1 }; + const resultDoc = createDoc(); + upTransform.mockImplementation((doc) => { + return { document: resultDoc }; + }); - transform(doc); + const definition: SavedObjectsModelVersion = { + modelChange: { + type: 'expansion', + transformation: { up: upTransform, down: jest.fn() }, + }, + }; - expect(upTransform).toHaveBeenCalledTimes(1); - expect(downTransform).not.toHaveBeenCalled(); - expect(upTransform).toHaveBeenCalledWith(doc, context); - }); + const transform = convertModelVersionTransformFn({ + log, + modelVersion: 1, + virtualVersion: '10.1.0', + definition, + type: 'up', + }); - it('returns the document from the model transform', () => { - const upTransform = createModelTransformFn(); + const doc = createDoc(); - const resultDoc = createDoc(); - upTransform.mockImplementation((doc) => { - return { document: resultDoc }; + const result = transform(doc); + expect(result).toEqual({ + transformedDoc: resultDoc, + additionalDocs: [], + }); }); + }); - const definition: SavedObjectsModelVersion = { - modelChange: { - type: 'expansion', - transformation: { up: upTransform, down: jest.fn() }, - }, - }; + describe('down transformation', () => { + it('generates a transform function calling the model transform', () => { + const upTransform = createModelTransformFn(); + const downTransform = createModelTransformFn(); - const transform = convertModelVersionTransformFn({ - log, - modelVersion: 1, - virtualVersion: '10.1.0', - definition, + const definition: SavedObjectsModelVersion = { + modelChange: { + type: 'expansion', + transformation: { up: upTransform, down: downTransform }, + }, + }; + + const transform = convertModelVersionTransformFn({ + log, + modelVersion: 1, + virtualVersion: '10.1.0', + definition, + type: 'down', + }); + + expect(upTransform).not.toHaveBeenCalled(); + expect(downTransform).not.toHaveBeenCalled(); + + const doc = createDoc(); + const context = { log, modelVersion: 1 }; + + transform(doc); + + expect(upTransform).not.toHaveBeenCalled(); + expect(downTransform).toHaveBeenCalledTimes(1); + expect(downTransform).toHaveBeenCalledWith(doc, context); }); - const doc = createDoc(); + it('returns the document from the model transform', () => { + const downTransform = createModelTransformFn(); + + const resultDoc = createDoc(); + downTransform.mockImplementation((doc) => { + return { document: resultDoc }; + }); + + const definition: SavedObjectsModelVersion = { + modelChange: { + type: 'expansion', + transformation: { up: jest.fn(), down: downTransform }, + }, + }; + + const transform = convertModelVersionTransformFn({ + log, + modelVersion: 1, + virtualVersion: '10.1.0', + definition, + type: 'down', + }); + + const doc = createDoc(); - const result = transform(doc); - expect(result).toEqual({ - transformedDoc: resultDoc, - additionalDocs: [], + const result = transform(doc); + expect(result).toEqual({ + transformedDoc: resultDoc, + additionalDocs: [], + }); }); }); }); diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.ts index cf6f14e2dc83a..40d08022a2099 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.ts @@ -44,6 +44,14 @@ export const getModelVersionTransforms = ({ modelVersion, virtualVersion, definition, + type: 'up', + }), + transformDown: convertModelVersionTransformFn({ + log, + modelVersion, + virtualVersion, + definition, + type: 'down', }), transformType: TransformType.Migrate, }; @@ -56,11 +64,13 @@ export const convertModelVersionTransformFn = ({ virtualVersion, modelVersion, definition, + type, log, }: { virtualVersion: string; modelVersion: number; definition: SavedObjectsModelVersion; + type: 'up' | 'down'; log: Logger; }): TransformFn => { if (!definition.modelChange.transformation) { @@ -70,7 +80,10 @@ export const convertModelVersionTransformFn = ({ log, modelVersion, }; - const modelTransformFn = definition.modelChange.transformation.up; + const modelTransformFn = + type === 'up' + ? definition.modelChange.transformation.up + : definition.modelChange.transformation.down; return function convertedTransform(doc: SavedObjectUnsanitizedDoc) { try { diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/types.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/types.ts index 84fe0f36532cf..257e7bf02fe1b 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/types.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/types.ts @@ -29,12 +29,14 @@ export interface TypeTransforms { * Internal representation of a document transformation */ export interface Transform { + /** The type of this transform */ + transformType: TransformType; /** The version this transform is registered for */ version: string; - /** The transformation function */ + /** The upward transformation function */ transform: TransformFn; - /** The type of this transform */ - transformType: TransformType; + /** The (optional) downward transformation function */ + transformDown?: TransformFn; } export enum TransformType { From 74cf5f85dbcfc3d54f8456eabc4fdb1c45e4e7fc Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 29 Mar 2023 08:41:57 +0000 Subject: [PATCH 02/14] [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' --- .../src/document_migrator/document_migrator.ts | 1 - .../src/document_migrator/document_migrator_pipeline.test.ts | 5 +---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts index 7f9b58affa067..398fda082a301 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts @@ -41,7 +41,6 @@ * given an empty migrationVersion property {} if no such property exists. */ -import _ from 'lodash'; import type { Logger } from '@kbn/logging'; import type { SavedObjectsMigrationVersion } from '@kbn/core-saved-objects-common'; import type { diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts index 0f3be26caa889..cf1e463524ebd 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts @@ -8,10 +8,7 @@ import _ from 'lodash'; import type { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server'; -import { - modelVersionVirtualMajor, - modelVersionToVirtualVersion, -} from '@kbn/core-saved-objects-base-server-internal'; +import { modelVersionToVirtualVersion } from '@kbn/core-saved-objects-base-server-internal'; import { Transform, TransformType, TypeTransforms, TransformFn } from './types'; import { DocumentMigratorPipeline } from './document_migrator_pipeline'; From 3f3699ace749f130ccc5425f19dee15685641db3 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 29 Mar 2023 14:17:13 +0200 Subject: [PATCH 03/14] add targetTypeVersion option to pipeline --- .../document_migrator.test.ts | 27 ++++++------ .../document_migrator_pipeline.test.ts | 41 +++++++++++++++++++ .../document_migrator_pipeline.ts | 29 +++++++------ 3 files changed, 70 insertions(+), 27 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.test.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.test.ts index 8daa51d95e9f0..97ac3d92b8262 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.test.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.test.ts @@ -466,7 +466,7 @@ describe('DocumentMigrator', () => { }); }); - it('allows changing type', () => { + it('does not allow changing type', () => { const migrator = new DocumentMigrator({ ...testOpts(), typeRegistry: createRegistry( @@ -485,20 +485,17 @@ describe('DocumentMigrator', () => { ), }); migrator.prepareMigrations(); - const actual = migrator.migrate({ - id: 'smelly', - type: 'dog', - attributes: { name: 'Callie' }, - typeMigrationVersion: '', - coreMigrationVersion: '8.8.0', - }); - expect(actual).toEqual({ - id: 'smelly', - type: 'cat', - attributes: { name: 'Kitty Callie' }, - coreMigrationVersion: '8.8.0', - typeMigrationVersion: '1.0.0', - }); + expect(() => + migrator.migrate({ + id: 'smelly', + type: 'dog', + attributes: { name: 'Callie' }, + typeMigrationVersion: '', + coreMigrationVersion: '8.8.0', + }) + ).toThrowErrorMatchingInlineSnapshot( + `"Changing a document's type during a migration is not supported."` + ); }); it('disallows updating a typeMigrationVersion prop to a lower version', () => { diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts index cf1e463524ebd..d8078b51185b0 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts @@ -247,5 +247,46 @@ describe('DocumentMigratorPipeline', () => { const outputDoc = pipeline.document; expect(outputDoc.typeMigrationVersion).toEqual(virtualModelVersion_3); }); + + it('supports specifying a `targetTypeVersion` and only run migrate transforms up to it', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + typeMigrationVersion: '8.5.0', + }); + + const migrate8_6_0_up = createTransformFn(); + const migrate8_7_0_up = createTransformFn(); + const migrate8_8_0_up = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { transformType: TransformType.Migrate, version: '8.6.0', transform: migrate8_6_0_up }, + { transformType: TransformType.Migrate, version: '8.7.0', transform: migrate8_7_0_up }, + { transformType: TransformType.Migrate, version: '8.8.0', transform: migrate8_8_0_up }, + ]); + + const pipeline = new DocumentMigratorPipeline({ + document, + kibanaVersion: '8.8.0', + convertNamespaceTypes: false, + migrations: { + foo: fooTransforms, + }, + targetTypeVersion: '8.7.0', + }); + + pipeline.run(); + + expect(migrate8_6_0_up).toHaveBeenCalledTimes(1); + expect(migrate8_6_0_up).toHaveBeenCalledWith({ ...document }); + + expect(migrate8_7_0_up).toHaveBeenCalledTimes(1); + expect(migrate8_7_0_up).toHaveBeenCalledWith({ ...document, typeMigrationVersion: '8.6.0' }); + + expect(migrate8_8_0_up).not.toHaveBeenCalled(); + + const outputDoc = pipeline.document; + expect(outputDoc.typeMigrationVersion).toEqual('8.7.0'); + }); }); }); diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts index 714fa4cf49c94..73fc64ee14abb 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts @@ -17,6 +17,9 @@ function isGreater(a?: string, b?: string) { return !!a && (!b || Semver.gt(a, b)); } +/* transform types using `coreMigrationVersion` and not `typeMigrationVersion` */ +const coreVersionTransformTypes = [TransformType.Core, TransformType.Reference]; + export class DocumentMigratorPipeline { public additionalDocs = [] as SavedObjectUnsanitizedDoc[]; public document: SavedObjectUnsanitizedDoc; @@ -25,38 +28,36 @@ export class DocumentMigratorPipeline { private migrations: ActiveMigrations; private kibanaVersion: string; private convertNamespaceTypes: boolean; + private targetTypeVersion: string; constructor({ document, migrations, kibanaVersion, convertNamespaceTypes, + targetTypeVersion, }: { document: SavedObjectUnsanitizedDoc; migrations: ActiveMigrations; kibanaVersion: string; convertNamespaceTypes: boolean; + targetTypeVersion?: string; }) { this.originalDoc = document; this.document = cloneDeep(document); this.migrations = migrations; this.kibanaVersion = kibanaVersion; this.convertNamespaceTypes = convertNamespaceTypes; + this.targetTypeVersion = targetTypeVersion || migrations[document.type]?.latestVersion.migrate; } protected *getPipeline(): Generator { while (this.hasPendingTransforms()) { - const { type: previousType } = this.document; - for (const transform of this.getPendingTransforms()) { yield transform; - if (this.document.type !== previousType) { - // In the initial implementation, all the transforms for the new type should be applied. - // And at the same time, documents with `undefined` in `typeMigrationVersion` are treated as the most recent ones. - // This is a workaround to get into the loop again and apply all the migrations for the new type. - this.document.typeMigrationVersion = ''; - break; + if (this.document.type !== this.originalDoc.type) { + throw new Error(`Changing a document's type during a migration is not supported.`); } } } @@ -75,7 +76,7 @@ export class DocumentMigratorPipeline { } return ( - isGreater(latestVersion?.migrate, typeMigrationVersion) || + isGreater(this.targetTypeVersion, typeMigrationVersion) || (this.convertNamespaceTypes && isGreater(latestVersion?.convert, typeMigrationVersion)) || (this.convertNamespaceTypes && isGreater(latestVersion?.reference, coreMigrationVersion)) ); @@ -106,7 +107,11 @@ export class DocumentMigratorPipeline { isGreater(version, typeMigrationVersion) ); case TransformType.Migrate: - return typeMigrationVersion != null && isGreater(version, typeMigrationVersion); + return ( + typeMigrationVersion != null && + isGreater(version, typeMigrationVersion) && + Semver.lte(version, this.targetTypeVersion) + ); } } @@ -161,7 +166,7 @@ export class DocumentMigratorPipeline { * as this could get us into an infinite loop. So, we explicitly check for that here. */ private assertUpgrade({ transformType, version }: Transform, previousVersion?: string) { - if ([TransformType.Core, TransformType.Reference].includes(transformType)) { + if (coreVersionTransformTypes.includes(transformType)) { return; } @@ -177,7 +182,7 @@ export class DocumentMigratorPipeline { private bumpVersion({ transformType, version }: Transform) { this.document = { ...this.document, - ...([TransformType.Core, TransformType.Reference].includes(transformType) + ...(coreVersionTransformTypes.includes(transformType) ? { coreMigrationVersion: maxVersion(this.document.coreMigrationVersion, version) } : { typeMigrationVersion: maxVersion(this.document.typeMigrationVersion, version) }), }; From 67cdaa2b594f830a4575756307cb9074bacfa76e Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 29 Mar 2023 14:33:31 +0200 Subject: [PATCH 04/14] trying to find the best approach --- .../document_migrator_pipeline.ts | 15 ++------- .../src/document_migrator/pipeline_utils.ts | 33 +++++++++++++++++++ .../src/document_migrator/utils.ts | 11 +++++++ 3 files changed, 46 insertions(+), 13 deletions(-) create mode 100644 packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipeline_utils.ts diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts index 73fc64ee14abb..71ea28ed9954f 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts @@ -12,14 +12,12 @@ import Semver from 'semver'; import type { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server'; import { ActiveMigrations, Transform, TransformType } from './types'; import { maxVersion } from './utils'; +import { coreVersionTransformTypes, applyVersion } from './pipeline_utils'; function isGreater(a?: string, b?: string) { return !!a && (!b || Semver.gt(a, b)); } -/* transform types using `coreMigrationVersion` and not `typeMigrationVersion` */ -const coreVersionTransformTypes = [TransformType.Core, TransformType.Reference]; - export class DocumentMigratorPipeline { public additionalDocs = [] as SavedObjectUnsanitizedDoc[]; public document: SavedObjectUnsanitizedDoc; @@ -179,15 +177,6 @@ export class DocumentMigratorPipeline { } } - private bumpVersion({ transformType, version }: Transform) { - this.document = { - ...this.document, - ...(coreVersionTransformTypes.includes(transformType) - ? { coreMigrationVersion: maxVersion(this.document.coreMigrationVersion, version) } - : { typeMigrationVersion: maxVersion(this.document.typeMigrationVersion, version) }), - }; - } - private ensureVersion({ coreMigrationVersion: currentCoreMigrationVersion, typeMigrationVersion: currentTypeMigrationVersion, @@ -217,7 +206,7 @@ export class DocumentMigratorPipeline { this.additionalDocs.push(...additionalDocs.map((document) => this.ensureVersion(document))); this.assertUpgrade(transform, previousVersion); - this.bumpVersion(transform); + this.document = applyVersion({ document: this.document, transform }); } this.assertCompatibility(); diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipeline_utils.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipeline_utils.ts new file mode 100644 index 0000000000000..190a39525152f --- /dev/null +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipeline_utils.ts @@ -0,0 +1,33 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import type { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server'; +import { type Transform, TransformType } from './types'; +// import { maxVersion } from './utils'; + +/** transform types using `coreMigrationVersion` and not `typeMigrationVersion` */ +export const coreVersionTransformTypes = [TransformType.Core, TransformType.Reference]; + +/** + * Apply the version of the given {@link Transform | transform} to the given {@link SavedObjectUnsanitizedDoc | document}. + * Will update `coreMigrationVersion` or `typeMigrationVersion` depending on the type of the transform. + */ +export const applyVersion = ({ + document, + transform, +}: { + document: SavedObjectUnsanitizedDoc; + transform: Transform; +}): SavedObjectUnsanitizedDoc => { + return { + ...document, + ...(coreVersionTransformTypes.includes(transform.transformType) + ? { coreMigrationVersion: transform.version } + : { typeMigrationVersion: transform.version }), + }; +}; diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/utils.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/utils.ts index f80500c2ca630..9ffd208a352cf 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/utils.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/utils.ts @@ -94,3 +94,14 @@ export function maxVersion(a?: string, b?: string) { return Semver.gt(a, b) ? a : b; } + +export function minVersion(a?: string, b?: string) { + if (!a) { + return b; + } + if (!b) { + return a; + } + + return Semver.lt(a, b) ? a : b; +} From 2b17e30770e4f8094b6cc6de5009954cef84efbc Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 29 Mar 2023 18:14:02 +0200 Subject: [PATCH 05/14] implement downgrade pipeline, move stuff around --- .../document_migrator/document_migrator.ts | 10 +- .../src/document_migrator/model_version.ts | 52 +++-- .../pipelines/downgrade_pipeline.test.ts | 181 ++++++++++++++++++ .../pipelines/downgrade_pipeline.ts | 135 +++++++++++++ .../src/document_migrator/pipelines/index.ts | 11 ++ .../src/document_migrator/pipelines/types.ts | 18 ++ .../upgrade_pipeline.test.ts} | 31 ++- .../upgrade_pipeline.ts} | 50 ++--- .../{pipeline_utils.ts => pipelines/utils.ts} | 48 ++++- .../src/document_migrator/utils.ts | 22 --- 10 files changed, 448 insertions(+), 110 deletions(-) create mode 100644 packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.test.ts create mode 100644 packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.ts create mode 100644 packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/index.ts create mode 100644 packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/types.ts rename packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/{document_migrator_pipeline.test.ts => pipelines/upgrade_pipeline.test.ts} (92%) rename packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/{document_migrator_pipeline.ts => pipelines/upgrade_pipeline.ts} (83%) rename packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/{pipeline_utils.ts => pipelines/utils.ts} (50%) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts index 398fda082a301..502506ea0fbd8 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts @@ -48,9 +48,9 @@ import type { ISavedObjectTypeRegistry, } from '@kbn/core-saved-objects-server'; import type { ActiveMigrations } from './types'; -import { maxVersion } from './utils'; +import { maxVersion } from './pipelines/utils'; import { buildActiveMigrations } from './build_active_migrations'; -import { DocumentMigratorPipeline } from './document_migrator_pipeline'; +import { DocumentUpgradePipeline } from './pipelines'; export type MigrateFn = (doc: SavedObjectUnsanitizedDoc) => SavedObjectUnsanitizedDoc; export type MigrateAndConvertFn = (doc: SavedObjectUnsanitizedDoc) => SavedObjectUnsanitizedDoc[]; @@ -144,15 +144,13 @@ export class DocumentMigrator implements VersionedTransformer { throw new Error('Migrations are not ready. Make sure prepareMigrations is called first.'); } - const pipeline = new DocumentMigratorPipeline({ + const pipeline = new DocumentUpgradePipeline({ document: doc, migrations: this.migrations, kibanaVersion: this.documentMigratorOptions.kibanaVersion, convertNamespaceTypes, }); - pipeline.run(); - - const { document, additionalDocs } = pipeline; + const { document, additionalDocs } = pipeline.run(); return { document, additionalDocs }; } diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.ts index 40d08022a2099..df31b5f082e01 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.ts @@ -20,6 +20,8 @@ import { import { TransformSavedObjectDocumentError } from '../core'; import { type Transform, type TransformFn, TransformType } from './types'; +export const noopTransform: TransformFn = (doc) => ({ transformedDoc: doc, additionalDocs: [] }); + export const getModelVersionTransforms = ({ typeDefinition, log, @@ -32,32 +34,28 @@ export const getModelVersionTransforms = ({ ? typeDefinition.modelVersions() : typeDefinition.modelVersions ?? {}; - const transforms = Object.entries(modelVersionMap) - .filter(([_, definition]) => !!definition.modelChange.transformation) - .map(([rawModelVersion, definition]) => { - const modelVersion = assertValidModelVersion(rawModelVersion); - const virtualVersion = modelVersionToVirtualVersion(modelVersion); - return { - version: virtualVersion, - transform: convertModelVersionTransformFn({ - log, - modelVersion, - virtualVersion, - definition, - type: 'up', - }), - transformDown: convertModelVersionTransformFn({ - log, - modelVersion, - virtualVersion, - definition, - type: 'down', - }), - transformType: TransformType.Migrate, - }; - }); - - return transforms; + return Object.entries(modelVersionMap).map(([rawModelVersion, definition]) => { + const modelVersion = assertValidModelVersion(rawModelVersion); + const virtualVersion = modelVersionToVirtualVersion(modelVersion); + return { + version: virtualVersion, + transform: convertModelVersionTransformFn({ + log, + modelVersion, + virtualVersion, + definition, + type: 'up', + }), + transformDown: convertModelVersionTransformFn({ + log, + modelVersion, + virtualVersion, + definition, + type: 'down', + }), + transformType: TransformType.Migrate, + }; + }); }; export const convertModelVersionTransformFn = ({ @@ -74,7 +72,7 @@ export const convertModelVersionTransformFn = ({ log: Logger; }): TransformFn => { if (!definition.modelChange.transformation) { - throw new Error('cannot convert model change without a transform'); + return noopTransform; } const context: SavedObjectModelTransformationContext = { log, diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.test.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.test.ts new file mode 100644 index 0000000000000..63ac63104b976 --- /dev/null +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.test.ts @@ -0,0 +1,181 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import _ from 'lodash'; +import type { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server'; +import { Transform, TransformType, TypeTransforms, TransformFn } from '../types'; +import { DocumentDowngradePipeline } from './downgrade_pipeline'; + +// snake case is way better for migration function names in this very specific scenario. +/* eslint-disable @typescript-eslint/naming-convention */ + +describe('DocumentMigratorPipeline', () => { + const defaultKibanaVersion = '8.8.0'; + + const createDoc = ( + parts: Partial> = {} + ): SavedObjectUnsanitizedDoc => ({ + id: 'test-doc', + type: 'test-type', + attributes: {}, + references: [], + coreMigrationVersion: defaultKibanaVersion, + ...parts, + }); + + const latestVersions = ( + parts: Partial> = {} + ): Record => ({ + [TransformType.Convert]: defaultKibanaVersion, + [TransformType.Migrate]: defaultKibanaVersion, + [TransformType.Core]: defaultKibanaVersion, + [TransformType.Reference]: defaultKibanaVersion, + ...parts, + }); + + const getTypeTransforms = (transforms: Transform[]): TypeTransforms => { + const versions = _.chain(transforms) + .groupBy('transformType') + .mapValues((items) => _.last(items)?.version) + .value() as Record; + + return { + transforms, + latestVersion: latestVersions(versions), + }; + }; + + const createTransformFn = (impl?: TransformFn): jest.MockedFunction => { + const defaultImpl: TransformFn = (doc) => ({ transformedDoc: doc, additionalDocs: [] }); + return jest.fn().mockImplementation(impl ?? defaultImpl); + }; + + it('calls multiple `Migrate` conversion function in order', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + typeMigrationVersion: '8.8.0', + }); + + const migrate8_6_0_up = createTransformFn(); + const migrate8_6_0_down = createTransformFn(); + const migrate8_7_0_up = createTransformFn(); + const migrate8_7_0_down = createTransformFn(); + const migrate8_8_0_up = createTransformFn(); + const migrate8_8_0_down = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { + transformType: TransformType.Migrate, + version: '8.6.0', + transform: migrate8_6_0_up, + transformDown: migrate8_6_0_down, + }, + { + transformType: TransformType.Migrate, + version: '8.7.0', + transform: migrate8_7_0_up, + transformDown: migrate8_7_0_down, + }, + { + transformType: TransformType.Migrate, + version: '8.8.0', + transform: migrate8_8_0_up, + transformDown: migrate8_8_0_down, + }, + ]); + + const pipeline = new DocumentDowngradePipeline({ + document, + kibanaVersion: '8.8.0', + typeTransforms: fooTransforms, + targetTypeVersion: '8.5.0', + }); + + const { document: outputDoc } = pipeline.run(); + + expect(migrate8_6_0_up).not.toHaveBeenCalled(); + expect(migrate8_7_0_up).not.toHaveBeenCalled(); + expect(migrate8_8_0_up).not.toHaveBeenCalled(); + + expect(migrate8_8_0_down).toHaveBeenCalledTimes(1); + expect(migrate8_8_0_down).toHaveBeenCalledWith(document); + + expect(migrate8_7_0_down).toHaveBeenCalledTimes(1); + expect(migrate8_7_0_down).toHaveBeenCalledWith({ ...document, typeMigrationVersion: '8.8.0' }); + + expect(migrate8_6_0_down).toHaveBeenCalledTimes(1); + expect(migrate8_6_0_down).toHaveBeenCalledWith({ ...document, typeMigrationVersion: '8.7.0' }); + + expect(migrate8_8_0_down.mock.invocationCallOrder[0]).toBeLessThan( + migrate8_7_0_down.mock.invocationCallOrder[0] + ); + expect(migrate8_7_0_down.mock.invocationCallOrder[0]).toBeLessThan( + migrate8_6_0_down.mock.invocationCallOrder[0] + ); + + expect(outputDoc.typeMigrationVersion).toEqual('8.5.0'); + }); + + it('does not call migrations with versions equal or below the requested version', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + typeMigrationVersion: '8.8.0', + }); + + const migrate8_6_0_up = createTransformFn(); + const migrate8_6_0_down = createTransformFn(); + const migrate8_7_0_up = createTransformFn(); + const migrate8_7_0_down = createTransformFn(); + const migrate8_8_0_up = createTransformFn(); + const migrate8_8_0_down = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { + transformType: TransformType.Migrate, + version: '8.6.0', + transform: migrate8_6_0_up, + transformDown: migrate8_6_0_down, + }, + { + transformType: TransformType.Migrate, + version: '8.7.0', + transform: migrate8_7_0_up, + transformDown: migrate8_7_0_down, + }, + { + transformType: TransformType.Migrate, + version: '8.8.0', + transform: migrate8_8_0_up, + transformDown: migrate8_8_0_down, + }, + ]); + + const pipeline = new DocumentDowngradePipeline({ + document, + kibanaVersion: '8.8.0', + typeTransforms: fooTransforms, + targetTypeVersion: '8.7.0', + }); + + const { document: outputDoc } = pipeline.run(); + + expect(migrate8_6_0_up).not.toHaveBeenCalled(); + expect(migrate8_7_0_up).not.toHaveBeenCalled(); + expect(migrate8_8_0_up).not.toHaveBeenCalled(); + + expect(migrate8_8_0_down).toHaveBeenCalledTimes(1); + expect(migrate8_8_0_down).toHaveBeenCalledWith(document); + + expect(migrate8_7_0_down).not.toHaveBeenCalled(); + expect(migrate8_6_0_down).not.toHaveBeenCalled(); + + expect(outputDoc.typeMigrationVersion).toEqual('8.7.0'); + }); +}); diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.ts new file mode 100644 index 0000000000000..d2b54b81e0b8e --- /dev/null +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.ts @@ -0,0 +1,135 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import Boom from '@hapi/boom'; +import { cloneDeep } from 'lodash'; +import Semver from 'semver'; +import type { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server'; +import { Transform, TransformType, TypeTransforms } from '../types'; +import type { MigrationPipeline, MigrationPipelineResult } from './types'; +import { applyVersion, assertValidCoreVersion, maxVersion } from './utils'; + +export class DocumentDowngradePipeline implements MigrationPipeline { + private document: SavedObjectUnsanitizedDoc; + private kibanaVersion: string; + private originalDoc: SavedObjectUnsanitizedDoc; + private typeTransforms: TypeTransforms; + private targetTypeVersion: string; + private targetCoreVersion?: string; + + constructor({ + document, + kibanaVersion, + typeTransforms, + targetTypeVersion, + targetCoreVersion, + }: { + document: SavedObjectUnsanitizedDoc; + typeTransforms: TypeTransforms; + kibanaVersion: string; + targetTypeVersion: string; + targetCoreVersion?: string; + }) { + this.originalDoc = document; + this.kibanaVersion = kibanaVersion; + this.document = cloneDeep(document); + this.typeTransforms = typeTransforms; + this.targetTypeVersion = targetTypeVersion; + this.targetCoreVersion = targetCoreVersion; + } + + run(): MigrationPipelineResult { + assertValidCoreVersion({ document: this.document, kibanaVersion: this.kibanaVersion }); + this.assertCompatibility(); + + for (const transform of this.getPendingTransforms()) { + if (!transform.transformDown) { + throw new Error( + `Could not apply transformation ${transform.transformType}:${transform.version}: no down conversion registered` + ); + } + const { transformedDoc } = transform.transformDown(this.document); + if (this.document.type !== this.originalDoc.type) { + throw new Error(`Changing a document's type during a migration is not supported.`); + } + this.document = applyVersion({ document: transformedDoc, transform }); + } + + this.document = this.ensureVersion(this.document); + + return { + document: this.document, + additionalDocs: [], + }; + } + + private getPendingTransforms() { + const { transforms } = this.typeTransforms; + return transforms.reverse().filter((transform) => this.isPendingTransform(transform)); + } + + private isPendingTransform({ transformType, version }: Transform) { + const { coreMigrationVersion, typeMigrationVersion } = this.document; + + switch (transformType) { + // reference and convert type transforms were deprecated before downward conversion were a thing + // so we will never need to revert such type of transforms (and they didn't implement a down conversion) + case TransformType.Reference: + case TransformType.Convert: + return false; + // including core transforms if targetCoreVersion is specified + case TransformType.Core: + return ( + this.targetCoreVersion && + (coreMigrationVersion == null || + Semver.gt(coreMigrationVersion, this.targetCoreVersion)) && + Semver.gt(version, this.targetCoreVersion) + ); + // including migrate transforms between the targetTypeVersion and the typeMigrationVersion + case TransformType.Migrate: + return ( + (typeMigrationVersion == null || Semver.gte(typeMigrationVersion, version)) && + Semver.gt(version, this.targetTypeVersion) + ); + } + } + + /** + * Verifies that the document version is not greater than the version supported by Kibana. + * If we have a document with some version and no migrations available for this type, + * the document belongs to a future version. + */ + private assertCompatibility() { + const { id, typeMigrationVersion: currentVersion } = this.document; + const latestVersion = maxVersion( + this.typeTransforms.latestVersion.migrate, + this.typeTransforms.latestVersion.convert + )!; + + if (currentVersion && Semver.gt(currentVersion, latestVersion)) { + throw Boom.badData( + `Document "${id}" belongs to a more recent version of Kibana [${currentVersion}] when the last known version is [${latestVersion}].`, + this.document + ); + } + } + + private ensureVersion({ + coreMigrationVersion: currentCoreMigrationVersion, + typeMigrationVersion: currentTypeMigrationVersion, + ...document + }: SavedObjectUnsanitizedDoc) { + const coreMigrationVersion = this.targetCoreVersion || currentCoreMigrationVersion; + + return { + ...document, + typeMigrationVersion: this.targetTypeVersion, + ...(coreMigrationVersion ? { coreMigrationVersion } : {}), + }; + } +} diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/index.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/index.ts new file mode 100644 index 0000000000000..b46f1b2da6add --- /dev/null +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/index.ts @@ -0,0 +1,11 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +export type { MigrationPipeline, MigrationPipelineResult } from './types'; +export { DocumentDowngradePipeline } from './downgrade_pipeline'; +export { DocumentUpgradePipeline } from './upgrade_pipeline'; diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/types.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/types.ts new file mode 100644 index 0000000000000..3c4183175d104 --- /dev/null +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/types.ts @@ -0,0 +1,18 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import type { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server'; + +export interface MigrationPipelineResult { + document: SavedObjectUnsanitizedDoc; + additionalDocs: SavedObjectUnsanitizedDoc[]; +} + +export interface MigrationPipeline { + run(): MigrationPipelineResult; +} diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/upgrade_pipeline.test.ts similarity index 92% rename from packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts rename to packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/upgrade_pipeline.test.ts index d8078b51185b0..338c1088d002f 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.test.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/upgrade_pipeline.test.ts @@ -9,8 +9,8 @@ import _ from 'lodash'; import type { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server'; import { modelVersionToVirtualVersion } from '@kbn/core-saved-objects-base-server-internal'; -import { Transform, TransformType, TypeTransforms, TransformFn } from './types'; -import { DocumentMigratorPipeline } from './document_migrator_pipeline'; +import { Transform, TransformType, TypeTransforms, TransformFn } from '../types'; +import { DocumentUpgradePipeline } from './upgrade_pipeline'; // snake case is way better for migration function names in this very specific scenario. /* eslint-disable @typescript-eslint/naming-convention */ @@ -70,7 +70,7 @@ describe('DocumentMigratorPipeline', () => { { transformType: TransformType.Migrate, version: '8.8.0', transform: migrate8_8_0_up }, ]); - const pipeline = new DocumentMigratorPipeline({ + const pipeline = new DocumentUpgradePipeline({ document, kibanaVersion: '8.8.0', convertNamespaceTypes: false, @@ -79,12 +79,11 @@ describe('DocumentMigratorPipeline', () => { }, }); - pipeline.run(); + const { document: outputDoc } = pipeline.run(); expect(migrate8_8_0_up).toHaveBeenCalledTimes(1); expect(migrate8_8_0_up).toHaveBeenCalledWith(document); - const outputDoc = pipeline.document; expect(outputDoc.typeMigrationVersion).toEqual('8.8.0'); }); @@ -105,7 +104,7 @@ describe('DocumentMigratorPipeline', () => { { transformType: TransformType.Migrate, version: '8.8.0', transform: migrate8_8_0_up }, ]); - const pipeline = new DocumentMigratorPipeline({ + const pipeline = new DocumentUpgradePipeline({ document, kibanaVersion: '8.8.0', convertNamespaceTypes: false, @@ -114,7 +113,7 @@ describe('DocumentMigratorPipeline', () => { }, }); - pipeline.run(); + const { document: outputDoc } = pipeline.run(); expect(migrate8_6_0_up).toHaveBeenCalledTimes(1); expect(migrate8_6_0_up).toHaveBeenCalledWith(document); @@ -132,7 +131,6 @@ describe('DocumentMigratorPipeline', () => { migrate8_8_0_up.mock.invocationCallOrder[0] ); - const outputDoc = pipeline.document; expect(outputDoc.typeMigrationVersion).toEqual('8.8.0'); }); @@ -153,7 +151,7 @@ describe('DocumentMigratorPipeline', () => { { transformType: TransformType.Migrate, version: '8.8.0', transform: migrate8_8_0_up }, ]); - const pipeline = new DocumentMigratorPipeline({ + const pipeline = new DocumentUpgradePipeline({ document, kibanaVersion: '8.8.0', convertNamespaceTypes: false, @@ -162,7 +160,7 @@ describe('DocumentMigratorPipeline', () => { }, }); - pipeline.run(); + const { document: outputDoc } = pipeline.run(); expect(migrate8_6_0_up).not.toHaveBeenCalled(); @@ -171,7 +169,6 @@ describe('DocumentMigratorPipeline', () => { expect(migrate8_8_0_up).toHaveBeenCalledTimes(1); expect(migrate8_8_0_up).toHaveBeenCalledWith({ ...document, typeMigrationVersion: '8.7.0' }); - const outputDoc = pipeline.document; expect(outputDoc.typeMigrationVersion).toEqual('8.8.0'); }); @@ -188,7 +185,7 @@ describe('DocumentMigratorPipeline', () => { { transformType: TransformType.Migrate, version: '8.8.0', transform: migrate8_8_0_up }, ]); - const pipeline = new DocumentMigratorPipeline({ + const pipeline = new DocumentUpgradePipeline({ document, kibanaVersion: '8.8.0', convertNamespaceTypes: false, @@ -223,7 +220,7 @@ describe('DocumentMigratorPipeline', () => { }, ]); - const pipeline = new DocumentMigratorPipeline({ + const pipeline = new DocumentUpgradePipeline({ document, kibanaVersion: '8.8.0', convertNamespaceTypes: false, @@ -232,7 +229,7 @@ describe('DocumentMigratorPipeline', () => { }, }); - pipeline.run(); + const { document: outputDoc } = pipeline.run(); expect(migrate8_8_0_up).toHaveBeenCalledTimes(1); expect(migrate8_8_0_up).toHaveBeenCalledWith({ ...document }); @@ -244,7 +241,6 @@ describe('DocumentMigratorPipeline', () => { migrate_mv_3.mock.invocationCallOrder[0] ); - const outputDoc = pipeline.document; expect(outputDoc.typeMigrationVersion).toEqual(virtualModelVersion_3); }); @@ -265,7 +261,7 @@ describe('DocumentMigratorPipeline', () => { { transformType: TransformType.Migrate, version: '8.8.0', transform: migrate8_8_0_up }, ]); - const pipeline = new DocumentMigratorPipeline({ + const pipeline = new DocumentUpgradePipeline({ document, kibanaVersion: '8.8.0', convertNamespaceTypes: false, @@ -275,7 +271,7 @@ describe('DocumentMigratorPipeline', () => { targetTypeVersion: '8.7.0', }); - pipeline.run(); + const { document: outputDoc } = pipeline.run(); expect(migrate8_6_0_up).toHaveBeenCalledTimes(1); expect(migrate8_6_0_up).toHaveBeenCalledWith({ ...document }); @@ -285,7 +281,6 @@ describe('DocumentMigratorPipeline', () => { expect(migrate8_8_0_up).not.toHaveBeenCalled(); - const outputDoc = pipeline.document; expect(outputDoc.typeMigrationVersion).toEqual('8.7.0'); }); }); diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/upgrade_pipeline.ts similarity index 83% rename from packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts rename to packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/upgrade_pipeline.ts index 71ea28ed9954f..bd70590668e70 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator_pipeline.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/upgrade_pipeline.ts @@ -10,18 +10,22 @@ import Boom from '@hapi/boom'; import { cloneDeep } from 'lodash'; import Semver from 'semver'; import type { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server'; -import { ActiveMigrations, Transform, TransformType } from './types'; -import { maxVersion } from './utils'; -import { coreVersionTransformTypes, applyVersion } from './pipeline_utils'; +import { ActiveMigrations, Transform, TransformType } from '../types'; +import type { MigrationPipeline, MigrationPipelineResult } from './types'; +import { + coreVersionTransformTypes, + applyVersion, + assertValidCoreVersion, + maxVersion, +} from './utils'; function isGreater(a?: string, b?: string) { return !!a && (!b || Semver.gt(a, b)); } -export class DocumentMigratorPipeline { - public additionalDocs = [] as SavedObjectUnsanitizedDoc[]; - public document: SavedObjectUnsanitizedDoc; - +export class DocumentUpgradePipeline implements MigrationPipeline { + private additionalDocs = [] as SavedObjectUnsanitizedDoc[]; + private document: SavedObjectUnsanitizedDoc; private originalDoc: SavedObjectUnsanitizedDoc; private migrations: ActiveMigrations; private kibanaVersion: string; @@ -113,32 +117,6 @@ export class DocumentMigratorPipeline { } } - /** - * Asserts the object's core version is valid and not greater than the current Kibana version. - * Hence, the object does not belong to a more recent version of Kibana. - */ - private assertValidity() { - const { id, coreMigrationVersion } = this.document; - if (!coreMigrationVersion) { - return; - } - - if (!Semver.valid(coreMigrationVersion)) { - throw Boom.badData( - `Document "${id}" has an invalid "coreMigrationVersion" [${coreMigrationVersion}]. This must be a semver value.`, - this.document - ); - } - - if (Semver.gt(coreMigrationVersion, this.kibanaVersion)) { - throw Boom.badData( - `Document "${id}" has a "coreMigrationVersion" which belongs to a more recent version` + - ` of Kibana [${coreMigrationVersion}]. The current version is [${this.kibanaVersion}].`, - this.document - ); - } - } - /** * Verifies that the document version is not greater than the version supported by Kibana. * If we have a document with some version and no migrations available for this type, @@ -196,8 +174,8 @@ export class DocumentMigratorPipeline { }; } - run(): void { - this.assertValidity(); + run(): MigrationPipelineResult { + assertValidCoreVersion({ document: this.document, kibanaVersion: this.kibanaVersion }); for (const transform of this.getPipeline()) { const { typeMigrationVersion: previousVersion } = this.document; @@ -212,5 +190,7 @@ export class DocumentMigratorPipeline { this.assertCompatibility(); this.document = this.ensureVersion(this.document); + + return { document: this.document, additionalDocs: this.additionalDocs }; } } diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipeline_utils.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/utils.ts similarity index 50% rename from packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipeline_utils.ts rename to packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/utils.ts index 190a39525152f..c111788e71be5 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipeline_utils.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/utils.ts @@ -6,9 +6,10 @@ * Side Public License, v 1. */ +import Semver from 'semver'; +import Boom from '@hapi/boom'; import type { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server'; -import { type Transform, TransformType } from './types'; -// import { maxVersion } from './utils'; +import { type Transform, TransformType } from '../types'; /** transform types using `coreMigrationVersion` and not `typeMigrationVersion` */ export const coreVersionTransformTypes = [TransformType.Core, TransformType.Reference]; @@ -31,3 +32,46 @@ export const applyVersion = ({ : { typeMigrationVersion: transform.version }), }; }; + +/** + * Asserts the document's core version is valid and not greater than the current Kibana version. + * Hence, the object does not belong to a more recent version of Kibana. + */ +export const assertValidCoreVersion = ({ + kibanaVersion, + document, +}: { + document: SavedObjectUnsanitizedDoc; + kibanaVersion: string; +}) => { + const { id, coreMigrationVersion } = document; + if (!coreMigrationVersion) { + return; + } + + if (!Semver.valid(coreMigrationVersion)) { + throw Boom.badData( + `Document "${id}" has an invalid "coreMigrationVersion" [${coreMigrationVersion}]. This must be a semver value.`, + document + ); + } + + if (Semver.gt(coreMigrationVersion, kibanaVersion)) { + throw Boom.badData( + `Document "${id}" has a "coreMigrationVersion" which belongs to a more recent version` + + ` of Kibana [${coreMigrationVersion}]. The current version is [${kibanaVersion}].`, + document + ); + } +}; + +export function maxVersion(a?: string, b?: string) { + if (!a) { + return b; + } + if (!b) { + return a; + } + + return Semver.gt(a, b) ? a : b; +} diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/utils.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/utils.ts index 9ffd208a352cf..63b0462714e02 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/utils.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/utils.ts @@ -83,25 +83,3 @@ export function transformComparator(a: Transform, b: Transform) { return Semver.compare(a.version, b.version) || aPriority - bPriority; } - -export function maxVersion(a?: string, b?: string) { - if (!a) { - return b; - } - if (!b) { - return a; - } - - return Semver.gt(a, b) ? a : b; -} - -export function minVersion(a?: string, b?: string) { - if (!a) { - return b; - } - if (!b) { - return a; - } - - return Semver.lt(a, b) ? a : b; -} From 29d1d362c5b41ce1a2d961b319bcee5891e93a6d Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 30 Mar 2023 07:44:39 +0200 Subject: [PATCH 06/14] adapt model version conversion unit test --- .../src/document_migrator/model_version.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.test.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.test.ts index a5a5e578a0b2e..55d59519c3ab6 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.test.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.test.ts @@ -38,7 +38,7 @@ describe('getModelVersionTransforms', () => { log = loggerMock.create(); }); - it('generate transforms for model version having a transformation', () => { + it('generate transforms for all model versions', () => { const typeDefinition = createType({ name: 'foo', modelVersions: { @@ -67,6 +67,7 @@ describe('getModelVersionTransforms', () => { expect(transforms).toEqual([ expectTransform(TransformType.Migrate, '10.1.0'), + expectTransform(TransformType.Migrate, '10.2.0'), expectTransform(TransformType.Migrate, '10.3.0'), ]); }); @@ -100,6 +101,7 @@ describe('getModelVersionTransforms', () => { expect(transforms).toEqual([ expectTransform(TransformType.Migrate, '10.1.0'), + expectTransform(TransformType.Migrate, '10.2.0'), expectTransform(TransformType.Migrate, '10.3.0'), ]); }); From 651c34f6470344dffbbb793139210e39e1c7f7ac Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 30 Mar 2023 07:47:25 +0200 Subject: [PATCH 07/14] remove export --- .../src/document_migrator/model_version.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.ts index df31b5f082e01..86838891292ee 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/model_version.ts @@ -20,7 +20,7 @@ import { import { TransformSavedObjectDocumentError } from '../core'; import { type Transform, type TransformFn, TransformType } from './types'; -export const noopTransform: TransformFn = (doc) => ({ transformedDoc: doc, additionalDocs: [] }); +const noopTransform: TransformFn = (doc) => ({ transformedDoc: doc, additionalDocs: [] }); export const getModelVersionTransforms = ({ typeDefinition, From 6e22e609aaedb4d95ef250eb9265dc8472df36f2 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 30 Mar 2023 07:52:11 +0200 Subject: [PATCH 08/14] removing outdated comment --- .../document_migrator/document_migrator.ts | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts index 502506ea0fbd8..5e7e1054c95cb 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts @@ -6,41 +6,6 @@ * Side Public License, v 1. */ -/* - * This file contains logic for transforming / migrating a saved object document. - * - * At first, it may seem as if this could be a simple filter + reduce operation, - * running the document through a linear set of transform functions until it is - * up to date, but there are some edge cases that make it more complicated. - * - * A transform can add a new property, rename an existing property, remove a property, etc. - * This means that we aren't able to do a reduce over a fixed list of properties, as - * each transform operation could essentially change what transforms should be applied - * next. - * - * The basic algorithm, then, is this: - * - * While there are any unmigrated properties in the doc, find the next unmigrated property, - * and run the doc through the transforms that target that property. - * - * This way, we keep looping until there are no transforms left to apply, and we properly - * handle property addition / deletion / renaming. - * - * A caveat is that this means we must restrict what a migration can do to the doc's - * migrationVersion itself. Migrations should *not* make any changes to the migrationVersion property. - * - * One last gotcha is that any docs which have no migrationVersion are assumed to be up-to-date. - * This is because Kibana UI and other clients really can't be expected build the migrationVersion - * in a reliable way. Instead, callers of our APIs are expected to send us up-to-date documents, - * and those documents are simply given a stamp of approval by this transformer. This is why it is - * important for migration authors to *also* write a saved object validation that will prevent this - * assumption from inserting out-of-date documents into the index. - * - * If the client(s) send us documents with migrationVersion specified, we will migrate them as - * appropriate. This means for data import scenarios, any documetns being imported should be explicitly - * given an empty migrationVersion property {} if no such property exists. - */ - import type { Logger } from '@kbn/logging'; import type { SavedObjectsMigrationVersion } from '@kbn/core-saved-objects-common'; import type { From 2e629de0f63914d903270ee9f26c50fa9c204401 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 30 Mar 2023 08:02:47 +0200 Subject: [PATCH 09/14] some cleanup --- .../src/core/migrate_raw_docs.ts | 4 +++- .../document_migrator/document_migrator.ts | 21 +++++++------------ .../src/kibana_migrator.ts | 4 ++-- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/migrate_raw_docs.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/migrate_raw_docs.ts index c83cab1c1d0bc..26e843ad99412 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/migrate_raw_docs.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/migrate_raw_docs.ts @@ -17,9 +17,11 @@ import type { SavedObjectUnsanitizedDoc, ISavedObjectsSerializer, } from '@kbn/core-saved-objects-server'; -import type { MigrateAndConvertFn } from '../document_migrator/document_migrator'; +import type { VersionedTransformer } from '../document_migrator'; import { TransformSavedObjectDocumentError } from '.'; +type MigrateAndConvertFn = VersionedTransformer['migrateAndConvert']; + export interface DocumentsTransformFailed { readonly type: string; readonly corruptDocumentIds: string[]; diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts index 5e7e1054c95cb..7a22ee3051acc 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts @@ -17,9 +17,6 @@ import { maxVersion } from './pipelines/utils'; import { buildActiveMigrations } from './build_active_migrations'; import { DocumentUpgradePipeline } from './pipelines'; -export type MigrateFn = (doc: SavedObjectUnsanitizedDoc) => SavedObjectUnsanitizedDoc; -export type MigrateAndConvertFn = (doc: SavedObjectUnsanitizedDoc) => SavedObjectUnsanitizedDoc[]; - interface TransformOptions { convertNamespaceTypes?: boolean; } @@ -35,31 +32,29 @@ interface DocumentMigratorOptions { * Manages migration of individual documents. */ export interface VersionedTransformer { - migrationVersion: SavedObjectsMigrationVersion; - migrate: MigrateFn; - migrateAndConvert: MigrateAndConvertFn; - prepareMigrations: () => void; + migrate: (doc: SavedObjectUnsanitizedDoc) => SavedObjectUnsanitizedDoc; + migrateAndConvert: (doc: SavedObjectUnsanitizedDoc) => SavedObjectUnsanitizedDoc[]; } /** * A concrete implementation of the VersionedTransformer interface. */ export class DocumentMigrator implements VersionedTransformer { - private documentMigratorOptions: DocumentMigratorOptions; + private options: DocumentMigratorOptions; private migrations?: ActiveMigrations; /** * Creates an instance of DocumentMigrator. * - * @param {DocumentMigratorOptions} opts + * @param {DocumentMigratorOptions} options * @prop {string} kibanaVersion - The current version of Kibana * @prop {SavedObjectTypeRegistry} typeRegistry - The type registry to get type migrations from * @prop {string} convertVersion - The version of Kibana in which documents can be converted to multi-namespace types * @prop {Logger} log - The migration logger * @memberof DocumentMigrator */ - constructor(documentMigratorOptions: DocumentMigratorOptions) { - this.documentMigratorOptions = documentMigratorOptions; + constructor(options: DocumentMigratorOptions) { + this.options = options; } /** @@ -92,7 +87,7 @@ export class DocumentMigrator implements VersionedTransformer { */ public prepareMigrations = () => { - const { typeRegistry, kibanaVersion, log, convertVersion } = this.documentMigratorOptions; + const { typeRegistry, kibanaVersion, log, convertVersion } = this.options; this.migrations = buildActiveMigrations({ typeRegistry, kibanaVersion, @@ -112,7 +107,7 @@ export class DocumentMigrator implements VersionedTransformer { const pipeline = new DocumentUpgradePipeline({ document: doc, migrations: this.migrations, - kibanaVersion: this.documentMigratorOptions.kibanaVersion, + kibanaVersion: this.options.kibanaVersion, convertNamespaceTypes, }); const { document, additionalDocs } = pipeline.run(); diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/kibana_migrator.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/kibana_migrator.ts index d63cc69d8f7f4..ef5166f8528fc 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/kibana_migrator.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/kibana_migrator.ts @@ -31,7 +31,7 @@ import { type MigrationResult, } from '@kbn/core-saved-objects-base-server-internal'; import { buildActiveMappings, buildTypesMappings } from './core'; -import { DocumentMigrator, type VersionedTransformer } from './document_migrator'; +import { DocumentMigrator } from './document_migrator'; import { createIndexMap } from './core/build_index_map'; import { runResilientMigrator } from './run_resilient_migrator'; import { migrateRawDocsSafely } from './core/migrate_raw_docs'; @@ -57,7 +57,7 @@ export interface KibanaMigratorOptions { */ export class KibanaMigrator implements IKibanaMigrator { private readonly client: ElasticsearchClient; - private readonly documentMigrator: VersionedTransformer; + private readonly documentMigrator: DocumentMigrator; private readonly kibanaIndex: string; private readonly log: Logger; private readonly mappingProperties: SavedObjectsTypeMappingDefinitions; From f3738f828c38fbb36301851f1fea9cf0ec40a593 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 30 Mar 2023 08:05:43 +0200 Subject: [PATCH 10/14] more cleanup --- .../document_migrator/document_migrator.ts | 64 ++++++++----------- 1 file changed, 27 insertions(+), 37 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts index 7a22ee3051acc..9f839f23a3e09 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts @@ -29,10 +29,17 @@ interface DocumentMigratorOptions { } /** - * Manages migration of individual documents. + * Manages transformations of individual documents. */ export interface VersionedTransformer { + /** + * Migrates a document to its latest version. + */ migrate: (doc: SavedObjectUnsanitizedDoc) => SavedObjectUnsanitizedDoc; + /** + * Migrates a document to the latest version and applies type conversions if applicable. + * Also returns any additional document(s) that may have been created during the transformation process. + */ migrateAndConvert: (doc: SavedObjectUnsanitizedDoc) => SavedObjectUnsanitizedDoc[]; } @@ -51,7 +58,6 @@ export class DocumentMigrator implements VersionedTransformer { * @prop {SavedObjectTypeRegistry} typeRegistry - The type registry to get type migrations from * @prop {string} convertVersion - The version of Kibana in which documents can be converted to multi-namespace types * @prop {Logger} log - The migration logger - * @memberof DocumentMigrator */ constructor(options: DocumentMigratorOptions) { this.options = options; @@ -59,10 +65,6 @@ export class DocumentMigrator implements VersionedTransformer { /** * Gets the latest version of each migrate-able property. - * - * @readonly - * @type {SavedObjectsMigrationVersion} - * @memberof DocumentMigrator */ public get migrationVersion(): SavedObjectsMigrationVersion { if (!this.migrations) { @@ -81,11 +83,7 @@ export class DocumentMigrator implements VersionedTransformer { /** * Prepares active migrations and document transformer function. - * - * @returns {void} - * @memberof DocumentMigrator */ - public prepareMigrations = () => { const { typeRegistry, kibanaVersion, log, convertVersion } = this.options; this.migrations = buildActiveMigrations({ @@ -96,31 +94,8 @@ export class DocumentMigrator implements VersionedTransformer { }); }; - private transform( - doc: SavedObjectUnsanitizedDoc, - { convertNamespaceTypes = false }: TransformOptions = {} - ) { - if (!this.migrations) { - throw new Error('Migrations are not ready. Make sure prepareMigrations is called first.'); - } - - const pipeline = new DocumentUpgradePipeline({ - document: doc, - migrations: this.migrations, - kibanaVersion: this.options.kibanaVersion, - convertNamespaceTypes, - }); - const { document, additionalDocs } = pipeline.run(); - - return { document, additionalDocs }; - } - /** * Migrates a document to the latest version. - * - * @param {SavedObjectUnsanitizedDoc} doc - * @returns {SavedObjectUnsanitizedDoc} - * @memberof DocumentMigrator */ public migrate = (doc: SavedObjectUnsanitizedDoc): SavedObjectUnsanitizedDoc => { const { document } = this.transform(doc); @@ -131,14 +106,29 @@ export class DocumentMigrator implements VersionedTransformer { /** * Migrates a document to the latest version and applies type conversions if applicable. Also returns any additional document(s) that may * have been created during the transformation process. - * - * @param {SavedObjectUnsanitizedDoc} doc - * @returns {SavedObjectUnsanitizedDoc} - * @memberof DocumentMigrator */ public migrateAndConvert = (doc: SavedObjectUnsanitizedDoc): SavedObjectUnsanitizedDoc[] => { const { document, additionalDocs } = this.transform(doc, { convertNamespaceTypes: true }); return [document, ...additionalDocs]; }; + + private transform( + doc: SavedObjectUnsanitizedDoc, + { convertNamespaceTypes = false }: TransformOptions = {} + ) { + if (!this.migrations) { + throw new Error('Migrations are not ready. Make sure prepareMigrations is called first.'); + } + + const pipeline = new DocumentUpgradePipeline({ + document: doc, + migrations: this.migrations, + kibanaVersion: this.options.kibanaVersion, + convertNamespaceTypes, + }); + const { document, additionalDocs } = pipeline.run(); + + return { document, additionalDocs }; + } } From 79231b52ba25b3d1c9de109b1eb438523ed0f066 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 30 Mar 2023 08:31:05 +0200 Subject: [PATCH 11/14] implements transformDown --- .../document_migrator/document_migrator.ts | 27 ++- .../pipelines/downgrade_pipeline.test.ts | 212 +++++++++++++++++- .../pipelines/downgrade_pipeline.ts | 18 +- .../src/zdt/test_helpers/document_migrator.ts | 3 +- 4 files changed, 247 insertions(+), 13 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts index 9f839f23a3e09..2d10ff278e7c6 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.ts @@ -15,7 +15,7 @@ import type { import type { ActiveMigrations } from './types'; import { maxVersion } from './pipelines/utils'; import { buildActiveMigrations } from './build_active_migrations'; -import { DocumentUpgradePipeline } from './pipelines'; +import { DocumentUpgradePipeline, DocumentDowngradePipeline } from './pipelines'; interface TransformOptions { convertNamespaceTypes?: boolean; @@ -41,6 +41,13 @@ export interface VersionedTransformer { * Also returns any additional document(s) that may have been created during the transformation process. */ migrateAndConvert: (doc: SavedObjectUnsanitizedDoc) => SavedObjectUnsanitizedDoc[]; + /** + * Converts a document down to the specified version. + */ + transformDown: ( + doc: SavedObjectUnsanitizedDoc, + options: { targetTypeVersion: string } + ) => SavedObjectUnsanitizedDoc; } /** @@ -113,6 +120,24 @@ export class DocumentMigrator implements VersionedTransformer { return [document, ...additionalDocs]; }; + public transformDown = ( + doc: SavedObjectUnsanitizedDoc, + options: { targetTypeVersion: string } + ): SavedObjectUnsanitizedDoc => { + if (!this.migrations) { + throw new Error('Migrations are not ready. Make sure prepareMigrations is called first.'); + } + + const pipeline = new DocumentDowngradePipeline({ + document: doc, + typeTransforms: this.migrations[doc.type], + kibanaVersion: this.options.kibanaVersion, + targetTypeVersion: options.targetTypeVersion, + }); + const { document } = pipeline.run(); + return document; + }; + private transform( doc: SavedObjectUnsanitizedDoc, { convertNamespaceTypes = false }: TransformOptions = {} diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.test.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.test.ts index 63ac63104b976..451d23abab800 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.test.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.test.ts @@ -55,7 +55,7 @@ describe('DocumentMigratorPipeline', () => { return jest.fn().mockImplementation(impl ?? defaultImpl); }; - it('calls multiple `Migrate` conversion function in order', () => { + it('calls multiple `Migrate` transform functions in order', () => { const document = createDoc({ id: 'foo-1', type: 'foo', @@ -122,7 +122,7 @@ describe('DocumentMigratorPipeline', () => { expect(outputDoc.typeMigrationVersion).toEqual('8.5.0'); }); - it('does not call migrations with versions equal or below the requested version', () => { + it('does not call transforms with versions equal or below the requested version', () => { const document = createDoc({ id: 'foo-1', type: 'foo', @@ -178,4 +178,212 @@ describe('DocumentMigratorPipeline', () => { expect(outputDoc.typeMigrationVersion).toEqual('8.7.0'); }); + + it('throws trying to apply a transform without down fn', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + typeMigrationVersion: '8.8.0', + }); + + const migrate8_6_0_up = createTransformFn(); + const migrate8_6_0_down = createTransformFn(); + const migrate8_7_0_up = createTransformFn(); + const migrate8_8_0_up = createTransformFn(); + const migrate8_8_0_down = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { + transformType: TransformType.Migrate, + version: '8.6.0', + transform: migrate8_6_0_up, + transformDown: migrate8_6_0_down, + }, + { + transformType: TransformType.Migrate, + version: '8.7.0', + transform: migrate8_7_0_up, + }, + { + transformType: TransformType.Migrate, + version: '8.8.0', + transform: migrate8_8_0_up, + transformDown: migrate8_8_0_down, + }, + ]); + + const pipeline = new DocumentDowngradePipeline({ + document, + kibanaVersion: '8.8.0', + typeTransforms: fooTransforms, + targetTypeVersion: '8.5.0', + }); + + expect(() => pipeline.run()).toThrowErrorMatchingInlineSnapshot( + `"Could not apply transformation migrate:8.7.0: no down conversion registered"` + ); + }); + + it('throws trying to downgrade to a higher version', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + typeMigrationVersion: '8.7.0', + }); + + const migrate8_6_0_up = createTransformFn(); + const migrate8_6_0_down = createTransformFn(); + const migrate8_7_0_up = createTransformFn(); + const migrate8_8_0_up = createTransformFn(); + const migrate8_8_0_down = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { + transformType: TransformType.Migrate, + version: '8.6.0', + transform: migrate8_6_0_up, + transformDown: migrate8_6_0_down, + }, + { + transformType: TransformType.Migrate, + version: '8.7.0', + transform: migrate8_7_0_up, + }, + { + transformType: TransformType.Migrate, + version: '8.8.0', + transform: migrate8_8_0_up, + transformDown: migrate8_8_0_down, + }, + ]); + + const pipeline = new DocumentDowngradePipeline({ + document, + kibanaVersion: '8.8.0', + typeTransforms: fooTransforms, + targetTypeVersion: '8.8.0', + }); + + expect(() => pipeline.run()).toThrowErrorMatchingInlineSnapshot( + `"Trying to transform down to a higher version: 8.7.0 to 8.8.0"` + ); + }); + + it('calls no transforms when converting down to the same version', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + typeMigrationVersion: '8.7.0', + }); + + const migrate8_6_0_up = createTransformFn(); + const migrate8_6_0_down = createTransformFn(); + const migrate8_7_0_up = createTransformFn(); + const migrate8_7_0_down = createTransformFn(); + const migrate8_8_0_up = createTransformFn(); + const migrate8_8_0_down = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { + transformType: TransformType.Migrate, + version: '8.6.0', + transform: migrate8_6_0_up, + transformDown: migrate8_6_0_down, + }, + { + transformType: TransformType.Migrate, + version: '8.7.0', + transform: migrate8_7_0_up, + transformDown: migrate8_7_0_down, + }, + { + transformType: TransformType.Migrate, + version: '8.8.0', + transform: migrate8_8_0_up, + transformDown: migrate8_8_0_down, + }, + ]); + + const pipeline = new DocumentDowngradePipeline({ + document, + kibanaVersion: '8.8.0', + typeTransforms: fooTransforms, + targetTypeVersion: '8.7.0', + }); + + const { document: outputDoc } = pipeline.run(); + + expect(migrate8_6_0_down).not.toHaveBeenCalled(); + expect(migrate8_7_0_down).not.toHaveBeenCalled(); + expect(migrate8_8_0_down).not.toHaveBeenCalled(); + + expect(outputDoc.typeMigrationVersion).toEqual('8.7.0'); + }); + + it('considers undefined typeMigrationVersion as being the highest current version', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + }); + document.typeMigrationVersion = undefined; + + const migrate8_6_0_up = createTransformFn(); + const migrate8_6_0_down = createTransformFn(); + const migrate8_7_0_up = createTransformFn(); + const migrate8_7_0_down = createTransformFn(); + const migrate8_8_0_up = createTransformFn(); + const migrate8_8_0_down = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { + transformType: TransformType.Migrate, + version: '8.6.0', + transform: migrate8_6_0_up, + transformDown: migrate8_6_0_down, + }, + { + transformType: TransformType.Migrate, + version: '8.7.0', + transform: migrate8_7_0_up, + transformDown: migrate8_7_0_down, + }, + { + transformType: TransformType.Migrate, + version: '8.8.0', + transform: migrate8_8_0_up, + transformDown: migrate8_8_0_down, + }, + ]); + + const pipeline = new DocumentDowngradePipeline({ + document, + kibanaVersion: '8.8.0', + typeTransforms: fooTransforms, + targetTypeVersion: '8.5.0', + }); + + const { document: outputDoc } = pipeline.run(); + + expect(migrate8_6_0_up).not.toHaveBeenCalled(); + expect(migrate8_7_0_up).not.toHaveBeenCalled(); + expect(migrate8_8_0_up).not.toHaveBeenCalled(); + + expect(migrate8_8_0_down).toHaveBeenCalledTimes(1); + expect(migrate8_8_0_down).toHaveBeenCalledWith(document); + + expect(migrate8_7_0_down).toHaveBeenCalledTimes(1); + expect(migrate8_7_0_down).toHaveBeenCalledWith({ ...document, typeMigrationVersion: '8.8.0' }); + + expect(migrate8_6_0_down).toHaveBeenCalledTimes(1); + expect(migrate8_6_0_down).toHaveBeenCalledWith({ ...document, typeMigrationVersion: '8.7.0' }); + + expect(migrate8_8_0_down.mock.invocationCallOrder[0]).toBeLessThan( + migrate8_7_0_down.mock.invocationCallOrder[0] + ); + expect(migrate8_7_0_down.mock.invocationCallOrder[0]).toBeLessThan( + migrate8_6_0_down.mock.invocationCallOrder[0] + ); + + expect(outputDoc.typeMigrationVersion).toEqual('8.5.0'); + }); }); diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.ts index d2b54b81e0b8e..63d9930af14f3 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.ts @@ -12,7 +12,7 @@ import Semver from 'semver'; import type { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server'; import { Transform, TransformType, TypeTransforms } from '../types'; import type { MigrationPipeline, MigrationPipelineResult } from './types'; -import { applyVersion, assertValidCoreVersion, maxVersion } from './utils'; +import { applyVersion, assertValidCoreVersion } from './utils'; export class DocumentDowngradePipeline implements MigrationPipeline { private document: SavedObjectUnsanitizedDoc; @@ -100,16 +100,12 @@ export class DocumentDowngradePipeline implements MigrationPipeline { } /** - * Verifies that the document version is not greater than the version supported by Kibana. - * If we have a document with some version and no migrations available for this type, - * the document belongs to a future version. + * Verifies that the current document version is not greater than the version supported by Kibana. + * And that the targetTypeVersion is not greater than the document's */ private assertCompatibility() { const { id, typeMigrationVersion: currentVersion } = this.document; - const latestVersion = maxVersion( - this.typeTransforms.latestVersion.migrate, - this.typeTransforms.latestVersion.convert - )!; + const latestVersion = this.typeTransforms.latestVersion.migrate; if (currentVersion && Semver.gt(currentVersion, latestVersion)) { throw Boom.badData( @@ -117,6 +113,12 @@ export class DocumentDowngradePipeline implements MigrationPipeline { this.document ); } + + if (currentVersion && Semver.gt(this.targetTypeVersion, currentVersion)) { + throw new Error( + `Trying to transform down to a higher version: ${currentVersion} to ${this.targetTypeVersion}` + ); + } } private ensureVersion({ diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/zdt/test_helpers/document_migrator.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/zdt/test_helpers/document_migrator.ts index 524da05952055..8e4167d277d67 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/zdt/test_helpers/document_migrator.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/zdt/test_helpers/document_migrator.ts @@ -10,9 +10,8 @@ import type { VersionedTransformer } from '../../document_migrator'; export const createDocumentMigrator = (): jest.Mocked => { return { - migrationVersion: {}, migrate: jest.fn().mockImplementation((doc: unknown) => doc), migrateAndConvert: jest.fn().mockImplementation((doc: unknown) => [doc]), - prepareMigrations: jest.fn(), + transformDown: jest.fn().mockImplementation((doc: unknown) => doc), }; }; From a615064b0f0fecf76d666cf9493e7b4ce6ffa041 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 30 Mar 2023 09:39:31 +0200 Subject: [PATCH 12/14] add more test coverage --- .../document_migrator.test.ts | 168 +++++++++++++- .../pipelines/downgrade_pipeline.test.ts | 217 ++++++++++++++++++ 2 files changed, 384 insertions(+), 1 deletion(-) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.test.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.test.ts index 97ac3d92b8262..6209a77c30c24 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.test.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.test.ts @@ -12,10 +12,15 @@ import { } from './document_migrator.test.mock'; import { set } from '@kbn/safer-lodash-set'; import _ from 'lodash'; -import type { SavedObjectUnsanitizedDoc, SavedObjectsType } from '@kbn/core-saved-objects-server'; +import type { + SavedObjectUnsanitizedDoc, + SavedObjectsType, + SavedObjectModelTransformationFn, +} from '@kbn/core-saved-objects-server'; import { SavedObjectTypeRegistry, LEGACY_URL_ALIAS_TYPE, + modelVersionToVirtualVersion, } from '@kbn/core-saved-objects-base-server-internal'; import { DocumentMigrator } from './document_migrator'; import { TransformSavedObjectDocumentError } from '../core/transform_saved_object_document_error'; @@ -49,6 +54,17 @@ const createRegistry = (...types: Array>) => { return registry; }; +const createDoc = ( + parts: Partial> = {} +): SavedObjectUnsanitizedDoc => ({ + id: 'test-doc', + type: 'test-type', + attributes: {}, + references: [], + coreMigrationVersion: kibanaVersion, + ...parts, +}); + beforeEach(() => { mockGetConvertedObjectId.mockClear(); validateTypeMigrationsMock.mockReset(); @@ -1326,6 +1342,156 @@ describe('DocumentMigrator', () => { }); }); }); + + describe('down transformation', () => { + let migrator: DocumentMigrator; + let transforms: Record>>; + + const createTransformFn = ( + impl?: SavedObjectModelTransformationFn + ): jest.MockedFunction> => { + const defaultImpl: SavedObjectModelTransformationFn = (doc) => ({ + document: doc, + }); + return jest.fn().mockImplementation(impl ?? defaultImpl); + }; + + const transformCall = (num: number) => transforms[num].mock.invocationCallOrder[0]; + + beforeEach(() => { + const migrate1 = createTransformFn((doc) => { + doc.attributes.wentThoughDown1 = true; + return { document: doc }; + }); + const migrate2 = createTransformFn((doc) => { + doc.attributes.wentThoughDown2 = true; + return { document: doc }; + }); + const migrate3 = createTransformFn((doc) => { + doc.attributes.wentThoughDown3 = true; + return { document: doc }; + }); + + transforms = { + 1: migrate1, + 2: migrate2, + 3: migrate3, + }; + + migrator = new DocumentMigrator({ + ...testOpts(), + typeRegistry: createRegistry({ + name: 'down-test', + switchToModelVersionAt: '8.0.0', + modelVersions: { + 1: { + modelChange: { + type: 'expansion', + transformation: { up: jest.fn(), down: migrate1 }, + }, + }, + 2: { + modelChange: { + type: 'expansion', + transformation: { up: jest.fn(), down: migrate2 }, + }, + }, + 3: { + modelChange: { + type: 'expansion', + transformation: { up: jest.fn(), down: migrate3 }, + }, + }, + }, + }), + }); + migrator.prepareMigrations(); + }); + + it('applies the expected conversion', () => { + const document = createDoc({ + type: 'down-test', + typeMigrationVersion: modelVersionToVirtualVersion(2), + }); + + const result = migrator.transformDown(document, { + targetTypeVersion: modelVersionToVirtualVersion(1), + }); + + expect(result.typeMigrationVersion).toEqual(modelVersionToVirtualVersion(1)); + expect(result.attributes).toEqual({ + wentThoughDown2: true, + }); + }); + + it('applies all the expected conversions', () => { + const document = createDoc({ + type: 'down-test', + typeMigrationVersion: modelVersionToVirtualVersion(3), + }); + + const result = migrator.transformDown(document, { + targetTypeVersion: modelVersionToVirtualVersion(0), + }); + + expect(result.typeMigrationVersion).toEqual(modelVersionToVirtualVersion(0)); + expect(result.attributes).toEqual({ + wentThoughDown1: true, + wentThoughDown2: true, + wentThoughDown3: true, + }); + }); + + it('applies the conversions in order', () => { + const document = createDoc({ + type: 'down-test', + typeMigrationVersion: modelVersionToVirtualVersion(3), + }); + + const result = migrator.transformDown(document, { + targetTypeVersion: modelVersionToVirtualVersion(0), + }); + + expect(result.typeMigrationVersion).toEqual(modelVersionToVirtualVersion(0)); + + expect(transforms[1]).toHaveBeenCalledTimes(1); + expect(transforms[2]).toHaveBeenCalledTimes(1); + expect(transforms[3]).toHaveBeenCalledTimes(1); + + expect(transformCall(3)).toBeLessThan(transformCall(2)); + expect(transformCall(2)).toBeLessThan(transformCall(1)); + }); + + it('throw when trying to transform to a higher version', () => { + const document = createDoc({ + type: 'down-test', + typeMigrationVersion: modelVersionToVirtualVersion(2), + }); + + expect(() => + migrator.transformDown(document, { + targetTypeVersion: modelVersionToVirtualVersion(3), + }) + ).toThrowErrorMatchingInlineSnapshot( + `"Trying to transform down to a higher version: 10.2.0 to 10.3.0"` + ); + }); + + it('throw when trying to transform a document from a higher version than the max one', () => { + const document = createDoc({ + type: 'down-test', + typeMigrationVersion: modelVersionToVirtualVersion(4), + }); + + expect(() => + migrator.transformDown(document, { + targetTypeVersion: modelVersionToVirtualVersion(2), + }) + ).toThrowErrorMatchingInlineSnapshot( + `"Document \\"test-doc\\" belongs to a more recent version of Kibana [10.4.0] when the last known version is [10.3.0]."` + ); + }); + }); }); function renameAttr(path: string, newPath: string) { diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.test.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.test.ts index 451d23abab800..3041197ec56a2 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.test.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.test.ts @@ -386,4 +386,221 @@ describe('DocumentMigratorPipeline', () => { expect(outputDoc.typeMigrationVersion).toEqual('8.5.0'); }); + + it('ignores convert type transforms', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + coreMigrationVersion: '8.7.0', + typeMigrationVersion: '8.7.0', + }); + + const migrate8_7_0_down = createTransformFn(); + const convert8_7_0_down = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { + transformType: TransformType.Migrate, + version: '8.7.0', + transform: jest.fn(), + transformDown: migrate8_7_0_down, + }, + { + transformType: TransformType.Convert, + version: '8.7.0', + transform: jest.fn(), + transformDown: convert8_7_0_down, + }, + ]); + + const pipeline = new DocumentDowngradePipeline({ + document, + kibanaVersion: '8.8.0', + typeTransforms: fooTransforms, + targetTypeVersion: '8.6.0', + }); + + const { document: outputDoc } = pipeline.run(); + + expect(migrate8_7_0_down).toHaveBeenCalledTimes(1); + expect(convert8_7_0_down).not.toHaveBeenCalled(); + + expect(outputDoc.typeMigrationVersion).toEqual('8.6.0'); + }); + + it('ignores reference type transforms', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + coreMigrationVersion: '8.7.0', + typeMigrationVersion: '8.7.0', + }); + + const migrate8_7_0_down = createTransformFn(); + const reference8_7_0_down = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { + transformType: TransformType.Migrate, + version: '8.7.0', + transform: jest.fn(), + transformDown: migrate8_7_0_down, + }, + { + transformType: TransformType.Reference, + version: '8.7.0', + transform: jest.fn(), + transformDown: reference8_7_0_down, + }, + ]); + + const pipeline = new DocumentDowngradePipeline({ + document, + kibanaVersion: '8.8.0', + typeTransforms: fooTransforms, + targetTypeVersion: '8.6.0', + }); + + const { document: outputDoc } = pipeline.run(); + + expect(migrate8_7_0_down).toHaveBeenCalledTimes(1); + expect(reference8_7_0_down).not.toHaveBeenCalled(); + + expect(outputDoc.typeMigrationVersion).toEqual('8.6.0'); + }); + + it('ignores core type transforms if targetCoreVersion is not specified', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + coreMigrationVersion: '8.7.0', + typeMigrationVersion: '8.7.0', + }); + + const migrate8_7_0_down = createTransformFn(); + const core8_7_0_down = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { + transformType: TransformType.Migrate, + version: '8.7.0', + transform: jest.fn(), + transformDown: migrate8_7_0_down, + }, + { + transformType: TransformType.Core, + version: '8.7.0', + transform: jest.fn(), + transformDown: core8_7_0_down, + }, + ]); + + const pipeline = new DocumentDowngradePipeline({ + document, + kibanaVersion: '8.8.0', + typeTransforms: fooTransforms, + targetTypeVersion: '8.6.0', + }); + + const { document: outputDoc } = pipeline.run(); + + expect(migrate8_7_0_down).toHaveBeenCalledTimes(1); + expect(core8_7_0_down).not.toHaveBeenCalled(); + + expect(outputDoc.typeMigrationVersion).toEqual('8.6.0'); + expect(outputDoc.coreMigrationVersion).toEqual('8.7.0'); + }); + + it('applies core type transforms if targetCoreVersion is specified', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + coreMigrationVersion: '8.7.0', + typeMigrationVersion: '8.7.0', + }); + + const migrate8_7_0_down = createTransformFn(); + const core8_7_0_down = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { + transformType: TransformType.Migrate, + version: '8.7.0', + transform: jest.fn(), + transformDown: migrate8_7_0_down, + }, + { + transformType: TransformType.Core, + version: '8.7.0', + transform: jest.fn(), + transformDown: core8_7_0_down, + }, + ]); + + const pipeline = new DocumentDowngradePipeline({ + document, + kibanaVersion: '8.8.0', + typeTransforms: fooTransforms, + targetTypeVersion: '8.6.0', + targetCoreVersion: '8.6.0', + }); + + const { document: outputDoc } = pipeline.run(); + + expect(migrate8_7_0_down).toHaveBeenCalledTimes(1); + expect(core8_7_0_down).toHaveBeenCalledTimes(1); + + expect(outputDoc.typeMigrationVersion).toEqual('8.6.0'); + expect(outputDoc.coreMigrationVersion).toEqual('8.6.0'); + }); + + it('applies all expected core type transforms when targetCoreVersion is specified', () => { + const document = createDoc({ + id: 'foo-1', + type: 'foo', + coreMigrationVersion: '8.9.0', + typeMigrationVersion: '8.8.0', + }); + + const core8_7_0_down = createTransformFn(); + const core8_8_0_down = createTransformFn(); + const core8_9_0_down = createTransformFn(); + + const fooTransforms = getTypeTransforms([ + { + transformType: TransformType.Core, + version: '8.7.0', + transform: jest.fn(), + transformDown: core8_7_0_down, + }, + { + transformType: TransformType.Core, + version: '8.8.0', + transform: jest.fn(), + transformDown: core8_8_0_down, + }, + { + transformType: TransformType.Core, + version: '8.9.0', + transform: jest.fn(), + transformDown: core8_9_0_down, + }, + ]); + + const pipeline = new DocumentDowngradePipeline({ + document, + kibanaVersion: '8.9.0', + typeTransforms: fooTransforms, + targetTypeVersion: '8.7.0', + targetCoreVersion: '8.7.0', + }); + + const { document: outputDoc } = pipeline.run(); + + expect(core8_7_0_down).not.toHaveBeenCalled(); + expect(core8_8_0_down).toHaveBeenCalledTimes(1); + expect(core8_9_0_down).toHaveBeenCalledTimes(1); + + expect(outputDoc.coreMigrationVersion).toEqual('8.7.0'); + }); }); From a5a43bf3fc0ac6500a80a0a42b0eb400fecb37a8 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 30 Mar 2023 09:48:25 +0200 Subject: [PATCH 13/14] one more smoke test --- .../document_migrator.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.test.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.test.ts index 6209a77c30c24..de44f163ec94e 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.test.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/document_migrator.test.ts @@ -1382,6 +1382,10 @@ describe('DocumentMigrator', () => { ...testOpts(), typeRegistry: createRegistry({ name: 'down-test', + migrations: { + '7.8.0': jest.fn(), + '7.9.0': jest.fn(), + }, switchToModelVersionAt: '8.0.0', modelVersions: { 1: { @@ -1491,6 +1495,21 @@ describe('DocumentMigrator', () => { `"Document \\"test-doc\\" belongs to a more recent version of Kibana [10.4.0] when the last known version is [10.3.0]."` ); }); + + it('throw when trying to transform to a version without down conversion', () => { + const document = createDoc({ + type: 'down-test', + typeMigrationVersion: modelVersionToVirtualVersion(2), + }); + + expect(() => + migrator.transformDown(document, { + targetTypeVersion: '7.8.0', + }) + ).toThrowErrorMatchingInlineSnapshot( + `"Could not apply transformation migrate:7.9.0: no down conversion registered"` + ); + }); }); }); From 8cfd32697471c8e1f45cc3e37e398b3a8370f95a Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 3 Apr 2023 07:49:24 +0200 Subject: [PATCH 14/14] review comments --- .../src/document_migrator/pipelines/downgrade_pipeline.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.ts b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.ts index 63d9930af14f3..95af2a3022598 100644 --- a/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.ts +++ b/packages/core/saved-objects/core-saved-objects-migration-server-internal/src/document_migrator/pipelines/downgrade_pipeline.ts @@ -6,7 +6,6 @@ * Side Public License, v 1. */ -import Boom from '@hapi/boom'; import { cloneDeep } from 'lodash'; import Semver from 'semver'; import type { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server'; @@ -108,9 +107,8 @@ export class DocumentDowngradePipeline implements MigrationPipeline { const latestVersion = this.typeTransforms.latestVersion.migrate; if (currentVersion && Semver.gt(currentVersion, latestVersion)) { - throw Boom.badData( - `Document "${id}" belongs to a more recent version of Kibana [${currentVersion}] when the last known version is [${latestVersion}].`, - this.document + throw new Error( + `Document "${id}" belongs to a more recent version of Kibana [${currentVersion}] when the last known version is [${latestVersion}].` ); }