Merge b99ed65051 into fc7655c808

CHANGELOG.md

@@ -1261,7 +1261,7 @@ v2025.12.0で行われた「configの`trustProxy`のデフォルト値を`false`
 - Fix: カスタム絵文字の画像読み込みに失敗した際はテキストではなくダミー画像を表示 #13487
 
 ### Server
--
+- Fix: FTT有効かつDBフォールバック有効時、STLのようにタイムラインのソースが複数だとFTTとDBのフォールバック間で取得されないノートがある問題
 
 ## 2024.3.0
 

packages/backend/src/core/FanoutTimelineEndpointService.ts

@@ -43,6 +43,7 @@ type TimelineOptions = {
 	excludePureRenotes: boolean;
 	ignoreAuthorFromUserSuspension?: boolean;
 	dbFallback: (untilId: string | null, sinceId: string | null, limit: number) => Promise<MiNote[]>,
+	preventEmptyTimelineDbFallback?: boolean;
 };
 
 @Injectable()
@@ -77,12 +78,26 @@ export class FanoutTimelineEndpointService {
 
 		const redisResult = await this.fanoutTimelineService.getMulti(ps.redisTimelines, ps.untilId, ps.sinceId);
 
+		// オプション無効時、取得したredisResultのうち、2つ以上ソースがあり、1つでも空であればDBにフォールバックする
+		let shouldFallbackToDb = ps.useDbFallback &&
+			(ps.preventEmptyTimelineDbFallback !== true && redisResult.length > 1 && redisResult.some(ids => ids.length === 0));
+
+		// 取得したresultの中で最古のIDのうち、最も新しいものを取得
+		// ids自体が空配列の場合、ids[ids.length - 1]はundefinedになるため、filterでnullを除外する
+		// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
+		const fttThresholdId = redisResult.map(ids => ascending ? ids[0] : ids[ids.length - 1]).filter(id => id != null).sort().pop();
+
 		// TODO: いい感じにgetMulti内でソート済だからuniqするときにredisResultが全てソート済なのを利用して再ソートを避けたい
-		const redisResultIds = Array.from(new Set(redisResult.flat(1))).sort(idCompare);
+		let redisResultIds = shouldFallbackToDb ? [] : Array.from(new Set(redisResult.flat(1)));
+		if (ps.useDbFallback && fttThresholdId != null) {
+			redisResultIds = redisResultIds.filter(id => id >= fttThresholdId);
+		}
+		redisResultIds.sort(idCompare);
 
 		let noteIds = redisResultIds.slice(0, ps.limit);
+
 		const oldestNoteId = ascending ? redisResultIds[0] : redisResultIds[redisResultIds.length - 1];
-		const shouldFallbackToDb = noteIds.length === 0 || ps.sinceId != null && ps.sinceId < oldestNoteId;
+		shouldFallbackToDb ||= ps.useDbFallback && (noteIds.length === 0 || ps.sinceId != null && ps.sinceId < oldestNoteId);
 
 		if (!shouldFallbackToDb) {
 			let filter = ps.noteFilter ?? (_note => true) as NoteFilter;

packages/backend/src/server/api/endpoints/users/notes.ts

@@ -151,6 +151,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 					withFiles: ps.withFiles,
 					withRenotes: ps.withRenotes,
 				}, me),
+				preventEmptyTimelineDbFallback: true,
 			});
 
 			return timeline;

packages/backend/test/unit/FanoutTimelineEndpointService.ts

