diff --git a/packages/backend/src/core/FanoutTimelineEndpointService.ts b/packages/backend/src/core/FanoutTimelineEndpointService.ts index b5bbc0c3cf..2d6b7eab06 100644 --- a/packages/backend/src/core/FanoutTimelineEndpointService.ts +++ b/packages/backend/src/core/FanoutTimelineEndpointService.ts @@ -83,12 +83,18 @@ export class FanoutTimelineEndpointService { (ps.preventEmptyTimelineDbFallback !== true && redisResult.length > 1 && redisResult.some(ids => ids.length === 0)); // 取得したresultの中で最古のIDのうち、最も新しいものを取得 - const fttThresholdId = redisResult.map(ids => ids[0]).sort()[0]; + // 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 = shouldFallbackToDb ? [] : 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.filter(id => id >= fttThresholdId).slice(0, ps.limit); + let noteIds = redisResultIds.slice(0, ps.limit); const oldestNoteId = ascending ? redisResultIds[0] : redisResultIds[redisResultIds.length - 1]; shouldFallbackToDb ||= ps.useDbFallback && (noteIds.length === 0 || ps.sinceId != null && ps.sinceId < oldestNoteId); diff --git a/packages/backend/test/unit/FanoutTimelineEndpointService.ts b/packages/backend/test/unit/FanoutTimelineEndpointService.ts new file mode 100644 index 0000000000..375bf5a97a --- /dev/null +++ b/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; + let notesRepository: NotesRepository; + let usersRepository: UsersRepository; + let userProfilesRepository: UserProfilesRepository; + let idService: IdService; + + let alice: MiUser; + + async function createUser(data: Partial = {}) { + 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 = {}) { + 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); + fanoutTimelineService = app.get(FanoutTimelineService) as jest.Mocked; + notesRepository = app.get(DI.notesRepository); + usersRepository = app.get(DI.usersRepository); + userProfilesRepository = app.get(DI.userProfilesRepository); + idService = app.get(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]); + }); +});