diff --git a/frontend/sync-client/src/file-operations/file-operations.test.ts b/frontend/sync-client/src/file-operations/file-operations.test.ts new file mode 100644 index 00000000..85364eb2 --- /dev/null +++ b/frontend/sync-client/src/file-operations/file-operations.test.ts @@ -0,0 +1,102 @@ +import { FileSystemOperations } from "sync-client"; +import type { RelativePath } from "../persistence/database"; +import { FileOperations } from "./file-operations"; +import { Logger } from "../tracing/logger"; +import assert from "assert"; + +describe("File operations", () => { + class FakeFileSystemOperations implements FileSystemOperations { + public readonly names = new Set(); + + public listAllFiles(): Promise { + throw new Error("Method not implemented."); + } + public read(path: RelativePath): Promise { + throw new Error("Method not implemented."); + } + public async write( + path: RelativePath, + _content: Uint8Array + ): Promise { + this.names.add(path); + } + public atomicUpdateText( + path: RelativePath, + updater: (currentContent: string) => string + ): Promise { + throw new Error("Method not implemented."); + } + public getFileSize(path: RelativePath): Promise { + throw new Error("Method not implemented."); + } + public getModificationTime(path: RelativePath): Promise { + throw new Error("Method not implemented."); + } + public async exists(path: RelativePath): Promise { + return this.names.has(path); + } + public async createDirectory(path: RelativePath): Promise {} + public delete(path: RelativePath): Promise { + throw new Error("Method not implemented."); + } + public async rename( + oldPath: RelativePath, + newPath: RelativePath + ): Promise { + this.names.delete(oldPath); + this.names.add(newPath); + } + } + + test("should deconflict renames", async () => { + let fs = new FakeFileSystemOperations(); + let fileOperations = new FileOperations(new Logger(), fs); + + await fileOperations.create("a", new Uint8Array()); + assertSetOnlyContains(fs.names, "a"); + await fileOperations.move("a", "b"); + assertSetOnlyContains(fs.names, "b"); + + await fileOperations.create("c", new Uint8Array()); + assertSetOnlyContains(fs.names, "b", "c"); + + await fileOperations.move("c", "b"); + assertSetOnlyContains(fs.names, "b", "b (1)"); + + await fileOperations.create("c", new Uint8Array()); + await fileOperations.move("c", "b"); + assertSetOnlyContains(fs.names, "b", "b (1)", "b (2)"); + }); + + test("should deconflict renames with file extension", async () => { + let fs = new FakeFileSystemOperations(); + let fileOperations = new FileOperations(new Logger(), fs); + + await fileOperations.create("b.md", new Uint8Array()); + await fileOperations.create("c.md", new Uint8Array()); + await fileOperations.move("c.md", "b.md"); + assertSetOnlyContains(fs.names, "b.md", "b (1).md"); + }); + + test("should deconflict renames with paths", async () => { + let fs = new FakeFileSystemOperations(); + let fileOperations = new FileOperations(new Logger(), fs); + + await fileOperations.create("a/b.c/d", new Uint8Array()); + await fileOperations.create("a/b.c/e", new Uint8Array()); + await fileOperations.move("a/b.c/d", "a/b.c/e"); + assertSetOnlyContains(fs.names, "a/b.c/e", "a/b.c/e (1)"); + }); +}); + +function assertSetOnlyContains(set: Set, ...values: T[]) { + assert( + set.size === values.length && + Array.from(set).every((value) => values.includes(value)), + `Expected set to contain only ${values.map((v) => '"' + v + '"').join(", ")}, but it contained ${Array.from( + set + ) + .map((v) => '"' + v + '"') + .join(", ")}` + ); +} diff --git a/frontend/sync-client/src/file-operations/file-operations.ts b/frontend/sync-client/src/file-operations/file-operations.ts index 4bd29402..f994a2ed 100644 --- a/frontend/sync-client/src/file-operations/file-operations.ts +++ b/frontend/sync-client/src/file-operations/file-operations.ts @@ -21,7 +21,6 @@ export class FileOperations { } public async read(path: RelativePath): Promise { - this.logger.debug(`Reading file: ${path}`); const content = await this.fs.read(path); if (isBinary(content)) { @@ -38,17 +37,14 @@ export class FileOperations { } public async getFileSize(path: RelativePath): Promise { - this.logger.debug(`Getting file size: ${path}`); return this.fs.getFileSize(path); } public async getModificationTime(path: RelativePath): Promise { - this.logger.debug(`Getting modification time: ${path}`); return this.fs.getModificationTime(path); } public async exists(path: RelativePath): Promise { - this.logger.debug(`Checking existence of ${path}`); return this.fs.exists(path); } @@ -58,7 +54,6 @@ export class FileOperations { path: RelativePath, newContent: Uint8Array ): Promise { - this.logger.debug(`Creating file: ${path}`); if (await this.fs.exists(path)) { this.logger.debug( `Didn't expect ${path} to exist, when trying to create it, merging instead` @@ -79,7 +74,6 @@ export class FileOperations { expectedContent: Uint8Array, newContent: Uint8Array ): Promise { - this.logger.debug(`Writing file: ${path}`); if (!(await this.fs.exists(path))) { this.logger.debug( `The caller assumed ${path} exists, but it no longer, so we wont recreate it` @@ -133,24 +127,25 @@ export class FileOperations { oldPath: RelativePath, newPath: RelativePath ): Promise { - this.logger.debug(`Moving file: ${oldPath} -> ${newPath}`); - if (oldPath === newPath) { return; } - await this.createParentDirectories(newPath); + 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}'` + ); + this.fs.rename(newPath, deconflictedPath); + } else { + await this.createParentDirectories(newPath); + } + await this.fs.rename(oldPath, newPath); } - public isFileEligibleForSync(_path: RelativePath): boolean { - return true; - // TODO: figure this out - // if (Platform.isDesktopApp) { - // return true; - // } - - // return isFileTypeMergable(path); + public isFileEligibleForSync(path: RelativePath): boolean { + return isFileTypeMergable(path); } private async createParentDirectories(path: string): Promise { @@ -165,4 +160,34 @@ export class FileOperations { } } } + + private async deconflictPath(path: RelativePath): Promise { + const pathParts = path.split("/"); + const fileName = pathParts.pop()!; + + let directory = pathParts.join("/"); + if (directory) { + directory += "/"; + } + + const nameParts = fileName.split("."); + const extension = + nameParts.length > 1 ? "." + nameParts[nameParts.length - 1] : ""; + const stem = extension ? nameParts.slice(0, -1).join(".") : fileName; + let currentCount = Number.parseInt( + / \((\d+)\)$/.exec(stem)?.groups?.[0] ?? "0" + ); + + while (true) { + const newName = + currentCount === 0 + ? `${directory}${stem}${extension}` + : `${directory}${stem} (${currentCount})${extension}`; + if (await this.fs.exists(newName)) { + currentCount++; + } else { + return newName; + } + } + } }