[Workspace] Delete the virtual global workspace (#7165) (#7189)

* Delete the virtual global workspace * Changeset file for PR #7165 created/updated * optimize the code * optimize the code * optimize the code * optimize the code * delete the useless code * delete the useless code --------- (cherry picked from commit 20cefab) Signed-off-by: yubonluo <yubonluo@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
opensearch-project · Jul 9, 2024 · e9ba24f · e9ba24f
1 parent 5d33328
commit e9ba24f
Show file tree

Hide file tree

Showing 12 changed files with 25 additions and 163 deletions.
diff --git a/changelogs/fragments/7165.yml 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))
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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/...d_objects_management/public/management_section/objects_table/saved_objects_table.test.tsx b/...d_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/.../saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx b/.../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<SavedObjectsTableProps, SavedOb
       // application home
       if (!currentWorkspaceId) {
         // public workspace is virtual at this moment
-        return availableWorkspaces?.map((ws) => ws.id).concat(PUBLIC_WORKSPACE_ID);
+        return availableWorkspaces?.map((ws) => ws.id);
       } else {
         return [currentWorkspaceId];
       }
@@ -205,7 +204,6 @@ export class SavedObjectsTable extends Component<SavedObjectsTableProps, SavedOb
   private get workspaceNameIdLookup() {
     const { availableWorkspaces } = this.state;
     const workspaceNameIdMap = new Map<string, string>();
-    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<SavedObjectsTableProps, SavedOb
     // Add workspace filter
     if (workspaceEnabled && availableWorkspaces?.length) {
       const wsCounts = savedObjectCounts.workspaces || {};
-      const publicWorkspaceExists =
-        availableWorkspaces.findIndex((workspace) => 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<SavedObjectsTableProps, SavedOb
           };
         });
 
-      // add public workspace option only if we don't have it as real workspace
-      if (!currentWorkspaceId && !publicWorkspaceExists) {
-        wsFilterOptions.push({
-          name: PUBLIC_WORKSPACE_NAME,
-          value: PUBLIC_WORKSPACE_NAME,
-          view: `${PUBLIC_WORKSPACE_NAME} (${wsCounts[PUBLIC_WORKSPACE_ID] || 0})`,
-        });
-      }
-
       filters.push({
         type: 'field_value_selection',
         field: 'workspaces',

diff --git a/src/plugins/saved_objects_management/server/routes/scroll_count.ts b/src/plugins/saved_objects_management/server/routes/scroll_count.ts
@@ -30,7 +30,6 @@
 
 import { schema } from '@osd/config-schema';
 import { IRouter, SavedObjectsFindOptions } from 'src/core/server';
-import { PUBLIC_WORKSPACE_ID } from '../../../../core/server';
 import { findAll } from '../lib';
 
 export const registerScrollForCountRoute = (router: IRouter) => {
@@ -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
@@ -137,7 +137,7 @@ export class WorkspacePlugin implements Plugin<WorkspacePluginSetup, WorkspacePl
     core.savedObjects.addClientWrapper(
       PRIORITY_FOR_WORKSPACE_ID_CONSUMER_WRAPPER,
       WORKSPACE_ID_CONSUMER_WRAPPER_ID,
-      new WorkspaceIdConsumerWrapper(isPermissionControlEnabled).wrapperFactory
+      new WorkspaceIdConsumerWrapper().wrapperFactory
     );
 
     const maxImportExportSize = core.savedObjects.getImportExportObjectLimit();

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
@@ -5,7 +5,6 @@
 
 import { updateWorkspaceState } from '../../../../core/server/utils';
 import { SavedObject } from '../../../../core/public';
-import { PUBLIC_WORKSPACE_ID } from '../../../../core/server';
 import { httpServerMock, savedObjectsClientMock, coreMock } from '../../../../core/server/mocks';
 import { WorkspaceIdConsumerWrapper } from './workspace_id_consumer_wrapper';
 
@@ -116,69 +115,8 @@ describe('WorkspaceIdConsumerWrapper', () => {
       });
     });
 
-    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
@@ -12,8 +12,6 @@ import {
   SavedObjectsCheckConflictsObject,
   OpenSearchDashboardsRequest,
   SavedObjectsFindOptions,
-  PUBLIC_WORKSPACE_ID,
-  WORKSPACE_TYPE,
 } from '../../../../core/server';
 
 type WorkspaceOptions = Pick<SavedObjectsBaseOptions, 'workspaces'> | 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() {}
 }