diff --git a/CHANGELOG.md b/CHANGELOG.md index 4787ea4efd6..6ed77861875 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ - Fix: Sentry 使用環境下にて、外部送信リクエストへ `sentry-trace` / `baggage` ヘッダーが既定で付与されないように - Fix: フォロワー限定投稿へのリプライをホーム投稿に出来る問題を修正 - Fix: ファイルをアップロードするAPIにて、処理終了後に一時ファイルが削除されないことがある問題を修正 +- Fix: 表示されない通知が残り通知数として表示され、未読バッジが通常操作で消えなくなる問題を修正 (#17427) ## 2026.6.0 diff --git a/packages/backend/src/core/NotificationService.ts b/packages/backend/src/core/NotificationService.ts index 670cecf01ee..c644ec293ce 100644 --- a/packages/backend/src/core/NotificationService.ts +++ b/packages/backend/src/core/NotificationService.ts @@ -64,7 +64,7 @@ export class NotificationService implements OnApplicationShutdown { this.redisClient.set(`latestReadNotification:${userId}`, latestNotificationId); - if (force || latestReadNotificationId == null || (latestReadNotificationId < latestNotificationId)) { + if (force || latestReadNotificationId == null || (this.compareXListIds(latestReadNotificationId, latestNotificationId) < 0)) { return this.postReadAllNotifications(userId); } } @@ -189,7 +189,7 @@ export class NotificationService implements OnApplicationShutdown { const interval = notification.type === 'test' ? 0 : 2000; setTimeout(interval, 'unread notification', { signal: this.#shutdownController.signal }).then(async () => { const latestReadNotificationId = await this.redisClient.get(`latestReadNotification:${notifieeId}`); - if (latestReadNotificationId && (latestReadNotificationId >= redisId)) return; + if (latestReadNotificationId && (this.compareXListIds(latestReadNotificationId, redisId) >= 0)) return; this.globalEventService.publishMainStream(notifieeId, 'unreadNotification', packed); this.pushNotificationService.pushNotification(notifieeId, 'notification', packed); @@ -244,6 +244,16 @@ export class NotificationService implements OnApplicationShutdown { this.#shutdownController.abort(); } + // Redis Stream ID (`ms-seq`) を Redis 本体と同じ数値順で比較する。 + // JS の文字列比較は同一 ms で seq の桁数が異なる ID の順序を誤る ("...-9" < "...-10" が false になる) ため使わないこと + private compareXListIds(a: string, b: string): number { + const [aMs = 0n, aSeq = 0n] = a.split('-').map(BigInt); + const [bMs = 0n, bSeq = 0n] = b.split('-').map(BigInt); + if (aMs !== bMs) return aMs < bMs ? -1 : 1; + if (aSeq !== bSeq) return aSeq < bSeq ? -1 : 1; + return 0; + } + private toXListId(id: string): string { const { date, additional } = this.idService.parseFull(id); // Redis Stream sequenceはunit64制約があるため、収まらない場合は下位64bitを取る diff --git a/packages/backend/src/core/entities/UserEntityService.ts b/packages/backend/src/core/entities/UserEntityService.ts index 996f0bad2e8..2e858bd3a0e 100644 --- a/packages/backend/src/core/entities/UserEntityService.ts +++ b/packages/backend/src/core/entities/UserEntityService.ts @@ -49,13 +49,19 @@ import type { CustomEmojiService } from '@/core/CustomEmojiService.js'; import { AvatarDecorationService } from '@/core/AvatarDecorationService.js'; import { ChatService } from '@/core/ChatService.js'; import type { OnModuleInit } from '@nestjs/common'; +import type { MiNotification } from '@/models/Notification.js'; import type { NoteEntityService } from './NoteEntityService.js'; +import type { NotificationEntityService } from './NotificationEntityService.js'; import type { PageEntityService } from './PageEntityService.js'; import { toArray } from '@/misc/prelude/array.js'; const Ajv = _Ajv.default; const ajv = new Ajv(); +// 未読通知数はこの件数で打ち切る (UI は 99 超を "99+" と表示するため、それ以上数えても意味がない) +const UNREAD_NOTIFICATION_COUNT_LIMIT = 100; +const UNREAD_NOTIFICATION_BATCH_SIZE = 100; + function isLocalUser(user: MiUser): user is MiLocalUser; function isLocalUser(user: T): user is (T & { host: null; }); @@ -87,6 +93,7 @@ export type UserRelation = { export class UserEntityService implements OnModuleInit { private apPersonService: ApPersonService; private noteEntityService: NoteEntityService; + private notificationEntityService: NotificationEntityService; private pageEntityService: PageEntityService; private customEmojiService: CustomEmojiService; private announcementService: AnnouncementService; @@ -143,6 +150,7 @@ export class UserEntityService implements OnModuleInit { onModuleInit() { this.apPersonService = this.moduleRef.get('ApPersonService'); this.noteEntityService = this.moduleRef.get('NoteEntityService'); + this.notificationEntityService = this.moduleRef.get('NotificationEntityService'); this.pageEntityService = this.moduleRef.get('PageEntityService'); this.customEmojiService = this.moduleRef.get('CustomEmojiService'); this.announcementService = this.moduleRef.get('AnnouncementService'); @@ -342,21 +350,42 @@ export class UserEntityService implements OnModuleInit { const latestReadNotificationId = await this.redisClient.get(`latestReadNotification:${userId}`); - if (!latestReadNotificationId) { - response.unreadCount = await this.redisClient.xlen(`notificationTimeline:${userId}`); - } else { - const latestNotificationIdsRes = await this.redisClient.xrevrange( + // Stream 上のエントリ数をそのまま未読カウントにすると、 + // packMany で除外される通知 (削除済みノート・サスペンドされた notifier・ + // 解決済みフォローリクエスト・削除済みロール等) も含めてしまい、 + // 「通知ページには表示されないのにバッジだけ残る」という乖離が発生する (misskey-dev/misskey#17427)。 + // API 表示と一致させるため、packMany と同じバリデータを通した件数を未読数とする。 + // + // ただし未読範囲全件 (最大 perUserNotificationsMaxCount 件) を毎回 pack すると + // MeDetailed を返すホットパスで DB 参照が嵩むため、バッチに分けて pack し、 + // UNREAD_NOTIFICATION_COUNT_LIMIT 件に達した時点で打ち切る (UI の表示は 99 超で "99+")。 + // latestReadNotificationId が無い (一度も既読化していない) 場合は Stream 全体が未読範囲。 + // 既読位置がある場合は exclusive 比較で「既読位置より新しい」エントリだけ拾う。 + let cursor = latestReadNotificationId ? '(' + latestReadNotificationId : '-'; + + while (response.unreadCount < UNREAD_NOTIFICATION_COUNT_LIMIT) { + const notificationsRes = await this.redisClient.xrange( `notificationTimeline:${userId}`, + cursor, '+', - latestReadNotificationId, + 'COUNT', UNREAD_NOTIFICATION_BATCH_SIZE, ); - response.unreadCount = (latestNotificationIdsRes.length - 1 >= 0) ? latestNotificationIdsRes.length - 1 : 0; + if (notificationsRes.length === 0) break; + + const notifications = notificationsRes.map(x => JSON.parse(x[1][1])) as MiNotification[]; + const validNotifications = await this.notificationEntityService.packMany(notifications, userId); + response.unreadCount += validNotifications.length; + + if (notificationsRes.length < UNREAD_NOTIFICATION_BATCH_SIZE) break; + cursor = '(' + notificationsRes[notificationsRes.length - 1][0]; } - if (response.unreadCount > 0) { - response.hasUnread = true; + // バッチ境界で LIMIT を超え得るため飽和させる + if (response.unreadCount > UNREAD_NOTIFICATION_COUNT_LIMIT) { + response.unreadCount = UNREAD_NOTIFICATION_COUNT_LIMIT; } + response.hasUnread = response.unreadCount > 0; return response; } diff --git a/packages/backend/src/models/json-schema/user.ts b/packages/backend/src/models/json-schema/user.ts index f71ec1d023e..9e63db49f8a 100644 --- a/packages/backend/src/models/json-schema/user.ts +++ b/packages/backend/src/models/json-schema/user.ts @@ -564,6 +564,7 @@ export const packedMeDetailedOnlySchema = { unreadNotificationsCount: { type: 'number', nullable: false, optional: false, + description: 'Saturates at 100. The actual number of unread notifications may be larger.', }, mutedWords: { type: 'array', diff --git a/packages/backend/test/e2e/notification-unread-count.ts b/packages/backend/test/e2e/notification-unread-count.ts new file mode 100644 index 00000000000..88bd00b1c0b --- /dev/null +++ b/packages/backend/test/e2e/notification-unread-count.ts @@ -0,0 +1,236 @@ +/* + * SPDX-FileCopyrightText: syuilo and misskey-project + * SPDX-License-Identifier: AGPL-3.0-only + */ + +process.env.NODE_ENV = 'test'; + +import * as assert from 'node:assert'; +import { setTimeout } from 'node:timers/promises'; +import * as Redis from 'ioredis'; +import { afterAll, beforeAll, beforeEach, describe, test } from 'vitest'; +import { api, post, signup, waitFire } from '../utils.js'; +import type * as misskey from 'misskey-js'; + +// このテストは https://github.com/misskey-dev/misskey/issues/17427 +// 「通知バルーンが消えない問題」の回帰防止。 +// +// 旧 UserEntityService.getNotificationsInfo() は Redis Stream の生エントリ数で +// 未読カウントを返していたため、packMany で除外される通知 (解決済みフォロー +// リクエスト・削除済みノート・サスペンドされた notifier 等) もカウントに含まれ、 +// 「通知ページを開いても何もないのにバッジだけ残る」状態を引き起こしていた。 +// +// 修正後は packMany 相当のバリデータを通した件数を返すため、 +// API レスポンスとバッジカウントは常に一致する。 +describe('Notification unread count consistency (misskey-dev/misskey#17427)', () => { + let alice: misskey.entities.SignupResponse; + let bob: misskey.entities.SignupResponse; + + beforeAll(async () => { + alice = await signup({ username: 'alice_n17427' }); + bob = await signup({ username: 'bob_n17427' }); + }, 1000 * 60 * 2); + + // 各テスト間で Stream エントリ自体を破棄してテスト間の汚染を防ぐ。 + // mark-all-as-read だと latestReadNotificationId を進めるだけで Stream は残る。 + beforeEach(async () => { + await api('notifications/flush', {}, alice); + await api('notifications/flush', {}, bob); + await setTimeout(500); + }); + + test('承認済みフォローリクエスト通知は表示されず、カウントにも含まれない', async () => { + // alice をロックする → bob がフォローするとフォローリクエストになる + await api('i/update', { isLocked: true }, alice); + + // bob → alice フォローリクエスト発生 (alice に receiveFollowRequest 通知) + await api('following/create', { userId: alice.id }, bob); + + // createNotification の setTimeout(2000) 後に unreadNotification が発火するため待機 + await setTimeout(2500); + + // 承認前: 通知が 1 件未読として見える状態 + const beforeAccept = await api('i', {}, alice); + assert.strictEqual(beforeAccept.status, 200); + assert.strictEqual(beforeAccept.body.unreadNotificationsCount, 1, + '承認前: フォローリクエスト通知でカウント 1'); + assert.strictEqual(beforeAccept.body.hasUnreadNotification, true); + + // alice 側で承認 → receiveFollowRequest が解決済みになる & + // 同時に follow 通知が新規発生する (フォロワー追加の通知) + await api('following/requests/accept', { userId: bob.id }, alice); + await setTimeout(200); + + // 承認後の状態を観測する。 + // markAsRead=false で内部の readAllNotification を呼ばないようにする + const afterAcceptI = await api('i', {}, alice); + const notificationsRes = await api('i/notifications', { markAsRead: false }, alice); + assert.strictEqual(notificationsRes.status, 200); + + // receiveFollowRequest 通知は packMany で除外されているはず + const remainingReceiveFollowRequest = notificationsRes.body.filter( + (n: { type: string }) => n.type === 'receiveFollowRequest', + ); + assert.strictEqual(remainingReceiveFollowRequest.length, 0, + '承認後: 解決済みの receiveFollowRequest 通知は API レスポンスに現れない'); + + // 修正の本旨: バッジカウントは API レスポンスの件数と一致する + assert.strictEqual(afterAcceptI.body.unreadNotificationsCount, notificationsRes.body.length, + 'unreadNotificationsCount は packMany フィルタ後の件数と一致する (Stream の生件数ではなく)'); + + // 後片付け + await api('i/update', { isLocked: false }, alice); + await api('following/delete', { userId: alice.id }, bob); + }, 1000 * 30); + + test('mention 通知が来たときは API 件数 = バッジカウントが揃う', async () => { + await post(bob, { text: `@alice_n17427 hi ${Date.now()}` }); + // createNotificationInternal の setTimeout(2000) 経過待ち + await setTimeout(2500); + + const userInfo = await api('i', {}, alice); + const notificationsRes = await api('i/notifications', { markAsRead: false }, alice); + + assert.strictEqual(userInfo.body.unreadNotificationsCount, notificationsRes.body.length, + 'unreadNotificationsCount は packMany フィルタ後の件数と一致する'); + assert.ok(userInfo.body.unreadNotificationsCount >= 1, + 'mention 通知 1 件以上で unreadNotificationsCount >= 1'); + }, 1000 * 30); + + test('元のメンション元ノートが削除されると、その通知はカウントから除外される', async () => { + // bob から alice にメンション → alice に通知が立つ + const bobNote = await post(bob, { text: `@alice_n17427 will be deleted ${Date.now()}` }); + await setTimeout(2500); + + // 通知が立った状態を確認 + const beforeDelete = await api('i', {}, alice); + assert.ok(beforeDelete.body.unreadNotificationsCount >= 1, 'ノート削除前はカウント >= 1'); + + // bob がノートを削除する → packMany で「ノート無し」として除外されるはず + await api('notes/delete', { noteId: bobNote.id }, bob); + await setTimeout(500); + + const afterDelete = await api('i', {}, alice); + const notificationsRes = await api('i/notifications', { markAsRead: false }, alice); + + // API レスポンスから該当通知が消えていることを確認 + const stillVisible = notificationsRes.body.filter( + (n: { type: string; note?: { id: string } }) => n.type === 'mention' && n.note?.id === bobNote.id, + ); + assert.strictEqual(stillVisible.length, 0, + 'ノート削除後: mention 通知は packMany で除外され API レスポンスに現れない'); + + // バッジカウントも同様に減っているはず (修正前は Stream に残っていたのでカウントだけ残る現象が出た) + assert.strictEqual(afterDelete.body.unreadNotificationsCount, notificationsRes.body.length, + 'ノート削除後: unreadNotificationsCount は API レスポンス件数と一致する'); + }, 1000 * 30); +}); + +// 「通知タブを開いても未読バルーンが消えず、リロードでのみ消える」現象の切り分けテスト。 +// +// フロントは通知タブを開くと i/notifications-grouped (markAsRead=true 既定) を叩き、 +// バックエンドの NotificationService.readAllNotification() が既読位置を進めた場合に +// mainStream へ 'readAllNotifications' を publish する。フロントのバルーンはこの +// イベントを受けて初めて消えるため、イベントが publish されない経路が 1 つでもあると +// 「バックエンドは既読 (リロードでバッジ 0) なのにバルーンだけ残る」状態になる。 +describe('Notification badge: readAllNotifications event delivery', () => { + let carol: misskey.entities.SignupResponse; + let dave: misskey.entities.SignupResponse; + let redis: Redis.Redis; + + beforeAll(async () => { + carol = await signup({ username: 'carol_n17427' }); + dave = await signup({ username: 'dave_n17427' }); + // keyPrefix はサーバー側 (RedisService) と同じく config.redis.prefix (= url のホスト名) に合わせる + redis = new Redis.Redis({ host: '127.0.0.1', port: 56312, keyPrefix: 'misskey.local:' }); + }, 1000 * 60 * 2); + + afterAll(async () => { + redis.disconnect(); + }); + + beforeEach(async () => { + await api('notifications/flush', {}, carol); + await api('notifications/flush', {}, dave); + await setTimeout(500); + }); + + test('通常フロー: 通知タブを開く (i/notifications-grouped) と readAllNotifications が飛ぶ', async () => { + await post(dave, { text: `@carol_n17427 hi ${Date.now()}` }); + await setTimeout(2500); + + const before = await api('i', {}, carol); + assert.ok(before.body.unreadNotificationsCount >= 1, '前提: 未読が 1 件以上ある'); + + const fired = await waitFire( + carol, 'main', + () => api('i/notifications-grouped', {}, carol), + msg => msg.type === 'readAllNotifications', + ); + assert.strictEqual(fired, true, + '通知タブを開いたら mainStream に readAllNotifications が publish されるべき'); + + const after = await api('i', {}, carol); + assert.strictEqual(after.body.unreadNotificationsCount, 0); + }, 1000 * 30); + + test('フォロリク解決フロー: 表示 0 件でも通知タブを開けば readAllNotifications が飛ぶ', async () => { + await api('i/update', { isLocked: true }, carol); + await api('following/create', { userId: carol.id }, dave); + await setTimeout(2500); + + // 通知ページを開かずにフォローリクエストページから承認する + await api('following/requests/accept', { userId: dave.id }, carol); + await setTimeout(2500); + + const fired = await waitFire( + carol, 'main', + () => api('i/notifications-grouped', {}, carol), + msg => msg.type === 'readAllNotifications', + ); + assert.strictEqual(fired, true, + '表示対象が減っていても既読化イベントは publish されるべき'); + + const after = await api('i', {}, carol); + assert.strictEqual(after.body.unreadNotificationsCount, 0); + + await api('i/update', { isLocked: false }, carol); + await api('following/delete', { userId: carol.id }, dave); + }, 1000 * 30); + + test('同一ミリ秒に 10 件以上通知が積まれた場合でも readAllNotifications が飛ぶ (Stream ID の文字列比較の罠)', async () => { + // 実在する通知エントリの JSON を種として使うため、まず実通知を 2 件発生させる + await post(dave, { text: `@carol_n17427 seed1 ${Date.now()}` }); + await post(dave, { text: `@carol_n17427 seed2 ${Date.now()}` }); + await setTimeout(2500); + + const streamKey = `notificationTimeline:${carol.id}`; + const entries = await redis.xrange(streamKey, '-', '+'); + assert.ok(entries.length >= 2, '前提: Stream に実通知が 2 件以上ある'); + + const [json1, json2] = [entries[0][1][1], entries[1][1][1]]; + const lastId = entries[entries.length - 1][0]; + const baseMs = Number(lastId.split('-')[0]) + 1000; + + // 「同一 ms 内で seq=9 まで読み、その後同一 ms 内に seq=10 が積まれた」状態を再現する。 + // Redis Stream ID は数値比較では 9 < 10 だが、JS の文字列比較では '9' > '10'。 + await redis.xadd(streamKey, `${baseMs}-9`, 'data', json1); + await redis.xadd(streamKey, `${baseMs}-10`, 'data', json2); + await redis.set(`latestReadNotification:${carol.id}`, `${baseMs}-9`); + + const before = await api('i', {}, carol); + assert.strictEqual(before.body.unreadNotificationsCount, 1, + '前提: seq=10 のエントリ 1 件だけが未読 (Redis は数値比較で正しく認識する)'); + + const fired = await waitFire( + carol, 'main', + () => api('i/notifications-grouped', {}, carol), + msg => msg.type === 'readAllNotifications', + ); + assert.strictEqual(fired, true, + '未読が残っている以上、既読化時に readAllNotifications が publish されるべき'); + + const after = await api('i', {}, carol); + assert.strictEqual(after.body.unreadNotificationsCount, 0); + }, 1000 * 30); +}); diff --git a/packages/backend/test/unit/entities/UserEntityService.ts b/packages/backend/test/unit/entities/UserEntityService.ts index 1b5f9ed874c..094695907d6 100644 --- a/packages/backend/test/unit/entities/UserEntityService.ts +++ b/packages/backend/test/unit/entities/UserEntityService.ts @@ -23,7 +23,10 @@ import { DI } from '@/di-symbols.js'; import { AvatarDecorationService } from '@/core/AvatarDecorationService.js'; import { ApPersonService } from '@/core/activitypub/models/ApPersonService.js'; import { NoteEntityService } from '@/core/entities/NoteEntityService.js'; +import { NotificationEntityService } from '@/core/entities/NotificationEntityService.js'; import { PageEntityService } from '@/core/entities/PageEntityService.js'; +import { RoleEntityService } from '@/core/entities/RoleEntityService.js'; +import { ChatEntityService } from '@/core/entities/ChatEntityService.js'; import { CustomEmojiService } from '@/core/CustomEmojiService.js'; import { AnnouncementService } from '@/core/AnnouncementService.js'; import { RoleService } from '@/core/RoleService.js'; @@ -145,7 +148,10 @@ describe('UserEntityService', () => { UserEntityService, ApPersonService, NoteEntityService, + NotificationEntityService, PageEntityService, + RoleEntityService, + ChatEntityService, CustomEmojiService, AnnouncementService, RoleService, diff --git a/packages/misskey-js/src/autogen/types.ts b/packages/misskey-js/src/autogen/types.ts index 0f5db3e205e..4a86936e910 100644 --- a/packages/misskey-js/src/autogen/types.ts +++ b/packages/misskey-js/src/autogen/types.ts @@ -4163,6 +4163,7 @@ export type components = { hasUnreadChatMessages: boolean; hasUnreadNotification: boolean; hasPendingReceivedFollowRequest: boolean; + /** @description Saturates at 100. The actual number of unread notifications may be larger. */ unreadNotificationsCount: number; mutedWords: string[][]; hardMutedWords: string[][];