Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add translation files to CDN assets #181650

Merged
merged 6 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,19 @@ describe('StaticAssets', () => {
});
});

describe('#isUsingCdn()', () => {
it('returns false when the CDN is not configured', () => {
staticAssets = new StaticAssets(args);
expect(staticAssets.isUsingCdn()).toBe(false);
});

it('returns true when the CDN is configured', () => {
args.cdnConfig = CdnConfig.from({ url: 'https://cdn.example.com/test' });
staticAssets = new StaticAssets(args);
expect(staticAssets.isUsingCdn()).toBe(true);
});
});

describe('#getPluginAssetHref()', () => {
it('returns the expected value when CDN is not configured', () => {
staticAssets = new StaticAssets(args);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ import {

export interface InternalStaticAssets {
getHrefBase(): string;
/**
* Returns true if a CDN has been configured and should be used to serve static assets.
* Should only be used in scenarios where different behavior has to be used when CDN is enabled or not.
*/
isUsingCdn(): boolean;
/**
* Intended for use by server code rendering UI or generating links to static assets
* that will ultimately be called from the browser and must respect settings like
Expand Down Expand Up @@ -67,6 +72,10 @@ export class StaticAssets implements InternalStaticAssets {
this.assetsServerPathBase = `/${shaDigest}`;
}

public isUsingCdn() {
return this.hasCdnHost;
}

/**
* Returns a href (hypertext reference) intended to be used as the base for constructing
* other hrefs to static assets.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ const createInternalStaticAssetsMock = (
basePath: BasePathMocked,
cdnUrl: undefined | string = undefined
): InternalStaticAssetsMocked => ({
isUsingCdn: jest.fn().mockReturnValue(!!cdnUrl),
getHrefBase: jest.fn().mockReturnValue(cdnUrl ?? basePath.serverBasePath),
getPluginAssetHref: jest.fn().mockReturnValue(cdnUrl ?? basePath.serverBasePath),
getPluginServerPath: jest.fn((v, _) => v),
Expand Down
2 changes: 2 additions & 0 deletions packages/core/i18n/core-i18n-server-internal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,5 @@

export type { I18nConfigType, InternalI18nServicePreboot } from './src';
export { config, I18nService } from './src';
export { getKibanaTranslationFiles } from './src/get_kibana_translation_files';
export { supportedLocale } from './src/constants';
12 changes: 12 additions & 0 deletions packages/core/i18n/core-i18n-server-internal/src/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

/**
* List of all locales that are officially supported.
*/
export const supportedLocale = ['en', 'fr-FR', 'ja-JP', 'zh-CN'];
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,42 @@ function renderTestCases(
const data = JSON.parse(dom('kbn-injected-metadata').attr('data') ?? '""');
expect(data.logging).toEqual(loggingConfig);
});

it('use the correct translation url when CDN is enabled', async () => {
const userSettings = { 'theme:darkMode': { userValue: true } };
uiSettings.client.getUserProvided.mockResolvedValue(userSettings);

const [render, deps] = await getRender();

(deps.http.staticAssets.getHrefBase as jest.Mock).mockReturnValueOnce('http://foo.bar:1773');
(deps.http.staticAssets.isUsingCdn as jest.Mock).mockReturnValueOnce(true);

const content = await render(createKibanaRequest(), uiSettings, {
isAnonymousPage: false,
});
const dom = load(content);
const data = JSON.parse(dom('kbn-injected-metadata').attr('data') ?? '""');
expect(data.i18n.translationsUrl).toEqual('http://foo.bar:1773/translations/en.json');
});

it('use the correct translation url when CDN is disabled', async () => {
const userSettings = { 'theme:darkMode': { userValue: true } };
uiSettings.client.getUserProvided.mockResolvedValue(userSettings);

const [render, deps] = await getRender();

(deps.http.staticAssets.getHrefBase as jest.Mock).mockReturnValueOnce('http://foo.bar:1773');
(deps.http.staticAssets.isUsingCdn as jest.Mock).mockReturnValueOnce(false);

const content = await render(createKibanaRequest(), uiSettings, {
isAnonymousPage: false,
});
const dom = load(content);
const data = JSON.parse(dom('kbn-injected-metadata').attr('data') ?? '""');
expect(data.i18n.translationsUrl).toEqual(
'/mock-server-basepath/translations/MOCK_HASH/en.json'
);
});
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ export class RenderingService {
packageInfo: this.coreContext.env.packageInfo,
};
const staticAssetsHrefBase = http.staticAssets.getHrefBase();
const usingCdn = http.staticAssets.isUsingCdn();
const basePath = http.basePath.get(request);
const { serverBasePath, publicBaseUrl } = http.basePath;

Expand Down Expand Up @@ -205,8 +206,14 @@ export class RenderingService {

const loggingConfig = await getBrowserLoggingConfig(this.coreContext.configService);

const translationHash = i18n.getTranslationHash();
const translationsUrl = `${serverBasePath}/translations/${translationHash}/${i18nLib.getLocale()}.json`;
const locale = i18nLib.getLocale();
let translationsUrl: string;
if (usingCdn) {
translationsUrl = `${staticAssetsHrefBase}/translations/${locale}.json`;
} else {
const translationHash = i18n.getTranslationHash();
translationsUrl = `${serverBasePath}/translations/${translationHash}/${locale}.json`;
}

const filteredPlugins = filterUiPlugins({ uiPlugins, isAnonymousPage });
const bootstrapScript = isAnonymousPage ? 'bootstrap-anonymous.js' : 'bootstrap.js';
Expand All @@ -215,7 +222,7 @@ export class RenderingService {
uiPublicUrl: `${staticAssetsHrefBase}/ui`,
bootstrapScriptUrl: `${basePath}/${bootstrapScript}`,
i18n: i18nLib.translate,
locale: i18nLib.getLocale(),
locale,
themeVersion,
darkMode,
stylesheetPaths: commonStylesheetPaths,
Expand All @@ -239,7 +246,6 @@ export class RenderingService {
clusterInfo,
anonymousStatusPage: status?.isStatusPageAnonymous() ?? false,
i18n: {
// TODO: Make this load as part of static assets!
translationsUrl,
},
theme: {
Expand Down
29 changes: 26 additions & 3 deletions src/dev/build/tasks/create_cdn_assets_task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ import { access } from 'fs/promises';
import { resolve, dirname } from 'path';
import { asyncForEach } from '@kbn/std';
import { Jsonc } from '@kbn/repo-packages';
import { getKibanaTranslationFiles, supportedLocale } from '@kbn/core-i18n-server-internal';
import { i18n, i18nLoader } from '@kbn/i18n';

import del from 'del';
import globby from 'globby';

import { mkdirp, compressTar, Task, copyAll } from '../lib';
import { mkdirp, compressTar, Task, copyAll, write } from '../lib';

export const CreateCdnAssets: Task = {
description: 'Creating CDN assets',
Expand All @@ -31,9 +33,19 @@ export const CreateCdnAssets: Task = {
await del(assets);
await mkdirp(assets);

// Plugins

const plugins = globby.sync([`${buildSource}/node_modules/@kbn/**/*/kibana.jsonc`]);

// translation files
const pluginPaths = plugins.map((plugin) => resolve(dirname(plugin)));
for (const locale of supportedLocale) {
const translationFileContent = await generateTranslationFile(locale, pluginPaths);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this deterministic? Any reason not to copy over the files from node_modules/@kbn/translations-plugin/translations?

Copy link
Contributor Author

@pgayvallet pgayvallet Apr 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the actual translation files sent to the server aren't following the exact same format than the ones in node_modules/@kbn/translations-plugin/translations (even if in practice, the files from the translations plugin is the only "source" to generate those final ones).

await write(
resolve(assets, buildSha, `translations`, `${locale}.json`),
translationFileContent
);
}

// Plugins static assets
await asyncForEach(plugins, async (path) => {
const manifest = Jsonc.parse(readFileSync(path, 'utf8')) as any;
if (manifest?.plugin?.id) {
Expand Down Expand Up @@ -101,3 +113,14 @@ export const CreateCdnAssets: Task = {
});
},
};

async function generateTranslationFile(locale: string, pluginPaths: string[]) {
const translationFiles = await getKibanaTranslationFiles(locale, pluginPaths);
i18nLoader.registerTranslationFiles(translationFiles);
const translations = await i18nLoader.getTranslationsByLocale(locale);
i18n.init({
locale,
...translations,
});
return JSON.stringify(i18n.getTranslation());
}
1 change: 1 addition & 0 deletions src/dev/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,6 @@
"@kbn/core-test-helpers-so-type-serializer",
"@kbn/core-test-helpers-kbn-server",
"@kbn/dev-proc-runner",
"@kbn/core-i18n-server-internal",
]
}