[Backport workspace-pr-integr] Add workspace id in base path (#295)

* [Backport workspace-pr-integr] Add workspace id in base path Signed-off-by: SuZhou-Joe <[email protected]> * feat: remove useless code Signed-off-by: SuZhou-Joe <[email protected]> * feat: side effect Signed-off-by: SuZhou-Joe <[email protected]> * feat: update snapshot Signed-off-by: SuZhou-Joe <[email protected]> * feat: update snapshot Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]>
ruanyl · Mar 15, 2024 · bb8d0ac · bb8d0ac
1 parent a92b40f
commit bb8d0ac
Show file tree

Hide file tree

Showing 16 changed files with 97 additions and 71 deletions.
diff --git a/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav.test.tsx.snap
diff --git a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap
diff --git a/src/core/public/http/base_path.test.ts b/src/core/public/http/base_path.test.ts
@@ -111,35 +111,50 @@ describe('BasePath', () => {
     });
   });
 
-  describe('workspaceBasePath', () => {
-    it('get path with workspace', () => {
-      expect(new BasePath('/foo/bar', '/foo/bar', '/workspace').get()).toEqual(
-        '/foo/bar/workspace'
+  describe('clientBasePath', () => {
+    it('get with clientBasePath provided when construct', () => {
+      expect(new BasePath('/foo/bar', '/foo/bar', '/client_base_path').get()).toEqual(
+        '/foo/bar/client_base_path'
       );
     });
 
-    it('getBasePath with workspace provided', () => {
-      expect(new BasePath('/foo/bar', '/foo/bar', '/workspace').getBasePath()).toEqual('/foo/bar');
+    it('getBasePath with clientBasePath provided when construct', () => {
+      expect(new BasePath('/foo/bar', '/foo/bar', '/client_base_path').getBasePath()).toEqual(
+        '/foo/bar'
+      );
     });
 
-    it('prepend with workspace provided', () => {
-      expect(new BasePath('/foo/bar', '/foo/bar', '/workspace').prepend('/prepend')).toEqual(
-        '/foo/bar/workspace/prepend'
+    it('prepend with clientBasePath provided when construct', () => {
+      expect(new BasePath('/foo/bar', '/foo/bar', '/client_base_path').prepend('/prepend')).toEqual(
+        '/foo/bar/client_base_path/prepend'
       );
     });
 
-    it('prepend with workspace provided but calls without workspace', () => {
+    it('construct with clientBasePath provided but calls prepend with withoutClientBasePath is true', () => {
       expect(
-        new BasePath('/foo/bar', '/foo/bar', '/workspace').prepend('/prepend', {
-          withoutWorkspace: true,
+        new BasePath('/foo/bar', '/foo/bar', '/client_base_path').prepend('/prepend', {
+          withoutClientBasePath: true,
         })
       ).toEqual('/foo/bar/prepend');
     });
 
-    it('remove with workspace provided', () => {
+    it('remove with clientBasePath provided when construct', () => {
       expect(
-        new BasePath('/foo/bar', '/foo/bar', '/workspace').remove('/foo/bar/workspace/remove')
+        new BasePath('/foo/bar', '/foo/bar', '/client_base_path').remove(
+          '/foo/bar/client_base_path/remove'
+        )
       ).toEqual('/remove');
     });
+
+    it('construct with clientBasePath provided but calls remove with withoutClientBasePath is true', () => {
+      expect(
+        new BasePath('/foo/bar', '/foo/bar', '/client_base_path').remove(
+          '/foo/bar/client_base_path/remove',
+          {
+            withoutClientBasePath: true,
+          }
+        )
+      ).toEqual('/client_base_path/remove');
+    });
   });
 });
diff --git a/src/core/public/http/base_path.ts b/src/core/public/http/base_path.ts
@@ -35,20 +35,20 @@ export class BasePath {
   constructor(
     private readonly basePath: string = '',
     public readonly serverBasePath: string = basePath,
-    private readonly workspaceBasePath: string = ''
+    private readonly clientBasePath: string = ''
   ) {}
 
   public get = () => {
-    return `${this.basePath}${this.workspaceBasePath}`;
+    return `${this.basePath}${this.clientBasePath}`;
   };
 
   public getBasePath = () => {
     return this.basePath;
   };
 
   public prepend = (path: string, prependOptions?: PrependOptions): string => {
-    const { withoutWorkspace } = prependOptions || {};
-    const basePath = withoutWorkspace ? this.basePath : this.get();
+    const { withoutClientBasePath } = prependOptions || {};
+    const basePath = withoutClientBasePath ? this.basePath : this.get();
     if (!basePath) return path;
     return modifyUrl(path, (parts) => {
       if (!parts.hostname && parts.pathname && parts.pathname.startsWith('/')) {
@@ -58,8 +58,8 @@ export class BasePath {
   };
 
   public remove = (path: string, prependOptions?: PrependOptions): string => {
-    const { withoutWorkspace } = prependOptions || {};
-    const basePath = withoutWorkspace ? this.basePath : this.get();
+    const { withoutClientBasePath } = prependOptions || {};
+    const basePath = withoutClientBasePath ? this.basePath : this.get();
     if (!basePath) {
       return path;
     }

diff --git a/src/core/public/http/http_service.mock.ts b/src/core/public/http/http_service.mock.ts
@@ -39,7 +39,7 @@ export type HttpSetupMock = jest.Mocked<HttpSetup> & {
   anonymousPaths: jest.Mocked<HttpSetup['anonymousPaths']>;
 };
 
-const createServiceMock = ({ basePath = '', workspaceBasePath = '' } = {}): HttpSetupMock => ({
+const createServiceMock = ({ basePath = '', clientBasePath = '' } = {}): HttpSetupMock => ({
   fetch: jest.fn(),
   get: jest.fn(),
   head: jest.fn(),
@@ -48,7 +48,7 @@ const createServiceMock = ({ basePath = '', workspaceBasePath = '' } = {}): Http
   patch: jest.fn(),
   delete: jest.fn(),
   options: jest.fn(),
-  basePath: new BasePath(basePath, undefined, workspaceBasePath),
+  basePath: new BasePath(basePath, undefined, clientBasePath),
   anonymousPaths: {
     register: jest.fn(),
     isAnonymous: jest.fn(),
@@ -58,14 +58,14 @@ const createServiceMock = ({ basePath = '', workspaceBasePath = '' } = {}): Http
   intercept: jest.fn(),
 });
 
-const createMock = ({ basePath = '', workspaceBasePath = '' } = {}) => {
+const createMock = ({ basePath = '', clientBasePath = '' } = {}) => {
   const mocked: jest.Mocked<PublicMethodsOf<HttpService>> = {
     setup: jest.fn(),
     start: jest.fn(),
     stop: jest.fn(),
   };
-  mocked.setup.mockReturnValue(createServiceMock({ basePath, workspaceBasePath }));
-  mocked.start.mockReturnValue(createServiceMock({ basePath, workspaceBasePath }));
+  mocked.setup.mockReturnValue(createServiceMock({ basePath, clientBasePath }));
+  mocked.start.mockReturnValue(createServiceMock({ basePath, clientBasePath }));
   return mocked;
 };
 

diff --git a/src/core/public/http/http_service.ts b/src/core/public/http/http_service.ts
@@ -52,15 +52,15 @@ export class HttpService implements CoreService<HttpSetup, HttpStart> {
 
   public setup({ injectedMetadata, fatalErrors }: HttpDeps): HttpSetup {
     const opensearchDashboardsVersion = injectedMetadata.getOpenSearchDashboardsVersion();
-    let workspaceBasePath = '';
+    let clientBasePath = '';
     const workspaceId = getWorkspaceIdFromUrl(window.location.href);
     if (workspaceId) {
-      workspaceBasePath = `${WORKSPACE_PATH_PREFIX}/${workspaceId}`;
+      clientBasePath = `${WORKSPACE_PATH_PREFIX}/${workspaceId}`;
     }
     const basePath = new BasePath(
       injectedMetadata.getBasePath(),
       injectedMetadata.getServerBasePath(),
-      workspaceBasePath
+      clientBasePath
     );
     const fetchService = new Fetch({ basePath, opensearchDashboardsVersion });
     const loadingCount = this.loadingCount.setup({ fatalErrors });

diff --git a/src/core/public/http/types.ts b/src/core/public/http/types.ts
@@ -90,11 +90,14 @@ export type HttpStart = HttpSetup;
 /**
  * prepend options
  *
- * withoutWorkspace option will prepend a relative url with only basePath
- * workspaceId will rewrite the /w/{workspaceId} part, if workspace id is an empty string, prepend will remove the workspaceId part
+ * withoutClientBasePath option will prepend a relative url with serverBasePath only.
+ * For now, clientBasePath is consist of:
+ * workspacePath, which has the pattern of /w/{workspaceId}.
+ *
+ * In the future, clientBasePath may have other parts but keep `withoutClientBasePath` for now to not over-design the interface,
  */
 export interface PrependOptions {
-  withoutWorkspace?: boolean;
+  withoutClientBasePath?: boolean;
 }
 
 /**
@@ -103,22 +106,22 @@ export interface PrependOptions {
  */
 export interface IBasePath {
   /**
-   * Gets the `basePath + workspace` string.
+   * Gets the `basePath + clientBasePath` string.
    */
   get: () => string;
 
   /**
-   * Gets the `basePath
+   * Gets the `basePath` string
    */
   getBasePath: () => string;
 
   /**
-   * Prepends `path` with the basePath + workspace.
+   * Prepends `path` with the basePath + clientBasePath.
    */
   prepend: (url: string, prependOptions?: PrependOptions) => string;
 
   /**
-   * Removes the prepended basePath + workspace from the `path`.
+   * Removes the prepended basePath + clientBasePath from the `path`.
    */
   remove: (url: string, prependOptions?: PrependOptions) => string;
 

diff --git a/src/core/utils/workspace.test.ts b/src/core/utils/workspace.test.ts
@@ -14,19 +14,27 @@ describe('#getWorkspaceIdFromUrl', () => {
   it('return empty when there is not a match', () => {
     expect(getWorkspaceIdFromUrl('http://localhost/w2/foo')).toEqual('');
   });
+
+  it('return workspace when there is a match with basePath provided', () => {
+    expect(getWorkspaceIdFromUrl('http://localhost/basepath/w/foo', '/basepath')).toEqual('foo');
+  });
+
+  it('return empty when there is a match without basePath but basePath provided', () => {
+    expect(getWorkspaceIdFromUrl('http://localhost/w/foo', '/w')).toEqual('');
+  });
 });
 
 describe('#formatUrlWithWorkspaceId', () => {
-  const basePathWithoutWorkspaceBasePath = httpServiceMock.createSetupContract().basePath;
+  const basePathWithoutClientBasePath = httpServiceMock.createSetupContract().basePath;
   it('return url with workspace prefix when format with a id provided', () => {
     expect(
-      formatUrlWithWorkspaceId('/app/dashboard', 'foo', basePathWithoutWorkspaceBasePath)
+      formatUrlWithWorkspaceId('/app/dashboard', 'foo', basePathWithoutClientBasePath)
     ).toEqual('http://localhost/w/foo/app/dashboard');
   });
 
   it('return url without workspace prefix when format without a id', () => {
     expect(
-      formatUrlWithWorkspaceId('/w/foo/app/dashboard', '', basePathWithoutWorkspaceBasePath)
+      formatUrlWithWorkspaceId('/w/foo/app/dashboard', '', basePathWithoutClientBasePath)
     ).toEqual('http://localhost/app/dashboard');
   });
 });