From a7b518d7eae388133a2a90096c6499277a5d2608 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Mon, 24 Feb 2025 23:01:24 +0000 Subject: [PATCH] Fix deconflicting --- .../file-operations/file-operations.test.ts | 18 ++++++++++++++++- .../src/file-operations/file-operations.ts | 20 +++++++++++++------ frontend/sync-client/src/sync-client.ts | 2 +- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/frontend/sync-client/src/file-operations/file-operations.test.ts b/frontend/sync-client/src/file-operations/file-operations.test.ts index 1fd99ae..2e7c57b 100644 --- a/frontend/sync-client/src/file-operations/file-operations.test.ts +++ b/frontend/sync-client/src/file-operations/file-operations.test.ts @@ -2,7 +2,7 @@ import type { FileSystemOperations } from "sync-client"; import type { Database, RelativePath } from "../persistence/database"; import { FileOperations } from "./file-operations"; import { Logger } from "../tracing/logger"; -import { assertSetContainsExactly } from "src/utils/assert-set-contains-exactly"; +import { assertSetContainsExactly } from "../utils/assert-set-contains-exactly"; describe("File operations", () => { class MockDatabase { @@ -95,6 +95,22 @@ describe("File operations", () => { await fileOperations.create("c.md", new Uint8Array()); await fileOperations.move("c.md", "b.md"); assertSetContainsExactly(fs.names, "b.md", "b (1).md"); + + await fileOperations.create("d.md", new Uint8Array()); + await fileOperations.move("d.md", "b.md"); + assertSetContainsExactly(fs.names, "b.md", "b (1).md", "b (2).md"); + + await fileOperations.create("file-23.md", new Uint8Array()); + await fileOperations.create("file-23 (1).md", new Uint8Array()); + await fileOperations.move("file-23.md", "file-23 (1).md"); + assertSetContainsExactly( + fs.names, + "b.md", + "b (1).md", + "b (2).md", + "file-23 (1).md", + "file-23 (2).md" + ); }); test("should deconflict renames with paths", async () => { diff --git a/frontend/sync-client/src/file-operations/file-operations.ts b/frontend/sync-client/src/file-operations/file-operations.ts index f994a2e..d93cede 100644 --- a/frontend/sync-client/src/file-operations/file-operations.ts +++ b/frontend/sync-client/src/file-operations/file-operations.ts @@ -1,14 +1,16 @@ import type { Logger } from "src/tracing/logger"; import type { FileSystemOperations } from "./filesystem-operations"; -import type { RelativePath } from "src/persistence/database"; +import type { Database, RelativePath } from "src/persistence/database"; import { isBinary, isFileTypeMergable, mergeText } from "sync_lib"; import { SafeFileSystemOperations } from "./safe-filesystem-operations"; export class FileOperations { private readonly fs: SafeFileSystemOperations; + private static readonly PARENTHESES_REGEX = / \((\d+)\)$/; public constructor( private readonly logger: Logger, + private readonly database: Database, fs: FileSystemOperations ) { this.fs = new SafeFileSystemOperations(fs); @@ -134,9 +136,10 @@ export class FileOperations { if (await this.fs.exists(newPath)) { const deconflictedPath = await this.deconflictPath(newPath); this.logger.debug( - `Conflict when moving '${oldPath}' to '${newPath}', '${newPath}' already exists, deconflicting by moving it to '${deconflictedPath}'` + `Conflict when moving '${oldPath}' to '${newPath}', latter already exists, deconflicting by moving it to '${deconflictedPath}'` ); - this.fs.rename(newPath, deconflictedPath); + await this.database.updatePath(newPath, deconflictedPath); + await this.fs.rename(newPath, deconflictedPath); } else { await this.createParentDirectories(newPath); } @@ -163,7 +166,10 @@ export class FileOperations { private async deconflictPath(path: RelativePath): Promise { const pathParts = path.split("/"); - const fileName = pathParts.pop()!; + const fileName = pathParts.pop(); + if (fileName == "" || fileName == null) { + throw new Error(`Path '${path}' cannot be empty`); + } let directory = pathParts.join("/"); if (directory) { @@ -173,11 +179,13 @@ export class FileOperations { const nameParts = fileName.split("."); const extension = nameParts.length > 1 ? "." + nameParts[nameParts.length - 1] : ""; - const stem = extension ? nameParts.slice(0, -1).join(".") : fileName; + let stem = extension ? nameParts.slice(0, -1).join(".") : fileName; let currentCount = Number.parseInt( - / \((\d+)\)$/.exec(stem)?.groups?.[0] ?? "0" + FileOperations.PARENTHESES_REGEX.exec(stem)?.groups?.[0] ?? "0" ); + stem = stem.replace(FileOperations.PARENTHESES_REGEX, ""); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition while (true) { const newName = currentCount === 0 diff --git a/frontend/sync-client/src/sync-client.ts b/frontend/sync-client/src/sync-client.ts index 0325962..2338808 100644 --- a/frontend/sync-client/src/sync-client.ts +++ b/frontend/sync-client/src/sync-client.ts @@ -93,7 +93,7 @@ export class SyncClient { database, settings, syncService, - new FileOperations(logger, fs), + new FileOperations(logger, database, fs), history );