Unify base path in HttpService (elastic#38237) (elastic#39049)

* unify modifyUrl on client and server

* create BasePath as a separate entity on server

* use BasePath class in http server

* use BasePath a separate entity on client

* use BasePath class on Http service on the client

* switch client code to the new api

* improve setver http service mocks

* address comments #1

* address comments #2

* update docs

* add comment why we define own typings

docs/development/core/public/kibana-plugin-public.httpservicebase.basepath.md

@@ -0,0 +1,15 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [HttpServiceBase](./kibana-plugin-public.httpservicebase.md) &gt; [basePath](./kibana-plugin-public.httpservicebase.basepath.md)
+
+## HttpServiceBase.basePath property
+
+<b>Signature:</b>
+
+```typescript
+basePath: {
+        get: () => string;
+        prepend: (url: string) => string;
+        remove: (url: string) => string;
+    };
+```

docs/development/core/public/kibana-plugin-public.httpservicebase.md

@@ -15,6 +15,7 @@ export interface HttpServiceBase
 
 |  Property | Type | Description |
 |  --- | --- | --- |
+|  [basePath](./kibana-plugin-public.httpservicebase.basepath.md) | <code>{</code><br/><code>        get: () =&gt; string;</code><br/><code>        prepend: (url: string) =&gt; string;</code><br/><code>        remove: (url: string) =&gt; string;</code><br/><code>    }</code> |  |
 |  [delete](./kibana-plugin-public.httpservicebase.delete.md) | <code>HttpHandler</code> |  |
 |  [fetch](./kibana-plugin-public.httpservicebase.fetch.md) | <code>HttpHandler</code> |  |
 |  [get](./kibana-plugin-public.httpservicebase.get.md) | <code>HttpHandler</code> |  |
@@ -29,11 +30,8 @@ export interface HttpServiceBase
 |  Method | Description |
 |  --- | --- |
 |  [addLoadingCount(count$)](./kibana-plugin-public.httpservicebase.addloadingcount.md) |  |
-|  [getBasePath()](./kibana-plugin-public.httpservicebase.getbasepath.md) |  |
 |  [getLoadingCount$()](./kibana-plugin-public.httpservicebase.getloadingcount$.md) |  |
 |  [intercept(interceptor)](./kibana-plugin-public.httpservicebase.intercept.md) |  |
-|  [prependBasePath(path)](./kibana-plugin-public.httpservicebase.prependbasepath.md) |  |
 |  [removeAllInterceptors()](./kibana-plugin-public.httpservicebase.removeallinterceptors.md) |  |
-|  [removeBasePath(path)](./kibana-plugin-public.httpservicebase.removebasepath.md) |  |
 |  [stop()](./kibana-plugin-public.httpservicebase.stop.md) |  |
 

docs/development/core/server/kibana-plugin-server.coresetup.http.md

@@ -11,8 +11,7 @@ http: {
         registerOnPreAuth: HttpServiceSetup['registerOnPreAuth'];
         registerAuth: HttpServiceSetup['registerAuth'];
         registerOnPostAuth: HttpServiceSetup['registerOnPostAuth'];
-        getBasePathFor: HttpServiceSetup['getBasePathFor'];
-        setBasePathFor: HttpServiceSetup['setBasePathFor'];
+        basePath: HttpServiceSetup['basePath'];
         createNewServer: HttpServiceSetup['createNewServer'];
     };
 ```

docs/development/core/server/kibana-plugin-server.coresetup.md

@@ -17,5 +17,5 @@ export interface CoreSetup
 |  Property | Type | Description |
 |  --- | --- | --- |
 |  [elasticsearch](./kibana-plugin-server.coresetup.elasticsearch.md) | <code>{</code><br/><code>        adminClient$: Observable&lt;ClusterClient&gt;;</code><br/><code>        dataClient$: Observable&lt;ClusterClient&gt;;</code><br/><code>    }</code> |  |
-|  [http](./kibana-plugin-server.coresetup.http.md) | <code>{</code><br/><code>        registerOnPreAuth: HttpServiceSetup['registerOnPreAuth'];</code><br/><code>        registerAuth: HttpServiceSetup['registerAuth'];</code><br/><code>        registerOnPostAuth: HttpServiceSetup['registerOnPostAuth'];</code><br/><code>        getBasePathFor: HttpServiceSetup['getBasePathFor'];</code><br/><code>        setBasePathFor: HttpServiceSetup['setBasePathFor'];</code><br/><code>        createNewServer: HttpServiceSetup['createNewServer'];</code><br/><code>    }</code> |  |
+|  [http](./kibana-plugin-server.coresetup.http.md) | <code>{</code><br/><code>        registerOnPreAuth: HttpServiceSetup['registerOnPreAuth'];</code><br/><code>        registerAuth: HttpServiceSetup['registerAuth'];</code><br/><code>        registerOnPostAuth: HttpServiceSetup['registerOnPostAuth'];</code><br/><code>        basePath: HttpServiceSetup['basePath'];</code><br/><code>        createNewServer: HttpServiceSetup['createNewServer'];</code><br/><code>    }</code> |  |
 

src/core/public/chrome/nav_links/nav_links_service.test.ts

@@ -29,7 +29,9 @@ const mockAppService = {
 } as any;
 
 const mockHttp = {
-  prependBasePath: (url: string) => `wow${url}`,
+  basePath: {
+    prepend: (url: string) => `wow${url}`,
+  },
 } as any;
 
 describe('NavLinksService', () => {

src/core/public/chrome/nav_links/nav_links_service.ts

@@ -42,7 +42,7 @@ export class NavLinksService {
               new NavLinkWrapper({
                 ...app,
                 // Either rootRoute or appUrl must be defined.
-                baseUrl: relativeToAbsolute(http.prependBasePath((app.rootRoute || app.appUrl)!)),
+                baseUrl: relativeToAbsolute(http.basePath.prepend((app.rootRoute || app.appUrl)!)),
               }),
             ] as [string, NavLinkWrapper]
         )

src/core/public/http/base_path_service.test.ts

@@ -0,0 +1,91 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { BasePath } from './base_path_service';
+
+describe('BasePath', () => {
+  describe('#get()', () => {
+    it('returns an empty string if no basePath not provided', () => {
+      expect(new BasePath().get()).toBe('');
+    });
+
+    it('returns basePath value if provided', () => {
+      expect(new BasePath('/foo').get()).toBe('/foo');
+    });
+
+    describe('#prepend()', () => {
+      it('adds the base path to the path if it is relative and starts with a slash', () => {
+        const basePath = new BasePath('/foo/bar');
+
+        expect(basePath.prepend('/a/b')).toBe('/foo/bar/a/b');
+      });
+
+      it('leaves the query string and hash of path unchanged', () => {
+        const basePath = new BasePath('/foo/bar');
+
+        expect(basePath.prepend('/a/b?x=y#c/d/e')).toBe('/foo/bar/a/b?x=y#c/d/e');
+      });
+
+      it('returns the path unchanged if it does not start with a slash', () => {
+        const basePath = new BasePath('/foo/bar');
+
+        expect(basePath.prepend('a/b')).toBe('a/b');
+      });
+
+      it('returns the path unchanged it it has a hostname', () => {
+        const basePath = new BasePath('/foo/bar');
+
+        expect(basePath.prepend('http://localhost:5601/a/b')).toBe('http://localhost:5601/a/b');
+      });
+    });
+
+    describe('#remove()', () => {
+      it('removes the basePath if relative path starts with it', () => {
+        const basePath = new BasePath('/foo/bar');
+
+        expect(basePath.remove('/foo/bar/a/b')).toBe('/a/b');
+      });
+
+      it('leaves query string and hash intact', () => {
+        const basePath = new BasePath('/foo/bar');
+
+        expect(basePath.remove('/foo/bar/a/b?c=y#1234')).toBe('/a/b?c=y#1234');
+      });
+
+      it('ignores urls with hostnames', () => {
+        const basePath = new BasePath('/foo/bar');
+
+        expect(basePath.remove('http://localhost:5601/foo/bar/a/b')).toBe(
+          'http://localhost:5601/foo/bar/a/b'
+        );
+      });
+
+      it('returns slash if path is just basePath', () => {
+        const basePath = new BasePath('/foo/bar');
+
+        expect(basePath.remove('/foo/bar')).toBe('/');
+      });
+
+      it('returns full path if basePath is not its own segment', () => {
+        const basePath = new BasePath('/foo/bar');
+
+        expect(basePath.remove('/foo/barhop')).toBe('/foo/barhop');
+      });
+    });
+  });
+});

src/core/public/http/base_path_service.ts

@@ -0,0 +1,71 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { modifyUrl } from '../../utils';
+
+export class BasePath {
+  constructor(private readonly basePath: string = '') {}
+
+  public get = () => {
+    return this.basePath;
+  };
+
+  public prepend = (path: string): string => {
+    if (!this.basePath) return path;
+    return modifyUrl(path, parts => {
+      if (!parts.hostname && parts.pathname && parts.pathname.startsWith('/')) {
+        parts.pathname = `${this.basePath}${parts.pathname}`;
+      }
+    });
+  };
+
+  public remove = (path: string): string => {
+    if (!this.basePath) {
+      return path;
+    }
+
+    if (path === this.basePath) {
+      return '/';
+    }
+
+    if (path.startsWith(`${this.basePath}/`)) {
+      return path.slice(this.basePath.length);
+    }
+
+    return path;
+  };
+}

src/core/public/http/http_service.mock.ts

@@ -18,9 +18,13 @@
  */
 
 import { HttpService } from './http_service';
-import { HttpSetup, HttpStart } from './types';
+import { HttpSetup } from './types';
 
-const createServiceMock = () => ({
+type ServiceSetupMockType = jest.Mocked<HttpSetup> & {
+  basePath: jest.Mocked<HttpSetup['basePath']>;
+};
+
+const createServiceMock = (): ServiceSetupMockType => ({
   fetch: jest.fn(),
   get: jest.fn(),
   head: jest.fn(),
@@ -29,23 +33,31 @@ const createServiceMock = () => ({
   patch: jest.fn(),
   delete: jest.fn(),
   options: jest.fn(),
-  getBasePath: jest.fn(),
-  prependBasePath: jest.fn(),
-  removeBasePath: jest.fn(),
+  basePath: {
+    get: jest.fn(),
+    prepend: jest.fn(),
+    remove: jest.fn(),
+  },
   addLoadingCount: jest.fn(),
   getLoadingCount$: jest.fn(),
   stop: jest.fn(),
   intercept: jest.fn(),
   removeAllInterceptors: jest.fn(),
 });
 
-const createSetupContractMock = (): jest.Mocked<HttpSetup> => createServiceMock();
-const createStartContractMock = (): jest.Mocked<HttpStart> => createServiceMock();
-const createMock = (): jest.Mocked<PublicMethodsOf<HttpService>> => ({
-  setup: jest.fn().mockReturnValue(createSetupContractMock()),
-  start: jest.fn().mockReturnValue(createStartContractMock()),
-  stop: jest.fn(),
-});
+const createSetupContractMock = () => createServiceMock();
+const createStartContractMock = () => createServiceMock();
+
+const createMock = () => {
+  const mocked: jest.Mocked<Required<HttpService>> = {
+    setup: jest.fn(),
+    start: jest.fn(),
+    stop: jest.fn(),
+  };
+  mocked.setup.mockReturnValue(createSetupContractMock());
+  mocked.start.mockReturnValue(createSetupContractMock());
+  return mocked;
+};
 
 export const httpServiceMock = {
   create: createMock,