From f636dd3cae0811c2fb8017229756e18c509f9380 Mon Sep 17 00:00:00 2001 From: Jesse Wright <63333554+jeswr@users.noreply.github.com> Date: Fri, 31 Jul 2026 03:44:47 +0100 Subject: [PATCH 1/8] perf(mapping): avoid O(folder) readdir when resolving document extensions ExtensionBasedMapper.mapUrlToDocumentPath did a full readdir(folder) on every document read without a known content-type, to discover the file's extension. For large directories - e.g. the internal account-index storage with tens of thousands of flat entries - this is O(folder size) per read, dominating CPU under auth/OIDC load: every login findByEmail / client lookup scanned the whole index directory (positive AND negative lookups). - Probe the exact file and the common `$.` variants via `stat` (O(1)) before falling back to readdir, so positive lookups avoid the scan. - Skip the readdir fallback entirely for the reserved `/.internal/` storage, whose resources are always JSON, so negative index lookups (e.g. a login for a non-existent email) are O(1) too. Pod resources keep the readdir fallback since they may use arbitrary extensions. Behaviour-preserving (verified by algorithm-equivalence tests across exact match, common/uncommon extension, empty name, directory entries; internal reads confirmed JSON-only). Measured on a production instance: node CPU 232% -> ~48% and stable, index directory scans ~90/3s -> 0, pod root read 3.08s -> ~80ms. Co-Authored-By: Claude Opus 4.8 --- src/storage/mapping/ExtensionBasedMapper.ts | 59 +++++++++++++++++-- .../mapping/ExtensionBasedMapper.test.ts | 43 ++++++++++++++ 2 files changed, 96 insertions(+), 6 deletions(-) diff --git a/src/storage/mapping/ExtensionBasedMapper.ts b/src/storage/mapping/ExtensionBasedMapper.ts index da148fcb96..e8494d7190 100644 --- a/src/storage/mapping/ExtensionBasedMapper.ts +++ b/src/storage/mapping/ExtensionBasedMapper.ts @@ -17,6 +17,18 @@ export class ExtensionBasedMapper extends BaseFileIdentifierMapper { private readonly customTypes: Record; private readonly customExtensions: Record; + /** + * Extensions probed directly (via `stat`) before falling back to a directory scan + * when resolving a document whose content-type is not known ahead of time. + * Ordered most-common-first. Any resource stored with an extension not in this list + * still resolves correctly through the `readdir` fallback in {@link mapUrlToDocumentPath}; + * this list only exists to avoid an O(folder size) scan on the common case, which is + * pathological for large internal index directories (tens of thousands of entries). + */ + private static readonly commonExtensions = [ + 'json', 'ttl', 'nq', 'nt', 'jsonld', 'trig', 'n3', 'rdf', 'html', 'txt', + ]; + public constructor( base: string, rootFilepath: string, @@ -50,12 +62,47 @@ export class ExtensionBasedMapper extends BaseFileIdentifierMapper { // Find a matching file const [ , folder, documentName ] = /^(.*\/)([^/]*)$/u.exec(filePath)!; let fileName: string | undefined; - try { - const files = await fsPromises.readdir(folder); - fileName = files.find((file): boolean => - file.startsWith(documentName) && /^(?:\$\..+)?$/u.test(file.slice(documentName.length))); - } catch { - // Parent folder does not exist (or is not a folder) + // Fast path: probe the exact file and the common `$.` variants directly with + // `stat` (O(1) each). This avoids a full `readdir` of `folder`, which is O(folder size) + // and becomes a severe bottleneck for large internal index directories where every + // lookup would otherwise scan tens of thousands of unrelated entries. + // The fixed candidate order below is safe even though it differs from the (arbitrary) + // `readdir` iteration order: a resource can only ever have a single stored representation, + // because `FileDataAccessor.verifyExistingExtension` removes any previously-stored file + // with a different extension on every write. So at most one candidate can match. + // Guard against an empty document name (a path ending in `/`): a `stat` of the folder + // itself would spuriously match. Such paths fall through to the `readdir` fallback below. + if (documentName) { + const candidates = [ documentName, ...ExtensionBasedMapper.commonExtensions.map( + (extension): string => `${documentName}$.${extension}`, + ) ]; + for (const candidate of candidates) { + try { + await fsPromises.stat(joinFilePath(folder, candidate)); + fileName = candidate; + break; + } catch { + // Candidate does not exist; try the next one. + } + } + } + // Fallback: resource stored with a less common extension (or an unusual name). + // Scan the folder exactly as before, preserving correctness for pod resources (which may + // use arbitrary extensions). Skipped for the reserved root-level internal storage + // (`/.internal/`), whose resources are always JSON and whose index directories can hold + // tens of thousands of entries — a scan there is unnecessary and pathological, and is the + // dominant cost of negative index lookups (e.g. a login for a non-existent email). The + // check anchors on the request path's first segment, so a nested `.internal` container + // (which could legitimately hold arbitrary extensions) still uses the readdir fallback. + const isInternalStorage = new URL(identifier.path).pathname.startsWith('/.internal/'); + if (!fileName && !isInternalStorage) { + try { + const files = await fsPromises.readdir(folder); + fileName = files.find((file): boolean => + file.startsWith(documentName) && /^(?:\$\..+)?$/u.test(file.slice(documentName.length))); + } catch { + // Parent folder does not exist (or is not a folder) + } } if (fileName) { filePath = joinFilePath(folder, fileName); diff --git a/test/unit/storage/mapping/ExtensionBasedMapper.test.ts b/test/unit/storage/mapping/ExtensionBasedMapper.test.ts index 7583110941..95617a430c 100644 --- a/test/unit/storage/mapping/ExtensionBasedMapper.test.ts +++ b/test/unit/storage/mapping/ExtensionBasedMapper.test.ts @@ -159,6 +159,49 @@ describe('An ExtensionBasedMapper', (): void => { isMetadata: false, }); }); + + it('resolves a common extension via a stat fast-path without scanning the directory.', + async(): Promise => { + fsPromises.stat = jest.fn().mockImplementation(async(path: string): Promise => { + if (path !== `${rootFilepath}test$.ttl`) { + throw new Error('ENOENT'); + } + }); + fsPromises.readdir.mockRejectedValue(new Error('readdir should not be called on the fast path')); + await expect(mapper.mapUrlToFilePath({ path: `${base}test` }, false)).resolves.toEqual({ + identifier: { path: `${base}test` }, + filePath: `${rootFilepath}test$.ttl`, + contentType: 'text/turtle', + isMetadata: false, + }); + expect(fsPromises.readdir).not.toHaveBeenCalled(); + }); + + it('does not scan the directory for a missing resource in the reserved /.internal/ storage.', + async(): Promise => { + fsPromises.stat = jest.fn().mockRejectedValue(new Error('ENOENT')); + fsPromises.readdir.mockRejectedValue(new Error('readdir should not be called for internal storage')); + const result = await mapper.mapUrlToFilePath({ path: `${base}.internal/accounts/index/missing` }, false); + expect(result).toMatchObject({ + identifier: { path: `${base}.internal/accounts/index/missing` }, + filePath: `${rootFilepath}.internal/accounts/index/missing`, + isMetadata: false, + }); + expect(fsPromises.readdir).not.toHaveBeenCalled(); + }); + + it('falls back to a directory scan for a pod resource with an uncommon extension.', + async(): Promise => { + fsPromises.stat = jest.fn().mockRejectedValue(new Error('ENOENT')); + fsPromises.readdir.mockReturnValue([ 'pic$.weird' ]); + const result = await mapper.mapUrlToFilePath({ path: `${base}pod/pic` }, false); + expect(result).toMatchObject({ + identifier: { path: `${base}pod/pic` }, + filePath: `${rootFilepath}pod/pic$.weird`, + isMetadata: false, + }); + expect(fsPromises.readdir).toHaveBeenCalledTimes(1); + }); }); describe('mapFilePathToUrl', (): void => { From c4902fe8ab2bf99b5977fe1d9c3dc73a454d3e13 Mon Sep 17 00:00:00 2001 From: Jesse Wright <63333554+jeswr@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:07:36 +0100 Subject: [PATCH 2/8] docs: trim extension lookup comments --- src/storage/mapping/ExtensionBasedMapper.ts | 32 ++++----------------- 1 file changed, 5 insertions(+), 27 deletions(-) diff --git a/src/storage/mapping/ExtensionBasedMapper.ts b/src/storage/mapping/ExtensionBasedMapper.ts index e8494d7190..7484111ead 100644 --- a/src/storage/mapping/ExtensionBasedMapper.ts +++ b/src/storage/mapping/ExtensionBasedMapper.ts @@ -17,14 +17,7 @@ export class ExtensionBasedMapper extends BaseFileIdentifierMapper { private readonly customTypes: Record; private readonly customExtensions: Record; - /** - * Extensions probed directly (via `stat`) before falling back to a directory scan - * when resolving a document whose content-type is not known ahead of time. - * Ordered most-common-first. Any resource stored with an extension not in this list - * still resolves correctly through the `readdir` fallback in {@link mapUrlToDocumentPath}; - * this list only exists to avoid an O(folder size) scan on the common case, which is - * pathological for large internal index directories (tens of thousands of entries). - */ + /** Extensions probed before falling back to a directory scan, ordered by expected frequency. */ private static readonly commonExtensions = [ 'json', 'ttl', 'nq', 'nt', 'jsonld', 'trig', 'n3', 'rdf', 'html', 'txt', ]; @@ -62,16 +55,8 @@ export class ExtensionBasedMapper extends BaseFileIdentifierMapper { // Find a matching file const [ , folder, documentName ] = /^(.*\/)([^/]*)$/u.exec(filePath)!; let fileName: string | undefined; - // Fast path: probe the exact file and the common `$.` variants directly with - // `stat` (O(1) each). This avoids a full `readdir` of `folder`, which is O(folder size) - // and becomes a severe bottleneck for large internal index directories where every - // lookup would otherwise scan tens of thousands of unrelated entries. - // The fixed candidate order below is safe even though it differs from the (arbitrary) - // `readdir` iteration order: a resource can only ever have a single stored representation, - // because `FileDataAccessor.verifyExistingExtension` removes any previously-stored file - // with a different extension on every write. So at most one candidate can match. - // Guard against an empty document name (a path ending in `/`): a `stat` of the folder - // itself would spuriously match. Such paths fall through to the `readdir` fallback below. + // Probe common forms before scanning the directory. + // An empty document name would cause `stat` to match the folder itself. if (documentName) { const candidates = [ documentName, ...ExtensionBasedMapper.commonExtensions.map( (extension): string => `${documentName}$.${extension}`, @@ -82,18 +67,11 @@ export class ExtensionBasedMapper extends BaseFileIdentifierMapper { fileName = candidate; break; } catch { - // Candidate does not exist; try the next one. + // Try the next candidate. } } } - // Fallback: resource stored with a less common extension (or an unusual name). - // Scan the folder exactly as before, preserving correctness for pod resources (which may - // use arbitrary extensions). Skipped for the reserved root-level internal storage - // (`/.internal/`), whose resources are always JSON and whose index directories can hold - // tens of thousands of entries — a scan there is unnecessary and pathological, and is the - // dominant cost of negative index lookups (e.g. a login for a non-existent email). The - // check anchors on the request path's first segment, so a nested `.internal` container - // (which could legitimately hold arbitrary extensions) still uses the readdir fallback. + // Internal resources use known extensions, so their potentially large directories need no fallback scan. const isInternalStorage = new URL(identifier.path).pathname.startsWith('/.internal/'); if (!fileName && !isInternalStorage) { try { From 308d62aaeca331ad81a123fb0dcb30df396daf31 Mon Sep 17 00:00:00 2001 From: Jesse Wright <63333554+jeswr@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:11:53 +0100 Subject: [PATCH 3/8] style(mapping): align extension lookup tests --- src/storage/mapping/ExtensionBasedMapper.ts | 11 ++- .../mapping/ExtensionBasedMapper.test.ts | 71 +++++++++---------- 2 files changed, 44 insertions(+), 38 deletions(-) diff --git a/src/storage/mapping/ExtensionBasedMapper.ts b/src/storage/mapping/ExtensionBasedMapper.ts index 7484111ead..6f36fd9cb9 100644 --- a/src/storage/mapping/ExtensionBasedMapper.ts +++ b/src/storage/mapping/ExtensionBasedMapper.ts @@ -19,7 +19,16 @@ export class ExtensionBasedMapper extends BaseFileIdentifierMapper { /** Extensions probed before falling back to a directory scan, ordered by expected frequency. */ private static readonly commonExtensions = [ - 'json', 'ttl', 'nq', 'nt', 'jsonld', 'trig', 'n3', 'rdf', 'html', 'txt', + 'json', + 'ttl', + 'nq', + 'nt', + 'jsonld', + 'trig', + 'n3', + 'rdf', + 'html', + 'txt', ]; public constructor( diff --git a/test/unit/storage/mapping/ExtensionBasedMapper.test.ts b/test/unit/storage/mapping/ExtensionBasedMapper.test.ts index 95617a430c..a9f0d35273 100644 --- a/test/unit/storage/mapping/ExtensionBasedMapper.test.ts +++ b/test/unit/storage/mapping/ExtensionBasedMapper.test.ts @@ -160,48 +160,45 @@ describe('An ExtensionBasedMapper', (): void => { }); }); - it('resolves a common extension via a stat fast-path without scanning the directory.', - async(): Promise => { - fsPromises.stat = jest.fn().mockImplementation(async(path: string): Promise => { - if (path !== `${rootFilepath}test$.ttl`) { - throw new Error('ENOENT'); - } - }); - fsPromises.readdir.mockRejectedValue(new Error('readdir should not be called on the fast path')); - await expect(mapper.mapUrlToFilePath({ path: `${base}test` }, false)).resolves.toEqual({ - identifier: { path: `${base}test` }, - filePath: `${rootFilepath}test$.ttl`, - contentType: 'text/turtle', - isMetadata: false, - }); - expect(fsPromises.readdir).not.toHaveBeenCalled(); + it('resolves a common extension via a stat fast-path without scanning the directory.', async(): Promise => { + jest.spyOn(fsPromises, 'stat').mockImplementation(async(path: string): Promise => { + if (path !== `${rootFilepath}test$.ttl`) { + throw new Error('ENOENT'); + } }); + fsPromises.readdir.mockRejectedValue(new Error('readdir should not be called on the fast path')); + await expect(mapper.mapUrlToFilePath({ path: `${base}test` }, false)).resolves.toEqual({ + identifier: { path: `${base}test` }, + filePath: `${rootFilepath}test$.ttl`, + contentType: 'text/turtle', + isMetadata: false, + }); + expect(fsPromises.readdir).not.toHaveBeenCalled(); + }); - it('does not scan the directory for a missing resource in the reserved /.internal/ storage.', - async(): Promise => { - fsPromises.stat = jest.fn().mockRejectedValue(new Error('ENOENT')); - fsPromises.readdir.mockRejectedValue(new Error('readdir should not be called for internal storage')); - const result = await mapper.mapUrlToFilePath({ path: `${base}.internal/accounts/index/missing` }, false); - expect(result).toMatchObject({ - identifier: { path: `${base}.internal/accounts/index/missing` }, - filePath: `${rootFilepath}.internal/accounts/index/missing`, - isMetadata: false, - }); - expect(fsPromises.readdir).not.toHaveBeenCalled(); + it('does not scan internal storage for a missing resource.', async(): Promise => { + jest.spyOn(fsPromises, 'stat').mockImplementation().mockRejectedValue(new Error('ENOENT')); + fsPromises.readdir.mockRejectedValue(new Error('readdir should not be called for internal storage')); + const result = await mapper.mapUrlToFilePath({ path: `${base}.internal/accounts/index/missing` }, false); + expect(result).toMatchObject({ + identifier: { path: `${base}.internal/accounts/index/missing` }, + filePath: `${rootFilepath}.internal/accounts/index/missing`, + isMetadata: false, }); + expect(fsPromises.readdir).not.toHaveBeenCalled(); + }); - it('falls back to a directory scan for a pod resource with an uncommon extension.', - async(): Promise => { - fsPromises.stat = jest.fn().mockRejectedValue(new Error('ENOENT')); - fsPromises.readdir.mockReturnValue([ 'pic$.weird' ]); - const result = await mapper.mapUrlToFilePath({ path: `${base}pod/pic` }, false); - expect(result).toMatchObject({ - identifier: { path: `${base}pod/pic` }, - filePath: `${rootFilepath}pod/pic$.weird`, - isMetadata: false, - }); - expect(fsPromises.readdir).toHaveBeenCalledTimes(1); + it('falls back to a directory scan for a pod resource with an uncommon extension.', async(): Promise => { + jest.spyOn(fsPromises, 'stat').mockImplementation().mockRejectedValue(new Error('ENOENT')); + fsPromises.readdir.mockReturnValue([ 'pic$.weird' ]); + const result = await mapper.mapUrlToFilePath({ path: `${base}pod/pic` }, false); + expect(result).toMatchObject({ + identifier: { path: `${base}pod/pic` }, + filePath: `${rootFilepath}pod/pic$.weird`, + isMetadata: false, }); + expect(fsPromises.readdir).toHaveBeenCalledTimes(1); + }); }); describe('mapFilePathToUrl', (): void => { From 0997a3a06345793e8b35641ebb55a32b93a687d4 Mon Sep 17 00:00:00 2001 From: Jesse Wright <63333554+jeswr@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:35:28 +0100 Subject: [PATCH 4/8] test(mapping): initialize stat mock --- test/unit/storage/mapping/ExtensionBasedMapper.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/unit/storage/mapping/ExtensionBasedMapper.test.ts b/test/unit/storage/mapping/ExtensionBasedMapper.test.ts index a9f0d35273..8e012701f0 100644 --- a/test/unit/storage/mapping/ExtensionBasedMapper.test.ts +++ b/test/unit/storage/mapping/ExtensionBasedMapper.test.ts @@ -20,6 +20,7 @@ describe('An ExtensionBasedMapper', (): void => { jest.clearAllMocks(); fs.promises = { readdir: jest.fn(), + stat: jest.fn().mockRejectedValue(new Error('ENOENT')), } as any; fsPromises = fs.promises as any; }); @@ -161,7 +162,7 @@ describe('An ExtensionBasedMapper', (): void => { }); it('resolves a common extension via a stat fast-path without scanning the directory.', async(): Promise => { - jest.spyOn(fsPromises, 'stat').mockImplementation(async(path: string): Promise => { + fsPromises.stat.mockImplementation(async(path: string): Promise => { if (path !== `${rootFilepath}test$.ttl`) { throw new Error('ENOENT'); } @@ -177,7 +178,7 @@ describe('An ExtensionBasedMapper', (): void => { }); it('does not scan internal storage for a missing resource.', async(): Promise => { - jest.spyOn(fsPromises, 'stat').mockImplementation().mockRejectedValue(new Error('ENOENT')); + fsPromises.stat.mockRejectedValue(new Error('ENOENT')); fsPromises.readdir.mockRejectedValue(new Error('readdir should not be called for internal storage')); const result = await mapper.mapUrlToFilePath({ path: `${base}.internal/accounts/index/missing` }, false); expect(result).toMatchObject({ @@ -189,7 +190,7 @@ describe('An ExtensionBasedMapper', (): void => { }); it('falls back to a directory scan for a pod resource with an uncommon extension.', async(): Promise => { - jest.spyOn(fsPromises, 'stat').mockImplementation().mockRejectedValue(new Error('ENOENT')); + fsPromises.stat.mockRejectedValue(new Error('ENOENT')); fsPromises.readdir.mockReturnValue([ 'pic$.weird' ]); const result = await mapper.mapUrlToFilePath({ path: `${base}pod/pic` }, false); expect(result).toMatchObject({ From 5338d47cf0895d4565cbc3820c23ac058b400776 Mon Sep 17 00:00:00 2001 From: Jesse Wright <63333554+jeswr@users.noreply.github.com> Date: Wed, 19 Aug 2026 00:11:49 +0100 Subject: [PATCH 5/8] refactor(mapping): route known internal content types --- config/util/identifiers/subdomain.json | 15 ++- config/util/identifiers/suffix.json | 13 ++- src/index.ts | 1 + .../mapping/ContainerContentTypeMapper.ts | 45 +++++++++ src/storage/mapping/ExtensionBasedMapper.ts | 46 ++------- .../ContainerContentTypeMapper.test.ts | 98 +++++++++++++++++++ .../mapping/ExtensionBasedMapper.test.ts | 41 -------- 7 files changed, 171 insertions(+), 88 deletions(-) create mode 100644 src/storage/mapping/ContainerContentTypeMapper.ts create mode 100644 test/unit/storage/mapping/ContainerContentTypeMapper.test.ts diff --git a/config/util/identifiers/subdomain.json b/config/util/identifiers/subdomain.json index a9fc9ae9e9..7d324883e9 100644 --- a/config/util/identifiers/subdomain.json +++ b/config/util/identifiers/subdomain.json @@ -16,10 +16,17 @@ { "comment": "Only required when using a file-based backend.", "@id": "urn:solid-server:default:FileIdentifierMapper", - "@type": "SubdomainExtensionBasedMapper", - "base": { "@id": "urn:solid-server:default:variable:baseUrl" }, - "rootFilepath": { "@id": "urn:solid-server:default:variable:rootFilePath" }, - "baseSubdomain": "www" + "@type": "ContainerContentTypeMapper", + "source": { + "@type": "SubdomainExtensionBasedMapper", + "base": { "@id": "urn:solid-server:default:variable:baseUrl" }, + "rootFilepath": { "@id": "urn:solid-server:default:variable:rootFilePath" }, + "baseSubdomain": "www" + }, + "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" }, + "container": "/.internal/", + "documentContentType": "application/json", + "metadataContentType": "text/turtle" } ] } diff --git a/config/util/identifiers/suffix.json b/config/util/identifiers/suffix.json index 5c2e65fe5c..036b1c992a 100644 --- a/config/util/identifiers/suffix.json +++ b/config/util/identifiers/suffix.json @@ -16,9 +16,16 @@ { "comment": "Only required when using a file-based backend.", "@id": "urn:solid-server:default:FileIdentifierMapper", - "@type": "ExtensionBasedMapper", - "base": { "@id": "urn:solid-server:default:variable:baseUrl" }, - "rootFilepath": { "@id": "urn:solid-server:default:variable:rootFilePath" } + "@type": "ContainerContentTypeMapper", + "source": { + "@type": "ExtensionBasedMapper", + "base": { "@id": "urn:solid-server:default:variable:baseUrl" }, + "rootFilepath": { "@id": "urn:solid-server:default:variable:rootFilePath" } + }, + "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" }, + "container": "/.internal/", + "documentContentType": "application/json", + "metadataContentType": "text/turtle" } ] } diff --git a/src/index.ts b/src/index.ts index d9312af1bd..63fb42dfb6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -492,6 +492,7 @@ export * from './storage/keyvalue/WrappedIndexedStorage'; // Storage/Mapping export * from './storage/mapping/BaseFileIdentifierMapper'; +export * from './storage/mapping/ContainerContentTypeMapper'; export * from './storage/mapping/ExtensionBasedMapper'; export * from './storage/mapping/FileIdentifierMapper'; export * from './storage/mapping/FixedContentTypeMapper'; diff --git a/src/storage/mapping/ContainerContentTypeMapper.ts b/src/storage/mapping/ContainerContentTypeMapper.ts new file mode 100644 index 0000000000..d1fe42e694 --- /dev/null +++ b/src/storage/mapping/ContainerContentTypeMapper.ts @@ -0,0 +1,45 @@ +import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier'; +import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; +import { ensureTrailingSlash, joinUrl } from '../../util/PathUtil'; +import type { FileIdentifierMapper, ResourceLink } from './FileIdentifierMapper'; + +/** + * Provides the content types for resources stored in a specific container. + */ +export class ContainerContentTypeMapper implements FileIdentifierMapper { + private readonly source: FileIdentifierMapper; + private readonly documentContentType: string; + private readonly metadataContentType: string; + private readonly containerUrl: string; + + public constructor( + source: FileIdentifierMapper, + baseUrl: string, + container: string, + documentContentType: string, + metadataContentType: string, + ) { + this.source = source; + this.documentContentType = documentContentType; + this.metadataContentType = metadataContentType; + this.containerUrl = ensureTrailingSlash(joinUrl(baseUrl, container)); + } + + public async mapUrlToFilePath(identifier: ResourceIdentifier, isMetadata: boolean, contentType?: string): + Promise { + if (identifier.path.startsWith(this.containerUrl)) { + const storedContentType = isMetadata ? this.metadataContentType : this.documentContentType; + if (contentType && contentType !== storedContentType) { + throw new NotImplementedHttpError( + `Unsupported content type ${contentType}, only ${storedContentType} is allowed`, + ); + } + contentType = storedContentType; + } + return this.source.mapUrlToFilePath(identifier, isMetadata, contentType); + } + + public async mapFilePathToUrl(filePath: string, isContainer: boolean): Promise { + return this.source.mapFilePathToUrl(filePath, isContainer); + } +} diff --git a/src/storage/mapping/ExtensionBasedMapper.ts b/src/storage/mapping/ExtensionBasedMapper.ts index 6f36fd9cb9..da148fcb96 100644 --- a/src/storage/mapping/ExtensionBasedMapper.ts +++ b/src/storage/mapping/ExtensionBasedMapper.ts @@ -17,20 +17,6 @@ export class ExtensionBasedMapper extends BaseFileIdentifierMapper { private readonly customTypes: Record; private readonly customExtensions: Record; - /** Extensions probed before falling back to a directory scan, ordered by expected frequency. */ - private static readonly commonExtensions = [ - 'json', - 'ttl', - 'nq', - 'nt', - 'jsonld', - 'trig', - 'n3', - 'rdf', - 'html', - 'txt', - ]; - public constructor( base: string, rootFilepath: string, @@ -64,32 +50,12 @@ export class ExtensionBasedMapper extends BaseFileIdentifierMapper { // Find a matching file const [ , folder, documentName ] = /^(.*\/)([^/]*)$/u.exec(filePath)!; let fileName: string | undefined; - // Probe common forms before scanning the directory. - // An empty document name would cause `stat` to match the folder itself. - if (documentName) { - const candidates = [ documentName, ...ExtensionBasedMapper.commonExtensions.map( - (extension): string => `${documentName}$.${extension}`, - ) ]; - for (const candidate of candidates) { - try { - await fsPromises.stat(joinFilePath(folder, candidate)); - fileName = candidate; - break; - } catch { - // Try the next candidate. - } - } - } - // Internal resources use known extensions, so their potentially large directories need no fallback scan. - const isInternalStorage = new URL(identifier.path).pathname.startsWith('/.internal/'); - if (!fileName && !isInternalStorage) { - try { - const files = await fsPromises.readdir(folder); - fileName = files.find((file): boolean => - file.startsWith(documentName) && /^(?:\$\..+)?$/u.test(file.slice(documentName.length))); - } catch { - // Parent folder does not exist (or is not a folder) - } + try { + const files = await fsPromises.readdir(folder); + fileName = files.find((file): boolean => + file.startsWith(documentName) && /^(?:\$\..+)?$/u.test(file.slice(documentName.length))); + } catch { + // Parent folder does not exist (or is not a folder) } if (fileName) { filePath = joinFilePath(folder, fileName); diff --git a/test/unit/storage/mapping/ContainerContentTypeMapper.test.ts b/test/unit/storage/mapping/ContainerContentTypeMapper.test.ts new file mode 100644 index 0000000000..124d05d917 --- /dev/null +++ b/test/unit/storage/mapping/ContainerContentTypeMapper.test.ts @@ -0,0 +1,98 @@ +import type { ResourceIdentifier } from '../../../../src/http/representation/ResourceIdentifier'; +import { ContainerContentTypeMapper } from '../../../../src/storage/mapping/ContainerContentTypeMapper'; +import { ExtensionBasedMapper } from '../../../../src/storage/mapping/ExtensionBasedMapper'; +import type { FileIdentifierMapper, ResourceLink } from '../../../../src/storage/mapping/FileIdentifierMapper'; +import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError'; + +describe('A ContainerContentTypeMapper', (): void => { + const baseUrl = 'http://example.com/'; + let source: jest.Mocked; + let mapper: ContainerContentTypeMapper; + + beforeEach((): void => { + source = { + mapUrlToFilePath: jest.fn< + Promise, + Parameters + >(async(identifier: ResourceIdentifier): Promise => ({ + identifier, + filePath: 'source', + isMetadata: false, + })), + mapFilePathToUrl: jest.fn< + Promise, + Parameters + >(async(): Promise => ({ + identifier: { path: 'source' }, + filePath: 'source', + isMetadata: false, + })), + }; + mapper = new ContainerContentTypeMapper( + source, + baseUrl, + '/.internal/', + 'application/json', + 'text/turtle', + ); + }); + + it('provides the document content type in the configured container.', async(): Promise => { + const identifier = { path: `${baseUrl}.internal/accounts/id` }; + await mapper.mapUrlToFilePath(identifier, false); + expect(source.mapUrlToFilePath).toHaveBeenCalledWith(identifier, false, 'application/json'); + }); + + it('provides the metadata content type in the configured container.', async(): Promise => { + const identifier = { path: `${baseUrl}.internal/accounts/id` }; + await mapper.mapUrlToFilePath(identifier, true); + expect(source.mapUrlToFilePath).toHaveBeenCalledWith(identifier, true, 'text/turtle'); + }); + + it('maps internal resources without changing existing file names.', async(): Promise => { + mapper = new ContainerContentTypeMapper( + new ExtensionBasedMapper(baseUrl, '/data/'), + baseUrl, + '/.internal/', + 'application/json', + 'text/turtle', + ); + await expect(mapper.mapUrlToFilePath({ path: `${baseUrl}.internal/id` }, false)).resolves.toMatchObject({ + filePath: '/data/.internal/id$.json', + contentType: 'application/json', + }); + await expect(mapper.mapUrlToFilePath({ path: `${baseUrl}.internal/id.json` }, false)).resolves.toMatchObject({ + filePath: '/data/.internal/id.json', + contentType: 'application/json', + }); + await expect(mapper.mapUrlToFilePath({ path: `${baseUrl}.internal/id` }, true)).resolves.toMatchObject({ + filePath: '/data/.internal/id.meta', + contentType: 'text/turtle', + }); + }); + + it('rejects another content type in the configured container.', async(): Promise => { + const identifier = { path: `${baseUrl}.internal/accounts/id` }; + await expect(mapper.mapUrlToFilePath(identifier, false, 'text/plain')).rejects + .toThrow(NotImplementedHttpError); + expect(source.mapUrlToFilePath).not.toHaveBeenCalled(); + }); + + it('preserves content types outside the configured container.', async(): Promise => { + const identifier = { path: `${baseUrl}pod/resource` }; + await mapper.mapUrlToFilePath(identifier, false, 'text/plain'); + expect(source.mapUrlToFilePath).toHaveBeenCalledWith(identifier, false, 'text/plain'); + }); + + it('does not match containers with the same prefix.', async(): Promise => { + const identifier = { path: `${baseUrl}.internal-other/resource` }; + await mapper.mapUrlToFilePath(identifier, false); + expect(source.mapUrlToFilePath).toHaveBeenCalledWith(identifier, false, undefined); + }); + + it('maps file paths through the source mapper.', async(): Promise => { + const filePath = '/data/.internal/accounts/id$.json'; + await mapper.mapFilePathToUrl(filePath, false); + expect(source.mapFilePathToUrl).toHaveBeenCalledWith(filePath, false); + }); +}); diff --git a/test/unit/storage/mapping/ExtensionBasedMapper.test.ts b/test/unit/storage/mapping/ExtensionBasedMapper.test.ts index 8e012701f0..7583110941 100644 --- a/test/unit/storage/mapping/ExtensionBasedMapper.test.ts +++ b/test/unit/storage/mapping/ExtensionBasedMapper.test.ts @@ -20,7 +20,6 @@ describe('An ExtensionBasedMapper', (): void => { jest.clearAllMocks(); fs.promises = { readdir: jest.fn(), - stat: jest.fn().mockRejectedValue(new Error('ENOENT')), } as any; fsPromises = fs.promises as any; }); @@ -160,46 +159,6 @@ describe('An ExtensionBasedMapper', (): void => { isMetadata: false, }); }); - - it('resolves a common extension via a stat fast-path without scanning the directory.', async(): Promise => { - fsPromises.stat.mockImplementation(async(path: string): Promise => { - if (path !== `${rootFilepath}test$.ttl`) { - throw new Error('ENOENT'); - } - }); - fsPromises.readdir.mockRejectedValue(new Error('readdir should not be called on the fast path')); - await expect(mapper.mapUrlToFilePath({ path: `${base}test` }, false)).resolves.toEqual({ - identifier: { path: `${base}test` }, - filePath: `${rootFilepath}test$.ttl`, - contentType: 'text/turtle', - isMetadata: false, - }); - expect(fsPromises.readdir).not.toHaveBeenCalled(); - }); - - it('does not scan internal storage for a missing resource.', async(): Promise => { - fsPromises.stat.mockRejectedValue(new Error('ENOENT')); - fsPromises.readdir.mockRejectedValue(new Error('readdir should not be called for internal storage')); - const result = await mapper.mapUrlToFilePath({ path: `${base}.internal/accounts/index/missing` }, false); - expect(result).toMatchObject({ - identifier: { path: `${base}.internal/accounts/index/missing` }, - filePath: `${rootFilepath}.internal/accounts/index/missing`, - isMetadata: false, - }); - expect(fsPromises.readdir).not.toHaveBeenCalled(); - }); - - it('falls back to a directory scan for a pod resource with an uncommon extension.', async(): Promise => { - fsPromises.stat.mockRejectedValue(new Error('ENOENT')); - fsPromises.readdir.mockReturnValue([ 'pic$.weird' ]); - const result = await mapper.mapUrlToFilePath({ path: `${base}pod/pic` }, false); - expect(result).toMatchObject({ - identifier: { path: `${base}pod/pic` }, - filePath: `${rootFilepath}pod/pic$.weird`, - isMetadata: false, - }); - expect(fsPromises.readdir).toHaveBeenCalledTimes(1); - }); }); describe('mapFilePathToUrl', (): void => { From fb2c04060dbc21dd27ee3449ddc4346e654cb9ca Mon Sep 17 00:00:00 2001 From: Jesse Wright <63333554+jeswr@users.noreply.github.com> Date: Wed, 19 Aug 2026 06:37:59 +0100 Subject: [PATCH 6/8] fix(mapping): preserve internal fast path for encoded URLs --- .../mapping/ContainerContentTypeMapper.ts | 22 +++++++++++++---- .../ContainerContentTypeMapper.test.ts | 24 +++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/storage/mapping/ContainerContentTypeMapper.ts b/src/storage/mapping/ContainerContentTypeMapper.ts index d1fe42e694..be3d355171 100644 --- a/src/storage/mapping/ContainerContentTypeMapper.ts +++ b/src/storage/mapping/ContainerContentTypeMapper.ts @@ -1,6 +1,6 @@ import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier'; import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; -import { ensureTrailingSlash, joinUrl } from '../../util/PathUtil'; +import { decodeUriPathComponents, ensureTrailingSlash, joinUrl, trimTrailingSlashes } from '../../util/PathUtil'; import type { FileIdentifierMapper, ResourceLink } from './FileIdentifierMapper'; /** @@ -10,7 +10,8 @@ export class ContainerContentTypeMapper implements FileIdentifierMapper { private readonly source: FileIdentifierMapper; private readonly documentContentType: string; private readonly metadataContentType: string; - private readonly containerUrl: string; + private readonly baseUrl: string; + private readonly containerPath: string; public constructor( source: FileIdentifierMapper, @@ -22,12 +23,14 @@ export class ContainerContentTypeMapper implements FileIdentifierMapper { this.source = source; this.documentContentType = documentContentType; this.metadataContentType = metadataContentType; - this.containerUrl = ensureTrailingSlash(joinUrl(baseUrl, container)); + this.baseUrl = trimTrailingSlashes(baseUrl); + const containerUrl = ensureTrailingSlash(joinUrl(baseUrl, container)); + this.containerPath = decodeUriPathComponents(containerUrl.slice(this.baseUrl.length)); } public async mapUrlToFilePath(identifier: ResourceIdentifier, isMetadata: boolean, contentType?: string): Promise { - if (identifier.path.startsWith(this.containerUrl)) { + if (this.isContained(identifier)) { const storedContentType = isMetadata ? this.metadataContentType : this.documentContentType; if (contentType && contentType !== storedContentType) { throw new NotImplementedHttpError( @@ -42,4 +45,15 @@ export class ContainerContentTypeMapper implements FileIdentifierMapper { public async mapFilePathToUrl(filePath: string, isContainer: boolean): Promise { return this.source.mapFilePathToUrl(filePath, isContainer); } + + private isContained(identifier: ResourceIdentifier): boolean { + if (!identifier.path.startsWith(this.baseUrl)) { + return false; + } + try { + return decodeUriPathComponents(identifier.path.slice(this.baseUrl.length)).startsWith(this.containerPath); + } catch { + return false; + } + } } diff --git a/test/unit/storage/mapping/ContainerContentTypeMapper.test.ts b/test/unit/storage/mapping/ContainerContentTypeMapper.test.ts index 124d05d917..99bff08206 100644 --- a/test/unit/storage/mapping/ContainerContentTypeMapper.test.ts +++ b/test/unit/storage/mapping/ContainerContentTypeMapper.test.ts @@ -1,15 +1,22 @@ +import fs from 'node:fs'; import type { ResourceIdentifier } from '../../../../src/http/representation/ResourceIdentifier'; import { ContainerContentTypeMapper } from '../../../../src/storage/mapping/ContainerContentTypeMapper'; import { ExtensionBasedMapper } from '../../../../src/storage/mapping/ExtensionBasedMapper'; import type { FileIdentifierMapper, ResourceLink } from '../../../../src/storage/mapping/FileIdentifierMapper'; import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError'; +jest.mock('node:fs'); + describe('A ContainerContentTypeMapper', (): void => { const baseUrl = 'http://example.com/'; + let fsPromises: Record; let source: jest.Mocked; let mapper: ContainerContentTypeMapper; beforeEach((): void => { + jest.clearAllMocks(); + fs.promises = { readdir: jest.fn() } as any; + fsPromises = fs.promises as any; source = { mapUrlToFilePath: jest.fn< Promise, @@ -69,6 +76,11 @@ describe('A ContainerContentTypeMapper', (): void => { filePath: '/data/.internal/id.meta', contentType: 'text/turtle', }); + await expect(mapper.mapUrlToFilePath({ path: `${baseUrl}%2Einternal/id` }, false)).resolves.toMatchObject({ + filePath: '/data/.internal/id$.json', + contentType: 'application/json', + }); + expect(fsPromises.readdir).not.toHaveBeenCalled(); }); it('rejects another content type in the configured container.', async(): Promise => { @@ -84,6 +96,18 @@ describe('A ContainerContentTypeMapper', (): void => { expect(source.mapUrlToFilePath).toHaveBeenCalledWith(identifier, false, 'text/plain'); }); + it('delegates identifiers outside the configured base URL.', async(): Promise => { + const identifier = { path: 'http://other.example/.internal/resource' }; + await mapper.mapUrlToFilePath(identifier, false); + expect(source.mapUrlToFilePath).toHaveBeenCalledWith(identifier, false, undefined); + }); + + it('delegates identifiers with invalid encoding.', async(): Promise => { + const identifier = { path: `${baseUrl}%` }; + await mapper.mapUrlToFilePath(identifier, false); + expect(source.mapUrlToFilePath).toHaveBeenCalledWith(identifier, false, undefined); + }); + it('does not match containers with the same prefix.', async(): Promise => { const identifier = { path: `${baseUrl}.internal-other/resource` }; await mapper.mapUrlToFilePath(identifier, false); From bdb7d4b5025200722ec49a946260111fe79d2388 Mon Sep 17 00:00:00 2001 From: Jesse Wright <63333554+jeswr@users.noreply.github.com> Date: Wed, 19 Aug 2026 06:56:30 +0100 Subject: [PATCH 7/8] test(integration): stop server before cleanup --- test/integration/Conditions.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/Conditions.test.ts b/test/integration/Conditions.test.ts index 04281a15cb..d27e65332f 100644 --- a/test/integration/Conditions.test.ts +++ b/test/integration/Conditions.test.ts @@ -63,8 +63,8 @@ describe.each(stores)('A server supporting conditions with %s', (name, { storeCo }); afterAll(async(): Promise => { - await teardown(); await app.stop(); + await teardown(); }); it('prevents operations on existing resources with "if-none-match: *" header.', async(): Promise => { From 4c8383e916c244a9925890ca279a3ea1be207b49 Mon Sep 17 00:00:00 2001 From: Jesse Wright <63333554+jeswr@users.noreply.github.com> Date: Wed, 19 Aug 2026 07:06:49 +0100 Subject: [PATCH 8/8] test(integration): retry temporary folder cleanup --- test/integration/Config.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/Config.ts b/test/integration/Config.ts index 1c0b5f373b..b217ab7499 100644 --- a/test/integration/Config.ts +++ b/test/integration/Config.ts @@ -1,6 +1,6 @@ +import { promises as fsPromises } from 'node:fs'; import type { IModuleState } from 'componentsjs'; import { ComponentsManager } from 'componentsjs'; -import { remove } from 'fs-extra'; import { joinFilePath } from '../../src/util/PathUtil'; let cachedModuleState: IModuleState; @@ -47,7 +47,7 @@ export function getTestFolder(name: string): string { } export async function removeFolder(folder: string): Promise { - await remove(folder); + await fsPromises.rm(folder, { force: true, maxRetries: 3, recursive: true }); } export function getDefaultVariables(port: number, baseUrl?: string): Record {