diff --git a/src/http-data-source.ts b/src/http-data-source.ts index 77748fe..1acf8d2 100644 --- a/src/http-data-source.ts +++ b/src/http-data-source.ts @@ -48,6 +48,8 @@ export type Request = { origin: string path: string method: HttpMethod + // Indicates if the response of this request should be memoized + memoize?: boolean headers: Dictionary } & CacheTTLOptions @@ -87,7 +89,7 @@ export abstract class HTTPDataSource extends DataSource { private logger?: Logger private cache!: KeyValueCache private globalRequestOptions?: RequestOptions - private readonly memoizedResults: QuickLRU>> + private readonly memoizedResults: QuickLRU> constructor(public readonly baseURL: string, private readonly options?: HTTPDataSourceOptions) { super() @@ -141,6 +143,16 @@ export abstract class HTTPDataSource extends DataSource { return statusCodeCacheableByDefault.has(response.statusCode) && request.method === 'GET' } + /** + * Checks if the GET request is memoizable. This validation is performed before the + * response is set in **memoizedResults**. + * @param request + * @returns *true* if request should be memoized + */ + protected isRequestMemoizable(request: Request): boolean { + return Boolean(request.memoize) && request.method === 'GET' + } + /** * onCacheKeyCalculation returns the key for the GET request. * The key is used to memoize the request in the LRU cache. @@ -185,6 +197,14 @@ export abstract class HTTPDataSource extends DataSource { protected onError?(_error: Error, requestOptions: Request): void + /** + * Execute a HTTP GET request. + * Note that the **memoizedResults** and **cache** will be checked before request is made. + * By default the received response will be memoized. + * + * @param path the path to the resource + * @param requestOptions + */ public async get( path: string, requestOptions?: RequestOptions, @@ -193,6 +213,7 @@ export abstract class HTTPDataSource extends DataSource { headers: {}, query: {}, body: null, + memoize: true, context: {}, ...requestOptions, method: 'GET', @@ -312,6 +333,10 @@ export abstract class HTTPDataSource extends DataSource { this.onResponse(request, response) + if (this.isRequestMemoizable(request)) { + this.memoizedResults.set(cacheKey, response) + } + // let's see if we can fill the shared cache if (request.requestCache && this.isResponseCacheable(request, response)) { response.maxTtl = request.requestCache.maxTtl @@ -390,7 +415,7 @@ export abstract class HTTPDataSource extends DataSource { return cachedResponse } const response = this.performRequest(options, cacheKey) - this.memoizedResults.set(cacheKey, response) + return response } catch (error: any) { this.logger?.error(`Cache item '${cacheKey}' could not be loaded: ${error.message}`) @@ -398,7 +423,6 @@ export abstract class HTTPDataSource extends DataSource { } const response = this.performRequest(options, cacheKey) - this.memoizedResults.set(cacheKey, response) return response } diff --git a/test/http-data-source.test.ts b/test/http-data-source.test.ts index de3cc03..3ca3bfb 100644 --- a/test/http-data-source.test.ts +++ b/test/http-data-source.test.ts @@ -38,7 +38,7 @@ test('Should be able to make a simple GET call', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -78,7 +78,7 @@ test('Should be able to make a simple POST call', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -125,7 +125,7 @@ test('Should be able to make a simple POST with JSON body', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -166,7 +166,7 @@ test('Should be able to make a simple DELETE call', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -203,7 +203,7 @@ test('Should be able to make a simple PUT call', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -240,7 +240,7 @@ test('Should be able to make a simple PATCH call', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -278,7 +278,7 @@ test('Should be able to pass query params', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -317,7 +317,7 @@ test('Should error on HTTP errors > 299 and != 304', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -372,7 +372,7 @@ test('Should not parse content as JSON when content-type header is missing', asy server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -409,7 +409,7 @@ test('Should memoize subsequent GET calls to the same endpoint', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -445,6 +445,150 @@ test('Should memoize subsequent GET calls to the same endpoint', async (t) => { t.falsy(response.maxTtl) }) +test('Should memoize subsequent GET calls to the same endpoint when the memoize option is undefined', async (t) => { + t.plan(17) + + const path = '/' + + const MAX_SUBSEQUENT_CALLS = 3 + + const wanted = { name: 'foo' } + + const server = http.createServer((req, res) => { + t.is(req.method, 'GET') + res.writeHead(200, { + 'content-type': 'application/json', + }) + res.write(JSON.stringify(wanted)) + setTimeout(() => res.end(), 50).unref() + res.socket?.unref() + }) + + t.teardown(server.close.bind(server)) + + server.listen() + + const baseURL = getBaseUrlOf(server) + + const dataSource = new (class extends HTTPDataSource { + constructor() { + super(baseURL) + } + getFoo() { + return this.get(path) + } + })() + + const firstCall = await dataSource.getFoo() + t.deepEqual(firstCall.body, wanted) + t.false(firstCall.isFromCache) + t.false(firstCall.memoized) + t.falsy(firstCall.maxTtl) + + for (let currentCall = 0; currentCall < MAX_SUBSEQUENT_CALLS; currentCall++) { + const subsequentCall = await dataSource.getFoo() + + t.deepEqual(subsequentCall.body, wanted) + t.false(subsequentCall.isFromCache) + t.true(subsequentCall.memoized) + t.falsy(subsequentCall.maxTtl) + } +}) + +test('Should memoize subsequent GET calls to the same endpoint when the memoize option is true', async (t) => { + t.plan(17) + + const path = '/' + + const MAX_SUBSEQUENT_CALLS = 3 + + const wanted = { name: 'foo' } + + const server = http.createServer((req, res) => { + t.is(req.method, 'GET') + res.writeHead(200, { + 'content-type': 'application/json', + }) + res.write(JSON.stringify(wanted)) + setTimeout(() => res.end(), 50).unref() + res.socket?.unref() + }) + + t.teardown(server.close.bind(server)) + + server.listen() + + const baseURL = getBaseUrlOf(server) + + const dataSource = new (class extends HTTPDataSource { + constructor() { + super(baseURL) + } + getFoo() { + return this.get(path, { memoize: true }) + } + })() + + const firstCall = await dataSource.getFoo() + t.deepEqual(firstCall.body, wanted) + t.false(firstCall.isFromCache) + t.false(firstCall.memoized) + t.falsy(firstCall.maxTtl) + + for (let currentCall = 0; currentCall < MAX_SUBSEQUENT_CALLS; currentCall++) { + const subsequentCall = await dataSource.getFoo() + + t.deepEqual(subsequentCall.body, wanted) + t.false(subsequentCall.isFromCache) + t.true(subsequentCall.memoized) + t.falsy(subsequentCall.maxTtl) + } +}) + +test('Should not memoize subsequent GET calls to the same endpoint when the memoize option is false', async (t) => { + t.plan(15) + + const path = '/' + + const MAX_CALLS = 3 + + const wanted = { name: 'foo' } + + const server = http.createServer((req, res) => { + t.is(req.method, 'GET') + res.writeHead(200, { + 'content-type': 'application/json', + }) + res.write(JSON.stringify(wanted)) + setTimeout(() => res.end(), 50).unref() + res.socket?.unref() + }) + + t.teardown(server.close.bind(server)) + + server.listen() + + const baseURL = getBaseUrlOf(server) + + const dataSource = new (class extends HTTPDataSource { + constructor() { + super(baseURL) + } + getFoo() { + return this.get(path, { memoize: false }) + } + })() + + for (let currentCall = 0; currentCall < MAX_CALLS; currentCall++) { + const subsequentCall = await dataSource.getFoo() + + t.deepEqual(subsequentCall.body, wanted) + t.false(subsequentCall.isFromCache) + t.false(subsequentCall.memoized) + t.falsy(subsequentCall.maxTtl) + } +}) + test('Should not memoize subsequent GET calls for unsuccessful responses', async (t) => { t.plan(17) @@ -467,7 +611,7 @@ test('Should not memoize subsequent GET calls for unsuccessful responses', async server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -529,7 +673,7 @@ test('Should be able to define a custom cache key for request memoization', asyn server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -574,7 +718,7 @@ test('Should correctly calculate and sort query parameters', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -611,7 +755,7 @@ test('Should call onError on request error', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -668,7 +812,7 @@ test('Should be possible to pass a request context', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -713,7 +857,7 @@ test.cb('Should abort request when abortController signal is called', (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const abortController = new AbortController() @@ -769,7 +913,7 @@ test.cb('Should timeout because server does not respond fast enough', (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -826,7 +970,7 @@ test('Should be able to modify request in willSendRequest', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -869,7 +1013,7 @@ test('Should be able to define base headers for every request', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -917,7 +1061,7 @@ test('Initialize data source with cache and context', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -976,7 +1120,7 @@ test('Should cache a GET response and respond with the result on subsequent call server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) let dataSource = new (class extends HTTPDataSource { constructor() { @@ -1093,7 +1237,7 @@ test('Should respond with stale-if-error cache on origin error', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) let dataSource = new (class extends HTTPDataSource { constructor() { @@ -1187,7 +1331,7 @@ test('Should not cache POST requests', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -1254,7 +1398,7 @@ test('Should only cache GET successful responses with the correct status code', server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -1354,11 +1498,9 @@ test.serial('Global maxAge should be used when no maxAge was set or similar.', a server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) - let testResponse: Response | { - memoized: boolean - } = { + let testResponse: Pick, 'memoized'> = { memoized: false, } const maxAge = 10000 @@ -1366,8 +1508,8 @@ test.serial('Global maxAge should be used when no maxAge was set or similar.', a constructor() { super(baseURL, { lru: { - maxAge: maxAge - } + maxAge: maxAge, + }, }) } getFoo() { @@ -1402,7 +1544,7 @@ test('Response is not cached due to origin error', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const dataSource = new (class extends HTTPDataSource { constructor() { @@ -1472,7 +1614,7 @@ test('Should be able to pass custom Undici Pool', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const pool = new Pool(baseURL) const dataSource = new (class extends HTTPDataSource { @@ -1498,17 +1640,19 @@ test('Should be merge headers', async (t) => { const mockHeaders = { 'test-a': 'a', - 'test-b': 'b' + 'test-b': 'b', } const server = http.createServer((req, res) => { t.is(req.method, 'GET') res.writeHead(200, { 'content-type': 'application/json', }) - res.write(JSON.stringify({ - 'test-a': req.headers['test-a'], - 'test-b': req.headers['test-b'] - })) + res.write( + JSON.stringify({ + 'test-a': req.headers['test-a'], + 'test-b': req.headers['test-b'], + }), + ) res.end() res.socket?.unref() }) @@ -1517,7 +1661,7 @@ test('Should be merge headers', async (t) => { server.listen() - const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}` + const baseURL = getBaseUrlOf(server) const pool = new Pool(baseURL) const dataSource = new (class extends HTTPDataSource { @@ -1526,15 +1670,17 @@ test('Should be merge headers', async (t) => { pool, requestOptions: { headers: { - 'test-a': mockHeaders['test-a'] - } - } + 'test-a': mockHeaders['test-a'], + }, + }, }) } getFoo() { - return this.get(path, {headers: { - 'test-b': mockHeaders['test-b'] - }}) + return this.get(path, { + headers: { + 'test-b': mockHeaders['test-b'], + }, + }) } })() @@ -1542,3 +1688,8 @@ test('Should be merge headers', async (t) => { t.deepEqual(response.body, mockHeaders) }) + +// Utils +function getBaseUrlOf(server: http.Server) { + return `http://localhost:${(server.address() as AddressInfo)?.port}` +}