fix: Support entries function in JsonResourceStorage

src/storage/keyvalue/EncodingPathStorage.ts

@@ -41,6 +41,10 @@ export class EncodingPathStorage<T> implements KeyValueStorage<string, T> {
 
   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 ];
     }

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<T> implements KeyValueStorage<string, T> {
 
   public async get(key: string): Promise<T | undefined> {
     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<T> implements KeyValueStorage<string, T> {
   }
 
   public async has(key: string): Promise<boolean> {
-    const identifier = this.createIdentifier(key);
+    const identifier = this.keyToIdentifier(key);
     return await this.source.hasResource(identifier);
   }
 
   public async set(key: string, value: unknown): Promise<this> {
-    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<T> implements KeyValueStorage<string, T> {
 
   public async delete(key: string): Promise<boolean> {
     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<T> implements KeyValueStorage<string, T> {
     }
   }
 
-  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<Representation | undefined> {
+    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));
+  }
 }

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")

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<void> => {
+    // 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 ]);
+  });
 });

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<string, string>;
+
+  let store: jest.Mocked<ResourceStore>;
   let storage: JsonResourceStorage<unknown>;
 
   beforeEach(async(): Promise<void> => {
-    const data: Record<string, string> = { };
+    data = new Map<string, string>();
     store = {
-      async hasResource(identifier: ResourceIdentifier): Promise<boolean> {
-        return Boolean(data[identifier.path]);
-      },
-      async getRepresentation(identifier: ResourceIdentifier): Promise<Representation> {
+      hasResource: jest.fn(async(id: ResourceIdentifier): Promise<boolean> => data.has(id.path)),
+      getRepresentation: jest.fn(async(id: ResourceIdentifier): Promise<Representation> => {
+        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<void> => {
+        data.set(id.path, await readableToString(representation.data));
+      }),
+      deleteResource: jest.fn(async(identifier: ResourceIdentifier): Promise<void> => {
+        if (!data.has(identifier.path)) {
           throw new NotFoundHttpError();
         }
-        return new BasicRepresentation(data[identifier.path], identifier);
-      },
-      async setRepresentation(identifier: ResourceIdentifier, representation: Representation): Promise<void> {
-        data[identifier.path] = await readableToString(representation.data);
-      },
-      async deleteResource(identifier: ResourceIdentifier): Promise<void> {
-        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<void> => {
-    await expect(storage.get(identifier1)).resolves.toBeUndefined();
-  });
-
-  it('errors when trying to request entries.', async(): Promise<void> => {
-    expect((): never => storage.entries()).toThrow(NotImplementedHttpError);
+    await expect(storage.get(path1)).resolves.toBeUndefined();
   });
 
   it('returns data if it was set beforehand.', async(): Promise<void> => {
-    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<void> => {
-    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<void> => {
-    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<void> => {
-    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<void> => {
+    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<void> => {
-    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<void> => {
+    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<void> => {
+    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<void> => {
+    //
   });
 });

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');