feat: Add auxiliary support to LockingResourceStore

This way locks are always on the associated resource
if there is a request on an auxiliary resource.

config/presets/storage-wrapper.json

@@ -38,8 +38,10 @@
       },
       "LockingResourceStore:_locks": {
         "@id": "urn:solid-server:default:ResourceLocker"
+      },
+      "LockingResourceStore:_strategy": {
+        "@id": "urn:solid-server:default:AuxiliaryStrategy"
       }
-
     },
 
     {

src/storage/LockingResourceStore.ts

@@ -1,6 +1,7 @@
 import type { Readable } from 'stream';
 import { promisify } from 'util';
 import eos from 'end-of-stream';
+import type { AuxiliaryIdentifierStrategy } from '../ldp/auxiliary/AuxiliaryIdentifierStrategy';
 import type { Patch } from '../ldp/http/Patch';
 import { BasicRepresentation } from '../ldp/representation/BasicRepresentation';
 import type { Representation } from '../ldp/representation/Representation';
@@ -17,46 +18,61 @@ const endOfStream = promisify(eos);
  * Store that for every call acquires a lock before executing it on the requested resource,
  * and releases it afterwards.
  * In case the request returns a Representation the lock will only be released when the data stream is finished.
+ *
+ * For auxiliary resources the lock will be applied to the associated resource.
+ * The actual operation is still executed on the auxiliary resource.
  */
 export class LockingResourceStore implements AtomicResourceStore {
   protected readonly logger = getLoggerFor(this);
 
   private readonly source: ResourceStore;
   private readonly locks: ExpiringReadWriteLocker;
+  private readonly strategy: AuxiliaryIdentifierStrategy;
 
-  public constructor(source: ResourceStore, locks: ExpiringReadWriteLocker) {
+  public constructor(source: ResourceStore, locks: ExpiringReadWriteLocker, strategy: AuxiliaryIdentifierStrategy) {
     this.source = source;
     this.locks = locks;
+    this.strategy = strategy;
   }
 
   public async getRepresentation(identifier: ResourceIdentifier, preferences: RepresentationPreferences,
     conditions?: Conditions): Promise<Representation> {
-    return this.lockedRepresentationRun(identifier,
+    return this.lockedRepresentationRun(this.getLockIdentifier(identifier),
       async(): Promise<Representation> => this.source.getRepresentation(identifier, preferences, conditions));
   }
 
   public async addResource(container: ResourceIdentifier, representation: Representation,
     conditions?: Conditions): Promise<ResourceIdentifier> {
-    return this.locks.withWriteLock(container,
+    return this.locks.withWriteLock(this.getLockIdentifier(container),
       async(): Promise<ResourceIdentifier> => this.source.addResource(container, representation, conditions));
   }
 
   public async setRepresentation(identifier: ResourceIdentifier, representation: Representation,
     conditions?: Conditions): Promise<void> {
-    return this.locks.withWriteLock(identifier,
+    return this.locks.withWriteLock(this.getLockIdentifier(identifier),
       async(): Promise<void> => this.source.setRepresentation(identifier, representation, conditions));
   }
 
   public async deleteResource(identifier: ResourceIdentifier, conditions?: Conditions): Promise<void> {
-    return this.locks.withWriteLock(identifier,
+    return this.locks.withWriteLock(this.getLockIdentifier(identifier),
       async(): Promise<void> => this.source.deleteResource(identifier, conditions));
   }
 
   public async modifyResource(identifier: ResourceIdentifier, patch: Patch, conditions?: Conditions): Promise<void> {
-    return this.locks.withWriteLock(identifier,
+    return this.locks.withWriteLock(this.getLockIdentifier(identifier),
       async(): Promise<void> => this.source.modifyResource(identifier, patch, conditions));
   }
 
+  /**
+   * Acquires the correct identifier to lock this resource.
+   * For auxiliary resources this means the associated identifier.
+   */
+  protected getLockIdentifier(identifier: ResourceIdentifier): ResourceIdentifier {
+    return this.strategy.isAuxiliaryIdentifier(identifier) ?
+      this.strategy.getAssociatedIdentifier(identifier) :
+      identifier;
+  }
+
   /**
    * Acquires a lock that is only released when all data of the resulting representation data has been read,
    * an error occurs, or the timeout has been triggered.

test/integration/LockingResourceStore.test.ts

@@ -1,4 +1,5 @@
 import { RootContainerInitializer } from '../../src/init/RootContainerInitializer';
+import { RoutingAuxiliaryStrategy } from '../../src/ldp/auxiliary/RoutingAuxiliaryStrategy';
 import { BasicRepresentation } from '../../src/ldp/representation/BasicRepresentation';
 import type { Representation } from '../../src/ldp/representation/Representation';
 import { InMemoryDataAccessor } from '../../src/storage/accessors/InMemoryDataAccessor';
@@ -29,9 +30,15 @@ describe('A LockingResourceStore', (): void => {
   beforeEach(async(): Promise<void> => {
     jest.clearAllMocks();
 
+    // Not relevant for these tests
+    const strategy = new RoutingAuxiliaryStrategy([]);
+
     const base = 'http://test.com/';
     path = `${base}path`;
-    source = new DataAccessorBasedStore(new InMemoryDataAccessor(base), new SingleRootIdentifierStrategy(base));
+    source = new DataAccessorBasedStore(
+      new InMemoryDataAccessor(base),
+      new SingleRootIdentifierStrategy(base),
+    );
 
     // Initialize store
     const initializer = new RootContainerInitializer({ store: source, baseUrl: BASE });
@@ -40,7 +47,7 @@ describe('A LockingResourceStore', (): void => {
     locker = new EqualReadWriteLocker(new SingleThreadedResourceLocker());
     expiringLocker = new WrappedExpiringReadWriteLocker(locker, 1000);
 
-    store = new LockingResourceStore(source, expiringLocker);
+    store = new LockingResourceStore(source, expiringLocker, strategy);
 
     // Spy on a real ResourceLocker and ResourceStore instance
     getRepresentationSpy = jest.spyOn(source, 'getRepresentation');

test/unit/storage/LockingResourceStore.test.ts

@@ -1,4 +1,5 @@
 import { EventEmitter } from 'events';
+import type { AuxiliaryIdentifierStrategy } from '../../../src/ldp/auxiliary/AuxiliaryIdentifierStrategy';
 import type { Patch } from '../../../src/ldp/http/Patch';
 import type { Representation } from '../../../src/ldp/representation/Representation';
 import type { ResourceIdentifier } from '../../../src/ldp/representation/ResourceIdentifier';
@@ -12,9 +13,13 @@ function emptyFn(): void {
 }
 
 describe('A LockingResourceStore', (): void => {
+  const auxiliaryId = { path: 'http://test.com/foo.dummy' };
+  const associatedId = { path: 'http://test.com/foo' };
+  const data = { data: 'data!' } as any;
   let store: LockingResourceStore;
   let locker: ExpiringReadWriteLocker;
   let source: ResourceStore;
+  let strategy: AuxiliaryIdentifierStrategy;
   let order: string[];
   let timeoutTrigger: EventEmitter;
 
@@ -64,7 +69,12 @@ describe('A LockingResourceStore', (): void => {
       }),
     };
 
-    store = new LockingResourceStore(source, locker);
+    strategy = {
+      isAuxiliaryIdentifier: jest.fn((id: ResourceIdentifier): any => id.path.endsWith('.dummy')),
+      getAssociatedIdentifier: jest.fn((id: ResourceIdentifier): any => ({ path: id.path.slice(0, -6) })),
+    } as any;
+
+    store = new LockingResourceStore(source, locker, strategy);
   });
 
   function registerEventOrder(eventSource: EventEmitter, event: string): void {
@@ -74,34 +84,70 @@ describe('A LockingResourceStore', (): void => {
   }
 
   it('acquires a lock on the container when adding a representation.', async(): Promise<void> => {
-    await store.addResource({ path: 'path' }, {} as Representation);
+    await store.addResource(associatedId, data);
     expect(locker.withWriteLock).toHaveBeenCalledTimes(1);
-    expect((locker.withWriteLock as jest.Mock).mock.calls[0][0]).toEqual({ path: 'path' });
+    expect((locker.withWriteLock as jest.Mock).mock.calls[0][0]).toEqual(associatedId);
     expect(source.addResource).toHaveBeenCalledTimes(1);
+    expect(source.addResource).toHaveBeenLastCalledWith(associatedId, data, undefined);
+    expect(order).toEqual([ 'lock write', 'addResource', 'unlock write' ]);
+
+    order = [];
+    await expect(store.addResource(auxiliaryId, data)).resolves.toBeUndefined();
+    expect(locker.withWriteLock).toHaveBeenCalledTimes(2);
+    expect((locker.withWriteLock as jest.Mock).mock.calls[1][0]).toEqual(associatedId);
+    expect(source.addResource).toHaveBeenCalledTimes(2);
+    expect(source.addResource).toHaveBeenLastCalledWith(auxiliaryId, data, undefined);
     expect(order).toEqual([ 'lock write', 'addResource', 'unlock write' ]);
   });
 
   it('acquires a lock on the resource when setting its representation.', async(): Promise<void> => {
-    await store.setRepresentation({ path: 'path' }, {} as Representation);
+    await store.setRepresentation(associatedId, data);
     expect(locker.withWriteLock).toHaveBeenCalledTimes(1);
-    expect((locker.withWriteLock as jest.Mock).mock.calls[0][0]).toEqual({ path: 'path' });
+    expect((locker.withWriteLock as jest.Mock).mock.calls[0][0]).toEqual(associatedId);
     expect(source.setRepresentation).toHaveBeenCalledTimes(1);
+    expect(source.setRepresentation).toHaveBeenLastCalledWith(associatedId, data, undefined);
+    expect(order).toEqual([ 'lock write', 'setRepresentation', 'unlock write' ]);
+
+    order = [];
+    await expect(store.setRepresentation(auxiliaryId, data)).resolves.toBeUndefined();
+    expect(locker.withWriteLock).toHaveBeenCalledTimes(2);
+    expect((locker.withWriteLock as jest.Mock).mock.calls[1][0]).toEqual(associatedId);
+    expect(source.setRepresentation).toHaveBeenCalledTimes(2);
+    expect(source.setRepresentation).toHaveBeenLastCalledWith(auxiliaryId, data, undefined);
     expect(order).toEqual([ 'lock write', 'setRepresentation', 'unlock write' ]);
   });
 
   it('acquires a lock on the resource when deleting it.', async(): Promise<void> => {
-    await store.deleteResource({ path: 'path' });
+    await store.deleteResource(associatedId);
     expect(locker.withWriteLock).toHaveBeenCalledTimes(1);
-    expect((locker.withWriteLock as jest.Mock).mock.calls[0][0]).toEqual({ path: 'path' });
+    expect((locker.withWriteLock as jest.Mock).mock.calls[0][0]).toEqual(associatedId);
     expect(source.deleteResource).toHaveBeenCalledTimes(1);
+    expect(source.deleteResource).toHaveBeenLastCalledWith(associatedId, undefined);
+    expect(order).toEqual([ 'lock write', 'deleteResource', 'unlock write' ]);
+
+    order = [];
+    await expect(store.deleteResource(auxiliaryId)).resolves.toBeUndefined();
+    expect(locker.withWriteLock).toHaveBeenCalledTimes(2);
+    expect((locker.withWriteLock as jest.Mock).mock.calls[1][0]).toEqual(associatedId);
+    expect(source.deleteResource).toHaveBeenCalledTimes(2);
+    expect(source.deleteResource).toHaveBeenLastCalledWith(auxiliaryId, undefined);
     expect(order).toEqual([ 'lock write', 'deleteResource', 'unlock write' ]);
   });
 
   it('acquires a lock on the resource when modifying its representation.', async(): Promise<void> => {
-    await store.modifyResource({ path: 'path' }, {} as Patch);
+    await store.modifyResource(associatedId, data as Patch);
     expect(locker.withWriteLock).toHaveBeenCalledTimes(1);
-    expect((locker.withWriteLock as jest.Mock).mock.calls[0][0]).toEqual({ path: 'path' });
+    expect((locker.withWriteLock as jest.Mock).mock.calls[0][0]).toEqual(associatedId);
     expect(source.modifyResource).toHaveBeenCalledTimes(1);
+    expect(source.modifyResource).toHaveBeenLastCalledWith(associatedId, data, undefined);
+    expect(order).toEqual([ 'lock write', 'modifyResource', 'unlock write' ]);
+
+    order = [];
+    await expect(store.modifyResource(auxiliaryId, data as Patch)).resolves.toBeUndefined();
+    expect(locker.withWriteLock).toHaveBeenCalledTimes(2);
+    expect((locker.withWriteLock as jest.Mock).mock.calls[1][0]).toEqual(associatedId);
+    expect(source.modifyResource).toHaveBeenCalledTimes(2);
+    expect(source.modifyResource).toHaveBeenLastCalledWith(auxiliaryId, data, undefined);
     expect(order).toEqual([ 'lock write', 'modifyResource', 'unlock write' ]);
   });
 
@@ -110,15 +156,15 @@ describe('A LockingResourceStore', (): void => {
       order.push('bad get');
       throw new Error('dummy');
     };
-    await expect(store.getRepresentation({ path: 'path' }, {})).rejects.toThrow('dummy');
+    await expect(store.getRepresentation(associatedId, {})).rejects.toThrow('dummy');
     expect(locker.withReadLock).toHaveBeenCalledTimes(1);
-    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual({ path: 'path' });
+    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual(associatedId);
     expect(order).toEqual([ 'lock read', 'bad get', 'unlock read' ]);
   });
 
   it('releases the lock on the resource when data has been read.', async(): Promise<void> => {
     // Read all data from the representation
-    const representation = await store.getRepresentation({ path: 'path' }, {});
+    const representation = await store.getRepresentation(associatedId, {});
     representation.data.on('data', (): any => true);
     registerEventOrder(representation.data, 'end');
 
@@ -127,14 +173,32 @@ describe('A LockingResourceStore', (): void => {
 
     // Verify the lock was acquired and released at the right time
     expect(locker.withReadLock).toHaveBeenCalledTimes(1);
-    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual({ path: 'path' });
+    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual(associatedId);
     expect(source.getRepresentation).toHaveBeenCalledTimes(1);
+    expect(source.getRepresentation).toHaveBeenLastCalledWith(associatedId, {}, undefined);
+    expect(order).toEqual([ 'lock read', 'getRepresentation', 'end', 'unlock read' ]);
+  });
+
+  it('acquires the lock on the associated resource when reading an auxiliary resource.', async(): Promise<void> => {
+    // Read all data from the representation
+    const representation = await store.getRepresentation(auxiliaryId, {});
+    representation.data.on('data', (): any => true);
+    registerEventOrder(representation.data, 'end');
+
+    // Provide opportunity for async events
+    await new Promise(setImmediate);
+
+    // Verify the lock was acquired and released at the right time
+    expect(locker.withReadLock).toHaveBeenCalledTimes(1);
+    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual(associatedId);
+    expect(source.getRepresentation).toHaveBeenCalledTimes(1);
+    expect(source.getRepresentation).toHaveBeenLastCalledWith(auxiliaryId, {}, undefined);
     expect(order).toEqual([ 'lock read', 'getRepresentation', 'end', 'unlock read' ]);
   });
 
   it('destroys the resource and releases the lock when the readable errors.', async(): Promise<void> => {
     // Make the representation error
-    const representation = await store.getRepresentation({ path: 'path' }, {});
+    const representation = await store.getRepresentation(associatedId, {});
     setImmediate((): any => representation.data.emit('error', new Error('Error on the readable')));
     registerEventOrder(representation.data, 'error');
     registerEventOrder(representation.data, 'close');
@@ -144,7 +208,7 @@ describe('A LockingResourceStore', (): void => {
 
     // Verify the lock was acquired and released at the right time
     expect(locker.withReadLock).toHaveBeenCalledTimes(1);
-    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual({ path: 'path' });
+    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual(associatedId);
     expect(source.getRepresentation).toHaveBeenCalledTimes(1);
     expect(representation.data.destroy).toHaveBeenCalledTimes(1);
     expect(order).toEqual([ 'lock read', 'getRepresentation', 'error', 'unlock read', 'close' ]);
@@ -152,7 +216,7 @@ describe('A LockingResourceStore', (): void => {
 
   it('releases the lock on the resource when readable is destroyed.', async(): Promise<void> => {
     // Make the representation close
-    const representation = await store.getRepresentation({ path: 'path' }, {});
+    const representation = await store.getRepresentation(associatedId, {});
     representation.data.destroy();
     registerEventOrder(representation.data, 'close');
 
@@ -161,33 +225,32 @@ describe('A LockingResourceStore', (): void => {
 
     // Verify the lock was acquired and released at the right time
     expect(locker.withReadLock).toHaveBeenCalledTimes(1);
-    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual({ path: 'path' });
+    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual(associatedId);
     expect(source.getRepresentation).toHaveBeenCalledTimes(1);
     expect(order).toEqual([ 'lock read', 'getRepresentation', 'close', 'unlock read' ]);
   });
 
   it('releases the lock only once when multiple events are triggered.', async(): Promise<void> => {
     // Read all data from the representation and trigger an additional close event
-    const representation = await store.getRepresentation({ path: 'path' }, {});
+    const representation = await store.getRepresentation(associatedId, {});
     representation.data.on('data', (): any => true);
     representation.data.prependListener('end', (): any => {
       order.push('end');
       representation.data.destroy();
     });
-    registerEventOrder(representation.data, 'close');
 
     // Provide opportunity for async events
     await new Promise(setImmediate);
 
     // Verify the lock was acquired and released at the right time
     expect(locker.withReadLock).toHaveBeenCalledTimes(1);
-    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual({ path: 'path' });
+    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual(associatedId);
     expect(source.getRepresentation).toHaveBeenCalledTimes(1);
-    expect(order).toEqual([ 'lock read', 'getRepresentation', 'end', 'close', 'unlock read' ]);
+    expect(order).toEqual([ 'lock read', 'getRepresentation', 'end', 'unlock read' ]);
   });
 
   it('releases the lock on the resource when readable times out.', async(): Promise<void> => {
-    const representation = await store.getRepresentation({ path: 'path' }, {});
+    const representation = await store.getRepresentation(associatedId, {});
     registerEventOrder(representation.data, 'close');
     registerEventOrder(representation.data, 'error');
 
@@ -198,7 +261,7 @@ describe('A LockingResourceStore', (): void => {
 
     // Verify the lock was acquired and released at the right time
     expect(locker.withReadLock).toHaveBeenCalledTimes(1);
-    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual({ path: 'path' });
+    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual(associatedId);
     expect(source.getRepresentation).toHaveBeenCalledTimes(1);
     expect(representation.data.destroy).toHaveBeenCalledTimes(1);
     expect(representation.data.destroy).toHaveBeenLastCalledWith(new Error('timeout'));
@@ -212,13 +275,13 @@ describe('A LockingResourceStore', (): void => {
       return new Promise(emptyFn);
     });
 
-    const prom = store.getRepresentation({ path: 'path' }, {});
+    const prom = store.getRepresentation(associatedId, {});
 
     timeoutTrigger.emit('timeout');
 
     await expect(prom).rejects.toThrow('timeout');
     expect(locker.withReadLock).toHaveBeenCalledTimes(1);
-    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual({ path: 'path' });
+    expect((locker.withReadLock as jest.Mock).mock.calls[0][0]).toEqual(associatedId);
     expect(source.getRepresentation).toHaveBeenCalledTimes(1);
     expect(order).toEqual([ 'lock read', 'useless get', 'timeout', 'unlock read' ]);
   });