Skip to content

Commit

Permalink
Unify base path in HttpService (elastic#38237)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mshustov authored Jun 16, 2019
1 parent 9c1bdb9 commit de5c452
Show file tree
Hide file tree
Showing 38 changed files with 521 additions and 525 deletions.
Original file line number Diff line number Diff line change
@@ -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;
};
```

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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> | |
Expand All @@ -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) | |

This file was deleted.

This file was deleted.

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

4 changes: 3 additions & 1 deletion src/core/public/chrome/nav_links/nav_links_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/core/public/chrome/nav_links/nav_links_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]
)
Expand Down
91 changes: 91 additions & 0 deletions src/core/public/http/base_path_service.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
});
});
71 changes: 71 additions & 0 deletions src/core/public/http/base_path_service.ts
Original file line number Diff line number Diff line change
@@ -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;
};
}
36 changes: 24 additions & 12 deletions src/core/public/http/http_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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,
Expand Down
Loading

0 comments on commit de5c452

Please sign in to comment.