From 53e0de3c66e1f3d2e2ee060c4ae9bf8343ca995e Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Thu, 1 Oct 2026 16:07:53 +0900 Subject: [PATCH] Avoid full post scans in timeline share filters Use correlated NOT EXISTS checks for shared-post authors instead of collecting every muted or blocked author's post ID. Reuse these checks across home, public, and list timelines while preserving expired mute and bidirectional block behavior. Add regression tests for ordinary posts and shares, both cursor directions, and both timeline inbox modes. Document the fix in the changelog. Fixes https://github.com/fedify-dev/hollo/issues/633 Assisted-by: Codex:gpt-6.1-sol Assisted-by: Claude Code:claude-opus-5-5 --- CHANGES.md | 7 + src/api/v1/timelines.test.ts | 144 ++++++++++++++++++- src/api/v1/timelines.ts | 264 ++++++++--------------------------- 3 files changed, 211 insertions(+), 204 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 9c7dd232..d37bf456 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,13 @@ Version 0.9.21 To be released. + - Fixed slow home, public, and list timeline queries on large databases by + checking the shared post directly when filtering muted or blocked authors. + [[#633], [#634]] + +[#633]: https://github.com/fedify-dev/hollo/issues/633 +[#634]: https://github.com/fedify-dev/hollo/pull/634 + Version 0.9.20 -------------- diff --git a/src/api/v1/timelines.test.ts b/src/api/v1/timelines.test.ts index d8c87ebd..18559337 100644 --- a/src/api/v1/timelines.test.ts +++ b/src/api/v1/timelines.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { cleanDatabase } from "../../../tests/helpers"; import { @@ -11,6 +11,7 @@ import db from "../../db"; import app from "../../index"; import { accounts, + blocks, follows, instances, listMembers, @@ -26,6 +27,21 @@ import type { Uuid } from "../../uuid"; import { uuidv7 } from "../../uuid"; import { timelineQuerySchema } from "./timelines"; +const timelineInboxMode = vi.hoisted(() => ({ + override: undefined as boolean | undefined, +})); + +vi.mock("../../federation/timeline", async (importOriginal) => { + const original = + await importOriginal(); + return { + ...original, + get TIMELINE_INBOXES() { + return timelineInboxMode.override ?? original.TIMELINE_INBOXES; + }, + }; +}); + describe("timelineQuerySchema", () => { it("caps large limits and keeps the lower bound at one", () => { expect.assertions(3); @@ -861,3 +877,129 @@ describe.sequential("/api/v1/timelines/public (pagination)", () => { expect(matches).toHaveLength(2); }); }); + +describe.each([true, false])("shared post filters (inboxes=%s)", (inboxes) => { + describe.each(["home", "public", "list"] as const)( + "/api/v1/timelines/%s (shared post filters)", + (timeline) => { + beforeEach(async () => { + timelineInboxMode.override = inboxes; + await cleanDatabase(); + }); + + afterEach(() => { + timelineInboxMode.override = undefined; + }); + + it("preserves ordinary posts and only shares of unfiltered authors", async () => { + const owner = await createAccount({ username: "share-owner" }); + const client = await createOAuthApplication({ scopes: ["read"] }); + const accessToken = await getAccessToken(client, owner, ["read"]); + const listId = uuidv7(); + await db.insert(lists).values({ + id: listId, + accountOwnerId: owner.id, + title: "Shared posts", + repliesPolicy: "list", + exclusive: false, + }); + await db.insert(listMembers).values({ listId, accountId: owner.id }); + + const lowerBound = uuidv7(); + const visibleIds: Uuid[] = []; + const candidateIds: Uuid[] = []; + const ordinaryPostId = uuidv7(); + await db.insert(posts).values({ + id: ordinaryPostId, + iri: `https://hollo.test/posts/${ordinaryPostId}`, + type: "Note", + accountId: owner.id, + visibility: "public", + published: new Date(), + }); + visibleIds.push(ordinaryPostId); + candidateIds.push(ordinaryPostId); + + for (const filter of [ + "none", + "indefinite-mute", + "active-mute", + "expired-mute", + "blocked", + "blocked-by", + ] as const) { + const author = await createAccount({ username: `share-${filter}` }); + if (filter.endsWith("mute")) { + await db.insert(mutes).values({ + id: uuidv7(), + accountId: owner.id, + mutedAccountId: author.id, + duration: filter === "indefinite-mute" ? null : "1 hour", + created: new Date( + Date.now() - (filter === "expired-mute" ? 7200000 : 60000), + ), + }); + } else if (filter === "blocked" || filter === "blocked-by") { + await db.insert(blocks).values({ + accountId: filter === "blocked" ? owner.id : author.id, + blockedAccountId: filter === "blocked" ? author.id : owner.id, + }); + } + const originalId = uuidv7(); + const shareId = uuidv7(); + await db.insert(posts).values([ + { + id: originalId, + iri: `https://hollo.test/posts/${originalId}`, + type: "Note", + accountId: author.id, + visibility: "unlisted", + published: new Date(), + }, + { + id: shareId, + iri: `https://hollo.test/posts/${shareId}`, + type: "Note", + accountId: owner.id, + sharingId: originalId, + visibility: "public", + published: new Date(), + }, + ]); + candidateIds.push(shareId); + if (filter === "none" || filter === "expired-mute") { + visibleIds.push(shareId); + } + } + const upperBound = uuidv7(); + await db + .insert(timelinePosts) + .values( + candidateIds.map((postId) => ({ accountId: owner.id, postId })), + ); + await db + .insert(listPosts) + .values(candidateIds.map((postId) => ({ listId, postId }))); + + const path = timeline === "list" ? `list/${listId}` : timeline; + for (const query of [ + "", + `?min_id=${lowerBound}`, + `?max_id=${upperBound}`, + ]) { + const response = await app.request( + `/api/v1/timelines/${path}${query}`, + { + headers: { authorization: bearerAuthorization(accessToken) }, + }, + ); + expect(response.status).toBe(200); + const json: { id: string }[] = await response.json(); + expect(json.map((post) => post.id)).toEqual( + [...visibleIds].reverse(), + ); + } + }); + }, + ); +}); diff --git a/src/api/v1/timelines.ts b/src/api/v1/timelines.ts index 903be8ca..f593e7f5 100644 --- a/src/api/v1/timelines.ts +++ b/src/api/v1/timelines.ts @@ -9,10 +9,12 @@ import { lt, lte, notInArray, + notExists, or, sql, type SQL, } from "drizzle-orm"; +import { alias } from "drizzle-orm/pg-core"; import { Hono } from "hono"; import { z } from "zod"; @@ -161,6 +163,61 @@ async function readTimelineSnapshot( }); } +function getSharedPostFilterConditions( + ownerId: Uuid, + outerPosts: typeof posts, +): (SQL | undefined)[] { + const sharedPosts = alias(posts, "shared_posts"); + return [ + // Hide the shared posts from the muted accounts: + notExists( + db + .select({ id: sharedPosts.id }) + .from(sharedPosts) + .innerJoin(mutes, eq(mutes.mutedAccountId, sharedPosts.accountId)) + .where( + and( + eq(sharedPosts.id, outerPosts.sharingId), + eq(mutes.accountId, ownerId), + or( + isNull(mutes.duration), + gt( + sql`${mutes.created} + ${mutes.duration}`, + sql`CURRENT_TIMESTAMP`, + ), + ), + ), + ), + ), + // Hide the shared posts from the blocked accounts: + notExists( + db + .select({ id: sharedPosts.id }) + .from(sharedPosts) + .innerJoin(blocks, eq(blocks.blockedAccountId, sharedPosts.accountId)) + .where( + and( + eq(sharedPosts.id, outerPosts.sharingId), + eq(blocks.accountId, ownerId), + ), + ), + ), + // Hide the shared posts from the accounts who blocked the owner: + notExists( + db + .select({ id: sharedPosts.id }) + .from(sharedPosts) + .innerJoin(blocks, eq(blocks.accountId, sharedPosts.accountId)) + .where( + and( + eq(sharedPosts.id, outerPosts.sharingId), + eq(blocks.blockedAccountId, ownerId), + ), + ), + ), + ]; +} + function getTimelinePostFilterConditions(ownerId: Uuid): (SQL | undefined)[] { return [ // Hide future posts @@ -200,53 +257,7 @@ function getTimelinePostFilterConditions(ownerId: Uuid): (SQL | undefined)[] { .from(blocks) .where(eq(blocks.blockedAccountId, ownerId)), ), - // Hide the shared posts from the muted accounts: - or( - isNull(posts.sharingId), - notInArray( - posts.sharingId, - db - .select({ id: posts.id }) - .from(posts) - .innerJoin(mutes, eq(mutes.mutedAccountId, posts.accountId)) - .where( - and( - eq(mutes.accountId, ownerId), - or( - isNull(mutes.duration), - gt( - sql`${mutes.created} + ${mutes.duration}`, - sql`CURRENT_TIMESTAMP`, - ), - ), - ), - ), - ), - ), - // Hide the shared posts from the blocked accounts: - or( - isNull(posts.sharingId), - notInArray( - posts.sharingId, - db - .select({ id: posts.id }) - .from(posts) - .innerJoin(blocks, eq(blocks.blockedAccountId, posts.accountId)) - .where(eq(blocks.accountId, ownerId)), - ), - ), - // Hide the shared posts from the accounts who blocked the owner: - or( - isNull(posts.sharingId), - notInArray( - posts.sharingId, - db - .select({ id: posts.id }) - .from(posts) - .innerJoin(blocks, eq(blocks.accountId, posts.accountId)) - .where(eq(blocks.blockedAccountId, ownerId)), - ), - ), + ...getSharedPostFilterConditions(ownerId, posts), ]; } @@ -318,56 +329,7 @@ app.get( .from(blocks) .where(eq(blocks.blockedAccountId, owner.id)), ), - // Hide the shared posts from the muted accounts: - or( - isNull(posts.sharingId), - notInArray( - posts.sharingId, - db - .select({ id: posts.id }) - .from(posts) - .innerJoin(mutes, eq(mutes.mutedAccountId, posts.accountId)) - .where( - and( - eq(mutes.accountId, owner.id), - or( - isNull(mutes.duration), - gt( - sql`${mutes.created} + ${mutes.duration}`, - sql`CURRENT_TIMESTAMP`, - ), - ), - ), - ), - ), - ), - // Hide the shared posts from the blocked accounts: - or( - isNull(posts.sharingId), - notInArray( - posts.sharingId, - db - .select({ id: posts.id }) - .from(posts) - .innerJoin( - blocks, - eq(blocks.blockedAccountId, posts.accountId), - ) - .where(eq(blocks.accountId, owner.id)), - ), - ), - // Hide the shared posts from the accounts who blocked the owner: - or( - isNull(posts.sharingId), - notInArray( - posts.sharingId, - db - .select({ id: posts.id }) - .from(posts) - .innerJoin(blocks, eq(blocks.accountId, posts.accountId)) - .where(eq(blocks.blockedAccountId, owner.id)), - ), - ), + ...getSharedPostFilterConditions(owner.id, posts), query.max_id == null ? undefined : lt(posts.id, query.max_id), lowerBound == null ? undefined : gt(posts.id, lowerBound), )!, @@ -540,59 +502,7 @@ app.get( .from(blocks) .where(eq(blocks.blockedAccountId, owner.id)), ), - // Hide the shared posts from the muted accounts: - or( - isNull(posts.sharingId), - notInArray( - posts.sharingId, - db - .select({ id: posts.id }) - .from(posts) - .innerJoin( - mutes, - eq(mutes.mutedAccountId, posts.accountId), - ) - .where( - and( - eq(mutes.accountId, owner.id), - or( - isNull(mutes.duration), - gt( - sql`${mutes.created} + ${mutes.duration}`, - sql`CURRENT_TIMESTAMP`, - ), - ), - ), - ), - ), - ), - // Hide the shared posts from the blocked accounts: - or( - isNull(posts.sharingId), - notInArray( - posts.sharingId, - db - .select({ id: posts.id }) - .from(posts) - .innerJoin( - blocks, - eq(blocks.blockedAccountId, posts.accountId), - ) - .where(eq(blocks.accountId, owner.id)), - ), - ), - // Hide the shared posts from the accounts who blocked the owner: - or( - isNull(posts.sharingId), - notInArray( - posts.sharingId, - db - .select({ id: posts.id }) - .from(posts) - .innerJoin(blocks, eq(blocks.accountId, posts.accountId)) - .where(eq(blocks.blockedAccountId, owner.id)), - ), - ), + ...getSharedPostFilterConditions(owner.id, posts), query.max_id == null ? undefined : lt(posts.id, query.max_id), lowerBound == null ? undefined : gt(posts.id, lowerBound), )!, @@ -755,59 +665,7 @@ app.get( .from(blocks) .where(eq(blocks.blockedAccountId, owner.id)), ), - // Hide the shared posts from the muted accounts: - or( - isNull(posts.sharingId), - notInArray( - posts.sharingId, - db - .select({ id: posts.id }) - .from(posts) - .innerJoin( - mutes, - eq(mutes.mutedAccountId, posts.accountId), - ) - .where( - and( - eq(mutes.accountId, owner.id), - or( - isNull(mutes.duration), - gt( - sql`${mutes.created} + ${mutes.duration}`, - sql`CURRENT_TIMESTAMP`, - ), - ), - ), - ), - ), - ), - // Hide the shared posts from the blocked accounts: - or( - isNull(posts.sharingId), - notInArray( - posts.sharingId, - db - .select({ id: posts.id }) - .from(posts) - .innerJoin( - blocks, - eq(blocks.blockedAccountId, posts.accountId), - ) - .where(eq(blocks.accountId, owner.id)), - ), - ), - // Hide the shared posts from the accounts who blocked the owner: - or( - isNull(posts.sharingId), - notInArray( - posts.sharingId, - db - .select({ id: posts.id }) - .from(posts) - .innerJoin(blocks, eq(blocks.accountId, posts.accountId)) - .where(eq(blocks.blockedAccountId, owner.id)), - ), - ), + ...getSharedPostFilterConditions(owner.id, posts), query.max_id == null ? undefined : lt(posts.id, query.max_id), lowerBound == null ? undefined : gt(posts.id, lowerBound), )!,