From ff1f4743242e63cf59ef46837cbd0f9c4b964cc9 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sun, 27 Sep 2026 11:09:36 +0100 Subject: [PATCH] . --- frontend/eslint.config.mjs | 15 +- frontend/sync-client/README.md | 44 +- frontend/sync-client/package.json | 4 +- .../src/file-operations/file-operations.ts | 228 +++-- .../src/file-operations/file-paths.ts | 242 +++-- .../src/persistence/client-metadata-store.ts | 55 ++ .../src/persistence/database.test.ts | 6 +- .../sync-client/src/persistence/database.ts | 52 +- .../src/persistence/settings.test.ts | 8 +- .../sync-client/src/persistence/settings.ts | 7 +- .../src/services/server-config.test.ts | 15 +- .../sync-client/src/services/server-config.ts | 6 +- .../src/services/sync-service.test.ts | 107 +- .../sync-client/src/services/sync-service.ts | 23 +- .../src/services/websocket-manager.test.ts | 87 +- .../src/services/websocket-manager.ts | 50 +- frontend/sync-client/src/sync-client.test.ts | 41 + frontend/sync-client/src/sync-client.ts | 299 +++--- .../src/sync-operations/content-sync.ts | 245 +++++ .../src/sync-operations/content.test.ts | 34 +- .../src/sync-operations/content.ts | 18 +- .../sync-operations/cursor-tracker.test.ts | 52 +- .../src/sync-operations/cursor-tracker.ts | 100 +- .../src/sync-operations/file-manifest.ts | 77 +- .../src/sync-operations/local-change-queue.ts | 73 ++ .../src/sync-operations/local-changes.ts | 66 +- .../sync-operations/outgoing-cursor.test.ts | 32 +- .../src/sync-operations/remote-changes.ts | 96 ++ .../sync-client/src/sync-operations/scan.ts | 217 ++-- .../sync-operations/sync-scheduler.test.ts | 138 +++ .../src/sync-operations/sync-scheduler.ts | 167 ++++ .../src/sync-operations/syncer.test.ts | 191 +--- .../sync-client/src/sync-operations/syncer.ts | 932 +++++------------- .../src/sync-operations/upload-queue.ts | 261 +++++ .../tracing/record-manifest-application.ts | 55 ++ .../sync-client/src/tracing/sync-history.ts | 10 +- frontend/sync-client/src/utils/abortable.ts | 3 +- .../sync-client/src/utils/await-all.test.ts | 1 + frontend/sync-client/src/utils/await-all.ts | 1 + .../data-structures/event-listeners.test.ts | 7 +- .../utils/data-structures/fix-sized-cache.ts | 11 +- .../src/utils/data-structures/locks.test.ts | 15 +- .../debugging/slow-web-socket-factory.ts | 4 +- .../sync-client/src/utils/get-random-color.ts | 1 + .../src/utils/globs-to-regexes.test.ts | 1 + .../sync-client/src/utils/globs-to-regexes.ts | 1 + frontend/sync-client/src/utils/is-binary.ts | 4 +- .../src/utils/is-file-type-mergeable.test.ts | 15 +- .../src/utils/is-file-type-mergeable.ts | 5 +- .../src/utils/line-and-column-to-position.ts | 9 +- .../src/utils/portable-path.test.ts | 7 +- .../sync-client/src/utils/portable-path.ts | 93 +- .../src/utils/position-to-line-and-column.ts | 2 +- .../sync-client/src/utils/rate-limit.test.ts | 6 +- frontend/sync-client/src/utils/rate-limit.ts | 5 +- .../src/utils/remove-from-array.ts | 1 + frontend/sync-client/tsconfig.json | 5 +- frontend/test-support/content-limits.test.ts | 9 +- .../test-support/engine-regressions.test.ts | 135 +-- frontend/test-support/event-delivery.test.ts | 4 +- .../test-support/file-application.test.ts | 44 +- frontend/test-support/listener-crash-child.ts | 2 +- frontend/test-support/polling.test.ts | 4 +- frontend/test-support/real-disk.test.ts | 6 +- frontend/test-support/recovery.test.ts | 10 +- frontend/test-support/sync-audit.test.ts | 85 +- frontend/test-support/sync-fixture.ts | 41 +- sync-server/README.md | 26 +- sync-server/src/app_state.rs | 2 +- sync-server/src/app_state/cursors.rs | 2 +- sync-server/src/app_state/database.rs | 11 +- sync-server/src/app_state/database/history.rs | 48 + sync-server/src/app_state/database/queries.rs | 78 +- sync-server/src/app_state/database/tests.rs | 44 +- .../src/app_state/websocket/broadcasts.rs | 6 +- sync-server/src/app_state/websocket/utils.rs | 4 +- sync-server/src/errors.rs | 20 + sync-server/src/server.rs | 4 +- sync-server/src/server/auth.rs | 8 +- sync-server/src/server/endpoints/events.rs | 2 +- .../fetch_document_version_content.rs | 18 +- .../fetch_latest_document_metadata.rs | 8 +- .../fetch_latest_document_version.rs | 14 +- .../src/server/endpoints/get_file_manifest.rs | 5 +- sync-server/src/server/endpoints/ping.rs | 7 +- .../src/server/endpoints/put_file_content.rs | 49 +- .../src/server/endpoints/put_file_manifest.rs | 40 +- sync-server/src/server/endpoints/utils.rs | 33 +- .../src/server/endpoints/vault_snapshot.rs | 2 +- sync-server/src/server/endpoints/websocket.rs | 164 ++- sync-server/src/server/history.rs | 2 +- sync-server/src/server/tests.rs | 2 +- sync-server/src/utils/portable_path.rs | 125 ++- 93 files changed, 3291 insertions(+), 2023 deletions(-) create mode 100644 frontend/sync-client/src/persistence/client-metadata-store.ts create mode 100644 frontend/sync-client/src/sync-client.test.ts create mode 100644 frontend/sync-client/src/sync-operations/content-sync.ts create mode 100644 frontend/sync-client/src/sync-operations/local-change-queue.ts create mode 100644 frontend/sync-client/src/sync-operations/remote-changes.ts create mode 100644 frontend/sync-client/src/sync-operations/sync-scheduler.test.ts create mode 100644 frontend/sync-client/src/sync-operations/sync-scheduler.ts create mode 100644 frontend/sync-client/src/sync-operations/upload-queue.ts create mode 100644 frontend/sync-client/src/tracing/record-manifest-application.ts create mode 100644 sync-server/src/app_state/database/history.rs diff --git a/frontend/eslint.config.mjs b/frontend/eslint.config.mjs index 1e33ac41..2675b751 100644 --- a/frontend/eslint.config.mjs +++ b/frontend/eslint.config.mjs @@ -5,7 +5,8 @@ import unusedImports from "eslint-plugin-unused-imports"; export default [ { ignores: [ - "sync-client/src/services/types.ts", + // Generated by ts-rs in sync-server; lint the Rust definitions instead. + "sync-client/src/services/types/**", "**/dist/", "**/*.mjs", "**/*.js" @@ -90,5 +91,15 @@ export default [ tsconfigRootDir: import.meta.dirname } } - }) + }), + { + files: ["sync-client/**/*.ts"], + rules: { + curly: ["error", "all"], + "padding-line-between-statements": [ + "error", + { blankLine: "always", prev: "block-like", next: "*" } + ] + } + } ]; diff --git a/frontend/sync-client/README.md b/frontend/sync-client/README.md index bfe89ada..86cd24af 100644 --- a/frontend/sync-client/README.md +++ b/frontend/sync-client/README.md @@ -1,6 +1,6 @@ # Sync client (API v4) -The engine uses one serialized loop per vault and requires Web Crypto (SHA-256). Local notifications and WebSocket +The sync coordinator uses one serialized loop per vault and requires Web Crypto (SHA-256). Local notifications and WebSocket `vaultChanged` hints wake it; complete disk scans and the durable server event log are the sources of truth. Periodic reconciliation is opt-in: set `syncIntervalMs` to a positive interval to repair missed notifications. Unset or `0` disables polling. @@ -20,6 +20,46 @@ resuming starts a fresh session. Connection checks can run while sync is disable WebSocket shutdown retires callbacks immediately and drains message handlers already running; it does not wait for the transport's close handshake. +## Code layout + +Start with `src/sync-operations/syncer.ts` for the sync workflow: resolve the saved +upload, fetch the complete remote history, apply it, and prepare the next upload. +It coordinates the following components: + +| Component | Responsibility | +| -------------------------------------- | --------------------------------------------------------------------------------------------- | +| `sync-scheduler.ts` | Serialize attempts, coalesce notifications, stop, and schedule polling or retries. | +| `local-change-queue.ts` | Persist create/delete/move notifications and invalidate plans when identities change. | +| `scan.ts` | Inventory disk files, infer identities, and commit the scan before consuming notifications. | +| `remote-changes.ts` | Fetch and validate every event page before returning the final remote state. | +| `content-sync.ts` | Fetch merge bases, plan content writes, encode uploads, and maintain the optional diff cache. | +| `upload-queue.ts` | Persist exact requests, handle acknowledgements, and retain rejected requests for retry. | +| `file-operations/` | Apply paths and content to disk, incorporating notifications between files. | +| `persistence/database.ts` | Define `StoredDatabase` and atomically commit or reload it. | +| `persistence/client-metadata-store.ts` | Serialize database, settings, checkpoint, and notification saves into the shared store. | + +`requestSync()` schedules work; `synchronize()` runs an attempt; `refreshLocalFiles()` +flushes notifications and calls `scanLocalFiles()`. `initializeFromServerSnapshot()` +applies the initial server snapshot, while `applyRemoteChanges()` merges and applies +an observed remote state. These operations keep the same persisted database format. + +The scheduler's `SyncPhase` enum makes its transitions explicit: + +| Event | Transition | +| ----------------------------------- | ------------------------------------------------------------------- | +| Start or explicit request | Idle, waiting, or blocked → run requested | +| Begin a pass | Run requested → running | +| Notification during a pass | Running → run requested; repeated notifications coalesce | +| Successful completion | Running → idle; optionally schedule the polling timer | +| Transient failure | Running → waiting to retry | +| Authentication or permanent failure | Running → blocked until an explicit request | +| Stop | Running → stopping → stopped; idle timers are cancelled immediately | + +From `frontend/`, run `npm run lint -w sync-client`, `npm run check -w sync-client`, +and `npm run test -w sync-client`. `npm run test:harness` covers interrupted saves, +file application, and recovery. Protocol types under `src/services/types/` are +generated from Rust and excluded from ESLint; handwritten TypeScript requires braces. + ## State and policy The persisted state contains the incorporated server file manifest, the current @@ -127,7 +167,7 @@ Power loss may lose bytes or rename identity and may cause a merge to run again. Metadata is the one atomic boundary: persistence `save` replaces a complete value and an interrupted/failed save leaves an old or new complete value, never a torn object. A failed save is reloaded before retrying. No ordering between metadata and -user-file durability is required. Settings and engine saves are serialized. +user-file durability is required. Settings and database saves are serialized. Keep metadata outside the user-file namespace or in `.vault-link-sync`, which is excluded from scans. Changing a client's vault once it holds identities, offline notifications, pending requests or an incorporated event cursor requires a separate state store. diff --git a/frontend/sync-client/package.json b/frontend/sync-client/package.json index bdc094cb..19ddd759 100644 --- a/frontend/sync-client/package.json +++ b/frontend/sync-client/package.json @@ -10,7 +10,9 @@ "scripts": { "dev": "webpack watch --mode development", "build": "webpack --mode production", - "test": "tsx --test 'src/**/*.test.ts'" + "test": "tsx --test 'src/**/*.test.ts'", + "lint": "eslint . --max-warnings 0", + "check": "tsc --noEmit" }, "devDependencies": { "byte-base64": "^1.1.0", diff --git a/frontend/sync-client/src/file-operations/file-operations.ts b/frontend/sync-client/src/file-operations/file-operations.ts index a3c36582..ab91713e 100644 --- a/frontend/sync-client/src/file-operations/file-operations.ts +++ b/frontend/sync-client/src/file-operations/file-operations.ts @@ -3,7 +3,7 @@ import type { FileSystemOperations } from "./filesystem-operations"; import type { Database, DocumentId, - EngineState, + StoredDatabase, RelativePath } from "../persistence/database"; import { @@ -21,7 +21,7 @@ import { decodeText } from "../utils/decode-text"; import type { ServerConfig } from "../services/server-config"; import { applyLocalChange, - unappliedChanges, + getUnappliedChanges, type LocalChange } from "../sync-operations/local-changes"; import { LocalChangesDuringReconciliation } from "../errors/errors"; @@ -33,9 +33,18 @@ export interface FileWrite { type Mutation = (action: () => Promise) => Promise; +interface FileApplicationContext { + current: StoredDatabase; + next: StoredDatabase; + writes: Partial>; + paths: Set; + guard: () => void; + mutate: Mutation; +} + /** Apply files first, then save metadata. Repeating a content merge is idempotent: reconcile(p, a, a) == a */ export class FileOperations { - private applying?: EngineState; + private applying?: StoredDatabase; private readonly paths: FilePaths; public constructor( @@ -60,7 +69,7 @@ export class FileOperations { this.paths = new FilePaths(fs, isProtectedFile); } - public async snapshot( + public async readSnapshot( path: RelativePath ): Promise { const snapshot = await this.fs.read(path); @@ -79,16 +88,19 @@ export class FileOperations { ): void { const state = structuredClone(this.applying ?? this.database.state); - if (state.lastSeenUpdateId === undefined) return; + if (state.lastSeenUpdateId === undefined) { + return; + } - for (const pending of unappliedChanges(state, previous)) + for (const pending of getUnappliedChanges(state, previous)) { applyLocalChange(state, pending, () => false); + } applyLocalChange(state, change, () => false); } - public async apply( - next: EngineState, + public async applyChanges( + next: StoredDatabase, writes: Partial> = {}, validatePlan: () => void = () => undefined ): Promise { @@ -96,16 +108,22 @@ export class FileOperations { const changes = this.localChanges.entries().length; let started = false; const guard = (): void => { - if (started) return; + if (started) { + return; + } + validatePlan(); - if (this.localChanges.entries().length > changes) + if (this.localChanges.entries().length > changes) { throw new LocalChangesDuringReconciliation(); + } }; + const mutate: Mutation = async (action) => { guard(); started = true; await action(); }; + guard(); const current = structuredClone(this.database.state); current.lastSeenUpdateId = next.lastSeenUpdateId; @@ -114,7 +132,7 @@ export class FileOperations { const observeChanges = async (): Promise => { await this.localChanges.flush(); guard(); - for (const change of unappliedChanges( + for (const change of getUnappliedChanges( next, this.localChanges.entries() )) { @@ -130,91 +148,23 @@ export class FileOperations { } } }; + try { for (const id of new Set([ ...Object.keys(current.local), ...Object.keys(next.local) ])) { await observeChanges(); - let from = - current.documents[id]?.observedHash !== undefined - ? current.local[id] - : undefined; - let to = next.local[id]; - const write = writes[id]; - if (!write && (from === undefined || from === to)) continue; - if (from !== undefined) { - paths.add(from); - const info = await this.fs.stat(from); - guard(); - if (info?.kind !== "file") { - delete next.local[id]; - continue; - } - if (this.isProtectedFile(from, info.size)) { - next.local[id] = from; - next.documents[id] = structuredClone( - current.documents[id] - ); - (next.excluded ??= {})[id] = - current.fileManifest.entries[id] ?? null; - continue; - } - } - if (to === undefined) { - if (from !== undefined) { - await mutate(() => this.fs.deleteFile(from!)); - delete current.local[id]; - await this.paths.removeEmptyParents(from); - } - continue; - } - if (from !== to) { - to = await this.paths.prepareDestination( - next, - current, - id, - to, - mutate - ); - next.local[id] = to; - from = - current.documents[id]?.observedHash !== undefined - ? current.local[id] - : undefined; - if (from !== undefined) { - await mutate(async () => { - current.local[id] = to; - await this.fs.rename(from!, to); - }); - await this.paths.removeEmptyParents(from); - } - } - if (write) { - const snapshot = - from === undefined ? undefined : await this.fs.read(to); - if (from !== undefined && !snapshot) { - delete next.local[id]; - continue; - } - const extensions = (await this.serverConfig.getConfig()) - .mergeableFileExtensions; - await mutate(async () => { - current.local[id] = to; - current.documents[id] = { observedHash: "" }; - next.documents[id].observedHash = - await this.updateContent( - to, - snapshot, - write, - extensions - ); - }); - } - paths.add(to); - current.local[id] = to; - current.documents[id] = structuredClone(next.documents[id]); + await this.applyDocument(id, { + current, + next, + writes, + paths, + guard, + mutate + }); } + await observeChanges(); await this.database.commit(next); } finally { @@ -223,6 +173,105 @@ export class FileOperations { } } + private async applyDocument( + id: DocumentId, + context: FileApplicationContext + ): Promise { + const { current, next, writes, paths, guard, mutate } = context; + let from = + current.documents[id]?.observedHash !== undefined + ? current.local[id] + : undefined; + let to = next.local[id]; + const write = writes[id]; + if (!write && (from === undefined || from === to)) { + return; + } + + if (from !== undefined) { + paths.add(from); + const info = await this.fs.stat(from); + guard(); + if (info?.kind !== "file") { + Reflect.deleteProperty(next.local, id); + return; + } + + if (this.isProtectedFile(from, info.size)) { + next.local[id] = from; + next.documents[id] = structuredClone( + current.documents[id] ?? {} + ); + (next.excluded ??= {})[id] = + current.fileManifest.entries[id] ?? null; + return; + } + } + + if (to === undefined) { + if (from !== undefined) { + const source = from; + await mutate(async () => this.fs.deleteFile(source)); + Reflect.deleteProperty(current.local, id); + await this.paths.removeEmptyParents(from); + } + + return; + } + + if (from !== to) { + to = await this.paths.prepareDestination( + next, + current, + id, + to, + mutate + ); + next.local[id] = to; + from = + current.documents[id]?.observedHash !== undefined + ? current.local[id] + : undefined; + if (from !== undefined) { + const source = from; + const destination = to; + await mutate(async () => { + current.local[id] = destination; + await this.fs.rename(source, destination); + }); + await this.paths.removeEmptyParents(from); + } + } + + if (write) { + const snapshot = + from === undefined ? undefined : await this.fs.read(to); + if (from !== undefined && !snapshot) { + Reflect.deleteProperty(next.local, id); + return; + } + + const extensions = (await this.serverConfig.getConfig()) + .mergeableFileExtensions; + const destination = to; + await mutate(async () => { + current.local[id] = destination; + current.documents[id] = { observedHash: "" }; + (next.documents[id] ??= {}).observedHash = + await this.updateContent( + destination, + snapshot, + write, + extensions + ); + }); + } + + paths.add(to); + current.local[id] = to; + current.documents[id] = structuredClone(next.documents[id] ?? {}); + } + private async updateContent( path: RelativePath, current: FileSnapshot | undefined, @@ -245,6 +294,7 @@ export class FileOperations { ); return hash(new TextEncoder().encode(text)); } + const result = current && base ? await mergeContent( diff --git a/frontend/sync-client/src/file-operations/file-paths.ts b/frontend/sync-client/src/file-operations/file-paths.ts index ac71411b..b5dbda59 100644 --- a/frontend/sync-client/src/file-operations/file-paths.ts +++ b/frontend/sync-client/src/file-operations/file-paths.ts @@ -2,11 +2,22 @@ import { v4 as uuid } from "uuid"; import type { FileSystemOperations } from "./filesystem-operations"; import type { DocumentId, - EngineState, + StoredDatabase, RelativePath } from "../persistence/database"; import { allocatePortablePath, isInternalPath } from "../utils/portable-path"; +interface PathObstruction { + path: RelativePath; + kind: "file" | "directory"; +} + +interface Occupant { + id: DocumentId; + path: RelativePath; +} +type Mutation = (action: () => Promise) => Promise; + /** Resolve occupied rename destinations without discarding another file. */ export class FilePaths { public constructor( @@ -18,107 +29,169 @@ export class FilePaths { ) {} public async prepareDestination( - next: EngineState, - current: EngineState, + next: StoredDatabase, + current: StoredDatabase, id: DocumentId, path: RelativePath, mutate: (action: () => Promise) => Promise ): Promise { try { for (;;) { - const obstruction = await this.obstruction(path); - if (!obstruction) break; - const children = - obstruction.kind === "file" - ? [obstruction.path] - : await this.fs.listFilesRecursively(obstruction.path); - const occupants = []; - for (const child of children) { - const occupant = Object.keys(current.local).find( - (key) => - current.documents[key]?.observedHash !== - undefined && current.local[key] === child - ); - const info = await this.fs.stat(child); - if ( - !info || - this.isProtectedFile(child, info.size) || - (obstruction.kind === "directory" && - (occupant === undefined || - next.local[occupant] === child)) - ) - break; - occupants.push({ id: occupant ?? uuid(), path: child }); + const obstruction = await this.findObstruction(path); + if (!obstruction) { + break; } - if (occupants.length !== children.length) { - path = await this.conflictPath(next, id, path); + + const occupants = await this.findMovableOccupants( + obstruction, + current, + next + ); + if (occupants === undefined) { + path = await this.allocateConflictPath(next, id, path); continue; } - for (const occupant of occupants) { - const displaced = await this.conflictPath( - next, - occupant.id, - occupant.path.split("/").at(-1)! - ); - await mutate(async () => { - current.local[occupant.id] = displaced; - await this.fs.rename(occupant.path, displaced); - }); - if (!current.documents[occupant.id]) { - current.documents[occupant.id] = { observedHash: "" }; - next.documents[occupant.id] = { observedHash: "" }; - next.local[occupant.id] = displaced; - } else if (next.local[occupant.id] === occupant.path) - next.local[occupant.id] = displaced; - } - if (obstruction.kind === "directory") - await mutate(() => + + await this.relocateOccupants(occupants, current, next, mutate); + if (obstruction.kind === "directory") { + await mutate(async () => this.fs.deleteDirectory(obstruction.path) ); + } } - await this.ensureParents(path); + + await this.createParentDirectories(path); } catch (error) { - const code = - typeof error === "object" && error !== null && "code" in error - ? error.code - : undefined; - if ( - ![ - "ENAMETOOLONG", - "EINVAL", - "ENOTSUP", - "EOPNOTSUPP", - "EACCES", - "EPERM" - ].includes(String(code)) - ) - throw error; - const extension = - /\.[^.]*$/u.exec(path.split("/").at(-1) ?? "")?.[0] ?? ""; - path = await this.conflictPath( - next, - id, - `Recovered ${id}${extension}` - ); - await this.ensureParents(path); + return this.recoverDestination(next, id, path, error); } + return path; } - private async obstruction( + public async removeEmptyParents(path: RelativePath): Promise { + let parent = path.split("/").slice(0, -1).join("/"); + while (parent && !isInternalPath(parent)) { + if ((await this.fs.listFilesRecursively(parent)).length) { + break; + } + + try { + await this.fs.deleteDirectory(parent); + } catch { + break; + } + + parent = parent.split("/").slice(0, -1).join("/"); + } + } + + private async findMovableOccupants( + obstruction: PathObstruction, + current: StoredDatabase, + next: StoredDatabase + ): Promise { + const children = + obstruction.kind === "file" + ? [obstruction.path] + : await this.fs.listFilesRecursively(obstruction.path); + const occupants = []; + for (const child of children) { + const occupant = Object.keys(current.local).find( + (key) => + current.documents[key]?.observedHash !== undefined && + current.local[key] === child + ); + const info = await this.fs.stat(child); + if ( + !info || + this.isProtectedFile(child, info.size) || + (obstruction.kind === "directory" && + (occupant === undefined || next.local[occupant] === child)) + ) { + return undefined; + } + + occupants.push({ id: occupant ?? uuid(), path: child }); + } + + return occupants; + } + + private async relocateOccupants( + occupants: Occupant[], + current: StoredDatabase, + next: StoredDatabase, + mutate: Mutation + ): Promise { + for (const occupant of occupants) { + const displaced = await this.allocateConflictPath( + next, + occupant.id, + occupant.path.split("/").at(-1) ?? occupant.path + ); + await mutate(async () => { + current.local[occupant.id] = displaced; + await this.fs.rename(occupant.path, displaced); + }); + if (!current.documents[occupant.id]) { + current.documents[occupant.id] = { observedHash: "" }; + next.documents[occupant.id] = { observedHash: "" }; + next.local[occupant.id] = displaced; + } else if (next.local[occupant.id] === occupant.path) { + next.local[occupant.id] = displaced; + } + } + } + + private async recoverDestination( + next: StoredDatabase, + id: DocumentId, + path: RelativePath, + error: unknown + ): Promise { + const code = + typeof error === "object" && error !== null && "code" in error + ? error.code + : undefined; + if ( + ![ + "ENAMETOOLONG", + "EINVAL", + "ENOTSUP", + "EOPNOTSUPP", + "EACCES", + "EPERM" + ].includes(String(code)) + ) { + throw error; + } + + const extension = + /\.[^.]*$/u.exec(path.split("/").at(-1) ?? "")?.[0] ?? ""; + path = await this.allocateConflictPath( + next, + id, + `Recovered ${id}${extension}` + ); + await this.createParentDirectories(path); + return path; + } + + private async findObstruction( path: RelativePath - ): Promise<{ path: RelativePath; kind: "file" | "directory" } | undefined> { + ): Promise { const parts = path.split("/"); for (let i = 0; i < parts.length; i++) { const prefix = parts.slice(0, i + 1).join("/"); const info = await this.fs.stat(prefix); - if (info && (info.kind === "file" || i === parts.length - 1)) + if (info && (info.kind === "file" || i === parts.length - 1)) { return { path: prefix, kind: info.kind }; + } } } - private async conflictPath( - next: EngineState, + private async allocateConflictPath( + next: StoredDatabase, id: DocumentId, path: RelativePath ): Promise { @@ -131,21 +204,10 @@ export class FilePaths { return allocatePortablePath(path, id, [...occupied, ...files]); } - private async ensureParents(path: RelativePath): Promise { + private async createParentDirectories(path: RelativePath): Promise { const parent = path.split("/").slice(0, -1).join("/"); - if (parent) await this.fs.createDirectory(parent); - } - - public async removeEmptyParents(path: RelativePath): Promise { - let parent = path.split("/").slice(0, -1).join("/"); - while (parent && !isInternalPath(parent)) { - if ((await this.fs.listFilesRecursively(parent)).length) break; - try { - await this.fs.deleteDirectory(parent); - } catch { - break; - } - parent = parent.split("/").slice(0, -1).join("/"); + if (parent) { + await this.fs.createDirectory(parent); } } } diff --git a/frontend/sync-client/src/persistence/client-metadata-store.ts b/frontend/sync-client/src/persistence/client-metadata-store.ts new file mode 100644 index 00000000..7035ae00 --- /dev/null +++ b/frontend/sync-client/src/persistence/client-metadata-store.ts @@ -0,0 +1,55 @@ +import type { StoredClient } from "../sync-client"; +import type { StoredDatabase } from "./database"; +import type { MetadataPersistenceProvider } from "./metadata-persistence-provider"; +import { Lock } from "../utils/data-structures/locks"; + +/** Serialize partial updates to the shared settings, database and notification store. */ +export class ClientMetadataStore { + private readonly lock = new Lock(); + private reloadBeforeSave = false; + + private constructor( + public stored: StoredClient, + private readonly persistence: MetadataPersistenceProvider + ) {} + + public static async load( + persistence: MetadataPersistenceProvider + ): Promise { + return new ClientMetadataStore( + (await persistence.load()) ?? {}, + persistence + ); + } + + public async save(update: StoredClient): Promise { + await this.lock.withLock(async () => { + if (this.reloadBeforeSave) { + await this.reload(); + } + + const next = { ...this.stored, ...structuredClone(update) }; + try { + await this.persistence.save(next); + this.stored = next; + } catch (error) { + // A failed save may have committed. Reload before another update + // so settings or notifications cannot roll the database back. + this.reloadBeforeSave = true; + throw error; + } + }); + } + + public async reloadDatabase(): Promise { + return this.lock.withLock(async () => { + await this.reload(); + return this.stored.database; + }); + } + + private async reload(): Promise { + this.stored = (await this.persistence.load()) ?? {}; + this.reloadBeforeSave = false; + } +} diff --git a/frontend/sync-client/src/persistence/database.test.ts b/frontend/sync-client/src/persistence/database.test.ts index 2304b940..e193801d 100644 --- a/frontend/sync-client/src/persistence/database.test.ts +++ b/frontend/sync-client/src/persistence/database.test.ts @@ -1,9 +1,9 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { Database, emptyState } from "./database"; +import { Database, createEmptyDatabase } from "./database"; test("current state keeps absent optional fields absent across save and reload", async () => { - const initial = emptyState("vault"); + const initial = createEmptyDatabase("vault"); initial.local = { local: "local.md", remote: "remote.md" }; initial.documents = { local: { observedHash: "local" }, @@ -28,7 +28,7 @@ test("current state keeps absent optional fields absent across save and reload", }); test("current reset state is restored after an uncertain save", async () => { - const saved = emptyState("vault"); + const saved = createEmptyDatabase("vault"); saved.local = { note: "note.md" }; saved.documents.note = { base: { vaultUpdateId: null, hash: "clean" }, diff --git a/frontend/sync-client/src/persistence/database.ts b/frontend/sync-client/src/persistence/database.ts index 96f3de64..234756af 100644 --- a/frontend/sync-client/src/persistence/database.ts +++ b/frontend/sync-client/src/persistence/database.ts @@ -22,13 +22,17 @@ export interface DocumentState { export type { PendingRequest } from "./requests"; -export interface EngineState { +export interface StoredDatabase { vaultKey: string; + /** Last incorporated server manifest; the base for the next three-way path merge. */ fileManifest: FileManifest; + /** Current local document IDs and paths, including files awaiting upload or download. */ local: FileManifestEntries; + /** Per-document merge bases and observations; content bytes are not stored here. */ documents: Record; /** Absent until a vault snapshot is applied; also marks a durable history reset. */ lastSeenUpdateId?: VaultUpdateId; + /** Exact request to resolve before observing a newer remote base. */ pending?: PendingRequest; /** Commits consumption of the saved notification queue with the identity map. */ lastAppliedLocalChangeId?: string; @@ -36,9 +40,7 @@ export interface EngineState { excluded?: Record; } -export type StoredDatabase = EngineState; - -export function emptyState(vaultKey: string): StoredDatabase { +export function createEmptyDatabase(vaultKey: string): StoredDatabase { return { vaultKey, fileManifest: { fileManifestId: 0, entries: {} }, @@ -47,7 +49,7 @@ export function emptyState(vaultKey: string): StoredDatabase { }; } -function canRebindEmpty(state: StoredDatabase): boolean { +function canBindToAnotherVault(state: StoredDatabase): boolean { return ( state.lastSeenUpdateId === undefined && !state.pending && @@ -66,32 +68,44 @@ export class Database { ) { const state = initial === undefined - ? emptyState(vaultKey) + ? createEmptyDatabase(vaultKey) : structuredClone(initial); - if (state.vaultKey !== vaultKey && !canRebindEmpty(state)) { + if (state.vaultKey !== vaultKey && !canBindToAnotherVault(state)) { throw new Error( "Incompatible vault identity. Preserve it for recovery and initialize a separate state store." ); } + this.state = { ...state, vaultKey }; } + public get length(): number { + return Object.keys(this.state.local).length; + } + /** Bind an empty database to the configured vault. */ public async bindVault(vaultKey: string): Promise { - if (this.state.vaultKey === vaultKey) return; - if (!canRebindEmpty(this.state)) + if (this.state.vaultKey === vaultKey) { + return; + } + + if (!canBindToAnotherVault(this.state)) { throw new Error( "Incompatible vault identity. Preserve it for recovery and initialize a separate state store." ); + } + await this.commit({ ...this.state, vaultKey }); } public async commit(next: StoredDatabase): Promise { const snapshot = structuredClone(next); - if (this.needsReload) + if (this.needsReload) { throw new Error( "Reload saved state before retrying an uncertain save" ); + } + this.needsReload = true; await this.saveData(snapshot); this.state = snapshot; @@ -99,20 +113,26 @@ export class Database { } public async recoverPersistence(): Promise { - if (!this.needsReload) return; + if (!this.needsReload) { + return; + } + const saved = await this.loadData(); - if (saved) this.state = structuredClone(saved); + if (saved) { + this.state = structuredClone(saved); + } + this.needsReload = false; } + public cloneState(): StoredDatabase { + return structuredClone(this.state); + } + public async save(): Promise { await this.commit(this.state); } - public get length(): number { - return Object.keys(this.state.local).length; - } - public findDocumentId(path: RelativePath): DocumentId | undefined { return Object.keys(this.state.local).find( (id) => this.state.local[id] === path diff --git a/frontend/sync-client/src/persistence/settings.test.ts b/frontend/sync-client/src/persistence/settings.test.ts index cff24895..915ce77e 100644 --- a/frontend/sync-client/src/persistence/settings.test.ts +++ b/frontend/sync-client/src/persistence/settings.test.ts @@ -9,7 +9,7 @@ describe("Settings logging", () => { const settings = new Settings( logger, { token: "loaded-secret" }, - async () => {} + async () => undefined ); await settings.setSettings({ token: "updated-secret" }); @@ -43,7 +43,7 @@ it("owns settings arrays instead of sharing them with callers and listeners", as const settings = new Settings( new Logger(), { ignorePatterns: original }, - async () => {} + async () => undefined ); original.push("external mutation"); assert.deepEqual(settings.getSettings().ignorePatterns, ["original"]); @@ -67,7 +67,9 @@ it("file eligibility follows successfully saved settings", async () => { maxFileSizeMB: 1 }, async () => { - if (fail) throw new Error("save failed"); + if (fail) { + throw new Error("save failed"); + } } ); assert(settings.isIgnored("private/note.md")); diff --git a/frontend/sync-client/src/persistence/settings.ts b/frontend/sync-client/src/persistence/settings.ts index c06c2b0a..c3575548 100644 --- a/frontend/sync-client/src/persistence/settings.ts +++ b/frontend/sync-client/src/persistence/settings.ts @@ -112,17 +112,20 @@ export class Settings { await this.saveData(next); this.settings = next; - if (update.ignorePatterns !== undefined) + if (update.ignorePatterns !== undefined) { this.ignorePatterns = globsToRegexes( next.ignorePatterns, this.logger ); + } return [this.getSettings(), oldSettings] as const; }); // Listeners can themselves update settings. Never await them while // holding a lock needed by the operation they invoke. - if (notify) await this.onSettingsChanged.triggerAsync(...change); + if (notify) { + await this.onSettingsChanged.triggerAsync(...change); + } } } diff --git a/frontend/sync-client/src/services/server-config.test.ts b/frontend/sync-client/src/services/server-config.test.ts index 735d798f..410b2b67 100644 --- a/frontend/sync-client/src/services/server-config.test.ts +++ b/frontend/sync-client/src/services/server-config.test.ts @@ -1,3 +1,4 @@ +import { awaitAll } from "../utils/await-all"; import assert from "node:assert/strict"; import { test } from "node:test"; import { ServerConfig } from "./server-config"; @@ -12,28 +13,31 @@ const config = { mergeableFileExtensions: ["md"], serverVersion: "test" }; -const make = (fetch: typeof globalThis.fetch) => +const make = (fetch: typeof globalThis.fetch): ServerConfig => new ServerConfig( new SyncService( "device", new Settings( new Logger(), { remoteUri: "http://test" }, - async () => {} + async () => undefined ), fetch, - { get: () => undefined, save: async () => {} } + { get: () => undefined, save: async () => undefined } ) ); test("configuration shares one validated request and retries failures", async () => { let calls = 0; const server = make(async () => { - if (++calls === 1) throw new Error("offline"); + if (++calls === 1) { + throw new Error("offline"); + } + return Response.json(config); }); await assert.rejects(server.getConfig(), /offline/); - const [a, b] = await Promise.all([server.getConfig(), server.getConfig()]); + const [a, b] = await awaitAll([server.getConfig(), server.getConfig()]); assert.equal(a, b); assert.equal(calls, 2); await server.getConfig(); @@ -74,6 +78,7 @@ for (const fail of [false, true]) { ); await old; } + assert.equal(await server.getConfig(), current); assert.equal(calls, 2); }); diff --git a/frontend/sync-client/src/services/server-config.ts b/frontend/sync-client/src/services/server-config.ts index a79014fc..3f980744 100644 --- a/frontend/sync-client/src/services/server-config.ts +++ b/frontend/sync-client/src/services/server-config.ts @@ -59,12 +59,16 @@ export class ServerConfig { return config; }) .catch((error: unknown) => { - if (this.config === pending) this.config = undefined; + if (this.config === pending) { + this.config = undefined; + } + throw error; }); this.config = pending; } + return this.config; } diff --git a/frontend/sync-client/src/services/sync-service.test.ts b/frontend/sync-client/src/services/sync-service.test.ts index 18d8e98a..f3603974 100644 --- a/frontend/sync-client/src/services/sync-service.test.ts +++ b/frontend/sync-client/src/services/sync-service.test.ts @@ -5,7 +5,7 @@ import { Logger } from "../tracing/logger"; import { SyncResetError } from "../errors/errors"; import { SyncService } from "./sync-service"; -const settings = () => +const settings = (): Settings => new Settings( new Logger(), { @@ -13,18 +13,26 @@ const settings = () => vaultName: "test", requestTimeoutMs: 1000 }, - async () => { } + async () => undefined ); -test("paused sync sends no requests, while connection checks still work", async () => { +test("paused sync sends no requests, while connection checks still work", async (): Promise => { let requests = 0; - const service = new SyncService("device", settings(), async () => { - requests++; - return Response.json( - { headEventId: 0, endEventId: 0, events: [] }, - { headers: { "X-Vault-Link-History": "0:empty" } } - ); - }, { get: () => undefined, save: async () => { } }); + const service = new SyncService( + "device", + settings(), + async () => { + requests++; + return Response.json( + { headEventId: 0, endEventId: 0, events: [] }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); + }, + { + get: (): undefined => undefined, + save: async (): Promise => undefined + } + ); await assert.rejects(service.getEvents(0), SyncResetError); assert.equal(requests, 0); await service.ping(); @@ -34,7 +42,7 @@ test("paused sync sends no requests, while connection checks still work", async assert.equal(requests, 2); }); -test("pause aborts requests and fences late responses from an injected fetch", async () => { +test("pause aborts requests and fences late responses from an injected fetch", async (): Promise => { const response = Promise.withResolvers(); const signals: AbortSignal[] = []; const saved: (string | undefined)[] = []; @@ -42,17 +50,18 @@ test("pause aborts requests and fences late responses from an injected fetch", a "device", settings(), async (_, init) => { - signals.push(init!.signal!); + assert.ok(init?.signal); + signals.push(init.signal); return signals.length === 1 ? response.promise : Response.json( - { headEventId: 0, endEventId: 0, events: [] }, - { headers: { "X-Vault-Link-History": "0:empty" } } - ); + { headEventId: 0, endEventId: 0, events: [] }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); }, { - get: () => undefined, - save: async (value) => { + get: (): undefined => undefined, + save: async (value): Promise => { saved.push(value); } } @@ -61,10 +70,10 @@ test("pause aborts requests and fences late responses from an injected fetch", a const first = service.getEvents(0); service.pause(); await assert.rejects(first, SyncResetError); - assert.equal(signals[0].aborted, true); + assert.equal(signals[0]?.aborted, true); service.resume(); await service.getEvents(0); - assert.equal(signals[1].aborted, false); + assert.equal(signals[1]?.aborted, false); response.resolve( Response.json( {}, @@ -76,49 +85,53 @@ test("pause aborts requests and fences late responses from an injected fetch", a }); for (const binary of [false, true]) { - test(`pause interrupts a stalled ${binary ? "binary" : "JSON"} response body`, async () => { - const headersSaved = Promise.withResolvers(); - let body!: ReadableStreamDefaultController; + test(`pause interrupts a stalled ${binary ? "binary" : "JSON"} response body`, async (): Promise => { + const headersSaved = Promise.withResolvers(); + const bodyReady = + Promise.withResolvers< + ReadableStreamDefaultController + >(); const service = new SyncService( "device", settings(), async () => new Response( - new ReadableStream({ - start(controller) { - body = controller; + new ReadableStream({ + start(controller): void { + bodyReady.resolve(controller); } }), { headers: { "X-Vault-Link-History": "1:head" } } ), { - get: () => undefined, - save: async () => { - headersSaved.resolve(); + get: (): undefined => undefined, + save: async (): Promise => { + headersSaved.resolve(undefined); } } ); service.resume(); const request = binary ? service.getDocumentVersionContent({ - documentId: "a", - vaultUpdateId: 1 - }) + documentId: "a", + vaultUpdateId: 1 + }) : service.getEvents(0); await headersSaved.promise; await new Promise((resolve) => setImmediate(resolve)); service.pause(); await assert.rejects(request, SyncResetError); + const body = await bodyReady.promise; body.enqueue(new TextEncoder().encode("{}")); body.close(); }); } -test("pause waits for a checkpoint save already in progress", async () => { - const saving = Promise.withResolvers(); - const release = Promise.withResolvers(); +test("pause waits for a checkpoint save already in progress", async (): Promise => { + const saving = Promise.withResolvers(); + const release = Promise.withResolvers(); let settled = false; - let checkpoint: string | undefined; + let checkpoint: string | undefined = undefined; const service = new SyncService( "device", settings(), @@ -130,9 +143,9 @@ test("pause waits for a checkpoint save already in progress", async () => { } ), { - get: () => checkpoint, - save: async (value) => { - saving.resolve(); + get: (): string | undefined => checkpoint, + save: async (value): Promise => { + saving.resolve(undefined); await release.promise; checkpoint = value; } @@ -146,13 +159,13 @@ test("pause waits for a checkpoint save already in progress", async () => { service.pause(); await new Promise((resolve) => setImmediate(resolve)); assert.equal(settled, false); - release.resolve(); + release.resolve(undefined); await assert.rejects(request, SyncResetError); assert.equal(checkpoint, "1:head"); }); -test("history saves only new checkpoints, including a changed token at the same event ID", async () => { - let checkpoint: string | undefined; +test("history saves only new checkpoints, including a changed token at the same event ID", async (): Promise => { + let checkpoint: string | undefined = undefined; let responseCheckpoint = "1:first"; const saved: (string | undefined)[] = []; const service = new SyncService( @@ -164,8 +177,8 @@ test("history saves only new checkpoints, including a changed token at the same { headers: { "X-Vault-Link-History": responseCheckpoint } } ), { - get: () => checkpoint, - save: async (value) => { + get: (): string | undefined => checkpoint, + save: async (value): Promise => { checkpoint = value; saved.push(value); } @@ -183,6 +196,7 @@ test("history saves only new checkpoints, including a changed token at the same responseCheckpoint = value; await service.getEvents(0); } + assert.deepEqual(saved, ["1:first", "2:next", "2:replacement"]); }); @@ -195,9 +209,10 @@ for (const checkpoint of [undefined, ""]) { Response.json( { headEventId: 0, endEventId: 0, events: [] }, { - headers: checkpoint === undefined - ? {} - : { "X-Vault-Link-History": checkpoint } + headers: + checkpoint === undefined + ? {} + : { "X-Vault-Link-History": checkpoint } } ), { diff --git a/frontend/sync-client/src/services/sync-service.ts b/frontend/sync-client/src/services/sync-service.ts index aa9c2986..ab1fbab3 100644 --- a/frontend/sync-client/src/services/sync-service.ts +++ b/frontend/sync-client/src/services/sync-service.ts @@ -17,17 +17,19 @@ import { } from "../errors/errors"; export class SyncService { + private readonly fetchImplementation: typeof fetch; private session = new AbortController(); public constructor( private readonly deviceId: string, private readonly settings: Settings, - private readonly fetchImplementation: typeof fetch = globalThis.fetch, + fetchImplementation: typeof fetch | undefined, private readonly history: { get: () => string | undefined; save: (checkpoint: string | undefined) => Promise; } ) { + this.fetchImplementation = fetchImplementation ?? globalThis.fetch; this.pause(); } @@ -92,15 +94,21 @@ export class SyncService { private async recordHistory(response: Response): Promise { const checkpoint = response.headers.get("x-vault-link-history"); - if (checkpoint === null || checkpoint === "") + if (checkpoint === null || checkpoint === "") { throw new PermanentSyncError("Missing server history checkpoint"); + } + const previous = this.history.get(); - if (checkpoint === previous) return; + if (checkpoint === previous) { + return; + } + if ( previous === undefined || Number(checkpoint.split(":")[0]) >= Number(previous.split(":")[0]) - ) + ) { await this.history.save(checkpoint); + } } private async request( @@ -143,10 +151,11 @@ export class SyncService { ); signal.throwIfAborted(); - if (response.headers.get("x-vault-link-history-mismatch") === "1") + if (response.headers.get("x-vault-link-history-mismatch") === "1") { throw new ServerHistoryChangedError( "Server history changed; recovering local work" ); + } if (!response.ok) { this.throwForErrorResponse( @@ -156,7 +165,9 @@ export class SyncService { } // Finish metadata saves even if the transport is paused meanwhile. - if (!independent) await this.recordHistory(response); + if (!independent) { + await this.recordHistory(response); + } return abortable(signal, async () => decode(response)); } diff --git a/frontend/sync-client/src/services/websocket-manager.test.ts b/frontend/sync-client/src/services/websocket-manager.test.ts index 74cf2848..587c7da0 100644 --- a/frontend/sync-client/src/services/websocket-manager.test.ts +++ b/frontend/sync-client/src/services/websocket-manager.test.ts @@ -41,7 +41,7 @@ class MockWebSocket { public sentMessages: string[] = []; - public constructor(public url: string) { + public constructor(public url: string | URL) { setTimeout(() => { if (this.readyState === WebSocket.CONNECTING) { this.readyState = WebSocket.OPEN; @@ -54,6 +54,7 @@ class MockWebSocket { if (this.readyState !== WebSocket.OPEN) { throw new Error("WebSocket is not open"); } + this.sentMessages.push(data); } @@ -100,6 +101,7 @@ describe("WebSocketManager", () => { const noop = (): void => { // Intentionally empty for mock }; + mockLogger = { info: createMockFn(noop), warn: createMockFn(noop), @@ -344,9 +346,15 @@ it( server.listen(0, "127.0.0.1", resolve) ); const address = server.address(); - assert(address && typeof address !== "string"); + assert(address !== null && typeof address !== "string"); await new Promise((resolve, reject) => - server.close((error) => (error ? reject(error) : resolve())) + server.close((error) => { + if (error !== undefined) { + reject(error); + } else { + resolve(); + } + }) ); const logger = new Logger(); const settings = new Settings( @@ -355,7 +363,7 @@ it( remoteUri: `http://127.0.0.1:${address.port}`, webSocketRetryIntervalMs: 10 }, - async () => {} + async () => undefined ); const manager = new WebSocketManager( "test", @@ -364,9 +372,11 @@ it( WebSocket ); let closes = 0; - const disconnected = Promise.withResolvers(); + const disconnected = Promise.withResolvers(); manager.onWebSocketStatusChanged.add((connected) => { - if (!connected && ++closes >= 2) disconnected.resolve(); + if (!connected && ++closes >= 2) { + disconnected.resolve(undefined); + } }); try { manager.start(); @@ -384,10 +394,12 @@ for (const failedAttempt of [1, 2]) { context.mock.timers.enable({ apis: ["setTimeout"] }); let attempts = 0; class FailingSocket extends MockWebSocket { - static OPEN = 1; - constructor(url: string) { - if (++attempts === failedAttempt) + public static OPEN = 1; + public constructor(url: string) { + if (++attempts === failedAttempt) { throw new Error("socket allocation failed"); + } + super(url); } } @@ -395,7 +407,7 @@ for (const failedAttempt of [1, 2]) { const settings = new Settings( logger, { remoteUri: "http://test", webSocketRetryIntervalMs: 1000 }, - async () => {} + async () => undefined ); const manager = new WebSocketManager( "test", @@ -404,7 +416,9 @@ for (const failedAttempt of [1, 2]) { FailingSocket as unknown as typeof WebSocket ); try { - assert.doesNotThrow(() => manager.start()); + assert.doesNotThrow(() => { + manager.start(); + }); context.mock.timers.tick(0); if (failedAttempt === 2) { const socket = ( @@ -412,7 +426,10 @@ for (const failedAttempt of [1, 2]) { ).webSocket; socket.close(); } - assert.doesNotThrow(() => context.mock.timers.tick(1000)); + + assert.doesNotThrow(() => { + context.mock.timers.tick(1000); + }); context.mock.timers.tick(1000); assert.equal(attempts, failedAttempt + 1); assert.equal(manager.isWebSocketConnected, true); @@ -426,16 +443,18 @@ for (const closeBehavior of ["throws", "stays open"]) { it(`stop retires a socket even when close ${closeBehavior}`, async (context) => { context.mock.timers.enable({ apis: ["setTimeout"] }); class BrokenCloseSocket extends MockWebSocket { - static OPEN = 1; - close() { - if (closeBehavior === "throws") throw new Error("close failed"); + public static OPEN = 1; + public close(): void { + if (closeBehavior === "throws") { + throw new Error("close failed"); + } } } const logger = new Logger(); const settings = new Settings( logger, { remoteUri: "http://test" }, - async () => {} + async () => undefined ); const manager = new WebSocketManager( "test", @@ -481,20 +500,20 @@ it("stop retires the socket immediately but drains messages already being handle new Settings( logger, { remoteUri: "http://test", token: "test-token" }, - async () => {} + async () => undefined ), MockWebSocket as unknown as typeof WebSocket ); - const release = Promise.withResolvers(); + const release = Promise.withResolvers(); context.after(async () => { - release.resolve(); + release.resolve(undefined); await manager.stop(); }); manager.start(); context.mock.timers.tick(0); const socket = (manager as unknown as { webSocket: MockWebSocket }) .webSocket; - assert.deepEqual(JSON.parse(socket.sentMessages[0]), { + assert.deepEqual(JSON.parse(socket.sentMessages[0] ?? ""), { type: "handshake", token: "test-token", deviceId: "device" @@ -509,7 +528,7 @@ it("stop retires the socket immediately but drains messages already being handle assert.equal(socket.onmessage, null); await Promise.resolve(); assert(!finished); - release.resolve(); + release.resolve(undefined); await stopping; assert(finished); }); @@ -517,8 +536,8 @@ it("stop retires the socket immediately but drains messages already being handle it("a handshake send failure retires the connection without escaping the open callback", async (context) => { context.mock.timers.enable({ apis: ["setTimeout"] }); class BrokenSendSocket extends MockWebSocket { - static OPEN = 1; - send() { + public static OPEN = 1; + public send(): void { throw new Error("transport closed during open"); } } @@ -526,7 +545,7 @@ it("a handshake send failure retires the connection without escaping the open ca const settings = new Settings( logger, { remoteUri: "http://test" }, - async () => {} + async () => undefined ); const manager = new WebSocketManager( "test", @@ -538,7 +557,9 @@ it("a handshake send failure retires the connection without escaping the open ca manager.onWebSocketStatusChanged.add((status) => statuses.push(status)); try { manager.start(); - assert.doesNotThrow(() => context.mock.timers.tick(0)); + assert.doesNotThrow(() => { + context.mock.timers.tick(0); + }); assert.equal(manager.isWebSocketConnected, false); assert.equal(statuses.at(-1), false); assert( @@ -554,8 +575,8 @@ it("reconnects a connection that stops delivering messages", async (context) => context.mock.timers.enable({ apis: ["setTimeout"] }); let attempts = 0; class CountingSocket extends MockWebSocket { - static OPEN = 1; - constructor(url: string) { + public static OPEN = 1; + public constructor(url: string) { super(url); attempts++; } @@ -564,7 +585,7 @@ it("reconnects a connection that stops delivering messages", async (context) => const settings = new Settings( logger, { remoteUri: "http://test", webSocketRetryIntervalMs: 1000 }, - async () => {} + async () => undefined ); const manager = new WebSocketManager( "test", @@ -601,8 +622,8 @@ it("does not reconnect while messages keep arriving before the deadline", async context.mock.timers.enable({ apis: ["setTimeout"] }); let attempts = 0; class CountingSocket extends MockWebSocket { - static OPEN = 1; - constructor(url: string) { + public static OPEN = 1; + public constructor(url: string) { super(url); attempts++; } @@ -611,7 +632,7 @@ it("does not reconnect while messages keep arriving before the deadline", async const settings = new Settings( logger, { remoteUri: "http://test", webSocketRetryIntervalMs: 1000 }, - async () => {} + async () => undefined ); const manager = new WebSocketManager( "test", @@ -621,7 +642,9 @@ it("does not reconnect while messages keep arriving before the deadline", async ); let disconnected = 0; manager.onWebSocketStatusChanged.add((status) => { - if (!status) disconnected++; + if (!status) { + disconnected++; + } }); try { manager.start(); diff --git a/frontend/sync-client/src/services/websocket-manager.ts b/frontend/sync-client/src/services/websocket-manager.ts index 6fd328ae..b02f9a07 100644 --- a/frontend/sync-client/src/services/websocket-manager.ts +++ b/frontend/sync-client/src/services/websocket-manager.ts @@ -56,7 +56,10 @@ export class WebSocketManager { } public start(): void { - if (!this.isStopped) return; + if (!this.isStopped) { + return; + } + this.isStopped = false; this.initializeWebSocket(); } @@ -106,7 +109,10 @@ export class WebSocketManager { // proves nothing: silence past the heartbeat deadline means a // half-open connection. this.receiveTimeoutId = setTimeout(() => { - if (this.webSocket !== socket || this.isStopped) return; + if (this.webSocket !== socket || this.isStopped) { + return; + } + this.logger.warn("WebSocket receive deadline expired"); this.reconnect(socket); }, WEBSOCKET_RECEIVE_TIMEOUT_MS); @@ -117,7 +123,10 @@ export class WebSocketManager { private retireSocket(): void { clearTimeout(this.receiveTimeoutId); const socket = this.webSocket; - if (!socket) return; + if (!socket) { + return; + } + this.webSocket = undefined; socket.onopen = null; socket.onclose = null; @@ -125,6 +134,7 @@ export class WebSocketManager { socket.onerror = (): void => { /* Retired transports may still emit errors. */ }; + this.onWebSocketStatusChanged.trigger(false); try { socket.close(1000, "WebSocket retired"); @@ -134,13 +144,19 @@ export class WebSocketManager { } private reconnect(socket: WebSocket): void { - if (this.webSocket !== socket) return; + if (this.webSocket !== socket) { + return; + } + this.retireSocket(); this.scheduleReconnect(); } private scheduleReconnect(): void { - if (this.isStopped) return; + if (this.isStopped) { + return; + } + clearTimeout(this.reconnectTimeoutId); this.reconnectTimeoutId = setTimeout(() => { this.reconnectTimeoutId = undefined; @@ -149,7 +165,10 @@ export class WebSocketManager { } private initializeWebSocket(): void { - if (this.isStopped) return; + if (this.isStopped) { + return; + } + try { this.openWebSocket(); } catch (error) { @@ -183,7 +202,10 @@ export class WebSocketManager { }; socket.onopen = (): void => { - if (this.webSocket !== socket || this.isStopped) return; + if (this.webSocket !== socket || this.isStopped) { + return; + } + try { const handshake: WebSocketClientMessage = { type: "handshake", @@ -194,8 +216,9 @@ export class WebSocketManager { this.logger.info("WebSocket connection opened"); this.onWebSocketStatusChanged.trigger(true); // An observer may stop or replace the connection during delivery. - if (this.webSocket !== socket) + if (this.webSocket !== socket) { this.onWebSocketStatusChanged.trigger(false); + } } catch (error) { this.logger.warn( `WebSocket handshake failed: ${String(error)}` @@ -205,15 +228,22 @@ export class WebSocketManager { }; socket.onmessage = (event): void => { - if (this.webSocket !== socket || this.isStopped) return; + if (this.webSocket !== socket || this.isStopped) { + return; + } + try { // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion const message = JSON.parse( event.data ) as WebSocketServerMessage; - if (["vaultChanged", "cursorPositions"].includes(message.type)) + if ( + ["vaultChanged", "cursorPositions"].includes(message.type) + ) { this.expectResponse(socket); + } + // Track the message handling promise const messageHandlingPromise = this.handleWebSocketMessage( message diff --git a/frontend/sync-client/src/sync-client.test.ts b/frontend/sync-client/src/sync-client.test.ts new file mode 100644 index 00000000..14ffc4db --- /dev/null +++ b/frontend/sync-client/src/sync-client.test.ts @@ -0,0 +1,41 @@ +/* eslint-disable @typescript-eslint/no-unsafe-type-assertion -- No filesystem operation is reached before initialization. */ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { SyncClient } from "./sync-client"; +import type { FileSystemOperations } from "./file-operations/filesystem-operations"; +import { awaitAll } from "./utils/await-all"; + +test("destroy during startup metadata work cannot restart the client", async () => { + const saving = Promise.withResolvers(); + const release = Promise.withResolvers(); + const client = await SyncClient.create({ + fs: {} as FileSystemOperations, + persistence: { + load: async () => ({ + settings: { + isSyncEnabled: true, + enableTelemetry: false, + remoteUri: "http://test" + } + }), + save: async () => { + saving.resolve(undefined); + await release.promise; + } + }, + fetch: async () => { + assert.fail("a destroyed client must not start network requests"); + } + }); + + const notification = client.syncLocallyCreatedFile("note.md"); + await saving.promise; + const starting = client.start(); + // Let start acquire the lifecycle lock and begin draining notification saves. + await Promise.resolve(); + const destroying = client.destroy(); + release.resolve(undefined); + await awaitAll([notification, starting, destroying]); + + await assert.rejects(client.start(), /destroyed/u); +}); diff --git a/frontend/sync-client/src/sync-client.ts b/frontend/sync-client/src/sync-client.ts index ded8311f..d650d55f 100644 --- a/frontend/sync-client/src/sync-client.ts +++ b/frontend/sync-client/src/sync-client.ts @@ -1,3 +1,5 @@ +import type { NetworkConnectionStatus } from "./types/network-connection-status"; +import { ClientMetadataStore } from "./persistence/client-metadata-store"; import * as Sentry from "@sentry/browser"; import { setUpTelemetry } from "./utils/set-up-telemetry"; import type { MetadataPersistenceProvider } from "./persistence/metadata-persistence-provider"; @@ -28,7 +30,7 @@ import { isInternalPath } from "./utils/portable-path"; import { FixedSizeDocumentCache } from "./utils/data-structures/fix-sized-cache"; import { Lock } from "./utils/data-structures/locks"; import { - unappliedChanges, + getUnappliedChanges, type LocalChange } from "./sync-operations/local-changes"; @@ -40,17 +42,22 @@ export type StoredClient = Partial<{ historyCheckpoint: { vaultKey: string; checkpoint?: string }; }>; -const vaultKey = (settings: SyncSettings): string => +const getVaultKey = (settings: SyncSettings): string => JSON.stringify([ settings.remoteUri.replace(/\/$/u, ""), settings.vaultName ]); +enum ClientPhase { + Created = "created", + Started = "started", + Destroyed = "destroyed" +} + export class SyncClient { private readonly lifecycle = new Lock(); private destroying?: Promise; - private started = false; - private destroyed = false; + private phase = ClientPhase.Created; private unloadTelemetry?: () => void; private readonly unsubscribeLog: () => void; private constructor( @@ -74,18 +81,47 @@ export class SyncClient { }); syncer.onReadyForEvents.add(() => { if ( - this.started && - !this.destroyed && + this.phase === ClientPhase.Started && settings.getSettings().isSyncEnabled - ) + ) { webSocketManager.start(); + } }); this.unsubscribeLog = logger.onLogEmitted.add((log) => { - if (log.level === LogLevel.ERROR && Sentry.isInitialized()) + if (log.level === LogLevel.ERROR && Sentry.isInitialized()) { Sentry.captureMessage(log.message); + } }); } + public get documentCount(): number { + return this.database.length; + } + + public get isWebSocketConnected(): boolean { + return this.webSocketManager.isWebSocketConnected; + } + + public get onSyncHistoryUpdated(): SyncHistory["onHistoryUpdated"] { + return this.history.onHistoryUpdated; + } + + public get onSettingsChanged(): Settings["onSettingsChanged"] { + return this.settings.onSettingsChanged; + } + + public get onRemainingOperationsCountChanged(): Syncer["onRemainingOperationsCountChanged"] { + return this.syncer.onRemainingOperationsCountChanged; + } + + public get onWebSocketStatusChanged(): WebSocketManager["onWebSocketStatusChanged"] { + return this.webSocketManager.onWebSocketStatusChanged; + } + + public get onRemoteCursorsUpdated(): CursorTracker["onRemoteCursorsUpdated"] { + return this.cursorTracker.onRemoteCursorsUpdated; + } + public static async create({ fs, persistence, @@ -98,65 +134,43 @@ export class SyncClient { webSocket?: typeof globalThis.WebSocket; }): Promise { const logger = new Logger(); - let stored = (await persistence.load()) ?? {}; - const persistenceLock = new Lock(); - let reloadBeforeSave = false; - // Settings and engine state share a store. Serialize complete metadata - // replacements so one cannot overwrite a newer save from the other. + const metadata = await ClientMetadataStore.load(persistence); + const { stored } = metadata; const save = async (update: StoredClient): Promise => - persistenceLock.withLock(async () => { - if (reloadBeforeSave) { - stored = (await persistence.load()) ?? {}; - reloadBeforeSave = false; - } - const next = { ...stored, ...structuredClone(update) }; - try { - await persistence.save(next); - stored = next; - } catch (error) { - // A failed save may have committed. Do not let a queued - // settings/notification save roll back that engine state. - reloadBeforeSave = true; - throw error; - } - }); + metadata.save(update); const settings = new Settings(logger, stored.settings, async (data) => save({ settings: data }) ); const database = new Database( stored.database, async (data) => save({ database: data }), - vaultKey(settings.getSettings()), - async () => - persistenceLock.withLock(async () => { - stored = (await persistence.load()) ?? {}; - reloadBeforeSave = false; - return stored.database; - }) + getVaultKey(settings.getSettings()), + async () => metadata.reloadDatabase() ); - const localChanges = unappliedChanges( + const localChanges = getUnappliedChanges( database.state, stored.localChanges ?? [] ); if ( localChanges.length && - stored.localChangesVaultKey !== vaultKey(settings.getSettings()) + stored.localChangesVaultKey !== getVaultKey(settings.getSettings()) ) { throw new Error( "Offline notifications belong to another vault; use a separate state store" ); } + const deviceId = createClientId(); const service = new SyncService(deviceId, settings, fetch, { - get: () => - stored.historyCheckpoint?.vaultKey === - vaultKey(settings.getSettings()) - ? stored.historyCheckpoint.checkpoint + get: (): string | undefined => + metadata.stored.historyCheckpoint?.vaultKey === + getVaultKey(settings.getSettings()) + ? metadata.stored.historyCheckpoint.checkpoint : undefined, - save: async (checkpoint) => + save: async (checkpoint): Promise => save({ historyCheckpoint: { - vaultKey: vaultKey(settings.getSettings()), + vaultKey: getVaultKey(settings.getSettings()), checkpoint } }) @@ -168,11 +182,15 @@ export class SyncClient { database, serverConfig, (paths) => { - for (const path of paths) notifier.trigger(path); + for (const path of paths) { + notifier.trigger(path); + } }, { - entries: () => localChanges, - flush: async () => syncer.flushLocalChanges() + entries: (): LocalChange[] => localChanges, + // Invoked after all components have been constructed. + // eslint-disable-next-line @typescript-eslint/no-use-before-define + flush: async (): Promise => syncer.flushLocalChanges() }, (path, size) => settings.isIgnored(path) || settings.isOversized(size) @@ -202,7 +220,9 @@ export class SyncClient { save: async (entries) => save({ localChanges: entries, - localChangesVaultKey: vaultKey(settings.getSettings()) + localChangesVaultKey: getVaultKey( + settings.getSettings() + ) }) } ); @@ -223,39 +243,7 @@ export class SyncClient { ); } - private check(): void { - if (this.destroyed) throw new Error("SyncClient has been destroyed"); - } - - public get documentCount(): number { - return this.database.length; - } - - public get isWebSocketConnected(): boolean { - return this.webSocketManager.isWebSocketConnected; - } - - public get onSyncHistoryUpdated() { - return this.history.onHistoryUpdated; - } - - public get onSettingsChanged() { - return this.settings.onSettingsChanged; - } - - public get onRemainingOperationsCountChanged() { - return this.syncer.onRemainingOperationsCountChanged; - } - - public get onWebSocketStatusChanged() { - return this.webSocketManager.onWebSocketStatusChanged; - } - - public get onRemoteCursorsUpdated() { - return this.cursorTracker.onRemoteCursorsUpdated; - } - - public getHistoryEntries() { + public getHistoryEntries(): SyncHistory["entries"] { return this.history.entries; } @@ -265,43 +253,31 @@ export class SyncClient { public async start(): Promise { await this.lifecycle.withLock(async () => { - this.check(); - if (this.started) + this.assertNotDestroyed(); + if (this.phase === ClientPhase.Started) { throw new Error("SyncClient has already been started"); + } + await this.syncer.reloadLocalState(); await this.database.bindVault( - vaultKey(this.settings.getSettings()) + getVaultKey(this.settings.getSettings()) ); - this.started = true; + // destroy() can interrupt the metadata work above before acquiring + // the lifecycle lock. Its terminal state must not be overwritten. + if (this.phase === ClientPhase.Destroyed) { + return; + } + + this.phase = ClientPhase.Started; this.configureTelemetry(); await this.resume(); }); } - private async resume(): Promise { - if ( - !this.started || - this.destroyed || - !this.settings.getSettings().isSyncEnabled - ) - return; - await this.database.bindVault(vaultKey(this.settings.getSettings())); - this.service.resume(); - this.syncer.start(); - await this.syncer.waitUntilFinished(); - } - - private async pause(): Promise { - const stopped = this.syncer.stop(); - this.service.pause(); - await this.webSocketManager.stop(); - await stopped; - } - /** Restart transports; never discard pending requests. */ public async reset(): Promise { await this.lifecycle.withLock(async () => { - this.check(); + this.assertNotDestroyed(); await this.pause(); this.serverConfig.reset(); this.cursorTracker.reset(); @@ -313,28 +289,32 @@ export class SyncClient { public async setSettings(change: Partial): Promise { const update = structuredClone(change); const notification = await this.lifecycle.withLock(async () => { - this.check(); + this.assertNotDestroyed(); const old = this.settings.getSettings(); const next = { ...old, ...update }; if ( - vaultKey(next) !== vaultKey(old) && - (this.started || + getVaultKey(next) !== getVaultKey(old) && + (this.phase === ClientPhase.Started || this.database.state.lastSeenUpdateId !== undefined || this.database.state.pending || this.syncer.hasLocalChanges || Object.keys(this.database.state.local).length) - ) + ) { throw new Error( "Changing vaults requires a separate state store and client; existing sync state must not be reset" ); + } try { - if (this.started) await this.pause(); + if (this.phase === ClientPhase.Started) { + await this.pause(); + } + await this.database.recoverPersistence(); await this.settings.setSettings(update, false); await this.syncer.retryRejectedRequests(); this.contentCache.resize(next.diffCacheSizeMB * 1024 * 1024); - await this.database.bindVault(vaultKey(next)); + await this.database.bindVault(getVaultKey(next)); } catch (error) { this.serverConfig.reset(); this.contentCache.resize( @@ -348,6 +328,7 @@ export class SyncClient { }); throw error; } + this.serverConfig.reset(); this.configureTelemetry(); await this.resume(); @@ -368,7 +349,7 @@ export class SyncClient { await this.setSettings({ ...DEFAULT_SETTINGS, ...stored?.settings }); } - public async checkConnection() { + public async checkConnection(): Promise { const result = await this.serverConfig.checkConnection(); return { isSuccessful: result.isSuccessful, @@ -379,15 +360,21 @@ export class SyncClient { /** Notifications enqueue work. Use waitUntilFinished() to await convergence. */ public async syncLocallyCreatedFile(path: RelativePath): Promise { - this.check(); - if (isInternalPath(path)) return; + this.assertNotDestroyed(); + if (isInternalPath(path)) { + return; + } + this.fileChanges.trigger(path); await this.syncer.syncLocallyCreatedFile(path); } public async syncLocallyDeletedFile(path: RelativePath): Promise { - this.check(); - if (isInternalPath(path)) return; + this.assertNotDestroyed(); + if (isInternalPath(path)) { + return; + } + this.fileChanges.trigger(path); await this.syncer.syncLocallyDeletedFile(path); } @@ -396,12 +383,16 @@ export class SyncClient { oldPath?: RelativePath; relativePath: RelativePath; }): Promise { - this.check(); + this.assertNotDestroyed(); if ( isInternalPath(change.relativePath) || - (change.oldPath && isInternalPath(change.oldPath)) - ) + (change.oldPath !== undefined && + change.oldPath !== "" && + isInternalPath(change.oldPath)) + ) { return; + } + this.fileChanges.trigger(change.relativePath); await this.syncer.syncLocallyUpdatedFile(change); } @@ -409,39 +400,32 @@ export class SyncClient { public async updateLocalCursors( cursors: Record ): Promise { - this.check(); + this.assertNotDestroyed(); await this.cursorTracker.sendLocalCursorsToServer(cursors); } public getDocumentSyncingStatus(path: RelativePath): DocumentSyncStatus { - this.check(); - if (!this.settings.getSettings().isSyncEnabled) + this.assertNotDestroyed(); + if (!this.settings.getSettings().isSyncEnabled) { return DocumentSyncStatus.SYNCING_IS_DISABLED; + } + return this.syncer.isDocumentUpToDate(path) ? DocumentSyncStatus.UP_TO_DATE : DocumentSyncStatus.SYNCING; } public async waitUntilFinished(): Promise { - this.check(); + this.assertNotDestroyed(); await this.syncer.waitUntilFinished(); } - private configureTelemetry(): void { - if ( - this.settings.getSettings().enableTelemetry && - typeof window !== "undefined" - ) { - this.unloadTelemetry ??= setUpTelemetry(); - } else { - this.unloadTelemetry?.(); - this.unloadTelemetry = undefined; - } - } - public async destroy(): Promise { - if (this.destroying) return this.destroying; - this.destroyed = true; + if (this.destroying) { + return this.destroying; + } + + this.phase = ClientPhase.Destroyed; // Interrupt a start/reset awaiting network progress before waiting for // its lifecycle operation to finish. this.service.pause(); @@ -459,4 +443,43 @@ export class SyncClient { }); return this.destroying; } + + private assertNotDestroyed(): void { + if (this.phase === ClientPhase.Destroyed) { + throw new Error("SyncClient has been destroyed"); + } + } + + private async resume(): Promise { + if ( + this.phase !== ClientPhase.Started || + !this.settings.getSettings().isSyncEnabled + ) { + return; + } + + await this.database.bindVault(getVaultKey(this.settings.getSettings())); + this.service.resume(); + this.syncer.start(); + await this.syncer.waitUntilFinished(); + } + + private async pause(): Promise { + const stopped = this.syncer.stop(); + this.service.pause(); + await this.webSocketManager.stop(); + await stopped; + } + + private configureTelemetry(): void { + if ( + this.settings.getSettings().enableTelemetry && + typeof window !== "undefined" + ) { + this.unloadTelemetry ??= setUpTelemetry(); + } else { + this.unloadTelemetry?.(); + this.unloadTelemetry = undefined; + } + } } diff --git a/frontend/sync-client/src/sync-operations/content-sync.ts b/frontend/sync-client/src/sync-operations/content-sync.ts new file mode 100644 index 00000000..c52ed752 --- /dev/null +++ b/frontend/sync-client/src/sync-operations/content-sync.ts @@ -0,0 +1,245 @@ +import { base64ToBytes, bytesToBase64 } from "byte-base64"; +import { diff, undiff } from "reconcile-text"; +import type { + Database, + StoredDatabase, + PendingRequest, + ContentHead, + DocumentId, + RelativePath, + VaultUpdateId +} from "../persistence/database"; +import { hashSnapshot, type HashedSnapshot } from "../snapshot"; +import type { + FileOperations, + FileWrite +} from "../file-operations/file-operations"; +import type { Settings } from "../persistence/settings"; +import type { SyncService } from "../services/sync-service"; +import type { ServerConfig } from "../services/server-config"; +import type { DocumentVersionWithoutContent } from "../services/types/DocumentVersionWithoutContent"; +import type { PushContent } from "../services/types/PushContent"; +import type { FixedSizeDocumentCache } from "../utils/data-structures/fix-sized-cache"; +import { isBinary } from "../utils/is-binary"; +import { isFileTypeMergeable } from "../utils/is-file-type-mergeable"; +import { decodeText } from "../utils/decode-text"; +import { + LocalChangesDuringReconciliation, + PermanentSyncError +} from "../errors/errors"; +import { mergeContent } from "./content"; + +/** Store only the remote version metadata, never response content or cursors. */ +export function selectContentHead(version: ContentHead): ContentHead { + return { + vaultUpdateId: version.vaultUpdateId, + contentSize: version.contentSize + }; +} + +/** Fetch and merge content, and encode uploads using the optional diff cache. */ +export class ContentSync { + public constructor( + private readonly database: Database, + private readonly settings: Settings, + private readonly service: SyncService, + private readonly files: FileOperations, + private readonly serverConfig: ServerConfig, + private readonly contentCache: FixedSizeDocumentCache + ) {} + + public async createUploadPayload( + path: RelativePath, + parentVersionId: VaultUpdateId | undefined, + snapshot: HashedSnapshot + ): Promise { + const { content } = snapshot; + const parent = + parentVersionId === undefined + ? undefined + : this.contentCache.get(parentVersionId); + if ( + parent !== undefined && + !isBinary(content) && + !isBinary(parent) && + isFileTypeMergeable( + path, + (await this.serverConfig.getConfig()).mergeableFileExtensions + ) + ) { + const value = diff(decodeText(parent), decodeText(content)); + // Match the server's work bound; large edit sets remain syncable. + if (value.length <= 10_000) { + return { type: "Diff", value }; + } + } + + return { type: "Snapshot", value: bytesToBase64(content) }; + } + + public async fetchSnapshot( + head: Pick< + DocumentVersionWithoutContent, + "documentId" | "vaultUpdateId" + >, + path: RelativePath + ): Promise { + const content = await this.service.getDocumentVersionContent(head); + await this.cacheContent(head.vaultUpdateId, path, content); + return hashSnapshot({ content }); + } + + public async planDownload( + next: StoredDatabase, + id: DocumentId, + path: RelativePath + ): Promise { + const document = (next.documents[id] ??= {}); + const currentPath = this.database.state.local[id]; + if (currentPath === undefined) { + delete document.observedHash; + } + + if ( + document.observedHash !== undefined && + document.remote && + document.remote.vaultUpdateId <= (document.base?.vaultUpdateId ?? 0) + ) { + delete document.remote; + } + + if ( + next.excluded?.[id] !== undefined || + this.settings.isIgnored(path) || + (next.fileManifest.entries[id] === undefined && !document.remote) || + (currentPath !== undefined && + currentPath !== "" && + document.base && + document.observedHash !== undefined && + !document.remote) + ) { + return; + } + + const head = + document.remote ?? (await this.service.getDocumentMetadata(id)); + // The transport is injectable; keep the runtime check for malformed responses. + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition, @typescript-eslint/strict-boolean-expressions + if (!head) { + throw new PermanentSyncError( + `File manifest references missing content: ${id}` + ); + } + + document.remote = selectContentHead(head); + if (this.settings.isOversized(head.contentSize)) { + return; + } + + const local = + currentPath !== undefined && + currentPath !== "" && + (await this.files.fs.stat(currentPath))?.kind === "file" + ? await this.files.readSnapshot(currentPath) + : undefined; + if ( + local !== undefined && + this.settings.isOversized(local.content.length) + ) { + return undefined; + } + + if ( + document.base?.vaultUpdateId === null && + local && + local.hash !== document.base.hash + ) { + throw new LocalChangesDuringReconciliation(); + } + + const base = + local && document.base + ? document.base.vaultUpdateId === null + ? local + : await this.fetchSnapshot( + { + documentId: id, + vaultUpdateId: document.base.vaultUpdateId + }, + path + ) + : undefined; + // Cache the new head after the older merge base so it remains the most + // recently used version and can back the follow-up local diff. + const remote = await this.fetchSnapshot( + { documentId: id, vaultUpdateId: head.vaultUpdateId }, + path + ); + let result = remote; + if (local !== undefined) { + result = await mergeContent( + path, + base, + local, + remote, + (await this.serverConfig.getConfig()).mergeableFileExtensions + ); + } + + document.base = { + vaultUpdateId: head.vaultUpdateId, + hash: remote.hash + }; + delete document.remote; + document.observedHash = result.hash; + return local?.hash === result.hash + ? undefined + : { expected: local, replacement: result }; + } + + public async cacheAcceptedUpload( + pending: Extract, + version: VaultUpdateId + ): Promise { + const { content, parentVersionId } = pending.request; + let bytes: Uint8Array | undefined = undefined; + if (content.type === "Snapshot") { + bytes = base64ToBytes(content.value); + } else { + const parent = + parentVersionId === null + ? undefined + : this.contentCache.get(parentVersionId); + // A restart may have evicted the parent. Future uploads can use snapshots. + if (!parent) { + return; + } + + const text = decodeText(parent); + bytes = new TextEncoder().encode(undiff(text, content.value)); + } + + await this.cacheContent( + version, + this.database.state.local[pending.documentId], + bytes + ); + } + + private async cacheContent( + updateId: VaultUpdateId, + path: RelativePath | undefined, + content: Uint8Array + ): Promise { + if ( + path !== undefined && + !isBinary(content) && + isFileTypeMergeable( + path, + (await this.serverConfig.getConfig()).mergeableFileExtensions + ) + ) { + this.contentCache.put(updateId, content); + } + } +} diff --git a/frontend/sync-client/src/sync-operations/content.test.ts b/frontend/sync-client/src/sync-operations/content.test.ts index bb5f227f..6a194e3a 100644 --- a/frontend/sync-client/src/sync-operations/content.test.ts +++ b/frontend/sync-client/src/sync-operations/content.test.ts @@ -1,10 +1,10 @@ import assert from "node:assert/strict"; import { describe, it } from "node:test"; -import { hashSnapshot } from "../snapshot"; +import { hashSnapshot, type HashedSnapshot } from "../snapshot"; import { mergeContent } from "./content"; const extensions = ["md"]; -const snapshot = async (text: string) => +const createSnapshot = async (text: string): Promise => hashSnapshot({ content: new TextEncoder().encode(text) }); describe("content reconciliation without file timestamps", () => { @@ -21,8 +21,8 @@ describe("content reconciliation without file timestamps", () => { }); it("keeps local binary edits when the server content is unchanged", async () => { - const base = await snapshot("\0base"); - const local = await snapshot("\0local"); + const base = await createSnapshot("\0base"); + const local = await createSnapshot("\0local"); const merged = await mergeContent( "file.bin", base, @@ -34,8 +34,8 @@ describe("content reconciliation without file timestamps", () => { }); it("incorporates server binary edits when local content is unchanged", async () => { - const base = await snapshot("\0base"); - const remote = await snapshot("\0remote"); + const base = await createSnapshot("\0base"); + const remote = await createSnapshot("\0remote"); const merged = await mergeContent( "file.bin", base, @@ -51,10 +51,10 @@ describe("content reconciliation without file timestamps", () => { ["file.bin", "\0"], ["file.md", "\0"], ["file.json", ""] - ]) { - const base = await snapshot(`${prefix}base`); - const local = await snapshot(`${prefix}local`); - const remote = await snapshot(`${prefix}remote`); + ] as const) { + const base = await createSnapshot(`${prefix}base`); + const local = await createSnapshot(`${prefix}local`); + const remote = await createSnapshot(`${prefix}remote`); const merged = await mergeContent( path, base, @@ -67,8 +67,8 @@ describe("content reconciliation without file timestamps", () => { }); it("keeps the server for differing binary content without a common base", async () => { - const local = await snapshot("\0local"); - const remote = await snapshot("\0remote"); + const local = await createSnapshot("\0local"); + const remote = await createSnapshot("\0remote"); const merged = await mergeContent( "file.bin", undefined, @@ -82,9 +82,13 @@ describe("content reconciliation without file timestamps", () => { it("still merges independent text edits from both sides", async () => { const merged = await mergeContent( "note.md", - await snapshot("First paragraph.\n\nLast paragraph.\n"), - await snapshot("Updated first paragraph.\n\nLast paragraph.\n"), - await snapshot("First paragraph.\n\nUpdated last paragraph.\n"), + await createSnapshot("First paragraph.\n\nLast paragraph.\n"), + await createSnapshot( + "Updated first paragraph.\n\nLast paragraph.\n" + ), + await createSnapshot( + "First paragraph.\n\nUpdated last paragraph.\n" + ), extensions ); assert.equal( diff --git a/frontend/sync-client/src/sync-operations/content.ts b/frontend/sync-client/src/sync-operations/content.ts index a44e0bb0..866b5994 100644 --- a/frontend/sync-client/src/sync-operations/content.ts +++ b/frontend/sync-client/src/sync-operations/content.ts @@ -12,15 +12,20 @@ export async function mergeContent( remote: HashedSnapshot, extensions: string[] ): Promise { - if (local.hash === remote.hash) + if (local.hash === remote.hash) { return { ...remote, cursors: local.cursors }; + } - if (!local.cursors && local.hash === base?.hash) return remote; + if (!local.cursors && local.hash === base?.hash) { + return remote; + } - if (remote.hash === base?.hash) return local; + if (remote.hash === base?.hash) { + return local; + } const inputs = [ base?.content ?? new Uint8Array(), @@ -29,10 +34,13 @@ export async function mergeContent( ]; // Concurrent unmergeable edits keep the server's content. - if (!isFileTypeMergeable(path, extensions) || inputs.some(isBinary)) + if (!isFileTypeMergeable(path, extensions) || inputs.some(isBinary)) { return remote; + } - const [parentText, localText, remoteText] = inputs.map(decodeText); + const parentText = decodeText(base?.content ?? new Uint8Array()); + const localText = decodeText(local.content); + const remoteText = decodeText(remote.content); const merged = reconcile( parentText, diff --git a/frontend/sync-client/src/sync-operations/cursor-tracker.test.ts b/frontend/sync-client/src/sync-operations/cursor-tracker.test.ts index 57ac5a1b..23377cf8 100644 --- a/frontend/sync-client/src/sync-operations/cursor-tracker.test.ts +++ b/frontend/sync-client/src/sync-operations/cursor-tracker.test.ts @@ -3,7 +3,7 @@ import assert from "node:assert/strict"; import { it } from "node:test"; import { Database, - emptyState, + createEmptyDatabase, type RelativePath, type VaultUpdateId } from "../persistence/database"; @@ -15,7 +15,7 @@ import { hash } from "../utils/hash"; import { CursorTracker } from "./cursor-tracker"; function createDatabase(digest: string): Database { - const state = emptyState("test"); + const state = createEmptyDatabase("test"); state.local.document = "note.md"; state.documents.document = { base: { vaultUpdateId: 1, hash: digest } }; return new Database( @@ -35,7 +35,7 @@ it("keeps the last usable cursor while retaining only the latest future update", onRemoteCursorsUpdateReceived: new EventListeners< (cursors: ClientCursors[]) => Promise >(), - updateLocalCursors: () => {} + updateLocalCursors: () => undefined } as unknown as WebSocketManager; const notifier = new EventListeners<(path: RelativePath) => unknown>(); const tracker = new CursorTracker( @@ -44,7 +44,9 @@ it("keeps the last usable cursor while retaining only the latest future update", { read: async () => ({ content }) } as unknown as FileSystemOperations, notifier ); - context.after(() => tracker.reset()); + context.after(() => { + tracker.reset(); + }); let received: ClientCursors[] = []; tracker.onRemoteCursorsUpdated.add((cursors) => { received = cursors; @@ -65,15 +67,17 @@ it("keeps the last usable cursor while retaining only the latest future update", await websocket.onRemoteCursorsUpdateReceived.triggerAsync([ cursor(remoteVersion) ]); - assert.equal(received[0].documentsWithCursors[0].vault_update_id, 1); + assert.equal(received[0]?.documentsWithCursors[0]?.vault_update_id, 1); } + + assert.ok(database.state.documents.document); database.state.documents.document.base = { vaultUpdateId: 2, hash: contentHash }; await notifier.triggerAsync("note.md"); assert.equal( - received[0].documentsWithCursors[0].vault_update_id, + received[0]?.documentsWithCursors[0]?.vault_update_id, 1, "superseded future cursors must not reappear" ); @@ -128,6 +132,7 @@ it( // cannot guarantee that the cursor snapshot has been published. await published.promise; }; + try { await tracker.sendLocalCursorsToServer({ "note.md": [{ start: 1, end: 2 }] @@ -149,11 +154,11 @@ it( database.state.local.document = path; await notifier.triggerAsync(path); assert.equal( - sent.at(-1)!.documentsWithCursors[0].relative_path, + sent.at(-1)?.documentsWithCursors[0]?.relative_path, path ); websocket.onWebSocketStatusChanged.trigger(false); - const count = sent.length; + const count = Number(sent.length); const readCount = read.mock.callCount(); context.mock.timers.tick(30_000); // A surviving heartbeat would start a snapshot read in the send lock's @@ -176,8 +181,8 @@ for (const interruption of ["disconnect", "reset"] as const) { it(`an in-flight cursor read cannot resurrect presence after ${interruption}`, async () => { const content = new TextEncoder().encode("current"); const contentHash = await hash(content); - const entered = Promise.withResolvers(); - const release = Promise.withResolvers(); + const entered = Promise.withResolvers(); + const release = Promise.withResolvers(); const database = createDatabase(contentHash); const websocket = { onWebSocketStatusChanged: new EventListeners< @@ -186,14 +191,14 @@ for (const interruption of ["disconnect", "reset"] as const) { onRemoteCursorsUpdateReceived: new EventListeners< (cursors: ClientCursors[]) => Promise >(), - updateLocalCursors: () => {} + updateLocalCursors: () => undefined } as unknown as WebSocketManager; const tracker = new CursorTracker( database, websocket, { read: async () => { - entered.resolve(); + entered.resolve(undefined); await release.promise; return { content }; } @@ -223,10 +228,14 @@ for (const interruption of ["disconnect", "reset"] as const) { const queued = websocket.onRemoteCursorsUpdateReceived.triggerAsync([ cursor ]); - if (interruption === "reset") tracker.reset(); - else websocket.onWebSocketStatusChanged.trigger(false); + if (interruption === "reset") { + tracker.reset(); + } else { + websocket.onWebSocketStatusChanged.trigger(false); + } + assert.deepEqual(received, []); - release.resolve(); + release.resolve(undefined); await first; await queued; assert.deepEqual( @@ -241,8 +250,8 @@ for (const interruption of ["disconnect", "reset"] as const) { it("a remote cursor becomes outdated if the document advances during its snapshot read", async () => { const content = new TextEncoder().encode("old"); const database = createDatabase(await hash(content)); - const entered = Promise.withResolvers(), - release = Promise.withResolvers(); + const entered = Promise.withResolvers(), + release = Promise.withResolvers(); const websocket = { onWebSocketStatusChanged: new EventListeners< (connected: boolean) => unknown @@ -250,21 +259,21 @@ it("a remote cursor becomes outdated if the document advances during its snapsho onRemoteCursorsUpdateReceived: new EventListeners< (cursors: ClientCursors[]) => Promise >(), - updateLocalCursors: () => {} + updateLocalCursors: () => undefined } as unknown as WebSocketManager; const tracker = new CursorTracker( database, websocket, { read: async () => { - entered.resolve(); + entered.resolve(undefined); await release.promise; return { content }; } } as unknown as FileSystemOperations, new EventListeners<(path: RelativePath) => unknown>() ); - let outdated: boolean | undefined; + let outdated: boolean | undefined = undefined; tracker.onRemoteCursorsUpdated.add((cursors) => { outdated = cursors[0]?.isOutdated; }); @@ -283,11 +292,12 @@ it("a remote cursor becomes outdated if the document advances during its snapsho } ]); await entered.promise; + assert.ok(database.state.documents.document); database.state.documents.document.base = { vaultUpdateId: 2, hash: await hash(new TextEncoder().encode("new content")) }; - release.resolve(); + release.resolve(undefined); await update; assert.equal(outdated, true); tracker.reset(); diff --git a/frontend/sync-client/src/sync-operations/cursor-tracker.ts b/frontend/sync-client/src/sync-operations/cursor-tracker.ts index 599e83eb..98fc3ee5 100644 --- a/frontend/sync-client/src/sync-operations/cursor-tracker.ts +++ b/frontend/sync-client/src/sync-operations/cursor-tracker.ts @@ -1,4 +1,3 @@ -/* eslint-disable @typescript-eslint/no-unnecessary-condition -- Persisted document and path maps are sparse at runtime. */ import type { FileSystemOperations } from "../file-operations/filesystem-operations"; import type { Database, @@ -47,25 +46,31 @@ export class CursorTracker { this.reset(); return; } + this.resetSession(); - const refresh = () => { + const refresh = (): void => { void this.publishLocalCursors(true).catch(() => { // A concurrent move/deletion can invalidate an editor view; // the next notification or heartbeat retries the snapshot. }); }; + refresh(); this.cursorTimer = setInterval( refresh, CURSOR_HEARTBEAT_INTERVAL_MS ); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- Browser timers have no unref method. this.cursorTimer.unref?.(); }); this.webSocketManager.onRemoteCursorsUpdateReceived.add( async (clientCursors) => { const generation = this.sessionGeneration; await this.updateLock.withLock(async () => { - if (generation !== this.sessionGeneration) return; + if (generation !== this.sessionGeneration) { + return; + } + // The latest message will contain all active clients, so we can delete the ones // from the local list which are no longer active. const allIds = new Set( @@ -84,7 +89,10 @@ export class CursorTracker { }); } - if (generation !== this.sessionGeneration) return; + if (generation !== this.sessionGeneration) { + return; + } + this.knownRemoteCursors = updatedKnownRemoteCursors; this.onRemoteCursorsUpdated.trigger( this.getRelevantAndPruneKnownClientCursors() @@ -97,7 +105,10 @@ export class CursorTracker { const generation = this.sessionGeneration; await this.publishLocalCursors(true); const cursors = await this.updateLock.withLock(async () => { - if (generation !== this.sessionGeneration) return []; + if (generation !== this.sessionGeneration) { + return []; + } + for (const clientCursor of this.knownRemoteCursors) { if ( clientCursor.documentsWithCursors.some( @@ -112,13 +123,15 @@ export class CursorTracker { await this.getDocumentsUpToDateness(clientCursor); } } + return generation === this.sessionGeneration ? this.getRelevantAndPruneKnownClientCursors() : []; }); - if (generation === this.sessionGeneration) + if (generation === this.sessionGeneration) { this.onRemoteCursorsUpdated.trigger(cursors); + } }); } @@ -131,10 +144,19 @@ export class CursorTracker { await this.publishLocalCursors(false); } + public reset(): void { + this.resetSession(); + this.knownRemoteCursors = []; + this.onRemoteCursorsUpdated.trigger([]); + } + private async publishLocalCursors(force: boolean): Promise { const generation = this.sessionGeneration; await this.sendLock.withLock(async () => { - if (generation !== this.sessionGeneration) return; + if (generation !== this.sessionGeneration) { + return; + } + const documentsWithCursors: DocumentWithCursors[] = []; for (const [editorPath, cursors] of Object.entries( @@ -143,13 +165,17 @@ export class CursorTracker { const id = this.localCursorDocuments[editorPath] ?? this.database.findDocumentId(editorPath); - if (id === undefined) continue; + if (id === undefined) { + continue; + } + this.localCursorDocuments[editorPath] = id; const relativePath = this.database.state.local[id]; const base = this.database.state.documents[id]?.base; // Wait until the document has a path and an acknowledged version. - if (relativePath === undefined || base?.vaultUpdateId == null) + if (relativePath === undefined || base?.vaultUpdateId == null) { continue; + } documentsWithCursors.push({ relative_path: relativePath, @@ -163,29 +189,33 @@ export class CursorTracker { } const input = JSON.stringify(documentsWithCursors); - if (!force && this.lastCursorInput === input) return; + if (!force && this.lastCursorInput === input) { + return; + } + this.lastCursorInput = input; for (const doc of documentsWithCursors) { - if (!(await this.matchesLocalVersion(doc))) + if (!(await this.matchesLocalVersion(doc))) { doc.vault_update_id = null; + } + } + + if (generation !== this.sessionGeneration) { + return; } - if (generation !== this.sessionGeneration) return; const message = JSON.stringify(documentsWithCursors); - if (!force && this.lastCursorMessage === message) return; + if (!force && this.lastCursorMessage === message) { + return; + } + this.lastCursorMessage = message; this.webSocketManager.updateLocalCursors({ documentsWithCursors }); }); } - public reset(): void { - this.resetSession(); - this.knownRemoteCursors = []; - this.onRemoteCursorsUpdated.trigger([]); - } - private resetSession(): void { this.sessionGeneration++; clearInterval(this.cursorTimer); @@ -209,6 +239,7 @@ export class CursorTracker { retainedCursors.unshift(clientCursors); retainedFuture.add(clientCursors.deviceId); } + continue; } @@ -245,26 +276,40 @@ export class CursorTracker { let result = DocumentUpToDateness.UpToDate; for (const document of clientCursor.documentsWithCursors) { const status = await this.getDocumentUpToDateness(document); - if (status === DocumentUpToDateness.Later) return status; - if (status === DocumentUpToDateness.Prior) result = status; + if (status === DocumentUpToDateness.Later) { + return status; + } + + if (status === DocumentUpToDateness.Prior) { + result = status; + } } + return result; } private async getDocumentUpToDateness( document: DocumentWithCursors ): Promise { - if (document.vault_update_id === null) + if (document.vault_update_id === null) { return DocumentUpToDateness.Prior; + } + const path = this.database.state.local[document.document_id]; - if (path === undefined) return DocumentUpToDateness.Later; + if (path === undefined) { + return DocumentUpToDateness.Later; + } + const version = this.database.state.documents[document.document_id]?.base ?.vaultUpdateId ?? 0; - if (version < document.vault_update_id) + if (version < document.vault_update_id) { return DocumentUpToDateness.Later; - if (version > document.vault_update_id) + } + + if (version > document.vault_update_id) { return DocumentUpToDateness.Prior; + } return (await this.matchesLocalVersion({ ...document, @@ -283,7 +328,10 @@ export class CursorTracker { const snapshot = await this.fs .read(relative_path) .catch(() => undefined); - if (!snapshot) return false; + if (!snapshot) { + return false; + } + const digest = await hash(snapshot.content); const { local, documents } = this.database.state; const base = documents[document_id]?.base; diff --git a/frontend/sync-client/src/sync-operations/file-manifest.ts b/frontend/sync-client/src/sync-operations/file-manifest.ts index faa4a29c..aed1624d 100644 --- a/frontend/sync-client/src/sync-operations/file-manifest.ts +++ b/frontend/sync-client/src/sync-operations/file-manifest.ts @@ -1,7 +1,7 @@ import type { FileManifest } from "../services/types/FileManifest"; import type { DocumentId, - EngineState, + StoredDatabase, RelativePath } from "../persistence/database"; import type { FileManifestEntries } from "../types/file-manifest-entries"; @@ -14,7 +14,7 @@ export function validateFileManifest(entries: FileManifestEntries): void { validatePortablePaths(Object.values(entries)); } -export function sameFileManifest( +export function haveSameEntries( a: FileManifestEntries, b: FileManifestEntries ): boolean { @@ -31,20 +31,28 @@ export function resolvePaths( reservedPaths: readonly RelativePath[] = [] ): FileManifestEntries { const result: FileManifestEntries = {}; - for (const id of protectedIds) - if (proposed[id] !== undefined) result[id] = proposed[id]; + for (const id of protectedIds) { + if (proposed[id] !== undefined) { + result[id] = proposed[id]; + } + } + const occupied = [...Object.values(result), ...reservedPaths]; - const ids = Object.keys(proposed).sort( - (a, b) => + const entries = Object.entries(proposed).sort( + ([a], [b]) => Number(remote[b] === proposed[b]) - Number(remote[a] === proposed[a]) || (a < b ? -1 : a > b ? 1 : 0) ); - for (const id of ids) { - if (result[id] !== undefined) continue; - result[id] = allocatePortablePath(proposed[id], id, occupied); + for (const [id, path] of entries) { + if (result[id] !== undefined) { + continue; + } + + result[id] = allocatePortablePath(path, id, occupied); occupied.push(result[id]); } + return result; } @@ -60,14 +68,17 @@ export function mergeFileManifests( ...Object.keys(remote) ])) { const path = remote[id] === base[id] ? local[id] : remote[id]; - if (path !== undefined) proposed[id] = path; + if (path !== undefined) { + proposed[id] = path; + } } + return resolvePaths(proposed, remote); } /** Merge remote paths while retaining excluded occupants and their original bases. */ export function reconcileFileManifest( - next: EngineState, + next: StoredDatabase, remote: FileManifest, isIgnored: (path: RelativePath) => boolean, unsyncablePaths: ReadonlySet @@ -75,9 +86,13 @@ export function reconcileFileManifest( const before = next.local; const base = { ...next.fileManifest.entries }; for (const [id, path] of Object.entries(next.excluded ?? {})) { - if (path === null) delete base[id]; - else base[id] = path; + if (path === null) { + Reflect.deleteProperty(base, id); + } else { + base[id] = path; + } } + const previousManifest = next.fileManifest.entries; next.local = mergeFileManifests(base, next.local, remote.entries); next.fileManifest = remote; @@ -89,15 +104,22 @@ export function reconcileFileManifest( (path !== undefined && isIgnored(path)) ) { next.excluded ??= {}; - if (next.excluded[id] === undefined) + if (next.excluded[id] === undefined) { next.excluded[id] = previousManifest[id] ?? null; + } + next.local[id] = oldPath; - } else delete next.excluded?.[id]; - } - for (const [id, path] of Object.entries(next.excluded ?? {})) { - if (before[id] === undefined && (path === null || !isIgnored(path))) + } else { delete next.excluded?.[id]; + } } + + for (const [id, path] of Object.entries(next.excluded ?? {})) { + if (before[id] === undefined && (path === null || !isIgnored(path))) { + delete next.excluded?.[id]; + } + } + next.local = resolvePaths( next.local, remote.entries, @@ -106,25 +128,30 @@ export function reconcileFileManifest( ); } -export function manifestToSubmit( - state: EngineState, +export function buildUploadManifest( + state: StoredDatabase, unsyncablePaths: ReadonlySet ): FileManifestEntries { // Do not publish a new document until its initial content is acknowledged. const entries = { ...state.local }; for (const id of Object.keys(state.excluded ?? {})) { const remotePath = state.fileManifest.entries[id]; - if (remotePath === undefined) delete entries[id]; - else entries[id] = remotePath; + if (remotePath === undefined) { + Reflect.deleteProperty(entries, id); + } else { + entries[id] = remotePath; + } } + for (const id of Object.keys(entries)) { if ( !state.documents[id]?.base && state.fileManifest.entries[id] === undefined ) { - delete entries[id]; + Reflect.deleteProperty(entries, id); } } + return resolvePaths( entries, state.fileManifest.entries, @@ -132,7 +159,9 @@ export function manifestToSubmit( [ ...unsyncablePaths, ...Object.keys(state.excluded ?? {}).flatMap((id) => - state.local[id] ? [state.local[id]] : [] + state.local[id] !== undefined && state.local[id] !== "" + ? [state.local[id]] + : [] ) ] ); diff --git a/frontend/sync-client/src/sync-operations/local-change-queue.ts b/frontend/sync-client/src/sync-operations/local-change-queue.ts new file mode 100644 index 00000000..f6a987af --- /dev/null +++ b/frontend/sync-client/src/sync-operations/local-change-queue.ts @@ -0,0 +1,73 @@ +import type { RelativePath } from "../persistence/database"; +import type { FileOperations } from "../file-operations/file-operations"; +import type { LocalChange } from "./local-changes"; +import { LocalChangesDuringReconciliation } from "../errors/errors"; + +export interface LocalChangeStorage { + entries: LocalChange[]; + save: (entries: LocalChange[]) => Promise; +} + +/** Persist notifications in order and invalidate plans when file identities change. */ +export class LocalChangeQueue { + public readonly entries: LocalChange[]; + public readonly pendingPaths = new Set(); + private localGeneration = 0; + private notificationSaving: Promise = Promise.resolve(); + private notificationSaveFailed = false; + + public constructor( + private readonly files: FileOperations, + private readonly onChange: () => void, + private readonly storage: LocalChangeStorage + ) { + this.entries = storage.entries; + } + + public async saveRemainingChanges(): Promise { + await this.storage.save(structuredClone(this.entries)); + } + + public async record(change: LocalChange): Promise { + this.localGeneration++; + if (change.type === "move") { + this.pendingPaths.add(change.oldPath); + this.pendingPaths.add(change.relativePath); + } else { + this.pendingPaths.add(change.path); + } + + const previous = [...this.entries]; + this.entries.push(change); + const saving = this.notificationSaving.then(async () => { + this.files.captureChange(change, previous); + await this.storage.save(structuredClone(this.entries)); + this.notificationSaveFailed = false; + }); + this.notificationSaving = saving.catch(() => { + this.notificationSaveFailed = true; + }); + try { + await saving; + } finally { + this.onChange(); + } + } + + public async flush(): Promise { + await this.notificationSaving; + if (this.notificationSaveFailed) { + await this.storage.save(structuredClone(this.entries)); + this.notificationSaveFailed = false; + } + } + + public createGuard(): () => void { + const generation = this.localGeneration; + return () => { + if (generation !== this.localGeneration) { + throw new LocalChangesDuringReconciliation(); + } + }; + } +} diff --git a/frontend/sync-client/src/sync-operations/local-changes.ts b/frontend/sync-client/src/sync-operations/local-changes.ts index d7261eaf..52f512d5 100644 --- a/frontend/sync-client/src/sync-operations/local-changes.ts +++ b/frontend/sync-client/src/sync-operations/local-changes.ts @@ -1,5 +1,9 @@ import { v4 as uuid } from "uuid"; -import type { DocumentId, EngineState, RelativePath } from "../persistence/database"; +import type { + DocumentId, + StoredDatabase, + RelativePath +} from "../persistence/database"; import type { FileManifestEntries } from "../types/file-manifest-entries"; import type { VaultSnapshot } from "../services/types/VaultSnapshot"; import { arePathAliases } from "../utils/portable-path"; @@ -9,13 +13,13 @@ export type LocalChange = { /** Identities at notification time, before a remote application can reuse paths. */ identities?: FileManifestEntries; } & ( - | { type: "move"; oldPath: RelativePath; relativePath: RelativePath } - | { type: "create"; path: RelativePath; documentId?: DocumentId } - | { type: "delete"; path: RelativePath } - ); + | { type: "move"; oldPath: RelativePath; relativePath: RelativePath } + | { type: "create"; path: RelativePath; documentId?: DocumentId } + | { type: "delete"; path: RelativePath } +); -export function unappliedChanges( - state: EngineState, +export function getUnappliedChanges( + state: StoredDatabase, changes: readonly LocalChange[] ): LocalChange[] { const applied = changes.findIndex( @@ -25,8 +29,24 @@ export function unappliedChanges( return changes.slice(applied + 1); } +/** Reuse a server identity on the first scan only if it has no local occupant. */ +export function findUntrackedRemoteDocument( + database: StoredDatabase, + path: RelativePath, + initial?: VaultSnapshot +): DocumentId | undefined { + if (initial === undefined) { + return undefined; + } + + return Object.entries(initial.fileManifest.entries).find( + ([id, remotePath]) => + database.local[id] === undefined && arePathAliases(remotePath, path) + )?.[0]; +} + export function applyLocalChange( - next: EngineState, + next: StoredDatabase, change: LocalChange, ignored: (path: RelativePath) => boolean, initial?: VaultSnapshot @@ -35,23 +55,17 @@ export function applyLocalChange( if ( ignored(change.path) || Object.values(next.local).includes(change.path) - ) + ) { return; + } + const id = (change.documentId ??= - (initial && - Object.keys(initial.fileManifest.entries).find( - (id) => - !next.local[id] && - arePathAliases( - initial.fileManifest.entries[id], - change.path - ) - )) || - uuid()); + findUntrackedRemoteDocument(next, change.path, initial) ?? uuid()); next.local[id] = change.path; next.documents[id] ??= { observedHash: "" }; return; } + const from = change.type === "move" ? change.oldPath : change.path; change.identities ??= Object.fromEntries( Object.entries(next.local).filter( @@ -64,14 +78,20 @@ export function applyLocalChange( if (change.type === "delete") { if (ignored(path)) { next.excluded ??= {}; - if (next.excluded[id] === undefined) + if (next.excluded[id] === undefined) { next.excluded[id] = next.fileManifest.entries[id] ?? null; + } } - delete next.local[id]; + + Reflect.deleteProperty(next.local, id); } else { const to = change.relativePath + path.slice(from.length); - for (const [other, otherPath] of Object.entries(next.local)) - if (other !== id && otherPath === to) delete next.local[other]; + for (const [other, otherPath] of Object.entries(next.local)) { + if (other !== id && otherPath === to) { + Reflect.deleteProperty(next.local, other); + } + } + next.local[id] = to; } } diff --git a/frontend/sync-client/src/sync-operations/outgoing-cursor.test.ts b/frontend/sync-client/src/sync-operations/outgoing-cursor.test.ts index 71b7d148..90119c8f 100644 --- a/frontend/sync-client/src/sync-operations/outgoing-cursor.test.ts +++ b/frontend/sync-client/src/sync-operations/outgoing-cursor.test.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/no-unsafe-type-assertion -- Test doubles implement only the exercised adapter methods. */ import assert from "node:assert/strict"; import { test } from "node:test"; import { CursorTracker } from "./cursor-tracker"; @@ -5,19 +6,19 @@ import { EventListeners } from "../utils/data-structures/event-listeners"; import { hash } from "../utils/hash"; import { Database, - emptyState, + createEmptyDatabase, type RelativePath } from "../persistence/database"; import type { FileSystemOperations } from "../file-operations/filesystem-operations"; import type { WebSocketManager } from "../services/websocket-manager"; import type { DocumentWithCursors } from "../services/types/DocumentWithCursors"; -for (const change of ["version", "identity", "path", "delete"] as const) - test(`outgoing cursor is dirty when ${change} changes during its snapshot read`, async () => { +for (const change of ["version", "identity", "path", "delete"] as const) { + test(`outgoing cursor is dirty when ${change} changes during its snapshot read`, async (): Promise => { // Equal bytes deliberately defeat a freshness check that compares only hashes. const content = new TextEncoder().encode("same bytes"); const digest = await hash(content); - const state = emptyState("test"); + const state = createEmptyDatabase("test"); state.local.a = "a.md"; state.documents.a = { base: { vaultUpdateId: 1, hash: digest } }; const database = new Database( @@ -32,19 +33,25 @@ for (const change of ["version", "identity", "path", "delete"] as const) onRemoteCursorsUpdateReceived: new EventListeners(), updateLocalCursors: (data: { documentsWithCursors: DocumentWithCursors[]; - }) => sent.push(data) + }): number => sent.push(data) }; const files = { - read: async () => { + read: async (): Promise<{ content: Uint8Array }> => { const current = database.state; - if (change === "delete") delete current.local.a; - else if (change === "version") - current.documents.a.base!.vaultUpdateId = 2; - else if (change === "identity") { + assert.ok(current.documents.a?.base); + assert.ok(current.local.a !== undefined); + if (change === "delete") { + delete current.local.a; + } else if (change === "version") { + current.documents.a.base.vaultUpdateId = 2; + } else if (change === "identity") { current.local.replacement = current.local.a; current.documents.replacement = current.documents.a; delete current.local.a; - } else current.local.a = "moved.md"; + } else { + current.local.a = "moved.md"; + } + return { content }; } }; @@ -58,6 +65,7 @@ for (const change of ["version", "identity", "path", "delete"] as const) "a.md": [{ start: 4, end: 4 }] }); assert.equal(sent.length, 1); - assert.equal(sent[0].documentsWithCursors[0].vault_update_id, null); + assert.equal(sent[0]?.documentsWithCursors[0]?.vault_update_id, null); tracker.reset(); }); +} diff --git a/frontend/sync-client/src/sync-operations/remote-changes.ts b/frontend/sync-client/src/sync-operations/remote-changes.ts new file mode 100644 index 00000000..385bd94f --- /dev/null +++ b/frontend/sync-client/src/sync-operations/remote-changes.ts @@ -0,0 +1,96 @@ +import type { VaultSnapshot } from "../services/types/VaultSnapshot"; +import type { + StoredDatabase, + DocumentId, + ContentHead, + VaultUpdateId +} from "../persistence/database"; +import type { SyncService } from "../services/sync-service"; +import type { EventBatch } from "../services/types/EventBatch"; +import type { FileManifest } from "../services/types/FileManifest"; +import { PermanentSyncError } from "../errors/errors"; +import { selectContentHead } from "./content-sync"; + +export interface RemoteChanges { + manifest: FileManifest; + cursor: VaultUpdateId; + heads: Record; +} + +/** Fetch and validate the complete event stream before applying any changes. */ +export async function fetchRemoteChanges( + service: SyncService, + database: StoredDatabase, + batch: EventBatch +): Promise { + let cursor = database.lastSeenUpdateId ?? 0; + let manifest = database.fileManifest; + const heads: Record = {}; + for (const [id, doc] of Object.entries(database.documents)) { + if (doc.remote) { + heads[id] = doc.remote; + } + } + + // Fold every page before touching local files. If fetching is interrupted, + // discard this work and retry from the last committed cursor. + for (;;) { + const after = cursor; + for (const event of batch.events) { + if (event.eventId <= cursor) { + continue; + } + + if (event.eventId !== cursor + 1) { + throw new PermanentSyncError( + "Non-contiguous event history; refusing to skip changes" + ); + } + + cursor = event.eventId; + if (event.type === "fileManifest") { + if ( + event.fileManifest.fileManifestId > manifest.fileManifestId + ) { + manifest = event.fileManifest; + } + } else { + const previous = heads[event.document.documentId]; + if ( + previous === undefined || + previous.vaultUpdateId < event.document.vaultUpdateId + ) { + heads[event.document.documentId] = selectContentHead( + event.document + ); + } + } + } + + const end = batch.endEventId; + if ( + end !== cursor || + end > batch.headEventId || + (end === after && end < batch.headEventId) + ) { + throw new PermanentSyncError("Incomplete event batch"); + } + + if (end === batch.headEventId) { + break; + } + + batch = await service.getEvents(cursor); + } + + // Three-way merging compares snapshots. Replaying intermediate remote + // edits can discard a local edit even when the remote ends at its base. + return { manifest, cursor, heads }; +} + +export interface RemoteApplicationOptions { + cursor?: VaultUpdateId; + initial?: VaultSnapshot; + clearPending?: boolean; + heads?: Record; +} diff --git a/frontend/sync-client/src/sync-operations/scan.ts b/frontend/sync-client/src/sync-operations/scan.ts index cb8546cc..d1abb205 100644 --- a/frontend/sync-client/src/sync-operations/scan.ts +++ b/frontend/sync-client/src/sync-operations/scan.ts @@ -3,18 +3,16 @@ import type { FileOperations } from "../file-operations/file-operations"; import type { Database, DocumentId, - RelativePath + RelativePath, + StoredDatabase } from "../persistence/database"; import type { VaultSnapshot } from "../services/types/VaultSnapshot"; import { EMPTY_HASH, hash } from "../utils/hash"; -import { - arePathAliases, - findPathWithSameSpelling -} from "../utils/portable-path"; - +import { findPathWithSameSpelling } from "../utils/portable-path"; import { applyLocalChange, - unappliedChanges, + findUntrackedRemoteDocument, + getUnappliedChanges, type LocalChange } from "./local-changes"; export type { LocalChange } from "./local-changes"; @@ -28,126 +26,187 @@ interface ScanOptions { oversized: (size: number) => boolean; } -/** Scans are complete or fail. Missing unmaterialized/ignored entries are not deletions. */ -export async function scanLocalFiles( - { - database, - guard, - changes: queuedChanges, - files, - ignored, - oversized - }: ScanOptions, - initial?: VaultSnapshot -): Promise> { - const next = structuredClone(database.state); - // Leave the prefix queued until its resulting state is saved. - // A persisted change ID makes replay safe after an uncertain save. - const changes = [...queuedChanges]; - const unsyncablePaths = new Set(); - for (const change of unappliedChanges(next, changes)) { - applyLocalChange(next, change, ignored, initial); - next.lastAppliedLocalChangeId = change.changeId; - } +interface DiskInventory { + present: Set; + hashes: Map; + unsyncablePaths: Set; +} + +/** Keep an accessible Unicode spelling when the filesystem reports an alias. */ +async function listAccessiblePaths( + files: FileOperations, + database: StoredDatabase +): Promise { const paths: RelativePath[] = []; const reportedPaths = await files.listFilesRecursively(); + for (const path of reportedPaths) { - // Some filesystems may report a different Unicode spelling after a - // rename. Keep an already-accessible spelling when possible. - const alias = findPathWithSameSpelling(Object.values(next.local), path); - paths.push( - alias && - !reportedPaths.includes(alias) && - (await files.fs.exists(alias)) - ? alias - : path + const alias = findPathWithSameSpelling( + Object.values(database.local), + path ); + const canKeepAlias = + alias !== undefined && + !reportedPaths.includes(alias) && + (await files.fs.exists(alias)); + paths.push(canKeepAlias ? alias : path); } - const present = new Set(paths); + + return paths; +} + +async function readDiskInventory( + paths: RelativePath[], + { files, ignored, oversized }: ScanOptions +): Promise { const hashes = new Map(); + const unsyncablePaths = new Set(); + for (const path of paths) { if (ignored(path)) { unsyncablePaths.add(path); continue; } + const info = await files.fs.stat(path); - if (!info || info.kind !== "file") + if (info?.kind !== "file") { throw new Error(`File changed during scan: ${path}`); + } + if (oversized(info.size)) { unsyncablePaths.add(path); continue; } + const snapshot = await files.fs.read(path); - if (!snapshot) throw new Error(`File changed during scan: ${path}`); - if (!oversized(snapshot.content.length)) { - hashes.set(path, await hash(snapshot.content)); - } else { + if (snapshot === undefined) { + throw new Error(`File changed during scan: ${path}`); + } + + if (oversized(snapshot.content.length)) { unsyncablePaths.add(path); + } else { + hashes.set(path, await hash(snapshot.content)); } } + + return { present: new Set(paths), hashes, unsyncablePaths }; +} + +/** Match a rename only when both its old identity and new hash are unambiguous. */ +function assignFileIdentities( + next: StoredDatabase, + { present, hashes }: DiskInventory, + ignored: ScanOptions["ignored"], + initial?: VaultSnapshot +): void { + // Missing unmaterialized or ignored entries are not deletions. const missing = new Set( - Object.keys(next.local).filter( - (id) => - next.documents[id]?.observedHash !== undefined && - !ignored(next.local[id]) && - !present.has(next.local[id]) - ) + Object.entries(next.local) + .filter( + ([id, path]) => + next.documents[id]?.observedHash !== undefined && + !ignored(path) && + !present.has(path) + ) + .map(([id]) => id) ); const knownPaths = new Set(Object.values(next.local)); - const unknown = [...hashes.keys()].filter((path) => !knownPaths.has(path)); - for (const path of unknown) { - const digest = hashes.get(path)!; + const unknown = [...hashes.entries()].filter( + ([path]) => !knownPaths.has(path) + ); + + for (const [path, digest] of unknown) { const matches = [...missing].filter( (id) => next.documents[id]?.observedHash === digest ); - const sameHash = unknown.filter( - (other) => hashes.get(other) === digest + const matchingPaths = unknown.filter( + ([, otherHash]) => otherHash === digest ); - let id: DocumentId | undefined; + let id: DocumentId | undefined = undefined; + + const [onlyMatch] = matches; if ( digest !== EMPTY_HASH && matches.length === 1 && - sameHash.length === 1 + onlyMatch !== undefined && + matchingPaths.length === 1 ) { - id = matches[0]; + id = onlyMatch; missing.delete(id); - } else if (initial) { - id = Object.keys(initial.fileManifest.entries).find( - (candidate) => - !next.local[candidate] && - arePathAliases( - initial.fileManifest.entries[candidate], - path - ) - ); + } else { + id = findUntrackedRemoteDocument(next, path, initial); } + id ??= uuid(); next.local[id] = path; next.documents[id] ??= { observedHash: "" }; } - for (const id of missing) delete next.local[id]; + + for (const id of missing) { + Reflect.deleteProperty(next.local, id); + } +} + +function updateObservedFiles( + next: StoredDatabase, + { hashes, unsyncablePaths }: DiskInventory, + ignored: ScanOptions["ignored"] +): void { for (const [id, path] of Object.entries(next.local)) { if (unsyncablePaths.has(path) || ignored(path)) { next.excluded ??= {}; - if (next.excluded[id] === undefined) + if (next.excluded[id] === undefined) { next.excluded[id] = next.fileManifest.entries[id] ?? null; + } } + const digest = hashes.get(path); - if (digest !== undefined) { - const doc = (next.documents[id] ??= {}); - // The old history no longer contains this base. Preserve any edit - // made during recovery as a new file instead of overwriting it. - if (doc.base?.vaultUpdateId === null && doc.base.hash !== digest) { - const recoveredId = uuid(); - next.local[recoveredId] = path; - next.documents[recoveredId] = { observedHash: digest }; - delete next.local[id]; - } else doc.observedHash = digest; + if (digest === undefined) { + continue; + } + + const document = (next.documents[id] ??= {}); + // After a history reset, an edit to a formerly clean file gets a new + // identity so the replacement server history cannot overwrite it. + if ( + document.base?.vaultUpdateId === null && + document.base.hash !== digest + ) { + const recoveredId = uuid(); + next.local[recoveredId] = path; + next.documents[recoveredId] = { observedHash: digest }; + Reflect.deleteProperty(next.local, id); + } else { + document.observedHash = digest; } } +} + +/** Commit a complete scan before consuming its saved notification prefix. */ +export async function scanLocalFiles( + options: ScanOptions, + initial?: VaultSnapshot +): Promise> { + const { database, guard, changes: queuedChanges, files, ignored } = options; + const next = database.cloneState(); + const changes = [...queuedChanges]; + + for (const change of getUnappliedChanges(next, changes)) { + applyLocalChange(next, change, ignored, initial); + next.lastAppliedLocalChangeId = change.changeId; + } + + const paths = await listAccessiblePaths(files, next); + const inventory = await readDiskInventory(paths, options); + assignFileIdentities(next, inventory, ignored, initial); + updateObservedFiles(next, inventory, ignored); + guard(); await database.commit(next); + // A persisted change ID makes replay safe after an uncertain save. queuedChanges.splice(0, changes.length); guard(); - return unsyncablePaths; + + return inventory.unsyncablePaths; } diff --git a/frontend/sync-client/src/sync-operations/sync-scheduler.test.ts b/frontend/sync-client/src/sync-operations/sync-scheduler.test.ts new file mode 100644 index 00000000..2103956d --- /dev/null +++ b/frontend/sync-client/src/sync-operations/sync-scheduler.test.ts @@ -0,0 +1,138 @@ +import assert from "node:assert/strict"; +import { afterEach, beforeEach, describe, it, mock } from "node:test"; +import { Settings, type SyncSettings } from "../persistence/settings"; +import { Logger } from "../tracing/logger"; +import { PermanentSyncError } from "../errors/errors"; +import { SyncPhase, SyncScheduler } from "./sync-scheduler"; + +function createScheduler( + overrides: Partial = {}, + pass: () => Promise = async () => undefined +): { scheduler: SyncScheduler; runCount: () => number } { + let runs = 0; + const scheduler = new SyncScheduler( + new Settings(new Logger(), overrides, async () => undefined), + async () => { + do { + scheduler.beginPass(); + runs++; + await pass(); + } while (scheduler.needsAnotherPass); + }, + () => undefined, + () => false + ); + return { scheduler, runCount: () => runs }; +} + +describe("Sync scheduler transitions", () => { + beforeEach(() => { + mock.timers.enable({ apis: ["setTimeout"] }); + }); + afterEach(() => { + mock.timers.reset(); + }); + + for (const interval of [undefined, 0]) { + it(`leaves polling disabled for interval ${interval}`, async () => { + const { scheduler, runCount } = createScheduler({ + syncIntervalMs: interval + }); + scheduler.start(); + await scheduler.waitUntilFinished(); + assert.equal(scheduler.currentPhase, SyncPhase.Idle); + mock.timers.tick(60_000); + assert.equal(runCount(), 1); + await scheduler.stop(); + assert.equal(scheduler.currentPhase, SyncPhase.Stopped); + }); + } + + it("polls after the configured interval and cancels the timer on stop", async () => { + const { scheduler, runCount } = createScheduler({ syncIntervalMs: 25 }); + scheduler.start(); + await scheduler.waitUntilFinished(); + mock.timers.tick(24); + assert.equal(runCount(), 1); + mock.timers.tick(1); + assert.equal(runCount(), 2); + await scheduler.waitUntilFinished(); + await scheduler.stop(); + mock.timers.tick(25); + assert.equal(runCount(), 2); + }); + + it("coalesces notifications during a pass without running concurrently", async () => { + const gate = Promise.withResolvers(); + const { scheduler, runCount } = createScheduler( + {}, + async () => gate.promise + ); + scheduler.start(); + assert.equal(scheduler.currentPhase, SyncPhase.Running); + scheduler.requestSync(); + scheduler.requestSync(); + assert.equal(scheduler.currentPhase, SyncPhase.RunRequested); + assert.equal(runCount(), 1); + gate.resolve(undefined); + await scheduler.waitUntilFinished(); + assert.equal(runCount(), 2); + assert.equal(scheduler.currentPhase, SyncPhase.Idle); + await scheduler.stop(); + }); + + it("drains an active pass on stop and discards queued notifications", async () => { + const gate = Promise.withResolvers(); + const { scheduler, runCount } = createScheduler( + {}, + async () => gate.promise + ); + scheduler.start(); + scheduler.requestSync(); + const stopped = scheduler.stop(); + assert.equal(scheduler.currentPhase, SyncPhase.Stopping); + scheduler.requestSync(); + gate.resolve(undefined); + await stopped; + assert.equal(scheduler.currentPhase, SyncPhase.Stopped); + assert.equal(runCount(), 1); + }); + + it("retries transient failures and returns to idle on success", async () => { + let attempts = 0; + const { scheduler } = createScheduler( + { networkRetryIntervalMs: 10 }, + async () => { + if (++attempts === 1) { + throw new Error("offline"); + } + } + ); + scheduler.start(); + await assert.rejects(scheduler.waitUntilFinished(), /offline/u); + assert.equal(scheduler.currentPhase, SyncPhase.WaitingToRetry); + mock.timers.tick(10); + await scheduler.waitUntilFinished(); + assert.equal(scheduler.currentPhase, SyncPhase.Idle); + assert.equal(attempts, 2); + await scheduler.stop(); + }); + + it("blocks automatic retries of permanent errors but accepts an explicit request", async () => { + const { scheduler, runCount } = createScheduler( + { networkRetryIntervalMs: 10 }, + async () => { + throw new PermanentSyncError("rejected"); + } + ); + scheduler.start(); + await assert.rejects(scheduler.waitUntilFinished(), /rejected/u); + assert.equal(scheduler.currentPhase, SyncPhase.Blocked); + mock.timers.tick(100); + assert.equal(runCount(), 1); + scheduler.requestSync(); + await assert.rejects(scheduler.waitUntilFinished(), /rejected/u); + assert.equal(runCount(), 2); + await scheduler.stop(); + }); +}); diff --git a/frontend/sync-client/src/sync-operations/sync-scheduler.ts b/frontend/sync-client/src/sync-operations/sync-scheduler.ts new file mode 100644 index 00000000..8cfc224b --- /dev/null +++ b/frontend/sync-client/src/sync-operations/sync-scheduler.ts @@ -0,0 +1,167 @@ +import { + AuthenticationError, + PermanentSyncError, + ServerVersionMismatchError +} from "../errors/errors"; +import type { Settings } from "../persistence/settings"; + +export enum SyncPhase { + Stopped = "stopped", + Idle = "idle", + Running = "running", + RunRequested = "run-requested", + Stopping = "stopping", + WaitingToRetry = "waiting-to-retry", + Blocked = "blocked" +} + +/** + * Serializes sync attempts and owns their timers. + * + * start: Stopped -> Idle -> RunRequested + * requestSync: Idle / WaitingToRetry / Blocked -> RunRequested + * beginPass: RunRequested -> Running + * requestSync during a pass: Running -> RunRequested (coalesces notifications) + * completion: Running -> Idle; failure -> WaitingToRetry or Blocked + * stop: active attempt -> Stopping -> Stopped; otherwise -> Stopped + * + * A pass may finish after stop is requested, but cannot schedule more work. + */ +export class SyncScheduler { + private phase = SyncPhase.Stopped; + private activeAttempt?: Promise; + private timer?: ReturnType; + private failure?: Error; + + public constructor( + private readonly settings: Settings, + private readonly synchronize: () => Promise, + private readonly reportRemainingWork: (count: number) => void, + private readonly hasPendingUpload: () => boolean + ) {} + + public get currentPhase(): SyncPhase { + return this.phase; + } + + public get isStopped(): boolean { + return ( + this.phase === SyncPhase.Stopped || + this.phase === SyncPhase.Stopping + ); + } + + public get isBusy(): boolean { + return ( + this.activeAttempt !== undefined || + this.needsAnotherPass || + this.failure !== undefined + ); + } + + public get needsAnotherPass(): boolean { + return this.phase === SyncPhase.RunRequested; + } + + public start(): void { + this.failure = undefined; + this.phase = + this.activeAttempt === undefined + ? SyncPhase.Idle + : SyncPhase.RunRequested; + this.requestSync(); + } + + public async stop(): Promise { + clearTimeout(this.timer); + this.phase = + this.activeAttempt === undefined + ? SyncPhase.Stopped + : SyncPhase.Stopping; + await this.activeAttempt; + } + + public requestSync(): void { + if (this.isStopped) { + return; + } + + this.phase = SyncPhase.RunRequested; + if (this.activeAttempt !== undefined) { + return; + } + + clearTimeout(this.timer); + this.failure = undefined; + this.reportRemainingWork(1); + this.activeAttempt = this.synchronize() + .catch((error: unknown) => { + this.failure = + error instanceof Error + ? error + : new Error(String(error), { cause: error }); + }) + .finally(() => { + this.finishAttempt(); + }); + } + + /** Consume all notifications received before the next pass begins. */ + public beginPass(): void { + if (!this.isStopped) { + this.phase = SyncPhase.Running; + } + } + + public async waitUntilFinished(): Promise { + while (this.activeAttempt !== undefined) { + await this.activeAttempt; + } + + if (this.failure !== undefined) { + throw this.failure; + } + } + + private finishAttempt(): void { + this.activeAttempt = undefined; + this.reportRemainingWork( + this.failure !== undefined || this.hasPendingUpload() ? 1 : 0 + ); + + if (this.isStopped) { + this.phase = SyncPhase.Stopped; + return; + } + + if (this.needsAnotherPass && this.failure === undefined) { + this.requestSync(); + return; + } + + if ( + this.failure instanceof PermanentSyncError || + this.failure instanceof AuthenticationError || + this.failure instanceof ServerVersionMismatchError + ) { + this.phase = SyncPhase.Blocked; + return; + } + + this.phase = + this.failure === undefined + ? SyncPhase.Idle + : SyncPhase.WaitingToRetry; + const { syncIntervalMs, networkRetryIntervalMs } = + this.settings.getSettings(); + const intervalMs = + this.failure === undefined + ? syncIntervalMs + : networkRetryIntervalMs; + if (intervalMs !== undefined && intervalMs !== 0) { + this.timer = setTimeout(() => { + this.requestSync(); + }, intervalMs); + } + } +} diff --git a/frontend/sync-client/src/sync-operations/syncer.test.ts b/frontend/sync-client/src/sync-operations/syncer.test.ts index 3147035e..5b6b53f2 100644 --- a/frontend/sync-client/src/sync-operations/syncer.test.ts +++ b/frontend/sync-client/src/sync-operations/syncer.test.ts @@ -1,17 +1,9 @@ +import { ContentSync, selectContentHead } from "./content-sync"; /* eslint-disable @typescript-eslint/no-unsafe-type-assertion */ import assert from "node:assert"; -import { afterEach, beforeEach, describe, it, mock } from "node:test"; -import type { - Database, - DocumentId, - RelativePath, - VaultUpdateId -} from "../persistence/database"; -import { - DEFAULT_SETTINGS, - Settings, - type SyncSettings -} from "../persistence/settings"; +import { describe, it } from "node:test"; +import type { Database, RelativePath } from "../persistence/database"; +import { Settings } from "../persistence/settings"; import type { FileOperations } from "../file-operations/file-operations"; import type { SyncService } from "../services/sync-service"; import type { ServerConfig } from "../services/server-config"; @@ -24,104 +16,6 @@ import { FixedSizeDocumentCache } from "../utils/data-structures/fix-sized-cache import { hashSnapshot } from "../snapshot"; import { undiff } from "reconcile-text"; -function createSyncer(overrides: Partial = {}): { - syncer: Syncer; - runCount: () => number; -} { - const settings = new Settings( - new Logger(), - overrides, - async () => undefined - ); - const database = { - state: {} - } as unknown as Database; - const websocket = { - onWebSocketStatusChanged: new EventListeners< - (connected: boolean) => unknown - >(), - onRemoteVaultUpdateReceived: new EventListeners<() => unknown>() - } as unknown as WebSocketManager; - const syncer = new Syncer( - {} as Logger, - database, - settings, - {} as SyncService, - websocket, - {} as FileOperations, - {} as ServerConfig, - {} as SyncHistory, - new FixedSizeDocumentCache(1024) - ); - let runs = 0; - const internals = syncer as unknown as { - dirty: boolean; - run: () => Promise; - }; - internals.run = async (): Promise => { - runs++; - internals.dirty = false; - }; - - return { syncer, runCount: () => runs }; -} - -describe("Syncer interval", () => { - beforeEach(() => { - mock.timers.enable({ apis: ["setTimeout"] }); - }); - - afterEach(() => { - mock.timers.reset(); - }); - - it("does not schedule a timer when syncIntervalMs is unset", async () => { - const { syncer, runCount } = createSyncer({ - syncIntervalMs: undefined - }); - - syncer.start(); - await syncer.waitUntilFinished(); - mock.timers.tick(60_000); - - assert.strictEqual(runCount(), 1); - await syncer.stop(); - }); - - it("keeps periodic reconciliation opt-in by default", async () => { - assert.strictEqual(DEFAULT_SETTINGS.syncIntervalMs, undefined); - const { syncer, runCount } = createSyncer(); - syncer.start(); - await syncer.waitUntilFinished(); - mock.timers.tick(60_000); - assert.strictEqual(runCount(), 1); - await syncer.stop(); - }); - - it("does not poll when explicitly disabled with zero", async () => { - const { syncer, runCount } = createSyncer({ syncIntervalMs: 0 }); - syncer.start(); - await syncer.waitUntilFinished(); - mock.timers.tick(60_000); - assert.strictEqual(runCount(), 1); - await syncer.stop(); - }); - - it("wakes after the configured sync interval", async () => { - const { syncer, runCount } = createSyncer({ syncIntervalMs: 25 }); - - syncer.start(); - await syncer.waitUntilFinished(); - mock.timers.tick(24); - assert.strictEqual(runCount(), 1); - - mock.timers.tick(1); - assert.strictEqual(runCount(), 2); - await syncer.waitUntilFinished(); - await syncer.stop(); - }); -}); - describe("Syncer per-file state and event delivery", () => { const head = { vaultUpdateId: 1, @@ -132,7 +26,11 @@ describe("Syncer per-file state and event delivery", () => { contentSize: 4 }; - function fixture() { + function fixture(): { + database: Database; + websocket: WebSocketManager; + syncer: Syncer; + } { const database = { state: { local: { document: "note.md" }, @@ -193,22 +91,17 @@ describe("Syncer per-file state and event delivery", () => { assert.strictEqual(syncer.isDocumentUpToDate("missing.md"), false); const internals = syncer as unknown as { - dirty: boolean; unsyncablePaths: Set; }; - internals.dirty = true; - assert.strictEqual(syncer.isDocumentUpToDate("note.md"), true); internals.unsyncablePaths.add("note.md"); assert.strictEqual(syncer.isDocumentUpToDate("note.md"), false); }); it("projects content responses to metadata before persistence", () => { - const { syncer } = fixture(); - const projected = ( - syncer as unknown as { - contentHead: (value: typeof head) => Record; - } - ).contentHead({ ...head, contentBase64: "secret" } as typeof head); + const projected = selectContentHead({ + ...head, + contentBase64: "secret" + } as typeof head); assert.deepEqual(projected, { vaultUpdateId: head.vaultUpdateId, contentSize: head.contentSize @@ -221,19 +114,12 @@ describe("Syncer diff uploads", () => { const encoder = new TextEncoder(); function fixture(cacheSize = 1024): { - syncer: Syncer; + content: ContentSync; parent: Uint8Array; } { const cache = new FixedSizeDocumentCache(cacheSize); const parent = encoder.encode("one two three\n"); - const websocket = { - onWebSocketStatusChanged: new EventListeners< - (connected: boolean) => unknown - >(), - onRemoteVaultUpdateReceived: new EventListeners<() => unknown>() - } as unknown as WebSocketManager; - const syncer = new Syncer( - {} as Logger, + const content = new ContentSync( { state: {} } as Database, new Settings( new Logger(), @@ -243,42 +129,23 @@ describe("Syncer diff uploads", () => { { getDocumentVersionContent: async () => parent } as unknown as SyncService, - websocket, {} as FileOperations, { getConfig: async () => ({ mergeableFileExtensions: ["md"] }) } as ServerConfig, - {} as SyncHistory, cache ); - return { syncer, parent }; + return { content, parent }; } it("sends a diff when the fetched parent is still cached", async () => { - const { syncer, parent } = fixture(); - const internals = syncer as unknown as { - remoteSnapshot: ( - head: { - vaultUpdateId: VaultUpdateId; - documentId: DocumentId; - }, - path: RelativePath - ) => Promise; - pushContent: ( - path: RelativePath, - parentVersionId: VaultUpdateId, - snapshot: Awaited> - ) => Promise< - | { type: "Diff"; value: (number | string)[] } - | { type: "Snapshot"; value: string } - >; - }; - await internals.remoteSnapshot( + const { content, parent } = fixture(); + await content.fetchSnapshot( { vaultUpdateId: 7, documentId: "document" }, "note.md" ); const changed = "one changed three\n"; - const payload = await internals.pushContent( + const payload = await content.createUploadPayload( "note.md", 7, await hashSnapshot({ content: encoder.encode(changed) }) @@ -294,26 +161,12 @@ describe("Syncer diff uploads", () => { }); it("falls back to a snapshot when the parent cannot fit in the cache", async () => { - const { syncer } = fixture(0); - const internals = syncer as unknown as { - remoteSnapshot: ( - head: { - vaultUpdateId: VaultUpdateId; - documentId: DocumentId; - }, - path: RelativePath - ) => Promise; - pushContent: ( - path: RelativePath, - parentVersionId: VaultUpdateId, - snapshot: Awaited> - ) => Promise<{ type: string; value: unknown }>; - }; - await internals.remoteSnapshot( + const { content } = fixture(0); + await content.fetchSnapshot( { vaultUpdateId: 7, documentId: "document" }, "note.md" ); - const payload = await internals.pushContent( + const payload = await content.createUploadPayload( "note.md", 7, await hashSnapshot({ content: encoder.encode("changed") }) diff --git a/frontend/sync-client/src/sync-operations/syncer.ts b/frontend/sync-client/src/sync-operations/syncer.ts index 4ab618d6..68ad8198 100644 --- a/frontend/sync-client/src/sync-operations/syncer.ts +++ b/frontend/sync-client/src/sync-operations/syncer.ts @@ -1,19 +1,21 @@ +import { UploadQueue } from "./upload-queue"; +import type { RemoteApplicationOptions } from "./remote-changes"; +import { + LocalChangeQueue, + type LocalChangeStorage +} from "./local-change-queue"; +import { SyncScheduler } from "./sync-scheduler"; +import { ContentSync, selectContentHead } from "./content-sync"; +import { fetchRemoteChanges } from "./remote-changes"; +import { recordManifestApplication } from "../tracing/record-manifest-application"; import type { SyncHistory } from "../tracing/sync-history"; -import { SyncStatus, SyncType } from "../tracing/sync-history"; import { v4 as uuid } from "uuid"; -import { base64ToBytes, bytesToBase64 } from "byte-base64"; -import { diff, undiff } from "reconcile-text"; import type { Database, - EngineState, - PendingRequest, - ContentHead, DocumentId, - RelativePath, - VaultUpdateId + RelativePath } from "../persistence/database"; -import { emptyState } from "../persistence/database"; -import { hashSnapshot, type HashedSnapshot } from "../snapshot"; +import { createEmptyDatabase } from "../persistence/database"; import type { FileOperations, FileWrite @@ -22,33 +24,23 @@ import type { Settings } from "../persistence/settings"; import type { SyncService } from "../services/sync-service"; import type { ServerConfig } from "../services/server-config"; import type { WebSocketManager } from "../services/websocket-manager"; -import type { DocumentVersionWithoutContent } from "../services/types/DocumentVersionWithoutContent"; -import type { EventBatch } from "../services/types/EventBatch"; -import type { FileManifestEntries } from "../types/file-manifest-entries"; import type { FileManifest } from "../services/types/FileManifest"; import type { VaultSnapshot } from "../services/types/VaultSnapshot"; -import type { PushContent } from "../services/types/PushContent"; import type { Logger } from "../tracing/logger"; import { EventListeners } from "../utils/data-structures/event-listeners"; -import { mergeContent } from "./content"; -import { scanLocalFiles, type LocalChange } from "./scan"; +import { scanLocalFiles } from "./scan"; import { reconcileFileManifest, - manifestToSubmit, + buildUploadManifest, resolvePaths, - sameFileManifest, + haveSameEntries, validateFileManifest } from "./file-manifest"; import { isInternalPath } from "../utils/portable-path"; -import { isBinary } from "../utils/is-binary"; -import { isFileTypeMergeable } from "../utils/is-file-type-mergeable"; -import { decodeText } from "../utils/decode-text"; import type { FixedSizeDocumentCache } from "../utils/data-structures/fix-sized-cache"; import { - AuthenticationError, LocalChangesDuringReconciliation, PermanentSyncError, - ServerVersionMismatchError, SyncResetError, ServerHistoryChangedError } from "../errors/errors"; @@ -60,16 +52,10 @@ export class Syncer { (count: number) => unknown >(); - private running?: Promise; - private timer?: ReturnType; - private stopped = true; - private dirty = false; - private localGeneration = 0; - private failure?: unknown; - private readonly changes: LocalChange[]; - private notificationSaving: Promise = Promise.resolve(); - private notificationSaveFailed = false; - private readonly pendingPaths = new Set(); + private readonly content: ContentSync; + private readonly uploads: UploadQueue; + private readonly scheduler: SyncScheduler; + private readonly localChanges: LocalChangeQueue; private unsyncablePaths = new Set(); private hasScanned = false; @@ -83,19 +69,51 @@ export class Syncer { private readonly serverConfig: ServerConfig, private readonly history: SyncHistory, private readonly contentCache: FixedSizeDocumentCache, - private readonly localChangeStorage: { - entries: LocalChange[]; - save: (entries: LocalChange[]) => Promise; - } = { entries: [], save: async () => {} } + localChangeStorage: LocalChangeStorage = { + entries: [], + save: async () => undefined + } ) { - this.changes = localChangeStorage.entries; + this.scheduler = new SyncScheduler( + settings, + async () => this.synchronize(), + (count) => { + this.onRemainingOperationsCountChanged.trigger(count); + }, + () => this.database.state.pending !== undefined + ); + this.content = new ContentSync( + database, + settings, + service, + files, + serverConfig, + contentCache + ); + this.uploads = new UploadQueue( + database, + settings, + service, + files, + this.content, + history, + async (manifest, options) => + this.applyRemoteChanges(manifest, options) + ); + this.localChanges = new LocalChangeQueue( + files, + () => { + this.requestSync(); + }, + localChangeStorage + ); // Failed upgrades and lost transports still need HTTP catchup, // including when the optional polling interval is disabled. websocket.onWebSocketStatusChanged.add(() => { - this.wake(); + this.requestSync(); }); websocket.onRemoteVaultUpdateReceived.add(async () => { - this.wake(); + this.requestSync(); }); } @@ -104,15 +122,12 @@ export class Syncer { } public get hasLocalChanges(): boolean { - return this.changes.length > 0; + return this.localChanges.entries.length > 0; } public get isBusy(): boolean { return ( - !!this.running || - this.dirty || - !!this.database.state.pending || - !!this.failure + this.scheduler.isBusy || this.database.state.pending !== undefined ); } @@ -120,7 +135,7 @@ export class Syncer { if ( this.database.state.lastSeenUpdateId === undefined || !this.hasScanned || - this.pendingPaths.has(path) || + this.localChanges.pendingPaths.has(path) || this.unsyncablePaths.has(path) || this.settings.isIgnored(path) ) { @@ -128,7 +143,10 @@ export class Syncer { } const id = this.database.findDocumentId(path); - if (id === undefined) return false; + if (id === undefined) { + return false; + } + const document = this.database.state.documents[id]; if ( document?.observedHash === undefined || @@ -139,14 +157,19 @@ export class Syncer { return false; } - const remote = document.remote; - if (remote && remote.vaultUpdateId > (document.base.vaultUpdateId ?? 0)) + const { remote } = document; + if ( + remote && + remote.vaultUpdateId > (document.base.vaultUpdateId ?? 0) + ) { return false; + } const { pending } = this.database.state; if (pending?.type === "content" && pending.documentId === id) { return false; } + if ( pending?.type === "fileManifest" && pending.request.entries[id] !== @@ -159,81 +182,28 @@ export class Syncer { } public start(): void { - this.stopped = false; - this.failure = undefined; this.hasScanned = false; - this.wake(); + this.scheduler.start(); } public async stop(): Promise { - this.stopped = true; - clearTimeout(this.timer); - await this.running; + await this.scheduler.stop(); } - public wake(): void { - if (this.stopped) { - return; - } - - this.dirty = true; - - if (this.running) { - return; - } - - clearTimeout(this.timer); - - this.running = this.run().finally(() => { - this.running = undefined; - this.onRemainingOperationsCountChanged.trigger( - this.failure || this.database.state.pending ? 1 : 0 - ); - - if (this.stopped) { - return; - } - - if (this.dirty && !this.failure) { - this.wake(); - return; - } - - if ( - this.failure instanceof PermanentSyncError || - this.failure instanceof AuthenticationError || - this.failure instanceof ServerVersionMismatchError - ) { - return; - } - - const intervalMs = - this.failure === undefined - ? this.settings.getSettings().syncIntervalMs - : this.settings.getSettings().networkRetryIntervalMs; - if (intervalMs === undefined || intervalMs === 0) { - return; - } - - this.timer = setTimeout(() => { - this.wake(); - }, intervalMs); - }); + public requestSync(): void { + this.scheduler.requestSync(); } public async waitUntilFinished(): Promise { - while (this.running) { - await this.running; - } - if (this.failure) { - throw this.failure; - } + await this.scheduler.waitUntilFinished(); const rejection = this.database.state.pending?.rejection; - if (rejection !== undefined) throw new PermanentSyncError(rejection); + if (rejection !== undefined) { + throw new PermanentSyncError(rejection); + } } public async syncLocallyCreatedFile(path: RelativePath): Promise { - await this.recordLocalChange({ + await this.localChanges.record({ type: "create", path, changeId: uuid() @@ -241,7 +211,7 @@ export class Syncer { } public async syncLocallyDeletedFile(path: RelativePath): Promise { - await this.recordLocalChange({ + await this.localChanges.record({ type: "delete", path, changeId: uuid() @@ -256,11 +226,12 @@ export class Syncer { relativePath: RelativePath; }): Promise { if ( - oldPath && + oldPath !== undefined && + oldPath !== "" && !isInternalPath(oldPath) && !isInternalPath(relativePath) ) { - await this.recordLocalChange({ + await this.localChanges.record({ type: "move", oldPath, relativePath, @@ -268,112 +239,134 @@ export class Syncer { }); } else { // Keystrokes wake reconciliation without invalidating identity plans. - this.pendingPaths.add(relativePath); - this.wake(); + this.localChanges.pendingPaths.add(relativePath); + this.requestSync(); } } + public async flushLocalChanges(): Promise { + await this.localChanges.flush(); + } + /** Reload an uncertain metadata save before starting another attempt. */ public async reloadLocalState(): Promise { await this.flushLocalChanges(); await this.database.recoverPersistence(); } - private async recordLocalChange(change: LocalChange): Promise { - this.localGeneration++; - if (change.type === "move") { - this.pendingPaths.add(change.oldPath); - this.pendingPaths.add(change.relativePath); - } else this.pendingPaths.add(change.path); - const previous = [...this.changes]; - this.changes.push(change); - const saving = this.notificationSaving.then(async () => { - this.files.captureChange(change, previous); - await this.localChangeStorage.save(structuredClone(this.changes)); - this.notificationSaveFailed = false; - }); - this.notificationSaving = saving.catch(() => { - this.notificationSaveFailed = true; - }); - try { - await saving; - } finally { - this.wake(); - } + public async retryRejectedRequests(): Promise { + await this.uploads.retryRejectedRequests(); } - public async flushLocalChanges(): Promise { - await this.notificationSaving; - if (this.notificationSaveFailed) { - await this.localChangeStorage.save(structuredClone(this.changes)); - this.notificationSaveFailed = false; - } - } - - private async run(): Promise { - this.onRemainingOperationsCountChanged.trigger(1); - this.failure = undefined; + private async synchronize(): Promise { try { - await this.reloadLocalState(); - const rejection = this.database.state.pending?.rejection; - if (rejection !== undefined) - throw new PermanentSyncError(rejection); - if (!this.isFirstSyncComplete) await this.service.resetHistory(); - await this.serverConfig.getConfig(); - if (!this.isFirstSyncComplete) await this.bootstrap(); - if (!this.stopped) this.onReadyForEvents.trigger(); - do { - this.dirty = false; - // An unknown accepted request must be resolved before observing a - // newer base, otherwise we can merge our own edit a second time. - if (this.database.state.pending) { - await this.finishPending(); - } - const batch = await this.service.getEvents( - this.database.state.lastSeenUpdateId! - ); - await this.incorporateEventBatch(batch); - if (await this.preparePush()) { - await this.finishPending(); - this.dirty = true; - } - } while (this.dirty && !this.stopped); - if (!this.dirty && !this.database.state.pending) - this.pendingPaths.clear(); + await this.synchronizeUntilIdle(); } catch (error) { - if (error instanceof ServerHistoryChangedError && !this.stopped) { - try { - await this.recoverServerHistory(); - this.dirty = true; - return; - } catch (recoveryError) { - error = recoveryError; - } + await this.handleSyncFailure(error); + } + } + + private async synchronizeUntilIdle(): Promise { + await this.reloadLocalState(); + const rejection = this.database.state.pending?.rejection; + if (rejection !== undefined) { + throw new PermanentSyncError(rejection); + } + + if (!this.isFirstSyncComplete) { + await this.service.resetHistory(); + } + + await this.serverConfig.getConfig(); + if (!this.isFirstSyncComplete) { + await this.initializeFromServerSnapshot(); + } + + if (!this.scheduler.isStopped) { + this.onReadyForEvents.trigger(); + } + + do { + this.scheduler.beginPass(); + // An unknown accepted request must be resolved before observing a + // newer base, otherwise we can merge our own edit a second time. + if (this.database.state.pending) { + await this.uploads.completePendingUpload(); } - if (error instanceof LocalChangesDuringReconciliation) { - this.dirty = true; + + const cursor = this.database.state.lastSeenUpdateId; + if (cursor === undefined) { + throw new Error( + "Cannot fetch events before initializing the vault snapshot" + ); + } + + const batch = await this.service.getEvents(cursor); + const remote = await fetchRemoteChanges( + this.service, + this.database.state, + batch + ); + await this.applyRemoteChanges(remote.manifest, remote); + if (await this.prepareNextUpload()) { + await this.uploads.completePendingUpload(); + this.requestSync(); + } + } while (this.scheduler.needsAnotherPass && !this.scheduler.isStopped); + + if (!this.scheduler.needsAnotherPass && !this.database.state.pending) { + this.localChanges.pendingPaths.clear(); + } + } + + private async handleSyncFailure(error: unknown): Promise { + if ( + error instanceof ServerHistoryChangedError && + !this.scheduler.isStopped + ) { + try { + await this.recoverServerHistory(); + this.requestSync(); + return; + } catch (recoveryError) { + this.handleAttemptFailure(recoveryError); return; } - if (!(error instanceof SyncResetError && this.stopped)) { - this.failure = error; - this.serverConfig.reset(); - this.logger.error(`Sync paused for retry: ${String(error)}`); - } } + + this.handleAttemptFailure(error); + } + + private handleAttemptFailure(error: unknown): void { + if (error instanceof LocalChangesDuringReconciliation) { + this.requestSync(); + return; + } + + if (error instanceof SyncResetError && this.scheduler.isStopped) { + return; + } + + this.serverConfig.reset(); + this.logger.error(`Sync paused for retry: ${String(error)}`); + throw error; } private async recoverServerHistory(): Promise { this.onServerHistoryChanged.trigger(); this.contentCache.reset(); this.hasScanned = false; - await this.scan(); - const guard = this.localGuard(); + await this.refreshLocalFiles(); + const guard = this.localChanges.createGuard(); const previous = this.database.state; - const next = emptyState(previous.vaultKey); + const next = createEmptyDatabase(previous.vaultKey); next.lastAppliedLocalChangeId = previous.lastAppliedLocalChangeId; for (const [id, path] of Object.entries(previous.local)) { const info = await this.files.fs.stat(path); - if (info?.kind !== "file") continue; + if (info?.kind !== "file") { + continue; + } + const doc = previous.documents[id]; if ( this.settings.isIgnored(path) || @@ -385,8 +378,12 @@ export class Syncer { previous.fileManifest.entries[id] ?? null; continue; } - const snapshot = await this.files.snapshot(path); - if (!snapshot) throw new LocalChangesDuringReconciliation(); + + const snapshot = await this.files.readSnapshot(path); + if (!snapshot) { + throw new LocalChangesDuringReconciliation(); + } + const clean = doc?.base?.hash === snapshot.hash; const localId = doc?.base && !clean ? uuid() : id; next.local[localId] = path; @@ -397,85 +394,29 @@ export class Syncer { : undefined }; } + guard(); // An absent cursor durably abandons all old requests and version IDs. - // The next bootstrap clears the transport checkpoint before any request. + // The next initialization clears the transport checkpoint before any request. await this.database.commit(next); } - private localGuard(): () => void { - const generation = this.localGeneration; - return () => { - if (generation !== this.localGeneration) { - throw new LocalChangesDuringReconciliation(); - } - }; - } - - private async cacheContent( - updateId: VaultUpdateId, - path: RelativePath | undefined, - content: Uint8Array - ): Promise { - if ( - path !== undefined && - !isBinary(content) && - isFileTypeMergeable( - path, - (await this.serverConfig.getConfig()).mergeableFileExtensions - ) - ) { - this.contentCache.put(updateId, content); - } - } - - private async pushContent( - path: RelativePath, - parentVersionId: VaultUpdateId | undefined, - snapshot: HashedSnapshot - ): Promise { - const { content } = snapshot; - const parent = - parentVersionId === undefined - ? undefined - : this.contentCache.get(parentVersionId); - if ( - parent !== undefined && - !isBinary(content) && - !isBinary(parent) && - isFileTypeMergeable( - path, - (await this.serverConfig.getConfig()).mergeableFileExtensions - ) - ) { - const value = diff(decodeText(parent), decodeText(content)); - // Match the server's work bound; large edit sets remain syncable. - if (value.length <= 10_000) return { type: "Diff", value }; - } - - return { type: "Snapshot", value: bytesToBase64(content) }; - } - - private next(): EngineState { - return structuredClone(this.database.state); - } - - private async bootstrap(): Promise { + private async initializeFromServerSnapshot(): Promise { const vaultSnapshot = await this.service.getVaultSnapshot(); - await this.reconcile(vaultSnapshot.fileManifest, { + await this.applyRemoteChanges(vaultSnapshot.fileManifest, { cursor: vaultSnapshot.headEventId, initial: vaultSnapshot }); } - private async scan(initial?: VaultSnapshot): Promise { + private async refreshLocalFiles(initial?: VaultSnapshot): Promise { await this.flushLocalChanges(); - const hadChanges = this.changes.length > 0; + const hadChanges = this.localChanges.entries.length > 0; this.unsyncablePaths = await scanLocalFiles( { database: this.database, - guard: this.localGuard(), - changes: this.changes, + guard: this.localChanges.createGuard(), + changes: this.localChanges.entries, files: this.files, ignored: (path) => this.settings.isIgnored(path), oversized: (size) => this.settings.isOversized(size) @@ -483,226 +424,33 @@ export class Syncer { initial ); this.hasScanned = true; - if (hadChanges) - await this.localChangeStorage.save(structuredClone(this.changes)); - } - - private async incorporateEventBatch(batch: EventBatch): Promise { - let cursor = this.database.state.lastSeenUpdateId ?? 0; - let manifest = this.database.state.fileManifest; - const heads: Record = {}; - for (const [id, doc] of Object.entries(this.database.state.documents)) - if (doc.remote) heads[id] = doc.remote; - // Fold every page before touching local files. If fetching is interrupted, - // discard this work and retry from the last committed cursor. - for (;;) { - const after = cursor; - for (const event of batch.events) { - if (event.eventId <= cursor) continue; - if (event.eventId !== cursor + 1) - throw new PermanentSyncError( - "Non-contiguous event history; refusing to skip changes" - ); - cursor = event.eventId; - if (event.type === "fileManifest") { - if ( - event.fileManifest.fileManifestId > - manifest.fileManifestId - ) - manifest = event.fileManifest; - } else if ( - !heads[event.document.documentId] || - heads[event.document.documentId].vaultUpdateId < - event.document.vaultUpdateId - ) { - heads[event.document.documentId] = this.contentHead( - event.document - ); - } - } - const end = batch.endEventId; - if ( - end !== cursor || - end > batch.headEventId || - (end === after && end < batch.headEventId) - ) - throw new PermanentSyncError("Incomplete event batch"); - if (end === batch.headEventId) break; - batch = await this.service.getEvents(cursor); - } - // Three-way merging compares snapshots. Replaying intermediate remote - // edits can discard a local edit even when the remote ends at its base. - await this.reconcile(manifest, { cursor, heads }); - } - - private contentHead(version: ContentHead): ContentHead { - return { - vaultUpdateId: version.vaultUpdateId, - contentSize: version.contentSize - }; - } - - private recordManifestApplication( - before: Readonly, - after: Readonly, - writes: Readonly>, - message: string - ): void { - for (const id of new Set([ - ...Object.keys(before), - ...Object.keys(after) - ])) { - const from = before[id]; - const to = after[id]; - if (from === to && !writes[id]) continue; - this.history.addHistoryEntry({ - status: SyncStatus.SUCCESS, - message, - details: - from && to && from !== to - ? { - type: SyncType.MOVE, - relativePath: to, - movedFrom: from - } - : !from && to - ? { type: SyncType.CREATE, relativePath: to } - : from && !to - ? { type: SyncType.DELETE, relativePath: from } - : { - type: SyncType.UPDATE, - relativePath: to ?? from - } - }); + if (hadChanges) { + await this.localChanges.saveRemainingChanges(); } } - private async remoteSnapshot( - head: Pick< - DocumentVersionWithoutContent, - "documentId" | "vaultUpdateId" - >, - path: RelativePath - ): Promise { - const content = await this.service.getDocumentVersionContent(head); - await this.cacheContent(head.vaultUpdateId, path, content); - return hashSnapshot({ content }); - } - - private async contentWrite( - next: EngineState, - id: DocumentId, - path: RelativePath - ): Promise { - const document = (next.documents[id] ??= {}); - const currentPath = this.database.state.local[id]; - if (currentPath === undefined) delete document.observedHash; - if ( - document.observedHash !== undefined && - document.remote && - document.remote.vaultUpdateId <= (document.base?.vaultUpdateId ?? 0) - ) - delete document.remote; - if ( - next.excluded?.[id] !== undefined || - this.settings.isIgnored(path) || - (next.fileManifest.entries[id] === undefined && !document.remote) || - (currentPath && - document.base && - document.observedHash !== undefined && - !document.remote) - ) - return; - - const head = - document.remote ?? (await this.service.getDocumentMetadata(id)); - if (!head) - throw new PermanentSyncError( - `File manifest references missing content: ${id}` - ); - document.remote = this.contentHead(head); - if (this.settings.isOversized(head.contentSize)) return; - const local = - currentPath && - (await this.files.fs.stat(currentPath))?.kind === "file" - ? await this.files.snapshot(currentPath) - : undefined; - if ( - local !== undefined && - this.settings.isOversized(local.content.length) - ) { - return undefined; - } - if ( - document.base?.vaultUpdateId === null && - local && - local.hash !== document.base.hash - ) - throw new LocalChangesDuringReconciliation(); - const base = - local && document.base - ? document.base.vaultUpdateId === null - ? local - : await this.remoteSnapshot( - { - documentId: id, - vaultUpdateId: document.base.vaultUpdateId - }, - path - ) - : undefined; - // Cache the new head after the older merge base so it remains the most - // recently used version and can back the follow-up local diff. - const remote = await this.remoteSnapshot( - { documentId: id, vaultUpdateId: head.vaultUpdateId }, - path - ); - let result = remote; - if (local !== undefined) { - result = await mergeContent( - path, - base, - local, - remote, - (await this.serverConfig.getConfig()).mergeableFileExtensions - ); - } - document.base = { - vaultUpdateId: head.vaultUpdateId, - hash: remote.hash - }; - delete document.remote; - document.observedHash = result.hash; - return local?.hash === result.hash - ? undefined - : { expected: local, replacement: result }; - } - - private async reconcile( + private async applyRemoteChanges( remote: FileManifest, { cursor, initial, clearPending = false, heads - }: { - cursor?: VaultUpdateId; - initial?: VaultSnapshot; - clearPending?: boolean; - heads?: Record; - } = {} + }: RemoteApplicationOptions = {} ): Promise { validateFileManifest(remote.entries); - await this.scan(initial); + await this.refreshLocalFiles(initial); const before = { ...this.database.state.local }; - const guard = this.localGuard(); - const next = this.next(); + const guard = this.localChanges.createGuard(); + const next = this.database.cloneState(); if (cursor !== undefined) { next.lastSeenUpdateId = cursor; } + if (clearPending) { delete next.pending; } + if (remote.fileManifestId < next.fileManifest.fileManifestId) { await this.database.commit(next); return; @@ -714,20 +462,28 @@ export class Syncer { (path) => this.settings.isIgnored(path), this.unsyncablePaths ); - for (const [id, head] of Object.entries(heads ?? {})) + for (const [id, head] of Object.entries(heads ?? {})) { (next.documents[id] ??= {}).remote = head; - if (initial) { - for (const head of initial.documents) - (next.documents[head.documentId] ??= {}).remote = - this.contentHead(head); } + + if (initial) { + for (const head of initial.documents) { + (next.documents[head.documentId] ??= {}).remote = + selectContentHead(head); + } + } + const writes: Record = {}; for (const [id, path] of Object.entries(next.local)) { - const write = await this.contentWrite(next, id, path); - if (write) writes[id] = write; + const write = await this.content.planDownload(next, id, path); + if (write) { + writes[id] = write; + } } - await this.files.apply(next, writes, guard); - this.recordManifestApplication( + + await this.files.applyChanges(next, writes, guard); + recordManifestApplication( + this.history, before, this.database.state.local, writes, @@ -735,237 +491,59 @@ export class Syncer { ); } - private async preparePush(): Promise { - if (this.stopped || this.database.state.pending) return false; - - let guard = this.localGuard(); + private async prepareUploadPaths(): Promise<() => void> { + let guard = this.localChanges.createGuard(); const canonical = resolvePaths( this.database.state.local, this.database.state.fileManifest.entries, Object.keys(this.database.state.excluded ?? {}), [...this.unsyncablePaths] ); - if (!sameFileManifest(canonical, this.database.state.local)) { - await this.files.apply( - { ...this.next(), local: canonical }, + if (!haveSameEntries(canonical, this.database.state.local)) { + await this.files.applyChanges( + { ...this.database.cloneState(), local: canonical }, {}, guard ); - await this.scan(); - guard = this.localGuard(); + await this.refreshLocalFiles(); + guard = this.localChanges.createGuard(); } // Resolve the map after restoring excluded server paths, then make // those allocations visible locally before submitting a manifest. - const submitted = manifestToSubmit( + const submitted = buildUploadManifest( this.database.state, this.unsyncablePaths ); const local = { ...this.database.state.local }; - for (const [id, path] of Object.entries(submitted)) - if (this.database.state.excluded?.[id] === undefined) + for (const [id, path] of Object.entries(submitted)) { + if (this.database.state.excluded?.[id] === undefined) { local[id] = path; - if (!sameFileManifest(local, this.database.state.local)) { - await this.files.apply({ ...this.next(), local }, {}, guard); - await this.scan(); - guard = this.localGuard(); - } - for (const [id, path] of Object.entries(this.database.state.local)) { - if ( - this.settings.isIgnored(path) || - this.database.state.excluded?.[id] !== undefined || - this.database.state.documents[id]?.observedHash === undefined || - this.unsyncablePaths.has(path) || - this.settings.isOversized( - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- Document heads are a sparse ID map at runtime. - this.database.state.documents[id]?.remote?.contentSize ?? 0 - ) - ) { - continue; } - const snapshot = await this.files.snapshot(path); - if ( - !snapshot || - this.settings.isOversized(snapshot.content.length) - ) { - continue; - } - const { base } = this.database.state.documents[id]; - if (base?.hash === snapshot.hash) continue; - guard(); - await this.database.commit({ - ...this.next(), - pending: { - type: "content", - documentId: id, - hash: snapshot.hash, - request: { - requestId: uuid(), - parentVersionId: base?.vaultUpdateId ?? null, - content: await this.pushContent( - path, - base?.vaultUpdateId ?? undefined, - snapshot - ) - } - } - }); - return true; } - const entries = manifestToSubmit( - this.database.state, - this.unsyncablePaths - ); - validateFileManifest(entries); + if (!haveSameEntries(local, this.database.state.local)) { + await this.files.applyChanges( + { ...this.database.cloneState(), local }, + {}, + guard + ); + await this.refreshLocalFiles(); + guard = this.localChanges.createGuard(); + } + + return guard; + } + + private async prepareNextUpload(): Promise { if ( - !sameFileManifest(entries, this.database.state.fileManifest.entries) + this.scheduler.isStopped || + this.database.state.pending !== undefined ) { - guard(); - await this.database.commit({ - ...this.next(), - pending: { - type: "fileManifest", - request: { - requestId: uuid(), - parentFileManifestId: - this.database.state.fileManifest.fileManifestId, - entries - } - } - }); - return true; + return false; } - return false; - } - /** An explicit reset or settings change retries the exact rejected request. */ - public async retryRejectedRequests(): Promise { - await this.database.recoverPersistence(); - const next = this.next(); - if (next.pending?.rejection === undefined) return; - delete next.pending.rejection; - await this.database.commit(next); - this.failure = undefined; - } - - private async sendPending( - pending: PendingRequest, - send: () => Promise - ): Promise { - if (pending.rejection !== undefined) - throw new PermanentSyncError(pending.rejection); - try { - return await send(); - } catch (error) { - if (!(error instanceof PermanentSyncError)) throw error; - // Keep the exact request: an earlier attempt may have committed even - // though this retry was rejected. Resolve it before observing new bases. - await this.database.commit({ - ...this.next(), - pending: { ...pending, rejection: error.message } - }); - this.history.addHistoryEntry({ - status: SyncStatus.ERROR, - message: error.message, - details: { - type: SyncType.SKIPPED, - relativePath: - pending.type === "content" - ? (this.database.state.local[pending.documentId] ?? - pending.documentId) - : "File manifest" - } - }); - throw error; - } - } - - private async finishPending(): Promise { - const { pending } = this.database.state; - if (!pending) return; - - // Responses are ephemeral. An interrupted application retries the exact - // saved request and gets its original acknowledgement from the server. - if (pending.type === "fileManifest") { - const response = await this.sendPending(pending, async () => - this.service.pushFileManifest(pending.request) - ); - if (response.type === "StaleBase") { - await this.reconcile(response, { clearPending: true }); - } else { - const next = this.next(); - next.fileManifest = { - fileManifestId: response.fileManifestId, - entries: pending.request.entries - }; - delete next.pending; - const previous = this.database.state.fileManifest.entries; - await this.database.commit(next); - this.recordManifestApplication( - previous, - next.fileManifest.entries, - {}, - "File manifest change committed" - ); - } - } else { - const response = await this.sendPending(pending, async () => - this.service.putFileContent(pending.documentId, pending.request) - ); - if (response.type === "StaleBase") { - await this.reconcile(this.database.state.fileManifest, { - clearPending: true, - heads: { [pending.documentId]: this.contentHead(response) } - }); - } else { - const next = this.next(); - const doc = (next.documents[pending.documentId] ??= {}); - doc.base = { - vaultUpdateId: response.vaultUpdateId, - hash: pending.hash - }; - delete doc.remote; - delete next.pending; - await this.database.commit(next); - await this.cacheSubmission(pending, response.vaultUpdateId); - this.history.addHistoryEntry({ - status: SyncStatus.SUCCESS, - message: "Content committed", - details: { - type: SyncType.UPDATE, - relativePath: - next.local[pending.documentId] ?? pending.documentId - }, - author: response.userId, - timestamp: new Date(response.updatedDate) - }); - } - } - } - - /** Warm the optional diff cache using the wire payload, never another saved copy. */ - private async cacheSubmission( - pending: Extract, - version: VaultUpdateId - ): Promise { - const { content, parentVersionId } = pending.request; - let bytes: Uint8Array; - if (content.type === "Snapshot") bytes = base64ToBytes(content.value); - else { - const parent = - parentVersionId === null - ? undefined - : this.contentCache.get(parentVersionId); - // A restart may have evicted the parent. Future uploads can use snapshots. - if (!parent) return; - const text = decodeText(parent); - bytes = new TextEncoder().encode(undiff(text, content.value)); - } - await this.cacheContent( - version, - this.database.state.local[pending.documentId], - bytes - ); + const guard = await this.prepareUploadPaths(); + return this.uploads.persistNextUpload(this.unsyncablePaths, guard); } } diff --git a/frontend/sync-client/src/sync-operations/upload-queue.ts b/frontend/sync-client/src/sync-operations/upload-queue.ts new file mode 100644 index 00000000..a035ebc9 --- /dev/null +++ b/frontend/sync-client/src/sync-operations/upload-queue.ts @@ -0,0 +1,261 @@ +import { v4 as uuid } from "uuid"; +import type { + Database, + PendingRequest, + RelativePath +} from "../persistence/database"; +import type { Settings } from "../persistence/settings"; +import type { SyncService } from "../services/sync-service"; +import type { FileOperations } from "../file-operations/file-operations"; +import type { FileManifest } from "../services/types/FileManifest"; +import { PermanentSyncError } from "../errors/errors"; +import { + SyncStatus, + SyncType, + type SyncHistory +} from "../tracing/sync-history"; +import { recordManifestApplication } from "../tracing/record-manifest-application"; +import type { ContentSync } from "./content-sync"; +import { selectContentHead } from "./content-sync"; +import { + buildUploadManifest, + haveSameEntries, + validateFileManifest +} from "./file-manifest"; +import type { RemoteApplicationOptions } from "./remote-changes"; + +/** Save exact requests before sending, and retire them only after applying an acknowledgement. */ +export class UploadQueue { + public constructor( + private readonly database: Database, + private readonly settings: Settings, + private readonly service: SyncService, + private readonly files: FileOperations, + private readonly content: ContentSync, + private readonly history: SyncHistory, + private readonly applyRemoteChanges: ( + manifest: FileManifest, + options: RemoteApplicationOptions + ) => Promise + ) {} + + public async persistNextUpload( + unsyncablePaths: ReadonlySet, + guard: () => void + ): Promise { + if (await this.persistChangedContent(unsyncablePaths, guard)) { + return true; + } + + const entries = buildUploadManifest( + this.database.state, + unsyncablePaths + ); + validateFileManifest(entries); + if ( + !haveSameEntries(entries, this.database.state.fileManifest.entries) + ) { + guard(); + await this.database.commit({ + ...this.database.cloneState(), + pending: { + type: "fileManifest", + request: { + requestId: uuid(), + parentFileManifestId: + this.database.state.fileManifest.fileManifestId, + entries + } + } + }); + return true; + } + + return false; + } + + public async retryRejectedRequests(): Promise { + await this.database.recoverPersistence(); + const next = this.database.cloneState(); + if (next.pending?.rejection === undefined) { + return; + } + + delete next.pending.rejection; + await this.database.commit(next); + } + + public async completePendingUpload(): Promise { + const { pending } = this.database.state; + if (!pending) { + return; + } + + // Interrupted application retries this exact saved request and receives + // its original acknowledgement, even if the server has advanced since. + if (pending.type === "fileManifest") { + await this.completeManifestUpload(pending); + } else { + await this.completeContentUpload(pending); + } + } + + private async persistChangedContent( + unsyncablePaths: ReadonlySet, + guard: () => void + ): Promise { + for (const [id, path] of Object.entries(this.database.state.local)) { + if ( + this.settings.isIgnored(path) || + this.database.state.excluded?.[id] !== undefined || + this.database.state.documents[id]?.observedHash === undefined || + unsyncablePaths.has(path) || + this.settings.isOversized( + this.database.state.documents[id].remote?.contentSize ?? 0 + ) + ) { + continue; + } + + const snapshot = await this.files.readSnapshot(path); + if ( + !snapshot || + this.settings.isOversized(snapshot.content.length) + ) { + continue; + } + + const { base } = this.database.state.documents[id]; + if (base?.hash === snapshot.hash) { + continue; + } + + guard(); + await this.database.commit({ + ...this.database.cloneState(), + pending: { + type: "content", + documentId: id, + hash: snapshot.hash, + request: { + requestId: uuid(), + parentVersionId: base?.vaultUpdateId ?? null, + content: await this.content.createUploadPayload( + path, + base?.vaultUpdateId ?? undefined, + snapshot + ) + } + } + }); + return true; + } + + return false; + } + + private async completeManifestUpload( + pending: Extract + ): Promise { + const response = await this.sendPersistedRequest(pending, async () => + this.service.pushFileManifest(pending.request) + ); + if (response.type === "StaleBase") { + await this.applyRemoteChanges(response, { clearPending: true }); + } else { + const next = this.database.cloneState(); + next.fileManifest = { + fileManifestId: response.fileManifestId, + entries: pending.request.entries + }; + delete next.pending; + const previous = this.database.state.fileManifest.entries; + await this.database.commit(next); + recordManifestApplication( + this.history, + previous, + next.fileManifest.entries, + {}, + "File manifest change committed" + ); + } + } + + private async completeContentUpload( + pending: Extract + ): Promise { + const response = await this.sendPersistedRequest(pending, async () => + this.service.putFileContent(pending.documentId, pending.request) + ); + if (response.type === "StaleBase") { + await this.applyRemoteChanges(this.database.state.fileManifest, { + clearPending: true, + heads: { + [pending.documentId]: selectContentHead(response) + } + }); + } else { + const next = this.database.cloneState(); + const doc = (next.documents[pending.documentId] ??= {}); + doc.base = { + vaultUpdateId: response.vaultUpdateId, + hash: pending.hash + }; + delete doc.remote; + delete next.pending; + await this.database.commit(next); + await this.content.cacheAcceptedUpload( + pending, + response.vaultUpdateId + ); + this.history.addHistoryEntry({ + status: SyncStatus.SUCCESS, + message: "Content committed", + details: { + type: SyncType.UPDATE, + relativePath: + next.local[pending.documentId] ?? pending.documentId + }, + author: response.userId, + timestamp: new Date(response.updatedDate) + }); + } + } + + private async sendPersistedRequest( + pending: PendingRequest, + send: () => Promise + ): Promise { + if (pending.rejection !== undefined) { + throw new PermanentSyncError(pending.rejection); + } + + try { + return await send(); + } catch (error) { + if (!(error instanceof PermanentSyncError)) { + throw error; + } + + // Keep the exact request: an earlier attempt may have committed even + // though this retry was rejected. Resolve it before observing new bases. + await this.database.commit({ + ...this.database.cloneState(), + pending: { ...pending, rejection: error.message } + }); + this.history.addHistoryEntry({ + status: SyncStatus.ERROR, + message: error.message, + details: { + type: SyncType.SKIPPED, + relativePath: + pending.type === "content" + ? (this.database.state.local[pending.documentId] ?? + pending.documentId) + : "File manifest" + } + }); + throw error; + } + } +} diff --git a/frontend/sync-client/src/tracing/record-manifest-application.ts b/frontend/sync-client/src/tracing/record-manifest-application.ts new file mode 100644 index 00000000..e09e6bb5 --- /dev/null +++ b/frontend/sync-client/src/tracing/record-manifest-application.ts @@ -0,0 +1,55 @@ +import { + SyncStatus, + SyncType, + type SyncHistory, + type SyncDetails +} from "./sync-history"; +import type { DocumentId, RelativePath } from "../persistence/database"; +import type { FileManifestEntries } from "../types/file-manifest-entries"; +import type { FileWrite } from "../file-operations/file-operations"; + +function describeFileChange( + from: RelativePath | undefined, + to: RelativePath | undefined +): SyncDetails | undefined { + if (from !== undefined && to !== undefined) { + return from === to + ? { type: SyncType.UPDATE, relativePath: to } + : { type: SyncType.MOVE, relativePath: to, movedFrom: from }; + } + + if (to !== undefined) { + return { type: SyncType.CREATE, relativePath: to }; + } + + if (from !== undefined) { + return { type: SyncType.DELETE, relativePath: from }; + } + + return undefined; +} + +export function recordManifestApplication( + history: SyncHistory, + before: Readonly, + after: Readonly, + writes: Readonly>, + message: string +): void { + for (const id of new Set([...Object.keys(before), ...Object.keys(after)])) { + const from = before[id]; + const to = after[id]; + if (from === to && writes[id] === undefined) { + continue; + } + + const details = describeFileChange(from, to); + if (details !== undefined) { + history.addHistoryEntry({ + status: SyncStatus.SUCCESS, + message, + details + }); + } + } +} diff --git a/frontend/sync-client/src/tracing/sync-history.ts b/frontend/sync-client/src/tracing/sync-history.ts index 31f77283..a0e0b348 100644 --- a/frontend/sync-client/src/tracing/sync-history.ts +++ b/frontend/sync-client/src/tracing/sync-history.ts @@ -88,11 +88,11 @@ export class SyncHistory { } /** - * Insert the entry at the beginning of the history list. If the entry - * already in the list, it will get moved to the beginning and updated. - * - * If the entry list is too long, the oldest entry will be removed. - */ + * Insert the entry at the beginning of the history list. If the entry + * already in the list, it will get moved to the beginning and updated. + * + * If the entry list is too long, the oldest entry will be removed. + */ public addHistoryEntry(entry: CommonHistoryEntry): void { const historyEntry = { ...entry, diff --git a/frontend/sync-client/src/utils/abortable.ts b/frontend/sync-client/src/utils/abortable.ts index 96044429..27110a04 100644 --- a/frontend/sync-client/src/utils/abortable.ts +++ b/frontend/sync-client/src/utils/abortable.ts @@ -13,6 +13,7 @@ export async function abortable( // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors -- Only reset errors and timeout errors reach this helper. reject(signal.reason); }; + signal.addEventListener("abort", abort, { once: true }); }), operation() @@ -20,4 +21,4 @@ export async function abortable( } finally { signal.removeEventListener("abort", abort); } -} \ No newline at end of file +} diff --git a/frontend/sync-client/src/utils/await-all.test.ts b/frontend/sync-client/src/utils/await-all.test.ts index a8661307..0467b7c1 100644 --- a/frontend/sync-client/src/utils/await-all.test.ts +++ b/frontend/sync-client/src/utils/await-all.test.ts @@ -70,5 +70,6 @@ void test("awaitAll drains unfinished work before reporting a rejection", async } finally { pending.resolve(undefined); } + await checked; }); diff --git a/frontend/sync-client/src/utils/await-all.ts b/frontend/sync-client/src/utils/await-all.ts index 1f91663c..b69e6c0b 100644 --- a/frontend/sync-client/src/utils/await-all.ts +++ b/frontend/sync-client/src/utils/await-all.ts @@ -13,6 +13,7 @@ export async function awaitAll( if (res.status === "rejected") { throw res.reason; } + return res.value; }) as ResolvedTuple; } diff --git a/frontend/sync-client/src/utils/data-structures/event-listeners.test.ts b/frontend/sync-client/src/utils/data-structures/event-listeners.test.ts index bbf497b2..af74c76a 100644 --- a/frontend/sync-client/src/utils/data-structures/event-listeners.test.ts +++ b/frontend/sync-client/src/utils/data-structures/event-listeners.test.ts @@ -66,8 +66,10 @@ describe("EventListeners", () => { const listeners = new EventListeners<() => void>(); // eslint-disable-next-line @typescript-eslint/no-empty-function const listener1 = (): void => {}; + // eslint-disable-next-line @typescript-eslint/no-empty-function const listener2 = (): void => {}; + // eslint-disable-next-line @typescript-eslint/no-empty-function const listener3 = (): void => {}; @@ -114,6 +116,7 @@ describe("EventListeners", () => { const listener1 = (): void => { count1++; }; + const listener2 = (): void => { count2++; }; @@ -158,7 +161,7 @@ describe("EventListeners", () => { it("awaits every async listener even when another listener throws synchronously", async () => { const listeners = new EventListeners<() => unknown>(); - const release = Promise.withResolvers(); + const release = Promise.withResolvers(); const error = new Error("observer failed"); let completed = false; let laterListenerCalled = false; @@ -182,7 +185,7 @@ describe("EventListeners", () => { await Promise.resolve(); assert(laterListenerCalled); assert(!settled); - release.resolve(); + release.resolve(undefined); await result; assert(completed); }); diff --git a/frontend/sync-client/src/utils/data-structures/fix-sized-cache.ts b/frontend/sync-client/src/utils/data-structures/fix-sized-cache.ts index 02954a83..14e9e3e0 100644 --- a/frontend/sync-client/src/utils/data-structures/fix-sized-cache.ts +++ b/frontend/sync-client/src/utils/data-structures/fix-sized-cache.ts @@ -5,7 +5,7 @@ export class FixedSizeDocumentCache { private currentSizeInBytes = 0; private readonly cache = new Map(); - public constructor(private maxSizeInBytes: number) { } + public constructor(private maxSizeInBytes: number) {} public get(updateId: VaultUpdateId): Uint8Array | undefined { const content = this.cache.get(updateId); @@ -19,7 +19,9 @@ export class FixedSizeDocumentCache { } public put(updateId: VaultUpdateId, content: Uint8Array): void { - if (content.byteLength > this.maxSizeInBytes) return; + if (content.byteLength > this.maxSizeInBytes) { + return; + } this.currentSizeInBytes -= this.cache.get(updateId)?.byteLength ?? 0; this.cache.delete(updateId); @@ -41,7 +43,10 @@ export class FixedSizeDocumentCache { private fitBelowMaxSize(): void { for (const [id, content] of this.cache) { - if (this.currentSizeInBytes <= this.maxSizeInBytes) break; + if (this.currentSizeInBytes <= this.maxSizeInBytes) { + break; + } + this.cache.delete(id); this.currentSizeInBytes -= content.byteLength; } diff --git a/frontend/sync-client/src/utils/data-structures/locks.test.ts b/frontend/sync-client/src/utils/data-structures/locks.test.ts index e325496b..9819a5e6 100644 --- a/frontend/sync-client/src/utils/data-structures/locks.test.ts +++ b/frontend/sync-client/src/utils/data-structures/locks.test.ts @@ -4,12 +4,12 @@ import { Lock } from "./locks"; test("queued operations run in order and retain their results", async () => { const lock = new Lock(); - const held = Promise.withResolvers(); - const started = Promise.withResolvers(); + const held = Promise.withResolvers(); + const started = Promise.withResolvers(); const calls: number[] = []; const first = lock.withLock(async () => { calls.push(1); - started.resolve(); + started.resolve(undefined); await held.promise; calls.push(2); return "first"; @@ -24,7 +24,7 @@ test("queued operations run in order and retain their results", async () => { }); await started.promise; assert.deepEqual(calls, [1]); - held.resolve(); + held.resolve(undefined); assert.equal(await first, "first"); assert.equal(await second, "second"); assert.equal(await third, "third"); @@ -35,8 +35,11 @@ for (const asynchronous of [false, true]) { test(`a ${asynchronous ? "rejected promise" : "thrown error"} releases the lock`, async () => { const lock = new Lock(); const error = new Error("failed operation"); - const failed = lock.withLock(() => { - if (asynchronous) return Promise.reject(error); + const failed = lock.withLock(async () => { + if (asynchronous) { + return Promise.reject(error); + } + throw error; }); const next = lock.withLock(() => "continued"); diff --git a/frontend/sync-client/src/utils/debugging/slow-web-socket-factory.ts b/frontend/sync-client/src/utils/debugging/slow-web-socket-factory.ts index ff55da55..97381cab 100644 --- a/frontend/sync-client/src/utils/debugging/slow-web-socket-factory.ts +++ b/frontend/sync-client/src/utils/debugging/slow-web-socket-factory.ts @@ -7,9 +7,11 @@ export function slowWebSocketFactory( logger: Logger ): typeof WebSocket { const delay = async (): Promise => { - if (jitterScaleInSeconds > 0) + if (jitterScaleInSeconds > 0) { await sleep(Math.random() * jitterScaleInSeconds * 1000); + } }; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion return class FlakyWebSocket extends WebSocket { private readonly receiving = new Lock(); diff --git a/frontend/sync-client/src/utils/get-random-color.ts b/frontend/sync-client/src/utils/get-random-color.ts index 38015734..8e828ef1 100644 --- a/frontend/sync-client/src/utils/get-random-color.ts +++ b/frontend/sync-client/src/utils/get-random-color.ts @@ -4,6 +4,7 @@ export function getRandomColor(name: string): string { hash = (hash << 5) - hash + name.charCodeAt(i); hash |= 0; // Convert to 32bit integer } + const normalised = hash / 0x7fffffff; return `oklch(0.58 0.15 ${Math.round(Math.abs(normalised * 360))})`; } diff --git a/frontend/sync-client/src/utils/globs-to-regexes.test.ts b/frontend/sync-client/src/utils/globs-to-regexes.test.ts index f2b11787..2eed4911 100644 --- a/frontend/sync-client/src/utils/globs-to-regexes.test.ts +++ b/frontend/sync-client/src/utils/globs-to-regexes.test.ts @@ -7,6 +7,7 @@ describe("globsToRegexes", () => { it("basicExample", async () => { const [regex] = globsToRegexes([".git/**"], new Logger()); + assert.ok(regex); assert.ok(regex.test(".git/objects/object")); assert.ok(regex.test(".git/objects/.object")); }); diff --git a/frontend/sync-client/src/utils/globs-to-regexes.ts b/frontend/sync-client/src/utils/globs-to-regexes.ts index 1cd048d3..291b4e9e 100644 --- a/frontend/sync-client/src/utils/globs-to-regexes.ts +++ b/frontend/sync-client/src/utils/globs-to-regexes.ts @@ -13,6 +13,7 @@ export function globsToRegexes(globs: string[], logger: Logger): RegExp[] { `Failed to parse ${pattern}' as a glob pattern, skipping it` ); } + return result; }) // eslint-disable-next-line no-restricted-syntax -- Filtering out false values, not removing a specific item diff --git a/frontend/sync-client/src/utils/is-binary.ts b/frontend/sync-client/src/utils/is-binary.ts index a5b897e4..494c71ab 100644 --- a/frontend/sync-client/src/utils/is-binary.ts +++ b/frontend/sync-client/src/utils/is-binary.ts @@ -2,7 +2,9 @@ import { decodeText } from "./decode-text"; // Text is unlikely to contain null bytes, so we can use that to distinguish binary files. export function isBinary(content: Uint8Array): boolean { - if (content.includes(0)) return true; + if (content.includes(0)) { + return true; + } try { decodeText(content); diff --git a/frontend/sync-client/src/utils/is-file-type-mergeable.test.ts b/frontend/sync-client/src/utils/is-file-type-mergeable.test.ts index 78173b54..4c418268 100644 --- a/frontend/sync-client/src/utils/is-file-type-mergeable.test.ts +++ b/frontend/sync-client/src/utils/is-file-type-mergeable.test.ts @@ -5,13 +5,19 @@ import { isFileTypeMergeable } from "./is-file-type-mergeable"; const mergeableExtensions = ["md", "txt"]; describe("isFileTypeMergeable", () => { it("should return true for .md files", () => { - assert.strictEqual(isFileTypeMergeable(".md", mergeableExtensions), true); + assert.strictEqual( + isFileTypeMergeable(".md", mergeableExtensions), + true + ); assert.strictEqual( isFileTypeMergeable("hi.md", mergeableExtensions), true ); assert.strictEqual( - isFileTypeMergeable("my/path/to/my/document.md", mergeableExtensions), + isFileTypeMergeable( + "my/path/to/my/document.md", + mergeableExtensions + ), true ); }); @@ -40,7 +46,10 @@ describe("isFileTypeMergeable", () => { true ); assert.strictEqual( - isFileTypeMergeable("my/path/to/my/DOCUMENT.MD", mergeableExtensions), + isFileTypeMergeable( + "my/path/to/my/DOCUMENT.MD", + mergeableExtensions + ), true ); assert.strictEqual( diff --git a/frontend/sync-client/src/utils/is-file-type-mergeable.ts b/frontend/sync-client/src/utils/is-file-type-mergeable.ts index d62dc270..0c265966 100644 --- a/frontend/sync-client/src/utils/is-file-type-mergeable.ts +++ b/frontend/sync-client/src/utils/is-file-type-mergeable.ts @@ -4,7 +4,10 @@ export function isFileTypeMergeable( ): boolean { const name = pathOrFileName.split("/").at(-1) ?? ""; const separator = name.lastIndexOf("."); - if (separator < 0) return false; + if (separator < 0) { + return false; + } + const fileExtension = name.slice(separator + 1); return mergeableExtensions.includes(fileExtension.toLowerCase()); diff --git a/frontend/sync-client/src/utils/line-and-column-to-position.ts b/frontend/sync-client/src/utils/line-and-column-to-position.ts index 05ac7be8..f1fd1344 100644 --- a/frontend/sync-client/src/utils/line-and-column-to-position.ts +++ b/frontend/sync-client/src/utils/line-and-column-to-position.ts @@ -15,17 +15,18 @@ export function lineAndColumnToPosition( ): number { const lines = text.replaceAll("\r", "").split("\n"); - if (line >= lines.length) { + const selectedLine = lines[line]; + if (selectedLine === undefined) { throw new Error(`Line number ${line} is out of range.`); } - if (column > lines[line].length) { + if (column > selectedLine.length) { throw new Error(`Column number ${column} is out of range.`); } let position = 0; - for (let i = 0; i < line; i++) { - position += lines[i].length + 1; + for (const precedingLine of lines.slice(0, line)) { + position += precedingLine.length + 1; } position += column; diff --git a/frontend/sync-client/src/utils/portable-path.test.ts b/frontend/sync-client/src/utils/portable-path.test.ts index b8869eaf..bc79be8a 100644 --- a/frontend/sync-client/src/utils/portable-path.test.ts +++ b/frontend/sync-client/src/utils/portable-path.test.ts @@ -78,6 +78,9 @@ test("shared directories remain distinct from aliases and file ancestors", () => ["Notes", "Notes/one.md"], ["Notes/one.md", "Notes"], ["Notes/one.md", "Notes/one.md"] - ]) - assert.throws(() => validatePortablePaths(paths), /Conflicting path/); + ]) { + assert.throws(() => { + validatePortablePaths(paths); + }, /Conflicting path/); + } }); diff --git a/frontend/sync-client/src/utils/portable-path.ts b/frontend/sync-client/src/utils/portable-path.ts index 443c2e29..83adb6a6 100644 --- a/frontend/sync-client/src/utils/portable-path.ts +++ b/frontend/sync-client/src/utils/portable-path.ts @@ -2,14 +2,18 @@ import type { DocumentId, RelativePath } from "../persistence/database"; export const INTERNAL_DIRECTORY = ".vault-link-sync"; const MAX_COMPONENT_BYTES = 255; -const byteLength = (value: string): number => +const getByteLength = (value: string): number => new TextEncoder().encode(value).length; -function truncate(value: string, limit: number): string { +function truncateToByteLength(value: string, limit: number): string { let result = ""; for (const character of value) { - if (byteLength(result + character) > limit) break; + if (getByteLength(result + character) > limit) { + break; + } + result += character; } + return result; } @@ -17,13 +21,13 @@ const reservedDeviceName = /^(CON|PRN|AUX|NUL|CONIN\$|CONOUT\$|COM[1-9¹²³]|LPT[1-9¹²³])$/u; // NFC -> uppercase -> NFC matches the server's locale-independent alias rules. -const pathKey = (path: RelativePath): string => +const getPathKey = (path: RelativePath): string => path.normalize("NFC").toUpperCase().normalize("NFC"); export const arePathAliases = ( left: RelativePath, right: RelativePath -): boolean => pathKey(left) === pathKey(right); +): boolean => getPathKey(left) === getPathKey(right); export function findPathWithSameSpelling( paths: Iterable, @@ -31,18 +35,21 @@ export function findPathWithSameSpelling( ): RelativePath | undefined { const normalized = wanted.normalize("NFC"); for (const path of paths) { - if (path !== wanted && path.normalize("NFC") === normalized) + if (path !== wanted && path.normalize("NFC") === normalized) { return path; + } } + return undefined; } export const isInternalPath = (path: RelativePath): boolean => - arePathAliases(path.split("/")[0], INTERNAL_DIRECTORY); + arePathAliases(path.split("/")[0] ?? "", INTERNAL_DIRECTORY); function validatePath(path: RelativePath): void { - if (!path || path !== path.normalize("NFC") || isInternalPath(path)) + if (!path || path !== path.normalize("NFC") || isInternalPath(path)) { throw new Error(`Invalid portable path: ${path}`); + } for (const part of path.split("/")) { if ( @@ -51,10 +58,11 @@ function validatePath(path: RelativePath): void { part === ".." || /[<>:"\\|?*\p{Cc}]|[. ]$/u.test(part) || reservedDeviceName.test( - part.split(".")[0].replace(/ +$/u, "").toUpperCase() + (part.split(".")[0] ?? "").replace(/ +$/u, "").toUpperCase() ) - ) + ) { throw new Error(`Invalid portable path: ${path}`); + } } } @@ -71,13 +79,15 @@ export function validatePortablePaths(paths: Iterable): void { let parent = 0; parts.forEach((spelling, index) => { const file = index === parts.length - 1; - const key = `${parent}/${pathKey(spelling)}`; + const key = `${parent}/${getPathKey(spelling)}`; const previous = nodes.get(key); if ( previous && (previous.spelling !== spelling || previous.file || file) - ) + ) { throw new Error(`Conflicting path: ${path}`); + } + const node = previous ?? { id: nodes.size + 1, spelling, file }; nodes.set(key, node); parent = node.id; @@ -85,42 +95,56 @@ export function validatePortablePaths(paths: Iterable): void { } } -function portableName(part: string): string { +function sanitizeFileName(part: string): string { let result = part .normalize("NFC") .replace(/[<>:"\\|?*\p{Cc}]/gu, "_") .replace(/[. ]+$/u, "_"); - if (!result || result === "." || result === "..") result = "_"; + if (!result || result === "." || result === "..") { + result = "_"; + } + if ( reservedDeviceName.test( - result.split(".")[0].replace(/ +$/u, "").toUpperCase() + (result.split(".")[0] ?? "").replace(/ +$/u, "").toUpperCase() ) - ) + ) { result = "_" + result; + } + return result; } function fitName(name: string, suffix: string, file: boolean): string { const dot = file ? name.lastIndexOf(".") : -1; - const extension = dot > 0 ? truncate(name.slice(dot), 64) : ""; + const extension = dot > 0 ? truncateToByteLength(name.slice(dot), 64) : ""; const stem = dot > 0 ? name.slice(0, dot) : name; - return portableName( - truncate(stem, MAX_COMPONENT_BYTES - byteLength(suffix + extension)) + + return sanitizeFileName( + truncateToByteLength( + stem, + MAX_COMPONENT_BYTES - getByteLength(suffix + extension) + ) + suffix + extension ); } /** First aliased component, or the file occupying a shared path prefix. */ -function conflictIndex( +function findConflictIndex( left: readonly string[], right: readonly string[] ): number | undefined { const shared = Math.min(left.length, right.length); for (let i = 0; i < shared; i++) { - if (!arePathAliases(left[i], right[i])) return; - if (left[i] !== right[i]) return i; + if (!arePathAliases(left[i] ?? "", right[i] ?? "")) { + return; + } + + if (left[i] !== right[i]) { + return i; + } } + return shared - 1; } @@ -130,11 +154,11 @@ export function allocatePortablePath( id: DocumentId, occupied: readonly RelativePath[] ): RelativePath { - id = portableName(truncate(id, 64)); + id = sanitizeFileName(truncateToByteLength(id, 64)); const originals = wanted.split("/"); const parts = originals.map((part, index) => { - const portable = portableName(part); - return byteLength(portable) > MAX_COMPONENT_BYTES + const portable = sanitizeFileName(part); + return getByteLength(portable) > MAX_COMPONENT_BYTES ? fitName( portable, ` (conflict ${id})`, @@ -142,7 +166,10 @@ export function allocatePortablePath( ) : portable; }); - if (isInternalPath(parts.join("/"))) parts[0] = "_" + parts[0]; + if (isInternalPath(parts.join("/"))) { + parts[0] = "_" + parts[0]; + } + const current = [...parts]; const occupiedParts = occupied.map((path) => path.split("/")); const suffix = ` (conflict ${id})`; @@ -150,19 +177,25 @@ export function allocatePortablePath( for (let attempt = 0; attempt <= limit; attempt++) { let conflictAt: number | undefined = undefined; for (const other of occupiedParts) { - const index = conflictIndex(current, other); + const index = findConflictIndex(current, other); if ( index !== undefined && (conflictAt === undefined || index < conflictAt) - ) + ) { conflictAt = index; + } } - if (conflictAt === undefined) return current.join("/"); + + if (conflictAt === undefined) { + return current.join("/"); + } + current[conflictAt] = fitName( - parts[conflictAt], + parts[conflictAt] ?? "", suffix + (attempt ? ` (${attempt})` : ""), conflictAt === current.length - 1 ); } + throw new Error(`Unable to allocate a portable path: ${wanted}`); } diff --git a/frontend/sync-client/src/utils/position-to-line-and-column.ts b/frontend/sync-client/src/utils/position-to-line-and-column.ts index 969171d8..be0828fb 100644 --- a/frontend/sync-client/src/utils/position-to-line-and-column.ts +++ b/frontend/sync-client/src/utils/position-to-line-and-column.ts @@ -27,7 +27,7 @@ export function positionToLineAndColumn( const lines = textUpToPosition.split("\n"); const line = lines.length - 1; - const column = lines[lines.length - 1].length; + const column = (lines.at(-1) ?? "").length; return { line, column }; } diff --git a/frontend/sync-client/src/utils/rate-limit.test.ts b/frontend/sync-client/src/utils/rate-limit.test.ts index 6c7fb434..d68edd38 100644 --- a/frontend/sync-client/src/utils/rate-limit.test.ts +++ b/frontend/sync-client/src/utils/rate-limit.test.ts @@ -37,7 +37,7 @@ describe("rateLimit", () => { await promise2; assert.strictEqual(mockFn.mock.callCount(), 2); - assert.deepStrictEqual(mockFn.mock.calls[1].arguments, [2]); + assert.deepStrictEqual(mockFn.mock.calls[1]?.arguments, [2]); }); it("should use the most recent arguments if multiple calls are made within interval", async () => { @@ -58,7 +58,7 @@ describe("rateLimit", () => { assert.strictEqual(await promise3, undefined); assert.strictEqual(mockFn.mock.callCount(), 2); - assert.deepStrictEqual(mockFn.mock.calls[0].arguments, ["first"]); - assert.deepStrictEqual(mockFn.mock.calls[1].arguments, ["third"]); + assert.deepStrictEqual(mockFn.mock.calls[0]?.arguments, ["first"]); + assert.deepStrictEqual(mockFn.mock.calls[1]?.arguments, ["third"]); }); }); diff --git a/frontend/sync-client/src/utils/rate-limit.ts b/frontend/sync-client/src/utils/rate-limit.ts index 2c97d355..04f66b62 100644 --- a/frontend/sync-client/src/utils/rate-limit.ts +++ b/frontend/sync-client/src/utils/rate-limit.ts @@ -15,7 +15,10 @@ export function rateLimit( // Another waiter may already have consumed the latest arguments. // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- Other calls mutate pending while awaiting the cooldown. - if (pending === undefined) return; + if (pending === undefined) { + return; + } + args = pending; pending = undefined; } diff --git a/frontend/sync-client/src/utils/remove-from-array.ts b/frontend/sync-client/src/utils/remove-from-array.ts index 393b062f..1c97cd7a 100644 --- a/frontend/sync-client/src/utils/remove-from-array.ts +++ b/frontend/sync-client/src/utils/remove-from-array.ts @@ -13,5 +13,6 @@ export function removeFromArray(array: T[], item: T): boolean { array.splice(index, 1); return true; } + return false; } diff --git a/frontend/sync-client/tsconfig.json b/frontend/sync-client/tsconfig.json index 92caf072..19be558b 100644 --- a/frontend/sync-client/tsconfig.json +++ b/frontend/sync-client/tsconfig.json @@ -3,6 +3,7 @@ "module": "ESNext", "target": "ESNext", "strict": true, + "noUncheckedIndexedAccess": true, "allowSyntheticDefaultImports": true, "moduleResolution": "bundler", "lib": [ @@ -12,7 +13,5 @@ "declaration": true, "declarationDir": "./dist/types" }, - "exclude": [ - "./dist" - ] + "exclude": ["./dist"] } diff --git a/frontend/test-support/content-limits.test.ts b/frontend/test-support/content-limits.test.ts index 7b79a3b7..641decc4 100644 --- a/frontend/test-support/content-limits.test.ts +++ b/frontend/test-support/content-limits.test.ts @@ -85,14 +85,7 @@ test("highly fragmented local edits use a snapshot within the server operation b i % 2 ? `changed${i}` : `word${i}` ).join(" "); f.contentCache.put(1, bytes(original)); - const syncer = f.syncer as unknown as { - pushContent( - path: string, - version: number, - snapshot: unknown - ): Promise<{ type: string }>; - }; - const request = await syncer.pushContent( + const request = await f.internals.content.createUploadPayload( "a.md", 1, await hashSnapshot({ content: bytes(edited) }) diff --git a/frontend/test-support/engine-regressions.test.ts b/frontend/test-support/engine-regressions.test.ts index ce67fa47..198a4220 100644 --- a/frontend/test-support/engine-regressions.test.ts +++ b/frontend/test-support/engine-regressions.test.ts @@ -1,7 +1,8 @@ +import { SyncPhase } from "../sync-client/src/sync-operations/sync-scheduler"; import assert from "node:assert/strict"; import { PermanentSyncError } from "../sync-client/src/errors/errors"; import { test } from "node:test"; -import { Database, emptyState } from "../sync-client/src/persistence/database"; +import { Database, createEmptyDatabase } from "../sync-client/src/persistence/database"; import { Syncer } from "../sync-client/src/sync-operations/syncer"; import { SyncClient, type StoredClient } from "../sync-client/src/sync-client"; import { hashSnapshot } from "../sync-client/src/snapshot"; @@ -13,7 +14,7 @@ import { bytes, head, fixture } from "./sync-fixture"; test("offline content notifications need no saves and restart reads the current bytes", async () => { const disk = new MemoryDisk(); await disk.userWrite("a.md", bytes("original")); - const initial = emptyState(JSON.stringify(["http://offline.test", "test"])); + const initial = createEmptyDatabase(JSON.stringify(["http://offline.test", "test"])); initial.lastSeenUpdateId = 0; initial.local = { a: "a.md" }; initial.documents.a = { @@ -44,14 +45,14 @@ test("offline content notifications need no saves and restart reads the current const internals = client as unknown as { database: Database; syncer: { - stopped: boolean; - scan(): Promise; - preparePush(): Promise; + scheduler: { phase: SyncPhase }; + refreshLocalFiles(): Promise; + prepareNextUpload(): Promise; }; }; - await internals.syncer.scan(); - internals.syncer.stopped = false; - assert.equal(await internals.syncer.preparePush(), true); + await internals.syncer.refreshLocalFiles(); + internals.syncer.scheduler.phase = SyncPhase.Idle; + assert.equal(await internals.syncer.prepareNextUpload(), true); const pending = internals.database.state.pending; assert(pending?.type === "content"); assert.equal(pending.documentId, "a"); @@ -71,7 +72,7 @@ test("namespace notifications preserve the acknowledged prefix on restart", asyn const disk = new MemoryDisk(); await disk.userWrite("d.md", bytes("A")); await disk.userWrite("b.md", bytes("B")); - const initial = emptyState(JSON.stringify(["http://offline.test", "test"])); + const initial = createEmptyDatabase(JSON.stringify(["http://offline.test", "test"])); initial.lastSeenUpdateId = 0; initial.local = { a: "c.md", b: "b.md" }; initial.fileManifest.entries = { ...initial.local }; @@ -111,9 +112,9 @@ test("namespace notifications preserve the acknowledged prefix on restart", asyn try { const internals = client as unknown as { database: Database; - syncer: { scan(): Promise }; + syncer: { refreshLocalFiles(): Promise }; }; - await internals.syncer.scan(); + await internals.syncer.refreshLocalFiles(); assert.deepEqual(internals.database.state.local, { a: "d.md", b: "b.md" @@ -129,12 +130,12 @@ test("namespace notifications preserve the acknowledged prefix on restart", asyn test("remote binary replacement and deletion do not create backup artifacts", async () => { const f = await fixture({ "a.bin": "private edit" }); - await f.files.apply(structuredClone(f.database.state), { + await f.files.applyChanges(structuredClone(f.database.state), { a: { replacement: await hashSnapshot({ content: bytes("remote") }) } }); assert.deepEqual(await f.disk.listFilesRecursively(), ["a.bin"]); assert.deepEqual(f.disk.userFiles(), new Map([["a.bin", bytes("remote")]])); - await f.internals.reconcile({ + await f.internals.applyRemoteChanges({ fileManifestId: 2, entries: {} }); @@ -150,7 +151,7 @@ test("rejected content recovery remains idempotent across a rename and network f return bytes("base"); } }); - const snapshot = (await f.files.snapshot("a.md"))!; + const snapshot = (await f.files.readSnapshot("a.md"))!; f.database.state.documents.a.base = { ...head("a", 1, "base"), hash: (await hashSnapshot({ content: bytes("base") })).hash @@ -171,9 +172,9 @@ test("rejected content recovery remains idempotent across a rename and network f oldPath: "a.md", relativePath: "renamed.md" }); - await assert.rejects(f.internals.finishPending(), /offline/); + await assert.rejects(f.internals.uploads.completePendingUpload(), /offline/); offline = false; - await f.internals.finishPending(); + await f.internals.uploads.completePendingUpload(); assert.equal(f.database.state.pending, undefined); assert.equal(f.database.state.local.a, "renamed.md"); }); @@ -203,7 +204,7 @@ test("an unmaterialized file with a directory at its path is deconflicted", asyn f.database.state.fileManifest.entries.a = "folder"; await f.database.save(); f.database.state.documents.a.remote = head("a", 2, "remote"); - await f.internals.reconcile(f.database.state.fileManifest); + await f.internals.applyRemoteChanges(f.database.state.fileManifest); assert( [...f.disk.userFiles().values()].some( (value) => Buffer.from(value).toString() === "remote" @@ -219,7 +220,7 @@ test("an unmaterialized file with a directory at its path is deconflicted", asyn test("ignored materialized content survives remote deletion", async () => { const f = await fixture(); await f.settings.setSettings({ ignorePatterns: ["a.md"] }); - await f.internals.reconcile({ + await f.internals.applyRemoteChanges({ fileManifestId: 2, entries: {} }); @@ -227,8 +228,8 @@ test("ignored materialized content survives remote deletion", async () => { assert.deepEqual(f.database.state.fileManifest.entries, {}); }); -test("unfinished bootstrap cannot be rebound to another vault", async () => { - const database = emptyState(JSON.stringify(["http://example.test", "old"])); +test("unfinished initializeFromServerSnapshot cannot be rebound to another vault", async () => { + const database = createEmptyDatabase(JSON.stringify(["http://example.test", "old"])); // A scan may commit local identities before the first remote application. database.local = { a: "a.md" }; database.documents.a = { observedHash: "" }; @@ -279,7 +280,7 @@ test("a rejected upload pauses all sync across restarts until an explicit retry" const reads = eventReads; await f.disk.userWrite("a.md", bytes("small")); await f.syncer.syncLocallyUpdatedFile({ relativePath: "a.md" }); - f.syncer.wake(); + f.syncer.requestSync(); await assert.rejects(f.syncer.waitUntilFinished(), /413/); assert.equal(eventReads, reads); assert.equal(sent.length, 1); @@ -292,7 +293,7 @@ test("a rejected upload pauses all sync across restarts until an explicit retry" assert.equal(sent.length, 1); reject = false; await f.syncer.retryRejectedRequests(); - f.syncer.wake(); + f.syncer.requestSync(); await f.syncer.waitUntilFinished(); assert.deepEqual(sent.map(({ id }) => id), ["a", "a", "a", "b"]); assert.deepEqual(sent[1], sent[0], "resolve the old request before uploading corrected bytes"); @@ -311,11 +312,11 @@ test("a transient upload failure retries the exact request and recovers its ackn return { type: "Accepted", ...head("a", 2, "local") }; } }); - f.internals.stopped = false; - await f.internals.preparePush(); - await assert.rejects(f.internals.finishPending(), /lost upload reply/); + f.internals.scheduler.phase = SyncPhase.Idle; + await f.internals.prepareNextUpload(); + await assert.rejects(f.internals.uploads.completePendingUpload(), /lost upload reply/); assert(f.database.state.pending); - await f.internals.finishPending(); + await f.internals.uploads.completePendingUpload(); assert.equal(requests.length, 2); assert.deepEqual(requests[1], requests[0]); assert.equal(f.database.state.documents.a.base!.vaultUpdateId, 2); @@ -330,13 +331,13 @@ test("a persisted upload rejection pauses without filesystem writes", async () = throw new PermanentSyncError("HTTP 413"); } }); - f.internals.stopped = false; - await f.internals.preparePush(); + f.internals.scheduler.phase = SyncPhase.Idle; + await f.internals.prepareNextUpload(); f.disk.boundary = (label) => { if (!label.startsWith("read:")) throw new Error("Unexpected filesystem mutation"); }; - await assert.rejects(f.internals.finishPending(), /413/); + await assert.rejects(f.internals.uploads.completePendingUpload(), /413/); assert.equal(uploads, 1); assert.equal(f.database.state.pending?.rejection, "HTTP 413"); assert.deepEqual(f.disk.userFiles(), new Map([["a.md", bytes("local")]])); @@ -351,16 +352,16 @@ test("an explicit retry recovers an earlier accepted upload after a permanent re return { type: "Accepted", ...head("a", 2, "local") }; } }); - f.internals.stopped = false; - await f.internals.preparePush(); - await assert.rejects(f.internals.finishPending(), /413/); + f.internals.scheduler.phase = SyncPhase.Idle; + await f.internals.prepareNextUpload(); + await assert.rejects(f.internals.uploads.completePendingUpload(), /413/); await assert.rejects(f.syncer.waitUntilFinished(), /413/); await f.syncer.retryRejectedRequests(); - await f.internals.finishPending(); + await f.internals.uploads.completePendingUpload(); assert.deepEqual(requests[1], requests[0]); assert.equal(f.database.state.documents.a.base?.vaultUpdateId, 2); assert.equal(f.database.state.pending, undefined); - assert.equal(await f.internals.preparePush(), false); + assert.equal(await f.internals.prepareNextUpload(), false); await f.syncer.waitUntilFinished(); assert.deepEqual(f.disk.userFiles(), new Map([["a.md", bytes("local")]])); }); @@ -390,22 +391,22 @@ test("rejected manifests are resolved before publishing later namespace changes" base: { ...head("a", 1, "x"), hash: "hash" } }; await f.database.save(); - await assert.rejects(f.internals.finishPending(), /413/); - f.internals.stopped = false; - assert.equal(await f.internals.preparePush(), false); + await assert.rejects(f.internals.uploads.completePendingUpload(), /413/); + f.internals.scheduler.phase = SyncPhase.Idle; + assert.equal(await f.internals.prepareNextUpload(), false); f.database.state.local = {}; reject = false; await f.syncer.retryRejectedRequests(); - await f.internals.finishPending(); + await f.internals.uploads.completePendingUpload(); assert.deepEqual(f.database.state.fileManifest.entries, { a: "new.md" }); - assert(await f.internals.preparePush()); - await f.internals.finishPending(); + assert(await f.internals.prepareNextUpload()); + await f.internals.uploads.completePendingUpload(); assert.deepEqual(f.database.state.fileManifest.entries, {}); }); test("offline delete/recreate identities survive restarting before any network connection", async () => { const key = JSON.stringify(["http://offline.test", "test"]); - const initial = emptyState(key); + const initial = createEmptyDatabase(key); initial.lastSeenUpdateId = 0; initial.local.a = "a.md"; initial.documents.a = { observedHash: "" }; @@ -425,10 +426,10 @@ test("offline delete/recreate identities survive restarting before any network c const second = await SyncClient.create({ fs: disk, persistence }); try { const internals = second as unknown as { - syncer: { scan(): Promise }; + syncer: { refreshLocalFiles(): Promise }; database: Database; }; - await internals.syncer.scan(); + await internals.syncer.refreshLocalFiles(); assert.equal(internals.database.state.local.a, undefined); assert.equal( Object.values(internals.database.state.local).filter( @@ -466,12 +467,12 @@ test("offline notifications bind an otherwise empty client to its vault", async test("unignoring a deleted file resumes the deferred remote deletion without resurrection", async () => { const f = await fixture(); await f.settings.setSettings({ ignorePatterns: ["a.md"] }); - await f.internals.reconcile({ + await f.internals.applyRemoteChanges({ fileManifestId: 2, entries: {} }); await f.settings.setSettings({ ignorePatterns: [] }); - await f.internals.reconcile(f.database.state.fileManifest); + await f.internals.applyRemoteChanges(f.database.state.fileManifest); assert.equal(f.disk.userFiles().size, 0); assert.deepEqual(f.database.state.local, {}); }); @@ -479,7 +480,7 @@ test("unignoring a deleted file resumes the deferred remote deletion without res test("an ignored file does not move when a remote rename targets its path", async () => { const f = await fixture({ "a.md": "A", "b.md": "B" }); await f.settings.setSettings({ ignorePatterns: ["a.md"] }); - await f.internals.reconcile({ + await f.internals.applyRemoteChanges({ fileManifestId: 2, entries: { a: "c.md", b: "a.md" } }); @@ -490,8 +491,8 @@ test("an ignored file does not move when a remote rename targets its path", asyn ["b.md", bytes("B")] ]) ); - f.internals.stopped = false; - assert.equal(await f.internals.preparePush(), false); + f.internals.scheduler.phase = SyncPhase.Idle; + assert.equal(await f.internals.prepareNextUpload(), false); assert.deepEqual( f.disk.userFiles(), new Map([ @@ -522,7 +523,7 @@ for (const kind of ["path", "content"] as const) { relativePath: "c.md" }); } - await f.internals.incorporateEventBatch({ + await f.internals.applyEventBatch({ headEventId: 3, endEventId: 3, events: [2, 3].map((id) => @@ -622,15 +623,15 @@ test("ignoring a locally renamed file defers its move without forgetting the loc relativePath: "ignored.md" }); await f.settings.setSettings({ ignorePatterns: ["ignored.md"] }); - await f.internals.scan(); - f.internals.stopped = false; - assert.equal(await f.internals.preparePush(), false); - await f.internals.reconcile({ + await f.internals.refreshLocalFiles(); + f.internals.scheduler.phase = SyncPhase.Idle; + assert.equal(await f.internals.prepareNextUpload(), false); + await f.internals.applyRemoteChanges({ fileManifestId: 2, entries: { a: "a.md" } }); await f.settings.setSettings({ ignorePatterns: [] }); - await f.internals.reconcile(f.database.state.fileManifest); + await f.internals.applyRemoteChanges(f.database.state.fileManifest); assert.equal(f.database.state.local.a, "ignored.md"); assert.equal( Buffer.from( @@ -645,25 +646,25 @@ test("a notified deletion of an ignored file does not delete its server membersh await f.settings.setSettings({ ignorePatterns: ["a.md"] }); await f.disk.userDelete("a.md"); await f.syncer.syncLocallyDeletedFile("a.md"); - await f.internals.scan(); - f.internals.stopped = false; - assert.equal(await f.internals.preparePush(), false); + await f.internals.refreshLocalFiles(); + f.internals.scheduler.phase = SyncPhase.Idle; + assert.equal(await f.internals.prepareNextUpload(), false); assert.equal(f.database.state.local.a, undefined); await f.settings.setSettings({ ignorePatterns: [] }); - await f.internals.reconcile(f.database.state.fileManifest); - assert(await f.internals.preparePush()); + await f.internals.applyRemoteChanges(f.database.state.fileManifest); + assert(await f.internals.prepareNextUpload()); assert.equal(f.database.state.pending?.type, "fileManifest"); }); test("an incoming ignored document cannot force an existing ignored file to move", async () => { const f = await fixture(); await f.settings.setSettings({ ignorePatterns: ["a.md", "*conflict*"] }); - await f.internals.reconcile({ + await f.internals.applyRemoteChanges({ fileManifestId: 2, entries: { a: "b.md", other: "a.md" } }); - f.internals.stopped = false; - await f.internals.preparePush(); + f.internals.scheduler.phase = SyncPhase.Idle; + await f.internals.prepareNextUpload(); assert.deepEqual(f.disk.userFiles(), new Map([["a.md", bytes("local")]])); assert.equal( new Set(Object.values(f.database.state.local)).size, @@ -677,9 +678,9 @@ test("rejected content is reported to callers and an explicit retry clears the r throw new PermanentSyncError("HTTP 413"); } }); - f.internals.stopped = false; - await f.internals.preparePush(); - await assert.rejects(f.internals.finishPending(), /413/); + f.internals.scheduler.phase = SyncPhase.Idle; + await f.internals.prepareNextUpload(); + await assert.rejects(f.internals.uploads.completePendingUpload(), /413/); await assert.rejects(f.syncer.waitUntilFinished(), /413/); assert(f.syncer.isBusy); await f.syncer.retryRejectedRequests(); @@ -689,7 +690,7 @@ test("rejected content is reported to callers and an explicit retry clears the r test("notification saves cannot roll back an uncertain engine commit, even if the first reload fails", async () => { const key = JSON.stringify(["http://offline.test", "test"]); - const initial = emptyState(key); + const initial = createEmptyDatabase(key); initial.lastSeenUpdateId = 0; const disk = new MemoryDisk(); const store = new MemoryPersistence({ @@ -784,7 +785,7 @@ for (const edited of [false, true]) { }); if (edited) await f.disk.userWrite("a.md", bytes("my new edit")); f = await fixture({}, service, f); - await (f.syncer as unknown as { bootstrap(): Promise }).bootstrap(); + await (f.syncer as unknown as { initializeFromServerSnapshot(): Promise }).initializeFromServerSnapshot(); assert.equal(f.database.state.lastSeenUpdateId, 3); assert.deepEqual((await f.disk.read("a.md"))?.content, bytes("restored")); const other = Object.keys(f.database.state.local).filter((id) => id !== "a"); diff --git a/frontend/test-support/event-delivery.test.ts b/frontend/test-support/event-delivery.test.ts index 8f2701c7..70cfb469 100644 --- a/frontend/test-support/event-delivery.test.ts +++ b/frontend/test-support/event-delivery.test.ts @@ -2,7 +2,7 @@ import assert from "node:assert/strict"; import { test } from "node:test"; import { Database, - emptyState, + createEmptyDatabase, type StoredDatabase } from "../sync-client/src/persistence/database"; import { Settings } from "../sync-client/src/persistence/settings"; @@ -29,7 +29,7 @@ for (const [count, delivery] of [ { timeout: 5_000 }, async () => { const logger = new Logger(); - const initial = emptyState("event-delivery"); + const initial = createEmptyDatabase("event-delivery"); initial.lastSeenUpdateId = 0; const persistence = new MemoryPersistence({ database: initial }); const database = new Database( diff --git a/frontend/test-support/file-application.test.ts b/frontend/test-support/file-application.test.ts index 76c856a2..6b99147c 100644 --- a/frontend/test-support/file-application.test.ts +++ b/frontend/test-support/file-application.test.ts @@ -13,7 +13,7 @@ for (const equal of [false, true]) }; const next = structuredClone(f.database.state); next.local = { a: "b.md", b: "a.md" }; - await f.files.apply(next); + await f.files.applyChanges(next); assert.deepEqual(f.database.state.local, next.local); assert.deepEqual( f.disk.userFiles(), @@ -40,7 +40,7 @@ for (const direction of ["file-to-directory", "directory-to-file"]) ? { [ids[0]]: "folder/a.md", [ids[1]]: "b.md" } : { [ids[0]]: "folder", [ids[1]]: "other.md" }; const expected = structuredClone(next.local); - await f.files.apply(next); + await f.files.applyChanges(next); assert.deepEqual(f.database.state.local, expected); assert.deepEqual( new Set(f.disk.userFiles().keys()), @@ -59,7 +59,7 @@ test("an unexpected occupant is preserved under a visible conflict name", async await f.disk.userWrite("remote.md", bytes("unexpected")); const next = structuredClone(f.database.state); next.local.a = "remote.md"; - await f.files.apply(next); + await f.files.applyChanges(next); assert.equal(f.database.state.local.a, "remote.md"); assert.equal(f.disk.userFiles().size, 2); assert.deepEqual( @@ -73,9 +73,9 @@ test("an unexpected occupant is preserved under a visible conflict name", async test("content edited after planning is merged with the incoming snapshot", async () => { const f = await fixture({ "a.md": "base\n" }); - const expected = (await f.files.snapshot("a.md"))!; + const expected = (await f.files.readSnapshot("a.md"))!; await f.disk.userWrite("a.md", bytes("base\nlocal\n")); - await f.files.apply(structuredClone(f.database.state), { + await f.files.applyChanges(structuredClone(f.database.state), { a: { expected, replacement: await hashSnapshot({ @@ -94,7 +94,7 @@ test("deleting the source after planning does not resurrect it", async () => { const next = structuredClone(f.database.state); next.local.a = "remote.md"; await f.disk.userDelete("a.md"); - await f.files.apply(next, { + await f.files.applyChanges(next, { a: { replacement: await hashSnapshot({ content: bytes("remote") }) } }); assert.deepEqual(f.disk.userFiles(), new Map()); @@ -109,7 +109,7 @@ for (const protect of ["ignore", "size"] as const) if (protect === "ignore") await f.settings.setSetting("ignorePatterns", ["a.md"]); else await f.settings.setSetting("maxFileSizeMB", 0); - await f.files.apply(next, { + await f.files.applyChanges(next, { a: { replacement: await hashSnapshot({ content: bytes("remote") @@ -149,12 +149,12 @@ for (const when of ["planning", "installed"]) next.local = { a: "b.md", b: "a.md" }; if (when === "planning") await assert.rejects( - f.files.apply(next), + f.files.applyChanges(next), LocalChangesDuringReconciliation ); - else await f.files.apply(next); + else await f.files.applyChanges(next); assert(changed); - await f.internals.scan(); + await f.internals.refreshLocalFiles(); assert.equal(f.database.state.local.a, "editor.md"); assert.deepEqual( f.disk.userFiles().get(f.database.state.local.a), @@ -189,9 +189,9 @@ for (const action of ["move", "delete"] as const) }; const next = structuredClone(f.database.state); next.local = { a: "remote-a.md", b: "remote-b.md" }; - await f.files.apply(next); + await f.files.applyChanges(next); assert(changed); - await f.internals.scan(); + await f.internals.refreshLocalFiles(); assert.deepEqual(f.disk.userFiles().get("b.md"), bytes("replacement")); assert.equal(f.disk.userFiles().has("remote-b.md"), false); assert.equal( @@ -207,7 +207,7 @@ test("a file and nested directory exchange their exact paths", async () => { const [a, b] = Object.keys(f.database.state.local); const next = structuredClone(f.database.state); next.local = { [a]: "b", [b]: "a/deep/child" }; - await f.files.apply(next); + await f.files.applyChanges(next); assert.deepEqual(f.database.state.local, { [a]: "b", [b]: "a/deep/child" }); assert.deepEqual( f.disk.userFiles(), @@ -223,7 +223,7 @@ for (const phase of ["before", "durable"]) const f = await fixture({ "a.md": "base\n" }); const original = structuredClone(f.database.state); const write = { - expected: (await f.files.snapshot("a.md"))!, + expected: (await f.files.readSnapshot("a.md"))!, replacement: await hashSnapshot({ content: bytes("remote\nbase\n") }) @@ -239,25 +239,25 @@ for (const phase of ["before", "durable"]) } }; await assert.rejects( - f.files.apply(structuredClone(original), { a: write }), + f.files.applyChanges(structuredClone(original), { a: write }), /save failed/ ); f.persistence.boundary = () => {}; await f.database.recoverPersistence(); - await f.files.apply(structuredClone(original), { a: write }); + await f.files.applyChanges(structuredClone(original), { a: write }); assert.deepEqual( f.disk.userFiles().get("a.md"), bytes("remote\nbase\nlocal\n") ); assert.equal( f.database.state.documents.a.observedHash, - (await f.files.snapshot("a.md"))!.hash + (await f.files.readSnapshot("a.md"))!.hash ); }); test("atomic updater includes an edit made after the last snapshot without unlinking the file", async () => { const f = await fixture({ "a.md": "base\n" }); - const expected = (await f.files.snapshot("a.md"))!; + const expected = (await f.files.readSnapshot("a.md"))!; let edited = false; f.disk.boundary = async (label) => { assert( @@ -269,7 +269,7 @@ test("atomic updater includes an edit made after the last snapshot without unlin await f.disk.userWrite("a.md", bytes("base\nlatest edit\n")); } }; - await f.files.apply(structuredClone(f.database.state), { + await f.files.applyChanges(structuredClone(f.database.state), { a: { expected, replacement: await hashSnapshot({ @@ -284,7 +284,7 @@ test("atomic updater includes an edit made after the last snapshot without unlin ); assert.equal( f.database.state.documents.a.observedHash, - (await f.files.snapshot("a.md"))!.hash + (await f.files.readSnapshot("a.md"))!.hash ); }); @@ -295,9 +295,9 @@ test("a failed atomic update leaves database metadata unchanged", async () => { if (label === "before:write:a.md") throw new Error("update failed"); }; await assert.rejects( - f.files.apply(structuredClone(original), { + f.files.applyChanges(structuredClone(original), { a: { - expected: (await f.files.snapshot("a.md"))!, + expected: (await f.files.readSnapshot("a.md"))!, replacement: await hashSnapshot({ content: bytes("remote") }) diff --git a/frontend/test-support/listener-crash-child.ts b/frontend/test-support/listener-crash-child.ts index e9c5bed2..1799ead4 100644 --- a/frontend/test-support/listener-crash-child.ts +++ b/frontend/test-support/listener-crash-child.ts @@ -21,7 +21,7 @@ import { fixture } from "./sync-fixture"; f.syncer.start(); await f.syncer.waitUntilFinished(); const before = delivered; - f.syncer.wake(); + f.syncer.requestSync(); await f.syncer.waitUntilFinished(); assert(delivered > before, "Background sync must remain usable"); } finally { diff --git a/frontend/test-support/polling.test.ts b/frontend/test-support/polling.test.ts index c454e52a..e6a63961 100644 --- a/frontend/test-support/polling.test.ts +++ b/frontend/test-support/polling.test.ts @@ -2,7 +2,7 @@ import assert from "node:assert/strict"; import { test } from "node:test"; import { Database, - emptyState, + createEmptyDatabase, type StoredDatabase } from "../sync-client/src/persistence/database"; import { Settings } from "../sync-client/src/persistence/settings"; @@ -87,7 +87,7 @@ async function fixture(syncIntervalMs: number | undefined) { }; } }; - const initial = emptyState("polling-test"); + const initial = createEmptyDatabase("polling-test"); initial.lastSeenUpdateId = 2; initial.local = { [documentId]: "note.bin" }; initial.fileManifest = { fileManifestId: 2, entries: { ...initial.local } }; diff --git a/frontend/test-support/real-disk.test.ts b/frontend/test-support/real-disk.test.ts index 8a22592e..b8e04dd1 100644 --- a/frontend/test-support/real-disk.test.ts +++ b/frontend/test-support/real-disk.test.ts @@ -4,7 +4,7 @@ import * as fs from "node:fs/promises"; import * as path from "node:path"; import { tmpdir } from "node:os"; import { NodeFileSystemOperations } from "../local-client-cli/src/node-filesystem"; -import { Database, emptyState } from "../sync-client/src/persistence/database"; +import { Database, createEmptyDatabase } from "../sync-client/src/persistence/database"; import { FileOperations } from "../sync-client/src/file-operations/file-operations"; import type { ServerConfig } from "../sync-client/src/services/server-config"; import { hashSnapshot } from "../sync-client/src/snapshot"; @@ -97,7 +97,7 @@ test( "original bytes" ); } - const initial = emptyState("real"); + const initial = createEmptyDatabase("real"); initial.local = { a: "a.md", b: "b.md" }; for (const [id, relative] of Object.entries(initial.local)) initial.documents[id] = { @@ -119,7 +119,7 @@ test( } as ServerConfig); const next = structuredClone(initial); next.local = { a: "b.md", b: "a.md" }; - await operations.apply(next); + await operations.applyChanges(next); assert.equal( Buffer.from( (await disk.read("a.md"))!.content diff --git a/frontend/test-support/recovery.test.ts b/frontend/test-support/recovery.test.ts index cf09206c..d948603f 100644 --- a/frontend/test-support/recovery.test.ts +++ b/frontend/test-support/recovery.test.ts @@ -3,7 +3,7 @@ import { test } from "node:test"; import { MemoryDisk, MemoryPersistence } from "./storage"; import { Database, - emptyState, + createEmptyDatabase, type StoredDatabase } from "../sync-client/src/persistence/database"; import { FileOperations } from "../sync-client/src/file-operations/file-operations"; @@ -28,7 +28,7 @@ async function fixture( } ) { const disk = new MemoryDisk(); - const initial = emptyState("recovery-test"); + const initial = createEmptyDatabase("recovery-test"); for (const [id, { path, text }] of Object.entries(contents)) { await disk.userWrite(path, bytes(text)); initial.local[id] = path; @@ -211,7 +211,7 @@ for (const [name, create] of [ trace(label); }; baseline.persistence.boundary = trace; - await baseline.restart().files.apply(baseline.next, baseline.writes); + await baseline.restart().files.applyChanges(baseline.next, baseline.writes); assert.deepEqual(baseline.disk.userFiles(), baseline.expected); assert.deepEqual( baseline.persistence.snapshot().database!.local, @@ -242,7 +242,7 @@ for (const [name, create] of [ await assert.rejects( current .restart() - .files.apply(current.next, current.writes), + .files.applyChanges(current.next, current.writes), /interrupted/ ); assert(fired); @@ -279,7 +279,7 @@ for (const [name, create] of [ assert.equal( runtime.database.state.documents[id] .observedHash, - (await runtime.files.snapshot(path))!.hash + (await runtime.files.readSnapshot(path))!.hash ); } assert( diff --git a/frontend/test-support/sync-audit.test.ts b/frontend/test-support/sync-audit.test.ts index 6c477e78..fd0f31d9 100644 --- a/frontend/test-support/sync-audit.test.ts +++ b/frontend/test-support/sync-audit.test.ts @@ -1,9 +1,10 @@ +import { SyncPhase } from "../sync-client/src/sync-operations/sync-scheduler"; import assert from "node:assert/strict"; import { SUPPORTED_API_VERSION } from "../sync-client/src/consts"; import { PermanentSyncError } from "../sync-client/src/errors/errors"; import { test } from "node:test"; import type { Database } from "../sync-client/src/persistence/database"; -import { emptyState } from "../sync-client/src/persistence/database"; +import { createEmptyDatabase } from "../sync-client/src/persistence/database"; import { SyncClient } from "../sync-client/src/sync-client"; import { hashSnapshot } from "../sync-client/src/snapshot"; import { EventListeners } from "../sync-client/src/utils/data-structures/event-listeners"; @@ -32,9 +33,9 @@ test("ignored rename must not produce a duplicate submitted path", async () => { relativePath: "a.md" }); await f.settings.setSettings({ ignorePatterns: ["ignored.md"] }); - await f.internals.scan(); - f.internals.stopped = false; - await f.internals.preparePush(); + await f.internals.refreshLocalFiles(); + f.internals.scheduler.phase = SyncPhase.Idle; + await f.internals.prepareNextUpload(); const p = f.database.state.pending; assert(p?.type === "fileManifest"); assert.equal( @@ -51,12 +52,12 @@ test("ignored untracked ancestor must remain in place", async () => { await f.disk.userWrite("private", bytes("LOCAL PRIVATE")); await f.settings.setSettings({ ignorePatterns: ["private"] }); (f.database.state.documents.a ??= {}).remote = head("a", 2, "remote"); - await f.internals.reconcile({ + await f.internals.applyRemoteChanges({ fileManifestId: 3, entries: { a: "private/note.md" } }); - f.internals.stopped = false; - await f.internals.preparePush(); + f.internals.scheduler.phase = SyncPhase.Idle; + await f.internals.prepareNextUpload(); assert.deepEqual(await f.disk.read("private"), { content: bytes("LOCAL PRIVATE") }); @@ -70,7 +71,7 @@ test("oversized untracked file must remain in place", async () => { await f.disk.userWrite("a.md", bytes("LOCAL OVERSIZED")); await f.settings.setSettings({ maxFileSizeMB: 5 / (1024 * 1024) }); (f.database.state.documents.a ??= {}).remote = head("a", 2, "R"); - await f.internals.reconcile({ + await f.internals.applyRemoteChanges({ fileManifestId: 3, entries: { a: "a.md" } }); @@ -105,15 +106,15 @@ test("rejected unknown success must use submitted content as merge base", async }; f.database.state.lastSeenUpdateId = 1; await f.disk.userWrite("a.md", bytes("base A")); - f.internals.stopped = false; - await f.internals.preparePush(); + f.internals.scheduler.phase = SyncPhase.Idle; + await f.internals.prepareNextUpload(); const req = f.database.state.pending!.request.requestId; - await assert.rejects(f.internals.finishPending(), /413/); + await assert.rejects(f.internals.uploads.completePendingUpload(), /413/); await f.disk.userWrite("a.md", bytes("base")); reject = false; await f.syncer.retryRejectedRequests(); - await f.internals.finishPending(); - await f.internals.incorporateEventBatch({ + await f.internals.uploads.completePendingUpload(); + await f.internals.applyEventBatch({ headEventId: 3, endEventId: 3, events: [ @@ -150,7 +151,7 @@ test("bootstrap adopts same-path identity even after multiple notifications", as fileManifest: { fileManifestId: 2, entries: { remoteA: "a.md" } }, documents: [head("remoteA", 1, "remote a")] }; - await f.internals.scan(initial); + await f.internals.refreshLocalFiles(initial); assert.equal(f.database.state.local.remoteA, "a.md"); }); @@ -176,9 +177,9 @@ test("oversized remote head must not cause identical stale uploads forever", asy }; (f.database.state.documents.a ??= {}).remote = head("a", 2, remoteText); await f.settings.setSettings({ maxFileSizeMB: 10 / (1024 * 1024) }); - f.internals.stopped = false; + f.internals.scheduler.phase = SyncPhase.Idle; // Even a small local edit must wait until its required remote base is readable. - assert.equal(await f.internals.preparePush(), false); + assert.equal(await f.internals.prepareNextUpload(), false); assert.equal(attempts, 0); assert.equal(f.database.state.pending, undefined); }); @@ -205,12 +206,12 @@ test("local undo of accepted-but-rejected manifest must survive replay", async ( oldPath: "a.md", relativePath: "b.md" }); - await f.internals.scan(); - f.internals.stopped = false; - await f.internals.preparePush(); + await f.internals.refreshLocalFiles(); + f.internals.scheduler.phase = SyncPhase.Idle; + await f.internals.prepareNextUpload(); const req = f.database.state.pending!.request.requestId; - f.internals.stopped = true; - await assert.rejects(f.internals.finishPending(), /413/); + f.internals.scheduler.phase = SyncPhase.Stopped; + await assert.rejects(f.internals.uploads.completePendingUpload(), /413/); await f.disk.userRename("b.md", "a.md"); await f.syncer.syncLocallyUpdatedFile({ oldPath: "b.md", @@ -218,8 +219,8 @@ test("local undo of accepted-but-rejected manifest must survive replay", async ( }); reject = false; await f.syncer.retryRejectedRequests(); - await f.internals.finishPending(); - await f.internals.incorporateEventBatch({ + await f.internals.uploads.completePendingUpload(); + await f.internals.applyEventBatch({ headEventId: 2, endEventId: 2, events: [ @@ -397,7 +398,7 @@ test("oversized files should be filtered before reading complete snapshots", asy f.disk.boundary = (label) => { if (label === "read:snapshot:huge.bin") reads++; }; - await f.internals.scan(); + await f.internals.refreshLocalFiles(); assert.equal(reads, 0); }); @@ -412,7 +413,7 @@ test("permanent destination failure replans the affected file and finishes the b const next = structuredClone(f.database.state); next.local = { a: "unsupported/a.md", b: "good.md" }; next.fileManifest = { fileManifestId: 2, entries: { ...next.local } }; - await f.files.apply(next); + await f.files.applyChanges(next); await f.settings.setSettings({ ignorePatterns: ["unsupported/**"] }); assert.deepEqual( [...f.disk.userFiles().values()] @@ -508,7 +509,7 @@ for (const interrupted of [false, true]) { await f.database.save(); const saved = f.persistence.snapshot(); const catchUp = async () => - f.internals.incorporateEventBatch( + f.internals.applyEventBatch( await service.getEvents!(f.database.state.lastSeenUpdateId!) ); if (interrupted) { @@ -539,7 +540,7 @@ test("an ignored occupant arriving after the scan keeps its path and privacy", a ); await f.settings.setSettings({ ignorePatterns: ["private"] }); (f.database.state.documents.a ??= {}).remote = head("a", 2, "remote"); - await f.internals.reconcile({ + await f.internals.applyRemoteChanges({ fileManifestId: 3, entries: { a: "private/note.md" } }); @@ -547,8 +548,8 @@ test("an ignored occupant arriving after the scan keeps its path and privacy", a (await f.disk.read("private"))?.content, bytes("SECRET") ); - f.internals.stopped = false; - await f.internals.preparePush(); + f.internals.scheduler.phase = SyncPhase.Idle; + await f.internals.prepareNextUpload(); const { pending } = f.database.state; assert( pending?.type !== "content" || @@ -571,7 +572,7 @@ test("content notifications during every snapshot do not starve a complete names }); } }; - await f.internals.scan(); + await f.internals.refreshLocalFiles(); assert.equal(notifications, 2); assert.deepEqual(f.database.state.local, { a: "a.md", b: "b.md" }); }); @@ -604,10 +605,10 @@ for (const boundary of ["before:save", "durable:save"]) { }; f.database.state.lastSeenUpdateId = 1; await f.disk.userWrite("a.md", bytes("base A")); - f.internals.stopped = false; - await f.internals.preparePush(); + f.internals.scheduler.phase = SyncPhase.Idle; + await f.internals.prepareNextUpload(); const { requestId } = f.database.state.pending!.request; - await assert.rejects(f.internals.finishPending(), /413/); + await assert.rejects(f.internals.uploads.completePendingUpload(), /413/); await f.disk.userWrite("a.md", bytes("base")); const first = { headEventId: 3, @@ -639,15 +640,15 @@ for (const boundary of ["before:save", "durable:save"]) { if (label === boundary) throw new Error("injected crash"); }; await assert.rejects( - f.internals.finishPending(), + f.internals.uploads.completePendingUpload(), /injected crash/ ); f.persistence.boundary = () => { }; f.disk.crash(true); f = await fixture({}, service, f); assert.equal(f.database.state.lastSeenUpdateId, 1); - await f.internals.finishPending(); - await f.internals.incorporateEventBatch(first); + await f.internals.uploads.completePendingUpload(); + await f.internals.applyEventBatch(first); assert.equal(f.database.state.lastSeenUpdateId, 3); assert.deepEqual( (await f.disk.read("a.md"))?.content, @@ -753,21 +754,21 @@ for (const retry of [false, true]) { }; f.database.state.lastSeenUpdateId = 1; await f.disk.userWrite("a.md", bytes("edited")); - f.internals.stopped = false; - await f.internals.preparePush(); - if (retry) await assert.rejects(f.internals.finishPending(), /413/); + f.internals.scheduler.phase = SyncPhase.Idle; + await f.internals.prepareNextUpload(); + if (retry) await assert.rejects(f.internals.uploads.completePendingUpload(), /413/); // Recovery folds queued deletions into the durable namespace before // an outstanding request is retried. - f.internals.stopped = true; + f.internals.scheduler.phase = SyncPhase.Stopped; await f.disk.userDelete("a.md"); await f.syncer.syncLocallyDeletedFile("a.md"); - await f.internals.scan(); + await f.internals.refreshLocalFiles(); assert.deepEqual(f.database.state.local, {}); if (retry) { reject = false; await f.syncer.retryRejectedRequests(); } - await f.internals.finishPending(); + await f.internals.uploads.completePendingUpload(); assert.equal(f.database.state.pending, undefined); assert.deepEqual(f.database.state.local, {}); assert.equal(await f.disk.read("a.md"), undefined); diff --git a/frontend/test-support/sync-fixture.ts b/frontend/test-support/sync-fixture.ts index 69caa363..a14e5a10 100644 --- a/frontend/test-support/sync-fixture.ts +++ b/frontend/test-support/sync-fixture.ts @@ -1,6 +1,13 @@ +import type { UploadQueue } from "../sync-client/src/sync-operations/upload-queue"; +import type { ContentSync } from "../sync-client/src/sync-operations/content-sync"; +import { fetchRemoteChanges, type RemoteApplicationOptions } from "../sync-client/src/sync-operations/remote-changes"; +import type { EventBatch } from "../sync-client/src/services/types/EventBatch"; +import type { FileManifest } from "../sync-client/src/services/types/FileManifest"; +import type { VaultSnapshot } from "../sync-client/src/services/types/VaultSnapshot"; +import { SyncPhase } from "../sync-client/src/sync-operations/sync-scheduler"; import { Database, - emptyState, + createEmptyDatabase, type StoredDatabase } from "../sync-client/src/persistence/database"; import { Settings } from "../sync-client/src/persistence/settings"; @@ -35,7 +42,7 @@ export async function fixture( const disk = recovered?.disk ?? new MemoryDisk(); const initial = recovered ? (recovered.persistence.snapshot().database as StoredDatabase) - : emptyState("test"); + : createEmptyDatabase("test"); if (!recovered) { initial.lastSeenUpdateId = 0; for (const [path, content] of Object.entries(contents)) { @@ -99,19 +106,14 @@ export async function fixture( { entries: changes, save: async () => {} } ); const internals = syncer as unknown as { - finishPending: () => Promise; - preparePush: () => Promise; - scan: ( + content: ContentSync; + uploads: UploadQueue; + prepareNextUpload: () => Promise; + refreshLocalFiles: ( initial?: import("../sync-client/src/services/types/VaultSnapshot").VaultSnapshot ) => Promise; - incorporateEventBatch: ( - batch: import("../sync-client/src/services/types/EventBatch").EventBatch - ) => Promise; - stopped: boolean; - reconcile: (remote: { - fileManifestId: number; - entries: Record; - }) => Promise; + scheduler: { phase: SyncPhase }; + applyRemoteChanges: (remote: FileManifest, options?: RemoteApplicationOptions) => Promise; }; return { disk, @@ -119,7 +121,18 @@ export async function fixture( files, settings, syncer, - internals, + internals: { + content: internals.content, + uploads: internals.uploads, + scheduler: internals.scheduler, + prepareNextUpload: () => internals.prepareNextUpload(), + refreshLocalFiles: (initial?: VaultSnapshot) => internals.refreshLocalFiles(initial), + applyRemoteChanges: (manifest: FileManifest) => internals.applyRemoteChanges(manifest), + applyEventBatch: async (batch: EventBatch) => { + const remote = await fetchRemoteChanges(service as SyncService, database.state, batch); + await internals.applyRemoteChanges(remote.manifest, remote); + } + }, persistence, changes, contentCache diff --git a/sync-server/README.md b/sync-server/README.md index 6e604a2c..17a035cf 100644 --- a/sync-server/README.md +++ b/sync-server/README.md @@ -1,9 +1,29 @@ # Sync server +## Code layout -- ban using unwraps +HTTP handlers live in `src/server/endpoints/`. Content and manifest writes check +for an already accepted request, validate the parent version, write the new event +and data in one transaction, then commit before notifying connected clients. +`endpoints/utils.rs` shares request fingerprinting, replay checks, and commit/notify +ordering. Infrastructure errors convert to `SyncServerError`; validation chooses +client, authentication, or missing-resource errors explicitly. -## API v5 +`app_state/database.rs` manages per-vault connection pools. Its `queries`, +`mutations`, and `history` modules own data reads, writes, and history checkpoints. +`StoredDocumentVersion` describes database rows; the API response models remain +separate. `utils/portable_path.rs` validates individual filenames and the manifest's +file/directory relationships. + +The WebSocket endpoint authenticates once, registers a cursor session, and forwards +updates through `AuthenticatedConnection`. It separates durable-history checks, +client-message handling, and cursor filtering, and always removes the registered +session on exit. `app_state/cursors.rs` retains and expires cursor presence. + +Run `cargo fmt --check`, `cargo clippy --all-targets`, and `cargo test` in this +package. The Rust protocol models generate the client's `services/types/` files. + +## API v4 The server is a compare-and-swap store. Content versions do not contain paths or deletions. A separately versioned file manifest owns the namespace and membership. @@ -20,7 +40,7 @@ All routes below are under `/vaults/:vault_id` and require bearer authentication | `PUT /file-manifest` | CAS the complete file manifest. | | `GET /vault-snapshot` | File manifest, referenced content heads, and event watermark from one read transaction. | | `GET /events-since?after=N` | Every event after N, in order, with the current watermark. | -| `GET /ws` | Ordered event delivery and ephemeral cursor positions. | +| `GET /ws` | Vault-change hints and ephemeral cursor positions. | A content push includes a `Device-Id` header and this JSON body: diff --git a/sync-server/src/app_state.rs b/sync-server/src/app_state.rs index 5293ee1c..45a02fb3 100644 --- a/sync-server/src/app_state.rs +++ b/sync-server/src/app_state.rs @@ -24,7 +24,7 @@ impl AppState { let database = Database::try_new(&config.database).await?; let cursors: Cursors = Cursors::new(&config.database, &broadcasts); - Cursors::start_background_task(cursors.clone()); + Cursors::start_cursor_expiration_task(cursors.clone()); Ok(Self { config, diff --git a/sync-server/src/app_state/cursors.rs b/sync-server/src/app_state/cursors.rs index 231933f7..0a86db35 100644 --- a/sync-server/src/app_state/cursors.rs +++ b/sync-server/src/app_state/cursors.rs @@ -95,7 +95,7 @@ impl Cursors { .unwrap_or_default() } - pub fn start_background_task(self) { + pub fn start_cursor_expiration_task(self) { tokio::spawn(async move { loop { self.remove_expired_cursors().await; diff --git a/sync-server/src/app_state/database.rs b/sync-server/src/app_state/database.rs index 4fa2d394..442cc697 100644 --- a/sync-server/src/app_state/database.rs +++ b/sync-server/src/app_state/database.rs @@ -16,6 +16,7 @@ use tokio::time::Instant; pub mod models; +mod history; mod mutations; mod queries; @@ -83,7 +84,7 @@ impl Database { Ok(database) } - fn database_path(config: &DatabaseConfig, vault: &str) -> PathBuf { + fn get_database_path(config: &DatabaseConfig, vault: &str) -> PathBuf { // Hash the normalized vault name into a fixed-length ASCII filename so // filesystem case/Unicode aliasing (e.g. composed vs. decomposed é on // macOS) cannot make separately authorized vaults share a database. @@ -159,7 +160,7 @@ impl Database { let vault = normalize_string(vault); validate_vault_id(&vault)?; - if let Some(pool) = self.existing_pool(&vault).await { + if let Some(pool) = self.find_cached_pool(&vault).await { return Ok(pool); } @@ -170,11 +171,11 @@ impl Database { let _opening = opening.lock().await; - if let Some(pool) = self.existing_pool(&vault).await { + if let Some(pool) = self.find_cached_pool(&vault).await { return Ok(pool); } - let file_name = Self::database_path(&self.config, &vault); + let file_name = Self::get_database_path(&self.config, &vault); let pool = Self::open_database(&self.config, &file_name).await?; self.connection_pools.lock().await.insert( @@ -187,7 +188,7 @@ impl Database { Ok(pool) } - async fn existing_pool(&self, vault: &VaultId) -> Option> { + async fn find_cached_pool(&self, vault: &VaultId) -> Option> { self.connection_pools .lock() .await diff --git a/sync-server/src/app_state/database/history.rs b/sync-server/src/app_state/database/history.rs new file mode 100644 index 00000000..861c68f5 --- /dev/null +++ b/sync-server/src/app_state/database/history.rs @@ -0,0 +1,48 @@ +//! Checkpoints distinguish restored histories even when numeric event IDs are reused. +use super::Database; +use super::models::VaultId; +use anyhow::Result; + +const EMPTY_HISTORY_CHECKPOINT: &str = "0:empty"; + +impl Database { + pub async fn get_history_checkpoint(&self, vault: &VaultId) -> Result { + let mut tx = self.create_readonly_transaction(vault).await?; + + let row: Option<(i64, String)> = sqlx::query_as( + "SELECT event_id, event_token FROM events ORDER BY event_id DESC LIMIT 1", + ) + .fetch_optional(&mut *tx) + .await?; + + Ok(row.map_or_else( + || EMPTY_HISTORY_CHECKPOINT.to_owned(), + |(id, token)| format!("{id}:{token}"), + )) + } + + pub async fn contains_checkpoint(&self, vault: &VaultId, checkpoint: &str) -> Result { + if checkpoint == EMPTY_HISTORY_CHECKPOINT { + return Ok(true); + } + + let Some((id, token)) = checkpoint.split_once(':') else { + return Ok(false); + }; + + let Ok(id) = id.parse::() else { + return Ok(false); + }; + + let mut tx = self.create_readonly_transaction(vault).await?; + let exists: bool = sqlx::query_scalar( + "SELECT EXISTS(SELECT 1 FROM events WHERE event_id = ? AND event_token = ?)", + ) + .bind(id) + .bind(token) + .fetch_one(&mut *tx) + .await?; + + Ok(exists) + } +} diff --git a/sync-server/src/app_state/database/queries.rs b/sync-server/src/app_state/database/queries.rs index e7b41efc..5e5a8a2a 100644 --- a/sync-server/src/app_state/database/queries.rs +++ b/sync-server/src/app_state/database/queries.rs @@ -9,8 +9,6 @@ use anyhow::{Context as _, Result, ensure}; use crate::consts::MAX_EVENTS_PAGE_BYTES; -const EMPTY_HISTORY_CHECKPOINT: &str = "0:empty"; - impl Database { pub async fn get_missing_document( tx: &mut Transaction<'_>, @@ -36,9 +34,10 @@ impl Database { .fetch_optional(&mut **tx) .await?; match row { - Some((fingerprint, event_id)) => { - Ok(Some((fingerprint, Self::event_by_id(tx, event_id).await?))) - } + Some((fingerprint, event_id)) => Ok(Some(( + fingerprint, + Self::get_event_by_id(tx, event_id).await?, + ))), None => Ok(None), } } @@ -96,7 +95,7 @@ impl Database { }) } - pub async fn current_file_manifest(tx: &mut Transaction<'_>) -> Result { + pub async fn get_current_file_manifest(tx: &mut Transaction<'_>) -> Result { let Some(file_manifest_id) = sqlx::query_scalar::<_, VaultUpdateId>( "SELECT file_manifest_id FROM file_manifests ORDER BY file_manifest_id DESC LIMIT 1", ) @@ -106,12 +105,12 @@ impl Database { return Ok(FileManifest::default()); }; - Self::file_manifest_by_id(tx, file_manifest_id) + Self::get_file_manifest_by_id(tx, file_manifest_id) .await? .context("Latest file manifest is missing") } - async fn file_manifest_by_id( + async fn get_file_manifest_by_id( tx: &mut Transaction<'_>, file_manifest_id: VaultUpdateId, ) -> Result> { @@ -140,7 +139,10 @@ impl Database { })) } - async fn event_by_id(tx: &mut Transaction<'_>, event_id: VaultUpdateId) -> Result { + async fn get_event_by_id( + tx: &mut Transaction<'_>, + event_id: VaultUpdateId, + ) -> Result { let document = sqlx::query_as::<_, DocumentVersionWithoutContent>( "SELECT vault_update_id, document_id, updated_date, user_id, device_id, length(content) AS content_size @@ -152,13 +154,13 @@ impl Database { if let Some(document) = document { return Ok(VaultEvent::Content { document }); } - let file_manifest = Self::file_manifest_by_id(tx, event_id) + let file_manifest = Self::get_file_manifest_by_id(tx, event_id) .await? .context("Event has no normalized content or file manifest")?; Ok(VaultEvent::FileManifest { file_manifest }) } - pub async fn latest_event_id(tx: &mut Transaction<'_>) -> Result { + pub async fn get_latest_event_id(tx: &mut Transaction<'_>) -> Result { Ok( sqlx::query_scalar("SELECT COALESCE(MAX(event_id), 0) FROM events") .fetch_one(&mut **tx) @@ -166,10 +168,10 @@ impl Database { ) } - pub async fn vault_snapshot(&self, vault: &VaultId) -> Result { + pub async fn get_vault_snapshot(&self, vault: &VaultId) -> Result { let mut tx = self.create_readonly_transaction(vault).await?; - let head_event_id = Self::latest_event_id(&mut tx).await?; - let file_manifest = Self::current_file_manifest(&mut tx).await?; + let head_event_id = Self::get_latest_event_id(&mut tx).await?; + let file_manifest = Self::get_current_file_manifest(&mut tx).await?; let documents = sqlx::query_as::<_, DocumentVersionWithoutContent>( "SELECT d.vault_update_id, d.document_id, d.updated_date, d.user_id, d.device_id, @@ -189,14 +191,14 @@ impl Database { }) } - pub async fn events_after( + pub async fn get_events_after( &self, vault: &VaultId, after: VaultUpdateId, page_size: usize, ) -> Result { let mut tx = self.create_readonly_transaction(vault).await?; - let head_event_id = Self::latest_event_id(&mut tx).await?; + let head_event_id = Self::get_latest_event_id(&mut tx).await?; ensure!( (0..=head_event_id).contains(&after), @@ -218,7 +220,7 @@ impl Database { let event = EventRecord { event_id, request_id: request_id.parse()?, - event: Self::event_by_id(&mut tx, event_id).await?, + event: Self::get_event_by_id(&mut tx, event_id).await?, }; let size = serde_json::to_vec(&event)?.len(); // One manifest is atomic and may exceed this budget. Always allow @@ -238,45 +240,3 @@ impl Database { }) } } - -impl Database { - pub async fn history_checkpoint(&self, vault: &VaultId) -> Result { - let mut tx = self.create_readonly_transaction(vault).await?; - - let row: Option<(i64, String)> = sqlx::query_as( - "SELECT event_id, event_token FROM events ORDER BY event_id DESC LIMIT 1", - ) - .fetch_optional(&mut *tx) - .await?; - - Ok(row.map_or_else( - || EMPTY_HISTORY_CHECKPOINT.to_owned(), - |(id, token)| format!("{id}:{token}"), - )) - } - - pub async fn contains_checkpoint(&self, vault: &VaultId, checkpoint: &str) -> Result { - if checkpoint == EMPTY_HISTORY_CHECKPOINT { - return Ok(true); - } - - let Some((id, token)) = checkpoint.split_once(':') else { - return Ok(false); - }; - - let Ok(id) = id.parse::() else { - return Ok(false); - }; - - let mut tx = self.create_readonly_transaction(vault).await?; - let exists: bool = sqlx::query_scalar( - "SELECT EXISTS(SELECT 1 FROM events WHERE event_id = ? AND event_token = ?)", - ) - .bind(id) - .bind(token) - .fetch_one(&mut *tx) - .await?; - - Ok(exists) - } -} diff --git a/sync-server/src/app_state/database/tests.rs b/sync-server/src/app_state/database/tests.rs index 9fdcf753..810d484a 100644 --- a/sync-server/src/app_state/database/tests.rs +++ b/sync-server/src/app_state/database/tests.rs @@ -87,7 +87,7 @@ async fn rejects_invalid_vault_names_at_the_database_boundary() { "a\0b", ] { assert!( - database.vault_snapshot(&name.to_owned()).await.is_err(), + database.get_vault_snapshot(&name.to_owned()).await.is_err(), "accepted {name:?}" ); } @@ -117,7 +117,7 @@ async fn schema_initialization_preserves_existing_requests_and_content() { ); assert_eq!( reopened - .events_after(&vault, 0, DEFAULT_EVENTS_PAGE_SIZE) + .get_events_after(&vault, 0, DEFAULT_EVENTS_PAGE_SIZE) .await .unwrap() .events @@ -155,7 +155,7 @@ async fn snapshot_returns_latest_referenced_metadata_and_byte_lengths() { .unwrap(); tx.commit().await.unwrap(); - let snapshot = database.vault_snapshot(&vault).await.unwrap(); + let snapshot = database.get_vault_snapshot(&vault).await.unwrap(); assert_eq!(snapshot.head_event_id, manifest.file_manifest_id); assert_eq!(snapshot.file_manifest.entries, manifest.entries); assert_eq!(snapshot.documents.len(), 2); @@ -179,7 +179,7 @@ async fn replay_pages_are_bounded_and_cover_every_event_in_order() { let mut after = 0; while after < 150 { let batch = database - .events_after(&vault, after, DEFAULT_EVENTS_PAGE_SIZE) + .get_events_after(&vault, after, DEFAULT_EVENTS_PAGE_SIZE) .await .unwrap(); assert!(batch.events.len() <= 64, "catchup must bound each page"); @@ -192,7 +192,7 @@ async fn replay_pages_are_bounded_and_cover_every_event_in_order() { } assert!( database - .events_after(&vault, after, DEFAULT_EVENTS_PAGE_SIZE) + .get_events_after(&vault, after, DEFAULT_EVENTS_PAGE_SIZE) .await .unwrap() .events @@ -204,7 +204,10 @@ async fn replay_pages_are_bounded_and_cover_every_event_in_order() { async fn idle_cleanup_does_not_wait_for_an_active_vault() { let directory = tempfile::tempdir().unwrap(); let database = Database::try_new(&config(&directory)).await.unwrap(); - database.vault_snapshot(&"other".to_owned()).await.unwrap(); + database + .get_vault_snapshot(&"other".to_owned()) + .await + .unwrap(); let vault = "slow".to_owned(); let transaction = database.create_write_transaction(&vault).await.unwrap(); database @@ -219,7 +222,7 @@ async fn idle_cleanup_does_not_wait_for_an_active_vault() { .expect("cleanup must leave an active pool alone"); let other = tokio::time::timeout( Duration::from_millis(200), - database.vault_snapshot(&"other".to_owned()), + database.get_vault_snapshot(&"other".to_owned()), ) .await .expect("other vaults must remain accessible") @@ -299,16 +302,13 @@ async fn replay_byte_budget_prevents_accumulating_large_historical_manifests() { } tx.commit().await.unwrap(); let batch = database - .events_after(&vault, 0, DEFAULT_EVENTS_PAGE_SIZE) + .get_events_after(&vault, 0, DEFAULT_EVENTS_PAGE_SIZE) .await .unwrap(); assert!(!batch.events.is_empty()); assert!(batch.events.len() < 8); assert!(serde_json::to_vec(&batch).unwrap().len() < MAX_EVENTS_PAGE_BYTES + 1024); - assert_eq!( - batch.end_event_id, - batch.events.last().unwrap().event_id - ); + assert_eq!(batch.end_event_id, batch.events.last().unwrap().event_id); } #[tokio::test] @@ -316,11 +316,14 @@ async fn opening_a_locked_vault_does_not_block_unrelated_vaults() { let directory = tempfile::tempdir().unwrap(); let config = config(&directory); let database = Database::try_new(&config).await.unwrap(); - database.vault_snapshot(&"other".to_owned()).await.unwrap(); + database + .get_vault_snapshot(&"other".to_owned()) + .await + .unwrap(); let vault = "locked-opening".to_owned(); // A second process holds the writer lock while this process initializes // the schema. Even already-open, unrelated vaults must remain available. - let external = Database::open_database(&config, &Database::database_path(&config, &vault)) + let external = Database::open_database(&config, &Database::get_database_path(&config, &vault)) .await .unwrap(); let transaction = external.begin_with("BEGIN IMMEDIATE").await.unwrap(); @@ -330,7 +333,7 @@ async fn opening_a_locked_vault_does_not_block_unrelated_vaults() { let opening_is_blocked = !opening.is_finished(); let other = tokio::time::timeout( Duration::from_millis(300), - database.vault_snapshot(&"other".to_owned()), + database.get_vault_snapshot(&"other".to_owned()), ) .await; transaction.rollback().await.unwrap(); @@ -360,7 +363,7 @@ async fn concurrent_vault_opens_share_a_pool_and_cancelled_opens_can_retry() { config.max_connections_per_vault = 1; let database = Database::try_new(&config).await.unwrap(); let vault = "cancelled-opening".to_owned(); - let external = Database::open_database(&config, &Database::database_path(&config, &vault)) + let external = Database::open_database(&config, &Database::get_database_path(&config, &vault)) .await .unwrap(); let transaction = external.begin_with("BEGIN IMMEDIATE").await.unwrap(); @@ -401,7 +404,7 @@ async fn concurrent_vault_opens_share_a_pool_and_cancelled_opens_can_retry() { async fn one_corrupt_vault_does_not_prevent_startup_or_healthy_vault_access() { let directory = tempfile::tempdir().unwrap(); let config = config(&directory); - let bad = Database::database_path(&config, "broken"); + let bad = Database::get_database_path(&config, "broken"); tokio::fs::create_dir_all(bad.parent().unwrap()) .await .unwrap(); @@ -409,7 +412,12 @@ async fn one_corrupt_vault_does_not_prevent_startup_or_healthy_vault_access() { .await .unwrap(); let database = Database::try_new(&config).await.unwrap(); - assert!(database.vault_snapshot(&"broken".to_owned()).await.is_err()); + assert!( + database + .get_vault_snapshot(&"broken".to_owned()) + .await + .is_err() + ); let saved = insert_content(&database, "healthy", uuid::Uuid::new_v4(), b"safe").await; assert_eq!(saved.content, b"safe"); assert_eq!( diff --git a/sync-server/src/app_state/websocket/broadcasts.rs b/sync-server/src/app_state/websocket/broadcasts.rs index f97370f3..6617ff2e 100644 --- a/sync-server/src/app_state/websocket/broadcasts.rs +++ b/sync-server/src/app_state/websocket/broadcasts.rs @@ -39,7 +39,7 @@ impl Broadcasts { limit.try_acquire_owned().ok() } - async fn sender(&self, vault: VaultId) -> broadcast::Sender { + async fn get_or_create_sender(&self, vault: VaultId) -> broadcast::Sender { self.senders .lock() .await @@ -49,12 +49,12 @@ impl Broadcasts { } pub async fn get_receiver(&self, vault: VaultId) -> broadcast::Receiver { - self.sender(vault).await.subscribe() + self.get_or_create_sender(vault).await.subscribe() } pub async fn send(&self, vault: VaultId, notification: Notification) { // No listeners (or lagging listeners) is normal; they replay the log. - let _ = self.sender(vault).await.send(notification); + let _ = self.get_or_create_sender(vault).await.send(notification); } pub async fn notify_about_vault_update(&self, vault: VaultId) { diff --git a/sync-server/src/app_state/websocket/utils.rs b/sync-server/src/app_state/websocket/utils.rs index adfab14a..2ea57000 100644 --- a/sync-server/src/app_state/websocket/utils.rs +++ b/sync-server/src/app_state/websocket/utils.rs @@ -8,7 +8,7 @@ use crate::{ app_state::{AppState, database::models::VaultId}, config::user_config::User, errors::{SyncServerError, server_error, unauthenticated_error}, - server::auth::auth, + server::auth::authenticate_user, }; pub struct AuthenticatedWebSocketHandshake { @@ -28,7 +28,7 @@ pub fn get_authenticated_handshake( match message { WebSocketClientMessage::Handshake(handshake) => { - let user = auth(state, handshake.token.trim(), vault_id)?; + let user = authenticate_user(state, handshake.token.trim(), vault_id)?; Ok(AuthenticatedWebSocketHandshake { handshake, user }) } WebSocketClientMessage::CursorPositions(_) => Err(unauthenticated_error( diff --git a/sync-server/src/errors.rs b/sync-server/src/errors.rs index c505b8ae..e49defab 100644 --- a/sync-server/src/errors.rs +++ b/sync-server/src/errors.rs @@ -31,6 +31,26 @@ pub enum SyncServerError { PermissionDeniedError(#[source] anyhow::Error), } +// Infrastructure failures are server errors. Request validation and missing +// resources opt into their HTTP status explicitly at the point of validation. +impl From for SyncServerError { + fn from(error: anyhow::Error) -> Self { + server_error(error) + } +} + +impl From for SyncServerError { + fn from(error: sqlx::Error) -> Self { + server_error(error.into()) + } +} + +impl From for SyncServerError { + fn from(error: tokio::task::JoinError) -> Self { + server_error(error.into()) + } +} + impl SyncServerError { pub fn serialize(&self) -> SerializedError { match self { diff --git a/sync-server/src/server.rs b/sync-server/src/server.rs index 0a6145c6..0cfa87ef 100644 --- a/sync-server/src/server.rs +++ b/sync-server/src/server.rs @@ -46,7 +46,7 @@ pub async fn create_server(config: Config) -> Result<()> { let server_config = app_state.config.server.clone(); let app = Router::new() - .nest("/", get_authed_routes(app_state.clone())) + .nest("/", create_authenticated_routes(app_state.clone())) .route("/", get(endpoints::index::index)) .route("/vaults/:vault_id/ping", get(endpoints::ping::ping)) .route( @@ -100,7 +100,7 @@ pub async fn create_server(config: Config) -> Result<()> { start_server(app, &server_config).await } -fn get_authed_routes(app_state: AppState) -> Router { +fn create_authenticated_routes(app_state: AppState) -> Router { Router::new() .route( "/vaults/:vault_id/file-manifest", diff --git a/sync-server/src/server/auth.rs b/sync-server/src/server/auth.rs index c601ef54..74c424e9 100644 --- a/sync-server/src/server/auth.rs +++ b/sync-server/src/server/auth.rs @@ -32,14 +32,18 @@ pub async fn auth_middleware( .ok_or_else(|| unauthenticated_error(anyhow::anyhow!("Missing vault_id")))?, ); - let user = auth(&state, token, &vault_id)?; + let user = authenticate_user(&state, token, &vault_id)?; req.extensions_mut().insert(user); Ok(next.run(req).await) } -pub fn auth(state: &AppState, token: &str, vault_id: &VaultId) -> Result { +pub fn authenticate_user( + state: &AppState, + token: &str, + vault_id: &VaultId, +) -> Result { validate_vault_id(vault_id).map_err(client_error)?; let user = state .config diff --git a/sync-server/src/server/endpoints/events.rs b/sync-server/src/server/endpoints/events.rs index 3c2ffd30..8ce27115 100644 --- a/sync-server/src/server/endpoints/events.rs +++ b/sync-server/src/server/endpoints/events.rs @@ -35,7 +35,7 @@ pub async fn events( state .database - .events_after( + .get_events_after( &vault_id, query.after, query.page_size.unwrap_or(DEFAULT_EVENTS_PAGE_SIZE), diff --git a/sync-server/src/server/endpoints/fetch_document_version_content.rs b/sync-server/src/server/endpoints/fetch_document_version_content.rs index 9e981c5f..08599bfb 100644 --- a/sync-server/src/server/endpoints/fetch_document_version_content.rs +++ b/sync-server/src/server/endpoints/fetch_document_version_content.rs @@ -11,7 +11,7 @@ use crate::{ AppState, database::models::{DocumentId, VaultUpdateId}, }, - errors::{SyncServerError, not_found_error, server_error}, + errors::{SyncServerError, not_found_error}, }; #[axum::debug_handler] @@ -30,16 +30,12 @@ pub async fn fetch_document_version_content( let result = state .database .get_document_version(&vault_id, vault_update_id, None) - .await - .map_err(server_error)? - .map_or_else( - || { - Err(not_found_error(anyhow!( - "Document with vault update id `{vault_update_id}` not found", - ))) - }, - Ok, - )?; + .await? + .ok_or_else(|| { + not_found_error(anyhow!( + "Document with vault update id `{vault_update_id}` not found", + )) + })?; if result.document_id != document_id { return Err(not_found_error(anyhow!( diff --git a/sync-server/src/server/endpoints/fetch_latest_document_metadata.rs b/sync-server/src/server/endpoints/fetch_latest_document_metadata.rs index 75b1a1e5..1c029847 100644 --- a/sync-server/src/server/endpoints/fetch_latest_document_metadata.rs +++ b/sync-server/src/server/endpoints/fetch_latest_document_metadata.rs @@ -14,7 +14,7 @@ use crate::{ models::{DocumentId, DocumentVersionWithoutContent}, }, }, - errors::{SyncServerError, not_found_error, server_error}, + errors::{SyncServerError, not_found_error}, }; #[axum::debug_handler] @@ -27,12 +27,10 @@ pub async fn fetch_latest_document_metadata( let mut tx = state .database .create_readonly_transaction(&vault_id) - .await - .map_err(server_error)?; + .await?; let metadata = Database::get_latest_document_metadata(&mut tx, &document_id) - .await - .map_err(server_error)? + .await? .ok_or_else(|| not_found_error(anyhow!("Document does not exist")))?; Ok(Json(metadata)) diff --git a/sync-server/src/server/endpoints/fetch_latest_document_version.rs b/sync-server/src/server/endpoints/fetch_latest_document_version.rs index 870ac669..25f80e81 100644 --- a/sync-server/src/server/endpoints/fetch_latest_document_version.rs +++ b/sync-server/src/server/endpoints/fetch_latest_document_version.rs @@ -11,7 +11,7 @@ use crate::{ AppState, database::models::{DocumentId, DocumentVersion}, }, - errors::{SyncServerError, not_found_error, server_error}, + errors::{SyncServerError, not_found_error}, }; #[axum::debug_handler] @@ -24,16 +24,8 @@ pub async fn fetch_latest_document_version( let latest_version = state .database .get_latest_document_version(&vault_id, &document_id, None) - .await - .map_err(server_error)? - .map_or_else( - || { - Err(not_found_error(anyhow!( - "Document with id `{document_id}` not found", - ))) - }, - Ok, - )?; + .await? + .ok_or_else(|| not_found_error(anyhow!("Document with id `{document_id}` not found")))?; Ok(Json(latest_version.into())) } diff --git a/sync-server/src/server/endpoints/get_file_manifest.rs b/sync-server/src/server/endpoints/get_file_manifest.rs index 3f706696..d54ed06a 100644 --- a/sync-server/src/server/endpoints/get_file_manifest.rs +++ b/sync-server/src/server/endpoints/get_file_manifest.rs @@ -23,10 +23,9 @@ pub async fn get_file_manifest( let mut transaction = state .database .create_readonly_transaction(&vault_id) - .await - .map_err(server_error)?; + .await?; - Database::current_file_manifest(&mut transaction) + Database::get_current_file_manifest(&mut transaction) .await .map(Json) .map_err(server_error) diff --git a/sync-server/src/server/endpoints/ping.rs b/sync-server/src/server/endpoints/ping.rs index 412ed0d2..a49c25a0 100644 --- a/sync-server/src/server/endpoints/ping.rs +++ b/sync-server/src/server/endpoints/ping.rs @@ -9,7 +9,7 @@ use axum_extra::{ use log::debug; use super::VaultPath; -use crate::server::{auth::auth, responses::PingResponse}; +use crate::server::{auth::authenticate_user, responses::PingResponse}; use crate::{app_state::AppState, consts::SUPPORTED_API_VERSION, errors::SyncServerError}; #[axum::debug_handler] @@ -20,8 +20,9 @@ pub async fn ping( ) -> Result, SyncServerError> { debug!("Pinging vault `{vault_id}`"); - let is_authenticated = maybe_auth_header - .is_some_and(|auth_header| auth(&state, auth_header.token(), &vault_id).is_ok()); + let is_authenticated = maybe_auth_header.is_some_and(|auth_header| { + authenticate_user(&state, auth_header.token(), &vault_id).is_ok() + }); Ok(Json(PingResponse { server_version: env!("CARGO_PKG_VERSION").to_owned(), diff --git a/sync-server/src/server/endpoints/put_file_content.rs b/sync-server/src/server/endpoints/put_file_content.rs index 45656a92..feed8277 100644 --- a/sync-server/src/server/endpoints/put_file_content.rs +++ b/sync-server/src/server/endpoints/put_file_content.rs @@ -1,4 +1,7 @@ -use super::{VaultPath, utils::find_already_processed_event}; +use super::{ + VaultPath, + utils::{commit_and_notify, find_already_processed_event, fingerprint_request}, +}; use crate::{ app_state::{ AppState, @@ -24,7 +27,6 @@ use axum_extra::TypedHeader; use base64::{Engine as _, engine::general_purpose::STANDARD}; use log::debug; use reconcile_text::{BuiltinTokenizer, EditedText, NumberOrText}; -use sha2::{Digest, Sha256}; #[axum::debug_handler] pub async fn put_file_content( @@ -36,16 +38,9 @@ pub async fn put_file_content( ) -> Result, SyncServerError> { debug!("Pushing document `{document_id}` in vault `{vault_id}`"); - let fingerprint = Sha256::digest( - serde_json::to_vec(&("content", document_id, &push)) - .map_err(|error| server_error(error.into()))?, - ); + let fingerprint = fingerprint_request(&("content", document_id, &push))?; - let mut tx = state - .database - .create_write_transaction(&vault_id) - .await - .map_err(server_error)?; + let mut tx = state.database.create_write_transaction(&vault_id).await?; if let Some(event) = find_already_processed_event(&mut tx, push.request_id, &fingerprint).await? @@ -63,8 +58,7 @@ pub async fn put_file_content( let parent = state .database .get_latest_document_version(&vault_id, &document_id, Some(&mut tx)) - .await - .map_err(server_error)?; + .await?; if let Some(parent) = &parent { if Some(parent.vault_update_id) != push.parent_version_id { @@ -82,16 +76,13 @@ pub async fn put_file_content( let content = decode_content(parent, push.content); (tx, content) }) - .await - .map_err(|error| server_error(error.into()))?; + .await?; let mut tx = tx; let content = content.map_err(client_error)?; let version = StoredDocumentVersion { - vault_update_id: Database::allocate_event(&mut tx, push.request_id, &fingerprint) - .await - .map_err(server_error)?, + vault_update_id: Database::allocate_event(&mut tx, push.request_id, &fingerprint).await?, document_id, content, updated_date: chrono::Utc::now(), @@ -99,17 +90,11 @@ pub async fn put_file_content( device_id: device.0, }; - Database::insert_document_version(&mut tx, &version) - .await - .map_err(server_error)?; + Database::insert_document_version(&mut tx, &version).await?; let response = DocumentUpdateResponse::Accepted((&version).into()); - tx.commit() - .await - .map_err(|error| server_error(error.into()))?; - - state.broadcasts.notify_about_vault_update(vault_id).await; + commit_and_notify(tx, &state, vault_id).await?; Ok(Json(response)) } @@ -388,7 +373,9 @@ mod tests { .await .unwrap(); assert_eq!( - Database::latest_event_id(&mut transaction).await.unwrap(), + Database::get_latest_event_id(&mut transaction) + .await + .unwrap(), 2 ); } @@ -450,7 +437,7 @@ mod tests { } let events = reopened .database - .events_after(&vault.vault, 0, DEFAULT_EVENTS_PAGE_SIZE) + .get_events_after(&vault.vault, 0, DEFAULT_EVENTS_PAGE_SIZE) .await .unwrap(); assert_eq!(events.head_event_id, latest.vault_update_id); @@ -477,7 +464,7 @@ mod tests { let events = vault .state .database - .events_after(&vault.vault, 0, DEFAULT_EVENTS_PAGE_SIZE) + .get_events_after(&vault.vault, 0, DEFAULT_EVENTS_PAGE_SIZE) .await .unwrap(); assert_eq!(events.head_event_id, 1); @@ -514,7 +501,7 @@ mod tests { let events = vault .state .database - .events_after(&vault.vault, 0, DEFAULT_EVENTS_PAGE_SIZE) + .get_events_after(&vault.vault, 0, DEFAULT_EVENTS_PAGE_SIZE) .await .unwrap(); assert_eq!(events.head_event_id, 0); @@ -538,7 +525,7 @@ mod tests { let events = vault .state .database - .events_after(&vault.vault, 0, DEFAULT_EVENTS_PAGE_SIZE) + .get_events_after(&vault.vault, 0, DEFAULT_EVENTS_PAGE_SIZE) .await .unwrap(); assert_eq!(events.events.len(), 1); diff --git a/sync-server/src/server/endpoints/put_file_manifest.rs b/sync-server/src/server/endpoints/put_file_manifest.rs index 9ba9156a..b08cf4eb 100644 --- a/sync-server/src/server/endpoints/put_file_manifest.rs +++ b/sync-server/src/server/endpoints/put_file_manifest.rs @@ -4,9 +4,11 @@ use axum::{ extract::{Path, State}, }; use log::debug; -use sha2::{Digest, Sha256}; -use super::{VaultPath, utils::find_already_processed_event}; +use super::{ + VaultPath, + utils::{commit_and_notify, find_already_processed_event, fingerprint_request}, +}; use crate::{ app_state::{ AppState, @@ -15,7 +17,7 @@ use crate::{ models::{FileManifest, VaultEvent}, }, }, - errors::{SyncServerError, client_error, server_error}, + errors::{SyncServerError, client_error}, server::{requests::PushFileManifest, responses::FileManifestUpdateResponse}, }; @@ -28,16 +30,9 @@ pub async fn put_file_manifest( debug!("Pushing file manifest for vault `{vault_id}`"); // BTreeMap canonicalizes file manifest entries for request fingerprinting. - let fingerprint = Sha256::digest( - serde_json::to_vec(&("file_manifest", &push)) - .map_err(|error| server_error(error.into()))?, - ); + let fingerprint = fingerprint_request(&("file_manifest", &push))?; - let mut tx = state - .database - .create_write_transaction(&vault_id) - .await - .map_err(server_error)?; + let mut tx = state.database.create_write_transaction(&vault_id).await?; if let Some(event) = find_already_processed_event(&mut tx, push.request_id, &fingerprint).await? @@ -54,9 +49,7 @@ pub async fn put_file_manifest( }; } - let latest = Database::current_file_manifest(&mut tx) - .await - .map_err(server_error)?; + let latest = Database::get_current_file_manifest(&mut tx).await?; if latest.file_manifest_id != push.parent_file_manifest_id { return Ok(Json(FileManifestUpdateResponse::StaleBase(latest))); @@ -64,8 +57,7 @@ pub async fn put_file_manifest( if let Some(id) = Database::get_missing_document(&mut tx, &push.entries.keys().copied().collect::>()) - .await - .map_err(server_error)? + .await? { return Err(client_error(anyhow!( "File manifest references missing content: {id}" @@ -73,25 +65,17 @@ pub async fn put_file_manifest( } let file_manifest = FileManifest { - file_manifest_id: Database::allocate_event(&mut tx, push.request_id, &fingerprint) - .await - .map_err(server_error)?, + file_manifest_id: Database::allocate_event(&mut tx, push.request_id, &fingerprint).await?, entries: push.entries, }; - Database::insert_file_manifest(&mut tx, &file_manifest) - .await - .map_err(server_error)?; + Database::insert_file_manifest(&mut tx, &file_manifest).await?; let response = FileManifestUpdateResponse::Accepted { file_manifest_id: file_manifest.file_manifest_id, }; - tx.commit() - .await - .map_err(|error| server_error(error.into()))?; - - state.broadcasts.notify_about_vault_update(vault_id).await; + commit_and_notify(tx, &state, vault_id).await?; Ok(Json(response)) } diff --git a/sync-server/src/server/endpoints/utils.rs b/sync-server/src/server/endpoints/utils.rs index c0df4216..5d10e9f1 100644 --- a/sync-server/src/server/endpoints/utils.rs +++ b/sync-server/src/server/endpoints/utils.rs @@ -1,7 +1,13 @@ use anyhow::anyhow; use crate::{ - app_state::database::{Database, Transaction, models::VaultEvent}, + app_state::{ + AppState, + database::{ + Database, Transaction, + models::{VaultEvent, VaultId}, + }, + }, errors::{SyncServerError, client_error, server_error}, }; @@ -10,9 +16,7 @@ pub(super) async fn find_already_processed_event( request_id: uuid::Uuid, fingerprint: &[u8], ) -> Result, SyncServerError> { - let Some((stored_fingerprint, event)) = Database::get_request_event(tx, request_id) - .await - .map_err(server_error)? + let Some((stored_fingerprint, event)) = Database::get_request_event(tx, request_id).await? else { return Ok(None); }; @@ -25,3 +29,24 @@ pub(super) async fn find_already_processed_event( Ok(Some(event)) } + +/// Hash the canonical wire payload, including the operation and document ID. +pub(super) fn fingerprint_request( + payload: &impl serde::Serialize, +) -> Result<[u8; 32], SyncServerError> { + use sha2::{Digest, Sha256}; + + let encoded = serde_json::to_vec(payload).map_err(|error| server_error(error.into()))?; + Ok(Sha256::digest(encoded).into()) +} + +/// Never advertise a write before its event and data are durably committed. +pub(super) async fn commit_and_notify( + tx: Transaction<'_>, + state: &AppState, + vault: VaultId, +) -> Result<(), SyncServerError> { + tx.commit().await?; + state.broadcasts.notify_about_vault_update(vault).await; + Ok(()) +} diff --git a/sync-server/src/server/endpoints/vault_snapshot.rs b/sync-server/src/server/endpoints/vault_snapshot.rs index 93e5e4d1..b4071d6b 100644 --- a/sync-server/src/server/endpoints/vault_snapshot.rs +++ b/sync-server/src/server/endpoints/vault_snapshot.rs @@ -19,7 +19,7 @@ pub async fn vault_snapshot( state .database - .vault_snapshot(&vault_id) + .get_vault_snapshot(&vault_id) .await .map(Json) .map_err(server_error) diff --git a/sync-server/src/server/endpoints/websocket.rs b/sync-server/src/server/endpoints/websocket.rs index 0ef24919..326f2e1b 100644 --- a/sync-server/src/server/endpoints/websocket.rs +++ b/sync-server/src/server/endpoints/websocket.rs @@ -1,7 +1,7 @@ use crate::{ app_state::{ AppState, - database::models::VaultId, + database::models::{DeviceId, VaultId}, websocket::{ broadcasts::Notification, models::{CursorPositionFromServer, WebSocketClientMessage, WebSocketServerMessage}, @@ -9,7 +9,7 @@ use crate::{ }, }, consts::WEBSOCKET_HANDSHAKE_TIMEOUT, - errors::{SyncServerError, client_error, server_error}, + errors::{SyncServerError, client_error}, }; use axum::{ extract::{ @@ -19,7 +19,10 @@ use axum::{ http::StatusCode, response::{IntoResponse, Response}, }; -use futures::StreamExt; +use futures::{ + StreamExt, + stream::{SplitSink, SplitStream}, +}; use log::{debug, info}; use std::time::Duration; use tokio::sync::broadcast::error::RecvError; @@ -44,13 +47,13 @@ pub async fn websocket_handler( Ok(ws.on_upgrade(move |socket| async move { let _permit = permit; - if let Err(error) = websocket(state, socket, vault_id).await { + if let Err(error) = serve_connection(state, socket, vault_id).await { debug!("WebSocket disconnected: {error}"); } })) } -async fn websocket( +async fn serve_connection( state: AppState, socket: WebSocket, vault: VaultId, @@ -68,53 +71,16 @@ async fn websocket( let device = authenticated.handshake.device_id; let session = state.cursors.register_connection(&vault, &device).await; - let mut last_checkpoint = None; - let mut notifications = state.broadcasts.get_receiver(vault.clone()).await; - let mut timer = tokio::time::interval(Duration::from_secs(2)); - timer.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); - info!("WebSocket connected to vault {vault}"); - let result = async { - loop { - let checkpoint = state.database.history_checkpoint(&vault).await.map_err(server_error)?; - if last_checkpoint.as_ref() != Some(&checkpoint) { - send_update_over_websocket(&WebSocketServerMessage::VaultChanged, &mut sender).await?; - last_checkpoint = Some(checkpoint); - } - - tokio::select! { - _ = timer.tick() => {}, - notification = notifications.recv() => match notification { - Ok(update) => if let Notification::Cursors(mut positions) = update { - positions.clients.retain(|c| c.device_id != device); - send_update_over_websocket(&WebSocketServerMessage::CursorPositions(positions), &mut sender).await?; - }, - - Err(RecvError::Lagged(_)) => {}, // Check the durable head on the next iteration. - Err(RecvError::Closed) => break, - }, - - incoming = receiver.next() => match incoming { - Some(Ok(Message::Text(text))) => { - let message: WebSocketClientMessage = serde_json::from_str(&text).map_err(|error| client_error(error.into()))?; - match message { - WebSocketClientMessage::Handshake(_) => return Err(client_error(anyhow::anyhow!("Unexpected handshake"))), - WebSocketClientMessage::CursorPositions(positions) => { - state.cursors.update_cursors(vault.clone(), authenticated.user.name.clone(), &device, session, positions.documents_with_cursors).await; - let clients = state.cursors.get_cursors(&vault).await.into_iter().filter(|c| c.device_id != device).collect(); - send_update_over_websocket(&WebSocketServerMessage::CursorPositions(CursorPositionFromServer { clients }), &mut sender).await?; - } - } - }, - Some(Ok(Message::Ping(_) | Message::Pong(_))) => {}, - _ => break, - } - } - } - - Ok(()) - }.await; + let connection = AuthenticatedConnection { + state: &state, + vault: &vault, + device: &device, + user_name: &authenticated.user.name, + session, + }; + let result = connection.forward_updates(&mut sender, &mut receiver).await; state .cursors @@ -123,3 +89,101 @@ async fn websocket( result } + +/// A session owns cursor presence until it disconnects or a newer session replaces it. +struct AuthenticatedConnection<'a> { + state: &'a AppState, + vault: &'a VaultId, + device: &'a DeviceId, + user_name: &'a str, + session: uuid::Uuid, +} + +impl AuthenticatedConnection<'_> { + async fn forward_updates( + &self, + sender: &mut SplitSink, + receiver: &mut SplitStream, + ) -> Result<(), SyncServerError> { + let mut last_checkpoint = None; + let mut notifications = self.state.broadcasts.get_receiver(self.vault.clone()).await; + let mut timer = tokio::time::interval(Duration::from_secs(2)); + timer.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); + + loop { + let checkpoint = self + .state + .database + .get_history_checkpoint(self.vault) + .await?; + if last_checkpoint.as_ref() != Some(&checkpoint) { + send_update_over_websocket(&WebSocketServerMessage::VaultChanged, sender).await?; + last_checkpoint = Some(checkpoint); + } + + tokio::select! { + _ = timer.tick() => {}, + notification = notifications.recv() => { + match notification { + Ok(Notification::Cursors(positions)) => { + self.send_other_clients_cursors(positions, sender).await?; + } + // Recheck the durable head after either a notification or lag. + Ok(Notification::VaultUpdate) | Err(RecvError::Lagged(_)) => {}, + Err(RecvError::Closed) => break, + } + } + incoming = receiver.next() => { + match incoming { + Some(Ok(Message::Text(text))) => { + self.handle_client_message(&text, sender).await?; + } + Some(Ok(Message::Ping(_) | Message::Pong(_))) => {}, + _ => break, + } + } + } + } + + Ok(()) + } + + async fn send_other_clients_cursors( + &self, + mut positions: CursorPositionFromServer, + sender: &mut SplitSink, + ) -> Result<(), SyncServerError> { + positions + .clients + .retain(|client| client.device_id != *self.device); + send_update_over_websocket(&WebSocketServerMessage::CursorPositions(positions), sender) + .await + } + + async fn handle_client_message( + &self, + text: &str, + sender: &mut SplitSink, + ) -> Result<(), SyncServerError> { + let message: WebSocketClientMessage = + serde_json::from_str(text).map_err(|error| client_error(error.into()))?; + let WebSocketClientMessage::CursorPositions(positions) = message else { + return Err(client_error(anyhow::anyhow!("Unexpected handshake"))); + }; + + self.state + .cursors + .update_cursors( + self.vault.clone(), + self.user_name.to_owned(), + self.device, + self.session, + positions.documents_with_cursors, + ) + .await; + + let clients = self.state.cursors.get_cursors(self.vault).await; + self.send_other_clients_cursors(CursorPositionFromServer { clients }, sender) + .await + } +} diff --git a/sync-server/src/server/history.rs b/sync-server/src/server/history.rs index 943a2d83..2f851c8f 100644 --- a/sync-server/src/server/history.rs +++ b/sync-server/src/server/history.rs @@ -50,7 +50,7 @@ pub async fn history_middleware( if response.status().is_success() { let checkpoint = state .database - .history_checkpoint(&vault_id) + .get_history_checkpoint(&vault_id) .await .map_err(server_error)?; response.headers_mut().insert( diff --git a/sync-server/src/server/tests.rs b/sync-server/src/server/tests.rs index 01d5f842..16d44623 100644 --- a/sync-server/src/server/tests.rs +++ b/sync-server/src/server/tests.rs @@ -17,7 +17,7 @@ async fn app(directory: &tempfile::TempDir) -> Router { })).unwrap(); config.database.databases_directory_path = directory.path().join("databases"); let state = AppState::try_new(config).await.unwrap(); - get_authed_routes(state.clone()).with_state(state) + create_authenticated_routes(state.clone()).with_state(state) } async fn request(app: Router, path: &str, token: &str, body: Option) -> (StatusCode, Value) { diff --git a/sync-server/src/utils/portable_path.rs b/sync-server/src/utils/portable_path.rs index 7c170848..63d15c09 100644 --- a/sync-server/src/utils/portable_path.rs +++ b/sync-server/src/utils/portable_path.rs @@ -7,7 +7,7 @@ pub const INTERNAL_DIRECTORY: &str = ".vault-link-sync"; // Use the same locale-independent Unicode upper-case mapping in the client. // NFC -> uppercase -> NFC also catches ß/SS and final/ordinary sigma aliases. -pub fn path_key(path: &str) -> String { +pub fn get_path_key(path: &str) -> String { path.nfc() .collect::() .to_uppercase() @@ -15,8 +15,19 @@ pub fn path_key(path: &str) -> String { .collect() } +#[derive(Clone, Copy, PartialEq, Eq)] +enum PathKind { + File, + Directory, +} + +struct PathNode { + spelling: String, + kind: PathKind, +} + pub fn validate_file_manifest(entries: &FileManifestEntries) -> Result<()> { - let mut nodes: BTreeMap = BTreeMap::new(); + let mut nodes: BTreeMap = BTreeMap::new(); for path in entries.values() { ensure!(!path.is_empty(), "Path must not be empty"); @@ -29,59 +40,35 @@ pub fn validate_file_manifest(entries: &FileManifestEntries) -> Result<()> { let parts: Vec<_> = path.split('/').collect(); ensure!( - path_key(parts[0]) != path_key(INTERNAL_DIRECTORY), + get_path_key(parts[0]) != get_path_key(INTERNAL_DIRECTORY), "Reserved sync directory" ); for (index, part) in parts.iter().enumerate() { - ensure!( - !part.is_empty() && *part != "." && *part != "..", - "Invalid path component" - ); - - ensure!(!part.ends_with(['.', ' ']), "Trailing dot or space in path"); - - ensure!( - !part - .chars() - .any(|c| c.is_control() || c == '\\' || "<>:\"|?*".contains(c)), - "Non-portable filename" - ); - - let stem = part - .split('.') - .next() - .unwrap_or_default() - .trim_end_matches(' ') - .to_uppercase(); - - ensure!( - !matches!( - stem.as_str(), - "CON" | "PRN" | "AUX" | "NUL" | "CONIN$" | "CONOUT$" - ), - "Reserved filename" - ); - - if stem.starts_with("COM") || stem.starts_with("LPT") { - ensure!( - !matches!( - &stem[3..], - "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9" | "¹" | "²" | "³" - ), - "Reserved filename" - ); - } + validate_component(part)?; let prefix = parts[..=index].join("/"); - let is_file = index == parts.len() - 1; + let kind = if index == parts.len() - 1 { + PathKind::File + } else { + PathKind::Directory + }; - if let Some((spelling, previous_is_file)) = nodes.get(&path_key(&prefix)) { - if *spelling != prefix || *previous_is_file || is_file { + if let Some(previous) = nodes.get(&get_path_key(&prefix)) { + if previous.spelling != prefix + || previous.kind == PathKind::File + || kind == PathKind::File + { bail!("Conflicting path: {path}"); } } else { - nodes.insert(path_key(&prefix), (prefix, is_file)); + nodes.insert( + get_path_key(&prefix), + PathNode { + spelling: prefix, + kind, + }, + ); } } } @@ -89,9 +76,53 @@ pub fn validate_file_manifest(entries: &FileManifestEntries) -> Result<()> { Ok(()) } +/// Validate one filename independently of the manifest's path relationships. +fn validate_component(part: &str) -> Result<()> { + ensure!( + !part.is_empty() && part != "." && part != "..", + "Invalid path component" + ); + + ensure!(!part.ends_with(['.', ' ']), "Trailing dot or space in path"); + + ensure!( + !part + .chars() + .any(|c| c.is_control() || c == '\\' || "<>:\"|?*".contains(c)), + "Non-portable filename" + ); + + let stem = part + .split('.') + .next() + .unwrap_or_default() + .trim_end_matches(' ') + .to_uppercase(); + + ensure!( + !matches!( + stem.as_str(), + "CON" | "PRN" | "AUX" | "NUL" | "CONIN$" | "CONOUT$" + ), + "Reserved filename" + ); + + if stem.starts_with("COM") || stem.starts_with("LPT") { + ensure!( + !matches!( + &stem[3..], + "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9" | "¹" | "²" | "³" + ), + "Reserved filename" + ); + } + + Ok(()) +} + #[cfg(test)] mod tests { - use super::{FileManifestEntries, path_key, validate_file_manifest}; + use super::{FileManifestEntries, get_path_key, validate_file_manifest}; use uuid::Uuid; fn manifest(paths: &[&str]) -> FileManifestEntries { @@ -110,7 +141,7 @@ mod tests { ("Straße.md", "STRASSE.MD"), ("σς.md", "ΣΣ.MD"), ] { - assert_eq!(path_key(path), expected, "path: {path:?}"); + assert_eq!(get_path_key(path), expected, "path: {path:?}"); } }