From 20cefab4d39d5a310d86ffa971f471a6dae12e38 Mon Sep 17 00:00:00 2001 From: yuboluo Date: Mon, 8 Jul 2024 13:15:55 +0800 Subject: [PATCH] [Workspace] Delete the virtual global workspace (#7165) * Delete the virtual global workspace Signed-off-by: yubonluo * Changeset file for PR #7165 created/updated * optimize the code Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * delete the useless code Signed-off-by: yubonluo * delete the useless code Signed-off-by: yubonluo --------- Signed-off-by: yubonluo Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/7165.yml | 2 + src/core/public/index.ts | 2 - src/core/server/index.ts | 7 +- .../service/lib/search_dsl/query_params.ts | 10 --- src/core/utils/constants.ts | 12 ---- src/core/utils/index.ts | 7 +- .../saved_objects_table.test.tsx | 19 ++++-- .../objects_table/saved_objects_table.tsx | 15 +--- .../server/routes/scroll_count.ts | 3 +- src/plugins/workspace/server/plugin.ts | 2 +- .../workspace_id_consumer_wrapper.test.ts | 68 +------------------ .../workspace_id_consumer_wrapper.ts | 41 ++--------- 12 files changed, 25 insertions(+), 163 deletions(-) create mode 100644 changelogs/fragments/7165.yml diff --git a/changelogs/fragments/7165.yml b/changelogs/fragments/7165.yml new file mode 100644 index 000000000000..39c2b93329e0 --- /dev/null +++ b/changelogs/fragments/7165.yml @@ -0,0 +1,2 @@ +feat: +- [Workspace] Delete the virtual global workspace ([#7165](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7165)) \ No newline at end of file diff --git a/src/core/public/index.ts b/src/core/public/index.ts index 1cb2ceab63de..4566b1dd867b 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -103,8 +103,6 @@ export { DEFAULT_APP_CATEGORIES, WORKSPACE_TYPE, cleanWorkspaceId, - PUBLIC_WORKSPACE_ID, - PUBLIC_WORKSPACE_NAME, DEFAULT_NAV_GROUPS, } from '../utils'; export { diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 0a5ea20fb964..66d856642e32 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -357,12 +357,7 @@ export { } from './metrics'; export { AppCategory, WorkspaceAttribute } from '../types'; -export { - DEFAULT_APP_CATEGORIES, - PUBLIC_WORKSPACE_ID, - PUBLIC_WORKSPACE_NAME, - WORKSPACE_TYPE, -} from '../utils'; +export { DEFAULT_APP_CATEGORIES, WORKSPACE_TYPE } from '../utils'; export { SavedObject, 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 ba6e62d877b6..552203c3f3c9 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 @@ -134,16 +134,6 @@ function getClauseForType( * Gets the clause that will filter for the workspace. */ function getClauseForWorkspace(workspace: string) { - if (workspace === '*') { - return { - bool: { - must: { - match_all: {}, - }, - }, - }; - } - return { bool: { must: [{ term: { workspaces: workspace } }], diff --git a/src/core/utils/constants.ts b/src/core/utils/constants.ts index c05d2b06e041..ecc1b7e863c4 100644 --- a/src/core/utils/constants.ts +++ b/src/core/utils/constants.ts @@ -3,18 +3,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { i18n } from '@osd/i18n'; - export const WORKSPACE_TYPE = 'workspace'; export const WORKSPACE_PATH_PREFIX = '/w'; - -/** - * public workspace has parity with global tenant, - * it includes saved objects with `public` as its workspace or without any workspce info - */ -export const PUBLIC_WORKSPACE_ID = 'public'; - -export const PUBLIC_WORKSPACE_NAME = i18n.translate('workspaces.public.workspace.default.name', { - defaultMessage: 'Global workspace', -}); diff --git a/src/core/utils/index.ts b/src/core/utils/index.ts index a79b5d0bf2f7..d9294e30361f 100644 --- a/src/core/utils/index.ts +++ b/src/core/utils/index.ts @@ -37,11 +37,6 @@ export { IContextProvider, } from './context'; export { DEFAULT_APP_CATEGORIES } from './default_app_categories'; -export { - WORKSPACE_PATH_PREFIX, - WORKSPACE_TYPE, - PUBLIC_WORKSPACE_ID, - PUBLIC_WORKSPACE_NAME, -} from './constants'; +export { WORKSPACE_PATH_PREFIX, WORKSPACE_TYPE } from './constants'; export { getWorkspaceIdFromUrl, formatUrlWithWorkspaceId, cleanWorkspaceId } from './workspace'; export { DEFAULT_NAV_GROUPS } from './default_nav_groups'; diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.test.tsx b/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.test.tsx index 684f0a322faa..bee992e0196d 100644 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.test.tsx +++ b/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.test.tsx @@ -64,7 +64,6 @@ import { import { Flyout, Relationships } from './components'; import { SavedObjectWithMetadata } from '../../types'; import { WorkspaceObject } from 'opensearch-dashboards/public'; -import { PUBLIC_WORKSPACE_NAME, PUBLIC_WORKSPACE_ID } from '../../../../../core/public'; import { TableProps } from './components/table'; const allowedTypes = ['index-pattern', 'visualization', 'dashboard', 'search']; @@ -413,10 +412,17 @@ describe('SavedObjectsTable', () => { }); it('should export all, accounting for the current workspace criteria', async () => { - const component = shallowRender(); + const workspaceList: WorkspaceObject[] = [ + { + id: 'workspace1', + name: 'foo', + }, + ]; + workspaces.workspaceList$.next(workspaceList); + const component = shallowRender({ workspaces }); component.instance().onQueryChange({ - query: Query.parse(`test workspaces:("${PUBLIC_WORKSPACE_NAME}")`), + query: Query.parse(`test workspaces:("foo")`), }); // Ensure all promises resolve @@ -435,7 +441,7 @@ describe('SavedObjectsTable', () => { allowedTypes, 'test*', true, - [PUBLIC_WORKSPACE_ID] + ['workspace1'] ); expect(saveAsMock).toHaveBeenCalledWith(blob, 'export.ndjson'); expect(notifications.toasts.addSuccess).toHaveBeenCalledWith({ @@ -708,10 +714,9 @@ describe('SavedObjectsTable', () => { expect(filters.length).toBe(2); expect(filters[0].field).toBe('type'); expect(filters[1].field).toBe('workspaces'); - expect(filters[1].options.length).toBe(3); + expect(filters[1].options.length).toBe(2); expect(filters[1].options[0].value).toBe('foo'); expect(filters[1].options[1].value).toBe('bar'); - expect(filters[1].options[2].value).toBe(PUBLIC_WORKSPACE_NAME); }); it('workspace filter only include current workspaces when in a workspace', async () => { @@ -847,7 +852,7 @@ describe('SavedObjectsTable', () => { expect(findObjectsMock).toBeCalledWith( http, expect.objectContaining({ - workspaces: expect.arrayContaining(['workspace1', 'workspace2', PUBLIC_WORKSPACE_ID]), + workspaces: expect.arrayContaining(['workspace1', 'workspace2']), }) ); }); diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx b/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx index 04d7a46d24d2..8cc50bb19a80 100644 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx +++ b/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx @@ -70,7 +70,6 @@ import { WorkspaceAttribute, } from 'src/core/public'; import { Subscription } from 'rxjs'; -import { PUBLIC_WORKSPACE_ID, PUBLIC_WORKSPACE_NAME } from '../../../../../core/public'; import { RedirectAppLinks } from '../../../../opensearch_dashboards_react/public'; import { IndexPatternsContract } from '../../../../data/public'; import { @@ -195,7 +194,7 @@ export class SavedObjectsTable extends Component ws.id).concat(PUBLIC_WORKSPACE_ID); + return availableWorkspaces?.map((ws) => ws.id); } else { return [currentWorkspaceId]; } @@ -205,7 +204,6 @@ export class SavedObjectsTable extends Component(); - workspaceNameIdMap.set(PUBLIC_WORKSPACE_NAME, PUBLIC_WORKSPACE_ID); // workspace name is unique across the system availableWorkspaces?.forEach((workspace) => { workspaceNameIdMap.set(workspace.name, workspace.id); @@ -956,8 +954,6 @@ export class SavedObjectsTable extends Component workspace.id === PUBLIC_WORKSPACE_ID) > -1; const wsFilterOptions = availableWorkspaces .filter((ws) => { return this.workspaceIdQuery?.includes(ws.id); @@ -970,15 +966,6 @@ export class SavedObjectsTable extends Component { @@ -92,7 +91,7 @@ export const registerScrollForCountRoute = (router: IRouter) => { }); } if (requestHasWorkspaces) { - const resultWorkspaces = result.workspaces || [PUBLIC_WORKSPACE_ID]; + const resultWorkspaces = result.workspaces || []; resultWorkspaces.forEach((ws) => { counts.workspaces[ws] = counts.workspaces[ws] || 0; counts.workspaces[ws]++; diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 1479da984580..1081284b7d9e 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -137,7 +137,7 @@ export class WorkspacePlugin implements Plugin { }); }); - it(`Should set workspacesSearchOperator to OR when search with public workspace`, async () => { - await wrapperClient.find({ - type: 'dashboard', - workspaces: [PUBLIC_WORKSPACE_ID], - }); - expect(mockedClient.find).toBeCalledWith({ - type: 'dashboard', - workspaces: [PUBLIC_WORKSPACE_ID], - workspacesSearchOperator: 'OR', - }); - }); - - it(`Should set workspace as pubic when workspace is not specified`, async () => { - const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); - updateWorkspaceState(mockRequest, {}); - const mockedWrapperClient = wrapperInstance.wrapperFactory({ - client: mockedClient, - typeRegistry: requestHandlerContext.savedObjects.typeRegistry, - request: mockRequest, - }); - await mockedWrapperClient.find({ - type: ['dashboard', 'visualization'], - }); - expect(mockedClient.find).toBeCalledWith({ - type: ['dashboard', 'visualization'], - workspaces: [PUBLIC_WORKSPACE_ID], - workspacesSearchOperator: 'OR', - }); - }); - - it(`Should remove public workspace when permission control is enabled`, async () => { - const consumer = new WorkspaceIdConsumerWrapper(true); - const client = consumer.wrapperFactory({ - client: mockedClient, - typeRegistry: requestHandlerContext.savedObjects.typeRegistry, - request: workspaceEnabledMockRequest, - }); - await client.find({ - type: 'dashboard', - workspaces: ['bar', PUBLIC_WORKSPACE_ID], - }); - expect(mockedClient.find).toBeCalledWith({ - type: 'dashboard', - workspaces: ['bar'], - workspacesSearchOperator: 'OR', - }); - }); - - it(`Should not override workspacesSearchOperator when workspacesSearchOperator is specified`, async () => { - await wrapperClient.find({ - type: 'dashboard', - workspaces: [PUBLIC_WORKSPACE_ID], - workspacesSearchOperator: 'AND', - }); - expect(mockedClient.find).toBeCalledWith({ - type: 'dashboard', - workspaces: [PUBLIC_WORKSPACE_ID], - workspacesSearchOperator: 'AND', - }); - }); - - it(`Should not pass a empty workspace array`, async () => { - const workspaceIdConsumerWrapper = new WorkspaceIdConsumerWrapper(true); + it(`Should pass a empty workspace array`, async () => { + const workspaceIdConsumerWrapper = new WorkspaceIdConsumerWrapper(); const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); updateWorkspaceState(mockRequest, {}); const mockedWrapperClient = workspaceIdConsumerWrapper.wrapperFactory({ @@ -189,10 +127,8 @@ describe('WorkspaceIdConsumerWrapper', () => { await mockedWrapperClient.find({ type: ['dashboard', 'visualization'], }); - // empty workspace array will get deleted expect(mockedClient.find).toBeCalledWith({ type: ['dashboard', 'visualization'], - workspacesSearchOperator: 'OR', }); }); }); 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 9a41028908f9..1f9e196d1f49 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,8 +12,6 @@ import { SavedObjectsCheckConflictsObject, OpenSearchDashboardsRequest, SavedObjectsFindOptions, - PUBLIC_WORKSPACE_ID, - WORKSPACE_TYPE, } from '../../../../core/server'; type WorkspaceOptions = Pick | undefined; @@ -41,14 +39,6 @@ export class WorkspaceIdConsumerWrapper { }; } - private isWorkspaceType(type: SavedObjectsFindOptions['type']): boolean { - if (Array.isArray(type)) { - return type.every((item) => item === WORKSPACE_TYPE); - } - - return type === WORKSPACE_TYPE; - } - public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => { return { ...wrapperOptions.client, @@ -76,32 +66,9 @@ export class WorkspaceIdConsumerWrapper { ), delete: wrapperOptions.client.delete, find: (options: SavedObjectsFindOptions) => { - const findOptions = this.formatWorkspaceIdParams(wrapperOptions.request, options); - if (this.isWorkspaceType(findOptions.type)) { - return wrapperOptions.client.find(findOptions); - } - - // if workspace is enabled, we always find by workspace - if (!findOptions.workspaces || findOptions.workspaces.length === 0) { - findOptions.workspaces = [PUBLIC_WORKSPACE_ID]; - } - - // `PUBLIC_WORKSPACE_ID` includes both saved objects without any workspace and with `PUBLIC_WORKSPACE_ID` workspace - const index = findOptions.workspaces - ? findOptions.workspaces.indexOf(PUBLIC_WORKSPACE_ID) - : -1; - if (!findOptions.workspacesSearchOperator && findOptions.workspaces && index !== -1) { - findOptions.workspacesSearchOperator = 'OR'; - // remove this deletion logic when public workspace becomes to real - if (this.isPermissionControlEnabled) { - // remove public workspace to make sure we can pass permission control validation, more details in `WorkspaceSavedObjectsClientWrapper` - findOptions.workspaces.splice(index, 1); - } - } - if (findOptions.workspaces && findOptions.workspaces.length === 0) { - delete findOptions.workspaces; - } - return wrapperOptions.client.find(findOptions); + return wrapperOptions.client.find( + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ); }, bulkGet: wrapperOptions.client.bulkGet, get: wrapperOptions.client.get, @@ -113,5 +80,5 @@ export class WorkspaceIdConsumerWrapper { }; }; - constructor(private isPermissionControlEnabled?: boolean) {} + constructor() {} }