Merge 7ad8861c64 into fc7655c808

This reverts commit 48232ca57b.

packages/backend/src/core/FanoutTimelineEndpointService.ts

@@ -14,6 +14,7 @@ import type { NotesRepository } from '@/models/_.js';
 import { NoteEntityService } from '@/core/entities/NoteEntityService.js';
 import { FanoutTimelineName, FanoutTimelineService } from '@/core/FanoutTimelineService.js';
 import { UtilityService } from '@/core/UtilityService.js';
+import { IdService } from '@/core/IdService.js';
 import { isUserRelated } from '@/misc/is-user-related.js';
 import { isQuote, isRenote } from '@/misc/is-renote.js';
 import { CacheService } from '@/core/CacheService.js';
@@ -43,7 +44,6 @@ type TimelineOptions = {
 	excludePureRenotes: boolean;
 	ignoreAuthorFromUserSuspension?: boolean;
 	dbFallback: (untilId: string | null, sinceId: string | null, limit: number) => Promise<MiNote[]>,
-	preventEmptyTimelineDbFallback?: boolean;
 };
 
 @Injectable()
@@ -60,6 +60,7 @@ export class FanoutTimelineEndpointService {
 		private fanoutTimelineService: FanoutTimelineService,
 		private utilityService: UtilityService,
 		private channelMutingService: ChannelMutingService,
+		private idService: IdService,
 	) {
 	}
 
@@ -78,9 +79,20 @@ 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));
+		// 取得したredisResultのうち、2つ以上ソースがあり、1つでも空であればDBにフォールバックする
+		const trustedEmptyIndices = new Set<number>();
+		for (let i = 0; i < redisResult.length; i++) {
+			const ids = redisResult[i];
+			const dummyIdIndex = ids.findIndex(id => this.idService.parse(id).date.getTime() === 1);
+			if (dummyIdIndex !== -1) {
+				ids.splice(dummyIdIndex, 1);
+				if (ids.length === 0) {
+					trustedEmptyIndices.add(i);
+				}
+			}
+		}
+
+		let shouldFallbackToDb = ps.useDbFallback && (redisResult.length > 1 && redisResult.some((ids, i) => ids.length === 0 && !trustedEmptyIndices.has(i)));
 
 		// 取得したresultの中で最古のIDのうち、最も新しいものを取得
 		// ids自体が空配列の場合、ids[ids.length - 1]はundefinedになるため、filterでnullを除外する
@@ -217,7 +229,33 @@ export class FanoutTimelineEndpointService {
 			return [...redisTimeline, ...gotFromDb];
 		}
 
-		return await ps.dbFallback(ps.untilId, ps.sinceId, ps.limit);
+		// RedisおよびDBが空の場合、次回以降の無駄なDBアクセスを防ぐためダミーIDを保存する
+		const gotFromDb = await ps.dbFallback(ps.untilId, ps.sinceId, ps.limit);
+		const canInject = (
+			(redisResultIds.length === 0 && ps.sinceId == null && ps.untilId == null) &&
+			(gotFromDb.length < ps.limit)
+		);
+
+		if (canInject) {
+			const dummyId = this.idService.gen(1); // 1 = Detectable Dummy Timestamp
+
+			Promise.all(ps.redisTimelines.map((tl, i) => {
+				// 有効なソースかつ結果が空だった場合のみダミーを入れる
+				if (redisResult[i] && redisResult[i].length === 0) {
+					let isEmpty = true;
+					if (gotFromDb.length > 0) {
+						isEmpty = !gotFromDb.some(n => this.accepts(tl, n));
+					}
+
+					if (isEmpty) {
+						return this.fanoutTimelineService.injectDummyIfEmpty(tl, dummyId);
+					}
+				}
+				return Promise.resolve();
+			}));
+		}
+
+		return gotFromDb;
 	}
 
 	private async getAndFilterFromDb(noteIds: string[], noteFilter: NoteFilter, idCompare: (a: string, b: string) => number): Promise<MiNote[]> {
@@ -236,4 +274,32 @@ export class FanoutTimelineEndpointService {
 
 		return notes;
 	}
+
+	private accepts(tl: FanoutTimelineName, note: MiNote): boolean {
+		if (tl === 'localTimeline') {
+			return !note.userHost && !note.replyId && note.visibility === 'public';
+		} else if (tl === 'localTimelineWithFiles') {
+			return !note.userHost && !note.replyId && note.visibility === 'public' && note.fileIds.length > 0;
+		} else if (tl === 'localTimelineWithReplies') {
+			return !note.userHost && note.replyId != null && note.visibility === 'public';
+		} else if (tl.startsWith('localTimelineWithReplyTo:')) {
+			const id = tl.split(':')[1];
+			return !note.userHost && note.replyId != null && note.replyUserId === id;
+		} else if (tl.startsWith('userTimeline:')) {
+			const id = tl.split(':')[1];
+			return note.userId === id && !note.replyId;
+		} else if (tl.startsWith('userTimelineWithFiles:')) {
+			const id = tl.split(':')[1];
+			return note.userId === id && !note.replyId && note.fileIds.length > 0;
+		} else if (tl.startsWith('userTimelineWithReplies:')) {
+			const id = tl.split(':')[1];
+			return note.userId === id && note.replyId != null;
+		} else if (tl.startsWith('userTimelineWithChannel:')) {
+			const id = tl.split(':')[1];
+			return note.userId === id && note.channelId != null;
+		} else {
+			// TODO: homeTimeline系
+			return true;
+		}
+	}
 }

