Fix: 通知のページネーションで２つ以上読み込めなくなることがある問題 (#15277)

* fix: notifications-groupedのinclude/exclude typesに:groupedを指定できてしまう問題

* refactor: 通知の取得処理を Notification Service に移動

* feat: add function to parse additional part of id

* fix: 通知のページネーションが正しく動かない問題

Redisにのページネーションで使用する時間及びidとRedis上のものが混同されていたので、Misskeyが生成するものに寄せました。

* pnpm run build-misskey-js-with-types

* chore: XADDをretryするように

* fix: notifications-groupedでxrevrangeしているのを消し忘れていた

packages/backend/src/core/IdService.ts

@@ -7,13 +7,13 @@ import { Inject, Injectable } from '@nestjs/common';
 import { ulid } from 'ulid';
 import { DI } from '@/di-symbols.js';
 import type { Config } from '@/config.js';
-import { genAid, isSafeAidT, parseAid } from '@/misc/id/aid.js';
-import { genAidx, isSafeAidxT, parseAidx } from '@/misc/id/aidx.js';
-import { genMeid, isSafeMeidT, parseMeid } from '@/misc/id/meid.js';
-import { genMeidg, isSafeMeidgT, parseMeidg } from '@/misc/id/meidg.js';
-import { genObjectId, isSafeObjectIdT, parseObjectId } from '@/misc/id/object-id.js';
+import { genAid, isSafeAidT, parseAid, parseAidFull } from '@/misc/id/aid.js';
+import { genAidx, isSafeAidxT, parseAidx, parseAidxFull } from '@/misc/id/aidx.js';
+import { genMeid, isSafeMeidT, parseMeid, parseMeidFull } from '@/misc/id/meid.js';
+import { genMeidg, isSafeMeidgT, parseMeidg, parseMeidgFull } from '@/misc/id/meidg.js';
+import { genObjectId, isSafeObjectIdT, parseObjectId, parseObjectIdFull } from '@/misc/id/object-id.js';
 import { bindThis } from '@/decorators.js';
-import { parseUlid } from '@/misc/id/ulid.js';
+import { parseUlid, parseUlidFull } from '@/misc/id/ulid.js';
 
 @Injectable()
 export class IdService {
@@ -70,4 +70,18 @@ export class IdService {
 			default: throw new Error('unrecognized id generation method');
 		}
 	}
+
+	// Note: additional is at most 64 bits
+	@bindThis
+	public parseFull(id: string): { date: number; additional: bigint; } {
+		switch (this.method) {
+			case 'aid': return parseAidFull(id);
+			case 'aidx': return parseAidxFull(id);
+			case 'objectid': return parseObjectIdFull(id);
+			case 'meid': return parseMeidFull(id);
+			case 'meidg': return parseMeidgFull(id);
+			case 'ulid': return parseUlidFull(id);
+			default: throw new Error('unrecognized id generation method');
+		}
+	}
 }

packages/backend/src/core/NotificationService.ts

@@ -7,6 +7,7 @@ import { setTimeout } from 'node:timers/promises';
 import * as Redis from 'ioredis';
 import { Inject, Injectable, OnApplicationShutdown } from '@nestjs/common';
 import { In } from 'typeorm';
+import { ReplyError } from 'ioredis';
 import { DI } from '@/di-symbols.js';
 import type { UsersRepository } from '@/models/_.js';
 import type { MiUser } from '@/models/User.js';
@@ -19,7 +20,7 @@ import { IdService } from '@/core/IdService.js';
 import { CacheService } from '@/core/CacheService.js';
 import type { Config } from '@/config.js';
 import { UserListService } from '@/core/UserListService.js';
-import type { FilterUnionByProperty } from '@/types.js';
+import { FilterUnionByProperty, groupedNotificationTypes, obsoleteNotificationTypes } from '@/types.js';
 import { trackPromise } from '@/misc/promise-tracker.js';
 
 @Injectable()
@@ -145,21 +146,36 @@ export class NotificationService implements OnApplicationShutdown {
 			}
 		}
 
