From 7d1c9376823b1decee349889e6ea6d8f11e1e151 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Tue, 5 May 2020 12:39:47 +0200
Subject: [PATCH] refactor(platform): optimize getFileList and remove
branchName param (#6127)
---
lib/platform/azure/index.ts | 9 +---
lib/platform/bitbucket-server/index.ts | 8 +--
lib/platform/bitbucket/index.ts | 8 +--
lib/platform/bitbucket/utils.ts | 1 -
.../git/__snapshots__/storage.spec.ts.snap | 15 +++---
lib/platform/git/storage.spec.ts | 16 +++---
lib/platform/git/storage.ts | 54 ++++++++-----------
lib/platform/gitea/index.ts | 2 +-
lib/platform/github/index.ts | 4 +-
lib/platform/gitlab/index.ts | 4 +-
lib/workers/repository/extract/file-match.ts | 6 +--
11 files changed, 52 insertions(+), 75 deletions(-)
diff --git a/lib/platform/azure/index.ts b/lib/platform/azure/index.ts
index 903c723509..711fd0ba3d 100644
--- a/lib/platform/azure/index.ts
+++ b/lib/platform/azure/index.ts
@@ -48,7 +48,6 @@ interface Config {
project: string;
azureWorkItemId: string;
prList: Pr[];
- fileList: null;
repository: string;
}
@@ -182,10 +181,8 @@ export function getRepoForceRebase(): Promise<boolean> {
// Search
-export /* istanbul ignore next */ function getFileList(
- branchName?: string
-): Promise<string[]> {
- return config.storage.getFileList(branchName);
+export /* istanbul ignore next */ function getFileList(): Promise<string[]> {
+ return config.storage.getFileList();
}
export /* istanbul ignore next */ async function setBaseBranch(
@@ -194,9 +191,7 @@ export /* istanbul ignore next */ async function setBaseBranch(
logger.debug(`Setting baseBranch to ${branchName}`);
config.baseBranch = branchName;
delete config.baseCommitSHA;
- delete config.fileList;
const baseBranchSha = await config.storage.setBaseBranch(branchName);
- await getFileList(branchName);
return baseBranchSha;
}
diff --git a/lib/platform/bitbucket-server/index.ts b/lib/platform/bitbucket-server/index.ts
index 2d5f726bc5..3cdd5b269b 100644
--- a/lib/platform/bitbucket-server/index.ts
+++ b/lib/platform/bitbucket-server/index.ts
@@ -49,7 +49,6 @@ interface BbsConfig {
baseBranch: string;
bbUseDefaultReviewers: boolean;
defaultBranch: string;
- fileList: any[];
mergeMethod: string;
owner: string;
prList: Pr[];
@@ -273,11 +272,8 @@ export /* istanbul ignore next */ function setBranchPrefix(
// Search
// Get full file list
-export function getFileList(
- branchName: string = config.baseBranch
-): Promise<string[]> {
- logger.debug(`getFileList(${branchName})`);
- return config.storage.getFileList(branchName);
+export function getFileList(): Promise<string[]> {
+ return config.storage.getFileList();
}
// Branch
diff --git a/lib/platform/bitbucket/index.ts b/lib/platform/bitbucket/index.ts
index d516a162c3..a7c0f31919 100644
--- a/lib/platform/bitbucket/index.ts
+++ b/lib/platform/bitbucket/index.ts
@@ -173,8 +173,8 @@ export function getRepoForceRebase(): Promise<boolean> {
// Search
// Get full file list
-export function getFileList(branchName?: string): Promise<string[]> {
- return config.storage.getFileList(branchName);
+export function getFileList(): Promise<string[]> {
+ return config.storage.getFileList();
}
export async function setBaseBranch(
@@ -182,10 +182,7 @@ export async function setBaseBranch(
): Promise<string> {
logger.debug(`Setting baseBranch to ${branchName}`);
config.baseBranch = branchName;
- delete config.baseCommitSHA;
- delete config.fileList;
const baseBranchSha = await config.storage.setBaseBranch(branchName);
- await getFileList(branchName);
return baseBranchSha;
}
@@ -833,7 +830,6 @@ export async function mergePr(
},
}
);
- delete config.baseCommitSHA;
logger.debug('Automerging succeeded');
} catch (err) /* istanbul ignore next */ {
return false;
diff --git a/lib/platform/bitbucket/utils.ts b/lib/platform/bitbucket/utils.ts
index 1461bf2860..c4721cbbee 100644
--- a/lib/platform/bitbucket/utils.ts
+++ b/lib/platform/bitbucket/utils.ts
@@ -9,7 +9,6 @@ export interface Config {
baseBranch: string;
baseCommitSHA: string;
defaultBranch: string;
- fileList: any[];
has_issues: boolean;
mergeMethod: string;
owner: string;
diff --git a/lib/platform/git/__snapshots__/storage.spec.ts.snap b/lib/platform/git/__snapshots__/storage.spec.ts.snap
index 28860dd7e4..84944dea60 100644
--- a/lib/platform/git/__snapshots__/storage.spec.ts.snap
+++ b/lib/platform/git/__snapshots__/storage.spec.ts.snap
@@ -11,21 +11,24 @@ Array [
exports[`platform/git/storage getFile(filePath, branchName) returns null for 404 1`] = `[Error: repository-changed]`;
-exports[`platform/git/storage getFileList() defaults to master 1`] = `Array []`;
-
-exports[`platform/git/storage getFileList() should exclude submodules 1`] = `
+exports[`platform/git/storage getFileList() returns cached version 1`] = `
Array [
- ".gitmodules",
"file_to_delete",
"master_file",
"past_file",
]
`;
-exports[`platform/git/storage getFileList() should return the correct files 1`] = `
+exports[`platform/git/storage getFileList() returns from other branch 1`] = `
+Array [
+ "past_file",
+]
+`;
+
+exports[`platform/git/storage getFileList() should exclude submodules 1`] = `
Array [
+ ".gitmodules",
"file_to_delete",
- "future_file",
"master_file",
"past_file",
]
diff --git a/lib/platform/git/storage.spec.ts b/lib/platform/git/storage.spec.ts
index 30c12c7148..5f116968a6 100644
--- a/lib/platform/git/storage.spec.ts
+++ b/lib/platform/git/storage.spec.ts
@@ -82,12 +82,6 @@ describe('platform/git/storage', () => {
});
});
describe('getFileList()', () => {
- it('returns empty array if error', async () => {
- expect(await git.getFileList('not_found')).toEqual([]);
- });
- it('should return the correct files', async () => {
- expect(await git.getFileList('renovate/future_branch')).toMatchSnapshot();
- });
it('should exclude submodules', async () => {
const repo = Git(base.path).silent(true);
await repo.submoduleAdd(base.path, 'submodule');
@@ -97,12 +91,18 @@ describe('platform/git/storage', () => {
url: base.path,
});
expect(await fs.exists(tmpDir.path + '/.gitmodules')).toBeTruthy();
- expect(await git.getFileList('master')).toMatchSnapshot();
+ expect(await git.getFileList()).toMatchSnapshot();
await repo.reset(['--hard', 'HEAD^']);
});
- it('defaults to master', async () => {
+ it('returns cached version', async () => {
expect(await git.getFileList()).toMatchSnapshot();
});
+ it('returns from other branch', async () => {
+ await git.setBaseBranch('develop');
+ const res = await git.getFileList();
+ expect(res).toMatchSnapshot();
+ expect(res).toHaveLength(1);
+ });
});
describe('branchExists(branchName)', () => {
it('should return true if found', async () => {
diff --git a/lib/platform/git/storage.ts b/lib/platform/git/storage.ts
index 2aca6841e7..0856e19cfb 100644
--- a/lib/platform/git/storage.ts
+++ b/lib/platform/git/storage.ts
@@ -32,7 +32,7 @@ interface LocalConfig extends StorageConfig {
baseBranchSha: string;
branchExists: Record<string, boolean>;
branchPrefix: string;
- fileList: Record<string, Promise<string[]>>;
+ fileList?: string[];
}
// istanbul ignore next
@@ -109,7 +109,6 @@ export class Storage {
// eslint-disable-next-line no-multi-assign
const config: LocalConfig = (this._config = {
...args,
- fileList: {},
} as any);
// eslint-disable-next-line no-multi-assign
const cwd = (this._cwd = config.localDir);
@@ -260,6 +259,7 @@ export class Storage {
}
logger.debug(`Setting baseBranch to ${branchName}`);
this._config.baseBranch = branchName;
+ delete this._config.fileList;
try {
if (branchName !== 'master') {
this._config.baseBranchSha = (
@@ -305,36 +305,28 @@ export class Storage {
}
}
- async getFileList(branchName?: string): Promise<string[]> {
- const branch = branchName || this._config.baseBranch;
- if (this._config.fileList[branch] === undefined) {
- this._config.fileList[branch] = this.getFileListInner(branchName);
- }
- return this._config.fileList[branch];
- }
-
- async getFileListInner(branch: string): Promise<string[]> {
- const exists = await this.branchExists(branch);
- if (!exists) {
- return [];
- }
- const submodules = await this.getSubmodules();
- const files: string = await this._git.raw([
- 'ls-tree',
- '-r',
- '--name-only',
- 'origin/' + branch,
- ]);
- // istanbul ignore if
- if (!files) {
- return [];
+ async getFileList(): Promise<string[]> {
+ if (!this._config.fileList) {
+ const submodules = await this.getSubmodules();
+ const files: string = await this._git.raw([
+ 'ls-tree',
+ '-r',
+ '--name-only',
+ 'HEAD',
+ ]);
+ // istanbul ignore else
+ if (files) {
+ this._config.fileList = files
+ .split('\n')
+ .filter(Boolean)
+ .filter((file: string) =>
+ submodules.every((submodule: string) => !file.startsWith(submodule))
+ );
+ } else {
+ this._config.fileList = [];
+ }
}
- return files
- .split('\n')
- .filter(Boolean)
- .filter((file: string) =>
- submodules.every((submodule: string) => !file.startsWith(submodule))
- );
+ return this._config.fileList;
}
async getSubmodules(): Promise<string[]> {
diff --git a/lib/platform/gitea/index.ts b/lib/platform/gitea/index.ts
index cbdc65eb0f..e50378ddf0 100644
--- a/lib/platform/gitea/index.ts
+++ b/lib/platform/gitea/index.ts
@@ -929,7 +929,7 @@ const platform: Platform = {
},
getFileList(): Promise<string[]> {
- return config.storage.getFileList(config.baseBranch);
+ return config.storage.getFileList();
},
getAllRenovateBranches(branchPrefix: string): Promise<string[]> {
diff --git a/lib/platform/github/index.ts b/lib/platform/github/index.ts
index d1080f761c..9a1e7d5be4 100644
--- a/lib/platform/github/index.ts
+++ b/lib/platform/github/index.ts
@@ -535,8 +535,8 @@ export function setBranchPrefix(branchPrefix: string): Promise<void> {
// Search
// istanbul ignore next
-export function getFileList(branchName = config.baseBranch): Promise<string[]> {
- return config.storage.getFileList(branchName);
+export function getFileList(): Promise<string[]> {
+ return config.storage.getFileList();
}
// Branch
diff --git a/lib/platform/gitlab/index.ts b/lib/platform/gitlab/index.ts
index e3c620e7b2..e5418ad2a4 100644
--- a/lib/platform/gitlab/index.ts
+++ b/lib/platform/gitlab/index.ts
@@ -286,8 +286,8 @@ export /* istanbul ignore next */ function setBranchPrefix(
// Search
// Get full file list
-export function getFileList(branchName = config.baseBranch): Promise<string[]> {
- return config.storage.getFileList(branchName);
+export function getFileList(): Promise<string[]> {
+ return config.storage.getFileList();
}
// Returns true if branch exists, otherwise false
diff --git a/lib/workers/repository/extract/file-match.ts b/lib/workers/repository/extract/file-match.ts
index 04e6ee64f0..b3a976ce0c 100644
--- a/lib/workers/repository/extract/file-match.ts
+++ b/lib/workers/repository/extract/file-match.ts
@@ -35,10 +35,6 @@ export function filterIgnoredFiles(
);
}
-export function getFileList(): Promise<string[]> {
- return platform.getFileList();
-}
-
export function getFilteredFileList(
config: RenovateConfig,
fileList: string[]
@@ -52,7 +48,7 @@ export function getFilteredFileList(
export async function getMatchingFiles(
config: RenovateConfig
): Promise<string[]> {
- const allFiles = await getFileList();
+ const allFiles = await platform.getFileList();
const fileList = getFilteredFileList(config, allFiles);
const { fileMatch, manager } = config;
let matchedFiles = [];
--
GitLab