From 063437e5c1b83a36978469bcb9fbc818fe627dcf Mon Sep 17 00:00:00 2001
From: Ruben Verborgh <ruben@verborgh.org>
Date: Fri, 4 Sep 2020 23:15:35 +0200
Subject: [PATCH] refactor: More precise error messages

---
 .../SparqlPatchPermissionsExtractor.ts        | 30 +++++++++----------
 .../SparqlPatchPermissionsExtractor.test.ts   |  8 ++---
 2 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/src/ldp/permissions/SparqlPatchPermissionsExtractor.ts b/src/ldp/permissions/SparqlPatchPermissionsExtractor.ts
index 903e70648..92f2bc169 100644
--- a/src/ldp/permissions/SparqlPatchPermissionsExtractor.ts
+++ b/src/ldp/permissions/SparqlPatchPermissionsExtractor.ts
@@ -12,31 +12,29 @@ import { PermissionsExtractor } from './PermissionsExtractor';
  * while DELETEs require write permissions as well.
  */
 export class SparqlPatchPermissionsExtractor extends PermissionsExtractor {
-  public async canHandle(input: Operation): Promise<void> {
-    if (input.method !== 'PATCH') {
-      throw new UnsupportedHttpError('Only PATCH operations are supported.');
+  public async canHandle({ method, body }: Operation): Promise<void> {
+    if (method !== 'PATCH') {
+      throw new UnsupportedHttpError(`Cannot determine permissions of ${method}, only PATCH.`);
     }
-    if (!input.body) {
-      throw new UnsupportedHttpError('PATCH body is required to determine permissions.');
+    if (!body) {
+      throw new UnsupportedHttpError('Cannot determine permissions of PATCH operations without a body.');
     }
-    if (!this.isSparql(input.body)) {
-      throw new UnsupportedHttpError('Only SPARQL update PATCHes are supported.');
+    if (!this.isSparql(body)) {
+      throw new UnsupportedHttpError('Cannot determine permissions of non-SPARQL patches.');
     }
-    if (!this.isDeleteInsert(input.body.algebra)) {
-      throw new UnsupportedHttpError('Only DELETE/INSERT SPARQL update operations are supported.');
+    if (!this.isDeleteInsert(body.algebra)) {
+      throw new UnsupportedHttpError('Cannot determine permissions of a PATCH without DELETE/INSERT.');
     }
   }
 
-  public async handle(input: Operation): Promise<PermissionSet> {
+  public async handle({ body }: Operation): Promise<PermissionSet> {
     // Verified in `canHandle` call
-    const op = (input.body as SparqlUpdatePatch).algebra as Algebra.DeleteInsert;
-
-    const read = false;
-    const write = this.needsWrite(op);
+    const update = (body as SparqlUpdatePatch).algebra as Algebra.DeleteInsert;
 
     // Since `append` is a specific type of write, it is true if `write` is true.
-    const append = write || this.needsAppend(op);
-
+    const read = false;
+    const write = this.needsWrite(update);
+    const append = write || this.needsAppend(update);
     return { read, write, append };
   }
 
diff --git a/test/unit/ldp/permissions/SparqlPatchPermissionsExtractor.test.ts b/test/unit/ldp/permissions/SparqlPatchPermissionsExtractor.test.ts
index 67b7f8e02..5bf2badc8 100644
--- a/test/unit/ldp/permissions/SparqlPatchPermissionsExtractor.test.ts
+++ b/test/unit/ldp/permissions/SparqlPatchPermissionsExtractor.test.ts
@@ -12,14 +12,14 @@ describe('A SparqlPatchPermissionsExtractor', (): void => {
     const operation = { method: 'PATCH', body: { algebra: factory.createDeleteInsert() }} as unknown as Operation;
     await expect(extractor.canHandle(operation)).resolves.toBeUndefined();
     await expect(extractor.canHandle({ ...operation, method: 'GET' }))
-      .rejects.toThrow(new UnsupportedHttpError('Only PATCH operations are supported.'));
+      .rejects.toThrow(new UnsupportedHttpError('Cannot determine permissions of GET, only PATCH.'));
     await expect(extractor.canHandle({ ...operation, body: undefined }))
-      .rejects.toThrow(new UnsupportedHttpError('PATCH body is required to determine permissions.'));
+      .rejects.toThrow(new UnsupportedHttpError('Cannot determine permissions of PATCH operations without a body.'));
     await expect(extractor.canHandle({ ...operation, body: {} as SparqlUpdatePatch }))
-      .rejects.toThrow(new UnsupportedHttpError('Only SPARQL update PATCHes are supported.'));
+      .rejects.toThrow(new UnsupportedHttpError('Cannot determine permissions of non-SPARQL patches.'));
     await expect(extractor.canHandle({ ...operation,
       body: { algebra: factory.createMove('DEFAULT', 'DEFAULT') } as unknown as SparqlUpdatePatch }))
-      .rejects.toThrow(new UnsupportedHttpError('Only DELETE/INSERT SPARQL update operations are supported.'));
+      .rejects.toThrow(new UnsupportedHttpError('Cannot determine permissions of a PATCH without DELETE/INSERT.'));
   });
 
   it('requires append for INSERT operations.', async(): Promise<void> => {