-		const notification = {
-			id: this.idService.gen(),
-			createdAt: new Date(),
-			type: type,
-			...(notifierId ? {
-				notifierId,
-			} : {}),
-			...data,
-		} as any as FilterUnionByProperty<MiNotification, 'type', T>;
+		const createdAt = new Date();
+		let notification: FilterUnionByProperty<MiNotification, 'type', T>;
+		let redisId: string;
 
-		const redisIdPromise = this.redisClient.xadd(
-			`notificationTimeline:${notifieeId}`,
-			'MAXLEN', '~', this.config.perUserNotificationsMaxCount.toString(),
-			'*',
-			'data', JSON.stringify(notification));
+		do {
+			notification = {
+				id: this.idService.gen(),
+				createdAt,
+				type: type,
+				...(notifierId ? {
+					notifierId,
+				} : {}),
+				...data,
+			} as unknown as FilterUnionByProperty<MiNotification, 'type', T>;
+
+			try {
+				redisId = (await this.redisClient.xadd(
+					`notificationTimeline:${notifieeId}`,
+					'MAXLEN', '~', this.config.perUserNotificationsMaxCount.toString(),
+					this.toXListId(notification.id),
+					'data', JSON.stringify(notification)))!;
+			} catch (e) {
+				// The ID specified in XADD is equal or smaller than the target stream top item で失敗することがあるのでリトライ
+				if (e instanceof ReplyError) continue;
+				throw e;
+			}
+
+			break;
+			// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
+		} while (true);
 
 		const packed = await this.notificationEntityService.pack(notification, notifieeId, {});
 
@@ -173,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 >= (await redisIdPromise)!)) return;
+			if (latestReadNotificationId && (latestReadNotificationId >= redisId)) return;
 
 			this.globalEventService.publishMainStream(notifieeId, 'unreadNotification', packed);
 			this.pushNotificationService.pushNotification(notifieeId, 'notification', packed);
@@ -228,6 +244,79 @@ export class NotificationService implements OnApplicationShutdown {
 		this.#shutdownController.abort();
 	}
 
