From cd774b49fa468b23ecd162c502562080c8968c93 Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Thu, 5 Aug 2021 11:28:44 -0400 Subject: [PATCH] Add SavedObjectsUtils.getConvertedObjectId function Also change document migrator code to use this function instead of using uuidv5 directly. --- ....savedobjectsutils.getconvertedobjectid.md | 28 ++++++++ ...na-plugin-core-server.savedobjectsutils.md | 1 + .../core/document_migrator.test.mock.ts | 21 ++++++ .../migrations/core/document_migrator.test.ts | 66 ++++++++++++------- .../migrations/core/document_migrator.ts | 8 ++- .../lib/utils.test.mock.ts} | 5 +- .../saved_objects/service/lib/utils.test.ts | 28 ++++++-- .../server/saved_objects/service/lib/utils.ts | 20 +++++- src/core/server/server.api.md | 1 + 9 files changed, 142 insertions(+), 36 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.getconvertedobjectid.md create mode 100644 src/core/server/saved_objects/migrations/core/document_migrator.test.mock.ts rename src/core/server/saved_objects/{migrations/core/__mocks__/index.ts => service/lib/utils.test.mock.ts} (80%) diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.getconvertedobjectid.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.getconvertedobjectid.md new file mode 100644 index 0000000000000..c6a429d345ed1 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.getconvertedobjectid.md @@ -0,0 +1,28 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsUtils](./kibana-plugin-core-server.savedobjectsutils.md) > [getConvertedObjectId](./kibana-plugin-core-server.savedobjectsutils.getconvertedobjectid.md) + +## SavedObjectsUtils.getConvertedObjectId() method + +Uses a single-namespace object's "legacy ID" to determine what its new ID will be after it is converted to a multi-namespace type. + +Signature: + +```typescript +static getConvertedObjectId(namespace: string | undefined, type: string, id: string): string; +``` + +## Parameters + +| Parameter | Type | Description | +| --- | --- | --- | +| namespace | string | undefined | | +| type | string | | +| id | string | | + +Returns: + +`string` + +{string} The ID of the saved object after it is converted. + diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.md index 0148621e757b7..ab6382aca6a52 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.md @@ -24,5 +24,6 @@ export declare class SavedObjectsUtils | Method | Modifiers | Description | | --- | --- | --- | | [generateId()](./kibana-plugin-core-server.savedobjectsutils.generateid.md) | static | Generates a random ID for a saved objects. | +| [getConvertedObjectId(namespace, type, id)](./kibana-plugin-core-server.savedobjectsutils.getconvertedobjectid.md) | static | Uses a single-namespace object's "legacy ID" to determine what its new ID will be after it is converted to a multi-namespace type. | | [isRandomId(id)](./kibana-plugin-core-server.savedobjectsutils.israndomid.md) | static | Validates that a saved object ID has been randomly generated. | diff --git a/src/core/server/saved_objects/migrations/core/document_migrator.test.mock.ts b/src/core/server/saved_objects/migrations/core/document_migrator.test.mock.ts new file mode 100644 index 0000000000000..ee0b18af5ac0d --- /dev/null +++ b/src/core/server/saved_objects/migrations/core/document_migrator.test.mock.ts @@ -0,0 +1,21 @@ +/* + * 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. + */ + +const mockGetConvertedObjectId = jest.fn().mockReturnValue('uuidv5'); +jest.mock('../../service/lib/utils', () => { + const actual = jest.requireActual('../../service/lib/utils'); + return { + ...actual, + SavedObjectsUtils: { + ...actual.SavedObjectsUtils, + getConvertedObjectId: mockGetConvertedObjectId, + }, + }; +}); + +export { mockGetConvertedObjectId }; diff --git a/src/core/server/saved_objects/migrations/core/document_migrator.test.ts b/src/core/server/saved_objects/migrations/core/document_migrator.test.ts index 87b8ee0809064..64c1c4ce2fa9f 100644 --- a/src/core/server/saved_objects/migrations/core/document_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/core/document_migrator.test.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import { mockUuidv5 } from './__mocks__'; +import { mockGetConvertedObjectId } from './document_migrator.test.mock'; import { set } from '@elastic/safer-lodash-set'; import _ from 'lodash'; import { SavedObjectUnsanitizedDoc } from '../../serialization'; @@ -37,7 +37,7 @@ const createRegistry = (...types: Array>) => { }; beforeEach(() => { - mockUuidv5.mockClear(); + mockGetConvertedObjectId.mockClear(); }); describe('DocumentMigrator', () => { @@ -866,7 +866,7 @@ describe('DocumentMigrator', () => { namespace: 'foo-namespace', }; const actual = migrator.migrate(obj); - expect(mockUuidv5).not.toHaveBeenCalled(); + expect(mockGetConvertedObjectId).not.toHaveBeenCalled(); expect(actual).toEqual({ id: 'cowardly', type: 'dog', @@ -898,7 +898,7 @@ describe('DocumentMigrator', () => { it('in the default space', () => { const actual = migrator.migrateAndConvert(obj); - expect(mockUuidv5).not.toHaveBeenCalled(); + expect(mockGetConvertedObjectId).not.toHaveBeenCalled(); expect(actual).toEqual([ { id: 'bad', @@ -912,8 +912,8 @@ describe('DocumentMigrator', () => { it('in a non-default space', () => { const actual = migrator.migrateAndConvert({ ...obj, namespace: 'foo-namespace' }); - expect(mockUuidv5).toHaveBeenCalledTimes(1); - expect(mockUuidv5).toHaveBeenCalledWith('foo-namespace:toy:favorite', 'DNSUUID'); + expect(mockGetConvertedObjectId).toHaveBeenCalledTimes(1); + expect(mockGetConvertedObjectId).toHaveBeenCalledWith('foo-namespace', 'toy', 'favorite'); expect(actual).toEqual([ { id: 'bad', @@ -946,7 +946,7 @@ describe('DocumentMigrator', () => { it('in the default space', () => { const actual = migrator.migrateAndConvert(obj); - expect(mockUuidv5).not.toHaveBeenCalled(); + expect(mockGetConvertedObjectId).not.toHaveBeenCalled(); expect(actual).toEqual([ { id: 'loud', @@ -961,8 +961,8 @@ describe('DocumentMigrator', () => { it('in a non-default space', () => { const actual = migrator.migrateAndConvert({ ...obj, namespace: 'foo-namespace' }); - expect(mockUuidv5).toHaveBeenCalledTimes(1); - expect(mockUuidv5).toHaveBeenCalledWith('foo-namespace:dog:loud', 'DNSUUID'); + expect(mockGetConvertedObjectId).toHaveBeenCalledTimes(1); + expect(mockGetConvertedObjectId).toHaveBeenCalledWith('foo-namespace', 'dog', 'loud'); expect(actual).toEqual([ { id: 'uuidv5', @@ -1008,7 +1008,7 @@ describe('DocumentMigrator', () => { it('in the default space', () => { const actual = migrator.migrateAndConvert(obj); - expect(mockUuidv5).not.toHaveBeenCalled(); + expect(mockGetConvertedObjectId).not.toHaveBeenCalled(); expect(actual).toEqual([ { id: 'cute', @@ -1024,9 +1024,19 @@ describe('DocumentMigrator', () => { it('in a non-default space', () => { const actual = migrator.migrateAndConvert({ ...obj, namespace: 'foo-namespace' }); - expect(mockUuidv5).toHaveBeenCalledTimes(2); - expect(mockUuidv5).toHaveBeenNthCalledWith(1, 'foo-namespace:toy:favorite', 'DNSUUID'); - expect(mockUuidv5).toHaveBeenNthCalledWith(2, 'foo-namespace:dog:cute', 'DNSUUID'); + expect(mockGetConvertedObjectId).toHaveBeenCalledTimes(2); + expect(mockGetConvertedObjectId).toHaveBeenNthCalledWith( + 1, + 'foo-namespace', + 'toy', + 'favorite' + ); + expect(mockGetConvertedObjectId).toHaveBeenNthCalledWith( + 2, + 'foo-namespace', + 'dog', + 'cute' + ); expect(actual).toEqual([ { id: 'uuidv5', @@ -1080,7 +1090,7 @@ describe('DocumentMigrator', () => { it('in the default space', () => { const actual = migrator.migrateAndConvert(obj); - expect(mockUuidv5).not.toHaveBeenCalled(); + expect(mockGetConvertedObjectId).not.toHaveBeenCalled(); expect(actual).toEqual([ { id: 'sleepy', @@ -1095,8 +1105,8 @@ describe('DocumentMigrator', () => { it('in a non-default space', () => { const actual = migrator.migrateAndConvert({ ...obj, namespace: 'foo-namespace' }); - expect(mockUuidv5).toHaveBeenCalledTimes(1); - expect(mockUuidv5).toHaveBeenCalledWith('foo-namespace:toy:favorite', 'DNSUUID'); + expect(mockGetConvertedObjectId).toHaveBeenCalledTimes(1); + expect(mockGetConvertedObjectId).toHaveBeenCalledWith('foo-namespace', 'toy', 'favorite'); expect(actual).toEqual([ { id: 'sleepy', @@ -1134,7 +1144,7 @@ describe('DocumentMigrator', () => { it('in the default space', () => { const actual = migrator.migrateAndConvert(obj); - expect(mockUuidv5).not.toHaveBeenCalled(); + expect(mockGetConvertedObjectId).not.toHaveBeenCalled(); expect(actual).toEqual([ { id: 'hungry', @@ -1149,8 +1159,8 @@ describe('DocumentMigrator', () => { it('in a non-default space', () => { const actual = migrator.migrateAndConvert({ ...obj, namespace: 'foo-namespace' }); - expect(mockUuidv5).toHaveBeenCalledTimes(1); - expect(mockUuidv5).toHaveBeenCalledWith('foo-namespace:dog:hungry', 'DNSUUID'); + expect(mockGetConvertedObjectId).toHaveBeenCalledTimes(1); + expect(mockGetConvertedObjectId).toHaveBeenCalledWith('foo-namespace', 'dog', 'hungry'); expect(actual).toEqual([ { id: 'uuidv5', @@ -1204,7 +1214,7 @@ describe('DocumentMigrator', () => { it('in the default space', () => { const actual = migrator.migrateAndConvert(obj); - expect(mockUuidv5).not.toHaveBeenCalled(); + expect(mockGetConvertedObjectId).not.toHaveBeenCalled(); expect(actual).toEqual([ { id: 'pretty', @@ -1220,9 +1230,19 @@ describe('DocumentMigrator', () => { it('in a non-default space', () => { const actual = migrator.migrateAndConvert({ ...obj, namespace: 'foo-namespace' }); - expect(mockUuidv5).toHaveBeenCalledTimes(2); - expect(mockUuidv5).toHaveBeenNthCalledWith(1, 'foo-namespace:toy:favorite', 'DNSUUID'); - expect(mockUuidv5).toHaveBeenNthCalledWith(2, 'foo-namespace:dog:pretty', 'DNSUUID'); + expect(mockGetConvertedObjectId).toHaveBeenCalledTimes(2); + expect(mockGetConvertedObjectId).toHaveBeenNthCalledWith( + 1, + 'foo-namespace', + 'toy', + 'favorite' + ); + expect(mockGetConvertedObjectId).toHaveBeenNthCalledWith( + 2, + 'foo-namespace', + 'dog', + 'pretty' + ); expect(actual).toEqual([ { id: 'uuidv5', diff --git a/src/core/server/saved_objects/migrations/core/document_migrator.ts b/src/core/server/saved_objects/migrations/core/document_migrator.ts index 23f5d075d72e3..040aa81cdab3a 100644 --- a/src/core/server/saved_objects/migrations/core/document_migrator.ts +++ b/src/core/server/saved_objects/migrations/core/document_migrator.ts @@ -65,7 +65,7 @@ import { MigrationLogger } from './migration_logger'; import { TransformSavedObjectDocumentError } from '.'; import { ISavedObjectTypeRegistry } from '../../saved_objects_type_registry'; import { SavedObjectMigrationFn, SavedObjectMigrationMap } from '../types'; -import { DEFAULT_NAMESPACE_STRING } from '../../service/lib/utils'; +import { DEFAULT_NAMESPACE_STRING, SavedObjectsUtils } from '../../service/lib/utils'; import { LegacyUrlAlias, LEGACY_URL_ALIAS_TYPE } from '../../object_types'; const DEFAULT_MINIMUM_CONVERT_VERSION = '8.0.0'; @@ -556,7 +556,7 @@ function convertNamespaceType(doc: SavedObjectUnsanitizedDoc) { } const { id: originId, type } = otherAttrs; - const id = deterministicallyRegenerateObjectId(namespace, type, originId!); + const id = SavedObjectsUtils.getConvertedObjectId(namespace, type, originId!); if (namespace !== undefined) { const legacyUrlAlias: SavedObjectUnsanitizedDoc = { id: `${namespace}:${type}:${originId}`, @@ -616,7 +616,9 @@ function getReferenceTransforms(typeRegistry: ISavedObjectTypeRegistry): Transfo references: references.map(({ type, id, ...attrs }) => ({ ...attrs, type, - id: types.has(type) ? deterministicallyRegenerateObjectId(namespace, type, id) : id, + id: types.has(type) + ? SavedObjectsUtils.getConvertedObjectId(namespace, type, id) + : id, })), }, additionalDocs: [], diff --git a/src/core/server/saved_objects/migrations/core/__mocks__/index.ts b/src/core/server/saved_objects/service/lib/utils.test.mock.ts similarity index 80% rename from src/core/server/saved_objects/migrations/core/__mocks__/index.ts rename to src/core/server/saved_objects/service/lib/utils.test.mock.ts index d290bc57e2669..9e2b5c66bc91a 100644 --- a/src/core/server/saved_objects/migrations/core/__mocks__/index.ts +++ b/src/core/server/saved_objects/service/lib/utils.test.mock.ts @@ -6,8 +6,11 @@ * Side Public License, v 1. */ +const mockUuidv1 = jest.fn().mockReturnValue('uuidv1'); +jest.mock('uuid/v1', () => mockUuidv1); + const mockUuidv5 = jest.fn().mockReturnValue('uuidv5'); Object.defineProperty(mockUuidv5, 'DNS', { value: 'DNSUUID', writable: false }); jest.mock('uuid/v5', () => mockUuidv5); -export { mockUuidv5 }; +export { mockUuidv1, mockUuidv5 }; diff --git a/src/core/server/saved_objects/service/lib/utils.test.ts b/src/core/server/saved_objects/service/lib/utils.test.ts index 7101eafc72a32..0f502d8bfb9f1 100644 --- a/src/core/server/saved_objects/service/lib/utils.test.ts +++ b/src/core/server/saved_objects/service/lib/utils.test.ts @@ -6,14 +6,11 @@ * Side Public License, v 1. */ -import uuid from 'uuid'; +import { mockUuidv1, mockUuidv5 } from './utils.test.mock'; + import { SavedObjectsFindOptions } from '../../types'; import { SavedObjectsUtils } from './utils'; -jest.mock('uuid', () => ({ - v1: jest.fn().mockReturnValue('mock-uuid'), -})); - describe('SavedObjectsUtils', () => { const { namespaceIdToString, @@ -21,6 +18,7 @@ describe('SavedObjectsUtils', () => { createEmptyFindResponse, generateId, isRandomId, + getConvertedObjectId, } = SavedObjectsUtils; describe('#namespaceIdToString', () => { @@ -80,8 +78,8 @@ describe('SavedObjectsUtils', () => { describe('#generateId', () => { it('returns a valid uuid', () => { - expect(generateId()).toBe('mock-uuid'); - expect(uuid.v1).toHaveBeenCalled(); + expect(generateId()).toBe('uuidv1'); // default return value for mockUuidv1 + expect(mockUuidv1).toHaveBeenCalled(); }); }); @@ -93,4 +91,20 @@ describe('SavedObjectsUtils', () => { expect(isRandomId(undefined)).toBe(false); }); }); + + describe('#getConvertedObjectId', () => { + it('does not change the object ID if namespace is undefined or "default"', () => { + for (const namespace of [undefined, 'default']) { + const result = getConvertedObjectId(namespace, 'type', 'oldId'); + expect(result).toBe('oldId'); + expect(mockUuidv5).not.toHaveBeenCalled(); + } + }); + + it('changes the object ID if namespace is defined and not "default"', () => { + const result = getConvertedObjectId('namespace', 'type', 'oldId'); + expect(result).toBe('uuidv5'); // default return value for mockUuidv5 + expect(mockUuidv5).toHaveBeenCalledWith('namespace:type:oldId', 'DNSUUID'); + }); + }); }); diff --git a/src/core/server/saved_objects/service/lib/utils.ts b/src/core/server/saved_objects/service/lib/utils.ts index 494ac6ce9fad5..6942b3b376232 100644 --- a/src/core/server/saved_objects/service/lib/utils.ts +++ b/src/core/server/saved_objects/service/lib/utils.ts @@ -6,7 +6,8 @@ * Side Public License, v 1. */ -import uuid from 'uuid'; +import uuidv1 from 'uuid/v1'; +import uuidv5 from 'uuid/v5'; import { SavedObjectsFindOptions } from '../../types'; import { SavedObjectsFindResponse } from '..'; @@ -65,7 +66,7 @@ export class SavedObjectsUtils { * Generates a random ID for a saved objects. */ public static generateId() { - return uuid.v1(); + return uuidv1(); } /** @@ -77,4 +78,19 @@ export class SavedObjectsUtils { public static isRandomId(id: string | undefined) { return typeof id === 'string' && UUID_REGEX.test(id); } + + /** + * Uses a single-namespace object's "legacy ID" to determine what its new ID will be after it is converted to a multi-namespace type. + * + * @param {string} namespace The namespace of the saved object before it is converted. + * @param {string} type The type of the saved object before it is converted. + * @param {string} id The ID of the saved object before it is converted. + * @returns {string} The ID of the saved object after it is converted. + */ + public static getConvertedObjectId(namespace: string | undefined, type: string, id: string) { + if (SavedObjectsUtils.namespaceIdToString(namespace) === DEFAULT_NAMESPACE_STRING) { + return id; // Objects that exist in the Default space do not get new IDs when they are converted. + } + return uuidv5(`${namespace}:${type}:${id}`, uuidv5.DNS); // The uuidv5 namespace constant (uuidv5.DNS) is arbitrary. + } } diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index fbc51acfdc8ef..7f2ce38a5bdd4 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -3163,6 +3163,7 @@ export interface SavedObjectsUpdateResponse extends Omit({ page, perPage, }: SavedObjectsFindOptions) => SavedObjectsFindResponse; static generateId(): string; + static getConvertedObjectId(namespace: string | undefined, type: string, id: string): string; static isRandomId(id: string | undefined): boolean; static namespaceIdToString: (namespace?: string | undefined) => string; static namespaceStringToId: (namespace: string) => string | undefined;