Skip to content

Commit

Permalink
[Workspace] Fix: optimization on handling invalid workspace id in wor…
Browse files Browse the repository at this point in the history
…kspace_ui_settings wrapper (#6669)

* fix: invalid workspace id

Signed-off-by: SuZhou-Joe <[email protected]>

* 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 <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>

* feat: update error message

Signed-off-by: SuZhou-Joe <[email protected]>

* fix: type error

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: remove useless code

Signed-off-by: SuZhou-Joe <[email protected]>

---------

Signed-off-by: SuZhou-Joe <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Co-authored-by: Yulong Ruan <[email protected]>
(cherry picked from commit 29a09b0)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
3 people committed Apr 29, 2024
1 parent fcc06de commit 2953968
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 7 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/6669.yml
Original file line number Diff line number Diff line change
@@ -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))
2 changes: 1 addition & 1 deletion src/plugins/workspace/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export class WorkspacePlugin implements Plugin<WorkspacePluginSetup, WorkspacePl
);
this.proxyWorkspaceTrafficToRealHandler(core);

const workspaceUiSettingsClientWrapper = new WorkspaceUiSettingsClientWrapper();
const workspaceUiSettingsClientWrapper = new WorkspaceUiSettingsClientWrapper(this.logger);
this.workspaceUiSettingsClientWrapper = workspaceUiSettingsClientWrapper;
core.savedObjects.addClientWrapper(
PRIORITY_FOR_WORKSPACE_UI_SETTINGS_WRAPPER,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { loggerMock } from '@osd/logging/target/mocks';
import { httpServerMock, savedObjectsClientMock, coreMock } from '../../../../core/server/mocks';
import { WorkspaceUiSettingsClientWrapper } from './workspace_ui_settings_client_wrapper';
import { WORKSPACE_TYPE } from '../../../../core/server';
Expand All @@ -17,6 +18,7 @@ describe('WorkspaceUiSettingsClientWrapper', () => {
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') {
Expand All @@ -43,7 +45,7 @@ describe('WorkspaceUiSettingsClientWrapper', () => {
return Promise.reject();
});

const wrapper = new WorkspaceUiSettingsClientWrapper();
const wrapper = new WorkspaceUiSettingsClientWrapper(logger);
wrapper.setScopedClient(getClientMock);

return {
Expand All @@ -53,6 +55,7 @@ describe('WorkspaceUiSettingsClientWrapper', () => {
typeRegistry: requestHandlerContext.savedObjects.typeRegistry,
}),
clientMock,
logger,
};
};

Expand Down Expand Up @@ -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}`
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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'];

/**
Expand Down Expand Up @@ -61,13 +63,19 @@ export class WorkspaceUiSettingsClientWrapper {
options
);

const workspaceObject = await this.getWorkspaceTypeEnabledClient(
wrapperOptions.request
).get<WorkspaceAttribute>(WORKSPACE_TYPE, requestWorkspaceId);
let workspaceObject: SavedObject<WorkspaceAttribute> | 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<T>;
Expand Down Expand Up @@ -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<T>;
}
Expand Down

0 comments on commit 2953968

Please sign in to comment.