fix(server): DriveFile related N+1 query when call note packMany (again) (#10190)
* Revert "Revert "fix(server): DriveFile related N+1 query when call note packMany (#10133)""
This reverts commit a7c82eeabc.
* packManyByIdsMap: 存在チェックをしてなかったものは null を入れるように
* Note.packMany で reply とか renote がもうあったらそのファイルも引く
* テストを書く
* fix test
* fix test
* fix test
* fix test
This commit is contained in:
rinsuki
GitHub
parent
e4fc9ea816
commit
49f0837729
|
|
@ -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,33 @@ 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'> | null>> {
|
||||
const files = await this.driveFilesRepository.findBy({ id: In(fileIds) });
|
||||
const packedFiles = await this.packMany(files, options);
|
||||
const map = new Map<Packed<'DriveFile'>['id'], Packed<'DriveFile'> | null>(packedFiles.map(f => [f.id, f]));
|
||||
for (const id of fileIds) {
|
||||
if (!map.has(id)) map.set(id, null);
|
||||
}
|
||||
return map;
|
||||
}
|
||||
|
||||
@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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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';
|
||||
|
|
@ -248,6 +249,21 @@ export class NoteEntityService implements OnModuleInit {
|
|||
return true;
|
||||
}
|
||||
|
||||
@bindThis
|
||||
public async packAttachedFiles(fileIds: Note['fileIds'], packedFiles: Map<Note['fileIds'][number], Packed<'DriveFile'> | null>): Promise<Packed<'DriveFile'>[]> {
|
||||
const missingIds = [];
|
||||
for (const id of fileIds) {
|
||||
if (!packedFiles.has(id)) missingIds.push(id);
|
||||
}
|
||||
if (missingIds.length) {
|
||||
const additionalMap = await this.driveFileEntityService.packManyByIdsMap(missingIds);
|
||||
for (const [k, v] of additionalMap) {
|
||||
packedFiles.set(k, v);
|
||||
}
|
||||
}
|
||||
return fileIds.map(id => packedFiles.get(id)).filter(isNotNull);
|
||||
}
|
||||
|
||||
@bindThis
|
||||
public async pack(
|
||||
src: Note['id'] | Note,
|
||||
|
|
@ -257,6 +273,7 @@ export class NoteEntityService implements OnModuleInit {
|
|||
skipHide?: boolean;
|
||||
_hint_?: {
|
||||
myReactions: Map<Note['id'], NoteReaction | null>;
|
||||
packedFiles: Map<Note['fileIds'][number], Packed<'DriveFile'> | null>;
|
||||
};
|
||||
},
|
||||
): Promise<Packed<'Note'>> {
|
||||
|
|
@ -284,6 +301,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 +322,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 ? this.packAttachedFiles(note.fileIds, packedFiles) : this.driveFileEntityService.packManyByIds(note.fileIds),
|
||||
replyId: note.replyId,
|
||||
renoteId: note.renoteId,
|
||||
channelId: note.channelId ?? undefined,
|
||||
|
|
@ -388,11 +406,15 @@ export class NoteEntityService implements OnModuleInit {
|
|||
}
|
||||
|
||||
await this.customEmojiService.prefetchEmojis(this.customEmojiService.aggregateNoteEmojis(notes));
|
||||
// TODO: 本当は renote とか reply がないのに renoteId とか replyId があったらここで解決しておく
|
||||
const fileIds = notes.map(n => [n.fileIds, n.renote?.fileIds, n.reply?.fileIds]).flat(2).filter(isNotNull);
|
||||
const packedFiles = await this.driveFileEntityService.packManyByIdsMap(fileIds);
|
||||
|
||||
return await Promise.all(notes.map(n => this.pack(n, me, {
|
||||
...options,
|
||||
_hint_: {
|
||||
myReactions: myReactionsMap,
|
||||
packedFiles,
|
||||
},
|
||||
})));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ process.env.NODE_ENV = 'test';
|
|||
|
||||
import * as assert from 'assert';
|
||||
import { Note } from '@/models/entities/Note.js';
|
||||
import { signup, post, uploadUrl, startServer, initTestDb, api } from '../utils.js';
|
||||
import { signup, post, uploadUrl, startServer, initTestDb, api, uploadFile } from '../utils.js';
|
||||
import type { INestApplicationContext } from '@nestjs/common';
|
||||
|
||||
describe('Note', () => {
|
||||
|
|
@ -213,6 +213,122 @@ describe('Note', () => {
|
|||
assert.deepStrictEqual(noteDoc.mentions, [bob.id]);
|
||||
});
|
||||
|
||||
describe('添付ファイル情報', () => {
|
||||
test('ファイルを添付した場合、投稿成功時にファイル情報入りのレスポンスが帰ってくる', async () => {
|
||||
const file = await uploadFile(alice);
|
||||
const res = await api('/notes/create', {
|
||||
fileIds: [file.body.id],
|
||||
}, alice);
|
||||
|
||||
assert.strictEqual(res.status, 200);
|
||||
assert.strictEqual(typeof res.body === 'object' && !Array.isArray(res.body), true);
|
||||
assert.strictEqual(res.body.createdNote.files.length, 1);
|
||||
assert.strictEqual(res.body.createdNote.files[0].id, file.body.id);
|
||||
});
|
||||
|
||||
test('ファイルを添付した場合、タイムラインでファイル情報入りのレスポンスが帰ってくる', async () => {
|
||||
const file = await uploadFile(alice);
|
||||
const createdNote = await api('/notes/create', {
|
||||
fileIds: [file.body.id],
|
||||
}, alice);
|
||||
|
||||
assert.strictEqual(createdNote.status, 200);
|
||||
|
||||
const res = await api('/notes', {
|
||||
withFiles: true,
|
||||
}, alice);
|
||||
|
||||
assert.strictEqual(res.status, 200);
|
||||
assert.strictEqual(Array.isArray(res.body), true);
|
||||
const myNote = res.body.find((note: { id: string; files: { id: string }[] }) => note.id === createdNote.body.createdNote.id);
|
||||
assert.notEqual(myNote, null);
|
||||
assert.strictEqual(myNote.files.length, 1);
|
||||
assert.strictEqual(myNote.files[0].id, file.body.id);
|
||||
});
|
||||
|
||||
test('ファイルが添付されたノートをリノートした場合、タイムラインでファイル情報入りのレスポンスが帰ってくる', async () => {
|
||||
const file = await uploadFile(alice);
|
||||
const createdNote = await api('/notes/create', {
|
||||
fileIds: [file.body.id],
|
||||
}, alice);
|
||||
|
||||
assert.strictEqual(createdNote.status, 200);
|
||||
|
||||
const renoted = await api('/notes/create', {
|
||||
renoteId: createdNote.body.createdNote.id,
|
||||
}, alice);
|
||||
assert.strictEqual(renoted.status, 200);
|
||||
|
||||
const res = await api('/notes', {
|
||||
renote: true,
|
||||
}, alice);
|
||||
|
||||
assert.strictEqual(res.status, 200);
|
||||
assert.strictEqual(Array.isArray(res.body), true);
|
||||
const myNote = res.body.find((note: { id: string }) => note.id === renoted.body.createdNote.id);
|
||||
assert.notEqual(myNote, null);
|
||||
assert.strictEqual(myNote.renote.files.length, 1);
|
||||
assert.strictEqual(myNote.renote.files[0].id, file.body.id);
|
||||
});
|
||||
|
||||
test('ファイルが添付されたノートに返信した場合、タイムラインでファイル情報入りのレスポンスが帰ってくる', async () => {
|
||||
const file = await uploadFile(alice);
|
||||
const createdNote = await api('/notes/create', {
|
||||
fileIds: [file.body.id],
|
||||
}, alice);
|
||||
|
||||
assert.strictEqual(createdNote.status, 200);
|
||||
|
||||
const reply = await api('/notes/create', {
|
||||
replyId: createdNote.body.createdNote.id,
|
||||
text: 'this is reply',
|
||||
}, alice);
|
||||
assert.strictEqual(reply.status, 200);
|
||||
|
||||
const res = await api('/notes', {
|
||||
reply: true,
|
||||
}, alice);
|
||||
|
||||
assert.strictEqual(res.status, 200);
|
||||
assert.strictEqual(Array.isArray(res.body), true);
|
||||
const myNote = res.body.find((note: { id: string }) => note.id === reply.body.createdNote.id);
|
||||
assert.notEqual(myNote, null);
|
||||
assert.strictEqual(myNote.reply.files.length, 1);
|
||||
assert.strictEqual(myNote.reply.files[0].id, file.body.id);
|
||||
});
|
||||
|
||||
test('ファイルが添付されたノートへの返信をリノートした場合、タイムラインでファイル情報入りのレスポンスが帰ってくる', async () => {
|
||||
const file = await uploadFile(alice);
|
||||
const createdNote = await api('/notes/create', {
|
||||
fileIds: [file.body.id],
|
||||
}, alice);
|
||||
|
||||
assert.strictEqual(createdNote.status, 200);
|
||||
|
||||
const reply = await api('/notes/create', {
|
||||
replyId: createdNote.body.createdNote.id,
|
||||
text: 'this is reply',
|
||||
}, alice);
|
||||
assert.strictEqual(reply.status, 200);
|
||||
|
||||
const renoted = await api('/notes/create', {
|
||||
renoteId: reply.body.createdNote.id,
|
||||
}, alice);
|
||||
assert.strictEqual(renoted.status, 200);
|
||||
|
||||
const res = await api('/notes', {
|
||||
renote: true,
|
||||
}, alice);
|
||||
|
||||
assert.strictEqual(res.status, 200);
|
||||
assert.strictEqual(Array.isArray(res.body), true);
|
||||
const myNote = res.body.find((note: { id: string }) => note.id === renoted.body.createdNote.id);
|
||||
assert.notEqual(myNote, null);
|
||||
assert.strictEqual(myNote.renote.reply.files.length, 1);
|
||||
assert.strictEqual(myNote.renote.reply.files[0].id, file.body.id);
|
||||
});
|
||||
});
|
||||
|
||||
describe('notes/create', () => {
|
||||
test('投票を添付できる', async () => {
|
||||
const res = await api('/notes/create', {
|
||||
|
|
|
|||
Loading…
Reference in New Issue