refactor(platform): getRawFile and getJsonFile throw instead of null (#9413)

Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
Co-authored-by: Rhys Arkins <rhys@arkins.net>
Co-authored-by: Jamie Magee <JamieMagee@users.noreply.github.com>
This commit is contained in:
Sergei Zharinov 2021-04-07 08:23:11 +04:00 committed by GitHub
parent 4c1fac57f4
commit 36f8d1df0e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 114 additions and 151 deletions

View file

@ -1196,7 +1196,7 @@ describe('platform/azure', () => {
const res = await azure.getJsonFile('file.json'); const res = await azure.getJsonFile('file.json');
expect(res).toEqual(data); expect(res).toEqual(data);
}); });
it('returns null for malformed JSON', async () => { it('throws on malformed JSON', async () => {
azureApi.gitApi.mockImplementationOnce( azureApi.gitApi.mockImplementationOnce(
() => () =>
({ ({
@ -1205,18 +1205,18 @@ describe('platform/azure', () => {
), ),
} as any) } as any)
); );
const res = await azure.getJsonFile('file.json'); await expect(azure.getJsonFile('file.json')).rejects.toThrow();
expect(res).toBeNull();
}); });
it('returns null on errors', async () => { it('throws on errors', async () => {
azureApi.gitApi.mockImplementationOnce( azureApi.gitApi.mockImplementationOnce(
() => () =>
({ ({
getItemContent: jest.fn(() => Promise.reject('some error')), getItemContent: jest.fn(() => {
throw new Error('some error');
}),
} as any) } as any)
); );
const res = await azure.getJsonFile('file.json'); await expect(azure.getJsonFile('file.json')).rejects.toThrow();
expect(res).toBeNull();
}); });
}); });
}); });

View file

