From 77faa26e66012d145e747955f46dcaa635c3e4da Mon Sep 17 00:00:00 2001 From: zyoshoka <107108195+zyoshoka@users.noreply.github.com> Date: Sat, 22 Jun 2024 12:43:03 +0900 Subject: [PATCH] fix(backend): fallback if `sinceId` is older than the oldest in cache when using FTT (#14061) * fix(backend): fallback if `sinceId` is older than the oldest in cache when using FTT * Update CHANGELOG.md * chore: fix description of test --- CHANGELOG.md | 1 + .../src/core/FanoutTimelineEndpointService.ts | 13 +++---- packages/backend/test/e2e/timelines.ts | 35 +++++++++++++++++++ 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a40eddc5069..60d2f37f7446 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### Client ### Server +- Fix: FTT有効時、タイムライン用エンドポイントで`sinceId`にキャッシュ内最古のものより古いものを指定した場合に正しく結果が返ってこない問題を修正 ## 2024.5.0-kinel.3 diff --git a/packages/backend/src/core/FanoutTimelineEndpointService.ts b/packages/backend/src/core/FanoutTimelineEndpointService.ts index 253bc3c0c5d9..3dd3d54c9243 100644 --- a/packages/backend/src/core/FanoutTimelineEndpointService.ts +++ b/packages/backend/src/core/FanoutTimelineEndpointService.ts @@ -55,9 +55,6 @@ export class FanoutTimelineEndpointService { @bindThis private async getMiNotes(ps: TimelineOptions): Promise { - let noteIds: string[]; - let shouldFallbackToDb = false; - // 呼び出し元と以下の処理をシンプルにするためにdbFallbackを置き換える if (!ps.useDbFallback) ps.dbFallback = () => Promise.resolve([]); @@ -67,18 +64,18 @@ export class FanoutTimelineEndpointService { const redisResult = await this.fanoutTimelineService.getMulti(ps.redisTimelines, ps.untilId, ps.sinceId); // 取得したredisResultのうち、2つ以上ソースがあり、1つでも空であればDBにフォールバックする - shouldFallbackToDb = ps.useDbFallback && (redisResult.length > 1 && redisResult.some(ids => ids.length === 0)); + let shouldFallbackToDb = ps.useDbFallback && (redisResult.length > 1 && redisResult.some(ids => ids.length === 0)); // 取得したresultの中で最古のIDのうち、最も新しいものを取得 const thresholdId = redisResult.map(ids => ids[0]).sort()[0]; // TODO: いい感じにgetMulti内でソート済だからuniqするときにredisResultが全てソート済なのを利用して再ソートを避けたい - const redisResultIds = shouldFallbackToDb ? [] : Array.from(new Set(redisResult.flat(1))); + const redisResultIds = shouldFallbackToDb ? [] : Array.from(new Set(redisResult.flat(1))).sort(idCompare); - redisResultIds.sort(idCompare); - noteIds = redisResultIds.filter(id => id >= thresholdId).slice(0, ps.limit); + let noteIds = redisResultIds.filter(id => id >= thresholdId).slice(0, ps.limit); - shouldFallbackToDb = shouldFallbackToDb || (noteIds.length === 0); + const oldestNoteId = ascending ? redisResultIds[0] : redisResultIds[redisResultIds.length - 1]; + shouldFallbackToDb = shouldFallbackToDb || (noteIds.length === 0) || (ps.sinceId != null && ps.sinceId < oldestNoteId); if (!shouldFallbackToDb) { let filter = ps.noteFilter ?? (_note => true); diff --git a/packages/backend/test/e2e/timelines.ts b/packages/backend/test/e2e/timelines.ts index 05d7e5189bf1..1de4c44995b5 100644 --- a/packages/backend/test/e2e/timelines.ts +++ b/packages/backend/test/e2e/timelines.ts @@ -7,6 +7,8 @@ // pnpm jest -- e2e/timelines.ts import * as assert from 'assert'; +import { Redis } from 'ioredis'; +import { loadConfig } from '@/config.js'; import { api, post, randomString, sendEnvUpdateRequest, signup, sleep, uploadUrl } from '../utils.js'; function genHost() { @@ -17,7 +19,13 @@ function waitForPushToTl() { return sleep(500); } +let redisForTimelines: Redis; + describe('Timelines', () => { + beforeAll(() => { + redisForTimelines = new Redis(loadConfig().redisForTimelines); + }); + describe('Home TL', () => { test.concurrent('自分の visibility: followers なノートが含まれる', async () => { const [alice] = await Promise.all([signup()]); @@ -1373,6 +1381,33 @@ describe('Timelines', () => { assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), false); }); + + /** @see https://github.com/misskey-dev/misskey/issues/14000 */ + test.concurrent('FTT: sinceId にキャッシュより古いノートを指定しても、sinceId による絞り込みが正しく動作する', async () => { + const alice = await signup(); + const noteSince = await post(alice, { text: 'Note where id will be `sinceId`.' }); + const note1 = await post(alice, { text: '1' }); + const note2 = await post(alice, { text: '2' }); + await redisForTimelines.del('list:userTimeline:' + alice.id); + const note3 = await post(alice, { text: '3' }); + + const res = await api('users/notes', { userId: alice.id, sinceId: noteSince.id }); + assert.deepStrictEqual(res.body, [note1, note2, note3]); + }); + + test.concurrent('FTT: sinceId にキャッシュより古いノートを指定しても、sinceId と untilId による絞り込みが正しく動作する', async () => { + const alice = await signup(); + const noteSince = await post(alice, { text: 'Note where id will be `sinceId`.' }); + const note1 = await post(alice, { text: '1' }); + const note2 = await post(alice, { text: '2' }); + await redisForTimelines.del('list:userTimeline:' + alice.id); + const note3 = await post(alice, { text: '3' }); + const noteUntil = await post(alice, { text: 'Note where id will be `untilId`.' }); + await post(alice, { text: '4' }); + + const res = await api('users/notes', { userId: alice.id, sinceId: noteSince.id, untilId: noteUntil.id }); + assert.deepStrictEqual(res.body, [note3, note2, note1]); + }); }); // TODO: リノートミュート済みユーザーのテスト