From 290b78d87dee349ce76a8b4b4b2a4b0cd1e33676 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 25 May 2023 15:17:43 +0300 Subject: [PATCH] feat(http): Etag support (#22302) --- lib/util/http/bitbucket-server.ts | 9 +++- lib/util/http/bitbucket.ts | 6 +-- lib/util/http/gitea.ts | 9 +++- lib/util/http/github.ts | 3 +- lib/util/http/gitlab.ts | 9 +++- lib/util/http/index.spec.ts | 74 +++++++++++++++++++++++++++ lib/util/http/index.ts | 83 +++++++++++++++++++++++-------- lib/util/http/jira.ts | 9 +++- lib/util/http/types.ts | 9 ++++ 9 files changed, 179 insertions(+), 32 deletions(-) diff --git a/lib/util/http/bitbucket-server.ts b/lib/util/http/bitbucket-server.ts index 449754f4cecea7..f9a7fa3ac74d0d 100644 --- a/lib/util/http/bitbucket-server.ts +++ b/lib/util/http/bitbucket-server.ts @@ -1,5 +1,10 @@ import { resolveBaseUrl } from '../url'; -import type { HttpOptions, HttpResponse, InternalHttpOptions } from './types'; +import type { + HttpOptions, + HttpRequestOptions, + HttpResponse, + InternalHttpOptions, +} from './types'; import { Http } from '.'; let baseUrl: string; @@ -14,7 +19,7 @@ export class BitbucketServerHttp extends Http { protected override request( path: string, - options?: InternalHttpOptions + options?: InternalHttpOptions & HttpRequestOptions ): Promise> { const url = resolveBaseUrl(baseUrl, path); const opts = { diff --git a/lib/util/http/bitbucket.ts b/lib/util/http/bitbucket.ts index 0d4a51c28a9b50..34c5166ca8cb7a 100644 --- a/lib/util/http/bitbucket.ts +++ b/lib/util/http/bitbucket.ts @@ -2,7 +2,7 @@ import is from '@sindresorhus/is'; import { logger } from '../../logger'; import type { PagedResult } from '../../modules/platform/bitbucket/types'; import { parseUrl, resolveBaseUrl } from '../url'; -import type { HttpOptions, HttpResponse } from './types'; +import type { HttpOptions, HttpRequestOptions, HttpResponse } from './types'; import { Http } from '.'; const MAX_PAGES = 100; @@ -26,7 +26,7 @@ export class BitbucketHttp extends Http { protected override async request( path: string, - options?: BitbucketHttpOptions + options?: BitbucketHttpOptions & HttpRequestOptions ): Promise> { const opts = { baseUrl, ...options }; @@ -53,7 +53,7 @@ export class BitbucketHttp extends Http { while (is.nonEmptyString(nextURL) && page <= MAX_PAGES) { const nextResult = await super.request>( nextURL, - options + options as BitbucketHttpOptions ); resultBody.values.push(...nextResult.body.values); diff --git a/lib/util/http/gitea.ts b/lib/util/http/gitea.ts index 394f3589e4b0c4..32e2ea27d7aed1 100644 --- a/lib/util/http/gitea.ts +++ b/lib/util/http/gitea.ts @@ -1,6 +1,11 @@ import is from '@sindresorhus/is'; import { resolveBaseUrl } from '../url'; -import type { HttpOptions, HttpResponse, InternalHttpOptions } from './types'; +import type { + HttpOptions, + HttpRequestOptions, + HttpResponse, + InternalHttpOptions, +} from './types'; import { Http } from '.'; let baseUrl: string; @@ -36,7 +41,7 @@ export class GiteaHttp extends Http { protected override async request( path: string, - options?: InternalHttpOptions & GiteaHttpOptions + options?: InternalHttpOptions & GiteaHttpOptions & HttpRequestOptions ): Promise> { const resolvedUrl = resolveUrl(path, options?.baseUrl ?? baseUrl); const opts = { diff --git a/lib/util/http/github.ts b/lib/util/http/github.ts index 9dfb182d673673..440a042d3cc122 100644 --- a/lib/util/http/github.ts +++ b/lib/util/http/github.ts @@ -19,6 +19,7 @@ import type { GotLegacyError } from './legacy'; import type { GraphqlOptions, HttpOptions, + HttpRequestOptions, HttpResponse, InternalHttpOptions, } from './types'; @@ -271,7 +272,7 @@ export class GithubHttp extends Http { protected override async request( url: string | URL, - options?: InternalHttpOptions & GithubHttpOptions, + options?: InternalHttpOptions & GithubHttpOptions & HttpRequestOptions, okToRetry = true ): Promise> { const opts: GithubHttpOptions = { diff --git a/lib/util/http/gitlab.ts b/lib/util/http/gitlab.ts index 6a72c31cd8f7b6..9078bbeee0c08e 100644 --- a/lib/util/http/gitlab.ts +++ b/lib/util/http/gitlab.ts @@ -2,7 +2,12 @@ import is from '@sindresorhus/is'; import { logger } from '../../logger'; import { ExternalHostError } from '../../types/errors/external-host-error'; import { parseLinkHeader, parseUrl } from '../url'; -import type { HttpOptions, HttpResponse, InternalHttpOptions } from './types'; +import type { + HttpOptions, + HttpRequestOptions, + HttpResponse, + InternalHttpOptions, +} from './types'; import { Http } from '.'; let baseUrl = 'https://gitlab.com/api/v4/'; @@ -21,7 +26,7 @@ export class GitlabHttp extends Http { protected override async request( url: string | URL, - options?: InternalHttpOptions & GitlabHttpOptions + options?: InternalHttpOptions & GitlabHttpOptions & HttpRequestOptions ): Promise> { const opts = { baseUrl, diff --git a/lib/util/http/index.spec.ts b/lib/util/http/index.spec.ts index 9563f2995654d0..7e8668154cf204 100644 --- a/lib/util/http/index.spec.ts +++ b/lib/util/http/index.spec.ts @@ -425,4 +425,78 @@ describe('util/http/index', () => { expect(t2 - t1).toBeGreaterThanOrEqual(4000); }); }); + + describe('Etag caching', () => { + it('returns cached data for status=304', async () => { + type FooBar = { foo: string; bar: string }; + const data: FooBar = { foo: 'foo', bar: 'bar' }; + httpMock + .scope(baseUrl, { reqheaders: { 'If-None-Match': 'foobar' } }) + .get('/foo') + .reply(304); + + const res = await http.getJson(`/foo`, { + baseUrl, + etagCache: { + etag: 'foobar', + data, + }, + }); + + expect(res.statusCode).toBe(304); + expect(res.body).toEqual(data); + expect(res.body).not.toBe(data); + }); + + it('bypasses schema parsing', async () => { + const FooBar = z + .object({ foo: z.string(), bar: z.string() }) + .transform(({ foo, bar }) => ({ + foobar: `${foo}${bar}`.toUpperCase(), + })); + const data = FooBar.parse({ foo: 'foo', bar: 'bar' }); + httpMock + .scope(baseUrl, { reqheaders: { 'If-None-Match': 'foobar' } }) + .get('/foo') + .reply(304); + + const res = await http.getJson( + `/foo`, + { + baseUrl, + etagCache: { + etag: 'foobar', + data, + }, + }, + FooBar + ); + + expect(res.statusCode).toBe(304); + expect(res.body).toEqual(data); + expect(res.body).not.toBe(data); + }); + + it('returns new data for status=200', async () => { + type FooBar = { foo: string; bar: string }; + const oldData: FooBar = { foo: 'foo', bar: 'bar' }; + const newData: FooBar = { foo: 'FOO', bar: 'BAR' }; + httpMock + .scope(baseUrl, { reqheaders: { 'If-None-Match': 'foobar' } }) + .get('/foo') + .reply(200, newData); + + const res = await http.getJson(`/foo`, { + baseUrl, + etagCache: { + etag: 'foobar', + data: oldData, + }, + }); + + expect(res.statusCode).toBe(200); + expect(res.body).toEqual(newData); + expect(res.body).not.toBe(newData); + }); + }); }); diff --git a/lib/util/http/index.ts b/lib/util/http/index.ts index 20bca08a33bf96..f435c274609190 100644 --- a/lib/util/http/index.ts +++ b/lib/util/http/index.ts @@ -18,6 +18,7 @@ import type { GotJSONOptions, GotOptions, HttpOptions, + HttpRequestOptions, HttpResponse, InternalHttpOptions, RequestStats, @@ -28,7 +29,7 @@ import './legacy'; export { RequestError as HttpError }; type JsonArgs< - Opts extends HttpOptions, + Opts extends HttpOptions & HttpRequestOptions, ResT = unknown, Schema extends ZodType = ZodType > = { @@ -39,18 +40,24 @@ type JsonArgs< type Task = () => Promise>; -function cloneResponse( - response: HttpResponse +// Copying will help to avoid circular structure +// and mutation of the cached response. +function copyResponse( + response: HttpResponse, + deep: boolean ): HttpResponse { const { body, statusCode, headers } = response; - // clone body and headers so that the cached result doesn't get accidentally mutated - // Don't use json clone for buffers - return { - statusCode, - body: body instanceof Buffer ? (body.slice() as T) : clone(body), - headers: clone(headers), - authorization: !!response.authorization, - }; + return deep + ? { + statusCode, + body: body instanceof Buffer ? (body.slice() as T) : clone(body), + headers: clone(headers), + } + : { + statusCode, + body, + headers, + }; } function applyDefaultHeaders(options: Options): void { @@ -120,7 +127,7 @@ export class Http { protected async request( requestUrl: string | URL, - httpOptions: InternalHttpOptions = {} + httpOptions: InternalHttpOptions & HttpRequestOptions = {} ): Promise> { let url = requestUrl.toString(); if (httpOptions?.baseUrl) { @@ -136,6 +143,18 @@ export class Http { httpOptions ); + const etagCache = + httpOptions.etagCache && + (options.method === 'get' || options.method === 'head') + ? httpOptions.etagCache + : null; + if (etagCache) { + options.headers = { + ...options.headers, + 'If-None-Match': etagCache.etag, + }; + } + if (process.env.NODE_ENV === 'test') { options.retry = 0; } @@ -204,8 +223,10 @@ export class Http { try { const res = await resPromise; - res.authorization = !!options?.headers?.authorization; - return cloneResponse(res); + const deepCopyNeeded = !!memCacheKey && res.statusCode !== 304; + const resCopy = copyResponse(res, deepCopyNeeded); + resCopy.authorization = !!options?.headers?.authorization; + return resCopy; } catch (err) { const { abortOnError, abortIgnoreStatusCodes } = options; if (abortOnError && !abortIgnoreStatusCodes?.includes(err.statusCode)) { @@ -215,7 +236,10 @@ export class Http { } } - get(url: string, options: HttpOptions = {}): Promise { + get( + url: string, + options: HttpOptions & HttpRequestOptions = {} + ): Promise { return this.request(url, options); } @@ -235,7 +259,11 @@ export class Http { private async requestJson( method: InternalHttpOptions['method'], - { url, httpOptions: requestOptions, schema }: JsonArgs + { + url, + httpOptions: requestOptions, + schema, + }: JsonArgs, ResT> ): Promise> { const { body, ...httpOptions } = { ...requestOptions }; const opts: InternalHttpOptions = { @@ -253,11 +281,23 @@ export class Http { } const res = await this.request(url, opts); + const etagCacheHit = + httpOptions.etagCache && res.statusCode === 304 + ? clone(httpOptions.etagCache.data) + : null; + if (!schema) { + if (etagCacheHit) { + res.body = etagCacheHit; + } return res; } - res.body = await schema.parseAsync(res.body); + if (etagCacheHit) { + res.body = etagCacheHit; + } else { + res.body = await schema.parseAsync(res.body); + } return res; } @@ -281,19 +321,22 @@ export class Http { return res; } - getJson(url: string, options?: Opts): Promise>; + getJson( + url: string, + options?: Opts & HttpRequestOptions + ): Promise>; getJson = ZodType>( url: string, schema: Schema ): Promise>>; getJson = ZodType>( url: string, - options: Opts, + options: Opts & HttpRequestOptions>, schema: Schema ): Promise>>; getJson = ZodType>( arg1: string, - arg2?: Opts | Schema, + arg2?: (Opts & HttpRequestOptions) | Schema, arg3?: Schema ): Promise> { const args = this.resolveArgs(arg1, arg2, arg3); diff --git a/lib/util/http/jira.ts b/lib/util/http/jira.ts index df51c152a47c08..1137cc33cce8eb 100644 --- a/lib/util/http/jira.ts +++ b/lib/util/http/jira.ts @@ -1,4 +1,9 @@ -import type { HttpOptions, HttpResponse, InternalHttpOptions } from './types'; +import type { + HttpOptions, + HttpRequestOptions, + HttpResponse, + InternalHttpOptions, +} from './types'; import { Http } from '.'; let baseUrl: string; @@ -14,7 +19,7 @@ export class JiraHttp extends Http { protected override request( url: string | URL, - options?: InternalHttpOptions + options?: InternalHttpOptions & HttpRequestOptions ): Promise> { const opts = { baseUrl, ...options }; return super.request(url, opts); diff --git a/lib/util/http/types.ts b/lib/util/http/types.ts index 4b232982585ea2..861959a9ac122e 100644 --- a/lib/util/http/types.ts +++ b/lib/util/http/types.ts @@ -65,6 +65,15 @@ export interface HttpOptions { memCache?: boolean; } +export interface EtagCache { + etag: string; + data: T; +} + +export interface HttpRequestOptions { + etagCache?: EtagCache; +} + export interface InternalHttpOptions extends HttpOptions { json?: HttpOptions['body']; responseType?: 'json' | 'buffer';