From b085dee766e4303fca3e05903638fd46dfca2e23 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 12 Nov 2020 14:54:47 +0100 Subject: [PATCH 1/2] move i18n route to core --- .../server/i18n/i18n_service.test.mocks.ts | 5 ++ src/core/server/i18n/i18n_service.test.ts | 26 +++++-- src/core/server/i18n/i18n_service.ts | 8 ++- src/core/server/i18n/routes/index.ts | 25 +++++++ src/core/server/i18n/routes/translations.ts | 69 +++++++++++++++++++ src/core/server/server.ts | 6 +- src/legacy/ui/ui_render/ui_render_mixin.js | 32 --------- 7 files changed, 131 insertions(+), 40 deletions(-) create mode 100644 src/core/server/i18n/routes/index.ts create mode 100644 src/core/server/i18n/routes/translations.ts diff --git a/src/core/server/i18n/i18n_service.test.mocks.ts b/src/core/server/i18n/i18n_service.test.mocks.ts index 23f97a1404fff..d35141ecb111f 100644 --- a/src/core/server/i18n/i18n_service.test.mocks.ts +++ b/src/core/server/i18n/i18n_service.test.mocks.ts @@ -26,3 +26,8 @@ export const initTranslationsMock = jest.fn(); jest.doMock('./init_translations', () => ({ initTranslations: initTranslationsMock, })); + +export const registerRoutesMock = jest.fn(); +jest.doMock('./routes', () => ({ + registerRoutes: registerRoutesMock, +})); diff --git a/src/core/server/i18n/i18n_service.test.ts b/src/core/server/i18n/i18n_service.test.ts index 87de39a92ab26..e9deb96ccf88b 100644 --- a/src/core/server/i18n/i18n_service.test.ts +++ b/src/core/server/i18n/i18n_service.test.ts @@ -17,13 +17,18 @@ * under the License. */ -import { getKibanaTranslationFilesMock, initTranslationsMock } from './i18n_service.test.mocks'; +import { + getKibanaTranslationFilesMock, + initTranslationsMock, + registerRoutesMock, +} from './i18n_service.test.mocks'; import { BehaviorSubject } from 'rxjs'; import { I18nService } from './i18n_service'; import { configServiceMock } from '../config/mocks'; import { mockCoreContext } from '../core_context.mock'; +import { httpServiceMock } from '../http/http_service.mock'; const getConfigService = (locale = 'en') => { const configService = configServiceMock.create(); @@ -41,6 +46,7 @@ const getConfigService = (locale = 'en') => { describe('I18nService', () => { let service: I18nService; let configService: ReturnType; + let http: ReturnType; beforeEach(() => { jest.clearAllMocks(); @@ -48,6 +54,8 @@ describe('I18nService', () => { const coreContext = mockCoreContext.create({ configService }); service = new I18nService(coreContext); + + http = httpServiceMock.createInternalSetupContract(); }); describe('#setup', () => { @@ -55,7 +63,7 @@ describe('I18nService', () => { getKibanaTranslationFilesMock.mockResolvedValue([]); const pluginPaths = ['/pathA', '/pathB']; - await service.setup({ pluginPaths }); + await service.setup({ pluginPaths, http }); expect(getKibanaTranslationFilesMock).toHaveBeenCalledTimes(1); expect(getKibanaTranslationFilesMock).toHaveBeenCalledWith('en', pluginPaths); @@ -65,17 +73,27 @@ describe('I18nService', () => { const translationFiles = ['/path/to/file', 'path/to/another/file']; getKibanaTranslationFilesMock.mockResolvedValue(translationFiles); - await service.setup({ pluginPaths: [] }); + await service.setup({ pluginPaths: [], http }); expect(initTranslationsMock).toHaveBeenCalledTimes(1); expect(initTranslationsMock).toHaveBeenCalledWith('en', translationFiles); }); + it('calls `registerRoutesMock` with the correct parameters', async () => { + await service.setup({ pluginPaths: [], http }); + + expect(registerRoutesMock).toHaveBeenCalledTimes(1); + expect(registerRoutesMock).toHaveBeenCalledWith({ + locale: 'en', + router: expect.any(Object), + }); + }); + it('returns accessors for locale and translation files', async () => { const translationFiles = ['/path/to/file', 'path/to/another/file']; getKibanaTranslationFilesMock.mockResolvedValue(translationFiles); - const { getLocale, getTranslationFiles } = await service.setup({ pluginPaths: [] }); + const { getLocale, getTranslationFiles } = await service.setup({ pluginPaths: [], http }); expect(getLocale()).toEqual('en'); expect(getTranslationFiles()).toEqual(translationFiles); diff --git a/src/core/server/i18n/i18n_service.ts b/src/core/server/i18n/i18n_service.ts index fd32dd7fdd6ef..4a609ca5e2aea 100644 --- a/src/core/server/i18n/i18n_service.ts +++ b/src/core/server/i18n/i18n_service.ts @@ -21,11 +21,14 @@ import { take } from 'rxjs/operators'; import { Logger } from '../logging'; import { IConfigService } from '../config'; import { CoreContext } from '../core_context'; +import { InternalHttpServiceSetup } from '../http'; import { config as i18nConfigDef, I18nConfigType } from './i18n_config'; import { getKibanaTranslationFiles } from './get_kibana_translation_files'; import { initTranslations } from './init_translations'; +import { registerRoutes } from './routes'; interface SetupDeps { + http: InternalHttpServiceSetup; pluginPaths: string[]; } @@ -53,7 +56,7 @@ export class I18nService { this.configService = coreContext.configService; } - public async setup({ pluginPaths }: SetupDeps): Promise { + public async setup({ pluginPaths, http }: SetupDeps): Promise { const i18nConfig = await this.configService .atPath(i18nConfigDef.path) .pipe(take(1)) @@ -67,6 +70,9 @@ export class I18nService { this.log.debug(`Using translation files: [${translationFiles.join(', ')}]`); await initTranslations(locale, translationFiles); + const router = http.createRouter(''); + registerRoutes({ router, locale }); + return { getLocale: () => locale, getTranslationFiles: () => translationFiles, diff --git a/src/core/server/i18n/routes/index.ts b/src/core/server/i18n/routes/index.ts new file mode 100644 index 0000000000000..b0cce67b0aa4d --- /dev/null +++ b/src/core/server/i18n/routes/index.ts @@ -0,0 +1,25 @@ +/* + * 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 { IRouter } from '../../http'; +import { registerTranslationsRoute } from './translations'; + +export const registerRoutes = ({ router, locale }: { router: IRouter; locale: string }) => { + registerTranslationsRoute(router, locale); +}; diff --git a/src/core/server/i18n/routes/translations.ts b/src/core/server/i18n/routes/translations.ts new file mode 100644 index 0000000000000..0ac2f8ecc54ca --- /dev/null +++ b/src/core/server/i18n/routes/translations.ts @@ -0,0 +1,69 @@ +/* + * 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 { createHash } from 'crypto'; +import { i18n } from '@kbn/i18n'; +import { schema } from '@kbn/config-schema'; +import { IRouter } from '../../http'; + +interface TranslationCache { + translations: string; + hash: string; +} + +export const registerTranslationsRoute = (router: IRouter, locale: string) => { + let translationCache: TranslationCache; + + router.get( + { + path: '/translations/{locale}.json', + validate: { + params: schema.object({ + locale: schema.string(), + }), + }, + options: { + authRequired: false, + }, + }, + (ctx, req, res) => { + if (req.params.locale !== locale) { + return res.notFound({ + body: `Unknown locale: ${req.params.locale}`, + }); + } + if (!translationCache) { + const translations = JSON.stringify(i18n.getTranslation()); + const hash = createHash('sha1').update(translations).digest('hex'); + translationCache = { + translations, + hash, + }; + } + return res.ok({ + headers: { + 'content-type': 'application/json', + 'cache-control': 'must-revalidate', + etag: translationCache.hash, + }, + body: translationCache.translations, + }); + } + ); +}; diff --git a/src/core/server/server.ts b/src/core/server/server.ts index 55ed88e55a9f5..0f7e8cced999c 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -131,9 +131,6 @@ export class Server { await ensureValidConfiguration(this.configService, legacyConfigSetup); } - // setup i18n prior to any other service, to have translations ready - const i18nServiceSetup = await this.i18n.setup({ pluginPaths }); - const contextServiceSetup = this.context.setup({ // We inject a fake "legacy plugin" with dependencies on every plugin so that legacy plugins: // 1) Can access context from any KP plugin @@ -149,6 +146,9 @@ export class Server { context: contextServiceSetup, }); + // setup i18n prior to any other service, to have translations ready + const i18nServiceSetup = await this.i18n.setup({ http: httpSetup, pluginPaths }); + const capabilitiesSetup = this.capabilities.setup({ http: httpSetup }); const elasticsearchServiceSetup = await this.elasticsearch.setup({ diff --git a/src/legacy/ui/ui_render/ui_render_mixin.js b/src/legacy/ui/ui_render/ui_render_mixin.js index a02c2fca14c18..b8e80300957ba 100644 --- a/src/legacy/ui/ui_render/ui_render_mixin.js +++ b/src/legacy/ui/ui_render/ui_render_mixin.js @@ -17,9 +17,7 @@ * under the License. */ -import { createHash } from 'crypto'; import Boom from '@hapi/boom'; -import { i18n } from '@kbn/i18n'; import * as UiSharedDeps from '@kbn/ui-shared-deps'; import { KibanaRequest } from '../../../core/server'; import { AppBootstrap } from './bootstrap'; @@ -37,36 +35,6 @@ import { getApmConfig } from '../apm'; * @param {KbnServer['config']} config */ export function uiRenderMixin(kbnServer, server, config) { - const translationsCache = { translations: null, hash: null }; - server.route({ - path: '/translations/{locale}.json', - method: 'GET', - config: { auth: false }, - handler(request, h) { - // Kibana server loads translations only for a single locale - // that is specified in `i18n.locale` config value. - const { locale } = request.params; - if (i18n.getLocale() !== locale.toLowerCase()) { - throw Boom.notFound(`Unknown locale: ${locale}`); - } - - // Stringifying thousands of labels and calculating hash on the resulting - // string can be expensive so it makes sense to do it once and cache. - if (translationsCache.translations == null) { - translationsCache.translations = JSON.stringify(i18n.getTranslation()); - translationsCache.hash = createHash('sha1') - .update(translationsCache.translations) - .digest('hex'); - } - - return h - .response(translationsCache.translations) - .header('cache-control', 'must-revalidate') - .header('content-type', 'application/json') - .etag(translationsCache.hash); - }, - }); - const authEnabled = !!server.auth.settings.default; server.route({ path: '/bootstrap.js', From 83abbe12d179b1d6bb9a543f8c90a3b6cdeae73b Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 12 Nov 2020 15:12:34 +0100 Subject: [PATCH 2/2] add FTR test for endpoint --- src/core/server/i18n/routes/translations.ts | 2 +- test/api_integration/apis/core/compression.ts | 55 ++++++++++++++++++ test/api_integration/apis/core/index.js | 56 ------------------- test/api_integration/apis/core/index.ts | 27 +++++++++ .../api_integration/apis/core/translations.ts | 42 ++++++++++++++ 5 files changed, 125 insertions(+), 57 deletions(-) create mode 100644 test/api_integration/apis/core/compression.ts delete mode 100644 test/api_integration/apis/core/index.js create mode 100644 test/api_integration/apis/core/index.ts create mode 100644 test/api_integration/apis/core/translations.ts diff --git a/src/core/server/i18n/routes/translations.ts b/src/core/server/i18n/routes/translations.ts index 0ac2f8ecc54ca..c5cc9525d54aa 100644 --- a/src/core/server/i18n/routes/translations.ts +++ b/src/core/server/i18n/routes/translations.ts @@ -43,7 +43,7 @@ export const registerTranslationsRoute = (router: IRouter, locale: string) => { }, }, (ctx, req, res) => { - if (req.params.locale !== locale) { + if (req.params.locale.toLowerCase() !== locale.toLowerCase()) { return res.notFound({ body: `Unknown locale: ${req.params.locale}`, }); diff --git a/test/api_integration/apis/core/compression.ts b/test/api_integration/apis/core/compression.ts new file mode 100644 index 0000000000000..d7184e28ca3a4 --- /dev/null +++ b/test/api_integration/apis/core/compression.ts @@ -0,0 +1,55 @@ +/* + * 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 expect from '@kbn/expect'; +import { FtrProviderContext } from '../../ftr_provider_context'; + +export default function ({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + + describe('compression', () => { + it(`uses compression when there isn't a referer`, async () => { + await supertest + .get('/app/kibana') + .set('accept-encoding', 'gzip') + .then((response) => { + expect(response.header).to.have.property('content-encoding', 'gzip'); + }); + }); + + it(`uses compression when there is a whitelisted referer`, async () => { + await supertest + .get('/app/kibana') + .set('accept-encoding', 'gzip') + .set('referer', 'https://some-host.com') + .then((response) => { + expect(response.header).to.have.property('content-encoding', 'gzip'); + }); + }); + + it(`doesn't use compression when there is a non-whitelisted referer`, async () => { + await supertest + .get('/app/kibana') + .set('accept-encoding', 'gzip') + .set('referer', 'https://other.some-host.com') + .then((response) => { + expect(response.header).not.to.have.property('content-encoding'); + }); + }); + }); +} diff --git a/test/api_integration/apis/core/index.js b/test/api_integration/apis/core/index.js deleted file mode 100644 index ab9bb8d33c2dc..0000000000000 --- a/test/api_integration/apis/core/index.js +++ /dev/null @@ -1,56 +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 expect from '@kbn/expect'; - -export default function ({ getService }) { - const supertest = getService('supertest'); - - describe('core', () => { - describe('compression', () => { - it(`uses compression when there isn't a referer`, async () => { - await supertest - .get('/app/kibana') - .set('accept-encoding', 'gzip') - .then((response) => { - expect(response.headers).to.have.property('content-encoding', 'gzip'); - }); - }); - - it(`uses compression when there is a whitelisted referer`, async () => { - await supertest - .get('/app/kibana') - .set('accept-encoding', 'gzip') - .set('referer', 'https://some-host.com') - .then((response) => { - expect(response.headers).to.have.property('content-encoding', 'gzip'); - }); - }); - - it(`doesn't use compression when there is a non-whitelisted referer`, async () => { - await supertest - .get('/app/kibana') - .set('accept-encoding', 'gzip') - .set('referer', 'https://other.some-host.com') - .then((response) => { - expect(response.headers).not.to.have.property('content-encoding'); - }); - }); - }); - }); -} diff --git a/test/api_integration/apis/core/index.ts b/test/api_integration/apis/core/index.ts new file mode 100644 index 0000000000000..6a1d7db769df5 --- /dev/null +++ b/test/api_integration/apis/core/index.ts @@ -0,0 +1,27 @@ +/* + * 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 { FtrProviderContext } from '../../ftr_provider_context'; + +export default function ({ loadTestFile }: FtrProviderContext) { + describe('core', () => { + loadTestFile(require.resolve('./compression')); + loadTestFile(require.resolve('./translations')); + }); +} diff --git a/test/api_integration/apis/core/translations.ts b/test/api_integration/apis/core/translations.ts new file mode 100644 index 0000000000000..865d3d070f39a --- /dev/null +++ b/test/api_integration/apis/core/translations.ts @@ -0,0 +1,42 @@ +/* + * 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 expect from '@kbn/expect'; +import { FtrProviderContext } from '../../ftr_provider_context'; + +export default function ({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + + describe('translations', () => { + it(`returns the translations with the correct headers`, async () => { + await supertest.get('/translations/en.json').then((response) => { + expect(response.body.locale).to.eql('en'); + + expect(response.header).to.have.property('content-type', 'application/json; charset=utf-8'); + expect(response.header).to.have.property('cache-control', 'must-revalidate'); + expect(response.header).to.have.property('etag'); + }); + }); + + it(`returns a 404 when not using the correct locale`, async () => { + await supertest.get('/translations/foo.json').then((response) => { + expect(response.status).to.eql(404); + }); + }); + }); +}