Skip to content

Commit

Permalink
[ZDT] DocumentMigrator: support higher version documents (elastic#157895
Browse files Browse the repository at this point in the history
)

## Summary

Part of elastic#150312
(next steps depend on elastic#153117)

**This PR does two things:**
- introduce the concept of version persistence schema
- adapt the document migrator to support downward migrations for
documents of an higher version.

In the follow-up, we will then update the calls from the SOR to the
document migrator to allow downward conversions when we're using the ZDT
migration algorithm (which requires
elastic#153117 to be merged)

### Model version persistence schema.

*(This is what has also been named 'eviction schema' or 'known fields
schema'.)*

A new `SavedObjectsModelVersion.schemas.backwardConversion` property was
added to the model version definition.

This 'schema' can either be an arbitrary function, or a `schema.object`
from `@kbn/config-schema`

```ts
type SavedObjectModelVersionBackwardConversionSchema<
  InAttrs = unknown,
  OutAttrs = unknown
> = ObjectType | SavedObjectModelVersionBackwardConversionFn<InAttrs, OutAttrs>;
```

When specified for a version, the document's attributes will go thought
this schema during down conversions by the document migrator.

### Adapt the document migrator to support downward migrations for
documents of an higher version.

Add an `allowDowngrade` option to `DocumentMigrator.migrate` and
`KibanaMigrator.migrateDocument`. When this option is set to `true`, the
document migration will accept to 'downgrade' the document if necessary,
instead of throwing an error as done when the option is `false` or
unspecified (which was the only behavior prior to this PR's changes)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
2 people authored and delanni committed May 25, 2023
1 parent 040af58 commit 75d8f47
Show file tree
Hide file tree
Showing 30 changed files with 990 additions and 83 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export type {
KibanaMigratorStatus,
MigrationResult,
MigrationStatus,
MigrateDocumentOptions,
} from './src/migration';
export { parseObjectKey, getObjectKey, getIndexForType } from './src/utils';
export {
Expand All @@ -64,4 +65,5 @@ export {
getModelVersionDelta,
buildModelVersionTransformFn,
aggregateMappingAdditions,
convertModelVersionBackwardConversionSchema,
} from './src/model_version';
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ export type {
KibanaMigratorStatus,
MigrationStatus,
MigrationResult,
MigrateDocumentOptions,
} from './kibana_migrator';
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,22 @@ export interface IKibanaMigrator {
* @param doc - The saved object to migrate
* @returns `doc` with all registered migrations applied.
*/
migrateDocument(doc: SavedObjectUnsanitizedDoc): SavedObjectUnsanitizedDoc;
migrateDocument(
doc: SavedObjectUnsanitizedDoc,
options?: MigrateDocumentOptions
): SavedObjectUnsanitizedDoc;
}

/**
* Options for {@link IKibanaMigrator.migrateDocument}
* @internal
*/
export interface MigrateDocumentOptions {
/**
* Defines whether it is allowed to convert documents from an higher version or not.
* Defaults to `false`.
*/
allowDowngrade?: boolean;
}

/** @internal */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/*
* 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 { schema } from '@kbn/config-schema';
import { convertModelVersionBackwardConversionSchema } from './backward_conversion_schema';
import type {
SavedObjectUnsanitizedDoc,
SavedObjectModelVersionForwardCompatibilityFn,
} from '@kbn/core-saved-objects-server';

describe('convertModelVersionBackwardConversionSchema', () => {
const createDoc = (
parts: Partial<SavedObjectUnsanitizedDoc<any>>
): SavedObjectUnsanitizedDoc<any> => ({
id: 'id',
type: 'type',
attributes: {},
...parts,
});

describe('using functions', () => {
it('converts the schema', () => {
const conversionSchema: jest.MockedFunction<SavedObjectModelVersionForwardCompatibilityFn> =
jest.fn();
conversionSchema.mockImplementation((attrs) => attrs);

const doc = createDoc({ attributes: { foo: 'bar' } });
const converted = convertModelVersionBackwardConversionSchema(conversionSchema);

const output = converted(doc);

expect(conversionSchema).toHaveBeenCalledTimes(1);
expect(conversionSchema).toHaveBeenCalledWith({ foo: 'bar' });
expect(output).toEqual(doc);
});

it('returns the document with the updated properties', () => {
const conversionSchema: jest.MockedFunction<
SavedObjectModelVersionForwardCompatibilityFn<any, any>
> = jest.fn();
conversionSchema.mockImplementation((attrs) => ({ foo: attrs.foo }));

const doc = createDoc({ attributes: { foo: 'bar', hello: 'dolly' } });
const converted = convertModelVersionBackwardConversionSchema(conversionSchema);

const output = converted(doc);

expect(output).toEqual({
...doc,
attributes: { foo: 'bar' },
});
});

it('throws if the function throws', () => {
const conversionSchema: jest.MockedFunction<
SavedObjectModelVersionForwardCompatibilityFn<any, any>
> = jest.fn();
conversionSchema.mockImplementation(() => {
throw new Error('dang');
});

const doc = createDoc({});
const converted = convertModelVersionBackwardConversionSchema(conversionSchema);

expect(() => converted(doc)).toThrowErrorMatchingInlineSnapshot(`"dang"`);
});
});

describe('using config-schemas', () => {
it('converts the schema', () => {
const conversionSchema = schema.object(
{
foo: schema.maybe(schema.string()),
},
{ unknowns: 'ignore' }
);
const validateSpy = jest.spyOn(conversionSchema, 'validate');

const doc = createDoc({ attributes: { foo: 'bar' } });
const converted = convertModelVersionBackwardConversionSchema(conversionSchema);

const output = converted(doc);

expect(validateSpy).toHaveBeenCalledTimes(1);
expect(validateSpy).toHaveBeenCalledWith({ foo: 'bar' }, {});
expect(output).toEqual(doc);
});

it('returns the document with the updated properties', () => {
const conversionSchema = schema.object(
{
foo: schema.maybe(schema.string()),
},
{ unknowns: 'ignore' }
);

const doc = createDoc({ attributes: { foo: 'bar', hello: 'dolly' } });
const converted = convertModelVersionBackwardConversionSchema(conversionSchema);

const output = converted(doc);

expect(output).toEqual({
...doc,
attributes: {
foo: 'bar',
},
});
});

it('throws if the validation throws', () => {
const conversionSchema = schema.object(
{
foo: schema.maybe(schema.string()),
},
{ unknowns: 'forbid' }
);

const doc = createDoc({ attributes: { foo: 'bar', hello: 'dolly' } });
const converted = convertModelVersionBackwardConversionSchema(conversionSchema);

expect(() => converted(doc)).toThrowErrorMatchingInlineSnapshot(
`"[hello]: definition for this key is missing"`
);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* 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 { isConfigSchema, type ObjectType } from '@kbn/config-schema';
import type {
SavedObjectUnsanitizedDoc,
SavedObjectModelVersionForwardCompatibilitySchema,
} from '@kbn/core-saved-objects-server';

function isObjectType(
schema: SavedObjectModelVersionForwardCompatibilitySchema
): schema is ObjectType {
return isConfigSchema(schema);
}

export type ConvertedSchema = (doc: SavedObjectUnsanitizedDoc) => SavedObjectUnsanitizedDoc;

export const convertModelVersionBackwardConversionSchema = (
schema: SavedObjectModelVersionForwardCompatibilitySchema
): ConvertedSchema => {
if (isObjectType(schema)) {
return (doc) => {
const attrs = schema.validate(doc.attributes, {});
return {
...doc,
attributes: attrs,
};
};
} else {
return (doc) => {
const attrs = schema(doc.attributes);
return {
...doc,
attributes: attrs,
};
};
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@ export {
export { getModelVersionDelta } from './get_version_delta';
export { buildModelVersionTransformFn } from './build_transform_fn';
export { aggregateMappingAdditions } from './aggregate_model_changes';
export { convertModelVersionBackwardConversionSchema } from './backward_conversion_schema';
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ jest.doMock('./internal_transforms', () => ({
}));

export const getModelVersionTransformsMock = jest.fn();
export const getModelVersionSchemasMock = jest.fn();

jest.doMock('./model_version', () => ({
getModelVersionTransforms: getModelVersionTransformsMock,
getModelVersionSchemas: getModelVersionSchemasMock,
}));

export const validateTypeMigrationsMock = jest.fn();
Expand All @@ -33,5 +35,6 @@ export const resetAllMocks = () => {
getReferenceTransformsMock.mockReset().mockReturnValue([]);
getConversionTransformsMock.mockReset().mockReturnValue([]);
getModelVersionTransformsMock.mockReset().mockReturnValue([]);
getModelVersionSchemasMock.mockReset().mockReturnValue({});
validateTypeMigrationsMock.mockReset();
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
getCoreTransformsMock,
getConversionTransformsMock,
getModelVersionTransformsMock,
getModelVersionSchemasMock,
getReferenceTransformsMock,
resetAllMocks,
validateTypeMigrationsMock,
Expand Down Expand Up @@ -195,6 +196,60 @@ describe('buildActiveMigrations', () => {
});
});

describe('model version schemas', () => {
it('calls getModelVersionSchemas with the correct parameters', () => {
const foo = createType({ name: 'foo' });
const bar = createType({ name: 'bar' });

addType(foo);
addType(bar);

buildMigrations();

expect(getModelVersionSchemasMock).toHaveBeenCalledTimes(2);
expect(getModelVersionSchemasMock).toHaveBeenNthCalledWith(1, {
typeDefinition: foo,
});
expect(getModelVersionSchemasMock).toHaveBeenNthCalledWith(2, {
typeDefinition: bar,
});
});

it('adds the schemas from getModelVersionSchemas to each type', () => {
const foo = createType({ name: 'foo' });
const bar = createType({ name: 'bar' });

addType(foo);
addType(bar);

getModelVersionSchemasMock.mockImplementation(
({ typeDefinition }: { typeDefinition: SavedObjectsType }) => {
if (typeDefinition.name === 'foo') {
return {
'7.10.0': jest.fn(),
};
} else {
return {
'8.3.0': jest.fn(),
'8.4.0': jest.fn(),
};
}
}
);

const migrations = buildMigrations();

expect(Object.keys(migrations).sort()).toEqual(['bar', 'foo']);
expect(migrations.foo.versionSchemas).toEqual({
'7.10.0': expect.any(Function),
});
expect(migrations.bar.versionSchemas).toEqual({
'8.3.0': expect.any(Function),
'8.4.0': expect.any(Function),
});
});
});

describe('internal transforms', () => {
it('calls getReferenceTransforms with the correct parameters', () => {
const foo = createType({ name: 'foo' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
} from './internal_transforms';
import { validateTypeMigrations } from './validate_migrations';
import { transformComparator, convertMigrationFunction } from './utils';
import { getModelVersionTransforms } from './model_version';
import { getModelVersionTransforms, getModelVersionSchemas } from './model_version';

/**
* Converts migrations from a format that is convenient for callers to a format that
Expand Down Expand Up @@ -48,7 +48,7 @@ export function buildActiveMigrations({
referenceTransforms,
});

if (typeTransforms.transforms.length) {
if (typeTransforms.transforms.length || Object.keys(typeTransforms.versionSchemas).length) {
migrations[type.name] = typeTransforms;
}

Expand Down Expand Up @@ -90,6 +90,8 @@ const buildTypeTransforms = ({
...modelVersionTransforms,
].sort(transformComparator);

const modelVersionSchemas = getModelVersionSchemas({ typeDefinition: type });

return {
immediateVersion: _.chain(transforms)
.groupBy('transformType')
Expand All @@ -106,5 +108,8 @@ const buildTypeTransforms = ({
.mapValues((items) => _.last(items)?.version)
.value() as Record<TransformType, string>,
transforms,
versionSchemas: {
...modelVersionSchemas,
},
};
};
Loading

0 comments on commit 75d8f47

Please sign in to comment.