refactor: improve branch cache logic

lib/util/cache/repository/types.ts

@@ -29,10 +29,13 @@ export interface BranchUpgradeCache {
 export interface BranchCache {
   automerge: boolean;
   branchName: string;
-  isModified: boolean;
+  isModified: boolean | null;
+  isConflicted: boolean | null;
   prNo: number | null;
   sha: string | null;
-  parentSha: string | null;
+  baseBranchName: string;
+  baseBranchSha: string | null; // recent baseBranch SHA
+  parentSha: string | null; // baseBranch SHA when branch-commit was made by bot
   upgrades: BranchUpgradeCache[];
   branchFingerprint?: string;
 }

lib/util/git/behind-base-branch-cache.ts

@@ -1,10 +1,10 @@
 import { getCache } from '../cache/repository';
 
-// Compare cached parent Sha of a branch to the fetched base-branch sha to determine whether the branch is behind the base
+// Compare cached parentSha of a branch to the fetched base-branch sha to determine whether the branch is behind the base
 // Since cache is updated after each run, this will be sufficient to determine whether a branch is behind its parent.
 export function getCachedBehindBaseResult(
   branchName: string,
-  currentBaseBranchSha: string
+  baseBranchSha: string
 ): boolean | null {
   const cache = getCache();
   const { branches = [] } = cache;
@@ -16,5 +16,5 @@ export function getCachedBehindBaseResult(
     return null;
   }
 
-  return currentBaseBranchSha !== cachedBranch.parentSha;
+  return baseBranchSha !== cachedBranch.parentSha;
 }

lib/util/git/conflicts-cache.spec.ts

@@ -1,6 +1,6 @@
-import { mocked } from '../../../test/util';
+import { mocked, partial } from '../../../test/util';
 import * as _repositoryCache from '../cache/repository';
-import type { RepoCacheData } from '../cache/repository/types';
+import type { BranchCache, RepoCacheData } from '../cache/repository/types';
 import {
   getCachedConflictResult,
   setCachedConflictResult,
@@ -22,119 +22,172 @@ describe('util/git/conflicts-cache', () => {
       expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeNull();
     });
 
-    it('returns null if target key not found', () => {
-      expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeNull();
+    it('returns null if target branch name mismatch', () => {
+      repoCache.branches = [
+        partial<BranchCache>({
+          baseBranchName: 'foo',
+          branchName: 'aaa',
+          sha: '444',
+          baseBranchSha: '121',
+          isConflicted: true,
+        }),
+      ];
+      expect(
+        getCachedConflictResult('not_foo', '111', 'bar', '222')
+      ).toBeNull();
     });
 
     it('returns null if target SHA has changed', () => {
-      repoCache.gitConflicts = {
-        foo: { targetBranchSha: 'aaa', sourceBranches: {} },
-      };
-      expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeNull();
+      repoCache.branches = [
+        partial<BranchCache>({
+          baseBranchName: 'foo',
+          branchName: 'aaa',
+          sha: '444',
+          baseBranchSha: '121',
+          isConflicted: true,
+        }),
+      ];
+      expect(getCachedConflictResult('foo', '111', 'aaa', '444')).toBeNull();
     });
 
-    it('returns null if source key not found', () => {
-      repoCache.gitConflicts = {
-        foo: { targetBranchSha: '111', sourceBranches: {} },
-      };
+    it('returns null if source branch not found', () => {
+      repoCache.branches = [
+        partial<BranchCache>({
+          baseBranchName: 'foo',
+          baseBranchSha: '121',
+          isConflicted: true,
+        }),
+      ];
       expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeNull();
     });
 
-    it('returns null if source key has changed', () => {
-      repoCache.gitConflicts = {
-        foo: {
-          targetBranchSha: '111',
-          sourceBranches: {
-            bar: { sourceBranchSha: 'bbb', isConflicted: true },
-          },
-        },
-      };
+    it('returns null if source branch sha has changed', () => {
+      repoCache.branches = [
+        partial<BranchCache>({
+          baseBranchName: 'foo',
+          branchName: 'bar',
+          sha: '212',
+          baseBranchSha: '111',
+          isConflicted: true,
+        }),
+      ];
       expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeNull();
     });
 
     it('returns true', () => {
-      repoCache.gitConflicts = {
-        foo: {
-          targetBranchSha: '111',
-          sourceBranches: {
-            bar: { sourceBranchSha: '222', isConflicted: true },
-          },
-        },
-      };
+      repoCache.branches = [
+        partial<BranchCache>({
+          baseBranchName: 'foo',
+          branchName: 'bar',
+          sha: '222',
+          baseBranchSha: '111',
+          isConflicted: true,
+        }),
+      ];
       expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeTrue();
     });
 
     it('returns false', () => {
+      repoCache.branches = [
+        partial<BranchCache>({
+          baseBranchName: 'foo',
+          branchName: 'bar',
+          sha: '222',
+          baseBranchSha: '111',
+          isConflicted: false,
+        }),
+      ];
+      expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeFalse();
+    });
+
+    it('returns null and removes gitConflicts', () => {
+      repoCache.branches = [
+        partial<BranchCache>({
+          baseBranchName: 'foo',
+          branchName: 'bar',
+          sha: '222',
+          baseBranchSha: '111',
+          isConflicted: false,
+        }),
+      ];
       repoCache.gitConflicts = {
-        foo: {
-          targetBranchSha: '111',
+        targetBranchName: {
+          targetBranchSha: 'target_sha',
           sourceBranches: {
-            bar: { sourceBranchSha: '222', isConflicted: false },
+            sourceBranchName: {
+              sourceBranchSha: 'source_sha',
+              isConflicted: false,
+            },
           },
         },
       };
       expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeFalse();
+      expect(repoCache.gitConflicts).toBeUndefined();
     });
   });
 
   describe('setCachedConflictResult', () => {
     it('sets value for unpopulated cache', () => {
       setCachedConflictResult('foo', '111', 'bar', '222', true);
       expect(repoCache).toEqual({
-        gitConflicts: {
-          foo: {
-            targetBranchSha: '111',
-            sourceBranches: {
-              bar: { sourceBranchSha: '222', isConflicted: true },
-            },
+        branches: [
+          {
+            baseBranchName: 'foo',
+            branchName: 'bar',
+            sha: '222',
+            baseBranchSha: '111',
+            isConflicted: true,
           },
-        },
+        ],
       });
     });
 
-    it('replaces value when source SHA has changed', () => {
-      setCachedConflictResult('foo', '111', 'bar', '222', false);
+    it('replaces value when branch SHA has changed', () => {
+      setCachedConflictResult('foo', '101', 'bar', '222', false);
       setCachedConflictResult('foo', '111', 'bar', '333', false);
-      setCachedConflictResult('foo', '111', 'bar', '444', true);
+      setCachedConflictResult('foo', '121', 'bar', '444', true);
       expect(repoCache).toEqual({
-        gitConflicts: {
-          foo: {
-            targetBranchSha: '111',
-            sourceBranches: {
-              bar: { sourceBranchSha: '444', isConflicted: true },
-            },
+        branches: [
+          {
+            baseBranchName: 'foo',
+            branchName: 'bar',
+            sha: '444',
+            baseBranchSha: '121',
+            isConflicted: true,
           },
-        },
+        ],
       });
     });
 
-    it('replaces value when target SHA has changed', () => {
+    it('replaces value when target branch SHA has changed', () => {
       setCachedConflictResult('foo', '111', 'bar', '222', false);
       setCachedConflictResult('foo', 'aaa', 'bar', '222', true);
       expect(repoCache).toEqual({
-        gitConflicts: {
-          foo: {
-            targetBranchSha: 'aaa',
-            sourceBranches: {
-              bar: { sourceBranchSha: '222', isConflicted: true },
-            },
+        branches: [
+          {
+            baseBranchName: 'foo',
+            branchName: 'bar',
+            sha: '222',
+            baseBranchSha: 'aaa',
+            isConflicted: true,
           },
-        },
+        ],
       });
     });
 
     it('replaces value when both target and source SHA have changed', () => {
       setCachedConflictResult('foo', '111', 'bar', '222', true);
       setCachedConflictResult('foo', 'aaa', 'bar', 'bbb', false);
       expect(repoCache).toEqual({
-        gitConflicts: {
-          foo: {
-            targetBranchSha: 'aaa',
-            sourceBranches: {
-              bar: { sourceBranchSha: 'bbb', isConflicted: false },
-            },
+        branches: [
+          {
+            baseBranchName: 'foo',
+            branchName: 'bar',
+            sha: 'bbb',
+            baseBranchSha: 'aaa',
+            isConflicted: false,
           },
-        },
+        ],
       });
     });
   });

lib/util/git/conflicts-cache.ts

@@ -1,28 +1,30 @@
 import { getCache } from '../cache/repository';
+import type { BranchCache } from '../cache/repository/types';
 
 export function getCachedConflictResult(
   targetBranchName: string,
   targetBranchSha: string,
   sourceBranchName: string,
   sourceBranchSha: string
 ): boolean | null {
-  const { gitConflicts } = getCache();
-  if (!gitConflicts) {
-    return null;
-  }
-
-  const targetBranchConflicts = gitConflicts[targetBranchName];
-  if (targetBranchConflicts?.targetBranchSha !== targetBranchSha) {
-    return null;
+  const cache = getCache();
+  if (cache.gitConflicts) {
+    delete cache.gitConflicts;
   }
+  cache.branches ??= [];
+  const branch = cache.branches.find(
+    (br) => br.branchName === sourceBranchName
+  );
 
-  const sourceBranchConflict =
-    targetBranchConflicts.sourceBranches[sourceBranchName];
-  if (sourceBranchConflict?.sourceBranchSha !== sourceBranchSha) {
-    return null;
+  if (
+    branch?.baseBranchName === targetBranchName &&
+    branch?.baseBranchSha === targetBranchSha &&
+    branch?.sha === sourceBranchSha
+  ) {
+    return branch.isConflicted;
   }
 
-  return sourceBranchConflict.isConflicted;
+  return null;
 }
 
 export function setCachedConflictResult(
@@ -33,24 +35,24 @@ export function setCachedConflictResult(
   isConflicted: boolean
 ): void {
   const cache = getCache();
-  cache.gitConflicts ??= {};
-  const { gitConflicts } = cache;
-
-  let targetBranchConflicts = gitConflicts[targetBranchName];
-  if (targetBranchConflicts?.targetBranchSha !== targetBranchSha) {
-    gitConflicts[targetBranchName] = {
-      targetBranchSha,
-      sourceBranches: {},
-    };
-    targetBranchConflicts = gitConflicts[targetBranchName];
+  cache.branches ??= [];
+  let branch = cache.branches.find((br) => br.branchName === sourceBranchName);
+
+  if (!branch) {
+    branch = {
+      branchName: sourceBranchName,
+      baseBranchName: targetBranchName,
+    } as BranchCache;
+    cache.branches?.push(branch);
   }
 
-  const sourceBranchConflict =
-    targetBranchConflicts.sourceBranches[sourceBranchName];
-  if (sourceBranchConflict?.sourceBranchSha !== sourceBranchSha) {
-    targetBranchConflicts.sourceBranches[sourceBranchName] = {
-      sourceBranchSha,
-      isConflicted,
-    };
+  if (branch.baseBranchSha !== targetBranchSha) {
+    branch.baseBranchSha = targetBranchSha;
   }
+
+  if (branch.sha !== sourceBranchSha) {
+    branch.sha = sourceBranchSha;
+  }
+
+  branch.isConflicted = isConflicted;
 }

lib/util/git/index.spec.ts

@@ -315,23 +315,6 @@ describe('util/git/index', () => {
     });
   });
 
-  describe('getBranchParentSha(branchName)', () => {
-    it('should return sha if found', async () => {
-      const parentSha = await git.getBranchParentSha('renovate/future_branch');
-      expect(parentSha).toHaveLength(40);
-      expect(parentSha).toEqual(git.getBranchCommit(defaultBranch));
-    });
-
-    it('should return null if not found', async () => {
-      expect(await git.getBranchParentSha('not_found')).toBeNull();
-    });
-
-    it('should return cached value', async () => {
-      parentShaCache.getCachedBranchParentShaResult.mockReturnValueOnce('111');
-      expect(await git.getBranchParentSha('not_found')).toBe('111');
-    });
-  });
-
   describe('getBranchFiles(branchName)', () => {
     it('detects changed files compared to current base branch', async () => {
       const file: FileChange = {