Repository: solid/community-server
Pull request #134 (2020-09-04 12:46:25)

Make deployment test install packages
The current deployment test would piggy-back on the existing `node_modules` instead of installing its own, and as such was not a good test to see if the main dependencies are there (https://github.com/solid/community-server/issues/132).

I've reorganized the script to make this happen.


Patch:
diff --git a/.travis/validate-package.sh b/.travis/validate-package.sh
index 4179e53cc..e9e0ac5ae 100755
--- a/.travis/validate-package.sh
+++ b/.travis/validate-package.sh
@@ -1,31 +1,32 @@
 #!/usr/bin/env bash
 # Script to validate our packed package
 
+echo "Building and installing package"
 npm pack
-tar -xzf solid-community-server-*.tgz
-pushd package
+npm install -g solid-community-server-*.tgz
+rm solid-community-server-*.tgz
 
-# Check if our server can start at a given port
-node bin/server.js -p 8888 &
-pid=$!
-i=0
-EXITCODE=0
-until curl -s localhost:8888; do
-  sleep 1
+echo "Starting the server"
+community-solid-server -p 8888 &
+PID=$!
 
-  # Try for at most 10 seconds, assume failure otherwise
-  let i++
-  if [ $i -gt 10 ]; then
-    echo "Server start timeout"
-    echo "  Server may have failed to start, or is running at an unexpected port."
-    kill -9 $pid
-    EXITCODE=1
-    break;
-  fi
-done > /dev/null
-kill -9 $pid
+echo "Attempting access over HTTP"
+FAILURE=1
+if [ -z $PID ]; then
+  echo "Server failed to start"
+else
+  for i in {1..10}; do
+    sleep 1
+    if curl -s localhost:8888; then
+      echo "Server reached"
+      FAILURE=0
+      break
+    fi
+  done
+  kill -9 $PID
+fi
 
-popd
-rm -r package
-rm solid-community-server-*.tgz
-exit $EXITCODE
+echo "Uninstalling package"
+npm uninstall -g @solid/community-server
+
+exit $FAILURE
diff --git a/config/presets/ldp/permissions-extractor.json b/config/presets/ldp/permissions-extractor.json
index 3356f0cf9..910bf7766 100644
--- a/config/presets/ldp/permissions-extractor.json
+++ b/config/presets/ldp/permissions-extractor.json
@@ -6,7 +6,7 @@
       "@type": "CompositeAsyncHandler",
       "CompositeAsyncHandler:_handlers": [
         {
-          "@type": "BasePermissionsExtractor"
+          "@type": "MethodPermissionsExtractor"
         },
         {
           "@type": "SparqlPatchPermissionsExtractor"
diff --git a/index.ts b/index.ts
index 4e74f2cd5..83bedcfbc 100644
--- a/index.ts
+++ b/index.ts
@@ -43,7 +43,7 @@ export * from './src/ldp/operations/ResponseDescription';
 // LDP/Permissions
 export * from './src/ldp/permissions/PermissionSet';
 export * from './src/ldp/permissions/PermissionsExtractor';
-export * from './src/ldp/permissions/BasePermissionsExtractor';
+export * from './src/ldp/permissions/MethodPermissionsExtractor';
 export * from './src/ldp/permissions/SparqlPatchPermissionsExtractor';
 
 // LDP/Representation
diff --git a/package.json b/package.json
index 1a31957d2..f1a65cf4c 100644
--- a/package.json
+++ b/package.json
@@ -13,6 +13,7 @@
     "Ruben Taelman <ruben.taelman@ugent.be> (https://www.rubensworks.net/)",
     "Ruben Verborgh <ruben.verborgh@ugent.be> (https://ruben.verborgh.org/)"
   ],
+  "license": "MIT",
   "lsd:module": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server",
   "lsd:components": "components/components.jsonld",
   "lsd:contexts": {
@@ -22,10 +23,10 @@
     "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^1.0.0/components/": "components/",
     "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^1.0.0/config/": "config/"
   },
-  "main": "index.js",
-  "license": "MIT",
+  "main": "./index.js",
+  "types": "./index.d.ts",
   "bin": {
-    "community-solid-server": "bin/server.js"
+    "community-solid-server": "./bin/server.js"
   },
   "repository": "git@github.com:solid/community-server.git",
   "bugs": {
diff --git a/src/ldp/permissions/BasePermissionsExtractor.ts b/src/ldp/permissions/BasePermissionsExtractor.ts
deleted file mode 100644
index 944f254e3..000000000
--- a/src/ldp/permissions/BasePermissionsExtractor.ts
+++ /dev/null
@@ -1,32 +0,0 @@
-import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError';
-import { Operation } from '../operations/Operation';
-import { PermissionSet } from './PermissionSet';
-import { PermissionsExtractor } from './PermissionsExtractor';
-
-/**
- * Generates permissions for the base set of methods that always require the same permissions.
- * Specifically: GET, HEAD, POST, PUT and DELETE.
- */
-export class BasePermissionsExtractor extends PermissionsExtractor {
-  public async canHandle(input: Operation): Promise<void> {
-    if (!/^(?:HEAD|GET|POST|PUT|DELETE)$/u.test(input.method)) {
-      throw new UnsupportedHttpError(`Unsupported method: ${input.method}`);
-    }
-  }
-
-  public async handle(input: Operation): Promise<PermissionSet> {
-    const requiredPermissions = {
-      read: /^(?:HEAD|GET)$/u.test(input.method),
-      append: false,
-      write: /^(?:POST|PUT|DELETE)$/u.test(input.method),
-    };
-
-    const read = /^(?:HEAD|GET)$/u.test(input.method);
-    const write = /^(?:POST|PUT|DELETE)$/u.test(input.method);
-
-    // Since `append` is a specific type of write, it is true if `write` is true.
-    const append = requiredPermissions.write;
-
-    return { read, append, write };
-  }
-}
diff --git a/src/ldp/permissions/MethodPermissionsExtractor.ts b/src/ldp/permissions/MethodPermissionsExtractor.ts
new file mode 100644
index 000000000..8a1954e98
--- /dev/null
+++ b/src/ldp/permissions/MethodPermissionsExtractor.ts
@@ -0,0 +1,26 @@
+import { UnsupportedHttpError } from '../../util/errors/UnsupportedHttpError';
+import { Operation } from '../operations/Operation';
+import { PermissionSet } from './PermissionSet';
+import { PermissionsExtractor } from './PermissionsExtractor';
+
+const READ_METHODS = new Set([ 'GET', 'HEAD' ]);
+const WRITE_METHODS = new Set([ 'POST', 'PUT', 'DELETE' ]);
+const SUPPORTED_METHODS = new Set([ ...READ_METHODS, ...WRITE_METHODS ]);
+
+/**
+ * Generates permissions for the base set of methods that always require the same permissions.
+ * Specifically: GET, HEAD, POST, PUT and DELETE.
+ */
+export class MethodPermissionsExtractor extends PermissionsExtractor {
+  public async canHandle({ method }: Operation): Promise<void> {
+    if (!SUPPORTED_METHODS.has(method)) {
+      throw new UnsupportedHttpError(`Cannot determine permissions of ${method}`);
+    }
+  }
+
+  public async handle({ method }: Operation): Promise<PermissionSet> {
+    const read = READ_METHODS.has(method);
+    const write = WRITE_METHODS.has(method);
+    return { read, write, append: write };
+  }
+}
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/src/storage/PatchingStore.ts b/src/storage/PatchingStore.ts
index b71cc7d23..0dd9e2eeb 100644
--- a/src/storage/PatchingStore.ts
+++ b/src/storage/PatchingStore.ts
@@ -8,7 +8,7 @@ import { ResourceStore } from './ResourceStore';
 /**
  * {@link ResourceStore} using decorator pattern for the `modifyResource` function.
  * If the original store supports the {@link Patch}, behaviour will be identical,
- * otherwise one of the {@link PatchHandler}s supporting the given Patch will be called instead.
+ * otherwise the {@link PatchHandler} will be called instead.
  */
 export class PatchingStore extends PassthroughStore {
   private readonly patcher: PatchHandler;


Test patch:
diff --git a/test/integration/AuthenticatedLdpHandler.test.ts b/test/integration/AuthenticatedLdpHandler.test.ts
index a07daf0a3..993779c89 100644
--- a/test/integration/AuthenticatedLdpHandler.test.ts
+++ b/test/integration/AuthenticatedLdpHandler.test.ts
@@ -19,7 +19,7 @@ import { Operation } from '../../src/ldp/operations/Operation';
 import { PatchOperationHandler } from '../../src/ldp/operations/PatchOperationHandler';
 import { PostOperationHandler } from '../../src/ldp/operations/PostOperationHandler';
 import { ResponseDescription } from '../../src/ldp/operations/ResponseDescription';
-import { BasePermissionsExtractor } from '../../src/ldp/permissions/BasePermissionsExtractor';
+import { MethodPermissionsExtractor } from '../../src/ldp/permissions/MethodPermissionsExtractor';
 import { SparqlPatchPermissionsExtractor } from '../../src/ldp/permissions/SparqlPatchPermissionsExtractor';
 import { Representation } from '../../src/ldp/representation/Representation';
 import { HttpRequest } from '../../src/server/HttpRequest';
@@ -42,7 +42,7 @@ describe('An integrated AuthenticatedLdpHandler', (): void => {
     });
 
     const credentialsExtractor = new UnsecureWebIdExtractor();
-    const permissionsExtractor = new BasePermissionsExtractor();
+    const permissionsExtractor = new MethodPermissionsExtractor();
     const authorizer = new AllowEverythingAuthorizer();
 
     const store = new InMemoryResourceStore(new RuntimeConfig({ base: 'http://test.com/' }));
@@ -111,7 +111,7 @@ describe('An integrated AuthenticatedLdpHandler', (): void => {
 
     const credentialsExtractor = new UnsecureWebIdExtractor();
     const permissionsExtractor = new CompositeAsyncHandler([
-      new BasePermissionsExtractor(),
+      new MethodPermissionsExtractor(),
       new SparqlPatchPermissionsExtractor(),
     ]);
     const authorizer = new AllowEverythingAuthorizer();
diff --git a/test/integration/Authorization.test.ts b/test/integration/Authorization.test.ts
index 6e764ecd8..02cdcfc30 100644
--- a/test/integration/Authorization.test.ts
+++ b/test/integration/Authorization.test.ts
@@ -17,7 +17,7 @@ import { Operation } from '../../src/ldp/operations/Operation';
 import { PostOperationHandler } from '../../src/ldp/operations/PostOperationHandler';
 import { PutOperationHandler } from '../../src/ldp/operations/PutOperationHandler';
 import { ResponseDescription } from '../../src/ldp/operations/ResponseDescription';
-import { BasePermissionsExtractor } from '../../src/ldp/permissions/BasePermissionsExtractor';
+import { MethodPermissionsExtractor } from '../../src/ldp/permissions/MethodPermissionsExtractor';
 import { PermissionSet } from '../../src/ldp/permissions/PermissionSet';
 import { QuadToTurtleConverter } from '../../src/storage/conversion/QuadToTurtleConverter';
 import { TurtleToQuadConverter } from '../../src/storage/conversion/TurtleToQuadConverter';
@@ -88,7 +88,7 @@ describe('A server with authorization', (): void => {
   const convertingStore = new RepresentationConvertingStore(store, converter);
 
   const credentialsExtractor = new UnsecureWebIdExtractor();
-  const permissionsExtractor = new BasePermissionsExtractor();
+  const permissionsExtractor = new MethodPermissionsExtractor();
   const authorizer = new WebAclAuthorizer(
     new UrlBasedAclManager(),
     new UrlContainerManager(new RuntimeConfig({ base: 'http://test.com/' })),
diff --git a/test/unit/ldp/permissions/BasePermissionsExtractor.test.ts b/test/unit/ldp/permissions/BasePermissionsExtractor.test.ts
index 0380a959f..3c49d8e7e 100644
--- a/test/unit/ldp/permissions/BasePermissionsExtractor.test.ts
+++ b/test/unit/ldp/permissions/BasePermissionsExtractor.test.ts
@@ -1,9 +1,9 @@
 import { Operation } from '../../../../src/ldp/operations/Operation';
-import { BasePermissionsExtractor } from '../../../../src/ldp/permissions/BasePermissionsExtractor';
+import { MethodPermissionsExtractor } from '../../../../src/ldp/permissions/MethodPermissionsExtractor';
 import { UnsupportedHttpError } from '../../../../src/util/errors/UnsupportedHttpError';
 
-describe('A BasePermissionsExtractor', (): void => {
-  const extractor = new BasePermissionsExtractor();
+describe('A MethodPermissionsExtractor', (): void => {
+  const extractor = new MethodPermissionsExtractor();
 
   it('can handle HEAD/GET/POST/PUT/DELETE.', async(): Promise<void> => {
     await expect(extractor.canHandle({ method: 'HEAD' } as Operation)).resolves.toBeUndefined();
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> => {