From a015737581d68a3d5d503a20bafb609139057a90 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Fri, 28 Mar 2025 22:14:37 +0000 Subject: [PATCH] Make locks generic --- .../src/file-operations/document-locks.ts | 65 ------------------- .../locks.test.ts} | 62 +++++++++--------- frontend/sync-client/src/utils/locks.ts | 60 +++++++++++++++++ 3 files changed, 89 insertions(+), 98 deletions(-) delete mode 100644 frontend/sync-client/src/file-operations/document-locks.ts rename frontend/sync-client/src/{file-operations/document-locks.test.ts => utils/locks.test.ts} (55%) create mode 100644 frontend/sync-client/src/utils/locks.ts diff --git a/frontend/sync-client/src/file-operations/document-locks.ts b/frontend/sync-client/src/file-operations/document-locks.ts deleted file mode 100644 index 522ed02a..00000000 --- a/frontend/sync-client/src/file-operations/document-locks.ts +++ /dev/null @@ -1,65 +0,0 @@ -import type { Logger } from "../tracing/logger"; -import type { RelativePath } from "../persistence/database"; - -// Manages locks on documents to prevent concurrent modifications -// allowing the client's FileOperations implementation to be simpler. -// Locks are granted in a first-in-first-out order. -export class DocumentLocks { - private readonly locked = new Set(); - private readonly waiters = new Map void)[]>(); - - public constructor(private readonly logger: Logger) {} - - public tryLockDocument(relativePath: RelativePath): boolean { - if (this.locked.has(relativePath)) { - return false; - } - - this.locked.add(relativePath); - - return true; - } - - public async waitForDocumentLock( - relativePath: RelativePath - ): Promise { - if (this.tryLockDocument(relativePath)) { - return Promise.resolve(); - } - - this.logger.debug(`Waiting for lock on ${relativePath}`); - - return new Promise((resolve) => { - let waiting = this.waiters.get(relativePath); - if (!waiting) { - waiting = []; - this.waiters.set(relativePath, waiting); - } - - waiting.push(resolve); - }); - } - - public unlockDocument(relativePath: RelativePath): void { - if (!this.locked.has(relativePath)) { - throw new Error( - `Document ${relativePath} is not locked, cannot unlock` - ); - } - - // Remove the first element to ensure FIFO unblocking order - const nextWaiting = this.waiters.get(relativePath)?.shift(); - - if (nextWaiting) { - this.logger.debug(`Granted lock on ${relativePath}`); - nextWaiting(); - } else { - this.locked.delete(relativePath); - } - } - - public reset(): void { - this.locked.clear(); - this.waiters.clear(); - } -} diff --git a/frontend/sync-client/src/file-operations/document-locks.test.ts b/frontend/sync-client/src/utils/locks.test.ts similarity index 55% rename from frontend/sync-client/src/file-operations/document-locks.test.ts rename to frontend/sync-client/src/utils/locks.test.ts index 1b8394ba..f545e957 100644 --- a/frontend/sync-client/src/file-operations/document-locks.test.ts +++ b/frontend/sync-client/src/utils/locks.test.ts @@ -1,92 +1,88 @@ import { Logger } from "../tracing/logger"; import type { RelativePath } from "../persistence/database"; -import { DocumentLocks } from "./document-locks"; +import { Locks } from "./locks"; describe("Document lock", () => { const testPath: RelativePath = "test/document/path"; const logger = new Logger(); - let locks = new DocumentLocks(logger); + + // eslint-disable-next-line @typescript-eslint/init-declarations + let locks: Locks; beforeEach(() => { - locks = new DocumentLocks(logger); + locks = new Locks(logger); }); test("should lock a document successfully", () => { - const result = locks.tryLockDocument(testPath); + const result = locks.tryLock(testPath); expect(result).toBe(true); }); test("should not lock a document that is already locked", () => { - locks.tryLockDocument(testPath); - const result = locks.tryLockDocument(testPath); + locks.tryLock(testPath); + const result = locks.tryLock(testPath); expect(result).toBe(false); }); test("should unlock a locked document", () => { - locks.tryLockDocument(testPath); - locks.unlockDocument(testPath); - const result = locks.tryLockDocument(testPath); + locks.tryLock(testPath); + locks.unlock(testPath); + const result = locks.tryLock(testPath); expect(result).toBe(true); - locks.unlockDocument(testPath); + locks.unlock(testPath); }); test("should throw an error when unlocking a document that is not locked", () => { expect(() => { - locks.unlockDocument(testPath); + locks.unlock(testPath); }).toThrow(`Document ${testPath} is not locked, cannot unlock`); }); test("should wait for a document lock and resolve when unlocked", async () => { - locks.tryLockDocument(testPath); + locks.tryLock(testPath); let resolved = false; - const waitPromise = locks.waitForDocumentLock(testPath).then(() => { + const waitPromise = locks.waitForLock(testPath).then(() => { resolved = true; }); - locks.unlockDocument(testPath); + locks.unlock(testPath); await waitPromise; expect(resolved).toBe(true); }); test("should resolve multiple waiters in FIFO order", async () => { - locks.tryLockDocument(testPath); + locks.tryLock(testPath); let firstResolved = false; let secondResolved = false; let thirdResolved = false; - const firstWaitPromise = locks - .waitForDocumentLock(testPath) - .then(() => { - firstResolved = true; - }); + const firstWaitPromise = locks.waitForLock(testPath).then(() => { + firstResolved = true; + }); - const secondWaitPromise = locks - .waitForDocumentLock(testPath) - .then(() => { - secondResolved = true; - }); + const secondWaitPromise = locks.waitForLock(testPath).then(() => { + secondResolved = true; + }); - const thirdWaitPromise = locks - .waitForDocumentLock(testPath) - .then(() => { - thirdResolved = true; - }); + const thirdWaitPromise = locks.waitForLock(testPath).then(() => { + thirdResolved = true; + }); - locks.unlockDocument(testPath); + locks.unlock(testPath); await firstWaitPromise; expect(firstResolved).toBe(true); expect(secondResolved).toBe(false); expect(thirdResolved).toBe(false); - locks.unlockDocument(testPath); + locks.unlock(testPath); await secondWaitPromise; expect(secondResolved).toBe(true); expect(thirdResolved).toBe(false); - locks.unlockDocument(testPath); + locks.unlock(testPath); await thirdWaitPromise; expect(thirdResolved).toBe(true); }); diff --git a/frontend/sync-client/src/utils/locks.ts b/frontend/sync-client/src/utils/locks.ts new file mode 100644 index 00000000..542f8a88 --- /dev/null +++ b/frontend/sync-client/src/utils/locks.ts @@ -0,0 +1,60 @@ +import type { Logger } from "../tracing/logger"; + +// Manages locks on T to prevent concurrent modifications +// allowing the client's FileOperations implementation to be simpler. +// Locks are granted in a first-in-first-out order. +export class Locks { + private readonly locked = new Set(); + private readonly waiters = new Map void)[]>(); + + public constructor(private readonly logger: Logger) {} + + public tryLock(key: T): boolean { + if (this.locked.has(key)) { + return false; + } + + this.locked.add(key); + + return true; + } + + public async waitForLock(key: T): Promise { + if (this.tryLock(key)) { + return Promise.resolve(); + } + + this.logger.debug(`Waiting for lock on ${key}`); + + return new Promise((resolve) => { + let waiting = this.waiters.get(key); + if (!waiting) { + waiting = []; + this.waiters.set(key, waiting); + } + + waiting.push(resolve); + }); + } + + public unlock(key: T): void { + if (!this.locked.has(key)) { + throw new Error(`Document ${key} is not locked, cannot unlock`); + } + + // Remove the first element to ensure FIFO unblocking order + const nextWaiting = this.waiters.get(key)?.shift(); + + if (nextWaiting) { + this.logger.debug(`Granted lock on ${key}`); + nextWaiting(); + } else { + this.locked.delete(key); + } + } + + public reset(): void { + this.locked.clear(); + this.waiters.clear(); + } +}