Skip to content

Commit

Permalink
fix: Invalidate the cache if cspell version has changed. (#2580)
Browse files Browse the repository at this point in the history
* fix: Invalidate the cache if cspell version has changed.

* dev: Do not output the progress twice

* Revert "dev: Do not output the progress twice"

This reverts commit 619ab40.
  • Loading branch information
Jason3S committed Mar 14, 2022
1 parent 1a461f6 commit 2174928
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 19 deletions.
3 changes: 1 addition & 2 deletions packages/cspell/src/app.ts
@@ -1,6 +1,5 @@
import type { Command } from 'commander';
import { program } from 'commander';
import * as path from 'path';
import { satisfies as semverSatisfies } from 'semver';
import { commandCheck } from './commandCheck';
import { commandLink } from './commandLink';
Expand All @@ -9,7 +8,7 @@ import { commandSuggestion } from './commandSuggestion';
import { commandTrace } from './commandTrace';
import { ApplicationError } from './util/errors';
// eslint-disable-next-line @typescript-eslint/no-var-requires
const npmPackage = require(path.join(__dirname, '..', 'package.json'));
const npmPackage = require('../package.json');

export { LinterCliOptions as Options } from './options';
export { CheckFailed } from './util/errors';
Expand Down
5 changes: 4 additions & 1 deletion packages/cspell/src/lint/lint.ts
Expand Up @@ -28,6 +28,9 @@ import { getTimeMeasurer } from '../util/timer';
import * as util from '../util/util';
import { LintRequest } from './LintRequest';
import chalk = require('chalk');
// eslint-disable-next-line @typescript-eslint/no-var-requires
const npmPackage = require('../../package.json');
const version = npmPackage.version;

export async function runLint(cfg: LintRequest): Promise<RunResult> {
let { reporter } = cfg;
Expand Down Expand Up @@ -253,7 +256,7 @@ export async function runLint(cfg: LintRequest): Promise<RunResult> {
const { root } = cfg;

try {
const cacheSettings = await calcCacheSettings(configInfo.config, cfg.options, root);
const cacheSettings = await calcCacheSettings(configInfo.config, { ...cfg.options, version }, root);
const files = await determineFilesToCheck(configInfo, cfg, reporter, globInfo);

return await processFiles(files, configInfo, cacheSettings);
Expand Down
2 changes: 1 addition & 1 deletion packages/cspell/src/options.ts
@@ -1,6 +1,6 @@
import type { CacheOptions } from './util/cache';

export interface LinterOptions extends BaseOptions, CacheOptions {
export interface LinterOptions extends BaseOptions, Omit<CacheOptions, 'version'> {
/**
* Display verbose information
*/
Expand Down
5 changes: 5 additions & 0 deletions packages/cspell/src/util/cache/CacheOptions.ts
@@ -1,6 +1,11 @@
import type { CacheStrategy } from '@cspell/cspell-types';

export interface CacheOptions {
/**
* The version of `cspell` that made the cache entry.
* Cache entries must match the `major.minor` version.
*/
version: string;
/**
* Store the info about processed files in order to only operate on the changed ones.
*/
Expand Down
3 changes: 2 additions & 1 deletion packages/cspell/src/util/cache/DiskCache.test.ts
Expand Up @@ -39,7 +39,7 @@ describe('DiskCache', () => {
};

beforeEach(() => {
diskCache = new DiskCache('.foobar', false);
diskCache = new DiskCache('.foobar', false, 'version');
fileEntryCache = mockCreateFileEntryCache.mock.results[0].value;
});

Expand Down Expand Up @@ -173,6 +173,7 @@ function entry(result: CachedFileResult, dependencies: string[] = [], size = 100
data: {
r: result,
d: dependencies,
v: 'version',
},
},
};
Expand Down
15 changes: 13 additions & 2 deletions packages/cspell/src/util/cache/DiskCache.ts
Expand Up @@ -16,6 +16,8 @@ interface CachedData {
r: CachedFileResult;
/** dependencies */
d: string[];
/** meta version */
v: string;
}

interface CSpellCachedMetaData {
Expand All @@ -32,7 +34,7 @@ export class DiskCache implements CSpellLintResultCache {
private changedDependencies: Set<string> = new Set();
private knownDependencies: Set<string> = new Set();

constructor(cacheFileLocation: string, useCheckSum: boolean) {
constructor(cacheFileLocation: string, useCheckSum: boolean, readonly version: string) {
this.fileEntryCache = fileEntryCache.createFromFile(resolvePath(cacheFileLocation), useCheckSum);
}

Expand All @@ -41,13 +43,21 @@ export class DiskCache implements CSpellLintResultCache {
const meta = fileDescriptor.meta as CSpellCacheMeta;
const data = meta?.data;
const result = data?.r;
const versionMatches = this.version === data?.v;

// Cached lint results are valid if and only if:
// 1. The file is present in the filesystem
// 2. The file has not changed since the time it was previously linted
// 3. The CSpell configuration has not changed since the time the file was previously linted
// If any of these are not true, we will not reuse the lint results.
if (fileDescriptor.notFound || fileDescriptor.changed || !meta || !result || !this.checkDependencies(data.d)) {
if (
fileDescriptor.notFound ||
fileDescriptor.changed ||
!meta ||
!result ||
!versionMatches ||
!this.checkDependencies(data.d)
) {
return undefined;
}

Expand Down Expand Up @@ -77,6 +87,7 @@ export class DiskCache implements CSpellLintResultCache {
const data: CachedData = {
r: result,
d: dependsUponFiles,
v: this.version,
};

meta.data = data;
Expand Down
39 changes: 32 additions & 7 deletions packages/cspell/src/util/cache/createCache.test.ts
@@ -1,15 +1,17 @@
import { calcCacheSettings, createCache } from './createCache';
import { DiskCache } from './DiskCache';
import { CacheOptions } from './CacheOptions';
import * as path from 'path';
import { resolve as r } from 'path';
import { CreateCacheSettings } from '.';
import { CacheOptions } from './CacheOptions';
import { calcCacheSettings, createCache, __testing__ } from './createCache';
import { DiskCache } from './DiskCache';
import { DummyCache } from './DummyCache';

jest.mock('./DiskCache');

const mockedDiskCache = jest.mocked(DiskCache);

const version = '5.20.0-alpha.192';

const U = undefined;
const T = true;
const F = false;
Expand All @@ -30,8 +32,11 @@ describe('Validate calcCacheSettings', () => {
${{}} | ${{ cacheStrategy: 'content' }} | ${'.'} | ${cco(F, U, 'content')} | ${'override default strategy'}
${{}} | ${co(T, U, 'content')} | ${'.'} | ${cco(T, U, 'content')} | ${'override strategy'}
${cs(U, U, 'content')} | ${co(T, U, 'metadata')} | ${'.'} | ${cco(T, U, 'metadata')} | ${'override config strategy'}
${cs(T, U, 'content')} | ${{}} | ${'.'} | ${cco(T, U, 'content')} | ${'override default strategy'}
${cs(T, U, 'content')} | ${{ version }} | ${'.'} | ${cco(T, U, 'content')} | ${'override default strategy'}
`('calcCacheSettings $comment - $config $options $root', async ({ config, options, root, expected }) => {
if (!options.version) {
options.version = version;
}
expect(await calcCacheSettings({ cache: config }, options, root)).toEqual(expected);
});
});
Expand All @@ -52,6 +57,26 @@ describe('Validate createCache', () => {
});
});

describe('validate normalizeVersion', () => {
test.each`
version | expected
${'5.8'} | ${'5.8'}
${'5.8.30'} | ${'5.8'}
${'5.8.30-alpha.2'} | ${'5.8'}
${'5.28.30-alpha.2'} | ${'5.28'}
${'6.0.30-alpha.2'} | ${'6.0'}
`('normalizeVersion $version', ({ version, expected }) => {
expect(__testing__.normalizeVersion(version)).toBe(expected + __testing__.versionSuffix);
});
test.each`
version
${'5'}
${''}
`('normalizeVersion bad "$version"', ({ version }) => {
expect(() => __testing__.normalizeVersion(version)).toThrow();
});
});

/**
* CreateCacheSettings
*/
Expand All @@ -63,13 +88,13 @@ function cco(
if (cacheLocation) {
cacheLocation = path.resolve(process.cwd(), cacheLocation);
}
return { useCache, cacheLocation, cacheStrategy };
return { useCache, cacheLocation, cacheStrategy, version };
}

function cs(useCache?: boolean, cacheLocation?: string, cacheStrategy?: CacheOptions['cacheStrategy']) {
return { useCache, cacheLocation, cacheStrategy };
return { useCache, cacheLocation, cacheStrategy, version };
}

function co(cache?: boolean, cacheLocation?: string, cacheStrategy?: CacheOptions['cacheStrategy']) {
return { cache, cacheLocation, cacheStrategy };
return { cache, cacheLocation, cacheStrategy, version };
}
35 changes: 30 additions & 5 deletions packages/cspell/src/util/cache/createCache.ts
@@ -1,24 +1,33 @@
import { CacheSettings, CSpellSettings } from '@cspell/cspell-types';
import assert from 'assert';
import { stat } from 'fs-extra';
import path from 'path';
import { CacheOptions } from '.';
import { isError } from '../errors';
import { CSpellLintResultCache } from './CSpellLintResultCache';
import { DiskCache } from './DiskCache';
import { DummyCache } from './DummyCache';
import { stat } from 'fs-extra';
import { isError } from '../errors';

// cspell:word cspellcache
export const DEFAULT_CACHE_LOCATION = '.cspellcache';

export type CreateCacheSettings = Required<CacheSettings>;
export interface CreateCacheSettings extends Required<CacheSettings> {
/**
* cspell version used to validate cache entries.
*/
version: string;
}

const versionSuffix = '';

/**
* Creates CSpellLintResultCache (disk cache if caching is enabled in config or dummy otherwise)
*/

export function createCache(options: CreateCacheSettings): CSpellLintResultCache {
const { useCache, cacheLocation, cacheStrategy } = options;
return useCache ? new DiskCache(path.resolve(cacheLocation), cacheStrategy === 'content') : new DummyCache();
return useCache
? new DiskCache(path.resolve(cacheLocation), cacheStrategy === 'content', normalizeVersion(options.version))
: new DummyCache();
}

export async function calcCacheSettings(
Expand All @@ -37,6 +46,7 @@ export async function calcCacheSettings(
useCache,
cacheLocation,
cacheStrategy,
version: cacheOptions.version,
};
}

Expand All @@ -52,3 +62,18 @@ async function resolveCacheLocation(cacheLocation: string): Promise<string> {
throw err;
}
}

/**
* Normalizes the version and return only `major.minor + versionSuffix`
* @param version The cspell semantic version.
*/
function normalizeVersion(version: string): string {
const parts = version.split('.').slice(0, 2);
assert(parts.length === 2);
return parts.join('.') + versionSuffix;
}

export const __testing__ = {
normalizeVersion,
versionSuffix,
};

0 comments on commit 2174928

Please sign in to comment.