From 06b3198d0680f316e34adf691920feff2f638a74 Mon Sep 17 00:00:00 2001 From: Hailong Cui Date: Wed, 20 Sep 2023 13:15:54 +0800 Subject: [PATCH] add more unit test cases Signed-off-by: Hailong Cui --- src/core/server/index.ts | 2 + .../routes/integration_tests/share.test.ts | 172 +++++ src/core/server/saved_objects/routes/share.ts | 12 +- .../server/saved_objects/routes/utils.test.ts | 40 +- src/core/server/saved_objects/routes/utils.ts | 7 + .../service/lib/repository.test.js | 629 +++++++++++++++++- .../saved_objects/service/lib/repository.ts | 52 +- .../service/saved_objects_client.mock.ts | 1 + .../service/saved_objects_client.ts | 16 +- 9 files changed, 867 insertions(+), 64 deletions(-) create mode 100644 src/core/server/saved_objects/routes/integration_tests/share.test.ts diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 42725b1c90d4..01b24777b0bd 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -324,6 +324,8 @@ export { SavedObjectsAddToWorkspacesOptions, SavedObjectsAddToWorkspacesResponse, SavedObjectsDeleteByWorkspaceOptions, + SavedObjectsDeleteFromWorkspacesOptions, + SavedObjectsDeleteFromWorkspacesResponse, Permissions, ACL, Principals, diff --git a/src/core/server/saved_objects/routes/integration_tests/share.test.ts b/src/core/server/saved_objects/routes/integration_tests/share.test.ts new file mode 100644 index 000000000000..d5fe01ba4115 --- /dev/null +++ b/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>; + +describe('POST /api/saved_objects/_share', () => { + let server: SetupServerReturn['server']; + let httpSetup: SetupServerReturn['httpSetup']; + let handlerContext: SetupServerReturn['handlerContext']; + let savedObjectsClient: ReturnType; + let typeRegistry: ReturnType; + 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 } + ); + }); +}); diff --git a/src/core/server/saved_objects/routes/share.ts b/src/core/server/saved_objects/routes/share.ts index 6d767e08c227..544ae372871c 100644 --- a/src/core/server/saved_objects/routes/share.ts +++ b/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(', ')}`, }, }); } diff --git a/src/core/server/saved_objects/routes/utils.test.ts b/src/core/server/saved_objects/routes/utils.test.ts index b959d0e4ed48..68bb86b83365 100644 --- a/src/core/server/saved_objects/routes/utils.test.ts +++ b/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); + }); +}); diff --git a/src/core/server/saved_objects/routes/utils.ts b/src/core/server/saved_objects/routes/utils.ts index 73bedf20b942..769c02e0d7c9 100644 --- a/src/core/server/saved_objects/routes/utils.ts +++ b/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)); +} diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index f6a5e4857eea..816d1013e5be 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -430,6 +430,190 @@ describe('SavedObjectsRepository', () => { }); }); + describe('#addToWorkspaces', () => { + const id = 'some-id'; + const type = 'dashboard'; + const currentWs1 = 'public'; + const newWs1 = 'new-workspace-1'; + const newWs2 = 'new-workspace-2'; + + const sharedObjects = [ + { + type, + id, + workspaces: [currentWs1], + }, + ]; + + const getMockBulkUpdateResponse = (objects, newWorkspaces, errors = false) => ({ + errors, + items: objects.map(({ type, id, workspaces }) => ({ + update: { + _id: `${type}:${id}`, + ...mockVersionProps, + status: errors ? 404 : 200, + error: errors && { + type: 'document_missing_exception', + reason: `${type}:${id}: document missing`, + }, + get: { + _source: { + type, + id, + workspaces: [...new Set(workspaces.concat(newWorkspaces))], + }, + }, + result: 'updated', + }, + })), + }); + + const mockBulkGetResponse = (objects, _found = true) => { + objects.forEach((obj) => { + obj.found = _found; + }); + const mockResponse = getMockMgetResponse(objects); + client.mget.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(mockResponse) + ); + }; + + const addToWorkspacesSuccess = async (objects, workspaces, options) => { + mockBulkGetResponse(objects); + client.bulk.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise( + getMockBulkUpdateResponse(objects, workspaces) + ) + ); + const result = await savedObjectsRepository.addToWorkspaces(objects, workspaces, options); + expect(client.mget).toHaveBeenCalledTimes(1); + expect(client.bulk).toHaveBeenCalledTimes(1); + return result; + }; + + describe('client calls', () => { + it(`should use OpenSearch get action then bulk update action`, async () => { + await addToWorkspacesSuccess(sharedObjects, [newWs1, newWs2]); + }); + + it(`_source_includes should have workspaces`, async () => { + await addToWorkspacesSuccess(sharedObjects, [newWs1, newWs2]); + expect(client.bulk).toHaveBeenCalledWith( + expect.objectContaining({ _source_includes: ['workspaces'] }), + expect.anything() + ); + }); + + it(`defaults to a refresh setting of wait_for`, async () => { + await addToWorkspacesSuccess(sharedObjects, [newWs1, newWs2]); + expect(client.bulk).toHaveBeenCalledWith( + expect.objectContaining({ refresh: 'wait_for' }), + expect.anything() + ); + }); + }); + + describe('errors', () => { + const expectNotFoundError = async (savedObjects, workspaces, options) => { + await expect( + savedObjectsRepository.addToWorkspaces(savedObjects, workspaces, options) + ).rejects.toThrowError(createGenericNotFoundError(savedObjects[0].type, id)); + }; + const expectBadRequestError = async (savedObjects, workspaces, message) => { + await expect( + savedObjectsRepository.addToWorkspaces(savedObjects, workspaces) + ).rejects.toThrowError(createBadRequestError(message)); + }; + + it(`throws when shared saved objects is empty`, async () => { + await expectBadRequestError([], [newWs1], 'shared savedObjects must not be an empty array'); + expect(client.mget).not.toHaveBeenCalled(); + }); + + it(`throws when type is invalid`, async () => { + const objects = [ + { + type: 'unknownType', + id: id, + workspace: [currentWs1], + }, + ]; + await expectNotFoundError(objects, [newWs1]); + expect(client.mget).not.toHaveBeenCalled(); + expect(client.bulk).not.toHaveBeenCalled(); + }); + + it(`throws when type is hidden`, async () => { + const objects = [ + { + type: HIDDEN_TYPE, + id: id, + workspace: [currentWs1], + }, + ]; + await expectNotFoundError(objects, [newWs1]); + expect(client.mget).not.toHaveBeenCalled(); + expect(client.bulk).not.toHaveBeenCalled(); + }); + + it(`throws when workspaces is an empty array`, async () => { + const test = async (workspaces) => { + const message = 'workspaces must be a non-empty array of strings'; + await expectBadRequestError(sharedObjects, workspaces, message); + expect(client.mget).not.toHaveBeenCalled(); + }; + await test([]); + }); + + it(`throws when OpenSearch is unable to find the document during mget`, async () => { + mockBulkGetResponse(sharedObjects, false); + await expectNotFoundError(sharedObjects, [newWs1, newWs2]); + expect(client.mget).toHaveBeenCalledTimes(1); + }); + + it(`throws when the document exists, but not in this workspace`, async () => { + mockBulkGetResponse(sharedObjects); + await expectNotFoundError(sharedObjects, [newWs1, newWs1], { + workspaces: 'some-other-workspace', + }); + expect(client.mget).toHaveBeenCalledTimes(1); + }); + + it(`throws when OpenSearch is unable to find the document during bulk update`, async () => { + const newWorkspaces = [newWs1, newWs2]; + mockBulkGetResponse(sharedObjects); + client.bulk.mockResolvedValue( + opensearchClientMock.createSuccessTransportRequestPromise( + getMockBulkUpdateResponse(sharedObjects, newWorkspaces, true) + ) + ); + await expectBadRequestError( + sharedObjects, + newWorkspaces, + `Add to workspace failed with: ${type}:${id}: document missing` + ); + expect(client.mget).toHaveBeenCalledTimes(1); + expect(client.bulk).toHaveBeenCalledTimes(1); + }); + }); + + describe('returns', () => { + it(`returns all existing and new workspaces on success`, async () => { + const result = await addToWorkspacesSuccess(sharedObjects, [newWs1, newWs2]); + result.forEach((ret) => { + expect(ret.workspaces).toEqual([currentWs1, newWs1, newWs2]); + }); + }); + + it(`succeeds when adding existing workspaces`, async () => { + const result = await addToWorkspacesSuccess(sharedObjects, [currentWs1]); + result.forEach((ret) => { + expect(ret.workspaces).toEqual([currentWs1]); + }); + }); + }); + }); + describe('#bulkCreate', () => { const obj1 = { type: 'config', @@ -2218,29 +2402,33 @@ describe('SavedObjectsRepository', () => { const mockGet = async (type, id, options) => { const mockGetResponse = getMockGetResponse({ type, id }, options?.namespace, workspaces); - client.get.mockResolvedValue( + client.get.mockResolvedValueOnce( opensearchClientMock.createSuccessTransportRequestPromise(mockGetResponse) ); }; const deleteSuccess = async (type, id, options) => { - mockGet(type, id, options); + if (registry.isMultiNamespace(type)) { + mockGet(type, id, options); + } client.delete.mockResolvedValueOnce( opensearchClientMock.createSuccessTransportRequestPromise({ result: 'deleted' }) ); const result = await savedObjectsRepository.delete(type, id, options); - expect(client.get).toHaveBeenCalledTimes(registry.isMultiNamespace(type) ? 2 : 1); + expect(client.get).toHaveBeenCalledTimes(registry.isMultiNamespace(type) ? 1 : 0); return result; }; describe('client calls', () => { it(`should use the OpenSearch delete action when not using a multi-namespace type`, async () => { await deleteSuccess(type, id); + expect(client.get).not.toHaveBeenCalled(); expect(client.delete).toHaveBeenCalledTimes(1); }); it(`should use OpenSearch get action then delete action when using a multi-namespace type`, async () => { await deleteSuccess(MULTI_NAMESPACE_TYPE, id); + expect(client.get).toHaveBeenCalledTimes(1); expect(client.delete).toHaveBeenCalledTimes(1); }); @@ -2296,7 +2484,6 @@ describe('SavedObjectsRepository', () => { ); client.delete.mockClear(); - client.get.mockClear(); await deleteSuccess(MULTI_NAMESPACE_TYPE, id, { namespace }); expect(client.delete).toHaveBeenCalledWith( expect.objectContaining({ id: `${MULTI_NAMESPACE_TYPE}:${id}` }), @@ -2367,25 +2554,6 @@ describe('SavedObjectsRepository', () => { expect(client.get).toHaveBeenCalledTimes(1); }); - it(`throws when the document has multiple workspaces and the force option is not enabled`, async () => { - const workspaces = ['foo-workspace', 'bar-workspace']; - const response = getMockGetResponse({ - type: NAMESPACE_AGNOSTIC_TYPE, - id, - namespace, - workspaces, - }); - client.get.mockResolvedValueOnce( - opensearchClientMock.createSuccessTransportRequestPromise(response) - ); - await expect( - savedObjectsRepository.delete(NAMESPACE_AGNOSTIC_TYPE, id, { namespace }) - ).rejects.toThrowError( - 'Unable to delete saved object that exists in multiple workspaces, use the `force` option to delete it anyway' - ); - expect(client.get).toHaveBeenCalledTimes(1); - }); - it(`throws when the type is multi-namespace and the document has all namespaces and the force option is not enabled`, async () => { const response = getMockGetResponse({ type: MULTI_NAMESPACE_TYPE, id, namespace }); response._source.namespaces = ['*']; @@ -2401,7 +2569,6 @@ describe('SavedObjectsRepository', () => { }); it(`throws when OpenSearch is unable to find the document during delete`, async () => { - mockGet(type, id); client.delete.mockResolvedValueOnce( opensearchClientMock.createSuccessTransportRequestPromise({ result: 'not_found' }) ); @@ -2410,7 +2577,6 @@ describe('SavedObjectsRepository', () => { }); it(`throws when OpenSearch is unable to find the index during delete`, async () => { - mockGet(type, id); client.delete.mockResolvedValueOnce( opensearchClientMock.createSuccessTransportRequestPromise({ error: { type: 'index_not_found_exception' }, @@ -2421,7 +2587,6 @@ describe('SavedObjectsRepository', () => { }); it(`throws when OpenSearch returns an unexpected response`, async () => { - mockGet(type, id); client.delete.mockResolvedValueOnce( opensearchClientMock.createSuccessTransportRequestPromise({ result: 'something unexpected', @@ -2520,6 +2685,418 @@ describe('SavedObjectsRepository', () => { }); }); + describe('#deleteByWorkspace', () => { + const workspace = 'bar-workspace'; + const mockUpdateResults = { + took: 15, + timed_out: false, + total: 3, + updated: 2, + deleted: 1, + batches: 1, + version_conflicts: 0, + noops: 0, + retries: { bulk: 0, search: 0 }, + throttled_millis: 0, + requests_per_second: -1.0, + throttled_until_millis: 0, + failures: [], + }; + + const deleteByWorkspaceSuccess = async (workspace, options) => { + client.updateByQuery.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(mockUpdateResults) + ); + const result = await savedObjectsRepository.deleteByWorkspace(workspace, options); + expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledTimes(1); + expect(client.updateByQuery).toHaveBeenCalledTimes(1); + return result; + }; + + describe('client calls', () => { + it(`should use the OpenSearch updateByQuery action`, async () => { + await deleteByWorkspaceSuccess(workspace); + expect(client.updateByQuery).toHaveBeenCalledTimes(1); + }); + + it(`should use all indices for all types`, async () => { + await deleteByWorkspaceSuccess(workspace); + expect(client.updateByQuery).toHaveBeenCalledWith( + expect.objectContaining({ index: ['.opensearch_dashboards_test', 'custom'] }), + expect.anything() + ); + }); + }); + + describe('errors', () => { + it(`throws when workspace is not a string or is '*'`, async () => { + const test = async (workspace) => { + await expect(savedObjectsRepository.deleteByWorkspace(workspace)).rejects.toThrowError( + `workspace is required, and must be a string that is not equal to '*'` + ); + expect(client.updateByQuery).not.toHaveBeenCalled(); + }; + await test(undefined); + await test(null); + await test(['foo-workspace']); + await test(123); + await test(true); + await test(ALL_NAMESPACES_STRING); + }); + }); + + describe('returns', () => { + it(`returns the query results on success`, async () => { + const result = await deleteByWorkspaceSuccess(workspace); + expect(result).toEqual(mockUpdateResults); + }); + }); + + describe('search dsl', () => { + it(`constructs a query that have workspace as search critieria`, async () => { + await deleteByWorkspaceSuccess(workspace); + const allTypes = registry.getAllTypes().map((type) => type.name); + expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith(mappings, registry, { + workspaces: [workspace], + type: allTypes, + }); + }); + }); + }); + + describe('#deleteFromWorkspace', () => { + const id = 'fake-id'; + const type = 'dashboard'; + + const mockGetResponse = (type, id, workspaces) => { + // mock a document that exists in two namespaces + const mockResponse = getMockGetResponse({ type, id, workspaces }); + client.get.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(mockResponse) + ); + }; + + const deleteFromWorkspacesSuccess = async ( + type, + id, + workspaces, + currentWorkspaces, + options + ) => { + mockGetResponse(type, id, currentWorkspaces); + client.delete.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise({ + _id: `${type}:${id}`, + ...mockVersionProps, + result: 'deleted', + }) + ); + client.update.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise({ + _id: `${type}:${id}`, + ...mockVersionProps, + result: 'updated', + }) + ); + + return await savedObjectsRepository.deleteFromWorkspaces(type, id, workspaces, options); + }; + + describe('client calls', () => { + describe('delete action', () => { + const deleteFromWorkspacesSuccessDelete = async (expectFn, options, _type = type) => { + const test = async (workspaces) => { + await deleteFromWorkspacesSuccess(_type, id, workspaces, workspaces, options); + expectFn(); + client.delete.mockClear(); + client.get.mockClear(); + }; + await test(['foo-workspace']); + await test(['foo-workspace', 'bar-workspace']); + }; + + it(`should use OpenSearch get action then delete action if the object has no workspaces remaining`, async () => { + const expectFn = () => { + expect(client.delete).toHaveBeenCalledTimes(1); + expect(client.get).toHaveBeenCalledTimes(1); + }; + await deleteFromWorkspacesSuccessDelete(expectFn); + }); + + it(`formats the OpenSearch requests`, async () => { + const expectFn = () => { + expect(client.delete).toHaveBeenCalledWith( + expect.objectContaining({ + id: `${type}:${id}`, + }), + expect.anything() + ); + + const versionProperties = { + if_seq_no: mockVersionProps._seq_no, + if_primary_term: mockVersionProps._primary_term, + }; + expect(client.delete).toHaveBeenCalledWith( + expect.objectContaining({ + id: `${type}:${id}`, + ...versionProperties, + }), + expect.anything() + ); + }; + await deleteFromWorkspacesSuccessDelete(expectFn); + }); + + it(`defaults to a refresh setting of wait_for`, async () => { + await deleteFromWorkspacesSuccessDelete(() => + expect(client.delete).toHaveBeenCalledWith( + expect.objectContaining({ + refresh: 'wait_for', + }), + expect.anything() + ) + ); + }); + + it(`should use default index`, async () => { + const expectFn = () => + expect(client.delete).toHaveBeenCalledWith( + expect.objectContaining({ index: '.opensearch_dashboards_test' }), + expect.anything() + ); + await deleteFromWorkspacesSuccessDelete(expectFn); + }); + + it(`should use custom index`, async () => { + const expectFn = () => + expect(client.delete).toHaveBeenCalledWith( + expect.objectContaining({ index: 'custom' }), + expect.anything() + ); + await deleteFromWorkspacesSuccessDelete(expectFn, {}, CUSTOM_INDEX_TYPE); + }); + }); + + describe('update action', () => { + const deleteFromWorkspacesSuccessUpdate = async (expectFn, options, _type = type) => { + const test = async (remaining) => { + const deleteWorkspace = 'deleted-workspace'; + const currentWorkspaces = [deleteWorkspace].concat(remaining); + await deleteFromWorkspacesSuccess( + _type, + id, + [deleteWorkspace], + currentWorkspaces, + options + ); + expectFn(); + client.get.mockClear(); + client.update.mockClear(); + }; + await test(['foo-workspace']); + await test(['foo-workspace', 'bar-workspace']); + }; + + it(`should use OpenSearch get action then update action if the object has one or more workspace remaining`, async () => { + const expectFn = () => { + expect(client.update).toHaveBeenCalledTimes(1); + expect(client.get).toHaveBeenCalledTimes(1); + }; + await deleteFromWorkspacesSuccessUpdate(expectFn); + }); + + it(`formats the OpenSearch requests`, async () => { + let ctr = 0; + const expectFn = () => { + expect(client.update).toHaveBeenCalledWith( + expect.objectContaining({ + id: `${type}:${id}`, + }), + expect.anything() + ); + const workspaces = ctr++ === 0 ? ['foo-workspace'] : ['foo-workspace', 'bar-workspace']; + const versionProperties = { + if_seq_no: mockVersionProps._seq_no, + if_primary_term: mockVersionProps._primary_term, + }; + expect(client.update).toHaveBeenCalledWith( + expect.objectContaining({ + id: `${type}:${id}`, + ...versionProperties, + body: { doc: { ...mockTimestampFields, workspaces: workspaces } }, + }), + expect.anything() + ); + }; + await deleteFromWorkspacesSuccessUpdate(expectFn); + }); + + it(`defaults to a refresh setting of wait_for`, async () => { + const expectFn = () => + expect(client.update).toHaveBeenCalledWith( + expect.objectContaining({ + refresh: 'wait_for', + }), + expect.anything() + ); + await deleteFromWorkspacesSuccessUpdate(expectFn); + }); + + it(`should use default index`, async () => { + const expectFn = () => + expect(client.update).toHaveBeenCalledWith( + expect.objectContaining({ index: '.opensearch_dashboards_test' }), + expect.anything() + ); + await deleteFromWorkspacesSuccessUpdate(expectFn); + }); + + it(`should use custom index`, async () => { + const expectFn = () => + expect(client.update).toHaveBeenCalledWith( + expect.objectContaining({ index: 'custom' }), + expect.anything() + ); + await deleteFromWorkspacesSuccessUpdate(expectFn, {}, CUSTOM_INDEX_TYPE); + }); + }); + }); + + describe('errors', () => { + const expectNotFoundError = async (type, id, workspaces, options) => { + await expect( + savedObjectsRepository.deleteFromWorkspaces(type, id, workspaces, options) + ).rejects.toThrowError(createGenericNotFoundError(type, id)); + }; + const expectBadRequestError = async (type, id, workspaces, message) => { + await expect( + savedObjectsRepository.deleteFromWorkspaces(type, id, workspaces) + ).rejects.toThrowError(createBadRequestError(message)); + }; + + it(`throws when type is invalid`, async () => { + await expectNotFoundError('unknownType', id, ['foo', 'bar']); + expect(client.delete).not.toHaveBeenCalled(); + expect(client.update).not.toHaveBeenCalled(); + }); + + it(`throws when workspaces is an empty array`, async () => { + const test = async (workspaces) => { + const message = 'workspaces must be a non-empty array of strings'; + await expectBadRequestError(type, id, workspaces, message); + expect(client.delete).not.toHaveBeenCalled(); + expect(client.update).not.toHaveBeenCalled(); + }; + await test([]); + }); + + it(`throws when OpenSearch is unable to find the document during get`, async () => { + client.get.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise({ found: false }) + ); + await expectNotFoundError(type, id, ['foo', 'bar']); + expect(client.get).toHaveBeenCalledTimes(1); + expect(client.delete).not.toHaveBeenCalled(); + expect(client.update).not.toHaveBeenCalled(); + }); + + it(`throws when OpenSearch is unable to find the index during get`, async () => { + client.get.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }) + ); + await expectNotFoundError(type, id, ['foo', 'bar']); + expect(client.get).toHaveBeenCalledTimes(1); + }); + + it(`throws when OpenSearch is unable to find the document during delete`, async () => { + mockGetResponse(type, id, ['foo']); + client.delete.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise({ result: 'not_found' }) + ); + await expectNotFoundError(type, id, ['foo']); + expect(client.get).toHaveBeenCalledTimes(1); + expect(client.delete).toHaveBeenCalledTimes(1); + }); + + it(`throws when OpenSearch is unable to find the index during delete`, async () => { + mockGetResponse(type, id, ['foo']); + client.delete.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise({ + error: { type: 'index_not_found_exception' }, + }) + ); + await expectNotFoundError(type, id, ['foo']); + expect(client.get).toHaveBeenCalledTimes(1); + expect(client.delete).toHaveBeenCalledTimes(1); + }); + + it(`throws when OpenSearch returns an unexpected response`, async () => { + mockGetResponse(type, id, ['foo']); + client.delete.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise({ + result: 'something unexpected', + }) + ); + await expect( + savedObjectsRepository.deleteFromWorkspaces(type, id, ['foo']) + ).rejects.toThrowError('Unexpected OpenSearch DELETE response'); + expect(client.get).toHaveBeenCalledTimes(1); + expect(client.delete).toHaveBeenCalledTimes(1); + }); + + it(`throws when OpenSearch is unable to find the document during update`, async () => { + mockGetResponse(type, id, ['foo', 'bar']); + client.update.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }) + ); + await expectNotFoundError(type, id, ['foo']); + expect(client.get).toHaveBeenCalledTimes(1); + expect(client.update).toHaveBeenCalledTimes(1); + }); + }); + + describe('returns', () => { + it(`returns an empty workspaces array on success (delete)`, async () => { + const test = async (workspaces) => { + const result = await deleteFromWorkspacesSuccess(type, id, workspaces, workspaces); + expect(result).toEqual({ workspaces: [] }); + client.delete.mockClear(); + }; + await test(['foo']); + await test(['foo', 'bar']); + }); + + it(`returns remaining workspaces on success (update)`, async () => { + const test = async (remaining) => { + const deletedWorkspace = 'delete-workspace'; + const currentNamespaces = [deletedWorkspace].concat(remaining); + const result = await deleteFromWorkspacesSuccess( + type, + id, + [deletedWorkspace], + currentNamespaces + ); + expect(result).toEqual({ workspaces: remaining }); + client.delete.mockClear(); + }; + await test(['foo']); + await test(['foo', 'bar']); + }); + + it(`succeeds when the document doesn't exist in all of the targeted workspaces`, async () => { + const workspaceToRemove = ['foo']; + const currentWorkspaces = ['bar']; + const result = await deleteFromWorkspacesSuccess( + type, + id, + workspaceToRemove, + currentWorkspaces + ); + expect(result).toEqual({ workspaces: currentWorkspaces }); + }); + }); + }); + describe('#find', () => { const generateSearchResults = (namespace) => { return { diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 88e296a00039..a7d366b1c7e7 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -723,14 +723,6 @@ export class SavedObjectsRepository { } } - const obj = await this.get(type, id, { namespace }); - const existingWorkspace = obj.workspaces || []; - if (!force && existingWorkspace.length > 1) { - throw SavedObjectsErrorHelpers.createBadRequestError( - 'Unable to delete saved object that exists in multiple workspaces, use the `force` option to delete it anyway' - ); - } - const { body, statusCode } = await this.client.delete( { id: rawId, @@ -812,7 +804,7 @@ export class SavedObjectsRepository { } /** - * Deletes all objects from the provided workspace. + * Deletes all objects from the provided workspace. It used when delete a workspace. * * @param {string} workspace * @param options SavedObjectsDeleteByWorkspaceOptions @@ -822,7 +814,7 @@ export class SavedObjectsRepository { workspace: string, options: SavedObjectsDeleteByWorkspaceOptions = {} ): Promise { - if (!workspace || workspace === '*') { + if (!workspace || typeof workspace !== 'string' || workspace === '*') { throw new TypeError(`workspace is required, and must be a string that is not equal to '*'`); } @@ -1406,6 +1398,13 @@ export class SavedObjectsRepository { } } + /** + * Adds one or more workspaces to a given saved objects. This method and + * [`deleteFromWorkspaces`]{@link SavedObjectsRepository.deleteFromWorkspaces} are the only ways to change which workspace + * saved object is shared to. + * @param savedObjects saved objects that will shared to new workspaces + * @param workspaces new workspaces + */ async addToWorkspaces( savedObjects: SavedObjectsShareObjects[], workspaces: string[], @@ -1417,20 +1416,6 @@ export class SavedObjectsRepository { ); } - // saved objects must exist in specified workspace - if (options.workspaces) { - const invalidObjects = savedObjects.filter((obj) => { - if (obj.workspaces && obj.workspaces.length > 0) { - return intersection(obj.workspaces, options.workspaces).length === 0; - } - return false; - }); - if (invalidObjects && invalidObjects.length > 0) { - const [savedObj] = invalidObjects; - throw SavedObjectsErrorHelpers.createConflictError(savedObj.type, savedObj.id); - } - } - savedObjects.forEach(({ type, id }) => { if (!this._allowedTypes.includes(type)) { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); @@ -1446,6 +1431,25 @@ export class SavedObjectsRepository { const { refresh = DEFAULT_REFRESH_SETTING } = options; const savedObjectsBulkResponse = await this.bulkGet(savedObjects); + const errorObject = savedObjectsBulkResponse.saved_objects.find((obj) => !!obj.error); + if (errorObject) { + throw SavedObjectsErrorHelpers.decorateBadRequestError(new Error(errorObject.error?.message)); + } + + // saved objects must exist in specified workspace + if (options.workspaces) { + const invalidObjects = savedObjectsBulkResponse.saved_objects.filter((obj) => { + if (obj.workspaces && obj.workspaces.length > 0) { + return intersection(obj.workspaces, options.workspaces).length === 0; + } + return true; + }); + if (invalidObjects && invalidObjects.length > 0) { + const [savedObj] = invalidObjects; + throw SavedObjectsErrorHelpers.createGenericNotFoundError(savedObj.type, savedObj.id); + } + } + const docs = savedObjectsBulkResponse.saved_objects.map((obj) => { const { type, id } = obj; const rawId = this._serializer.generateRawId(undefined, type, id); diff --git a/src/core/server/saved_objects/service/saved_objects_client.mock.ts b/src/core/server/saved_objects/service/saved_objects_client.mock.ts index d39f101b18e2..763cb1bb1e92 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.mock.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.mock.ts @@ -45,6 +45,7 @@ const create = () => update: jest.fn(), addToNamespaces: jest.fn(), deleteFromNamespaces: jest.fn(), + addToWorkspaces: jest.fn(), } as unknown) as jest.Mocked); export const savedObjectsClientMock = { create }; 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 c94f3e980b31..4f430d7f7134 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -172,7 +172,7 @@ export interface SavedObjectsCheckConflictsResponse { }>; } -export type SavedObjectsShareObjects = Pick; +export type SavedObjectsShareObjects = Pick; /** * @@ -200,10 +200,10 @@ export interface SavedObjectsAddToNamespacesOptions extends SavedObjectsBaseOpti refresh?: MutatingOperationRefreshSetting; } -export type SavedObjectsAddToWorkspacesOptions = Pick< - SavedObjectsUpdateOptions, - 'refresh' | 'workspaces' ->; +export interface SavedObjectsAddToWorkspacesOptions extends SavedObjectsBaseOptions { + /** The OpenSearch Refresh setting for this operation */ + refresh?: MutatingOperationRefreshSetting; +} /** * @@ -228,12 +228,12 @@ export interface SavedObjectsDeleteFromNamespacesOptions extends SavedObjectsBas refresh?: MutatingOperationRefreshSetting; } -export interface SavedObjectsDeleteFromWorkspacesOptions { +export interface SavedObjectsDeleteFromWorkspacesOptions extends SavedObjectsBaseOptions { /** The OpenSearch Refresh setting for this operation */ refresh?: MutatingOperationRefreshSetting; } -export interface SavedObjectsDeleteByWorkspaceOptions { +export interface SavedObjectsDeleteByWorkspaceOptions extends SavedObjectsBaseOptions { /** The OpenSearch supports only boolean flag for this operation */ refresh?: boolean; } @@ -265,7 +265,7 @@ export interface SavedObjectsBulkUpdateOptions extends SavedObjectsBaseOptions { * * @public */ -export interface SavedObjectsDeleteOptions extends Omit { +export interface SavedObjectsDeleteOptions extends SavedObjectsBaseOptions { /** The OpenSearch Refresh setting for this operation */ refresh?: MutatingOperationRefreshSetting; /** Force deletion of an object that exists in multiple namespaces */