diff --git a/lib/util/cache/repository/types.ts b/lib/util/cache/repository/types.ts index 146b59be5bcbc588f585297e3405414fb226cba3..e11a5c69315f39762e3acc27ced03ef4bf0ffebc 100644 --- a/lib/util/cache/repository/types.ts +++ b/lib/util/cache/repository/types.ts @@ -27,14 +27,38 @@ export interface BranchUpgradeCache { } export interface BranchCache { + /** + *Whether this branch has automerge enabled + */ automerge: boolean; + /** + * Hash of the manager fingerprints and the update branch config + */ + branchFingerprint?: string; + /** + * Branch name + */ branchName: string; + /** + * Whether a person not listed in gitIgnoredAuthors updated the branch. + */ isModified: boolean; + /** + * Parent commit of branch sha + */ + parentSha: string | null; + /** + * Pr nunber of PR created from this branch + */ prNo: number | null; + /** + * The branch's most recent commit SHA + */ sha: string | null; - parentSha: string | null; + /** + * Details on the dependency upgrades that have been applied in this branch + */ upgrades: BranchUpgradeCache[]; - branchFingerprint?: string; } export interface RepoCacheData { diff --git a/lib/util/git/index.ts b/lib/util/git/index.ts index 769b0c646b2bcf482ae8043e9dacab71ed58b7fe..55b466ae0bac6333a4c9b27ed562591043222df5 100644 --- a/lib/util/git/index.ts +++ b/lib/util/git/index.ts @@ -612,6 +612,7 @@ export async function isBranchModified(branchName: string): Promise<boolean> { config.branchCommits[branchName] ); if (isModified !== null) { + logger.debug('Using cached result for isBranchModified'); return (config.branchIsModified[branchName] = isModified); } diff --git a/lib/util/git/modified-cache.spec.ts b/lib/util/git/modified-cache.spec.ts index 510e9aaec64292dd420b213e7105f59741f12dc5..58c3105c1163a58e76b7dc8afdb93b9670a894d0 100644 --- a/lib/util/git/modified-cache.spec.ts +++ b/lib/util/git/modified-cache.spec.ts @@ -1,4 +1,4 @@ -import { mocked } from '../../../test/util'; +import { logger, mocked, partial } from '../../../test/util'; import * as _repositoryCache from '../cache/repository'; import type { BranchCache, RepoCacheData } from '../cache/repository/types'; import { @@ -27,59 +27,128 @@ describe('util/git/modified-cache', () => { }); it('returns null if target SHA has changed', () => { - repoCache.branches = [{ branchName: 'foo', sha: 'aaa' } as BranchCache]; + repoCache.branches = [ + partial<BranchCache>({ branchName: 'foo', sha: 'aaa' }), + ]; expect(getCachedModifiedResult('foo', '111')).toBeNull(); }); - it('returns true', () => { + it('returns null if cache is partially defined', () => { repoCache.branches = [ - { branchName: 'foo', sha: '111', isModified: true } as BranchCache, + partial<BranchCache>({ + branchName: 'foo', + sha: '111', + }), ]; - expect(getCachedModifiedResult('foo', '111')).toBeTrue(); + expect(getCachedModifiedResult('foo', '111')).toBeNull(); }); - it('returns false', () => { + it('returns cached value', () => { repoCache.branches = [ - { branchName: 'foo', sha: '111', isModified: false } as BranchCache, + partial<BranchCache>({ + branchName: 'foo', + sha: '111', + isModified: false, + }), ]; expect(getCachedModifiedResult('foo', '111')).toBeFalse(); }); }); describe('setCachedModifiedResult', () => { - it('sets value for unpopulated cache', () => { + it('does not create new branch cache for when cache is empty', () => { setCachedModifiedResult('foo', '111', false); - expect(repoCache).toEqual({ - branches: [{ branchName: 'foo', sha: '111', isModified: false }], - }); + expect(repoCache).toEqual({}); + expect(logger.logger.debug).toHaveBeenCalledWith( + 'Branch cache not present for foo' + ); }); it('replaces value when SHA has changed', () => { - setCachedModifiedResult('foo', '111', false); - setCachedModifiedResult('foo', '121', false); + repoCache = { + branches: [ + partial<BranchCache>({ + branchName: 'foo', + sha: '121', + isModified: true, + }), + ], + }; + repositoryCache.getCache.mockReturnValue(repoCache); setCachedModifiedResult('foo', '131', false); expect(repoCache).toEqual({ - branches: [{ branchName: 'foo', sha: '131', isModified: false }], + branches: [ + { + branchName: 'foo', + sha: '131', + isModified: false, + }, + ], }); }); - it('replaces value when both value and SHA have changed', () => { - setCachedModifiedResult('foo', '111', false); - setCachedModifiedResult('foo', 'aaa', true); + it('adds SHA when it is not found', () => { + repoCache = { + branches: [ + partial<BranchCache>({ + branchName: 'foo', + }), + ], + }; + repositoryCache.getCache.mockReturnValue(repoCache); + setCachedModifiedResult('foo', '131', false); expect(repoCache).toEqual({ - branches: [{ branchName: 'foo', sha: 'aaa', isModified: true }], + branches: [ + { + branchName: 'foo', + sha: '131', + isModified: false, + }, + ], }); }); it('handles multiple branches', () => { + repoCache = { + branches: [ + partial<BranchCache>({ + branchName: 'foo-1', + sha: '111', + isModified: true, + }), + partial<BranchCache>({ + branchName: 'foo-2', + sha: 'aaa', + isModified: false, + }), + partial<BranchCache>({ + branchName: 'foo-3', + sha: '222', + isModified: true, + }), + ], + }; + repositoryCache.getCache.mockReturnValue(repoCache); setCachedModifiedResult('foo-1', '111', false); setCachedModifiedResult('foo-2', 'aaa', true); setCachedModifiedResult('foo-3', '222', false); expect(repoCache).toEqual({ branches: [ - { branchName: 'foo-1', sha: '111', isModified: false }, - { branchName: 'foo-2', sha: 'aaa', isModified: true }, - { branchName: 'foo-3', sha: '222', isModified: false }, + { + branchName: 'foo-1', + sha: '111', + isModified: false, + }, + { + branchName: 'foo-2', + sha: 'aaa', + isModified: true, + }, + { + branchName: 'foo-3', + sha: '222', + isModified: false, + }, ], }); }); diff --git a/lib/util/git/modified-cache.ts b/lib/util/git/modified-cache.ts index 1d46d8239612f3f91b8f8c9e648f900946980216..2b93bb470e6ea071a9b4d364fc83673624aa5028 100644 --- a/lib/util/git/modified-cache.ts +++ b/lib/util/git/modified-cache.ts @@ -1,6 +1,5 @@ -import is from '@sindresorhus/is'; +import { logger } from '../../logger'; import { getCache } from '../cache/repository'; -import type { BranchCache } from '../cache/repository/types'; export function getCachedModifiedResult( branchName: string, @@ -9,11 +8,11 @@ export function getCachedModifiedResult( const { branches } = getCache(); const branch = branches?.find((branch) => branch.branchName === branchName); - if (branch?.sha !== branchSha) { - return null; + if (branch?.sha === branchSha && branch.isModified !== undefined) { + return branch.isModified; } - return branch.isModified; + return null; } export function setCachedModifiedResult( @@ -22,18 +21,16 @@ export function setCachedModifiedResult( isModified: boolean ): void { const cache = getCache(); - cache.branches ??= []; - const { branches } = cache; - const branch = - branches?.find((branch) => branch.branchName === branchName) ?? - ({ branchName: branchName } as BranchCache); + const branch = cache.branches?.find( + (branch) => branch.branchName === branchName + ); - // if branch not present add it to cache - if (is.undefined(branch.sha)) { - branches.push(branch); + if (!branch) { + logger.debug(`Branch cache not present for ${branchName}`); + return; } - if (branch.sha !== branchSha) { + if (!branch.sha || branch.sha !== branchSha) { branch.sha = branchSha; } diff --git a/lib/workers/repository/process/write.spec.ts b/lib/workers/repository/process/write.spec.ts index 3d084b6ad9af46c6c335075ea93c613401f192a0..01a26e1993dda15c0a9f91d91fb8f22a39dc9d40 100644 --- a/lib/workers/repository/process/write.spec.ts +++ b/lib/workers/repository/process/write.spec.ts @@ -12,7 +12,10 @@ import { GlobalConfig } from '../../../config/global'; import { addMeta } from '../../../logger'; import { hashMap } from '../../../modules/manager'; import * as _repoCache from '../../../util/cache/repository'; -import type { BranchCache } from '../../../util/cache/repository/types'; +import type { + BranchCache, + RepoCacheData, +} from '../../../util/cache/repository/types'; import { Limit, isLimitReached } from '../../global/limits'; import { BranchConfig, BranchResult, BranchUpgradeConfig } from '../../types'; import * as _branchWorker from '../update/branch'; @@ -107,7 +110,7 @@ describe('workers/repository/process/write', () => { }); }); - it('return nowork if same updates', async () => { + it('return no-work if branch fingerprint is not different', async () => { const branches = partial<BranchConfig[]>([ { branchName: 'new/some-branch', @@ -137,7 +140,7 @@ describe('workers/repository/process/write', () => { expect(await writeUpdates(config, branches)).toBe('done'); }); - it('updates branch fingerprint when commit is made', async () => { + it('updates branch fingerprint when new commit is made', async () => { const branches = partial<BranchConfig[]>([ { branchName: 'new/some-branch', @@ -205,6 +208,40 @@ describe('workers/repository/process/write', () => { 'No branch cache found for new/some-branch' ); }); + + it('adds branch to cache when cache is not enabled', async () => { + const branches = partial<BranchConfig[]>([ + { + branchName: 'new/some-branch', + manager: 'npm', + upgrades: [ + { + manager: 'npm', + } as BranchUpgradeConfig, + ], + }, + ]); + const repoCacheObj = {} as RepoCacheData; + repoCache.getCache.mockReturnValueOnce(repoCacheObj); + branchWorker.processBranch.mockResolvedValueOnce({ + branchExists: true, + result: BranchResult.NoWork, + }); + git.getBranchCommit.mockReturnValue('101'); + git.branchExists.mockReturnValueOnce(true); + await writeUpdates(config, branches); + expect(logger.logger.debug).not.toHaveBeenCalledWith( + 'No branch cache found for new/some-branch' + ); + expect(repoCacheObj).toEqual({ + branches: [ + { + branchName: 'new/some-branch', + sha: '101', + }, + ], + }); + }); }); describe('canSkipBranchUpdateCheck()', () => { diff --git a/lib/workers/repository/process/write.ts b/lib/workers/repository/process/write.ts index f356ae793c1ae4dfd828646b201e5f9d49edf6eb..9b442079daf8e8464983e9087bc774f155cd5259 100644 --- a/lib/workers/repository/process/write.ts +++ b/lib/workers/repository/process/write.ts @@ -6,7 +6,7 @@ import { addMeta, logger, removeMeta } from '../../../logger'; import { hashMap } from '../../../modules/manager'; import { getCache } from '../../../util/cache/repository'; import type { BranchCache } from '../../../util/cache/repository/types'; -import { branchExists } from '../../../util/git'; +import { branchExists, getBranchCommit } from '../../../util/git'; import { Limit, incLimitedValue, setMaxLimit } from '../../global/limits'; import { BranchConfig, BranchResult } from '../../types'; import { processBranch } from '../update/branch'; @@ -45,7 +45,8 @@ export async function writeUpdates( .join(', ')}` ); const cache = getCache(); - const { branches: cachedBranches = [] } = cache; + cache.branches ??= []; + const { branches: cachedBranches } = cache; const prsRemaining = await getPrsRemaining(config, branches); logger.debug({ prsRemaining }, 'Calculated maximum PRs remaining this run'); setMaxLimit(Limit.PullRequests, prsRemaining); @@ -59,22 +60,28 @@ export async function writeUpdates( for (const branch of branches) { const { baseBranch, branchName } = branch; + const branchSha = getBranchCommit(branchName)!; const meta: Record<string, string> = { branch: branchName }; if (config.baseBranches?.length && baseBranch) { meta['baseBranch'] = baseBranch; } addMeta(meta); const branchExisted = branchExists(branchName); - let branchCache = {} as BranchCache; - if (branchExisted && config.repositoryCache === 'enabled') { - branchCache = - cachedBranches?.find((br) => br.branchName === branchName) ?? - ({} as BranchCache); + let branchState = cachedBranches.find((br) => br.branchName === branchName); - if (Object.keys(branchCache).length === 0) { - logger.debug(`No branch cache found for ${branch.branchName}`); + if (!branchState) { + if (branchExisted && config.repositoryCache === 'enabled') { + logger.debug(`No branch cache found for ${branchName}`); } + + // create a minimal branch state + branchState = { + branchName, + sha: branchSha, + } as BranchCache; + cachedBranches.push(branchState); } + const branchManagersFingerprint = hasha( [ ...new Set( @@ -89,7 +96,7 @@ export async function writeUpdates( branchManagersFingerprint, ]); branch.skipBranchUpdate = canSkipBranchUpdateCheck( - branchCache, + branchState, branchFingerprint ); const res = await processBranch(branch); @@ -97,9 +104,9 @@ export async function writeUpdates( branch.prNo = res?.prNo; branch.result = res?.result; branch.branchFingerprint = - res?.commitSha || !branchCache.branchFingerprint + res?.commitSha || !branchState.branchFingerprint ? branchFingerprint - : branchCache.branchFingerprint; + : branchState.branchFingerprint; if ( branch.result === BranchResult.Automerged &&