Skip to content

Commit

Permalink
refactor(http): Remove unused HTTP etag caching implementation (#28000)
Browse files Browse the repository at this point in the history
  • Loading branch information
zharinov committed Mar 18, 2024
1 parent bcfaca4 commit c258041
Show file tree
Hide file tree
Showing 10 changed files with 23 additions and 165 deletions.
8 changes: 2 additions & 6 deletions lib/modules/datasource/maven/util.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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 {
Expand Down
9 changes: 2 additions & 7 deletions 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;
Expand All @@ -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 = {
Expand Down
4 changes: 2 additions & 2 deletions lib/util/http/bitbucket.ts
Expand Up @@ -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;
Expand All @@ -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 };

Expand Down
9 changes: 2 additions & 7 deletions 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;
Expand Down Expand Up @@ -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 = {
Expand Down
3 changes: 1 addition & 2 deletions lib/util/http/github.ts
Expand Up @@ -19,7 +19,6 @@ import type { GotLegacyError } from './legacy';
import type {
GraphqlOptions,
HttpOptions,
HttpRequestOptions,
HttpResponse,
InternalHttpOptions,
} from './types';
Expand Down Expand Up @@ -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 = {
Expand Down
9 changes: 2 additions & 7 deletions lib/util/http/gitlab.ts
Expand Up @@ -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/';
Expand All @@ -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,
Expand Down
74 changes: 0 additions & 74 deletions lib/util/http/index.spec.ts
Expand Up @@ -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);
});
});
});
54 changes: 10 additions & 44 deletions lib/util/http/index.ts
Expand Up @@ -28,7 +28,6 @@ import type {
GotOptions,
GotTask,
HttpOptions,
HttpRequestOptions,
HttpResponse,
InternalHttpOptions,
} from './types';
Expand All @@ -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>,
> = {
Expand Down Expand Up @@ -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) {
Expand All @@ -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],
};
Expand Down Expand Up @@ -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);
}

Expand All @@ -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 = {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
Expand All @@ -440,15 +406,15 @@ 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<
ResT extends NonNullable<unknown>,
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);
Expand Down
9 changes: 2 additions & 7 deletions 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;
Expand All @@ -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);
Expand Down
9 changes: 0 additions & 9 deletions lib/util/http/types.ts
Expand Up @@ -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';
Expand Down

0 comments on commit c258041

Please sign in to comment.