Repository: solid/community-server (ts)

Issue:
Write to root container with SPARQL backend
It is not possible to write new metadata to the root container when using a `SparqlDataAccessor`. The culprit is here: https://github.com/solid/community-server/blob/7cae14acf7bd7230cf249a918398e7c253c290f2/src/storage/accessors/SparqlDataAccessor.ts#L114

Since the root container does not have a parent this is going to throw an error. Since the behaviour is going to be different for the root than for other container (regarding writing containment triples to the parent), we will need the `isRootContainer` check as described in https://github.com/solid/community-server/issues/394#issuecomment-739805093

Pull request description:
fix: Allow overwriting and deleting root container in SparqlDataAccessor 
Closes #410 . Didn't include tests for the memory store yet due to #417 but this part can already be merged.

Patch:
diff --git a/.eslintignore b/.eslintignore
index de4d1f007..18f2b36db 100644
--- a/.eslintignore
+++ b/.eslintignore
@@ -1,2 +1,3 @@
+coverage
 dist
 node_modules
diff --git a/.eslintrc.js b/.eslintrc.js
index 31fd967c9..960fc6e08 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -2,11 +2,13 @@ module.exports = {
   root: true,
   parser: '@typescript-eslint/parser',
   parserOptions: {
-    tsconfigRootDir: __dirname, // this is the reason this is a .js file
-    project: ['./tsconfig.json', './test/tsconfig.json'],
+    tsconfigRootDir: __dirname,
+    project: [ './tsconfig.json', './test/tsconfig.json' ],
   },
-  globals:  {
+  globals: {
+    // eslint-disable-next-line @typescript-eslint/naming-convention
     AsyncIterable: 'readonly',
+    // eslint-disable-next-line @typescript-eslint/naming-convention
     NodeJS: 'readonly',
   },
   plugins: [
@@ -16,86 +18,112 @@ module.exports = {
   ],
   extends: [
     'es/node',
+    'eslint:recommended',
     'plugin:import/errors',
     'plugin:import/warnings',
     'plugin:import/typescript',
   ],
   settings: {
     'import/resolver': {
-      'typescript': {
-        'alwaysTryTypes': true // always try to resolve types under `<root>@types` directory even it doesn't contain any source code, like `@types/rdf-js`
+      typescript: {
+        // Always try to resolve types under `<root>@types` directory
+        // even it doesn't contain any source code, like `@types/rdf-js`
+        alwaysTryTypes: true,
       },
-    }
+    },
   },
   rules: {
-    '@typescript-eslint/consistent-type-definitions': 'off', // there are valid typing reasons to have one or the other
+    // There are valid typing reasons to have one or the other
+    '@typescript-eslint/consistent-type-definitions': 'off',
     '@typescript-eslint/lines-between-class-members': [ 'error', { exceptAfterSingleLine: true }],
     '@typescript-eslint/no-empty-interface': 'off',
-    '@typescript-eslint/no-invalid-void-type': 'off', // breaks with default void in Asynchandler 2nd generic
-    '@typescript-eslint/no-unnecessary-condition': 'off', // problems with optional parameters
+    // Breaks with default void in AsyncHandler 2nd generic
+    '@typescript-eslint/no-invalid-void-type': 'off',
+    // Problems with optional parameters
+    '@typescript-eslint/no-unnecessary-condition': 'off',
     '@typescript-eslint/space-before-function-paren': [ 'error', 'never' ],
     '@typescript-eslint/unbound-method': 'off',
     '@typescript-eslint/unified-signatures': 'off',
-    'class-methods-use-this': 'off', // conflicts with functions from interfaces that sometimes don't require `this`
-    'comma-dangle': ['error', 'always-multiline'],
-    'dot-location': ['error', 'property'],
-    'generator-star-spacing': ['error', 'after'],
-    'lines-around-comment': 'off', // conflicts with padded-blocks
-    'lines-between-class-members': ['error', 'always', { exceptAfterSingleLine: true }],
-    'max-len': ['error', { code: 120, ignoreUrls: true }],
-    'new-cap': 'off', // used for RDF constants
-    'no-param-reassign': 'off', // necessary in constructor overloading
-    'no-underscore-dangle': 'off', // conflicts with external libraries
+    // Conflicts with functions from interfaces that sometimes don't require `this`
+    'class-methods-use-this': 'off',
+    'comma-dangle': [ 'error', 'always-multiline' ],
+    'dot-location': [ 'error', 'property' ],
+    'generator-star-spacing': [ 'error', 'after' ],
+    // Conflicts with padded-blocks
+    'lines-around-comment': 'off',
+    'lines-between-class-members': [ 'error', 'always', { exceptAfterSingleLine: true }],
+    'max-len': [ 'error', { code: 120, ignoreUrls: true }],
+    // Used for RDF constants
+    'new-cap': 'off',
+    // Necessary in constructor overloading
+    'no-param-reassign': 'off',
+    // Conflicts with external libraries
+    'no-underscore-dangle': 'off',
+    // Already checked by @typescript-eslint/no-unused-vars
+    'no-unused-vars': 'off',
     'padding-line-between-statements': 'off',
     'prefer-named-capture-group': 'off',
+    // Already generated by TypeScript
+    strict: 'off',
     'tsdoc/syntax': 'error',
     'unicorn/catch-error-name': 'off',
     'unicorn/import-index': 'off',
     'unicorn/import-style': 'off',
-    'unicorn/no-fn-reference-in-iterator': 'off', // this prevents some functional programming paradigms
+    // This prevents some functional programming paradigms
+    'unicorn/no-fn-reference-in-iterator': 'off',
     'unicorn/no-object-as-default-parameter': 'off',
     'unicorn/numeric-separators-style': 'off',
-    'unicorn/prefer-ternary': 'off', // can get ugly with large single statements
-    'yield-star-spacing': ['error', 'after'],
+    // Can get ugly with large single statements
+    'unicorn/prefer-ternary': 'off',
+    'yield-star-spacing': [ 'error', 'after' ],
 
     // Naming conventions
     '@typescript-eslint/naming-convention': [
       'error',
       {
         selector: 'default',
-        format: ['camelCase'],
+        format: [ 'camelCase' ],
         leadingUnderscore: 'forbid',
         trailingUnderscore: 'forbid',
       },
       {
         selector: 'variable',
-        format: ['camelCase', 'UPPER_CASE'],
+        format: [ 'camelCase', 'UPPER_CASE' ],
         leadingUnderscore: 'forbid',
         trailingUnderscore: 'forbid',
       },
       {
         selector: 'typeLike',
-        format: ['PascalCase'],
+        format: [ 'PascalCase' ],
       },
       {
         selector: [ 'typeParameter' ],
         format: [ 'PascalCase' ],
         prefix: [ 'T' ],
-      }
+      },
     ],
 
     // Import
-    '@typescript-eslint/consistent-type-imports': ['error', { prefer: 'type-imports' }],
-    'sort-imports': 'off', // Disabled in favor of eslint-plugin-import
-    'import/order': ['error', {
+    '@typescript-eslint/consistent-type-imports': [ 'error', { prefer: 'type-imports' }],
+    // Disabled in favor of eslint-plugin-import
+    'sort-imports': 'off',
+    'import/order': [ 'error', {
       alphabetize: {
         order: 'asc',
         caseInsensitive: true,
-      }
+      },
     }],
     'import/no-duplicates': 'error',
     'import/no-extraneous-dependencies': 'error',
-    'no-duplicate-imports': 'off', // doesn't work with type imports
+    // Doesn't work with type imports
+    'no-duplicate-imports': 'off',
     'unused-imports/no-unused-imports-ts': 'error',
   },
+
+  overrides: [
+    {
+      files: '*.js',
+      parser: 'espree',
+    },
+  ],
 };
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index aec4eed1e..2916ad70a 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -49,7 +49,7 @@ jobs:
       - name: Validate components files
         run: npm run validate
       - name: Type-check tests
-        run: npm run test:tsc
+        run: npm run test:ts
       - name: Run unit tests
         run: npm run test:unit
       - name: Submit unit test coverage
diff --git a/bin/server.js b/bin/server.js
index afaf57966..dcfa789f7 100755
--- a/bin/server.js
+++ b/bin/server.js
@@ -1,3 +1,3 @@
 #!/usr/bin/env node
-const { runCli } = require("..");
+const { runCli } = require('..');
 runCli();
diff --git a/jest.config.js b/jest.config.js
index 49758e919..de130e245 100644
--- a/jest.config.js
+++ b/jest.config.js
@@ -1,32 +1,33 @@
 module.exports = {
-  "globals": {
-    "ts-jest": {
-      "tsconfig": "tsconfig.json"
-    }
+  globals: {
+    'ts-jest': {
+      tsconfig: 'tsconfig.json',
+    },
   },
-  "transform": {
-    "^.+\\.ts$": "ts-jest"
+  transform: {
+    '^.+\\.ts$': 'ts-jest',
   },
-  "testRegex": "/test/(unit|integration)/.*\\.test\\.ts$",
-  "moduleFileExtensions": [
-    "ts",
-    "js"
+  testRegex: '/test/(unit|integration)/.*\\.test\\.ts$',
+  moduleFileExtensions: [
+    'ts',
+    'js',
   ],
-  "testEnvironment": "node",
-  "setupFilesAfterEnv": ["jest-rdf", "<rootDir>/test/util/SetupTests.ts"],
-  "collectCoverage": true,
-  "coveragePathIgnorePatterns": [
-    "/dist/",
-    "/node_modules/",
-    "/test/"
+  testEnvironment: 'node',
+  setupFilesAfterEnv: [ 'jest-rdf', '<rootDir>/test/util/SetupTests.ts' ],
+  collectCoverage: true,
+  coveragePathIgnorePatterns: [
+    '/dist/',
+    '/node_modules/',
+    '/test/',
   ],
-  "coverageThreshold": {
-    "./src": {
-      "branches": 100,
-      "functions": 100,
-      "lines": 100,
-      "statements": 100
-    }
+  coverageThreshold: {
+    './src': {
+      branches: 100,
+      functions: 100,
+      lines: 100,
+      statements: 100,
+    },
   },
-  "testTimeout": 10000, // Slower machines had problems calling the WebSocket integration callbacks on time
+  // Slower machines had problems calling the WebSocket integration callbacks on time
+  testTimeout: 10000,
 };
diff --git a/package.json b/package.json
index 3bddc94ef..aa08063d3 100644
--- a/package.json
+++ b/package.json
@@ -47,12 +47,12 @@
     "docker:start": "./test/docker/docker-start.sh",
     "docker:stop": "./test/docker/docker-stop.sh",
     "jest": "jest",
-    "lint": "eslint . --ext .ts --cache",
+    "lint": "eslint . --cache",
     "prepare": "npm run build",
     "start": "node ./bin/server.js -p 3000",
-    "test": "npm run test:tsc && npm run jest",
+    "test": "npm run test:ts && npm run jest",
     "test:deploy": "test/deploy/validate-package.sh",
-    "test:tsc": "tsc -p test --noEmit",
+    "test:ts": "tsc -p test --noEmit",
     "test:integration": "jest --coverageThreshold '{}' test/integration",
     "test:unit": "jest test/unit",
     "validate": "componentsjs-compile-config urn:solid-server:default:Initializer -c config/config-default.json -f > /dev/null",
diff --git a/src/storage/accessors/SparqlDataAccessor.ts b/src/storage/accessors/SparqlDataAccessor.ts
index 785d14dae..3bb9f13fe 100644
--- a/src/storage/accessors/SparqlDataAccessor.ts
+++ b/src/storage/accessors/SparqlDataAccessor.ts
@@ -113,7 +113,7 @@ export class SparqlDataAccessor implements DataAccessor {
    */
   public async writeContainer(identifier: ResourceIdentifier, metadata: RepresentationMetadata): Promise<void> {
     const { name, parent } = this.getRelatedNames(identifier);
-    return this.sendSparqlUpdate(this.sparqlInsert(name, parent, metadata));
+    return this.sendSparqlUpdate(this.sparqlInsert(name, metadata, parent));
   }
 
   /**
@@ -135,7 +135,7 @@ export class SparqlDataAccessor implements DataAccessor {
     // Not relevant since all content is triples
     metadata.removeAll(CONTENT_TYPE);
 
-    return this.sendSparqlUpdate(this.sparqlInsert(name, parent, metadata, triples));
+    return this.sendSparqlUpdate(this.sparqlInsert(name, metadata, parent, triples));
   }
 
   /**
@@ -148,10 +148,17 @@ export class SparqlDataAccessor implements DataAccessor {
 
   /**
    * Helper function to get named nodes corresponding to the identifier and its parent container.
+   * In case of a root container only the name will be returned.
    */
-  private getRelatedNames(identifier: ResourceIdentifier): { name: NamedNode; parent: NamedNode } {
-    const parentIdentifier = this.identifierStrategy.getParentContainer(identifier);
+  private getRelatedNames(identifier: ResourceIdentifier): { name: NamedNode; parent?: NamedNode } {
     const name = namedNode(identifier.path);
+
+    // Root containers don't have a parent
+    if (this.identifierStrategy.isRootContainer(identifier)) {
+      return { name };
+    }
+
+    const parentIdentifier = this.identifierStrategy.getParentContainer(identifier);
     const parent = namedNode(parentIdentifier.path);
     return { name, parent };
   }
@@ -219,14 +226,15 @@ export class SparqlDataAccessor implements DataAccessor {
    * @param metadata - New metadata of the resource.
    * @param triples - New data of the resource.
    */
-  private sparqlInsert(name: NamedNode, parent: NamedNode, metadata: RepresentationMetadata, triples?: Quad[]): Update {
+  private sparqlInsert(name: NamedNode, metadata: RepresentationMetadata, parent?: NamedNode, triples?: Quad[]):
+  Update {
     const metaName = this.getMetadataNode(name);
 
     // Insert new metadata and containment triple
-    const insert: GraphQuads[] = [
-      this.sparqlUpdateGraph(metaName, metadata.quads()),
-      this.sparqlUpdateGraph(parent, [ quad(parent, toNamedNode(LDP.contains), name) ]),
-    ];
+    const insert: GraphQuads[] = [ this.sparqlUpdateGraph(metaName, metadata.quads()) ];
+    if (parent) {
+      insert.push(this.sparqlUpdateGraph(parent, [ quad(parent, toNamedNode(LDP.contains), name) ]));
+    }
 
     // Necessary updates: delete metadata and insert new data
     const updates: UpdateOperation[] = [
@@ -256,21 +264,26 @@ export class SparqlDataAccessor implements DataAccessor {
   /**
    * Creates a query that deletes everything related to the given name.
    * @param name - Name of resource to delete.
-   * @param parent - Parent of the resource to delete so containment triple can be removed.
+   * @param parent - Parent of the resource to delete so the containment triple can be removed (unless root).
    */
-  private sparqlDelete(name: NamedNode, parent: NamedNode): Update {
-    return {
+  private sparqlDelete(name: NamedNode, parent?: NamedNode): Update {
+    const update: Update = {
       updates: [
         this.sparqlUpdateDeleteAll(name),
         this.sparqlUpdateDeleteAll(this.getMetadataNode(name)),
-        {
-          updateType: 'delete',
-          delete: [ this.sparqlUpdateGraph(parent, [ quad(parent, toNamedNode(LDP.contains), name) ]) ],
-        },
       ],
       type: 'update',
       prefixes: {},
     };
+
+    if (parent) {
+      update.updates.push({
+        updateType: 'delete',
+        delete: [ this.sparqlUpdateGraph(parent, [ quad(parent, toNamedNode(LDP.contains), name) ]) ],
+      });
+    }
+
+    return update;
   }
 
   /**
diff --git a/src/util/locking/SingleThreadedResourceLocker.ts b/src/util/locking/SingleThreadedResourceLocker.ts
index e981d25f5..082ff45f0 100644
--- a/src/util/locking/SingleThreadedResourceLocker.ts
+++ b/src/util/locking/SingleThreadedResourceLocker.ts
@@ -25,13 +25,16 @@ export class SingleThreadedResourceLocker implements ResourceLocker {
    */
   public async acquire(identifier: ResourceIdentifier): Promise<Lock> {
     this.logger.verbose(`Acquiring lock for ${identifier.path}`);
-    return new Promise(async(resolve): Promise<Lock> =>
+    return new Promise((resolve, reject): void => {
       this.locks.acquire(identifier.path, (done): void => {
         this.logger.verbose(`Acquired lock for ${identifier.path}`);
-        resolve({ release: async(): Promise<void> => {
-          this.logger.verbose(`Released lock for ${identifier.path}`);
-          done();
-        } });
-      }));
+        resolve({
+          release: async(): Promise<void> => {
+            this.logger.verbose(`Released lock for ${identifier.path}`);
+            done();
+          },
+        });
+      }).catch(reject);
+    });
   }
 }


Test patch:
diff --git a/test/unit/storage/accessors/FileDataAccessor.test.ts b/test/unit/storage/accessors/FileDataAccessor.test.ts
index 4e87a6aaa..769313d74 100644
--- a/test/unit/storage/accessors/FileDataAccessor.test.ts
+++ b/test/unit/storage/accessors/FileDataAccessor.test.ts
@@ -287,6 +287,12 @@ describe('A FileDataAccessor', (): void => {
       await expect(accessor.writeContainer({ path: `${base}container/` }, metadata)).resolves.toBeUndefined();
       expect(cache.data.container).toEqual({});
     });
+
+    it('can write to the root container.', async(): Promise<void> => {
+      metadata = new RepresentationMetadata({ path: `${base}` }, { likes: 'apples' });
+      await expect(accessor.writeContainer({ path: `${base}` }, metadata)).resolves.toBeUndefined();
+      expect(cache.data).toEqual({ '.meta': expect.stringMatching(`<${base}> <likes> "apples".`) });
+    });
   });
 
   describe('deleting a resource', (): void => {
@@ -334,5 +340,11 @@ describe('A FileDataAccessor', (): void => {
       await expect(accessor.deleteResource({ path: `${base}container/` })).resolves.toBeUndefined();
       expect(cache.data.container).toBeUndefined();
     });
+
+    it('can delete the root container.', async(): Promise<void> => {
+      cache.data = { };
+      await expect(accessor.deleteResource({ path: `${base}` })).resolves.toBeUndefined();
+      expect(cache.data).toBeUndefined();
+    });
   });
 });
diff --git a/test/unit/storage/accessors/SparqlDataAccessor.test.ts b/test/unit/storage/accessors/SparqlDataAccessor.test.ts
index 935c68780..4e4f3f8bb 100644
--- a/test/unit/storage/accessors/SparqlDataAccessor.test.ts
+++ b/test/unit/storage/accessors/SparqlDataAccessor.test.ts
@@ -172,6 +172,24 @@ describe('A SparqlDataAccessor', (): void => {
     ]));
   });
 
+  it('does not write containment triples when writing to a root container.', async(): Promise<void> => {
+    metadata = new RepresentationMetadata({ path: 'http://test.com/' },
+      { [RDF.type]: [ toNamedNode(LDP.Resource), toNamedNode(LDP.Container) ]});
+    await expect(accessor.writeContainer({ path: 'http://test.com/' }, metadata)).resolves.toBeUndefined();
+
+    expect(fetchUpdate).toHaveBeenCalledTimes(1);
+    expect(fetchUpdate.mock.calls[0][0]).toBe(endpoint);
+    expect(simplifyQuery(fetchUpdate.mock.calls[0][1])).toBe(simplifyQuery([
+      'DELETE WHERE { GRAPH <meta:http://test.com/> { ?s ?p ?o. } };',
+      'INSERT DATA {',
+      '  GRAPH <meta:http://test.com/> {',
+      '    <http://test.com/> <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <http://www.w3.org/ns/ldp#Resource>.',
+      '    <http://test.com/> <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <http://www.w3.org/ns/ldp#Container>.',
+      '  }',
+      '}',
+    ]));
+  });
+
   it('overwrites the data and metadata when writing a resource and updates parent.', async(): Promise<void> => {
     metadata = new RepresentationMetadata({ path: 'http://test.com/container/resource' },
       { [RDF.type]: [ toNamedNode(LDP.Resource) ]});
@@ -224,6 +242,19 @@ describe('A SparqlDataAccessor', (): void => {
     ]));
   });
 
+  it('does not try to remove containment triples when deleting a root container.', async(): Promise<void> => {
+    metadata = new RepresentationMetadata({ path: 'http://test.com/' },
+      { [RDF.type]: [ toNamedNode(LDP.Resource), toNamedNode(LDP.Container) ]});
+    await expect(accessor.deleteResource({ path: 'http://test.com/' })).resolves.toBeUndefined();
+
+    expect(fetchUpdate).toHaveBeenCalledTimes(1);
+    expect(fetchUpdate.mock.calls[0][0]).toBe(endpoint);
+    expect(simplifyQuery(fetchUpdate.mock.calls[0][1])).toBe(simplifyQuery([
+      'DELETE WHERE { GRAPH <http://test.com/> { ?s ?p ?o. } };',
+      'DELETE WHERE { GRAPH <meta:http://test.com/> { ?s ?p ?o. } }',
+    ]));
+  });
+
   it('errors when trying to write to a metadata document.', async(): Promise<void> => {
     await expect(accessor.writeDocument({ path: 'meta:http://test.com/container/resource' }, data, metadata))
       .rejects.toThrow(new ConflictHttpError('Not allowed to create NamedNodes with the metadata extension.'));