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.

Sign up for GitHub

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

[Workspace][Feature]Setup workspace skeleton and implement basic CRUD API #5075

Merged
merged 19 commits into from
Oct 30, 2023
Merged

[Workspace][Feature]Setup workspace skeleton and implement basic CRUD API #5075

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
1148ed1
feature: setup workspace skeleton and implement basic CRUD API on wor…
Sep 6, 2023
a7c0337
feat: remove useless required plugins and logger typo
SuZhou-Joe Sep 6, 2023
9a3f713
feat: setup public side skeleton
SuZhou-Joe Sep 7, 2023
5f6f4c3
temp: add unit test
SuZhou-Joe Sep 19, 2023
0449cdd
feat: add function test for workspace CRUD routes
SuZhou-Joe Sep 20, 2023
ebc1286
feat: use saved objects client instead of internal repository
SuZhou-Joe Sep 20, 2023
16a4c66
feat: update CHANGELOG
SuZhou-Joe Sep 20, 2023
759d03c
feat: exclude permission check wrapper
SuZhou-Joe Sep 20, 2023
6e1c656
feat: add integration test
SuZhou-Joe Sep 21, 2023
090e2fe
feat: add configuration
SuZhou-Joe Sep 21, 2023
bf53f12
feat: enable workspace flag when run workspace related test
SuZhou-Joe Sep 21, 2023
d0d5882
feat: optimization according to PR comments
SuZhou-Joe Sep 27, 2023
2f73b21
feat: add JSDoc for workspace client
SuZhou-Joe Oct 4, 2023
5219fce
Update src/plugins/workspace/server/integration_tests/routes.test.ts
SuZhou-Joe Oct 17, 2023
e80221e
feat: remove hard-coded delay
SuZhou-Joe Oct 17, 2023
46238e1
feat: optimize unit test
SuZhou-Joe Oct 17, 2023
87e8f71
fix: unit test
SuZhou-Joe Oct 18, 2023
86871a5
feat: Only allow workspace CRUD APIs to modify workspace metadata.
SuZhou-Joe Oct 27, 2023
bdb93cb
feat: add integration test for new changes
SuZhou-Joe Oct 28, 2023
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
Prev Previous commit
Next Next commit
feat: optimization according to PR comments 
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
  • Loading branch information
@SuZhou-Joe
SuZhou-Joe committed Oct 17, 2023
commit d0d58821d197c3aa830a04473980718ace231cce
6 changes: 4 additions & 2 deletions src/plugins/workspace/opensearch_dashboards.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
"id": "workspace",
"version": "opensearchDashboards",
"server": true,
"ui": true,
"requiredPlugins": [],
"ui": false,
joshuarrrr marked this conversation as resolved.
Show resolved Hide resolved
"requiredPlugins": [
"savedObjects"
Copy link
Member

Choose a reason for hiding this comment

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

I think savedObjects is one I suggested. Just in case, I would suggest in a follow-up verifying all your requiredPlugins and bundles

],
"optionalPlugins": [],
"requiredBundles": []
}
1 change: 1 addition & 0 deletions src/plugins/workspace/public/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/
SuZhou-Joe marked this conversation as resolved.
Show resolved Hide resolved

import { Plugin } from '../../../core/public';

