Skip to content
This repository has been archived by the owner on Mar 31, 2024. It is now read-only.

Commit

Permalink
Migrate base path APIs and UiSettings client to new platform (elastic…
Browse files Browse the repository at this point in the history
…#22694)

Fixes elastic#20697

This PR migrates the base path related methods from `ui/chrome` to `core.basePath` and the uiSettings to `core.uiSettings`. The two are not split into separate PRs because I'm kinda cramped for time right now so rather I split the changes up into two commits so that you can review them separately if you like. If you'd like I can submit them as separate PRs but the basePath PR will block the uiSettings PR either way.

There shouldn't be any API changes except one thing, since the existing implementation is so close to what we'd want from the new platform API I made the one change that is inconsistent with what we've been doing so far and moved `uiSettings.subscribe()` to `uiSettings.getUpdate$().subscribe()`. This method isn't super commonly used, but it is a breaking change that will likely impact plugins so I'll notify some folks if we decide to move forward this way. I can also make a super-light wrapper for angular that just updates this method if you prefer.
  • Loading branch information
Spencer committed Sep 8, 2018
1 parent 9a7fa9a commit f910529
Show file tree
Hide file tree
Showing 43 changed files with 1,799 additions and 625 deletions.
104 changes: 104 additions & 0 deletions src/core/public/base_path/base_path_service.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/*
* 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 { BasePathService } from './base_path_service';

function setup(options: any = {}) {
const injectedBasePath: string =
options.injectedBasePath === undefined ? '/foo/bar' : options.injectedBasePath;

const service = new BasePathService();

const injectedMetadata = {
getBasePath: jest.fn().mockReturnValue(injectedBasePath),
} as any;

const startContract = service.start({
injectedMetadata,
});

return {
service,
startContract,
injectedBasePath,
};
}

describe('startContract.get()', () => {
it('returns an empty string if no basePath is injected', () => {
const { startContract } = setup({ injectedBasePath: null });
expect(startContract.get()).toBe('');
});

it('returns the injected basePath', () => {
const { startContract } = setup();
expect(startContract.get()).toBe('/foo/bar');
});
});

describe('startContract.addToPath()', () => {
it('adds the base path to the path if it is relative and starts with a slash', () => {
const { startContract } = setup();
expect(startContract.addToPath('/a/b')).toBe('/foo/bar/a/b');
});

it('leaves the query string and hash of path unchanged', () => {
const { startContract } = setup();
expect(startContract.addToPath('/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 { startContract } = setup();
expect(startContract.addToPath('a/b')).toBe('a/b');
});

it('returns the path unchanged it it has a hostname', () => {
const { startContract } = setup();
expect(startContract.addToPath('http://localhost:5601/a/b')).toBe('http://localhost:5601/a/b');
});
});

describe('startContract.removeFromPath()', () => {
it('removes the basePath if relative path starts with it', () => {
const { startContract } = setup();
expect(startContract.removeFromPath('/foo/bar/a/b')).toBe('/a/b');
});

it('leaves query string and hash intact', () => {
const { startContract } = setup();
expect(startContract.removeFromPath('/foo/bar/a/b?c=y#1234')).toBe('/a/b?c=y#1234');
});

it('ignores urls with hostnames', () => {
const { startContract } = setup();
expect(startContract.removeFromPath('http://localhost:5601/foo/bar/a/b')).toBe(
'http://localhost:5601/foo/bar/a/b'
);
});

it('returns slash if path is just basePath', () => {
const { startContract } = setup();
expect(startContract.removeFromPath('/foo/bar')).toBe('/');
});

it('returns full path if basePath is not its own segment', () => {
const { startContract } = setup();
expect(startContract.removeFromPath('/foo/barhop')).toBe('/foo/barhop');
});
});
74 changes: 74 additions & 0 deletions src/core/public/base_path/base_path_service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* 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 { InjectedMetadataStartContract } from '../injected_metadata';
import { modifyUrl } from '../utils';

interface Deps {
injectedMetadata: InjectedMetadataStartContract;
}

export class BasePathService {
public start({ injectedMetadata }: Deps) {
const basePath = injectedMetadata.getBasePath() || '';

return {
/**
* Get the current basePath as defined by the server
*/
get() {
return basePath;
},

/**
* Add the current basePath to a path string.
* @param path A relative url including the leading `/`, otherwise it will be returned without modification
*/
addToPath(path: string) {
return modifyUrl(path, parts => {
if (!parts.hostname && parts.pathname && parts.pathname.startsWith('/')) {
parts.pathname = `${basePath}${parts.pathname}`;
}
});
},

/**
* Remove the basePath from a path that starts with it
* @param path A relative url that starts with the basePath, which will be stripped
*/
removeFromPath(path: string) {
if (!basePath) {
return path;
}

if (path === basePath) {
return '/';
}

if (path.startsWith(basePath + '/')) {
return path.slice(basePath.length);
}

return path;
},
};
}
}

