fix: Replace inefficient storage detection

This replaces the recursive backend calls to find the storage
by a new class that is aware what the storage URLs look like.

config/identity/registration/disabled.json

@@ -3,6 +3,12 @@
   "@graph": [
     {
       "comment": "Disable registration by not attaching a registration handler."
+    },
+    {
+      "comment": "If registration is disabled, the base URL of the server is the root storage.",
+      "@id": "urn:solid-server:default:StorageLocationStrategy",
+      "@type": "RootStorageLocationStrategy",
+      "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" }
     }
   ]
 }

config/identity/registration/enabled.json

@@ -4,6 +4,12 @@
     "css:config/identity/registration/route/registration.json"
   ],
   "@graph": [
+    {
+      "comment": "If registration is enabled, the pod locations will be root storages.",
+      "@id": "urn:solid-server:default:StorageLocationStrategy",
+      "@type": "PodStorageLocationStrategy",
+      "generator": { "@id": "urn:solid-server:default:IdentifierGenerator" }
+    },
     {
       "@id": "urn:solid-server:auth:password:InteractionRouteHandler",
       "@type": "WaterfallHandler",

config/ldp/metadata-writer/writers/storage-description.json

@@ -5,10 +5,8 @@
       "comment": "Adds a link header pointing to the storage description resource.",
       "@id": "urn:solid-server:default:MetadataWriter_StorageDescription",
       "@type": "StorageDescriptionAdvertiser",
-      "targetExtractor": { "@id": "urn:solid-server:default:TargetExtractor" },
-      "identifierStrategy": { "@id": "urn:solid-server:default:IdentifierStrategy" },
-      "store": { "@id": "urn:solid-server:default:ResourceStore" },
-      "path": { "@id": "urn:solid-server:default:variable:storageDescriptionPath" }
+      "storageStrategy": { "@id": "urn:solid-server:default:StorageLocationStrategy" },
+      "relativePath": { "@id": "urn:solid-server:default:variable:storageDescriptionPath" }
     }
   ]
 }

src/http/output/metadata/StorageDescriptionAdvertiser.ts

@@ -1,11 +1,9 @@
 import { getLoggerFor } from '../../../logging/LogUtil';
-import type { ResourceStore } from '../../../storage/ResourceStore';
+import type { StorageLocationStrategy } from '../../../server/description/StorageLocationStrategy';
 import { createErrorMessage } from '../../../util/errors/ErrorUtil';
 import { addHeader } from '../../../util/HeaderUtil';
-import type { IdentifierStrategy } from '../../../util/identifiers/IdentifierStrategy';
 import { joinUrl } from '../../../util/PathUtil';
-import { LDP, PIM, RDF, SOLID } from '../../../util/Vocabularies';
-import type { TargetExtractor } from '../../input/identifier/TargetExtractor';
+import { LDP, RDF, SOLID } from '../../../util/Vocabularies';
 import type { ResourceIdentifier } from '../../representation/ResourceIdentifier';
 import type { MetadataWriterInput } from './MetadataWriter';
 import { MetadataWriter } from './MetadataWriter';
