From 66d6096cceec516e2eaadfbf9a5cee4738fd4f2a Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 13 Mar 2024 17:33:09 +0800 Subject: [PATCH 01/21] feat: POC implementation Signed-off-by: SuZhou-Joe --- src/plugins/workspace/common/constants.ts | 2 + .../workspace/opensearch_dashboards.json | 4 +- src/plugins/workspace/server/constant.ts | 11 ++ src/plugins/workspace/server/plugin.ts | 13 ++ .../workspace_id_consumer_wrapper.ts | 117 ++++++++++++++++++ .../workspace/server/workspace_client.ts | 1 + 6 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 src/plugins/workspace/server/constant.ts create mode 100644 src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts index 2be4a9d121db..1781b7e32771 100644 --- a/src/plugins/workspace/common/constants.ts +++ b/src/plugins/workspace/common/constants.ts @@ -20,3 +20,5 @@ export enum WorkspacePermissionMode { LibraryRead = 'library_read', LibraryWrite = 'library_write', } + +export const WORKSPACE_ID_CONSUMER_WRAPPER_ID = 'workspace_id_consumer'; diff --git a/src/plugins/workspace/opensearch_dashboards.json b/src/plugins/workspace/opensearch_dashboards.json index 7d94a7491a00..0f1d851d5d76 100644 --- a/src/plugins/workspace/opensearch_dashboards.json +++ b/src/plugins/workspace/opensearch_dashboards.json @@ -3,9 +3,7 @@ "version": "opensearchDashboards", "server": true, "ui": true, - "requiredPlugins": [ - "savedObjects" - ], + "requiredPlugins": [], "optionalPlugins": ["savedObjectsManagement"], "requiredBundles": ["opensearchDashboardsReact"] } diff --git a/src/plugins/workspace/server/constant.ts b/src/plugins/workspace/server/constant.ts new file mode 100644 index 000000000000..d3cd904b1cf5 --- /dev/null +++ b/src/plugins/workspace/server/constant.ts @@ -0,0 +1,11 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * A symbol used for imply the workspace id in original url + * + * @Internal + */ +export const workspaceIdInUrlSymbol = Symbol('workspace_id_in_url'); diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 0fff0082476b..5c42ae476033 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -15,6 +15,7 @@ import { import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID, + WORKSPACE_ID_CONSUMER_WRAPPER_ID, } from '../common/constants'; import { IWorkspaceClientImpl } from './types'; import { WorkspaceClient } from './workspace_client'; @@ -27,6 +28,10 @@ import { SavedObjectsPermissionControlContract, } from './permission_control/client'; import { WorkspacePluginConfigType } from '../config'; +import { workspaceIdInUrlSymbol } from './constant'; +import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; +// eslint-disable-next-line @osd/eslint/no-restricted-paths +import { ensureRawRequest } from '../../../core/server/http/router'; // will be an issue as the ensureRawRequest is an internal implementation export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; @@ -47,6 +52,8 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); if (workspaceId) { + const rawRequest = ensureRawRequest(request); + rawRequest.headers[workspaceIdInUrlSymbol.toString()] = workspaceId; const requestUrl = new URL(request.url.toString()); requestUrl.pathname = cleanWorkspaceId(requestUrl.pathname); return toolkit.rewriteUrl(requestUrl.toString()); @@ -94,6 +101,12 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); } + core.savedObjects.addClientWrapper( + -2, + WORKSPACE_ID_CONSUMER_WRAPPER_ID, + new WorkspaceIdConsumerWrapper().wrapperFactory + ); + registerRoutes({ http: core.http, logger: this.logger, diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts new file mode 100644 index 000000000000..4c302fc93518 --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -0,0 +1,117 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + SavedObjectsBaseOptions, + SavedObjectsBulkCreateObject, + SavedObjectsClientWrapperFactory, + SavedObjectsCreateOptions, + SavedObjectsCheckConflictsObject, + OpenSearchDashboardsRequest, + SavedObjectsFindOptions, + SavedObjectsUpdateOptions, + SavedObjectsBulkUpdateOptions, + SavedObjectsBulkUpdateObject, + WORKSPACE_TYPE, +} from '../../../../core/server'; +import { workspaceIdInUrlSymbol } from '../constant'; + +type WorkspaceOptions = + | { + workspaces?: string[]; + } + | undefined; + +export class WorkspaceIdConsumerWrapper { + private typeRelatedToWorkspace(type: string | string[]): boolean { + if (Array.isArray(type)) { + return type.some((item) => item === WORKSPACE_TYPE); + } + + return type === WORKSPACE_TYPE; + } + private formatWorkspaceIdParams( + request: OpenSearchDashboardsRequest, + options: T + ): T { + if (!options) { + return options; + } + const { workspaces, ...others } = options; + const workspaceIdParsedFromUrl = request.headers[workspaceIdInUrlSymbol.toString()] as string; + const workspaceIdInUserOptions = options.workspaces; + let finalWorkspaces: string[] = []; + if (workspaceIdInUserOptions?.length) { + finalWorkspaces = workspaceIdInUserOptions; + } else if (workspaceIdParsedFromUrl) { + finalWorkspaces = [workspaceIdParsedFromUrl]; + } + + return { + ...(others as T), + ...(finalWorkspaces.length ? { workspaces: finalWorkspaces } : {}), + }; + } + public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => { + return { + ...wrapperOptions.client, + create: (type: string, attributes: T, options: SavedObjectsCreateOptions = {}) => + wrapperOptions.client.create( + type, + attributes, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + bulkCreate: ( + objects: Array>, + options: SavedObjectsCreateOptions = {} + ) => + wrapperOptions.client.bulkCreate( + objects, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + checkConflicts: ( + objects: SavedObjectsCheckConflictsObject[] = [], + options: SavedObjectsBaseOptions = {} + ) => + wrapperOptions.client.checkConflicts( + objects, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + delete: wrapperOptions.client.delete, + find: (options: SavedObjectsFindOptions) => + wrapperOptions.client.find( + this.typeRelatedToWorkspace(options.type) + ? options + : this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + bulkGet: wrapperOptions.client.bulkGet, + get: wrapperOptions.client.get, + update: ( + type: string, + id: string, + attributes: Partial, + options: SavedObjectsUpdateOptions = {} + ) => + wrapperOptions.client.update( + type, + id, + attributes, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + bulkUpdate: ( + objects: Array>, + options?: SavedObjectsBulkUpdateOptions + ) => + wrapperOptions.client.bulkUpdate( + objects, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + addToNamespaces: wrapperOptions.client.addToNamespaces, + deleteFromNamespaces: wrapperOptions.client.deleteFromNamespaces, + }; + }; + + constructor() {} +} diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index e7bdf97b54ec..0ffb72f016f2 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -21,6 +21,7 @@ import { IWorkspaceClientImpl, WorkspaceFindOptions, IResponse, IRequestDetail } import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; import { + WORKSPACE_ID_CONSUMER_WRAPPER_ID, WORKSPACE_OVERVIEW_APP_ID, WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, WORKSPACE_UPDATE_APP_ID, From d275f6173e5e68986505e2a8c050b29682102ae1 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 13 Mar 2024 17:37:46 +0800 Subject: [PATCH 02/21] feat: add some comment Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 5c42ae476033..ecea0bb84e71 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -101,6 +101,8 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); } + // TOO many client wrapper inside a single workspace plugin + // Try to combine conflict wrapper and this consumer wrapper. core.savedObjects.addClientWrapper( -2, WORKSPACE_ID_CONSUMER_WRAPPER_ID, From 65f6be3b585ea0c7e77445e2621525286567b406 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 13 Mar 2024 17:39:15 +0800 Subject: [PATCH 03/21] feat: revert dependency Signed-off-by: SuZhou-Joe --- src/plugins/workspace/opensearch_dashboards.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/plugins/workspace/opensearch_dashboards.json b/src/plugins/workspace/opensearch_dashboards.json index 0f1d851d5d76..7d94a7491a00 100644 --- a/src/plugins/workspace/opensearch_dashboards.json +++ b/src/plugins/workspace/opensearch_dashboards.json @@ -3,7 +3,9 @@ "version": "opensearchDashboards", "server": true, "ui": true, - "requiredPlugins": [], + "requiredPlugins": [ + "savedObjects" + ], "optionalPlugins": ["savedObjectsManagement"], "requiredBundles": ["opensearchDashboardsReact"] } From 9c5d2d9851474760abe4fde95cae9c5a321c5f59 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 13 Mar 2024 17:41:40 +0800 Subject: [PATCH 04/21] feat: update comment Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index ecea0bb84e71..3cedf77a4637 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -31,7 +31,7 @@ import { WorkspacePluginConfigType } from '../config'; import { workspaceIdInUrlSymbol } from './constant'; import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; // eslint-disable-next-line @osd/eslint/no-restricted-paths -import { ensureRawRequest } from '../../../core/server/http/router'; // will be an issue as the ensureRawRequest is an internal implementation +import { ensureRawRequest } from '../../../core/server/http/router'; // TODO: move the ensureRawRequest to a core module or the import will be an issue as the ensureRawRequest can only be import from core module export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; @@ -101,7 +101,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); } - // TOO many client wrapper inside a single workspace plugin + // TODO: TOO many client wrapper inside a single workspace plugin // Try to combine conflict wrapper and this consumer wrapper. core.savedObjects.addClientWrapper( -2, From d5638ff248adbb6fc4d777d5fe7645c1792a5233 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 14:53:01 +0800 Subject: [PATCH 05/21] feat: address one TODO Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/constant.ts | 11 ----------- src/plugins/workspace/server/plugin.ts | 14 ++++++++------ .../workspace_id_consumer_wrapper.ts | 15 ++++++++------- 3 files changed, 16 insertions(+), 24 deletions(-) delete mode 100644 src/plugins/workspace/server/constant.ts diff --git a/src/plugins/workspace/server/constant.ts b/src/plugins/workspace/server/constant.ts deleted file mode 100644 index d3cd904b1cf5..000000000000 --- a/src/plugins/workspace/server/constant.ts +++ /dev/null @@ -1,11 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -/** - * A symbol used for imply the workspace id in original url - * - * @Internal - */ -export const workspaceIdInUrlSymbol = Symbol('workspace_id_in_url'); diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 3cedf77a4637..21de1deeca10 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -21,17 +21,18 @@ import { IWorkspaceClientImpl } from './types'; import { WorkspaceClient } from './workspace_client'; import { registerRoutes } from './routes'; import { WorkspaceSavedObjectsClientWrapper } from './saved_objects'; -import { cleanWorkspaceId, getWorkspaceIdFromUrl } from '../../../core/server/utils'; +import { + cleanWorkspaceId, + getWorkspaceIdFromUrl, + updateWorkspaceState, +} from '../../../core/server/utils'; import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper_for_check_workspace_conflict'; import { SavedObjectsPermissionControl, SavedObjectsPermissionControlContract, } from './permission_control/client'; import { WorkspacePluginConfigType } from '../config'; -import { workspaceIdInUrlSymbol } from './constant'; import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; -// eslint-disable-next-line @osd/eslint/no-restricted-paths -import { ensureRawRequest } from '../../../core/server/http/router'; // TODO: move the ensureRawRequest to a core module or the import will be an issue as the ensureRawRequest can only be import from core module export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; @@ -52,8 +53,9 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); if (workspaceId) { - const rawRequest = ensureRawRequest(request); - rawRequest.headers[workspaceIdInUrlSymbol.toString()] = workspaceId; + updateWorkspaceState(request, { + id: workspaceId, + }); const requestUrl = new URL(request.url.toString()); requestUrl.pathname = cleanWorkspaceId(requestUrl.pathname); return toolkit.rewriteUrl(requestUrl.toString()); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 4c302fc93518..1135b8ca2eed 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { getWorkspaceState } from '../../../../core/server/utils'; import { SavedObjectsBaseOptions, SavedObjectsBulkCreateObject, @@ -16,7 +17,6 @@ import { SavedObjectsBulkUpdateObject, WORKSPACE_TYPE, } from '../../../../core/server'; -import { workspaceIdInUrlSymbol } from '../constant'; type WorkspaceOptions = | { @@ -40,13 +40,14 @@ export class WorkspaceIdConsumerWrapper { return options; } const { workspaces, ...others } = options; - const workspaceIdParsedFromUrl = request.headers[workspaceIdInUrlSymbol.toString()] as string; - const workspaceIdInUserOptions = options.workspaces; + const workspaceState = getWorkspaceState(request); + const workspaceIdParsedFromRequest = workspaceState?.id; + const workspaceIdsInUserOptions = options.workspaces; let finalWorkspaces: string[] = []; - if (workspaceIdInUserOptions?.length) { - finalWorkspaces = workspaceIdInUserOptions; - } else if (workspaceIdParsedFromUrl) { - finalWorkspaces = [workspaceIdParsedFromUrl]; + if (options.hasOwnProperty('workspaces')) { + finalWorkspaces = workspaceIdsInUserOptions || []; + } else if (workspaceIdParsedFromRequest) { + finalWorkspaces = [workspaceIdParsedFromRequest]; } return { From 45589cff80409190f98a403da659fa6e34d0ec40 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 15:24:25 +0800 Subject: [PATCH 06/21] feat: address TODO Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.ts | 14 +++++------ .../workspace_id_consumer_wrapper.ts | 25 ++----------------- .../workspace/server/workspace_client.ts | 7 +----- 3 files changed, 9 insertions(+), 37 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 21de1deeca10..088096df4d35 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -88,6 +88,12 @@ export class WorkspacePlugin implements Plugin<{}, {}> { this.workspaceConflictControl.wrapperFactory ); + core.savedObjects.addClientWrapper( + -2, + WORKSPACE_ID_CONSUMER_WRAPPER_ID, + new WorkspaceIdConsumerWrapper().wrapperFactory + ); + this.logger.info('Workspace permission control enabled:' + isPermissionControlEnabled); if (isPermissionControlEnabled) { this.permissionControl = new SavedObjectsPermissionControl(this.logger); @@ -103,14 +109,6 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); } - // TODO: TOO many client wrapper inside a single workspace plugin - // Try to combine conflict wrapper and this consumer wrapper. - core.savedObjects.addClientWrapper( - -2, - WORKSPACE_ID_CONSUMER_WRAPPER_ID, - new WorkspaceIdConsumerWrapper().wrapperFactory - ); - registerRoutes({ http: core.http, logger: this.logger, diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 1135b8ca2eed..321a026d943e 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -12,9 +12,6 @@ import { SavedObjectsCheckConflictsObject, OpenSearchDashboardsRequest, SavedObjectsFindOptions, - SavedObjectsUpdateOptions, - SavedObjectsBulkUpdateOptions, - SavedObjectsBulkUpdateObject, WORKSPACE_TYPE, } from '../../../../core/server'; @@ -89,26 +86,8 @@ export class WorkspaceIdConsumerWrapper { ), bulkGet: wrapperOptions.client.bulkGet, get: wrapperOptions.client.get, - update: ( - type: string, - id: string, - attributes: Partial, - options: SavedObjectsUpdateOptions = {} - ) => - wrapperOptions.client.update( - type, - id, - attributes, - this.formatWorkspaceIdParams(wrapperOptions.request, options) - ), - bulkUpdate: ( - objects: Array>, - options?: SavedObjectsBulkUpdateOptions - ) => - wrapperOptions.client.bulkUpdate( - objects, - this.formatWorkspaceIdParams(wrapperOptions.request, options) - ), + update: wrapperOptions.client.update, + bulkUpdate: wrapperOptions.client.bulkUpdate, addToNamespaces: wrapperOptions.client.addToNamespaces, deleteFromNamespaces: wrapperOptions.client.deleteFromNamespaces, }; diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index 0ffb72f016f2..05e323f63f02 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -20,12 +20,7 @@ import { import { IWorkspaceClientImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types'; import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; -import { - WORKSPACE_ID_CONSUMER_WRAPPER_ID, - WORKSPACE_OVERVIEW_APP_ID, - WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, - WORKSPACE_UPDATE_APP_ID, -} from '../common/constants'; +import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; const WORKSPACE_ID_SIZE = 6; From 5d990c8211931563612b03f1ce487fa177451e6b Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 16:46:15 +0800 Subject: [PATCH 07/21] feat: add unit test Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.test.ts | 65 +++++++++- .../workspace_id_consumer_wrapper.test.ts | 118 ++++++++++++++++++ .../workspace_id_consumer_wrapper.ts | 25 +--- .../workspace/server/workspace_client.ts | 12 +- 4 files changed, 198 insertions(+), 22 deletions(-) create mode 100644 src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts index c448fcf209f9..300b5e982a01 100644 --- a/src/plugins/workspace/server/plugin.test.ts +++ b/src/plugins/workspace/server/plugin.test.ts @@ -3,8 +3,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { coreMock } from '../../../core/server/mocks'; +import { OnPreRoutingHandler } from 'src/core/server'; +import { coreMock, httpServerMock } from '../../../core/server/mocks'; import { WorkspacePlugin } from './plugin'; +import { getWorkspaceState } from '../../../core/server/utils'; describe('Workspace server plugin', () => { it('#setup', async () => { @@ -27,5 +29,66 @@ describe('Workspace server plugin', () => { }, } `); + expect(setupMock.savedObjects.addClientWrapper).toBeCalledTimes(3); + }); + + it('#proxyWorkspaceTrafficToRealHandler', async () => { + const setupMock = coreMock.createSetup(); + const initializerContextConfigMock = coreMock.createPluginInitializerContext({ + enabled: true, + permission: { + enabled: true, + }, + }); + let onPreRoutingFn: OnPreRoutingHandler = () => httpServerMock.createResponseFactory().ok(); + setupMock.http.registerOnPreRouting.mockImplementation((fn) => { + onPreRoutingFn = fn; + return fn; + }); + const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); + await workspacePlugin.setup(setupMock); + const toolKitMock = httpServerMock.createToolkit(); + + const requestWithWorkspaceInUrl = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/w/foo/app', + }); + onPreRoutingFn(requestWithWorkspaceInUrl, httpServerMock.createResponseFactory(), toolKitMock); + expect(toolKitMock.rewriteUrl).toBeCalledWith('http://localhost/app'); + expect(toolKitMock.next).toBeCalledTimes(0); + expect(getWorkspaceState(requestWithWorkspaceInUrl)).toEqual({ + id: 'foo', + }); + + const requestWithoutWorkspaceInUrl = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/app', + }); + onPreRoutingFn( + requestWithoutWorkspaceInUrl, + httpServerMock.createResponseFactory(), + toolKitMock + ); + expect(toolKitMock.next).toBeCalledTimes(1); + }); + + it('#start', async () => { + const setupMock = coreMock.createSetup(); + const startMock = coreMock.createStart(); + const initializerContextConfigMock = coreMock.createPluginInitializerContext({ + enabled: true, + permission: { + enabled: true, + }, + }); + + const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); + await workspacePlugin.setup(setupMock); + await workspacePlugin.start(startMock); + expect(startMock.savedObjects.createSerializer).toBeCalledTimes(1); + }); + + it('#stop', () => { + const initializerContextConfigMock = coreMock.createPluginInitializerContext(); + const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); + workspacePlugin.stop(); }); }); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts new file mode 100644 index 000000000000..8715606da681 --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -0,0 +1,118 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { updateWorkspaceState } from '../../../../core/server/utils'; +import { SavedObject } from '../../../../core/public'; +import { httpServerMock, savedObjectsClientMock, coreMock } from '../../../../core/server/mocks'; +import { WorkspaceIdConsumerWrapper } from './workspace_id_consumer_wrapper'; + +describe('WorkspaceIdConsumerWrapper', () => { + const requestHandlerContext = coreMock.createRequestHandlerContext(); + const wrapperInstance = new WorkspaceIdConsumerWrapper(); + const mockedClient = savedObjectsClientMock.create(); + const workspaceEnabledMockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + updateWorkspaceState(workspaceEnabledMockRequest, { + id: 'foo', + }); + const wrapperClient = wrapperInstance.wrapperFactory({ + client: mockedClient, + typeRegistry: requestHandlerContext.savedObjects.typeRegistry, + request: workspaceEnabledMockRequest, + }); + const getSavedObject = (savedObject: Partial) => { + const payload: SavedObject = { + references: [], + id: '', + type: 'dashboard', + attributes: {}, + ...savedObject, + }; + + return payload; + }; + describe('create', () => { + beforeEach(() => { + mockedClient.create.mockClear(); + }); + it(`Should add workspaces parameters when create`, async () => { + await wrapperClient.create('dashboard', { + name: 'foo', + }); + + expect(mockedClient.create).toBeCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ + workspaces: ['foo'], + }) + ); + }); + + it(`Should use options.workspaces there is workspaces inside options`, async () => { + await wrapperClient.create( + 'dashboard', + { + name: 'foo', + }, + { + id: 'dashboard:foo', + overwrite: true, + workspaces: undefined, + } + ); + + expect(mockedClient.create.mock.calls[0][2]?.hasOwnProperty('workspaces')).toEqual(false); + }); + }); + + describe('bulkCreate', () => { + beforeEach(() => { + mockedClient.bulkCreate.mockClear(); + }); + it(`Should add workspaces parameters when bulk create`, async () => { + await wrapperClient.bulkCreate([ + getSavedObject({ + id: 'foo', + }), + ]); + + expect(mockedClient.bulkCreate).toBeCalledWith( + [{ attributes: {}, id: 'foo', references: [], type: 'dashboard' }], + { + workspaces: ['foo'], + } + ); + }); + }); + + describe('checkConflict', () => { + beforeEach(() => { + mockedClient.checkConflicts.mockClear(); + }); + + it(`Should add workspaces parameters when checkConflict`, async () => { + await wrapperClient.checkConflicts([]); + expect(mockedClient.checkConflicts).toBeCalledWith([], { + workspaces: ['foo'], + }); + }); + }); + + describe('find', () => { + beforeEach(() => { + mockedClient.find.mockClear(); + }); + + it(`Should add workspaces parameters when find`, async () => { + await wrapperClient.find({ + type: 'dashboard', + }); + expect(mockedClient.find).toBeCalledWith({ + type: 'dashboard', + workspaces: ['foo'], + }); + }); + }); +}); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 321a026d943e..bf450841f001 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -12,7 +12,6 @@ import { SavedObjectsCheckConflictsObject, OpenSearchDashboardsRequest, SavedObjectsFindOptions, - WORKSPACE_TYPE, } from '../../../../core/server'; type WorkspaceOptions = @@ -22,26 +21,16 @@ type WorkspaceOptions = | undefined; export class WorkspaceIdConsumerWrapper { - private typeRelatedToWorkspace(type: string | string[]): boolean { - if (Array.isArray(type)) { - return type.some((item) => item === WORKSPACE_TYPE); - } - - return type === WORKSPACE_TYPE; - } private formatWorkspaceIdParams( request: OpenSearchDashboardsRequest, - options: T + options?: T ): T { - if (!options) { - return options; - } - const { workspaces, ...others } = options; + const { workspaces, ...others } = options || {}; const workspaceState = getWorkspaceState(request); const workspaceIdParsedFromRequest = workspaceState?.id; - const workspaceIdsInUserOptions = options.workspaces; + const workspaceIdsInUserOptions = options?.workspaces; let finalWorkspaces: string[] = []; - if (options.hasOwnProperty('workspaces')) { + if (options?.hasOwnProperty('workspaces')) { finalWorkspaces = workspaceIdsInUserOptions || []; } else if (workspaceIdParsedFromRequest) { finalWorkspaces = [workspaceIdParsedFromRequest]; @@ -79,11 +68,7 @@ export class WorkspaceIdConsumerWrapper { ), delete: wrapperOptions.client.delete, find: (options: SavedObjectsFindOptions) => - wrapperOptions.client.find( - this.typeRelatedToWorkspace(options.type) - ? options - : this.formatWorkspaceIdParams(wrapperOptions.request, options) - ), + wrapperOptions.client.find(this.formatWorkspaceIdParams(wrapperOptions.request, options)), bulkGet: wrapperOptions.client.bulkGet, get: wrapperOptions.client.get, update: wrapperOptions.client.update, diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index 05e323f63f02..3378ab650cd1 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -20,7 +20,10 @@ import { import { IWorkspaceClientImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types'; import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; -import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; +import { + WORKSPACE_ID_CONSUMER_WRAPPER_ID, + WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, +} from '../common/constants'; const WORKSPACE_ID_SIZE = 6; @@ -56,6 +59,13 @@ export class WorkspaceClient implements IWorkspaceClientImpl { requestDetail: IRequestDetail ): SavedObjectsClientContract { return this.savedObjects?.getScopedClient(requestDetail.request, { + /** + * workspace object does not have workspaces field + * so need to bypass the consumer wrapper + * or it will append workspaces into the options.workspaces + * when list all the workspaces inside a workspace + */ + excludedWrappers: [WORKSPACE_ID_CONSUMER_WRAPPER_ID], includedHiddenTypes: [WORKSPACE_TYPE], }) as SavedObjectsClientContract; } From 08dc9e2be28177a037c14d4668cca0bfac042ffc Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 18:27:09 +0800 Subject: [PATCH 08/21] feat: some special logic on specific operation Signed-off-by: SuZhou-Joe --- .../workspace_id_consumer_wrapper.ts | 4 ++-- .../workspace_saved_objects_client_wrapper.ts | 3 +++ src/plugins/workspace/server/workspace_client.ts | 16 +++++++++------- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index bf450841f001..d24496ed31ef 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -30,8 +30,8 @@ export class WorkspaceIdConsumerWrapper { const workspaceIdParsedFromRequest = workspaceState?.id; const workspaceIdsInUserOptions = options?.workspaces; let finalWorkspaces: string[] = []; - if (options?.hasOwnProperty('workspaces')) { - finalWorkspaces = workspaceIdsInUserOptions || []; + if (workspaceIdsInUserOptions) { + finalWorkspaces = workspaceIdsInUserOptions; } else if (workspaceIdParsedFromRequest) { finalWorkspaces = [workspaceIdParsedFromRequest]; } diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index c515f555fa4b..1a97b2dac69a 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -468,6 +468,9 @@ export class WorkspaceSavedObjectsClientWrapper { [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.LibraryWrite] ), }, + // By declaring workspaces as empty array, + // workspaces won't be appended automatically into the options. + workspaces: [], }) ).saved_objects.map((item) => item.id); diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index 3378ab650cd1..a8b006c99a87 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -50,7 +50,15 @@ export class WorkspaceClient implements IWorkspaceClientImpl { requestDetail: IRequestDetail ): SavedObjectsClientContract | undefined { return this.savedObjects?.getScopedClient(requestDetail.request, { - excludedWrappers: [WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID], + excludedWrappers: [ + WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, + /** + * workspace object does not have workspaces field + * so need to bypass workspace id consumer wrapper + * for any kind of operation to saved objects client. + */ + WORKSPACE_ID_CONSUMER_WRAPPER_ID, + ], includedHiddenTypes: [WORKSPACE_TYPE], }); } @@ -59,12 +67,6 @@ export class WorkspaceClient implements IWorkspaceClientImpl { requestDetail: IRequestDetail ): SavedObjectsClientContract { return this.savedObjects?.getScopedClient(requestDetail.request, { - /** - * workspace object does not have workspaces field - * so need to bypass the consumer wrapper - * or it will append workspaces into the options.workspaces - * when list all the workspaces inside a workspace - */ excludedWrappers: [WORKSPACE_ID_CONSUMER_WRAPPER_ID], includedHiddenTypes: [WORKSPACE_TYPE], }) as SavedObjectsClientContract; From d3cccb8585197ab93e92a2e66f8acf354b9732b8 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 18:27:47 +0800 Subject: [PATCH 09/21] feat: add integration test Signed-off-by: SuZhou-Joe --- .../workspace_id_consumer_wrapper.test.ts | 242 ++++++++++++++++++ 1 file changed, 242 insertions(+) create mode 100644 src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts new file mode 100644 index 000000000000..381da5227d8d --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts @@ -0,0 +1,242 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { SavedObject } from 'src/core/types'; +import { isEqual } from 'lodash'; +import * as osdTestServer from '../../../../../core/test_helpers/osd_server'; + +const dashboard: Omit = { + type: 'dashboard', + attributes: {}, + references: [], +}; + +interface WorkspaceAttributes { + id: string; + name?: string; +} + +describe('workspace_id_consumer 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), + settings: { + osd: { + workspace: { + enabled: true, + }, + savedObjects: { + permission: { + enabled: true, + }, + }, + migrations: { + skip: false, + }, + }, + }, + }); + 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) => { + return resp.body.result; + }); + createdBarWorkspace = await createWorkspace({ + name: 'bar', + }).then((resp) => resp.body.result); + }, 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('saved objects client related CRUD', () => { + it('create', async () => { + const createResult = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/${dashboard.type}`) + .send({ + attributes: dashboard.attributes, + }) + .expect(200); + + expect(createResult.body.workspaces).toEqual([createdFooWorkspace.id]); + await deleteItem({ + type: dashboard.type, + id: createResult.body.id, + }); + }); + + it('bulk create', async () => { + await clearFooAndBar(); + const createResultFoo = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .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, [createdFooWorkspace.id]) + ) + ).toEqual(true); + await Promise.all( + [...createResultFoo.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, `/w/${createdFooWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/w/${createdBarWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const getResultFoo = await getItem({ + type: dashboard.type, + id: 'foo', + }); + const getResultBar = await getItem({ + type: dashboard.type, + id: 'bar', + }); + + /** + * import with workspaces when conflicts + */ + const importWithWorkspacesResult = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_import?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, `/w/${createdFooWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/w/${createdBarWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const findResult = await osdTestServer.request + .get(root, `/w/${createdBarWorkspace.id}/api/saved_objects/_find?type=${dashboard.type}`) + .expect(200); + + expect(findResult.body.total).toEqual(1); + expect(findResult.body.saved_objects[0].workspaces).toEqual([createdBarWorkspace.id]); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + }); +}); From 298c2cbd100381d681cbd1b9632019233ed58074 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sun, 24 Mar 2024 22:58:05 +0800 Subject: [PATCH 10/21] feat: declare workspaces as empty array for advanced settings Signed-off-by: SuZhou-Joe --- .../create_or_upgrade_saved_config.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts index 639dd09249ff..10d33713c878 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts @@ -63,7 +63,12 @@ export async function createOrUpgradeSavedConfig( try { // create the new SavedConfig - await savedObjectsClient.create('config', attributes, { id: version }); + await savedObjectsClient.create('config', attributes, { + id: version, + // config is a global object that should not belong to any of the workspaces + // declare it as empty array to prevent it being created as a workspace object + workspaces: [], + }); } catch (error) { if (handleWriteErrors) { if (SavedObjectsErrorHelpers.isConflictError(error)) { From 85c522dfd3f8887e4771fa964ab39f378f114ec9 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Mar 2024 10:41:25 +0800 Subject: [PATCH 11/21] feat: unified workspaces parameters when parsing from router Signed-off-by: SuZhou-Joe --- .../server/saved_objects/import/check_conflicts.ts | 2 +- src/core/server/saved_objects/routes/bulk_create.ts | 2 +- src/core/server/saved_objects/routes/create.ts | 2 +- src/core/server/saved_objects/routes/export.ts | 2 +- src/core/server/saved_objects/routes/find.ts | 2 +- .../saved_objects/service/saved_objects_client.ts | 4 ---- src/core/server/saved_objects/types.ts | 2 +- .../create_or_upgrade_saved_config.test.ts | 2 ++ .../create_or_upgrade_saved_config.ts | 4 ++-- .../workspace_id_consumer_wrapper.test.ts | 4 ++-- .../saved_objects/workspace_id_consumer_wrapper.ts | 10 +++------- 11 files changed, 15 insertions(+), 21 deletions(-) diff --git a/src/core/server/saved_objects/import/check_conflicts.ts b/src/core/server/saved_objects/import/check_conflicts.ts index f36bcf3a8a92..2c35ba147c5d 100644 --- a/src/core/server/saved_objects/import/check_conflicts.ts +++ b/src/core/server/saved_objects/import/check_conflicts.ts @@ -79,7 +79,7 @@ export async function checkConflicts({ }); const checkConflictsResult = await savedObjectsClient.checkConflicts(objectsToCheck, { namespace, - workspaces, + ...(workspaces ? { workspaces } : {}), }); const errorMap = checkConflictsResult.errors.reduce( (acc, { type, id, error }) => acc.set(`${type}:${id}`, error), diff --git a/src/core/server/saved_objects/routes/bulk_create.ts b/src/core/server/saved_objects/routes/bulk_create.ts index 056b1b795550..b8a020b4ea24 100644 --- a/src/core/server/saved_objects/routes/bulk_create.ts +++ b/src/core/server/saved_objects/routes/bulk_create.ts @@ -70,7 +70,7 @@ export const registerBulkCreateRoute = (router: IRouter) => { : undefined; const result = await context.core.savedObjects.client.bulkCreate(req.body, { overwrite, - workspaces, + ...(workspaces ? { workspaces } : {}), }); return res.ok({ body: result }); }) diff --git a/src/core/server/saved_objects/routes/create.ts b/src/core/server/saved_objects/routes/create.ts index 4d22bd244a03..2a7958aa9b78 100644 --- a/src/core/server/saved_objects/routes/create.ts +++ b/src/core/server/saved_objects/routes/create.ts @@ -71,7 +71,7 @@ export const registerCreateRoute = (router: IRouter) => { migrationVersion, references, initialNamespaces, - workspaces, + ...(workspaces ? { workspaces } : {}), }; const result = await context.core.savedObjects.client.create(type, attributes, options); return res.ok({ body: result }); diff --git a/src/core/server/saved_objects/routes/export.ts b/src/core/server/saved_objects/routes/export.ts index 9325b632e40f..4d9d5b7e8ec3 100644 --- a/src/core/server/saved_objects/routes/export.ts +++ b/src/core/server/saved_objects/routes/export.ts @@ -106,7 +106,7 @@ export const registerExportRoute = (router: IRouter, config: SavedObjectConfig) exportSizeLimit: maxImportExportSize, includeReferencesDeep, excludeExportDetails, - workspaces, + ...(workspaces ? { workspaces } : {}), }); const docsToExport: string[] = await createPromiseFromStreams([ diff --git a/src/core/server/saved_objects/routes/find.ts b/src/core/server/saved_objects/routes/find.ts index 36fa7c2cd9f5..42b47cf950fc 100644 --- a/src/core/server/saved_objects/routes/find.ts +++ b/src/core/server/saved_objects/routes/find.ts @@ -85,7 +85,7 @@ export const registerFindRoute = (router: IRouter) => { fields: typeof query.fields === 'string' ? [query.fields] : query.fields, filter: query.filter, namespaces, - workspaces, + ...(workspaces ? { workspaces } : {}), }); return res.ok({ body: result }); 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 e1c3d16a9258..1b268e033d97 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -69,10 +69,6 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions { * Note: this can only be used for multi-namespace object types. */ initialNamespaces?: string[]; - /** - * workspaces the new created objects belong to - */ - workspaces?: string[]; /** permission control describe by ACL object */ permissions?: Permissions; } diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index d21421dbe253..23170e0a1c47 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -132,7 +132,7 @@ export interface SavedObjectsBaseOptions { /** Specify the namespace for this operation */ namespace?: string; /** Specify the workspaces for this operation */ - workspaces?: string[]; + workspaces?: string[] | null; } /** diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts index eb23e78b1756..fc5a55da0091 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts @@ -98,6 +98,7 @@ describe('uiSettings/createOrUpgradeSavedConfig', function () { }, { id: version, + workspaces: null, } ); }); @@ -133,6 +134,7 @@ describe('uiSettings/createOrUpgradeSavedConfig', function () { }, { id: version, + workspaces: null, } ); }); diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts index 10d33713c878..6ca3baeb7a9f 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts @@ -66,8 +66,8 @@ export async function createOrUpgradeSavedConfig( await savedObjectsClient.create('config', attributes, { id: version, // config is a global object that should not belong to any of the workspaces - // declare it as empty array to prevent it being created as a workspace object - workspaces: [], + // declare it as null to prevent it being created as a workspace object + workspaces: null, }); } catch (error) { if (handleWriteErrors) { diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index 8715606da681..ea8445a4ccaf 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -50,7 +50,7 @@ describe('WorkspaceIdConsumerWrapper', () => { ); }); - it(`Should use options.workspaces there is workspaces inside options`, async () => { + it(`Should use options.workspaces when there is no workspaces inside options`, async () => { await wrapperClient.create( 'dashboard', { @@ -59,7 +59,7 @@ describe('WorkspaceIdConsumerWrapper', () => { { id: 'dashboard:foo', overwrite: true, - workspaces: undefined, + workspaces: null, } ); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index d24496ed31ef..bcf6fe4d5e8c 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -14,11 +14,7 @@ import { SavedObjectsFindOptions, } from '../../../../core/server'; -type WorkspaceOptions = - | { - workspaces?: string[]; - } - | undefined; +type WorkspaceOptions = Pick | undefined; export class WorkspaceIdConsumerWrapper { private formatWorkspaceIdParams( @@ -30,8 +26,8 @@ export class WorkspaceIdConsumerWrapper { const workspaceIdParsedFromRequest = workspaceState?.id; const workspaceIdsInUserOptions = options?.workspaces; let finalWorkspaces: string[] = []; - if (workspaceIdsInUserOptions) { - finalWorkspaces = workspaceIdsInUserOptions; + if (options?.hasOwnProperty('workspaces')) { + finalWorkspaces = workspaceIdsInUserOptions || []; } else if (workspaceIdParsedFromRequest) { finalWorkspaces = [workspaceIdParsedFromRequest]; } From cdbe9a83a5c87f4dce96e1550da94738dd86e3ac Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Mar 2024 11:30:12 +0800 Subject: [PATCH 12/21] feat: improve code coverage Signed-off-by: SuZhou-Joe --- .../server/saved_objects/workspace_id_consumer_wrapper.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index ea8445a4ccaf..d4b79868d0e5 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -93,7 +93,7 @@ describe('WorkspaceIdConsumerWrapper', () => { }); it(`Should add workspaces parameters when checkConflict`, async () => { - await wrapperClient.checkConflicts([]); + await wrapperClient.checkConflicts(); expect(mockedClient.checkConflicts).toBeCalledWith([], { workspaces: ['foo'], }); From cb7ebd96d134b6b505f544757d6535dbb2776326 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 27 Mar 2024 11:04:07 +0800 Subject: [PATCH 13/21] feat: declare workspaces as null Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/types.ts | 2 +- .../saved_objects/workspace_saved_objects_client_wrapper.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 23170e0a1c47..0b31baf5c111 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -112,7 +112,7 @@ export interface SavedObjectsFindOptions { /** 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[]; + workspaces?: string[] | null; /** By default the operator will be 'AND' */ workspacesSearchOperator?: 'AND' | 'OR'; /** diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index 1a97b2dac69a..48160c31af63 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -468,9 +468,9 @@ export class WorkspaceSavedObjectsClientWrapper { [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.LibraryWrite] ), }, - // By declaring workspaces as empty array, + // By declaring workspaces as null, // workspaces won't be appended automatically into the options. - workspaces: [], + workspaces: null, }) ).saved_objects.map((item) => item.id); From 294d2c557dd5d36e78455af3f3a7d2a426299584 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 27 Mar 2024 11:31:05 +0800 Subject: [PATCH 14/21] feat: use unified types Signed-off-by: SuZhou-Joe --- src/core/public/saved_objects/saved_objects_client.ts | 4 ++-- .../saved_objects/export/get_sorted_objects_for_export.ts | 4 ++-- src/core/server/saved_objects/import/check_conflicts.ts | 2 +- .../server/saved_objects/import/create_saved_objects.ts | 2 +- src/core/server/saved_objects/import/types.ts | 4 ++-- src/core/server/saved_objects/serialization/types.ts | 6 +++--- .../saved_objects/service/lib/search_dsl/query_params.ts | 2 +- .../saved_objects/service/lib/search_dsl/search_dsl.ts | 2 +- .../server/saved_objects/service/saved_objects_client.ts | 2 +- src/core/server/saved_objects/types.ts | 4 ++-- src/core/types/saved_objects.ts | 2 +- src/plugins/workspace/server/permission_control/client.ts | 2 +- 12 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/core/public/saved_objects/saved_objects_client.ts b/src/core/public/saved_objects/saved_objects_client.ts index f415e9c58596..ce3ebc710ed1 100644 --- a/src/core/public/saved_objects/saved_objects_client.ts +++ b/src/core/public/saved_objects/saved_objects_client.ts @@ -65,7 +65,7 @@ export interface SavedObjectsCreateOptions { /** {@inheritDoc SavedObjectsMigrationVersion} */ migrationVersion?: SavedObjectsMigrationVersion; references?: SavedObjectReference[]; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } /** @@ -83,7 +83,7 @@ export interface SavedObjectsBulkCreateObject extends SavedObjectsC export interface SavedObjectsBulkCreateOptions { /** If a document with the given `id` already exists, overwrite it's contents (default=false). */ overwrite?: boolean; - workspaces?: string[]; + workspaces?: SavedObjectsCreateOptions['workspaces']; } /** @public */ 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 0336fc702973..6c99db6c24af 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 @@ -61,7 +61,7 @@ export interface SavedObjectsExportOptions { /** optional namespace to override the namespace used by the savedObjectsClient. */ namespace?: string; /** optional workspaces to override the workspaces used by the savedObjectsClient. */ - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } /** @@ -97,7 +97,7 @@ async function fetchObjectsToExport({ exportSizeLimit: number; savedObjectsClient: SavedObjectsClientContract; namespace?: string; - workspaces?: string[]; + workspaces?: SavedObjectsExportOptions['workspaces']; }) { if ((types?.length ?? 0) > 0 && (objects?.length ?? 0) > 0) { throw Boom.badRequest(`Can't specify both "types" and "objects" properties when exporting`); diff --git a/src/core/server/saved_objects/import/check_conflicts.ts b/src/core/server/saved_objects/import/check_conflicts.ts index 2c35ba147c5d..d173af45f761 100644 --- a/src/core/server/saved_objects/import/check_conflicts.ts +++ b/src/core/server/saved_objects/import/check_conflicts.ts @@ -44,7 +44,7 @@ interface CheckConflictsParams { ignoreRegularConflicts?: boolean; retries?: SavedObjectsImportRetry[]; createNewCopies?: boolean; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } const isUnresolvableConflict = (error: SavedObjectError) => 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 6b0015851baf..f98e2e816b75 100644 --- a/src/core/server/saved_objects/import/create_saved_objects.ts +++ b/src/core/server/saved_objects/import/create_saved_objects.ts @@ -41,7 +41,7 @@ interface CreateSavedObjectsParams { overwrite?: boolean; dataSourceId?: string; dataSourceTitle?: string; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } interface CreateSavedObjectsResult { createdObjects: Array>; diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index 994b7e627189..689b8222b510 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -190,7 +190,7 @@ export interface SavedObjectsImportOptions { dataSourceId?: string; dataSourceTitle?: string; /** if specified, will import in given workspaces */ - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } /** @@ -215,7 +215,7 @@ export interface SavedObjectsResolveImportErrorsOptions { dataSourceId?: string; dataSourceTitle?: string; /** if specified, will import in given workspaces */ - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } export type CreatedObject = SavedObject & { destinationId?: string }; diff --git a/src/core/server/saved_objects/serialization/types.ts b/src/core/server/saved_objects/serialization/types.ts index f882596ce529..6b1f025e856d 100644 --- a/src/core/server/saved_objects/serialization/types.ts +++ b/src/core/server/saved_objects/serialization/types.ts @@ -29,7 +29,7 @@ */ import { Permissions } from '../permission_control'; -import { SavedObjectsMigrationVersion, SavedObjectReference } from '../types'; +import { SavedObjectsMigrationVersion, SavedObjectReference, SavedObject } from '../types'; /** * A raw document as represented directly in the saved object index. @@ -53,7 +53,7 @@ export interface SavedObjectsRawDocSource { updated_at?: string; references?: SavedObjectReference[]; originId?: string; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; [typeMapping: string]: any; } @@ -71,7 +71,7 @@ interface SavedObjectDoc { version?: string; updated_at?: string; originId?: string; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; permissions?: Permissions; } diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index abbef0850dba..318768fd83c2 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -167,7 +167,7 @@ interface QueryParams { defaultSearchOperator?: string; hasReference?: HasReferenceQueryParams; kueryNode?: KueryNode; - workspaces?: string[]; + workspaces?: SavedObjectsFindOptions['workspaces']; workspacesSearchOperator?: 'AND' | 'OR'; ACLSearchParams?: SavedObjectsFindOptions['ACLSearchParams']; } diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts index fa4311576638..626fc7efba3e 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts @@ -53,7 +53,7 @@ interface GetSearchDslOptions { id: string; }; kueryNode?: KueryNode; - workspaces?: string[]; + workspaces?: SavedObjectsFindOptions['workspaces']; workspacesSearchOperator?: 'AND' | 'OR'; ACLSearchParams?: SavedObjectsFindOptions['ACLSearchParams']; } 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 1b268e033d97..be02ef247d04 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -97,7 +97,7 @@ export interface SavedObjectsBulkCreateObject { /** * workspaces the objects belong to, will only be used when overwrite is enabled. */ - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } /** diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 0b31baf5c111..22093b90d6b3 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -112,7 +112,7 @@ export interface SavedObjectsFindOptions { /** 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[] | null; + workspaces?: SavedObjectsBaseOptions['workspaces']; /** By default the operator will be 'AND' */ workspacesSearchOperator?: 'AND' | 'OR'; /** @@ -132,7 +132,7 @@ export interface SavedObjectsBaseOptions { /** Specify the namespace for this operation */ namespace?: string; /** Specify the workspaces for this operation */ - workspaces?: string[] | null; + workspaces?: SavedObject['workspaces']; } /** diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index b37309338c9e..e91a5012098e 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -115,7 +115,7 @@ export interface SavedObject { */ originId?: string; /** Workspaces that this saved object exists in. */ - workspaces?: string[]; + workspaces?: string[] | null; /** Permissions that this saved objects exists in. */ permissions?: Permissions; } diff --git a/src/plugins/workspace/server/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts index bad46eb156a6..03a8fb0aaf3d 100644 --- a/src/plugins/workspace/server/permission_control/client.ts +++ b/src/plugins/workspace/server/permission_control/client.ts @@ -147,7 +147,7 @@ export class SavedObjectsPermissionControl { const principals = this.getPrincipalsFromRequest(request); const deniedObjects: Array< Pick & { - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; permissions?: Permissions; } > = []; From 42e0e051896c900ea9cc573c71b66f7a173c7e17 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 28 Mar 2024 10:46:18 +0800 Subject: [PATCH 15/21] feat: update comment Signed-off-by: SuZhou-Joe --- .../saved_objects/workspace_saved_objects_client_wrapper.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index 48160c31af63..d5497bc6475c 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -470,6 +470,7 @@ export class WorkspaceSavedObjectsClientWrapper { }, // By declaring workspaces as null, // workspaces won't be appended automatically into the options. + // or workspaces can not be found because workspace object do not have `workspaces` field. workspaces: null, }) ).saved_objects.map((item) => item.id); From 1b323162ecb8e844f2f6a345c5e507c0d71491d6 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 1 Apr 2024 17:13:00 +0800 Subject: [PATCH 16/21] feat: remove null Signed-off-by: SuZhou-Joe --- .../create_or_upgrade_saved_config.test.ts | 2 -- .../create_or_upgrade_saved_config.ts | 3 --- 2 files changed, 5 deletions(-) diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts index fc5a55da0091..eb23e78b1756 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts @@ -98,7 +98,6 @@ describe('uiSettings/createOrUpgradeSavedConfig', function () { }, { id: version, - workspaces: null, } ); }); @@ -134,7 +133,6 @@ describe('uiSettings/createOrUpgradeSavedConfig', function () { }, { id: version, - workspaces: null, } ); }); diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts index 6ca3baeb7a9f..6649966d8952 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts @@ -65,9 +65,6 @@ export async function createOrUpgradeSavedConfig( // create the new SavedConfig await savedObjectsClient.create('config', attributes, { id: version, - // config is a global object that should not belong to any of the workspaces - // declare it as null to prevent it being created as a workspace object - workspaces: null, }); } catch (error) { if (handleWriteErrors) { From b7a6cb7bec00944e89a9e51c25543f4be29f5ea6 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 2 Apr 2024 15:37:50 +0800 Subject: [PATCH 17/21] feat: address comments Signed-off-by: SuZhou-Joe --- .../create_or_upgrade_saved_config.ts | 4 +--- .../saved_objects/workspace_id_consumer_wrapper.test.ts | 2 +- .../server/saved_objects/workspace_id_consumer_wrapper.ts | 1 + 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts index 6649966d8952..639dd09249ff 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts @@ -63,9 +63,7 @@ export async function createOrUpgradeSavedConfig( try { // create the new SavedConfig - await savedObjectsClient.create('config', attributes, { - id: version, - }); + await savedObjectsClient.create('config', attributes, { id: version }); } catch (error) { if (handleWriteErrors) { if (SavedObjectsErrorHelpers.isConflictError(error)) { diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index d4b79868d0e5..21ef63c04867 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -50,7 +50,7 @@ describe('WorkspaceIdConsumerWrapper', () => { ); }); - it(`Should use options.workspaces when there is no workspaces inside options`, async () => { + it(`Should not use options.workspaces when there is no workspaces inside options`, async () => { await wrapperClient.create( 'dashboard', { diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index bcf6fe4d5e8c..8ebd6b301563 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -71,6 +71,7 @@ export class WorkspaceIdConsumerWrapper { bulkUpdate: wrapperOptions.client.bulkUpdate, addToNamespaces: wrapperOptions.client.addToNamespaces, deleteFromNamespaces: wrapperOptions.client.deleteFromNamespaces, + deleteByWorkspace: wrapperOptions.client.deleteByWorkspace, }; }; From dd6cebfca07f3f216578a407fef8cec279927337 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 3 Apr 2024 09:34:21 +0800 Subject: [PATCH 18/21] feat: use request app to store request workspace id Signed-off-by: SuZhou-Joe --- src/core/server/utils/workspace.test.ts | 4 ++-- src/core/server/utils/workspace.ts | 31 +++++++++++-------------- src/legacy/server/osd_server.d.ts | 8 ------- 3 files changed, 16 insertions(+), 27 deletions(-) diff --git a/src/core/server/utils/workspace.test.ts b/src/core/server/utils/workspace.test.ts index 49382cfac38f..7dfcff9e5d18 100644 --- a/src/core/server/utils/workspace.test.ts +++ b/src/core/server/utils/workspace.test.ts @@ -10,10 +10,10 @@ describe('updateWorkspaceState', () => { it('update with payload', () => { const requestMock = httpServerMock.createOpenSearchDashboardsRequest(); updateWorkspaceState(requestMock, { - id: 'foo', + requestWorkspaceId: 'foo', }); expect(getWorkspaceState(requestMock)).toEqual({ - id: 'foo', + requestWorkspaceId: 'foo', }); }); }); diff --git a/src/core/server/utils/workspace.ts b/src/core/server/utils/workspace.ts index c5dcf84b92d9..831755c414a5 100644 --- a/src/core/server/utils/workspace.ts +++ b/src/core/server/utils/workspace.ts @@ -3,14 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -/** - * This file is using {@link PluginsStates} to store workspace info into request. - * The best practice would be using {@link Server.register} to register plugins into the hapi server - * but OSD is wrappering the hapi server and the hapi server instance is hidden as internal implementation. - */ -import { PluginsStates } from '@hapi/hapi'; import { OpenSearchDashboardsRequest, ensureRawRequest } from '../http/router'; +export interface WorkspaceState { + requestWorkspaceId?: string; +} + /** * This function will be used as a proxy * because `ensureRequest` is only importable from core module. @@ -20,24 +18,23 @@ import { OpenSearchDashboardsRequest, ensureRawRequest } from '../http/router'; */ export const updateWorkspaceState = ( request: OpenSearchDashboardsRequest, - payload: Partial + payload: Partial ) => { const rawRequest = ensureRawRequest(request); - if (!rawRequest.plugins) { - rawRequest.plugins = {}; + if (!rawRequest.app) { + rawRequest.app = {}; } - if (!rawRequest.plugins.workspace) { - rawRequest.plugins.workspace = {}; - } - - rawRequest.plugins.workspace = { - ...rawRequest.plugins.workspace, + rawRequest.app = { + ...rawRequest.app, ...payload, }; }; -export const getWorkspaceState = (request: OpenSearchDashboardsRequest) => { - return ensureRawRequest(request).plugins?.workspace; +export const getWorkspaceState = (request: OpenSearchDashboardsRequest): WorkspaceState => { + const { requestWorkspaceId } = ensureRawRequest(request).app as WorkspaceState; + return { + requestWorkspaceId, + }; }; diff --git a/src/legacy/server/osd_server.d.ts b/src/legacy/server/osd_server.d.ts index 9d94349cf1c0..1f8535b59e87 100644 --- a/src/legacy/server/osd_server.d.ts +++ b/src/legacy/server/osd_server.d.ts @@ -60,14 +60,6 @@ declare module 'hapi' { } } -declare module '@hapi/hapi' { - interface PluginsStates { - workspace?: { - id?: string; - }; - } -} - type OsdMixinFunc = (osdServer: OsdServer, server: Server, config: any) => Promise | void; export interface PluginsSetup { From 7fd5160d3275661fc29ba48858e3ba1349ef75b6 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 3 Apr 2024 09:51:20 +0800 Subject: [PATCH 19/21] feat: use app state to store request workspace id Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.ts | 2 +- .../server/saved_objects/workspace_id_consumer_wrapper.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 088096df4d35..e7029f8387f6 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -54,7 +54,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { if (workspaceId) { updateWorkspaceState(request, { - id: workspaceId, + requestWorkspaceId: workspaceId, }); const requestUrl = new URL(request.url.toString()); requestUrl.pathname = cleanWorkspaceId(requestUrl.pathname); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 8ebd6b301563..74e8e99af71e 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -23,7 +23,7 @@ export class WorkspaceIdConsumerWrapper { ): T { const { workspaces, ...others } = options || {}; const workspaceState = getWorkspaceState(request); - const workspaceIdParsedFromRequest = workspaceState?.id; + const workspaceIdParsedFromRequest = workspaceState?.requestWorkspaceId; const workspaceIdsInUserOptions = options?.workspaces; let finalWorkspaces: string[] = []; if (options?.hasOwnProperty('workspaces')) { From 742ec22d5166f432a2d6f3e72dcaeeadbedf8b58 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 8 Apr 2024 10:18:11 +0800 Subject: [PATCH 20/21] refact: update types declaration Signed-off-by: SuZhou-Joe --- src/core/public/saved_objects/saved_objects_client.ts | 3 ++- .../export/get_sorted_objects_for_export.ts | 4 ++-- src/core/server/saved_objects/import/check_conflicts.ts | 3 ++- .../server/saved_objects/import/create_saved_objects.ts | 9 +++++++-- src/core/server/saved_objects/import/types.ts | 6 +++--- src/core/server/saved_objects/types.ts | 2 +- src/core/types/saved_objects.ts | 2 +- 7 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/core/public/saved_objects/saved_objects_client.ts b/src/core/public/saved_objects/saved_objects_client.ts index ce3ebc710ed1..d40b91188cfc 100644 --- a/src/core/public/saved_objects/saved_objects_client.ts +++ b/src/core/public/saved_objects/saved_objects_client.ts @@ -38,6 +38,7 @@ import { SavedObjectsClientContract as SavedObjectsApi, SavedObjectsFindOptions as SavedObjectFindOptionsServer, SavedObjectsMigrationVersion, + SavedObjectsBaseOptions, } from '../../server'; import { SimpleSavedObject } from './simple_saved_object'; @@ -65,7 +66,7 @@ export interface SavedObjectsCreateOptions { /** {@inheritDoc SavedObjectsMigrationVersion} */ migrationVersion?: SavedObjectsMigrationVersion; references?: SavedObjectReference[]; - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } /** 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 6c99db6c24af..44595b0c33ff 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 @@ -30,7 +30,7 @@ import Boom from '@hapi/boom'; import { createListStream } from '../../utils/streams'; -import { SavedObjectsClientContract, SavedObject } from '../types'; +import { SavedObjectsClientContract, SavedObject, SavedObjectsBaseOptions } from '../types'; import { fetchNestedDependencies } from './inject_nested_depdendencies'; import { sortObjects } from './sort_objects'; @@ -61,7 +61,7 @@ export interface SavedObjectsExportOptions { /** optional namespace to override the namespace used by the savedObjectsClient. */ namespace?: string; /** optional workspaces to override the workspaces used by the savedObjectsClient. */ - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } /** diff --git a/src/core/server/saved_objects/import/check_conflicts.ts b/src/core/server/saved_objects/import/check_conflicts.ts index d173af45f761..7afab618b1ff 100644 --- a/src/core/server/saved_objects/import/check_conflicts.ts +++ b/src/core/server/saved_objects/import/check_conflicts.ts @@ -35,6 +35,7 @@ import { SavedObjectsImportError, SavedObjectError, SavedObjectsImportRetry, + SavedObjectsBaseOptions, } from '../types'; interface CheckConflictsParams { @@ -44,7 +45,7 @@ interface CheckConflictsParams { ignoreRegularConflicts?: boolean; retries?: SavedObjectsImportRetry[]; createNewCopies?: boolean; - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } const isUnresolvableConflict = (error: SavedObjectError) => 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 f98e2e816b75..35d0ddd349fa 100644 --- a/src/core/server/saved_objects/import/create_saved_objects.ts +++ b/src/core/server/saved_objects/import/create_saved_objects.ts @@ -28,7 +28,12 @@ * under the License. */ -import { SavedObject, SavedObjectsClientContract, SavedObjectsImportError } from '../types'; +import { + SavedObject, + SavedObjectsBaseOptions, + SavedObjectsClientContract, + SavedObjectsImportError, +} from '../types'; import { extractErrors } from './extract_errors'; import { CreatedObject } from './types'; @@ -41,7 +46,7 @@ interface CreateSavedObjectsParams { overwrite?: boolean; dataSourceId?: string; dataSourceTitle?: string; - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } interface CreateSavedObjectsResult { createdObjects: Array>; diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index 689b8222b510..a243e08f83e0 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -29,7 +29,7 @@ */ import { Readable } from 'stream'; -import { SavedObjectsClientContract, SavedObject } from '../types'; +import { SavedObjectsClientContract, SavedObject, SavedObjectsBaseOptions } from '../types'; import { ISavedObjectTypeRegistry } from '..'; /** @@ -190,7 +190,7 @@ export interface SavedObjectsImportOptions { dataSourceId?: string; dataSourceTitle?: string; /** if specified, will import in given workspaces */ - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } /** @@ -215,7 +215,7 @@ export interface SavedObjectsResolveImportErrorsOptions { dataSourceId?: string; dataSourceTitle?: string; /** if specified, will import in given workspaces */ - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } export type CreatedObject = SavedObject & { destinationId?: string }; diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 22093b90d6b3..369cbfd53bf4 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -132,7 +132,7 @@ export interface SavedObjectsBaseOptions { /** Specify the namespace for this operation */ namespace?: string; /** Specify the workspaces for this operation */ - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObject['workspaces'] | null; } /** diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index 65ff595fa200..74890bb624a3 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -115,7 +115,7 @@ export interface SavedObject { */ originId?: string; /** Workspaces that this saved object exists in. */ - workspaces?: string[] | null; + workspaces?: string[]; /** Permissions that this saved objects exists in. */ permissions?: Permissions; } From ab0486e5e2811fc86e8c2df043c163c3c6c04e93 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 8 Apr 2024 11:32:41 +0800 Subject: [PATCH 21/21] fix: unit test error Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.test.ts | 2 +- .../server/saved_objects/workspace_id_consumer_wrapper.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts index a4586fecd3cd..0ad72b51b6dc 100644 --- a/src/plugins/workspace/server/plugin.test.ts +++ b/src/plugins/workspace/server/plugin.test.ts @@ -53,7 +53,7 @@ describe('Workspace server plugin', () => { expect(toolKitMock.rewriteUrl).toBeCalledWith('http://localhost/app'); expect(toolKitMock.next).toBeCalledTimes(0); expect(getWorkspaceState(requestWithWorkspaceInUrl)).toEqual({ - id: 'foo', + requestWorkspaceId: 'foo', }); const requestWithoutWorkspaceInUrl = httpServerMock.createOpenSearchDashboardsRequest({ diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index 21ef63c04867..112f31baf562 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -14,7 +14,7 @@ describe('WorkspaceIdConsumerWrapper', () => { const mockedClient = savedObjectsClientMock.create(); const workspaceEnabledMockRequest = httpServerMock.createOpenSearchDashboardsRequest(); updateWorkspaceState(workspaceEnabledMockRequest, { - id: 'foo', + requestWorkspaceId: 'foo', }); const wrapperClient = wrapperInstance.wrapperFactory({ client: mockedClient,