Skip to content

Commit

Permalink
[Cases] Mark lens migrations as deferred (elastic#159291)
Browse files Browse the repository at this point in the history 
## Summary

In Cases, users can add lens visualizations in a case. The lens's
attributes persist inside the `cases-comments` saved object. If the lens
team adds a migration a migration will run for the `cases-comments` SOs
to migrate any lens visualization inside the case comment. In this PR we
defer the lens migrations to be done on read when it is fetched by
users. This way, the cases comments will not be migrated at Kibana
startup.

Blocker for elastic#158468

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
@cnasikas @kibanamachine @saarikabhasi
2 people authored and saarikabhasi committed Jun 14, 2023
1 parent d69742f commit 3e89f27
Show file tree
Hide file tree
Showing 5 changed files with 164 additions and 51 deletions.
68 changes: 63 additions & 5 deletions x-pack/plugins/cases/server/saved_object_types/migrations/comments.test.ts
View file
Expand Up @@ -32,6 +32,13 @@ import type { MigrateFunction, MigrateFunctionsObject } from '@kbn/kibana-utils-
import type { SerializableRecord } from '@kbn/utility-types';
import { GENERATED_ALERT, SUB_CASE_SAVED_OBJECT } from './constants';
import { PersistableStateAttachmentTypeRegistry } from '../../attachment_framework/persistable_state_registry';
import { omit, partition } from 'lodash';
import type {
SavedObjectMigrationFn,
SavedObjectMigrationMap,
SavedObjectMigrationParams,
} from '@kbn/core-saved-objects-server';
import { gte } from 'semver';

describe('comments migrations', () => {
const contextMock = savedObjectsServiceMock.createMigrationContext();
Expand Down Expand Up @@ -108,7 +115,7 @@ describe('comments migrations', () => {
jest.clearAllMocks();
});

describe('lens embeddable migrations for by value panels', () => {
describe('lens migrations', () => {
describe('7.14.0 remove time zone from Lens visualization date histogram', () => {
const expectedLensVisualizationMigrated = {
title: 'MyRenamedOps',
Expand Down Expand Up @@ -271,6 +278,57 @@ describe('comments migrations', () => {
expect((columns[2] as { params: {} }).params).toEqual({ timeZone: 'do not delete' });
});
});

it('should marked lens migrations as deferred correctly', () => {
const lensEmbeddableFactory = makeLensEmbeddableFactory(
() => ({}),
() => ({}),
{}
);

const lensMigrations = lensEmbeddableFactory().migrations;
const lensMigrationObject =
typeof lensMigrations === 'function' ? lensMigrations() : lensMigrations || {};
const lensMigrationObjectWithFakeMigration = {
...lensMigrationObject,
'8.9.0': jest.fn(),
'8.10.0': jest.fn(),
};

const lensVersions = Object.keys(lensMigrationObjectWithFakeMigration);
const [lensVersionToBeDeferred, lensVersionToNotBeDeferred] = partition(
lensVersions,
(version) => gte(version, '8.9.0')
);

const migrations = createCommentsMigrations({
persistableStateAttachmentTypeRegistry: new PersistableStateAttachmentTypeRegistry(),
lensEmbeddableFactory: () => ({
id: 'test',
migrations: lensMigrationObjectWithFakeMigration,
}),
});

for (const version of lensVersionToBeDeferred) {
const migration = migrations[version] as SavedObjectMigrationParams;
expect(migration.deferred).toBe(true);
expect(migration.transform).toEqual(expect.any(Function));
}

for (const version of lensVersionToNotBeDeferred) {
const migration = migrations[version] as SavedObjectMigrationParams;
expect(migration.deferred).toBe(false);
expect(migration.transform).toEqual(expect.any(Function));
}

const migrationsWithoutLens = omit<SavedObjectMigrationMap>(migrations, lensVersions);

for (const version of Object.keys(migrationsWithoutLens)) {
const migration = migrationsWithoutLens[version] as SavedObjectMigrationFn;

expect(migration).toEqual(expect.any(Function));
}
});
});

describe('handles errors', () => {
Expand Down Expand Up @@ -298,9 +356,9 @@ describe('comments migrations', () => {
};

it('logs an error when it fails to parse invalid json', () => {
const commentMigrationFunction = migrateByValueLensVisualizations(migrationFunction);
const commentMigrationFunction = migrateByValueLensVisualizations(migrationFunction, '0.0.0');

const result = commentMigrationFunction(caseComment, contextMock);
const result = commentMigrationFunction.transform(caseComment, contextMock);
// the comment should remain unchanged when there is an error
expect(result.attributes.comment).toEqual(comment);

Expand All @@ -322,7 +380,7 @@ describe('comments migrations', () => {
describe('mergeSavedObjectMigrationMaps', () => {
it('logs an error when the passed migration functions fails', () => {
const migrationObj1 = {
'1.0.0': migrateByValueLensVisualizations(migrationFunction),
'1.0.0': migrateByValueLensVisualizations(migrationFunction, '1.0.0'),
} as unknown as MigrateFunctionsObject;

const migrationObj2 = {
Expand Down Expand Up @@ -351,7 +409,7 @@ describe('comments migrations', () => {

it('it does not log an error when the migration function does not use the context', () => {
const migrationObj1 = {
'1.0.0': migrateByValueLensVisualizations(migrationFunction),
'1.0.0': migrateByValueLensVisualizations(migrationFunction, '1.0.0'),
} as unknown as MigrateFunctionsObject;

const migrationObj2 = {
Expand Down
85 changes: 50 additions & 35 deletions x-pack/plugins/cases/server/saved_object_types/migrations/comments.ts
View file
Expand Up @@ -11,12 +11,12 @@ import type { MigrateFunction, MigrateFunctionsObject } from '@kbn/kibana-utils-
import type {
SavedObjectUnsanitizedDoc,
SavedObjectSanitizedDoc,
SavedObjectMigrationFn,
SavedObjectMigrationMap,
SavedObjectMigrationContext,
} from '@kbn/core/server';
import { mergeSavedObjectMigrationMaps } from '@kbn/core/server';
import type { LensServerPluginSetup } from '@kbn/lens-plugin/server';
import type { SavedObjectMigrationParams } from '@kbn/core-saved-objects-server';
import { CommentType } from '../../../common/api';
import type { LensMarkdownNode, MarkdownNode } from '../../../common/utils/markdown_plugins/utils';
import {
Expand All @@ -26,8 +26,8 @@ import {
} from '../../../common/utils/markdown_plugins/utils';
import type { SanitizedCaseOwner } from '.';
import { addOwnerToSO } from '.';
import { logError } from './utils';
import { GENERATED_ALERT, SUB_CASE_SAVED_OBJECT } from './constants';
import { isDeferredMigration, logError } from './utils';
import { GENERATED_ALERT, MIN_DEFERRED_KIBANA_VERSION, SUB_CASE_SAVED_OBJECT } from './constants';
import type { PersistableStateAttachmentTypeRegistry } from '../../attachment_framework/persistable_state_registry';

interface UnsanitizedComment {
Expand Down Expand Up @@ -63,8 +63,8 @@ export const createCommentsMigrations = (

const embeddableMigrations = mapValues<
MigrateFunctionsObject,
SavedObjectMigrationFn<{ comment?: string }>
>(lensMigrationObject, migrateByValueLensVisualizations) as MigrateFunctionsObject;
SavedObjectMigrationParams<{ comment?: string }, { comment?: string }>
>(lensMigrationObject, migrateByValueLensVisualizations);

const commentsMigrations = {
'7.11.0': (
Expand Down Expand Up @@ -119,39 +119,54 @@ export const createCommentsMigrations = (
return mergeSavedObjectMigrationMaps(commentsMigrations, embeddableMigrations);
};

export const migrateByValueLensVisualizations =
(migrate: MigrateFunction): SavedObjectMigrationFn<{ comment?: string }, { comment?: string }> =>
(doc: SavedObjectUnsanitizedDoc<{ comment?: string }>, context: SavedObjectMigrationContext) => {
if (doc.attributes.comment == null) {
return doc;
}
export const migrateByValueLensVisualizations = (
migrate: MigrateFunction,
migrationVersion: string
): SavedObjectMigrationParams<{ comment?: string }, { comment?: string }> => {
const deferred = isDeferredMigration(MIN_DEFERRED_KIBANA_VERSION, migrationVersion);

try {
const parsedComment = parseCommentString(doc.attributes.comment);
const migratedComment = parsedComment.children.map((comment) => {
if (isLensMarkdownNode(comment)) {
// casting here because ts complains that comment isn't serializable because LensMarkdownNode
// extends Node which has fields that conflict with SerializableRecord even though it is serializable
return migrate(comment as SerializableRecord) as LensMarkdownNode;
}

return comment;
});

const migratedMarkdown = { ...parsedComment, children: migratedComment };
return {
// @ts-expect-error: remove when core changes the types
deferred,
transform: (
doc: SavedObjectUnsanitizedDoc<{ comment?: string }>,
context: SavedObjectMigrationContext
) => {
if (doc.attributes.comment == null) {
return doc;
}

return {
...doc,
attributes: {
...doc.attributes,
comment: stringifyCommentWithoutTrailingNewline(doc.attributes.comment, migratedMarkdown),
},
};
} catch (error) {
logError({ id: doc.id, context, error, docType: 'comment', docKey: 'comment' });
return doc;
}
try {
const parsedComment = parseCommentString(doc.attributes.comment);
const migratedComment = parsedComment.children.map((comment) => {
if (isLensMarkdownNode(comment)) {
// casting here because ts complains that comment isn't serializable because LensMarkdownNode
// extends Node which has fields that conflict with SerializableRecord even though it is serializable
return migrate(comment as SerializableRecord) as LensMarkdownNode;
}

return comment;
});

const migratedMarkdown = { ...parsedComment, children: migratedComment };

return {
...doc,
attributes: {
...doc.attributes,
comment: stringifyCommentWithoutTrailingNewline(
doc.attributes.comment,
migratedMarkdown
),
},
};
} catch (error) {
logError({ id: doc.id, context, error, docType: 'comment', docKey: 'comment' });
return doc;
}
},
};
};

export const stringifyCommentWithoutTrailingNewline = (
originalComment: string,
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/cases/server/saved_object_types/migrations/constants.ts
View file
Expand Up @@ -12,3 +12,5 @@ export const GENERATED_ALERT = 'generated_alert';
export const COMMENT_ASSOCIATION_TYPE = 'case';
export const CASE_TYPE_INDIVIDUAL = 'individual';
export const SUB_CASE_SAVED_OBJECT = 'cases-sub-case';

export const MIN_DEFERRED_KIBANA_VERSION = '8.9.0';
47 changes: 36 additions & 11 deletions x-pack/plugins/cases/server/saved_object_types/migrations/utils.test.ts
View file
Expand Up @@ -7,7 +7,8 @@

import type { SavedObjectsMigrationLogger } from '@kbn/core/server';
import { migrationMocks } from '@kbn/core/server/mocks';
import { logError } from './utils';
import { MIN_DEFERRED_KIBANA_VERSION } from './constants';
import { isDeferredMigration, logError } from './utils';

describe('migration utils', () => {
const context = migrationMocks.createContext();
Expand All @@ -16,18 +17,19 @@ describe('migration utils', () => {
jest.clearAllMocks();
});

it('logs an error', () => {
const log = context.log as jest.Mocked<SavedObjectsMigrationLogger>;
describe('logError', () => {
it('logs an error', () => {
const log = context.log as jest.Mocked<SavedObjectsMigrationLogger>;

logError({
id: '1',
context,
error: new Error('an error'),
docType: 'a document',
docKey: 'key',
});
logError({
id: '1',
context,
error: new Error('an error'),
docType: 'a document',
docKey: 'key',
});

expect(log.error.mock.calls[0]).toMatchInlineSnapshot(`
expect(log.error.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"Failed to migrate a document with doc id: 1 version: 8.0.0 error: an error",
Object {
Expand All @@ -39,5 +41,28 @@ describe('migration utils', () => {
},
]
`);
});
});

describe('isDeferredMigration', () => {
it('should mark the migration as deferred if the migration version is greater than the Kibana version', () => {
expect(isDeferredMigration(MIN_DEFERRED_KIBANA_VERSION, '8.10.0')).toBe(true);
});

it('should mark the migration as not deferred if the migration version is smaller than the Kibana version', () => {
expect(isDeferredMigration(MIN_DEFERRED_KIBANA_VERSION, '8.8.0')).toBe(false);
});

it('should mark the migration as deferred if the migration version is equal to the Kibana version', () => {
expect(isDeferredMigration(MIN_DEFERRED_KIBANA_VERSION, '8.9.0')).toBe(true);
});

it('should return false if the Kibana version is not a valid semver', () => {
expect(isDeferredMigration('invalid', '8.8.0')).toBe(false);
});

it('should return false if the migration version is not a valid semver', () => {
expect(isDeferredMigration(MIN_DEFERRED_KIBANA_VERSION, 'invalid')).toBe(false);
});
});
});
13 changes: 13 additions & 0 deletions x-pack/plugins/cases/server/saved_object_types/migrations/utils.ts
View file
Expand Up @@ -5,6 +5,9 @@
* 2.0.
*/

import valid from 'semver/functions/valid';
import gte from 'semver/functions/gte';

import type {
LogMeta,
SavedObjectMigrationContext,
Expand Down Expand Up @@ -50,3 +53,13 @@ export function pipeMigrations<T>(...migrations: Array<CaseMigration<T>>): CaseM
return (doc: SavedObjectUnsanitizedDoc<T>) =>
migrations.reduce((migratedDoc, nextMigration) => nextMigration(migratedDoc), doc);
}

export const isDeferredMigration = (
minDeferredKibanaVersion: string,
migrationVersion: string
): boolean =>
Boolean(
valid(migrationVersion) &&
valid(minDeferredKibanaVersion) &&
gte(migrationVersion, minDeferredKibanaVersion)
);

0 comments on commit 3e89f27

Please sign in to comment.