@@ -18,18 +16,13 @@ import { MetadataWriter } from './MetadataWriter';
 export class StorageDescriptionAdvertiser extends MetadataWriter {
   protected readonly logger = getLoggerFor(this);
 
-  private readonly targetExtractor: TargetExtractor;
-  private readonly identifierStrategy: IdentifierStrategy;
-  private readonly store: ResourceStore;
-  private readonly path: string;
+  private readonly storageStrategy: StorageLocationStrategy;
+  private readonly relativePath: string;
 
-  public constructor(targetExtractor: TargetExtractor, identifierStrategy: IdentifierStrategy, store: ResourceStore,
-    path: string) {
+  public constructor(storageStrategy: StorageLocationStrategy, relativePath: string) {
     super();
-    this.identifierStrategy = identifierStrategy;
-    this.targetExtractor = targetExtractor;
-    this.store = store;
-    this.path = path;
+    this.storageStrategy = storageStrategy;
+    this.relativePath = relativePath;
   }
 
   public async handle({ response, metadata }: MetadataWriterInput): Promise<void> {
@@ -40,22 +33,13 @@ export class StorageDescriptionAdvertiser extends MetadataWriter {
     const identifier = { path: metadata.identifier.value };
     let storageRoot: ResourceIdentifier;
     try {
-      storageRoot = await this.findStorageRoot(identifier);
+      storageRoot = await this.storageStrategy.getStorageIdentifier(identifier);
+      this.logger.debug(`Found storage root ${storageRoot.path}`);
     } catch (error: unknown) {
       this.logger.error(`Unable to find storage root: ${createErrorMessage(error)}`);
       return;
     }
-    const storageDescription = joinUrl(storageRoot.path, this.path);
+    const storageDescription = joinUrl(storageRoot.path, this.relativePath);
     addHeader(response, 'Link', `<${storageDescription}>; rel="${SOLID.storageDescription}"`);
   }
-
-  private async findStorageRoot(identifier: ResourceIdentifier): Promise<ResourceIdentifier> {
-    const representation = await this.store.getRepresentation(identifier, {});
-    // We only need the metadata
-    representation.data.destroy();
-    if (representation.metadata.has(RDF.terms.type, PIM.terms.Storage)) {
-      return identifier;
-    }
-    return this.findStorageRoot(this.identifierStrategy.getParentContainer(identifier));
-  }
 }

src/index.ts

@@ -295,9 +295,12 @@ export * from './server/WacAllowHttpHandler';
 export * from './server/WebSocketServerConfigurator';
 
 // Server/Description
+export * from './server/description/PodStorageLocationStrategy';
+export * from './server/description/RootStorageLocationStrategy';
 export * from './server/description/StaticStorageDescriber';
 export * from './server/description/StorageDescriber';
 export * from './server/description/StorageDescriptionHandler';
+export * from './server/description/StorageLocationStrategy';
 
 // Server/Middleware
 export * from './server/middleware/AcpHeaderHandler';

src/pods/generate/IdentifierGenerator.ts

@@ -9,4 +9,10 @@ export interface IdentifierGenerator {
    * This is simply string generation, no resource-related checks are run.
    */
   generate: (name: string) => ResourceIdentifier;
+
+  /**
+   * Extracts the root pod this identifier would be in.
+   * This assumes the identifier of that pod was generated by the same instance of this interface.
+   */
+  extractPod: (identifier: ResourceIdentifier) => ResourceIdentifier;
 }

src/pods/generate/SubdomainIdentifierGenerator.ts

@@ -1,4 +1,5 @@
 import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier';
+import { BadRequestHttpError } from '../../util/errors/BadRequestHttpError';
 import { ensureTrailingSlash, extractScheme } from '../../util/PathUtil';
 import { sanitizeUrlPart } from '../../util/StringUtil';
 import type { IdentifierGenerator } from './IdentifierGenerator';
@@ -19,4 +20,24 @@ export class SubdomainIdentifierGenerator implements IdentifierGenerator {
     const cleanName = sanitizeUrlPart(name);
     return { path: `${this.baseParts.scheme}${cleanName}.${this.baseParts.rest}` };
   }
+
+  public extractPod(identifier: ResourceIdentifier): ResourceIdentifier {
+    const { path } = identifier;
+
+    // Invalid identifiers that have no result should never reach this point,
+    // but some safety checks just in case.
+    if (!path.startsWith(this.baseParts.scheme)) {
+      throw new BadRequestHttpError(`Invalid identifier ${path}`);
+    }
+
+    const idx = path.indexOf(this.baseParts.rest);
+
+    // If the idx is smaller than this, either there was no match, or there is no subdomain
+    if (idx <= this.baseParts.scheme.length) {
+      throw new BadRequestHttpError(`Invalid identifier ${path}`);
+    }
+
+    // Slice of everything after the base URL tail
+    return { path: path.slice(0, idx + this.baseParts.rest.length) };
+  }
 }

src/pods/generate/SuffixIdentifierGenerator.ts

@@ -1,4 +1,5 @@
 import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier';
+import { BadRequestHttpError } from '../../util/errors/BadRequestHttpError';
 import { ensureTrailingSlash } from '../../util/PathUtil';
 import { sanitizeUrlPart } from '../../util/StringUtil';
 import type { IdentifierGenerator } from './IdentifierGenerator';
@@ -18,4 +19,24 @@ export class SuffixIdentifierGenerator implements IdentifierGenerator {
     const cleanName = sanitizeUrlPart(name);
     return { path: ensureTrailingSlash(new URL(cleanName, this.base).href) };
   }
+
+  public extractPod(identifier: ResourceIdentifier): ResourceIdentifier {
+    const { path } = identifier;
+
+    // Invalid identifiers that have no result should never reach this point,
+    // but some safety checks just in case.
+    if (!path.startsWith(this.base)) {
+      throw new BadRequestHttpError(`Invalid identifier ${path}`);
+    }
+
+    // The first slash after the base URL indicates the first container on the path
+    const idx = path.indexOf('/', this.base.length + 1);
+
+    if (idx < 0) {
+      throw new BadRequestHttpError(`Invalid identifier ${path}`);
+    }
+
+    // Slice of everything after the first container
+    return { path: path.slice(0, idx + 1) };
+  }
 }