export class WorkspacePlugin implements Plugin<{}, {}, {}> {
zengyan-amazon marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
1 change: 1 addition & 0 deletions src/plugins/workspace/server/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { PluginConfigDescriptor, PluginInitializerContext } from '../../../core/server';
import { WorkspacePlugin } from './plugin';
import { configSchema } from '../config';
Expand Down
13 changes: 7 additions & 6 deletions src/plugins/workspace/server/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,21 @@
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import {
PluginInitializerContext,
CoreSetup,
Plugin,
Logger,
CoreStart,
} from '../../../core/server';
import { IWorkspaceDBImpl } from './types';
import { WorkspaceClientWithSavedObject } from './workspace_client';
import { IWorkspaceClientImpl } from './types';
import { WorkspaceClient } from './workspace_client';
import { registerRoutes } from './routes';

export class WorkspacePlugin implements Plugin<{}, {}> {
private readonly logger: Logger;
private client?: IWorkspaceDBImpl;
private client?: IWorkspaceClientImpl;

constructor(initializerContext: PluginInitializerContext) {
this.logger = initializerContext.logger.get('plugins', 'workspace');
Expand All @@ -24,14 +25,14 @@ export class WorkspacePlugin implements Plugin<{}, {}> {
public async setup(core: CoreSetup) {
this.logger.debug('Setting up Workspaces service');

this.client = new WorkspaceClientWithSavedObject(core);
this.client = new WorkspaceClient(core);
Copy link
Member

Choose a reason for hiding this comment

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

it seems the WorkspaceClient is a wrapper on top of savedObjectClient, which we usually do not do for other saved object types. Is there a specific reason we want a heavy client for workspace type?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the client now is a simple wrapper on top of savedObjectClient but in the following PRs more logic will be added into this client, and this PR is intended to setup the skeleton for them. And this client won't be too heavy as there will be only a single instance for each OSD server and we do not have any side effect inside the client.


SuZhou-Joe marked this conversation as resolved.
Show resolved Hide resolved
await this.client.setup(core);

registerRoutes({
http: core.http,
logger: this.logger,
client: this.client as IWorkspaceDBImpl,
client: this.client as IWorkspaceClientImpl,
});

return {
Expand All @@ -44,7 +45,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> {
this.client?.setSavedObjects(core.savedObjects);

return {
client: this.client as IWorkspaceDBImpl,
client: this.client as IWorkspaceClientImpl,
};
}

Expand Down
24 changes: 6 additions & 18 deletions src/plugins/workspace/server/routes/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/
import { schema } from '@osd/config-schema';

import { schema } from '@osd/config-schema';
import { CoreSetup, Logger } from '../../../../core/server';
import { IWorkspaceDBImpl } from '../types';
import { IWorkspaceClientImpl } from '../types';

const WORKSPACES_API_BASE_URL = '/api/workspaces';

Expand All @@ -23,7 +23,7 @@ export function registerRoutes({
logger,
http,
}: {
client: IWorkspaceDBImpl;
client: IWorkspaceClientImpl;
logger: Logger;
http: CoreSetup['http'];
}) {
Expand Down Expand Up @@ -78,17 +78,9 @@ export function registerRoutes({
},
id
);
if (!result.success) {
return res.ok({ body: result });
}

return res.ok({
body: {
...result,
result: {
...result.result,
},
},
body: result,
});
})
);
Expand All @@ -110,9 +102,7 @@ export function registerRoutes({
request: req,
logger,
},
{
...attributes,
}
attributes
);
return res.ok({ body: result });
})
Expand Down Expand Up @@ -140,9 +130,7 @@ export function registerRoutes({
logger,
},
id,
{
...attributes,
}
attributes
);
return res.ok({ body: result });
})
Expand Down
3 changes: 0 additions & 3 deletions src/plugins/workspace/server/saved_objects/workspace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ export const workspace: SavedObjectsType = {
description: {
type: 'text',
},
/**
* In opensearch, string[] is also mapped to text
*/
features: {
type: 'keyword',
},
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/workspace/server/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import {
Logger,
OpenSearchDashboardsRequest,
Expand All @@ -27,7 +28,7 @@ export interface IRequestDetail {
logger: Logger;
}

export interface IWorkspaceDBImpl {
export interface IWorkspaceClientImpl {
setup(dep: CoreSetup): Promise<IResponse<boolean>>;
setSavedObjects(savedObjects: SavedObjectsServiceStart): void;
create(
Expand Down
9 changes: 5 additions & 4 deletions src/plugins/workspace/server/workspace_client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { i18n } from '@osd/i18n';
import type {
SavedObject,
Expand All @@ -11,7 +12,7 @@ import type {
SavedObjectsServiceStart,
} from '../../../core/server';
import { WORKSPACE_TYPE } from '../../../core/server';
import { IWorkspaceDBImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types';
import { IWorkspaceClientImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types';
import { workspace } from './saved_objects';
import { generateRandomId } from './utils';
import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants';
Expand All @@ -22,7 +23,7 @@ const DUPLICATE_WORKSPACE_NAME_ERROR = i18n.translate('workspace.duplicate.name.
defaultMessage: 'workspace name has already been used, try with a different name',
});

export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl {
export class WorkspaceClient implements IWorkspaceClientImpl {
private setupDep: CoreSetup;
private savedObjects?: SavedObjectsServiceStart;

Expand Down Expand Up @@ -64,7 +65,7 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl {
public async create(
requestDetail: IRequestDetail,
payload: Omit<WorkspaceAttribute, 'id'>
): ReturnType<IWorkspaceDBImpl['create']> {
): ReturnType<IWorkspaceClientImpl['create']> {
try {
const attributes = payload;
const id = generateRandomId(WORKSPACE_ID_SIZE);
Expand Down Expand Up @@ -102,7 +103,7 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl {
public async list(
requestDetail: IRequestDetail,
options: WorkspaceFindOptions
): ReturnType<IWorkspaceDBImpl['list']> {
): ReturnType<IWorkspaceClientImpl['list']> {
try {
const {
saved_objects: savedObjects,
Expand Down
18 changes: 11 additions & 7 deletions test/api_integration/apis/workspace/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@

import expect from '@osd/expect';
import { WorkspaceAttribute } from 'opensearch-dashboards/server';
import { omit } from 'lodash';
import { FtrProviderContext } from '../../ftr_provider_context';

const omitId = <T extends { id?: string }>(object: T): Omit<T, 'id'> => {
const { id, ...others } = object;
return others;
};

const testWorkspace: WorkspaceAttribute = {
id: 'fake_id',
name: 'test_workspace',
Expand Down Expand Up @@ -47,7 +51,7 @@ export default function ({ getService }: FtrProviderContext) {
const result: any = await supertest
.post(`/api/workspaces`)
.send({
attributes: omit(testWorkspace, 'id'),
attributes: omitId(testWorkspace),
})
.set('osd-xsrf', 'opensearch-dashboards')
.expect(200);
Expand All @@ -59,7 +63,7 @@ export default function ({ getService }: FtrProviderContext) {
const result = await supertest
.post(`/api/workspaces`)
.send({
attributes: omit(testWorkspace, 'id'),
attributes: omitId(testWorkspace),
})
.set('osd-xsrf', 'opensearch-dashboards')
.expect(200);
Expand All @@ -71,7 +75,7 @@ export default function ({ getService }: FtrProviderContext) {
const result: any = await supertest
.post(`/api/workspaces`)
.send({
attributes: omit(testWorkspace, 'id'),
attributes: omitId(testWorkspace),
})
.set('osd-xsrf', 'opensearch-dashboards')
.expect(200);
Expand All @@ -80,7 +84,7 @@ export default function ({ getService }: FtrProviderContext) {
.put(`/api/workspaces/${result.body.result.id}`)
.send({
attributes: {
...omit(testWorkspace, 'id'),
...omitId(testWorkspace),
name: 'updated',
},
})
Expand All @@ -96,7 +100,7 @@ export default function ({ getService }: FtrProviderContext) {
const result: any = await supertest
.post(`/api/workspaces`)
.send({
attributes: omit(testWorkspace, 'id'),
attributes: omitId(testWorkspace),
})
.set('osd-xsrf', 'opensearch-dashboards')
.expect(200);
Expand All @@ -114,7 +118,7 @@ export default function ({ getService }: FtrProviderContext) {
await supertest
.post(`/api/workspaces`)
.send({
attributes: omit(testWorkspace, 'id'),
attributes: omitId(testWorkspace),
})
.set('osd-xsrf', 'opensearch-dashboards')
.expect(200);
Expand Down