packages/backend/src/core/FanoutTimelineService.ts

@@ -14,9 +14,9 @@ export type FanoutTimelineName = (
 	| `homeTimeline:${string}`
 	| `homeTimelineWithFiles:${string}` // only notes with files are included
 	// local timeline
-	| `localTimeline` // replies are not included
-	| `localTimelineWithFiles` // only non-reply notes with files are included
-	| `localTimelineWithReplies` // only replies are included
+	| 'localTimeline' // replies are not included
+	| 'localTimelineWithFiles' // only non-reply notes with files are included
+	| 'localTimelineWithReplies' // only replies are included
 	| `localTimelineWithReplyTo:${string}` // Only replies to specific local user are included. Parameter is reply user id.
 
 	// antenna
@@ -108,6 +108,21 @@ export class FanoutTimelineService {
 		});
 	}
 
+	@bindThis
+	injectDummy(tl: FanoutTimelineName, id: string) {
+		return this.redisForTimelines.lpush('list:' + tl, id);
+	}
+
+	@bindThis
+	public injectDummyIfEmpty(tl: FanoutTimelineName, id: string): Promise<boolean> {
+		return this.redisForTimelines.eval(
+			'if redis.call("LLEN", KEYS[1]) == 0 then redis.call("LPUSH", KEYS[1], ARGV[1]) return 1 else return 0 end',
+			1,
+			'list:' + tl,
+			id,
+		).then(res => res === 1);
+	}
+
 	@bindThis
 	public purge(name: FanoutTimelineName) {
 		return this.redisForTimelines.del('list:' + name);

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

@@ -151,7 +151,6 @@ 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

@@ -67,6 +67,8 @@ describe('FanoutTimelineEndpointService', () => {
 			.overrideProvider(FanoutTimelineService)
 			.useValue({
 				getMulti: jest.fn(),
+				injectDummy: jest.fn(),
+				injectDummyIfEmpty: jest.fn(),
 			})
 			.compile();
 
@@ -115,11 +117,10 @@ describe('FanoutTimelineEndpointService', () => {
 		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[],
+			redisTimelines: [`homeTimeline:${alice.id}`, 'localTimeline'] as FanoutTimelineName[],
 			useDbFallback: true,
 			limit: 10,
 			allowPartial: false,
@@ -130,9 +131,6 @@ describe('FanoutTimelineEndpointService', () => {
 			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();
@@ -172,8 +170,6 @@ describe('FanoutTimelineEndpointService', () => {
 
 		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);
@@ -224,7 +220,7 @@ describe('FanoutTimelineEndpointService', () => {
 		const dbFallback = jest.fn((untilId: string | null, sinceId: string | null, limit: number) => Promise.resolve([] as MiNote[]));
 
 		const ps = {
-			redisTimelines: ['homeTimeline', 'localTimeline'] as FanoutTimelineName[],
+			redisTimelines: [`homeTimeline:${alice.id}`, 'localTimeline'] as FanoutTimelineName[],
 			useDbFallback: false,
 			limit: 10,
 			allowPartial: true,
@@ -237,9 +233,70 @@ describe('FanoutTimelineEndpointService', () => {
 
 		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]);
 	});
+
+	// Test for dummy ID optimization
+	test('should inject dummy ID when DB fallback returns empty on initial load', async () => {
+		const redisResult: string[][] = [[], []]; // Empty timelines
+
+		fanoutTimelineService.getMulti.mockResolvedValue(redisResult);
+
+		// Mock dbFallback to return empty array
+		const dbFallback = jest.fn((_untilId: string | null, _sinceId: string | null, _limit: number) => Promise.resolve([] as MiNote[]));
+
+		const ps = {
+			redisTimelines: [`homeTimeline:${alice.id}`, 'localTimeline'] as FanoutTimelineName[],
+			useDbFallback: true,
+			limit: 10,
+			allowPartial: true,
+			excludePureRenotes: false,
+			dbFallback,
+			noteFilter: () => true,
+			untilId: null,
+			sinceId: null,
+		};
+
+		const result = await service.getMiNotes(ps);
+
+		expect(result).toEqual([]);
+		// Should have tried to inject dummy ID for both empty timelines
+		expect(fanoutTimelineService.injectDummyIfEmpty).toHaveBeenCalledTimes(2);
+		expect(fanoutTimelineService.injectDummyIfEmpty).toHaveBeenCalledWith(`homeTimeline:${alice.id}`, expect.any(String));
+		expect(fanoutTimelineService.injectDummyIfEmpty).toHaveBeenCalledWith('localTimeline', expect.any(String));
+	});
+
+	// Test for behavior when dummy ID exists
+	test('should return empty result when only dummy ID exists in Redis and DB has no newer data', async () => {
+		const now = Date.now();
+		const dummyId = idService.gen(now);
+		// Redis has only dummy ID
+		const redisResult: string[][] = [[dummyId]];
+
+		fanoutTimelineService.getMulti.mockResolvedValue(redisResult);
+
+		// Mock dbFallback (should be called to check for newer notes than the dummy ID)
+
+		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: true,
+			limit: 10,
+			allowPartial: false,
+			excludePureRenotes: false,
+			dbFallback,
+			noteFilter: () => true,
+			untilId: null,
+			sinceId: null,
+		};
+
+		const result = await service.getMiNotes(ps);
+
+		expect(result).toEqual([]);
+		// Fallback should be called to check for newer notes (ascending check from dummy ID)
+		expect(dbFallback).toHaveBeenCalled();
+	});
 });

packages/backend/test/unit/FanoutTimelineService.ts

@@ -0,0 +1,85 @@
+/*
+ * SPDX-FileCopyrightText: syuilo and misskey-project
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+import { describe, jest, test, expect, afterEach, beforeAll, afterAll } from '@jest/globals';
+import { Test, TestingModule } from '@nestjs/testing';
+import * as Redis from 'ioredis';
+import { FanoutTimelineService } from '@/core/FanoutTimelineService.js';
+import { IdService } from '@/core/IdService.js';
+import { DI } from '@/di-symbols.js';
+
+describe('FanoutTimelineService', () => {
+	let app: TestingModule;
+	let service: FanoutTimelineService;
+	let redisForTimelines: jest.Mocked<Redis.Redis>;
+	let idService: IdService;
+
+	beforeAll(async () => {
+		app = await Test.createTestingModule({
+			providers: [
+				FanoutTimelineService,
+				{
+					provide: IdService,
+					useValue: {
+						parse: jest.fn(),
+						gen: jest.fn(),
+					},
+				},
+				{
+					provide: DI.redisForTimelines,
+					useValue: {
+						eval: jest.fn(),
+						lpush: jest.fn(),
+						lrange: jest.fn(),
+						del: jest.fn(),
+						pipeline: jest.fn(() => ({
+							lpush: jest.fn(),
+							ltrim: jest.fn(),
+							lrange: jest.fn(),
+							exec: jest.fn(),
+						})),
+					},
+				},
+			],
+		}).compile();
+
+		app.enableShutdownHooks();
+
+		service = app.get<FanoutTimelineService>(FanoutTimelineService);
+		redisForTimelines = app.get(DI.redisForTimelines);
+		idService = app.get<IdService>(IdService);
+	});
+
+	afterAll(async () => {
+		await app.close();
+	});
+
+	afterEach(async () => {
+		jest.clearAllMocks();
+	});
+
+	test('injectDummyIfEmpty should call Redis EVAL with correct script', async () => {
+		redisForTimelines.eval.mockResolvedValue(1);
+
+		const result = await service.injectDummyIfEmpty('homeTimeline:123', 'dummyId');
+
+		expect(redisForTimelines.eval).toHaveBeenCalledWith(
+			expect.stringContaining('if redis.call("LLEN", KEYS[1]) == 0 then'),
+			1,
+			'list:homeTimeline:123',
+			'dummyId',
+		);
+		expect(result).toBe(true);
+	});
+
+	test('injectDummyIfEmpty should return false if list is not empty', async () => {
+		redisForTimelines.eval.mockResolvedValue(0);
+
+		const result = await service.injectDummyIfEmpty('homeTimeline:123', 'dummyId');
+
+		expect(redisForTimelines.eval).toHaveBeenCalled();
+		expect(result).toBe(false);
+	});
+});