src/server/description/PodStorageLocationStrategy.ts

@@ -0,0 +1,20 @@
+import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier';
+import type { IdentifierGenerator } from '../../pods/generate/IdentifierGenerator';
+import type { StorageLocationStrategy } from './StorageLocationStrategy';
+
+/**
+ * A {@link StorageLocationStrategy} to be used when the server has pods which each are a different storage.
+ * The {@link IdentifierGenerator} that is used to generate URLs for the pods
+ * is used here to determine what the root pod URL is.
+ */
+export class PodStorageLocationStrategy implements StorageLocationStrategy {
+  private readonly generator: IdentifierGenerator;
+
+  public constructor(generator: IdentifierGenerator) {
+    this.generator = generator;
+  }
+
+  public async getStorageIdentifier(identifier: ResourceIdentifier): Promise<ResourceIdentifier> {
+    return this.generator.extractPod(identifier);
+  }
+}

src/server/description/RootStorageLocationStrategy.ts

@@ -0,0 +1,17 @@
+import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier';
+import type { StorageLocationStrategy } from './StorageLocationStrategy';
+
+/**
+ * A {@link StorageLocationStrategy} to be used when the server has one storage in the root container of the server.
+ */
+export class RootStorageLocationStrategy implements StorageLocationStrategy {
+  private readonly root: ResourceIdentifier;
+
+  public constructor(baseUrl: string) {
+    this.root = { path: baseUrl };
+  }
+
+  public async getStorageIdentifier(): Promise<ResourceIdentifier> {
+    return this.root;
+  }
+}

src/server/description/StorageLocationStrategy.ts

@@ -0,0 +1,12 @@
+import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier';
+
+/**
+ * Interface used to find the storage a specific identifier is located in.
+ */
+export interface StorageLocationStrategy {
+  /**
+   * Returns the identifier of the storage that contains the given resource.
+   * Can error if the input identifier is not part of any storage.
+   */
+  getStorageIdentifier: (identifier: ResourceIdentifier) => Promise<ResourceIdentifier>;
+}

test/integration/config/ldp-with-acp.json

@@ -14,7 +14,7 @@
     "css:config/identity/handler/default.json",
     "css:config/identity/ownership/token.json",
     "css:config/identity/pod/static.json",
