Skip to content

Commit

Permalink
[Workspace] Delete saved objects by workspace (opensearch-project#6013)
Browse files Browse the repository at this point in the history
* [API] Delete saved objects by workspace (opensearch-project#216)

* Delete saved objects by workspace

Signed-off-by: Hailong Cui <ihailong@amazon.com>

fix osd boostrap

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* add unit test

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* fix can't delete workspace due to invalid permission

Signed-off-by: Hailong Cui <ihailong@amazon.com>

---------

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* update test case

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* Add change log

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* update change log format

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* update method comments to make it more clear

Signed-off-by: Hailong Cui <ihailong@amazon.com>

---------

Signed-off-by: Hailong Cui <ihailong@amazon.com>
  • Loading branch information
Hailong-am authored Mar 11, 2024
1 parent 8f0885d commit 735424b
Show file tree
Hide file tree
Showing 10 changed files with 230 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Multiple Datasource] Add Vega support to MDS by specifying a data source name in the Vega spec ([#5975](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5975))
- [Workspace] Consume workspace id in saved object client ([#6014](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6014))

- [Workspace] Add delete saved objects by workspace functionality([#6013](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6013))

### 🐛 Bug Fixes

- [BUG][Discover] Allow saved sort from search embeddable to load in Dashboard ([#5934](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5934))
Expand Down
1 change: 1 addition & 0 deletions src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ export {
exportSavedObjectsToStream,
importSavedObjectsFromStream,
resolveSavedObjectsImportErrors,
SavedObjectsDeleteByWorkspaceOptions,
} from './saved_objects';

export {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const create = (): jest.Mocked<ISavedObjectsRepository> => ({
deleteFromNamespaces: jest.fn(),
deleteByNamespace: jest.fn(),
incrementCounter: jest.fn(),
deleteByWorkspace: jest.fn(),
});

export const savedObjectsRepositoryMock = { create };
79 changes: 79 additions & 0 deletions src/core/server/saved_objects/service/lib/repository.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2585,6 +2585,85 @@ 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('#find', () => {
const generateSearchResults = (namespace) => {
return {
Expand Down
50 changes: 50 additions & 0 deletions src/core/server/saved_objects/service/lib/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import {
SavedObjectsAddToNamespacesResponse,
SavedObjectsDeleteFromNamespacesOptions,
SavedObjectsDeleteFromNamespacesResponse,
SavedObjectsDeleteByWorkspaceOptions,
} from '../saved_objects_client';
import {
SavedObject,
Expand Down Expand Up @@ -714,6 +715,55 @@ export class SavedObjectsRepository {
return body;
}

/**
* Deletes all objects from the provided workspace.
*
* @param {string} workspace - workspace id
* @param options SavedObjectsDeleteByWorkspaceOptions
* @returns {promise} - { took, timed_out, total, deleted, batches, version_conflicts, noops, retries, failures }
*/
async deleteByWorkspace(
workspace: string,
options: SavedObjectsDeleteByWorkspaceOptions = {}
): Promise<any> {
if (!workspace || typeof workspace !== 'string' || workspace === '*') {
throw new TypeError(`workspace is required, and must be a string that is not equal to '*'`);
}

const allTypes = Object.keys(getRootPropertiesObjects(this._mappings));

const { body } = await this.client.updateByQuery(
{
index: this.getIndicesForTypes(allTypes),
refresh: options.refresh,
body: {
script: {
source: `
if (!ctx._source.containsKey('workspaces')) {
ctx.op = "delete";
} else {
ctx._source['workspaces'].removeAll(Collections.singleton(params['workspace']));
if (ctx._source['workspaces'].empty) {
ctx.op = "delete";
}
}
`,
lang: 'painless',
params: { workspace },
},
conflicts: 'proceed',
...getSearchDsl(this._mappings, this._registry, {
workspaces: [workspace],
type: allTypes,
}),
},
},
{ ignore: [404] }
);

return body;
}

/**
* @param {object} [options={}]
* @property {(string|Array<string>)} [options.type]
Expand Down
15 changes: 15 additions & 0 deletions src/core/server/saved_objects/service/saved_objects_client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
21 changes: 21 additions & 0 deletions src/core/server/saved_objects/service/saved_objects_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,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
Expand Down Expand Up @@ -446,6 +455,18 @@ export class SavedObjectsClient {
return await this._repository.deleteFromNamespaces(type, id, namespaces, options);
}

/**
* delete saved objects by workspace id
* @param workspace
* @param options
*/
deleteByWorkspace = async (
workspace: string,
options: SavedObjectsDeleteByWorkspaceOptions = {}
): Promise<any> => {
return await this._repository.deleteByWorkspace(workspace, options);
};

/**
* Bulk Updates multiple SavedObject at once
*
Expand Down
46 changes: 43 additions & 3 deletions src/plugins/workspace/server/integration_tests/routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import { WorkspaceAttribute } from 'src/core/types';
import * as osdTestServer from '../../../../core/test_helpers/osd_server';
import { WORKSPACE_TYPE } from '../../../../core/server';

const omitId = <T extends { id?: string }>(object: T): Omit<T, 'id'> => {
const { id, ...others } = object;
Expand All @@ -20,6 +21,7 @@ const testWorkspace: WorkspaceAttribute = {
describe('workspace service', () => {
let root: ReturnType<typeof osdTestServer.createRoot>;
let opensearchServer: osdTestServer.TestOpenSearchUtils;
let osd: osdTestServer.TestOpenSearchDashboardsUtils;
beforeAll(async () => {
const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({
adjustTimeout: (t: number) => jest.setTimeout(t),
Expand All @@ -28,12 +30,13 @@ describe('workspace service', () => {
workspace: {
enabled: true,
},
migrations: { skip: false },
},
},
});
opensearchServer = await startOpenSearch();
const startOSDResp = await startOpenSearchDashboards();
root = startOSDResp.root;
osd = await startOpenSearchDashboards();
root = osd.root;
});
afterAll(async () => {
await root.shutdown();
Expand All @@ -47,9 +50,13 @@ describe('workspace service', () => {
page: 1,
})
.expect(200);
const savedObjectsRepository = osd.coreStart.savedObjects.createInternalRepository([
WORKSPACE_TYPE,
]);
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
savedObjectsRepository.delete(WORKSPACE_TYPE, item.id)
)
);
});
Expand Down Expand Up @@ -119,6 +126,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);
Expand All @@ -129,6 +146,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: omitId(reservedWorkspace),
})
.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
Expand Down
1 change: 1 addition & 0 deletions src/plugins/workspace/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const workspaceAttributesSchema = schema.object({
color: schema.maybe(schema.string()),
icon: schema.maybe(schema.string()),
defaultVISTheme: schema.maybe(schema.string()),
reserved: schema.maybe(schema.boolean()),
});

export function registerRoutes({
Expand Down
18 changes: 17 additions & 1 deletion src/plugins/workspace/server/workspace_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,23 @@ export class WorkspaceClient implements IWorkspaceClientImpl {
}
public async delete(requestDetail: IRequestDetail, id: string): Promise<IResponse<boolean>> {
try {
await this.getSavedObjectClientsFromRequestDetail(requestDetail).delete(WORKSPACE_TYPE, id);
const savedObjectClient = this.getSavedObjectClientsFromRequestDetail(requestDetail);
const workspaceInDB: SavedObject<WorkspaceAttribute> = await savedObjectClient.get(
WORKSPACE_TYPE,
id
);
if (workspaceInDB.attributes.reserved) {
return {
success: false,
error: i18n.translate('workspace.deleteReservedWorkspace.errorMessage', {
defaultMessage: 'Reserved workspace {id} is not allowed to delete.',
values: { id: workspaceInDB.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,
Expand Down

0 comments on commit 735424b

Please sign in to comment.