Repository: solid/community-server (ts)

Issue:
locks folder is visible in container listing when running from the file system

Pull request:
fix: Hide internal data by making it auxiliary
Closes #892 

I did not create an auxiliary strategy that identifies all resources starting with an `.` as auxiliary. Some reasons:
The strategy requires there to be an associated resource for an auxiliary resource. The most logical choice would be the container they are located in. The issue is that locking happens on the associated resource, so all access to any of those resources (in the same container) would require the same lock. Might not be horrible but perhaps also not great. On the other hand it is also not possible to construct an auxiliary identifier when taking an associated identifier as input. This is used when deleting a resource to first delete its auxiliary resources.

The locking issue is perhaps not that bad though, and the other parts might indicate an issue with the current implementation that can potentially be resolved, so if this is something that would be required we can make a new issue to support those as auxiliary resources.

Patch:
diff --git a/config/storage/key-value/resource-store.json b/config/storage/key-value/resource-store.json
index f6cbdcaa1..38efa90a3 100644
--- a/config/storage/key-value/resource-store.json
+++ b/config/storage/key-value/resource-store.json
@@ -56,6 +56,19 @@
           "PathBasedAuthorizer:_paths_value": { "@type": "DenyAllAuthorizer" }
         }
       ]
+    },
+    {
+      "comment": "Marks the /.internal/ storage container as an auxiliary resource, thereby hiding it from container representations.",
+      "@id": "urn:solid-server:default:AuxiliaryStrategy",
+      "RoutingAuxiliaryStrategy:_sources": [
+        {
+          "@type": "ComposedAuxiliaryStrategy",
+          "identifierStrategy": {
+            "@type": "SuffixAuxiliaryIdentifierStrategy",
+            "suffix": "/.internal/"
+          }
+        }
+      ]
     }
   ]
 }
diff --git a/config/util/auxiliary/strategies/acl.json b/config/util/auxiliary/strategies/acl.json
index ad8af3a02..931e1e1a3 100644
--- a/config/util/auxiliary/strategies/acl.json
+++ b/config/util/auxiliary/strategies/acl.json
@@ -15,7 +15,7 @@
         "@type": "RdfValidator",
         "converter": { "@id": "urn:solid-server:default:RepresentationConverter" }
       },
