Skip to content

Commit

Permalink
fix(cache): add extract revision, stop deleting skipReason (#33172)
Browse files Browse the repository at this point in the history
  • Loading branch information
rarkins authored Dec 17, 2024
1 parent 3b9464c commit 8ae7448
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 6 deletions.
1 change: 1 addition & 0 deletions lib/util/cache/repository/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type { RepoInitConfig } from '../../../workers/repository/init/types';
import type { PrBlockedBy } from '../../../workers/types';

export interface BaseBranchCache {
revision?: number;
sha: string; // branch commit sha
configHash: string; // object hash of config
extractionFingerprints: Record<string, string | undefined>; // matching manager fingerprints
Expand Down
22 changes: 21 additions & 1 deletion lib/workers/repository/process/extract-update.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ import { fingerprint } from '../../../util/fingerprint';
import type { LongCommitSha } from '../../../util/git/types';
import { generateFingerprintConfig } from '../extract/extract-fingerprint-config';
import * as _branchify from '../updates/branchify';
import { extract, isCacheExtractValid, lookup, update } from './extract-update';
import {
EXTRACT_CACHE_REVISION,
extract,
isCacheExtractValid,
lookup,
update,
} from './extract-update';

const createVulnerabilitiesMock = jest.fn();

Expand Down Expand Up @@ -97,6 +103,7 @@ describe('workers/repository/process/extract-update', () => {
repositoryCache.getCache.mockReturnValueOnce({
scan: {
master: {
revision: EXTRACT_CACHE_REVISION,
sha: '123test',
configHash: fingerprint(generateFingerprintConfig(config)),
extractionFingerprints: {},
Expand Down Expand Up @@ -150,6 +157,7 @@ describe('workers/repository/process/extract-update', () => {

beforeEach(() => {
cachedExtract = {
revision: EXTRACT_CACHE_REVISION,
sha: 'sha',
configHash: undefined as never,
extractionFingerprints: {},
Expand All @@ -162,6 +170,18 @@ describe('workers/repository/process/extract-update', () => {
expect(logger.logger.debug).toHaveBeenCalledTimes(0);
});

it('returns false if no revision', () => {
delete cachedExtract.revision;
expect(isCacheExtractValid('sha', 'hash', cachedExtract)).toBe(false);
expect(logger.logger.debug).toHaveBeenCalledTimes(1);
});

it('returns false if revision mismatch', () => {
cachedExtract.revision = -1;
expect(isCacheExtractValid('sha', 'hash', cachedExtract)).toBe(false);
expect(logger.logger.debug).toHaveBeenCalledTimes(1);
});

it('partial cache', () => {
expect(isCacheExtractValid('sha', 'hash', cachedExtract)).toBe(false);
expect(logger.logger.debug).toHaveBeenCalledTimes(0);
Expand Down
26 changes: 21 additions & 5 deletions lib/workers/repository/process/extract-update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ import { Vulnerabilities } from './vulnerabilities';
import type { WriteUpdateResult } from './write';
import { writeUpdates } from './write';

// Increment this if needing to cache bust ALL extract caches
export const EXTRACT_CACHE_REVISION = 1;

export interface ExtractResult {
branches: BranchConfig[];
branchList: string[];
Expand Down Expand Up @@ -69,7 +72,23 @@ export function isCacheExtractValid(
configHash: string,
cachedExtract?: BaseBranchCache,
): boolean {
if (!(cachedExtract?.sha && cachedExtract.configHash)) {
if (!cachedExtract) {
return false;
}

if (!cachedExtract.revision) {
logger.debug('Cached extract is missing revision, so cannot be used');
return false;
}

if (cachedExtract.revision !== EXTRACT_CACHE_REVISION) {
logger.debug(
`Extract cache revision has changed (old=${cachedExtract.revision}, new=${EXTRACT_CACHE_REVISION})`,
);
return false;
}

if (!(cachedExtract.sha && cachedExtract.configHash)) {
return false;
}
if (cachedExtract.sha !== baseBranchSha) {
Expand Down Expand Up @@ -128,10 +147,6 @@ export async function extract(
for (const file of files) {
for (const dep of file.deps) {
delete dep.updates;
if (dep.skipStage && dep.skipStage !== 'extract') {
delete dep.skipReason;
delete dep.skipStage;
}
}
}
}
Expand All @@ -146,6 +161,7 @@ export async function extract(
const { extractionFingerprints } = extractResult;
// TODO: fix types (#22198)
cache.scan[baseBranch!] = {
revision: EXTRACT_CACHE_REVISION,
sha: baseBranchSha!,
configHash,
extractionFingerprints,
Expand Down

0 comments on commit 8ae7448

Please sign in to comment.