fix: Remove locking from the SparqlUpdatePatchHandler

Due to not having re-entrant locks this would cause deadlocks
with the LockingResourceStore or require more advanced configurations.
If this is needed in the future we can potentially add a LockingPatchHandler.

src/storage/patch/SparqlUpdatePatchHandler.ts

@@ -11,7 +11,6 @@ import { getLoggerFor } from '../../logging/LogUtil';
 import { INTERNAL_QUADS } from '../../util/ContentTypes';
 import { NotFoundHttpError } from '../../util/errors/NotFoundHttpError';
 import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError';
-import type { ResourceLocker } from '../../util/locking/ResourceLocker';
 import type { ResourceStore } from '../ResourceStore';
 import { PatchHandler } from './PatchHandler';
 
@@ -23,12 +22,10 @@ export class SparqlUpdatePatchHandler extends PatchHandler {
   protected readonly logger = getLoggerFor(this);
 
   private readonly source: ResourceStore;
-  private readonly locker: ResourceLocker;
 
-  public constructor(source: ResourceStore, locker: ResourceLocker) {
+  public constructor(source: ResourceStore) {
     super();
     this.source = source;
-    this.locker = locker;
   }
 
   public async canHandle(input: {identifier: ResourceIdentifier; patch: SparqlUpdatePatch}): Promise<void> {
@@ -43,12 +40,7 @@ export class SparqlUpdatePatchHandler extends PatchHandler {
     const op = patch.algebra;
     this.validateUpdate(op);
 
-    const lock = await this.locker.acquire(identifier);
+    await this.applyPatch(identifier, op);
-    try {
-      await this.applyPatch(identifier, op);
-    } finally {
-      await lock.release();
-    }
   }
 
   private isDeleteInsert(op: Algebra.Operation): op is Algebra.DeleteInsert {

test/unit/storage/patch/SparqlUpdatePatchHandler.test.ts

@@ -3,29 +3,21 @@ import { namedNode, quad } from '@rdfjs/data-model';
 import arrayifyStream from 'arrayify-stream';
 import type { Quad } from 'rdf-js';
 import { translate } from 'sparqlalgebrajs';
-import streamifyArray from 'streamify-array';
 import type { SparqlUpdatePatch } from '../../../../src/ldp/http/SparqlUpdatePatch';
+import { BasicRepresentation } from '../../../../src/ldp/representation/BasicRepresentation';
 import { RepresentationMetadata } from '../../../../src/ldp/representation/RepresentationMetadata';
 import { SparqlUpdatePatchHandler } from '../../../../src/storage/patch/SparqlUpdatePatchHandler';
 import type { ResourceStore } from '../../../../src/storage/ResourceStore';
 import { INTERNAL_QUADS } from '../../../../src/util/ContentTypes';
 import { NotFoundHttpError } from '../../../../src/util/errors/NotFoundHttpError';
 import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError';
-import type { Lock } from '../../../../src/util/locking/Lock';
-import type { ResourceLocker } from '../../../../src/util/locking/ResourceLocker';
 
 describe('A SparqlUpdatePatchHandler', (): void => {
   let handler: SparqlUpdatePatchHandler;
-  let locker: ResourceLocker;
-  let lock: Lock;
-  let release: () => Promise<void>;
   let source: ResourceStore;
   let startQuads: Quad[];
-  let order: string[];
 
   beforeEach(async(): Promise<void> => {
-    order = [];
-
     startQuads = [ quad(
       namedNode('http://test.com/startS1'),
       namedNode('http://test.com/startP1'),
@@ -37,32 +29,14 @@ describe('A SparqlUpdatePatchHandler', (): void => {
     ) ];
 
     source = {
-      getRepresentation: jest.fn(async(): Promise<any> => {
+      getRepresentation: jest.fn(async(): Promise<any> => new BasicRepresentation(startQuads, 'internal/quads', false)),
-        order.push('getRepresentation');
+      setRepresentation: jest.fn(),
-        return {
-          dataType: 'quads',
-          data: streamifyArray([ ...startQuads ]),
-          metadata: null,
-        };
-      }),
-      setRepresentation: jest.fn(async(): Promise<any> => {
-        order.push('setRepresentation');
-      }),
       modifyResource: jest.fn(async(): Promise<any> => {
         throw new Error('noModify');
       }),
     } as unknown as ResourceStore;
 
-    release = jest.fn(async(): Promise<any> => order.push('release'));
+    handler = new SparqlUpdatePatchHandler(source);
-    locker = {
-      acquire: jest.fn(async(): Promise<any> => {
-        order.push('acquire');
-        lock = { release };
-        return lock;
-      }),
-    };
-
-    handler = new SparqlUpdatePatchHandler(source, locker);
   });
 
   async function basicChecks(quads: Quad[]): Promise<boolean> {
@@ -71,7 +45,6 @@ describe('A SparqlUpdatePatchHandler', (): void => {
       { path: 'path' }, { type: { [INTERNAL_QUADS]: 1 }},
     );
     expect(source.setRepresentation).toHaveBeenCalledTimes(1);
-    expect(order).toEqual([ 'acquire', 'getRepresentation', 'setRepresentation', 'release' ]);
     const setParams = (source.setRepresentation as jest.Mock).mock.calls[0];
     expect(setParams[0]).toEqual({ path: 'path' });
     expect(setParams[1]).toEqual(expect.objectContaining({
@@ -197,7 +170,6 @@ describe('A SparqlUpdatePatchHandler', (): void => {
         { quads: true },
       ) } as SparqlUpdatePatch });
     await expect(handle).rejects.toThrow('GRAPH statements are not supported');
-    expect(order).toEqual([]);
   });
 
   it('rejects GRAPH deletes.', async(): Promise<void> => {
@@ -208,7 +180,6 @@ describe('A SparqlUpdatePatchHandler', (): void => {
         { quads: true },
       ) } as SparqlUpdatePatch });
     await expect(handle).rejects.toThrow('GRAPH statements are not supported');
-    expect(order).toEqual([]);
   });
 
   it('rejects DELETE/INSERT updates with a non-empty WHERE.', async(): Promise<void> => {
@@ -220,7 +191,6 @@ describe('A SparqlUpdatePatchHandler', (): void => {
         { quads: true },
       ) } as SparqlUpdatePatch });
     await expect(handle).rejects.toThrow('WHERE statements are not supported');
-    expect(order).toEqual([]);
   });
 
   it('rejects DELETE WHERE updates with variables.', async(): Promise<void> => {
@@ -230,7 +200,6 @@ describe('A SparqlUpdatePatchHandler', (): void => {
         { quads: true },
       ) } as SparqlUpdatePatch });
     await expect(handle).rejects.toThrow('WHERE statements are not supported');
-    expect(order).toEqual([]);
   });
 
   it('rejects non-DELETE/INSERT updates.', async(): Promise<void> => {
@@ -240,12 +209,10 @@ describe('A SparqlUpdatePatchHandler', (): void => {
         { quads: true },
       ) } as SparqlUpdatePatch });
     await expect(handle).rejects.toThrow('Only DELETE/INSERT SPARQL update operations are supported');
-    expect(order).toEqual([]);
   });
 
-  it('releases the lock if an error occurs while patching.', async(): Promise<void> => {
+  it('throws the error returned by the store if there is one.', async(): Promise<void> => {
     source.getRepresentation = jest.fn(async(): Promise<any> => {
-      order.push('getRepresentation');
       throw new Error('error');
     });
 
@@ -256,14 +223,12 @@ describe('A SparqlUpdatePatchHandler', (): void => {
         { quads: true },
       ) } as SparqlUpdatePatch };
     await expect(handler.handle(input)).rejects.toThrow('error');
-    expect(order).toEqual([ 'acquire', 'getRepresentation', 'release' ]);
   });
 
   it('creates a new resource if it does not exist yet.', async(): Promise<void> => {
     // There is no initial data
     startQuads = [];
     source.getRepresentation = jest.fn((): any => {
-      order.push('getRepresentation');
       throw new NotFoundHttpError();
     });