From b61d46900fe70d58dc0817b00c3c9ba5fae800d3 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Thu, 28 Jan 2021 11:13:46 +0100 Subject: [PATCH] feat: Create new interface for lockers with only 1 kind of lock --- config/presets/storage-wrapper.json | 11 ++- src/index.ts | 6 +- src/storage/LockingResourceStore.ts | 6 +- src/util/locking/EqualReadWriteLocker.ts | 37 ++++++++ ...ceLocker.ts => ExpiringReadWriteLocker.ts} | 12 +-- src/util/locking/ReadWriteLocker.ts | 30 ++++++ src/util/locking/ResourceLocker.ts | 31 +++---- .../locking/SingleThreadedResourceLocker.ts | 54 ++++++----- ...r.ts => WrappedExpiringReadWriteLocker.ts} | 12 +-- test/integration/LockingResourceStore.test.ts | 15 +-- .../unit/storage/LockingResourceStore.test.ts | 4 +- .../util/locking/EqualReadWriteLocker.test.ts | 62 +++++++++++++ .../SingleThreadedResourceLocker.test.ts | 91 ++++++++++--------- ...=> WrappedExpiringReadWriteLocker.test.ts} | 12 +-- 14 files changed, 264 insertions(+), 119 deletions(-) create mode 100644 src/util/locking/EqualReadWriteLocker.ts rename src/util/locking/{ExpiringResourceLocker.ts => ExpiringReadWriteLocker.ts} (70%) create mode 100644 src/util/locking/ReadWriteLocker.ts rename src/util/locking/{WrappedExpiringResourceLocker.ts => WrappedExpiringReadWriteLocker.ts} (85%) create mode 100644 test/unit/util/locking/EqualReadWriteLocker.test.ts rename test/unit/util/locking/{WrappedExpiringResourceLocker.test.ts => WrappedExpiringReadWriteLocker.test.ts} (88%) diff --git a/config/presets/storage-wrapper.json b/config/presets/storage-wrapper.json index 369a01faa..a53517c82 100644 --- a/config/presets/storage-wrapper.json +++ b/config/presets/storage-wrapper.json @@ -49,11 +49,14 @@ { "@id": "urn:solid-server:default:ResourceLocker", - "@type": "WrappedExpiringResourceLocker", - "WrappedExpiringResourceLocker:_locker": { - "@type": "SingleThreadedResourceLocker" + "@type": "WrappedExpiringReadWriteLocker", + "WrappedExpiringReadWriteLocker:_locker": { + "@type": "EqualReadWriteLocker", + "EqualReadWriteLocker:_locker": { + "@type": "SingleThreadedResourceLocker" + } }, - "WrappedExpiringResourceLocker:_expiration": 3000 + "WrappedExpiringReadWriteLocker:_expiration": 3000 }, diff --git a/src/index.ts b/src/index.ts index 2c18b7a37..5a2847f9a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -199,10 +199,12 @@ export * from './util/identifiers/IdentifierStrategy'; export * from './util/identifiers/SingleRootIdentifierStrategy'; // Util/Locking -export * from './util/locking/ExpiringResourceLocker'; +export * from './util/locking/ExpiringReadWriteLocker'; +export * from './util/locking/EqualReadWriteLocker'; +export * from './util/locking/ReadWriteLocker'; export * from './util/locking/ResourceLocker'; export * from './util/locking/SingleThreadedResourceLocker'; -export * from './util/locking/WrappedExpiringResourceLocker'; +export * from './util/locking/WrappedExpiringReadWriteLocker'; // Util export * from './util/ContentTypes'; diff --git a/src/storage/LockingResourceStore.ts b/src/storage/LockingResourceStore.ts index 1a8bf267f..0971c0916 100644 --- a/src/storage/LockingResourceStore.ts +++ b/src/storage/LockingResourceStore.ts @@ -5,7 +5,7 @@ import type { Representation } from '../ldp/representation/Representation'; import type { RepresentationPreferences } from '../ldp/representation/RepresentationPreferences'; import type { ResourceIdentifier } from '../ldp/representation/ResourceIdentifier'; import { getLoggerFor } from '../logging/LogUtil'; -import type { ExpiringResourceLocker } from '../util/locking/ExpiringResourceLocker'; +import type { ExpiringReadWriteLocker } from '../util/locking/ExpiringReadWriteLocker'; import type { AtomicResourceStore } from './AtomicResourceStore'; import type { Conditions } from './Conditions'; import type { ResourceStore } from './ResourceStore'; @@ -19,9 +19,9 @@ export class LockingResourceStore implements AtomicResourceStore { protected readonly logger = getLoggerFor(this); private readonly source: ResourceStore; - private readonly locks: ExpiringResourceLocker; + private readonly locks: ExpiringReadWriteLocker; - public constructor(source: ResourceStore, locks: ExpiringResourceLocker) { + public constructor(source: ResourceStore, locks: ExpiringReadWriteLocker) { this.source = source; this.locks = locks; } diff --git a/src/util/locking/EqualReadWriteLocker.ts b/src/util/locking/EqualReadWriteLocker.ts new file mode 100644 index 000000000..33b216063 --- /dev/null +++ b/src/util/locking/EqualReadWriteLocker.ts @@ -0,0 +1,37 @@ +import type { ResourceIdentifier } from '../../ldp/representation/ResourceIdentifier'; +import type { ReadWriteLocker } from './ReadWriteLocker'; +import type { ResourceLocker } from './ResourceLocker'; + +/** + * A {@link ReadWriteLocker} that gives no priority to read or write operations: both use the same lock. + */ +export class EqualReadWriteLocker implements ReadWriteLocker { + private readonly locker: ResourceLocker; + + public constructor(locker: ResourceLocker) { + this.locker = locker; + } + + public async withReadLock(identifier: ResourceIdentifier, whileLocked: () => (Promise | T)): Promise { + return this.withLock(identifier, whileLocked); + } + + public async withWriteLock(identifier: ResourceIdentifier, whileLocked: () => (Promise | T)): Promise { + return this.withLock(identifier, whileLocked); + } + + /** + * Acquires a new lock for the requested identifier. + * Will resolve when the input function resolves. + * @param identifier - Identifier of resource that needs to be locked. + * @param whileLocked - Function to resolve while the resource is locked. + */ + private async withLock(identifier: ResourceIdentifier, whileLocked: () => T | Promise): Promise { + await this.locker.acquire(identifier); + try { + return await whileLocked(); + } finally { + await this.locker.release(identifier); + } + } +} diff --git a/src/util/locking/ExpiringResourceLocker.ts b/src/util/locking/ExpiringReadWriteLocker.ts similarity index 70% rename from src/util/locking/ExpiringResourceLocker.ts rename to src/util/locking/ExpiringReadWriteLocker.ts index 76b044bfd..562d32120 100644 --- a/src/util/locking/ExpiringResourceLocker.ts +++ b/src/util/locking/ExpiringReadWriteLocker.ts @@ -1,12 +1,12 @@ import type { ResourceIdentifier } from '../../ldp/representation/ResourceIdentifier'; -import type { ResourceLocker } from './ResourceLocker'; +import type { ReadWriteLocker } from './ReadWriteLocker'; /** - * A {@link ResourceLocker} where the locks expire after a given time. + * A {@link ReadWriteLocker} where the locks expire after a given time. */ -export interface ExpiringResourceLocker extends ResourceLocker { +export interface ExpiringReadWriteLocker extends ReadWriteLocker { /** - * As {@link ResourceLocker.withReadLock} but the locked function gets called with a `maintainLock` callback function + * As {@link ReadWriteLocker.withReadLock} but the locked function gets called with a `maintainLock` callback function * to reset the lock expiration every time it is called. * The resulting promise will reject once the lock expires. * @@ -18,8 +18,8 @@ export interface ExpiringResourceLocker extends ResourceLocker { => Promise; /** - * As {@link ResourceLocker.withWriteLock} but the locked function gets called with a `maintainLock` callback function - * to reset the lock expiration every time it is called. + * As {@link ReadWriteLocker.withWriteLock} but the locked function gets called with a `maintainLock` + * callback function to reset the lock expiration every time it is called. * The resulting promise will reject once the lock expires. * * @param identifier - Identifier of the resource that needs to be locked. diff --git a/src/util/locking/ReadWriteLocker.ts b/src/util/locking/ReadWriteLocker.ts new file mode 100644 index 000000000..327dadac3 --- /dev/null +++ b/src/util/locking/ReadWriteLocker.ts @@ -0,0 +1,30 @@ +import type { ResourceIdentifier } from '../../ldp/representation/ResourceIdentifier'; + +/** + * Allows the locking of resources which is needed for non-atomic {@link ResourceStore}s. + */ +export interface ReadWriteLocker { + /** + * Run the given function while the resource is locked. + * The lock will be released when the (async) input function resolves. + * This function should be used for operations that only require reading the resource. + * + * @param identifier - Identifier of the resource that needs to be locked. + * @param whileLocked - A function to execute while the resource is locked. + * + * @returns A promise resolving when the lock is released. + */ + withReadLock: (identifier: ResourceIdentifier, whileLocked: () => T | Promise) => Promise; + + /** + * Run the given function while the resource is locked. + * The lock will be released when the (async) input function resolves. + * This function should be used for operations that could modify the resource. + * + * @param identifier - Identifier of the resource that needs to be locked. + * @param whileLocked - A function to execute while the resource is locked. + * + * @returns A promise resolving when the lock is released. + */ + withWriteLock: (identifier: ResourceIdentifier, whileLocked: () => T | Promise) => Promise; +} diff --git a/src/util/locking/ResourceLocker.ts b/src/util/locking/ResourceLocker.ts index 53040d65e..dba45794b 100644 --- a/src/util/locking/ResourceLocker.ts +++ b/src/util/locking/ResourceLocker.ts @@ -1,30 +1,23 @@ import type { ResourceIdentifier } from '../../ldp/representation/ResourceIdentifier'; /** - * Allows the locking of resources which is needed for non-atomic {@link ResourceStore}s. + * An interface for classes that only have 1 way to lock interfaces. + * In general this should only be used by components implementing the {@link ReadWriteLocker} interface. + * Other components that require locking of resources should use that interface. */ export interface ResourceLocker { /** - * Run the given function while the resource is locked. - * The lock will be released when the (async) input function resolves. - * This function should be used for operations that only require reading the resource. - * - * @param identifier - Identifier of the resource that needs to be locked. - * @param whileLocked - A function to execute while the resource is locked. - * - * @returns A promise resolving when the lock is released. + * Acquires a lock on the requested identifier. + * The promise will resolve when the lock has been acquired. + * @param identifier - Resource to acquire a lock on. */ - withReadLock: (identifier: ResourceIdentifier, whileLocked: () => T | Promise) => Promise; + acquire: (identifier: ResourceIdentifier) => Promise; /** - * Run the given function while the resource is locked. - * The lock will be released when the (async) input function resolves. - * This function should be used for operations that could modify the resource. - * - * @param identifier - Identifier of the resource that needs to be locked. - * @param whileLocked - A function to execute while the resource is locked. - * - * @returns A promise resolving when the lock is released. + * Releases a lock on the requested identifier. + * The promise will resolve when the lock has been released. + * In case there is no lock on the resource an error should be thrown. + * @param identifier - Resource to release the lock on. */ - withWriteLock: (identifier: ResourceIdentifier, whileLocked: () => T | Promise) => Promise; + release: (identifier: ResourceIdentifier) => Promise; } diff --git a/src/util/locking/SingleThreadedResourceLocker.ts b/src/util/locking/SingleThreadedResourceLocker.ts index 54ec7d06c..2049cc297 100644 --- a/src/util/locking/SingleThreadedResourceLocker.ts +++ b/src/util/locking/SingleThreadedResourceLocker.ts @@ -1,46 +1,54 @@ import AsyncLock from 'async-lock'; import type { ResourceIdentifier } from '../../ldp/representation/ResourceIdentifier'; import { getLoggerFor } from '../../logging/LogUtil'; +import { InternalServerError } from '../errors/InternalServerError'; import type { ResourceLocker } from './ResourceLocker'; /** * A resource locker making use of the `async-lock` library. - * Read and write locks use the same locks so no preference is given to any operations. - * This should be changed at some point though, see #542. + * Note that all locks are kept in memory until they are unlocked which could potentially result + * in a memory leak if locks are never unlocked, so make sure this is covered with expiring locks for example, + * and/or proper `finally` handles. */ export class SingleThreadedResourceLocker implements ResourceLocker { protected readonly logger = getLoggerFor(this); - private readonly locks: AsyncLock; + private readonly locker: AsyncLock; + private readonly unlockCallbacks: Record void>; public constructor() { - this.locks = new AsyncLock(); + this.locker = new AsyncLock(); + this.unlockCallbacks = {}; } - public async withReadLock(identifier: ResourceIdentifier, whileLocked: () => T | Promise): Promise { - return this.withLock(identifier, whileLocked); + public async acquire(identifier: ResourceIdentifier): Promise { + const { path } = identifier; + this.logger.debug(`Acquiring lock for ${path}`); + return new Promise((resolve): void => { + this.locker.acquire(path, (done): void => { + this.unlockCallbacks[path] = done; + this.logger.debug(`Acquired lock for ${path}. ${this.getLockCount()} locks active.`); + resolve(); + }, (): void => { + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete this.unlockCallbacks[path]; + this.logger.debug(`Released lock for ${path}. ${this.getLockCount()} active locks remaining.`); + }); + }); } - public async withWriteLock(identifier: ResourceIdentifier, whileLocked: () => T | Promise): Promise { - return this.withLock(identifier, whileLocked); + public async release(identifier: ResourceIdentifier): Promise { + const { path } = identifier; + if (!this.unlockCallbacks[path]) { + throw new InternalServerError(`Trying to unlock resource that is not locked: ${path}`); + } + this.unlockCallbacks[path](); } /** - * Acquires a new lock for the requested identifier. - * Will resolve when the input function resolves. - * @param identifier - Identifier of resource that needs to be locked. - * @param whileLocked - Function to resolve while the resource is locked. + * Counts the number of active locks. */ - private async withLock(identifier: ResourceIdentifier, whileLocked: () => T | Promise): Promise { - this.logger.debug(`Acquiring lock for ${identifier.path}`); - - try { - return await this.locks.acquire(identifier.path, async(): Promise => { - this.logger.debug(`Acquired lock for ${identifier.path}`); - return whileLocked(); - }); - } finally { - this.logger.debug(`Released lock for ${identifier.path}`); - } + private getLockCount(): number { + return Object.keys(this.unlockCallbacks).length; } } diff --git a/src/util/locking/WrappedExpiringResourceLocker.ts b/src/util/locking/WrappedExpiringReadWriteLocker.ts similarity index 85% rename from src/util/locking/WrappedExpiringResourceLocker.ts rename to src/util/locking/WrappedExpiringReadWriteLocker.ts index 031f4f740..92d6b85dc 100644 --- a/src/util/locking/WrappedExpiringResourceLocker.ts +++ b/src/util/locking/WrappedExpiringReadWriteLocker.ts @@ -1,24 +1,24 @@ import type { ResourceIdentifier } from '../../ldp/representation/ResourceIdentifier'; import { getLoggerFor } from '../../logging/LogUtil'; import { InternalServerError } from '../errors/InternalServerError'; -import type { ExpiringResourceLocker } from './ExpiringResourceLocker'; -import type { ResourceLocker } from './ResourceLocker'; +import type { ExpiringReadWriteLocker } from './ExpiringReadWriteLocker'; +import type { ReadWriteLocker } from './ReadWriteLocker'; import Timeout = NodeJS.Timeout; /** - * Wraps around an existing {@link ResourceLocker} and adds expiration logic to prevent locks from getting stuck. + * Wraps around an existing {@link ReadWriteLocker} and adds expiration logic to prevent locks from getting stuck. */ -export class WrappedExpiringResourceLocker implements ExpiringResourceLocker { +export class WrappedExpiringReadWriteLocker implements ExpiringReadWriteLocker { protected readonly logger = getLoggerFor(this); - protected readonly locker: ResourceLocker; + protected readonly locker: ReadWriteLocker; protected readonly expiration: number; /** * @param locker - Instance of ResourceLocker to use for acquiring a lock. * @param expiration - Time in ms after which the lock expires. */ - public constructor(locker: ResourceLocker, expiration: number) { + public constructor(locker: ReadWriteLocker, expiration: number) { this.locker = locker; this.expiration = expiration; } diff --git a/test/integration/LockingResourceStore.test.ts b/test/integration/LockingResourceStore.test.ts index 05f04265f..9508f9cce 100644 --- a/test/integration/LockingResourceStore.test.ts +++ b/test/integration/LockingResourceStore.test.ts @@ -8,10 +8,11 @@ import type { ResourceStore } from '../../src/storage/ResourceStore'; import { APPLICATION_OCTET_STREAM } from '../../src/util/ContentTypes'; import { InternalServerError } from '../../src/util/errors/InternalServerError'; import { SingleRootIdentifierStrategy } from '../../src/util/identifiers/SingleRootIdentifierStrategy'; -import type { ExpiringResourceLocker } from '../../src/util/locking/ExpiringResourceLocker'; -import type { ResourceLocker } from '../../src/util/locking/ResourceLocker'; +import { EqualReadWriteLocker } from '../../src/util/locking/EqualReadWriteLocker'; +import type { ExpiringReadWriteLocker } from '../../src/util/locking/ExpiringReadWriteLocker'; +import type { ReadWriteLocker } from '../../src/util/locking/ReadWriteLocker'; import { SingleThreadedResourceLocker } from '../../src/util/locking/SingleThreadedResourceLocker'; -import { WrappedExpiringResourceLocker } from '../../src/util/locking/WrappedExpiringResourceLocker'; +import { WrappedExpiringReadWriteLocker } from '../../src/util/locking/WrappedExpiringReadWriteLocker'; import { guardedStreamFrom } from '../../src/util/StreamUtil'; import { BASE } from './Config'; @@ -20,8 +21,8 @@ jest.useFakeTimers(); describe('A LockingResourceStore', (): void => { let path: string; let store: LockingResourceStore; - let locker: ResourceLocker; - let expiringLocker: ExpiringResourceLocker; + let locker: ReadWriteLocker; + let expiringLocker: ExpiringReadWriteLocker; let source: ResourceStore; let getRepresentationSpy: jest.SpyInstance; @@ -36,8 +37,8 @@ describe('A LockingResourceStore', (): void => { const initializer = new RootContainerInitializer({ store: source, baseUrl: BASE }); await initializer.handleSafe(); - locker = new SingleThreadedResourceLocker(); - expiringLocker = new WrappedExpiringResourceLocker(locker, 1000); + locker = new EqualReadWriteLocker(new SingleThreadedResourceLocker()); + expiringLocker = new WrappedExpiringReadWriteLocker(locker, 1000); store = new LockingResourceStore(source, expiringLocker); diff --git a/test/unit/storage/LockingResourceStore.test.ts b/test/unit/storage/LockingResourceStore.test.ts index f8a24c7f8..642a9d1ee 100644 --- a/test/unit/storage/LockingResourceStore.test.ts +++ b/test/unit/storage/LockingResourceStore.test.ts @@ -4,7 +4,7 @@ import type { Representation } from '../../../src/ldp/representation/Representat import type { ResourceIdentifier } from '../../../src/ldp/representation/ResourceIdentifier'; import { LockingResourceStore } from '../../../src/storage/LockingResourceStore'; import type { ResourceStore } from '../../../src/storage/ResourceStore'; -import type { ExpiringResourceLocker } from '../../../src/util/locking/ExpiringResourceLocker'; +import type { ExpiringReadWriteLocker } from '../../../src/util/locking/ExpiringReadWriteLocker'; import { guardedStreamFrom } from '../../../src/util/StreamUtil'; function emptyFn(): void { @@ -13,7 +13,7 @@ function emptyFn(): void { describe('A LockingResourceStore', (): void => { let store: LockingResourceStore; - let locker: ExpiringResourceLocker; + let locker: ExpiringReadWriteLocker; let source: ResourceStore; let order: string[]; let timeoutTrigger: EventEmitter; diff --git a/test/unit/util/locking/EqualReadWriteLocker.test.ts b/test/unit/util/locking/EqualReadWriteLocker.test.ts new file mode 100644 index 000000000..028c2dbef --- /dev/null +++ b/test/unit/util/locking/EqualReadWriteLocker.test.ts @@ -0,0 +1,62 @@ +import { EqualReadWriteLocker } from '../../../../src/util/locking/EqualReadWriteLocker'; +import type { ResourceLocker } from '../../../../src/util/locking/ResourceLocker'; + +describe('An EqualReadWriteLocker', (): void => { + let sourceLocker: ResourceLocker; + let locker: EqualReadWriteLocker; + let emptyFn: () => void; + const identifier = { path: 'http://test.com/res' }; + + beforeEach(async(): Promise => { + emptyFn = jest.fn(); + + sourceLocker = { + acquire: jest.fn(), + release: jest.fn(), + }; + locker = new EqualReadWriteLocker(sourceLocker); + }); + + it('locks and unlocks a resource for a read lock.', async(): Promise => { + const prom = locker.withReadLock(identifier, emptyFn); + expect(sourceLocker.acquire).toHaveBeenCalledTimes(1); + expect(sourceLocker.acquire).toHaveBeenLastCalledWith(identifier); + expect(emptyFn).toHaveBeenCalledTimes(0); + expect(sourceLocker.release).toHaveBeenCalledTimes(0); + + await expect(prom).resolves.toBeUndefined(); + expect(sourceLocker.acquire).toHaveBeenCalledTimes(1); + expect(emptyFn).toHaveBeenCalledTimes(1); + expect(sourceLocker.release).toHaveBeenCalledTimes(1); + expect(sourceLocker.release).toHaveBeenLastCalledWith(identifier); + }); + + it('locks and unlocks a resource for a write lock.', async(): Promise => { + const prom = locker.withWriteLock(identifier, emptyFn); + expect(sourceLocker.acquire).toHaveBeenCalledTimes(1); + expect(sourceLocker.acquire).toHaveBeenLastCalledWith(identifier); + expect(emptyFn).toHaveBeenCalledTimes(0); + expect(sourceLocker.release).toHaveBeenCalledTimes(0); + + await expect(prom).resolves.toBeUndefined(); + expect(sourceLocker.acquire).toHaveBeenCalledTimes(1); + expect(emptyFn).toHaveBeenCalledTimes(1); + expect(sourceLocker.release).toHaveBeenCalledTimes(1); + expect(sourceLocker.release).toHaveBeenLastCalledWith(identifier); + }); + + it('unlocks correctly if the whileLocked function errors.', async(): Promise => { + emptyFn = jest.fn().mockRejectedValue(new Error('bad data!')); + const prom = locker.withWriteLock(identifier, emptyFn); + expect(sourceLocker.acquire).toHaveBeenCalledTimes(1); + expect(sourceLocker.acquire).toHaveBeenLastCalledWith(identifier); + expect(emptyFn).toHaveBeenCalledTimes(0); + expect(sourceLocker.release).toHaveBeenCalledTimes(0); + + await expect(prom).rejects.toThrow('bad data!'); + expect(sourceLocker.acquire).toHaveBeenCalledTimes(1); + expect(emptyFn).toHaveBeenCalledTimes(1); + expect(sourceLocker.release).toHaveBeenCalledTimes(1); + expect(sourceLocker.release).toHaveBeenLastCalledWith(identifier); + }); +}); diff --git a/test/unit/util/locking/SingleThreadedResourceLocker.test.ts b/test/unit/util/locking/SingleThreadedResourceLocker.test.ts index 8a9a161c3..c8a9d4fcf 100644 --- a/test/unit/util/locking/SingleThreadedResourceLocker.test.ts +++ b/test/unit/util/locking/SingleThreadedResourceLocker.test.ts @@ -1,62 +1,71 @@ +import { InternalServerError } from '../../../../src/util/errors/InternalServerError'; import { SingleThreadedResourceLocker } from '../../../../src/util/locking/SingleThreadedResourceLocker'; describe('A SingleThreadedResourceLocker', (): void => { let locker: SingleThreadedResourceLocker; - const identifier = { path: 'path' }; - let syncCb: () => string; - let asyncCb: () => Promise; + const identifier = { path: 'http://test.com/foo' }; beforeEach(async(): Promise => { locker = new SingleThreadedResourceLocker(); - syncCb = jest.fn((): string => 'sync'); - asyncCb = jest.fn(async(): Promise => new Promise((resolve): void => { - setImmediate((): void => resolve('async')); - })); }); - it('can run simple functions with a read lock.', async(): Promise => { - let prom = locker.withReadLock(identifier, syncCb); - await expect(prom).resolves.toBe('sync'); - expect(syncCb).toHaveBeenCalledTimes(1); - - prom = locker.withReadLock(identifier, asyncCb); - await expect(prom).resolves.toBe('async'); - expect(asyncCb).toHaveBeenCalledTimes(1); + it('can lock and unlock a resource.', async(): Promise => { + await expect(locker.acquire(identifier)).resolves.toBeUndefined(); + await expect(locker.release(identifier)).resolves.toBeUndefined(); }); - it('can run simple functions with a write lock.', async(): Promise => { - let prom = locker.withWriteLock(identifier, syncCb); - await expect(prom).resolves.toBe('sync'); - expect(syncCb).toHaveBeenCalledTimes(1); - - prom = locker.withWriteLock(identifier, asyncCb); - await expect(prom).resolves.toBe('async'); - expect(asyncCb).toHaveBeenCalledTimes(1); + it('can lock a resource again after it was unlocked.', async(): Promise => { + await expect(locker.acquire(identifier)).resolves.toBeUndefined(); + await expect(locker.release(identifier)).resolves.toBeUndefined(); + await expect(locker.acquire(identifier)).resolves.toBeUndefined(); }); + it('errors when unlocking a resource that was not locked.', async(): Promise => { + await expect(locker.acquire(identifier)).resolves.toBeUndefined(); + await expect(locker.release(identifier)).resolves.toBeUndefined(); + await expect(locker.release(identifier)).rejects.toThrow(InternalServerError); + }); + + /* eslint-disable jest/valid-expect-in-promise */ it('blocks lock acquisition until they are released.', async(): Promise => { const results: number[] = []; + const lock1 = locker.acquire(identifier); + const lock2 = locker.acquire(identifier); + const lock3 = locker.acquire(identifier); - const promSlow = locker.withWriteLock(identifier, async(): Promise => - new Promise((resolve): void => { - setImmediate((): void => { - results.push(1); - resolve(); - }); - })); - - const promFast = locker.withWriteLock(identifier, (): void => { + // Note the different order of calls + const prom2 = lock2.then(async(): Promise => { results.push(2); + return locker.release(identifier); }); - - await Promise.all([ promFast, promSlow ]); - expect(results).toEqual([ 1, 2 ]); + const prom3 = lock3.then(async(): Promise => { + results.push(3); + return locker.release(identifier); + }); + const prom1 = lock1.then(async(): Promise => { + results.push(1); + return locker.release(identifier); + }); + await Promise.all([ prom2, prom3, prom1 ]); + expect(results).toEqual([ 1, 2, 3 ]); }); - it('propagates errors.', async(): Promise => { - asyncCb = jest.fn(async(): Promise => new Promise((resolve, reject): void => { - setImmediate((): void => reject(new Error('test'))); - })); - const prom = locker.withReadLock(identifier, asyncCb); - await expect(prom).rejects.toThrow('test'); + it('can acquire different keys simultaneously.', async(): Promise => { + const results: number[] = []; + const lock1 = locker.acquire({ path: 'path1' }); + const lock2 = locker.acquire({ path: 'path2' }); + const lock3 = locker.acquire({ path: 'path3' }); + await lock2.then(async(): Promise => { + results.push(2); + return locker.release({ path: 'path2' }); + }); + await lock3.then(async(): Promise => { + results.push(3); + return locker.release({ path: 'path3' }); + }); + await lock1.then(async(): Promise => { + results.push(1); + return locker.release({ path: 'path1' }); + }); + expect(results).toEqual([ 2, 3, 1 ]); }); }); diff --git a/test/unit/util/locking/WrappedExpiringResourceLocker.test.ts b/test/unit/util/locking/WrappedExpiringReadWriteLocker.test.ts similarity index 88% rename from test/unit/util/locking/WrappedExpiringResourceLocker.test.ts rename to test/unit/util/locking/WrappedExpiringReadWriteLocker.test.ts index a4e930214..7d9b92128 100644 --- a/test/unit/util/locking/WrappedExpiringResourceLocker.test.ts +++ b/test/unit/util/locking/WrappedExpiringReadWriteLocker.test.ts @@ -1,15 +1,15 @@ import type { ResourceIdentifier } from '../../../../src/ldp/representation/ResourceIdentifier'; -import type { ResourceLocker } from '../../../../src/util/locking/ResourceLocker'; -import { WrappedExpiringResourceLocker } from '../../../../src/util/locking/WrappedExpiringResourceLocker'; +import type { ReadWriteLocker } from '../../../../src/util/locking/ReadWriteLocker'; +import { WrappedExpiringReadWriteLocker } from '../../../../src/util/locking/WrappedExpiringReadWriteLocker'; jest.useFakeTimers(); -describe('A WrappedExpiringResourceLocker', (): void => { +describe('A WrappedExpiringReadWriteLocker', (): void => { const identifier = { path: 'path' }; let syncCb: () => string; let asyncCb: () => Promise; - let wrappedLocker: ResourceLocker; - let locker: WrappedExpiringResourceLocker; + let wrappedLocker: ReadWriteLocker; + let locker: WrappedExpiringReadWriteLocker; const expiration = 1000; beforeEach(async(): Promise => { @@ -25,7 +25,7 @@ describe('A WrappedExpiringResourceLocker', (): void => { setImmediate((): void => resolve('async')); })); - locker = new WrappedExpiringResourceLocker(wrappedLocker, expiration); + locker = new WrappedExpiringReadWriteLocker(wrappedLocker, expiration); }); it('calls the wrapped locker for locking.', async(): Promise => {