[API] Delete saved objects by workspace (opensearch-project#216)

* Delete saved objects by workspace Signed-off-by: Hailong Cui <[email protected]> fix osd boostrap Signed-off-by: Hailong Cui <[email protected]> * add unit test Signed-off-by: Hailong Cui <[email protected]> * fix can't delete workspace due to invalid permission Signed-off-by: Hailong Cui <[email protected]> --------- Signed-off-by: Hailong Cui <[email protected]> (cherry picked from commit 7773811)
Hailong-am · Oct 13, 2023 · 1e5c81e · 1e5c81e
1 parent a74a6ee
commit 1e5c81e
Show file tree

Hide file tree

Showing 7 changed files with 108 additions and 43 deletions.
diff --git a/src/core/server/index.ts b/src/core/server/index.ts
@@ -477,7 +477,7 @@ export interface CoreSetup<TPluginsStart extends object = object, TStart = unkno
 export type StartServicesAccessor<
   TPluginsStart extends object = object,
   TStart = unknown
-> = () => Promise<[CoreStart, TPluginsStart, TStart]>;
+  > = () => Promise<[CoreStart, TPluginsStart, TStart]>;
 
 /**
  * Context passed to the plugins `start` method.

diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js
@@ -175,9 +175,8 @@ describe('SavedObjectsRepository', () => {
     return {
       // NOTE: OpenSearch returns more fields (_index, _type) but the SavedObjectsRepository method ignores these
       found: true,
-      _id: `${
-        registry.isSingleNamespace(type) && namespaceId ? `${namespaceId}:` : ''
-      }${type}:${id}`,
+      _id: `${registry.isSingleNamespace(type) && namespaceId ? `${namespaceId}:` : ''
+        }${type}:${id}`,
       ...mockVersionProps,
       _source: {
         ...(registry.isSingleNamespace(type) && { namespace: namespaceId }),
@@ -1444,9 +1443,8 @@ describe('SavedObjectsRepository', () => {
     const getMockBulkUpdateResponse = (objects, options, includeOriginId) => ({
       items: objects.map(({ type, id }) => ({
         update: {
-          _id: `${
-            registry.isSingleNamespace(type) && options?.namespace ? `${options?.namespace}:` : ''
-          }${type}:${id}`,
+          _id: `${registry.isSingleNamespace(type) && options?.namespace ? `${options?.namespace}:` : ''
+            }${type}:${id}`,
           ...mockVersionProps,
           get: {
             _source: {

diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts
@@ -421,13 +421,13 @@ export class SavedObjectsRepository {
       }));
     const bulkGetResponse = bulkGetDocs.length
       ? await this.client.mget(
-          {
-            body: {
-              docs: bulkGetDocs,
-            },
+        {
+          body: {
+            docs: bulkGetDocs,
           },
-          { ignore: [404] }
-        )
+        },
+        { ignore: [404] }
+      )
       : undefined;
 
     let bulkRequestIndexCounter = 0;
@@ -562,9 +562,9 @@ export class SavedObjectsRepository {
 
     const bulkResponse = bulkCreateParams.length
       ? await this.client.bulk({
-          refresh,
-          body: bulkCreateParams,
-        })
+        refresh,
+        body: bulkCreateParams,
+      })
       : undefined;
 
     return {
@@ -643,13 +643,13 @@ export class SavedObjectsRepository {
     }));
     const bulkGetResponse = bulkGetDocs.length
       ? await this.client.mget(
-          {
-            body: {
-              docs: bulkGetDocs,
-            },
+        {
+          body: {
+            docs: bulkGetDocs,
           },
-          { ignore: [404] }
-        )
+        },
+        { ignore: [404] }
+      )
       : undefined;
 
     const errors: SavedObjectsCheckConflictsResponse['errors'] = [];
@@ -804,7 +804,11 @@ export class SavedObjectsRepository {
   }
 
   /**
+<<<<<<< HEAD
    * Deletes all objects from the provided workspace. It used when delete a workspace.
+=======
+   * Deletes all objects from the provided workspace. It used when deleting a workspace.
+>>>>>>> 7773811875 ([API] Delete saved objects by workspace (#216))
    *
    * @param {string} workspace
    * @param options SavedObjectsDeleteByWorkspaceOptions
@@ -1059,13 +1063,13 @@ export class SavedObjectsRepository {
       }));
     const bulkGetResponse = bulkGetDocs.length
       ? await this.client.mget(
-          {
-            body: {
-              docs: bulkGetDocs,
-            },
+        {
+          body: {
+            docs: bulkGetDocs,
           },
-          { ignore: [404] }
-        )
+        },
+        { ignore: [404] }
+      )
       : undefined;
 
     return {
@@ -1692,15 +1696,15 @@ export class SavedObjectsRepository {
       }));
     const bulkGetResponse = bulkGetDocs.length
       ? await this.client.mget(
-          {
-            body: {
-              docs: bulkGetDocs,
-            },
+        {
+          body: {
+            docs: bulkGetDocs,
           },
-          {
-            ignore: [404],
-          }
-        )
+        },
+        {
+          ignore: [404],
+        }
+      )
       : undefined;
 
     let bulkUpdateRequestIndexCounter = 0;
@@ -1783,10 +1787,10 @@ export class SavedObjectsRepository {
     const { refresh = DEFAULT_REFRESH_SETTING } = options;
     const bulkUpdateResponse = bulkUpdateParams.length
       ? await this.client.bulk({
-          refresh,
-          body: bulkUpdateParams,
-          _source_includes: ['originId'],
-        })
+        refresh,
+        body: bulkUpdateParams,
+        _source_includes: ['originId'],
+      })
       : undefined;
 
     return {

diff --git a/src/core/server/saved_objects/service/saved_objects_client.test.js b/src/core/server/saved_objects/service/saved_objects_client.test.js
@@ -207,3 +207,18 @@ test(`#deleteFromNamespaces`, async () => {
   expect(mockRepository.deleteFromNamespaces).toHaveBeenCalledWith(type, id, namespaces, options);
   expect(result).toBe(returnValue);
 });
+
+test(`#deleteByWorkspace`, async () => {
+  const returnValue = Symbol();
+  const mockRepository = {
+    deleteByWorkspace: jest.fn().mockResolvedValue(returnValue),
+  };
+  const client = new SavedObjectsClient(mockRepository);
+
+  const workspace = Symbol();
+  const options = Symbol();
+  const result = await client.deleteByWorkspace(workspace, options);
+
+  expect(mockRepository.deleteByWorkspace).toHaveBeenCalledWith(workspace, options);
+  expect(result).toBe(returnValue);
+});
diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts
@@ -309,6 +309,15 @@ export interface SavedObjectsUpdateResponse<T = unknown>
   references: SavedObjectReference[] | undefined;
 }
 
+/**
+ *
+ * @public
+ */
+export interface SavedObjectsDeleteByWorkspaceOptions extends SavedObjectsBaseOptions {
+  /** The OpenSearch supports only boolean flag for this operation */
+  refresh?: boolean;
+}
+
 /**
  *
  * @public

diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts
@@ -8,6 +8,7 @@ import { omit } from 'lodash';
 import * as osdTestServer from '../../../../core/test_helpers/osd_server';
 import { WorkspaceRoutePermissionItem } from '../types';
 import { WorkspacePermissionMode } from '../../../../core/server';
+import { WORKSPACE_TYPE } from '../../../../core/server';
 
 const testWorkspace: WorkspaceAttribute & {
   permissions: WorkspaceRoutePermissionItem;
@@ -28,6 +29,7 @@ describe('workspace service', () => {
           workspace: {
             enabled: true,
           },
+          migrations: { skip: false },
         },
       },
     });
@@ -49,7 +51,10 @@ describe('workspace service', () => {
         .expect(200);
       await Promise.all(
         listResult.body.result.workspaces.map((item: WorkspaceAttribute) =>
-          osdTestServer.request.delete(root, `/api/workspaces/${item.id}`).expect(200)
+          // this will delete reserved workspace
+          osdTestServer.request
+            .delete(root, `/api/saved_objects/${WORKSPACE_TYPE}/${item.id}`)
+            .expect(200)
         )
       );
     });
@@ -119,6 +124,16 @@ describe('workspace service', () => {
         })
         .expect(200);
 
+      await osdTestServer.request
+        .post(root, `/api/saved_objects/index-pattern/logstash-*`)
+        .send({
+          attributes: {
+            title: 'logstash-*',
+          },
+          workspaces: [result.body.result.id],
+        })
+        .expect(200);
+
       await osdTestServer.request
         .delete(root, `/api/workspaces/${result.body.result.id}`)
         .expect(200);
@@ -129,6 +144,29 @@ describe('workspace service', () => {
       );
 
       expect(getResult.body.success).toEqual(false);
+
+      // saved objects been deleted
+      await osdTestServer.request
+        .get(root, `/api/saved_objects/index-pattern/logstash-*`)
+        .expect(404);
+    });
+    it('delete reserved workspace', async () => {
+      const reservedWorkspace: WorkspaceAttribute = { ...testWorkspace, reserved: true };
+      const result: any = await osdTestServer.request
+        .post(root, `/api/workspaces`)
+        .send({
+          attributes: omit(reservedWorkspace, 'id'),
+        })
+        .expect(200);
+
+      const deleteResult = await osdTestServer.request
+        .delete(root, `/api/workspaces/${result.body.result.id}`)
+        .expect(200);
+
+      expect(deleteResult.body.success).toEqual(false);
+      expect(deleteResult.body.error).toEqual(
+        `Reserved workspace ${result.body.result.id} is not allowed to delete.`
+      );
     });
     it('list', async () => {
       await osdTestServer.request

diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts
@@ -387,13 +387,14 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl {
         return {
           success: false,
           error: i18n.translate('workspace.deleteReservedWorkspace.errorMessage', {
-            defaultMessage: 'Reserved workspace {id} is not allowed to delete: ',
+            defaultMessage: 'Reserved workspace {id} is not allowed to delete.',
             values: { id: workspaceInDB.id },
           }),
         };
       }
-      await savedObjectClient.delete(WORKSPACE_TYPE, id);
       await savedObjectClient.deleteByWorkspace(id);
+      // delete workspace itself at last, deleteByWorkspace depends on the workspace to do permission check
+      await savedObjectClient.delete(WORKSPACE_TYPE, id);
       return {
         success: true,
         result: true,