Merge pull request #467 from dahlia/token-required-perf

Optimize `tokenRequired` middleware performance

Author: dahlia

CHANGES.md

@@ -94,6 +94,12 @@ To be released.
         are removed; UnoCSS emits a single _src/public/uno.css_ whose
         URL is cache-busted by file mtime.
 
+ -  Improved the performance of authenticated API requests by replacing the
+    complex multi-table JOIN query in the `tokenRequired` middleware with a
+    lightweight single-table lookup.  Account owner data is now fetched on
+    demand only by routes that actually need it, and requests that fail scope
+    validation no longer touch the accounts table at all.  [[#127], [#467]]
+
  -  Fixed a bug where incoming ActivityPub posts with timestamps more than
     12 hours in the future were accepted and stuck to the top of the
     federated timeline, enabling timeline manipulation via forged timestamps.
@@ -107,11 +113,13 @@ To be released.
 
 [FEP-044f]: https://w3id.org/fep/044f
 [#67]: https://github.com/fedify-dev/hollo/issues/67
+[#127]: https://github.com/fedify-dev/hollo/issues/127
 [#457]: https://github.com/fedify-dev/hollo/pull/457
 [#458]: https://github.com/fedify-dev/hollo/pull/458
 [#459]: https://github.com/fedify-dev/hollo/pull/459
 [#460]: https://github.com/fedify-dev/hollo/pull/460
 [#466]: https://github.com/fedify-dev/hollo/pull/466
+[#467]: https://github.com/fedify-dev/hollo/pull/467
 
 
 Version 0.8.1

src/api/v1/accounts.ts

@@ -42,7 +42,8 @@ import {
 import {
   scopeRequired,
   tokenRequired,
-  type Variables,
+  withAccountOwner,
+  type AccountOwnerVariables,
 } from "../../oauth/middleware";
 import {
   type Account,
@@ -63,21 +64,16 @@ import {
 import { isUuid, type Uuid } from "../../uuid";
 import { timelineQuerySchema } from "./timelines";
 
-const app = new Hono<{ Variables: Variables }>();
+const app = new Hono<{ Variables: AccountOwnerVariables }>();
 const allowedImageMimeTypes = ["image/gif", "image/jpeg", "image/png"];
 
 app.get(
   "/verify_credentials",
   tokenRequired,
   scopeRequired(["read:accounts", "profile"]),
+  withAccountOwner,
   async (c) => {
-    const accountOwner = c.get("token").accountOwner;
-    if (accountOwner == null) {
-      return c.json(
-        { error: "This method requires an authenticated user" },
-        422,
-      );
-    }
+    const accountOwner = c.get("accountOwner");
     return c.json(serializeAccountOwner(accountOwner, c.req.url));
   },
 );
@@ -86,6 +82,7 @@ app.patch(
   "/update_credentials",
   tokenRequired,
   scopeRequired(["write:accounts"]),
+  withAccountOwner,
   zValidator(
     "form",
     z.object({
@@ -117,13 +114,7 @@ app.patch(
       import("../../text"),
     ]);
     const disk = drive.use();
-    const owner = c.get("token").accountOwner;
-    if (owner == null) {
-      return c.json(
-        { error: "This method requires an authenticated user" },
-        422,
-      );
-    }
+    const owner = c.get("accountOwner");
     const account = owner.account;
     const form = c.req.valid("form");
     let avatarUrl: string | undefined;
@@ -266,14 +257,9 @@ app.get(
   "/relationships",
   tokenRequired,
   scopeRequired(["read:follows"]),
+  withAccountOwner,
   async (c) => {
-    const owner = c.get("token").accountOwner;
-    if (owner == null) {
-      return c.json(
-        { error: "This method requires an authenticated user" },
-        422,
-      );
-    }
+    const owner = c.get("accountOwner");
     const ids = (c.req.queries("id[]") ?? []).filter(isUuid);
     const accountList =
       ids.length > 0
@@ -315,7 +301,6 @@ app.get(
     }),
   ),
   async (c) => {
-    const owner = c.get("token")?.accountOwner;
     const query = c.req.valid("query");
     const acct = query.acct;
     let account:
@@ -338,15 +323,7 @@ app.get(
         return c.json({ error: "Record not found" }, 404);
       }
       const fedCtx = federation.createContext(c.req.raw, undefined);
-      const options =
-        owner == null
-          ? fedCtx
-          : {
-              contextLoader: fedCtx.contextLoader,
-              documentLoader: await fedCtx.getDocumentLoader({
-                username: owner.handle,
-              }),
-            };
+      const options = fedCtx;
       const actor = await lookupObject(acct, options);
       if (!isActor(actor)) return c.json({ error: "Record not found" }, 404);
       const loaded = await persistAccount(db, actor, c.req.url, options);
@@ -448,14 +425,9 @@ app.get(
   "/familiar_followers",
   tokenRequired,
   scopeRequired(["read:follows"]),
+  withAccountOwner,
   async (c) => {
-    const owner = c.get("token").accountOwner;
-    if (owner == null) {
-      return c.json(
-        { error: "This method requires an authenticated user" },
-        422,
-      );
-    }
+    const owner = c.get("accountOwner");
     const ids: Uuid[] = (c.req.queries("id[]") ?? []).filter(isUuid);
     const result: {
       id: string;
@@ -514,6 +486,7 @@ app.get(
   "/:id/statuses",
   tokenRequired,
   scopeRequired(["read:statuses"]),
+  withAccountOwner,
   zValidator(
     "query",
     timelineQuerySchema.extend({
@@ -527,13 +500,7 @@ app.get(
   async (c) => {
     const id = c.req.param("id");
     if (!isUuid(id)) return c.json({ error: "Record not found" }, 404);
-    const tokenOwner = c.get("token").accountOwner;
-    if (tokenOwner == null) {
-      return c.json(
-        { error: "This method requires an authenticated user" },
-        422,
-      );
-    }
+    const tokenOwner = c.get("accountOwner");
     const account = await db.query.accounts.findFirst({
       where: eq(accounts.id, id),
       with: {
@@ -701,16 +668,11 @@ app.post(
   "/:id/follow",
   tokenRequired,
   scopeRequired(["write:follows"]),
+  withAccountOwner,
   async (c) => {
     const id = c.req.param("id");
     if (!isUuid(id)) return c.json({ error: "Record not found" }, 404);
-    const owner = c.get("token").accountOwner;
-    if (owner == null) {
-      return c.json(
-        { error: "This method requires an authenticated user" },
-        422,
-      );
-    }
+    const owner = c.get("accountOwner");
     const following = await db.query.accounts.findFirst({
       where: eq(accounts.id, id),
       with: { owner: true },
@@ -755,16 +717,11 @@ app.post(
   "/:id/unfollow",
   tokenRequired,
   scopeRequired(["write:follows"]),
+  withAccountOwner,
   async (c) => {
     const id = c.req.param("id");
     if (!isUuid(id)) return c.json({ error: "Record not found" }, 404);
-    const owner = c.get("token").accountOwner;
-    if (owner == null) {
-      return c.json(
-        { error: "This method requires an authenticated user" },
-        422,
-      );
-    }
+    const owner = c.get("accountOwner");
     const following = await db.query.accounts.findFirst({
       where: eq(accounts.id, id),
       with: { owner: true },
@@ -841,16 +798,11 @@ app.get(
   "/:id/lists",
   tokenRequired,
   scopeRequired(["read:lists"]),
+  withAccountOwner,
   async (c) => {
     const accountId = c.req.param("id");
     if (!isUuid(accountId)) return c.json({ error: "Record not found" }, 404);
-    const owner = c.get("token").accountOwner;
-    if (owner == null) {
-      return c.json(
-        { error: "This method requires an authenticated user" },
-        422,
-      );
-    }
+    const owner = c.get("accountOwner");
     const listList = await db.query.lists.findMany({
       where: and(
         eq(lists.accountOwnerId, owner.id),
@@ -871,6 +823,7 @@ app.post(
   "/:id/mute",
   tokenRequired,
   scopeRequired(["write:mutes"]),
+  withAccountOwner,
   zValidator(
     "json",
     z.object({
@@ -881,13 +834,7 @@ app.post(
   async (c) => {
     const id = c.req.param("id");
     if (!isUuid(id)) return c.json({ error: "Record not found" }, 404);
-    const owner = c.get("token").accountOwner;
-    if (owner == null) {
-      return c.json(
-        { error: "This method requires an authenticated user" },
-        422,
-      );
-    }
+    const owner = c.get("accountOwner");
     const { notifications, duration } = c.req.valid("json");
     const account = await db.query.accounts.findFirst({
       where: eq(accounts.id, id),
@@ -950,16 +897,11 @@ app.post(
   "/:id/unmute",
   tokenRequired,
   scopeRequired(["write:mutes"]),
+  withAccountOwner,
   async (c) => {
     const id = c.req.param("id");
     if (!isUuid(id)) return c.json({ error: "Record not found" }, 404);
-    const owner = c.get("token").accountOwner;
-    if (owner == null) {
-      return c.json(
-        { error: "This method requires an authenticated user" },
-        422,
-      );
-    }
+    const owner = c.get("accountOwner");
     await db
       .delete(mutes)
       .where(and(eq(mutes.accountId, owner.id), eq(mutes.mutedAccountId, id)));
@@ -992,16 +934,11 @@ app.post(
   "/:id/block",
   tokenRequired,
   scopeRequired(["read:blocks"]),
+  withAccountOwner,
   async (c) => {
     const id = c.req.param("id");
     if (!isUuid(id)) return c.json({ error: "Record not found" }, 404);
-    const owner = c.get("token").accountOwner;
-    if (owner == null) {
-      return c.json(
-        { error: "This method requires an authenticated user" },
-        422,
-      );
-    }
+    const owner = c.get("accountOwner");
     const acct = await db.query.accounts.findFirst({
       where: eq(accounts.id, id),
       with: { owner: true },
@@ -1038,16 +975,11 @@ app.post(
   "/:id/unblock",
   tokenRequired,
   scopeRequired(["read:blocks"]),
+  withAccountOwner,
   async (c) => {
     const id = c.req.param("id");
     if (!isUuid(id)) return c.json({ error: "Record not found" }, 404);
-    const owner = c.get("token").accountOwner;
-    if (owner == null) {
-      return c.json(
-        { error: "This method requires an authenticated user" },
-        422,
-      );
-    }
+    const owner = c.get("accountOwner");
     const acct = await db.query.accounts.findFirst({
       where: eq(accounts.id, id),
       with: { owner: true },

src/api/v1/apps.ts

@@ -1,4 +1,5 @@
 import { getLogger } from "@logtape/logtape";
+import { eq } from "drizzle-orm";
 import { Hono } from "hono";
 import { z } from "zod";
 
@@ -113,14 +114,17 @@ app.post("/", async (c) => {
 
 app.get("/verify_credentials", tokenRequired, async (c) => {
   const token = c.get("token");
-  const app = token.application;
+  const application = await db.query.applications.findFirst({
+    where: eq(applications.id, token.applicationId),
+  });
+  if (application == null) return c.json({ error: "invalid_token" }, 401);
   return c.json({
-    id: app.id,
-    name: app.name,
-    website: app.website,
-    scopes: app.scopes,
-    redirect_uris: app.redirectUris,
-    redirect_uri: app.redirectUris.join(" "),
+    id: application.id,
+    name: application.name,
+    website: application.website,
+    scopes: application.scopes,
+    redirect_uris: application.redirectUris,
+    redirect_uri: application.redirectUris.join(" "),
   });
 });