diff --git a/packages/shim/src/fs/promises-mutations.test.js b/packages/shim/src/fs/promises-mutations.test.js new file mode 100644 index 0000000..22a4158 --- /dev/null +++ b/packages/shim/src/fs/promises-mutations.test.js @@ -0,0 +1,140 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { createFsPromises } from "./promises.js"; +import { registerPathResolver, _reset } from "./transforms.js"; +import { isRecentLocalOp } from "./echo-guard.js"; + +function makeDeps() { + const store = new Map(); + + const metadataCache = { + has: (p) => store.has(p), + get: (p) => (store.has(p) ? store.get(p) : null), + set: (p, m) => store.set(p, m), + delete: (p) => store.delete(p), + toStat: (p) => + store.has(p) + ? { + type: store.get(p).type, + isDirectory: () => store.get(p).type === "directory", + isFile: () => store.get(p).type === "file", + } + : null, + readdir: () => [], + }; + + const contentCache = { + get: () => null, + set: vi.fn(), + delete: vi.fn(), + invalidate: vi.fn(), + }; + + const transport = { + mkdir: vi.fn(async () => {}), + rmdir: vi.fn(async () => {}), + stat: vi.fn(async () => ({ type: "file", size: 1 })), + readFile: vi.fn(async () => { + throw new Error("transport.readFile should not be called"); + }), + }; + + return { metadataCache, contentCache, transport, store }; +} + +describe("promises directory mutations honor path resolvers", () => { + afterEach(() => _reset()); + + it("mkdir uses the resolved path for cache, echo-guard, and transport", async () => { + registerPathResolver( + (p) => p === "logical/dir", + () => "physical/dir", + ); + + const deps = makeDeps(); + const fs = createFsPromises( + deps.metadataCache, + deps.contentCache, + deps.transport, + ); + + await fs.mkdir("logical/dir", { recursive: true }); + + expect(deps.store.get("physical/dir")).toEqual({ type: "directory" }); + expect(deps.store.has("logical/dir")).toBe(false); + expect(deps.transport.mkdir).toHaveBeenCalledWith("physical/dir", true); + expect(isRecentLocalOp("physical/dir")).toBe(true); + expect(isRecentLocalOp("logical/dir")).toBe(false); + }); + + it("rmdir uses the resolved path for cache, echo-guard, and transport", async () => { + registerPathResolver( + (p) => p === "logical/dir", + () => "physical/dir", + ); + + const deps = makeDeps(); + const fs = createFsPromises( + deps.metadataCache, + deps.contentCache, + deps.transport, + ); + deps.store.set("physical/dir", { type: "directory" }); + + await fs.rmdir("logical/dir"); + + expect(deps.store.has("physical/dir")).toBe(false); + expect(deps.transport.rmdir).toHaveBeenCalledWith("physical/dir"); + expect(isRecentLocalOp("physical/dir")).toBe(true); + }); +}); + +describe("promises readFile existence", () => { + afterEach(() => _reset()); + + it("answers ENOENT from the cache for a missing non-redirected path, no transport", async () => { + const deps = makeDeps(); + const fs = createFsPromises( + deps.metadataCache, + deps.contentCache, + deps.transport, + ); + + await expect( + fs.readFile("/.obsidian/backlink.json", "utf8"), + ).rejects.toThrow(/ENOENT/); + expect(deps.transport.readFile).not.toHaveBeenCalled(); + }); + + it("falls back to the original path for a redirected miss", async () => { + registerPathResolver( + (p) => p === ".obsidian/workspace.json", + () => ".obsidian/workspace.Work.json", + ); + + const deps = makeDeps(); + deps.transport.readFile = vi.fn(async (p) => { + if (p === ".obsidian/workspace.Work.json") { + const e = new Error("ENOENT"); + e.code = "ENOENT"; + throw e; + } + + return "BASE"; + }); + + const fs = createFsPromises( + deps.metadataCache, + deps.contentCache, + deps.transport, + ); + + // Returns the base content after the redirect target 404s: the fallback fired. + await expect(fs.readFile("/.obsidian/workspace.json", "utf8")).resolves.toBe( + "BASE", + ); + expect(deps.transport.readFile).toHaveBeenCalledWith( + ".obsidian/workspace.Work.json", + "utf8", + ); + }); +}); diff --git a/packages/shim/src/fs/promises.js b/packages/shim/src/fs/promises.js index 391ed3a..6db10a0 100644 --- a/packages/shim/src/fs/promises.js +++ b/packages/shim/src/fs/promises.js @@ -4,6 +4,7 @@ import { applyReadTransform, applyWriteTransform, resolvePath, + resolvePathInfo, } from "./transforms.js"; import { hasVirtualFile, getVirtualFile } from "./virtual-files.js"; import { realpathSync } from "./realpath.js"; @@ -53,7 +54,7 @@ export function createFsPromises(metadataCache, contentCache, transport) { } const wantText = encoding === "utf8" || encoding === "utf-8"; - const resolved = resolvePath(path); + const { resolved, redirected } = resolvePathInfo(path); // Virtual plugin source overrides any cache/transport version. if (hasVirtualFile(resolved)) { @@ -86,8 +87,9 @@ export function createFsPromises(metadataCache, contentCache, transport) { throw e; } - if (!meta && resolved && resolved === path) { - // Throw ENOENT only when not redirected; redirected paths fall through to the transport's fallback. + if (!meta && !redirected) { + // The metadata cache holds every existing path (populated at bootstrap, kept current by the watcher). + // A cache miss on a non-redirected path is genuinely absent. Redirected paths fall through to the transport. const e = new Error( `ENOENT: no such file or directory, open '${path}'`, ); @@ -102,7 +104,7 @@ export function createFsPromises(metadataCache, contentCache, transport) { try { result = await transport.readFile(resolved, encoding); } catch (e) { - if (resolved !== path && e.code === "ENOENT") { + if (redirected && e.code === "ENOENT") { result = await transport.readFile(path, encoding); } else { throw e; @@ -207,16 +209,20 @@ export function createFsPromises(metadataCache, contentCache, transport) { const recursive = typeof options === "object" ? !!options.recursive : !!options; - markLocalOp(path); - metadataCache.set(path, { type: "directory" }); + const resolved = resolvePath(path); - await transport.mkdir(path, recursive); + markLocalOp(resolved); + metadataCache.set(resolved, { type: "directory" }); + + await transport.mkdir(resolved, recursive); }, async rmdir(path) { - markLocalOp(path); - metadataCache.delete(path); - await transport.rmdir(path); + const resolved = resolvePath(path); + + markLocalOp(resolved); + metadataCache.delete(resolved); + await transport.rmdir(resolved); }, async rm(path, options) { diff --git a/packages/shim/src/fs/sync-mutations.test.js b/packages/shim/src/fs/sync-mutations.test.js index 4e0bc9a..a93d20d 100644 --- a/packages/shim/src/fs/sync-mutations.test.js +++ b/packages/shim/src/fs/sync-mutations.test.js @@ -1,6 +1,7 @@ -import { describe, it, expect, vi } from "vitest"; +import { describe, it, expect, vi, afterEach } from "vitest"; import { createFsSync } from "./sync.js"; -import { resolvePath } from "./transforms.js"; +import { resolvePath, registerPathResolver, _reset } from "./transforms.js"; +import { isRecentLocalOp } from "./echo-guard.js"; function makeDeps() { const store = new Map(); @@ -43,6 +44,9 @@ function makeDeps() { appendFile: vi.fn(async () => {}), utimes: vi.fn(async () => {}), stat: vi.fn(async () => ({ type: "file", size: 1 })), + readFileSync: vi.fn(() => { + throw new Error("transport.readFileSync should not be called"); + }), }; return { metadataCache, contentCache, transport, store }; @@ -51,7 +55,11 @@ function makeDeps() { describe("sync fs mutations", () => { it("lstatSync mirrors statSync", () => { const deps = makeDeps(); - const fs = createFsSync(deps.metadataCache, deps.contentCache, deps.transport); + const fs = createFsSync( + deps.metadataCache, + deps.contentCache, + deps.transport, + ); deps.store.set(resolvePath("dir"), { type: "directory" }); expect(fs.lstatSync("dir").isDirectory()).toBe(true); @@ -59,7 +67,11 @@ describe("sync fs mutations", () => { it("mkdirSync updates the cache and fires the transport", () => { const deps = makeDeps(); - const fs = createFsSync(deps.metadataCache, deps.contentCache, deps.transport); + const fs = createFsSync( + deps.metadataCache, + deps.contentCache, + deps.transport, + ); fs.mkdirSync("newdir", { recursive: true }); @@ -69,7 +81,11 @@ describe("sync fs mutations", () => { it("rmSync deletes from the cache and fires the transport", () => { const deps = makeDeps(); - const fs = createFsSync(deps.metadataCache, deps.contentCache, deps.transport); + const fs = createFsSync( + deps.metadataCache, + deps.contentCache, + deps.transport, + ); const key = resolvePath("gone.md"); deps.store.set(key, { type: "file" }); @@ -81,7 +97,11 @@ describe("sync fs mutations", () => { it("renameSync moves cache metadata and fires the transport", () => { const deps = makeDeps(); - const fs = createFsSync(deps.metadataCache, deps.contentCache, deps.transport); + const fs = createFsSync( + deps.metadataCache, + deps.contentCache, + deps.transport, + ); const from = resolvePath("a.md"); const to = resolvePath("b.md"); deps.store.set(from, { type: "file", size: 2 }); @@ -95,7 +115,11 @@ describe("sync fs mutations", () => { it("copyFileSync optimistically mirrors source metadata and fires the transport", () => { const deps = makeDeps(); - const fs = createFsSync(deps.metadataCache, deps.contentCache, deps.transport); + const fs = createFsSync( + deps.metadataCache, + deps.contentCache, + deps.transport, + ); const srcKey = resolvePath("src.md"); const destKey = resolvePath("dest.md"); deps.store.set(srcKey, { type: "file", size: 9 }); @@ -108,7 +132,11 @@ describe("sync fs mutations", () => { it("utimesSync sets mtime and fires the transport", () => { const deps = makeDeps(); - const fs = createFsSync(deps.metadataCache, deps.contentCache, deps.transport); + const fs = createFsSync( + deps.metadataCache, + deps.contentCache, + deps.transport, + ); const key = resolvePath("note.md"); deps.store.set(key, { type: "file", mtime: 0 }); @@ -117,12 +145,101 @@ describe("sync fs mutations", () => { expect(deps.store.get(key).mtime).toBe(222); expect(deps.transport.utimes).toHaveBeenCalled(); }); +}); + +describe("directory mutations honor path resolvers", () => { + afterEach(() => _reset()); + + it("mkdirSync uses the resolved path for cache, echo-guard, and transport", () => { + registerPathResolver( + (p) => p === "logical/dir", + () => "physical/dir", + ); - it("chmodSync is a no-op that does not throw", () => { const deps = makeDeps(); - const fs = createFsSync(deps.metadataCache, deps.contentCache, deps.transport); + const fs = createFsSync( + deps.metadataCache, + deps.contentCache, + deps.transport, + ); - expect(() => fs.chmodSync("note.md", 0o644)).not.toThrow(); - expect(fs.chmodSync("note.md", 0o644)).toBeUndefined(); + fs.mkdirSync("logical/dir", { recursive: true }); + + expect(deps.store.get("physical/dir")).toEqual({ type: "directory" }); + expect(deps.store.has("logical/dir")).toBe(false); + expect(deps.transport.mkdir).toHaveBeenCalledWith("physical/dir", true); + expect(isRecentLocalOp("physical/dir")).toBe(true); + expect(isRecentLocalOp("logical/dir")).toBe(false); + }); + + it("rmdirSync uses the resolved path for cache, echo-guard, and transport", () => { + registerPathResolver( + (p) => p === "logical/dir", + () => "physical/dir", + ); + + const deps = makeDeps(); + const fs = createFsSync( + deps.metadataCache, + deps.contentCache, + deps.transport, + ); + deps.store.set("physical/dir", { type: "directory" }); + + fs.rmdirSync("logical/dir"); + + expect(deps.store.has("physical/dir")).toBe(false); + expect(deps.transport.rmdir).toHaveBeenCalledWith("physical/dir"); + expect(isRecentLocalOp("physical/dir")).toBe(true); + }); +}); + +describe("readFileSync existence", () => { + afterEach(() => _reset()); + + it("answers ENOENT from the cache for a missing non-redirected path, no transport", () => { + const deps = makeDeps(); + const fs = createFsSync( + deps.metadataCache, + deps.contentCache, + deps.transport, + ); + + // Leading slash: normalize strips it, so resolved !== the raw argument. + expect(() => fs.readFileSync("/.obsidian/backlink.json", "utf8")).toThrow( + /ENOENT/, + ); + expect(deps.transport.readFileSync).not.toHaveBeenCalled(); + }); + + it("falls back to the original path for a redirected miss", () => { + registerPathResolver( + (p) => p === ".obsidian/workspace.json", + () => ".obsidian/workspace.Work.json", + ); + + const deps = makeDeps(); + deps.transport.readFileSync = vi.fn((p) => { + if (p === ".obsidian/workspace.Work.json") { + const e = new Error("ENOENT"); + e.code = "ENOENT"; + throw e; + } + + return "BASE"; + }); + + const fs = createFsSync( + deps.metadataCache, + deps.contentCache, + deps.transport, + ); + + // Returns the base content after the redirect target 404s: the fallback fired. + expect(fs.readFileSync("/.obsidian/workspace.json", "utf8")).toBe("BASE"); + expect(deps.transport.readFileSync).toHaveBeenCalledWith( + ".obsidian/workspace.Work.json", + "utf8", + ); }); }); diff --git a/packages/shim/src/fs/sync.js b/packages/shim/src/fs/sync.js index 2db6f9b..e7b2edd 100644 --- a/packages/shim/src/fs/sync.js +++ b/packages/shim/src/fs/sync.js @@ -4,6 +4,7 @@ import { applyReadTransform, applyWriteTransform, resolvePath, + resolvePathInfo, } from "./transforms.js"; import { hasVirtualFile, getVirtualFile } from "./virtual-files.js"; @@ -69,7 +70,7 @@ export function createFsSync(metadataCache, contentCache, transport) { } const wantText = encoding === "utf8" || encoding === "utf-8"; - const resolved = resolvePath(path); + const { resolved, redirected } = resolvePathInfo(path); // Virtual plugin source overrides any cache or transport version. if (hasVirtualFile(resolved)) { @@ -108,13 +109,22 @@ export function createFsSync(metadataCache, contentCache, transport) { result = contentCache.get(resolved); } + // The metadata cache is kept fresh by the filewatcher and a miss here genuinely means the file is absent. + // Redirected paths fall through to the transport, so we can't trust the cache for them, but non-redirected misses are definitive. + if (result === null && !meta && !redirected) { + const e = new Error( + `ENOENT: no such file or directory, open '${path}'`, + ); + e.code = "ENOENT"; + throw e; + } + if (result === null) { - // ENOENT fallback: if the resolved path doesn't exist, try the original. - // Covers per-name workspace files that haven't been saved yet. + // A resolver can map a path onto a physical target that does not exist yet, so a redirected miss retries the original path before failing. try { result = transport.readFileSync(resolved, encoding); } catch (e) { - if (resolved !== path && e.code === "ENOENT") { + if (redirected && e.code === "ENOENT") { console.warn( "[shim:fs] readFileSync cache miss, using sync XHR:", path, @@ -206,20 +216,32 @@ export function createFsSync(metadataCache, contentCache, transport) { const recursive = typeof options === "object" ? !!options.recursive : !!options; - markLocalOp(path); - metadataCache.set(path, { type: "directory" }); + const resolved = resolvePath(path); - transport.mkdir(path, recursive).catch((e) => { - console.error("[shim:fs] mkdirSync background create failed:", path, e); + markLocalOp(resolved); + metadataCache.set(resolved, { type: "directory" }); + + transport.mkdir(resolved, recursive).catch((e) => { + console.error( + "[shim:fs] mkdirSync background create failed:", + resolved, + e, + ); }); }, rmdirSync(path) { - markLocalOp(path); - metadataCache.delete(path); + const resolved = resolvePath(path); - transport.rmdir(path).catch((e) => { - console.error("[shim:fs] rmdirSync background remove failed:", path, e); + markLocalOp(resolved); + metadataCache.delete(resolved); + + transport.rmdir(resolved).catch((e) => { + console.error( + "[shim:fs] rmdirSync background remove failed:", + resolved, + e, + ); }); }, diff --git a/packages/shim/src/fs/transforms.js b/packages/shim/src/fs/transforms.js index 46dca2c..ab6c597 100644 --- a/packages/shim/src/fs/transforms.js +++ b/packages/shim/src/fs/transforms.js @@ -12,7 +12,9 @@ export function registerPathResolver(matcher, resolver) { pathResolvers.push({ matcher, resolver }); } -export function resolvePath(path) { +// resolved is the physical path. +// redirected is true when a path resolver sent the request to a different path. +export function resolvePathInfo(path) { const norm = normalize(path); for (const { matcher, resolver } of pathResolvers) { @@ -21,13 +23,17 @@ export function resolvePath(path) { const resolved = resolver(norm); if (typeof resolved === "string" && resolved.length > 0) { - return resolved; + return { resolved, redirected: true }; } } } catch {} } - return norm; + return { resolved: norm, redirected: false }; +} + +export function resolvePath(path) { + return resolvePathInfo(path).resolved; } // --- Read transforms ---