diff --git a/packages/file-entry-cache/src/index.ts b/packages/file-entry-cache/src/index.ts index 200486b7..49376c86 100644 --- a/packages/file-entry-cache/src/index.ts +++ b/packages/file-entry-cache/src/index.ts @@ -128,7 +128,20 @@ export function create( if (cacheDirectory) { const cachePath = `${cacheDirectory}/${cacheId}`; if (fs.existsSync(cachePath)) { - fileEntryCache.cache = createFlatCacheFile(cachePath, opts.cache); + try { + fileEntryCache.cache = createFlatCacheFile(cachePath, opts.cache); + } catch (error) { + // If the cache file content is invalid (e.g. corrupted or non-JSON), + // start with an empty cache. The existing file will be overwritten on + // the next reconcile() rather than throwing. Unexpected errors (e.g. + // IO/permission failures on an otherwise valid file) are re-thrown so + // valid cache data is not silently discarded. + if (error instanceof SyntaxError) { + fileEntryCache.cache = new FlatCache(opts.cache); + } else { + throw error; + } + } } } @@ -150,6 +163,15 @@ export class FileEntryCache { private _logger?: ILogger; private _useAbsolutePathAsKey = false; private _useModifiedTime = true; + /** + * Snapshot of the persisted meta for each key as of the last load/reconcile. + * Change detection compares against this baseline (not the working cache) so + * that repeated `getFileDescriptor()` calls keep reporting a file as changed + * until the cache is reconciled. The set of keys also tracks which files were + * visited during the current session so that `reconcile()` only updates those. + */ + private _originalMeta: Map = + new Map(); /** * Create a new FileEntryCache instance @@ -203,6 +225,9 @@ export class FileEntryCache { */ public set cache(cache: FlatCache) { this._cache = cache; + // The baseline is derived from the cache, so reset it when the cache is + // replaced. It will be re-snapshotted lazily on the next getFileDescriptor. + this._originalMeta = new Map(); } /** @@ -368,6 +393,7 @@ export class FileEntryCache { */ public destroy() { this._cache.destroy(); + this._originalMeta = new Map(); } /** @@ -378,6 +404,7 @@ export class FileEntryCache { public removeEntry(filePath: string): void { const key = this.createFileKey(filePath); this._cache.removeKey(key); + this._originalMeta.delete(key); } /** @@ -385,11 +412,30 @@ export class FileEntryCache { * @method reconcile */ public reconcile(): void { - const { items } = this._cache; - for (const item of items) { - const fileDescriptor = this.getFileDescriptor(item.key); - if (fileDescriptor.notFound) { - this._cache.removeKey(item.key); + // Only reconcile files that were visited via getFileDescriptor() during + // this session. Entries that were never inspected keep their previously + // persisted meta untouched, so they are not silently revalidated. + for (const key of [...this._originalMeta.keys()]) { + const meta = this._cache.getKey(key); + if (!meta) { + // The entry was removed (e.g. file not found) during the session. + this._originalMeta.delete(key); + continue; + } + + try { + // Confirm the file still exists; if it was deleted during the + // session, drop it from the cache below. + fs.statSync(this.getAbsolutePath(key)); + // Persist the meta captured when the file was inspected. It already + // holds a consistent size/mtime/hash snapshot, so we promote it to + // the baseline rather than re-stat'ing (which would refresh + // size/mtime but not hash, leaving the baseline inconsistent). + this._originalMeta.set(key, { ...meta }); + } catch { + // The file no longer exists; drop it from the cache. + this._cache.removeKey(key); + this._originalMeta.delete(key); } } @@ -499,8 +545,22 @@ export class FileEntryCache { }; } - // If the file is not in the cache, add it - if (!metaCache) { + // Snapshot the baseline (the persisted state as of the last load/reconcile) + // the first time this key is seen in the current session. Change detection + // compares against this baseline rather than the working cache so that a + // file reported as changed keeps reporting as changed until reconcile(). + if (!this._originalMeta.has(result.key)) { + this._originalMeta.set( + result.key, + metaCache ? { ...metaCache } : undefined, + ); + } + + const baseline = this._originalMeta.get(result.key); + + // If there is no baseline, the file is new (or has not been reconciled yet) + // and is therefore considered changed. + if (baseline === undefined) { result.changed = true; this._cache.setKey(result.key, result.meta); this._logger?.debug({ filePath }, "File not in cache, marked as changed"); @@ -508,26 +568,26 @@ export class FileEntryCache { } // If the file is in the cache, check if the file has changed - if (useModifiedTimeValue && metaCache?.mtime !== result.meta?.mtime) { + if (useModifiedTimeValue && baseline.mtime !== result.meta?.mtime) { result.changed = true; this._logger?.debug( - { filePath, oldMtime: metaCache.mtime, newMtime: result.meta.mtime }, + { filePath, oldMtime: baseline.mtime, newMtime: result.meta.mtime }, "File changed: mtime differs", ); } - if (metaCache?.size !== result.meta?.size) { + if (baseline.size !== result.meta?.size) { result.changed = true; this._logger?.debug( - { filePath, oldSize: metaCache.size, newSize: result.meta.size }, + { filePath, oldSize: baseline.size, newSize: result.meta.size }, "File changed: size differs", ); } - if (useCheckSumValue && metaCache?.hash !== result.meta?.hash) { + if (useCheckSumValue && baseline.hash !== result.meta?.hash) { result.changed = true; this._logger?.debug( - { filePath, oldHash: metaCache.hash, newHash: result.meta.hash }, + { filePath, oldHash: baseline.hash, newHash: result.meta.hash }, "File changed: hash differs", ); } @@ -733,6 +793,11 @@ export class FileEntryCache { const meta = this._cache.getKey(key); this._cache.removeKey(key); this._cache.setKey(newKey, meta); + // Keep the change-detection baseline aligned with the renamed key. + if (this._originalMeta.has(key)) { + this._originalMeta.set(newKey, this._originalMeta.get(key)); + this._originalMeta.delete(key); + } } } } diff --git a/packages/file-entry-cache/test/index.test.ts b/packages/file-entry-cache/test/index.test.ts index 085c194c..9084d1ec 100644 --- a/packages/file-entry-cache/test/index.test.ts +++ b/packages/file-entry-cache/test/index.test.ts @@ -169,6 +169,9 @@ describe("file-entry-cache with options", () => { logs.some((l) => l.msg === "File not in cache, marked as changed"), ).toBe(true); // 450 + // Reconcile so the file becomes the cached baseline + fileEntryCache.reconcile(); + // Second call - file in cache, unchanged logs.length = 0; descriptor = fileEntryCache.getFileDescriptor(testFile); @@ -250,8 +253,9 @@ describe("file-entry-cache with options", () => { useCheckSum: false, // Disable checksum to use mtime }); - // First call - add file to cache + // First call - add file to cache and reconcile to set the baseline fileEntryCache.getFileDescriptor(testFile); + fileEntryCache.reconcile(); // Wait a bit and modify the file to change mtime await new Promise((resolve) => setTimeout(resolve, 10)); @@ -556,6 +560,9 @@ describe("getFileDescriptor()", () => { expect(fileDescriptor.key).toBe(testFile1); expect(fileDescriptor.changed).toBe(true); + // Reconcile so the file becomes the cached baseline before re-checking + fileEntryCache.reconcile(); + const fileDescriptor2 = fileEntryCache.getFileDescriptor(testFile1); expect(fileDescriptor2).toBeDefined(); expect(fileDescriptor2.key).toBe(testFile1); @@ -599,6 +606,8 @@ describe("getFileDescriptor()", () => { expect(fileDescriptor).toBeDefined(); expect(fileDescriptor.key).toBe(testFile1); expect(fileDescriptor.changed).toBe(true); + // Reconcile so the file becomes the cached baseline before re-checking + fileEntryCache.reconcile(); const fileDescriptor2 = fileEntryCache.getFileDescriptor(testFile1); expect(fileDescriptor2).toBeDefined(); expect(fileDescriptor2.key).toBe(testFile1); @@ -650,6 +659,9 @@ describe("getFileDescriptor()", () => { expect(fileDescriptor2.key).toBe(absPath); expect(fileDescriptor2.changed).toBe(true); + // Reconcile so both keys become cached baselines + fileEntryCache.reconcile(); + // Should be cached separately const fileDescriptor3 = fileEntryCache.getFileDescriptor(relPath); expect(fileDescriptor3.changed).toBe(false); @@ -694,6 +706,9 @@ describe("hasFileChanged()", () => { const fileEntryCache = new FileEntryCache(); const testFile1 = path.resolve("./.cacheHFC/test1.txt"); expect(fileEntryCache.hasFileChanged(testFile1)).toBe(true); + // Repeated calls keep reporting the file as changed until it is reconciled + expect(fileEntryCache.hasFileChanged(testFile1)).toBe(true); + fileEntryCache.reconcile(); expect(fileEntryCache.hasFileChanged(testFile1)).toBe(false); fs.writeFileSync(testFile1, "test4"); expect(fileEntryCache.hasFileChanged(testFile1)).toBe(true); @@ -729,6 +744,8 @@ describe("normalizeEntries()", () => { const file1 = `./${fileCacheName}/test1.txt`; const file2 = `./${fileCacheName}/test2.txt`; fileEntryCache.getFileDescriptor(file2); + // Reconcile so file2 becomes a cached baseline (unchanged on re-check) + fileEntryCache.reconcile(); const entries = fileEntryCache.normalizeEntries([file1, file2]); expect(entries[0].key).toBe(file1); expect(entries[0].changed).toBe(true); @@ -744,6 +761,8 @@ describe("normalizeEntries()", () => { fileEntryCache.getFileDescriptor(`./${fileCacheName}/test1.txt`); fileEntryCache.getFileDescriptor(`./${fileCacheName}/test2.txt`); fileEntryCache.getFileDescriptor(`./${fileCacheName}/test3.txt`); + // Reconcile so the files become cached baselines (unchanged on re-check) + fileEntryCache.reconcile(); fs.chmodSync(path.resolve(`./${fileCacheName}/test3.txt`), 0o000); const entries = fileEntryCache.normalizeEntries(); expect(entries.length).toBe(2); @@ -857,6 +876,10 @@ describe("analyzeFiles()", () => { recursive: true, force: true, }); + fs.rmSync(path.resolve("./.cacheAnalyzeFiles"), { + recursive: true, + force: true, + }); }); test("should analyze files", () => { @@ -895,6 +918,8 @@ describe("analyzeFiles()", () => { const analyzedFiles = fileEntryCache.analyzeFiles(files); expect(analyzedFiles).toBeDefined(); expect(analyzedFiles.changedFiles.length).toBe(4); + // Reconcile so the files become cached baselines (unchanged on re-check) + fileEntryCache.reconcile(); const testFile4 = path.resolve(`./${fileCacheName}/test4.txt`); fs.unlinkSync(testFile4); const analyzedFiles2 = fileEntryCache.analyzeFiles(files); @@ -937,6 +962,8 @@ describe("getUpdatedFiles()", () => { ]; const updatedFiles = fileEntryCache.getUpdatedFiles(files); expect(updatedFiles).toEqual(files); + // Reconcile so the files become cached baselines (no longer updated) + fileEntryCache.reconcile(); const updatedFiles2 = fileEntryCache.getUpdatedFiles(files); expect(updatedFiles2).toEqual([]); }); @@ -953,6 +980,8 @@ describe("getUpdatedFiles()", () => { ]; const updatedFiles = fileEntryCache.getUpdatedFiles(files); expect(updatedFiles).toEqual(files); + // Reconcile so the files become cached baselines before modifying one + fileEntryCache.reconcile(); const testFile4 = path.resolve(`./${fileCacheName}/test4.txt`); fs.writeFileSync(testFile4, "test5booosdkfjsldfkjsldkjfls"); const updatedFiles2 = fileEntryCache.getUpdatedFiles(files); diff --git a/packages/file-entry-cache/test/issue-1648.test.ts b/packages/file-entry-cache/test/issue-1648.test.ts new file mode 100644 index 00000000..b5ff917b --- /dev/null +++ b/packages/file-entry-cache/test/issue-1648.test.ts @@ -0,0 +1,167 @@ +import fs from "node:fs"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import fileEntryCache from "../src/index.js"; + +/** + * Regression tests for https://github.com/jaredwray/cacheable/issues/1648 + * + * These cover three behaviors that differed from file-entry-cache v8 and that + * ESLint relies on: + * 1. reconcile() must only update cache entries for files that were inspected + * via getFileDescriptor(), not every entry tracked in the cache. + * 2. getFileDescriptor() must keep reporting `changed: true` on repeated calls + * for the same file until the cache is reconciled. + * 3. create() must not throw when the cache file content is invalid; it should + * start fresh and overwrite the file on the next reconcile(). + */ +describe("issue #1648", () => { + const fileCacheName = "issue-1648-files"; + const cacheDir = ".cache-issue-1648"; + const cacheId = ".cache"; + + beforeEach(() => { + fs.mkdirSync(path.resolve(`./${fileCacheName}`), { recursive: true }); + fs.writeFileSync(path.resolve(`./${fileCacheName}/a.txt`), "a"); + fs.writeFileSync(path.resolve(`./${fileCacheName}/b.txt`), "b"); + }); + + afterEach(() => { + fs.rmSync(path.resolve(`./${fileCacheName}`), { + recursive: true, + force: true, + }); + fs.rmSync(path.resolve(`./${cacheDir}`), { recursive: true, force: true }); + }); + + test("1. reconcile() only updates files that were inspected this run", () => { + const fileA = path.resolve(`./${fileCacheName}/a.txt`); + const fileB = path.resolve(`./${fileCacheName}/b.txt`); + + // Run 1: inspect both files and persist them. + const run1 = fileEntryCache.create(cacheId, cacheDir); + expect(run1.getFileDescriptor(fileA).changed).toBe(true); + expect(run1.getFileDescriptor(fileB).changed).toBe(true); + run1.reconcile(); + + // Run 2: only file A is inspected. File B changes on disk in the meantime + // but is NOT inspected, so reconcile() must not revalidate it. + const run2 = fileEntryCache.create(cacheId, cacheDir); + expect(run2.getFileDescriptor(fileA).changed).toBe(false); + fs.writeFileSync(fileB, "b changed"); + run2.reconcile(); + + // Run 3: file B must still be reported as changed because it was never + // inspected (and therefore never revalidated) during run 2. + const run3 = fileEntryCache.create(cacheId, cacheDir); + expect(run3.getFileDescriptor(fileB).changed).toBe(true); + }); + + test("2. getFileDescriptor() keeps reporting changed until reconcile", () => { + const fileA = path.resolve(`./${fileCacheName}/a.txt`); + const cache = fileEntryCache.create(cacheId, cacheDir); + + // A brand new file is changed, and stays changed on subsequent calls. + expect(cache.getFileDescriptor(fileA).changed).toBe(true); + expect(cache.getFileDescriptor(fileA).changed).toBe(true); + expect(cache.getFileDescriptor(fileA).changed).toBe(true); + + // Once reconciled, it is considered unchanged. + cache.reconcile(); + expect(cache.getFileDescriptor(fileA).changed).toBe(false); + + // After a modification it reports changed again, and keeps doing so until + // the next reconcile. + fs.writeFileSync(fileA, "a modified"); + expect(cache.getFileDescriptor(fileA).changed).toBe(true); + expect(cache.getFileDescriptor(fileA).changed).toBe(true); + }); + + test("2b. getFileDescriptor() keeps reporting changed with useCheckSum", () => { + const fileA = path.resolve(`./${fileCacheName}/a.txt`); + const cache = fileEntryCache.create(cacheId, cacheDir, { + useCheckSum: true, + }); + + expect(cache.getFileDescriptor(fileA).changed).toBe(true); + expect(cache.getFileDescriptor(fileA).changed).toBe(true); + + cache.reconcile(); + expect(cache.getFileDescriptor(fileA).changed).toBe(false); + }); + + test("3. create() does not throw on invalid cache file content", () => { + const fileA = path.resolve(`./${fileCacheName}/a.txt`); + const cachePath = path.resolve(`./${cacheDir}/${cacheId}`); + + // Write a cache file with invalid (non-parseable) content. + fs.mkdirSync(path.resolve(`./${cacheDir}`), { recursive: true }); + fs.writeFileSync(cachePath, "this is not valid json {{{"); + + // create() should silently start fresh instead of throwing. + let cache!: ReturnType; + expect(() => { + cache = fileEntryCache.create(cacheId, cacheDir); + }).not.toThrow(); + + // The cache works and the corrupt file is overwritten on reconcile. + expect(cache.getFileDescriptor(fileA).changed).toBe(true); + cache.reconcile(); + + const reloaded = fileEntryCache.create(cacheId, cacheDir); + expect(reloaded.getFileDescriptor(fileA).changed).toBe(false); + }); + + test("reconcile() skips visited entries removed from the underlying cache", () => { + const fileA = path.resolve(`./${fileCacheName}/a.txt`); + const cache = fileEntryCache.create(cacheId, cacheDir); + + // Visit the file so it is tracked as a session baseline. + expect(cache.getFileDescriptor(fileA).changed).toBe(true); + + // Remove the entry directly from the underlying flat-cache, so the session + // baseline references a key that no longer exists in the cache. + cache.cache.removeKey(cache.createFileKey(fileA)); + + // reconcile() must skip the now-missing entry instead of throwing. + expect(() => cache.reconcile()).not.toThrow(); + expect(cache.cache.keys()).not.toContain(cache.createFileKey(fileA)); + }); + + test("3b. createFromFile() does not throw on invalid cache file content", () => { + const cachePath = path.resolve(`./${cacheDir}/${cacheId}`); + fs.mkdirSync(path.resolve(`./${cacheDir}`), { recursive: true }); + fs.writeFileSync(cachePath, "@@ not json @@"); + + expect(() => fileEntryCache.createFromFile(cachePath)).not.toThrow(); + }); + + test("3c. create() rethrows unexpected (non-parse) load errors", () => { + // A directory at the cache path causes a read (EISDIR) error rather than a + // parse error. This must propagate instead of silently discarding data. + const cachePath = path.resolve(`./${cacheDir}/${cacheId}`); + fs.mkdirSync(cachePath, { recursive: true }); + + expect(() => fileEntryCache.create(cacheId, cacheDir)).toThrow(); + }); + + test("a file modified between getFileDescriptor and reconcile is detected next run", () => { + // Regression for reconcile() refreshing size/mtime: the cached entry must + // reflect the content that was actually inspected, not a later edit. With + // useModifiedTime (no checksum), refreshing size/mtime at reconcile time + // would mask a change made after the file was inspected. + const fileA = path.resolve(`./${fileCacheName}/a.txt`); + const cache = fileEntryCache.create(cacheId, cacheDir); + + expect(cache.getFileDescriptor(fileA).changed).toBe(true); + + // Modify the file after inspecting it but before reconciling. + fs.writeFileSync(fileA, "a much longer content than before"); + cache.reconcile(); + + // The next run must still see the file as changed, because the cached + // entry corresponds to the previously-inspected (shorter) content. + const next = fileEntryCache.create(cacheId, cacheDir); + expect(next.getFileDescriptor(fileA).changed).toBe(true); + }); +});