export type BasePathStartContract = ReturnType<BasePathService['start']>;
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,4 @@
* under the License.
*/

// we select the modify_url directly so the other utils, which are not browser compatible, are not included
export { modifyUrl } from '../../../utils/modify_url';
export { BasePathService, BasePathStartContract } from './base_path_service';
43 changes: 43 additions & 0 deletions src/core/public/core_system.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
* under the License.
*/

import { BasePathService } from './base_path';
import { FatalErrorsService } from './fatal_errors';
import { InjectedMetadataService } from './injected_metadata';
import { LegacyPlatformService } from './legacy_platform';
import { LoadingCountService } from './loading_count';
import { NotificationsService } from './notifications';
import { UiSettingsService } from './ui_settings';

const MockLegacyPlatformService = jest.fn<LegacyPlatformService>(
function _MockLegacyPlatformService(this: any) {
Expand Down Expand Up @@ -76,6 +78,24 @@ jest.mock('./loading_count', () => ({
LoadingCountService: MockLoadingCountService,
}));

const mockBasePathStartContract = {};
const MockBasePathService = jest.fn<BasePathService>(function _MockNotificationsService(this: any) {
this.start = jest.fn().mockReturnValue(mockBasePathStartContract);
});
jest.mock('./base_path', () => ({
BasePathService: MockBasePathService,
}));

const mockUiSettingsContract = {};
const MockUiSettingsService = jest.fn<UiSettingsService>(function _MockNotificationsService(
this: any
) {
this.start = jest.fn().mockReturnValue(mockUiSettingsContract);
});
jest.mock('./ui_settings', () => ({
UiSettingsService: MockUiSettingsService,
}));

import { CoreSystem } from './core_system';
jest.spyOn(CoreSystem.prototype, 'stop');

Expand All @@ -101,6 +121,8 @@ describe('constructor', () => {
expect(MockFatalErrorsService).toHaveBeenCalledTimes(1);
expect(MockNotificationsService).toHaveBeenCalledTimes(1);
expect(MockLoadingCountService).toHaveBeenCalledTimes(1);
expect(MockBasePathService).toHaveBeenCalledTimes(1);
expect(MockUiSettingsService).toHaveBeenCalledTimes(1);
});

it('passes injectedMetadata param to InjectedMetadataService', () => {
Expand Down Expand Up @@ -221,6 +243,27 @@ describe('#start()', () => {
});
});

it('calls basePath#start()', () => {
startCore();
const [mockInstance] = MockBasePathService.mock.instances;
expect(mockInstance.start).toHaveBeenCalledTimes(1);
expect(mockInstance.start).toHaveBeenCalledWith({
injectedMetadata: mockInjectedMetadataStartContract,
});
});

it('calls uiSettings#start()', () => {
startCore();
const [mockInstance] = MockUiSettingsService.mock.instances;
expect(mockInstance.start).toHaveBeenCalledTimes(1);
expect(mockInstance.start).toHaveBeenCalledWith({
notifications: mockNotificationStartContract,
loadingCount: mockLoadingCountContract,
injectedMetadata: mockInjectedMetadataStartContract,
basePath: mockBasePathStartContract,
});
});

it('calls fatalErrors#start()', () => {
startCore();
const [mockInstance] = MockFatalErrorsService.mock.instances;
Expand Down
23 changes: 22 additions & 1 deletion src/core/public/core_system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@
*/

import './core.css';

import { BasePathService } from './base_path';
import { FatalErrorsService } from './fatal_errors';
import { InjectedMetadataParams, InjectedMetadataService } from './injected_metadata';
import { LegacyPlatformParams, LegacyPlatformService } from './legacy_platform';
import { LoadingCountService } from './loading_count';
import { NotificationsService } from './notifications';
import { UiSettingsService } from './ui_settings';

interface Params {
rootDomElement: HTMLElement;
Expand All @@ -43,6 +46,8 @@ export class CoreSystem {
private readonly legacyPlatform: LegacyPlatformService;
private readonly notifications: NotificationsService;
private readonly loadingCount: LoadingCountService;
private readonly uiSettings: UiSettingsService;
private readonly basePath: BasePathService;

private readonly rootDomElement: HTMLElement;
private readonly notificationsTargetDomElement: HTMLDivElement;
Expand Down Expand Up @@ -71,6 +76,8 @@ export class CoreSystem {
});

this.loadingCount = new LoadingCountService();
this.basePath = new BasePathService();
this.uiSettings = new UiSettingsService();

this.legacyPlatformTargetDomElement = document.createElement('div');
this.legacyPlatform = new LegacyPlatformService({
Expand All @@ -92,7 +99,21 @@ export class CoreSystem {
const injectedMetadata = this.injectedMetadata.start();
const fatalErrors = this.fatalErrors.start();
const loadingCount = this.loadingCount.start({ fatalErrors });
this.legacyPlatform.start({ injectedMetadata, fatalErrors, notifications, loadingCount });
const basePath = this.basePath.start({ injectedMetadata });
const uiSettings = this.uiSettings.start({
notifications,
loadingCount,
injectedMetadata,
basePath,
});
this.legacyPlatform.start({
injectedMetadata,
fatalErrors,
notifications,
loadingCount,
basePath,
uiSettings,
});
} catch (error) {
this.fatalErrors.add(error);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface InjectedMetadataParams {
injectedMetadata: {
version: string;
buildNumber: number;
basePath: string;
legacyMetadata: {
[key: string]: any;
};
Expand All @@ -42,6 +43,14 @@ export class InjectedMetadataService {

public start() {
return {
getBasePath: () => {
return this.state.basePath;
},

getKibanaVersion: () => {
return this.getKibanaVersion();
},

getLegacyMetadata: () => {
return this.state.legacyMetadata;
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,31 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`#start() load order useLegacyTestHarness = false loads ui/modules before ui/chrome, and both before legacy files 1`] = `
Array [
"ui/metadata",
"ui/notify/fatal_error",
"ui/notify/toasts",
"ui/chrome/api/loading_count",
"ui/chrome/api/base_path",
"ui/chrome/api/ui_settings",
"ui/chrome",
"legacy files",
]
`;

exports[`#start() load order useLegacyTestHarness = true loads ui/modules before ui/test_harness, and both before legacy files 1`] = `
Array [
"ui/metadata",
"ui/notify/fatal_error",
"ui/notify/toasts",
"ui/chrome/api/loading_count",
"ui/chrome/api/base_path",
"ui/chrome/api/ui_settings",
"ui/test_harness",
"legacy files",
]
`;

exports[`#stop() destroys the angular scope and empties the targetDomElement if angular is bootstraped to targetDomElement 1`] = `
<div
class="ng-scope"
Expand Down
Loading

0 comments on commit f910529

Please sign in to comment.