+	private toXListId(id: string): string {
+		const { date, additional } = this.idService.parseFull(id);
+		return date.toString() + '-' + additional.toString();
+	}
+
+	@bindThis
+	public async getNotifications(
+		userId: MiUser['id'],
+		{
+			sinceId,
+			untilId,
+			limit = 20,
+			includeTypes,
+			excludeTypes,
+		}: {
+			sinceId?: string,
+			untilId?: string,
+			limit?: number,
+			// any extra types are allowed, those are no-op
+			includeTypes?: (MiNotification['type'] | string)[],
+			excludeTypes?: (MiNotification['type'] | string)[],
+		},
+	): Promise<MiNotification[]> {
+		let sinceTime = sinceId ? this.toXListId(sinceId) : null;
+		let untilTime = untilId ? this.toXListId(untilId) : null;
+
+		let notifications: MiNotification[];
+		for (;;) {
+			let notificationsRes: [id: string, fields: string[]][];
+
+			// sinceidのみの場合は古い順、そうでない場合は新しい順。 QueryService.makePaginationQueryも参照
+			if (sinceTime && !untilTime) {
+				notificationsRes = await this.redisClient.xrange(
+					`notificationTimeline:${userId}`,
+					'(' + sinceTime,
+					'+',
+					'COUNT', limit);
+			} else {
+				notificationsRes = await this.redisClient.xrevrange(
+					`notificationTimeline:${userId}`,
+					untilTime ? '(' + untilTime : '+',
+					sinceTime ? '(' + sinceTime : '-',
+					'COUNT', limit);
+			}
+
+			if (notificationsRes.length === 0) {
+				return [];
+			}
+
+			notifications = notificationsRes.map(x => JSON.parse(x[1][1])) as MiNotification[];
+
+			if (includeTypes && includeTypes.length > 0) {
+				notifications = notifications.filter(notification => includeTypes.includes(notification.type));
+			} else if (excludeTypes && excludeTypes.length > 0) {
+				notifications = notifications.filter(notification => !excludeTypes.includes(notification.type));
+			}
+
+			if (notifications.length !== 0) {
+				// 通知が１件以上ある場合は返す
+				break;
+			}
+
+			// フィルタしたことで通知が0件になった場合、次のページを取得する
+			if (sinceId && !untilId) {
+				sinceTime = notificationsRes[notificationsRes.length - 1][0];
+			} else {
+				untilTime = notificationsRes[notificationsRes.length - 1][0];
+			}
+		}
+
+		return notifications;
+	}
+
 	@bindThis
 	public onApplicationShutdown(signal?: string | undefined): void {
 		this.dispose();

packages/backend/src/misc/bigint.ts

@@ -0,0 +1,40 @@
+/*
+ * SPDX-FileCopyrightText: syuilo and misskey-project
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+function parseBigIntChunked(str: string, base: number, chunkSize: number, powerOfChunkSize: bigint): bigint {
+	const chunks = [];
+	while (str.length > 0) {
+		chunks.unshift(str.slice(-chunkSize));
+		str = str.slice(0, -chunkSize);
+	}
+	let result = 0n;
+	for (const chunk of chunks) {
+		result *= powerOfChunkSize;
+		const int = parseInt(chunk, base);
+		if (Number.isNaN(int)) {
+			throw new Error('Invalid base36 string');
+		}
+		result += BigInt(int);
+	}
+	return result;
+}
+
+export function parseBigInt36(str: string): bigint {
+	// log_36(Number.MAX_SAFE_INTEGER) => 10.251599391715352
+	// so we process 10 chars at once
+	return parseBigIntChunked(str, 36, 10, 36n ** 10n);
+}
+
+export function parseBigInt16(str: string): bigint {
+	// log_16(Number.MAX_SAFE_INTEGER) => 13.25
+	// so we process 13 chars at once
+	return parseBigIntChunked(str, 16, 13, 16n ** 13n);
+}
+
+export function parseBigInt32(str: string): bigint {
+	// log_32(Number.MAX_SAFE_INTEGER) => 10.6
+	// so we process 10 chars at once
+	return parseBigIntChunked(str, 32, 10, 32n ** 10n);
+}

packages/backend/src/misc/id/aid.ts

@@ -7,6 +7,7 @@
 // 長さ8の[2000年1月1日からの経過ミリ秒をbase36でエンコードしたもの] + 長さ2の[ノイズ文字列]
 
 import * as crypto from 'node:crypto';
+import { parseBigInt36 } from '@/misc/bigint.js';
 
 export const aidRegExp = /^[0-9a-z]{10}$/;
 
@@ -35,6 +36,12 @@ export function parseAid(id: string): { date: Date; } {
 	return { date: new Date(time) };
 }
 
+export function parseAidFull(id: string): { date: number; additional: bigint; } {
+	const date = parseInt(id.slice(0, 8), 36) + TIME2000;
+	const additional = parseBigInt36(id.slice(8, 10));
+	return { date, additional };
+}
+
 export function isSafeAidT(t: number): boolean {
 	return t > TIME2000;
 }

packages/backend/src/misc/id/aidx.ts

@@ -9,6 +9,7 @@
 // https://misskey.m544.net/notes/71899acdcc9859ec5708ac24
 
 import { customAlphabet } from 'nanoid';
+import { parseBigInt36 } from '@/misc/bigint.js';
 
 export const aidxRegExp = /^[0-9a-z]{16}$/;
 
@@ -16,6 +17,7 @@ const TIME2000 = 946684800000;
 const TIME_LENGTH = 8;
 const NODE_LENGTH = 4;
 const NOISE_LENGTH = 4;
+const AIDX_LENGTH = TIME_LENGTH + NODE_LENGTH + NOISE_LENGTH;
 
 const nodeId = customAlphabet('0123456789abcdefghijklmnopqrstuvwxyz', NODE_LENGTH)();
 let counter = 0;
@@ -42,6 +44,12 @@ export function parseAidx(id: string): { date: Date; } {
 	return { date: new Date(time) };
 }
 
+export function parseAidxFull(id: string): { date: number; additional: bigint; } {
+	const date = parseInt(id.slice(0, TIME_LENGTH), 36) + TIME2000;
+	const additional = parseBigInt36(id.slice(TIME_LENGTH, AIDX_LENGTH));
+	return { date, additional };
+}
+
 export function isSafeAidxT(t: number): boolean {
 	return t > TIME2000;
 }

packages/backend/src/misc/id/meid.ts

@@ -3,6 +3,8 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 
+import { parseBigInt16 } from '@/misc/bigint.js';
+
 const CHARS = '0123456789abcdef';
 
 // same as object-id
@@ -39,6 +41,13 @@ export function parseMeid(id: string): { date: Date; } {
 	};
 }
 
+export function parseMeidFull(id: string): { date: number; additional: bigint; } {
+	return {
+		date: parseInt(id.slice(0, 12), 16) - 0x800000000000,
+		additional: parseBigInt16(id.slice(12, 24)),
+	};
+}
+
 export function isSafeMeidT(t: number): boolean {
 	return t > 0;
 }

packages/backend/src/misc/id/meidg.ts

@@ -3,6 +3,8 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 
+import { parseBigInt16 } from '@/misc/bigint.js';
+
 const CHARS = '0123456789abcdef';
 
 //  4bit Fixed hex value 'g'
@@ -39,6 +41,13 @@ export function parseMeidg(id: string): { date: Date; } {
 	};
 }
 
+export function parseMeidgFull(id: string): { date: number; additional: bigint; } {
+	return {
+		date: parseInt(id.slice(1, 12), 16),
+		additional: parseBigInt16(id.slice(12, 24)),
+	};
+}
+
 export function isSafeMeidgT(t: number): boolean {
 	return t > 0;
 }

packages/backend/src/misc/id/object-id.ts

@@ -3,6 +3,8 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 
+import { parseBigInt16 } from '@/misc/bigint.js';
+
 const CHARS = '0123456789abcdef';
 
 // same as meid
@@ -39,6 +41,13 @@ export function parseObjectId(id: string): { date: Date; } {
 	};
 }
 
+export function parseObjectIdFull(id: string): { date: number; additional: bigint; } {
+	return {
+		date: parseInt(id.slice(0, 8), 16) * 1000,
+		additional: parseBigInt16(id.slice(8, 24)),
+	};
+}
+
 export function isSafeObjectIdT(t: number): boolean {
 	return t > 0;
 }

packages/backend/src/misc/id/ulid.ts

@@ -5,15 +5,27 @@
 
 // Crockford's Base32
 // https://github.com/ulid/spec#encoding
+import { parseBigInt32 } from '@/misc/bigint.js';
+
 const CHARS = '0123456789ABCDEFGHJKMNPQRSTVWXYZ';
 
 export const ulidRegExp = /^[0123456789ABCDEFGHJKMNPQRSTVWXYZ]{26}$/;
 
-export function parseUlid(id: string): { date: Date; } {
-	const timestamp = id.slice(0, 10);
+function parseBase32(timestamp: string) {
 	let time = 0;
-	for (let i = 0; i < 10; i++) {
+	for (let i = 0; i < timestamp.length; i++) {
 		time = time * 32 + CHARS.indexOf(timestamp[i]);
 	}
-	return { date: new Date(time) };
+	return time;
+}
+
+export function parseUlid(id: string): { date: Date; } {
+	return { date: new Date(parseBase32(id.slice(0, 10))) };
+}
+
+export function parseUlidFull(id: string): { date: number; additional: bigint; } {
+	return {
+		date: parseBase32(id.slice(0, 10)),
+		additional: parseBigInt32(id.slice(10, 26)),
+	};
 }

packages/backend/src/server/api/endpoints/i/notifications-grouped.ts

@@ -7,7 +7,12 @@ import { In } from 'typeorm';
 import * as Redis from 'ioredis';
 import { Inject, Injectable } from '@nestjs/common';
 import type { NotesRepository } from '@/models/_.js';
-import { obsoleteNotificationTypes, groupedNotificationTypes, FilterUnionByProperty } from '@/types.js';
+import {
+	obsoleteNotificationTypes,
+	groupedNotificationTypes,
+	FilterUnionByProperty,
+	notificationTypes,
+} from '@/types.js';
 import { Endpoint } from '@/server/api/endpoint-base.js';
 import { NotificationEntityService } from '@/core/entities/NotificationEntityService.js';
 import { NotificationService } from '@/core/NotificationService.js';
@@ -47,10 +52,10 @@ export const paramDef = {
 		markAsRead: { type: 'boolean', default: true },
 		// 後方互換のため、廃止された通知タイプも受け付ける
 		includeTypes: { type: 'array', items: {
-			type: 'string', enum: [...groupedNotificationTypes, ...obsoleteNotificationTypes],
+			type: 'string', enum: [...notificationTypes, ...obsoleteNotificationTypes],
 		} },
 		excludeTypes: { type: 'array', items: {
-			type: 'string', enum: [...groupedNotificationTypes, ...obsoleteNotificationTypes],
+			type: 'string', enum: [...notificationTypes, ...obsoleteNotificationTypes],
 		} },
 	},
 	required: [],
@@ -74,31 +79,20 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 				return [];
 			}
 			// excludeTypes に全指定されている場合はクエリしない
-			if (groupedNotificationTypes.every(type => ps.excludeTypes?.includes(type))) {
+			if (notificationTypes.every(type => ps.excludeTypes?.includes(type))) {
 				return [];
 			}
 
 			const includeTypes = ps.includeTypes && ps.includeTypes.filter(type => !(obsoleteNotificationTypes).includes(type as any)) as typeof groupedNotificationTypes[number][];
 			const excludeTypes = ps.excludeTypes && ps.excludeTypes.filter(type => !(obsoleteNotificationTypes).includes(type as any)) as typeof groupedNotificationTypes[number][];
 
-			const limit = (ps.limit + EXTRA_LIMIT) + (ps.untilId ? 1 : 0) + (ps.sinceId ? 1 : 0); // untilIdに指定したものも含まれるため+1
-			const notificationsRes = await this.redisClient.xrevrange(
-				`notificationTimeline:${me.id}`,
-				ps.untilId ? this.idService.parse(ps.untilId).date.getTime() : '+',
-				ps.sinceId ? this.idService.parse(ps.sinceId).date.getTime() : '-',
-				'COUNT', limit);
-
-			if (notificationsRes.length === 0) {
-				return [];
-			}
-
-			let notifications = notificationsRes.map(x => JSON.parse(x[1][1])).filter(x => x.id !== ps.untilId && x !== ps.sinceId) as MiNotification[];
-
-			if (includeTypes && includeTypes.length > 0) {
-				notifications = notifications.filter(notification => includeTypes.includes(notification.type));
-			} else if (excludeTypes && excludeTypes.length > 0) {
-				notifications = notifications.filter(notification => !excludeTypes.includes(notification.type));
-			}
+			const notifications = await this.notificationService.getNotifications(me.id, {
+				sinceId: ps.sinceId,
+				untilId: ps.untilId,
+				limit: ps.limit,
+				includeTypes,
+				excludeTypes,
+			});
 
 			if (notifications.length === 0) {
 				return [];

packages/backend/src/server/api/endpoints/i/notifications.ts

@@ -82,52 +82,13 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 			const includeTypes = ps.includeTypes && ps.includeTypes.filter(type => !(obsoleteNotificationTypes).includes(type as any)) as typeof notificationTypes[number][];
 			const excludeTypes = ps.excludeTypes && ps.excludeTypes.filter(type => !(obsoleteNotificationTypes).includes(type as any)) as typeof notificationTypes[number][];
 
-			let sinceTime = ps.sinceId ? this.idService.parse(ps.sinceId).date.getTime().toString() : null;
-			let untilTime = ps.untilId ? this.idService.parse(ps.untilId).date.getTime().toString() : null;
-
-			let notifications: MiNotification[];
-			for (;;) {
-				let notificationsRes: [id: string, fields: string[]][];
-
-				// sinceidのみの場合は古い順、そうでない場合は新しい順。 QueryService.makePaginationQueryも参照
-				if (sinceTime && !untilTime) {
-					notificationsRes = await this.redisClient.xrange(
-						`notificationTimeline:${me.id}`,
-						'(' + sinceTime,
-						'+',
-						'COUNT', ps.limit);
-				} else {
-					notificationsRes = await this.redisClient.xrevrange(
-						`notificationTimeline:${me.id}`,
-						untilTime ? '(' + untilTime : '+',
-						sinceTime ? '(' + sinceTime : '-',
-						'COUNT', ps.limit);
-				}
-
-				if (notificationsRes.length === 0) {
-					return [];
-				}
-
-				notifications = notificationsRes.map(x => JSON.parse(x[1][1])) as MiNotification[];
-
-				if (includeTypes && includeTypes.length > 0) {
-					notifications = notifications.filter(notification => includeTypes.includes(notification.type));
-				} else if (excludeTypes && excludeTypes.length > 0) {
-					notifications = notifications.filter(notification => !excludeTypes.includes(notification.type));
-				}
-
-				if (notifications.length !== 0) {
-					// 通知が１件以上ある場合は返す
-					break;
-				}
-
-				// フィルタしたことで通知が0件になった場合、次のページを取得する
-				if (ps.sinceId && !ps.untilId) {
-					sinceTime = notificationsRes[notificationsRes.length - 1][0];
-				} else {
-					untilTime = notificationsRes[notificationsRes.length - 1][0];
-				}
-			}
+			const notifications = await this.notificationService.getNotifications(me.id, {
+				sinceId: ps.sinceId,
+				untilId: ps.untilId,
+				limit: ps.limit,
+				includeTypes,
+				excludeTypes,
+			});
 
 			// Mark all as read
 			if (ps.markAsRead) {

packages/misskey-js/src/autogen/types.ts

@@ -21732,8 +21732,8 @@ export type operations = {
           untilId?: string;
           /** @default true */
           markAsRead?: boolean;
-          includeTypes?: ('note' | 'follow' | 'mention' | 'reply' | 'renote' | 'quote' | 'reaction' | 'pollEnded' | 'receiveFollowRequest' | 'followRequestAccepted' | 'roleAssigned' | 'chatRoomInvitationReceived' | 'achievementEarned' | 'exportCompleted' | 'login' | 'createToken' | 'app' | 'test' | 'reaction:grouped' | 'renote:grouped' | 'pollVote' | 'groupInvited')[];
-          excludeTypes?: ('note' | 'follow' | 'mention' | 'reply' | 'renote' | 'quote' | 'reaction' | 'pollEnded' | 'receiveFollowRequest' | 'followRequestAccepted' | 'roleAssigned' | 'chatRoomInvitationReceived' | 'achievementEarned' | 'exportCompleted' | 'login' | 'createToken' | 'app' | 'test' | 'reaction:grouped' | 'renote:grouped' | 'pollVote' | 'groupInvited')[];
+          includeTypes?: ('note' | 'follow' | 'mention' | 'reply' | 'renote' | 'quote' | 'reaction' | 'pollEnded' | 'receiveFollowRequest' | 'followRequestAccepted' | 'roleAssigned' | 'chatRoomInvitationReceived' | 'achievementEarned' | 'exportCompleted' | 'login' | 'createToken' | 'app' | 'test' | 'pollVote' | 'groupInvited')[];
+          excludeTypes?: ('note' | 'follow' | 'mention' | 'reply' | 'renote' | 'quote' | 'reaction' | 'pollEnded' | 'receiveFollowRequest' | 'followRequestAccepted' | 'roleAssigned' | 'chatRoomInvitationReceived' | 'achievementEarned' | 'exportCompleted' | 'login' | 'createToken' | 'app' | 'test' | 'pollVote' | 'groupInvited')[];
         };
       };
     };