Skip to content

Commit 25e586a

Browse files
authored
Migrate the optimizer mixin to core (#94272)
* migrate optimizer mixin to core apps * fix core_app tests * add integration tests, extract selectCompressedFile * add CoreApp unit test * more unit tests * unit tests for bundle_route * more unit tests * remove /src/optimize/ from codeowners * fix case * NIT
1 parent bdcd2ec commit 25e586a

34 files changed

+917
-740
lines changed

.github/CODEOWNERS

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@
145145
# Operations
146146
/src/dev/ @elastic/kibana-operations
147147
/src/setup_node_env/ @elastic/kibana-operations
148-
/src/optimize/ @elastic/kibana-operations
149148
/packages/*eslint*/ @elastic/kibana-operations
150149
/packages/*babel*/ @elastic/kibana-operations
151150
/packages/kbn-dev-utils*/ @elastic/kibana-operations
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
export const createDynamicAssetHandlerMock = jest.fn();
10+
jest.doMock('./dynamic_asset_response', () => ({
11+
createDynamicAssetHandler: createDynamicAssetHandlerMock,
12+
}));
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
import { createDynamicAssetHandlerMock } from './bundle_route.test.mocks';
10+
11+
import { httpServiceMock } from '../../http/http_service.mock';
12+
import { FileHashCache } from './file_hash_cache';
13+
import { registerRouteForBundle } from './bundles_route';
14+
15+
describe('registerRouteForBundle', () => {
16+
let router: ReturnType<typeof httpServiceMock.createRouter>;
17+
let fileHashCache: FileHashCache;
18+
19+
beforeEach(() => {
20+
router = httpServiceMock.createRouter();
21+
fileHashCache = new FileHashCache();
22+
});
23+
24+
afterEach(() => {
25+
createDynamicAssetHandlerMock.mockReset();
26+
});
27+
28+
it('calls `router.get` with the correct parameters', () => {
29+
const handler = jest.fn();
30+
createDynamicAssetHandlerMock.mockReturnValue(handler);
31+
32+
registerRouteForBundle(router, {
33+
isDist: false,
34+
publicPath: '/public-path/',
35+
bundlesPath: '/bundle-path',
36+
fileHashCache,
37+
routePath: '/route-path/',
38+
});
39+
40+
expect(router.get).toHaveBeenCalledTimes(1);
41+
expect(router.get).toHaveBeenCalledWith(
42+
{
43+
path: '/route-path/{path*}',
44+
options: {
45+
authRequired: false,
46+
},
47+
validate: expect.any(Object),
48+
},
49+
handler
50+
);
51+
});
52+
53+
it('calls `createDynamicAssetHandler` with the correct parameters', () => {
54+
registerRouteForBundle(router, {
55+
isDist: false,
56+
publicPath: '/public-path/',
57+
bundlesPath: '/bundle-path',
58+
fileHashCache,
59+
routePath: '/route-path/',
60+
});
61+
62+
expect(createDynamicAssetHandlerMock).toHaveBeenCalledTimes(1);
63+
expect(createDynamicAssetHandlerMock).toHaveBeenCalledWith({
64+
isDist: false,
65+
publicPath: '/public-path/',
66+
bundlesPath: '/bundle-path',
67+
fileHashCache,
68+
});
69+
});
70+
});
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
import { schema } from '@kbn/config-schema';
10+
import { IRouter } from '../../http';
11+
import { createDynamicAssetHandler } from './dynamic_asset_response';
12+
import { FileHashCache } from './file_hash_cache';
13+
14+
export function registerRouteForBundle(
15+
router: IRouter,
16+
{
17+
publicPath,
18+
routePath,
19+
bundlesPath,
20+
fileHashCache,
21+
isDist,
22+
}: {
23+
publicPath: string;
24+
routePath: string;
25+
bundlesPath: string;
26+
fileHashCache: FileHashCache;
27+
isDist: boolean;
28+
}
29+
) {
30+
router.get(
31+
{
32+
path: `${routePath}{path*}`,
33+
options: {
34+
authRequired: false,
35+
},
36+
validate: {
37+
params: schema.object({
38+
path: schema.string(),
39+
}),
40+
},
41+
},
42+
createDynamicAssetHandler({
43+
publicPath,
44+
bundlesPath,
45+
isDist,
46+
fileHashCache,
47+
})
48+
);
49+
}
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
import { createReadStream } from 'fs';
10+
import { resolve, extname } from 'path';
11+
import mime from 'mime-types';
12+
import agent from 'elastic-apm-node';
13+
14+
import { fstat, close } from './fs';
15+
import { RequestHandler } from '../../http';
16+
import { IFileHashCache } from './file_hash_cache';
17+
import { getFileHash } from './file_hash';
18+
import { selectCompressedFile } from './select_compressed_file';
19+
20+
const MINUTE = 60;
21+
const HOUR = 60 * MINUTE;
22+
const DAY = 24 * HOUR;
23+
24+
/**
25+
* Serve asset for the requested path. This is designed
26+
* to replicate a subset of the features provided by Hapi's Inert
27+
* plugin including:
28+
* - ensure path is not traversing out of the bundle directory
29+
* - manage use file descriptors for file access to efficiently
30+
* interact with the file multiple times in each request
31+
* - generate and cache etag for the file
32+
* - write correct headers to response for client-side caching
33+
* and invalidation
34+
* - stream file to response
35+
*
36+
* It differs from Inert in some important ways:
37+
* - cached hash/etag is based on the file on disk, but modified
38+
* by the public path so that individual public paths have
39+
* different etags, but can share a cache
40+
*/
41+
export const createDynamicAssetHandler = ({
42+
bundlesPath,
43+
fileHashCache,
44+
isDist,
45+
publicPath,
46+
}: {
47+
bundlesPath: string;
48+
publicPath: string;
49+
fileHashCache: IFileHashCache;
50+
isDist: boolean;
51+
}): RequestHandler<{ path: string }, {}, {}> => {
52+
return async (ctx, req, res) => {
53+
agent.setTransactionName('GET ?/bundles/?');
54+
55+
let fd: number | undefined;
56+
let fileEncoding: 'gzip' | 'br' | undefined;
57+
58+
try {
59+
const path = resolve(bundlesPath, req.params.path);
60+
61+
// prevent path traversal, only process paths that resolve within bundlesPath
62+
if (!path.startsWith(bundlesPath)) {
63+
return res.forbidden({
64+
body: 'EACCES',
65+
});
66+
}
67+
68+
// we use and manage a file descriptor mostly because
69+
// that's what Inert does, and since we are accessing
70+
// the file 2 or 3 times per request it seems logical
71+
({ fd, fileEncoding } = await selectCompressedFile(
72+
req.headers['accept-encoding'] as string,
73+
path
74+
));
75+
76+
let headers: Record<string, string>;
77+
if (isDist) {
78+
headers = { 'cache-control': `max-age=${365 * DAY}` };
79+
} else {
80+
const stat = await fstat(fd);
81+
const hash = await getFileHash(fileHashCache, path, stat, fd);
82+
headers = {
83+
etag: `${hash}-${publicPath}`,
84+
'cache-control': 'must-revalidate',
85+
};
86+
}
87+
88+
// If we manually selected a compressed file, specify the encoding header.
89+
// Otherwise, let Hapi automatically gzip the response.
90+
if (fileEncoding) {
91+
headers['content-encoding'] = fileEncoding;
92+
}
93+
94+
const fileExt = extname(path);
95+
const contentType = mime.lookup(fileExt);
96+
const mediaType = mime.contentType(contentType || fileExt);
97+
headers['content-type'] = mediaType || '';
98+
99+
const content = createReadStream(null as any, {
100+
fd,
101+
start: 0,
102+
autoClose: true,
103+
});
104+
105+
return res.ok({
106+
body: content,
107+
headers,
108+
});
109+
} catch (error) {
110+
if (fd) {
111+
try {
112+
await close(fd);
113+
} catch (_) {
114+
// ignore errors from close, we already have one to report
115+
// and it's very likely they are the same
116+
}
117+
}
118+
if (error.code === 'ENOENT') {
119+
return res.notFound();
120+
}
121+
throw error;
122+
}
123+
};
124+
};
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
export const generateFileHashMock = jest.fn();
10+
export const getFileCacheKeyMock = jest.fn();
11+
12+
jest.doMock('./utils', () => ({
13+
generateFileHash: generateFileHashMock,
14+
getFileCacheKey: getFileCacheKeyMock,
15+
}));
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
import { generateFileHashMock, getFileCacheKeyMock } from './file_hash.test.mocks';
10+
11+
import { resolve } from 'path';
12+
import { Stats } from 'fs';
13+
import { getFileHash } from './file_hash';
14+
import { IFileHashCache } from './file_hash_cache';
15+
16+
const mockedCache = (): jest.Mocked<IFileHashCache> => ({
17+
del: jest.fn(),
18+
get: jest.fn(),
19+
set: jest.fn(),
20+
});
21+
22+
describe('getFileHash', () => {
23+
const sampleFilePath = resolve(__dirname, 'foo.js');
24+
const fd = 42;
25+
const stats: Stats = { ino: 42, size: 9000 } as any;
26+
27+
beforeEach(() => {
28+
getFileCacheKeyMock.mockImplementation((path: string, stat: Stats) => `${path}-${stat.ino}`);
29+
});
30+
31+
afterEach(() => {
32+
generateFileHashMock.mockReset();
33+
getFileCacheKeyMock.mockReset();
34+
});
35+
36+
it('returns the value from cache if present', async () => {
37+
const cache = mockedCache();
38+
cache.get.mockReturnValue(Promise.resolve('cached-hash'));
39+
40+
const hash = await getFileHash(cache, sampleFilePath, stats, fd);
41+
42+
expect(cache.get).toHaveBeenCalledTimes(1);
43+
expect(generateFileHashMock).not.toHaveBeenCalled();
44+
expect(hash).toEqual('cached-hash');
45+
});
46+
47+
it('computes the value if not present in cache', async () => {
48+
const cache = mockedCache();
49+
cache.get.mockReturnValue(undefined);
50+
51+
generateFileHashMock.mockReturnValue(Promise.resolve('computed-hash'));
52+
53+
const hash = await getFileHash(cache, sampleFilePath, stats, fd);
54+
55+
expect(generateFileHashMock).toHaveBeenCalledTimes(1);
56+
expect(generateFileHashMock).toHaveBeenCalledWith(fd);
57+
expect(hash).toEqual('computed-hash');
58+
});
59+
60+
it('sets the value in the cache if not present', async () => {
61+
const computedHashPromise = Promise.resolve('computed-hash');
62+
generateFileHashMock.mockReturnValue(computedHashPromise);
63+
64+
const cache = mockedCache();
65+
cache.get.mockReturnValue(undefined);
66+
67+
await getFileHash(cache, sampleFilePath, stats, fd);
68+
69+
expect(cache.set).toHaveBeenCalledTimes(1);
70+
expect(cache.set).toHaveBeenCalledWith(`${sampleFilePath}-${stats.ino}`, computedHashPromise);
71+
});
72+
});
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
import type { Stats } from 'fs';
10+
import { generateFileHash, getFileCacheKey } from './utils';
11+
import { IFileHashCache } from './file_hash_cache';
12+
13+
/**
14+
* Get the hash of a file via a file descriptor
15+
*/
16+
export async function getFileHash(cache: IFileHashCache, path: string, stat: Stats, fd: number) {
17+
const key = getFileCacheKey(path, stat);
18+
19+
const cached = cache.get(key);
20+
if (cached) {
21+
return await cached;
22+
}
23+
24+
const promise = generateFileHash(fd).catch((error) => {
25+
// don't cache failed attempts
26+
cache.del(key);
27+
throw error;
28+
});
29+
30+
cache.set(key, promise);
31+
return await promise;
32+
}

0 commit comments

Comments
 (0)