Skip to content

Commit

Permalink
feat: use getBranchParentSha cached result (#16724)
Browse files Browse the repository at this point in the history
  • Loading branch information
RahulGautamSingh committed Jul 24, 2022
1 parent a3c3f11 commit 9778ca6
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 4 deletions.
12 changes: 10 additions & 2 deletions lib/util/git/index.spec.ts
Expand Up @@ -12,18 +12,20 @@ import type { BranchCache } from '../cache/repository/types';
import { newlineRegex, regEx } from '../regex';
import * as _conflictsCache from './conflicts-cache';
import * as _modifiedCache from './modified-cache';
import * as _parentShaCache from './parent-sha-cache';
import type { FileChange } from './types';
import * as git from '.';
import { setNoVerify } from '.';

jest.mock('./conflicts-cache');
jest.mock('./modified-cache');
jest.mock('./parent-sha-cache');
jest.mock('delay');
jest.mock('../cache/repository');
const repoCache = mocked(_repoCache);
const conflictsCache = mocked(_conflictsCache);
const modifiedCache = mocked(_modifiedCache);

const parentShaCache = mocked(_parentShaCache);
// Class is no longer exported
const SimpleGit = Git().constructor as { prototype: ReturnType<typeof Git> };

Expand Down Expand Up @@ -114,6 +116,7 @@ describe('util/git/index', () => {
// override some local git settings for better testing
const local = Git(tmpDir.path);
await local.addConfig('commit.gpgsign', 'false');
parentShaCache.getCachedBranchParentShaResult.mockReturnValue(null);
});

afterEach(async () => {
Expand Down Expand Up @@ -319,9 +322,14 @@ describe('util/git/index', () => {
expect(parentSha).toEqual(git.getBranchCommit(defaultBranch));
});

it('should return false if not found', async () => {
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)', () => {
Expand Down
10 changes: 8 additions & 2 deletions lib/util/git/index.ts
Expand Up @@ -39,6 +39,7 @@ import {
getCachedModifiedResult,
setCachedModifiedResult,
} from './modified-cache';
import { getCachedBranchParentShaResult } from './parent-sha-cache';
import { configSigningKey, writePrivateKey } from './private-key';
import type {
CommitFilesConfig,
Expand Down Expand Up @@ -467,9 +468,14 @@ export function getBranchCommit(branchName: string): CommitSha | null {
export async function getBranchParentSha(
branchName: string
): Promise<CommitSha | null> {
const branchSha = getBranchCommit(branchName);
let parentSha = getCachedBranchParentShaResult(branchName, branchSha);
if (parentSha !== null) {
return parentSha;
}

try {
const branchSha = getBranchCommit(branchName);
const parentSha = await git.revparse([`${branchSha}^`]);
parentSha = await git.revparse([`${branchSha}^`]);
return parentSha;
} catch (err) {
logger.debug({ err }, 'Error getting branch parent sha');
Expand Down
71 changes: 71 additions & 0 deletions lib/util/git/parent-sha-cache.spec.ts
@@ -0,0 +1,71 @@
import { mocked } from '../../../test/util';
import * as _repositoryCache from '../cache/repository';
import type { BranchCache, RepoCacheData } from '../cache/repository/types';
import { getCachedBranchParentShaResult } from './parent-sha-cache';

jest.mock('../cache/repository');
const repositoryCache = mocked(_repositoryCache);

describe('util/git/parent-sha-cache', () => {
let repoCache: RepoCacheData = {};

beforeEach(() => {
repoCache = {};
repositoryCache.getCache.mockReturnValue(repoCache);
});

describe('getCachedBranchParentShaResult', () => {
it('returns null if cache is not populated', () => {
expect(getCachedBranchParentShaResult('foo', '111')).toBeNull();
});

it('returns null if target key not found', () => {
repositoryCache.getCache.mockReturnValue({
branches: [
{
branchName: 'not_foo',
sha: '111',
} as BranchCache,
],
});
expect(getCachedBranchParentShaResult('foo', '111')).toBeNull();
});

it('returns null if target key is null', () => {
repositoryCache.getCache.mockReturnValue({
branches: [
{
branchName: 'not_foo',
sha: null,
} as BranchCache,
],
});
expect(getCachedBranchParentShaResult('foo', '111')).toBeNull();
});

it('returns null if target SHA has changed', () => {
repositoryCache.getCache.mockReturnValue({
branches: [
{
branchName: 'foo',
sha: '222',
} as BranchCache,
],
});
expect(getCachedBranchParentShaResult('foo', '111')).toBeNull();
});

it('returns cached value', () => {
repositoryCache.getCache.mockReturnValue({
branches: [
{
branchName: 'foo',
sha: '111',
parentSha: '000',
} as BranchCache,
],
});
expect(getCachedBranchParentShaResult('foo', '111')).toBe('000');
});
});
});
15 changes: 15 additions & 0 deletions lib/util/git/parent-sha-cache.ts
@@ -0,0 +1,15 @@
import { getCache } from '../cache/repository';

export function getCachedBranchParentShaResult(
branchName: string,
branchSha: string | null
): string | null {
const { branches } = getCache();
const branch = branches?.find((branch) => branch.branchName === branchName);

if (branchSha === branch?.sha) {
return branch.parentSha;
}

return null;
}

0 comments on commit 9778ca6

Please sign in to comment.