feat: Verify conditions in DataAccessorBasedStore

src/index.ts

@@ -285,6 +285,7 @@ export * from './util/errors/InternalServerError';
 export * from './util/errors/MethodNotAllowedHttpError';
 export * from './util/errors/NotFoundHttpError';
 export * from './util/errors/NotImplementedHttpError';
+export * from './util/errors/PreconditionFailedHttpError';
 export * from './util/errors/SystemError';
 export * from './util/errors/UnauthorizedHttpError';
 export * from './util/errors/UnsupportedMediaTypeHttpError';

src/storage/DataAccessorBasedStore.ts

@@ -3,6 +3,7 @@ import { DataFactory } from 'n3';
 import type { NamedNode, Quad, Term } from 'rdf-js';
 import { v4 as uuid } from 'uuid';
 import type { AuxiliaryStrategy } from '../ldp/auxiliary/AuxiliaryStrategy';
+import type { Patch } from '../ldp/http/Patch';
 import { BasicRepresentation } from '../ldp/representation/BasicRepresentation';
 import type { Representation } from '../ldp/representation/Representation';
 import type { RepresentationMetadata } from '../ldp/representation/RepresentationMetadata';
@@ -16,6 +17,7 @@ import { ForbiddenHttpError } from '../util/errors/ForbiddenHttpError';
 import { MethodNotAllowedHttpError } from '../util/errors/MethodNotAllowedHttpError';
 import { NotFoundHttpError } from '../util/errors/NotFoundHttpError';
 import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
