Skip to content

Commit

Permalink
feat(http): Switch to pluggable HTTP cache implementation (#27966)
Browse files Browse the repository at this point in the history
Co-authored-by: Rhys Arkins <rhys@arkins.net>
  • Loading branch information
zharinov and rarkins committed Mar 19, 2024
1 parent 5e02f6e commit 8596967
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 166 deletions.
26 changes: 15 additions & 11 deletions lib/modules/platform/github/index.ts
Expand Up @@ -35,12 +35,10 @@ import type {
LongCommitSha,
} from '../../../util/git/types';
import * as hostRules from '../../../util/host-rules';
import { repoCacheProvider } from '../../../util/http/cache/repository-http-cache-provider';
import * as githubHttp from '../../../util/http/github';
import type { GithubHttpOptions } from '../../../util/http/github';
import type {
HttpResponse,
InternalHttpOptions,
} from '../../../util/http/types';
import type { HttpResponse } from '../../../util/http/types';
import { coerceObject } from '../../../util/object';
import { regEx } from '../../../util/regex';
import { sanitize } from '../../../util/sanitize';
Expand Down Expand Up @@ -312,7 +310,7 @@ async function getBranchProtection(
}
const res = await githubApi.getJson<BranchProtection>(
`repos/${config.repository}/branches/${escapeHash(branchName)}/protection`,
{ repoCache: true },
{ cacheProvider: repoCacheProvider },
);
return res.body;
}
Expand All @@ -323,11 +321,14 @@ export async function getRawFile(
branchOrTag?: string,
): Promise<string | null> {
const repo = repoName ?? config.repository;

// only use cache for the same org
const httpOptions: GithubHttpOptions = {};
const isSameOrg = repo?.split('/')?.[0] === config.repositoryOwner;
const httpOptions: InternalHttpOptions = {
// Only cache response if it's from the same org
repoCache: isSameOrg,
};
if (isSameOrg) {
httpOptions.cacheProvider = repoCacheProvider;
}

let url = `repos/${repo}/contents/${fileName}`;
if (branchOrTag) {
url += `?ref=` + branchOrTag;
Expand Down Expand Up @@ -1239,7 +1240,10 @@ export async function getIssue(
const repo = config.parentRepo ?? config.repository;
const { body: issue } = await githubApi.getJson(
`repos/${repo}/issues/${number}`,
{ memCache: useCache, repoCache: true },
{
memCache: useCache,
cacheProvider: repoCacheProvider,
},
Issue,
);
return issue;
Expand Down Expand Up @@ -1320,7 +1324,7 @@ export async function ensureIssue({
body: { body: issueBody },
} = await githubApi.getJson(
`repos/${repo}/issues/${issue.number}`,
{ repoCache: true },
{ cacheProvider: repoCacheProvider },
Issue,
);
if (
Expand Down
3 changes: 2 additions & 1 deletion lib/modules/platform/github/pr.ts
Expand Up @@ -2,6 +2,7 @@ import is from '@sindresorhus/is';
import { logger } from '../../../logger';
import { ExternalHostError } from '../../../types/errors/external-host-error';
import { getCache } from '../../../util/cache/repository';
import { repoCacheProvider } from '../../../util/http/cache/repository-http-cache-provider';
import type { GithubHttp, GithubHttpOptions } from '../../../util/http/github';
import { parseLinkHeader } from '../../../util/url';
import { ApiCache } from './api-cache';
Expand Down Expand Up @@ -64,7 +65,7 @@ export async function getPrCache(
while (needNextPageFetch && needNextPageSync) {
const opts: GithubHttpOptions = { paginate: false };
if (pageIdx === 1) {
opts.repoCache = true;
opts.cacheProvider = repoCacheProvider;
if (isInitial) {
// Speed up initial fetch
opts.paginate = true;
Expand Down
2 changes: 1 addition & 1 deletion lib/util/http/github.ts
Expand Up @@ -339,7 +339,7 @@ export class GithubHttp extends Http<GithubHttpOptions> {
nextUrl.searchParams.set('page', String(pageNumber));
return this.request<T>(
nextUrl,
{ ...opts, paginate: false, repoCache: false },
{ ...opts, paginate: false, cacheProvider: undefined },
okToRetry,
);
},
Expand Down
76 changes: 5 additions & 71 deletions lib/util/http/index.spec.ts
Expand Up @@ -6,6 +6,7 @@ import {
HOST_DISABLED,
} from '../../constants/error-messages';
import * as memCache from '../cache/memory';
import { resetCache } from '../cache/repository';
import * as hostRules from '../host-rules';
import * as queue from './queue';
import * as throttle from './throttle';
Expand All @@ -22,6 +23,7 @@ describe('util/http/index', () => {
hostRules.clear();
queue.clear();
throttle.clear();
resetCache();
});

it('get', async () => {
Expand Down Expand Up @@ -77,84 +79,16 @@ describe('util/http/index', () => {
})
.get('/')
.reply(200, '{ "test": true }', { etag: 'abc123' });
expect(
await http.getJson('http://renovate.com', { repoCache: true }),
).toEqual({
authorization: false,
body: {
test: true,
},
headers: {
etag: 'abc123',
},
statusCode: 200,
});

httpMock
.scope(baseUrl, {
reqheaders: {
accept: 'application/json',
},
})
.get('/')
.reply(304, '', { etag: 'abc123' });
expect(
await http.getJson('http://renovate.com', { repoCache: true }),
).toEqual({
authorization: false,
body: {
test: true,
},
headers: {
etag: 'abc123',
},
statusCode: 200,
});
});

it('uses last-modified header for caching', async () => {
httpMock
.scope(baseUrl, {
reqheaders: {
accept: 'application/json',
},
})
.get('/')
.reply(200, '{ "test": true }', {
'last-modified': 'Sun, 18 Feb 2024 18:00:05 GMT',
});
expect(
await http.getJson('http://renovate.com', { repoCache: true }),
).toEqual({
authorization: false,
body: {
test: true,
},
headers: {
'last-modified': 'Sun, 18 Feb 2024 18:00:05 GMT',
},
statusCode: 200,
});
const res = await http.getJson('http://renovate.com');

httpMock
.scope(baseUrl, {
reqheaders: {
accept: 'application/json',
},
})
.get('/')
.reply(304, '', {
'last-modified': 'Sun, 18 Feb 2024 18:00:05 GMT',
});
expect(
await http.getJson('http://renovate.com', { repoCache: true }),
).toEqual({
expect(res).toEqual({
authorization: false,
body: {
test: true,
},
headers: {
'last-modified': 'Sun, 18 Feb 2024 18:00:05 GMT',
etag: 'abc123',
},
statusCode: 200,
});
Expand Down
65 changes: 1 addition & 64 deletions lib/util/http/index.ts
Expand Up @@ -8,14 +8,9 @@ import { pkg } from '../../expose.cjs';
import { logger } from '../../logger';
import { ExternalHostError } from '../../types/errors/external-host-error';
import * as memCache from '../cache/memory';
import { getCache } from '../cache/repository';
import { hash } from '../hash';
import { type AsyncResult, Result } from '../result';
import {
HttpCacheStats,
type HttpRequestStatsDataPoint,
HttpStats,
} from '../stats';
import { type HttpRequestStatsDataPoint, HttpStats } from '../stats';
import { resolveBaseUrl } from '../url';
import { applyAuthorization, removeAuthorization } from './auth';
import { hooks } from './hooks';
Expand All @@ -27,7 +22,6 @@ import type {
GotJSONOptions,
GotOptions,
GotTask,
HttpCache,
HttpOptions,
HttpResponse,
InternalHttpOptions,
Expand Down Expand Up @@ -198,30 +192,6 @@ export class Http<Opts extends HttpOptions = HttpOptions> {

// istanbul ignore else: no cache tests
if (!resPromise) {
if (httpOptions.repoCache) {
const responseCache = getCache().httpCache?.[url] as
| HttpCache
| undefined;
// Prefer If-Modified-Since over If-None-Match
if (responseCache?.['lastModified']) {
logger.debug(
`http cache: trying cached Last-Modified "${responseCache?.['lastModified']}" for ${url}`,
);
options.headers = {
...options.headers,
'If-Modified-Since': responseCache['lastModified'],
};
} else if (responseCache?.etag) {
logger.debug(
`http cache: trying cached etag "${responseCache.etag}" for ${url}`,
);
options.headers = {
...options.headers,
'If-None-Match': responseCache.etag,
};
}
}

if (options.cacheProvider) {
await options.cacheProvider.setCacheHeaders(url, options);
}
Expand Down Expand Up @@ -256,39 +226,6 @@ export class Http<Opts extends HttpOptions = HttpOptions> {
const resCopy = copyResponse(res, deepCopyNeeded);
resCopy.authorization = !!options?.headers?.authorization;

if (httpOptions.repoCache) {
const cache = getCache();
cache.httpCache ??= {};
if (
resCopy.statusCode === 200 &&
(resCopy.headers?.etag ?? resCopy.headers['last-modified'])
) {
logger.debug(
`http cache: saving ${url} (etag=${resCopy.headers.etag}, lastModified=${resCopy.headers['last-modified']})`,
);
HttpCacheStats.incRemoteMisses(url);
cache.httpCache[url] = {
etag: resCopy.headers.etag,
httpResponse: copyResponse(res, deepCopyNeeded),
lastModified: resCopy.headers['last-modified'],
timeStamp: new Date().toISOString(),
};
}
const httpCache = cache.httpCache[url] as HttpCache | undefined;
if (resCopy.statusCode === 304 && httpCache) {
logger.debug(
`http cache: Using cached response: ${url} from ${httpCache.timeStamp}`,
);
HttpCacheStats.incRemoteHits(url);
const cacheCopy = copyResponse(
httpCache.httpResponse,
deepCopyNeeded,
);
cacheCopy.authorization = !!options?.headers?.authorization;
return cacheCopy as HttpResponse<T>;
}
}

if (options.cacheProvider) {
return await options.cacheProvider.wrapResponse(url, resCopy);
}
Expand Down
18 changes: 0 additions & 18 deletions lib/util/http/types.ts
Expand Up @@ -66,10 +66,6 @@ export interface HttpOptions {

token?: string;
memCache?: boolean;
/**
* @deprecated
*/
repoCache?: boolean;
cacheProvider?: HttpCacheProvider;
}

Expand All @@ -78,10 +74,6 @@ export interface InternalHttpOptions extends HttpOptions {
responseType?: 'json' | 'buffer';
method?: 'get' | 'post' | 'put' | 'patch' | 'delete' | 'head';
parseJson?: ParseJsonFunction;
/**
* @deprecated
*/
repoCache?: boolean;
}

export interface HttpHeaders extends IncomingHttpHeaders {
Expand All @@ -97,13 +89,3 @@ export interface HttpResponse<T = string> {

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

/**
* @deprecated
*/
export interface HttpCache {
etag?: string;
httpResponse: HttpResponse<unknown>;
lastModified?: string;
timeStamp: string;
}

0 comments on commit 8596967

Please sign in to comment.