diff --git a/.forgejo/workflows/e2e.yml b/.forgejo/workflows/e2e.yml index ce30c205..74a17bf0 100644 --- a/.forgejo/workflows/e2e.yml +++ b/.forgejo/workflows/e2e.yml @@ -42,13 +42,6 @@ jobs: toolchain: "1.92.0" components: clippy, rustfmt - - name: Setup rust - run: | - which sqlx || cargo install sqlx-cli - cd sync-server - sqlx database create --database-url sqlite://db.sqlite3 - sqlx migrate run --source src/app_state/database/migrations --database-url sqlite://db.sqlite3 - - name: E2E tests run: scripts/e2e.sh diff --git a/README.md b/README.md index f13c18a4..6f4a521a 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ - Install [`rustup`](https://rustup.rs): `curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh` - Install [`wasm-pack`](https://rustwasm.github.io/wasm-pack/installer): `curl https://rustwasm.github.io/wasm-pack/installer/init.sh -sSf | sh` -- `cargo install cargo-insta sqlx-cli` +- `cargo install cargo-insta` ### Install Obsidian on Linux @@ -35,7 +35,6 @@ flatpak run md.obsidian.Obsidian Start the server: ```sh -cargo install sqlx-cli cd sync-server cargo run config-e2e.yml ``` diff --git a/docs/architecture/sync-algorithm.md b/docs/architecture/sync-algorithm.md index 2df745a8..dbd12011 100644 --- a/docs/architecture/sync-algorithm.md +++ b/docs/architecture/sync-algorithm.md @@ -1,6 +1,6 @@ # Sync algorithm -The current API v5 server stores immutable content versions and a separately +The current API v4 server stores immutable content versions and a separately versioned file manifest. Clients reconcile snapshots with the current local files. The server does not merge competing edits. ## Identity and ordering @@ -32,14 +32,17 @@ head is retained only until it can be applied. The observed hash records local presence as well as rename evidence; there are no separate bootstrap or materialization flags. Scans rebuild physical exclusions. Original server paths remain saved while excluded, so ignoring a file cannot publish a deletion or -forget a deferred rename. Rejected payloads and messages live on the retained -requests that event replay already needs for uncertain acknowledgements. +forget a deferred rename. A permanently rejected upload pauses the whole sync +loop. Its exact pending request and rejection message remain saved across restarts; +an explicit reset or settings change retries it before reading newer remote state. +There is no separate list of rejected requests or late-receipt matching. For text, the client compares the common base, current local bytes and current remote bytes using `reconcile-text`. Configured extensions and valid UTF-8 without NUL bytes determine whether text merging is allowed. Diffs optimize transport; they do not authorize the server to merge a stale base. Large diff operation sets -are sent as snapshots. +are sent as snapshots. Snapshots remain bytes plus hashes in memory; only outgoing +wire payloads are encoded as base64. If only one side changed, that side is retained. Concurrent unmergeable changes keep the server version. First contact with existing local text has no shared ancestor and uses @@ -98,10 +101,13 @@ WebSocket `vaultChanged` hints wake HTTP replay. The server periodically compare its history checkpoint to detect missed broadcasts; transport liveness checks recover disconnected clients. Authentication and permanent errors remain visible; transient errors retry according to client settings. Observer callback failures -are isolated from the background sync loop. +are isolated from the background sync loop. The WebSocket manager sends its own +handshake before announcing a connection. Shutdown retires its callbacks and +drains existing message handlers without waiting for a close handshake. -API v5 clients and servers must be deployed together. V4 databases migrate without -removing history. CLI and desktop Obsidian share the Node filesystem adapter and +API v4 clients and servers must be deployed together. Only the current client +state and server schema are supported; format changes require fresh state stores. +CLI and desktop Obsidian share the Node filesystem adapter and complete-record metadata persistence. Node file moves use exclusive creation followed by unlink, so interruption may leave two ordinary files. Metadata alone is flushed before an atomic rename; there is no user-file durability protocol. diff --git a/docs/guide/server-setup.md b/docs/guide/server-setup.md index 1848db26..7506e924 100644 --- a/docs/guide/server-setup.md +++ b/docs/guide/server-setup.md @@ -75,21 +75,13 @@ chmod +x sync_server-linux-x86_64 ### Build from Source -Requirements: Rust 1.92.0+, SQLite development headers, SQLx CLI +Requirements: Rust 1.92.0+, SQLite development headers ```bash # Clone the repository git clone https://github.com/schmelczer/vault-link.git cd vault-link/sync-server -# Install SQLx CLI -cargo install sqlx-cli - -# Set up the database -sqlx database create --database-url sqlite://db.sqlite3 -sqlx migrate run --source src/app_state/database/migrations --database-url sqlite://db.sqlite3 -cargo sqlx prepare --workspace - # Build in release mode cargo build --release diff --git a/frontend/local-client-cli/README.md b/frontend/local-client-cli/README.md index 9c519a68..f4cfb9a7 100644 --- a/frontend/local-client-cli/README.md +++ b/frontend/local-client-cli/README.md @@ -211,8 +211,9 @@ docker build -f local-client-cli/Dockerfile -t vault-link-cli:test . The complete client record (database, settings, queued logical notifications and server history checkpoint) is saved atomically in `.vaultlink/sync-data.json`. -Existing database-only files migrate on the next save. Invalid or unreadable -metadata is reported instead of silently starting with empty state. +Only this complete-record format is supported; database-only files are not +converted. Invalid or unreadable JSON is reported instead of silently starting +with empty state. File moves use an exclusive copy followed by deletion of the source. An interruption may leave both files; the next scan reconciles the visible files. diff --git a/frontend/local-client-cli/src/client-persistence.test.ts b/frontend/local-client-cli/src/client-persistence.test.ts index b400d50f..2b002b89 100644 --- a/frontend/local-client-cli/src/client-persistence.test.ts +++ b/frontend/local-client-cli/src/client-persistence.test.ts @@ -6,7 +6,7 @@ import * as path from "node:path"; import { ClientPersistence } from "./client-persistence"; import type { StoredClient } from "sync-client"; -test("CLI persistence retains the whole record and migrates database-only saves", async (t) => { +test("CLI persistence retains the whole record and reports unreadable metadata", async (t) => { const directory = await fs.mkdtemp( path.join(tmpdir(), "vaultlink-metadata-") ); @@ -18,7 +18,13 @@ test("CLI persistence retains the whole record and migrates database-only saves" }); const record: StoredClient = { settings: { token: "old-token", vaultName: "vault" }, - database: { lastSeenUpdateId: 12 }, + database: { + vaultKey: "vault", + fileManifest: { fileManifestId: 0, entries: {} }, + local: {}, + documents: {}, + lastSeenUpdateId: 12 + }, localChanges: [ { type: "delete", path: "removed.md", changeId: "queued" } ], @@ -30,12 +36,6 @@ test("CLI persistence retains the whole record and migrates database-only saves" ...record, settings: { ...record.settings, token: "current-token" } }); - const legacy = { - fileManifest: { fileManifestId: 0, entries: {} }, - lastSeenUpdateId: 0 - }; - await fs.writeFile(file, JSON.stringify(legacy)); - assert.deepEqual((await store.load()).database, legacy); await fs.writeFile(file, "{truncated"); await assert.rejects(store.load(), SyntaxError); await fs.rm(file); diff --git a/frontend/local-client-cli/src/client-persistence.ts b/frontend/local-client-cli/src/client-persistence.ts index ce421654..5147f415 100644 --- a/frontend/local-client-cli/src/client-persistence.ts +++ b/frontend/local-client-cli/src/client-persistence.ts @@ -4,7 +4,6 @@ import { randomUUID } from "node:crypto"; import type { PersistenceProvider, StoredClient, - StoredDatabase, SyncSettings } from "sync-client"; @@ -35,11 +34,7 @@ export class ClientPersistence implements PersistenceProvider { Array.isArray(parsed) ) throw new Error("Invalid client metadata"); - // The sync engine validates and migrates the persisted fields. - const data = parsed as StoredClient & Partial; - // Earlier CLI versions saved the database alone. - const stored: StoredClient = - "fileManifest" in data ? { database: data } : data; + const stored = parsed as StoredClient; return { ...stored, settings: { ...stored.settings, ...this.settings } diff --git a/frontend/sync-client/README.md b/frontend/sync-client/README.md index 40de1fdf..bfe89ada 100644 --- a/frontend/sync-client/README.md +++ b/frontend/sync-client/README.md @@ -1,4 +1,4 @@ -# Sync client (API v5) +# Sync client (API v4) The engine 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 @@ -17,6 +17,8 @@ pending requests saved and are reported to the caller. Transient failures retry; authentication and protocol errors wait for an explicit wake or settings change. Pausing or destroying the client aborts its HTTP session, including body reads; resuming starts a fresh session. Connection checks can run while sync is disabled. +WebSocket shutdown retires callbacks immediately and drains message handlers +already running; it does not wait for the transport's close handshake. ## State and policy @@ -38,10 +40,13 @@ and content heads before merging. An interrupted fetch restarts from the last incorporated cursor; pagination progress is not persisted. Intermediate changes that the server has already reverted must not overwrite local edits. -A permanent upload rejection is recorded in metadata. An unchanged rejected payload is skipped so other files can sync; the -failure remains visible in history and `waitUntilFinished()`. Editing it, resetting -the client, or changing settings permits a fresh attempt. Earlier attempts that -committed despite a lost reply are incorporated through normal event replay. +A permanent upload rejection pauses all sync, including after restart. The exact +pending request and rejection message stay in metadata, and the failure remains +visible in history and `waitUntilFinished()`. Resetting the client or changing +settings retries that request before reading newer remote changes. This recovers +earlier attempts that committed despite a lost reply without separate receipt +bookkeeping. Local edits made while paused are uploaded after the pending request +is resolved; editing a file alone does not retry a rejected request. For each document ID (absence means deletion), use the server path if local is unchanged, use local if the server is unchanged, and use the server when both @@ -57,6 +62,8 @@ are implicit; empty directories are not synced. - Content is uploaded before new membership; the client chooses and persists UUIDs. - Text uses three-way `reconcile`. First contact at a matching path adopts the remote ID and reconciles differing text with an empty parent. +- In-memory snapshots contain bytes and their hash. Base64 encoding is limited to + wire payloads; pending requests persist the exact payload needed for retries. - Concurrent binary/unmergeable edits keep the server version. If only one side changed, keep that side. No local backup archive is created. - Deletion follows the file manifest decision, including when the file has local edits. @@ -84,11 +91,13 @@ snapshot. A file edited after this reset is also preserved as a separate local document; recovery never persists another copy of its content. Unsent edits with an old base become separate local documents; path conflicts preserve them under conflict names instead of applying a diff to an unrelated restored version. -Recovery resumes after interrupted state saves. Existing API v4 client state -without a checkpoint takes this same conservative bootstrap on upgrade. +Recovery resumes after interrupted state saves. Only the current persisted state +format is supported; old client stores are not converted. Status observers cannot stop the background engine by throwing or rejecting. Their failures are reported, and remaining observers continue receiving updates. +Remote cursor tracking retains the latest future update and the last usable +position for each device, so cursors remain visible while file content catches up. ## Required adapter contract @@ -129,4 +138,4 @@ between files using their document identities; the next scan rereads content. Co replacement. Unexpected eligible occupants receive visible conflict names; ignored and oversized occupants stay in place. External writers can still change files between observations, so these checks do not promise transactional file -contents or perfect identity reconstruction. There is no automated v2/v3 state migration. +contents or perfect identity reconstruction. There is no persisted-state migration. diff --git a/frontend/sync-client/src/consts.ts b/frontend/sync-client/src/consts.ts index 8d266eb8..15cea310 100644 --- a/frontend/sync-client/src/consts.ts +++ b/frontend/sync-client/src/consts.ts @@ -2,6 +2,5 @@ export const TIMEOUT_FOR_MERGING_HISTORY_ENTRIES_IN_SECONDS = 60; export const MAX_LOG_MESSAGE_COUNT = 100000; export const MAX_HISTORY_ENTRY_COUNT = 5000; export const SUPPORTED_API_VERSION = 4; -export const WEBSOCKET_DISCONNECT_TIMEOUT_IN_S = 10; export const CURSOR_HEARTBEAT_INTERVAL_MS = 15_000; export const WEBSOCKET_RECEIVE_TIMEOUT_MS = CURSOR_HEARTBEAT_INTERVAL_MS * 3; diff --git a/frontend/sync-client/src/errors/errors.ts b/frontend/sync-client/src/errors/errors.ts index 6026d692..3844ce10 100644 --- a/frontend/sync-client/src/errors/errors.ts +++ b/frontend/sync-client/src/errors/errors.ts @@ -1,3 +1,5 @@ +import type { RelativePath } from "../persistence/database"; + export class SyncResetError extends Error { public constructor() { super("SyncClient has been reset, cleaning up"); @@ -22,7 +24,7 @@ export class AuthenticationError extends Error { export class FileNotFoundError extends Error { public constructor( message: string, - public readonly filePath: string + public readonly filePath: RelativePath ) { super(message); this.name = "FileNotFoundError"; diff --git a/frontend/sync-client/src/file-operations/file-operations.ts b/frontend/sync-client/src/file-operations/file-operations.ts index aec5791e..ca7ff8a1 100644 --- a/frontend/sync-client/src/file-operations/file-operations.ts +++ b/frontend/sync-client/src/file-operations/file-operations.ts @@ -1,11 +1,15 @@ import { reconcile } from "reconcile-text"; import type { FileSystemOperations } from "./filesystem-operations"; -import type { Database, EngineState } from "../persistence/database"; +import type { + Database, + DocumentId, + EngineState, + RelativePath +} from "../persistence/database"; import { - fromStoredSnapshot, - toStoredSnapshot, + hashSnapshot, type FileSnapshot, - type StoredSnapshot + type HashedSnapshot } from "../snapshot"; import { isInternalPath } from "../utils/portable-path"; import { FilePaths } from "./file-paths"; @@ -25,8 +29,8 @@ import { } from "../errors/errors"; export interface FileWrite { - expected?: StoredSnapshot; - replacement: StoredSnapshot; + expected?: HashedSnapshot; + replacement: HashedSnapshot; } type Mutation = (action: () => Promise) => Promise; @@ -42,7 +46,7 @@ export class FileOperations { private readonly serverConfig: ServerConfig, private readonly filesApplied: ( - paths: readonly string[] + paths: readonly RelativePath[] ) => void = () => undefined, private readonly localChanges: { @@ -51,14 +55,14 @@ export class FileOperations { } = { entries: () => [], flush: async () => undefined }, private readonly isProtectedFile: ( - path: string, + path: RelativePath, size: number ) => boolean = () => false ) { this.paths = new FilePaths(fs, isProtectedFile); } - public async read(path: string): Promise { + public async read(path: RelativePath): Promise { const snapshot = await this.fs.read(path); if (!snapshot) @@ -67,18 +71,19 @@ export class FileOperations { return snapshot.content; } - public async snapshot(path: string): Promise { + public async snapshot( + path: RelativePath + ): Promise { const snapshot = await this.fs.read(path); - return snapshot && toStoredSnapshot(snapshot); + return snapshot && hashSnapshot(snapshot); } - public async listFilesRecursively(): Promise { + public async listFilesRecursively(): Promise { return (await this.fs.listFilesRecursively()).filter( (path) => !isInternalPath(path) ); } - public async captureChange( change: LocalChange, previous: readonly LocalChange[] @@ -95,7 +100,7 @@ export class FileOperations { public async apply( next: EngineState, - writes: Partial> = {}, + writes: Partial> = {}, validatePlan: () => void = () => undefined ): Promise { await this.localChanges.flush(); @@ -116,7 +121,7 @@ export class FileOperations { const current = structuredClone(this.database.state); current.lastSeenUpdateId = next.lastSeenUpdateId; this.applying = current; - const paths = new Set(); + const paths = new Set(); const observeChanges = async (): Promise => { await this.localChanges.flush(); guard(); @@ -129,7 +134,8 @@ export class FileOperations { state, change, (path) => - isInternalPath(path) || this.isProtectedFile(path, 0) + isInternalPath(path) || + this.isProtectedFile(path, 0) ); state.lastAppliedLocalChangeId = change.changeId; } @@ -141,9 +147,10 @@ export class FileOperations { ...Object.keys(next.local) ])) { await observeChanges(); - let from = current.documents[id]?.observedHash !== undefined - ? current.local[id] - : undefined; + 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; @@ -182,9 +189,10 @@ export class FileOperations { mutate ); next.local[id] = to; - from = current.documents[id]?.observedHash !== undefined - ? current.local[id] - : undefined; + from = + current.documents[id]?.observedHash !== undefined + ? current.local[id] + : undefined; if (from !== undefined) { await mutate(async () => { current.local[id] = to; @@ -195,9 +203,7 @@ export class FileOperations { } if (write) { const snapshot = - from === undefined - ? undefined - : await this.fs.read(to); + from === undefined ? undefined : await this.fs.read(to); if (from !== undefined && !snapshot) { delete next.local[id]; continue; @@ -229,13 +235,12 @@ export class FileOperations { } private async updateContent( - path: string, + path: RelativePath, current: FileSnapshot | undefined, write: FileWrite, extensions: string[] ): Promise { - const replacement = fromStoredSnapshot(write.replacement); - const base = write.expected && fromStoredSnapshot(write.expected); + const { replacement, expected: base } = write; if ( current && base && @@ -258,14 +263,17 @@ export class FileOperations { const result = current && write.expected ? await mergeContent( - path, - write.expected, - await toStoredSnapshot(current), - write.replacement, - extensions - ) + path, + write.expected, + await hashSnapshot(current), + write.replacement, + extensions + ) : write.replacement; - await this.fs.write(path, fromStoredSnapshot(result)); + await this.fs.write(path, { + content: result.content, + cursors: result.cursors + }); return result.hash; } } diff --git a/frontend/sync-client/src/file-operations/file-paths.ts b/frontend/sync-client/src/file-operations/file-paths.ts index 09474403..e1ce6bfa 100644 --- a/frontend/sync-client/src/file-operations/file-paths.ts +++ b/frontend/sync-client/src/file-operations/file-paths.ts @@ -1,22 +1,22 @@ import { v4 as uuid } from "uuid"; import type { FileSystemOperations } from "./filesystem-operations"; -import type { EngineState } from "../persistence/database"; +import type { DocumentId, EngineState, RelativePath } from "../persistence/database"; import { allocatePortablePath, isInternalPath } from "../utils/portable-path"; /** Resolve occupied rename destinations without discarding another file. */ export class FilePaths { public constructor( private readonly fs: FileSystemOperations, - private readonly isProtectedFile: (path: string, size: number) => boolean + private readonly isProtectedFile: (path: RelativePath, size: number) => boolean ) { } public async prepareDestination( next: EngineState, current: EngineState, - id: string, - path: string, + id: DocumentId, + path: RelativePath, mutate: (action: () => Promise) => Promise - ): Promise { + ): Promise { try { for (; ;) { const obstruction = await this.obstruction(path); @@ -97,8 +97,8 @@ export class FilePaths { } private async obstruction( - path: string - ): Promise<{ path: string; kind: "file" | "directory" } | undefined> { + path: RelativePath + ): Promise<{ path: RelativePath; kind: "file" | "directory" } | undefined> { const parts = path.split("/"); for (let i = 0; i < parts.length; i++) { const prefix = parts.slice(0, i + 1).join("/"); @@ -110,9 +110,9 @@ export class FilePaths { private async conflictPath( next: EngineState, - id: string, - path: string - ): Promise { + id: DocumentId, + path: RelativePath + ): Promise { const occupied = { ...next.local }; delete occupied[id]; const files = (await this.fs.listFilesRecursively()).filter( @@ -123,12 +123,12 @@ export class FilePaths { return allocatePortablePath(path, id, occupied); } - private async ensureParents(path: string): Promise { + private async ensureParents(path: RelativePath): Promise { const parent = path.split("/").slice(0, -1).join("/"); if (parent) await this.fs.createDirectory(parent); } - public async removeEmptyParents(path: string): Promise { + 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; diff --git a/frontend/sync-client/src/file-operations/filesystem-operations.ts b/frontend/sync-client/src/file-operations/filesystem-operations.ts index 02dd7cec..296297b1 100644 --- a/frontend/sync-client/src/file-operations/filesystem-operations.ts +++ b/frontend/sync-client/src/file-operations/filesystem-operations.ts @@ -8,24 +8,24 @@ import type { FileSnapshot } from "../snapshot"; * A successful scan is complete: unreadable directories must reject, not vanish. */ export interface FileSystemOperations { - listFilesRecursively: (root?: string) => Promise; + listFilesRecursively: (root?: RelativePath) => Promise; /** Return undefined on missing file */ - read: (path: string) => Promise; + read: (path: RelativePath) => Promise; /** Reject symlinks and special files rather than following them and return undefined. */ stat: ( - path: string + path: RelativePath ) => Promise<{ kind: "file" | "directory"; size: number } | undefined>; /** Includes directories. */ - exists: (path: string) => Promise; + exists: (path: RelativePath) => Promise; /** Recursive, idempotent directory creation. */ - createDirectory: (path: string) => Promise; + createDirectory: (path: RelativePath) => Promise; /** Create or overwrite a file, applying optional editor cursor metadata. */ - write: (path: string, snapshot: FileSnapshot) => Promise; + write: (path: RelativePath, snapshot: FileSnapshot) => Promise; // Atomically update the content of a text file. atomicUpdateText: ( @@ -34,11 +34,11 @@ export interface FileSystemOperations { ) => Promise; /** Move a file without replacing an existing destination. */ - rename: (from: string, to: string) => Promise; + rename: (from: RelativePath, to: RelativePath) => Promise; /** Idempotently unlink one regular file. */ - deleteFile: (path: string) => Promise; + deleteFile: (path: RelativePath) => Promise; /** Prune directories using rmdir only. Reject files and nonempty directories. */ - deleteDirectory: (path: string) => Promise; + deleteDirectory: (path: RelativePath) => Promise; } diff --git a/frontend/sync-client/src/index.ts b/frontend/sync-client/src/index.ts index ff901a7e..88a47698 100644 --- a/frontend/sync-client/src/index.ts +++ b/frontend/sync-client/src/index.ts @@ -21,7 +21,12 @@ export { export { Logger, LogLevel, LogLine } from "./tracing/logger"; export { type SyncSettings, DEFAULT_SETTINGS } from "./persistence/settings"; export { rateLimit } from "./utils/rate-limit"; -export type { RelativePath, StoredDatabase } from "./persistence/database"; +export type { + DocumentId, + RelativePath, + StoredDatabase, + VaultUpdateId +} from "./persistence/database"; export type { FileSystemOperations } from "./file-operations/filesystem-operations"; export type { FileSnapshot } from "./snapshot"; export type { MetadataPersistenceProvider as PersistenceProvider } from "./persistence/metadata-persistence-provider"; diff --git a/frontend/sync-client/src/persistence/database.test.ts b/frontend/sync-client/src/persistence/database.test.ts index ceaa14bc..2304b940 100644 --- a/frontend/sync-client/src/persistence/database.test.ts +++ b/frontend/sync-client/src/persistence/database.test.ts @@ -1,83 +1,38 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { Database, emptyState, type StoredDatabase } from "./database"; +import { Database, emptyState } from "./database"; -test("old state loads without duplicate flags, full recovery content, or acknowledged heads", async () => { - const head = { - documentId: "clean", - vaultUpdateId: 1, - contentSize: 3, - updatedDate: "2026-09-23", - userId: "user", - deviceId: "device" +test("current state keeps absent optional fields absent across save and reload", async () => { + const initial = emptyState("vault"); + initial.local = { local: "local.md", remote: "remote.md" }; + initial.documents = { + local: { observedHash: "local" }, + remote: { remote: { vaultUpdateId: 1, contentSize: 3 } }, + empty: {} }; - const legacy = { - ...emptyState("vault"), - initialized: true, - lastSeenUpdateId: 2, - protectedPaths: ["ignored.md"], - local: { - clean: "clean.md", - recovering: "recovering.md", - deferred: "deferred.md" - }, - documents: { - clean: { - materialized: true, - observedHash: "clean", - base: { ...head, hash: "clean" } + let saved = structuredClone(initial); + const restart = (): Database => + new Database( + saved, + async (next) => { + saved = structuredClone(next); }, - recovering: { - materialized: true, - observedHash: "edited", - recoveryBase: { contentBase64: "T0xE", hash: "old" } - }, - deferred: { materialized: false, observedHash: undefined } - }, - remoteHeads: { - clean: head, - deferred: { ...head, documentId: "deferred" } - } - }; - const saves: StoredDatabase[] = []; - const db = new Database( - legacy, - async (next) => { - saves.push(next); - }, - "vault", - async () => saves.at(-1) - ); + "vault", + async () => structuredClone(saved) + ); + const db = restart(); + assert.deepEqual(db.state, initial); await db.save(); - const saved = saves.at(-1); - assert(saved); - assert.deepEqual(saved.documents, { - clean: { - base: { vaultUpdateId: 1, hash: "clean" }, - observedHash: "clean" - }, - recovering: { - base: { vaultUpdateId: null, hash: "old" }, - observedHash: "edited" - }, - deferred: { - base: undefined, - observedHash: undefined, - remote: { vaultUpdateId: head.vaultUpdateId, contentSize: head.contentSize } - } - }); - assert.equal(saved.lastSeenUpdateId, 2); - for (const field of ["initialized", "remoteHeads", "protectedPaths"]) - assert(!(field in saved)); - assert(!JSON.stringify(saved).includes("contentBase64")); + assert.deepEqual(saved, initial); + assert.deepEqual(restart().state, initial); }); -test("old reset intent leaves the cursor absent after an uncertain save reload", async () => { - const legacy = { - ...emptyState("vault"), - initialized: false, - historyRecovery: true, - lastSeenUpdateId: 0 +test("current reset state is restored after an uncertain save", async () => { + const saved = emptyState("vault"); + saved.local = { note: "note.md" }; + saved.documents.note = { + base: { vaultUpdateId: null, hash: "clean" }, + observedHash: "clean" }; const db = new Database( undefined, @@ -85,10 +40,9 @@ test("old reset intent leaves the cursor absent after an uncertain save reload", throw new Error("interrupted"); }, "vault", - async () => legacy + async () => structuredClone(saved) ); await assert.rejects(db.save(), /interrupted/); await db.recoverPersistence(); - assert.equal(db.state.lastSeenUpdateId, undefined); - assert(!("historyRecovery" in db.state)); + assert.deepEqual(db.state, saved); }); diff --git a/frontend/sync-client/src/persistence/database.ts b/frontend/sync-client/src/persistence/database.ts index 9ae8e21f..6ab4d7e2 100644 --- a/frontend/sync-client/src/persistence/database.ts +++ b/frontend/sync-client/src/persistence/database.ts @@ -1,5 +1,4 @@ -import type { PendingRequest, UnconfirmedRequest } from "./requests"; -import { upgradeState } from "./upgrade-state"; +import type { PendingRequest } from "./requests"; import type { FileManifest } from "../services/types/FileManifest"; import type { FileManifestEntries } from "../types/file-manifest-entries"; @@ -8,48 +7,46 @@ export type DocumentId = string; export type RelativePath = string; export interface DocumentMetadata { - parentVersionId: number; + parentVersionId: VaultUpdateId; hash: string; } export interface ContentHead { - vaultUpdateId: number; + vaultUpdateId: VaultUpdateId; contentSize: number; } export interface DocumentState { /** A null version retains only the clean hash after a server history reset. */ - base?: { vaultUpdateId: number | null; hash: string }; + base?: { vaultUpdateId: VaultUpdateId | null; hash: string }; /** Last observed disk hash; an empty string means a notification awaits scanning. */ observedHash?: string; /** A remote version still to be incorporated, including excluded downloads. */ remote?: ContentHead; } -export type { PendingRequest, UnconfirmedRequest } from "./requests"; +export type { PendingRequest } from "./requests"; export interface EngineState { vaultKey: string; fileManifest: FileManifest; local: FileManifestEntries; - documents: Record; + documents: Record; /** Absent until a vault snapshot is applied; also marks a durable history reset. */ - lastSeenUpdateId?: number; + lastSeenUpdateId?: VaultUpdateId; pending?: PendingRequest; - /** A rejected retry does not prove an earlier attempt failed to commit. */ - unconfirmed?: UnconfirmedRequest[]; /** Commits consumption of the saved notification queue with the identity map. */ lastAppliedLocalChangeId?: string; /** Original server paths while local files are excluded; null means absent. */ - excluded?: Record; + excluded?: Record; } export type StoredDatabase = EngineState; // Read-only view used by cursor tracking and status reporting. export interface DocumentRecord { - documentId: string; - relativePath: string; + documentId: DocumentId; + relativePath: RelativePath; metadata?: DocumentMetadata; } @@ -62,13 +59,11 @@ export function emptyState(vaultKey: string): StoredDatabase { }; } -function canRebindEmpty(state: Partial | undefined): boolean { +function canRebindEmpty(state: StoredDatabase): boolean { return ( - state !== undefined && state.lastSeenUpdateId === undefined && !state.pending && - (state.unconfirmed?.length ?? 0) === 0 && - Object.keys(state.local ?? {}).length === 0 + Object.keys(state.local).length === 0 ); } @@ -76,15 +71,15 @@ export class Database { public state: StoredDatabase; private needsReload = false; public constructor( - initial: Partial | undefined, + initial: StoredDatabase | undefined, private readonly saveData: (data: StoredDatabase) => Promise, vaultKey: string, private readonly loadData: () => Promise ) { const state = - initial !== undefined && Object.keys(initial).length > 0 - ? upgradeState(initial as StoredDatabase) - : emptyState(vaultKey); + initial === undefined + ? emptyState(vaultKey) + : structuredClone(initial); if (state.vaultKey !== vaultKey && !canRebindEmpty(state)) { throw new Error( "Incompatible vault identity. Preserve it for recovery and initialize a separate state store." @@ -118,7 +113,7 @@ export class Database { public async recoverPersistence(): Promise { if (!this.needsReload) return; const saved = await this.loadData(); - if (saved) this.state = upgradeState(saved); + if (saved) this.state = structuredClone(saved); this.needsReload = false; } @@ -130,7 +125,7 @@ export class Database { return Object.keys(this.state.local).length; } - public getDocumentByDocumentId(id: string): DocumentRecord | undefined { + public getDocumentByDocumentId(id: DocumentId): DocumentRecord | undefined { const relativePath = this.state.local[id]; if (relativePath === undefined) return undefined; const base = this.state.documents[id]?.base; @@ -148,7 +143,7 @@ export class Database { } public getLatestDocumentByRelativePath( - path: string + path: RelativePath ): DocumentRecord | undefined { const id = Object.keys(this.state.local).find( (id) => this.state.local[id] === path diff --git a/frontend/sync-client/src/persistence/requests.ts b/frontend/sync-client/src/persistence/requests.ts index b99b3c67..9aa1f4c7 100644 --- a/frontend/sync-client/src/persistence/requests.ts +++ b/frontend/sync-client/src/persistence/requests.ts @@ -1,21 +1,14 @@ import type { PutFileContent } from "../services/types/PutFileContent"; import type { PushFileManifest } from "../services/types/PushFileManifest"; -import type { FileManifestEntries } from "../types/file-manifest-entries"; +import type { DocumentId } from "./database"; -/** Exact wire payload and the hash needed to advance the base after acknowledgement. */ -export type PendingRequest = - | { type: "content"; documentId: string; request: PutFileContent; hash: string } - | { type: "fileManifest"; request: PushFileManifest }; - -/** Enough to recognize a late receipt or suppress an unchanged rejected submission. */ -export type UnconfirmedRequest = { requestId: string; rejection?: string } & ( - | { type: "content"; documentId: string; hash: string } - | { type: "fileManifest"; entries: FileManifestEntries } +/** Exact wire payload retained until acknowledgement, including after rejection. */ +export type PendingRequest = { rejection?: string } & ( + | { + type: "content"; + documentId: DocumentId; + request: PutFileContent; + hash: string; + } + | { type: "fileManifest"; request: PushFileManifest } ); - -export function requestReceipt(request: PendingRequest, rejection?: string): UnconfirmedRequest { - const receipt = { requestId: request.request.requestId, rejection }; - return request.type === "content" - ? { ...receipt, type: "content", documentId: request.documentId, hash: request.hash } - : { ...receipt, type: "fileManifest", entries: request.request.entries }; -} diff --git a/frontend/sync-client/src/persistence/settings.ts b/frontend/sync-client/src/persistence/settings.ts index 1b592d65..c06c2b0a 100644 --- a/frontend/sync-client/src/persistence/settings.ts +++ b/frontend/sync-client/src/persistence/settings.ts @@ -1,3 +1,4 @@ +import type { RelativePath } from "./database"; import type { Logger } from "../tracing/logger"; import { globsToRegexes } from "../utils/globs-to-regexes"; import { isInternalPath } from "../utils/portable-path"; @@ -68,7 +69,7 @@ export class Settings { ); } - public isIgnored(path: string): boolean { + public isIgnored(path: RelativePath): boolean { return ( isInternalPath(path) || this.ignorePatterns.some((pattern) => pattern.test(path)) diff --git a/frontend/sync-client/src/persistence/upgrade-state.ts b/frontend/sync-client/src/persistence/upgrade-state.ts deleted file mode 100644 index 5c3ebf92..00000000 --- a/frontend/sync-client/src/persistence/upgrade-state.ts +++ /dev/null @@ -1,72 +0,0 @@ -import type { ContentHead, DocumentState, EngineState } from "./database"; -import { requestReceipt, type PendingRequest, type UnconfirmedRequest } from "./requests"; -import type { StoredSnapshot } from "../snapshot"; - -type PreviousRequest = PendingRequest & { - snapshot?: StoredSnapshot; - response?: unknown; - rejection?: string; -}; - -function upgradeRequest(request: PreviousRequest): PendingRequest { - return request.type === "content" - ? { - type: "content", documentId: request.documentId, - request: request.request, hash: request.snapshot?.hash ?? request.hash - } - : { type: "fileManifest", request: request.request }; -} - -/** Normalize earlier stores at the boundary; none of their duplicate data is saved again. */ -export function upgradeState(state: EngineState): EngineState { - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- Read previous persisted schemas at the migration boundary. - const legacy = state as Omit & { - initialized?: boolean; - historyRecovery?: boolean; - remoteHeads?: Record; - pending?: PreviousRequest; - unconfirmed?: (UnconfirmedRequest | PreviousRequest)[]; - documents: Record; - }; - const next: EngineState = { - vaultKey: state.vaultKey, - fileManifest: state.fileManifest, - local: state.local, - documents: {}, - lastSeenUpdateId: "initialized" in legacy && - (!legacy.initialized || legacy.historyRecovery === true) - ? undefined : state.lastSeenUpdateId, - pending: legacy.pending && upgradeRequest(legacy.pending), - unconfirmed: legacy.unconfirmed?.map((request) => - "request" in request - ? requestReceipt(upgradeRequest(request), request.rejection) - : request - ), - lastAppliedLocalChangeId: state.lastAppliedLocalChangeId, - excluded: state.excluded - }; - if (legacy.pending?.rejection && !legacy.pending.response) { - (next.unconfirmed ??= []).push(requestReceipt(upgradeRequest(legacy.pending), legacy.pending.rejection)); - delete next.pending; - } - for (const id of new Set([...Object.keys(legacy.documents), ...Object.keys(legacy.remoteHeads ?? {})])) { - const doc = legacy.documents[id] ?? {}; - const base = doc.recoveryBase - ? { vaultUpdateId: null, hash: doc.recoveryBase.hash } - : doc.base && doc.bootstrap !== true - ? { vaultUpdateId: doc.base.vaultUpdateId, hash: doc.base.hash } - : undefined; - const observedHash = "materialized" in doc - ? doc.materialized === true ? doc.observedHash ?? "" : undefined - : doc.observedHash; - const remote = doc.remote ?? legacy.remoteHeads?.[id]; - next.documents[id] = { base, observedHash }; - if (remote && (observedHash === undefined || remote.vaultUpdateId > (base?.vaultUpdateId ?? 0))) - next.documents[id].remote = { vaultUpdateId: remote.vaultUpdateId, contentSize: remote.contentSize }; - } - return next; -} diff --git a/frontend/sync-client/src/services/sync-service.test.ts b/frontend/sync-client/src/services/sync-service.test.ts index be8ae94c..18d8e98a 100644 --- a/frontend/sync-client/src/services/sync-service.test.ts +++ b/frontend/sync-client/src/services/sync-service.test.ts @@ -20,7 +20,10 @@ test("paused sync sends no requests, while connection checks still work", async let requests = 0; const service = new SyncService("device", settings(), async () => { requests++; - return Response.json({ headEventId: 0, events: [] }); + return Response.json( + { headEventId: 0, endEventId: 0, events: [] }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); }, { get: () => undefined, save: async () => { } }); await assert.rejects(service.getEvents(0), SyncResetError); assert.equal(requests, 0); @@ -42,7 +45,10 @@ test("pause aborts requests and fences late responses from an injected fetch", a signals.push(init!.signal!); return signals.length === 1 ? response.promise - : Response.json({ headEventId: 0, events: [] }); + : Response.json( + { headEventId: 0, endEventId: 0, events: [] }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); }, { get: () => undefined, @@ -66,7 +72,7 @@ test("pause aborts requests and fences late responses from an injected fetch", a ) ); await new Promise((resolve) => setImmediate(resolve)); - assert.deepEqual(saved, []); + assert.deepEqual(saved, ["0:empty"]); }); for (const binary of [false, true]) { @@ -179,3 +185,32 @@ test("history saves only new checkpoints, including a changed token at the same } assert.deepEqual(saved, ["1:first", "2:next", "2:replacement"]); }); + +for (const checkpoint of [undefined, ""]) { + test(`successful sync responses reject ${checkpoint === undefined ? "missing" : "empty"} history checkpoints`, async () => { + const service = new SyncService( + "device", + settings(), + async () => + Response.json( + { headEventId: 0, endEventId: 0, events: [] }, + { + headers: checkpoint === undefined + ? {} + : { "X-Vault-Link-History": checkpoint } + } + ), + { + get: (): undefined => undefined, + save: async (): Promise => { + assert.fail("must not save a missing checkpoint"); + } + } + ); + service.resume(); + await assert.rejects( + service.getEvents(0), + /Missing server history checkpoint/ + ); + }); +} diff --git a/frontend/sync-client/src/services/sync-service.ts b/frontend/sync-client/src/services/sync-service.ts index 561c6928..6f856920 100644 --- a/frontend/sync-client/src/services/sync-service.ts +++ b/frontend/sync-client/src/services/sync-service.ts @@ -1,6 +1,6 @@ +import type { DocumentId, VaultUpdateId } from "../persistence/database"; import type { Settings } from "../persistence/settings"; import { abortable } from "../utils/abortable"; -import { VaultHistoryStatus } from "../types/vault-history-status"; import type { DocumentUpdateResponse } from "./types/DocumentUpdateResponse"; import type { DocumentVersionWithoutContent } from "./types/DocumentVersionWithoutContent"; import type { EventBatch } from "./types/EventBatch"; @@ -31,12 +31,6 @@ export class SyncService { this.pause(); } - public get vaultHistoryStatus(): VaultHistoryStatus { - return this.history.get() !== undefined - ? VaultHistoryStatus.STORED - : VaultHistoryStatus.MISSING; - } - public pause(): void { this.session.abort(new SyncResetError()); } @@ -57,7 +51,7 @@ export class SyncService { return this.request("/vault-snapshot"); } - public async getEvents(after: number): Promise { + public async getEvents(after: VaultUpdateId): Promise { return this.request(`/events-since?after=${after}`); } @@ -68,13 +62,13 @@ export class SyncService { } public async putFileContent( - id: string, + id: DocumentId, request: PutFileContent ): Promise { return this.request(`/documents/${id}`, request); } - public async getDocumentMetadata(id: string): Promise { + public async getDocumentMetadata(id: DocumentId): Promise { return this.request(`/documents/${id}/metadata`); } @@ -82,8 +76,8 @@ export class SyncService { documentId, vaultUpdateId }: { - documentId: string; - vaultUpdateId: number; + documentId: DocumentId; + vaultUpdateId: VaultUpdateId; }): Promise { return this.request( `/documents/${documentId}/versions/${vaultUpdateId}/content`, @@ -95,27 +89,23 @@ export class SyncService { private getHistoryHeaders(): Record { const checkpoint = this.history.get(); - return checkpoint !== undefined && checkpoint !== "" + return checkpoint !== undefined ? { "X-Vault-Link-History": checkpoint } : {}; } private async recordHistory(response: Response): Promise { const checkpoint = response.headers.get("x-vault-link-history"); + if (checkpoint === null || checkpoint === "") + throw new PermanentSyncError("Missing server history checkpoint"); + const previous = this.history.get(); + if (checkpoint === previous) return; if ( - checkpoint !== null && - checkpoint !== "" - ) { - const previous = this.history.get(); - if (checkpoint === previous) return; - if ( - previous === undefined || - previous === "" || - Number(checkpoint.split(":")[0]) >= + previous === undefined || + Number(checkpoint.split(":")[0]) >= Number(previous.split(":")[0]) - ) - await this.history.save(checkpoint); - } + ) + await this.history.save(checkpoint); } private checkHistory(response: Response): void { diff --git a/frontend/sync-client/src/services/types/EventBatch.ts b/frontend/sync-client/src/services/types/EventBatch.ts index a3bfb573..feb7ef59 100644 --- a/frontend/sync-client/src/services/types/EventBatch.ts +++ b/frontend/sync-client/src/services/types/EventBatch.ts @@ -5,4 +5,4 @@ export type EventBatch = { headEventId: number, /** * Last event in this page; `head_event_id` is the vault's current head. */ -endEventId?: number, events: Array, }; +endEventId: number, events: Array, }; diff --git a/frontend/sync-client/src/services/types/PingResponse.ts b/frontend/sync-client/src/services/types/PingResponse.ts index c38845d2..1bd35bb5 100644 --- a/frontend/sync-client/src/services/types/PingResponse.ts +++ b/frontend/sync-client/src/services/types/PingResponse.ts @@ -18,7 +18,7 @@ isAuthenticated: boolean, */ mergeableFileExtensions: Array, /** - * API version ensuring backwards & forwards compatibility between the client + * API version that must match exactly between the client * and server. */ supportedApiVersion: number, }; diff --git a/frontend/sync-client/src/services/websocket-manager.test.ts b/frontend/sync-client/src/services/websocket-manager.test.ts index 4d2119cb..37aa2bbf 100644 --- a/frontend/sync-client/src/services/websocket-manager.test.ts +++ b/frontend/sync-client/src/services/websocket-manager.test.ts @@ -199,18 +199,22 @@ describe("WebSocketManager", () => { throw new Error("Buffer full"); }; - assert.throws(() => { - manager.sendHandshakeMessage({ - type: "handshake", - token: "test", - deviceId: "test" - }); - }); + assert.doesNotThrow(() => mockWs.onopen?.(new Event("open"))); + assert.equal(manager.isWebSocketConnected, false); + assert( + ( + mockLogger.warn as typeof mockLogger.warn & { + calls: [string][]; + } + ).calls.some(([message]) => + message.includes("WebSocket handshake failed") + ) + ); await manager.stop(); }); - it("completes stop with timeout protection", async () => { + it("completes stop after retiring the socket", async () => { const manager = new WebSocketManager( deviceId, mockLogger, @@ -222,7 +226,7 @@ describe("WebSocketManager", () => { await new Promise((resolve) => setTimeout(resolve, 50)); await manager.stop(); - assert.ok(true); + assert.equal(manager.isWebSocketConnected, false); }); it("uses a secure URL, preserves the configured prefix, and encodes the vault", async () => { @@ -451,19 +455,65 @@ for (const closeBehavior of ["throws", "stays open"]) { context.mock.timers.tick(0); const socket = (manager as unknown as { webSocket: MockWebSocket }) .webSocket; - const stopping = manager.stop(); - context.mock.timers.tick(10_000); - await stopping; + const delayedMessage = socket.onmessage; + await manager.stop(); // No close event or timer advance is needed. assert.equal(manager.isWebSocketConnected, false); assert.equal(connected, false); socket.simulateMessage({ type: "vaultChanged" }); + delayedMessage?.( + new MockMessageEvent("message", { + data: JSON.stringify({ type: "vaultChanged" }) + }) + ); await manager.waitUntilFinished(); assert.equal(delivered, 0); }); } +it("stop retires the socket immediately but drains messages already being handled", async (context) => { + context.mock.timers.enable({ apis: ["setTimeout"] }); + const logger = new Logger(); + const manager = new WebSocketManager( + "device", + logger, + new Settings( + logger, + { remoteUri: "http://test", token: "test-token" }, + async () => {} + ), + MockWebSocket as unknown as typeof WebSocket + ); + const release = Promise.withResolvers(); + context.after(async () => { + release.resolve(); + 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]), { + type: "handshake", + token: "test-token", + deviceId: "device" + }); + manager.onRemoteVaultUpdateReceived.add(async () => release.promise); + socket.simulateMessage({ type: "vaultChanged" }); + let finished = false; + const stopping = manager.stop().then(() => { + finished = true; + }); + assert.equal(manager.isWebSocketConnected, false); + assert.equal(socket.onmessage, null); + await Promise.resolve(); + assert(!finished); + release.resolve(); + await stopping; + assert(finished); +}); + 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 { @@ -484,14 +534,6 @@ it("a handshake send failure retires the connection without escaping the open ca settings, BrokenSendSocket as unknown as typeof WebSocket ); - manager.onWebSocketStatusChanged.add((connected) => { - if (connected) - manager.sendHandshakeMessage({ - type: "handshake", - token: "test", - deviceId: "test" - }); - }); const statuses: boolean[] = []; manager.onWebSocketStatusChanged.add((status) => statuses.push(status)); try { @@ -499,6 +541,10 @@ it("a handshake send failure retires the connection without escaping the open ca assert.doesNotThrow(() => context.mock.timers.tick(0)); assert.equal(manager.isWebSocketConnected, false); assert.equal(statuses.at(-1), false); + assert( + !statuses.includes(true), + "a failed handshake must never announce a connection" + ); } finally { await manager.stop(); } diff --git a/frontend/sync-client/src/services/websocket-manager.ts b/frontend/sync-client/src/services/websocket-manager.ts index 24107454..f82a2541 100644 --- a/frontend/sync-client/src/services/websocket-manager.ts +++ b/frontend/sync-client/src/services/websocket-manager.ts @@ -4,8 +4,7 @@ import type { WebSocketServerMessage } from "./types/WebSocketServerMessage"; import type { WebSocketClientMessage } from "./types/WebSocketClientMessage"; import type { CursorPositionFromClient } from "./types/CursorPositionFromClient"; import type { ClientCursors } from "./types/ClientCursors"; -import { createPromise } from "../utils/create-promise"; -import { WEBSOCKET_DISCONNECT_TIMEOUT_IN_S, WEBSOCKET_RECEIVE_TIMEOUT_MS } from "../consts"; +import { WEBSOCKET_RECEIVE_TIMEOUT_MS } from "../consts"; import { removeFromArray } from "../utils/remove-from-array"; import { EventListeners } from "../utils/data-structures/event-listeners"; import { awaitAll } from "../utils/await-all"; @@ -24,7 +23,6 @@ export class WebSocketManager { >(); private isStopped = true; - private resolveDisconnectingPromise: null | (() => unknown) = null; private receiveTimeoutId: ReturnType | undefined; private reconnectTimeoutId: ReturnType | undefined; @@ -69,58 +67,10 @@ export class WebSocketManager { } public async stop(): Promise { - const [promise, resolve] = createPromise(); - this.resolveDisconnectingPromise = resolve; - this.isStopped = true; - clearTimeout(this.receiveTimeoutId); - - if (this.reconnectTimeoutId !== undefined) { - clearTimeout(this.reconnectTimeoutId); - this.reconnectTimeoutId = undefined; - } - - const socket = this.webSocket; - try { - socket?.close(1000, "WebSocketManager has been stopped"); - } catch (error) { - this.logger.warn(`Failed to close WebSocket: ${String(error)}`); - if (socket) this.reconnect(socket); - } - - // eslint-disable-next-line @typescript-eslint/init-declarations - let timeoutId: ReturnType | undefined; - const timeoutPromise = new Promise((_, reject) => { - timeoutId = setTimeout(() => { - reject( - new Error( - `Timeout waiting for WebSocket to close after ${WEBSOCKET_DISCONNECT_TIMEOUT_IN_S} seconds` - ) - ); - }, WEBSOCKET_DISCONNECT_TIMEOUT_IN_S * 1000); - }); - - try { - while (this.isWebSocketConnected) { - await Promise.race([promise, timeoutPromise]); - } - } catch (error) { - this.logger.error( - `Error while waiting for WebSocket to close: ${String(error)}` - ); - // Force cleanup even if close didn't work - this.resolveDisconnectingPromise(); - this.resolveDisconnectingPromise = null; - } finally { - // Clear timeout to prevent unhandled rejection - if (timeoutId !== undefined) { - clearTimeout(timeoutId); - } - // A throwing, silent or timed-out close must still fence callbacks - // before a stopped client can release its state or restart. - if (socket) this.reconnect(socket); - } - + clearTimeout(this.reconnectTimeoutId); + this.reconnectTimeoutId = undefined; + this.retireSocket(); await this.waitUntilFinished(); } @@ -128,27 +78,6 @@ export class WebSocketManager { await awaitAll(this.outstandingPromises); } - public sendHandshakeMessage( - message: WebSocketClientMessage & { type: "handshake" } - ): void { - const { webSocket } = this; - if (!webSocket) { - throw new Error( - "WebSocket is not connected, cannot send handshake message" - ); - } - - try { - webSocket.send(JSON.stringify(message)); - } catch (error) { - this.logger.error( - `Failed to send handshake message: ${String(error)}` - ); - this.reconnect(webSocket); - throw error; - } - } - public updateLocalCursors(cursorPositions: CursorPositionFromClient): void { if (!this.isWebSocketConnected || !this.webSocket) { // A missing cursor update is fine, we can just skip it if needed @@ -190,30 +119,31 @@ export class WebSocketManager { this.receiveTimeoutId.unref?.(); } - private reconnect(socket: WebSocket): void { - if (this.webSocket !== socket) return; + private retireSocket(): void { clearTimeout(this.receiveTimeoutId); + const socket = this.webSocket; + if (!socket) return; + this.webSocket = undefined; socket.onopen = null; socket.onclose = null; socket.onmessage = null; socket.onerror = (): void => { /* Retired transports may still emit errors. */ }; - this.webSocket = undefined; this.onWebSocketStatusChanged.trigger(false); try { - socket.close(); - } catch { - /* The transport may already be gone. */ - } - if (this.isStopped) { - this.resolveDisconnectingPromise?.(); - this.resolveDisconnectingPromise = null; - } else { - this.scheduleReconnect(); + socket.close(1000, "WebSocket retired"); + } catch (error) { + this.logger.warn(`Failed to close WebSocket: ${String(error)}`); } } + private reconnect(socket: WebSocket): void { + if (this.webSocket !== socket) return; + this.retireSocket(); + this.scheduleReconnect(); + } + private scheduleReconnect(): void { if (this.isStopped) return; clearTimeout(this.reconnectTimeoutId); @@ -237,23 +167,7 @@ export class WebSocketManager { } private openWebSocket(): void { - // Clean up old WebSocket handlers to prevent race conditions - if (this.webSocket) { - try { - // Remove handlers to prevent them from firing after new connection - this.webSocket.onopen = null; - this.webSocket.onclose = null; - this.webSocket.onmessage = null; - this.webSocket.onerror = (): void => { - /* Retired transports may still emit errors. */ - }; - this.webSocket.close(); - } catch (e) { - this.logger.error( - `Failed to close previous WebSocket connection: ${e}` - ); - } - } + this.retireSocket(); const { remoteUri, vaultName } = this.settings.getSettings(); const wsUri = new URL(remoteUri); @@ -274,19 +188,17 @@ export class WebSocketManager { }; this.webSocket.onopen = (): void => { - // Check if we've been stopped while connecting - if (this.isStopped) { - this.webSocket?.close( - 1000, - "WebSocketManager was stopped during connection" - ); - return; - } - this.logger.info("WebSocket connection opened"); + if (this.webSocket !== socket || this.isStopped) return; try { + const handshake: WebSocketClientMessage = { + type: "handshake", + token: this.settings.getSettings().token, + deviceId: this.deviceId + }; + socket.send(JSON.stringify(handshake)); + this.logger.info("WebSocket connection opened"); this.onWebSocketStatusChanged.trigger(true); - // A handshake failure can retire this socket from inside an - // observer. Finish delivery with its actual connection state. + // An observer may stop or replace the connection during delivery. if (this.webSocket !== socket) this.onWebSocketStatusChanged.trigger(false); } catch (error) { @@ -298,6 +210,7 @@ export class WebSocketManager { }; this.webSocket.onmessage = (event): void => { + if (this.webSocket !== socket || this.isStopped) return; try { // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion const message = JSON.parse( @@ -322,7 +235,7 @@ export class WebSocketManager { ); }); - void this.outstandingPromises.push(messageHandlingPromise); // ignore the returned promise + this.outstandingPromises.push(messageHandlingPromise); } catch (error) { this.logger.error( `Error parsing WebSocket message: ${String(error)}` diff --git a/frontend/sync-client/src/snapshot.ts b/frontend/sync-client/src/snapshot.ts index 9a44a903..6161b805 100644 --- a/frontend/sync-client/src/snapshot.ts +++ b/frontend/sync-client/src/snapshot.ts @@ -1,4 +1,3 @@ -import { base64ToBytes, bytesToBase64 } from "byte-base64"; import type { CursorPosition } from "reconcile-text"; import { hash } from "./utils/hash"; @@ -8,25 +7,14 @@ export interface FileSnapshot { cursors?: CursorPosition[]; } -export interface StoredSnapshot { - contentBase64: string; +export interface HashedSnapshot extends FileSnapshot { hash: string; - cursors?: CursorPosition[]; } -export async function toStoredSnapshot( +/** Own the bytes so later adapter mutations cannot invalidate the hash. */ +export async function hashSnapshot( snapshot: FileSnapshot -): Promise { - return { - contentBase64: bytesToBase64(snapshot.content), - cursors: snapshot.cursors, - hash: await hash(snapshot.content) - }; -} - -export function fromStoredSnapshot(snapshot: StoredSnapshot): FileSnapshot { - return { - content: base64ToBytes(snapshot.contentBase64), - cursors: snapshot.cursors - }; +): Promise { + const content = new Uint8Array(snapshot.content); + return { content, cursors: snapshot.cursors, hash: await hash(content) }; } diff --git a/frontend/sync-client/src/sync-client.ts b/frontend/sync-client/src/sync-client.ts index 41c7e5da..abf79eeb 100644 --- a/frontend/sync-client/src/sync-client.ts +++ b/frontend/sync-client/src/sync-client.ts @@ -1,7 +1,11 @@ import * as Sentry from "@sentry/browser"; import { setUpTelemetry } from "./utils/set-up-telemetry"; import type { MetadataPersistenceProvider } from "./persistence/metadata-persistence-provider"; -import { Database, type StoredDatabase } from "./persistence/database"; +import { + Database, + type RelativePath, + type StoredDatabase +} from "./persistence/database"; import { Settings, DEFAULT_SETTINGS, @@ -16,7 +20,7 @@ import { ServerConfig } from "./services/server-config"; import { WebSocketManager } from "./services/websocket-manager"; import { Syncer } from "./sync-operations/syncer"; import { CursorTracker } from "./sync-operations/cursor-tracker"; -import { FileChangeNotifier } from "./sync-operations/file-change-notifier"; +import { EventListeners } from "./utils/data-structures/event-listeners"; import { createClientId } from "./utils/create-client-id"; import { DocumentSyncStatus } from "./types/document-sync-status"; import type { CursorSpan } from "./services/types/CursorSpan"; @@ -30,11 +34,8 @@ import { export type StoredClient = Partial<{ settings: Partial; - database: Partial; - localChanges: ( - | LocalChange - | { type: "update"; path: string; changeId?: string } - )[]; + database: StoredDatabase; + localChanges: LocalChange[]; localChangesVaultKey: string; historyCheckpoint: { vaultKey: string; checkpoint?: string }; }>; @@ -62,7 +63,9 @@ export class SyncClient { private readonly service: SyncService, private readonly serverConfig: ServerConfig, private readonly cursorTracker: CursorTracker, - private readonly fileChangeNotifier: FileChangeNotifier, + private readonly fileChanges: EventListeners< + (path: RelativePath) => unknown + >, private readonly contentCache: FixedSizeDocumentCache, private readonly persistence: MetadataPersistenceProvider ) { @@ -96,12 +99,12 @@ export class SyncClient { }): Promise { const logger = new Logger(); let stored = (await persistence.load()) ?? {}; - let saving: Promise = Promise.resolve(); + 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 save = async (update: StoredClient): Promise => { - const operation = saving.then(async () => { + const save = async (update: StoredClient): Promise => + persistenceLock.withLock(async () => { if (reloadBeforeSave) { stored = (await persistence.load()) ?? {}; reloadBeforeSave = false; @@ -117,9 +120,6 @@ export class SyncClient { throw error; } }); - saving = operation.catch(() => { }); - return operation; - }; const settings = new Settings(logger, stored.settings, async (data) => save({ settings: data }) ); @@ -127,19 +127,17 @@ export class SyncClient { stored.database, async (data) => save({ database: data }), vaultKey(settings.getSettings()), - async () => { - await saving; - stored = (await persistence.load()) ?? {}; - return stored.database as StoredDatabase | undefined; - } + async () => + persistenceLock.withLock(async () => { + stored = (await persistence.load()) ?? {}; + reloadBeforeSave = false; + return stored.database; + }) ); - // Older stores also queued content updates. Trim the acknowledged prefix - // before discarding them, since its marker may belong to an update. const localChanges = unappliedChanges( database.state, stored.localChanges ?? [] - // eslint-disable-next-line no-restricted-syntax -- Migration needs a narrowed copy, not removal of a specific object. - ).filter((change) => change.type !== "update"); + ); if ( localChanges.length && stored.localChangesVaultKey !== vaultKey(settings.getSettings()) @@ -152,7 +150,7 @@ export class SyncClient { const service = new SyncService(deviceId, settings, fetch, { get: () => stored.historyCheckpoint?.vaultKey === - vaultKey(settings.getSettings()) + vaultKey(settings.getSettings()) ? stored.historyCheckpoint.checkpoint : undefined, save: async (checkpoint) => @@ -164,18 +162,17 @@ export class SyncClient { }) }); const serverConfig = new ServerConfig(service); - const notifier = new FileChangeNotifier(); - let syncer: Syncer; + const notifier = new EventListeners<(path: RelativePath) => unknown>(); const files = new FileOperations( fs, database, serverConfig, (paths) => { - for (const path of paths) notifier.notifyOfFileChange(path); + for (const path of paths) notifier.trigger(path); }, { entries: () => localChanges, - flush: async () => syncer?.flushLocalChanges() + flush: async () => syncer.flushLocalChanges() }, (path, size) => settings.isIgnored(path) || settings.isOversized(size) @@ -190,8 +187,7 @@ export class SyncClient { const contentCache = new FixedSizeDocumentCache( settings.getSettings().diffCacheSizeMB * 1024 * 1024 ); - syncer = new Syncer( - deviceId, + const syncer: Syncer = new Syncer( logger, database, settings, @@ -380,23 +376,23 @@ export class SyncClient { } /** Notifications enqueue work. Use waitUntilFinished() to await convergence. */ - public async syncLocallyCreatedFile(path: string): Promise { + public async syncLocallyCreatedFile(path: RelativePath): Promise { this.check(); if (isInternalPath(path)) return; - this.fileChangeNotifier.notifyOfFileChange(path); + this.fileChanges.trigger(path); await this.syncer.syncLocallyCreatedFile(path); } - public async syncLocallyDeletedFile(path: string): Promise { + public async syncLocallyDeletedFile(path: RelativePath): Promise { this.check(); if (isInternalPath(path)) return; - this.fileChangeNotifier.notifyOfFileChange(path); + this.fileChanges.trigger(path); await this.syncer.syncLocallyDeletedFile(path); } public async syncLocallyUpdatedFile(change: { - oldPath?: string; - relativePath: string; + oldPath?: RelativePath; + relativePath: RelativePath; }): Promise { this.check(); if ( @@ -404,18 +400,18 @@ export class SyncClient { (change.oldPath && isInternalPath(change.oldPath)) ) return; - this.fileChangeNotifier.notifyOfFileChange(change.relativePath); + this.fileChanges.trigger(change.relativePath); await this.syncer.syncLocallyUpdatedFile(change); } public async updateLocalCursors( - cursors: Record + cursors: Record ): Promise { this.check(); await this.cursorTracker.sendLocalCursorsToServer(cursors); } - public getDocumentSyncingStatus(path: string): DocumentSyncStatus { + public getDocumentSyncingStatus(path: RelativePath): DocumentSyncStatus { this.check(); if (!this.settings.getSettings().isSyncEnabled) return DocumentSyncStatus.SYNCING_IS_DISABLED; diff --git a/frontend/sync-client/src/sync-operations/content.test.ts b/frontend/sync-client/src/sync-operations/content.test.ts index e33c935e..bb5f227f 100644 --- a/frontend/sync-client/src/sync-operations/content.test.ts +++ b/frontend/sync-client/src/sync-operations/content.test.ts @@ -1,22 +1,23 @@ import assert from "node:assert/strict"; import { describe, it } from "node:test"; -import { fromStoredSnapshot, toStoredSnapshot } from "../snapshot"; +import { hashSnapshot } from "../snapshot"; import { mergeContent } from "./content"; const extensions = ["md"]; const snapshot = async (text: string) => - toStoredSnapshot({ content: new TextEncoder().encode(text) }); + hashSnapshot({ content: new TextEncoder().encode(text) }); describe("content reconciliation without file timestamps", () => { - it("round-trips content and editor selections without timestamps", async () => { + it("hashes snapshots without changing bytes or editor selections", async () => { const original = { content: new Uint8Array([0, 255, 13, 10]), cursors: [] }; - assert.deepEqual( - fromStoredSnapshot(await toStoredSnapshot(original)), - original - ); + const snapshot = await hashSnapshot(original); + assert.deepEqual(snapshot.content, original.content); + assert.deepEqual(snapshot.cursors, original.cursors); + original.content.fill(1); + assert.deepEqual(snapshot.content, new Uint8Array([0, 255, 13, 10])); }); it("keeps local binary edits when the server content is unchanged", async () => { @@ -29,7 +30,7 @@ describe("content reconciliation without file timestamps", () => { base, extensions ); - assert.equal(merged.contentBase64, local.contentBase64); + assert.deepEqual(merged.content, local.content); }); it("incorporates server binary edits when local content is unchanged", async () => { @@ -42,7 +43,7 @@ describe("content reconciliation without file timestamps", () => { remote, extensions ); - assert.equal(merged.contentBase64, remote.contentBase64); + assert.deepEqual(merged.content, remote.content); }); it("keeps the server on concurrent binary or unmergeable text edits", async () => { @@ -61,7 +62,7 @@ describe("content reconciliation without file timestamps", () => { remote, extensions ); - assert.equal(merged.contentBase64, remote.contentBase64, path); + assert.deepEqual(merged.content, remote.content, path); } }); @@ -75,7 +76,7 @@ describe("content reconciliation without file timestamps", () => { remote, extensions ); - assert.equal(merged.contentBase64, remote.contentBase64); + assert.deepEqual(merged.content, remote.content); }); it("still merges independent text edits from both sides", async () => { @@ -87,7 +88,7 @@ describe("content reconciliation without file timestamps", () => { extensions ); assert.equal( - new TextDecoder().decode(fromStoredSnapshot(merged).content), + new TextDecoder().decode(merged.content), "Updated first paragraph.\n\nUpdated last paragraph.\n" ); }); diff --git a/frontend/sync-client/src/sync-operations/content.ts b/frontend/sync-client/src/sync-operations/content.ts index 756615fa..308068f5 100644 --- a/frontend/sync-client/src/sync-operations/content.ts +++ b/frontend/sync-client/src/sync-operations/content.ts @@ -1,32 +1,31 @@ -import { base64ToBytes } from "byte-base64"; +import type { RelativePath } from "../persistence/database"; import { reconcile } from "reconcile-text"; -import { toStoredSnapshot, type StoredSnapshot } from "../snapshot"; +import { hashSnapshot, type HashedSnapshot } from "../snapshot"; import { isBinary } from "../utils/is-binary"; import { isFileTypeMergeable } from "../utils/is-file-type-mergeable"; export async function mergeContent( - path: string, - base: StoredSnapshot | undefined, - local: StoredSnapshot, - remote: StoredSnapshot, + path: RelativePath, + base: HashedSnapshot | undefined, + local: HashedSnapshot, + remote: HashedSnapshot, extensions: string[] -): Promise { - if (local.contentBase64 === remote.contentBase64) +): Promise { + if (local.hash === remote.hash) return { ...remote, cursors: local.cursors }; - if (!local.cursors && local.contentBase64 === base?.contentBase64) - return remote; + if (!local.cursors && local.hash === base?.hash) return remote; - if (remote.contentBase64 === base?.contentBase64) return local; + if (remote.hash === base?.hash) return local; const inputs = [ - base?.contentBase64 ?? "", - local.contentBase64, - remote.contentBase64 - ].map(base64ToBytes); + base?.content ?? new Uint8Array(), + local.content, + remote.content + ]; // Concurrent unmergeable edits keep the server's content. if (!isFileTypeMergeable(path, extensions) || inputs.some(isBinary)) @@ -42,7 +41,7 @@ export async function mergeContent( remoteText ); - return toStoredSnapshot({ + return hashSnapshot({ content: new TextEncoder().encode(merged.text), cursors: local.cursors ? merged.cursors : undefined }); 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 9b1c80da..a864bc99 100644 --- a/frontend/sync-client/src/sync-operations/cursor-tracker.test.ts +++ b/frontend/sync-client/src/sync-operations/cursor-tracker.test.ts @@ -1,71 +1,79 @@ /* eslint-disable @typescript-eslint/no-unsafe-type-assertion */ import assert from "node:assert/strict"; import { it } from "node:test"; -import type { Database } from "../persistence/database"; +import type { + Database, + RelativePath, + VaultUpdateId +} from "../persistence/database"; import type { FileOperations } from "../file-operations/file-operations"; import type { WebSocketManager } from "../services/websocket-manager"; import type { ClientCursors } from "../services/types/ClientCursors"; -import { DocumentUpToDateness } from "../types/document-up-to-dateness"; import { EventListeners } from "../utils/data-structures/event-listeners"; import { hash } from "../utils/hash"; import { CursorTracker } from "./cursor-tracker"; -import { FileChangeNotifier } from "./file-change-notifier"; -it("retains future cursors and publishes them when content catches up", async () => { +it("keeps the last usable cursor while retaining only the latest future update", async (context) => { const content = new TextEncoder().encode("current"); const contentHash = await hash(content); + let version = 1; const database = { getDocumentByDocumentId: () => ({ documentId: "document", relativePath: "note.md", - metadata: { parentVersionId: 2, hash: contentHash } + metadata: { parentVersionId: version, hash: contentHash } }) } as unknown as Database; const websocket = { - onWebSocketStatusChanged: new EventListeners< - (connected: boolean) => unknown - >(), + onWebSocketStatusChanged: new EventListeners(), onRemoteCursorsUpdateReceived: new EventListeners< (cursors: ClientCursors[]) => Promise >(), updateLocalCursors: () => {} } as unknown as WebSocketManager; - const files = { read: async () => content } as unknown as FileOperations; - const notifier = new FileChangeNotifier(); - const tracker = new CursorTracker(database, websocket, files, notifier); - const future = { + const notifier = new EventListeners<(path: RelativePath) => unknown>(); + const tracker = new CursorTracker( + database, + websocket, + { read: async () => content } as unknown as FileOperations, + notifier + ); + context.after(() => tracker.reset()); + let received: ClientCursors[] = []; + tracker.onRemoteCursorsUpdated.add((cursors) => { + received = cursors; + }); + const cursor = (version: number): ClientCursors => ({ userName: "Other", deviceId: "device", documentsWithCursors: [ { document_id: "document", relative_path: "old-name.md", - vault_update_id: 2, - cursors: [{ start: 0, end: 1 }] + vault_update_id: version, + cursors: [{ start: version, end: version }] } - ], - upToDateness: DocumentUpToDateness.Later - }; - const internals = tracker as unknown as { - knownRemoteCursors: (typeof future)[]; - getRelevantAndPruneKnownClientCursors: () => ClientCursors[]; - }; - internals.knownRemoteCursors = [future]; - - assert.deepEqual(internals.getRelevantAndPruneKnownClientCursors(), []); - assert.strictEqual(internals.knownRemoteCursors.length, 1); - - let received: ClientCursors[] = []; - tracker.onRemoteCursorsUpdated.add((cursors) => { - received = cursors; + ] }); - await notifier.onFileChanged.triggerAsync("note.md"); - - assert.strictEqual(received.length, 1); - assert.strictEqual( - received[0].documentsWithCursors[0].relative_path, - "note.md" + for (const remoteVersion of [1, 2, 3]) { + await websocket.onRemoteCursorsUpdateReceived.triggerAsync([ + cursor(remoteVersion) + ]); + assert.equal(received[0].documentsWithCursors[0].vault_update_id, 1); + } + version = 2; + await notifier.triggerAsync("note.md"); + assert.equal( + received[0].documentsWithCursors[0].vault_update_id, + 1, + "superseded future cursors must not reappear" ); + version = 3; + await notifier.triggerAsync("note.md"); + assert.equal(received[0].documentsWithCursors[0].vault_update_id, 3); + assert.equal(received[0].documentsWithCursors[0].relative_path, "note.md"); + await websocket.onRemoteCursorsUpdateReceived.triggerAsync([]); + assert.deepEqual(received, []); }); it("refreshes unchanged cursors on reconnect and heartbeat, and follows the document through renames", async (context) => { @@ -79,14 +87,14 @@ it("refreshes unchanged cursors on reconnect and heartbeat, and follows the docu }); const contentHash = await hash(content); const database = { - getLatestDocumentByRelativePath: (candidate: string) => + getLatestDocumentByRelativePath: (candidate: RelativePath) => candidate === path ? record() : undefined, getDocumentByDocumentId: () => record() } as unknown as Database; const sent: { documentsWithCursors: { - relative_path: string; - vault_update_id: number | null; + relative_path: RelativePath; + vault_update_id: VaultUpdateId | null; }[]; }[] = []; const websocket = { @@ -98,7 +106,7 @@ it("refreshes unchanged cursors on reconnect and heartbeat, and follows the docu sent.push(value); } } as unknown as WebSocketManager; - const notifier = new FileChangeNotifier(); + const notifier = new EventListeners<(path: RelativePath) => unknown>(); const tracker = new CursorTracker( database, websocket, @@ -125,7 +133,7 @@ it("refreshes unchanged cursors on reconnect and heartbeat, and follows the docu await drain(); assert.equal(sent.length, 3); path = "renamed.md"; - await notifier.onFileChanged.triggerAsync(path); + await notifier.triggerAsync(path); assert.equal(sent.at(-1)!.documentsWithCursors[0].relative_path, path); websocket.onWebSocketStatusChanged.trigger(false); const count = sent.length; @@ -173,7 +181,7 @@ for (const interruption of ["disconnect", "reset"] as const) { return content; } } as unknown as FileOperations, - new FileChangeNotifier() + new EventListeners<(path: RelativePath) => unknown>() ); let received: ClientCursors[] = []; tracker.onRemoteCursorsUpdated.add((cursors) => { @@ -243,7 +251,7 @@ it("a remote cursor becomes outdated if the document advances during its snapsho return content; } } as unknown as FileOperations, - new FileChangeNotifier() + new EventListeners<(path: RelativePath) => unknown>() ); let outdated: boolean | undefined; tracker.onRemoteCursorsUpdated.add((cursors) => { diff --git a/frontend/sync-client/src/sync-operations/cursor-tracker.ts b/frontend/sync-client/src/sync-operations/cursor-tracker.ts index 02399c2a..086eaa20 100644 --- a/frontend/sync-client/src/sync-operations/cursor-tracker.ts +++ b/frontend/sync-client/src/sync-operations/cursor-tracker.ts @@ -1,5 +1,9 @@ import type { FileOperations } from "../file-operations/file-operations"; -import type { Database, RelativePath } from "../persistence/database"; +import type { + Database, + DocumentId, + RelativePath +} from "../persistence/database"; import type { ClientCursors } from "../services/types/ClientCursors"; import type { CursorSpan } from "../services/types/CursorSpan"; import type { DocumentWithCursors } from "../services/types/DocumentWithCursors"; @@ -7,7 +11,6 @@ import type { WebSocketManager } from "../services/websocket-manager"; import type { MaybeOutdatedClientCursors } from "../types/maybe-outdated-client-cursors"; import { DocumentUpToDateness } from "../types/document-up-to-dateness"; import { hash } from "../utils/hash"; -import type { FileChangeNotifier } from "./file-change-notifier"; import { Lock } from "../utils/data-structures/locks"; import { EventListeners } from "../utils/data-structures/event-listeners"; import { CURSOR_HEARTBEAT_INTERVAL_MS } from "../consts"; @@ -30,11 +33,10 @@ export class CursorTracker { upToDateness: DocumentUpToDateness; })[] = []; - private lastLocalCursorState: DocumentWithCursors[] = []; - private lastLocalCursorStateWithoutDirtyDocuments: DocumentWithCursors[] = - []; - private localCursors: Record = {}; - private localCursorDocuments: Record = {}; + private lastCursorInput = "[]"; + private lastCursorMessage = "[]"; + private localCursors: Record = {}; + private localCursorDocuments: Record = {}; private cursorTimer?: ReturnType; private readonly sendLock = new Lock(); @@ -42,13 +44,13 @@ export class CursorTracker { private readonly database: Database, private readonly webSocketManager: WebSocketManager, private readonly fileOperations: FileOperations, - private readonly fileChangeNotifier: FileChangeNotifier + fileChanges: EventListeners<(path: RelativePath) => unknown> ) { this.webSocketManager.onWebSocketStatusChanged.add((connected) => { this.sessionGeneration++; clearInterval(this.cursorTimer); - this.lastLocalCursorState = []; - this.lastLocalCursorStateWithoutDirtyDocuments = []; + this.lastCursorInput = "[]"; + this.lastCursorMessage = "[]"; if (!connected) { this.knownRemoteCursors = []; this.onRemoteCursorsUpdated.trigger([]); @@ -99,7 +101,7 @@ export class CursorTracker { } ); - this.fileChangeNotifier.onFileChanged.add(async (relativePath) => { + fileChanges.add(async (relativePath) => { const generation = this.sessionGeneration; await this.publishLocalCursors(true); const cursors = await this.updateLock.withLock(async () => { @@ -174,52 +176,19 @@ export class CursorTracker { }); } - if ( - !force && - JSON.stringify(this.lastLocalCursorState) === - JSON.stringify(documentsWithCursors) - ) { - // Caching step to avoid reading the edited files all the time - return; - } - this.lastLocalCursorState = structuredClone(documentsWithCursors); + const input = JSON.stringify(documentsWithCursors); + if (!force && this.lastCursorInput === input) return; + this.lastCursorInput = input; for (const doc of documentsWithCursors) { - let readContent: Uint8Array; - try { - readContent = await this.fileOperations.read( - doc.relative_path - ); - } catch { + if (!(await this.matchesLocalVersion(doc))) doc.vault_update_id = null; - continue; - } - const digest = await hash(readContent); - const record = this.database.getLatestDocumentByRelativePath( - doc.relative_path - ); - if ( - record?.documentId !== doc.document_id || - record.relativePath !== doc.relative_path || - record.metadata?.parentVersionId !== doc.vault_update_id || - record.metadata.hash !== digest - ) { - doc.vault_update_id = null; - } } if (generation !== this.sessionGeneration) return; - if ( - !force && - JSON.stringify( - this.lastLocalCursorStateWithoutDirtyDocuments - ) === JSON.stringify(documentsWithCursors) - ) { - return; - } - - this.lastLocalCursorStateWithoutDirtyDocuments = - documentsWithCursors; + const message = JSON.stringify(documentsWithCursors); + if (!force && this.lastCursorMessage === message) return; + this.lastCursorMessage = message; this.webSocketManager.updateLocalCursors({ documentsWithCursors }); }); @@ -230,8 +199,8 @@ export class CursorTracker { clearInterval(this.cursorTimer); this.knownRemoteCursors = []; this.onRemoteCursorsUpdated.trigger([]); - this.lastLocalCursorState = []; - this.lastLocalCursorStateWithoutDirtyDocuments = []; + this.lastCursorInput = "[]"; + this.lastCursorMessage = "[]"; // Keep serialization intact while old reads drain. A generation fence // discards both active and queued work from the retired connection. } @@ -286,28 +255,13 @@ export class CursorTracker { private async getDocumentsUpToDateness( clientCursor: ClientCursors ): Promise { - const results = []; + let result = DocumentUpToDateness.UpToDate; for (const document of clientCursor.documentsWithCursors) { - results.push(await this.getDocumentUpToDateness(document)); + const status = await this.getDocumentUpToDateness(document); + if (status === DocumentUpToDateness.Later) return status; + if (status === DocumentUpToDateness.Prior) result = status; } - - if ( - results.every((result) => result === DocumentUpToDateness.UpToDate) - ) { - return DocumentUpToDateness.UpToDate; - } - - if ( - results.every( - (result) => - result === DocumentUpToDateness.UpToDate || - result === DocumentUpToDateness.Prior - ) - ) { - return DocumentUpToDateness.Prior; - } - - return DocumentUpToDateness.Later; + return result; } private async getDocumentUpToDateness( @@ -335,24 +289,30 @@ export class CursorTracker { return DocumentUpToDateness.Prior; } - let currentContent: Uint8Array; - try { - currentContent = await this.fileOperations.read( - record.relativePath - ); - } catch { - return DocumentUpToDateness.Prior; - } - - const contentHash = await hash(currentContent); - const current = this.database.getDocumentByDocumentId( - document.document_id - ); - return current?.metadata?.parentVersionId === - document.vault_update_id && - current.relativePath === record.relativePath && - current.metadata.hash === contentHash + return (await this.matchesLocalVersion({ + ...document, + relative_path: record.relativePath + })) ? DocumentUpToDateness.UpToDate : DocumentUpToDateness.Prior; } + + /** Hash a coherent read and verify that its identity, path and base still match. */ + private async matchesLocalVersion({ + document_id, + relative_path, + vault_update_id + }: DocumentWithCursors): Promise { + const content = await this.fileOperations + .read(relative_path) + .catch(() => undefined); + if (!content) return false; + const digest = await hash(content); + const current = this.database.getDocumentByDocumentId(document_id); + return ( + current?.metadata?.parentVersionId === vault_update_id && + current.relativePath === relative_path && + current.metadata.hash === digest + ); + } } diff --git a/frontend/sync-client/src/sync-operations/file-change-notifier.ts b/frontend/sync-client/src/sync-operations/file-change-notifier.ts deleted file mode 100644 index d1e49d62..00000000 --- a/frontend/sync-client/src/sync-operations/file-change-notifier.ts +++ /dev/null @@ -1,12 +0,0 @@ -import type { RelativePath } from "../persistence/database"; -import { EventListeners } from "../utils/data-structures/event-listeners"; - -export class FileChangeNotifier { - public readonly onFileChanged = new EventListeners< - (filePath: RelativePath) => unknown - >(); - - public notifyOfFileChange(filePath: RelativePath): void { - this.onFileChanged.trigger(filePath); - } -} diff --git a/frontend/sync-client/src/sync-operations/file-manifest.ts b/frontend/sync-client/src/sync-operations/file-manifest.ts index fa4cc1b5..683c788b 100644 --- a/frontend/sync-client/src/sync-operations/file-manifest.ts +++ b/frontend/sync-client/src/sync-operations/file-manifest.ts @@ -1,3 +1,9 @@ +import type { FileManifest } from "../services/types/FileManifest"; +import type { + DocumentId, + EngineState, + RelativePath +} from "../persistence/database"; import type { FileManifestEntries } from "../types/file-manifest-entries"; import { allocatePortablePath, @@ -21,8 +27,8 @@ export function sameFileManifest( export function resolvePaths( proposed: FileManifestEntries, remote: FileManifestEntries, - protectedIds: readonly string[] = [], - reservedPaths: readonly string[] = [] + protectedIds: readonly DocumentId[] = [], + reservedPaths: readonly RelativePath[] = [] ): FileManifestEntries { const result: FileManifestEntries = {}; for (const id of protectedIds) @@ -34,7 +40,7 @@ export function resolvePaths( const ids = Object.keys(proposed).sort( (a, b) => Number(remote[b] === proposed[b]) - - Number(remote[a] === proposed[a]) || + Number(remote[a] === proposed[a]) || (a < b ? -1 : a > b ? 1 : 0) ); for (const id of ids.filter( @@ -57,17 +63,81 @@ export function mergeFileManifests( ...Object.keys(local), ...Object.keys(remote) ])) { - let path: string | undefined; - - if (local[id] === base[id]) { - path = remote[id]; - } else if (remote[id] === base[id]) { - path = local[id]; - } else { - path = remote[id]; - } - + const path = remote[id] === base[id] ? local[id] : remote[id]; 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, + remote: FileManifest, + isIgnored: (path: RelativePath) => boolean, + unsyncablePaths: ReadonlySet +): void { + 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; + } + const previousManifest = next.fileManifest.entries; + next.local = mergeFileManifests(base, next.local, remote.entries); + next.fileManifest = remote; + for (const [id, oldPath] of Object.entries(before)) { + const path = next.local[id]; + if ( + isIgnored(oldPath) || + unsyncablePaths.has(oldPath) || + (path !== undefined && isIgnored(path)) + ) { + next.excluded ??= {}; + 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))) + delete next.excluded?.[id]; + } + next.local = resolvePaths( + next.local, + remote.entries, + Object.keys(next.excluded ?? {}), + [...unsyncablePaths] + ); +} + +export function manifestToSubmit( + state: EngineState, + 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; + } + for (const id of Object.keys(entries)) { + if ( + !state.documents[id]?.base && + state.fileManifest.entries[id] === undefined + ) { + delete entries[id]; + } + } + return resolvePaths( + entries, + state.fileManifest.entries, + Object.keys(state.excluded ?? {}), + [ + ...unsyncablePaths, + ...Object.keys(state.excluded ?? {}).flatMap((id) => + state.local[id] ? [state.local[id]] : [] + ) + ] + ); +} diff --git a/frontend/sync-client/src/sync-operations/local-changes.ts b/frontend/sync-client/src/sync-operations/local-changes.ts index 58fdf39e..d7261eaf 100644 --- a/frontend/sync-client/src/sync-operations/local-changes.ts +++ b/frontend/sync-client/src/sync-operations/local-changes.ts @@ -1,24 +1,23 @@ import { v4 as uuid } from "uuid"; -import type { EngineState } from "../persistence/database"; +import type { DocumentId, EngineState, 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"; export type LocalChange = { - changeId?: string; + changeId: string; /** Identities at notification time, before a remote application can reuse paths. */ - identities?: Record; + identities?: FileManifestEntries; } & ( - | { type: "move"; oldPath: string; relativePath: string } - | { type: "create"; path: string; documentId?: string } - | { type: "delete"; path: string } + | { type: "move"; oldPath: RelativePath; relativePath: RelativePath } + | { type: "create"; path: RelativePath; documentId?: DocumentId } + | { type: "delete"; path: RelativePath } ); -export function unappliedChanges( +export function unappliedChanges( state: EngineState, - changes: readonly T[] -): T[] { - for (const change of changes) change.changeId ??= uuid(); - + changes: readonly LocalChange[] +): LocalChange[] { const applied = changes.findIndex( (change) => change.changeId === state.lastAppliedLocalChangeId ); @@ -29,7 +28,7 @@ export function unappliedChanges( export function applyLocalChange( next: EngineState, change: LocalChange, - ignored: (path: string) => boolean, + ignored: (path: RelativePath) => boolean, initial?: VaultSnapshot ): void { if (change.type === "create") { 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 5ccfa7a8..f67f570f 100644 --- a/frontend/sync-client/src/sync-operations/outgoing-cursor.test.ts +++ b/frontend/sync-client/src/sync-operations/outgoing-cursor.test.ts @@ -1,10 +1,14 @@ import assert from "node:assert/strict"; import { test } from "node:test"; import { CursorTracker } from "./cursor-tracker"; -import { FileChangeNotifier } from "./file-change-notifier"; import { EventListeners } from "../utils/data-structures/event-listeners"; import { hash } from "../utils/hash"; -import type { Database, DocumentRecord } from "../persistence/database"; +import type { + Database, + DocumentId, + DocumentRecord, + RelativePath +} from "../persistence/database"; import type { FileOperations } from "../file-operations/file-operations"; import type { WebSocketManager } from "../services/websocket-manager"; import type { DocumentWithCursors } from "../services/types/DocumentWithCursors"; @@ -40,14 +44,14 @@ for (const change of ["version", "identity", "path", "delete"] as const) }; const tracker = new CursorTracker( { - getDocumentByDocumentId: (id: string) => + getDocumentByDocumentId: (id: DocumentId) => current?.documentId === id ? current : undefined, - getLatestDocumentByRelativePath: (path: string) => + getLatestDocumentByRelativePath: (path: RelativePath) => current?.relativePath === path ? current : undefined } as unknown as Database, websocket as unknown as WebSocketManager, files as unknown as FileOperations, - new FileChangeNotifier() + new EventListeners<(path: RelativePath) => unknown>() ); await tracker.sendLocalCursorsToServer({ "a.md": [{ start: 4, end: 4 }] diff --git a/frontend/sync-client/src/sync-operations/scan.ts b/frontend/sync-client/src/sync-operations/scan.ts index 0ded18ed..0e183951 100644 --- a/frontend/sync-client/src/sync-operations/scan.ts +++ b/frontend/sync-client/src/sync-operations/scan.ts @@ -1,6 +1,6 @@ import { v4 as uuid } from "uuid"; import type { FileOperations } from "../file-operations/file-operations"; -import type { EngineState } from "../persistence/database"; +import type { DocumentId, EngineState, RelativePath } from "../persistence/database"; import type { VaultSnapshot } from "../services/types/VaultSnapshot"; import { EMPTY_HASH, hash } from "../utils/hash"; import { @@ -20,7 +20,7 @@ interface ScanOptions { guard: () => void; changes: LocalChange[]; files: FileOperations; - ignored: (path: string) => boolean; + ignored: (path: RelativePath) => boolean; oversized: (size: number) => boolean; commit: (next: EngineState) => Promise; } @@ -37,16 +37,16 @@ export async function scanLocalFiles( commit }: ScanOptions, initial?: VaultSnapshot -): Promise> { +): Promise> { // 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(); + const unsyncablePaths = new Set(); for (const change of unappliedChanges(next, changes)) { applyLocalChange(next, change, ignored, initial); next.lastAppliedLocalChangeId = change.changeId; } - const paths: string[] = []; + const paths: RelativePath[] = []; const reportedPaths = await files.listFilesRecursively(); for (const path of reportedPaths) { // Some filesystems may report a different Unicode spelling after a @@ -61,7 +61,7 @@ export async function scanLocalFiles( ); } const present = new Set(paths); - const snapshots = new Map(); + const snapshots = new Map(); for (const path of paths) { if (ignored(path)) { unsyncablePaths.add(path); @@ -102,7 +102,7 @@ export async function scanLocalFiles( const sameHash = unknown.filter( (other) => snapshots.get(other)?.hash === snapshot.hash ); - let id: string | undefined; + let id: DocumentId | undefined; if ( snapshot.hash !== EMPTY_HASH && matches.length === 1 && diff --git a/frontend/sync-client/src/sync-operations/syncer.test.ts b/frontend/sync-client/src/sync-operations/syncer.test.ts index e5a94e8e..bd7afcad 100644 --- a/frontend/sync-client/src/sync-operations/syncer.test.ts +++ b/frontend/sync-client/src/sync-operations/syncer.test.ts @@ -1,7 +1,12 @@ /* 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 } from "../persistence/database"; +import type { + Database, + DocumentId, + RelativePath, + VaultUpdateId +} from "../persistence/database"; import { DEFAULT_SETTINGS, Settings, @@ -16,7 +21,7 @@ import type { SyncHistory } from "../tracing/sync-history"; import { EventListeners } from "../utils/data-structures/event-listeners"; import { Syncer } from "./syncer"; import { FixedSizeDocumentCache } from "../utils/data-structures/fix-sized-cache"; -import { toStoredSnapshot } from "../snapshot"; +import { hashSnapshot } from "../snapshot"; import { undiff } from "reconcile-text"; function createSyncer(overrides: Partial = {}): { @@ -38,7 +43,6 @@ function createSyncer(overrides: Partial = {}): { onRemoteVaultUpdateReceived: new EventListeners<() => unknown>() } as unknown as WebSocketManager; const syncer = new Syncer( - "device", {} as Logger, database, settings, @@ -146,7 +150,7 @@ describe("Syncer per-file state and event delivery", () => { }, lastSeenUpdateId: 0 }, - getLatestDocumentByRelativePath: (path: string) => + getLatestDocumentByRelativePath: (path: RelativePath) => path === "note.md" ? { documentId: "document", @@ -164,11 +168,10 @@ describe("Syncer per-file state and event delivery", () => { } as unknown as WebSocketManager; const service = { events: async () => { - return { headEventId: 0, events: [] }; + return { headEventId: 0, endEventId: 0, events: [] }; } } as unknown as SyncService; const syncer = new Syncer( - "device", {} as Logger, database, new Settings( @@ -198,7 +201,7 @@ describe("Syncer per-file state and event delivery", () => { const internals = syncer as unknown as { dirty: boolean; - unsyncablePaths: Set; + unsyncablePaths: Set; }; internals.dirty = true; assert.strictEqual(syncer.isDocumentUpToDate("note.md"), true); @@ -234,7 +237,6 @@ describe("Syncer diff uploads", () => { onRemoteVaultUpdateReceived: new EventListeners<() => unknown>() } as unknown as WebSocketManager; const syncer = new Syncer( - "device", {} as Logger, { state: {} } as Database, new Settings( @@ -261,15 +263,15 @@ describe("Syncer diff uploads", () => { const internals = syncer as unknown as { remoteSnapshot: ( head: { - vaultUpdateId: number; - documentId: string; + vaultUpdateId: VaultUpdateId; + documentId: DocumentId; }, - path: string + path: RelativePath ) => Promise; pushContent: ( - path: string, - parentVersionId: number, - snapshot: Awaited> + path: RelativePath, + parentVersionId: VaultUpdateId, + snapshot: Awaited> ) => Promise< | { type: "Diff"; value: (number | string)[] } | { type: "Snapshot"; value: string } @@ -283,7 +285,7 @@ describe("Syncer diff uploads", () => { const payload = await internals.pushContent( "note.md", 7, - await toStoredSnapshot({ content: encoder.encode(changed) }) + await hashSnapshot({ content: encoder.encode(changed) }) ); assert.equal(payload.type, "Diff"); @@ -300,15 +302,15 @@ describe("Syncer diff uploads", () => { const internals = syncer as unknown as { remoteSnapshot: ( head: { - vaultUpdateId: number; - documentId: string; + vaultUpdateId: VaultUpdateId; + documentId: DocumentId; }, - path: string + path: RelativePath ) => Promise; pushContent: ( - path: string, - parentVersionId: number, - snapshot: Awaited> + path: RelativePath, + parentVersionId: VaultUpdateId, + snapshot: Awaited> ) => Promise<{ type: string; value: unknown }>; }; await internals.remoteSnapshot( @@ -318,7 +320,7 @@ describe("Syncer diff uploads", () => { const payload = await internals.pushContent( "note.md", 7, - await toStoredSnapshot({ content: encoder.encode("changed") }) + await hashSnapshot({ content: encoder.encode("changed") }) ); assert.equal(payload.type, "Snapshot"); diff --git a/frontend/sync-client/src/sync-operations/syncer.ts b/frontend/sync-client/src/sync-operations/syncer.ts index 9c42a6dc..04e4f794 100644 --- a/frontend/sync-client/src/sync-operations/syncer.ts +++ b/frontend/sync-client/src/sync-operations/syncer.ts @@ -1,18 +1,19 @@ -import { requestReceipt } from "../persistence/requests"; import type { SyncHistory } from "../tracing/sync-history"; import { SyncStatus, SyncType } from "../tracing/sync-history"; import { v4 as uuid } from "uuid"; -import { base64ToBytes } from "byte-base64"; +import { base64ToBytes, bytesToBase64 } from "byte-base64"; import { diff, undiff } from "reconcile-text"; import type { Database, EngineState, PendingRequest, - UnconfirmedRequest, - ContentHead + ContentHead, + DocumentId, + RelativePath, + VaultUpdateId } from "../persistence/database"; import { emptyState } from "../persistence/database"; -import { toStoredSnapshot, type StoredSnapshot } from "../snapshot"; +import { hashSnapshot, type HashedSnapshot } from "../snapshot"; import type { FileOperations, FileWrite @@ -23,22 +24,21 @@ 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 { EventRecord } from "../services/types/EventRecord"; +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 { VaultHistoryStatus } from "../types/vault-history-status"; import { mergeContent } from "./content"; import { scanLocalFiles, type LocalChange } from "./scan"; import { - mergeFileManifests, + reconcileFileManifest, + manifestToSubmit, resolvePaths, sameFileManifest, validateFileManifest } from "./file-manifest"; -import { removeFromArray } from "../utils/remove-from-array"; import { isInternalPath } from "../utils/portable-path"; import { isBinary } from "../utils/is-binary"; import { isFileTypeMergeable } from "../utils/is-file-type-mergeable"; @@ -68,12 +68,11 @@ export class Syncer { private readonly changes: LocalChange[]; private notificationSaving: Promise = Promise.resolve(); private notificationSaveFailed = false; - private readonly pendingPaths = new Set(); - private unsyncablePaths = new Set(); + private readonly pendingPaths = new Set(); + private unsyncablePaths = new Set(); private hasScanned = false; public constructor( - deviceId: string, private readonly logger: Logger, private readonly database: Database, private readonly settings: Settings, @@ -89,16 +88,9 @@ export class Syncer { } = { entries: [], save: async () => {} } ) { this.changes = localChangeStorage.entries; - websocket.onWebSocketStatusChanged.add((connected) => { - if (connected) { - websocket.sendHandshakeMessage({ - type: "handshake", - token: settings.getSettings().token, - deviceId - }); - } - // Failed upgrades and lost transports still need HTTP catchup, - // including when the optional polling interval is disabled. + // Failed upgrades and lost transports still need HTTP catchup, + // including when the optional polling interval is disabled. + websocket.onWebSocketStatusChanged.add(() => { this.wake(); }); websocket.onRemoteVaultUpdateReceived.add(async () => { @@ -119,12 +111,11 @@ export class Syncer { !!this.running || this.dirty || !!this.database.state.pending || - !!this.failure || - !!this.rejectionMessage + !!this.failure ); } - public isDocumentUpToDate(path: string): boolean { + public isDocumentUpToDate(path: RelativePath): boolean { if ( this.database.state.lastSeenUpdateId === undefined || !this.hasScanned || @@ -198,11 +189,7 @@ export class Syncer { this.running = this.run().finally(() => { this.running = undefined; this.onRemainingOperationsCountChanged.trigger( - this.failure || - this.database.state.pending || - this.rejectionMessage - ? 1 - : 0 + this.failure || this.database.state.pending ? 1 : 0 ); if (this.stopped) { @@ -243,28 +230,36 @@ export class Syncer { if (this.failure) { throw this.failure; } - if (this.rejectionMessage) - throw new PermanentSyncError(this.rejectionMessage); + const rejection = this.database.state.pending?.rejection; + if (rejection !== undefined) throw new PermanentSyncError(rejection); } - public async syncLocallyCreatedFile(_path: string): Promise { + public async syncLocallyCreatedFile(_path: RelativePath): Promise { this.localGeneration++; this.pendingPaths.add(_path); - await this.recordLocalChange({ type: "create", path: _path }); + await this.recordLocalChange({ + type: "create", + path: _path, + changeId: uuid() + }); } - public async syncLocallyDeletedFile(_path: string): Promise { + public async syncLocallyDeletedFile(_path: RelativePath): Promise { this.localGeneration++; this.pendingPaths.add(_path); - await this.recordLocalChange({ type: "delete", path: _path }); + await this.recordLocalChange({ + type: "delete", + path: _path, + changeId: uuid() + }); } public async syncLocallyUpdatedFile({ oldPath, relativePath }: { - oldPath?: string; - relativePath: string; + oldPath?: RelativePath; + relativePath: RelativePath; }): Promise { // Content snapshots are re-read before writing. Only namespace // notifications invalidate an identity plan; keystrokes must not starve @@ -280,7 +275,8 @@ export class Syncer { await this.recordLocalChange({ type: "move", oldPath, - relativePath + relativePath, + changeId: uuid() }); } this.wake(); @@ -293,7 +289,6 @@ export class Syncer { } private async recordLocalChange(change: LocalChange): Promise { - change.changeId = uuid(); const previous = [...this.changes]; this.changes.push(change); const saving = this.notificationSaving.then(async () => { @@ -324,11 +319,9 @@ export class Syncer { this.failure = undefined; try { await this.reloadLocalState(); - if ( - this.isFirstSyncComplete && - this.service.vaultHistoryStatus === VaultHistoryStatus.MISSING - ) - await this.recoverServerHistory(); + const rejection = this.database.state.pending?.rejection; + if (rejection !== undefined) + throw new PermanentSyncError(rejection); if (!this.isFirstSyncComplete) await this.service.resetHistory(); await this.serverConfig.initialize(); if (!this.isFirstSyncComplete) await this.bootstrap(); @@ -424,8 +417,8 @@ export class Syncer { } private async cacheContent( - updateId: number, - path: string | undefined, + updateId: VaultUpdateId, + path: RelativePath | undefined, content: Uint8Array ): Promise { if ( @@ -441,11 +434,11 @@ export class Syncer { } private async pushContent( - path: string, - parentVersionId: number | undefined, - snapshot: StoredSnapshot + path: RelativePath, + parentVersionId: VaultUpdateId | undefined, + snapshot: HashedSnapshot ): Promise { - const content = base64ToBytes(snapshot.contentBase64); + const { content } = snapshot; const parent = parentVersionId === undefined ? undefined @@ -469,7 +462,7 @@ export class Syncer { if (value.length <= 10_000) return { type: "Diff", value }; } - return { type: "Snapshot", value: snapshot.contentBase64 }; + return { type: "Snapshot", value: bytesToBase64(content) }; } private next(): EngineState { @@ -507,10 +500,9 @@ export class Syncer { private async incorporateEventBatch(batch: EventBatch): Promise { let cursor = this.database.state.lastSeenUpdateId ?? 0; let manifest = this.database.state.fileManifest; - const heads: Record = {}; + const heads: Record = {}; for (const [id, doc] of Object.entries(this.database.state.documents)) if (doc.remote) heads[id] = doc.remote; - const receipts: EventRecord[] = []; // Fold every page before touching local files. If fetching is interrupted, // discard this work and retry from the last committed cursor. for (;;) { @@ -522,13 +514,6 @@ export class Syncer { "Non-contiguous event history; refusing to skip changes" ); cursor = event.eventId; - if ( - this.database.state.unconfirmed?.some( - (pending) => - pending.requestId === event.requestId - ) === true - ) - receipts.push(event); if (event.type === "fileManifest") { if ( event.fileManifest.fileManifestId > @@ -545,7 +530,7 @@ export class Syncer { ); } } - const end = batch.endEventId ?? batch.headEventId; + const end = batch.endEventId; if ( end !== cursor || end > batch.headEventId || @@ -555,51 +540,12 @@ export class Syncer { if (end === batch.headEventId) break; batch = await this.service.getEvents(cursor); } - // Recover our submitted bases before folding later remote edits. A - // permanent retry failure cannot cancel an earlier in-flight attempt. - const next = this.next(); - let acknowledged = false; - for (const event of receipts) { - const pending = next.unconfirmed?.find( - (candidate) => candidate.requestId === event.requestId - ); - if (!pending) continue; - if (event.type === "content" && pending.type === "content") { - const doc = next.documents[pending.documentId]; - if ( - !doc.base || - (doc.base.vaultUpdateId ?? 0) < event.eventId - ) { - doc.base = { - vaultUpdateId: event.eventId, - hash: pending.hash - }; - this.clearRejections(next, pending.documentId); - } - } else if ( - event.type === "fileManifest" && - pending.type === "fileManifest" - ) { - if (next.fileManifest.fileManifestId < event.eventId) { - next.fileManifest = event.fileManifest; - this.clearRejections(next, "manifest"); - } - } else - throw new PermanentSyncError( - "Request receipt type does not match submission" - ); - if (next.unconfirmed) removeFromArray(next.unconfirmed, pending); - acknowledged = true; - } - if (acknowledged) await this.database.commit(next); // 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 { + private contentHead(version: ContentHead): ContentHead { return { vaultUpdateId: version.vaultUpdateId, contentSize: version.contentSize @@ -607,9 +553,9 @@ export class Syncer { } private recordManifestApplication( - before: Readonly>, - after: Readonly>, - writes: Readonly>, + before: Readonly, + after: Readonly, + writes: Readonly>, message: string ): void { for (const id of new Set([ @@ -646,28 +592,46 @@ export class Syncer { DocumentVersionWithoutContent, "documentId" | "vaultUpdateId" >, - path: string - ): Promise { + path: RelativePath + ): Promise { const content = await this.service.getDocumentVersionContent(head); await this.cacheContent(head.vaultUpdateId, path, content); - return toStoredSnapshot({ content }); + return hashSnapshot({ content }); } private async contentWrite( next: EngineState, - id: string, - head: ContentHead, - path: string + 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) || - this.settings.isOversized(head.contentSize) - ) { - return undefined; - } - const currentPath = this.database.state.local[id]; + (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" @@ -675,7 +639,7 @@ export class Syncer { : undefined; if ( local !== undefined && - this.settings.isOversized(base64ToBytes(local.contentBase64).length) + this.settings.isOversized(local.content.length) ) { return undefined; } @@ -699,7 +663,10 @@ export class Syncer { : 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); + const remote = await this.remoteSnapshot( + { documentId: id, vaultUpdateId: head.vaultUpdateId }, + path + ); let result = remote; if (local !== undefined) { result = await mergeContent( @@ -716,7 +683,7 @@ export class Syncer { }; delete document.remote; document.observedHash = result.hash; - return local?.contentBase64 === result.contentBase64 + return local?.hash === result.hash ? undefined : { expected: local, replacement: result }; } @@ -729,10 +696,10 @@ export class Syncer { clearPending = false, heads }: { - cursor?: number; + cursor?: VaultUpdateId; initial?: VaultSnapshot; clearPending?: boolean; - heads?: Record; + heads?: Record; } = {} ): Promise { validateFileManifest(remote.entries); @@ -751,91 +718,35 @@ export class Syncer { return; } - const base = { ...next.fileManifest.entries }; - for (const [id, path] of Object.entries(next.excluded ?? {})) { - if (path === null) delete base[id]; - else base[id] = path; - } - const previousManifest = next.fileManifest.entries; - next.local = mergeFileManifests(base, next.local, remote.entries); - next.fileManifest = remote; + reconcileFileManifest( + next, + remote, + (path) => this.settings.isIgnored(path), + this.unsyncablePaths + ); for (const [id, head] of Object.entries(heads ?? {})) (next.documents[id] ??= {}).remote = head; - for (const [id, oldPath] of Object.entries(before)) { - const path = next.local[id]; - if ( - this.settings.isIgnored(oldPath) || - this.unsyncablePaths.has(oldPath) || - (path !== undefined && this.settings.isIgnored(path)) - ) { - next.excluded ??= {}; - if (next.excluded[id] === undefined) - next.excluded[id] = previousManifest[id] ?? null; - next.local[id] = oldPath; - } else if (next.excluded) delete next.excluded[id]; - } - for (const [id, path] of Object.entries(next.excluded ?? {})) { - if ( - before[id] === undefined && - (path === null || !this.settings.isIgnored(path)) - ) - delete next.excluded![id]; - } - next.local = resolvePaths( - next.local, - remote.entries, - Object.keys(next.excluded ?? {}), - [...this.unsyncablePaths] - ); if (initial) { for (const head of initial.documents) (next.documents[head.documentId] ??= {}).remote = this.contentHead(head); } - const writes: Record = {}; + const writes: Record = {}; for (const [id, path] of Object.entries(next.local)) { - const doc = (next.documents[id] ??= {}); - if (before[id] === undefined) delete doc.observedHash; - if ( - doc.observedHash !== undefined && - doc.remote && - doc.remote.vaultUpdateId <= (doc.base?.vaultUpdateId ?? 0) - ) - delete doc.remote; - if ( - next.excluded?.[id] === undefined && - !this.settings.isIgnored(path) && - (remote.entries[id] !== undefined || - doc.remote !== undefined) && - (!this.database.state.local[id] || - !doc.base || - doc.observedHash === undefined || - (doc.remote?.vaultUpdateId ?? 0) > - (doc.base.vaultUpdateId ?? 0)) - ) { - const knownHead = doc.remote; - const head = - knownHead ?? (await this.service.getDocumentMetadata(id)); - if (!head) { - throw new PermanentSyncError( - `File manifest references missing content: ${id}` - ); - } - doc.remote = this.contentHead(head); - const write = await this.contentWrite(next, id, head, path); - if (write) { - writes[id] = write; - } - } + const write = await this.contentWrite(next, id, path); + if (write) writes[id] = write; } await this.files.apply(next, writes, guard); - this.recordManifestApplication(before, this.database.state.local, writes, "Remote change applied"); + this.recordManifestApplication( + before, + this.database.state.local, + writes, + "Remote change applied" + ); } private async preparePush(): Promise { - if (this.stopped) { - return false; - } + if (this.stopped || this.database.state.pending) return false; let guard = this.localGuard(); const canonical = resolvePaths( @@ -856,7 +767,10 @@ export class Syncer { // Resolve the map after restoring excluded server paths, then make // those allocations visible locally before submitting a manifest. - const submitted = this.manifestToSubmit(); + const submitted = manifestToSubmit( + 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) @@ -882,24 +796,12 @@ export class Syncer { const snapshot = await this.files.snapshot(path); if ( !snapshot || - this.settings.isOversized( - base64ToBytes(snapshot.contentBase64).length - ) + this.settings.isOversized(snapshot.content.length) ) { continue; } const { base } = this.database.state.documents[id]; - if ( - base?.hash === snapshot.hash || - this.rejections.some( - (request) => - request.type === "content" && - request.documentId === id && - request.hash === snapshot.hash - ) - ) { - continue; - } + if (base?.hash === snapshot.hash) continue; guard(); await this.database.commit({ ...this.next(), @@ -921,18 +823,13 @@ export class Syncer { return true; } - const entries = this.manifestToSubmit(); + const entries = manifestToSubmit( + this.database.state, + this.unsyncablePaths + ); validateFileManifest(entries); if ( - !sameFileManifest( - entries, - this.database.state.fileManifest.entries - ) && - !this.rejections.some( - (request) => - request.type === "fileManifest" && - sameFileManifest(entries, request.entries) - ) + !sameFileManifest(entries, this.database.state.fileManifest.entries) ) { guard(); await this.database.commit({ @@ -952,115 +849,46 @@ export class Syncer { return false; } - private manifestToSubmit(): Record { - // Do not publish a new document until its initial content is acknowledged. - const entries = { ...this.database.state.local }; - for (const id of Object.keys(this.database.state.excluded ?? {})) { - const remotePath = this.database.state.fileManifest.entries[id]; - if (remotePath === undefined) delete entries[id]; - else entries[id] = remotePath; - } - for (const id of Object.keys(entries)) { - if ( - !this.database.state.documents[id]?.base && - this.database.state.fileManifest.entries[id] === undefined - ) { - delete entries[id]; - } - } - return resolvePaths( - entries, - this.database.state.fileManifest.entries, - Object.keys(this.database.state.excluded ?? {}), - [ - ...this.unsyncablePaths, - ...Object.keys(this.database.state.excluded ?? {}).flatMap( - (id) => - this.database.state.local[id] - ? [this.database.state.local[id]] - : [] - ) - ] - ); - } - - private get rejections(): UnconfirmedRequest[] { - return (this.database.state.unconfirmed ?? []).filter( - (request) => request.rejection !== undefined - ); - } - - private get rejectionMessage(): string | undefined { - const { state } = this.database; - for (const request of this.rejections) { - if (request.type === "content") { - const id = request.documentId; - const path = state.local[id]; - const doc = state.documents[id]; - if ( - path !== undefined && - doc?.observedHash === request.hash && - doc.base?.hash !== request.hash && - !this.settings.isIgnored(path) && - state.excluded?.[id] === undefined - ) - return request.rejection; - } else if ( - sameFileManifest( - this.manifestToSubmit(), - request.entries - ) && - !sameFileManifest( - state.fileManifest.entries, - request.entries - ) - ) - return request.rejection; - } - } - - private clearRejections(state: EngineState, id?: string): void { - for (const request of state.unconfirmed ?? []) - if ( - id === undefined || - (request.type === "content" - ? request.documentId - : "manifest") === id - ) - delete request.rejection; - } - - /** An explicit reset or settings change permits fresh attempts after rejection. */ + /** An explicit reset or settings change retries the exact rejected request. */ public async retryRejectedRequests(): Promise { await this.database.recoverPersistence(); - if (!this.rejections.length) return; const next = this.next(); - this.clearRejections(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 { + 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; - const next = this.next(); - (next.unconfirmed ??= []).push(requestReceipt(pending, error.message)); - delete next.pending; - await this.database.commit(next); + // 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" - ? (next.local[pending.documentId] ?? pending.documentId) - : "File manifest" + relativePath: + pending.type === "content" + ? (this.database.state.local[pending.documentId] ?? + pending.documentId) + : "File manifest" } }); - return; + throw error; } - } private async finishPending(): Promise { @@ -1071,13 +899,12 @@ export class Syncer { // 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) return; + this.service.pushFileManifest(pending.request) + ); if (response.type === "StaleBase") { await this.reconcile(response, { clearPending: true }); } else { const next = this.next(); - this.clearRejections(next, "manifest"); next.fileManifest = { fileManifestId: response.fileManifestId, entries: pending.request.entries @@ -1085,12 +912,17 @@ export class Syncer { 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"); + 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) return; + this.service.putFileContent(pending.documentId, pending.request) + ); if (response.type === "StaleBase") { await this.reconcile(this.database.state.fileManifest, { clearPending: true, @@ -1099,8 +931,10 @@ export class Syncer { } else { const next = this.next(); const doc = (next.documents[pending.documentId] ??= {}); - doc.base = { vaultUpdateId: response.vaultUpdateId, hash: pending.hash }; - this.clearRejections(next, pending.documentId); + doc.base = { + vaultUpdateId: response.vaultUpdateId, + hash: pending.hash + }; delete doc.remote; delete next.pending; await this.database.commit(next); @@ -1110,7 +944,8 @@ export class Syncer { message: "Content committed", details: { type: SyncType.UPDATE, - relativePath: next.local[pending.documentId] ?? pending.documentId + relativePath: + next.local[pending.documentId] ?? pending.documentId }, author: response.userId, timestamp: new Date(response.updatedDate) @@ -1120,17 +955,30 @@ export class Syncer { } /** Warm the optional diff cache using the wire payload, never another saved copy. */ - private async cacheSubmission(pending: Extract, version: number): Promise { + 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); + 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 = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }).decode(parent); + const text = new TextDecoder("utf-8", { + fatal: true, + ignoreBOM: true + }).decode(parent); bytes = new TextEncoder().encode(undiff(text, content.value)); } - await this.cacheContent(version, this.database.state.local[pending.documentId], bytes); + await this.cacheContent( + version, + this.database.state.local[pending.documentId], + bytes + ); } } diff --git a/frontend/sync-client/src/types/file-manifest-entries.ts b/frontend/sync-client/src/types/file-manifest-entries.ts index 1e23dd06..c9acccf9 100644 --- a/frontend/sync-client/src/types/file-manifest-entries.ts +++ b/frontend/sync-client/src/types/file-manifest-entries.ts @@ -1,3 +1,3 @@ -import type { FileManifest } from "../services/types/FileManifest"; +import type { DocumentId, RelativePath } from "../persistence/database"; -export type FileManifestEntries = FileManifest["entries"]; +export type FileManifestEntries = Record; diff --git a/frontend/sync-client/src/types/vault-history-status.ts b/frontend/sync-client/src/types/vault-history-status.ts deleted file mode 100644 index 74478822..00000000 --- a/frontend/sync-client/src/types/vault-history-status.ts +++ /dev/null @@ -1,4 +0,0 @@ -export enum VaultHistoryStatus { - MISSING = "missing", - STORED = "stored" -} \ No newline at end of file diff --git a/frontend/sync-client/src/utils/create-promise.ts b/frontend/sync-client/src/utils/create-promise.ts deleted file mode 100644 index a49196ee..00000000 --- a/frontend/sync-client/src/utils/create-promise.ts +++ /dev/null @@ -1,25 +0,0 @@ -type ResolveFunction = undefined extends T - ? (value?: T) => unknown - : (value: T) => unknown; - -/** - * A type-safe utility function to create a Promise with resolve and reject functions. - * @returns A tuple containing a Promise, a resolve function, and a reject function. - */ -export function createPromise(): [ - Promise, - ResolveFunction, - (error: unknown) => unknown -] { - let resolve: undefined | ResolveFunction = undefined; - let reject: undefined | ((error: unknown) => unknown) = undefined; - - const creationPromise = new Promise( - (resolve_, reject_) => - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - ((resolve = resolve_ as ResolveFunction), (reject = reject_)) - ); - - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return [creationPromise, resolve!, reject!]; -} 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 47c850f9..bbf497b2 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 @@ -156,6 +156,37 @@ describe("EventListeners", () => { assert.strictEqual(results.length, 3); }); + it("awaits every async listener even when another listener throws synchronously", async () => { + const listeners = new EventListeners<() => unknown>(); + const release = Promise.withResolvers(); + const error = new Error("observer failed"); + let completed = false; + let laterListenerCalled = false; + listeners.add(async () => { + await release.promise; + completed = true; + }); + listeners.add(() => { + throw error; + }); + listeners.add(() => { + laterListenerCalled = true; + }); + + let settled = false; + const result = assert + .rejects(listeners.triggerAsync(), error) + .then(() => { + settled = true; + }); + await Promise.resolve(); + assert(laterListenerCalled); + assert(!settled); + release.resolve(); + await result; + assert(completed); + }); + it("should not trigger cleared listeners", () => { const listeners = new EventListeners<() => void>(); let called = false; diff --git a/frontend/sync-client/src/utils/data-structures/event-listeners.ts b/frontend/sync-client/src/utils/data-structures/event-listeners.ts index 42a13eac..b1fd7219 100644 --- a/frontend/sync-client/src/utils/data-structures/event-listeners.ts +++ b/frontend/sync-client/src/utils/data-structures/event-listeners.ts @@ -12,7 +12,7 @@ export class EventListeners any> { private readonly onError: (error: unknown) => void = (error) => { console.error("Event observer failed", error); } - ) { } + ) {} public get count(): number { return this.listeners.length; @@ -69,14 +69,9 @@ export class EventListeners any> { */ public async triggerAsync(...args: Parameters): Promise { await awaitAll( - this.listeners - .map((listener) => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - return listener(...args); - }) - .filter((result): result is Promise => { - return result instanceof Promise; - }) + [...this.listeners].map(async (listener) => { + await listener(...args); + }) ); } diff --git a/frontend/sync-client/src/utils/portable-path.ts b/frontend/sync-client/src/utils/portable-path.ts index f794cae7..fc86e5ec 100644 --- a/frontend/sync-client/src/utils/portable-path.ts +++ b/frontend/sync-client/src/utils/portable-path.ts @@ -1,3 +1,6 @@ +import type { DocumentId, RelativePath } from "../persistence/database"; +import type { FileManifestEntries } from "../types/file-manifest-entries"; + export const INTERNAL_DIRECTORY = ".vault-link-sync"; const MAX_COMPONENT_BYTES = 255; const byteLength = (value: string): number => @@ -15,16 +18,16 @@ 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: string): string => +const pathKey = (path: RelativePath): string => path.normalize("NFC").toUpperCase().normalize("NFC"); -export const arePathAliases = (left: string, right: string): boolean => +export const arePathAliases = (left: RelativePath, right: RelativePath): boolean => pathKey(left) === pathKey(right); export function findPathWithSameSpelling( - paths: Iterable, - wanted: string -): string | undefined { + paths: Iterable, + wanted: RelativePath +): RelativePath | undefined { const normalized = wanted.normalize("NFC"); for (const path of paths) { if (path !== wanted && path.normalize("NFC") === normalized) @@ -33,10 +36,10 @@ export function findPathWithSameSpelling( return undefined; } -export const isInternalPath = (path: string): boolean => +export const isInternalPath = (path: RelativePath): boolean => arePathAliases(path.split("/")[0], INTERNAL_DIRECTORY); -function validatePath(path: string): void { +function validatePath(path: RelativePath): void { if (!path || path !== path.normalize("NFC") || isInternalPath(path)) throw new Error(`Invalid portable path: ${path}`); @@ -54,7 +57,7 @@ function validatePath(path: string): void { } } -export function validatePortablePaths(paths: Iterable): void { +export function validatePortablePaths(paths: Iterable): void { // Component keys keep storage proportional to the input, even when a // manifest contains thousands of directory levels. const nodes = new Map< @@ -107,7 +110,7 @@ function fitName(name: string, suffix: string, file: boolean): string { ); } -function pathsConflict(left: string, right: string): boolean { +function pathsConflict(left: RelativePath, right: RelativePath): boolean { const a = left.split("/"), b = right.split("/"); for (let i = 0; i < Math.min(a.length, b.length); i++) { @@ -118,9 +121,9 @@ function pathsConflict(left: string, right: string): boolean { } function isAvailablePath( - candidate: string, - id: string, - occupied: Readonly> + candidate: RelativePath, + id: DocumentId, + occupied: Readonly ): boolean { validatePath(candidate); return Object.entries(occupied).every( @@ -130,10 +133,10 @@ function isAvailablePath( /** Deterministic allocation also handles a file blocking an ancestor directory. */ export function allocatePortablePath( - wanted: string, - id: string, - occupied: Readonly> -): string { + wanted: RelativePath, + id: DocumentId, + occupied: Readonly +): RelativePath { id = portableName(truncate(id, 64)); const originals = wanted.split("/"); const parts = originals.map((part, index) => { diff --git a/frontend/sync-client/src/utils/rate-limit.ts b/frontend/sync-client/src/utils/rate-limit.ts index 52cbbce7..03232def 100644 --- a/frontend/sync-client/src/utils/rate-limit.ts +++ b/frontend/sync-client/src/utils/rate-limit.ts @@ -1,4 +1,3 @@ -import { createPromise } from "./create-promise"; import { sleep } from "./sleep"; /** @@ -45,17 +44,11 @@ export function rateLimit< newArgs = undefined; } - const [promise, resolve] = createPromise(); - running = promise; - sleep( + running = sleep( typeof minIntervalMs === "function" ? minIntervalMs() : minIntervalMs - ) - .then(resolve) - .catch(() => { - // sleep cannot fail - }); + ); return fn(...args); }; diff --git a/frontend/test-support/canonical.ts b/frontend/test-support/canonical.ts index 4fb1c751..8eb7a3f1 100644 --- a/frontend/test-support/canonical.ts +++ b/frontend/test-support/canonical.ts @@ -124,7 +124,7 @@ export async function assertCanonical( do { const batch = await getJson<{ headEventId: number; - endEventId?: number; + endEventId: number; events: { eventId: number; requestId: string }[]; }>(`${vaultUrl}/events-since?after=${after}`, token); assert.equal( @@ -132,7 +132,7 @@ export async function assertCanonical( snapshot.headEventId, "Server changed while checking convergence" ); - const end = batch.endEventId ?? batch.headEventId; + const end = batch.endEventId; assert.equal( end, batch.events.at(-1)?.eventId ?? after, diff --git a/frontend/test-support/content-limits.test.ts b/frontend/test-support/content-limits.test.ts index 09627b58..7b79a3b7 100644 --- a/frontend/test-support/content-limits.test.ts +++ b/frontend/test-support/content-limits.test.ts @@ -75,7 +75,7 @@ test("stale content acknowledgements contain metadata without the excluded paylo test("highly fragmented local edits use a snapshot within the server operation budget", async () => { const { fixture } = await import("./sync-fixture"); - const { toStoredSnapshot } = await import("../sync-client/src/snapshot"); + const { hashSnapshot } = await import("../sync-client/src/snapshot"); const f = await fixture({}); f.contentCache.resize(1024 * 1024); const original = Array.from({ length: 12_000 }, (_, i) => `word${i}`).join( @@ -95,7 +95,7 @@ test("highly fragmented local edits use a snapshot within the server operation b const request = await syncer.pushContent( "a.md", 1, - await toStoredSnapshot({ content: bytes(edited) }) + await hashSnapshot({ content: bytes(edited) }) ); assert.equal(request.type, "Snapshot"); }); diff --git a/frontend/test-support/engine-regressions.test.ts b/frontend/test-support/engine-regressions.test.ts index 48bd3842..8eda52b8 100644 --- a/frontend/test-support/engine-regressions.test.ts +++ b/frontend/test-support/engine-regressions.test.ts @@ -4,7 +4,7 @@ import { test } from "node:test"; import { Database, emptyState } 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 { toStoredSnapshot } from "../sync-client/src/snapshot"; +import { hashSnapshot } from "../sync-client/src/snapshot"; import { allocatePortablePath } from "../sync-client/src/utils/portable-path"; import { MemoryDisk, MemoryPersistence } from "./storage"; @@ -18,7 +18,7 @@ test("offline content notifications need no saves and restart reads the current initial.local = { a: "a.md" }; initial.documents.a = { - observedHash: (await toStoredSnapshot({ content: bytes("original") })) + observedHash: (await hashSnapshot({ content: bytes("original") })) .hash }; const store = new MemoryPersistence({ @@ -67,7 +67,7 @@ test("offline content notifications need no saves and restart reads the current } }); -test("legacy content notifications preserve the acknowledged namespace prefix on restart", async () => { +test("namespace notifications preserve the acknowledged prefix on restart", async () => { const disk = new MemoryDisk(); await disk.userWrite("d.md", bytes("A")); await disk.userWrite("b.md", bytes("B")); @@ -81,10 +81,10 @@ test("legacy content notifications preserve the acknowledged namespace prefix on ]) initial.documents[id] = { - observedHash: (await toStoredSnapshot({ content: bytes(content) })) + observedHash: (await hashSnapshot({ content: bytes(content) })) .hash }; - initial.lastAppliedLocalChangeId = "consumed-update"; + initial.lastAppliedLocalChangeId = "consumed-move"; const saved: StoredClient = { database: initial, settings: { remoteUri: "http://offline.test", vaultName: "test" }, @@ -97,8 +97,6 @@ test("legacy content notifications preserve the acknowledged namespace prefix on identities: { a: "a.md" }, changeId: "consumed-move" }, - { type: "update", path: "c.md", changeId: "consumed-update" }, - { type: "update", path: "b.md", changeId: "new-update" }, { type: "move", oldPath: "c.md", @@ -132,7 +130,7 @@ test("legacy content notifications preserve the acknowledged namespace prefix on 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), { - a: { replacement: await toStoredSnapshot({ content: bytes("remote") }) } + 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")]])); @@ -155,7 +153,7 @@ test("rejected content recovery remains idempotent across a rename and network f const snapshot = (await f.files.snapshot("a.md"))!; f.database.state.documents.a.base = { ...head("a", 1, "base"), - hash: (await toStoredSnapshot({ content: bytes("base") })).hash + hash: (await hashSnapshot({ content: bytes("base") })).hash }; f.database.state.pending = { type: "content", @@ -164,7 +162,7 @@ test("rejected content recovery remains idempotent across a rename and network f request: { requestId: "request", parentVersionId: 1, - content: { type: "Snapshot", value: snapshot.contentBase64 } + content: { type: "Snapshot", value: Buffer.from(snapshot.content).toString("base64") } } }; await f.database.save(); @@ -253,38 +251,55 @@ test("unfinished bootstrap cannot be rebound to another vault", async () => { } }); -test("a rejected upload is retained locally and does not block another file or a corrected snapshot", async () => { +test("a rejected upload pauses all sync across restarts until an explicit retry", async (context) => { let reject = true; - const sent: string[] = []; - const f = await fixture( - { "a.md": "too large", "b.md": "other" }, - { - putFileContent: async (id) => { - sent.push(id); - if (id === "a" && reject) - throw new PermanentSyncError("HTTP 413"); - return { - type: "Accepted", - ...head(id, sent.length + 2, "small") - }; - } + let eventReads = 0; + const sent: { id: string; request: unknown }[] = []; + const service = { + getEvents: async () => { + eventReads++; + return { headEventId: 0, endEventId: 0, events: [] }; + }, + putFileContent: async (id: string, request: unknown) => { + sent.push({ id, request: structuredClone(request) }); + if (reject) throw new PermanentSyncError("HTTP 413"); + return { type: "Accepted" as const, ...head(id, sent.length + 2, "small") }; } - ); - f.internals.stopped = false; - assert(await f.internals.preparePush()); - await f.internals.finishPending(); - assert.equal(f.database.state.pending, undefined); - assert.match(f.database.state.unconfirmed![0].rejection!, /413/); - assert(await f.internals.preparePush()); - await f.internals.finishPending(); - assert.deepEqual(sent, ["a", "b"]); - assert.equal(await f.internals.preparePush(), false); + }; + let f = await fixture({ "a.md": "too large", "b.md": "other" }, service); + context.after(async () => f.syncer.stop()); + for (const doc of Object.values(f.database.state.documents)) + doc.base = { vaultUpdateId: 1, hash: "previous" }; + await f.database.save(); + f.syncer.start(); + await assert.rejects(f.syncer.waitUntilFinished(), /413/); + const pending = structuredClone(f.database.state.pending!); + assert.equal(pending.rejection, "HTTP 413"); + assert.deepEqual(sent.map(({ id }) => id), ["a"]); + const reads = eventReads; await f.disk.userWrite("a.md", bytes("small")); + await f.syncer.syncLocallyUpdatedFile({ relativePath: "a.md" }); + f.syncer.wake(); + await assert.rejects(f.syncer.waitUntilFinished(), /413/); + assert.equal(eventReads, reads); + assert.equal(sent.length, 1); + await f.syncer.stop(); + + f = await fixture({}, service, f); + f.syncer.start(); + await assert.rejects(f.syncer.waitUntilFinished(), /413/); + assert.equal(eventReads, reads); + assert.equal(sent.length, 1); reject = false; - assert(await f.internals.preparePush()); - await f.internals.finishPending(); - assert.deepEqual(sent, ["a", "b", "a"]); - assert.equal(f.database.state.unconfirmed?.find((request) => request.rejection), undefined); + await f.syncer.retryRejectedRequests(); + f.syncer.wake(); + 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"); + assert.notDeepEqual(sent[2].request, sent[0].request); + assert.equal(f.database.state.pending, undefined); + assert.deepEqual((await f.disk.read("a.md"))?.content, bytes("small")); + await f.syncer.stop(); }); test("a transient upload failure retries the exact request and recovers its acknowledgement", async () => { @@ -304,11 +319,10 @@ test("a transient upload failure retries the exact request and recovers its ackn assert.equal(requests.length, 2); assert.deepEqual(requests[1], requests[0]); assert.equal(f.database.state.documents.a.base!.vaultUpdateId, 2); - assert.equal(f.database.state.unconfirmed?.find((request) => request.rejection), undefined); assert.equal(f.database.state.pending, undefined); }); -test("a persisted upload rejection finishes without filesystem writes", async () => { +test("a persisted upload rejection pauses without filesystem writes", async () => { let uploads = 0; const f = await fixture(undefined, { putFileContent: async () => { @@ -322,41 +336,28 @@ test("a persisted upload rejection finishes without filesystem writes", async () if (!label.startsWith("read:")) throw new Error("Unexpected filesystem mutation"); }; - await f.internals.finishPending(); + await assert.rejects(f.internals.finishPending(), /413/); assert.equal(uploads, 1); - assert.equal(f.database.state.pending, undefined); - assert.equal(f.database.state.unconfirmed![0].rejection, "HTTP 413"); + assert.equal(f.database.state.pending?.rejection, "HTTP 413"); assert.deepEqual(f.disk.userFiles(), new Map([["a.md", bytes("local")]])); }); -test("event replay incorporates an earlier accepted upload after a permanent retry rejection", async () => { +test("an explicit retry recovers an earlier accepted upload after a permanent rejection", async () => { + const requests: unknown[] = []; const f = await fixture(undefined, { - putFileContent: async () => { - throw new PermanentSyncError( - "HTTP 413 after proxy reconfiguration" - ); - }, - getDocumentVersionContent: async () => bytes("local") + putFileContent: async (_id, request) => { + requests.push(structuredClone(request)); + if (requests.length === 1) throw new PermanentSyncError("HTTP 413 after proxy reconfiguration"); + return { type: "Accepted", ...head("a", 2, "local") }; + } }); f.internals.stopped = false; await f.internals.preparePush(); - const { pending } = f.database.state; - assert(pending); - const { requestId } = pending.request; - await f.internals.finishPending(); + await assert.rejects(f.internals.finishPending(), /413/); await assert.rejects(f.syncer.waitUntilFinished(), /413/); - f.database.state.lastSeenUpdateId = 1; - await f.internals.incorporateEventBatch({ - headEventId: 2, - events: [ - { - eventId: 2, - requestId, - type: "content", - document: head("a", 2, "local") - } - ] - }); + await f.syncer.retryRejectedRequests(); + await f.internals.finishPending(); + 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); @@ -364,7 +365,7 @@ test("event replay incorporates an earlier accepted upload after a permanent ret assert.deepEqual(f.disk.userFiles(), new Map([["a.md", bytes("local")]])); }); -test("rejected manifests can be replaced after editing the local namespace", async () => { +test("rejected manifests are resolved before publishing later namespace changes", async () => { let reject = true; const f = await fixture( {}, @@ -389,11 +390,14 @@ test("rejected manifests can be replaced after editing the local namespace", asy base: { ...head("a", 1, "x"), hash: "hash" } }; await f.database.save(); - await f.internals.finishPending(); + await assert.rejects(f.internals.finishPending(), /413/); f.internals.stopped = false; assert.equal(await f.internals.preparePush(), false); f.database.state.local = {}; reject = false; + await f.syncer.retryRejectedRequests(); + await f.internals.finishPending(); + assert.deepEqual(f.database.state.fileManifest.entries, { a: "new.md" }); assert(await f.internals.preparePush()); await f.internals.finishPending(); assert.deepEqual(f.database.state.fileManifest.entries, {}); @@ -509,7 +513,7 @@ for (const kind of ["path", "content"] as const) { f.database.state.lastSeenUpdateId = 1; f.database.state.documents.a.base = { ...head("a", 1, "base"), - hash: (await toStoredSnapshot({ content: bytes("base") })).hash + hash: (await hashSnapshot({ content: bytes("base") })).hash }; if (kind === "path") { await f.disk.userRename("a.md", "c.md"); @@ -520,6 +524,7 @@ for (const kind of ["path", "content"] as const) { } await f.internals.incorporateEventBatch({ headEventId: 3, + endEventId: 3, events: [2, 3].map((id) => kind === "path" ? { @@ -674,11 +679,12 @@ test("rejected content is reported to callers and an explicit retry clears the r }); f.internals.stopped = false; await f.internals.preparePush(); - await f.internals.finishPending(); + await assert.rejects(f.internals.finishPending(), /413/); await assert.rejects(f.syncer.waitUntilFinished(), /413/); assert(f.syncer.isBusy); await f.syncer.retryRejectedRequests(); - assert(await f.internals.preparePush()); + assert.equal(f.database.state.pending?.rejection, undefined); + assert(f.database.state.pending, "explicit retry must retain the exact request"); }); test("notification saves cannot roll back an uncertain engine commit, even if the first reload fails", async () => { diff --git a/frontend/test-support/event-delivery.test.ts b/frontend/test-support/event-delivery.test.ts index d01649f2..d237903b 100644 --- a/frontend/test-support/event-delivery.test.ts +++ b/frontend/test-support/event-delivery.test.ts @@ -91,6 +91,7 @@ for (const [count, delivery] of [ fetchedAfter.push(after); return { headEventId: available, + endEventId: available, events: events.filter( (event) => event.eventId > after && @@ -108,7 +109,6 @@ for (const [count, delivery] of [ >() }; const syncer = new Syncer( - "device", logger, database, settings, @@ -122,6 +122,7 @@ for (const [count, delivery] of [ const deliver = async (event: EventRecord) => { await websocket.onRemoteVaultUpdateReceived.triggerAsync({ headEventId: event.eventId, + endEventId: event.eventId, // Payloads are deliberately inconsistent with HTTP. The socket // only wakes the loop; it cannot supply reconciliation state. events: [ diff --git a/frontend/test-support/file-application.test.ts b/frontend/test-support/file-application.test.ts index ce53eba7..76c856a2 100644 --- a/frontend/test-support/file-application.test.ts +++ b/frontend/test-support/file-application.test.ts @@ -1,7 +1,7 @@ import assert from "node:assert/strict"; import { test } from "node:test"; import { fixture, bytes } from "./sync-fixture"; -import { toStoredSnapshot } from "../sync-client/src/snapshot"; +import { hashSnapshot } from "../sync-client/src/snapshot"; import { LocalChangesDuringReconciliation } from "../sync-client/src/errors/errors"; for (const equal of [false, true]) @@ -78,7 +78,7 @@ test("content edited after planning is merged with the incoming snapshot", async await f.files.apply(structuredClone(f.database.state), { a: { expected, - replacement: await toStoredSnapshot({ + replacement: await hashSnapshot({ content: bytes("remote\nbase\n") }) } @@ -95,7 +95,7 @@ test("deleting the source after planning does not resurrect it", async () => { next.local.a = "remote.md"; await f.disk.userDelete("a.md"); await f.files.apply(next, { - a: { replacement: await toStoredSnapshot({ content: bytes("remote") }) } + a: { replacement: await hashSnapshot({ content: bytes("remote") }) } }); assert.deepEqual(f.disk.userFiles(), new Map()); assert.deepEqual(f.database.state.local, {}); @@ -111,7 +111,7 @@ for (const protect of ["ignore", "size"] as const) else await f.settings.setSetting("maxFileSizeMB", 0); await f.files.apply(next, { a: { - replacement: await toStoredSnapshot({ + replacement: await hashSnapshot({ content: bytes("remote") }) } @@ -224,7 +224,7 @@ for (const phase of ["before", "durable"]) const original = structuredClone(f.database.state); const write = { expected: (await f.files.snapshot("a.md"))!, - replacement: await toStoredSnapshot({ + replacement: await hashSnapshot({ content: bytes("remote\nbase\n") }) }; @@ -272,7 +272,7 @@ test("atomic updater includes an edit made after the last snapshot without unlin await f.files.apply(structuredClone(f.database.state), { a: { expected, - replacement: await toStoredSnapshot({ + replacement: await hashSnapshot({ content: bytes("remote\nbase\n") }) } @@ -298,7 +298,7 @@ test("a failed atomic update leaves database metadata unchanged", async () => { f.files.apply(structuredClone(original), { a: { expected: (await f.files.snapshot("a.md"))!, - replacement: await toStoredSnapshot({ + 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 3bcac5f1..e9c5bed2 100644 --- a/frontend/test-support/listener-crash-child.ts +++ b/frontend/test-support/listener-crash-child.ts @@ -4,7 +4,7 @@ import { fixture } from "./sync-fixture"; const f = await fixture( {}, { - getEvents: async () => ({ headEventId: 0, events: [] }) + getEvents: async () => ({ headEventId: 0, endEventId: 0, events: [] }) } ); let delivered = 0; diff --git a/frontend/test-support/oracle-regressions.test.ts b/frontend/test-support/oracle-regressions.test.ts index dbd21b40..2858f7c5 100644 --- a/frontend/test-support/oracle-regressions.test.ts +++ b/frontend/test-support/oracle-regressions.test.ts @@ -61,6 +61,7 @@ test("canonical checks independently verify both persisted content hashes", asyn : url.includes("/events-since") ? { headEventId: 2, + endEventId: 2, events: [ { eventId: 1, requestId: "create" }, { eventId: 2, requestId: "manifest" } diff --git a/frontend/test-support/polling.test.ts b/frontend/test-support/polling.test.ts index dabc49c3..6a3b538d 100644 --- a/frontend/test-support/polling.test.ts +++ b/frontend/test-support/polling.test.ts @@ -8,7 +8,7 @@ import { import { Settings } from "../sync-client/src/persistence/settings"; import { FileOperations } from "../sync-client/src/file-operations/file-operations"; import { Syncer } from "../sync-client/src/sync-operations/syncer"; -import { toStoredSnapshot } from "../sync-client/src/snapshot"; +import { hashSnapshot } from "../sync-client/src/snapshot"; import { Logger } from "../sync-client/src/tracing/logger"; import { SyncHistory } from "../sync-client/src/tracing/sync-history"; import { FixedSizeDocumentCache } from "../sync-client/src/utils/data-structures/fix-sized-cache"; @@ -56,6 +56,7 @@ async function fixture(syncIntervalMs: number | undefined) { const batch = (after = 2): EventBatch => structuredClone({ headEventId, + endEventId: headEventId, events: events.filter((event) => event.eventId > after) }); const service: Partial = { @@ -90,7 +91,7 @@ async function fixture(syncIntervalMs: number | undefined) { initial.lastSeenUpdateId = 2; initial.local = { [documentId]: "note.bin" }; initial.fileManifest = { fileManifestId: 2, entries: { ...initial.local } }; - const hash = (await toStoredSnapshot({ content: bytes("base") })).hash; + const hash = (await hashSnapshot({ content: bytes("base") })).hash; initial.documents[documentId] = { remote: latest, observedHash: hash, @@ -123,7 +124,6 @@ async function fixture(syncIntervalMs: number | undefined) { >() }; const syncer = new Syncer( - "device", logger, database, settings, diff --git a/frontend/test-support/protocol-audit.test.ts b/frontend/test-support/protocol-audit.test.ts index 6e115f8a..4e318545 100644 --- a/frontend/test-support/protocol-audit.test.ts +++ b/frontend/test-support/protocol-audit.test.ts @@ -265,7 +265,9 @@ test( mode = "reject-retry"; await disk.userWrite("a.md", bytes("base")); await client.syncLocallyUpdatedFile({ relativePath: "a.md" }); - await client.waitUntilFinished(); + await assert.rejects(client.waitUntilFinished(), /413/); + assert.deepEqual((await disk.read("a.md"))?.content, bytes("base")); + await client.reset(); const result = Buffer.from( (await disk.read("a.md"))!.content ).toString(); diff --git a/frontend/test-support/real-disk.test.ts b/frontend/test-support/real-disk.test.ts index ad25f9d6..8a22592e 100644 --- a/frontend/test-support/real-disk.test.ts +++ b/frontend/test-support/real-disk.test.ts @@ -7,7 +7,7 @@ import { NodeFileSystemOperations } from "../local-client-cli/src/node-filesyste import { Database, emptyState } 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 { toStoredSnapshot } from "../sync-client/src/snapshot"; +import { hashSnapshot } from "../sync-client/src/snapshot"; test( "real filesystem: direct updates, unsafe entries, and file swaps", @@ -103,7 +103,7 @@ test( initial.documents[id] = { observedHash: ( - await toStoredSnapshot( + await hashSnapshot( (await disk.read(relative))! ) ).hash diff --git a/frontend/test-support/recovery.test.ts b/frontend/test-support/recovery.test.ts index 70975c35..0fe61fcd 100644 --- a/frontend/test-support/recovery.test.ts +++ b/frontend/test-support/recovery.test.ts @@ -7,7 +7,7 @@ import { type StoredDatabase } from "../sync-client/src/persistence/database"; import { FileOperations } from "../sync-client/src/file-operations/file-operations"; -import { toStoredSnapshot } from "../sync-client/src/snapshot"; +import { hashSnapshot } from "../sync-client/src/snapshot"; import type { ServerConfig } from "../sync-client/src/services/server-config"; import { mergeFileManifests } from "../sync-client/src/sync-operations/file-manifest"; import { assertManifest, pathDecision } from "./oracles"; @@ -37,7 +37,7 @@ async function fixture( initial.documents[id] = { observedHash: ( - await toStoredSnapshot((await disk.read(path))!) + await hashSnapshot((await disk.read(path))!) ).hash }; const persistence = new MemoryPersistence({ database: initial }); @@ -75,7 +75,7 @@ async function swapFixture() { next.local = { a: "b.md", b: "a.md", c: "nested/c.md" }; next.documents.c = {}; const writes = { - c: { replacement: await toStoredSnapshot({ content: bytes("C") }) } + c: { replacement: await hashSnapshot({ content: bytes("C") }) } }; const expected = new Map([ ["a.md", bytes("B")], @@ -129,8 +129,8 @@ async function mergeFixture() { next: structuredClone(current.initial), writes: { a: { - expected: await toStoredSnapshot({ content: bytes(base) }), - replacement: await toStoredSnapshot({ content: bytes(remote) }) + expected: await hashSnapshot({ content: bytes(base) }), + replacement: await hashSnapshot({ content: bytes(remote) }) } }, expected: new Map([ @@ -154,7 +154,7 @@ async function deletionFixture() { next, writes: { a: { - replacement: await toStoredSnapshot({ + replacement: await hashSnapshot({ content: bytes("remote binary\0") }) } @@ -178,8 +178,8 @@ async function permanentDestinationFixture() { }, writes: { a: { - expected: await toStoredSnapshot({ content: bytes("A") }), - replacement: await toStoredSnapshot({ + expected: await hashSnapshot({ content: bytes("A") }), + replacement: await hashSnapshot({ content: bytes("REMOTE") }) } @@ -418,8 +418,8 @@ for (const phase of ["before", "durable"]) { await current.disk.userDelete("a.md"); await current.disk.userWrite("a.md", bytes("new incarnation")); const changes: LocalChange[] = [ - { type: "delete", path: "a.md" }, - { type: "create", path: "a.md" } + { type: "delete", path: "a.md", changeId: "delete" }, + { type: "create", path: "a.md", changeId: "recreate" } ]; const scan = async () => { const runtime = current.restart(); @@ -475,7 +475,12 @@ for (const editedContent of ["new bytes", "B"]) { await current.disk.userRename("a.md", "moved.md"); await current.disk.userWrite("moved.md", bytes(editedContent)); const changes: LocalChange[] = [ - { type: "move", oldPath: "a.md", relativePath: "moved.md" } + { + type: "move", + oldPath: "a.md", + relativePath: "moved.md", + changeId: "move" + } ]; await scanChanges(runtime, changes); assert.deepEqual( @@ -509,7 +514,12 @@ for (const equalContent of [false, true]) { ["temporary.md", "b.md"] ]) { await current.disk.userRename(oldPath, relativePath); - changes.push({ type: "move", oldPath, relativePath }); + changes.push({ + type: "move", + oldPath, + relativePath, + changeId: oldPath + }); } const expected = { a: "b.md", b: "a.md" }; const expectedFiles = new Map([ diff --git a/frontend/test-support/server-restore.test.ts b/frontend/test-support/server-restore.test.ts index a7562a03..553c4324 100644 --- a/frontend/test-support/server-restore.test.ts +++ b/frontend/test-support/server-restore.test.ts @@ -9,7 +9,6 @@ for (const mode of [ "behind", "dirty", "pending", - "legacy-state", ...["before:save", "durable:save"].flatMap((boundary) => [1, 2, 3, 4, 5].map((ordinal) => `${boundary}:${ordinal}`) ) @@ -124,16 +123,6 @@ for (const mode of [ await f.reopenClient(); await f.client.start(); } - if (mode === "legacy-state") { - await f.client.destroy(); - const stored = f.store.snapshot() as ReturnType< - typeof f.store.snapshot - > & { historyCheckpoint?: unknown }; - delete stored.historyCheckpoint; - await f.store.save(stored); - await f.reopenClient(); - await f.client.start(); - } await f.client.setSetting("isSyncEnabled", true); const actual = Buffer.from( (await f.disk.read("a.md"))!.content diff --git a/frontend/test-support/sync-audit.test.ts b/frontend/test-support/sync-audit.test.ts index 4a36d460..f26bbb33 100644 --- a/frontend/test-support/sync-audit.test.ts +++ b/frontend/test-support/sync-audit.test.ts @@ -5,7 +5,7 @@ import { test } from "node:test"; import type { Database } from "../sync-client/src/persistence/database"; import { emptyState } from "../sync-client/src/persistence/database"; import { SyncClient } from "../sync-client/src/sync-client"; -import { toStoredSnapshot } from "../sync-client/src/snapshot"; +import { hashSnapshot } from "../sync-client/src/snapshot"; import { EventListeners } from "../sync-client/src/utils/data-structures/event-listeners"; import type { SyncService } from "../sync-client/src/services/sync-service"; import type { WebSocketManager } from "../sync-client/src/services/websocket-manager"; @@ -81,11 +81,13 @@ test("oversized untracked file must remain in place", async () => { }); test("rejected unknown success must use submitted content as merge base", async () => { + let reject = true; const f = await fixture( { "a.md": "base" }, { putFileContent: async () => { - throw new PermanentSyncError("HTTP 413"); + if (reject) throw new PermanentSyncError("HTTP 413"); + return { type: "Accepted", ...head("a", 2, "base A") }; }, getDocumentVersionContent: async ({ vaultUpdateId }) => bytes( @@ -106,10 +108,14 @@ test("rejected unknown success must use submitted content as merge base", async f.internals.stopped = false; await f.internals.preparePush(); const req = f.database.state.pending!.request.requestId; - await f.internals.finishPending(); + await assert.rejects(f.internals.finishPending(), /413/); await f.disk.userWrite("a.md", bytes("base")); + reject = false; + await f.syncer.retryRejectedRequests(); + await f.internals.finishPending(); await f.internals.incorporateEventBatch({ headEventId: 3, + endEventId: 3, events: [ { eventId: 2, @@ -166,7 +172,7 @@ test("oversized remote head must not cause identical stale uploads forever", asy ); f.database.state.documents.a.base = { ...head("a", 1, "base"), - hash: (await toStoredSnapshot({ content: bytes("base") })).hash + hash: (await hashSnapshot({ content: bytes("base") })).hash }; (f.database.state.documents.a ??= {}).remote = head("a", 2, remoteText); await f.settings.setSettings({ maxFileSizeMB: 10 / (1024 * 1024) }); @@ -178,11 +184,13 @@ test("oversized remote head must not cause identical stale uploads forever", asy }); test("local undo of accepted-but-rejected manifest must survive replay", async () => { + let reject = true; const f = await fixture( { "a.md": "A" }, { pushFileManifest: async () => { - throw new PermanentSyncError("HTTP 413"); + if (reject) throw new PermanentSyncError("HTTP 413"); + return { type: "Accepted", fileManifestId: 2 }; }, getDocumentVersionContent: async () => bytes("A") } @@ -202,14 +210,18 @@ test("local undo of accepted-but-rejected manifest must survive replay", async ( await f.internals.preparePush(); const req = f.database.state.pending!.request.requestId; f.internals.stopped = true; - await f.internals.finishPending(); + await assert.rejects(f.internals.finishPending(), /413/); await f.disk.userRename("b.md", "a.md"); await f.syncer.syncLocallyUpdatedFile({ oldPath: "b.md", relativePath: "a.md" }); + reject = false; + await f.syncer.retryRejectedRequests(); + await f.internals.finishPending(); await f.internals.incorporateEventBatch({ headEventId: 2, + endEventId: 2, events: [ { eventId: 2, @@ -244,9 +256,6 @@ test("a dirty cursor update must invalidate old precise positions", async () => const { CursorTracker } = await import( "../sync-client/src/sync-operations/cursor-tracker" ); - const { FileChangeNotifier } = await import( - "../sync-client/src/sync-operations/file-change-notifier" - ); const f = await fixture({ "a.md": "base" }); f.database.state.documents.a.base = { ...head("a", 1, "base"), @@ -261,7 +270,7 @@ test("a dirty cursor update must invalidate old precise positions", async () => f.database, ws as unknown as WebSocketManager, f.files, - new FileChangeNotifier() + new EventListeners() ); let latest: import("../sync-client/src/types/maybe-outdated-client-cursors").MaybeOutdatedClientCursors[] = []; @@ -335,21 +344,35 @@ test("silent open WebSocket must eventually trigger catchup or reconnect", async fetch: async (input) => { const path = String(input); if (path.endsWith("/ping")) - return Response.json({ - supportedApiVersion: SUPPORTED_API_VERSION, - isAuthenticated: true, - mergeableFileExtensions: ["md"], - serverVersion: "test" - }); + return Response.json( + { + supportedApiVersion: SUPPORTED_API_VERSION, + isAuthenticated: true, + mergeableFileExtensions: ["md"], + serverVersion: "test" + }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); + if (path.endsWith("/file-manifest")) + return Response.json( + { type: "Accepted", fileManifestId: 0 }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); if (path.endsWith("/vault-snapshot")) - return Response.json({ - headEventId: 0, - fileManifest: { fileManifestId: 0, entries: {} }, - documents: [] - }); + return Response.json( + { + headEventId: 0, + fileManifest: { fileManifestId: 0, entries: {} }, + documents: [] + }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); if (path.includes("/events-since")) { eventReads++; - return Response.json({ headEventId: 0, events: [] }); + return Response.json( + { headEventId: 0, endEventId: 0, events: [] }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); } throw new Error("Unexpected request " + path); } @@ -478,7 +501,7 @@ for (const interrupted of [false, true]) { f = await fixture({ "a.md": "base LOCAL" }, service); f.database.state.documents.a.base = { ...head("a", 1, "base"), - hash: (await toStoredSnapshot({ content: bytes("base") })).hash + hash: (await hashSnapshot({ content: bytes("base") })).hash }; (f.database.state.documents.a ??= {}).remote = head("a", 1, "base"); f.database.state.lastSeenUpdateId = 1; @@ -554,14 +577,16 @@ test("content notifications during every snapshot do not starve a complete names }); for (const boundary of ["before:save", "durable:save"]) { - test(`receipt incorporation after paged replay survives ${boundary} and process replacement`, async () => { + test(`retried acknowledgement before paged replay survives ${boundary} and process replacement`, async () => { + let reject = true; const service: Partial = { getEvents: async (after) => { assert.equal(after, 2); return page; }, putFileContent: async () => { - throw new PermanentSyncError("HTTP 413"); + if (reject) throw new PermanentSyncError("HTTP 413"); + return { type: "Accepted", ...head("a", 2, "base A") }; }, getDocumentVersionContent: async ({ vaultUpdateId }) => bytes( @@ -582,7 +607,7 @@ for (const boundary of ["before:save", "durable:save"]) { f.internals.stopped = false; await f.internals.preparePush(); const { requestId } = f.database.state.pending!.request; - await f.internals.finishPending(); + await assert.rejects(f.internals.finishPending(), /413/); await f.disk.userWrite("a.md", bytes("base")); const first = { headEventId: 3, @@ -608,17 +633,20 @@ for (const boundary of ["before:save", "durable:save"]) { } ] }; + reject = false; + await f.syncer.retryRejectedRequests(); f.persistence.boundary = (label) => { if (label === boundary) throw new Error("injected crash"); }; await assert.rejects( - f.internals.incorporateEventBatch(first), + f.internals.finishPending(), /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); assert.equal(f.database.state.lastSeenUpdateId, 3); assert.deepEqual( @@ -653,19 +681,28 @@ test("initial vault settings bind the engine and survive restart", async () => { webSocket: Socket as unknown as typeof WebSocket, fetch: async (input: RequestInfo | URL) => { if (String(input).endsWith("/ping")) - return Response.json({ - supportedApiVersion: SUPPORTED_API_VERSION, - isAuthenticated: true, - mergeableFileExtensions: ["md"], - serverVersion: "test" - }); + return Response.json( + { + supportedApiVersion: SUPPORTED_API_VERSION, + isAuthenticated: true, + mergeableFileExtensions: ["md"], + serverVersion: "test" + }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); if (String(input).endsWith("/vault-snapshot")) - return Response.json({ - headEventId: 0, - fileManifest: { fileManifestId: 0, entries: {} }, - documents: [] - }); - return Response.json({ headEventId: 0, events: [] }); + return Response.json( + { + headEventId: 0, + fileManifest: { fileManifestId: 0, entries: {} }, + documents: [] + }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); + return Response.json( + { headEventId: 0, endEventId: 0, events: [] }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); } }; let client = await SyncClient.create(options); @@ -695,13 +732,14 @@ test("disabling sync persists across reset without resuming the transport", asyn } }); -for (const receipt of [false, true]) { - test(`${receipt ? "replayed" : "direct"} upload acknowledgement survives a local deletion`, async () => { +for (const retry of [false, true]) { + test(`${retry ? "retried" : "direct"} upload acknowledgement survives a local deletion`, async () => { + let reject = retry; const f = await fixture( { "a.md": "base" }, { putFileContent: async () => { - if (receipt) + if (reject) throw new PermanentSyncError( "HTTP 413 after lost success" ); @@ -717,30 +755,19 @@ for (const receipt of [false, true]) { await f.disk.userWrite("a.md", bytes("edited")); f.internals.stopped = false; await f.internals.preparePush(); - const { requestId } = f.database.state.pending!.request; - if (receipt) await f.internals.finishPending(); + if (retry) await assert.rejects(f.internals.finishPending(), /413/); // Recovery folds queued deletions into the durable namespace before - // an outstanding request or its delayed receipt is handled. + // an outstanding request is retried. f.internals.stopped = true; await f.disk.userDelete("a.md"); await f.syncer.syncLocallyDeletedFile("a.md"); await f.internals.scan(); assert.deepEqual(f.database.state.local, {}); - if (receipt) { - await f.internals.incorporateEventBatch({ - headEventId: 2, - events: [ - { - eventId: 2, - requestId, - type: "content", - document: head("a", 2, "edited") - } - ] - }); - assert.equal(f.database.state.lastSeenUpdateId, 2); - assert.equal(f.database.state.unconfirmed?.length ?? 0, 0); - } else await f.internals.finishPending(); + if (retry) { + reject = false; + await f.syncer.retryRejectedRequests(); + } + await f.internals.finishPending(); assert.equal(f.database.state.pending, undefined); assert.deepEqual(f.database.state.local, {}); assert.equal(await f.disk.read("a.md"), undefined); @@ -781,21 +808,35 @@ async function settingsLifecycleFixture() { fetch: async (input) => { const path = String(input); if (path.endsWith("/ping")) - return Response.json({ - supportedApiVersion: SUPPORTED_API_VERSION, - isAuthenticated: true, - mergeableFileExtensions: ["md"], - serverVersion: "test" - }); + return Response.json( + { + supportedApiVersion: SUPPORTED_API_VERSION, + isAuthenticated: true, + mergeableFileExtensions: ["md"], + serverVersion: "test" + }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); + if (path.endsWith("/file-manifest")) + return Response.json( + { type: "Accepted", fileManifestId: 0 }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); if (path.endsWith("/vault-snapshot")) - return Response.json({ - headEventId: 0, - fileManifest: { fileManifestId: 0, entries: {} }, - documents: [] - }); + return Response.json( + { + headEventId: 0, + fileManifest: { fileManifestId: 0, entries: {} }, + documents: [] + }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); if (path.includes("/events-since")) { eventReads++; - return Response.json({ headEventId: 0, events: [] }); + return Response.json( + { headEventId: 0, endEventId: 0, events: [] }, + { headers: { "X-Vault-Link-History": "0:empty" } } + ); } throw new Error("Unexpected request " + path); } @@ -818,12 +859,11 @@ for (const boundary of ["before:save", "durable:save"]) { const { database } = f.client as unknown as { database: Database }; await database.commit({ ...database.state, - unconfirmed: [{ + pending: { type: "fileManifest", - requestId: "rejected", - entries: { retired: "old.md" }, - rejection: "retired rejection" - }] + request: { requestId: "rejected", parentFileManifestId: 0, entries: {} }, + rejection: "upload rejected" + } }); let saves = 0, failed = false; @@ -840,6 +880,10 @@ for (const boundary of ["before:save", "durable:save"]) { ); assert(failed); assert.equal(f.client.getSettings().maxFileSizeMB, 20); + if (boundary === "before:save") { + await assert.rejects(f.client.waitUntilFinished(), /upload rejected/); + await f.client.reset(); + } const before = f.reads(); await f.client.syncLocallyUpdatedFile({ relativePath: "absent.md" diff --git a/frontend/test-support/sync-fixture.ts b/frontend/test-support/sync-fixture.ts index 7128bb6d..2947ba87 100644 --- a/frontend/test-support/sync-fixture.ts +++ b/frontend/test-support/sync-fixture.ts @@ -6,7 +6,7 @@ import { import { Settings } from "../sync-client/src/persistence/settings"; import { FileOperations } from "../sync-client/src/file-operations/file-operations"; import { Syncer } from "../sync-client/src/sync-operations/syncer"; -import { toStoredSnapshot } from "../sync-client/src/snapshot"; +import { hashSnapshot } from "../sync-client/src/snapshot"; import { Logger } from "../sync-client/src/tracing/logger"; import { SyncHistory } from "../sync-client/src/tracing/sync-history"; import { FixedSizeDocumentCache } from "../sync-client/src/utils/data-structures/fix-sized-cache"; @@ -45,7 +45,7 @@ export async function fixture( initial.documents[id] = { observedHash: ( - await toStoredSnapshot({ content: bytes(content) }) + await hashSnapshot({ content: bytes(content) }) ).hash }; } @@ -88,7 +88,6 @@ export async function fixture( } as unknown as WebSocketManager; const contentCache = new FixedSizeDocumentCache(1024); const syncer = new Syncer( - "device", logger, database, settings, diff --git a/scripts/build-sync-server-binaries.sh b/scripts/build-sync-server-binaries.sh index 8d690935..d199ead6 100755 --- a/scripts/build-sync-server-binaries.sh +++ b/scripts/build-sync-server-binaries.sh @@ -4,10 +4,6 @@ set -e cd "$(dirname "$0")/../sync-server" -# Setup database -sqlx database create --database-url sqlite://db.sqlite3 2>/dev/null || true -sqlx migrate run --source src/app_state/database/migrations --database-url sqlite://db.sqlite3 - targets=${@:-"x86_64-unknown-linux-gnu x86_64-unknown-linux-musl aarch64-unknown-linux-gnu x86_64-pc-windows-gnu"} mkdir -p artifacts diff --git a/scripts/check.sh b/scripts/check.sh index 9f5ee98c..a774d14a 100755 --- a/scripts/check.sh +++ b/scripts/check.sh @@ -14,10 +14,6 @@ echo "Running checks in sync-server" ( cd sync-server - which sqlx || cargo install sqlx-cli - sqlx database create --database-url sqlite://db.sqlite3 - sqlx migrate run --source src/app_state/database/migrations --database-url sqlite://db.sqlite3 - cargo test --verbose if [[ "$FIX_MODE" == true ]]; then diff --git a/sync-server/.env b/sync-server/.env deleted file mode 100644 index b47c1b5c..00000000 --- a/sync-server/.env +++ /dev/null @@ -1 +0,0 @@ -DATABASE_URL=sqlite://db.sqlite3 diff --git a/sync-server/Cargo.toml b/sync-server/Cargo.toml index 7c2c1ab5..a9aac137 100644 --- a/sync-server/Cargo.toml +++ b/sync-server/Cargo.toml @@ -21,7 +21,7 @@ tower-http = { version = "0.6.1", features = ["cors", "trace", "limit", "timeout tracing = "0.1.41" tracing-subscriber = { version = "0.3.20", features = ["fmt", "env-filter"]} humantime-serde = "1.1.1" -sqlx = { version = "0.8.6", features = ["sqlite", "runtime-tokio", "uuid", "chrono"] } +sqlx = { version = "0.8.6", default-features = false, features = ["sqlite", "runtime-tokio", "uuid", "chrono", "macros"] } chrono = { version = "0.4.41", features = ["serde"] } rand = "0.9.0" regex = "1.12.2" diff --git a/sync-server/Dockerfile b/sync-server/Dockerfile index 77599b2c..32767ee6 100644 --- a/sync-server/Dockerfile +++ b/sync-server/Dockerfile @@ -5,14 +5,11 @@ WORKDIR /usr/src/backend RUN apt update && \ apt install -y \ libssl-dev \ - pkg-config && \ - cargo install sqlx-cli + pkg-config COPY . . -RUN sqlx database create --database-url sqlite://db.sqlite3 && \ - sqlx migrate run --source src/app_state/database/migrations --database-url sqlite://db.sqlite3 && \ - cargo build --release +RUN cargo build --release FROM debian:trixie-slim diff --git a/sync-server/README.md b/sync-server/README.md index 23548219..6e604a2c 100644 --- a/sync-server/README.md +++ b/sync-server/README.md @@ -127,11 +127,12 @@ flushes database directory entries; the filesystem must support those durability operations. Content, file manifests, and events (including request fingerprints) are retained indefinitely. -Deploy API v5 clients and server together. The v4 database schema migrates by -adding event-incarnation tokens without deleting content or receipts. Schemas -from before v4 still require a separate database directory and explicit recovery. -Vault databases open and migrate independently on first access; one corrupt or -incompatible vault does not prevent healthy vaults from serving requests. +Deploy API v5 clients and server together. Only the current database schema is +supported; there are no schema migrations. Empty vault databases are initialized +from `src/app_state/database/schema.sql` on first access. Existing databases are +opened without conversion. Use fresh database and client state directories when +changing formats. Each vault opens independently, so one corrupt vault does not +prevent healthy vaults from serving requests. Diffs are limited to 10,000 items and reconstructed in one pass over Unicode scalars on a blocking worker, before taking the SQLite writer transaction. The diff --git a/sync-server/build.rs b/sync-server/build.rs deleted file mode 100644 index ee539ea3..00000000 --- a/sync-server/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -// generated by `sqlx migrate build-script` -fn main() { - // trigger recompilation when a new migration is added - println!("cargo:rerun-if-changed=src/app_state/database/migrations"); -} diff --git a/sync-server/src/app_state/database.rs b/sync-server/src/app_state/database.rs index e5c492cb..4fa2d394 100644 --- a/sync-server/src/app_state/database.rs +++ b/sync-server/src/app_state/database.rs @@ -9,7 +9,7 @@ use anyhow::{Context as _, Result}; use log::info; use models::VaultId; use sha2::{Digest, Sha256}; -use sqlx::{ConnectOptions, sqlite::SqliteConnectOptions}; +use sqlx::{ConnectOptions, Executor, sqlite::SqliteConnectOptions}; use sqlx::{Pool, Sqlite, sqlite::SqlitePoolOptions}; use tokio::sync::Mutex; use tokio::time::Instant; @@ -115,7 +115,7 @@ impl Database { .await .with_context(|| format!("Cannot open database at `{}`", file_name.display()))?; - Self::run_migrations(&pool).await?; + Self::initialize_schema(&pool).await?; if let Some(directory) = file_name.parent() { #[cfg(unix)] @@ -138,11 +138,21 @@ impl Database { Ok(()) } - async fn run_migrations(pool: &Pool) -> Result<()> { - sqlx::migrate!("src/app_state/database/migrations") - .run(pool) + async fn initialize_schema(pool: &Pool) -> Result<()> { + let mut tx = pool.begin_with("BEGIN IMMEDIATE").await?; + let initialized: bool = sqlx::query_scalar( + "SELECT EXISTS(SELECT 1 FROM sqlite_schema WHERE name NOT LIKE 'sqlite_%')", + ) + .fetch_one(&mut *tx) + .await?; + if !initialized { + tx.execute(include_str!("database/schema.sql")) + .await + .context("Cannot initialize vault database")?; + } + tx.commit() .await - .context("Cannot check for pending migrations") + .context("Cannot commit vault database initialization") } async fn get_connection_pool(&self, vault: &VaultId) -> Result> { diff --git a/sync-server/src/app_state/database/models.rs b/sync-server/src/app_state/database/models.rs index 5330a8dc..084b1cb6 100644 --- a/sync-server/src/app_state/database/models.rs +++ b/sync-server/src/app_state/database/models.rs @@ -125,9 +125,8 @@ pub struct EventBatch { pub head_event_id: VaultUpdateId, /// Last event in this page; `head_event_id` is the vault's current head. - #[serde(skip_serializing_if = "Option::is_none")] - #[ts(optional, as = "Option")] - pub end_event_id: Option, + #[ts(as = "f64")] + pub end_event_id: VaultUpdateId, pub events: Vec, } diff --git a/sync-server/src/app_state/database/queries.rs b/sync-server/src/app_state/database/queries.rs index 162b0fe6..e7b41efc 100644 --- a/sync-server/src/app_state/database/queries.rs +++ b/sync-server/src/app_state/database/queries.rs @@ -233,7 +233,7 @@ impl Database { Ok(EventBatch { head_event_id, - end_event_id: Some(end_event_id), + end_event_id, events, }) } diff --git a/sync-server/src/app_state/database/migrations/20241207143519_bootstrap.sql b/sync-server/src/app_state/database/schema.sql similarity index 98% rename from sync-server/src/app_state/database/migrations/20241207143519_bootstrap.sql rename to sync-server/src/app_state/database/schema.sql index f243d1e0..d19be11d 100644 --- a/sync-server/src/app_state/database/migrations/20241207143519_bootstrap.sql +++ b/sync-server/src/app_state/database/schema.sql @@ -7,7 +7,7 @@ CREATE TABLE events ( -- request, so the history checkpoint `event_id:event_token` fails whenever -- a restored backup reuses a numeric ID or a client resubmits the same -- request UUID against a different history - event_token TEXT, + event_token TEXT NOT NULL, -- Client-chosen idempotency key, stable across retries so the server can -- deduplicate a replayed request request_id TEXT NOT NULL UNIQUE, diff --git a/sync-server/src/app_state/database/tests.rs b/sync-server/src/app_state/database/tests.rs index 25de0e5f..9fdcf753 100644 --- a/sync-server/src/app_state/database/tests.rs +++ b/sync-server/src/app_state/database/tests.rs @@ -96,15 +96,15 @@ async fn rejects_invalid_vault_names_at_the_database_boundary() { } #[tokio::test] -async fn migrations_can_be_reapplied_without_losing_requests_or_content() { +async fn schema_initialization_preserves_existing_requests_and_content() { let directory = tempfile::tempdir().unwrap(); let config = config(&directory); let database = Database::try_new(&config).await.unwrap(); - let vault = "migrations".to_owned(); + let vault = "initialization".to_owned(); let version = insert_content(&database, &vault, uuid::Uuid::new_v4(), b"only copy").await; let pool = database.get_connection_pool(&vault).await.unwrap(); - Database::run_migrations(&pool).await.unwrap(); - Database::run_migrations(&pool).await.unwrap(); + Database::initialize_schema(&pool).await.unwrap(); + Database::initialize_schema(&pool).await.unwrap(); let reopened = Database::try_new(&config).await.unwrap(); assert_eq!( reopened @@ -307,7 +307,7 @@ async fn replay_byte_budget_prevents_accumulating_large_historical_manifests() { assert!(serde_json::to_vec(&batch).unwrap().len() < MAX_EVENTS_PAGE_BYTES + 1024); assert_eq!( batch.end_event_id, - batch.events.last().map(|event| event.event_id) + batch.events.last().unwrap().event_id ); } @@ -318,8 +318,8 @@ async fn opening_a_locked_vault_does_not_block_unrelated_vaults() { let database = Database::try_new(&config).await.unwrap(); database.vault_snapshot(&"other".to_owned()).await.unwrap(); let vault = "locked-opening".to_owned(); - // A second process holds the writer lock while this process lazily checks - // migrations. Even already-open, unrelated vaults must remain available. + // 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)) .await .unwrap(); @@ -346,7 +346,7 @@ async fn opening_a_locked_vault_does_not_block_unrelated_vaults() { ); assert_eq!( other - .expect("a blocked migration must not lock every vault") + .expect("a blocked database open must not lock every vault") .unwrap() .head_event_id, 0 diff --git a/sync-server/src/server/responses.rs b/sync-server/src/server/responses.rs index e8d85687..ad0742b3 100644 --- a/sync-server/src/server/responses.rs +++ b/sync-server/src/server/responses.rs @@ -20,7 +20,7 @@ pub struct PingResponse { /// List of file extensions that are allowed to be merged. pub mergeable_file_extensions: Vec, - /// API version ensuring backwards & forwards compatibility between the client + /// API version that must match exactly between the client /// and server. pub supported_api_version: u32, }