diff --git a/src/storage/keyvalue/EncodingPathStorage.ts b/src/storage/keyvalue/EncodingPathStorage.ts index bc29f4e8d..d92905bc7 100644 --- a/src/storage/keyvalue/EncodingPathStorage.ts +++ b/src/storage/keyvalue/EncodingPathStorage.ts @@ -41,6 +41,10 @@ export class EncodingPathStorage implements KeyValueStorage { public async* entries(): AsyncIterableIterator<[string, T]> { for await (const [ path, value ] of this.source.entries()) { + // The only relevant entries for this storage are those that start with the base path + if (!path.startsWith(this.basePath)) { + continue; + } const key = this.pathToKey(path); yield [ key, value ]; } diff --git a/src/storage/keyvalue/JsonResourceStorage.ts b/src/storage/keyvalue/JsonResourceStorage.ts index c8d747d35..21c1cf809 100644 --- a/src/storage/keyvalue/JsonResourceStorage.ts +++ b/src/storage/keyvalue/JsonResourceStorage.ts @@ -1,9 +1,10 @@ import { BasicRepresentation } from '../../http/representation/BasicRepresentation'; +import type { Representation } from '../../http/representation/Representation'; import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier'; import { NotFoundHttpError } from '../../util/errors/NotFoundHttpError'; -import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; -import { ensureTrailingSlash, joinUrl } from '../../util/PathUtil'; +import { ensureLeadingSlash, ensureTrailingSlash, isContainerIdentifier, joinUrl } from '../../util/PathUtil'; import { readableToString } from '../../util/StreamUtil'; +import { LDP } from '../../util/Vocabularies'; import type { ResourceStore } from '../ResourceStore'; import type { KeyValueStorage } from './KeyValueStorage'; @@ -11,6 +12,8 @@ import type { KeyValueStorage } from './KeyValueStorage'; * A {@link KeyValueStorage} for JSON-like objects using a {@link ResourceStore} as backend. * * Creates a base URL by joining the input base URL with the container string. + * The storage assumes it has ownership over all entries in the target container + * so no other classes should access resources there to prevent issues. * * Assumes the input keys can be safely used to generate identifiers, * which will be appended to the stored base URL. @@ -28,7 +31,7 @@ export class JsonResourceStorage implements KeyValueStorage { public async get(key: string): Promise { try { - const identifier = this.createIdentifier(key); + const identifier = this.keyToIdentifier(key); const representation = await this.source.getRepresentation(identifier, { type: { 'application/json': 1 }}); return JSON.parse(await readableToString(representation.data)); } catch (error: unknown) { @@ -39,12 +42,12 @@ export class JsonResourceStorage implements KeyValueStorage { } public async has(key: string): Promise { - const identifier = this.createIdentifier(key); + const identifier = this.keyToIdentifier(key); return await this.source.hasResource(identifier); } public async set(key: string, value: unknown): Promise { - const identifier = this.createIdentifier(key); + const identifier = this.keyToIdentifier(key); const representation = new BasicRepresentation(JSON.stringify(value), identifier, 'application/json'); await this.source.setRepresentation(identifier, representation); return this; @@ -52,7 +55,7 @@ export class JsonResourceStorage implements KeyValueStorage { public async delete(key: string): Promise { try { - const identifier = this.createIdentifier(key); + const identifier = this.keyToIdentifier(key); await this.source.deleteResource(identifier); return true; } catch (error: unknown) { @@ -63,15 +66,65 @@ export class JsonResourceStorage implements KeyValueStorage { } } - public entries(): never { - // There is no way of knowing which resources were added, or we should keep track in an index file - throw new NotImplementedHttpError(); + public async* entries(): AsyncIterableIterator<[string, T]> { + yield* this.getResourceEntries({ path: this.container }); + } + + /** + * Recursively iterates through the container to find all documents. + */ + private async* getResourceEntries(identifier: ResourceIdentifier): AsyncIterableIterator<[string, T]> { + const representation = await this.safelyGetResource(identifier); + if (representation) { + if (isContainerIdentifier(identifier)) { + // Only need the metadata + representation.data.destroy(); + const members = representation.metadata.getAll(LDP.terms.contains).map((term): string => term.value); + for (const path of members) { + yield* this.getResourceEntries({ path }); + } + } else { + const json = JSON.parse(await readableToString(representation.data)); + yield [ this.identifierToKey(identifier), json ]; + } + } + } + + /** + * Returns the representation for the given identifier. + * Returns undefined if a 404 error is thrown. + * Re-throws the error in all other cases. + */ + private async safelyGetResource(identifier: ResourceIdentifier): Promise { + let representation: Representation | undefined; + try { + const preferences = isContainerIdentifier(identifier) ? {} : { type: { 'application/json': 1 }}; + representation = await this.source.getRepresentation(identifier, preferences); + } catch (error: unknown) { + // Can happen if resource is deleted by this point. + // When using this for internal data this can specifically happen quite often with locks. + if (!NotFoundHttpError.isInstance(error)) { + throw error; + } + } + return representation; } /** * Converts a key into an identifier for internal storage. */ - private createIdentifier(key: string): ResourceIdentifier { + private keyToIdentifier(key: string): ResourceIdentifier { return { path: joinUrl(this.container, key) }; } + + /** + * Converts an internal identifier to an external key. + */ + private identifierToKey(identifier: ResourceIdentifier): string { + // Due to the usage of `joinUrl` we don't know for sure if there was a preceding slash, + // so we always add one for consistency. + // In practice this would only be an issue if a class depends + // on the `entries` results matching a key that was sent before. + return ensureLeadingSlash(identifier.path.slice(this.container.length)); + } } diff --git a/src/util/PathUtil.ts b/src/util/PathUtil.ts index 29a1520d8..aa2f8a830 100644 --- a/src/util/PathUtil.ts +++ b/src/util/PathUtil.ts @@ -83,6 +83,19 @@ export function trimTrailingSlashes(path: string): string { return path.replace(/\/+$/u, ''); } +/** + * Makes sure the input path has exactly 1 slash at the beginning. + * Multiple slashes will get merged into one. + * If there is no slash it will be added. + * + * @param path - Path to check. + * + * @returns The potentially changed path. + */ +export function ensureLeadingSlash(path: string): string { + return path.replace(/^\/*/u, '/'); +} + /** * Extracts the extension (without dot) from a path. * Custom function since `path.extname` does not work on all cases (e.g. ".acl") diff --git a/test/unit/storage/keyvalue/EncodingPathStorage.test.ts b/test/unit/storage/keyvalue/EncodingPathStorage.test.ts index 7ab472206..6e1c7abe5 100644 --- a/test/unit/storage/keyvalue/EncodingPathStorage.test.ts +++ b/test/unit/storage/keyvalue/EncodingPathStorage.test.ts @@ -30,4 +30,22 @@ describe('An EncodingPathStorage', (): void => { await expect(storage.delete(key)).resolves.toBe(true); expect([ ...map.keys() ]).toHaveLength(0); }); + + it('only returns entries from the source storage matching the relative path.', async(): Promise => { + // Base 64 encoding of 'key' + const encodedKey = 'a2V5'; + const generatedPath = `${relativePath}${encodedKey}`; + const otherPath = `/otherContainer/${encodedKey}`; + const data = 'data'; + + map.set(generatedPath, data); + map.set(otherPath, data); + + const results = []; + for await (const entry of storage.entries()) { + results.push(entry); + } + expect(results).toHaveLength(1); + expect(results[0]).toEqual([ 'key', data ]); + }); }); diff --git a/test/unit/storage/keyvalue/JsonResourceStorage.test.ts b/test/unit/storage/keyvalue/JsonResourceStorage.test.ts index d6687027d..9757933b3 100644 --- a/test/unit/storage/keyvalue/JsonResourceStorage.test.ts +++ b/test/unit/storage/keyvalue/JsonResourceStorage.test.ts @@ -5,92 +5,125 @@ import type { ResourceIdentifier } from '../../../../src/http/representation/Res import { JsonResourceStorage } from '../../../../src/storage/keyvalue/JsonResourceStorage'; import type { ResourceStore } from '../../../../src/storage/ResourceStore'; import { NotFoundHttpError } from '../../../../src/util/errors/NotFoundHttpError'; -import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError'; +import { isContainerIdentifier } from '../../../../src/util/PathUtil'; import { readableToString } from '../../../../src/util/StreamUtil'; import { LDP } from '../../../../src/util/Vocabularies'; describe('A JsonResourceStorage', (): void => { const baseUrl = 'http://test.com/'; const container = '/data/'; - const identifier1 = 'http://test.com/foo'; - const identifier2 = 'http://test.com/bar'; - let store: ResourceStore; + const path1 = '/foo'; + const path2 = '/bar'; + const subPath = '/container/document'; + const containerIdentifier = 'http://test.com/data/'; + const subContainerIdentifier = 'http://test.com/data/container/'; + let data: Map; + + let store: jest.Mocked; let storage: JsonResourceStorage; beforeEach(async(): Promise => { - const data: Record = { }; + data = new Map(); store = { - async hasResource(identifier: ResourceIdentifier): Promise { - return Boolean(data[identifier.path]); - }, - async getRepresentation(identifier: ResourceIdentifier): Promise { + hasResource: jest.fn(async(id: ResourceIdentifier): Promise => data.has(id.path)), + getRepresentation: jest.fn(async(id: ResourceIdentifier): Promise => { + if (!data.has(id.path)) { + throw new NotFoundHttpError(); + } // Simulate container metadata - if (identifier.path === 'http://test.com/data/' && Object.keys(data).length > 0) { - const metadata = new RepresentationMetadata({ [LDP.contains]: Object.keys(data) }); + if (isContainerIdentifier(id)) { + const keys = [ ...data.keys() ].filter((key): boolean => key.startsWith(id.path) && + /^[^/]+\/?$/u.test(key.slice(id.path.length))); + const metadata = new RepresentationMetadata({ [LDP.contains]: keys }); return new BasicRepresentation('', metadata); } - if (!data[identifier.path]) { + return new BasicRepresentation(data.get(id.path)!, id); + }), + setRepresentation: jest.fn(async(id: ResourceIdentifier, representation: Representation): Promise => { + data.set(id.path, await readableToString(representation.data)); + }), + deleteResource: jest.fn(async(identifier: ResourceIdentifier): Promise => { + if (!data.has(identifier.path)) { throw new NotFoundHttpError(); } - return new BasicRepresentation(data[identifier.path], identifier); - }, - async setRepresentation(identifier: ResourceIdentifier, representation: Representation): Promise { - data[identifier.path] = await readableToString(representation.data); - }, - async deleteResource(identifier: ResourceIdentifier): Promise { - if (!data[identifier.path]) { - throw new NotFoundHttpError(); - } - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete data[identifier.path]; - }, + data.delete(identifier.path); + }), } as any; storage = new JsonResourceStorage(store, baseUrl, container); }); it('returns undefined if there is no matching data.', async(): Promise => { - await expect(storage.get(identifier1)).resolves.toBeUndefined(); - }); - - it('errors when trying to request entries.', async(): Promise => { - expect((): never => storage.entries()).toThrow(NotImplementedHttpError); + await expect(storage.get(path1)).resolves.toBeUndefined(); }); it('returns data if it was set beforehand.', async(): Promise => { - await expect(storage.set(identifier1, 'apple')).resolves.toBe(storage); - await expect(storage.get(identifier1)).resolves.toBe('apple'); + await expect(storage.set(path1, 'apple')).resolves.toBe(storage); + await expect(storage.get(path1)).resolves.toBe('apple'); }); it('can check if data is present.', async(): Promise => { - await expect(storage.has(identifier1)).resolves.toBe(false); - await expect(storage.set(identifier1, 'apple')).resolves.toBe(storage); - await expect(storage.has(identifier1)).resolves.toBe(true); + await expect(storage.has(path1)).resolves.toBe(false); + await expect(storage.set(path1, 'apple')).resolves.toBe(storage); + await expect(storage.has(path1)).resolves.toBe(true); }); it('can delete data.', async(): Promise => { - await expect(storage.has(identifier1)).resolves.toBe(false); - await expect(storage.delete(identifier1)).resolves.toBe(false); - await expect(storage.has(identifier1)).resolves.toBe(false); - await expect(storage.set(identifier1, 'apple')).resolves.toBe(storage); - await expect(storage.has(identifier1)).resolves.toBe(true); - await expect(storage.delete(identifier1)).resolves.toBe(true); - await expect(storage.has(identifier1)).resolves.toBe(false); + await expect(storage.has(path1)).resolves.toBe(false); + await expect(storage.delete(path1)).resolves.toBe(false); + await expect(storage.has(path1)).resolves.toBe(false); + await expect(storage.set(path1, 'apple')).resolves.toBe(storage); + await expect(storage.has(path1)).resolves.toBe(true); + await expect(storage.delete(path1)).resolves.toBe(true); + await expect(storage.has(path1)).resolves.toBe(false); }); - it('can handle multiple identifiers.', async(): Promise => { - await expect(storage.set(identifier1, 'apple')).resolves.toBe(storage); - await expect(storage.has(identifier1)).resolves.toBe(true); - await expect(storage.has(identifier2)).resolves.toBe(false); - await expect(storage.set(identifier2, 'pear')).resolves.toBe(storage); - await expect(storage.get(identifier1)).resolves.toBe('apple'); + it('can handle multiple paths.', async(): Promise => { + await expect(storage.set(path1, 'apple')).resolves.toBe(storage); + await expect(storage.has(path1)).resolves.toBe(true); + await expect(storage.has(path2)).resolves.toBe(false); + await expect(storage.set(path2, 'pear')).resolves.toBe(storage); + await expect(storage.get(path1)).resolves.toBe('apple'); }); it('re-throws errors thrown by the store.', async(): Promise => { - store.getRepresentation = jest.fn().mockRejectedValue(new Error('bad GET')); - await expect(storage.get(identifier1)).rejects.toThrow('bad GET'); + store.getRepresentation.mockRejectedValue(new Error('bad GET')); + await expect(storage.get(path1)).rejects.toThrow('bad GET'); + await expect(storage.entries().next()).rejects.toThrow('bad GET'); - store.deleteResource = jest.fn().mockRejectedValueOnce(new Error('bad DELETE')); - await expect(storage.delete(identifier1)).rejects.toThrow('bad DELETE'); + store.deleteResource.mockRejectedValueOnce(new Error('bad DELETE')); + await expect(storage.delete(path1)).rejects.toThrow('bad DELETE'); + }); + + it('returns no entries if no data was added.', async(): Promise => { + const entries = []; + for await (const entry of storage.entries()) { + entries.push(entry); + } + expect(entries).toHaveLength(0); + }); + + it('recursively accesses containers to find entries.', async(): Promise => { + await expect(storage.set(path1, 'path1')).resolves.toBe(storage); + await expect(storage.set(path2, 'path2')).resolves.toBe(storage); + await expect(storage.set(subPath, 'subDocument')).resolves.toBe(storage); + + // Need to manually insert the containers as they don't get created by the dummy store above + data.set(containerIdentifier, ''); + data.set(subContainerIdentifier, ''); + + const entries = []; + for await (const entry of storage.entries()) { + entries.push(entry); + } + expect(entries).toEqual([ + [ path1, 'path1' ], + [ path2, 'path2' ], + [ subPath, 'subDocument' ], + ]); + }); + + it('can handle resources being deleted while iterating in the entries call.', async(): Promise => { + // }); }); diff --git a/test/unit/util/PathUtil.test.ts b/test/unit/util/PathUtil.test.ts index d809d20b0..0d5c49ee4 100644 --- a/test/unit/util/PathUtil.test.ts +++ b/test/unit/util/PathUtil.test.ts @@ -7,6 +7,7 @@ import { createSubdomainRegexp, decodeUriPathComponents, encodeUriPathComponents, + ensureLeadingSlash, ensureTrailingSlash, extractScheme, getExtension, @@ -77,6 +78,15 @@ describe('PathUtil', (): void => { }); }); + describe('#ensureLeadingSlash', (): void => { + it('makes sure there is always exactly 1 slash.', (): void => { + expect(ensureLeadingSlash('test')).toBe('/test'); + expect(ensureLeadingSlash('/test')).toBe('/test'); + expect(ensureLeadingSlash('//test')).toBe('/test'); + expect(ensureLeadingSlash('///test')).toBe('/test'); + }); + }); + describe('#getExtension', (): void => { it('returns the extension of a path.', (): void => { expect(getExtension('/a/b.txt')).toBe('txt');