From e48f57a3d85e84f306b6e25688cffc1ba2b351ef Mon Sep 17 00:00:00 2001 From: Jesse Wierzbinski Date: Wed, 8 May 2024 13:19:53 -1000 Subject: [PATCH] perf(database): :zap: Improve performance when fetching timelines by fetching all data in a single SQL query --- database/entities/Notification.ts | 4 +- database/entities/Status.ts | 53 ++++++++ packages/database-interface/note.ts | 121 ++++++++---------- packages/database-interface/timeline.ts | 13 +- server/api/api/v1/accounts/:id/statuses.ts | 2 + server/api/api/v1/favourites/index.ts | 1 + server/api/api/v1/notifications/:id/index.ts | 12 +- server/api/api/v1/notifications/index.ts | 1 + server/api/api/v1/statuses/:id/context.ts | 2 +- server/api/api/v1/statuses/:id/favourite.ts | 2 +- .../api/api/v1/statuses/:id/favourited_by.ts | 2 +- server/api/api/v1/statuses/:id/index.ts | 2 +- server/api/api/v1/statuses/:id/pin.ts | 2 +- server/api/api/v1/statuses/:id/reblog.ts | 4 +- .../api/api/v1/statuses/:id/reblogged_by.ts | 2 +- server/api/api/v1/statuses/:id/source.ts | 2 +- server/api/api/v1/statuses/:id/unfavourite.ts | 2 +- server/api/api/v1/statuses/:id/unpin.ts | 2 +- server/api/api/v1/statuses/:id/unreblog.ts | 6 +- server/api/api/v1/timelines/home.ts | 1 + server/api/api/v1/timelines/public.ts | 1 + server/api/api/v2/search/index.ts | 4 + tests/utils.ts | 3 + utils/timelines.ts | 3 +- 24 files changed, 158 insertions(+), 89 deletions(-) diff --git a/database/entities/Notification.ts b/database/entities/Notification.ts index 9548302c..c9647274 100644 --- a/database/entities/Notification.ts +++ b/database/entities/Notification.ts @@ -21,6 +21,7 @@ export type NotificationWithRelations = Notification & { export const findManyNotifications = async ( query: Parameters[0], + userId?: string, ): Promise => { const output = await db.query.Notifications.findMany({ ...query, @@ -42,7 +43,8 @@ export const findManyNotifications = async ( output.map(async (notif) => ({ ...notif, account: transformOutputToUserWithRelations(notif.account), - status: (await Note.fromId(notif.noteId))?.getStatus() ?? null, + status: + (await Note.fromId(notif.noteId, userId))?.getStatus() ?? null, })), ); }; diff --git a/database/entities/Status.ts b/database/entities/Status.ts index 4bff89cb..b1b9820a 100644 --- a/database/entities/Status.ts +++ b/database/entities/Status.ts @@ -61,6 +61,10 @@ export type StatusWithRelations = Status & { reblogCount: number; likeCount: number; replyCount: number; + pinned: boolean; + reblogged: boolean; + muted: boolean; + liked: boolean; }; export type StatusWithoutRecursiveRelations = Omit< @@ -75,6 +79,7 @@ export type StatusWithoutRecursiveRelations = Omit< */ export const findManyNotes = async ( query: Parameters[0], + userId?: string, ): Promise => { const output = await db.query.Notes.findMany({ ...query, @@ -149,6 +154,26 @@ export const findManyNotes = async ( sql`(SELECT COUNT(*) FROM "Notes" WHERE "Notes"."replyId" = "Notes_reblog".id)`.as( "reply_count", ), + pinned: userId + ? sql`EXISTS (SELECT 1 FROM "UserToPinnedNotes" WHERE "UserToPinnedNotes"."noteId" = "Notes_reblog".id AND "UserToPinnedNotes"."userId" = ${userId})`.as( + "pinned", + ) + : sql`false`.as("pinned"), + reblogged: userId + ? sql`EXISTS (SELECT 1 FROM "Notes" WHERE "Notes"."authorId" = ${userId} AND "Notes"."reblogId" = "Notes_reblog".id)`.as( + "reblogged", + ) + : sql`false`.as("reblogged"), + muted: userId + ? sql`EXISTS (SELECT 1 FROM "Relationships" WHERE "Relationships"."ownerId" = ${userId} AND "Relationships"."subjectId" = "Notes_reblog"."authorId" AND "Relationships"."muting" = true)`.as( + "muted", + ) + : sql`false`.as("muted"), + liked: userId + ? sql`EXISTS (SELECT 1 FROM "Likes" WHERE "Likes"."likedId" = "Notes_reblog".id AND "Likes"."likerId" = ${userId})`.as( + "liked", + ) + : sql`false`.as("liked"), }, }, reply: true, @@ -167,6 +192,26 @@ export const findManyNotes = async ( sql`(SELECT COUNT(*) FROM "Notes" WHERE "Notes"."replyId" = "Notes".id)`.as( "reply_count", ), + pinned: userId + ? sql`EXISTS (SELECT 1 FROM "UserToPinnedNotes" WHERE "UserToPinnedNotes"."noteId" = "Notes".id AND "UserToPinnedNotes"."userId" = ${userId})`.as( + "pinned", + ) + : sql`false`.as("pinned"), + reblogged: userId + ? sql`EXISTS (SELECT 1 FROM "Notes" WHERE "Notes"."authorId" = ${userId} AND "Notes"."reblogId" = "Notes".id)`.as( + "reblogged", + ) + : sql`false`.as("reblogged"), + muted: userId + ? sql`EXISTS (SELECT 1 FROM "Relationships" WHERE "Relationships"."ownerId" = ${userId} AND "Relationships"."subjectId" = "Notes"."authorId" AND "Relationships"."muting" = true)`.as( + "muted", + ) + : sql`false`.as("muted"), + liked: userId + ? sql`EXISTS (SELECT 1 FROM "Likes" WHERE "Likes"."likedId" = "Notes".id AND "Likes"."likerId" = ${userId})`.as( + "liked", + ) + : sql`false`.as("liked"), ...query?.extras, }, }); @@ -190,10 +235,18 @@ export const findManyNotes = async ( reblogCount: Number(post.reblog.reblogCount), likeCount: Number(post.reblog.likeCount), replyCount: Number(post.reblog.replyCount), + pinned: Boolean(post.reblog.pinned), + reblogged: Boolean(post.reblog.reblogged), + muted: Boolean(post.reblog.muted), + liked: Boolean(post.reblog.liked), }, reblogCount: Number(post.reblogCount), likeCount: Number(post.likeCount), replyCount: Number(post.replyCount), + pinned: Boolean(post.pinned), + reblogged: Boolean(post.reblogged), + muted: Boolean(post.muted), + liked: Boolean(post.liked), })); }; diff --git a/packages/database-interface/note.ts b/packages/database-interface/note.ts index 191ec3bf..c6718a52 100644 --- a/packages/database-interface/note.ts +++ b/packages/database-interface/note.ts @@ -54,25 +54,38 @@ import { User } from "./user"; export class Note { private constructor(private status: StatusWithRelations) {} - static async fromId(id: string | null): Promise { + static async fromId( + id: string | null, + userId?: string, + ): Promise { if (!id) return null; - return await Note.fromSql(eq(Notes.id, id)); + return await Note.fromSql(eq(Notes.id, id), undefined, userId); } - static async fromIds(ids: string[]): Promise { - return await Note.manyFromSql(inArray(Notes.id, ids)); + static async fromIds(ids: string[], userId?: string): Promise { + return await Note.manyFromSql( + inArray(Notes.id, ids), + undefined, + undefined, + undefined, + userId, + ); } static async fromSql( sql: SQL | undefined, orderBy: SQL | undefined = desc(Notes.id), + userId?: string, ) { - const found = await findManyNotes({ - where: sql, - orderBy, - limit: 1, - }); + const found = await findManyNotes( + { + where: sql, + orderBy, + limit: 1, + }, + userId, + ); if (!found[0]) return null; return new Note(found[0]); @@ -83,13 +96,17 @@ export class Note { orderBy: SQL | undefined = desc(Notes.id), limit?: number, offset?: number, + userId?: string, ) { - const found = await findManyNotes({ - where: sql, - orderBy, - limit, - offset, - }); + const found = await findManyNotes( + { + where: sql, + orderBy, + limit, + offset, + }, + userId, + ); return found.map((s) => new Note(s)); } @@ -176,8 +193,14 @@ export class Note { )[0].count; } - async getReplyChildren() { - return await Note.manyFromSql(eq(Notes.replyId, this.status.id)); + async getReplyChildren(userId?: string) { + return await Note.manyFromSql( + eq(Notes.replyId, this.status.id), + undefined, + undefined, + undefined, + userId, + ); } static async insert(values: InferInsertModel) { @@ -275,7 +298,7 @@ export class Note { } } - return await Note.fromId(newNote.id); + return await Note.fromId(newNote.id, newNote.authorId); } async updateFromData( @@ -358,7 +381,7 @@ export class Note { .where(inArray(Attachments.id, media_attachments)); } - return await Note.fromId(newNote.id); + return await Note.fromId(newNote.id, newNote.authorId); } async delete() { @@ -414,47 +437,6 @@ export class Note { async toAPI(userFetching?: User | null): Promise { const data = this.getStatus(); - const [pinnedByUser, rebloggedByUser, mutedByUser, likedByUser] = ( - await Promise.all([ - userFetching - ? db.query.UserToPinnedNotes.findFirst({ - where: (relation, { and, eq }) => - and( - eq(relation.noteId, data.id), - eq(relation.userId, userFetching?.id), - ), - }) - : false, - userFetching - ? Note.fromSql( - and( - eq(Notes.authorId, userFetching?.id), - eq(Notes.reblogId, data.id), - ), - ) - : false, - userFetching - ? db.query.Relationships.findFirst({ - where: (relationship, { and, eq }) => - and( - eq(relationship.ownerId, userFetching.id), - eq(relationship.subjectId, data.authorId), - eq(relationship.muting, true), - ), - }) - : false, - userFetching - ? db.query.Likes.findFirst({ - where: (like, { and, eq }) => - and( - eq(like.likedId, data.id), - eq(like.likerId, userFetching.id), - ), - }) - : false, - ]) - ).map((r) => !!r); - // Convert mentions of local users from @username@host to @username const mentionedLocalUsers = data.mentions.filter( (mention) => mention.instanceId === null, @@ -488,7 +470,7 @@ export class Note { card: null, content: replacedContent, emojis: data.emojis.map((emoji) => emojiToAPI(emoji)), - favourited: likedByUser, + favourited: data.liked, favourites_count: data.likeCount, media_attachments: (data.attachments ?? []).map( (a) => attachmentToAPI(a) as APIAttachment, @@ -504,8 +486,8 @@ export class Note { username: mention.username, })), language: null, - muted: mutedByUser, - pinned: pinnedByUser, + muted: data.muted, + pinned: data.pinned, // TODO: Add polls poll: null, reblog: data.reblog @@ -513,7 +495,7 @@ export class Note { data.reblog as StatusWithRelations, ).toAPI(userFetching) : null, - reblogged: rebloggedByUser, + reblogged: data.reblogged, reblogs_count: data.reblogCount, replies_count: data.replyCount, sensitive: data.sensitive, @@ -525,8 +507,8 @@ export class Note { bookmarked: false, // @ts-expect-error Glitch-SOC extension quote: data.quotingId - ? (await Note.fromId(data.quotingId).then((n) => - n?.toAPI(userFetching), + ? (await Note.fromId(data.quotingId, userFetching?.id).then( + (n) => n?.toAPI(userFetching), )) ?? null : null, quote_id: data.quotingId || undefined, @@ -589,7 +571,10 @@ export class Note { let currentStatus: Note = this; while (currentStatus.getStatus().replyId) { - const parent = await Note.fromId(currentStatus.getStatus().replyId); + const parent = await Note.fromId( + currentStatus.getStatus().replyId, + fetcher?.id, + ); if (!parent) { break; @@ -612,7 +597,7 @@ export class Note { */ async getDescendants(fetcher: User | null, depth = 0) { const descendants: Note[] = []; - for (const child of await this.getReplyChildren()) { + for (const child of await this.getReplyChildren(fetcher?.id)) { descendants.push(child); if (depth < 20) { diff --git a/packages/database-interface/timeline.ts b/packages/database-interface/timeline.ts index 76c27e66..e6e1b3a7 100644 --- a/packages/database-interface/timeline.ts +++ b/packages/database-interface/timeline.ts @@ -16,11 +16,13 @@ export class Timeline { sql: SQL | undefined, limit: number, url: string, + userId?: string, ) { return new Timeline(TimelineType.NOTE).fetchTimeline( sql, limit, url, + userId, ); } @@ -40,13 +42,22 @@ export class Timeline { sql: SQL | undefined, limit: number, url: string, + userId?: string, ) { const notes: Note[] = []; const users: User[] = []; switch (this.type) { case TimelineType.NOTE: - notes.push(...(await Note.manyFromSql(sql, undefined, limit))); + notes.push( + ...(await Note.manyFromSql( + sql, + undefined, + limit, + undefined, + userId, + )), + ); break; case TimelineType.USER: users.push(...(await User.manyFromSql(sql, undefined, limit))); diff --git a/server/api/api/v1/accounts/:id/statuses.ts b/server/api/api/v1/accounts/:id/statuses.ts index c73f183e..aa8f6fbe 100644 --- a/server/api/api/v1/accounts/:id/statuses.ts +++ b/server/api/api/v1/accounts/:id/statuses.ts @@ -62,6 +62,7 @@ export default (app: Hono) => auth(meta.auth), async (context) => { const { id } = context.req.valid("param"); + const { user } = context.req.valid("header"); const otherUser = await User.fromId(id); @@ -95,6 +96,7 @@ export default (app: Hono) => ), limit, context.req.url, + user?.id, ); return jsonResponse( diff --git a/server/api/api/v1/favourites/index.ts b/server/api/api/v1/favourites/index.ts index 3b804b34..fd2ca0fa 100644 --- a/server/api/api/v1/favourites/index.ts +++ b/server/api/api/v1/favourites/index.ts @@ -52,6 +52,7 @@ export default (app: Hono) => ), limit, context.req.url, + user?.id, ); return jsonResponse( diff --git a/server/api/api/v1/notifications/:id/index.ts b/server/api/api/v1/notifications/:id/index.ts index b64c18ce..b9387e54 100644 --- a/server/api/api/v1/notifications/:id/index.ts +++ b/server/api/api/v1/notifications/:id/index.ts @@ -37,10 +37,14 @@ export default (app: Hono) => if (!user) return errorResponse("Unauthorized", 401); const notification = ( - await findManyNotifications({ - where: (notification, { eq }) => eq(notification.id, id), - limit: 1, - }) + await findManyNotifications( + { + where: (notification, { eq }) => + eq(notification.id, id), + limit: 1, + }, + user.id, + ) )[0]; if (!notification) diff --git a/server/api/api/v1/notifications/index.ts b/server/api/api/v1/notifications/index.ts index bb0db8e7..0d44511b 100644 --- a/server/api/api/v1/notifications/index.ts +++ b/server/api/api/v1/notifications/index.ts @@ -174,6 +174,7 @@ export default (app: Hono) => desc(notification.id), }, context.req.raw, + user.id, ); return jsonResponse( diff --git a/server/api/api/v1/statuses/:id/context.ts b/server/api/api/v1/statuses/:id/context.ts index 6bd8c355..76189905 100644 --- a/server/api/api/v1/statuses/:id/context.ts +++ b/server/api/api/v1/statuses/:id/context.ts @@ -34,7 +34,7 @@ export default (app: Hono) => const { user } = context.req.valid("header"); - const foundStatus = await Note.fromId(id); + const foundStatus = await Note.fromId(id, user?.id); if (!foundStatus) return errorResponse("Record not found", 404); diff --git a/server/api/api/v1/statuses/:id/favourite.ts b/server/api/api/v1/statuses/:id/favourite.ts index 19d9257d..1dc46247 100644 --- a/server/api/api/v1/statuses/:id/favourite.ts +++ b/server/api/api/v1/statuses/:id/favourite.ts @@ -39,7 +39,7 @@ export default (app: Hono) => if (!user) return errorResponse("Unauthorized", 401); - const note = await Note.fromId(id); + const note = await Note.fromId(id, user?.id); if (!note?.isViewableByUser(user)) return errorResponse("Record not found", 404); diff --git a/server/api/api/v1/statuses/:id/favourited_by.ts b/server/api/api/v1/statuses/:id/favourited_by.ts index 8dcc403c..04cd20b1 100644 --- a/server/api/api/v1/statuses/:id/favourited_by.ts +++ b/server/api/api/v1/statuses/:id/favourited_by.ts @@ -48,7 +48,7 @@ export default (app: Hono) => if (!user) return errorResponse("Unauthorized", 401); - const status = await Note.fromId(id); + const status = await Note.fromId(id, user?.id); if (!status?.isViewableByUser(user)) return errorResponse("Record not found", 404); diff --git a/server/api/api/v1/statuses/:id/index.ts b/server/api/api/v1/statuses/:id/index.ts index 3fc8e231..2d3752e6 100644 --- a/server/api/api/v1/statuses/:id/index.ts +++ b/server/api/api/v1/statuses/:id/index.ts @@ -72,7 +72,7 @@ export default (app: Hono) => const { id } = context.req.valid("param"); const { user } = context.req.valid("header"); - const foundStatus = await Note.fromId(id); + const foundStatus = await Note.fromId(id, user?.id); if (!foundStatus?.isViewableByUser(user)) return errorResponse("Record not found", 404); diff --git a/server/api/api/v1/statuses/:id/pin.ts b/server/api/api/v1/statuses/:id/pin.ts index 0c1e2bc0..e6464fc1 100644 --- a/server/api/api/v1/statuses/:id/pin.ts +++ b/server/api/api/v1/statuses/:id/pin.ts @@ -36,7 +36,7 @@ export default (app: Hono) => if (!user) return errorResponse("Unauthorized", 401); - const foundStatus = await Note.fromId(id); + const foundStatus = await Note.fromId(id, user?.id); if (!foundStatus) return errorResponse("Record not found", 404); diff --git a/server/api/api/v1/statuses/:id/reblog.ts b/server/api/api/v1/statuses/:id/reblog.ts index b1025b06..e06f2a8c 100644 --- a/server/api/api/v1/statuses/:id/reblog.ts +++ b/server/api/api/v1/statuses/:id/reblog.ts @@ -43,7 +43,7 @@ export default (app: Hono) => if (!user) return errorResponse("Unauthorized", 401); - const foundStatus = await Note.fromId(id); + const foundStatus = await Note.fromId(id, user.id); if (!foundStatus?.isViewableByUser(user)) return errorResponse("Record not found", 404); @@ -72,7 +72,7 @@ export default (app: Hono) => return errorResponse("Failed to reblog", 500); } - const finalNewReblog = await Note.fromId(newReblog.id); + const finalNewReblog = await Note.fromId(newReblog.id, user?.id); if (!finalNewReblog) { return errorResponse("Failed to reblog", 500); diff --git a/server/api/api/v1/statuses/:id/reblogged_by.ts b/server/api/api/v1/statuses/:id/reblogged_by.ts index 77bbff9a..beba4486 100644 --- a/server/api/api/v1/statuses/:id/reblogged_by.ts +++ b/server/api/api/v1/statuses/:id/reblogged_by.ts @@ -47,7 +47,7 @@ export default (app: Hono) => if (!user) return errorResponse("Unauthorized", 401); - const status = await Note.fromId(id); + const status = await Note.fromId(id, user.id); if (!status?.isViewableByUser(user)) return errorResponse("Record not found", 404); diff --git a/server/api/api/v1/statuses/:id/source.ts b/server/api/api/v1/statuses/:id/source.ts index 7fcff32d..8aa8edba 100644 --- a/server/api/api/v1/statuses/:id/source.ts +++ b/server/api/api/v1/statuses/:id/source.ts @@ -36,7 +36,7 @@ export default (app: Hono) => if (!user) return errorResponse("Unauthorized", 401); - const status = await Note.fromId(id); + const status = await Note.fromId(id, user.id); if (!status?.isViewableByUser(user)) return errorResponse("Record not found", 404); diff --git a/server/api/api/v1/statuses/:id/unfavourite.ts b/server/api/api/v1/statuses/:id/unfavourite.ts index 9f183e16..b1de85a6 100644 --- a/server/api/api/v1/statuses/:id/unfavourite.ts +++ b/server/api/api/v1/statuses/:id/unfavourite.ts @@ -37,7 +37,7 @@ export default (app: Hono) => if (!user) return errorResponse("Unauthorized", 401); - const note = await Note.fromId(id); + const note = await Note.fromId(id, user.id); if (!note?.isViewableByUser(user)) return errorResponse("Record not found", 404); diff --git a/server/api/api/v1/statuses/:id/unpin.ts b/server/api/api/v1/statuses/:id/unpin.ts index 4188953b..1fd9fc40 100644 --- a/server/api/api/v1/statuses/:id/unpin.ts +++ b/server/api/api/v1/statuses/:id/unpin.ts @@ -35,7 +35,7 @@ export default (app: Hono) => if (!user) return errorResponse("Unauthorized", 401); - const status = await Note.fromId(id); + const status = await Note.fromId(id, user.id); if (!status) return errorResponse("Record not found", 404); diff --git a/server/api/api/v1/statuses/:id/unreblog.ts b/server/api/api/v1/statuses/:id/unreblog.ts index efa7ece2..f125bc3f 100644 --- a/server/api/api/v1/statuses/:id/unreblog.ts +++ b/server/api/api/v1/statuses/:id/unreblog.ts @@ -38,9 +38,7 @@ export default (app: Hono) => if (!user) return errorResponse("Unauthorized", 401); - if (!user) return errorResponse("Unauthorized", 401); - - const foundStatus = await Note.fromId(id); + const foundStatus = await Note.fromId(id, user.id); // Check if user is authorized to view this status (if it's private) if (!foundStatus?.isViewableByUser(user)) @@ -51,6 +49,8 @@ export default (app: Hono) => eq(Notes.authorId, user.id), eq(Notes.reblogId, foundStatus.getStatus().id), ), + undefined, + user?.id, ); if (!existingReblog) { diff --git a/server/api/api/v1/timelines/home.ts b/server/api/api/v1/timelines/home.ts index 7a25a2ac..3594b50a 100644 --- a/server/api/api/v1/timelines/home.ts +++ b/server/api/api/v1/timelines/home.ts @@ -58,6 +58,7 @@ export default (app: Hono) => ), limit, context.req.url, + user.id, ); return jsonResponse( diff --git a/server/api/api/v1/timelines/public.ts b/server/api/api/v1/timelines/public.ts index febe2024..35660fc6 100644 --- a/server/api/api/v1/timelines/public.ts +++ b/server/api/api/v1/timelines/public.ts @@ -79,6 +79,7 @@ export default (app: Hono) => ), limit, context.req.url, + user?.id, ); return jsonResponse( diff --git a/server/api/api/v2/search/index.ts b/server/api/api/v2/search/index.ts index 58c97612..82d3c8ab 100644 --- a/server/api/api/v2/search/index.ts +++ b/server/api/api/v2/search/index.ts @@ -186,6 +186,10 @@ export default (app: Hono) => })` : undefined, ), + undefined, + undefined, + undefined, + self?.id, ); return jsonResponse({ diff --git a/tests/utils.ts b/tests/utils.ts index 98f0227f..81d91cf0 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -106,6 +106,9 @@ export const getTestStatuses = async ( statuses.map((s) => s.id), ), asc(Notes.id), + undefined, + undefined, + user.id, ) ).map((n) => n.getStatus()); }; diff --git a/utils/timelines.ts b/utils/timelines.ts index c80c6a43..0fa143c0 100644 --- a/utils/timelines.ts +++ b/utils/timelines.ts @@ -17,11 +17,12 @@ export async function fetchTimeline( | Parameters[0] | Parameters[0], req: Request, + userId?: string, ) { // BEFORE: Before in a top-to-bottom order, so the most recent posts // AFTER: After in a top-to-bottom order, so the oldest posts // @ts-expect-error This is a hack to get around the fact that Prisma doesn't have a common base type for all models - const objects = (await model(args)) as T[]; + const objects = (await model(args, userId)) as T[]; // Constuct HTTP Link header (next and prev) only if there are more statuses const linkHeader = [];