-
+    "css:config/identity/registration/disabled.json",
     "css:config/ldp/authentication/debug-auth-header.json",
     "css:config/ldp/authorization/acp.json",
     "css:config/ldp/handler/default.json",

test/integration/config/permission-table.json

@@ -14,7 +14,7 @@
     "css:config/identity/handler/default.json",
     "css:config/identity/ownership/token.json",
     "css:config/identity/pod/static.json",
-
+    "css:config/identity/registration/disabled.json",
     "css:config/ldp/authentication/debug-auth-header.json",
 
     "css:config/ldp/handler/default.json",

test/integration/config/server-redis-lock.json

@@ -14,7 +14,7 @@
     "css:config/identity/pod/static.json",
 
 
-
+    "css:config/identity/registration/disabled.json",
     "css:config/ldp/authentication/debug-auth-header.json",
     "css:config/ldp/authorization/allow-all.json",
     "css:config/ldp/handler/default.json",

test/integration/config/webhook-notifications.json

@@ -14,7 +14,7 @@
     "css:config/identity/handler/default.json",
     "css:config/identity/ownership/token.json",
     "css:config/identity/pod/static.json",
-    "css:config/identity/registration/enabled.json",
+    "css:config/identity/registration/disabled.json",
     "css:config/ldp/authentication/debug-auth-header.json",
     "css:config/ldp/authorization/webacl.json",
     "css:config/ldp/handler/default.json",

test/integration/config/websocket-notifications.json

@@ -14,7 +14,7 @@
     "css:config/identity/handler/default.json",
     "css:config/identity/ownership/token.json",
     "css:config/identity/pod/static.json",
-    "css:config/identity/registration/enabled.json",
+    "css:config/identity/registration/disabled.json",
     "css:config/ldp/authentication/debug-auth-header.json",
     "css:config/ldp/authorization/webacl.json",
     "css:config/ldp/handler/default.json",

test/unit/http/output/metadata/StorageDescriptionAdvertiser.test.ts

@@ -1,24 +1,18 @@
-import type { TargetExtractor } from '../../../../../src/http/input/identifier/TargetExtractor';
 import type { MetadataWriterInput } from '../../../../../src/http/output/metadata/MetadataWriter';
 import { StorageDescriptionAdvertiser } from '../../../../../src/http/output/metadata/StorageDescriptionAdvertiser';
-import { BasicRepresentation } from '../../../../../src/http/representation/BasicRepresentation';
 import { RepresentationMetadata } from '../../../../../src/http/representation/RepresentationMetadata';
-import type { ResourceIdentifier } from '../../../../../src/http/representation/ResourceIdentifier';
+import type { StorageLocationStrategy } from '../../../../../src/server/description/StorageLocationStrategy';
 import type { HttpResponse } from '../../../../../src/server/HttpResponse';
