Add more unit test for share/delete api

src/core/server/index.ts

@@ -324,6 +324,8 @@ export {
   SavedObjectsAddToWorkspacesOptions,
   SavedObjectsAddToWorkspacesResponse,
   SavedObjectsDeleteByWorkspaceOptions,
+  SavedObjectsDeleteFromWorkspacesOptions,
+  SavedObjectsDeleteFromWorkspacesResponse,
   Permissions,
   ACL,
   Principals,

src/core/server/saved_objects/routes/integration_tests/share.test.ts

@@ -0,0 +1,172 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import supertest from 'supertest';
+import { UnwrapPromise } from '@osd/utility-types';
+import { registerShareRoute } from '../share';
+import { savedObjectsClientMock } from '../../../../../core/server/mocks';
+import { createExportableType, setupServer } from '../test_utils';
+import { WORKSPACE_TYPE } from '../../../../utils/constants';
+import { typeRegistryMock } from '../../saved_objects_type_registry.mock';
+
+type SetupServerReturn = UnwrapPromise<ReturnType<typeof setupServer>>;
+
+describe('POST /api/saved_objects/_share', () => {
+  let server: SetupServerReturn['server'];
+  let httpSetup: SetupServerReturn['httpSetup'];
+  let handlerContext: SetupServerReturn['handlerContext'];
+  let savedObjectsClient: ReturnType<typeof savedObjectsClientMock.create>;
+  let typeRegistry: ReturnType<typeof typeRegistryMock.create>;
+  const allowedTypes = ['index-pattern', 'dashboard', 'settings'];
+
+  beforeEach(async () => {
+    const clientResponse = [
+      {
+        id: 'abc123',
+        type: 'index-pattern',
+        workspaces: ['ws-1', 'ws-2'],
+      },
+    ];
+
+    const bulkGetResponse = {
+      saved_objects: [
+        {
+          id: 'abc123',
+          type: 'index-pattern',
+          title: 'logstash-*',
+          version: 'foo',
+          references: [],
+          attributes: {},
+          workspaces: ['ws-1'],
+        },
+      ],
+    };
+
+    ({ server, httpSetup, handlerContext } = await setupServer());
+    typeRegistry = handlerContext.savedObjects.typeRegistry;
+    typeRegistry.getAllTypes.mockReturnValue(allowedTypes.map(createExportableType));
+
+    savedObjectsClient = handlerContext.savedObjects.client;
+    savedObjectsClient.addToWorkspaces.mockResolvedValue(clientResponse);
+    savedObjectsClient.bulkGet.mockImplementation(() => Promise.resolve(bulkGetResponse));
+
+    const router = httpSetup.createRouter('/api/saved_objects/');
+    registerShareRoute(router);
+
+    await server.start();
+  });
+
+  afterEach(async () => {
+    await server.stop();
+  });
+
+  it('workspace itself are not allowed to share', async () => {
+    const result = await supertest(httpSetup.server.listener)
+      .post('/api/saved_objects/_share')
+      .send({
+        objects: [
+          {
+            id: 'abc123',
+            type: WORKSPACE_TYPE,
+          },
+        ],
+        targetWorkspaceIds: ['ws-2'],
+      })
+      .expect(400);
+
+    expect(result.body.message).toEqual(
+      `Trying to share object(s) with non-shareable types: ${WORKSPACE_TYPE}:abc123`
+    );
+  });
+
+  it('ignore legacy saved objects when share', async () => {
+    const bulkGetResponse = {
+      saved_objects: [
+        {
+          id: 'settings-1.0',
+          type: 'settings',
+          title: 'Advanced-settings',
+          version: 'foo',
+          references: [],
+          attributes: {},
+          workspaces: undefined,
+        },
+      ],
+    };
+    savedObjectsClient.bulkGet.mockImplementation(() => Promise.resolve(bulkGetResponse));
+
+    const clientResponse = [
+      {
+        id: 'settings-1.0',
+        type: 'settings',
+        workspaces: undefined,
+      },
+    ];
+
+    const result = await supertest(httpSetup.server.listener)
+      .post('/api/saved_objects/_share')
+      .send({
+        objects: [
+          {
+            id: 'settings-1.0',
+            type: 'settings',
+          },
+        ],
+        targetWorkspaceIds: ['ws-2'],
+      });
+
+    expect(result.body).toEqual(clientResponse);
+  });
+
+  it('formats successful response', async () => {
+    const clientResponse = [
+      {
+        id: 'abc123',
+        type: 'index-pattern',
+        workspaces: ['ws-1', 'ws-2'],
+      },
+    ];
+    const result = await supertest(httpSetup.server.listener)
+      .post('/api/saved_objects/_share')
+      .send({
+        objects: [
+          {
+            id: 'abc123',
+            type: 'index-pattern',
+          },
+        ],
+        targetWorkspaceIds: ['ws-2'],
+      })
+      .expect(200);
+
+    expect(result.body).toEqual(clientResponse);
+  });
+
+  it('calls upon savedObjectClient.addToWorkspaces', async () => {
+    await supertest(httpSetup.server.listener)
+      .post('/api/saved_objects/_share')
+      .send({
+        objects: [
+          {
+            id: 'abc123',
+            type: 'index-pattern',
+          },
+        ],
+        targetWorkspaceIds: ['ws-2'],
+      })
+      .expect(200);
+
+    expect(savedObjectsClient.addToWorkspaces).toHaveBeenCalledWith(
+      [
+        {
+          id: 'abc123',
+          type: 'index-pattern',
+          workspaces: ['ws-1'],
+        },
+      ],
+      ['ws-2'],
+      { workspaces: undefined }
+    );
+  });
+});

src/core/server/saved_objects/routes/share.ts

@@ -6,7 +6,7 @@
 import { schema } from '@osd/config-schema';
 import { IRouter } from '../../http';
 import { exportSavedObjectsToStream } from '../export';
-import { validateObjects } from './utils';
+import { filterInvalidObjects } from './utils';
 import { collectSavedObjects } from '../import/collect_saved_objects';
 import { WORKSPACE_TYPE } from '../../../utils';
 
@@ -39,12 +39,14 @@ export const registerShareRoute = (router: IRouter) => {
         .filter((type) => type.name !== WORKSPACE_TYPE)
         .map((t) => t.name);
 
-      if (objects) {
-        const validationError = validateObjects(objects, supportedTypes);
-        if (validationError) {
+      if (objects.length) {
+        const invalidObjects = filterInvalidObjects(objects, supportedTypes);
+        if (invalidObjects.length) {
           return res.badRequest({
             body: {
-              message: validationError,
+              message: `Trying to share object(s) with non-shareable types: ${invalidObjects
+                .map((obj) => `${obj.type}:${obj.id}`)
+                .join(', ')}`,
             },
           });
         }

src/core/server/saved_objects/routes/utils.test.ts

@@ -28,7 +28,12 @@
  * under the License.
  */
 
-import { createSavedObjectsStreamFromNdJson, validateTypes, validateObjects } from './utils';
+import {
+  createSavedObjectsStreamFromNdJson,
+  validateTypes,
+  validateObjects,
+  filterInvalidObjects,
+} from './utils';
 import { Readable } from 'stream';
 import { createPromiseFromStreams, createConcatStream } from '../../utils/streams';
 
@@ -165,3 +170,36 @@ describe('validateObjects', () => {
     ).toBeUndefined();
   });
 });
+
+describe('filterInvalidObjects', () => {
+  const allowedTypes = ['config', 'index-pattern', 'dashboard'];
+
+  it('returns invalid objects that types are not allowed', () => {
+    expect(
+      filterInvalidObjects(
+        [
+          { id: '1', type: 'config' },
+          { id: '1', type: 'not-allowed' },
+          { id: '42', type: 'not-allowed-either' },
+        ],
+        allowedTypes
+      )
+    ).toMatchObject([
+      { id: '1', type: 'not-allowed' },
+      { id: '42', type: 'not-allowed-either' },
+    ]);
+  });
+
+  it('returns empty array if all objects have allowed types', () => {
+    expect(
+      validateObjects(
+        [
+          { id: '1', type: 'config' },
+          { id: '2', type: 'config' },
+          { id: '1', type: 'index-pattern' },
+        ],
+        allowedTypes
+      )
+    ).toEqual(undefined);
+  });
+});

src/core/server/saved_objects/routes/utils.ts

@@ -73,3 +73,10 @@ export function validateObjects(
       .join(', ')}`;
   }
 }
+
+export function filterInvalidObjects(
+  objects: Array<{ id: string; type: string }>,
+  supportedTypes: string[]
+): Array<{ id: string; type: string }> {
+  return objects.filter((obj) => !supportedTypes.includes(obj.type));
+}