From 29a09b0675cdb3936baf206c1fb93253fda0fdfd Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 30 Apr 2024 00:23:15 +0800 Subject: [PATCH] [Workspace] Fix: optimization on handling invalid workspace id in workspace_ui_settings wrapper (#6669) * fix: invalid workspace id Signed-off-by: SuZhou-Joe * Changeset file for PR #6669 created/updated * Update src/plugins/workspace/server/saved_objects/workspace_ui_settings_client_wrapper.ts Co-authored-by: Yulong Ruan Signed-off-by: SuZhou-Joe * feat: update error message Signed-off-by: SuZhou-Joe * fix: type error Signed-off-by: SuZhou-Joe * feat: remove useless code Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Yulong Ruan --- changelogs/fragments/6669.yml | 2 + src/plugins/workspace/server/plugin.ts | 2 +- ...rkspace_ui_settings_client_wrapper.test.ts | 41 ++++++++++++++++++- .../workspace_ui_settings_client_wrapper.ts | 20 ++++++--- 4 files changed, 58 insertions(+), 7 deletions(-) create mode 100644 changelogs/fragments/6669.yml diff --git a/changelogs/fragments/6669.yml b/changelogs/fragments/6669.yml new file mode 100644 index 000000000000..1e68c7b5477a --- /dev/null +++ b/changelogs/fragments/6669.yml @@ -0,0 +1,2 @@ +fix: +- Optimization on handling invalid workspace id in workspace_ui_settings wrapper ([#6669](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6669)) \ No newline at end of file diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 7956d7928a9e..b53a00b39afa 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -94,7 +94,7 @@ export class WorkspacePlugin implements Plugin { const getClientMock = jest.fn().mockReturnValue(clientMock); const requestHandlerContext = coreMock.createRequestHandlerContext(); const requestMock = httpServerMock.createOpenSearchDashboardsRequest(); + const logger = loggerMock.create(); clientMock.get.mockImplementation(async (type, id) => { if (type === 'config') { @@ -43,7 +45,7 @@ describe('WorkspaceUiSettingsClientWrapper', () => { return Promise.reject(); }); - const wrapper = new WorkspaceUiSettingsClientWrapper(); + const wrapper = new WorkspaceUiSettingsClientWrapper(logger); wrapper.setScopedClient(getClientMock); return { @@ -53,6 +55,7 @@ describe('WorkspaceUiSettingsClientWrapper', () => { typeRegistry: requestHandlerContext.savedObjects.typeRegistry, }), clientMock, + logger, }; }; @@ -136,4 +139,40 @@ describe('WorkspaceUiSettingsClientWrapper', () => { {} ); }); + + it('should not throw error if the workspace id is not valid', async () => { + const invalidWorkspaceId = 'invalid-workspace-id'; + // Currently in a workspace + jest + .spyOn(utils, 'getWorkspaceState') + .mockReturnValue({ requestWorkspaceId: invalidWorkspaceId }); + + const { wrappedClient, clientMock, logger } = createWrappedClient(); + clientMock.get.mockImplementation(async (type, id) => { + if (type === 'config') { + return Promise.resolve({ + id, + references: [], + type: 'config', + attributes: { + defaultIndex: 'default-index-global', + }, + }); + } + return Promise.reject('not found'); + }); + + const config = await wrappedClient.get('config', '3.0.0'); + expect(config).toEqual({ + attributes: { + defaultIndex: 'default-index-global', + }, + id: '3.0.0', + references: [], + type: 'config', + }); + expect(logger.error).toBeCalledWith( + `Unable to get workspaceObject with id: ${invalidWorkspaceId}` + ); + }); }); diff --git a/src/plugins/workspace/server/saved_objects/workspace_ui_settings_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_ui_settings_client_wrapper.ts index b2d8ebb80323..9cc860ec903e 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_ui_settings_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_ui_settings_client_wrapper.ts @@ -17,12 +17,14 @@ import { SavedObjectsClientContract, } from '../../../../core/server'; import { WORKSPACE_UI_SETTINGS_CLIENT_WRAPPER_ID } from '../../common/constants'; +import { Logger } from '../../../../core/server'; /** * This saved object client wrapper offers methods to get and update UI settings considering * the context of the current workspace. */ export class WorkspaceUiSettingsClientWrapper { + constructor(private readonly logger: Logger) {} private getScopedClient?: SavedObjectsServiceStart['getScopedClient']; /** @@ -61,13 +63,19 @@ export class WorkspaceUiSettingsClientWrapper { options ); - const workspaceObject = await this.getWorkspaceTypeEnabledClient( - wrapperOptions.request - ).get(WORKSPACE_TYPE, requestWorkspaceId); + let workspaceObject: SavedObject | null = null; + + try { + workspaceObject = await this.getWorkspaceTypeEnabledClient(wrapperOptions.request).get< + WorkspaceAttribute + >(WORKSPACE_TYPE, requestWorkspaceId); + } catch (e) { + this.logger.error(`Unable to get workspaceObject with id: ${requestWorkspaceId}`); + } configObject.attributes = { ...configObject.attributes, - ...workspaceObject.attributes.uiSettings, + ...(workspaceObject ? workspaceObject.attributes.uiSettings : {}), }; return configObject as SavedObject; @@ -111,7 +119,9 @@ export class WorkspaceUiSettingsClientWrapper { options ); - configObject.attributes = workspaceUpdateResult.attributes.uiSettings; + if (workspaceUpdateResult.attributes.uiSettings) { + configObject.attributes = workspaceUpdateResult.attributes.uiSettings; + } return configObject as SavedObjectsUpdateResponse; }