From b52c09fecc7b1e9a34a9c74f48f1bb449f6cb0e3 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sat, 25 Apr 2026 13:40:34 +0100 Subject: [PATCH] Small changes --- frontend/sync-client/src/sync-client.ts | 2 +- .../src/sync-operations/conflict-path.ts | 10 +- .../offline-change-detector.ts | 250 +++--------------- .../sync-operations/sync-event-queue.test.ts | 4 +- .../src/sync-operations/sync-event-queue.ts | 90 ++----- .../sync-client/src/sync-operations/syncer.ts | 2 +- .../sync-client/src/sync-operations/types.ts | 9 +- .../src/utils/find-matching-file.ts | 4 +- 8 files changed, 74 insertions(+), 297 deletions(-) diff --git a/frontend/sync-client/src/sync-client.ts b/frontend/sync-client/src/sync-client.ts index 39b0f000..65580420 100644 --- a/frontend/sync-client/src/sync-client.ts +++ b/frontend/sync-client/src/sync-client.ts @@ -324,7 +324,7 @@ export class SyncClient { await this.pause(); this.logger.info("Resetting SyncClient's local state"); - this.syncEventQueue.resetState(); + this.syncEventQueue.clearAllState(); await this.syncEventQueue.save(); this.resetInMemoryState(); this.hasFinishedOfflineSync = false; diff --git a/frontend/sync-client/src/sync-operations/conflict-path.ts b/frontend/sync-client/src/sync-operations/conflict-path.ts index 7a634bb4..69942750 100644 --- a/frontend/sync-client/src/sync-operations/conflict-path.ts +++ b/frontend/sync-client/src/sync-operations/conflict-path.ts @@ -8,18 +8,12 @@ export const CONFLICT_PATH_REGEX = /(?:^|\/)conflict-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}-[^/]*$/u; -// Safe segment length for common filesystems (ext4 / NTFS / APFS all cap -// at 255 bytes). `conflict-<36-char-uuid>-` adds 46 bytes; reserve a few -// extra bytes for a future prefix bump and leave room for multi-byte UTF-8 -// characters in the original name. + const CONFLICT_PREFIX_LEN = "conflict-".length + 36 + 1; const MAX_SEGMENT_BYTES = 255; const MAX_ORIGINAL_BYTES = MAX_SEGMENT_BYTES - CONFLICT_PREFIX_LEN - 4; export function buildConflictFileName(fileName: string): string { - // Truncate the original name if keeping it whole would bust the - // filesystem's segment-length cap. Preserve the trailing extension - // so the file is still recognizable / openable. const safeName = truncateFileNameToByteLimit(fileName, MAX_ORIGINAL_BYTES); return `conflict-${crypto.randomUUID()}-${safeName}`; } @@ -40,8 +34,6 @@ function truncateFileNameToByteLimit( const extensionBytes = encoder.encode(extension).byteLength; const stemBudget = Math.max(0, maxBytes - extensionBytes); - // Walk the stem by grapheme cluster so we never split an emoji sequence - // (e.g. ZWJ families, skin-tone modifiers) or a base+combining-mark pair. const segmenter = new Intl.Segmenter(undefined, { granularity: "grapheme" }); let truncatedStem = ""; let usedBytes = 0; diff --git a/frontend/sync-client/src/sync-operations/offline-change-detector.ts b/frontend/sync-client/src/sync-operations/offline-change-detector.ts index c90f6a78..8bb8c27c 100644 --- a/frontend/sync-client/src/sync-operations/offline-change-detector.ts +++ b/frontend/sync-client/src/sync-operations/offline-change-detector.ts @@ -1,4 +1,4 @@ -import type { DocumentRecord, RelativePath } from "./types"; +import type { DocumentRecord, DocumentWithPath, RelativePath } from "./types"; import { SyncEventType } from "./types"; import type { Logger } from "../tracing/logger"; import { hash } from "../utils/hash"; @@ -6,23 +6,9 @@ import type { FileOperations } from "../file-operations/file-operations"; import { findMatchingFile } from "../utils/find-matching-file"; import { FileNotFoundError } from "../errors/file-not-found-error"; import type { SyncEventQueue } from "./sync-event-queue"; +import { removeFromArray } from "../utils/remove-from-array"; -interface DocumentWithPath { - path: RelativePath; - record: DocumentRecord; -} -interface SyncInstruction { - type: "update" | "create"; - relativePath: string; - oldPath?: string; -} - -interface OfflineChangeDetectorDeps { - logger: Logger; - operations: FileOperations; - queue: SyncEventQueue; -} /** * Scans the local filesystem and the document database to determine @@ -30,218 +16,64 @@ interface OfflineChangeDetectorDeps { * client was offline, then enqueues the appropriate sync events. */ export async function scheduleOfflineChanges( - deps: OfflineChangeDetectorDeps, + logger: Logger, + operations: FileOperations, + queue: SyncEventQueue, enqueueCreate: (path: RelativePath) => void, enqueueUpdate: (args: { oldPath?: RelativePath; relativePath: RelativePath }) => void, enqueueDelete: (path: RelativePath) => void, ): Promise { - const { logger, operations, queue } = deps; - const allLocalFiles = await operations.listFilesRecursively(); logger.info(`Scheduling sync for ${allLocalFiles.length} local files`); + const allDocuments = queue.allSettledDocuments(); - queue.clear(); + const locallyPossiblyDeletedFiles: DocumentWithPath[] = []; - const allDocuments = new Map(queue.allSettledDocuments()); - const locallyRenamedPaths = enqueueRenamedDocuments(deps, allDocuments); - - const deletedCandidates = await findLocallyDeletedFiles(operations, allDocuments); - - const instructions = await buildSyncInstructions( - deps, - allLocalFiles, - locallyRenamedPaths, - deletedCandidates, - ); - - // Enqueue deletes first - for (const { path } of deletedCandidates) { - logger.debug(`Document ${path} has been deleted locally, scheduling sync to delete it`); - enqueueDelete(path); - } - - // Then updates/moves - for (const instruction of instructions) { - if (instruction.type === "update") { - enqueueUpdate({ - oldPath: instruction.oldPath, - relativePath: instruction.relativePath, - }); + for (const [path, record] of allDocuments.entries()) { + if ( + record !== undefined + ) { + locallyPossiblyDeletedFiles.push({ path, record }); } } - // Creates last so the server can merge with existing documents - for (const instruction of instructions) { - if (instruction.type === "create") { - enqueueCreate(instruction.relativePath); - } - } -} + const locallyPossibleCreatedFiles: RelativePath[] = []; + const syncedLocalFiles: RelativePath[] = []; -function enqueueRenamedDocuments( - { queue, logger }: OfflineChangeDetectorDeps, - allDocuments: Map, -): Set { - const locallyRenamedPaths = new Set(); - - for (const [path, record] of allDocuments) { - const remoteRelPath = record.remoteRelativePath; - const hasLocalRename = remoteRelPath !== undefined && remoteRelPath !== path; - - if (hasLocalRename) { - queue.enqueue({ type: SyncEventType.LocalUpdate, path }); - locallyRenamedPaths.add(path); - logger.debug(`Document ${path} was renamed locally (from ${remoteRelPath}), scheduling sync`); + for (const localFile of allLocalFiles) { + if (allDocuments.has(localFile) + ) { + syncedLocalFiles.push(localFile); + } else { + locallyPossibleCreatedFiles.push(localFile); } } - return locallyRenamedPaths; -} - -async function findLocallyDeletedFiles( - operations: FileOperations, - allDocuments: Map, -): Promise { - const result: DocumentWithPath[] = []; - - for (const [path, record] of allDocuments) { - if (!(await operations.exists(path))) { - result.push({ path, record }); - } - } - - return result; -} - -async function buildSyncInstructions( - deps: OfflineChangeDetectorDeps, - allLocalFiles: RelativePath[], - locallyRenamedPaths: Set, - deletedCandidates: DocumentWithPath[], -): Promise { - const { logger, operations, queue } = deps; - const instructions: SyncInstruction[] = []; - - for (const relativePath of allLocalFiles) { - if (locallyRenamedPaths.has(relativePath)) { - continue; - } - - const existingRecord = queue.getSettledDocumentByPath(relativePath); - - if (existingRecord !== undefined) { - const result = await handleExistingDocument( - deps, - relativePath, - existingRecord, - deletedCandidates, - ); - if (result !== undefined) { - if (result.updatedDeletedCandidates !== undefined) { - deletedCandidates = result.updatedDeletedCandidates; - } - if (result.instruction !== undefined) { - instructions.push(result.instruction); - } - continue; - } + for (const path of locallyPossibleCreatedFiles) { + const content = await operations.read(path); + const contentHash = await hash(content); + const matchingDeletedFile = await findMatchingFile(contentHash, locallyPossiblyDeletedFiles); + if (matchingDeletedFile !== undefined) { logger.debug( - `Document ${relativePath} might have been updated locally, scheduling sync to validate and update it`, + `File ${path} might have been moved from ${matchingDeletedFile.path} while offline, scheduling sync to move it`, ); - instructions.push({ type: "update", relativePath }); - continue; + enqueueUpdate({ oldPath: matchingDeletedFile.path, relativePath: path }); + removeFromArray(locallyPossiblyDeletedFiles, matchingDeletedFile); + removeFromArray(locallyPossibleCreatedFiles, path); } - - const result = await handleNewFile(deps, relativePath, deletedCandidates); - if (result.updatedDeletedCandidates !== undefined) { - deletedCandidates = result.updatedDeletedCandidates; - } - instructions.push(result.instruction); } - return instructions; -} - -async function handleExistingDocument( - { logger, operations }: OfflineChangeDetectorDeps, - relativePath: RelativePath, - existingRecord: DocumentRecord, - deletedCandidates: DocumentWithPath[], -): Promise< - | { instruction?: SyncInstruction; updatedDeletedCandidates?: DocumentWithPath[] } - | undefined -> { - if (deletedCandidates.length === 0) { - return undefined; - } - - let contentHash: string | undefined; - try { - const bytes = await operations.read(relativePath); - contentHash = await hash(bytes); - } catch (e) { - if (e instanceof FileNotFoundError) return { instruction: undefined }; - throw e; - } - - if (contentHash === existingRecord.remoteHash) { - return undefined; - } - - const originalFile = await findMatchingFile(contentHash, deletedCandidates); - if (originalFile === undefined) { - return undefined; - } - - // This file was moved here from a different path, displacing the existing document - const updatedDeletedCandidates = [ - ...deletedCandidates.filter((item) => item.path !== originalFile.path), - { path: relativePath, record: existingRecord }, - ]; - - logger.debug( - `Document '${originalFile.path}' was moved to ${relativePath} (displacing existing document), scheduling sync to move it`, - ); - - return { - instruction: { type: "update", oldPath: originalFile.path, relativePath }, - updatedDeletedCandidates, - }; -} - -async function handleNewFile( - { logger, operations }: OfflineChangeDetectorDeps, - relativePath: RelativePath, - deletedCandidates: DocumentWithPath[], -): Promise<{ instruction: SyncInstruction; updatedDeletedCandidates?: DocumentWithPath[] }> { - let contentHash: string | undefined; - try { - const contentBytes = await operations.read(relativePath); - contentHash = await hash(contentBytes); - } catch (e) { - if (e instanceof FileNotFoundError) { - return { instruction: { type: "create", relativePath } }; - } - throw e; - } - - const originalFile = await findMatchingFile(contentHash, deletedCandidates); - if (originalFile !== undefined) { - const updatedDeletedCandidates = deletedCandidates.filter( - (item) => item.path !== originalFile.path, - ); - - logger.debug( - `Document '${originalFile.path}' was not found under its current path in the database but was found under a different path (${relativePath}), scheduling sync to move it`, - ); - - return { - instruction: { type: "update", oldPath: originalFile.path, relativePath }, - updatedDeletedCandidates, - }; - } - - logger.debug(`Document ${relativePath} not found in database, scheduling sync to create it`); - return { instruction: { type: SyncEventType.LocalCreate, relativePath } }; + for (const path of locallyPossibleCreatedFiles) { + logger.debug(`File ${path} was created while offline, scheduling sync to create it`); + enqueueCreate(path); + } + + for (const item of locallyPossiblyDeletedFiles) { + enqueueDelete(item.path); + } + + for (const path of syncedLocalFiles) { + enqueueUpdate({ relativePath: path }); + } } diff --git a/frontend/sync-client/src/sync-operations/sync-event-queue.test.ts b/frontend/sync-client/src/sync-operations/sync-event-queue.test.ts index 23f31891..3e644057 100644 --- a/frontend/sync-client/src/sync-operations/sync-event-queue.test.ts +++ b/frontend/sync-client/src/sync-operations/sync-event-queue.test.ts @@ -316,7 +316,7 @@ describe("SyncEventQueue", () => { assert.ok(promiseA !== undefined); assert.ok(promiseB !== undefined); - queue.clear(); + queue.clearPending(); await assert.rejects(promiseA); await assert.rejects(promiseB); @@ -360,7 +360,7 @@ describe("SyncEventQueue", () => { assert.strictEqual(queue.pendingUpdateCount, 2); - queue.clear(); + queue.clearPending(); assert.strictEqual(queue.pendingUpdateCount, 0); assert.strictEqual(queue.syncedDocumentCount, 1); diff --git a/frontend/sync-client/src/sync-operations/sync-event-queue.ts b/frontend/sync-client/src/sync-operations/sync-event-queue.ts index 401541b1..a65c2fbd 100644 --- a/frontend/sync-client/src/sync-operations/sync-event-queue.ts +++ b/frontend/sync-client/src/sync-operations/sync-event-queue.ts @@ -4,6 +4,7 @@ import { globsToRegexes } from "../utils/globs-to-regexes"; import { CONFLICT_PATH_REGEX } from "./conflict-path"; import { removeFromArray } from "../utils/remove-from-array"; import { + DocumentWithPath, SyncEventType, type DocumentId, type DocumentRecord, @@ -50,7 +51,7 @@ export class SyncEventQueue { private readonly saveData: (data: StoredSyncState) => Promise ) { this.ignorePatterns = [ - CONFLICT_PATH_REGEX, + CONFLICT_PATH_REGEX, // conflict paths need to be resolved before they can be synced again ...globsToRegexes( this.settings.getSettings().ignorePatterns, this.logger @@ -85,12 +86,7 @@ export class SyncEventQueue { } public enqueue(input: FileSyncEvent): void { - if (input.type === SyncEventType.RemoteChange) { - this.events.push(input); - return; - } - - const { path } = input; + const path = (input.type === SyncEventType.RemoteChange) ? input.remoteVersion.relativePath : input.path; if (this.ignorePatterns.some((pattern) => pattern.test(path))) { this.logger.info( @@ -99,6 +95,12 @@ export class SyncEventQueue { return; } + if (input.type === SyncEventType.RemoteChange) { + this.events.push(input); + return; + } + + if (input.type === SyncEventType.LocalCreate) { this.events.push({ type: SyncEventType.LocalCreate, path, originalPath: path, resolvers: Promise.withResolvers() }); return; @@ -111,7 +113,6 @@ export class SyncEventQueue { if (documentId === undefined) { // we can get here when deleting a local document after a remote update - return; } @@ -173,7 +174,7 @@ export class SyncEventQueue { public getDocumentByDocumentId( target: DocumentId - ): { path: RelativePath; record: DocumentRecord } | undefined { + ): DocumentWithPath | undefined { for (const [path, record] of this.documents) { if (record.documentId === target) { return { path, record }; @@ -186,7 +187,7 @@ export class SyncEventQueue { public getDocumentByDocumentIdOrFail( target: DocumentId - ): { path: RelativePath; record: DocumentRecord } { + ): DocumentWithPath { const result = this.getDocumentByDocumentId(target); if (!result) { throw new Error(`No document found with id ${target}`); @@ -215,48 +216,10 @@ export class SyncEventQueue { return this.documents.get(path); } - - - - - public allSettledDocuments(): [RelativePath, DocumentRecord][] { - return Array.from(this.documents.entries()); + public allSettledDocuments(): Map { + return new Map(this.documents.entries()); } - /** - * Returns the set of paths we expect to exist on disk by replaying - * the event queue on top of the settled documents map. - */ - public trackedPaths(): Set { - const paths = new Set(this.documents.keys()); - // Track current path for each pending create so moves can be applied - const pendingPaths = new Map, RelativePath>(); - - for (const event of this.events) { - if (event.type === SyncEventType.LocalCreate) { - paths.add(event.path); - if (event.resolvers !== undefined) { - pendingPaths.set(event.resolvers.promise, event.path); - } - } else if (event.type === SyncEventType.LocalDelete) { - if (typeof event.documentId === "string") { - const path = this.getDocumentByDocumentId(event.documentId)?.path; - if (path) { - paths.delete(path); - } else { - throw new Error(`Delete event for unknown documentId ${event.documentId}`); - } - } else { - const path = pendingPaths.get(event.documentId); - if (!path) { - throw new Error(`Delete event with unresolved documentId promise`); - } - paths.delete(path); - } - } // no need to handle SyncLocal as path updates are applied to this.documents immediately when the event is enqueued - } - return paths; - } public hasPendingEventsForPath(path: RelativePath): boolean { const record = this.documents.get(path); @@ -288,36 +251,19 @@ export class SyncEventQueue { } - public resetState(): void { - this.rejectAllPendingCreates(); + public async clearAllState(): Promise { + this.clearPending(); this.documents.clear(); - this.saveInTheBackground(); + this.lastSeenUpdateId = -1; + await this.save(); } - public clear(): void { + public clearPending(): void { this.rejectAllPendingCreates(); this.events.length = 0; } - - public removeAllEventsForDocumentId(documentId: DocumentId): void { - for (let i = this.events.length - 1; i >= 0; i--) { - const e = this.events[i]; - if ( - (e.type === SyncEventType.LocalUpdate && - e.documentId === documentId) || - (e.type === SyncEventType.RemoteChange && - e.remoteVersion.documentId === documentId) || - (e.type === SyncEventType.LocalDelete && - e.documentId === documentId) - ) { - // eslint-disable-next-line no-restricted-syntax -- Bulk removal by predicate, not single-item removal - this.events.splice(i, 1); - } - } - } - private updatePendingCreatePath( oldPath: RelativePath, newPath: RelativePath diff --git a/frontend/sync-client/src/sync-operations/syncer.ts b/frontend/sync-client/src/sync-operations/syncer.ts index 3dbcd6cf..bf488749 100644 --- a/frontend/sync-client/src/sync-operations/syncer.ts +++ b/frontend/sync-client/src/sync-operations/syncer.ts @@ -154,7 +154,7 @@ export class Syncer { public reset(): void { this._isFirstSyncStarted = false; - this.queue.clear(); + this.queue.clearPending(); // Don't null the reference synchronously — if the scan is // still in flight, the next reconnect would spawn a second // concurrent scan racing on the same queue. Defer the diff --git a/frontend/sync-client/src/sync-operations/types.ts b/frontend/sync-client/src/sync-operations/types.ts index 9642665a..9d2cedac 100644 --- a/frontend/sync-client/src/sync-operations/types.ts +++ b/frontend/sync-client/src/sync-operations/types.ts @@ -11,6 +11,11 @@ export interface DocumentRecord { remoteRelativePath: RelativePath; } +export interface DocumentWithPath { + path: RelativePath; + record: DocumentRecord; +} + export interface StoredDocument extends DocumentRecord { relativePath: RelativePath; } @@ -29,7 +34,9 @@ export enum SyncEventType { export type FileSyncEvent = | { type: SyncEventType.LocalCreate; path: RelativePath } - | { type: SyncEventType.LocalUpdate; path: RelativePath; oldPath?: RelativePath } + | { + type: SyncEventType.LocalUpdate; path: RelativePath; oldPath?: RelativePath // oldPath is undefined for content changes + } | { type: SyncEventType.LocalDelete; path: RelativePath } | { type: SyncEventType.RemoteChange; remoteVersion: DocumentVersionWithoutContent }; diff --git a/frontend/sync-client/src/utils/find-matching-file.ts b/frontend/sync-client/src/utils/find-matching-file.ts index 4c748925..1b8df384 100644 --- a/frontend/sync-client/src/utils/find-matching-file.ts +++ b/frontend/sync-client/src/utils/find-matching-file.ts @@ -1,11 +1,11 @@ -import type { DocumentRecord, RelativePath } from "../sync-operations/types"; +import type { DocumentRecord, DocumentWithPath, RelativePath } from "../sync-operations/types"; import { EMPTY_HASH } from "./hash"; // TODO: make this smarter so that offline files can be renamed & edited at the same time export async function findMatchingFile( contentHash: string, candidates: { path: RelativePath; record: DocumentRecord }[] -): Promise<{ path: RelativePath; record: DocumentRecord } | undefined> { +): Promise { if (contentHash === await EMPTY_HASH) { return undefined; }