Skip to content

Commit

Permalink
[Workspace] Add base path when parsing url in http service (#6233)
Browse files Browse the repository at this point in the history
* fix: add base path when parse url in http service

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

* feat: add CHANGELOG

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

* feat: optimize unit test cases for parse clientBasePath from url when basePath enabled

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

* feat: add empty line before getWorkspaceIdFromURL method

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

* feat: optimize comment

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

* feat: optimize code

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

---------

Signed-off-by: SuZhou-Joe <[email protected]>
Co-authored-by: Xinrui Bai-amazon <[email protected]>
  • Loading branch information
SuZhou-Joe and xinruiba authored Mar 22, 2024
1 parent 4a8e3e8 commit 0dce00a
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [BUG][MD]Expose picker using function in data source management plugin setup([#6030](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6030))
- [BUG][Multiple Datasource] Fix data source filter bug and add tests ([#6152](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6152))
- [BUG][Multiple Datasource] Fix obsolete snapshots for test within data source management plugin ([#6185](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6185))
- [Workspace] Add base path when parse url in http service ([#6233](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6233))

### 🚞 Infrastructure

Expand Down
7 changes: 4 additions & 3 deletions src/core/public/http/http_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,21 +83,22 @@ describe('#setup()', () => {
expect(setupResult.basePath.get()).toEqual('');
});

it('setup basePath with workspaceId provided in window.location.href', () => {
it('setup basePath with workspaceId provided in window.location.href and basePath present in injectedMetadata', () => {
const windowSpy = jest.spyOn(window, 'window', 'get');
windowSpy.mockImplementation(
() =>
({
location: {
href: 'http://localhost/w/workspaceId/app',
href: 'http://localhost/base_path/w/workspaceId/app',
},
} as any)
);
const injectedMetadata = injectedMetadataServiceMock.createSetupContract();
injectedMetadata.getBasePath.mockReturnValue('/base_path');
const fatalErrors = fatalErrorsServiceMock.createSetupContract();
const httpService = new HttpService();
const setupResult = httpService.setup({ fatalErrors, injectedMetadata });
expect(setupResult.basePath.get()).toEqual('/w/workspaceId');
expect(setupResult.basePath.get()).toEqual('/base_path/w/workspaceId');
windowSpy.mockRestore();
});
});
Expand Down
2 changes: 1 addition & 1 deletion src/core/public/http/http_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export class HttpService implements CoreService<HttpSetup, HttpStart> {
public setup({ injectedMetadata, fatalErrors }: HttpDeps): HttpSetup {
const opensearchDashboardsVersion = injectedMetadata.getOpenSearchDashboardsVersion();
let clientBasePath = '';
const workspaceId = getWorkspaceIdFromUrl(window.location.href);
const workspaceId = getWorkspaceIdFromUrl(window.location.href, injectedMetadata.getBasePath());
if (workspaceId) {
clientBasePath = `${WORKSPACE_PATH_PREFIX}/${workspaceId}`;
}
Expand Down
4 changes: 2 additions & 2 deletions src/core/utils/workspace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import { httpServiceMock } from '../public/mocks';

describe('#getWorkspaceIdFromUrl', () => {
it('return workspace when there is a match', () => {
expect(getWorkspaceIdFromUrl('http://localhost/w/foo')).toEqual('foo');
expect(getWorkspaceIdFromUrl('http://localhost/w/foo', '')).toEqual('foo');
});

it('return empty when there is not a match', () => {
expect(getWorkspaceIdFromUrl('http://localhost/w2/foo')).toEqual('');
expect(getWorkspaceIdFromUrl('http://localhost/w2/foo', '')).toEqual('');
});

it('return workspace when there is a match with basePath provided', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/core/utils/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { WORKSPACE_PATH_PREFIX } from './constants';
import { IBasePath } from '../public';

export const getWorkspaceIdFromUrl = (url: string, basePath?: string): string => {
export const getWorkspaceIdFromUrl = (url: string, basePath: string): string => {
const regexp = new RegExp(`^${basePath || ''}\/w\/([^\/]*)`);
const urlObject = new URL(url);
const matchedResult = urlObject.pathname.match(regexp);
Expand Down
9 changes: 4 additions & 5 deletions src/plugins/workspace/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,17 @@ export class WorkspacePlugin implements Plugin<{}, {}, {}> {
}
}

private getWorkspaceIdFromURL(basePath?: string): string | null {
return getWorkspaceIdFromUrl(window.location.href, basePath);
}

public async setup(core: CoreSetup) {
const workspaceClient = new WorkspaceClient(core.http, core.workspaces);
await workspaceClient.init();

/**
* Retrieve workspace id from url
*/
const workspaceId = this.getWorkspaceIdFromURL(core.http.basePath.getBasePath());
const workspaceId = getWorkspaceIdFromUrl(
window.location.href,
core.http.basePath.getBasePath()
);

if (workspaceId) {
const result = await workspaceClient.enterWorkspace(workspaceId);
Expand Down
5 changes: 4 additions & 1 deletion src/plugins/workspace/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ export class WorkspacePlugin implements Plugin<WorkspacePluginSetup, WorkspacePl
* Proxy all {basePath}/w/{workspaceId}{osdPath*} paths to {basePath}{osdPath*}
*/
setupDeps.http.registerOnPreRouting(async (request, response, toolkit) => {
const workspaceId = getWorkspaceIdFromUrl(request.url.toString());
const workspaceId = getWorkspaceIdFromUrl(
request.url.toString(),
'' // No need to pass basePath here because the request.url will be rewrite by registerOnPreRouting method in `src/core/server/http/http_server.ts`
);

if (workspaceId) {
const requestUrl = new URL(request.url.toString());
Expand Down

0 comments on commit 0dce00a

Please sign in to comment.