Fix FederationBuilder.build() multiple calls error

Implement proper cloning of mutable state to prevent errors when
build() is called multiple times on the same builder instance.

Author: dahlia

CHANGES.md

@@ -27,6 +27,8 @@ To be released.  Note that 1.6.0 was skipped due to a mistake in the versioning.
 
  -  Added `Router.trailingSlashInsensitive` property.
 
+ -  Added `Router.clone()` method.
+
  -  Implemented HTTP Message Signatures ([RFC 9421]) with double-knocking.
     Currently, it only works with RSA-PKCS#1-v1.5.  [[#208]]
 

fedify/federation/builder.test.ts

@@ -102,6 +102,8 @@ test("FederationBuilder", async (t) => {
         impl.router.build("inbox", { identifier: "user1" }),
         "/users/user1/inbox",
       );
+
+      await builder.build({ kv }); // Ensure build can be called multiple times
     },
   );
 

fedify/federation/builder.ts

@@ -104,26 +104,46 @@ export class FederationBuilderImpl<TContextData>
     const { FederationImpl } = await import("./middleware.ts");
     const f = new FederationImpl(options);
 
+    // In order to ensure `build()` can be called multiple times and
+    // each instance does not share their state, we clone everything
+    // that is mutable.  This includes the router and callbacks.
+
     // Assign the existing router instance but preserve the settings
     // Keep the original trailingSlashInsensitive configuration
     const trailingSlashInsensitiveValue = f.router.trailingSlashInsensitive;
-    f.router = this.router;
+    f.router = this.router.clone();
     f.router.trailingSlashInsensitive = trailingSlashInsensitiveValue;
     f._initializeRouter();
 
-    f.actorCallbacks = this.actorCallbacks;
+    f.actorCallbacks = this.actorCallbacks == null
+      ? undefined
+      : { ...this.actorCallbacks };
     f.nodeInfoDispatcher = this.nodeInfoDispatcher;
-    f.objectCallbacks = this.objectCallbacks;
-    f.objectTypeIds = this.objectTypeIds;
+    f.objectCallbacks = { ...this.objectCallbacks };
+    f.objectTypeIds = { ...this.objectTypeIds };
     f.inboxPath = this.inboxPath;
-    f.inboxCallbacks = this.inboxCallbacks;
-    f.outboxCallbacks = this.outboxCallbacks;
-    f.followingCallbacks = this.followingCallbacks;
-    f.followersCallbacks = this.followersCallbacks;
-    f.likedCallbacks = this.likedCallbacks;
-    f.featuredCallbacks = this.featuredCallbacks;
-    f.featuredTagsCallbacks = this.featuredTagsCallbacks;
-    f.inboxListeners = this.inboxListeners;
+    f.inboxCallbacks = this.inboxCallbacks == null
+      ? undefined
+      : { ...this.inboxCallbacks };
+    f.outboxCallbacks = this.outboxCallbacks == null
+      ? undefined
+      : { ...this.outboxCallbacks };
+    f.followingCallbacks = this.followingCallbacks == null
+      ? undefined
+      : { ...this.followingCallbacks };
+    f.followersCallbacks = this.followersCallbacks == null
+      ? undefined
+      : { ...this.followersCallbacks };
+    f.likedCallbacks = this.likedCallbacks == null
+      ? undefined
+      : { ...this.likedCallbacks };
+    f.featuredCallbacks = this.featuredCallbacks == null
+      ? undefined
+      : { ...this.featuredCallbacks };
+    f.featuredTagsCallbacks = this.featuredTagsCallbacks == null
+      ? undefined
+      : { ...this.featuredTagsCallbacks };
+    f.inboxListeners = this.inboxListeners?.clone();
     f.inboxErrorHandler = this.inboxErrorHandler;
     f.sharedInboxKeyDispatcher = this.sharedInboxKeyDispatcher;
     return f;

fedify/federation/inbox.ts

@@ -27,6 +27,12 @@ export class InboxListenerSet<TContextData> {
     this.#listeners = new Map();
   }
 
+  clone(): InboxListenerSet<TContextData> {
+    const clone = new InboxListenerSet<TContextData>();
+    clone.#listeners = new Map(this.#listeners);
+    return clone;
+  }
+
   add<TActivity extends Activity>(
     // deno-lint-ignore no-explicit-any
     type: new (...args: any[]) => TActivity,

fedify/federation/router.test.ts

@@ -1,4 +1,4 @@
-import { assertEquals, assertThrows } from "@std/assert";
+import { assert, assertEquals, assertFalse, assertThrows } from "@std/assert";
 import { test } from "../testing/mod.ts";
 import { Router, RouterError, type RouterOptions } from "./router.ts";
 
@@ -13,6 +13,21 @@ function setUp(options: RouterOptions = {}): Router {
   return router;
 }
 
+test("Router.clone()", () => {
+  const original = setUp();
+  const clone = original.clone();
+  clone.add("/users/{name}/friends", "friends");
+
+  assert(clone.has("friends"));
+  assertEquals(clone.route("/users/alice/friends"), {
+    name: "friends",
+    template: "/users/{name}/friends",
+    values: { name: "alice" },
+  });
+  assertFalse(original.has("friends"));
+  assertEquals(original.route("/users/alice/friends"), null);
+});
+
 test("Router.add()", () => {
   const router = new Router();
   assertEquals(router.add("/users", "users"), new Set());

fedify/federation/router.ts

@@ -1,4 +1,5 @@
 // @ts-ignore TS7016
+import { cloneDeep } from "@es-toolkit/es-toolkit";
 import { Router as InnerRouter } from "uri-template-router";
 import { parseTemplate, type Template } from "url-template";
 
@@ -34,6 +35,17 @@ export interface RouterRouteResult {
   values: Record<string, string>;
 }
 
+function cloneInnerRouter(router: InnerRouter): InnerRouter {
+  const clone = new InnerRouter();
+  clone.nid = router.nid;
+  clone.fsm = cloneDeep(router.fsm);
+  clone.routeSet = new Set(router.routeSet);
+  clone.templateRouteMap = new Map(router.templateRouteMap);
+  clone.valueRouteMap = new Map(router.valueRouteMap);
+  clone.hierarchy = cloneDeep(router.hierarchy);
+  return clone;
+}
+
 /**
  * URL router and constructor based on URI Template
  * ([RFC 6570](https://tools.ietf.org/html/rfc6570)).
@@ -60,6 +72,16 @@ export class Router {
     this.trailingSlashInsensitive = options.trailingSlashInsensitive ?? false;
   }
 
+  clone(): Router {
+    const clone = new Router({
+      trailingSlashInsensitive: this.trailingSlashInsensitive,
+    });
+    clone.#router = cloneInnerRouter(this.#router);
+    clone.#templates = { ...this.#templates };
+    clone.#templateStrings = { ...this.#templateStrings };
+    return clone;
+  }
+
   /**
    * Checks if a path name exists in the router.
    * @param name The name of the path.