@@ -0,0 +1,245 @@
+/*
+ * SPDX-FileCopyrightText: syuilo and misskey-project
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+import { describe, jest, test, expect, beforeEach, afterEach, beforeAll, afterAll } from '@jest/globals';
+import { Test, TestingModule } from '@nestjs/testing';
+import { GlobalModule } from '@/GlobalModule.js';
+import { CoreModule } from '@/core/CoreModule.js';
+import { FanoutTimelineEndpointService } from '@/core/FanoutTimelineEndpointService.js';
+import { FanoutTimelineService, FanoutTimelineName } from '@/core/FanoutTimelineService.js';
+import { IdService } from '@/core/IdService.js';
+import { NotesRepository, UsersRepository, UserProfilesRepository, MiUser, MiNote } from '@/models/_.js';
+import { DI } from '@/di-symbols.js';
+
+describe('FanoutTimelineEndpointService', () => {
+	let app: TestingModule;
+	let service: FanoutTimelineEndpointService;
+	let fanoutTimelineService: jest.Mocked<FanoutTimelineService>;
+	let notesRepository: NotesRepository;
+	let usersRepository: UsersRepository;
+	let userProfilesRepository: UserProfilesRepository;
+	let idService: IdService;
+
+	let alice: MiUser;
+
+	async function createUser(data: Partial<MiUser> = {}) {
+		const user = await usersRepository
+			.insert({
+				id: idService.gen(),
+				username: 'username',
+				usernameLower: 'username',
+				...data,
+			})
+			.then(x => usersRepository.findOneByOrFail(x.identifiers[0]));
+
+		await userProfilesRepository.insert({
+			userId: user.id,
+		});
+
+		return user;
+	}
+
+	async function createNote(data: Partial<MiNote> = {}) {
+		return await notesRepository
+			.insert({
+				id: idService.gen(),
+				userId: alice.id,
+				text: 'test',
+				visibility: 'public',
+				localOnly: false,
+				...data,
+			})
+			.then(x => notesRepository.findOneByOrFail(x.identifiers[0]));
+	}
+
+	beforeAll(async () => {
+		app = await Test.createTestingModule({
+			imports: [
+				GlobalModule,
+				CoreModule,
+			],
+			providers: [
+				FanoutTimelineEndpointService,
+			],
+		})
+			.overrideProvider(FanoutTimelineService)
+			.useValue({
+				getMulti: jest.fn(),
+			})
+			.compile();
+
+		app.enableShutdownHooks();
+
+		service = app.get<FanoutTimelineEndpointService>(FanoutTimelineEndpointService);
+		fanoutTimelineService = app.get(FanoutTimelineService) as jest.Mocked<FanoutTimelineService>;
+		notesRepository = app.get<NotesRepository>(DI.notesRepository);
+		usersRepository = app.get<UsersRepository>(DI.usersRepository);
+		userProfilesRepository = app.get<UserProfilesRepository>(DI.userProfilesRepository);
+		idService = app.get<IdService>(IdService);
+	});
+
+	afterAll(async () => {
+		await app.close();
+	});
+
+	beforeEach(async () => {
+		alice = await createUser({ username: 'alice', usernameLower: 'alice' });
+	});
+
+	afterEach(async () => {
+		jest.clearAllMocks();
+		await notesRepository.deleteAll();
+		await userProfilesRepository.deleteAll();
+		await usersRepository.deleteAll();
+	});
+
+	test('should use correctly calculated threshold (Max of Oldest) when merging disjoint timelines', async () => {
+		const now = Date.now();
+		// HTL: Recent (T-2m to T-4m)
+		const htlNote1 = await createNote({ id: idService.gen(now - 1000 * 60 * 2) });
+		const htlNote2 = await createNote({ id: idService.gen(now - 1000 * 60 * 3) });
+		const htlNote3 = await createNote({ id: idService.gen(now - 1000 * 60 * 4) }); // End of HTL (T-4m)
+
+		const htlIds = [htlNote1.id, htlNote2.id, htlNote3.id];
+
+		// LTL: Old (T-60m to T-62m)
+		const ltlNote1 = await createNote({ id: idService.gen(now - 1000 * 60 * 60) });
+		const ltlNote2 = await createNote({ id: idService.gen(now - 1000 * 60 * 61) });
+		const ltlNote3 = await createNote({ id: idService.gen(now - 1000 * 60 * 62) });
+
+		const ltlIds = [ltlNote1.id, ltlNote2.id, ltlNote3.id];
+
+		// Mock FanoutTimelineService to return these IDs
+		fanoutTimelineService.getMulti.mockResolvedValue([htlIds, ltlIds]);
+
+		// dbFallback spy
+		// eslint-disable-next-line @typescript-eslint/no-unused-vars
+		const dbFallback = jest.fn((_untilId: string | null, _sinceId: string | null, _limit: number) => Promise.resolve([] as MiNote[]));
+
+		const ps = {
+			redisTimelines: ['homeTimeline', 'localTimeline'] as FanoutTimelineName[],
+			useDbFallback: true,
+			limit: 10,
+			allowPartial: false,
+			excludePureRenotes: false,
+			dbFallback,
+			noteFilter: () => false, // Simulate strict filtering (force fallback)
+			untilId: null,
+			sinceId: null,
+		};
+
+		// See comments in original file for logic explanation.
+		// Essentially, we expect the fallback to start from the end of the most recent reliable timeline (HTL).
+
+		await service.getMiNotes(ps);
+
+		expect(dbFallback).toHaveBeenCalled();
+		const callArgs = dbFallback.mock.calls[0];
+		const untilId = callArgs[0] as string;
+
+		// We expect untilId to be the HTL oldest (htlNote3.id), NOT the LTL newest (ltlNote1.id).
+		expect(untilId).toBe(htlNote3.id);
+		expect(untilId > ltlNote1.id).toBe(true);
+	});
+
+	test('should maintain correct pagination cursor when using sinceId (ascending)', async () => {
+		const now = Date.now();
+		// Ascending: Oldest to Newest.
+		const note1 = await createNote({ id: idService.gen(now - 3000) });
+		const note2 = await createNote({ id: idService.gen(now - 2000) });
+		const note3 = await createNote({ id: idService.gen(now - 1000) });
+
+		const ids = [note1.id, note2.id, note3.id];
+
+		fanoutTimelineService.getMulti.mockResolvedValue([ids]);
+
+		// Mock dbFallback to return empty array
+		// eslint-disable-next-line @typescript-eslint/no-unused-vars
+		const dbFallback = jest.fn((untilId: string | null, sinceId: string | null, limit: number) => Promise.resolve([] as MiNote[]));
+
+		const ps = {
+			redisTimelines: [`homeTimeline:${alice.id}`] as FanoutTimelineName[],
+			useDbFallback: false, // Disable fallback to check Redis filtering logic directly
+			limit: 2,
+			allowPartial: true,
+			excludePureRenotes: false,
+			dbFallback,
+			untilId: null,
+			sinceId: idService.gen(now - 4000),
+		};
+
+		const result = await service.getMiNotes(ps);
+
+		// With the fix, we should get note1 and note2.
+		// Without the fix, we would get only note3 (or empty if limit blocked it).
+		expect(result).toHaveLength(2);
+		expect(result[0].id).toBe(note1.id);
+		expect(result[1].id).toBe(note2.id);
+	});
+
+	test('should not fallback to DB when useDbFallback is false even if insufficient notes', async () => {
+		const now = Date.now();
+		const note1 = await createNote({ id: idService.gen(now) });
+		const ids = [note1.id];
+
+		fanoutTimelineService.getMulti.mockResolvedValue([ids]);
+
+		const dbFallback = jest.fn((_untilId: string | null, _sinceId: string | null, _limit: number) => Promise.resolve([] as MiNote[]));
+
+		const ps = {
+			redisTimelines: [`homeTimeline:${alice.id}`] as FanoutTimelineName[],
+			useDbFallback: false,
+			limit: 10,
+			allowPartial: false,
+			excludePureRenotes: false,
+			dbFallback,
+			noteFilter: () => false, // Filter out everything
+			untilId: null,
+			sinceId: null,
+		};
+
+		const result = await service.getMiNotes(ps);
+
+		expect(dbFallback).not.toHaveBeenCalled();
+		expect(result).toEqual([]);
+	});
+
+	test('should merge disjoint timelines correctly when useDbFallback is false', async () => {
+		const now = Date.now();
+		// TL1: Recent
+		const note1 = await createNote({ id: idService.gen(now - 1000) });
+		const note2 = await createNote({ id: idService.gen(now - 2000) });
+		// TL2: Old
+		const note3 = await createNote({ id: idService.gen(now - 5000) });
+		const note4 = await createNote({ id: idService.gen(now - 6000) });
+
+		const ids1 = [note1.id, note2.id];
+		const ids2 = [note3.id, note4.id];
+
+		fanoutTimelineService.getMulti.mockResolvedValue([ids1, ids2]);
+
+		// eslint-disable-next-line @typescript-eslint/no-unused-vars
+		const dbFallback = jest.fn((untilId: string | null, sinceId: string | null, limit: number) => Promise.resolve([] as MiNote[]));
+
+		const ps = {
+			redisTimelines: ['homeTimeline', 'localTimeline'] as FanoutTimelineName[],
+			useDbFallback: false,
+			limit: 10,
+			allowPartial: true,
+			excludePureRenotes: false,
+			dbFallback,
+			noteFilter: () => true, // Accept all
+			untilId: null,
+			sinceId: null,
+		};
+
+		const result = await service.getMiNotes(ps);
+
+		// With the previous logic, note3 and note4 would be filtered out because they are older than the "threshold" (end of TL1).
+		// With the fixed logic (skipping filter when !useDbFallback), all notes should be present.
+		expect(result).toHaveLength(4);
+		expect(result.map(n => n.id)).toEqual([note1.id, note2.id, note3.id, note4.id]);
+	});
+});