-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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
- Loading branch information
1 parent
bdcd2ec
commit 25e586a
Showing
34 changed files
with
917 additions
and
740 deletions.
There are no files selected for viewing
Validating CODEOWNERS rules …
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
12 changes: 12 additions & 0 deletions
12
src/core/server/core_app/bundle_routes/bundle_route.test.mocks.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. | ||
*/ | ||
|
||
export const createDynamicAssetHandlerMock = jest.fn(); | ||
jest.doMock('./dynamic_asset_response', () => ({ | ||
createDynamicAssetHandler: createDynamicAssetHandlerMock, | ||
})); |
70 changes: 70 additions & 0 deletions
70
src/core/server/core_app/bundle_routes/bundle_route.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
/* | ||
* 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. | ||
*/ | ||
|
||
import { createDynamicAssetHandlerMock } from './bundle_route.test.mocks'; | ||
|
||
import { httpServiceMock } from '../../http/http_service.mock'; | ||
import { FileHashCache } from './file_hash_cache'; | ||
import { registerRouteForBundle } from './bundles_route'; | ||
|
||
describe('registerRouteForBundle', () => { | ||
let router: ReturnType<typeof httpServiceMock.createRouter>; | ||
let fileHashCache: FileHashCache; | ||
|
||
beforeEach(() => { | ||
router = httpServiceMock.createRouter(); | ||
fileHashCache = new FileHashCache(); | ||
}); | ||
|
||
afterEach(() => { | ||
createDynamicAssetHandlerMock.mockReset(); | ||
}); | ||
|
||
it('calls `router.get` with the correct parameters', () => { | ||
const handler = jest.fn(); | ||
createDynamicAssetHandlerMock.mockReturnValue(handler); | ||
|
||
registerRouteForBundle(router, { | ||
isDist: false, | ||
publicPath: '/public-path/', | ||
bundlesPath: '/bundle-path', | ||
fileHashCache, | ||
routePath: '/route-path/', | ||
}); | ||
|
||
expect(router.get).toHaveBeenCalledTimes(1); | ||
expect(router.get).toHaveBeenCalledWith( | ||
{ | ||
path: '/route-path/{path*}', | ||
options: { | ||
authRequired: false, | ||
}, | ||
validate: expect.any(Object), | ||
}, | ||
handler | ||
); | ||
}); | ||
|
||
it('calls `createDynamicAssetHandler` with the correct parameters', () => { | ||
registerRouteForBundle(router, { | ||
isDist: false, | ||
publicPath: '/public-path/', | ||
bundlesPath: '/bundle-path', | ||
fileHashCache, | ||
routePath: '/route-path/', | ||
}); | ||
|
||
expect(createDynamicAssetHandlerMock).toHaveBeenCalledTimes(1); | ||
expect(createDynamicAssetHandlerMock).toHaveBeenCalledWith({ | ||
isDist: false, | ||
publicPath: '/public-path/', | ||
bundlesPath: '/bundle-path', | ||
fileHashCache, | ||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
/* | ||
* 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. | ||
*/ | ||
|
||
import { schema } from '@kbn/config-schema'; | ||
import { IRouter } from '../../http'; | ||
import { createDynamicAssetHandler } from './dynamic_asset_response'; | ||
import { FileHashCache } from './file_hash_cache'; | ||
|
||
export function registerRouteForBundle( | ||
router: IRouter, | ||
{ | ||
publicPath, | ||
routePath, | ||
bundlesPath, | ||
fileHashCache, | ||
isDist, | ||
}: { | ||
publicPath: string; | ||
routePath: string; | ||
bundlesPath: string; | ||
fileHashCache: FileHashCache; | ||
isDist: boolean; | ||
} | ||
) { | ||
router.get( | ||
{ | ||
path: `${routePath}{path*}`, | ||
options: { | ||
authRequired: false, | ||
}, | ||
validate: { | ||
params: schema.object({ | ||
path: schema.string(), | ||
}), | ||
}, | ||
}, | ||
createDynamicAssetHandler({ | ||
publicPath, | ||
bundlesPath, | ||
isDist, | ||
fileHashCache, | ||
}) | ||
); | ||
} |
124 changes: 124 additions & 0 deletions
124
src/core/server/core_app/bundle_routes/dynamic_asset_response.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
/* | ||
* 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. | ||
*/ | ||
|
||
import { createReadStream } from 'fs'; | ||
import { resolve, extname } from 'path'; | ||
import mime from 'mime-types'; | ||
import agent from 'elastic-apm-node'; | ||
|
||
import { fstat, close } from './fs'; | ||
import { RequestHandler } from '../../http'; | ||
import { IFileHashCache } from './file_hash_cache'; | ||
import { getFileHash } from './file_hash'; | ||
import { selectCompressedFile } from './select_compressed_file'; | ||
|
||
const MINUTE = 60; | ||
const HOUR = 60 * MINUTE; | ||
const DAY = 24 * HOUR; | ||
|
||
/** | ||
* Serve asset for the requested path. This is designed | ||
* to replicate a subset of the features provided by Hapi's Inert | ||
* plugin including: | ||
* - ensure path is not traversing out of the bundle directory | ||
* - manage use file descriptors for file access to efficiently | ||
* interact with the file multiple times in each request | ||
* - generate and cache etag for the file | ||
* - write correct headers to response for client-side caching | ||
* and invalidation | ||
* - stream file to response | ||
* | ||
* It differs from Inert in some important ways: | ||
* - cached hash/etag is based on the file on disk, but modified | ||
* by the public path so that individual public paths have | ||
* different etags, but can share a cache | ||
*/ | ||
export const createDynamicAssetHandler = ({ | ||
bundlesPath, | ||
fileHashCache, | ||
isDist, | ||
publicPath, | ||
}: { | ||
bundlesPath: string; | ||
publicPath: string; | ||
fileHashCache: IFileHashCache; | ||
isDist: boolean; | ||
}): RequestHandler<{ path: string }, {}, {}> => { | ||
return async (ctx, req, res) => { | ||
agent.setTransactionName('GET ?/bundles/?'); | ||
|
||
let fd: number | undefined; | ||
let fileEncoding: 'gzip' | 'br' | undefined; | ||
|
||
try { | ||
const path = resolve(bundlesPath, req.params.path); | ||
|
||
// prevent path traversal, only process paths that resolve within bundlesPath | ||
if (!path.startsWith(bundlesPath)) { | ||
return res.forbidden({ | ||
body: 'EACCES', | ||
}); | ||
} | ||
|
||
// we use and manage a file descriptor mostly because | ||
// that's what Inert does, and since we are accessing | ||
// the file 2 or 3 times per request it seems logical | ||
({ fd, fileEncoding } = await selectCompressedFile( | ||
req.headers['accept-encoding'] as string, | ||
path | ||
)); | ||
|
||
let headers: Record<string, string>; | ||
if (isDist) { | ||
headers = { 'cache-control': `max-age=${365 * DAY}` }; | ||
} else { | ||
const stat = await fstat(fd); | ||
const hash = await getFileHash(fileHashCache, path, stat, fd); | ||
headers = { | ||
etag: `${hash}-${publicPath}`, | ||
'cache-control': 'must-revalidate', | ||
}; | ||
} | ||
|
||
// If we manually selected a compressed file, specify the encoding header. | ||
// Otherwise, let Hapi automatically gzip the response. | ||
if (fileEncoding) { | ||
headers['content-encoding'] = fileEncoding; | ||
} | ||
|
||
const fileExt = extname(path); | ||
const contentType = mime.lookup(fileExt); | ||
const mediaType = mime.contentType(contentType || fileExt); | ||
headers['content-type'] = mediaType || ''; | ||
|
||
const content = createReadStream(null as any, { | ||
fd, | ||
start: 0, | ||
autoClose: true, | ||
}); | ||
|
||
return res.ok({ | ||
body: content, | ||
headers, | ||
}); | ||
} catch (error) { | ||
if (fd) { | ||
try { | ||
await close(fd); | ||
} catch (_) { | ||
// ignore errors from close, we already have one to report | ||
// and it's very likely they are the same | ||
} | ||
} | ||
if (error.code === 'ENOENT') { | ||
return res.notFound(); | ||
} | ||
throw error; | ||
} | ||
}; | ||
}; |
15 changes: 15 additions & 0 deletions
15
src/core/server/core_app/bundle_routes/file_hash.test.mocks.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
/* | ||
* 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. | ||
*/ | ||
|
||
export const generateFileHashMock = jest.fn(); | ||
export const getFileCacheKeyMock = jest.fn(); | ||
|
||
jest.doMock('./utils', () => ({ | ||
generateFileHash: generateFileHashMock, | ||
getFileCacheKey: getFileCacheKeyMock, | ||
})); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
/* | ||
* 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. | ||
*/ | ||
|
||
import { generateFileHashMock, getFileCacheKeyMock } from './file_hash.test.mocks'; | ||
|
||
import { resolve } from 'path'; | ||
import { Stats } from 'fs'; | ||
import { getFileHash } from './file_hash'; | ||
import { IFileHashCache } from './file_hash_cache'; | ||
|
||
const mockedCache = (): jest.Mocked<IFileHashCache> => ({ | ||
del: jest.fn(), | ||
get: jest.fn(), | ||
set: jest.fn(), | ||
}); | ||
|
||
describe('getFileHash', () => { | ||
const sampleFilePath = resolve(__dirname, 'foo.js'); | ||
const fd = 42; | ||
const stats: Stats = { ino: 42, size: 9000 } as any; | ||
|
||
beforeEach(() => { | ||
getFileCacheKeyMock.mockImplementation((path: string, stat: Stats) => `${path}-${stat.ino}`); | ||
}); | ||
|
||
afterEach(() => { | ||
generateFileHashMock.mockReset(); | ||
getFileCacheKeyMock.mockReset(); | ||
}); | ||
|
||
it('returns the value from cache if present', async () => { | ||
const cache = mockedCache(); | ||
cache.get.mockReturnValue(Promise.resolve('cached-hash')); | ||
|
||
const hash = await getFileHash(cache, sampleFilePath, stats, fd); | ||
|
||
expect(cache.get).toHaveBeenCalledTimes(1); | ||
expect(generateFileHashMock).not.toHaveBeenCalled(); | ||
expect(hash).toEqual('cached-hash'); | ||
}); | ||
|
||
it('computes the value if not present in cache', async () => { | ||
const cache = mockedCache(); | ||
cache.get.mockReturnValue(undefined); | ||
|
||
generateFileHashMock.mockReturnValue(Promise.resolve('computed-hash')); | ||
|
||
const hash = await getFileHash(cache, sampleFilePath, stats, fd); | ||
|
||
expect(generateFileHashMock).toHaveBeenCalledTimes(1); | ||
expect(generateFileHashMock).toHaveBeenCalledWith(fd); | ||
expect(hash).toEqual('computed-hash'); | ||
}); | ||
|
||
it('sets the value in the cache if not present', async () => { | ||
const computedHashPromise = Promise.resolve('computed-hash'); | ||
generateFileHashMock.mockReturnValue(computedHashPromise); | ||
|
||
const cache = mockedCache(); | ||
cache.get.mockReturnValue(undefined); | ||
|
||
await getFileHash(cache, sampleFilePath, stats, fd); | ||
|
||
expect(cache.set).toHaveBeenCalledTimes(1); | ||
expect(cache.set).toHaveBeenCalledWith(`${sampleFilePath}-${stats.ino}`, computedHashPromise); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
/* | ||
* 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. | ||
*/ | ||
|
||
import type { Stats } from 'fs'; | ||
import { generateFileHash, getFileCacheKey } from './utils'; | ||
import { IFileHashCache } from './file_hash_cache'; | ||
|
||
/** | ||
* Get the hash of a file via a file descriptor | ||
*/ | ||
export async function getFileHash(cache: IFileHashCache, path: string, stat: Stats, fd: number) { | ||
const key = getFileCacheKey(path, stat); | ||
|
||
const cached = cache.get(key); | ||
if (cached) { | ||
return await cached; | ||
} | ||
|
||
const promise = generateFileHash(fd).catch((error) => { | ||
// don't cache failed attempts | ||
cache.del(key); | ||
throw error; | ||
}); | ||
|
||
cache.set(key, promise); | ||
return await promise; | ||
} |
Oops, something went wrong.