diff --git a/src/common/utils.path.unit.spec.ts b/src/common/utils.path.unit.spec.ts new file mode 100644 index 00000000..2f008b86 --- /dev/null +++ b/src/common/utils.path.unit.spec.ts @@ -0,0 +1,64 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + normalizePath: vi.fn((path: string) => `normalised(${path})`), + path2idBase: vi.fn(async (path: string) => path), + id2pathBase: vi.fn((path: string) => path), + expandFilePathPrefix: vi.fn((path: string): [string, string] => { + if (path.startsWith("i:")) return ["i:", path.substring(2)]; + return ["", path]; + }), +})); + +vi.mock("@/deps.ts", () => ({ + normalizePath: mocks.normalizePath, + Platform: {}, + requestUrl: vi.fn(), +})); + +vi.mock("@vrtmrz/livesync-commonlib/compat/string_and_binary/path", () => ({ + path2id_base: mocks.path2idBase, + id2path_base: mocks.id2pathBase, + expandFilePathPrefix: mocks.expandFilePathPrefix, + isValidFilenameInLinux: vi.fn(), + isValidFilenameInDarwin: vi.fn(), + isValidFilenameInWidows: vi.fn(), + isValidFilenameInAndroid: vi.fn(), + stripAllPrefixes: vi.fn(), +})); + +describe("path ID normalisation", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it.each([ + ["Folder/Note.md", "", "Folder/Note.md"], + ["Folder/Poem: Example.md", "", "Folder/Poem: Example.md"], + ["Folder/Poem: Example: Final Draft.md", "", "Folder/Poem: Example: Final Draft.md"], + ["i:Folder/Poem: Example.md", "i:", "Folder/Poem: Example.md"], + ])("normalises the complete path body for %s", async (filename, prefix, body) => { + const { path2id } = await import("./utils.ts"); + + const result = await path2id(filename as never, false, false); + + expect(mocks.normalizePath).toHaveBeenCalledWith(body); + expect(mocks.path2idBase).toHaveBeenCalledWith(`${prefix}normalised(${body})`, false, false); + expect(result).toBe(`${prefix}normalised(${body})`); + }); + + it.each([ + ["Folder/Note.md", "", "Folder/Note.md"], + ["Folder/Poem: Example.md", "", "Folder/Poem: Example.md"], + ["Folder/Poem: Example: Final Draft.md", "", "Folder/Poem: Example: Final Draft.md"], + ["i:Folder/Poem: Example.md", "i:", "Folder/Poem: Example.md"], + ])("preserves the path namespace while normalising %s", async (filename, prefix, body) => { + mocks.id2pathBase.mockReturnValue(filename); + const { id2path } = await import("./utils.ts"); + + const result = id2path(filename as never); + + expect(mocks.normalizePath).toHaveBeenCalledWith(body); + expect(result).toBe(`${prefix}normalised(${body})`); + }); +}); diff --git a/src/common/utils.ts b/src/common/utils.ts index f9d60799..f4bc621d 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -7,6 +7,7 @@ import { isValidFilenameInWidows, isValidFilenameInAndroid, stripAllPrefixes, + expandFilePathPrefix, } from "@vrtmrz/livesync-commonlib/compat/string_and_binary/path"; import { Logger } from "@vrtmrz/livesync-commonlib/compat/common/logger"; @@ -41,22 +42,18 @@ export async function path2id( obfuscatePassphrase: string | false, caseInsensitive: boolean ): Promise { - const temp = filename.split(":"); - const path = temp.pop(); + const [prefix, path] = expandFilePathPrefix(filename); const normalizedPath = normalizePath(path as FilePath); - temp.push(normalizedPath); - const fixedPath = temp.join(":") as FilePathWithPrefix; + const fixedPath = `${prefix}${normalizedPath}` as FilePathWithPrefix; const out = await path2id_base(fixedPath, obfuscatePassphrase, caseInsensitive); return out; } export function id2path(id: DocumentID, entry?: EntryHasPath): FilePathWithPrefix { const filename = id2path_base(id, entry); - const temp = filename.split(":"); - const path = temp.pop(); + const [prefix, path] = expandFilePathPrefix(filename); const normalizedPath = normalizePath(path as FilePath); - temp.push(normalizedPath); - const fixedPath = temp.join(":") as FilePathWithPrefix; + const fixedPath = `${prefix}${normalizedPath}` as FilePathWithPrefix; return fixedPath; } diff --git a/src/serviceFeatures/redFlag.simpleFetch.ts b/src/serviceFeatures/redFlag.simpleFetch.ts index 1578ab1c..90355a26 100644 --- a/src/serviceFeatures/redFlag.simpleFetch.ts +++ b/src/serviceFeatures/redFlag.simpleFetch.ts @@ -190,6 +190,20 @@ export async function askAndPerformFastSetupOnScheduledFetchAll( log: LogFunction, cleanupFlag: () => Promise ): Promise { + if (host.services.setting.currentSettings().maxMTimeForReflectEvents > 0) { + // Simple Fetch reconciles storage with the local database after fetching, past the check which + // refuses that scan in remediation mode. Skipping only the scan would restore nothing from most + // remotes: reflection of received documents stays suspended while Simple Fetch fetches, so Object + // Storage and P2P remotes discard them, and CouchDB Fast Fetch writes them straight into the + // database. The detailed flow at least states the restriction and offers to clear it before + // rebuilding, instead of quietly reconciling past it. + log( + "Remediation mode is active, so the detailed fetch flow is used instead of Simple Fetch.", + LOG_LEVEL_NOTICE + ); + clearRememberedSimpleFetchMode(host); + return undefined; + } const result = await askSimpleFetchMode(host); if (result === "cancelled") { log("Fetch cancelled by user.", LOG_LEVEL_NOTICE); diff --git a/src/serviceFeatures/redFlag.unit.spec.ts b/src/serviceFeatures/redFlag.unit.spec.ts index ff5bcb7c..9c1a2bf8 100644 --- a/src/serviceFeatures/redFlag.unit.spec.ts +++ b/src/serviceFeatures/redFlag.unit.spec.ts @@ -771,6 +771,24 @@ describe("Red Flag Feature", () => { }); describe("askAndPerformFastSetupOnScheduledFetchAll", () => { + it("uses the detailed flow instead of Simple Fetch while remediation mode is active", async () => { + const host = createHostMock(); + const log = createLoggerMock(); + const cleanupFlag = vi.fn().mockResolvedValue(undefined); + + Object.assign(host.mocks.setting.settings, { + maxMTimeForReflectEvents: Date.parse("2026-09-01T00:00:00Z"), + }); + + await expect(askAndPerformFastSetupOnScheduledFetchAll(host as any, log, cleanupFlag)).resolves.toBe( + undefined + ); + + expect(host.mocks.ui.confirm.confirmWithMessage).not.toHaveBeenCalled(); + expect(host.mocks.setting.deleteSmallConfig).toHaveBeenCalledWith("simple-fetch-mode"); + expect(cleanupFlag).not.toHaveBeenCalled(); + }); + it("releases both reflection suspensions after Fast Setup succeeds", async () => { const host = createHostMock(); const log = createLoggerMock(); diff --git a/src/serviceFeatures/replication/ReplicateResultProcessor.ts b/src/serviceFeatures/replication/ReplicateResultProcessor.ts index 9bd05505..e37f3c16 100644 --- a/src/serviceFeatures/replication/ReplicateResultProcessor.ts +++ b/src/serviceFeatures/replication/ReplicateResultProcessor.ts @@ -35,7 +35,7 @@ type ReplicateResultProcessorSettings = Pick< >; type ReplicateResultProcessorServices = Pick< LiveSyncBaseCore["services"], - "appLifecycle" | "path" | "replication" | "vault" + "appLifecycle" | "database" | "path" | "replication" | "vault" >; /** @@ -115,10 +115,27 @@ export class ReplicateResultProcessor { // If true, the processing queue processor bails the loop. private _suspended: boolean = false; + /** + * Whether the application accepts replicated documents being applied. + * + * Remediation mode refuses the reconciliation scan which readiness depends upon, so the + * application stays unready for as long as the modification-time limit is configured. + * Applying the received documents is what that mode exists for, and `parseDocumentChange` keeps + * each one within the limit, so readiness is not required while the mode is active. + */ + private get acceptsResultApplication() { + if (this.services.appLifecycle.isReady()) return true; + if (this.context.currentSettings().maxMTimeForReflectEvents <= 0) return false; + // A fetch resets the local database, and a remote which reflects while fetching leaves this + // processor unsuspended throughout. A document applied then cannot gather its chunks and is + // dropped, so the database itself must still be usable. + return this.services.database.isDatabaseReady(); + } + public get isSuspended() { return ( this._suspended || - !this.services.appLifecycle.isReady() || + !this.acceptsResultApplication || this.context.currentSettings().suspendParseReplicationResult || this.services.appLifecycle.isSuspended() ); diff --git a/src/serviceFeatures/replication/ReplicateResultProcessor.unit.spec.ts b/src/serviceFeatures/replication/ReplicateResultProcessor.unit.spec.ts index db5ec9d4..2cded809 100644 --- a/src/serviceFeatures/replication/ReplicateResultProcessor.unit.spec.ts +++ b/src/serviceFeatures/replication/ReplicateResultProcessor.unit.spec.ts @@ -1,7 +1,11 @@ import { promiseWithResolvers } from "octagonal-wheels/promises"; import { reactiveSource } from "octagonal-wheels/dataobject/reactive"; import { describe, expect, it, vi } from "vitest"; -import { VER, type EntryDoc } from "@vrtmrz/livesync-commonlib/compat/common/types"; +import { VER, type EntryDoc, type FilePathWithPrefix } from "@vrtmrz/livesync-commonlib/compat/common/types"; +import { + isValidFilenameInAndroid, + isValidFilenameInWidows, +} from "@vrtmrz/livesync-commonlib/compat/string_and_binary/path"; import { defaultLogger, LOG_LEVEL_DEBUG, @@ -28,6 +32,9 @@ function note(id: string): PouchDB.Core.ExistingDocument { type SetupOptions = { applicationReady?: boolean; + databaseReady?: boolean; + maxMTimeForReflectEvents?: number; + isValidPath?: (path: string) => boolean; processSynchroniseResult?: (entry: unknown) => Promise; setSnapshot?: (key: string, value: unknown) => Promise; }; @@ -38,9 +45,12 @@ function setup(options: SetupOptions = {}) { const runBoundedLocalApplicationActivity = vi.fn(async (task: () => Promise) => await task()); const onCloseActiveReplication = vi.fn(async () => true); const isReady = vi.fn(() => options.applicationReady ?? true); + const isValidPath = vi.fn(options.isValidPath ?? (() => true)); + const getDBEntryFromMeta = vi.fn(async (entry: object) => ({ ...entry, data: "x" })); const core = { services: { appLifecycle: { isReady, isSuspended: () => false }, + database: { isDatabaseReady: () => options.databaseReady ?? true }, path: { getPath: (entry: { path: string }) => entry.path }, replication: { databaseQueueCount: reactiveSource(0), @@ -54,17 +64,20 @@ function setup(options: SetupOptions = {}) { vault: { isTargetFile: vi.fn(async () => true), isFileSizeTooLarge: vi.fn(() => false), - isValidPath: vi.fn(() => true), + isValidPath, }, }, kvDB: { set: setSnapshot }, localDatabase: { getRaw: vi.fn(async (id: string) => ({ _id: id, _rev: "1-test" })), - getDBEntryFromMeta: vi.fn(async (entry: object) => ({ ...entry, data: "x" })), + getDBEntryFromMeta, }, }; const processor = new ReplicateResultProcessor({ - currentSettings: () => ({ maxMTimeForReflectEvents: 0, suspendParseReplicationResult: false }), + currentSettings: () => ({ + maxMTimeForReflectEvents: options.maxMTimeForReflectEvents ?? 0, + suspendParseReplicationResult: false, + }), getKeyValueDB: () => core.kvDB, getLocalDatabase: () => core.localDatabase, requestActiveReplicatorRetirement: () => { @@ -74,7 +87,9 @@ function setup(options: SetupOptions = {}) { services: core.services, } as never); return { + getDBEntryFromMeta, isReady, + isValidPath, onCloseActiveReplication, processor, processSynchroniseResult, @@ -83,6 +98,24 @@ function setup(options: SetupOptions = {}) { } describe("ReplicateResultProcessor", () => { + it.each([ + ["Windows", isValidFilenameInWidows], + ["Android", isValidFilenameInAndroid], + ])("does not reflect a replicated colon path into the %s Vault", async (_platform, validatePath) => { + const path = "Folder/Poem: Example.md" as FilePathWithPrefix; + const document = { ...note("colon-path"), path }; + const { getDBEntryFromMeta, isValidPath, processor, processSynchroniseResult } = setup({ + isValidPath: validatePath, + }); + + processor.enqueueAll([document]); + + await vi.waitFor(() => expect(isValidPath).toHaveBeenCalledWith(path)); + await vi.waitFor(() => expect(processor["_processingChanges"]).toHaveLength(0)); + expect(getDBEntryFromMeta).toHaveBeenCalledWith(expect.objectContaining({ path }), false, true); + expect(processSynchroniseResult).not.toHaveBeenCalled(); + }); + it("resumes another document after in-flight updates to one document fill the application slots", async () => { const hotGate = promiseWithResolvers(); const { processor, processSynchroniseResult } = setup({ @@ -124,6 +157,45 @@ describe("ReplicateResultProcessor", () => { expect(isReady).toHaveBeenCalledOnce(); }); + it("applies results in remediation mode, which never reports readiness", () => { + const { processor } = setup({ + applicationReady: false, + maxMTimeForReflectEvents: Date.parse("2026-09-01T00:00:00Z"), + }); + + expect(processor.isSuspended).toBe(false); + }); + + it("holds results in remediation mode while the local database is being rebuilt", () => { + const { processor } = setup({ + applicationReady: false, + databaseReady: false, + maxMTimeForReflectEvents: Date.parse("2026-09-01T00:00:00Z"), + }); + + expect(processor.isSuspended).toBe(true); + }); + + it("still skips a document modified after the limit while the application is unready", async () => { + const maxMTimeForReflectEvents = Date.parse("2026-09-01T00:00:00Z"); + const { processor, processSynchroniseResult } = setup({ + applicationReady: false, + maxMTimeForReflectEvents, + }); + + const tooRecent = { + ...note("too-recent"), + mtime: maxMTimeForReflectEvents + 1, + } as PouchDB.Core.ExistingDocument; + processor.enqueueAll([tooRecent]); + + await vi.waitFor(() => { + expect(processor["_queuedChanges"]).toHaveLength(0); + expect(processor["_processingChanges"]).toHaveLength(0); + }); + expect(processSynchroniseResult).not.toHaveBeenCalled(); + }); + it("retires active ownership when a newer remote version is observed", async () => { const { onCloseActiveReplication, processor } = setup(); const versionInfo = { diff --git a/src/serviceFeatures/replication/index.ts b/src/serviceFeatures/replication/index.ts index a8046bd9..74a4f1e4 100644 --- a/src/serviceFeatures/replication/index.ts +++ b/src/serviceFeatures/replication/index.ts @@ -51,6 +51,7 @@ export function useReplicationFeature ({ relativePath: `${folders[index % folders.length]}/note-${index}.md`, body: `# Descendant ${index}\n\nThis body must survive a parent folder rename.\n`, })); +notes.push( + { relativePath: "alpha/Poem: Example.md", body: "First poem\n" }, + { relativePath: "beta/Poem: Example.md", body: "Second poem\n" }, + { relativePath: "alpha/deep/Poem: Part: Example.md", body: "Poem with multiple colons\n" } +); async function main(): Promise { const binary = requireObsidianBinary(); @@ -45,7 +51,7 @@ async function main(): Promise { localStorageEntries: createE2eObsidianDeviceLocalState(vault.name), }); await waitForLiveSyncCoreReady(cliBinary, session.cliEnv); - const result = await evalObsidianJson<{ descendants: number; renamed: number; deleted: number }>( + const result = await evalObsidianJson<{ descendants: number; renamed: number; deleted: number; missingRejected: boolean }>( cliBinary, `(async()=>{ const core=app.plugins.plugins['obsidian-livesync'].core; @@ -54,6 +60,7 @@ async function main(): Promise { const originalRoot=${JSON.stringify(originalRoot)}; const renamedRoot=${JSON.stringify(renamedRoot)}; const outsidePath=${JSON.stringify(outsidePath)}; + const missingColonPath=${JSON.stringify(missingColonPath)}; const renamed=new Set(), deleted=new Set(); const refs=[ app.vault.on('rename',(file,oldPath)=>{ @@ -112,11 +119,23 @@ async function main(): Promise { await app.vault.createFolder(originalRoot); for(const folder of ${JSON.stringify(folders)}) await app.vault.createFolder(originalRoot+'/'+folder); - await Promise.all(notes.map(note=>app.vault.create(originalRoot+'/'+note.relativePath,note.body))); + // Obsidian indexes imported colon names but rejects them in Vault.create. + await Promise.all(notes.map(note=>note.relativePath.includes(':') + ? app.vault.adapter.write(originalRoot+'/'+note.relativePath,note.body) + : app.vault.create(originalRoot+'/'+note.relativePath,note.body))); await app.vault.create(outsidePath,'Outside note'); await waitFor('Initial batch',async()=>[ ...await liveBatch(originalRoot), ...await liveErrors(outsidePath,'Outside note'), ]); + for(const note of notes){ + const path=originalRoot+'/'+note.relativePath; + if(!await core.serviceModules.fileHandler.dbToStorage(await meta(path),null,true)) + throw new Error('Database reflection failed: '+path); + } + await waitFor('Reflected batch',()=>liveBatch(originalRoot)); + const expectedPaths=new Set([outsidePath,...notes.map(note=>originalRoot+'/'+note.relativePath)]); + const unexpected=app.vault.getFiles().map(file=>file.path).filter(path=>!expectedPaths.has(path)); + if(unexpected.length) throw new Error('Unexpected reflected files: '+unexpected.join(', ')); const originalIds=await Promise.all(notes.map(async note=>(await meta(originalRoot+'/'+note.relativePath))._id)); // Rename the parent once: Obsidian must emit every descendant event. @@ -144,7 +163,40 @@ async function main(): Promise { if(app.vault.getAbstractFileByPath(renamedRoot)) throw new Error('Deleted folder remains'); await app.vault.modify(app.vault.getAbstractFileByPath(outsidePath),'Outside note updated'); await waitFor('Outside update',()=>liveErrors(outsidePath,'Outside note updated')); - return JSON.stringify({descendants:notes.length,renamed:renamed.size,deleted:deleted.size}); + + // A received database entry must not create a different Vault file when Obsidian rejects its name. + const filesBefore=new Set(app.vault.getFiles().map(file=>file.path)); + const incomingBody='Received colon note\\n'; + const incomingData=new Blob([incomingBody],{type:'text/plain'}); + const incomingId=await core.services.path.path2id(missingColonPath); + const incomingTime=Date.now(); + const saved=await core.localDatabase.putDBEntry({ + _id:incomingId,path:missingColonPath,data:incomingData, + ctime:incomingTime,mtime:incomingTime,size:incomingData.size, + children:[],datatype:'plain',type:'plain',eden:{}, + }); + if(!saved?.ok) throw new Error('Could not seed received Metadata: '+missingColonPath); + const incomingMeta=await meta(missingColonPath); + if(!incomingMeta || incomingMeta._id!==incomingId || incomingMeta.path!==missingColonPath) + throw new Error('Received Metadata has the wrong path: '+missingColonPath); + const incomingEntry=await core.localDatabase.getDBEntry(missingColonPath,{rev:incomingMeta._rev},false,true,true); + if(!incomingEntry || getContent(incomingEntry)!==incomingBody) + throw new Error('Received content could not be read: '+missingColonPath); + let creationFailure=''; + try{ + const reflected=await core.serviceModules.fileHandler.dbToStorage(incomingMeta,null,true); + if(reflected) throw new Error('Obsidian unexpectedly created: '+missingColonPath); + }catch(error){ + creationFailure=String(error); + if(!creationFailure.includes('File name cannot contain')) throw error; + } + if(!creationFailure) throw new Error('Missing name rejection: '+missingColonPath); + const filesAfter=app.vault.getFiles().map(file=>file.path); + const newFiles=filesAfter.filter(path=>!filesBefore.has(path)); + if(newFiles.length) throw new Error('Received note was written under another name: '+newFiles.join(', ')); + if((await meta(missingColonPath))?.path!==missingColonPath) + throw new Error('Received Metadata changed after rejection: '+missingColonPath); + return JSON.stringify({descendants:notes.length,renamed:renamed.size,deleted:deleted.size,missingRejected:true}); }finally{ for(const ref of refs) app.vault.offref(ref); } @@ -153,7 +205,8 @@ async function main(): Promise { ); console.log( `Folder batch: ${result.descendants} descendants persisted, renamed, and deleted; ` + - `${result.renamed} rename and ${result.deleted} delete events observed; outside note remained writable.` + `${result.renamed} rename and ${result.deleted} delete events observed; outside note remained writable; ` + + `missing colon note rejected without an alternate file: ${result.missingRejected}.` ); } finally { if (session) await session.app.stop(); diff --git a/updates.md b/updates.md index 2877d8d4..3cb06600 100644 --- a/updates.md +++ b/updates.md @@ -12,10 +12,14 @@ Earlier releases remain available in the 1.0 release history, the 1.0 preview hi ## Unreleased -### Synchronisation +### Synchronisation and storage #### Fixed +- Files with colons in their names now retain their full paths in synchronisation data instead of appearing as incorrectly named copies at the Vault root. (#1206) + - Obsidian may refuse to create a missing file with such a name. LiveSync also treats these names as invalid on Windows and Android, so the file may not appear in those devices' Vaults. Existing misplaced copies are left for you to review; this change does not remove them automatically. +- Received changes are applied again while remediation mode is active. That mode prevents the scan which readiness depends upon, so nothing had been applied since the plug-in began waiting for readiness, not even changes older than the configured modification-time limit. The limit itself is still enforced for every change, and application waits for a usable local database so that a change arriving during a fetch is not dropped. +- A scheduled fetch no longer offers Simple Fetch while remediation mode is active. Simple Fetch reconciles the Vault with the local database past the restriction, which could store the current files or apply changes newer than the limit; the detailed flow states the restriction and offers to clear it first (#1202). - On start-up, an unchanged file with a missing local revision record can be recognised before newer content arrives, avoiding an unnecessary conflict. Files with actual local edits still require conflict review. (#1207) ## 1.0.30