Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feat(http): Etag support (#22302)
  • Loading branch information
zharinov committed May 25, 2023
1 parent 0417c11 commit 290b78d
Show file tree
Hide file tree
Showing 9 changed files with 179 additions and 32 deletions.
9 changes: 7 additions & 2 deletions 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;
Expand All @@ -14,7 +19,7 @@ export class BitbucketServerHttp extends Http {

protected override request<T>(
path: string,
options?: InternalHttpOptions
options?: InternalHttpOptions & HttpRequestOptions<T>
): Promise<HttpResponse<T>> {
const url = resolveBaseUrl(baseUrl, path);
const opts = {
Expand Down
6 changes: 3 additions & 3 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, HttpResponse } from './types';
import type { HttpOptions, HttpRequestOptions, 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
options?: BitbucketHttpOptions & HttpRequestOptions<T>
): Promise<HttpResponse<T>> {
const opts = { baseUrl, ...options };

Expand All @@ -53,7 +53,7 @@ export class BitbucketHttp extends Http<BitbucketHttpOptions> {
while (is.nonEmptyString(nextURL) && page <= MAX_PAGES) {
const nextResult = await super.request<PagedResult<T>>(
nextURL,
options
options as BitbucketHttpOptions
);

resultBody.values.push(...nextResult.body.values);
Expand Down
9 changes: 7 additions & 2 deletions 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;
Expand Down Expand Up @@ -36,7 +41,7 @@ export class GiteaHttp extends Http<GiteaHttpOptions> {

protected override async request<T>(
path: string,
options?: InternalHttpOptions & GiteaHttpOptions
options?: InternalHttpOptions & GiteaHttpOptions & HttpRequestOptions<T>
): Promise<HttpResponse<T>> {
const resolvedUrl = resolveUrl(path, options?.baseUrl ?? baseUrl);
const opts = {
Expand Down
3 changes: 2 additions & 1 deletion lib/util/http/github.ts
Expand Up @@ -19,6 +19,7 @@ import type { GotLegacyError } from './legacy';
import type {
GraphqlOptions,
HttpOptions,
HttpRequestOptions,
HttpResponse,
InternalHttpOptions,
} from './types';
Expand Down Expand Up @@ -271,7 +272,7 @@ export class GithubHttp extends Http<GithubHttpOptions> {

protected override async request<T>(
url: string | URL,
options?: InternalHttpOptions & GithubHttpOptions,
options?: InternalHttpOptions & GithubHttpOptions & HttpRequestOptions<T>,
okToRetry = true
): Promise<HttpResponse<T>> {
const opts: GithubHttpOptions = {
Expand Down
9 changes: 7 additions & 2 deletions lib/util/http/gitlab.ts
Expand Up @@ -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/';
Expand All @@ -21,7 +26,7 @@ export class GitlabHttp extends Http<GitlabHttpOptions> {

protected override async request<T>(
url: string | URL,
options?: InternalHttpOptions & GitlabHttpOptions
options?: InternalHttpOptions & GitlabHttpOptions & HttpRequestOptions<T>
): Promise<HttpResponse<T>> {
const opts = {
baseUrl,
Expand Down
74 changes: 74 additions & 0 deletions lib/util/http/index.spec.ts
Expand Up @@ -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<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);
});
});
});
83 changes: 63 additions & 20 deletions lib/util/http/index.ts
Expand Up @@ -18,6 +18,7 @@ import type {
GotJSONOptions,
GotOptions,
HttpOptions,
HttpRequestOptions,
HttpResponse,
InternalHttpOptions,
RequestStats,
Expand All @@ -28,7 +29,7 @@ import './legacy';
export { RequestError as HttpError };

type JsonArgs<
Opts extends HttpOptions,
Opts extends HttpOptions & HttpRequestOptions<ResT>,
ResT = unknown,
Schema extends ZodType<ResT> = ZodType<ResT>
> = {
Expand All @@ -39,18 +40,24 @@ type JsonArgs<

type Task<T> = () => Promise<HttpResponse<T>>;

function cloneResponse<T extends Buffer | string | any>(
response: HttpResponse<T>
// Copying will help to avoid circular structure
// and mutation of the cached response.
function copyResponse<T extends Buffer | string | any>(
response: HttpResponse<T>,
deep: boolean
): HttpResponse<T> {
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<T>(body),
headers: clone(headers),
authorization: !!response.authorization,
};
return deep
? {
statusCode,
body: body instanceof Buffer ? (body.slice() as T) : clone<T>(body),
headers: clone(headers),
}
: {
statusCode,
body,
headers,
};
}

function applyDefaultHeaders(options: Options): void {
Expand Down Expand Up @@ -120,7 +127,7 @@ export class Http<Opts extends HttpOptions = HttpOptions> {

protected async request<T>(
requestUrl: string | URL,
httpOptions: InternalHttpOptions = {}
httpOptions: InternalHttpOptions & HttpRequestOptions<T> = {}
): Promise<HttpResponse<T>> {
let url = requestUrl.toString();
if (httpOptions?.baseUrl) {
Expand All @@ -136,6 +143,18 @@ export class Http<Opts extends HttpOptions = HttpOptions> {
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;
}
Expand Down Expand Up @@ -204,8 +223,10 @@ export class Http<Opts extends HttpOptions = HttpOptions> {

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)) {
Expand All @@ -215,7 +236,10 @@ export class Http<Opts extends HttpOptions = HttpOptions> {
}
}

get(url: string, options: HttpOptions = {}): Promise<HttpResponse> {
get(
url: string,
options: HttpOptions & HttpRequestOptions<string> = {}
): Promise<HttpResponse> {
return this.request<string>(url, options);
}

Expand All @@ -235,7 +259,11 @@ export class Http<Opts extends HttpOptions = HttpOptions> {

private async requestJson<ResT = unknown>(
method: InternalHttpOptions['method'],
{ url, httpOptions: requestOptions, schema }: JsonArgs<Opts, ResT>
{
url,
httpOptions: requestOptions,
schema,
}: JsonArgs<Opts & HttpRequestOptions<ResT>, ResT>
): Promise<HttpResponse<ResT>> {
const { body, ...httpOptions } = { ...requestOptions };
const opts: InternalHttpOptions = {
Expand All @@ -253,11 +281,23 @@ 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;
}

res.body = await schema.parseAsync(res.body);
if (etagCacheHit) {
res.body = etagCacheHit;
} else {
res.body = await schema.parseAsync(res.body);
}
return res;
}

Expand All @@ -281,19 +321,22 @@ export class Http<Opts extends HttpOptions = HttpOptions> {
return res;
}

getJson<ResT>(url: string, options?: Opts): Promise<HttpResponse<ResT>>;
getJson<ResT>(
url: string,
options?: Opts & HttpRequestOptions<ResT>
): 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,
options: Opts & HttpRequestOptions<Infer<Schema>>,
schema: Schema
): Promise<HttpResponse<Infer<Schema>>>;
getJson<ResT = unknown, Schema extends ZodType<ResT> = ZodType<ResT>>(
arg1: string,
arg2?: Opts | Schema,
arg2?: (Opts & HttpRequestOptions<ResT>) | Schema,
arg3?: Schema
): Promise<HttpResponse<ResT>> {
const args = this.resolveArgs<ResT>(arg1, arg2, arg3);
Expand Down
9 changes: 7 additions & 2 deletions 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;
Expand All @@ -14,7 +19,7 @@ export class JiraHttp extends Http {

protected override request<T>(
url: string | URL,
options?: InternalHttpOptions
options?: InternalHttpOptions & HttpRequestOptions<T>
): Promise<HttpResponse<T>> {
const opts = { baseUrl, ...options };
return super.request<T>(url, opts);
Expand Down
9 changes: 9 additions & 0 deletions lib/util/http/types.ts
Expand Up @@ -65,6 +65,15 @@ export interface HttpOptions {
memCache?: 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 290b78d

Please sign in to comment.