Merge pull request #506 from dahlia/kvstore-list

Make `KvStore.list()` method required instead of optional

Author: dahlia

CHANGES.md

@@ -90,6 +90,11 @@ To be released.
      -  Removed `@fedify/fedify/x/sveltekit` in favor of `@fedify/sveltekit`.
      -  Removed `@fedify/fedify/x/fresh` (Fresh integration). [[#466]]
 
+ -  The `KvStore.list()` method is now required instead of optional.
+    This method was added as optional in version 1.10.0 to give existing
+    implementations time to add support.  All official `KvStore` implementations
+    already support this method.  [[#499], [#506]]
+
 [#280]: https://github.com/fedify-dev/fedify/issues/280
 [#366]: https://github.com/fedify-dev/fedify/issues/366
 [#376]: https://github.com/fedify-dev/fedify/issues/376
@@ -102,6 +107,8 @@ To be released.
 [#451]: https://github.com/fedify-dev/fedify/pull/451
 [#391]: https://github.com/fedify-dev/fedify/pull/391
 [#466]: https://github.com/fedify-dev/fedify/issues/466
+[#499]: https://github.com/fedify-dev/fedify/issues/499
+[#506]: https://github.com/fedify-dev/fedify/pull/506
 
 ### @fedify/cli
 

docs/manual/kv.md

@@ -403,8 +403,8 @@ If the provided implementations don't meet your needs, you can create a custom
 ### Implement the `KvStore` interface
 
 Create a class that implements the `KvStore` interface.  The interface defines
-three methods: `~KvStore.get()`, `~KvStore.set()`, `~KvStore.delete()`, and
-optionally `~KvStore.cas()` and `~KvStore.list()`.
+four methods: `~KvStore.get()`, `~KvStore.set()`, `~KvStore.delete()`, and
+`~KvStore.list()`, and optionally `~KvStore.cas()`.
 
 ~~~~ typescript twoslash
 import type {
@@ -446,7 +446,7 @@ class MyCustomKvStore implements KvStore {
   }
 
   async *list(prefix?: KvKey): AsyncIterable<KvStoreListEntry> {
-    // Implement list logic if needed
+    // Implement list logic
   }
 }
 ~~~~
@@ -506,6 +506,7 @@ class MyCustomKvStore implements KvStore {
     options?: KvStoreSetOptions
   ): Promise<void> { }
   async delete(key: KvKey): Promise<void> { }
+  async *list(): AsyncIterable<{ key: KvKey; value: unknown }> { }
 // ---cut-before---
 async get<T = unknown>(key: KvKey): Promise<T | undefined> {
   const serializedKey = this.serializeKey(key);
@@ -555,6 +556,7 @@ class MyCustomKvStore implements KvStore {
     return undefined;
   }
   async delete(key: KvKey): Promise<void> { }
+  async *list(): AsyncIterable<{ key: KvKey; value: unknown }> { }
 // ---cut-before---
 async set(
   key: KvKey,
@@ -611,6 +613,7 @@ class MyCustomKvStore implements KvStore {
     value: unknown,
     options?: KvStoreSetOptions
   ): Promise<void> { }
+  async *list(): AsyncIterable<{ key: KvKey; value: unknown }> { }
 // ---cut-before---
 async delete(key: KvKey): Promise<void> {
   const serializedKey = this.serializeKey(key);
@@ -667,6 +670,7 @@ class MyCustomKvStore implements KvStore {
     options?: KvStoreSetOptions
   ): Promise<void> { }
   async delete(key: KvKey): Promise<void> { }
+  async *list(): AsyncIterable<{ key: KvKey; value: unknown }> { }
 // ---cut-before---
 async cas(
   key: KvKey,
@@ -684,14 +688,16 @@ async cas(
 }
 ~~~~
 
-### Implement `~KvStore.list()` method (optional)
+### Implement `~KvStore.list()` method
 
 *This API is available since Fedify 1.10.0.*
 
-If your storage backend supports prefix scanning, you can implement the
-`~KvStore.list()` method.  This method allows you to enumerate all entries
-whose keys start with a given prefix.  This is useful for implementing
-batch operations or iterating over related entries.
+*Since Fedify 2.0.0, this method is required instead of optional.*
+
+The `~KvStore.list()` method allows you to enumerate all entries whose keys
+start with a given prefix.  This is useful for implementing batch operations
+or iterating over related entries.  If your storage backend supports prefix
+scanning, you can use it to implement this method efficiently.
 
 ~~~~ typescript twoslash
 import type {
@@ -778,6 +784,8 @@ class MyCustomKvStore implements KvStore {
   }
   async delete(key: KvKey): Promise<void> {
   }
+  async *list(): AsyncIterable<{ key: KvKey; value: unknown }> {
+  }
 }
 // ---cut-before---
 import { createFederation } from "@fedify/fedify";

docs/package.json

@@ -62,7 +62,7 @@
   },
   "scripts": {
     "dev": "cd ../ && pnpm run --filter '!{docs}' --filter='!./examples/**' -r build && cd docs/ && vitepress dev --host",
-    "build": "cd ../ && pnpm run --filter '!{docs}' --filter '!./examples/**' -r build && cd docs/ && vitepress build",
+    "build": "cd ../ && pnpm run --filter '!{docs}' --filter '!./examples/**' -r build && cd docs/ && NODE_OPTIONS='--max-old-space-size=8192' vitepress build",
     "preview": "cd ../ && pnpm run --filter '!{docs}' --filter '!./examples/**' -r build && cd docs/ && vitepress preview"
   }
 }

packages/cfworkers/test/kv.test.ts

@@ -49,7 +49,7 @@ describe("WorkersKvStore", () => {
     await store.set(["list-other", "x"], "value-x");
 
     const entries: { key: readonly unknown[]; value: unknown }[] = [];
-    for await (const entry of store.list!(["list-prefix"])) {
+    for await (const entry of store.list(["list-prefix"])) {
       entries.push({ key: entry.key, value: entry.value });
     }
 
@@ -68,7 +68,7 @@ describe("WorkersKvStore", () => {
     await store.set(["single-b"], "value-b");
 
     const entries: { key: readonly unknown[]; value: unknown }[] = [];
-    for await (const entry of store.list!(["single-a"])) {
+    for await (const entry of store.list(["single-a"])) {
       entries.push({ key: entry.key, value: entry.value });
     }
 
@@ -85,7 +85,7 @@ describe("WorkersKvStore", () => {
     await store.set(["empty-test", "d", "e", "f"], "value-def");
 
     const entries: { key: readonly unknown[]; value: unknown }[] = [];
-    for await (const entry of store.list!()) {
+    for await (const entry of store.list()) {
       // Only count our test entries
       if (
         Array.isArray(entry.key) && entry.key[0] === "empty-test"

packages/denokv/src/mod.test.ts

@@ -41,7 +41,7 @@ Deno.test("DenoKvStore", async (t) => {
     await store.set(["other", "x"], "value-x");
 
     const entries: { key: Deno.KvKey; value: unknown }[] = [];
-    for await (const entry of store.list!(["prefix"])) {
+    for await (const entry of store.list(["prefix"])) {
       entries.push(entry);
     }
 
@@ -63,7 +63,7 @@ Deno.test("DenoKvStore", async (t) => {
     await store.set(["b"], "value-b");
 
     const entries: { key: Deno.KvKey; value: unknown }[] = [];
-    for await (const entry of store.list!(["a"])) {
+    for await (const entry of store.list(["a"])) {
       entries.push(entry);
     }
 
@@ -85,7 +85,7 @@ Deno.test("DenoKvStore", async (t) => {
       await store.set(["exact", "child2"], "child2-value");
 
       const entries: { key: Deno.KvKey; value: unknown }[] = [];
-      for await (const entry of store.list!(["exact"])) {
+      for await (const entry of store.list(["exact"])) {
         entries.push(entry);
       }
 
@@ -118,7 +118,7 @@ Deno.test("DenoKvStore", async (t) => {
     await store.set(["d", "e", "f"], "value-def");
 
     const entries: { key: Deno.KvKey; value: unknown }[] = [];
-    for await (const entry of store.list!()) {
+    for await (const entry of store.list()) {
       entries.push(entry);
     }
 

packages/fedify/src/federation/kv.test.ts

@@ -44,15 +44,15 @@ test("MemoryKvStore", async (t) => {
     await store.set(["prefix"], "exact-match");
 
     // Test: list with prefix
-    const entries = await Array.fromAsync(store.list!(["prefix"]));
+    const entries = await Array.fromAsync(store.list(["prefix"]));
     assertEquals(entries.length, 4); // prefix, prefix/a, prefix/b, prefix/nested/c
 
     // Test: verify a value
     const entryA = entries.find((e) => e.key.length === 2 && e.key[1] === "a");
     assertEquals(entryA?.value, "value-a");
 
     // Test: non-matching prefix returns empty
-    const noMatch = await Array.fromAsync(store.list!(["nonexistent"]));
+    const noMatch = await Array.fromAsync(store.list(["nonexistent"]));
     assertEquals(noMatch.length, 0);
 
     // Cleanup
@@ -71,7 +71,7 @@ test("MemoryKvStore", async (t) => {
 
     await new Promise((r) => setTimeout(r, 10));
 
-    const entries = await Array.fromAsync(store.list!(["expired"]));
+    const entries = await Array.fromAsync(store.list(["expired"]));
 
     assertEquals(entries.length, 1);
     assertEquals(entries[0].value, "valid-value");
@@ -89,7 +89,7 @@ test("MemoryKvStore", async (t) => {
     await store.set(["d", "e", "f"], "value-def");
 
     // Test: empty prefix should return all entries
-    const entries = await Array.fromAsync(store.list!());
+    const entries = await Array.fromAsync(store.list());
 
     assertEquals(entries.length, 3);
 

packages/fedify/src/federation/kv.ts

@@ -87,8 +87,9 @@ export interface KvStore {
    *               are returned.
    * @returns An async iterable of entries matching the prefix.
    * @since 1.10.0
+   * @since 2.0.0 This method is now required instead of optional.
    */
-  list?: (prefix?: KvKey) => AsyncIterable<KvStoreListEntry>;
+  list(prefix?: KvKey): AsyncIterable<KvStoreListEntry>;
 }
 
 /**

packages/fedify/src/otel/exporter.test.ts

@@ -1,8 +1,13 @@
-import { assertEquals, assertThrows } from "@std/assert";
+import { assertEquals } from "@std/assert";
 import type { HrTime, SpanContext, SpanStatus } from "@opentelemetry/api";
 import { SpanKind, SpanStatusCode, TraceFlags } from "@opentelemetry/api";
 import type { ReadableSpan, TimedEvent } from "@opentelemetry/sdk-trace-base";
-import { type KvKey, type KvStore, MemoryKvStore } from "../federation/kv.ts";
+import {
+  type KvKey,
+  type KvStore,
+  type KvStoreListEntry,
+  MemoryKvStore,
+} from "../federation/kv.ts";
 import { test } from "../testing/mod.ts";
 import { FedifySpanExporter } from "./exporter.ts";
 
@@ -86,40 +91,12 @@ function createActivitySentEvent(options: {
 }
 
 test("FedifySpanExporter", async (t) => {
-  await t.step(
-    "constructor throws if KvStore has neither list() nor cas()",
-    () => {
-      const kv: KvStore = {
-        get: () => Promise.resolve(undefined),
-        set: () => Promise.resolve(),
-        delete: () => Promise.resolve(),
-      };
-
-      assertThrows(
-        () => new FedifySpanExporter(kv),
-        Error,
-        "KvStore must support either list() or cas()",
-      );
-    },
-  );
-
   await t.step("constructor accepts KvStore with list()", () => {
     const kv = new MemoryKvStore();
     const exporter = new FedifySpanExporter(kv);
     assertEquals(exporter instanceof FedifySpanExporter, true);
   });
 
-  await t.step("constructor accepts KvStore with cas() only", () => {
-    const kv: KvStore = {
-      get: () => Promise.resolve(undefined),
-      set: () => Promise.resolve(),
-      delete: () => Promise.resolve(),
-      cas: () => Promise.resolve(true),
-    };
-    const exporter = new FedifySpanExporter(kv);
-    assertEquals(exporter instanceof FedifySpanExporter, true);
-  });
-
   await t.step("export() stores inbound activity from span event", async () => {
     const kv = new MemoryKvStore();
     const exporter = new FedifySpanExporter(kv);
@@ -369,7 +346,7 @@ test("FedifySpanExporter", async (t) => {
     await exporter.shutdown();
   });
 
-  await t.step("works with cas()-only KvStore", async () => {
+  await t.step("works with custom KvStore implementation", async () => {
     const storedData: Record<string, unknown> = {};
 
     const kv: KvStore = {
@@ -387,14 +364,15 @@ test("FedifySpanExporter", async (t) => {
         delete storedData[k];
         return Promise.resolve();
       },
-      cas: (key: KvKey, expected: unknown, newValue: unknown) => {
-        const k = JSON.stringify(key);
-        const current = storedData[k];
-        if (JSON.stringify(current) === JSON.stringify(expected)) {
-          storedData[k] = newValue;
-          return Promise.resolve(true);
+      async *list(prefix?: KvKey): AsyncIterable<KvStoreListEntry> {
+        for (const [encodedKey, value] of Object.entries(storedData)) {
+          const key = JSON.parse(encodedKey) as KvKey;
+          if (prefix != null) {
+            if (key.length < prefix.length) continue;
+            if (!prefix.every((p, i) => key[i] === p)) continue;
+          }
+          yield { key, value };
         }
-        return Promise.resolve(false);
       },
     };