From 292931a29ee4bd3edce887dada1119d1e8848746 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 18 Oct 2023 16:22:34 +0800 Subject: [PATCH] refact: move workspace specific logic to savedObjectWrapper Signed-off-by: SuZhou-Joe --- .../saved_objects/service/lib/repository.ts | 90 +----- .../service/saved_objects_client.ts | 8 + src/plugins/workspace/common/constants.ts | 2 + src/plugins/workspace/server/plugin.ts | 12 + ...pper_for_check_workspace_conflict.test.ts} | 14 +- ...ts_wrapper_for_check_workspace_conflict.ts | 257 ++++++++++++++++++ 6 files changed, 297 insertions(+), 86 deletions(-) rename src/{core/server/saved_objects/service/lib/integration_tests/repository.test.ts => plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts} (96%) create mode 100644 src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 45997e6cf6b9..15e32336eb3b 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -280,29 +280,6 @@ export class SavedObjectsRepository { } } - let savedObjectWorkspaces = workspaces; - - if (id && overwrite && workspaces) { - let currentItem; - try { - currentItem = await this.get(type, id); - } catch (e) { - // this.get will throw an error when no items can be found - } - if (currentItem) { - if ( - SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( - workspaces, - currentItem.workspaces - ).length - ) { - throw SavedObjectsErrorHelpers.createConflictError(type, id); - } else { - savedObjectWorkspaces = currentItem.workspaces; - } - } - } - const migrated = this._migrator.migrateDocument({ id, type, @@ -313,7 +290,7 @@ export class SavedObjectsRepository { migrationVersion, updated_at: time, ...(Array.isArray(references) && { references }), - ...(Array.isArray(savedObjectWorkspaces) && { workspaces: savedObjectWorkspaces }), + ...(Array.isArray(workspaces) && { workspaces }), }); const raw = this._serializer.savedObjectToRaw(migrated as SavedObjectSanitizedDoc); @@ -380,16 +357,12 @@ export class SavedObjectsRepository { const method = object.id && overwrite ? 'index' : 'create'; const requiresNamespacesCheck = object.id && this._registry.isMultiNamespace(object.type); - /** - * It requires a check when overwriting objects to target workspaces - */ - const requiresWorkspaceCheck = !!(object.id && options.workspaces); if (object.id == null) object.id = uuid.v1(); let opensearchRequestIndexPayload = {}; - if (requiresNamespacesCheck || requiresWorkspaceCheck) { + if (requiresNamespacesCheck) { opensearchRequestIndexPayload = { opensearchRequestIndex: bulkGetRequestIndexCounter, }; @@ -412,7 +385,7 @@ export class SavedObjectsRepository { .map(({ value: { object: { type, id } } }) => ({ _id: this._serializer.generateRawId(namespace, type, id), _index: this.getIndexForType(type), - _source: ['type', 'namespaces', 'workspaces'], + _source: ['type', 'namespaces'], })); const bulkGetResponse = bulkGetDocs.length ? await this.client.mget( @@ -479,45 +452,7 @@ export class SavedObjectsRepository { let savedObjectWorkspaces: string[] | undefined = options.workspaces; if (expectedBulkGetResult.value.method !== 'create') { - const rawId = this._serializer.generateRawId(namespace, object.type, object.id); - const findObject = - bulkGetResponse?.statusCode !== 404 - ? bulkGetResponse?.body.docs?.find((item) => item._id === rawId) - : null; - /** - * When it is about to overwrite a object into options.workspace. - * We need to check if the options.workspaces is the subset of object.workspaces, - * Or it will be treated as a conflict - */ - if (findObject && findObject.found) { - const transformedObject = this._serializer.rawToSavedObject( - findObject as SavedObjectsRawDoc - ) as SavedObject; - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( - options.workspaces, - transformedObject.workspaces - ); - if (filteredWorkspaces.length) { - /** - * options.workspaces is not a subset of object.workspaces, - * return a conflict error. - */ - const { id, type } = object; - return { - tag: 'Left' as 'Left', - error: { - id, - type, - error: { - ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)), - metadata: { isNotOverwritable: true }, - }, - }, - }; - } else { - savedObjectWorkspaces = transformedObject.workspaces; - } - } + savedObjectWorkspaces = object.workspaces; } const expectedResult = { @@ -534,7 +469,7 @@ export class SavedObjectsRepository { updated_at: time, references: object.references || [], originId: object.originId, - workspaces: savedObjectWorkspaces, + ...(savedObjectWorkspaces && { workspaces: savedObjectWorkspaces }), }) as SavedObjectSanitizedDoc ), }; @@ -632,7 +567,7 @@ export class SavedObjectsRepository { const bulkGetDocs = expectedBulkGetResults.filter(isRight).map(({ value: { type, id } }) => ({ _id: this._serializer.generateRawId(namespace, type, id), _index: this.getIndexForType(type), - _source: ['type', 'namespaces', 'workspaces'], + _source: ['type', 'namespaces'], })); const bulkGetResponse = bulkGetDocs.length ? await this.client.mget( @@ -655,24 +590,13 @@ export class SavedObjectsRepository { const { type, id, opensearchRequestIndex } = expectedResult.value; const doc = bulkGetResponse?.body.docs[opensearchRequestIndex]; if (doc?.found) { - let workspaceConflict = false; - if (options.workspaces) { - const transformedObject = this._serializer.rawToSavedObject(doc as SavedObjectsRawDoc); - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( - options.workspaces, - transformedObject.workspaces - ); - if (filteredWorkspaces.length) { - workspaceConflict = true; - } - } errors.push({ id, type, error: { ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)), // @ts-expect-error MultiGetHit._source is optional - ...((!this.rawDocExistsInNamespace(doc!, namespace) || workspaceConflict) && { + ...(!this.rawDocExistsInNamespace(doc!, namespace) && { metadata: { isNotOverwritable: true }, }), }, diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts index a087dc6c388a..19c337b9172f 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -68,6 +68,10 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions { * Note: this can only be used for multi-namespace object types. */ initialNamespaces?: string[]; + /** + * workspaces the new created objects belong to + */ + workspaces?: string[]; } /** @@ -91,6 +95,10 @@ export interface SavedObjectsBulkCreateObject { * Note: this can only be used for multi-namespace object types. */ initialNamespaces?: string[]; + /** + * workspaces the objects belong to, will only be used when overwrite is enabled. + */ + workspaces?: string[]; } /** diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts index b6bd7b00f676..e60bb6aea0eb 100644 --- a/src/plugins/workspace/common/constants.ts +++ b/src/plugins/workspace/common/constants.ts @@ -4,3 +4,5 @@ */ export const WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID = 'workspace'; +export const WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID = + 'workspace_conflict_control'; diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 38e8a3c18f8c..daaed047d15f 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -13,10 +13,13 @@ import { import { IWorkspaceClientImpl } from './types'; import { WorkspaceClient } from './workspace_client'; import { registerRoutes } from './routes'; +import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper_for_check_workspace_conflict'; +import { WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; private client?: IWorkspaceClientImpl; + private workspaceConflictControl?: WorkspaceConflictSavedObjectsClientWrapper; constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get('plugins', 'workspace'); @@ -29,6 +32,14 @@ export class WorkspacePlugin implements Plugin<{}, {}> { await this.client.setup(core); + this.workspaceConflictControl = new WorkspaceConflictSavedObjectsClientWrapper(); + + core.savedObjects.addClientWrapper( + -1, + WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID, + this.workspaceConflictControl.wrapperFactory + ); + registerRoutes({ http: core.http, logger: this.logger, @@ -43,6 +54,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { public start(core: CoreStart) { this.logger.debug('Starting Workspace service'); this.client?.setSavedObjects(core.savedObjects); + this.workspaceConflictControl?.setSerializer(core.savedObjects.createSerializer()); return { client: this.client as IWorkspaceClientImpl, diff --git a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts similarity index 96% rename from src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts rename to src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts index b601de985dc0..b41f5a9eb08b 100644 --- a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -2,10 +2,11 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ + import { SavedObject } from 'src/core/types'; import { isEqual } from 'lodash'; -import * as osdTestServer from '../../../../../test_helpers/osd_server'; import { Readable } from 'stream'; +import * as osdTestServer from '../../../../../core/test_helpers/osd_server'; const dashboard: Omit = { type: 'dashboard', @@ -13,12 +14,19 @@ const dashboard: Omit = { references: [], }; -describe('repository integration test', () => { +describe('saved_objects_wrapper_for_check_workspace_conflict integration test', () => { let root: ReturnType; let opensearchServer: osdTestServer.TestOpenSearchUtils; beforeAll(async () => { const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ adjustTimeout: (t: number) => jest.setTimeout(t), + settings: { + osd: { + workspace: { + enabled: true, + }, + }, + }, }); opensearchServer = await startOpenSearch(); const startOSDResp = await startOpenSearchDashboards(); @@ -35,7 +43,7 @@ describe('repository integration test', () => { (await osdTestServer.request.delete(root, `/api/saved_objects/${object.type}/${object.id}`)) .statusCode ) - ); + ).toEqual(true); }; const getItem = async (object: Pick) => { diff --git a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts new file mode 100644 index 000000000000..6d8392f3b191 --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts @@ -0,0 +1,257 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import Boom from '@hapi/boom'; +import { + SavedObject, + SavedObjectsBaseOptions, + SavedObjectsBulkCreateObject, + SavedObjectsBulkResponse, + SavedObjectsClientWrapperFactory, + SavedObjectsCreateOptions, + SavedObjectsErrorHelpers, + SavedObjectsUtils, + SavedObjectsSerializer, + SavedObjectsCheckConflictsObject, + SavedObjectsCheckConflictsResponse, +} from '../../../../core/server'; + +const errorContent = (error: Boom.Boom) => error.output.payload; + +export class WorkspaceConflictSavedObjectsClientWrapper { + private _serializer?: SavedObjectsSerializer; + public setSerializer(serializer: SavedObjectsSerializer) { + this._serializer = serializer; + } + private getRawId(props: { namespace?: string; id: string; type: string }) { + return ( + this._serializer?.generateRawId(props.namespace, props.type, props.id) || + `${props.type}:${props.id}` + ); + } + public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => { + const createWithWorkspaceConflictCheck = async ( + type: string, + attributes: T, + options: SavedObjectsCreateOptions = {} + ) => { + const { workspaces, id, overwrite } = options; + let savedObjectWorkspaces = options?.workspaces; + + if (id && overwrite && workspaces) { + let currentItem; + try { + currentItem = await wrapperOptions.client.get(type, id); + } catch (e) { + // this.get will throw an error when no items can be found + } + if (currentItem) { + if ( + SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( + workspaces, + currentItem.workspaces + ).length + ) { + throw SavedObjectsErrorHelpers.createConflictError(type, id); + } else { + savedObjectWorkspaces = currentItem.workspaces; + } + } + } + + return await wrapperOptions.client.create(type, attributes, { + ...options, + workspaces: savedObjectWorkspaces, + }); + }; + + const bulkCreateWithWorkspaceConflictCheck = async ( + objects: Array>, + options: SavedObjectsCreateOptions = {} + ): Promise> => { + const { overwrite, namespace } = options; + const bulkGetDocs = objects + .filter((object) => !!(object.id && options.workspaces)) + .map((object) => { + const { type, id } = object; + /** + * It requires a check when overwriting objects to target workspaces + */ + return { + type, + id: id as string, + fields: ['id', 'workspaces'], + }; + }); + const objectsConflictWithWorkspace: SavedObject[] = []; + const objectsMapWorkspaces: Record = {}; + if (bulkGetDocs.length) { + const bulkGetResult = await wrapperOptions.client.bulkGet(bulkGetDocs); + + bulkGetResult.saved_objects.forEach((object) => { + if (!object.error && object.id && overwrite) { + /** + * When it is about to overwrite a object into options.workspace. + * We need to check if the options.workspaces is the subset of object.workspaces, + * Or it will be treated as a conflict + */ + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( + options.workspaces, + object.workspaces + ); + const { id, type } = object; + if (filteredWorkspaces.length) { + /** + * options.workspaces is not a subset of object.workspaces, + * return a conflict error. + */ + objectsConflictWithWorkspace.push({ + id, + type, + attributes: {}, + references: [], + error: { + ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)), + metadata: { isNotOverwritable: true }, + }, + }); + } else { + objectsMapWorkspaces[this.getRawId({ namespace, type, id })] = object.workspaces; + } + } + }); + } + + const objectsFilteredByError = objects.filter( + (item) => + !objectsConflictWithWorkspace.find( + (errorItems) => + this.getRawId({ namespace, type: errorItems.type, id: errorItems.id }) === + this.getRawId({ namespace, type: item.type, id: item.id as string }) + ) + ); + let objectsPayload = objectsFilteredByError; + if (overwrite) { + objectsPayload = objectsPayload.map((item) => { + if (item.id) { + item.workspaces = + objectsMapWorkspaces[ + this.getRawId({ + namespace, + id: item.id, + type: item.type, + }) + ]; + } + + return item; + }); + } + const realBulkCreateResult = await wrapperOptions.client.bulkCreate(objectsPayload, options); + const result: SavedObjectsBulkResponse = { + ...realBulkCreateResult, + saved_objects: [...objectsConflictWithWorkspace, ...realBulkCreateResult.saved_objects], + }; + return result as SavedObjectsBulkResponse; + }; + + const checkConflictWithWorkspaceConflictCheck = async ( + objects: SavedObjectsCheckConflictsObject[] = [], + options: SavedObjectsBaseOptions = {} + ) => { + const objectsConflictWithWorkspace: SavedObjectsCheckConflictsResponse['errors'] = []; + if (options.workspaces) { + if (objects.length === 0) { + return { errors: [] }; + } + + const bulkGetDocs: any[] = objects.map((object) => { + const { type, id } = object; + + return { + type, + id, + fields: ['id', 'workspaces'], + }; + }); + + if (bulkGetDocs.length) { + const bulkGetResult = await wrapperOptions.client.bulkGet(bulkGetDocs); + + bulkGetResult.saved_objects.forEach((object) => { + const { id, type } = object; + if (!object.error) { + let workspaceConflict = false; + if (options.workspaces) { + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( + options.workspaces, + object.workspaces + ); + if (filteredWorkspaces.length) { + workspaceConflict = true; + } + } + if (workspaceConflict) { + objectsConflictWithWorkspace.push({ + id, + type, + error: { + ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)), + metadata: { isNotOverwritable: true }, + }, + }); + } + } + }); + } + } + + const objectsFilteredByError = objects.filter( + (item) => + !objectsConflictWithWorkspace.find( + (errorItems) => + this.getRawId({ + namespace: options.namespace, + type: errorItems.type, + id: errorItems.id, + }) === + this.getRawId({ + namespace: options.namespace, + type: item.type, + id: item.id as string, + }) + ) + ); + const realBulkCreateResult = await wrapperOptions.client.checkConflicts( + objectsFilteredByError, + options + ); + const result: SavedObjectsCheckConflictsResponse = { + ...realBulkCreateResult, + errors: [...objectsConflictWithWorkspace, ...realBulkCreateResult.errors], + }; + + return result; + }; + + return { + ...wrapperOptions.client, + create: createWithWorkspaceConflictCheck, + bulkCreate: bulkCreateWithWorkspaceConflictCheck, + checkConflicts: checkConflictWithWorkspaceConflictCheck, + delete: wrapperOptions.client.delete, + find: wrapperOptions.client.find, + bulkGet: wrapperOptions.client.bulkGet, + get: wrapperOptions.client.get, + update: wrapperOptions.client.update, + bulkUpdate: wrapperOptions.client.bulkUpdate, + errors: wrapperOptions.client.errors, + addToNamespaces: wrapperOptions.client.addToNamespaces, + deleteFromNamespaces: wrapperOptions.client.deleteFromNamespaces, + }; + }; + + constructor() {} +}