+import { PreconditionFailedHttpError } from '../util/errors/PreconditionFailedHttpError';
 import type { IdentifierStrategy } from '../util/identifiers/IdentifierStrategy';
 import {
   ensureTrailingSlash,
@@ -39,6 +41,7 @@ import {
   PREFERRED_PREFIX_TERM,
 } from '../util/Vocabularies';
 import type { DataAccessor } from './accessors/DataAccessor';
+import type { Conditions } from './Conditions';
 import type { ResourceStore } from './ResourceStore';
 
 /**
@@ -134,7 +137,8 @@ export class DataAccessorBasedStore implements ResourceStore {
     return representation;
   }
 
-  public async addResource(container: ResourceIdentifier, representation: Representation): Promise<ResourceIdentifier> {
+  public async addResource(container: ResourceIdentifier, representation: Representation, conditions?: Conditions):
+  Promise<ResourceIdentifier> {
     this.validateIdentifier(container);
 
     // Ensure the representation is supported by the accessor
@@ -153,10 +157,12 @@ export class DataAccessorBasedStore implements ResourceStore {
     // Solid, §5: "Servers MUST respond with the 405 status code to requests using HTTP methods
     // that are not supported by the target resource."
     // https://solid.github.io/specification/protocol#reading-writing-resources
-    if (parentMetadata && !isContainerPath(parentMetadata.identifier.value)) {
+    if (!isContainerPath(parentMetadata.identifier.value)) {
       throw new MethodNotAllowedHttpError('The given path is not a container.');
     }
 
+    this.validateConditions(conditions, parentMetadata);
+
     // Solid, §5.1: "Servers MAY allow clients to suggest the URI of a resource created through POST,
     // using the HTTP Slug header as defined in [RFC5023].
     // Clients who want the server to assign a URI of a resource, MUST use the POST request."
@@ -169,8 +175,8 @@ export class DataAccessorBasedStore implements ResourceStore {
     return newID;
   }
 
-  public async setRepresentation(identifier: ResourceIdentifier, representation: Representation):
-  Promise<ResourceIdentifier[]> {
+  public async setRepresentation(identifier: ResourceIdentifier, representation: Representation,
+    conditions?: Conditions): Promise<ResourceIdentifier[]> {
     this.validateIdentifier(identifier);
 
     // Ensure the representation is supported by the accessor
@@ -196,15 +202,31 @@ export class DataAccessorBasedStore implements ResourceStore {
       throw new BadRequestHttpError('Containers should have a `/` at the end of their path, resources should not.');
     }
 
+    this.validateConditions(conditions, oldMetadata);
+
     // Potentially have to create containers if it didn't exist yet
     return this.writeData(identifier, representation, isContainer, !oldMetadata, Boolean(oldMetadata));
   }
 
-  public async modifyResource(): Promise<ResourceIdentifier[]> {
+  public async modifyResource(identifier: ResourceIdentifier, patch: Patch,
+    conditions?: Conditions): Promise<ResourceIdentifier[]> {
+    if (conditions) {
+      let metadata: RepresentationMetadata | undefined;
+      try {
+        metadata = await this.accessor.getMetadata(identifier);
+      } catch (error: unknown) {
+        if (!NotFoundHttpError.isInstance(error)) {
+          throw error;
+        }
+      }
+
+      this.validateConditions(conditions, metadata);
+    }
+
     throw new NotImplementedHttpError('Patches are not supported by the default store.');
   }
 
-  public async deleteResource(identifier: ResourceIdentifier): Promise<ResourceIdentifier[]> {
+  public async deleteResource(identifier: ResourceIdentifier, conditions?: Conditions): Promise<ResourceIdentifier[]> {
     this.validateIdentifier(identifier);
     const metadata = await this.accessor.getMetadata(identifier);
     // Solid, §5.4: "When a DELETE request targets storage’s root container or its associated ACL resource,
@@ -229,8 +251,11 @@ export class DataAccessorBasedStore implements ResourceStore {
     if (isContainerIdentifier(identifier) && await this.hasProperChildren(identifier)) {
       throw new ConflictHttpError('Can only delete empty containers.');
     }
-    // Solid, §5.4: "When a contained resource is deleted, the server MUST also delete the associated auxiliary
-    // resources"
+
+    this.validateConditions(conditions, metadata);
+
+    // Solid, §5.4: "When a contained resource is deleted,
+    // the server MUST also delete the associated auxiliary resources"
     // https://solid.github.io/specification/protocol#deleting-resources
     const deleted = [ identifier ];
     if (!this.auxiliaryStrategy.isAuxiliaryIdentifier(identifier)) {
@@ -261,6 +286,17 @@ export class DataAccessorBasedStore implements ResourceStore {
     }
   }
 
+  /**
+   * Verify if the given metadata matches the conditions.
+   */
+  protected validateConditions(conditions?: Conditions, metadata?: RepresentationMetadata): void {
+    // The 412 (Precondition Failed) status code indicates
+    // that one or more conditions given in the request header fields evaluated to false when tested on the server.
+    if (conditions && !conditions.matchesMetadata(metadata)) {
+      throw new PreconditionFailedHttpError();
+    }
+  }
+
   /**
    * Returns the metadata matching the identifier, ignoring the presence of a trailing slash or not.
    *

src/storage/PatchingStore.ts

@@ -1,5 +1,6 @@
 import type { Patch } from '../ldp/http/Patch';
 import type { ResourceIdentifier } from '../ldp/representation/ResourceIdentifier';
+import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
 import type { Conditions } from './Conditions';
 import { PassthroughStore } from './PassthroughStore';
 import type { PatchHandler } from './patch/PatchHandler';
@@ -22,8 +23,11 @@ export class PatchingStore<T extends ResourceStore = ResourceStore> extends Pass
     conditions?: Conditions): Promise<ResourceIdentifier[]> {
     try {
       return await this.source.modifyResource(identifier, patch, conditions);
-    } catch {
-      return this.patcher.handleSafe({ source: this.source, identifier, patch });
+    } catch (error: unknown) {
+      if (NotImplementedHttpError.isInstance(error)) {
+        return this.patcher.handleSafe({ source: this.source, identifier, patch });
+      }
+      throw error;
     }
   }
 }

src/util/errors/PreconditionFailedHttpError.ts

@@ -0,0 +1,15 @@
+import type { HttpErrorOptions } from './HttpError';
+import { HttpError } from './HttpError';
+
+/**
+ * An error thrown when access was denied due to the conditions on the request.
+ */
+export class PreconditionFailedHttpError extends HttpError {
+  public constructor(message?: string, options?: HttpErrorOptions) {
+    super(412, 'PreconditionFailedHttpError', message, options);
+  }
+
+  public static isInstance(error: any): error is PreconditionFailedHttpError {
+    return HttpError.isInstance(error) && error.statusCode === 412;
+  }
+}

test/unit/storage/DataAccessorBasedStore.test.ts

@@ -9,6 +9,7 @@ import type { Representation } from '../../../src/ldp/representation/Representat
 import { RepresentationMetadata } from '../../../src/ldp/representation/RepresentationMetadata';
 import type { ResourceIdentifier } from '../../../src/ldp/representation/ResourceIdentifier';
 import type { DataAccessor } from '../../../src/storage/accessors/DataAccessor';
+import { BasicConditions } from '../../../src/storage/BasicConditions';
 import { DataAccessorBasedStore } from '../../../src/storage/DataAccessorBasedStore';
 import { INTERNAL_QUADS } from '../../../src/util/ContentTypes';
 import { BadRequestHttpError } from '../../../src/util/errors/BadRequestHttpError';
@@ -17,6 +18,7 @@ import { ForbiddenHttpError } from '../../../src/util/errors/ForbiddenHttpError'
 import { MethodNotAllowedHttpError } from '../../../src/util/errors/MethodNotAllowedHttpError';
 import { NotFoundHttpError } from '../../../src/util/errors/NotFoundHttpError';
 import { NotImplementedHttpError } from '../../../src/util/errors/NotImplementedHttpError';
+import { PreconditionFailedHttpError } from '../../../src/util/errors/PreconditionFailedHttpError';
 import type { Guarded } from '../../../src/util/GuardedStream';
 import { SingleRootIdentifierStrategy } from '../../../src/util/identifiers/SingleRootIdentifierStrategy';
 import { trimTrailingSlashes } from '../../../src/util/PathUtil';
@@ -117,8 +119,8 @@ class SimpleSuffixStrategy implements AuxiliaryStrategy {
 }
 
 describe('A DataAccessorBasedStore', (): void => {
-  const now = new Date(1234567489);
-  const later = new Date(987654321);
+  const now = new Date(2020, 5, 12);
+  const later = new Date(2021, 6, 13);
   let mockDate: jest.SpyInstance;
   let store: DataAccessorBasedStore;
   let accessor: SimpleDataAccessor;
@@ -233,6 +235,13 @@ describe('A DataAccessorBasedStore', (): void => {
       await expect(result).rejects.toThrow('The given path is not a container.');
     });
 
+    it('throws a 412 if the conditions are not matched.', async(): Promise<void> => {
+      const resourceID = { path: root };
+      const conditions = new BasicConditions({ notMatchesETag: [ '*' ]});
+      await expect(store.addResource(resourceID, representation, conditions))
+        .rejects.toThrow(PreconditionFailedHttpError);
+    });
+
     it('errors when trying to create a container with non-RDF data.', async(): Promise<void> => {
       const resourceID = { path: root };
       representation.metadata.add(RDF.type, LDP.terms.Container);
@@ -341,6 +350,13 @@ describe('A DataAccessorBasedStore', (): void => {
       await expect(prom).rejects.toThrow(ForbiddenHttpError);
     });
 
+    it('throws a 412 if the conditions are not matched.', async(): Promise<void> => {
+      const resourceID = { path: root };
+      const conditions = new BasicConditions({ notMatchesETag: [ '*' ]});
+      await expect(store.setRepresentation(resourceID, representation, conditions))
+        .rejects.toThrow(PreconditionFailedHttpError);
+    });
+
     // As discussed in #475, trimming the trailing slash of a root container in getNormalizedMetadata
     // can result in undefined behaviour since there is no parent container.
     it('will not trim the slash of root containers since there is no parent.', async(): Promise<void> => {
@@ -527,8 +543,33 @@ describe('A DataAccessorBasedStore', (): void => {
   });
 
   describe('modifying a Representation', (): void => {
+    it('throws a 412 if the conditions are not matched.', async(): Promise<void> => {
+      const resourceID = { path: root };
+      const conditions = new BasicConditions({ notMatchesETag: [ '*' ]});
+      await expect(store.modifyResource(resourceID, representation, conditions))
+        .rejects.toThrow(PreconditionFailedHttpError);
+    });
+
+    it('throws a 412 if the conditions are not matched on resources that do not exist.', async(): Promise<void> => {
+      const resourceID = { path: `${root}notHere` };
+      const conditions = new BasicConditions({ matchesETag: [ '*' ]});
+      await expect(store.modifyResource(resourceID, representation, conditions))
+        .rejects.toThrow(PreconditionFailedHttpError);
+    });
+
+    it('re-throws the error if something goes wrong accessing the metadata.', async(): Promise<void> => {
+      accessor.getMetadata = jest.fn(async(): Promise<any> => {
+        throw new Error('error');
+      });
+
+      const resourceID = { path: root };
+      const conditions = new BasicConditions({ notMatchesETag: [ '*' ]});
+      await expect(store.modifyResource(resourceID, representation, conditions))
+        .rejects.toThrow('error');
+    });
+
     it('is not supported.', async(): Promise<void> => {
-      const result = store.modifyResource();
+      const result = store.modifyResource({ path: root }, representation);
       await expect(result).rejects.toThrow(NotImplementedHttpError);
       await expect(result).rejects.toThrow('Patches are not supported by the default store.');
     });
@@ -569,6 +610,13 @@ describe('A DataAccessorBasedStore', (): void => {
       await expect(result).rejects.toThrow('Can only delete empty containers.');
     });
 
+    it('throws a 412 if the conditions are not matched.', async(): Promise<void> => {
+      const resourceID = { path: root };
+      const conditions = new BasicConditions({ notMatchesETag: [ '*' ]});
+      await expect(store.deleteResource(resourceID, conditions))
+        .rejects.toThrow(PreconditionFailedHttpError);
+    });
+
     it('will delete resources.', async(): Promise<void> => {
       accessor.data[`${root}resource`] = representation;
       await expect(store.deleteResource({ path: `${root}resource` })).resolves.toEqual([

test/unit/storage/PatchingStore.test.ts

@@ -2,6 +2,7 @@ import type { Patch } from '../../../src/ldp/http/Patch';
 import type { PatchHandler } from '../../../src/storage/patch/PatchHandler';
 import { PatchingStore } from '../../../src/storage/PatchingStore';
 import type { ResourceStore } from '../../../src/storage/ResourceStore';
+import { NotImplementedHttpError } from '../../../src/util/errors/NotImplementedHttpError';
 
 describe('A PatchingStore', (): void => {
   let store: PatchingStore;
@@ -26,15 +27,25 @@ describe('A PatchingStore', (): void => {
     expect(source.modifyResource).toHaveBeenLastCalledWith({ path: 'modifyPath' }, {}, undefined);
   });
 
-  it('calls its patcher if modifyResource failed.', async(): Promise<void> => {
+  it('calls its patcher if modifyResource is not implemented.', async(): Promise<void> => {
     source.modifyResource = jest.fn(async(): Promise<any> => {
-      throw new Error('dummy');
+      throw new NotImplementedHttpError();
     });
     await expect(store.modifyResource({ path: 'modifyPath' }, {} as Patch)).resolves.toBe('patcher');
     expect(source.modifyResource).toHaveBeenCalledTimes(1);
     expect(source.modifyResource).toHaveBeenLastCalledWith({ path: 'modifyPath' }, {}, undefined);
-    await expect((source.modifyResource as jest.Mock).mock.results[0].value).rejects.toThrow('dummy');
+    await expect((source.modifyResource as jest.Mock).mock.results[0].value).rejects.toThrow(NotImplementedHttpError);
     expect(handleSafeFn).toHaveBeenCalledTimes(1);
     expect(handleSafeFn).toHaveBeenLastCalledWith({ source, identifier: { path: 'modifyPath' }, patch: {}});
   });
+
+  it('rethrows source modifyResource errors.', async(): Promise<void> => {
+    source.modifyResource = jest.fn(async(): Promise<any> => {
+      throw new Error('dummy');
+    });
+    await expect(store.modifyResource({ path: 'modifyPath' }, {} as Patch)).rejects.toThrow('dummy');
+    expect(source.modifyResource).toHaveBeenCalledTimes(1);
+    expect(source.modifyResource).toHaveBeenLastCalledWith({ path: 'modifyPath' }, {}, undefined);
+    expect(handleSafeFn).toHaveBeenCalledTimes(0);
+  });
 });

test/unit/util/errors/HttpError.test.ts

@@ -7,6 +7,7 @@ import { InternalServerError } from '../../../../src/util/errors/InternalServerE
 import { MethodNotAllowedHttpError } from '../../../../src/util/errors/MethodNotAllowedHttpError';
 import { NotFoundHttpError } from '../../../../src/util/errors/NotFoundHttpError';
 import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError';
+import { PreconditionFailedHttpError } from '../../../../src/util/errors/PreconditionFailedHttpError';
 import { UnauthorizedHttpError } from '../../../../src/util/errors/UnauthorizedHttpError';
 import { UnsupportedMediaTypeHttpError } from '../../../../src/util/errors/UnsupportedMediaTypeHttpError';
 
@@ -25,7 +26,7 @@ describe('HttpError', (): void => {
     [ 'NotFoundHttpError', 404, NotFoundHttpError ],
     [ 'MethodNotAllowedHttpError', 405, MethodNotAllowedHttpError ],
     [ 'ConflictHttpError', 409, ConflictHttpError ],
-    [ 'MethodNotAllowedHttpError', 405, MethodNotAllowedHttpError ],
+    [ 'PreconditionFailedHttpError', 412, PreconditionFailedHttpError ],
     [ 'UnsupportedMediaTypeHttpError', 415, UnsupportedMediaTypeHttpError ],
     [ 'InternalServerError', 500, InternalServerError ],
     [ 'NotImplementedHttpError', 501, NotImplementedHttpError ],