Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API] Delete saved objects by workspace #216

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,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 @@ -2637,6 +2637,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 @@ -796,6 +797,55 @@ export class SavedObjectsRepository {
return body;
}

/**
* Deletes all objects from the provided workspace. It used when deleting a workspace.
*
* @param {string} workspace
* @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')) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: will the results contain object without workspaces field?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base on current implementation, it won't.

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
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 @@ -277,6 +277,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 @@ -433,6 +442,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
40 changes: 39 additions & 1 deletion src/plugins/workspace/server/integration_tests/routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import { WorkspaceAttribute } from 'src/core/types';
import { omit } from 'lodash';
import * as osdTestServer from '../../../../core/test_helpers/osd_server';
import { WORKSPACE_TYPE } from '../../../../core/server';

const testWorkspace: WorkspaceAttribute = {
id: 'fake_id',
Expand All @@ -24,6 +25,7 @@ describe('workspace service', () => {
workspace: {
enabled: true,
},
migrations: { skip: false },
},
},
});
Expand All @@ -45,7 +47,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)
)
);
});
Expand Down Expand Up @@ -115,6 +120,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 @@ -125,6 +140,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
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 @@ -182,7 +182,23 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl {
}
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
Loading