From 9c1bdb929871fb183a58f0d96005252d29365038 Mon Sep 17 00:00:00 2001 From: Spencer Date: Sun, 16 Jun 2019 07:23:36 -0700 Subject: [PATCH 01/10] [percy] only execute percy setup when necessary (#39043) --- src/dev/ci_setup/setup.sh | 14 -------------- src/dev/ci_setup/setup_percy.sh | 17 +++++++++++++++++ test/scripts/jenkins_visual_regression.sh | 2 ++ test/scripts/jenkins_xpack_visual_regression.sh | 2 ++ 4 files changed, 21 insertions(+), 14 deletions(-) create mode 100755 src/dev/ci_setup/setup_percy.sh diff --git a/src/dev/ci_setup/setup.sh b/src/dev/ci_setup/setup.sh index 9f3c279b477de2..1d9ed703fe94de 100755 --- a/src/dev/ci_setup/setup.sh +++ b/src/dev/ci_setup/setup.sh @@ -149,13 +149,6 @@ else echo "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~" fi -### -### skip chomium download, use the system chrome install -### -export PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=true -PUPPETEER_EXECUTABLE_PATH="$(command -v google-chrome-stable)" -export PUPPETEER_EXECUTABLE_PATH - ### ### install dependencies ### @@ -172,13 +165,6 @@ if [ "$GIT_CHANGES" ]; then exit 1 fi -### -### Set Percy parallel build support environment vars -### -eval "$(node ./src/dev/ci_setup/get_percy_env)" -echo " -- PERCY_PARALLEL_NONCE='$PERCY_PARALLEL_NONCE'" -echo " -- PERCY_PARALLEL_TOTAL='$PERCY_PARALLEL_TOTAL'" - ### ### rebuild kbn-pm distributable to ensure it's not out of date ### diff --git a/src/dev/ci_setup/setup_percy.sh b/src/dev/ci_setup/setup_percy.sh new file mode 100755 index 00000000000000..c008dc1d5cdbdc --- /dev/null +++ b/src/dev/ci_setup/setup_percy.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash + +set -e + +### +### skip chomium download, use the system chrome install +### +export PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=true +PUPPETEER_EXECUTABLE_PATH="$(command -v google-chrome-stable)" +export PUPPETEER_EXECUTABLE_PATH + +### +### Set Percy parallel build support environment vars +### +eval "$(node ./src/dev/ci_setup/get_percy_env)" +echo " -- PERCY_PARALLEL_NONCE='$PERCY_PARALLEL_NONCE'" +echo " -- PERCY_PARALLEL_TOTAL='$PERCY_PARALLEL_TOTAL'" diff --git a/test/scripts/jenkins_visual_regression.sh b/test/scripts/jenkins_visual_regression.sh index c6bb148eccbfa3..5411faeb10dab0 100755 --- a/test/scripts/jenkins_visual_regression.sh +++ b/test/scripts/jenkins_visual_regression.sh @@ -3,6 +3,8 @@ set -e trap 'node "$KIBANA_DIR/src/dev/failed_tests/cli"' EXIT +source "$KIBANA_DIR/src/dev/ci_setup/setup_percy.sh" + node scripts/build --debug --oss; linuxBuild="$(find "$KIBANA_DIR/target" -name 'kibana-oss-*-linux-x86_64.tar.gz')" installDir="$PARENT_DIR/install/kibana" diff --git a/test/scripts/jenkins_xpack_visual_regression.sh b/test/scripts/jenkins_xpack_visual_regression.sh index 9f8fdce6ec220a..5f27bf66ffcedf 100755 --- a/test/scripts/jenkins_xpack_visual_regression.sh +++ b/test/scripts/jenkins_xpack_visual_regression.sh @@ -3,6 +3,8 @@ set -e trap 'node "$KIBANA_DIR/src/dev/failed_tests/cli"' EXIT +source "$KIBANA_DIR/src/dev/ci_setup/setup_percy.sh" + node scripts/build --debug --no-oss; linuxBuild="$(find "$KIBANA_DIR/target" -name 'kibana-*-linux-x86_64.tar.gz')" installDir="$PARENT_DIR/install/kibana" From de5c452f14a5b6ad2fb05baa343889ac36226cdc Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Sun, 16 Jun 2019 16:36:02 +0200 Subject: [PATCH 02/10] Unify base path in HttpService (#38237) * 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 --- ...-plugin-public.httpservicebase.basepath.md | 15 ++ ...ugin-public.httpservicebase.getbasepath.md | 15 -- .../kibana-plugin-public.httpservicebase.md | 4 +- ...-public.httpservicebase.prependbasepath.md | 22 --- ...n-public.httpservicebase.removebasepath.md | 22 --- .../kibana-plugin-server.coresetup.http.md | 3 +- .../server/kibana-plugin-server.coresetup.md | 2 +- .../nav_links/nav_links_service.test.ts | 4 +- .../chrome/nav_links/nav_links_service.ts | 2 +- .../public/http/base_path_service.test.ts | 91 ++++++++++++ src/core/public/http/base_path_service.ts | 71 ++++++++++ src/core/public/http/http_service.mock.ts | 36 +++-- src/core/public/http/http_service.test.ts | 68 +-------- src/core/public/http/http_setup.ts | 38 +---- src/core/public/http/types.ts | 8 +- .../public/plugins/plugins_service.test.ts | 6 +- src/core/public/plugins/plugins_service.ts | 2 +- src/core/public/public.api.md | 12 +- .../ui_settings_service.test.ts.snap | 8 +- src/core/public/utils/index.ts | 1 - src/core/public/utils/modify_url.test.ts | 59 -------- src/core/public/utils/modify_url.ts | 95 ------------- .../server/http/base_path_service.test.ts | 132 ++++++++++++++++++ src/core/server/http/base_path_service.ts | 76 ++++++++++ src/core/server/http/http_server.test.ts | 81 ----------- src/core/server/http/http_server.ts | 64 +++------ src/core/server/http/http_service.mock.ts | 25 ++-- .../integration_tests/http_service.test.ts | 6 +- src/core/server/index.ts | 3 +- src/core/server/plugins/plugin_context.ts | 3 +- src/core/server/server.api.md | 3 +- src/core/utils/url.ts | 23 +-- .../server/http/setup_base_path_provider.js | 4 +- .../ui/public/chrome/api/base_path.test.ts | 30 ++-- src/legacy/ui/public/chrome/api/base_path.ts | 6 +- .../public/legacy_compat/angular_config.tsx | 2 +- .../notify/app_redirect/app_redirect.js | 2 +- src/legacy/ui/public/url/kibana_parsed_url.ts | 2 +- 38 files changed, 521 insertions(+), 525 deletions(-) create mode 100644 docs/development/core/public/kibana-plugin-public.httpservicebase.basepath.md delete mode 100644 docs/development/core/public/kibana-plugin-public.httpservicebase.getbasepath.md delete mode 100644 docs/development/core/public/kibana-plugin-public.httpservicebase.prependbasepath.md delete mode 100644 docs/development/core/public/kibana-plugin-public.httpservicebase.removebasepath.md create mode 100644 src/core/public/http/base_path_service.test.ts create mode 100644 src/core/public/http/base_path_service.ts delete mode 100644 src/core/public/utils/modify_url.test.ts delete mode 100644 src/core/public/utils/modify_url.ts create mode 100644 src/core/server/http/base_path_service.test.ts create mode 100644 src/core/server/http/base_path_service.ts diff --git a/docs/development/core/public/kibana-plugin-public.httpservicebase.basepath.md b/docs/development/core/public/kibana-plugin-public.httpservicebase.basepath.md new file mode 100644 index 00000000000000..6794199dc9dbd1 --- /dev/null +++ b/docs/development/core/public/kibana-plugin-public.httpservicebase.basepath.md @@ -0,0 +1,15 @@ + + +[Home](./index.md) > [kibana-plugin-public](./kibana-plugin-public.md) > [HttpServiceBase](./kibana-plugin-public.httpservicebase.md) > [basePath](./kibana-plugin-public.httpservicebase.basepath.md) + +## HttpServiceBase.basePath property + +Signature: + +```typescript +basePath: { + get: () => string; + prepend: (url: string) => string; + remove: (url: string) => string; + }; +``` diff --git a/docs/development/core/public/kibana-plugin-public.httpservicebase.getbasepath.md b/docs/development/core/public/kibana-plugin-public.httpservicebase.getbasepath.md deleted file mode 100644 index 918f0514b3c4bf..00000000000000 --- a/docs/development/core/public/kibana-plugin-public.httpservicebase.getbasepath.md +++ /dev/null @@ -1,15 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-public](./kibana-plugin-public.md) > [HttpServiceBase](./kibana-plugin-public.httpservicebase.md) > [getBasePath](./kibana-plugin-public.httpservicebase.getbasepath.md) - -## HttpServiceBase.getBasePath() method - -Signature: - -```typescript -getBasePath(): string; -``` -Returns: - -`string` - diff --git a/docs/development/core/public/kibana-plugin-public.httpservicebase.md b/docs/development/core/public/kibana-plugin-public.httpservicebase.md index 2287855e20122b..aa08108e2cdb0d 100644 --- a/docs/development/core/public/kibana-plugin-public.httpservicebase.md +++ b/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) | {
get: () => string;
prepend: (url: string) => string;
remove: (url: string) => string;
} | | | [delete](./kibana-plugin-public.httpservicebase.delete.md) | HttpHandler | | | [fetch](./kibana-plugin-public.httpservicebase.fetch.md) | HttpHandler | | | [get](./kibana-plugin-public.httpservicebase.get.md) | HttpHandler | | @@ -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) | | diff --git a/docs/development/core/public/kibana-plugin-public.httpservicebase.prependbasepath.md b/docs/development/core/public/kibana-plugin-public.httpservicebase.prependbasepath.md deleted file mode 100644 index a4ae95eb1c3078..00000000000000 --- a/docs/development/core/public/kibana-plugin-public.httpservicebase.prependbasepath.md +++ /dev/null @@ -1,22 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-public](./kibana-plugin-public.md) > [HttpServiceBase](./kibana-plugin-public.httpservicebase.md) > [prependBasePath](./kibana-plugin-public.httpservicebase.prependbasepath.md) - -## HttpServiceBase.prependBasePath() method - -Signature: - -```typescript -prependBasePath(path: string): string; -``` - -## Parameters - -| Parameter | Type | Description | -| --- | --- | --- | -| path | string | | - -Returns: - -`string` - diff --git a/docs/development/core/public/kibana-plugin-public.httpservicebase.removebasepath.md b/docs/development/core/public/kibana-plugin-public.httpservicebase.removebasepath.md deleted file mode 100644 index 90fefb1532a68e..00000000000000 --- a/docs/development/core/public/kibana-plugin-public.httpservicebase.removebasepath.md +++ /dev/null @@ -1,22 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-public](./kibana-plugin-public.md) > [HttpServiceBase](./kibana-plugin-public.httpservicebase.md) > [removeBasePath](./kibana-plugin-public.httpservicebase.removebasepath.md) - -## HttpServiceBase.removeBasePath() method - -Signature: - -```typescript -removeBasePath(path: string): string; -``` - -## Parameters - -| Parameter | Type | Description | -| --- | --- | --- | -| path | string | | - -Returns: - -`string` - diff --git a/docs/development/core/server/kibana-plugin-server.coresetup.http.md b/docs/development/core/server/kibana-plugin-server.coresetup.http.md index d6b87006da5330..cba8a5832058d0 100644 --- a/docs/development/core/server/kibana-plugin-server.coresetup.http.md +++ b/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']; }; ``` diff --git a/docs/development/core/server/kibana-plugin-server.coresetup.md b/docs/development/core/server/kibana-plugin-server.coresetup.md index a361707664b877..352e2f314da30f 100644 --- a/docs/development/core/server/kibana-plugin-server.coresetup.md +++ b/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) | {
adminClient$: Observable<ClusterClient>;
dataClient$: Observable<ClusterClient>;
} | | -| [http](./kibana-plugin-server.coresetup.http.md) | {
registerOnPreAuth: HttpServiceSetup['registerOnPreAuth'];
registerAuth: HttpServiceSetup['registerAuth'];
registerOnPostAuth: HttpServiceSetup['registerOnPostAuth'];
getBasePathFor: HttpServiceSetup['getBasePathFor'];
setBasePathFor: HttpServiceSetup['setBasePathFor'];
createNewServer: HttpServiceSetup['createNewServer'];
} | | +| [http](./kibana-plugin-server.coresetup.http.md) | {
registerOnPreAuth: HttpServiceSetup['registerOnPreAuth'];
registerAuth: HttpServiceSetup['registerAuth'];
registerOnPostAuth: HttpServiceSetup['registerOnPostAuth'];
basePath: HttpServiceSetup['basePath'];
createNewServer: HttpServiceSetup['createNewServer'];
} | | diff --git a/src/core/public/chrome/nav_links/nav_links_service.test.ts b/src/core/public/chrome/nav_links/nav_links_service.test.ts index d6ccdf84c0f2c8..fe74ae3a7d9a24 100644 --- a/src/core/public/chrome/nav_links/nav_links_service.test.ts +++ b/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', () => { diff --git a/src/core/public/chrome/nav_links/nav_links_service.ts b/src/core/public/chrome/nav_links/nav_links_service.ts index e8ea30ff6f61d0..60f5e4574752e3 100644 --- a/src/core/public/chrome/nav_links/nav_links_service.ts +++ b/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] ) diff --git a/src/core/public/http/base_path_service.test.ts b/src/core/public/http/base_path_service.test.ts new file mode 100644 index 00000000000000..65403c906e614e --- /dev/null +++ b/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'); + }); + }); + }); +}); diff --git a/src/core/public/http/base_path_service.ts b/src/core/public/http/base_path_service.ts new file mode 100644 index 00000000000000..6352327c41625b --- /dev/null +++ b/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; + }; +} diff --git a/src/core/public/http/http_service.mock.ts b/src/core/public/http/http_service.mock.ts index 3f5611ba7f7df5..4ca925a19af818 100644 --- a/src/core/public/http/http_service.mock.ts +++ b/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 & { + basePath: jest.Mocked; +}; + +const createServiceMock = (): ServiceSetupMockType => ({ fetch: jest.fn(), get: jest.fn(), head: jest.fn(), @@ -29,9 +33,11 @@ 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(), @@ -39,13 +45,19 @@ const createServiceMock = () => ({ removeAllInterceptors: jest.fn(), }); -const createSetupContractMock = (): jest.Mocked => createServiceMock(); -const createStartContractMock = (): jest.Mocked => createServiceMock(); -const createMock = (): jest.Mocked> => ({ - 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> = { + setup: jest.fn(), + start: jest.fn(), + stop: jest.fn(), + }; + mocked.setup.mockReturnValue(createSetupContractMock()); + mocked.start.mockReturnValue(createSetupContractMock()); + return mocked; +}; export const httpServiceMock = { create: createMock, diff --git a/src/core/public/http/http_service.test.ts b/src/core/public/http/http_service.test.ts index d40152157ec93e..68fb08bfb38e36 100644 --- a/src/core/public/http/http_service.test.ts +++ b/src/core/public/http/http_service.test.ts @@ -29,79 +29,19 @@ const setupFakeBasePath: SetupTap = injectedMetadata => { injectedMetadata.getBasePath.mockReturnValue('/foo/bar'); }; -describe('getBasePath', () => { +describe('basePath.get()', () => { it('returns an empty string if no basePath is injected', () => { const { http } = setup(injectedMetadata => { - injectedMetadata.getBasePath.mockReturnValue(''); + injectedMetadata.getBasePath.mockReturnValue(undefined as any); }); - expect(http.getBasePath()).toBe(''); + expect(http.basePath.get()).toBe(''); }); it('returns the injected basePath', () => { const { http } = setup(setupFakeBasePath); - expect(http.getBasePath()).toBe('/foo/bar'); - }); -}); - -describe('prependBasePath', () => { - it('adds the base path to the path if it is relative and starts with a slash', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.prependBasePath('/a/b')).toBe('/foo/bar/a/b'); - }); - - it('leaves the query string and hash of path unchanged', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.prependBasePath('/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 { http } = setup(setupFakeBasePath); - - expect(http.prependBasePath('a/b')).toBe('a/b'); - }); - - it('returns the path unchanged it it has a hostname', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.prependBasePath('http://localhost:5601/a/b')).toBe('http://localhost:5601/a/b'); - }); -}); - -describe('removeBasePath', () => { - it('removes the basePath if relative path starts with it', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.removeBasePath('/foo/bar/a/b')).toBe('/a/b'); - }); - - it('leaves query string and hash intact', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.removeBasePath('/foo/bar/a/b?c=y#1234')).toBe('/a/b?c=y#1234'); - }); - - it('ignores urls with hostnames', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.removeBasePath('http://localhost:5601/foo/bar/a/b')).toBe( - 'http://localhost:5601/foo/bar/a/b' - ); - }); - - it('returns slash if path is just basePath', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.removeBasePath('/foo/bar')).toBe('/'); - }); - - it('returns full path if basePath is not its own segment', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.removeBasePath('/foo/barhop')).toBe('/foo/barhop'); + expect(http.basePath.get()).toBe('/foo/bar'); }); }); diff --git a/src/core/public/http/http_setup.ts b/src/core/public/http/http_setup.ts index c12ec4be5d3a3d..0e270b3e2610ee 100644 --- a/src/core/public/http/http_setup.ts +++ b/src/core/public/http/http_setup.ts @@ -31,11 +31,11 @@ import { merge } from 'lodash'; import { format } from 'url'; import { InjectedMetadataSetup } from '../injected_metadata'; import { FatalErrorsSetup } from '../fatal_errors'; -import { modifyUrl } from '../utils'; import { HttpFetchOptions, HttpServiceBase, HttpInterceptor, HttpResponse } from './types'; import { HttpInterceptController } from './http_intercept_controller'; import { HttpFetchError } from './http_fetch_error'; import { HttpInterceptHaltError } from './http_intercept_halt_error'; +import { BasePath } from './base_path_service'; const JSON_CONTENT = /^(application\/(json|x-javascript)|text\/(x-)?javascript|x-json)(;.*)?$/; const NDJSON_CONTENT = /^(application\/ndjson)(;.*)?$/; @@ -48,7 +48,7 @@ export const setup = ( const stop$ = new Subject(); const interceptors = new Set(); const kibanaVersion = injectedMetadata.getKibanaVersion(); - const basePath = injectedMetadata.getBasePath() || ''; + const basePath = new BasePath(injectedMetadata.getBasePath()); function intercept(interceptor: HttpInterceptor) { interceptors.add(interceptor); @@ -60,14 +60,6 @@ export const setup = ( interceptors.clear(); } - function prependBasePath(path: string): string { - return modifyUrl(path, parts => { - if (!parts.hostname && parts.pathname && parts.pathname.startsWith('/')) { - parts.pathname = `${basePath}${parts.pathname}`; - } - }); - } - function createRequest(path: string, options?: HttpFetchOptions) { const { query, prependBasePath: shouldPrependBasePath, ...fetchOptions } = merge( { @@ -82,7 +74,7 @@ export const setup = ( options || {} ); const url = format({ - pathname: shouldPrependBasePath ? prependBasePath(path) : path, + pathname: shouldPrependBasePath ? basePath.prepend(path) : path, query, }); @@ -255,26 +247,6 @@ export const setup = ( loadingCount$.complete(); } - function getBasePath() { - return basePath; - } - - function removeBasePath(path: string): string { - if (!basePath) { - return path; - } - - if (path === basePath) { - return '/'; - } - - if (path.startsWith(`${basePath}/`)) { - return path.slice(basePath.length); - } - - return path; - } - function addLoadingCount(count$: Observable) { count$ .pipe( @@ -314,9 +286,7 @@ export const setup = ( return { stop, - getBasePath, - prependBasePath, - removeBasePath, + basePath, intercept, removeAllInterceptors, fetch, diff --git a/src/core/public/http/types.ts b/src/core/public/http/types.ts index 9f28d03e4e5af6..79527e528ac400 100644 --- a/src/core/public/http/types.ts +++ b/src/core/public/http/types.ts @@ -26,9 +26,11 @@ import { HttpFetchError } from './http_fetch_error'; /** @public */ export interface HttpServiceBase { stop(): void; - getBasePath(): string; - prependBasePath(path: string): string; - removeBasePath(path: string): string; + basePath: { + get: () => string; + prepend: (url: string) => string; + remove: (url: string) => string; + }; intercept(interceptor: HttpInterceptor): () => void; removeAllInterceptors(): void; fetch: HttpHandler; diff --git a/src/core/public/plugins/plugins_service.test.ts b/src/core/public/plugins/plugins_service.test.ts index 84d4f33fdb153e..84f01eda00ca94 100644 --- a/src/core/public/plugins/plugins_service.test.ts +++ b/src/core/public/plugins/plugins_service.test.ts @@ -167,9 +167,9 @@ test('`PluginsService.setup` calls loadPluginBundles with http and plugins', asy await pluginsService.setup(mockSetupDeps); expect(mockLoadPluginBundle).toHaveBeenCalledTimes(3); - expect(mockLoadPluginBundle).toHaveBeenCalledWith(mockSetupDeps.http.prependBasePath, 'pluginA'); - expect(mockLoadPluginBundle).toHaveBeenCalledWith(mockSetupDeps.http.prependBasePath, 'pluginB'); - expect(mockLoadPluginBundle).toHaveBeenCalledWith(mockSetupDeps.http.prependBasePath, 'pluginC'); + expect(mockLoadPluginBundle).toHaveBeenCalledWith(mockSetupDeps.http.basePath.prepend, 'pluginA'); + expect(mockLoadPluginBundle).toHaveBeenCalledWith(mockSetupDeps.http.basePath.prepend, 'pluginB'); + expect(mockLoadPluginBundle).toHaveBeenCalledWith(mockSetupDeps.http.basePath.prepend, 'pluginC'); }); test('`PluginsService.setup` initalizes plugins with CoreContext', async () => { diff --git a/src/core/public/plugins/plugins_service.ts b/src/core/public/plugins/plugins_service.ts index c3b337a8956ec7..6d49a77ad08eb4 100644 --- a/src/core/public/plugins/plugins_service.ts +++ b/src/core/public/plugins/plugins_service.ts @@ -70,7 +70,7 @@ export class PluginsService implements CoreService(); diff --git a/src/core/public/public.api.md b/src/core/public/public.api.md index 7cfbe61abff3ac..ed5fbb26eba16d 100644 --- a/src/core/public/public.api.md +++ b/src/core/public/public.api.md @@ -190,6 +190,12 @@ export interface HttpServiceBase { // (undocumented) addLoadingCount(count$: Observable): void; // (undocumented) + basePath: { + get: () => string; + prepend: (url: string) => string; + remove: (url: string) => string; + }; + // (undocumented) delete: HttpHandler; // Warning: (ae-forgotten-export) The symbol "HttpHandler" needs to be exported by the entry point index.d.ts // @@ -198,8 +204,6 @@ export interface HttpServiceBase { // (undocumented) get: HttpHandler; // (undocumented) - getBasePath(): string; - // (undocumented) getLoadingCount$(): Observable; // (undocumented) head: HttpHandler; @@ -212,14 +216,10 @@ export interface HttpServiceBase { // (undocumented) post: HttpHandler; // (undocumented) - prependBasePath(path: string): string; - // (undocumented) put: HttpHandler; // (undocumented) removeAllInterceptors(): void; // (undocumented) - removeBasePath(path: string): string; - // (undocumented) stop(): void; } diff --git a/src/core/public/ui_settings/__snapshots__/ui_settings_service.test.ts.snap b/src/core/public/ui_settings/__snapshots__/ui_settings_service.test.ts.snap index edbdef3f050990..f607c924a9e683 100644 --- a/src/core/public/ui_settings/__snapshots__/ui_settings_service.test.ts.snap +++ b/src/core/public/ui_settings/__snapshots__/ui_settings_service.test.ts.snap @@ -20,20 +20,22 @@ exports[`#setup constructs UiSettingsClient and UiSettingsApi: UiSettingsApi arg }, ], }, + "basePath": Object { + "get": [MockFunction], + "prepend": [MockFunction], + "remove": [MockFunction], + }, "delete": [MockFunction], "fetch": [MockFunction], "get": [MockFunction], - "getBasePath": [MockFunction], "getLoadingCount$": [MockFunction], "head": [MockFunction], "intercept": [MockFunction], "options": [MockFunction], "patch": [MockFunction], "post": [MockFunction], - "prependBasePath": [MockFunction], "put": [MockFunction], "removeAllInterceptors": [MockFunction], - "removeBasePath": [MockFunction], "stop": [MockFunction], }, ], diff --git a/src/core/public/utils/index.ts b/src/core/public/utils/index.ts index 786e12e4de688d..300fb1e43e29ae 100644 --- a/src/core/public/utils/index.ts +++ b/src/core/public/utils/index.ts @@ -17,5 +17,4 @@ * under the License. */ -export { modifyUrl } from './modify_url'; export { shareWeakReplay } from './share_weak_replay'; diff --git a/src/core/public/utils/modify_url.test.ts b/src/core/public/utils/modify_url.test.ts deleted file mode 100644 index 3b8091b7c3074c..00000000000000 --- a/src/core/public/utils/modify_url.test.ts +++ /dev/null @@ -1,59 +0,0 @@ -/* - * 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 './modify_url'; - -it('supports returning a new url spec', () => { - expect(modifyUrl('http://localhost', () => ({}))).toBe(''); -}); - -it('supports modifying the passed object', () => { - expect( - modifyUrl('http://localhost', parsed => { - parsed.port = '9999'; - parsed.auth = 'foo:bar'; - }) - ).toBe('http://foo:bar@localhost:9999/'); -}); - -it('supports changing pathname', () => { - expect( - modifyUrl('http://localhost/some/path', parsed => { - parsed.pathname += '/subpath'; - }) - ).toBe('http://localhost/some/path/subpath'); -}); - -it('supports changing port', () => { - expect( - modifyUrl('http://localhost:5601', parsed => { - parsed.port = String(Number(parsed.port) + 1); - }) - ).toBe('http://localhost:5602/'); -}); - -it('supports changing protocol', () => { - expect( - modifyUrl('http://localhost', parsed => { - parsed.protocol = 'mail'; - parsed.slashes = false; - parsed.pathname = undefined; - }) - ).toBe('mail:localhost'); -}); diff --git a/src/core/public/utils/modify_url.ts b/src/core/public/utils/modify_url.ts deleted file mode 100644 index a441c7eed6a38e..00000000000000 --- a/src/core/public/utils/modify_url.ts +++ /dev/null @@ -1,95 +0,0 @@ -/* - * 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 { format as formatUrl, parse as parseUrl } from 'url'; - -interface UrlParts { - protocol?: string; - slashes?: boolean; - auth?: string; - hostname?: string; - port?: string; - pathname?: string; - query: { [key: string]: string | string[] | undefined }; - hash?: string; -} - -/** - * Takes a URL and a function that takes the meaningful parts - * of the URL as a key-value object, modifies some or all of - * the parts, and returns the modified parts formatted again - * as a url. - * - * Url Parts sent: - * - protocol - * - slashes (does the url have the //) - * - auth - * - hostname (just the name of the host, no port or auth information) - * - port - * - pathname (the path after the hostname, no query or hash, starts - * with a slash if there was a path) - * - query (always an object, even when no query on original url) - * - hash - * - * Why? - * - The default url library in node produces several conflicting - * properties on the "parsed" output. Modifying any of these might - * lead to the modifications being ignored (depending on which - * property was modified) - * - It's not always clear wither to use path/pathname, host/hostname, - * so this tries to add helpful constraints - * - * @param url the url to parse - * @param block a function that will modify the parsed url, or return a new one - */ -export function modifyUrl(url: string, block: (parts: UrlParts) => Partial | void) { - const parsed = parseUrl(url, true); - - // copy over the most specific version of each - // property. By default, the parsed url includes - // several conflicting properties (like path and - // pathname + search, or search and query) and keeping - // track of which property is actually used when they - // are formatted is harder than necessary - const meaningfulParts = { - protocol: parsed.protocol, - slashes: parsed.slashes, - auth: parsed.auth, - hostname: parsed.hostname, - port: parsed.port, - pathname: parsed.pathname, - query: parsed.query || {}, - hash: parsed.hash, - }; - - // the block modifies the meaningfulParts object, or returns a new one - const modifiedParts = block(meaningfulParts) || meaningfulParts; - - // format the modified/replaced meaningfulParts back into a url - return formatUrl({ - protocol: modifiedParts.protocol, - slashes: modifiedParts.slashes, - auth: modifiedParts.auth, - hostname: modifiedParts.hostname, - port: modifiedParts.port, - pathname: modifiedParts.pathname, - query: modifiedParts.query, - hash: modifiedParts.hash, - }); -} diff --git a/src/core/server/http/base_path_service.test.ts b/src/core/server/http/base_path_service.test.ts new file mode 100644 index 00000000000000..ffbbe158cb2d4d --- /dev/null +++ b/src/core/server/http/base_path_service.test.ts @@ -0,0 +1,132 @@ +/* + * 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'; +import { KibanaRequest } from './router'; +import { httpServerMock } from './http_server.mocks'; + +describe('BasePath', () => { + describe('#get()', () => { + it('returns base path associated with an incoming Legacy.Request request', () => { + const request = httpServerMock.createRawRequest(); + + const basePath = new BasePath(); + basePath.set(request, '/baz/'); + expect(basePath.get(request)).toBe('/baz/'); + }); + + it('returns base path associated with an incoming KibanaRequest', () => { + const request = httpServerMock.createRawRequest(); + const basePath = new BasePath(); + + basePath.set(KibanaRequest.from(request, undefined), '/baz/'); + expect(basePath.get(KibanaRequest.from(request, undefined))).toBe('/baz/'); + }); + + it('operates with both Legacy.Request/KibanaRequest requests', () => { + const request = httpServerMock.createRawRequest(); + const basePath = new BasePath(); + + basePath.set(request, '/baz/'); + expect(basePath.get(KibanaRequest.from(request, undefined))).toBe('/baz/'); + }); + + it('is based on server base path', () => { + const request = httpServerMock.createRawRequest(); + const basePath = new BasePath('/foo/bar'); + + basePath.set(request, '/baz/'); + expect(basePath.get(request)).toBe('/foo/bar/baz/'); + }); + }); + + describe('#set()', () => { + it('#set() cannot be set twice for one request', () => { + const request = httpServerMock.createRawRequest(); + const basePath = new BasePath('/foo/bar'); + + const setPath = () => basePath.set(request, 'baz/'); + setPath(); + + expect(setPath).toThrowErrorMatchingInlineSnapshot( + `"Request basePath was previously set. Setting multiple times is not supported."` + ); + }); + }); + + 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'); + }); + }); +}); diff --git a/src/core/server/http/base_path_service.ts b/src/core/server/http/base_path_service.ts new file mode 100644 index 00000000000000..a6a868547bfb1f --- /dev/null +++ b/src/core/server/http/base_path_service.ts @@ -0,0 +1,76 @@ +/* + * 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 { Request } from 'hapi'; +import { KibanaRequest, toRawRequest } from './router'; + +import { modifyUrl } from '../../utils'; + +const getIncomingMessage = (request: KibanaRequest | Request) => + request instanceof KibanaRequest ? toRawRequest(request).raw.req : request.raw.req; + +export class BasePath { + private readonly basePathCache = new WeakMap, string>(); + + constructor(private readonly serverBasePath?: string) {} + + public get = (request: KibanaRequest | Request) => { + const incomingMessage = getIncomingMessage(request); + + const requestScopePath = this.basePathCache.get(incomingMessage) || ''; + const serverBasePath = this.serverBasePath || ''; + return `${serverBasePath}${requestScopePath}`; + }; + + // should work only for KibanaRequest as soon as spaces migrate to NP + public set = (request: KibanaRequest | Request, requestSpecificBasePath: string) => { + const incomingMessage = getIncomingMessage(request); + + if (this.basePathCache.has(incomingMessage)) { + throw new Error( + 'Request basePath was previously set. Setting multiple times is not supported.' + ); + } + this.basePathCache.set(incomingMessage, requestSpecificBasePath); + }; + + public prepend = (path: string): string => { + if (!this.serverBasePath) return path; + return modifyUrl(path, parts => { + if (!parts.hostname && parts.pathname && parts.pathname.startsWith('/')) { + parts.pathname = `${this.serverBasePath}${parts.pathname}`; + } + }); + }; + + public remove = (path: string): string => { + if (!this.serverBasePath) { + return path; + } + + if (path === this.serverBasePath) { + return '/'; + } + + if (path.startsWith(`${this.serverBasePath}/`)) { + return path.slice(this.serverBasePath.length); + } + + return path; + }; +} diff --git a/src/core/server/http/http_server.test.ts b/src/core/server/http/http_server.test.ts index 9eb786ebb586a9..3676b4deaec464 100644 --- a/src/core/server/http/http_server.test.ts +++ b/src/core/server/http/http_server.test.ts @@ -32,8 +32,6 @@ import { ByteSizeValue } from '@kbn/config-schema'; import { HttpConfig, Router } from '.'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { HttpServer } from './http_server'; -import { KibanaRequest } from './router'; -import { httpServerMock } from './http_server.mocks'; const chance = new Chance(); @@ -603,85 +601,6 @@ test('throws an error if starts without set up', async () => { ); }); -test('#getBasePathFor() returns base path associated with an incoming request', async () => { - const { - getBasePathFor, - setBasePathFor, - registerRouter, - server: innerServer, - registerOnPostAuth, - } = await server.setup(config); - - const path = '/base-path'; - registerOnPostAuth((req, t) => { - setBasePathFor(req, path); - return t.next(); - }); - - const router = new Router('/'); - router.get({ path: '/', validate: false }, (req, res) => res.ok({ key: getBasePathFor(req) })); - registerRouter(router); - - await server.start(); - await supertest(innerServer.listener) - .get('/') - .expect(200) - .then(res => { - expect(res.body).toEqual({ key: path }); - }); -}); - -test('#getBasePathFor() is based on server base path', async () => { - const configWithBasePath = { - ...config, - basePath: '/bar', - }; - const { - getBasePathFor, - setBasePathFor, - registerRouter, - server: innerServer, - registerOnPostAuth, - } = await server.setup(configWithBasePath); - - const path = '/base-path'; - registerOnPostAuth((req, t) => { - setBasePathFor(req, path); - return t.next(); - }); - - const router = new Router('/'); - router.get({ path: '/', validate: false }, (req, res) => res.ok({ key: getBasePathFor(req) })); - registerRouter(router); - - await server.start(); - await supertest(innerServer.listener) - .get('/') - .expect(200) - .then(res => { - expect(res.body).toEqual({ key: `${configWithBasePath.basePath}${path}` }); - }); -}); - -test('#setBasePathFor() cannot be set twice for one request', async () => { - const kibanaRequestFactory = { - from() { - return KibanaRequest.from(httpServerMock.createRawRequest()); - }, - }; - jest.doMock('./router/request', () => ({ - KibanaRequest: jest.fn(() => kibanaRequestFactory), - })); - - const { setBasePathFor } = await server.setup(config); - const req = kibanaRequestFactory.from(); - const setPath = () => setBasePathFor(req, '/path'); - - setPath(); - expect(setPath).toThrowErrorMatchingInlineSnapshot( - `"Request basePath was previously set. Setting multiple times is not supported."` - ); -}); const cookieOptions = { name: 'sid', encryptionKey: 'something_at_least_32_characters', diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts index ab2d227193dce8..1cff8cd1d312e2 100644 --- a/src/core/server/http/http_server.ts +++ b/src/core/server/http/http_server.ts @@ -19,23 +19,20 @@ import { Request, Server, ServerOptions } from 'hapi'; -import { modifyUrl } from '../../utils'; import { Logger } from '../logging'; import { HttpConfig } from './http_config'; import { createServer, getServerOptions } from './http_tools'; import { adoptToHapiAuthFormat, AuthenticationHandler } from './lifecycle/auth'; import { adoptToHapiOnPostAuthFormat, OnPostAuthHandler } from './lifecycle/on_post_auth'; import { adoptToHapiOnPreAuthFormat, OnPreAuthHandler } from './lifecycle/on_pre_auth'; -import { Router, KibanaRequest, toRawRequest } from './router'; +import { Router, KibanaRequest } from './router'; import { SessionStorageCookieOptions, createCookieSessionStorageFactory, } from './cookie_session_storage'; import { SessionStorageFactory } from './session_storage'; import { AuthStateStorage } from './auth_state_storage'; - -const getIncomingMessage = (request: KibanaRequest | Request) => - request instanceof KibanaRequest ? toRawRequest(request).raw.req : request.raw.req; +import { BasePath } from './base_path_service'; export interface HttpServerSetup { server: Server; @@ -67,8 +64,12 @@ export interface HttpServerSetup { * (from the first registered to the last). */ registerOnPostAuth: (handler: OnPostAuthHandler) => void; - getBasePathFor: (request: KibanaRequest | Request) => string; - setBasePathFor: (request: KibanaRequest | Request, basePath: string) => void; + basePath: { + get: (request: KibanaRequest | Request) => string; + set: (request: KibanaRequest | Request, basePath: string) => void; + prepend: (url: string) => string; + remove: (url: string) => string; + }; auth: { get: AuthStateStorage['get']; isAuthenticated: AuthStateStorage['isAuthenticated']; @@ -80,7 +81,6 @@ export class HttpServer { private config?: HttpConfig; private registeredRouters = new Set(); private authRegistered = false; - private basePathCache = new WeakMap, string>(); private readonly authState: AuthStateStorage; @@ -101,33 +101,13 @@ export class HttpServer { this.registeredRouters.add(router); } - // passing hapi Request works for BWC. can be deleted once we remove legacy server. - private getBasePathFor(config: HttpConfig, request: KibanaRequest | Request) { - const incomingMessage = getIncomingMessage(request); - - const requestScopePath = this.basePathCache.get(incomingMessage) || ''; - const serverBasePath = config.basePath || ''; - return `${serverBasePath}${requestScopePath}`; - } - - // should work only for KibanaRequest as soon as spaces migrate to NP - private setBasePathFor(request: KibanaRequest | Request, basePath: string) { - const incomingMessage = getIncomingMessage(request); - - if (this.basePathCache.has(incomingMessage)) { - throw new Error( - 'Request basePath was previously set. Setting multiple times is not supported.' - ); - } - this.basePathCache.set(incomingMessage, basePath); - } - public setup(config: HttpConfig): HttpServerSetup { const serverOptions = getServerOptions(config); this.server = createServer(serverOptions); this.config = config; - this.setupBasePathRewrite(config); + const basePathService = new BasePath(config.basePath); + this.setupBasePathRewrite(config, basePathService); return { options: serverOptions, @@ -136,8 +116,7 @@ export class HttpServer { registerOnPostAuth: this.registerOnPostAuth.bind(this), registerAuth: (fn: AuthenticationHandler, cookieOptions: SessionStorageCookieOptions) => this.registerAuth(fn, cookieOptions, config.basePath), - getBasePathFor: this.getBasePathFor.bind(this, config), - setBasePathFor: this.setBasePathFor.bind(this), + basePath: basePathService, auth: { get: this.authState.get, isAuthenticated: this.authState.isAuthenticated, @@ -185,26 +164,19 @@ export class HttpServer { this.server = undefined; } - private setupBasePathRewrite(config: HttpConfig) { + private setupBasePathRewrite(config: HttpConfig, basePathService: BasePath) { if (config.basePath === undefined || !config.rewriteBasePath) { return; } - const basePath = config.basePath; this.registerOnPreAuth((request, toolkit) => { - const newURL = modifyUrl(request.url.href!, urlParts => { - if (urlParts.pathname != null && urlParts.pathname.startsWith(basePath)) { - urlParts.pathname = urlParts.pathname.replace(basePath, '') || '/'; - } else { - return {}; - } - }); - - if (!newURL) { - return toolkit.rejected(new Error('not found'), { statusCode: 404 }); + const oldUrl = request.url.href!; + const newURL = basePathService.remove(oldUrl); + const shouldRedirect = newURL !== oldUrl; + if (shouldRedirect) { + return toolkit.redirected(newURL, { forward: true }); } - - return toolkit.redirected(newURL, { forward: true }); + return toolkit.rejected(new Error('not found'), { statusCode: 404 }); }); } diff --git a/src/core/server/http/http_service.mock.ts b/src/core/server/http/http_service.mock.ts index 7dccacee8509af..07040560f89cd5 100644 --- a/src/core/server/http/http_service.mock.ts +++ b/src/core/server/http/http_service.mock.ts @@ -19,27 +19,34 @@ import { Server, ServerOptions } from 'hapi'; import { HttpService } from './http_service'; -import { HttpConfig } from './http_config'; import { HttpServerSetup } from './http_server'; +import { HttpServiceSetup } from './http_service'; +type ServiceSetupMockType = jest.Mocked & { + basePath: jest.Mocked; +}; const createSetupContractMock = () => { - const setupContract = { - options: {} as ServerOptions, + const setupContract: ServiceSetupMockType = { + options: ({} as unknown) as ServerOptions, + // we can mock some hapi server method when we need it + server: {} as Server, registerOnPreAuth: jest.fn(), registerAuth: jest.fn(), registerOnPostAuth: jest.fn(), registerRouter: jest.fn(), - getBasePathFor: jest.fn(), - setBasePathFor: jest.fn(), - // we can mock some hapi server method when we need it - server: {} as Server, + basePath: { + get: jest.fn(), + set: jest.fn(), + prepend: jest.fn(), + remove: jest.fn(), + }, auth: { get: jest.fn(), isAuthenticated: jest.fn(), }, - createNewServer: async (cfg: Partial): Promise => - ({} as HttpServerSetup), + createNewServer: jest.fn(), }; + setupContract.createNewServer.mockResolvedValue({} as HttpServerSetup); return setupContract; }; diff --git a/src/core/server/http/integration_tests/http_service.test.ts b/src/core/server/http/integration_tests/http_service.test.ts index 100efc4e2a607a..21207d0b90e25a 100644 --- a/src/core/server/http/integration_tests/http_service.test.ts +++ b/src/core/server/http/integration_tests/http_service.test.ts @@ -238,7 +238,7 @@ describe('http service', () => { }); }); - describe('#getBasePathFor()/#setBasePathFor()', () => { + describe('#basePath()', () => { let root: ReturnType; beforeEach(async () => { root = kbnTestServer.createRoot(); @@ -249,7 +249,7 @@ describe('http service', () => { const reqBasePath = '/requests-specific-base-path'; const { http } = await root.setup(); http.registerOnPreAuth((req, t) => { - http.setBasePathFor(req, reqBasePath); + http.basePath.set(req, reqBasePath); return t.next(); }); @@ -260,7 +260,7 @@ describe('http service', () => { kbnServer.server.route({ method: 'GET', path: legacyUrl, - handler: kbnServer.newPlatform.setup.core.http.getBasePathFor, + handler: kbnServer.newPlatform.setup.core.http.basePath.get, }); await kbnTestServer.request.get(root, legacyUrl).expect(200, reqBasePath); diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 4cebe24f39f155..f901b25e1ae874 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -91,8 +91,7 @@ export interface CoreSetup { registerOnPreAuth: HttpServiceSetup['registerOnPreAuth']; registerAuth: HttpServiceSetup['registerAuth']; registerOnPostAuth: HttpServiceSetup['registerOnPostAuth']; - getBasePathFor: HttpServiceSetup['getBasePathFor']; - setBasePathFor: HttpServiceSetup['setBasePathFor']; + basePath: HttpServiceSetup['basePath']; createNewServer: HttpServiceSetup['createNewServer']; }; } diff --git a/src/core/server/plugins/plugin_context.ts b/src/core/server/plugins/plugin_context.ts index 9dfc2df1c2d20d..10912ca6084856 100644 --- a/src/core/server/plugins/plugin_context.ts +++ b/src/core/server/plugins/plugin_context.ts @@ -120,8 +120,7 @@ export function createPluginSetupContext( registerOnPreAuth: deps.http.registerOnPreAuth, registerAuth: deps.http.registerAuth, registerOnPostAuth: deps.http.registerOnPostAuth, - getBasePathFor: deps.http.getBasePathFor, - setBasePathFor: deps.http.setBasePathFor, + basePath: deps.http.basePath, createNewServer: deps.http.createNewServer, }, }; diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 43712d63abb530..c0e1ce90287065 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -87,8 +87,7 @@ export interface CoreSetup { registerOnPreAuth: HttpServiceSetup['registerOnPreAuth']; registerAuth: HttpServiceSetup['registerAuth']; registerOnPostAuth: HttpServiceSetup['registerOnPostAuth']; - getBasePathFor: HttpServiceSetup['getBasePathFor']; - setBasePathFor: HttpServiceSetup['setBasePathFor']; + basePath: HttpServiceSetup['basePath']; createNewServer: HttpServiceSetup['createNewServer']; }; } diff --git a/src/core/utils/url.ts b/src/core/utils/url.ts index 1f566b4897d102..67b379e729ca46 100644 --- a/src/core/utils/url.ts +++ b/src/core/utils/url.ts @@ -20,15 +20,20 @@ import { ParsedUrlQuery } from 'querystring'; import { format as formatUrl, parse as parseUrl, UrlObject } from 'url'; +/** + * We define our own typings because the current version of @types/node + * declares properties to be optional "hostname?: string". + * Although, parse call returns "hostname: null | string". + */ export interface URLMeaningfulParts { - auth: string | null; - hash: string | null; - hostname: string | null; - pathname: string | null; - protocol: string | null; - slashes: boolean | null; - port: string | null; - query: ParsedUrlQuery | {}; + auth?: string | null; + hash?: string | null; + hostname?: string | null; + pathname?: string | null; + protocol?: string | null; + slashes?: boolean | null; + port?: string | null; + query: ParsedUrlQuery; } /** @@ -62,7 +67,7 @@ export interface URLMeaningfulParts { */ export function modifyUrl( url: string, - urlModifier: (urlParts: URLMeaningfulParts) => Partial | undefined + urlModifier: (urlParts: URLMeaningfulParts) => Partial | void ) { const parsed = parseUrl(url, true) as URLMeaningfulParts; diff --git a/src/legacy/server/http/setup_base_path_provider.js b/src/legacy/server/http/setup_base_path_provider.js index 8cf6cc1fde512d..c4873cb8da8b81 100644 --- a/src/legacy/server/http/setup_base_path_provider.js +++ b/src/legacy/server/http/setup_base_path_provider.js @@ -20,11 +20,11 @@ export function setupBasePathProvider(kbnServer) { kbnServer.server.decorate('request', 'setBasePath', function (basePath) { const request = this; - kbnServer.newPlatform.setup.core.http.setBasePathFor(request, basePath); + kbnServer.newPlatform.setup.core.http.basePath.set(request, basePath); }); kbnServer.server.decorate('request', 'getBasePath', function () { const request = this; - return kbnServer.newPlatform.setup.core.http.getBasePathFor(request); + return kbnServer.newPlatform.setup.core.http.basePath.get(request); }); } diff --git a/src/legacy/ui/public/chrome/api/base_path.test.ts b/src/legacy/ui/public/chrome/api/base_path.test.ts index 448872e87e4582..812635ba364838 100644 --- a/src/legacy/ui/public/chrome/api/base_path.test.ts +++ b/src/legacy/ui/public/chrome/api/base_path.test.ts @@ -26,40 +26,40 @@ function initChrome() { return chrome; } -newPlatformHttp.getBasePath.mockImplementation(() => 'gotBasePath'); -newPlatformHttp.prependBasePath.mockImplementation(() => 'addedToPath'); -newPlatformHttp.removeBasePath.mockImplementation(() => 'removedFromPath'); +newPlatformHttp.basePath.get.mockImplementation(() => 'gotBasePath'); +newPlatformHttp.basePath.prepend.mockImplementation(() => 'addedToPath'); +newPlatformHttp.basePath.remove.mockImplementation(() => 'removedFromPath'); beforeEach(() => { jest.clearAllMocks(); }); describe('#getBasePath()', () => { - it('proxies to newPlatformHttp.getBasePath()', () => { + it('proxies to newPlatformHttp.basePath.get()', () => { const chrome = initChrome(); - expect(newPlatformHttp.prependBasePath).not.toHaveBeenCalled(); + expect(newPlatformHttp.basePath.prepend).not.toHaveBeenCalled(); expect(chrome.getBasePath()).toBe('gotBasePath'); - expect(newPlatformHttp.getBasePath).toHaveBeenCalledTimes(1); - expect(newPlatformHttp.getBasePath).toHaveBeenCalledWith(); + expect(newPlatformHttp.basePath.get).toHaveBeenCalledTimes(1); + expect(newPlatformHttp.basePath.get).toHaveBeenCalledWith(); }); }); describe('#addBasePath()', () => { - it('proxies to newPlatformHttp.prependBasePath(path)', () => { + it('proxies to newPlatformHttp.basePath.prepend(path)', () => { const chrome = initChrome(); - expect(newPlatformHttp.prependBasePath).not.toHaveBeenCalled(); + expect(newPlatformHttp.basePath.prepend).not.toHaveBeenCalled(); expect(chrome.addBasePath('foo/bar')).toBe('addedToPath'); - expect(newPlatformHttp.prependBasePath).toHaveBeenCalledTimes(1); - expect(newPlatformHttp.prependBasePath).toHaveBeenCalledWith('foo/bar'); + expect(newPlatformHttp.basePath.prepend).toHaveBeenCalledTimes(1); + expect(newPlatformHttp.basePath.prepend).toHaveBeenCalledWith('foo/bar'); }); }); describe('#removeBasePath', () => { - it('proxies to newPlatformBasePath.removeBasePath(path)', () => { + it('proxies to newPlatformBasePath.basePath.remove(path)', () => { const chrome = initChrome(); - expect(newPlatformHttp.removeBasePath).not.toHaveBeenCalled(); + expect(newPlatformHttp.basePath.remove).not.toHaveBeenCalled(); expect(chrome.removeBasePath('foo/bar')).toBe('removedFromPath'); - expect(newPlatformHttp.removeBasePath).toHaveBeenCalledTimes(1); - expect(newPlatformHttp.removeBasePath).toHaveBeenCalledWith('foo/bar'); + expect(newPlatformHttp.basePath.remove).toHaveBeenCalledTimes(1); + expect(newPlatformHttp.basePath.remove).toHaveBeenCalledWith('foo/bar'); }); }); diff --git a/src/legacy/ui/public/chrome/api/base_path.ts b/src/legacy/ui/public/chrome/api/base_path.ts index cb342627f09d53..da823425b7c226 100644 --- a/src/legacy/ui/public/chrome/api/base_path.ts +++ b/src/legacy/ui/public/chrome/api/base_path.ts @@ -22,7 +22,7 @@ import { npSetup } from 'ui/new_platform'; const newPlatformHttp = npSetup.core.http; export function initChromeBasePathApi(chrome: { [key: string]: any }) { - chrome.getBasePath = newPlatformHttp.getBasePath.bind(newPlatformHttp); - chrome.addBasePath = newPlatformHttp.prependBasePath.bind(newPlatformHttp); - chrome.removeBasePath = newPlatformHttp.removeBasePath.bind(newPlatformHttp); + chrome.getBasePath = newPlatformHttp.basePath.get; + chrome.addBasePath = newPlatformHttp.basePath.prepend; + chrome.removeBasePath = newPlatformHttp.basePath.remove; } diff --git a/src/legacy/ui/public/legacy_compat/angular_config.tsx b/src/legacy/ui/public/legacy_compat/angular_config.tsx index d4386a5d131151..1e22003b328338 100644 --- a/src/legacy/ui/public/legacy_compat/angular_config.tsx +++ b/src/legacy/ui/public/legacy_compat/angular_config.tsx @@ -79,7 +79,7 @@ export const configureAppAngularModule = (angularModule: IModule) => { const getEsUrl = (newPlatform: InternalCoreStart) => { const a = document.createElement('a'); - a.href = newPlatform.http.prependBasePath('/elasticsearch'); + a.href = newPlatform.http.basePath.prepend('/elasticsearch'); const protocolPort = /https/.test(a.protocol) ? 443 : 80; const port = a.port || protocolPort; return { diff --git a/src/legacy/ui/public/notify/app_redirect/app_redirect.js b/src/legacy/ui/public/notify/app_redirect/app_redirect.js index d3f66c1eb59acd..a92e5401e5e755 100644 --- a/src/legacy/ui/public/notify/app_redirect/app_redirect.js +++ b/src/legacy/ui/public/notify/app_redirect/app_redirect.js @@ -17,7 +17,7 @@ * under the License. */ -import { modifyUrl } from '../../../../../core/public/utils'; +import { modifyUrl } from '../../../../../core/utils'; import { toastNotifications } from '../toasts'; const APP_REDIRECT_MESSAGE_PARAM = 'app_redirect_message'; diff --git a/src/legacy/ui/public/url/kibana_parsed_url.ts b/src/legacy/ui/public/url/kibana_parsed_url.ts index d431c775d1d2e6..93d2e17d6038f6 100644 --- a/src/legacy/ui/public/url/kibana_parsed_url.ts +++ b/src/legacy/ui/public/url/kibana_parsed_url.ts @@ -19,7 +19,7 @@ import { parse } from 'url'; -import { modifyUrl } from '../../../../core/public/utils'; +import { modifyUrl } from '../../../../core/utils'; import { prependPath } from './prepend_path'; interface Options { From 399adcddcd8094033d7ea85414301da243debb48 Mon Sep 17 00:00:00 2001 From: Angela Chuang <6295984+angorayc@users.noreply.github.com> Date: Mon, 17 Jun 2019 01:29:40 +0800 Subject: [PATCH 03/10] [SIEM] add middleware for handling refetch (#38697) * add middleware for handling refetch * add unit test for error links * add unit test --- .../public/containers/errors/index.test.tsx | 108 ++++++++++++++++++ .../siem/public/containers/errors/index.tsx | 26 ++++- .../siem/public/lib/compose/helpers.test.ts | 40 +++++++ .../siem/public/lib/compose/helpers.ts | 27 +++++ .../siem/public/lib/compose/kibana_compose.ts | 19 +-- 5 files changed, 199 insertions(+), 21 deletions(-) create mode 100644 x-pack/plugins/siem/public/containers/errors/index.test.tsx create mode 100644 x-pack/plugins/siem/public/lib/compose/helpers.test.ts create mode 100644 x-pack/plugins/siem/public/lib/compose/helpers.ts diff --git a/x-pack/plugins/siem/public/containers/errors/index.test.tsx b/x-pack/plugins/siem/public/containers/errors/index.test.tsx new file mode 100644 index 00000000000000..e1b192df104d74 --- /dev/null +++ b/x-pack/plugins/siem/public/containers/errors/index.test.tsx @@ -0,0 +1,108 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { reTryOneTimeOnErrorHandler, errorLinkHandler } from '.'; +import { ServerError } from 'apollo-link-http-common'; +import { Operation } from 'apollo-link'; +import { GraphQLError } from 'graphql'; +import * as store from '../../store'; +import { onError } from 'apollo-link-error'; + +const mockDispatch = jest.fn(); +jest.mock('apollo-link-error'); +jest.mock('../../store'); +// @ts-ignore +store.getStore.mockReturnValue({ dispatch: mockDispatch }); + +describe('errorLinkHandler', () => { + const mockGraphQLErrors: GraphQLError = { + message: 'GraphQLError', + } as GraphQLError; + const mockNetworkError: ServerError = { + result: {}, + statusCode: 503, + name: '', + message: 'error', + response: { + ok: false, + } as Response, + }; + const mockOperation: Operation = {} as Operation; + const mockForward = jest.fn(); + + afterEach(() => { + mockDispatch.mockClear(); + }); + + test('it should display error if graphQLErrors exist', () => { + errorLinkHandler({ + graphQLErrors: [mockGraphQLErrors], + operation: mockOperation, + forward: mockForward, + }); + + expect(store.getStore).toBeCalled(); + expect(mockDispatch.mock.calls.length).toBe(1); + }); + + test('it should display error if networkError exist', () => { + errorLinkHandler({ + networkError: mockNetworkError, + operation: mockOperation, + forward: mockForward, + }); + + expect(store.getStore).toBeCalled(); + expect(mockDispatch.mock.calls.length).toBe(1); + }); +}); + +describe('errorLink', () => { + test('onError should be called with errorLinkHandler', () => { + expect(onError).toHaveBeenCalledWith(errorLinkHandler); + }); +}); + +describe('reTryOneTimeOnErrorHandler', () => { + const mockNetworkError: ServerError = { + result: {}, + statusCode: 503, + name: '', + message: 'error', + response: { + ok: false, + } as Response, + }; + const mockOperation: Operation = {} as Operation; + const mockForward = jest.fn(); + + afterEach(() => { + mockForward.mockClear(); + }); + test('it should retry only if network status code is 503', () => { + reTryOneTimeOnErrorHandler({ + networkError: mockNetworkError, + operation: mockOperation, + forward: mockForward, + }); + expect(mockForward).toBeCalledWith(mockOperation); + }); + + test('it should not retry if other error happens', () => { + reTryOneTimeOnErrorHandler({ + networkError: { ...mockNetworkError, statusCode: 500 }, + operation: mockOperation, + forward: mockForward, + }); + expect(mockForward).not.toBeCalled(); + }); +}); + +describe('reTryOneTimeOnErrorLink', () => { + test('onError should be called with reTryOneTimeOnErrorHandler', () => { + expect(onError).toHaveBeenCalledWith(reTryOneTimeOnErrorHandler); + }); +}); diff --git a/x-pack/plugins/siem/public/containers/errors/index.tsx b/x-pack/plugins/siem/public/containers/errors/index.tsx index c7be81e2c4b617..01be648924b7d3 100644 --- a/x-pack/plugins/siem/public/containers/errors/index.tsx +++ b/x-pack/plugins/siem/public/containers/errors/index.tsx @@ -4,8 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import { onError } from 'apollo-link-error'; -import { throttle, noop } from 'lodash/fp'; +import { onError, ErrorLink } from 'apollo-link-error'; +import { get, throttle, noop } from 'lodash/fp'; + import uuid from 'uuid'; import * as i18n from './translations'; @@ -13,9 +14,10 @@ import * as i18n from './translations'; import { getStore } from '../../store'; import { appActions } from '../../store/actions'; -export const errorLink = onError(({ graphQLErrors, networkError }) => { +export const errorLinkHandler: ErrorLink.ErrorHandler = ({ graphQLErrors, networkError }) => { const store = getStore(); const dispatch = throttle(50, store != null ? store.dispatch : noop); + if (graphQLErrors != null && store != null) { dispatch( appActions.addError({ @@ -35,4 +37,20 @@ export const errorLink = onError(({ graphQLErrors, networkError }) => { }) ); } -}); +}; +export const errorLink = onError(errorLinkHandler); + +export const reTryOneTimeOnErrorHandler: ErrorLink.ErrorHandler = ({ + networkError, + operation, + forward, +}) => { + if (networkError != null) { + const statusCode = get('statusCode', networkError); + if (statusCode != null && statusCode === 503) { + return forward(operation); + } + } +}; + +export const reTryOneTimeOnErrorLink = onError(reTryOneTimeOnErrorHandler); diff --git a/x-pack/plugins/siem/public/lib/compose/helpers.test.ts b/x-pack/plugins/siem/public/lib/compose/helpers.test.ts new file mode 100644 index 00000000000000..b135e299e2e432 --- /dev/null +++ b/x-pack/plugins/siem/public/lib/compose/helpers.test.ts @@ -0,0 +1,40 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { InMemoryCache, IntrospectionFragmentMatcher } from 'apollo-cache-inmemory'; +import { errorLink, reTryOneTimeOnErrorLink } from '../../containers/errors'; +import { getLinks } from './helpers'; +import { withClientState } from 'apollo-link-state'; +import * as apolloLinkHttp from 'apollo-link-http'; +import introspectionQueryResultData from '../../graphql/introspection.json'; + +jest.mock('apollo-cache-inmemory'); +jest.mock('apollo-link-http'); +jest.mock('apollo-link-state'); +jest.mock('../../containers/errors'); +const mockWithClientState = 'mockWithClientState'; +const mockHttpLink = { mockHttpLink: 'mockHttpLink' }; + +// @ts-ignore +withClientState.mockReturnValue(mockWithClientState); +// @ts-ignore +apolloLinkHttp.HttpLink.mockImplementation(() => mockHttpLink); + +describe('getLinks helper', () => { + test('It should return links in correct order', () => { + const mockCache = new InMemoryCache({ + dataIdFromObject: () => null, + fragmentMatcher: new IntrospectionFragmentMatcher({ + introspectionQueryResultData, + }), + }); + const links = getLinks(mockCache); + expect(links[0]).toEqual(errorLink); + expect(links[1]).toEqual(reTryOneTimeOnErrorLink); + expect(links[2]).toEqual(mockWithClientState); + expect(links[3]).toEqual(mockHttpLink); + }); +}); diff --git a/x-pack/plugins/siem/public/lib/compose/helpers.ts b/x-pack/plugins/siem/public/lib/compose/helpers.ts new file mode 100644 index 00000000000000..0bd0e8d4ec7bc0 --- /dev/null +++ b/x-pack/plugins/siem/public/lib/compose/helpers.ts @@ -0,0 +1,27 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { HttpLink } from 'apollo-link-http'; +import { withClientState } from 'apollo-link-state'; +import { InMemoryCache } from 'apollo-cache-inmemory'; +import chrome from 'ui/chrome'; +import { errorLink, reTryOneTimeOnErrorLink } from '../../containers/errors'; + +export const getLinks = (cache: InMemoryCache) => [ + errorLink, + reTryOneTimeOnErrorLink, + withClientState({ + cache, + resolvers: {}, + }), + new HttpLink({ + credentials: 'same-origin', + headers: { + 'kbn-xsrf': chrome.getXsrfToken(), + }, + uri: `${chrome.getBasePath()}/api/siem/graphql`, + }), +]; diff --git a/x-pack/plugins/siem/public/lib/compose/kibana_compose.ts b/x-pack/plugins/siem/public/lib/compose/kibana_compose.ts index 0fdc17effd498a..af488150d876bc 100644 --- a/x-pack/plugins/siem/public/lib/compose/kibana_compose.ts +++ b/x-pack/plugins/siem/public/lib/compose/kibana_compose.ts @@ -7,8 +7,6 @@ import { InMemoryCache, IntrospectionFragmentMatcher } from 'apollo-cache-inmemory'; import ApolloClient from 'apollo-client'; import { ApolloLink } from 'apollo-link'; -import { HttpLink } from 'apollo-link-http'; -import { withClientState } from 'apollo-link-state'; import 'ui/autoload/all'; // @ts-ignore: path dynamic for kibana import chrome from 'ui/chrome'; @@ -18,11 +16,11 @@ import uiRoutes from 'ui/routes'; // @ts-ignore: path dynamic for kibana import { timezoneProvider } from 'ui/vis/lib/timezone'; -import { errorLink } from '../../containers/errors'; import introspectionQueryResultData from '../../graphql/introspection.json'; import { AppKibanaFrameworkAdapter } from '../adapters/framework/kibana_framework_adapter'; import { AppKibanaObservableApiAdapter } from '../adapters/observable_api/kibana_observable_api'; import { AppFrontendLibs } from '../lib'; +import { getLinks } from './helpers'; export function compose(): AppFrontendLibs { const cache = new InMemoryCache({ @@ -40,20 +38,7 @@ export function compose(): AppFrontendLibs { const graphQLOptions = { connectToDevTools: process.env.NODE_ENV !== 'production', cache, - link: ApolloLink.from([ - errorLink, - withClientState({ - cache, - resolvers: {}, - }), - new HttpLink({ - credentials: 'same-origin', - headers: { - 'kbn-xsrf': chrome.getXsrfToken(), - }, - uri: `${chrome.getBasePath()}/api/siem/graphql`, - }), - ]), + link: ApolloLink.from(getLinks(cache)), }; const apolloClient = new ApolloClient(graphQLOptions); From 0351559aa8177b068c935215182cc54e3f57470e Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 17 Jun 2019 11:44:57 +0200 Subject: [PATCH 04/10] Set XAxis ordered.interval differently for date and numeric values in histogram (#38824) * Add test for numeric interval values --- .../point_series/__tests__/_init_x_axis.js | 14 +++++++++++++- .../agg_response/point_series/_init_x_axis.js | 3 ++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/legacy/ui/public/agg_response/point_series/__tests__/_init_x_axis.js b/src/legacy/ui/public/agg_response/point_series/__tests__/_init_x_axis.js index a3ef8810b6203b..2defd090f9b370 100644 --- a/src/legacy/ui/public/agg_response/point_series/__tests__/_init_x_axis.js +++ b/src/legacy/ui/public/agg_response/point_series/__tests__/_init_x_axis.js @@ -106,8 +106,9 @@ describe('initXAxis', function () { }); }); - it('reads the interval param from the x agg', function () { + it('reads the date interval param from the x agg', function () { chart.aspects.x[0].params.interval = 'P1D'; + chart.aspects.x[0].params.date = true; initXAxis(chart, table); expect(chart) .to.have.property('xAxisLabel', 'label') @@ -117,4 +118,15 @@ describe('initXAxis', function () { expect(moment.isDuration(chart.ordered.interval)).to.be(true); expect(chart.ordered.interval.toISOString()).to.eql('P1D'); }); + + it('reads the numeric interval param from the x agg', function () { + chart.aspects.x[0].params.interval = 0.5; + initXAxis(chart, table); + expect(chart) + .to.have.property('xAxisLabel', 'label') + .and.have.property('xAxisFormat', chart.aspects.x[0].format) + .and.have.property('ordered'); + + expect(chart.ordered.interval).to.eql(0.5); + }); }); diff --git a/src/legacy/ui/public/agg_response/point_series/_init_x_axis.js b/src/legacy/ui/public/agg_response/point_series/_init_x_axis.js index ddd6d6bc4bb4ff..6a2ca698901f67 100644 --- a/src/legacy/ui/public/agg_response/point_series/_init_x_axis.js +++ b/src/legacy/ui/public/agg_response/point_series/_init_x_axis.js @@ -28,9 +28,10 @@ export function initXAxis(chart, table) { : uniq(table.rows.map(r => r[accessor])); chart.xAxisFormat = format; chart.xAxisLabel = title; + if (params.interval) { chart.ordered = { - interval: moment.duration(params.interval), + interval: params.date ? moment.duration(params.interval) : params.interval, }; } } From 2228863440773c8335cc5a70e24d7d7d74e9cbd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20St=C3=BCrmer?= Date: Mon, 17 Jun 2019 06:33:11 -0400 Subject: [PATCH 05/10] [Infra/Logs UI][skip ci] Add infra plugin readme with contribution notes (#38696) This adds a README file to the `infra` plugin directory with additional contribution notes. Co-Authored-By: Kerry Gallagher --- x-pack/plugins/infra/README.md | 130 +++++++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 x-pack/plugins/infra/README.md diff --git a/x-pack/plugins/infra/README.md b/x-pack/plugins/infra/README.md new file mode 100644 index 00000000000000..6287b686dad235 --- /dev/null +++ b/x-pack/plugins/infra/README.md @@ -0,0 +1,130 @@ +# The `infra` plugin + +This is the home of the `infra` plugin, which aims to provide a solution for +the infrastructure monitoring use-case within Kibana. + +## UI Structure + +The plugin provides two main apps in Kibana - the *Infrastructure UI* and the +*Logs UI*. Both are reachable via their own main navigation items and via links +from other parts of Kibana. + +The *Infrastructure UI* consists of three main screens, which are the +*Inventory*, the *Node details* and the *Metrics explorer*. + +The *Logs UI* provides one log viewer screen. + +## Communicating + +In order to address the whole infrastructure monitoring team, the +@elastic/infra-logs-ui team alias can be used as a mention or in review +requests. + +The [Infrastructure forum] and [Logs forum] on Discuss are frequented by the +team as well. + +## Contributing + +Since the `infra` plugin lives within the Kibana repository, [Kibana's +contribution procedures](../../../CONTRIBUTING.md) apply. In addition to that, +this section details a few plugin-specific aspects. + +### Ingesting metrics for development + +The *Infrastructure UI* displays [ECS]-compatible metric data from indices +matching the `metricbeat-*` pattern by default. The primary way to ingest these +is by running [Metricbeat] to deliver metrics to the development Elasticsearch +cluster. It can be used to ingest development host metrics using the `system` +module. + +A setup that ingests docker and nginx metrics is described in +[./docs/test_setups/infra_metricbeat_docker_nginx.md]. + +### Ingesting logs for development + +Similarly, the *Logs UI* assumes [ECS]-compatible log data to be present in +indices matching the `filebeat-*` pattern. At the time of writing the minimum +required fields are `@timestamp` and `message`, but the presence of other [ECS] +fields enable additional functionality such as linking to and from other +solutions. + +The primary way to ingest such log data is via [Filebeat]. A convenient source +of log entries are the Kibana and Elasticsearch log files produced by the +development environment itself. These can easily be consumed by enabling the modules + +``` +$ filebeat modules enable elasticsearch +$ filebeat modules enable kibana +``` + +and then editing the `modules.d/elasticsearch.yml` and `modules.d/kibana.yml` +to change the `var.paths` settings to contain paths to the development +environment's log files, e.g.: + +``` +- module: elasticsearch + server: + enabled: true + var.paths: + - "${WORK_ENVIRONMENT}/kibana/.es/8.0.0/logs/elasticsearch_server.json" + var.convert_timezone: true +``` + +### Creating PRs + +As with all of Kibana, we welcome contributions from everyone. The usual +life-cycle of a PR looks like the following: + +1. **Create draft PR**: To make ongoing work visible, we recommend creating + [draft PRs] as soon as possible. PRs are usually targetted at `master` and + backported later. The checklist in the PR description template can be used + to guide the progress of the PR. +2. **Label the PR**: To ensure that a newly created PR gets the attention of + the @elastic/infra-logs-ui team, the following label should be applied to + PRs: + * `Team:infra-logs-ui` + * `Feature:Infra UI` if it relates to the *Intrastructure UI* + * `Feature:Logs UI` if it relates to the *Logs UI* + * `[zube]: In Progress` to track the stage of the PR + * Version labels for merge and backport targets (see [Kibana's contribution + procedures]), usually: + * the version that `master` currently represents + * the version of the next minor release + * Release note labels (see [Kibana's contribution procedures]) + * `release_note:enhancement` if the PR contains a new feature or enhancement + * `release_note:fix` if the PR contains an external-facing fix + * `release_note:breaking` if the PR contains a breaking change + * `release_note:deprecation` if the PR contains deprecations of publicly + documented features. + * `release_note:skip` if the PR contains only house-keeping changes, fixes + to unreleased code or documentation changes +3. **Satisfy CI**: The PR will automatically be picked up by the CI system, + which will run the full test suite as well as some additional checks. A + comment containing `jenkins, test this` can be used to manually trigger a CI + run. The result will be reported on the PR itself. Out of courtesy for the + reviewers the checks should pass before requesting reviews. +4. **Request reviews**: Once the PR is ready for reviews it can be marked as + such by [changing the PR state to ready]. In addition the label `[zube]: In + Progress` should be replaced with `[zube]: In Review` and `review`. If the + GitHub automation doesn't automatically request a review from + `@elastic/infra-logs-ui` it should be requested manually. +5. **Incorporate review feedback**: Usually one reviewer's approval is + sufficient. Particularly complicated or cross-cutting concerns might warrant + multiple reviewers. +6. **Merge**: Once CI is green and the reviewers are approve, PRs in the Kibana + repo are "squash-merged" to `master` to keep the history clean. +7. **Backport**: After merging to `master`, the PR is backported to the + branches that represent the versions indicated by the labels. The `yarn + backport` command can be used to automate most of the process. + +There are always exceptions to the rule, so seeking guidance about any of the +steps is highly recommended. + +[Kibana's contribution procedures]: ../../../CONTRIBUTING.md +[Infrastructure forum]: https://discuss.elastic.co/c/infrastructure +[Logs forum]: https://discuss.elastic.co/c/logs +[ECS]: https://github.com/elastic/ecs/ +[Metricbeat]: https://www.elastic.co/products/beats/metricbeat +[Filebeat]: https://www.elastic.co/products/beats/filebeat +[draft PRs]: https://help.github.com/en/articles/about-pull-requests#draft-pull-requests +[changing the PR state to ready]: https://help.github.com/en/articles/changing-the-stage-of-a-pull-request From 5b9c231a51a0e9b14b1dbe31f717e987cba9a5c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20St=C3=BCrmer?= Date: Mon, 17 Jun 2019 06:43:04 -0400 Subject: [PATCH 06/10] [Infra UI] Make queries more robust against missing indices (#38976) This fixes several queries that failed when indices didn't exist and were not specified using a wildcard at the same time. --- .../lib/adapters/metadata/elasticsearch_metadata_adapter.ts | 4 ++++ .../infra/server/lib/adapters/metrics/lib/check_valid_node.ts | 2 ++ .../infra/server/routes/metrics_explorer/lib/get_groupings.ts | 2 ++ 3 files changed, 8 insertions(+) diff --git a/x-pack/plugins/infra/server/lib/adapters/metadata/elasticsearch_metadata_adapter.ts b/x-pack/plugins/infra/server/lib/adapters/metadata/elasticsearch_metadata_adapter.ts index b48ffb18947e5c..a648e87e4f2424 100644 --- a/x-pack/plugins/infra/server/lib/adapters/metadata/elasticsearch_metadata_adapter.ts +++ b/x-pack/plugins/infra/server/lib/adapters/metadata/elasticsearch_metadata_adapter.ts @@ -28,6 +28,8 @@ export class ElasticsearchMetadataAdapter implements InfraMetadataAdapter { ): Promise { const idFieldName = getIdFieldName(sourceConfiguration, nodeType); const metricQuery = { + allowNoIndices: true, + ignoreUnavailable: true, index: sourceConfiguration.metricAlias, body: { query: { @@ -80,6 +82,8 @@ export class ElasticsearchMetadataAdapter implements InfraMetadataAdapter { ): Promise { const idFieldName = getIdFieldName(sourceConfiguration, nodeType); const logQuery = { + allowNoIndices: true, + ignoreUnavailable: true, index: sourceConfiguration.logAlias, body: { query: { diff --git a/x-pack/plugins/infra/server/lib/adapters/metrics/lib/check_valid_node.ts b/x-pack/plugins/infra/server/lib/adapters/metrics/lib/check_valid_node.ts index 88847acb290875..bca509334b6924 100644 --- a/x-pack/plugins/infra/server/lib/adapters/metrics/lib/check_valid_node.ts +++ b/x-pack/plugins/infra/server/lib/adapters/metrics/lib/check_valid_node.ts @@ -13,6 +13,8 @@ export const checkValidNode = async ( id: string ): Promise => { const params = { + allowNoIndices: true, + ignoreUnavailable: true, index: indexPattern, terminateAfter: 1, body: { diff --git a/x-pack/plugins/infra/server/routes/metrics_explorer/lib/get_groupings.ts b/x-pack/plugins/infra/server/routes/metrics_explorer/lib/get_groupings.ts index 45580a64b358cf..7ab0a97ff99cc6 100644 --- a/x-pack/plugins/infra/server/routes/metrics_explorer/lib/get_groupings.ts +++ b/x-pack/plugins/infra/server/routes/metrics_explorer/lib/get_groupings.ts @@ -32,6 +32,8 @@ export const getGroupings = async ( } const limit = options.limit || 9; const params = { + allowNoIndices: true, + ignoreUnavailable: true, index: options.indexPattern, body: { size: 0, From b36aad350a4aea8eb03b87d4129ccb14cbf1b5bd Mon Sep 17 00:00:00 2001 From: Ahmad Bamieh Date: Mon, 17 Jun 2019 16:03:12 +0300 Subject: [PATCH 07/10] [i18n] extract untracked translations and prettier logging (#35171) * extract untracked translations and prettier logging * self code review * Update src/dev/run_i18n_check.ts Co-Authored-By: Bamieh * updating listR * run new i18n_check and fix errors * kbnEmbeddables -> embeddableApi * remove any type * Update src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/add_panel/add_panel_flyout.tsx Co-Authored-By: Stacey Gammon * Update src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/add_panel/add_panel_action.tsx Co-Authored-By: Stacey Gammon * self code review fixes * fix extract and integrate scripts * ts-ignore js file --- .i18nrc.json | 2 + src/dev/i18n/extract_default_translations.js | 79 ++++++++----- src/dev/i18n/index.ts | 2 + src/dev/i18n/integrate_locale_files.ts | 2 +- src/dev/i18n/tasks/check_compatibility.ts | 48 ++++++++ .../tasks/extract_default_translations.ts | 57 ++++----- .../tasks/extract_untracked_translations.ts | 109 ++++++++++++++++++ src/dev/i18n/tasks/index.ts | 2 + src/dev/run_i18n_check.ts | 66 ++++++----- src/dev/run_i18n_extract.ts | 54 +++++++-- src/dev/run_i18n_integrate.ts | 48 ++++++-- .../build_eui_context_menu_panels.ts | 2 +- .../add_panel/add_panel_action.tsx | 2 +- .../add_panel/add_panel_flyout.tsx | 10 +- .../customize_panel_action.tsx | 2 +- .../customize_title/customize_panel_modal.tsx | 18 ++- .../customize_title/customize_title_form.tsx | 6 +- .../panel_actions/edit_panel_action.tsx | 2 +- .../panel_actions/inspect_panel_action.tsx | 2 +- .../panel_actions/remove_panel_action.tsx | 2 +- .../panel/panel_header/panel_header.tsx | 2 +- .../panel/panel_header/panel_options_menu.tsx | 2 +- .../contact_card_embeddable_factory.tsx | 2 +- .../filterable_container_factory.ts | 2 +- .../filterable_embeddable_factory.ts | 2 +- .../hello_world_embeddable_factory.ts | 2 +- 26 files changed, 381 insertions(+), 146 deletions(-) create mode 100644 src/dev/i18n/tasks/check_compatibility.ts create mode 100644 src/dev/i18n/tasks/extract_untracked_translations.ts diff --git a/.i18nrc.json b/.i18nrc.json index aa6d953a257c11..f7152c485ca581 100644 --- a/.i18nrc.json +++ b/.i18nrc.json @@ -10,6 +10,7 @@ "interpreter": "src/legacy/core_plugins/interpreter", "kbn": "src/legacy/core_plugins/kibana", "kbnDocViews": "src/legacy/core_plugins/kbn_doc_views", + "embeddableApi": "src/legacy/core_plugins/embeddable_api", "kbnVislibVisTypes": "src/legacy/core_plugins/kbn_vislib_vis_types", "markdownVis": "src/legacy/core_plugins/markdown_vis", "metricVis": "src/legacy/core_plugins/metric_vis", @@ -25,6 +26,7 @@ "xpack.apm": "x-pack/plugins/apm", "xpack.beatsManagement": "x-pack/plugins/beats_management", "xpack.canvas": "x-pack/plugins/canvas", + "xpack.code": "x-pack/plugins/code", "xpack.crossClusterReplication": "x-pack/plugins/cross_cluster_replication", "xpack.dashboardMode": "x-pack/plugins/dashboard_mode", "xpack.graph": "x-pack/plugins/graph", diff --git a/src/dev/i18n/extract_default_translations.js b/src/dev/i18n/extract_default_translations.js index 9a992d7406ef47..a449d19bc6e873 100644 --- a/src/dev/i18n/extract_default_translations.js +++ b/src/dev/i18n/extract_default_translations.js @@ -62,11 +62,20 @@ See .i18nrc.json for the list of supported namespaces.`) } } -export async function extractMessagesFromPathToMap(inputPath, targetMap, config, reporter) { +export async function matchEntriesWithExctractors(inputPath, options = {}) { + const { + additionalIgnore = [], + mark = false, + absolute = false, + } = options; + const ignore = ['**/node_modules/**', '**/__tests__/**', '**/*.test.{js,jsx,ts,tsx}', '**/*.d.ts'].concat(additionalIgnore); + const entries = await globAsync('*.{js,jsx,pug,ts,tsx,html}', { cwd: inputPath, matchBase: true, - ignore: ['**/node_modules/**', '**/__tests__/**', '**/*.test.{js,jsx,ts,tsx}', '**/*.d.ts'], + ignore, + mark, + absolute, }); const { htmlEntries, codeEntries, pugEntries } = entries.reduce( @@ -86,37 +95,43 @@ export async function extractMessagesFromPathToMap(inputPath, targetMap, config, { htmlEntries: [], codeEntries: [], pugEntries: [] } ); - await Promise.all( - [ - [htmlEntries, extractHtmlMessages], - [codeEntries, extractCodeMessages], - [pugEntries, extractPugMessages], - ].map(async ([entries, extractFunction]) => { - const files = await Promise.all( - filterEntries(entries, config.exclude).map(async entry => { - return { - name: entry, - content: await readFileAsync(entry), - }; - }) - ); - - for (const { name, content } of files) { - const reporterWithContext = reporter.withContext({ name }); - - try { - for (const [id, value] of extractFunction(content, reporterWithContext)) { - validateMessageNamespace(id, name, config.paths, reporterWithContext); - addMessageToMap(targetMap, id, value, reporterWithContext); - } - } catch (error) { - if (!isFailError(error)) { - throw error; - } + return [ + [htmlEntries, extractHtmlMessages], + [codeEntries, extractCodeMessages], + [pugEntries, extractPugMessages], + ]; +} - reporterWithContext.report(error); +export async function extractMessagesFromPathToMap(inputPath, targetMap, config, reporter) { + const categorizedEntries = await matchEntriesWithExctractors(inputPath); + return Promise.all( + categorizedEntries + .map(async ([entries, extractFunction]) => { + const files = await Promise.all( + filterEntries(entries, config.exclude).map(async entry => { + return { + name: entry, + content: await readFileAsync(entry), + }; + }) + ); + + for (const { name, content } of files) { + const reporterWithContext = reporter.withContext({ name }); + + try { + for (const [id, value] of extractFunction(content, reporterWithContext)) { + validateMessageNamespace(id, name, config.paths, reporterWithContext); + addMessageToMap(targetMap, id, value, reporterWithContext); + } + } catch (error) { + if (!isFailError(error)) { + throw error; + } + + reporterWithContext.report(error); + } } - } - }) + }) ); } diff --git a/src/dev/i18n/index.ts b/src/dev/i18n/index.ts index 604d7bab72c926..8f3e5955097399 100644 --- a/src/dev/i18n/index.ts +++ b/src/dev/i18n/index.ts @@ -20,6 +20,8 @@ // @ts-ignore export { extractMessagesFromPathToMap } from './extract_default_translations'; // @ts-ignore +export { matchEntriesWithExctractors } from './extract_default_translations'; +// @ts-ignore export { writeFileAsync, readFileAsync, normalizePath, ErrorReporter } from './utils'; export { serializeToJson, serializeToJson5 } from './serializers'; export { I18nConfig, filterConfigPaths, mergeConfigs } from './config'; diff --git a/src/dev/i18n/integrate_locale_files.ts b/src/dev/i18n/integrate_locale_files.ts index b090eedce443d0..7ed5e788be2985 100644 --- a/src/dev/i18n/integrate_locale_files.ts +++ b/src/dev/i18n/integrate_locale_files.ts @@ -37,7 +37,7 @@ import { createFailError } from '../run'; import { I18nConfig } from './config'; import { serializeToJson } from './serializers'; -interface IntegrateOptions { +export interface IntegrateOptions { sourceFileName: string; targetFileName?: string; dryRun: boolean; diff --git a/src/dev/i18n/tasks/check_compatibility.ts b/src/dev/i18n/tasks/check_compatibility.ts new file mode 100644 index 00000000000000..3c5f8c23466a47 --- /dev/null +++ b/src/dev/i18n/tasks/check_compatibility.ts @@ -0,0 +1,48 @@ +/* + * 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 { ToolingLog } from '@kbn/dev-utils'; +import { integrateLocaleFiles, I18nConfig } from '..'; + +export interface I18nFlags { + fix: boolean; + ignoreIncompatible: boolean; + ignoreUnused: boolean; + ignoreMissing: boolean; +} + +export function checkCompatibility(config: I18nConfig, flags: I18nFlags, log: ToolingLog) { + const { fix, ignoreIncompatible, ignoreUnused, ignoreMissing } = flags; + return config.translations.map(translationsPath => ({ + task: async ({ messages }: { messages: Map }) => { + // If `fix` is set we should try apply all possible fixes and override translations file. + await integrateLocaleFiles(messages, { + dryRun: !fix, + ignoreIncompatible: fix || ignoreIncompatible, + ignoreUnused: fix || ignoreUnused, + ignoreMissing: fix || ignoreMissing, + sourceFileName: translationsPath, + targetFileName: fix ? translationsPath : undefined, + config, + log, + }); + }, + title: `Compatibility check with ${translationsPath}`, + })); +} diff --git a/src/dev/i18n/tasks/extract_default_translations.ts b/src/dev/i18n/tasks/extract_default_translations.ts index 02e45350e249a9..92bf9663e975ed 100644 --- a/src/dev/i18n/tasks/extract_default_translations.ts +++ b/src/dev/i18n/tasks/extract_default_translations.ts @@ -18,19 +18,18 @@ */ import chalk from 'chalk'; -import Listr from 'listr'; - import { ErrorReporter, extractMessagesFromPathToMap, filterConfigPaths, I18nConfig } from '..'; import { createFailError } from '../../run'; -export async function extractDefaultMessages({ +export function extractDefaultMessages({ path, config, }: { path?: string | string[]; config: I18nConfig; }) { - const filteredPaths = filterConfigPaths(Array.isArray(path) ? path : [path || './'], config); + const inputPaths = Array.isArray(path) ? path : [path || './']; + const filteredPaths = filterConfigPaths(inputPaths, config) as string[]; if (filteredPaths.length === 0) { throw createFailError( `${chalk.white.bgRed( @@ -39,36 +38,22 @@ export async function extractDefaultMessages({ ); } - const reporter = new ErrorReporter(); - - const list = new Listr( - filteredPaths.map(filteredPath => ({ - task: async (messages: Map) => { - const initialErrorsNumber = reporter.errors.length; - - // Return result if no new errors were reported for this path. - const result = await extractMessagesFromPathToMap(filteredPath, messages, config, reporter); - if (reporter.errors.length === initialErrorsNumber) { - return result; - } - - // Throw an empty error to make Listr mark the task as failed without any message. - throw new Error(''); - }, - title: filteredPath, - })), - { - exitOnError: false, - } - ); - - try { - return await list.run(new Map()); - } catch (error) { - if (error.name === 'ListrError' && reporter.errors.length) { - throw createFailError(reporter.errors.join('\n\n')); - } - - throw error; - } + return filteredPaths.map(filteredPath => ({ + task: async (context: { + messages: Map; + reporter: ErrorReporter; + }) => { + const { messages, reporter } = context; + const initialErrorsNumber = reporter.errors.length; + + // Return result if no new errors were reported for this path. + const result = await extractMessagesFromPathToMap(filteredPath, messages, config, reporter); + if (reporter.errors.length === initialErrorsNumber) { + return result; + } + + throw reporter; + }, + title: filteredPath, + })); } diff --git a/src/dev/i18n/tasks/extract_untracked_translations.ts b/src/dev/i18n/tasks/extract_untracked_translations.ts new file mode 100644 index 00000000000000..cf83f02b76d82d --- /dev/null +++ b/src/dev/i18n/tasks/extract_untracked_translations.ts @@ -0,0 +1,109 @@ +/* + * 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 { + I18nConfig, + matchEntriesWithExctractors, + normalizePath, + readFileAsync, + ErrorReporter, +} from '..'; +import { createFailError } from '../../run'; + +function filterEntries(entries: string[], exclude: string[]) { + return entries.filter((entry: string) => + exclude.every((excludedPath: string) => !normalizePath(entry).startsWith(excludedPath)) + ); +} + +export async function extractUntrackedMessagesTask({ + path, + config, + reporter, +}: { + path?: string | string[]; + config: I18nConfig; + reporter: any; +}) { + const inputPaths = Array.isArray(path) ? path : [path || './']; + const availablePaths = Object.values(config.paths); + const ignore = availablePaths.concat([ + '**/build/**', + '**/webpackShims/**', + '**/__fixtures__/**', + '**/packages/kbn-i18n/**', + '**/packages/kbn-plugin-generator/sao_template/**', + '**/packages/kbn-ui-framework/generator-kui/**', + '**/target/**', + '**/test/**', + '**/scripts/**', + '**/src/dev/**', + '**/target/**', + '**/dist/**', + ]); + for (const inputPath of inputPaths) { + const categorizedEntries = await matchEntriesWithExctractors(inputPath, { + additionalIgnore: ignore, + mark: true, + absolute: true, + }); + + for (const [entries, extractFunction] of categorizedEntries) { + const files = await Promise.all( + filterEntries(entries, config.exclude) + .filter(entry => { + const normalizedEntry = normalizePath(entry); + return !availablePaths.some( + availablePath => + normalizedEntry.startsWith(`${normalizePath(availablePath)}/`) || + normalizePath(availablePath) === normalizedEntry + ); + }) + .map(async (entry: any) => ({ + name: entry, + content: await readFileAsync(entry), + })) + ); + + for (const { name, content } of files) { + const reporterWithContext = reporter.withContext({ name }); + for (const [id] of extractFunction(content, reporterWithContext)) { + const errorMessage = `Untracked file contains i18n label (${id}).`; + reporterWithContext.report(createFailError(errorMessage)); + } + } + } + } +} + +export function extractUntrackedMessages(srcPaths: string[], config: I18nConfig) { + return srcPaths.map(srcPath => ({ + title: `Checking untracked messages in ${srcPath}`, + task: async (context: { reporter: ErrorReporter }) => { + const { reporter } = context; + const initialErrorsNumber = reporter.errors.length; + const result = await extractUntrackedMessagesTask({ path: srcPath, config, reporter }); + if (reporter.errors.length === initialErrorsNumber) { + return result; + } + + throw reporter; + }, + })); +} diff --git a/src/dev/i18n/tasks/index.ts b/src/dev/i18n/tasks/index.ts index 00b3466d59276f..bd33baa989e0fd 100644 --- a/src/dev/i18n/tasks/index.ts +++ b/src/dev/i18n/tasks/index.ts @@ -18,3 +18,5 @@ */ export { extractDefaultMessages } from './extract_default_translations'; +export { extractUntrackedMessages } from './extract_untracked_translations'; +export { checkCompatibility } from './check_compatibility'; diff --git a/src/dev/run_i18n_check.ts b/src/dev/run_i18n_check.ts index cb8fc0ca7902b7..aad5da08df4add 100644 --- a/src/dev/run_i18n_check.ts +++ b/src/dev/run_i18n_check.ts @@ -20,8 +20,8 @@ import chalk from 'chalk'; import Listr from 'listr'; -import { integrateLocaleFiles, mergeConfigs } from './i18n'; -import { extractDefaultMessages } from './i18n/tasks'; +import { ErrorReporter, mergeConfigs } from './i18n'; +import { extractDefaultMessages, extractUntrackedMessages, checkCompatibility } from './i18n/tasks'; import { createFailError, run } from './run'; run( @@ -60,48 +60,58 @@ run( } const config = await mergeConfigs(includeConfig); - const defaultMessages = await extractDefaultMessages({ path, config }); + const srcPaths = Array().concat(path || ['./src', './packages', './x-pack']); if (config.translations.length === 0) { return; } const list = new Listr( - config.translations.map(translationsPath => ({ - task: async () => { - // If `--fix` is set we should try apply all possible fixes and override translations file. - await integrateLocaleFiles(defaultMessages, { - sourceFileName: translationsPath, - targetFileName: fix ? translationsPath : undefined, - dryRun: !fix, - ignoreIncompatible: fix || !!ignoreIncompatible, - ignoreUnused: fix || !!ignoreUnused, - ignoreMissing: fix || !!ignoreMissing, - config, - log, - }); + [ + { + title: 'Checking For Untracked Messages', + task: () => new Listr(extractUntrackedMessages(srcPaths, config), { exitOnError: true }), }, - title: `Compatibility check with ${translationsPath}`, - })), + { + title: 'Validating Default Messages', + task: () => + new Listr(extractDefaultMessages({ path: srcPaths, config }), { exitOnError: true }), + }, + { + title: 'Compatibility Checks', + task: () => + new Listr( + checkCompatibility( + config, + { + ignoreIncompatible: !!ignoreIncompatible, + ignoreUnused: !!ignoreUnused, + ignoreMissing: !!ignoreMissing, + fix, + }, + log + ), + { exitOnError: true } + ), + }, + ], { - concurrent: true, - exitOnError: false, + concurrent: false, + exitOnError: true, } ); try { - await list.run(); + const reporter = new ErrorReporter(); + const messages: Map = new Map(); + await list.run({ messages, reporter }); } catch (error) { process.exitCode = 1; - - if (!error.errors) { + if (error instanceof ErrorReporter) { + error.errors.forEach((e: string | Error) => log.error(e)); + } else { log.error('Unhandled exception!'); log.error(error); - process.exit(); - } - - for (const e of error.errors) { - log.error(e); } } }, diff --git a/src/dev/run_i18n_extract.ts b/src/dev/run_i18n_extract.ts index 6670e81949609a..63f7429ec420ef 100644 --- a/src/dev/run_i18n_extract.ts +++ b/src/dev/run_i18n_extract.ts @@ -18,9 +18,16 @@ */ import chalk from 'chalk'; +import Listr from 'listr'; import { resolve } from 'path'; -import { mergeConfigs, serializeToJson, serializeToJson5, writeFileAsync } from './i18n'; +import { + ErrorReporter, + mergeConfigs, + serializeToJson, + serializeToJson5, + writeFileAsync, +} from './i18n'; import { extractDefaultMessages } from './i18n/tasks'; import { createFailError, run } from './run'; @@ -32,6 +39,7 @@ run( 'output-format': outputFormat, 'include-config': includeConfig, }, + log, }) => { if (!outputDir || typeof outputDir !== 'string') { throw createFailError( @@ -46,18 +54,42 @@ run( } const config = await mergeConfigs(includeConfig); - const defaultMessages = await extractDefaultMessages({ path, config }); - // Messages shouldn't be written to a file if output is not supplied. - if (!outputDir || !defaultMessages.size) { - return; - } + const list = new Listr([ + { + title: 'Extracting Default Messages', + task: () => new Listr(extractDefaultMessages({ path, config }), { exitOnError: true }), + }, + { + title: 'Writing to file', + enabled: ctx => outputDir && ctx.messages.size, + task: async ctx => { + const sortedMessages = [...ctx.messages].sort(([key1], [key2]) => + key1.localeCompare(key2) + ); + await writeFileAsync( + resolve(outputDir, 'en.json'), + outputFormat === 'json5' + ? serializeToJson5(sortedMessages) + : serializeToJson(sortedMessages) + ); + }, + }, + ]); - const sortedMessages = [...defaultMessages].sort(([key1], [key2]) => key1.localeCompare(key2)); - await writeFileAsync( - resolve(outputDir, 'en.json'), - outputFormat === 'json5' ? serializeToJson5(sortedMessages) : serializeToJson(sortedMessages) - ); + try { + const reporter = new ErrorReporter(); + const messages: Map = new Map(); + await list.run({ messages, reporter }); + } catch (error) { + process.exitCode = 1; + if (error instanceof ErrorReporter) { + error.errors.forEach((e: string | Error) => log.error(e)); + } else { + log.error('Unhandled exception!'); + log.error(error); + } + } }, { flags: { diff --git a/src/dev/run_i18n_integrate.ts b/src/dev/run_i18n_integrate.ts index e5cf21fb88ba3c..b80dd8deac274a 100644 --- a/src/dev/run_i18n_integrate.ts +++ b/src/dev/run_i18n_integrate.ts @@ -18,8 +18,9 @@ */ import chalk from 'chalk'; +import Listr from 'listr'; -import { integrateLocaleFiles, mergeConfigs } from './i18n'; +import { ErrorReporter, integrateLocaleFiles, mergeConfigs } from './i18n'; import { extractDefaultMessages } from './i18n/tasks'; import { createFailError, run } from './run'; @@ -75,18 +76,41 @@ run( } const config = await mergeConfigs(includeConfig); - const defaultMessages = await extractDefaultMessages({ path, config }); + const list = new Listr([ + { + title: 'Extracting Default Messages', + task: () => new Listr(extractDefaultMessages({ path, config }), { exitOnError: true }), + }, + { + title: 'Intregrating Locale File', + task: async ({ messages }) => { + await integrateLocaleFiles(messages, { + sourceFileName: source, + targetFileName: target, + dryRun, + ignoreIncompatible, + ignoreUnused, + ignoreMissing, + config, + log, + }); + }, + }, + ]); - await integrateLocaleFiles(defaultMessages, { - sourceFileName: source, - targetFileName: target, - dryRun, - ignoreIncompatible, - ignoreUnused, - ignoreMissing, - config, - log, - }); + try { + const reporter = new ErrorReporter(); + const messages: Map = new Map(); + await list.run({ messages, reporter }); + } catch (error) { + process.exitCode = 1; + if (error instanceof ErrorReporter) { + error.errors.forEach((e: string | Error) => log.error(e)); + } else { + log.error('Unhandled exception!'); + log.error(error); + } + } }, { flags: { diff --git a/src/legacy/core_plugins/embeddable_api/public/context_menu_actions/build_eui_context_menu_panels.ts b/src/legacy/core_plugins/embeddable_api/public/context_menu_actions/build_eui_context_menu_panels.ts index 5187b929373dec..a22d22615cf95a 100644 --- a/src/legacy/core_plugins/embeddable_api/public/context_menu_actions/build_eui_context_menu_panels.ts +++ b/src/legacy/core_plugins/embeddable_api/public/context_menu_actions/build_eui_context_menu_panels.ts @@ -42,7 +42,7 @@ export async function buildContextMenuForActions({ return { id: 'mainMenu', - title: i18n.translate('embeddableAPI.actionPanel.title', { + title: i18n.translate('embeddableApi.actionPanel.title', { defaultMessage: 'Options', }), items: menuItems, diff --git a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/add_panel/add_panel_action.tsx b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/add_panel/add_panel_action.tsx index 859194cc6f673a..5f0571f3d140c9 100644 --- a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/add_panel/add_panel_action.tsx +++ b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/add_panel/add_panel_action.tsx @@ -35,7 +35,7 @@ export class AddPanelAction extends Action { } public getDisplayName() { - return i18n.translate('kbn.embeddable.panel.addPanel.displayName', { + return i18n.translate('embeddableApi.addPanel.displayName', { defaultMessage: 'Add panel', }); } diff --git a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/add_panel/add_panel_flyout.tsx b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/add_panel/add_panel_flyout.tsx index 6477378aa2bc51..48e127fb063e82 100644 --- a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/add_panel/add_panel_flyout.tsx +++ b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/add_panel/add_panel_flyout.tsx @@ -63,7 +63,7 @@ export class AddPanelFlyout extends React.Component { this.lastToast = toastNotifications.addSuccess({ title: i18n.translate( - 'kbn.embeddables.addPanel.savedObjectAddedToContainerSuccessMessageTitle', + 'embeddableApi.addPanel.savedObjectAddedToContainerSuccessMessageTitle', { defaultMessage: '{savedObjectName} was added', values: { @@ -103,7 +103,7 @@ export class AddPanelFlyout extends React.Component { inputDisplay: ( @@ -119,7 +119,7 @@ export class AddPanelFlyout extends React.Component { inputDisplay: ( {

- +

@@ -159,7 +159,7 @@ export class AddPanelFlyout extends React.Component { > } showFilter={true} - noItemsMessage={i18n.translate('kbn.embeddables.addPanel.noMatchingObjectsMessage', { + noItemsMessage={i18n.translate('embeddableApi.addPanel.noMatchingObjectsMessage', { defaultMessage: 'No matching objects found.', })} /> diff --git a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/customize_title/customize_panel_action.tsx b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/customize_title/customize_panel_action.tsx index 8673ab8b6f14b2..d208b941ee57c0 100644 --- a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/customize_title/customize_panel_action.tsx +++ b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/customize_title/customize_panel_action.tsx @@ -40,7 +40,7 @@ export class CustomizePanelTitleAction extends Action { } public getDisplayName() { - return i18n.translate('kbn.embeddables.panel.customizePanel.displayName', { + return i18n.translate('embeddableApi.customizePanel.action.displayName', { defaultMessage: 'Customize panel', }); } diff --git a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/customize_title/customize_panel_modal.tsx b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/customize_title/customize_panel_modal.tsx index 91de837fabc180..8bbc7b2f2c6e5f 100644 --- a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/customize_title/customize_panel_modal.tsx +++ b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/customize_title/customize_panel_modal.tsx @@ -97,7 +97,7 @@ export class CustomizePanelModalUi extends Component label={ } onChange={this.onHideTitleToggle} @@ -105,7 +105,7 @@ export class CustomizePanelModalUi extends Component @@ -119,7 +119,7 @@ export class CustomizePanelModalUi extends Component value={this.state.title || ''} onChange={e => this.updateTitle(e.target.value)} aria-label={this.props.intl.formatMessage({ - id: 'kbn.embeddable.panel.optionsMenuForm.panelTitleInputAriaLabel', + id: 'embeddableApi.customizePanel.modal.optionsMenuForm.panelTitleInputAriaLabel', defaultMessage: 'Enter a custom title for your panel', })} append={ @@ -129,7 +129,7 @@ export class CustomizePanelModalUi extends Component disabled={this.state.hideTitle} > @@ -142,11 +142,17 @@ export class CustomizePanelModalUi extends Component onClick={() => this.props.updateTitle(this.props.embeddable.getOutput().title)} > {' '} - + - + diff --git a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/customize_title/customize_title_form.tsx b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/customize_title/customize_title_form.tsx index c4b9f9ff9a40ae..aa7a0b9dd781f5 100644 --- a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/customize_title/customize_title_form.tsx +++ b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/customize_title/customize_title_form.tsx @@ -46,7 +46,7 @@ function CustomizeTitleFormUi({
@@ -58,7 +58,7 @@ function CustomizeTitleFormUi({ value={title} onChange={onInputChange} aria-label={intl.formatMessage({ - id: 'kbn.dashboard.panel.optionsMenuForm.panelTitleInputAriaLabel', + id: 'embeddableApi.customizeTitle.optionsMenuForm.panelTitleInputAriaLabel', defaultMessage: 'Changes to this input are applied immediately. Press enter to exit.', })} /> @@ -66,7 +66,7 @@ function CustomizeTitleFormUi({ diff --git a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/edit_panel_action.tsx b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/edit_panel_action.tsx index 4e7a0a63edf33a..e64a793295b353 100644 --- a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/edit_panel_action.tsx +++ b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/edit_panel_action.tsx @@ -40,7 +40,7 @@ export class EditPanelAction extends Action { if (!factory) { throw new EmbeddableFactoryNotFoundError(embeddable.type); } - return i18n.translate('kbn.dashboard.panel.editPanel.displayName', { + return i18n.translate('embeddableApi.panel.editPanel.displayName', { defaultMessage: 'Edit {value}', values: { value: factory.getDisplayName(), diff --git a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/inspect_panel_action.tsx b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/inspect_panel_action.tsx index 3656bdc3f83a61..2346dd76a69ce5 100644 --- a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/inspect_panel_action.tsx +++ b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/inspect_panel_action.tsx @@ -34,7 +34,7 @@ export class InspectPanelAction extends Action { } public getDisplayName() { - return i18n.translate('kbn.embeddable.panel.inspectPanel.displayName', { + return i18n.translate('embeddableApi.panel.inspectPanel.displayName', { defaultMessage: 'Inspect', }); } diff --git a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/remove_panel_action.tsx b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/remove_panel_action.tsx index 62fc04436abdad..04dcb3d132ec0e 100644 --- a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/remove_panel_action.tsx +++ b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_actions/remove_panel_action.tsx @@ -44,7 +44,7 @@ export class RemovePanelAction extends Action { } public getDisplayName() { - return i18n.translate('kbn.embeddable.panel.removePanel.displayName', { + return i18n.translate('embeddableApi.panel.removePanel.displayName', { defaultMessage: 'Delete from dashboard', }); } diff --git a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_header.tsx b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_header.tsx index bea899fd633358..3d6f82b94693db 100644 --- a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_header.tsx +++ b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_header.tsx @@ -70,7 +70,7 @@ function PanelHeaderUi({ title={title} aria-label={intl.formatMessage( { - id: 'kbn.dashboard.panel.dashboardPanelAriaLabel', + id: 'embeddableApi.panel.dashboardPanelAriaLabel', defaultMessage: 'Dashboard panel: {title}', }, { diff --git a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_options_menu.tsx b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_options_menu.tsx index 351e007357e5af..dfffd0b5043286 100644 --- a/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_options_menu.tsx +++ b/src/legacy/core_plugins/embeddable_api/public/panel/panel_header/panel_options_menu.tsx @@ -84,7 +84,7 @@ class PanelOptionsMenuUi extends React.Component color="text" className="embPanel__optionsMenuButton" aria-label={intl.formatMessage({ - id: 'kbn.dashboard.panel.optionsMenu.panelOptionsButtonAriaLabel', + id: 'embeddableApi.panel.optionsMenu.panelOptionsButtonAriaLabel', defaultMessage: 'Panel options', })} data-test-subj="embeddablePanelToggleMenuIcon" diff --git a/src/legacy/core_plugins/embeddable_api/public/test_samples/embeddables/contact_card/contact_card_embeddable_factory.tsx b/src/legacy/core_plugins/embeddable_api/public/test_samples/embeddables/contact_card/contact_card_embeddable_factory.tsx index 22c0364a1eb20c..87fb7b1169f249 100644 --- a/src/legacy/core_plugins/embeddable_api/public/test_samples/embeddables/contact_card/contact_card_embeddable_factory.tsx +++ b/src/legacy/core_plugins/embeddable_api/public/test_samples/embeddables/contact_card/contact_card_embeddable_factory.tsx @@ -35,7 +35,7 @@ export class ContactCardEmbeddableFactory extends EmbeddableFactory Date: Mon, 17 Jun 2019 15:45:26 +0200 Subject: [PATCH 08/10] Add required default markdown visState (#38390) The markdownVis expression has the markdown string property configured as required.The current implementation of the markdown vis doesn't have a default value (actually it's undefined) and this cause the error to be thrown when creating a new markdown vis. fix #38127 --- src/legacy/core_plugins/markdown_vis/public/markdown_vis.js | 3 ++- .../__snapshots__/build_pipeline.test.js.snap | 2 ++ .../loader/pipeline_helpers/build_pipeline.test.js | 6 ++++++ .../visualize/loader/pipeline_helpers/build_pipeline.ts | 5 ++++- 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/legacy/core_plugins/markdown_vis/public/markdown_vis.js b/src/legacy/core_plugins/markdown_vis/public/markdown_vis.js index 18afbb94eee05e..f148cd8d39fd91 100644 --- a/src/legacy/core_plugins/markdown_vis/public/markdown_vis.js +++ b/src/legacy/core_plugins/markdown_vis/public/markdown_vis.js @@ -43,7 +43,8 @@ function MarkdownVisProvider() { component: MarkdownVisWrapper, defaults: { fontSize: 12, - openLinksInNewTab: false + openLinksInNewTab: false, + markdown: '', } }, editorConfig: { diff --git a/src/legacy/ui/public/visualize/loader/pipeline_helpers/__snapshots__/build_pipeline.test.js.snap b/src/legacy/ui/public/visualize/loader/pipeline_helpers/__snapshots__/build_pipeline.test.js.snap index 920a29ba2f5b69..0eeb18db708503 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_helpers/__snapshots__/build_pipeline.test.js.snap +++ b/src/legacy/ui/public/visualize/loader/pipeline_helpers/__snapshots__/build_pipeline.test.js.snap @@ -36,4 +36,6 @@ exports[`visualize loader pipeline helpers: build pipeline buildPipelineVisFunct exports[`visualize loader pipeline helpers: build pipeline buildPipelineVisFunction handles timelion function 1`] = `"timelion_vis expression='foo' interval='bar' "`; +exports[`visualize loader pipeline helpers: build pipeline buildPipelineVisFunction handles undefined markdown function 1`] = `"markdownvis '' fontSize=12 openLinksInNewTab=true "`; + exports[`visualize loader pipeline helpers: build pipeline buildPipelineVisFunction handles vega function 1`] = `"vega spec='this is a test' "`; diff --git a/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.test.js b/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.test.js index d8d43a10499807..435108c8023e78 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.test.js +++ b/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.test.js @@ -92,6 +92,12 @@ describe('visualize loader pipeline helpers: build pipeline', () => { expect(actual).toMatchSnapshot(); }); + it('handles undefined markdown function', () => { + const params = { fontSize: 12, openLinksInNewTab: true, foo: 'bar' }; + const actual = buildPipelineVisFunction.markdown({ params }); + expect(actual).toMatchSnapshot(); + }); + describe('handles table function', () => { it('without splits or buckets', () => { const params = { foo: 'bar' }; diff --git a/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts b/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts index b26be35e6a268c..1488fa0658912e 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts @@ -226,7 +226,10 @@ export const buildPipelineVisFunction: BuildPipelineVisFunction = { }, markdown: visState => { const { markdown, fontSize, openLinksInNewTab } = visState.params; - const escapedMarkdown = escapeString(markdown); + let escapedMarkdown = ''; + if (typeof markdown === 'string' || markdown instanceof String) { + escapedMarkdown = escapeString(markdown.toString()); + } let expr = `markdownvis '${escapedMarkdown}' `; if (fontSize) { expr += ` fontSize=${fontSize} `; From 922e111dd504cb0a5f4850042bdbd68fc859ec52 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Mon, 17 Jun 2019 15:45:53 +0200 Subject: [PATCH 09/10] Upgrade @elastic/charts to 6.0.1 (#38783) * Upgrade @elastic/charts to 5.2.0 * Upgrade @elastic/charts to 6.0.1 * Add darkMode theme mock --- package.json | 4 ++-- src/legacy/ui/ui_render/ui_render_mixin.js | 3 +++ .../public/components/metrics_explorer/chart.tsx | 4 ++-- .../metrics_explorer/helpers/get_chart_theme.ts | 13 +++++++++++++ .../components/metrics_explorer/line_series.tsx | 1 - .../siem/public/components/charts/areachart.tsx | 1 - .../siem/public/components/charts/common.tsx | 7 ++++++- x-pack/plugins/siem/public/mock/ui_settings.ts | 2 ++ yarn.lock | 16 +++++++++++----- 9 files changed, 39 insertions(+), 12 deletions(-) create mode 100644 x-pack/plugins/infra/public/components/metrics_explorer/helpers/get_chart_theme.ts diff --git a/package.json b/package.json index 5ae8763ffd181c..b9cee916718ac0 100644 --- a/package.json +++ b/package.json @@ -102,7 +102,7 @@ "@babel/core": "7.4.5", "@babel/polyfill": "7.4.4", "@babel/register": "7.4.4", - "@elastic/charts": "^4.2.6", + "@elastic/charts": "^6.0.1", "@elastic/datemath": "5.0.2", "@elastic/eui": "11.3.2", "@elastic/filesaver": "1.1.2", @@ -441,4 +441,4 @@ "node": "10.15.2", "yarn": "^1.10.1" } -} +} \ No newline at end of file diff --git a/src/legacy/ui/ui_render/ui_render_mixin.js b/src/legacy/ui/ui_render/ui_render_mixin.js index 597fbc422e6bf5..128a0bac6e3274 100644 --- a/src/legacy/ui/ui_render/ui_render_mixin.js +++ b/src/legacy/ui/ui_render/ui_render_mixin.js @@ -56,6 +56,7 @@ export function uiRenderMixin(kbnServer, server, config) { server.exposeStaticDir('/node_modules/@elastic/eui/dist/{path*}', fromRoot('node_modules/@elastic/eui/dist')); server.exposeStaticDir('/node_modules/@kbn/ui-framework/dist/{path*}', fromRoot('node_modules/@kbn/ui-framework/dist')); + server.exposeStaticDir('/node_modules/@elastic/charts/dist/{path*}', fromRoot('node_modules/@elastic/charts/dist')); const translationsCache = { translations: null, hash: null }; server.route({ @@ -120,9 +121,11 @@ export function uiRenderMixin(kbnServer, server, config) { [ `${basePath}/node_modules/@elastic/eui/dist/eui_theme_dark.css`, `${basePath}/node_modules/@kbn/ui-framework/dist/kui_dark.css`, + `${basePath}/node_modules/@elastic/charts/dist/theme_only_dark.css`, ] : [ `${basePath}/node_modules/@elastic/eui/dist/eui_theme_light.css`, `${basePath}/node_modules/@kbn/ui-framework/dist/kui_light.css`, + `${basePath}/node_modules/@elastic/charts/dist/theme_only_light.css`, ] ), `${regularBundlePath}/${darkMode ? 'dark' : 'light'}_theme.style.css`, diff --git a/x-pack/plugins/infra/public/components/metrics_explorer/chart.tsx b/x-pack/plugins/infra/public/components/metrics_explorer/chart.tsx index dfeb88553f209d..8d86458ca35530 100644 --- a/x-pack/plugins/infra/public/components/metrics_explorer/chart.tsx +++ b/x-pack/plugins/infra/public/components/metrics_explorer/chart.tsx @@ -8,7 +8,6 @@ import React, { useCallback } from 'react'; import { InjectedIntl, injectI18n } from '@kbn/i18n/react'; import { EuiTitle, EuiToolTip, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; import { Chart, Axis, Position, timeFormatter, getAxisId, Settings } from '@elastic/charts'; -import '@elastic/charts/dist/style.css'; import { first } from 'lodash'; import { niceTimeFormatByDay } from '@elastic/charts/dist/utils/data/formatters'; import moment from 'moment'; @@ -24,6 +23,7 @@ import { MetricsExplorerChartContextMenu } from './chart_context_menu'; import { SourceQuery } from '../../graphql/types'; import { MetricsExplorerEmptyChart } from './empty_chart'; import { MetricsExplorerNoMetrics } from './no_metrics'; +import { getChartTheme } from './helpers/get_chart_theme'; interface Props { intl: InjectedIntl; @@ -103,7 +103,7 @@ export const MetricsExplorerChart = injectI18n( tickFormat={dateFormatter} /> - + ) : options.metrics.length > 0 ? ( diff --git a/x-pack/plugins/infra/public/components/metrics_explorer/helpers/get_chart_theme.ts b/x-pack/plugins/infra/public/components/metrics_explorer/helpers/get_chart_theme.ts new file mode 100644 index 00000000000000..7be2e54d47d75d --- /dev/null +++ b/x-pack/plugins/infra/public/components/metrics_explorer/helpers/get_chart_theme.ts @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import chrome from 'ui/chrome'; +import { Theme, LIGHT_THEME, DARK_THEME } from '@elastic/charts'; + +export function getChartTheme(): Theme { + const isDarkMode = chrome.getUiSettingsClient().get('theme:darkMode'); + return isDarkMode ? DARK_THEME : LIGHT_THEME; +} diff --git a/x-pack/plugins/infra/public/components/metrics_explorer/line_series.tsx b/x-pack/plugins/infra/public/components/metrics_explorer/line_series.tsx index 3c0924c4ca2cf4..6e29d943627988 100644 --- a/x-pack/plugins/infra/public/components/metrics_explorer/line_series.tsx +++ b/x-pack/plugins/infra/public/components/metrics_explorer/line_series.tsx @@ -12,7 +12,6 @@ import { DataSeriesColorsValues, CustomSeriesColorsMap, } from '@elastic/charts'; -import '@elastic/charts/dist/style.css'; import { MetricsExplorerSeries } from '../../../server/routes/metrics_explorer/types'; import { colorTransformer, MetricsExplorerColor } from '../../../common/color_palette'; import { createMetricLabel } from './helpers/create_metric_label'; diff --git a/x-pack/plugins/siem/public/components/charts/areachart.tsx b/x-pack/plugins/siem/public/components/charts/areachart.tsx index 0e2c74509f2c02..67cd9f730a0dd6 100644 --- a/x-pack/plugins/siem/public/components/charts/areachart.tsx +++ b/x-pack/plugins/siem/public/components/charts/areachart.tsx @@ -15,7 +15,6 @@ import { ScaleType, Settings, } from '@elastic/charts'; -import '@elastic/charts/dist/style.css'; import { ChartConfigsData, ChartHolder, diff --git a/x-pack/plugins/siem/public/components/charts/common.tsx b/x-pack/plugins/siem/public/components/charts/common.tsx index cbae797cd977ed..1d3b0ed3531dad 100644 --- a/x-pack/plugins/siem/public/components/charts/common.tsx +++ b/x-pack/plugins/siem/public/components/charts/common.tsx @@ -12,8 +12,11 @@ import { getSpecId, mergeWithDefaultTheme, PartialTheme, + LIGHT_THEME, + DARK_THEME, } from '@elastic/charts'; import { i18n } from '@kbn/i18n'; +import chrome from 'ui/chrome'; const chartHeight = 74; const FlexGroup = styled(EuiFlexGroup)` @@ -102,5 +105,7 @@ export const getTheme = () => { barsPadding: 0.5, }, }; - return mergeWithDefaultTheme(theme); + const isDarkMode = chrome.getUiSettingsClient().get('theme:darkMode'); + const defaultTheme = isDarkMode ? DARK_THEME : LIGHT_THEME; + return mergeWithDefaultTheme(theme, defaultTheme); }; diff --git a/x-pack/plugins/siem/public/mock/ui_settings.ts b/x-pack/plugins/siem/public/mock/ui_settings.ts index c6d72de77282d9..07c14631cd96c2 100644 --- a/x-pack/plugins/siem/public/mock/ui_settings.ts +++ b/x-pack/plugins/siem/public/mock/ui_settings.ts @@ -14,6 +14,8 @@ chrome.getUiSettingsClient().get.mockImplementation((key: string) => { return { pause: false, value: 0 }; case 'siem:defaultIndex': return ['auditbeat-*', 'filebeat-*', 'packetbeat-*', 'winlogbeat-*']; + case 'theme:darkMode': + return false; default: throw new Error(`Unexpected config key: ${key}`); } diff --git a/yarn.lock b/yarn.lock index 3ef8ec04ba8961..35a11f7d1d518b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1546,10 +1546,10 @@ lodash "^4.17.11" to-fast-properties "^2.0.0" -"@elastic/charts@^4.2.6": - version "4.2.6" - resolved "https://registry.yarnpkg.com/@elastic/charts/-/charts-4.2.6.tgz#19b8fa79758f58ebae6eeba750bea6a418b15659" - integrity sha512-ROsv3YwSCDsp09ObeVkAL9YsiNJsAY2kAtBwovRaNCcotI9ybNSQcqbTLlC9vZkduyQSsHBvRL/RAwY8btXE8g== +"@elastic/charts@^6.0.1": + version "6.0.1" + resolved "https://registry.yarnpkg.com/@elastic/charts/-/charts-6.0.1.tgz#dd3fd512946870d941baf16402ec82741d680ae9" + integrity sha512-31n/ceU5b0USc2Mfbli+/xBadjWCbakP2Nr/KjIWaJCMIljY/VXkeSWL//CK+wA3T19utyxQWlP+s2B9Ol0isA== dependencies: "@types/d3-shape" "^1.3.1" "@types/luxon" "^1.11.1" @@ -1560,7 +1560,6 @@ d3-shape "^1.3.4" fp-ts "^1.14.2" konva "^2.6.0" - lodash "^4.17.11" luxon "^1.11.3" mobx "^4.9.2" mobx-react "^5.4.3" @@ -1571,6 +1570,8 @@ react-konva "16.8.3" react-spring "^8.0.8" resize-observer-polyfill "^1.5.1" + ts-debounce "^1.0.0" + uuid "^3.3.2" "@elastic/elasticsearch@^7.0.0-rc.2": version "7.0.0-rc.2" @@ -26779,6 +26780,11 @@ trunc-text@1.0.2: resolved "https://registry.yarnpkg.com/trunc-text/-/trunc-text-1.0.2.tgz#b582bb3ddea9c9adc25017d737c48ebdd2157406" integrity sha1-tYK7Pd6pya3CUBfXN8SOvdIVdAY= +ts-debounce@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/ts-debounce/-/ts-debounce-1.0.0.tgz#e433301744ba75fe25466f7f23e1382c646aae6a" + integrity sha512-V+IzWj418IoqqxVJD6I0zjPtgIyvAJ8VyViqzcxZ0JRiJXsi5mCmy1yUKkWd2gUygT28a8JsVFCgqdrf2pLUHQ== + ts-invariant@^0.2.1: version "0.2.1" resolved "https://registry.yarnpkg.com/ts-invariant/-/ts-invariant-0.2.1.tgz#3d587f9d6e3bded97bf9ec17951dd9814d5a9d3f" From de927603d799e13499961d7f6141b61efcd5ec49 Mon Sep 17 00:00:00 2001 From: Angela Chuang <6295984+angorayc@users.noreply.github.com> Date: Mon, 17 Jun 2019 23:08:35 +0800 Subject: [PATCH 10/10] update button size (#39051) --- .../siem/public/components/timeline/properties/helpers.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/siem/public/components/timeline/properties/helpers.tsx b/x-pack/plugins/siem/public/components/timeline/properties/helpers.tsx index 3fb527927c5432..b9b31b65f8e6b2 100644 --- a/x-pack/plugins/siem/public/components/timeline/properties/helpers.tsx +++ b/x-pack/plugins/siem/public/components/timeline/properties/helpers.tsx @@ -154,7 +154,7 @@ const LargeNotesButton = pure<{ noteIds: string[]; text?: string; toggleShowNote toggleShowNotes()} - size="l" + size="m" >