From 4060458fcca12f428f3a758e2267bef035d807ba Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 18 Oct 2023 16:22:34 +0800 Subject: [PATCH 01/31] refact: move workspace specific logic to savedObjectWrapper Signed-off-by: SuZhou-Joe --- .../saved_objects/service/lib/repository.ts | 78 +---- .../service/saved_objects_client.ts | 8 + src/plugins/workspace/common/constants.ts | 2 + src/plugins/workspace/server/plugin.ts | 15 +- ...apper_for_check_workspace_conflict.test.ts | 318 ++++++++++++++++++ ...ts_wrapper_for_check_workspace_conflict.ts | 257 ++++++++++++++ 6 files changed, 606 insertions(+), 72 deletions(-) create mode 100644 src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts 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 3ee7889475d2..db945faae9a7 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -287,26 +287,6 @@ export class SavedObjectsRepository { } } - let savedObjectWorkspaces = workspaces; - - if (id && overwrite) { - try { - const currentItem = await this.get(type, id); - if ( - SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( - workspaces, - currentItem.workspaces - ).length - ) { - throw SavedObjectsErrorHelpers.createConflictError(type, id); - } else { - savedObjectWorkspaces = currentItem.workspaces; - } - } catch (e) { - // this.get will throw an error when no items can be found - } - } - const migrated = this._migrator.migrateDocument({ id, type, @@ -317,7 +297,7 @@ export class SavedObjectsRepository { migrationVersion, updated_at: time, ...(Array.isArray(references) && { references }), - ...(Array.isArray(savedObjectWorkspaces) && { workspaces: savedObjectWorkspaces }), + ...(Array.isArray(workspaces) && { workspaces }), ...(permissions && { permissions }), }); @@ -385,16 +365,12 @@ export class SavedObjectsRepository { const method = object.id && overwrite ? 'index' : 'create'; const requiresNamespacesCheck = object.id && this._registry.isMultiNamespace(object.type); - /** - * Only when importing an object to a target workspace should we check if the object is workspace-specific. - */ - const requiresWorkspaceCheck = object.id; if (object.id == null) object.id = uuid.v1(); let opensearchRequestIndexPayload = {}; - if (requiresNamespacesCheck || requiresWorkspaceCheck) { + if (requiresNamespacesCheck) { opensearchRequestIndexPayload = { opensearchRequestIndex: bulkGetRequestIndexCounter, }; @@ -417,7 +393,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( @@ -495,36 +471,7 @@ export class SavedObjectsRepository { savedObjectWorkspaces = 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; - if (findObject && findObject.found) { - const transformedObject = this._serializer.rawToSavedObject( - findObject as SavedObjectsRawDoc - ) as SavedObject; - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( - options.workspaces, - transformedObject.workspaces - ); - if (filteredWorkspaces.length) { - 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 = { @@ -541,7 +488,7 @@ export class SavedObjectsRepository { updated_at: time, references: object.references || [], originId: object.originId, - workspaces: savedObjectWorkspaces, + ...(savedObjectWorkspaces && { workspaces: savedObjectWorkspaces }), }) as SavedObjectSanitizedDoc ), }; @@ -639,7 +586,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( @@ -662,24 +609,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.filterWorkspacesAccordingToBaseWorkspaces( - 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 4f430d7f7134..c9990977bb48 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -71,6 +71,10 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions { initialNamespaces?: string[]; /** permission control describe by ACL object */ permissions?: Permissions; + /** + * workspaces the new created objects belong to + */ + workspaces?: string[]; } /** @@ -94,6 +98,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 ef0821c88619..426055588905 100644 --- a/src/plugins/workspace/common/constants.ts +++ b/src/plugins/workspace/common/constants.ts @@ -19,3 +19,5 @@ export const PATHS = { export const WORKSPACE_OP_TYPE_CREATE = 'create'; export const WORKSPACE_OP_TYPE_UPDATE = 'update'; 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 a7cdb8c97d4b..5d146a54ba3a 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -16,7 +16,10 @@ import { IWorkspaceDBImpl } from './types'; import { WorkspaceClientWithSavedObject } from './workspace_client'; import { WorkspaceSavedObjectsClientWrapper } from './saved_objects'; import { registerRoutes } from './routes'; -import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; +import { + WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, + WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID, +} from '../common/constants'; import { SavedObjectsPermissionControl, SavedObjectsPermissionControlContract, @@ -24,12 +27,14 @@ import { import { registerPermissionCheckRoutes } from './permission_control/routes'; import { WorkspacePluginConfigType } from '../config'; import { cleanWorkspaceId, getWorkspaceIdFromUrl } from '../../../core/server/utils'; +import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper_for_check_workspace_conflict'; export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; private client?: IWorkspaceDBImpl; private permissionControl?: SavedObjectsPermissionControlContract; private readonly config$: Observable; + private workspaceConflictControl?: WorkspaceConflictSavedObjectsClientWrapper; private proxyWorkspaceTrafficToRealHandler(setupDeps: CoreSetup) { /** @@ -83,6 +88,13 @@ export class WorkspacePlugin implements Plugin<{}, {}> { } this.proxyWorkspaceTrafficToRealHandler(core); + this.workspaceConflictControl = new WorkspaceConflictSavedObjectsClientWrapper(); + + core.savedObjects.addClientWrapper( + -1, + WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID, + this.workspaceConflictControl.wrapperFactory + ); registerRoutes({ http: core.http, @@ -110,6 +122,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { this.logger.debug('Starting SavedObjects service'); this.permissionControl?.setup(core.savedObjects.getScopedClient); this.client?.setSavedObjects(core.savedObjects); + this.workspaceConflictControl?.setSerializer(core.savedObjects.createSerializer()); return { client: this.client as IWorkspaceDBImpl, diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts new file mode 100644 index 000000000000..b41f5a9eb08b --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -0,0 +1,318 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { SavedObject } from 'src/core/types'; +import { isEqual } from 'lodash'; +import { Readable } from 'stream'; +import * as osdTestServer from '../../../../../core/test_helpers/osd_server'; + +const dashboard: Omit = { + type: 'dashboard', + attributes: {}, + references: [], +}; + +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(); + root = startOSDResp.root; + }, 30000); + afterAll(async () => { + await root.shutdown(); + await opensearchServer.stop(); + }); + + const deleteItem = async (object: Pick) => { + expect( + [200, 404].includes( + (await osdTestServer.request.delete(root, `/api/saved_objects/${object.type}/${object.id}`)) + .statusCode + ) + ).toEqual(true); + }; + + const getItem = async (object: Pick) => { + return await osdTestServer.request + .get(root, `/api/saved_objects/${object.type}/${object.id}`) + .expect(200); + }; + + const clearFooAndBar = async () => { + await deleteItem({ + type: dashboard.type, + id: 'foo', + }); + await deleteItem({ + type: dashboard.type, + id: 'bar', + }); + }; + + describe('workspace related CRUD', () => { + it('create', async () => { + const createResult = await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}`) + .send({ + attributes: dashboard.attributes, + workspaces: ['foo'], + }) + .expect(200); + + expect(createResult.body.workspaces).toEqual(['foo']); + await deleteItem({ + type: dashboard.type, + id: createResult.body.id, + }); + }); + + it('create-with-override', async () => { + const createResult = await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}`) + .send({ + attributes: dashboard.attributes, + workspaces: ['foo'], + }) + .expect(200); + + await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}/${createResult.body.id}?overwrite=true`) + .send({ + attributes: dashboard.attributes, + workspaces: ['bar'], + }) + .expect(409); + + await deleteItem({ + type: dashboard.type, + id: createResult.body.id, + }); + }); + + it('bulk create', async () => { + await clearFooAndBar(); + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + expect((createResultFoo.body.saved_objects as any[]).some((item) => item.error)).toEqual( + false + ); + expect( + (createResultFoo.body.saved_objects as any[]).every((item) => + isEqual(item.workspaces, ['foo']) + ) + ).toEqual(true); + expect((createResultBar.body.saved_objects as any[]).some((item) => item.error)).toEqual( + false + ); + expect( + (createResultBar.body.saved_objects as any[]).every((item) => + isEqual(item.workspaces, ['bar']) + ) + ).toEqual(true); + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + + it('bulk create with conflict', async () => { + await clearFooAndBar(); + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + /** + * overwrite with workspaces + */ + const overwriteWithWorkspacesResult = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?overwrite=true&workspaces=foo`) + .send([ + { + ...dashboard, + id: 'bar', + }, + { + ...dashboard, + id: 'foo', + attributes: { + title: 'foo', + }, + }, + ]) + .expect(200); + + expect(overwriteWithWorkspacesResult.body.saved_objects[0].error.statusCode).toEqual(409); + expect(overwriteWithWorkspacesResult.body.saved_objects[1].attributes.title).toEqual('foo'); + expect(overwriteWithWorkspacesResult.body.saved_objects[1].workspaces).toEqual(['foo']); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + + it('checkConflicts when importing ndjson', async () => { + await clearFooAndBar(); + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const getResultFoo = await getItem({ + type: dashboard.type, + id: 'foo', + }); + const getResultBar = await getItem({ + type: dashboard.type, + id: 'bar', + }); + + const readableStream = new Readable(); + readableStream.push( + `Content-Disposition: form-data; name="file"; filename="tmp.ndjson"\r\n\r\n` + ); + readableStream.push( + [JSON.stringify(getResultFoo.body), JSON.stringify(getResultBar.body)].join('\n') + ); + readableStream.push(null); + + /** + * import with workspaces when conflicts + */ + const importWithWorkspacesResult = await osdTestServer.request + .post(root, `/api/saved_objects/_import?workspaces=foo&overwrite=false`) + .attach( + 'file', + Buffer.from( + [JSON.stringify(getResultFoo.body), JSON.stringify(getResultBar.body)].join('\n'), + 'utf-8' + ), + 'tmp.ndjson' + ) + .expect(200); + + expect(importWithWorkspacesResult.body.success).toEqual(false); + expect(importWithWorkspacesResult.body.errors.length).toEqual(1); + expect(importWithWorkspacesResult.body.errors[0].id).toEqual('foo'); + expect(importWithWorkspacesResult.body.errors[0].error.type).toEqual('conflict'); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + + it('find by workspaces', async () => { + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const findResult = await osdTestServer.request + .get(root, `/api/saved_objects/_find?workspaces=bar&type=${dashboard.type}`) + .expect(200); + + expect(findResult.body.total).toEqual(1); + expect(findResult.body.saved_objects[0].workspaces).toEqual(['bar']); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + }); +}); 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() {} +} From 41204e2fb9940013bc766f48b7629d53c2c34342 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 18 Oct 2023 16:53:42 +0800 Subject: [PATCH 02/31] feat: remove unnecessary changes Signed-off-by: SuZhou-Joe --- .../server/saved_objects/service/lib/repository.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index db945faae9a7..37bc044885a1 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -368,21 +368,12 @@ export class SavedObjectsRepository { if (object.id == null) object.id = uuid.v1(); - let opensearchRequestIndexPayload = {}; - - if (requiresNamespacesCheck) { - opensearchRequestIndexPayload = { - opensearchRequestIndex: bulkGetRequestIndexCounter, - }; - bulkGetRequestIndexCounter++; - } - return { tag: 'Right' as 'Right', value: { method, object, - ...opensearchRequestIndexPayload, + ...(requiresNamespacesCheck && { opensearchRequestIndex: bulkGetRequestIndexCounter++ }), }, }; }); From 888b77e0d74a797e18dcb0401c20ca22d7048b65 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 23 Feb 2024 12:35:03 +0800 Subject: [PATCH 03/31] fix: unit test Signed-off-by: SuZhou-Joe --- .../service/lib/repository.test.js | 24 ++++--------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index 816d1013e5be..089a2b5cb79f 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -651,7 +651,6 @@ describe('SavedObjectsRepository', () => { }; const bulkCreateSuccess = async (objects, options) => { - const originalObjects = JSON.parse(JSON.stringify(objects)); const multiNamespaceObjects = objects.filter( ({ type, id }) => registry.isMultiNamespace(type) && id ); @@ -666,9 +665,7 @@ describe('SavedObjectsRepository', () => { opensearchClientMock.createSuccessTransportRequestPromise(response) ); const result = await savedObjectsRepository.bulkCreate(objects, options); - expect(client.mget).toHaveBeenCalledTimes( - multiNamespaceObjects?.length || originalObjects?.some((item) => item.id) ? 1 : 0 - ); + expect(client.mget).toHaveBeenCalledTimes(multiNamespaceObjects?.length ? 1 : 0); return result; }; @@ -726,10 +723,7 @@ describe('SavedObjectsRepository', () => { await bulkCreateSuccess(objects); expect(client.bulk).toHaveBeenCalledTimes(1); expect(client.mget).toHaveBeenCalledTimes(1); - const docs = [ - expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), - expect.objectContaining({ _id: `${MULTI_NAMESPACE_TYPE}:${obj2.id}` }), - ]; + const docs = [expect.objectContaining({ _id: `${MULTI_NAMESPACE_TYPE}:${obj2.id}` })]; expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); }); @@ -2050,17 +2044,9 @@ describe('SavedObjectsRepository', () => { const createSuccess = async (type, attributes, options) => { const result = await savedObjectsRepository.create(type, attributes, options); - let count = 0; - if (options?.overwrite && options?.id) { - /** - * workspace will call extra one to get latest status of current object - */ - count++; - } - if (registry.isMultiNamespace(type) && options.overwrite) { - count++; - } - expect(client.get).toHaveBeenCalledTimes(count); + expect(client.get).toHaveBeenCalledTimes( + registry.isMultiNamespace(type) && options.overwrite ? 1 : 0 + ); return result; }; From 6560073244829d594d5dcc496d60bbf5ebf0cac3 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 23 Feb 2024 12:37:41 +0800 Subject: [PATCH 04/31] fix: some error Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/service/lib/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/saved_objects/service/lib/utils.ts b/src/core/server/saved_objects/service/lib/utils.ts index 490c2b7083d2..ca719887986d 100644 --- a/src/core/server/saved_objects/service/lib/utils.ts +++ b/src/core/server/saved_objects/service/lib/utils.ts @@ -81,7 +81,7 @@ export class SavedObjectsUtils { saved_objects: [], }); - public static filterWorkspacesAccordingToBaseWorkspaces( + public static filterWorkspacesAccordingToSourceWorkspaces( targetWorkspaces?: string[], baseWorkspaces?: string[] ): string[] { From d1c3c235669d58ab9163f5e065eba122a5ae1eb0 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 23 Feb 2024 15:26:03 +0800 Subject: [PATCH 05/31] fix: some error Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/import/regenerate_ids.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/saved_objects/import/regenerate_ids.ts b/src/core/server/saved_objects/import/regenerate_ids.ts index 27b10e575f44..709c4016bcec 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.ts +++ b/src/core/server/saved_objects/import/regenerate_ids.ts @@ -68,7 +68,7 @@ export const regenerateIdsWithReference = async (props: { return acc; } - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( workspaces, object.workspaces ); From c7b0e816d8fa3203de92f4a3276f525b984fcb44 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 23 Feb 2024 16:11:07 +0800 Subject: [PATCH 06/31] fix: unit test error Signed-off-by: SuZhou-Joe --- .../saved_objects/service/lib/repository.test.js | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index 089a2b5cb79f..a720db3798ba 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -1001,12 +1001,6 @@ describe('SavedObjectsRepository', () => { const response1 = { status: 200, docs: [ - { - found: true, - _source: { - type: obj1.type, - }, - }, { found: true, _source: { @@ -1030,11 +1024,7 @@ describe('SavedObjectsRepository', () => { expect(client.mget).toHaveBeenCalled(); const body1 = { - docs: [ - expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), - expect.objectContaining({ _id: `${obj.type}:${obj.id}` }), - expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), - ], + docs: [expect.objectContaining({ _id: `${obj.type}:${obj.id}` })], }; expect(client.mget).toHaveBeenCalledWith( expect.objectContaining({ body: body1 }), From 8240051de1b85697e24be610c0d3a026cddb077c Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 26 Feb 2024 08:37:57 +0000 Subject: [PATCH 07/31] feat: fix test error Signed-off-by: SuZhou-Joe --- ...apper_for_check_workspace_conflict.test.ts | 74 ++++++++++++++----- 1 file changed, 55 insertions(+), 19 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts index b41f5a9eb08b..05bcd7e6c067 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -14,9 +14,20 @@ const dashboard: Omit = { references: [], }; +interface WorkspaceAttributes { + id: string; + name?: string; +} + describe('saved_objects_wrapper_for_check_workspace_conflict integration test', () => { let root: ReturnType; let opensearchServer: osdTestServer.TestOpenSearchUtils; + let createdFooWorkspace: WorkspaceAttributes = { + id: '', + }; + let createdBarWorkspace: WorkspaceAttributes = { + id: '', + }; beforeAll(async () => { const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ adjustTimeout: (t: number) => jest.setTimeout(t), @@ -24,6 +35,9 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', osd: { workspace: { enabled: true, + permission: { + enabled: true, + }, }, }, }, @@ -31,6 +45,17 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', opensearchServer = await startOpenSearch(); const startOSDResp = await startOpenSearchDashboards(); root = startOSDResp.root; + const createWorkspace = (workspaceAttribute: Omit) => + osdTestServer.request.post(root, `/api/workspaces`).send({ + attributes: workspaceAttribute, + }); + + createdFooWorkspace = await createWorkspace({ + name: 'foo', + }).then((resp) => resp.body.result); + createdBarWorkspace = await createWorkspace({ + name: 'bar', + }).then((resp) => resp.body.result); }, 30000); afterAll(async () => { await root.shutdown(); @@ -69,11 +94,11 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .post(root, `/api/saved_objects/${dashboard.type}`) .send({ attributes: dashboard.attributes, - workspaces: ['foo'], + workspaces: [createdFooWorkspace.id], }) .expect(200); - expect(createResult.body.workspaces).toEqual(['foo']); + expect(createResult.body.workspaces).toEqual([createdFooWorkspace.id]); await deleteItem({ type: dashboard.type, id: createResult.body.id, @@ -85,7 +110,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .post(root, `/api/saved_objects/${dashboard.type}`) .send({ attributes: dashboard.attributes, - workspaces: ['foo'], + workspaces: [createdFooWorkspace.id], }) .expect(200); @@ -93,7 +118,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .post(root, `/api/saved_objects/${dashboard.type}/${createResult.body.id}?overwrite=true`) .send({ attributes: dashboard.attributes, - workspaces: ['bar'], + workspaces: [createdBarWorkspace.id], }) .expect(409); @@ -106,7 +131,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', it('bulk create', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdFooWorkspace.id}`) .send([ { ...dashboard, @@ -116,7 +141,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .expect(200); const createResultBar = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdBarWorkspace.id}`) .send([ { ...dashboard, @@ -130,7 +155,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', ); expect( (createResultFoo.body.saved_objects as any[]).every((item) => - isEqual(item.workspaces, ['foo']) + isEqual(item.workspaces, [createdFooWorkspace.id]) ) ).toEqual(true); expect((createResultBar.body.saved_objects as any[]).some((item) => item.error)).toEqual( @@ -138,7 +163,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', ); expect( (createResultBar.body.saved_objects as any[]).every((item) => - isEqual(item.workspaces, ['bar']) + isEqual(item.workspaces, [createdBarWorkspace.id]) ) ).toEqual(true); await Promise.all( @@ -154,7 +179,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', it('bulk create with conflict', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdFooWorkspace.id}`) .send([ { ...dashboard, @@ -164,7 +189,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .expect(200); const createResultBar = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdBarWorkspace.id}`) .send([ { ...dashboard, @@ -177,7 +202,10 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', * overwrite with workspaces */ const overwriteWithWorkspacesResult = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?overwrite=true&workspaces=foo`) + .post( + root, + `/api/saved_objects/_bulk_create?overwrite=true&workspaces=${createdFooWorkspace.id}` + ) .send([ { ...dashboard, @@ -195,7 +223,9 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', expect(overwriteWithWorkspacesResult.body.saved_objects[0].error.statusCode).toEqual(409); expect(overwriteWithWorkspacesResult.body.saved_objects[1].attributes.title).toEqual('foo'); - expect(overwriteWithWorkspacesResult.body.saved_objects[1].workspaces).toEqual(['foo']); + expect(overwriteWithWorkspacesResult.body.saved_objects[1].workspaces).toEqual([ + createdFooWorkspace.id, + ]); await Promise.all( [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => @@ -210,7 +240,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', it('checkConflicts when importing ndjson', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdFooWorkspace.id}`) .send([ { ...dashboard, @@ -220,7 +250,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .expect(200); const createResultBar = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdBarWorkspace.id}`) .send([ { ...dashboard, @@ -251,7 +281,10 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', * import with workspaces when conflicts */ const importWithWorkspacesResult = await osdTestServer.request - .post(root, `/api/saved_objects/_import?workspaces=foo&overwrite=false`) + .post( + root, + `/api/saved_objects/_import?workspaces=${createdFooWorkspace.id}&overwrite=false` + ) .attach( 'file', Buffer.from( @@ -279,7 +312,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', it('find by workspaces', async () => { const createResultFoo = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdFooWorkspace.id}`) .send([ { ...dashboard, @@ -289,7 +322,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .expect(200); const createResultBar = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdBarWorkspace.id}`) .send([ { ...dashboard, @@ -299,11 +332,14 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .expect(200); const findResult = await osdTestServer.request - .get(root, `/api/saved_objects/_find?workspaces=bar&type=${dashboard.type}`) + .get( + root, + `/api/saved_objects/_find?workspaces=${createdBarWorkspace.id}&type=${dashboard.type}` + ) .expect(200); expect(findResult.body.total).toEqual(1); - expect(findResult.body.saved_objects[0].workspaces).toEqual(['bar']); + expect(findResult.body.saved_objects[0].workspaces).toEqual([createdBarWorkspace.id]); await Promise.all( [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => From e79bdd9ee011ad67a56ca3e9712f6c860c5ff86a Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 18 Oct 2023 16:22:34 +0800 Subject: [PATCH 08/31] refact: move workspace specific logic to savedObjectWrapper Signed-off-by: SuZhou-Joe --- .../saved_objects/service/lib/repository.ts | 58 +--- .../service/saved_objects_client.ts | 8 + src/plugins/workspace/common/constants.ts | 2 + src/plugins/workspace/server/plugin.ts | 12 +- ...apper_for_check_workspace_conflict.test.ts | 318 ++++++++++++++++++ ...ts_wrapper_for_check_workspace_conflict.ts | 257 ++++++++++++++ 6 files changed, 600 insertions(+), 55 deletions(-) create mode 100644 src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts 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 3ee7889475d2..0e599e8d10d4 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -287,26 +287,6 @@ export class SavedObjectsRepository { } } - let savedObjectWorkspaces = workspaces; - - if (id && overwrite) { - try { - const currentItem = await this.get(type, id); - if ( - SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( - workspaces, - currentItem.workspaces - ).length - ) { - throw SavedObjectsErrorHelpers.createConflictError(type, id); - } else { - savedObjectWorkspaces = currentItem.workspaces; - } - } catch (e) { - // this.get will throw an error when no items can be found - } - } - const migrated = this._migrator.migrateDocument({ id, type, @@ -317,7 +297,7 @@ export class SavedObjectsRepository { migrationVersion, updated_at: time, ...(Array.isArray(references) && { references }), - ...(Array.isArray(savedObjectWorkspaces) && { workspaces: savedObjectWorkspaces }), + ...(Array.isArray(workspaces) && { workspaces }), ...(permissions && { permissions }), }); @@ -445,7 +425,6 @@ export class SavedObjectsRepository { object: { initialNamespaces, version, ...object }, method, } = expectedBulkGetResult.value; - let savedObjectWorkspaces: string[] | undefined; if (opensearchRequestIndex !== undefined) { const indexFound = bulkGetResponse?.statusCode !== 404; const actualResult = indexFound @@ -492,39 +471,10 @@ export class SavedObjectsRepository { versionProperties = getExpectedVersionProperties(version); } - savedObjectWorkspaces = options.workspaces; + let savedObjectWorkspaces = 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; - if (findObject && findObject.found) { - const transformedObject = this._serializer.rawToSavedObject( - findObject as SavedObjectsRawDoc - ) as SavedObject; - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( - options.workspaces, - transformedObject.workspaces - ); - if (filteredWorkspaces.length) { - 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 = { @@ -541,7 +491,7 @@ export class SavedObjectsRepository { updated_at: time, references: object.references || [], originId: object.originId, - workspaces: savedObjectWorkspaces, + ...(savedObjectWorkspaces && { workspaces: savedObjectWorkspaces }), }) as SavedObjectSanitizedDoc ), }; 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 4f430d7f7134..c9990977bb48 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -71,6 +71,10 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions { initialNamespaces?: string[]; /** permission control describe by ACL object */ permissions?: Permissions; + /** + * workspaces the new created objects belong to + */ + workspaces?: string[]; } /** @@ -94,6 +98,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 ef0821c88619..426055588905 100644 --- a/src/plugins/workspace/common/constants.ts +++ b/src/plugins/workspace/common/constants.ts @@ -19,3 +19,5 @@ export const PATHS = { export const WORKSPACE_OP_TYPE_CREATE = 'create'; export const WORKSPACE_OP_TYPE_UPDATE = 'update'; 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 12c101cae6bb..f7abb426af50 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -11,7 +11,6 @@ import { Plugin, Logger, SavedObjectsClient, - WORKSPACE_TYPE, } from '../../../core/server'; import { IWorkspaceClientImpl } from './types'; import { WorkspaceClientWithSavedObject } from './workspace_client'; @@ -25,6 +24,8 @@ import { import { registerPermissionCheckRoutes } from './permission_control/routes'; import { ConfigSchema } from '../config'; import { cleanWorkspaceId, getWorkspaceIdFromUrl } from '../../../core/server/utils'; +import { WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; +import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper_for_check_workspace_conflict'; export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; @@ -32,6 +33,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { private permissionControl?: SavedObjectsPermissionControlContract; private readonly config$: Observable; private workspaceSavedObjectsClientWrapper?: WorkspaceSavedObjectsClientWrapper; + private workspaceConflictControl?: WorkspaceConflictSavedObjectsClientWrapper; private proxyWorkspaceTrafficToRealHandler(setupDeps: CoreSetup) { /** @@ -85,6 +87,13 @@ export class WorkspacePlugin implements Plugin<{}, {}> { } this.proxyWorkspaceTrafficToRealHandler(core); + this.workspaceConflictControl = new WorkspaceConflictSavedObjectsClientWrapper(); + + core.savedObjects.addClientWrapper( + -1, + WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID, + this.workspaceConflictControl.wrapperFactory + ); registerRoutes({ http: core.http, @@ -114,6 +123,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { this.permissionControl?.setup(core.savedObjects.getScopedClient); this.client?.setSavedObjects(core.savedObjects); this.workspaceSavedObjectsClientWrapper?.setScopedClient(core.savedObjects.getScopedClient); + this.workspaceConflictControl?.setSerializer(core.savedObjects.createSerializer()); return { client: this.client as IWorkspaceClientImpl, diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts new file mode 100644 index 000000000000..b41f5a9eb08b --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -0,0 +1,318 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { SavedObject } from 'src/core/types'; +import { isEqual } from 'lodash'; +import { Readable } from 'stream'; +import * as osdTestServer from '../../../../../core/test_helpers/osd_server'; + +const dashboard: Omit = { + type: 'dashboard', + attributes: {}, + references: [], +}; + +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(); + root = startOSDResp.root; + }, 30000); + afterAll(async () => { + await root.shutdown(); + await opensearchServer.stop(); + }); + + const deleteItem = async (object: Pick) => { + expect( + [200, 404].includes( + (await osdTestServer.request.delete(root, `/api/saved_objects/${object.type}/${object.id}`)) + .statusCode + ) + ).toEqual(true); + }; + + const getItem = async (object: Pick) => { + return await osdTestServer.request + .get(root, `/api/saved_objects/${object.type}/${object.id}`) + .expect(200); + }; + + const clearFooAndBar = async () => { + await deleteItem({ + type: dashboard.type, + id: 'foo', + }); + await deleteItem({ + type: dashboard.type, + id: 'bar', + }); + }; + + describe('workspace related CRUD', () => { + it('create', async () => { + const createResult = await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}`) + .send({ + attributes: dashboard.attributes, + workspaces: ['foo'], + }) + .expect(200); + + expect(createResult.body.workspaces).toEqual(['foo']); + await deleteItem({ + type: dashboard.type, + id: createResult.body.id, + }); + }); + + it('create-with-override', async () => { + const createResult = await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}`) + .send({ + attributes: dashboard.attributes, + workspaces: ['foo'], + }) + .expect(200); + + await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}/${createResult.body.id}?overwrite=true`) + .send({ + attributes: dashboard.attributes, + workspaces: ['bar'], + }) + .expect(409); + + await deleteItem({ + type: dashboard.type, + id: createResult.body.id, + }); + }); + + it('bulk create', async () => { + await clearFooAndBar(); + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + expect((createResultFoo.body.saved_objects as any[]).some((item) => item.error)).toEqual( + false + ); + expect( + (createResultFoo.body.saved_objects as any[]).every((item) => + isEqual(item.workspaces, ['foo']) + ) + ).toEqual(true); + expect((createResultBar.body.saved_objects as any[]).some((item) => item.error)).toEqual( + false + ); + expect( + (createResultBar.body.saved_objects as any[]).every((item) => + isEqual(item.workspaces, ['bar']) + ) + ).toEqual(true); + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + + it('bulk create with conflict', async () => { + await clearFooAndBar(); + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + /** + * overwrite with workspaces + */ + const overwriteWithWorkspacesResult = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?overwrite=true&workspaces=foo`) + .send([ + { + ...dashboard, + id: 'bar', + }, + { + ...dashboard, + id: 'foo', + attributes: { + title: 'foo', + }, + }, + ]) + .expect(200); + + expect(overwriteWithWorkspacesResult.body.saved_objects[0].error.statusCode).toEqual(409); + expect(overwriteWithWorkspacesResult.body.saved_objects[1].attributes.title).toEqual('foo'); + expect(overwriteWithWorkspacesResult.body.saved_objects[1].workspaces).toEqual(['foo']); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + + it('checkConflicts when importing ndjson', async () => { + await clearFooAndBar(); + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const getResultFoo = await getItem({ + type: dashboard.type, + id: 'foo', + }); + const getResultBar = await getItem({ + type: dashboard.type, + id: 'bar', + }); + + const readableStream = new Readable(); + readableStream.push( + `Content-Disposition: form-data; name="file"; filename="tmp.ndjson"\r\n\r\n` + ); + readableStream.push( + [JSON.stringify(getResultFoo.body), JSON.stringify(getResultBar.body)].join('\n') + ); + readableStream.push(null); + + /** + * import with workspaces when conflicts + */ + const importWithWorkspacesResult = await osdTestServer.request + .post(root, `/api/saved_objects/_import?workspaces=foo&overwrite=false`) + .attach( + 'file', + Buffer.from( + [JSON.stringify(getResultFoo.body), JSON.stringify(getResultBar.body)].join('\n'), + 'utf-8' + ), + 'tmp.ndjson' + ) + .expect(200); + + expect(importWithWorkspacesResult.body.success).toEqual(false); + expect(importWithWorkspacesResult.body.errors.length).toEqual(1); + expect(importWithWorkspacesResult.body.errors[0].id).toEqual('foo'); + expect(importWithWorkspacesResult.body.errors[0].error.type).toEqual('conflict'); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + + it('find by workspaces', async () => { + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const findResult = await osdTestServer.request + .get(root, `/api/saved_objects/_find?workspaces=bar&type=${dashboard.type}`) + .expect(200); + + expect(findResult.body.total).toEqual(1); + expect(findResult.body.saved_objects[0].workspaces).toEqual(['bar']); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + }); +}); 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() {} +} From f21cfe5c0456f2614642b174c1e76ca8ffec6acb Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 23 Feb 2024 12:37:41 +0800 Subject: [PATCH 09/31] fix: some error Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/import/regenerate_ids.ts | 2 +- src/core/server/saved_objects/service/lib/repository.ts | 2 +- src/core/server/saved_objects/service/lib/utils.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/server/saved_objects/import/regenerate_ids.ts b/src/core/server/saved_objects/import/regenerate_ids.ts index b412bb80bc6b..522a150c4e6f 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.ts +++ b/src/core/server/saved_objects/import/regenerate_ids.ts @@ -73,7 +73,7 @@ export const regenerateIdsWithReference = async (props: { return acc; } - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( workspaces, object.workspaces ); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 0e599e8d10d4..96308104d6a0 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -615,7 +615,7 @@ export class SavedObjectsRepository { let workspaceConflict = false; if (options.workspaces) { const transformedObject = this._serializer.rawToSavedObject(doc as SavedObjectsRawDoc); - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( options.workspaces, transformedObject.workspaces ); diff --git a/src/core/server/saved_objects/service/lib/utils.ts b/src/core/server/saved_objects/service/lib/utils.ts index 490c2b7083d2..ca719887986d 100644 --- a/src/core/server/saved_objects/service/lib/utils.ts +++ b/src/core/server/saved_objects/service/lib/utils.ts @@ -81,7 +81,7 @@ export class SavedObjectsUtils { saved_objects: [], }); - public static filterWorkspacesAccordingToBaseWorkspaces( + public static filterWorkspacesAccordingToSourceWorkspaces( targetWorkspaces?: string[], baseWorkspaces?: string[] ): string[] { From fedb9743df9be8e33c9a9156eeb47004135adc36 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 26 Feb 2024 08:37:57 +0000 Subject: [PATCH 10/31] feat: fix test error Signed-off-by: SuZhou-Joe --- ...apper_for_check_workspace_conflict.test.ts | 74 ++++++++++++++----- 1 file changed, 55 insertions(+), 19 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts index b41f5a9eb08b..05bcd7e6c067 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -14,9 +14,20 @@ const dashboard: Omit = { references: [], }; +interface WorkspaceAttributes { + id: string; + name?: string; +} + describe('saved_objects_wrapper_for_check_workspace_conflict integration test', () => { let root: ReturnType; let opensearchServer: osdTestServer.TestOpenSearchUtils; + let createdFooWorkspace: WorkspaceAttributes = { + id: '', + }; + let createdBarWorkspace: WorkspaceAttributes = { + id: '', + }; beforeAll(async () => { const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ adjustTimeout: (t: number) => jest.setTimeout(t), @@ -24,6 +35,9 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', osd: { workspace: { enabled: true, + permission: { + enabled: true, + }, }, }, }, @@ -31,6 +45,17 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', opensearchServer = await startOpenSearch(); const startOSDResp = await startOpenSearchDashboards(); root = startOSDResp.root; + const createWorkspace = (workspaceAttribute: Omit) => + osdTestServer.request.post(root, `/api/workspaces`).send({ + attributes: workspaceAttribute, + }); + + createdFooWorkspace = await createWorkspace({ + name: 'foo', + }).then((resp) => resp.body.result); + createdBarWorkspace = await createWorkspace({ + name: 'bar', + }).then((resp) => resp.body.result); }, 30000); afterAll(async () => { await root.shutdown(); @@ -69,11 +94,11 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .post(root, `/api/saved_objects/${dashboard.type}`) .send({ attributes: dashboard.attributes, - workspaces: ['foo'], + workspaces: [createdFooWorkspace.id], }) .expect(200); - expect(createResult.body.workspaces).toEqual(['foo']); + expect(createResult.body.workspaces).toEqual([createdFooWorkspace.id]); await deleteItem({ type: dashboard.type, id: createResult.body.id, @@ -85,7 +110,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .post(root, `/api/saved_objects/${dashboard.type}`) .send({ attributes: dashboard.attributes, - workspaces: ['foo'], + workspaces: [createdFooWorkspace.id], }) .expect(200); @@ -93,7 +118,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .post(root, `/api/saved_objects/${dashboard.type}/${createResult.body.id}?overwrite=true`) .send({ attributes: dashboard.attributes, - workspaces: ['bar'], + workspaces: [createdBarWorkspace.id], }) .expect(409); @@ -106,7 +131,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', it('bulk create', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdFooWorkspace.id}`) .send([ { ...dashboard, @@ -116,7 +141,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .expect(200); const createResultBar = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdBarWorkspace.id}`) .send([ { ...dashboard, @@ -130,7 +155,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', ); expect( (createResultFoo.body.saved_objects as any[]).every((item) => - isEqual(item.workspaces, ['foo']) + isEqual(item.workspaces, [createdFooWorkspace.id]) ) ).toEqual(true); expect((createResultBar.body.saved_objects as any[]).some((item) => item.error)).toEqual( @@ -138,7 +163,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', ); expect( (createResultBar.body.saved_objects as any[]).every((item) => - isEqual(item.workspaces, ['bar']) + isEqual(item.workspaces, [createdBarWorkspace.id]) ) ).toEqual(true); await Promise.all( @@ -154,7 +179,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', it('bulk create with conflict', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdFooWorkspace.id}`) .send([ { ...dashboard, @@ -164,7 +189,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .expect(200); const createResultBar = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdBarWorkspace.id}`) .send([ { ...dashboard, @@ -177,7 +202,10 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', * overwrite with workspaces */ const overwriteWithWorkspacesResult = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?overwrite=true&workspaces=foo`) + .post( + root, + `/api/saved_objects/_bulk_create?overwrite=true&workspaces=${createdFooWorkspace.id}` + ) .send([ { ...dashboard, @@ -195,7 +223,9 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', expect(overwriteWithWorkspacesResult.body.saved_objects[0].error.statusCode).toEqual(409); expect(overwriteWithWorkspacesResult.body.saved_objects[1].attributes.title).toEqual('foo'); - expect(overwriteWithWorkspacesResult.body.saved_objects[1].workspaces).toEqual(['foo']); + expect(overwriteWithWorkspacesResult.body.saved_objects[1].workspaces).toEqual([ + createdFooWorkspace.id, + ]); await Promise.all( [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => @@ -210,7 +240,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', it('checkConflicts when importing ndjson', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdFooWorkspace.id}`) .send([ { ...dashboard, @@ -220,7 +250,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .expect(200); const createResultBar = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdBarWorkspace.id}`) .send([ { ...dashboard, @@ -251,7 +281,10 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', * import with workspaces when conflicts */ const importWithWorkspacesResult = await osdTestServer.request - .post(root, `/api/saved_objects/_import?workspaces=foo&overwrite=false`) + .post( + root, + `/api/saved_objects/_import?workspaces=${createdFooWorkspace.id}&overwrite=false` + ) .attach( 'file', Buffer.from( @@ -279,7 +312,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', it('find by workspaces', async () => { const createResultFoo = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdFooWorkspace.id}`) .send([ { ...dashboard, @@ -289,7 +322,7 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .expect(200); const createResultBar = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .post(root, `/api/saved_objects/_bulk_create?workspaces=${createdBarWorkspace.id}`) .send([ { ...dashboard, @@ -299,11 +332,14 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', .expect(200); const findResult = await osdTestServer.request - .get(root, `/api/saved_objects/_find?workspaces=bar&type=${dashboard.type}`) + .get( + root, + `/api/saved_objects/_find?workspaces=${createdBarWorkspace.id}&type=${dashboard.type}` + ) .expect(200); expect(findResult.body.total).toEqual(1); - expect(findResult.body.saved_objects[0].workspaces).toEqual(['bar']); + expect(findResult.body.saved_objects[0].workspaces).toEqual([createdBarWorkspace.id]); await Promise.all( [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => From f59df29fef6529301b52dec33583bde01cb011a3 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 26 Feb 2024 09:12:53 +0000 Subject: [PATCH 11/31] feat: remove useless config in test Signed-off-by: SuZhou-Joe --- .../saved_objects_wrapper_for_check_workspace_conflict.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts index 05bcd7e6c067..1c8f030cffd3 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -35,9 +35,6 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', osd: { workspace: { enabled: true, - permission: { - enabled: true, - }, }, }, }, From 19f1b8fbdc6fcc1f3a9b230454067c8756a59d73 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 26 Feb 2024 09:17:14 +0000 Subject: [PATCH 12/31] feat: add CHANGELOG Signed-off-by: SuZhou-Joe --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d49655b69e70..35c10e1d5f27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,11 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple Datasource] Add interfaces to register add-on authentication method from plug-in module ([#5851](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5851)) - [Multiple Datasource] Able to Hide "Local Cluster" option from datasource DropDown ([#5827](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5827)) - [Multiple Datasource] Add api registry and allow it to be added into client config in data source plugin ([#5895](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5895)) +- [Multiple Datasource] Concatenate data source name with index pattern name and change delimiter to double colon ([#5907](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5907)) +- [Multiple Datasource] Refactor client and legacy client to use authentication registry ([#5881](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5881)) +- [Multiple Datasource] Improved error handling for the search API when a null value is passed for the dataSourceId ([#5882](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5882)) +- [Multiple Datasource] Hide/Show authentication method in multi data source plugin based on configuration ([#5916](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5916)) +- [Workspace] Optional workspaces params in repository ([#5949](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5949)) ### 🐛 Bug Fixes From b4e97d5a75e8ff3fa5e9d80293ae3f594baf4d2e Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 26 Sep 2023 14:21:20 +0800 Subject: [PATCH 13/31] feat: add more unit test Signed-off-by: SuZhou-Joe --- .../service/lib/repository.test.js | 166 +++++++++++++++++- .../lib/search_dsl/query_params.test.ts | 21 +++ 2 files changed, 186 insertions(+), 1 deletion(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index 816d1013e5be..8914833ea7dc 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -27,7 +27,7 @@ * specific language governing permissions and limitations * under the License. */ - +import { omit } from 'lodash'; import { SavedObjectsRepository } from './repository'; import * as getSearchDslNS from './search_dsl/search_dsl'; import { SavedObjectsErrorHelpers } from './errors'; @@ -629,6 +629,7 @@ describe('SavedObjectsRepository', () => { references: [{ name: 'ref_0', type: 'test', id: '2' }], }; const namespace = 'foo-namespace'; + const workspace = 'foo-workspace'; const getMockBulkCreateResponse = (objects, namespace) => { return { @@ -733,6 +734,18 @@ describe('SavedObjectsRepository', () => { expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); }); + it(`should use the OpenSearch mget action before bulk action for any types, when id is defined`, async () => { + const objects = [obj1, obj2]; + await bulkCreateSuccess(objects); + expect(client.bulk).toHaveBeenCalledTimes(1); + expect(client.mget).toHaveBeenCalledTimes(1); + const docs = [ + expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), + expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), + ]; + expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); + }); + it(`should use the OpenSearch create method if ID is undefined and overwrite=true`, async () => { const objects = [obj1, obj2].map((obj) => ({ ...obj, id: undefined })); await bulkCreateSuccess(objects, { overwrite: true }); @@ -922,6 +935,16 @@ describe('SavedObjectsRepository', () => { await bulkCreateSuccess(objects, { namespace }); expectClientCallArgsAction(objects, { method: 'create', getId }); }); + + it(`adds workspaces to request body for any types`, async () => { + await bulkCreateSuccess([obj1, obj2], { workspaces: [workspace] }); + const expected = expect.objectContaining({ workspaces: [workspace] }); + const body = [expect.any(Object), expected, expect.any(Object), expected]; + expect(client.bulk).toHaveBeenCalledWith( + expect.objectContaining({ body }), + expect.anything() + ); + }); }); describe('errors', () => { @@ -1081,6 +1104,74 @@ describe('SavedObjectsRepository', () => { const expectedError = expectErrorResult(obj3, { message: JSON.stringify(opensearchError) }); await bulkCreateError(obj3, opensearchError, expectedError); }); + + it(`returns error when there is a conflict with an existing saved object according to workspaces`, async () => { + const obj = { ...obj3, workspaces: ['foo'] }; + const response1 = { + status: 200, + docs: [ + { + found: true, + _id: `${obj1.type}:${obj1.id}`, + _source: { + type: obj1.type, + workspaces: ['bar'], + }, + }, + { + found: true, + _id: `${obj.type}:${obj.id}`, + _source: { + type: obj.type, + workspaces: obj.workspaces, + }, + }, + { + found: true, + _id: `${obj2.type}:${obj2.id}`, + _source: { + type: obj2.type, + }, + }, + ], + }; + client.mget.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response1) + ); + const response2 = getMockBulkCreateResponse([obj1, obj, obj2]); + client.bulk.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response2) + ); + + const options = { overwrite: true, workspaces: ['bar'] }; + const result = await savedObjectsRepository.bulkCreate([obj1, obj, obj2], options); + expect(client.bulk).toHaveBeenCalled(); + expect(client.mget).toHaveBeenCalled(); + + const body1 = { + docs: [ + expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), + expect.objectContaining({ _id: `${obj.type}:${obj.id}` }), + expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), + ], + }; + expect(client.mget).toHaveBeenCalledWith( + expect.objectContaining({ body: body1 }), + expect.anything() + ); + const body2 = [...expectObjArgs(obj1)]; + expect(client.bulk).toHaveBeenCalledWith( + expect.objectContaining({ body: body2 }), + expect.anything() + ); + expect(result).toEqual({ + saved_objects: [ + expectSuccess(obj1), + expectErrorConflict(obj, { metadata: { isNotOverwritable: true } }), + expectErrorConflict(obj2, { metadata: { isNotOverwritable: true } }), + ], + }); + }); }); describe('migration', () => { @@ -1903,6 +1994,8 @@ describe('SavedObjectsRepository', () => { const obj5 = { type: MULTI_NAMESPACE_TYPE, id: 'five' }; const obj6 = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'six' }; const obj7 = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'seven' }; + const obj8 = { type: 'dashboard', id: 'eight', workspaces: ['foo'] }; + const obj9 = { type: 'dashboard', id: 'nine', workspaces: ['bar'] }; const namespace = 'foo-namespace'; const checkConflicts = async (objects, options) => @@ -1994,6 +2087,8 @@ describe('SavedObjectsRepository', () => { { found: false }, getMockGetResponse(obj6), { found: false }, + getMockGetResponse(obj7), + getMockGetResponse(obj8), ], }; client.mget.mockResolvedValue( @@ -2022,6 +2117,36 @@ describe('SavedObjectsRepository', () => { ], }); }); + + it(`expected results with workspaces`, async () => { + const objects = [obj8, obj9]; + const response = { + status: 200, + docs: [getMockGetResponse(obj8), getMockGetResponse(obj9)], + }; + client.mget.mockResolvedValue( + opensearchClientMock.createSuccessTransportRequestPromise(response) + ); + + const result = await checkConflicts(objects, { + workspaces: ['foo'], + }); + expect(client.mget).toHaveBeenCalledTimes(1); + expect(result).toEqual({ + errors: [ + { ...omit(obj8, 'workspaces'), error: createConflictError(obj8.type, obj8.id) }, + { + ...omit(obj9, 'workspaces'), + error: { + ...createConflictError(obj9.type, obj9.id), + metadata: { + isNotOverwritable: true, + }, + }, + }, + ], + }); + }); }); }); @@ -2078,6 +2203,7 @@ describe('SavedObjectsRepository', () => { it(`should use the OpenSearch index action if ID is defined and overwrite=true`, async () => { await createSuccess(type, attributes, { id, overwrite: true }); expect(client.index).toHaveBeenCalled(); + expect(client.get).toHaveBeenCalled(); }); it(`should use the OpenSearch index with version if ID and version are defined and overwrite=true`, async () => { @@ -2252,6 +2378,29 @@ describe('SavedObjectsRepository', () => { expect.anything() ); }); + + it(`doesn't modify workspaces when overwrite without target workspaces`, async () => { + const response = getMockGetResponse({ workspaces: ['foo'], id }); + client.get.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response) + ); + + await savedObjectsRepository.create('dashboard', attributes, { + id, + overwrite: true, + workspaces: [], + }); + + expect(client.index).toHaveBeenCalledWith( + expect.objectContaining({ + id: `dashboard:${id}`, + body: expect.objectContaining({ + workspaces: ['foo'], + }), + }), + expect.anything() + ); + }); }); describe('errors', () => { @@ -2312,6 +2461,21 @@ describe('SavedObjectsRepository', () => { expect(client.get).toHaveBeenCalled(); }); + it(`throws error when there is a conflict with an existing workspaces saved object`, async () => { + const response = getMockGetResponse({ workspaces: ['foo'], id }); + client.get.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response) + ); + await expect( + savedObjectsRepository.create('dashboard', attributes, { + id, + overwrite: true, + workspaces: ['bar'], + }) + ).rejects.toThrowError(createConflictError('dashboard', id)); + expect(client.get).toHaveBeenCalled(); + }); + it.todo(`throws when automatic index creation fails`); it.todo(`throws when an unexpected failure occurs`); diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts index a4c1e602829c..9c4cb9519102 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts @@ -644,6 +644,27 @@ describe('#getQueryParams', () => { ]); }); }); + + describe('when using workspace search', () => { + it('using normal workspaces', () => { + const result: Result = getQueryParams({ + registry, + workspaces: ['foo'], + }); + expect(result.query.bool.filter[1]).toEqual({ + bool: { + should: [ + { + bool: { + must: [{ term: { workspaces: 'foo' } }], + }, + }, + ], + minimum_should_match: 1, + }, + }); + }); + }); }); describe('namespaces property', () => { From c8809f0f19910742e476140c4dab17d690ff3378 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 18 Oct 2023 10:22:31 +0800 Subject: [PATCH 14/31] fix: unit test Signed-off-by: SuZhou-Joe --- .../import/import_saved_objects.test.ts | 4 +++- .../saved_objects/import/validate_references.ts | 13 +++++++++++-- src/core/server/saved_objects/types.ts | 1 + 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/core/server/saved_objects/import/import_saved_objects.test.ts b/src/core/server/saved_objects/import/import_saved_objects.test.ts index dcb8d685d42c..1224142ccb55 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.test.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.test.ts @@ -180,7 +180,9 @@ describe('#importSavedObjectsFromStream', () => { expect(validateReferences).toHaveBeenCalledWith( collectedObjects, savedObjectsClient, - namespace + namespace, + undefined, + undefined ); }); diff --git a/src/core/server/saved_objects/import/validate_references.ts b/src/core/server/saved_objects/import/validate_references.ts index fb75eb837443..7a931aec998d 100644 --- a/src/core/server/saved_objects/import/validate_references.ts +++ b/src/core/server/saved_objects/import/validate_references.ts @@ -48,7 +48,8 @@ export async function getNonExistingReferenceAsKeys( savedObjects: SavedObject[], savedObjectsClient: SavedObjectsClientContract, namespace?: string, - retries?: SavedObjectsImportRetry[] + retries?: SavedObjectsImportRetry[], + workspaces?: string[] ) { const objectsToSkip = getObjectsToSkip(retries); const collector = new Map(); @@ -72,8 +73,16 @@ export async function getNonExistingReferenceAsKeys( return []; } + const fields = ['id']; + if (workspaces?.length) { + fields.push('workspaces'); + } + // Fetch references to see if they exist - const bulkGetOpts = Array.from(collector.values()).map((obj) => ({ ...obj, fields: ['id'] })); + const bulkGetOpts = Array.from(collector.values()).map((obj) => ({ + ...obj, + fields, + })); const bulkGetResponse = await savedObjectsClient.bulkGet(bulkGetOpts, { namespace }); // Error handling diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 90ffcd311fca..fef8fc7921f5 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -113,6 +113,7 @@ export interface SavedObjectsFindOptions { typeToNamespacesMap?: Map; /** An optional OpenSearch preference value to be used for the query **/ preference?: string; + /** If specified, will only retrieve objects that are in the workspaces */ workspaces?: string[]; /** * The params here will be combined with bool clause and is used for filtering with ACL structure. From d8e52faf18fd4d69da0ced44040429c914fa0d08 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 27 Feb 2024 11:19:02 +0800 Subject: [PATCH 15/31] feat: revert test in repository.test.js Signed-off-by: SuZhou-Joe --- .../import/validate_references.ts | 10 +- .../service/lib/repository.test.js | 152 +----------------- 2 files changed, 3 insertions(+), 159 deletions(-) diff --git a/src/core/server/saved_objects/import/validate_references.ts b/src/core/server/saved_objects/import/validate_references.ts index 7a931aec998d..545f26ebe1af 100644 --- a/src/core/server/saved_objects/import/validate_references.ts +++ b/src/core/server/saved_objects/import/validate_references.ts @@ -48,8 +48,7 @@ export async function getNonExistingReferenceAsKeys( savedObjects: SavedObject[], savedObjectsClient: SavedObjectsClientContract, namespace?: string, - retries?: SavedObjectsImportRetry[], - workspaces?: string[] + retries?: SavedObjectsImportRetry[] ) { const objectsToSkip = getObjectsToSkip(retries); const collector = new Map(); @@ -73,15 +72,10 @@ export async function getNonExistingReferenceAsKeys( return []; } - const fields = ['id']; - if (workspaces?.length) { - fields.push('workspaces'); - } - // Fetch references to see if they exist const bulkGetOpts = Array.from(collector.values()).map((obj) => ({ ...obj, - fields, + fields: ['id'], })); const bulkGetResponse = await savedObjectsClient.bulkGet(bulkGetOpts, { namespace }); diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index 8914833ea7dc..259b2ac28adf 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -27,7 +27,7 @@ * specific language governing permissions and limitations * under the License. */ -import { omit } from 'lodash'; + import { SavedObjectsRepository } from './repository'; import * as getSearchDslNS from './search_dsl/search_dsl'; import { SavedObjectsErrorHelpers } from './errors'; @@ -734,18 +734,6 @@ describe('SavedObjectsRepository', () => { expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); }); - it(`should use the OpenSearch mget action before bulk action for any types, when id is defined`, async () => { - const objects = [obj1, obj2]; - await bulkCreateSuccess(objects); - expect(client.bulk).toHaveBeenCalledTimes(1); - expect(client.mget).toHaveBeenCalledTimes(1); - const docs = [ - expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), - expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), - ]; - expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); - }); - it(`should use the OpenSearch create method if ID is undefined and overwrite=true`, async () => { const objects = [obj1, obj2].map((obj) => ({ ...obj, id: undefined })); await bulkCreateSuccess(objects, { overwrite: true }); @@ -1104,74 +1092,6 @@ describe('SavedObjectsRepository', () => { const expectedError = expectErrorResult(obj3, { message: JSON.stringify(opensearchError) }); await bulkCreateError(obj3, opensearchError, expectedError); }); - - it(`returns error when there is a conflict with an existing saved object according to workspaces`, async () => { - const obj = { ...obj3, workspaces: ['foo'] }; - const response1 = { - status: 200, - docs: [ - { - found: true, - _id: `${obj1.type}:${obj1.id}`, - _source: { - type: obj1.type, - workspaces: ['bar'], - }, - }, - { - found: true, - _id: `${obj.type}:${obj.id}`, - _source: { - type: obj.type, - workspaces: obj.workspaces, - }, - }, - { - found: true, - _id: `${obj2.type}:${obj2.id}`, - _source: { - type: obj2.type, - }, - }, - ], - }; - client.mget.mockResolvedValueOnce( - opensearchClientMock.createSuccessTransportRequestPromise(response1) - ); - const response2 = getMockBulkCreateResponse([obj1, obj, obj2]); - client.bulk.mockResolvedValueOnce( - opensearchClientMock.createSuccessTransportRequestPromise(response2) - ); - - const options = { overwrite: true, workspaces: ['bar'] }; - const result = await savedObjectsRepository.bulkCreate([obj1, obj, obj2], options); - expect(client.bulk).toHaveBeenCalled(); - expect(client.mget).toHaveBeenCalled(); - - const body1 = { - docs: [ - expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), - expect.objectContaining({ _id: `${obj.type}:${obj.id}` }), - expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), - ], - }; - expect(client.mget).toHaveBeenCalledWith( - expect.objectContaining({ body: body1 }), - expect.anything() - ); - const body2 = [...expectObjArgs(obj1)]; - expect(client.bulk).toHaveBeenCalledWith( - expect.objectContaining({ body: body2 }), - expect.anything() - ); - expect(result).toEqual({ - saved_objects: [ - expectSuccess(obj1), - expectErrorConflict(obj, { metadata: { isNotOverwritable: true } }), - expectErrorConflict(obj2, { metadata: { isNotOverwritable: true } }), - ], - }); - }); }); describe('migration', () => { @@ -1995,7 +1915,6 @@ describe('SavedObjectsRepository', () => { const obj6 = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'six' }; const obj7 = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'seven' }; const obj8 = { type: 'dashboard', id: 'eight', workspaces: ['foo'] }; - const obj9 = { type: 'dashboard', id: 'nine', workspaces: ['bar'] }; const namespace = 'foo-namespace'; const checkConflicts = async (objects, options) => @@ -2117,36 +2036,6 @@ describe('SavedObjectsRepository', () => { ], }); }); - - it(`expected results with workspaces`, async () => { - const objects = [obj8, obj9]; - const response = { - status: 200, - docs: [getMockGetResponse(obj8), getMockGetResponse(obj9)], - }; - client.mget.mockResolvedValue( - opensearchClientMock.createSuccessTransportRequestPromise(response) - ); - - const result = await checkConflicts(objects, { - workspaces: ['foo'], - }); - expect(client.mget).toHaveBeenCalledTimes(1); - expect(result).toEqual({ - errors: [ - { ...omit(obj8, 'workspaces'), error: createConflictError(obj8.type, obj8.id) }, - { - ...omit(obj9, 'workspaces'), - error: { - ...createConflictError(obj9.type, obj9.id), - metadata: { - isNotOverwritable: true, - }, - }, - }, - ], - }); - }); }); }); @@ -2203,7 +2092,6 @@ describe('SavedObjectsRepository', () => { it(`should use the OpenSearch index action if ID is defined and overwrite=true`, async () => { await createSuccess(type, attributes, { id, overwrite: true }); expect(client.index).toHaveBeenCalled(); - expect(client.get).toHaveBeenCalled(); }); it(`should use the OpenSearch index with version if ID and version are defined and overwrite=true`, async () => { @@ -2378,29 +2266,6 @@ describe('SavedObjectsRepository', () => { expect.anything() ); }); - - it(`doesn't modify workspaces when overwrite without target workspaces`, async () => { - const response = getMockGetResponse({ workspaces: ['foo'], id }); - client.get.mockResolvedValueOnce( - opensearchClientMock.createSuccessTransportRequestPromise(response) - ); - - await savedObjectsRepository.create('dashboard', attributes, { - id, - overwrite: true, - workspaces: [], - }); - - expect(client.index).toHaveBeenCalledWith( - expect.objectContaining({ - id: `dashboard:${id}`, - body: expect.objectContaining({ - workspaces: ['foo'], - }), - }), - expect.anything() - ); - }); }); describe('errors', () => { @@ -2461,21 +2326,6 @@ describe('SavedObjectsRepository', () => { expect(client.get).toHaveBeenCalled(); }); - it(`throws error when there is a conflict with an existing workspaces saved object`, async () => { - const response = getMockGetResponse({ workspaces: ['foo'], id }); - client.get.mockResolvedValueOnce( - opensearchClientMock.createSuccessTransportRequestPromise(response) - ); - await expect( - savedObjectsRepository.create('dashboard', attributes, { - id, - overwrite: true, - workspaces: ['bar'], - }) - ).rejects.toThrowError(createConflictError('dashboard', id)); - expect(client.get).toHaveBeenCalled(); - }); - it.todo(`throws when automatic index creation fails`); it.todo(`throws when an unexpected failure occurs`); From 6afe4525b507d70abb18991766bf37fc1ab8a642 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 27 Feb 2024 11:20:50 +0800 Subject: [PATCH 16/31] feat: revert test in import_saved_objects.test.ts Signed-off-by: SuZhou-Joe --- .../server/saved_objects/import/import_saved_objects.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/core/server/saved_objects/import/import_saved_objects.test.ts b/src/core/server/saved_objects/import/import_saved_objects.test.ts index 1224142ccb55..dcb8d685d42c 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.test.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.test.ts @@ -180,9 +180,7 @@ describe('#importSavedObjectsFromStream', () => { expect(validateReferences).toHaveBeenCalledWith( collectedObjects, savedObjectsClient, - namespace, - undefined, - undefined + namespace ); }); From 0fd22297be82c804ca481d71be9337f350334119 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 27 Feb 2024 11:24:23 +0800 Subject: [PATCH 17/31] feat: revert test in repository.test.js Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/service/lib/repository.test.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index 259b2ac28adf..b1bee265266c 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -1914,7 +1914,6 @@ describe('SavedObjectsRepository', () => { const obj5 = { type: MULTI_NAMESPACE_TYPE, id: 'five' }; const obj6 = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'six' }; const obj7 = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'seven' }; - const obj8 = { type: 'dashboard', id: 'eight', workspaces: ['foo'] }; const namespace = 'foo-namespace'; const checkConflicts = async (objects, options) => @@ -2006,8 +2005,6 @@ describe('SavedObjectsRepository', () => { { found: false }, getMockGetResponse(obj6), { found: false }, - getMockGetResponse(obj7), - getMockGetResponse(obj8), ], }; client.mget.mockResolvedValue( From 38071b36ce671d3f89453311859501f76df3a1b7 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 27 Feb 2024 13:16:35 +0800 Subject: [PATCH 18/31] feat: add type Signed-off-by: SuZhou-Joe --- src/core/types/saved_objects.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index fccce5f72947..c1c8ede27593 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -114,6 +114,7 @@ export interface SavedObject { * space. */ originId?: string; + /** Workspace(s) that this saved object exists in. */ workspaces?: string[]; permissions?: Permissions; } From da391ce6d0669a4d765ffee9407349eff1d9f345 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 27 Feb 2024 16:32:35 +0800 Subject: [PATCH 19/31] feat: optimize code and add comment Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/types.ts | 1 + ...apper_for_check_workspace_conflict.test.ts | 351 ++++++++++++++++++ ...ts_wrapper_for_check_workspace_conflict.ts | 153 +++++--- 3 files changed, 454 insertions(+), 51 deletions(-) create mode 100644 src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index fef8fc7921f5..d5333fb40aea 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -132,6 +132,7 @@ export interface SavedObjectsFindOptions { export interface SavedObjectsBaseOptions { /** Specify the namespace for this operation */ namespace?: string; + /** Specify the workspaces for this operation */ workspaces?: string[]; } diff --git a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts new file mode 100644 index 000000000000..cac06d789822 --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -0,0 +1,351 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { SavedObject } from '../../../../core/public'; +import { httpServerMock, savedObjectsClientMock, coreMock } from '../../../../core/server/mocks'; +import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects_wrapper_for_check_workspace_conflict'; +import { SavedObjectsSerializer } from '../../../../core/server'; + +describe('WorkspaceConflictSavedObjectsClientWrapper', () => { + const requestHandlerContext = coreMock.createRequestHandlerContext(); + const wrapperInstance = new WorkspaceConflictSavedObjectsClientWrapper(); + const mockedClient = savedObjectsClientMock.create(); + const wrapperClient = wrapperInstance.wrapperFactory({ + client: mockedClient, + typeRegistry: requestHandlerContext.savedObjects.typeRegistry, + request: httpServerMock.createOpenSearchDashboardsRequest(), + }); + const savedObjectsSerializer = new SavedObjectsSerializer( + requestHandlerContext.savedObjects.typeRegistry + ); + const getSavedObject = (savedObject: Partial) => { + const payload: SavedObject = { + references: [], + id: '', + type: 'dashboard', + attributes: {}, + ...savedObject, + }; + + return payload; + }; + wrapperInstance.setSerializer(savedObjectsSerializer); + describe('createWithWorkspaceConflictCheck', () => { + it(`Should reserve the workspace params when overwrite with empty workspaces`, async () => { + mockedClient.get.mockResolvedValueOnce( + getSavedObject({ + id: 'dashboard:foo', + workspaces: ['foo'], + }) + ); + + await wrapperClient.create( + 'dashboard', + { + name: 'foo', + }, + { + id: 'dashboard:foo', + overwrite: true, + workspaces: [], + } + ); + + expect(mockedClient.create).toBeCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ + workspaces: ['foo'], + }) + ); + }); + + it(`Should return error when overwrite with conflict workspaces`, async () => { + mockedClient.get.mockResolvedValueOnce( + getSavedObject({ + id: 'dashboard:foo', + workspaces: ['foo'], + }) + ); + + await expect( + wrapperClient.create( + 'dashboard', + { + name: 'foo', + }, + { + id: 'dashboard:foo', + overwrite: true, + workspaces: ['bar'], + } + ) + ).rejects.toThrowError('Saved object [dashboard/dashboard:foo] conflict'); + }); + }); + + describe('bulkCreateWithWorkspaceConflictCheck', () => { + beforeEach(() => { + mockedClient.bulkCreate.mockClear(); + }); + it(`Should create objects when no workspaces and id present`, async () => { + mockedClient.bulkCreate.mockResolvedValueOnce({ + saved_objects: [], + }); + await wrapperClient.bulkCreate([ + getSavedObject({ + id: 'foo', + }), + ]); + + expect(mockedClient.bulkGet).not.toBeCalled(); + expect(mockedClient.bulkCreate).toBeCalledWith( + [{ attributes: {}, id: 'foo', references: [], type: 'dashboard' }], + {} + ); + }); + + it(`Should create objects when not overwrite`, async () => { + mockedClient.bulkCreate.mockResolvedValueOnce({ + saved_objects: [], + }); + await wrapperClient.bulkCreate([ + getSavedObject({ + id: 'foo', + workspaces: ['foo'], + }), + ]); + + expect(mockedClient.bulkGet).not.toBeCalled(); + expect(mockedClient.bulkCreate).toBeCalledWith( + [{ attributes: {}, id: 'foo', references: [], type: 'dashboard', workspaces: ['foo'] }], + {} + ); + }); + + it(`Should check conflict on workspace when overwrite`, async () => { + mockedClient.bulkCreate.mockResolvedValueOnce({ + saved_objects: [ + getSavedObject({ + id: 'foo', + workspaces: ['foo'], + }), + getSavedObject({ + id: 'bar', + workspaces: ['foo', 'bar'], + }), + ], + }); + mockedClient.bulkGet.mockResolvedValueOnce({ + saved_objects: [ + getSavedObject({ + id: 'foo', + workspaces: ['foo'], + }), + getSavedObject({ + id: 'bar', + workspaces: ['foo', 'bar'], + }), + getSavedObject({ + id: 'baz', + workspaces: ['baz'], + }), + getSavedObject({ + id: 'qux', + error: { + statusCode: 404, + message: 'object not found', + error: 'object not found', + }, + }), + ], + }); + const result = await wrapperClient.bulkCreate( + [ + getSavedObject({ + id: 'foo', + }), + getSavedObject({ + id: 'bar', + }), + getSavedObject({ + id: 'baz', + }), + getSavedObject({ + id: 'qux', + }), + ], + { + overwrite: true, + workspaces: ['foo'], + } + ); + + expect(mockedClient.bulkGet).toBeCalled(); + expect(mockedClient.bulkCreate).toBeCalledWith( + [ + { attributes: {}, id: 'foo', references: [], type: 'dashboard', workspaces: ['foo'] }, + { + attributes: {}, + id: 'bar', + references: [], + type: 'dashboard', + workspaces: ['foo', 'bar'], + }, + { + attributes: {}, + id: 'qux', + references: [], + type: 'dashboard', + }, + ], + { + overwrite: true, + workspaces: ['foo'], + } + ); + expect(result).toMatchInlineSnapshot(` + Object { + "saved_objects": Array [ + Object { + "attributes": Object {}, + "error": Object { + "error": "Conflict", + "message": "Saved object [dashboard/baz] conflict", + "metadata": Object { + "isNotOverwritable": true, + }, + "statusCode": 409, + }, + "id": "baz", + "references": Array [], + "type": "dashboard", + }, + Object { + "attributes": Object {}, + "id": "foo", + "references": Array [], + "type": "dashboard", + "workspaces": Array [ + "foo", + ], + }, + Object { + "attributes": Object {}, + "id": "bar", + "references": Array [], + "type": "dashboard", + "workspaces": Array [ + "foo", + "bar", + ], + }, + ], + } + `); + }); + }); + + describe('checkConflictWithWorkspaceConflictCheck', () => { + beforeEach(() => { + mockedClient.bulkGet.mockClear(); + }); + + it(`Return early when no objects`, async () => { + const result = await wrapperClient.checkConflicts([]); + expect(result.errors).toEqual([]); + expect(mockedClient.bulkGet).not.toBeCalled(); + }); + + it(`Should filter out workspace conflict objects`, async () => { + mockedClient.checkConflicts.mockResolvedValueOnce({ + errors: [], + }); + mockedClient.bulkGet.mockResolvedValueOnce({ + saved_objects: [ + getSavedObject({ + id: 'foo', + workspaces: ['foo'], + }), + getSavedObject({ + id: 'bar', + workspaces: ['foo', 'bar'], + }), + getSavedObject({ + id: 'baz', + workspaces: ['baz'], + }), + getSavedObject({ + id: 'qux', + error: { + statusCode: 404, + message: 'object not found', + error: 'object not found', + }, + }), + ], + }); + const result = await wrapperClient.checkConflicts( + [ + getSavedObject({ + id: 'foo', + }), + getSavedObject({ + id: 'bar', + }), + getSavedObject({ + id: 'baz', + }), + getSavedObject({ + id: 'qux', + }), + ], + { + workspaces: ['foo'], + } + ); + + expect(mockedClient.bulkGet).toBeCalled(); + expect(mockedClient.checkConflicts).toBeCalledWith( + [ + { attributes: {}, id: 'foo', references: [], type: 'dashboard' }, + { + attributes: {}, + id: 'bar', + references: [], + type: 'dashboard', + }, + { + attributes: {}, + id: 'qux', + references: [], + type: 'dashboard', + }, + ], + { + workspaces: ['foo'], + } + ); + expect(result).toMatchInlineSnapshot(` + Object { + "errors": Array [ + Object { + "error": Object { + "error": "Conflict", + "message": "Saved object [dashboard/baz] conflict", + "metadata": Object { + "isNotOverwritable": true, + }, + "statusCode": 409, + }, + "id": "baz", + "type": "dashboard", + }, + ], + } + `); + }); + }); +}); 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 index 6d8392f3b191..fe8586a76acf 100644 --- 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 @@ -40,12 +40,16 @@ export class WorkspaceConflictSavedObjectsClientWrapper { const { workspaces, id, overwrite } = options; let savedObjectWorkspaces = options?.workspaces; - if (id && overwrite && workspaces) { + /** + * Check if overwrite with id + * If so, need to reserve the workspace params + */ + if (id && overwrite) { 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 item can not be found, supress the error and create the object } if (currentItem) { if ( @@ -72,26 +76,37 @@ export class WorkspaceConflictSavedObjectsClientWrapper { 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'], - }; - }); + /** + * When overwrite, filter out all the objects that have ids + */ + const bulkGetDocs = overwrite + ? objects + .filter((object) => !!object.id) + .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) { + /** + * Get latest status of objects + */ const bulkGetResult = await wrapperOptions.client.bulkGet(bulkGetDocs); bulkGetResult.saved_objects.forEach((object) => { - if (!object.error && object.id && overwrite) { + /** + * Skip the items with error, wrapperOptions.client will handle the error + */ + if (!object.error && object.id) { /** * 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, @@ -105,7 +120,7 @@ export class WorkspaceConflictSavedObjectsClientWrapper { if (filteredWorkspaces.length) { /** * options.workspaces is not a subset of object.workspaces, - * return a conflict error. + * Add the item into conflict array. */ objectsConflictWithWorkspace.push({ id, @@ -118,13 +133,20 @@ export class WorkspaceConflictSavedObjectsClientWrapper { }, }); } else { + /** + * options.workspaces is a subset of object's workspaces + * Add the workspaces status into a objectId -> workspaces pairs for later use. + */ objectsMapWorkspaces[this.getRawId({ namespace, type, id })] = object.workspaces; } } }); } - const objectsFilteredByError = objects.filter( + /** + * Get all the objects that do not conflict on workspaces + */ + const objectsNoWorkspaceConflictError = objects.filter( (item) => !objectsConflictWithWorkspace.find( (errorItems) => @@ -132,29 +154,43 @@ export class WorkspaceConflictSavedObjectsClientWrapper { 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, - }) - ]; + + /** + * Add the workspaces params back based on objects' workspaces value in index. + */ + const objectsPayload = objectsNoWorkspaceConflictError.map((item) => { + if (item.id) { + const workspacesParamsInIndex = + objectsMapWorkspaces[ + this.getRawId({ + namespace, + id: item.id, + type: item.type, + }) + ]; + if (workspacesParamsInIndex) { + item.workspaces = workspacesParamsInIndex; } + } - return item; - }); - } + return item; + }); + + /** + * Bypass those objects that are not conflict on workspaces check. + */ const realBulkCreateResult = await wrapperOptions.client.bulkCreate(objectsPayload, options); - const result: SavedObjectsBulkResponse = { + + /** + * Merge the workspaceConflict result and real client bulkCreate result. + */ + return { ...realBulkCreateResult, - saved_objects: [...objectsConflictWithWorkspace, ...realBulkCreateResult.saved_objects], - }; - return result as SavedObjectsBulkResponse; + saved_objects: [ + ...objectsConflictWithWorkspace, + ...(realBulkCreateResult?.saved_objects || []), + ], + } as SavedObjectsBulkResponse; }; const checkConflictWithWorkspaceConflictCheck = async ( @@ -162,11 +198,17 @@ export class WorkspaceConflictSavedObjectsClientWrapper { options: SavedObjectsBaseOptions = {} ) => { const objectsConflictWithWorkspace: SavedObjectsCheckConflictsResponse['errors'] = []; - if (options.workspaces) { - if (objects.length === 0) { - return { errors: [] }; - } + /** + * Fail early when no objects + */ + if (objects.length === 0) { + return { errors: [] }; + } + /** + * Workspace conflict only happens when target workspaces params present. + */ + if (options.workspaces) { const bulkGetDocs: any[] = objects.map((object) => { const { type, id } = object; @@ -182,16 +224,17 @@ export class WorkspaceConflictSavedObjectsClientWrapper { bulkGetResult.saved_objects.forEach((object) => { const { id, type } = object; + /** + * Skip the error ones, real checkConflict in repository will handle that. + */ if (!object.error) { let workspaceConflict = false; - if (options.workspaces) { - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( - options.workspaces, - object.workspaces - ); - if (filteredWorkspaces.length) { - workspaceConflict = true; - } + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( + options.workspaces, + object.workspaces + ); + if (filteredWorkspaces.length) { + workspaceConflict = true; } if (workspaceConflict) { objectsConflictWithWorkspace.push({ @@ -208,7 +251,7 @@ export class WorkspaceConflictSavedObjectsClientWrapper { } } - const objectsFilteredByError = objects.filter( + const objectsNoWorkspaceConflictError = objects.filter( (item) => !objectsConflictWithWorkspace.find( (errorItems) => @@ -224,10 +267,18 @@ export class WorkspaceConflictSavedObjectsClientWrapper { }) ) ); + + /** + * Bypass those objects that are not conflict on workspaces + */ const realBulkCreateResult = await wrapperOptions.client.checkConflicts( - objectsFilteredByError, + objectsNoWorkspaceConflictError, options ); + + /** + * Merge results from two conflict check. + */ const result: SavedObjectsCheckConflictsResponse = { ...realBulkCreateResult, errors: [...objectsConflictWithWorkspace, ...realBulkCreateResult.errors], From 406da0ed31962a01934aab4cd9cfa2b9213a5d58 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 27 Feb 2024 17:23:47 +0800 Subject: [PATCH 20/31] fix: unit test error Signed-off-by: SuZhou-Joe --- .../saved_objects/export/get_sorted_objects_for_export.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts index 8ca085639f10..189318522bec 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts @@ -125,7 +125,7 @@ async function fetchObjectsToExport({ search, perPage: exportSizeLimit, namespaces: namespace ? [namespace] : undefined, - workspaces, + ...(workspaces ? { workspaces } : {}), }); if (findResponse.total > exportSizeLimit) { throw Boom.badRequest(`Can't export more than ${exportSizeLimit} objects`); From ac0df5f86c3875a582b878bbfd4eb85ce0af36b4 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 28 Feb 2024 01:42:49 +0000 Subject: [PATCH 21/31] fix: integration test fail Signed-off-by: SuZhou-Joe --- .../saved_objects/import/create_saved_objects.ts | 2 ++ .../saved_objects/import/import_saved_objects.ts | 2 ++ src/core/server/saved_objects/import/types.ts | 2 ++ src/core/server/saved_objects/routes/import.ts | 4 ++++ ...bjects_wrapper_for_check_workspace_conflict.test.ts | 10 ---------- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/core/server/saved_objects/import/create_saved_objects.ts b/src/core/server/saved_objects/import/create_saved_objects.ts index 67bc8dfc6227..692826c4ac8a 100644 --- a/src/core/server/saved_objects/import/create_saved_objects.ts +++ b/src/core/server/saved_objects/import/create_saved_objects.ts @@ -42,6 +42,7 @@ interface CreateSavedObjectsParams { workspaces?: string[]; dataSourceId?: string; dataSourceTitle?: string; + workspaces?: string[]; } interface CreateSavedObjectsResult { createdObjects: Array>; @@ -62,6 +63,7 @@ export const createSavedObjects = async ({ workspaces, dataSourceId, dataSourceTitle, + workspaces, }: CreateSavedObjectsParams): Promise> => { // filter out any objects that resulted in errors const errorSet = accumulatedErrors.reduce( diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts index 51a790aca5d2..406057d37a24 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -58,6 +58,7 @@ export async function importSavedObjectsFromStream({ workspaces, dataSourceId, dataSourceTitle, + workspaces, }: SavedObjectsImportOptions): Promise { let errorAccumulator: SavedObjectsImportError[] = []; const supportedTypes = typeRegistry.getImportableAndExportableTypes().map((type) => type.name); @@ -152,6 +153,7 @@ export async function importSavedObjectsFromStream({ ...(workspaces ? { workspaces } : {}), dataSourceId, dataSourceTitle, + ...(workspaces ? { workspaces } : {}), }; const createSavedObjectsResult = await createSavedObjects(createSavedObjectsParams); errorAccumulator = [...errorAccumulator, ...createSavedObjectsResult.errors]; diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index a02d42e5c363..5bfd46dc78a8 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -191,6 +191,8 @@ export interface SavedObjectsImportOptions { workspaces?: string[]; dataSourceId?: string; dataSourceTitle?: string; + /** if specified, will import in given workspaces */ + workspaces?: string[]; } /** diff --git a/src/core/server/saved_objects/routes/import.ts b/src/core/server/saved_objects/routes/import.ts index 6c7cc77d9b84..c8878439b6c8 100644 --- a/src/core/server/saved_objects/routes/import.ts +++ b/src/core/server/saved_objects/routes/import.ts @@ -64,6 +64,9 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig) schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) ), dataSourceId: schema.maybe(schema.string({ defaultValue: '' })), + workspaces: schema.maybe( + schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) + ), }, { validate: (object) => { @@ -126,6 +129,7 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig) workspaces, dataSourceId, dataSourceTitle, + workspaces, }); return res.ok({ body: result }); diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts index 1c8f030cffd3..570dbb6a59e8 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -5,7 +5,6 @@ import { SavedObject } from 'src/core/types'; import { isEqual } from 'lodash'; -import { Readable } from 'stream'; import * as osdTestServer from '../../../../../core/test_helpers/osd_server'; const dashboard: Omit = { @@ -265,15 +264,6 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', id: 'bar', }); - const readableStream = new Readable(); - readableStream.push( - `Content-Disposition: form-data; name="file"; filename="tmp.ndjson"\r\n\r\n` - ); - readableStream.push( - [JSON.stringify(getResultFoo.body), JSON.stringify(getResultBar.body)].join('\n') - ); - readableStream.push(null); - /** * import with workspaces when conflicts */ From 1efb8ba2aecbd26192156c5ecbcb438a2984b077 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 28 Feb 2024 10:32:34 +0800 Subject: [PATCH 22/31] feat: add missing code Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/import/resolve_import_errors.ts | 2 ++ src/core/server/saved_objects/import/types.ts | 2 ++ .../saved_objects/migrations/core/index_migrator.test.ts | 3 +++ src/core/server/saved_objects/routes/resolve_import_errors.ts | 3 +++ 4 files changed, 10 insertions(+) diff --git a/src/core/server/saved_objects/import/resolve_import_errors.ts b/src/core/server/saved_objects/import/resolve_import_errors.ts index a3d10c6f1ace..5d6557ff4429 100644 --- a/src/core/server/saved_objects/import/resolve_import_errors.ts +++ b/src/core/server/saved_objects/import/resolve_import_errors.ts @@ -62,6 +62,7 @@ export async function resolveSavedObjectsImportErrors({ workspaces, dataSourceId, dataSourceTitle, + workspaces, }: SavedObjectsResolveImportErrorsOptions): Promise { // throw a BadRequest error if we see invalid retries validateRetries(retries); @@ -166,6 +167,7 @@ export async function resolveSavedObjectsImportErrors({ workspaces, dataSourceId, dataSourceTitle, + workspaces, }; const { createdObjects, errors: bulkCreateErrors } = await createSavedObjects( createSavedObjectsParams diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index 5bfd46dc78a8..a8d2ae11c8e5 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -218,6 +218,8 @@ export interface SavedObjectsResolveImportErrorsOptions { workspaces?: string[]; dataSourceId?: string; dataSourceTitle?: string; + /** if specified, will import in given workspaces */ + workspaces?: string[]; } export type CreatedObject = SavedObject & { destinationId?: string }; diff --git a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts index 08bc4162a807..ad730c1f3330 100644 --- a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts @@ -159,6 +159,7 @@ describe('IndexMigrator', () => { id: { type: 'keyword' }, }, }, + workspaces: { type: 'keyword' }, }, }, settings: { number_of_shards: 1, auto_expand_replicas: '0-1' }, @@ -336,6 +337,7 @@ describe('IndexMigrator', () => { id: { type: 'keyword' }, }, }, + workspaces: { type: 'keyword' }, }, }, settings: { number_of_shards: 1, auto_expand_replicas: '0-1' }, @@ -456,6 +458,7 @@ describe('IndexMigrator', () => { id: { type: 'keyword' }, }, }, + workspaces: { type: 'keyword' }, }, }, settings: { number_of_shards: 1, auto_expand_replicas: '0-1' }, diff --git a/src/core/server/saved_objects/routes/resolve_import_errors.ts b/src/core/server/saved_objects/routes/resolve_import_errors.ts index dedcc960a675..0ea91e43a777 100644 --- a/src/core/server/saved_objects/routes/resolve_import_errors.ts +++ b/src/core/server/saved_objects/routes/resolve_import_errors.ts @@ -62,6 +62,9 @@ export const registerResolveImportErrorsRoute = (router: IRouter, config: SavedO schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) ), dataSourceId: schema.maybe(schema.string({ defaultValue: '' })), + workspaces: schema.maybe( + schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) + ), }), body: schema.object({ file: schema.stream(), From 225d64da819a17a6b6a4bd5ea62237abebada628 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 28 Feb 2024 11:36:36 +0800 Subject: [PATCH 23/31] feat: fix bootstrap error Signed-off-by: SuZhou-Joe --- .../server/saved_objects/import/create_saved_objects.ts | 2 -- .../server/saved_objects/import/import_saved_objects.ts | 1 - .../server/saved_objects/import/resolve_import_errors.ts | 2 -- src/core/server/saved_objects/import/types.ts | 8 ++------ .../saved_objects/migrations/core/index_migrator.test.ts | 3 --- src/core/server/saved_objects/routes/import.ts | 4 ---- .../server/saved_objects/routes/resolve_import_errors.ts | 3 --- 7 files changed, 2 insertions(+), 21 deletions(-) diff --git a/src/core/server/saved_objects/import/create_saved_objects.ts b/src/core/server/saved_objects/import/create_saved_objects.ts index 692826c4ac8a..67bc8dfc6227 100644 --- a/src/core/server/saved_objects/import/create_saved_objects.ts +++ b/src/core/server/saved_objects/import/create_saved_objects.ts @@ -42,7 +42,6 @@ interface CreateSavedObjectsParams { workspaces?: string[]; dataSourceId?: string; dataSourceTitle?: string; - workspaces?: string[]; } interface CreateSavedObjectsResult { createdObjects: Array>; @@ -63,7 +62,6 @@ export const createSavedObjects = async ({ workspaces, dataSourceId, dataSourceTitle, - workspaces, }: CreateSavedObjectsParams): Promise> => { // filter out any objects that resulted in errors const errorSet = accumulatedErrors.reduce( diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts index 406057d37a24..ea67d74f5ff8 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -58,7 +58,6 @@ export async function importSavedObjectsFromStream({ workspaces, dataSourceId, dataSourceTitle, - workspaces, }: SavedObjectsImportOptions): Promise { let errorAccumulator: SavedObjectsImportError[] = []; const supportedTypes = typeRegistry.getImportableAndExportableTypes().map((type) => type.name); diff --git a/src/core/server/saved_objects/import/resolve_import_errors.ts b/src/core/server/saved_objects/import/resolve_import_errors.ts index 5d6557ff4429..a3d10c6f1ace 100644 --- a/src/core/server/saved_objects/import/resolve_import_errors.ts +++ b/src/core/server/saved_objects/import/resolve_import_errors.ts @@ -62,7 +62,6 @@ export async function resolveSavedObjectsImportErrors({ workspaces, dataSourceId, dataSourceTitle, - workspaces, }: SavedObjectsResolveImportErrorsOptions): Promise { // throw a BadRequest error if we see invalid retries validateRetries(retries); @@ -167,7 +166,6 @@ export async function resolveSavedObjectsImportErrors({ workspaces, dataSourceId, dataSourceTitle, - workspaces, }; const { createdObjects, errors: bulkCreateErrors } = await createSavedObjects( createSavedObjectsParams diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index a8d2ae11c8e5..d0433b72766a 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -187,12 +187,10 @@ export interface SavedObjectsImportOptions { namespace?: string; /** If true, will create new copies of import objects, each with a random `id` and undefined `originId`. */ createNewCopies: boolean; - /** if specified, will import in given workspaces, else will import as global object */ + /** if specified, will import in given workspaces */ workspaces?: string[]; dataSourceId?: string; dataSourceTitle?: string; - /** if specified, will import in given workspaces */ - workspaces?: string[]; } /** @@ -214,12 +212,10 @@ export interface SavedObjectsResolveImportErrorsOptions { namespace?: string; /** If true, will create new copies of import objects, each with a random `id` and undefined `originId`. */ createNewCopies: boolean; - /** if specified, will import in given workspaces, else will import as global object */ + /** if specified, will import in given workspaces */ workspaces?: string[]; dataSourceId?: string; dataSourceTitle?: string; - /** if specified, will import in given workspaces */ - workspaces?: string[]; } export type CreatedObject = SavedObject & { destinationId?: string }; diff --git a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts index ad730c1f3330..08bc4162a807 100644 --- a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts @@ -159,7 +159,6 @@ describe('IndexMigrator', () => { id: { type: 'keyword' }, }, }, - workspaces: { type: 'keyword' }, }, }, settings: { number_of_shards: 1, auto_expand_replicas: '0-1' }, @@ -337,7 +336,6 @@ describe('IndexMigrator', () => { id: { type: 'keyword' }, }, }, - workspaces: { type: 'keyword' }, }, }, settings: { number_of_shards: 1, auto_expand_replicas: '0-1' }, @@ -458,7 +456,6 @@ describe('IndexMigrator', () => { id: { type: 'keyword' }, }, }, - workspaces: { type: 'keyword' }, }, }, settings: { number_of_shards: 1, auto_expand_replicas: '0-1' }, diff --git a/src/core/server/saved_objects/routes/import.ts b/src/core/server/saved_objects/routes/import.ts index c8878439b6c8..6c7cc77d9b84 100644 --- a/src/core/server/saved_objects/routes/import.ts +++ b/src/core/server/saved_objects/routes/import.ts @@ -64,9 +64,6 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig) schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) ), dataSourceId: schema.maybe(schema.string({ defaultValue: '' })), - workspaces: schema.maybe( - schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) - ), }, { validate: (object) => { @@ -129,7 +126,6 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig) workspaces, dataSourceId, dataSourceTitle, - workspaces, }); return res.ok({ body: result }); diff --git a/src/core/server/saved_objects/routes/resolve_import_errors.ts b/src/core/server/saved_objects/routes/resolve_import_errors.ts index 0ea91e43a777..dedcc960a675 100644 --- a/src/core/server/saved_objects/routes/resolve_import_errors.ts +++ b/src/core/server/saved_objects/routes/resolve_import_errors.ts @@ -62,9 +62,6 @@ export const registerResolveImportErrorsRoute = (router: IRouter, config: SavedO schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) ), dataSourceId: schema.maybe(schema.string({ defaultValue: '' })), - workspaces: schema.maybe( - schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) - ), }), body: schema.object({ file: schema.stream(), From fd447ff35b6926fea1d78ca9bb8bef70dd8d58c1 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 28 Feb 2024 11:56:49 +0800 Subject: [PATCH 24/31] feat: remove useless code Signed-off-by: SuZhou-Joe --- .../saved_objects/import/import_saved_objects.ts | 7 ------- .../saved_objects/service/lib/repository.ts | 15 ++------------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts index ea67d74f5ff8..213d09767fbb 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -86,13 +86,6 @@ export async function importSavedObjectsFromStream({ // randomly generated id importIdMap = regenerateIds(collectSavedObjectsResult.collectedObjects, dataSourceId); } else { - importIdMap = await regenerateIdsWithReference({ - savedObjects: collectSavedObjectsResult.collectedObjects, - savedObjectsClient, - workspaces, - objectLimit, - importIdMap, - }); // in check conclict and override mode // Check single-namespace objects for conflicts in this namespace, and check multi-namespace objects for conflicts across all namespaces const checkConflictsParams = { diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 96308104d6a0..7418fc71ba88 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -589,7 +589,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( @@ -612,24 +612,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 }, }), }, From 672b3eea544aea7c1eff27442ee53d73f413a67a Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 28 Feb 2024 13:16:57 +0800 Subject: [PATCH 25/31] feat: remove useless code Signed-off-by: SuZhou-Joe --- .../import/import_saved_objects.test.ts | 12 +----- .../import/import_saved_objects.ts | 2 +- .../saved_objects/import/regenerate_ids.ts | 40 +------------------ 3 files changed, 3 insertions(+), 51 deletions(-) diff --git a/src/core/server/saved_objects/import/import_saved_objects.test.ts b/src/core/server/saved_objects/import/import_saved_objects.test.ts index dcb8d685d42c..3dda6931bd1e 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.test.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.test.ts @@ -42,7 +42,7 @@ import { typeRegistryMock } from '../saved_objects_type_registry.mock'; import { importSavedObjectsFromStream } from './import_saved_objects'; import { collectSavedObjects } from './collect_saved_objects'; -import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; +import { regenerateIds } from './regenerate_ids'; import { validateReferences } from './validate_references'; import { checkConflicts } from './check_conflicts'; import { checkOriginConflicts } from './check_origin_conflicts'; @@ -70,7 +70,6 @@ describe('#importSavedObjectsFromStream', () => { importIdMap: new Map(), }); getMockFn(regenerateIds).mockReturnValue(new Map()); - getMockFn(regenerateIdsWithReference).mockReturnValue(Promise.resolve(new Map())); getMockFn(validateReferences).mockResolvedValue([]); getMockFn(checkConflicts).mockResolvedValue({ errors: [], @@ -279,15 +278,6 @@ describe('#importSavedObjectsFromStream', () => { ]), }); getMockFn(validateReferences).mockResolvedValue([errors[1]]); - getMockFn(regenerateIdsWithReference).mockResolvedValue( - Promise.resolve( - new Map([ - ['foo', {}], - ['bar', {}], - ['baz', {}], - ]) - ) - ); getMockFn(checkConflicts).mockResolvedValue({ errors: [errors[2]], filteredObjects, diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts index 213d09767fbb..7cdb6970ca9d 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -38,7 +38,7 @@ import { validateReferences } from './validate_references'; import { checkOriginConflicts } from './check_origin_conflicts'; import { createSavedObjects } from './create_saved_objects'; import { checkConflicts } from './check_conflicts'; -import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; +import { regenerateIds } from './regenerate_ids'; import { checkConflictsForDataSource } from './check_conflict_for_data_source'; /** diff --git a/src/core/server/saved_objects/import/regenerate_ids.ts b/src/core/server/saved_objects/import/regenerate_ids.ts index 522a150c4e6f..f1092bed7f55 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.ts +++ b/src/core/server/saved_objects/import/regenerate_ids.ts @@ -29,8 +29,7 @@ */ import { v4 as uuidv4 } from 'uuid'; -import { SavedObject, SavedObjectsClientContract } from '../types'; -import { SavedObjectsUtils } from '../service'; +import { SavedObject } from '../types'; /** * Takes an array of saved objects and returns an importIdMap of randomly-generated new IDs. @@ -48,40 +47,3 @@ export const regenerateIds = (objects: SavedObject[], dataSourceId: string | und }, new Map()); return importIdMap; }; - -export const regenerateIdsWithReference = async (props: { - savedObjects: SavedObject[]; - savedObjectsClient: SavedObjectsClientContract; - workspaces?: string[]; - objectLimit: number; - importIdMap: Map; -}): Promise> => { - const { savedObjects, savedObjectsClient, workspaces, importIdMap } = props; - if (!workspaces || !workspaces.length) { - return savedObjects.reduce((acc, object) => { - return acc.set(`${object.type}:${object.id}`, { id: object.id, omitOriginId: false }); - }, importIdMap); - } - - const bulkGetResult = await savedObjectsClient.bulkGet( - savedObjects.map((item) => ({ type: item.type, id: item.id })) - ); - - return bulkGetResult.saved_objects.reduce((acc, object) => { - if (object.error?.statusCode === 404) { - acc.set(`${object.type}:${object.id}`, { id: object.id, omitOriginId: true }); - return acc; - } - - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( - workspaces, - object.workspaces - ); - if (filteredWorkspaces.length) { - acc.set(`${object.type}:${object.id}`, { id: uuidv4(), omitOriginId: true }); - } else { - acc.set(`${object.type}:${object.id}`, { id: object.id, omitOriginId: false }); - } - return acc; - }, importIdMap); -}; From ddbeb5860fad0d00effeceacdc5cd6cc66640946 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 28 Feb 2024 13:23:16 +0800 Subject: [PATCH 26/31] feat: remove useless code Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/service/saved_objects_client.ts | 4 ---- 1 file changed, 4 deletions(-) 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 c9990977bb48..a53684c833e2 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -71,10 +71,6 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions { initialNamespaces?: string[]; /** permission control describe by ACL object */ permissions?: Permissions; - /** - * workspaces the new created objects belong to - */ - workspaces?: string[]; } /** From e59ab7124eb242f10a2d14524d86bffe7dc5cd8e Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 28 Feb 2024 13:31:49 +0800 Subject: [PATCH 27/31] feat: optimize code Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/service/saved_objects_client.ts | 4 ---- 1 file changed, 4 deletions(-) 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 c9990977bb48..a53684c833e2 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -71,10 +71,6 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions { initialNamespaces?: string[]; /** permission control describe by ACL object */ permissions?: Permissions; - /** - * workspaces the new created objects belong to - */ - workspaces?: string[]; } /** From cbcc035329b0ba5acf5a7559b39a1c9271de9224 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 28 Feb 2024 18:04:23 +0800 Subject: [PATCH 28/31] fix: unit test Signed-off-by: SuZhou-Joe --- .../get_sorted_objects_for_export.test.ts | 4 -- .../open_search_panel.test.js.snap | 69 ------------------- 2 files changed, 73 deletions(-) delete mode 100644 src/plugins/discover_legacy/public/application/components/top_nav/__snapshots__/open_search_panel.test.js.snap diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts index 952a74a76940..19737c9f8dec 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts @@ -128,7 +128,6 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], - workspaces: undefined, }, ], ], @@ -219,7 +218,6 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], - workspaces: undefined, }, ], ], @@ -370,7 +368,6 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], - workspaces: undefined, }, ], ], @@ -462,7 +459,6 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], - workspaces: undefined, }, ], ], diff --git a/src/plugins/discover_legacy/public/application/components/top_nav/__snapshots__/open_search_panel.test.js.snap b/src/plugins/discover_legacy/public/application/components/top_nav/__snapshots__/open_search_panel.test.js.snap deleted file mode 100644 index 54c90bd3ce92..000000000000 --- a/src/plugins/discover_legacy/public/application/components/top_nav/__snapshots__/open_search_panel.test.js.snap +++ /dev/null @@ -1,69 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`render 1`] = ` - - - -

- -

- - - - - } - onChoose={[Function]} - savedObjectMetaData={ - Array [ - Object { - "getIconForSavedObject": [Function], - "name": "Saved search", - "type": "search", - }, - ] - } - savedObjects={Object {}} - uiSettings={Object {}} - /> - - - - - - - - - - - -`; From 200b204028bddae953a97cb431338b1c0e956cfd Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 28 Feb 2024 18:30:51 +0800 Subject: [PATCH 29/31] feat: remove useless code Signed-off-by: SuZhou-Joe --- .../components/top_nav/open_search_panel.js | 122 ------------------ 1 file changed, 122 deletions(-) delete mode 100644 src/plugins/discover_legacy/public/application/components/top_nav/open_search_panel.js diff --git a/src/plugins/discover_legacy/public/application/components/top_nav/open_search_panel.js b/src/plugins/discover_legacy/public/application/components/top_nav/open_search_panel.js deleted file mode 100644 index 7f5d04d415e7..000000000000 --- a/src/plugins/discover_legacy/public/application/components/top_nav/open_search_panel.js +++ /dev/null @@ -1,122 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Any modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import React from 'react'; -import PropTypes from 'prop-types'; -import rison from 'rison-node'; -import { i18n } from '@osd/i18n'; -import { FormattedMessage } from '@osd/i18n/react'; -import { - EuiButton, - EuiFlexGroup, - EuiFlexItem, - EuiFlyout, - EuiFlyoutHeader, - EuiFlyoutFooter, - EuiFlyoutBody, - EuiTitle, -} from '@elastic/eui'; -import { SavedObjectFinderUi } from '../../../../../saved_objects/public'; -import { getServices } from '../../../opensearch_dashboards_services'; - -const SEARCH_OBJECT_TYPE = 'search'; - -export function OpenSearchPanel(props) { - const { - core: { uiSettings, savedObjects }, - addBasePath, - } = getServices(); - - return ( - - - -

- -

- - - - - } - savedObjectMetaData={[ - { - type: SEARCH_OBJECT_TYPE, - getIconForSavedObject: () => 'search', - name: i18n.translate('discover.savedSearch.savedObjectName', { - defaultMessage: 'Saved search', - }), - }, - ]} - onChoose={(id) => { - window.location.assign(props.makeUrl(id)); - props.onClose(); - }} - uiSettings={uiSettings} - savedObjects={savedObjects} - /> - - - - - {/* eslint-disable-next-line @elastic/eui/href-or-on-click */} - - - - - - - - ); -} - -OpenSearchPanel.propTypes = { - onClose: PropTypes.func.isRequired, - makeUrl: PropTypes.func.isRequired, -}; From 21a6b2218e2a94fb0cca02ad58feed44a7d57f17 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 29 Feb 2024 15:59:47 +0800 Subject: [PATCH 30/31] feat: optimize code Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/service/lib/utils.ts | 7 ------- ...objects_wrapper_for_check_workspace_conflict.ts | 14 ++++++++------ 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/utils.ts b/src/core/server/saved_objects/service/lib/utils.ts index ca719887986d..4823e52d77c9 100644 --- a/src/core/server/saved_objects/service/lib/utils.ts +++ b/src/core/server/saved_objects/service/lib/utils.ts @@ -80,11 +80,4 @@ export class SavedObjectsUtils { total: 0, saved_objects: [], }); - - public static filterWorkspacesAccordingToSourceWorkspaces( - targetWorkspaces?: string[], - baseWorkspaces?: string[] - ): string[] { - return targetWorkspaces?.filter((item) => !baseWorkspaces?.includes(item)) || []; - } } 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 index fe8586a76acf..f2d0eb2c732c 100644 --- 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 @@ -20,6 +20,11 @@ import { const errorContent = (error: Boom.Boom) => error.output.payload; +const filterWorkspacesAccordingToSourceWorkspaces = ( + targetWorkspaces?: string[], + baseWorkspaces?: string[] +): string[] => targetWorkspaces?.filter((item) => !baseWorkspaces?.includes(item)) || []; + export class WorkspaceConflictSavedObjectsClientWrapper { private _serializer?: SavedObjectsSerializer; public setSerializer(serializer: SavedObjectsSerializer) { @@ -53,10 +58,7 @@ export class WorkspaceConflictSavedObjectsClientWrapper { } if (currentItem) { if ( - SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( - workspaces, - currentItem.workspaces - ).length + filterWorkspacesAccordingToSourceWorkspaces(workspaces, currentItem.workspaces).length ) { throw SavedObjectsErrorHelpers.createConflictError(type, id); } else { @@ -112,7 +114,7 @@ export class WorkspaceConflictSavedObjectsClientWrapper { * 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( + const filteredWorkspaces = filterWorkspacesAccordingToSourceWorkspaces( options.workspaces, object.workspaces ); @@ -229,7 +231,7 @@ export class WorkspaceConflictSavedObjectsClientWrapper { */ if (!object.error) { let workspaceConflict = false; - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( + const filteredWorkspaces = filterWorkspacesAccordingToSourceWorkspaces( options.workspaces, object.workspaces ); From 25c32e5cca26c69979769ff8b5974a73b9abcb1b Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 29 Feb 2024 16:01:55 +0800 Subject: [PATCH 31/31] feat: remove useless CHANGELOG Signed-off-by: SuZhou-Joe --- CHANGELOG.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35c10e1d5f27..d49655b69e70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,11 +41,6 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple Datasource] Add interfaces to register add-on authentication method from plug-in module ([#5851](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5851)) - [Multiple Datasource] Able to Hide "Local Cluster" option from datasource DropDown ([#5827](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5827)) - [Multiple Datasource] Add api registry and allow it to be added into client config in data source plugin ([#5895](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5895)) -- [Multiple Datasource] Concatenate data source name with index pattern name and change delimiter to double colon ([#5907](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5907)) -- [Multiple Datasource] Refactor client and legacy client to use authentication registry ([#5881](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5881)) -- [Multiple Datasource] Improved error handling for the search API when a null value is passed for the dataSourceId ([#5882](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5882)) -- [Multiple Datasource] Hide/Show authentication method in multi data source plugin based on configuration ([#5916](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5916)) -- [Workspace] Optional workspaces params in repository ([#5949](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5949)) ### 🐛 Bug Fixes