Negotiate after determining if the resource exists

Fix #34
dahlia · Apr 5, 2024 · 7dcf4b2 · 7dcf4b2
1 parent d62ee37
commit 7dcf4b2
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 8 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -13,6 +13,14 @@ Version 0.5.1
 
 To be released.
 
+ -  Fixed a bug of `Federation` that its actor/collection dispatchers had done
+    content negotiation before determining if the resource exists or not.
+    It also fixed a bug that `integrateHandler()` from `@fedify/fedify/x/fresh`
+    had responded with `406 Not Acceptable` instead of `404 Not Found` when
+    the resource does not exist in the web browser.  [[#34]]
+
+[#34]: https://github.com/dahlia/fedify/issues/34
+
 
 Version 0.5.0
 -------------

diff --git a/federation/handler.test.ts b/federation/handler.test.ts
@@ -100,6 +100,21 @@ Deno.test("handleActor()", async () => {
   assertEquals(onNotAcceptableCalled, context.request);
 
   onNotAcceptableCalled = null;
+  response = await handleActor(
+    context.request,
+    {
+      context,
+      handle: "no-one",
+      actorDispatcher,
+      onNotFound,
+      onNotAcceptable,
+    },
+  );
+  assertEquals(response.status, 404);
+  assertEquals(onNotFoundCalled, context.request);
+  assertEquals(onNotAcceptableCalled, null);
+
+  onNotFoundCalled = null;
   context = createRequestContext<void>({
     ...context,
     request: new Request(context.url, {
@@ -235,6 +250,21 @@ Deno.test("handleCollection()", async () => {
   assertEquals(onNotAcceptableCalled, context.request);
 
   onNotAcceptableCalled = null;
+  response = await handleCollection(
+    context.request,
+    {
+      context,
+      handle: "no-one",
+      collectionCallbacks: { dispatcher },
+      onNotFound,
+      onNotAcceptable,
+    },
+  );
+  assertEquals(response.status, 404);
+  assertEquals(onNotFoundCalled, context.request);
+  assertEquals(onNotAcceptableCalled, null);
+
+  onNotFoundCalled = null;
   context = createRequestContext<void>({
     ...context,
     request: new Request(context.url, {

diff --git a/federation/handler.ts b/federation/handler.ts
@@ -53,16 +53,16 @@ export async function handleActor<TContextData>(
     const response = onNotFound(request);
     return response instanceof Promise ? await response : response;
   }
-  if (!acceptsJsonLd(request)) {
-    const response = onNotAcceptable(request);
-    return response instanceof Promise ? await response : response;
-  }
   const key = await context.getActorKey(handle);
   const actor = await actorDispatcher(context, handle, key);
   if (actor == null) {
     const response = onNotFound(request);
     return response instanceof Promise ? await response : response;
   }
+  if (!acceptsJsonLd(request)) {
+    const response = onNotAcceptable(request);
+    return response instanceof Promise ? await response : response;
+  }
   const jsonLd = await actor.toJsonLd(context);
   return new Response(JSON.stringify(jsonLd), {
     headers: {
@@ -122,10 +122,6 @@ export async function handleCollection<
     const response = onNotFound(request);
     return response instanceof Promise ? await response : response;
   }
-  if (!acceptsJsonLd(request)) {
-    const response = onNotAcceptable(request);
-    return response instanceof Promise ? await response : response;
-  }
   const url = new URL(request.url);
   const cursor = url.searchParams.get("cursor");
   let collection: OrderedCollection | OrderedCollectionPage;
@@ -200,6 +196,10 @@ export async function handleCollection<
     partOf.searchParams.delete("cursor");
     collection = new OrderedCollectionPage({ prev, next, items, partOf });
   }
+  if (!acceptsJsonLd(request)) {
+    const response = onNotAcceptable(request);
+    return response instanceof Promise ? await response : response;
+  }
   const jsonLd = await collection.toJsonLd(context);
   return new Response(JSON.stringify(jsonLd), {
     headers: {