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), )!,