-import type { ResourceStore } from '../../../../../src/storage/ResourceStore';
-import { SingleRootIdentifierStrategy } from '../../../../../src/util/identifiers/SingleRootIdentifierStrategy';
-import { joinUrl } from '../../../../../src/util/PathUtil';
-import { LDP, PIM, RDF } from '../../../../../src/util/Vocabularies';
+import { BadRequestHttpError } from '../../../../../src/util/errors/BadRequestHttpError';
+import { LDP, RDF } from '../../../../../src/util/Vocabularies';
 
 describe('A StorageDescriptionAdvertiser', (): void => {
   let metadata: RepresentationMetadata;
   let response: jest.Mocked<HttpResponse>;
   let input: MetadataWriterInput;
-  const baseUrl = 'http://example.com/';
-  const path = '.well-known/solid';
-  let targetExtractor: jest.Mocked<TargetExtractor>;
-  const identifierStrategy = new SingleRootIdentifierStrategy(baseUrl);
-  let store: jest.Mocked<ResourceStore>;
+  const storageIdentifier = { path: 'http://example.com/foo/' };
+  let strategy: jest.Mocked<StorageLocationStrategy>;
+  const relativePath = '.well-known/solid';
   let advertiser: StorageDescriptionAdvertiser;
 
   beforeEach(async(): Promise<void> => {
@@ -33,15 +27,11 @@ describe('A StorageDescriptionAdvertiser', (): void => {
 
     input = { metadata, response };
 
-    targetExtractor = {
-      handleSafe: jest.fn(({ request: req }): ResourceIdentifier => ({ path: joinUrl(baseUrl, req.url!) })),
-    } as any;
+    strategy = {
+      getStorageIdentifier: jest.fn().mockResolvedValue(storageIdentifier),
+    };
 
-    store = {
-      getRepresentation: jest.fn().mockResolvedValue(new BasicRepresentation('', { [RDF.type]: PIM.terms.Storage })),
-    } as any;
-
-    advertiser = new StorageDescriptionAdvertiser(targetExtractor, identifierStrategy, store, path);
+    advertiser = new StorageDescriptionAdvertiser(strategy, relativePath);
   });
 
   it('adds a storage description link header.', async(): Promise<void> => {
@@ -59,7 +49,7 @@ describe('A StorageDescriptionAdvertiser', (): void => {
 
   it('does nothing if it cannot find a storage root.', async(): Promise<void> => {
     // No storage container will be found
-    store.getRepresentation.mockResolvedValue(new BasicRepresentation());
+    strategy.getStorageIdentifier.mockRejectedValue(new BadRequestHttpError('bad identifier'));
     await expect(advertiser.handle(input)).resolves.toBeUndefined();
     expect(response.setHeader).toHaveBeenCalledTimes(0);
   });

test/unit/identity/interaction/email-password/util/RegistrationManager.test.ts

@@ -36,6 +36,7 @@ describe('A RegistrationManager', (): void => {
 
     identifierGenerator = {
       generate: jest.fn((name: string): ResourceIdentifier => ({ path: `${baseUrl}${name}/` })),
+      extractPod: jest.fn(),
     };
 
     ownershipValidator = {

test/unit/pods/generate/SubdomainIdentifierGenerator.test.ts

@@ -1,14 +1,35 @@
 import { SubdomainIdentifierGenerator } from '../../../../src/pods/generate/SubdomainIdentifierGenerator';
+import { BadRequestHttpError } from '../../../../src/util/errors/BadRequestHttpError';
 
 describe('A SubdomainIdentifierGenerator', (): void => {
-  const base = 'http://test.com/';
+  const base = 'http://example.com/';
   const generator = new SubdomainIdentifierGenerator(base);
 
   it('generates identifiers by using the slug as subdomain.', async(): Promise<void> => {
-    expect(generator.generate('slug')).toEqual({ path: 'http://slug.test.com/' });
+    expect(generator.generate('slug')).toEqual({ path: 'http://slug.example.com/' });
   });
 
   it('converts slugs using punycode.', async(): Promise<void> => {
-    expect(generator.generate('sàl/u㋡g')).toEqual({ path: 'http://s-l-u-g.test.com/' });
+    expect(generator.generate('sàl/u㋡g')).toEqual({ path: 'http://s-l-u-g.example.com/' });
+  });
+
+  it('can extract the pod from an identifier.', async(): Promise<void> => {
+    const identifier = { path: 'http://foo.example.com/bar/baz' };
+    expect(generator.extractPod(identifier)).toEqual({ path: 'http://foo.example.com/' });
+  });
+
+  it('can detect if the identifier itself is the pod.', async(): Promise<void> => {
+    const identifier = { path: 'http://foo.example.com/' };
+    expect(generator.extractPod(identifier)).toEqual({ path: 'http://foo.example.com/' });
+  });
+
+  it('errors when extracting if the identifier has the wrong scheme.', async(): Promise<void> => {
+    const identifier = { path: 'https://foo.example.com/bar/baz' };
+    expect((): any => generator.extractPod(identifier)).toThrow(BadRequestHttpError);
+  });
+
+  it('errors when extracting if there is no pod.', async(): Promise<void> => {
+    const identifier = { path: 'http://example.com/bar/baz' };
+    expect((): any => generator.extractPod(identifier)).toThrow(BadRequestHttpError);
   });
 });

test/unit/pods/generate/SuffixIdentifierGenerator.test.ts

@@ -1,7 +1,8 @@
 import { SuffixIdentifierGenerator } from '../../../../src/pods/generate/SuffixIdentifierGenerator';
+import { BadRequestHttpError } from '../../../../src/util/errors/BadRequestHttpError';
 
 describe('A SuffixIdentifierGenerator', (): void => {
-  const base = 'http://test.com/';
+  const base = 'http://example.com/';
   const generator = new SuffixIdentifierGenerator(base);
 
   it('generates identifiers by appending the slug.', async(): Promise<void> => {
@@ -11,4 +12,24 @@ describe('A SuffixIdentifierGenerator', (): void => {
   it('converts non-alphanumerics to dashes.', async(): Promise<void> => {
     expect(generator.generate('sàl/u㋡g')).toEqual({ path: `${base}s-l-u-g/` });
   });
+
+  it('can extract the pod from an identifier.', async(): Promise<void> => {
+    const identifier = { path: 'http://example.com/foo/bar/baz' };
+    expect(generator.extractPod(identifier)).toEqual({ path: 'http://example.com/foo/' });
+  });
+
+  it('can detect if the identifier itself is the pod.', async(): Promise<void> => {
+    const identifier = { path: 'http://example.com/foo/' };
+    expect(generator.extractPod(identifier)).toEqual({ path: 'http://example.com/foo/' });
+  });
+
+  it('errors when extracting if the identifier is in the wrong domain.', async(): Promise<void> => {
+    const identifier = { path: 'http://bad.example.com/foo/bar/baz' };
+    expect((): any => generator.extractPod(identifier)).toThrow(BadRequestHttpError);
+  });
+
+  it('errors when extracting if there is no pod.', async(): Promise<void> => {
+    const identifier = { path: 'http://example.com/foo' };
+    expect((): any => generator.extractPod(identifier)).toThrow(BadRequestHttpError);
+  });
 });

test/unit/server/description/PodStorageLocationStrategy.test.ts

@@ -0,0 +1,15 @@
+import type { IdentifierGenerator } from '../../../../src/pods/generate/IdentifierGenerator';
+import { PodStorageLocationStrategy } from '../../../../src/server/description/PodStorageLocationStrategy';
+
+describe('A PodStorageLocationStrategy', (): void => {
+  const generator: IdentifierGenerator = {
+    generate: jest.fn(),
+    extractPod: jest.fn().mockReturnValue({ path: 'http://example.com/' }),
+  };
+  const strategy = new PodStorageLocationStrategy(generator);
+
+  it('returns the result of the identifier generator.', async(): Promise<void> => {
+    await expect(strategy.getStorageIdentifier({ path: 'http://example.com/whatever' }))
+      .resolves.toEqual({ path: 'http://example.com/' });
+  });
+});

test/unit/server/description/RootStorageLocationStrategy.test.ts

@@ -0,0 +1,10 @@
+import { RootStorageLocationStrategy } from '../../../../src/server/description/RootStorageLocationStrategy';
+
+describe('A RootStorageLocationStrategy', (): void => {
+  const baseUrl = 'http://example.com/';
+  const strategy = new RootStorageLocationStrategy(baseUrl);
+
+  it('returns the base URL.', async(): Promise<void> => {
+    await expect(strategy.getStorageIdentifier()).resolves.toEqual({ path: baseUrl });
+  });
+});