fix(server): DriveFile related N+1 query when call note packMany (#10133)

* fix(server): DriveFile related N+1 query when call note packMany

* Update packages/backend/src/misc/is-not-null.ts

Co-authored-by: Acid Chicken (硫酸鶏) <root@acid-chicken.com>

* ignore lint

* 途中でやめたやつが混入していた

* fix: 順番通りである必要がある場所で順番通りになっていなかった

---------

Co-authored-by: Acid Chicken (硫酸鶏) <root@acid-chicken.com>

packages/backend/src/core/entities/DriveFileEntityService.ts

@@ -1,5 +1,5 @@
 import { forwardRef, Inject, Injectable } from '@nestjs/common';
-import { DataSource } from 'typeorm';
+import { DataSource, In } from 'typeorm';
 import { DI } from '@/di-symbols.js';
 import type { NotesRepository, DriveFilesRepository } from '@/models/index.js';
 import type { Config } from '@/config.js';
@@ -21,6 +21,7 @@ type PackOptions = {
 };
 import { bindThis } from '@/decorators.js';
 import { isMimeImage } from '@/misc/is-mime-image.js';
+import { isNotNull } from '@/misc/is-not-null.js';
 
 @Injectable()
 export class DriveFileEntityService {
@@ -255,10 +256,29 @@ export class DriveFileEntityService {
 
 	@bindThis
 	public async packMany(
-		files: (DriveFile['id'] | DriveFile)[],
+		files: DriveFile[],
 		options?: PackOptions,
 	): Promise<Packed<'DriveFile'>[]> {
 		const items = await Promise.all(files.map(f => this.packNullable(f, options)));
 		return items.filter((x): x is Packed<'DriveFile'> => x != null);
 	}
+
+	@bindThis
+	public async packManyByIdsMap(
+		fileIds: DriveFile['id'][],
+		options?: PackOptions,
+	): Promise<Map<Packed<'DriveFile'>['id'], Packed<'DriveFile'>>> {
+		const files = await this.driveFilesRepository.findBy({ id: In(fileIds) });
+		const packedFiles = await this.packMany(files, options);
+		return new Map(packedFiles.map(f => [f.id, f]));
+	}
+
+	@bindThis
+	public async packManyByIds(
+		fileIds: DriveFile['id'][],
+		options?: PackOptions,
+	): Promise<Packed<'DriveFile'>[]> {
+		const filesMap = await this.packManyByIdsMap(fileIds, options);
+		return fileIds.map(id => filesMap.get(id)).filter(isNotNull);
+	}
 }

packages/backend/src/core/entities/GalleryPostEntityService.ts

@@ -41,7 +41,8 @@ export class GalleryPostEntityService {
 			title: post.title,
 			description: post.description,
 			fileIds: post.fileIds,
-			files: this.driveFileEntityService.packMany(post.fileIds),
+			// TODO: packMany causes N+1 queries
+			files: this.driveFileEntityService.packManyByIds(post.fileIds),
 			tags: post.tags.length > 0 ? post.tags : undefined,
 			isSensitive: post.isSensitive,
 			likedCount: post.likedCount,

packages/backend/src/core/entities/NoteEntityService.ts

@@ -11,6 +11,7 @@ import type { Note } from '@/models/entities/Note.js';
 import type { NoteReaction } from '@/models/entities/NoteReaction.js';
 import type { UsersRepository, NotesRepository, FollowingsRepository, PollsRepository, PollVotesRepository, NoteReactionsRepository, ChannelsRepository, DriveFilesRepository } from '@/models/index.js';
 import { bindThis } from '@/decorators.js';
+import { isNotNull } from '@/misc/is-not-null.js';
 import type { OnModuleInit } from '@nestjs/common';
 import type { CustomEmojiService } from '../CustomEmojiService.js';
 import type { ReactionService } from '../ReactionService.js';
@@ -257,6 +258,7 @@ export class NoteEntityService implements OnModuleInit {
 			skipHide?: boolean;
 			_hint_?: {
 				myReactions: Map<Note['id'], NoteReaction | null>;
+				packedFiles: Map<Note['fileIds'][number], Packed<'DriveFile'>>;
 			};
 		},
 	): Promise<Packed<'Note'>> {
@@ -284,6 +286,7 @@ export class NoteEntityService implements OnModuleInit {
 		const reactionEmojiNames = Object.keys(note.reactions)
 			.filter(x => x.startsWith(':') && x.includes('@') && !x.includes('@.')) // リモートカスタム絵文字のみ
 			.map(x => this.reactionService.decodeReaction(x).reaction.replaceAll(':', ''));
+		const packedFiles = options?._hint_?.packedFiles;
 
 		const packed: Packed<'Note'> = await awaitAll({
 			id: note.id,
@@ -304,7 +307,7 @@ export class NoteEntityService implements OnModuleInit {
 			emojis: host != null ? this.customEmojiService.populateEmojis(note.emojis, host) : undefined,
 			tags: note.tags.length > 0 ? note.tags : undefined,
 			fileIds: note.fileIds,
-			files: this.driveFileEntityService.packMany(note.fileIds),
+			files: packedFiles != null ? note.fileIds.map(fi => packedFiles.get(fi)).filter(isNotNull) : this.driveFileEntityService.packManyByIds(note.fileIds),
 			replyId: note.replyId,
 			renoteId: note.renoteId,
 			channelId: note.channelId ?? undefined,
@@ -388,11 +391,14 @@ export class NoteEntityService implements OnModuleInit {
 		}
 
 		await this.customEmojiService.prefetchEmojis(this.customEmojiService.aggregateNoteEmojis(notes));
+		const fileIds = notes.flatMap(n => n.fileIds);
+		const packedFiles = await this.driveFileEntityService.packManyByIdsMap(fileIds);
 
 		return await Promise.all(notes.map(n => this.pack(n, me, {
 			...options,
 			_hint_: {
 				myReactions: myReactionsMap,
+				packedFiles,
 			},
 		})));
 	}

packages/backend/src/core/entities/NotificationEntityService.ts

@@ -1,19 +1,21 @@
 import { Inject, Injectable } from '@nestjs/common';
-import { In } from 'typeorm';
 import { ModuleRef } from '@nestjs/core';
 import { DI } from '@/di-symbols.js';
 import type { AccessTokensRepository, NoteReactionsRepository, NotificationsRepository, User } from '@/models/index.js';
 import { awaitAll } from '@/misc/prelude/await-all.js';
 import type { Notification } from '@/models/entities/Notification.js';
-import type { NoteReaction } from '@/models/entities/NoteReaction.js';
 import type { Note } from '@/models/entities/Note.js';
 import type { Packed } from '@/misc/schema.js';
 import { bindThis } from '@/decorators.js';
+import { isNotNull } from '@/misc/is-not-null.js';
+import { notificationTypes } from '@/types.js';
 import type { OnModuleInit } from '@nestjs/common';
 import type { CustomEmojiService } from '../CustomEmojiService.js';
 import type { UserEntityService } from './UserEntityService.js';
 import type { NoteEntityService } from './NoteEntityService.js';
 
+const NOTE_REQUIRED_NOTIFICATION_TYPES = new Set(['mention', 'reply', 'renote', 'quote', 'reaction', 'pollEnded'] as (typeof notificationTypes[number])[]);
+
 @Injectable()
 export class NotificationEntityService implements OnModuleInit {
 	private userEntityService: UserEntityService;
@@ -48,13 +50,20 @@ export class NotificationEntityService implements OnModuleInit {
 	public async pack(
 		src: Notification['id'] | Notification,
 		options: {
-			_hintForEachNotes_?: {
-				myReactions: Map<Note['id'], NoteReaction | null>;
+			_hint_?: {
+				packedNotes: Map<Note['id'], Packed<'Note'>>;
 			};
 		},
 	): Promise<Packed<'Notification'>> {
 		const notification = typeof src === 'object' ? src : await this.notificationsRepository.findOneByOrFail({ id: src });
 		const token = notification.appAccessTokenId ? await this.accessTokensRepository.findOneByOrFail({ id: notification.appAccessTokenId }) : null;
+		const noteIfNeed = NOTE_REQUIRED_NOTIFICATION_TYPES.has(notification.type) && notification.noteId != null ? (
+			options._hint_?.packedNotes != null
+				? options._hint_.packedNotes.get(notification.noteId)
+				: this.noteEntityService.pack(notification.note ?? notification.noteId!, { id: notification.notifieeId }, {
+					detail: true,
+				})
+		) : undefined;
 
 		return await awaitAll({
 			id: notification.id,
@@ -63,43 +72,10 @@ export class NotificationEntityService implements OnModuleInit {
 			isRead: notification.isRead,
 			userId: notification.notifierId,
 			user: notification.notifierId ? this.userEntityService.pack(notification.notifier ?? notification.notifierId) : null,
-			...(notification.type === 'mention' ? {
-				note: this.noteEntityService.pack(notification.note ?? notification.noteId!, { id: notification.notifieeId }, {
-					detail: true,
-					_hint_: options._hintForEachNotes_,
-				}),
-			} : {}),
-			...(notification.type === 'reply' ? {
-				note: this.noteEntityService.pack(notification.note ?? notification.noteId!, { id: notification.notifieeId }, {
-					detail: true,
-					_hint_: options._hintForEachNotes_,
-				}),
-			} : {}),
-			...(notification.type === 'renote' ? {
-				note: this.noteEntityService.pack(notification.note ?? notification.noteId!, { id: notification.notifieeId }, {
-					detail: true,
-					_hint_: options._hintForEachNotes_,
-				}),
-			} : {}),
-			...(notification.type === 'quote' ? {
-				note: this.noteEntityService.pack(notification.note ?? notification.noteId!, { id: notification.notifieeId }, {
-					detail: true,
-					_hint_: options._hintForEachNotes_,
-				}),
-			} : {}),
+			...(noteIfNeed != null ? { note: noteIfNeed } : {}),
 			...(notification.type === 'reaction' ? {
-				note: this.noteEntityService.pack(notification.note ?? notification.noteId!, { id: notification.notifieeId }, {
-					detail: true,
-					_hint_: options._hintForEachNotes_,
-				}),
 				reaction: notification.reaction,
 			} : {}),
-			...(notification.type === 'pollEnded' ? {
-				note: this.noteEntityService.pack(notification.note ?? notification.noteId!, { id: notification.notifieeId }, {
-					detail: true,
-					_hint_: options._hintForEachNotes_,
-				}),
-			} : {}),
 			...(notification.type === 'achievementEarned' ? {
 				achievement: notification.achievement,
 			} : {}),
@@ -111,6 +87,9 @@ export class NotificationEntityService implements OnModuleInit {
 		});
 	}
 
+	/**
+	 * @param notifications you should join "note" property when fetch from DB, and all notifieeId should be same as meId
+	 */
 	@bindThis
 	public async packMany(
 		notifications: Notification[],
@@ -118,25 +97,22 @@ export class NotificationEntityService implements OnModuleInit {
 	) {
 		if (notifications.length === 0) return [];
 		
-		const notes = notifications.filter(x => x.note != null).map(x => x.note!);
-		const noteIds = notes.map(n => n.id);
-		const myReactionsMap = new Map<Note['id'], NoteReaction | null>();
-		const renoteIds = notes.filter(n => n.renoteId != null).map(n => n.renoteId!);
-		const targets = [...noteIds, ...renoteIds];
-		const myReactions = await this.noteReactionsRepository.findBy({
-			userId: meId,
-			noteId: In(targets),
-		});
-
-		for (const target of targets) {
-			myReactionsMap.set(target, myReactions.find(reaction => reaction.noteId === target) ?? null);
+		for (const notification of notifications) {
+			if (meId !== notification.notifieeId) {
+				// because we call note packMany with meId, all notifieeId should be same as meId
+				throw new Error('TRY_TO_PACK_ANOTHER_USER_NOTIFICATION');
+			}
 		}
 
-		await this.customEmojiService.prefetchEmojis(this.customEmojiService.aggregateNoteEmojis(notes));
+		const notes = notifications.map(x => x.note).filter(isNotNull);
+		const packedNotesArray = await this.noteEntityService.packMany(notes, { id: meId }, {
+			detail: true,
+		});
+		const packedNotes = new Map(packedNotesArray.map(p => [p.id, p]));
 
 		return await Promise.all(notifications.map(x => this.pack(x, {
-			_hintForEachNotes_: {
-				myReactions: myReactionsMap,
+			_hint_: {
+				packedNotes,
 			},
 		})));
 	}

packages/backend/src/misc/is-not-null.ts

@@ -0,0 +1,5 @@
+// we are using {} as "any non-nullish value" as expected
+// eslint-disable-next-line @typescript-eslint/ban-types
+export function isNotNull<T extends {}>(input: T | undefined | null): input is T {
+	return input != null;
+}