From c25804194da6b55e92113c60cef921b0071b41a8 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov <zharinov@users.noreply.github.com> Date: Mon, 18 Mar 2024 10:58:31 -0300 Subject: [PATCH] refactor(http): Remove unused HTTP etag caching implementation (#28000) --- lib/modules/datasource/maven/util.ts | 8 +-- lib/util/http/bitbucket-server.ts | 9 +--- lib/util/http/bitbucket.ts | 4 +- 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 | 54 ++++---------------- lib/util/http/jira.ts | 9 +--- lib/util/http/types.ts | 9 ---- 10 files changed, 23 insertions(+), 165 deletions(-) diff --git a/lib/modules/datasource/maven/util.ts b/lib/modules/datasource/maven/util.ts index f45d48e4bd..24b92cc02d 100644 --- a/lib/modules/datasource/maven/util.ts +++ b/lib/modules/datasource/maven/util.ts @@ -6,11 +6,7 @@ import { HOST_DISABLED } from '../../../constants/error-messages'; import { logger } from '../../../logger'; import { ExternalHostError } from '../../../types/errors/external-host-error'; import type { Http } from '../../../util/http'; -import type { - HttpOptions, - HttpRequestOptions, - HttpResponse, -} from '../../../util/http/types'; +import type { HttpOptions, HttpResponse } from '../../../util/http/types'; import { regEx } from '../../../util/regex'; import { getS3Client, parseS3Url } from '../../../util/s3'; import { streamToString } from '../../../util/streams'; @@ -69,7 +65,7 @@ function isUnsupportedHostError(err: { name: string }): boolean { export async function downloadHttpProtocol( http: Http, pkgUrl: URL | string, - opts: HttpOptions & HttpRequestOptions<string> = {}, + opts: HttpOptions = {}, ): Promise<Partial<HttpResponse>> { let raw: HttpResponse; try { diff --git a/lib/util/http/bitbucket-server.ts b/lib/util/http/bitbucket-server.ts index 463e37d3c0..2189e8188a 100644 --- a/lib/util/http/bitbucket-server.ts +++ b/lib/util/http/bitbucket-server.ts @@ -1,10 +1,5 @@ import { resolveBaseUrl } from '../url'; -import type { - HttpOptions, - HttpRequestOptions, - HttpResponse, - InternalHttpOptions, -} from './types'; +import type { HttpOptions, HttpResponse, InternalHttpOptions } from './types'; import { Http } from '.'; let baseUrl: string; @@ -19,7 +14,7 @@ export class BitbucketServerHttp extends Http { protected override request<T>( path: string, - options?: InternalHttpOptions & HttpRequestOptions<T>, + options?: InternalHttpOptions, ): Promise<HttpResponse<T>> { const url = resolveBaseUrl(baseUrl, path); const opts = { diff --git a/lib/util/http/bitbucket.ts b/lib/util/http/bitbucket.ts index de4c406dff..138c31aff6 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, HttpRequestOptions, HttpResponse } from './types'; +import type { HttpOptions, HttpResponse } from './types'; import { Http } from '.'; const MAX_PAGES = 100; @@ -26,7 +26,7 @@ export class BitbucketHttp extends Http<BitbucketHttpOptions> { protected override async request<T>( path: string, - options?: BitbucketHttpOptions & HttpRequestOptions<T>, + options?: BitbucketHttpOptions, ): Promise<HttpResponse<T>> { const opts = { baseUrl, ...options }; diff --git a/lib/util/http/gitea.ts b/lib/util/http/gitea.ts index c8296939e4..f69470c44e 100644 --- a/lib/util/http/gitea.ts +++ b/lib/util/http/gitea.ts @@ -1,11 +1,6 @@ import is from '@sindresorhus/is'; import { resolveBaseUrl } from '../url'; -import type { - HttpOptions, - HttpRequestOptions, - HttpResponse, - InternalHttpOptions, -} from './types'; +import type { HttpOptions, HttpResponse, InternalHttpOptions } from './types'; import { Http } from '.'; let baseUrl: string; @@ -41,7 +36,7 @@ export class GiteaHttp extends Http<GiteaHttpOptions> { protected override async request<T>( path: string, - options?: InternalHttpOptions & GiteaHttpOptions & HttpRequestOptions<T>, + options?: InternalHttpOptions & GiteaHttpOptions, ): Promise<HttpResponse<T>> { const resolvedUrl = resolveUrl(path, options?.baseUrl ?? baseUrl); const opts = { diff --git a/lib/util/http/github.ts b/lib/util/http/github.ts index 65f9e6e81b..97f20590ce 100644 --- a/lib/util/http/github.ts +++ b/lib/util/http/github.ts @@ -19,7 +19,6 @@ import type { GotLegacyError } from './legacy'; import type { GraphqlOptions, HttpOptions, - HttpRequestOptions, HttpResponse, InternalHttpOptions, } from './types'; @@ -274,7 +273,7 @@ export class GithubHttp extends Http<GithubHttpOptions> { protected override async request<T>( url: string | URL, - options?: InternalHttpOptions & GithubHttpOptions & HttpRequestOptions<T>, + options?: InternalHttpOptions & GithubHttpOptions, okToRetry = true, ): Promise<HttpResponse<T>> { const opts: GithubHttpOptions = { diff --git a/lib/util/http/gitlab.ts b/lib/util/http/gitlab.ts index 770e9d1cf4..e81dde8377 100644 --- a/lib/util/http/gitlab.ts +++ b/lib/util/http/gitlab.ts @@ -2,12 +2,7 @@ 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, - HttpRequestOptions, - HttpResponse, - InternalHttpOptions, -} from './types'; +import type { HttpOptions, HttpResponse, InternalHttpOptions } from './types'; import { Http } from '.'; let baseUrl = 'https://gitlab.com/api/v4/'; @@ -26,7 +21,7 @@ export class GitlabHttp extends Http<GitlabHttpOptions> { protected override async request<T>( url: string | URL, - options?: InternalHttpOptions & GitlabHttpOptions & HttpRequestOptions<T>, + options?: InternalHttpOptions & GitlabHttpOptions, ): Promise<HttpResponse<T>> { const opts = { baseUrl, diff --git a/lib/util/http/index.spec.ts b/lib/util/http/index.spec.ts index 4541f6b86a..d3e4bb3b00 100644 --- a/lib/util/http/index.spec.ts +++ b/lib/util/http/index.spec.ts @@ -546,78 +546,4 @@ 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<FooBar>(`/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<FooBar>(`/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 d6139238ef..fbf92cb044 100644 --- a/lib/util/http/index.ts +++ b/lib/util/http/index.ts @@ -28,7 +28,6 @@ import type { GotOptions, GotTask, HttpOptions, - HttpRequestOptions, HttpResponse, InternalHttpOptions, } from './types'; @@ -41,7 +40,7 @@ export class EmptyResultError extends Error {} export type SafeJsonError = RequestError | ZodError | EmptyResultError; type JsonArgs< - Opts extends HttpOptions & HttpRequestOptions<ResT>, + Opts extends HttpOptions, ResT = unknown, Schema extends ZodType<ResT> = ZodType<ResT>, > = { @@ -158,7 +157,7 @@ export class Http<Opts extends HttpOptions = HttpOptions> { protected async request<T>( requestUrl: string | URL, - httpOptions: InternalHttpOptions & HttpRequestOptions<T>, + httpOptions: InternalHttpOptions, ): Promise<HttpResponse<T>> { let url = requestUrl.toString(); if (httpOptions?.baseUrl) { @@ -176,17 +175,6 @@ export class Http<Opts extends HttpOptions = HttpOptions> { logger.trace(`HTTP request: ${options.method.toUpperCase()} ${url}`); - const etagCache = - httpOptions.etagCache && options.method === 'get' - ? httpOptions.etagCache - : null; - if (etagCache) { - options.headers = { - ...options.headers, - 'If-None-Match': etagCache.etag, - }; - } - options.hooks = { beforeRedirect: [removeAuthorization], }; @@ -314,10 +302,7 @@ export class Http<Opts extends HttpOptions = HttpOptions> { } } - get( - url: string, - options: HttpOptions & HttpRequestOptions<string> = {}, - ): Promise<HttpResponse> { + get(url: string, options: HttpOptions = {}): Promise<HttpResponse> { return this.request<string>(url, options); } @@ -337,11 +322,7 @@ export class Http<Opts extends HttpOptions = HttpOptions> { private async requestJson<ResT = unknown>( method: InternalHttpOptions['method'], - { - url, - httpOptions: requestOptions, - schema, - }: JsonArgs<Opts & HttpRequestOptions<ResT>, ResT>, + { url, httpOptions: requestOptions, schema }: JsonArgs<Opts, ResT>, ): Promise<HttpResponse<ResT>> { const { body, ...httpOptions } = { ...requestOptions }; const opts: InternalHttpOptions = { @@ -359,23 +340,11 @@ export class Http<Opts extends HttpOptions = HttpOptions> { } const res = await this.request<ResT>(url, opts); - const etagCacheHit = - httpOptions.etagCache && res.statusCode === 304 - ? clone(httpOptions.etagCache.data) - : null; - if (!schema) { - if (etagCacheHit) { - res.body = etagCacheHit; - } return res; } - if (etagCacheHit) { - res.body = etagCacheHit; - } else { - res.body = await schema.parseAsync(res.body); - } + res.body = await schema.parseAsync(res.body); return res; } @@ -409,22 +378,19 @@ export class Http<Opts extends HttpOptions = HttpOptions> { }); } - getJson<ResT>( - url: string, - options?: Opts & HttpRequestOptions<ResT>, - ): Promise<HttpResponse<ResT>>; + getJson<ResT>(url: string, options?: Opts): Promise<HttpResponse<ResT>>; getJson<ResT, Schema extends ZodType<ResT> = ZodType<ResT>>( url: string, schema: Schema, ): Promise<HttpResponse<Infer<Schema>>>; getJson<ResT, Schema extends ZodType<ResT> = ZodType<ResT>>( url: string, - options: Opts & HttpRequestOptions<Infer<Schema>>, + options: Opts, schema: Schema, ): Promise<HttpResponse<Infer<Schema>>>; getJson<ResT = unknown, Schema extends ZodType<ResT> = ZodType<ResT>>( arg1: string, - arg2?: (Opts & HttpRequestOptions<ResT>) | Schema, + arg2?: Opts | Schema, arg3?: Schema, ): Promise<HttpResponse<ResT>> { const args = this.resolveArgs<ResT>(arg1, arg2, arg3); @@ -440,7 +406,7 @@ export class Http<Opts extends HttpOptions = HttpOptions> { Schema extends ZodType<ResT> = ZodType<ResT>, >( url: string, - options: Opts & HttpRequestOptions<Infer<Schema>>, + options: Opts, schema: Schema, ): AsyncResult<Infer<Schema>, SafeJsonError>; getJsonSafe< @@ -448,7 +414,7 @@ export class Http<Opts extends HttpOptions = HttpOptions> { Schema extends ZodType<ResT> = ZodType<ResT>, >( arg1: string, - arg2?: (Opts & HttpRequestOptions<ResT>) | Schema, + arg2?: Opts | Schema, arg3?: Schema, ): AsyncResult<ResT, SafeJsonError> { const args = this.resolveArgs<ResT>(arg1, arg2, arg3); diff --git a/lib/util/http/jira.ts b/lib/util/http/jira.ts index 7463d7b6eb..706780b284 100644 --- a/lib/util/http/jira.ts +++ b/lib/util/http/jira.ts @@ -1,9 +1,4 @@ -import type { - HttpOptions, - HttpRequestOptions, - HttpResponse, - InternalHttpOptions, -} from './types'; +import type { HttpOptions, HttpResponse, InternalHttpOptions } from './types'; import { Http } from '.'; let baseUrl: string; @@ -19,7 +14,7 @@ export class JiraHttp extends Http { protected override request<T>( url: string | URL, - options?: InternalHttpOptions & HttpRequestOptions<T>, + options?: InternalHttpOptions, ): Promise<HttpResponse<T>> { const opts = { baseUrl, ...options }; return super.request<T>(url, opts); diff --git a/lib/util/http/types.ts b/lib/util/http/types.ts index 5969aeade4..c09fce3aa9 100644 --- a/lib/util/http/types.ts +++ b/lib/util/http/types.ts @@ -68,15 +68,6 @@ export interface HttpOptions { repoCache?: boolean; } -export interface EtagCache<T = any> { - etag: string; - data: T; -} - -export interface HttpRequestOptions<T = any> { - etagCache?: EtagCache<T>; -} - export interface InternalHttpOptions extends HttpOptions { json?: HttpOptions['body']; responseType?: 'json' | 'buffer'; -- GitLab