-      "isRootRequired": true
+      "requiredInRoot": true
     },
     {
       "@id": "urn:solid-server:default:AclIdentifierStrategy",
diff --git a/src/ldp/auxiliary/AuxiliaryStrategy.ts b/src/ldp/auxiliary/AuxiliaryStrategy.ts
index 1b2460703..c3eb27636 100644
--- a/src/ldp/auxiliary/AuxiliaryStrategy.ts
+++ b/src/ldp/auxiliary/AuxiliaryStrategy.ts
@@ -10,11 +10,11 @@ import type { AuxiliaryIdentifierStrategy } from './AuxiliaryIdentifierStrategy'
  */
 export interface AuxiliaryStrategy extends AuxiliaryIdentifierStrategy {
   /**
-   * Whether this auxiliary resource in a root storage container.
+   * Whether the root storage container requires this auxiliary resource to be present.
    * If yes, this means they can't be deleted individually from such a container.
    * @param identifier - Identifier of the auxiliary resource.
    */
-  isRootRequired: (identifier: ResourceIdentifier) => boolean;
+  isRequiredInRoot: (identifier: ResourceIdentifier) => boolean;
 
   /**
    * Adds metadata related to this auxiliary resource,
diff --git a/src/ldp/auxiliary/ComposedAuxiliaryStrategy.ts b/src/ldp/auxiliary/ComposedAuxiliaryStrategy.ts
index 4f1196b31..b253c37fd 100644
--- a/src/ldp/auxiliary/ComposedAuxiliaryStrategy.ts
+++ b/src/ldp/auxiliary/ComposedAuxiliaryStrategy.ts
@@ -14,14 +14,14 @@ export class ComposedAuxiliaryStrategy implements AuxiliaryStrategy {
   private readonly identifierStrategy: AuxiliaryIdentifierStrategy;
   private readonly metadataGenerator?: MetadataGenerator;
   private readonly validator?: Validator;
-  private readonly rootRequired: boolean;
+  private readonly requiredInRoot: boolean;
 
   public constructor(identifierStrategy: AuxiliaryIdentifierStrategy, metadataGenerator?: MetadataGenerator,
-    validator?: Validator, isRootRequired = false) {
+    validator?: Validator, requiredInRoot = false) {
     this.identifierStrategy = identifierStrategy;
     this.metadataGenerator = metadataGenerator;
     this.validator = validator;
-    this.rootRequired = isRootRequired;
+    this.requiredInRoot = requiredInRoot;
   }
 
   public getAuxiliaryIdentifier(identifier: ResourceIdentifier): ResourceIdentifier {
@@ -40,8 +40,8 @@ export class ComposedAuxiliaryStrategy implements AuxiliaryStrategy {
     return this.identifierStrategy.getAssociatedIdentifier(identifier);
   }
 
-  public isRootRequired(): boolean {
-    return this.rootRequired;
+  public isRequiredInRoot(): boolean {
+    return this.requiredInRoot;
   }
 
   public async addMetadata(metadata: RepresentationMetadata): Promise<void> {
diff --git a/src/ldp/auxiliary/RoutingAuxiliaryStrategy.ts b/src/ldp/auxiliary/RoutingAuxiliaryStrategy.ts
index 36047bcf0..ad93ba191 100644
--- a/src/ldp/auxiliary/RoutingAuxiliaryStrategy.ts
+++ b/src/ldp/auxiliary/RoutingAuxiliaryStrategy.ts
@@ -18,9 +18,9 @@ export class RoutingAuxiliaryStrategy extends RoutingAuxiliaryIdentifierStrategy
     super(sources);
   }
 
-  public isRootRequired(identifier: ResourceIdentifier): boolean {
+  public isRequiredInRoot(identifier: ResourceIdentifier): boolean {
     const source = this.getMatchingSource(identifier);
-    return source.isRootRequired(identifier);
+    return source.isRequiredInRoot(identifier);
   }
 
   public async addMetadata(metadata: RepresentationMetadata): Promise<void> {
diff --git a/src/storage/DataAccessorBasedStore.ts b/src/storage/DataAccessorBasedStore.ts
index bd1479312..3ab8c3ef4 100644
--- a/src/storage/DataAccessorBasedStore.ts
+++ b/src/storage/DataAccessorBasedStore.ts
@@ -235,7 +235,8 @@ export class DataAccessorBasedStore implements ResourceStore {
     if (this.isRootStorage(metadata)) {
       throw new MethodNotAllowedHttpError('Cannot delete a root storage container.');
     }
-    if (this.auxiliaryStrategy.isAuxiliaryIdentifier(identifier) && this.auxiliaryStrategy.isRootRequired(identifier)) {
+    if (this.auxiliaryStrategy.isAuxiliaryIdentifier(identifier) &&
+      this.auxiliaryStrategy.isRequiredInRoot(identifier)) {
       const associatedIdentifier = this.auxiliaryStrategy.getAssociatedIdentifier(identifier);
       const parentMetadata = await this.accessor.getMetadata(associatedIdentifier);
       if (this.isRootStorage(parentMetadata)) {


Test patch:
diff --git a/test/integration/LdpHandlerWithoutAuth.test.ts b/test/integration/LdpHandlerWithoutAuth.test.ts
index e5e1805a2..c68cf7027 100644
--- a/test/integration/LdpHandlerWithoutAuth.test.ts
+++ b/test/integration/LdpHandlerWithoutAuth.test.ts
@@ -1,6 +1,6 @@
 import { createReadStream } from 'fs';
 import fetch from 'cross-fetch';
-import { DataFactory, Parser } from 'n3';
+import { DataFactory, Parser, Store } from 'n3';
 import { joinFilePath, PIM, RDF } from '../../src/';
 import type { App } from '../../src/';
 import { LDP } from '../../src/util/Vocabularies';
@@ -90,9 +90,11 @@ describe.each(stores)('An LDP handler allowing all requests %s', (name, { storeC
 
     const parser = new Parser({ baseIRI: baseUrl });
     const quads = parser.parse(await response.text());
-    expect(quads.some((entry): boolean => entry.equals(
-      quad(namedNode(baseUrl), RDF.terms.type, LDP.terms.Container),
-    ))).toBe(true);
+    const store = new Store(quads);
+    expect(store.countQuads(namedNode(baseUrl), RDF.terms.type, LDP.terms.Container, null)).toBe(1);
+    const contains = store.getObjects(namedNode(baseUrl), LDP.terms.contains, null);
+    expect(contains).toHaveLength(1);
+    expect(contains[0].value).toBe(`${baseUrl}index.html`);
   });
 
   it('can add a document to the store, read it and delete it.', async(): Promise<void> => {
diff --git a/test/unit/ldp/auxiliary/ComposedAuxiliaryStrategy.test.ts b/test/unit/ldp/auxiliary/ComposedAuxiliaryStrategy.test.ts
index 8bd24a5a0..f09b7962b 100644
--- a/test/unit/ldp/auxiliary/ComposedAuxiliaryStrategy.test.ts
+++ b/test/unit/ldp/auxiliary/ComposedAuxiliaryStrategy.test.ts
@@ -45,8 +45,8 @@ describe('A ComposedAuxiliaryStrategy', (): void => {
     expect(identifierStrategy.isAuxiliaryIdentifier).toHaveBeenLastCalledWith(identifier);
   });
 
-  it('returns the injected value for isRootRequired.', async(): Promise<void> => {
-    expect(strategy.isRootRequired()).toBe(true);
+  it('returns the injected value for isRequiredInRoot.', async(): Promise<void> => {
+    expect(strategy.isRequiredInRoot()).toBe(true);
   });
 
   it('adds metadata through the MetadataGenerator.', async(): Promise<void> => {
@@ -63,9 +63,9 @@ describe('A ComposedAuxiliaryStrategy', (): void => {
     expect(validator.handleSafe).toHaveBeenLastCalledWith(representation);
   });
 
-  it('defaults isRootRequired to false.', async(): Promise<void> => {
+  it('defaults isRequiredInRoot to false.', async(): Promise<void> => {
     strategy = new ComposedAuxiliaryStrategy(identifierStrategy, metadataGenerator, validator);
-    expect(strategy.isRootRequired()).toBe(false);
+    expect(strategy.isRequiredInRoot()).toBe(false);
   });
 
   it('does not add metadata or validate if the corresponding classes are not injected.', async(): Promise<void> => {
diff --git a/test/unit/ldp/auxiliary/RoutingAuxiliaryStrategy.test.ts b/test/unit/ldp/auxiliary/RoutingAuxiliaryStrategy.test.ts
index 3165ce352..b239f31cf 100644
--- a/test/unit/ldp/auxiliary/RoutingAuxiliaryStrategy.test.ts
+++ b/test/unit/ldp/auxiliary/RoutingAuxiliaryStrategy.test.ts
@@ -27,7 +27,7 @@ class SimpleSuffixStrategy implements AuxiliaryStrategy {
     return { path: identifier.path.slice(0, -this.suffix.length) };
   }
 
-  public isRootRequired(): boolean {
+  public isRequiredInRoot(): boolean {
     return true;
   }
 
@@ -77,13 +77,13 @@ describe('A RoutingAuxiliaryStrategy', (): void => {
     expect(sources[1].addMetadata).toHaveBeenLastCalledWith(metadata);
   });
 
-  it('#isRootRequired returns the result of the correct source.', async(): Promise<void> => {
-    sources[0].isRootRequired = jest.fn();
-    sources[1].isRootRequired = jest.fn();
-    strategy.isRootRequired(dummy2Id);
-    expect(sources[0].isRootRequired).toHaveBeenCalledTimes(0);
-    expect(sources[1].isRootRequired).toHaveBeenCalledTimes(1);
-    expect(sources[1].isRootRequired).toHaveBeenLastCalledWith(dummy2Id);
+  it('#isRequiredInRoot returns the result of the correct source.', async(): Promise<void> => {
+    sources[0].isRequiredInRoot = jest.fn();
+    sources[1].isRequiredInRoot = jest.fn();
+    strategy.isRequiredInRoot(dummy2Id);
+    expect(sources[0].isRequiredInRoot).toHaveBeenCalledTimes(0);
+    expect(sources[1].isRequiredInRoot).toHaveBeenCalledTimes(1);
+    expect(sources[1].isRequiredInRoot).toHaveBeenLastCalledWith(dummy2Id);
   });
 
   it('#validates using the correct validator.', async(): Promise<void> => {
diff --git a/test/unit/storage/DataAccessorBasedStore.test.ts b/test/unit/storage/DataAccessorBasedStore.test.ts
index d88308502..a3ffcd5c9 100644
--- a/test/unit/storage/DataAccessorBasedStore.test.ts
+++ b/test/unit/storage/DataAccessorBasedStore.test.ts
@@ -107,7 +107,7 @@ class SimpleSuffixStrategy implements AuxiliaryStrategy {
     return { path: identifier.path.slice(0, -this.suffix.length) };
   }
 
-  public isRootRequired(): boolean {
+  public isRequiredInRoot(): boolean {
     return false;
   }
 
@@ -602,7 +602,7 @@ describe('A DataAccessorBasedStore', (): void => {
       storageMetadata.add(RDF.type, PIM.terms.Storage);
       accessor.data[`${root}container/`] = new BasicRepresentation(representation.data, storageMetadata);
       accessor.data[`${root}container/.dummy`] = representation;
-      auxiliaryStrategy.isRootRequired = jest.fn().mockReturnValue(true);
+      auxiliaryStrategy.isRequiredInRoot = jest.fn().mockReturnValue(true);
       const result = store.deleteResource({ path: `${root}container/.dummy` });
       await expect(result).rejects.toThrow(MethodNotAllowedHttpError);
       await expect(result).rejects.toThrow(
@@ -648,7 +648,7 @@ describe('A DataAccessorBasedStore', (): void => {
       const storageMetadata = new RepresentationMetadata(representation.metadata);
       accessor.data[`${root}container/`] = new BasicRepresentation(representation.data, storageMetadata);
       accessor.data[`${root}container/.dummy`] = representation;
-      auxiliaryStrategy.isRootRequired = jest.fn().mockReturnValue(true);
+      auxiliaryStrategy.isRequiredInRoot = jest.fn().mockReturnValue(true);
       await expect(store.deleteResource({ path: `${root}container/.dummy` })).resolves.toEqual([
         { path: `${root}container/.dummy` },
         { path: `${root}container/` },