@ -108,26 +108,18 @@ export async function getRawFile(
fileName: string, fileName: string,
repo: string = config.repoId repo: string = config.repoId
): Promise<string | null> { ): Promise<string | null> {
try { const azureApiGit = await azureApi.gitApi();
const azureApiGit = await azureApi.gitApi(); const buf = await azureApiGit.getItemContent(repo, fileName);
const buf = await azureApiGit.getItemContent(repo, fileName); const str = await streamToString(buf);
const str = await streamToString(buf); return str;
return str;
} catch (err) {
return null;
}
} }
export async function getJsonFile( export async function getJsonFile(
fileName: string, fileName: string,
repo: string = config.repoId repo: string = config.repoId
): Promise<any | null> { ): Promise<any | null> {
try { const raw = await getRawFile(fileName, repo);
const raw = await getRawFile(fileName, repo); return JSON.parse(raw);
return raw && JSON.parse(raw);
} catch (err) {
return null;
}
} }
export async function initRepo({ export async function initRepo({

View file

@ -1931,7 +1931,7 @@ Array [
] ]
`; `;
exports[`platform/bitbucket-server/index endpoint with no path getJsonFile() returns null for long content 1`] = ` exports[`platform/bitbucket-server/index endpoint with no path getJsonFile() throws on errors 1`] = `
Array [ Array [
Object { Object {
"headers": Object { "headers": Object {
@ -1972,7 +1972,7 @@ Array [
] ]
`; `;
exports[`platform/bitbucket-server/index endpoint with no path getJsonFile() returns null for malformed JSON 1`] = ` exports[`platform/bitbucket-server/index endpoint with no path getJsonFile() throws on long content 1`] = `
Array [ Array [
Object { Object {
"headers": Object { "headers": Object {
@ -2013,7 +2013,7 @@ Array [
] ]
`; `;
exports[`platform/bitbucket-server/index endpoint with no path getJsonFile() returns null on errors 1`] = ` exports[`platform/bitbucket-server/index endpoint with no path getJsonFile() throws on malformed JSON 1`] = `
Array [ Array [
Object { Object {
"headers": Object { "headers": Object {
@ -6062,7 +6062,7 @@ Array [
] ]
`; `;
exports[`platform/bitbucket-server/index endpoint with path getJsonFile() returns null for long content 1`] = ` exports[`platform/bitbucket-server/index endpoint with path getJsonFile() throws on errors 1`] = `
Array [ Array [
Object { Object {
"headers": Object { "headers": Object {
@ -6103,7 +6103,7 @@ Array [
] ]
`; `;
exports[`platform/bitbucket-server/index endpoint with path getJsonFile() returns null for malformed JSON 1`] = ` exports[`platform/bitbucket-server/index endpoint with path getJsonFile() throws on long content 1`] = `
Array [ Array [
Object { Object {
"headers": Object { "headers": Object {
@ -6144,7 +6144,7 @@ Array [
] ]
`; `;
exports[`platform/bitbucket-server/index endpoint with path getJsonFile() returns null on errors 1`] = ` exports[`platform/bitbucket-server/index endpoint with path getJsonFile() throws on malformed JSON 1`] = `
Array [ Array [
Object { Object {
"headers": Object { "headers": Object {

View file

@ -2107,7 +2107,7 @@ Followed by some information.
expect(res).toEqual(data); expect(res).toEqual(data);
expect(httpMock.getTrace()).toMatchSnapshot(); expect(httpMock.getTrace()).toMatchSnapshot();
}); });
it('returns null for malformed JSON', async () => { it('throws on malformed JSON', async () => {
const scope = await initRepo(); const scope = await initRepo();
scope scope
.get( .get(
@ -2117,11 +2117,10 @@ Followed by some information.
isLastPage: true, isLastPage: true,
lines: [{ text: '!@#' }], lines: [{ text: '!@#' }],
}); });
const res = await bitbucket.getJsonFile('file.json'); await expect(bitbucket.getJsonFile('file.json')).rejects.toThrow();
expect(res).toBeNull();
expect(httpMock.getTrace()).toMatchSnapshot(); expect(httpMock.getTrace()).toMatchSnapshot();
}); });
it('returns null for long content', async () => { it('throws on long content', async () => {
const scope = await initRepo(); const scope = await initRepo();
scope scope
.get( .get(
@ -2131,19 +2130,17 @@ Followed by some information.
isLastPage: false, isLastPage: false,
lines: [{ text: '{' }], lines: [{ text: '{' }],
}); });
const res = await bitbucket.getJsonFile('file.json'); await expect(bitbucket.getJsonFile('file.json')).rejects.toThrow();
expect(res).toBeNull();
expect(httpMock.getTrace()).toMatchSnapshot(); expect(httpMock.getTrace()).toMatchSnapshot();
}); });
it('returns null on errors', async () => { it('throws on errors', async () => {
const scope = await initRepo(); const scope = await initRepo();
scope scope
.get( .get(
`${urlPath}/rest/api/1.0/projects/SOME/repos/repo/browse/file.json?limit=20000` `${urlPath}/rest/api/1.0/projects/SOME/repos/repo/browse/file.json?limit=20000`
) )
.replyWithError('some error'); .replyWithError('some error');
const res = await bitbucket.getJsonFile('file.json'); await expect(bitbucket.getJsonFile('file.json')).rejects.toThrow();
expect(res).toBeNull();
expect(httpMock.getTrace()).toMatchSnapshot(); expect(httpMock.getTrace()).toMatchSnapshot();
}); });
}); });

View file

@ -122,31 +122,24 @@ export async function getRawFile(
fileName: string, fileName: string,
repo: string = config.repository repo: string = config.repository
): Promise<string | null> { ): Promise<string | null> {
try { const [project, slug] = repo.split('/');
const [project, slug] = repo.split('/'); const fileUrl = `./rest/api/1.0/projects/${project}/repos/${slug}/browse/${fileName}?limit=20000`;
const fileUrl = `./rest/api/1.0/projects/${project}/repos/${slug}/browse/${fileName}?limit=20000`; const res = await bitbucketServerHttp.getJson<FileData>(fileUrl);
const res = await bitbucketServerHttp.getJson<FileData>(fileUrl); const { isLastPage, lines, size } = res.body;
const { isLastPage, lines, size } = res.body; if (isLastPage) {
if (isLastPage) { return lines.map(({ text }) => text).join('');
return lines.map(({ text }) => text).join('');
}
logger.warn({ size }, `The file is too big`);
} catch (err) {
// no-op
} }
return null; const msg = `The file is too big (${size}B)`;
logger.warn({ size }, msg);
throw new Error(msg);
} }
export async function getJsonFile( export async function getJsonFile(
fileName: string, fileName: string,
repo: string = config.repository repo: string = config.repository
): Promise<any | null> { ): Promise<any | null> {
try { const raw = await getRawFile(fileName, repo);
const raw = await getRawFile(fileName, repo); return JSON.parse(raw);
return raw && JSON.parse(raw);
} catch (err) {
return null;
}
} }
// Initialize BitBucket Server by getting base branch // Initialize BitBucket Server by getting base branch

View file

@ -921,7 +921,7 @@ Array [
] ]
`; `;
exports[`platform/bitbucket getJsonFile() returns null for malformed JSON 1`] = ` exports[`platform/bitbucket getJsonFile() throws on errors 1`] = `
Array [ Array [
Object { Object {
"headers": Object { "headers": Object {
@ -947,7 +947,7 @@ Array [
] ]
`; `;
exports[`platform/bitbucket getJsonFile() returns null on errors 1`] = ` exports[`platform/bitbucket getJsonFile() throws on malformed JSON 1`] = `
Array [ Array [
Object { Object {
"headers": Object { "headers": Object {

View file

@ -851,22 +851,20 @@ describe('platform/bitbucket', () => {
expect(res).toEqual(data); expect(res).toEqual(data);
expect(httpMock.getTrace()).toMatchSnapshot(); expect(httpMock.getTrace()).toMatchSnapshot();
}); });
it('returns null for malformed JSON', async () => { it('throws on malformed JSON', async () => {
const scope = await initRepoMock(); const scope = await initRepoMock();
scope scope
.get('/2.0/repositories/some/repo/src/HEAD/file.json') .get('/2.0/repositories/some/repo/src/HEAD/file.json')
.reply(200, '!@#'); .reply(200, '!@#');
const res = await bitbucket.getJsonFile('file.json'); await expect(bitbucket.getJsonFile('file.json')).rejects.toThrow();
expect(res).toBeNull();
expect(httpMock.getTrace()).toMatchSnapshot(); expect(httpMock.getTrace()).toMatchSnapshot();
}); });
it('returns null on errors', async () => { it('throws on errors', async () => {
const scope = await initRepoMock(); const scope = await initRepoMock();
scope scope
.get('/2.0/repositories/some/repo/src/HEAD/file.json') .get('/2.0/repositories/some/repo/src/HEAD/file.json')
.replyWithError('some error'); .replyWithError('some error');
const res = await bitbucket.getJsonFile('file.json'); await expect(bitbucket.getJsonFile('file.json')).rejects.toThrow();
expect(res).toBeNull();
expect(httpMock.getTrace()).toMatchSnapshot(); expect(httpMock.getTrace()).toMatchSnapshot();
}); });
}); });

View file

@ -103,27 +103,19 @@ export async function getRawFile(
fileName: string, fileName: string,
repo: string = config.repository repo: string = config.repository
): Promise<string | null> { ): Promise<string | null> {
try { // See: https://developer.atlassian.com/bitbucket/api/2/reference/resource/repositories/%7Bworkspace%7D/%7Brepo_slug%7D/src/%7Bcommit%7D/%7Bpath%7D
// See: https://developer.atlassian.com/bitbucket/api/2/reference/resource/repositories/%7Bworkspace%7D/%7Brepo_slug%7D/src/%7Bcommit%7D/%7Bpath%7D const path = fileName;
const path = fileName; const url = `/2.0/repositories/${repo}/src/HEAD/${path}`;
const url = `/2.0/repositories/${repo}/src/HEAD/${path}`; const res = await bitbucketHttp.get(url);
const res = await bitbucketHttp.get(url); return res.body;
return res.body;
} catch (err) {
return null;
}
} }
export async function getJsonFile( export async function getJsonFile(
fileName: string, fileName: string,
repo: string = config.repository repo: string = config.repository
): Promise<any | null> { ): Promise<any | null> {
try { const raw = await getRawFile(fileName, repo);
const raw = await getRawFile(fileName, repo); return JSON.parse(raw);
return raw && JSON.parse(raw);
} catch (err) {
return null;
}
} }
// Initialize bitbucket by getting base branch and SHA // Initialize bitbucket by getting base branch and SHA

View file

@ -1363,19 +1363,17 @@ describe(getName(__filename), () => {
const res = await gitea.getJsonFile('file.json'); const res = await gitea.getJsonFile('file.json');
expect(res).toEqual(data); expect(res).toEqual(data);
}); });
it('returns null for malformed JSON', async () => { it('throws on malformed JSON', async () => {
helper.getRepoContents.mockResolvedValueOnce({ helper.getRepoContents.mockResolvedValueOnce({
contentString: '!@#', contentString: '!@#',
} as never); } as never);
await initFakeRepo({ full_name: 'some/repo' }); await initFakeRepo({ full_name: 'some/repo' });
const res = await gitea.getJsonFile('file.json'); await expect(gitea.getJsonFile('file.json')).rejects.toThrow();
expect(res).toBeNull();
}); });
it('returns null on errors', async () => { it('throws on errors', async () => {
helper.getRepoContents.mockRejectedValueOnce('some error'); helper.getRepoContents.mockRejectedValueOnce(new Error('some error'));
await initFakeRepo({ full_name: 'some/repo' }); await initFakeRepo({ full_name: 'some/repo' });
const res = await gitea.getJsonFile('file.json'); await expect(gitea.getJsonFile('file.json')).rejects.toThrow();
expect(res).toBeNull();
}); });
}); });
}); });

View file

@ -212,24 +212,16 @@ const platform: Platform = {
fileName: string, fileName: string,
repo: string = config.repository repo: string = config.repository
): Promise<string | null> { ): Promise<string | null> {
try { const contents = await helper.getRepoContents(repo, fileName);
const contents = await helper.getRepoContents(repo, fileName); return contents.contentString;
return contents.contentString;
} catch (err) /* istanbul ignore next */ {
return null;
}
}, },
async getJsonFile( async getJsonFile(
fileName: string, fileName: string,
repo: string = config.repository repo: string = config.repository
): Promise<any | null> { ): Promise<any | null> {
try { const raw = await platform.getRawFile(fileName, repo);
const raw = await platform.getRawFile(fileName, repo); return JSON.parse(raw);
return raw && JSON.parse(raw);
} catch (err) /* istanbul ignore next */ {
return null;
}
}, },
async initRepo({ async initRepo({

View file

@ -3454,7 +3454,7 @@ Array [
] ]
`; `;
exports[`platform/github getJsonFile() returns null for malformed JSON 1`] = ` exports[`platform/github getJsonFile() throws on errors 1`] = `
Array [ Array [
Object { Object {
"graphql": Object { "graphql": Object {
@ -3505,7 +3505,7 @@ Array [
] ]
`; `;
exports[`platform/github getJsonFile() returns null on errors 1`] = ` exports[`platform/github getJsonFile() throws on malformed JSON 1`] = `
Array [ Array [
Object { Object {
"graphql": Object { "graphql": Object {

View file

@ -2152,18 +2152,17 @@ describe('platform/github', () => {
expect(res).toEqual(data); expect(res).toEqual(data);
expect(httpMock.getTrace()).toMatchSnapshot(); expect(httpMock.getTrace()).toMatchSnapshot();
}); });
it('returns null for malformed JSON', async () => { it('throws on malformed JSON', async () => {
const scope = httpMock.scope(githubApiHost); const scope = httpMock.scope(githubApiHost);
initRepoMock(scope, 'some/repo'); initRepoMock(scope, 'some/repo');
await github.initRepo({ repository: 'some/repo', token: 'token' } as any); await github.initRepo({ repository: 'some/repo', token: 'token' } as any);
scope.get('/repos/some/repo/contents/file.json').reply(200, { scope.get('/repos/some/repo/contents/file.json').reply(200, {
content: Buffer.from('!@#').toString('base64'), content: Buffer.from('!@#').toString('base64'),
}); });
const res = await github.getJsonFile('file.json'); await expect(github.getJsonFile('file.json')).rejects.toThrow();
expect(res).toBeNull();
expect(httpMock.getTrace()).toMatchSnapshot(); expect(httpMock.getTrace()).toMatchSnapshot();
}); });
it('returns null on errors', async () => { it('throws on errors', async () => {
const scope = httpMock.scope(githubApiHost); const scope = httpMock.scope(githubApiHost);
initRepoMock(scope, 'some/repo'); initRepoMock(scope, 'some/repo');
await github.initRepo({ repository: 'some/repo', token: 'token' } as any); await github.initRepo({ repository: 'some/repo', token: 'token' } as any);
@ -2171,8 +2170,7 @@ describe('platform/github', () => {
.get('/repos/some/repo/contents/file.json') .get('/repos/some/repo/contents/file.json')
.replyWithError('some error'); .replyWithError('some error');
const res = await github.getJsonFile('file.json'); await expect(github.getJsonFile('file.json')).rejects.toThrow();
expect(res).toBeNull();
expect(httpMock.getTrace()).toMatchSnapshot(); expect(httpMock.getTrace()).toMatchSnapshot();
}); });
}); });

View file

@ -145,27 +145,19 @@ export async function getRawFile(
fileName: string, fileName: string,
repo: string = config.repository repo: string = config.repository
): Promise<string | null> { ): Promise<string | null> {
try { const url = `repos/${repo}/contents/${fileName}`;
const url = `repos/${repo}/contents/${fileName}`; const res = await githubApi.getJson<{ content: string }>(url);
const res = await githubApi.getJson<{ content: string }>(url); const buf = res.body.content;
const buf = res.body.content; const str = Buffer.from(buf, 'base64').toString();
const str = Buffer.from(buf, 'base64').toString(); return str;
return str;
} catch (err) {
return null;
}
} }
export async function getJsonFile( export async function getJsonFile(
fileName: string, fileName: string,
repo: string = config.repository repo: string = config.repository
): Promise<any | null> { ): Promise<any | null> {
try { const raw = await getRawFile(fileName, repo);
const raw = await getRawFile(fileName, repo); return JSON.parse(raw);
return raw && JSON.parse(raw);
} catch (err) {
return null;
}
} }
let existingRepos; let existingRepos;

View file

@ -1551,7 +1551,7 @@ Array [
] ]
`; `;
exports[`platform/gitlab getJsonFile() returns null for malformed JSON 1`] = ` exports[`platform/gitlab getJsonFile() throws on errors 1`] = `
Array [ Array [
Object { Object {
"headers": Object { "headers": Object {
@ -1578,7 +1578,7 @@ Array [
] ]
`; `;
exports[`platform/gitlab getJsonFile() returns null on errors 1`] = ` exports[`platform/gitlab getJsonFile() throws on malformed JSON 1`] = `
Array [ Array [
Object { Object {
"headers": Object { "headers": Object {
@ -1600,7 +1600,7 @@ Array [
"user-agent": "https://github.com/renovatebot/renovate", "user-agent": "https://github.com/renovatebot/renovate",
}, },
"method": "GET", "method": "GET",
"url": "https://gitlab.com/api/v4/projects/some%2Frepo/repository/files/file.json?ref=HEAD", "url": "https://gitlab.com/api/v4/projects/some%2Frepo/repository/files/dir%2Ffile.json?ref=HEAD",
}, },
] ]
`; `;

View file

@ -1447,7 +1447,7 @@ These updates have all been created already. Click a checkbox below to force a r
expect(res).toEqual(data); expect(res).toEqual(data);
expect(httpMock.getTrace()).toMatchSnapshot(); expect(httpMock.getTrace()).toMatchSnapshot();
}); });
it('returns null for malformed JSON', async () => { it('throws on malformed JSON', async () => {
const scope = await initRepo(); const scope = await initRepo();
scope scope
.get( .get(
@ -1456,17 +1456,17 @@ These updates have all been created already. Click a checkbox below to force a r
.reply(200, { .reply(200, {
content: Buffer.from('!@#').toString('base64'), content: Buffer.from('!@#').toString('base64'),
}); });
const res = await gitlab.getJsonFile('dir/file.json'); await expect(gitlab.getJsonFile('dir/file.json')).rejects.toThrow();
expect(res).toBeNull();
expect(httpMock.getTrace()).toMatchSnapshot(); expect(httpMock.getTrace()).toMatchSnapshot();
}); });
it('returns null on errors', async () => { it('throws on errors', async () => {
const scope = await initRepo(); const scope = await initRepo();
scope scope
.get('/api/v4/projects/some%2Frepo/repository/files/file.json?ref=HEAD') .get(
'/api/v4/projects/some%2Frepo/repository/files/dir%2Ffile.json?ref=HEAD'
)
.replyWithError('some error'); .replyWithError('some error');
const res = await gitlab.getJsonFile('file.json'); await expect(gitlab.getJsonFile('dir/file.json')).rejects.toThrow();
expect(res).toBeNull();
expect(httpMock.getTrace()).toMatchSnapshot(); expect(httpMock.getTrace()).toMatchSnapshot();
}); });
}); });

View file

@ -143,28 +143,20 @@ export async function getRawFile(
fileName: string, fileName: string,
repo: string = config.repository repo: string = config.repository
): Promise<string | null> { ): Promise<string | null> {
try { const escapedFileName = urlEscape(fileName);
const escapedFileName = urlEscape(fileName); const url = `projects/${repo}/repository/files/${escapedFileName}?ref=HEAD`;
const url = `projects/${repo}/repository/files/${escapedFileName}?ref=HEAD`; const res = await gitlabApi.getJson<{ content: string }>(url);
const res = await gitlabApi.getJson<{ content: string }>(url); const buf = res.body.content;
const buf = res.body.content; const str = Buffer.from(buf, 'base64').toString();
const str = Buffer.from(buf, 'base64').toString(); return str;
return str;
} catch (err) {
return null;
}
} }
export async function getJsonFile( export async function getJsonFile(
fileName: string, fileName: string,
repo: string = config.repository repo: string = config.repository
): Promise<any | null> { ): Promise<any | null> {
try { const raw = await getRawFile(fileName, repo);
const raw = await getRawFile(fileName, repo); return JSON.parse(raw);
return raw && JSON.parse(raw);
} catch (err) {
return null;
}
} }
// Initialize GitLab by getting base branch // Initialize GitLab by getting base branch

View file

@ -53,6 +53,21 @@ describe('workers/repository/init/apis', () => {
}) })
).rejects.toThrow(REPOSITORY_FORKED); ).rejects.toThrow(REPOSITORY_FORKED);
}); });
it('ignores platform.getJsonFile() failures', async () => {
platform.initRepo.mockResolvedValueOnce({
defaultBranch: 'master',
isFork: false,
});
platform.getJsonFile.mockRejectedValue(new Error());
await expect(
initApis({
...config,
optimizeForDisabled: true,
includeForks: false,
isFork: true,
})
).resolves.not.toThrow();
});
it('uses the onboardingConfigFileName if set', async () => { it('uses the onboardingConfigFileName if set', async () => {
platform.initRepo.mockResolvedValueOnce({ platform.initRepo.mockResolvedValueOnce({
defaultBranch: 'master', defaultBranch: 'master',

View file

@ -17,13 +17,19 @@ const defaultConfigFile = (config: RenovateConfig): string =>
? config.onboardingConfigFileName ? config.onboardingConfigFileName
: configFileNames[0]; : configFileNames[0];
async function getJsonFile(file: string): Promise<RenovateConfig | null> {
try {
return await platform.getJsonFile(file);
} catch (err) {
return null;
}
}
async function validateOptimizeForDisabled( async function validateOptimizeForDisabled(
config: RenovateConfig config: RenovateConfig
): Promise<void> { ): Promise<void> {
if (config.optimizeForDisabled) { if (config.optimizeForDisabled) {
const renovateConfig = await platform.getJsonFile( const renovateConfig = await getJsonFile(defaultConfigFile(config));
defaultConfigFile(config)
);
if (renovateConfig?.enabled === false) { if (renovateConfig?.enabled === false) {
throw new Error(REPOSITORY_DISABLED_BY_CONFIG); throw new Error(REPOSITORY_DISABLED_BY_CONFIG);
} }
@ -32,9 +38,7 @@ async function validateOptimizeForDisabled(
async function validateIncludeForks(config: RenovateConfig): Promise<void> { async function validateIncludeForks(config: RenovateConfig): Promise<void> {
if (!config.includeForks && config.isFork) { if (!config.includeForks && config.isFork) {
const renovateConfig = await platform.getJsonFile( const renovateConfig = await getJsonFile(defaultConfigFile(config));
defaultConfigFile(config)
);
if (!renovateConfig?.includeForks) { if (!renovateConfig?.includeForks) {
throw new Error(REPOSITORY_FORKED); throw new Error(REPOSITORY_FORKED);
} }