Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix issue with stale cache entries #2673

Merged
merged 3 commits into from
Apr 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/cspell-gitignore/src/GitIgnore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,11 @@ export class GitIgnore {
const parent = path.dirname(directory);
const parentHierarchy =
parent !== directory && contains(root, parent) ? await this.findGitIgnoreHierarchy(parent) : undefined;
const gif = await loadGitIgnore(directory);
if (!gif) {
const git = await loadGitIgnore(directory);
if (!git) {
return parentHierarchy || new GitIgnoreHierarchy([]);
}
const chain = parentHierarchy?.gitIgnoreChain.concat([gif]) ?? [gif];
const chain = parentHierarchy?.gitIgnoreChain.concat([git]) ?? [git];
return new GitIgnoreHierarchy(chain);
}

Expand Down
8 changes: 4 additions & 4 deletions packages/cspell-gitignore/src/GitIgnoreFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ export class GitIgnoreHierarchy {
}

isIgnored(file: string): boolean {
for (const gif of this.gitIgnoreChain) {
if (gif.isIgnored(file)) return true;
for (const git of this.gitIgnoreChain) {
if (git.isIgnored(file)) return true;
}

return false;
Expand All @@ -71,8 +71,8 @@ export class GitIgnoreHierarchy {
* @returns IsIgnoredExResult of the match or undefined if there was no match.
*/
isIgnoredEx(file: string): IsIgnoredExResult | undefined {
for (const gif of this.gitIgnoreChain) {
const r = gif.isIgnoredEx(file);
for (const git of this.gitIgnoreChain) {
const r = git.isIgnoredEx(file);
if (r.matched) return r;
}

Expand Down
8 changes: 5 additions & 3 deletions packages/cspell/src/util/cache/DiskCache.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import * as FileEntryCacheModule from 'file-entry-cache';
import * as path from 'path';
import * as fileHelper from '../../util/fileHelper';
import { CachedFileResult, DiskCache, CSpellCacheMeta } from './DiskCache';
import { CachedFileResult, DiskCache, CSpellCacheMeta, __testing__ } from './DiskCache';

const { calcVersion } = __testing__;

jest.mock('./getConfigHash', () => ({
getConfigHash: jest.fn().mockReturnValue('TEST_CONFIG_HASH'),
Expand Down Expand Up @@ -172,8 +174,8 @@ function entry(result: CachedFileResult, dependencies: string[] = [], size = 100
size,
data: {
r: result,
d: dependencies,
v: 'version',
d: dependencies.map((f) => ({ f, h: 'hash' })),
v: calcVersion('version'),
},
},
};
Expand Down
191 changes: 164 additions & 27 deletions packages/cspell/src/util/cache/DiskCache.ts
Original file line number Diff line number Diff line change
@@ -1,41 +1,79 @@
import * as crypto from 'crypto';
import type { FileDescriptor, FileEntryCache } from 'file-entry-cache';
import * as fileEntryCache from 'file-entry-cache';
import * as fs from 'fs';
import { resolve as resolvePath } from 'path';
import type { FileResult } from '../../util/fileHelper';
import { readFileInfo } from '../../util/fileHelper';
import type { CSpellLintResultCache } from './CSpellLintResultCache';
import { ShallowObjectCollection } from './ObjectCollection';

export type CachedFileResult = Omit<FileResult, 'fileInfo' | 'elapsedTimeMs'>;
export type CachedFileResult = Omit<FileResult, 'fileInfo' | 'elapsedTimeMs' | 'cached'>;

/**
* This is the data cached.
* Property names are short to help keep the cache file size small.
*/
interface CachedData {
/** meta version + suffix */
v?: string;
/** results */
r: CachedFileResult;
r?: CachedFileResult;
/** dependencies */
d: string[];
/** meta version */
v: string;
d?: Dependency[];
}

interface Dependency {
/** filename */
f: string;
/** hash of file contents */
h?: string | undefined;
}

interface CSpellCachedMetaData {
data?: CachedData;
}

export type CSpellCacheMeta = (FileDescriptor['meta'] & CSpellCachedMetaData) | undefined;
type Meta = FileDescriptor['meta'];

export type CSpellCacheMeta = (Meta & CSpellCachedMetaData) | undefined;

type CacheDataKeys = {
[K in keyof Required<CachedData>]: K;
};

const cacheDataKeys: CacheDataKeys = {
v: 'v',
r: 'r',
d: 'd',
};

/**
* Meta Data Version is used to detect if the structure of the meta data has changed.
* This is used in combination with the Suffix and the version of CSpell.
*/
const META_DATA_BASE_VERSION = '1';
const META_DATA_VERSION_SUFFIX = '-' + META_DATA_BASE_VERSION + '-' + Object.keys(cacheDataKeys).join('|');

interface DependencyCacheTree {
d?: Dependency[];
c?: Map<string, DependencyCacheTree>;
}

/**
* Caches cspell results on disk
*/
export class DiskCache implements CSpellLintResultCache {
private fileEntryCache: FileEntryCache;
private changedDependencies: Set<string> = new Set();
private knownDependencies: Set<string> = new Set();
private dependencyCache: Map<string, Dependency> = new Map();
private dependencyCacheTree: DependencyCacheTree = {};
private objectCollection = new ShallowObjectCollection<CachedData>();
private ocCacheFileResult = new ShallowObjectCollection<CachedFileResult>();
readonly version: string;

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

public async getCachedLintResults(filename: string): Promise<FileResult | undefined> {
Expand All @@ -61,6 +99,14 @@ export class DiskCache implements CSpellLintResultCache {
return undefined;
}

const dd = { ...data };

if (dd.d) {
dd.d = setTreeEntry(this.dependencyCacheTree, dd.d);
}
dd.r = dd.r && this.normalizeResult(dd.r);
meta.data = this.objectCollection.get(dd);

// Skip reading empty files and files without lint error
const hasErrors = !!result && (result.errors > 0 || result.configErrors > 0 || result.issues.length > 0);
const cached = true;
Expand All @@ -75,7 +121,7 @@ export class DiskCache implements CSpellLintResultCache {
}

public setCachedLintResults(
{ fileInfo, elapsedTimeMs: _, ...result }: FileResult,
{ fileInfo, elapsedTimeMs: _, cached: __, ...result }: FileResult,
dependsUponFiles: string[]
): void {
const fileDescriptor = this.fileEntryCache.getFileDescriptor(fileInfo.filename);
Expand All @@ -84,37 +130,128 @@ export class DiskCache implements CSpellLintResultCache {
return;
}

const data: CachedData = {
r: result,
d: dependsUponFiles,
const data: CachedData = this.objectCollection.get({
v: this.version,
};
r: this.normalizeResult(result),
d: this.calcDependencyHashes(dependsUponFiles),
});

meta.data = data;
this.cacheDependencies(dependsUponFiles);
}

public reconcile(): void {
this.fileEntryCache.reconcile();
}

private cacheDependencies(files: string[]) {
this.fileEntryCache.analyzeFiles(files);
private normalizeResult(result: CachedFileResult): CachedFileResult {
const { issues, processed, errors, configErrors, ...rest } = result;
if (!Object.keys(rest).length) {
return this.ocCacheFileResult.get(result);
}
return this.ocCacheFileResult.get({ issues, processed, errors, configErrors });
}

private calcDependencyHashes(dependsUponFiles: string[]): Dependency[] {
dependsUponFiles.sort();

const c = getTreeEntry(this.dependencyCacheTree, dependsUponFiles);
if (c?.d) {
return c.d;
}

const dependencies: Dependency[] = dependsUponFiles.map((f) => this.getDependency(f));

return setTreeEntry(this.dependencyCacheTree, dependencies);
}

private checkDependency(dep: Dependency): boolean {
const cDep = this.dependencyCache.get(dep.f);

if (cDep && compDep(dep, cDep)) return true;
if (cDep) return false;

const d = this.getFileDep(dep.f);
if (compDep(dep, d)) {
this.dependencyCache.set(dep.f, dep);
return true;
}
this.dependencyCache.set(d.f, d);
return false;
}

private getDependency(file: string): Dependency {
const dep = this.dependencyCache.get(file);
if (dep) return dep;
const d = this.getFileDep(file);
this.dependencyCache.set(file, d);
return d;
}

private getFileDep(file: string): Dependency {
let h: string;
try {
const buffer = fs.readFileSync(file);
h = this.getHash(buffer);
} catch (e) {
return { f: file };
}
return { f: file, h };
}

private checkDependencies(files: string[]): boolean {
for (const file of files) {
if (this.changedDependencies.has(file)) {
private checkDependencies(dependencies: Dependency[] | undefined): boolean {
if (!dependencies) return false;
for (const dep of dependencies) {
if (!this.checkDependency(dep)) {
return false;
}
}
const unknown = files.filter((f) => !this.knownDependencies.has(f));
if (!unknown.length) {
return true;
return true;
}

private getHash(buffer: Buffer): string {
return crypto.createHash('md5').update(buffer).digest('hex');
}
}

function getTreeEntry(tree: DependencyCacheTree, keys: string[]): DependencyCacheTree | undefined {
let r: DependencyCacheTree | undefined = tree;
for (const k of keys) {
r = r.c?.get(k);
if (!r) return r;
}
return r;
}

function setTreeEntry(tree: DependencyCacheTree, deps: Dependency[], update = false): Dependency[] {
let r = tree;
for (const d of deps) {
const k = d.f;
if (!r.c) {
r.c = new Map();
}
const cn = r.c.get(k);
const n = cn ?? {};
if (!cn) {
r.c.set(k, n);
}
const { changedFiles, notFoundFiles } = this.fileEntryCache.analyzeFiles(files);
changedFiles.map((f) => this.changedDependencies.add(f));
unknown.forEach((f) => this.knownDependencies.add(f));
return changedFiles.length === 0 && notFoundFiles.length === 0;
r = n;
}
let d = r.d;
if (!d || (r.d && update)) {
r.d = deps;
d = deps;
}
return d;
}

function compDep(a: Dependency, b: Dependency) {
return a.f === b.f && a.h === b.h;
}

function calcVersion(version: string): string {
return version + META_DATA_VERSION_SUFFIX;
}

export const __testing__ = {
calcVersion,
};
57 changes: 57 additions & 0 deletions packages/cspell/src/util/cache/ObjectCollection.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { ShallowObjectCollection, Collection } from './ObjectCollection';

const objects = (function () {
const empty = {};
const a = { name: 'a' };
const b = { name: 'b' };
const bb = { name: 'b', value: 42 };
const c = { kind: 'c', name: 'a' };
const n = { kind: 'c', a };
const ab = { a: a, b: b };
const arr = ['hello', a, b, c];
const cir = { values: [] };
(cir.values as unknown[]).push(cir);
return { empty, a, b, bb, c, n, ab, arr, cir };
})();

describe('ObjectCollection', () => {
const { empty, a, b, bb, c, n, ab } = objects;
test.each`
objects | expected
${[a, b, { ...a }, bb, c, n, empty, {}]} | ${[a, b, a, bb, c, n, empty, empty]}
${['', 0, a, 42, empty, c, 'hello', {}, { ...c }]} | ${['', 0, a, 42, empty, c, 'hello', empty, c]}
${[a, b, n, ab, { a: { ...a }, b: { ...b } }, { kind: 'c', a: { ...a } }]} | ${[a, b, n, ab, ab, n]}
`('ShallowObjectCollection $objects', ({ objects, expected }: { objects: object[]; expected: unknown[] }) => {
const c = new ShallowObjectCollection();
const r = objects.map((v) => c.get(v));
expect(r).toEqual(expected);
});
});

describe('Collection', () => {
const { empty, a, b, bb, c, n, ab, arr, cir } = objects;
const sym = Symbol('test');
const f = Object.freeze({ ...a });
const nestedCircular = { cir };

test.each`
objects | expected | comment
${[a, b, { ...a }, bb, c, n, empty, {}]} | ${[a, b, a, bb, c, n, empty, empty]} | ${''}
${['', 0, a, 42, empty, c, 'hello', {}, { ...c }]} | ${['', 0, a, 42, empty, c, 'hello', empty, c]} | ${''}
${[a, b, n, ab, { a: { ...a }, b: { ...b } }, { kind: 'c', a: { ...a } }]} | ${[a, b, n, ab, ab, n]} | ${'Nested objects match objects in collection'}
${[a, { ...a, b: undefined }]} | ${[a, a]} | ${'Undefined values are removed.'}
${[describe, undefined, null, 0, sym, sym]} | ${[describe, undefined, null, 0, sym, sym]} | ${'random things'}
${[arr, ['hello', a, b, c]]} | ${[arr, arr]} | ${'arrays'}
${[cir, { values: [cir] }]} | ${[cir, cir]} | ${'circular'}
${[nestedCircular, { cir }]} | ${[nestedCircular, nestedCircular]} | ${'nested circular'}
${[a, f]} | ${[a, a]} | ${'first object wins'}
${[f, a]} | ${[f, f]} | ${'reverse frozen objects'}
`('Collection $comment $objects', ({ objects, expected }: { objects: object[]; expected: unknown[] }) => {
const c = new Collection();
const r = objects.map((v) => c.add(v));
expect(r).toEqual(expected);
for (let i = 0; i < r.length; ++i) {
expect(r[i]).toBe(expected[i]);
}
});
});
Loading