From 417bb2d31e5eaae08e0500a0525478606bfaea38 Mon Sep 17 00:00:00 2001 From: dakkar Date: Sun, 11 Feb 2024 12:21:56 +0000 Subject: [PATCH 01/50] Never return broken notifications #409 Since notifications are stored in Redis, we can't expect relational integrity: deleting a user will *not* delete notifications that mention it. But if we return notifications with missing bits (a `follow` without a `user`, for example), the frontend will get very confused and throw an exception while trying to render them. This change makes sure we never expose those broken notifications. For uniformity, I've applied the same logic to notes and roles mentioned in notifications, even if nobody reported breakage in those cases. Tested by creating a few types of notifications with a `notifierId`, then deleting their user. (cherry picked from commit 421f8d49e5d7a8dc3a798cc54716c767df8be3cb) --- .../entities/NotificationEntityService.ts | 75 +++++++++++++++---- 1 file changed, 59 insertions(+), 16 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 0663898edbd5..a572fe320cdd 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -64,21 +64,38 @@ export class NotificationEntityService implements OnModuleInit { packedNotes: Map>; packedUsers: Map>; }, - ): Promise> { + ): Promise | null> { const notification = src; - const noteIfNeed = NOTE_REQUIRED_NOTIFICATION_TYPES.has(notification.type) && 'noteId' in notification ? ( + const needsNote = NOTE_REQUIRED_NOTIFICATION_TYPES.has(notification.type) && 'noteId' in notification; + const noteIfNeed = needsNote ? ( hint?.packedNotes != null ? hint.packedNotes.get(notification.noteId) : this.noteEntityService.pack(notification.noteId, { id: meId }, { detail: true, }) ) : undefined; - const userIfNeed = 'notifierId' in notification ? ( + // if the note has been deleted, don't show this notification + if (needsNote && !noteIfNeed) { + return null; + } + + const needsUser = 'notifierId' in notification; + const userIfNeed = needsUser ? ( hint?.packedUsers != null ? hint.packedUsers.get(notification.notifierId) : this.userEntityService.pack(notification.notifierId, { id: meId }) ) : undefined; - const role = notification.type === 'roleAssigned' ? await this.roleEntityService.pack(notification.roleId) : undefined; + // if the user has been deleted, don't show this notification + if (needsUser && !userIfNeed) { + return null; + } + + const needsRole = notification.type === 'roleAssigned'; + const role = needsRole ? await this.roleEntityService.pack(notification.roleId) : undefined; + // if the role has been deleted, don't show this notification + if (needsRole && !role) { + return null; + } return await awaitAll({ id: notification.id, @@ -141,10 +158,10 @@ export class NotificationEntityService implements OnModuleInit { validNotifications = validNotifications.filter(x => (x.type !== 'receiveFollowRequest') || reqs.some(r => r.followerId === x.notifierId)); } - return await Promise.all(validNotifications.map(x => this.pack(x, meId, {}, { + return (await Promise.all(validNotifications.map(x => this.pack(x, meId, {}, { packedNotes, packedUsers, - }))); + })))).filter(n => n); } @bindThis @@ -159,23 +176,34 @@ export class NotificationEntityService implements OnModuleInit { packedNotes: Map>; packedUsers: Map>; }, - ): Promise> { + ): Promise | null> { const notification = src; - const noteIfNeed = NOTE_REQUIRED_GROUPED_NOTIFICATION_TYPES.has(notification.type) && 'noteId' in notification ? ( + const needsNote = NOTE_REQUIRED_GROUPED_NOTIFICATION_TYPES.has(notification.type) && 'noteId' in notification; + const noteIfNeed = needsNote ? ( hint?.packedNotes != null ? hint.packedNotes.get(notification.noteId) : this.noteEntityService.pack(notification.noteId, { id: meId }, { detail: true, }) ) : undefined; - const userIfNeed = 'notifierId' in notification ? ( + // if the note has been deleted, don't show this notification + if (needsNote && !noteIfNeed) { + return null; + } + + const needsUser = 'notifierId' in notification; + const userIfNeed = needsUser ? ( hint?.packedUsers != null ? hint.packedUsers.get(notification.notifierId) : this.userEntityService.pack(notification.notifierId, { id: meId }) ) : undefined; + // if the user has been deleted, don't show this notification + if (needsUser && !userIfNeed) { + return null; + } if (notification.type === 'reaction:grouped') { - const reactions = await Promise.all(notification.reactions.map(async reaction => { + const reactions = (await Promise.all(notification.reactions.map(async reaction => { const user = hint?.packedUsers != null ? hint.packedUsers.get(reaction.userId)! : await this.userEntityService.pack(reaction.userId, { id: meId }); @@ -183,7 +211,12 @@ export class NotificationEntityService implements OnModuleInit { user, reaction: reaction.reaction, }; - })); + }))).filter(r => r.user); + // if all users have been deleted, don't show this notification + if (!reactions.length) { + return null; + } + return await awaitAll({ id: notification.id, createdAt: new Date(notification.createdAt).toISOString(), @@ -192,14 +225,19 @@ export class NotificationEntityService implements OnModuleInit { reactions, }); } else if (notification.type === 'renote:grouped') { - const users = await Promise.all(notification.userIds.map(userId => { + const users = (await Promise.all(notification.userIds.map(userId => { const packedUser = hint?.packedUsers != null ? hint.packedUsers.get(userId) : null; if (packedUser) { return packedUser; } return this.userEntityService.pack(userId, { id: meId }); - })); + }))).filter(u => u); + // if all users have been deleted, don't show this notification + if (!users.length) { + return null; + } + return await awaitAll({ id: notification.id, createdAt: new Date(notification.createdAt).toISOString(), @@ -209,7 +247,12 @@ export class NotificationEntityService implements OnModuleInit { }); } - const role = notification.type === 'roleAssigned' ? await this.roleEntityService.pack(notification.roleId) : undefined; + const needsRole = notification.type === 'roleAssigned'; + const role = needsRole ? await this.roleEntityService.pack(notification.roleId) : undefined; + // if the role has been deleted, don't show this notification + if (needsRole && !role) { + return null; + } return await awaitAll({ id: notification.id, @@ -277,9 +320,9 @@ export class NotificationEntityService implements OnModuleInit { validNotifications = validNotifications.filter(x => (x.type !== 'receiveFollowRequest') || reqs.some(r => r.followerId === x.notifierId)); } - return await Promise.all(validNotifications.map(x => this.packGrouped(x, meId, {}, { + return (await Promise.all(validNotifications.map(x => this.packGrouped(x, meId, {}, { packedNotes, packedUsers, - }))); + })))).filter(n => n); } } From fae7cb20dce81a1e864d4d388323d4a036aea181 Mon Sep 17 00:00:00 2001 From: kakkokari-gtyih Date: Tue, 13 Feb 2024 15:23:49 +0900 Subject: [PATCH 02/50] Update Changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f41188a4da22..c545724424ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ - Enhance: モデレーターはすべてのユーザーのリアクション一覧を見られるように - Fix: 特定のキーワード及び正規表現にマッチする文字列を含むノートが投稿された際、エラーに出来るような設定項目を追加 #13207 * デフォルトは空欄なので適用前と同等の動作になります +- Fix: 破損した通知をクライアントに送信しないように + * 通知欄が無限にリロードされる問題が改善する可能性があります ### Client - Feat: 新しいゲームを追加 From 8d68c5078e4fdc39b2064ef73dd95f4ae27ae3b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=8B=E3=81=A3=E3=81=93=E3=81=8B=E3=82=8A?= <67428053+kakkokari-gtyih@users.noreply.github.com> Date: Mon, 19 Feb 2024 17:54:33 +0900 Subject: [PATCH 03/50] Update CHANGELOG.md --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68c6bce41cee..6d2070699419 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,9 @@ ### Server - Fix: nodeinfoにenableMcaptchaとenableTurnstileが無いのを修正 - +- Fix: 破損した通知をクライアントに送信しないように + * 通知欄が無限にリロードされる問題が改善する可能性があります + ## 2024.2.0 ### Note @@ -39,8 +41,6 @@ * すべてのリモートユーザーのリアクション一覧を見えないようにします - Fix: 特定のキーワード及び正規表現にマッチする文字列を含むノートが投稿された際、エラーに出来るような設定項目を追加 #13207 * デフォルトは空欄なので適用前と同等の動作になります -- Fix: 破損した通知をクライアントに送信しないように - * 通知欄が無限にリロードされる問題が改善する可能性があります ### Client - Feat: 新しいゲームを追加 From 35609b40d39f920ce79a007c7385a322a10d3a24 Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 00:40:33 +0900 Subject: [PATCH 04/50] =?UTF-8?q?enhance:=20=E9=80=9A=E7=9F=A5=E3=81=8C?= =?UTF-8?q?=E3=83=9F=E3=83=A5=E3=83=BC=E3=83=88=E3=82=92=E8=80=83=E6=85=AE?= =?UTF-8?q?=E3=81=99=E3=82=8B=E3=82=88=E3=81=86=E3=81=AB=E3=81=99=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../api/endpoints/i/notifications-grouped.ts | 30 ++++++++++++++++++- .../server/api/endpoints/i/notifications.ts | 30 ++++++++++++++++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts index 703808d2792a..8adf16294cef 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts @@ -6,7 +6,7 @@ import { Brackets, In } from 'typeorm'; import * as Redis from 'ioredis'; import { Inject, Injectable } from '@nestjs/common'; -import type { NotesRepository } from '@/models/_.js'; +import type { NotesRepository, UsersRepository } from '@/models/_.js'; import { obsoleteNotificationTypes, notificationTypes, FilterUnionByProperty } from '@/types.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; import { NoteReadService } from '@/core/NoteReadService.js'; @@ -15,6 +15,7 @@ import { NotificationService } from '@/core/NotificationService.js'; import { DI } from '@/di-symbols.js'; import { IdService } from '@/core/IdService.js'; import { MiGroupedNotification, MiNotification } from '@/models/Notification.js'; +import { CacheService } from '@/core/CacheService.js'; export const meta = { tags: ['account', 'notifications'], @@ -66,10 +67,14 @@ export default class extends Endpoint { // eslint- @Inject(DI.notesRepository) private notesRepository: NotesRepository, + @Inject(DI.usersRepository) + private usersRepository: UsersRepository, + private idService: IdService, private notificationEntityService: NotificationEntityService, private notificationService: NotificationService, private noteReadService: NoteReadService, + private cacheService: CacheService, ) { super(meta, paramDef, async (ps, me) => { const EXTRA_LIMIT = 100; @@ -105,6 +110,29 @@ export default class extends Endpoint { // eslint- notifications = notifications.filter(notification => !excludeTypes.includes(notification.type)); } + //#region Check muting + + const [ + userIdsWhoMeMuting, + userMutedInstances, + ] = await Promise.all([ + this.cacheService.userMutingsCache.fetch(me.id), + this.cacheService.userProfileCache.fetch(me.id).then(p => new Set(p.mutedInstances)), + ]); + + notifications = (await Promise.all(notifications.map(async (notification): Promise => { + if (!('notifierId' in notification)) return null; + if (userIdsWhoMeMuting.has(notification.notifierId)) return null; + + const notifier = await this.usersRepository.findOneBy({ id: notification.notifierId }); + if (notifier === null) return null; + if (notifier.host && userMutedInstances.has(notifier.host)) return null; + + return notification; + }))).filter((notification): notification is MiNotification => notification !== null); + + //#endregion Check muting + if (notifications.length === 0) { return []; } diff --git a/packages/backend/src/server/api/endpoints/i/notifications.ts b/packages/backend/src/server/api/endpoints/i/notifications.ts index 52b6749e3fa0..0ae8b89d2fce 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications.ts @@ -6,7 +6,7 @@ import { Brackets, In } from 'typeorm'; import * as Redis from 'ioredis'; import { Inject, Injectable } from '@nestjs/common'; -import type { NotesRepository } from '@/models/_.js'; +import type { NotesRepository, UsersRepository } from '@/models/_.js'; import { obsoleteNotificationTypes, notificationTypes, FilterUnionByProperty } from '@/types.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; import { NoteReadService } from '@/core/NoteReadService.js'; @@ -15,6 +15,7 @@ import { NotificationService } from '@/core/NotificationService.js'; import { DI } from '@/di-symbols.js'; import { IdService } from '@/core/IdService.js'; import { MiNotification } from '@/models/Notification.js'; +import { CacheService } from '@/core/CacheService.js'; export const meta = { tags: ['account', 'notifications'], @@ -66,10 +67,14 @@ export default class extends Endpoint { // eslint- @Inject(DI.notesRepository) private notesRepository: NotesRepository, + @Inject(DI.usersRepository) + private usersRepository: UsersRepository, + private idService: IdService, private notificationEntityService: NotificationEntityService, private notificationService: NotificationService, private noteReadService: NoteReadService, + private cacheService: CacheService, ) { super(meta, paramDef, async (ps, me) => { // includeTypes が空の場合はクエリしない @@ -103,6 +108,29 @@ export default class extends Endpoint { // eslint- notifications = notifications.filter(notification => !excludeTypes.includes(notification.type)); } + //#region Check muting + + const [ + userIdsWhoMeMuting, + userMutedInstances, + ] = await Promise.all([ + this.cacheService.userMutingsCache.fetch(me.id), + this.cacheService.userProfileCache.fetch(me.id).then(p => new Set(p.mutedInstances)), + ]); + + notifications = (await Promise.all(notifications.map(async (notification): Promise => { + if (!('notifierId' in notification)) return null; + if (userIdsWhoMeMuting.has(notification.notifierId)) return null; + + const notifier = await this.usersRepository.findOneBy({ id: notification.notifierId }); + if (notifier === null) return null; + if (notifier.host && userMutedInstances.has(notifier.host)) return null; + + return notification; + }))).filter((notification): notification is MiNotification => notification !== null); + + //#endregion Check muting + if (notifications.length === 0) { return []; } From b1e57e571dfd9a7d8b2430294473c2053cc3ea33 Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 00:52:25 +0900 Subject: [PATCH 05/50] =?UTF-8?q?enhance:=20=E9=80=9A=E7=9F=A5=E3=81=8C?= =?UTF-8?q?=E5=87=8D=E7=B5=90=E3=82=82=E8=80=83=E6=85=AE=E3=81=99=E3=82=8B?= =?UTF-8?q?=E3=82=88=E3=81=86=E3=81=AB=E3=81=99=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/server/api/endpoints/i/notifications-grouped.ts | 6 ++++-- .../backend/src/server/api/endpoints/i/notifications.ts | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts index 8adf16294cef..eafb1695ba27 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts @@ -110,7 +110,7 @@ export default class extends Endpoint { // eslint- notifications = notifications.filter(notification => !excludeTypes.includes(notification.type)); } - //#region Check muting + //#region Check muting & suspended const [ userIdsWhoMeMuting, @@ -128,10 +128,12 @@ export default class extends Endpoint { // eslint- if (notifier === null) return null; if (notifier.host && userMutedInstances.has(notifier.host)) return null; + if (notifier.isSuspended) return null; + return notification; }))).filter((notification): notification is MiNotification => notification !== null); - //#endregion Check muting + //#endregion Check muting & suspended if (notifications.length === 0) { return []; diff --git a/packages/backend/src/server/api/endpoints/i/notifications.ts b/packages/backend/src/server/api/endpoints/i/notifications.ts index 0ae8b89d2fce..3a2bc819f902 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications.ts @@ -108,7 +108,7 @@ export default class extends Endpoint { // eslint- notifications = notifications.filter(notification => !excludeTypes.includes(notification.type)); } - //#region Check muting + //#region Check muting & suspended const [ userIdsWhoMeMuting, @@ -126,10 +126,12 @@ export default class extends Endpoint { // eslint- if (notifier === null) return null; if (notifier.host && userMutedInstances.has(notifier.host)) return null; + if (notifier.isSuspended) return null; + return notification; }))).filter((notification): notification is MiNotification => notification !== null); - //#endregion Check muting + //#endregion Check muting & suspended if (notifications.length === 0) { return []; From 09a9484e4ffc8009864a6b5d0f6e55f9f1e1da1b Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 01:22:21 +0900 Subject: [PATCH 06/50] =?UTF-8?q?fix:=20notifierId=E3=81=8C=E3=81=AA?= =?UTF-8?q?=E3=81=84=E9=80=9A=E7=9F=A5=E3=81=8C=E6=B6=88=E3=81=88=E3=81=A6?= =?UTF-8?q?=E3=81=97=E3=81=BE=E3=81=86=E5=95=8F=E9=A1=8C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/backend/src/server/api/endpoints/i/notifications.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/server/api/endpoints/i/notifications.ts b/packages/backend/src/server/api/endpoints/i/notifications.ts index 3a2bc819f902..632b3eeffb71 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications.ts @@ -119,7 +119,7 @@ export default class extends Endpoint { // eslint- ]); notifications = (await Promise.all(notifications.map(async (notification): Promise => { - if (!('notifierId' in notification)) return null; + if (!('notifierId' in notification)) return notification; if (userIdsWhoMeMuting.has(notification.notifierId)) return null; const notifier = await this.usersRepository.findOneBy({ id: notification.notifierId }); From 094e10a278ff48bd9c466cbf1998bfcf9c0b4b67 Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 01:28:34 +0900 Subject: [PATCH 07/50] =?UTF-8?q?Add=20tests=20(=E9=80=9A=E7=9F=A5?= =?UTF-8?q?=E3=81=8C=E3=83=9F=E3=83=A5=E3=83=BC=E3=83=88=E3=82=92=E8=80=83?= =?UTF-8?q?=E6=85=AE=E3=81=97=E3=81=A6=E3=81=84=E3=82=8B=E3=81=8B=E3=81=A9?= =?UTF-8?q?=E3=81=86=E3=81=8B)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/backend/test/e2e/mute.ts | 179 ++++++++++++++++++++++++++++++ 1 file changed, 179 insertions(+) diff --git a/packages/backend/test/e2e/mute.ts b/packages/backend/test/e2e/mute.ts index e63067cd62ad..9cd6e2c8a2fc 100644 --- a/packages/backend/test/e2e/mute.ts +++ b/packages/backend/test/e2e/mute.ts @@ -117,5 +117,184 @@ describe('Mute', () => { assert.strictEqual(res.body.some((notification: any) => notification.userId === bob.id), true); assert.strictEqual(res.body.some((notification: any) => notification.userId === carol.id), false); }); + test('通知にミュートしているユーザーからのリプライが含まれない', async () => { + const aliceNote = await post(alice, { text: 'hi' }); + await post(bob, { text: '@alice hi', replyId: aliceNote.id }); + await post(carol, { text: '@alice hi', replyId: aliceNote.id }); + + const res = await api('/i/notifications', {}, alice); + + assert.strictEqual(res.status, 200); + assert.strictEqual(Array.isArray(res.body), true); + + assert.strictEqual(res.body.some((notification: any) => notification.userId === bob.id), true); + assert.strictEqual(res.body.some((notification: any) => notification.userId === carol.id), false); + }); + + test('通知にミュートしているユーザーからのリプライが含まれない', async () => { + await post(alice, { text: 'hi' }); + await post(bob, { text: '@alice hi' }); + await post(carol, { text: '@alice hi' }); + + const res = await api('/i/notifications', {}, alice); + + assert.strictEqual(res.status, 200); + assert.strictEqual(Array.isArray(res.body), true); + + assert.strictEqual(res.body.some((notification: any) => notification.userId === bob.id), true); + assert.strictEqual(res.body.some((notification: any) => notification.userId === carol.id), false); + }); + + test('通知にミュートしているユーザーからの引用リノートが含まれない', async () => { + const aliceNote = await post(alice, { text: 'hi' }); + await post(bob, { text: 'hi', renoteId: aliceNote.id }); + await post(carol, { text: 'hi', renoteId: aliceNote.id }); + + const res = await api('/i/notifications', {}, alice); + + assert.strictEqual(res.status, 200); + assert.strictEqual(Array.isArray(res.body), true); + + assert.strictEqual(res.body.some((notification: any) => notification.userId === bob.id), true); + assert.strictEqual(res.body.some((notification: any) => notification.userId === carol.id), false); + }); + + test('通知にミュートしているユーザーからのリノートが含まれない', async () => { + const aliceNote = await post(alice, { text: 'hi' }); + await post(bob, { renoteId: aliceNote.id }); + await post(carol, { renoteId: aliceNote.id }); + + const res = await api('/i/notifications', {}, alice); + + assert.strictEqual(res.status, 200); + assert.strictEqual(Array.isArray(res.body), true); + + assert.strictEqual(res.body.some((notification: any) => notification.userId === bob.id), true); + assert.strictEqual(res.body.some((notification: any) => notification.userId === carol.id), false); + }); + + test('通知にミュートしているユーザーからのフォロー通知が含まれない', async () => { + await api('/i/follow', { userId: alice.id }, bob); + await api('/i/follow', { userId: alice.id }, carol); + + const res = await api('/i/notifications', {}, alice); + + assert.strictEqual(res.status, 200); + assert.strictEqual(Array.isArray(res.body), true); + + assert.strictEqual(res.body.some((notification: any) => notification.userId === bob.id), true); + assert.strictEqual(res.body.some((notification: any) => notification.userId === carol.id), false); + }); + + test('通知にミュートしているユーザーからのフォローリクエストが含まれない', async () => { + await api('/i/update/', { isLocked: true }, alice); + await api('/following/create', { userId: alice.id }, bob); + await api('/following/create', { userId: alice.id }, carol); + + const res = await api('/i/notifications', {}, alice); + + assert.strictEqual(res.status, 200); + assert.strictEqual(Array.isArray(res.body), true); + + assert.strictEqual(res.body.some((notification: any) => notification.userId === bob.id), true); + assert.strictEqual(res.body.some((notification: any) => notification.userId === carol.id), false); + }); + }); + + describe('Notification (Grouped)', () => { + test('通知にミュートしているユーザーの通知が含まれない(リアクション)', async () => { + const aliceNote = await post(alice, { text: 'hi' }); + await react(bob, aliceNote, 'like'); + await react(carol, aliceNote, 'like'); + + const res = await api('/i/notifications-grouped-gropued', {}, alice); + + assert.strictEqual(res.status, 200); + assert.strictEqual(Array.isArray(res.body), true); + assert.strictEqual(res.body.some((notification: any) => notification.userId === bob.id), true); + assert.strictEqual(res.body.some((notification: any) => notification.userId === carol.id), false); + }); + test('通知にミュートしているユーザーからのリプライが含まれない', async () => { + const aliceNote = await post(alice, { text: 'hi' }); + await post(bob, { text: '@alice hi', replyId: aliceNote.id }); + await post(carol, { text: '@alice hi', replyId: aliceNote.id }); + + const res = await api('/i/notifications-grouped', {}, alice); + + assert.strictEqual(res.status, 200); + assert.strictEqual(Array.isArray(res.body), true); + + assert.strictEqual(res.body.some((notification: any) => notification.userId === bob.id), true); + assert.strictEqual(res.body.some((notification: any) => notification.userId === carol.id), false); + }); + + test('通知にミュートしているユーザーからのリプライが含まれない', async () => { + await post(alice, { text: 'hi' }); + await post(bob, { text: '@alice hi' }); + await post(carol, { text: '@alice hi' }); + + const res = await api('/i/notifications-grouped', {}, alice); + + assert.strictEqual(res.status, 200); + assert.strictEqual(Array.isArray(res.body), true); + + assert.strictEqual(res.body.some((notification: any) => notification.userId === bob.id), true); + assert.strictEqual(res.body.some((notification: any) => notification.userId === carol.id), false); + }); + + test('通知にミュートしているユーザーからの引用リノートが含まれない', async () => { + const aliceNote = await post(alice, { text: 'hi' }); + await post(bob, { text: 'hi', renoteId: aliceNote.id }); + await post(carol, { text: 'hi', renoteId: aliceNote.id }); + + const res = await api('/i/notifications-grouped', {}, alice); + + assert.strictEqual(res.status, 200); + assert.strictEqual(Array.isArray(res.body), true); + + assert.strictEqual(res.body.some((notification: any) => notification.userId === bob.id), true); + assert.strictEqual(res.body.some((notification: any) => notification.userId === carol.id), false); + }); + + test('通知にミュートしているユーザーからのリノートが含まれない', async () => { + const aliceNote = await post(alice, { text: 'hi' }); + await post(bob, { renoteId: aliceNote.id }); + await post(carol, { renoteId: aliceNote.id }); + + const res = await api('/i/notifications-grouped', {}, alice); + + assert.strictEqual(res.status, 200); + assert.strictEqual(Array.isArray(res.body), true); + + assert.strictEqual(res.body.some((notification: any) => notification.userId === bob.id), true); + assert.strictEqual(res.body.some((notification: any) => notification.userId === carol.id), false); + }); + + test('通知にミュートしているユーザーからのフォロー通知が含まれない', async () => { + await api('/i/follow', { userId: alice.id }, bob); + await api('/i/follow', { userId: alice.id }, carol); + + const res = await api('/i/notifications-grouped', {}, alice); + + assert.strictEqual(res.status, 200); + assert.strictEqual(Array.isArray(res.body), true); + + assert.strictEqual(res.body.some((notification: any) => notification.userId === bob.id), true); + assert.strictEqual(res.body.some((notification: any) => notification.userId === carol.id), false); + }); + + test('通知にミュートしているユーザーからのフォローリクエストが含まれない', async () => { + await api('/i/update/', { isLocked: true }, alice); + await api('/following/create', { userId: alice.id }, bob); + await api('/following/create', { userId: alice.id }, carol); + + const res = await api('/i/notifications-grouped', {}, alice); + + assert.strictEqual(res.status, 200); + assert.strictEqual(Array.isArray(res.body), true); + + assert.strictEqual(res.body.some((notification: any) => notification.userId === bob.id), true); + assert.strictEqual(res.body.some((notification: any) => notification.userId === carol.id), false); + }); }); }); From c21b6d95ae000b5178199950559f735629beacfb Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 01:30:14 +0900 Subject: [PATCH 08/50] =?UTF-8?q?fix:=20notifierId=E3=81=8C=E3=81=AA?= =?UTF-8?q?=E3=81=84=E9=80=9A=E7=9F=A5=E3=81=8C=E6=B6=88=E3=81=88=E3=81=A6?= =?UTF-8?q?=E3=81=97=E3=81=BE=E3=81=86=E5=95=8F=E9=A1=8C=20(grouped)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/server/api/endpoints/i/notifications-grouped.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts index eafb1695ba27..726e93b765cb 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts @@ -6,6 +6,7 @@ import { Brackets, In } from 'typeorm'; import * as Redis from 'ioredis'; import { Inject, Injectable } from '@nestjs/common'; +import { not } from 'ajv/dist/compile/codegen/index.js'; import type { NotesRepository, UsersRepository } from '@/models/_.js'; import { obsoleteNotificationTypes, notificationTypes, FilterUnionByProperty } from '@/types.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; @@ -121,7 +122,7 @@ export default class extends Endpoint { // eslint- ]); notifications = (await Promise.all(notifications.map(async (notification): Promise => { - if (!('notifierId' in notification)) return null; + if (!('notifierId' in notification)) return notification; if (userIdsWhoMeMuting.has(notification.notifierId)) return null; const notifier = await this.usersRepository.findOneBy({ id: notification.notifierId }); From c70c2e7534c94a2a5c353ef2bccc3b1eb5b3e791 Mon Sep 17 00:00:00 2001 From: taichan <40626578+tai-cha@users.noreply.github.com> Date: Tue, 20 Feb 2024 01:35:14 +0900 Subject: [PATCH 09/50] Remove unused import --- .../backend/src/server/api/endpoints/i/notifications-grouped.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts index 726e93b765cb..b58b7b6227de 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts @@ -6,7 +6,6 @@ import { Brackets, In } from 'typeorm'; import * as Redis from 'ioredis'; import { Inject, Injectable } from '@nestjs/common'; -import { not } from 'ajv/dist/compile/codegen/index.js'; import type { NotesRepository, UsersRepository } from '@/models/_.js'; import { obsoleteNotificationTypes, notificationTypes, FilterUnionByProperty } from '@/types.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; From 19296a0d6baaa2bedb9c8ea9bc7ee3d62f311f36 Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 09:57:21 +0900 Subject: [PATCH 10/50] Fix: typo --- packages/backend/test/e2e/mute.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/test/e2e/mute.ts b/packages/backend/test/e2e/mute.ts index 9cd6e2c8a2fc..1e4225184af4 100644 --- a/packages/backend/test/e2e/mute.ts +++ b/packages/backend/test/e2e/mute.ts @@ -207,7 +207,7 @@ describe('Mute', () => { await react(bob, aliceNote, 'like'); await react(carol, aliceNote, 'like'); - const res = await api('/i/notifications-grouped-gropued', {}, alice); + const res = await api('/i/notifications-grouped', {}, alice); assert.strictEqual(res.status, 200); assert.strictEqual(Array.isArray(res.body), true); From f0c1c08314b6e6c04431ec4c6b2592ac35d766bf Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 14:26:18 +0900 Subject: [PATCH 11/50] =?UTF-8?q?Revert=20"enhance:=20=E9=80=9A=E7=9F=A5?= =?UTF-8?q?=E3=81=8C=E5=87=8D=E7=B5=90=E3=82=82=E8=80=83=E6=85=AE=E3=81=99?= =?UTF-8?q?=E3=82=8B=E3=82=88=E3=81=86=E3=81=AB=E3=81=99=E3=82=8B"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit b1e57e571dfd9a7d8b2430294473c2053cc3ea33. --- .../src/server/api/endpoints/i/notifications-grouped.ts | 6 ++---- .../backend/src/server/api/endpoints/i/notifications.ts | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts index b58b7b6227de..d688c71cd5d9 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts @@ -110,7 +110,7 @@ export default class extends Endpoint { // eslint- notifications = notifications.filter(notification => !excludeTypes.includes(notification.type)); } - //#region Check muting & suspended + //#region Check muting const [ userIdsWhoMeMuting, @@ -128,12 +128,10 @@ export default class extends Endpoint { // eslint- if (notifier === null) return null; if (notifier.host && userMutedInstances.has(notifier.host)) return null; - if (notifier.isSuspended) return null; - return notification; }))).filter((notification): notification is MiNotification => notification !== null); - //#endregion Check muting & suspended + //#endregion Check muting if (notifications.length === 0) { return []; diff --git a/packages/backend/src/server/api/endpoints/i/notifications.ts b/packages/backend/src/server/api/endpoints/i/notifications.ts index 632b3eeffb71..3714bbeac0ed 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications.ts @@ -108,7 +108,7 @@ export default class extends Endpoint { // eslint- notifications = notifications.filter(notification => !excludeTypes.includes(notification.type)); } - //#region Check muting & suspended + //#region Check muting const [ userIdsWhoMeMuting, @@ -126,12 +126,10 @@ export default class extends Endpoint { // eslint- if (notifier === null) return null; if (notifier.host && userMutedInstances.has(notifier.host)) return null; - if (notifier.isSuspended) return null; - return notification; }))).filter((notification): notification is MiNotification => notification !== null); - //#endregion Check muting & suspended + //#endregion Check muting if (notifications.length === 0) { return []; From 1304a9fbb5be1bd07ad65cc4f90a4a003dde5192 Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 14:28:28 +0900 Subject: [PATCH 12/50] Revert API handling --- .../api/endpoints/i/notifications-grouped.ts | 23 ------------------- .../server/api/endpoints/i/notifications.ts | 23 ------------------- 2 files changed, 46 deletions(-) diff --git a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts index d688c71cd5d9..c465e759447d 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts @@ -110,29 +110,6 @@ export default class extends Endpoint { // eslint- notifications = notifications.filter(notification => !excludeTypes.includes(notification.type)); } - //#region Check muting - - const [ - userIdsWhoMeMuting, - userMutedInstances, - ] = await Promise.all([ - this.cacheService.userMutingsCache.fetch(me.id), - this.cacheService.userProfileCache.fetch(me.id).then(p => new Set(p.mutedInstances)), - ]); - - notifications = (await Promise.all(notifications.map(async (notification): Promise => { - if (!('notifierId' in notification)) return notification; - if (userIdsWhoMeMuting.has(notification.notifierId)) return null; - - const notifier = await this.usersRepository.findOneBy({ id: notification.notifierId }); - if (notifier === null) return null; - if (notifier.host && userMutedInstances.has(notifier.host)) return null; - - return notification; - }))).filter((notification): notification is MiNotification => notification !== null); - - //#endregion Check muting - if (notifications.length === 0) { return []; } diff --git a/packages/backend/src/server/api/endpoints/i/notifications.ts b/packages/backend/src/server/api/endpoints/i/notifications.ts index 3714bbeac0ed..c738d90509f4 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications.ts @@ -108,29 +108,6 @@ export default class extends Endpoint { // eslint- notifications = notifications.filter(notification => !excludeTypes.includes(notification.type)); } - //#region Check muting - - const [ - userIdsWhoMeMuting, - userMutedInstances, - ] = await Promise.all([ - this.cacheService.userMutingsCache.fetch(me.id), - this.cacheService.userProfileCache.fetch(me.id).then(p => new Set(p.mutedInstances)), - ]); - - notifications = (await Promise.all(notifications.map(async (notification): Promise => { - if (!('notifierId' in notification)) return notification; - if (userIdsWhoMeMuting.has(notification.notifierId)) return null; - - const notifier = await this.usersRepository.findOneBy({ id: notification.notifierId }); - if (notifier === null) return null; - if (notifier.host && userMutedInstances.has(notifier.host)) return null; - - return notification; - }))).filter((notification): notification is MiNotification => notification !== null); - - //#endregion Check muting - if (notifications.length === 0) { return []; } From 45a067788bd54c927f28689d5b7ecd313662e9c6 Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 14:32:00 +0900 Subject: [PATCH 13/50] Remove unused imports --- .../src/server/api/endpoints/i/notifications-grouped.ts | 7 +------ .../backend/src/server/api/endpoints/i/notifications.ts | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts index c465e759447d..703808d2792a 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts @@ -6,7 +6,7 @@ import { Brackets, In } from 'typeorm'; import * as Redis from 'ioredis'; import { Inject, Injectable } from '@nestjs/common'; -import type { NotesRepository, UsersRepository } from '@/models/_.js'; +import type { NotesRepository } from '@/models/_.js'; import { obsoleteNotificationTypes, notificationTypes, FilterUnionByProperty } from '@/types.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; import { NoteReadService } from '@/core/NoteReadService.js'; @@ -15,7 +15,6 @@ import { NotificationService } from '@/core/NotificationService.js'; import { DI } from '@/di-symbols.js'; import { IdService } from '@/core/IdService.js'; import { MiGroupedNotification, MiNotification } from '@/models/Notification.js'; -import { CacheService } from '@/core/CacheService.js'; export const meta = { tags: ['account', 'notifications'], @@ -67,14 +66,10 @@ export default class extends Endpoint { // eslint- @Inject(DI.notesRepository) private notesRepository: NotesRepository, - @Inject(DI.usersRepository) - private usersRepository: UsersRepository, - private idService: IdService, private notificationEntityService: NotificationEntityService, private notificationService: NotificationService, private noteReadService: NoteReadService, - private cacheService: CacheService, ) { super(meta, paramDef, async (ps, me) => { const EXTRA_LIMIT = 100; diff --git a/packages/backend/src/server/api/endpoints/i/notifications.ts b/packages/backend/src/server/api/endpoints/i/notifications.ts index c738d90509f4..52b6749e3fa0 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications.ts @@ -6,7 +6,7 @@ import { Brackets, In } from 'typeorm'; import * as Redis from 'ioredis'; import { Inject, Injectable } from '@nestjs/common'; -import type { NotesRepository, UsersRepository } from '@/models/_.js'; +import type { NotesRepository } from '@/models/_.js'; import { obsoleteNotificationTypes, notificationTypes, FilterUnionByProperty } from '@/types.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; import { NoteReadService } from '@/core/NoteReadService.js'; @@ -15,7 +15,6 @@ import { NotificationService } from '@/core/NotificationService.js'; import { DI } from '@/di-symbols.js'; import { IdService } from '@/core/IdService.js'; import { MiNotification } from '@/models/Notification.js'; -import { CacheService } from '@/core/CacheService.js'; export const meta = { tags: ['account', 'notifications'], @@ -67,14 +66,10 @@ export default class extends Endpoint { // eslint- @Inject(DI.notesRepository) private notesRepository: NotesRepository, - @Inject(DI.usersRepository) - private usersRepository: UsersRepository, - private idService: IdService, private notificationEntityService: NotificationEntityService, private notificationService: NotificationService, private noteReadService: NoteReadService, - private cacheService: CacheService, ) { super(meta, paramDef, async (ps, me) => { // includeTypes が空の場合はクエリしない From b09abb8c5953fecfd96ffba5da40397ed4dd7e2f Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 14:34:50 +0900 Subject: [PATCH 14/50] enhance: Check if notifierId is valid in NotificationEntityService --- .../entities/NotificationEntityService.ts | 75 ++++++++++++++++++- 1 file changed, 73 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 0663898edbd5..f6409914eb62 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -15,6 +15,7 @@ import type { Packed } from '@/misc/json-schema.js'; import { bindThis } from '@/decorators.js'; import { isNotNull } from '@/misc/is-not-null.js'; import { FilterUnionByProperty, notificationTypes } from '@/types.js'; +import { CacheService } from '@/core/CacheService.js'; import { RoleEntityService } from './RoleEntityService.js'; import type { OnModuleInit } from '@nestjs/common'; import type { UserEntityService } from './UserEntityService.js'; @@ -41,6 +42,8 @@ export class NotificationEntityService implements OnModuleInit { @Inject(DI.followRequestsRepository) private followRequestsRepository: FollowRequestsRepository, + private cacheService: CacheService, + //private userEntityService: UserEntityService, //private noteEntityService: NoteEntityService, ) { @@ -64,8 +67,11 @@ export class NotificationEntityService implements OnModuleInit { packedNotes: Map>; packedUsers: Map>; }, - ): Promise> { + ): Promise|null> { const notification = src; + + if (!(await this.#isValidNotifier(notification, meId))) return null; + const noteIfNeed = NOTE_REQUIRED_NOTIFICATION_TYPES.has(notification.type) && 'noteId' in notification ? ( hint?.packedNotes != null ? hint.packedNotes.get(notification.noteId) @@ -113,6 +119,8 @@ export class NotificationEntityService implements OnModuleInit { let validNotifications = notifications; + validNotifications = await this.#filterValidNotifier(validNotifications, meId); + const noteIds = validNotifications.map(x => 'noteId' in x ? x.noteId : null).filter(isNotNull); const notes = noteIds.length > 0 ? await this.notesRepository.find({ where: { id: In(noteIds) }, @@ -159,8 +167,11 @@ export class NotificationEntityService implements OnModuleInit { packedNotes: Map>; packedUsers: Map>; }, - ): Promise> { + ): Promise|null> { const notification = src; + + if (!(await this.#isValidNotifier(notification, meId))) return null; + const noteIfNeed = NOTE_REQUIRED_GROUPED_NOTIFICATION_TYPES.has(notification.type) && 'noteId' in notification ? ( hint?.packedNotes != null ? hint.packedNotes.get(notification.noteId) @@ -244,6 +255,8 @@ export class NotificationEntityService implements OnModuleInit { let validNotifications = notifications; + validNotifications = await this.#filterValidNotifier(validNotifications, meId); + const noteIds = validNotifications.map(x => 'noteId' in x ? x.noteId : null).filter(isNotNull); const notes = noteIds.length > 0 ? await this.notesRepository.find({ where: { id: In(noteIds) }, @@ -282,4 +295,62 @@ export class NotificationEntityService implements OnModuleInit { packedUsers, }))); } + + /** + * notifierが存在するか、ミュートされていないか、サスペンドされていないかを確認する + */ + async #isValidNotifier ( + notification: T, + meId: MiUser['id'], + ) : Promise { + const [ + userIdsWhoMeMuting, + userMutedInstances, + ] = await Promise.all([ + this.cacheService.userMutingsCache.fetch(meId), + this.cacheService.userProfileCache.fetch(meId).then(p => new Set(p.mutedInstances)), + ]); + + if (!('notifierId' in notification)) return true; + if (userIdsWhoMeMuting.has(notification.notifierId)) return false; + + const notifier = await this.usersRepository.findOneBy({ id: notification.notifierId }); + if (notifier === null) return false; + if (notifier.host && userMutedInstances.has(notifier.host)) return false; + + if (notifier.isSuspended) return false; + + return true; + } + + /** + * notifierが存在するか、ミュートされていないか、サスペンドされていないかを複数確認する + */ + async #filterValidNotifier ( + notifications: T[], + meId: MiUser['id'], + ) : Promise { + const [ + userIdsWhoMeMuting, + userMutedInstances, + ] = await Promise.all([ + this.cacheService.userMutingsCache.fetch(meId), + this.cacheService.userProfileCache.fetch(meId).then(p => new Set(p.mutedInstances)), + ]); + + const filteredNotifications = ((await Promise.all(notifications.map(async (notification) => { + if (!('notifierId' in notification)) return notification; + if (userIdsWhoMeMuting.has(notification.notifierId)) return null; + + const notifier = await this.usersRepository.findOneBy({ id: notification.notifierId }); + if (notifier === null) return null; + if (notifier.host && userMutedInstances.has(notifier.host)) return null; + + if (notifier.isSuspended) return null; + + return notification; + }))) as [T|null] ).filter((notification): notification is T => notification !== null); + + return filteredNotifications; + } } From ef6525214888f5d744bc9c6cc537c5c380215bfa Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 14:48:49 +0900 Subject: [PATCH 15/50] =?UTF-8?q?=E9=80=9A=E7=9F=A5=E4=BD=9C=E6=88=90?= =?UTF-8?q?=E6=99=82=E3=81=ABpack=E3=81=97=E3=81=A6null=E3=81=AB=E3=81=AA?= =?UTF-8?q?=E3=81=A3=E3=81=9F=E3=82=89=E3=81=82=E3=81=A8=E3=81=AE=E5=87=A6?= =?UTF-8?q?=E7=90=86=E3=82=92=E3=82=84=E3=82=81=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/backend/src/core/NotificationService.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/backend/src/core/NotificationService.ts b/packages/backend/src/core/NotificationService.ts index ee16193579fe..54aae4b47e1c 100644 --- a/packages/backend/src/core/NotificationService.ts +++ b/packages/backend/src/core/NotificationService.ts @@ -155,6 +155,8 @@ export class NotificationService implements OnApplicationShutdown { const packed = await this.notificationEntityService.pack(notification, notifieeId, {}); + if (packed === null) return null; + // Publish notification event this.globalEventService.publishMainStream(notifieeId, 'notification', packed); From 239a6952f717add53d52c3e701e7362eb1987645 Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 14:57:20 +0900 Subject: [PATCH 16/50] Remove duplication of valid notifier check --- .../entities/NotificationEntityService.ts | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index f6409914eb62..cbe93fc0c02b 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -119,8 +119,6 @@ export class NotificationEntityService implements OnModuleInit { let validNotifications = notifications; - validNotifications = await this.#filterValidNotifier(validNotifications, meId); - const noteIds = validNotifications.map(x => 'noteId' in x ? x.noteId : null).filter(isNotNull); const notes = noteIds.length > 0 ? await this.notesRepository.find({ where: { id: In(noteIds) }, @@ -255,8 +253,6 @@ export class NotificationEntityService implements OnModuleInit { let validNotifications = notifications; - validNotifications = await this.#filterValidNotifier(validNotifications, meId); - const noteIds = validNotifications.map(x => 'noteId' in x ? x.noteId : null).filter(isNotNull); const notes = noteIds.length > 0 ? await this.notesRepository.find({ where: { id: In(noteIds) }, @@ -322,35 +318,4 @@ export class NotificationEntityService implements OnModuleInit { return true; } - - /** - * notifierが存在するか、ミュートされていないか、サスペンドされていないかを複数確認する - */ - async #filterValidNotifier ( - notifications: T[], - meId: MiUser['id'], - ) : Promise { - const [ - userIdsWhoMeMuting, - userMutedInstances, - ] = await Promise.all([ - this.cacheService.userMutingsCache.fetch(meId), - this.cacheService.userProfileCache.fetch(meId).then(p => new Set(p.mutedInstances)), - ]); - - const filteredNotifications = ((await Promise.all(notifications.map(async (notification) => { - if (!('notifierId' in notification)) return notification; - if (userIdsWhoMeMuting.has(notification.notifierId)) return null; - - const notifier = await this.usersRepository.findOneBy({ id: notification.notifierId }); - if (notifier === null) return null; - if (notifier.host && userMutedInstances.has(notifier.host)) return null; - - if (notifier.isSuspended) return null; - - return notification; - }))) as [T|null] ).filter((notification): notification is T => notification !== null); - - return filteredNotifications; - } } From d53b837994b8256b2bcf1cb7dcabb07a7d6af837 Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 15:03:10 +0900 Subject: [PATCH 17/50] add filter notification is not null --- .../src/core/entities/NotificationEntityService.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index cbe93fc0c02b..ebfb9d9388c6 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -114,7 +114,7 @@ export class NotificationEntityService implements OnModuleInit { public async packMany( notifications: MiNotification[], meId: MiUser['id'], - ) { + ): Promise { if (notifications.length === 0) return []; let validNotifications = notifications; @@ -147,10 +147,10 @@ export class NotificationEntityService implements OnModuleInit { validNotifications = validNotifications.filter(x => (x.type !== 'receiveFollowRequest') || reqs.some(r => r.followerId === x.notifierId)); } - return await Promise.all(validNotifications.map(x => this.pack(x, meId, {}, { + return (await Promise.all(validNotifications.map(x => this.pack(x, meId, {}, { packedNotes, packedUsers, - }))); + })))).filter(isNotNull); } @bindThis @@ -248,7 +248,7 @@ export class NotificationEntityService implements OnModuleInit { public async packGroupedMany( notifications: MiGroupedNotification[], meId: MiUser['id'], - ) { + ) : Promise { if (notifications.length === 0) return []; let validNotifications = notifications; @@ -286,10 +286,10 @@ export class NotificationEntityService implements OnModuleInit { validNotifications = validNotifications.filter(x => (x.type !== 'receiveFollowRequest') || reqs.some(r => r.followerId === x.notifierId)); } - return await Promise.all(validNotifications.map(x => this.packGrouped(x, meId, {}, { + return (await Promise.all(validNotifications.map(x => this.packGrouped(x, meId, {}, { packedNotes, packedUsers, - }))); + })))).filter(isNotNull); } /** From ffb853bf2d861644f6b1c3f52f67588e3a0c6ee7 Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 15:18:26 +0900 Subject: [PATCH 18/50] Revert "Remove duplication of valid notifier check" This reverts commit 239a6952f717add53d52c3e701e7362eb1987645. --- .../entities/NotificationEntityService.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index ebfb9d9388c6..886a9e288d7e 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -119,6 +119,8 @@ export class NotificationEntityService implements OnModuleInit { let validNotifications = notifications; + validNotifications = await this.#filterValidNotifier(validNotifications, meId); + const noteIds = validNotifications.map(x => 'noteId' in x ? x.noteId : null).filter(isNotNull); const notes = noteIds.length > 0 ? await this.notesRepository.find({ where: { id: In(noteIds) }, @@ -253,6 +255,8 @@ export class NotificationEntityService implements OnModuleInit { let validNotifications = notifications; + validNotifications = await this.#filterValidNotifier(validNotifications, meId); + const noteIds = validNotifications.map(x => 'noteId' in x ? x.noteId : null).filter(isNotNull); const notes = noteIds.length > 0 ? await this.notesRepository.find({ where: { id: In(noteIds) }, @@ -318,4 +322,35 @@ export class NotificationEntityService implements OnModuleInit { return true; } + + /** + * notifierが存在するか、ミュートされていないか、サスペンドされていないかを複数確認する + */ + async #filterValidNotifier ( + notifications: T[], + meId: MiUser['id'], + ) : Promise { + const [ + userIdsWhoMeMuting, + userMutedInstances, + ] = await Promise.all([ + this.cacheService.userMutingsCache.fetch(meId), + this.cacheService.userProfileCache.fetch(meId).then(p => new Set(p.mutedInstances)), + ]); + + const filteredNotifications = ((await Promise.all(notifications.map(async (notification) => { + if (!('notifierId' in notification)) return notification; + if (userIdsWhoMeMuting.has(notification.notifierId)) return null; + + const notifier = await this.usersRepository.findOneBy({ id: notification.notifierId }); + if (notifier === null) return null; + if (notifier.host && userMutedInstances.has(notifier.host)) return null; + + if (notifier.isSuspended) return null; + + return notification; + }))) as [T|null] ).filter((notification): notification is T => notification !== null); + + return filteredNotifications; + } } From 3693ce9f00b3fbfecd76f7c1d33a9d4745b3619d Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 15:21:23 +0900 Subject: [PATCH 19/50] Improve performance --- .../src/core/entities/NotificationEntityService.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 886a9e288d7e..26287ea544ae 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -61,7 +61,7 @@ export class NotificationEntityService implements OnModuleInit { meId: MiUser['id'], // eslint-disable-next-line @typescript-eslint/ban-types options: { - + checkValidNotifier?: boolean; }, hint?: { packedNotes: Map>; @@ -70,7 +70,7 @@ export class NotificationEntityService implements OnModuleInit { ): Promise|null> { const notification = src; - if (!(await this.#isValidNotifier(notification, meId))) return null; + if (options.checkValidNotifier && !(await this.#isValidNotifier(notification, meId))) return null; const noteIfNeed = NOTE_REQUIRED_NOTIFICATION_TYPES.has(notification.type) && 'noteId' in notification ? ( hint?.packedNotes != null @@ -149,7 +149,7 @@ export class NotificationEntityService implements OnModuleInit { validNotifications = validNotifications.filter(x => (x.type !== 'receiveFollowRequest') || reqs.some(r => r.followerId === x.notifierId)); } - return (await Promise.all(validNotifications.map(x => this.pack(x, meId, {}, { + return (await Promise.all(validNotifications.map(x => this.pack(x, meId, { checkValidNotifier: false }, { packedNotes, packedUsers, })))).filter(isNotNull); @@ -290,7 +290,7 @@ export class NotificationEntityService implements OnModuleInit { validNotifications = validNotifications.filter(x => (x.type !== 'receiveFollowRequest') || reqs.some(r => r.followerId === x.notifierId)); } - return (await Promise.all(validNotifications.map(x => this.packGrouped(x, meId, {}, { + return (await Promise.all(validNotifications.map(x => this.packGrouped(x, meId, { checkValidNotifier: false }, { packedNotes, packedUsers, })))).filter(isNotNull); From ff7f7c88354217781389f9839adab751f434ec99 Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 15:22:59 +0900 Subject: [PATCH 20/50] Fix packGrouped --- .../backend/src/core/entities/NotificationEntityService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 26287ea544ae..8f74eb80247c 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -161,7 +161,7 @@ export class NotificationEntityService implements OnModuleInit { meId: MiUser['id'], // eslint-disable-next-line @typescript-eslint/ban-types options: { - + checkValidNotifier?: boolean; }, hint?: { packedNotes: Map>; @@ -170,7 +170,7 @@ export class NotificationEntityService implements OnModuleInit { ): Promise|null> { const notification = src; - if (!(await this.#isValidNotifier(notification, meId))) return null; + if ( options.checkValidNotifier && !(await this.#isValidNotifier(notification, meId))) return null; const noteIfNeed = NOTE_REQUIRED_GROUPED_NOTIFICATION_TYPES.has(notification.type) && 'noteId' in notification ? ( hint?.packedNotes != null From f5ae6630bdd0b4a6801a3407a9eb3c3de732c425 Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 15:36:30 +0900 Subject: [PATCH 21/50] =?UTF-8?q?Refactor:=20=E5=88=A4=E5=AE=9A=E9=83=A8?= =?UTF-8?q?=E5=88=86=E3=82=92=E5=85=B1=E9=80=9A=E5=8C=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../entities/NotificationEntityService.ts | 48 ++++++++++--------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 8f74eb80247c..16451790dde0 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -297,20 +297,14 @@ export class NotificationEntityService implements OnModuleInit { } /** - * notifierが存在するか、ミュートされていないか、サスペンドされていないかを確認する + * notifierが存在するか、ミュートされていないか、サスペンドされていないかを確認するvalidator */ - async #isValidNotifier ( + async #validateNotifier ( notification: T, meId: MiUser['id'], - ) : Promise { - const [ - userIdsWhoMeMuting, - userMutedInstances, - ] = await Promise.all([ - this.cacheService.userMutingsCache.fetch(meId), - this.cacheService.userProfileCache.fetch(meId).then(p => new Set(p.mutedInstances)), - ]); - + userIdsWhoMeMuting: Set, + userMutedInstances: Set, + ): Promise { if (!('notifierId' in notification)) return true; if (userIdsWhoMeMuting.has(notification.notifierId)) return false; @@ -324,7 +318,25 @@ export class NotificationEntityService implements OnModuleInit { } /** - * notifierが存在するか、ミュートされていないか、サスペンドされていないかを複数確認する + * notifierが存在するか、ミュートされていないか、サスペンドされていないかを実際に確認する + */ + async #isValidNotifier ( + notification: T, + meId: MiUser['id'], + ) : Promise { + const [ + userIdsWhoMeMuting, + userMutedInstances, + ] = await Promise.all([ + this.cacheService.userMutingsCache.fetch(meId), + this.cacheService.userProfileCache.fetch(meId).then(p => new Set(p.mutedInstances)), + ]); + + return this.#validateNotifier(notification, meId, userIdsWhoMeMuting, userMutedInstances); + } + + /** + * notifierが存在するか、ミュートされていないか、サスペンドされていないかを実際に複数確認する */ async #filterValidNotifier ( notifications: T[], @@ -339,16 +351,8 @@ export class NotificationEntityService implements OnModuleInit { ]); const filteredNotifications = ((await Promise.all(notifications.map(async (notification) => { - if (!('notifierId' in notification)) return notification; - if (userIdsWhoMeMuting.has(notification.notifierId)) return null; - - const notifier = await this.usersRepository.findOneBy({ id: notification.notifierId }); - if (notifier === null) return null; - if (notifier.host && userMutedInstances.has(notifier.host)) return null; - - if (notifier.isSuspended) return null; - - return notification; + const isValid = await this.#validateNotifier(notification, meId, userIdsWhoMeMuting, userMutedInstances); + return isValid ? notification : null; }))) as [T|null] ).filter((notification): notification is T => notification !== null); return filteredNotifications; From 823b7c3269067c02504277207a9bfd087ec01c92 Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 15:40:36 +0900 Subject: [PATCH 22/50] Fix condition --- .../backend/src/core/entities/NotificationEntityService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 16451790dde0..24ae0bdf3e47 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -70,7 +70,7 @@ export class NotificationEntityService implements OnModuleInit { ): Promise|null> { const notification = src; - if (options.checkValidNotifier && !(await this.#isValidNotifier(notification, meId))) return null; + if (options.checkValidNotifier !== false && !(await this.#isValidNotifier(notification, meId))) return null; const noteIfNeed = NOTE_REQUIRED_NOTIFICATION_TYPES.has(notification.type) && 'noteId' in notification ? ( hint?.packedNotes != null @@ -170,7 +170,7 @@ export class NotificationEntityService implements OnModuleInit { ): Promise|null> { const notification = src; - if ( options.checkValidNotifier && !(await this.#isValidNotifier(notification, meId))) return null; + if ( options.checkValidNotifier !== false && !(await this.#isValidNotifier(notification, meId))) return null; const noteIfNeed = NOTE_REQUIRED_GROUPED_NOTIFICATION_TYPES.has(notification.type) && 'noteId' in notification ? ( hint?.packedNotes != null From 7c9bacbbd824a850b2d1061b3476e24806375a23 Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 15:44:13 +0900 Subject: [PATCH 23/50] use isNotNull --- packages/backend/src/core/entities/NotificationEntityService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 24ae0bdf3e47..824f0b7fdafa 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -353,7 +353,7 @@ export class NotificationEntityService implements OnModuleInit { const filteredNotifications = ((await Promise.all(notifications.map(async (notification) => { const isValid = await this.#validateNotifier(notification, meId, userIdsWhoMeMuting, userMutedInstances); return isValid ? notification : null; - }))) as [T|null] ).filter((notification): notification is T => notification !== null); + }))) as [T|null] ).filter(isNotNull); return filteredNotifications; } From 659f8d9d20f0e9ae09acf5db97f106a78cd76ae3 Mon Sep 17 00:00:00 2001 From: taichan Date: Tue, 20 Feb 2024 15:56:28 +0900 Subject: [PATCH 24/50] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77a03ff68434..51ec31397070 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ ## 202x.x.x (unreleased) ### General +- 通知がミュート、凍結を考慮するようになりました ### Client - Enhance: ノート作成画面のファイル添付メニューの区切り線の位置を調整 From 0cf8946fc7ece830d4c9a136fdc18bebada5a183 Mon Sep 17 00:00:00 2001 From: taichanne30 Date: Wed, 21 Feb 2024 03:15:08 +0900 Subject: [PATCH 25/50] =?UTF-8?q?filter=E3=81=AE=E6=94=B9=E5=96=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../backend/src/core/entities/NotificationEntityService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 48ea78289328..080d0c9f26cd 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -222,7 +222,7 @@ export class NotificationEntityService implements OnModuleInit { user, reaction: reaction.reaction, }; - }))).filter(r => r.user); + }))).filter(r => isNotNull(r.user)); // if all users have been deleted, don't show this notification if (!reactions.length) { return null; @@ -243,7 +243,7 @@ export class NotificationEntityService implements OnModuleInit { } return this.userEntityService.pack(userId, { id: meId }); - }))).filter(u => u); + }))).filter(isNotNull); // if all users have been deleted, don't show this notification if (!users.length) { return null; From 5dae5ba3b46839f33d353d9ba7e49f982da3e623 Mon Sep 17 00:00:00 2001 From: taichanne30 Date: Wed, 21 Feb 2024 04:19:31 +0900 Subject: [PATCH 26/50] =?UTF-8?q?Refactor:=20DONT=20REPEAT=20YOURSELF=20No?= =?UTF-8?q?te:=20GroupedNotification=E3=81=AFNotification=E3=81=AE?= =?UTF-8?q?=E6=8B=A1=E5=BC=B5=E3=81=AA=E3=81=AE=E3=81=A7=E3=81=9D=E3=81=AE?= =?UTF-8?q?=E4=BE=8B=E5=A4=96=E3=81=A0=E3=81=91=E6=9B=B8=E3=81=91=E3=81=B0?= =?UTF-8?q?=E5=9F=BA=E6=9C=AC=E7=9A=84=E3=81=AB=E5=85=B1=E9=80=9A=E3=81=AE?= =?UTF-8?q?=E5=87=A6=E7=90=86=E3=81=AB=E3=81=AA=E3=82=8A=E8=A4=87=E9=9B=91?= =?UTF-8?q?=E3=81=AA=E5=80=8B=E5=88=A5=E3=81=AE=E5=87=A6=E7=90=86=E3=81=AF?= =?UTF-8?q?=E5=A2=97=E3=81=88=E3=81=AB=E3=81=8F=E3=81=84=E3=81=A8=E6=80=9D?= =?UTF-8?q?=E3=82=8F=E3=82=8C=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../entities/NotificationEntityService.ts | 194 ++++++------------ packages/backend/src/types.ts | 6 +- 2 files changed, 68 insertions(+), 132 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 080d0c9f26cd..3c5dc24b424e 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -21,8 +21,7 @@ import type { OnModuleInit } from '@nestjs/common'; import type { UserEntityService } from './UserEntityService.js'; import type { NoteEntityService } from './NoteEntityService.js'; -const NOTE_REQUIRED_NOTIFICATION_TYPES = new Set(['note', 'mention', 'reply', 'renote', 'quote', 'reaction', 'pollEnded'] as (typeof notificationTypes[number])[]); -const NOTE_REQUIRED_GROUPED_NOTIFICATION_TYPES = new Set(['note', 'mention', 'reply', 'renote', 'renote:grouped', 'quote', 'reaction', 'reaction:grouped', 'pollEnded']); +const NOTE_REQUIRED_NOTIFICATION_TYPES = new Set(['note', 'mention', 'reply', 'renote', 'renote:grouped', 'quote', 'reaction', 'reaction:grouped', 'pollEnded'] as (typeof notificationTypes[number])[]); @Injectable() export class NotificationEntityService implements OnModuleInit { @@ -55,9 +54,11 @@ export class NotificationEntityService implements OnModuleInit { this.roleEntityService = this.moduleRef.get('RoleEntityService'); } - @bindThis - public async pack( - src: MiNotification, + /** + * 通知をパックする共通処理 + */ + async #packInternal ( + src: T, meId: MiUser['id'], // eslint-disable-next-line @typescript-eslint/ban-types options: { @@ -67,7 +68,7 @@ export class NotificationEntityService implements OnModuleInit { packedNotes: Map>; packedUsers: Map>; }, - ): Promise | null> { + ) : Promise | null> { const notification = src; if (options.checkValidNotifier !== false && !(await this.#isValidNotifier(notification, meId))) return null; @@ -96,123 +97,7 @@ export class NotificationEntityService implements OnModuleInit { return null; } - const needsRole = notification.type === 'roleAssigned'; - const role = needsRole ? await this.roleEntityService.pack(notification.roleId) : undefined; - // if the role has been deleted, don't show this notification - if (needsRole && !role) { - return null; - } - - return await awaitAll({ - id: notification.id, - createdAt: new Date(notification.createdAt).toISOString(), - type: notification.type, - userId: 'notifierId' in notification ? notification.notifierId : undefined, - ...(userIfNeed != null ? { user: userIfNeed } : {}), - ...(noteIfNeed != null ? { note: noteIfNeed } : {}), - ...(notification.type === 'reaction' ? { - reaction: notification.reaction, - } : {}), - ...(notification.type === 'roleAssigned' ? { - role: role, - } : {}), - ...(notification.type === 'achievementEarned' ? { - achievement: notification.achievement, - } : {}), - ...(notification.type === 'app' ? { - body: notification.customBody, - header: notification.customHeader, - icon: notification.customIcon, - } : {}), - }); - } - - @bindThis - public async packMany( - notifications: MiNotification[], - meId: MiUser['id'], - ): Promise { - if (notifications.length === 0) return []; - - let validNotifications = notifications; - - validNotifications = await this.#filterValidNotifier(validNotifications, meId); - - const noteIds = validNotifications.map(x => 'noteId' in x ? x.noteId : null).filter(isNotNull); - const notes = noteIds.length > 0 ? await this.notesRepository.find({ - where: { id: In(noteIds) }, - relations: ['user', 'reply', 'reply.user', 'renote', 'renote.user'], - }) : []; - const packedNotesArray = await this.noteEntityService.packMany(notes, { id: meId }, { - detail: true, - }); - const packedNotes = new Map(packedNotesArray.map(p => [p.id, p])); - - validNotifications = validNotifications.filter(x => !('noteId' in x) || packedNotes.has(x.noteId)); - - const userIds = validNotifications.map(x => 'notifierId' in x ? x.notifierId : null).filter(isNotNull); - const users = userIds.length > 0 ? await this.usersRepository.find({ - where: { id: In(userIds) }, - }) : []; - const packedUsersArray = await this.userEntityService.packMany(users, { id: meId }); - const packedUsers = new Map(packedUsersArray.map(p => [p.id, p])); - - // 既に解決されたフォローリクエストの通知を除外 - const followRequestNotifications = validNotifications.filter((x): x is FilterUnionByProperty => x.type === 'receiveFollowRequest'); - if (followRequestNotifications.length > 0) { - const reqs = await this.followRequestsRepository.find({ - where: { followerId: In(followRequestNotifications.map(x => x.notifierId)) }, - }); - validNotifications = validNotifications.filter(x => (x.type !== 'receiveFollowRequest') || reqs.some(r => r.followerId === x.notifierId)); - } - - return (await Promise.all(validNotifications.map(x => this.pack(x, meId, { checkValidNotifier: false }, { - packedNotes, - packedUsers, - })))).filter(isNotNull); - } - - @bindThis - public async packGrouped( - src: MiGroupedNotification, - meId: MiUser['id'], - // eslint-disable-next-line @typescript-eslint/ban-types - options: { - checkValidNotifier?: boolean; - }, - hint?: { - packedNotes: Map>; - packedUsers: Map>; - }, - ): Promise | null> { - const notification = src; - - if ( options.checkValidNotifier !== false && !(await this.#isValidNotifier(notification, meId))) return null; - - const needsNote = NOTE_REQUIRED_GROUPED_NOTIFICATION_TYPES.has(notification.type) && 'noteId' in notification; - const noteIfNeed = needsNote ? ( - hint?.packedNotes != null - ? hint.packedNotes.get(notification.noteId) - : this.noteEntityService.pack(notification.noteId, { id: meId }, { - detail: true, - }) - ) : undefined; - // if the note has been deleted, don't show this notification - if (needsNote && !noteIfNeed) { - return null; - } - - const needsUser = 'notifierId' in notification; - const userIfNeed = needsUser ? ( - hint?.packedUsers != null - ? hint.packedUsers.get(notification.notifierId) - : this.userEntityService.pack(notification.notifierId, { id: meId }) - ) : undefined; - // if the user has been deleted, don't show this notification - if (needsUser && !userIfNeed) { - return null; - } - + // #region Grouped notifications if (notification.type === 'reaction:grouped') { const reactions = (await Promise.all(notification.reactions.map(async reaction => { const user = hint?.packedUsers != null @@ -257,6 +142,7 @@ export class NotificationEntityService implements OnModuleInit { users, }); } + // #endregion const needsRole = notification.type === 'roleAssigned'; const role = needsRole ? await this.roleEntityService.pack(notification.roleId) : undefined; @@ -289,11 +175,10 @@ export class NotificationEntityService implements OnModuleInit { }); } - @bindThis - public async packGroupedMany( - notifications: MiGroupedNotification[], + async #packManyInternal ( + notifications: T[], meId: MiUser['id'], - ) : Promise { + ): Promise { if (notifications.length === 0) return []; let validNotifications = notifications; @@ -325,7 +210,7 @@ export class NotificationEntityService implements OnModuleInit { const packedUsers = new Map(packedUsersArray.map(p => [p.id, p])); // 既に解決されたフォローリクエストの通知を除外 - const followRequestNotifications = validNotifications.filter((x): x is FilterUnionByProperty => x.type === 'receiveFollowRequest'); + const followRequestNotifications = validNotifications.filter((x): x is FilterUnionByProperty => x.type === 'receiveFollowRequest'); if (followRequestNotifications.length > 0) { const reqs = await this.followRequestsRepository.find({ where: { followerId: In(followRequestNotifications.map(x => x.notifierId)) }, @@ -339,12 +224,59 @@ export class NotificationEntityService implements OnModuleInit { })))).filter(isNotNull); } + @bindThis + public async pack( + src: MiNotification, + meId: MiUser['id'], + // eslint-disable-next-line @typescript-eslint/ban-types + options: { + checkValidNotifier?: boolean; + }, + hint?: { + packedNotes: Map>; + packedUsers: Map>; + }, + ): Promise | null> { + return this.#packInternal(src, meId, options, hint); + } + + @bindThis + public async packMany( + notifications: MiNotification[], + meId: MiUser['id'], + ): Promise { + return await this.#packManyInternal(notifications, meId); + } + + @bindThis + public async packGrouped( + src: MiGroupedNotification, + meId: MiUser['id'], + // eslint-disable-next-line @typescript-eslint/ban-types + options: { + checkValidNotifier?: boolean; + }, + hint?: { + packedNotes: Map>; + packedUsers: Map>; + }, + ): Promise | null> { + return await this.#packInternal(src, meId, options, hint); + } + + @bindThis + public async packGroupedMany( + notifications: MiGroupedNotification[], + meId: MiUser['id'], + ) : Promise { + return await this.#packManyInternal(notifications, meId); + } + /** * notifierが存在するか、ミュートされていないか、サスペンドされていないかを確認するvalidator */ async #validateNotifier ( notification: T, - meId: MiUser['id'], userIdsWhoMeMuting: Set, userMutedInstances: Set, ): Promise { @@ -375,7 +307,7 @@ export class NotificationEntityService implements OnModuleInit { this.cacheService.userProfileCache.fetch(meId).then(p => new Set(p.mutedInstances)), ]); - return this.#validateNotifier(notification, meId, userIdsWhoMeMuting, userMutedInstances); + return this.#validateNotifier(notification, userIdsWhoMeMuting, userMutedInstances); } /** @@ -394,7 +326,7 @@ export class NotificationEntityService implements OnModuleInit { ]); const filteredNotifications = ((await Promise.all(notifications.map(async (notification) => { - const isValid = await this.#validateNotifier(notification, meId, userIdsWhoMeMuting, userMutedInstances); + const isValid = await this.#validateNotifier(notification, userIdsWhoMeMuting, userMutedInstances); return isValid ? notification : null; }))) as [T|null] ).filter(isNotNull); diff --git a/packages/backend/src/types.ts b/packages/backend/src/types.ts index fdcd2c062950..633547419acb 100644 --- a/packages/backend/src/types.ts +++ b/packages/backend/src/types.ts @@ -18,6 +18,7 @@ * achievementEarned - 実績を獲得 * app - アプリ通知 * test - テスト通知（サーバー側） + * */ export const notificationTypes = [ 'note', @@ -33,7 +34,10 @@ export const notificationTypes = [ 'roleAssigned', 'achievementEarned', 'app', - 'test'] as const; + 'test', + 'reaction:grouped', + 'renote:grouped', +] as const; export const obsoleteNotificationTypes = ['pollVote', 'groupInvited'] as const; export const noteVisibilities = ['public', 'home', 'followers', 'specified'] as const; From f9856368d3589cd08ed0f61a2e7f41ffba3bffd3 Mon Sep 17 00:00:00 2001 From: taichanne30 Date: Wed, 21 Feb 2024 04:33:21 +0900 Subject: [PATCH 27/50] Add groupedNotificationTypes --- .../src/core/entities/NotificationEntityService.ts | 4 ++-- .../server/api/endpoints/i/notifications-grouped.ts | 12 ++++++------ packages/backend/src/types.ts | 5 +++++ 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 3c5dc24b424e..e2a6ab6e8e34 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -14,14 +14,14 @@ import type { MiNote } from '@/models/Note.js'; import type { Packed } from '@/misc/json-schema.js'; import { bindThis } from '@/decorators.js'; import { isNotNull } from '@/misc/is-not-null.js'; -import { FilterUnionByProperty, notificationTypes } from '@/types.js'; +import { FilterUnionByProperty, groupedNotificationTypes } from '@/types.js'; import { CacheService } from '@/core/CacheService.js'; import { RoleEntityService } from './RoleEntityService.js'; import type { OnModuleInit } from '@nestjs/common'; import type { UserEntityService } from './UserEntityService.js'; import type { NoteEntityService } from './NoteEntityService.js'; -const NOTE_REQUIRED_NOTIFICATION_TYPES = new Set(['note', 'mention', 'reply', 'renote', 'renote:grouped', 'quote', 'reaction', 'reaction:grouped', 'pollEnded'] as (typeof notificationTypes[number])[]); +const NOTE_REQUIRED_NOTIFICATION_TYPES = new Set(['note', 'mention', 'reply', 'renote', 'renote:grouped', 'quote', 'reaction', 'reaction:grouped', 'pollEnded'] as (typeof groupedNotificationTypes[number])[]); @Injectable() export class NotificationEntityService implements OnModuleInit { diff --git a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts index 703808d2792a..715927e9919d 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts @@ -7,7 +7,7 @@ import { Brackets, In } from 'typeorm'; import * as Redis from 'ioredis'; import { Inject, Injectable } from '@nestjs/common'; import type { NotesRepository } from '@/models/_.js'; -import { obsoleteNotificationTypes, notificationTypes, FilterUnionByProperty } from '@/types.js'; +import { obsoleteNotificationTypes, groupedNotificationTypes, FilterUnionByProperty } from '@/types.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; import { NoteReadService } from '@/core/NoteReadService.js'; import { NotificationEntityService } from '@/core/entities/NotificationEntityService.js'; @@ -48,10 +48,10 @@ export const paramDef = { markAsRead: { type: 'boolean', default: true }, // 後方互換のため、廃止された通知タイプも受け付ける includeTypes: { type: 'array', items: { - type: 'string', enum: [...notificationTypes, ...obsoleteNotificationTypes], + type: 'string', enum: [...groupedNotificationTypes, ...obsoleteNotificationTypes], } }, excludeTypes: { type: 'array', items: { - type: 'string', enum: [...notificationTypes, ...obsoleteNotificationTypes], + type: 'string', enum: [...groupedNotificationTypes, ...obsoleteNotificationTypes], } }, }, required: [], @@ -79,12 +79,12 @@ export default class extends Endpoint { // eslint- return []; } // excludeTypes に全指定されている場合はクエリしない - if (notificationTypes.every(type => ps.excludeTypes?.includes(type))) { + if (groupedNotificationTypes.every(type => ps.excludeTypes?.includes(type))) { return []; } - 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][]; + 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( diff --git a/packages/backend/src/types.ts b/packages/backend/src/types.ts index 633547419acb..020de2cda620 100644 --- a/packages/backend/src/types.ts +++ b/packages/backend/src/types.ts @@ -35,9 +35,14 @@ export const notificationTypes = [ 'achievementEarned', 'app', 'test', +] as const; + +export const groupedNotificationTypes = [ + ...notificationTypes, 'reaction:grouped', 'renote:grouped', ] as const; + export const obsoleteNotificationTypes = ['pollVote', 'groupInvited'] as const; export const noteVisibilities = ['public', 'home', 'followers', 'specified'] as const; From 6c69a4307a58a7b91fe78a643f2130da186488d8 Mon Sep 17 00:00:00 2001 From: taichanne30 Date: Wed, 21 Feb 2024 04:34:36 +0900 Subject: [PATCH 28/50] Update misskey-js typedef --- packages/misskey-js/src/autogen/types.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/misskey-js/src/autogen/types.ts b/packages/misskey-js/src/autogen/types.ts index d0d8573b407c..742759545ee8 100644 --- a/packages/misskey-js/src/autogen/types.ts +++ b/packages/misskey-js/src/autogen/types.ts @@ -17582,8 +17582,8 @@ export type operations = { untilId?: string; /** @default true */ markAsRead?: boolean; - includeTypes?: ('note' | 'follow' | 'mention' | 'reply' | 'renote' | 'quote' | 'reaction' | 'pollEnded' | 'receiveFollowRequest' | 'followRequestAccepted' | 'roleAssigned' | 'achievementEarned' | 'app' | 'test' | 'pollVote' | 'groupInvited')[]; - excludeTypes?: ('note' | 'follow' | 'mention' | 'reply' | 'renote' | 'quote' | 'reaction' | 'pollEnded' | 'receiveFollowRequest' | 'followRequestAccepted' | 'roleAssigned' | 'achievementEarned' | 'app' | 'test' | 'pollVote' | 'groupInvited')[]; + includeTypes?: ('note' | 'follow' | 'mention' | 'reply' | 'renote' | 'quote' | 'reaction' | 'pollEnded' | 'receiveFollowRequest' | 'followRequestAccepted' | 'roleAssigned' | 'achievementEarned' | 'app' | 'test' | 'reaction:grouped' | 'renote:grouped' | 'pollVote' | 'groupInvited')[]; + excludeTypes?: ('note' | 'follow' | 'mention' | 'reply' | 'renote' | 'quote' | 'reaction' | 'pollEnded' | 'receiveFollowRequest' | 'followRequestAccepted' | 'roleAssigned' | 'achievementEarned' | 'app' | 'test' | 'reaction:grouped' | 'renote:grouped' | 'pollVote' | 'groupInvited')[]; }; }; }; From ed33ac714126e3c1a5c3d7a84e7bda368b53a20f Mon Sep 17 00:00:00 2001 From: taichanne30 Date: Thu, 22 Feb 2024 01:25:16 +0900 Subject: [PATCH 29/50] Refactor: less sql calls --- .../entities/NotificationEntityService.ts | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index e2a6ab6e8e34..7bf908ec3349 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -279,11 +279,13 @@ export class NotificationEntityService implements OnModuleInit { notification: T, userIdsWhoMeMuting: Set, userMutedInstances: Set, + notifiers: MiUser[], ): Promise { if (!('notifierId' in notification)) return true; if (userIdsWhoMeMuting.has(notification.notifierId)) return false; - const notifier = await this.usersRepository.findOneBy({ id: notification.notifierId }); + const notifier = notifiers.find(x => x.id === notification.notifierId) ?? null; + if (notifier === null) return false; if (notifier.host && userMutedInstances.has(notifier.host)) return false; @@ -307,7 +309,13 @@ export class NotificationEntityService implements OnModuleInit { this.cacheService.userProfileCache.fetch(meId).then(p => new Set(p.mutedInstances)), ]); - return this.#validateNotifier(notification, userIdsWhoMeMuting, userMutedInstances); + const notifiers = (await Promise.all([notification] + .map(x => 'notifierId' in x ? x.notifierId : null) + .filter(isNotNull) + .map(async id => await this.usersRepository.findOneBy({ id })))) + .filter(isNotNull); + + return this.#validateNotifier(notification, userIdsWhoMeMuting, userMutedInstances, notifiers); } /** @@ -325,8 +333,13 @@ export class NotificationEntityService implements OnModuleInit { this.cacheService.userProfileCache.fetch(meId).then(p => new Set(p.mutedInstances)), ]); + const notifierIds = notifications.map(notification => 'notifierId' in notification ? notification.notifierId : null).filter(isNotNull); + const notifiers = notifierIds.length > 0 ? await this.usersRepository.find({ + where: { id: In(notifierIds) }, + }) : []; + const filteredNotifications = ((await Promise.all(notifications.map(async (notification) => { - const isValid = await this.#validateNotifier(notification, userIdsWhoMeMuting, userMutedInstances); + const isValid = await this.#validateNotifier(notification, userIdsWhoMeMuting, userMutedInstances, notifiers); return isValid ? notification : null; }))) as [T|null] ).filter(isNotNull); From e2443d8dc502bfa708f5c74e4389cbf9d0e419c9 Mon Sep 17 00:00:00 2001 From: tamaina Date: Wed, 21 Feb 2024 18:08:27 +0000 Subject: [PATCH 30/50] refactor --- .../entities/NotificationEntityService.ts | 76 ++++++++----------- .../api/endpoints/i/notifications-grouped.ts | 11 +-- .../server/api/endpoints/i/notifications.ts | 11 +-- 3 files changed, 33 insertions(+), 65 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 7bf908ec3349..f5df3d9431ef 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -16,6 +16,8 @@ import { bindThis } from '@/decorators.js'; import { isNotNull } from '@/misc/is-not-null.js'; import { FilterUnionByProperty, groupedNotificationTypes } from '@/types.js'; import { CacheService } from '@/core/CacheService.js'; +import { NoteReadService } from '@/core/NoteReadService.js'; +import { trackPromise } from '@/misc/promise-tracker.js'; import { RoleEntityService } from './RoleEntityService.js'; import type { OnModuleInit } from '@nestjs/common'; import type { UserEntityService } from './UserEntityService.js'; @@ -42,6 +44,7 @@ export class NotificationEntityService implements OnModuleInit { private followRequestsRepository: FollowRequestsRepository, private cacheService: CacheService, + private noteReadService: NoteReadService, //private userEntityService: UserEntityService, //private noteEntityService: NoteEntityService, @@ -82,9 +85,7 @@ export class NotificationEntityService implements OnModuleInit { }) ) : undefined; // if the note has been deleted, don't show this notification - if (needsNote && !noteIfNeed) { - return null; - } + if (needsNote && !noteIfNeed) return null; const needsUser = 'notifierId' in notification; const userIfNeed = needsUser ? ( @@ -93,9 +94,7 @@ export class NotificationEntityService implements OnModuleInit { : this.userEntityService.pack(notification.notifierId, { id: meId }) ) : undefined; // if the user has been deleted, don't show this notification - if (needsUser && !userIfNeed) { - return null; - } + if (needsUser && !userIfNeed) return null; // #region Grouped notifications if (notification.type === 'reaction:grouped') { @@ -178,6 +177,7 @@ export class NotificationEntityService implements OnModuleInit { async #packManyInternal ( notifications: T[], meId: MiUser['id'], + markNotesAsRead = false, ): Promise { if (notifications.length === 0) return []; @@ -218,15 +218,29 @@ export class NotificationEntityService implements OnModuleInit { validNotifications = validNotifications.filter(x => (x.type !== 'receiveFollowRequest') || reqs.some(r => r.followerId === x.notifierId)); } - return (await Promise.all(validNotifications.map(x => this.packGrouped(x, meId, { checkValidNotifier: false }, { - packedNotes, - packedUsers, - })))).filter(isNotNull); + const packPromises = validNotifications.map(x => { + return this.pack( + x, + meId, + { checkValidNotifier: false }, + { packedNotes, packedUsers }, + ); + }); + + if (markNotesAsRead) { + try { + trackPromise(this.noteReadService.read(meId, notes)); + } catch (e) { + // console.error('error thrown by NoteReadService.read', e); + } + } + + return (await Promise.all(packPromises)).filter(isNotNull); } @bindThis public async pack( - src: MiNotification, + src: MiNotification | MiGroupedNotification, meId: MiUser['id'], // eslint-disable-next-line @typescript-eslint/ban-types options: { @@ -237,39 +251,25 @@ export class NotificationEntityService implements OnModuleInit { packedUsers: Map>; }, ): Promise | null> { - return this.#packInternal(src, meId, options, hint); + return await this.#packInternal(src, meId, options, hint); } @bindThis public async packMany( notifications: MiNotification[], meId: MiUser['id'], + markNotesAsRead = false, ): Promise { - return await this.#packManyInternal(notifications, meId); - } - - @bindThis - public async packGrouped( - src: MiGroupedNotification, - meId: MiUser['id'], - // eslint-disable-next-line @typescript-eslint/ban-types - options: { - checkValidNotifier?: boolean; - }, - hint?: { - packedNotes: Map>; - packedUsers: Map>; - }, - ): Promise | null> { - return await this.#packInternal(src, meId, options, hint); + return await this.#packManyInternal(notifications, meId, markNotesAsRead); } @bindThis public async packGroupedMany( notifications: MiGroupedNotification[], meId: MiUser['id'], + markNotesAsRead = false, ) : Promise { - return await this.#packManyInternal(notifications, meId); + return await this.#packManyInternal(notifications, meId, markNotesAsRead); } /** @@ -301,21 +301,7 @@ export class NotificationEntityService implements OnModuleInit { notification: T, meId: MiUser['id'], ) : Promise { - const [ - userIdsWhoMeMuting, - userMutedInstances, - ] = await Promise.all([ - this.cacheService.userMutingsCache.fetch(meId), - this.cacheService.userProfileCache.fetch(meId).then(p => new Set(p.mutedInstances)), - ]); - - const notifiers = (await Promise.all([notification] - .map(x => 'notifierId' in x ? x.notifierId : null) - .filter(isNotNull) - .map(async id => await this.usersRepository.findOneBy({ id })))) - .filter(isNotNull); - - return this.#validateNotifier(notification, userIdsWhoMeMuting, userMutedInstances, notifiers); + return (await this.#filterValidNotifier([notification], meId)).length === 1; } /** diff --git a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts index 715927e9919d..ff1d3c43e2e9 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts @@ -163,16 +163,7 @@ export default class extends Endpoint { // eslint- groupedNotifications = groupedNotifications.slice(0, ps.limit); - const noteIds = groupedNotifications - .filter((notification): notification is FilterUnionByProperty => ['mention', 'reply', 'quote'].includes(notification.type)) - .map(notification => notification.noteId!); - - if (noteIds.length > 0) { - const notes = await this.notesRepository.findBy({ id: In(noteIds) }); - this.noteReadService.read(me.id, notes); - } - - return await this.notificationEntityService.packGroupedMany(groupedNotifications, me.id); + return await this.notificationEntityService.packGroupedMany(groupedNotifications, me.id, true); }); } } diff --git a/packages/backend/src/server/api/endpoints/i/notifications.ts b/packages/backend/src/server/api/endpoints/i/notifications.ts index 52b6749e3fa0..efefbc286508 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications.ts @@ -112,16 +112,7 @@ export default class extends Endpoint { // eslint- this.notificationService.readAllNotification(me.id); } - const noteIds = notifications - .filter((notification): notification is FilterUnionByProperty => ['mention', 'reply', 'quote'].includes(notification.type)) - .map(notification => notification.noteId); - - if (noteIds.length > 0) { - const notes = await this.notesRepository.findBy({ id: In(noteIds) }); - this.noteReadService.read(me.id, notes); - } - - return await this.notificationEntityService.packMany(notifications, me.id); + return await this.notificationEntityService.packMany(notifications, me.id, true); }); } } From 60109cc4b8bd37f6348fbc055973f3e13789be72 Mon Sep 17 00:00:00 2001 From: tamaina Date: Wed, 21 Feb 2024 18:09:34 +0000 Subject: [PATCH 31/50] clean up --- .../src/server/api/endpoints/i/notifications-grouped.ts | 7 ------- .../backend/src/server/api/endpoints/i/notifications.ts | 7 ------- 2 files changed, 14 deletions(-) diff --git a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts index ff1d3c43e2e9..5cf4b049dd3d 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts @@ -3,13 +3,10 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import { Brackets, 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 { Endpoint } from '@/server/api/endpoint-base.js'; -import { NoteReadService } from '@/core/NoteReadService.js'; import { NotificationEntityService } from '@/core/entities/NotificationEntityService.js'; import { NotificationService } from '@/core/NotificationService.js'; import { DI } from '@/di-symbols.js'; @@ -63,13 +60,9 @@ export default class extends Endpoint { // eslint- @Inject(DI.redis) private redisClient: Redis.Redis, - @Inject(DI.notesRepository) - private notesRepository: NotesRepository, - private idService: IdService, private notificationEntityService: NotificationEntityService, private notificationService: NotificationService, - private noteReadService: NoteReadService, ) { super(meta, paramDef, async (ps, me) => { const EXTRA_LIMIT = 100; diff --git a/packages/backend/src/server/api/endpoints/i/notifications.ts b/packages/backend/src/server/api/endpoints/i/notifications.ts index efefbc286508..671bbe028bd1 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications.ts @@ -3,13 +3,10 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import { Brackets, In } from 'typeorm'; import * as Redis from 'ioredis'; import { Inject, Injectable } from '@nestjs/common'; -import type { NotesRepository } from '@/models/_.js'; import { obsoleteNotificationTypes, notificationTypes, FilterUnionByProperty } from '@/types.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; -import { NoteReadService } from '@/core/NoteReadService.js'; import { NotificationEntityService } from '@/core/entities/NotificationEntityService.js'; import { NotificationService } from '@/core/NotificationService.js'; import { DI } from '@/di-symbols.js'; @@ -63,13 +60,9 @@ export default class extends Endpoint { // eslint- @Inject(DI.redis) private redisClient: Redis.Redis, - @Inject(DI.notesRepository) - private notesRepository: NotesRepository, - private idService: IdService, private notificationEntityService: NotificationEntityService, private notificationService: NotificationService, - private noteReadService: NoteReadService, ) { super(meta, paramDef, async (ps, me) => { // includeTypes が空の場合はクエリしない From 7753124ec1d1fc40454f59820f5478ad5f28cc56 Mon Sep 17 00:00:00 2001 From: tamaina Date: Wed, 21 Feb 2024 18:36:38 +0000 Subject: [PATCH 32/50] filter notes to mark as read --- .../src/core/entities/NotificationEntityService.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index f5df3d9431ef..fc8881324b9a 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -24,6 +24,7 @@ import type { UserEntityService } from './UserEntityService.js'; import type { NoteEntityService } from './NoteEntityService.js'; const NOTE_REQUIRED_NOTIFICATION_TYPES = new Set(['note', 'mention', 'reply', 'renote', 'renote:grouped', 'quote', 'reaction', 'reaction:grouped', 'pollEnded'] as (typeof groupedNotificationTypes[number])[]); +const MARK_NOTE_READ_NOTIFICATION_TYPES = new Set(['mention', 'reply', 'quote'] as (typeof groupedNotificationTypes[number])[]); @Injectable() export class NotificationEntityService implements OnModuleInit { @@ -229,7 +230,13 @@ export class NotificationEntityService implements OnModuleInit { if (markNotesAsRead) { try { - trackPromise(this.noteReadService.read(meId, notes)); + const noteIdsToRead = validNotifications.reduce((acc, x) => { + if (MARK_NOTE_READ_NOTIFICATION_TYPES.has(x.type) && 'noteId' in x) { + acc.add(x.noteId); + } + return acc; + }, new Set()); + trackPromise(this.noteReadService.read(meId, notes.filter(x => noteIdsToRead.has(x.id)))); } catch (e) { // console.error('error thrown by NoteReadService.read', e); } From 6b45b814c95553609bf8ae5123827ecb3298e089 Mon Sep 17 00:00:00 2001 From: tamaina Date: Wed, 21 Feb 2024 18:44:11 +0000 Subject: [PATCH 33/50] =?UTF-8?q?packed=20note=E3=81=8Cmap=E3=81=AA?= =?UTF-8?q?=E3=81=AE=E3=81=A7=E3=81=9D=E3=81=A1=E3=82=89=E3=82=92=E4=BD=BF?= =?UTF-8?q?=E3=81=86?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/core/entities/NotificationEntityService.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index fc8881324b9a..e10b68df02a7 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -230,13 +230,14 @@ export class NotificationEntityService implements OnModuleInit { if (markNotesAsRead) { try { - const noteIdsToRead = validNotifications.reduce((acc, x) => { + const notesToRead = validNotifications.reduce((acc, x) => { if (MARK_NOTE_READ_NOTIFICATION_TYPES.has(x.type) && 'noteId' in x) { - acc.add(x.noteId); + const note = packedNotes.get(x.noteId); + if (note) acc.add(note); } return acc; - }, new Set()); - trackPromise(this.noteReadService.read(meId, notes.filter(x => noteIdsToRead.has(x.id)))); + }, new Set>()); + trackPromise(this.noteReadService.read(meId, Array.from(notesToRead))); } catch (e) { // console.error('error thrown by NoteReadService.read', e); } From 6be1d986cdb4e75549de4352b3721bd9be631c67 Mon Sep 17 00:00:00 2001 From: tamaina Date: Wed, 21 Feb 2024 18:45:24 +0000 Subject: [PATCH 34/50] if (notesToRead.size > 0) --- .../backend/src/core/entities/NotificationEntityService.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index e10b68df02a7..cc40a3f754a9 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -237,7 +237,9 @@ export class NotificationEntityService implements OnModuleInit { } return acc; }, new Set>()); - trackPromise(this.noteReadService.read(meId, Array.from(notesToRead))); + if (notesToRead.size > 0) { + trackPromise(this.noteReadService.read(meId, Array.from(notesToRead))); + } } catch (e) { // console.error('error thrown by NoteReadService.read', e); } From 22e2324f9633bddba50769ef838bc5ddb4564c88 Mon Sep 17 00:00:00 2001 From: tamaina Date: Wed, 21 Feb 2024 18:46:52 +0000 Subject: [PATCH 35/50] if (notes.length === 0) return; --- packages/backend/src/core/NoteReadService.ts | 2 ++ .../backend/src/core/entities/NotificationEntityService.ts | 4 +--- packages/backend/src/server/api/endpoints/antennas/notes.ts | 4 +--- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/backend/src/core/NoteReadService.ts b/packages/backend/src/core/NoteReadService.ts index feef024602ff..b3746b62ea0e 100644 --- a/packages/backend/src/core/NoteReadService.ts +++ b/packages/backend/src/core/NoteReadService.ts @@ -88,6 +88,8 @@ export class NoteReadService implements OnApplicationShutdown { userId: MiUser['id'], notes: (MiNote | Packed<'Note'>)[], ): Promise { + if (notes.length === 0) return; + const readMentions: (MiNote | Packed<'Note'>)[] = []; const readSpecifiedNotes: (MiNote | Packed<'Note'>)[] = []; diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index cc40a3f754a9..e10b68df02a7 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -237,9 +237,7 @@ export class NotificationEntityService implements OnModuleInit { } return acc; }, new Set>()); - if (notesToRead.size > 0) { - trackPromise(this.noteReadService.read(meId, Array.from(notesToRead))); - } + trackPromise(this.noteReadService.read(meId, Array.from(notesToRead))); } catch (e) { // console.error('error thrown by NoteReadService.read', e); } diff --git a/packages/backend/src/server/api/endpoints/antennas/notes.ts b/packages/backend/src/server/api/endpoints/antennas/notes.ts index 39f3fab21ee0..f4dfe1ecc4d7 100644 --- a/packages/backend/src/server/api/endpoints/antennas/notes.ts +++ b/packages/backend/src/server/api/endpoints/antennas/notes.ts @@ -124,9 +124,7 @@ export default class extends Endpoint { // eslint- notes.sort((a, b) => a.id > b.id ? -1 : 1); } - if (notes.length > 0) { - this.noteReadService.read(me.id, notes); - } + this.noteReadService.read(me.id, notes); return await this.noteEntityService.packMany(notes, me); }); From 9f2b402eeb7c1ccbcade678267e6bcdbdb254dc9 Mon Sep 17 00:00:00 2001 From: tamaina Date: Wed, 21 Feb 2024 18:49:33 +0000 Subject: [PATCH 36/50] fix --- .../src/core/entities/NotificationEntityService.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index e10b68df02a7..9c1bfd6fddeb 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -231,13 +231,13 @@ export class NotificationEntityService implements OnModuleInit { if (markNotesAsRead) { try { const notesToRead = validNotifications.reduce((acc, x) => { - if (MARK_NOTE_READ_NOTIFICATION_TYPES.has(x.type) && 'noteId' in x) { + if (MARK_NOTE_READ_NOTIFICATION_TYPES.has(x.type) && 'noteId' in x && !acc.has(x.noteId)) { const note = packedNotes.get(x.noteId); - if (note) acc.add(note); + if (note) acc.set(x.noteId, note); } return acc; - }, new Set>()); - trackPromise(this.noteReadService.read(meId, Array.from(notesToRead))); + }, new Map>()); + trackPromise(this.noteReadService.read(meId, Array.from(notesToRead.values()))); } catch (e) { // console.error('error thrown by NoteReadService.read', e); } From 665cf2460aae6a6e48f79c1f827575b9055ca45a Mon Sep 17 00:00:00 2001 From: tamaina Date: Wed, 21 Feb 2024 19:03:11 +0000 Subject: [PATCH 37/50] Revert "if (notes.length === 0) return;" This reverts commit 22e2324f9633bddba50769ef838bc5ddb4564c88. --- packages/backend/src/core/NoteReadService.ts | 2 -- packages/backend/src/server/api/endpoints/antennas/notes.ts | 4 +++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/core/NoteReadService.ts b/packages/backend/src/core/NoteReadService.ts index b3746b62ea0e..feef024602ff 100644 --- a/packages/backend/src/core/NoteReadService.ts +++ b/packages/backend/src/core/NoteReadService.ts @@ -88,8 +88,6 @@ export class NoteReadService implements OnApplicationShutdown { userId: MiUser['id'], notes: (MiNote | Packed<'Note'>)[], ): Promise { - if (notes.length === 0) return; - const readMentions: (MiNote | Packed<'Note'>)[] = []; const readSpecifiedNotes: (MiNote | Packed<'Note'>)[] = []; diff --git a/packages/backend/src/server/api/endpoints/antennas/notes.ts b/packages/backend/src/server/api/endpoints/antennas/notes.ts index f4dfe1ecc4d7..39f3fab21ee0 100644 --- a/packages/backend/src/server/api/endpoints/antennas/notes.ts +++ b/packages/backend/src/server/api/endpoints/antennas/notes.ts @@ -124,7 +124,9 @@ export default class extends Endpoint { // eslint- notes.sort((a, b) => a.id > b.id ? -1 : 1); } - this.noteReadService.read(me.id, notes); + if (notes.length > 0) { + this.noteReadService.read(me.id, notes); + } return await this.noteEntityService.packMany(notes, me); }); From ba6cb939fc8adb95b29c7c026e61c1417c049d3f Mon Sep 17 00:00:00 2001 From: tamaina Date: Wed, 21 Feb 2024 19:06:55 +0000 Subject: [PATCH 38/50] :art: --- .../src/core/entities/NotificationEntityService.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 9c1bfd6fddeb..3c7e364073cf 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -72,7 +72,7 @@ export class NotificationEntityService implements OnModuleInit { packedNotes: Map>; packedUsers: Map>; }, - ) : Promise | null> { + ): Promise | null> { const notification = src; if (options.checkValidNotifier !== false && !(await this.#isValidNotifier(notification, meId))) return null; @@ -276,7 +276,7 @@ export class NotificationEntityService implements OnModuleInit { notifications: MiGroupedNotification[], meId: MiUser['id'], markNotesAsRead = false, - ) : Promise { + ): Promise { return await this.#packManyInternal(notifications, meId, markNotesAsRead); } @@ -308,7 +308,7 @@ export class NotificationEntityService implements OnModuleInit { async #isValidNotifier ( notification: T, meId: MiUser['id'], - ) : Promise { + ): Promise { return (await this.#filterValidNotifier([notification], meId)).length === 1; } @@ -318,7 +318,7 @@ export class NotificationEntityService implements OnModuleInit { async #filterValidNotifier ( notifications: T[], meId: MiUser['id'], - ) : Promise { + ): Promise { const [ userIdsWhoMeMuting, userMutedInstances, From 45fe97441902e98204ef2c7cbe5ea3c47e8ba3a6 Mon Sep 17 00:00:00 2001 From: tamaina Date: Wed, 21 Feb 2024 19:11:51 +0000 Subject: [PATCH 39/50] console.error --- packages/backend/src/core/entities/NotificationEntityService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 3c7e364073cf..400b4d01e6e9 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -239,7 +239,7 @@ export class NotificationEntityService implements OnModuleInit { }, new Map>()); trackPromise(this.noteReadService.read(meId, Array.from(notesToRead.values()))); } catch (e) { - // console.error('error thrown by NoteReadService.read', e); + console.error('error thrown by NoteReadService.read', e); } } From 810656937e88c05613738de90d1bd7ac2f354268 Mon Sep 17 00:00:00 2001 From: tamaina Date: Wed, 21 Feb 2024 19:12:22 +0000 Subject: [PATCH 40/50] err --- .../backend/src/core/entities/NotificationEntityService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 400b4d01e6e9..e7b86595a46e 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -238,8 +238,8 @@ export class NotificationEntityService implements OnModuleInit { return acc; }, new Map>()); trackPromise(this.noteReadService.read(meId, Array.from(notesToRead.values()))); - } catch (e) { - console.error('error thrown by NoteReadService.read', e); + } catch (err) { + console.error('error thrown by NoteReadService.read', err); } } From 25f441d8605ff7a25db24e2d0f50b9535edd3626 Mon Sep 17 00:00:00 2001 From: tamaina Date: Wed, 21 Feb 2024 19:13:23 +0000 Subject: [PATCH 41/50] remove try-catch --- .../entities/NotificationEntityService.ts | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index e7b86595a46e..2e2e873c3b43 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -229,18 +229,14 @@ export class NotificationEntityService implements OnModuleInit { }); if (markNotesAsRead) { - try { - const notesToRead = validNotifications.reduce((acc, x) => { - if (MARK_NOTE_READ_NOTIFICATION_TYPES.has(x.type) && 'noteId' in x && !acc.has(x.noteId)) { - const note = packedNotes.get(x.noteId); - if (note) acc.set(x.noteId, note); - } - return acc; - }, new Map>()); - trackPromise(this.noteReadService.read(meId, Array.from(notesToRead.values()))); - } catch (err) { - console.error('error thrown by NoteReadService.read', err); - } + const notesToRead = validNotifications.reduce((acc, x) => { + if (MARK_NOTE_READ_NOTIFICATION_TYPES.has(x.type) && 'noteId' in x && !acc.has(x.noteId)) { + const note = packedNotes.get(x.noteId); + if (note) acc.set(x.noteId, note); + } + return acc; + }, new Map>()); + trackPromise(this.noteReadService.read(meId, Array.from(notesToRead.values()))); } return (await Promise.all(packPromises)).filter(isNotNull); From 47264afd8406f47a7f5ac24764f8141ac5760106 Mon Sep 17 00:00:00 2001 From: tamaina Date: Wed, 21 Feb 2024 19:15:06 +0000 Subject: [PATCH 42/50] =?UTF-8?q?=E4=B8=8D=E8=A6=81=E3=81=AA=E3=82=B8?= =?UTF-8?q?=E3=82=A7=E3=83=8D=E3=83=AA=E3=82=AF=E3=82=B9=E3=82=92=E9=99=A4?= =?UTF-8?q?=E5=8E=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../backend/src/core/entities/NotificationEntityService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 2e2e873c3b43..4cffff6eb6c0 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -301,8 +301,8 @@ export class NotificationEntityService implements OnModuleInit { /** * notifierが存在するか、ミュートされていないか、サスペンドされていないかを実際に確認する */ - async #isValidNotifier ( - notification: T, + async #isValidNotifier( + notification: MiNotification | MiGroupedNotification, meId: MiUser['id'], ): Promise { return (await this.#filterValidNotifier([notification], meId)).length === 1; From 9729f73d6c32ca73862709591d44f756126d23d8 Mon Sep 17 00:00:00 2001 From: taichanne30 Date: Sat, 24 Feb 2024 01:59:29 +0900 Subject: [PATCH 43/50] =?UTF-8?q?Revert=20=20(=E6=97=A2=E8=AA=AD=E5=87=A6?= =?UTF-8?q?=E7=90=86=E3=82=92pack=E5=86=85=E3=81=A7=E8=A1=8C=E3=81=86?= =?UTF-8?q?=E3=82=82=E3=81=AE=E3=82=92=E5=85=83=E3=81=AB=E6=88=BB=E3=81=99?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../entities/NotificationEntityService.ts | 20 ++----------------- .../api/endpoints/i/notifications-grouped.ts | 17 +++++++++++++++- .../server/api/endpoints/i/notifications.ts | 18 ++++++++++++++++- 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 4cffff6eb6c0..ca30478350f5 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -17,14 +17,12 @@ import { isNotNull } from '@/misc/is-not-null.js'; import { FilterUnionByProperty, groupedNotificationTypes } from '@/types.js'; import { CacheService } from '@/core/CacheService.js'; import { NoteReadService } from '@/core/NoteReadService.js'; -import { trackPromise } from '@/misc/promise-tracker.js'; import { RoleEntityService } from './RoleEntityService.js'; import type { OnModuleInit } from '@nestjs/common'; import type { UserEntityService } from './UserEntityService.js'; import type { NoteEntityService } from './NoteEntityService.js'; const NOTE_REQUIRED_NOTIFICATION_TYPES = new Set(['note', 'mention', 'reply', 'renote', 'renote:grouped', 'quote', 'reaction', 'reaction:grouped', 'pollEnded'] as (typeof groupedNotificationTypes[number])[]); -const MARK_NOTE_READ_NOTIFICATION_TYPES = new Set(['mention', 'reply', 'quote'] as (typeof groupedNotificationTypes[number])[]); @Injectable() export class NotificationEntityService implements OnModuleInit { @@ -178,7 +176,6 @@ export class NotificationEntityService implements OnModuleInit { async #packManyInternal ( notifications: T[], meId: MiUser['id'], - markNotesAsRead = false, ): Promise { if (notifications.length === 0) return []; @@ -228,17 +225,6 @@ export class NotificationEntityService implements OnModuleInit { ); }); - if (markNotesAsRead) { - const notesToRead = validNotifications.reduce((acc, x) => { - if (MARK_NOTE_READ_NOTIFICATION_TYPES.has(x.type) && 'noteId' in x && !acc.has(x.noteId)) { - const note = packedNotes.get(x.noteId); - if (note) acc.set(x.noteId, note); - } - return acc; - }, new Map>()); - trackPromise(this.noteReadService.read(meId, Array.from(notesToRead.values()))); - } - return (await Promise.all(packPromises)).filter(isNotNull); } @@ -262,18 +248,16 @@ export class NotificationEntityService implements OnModuleInit { public async packMany( notifications: MiNotification[], meId: MiUser['id'], - markNotesAsRead = false, ): Promise { - return await this.#packManyInternal(notifications, meId, markNotesAsRead); + return await this.#packManyInternal(notifications, meId); } @bindThis public async packGroupedMany( notifications: MiGroupedNotification[], meId: MiUser['id'], - markNotesAsRead = false, ): Promise { - return await this.#packManyInternal(notifications, meId, markNotesAsRead); + return await this.#packManyInternal(notifications, meId); } /** diff --git a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts index 5cf4b049dd3d..dc6ffd3e02e2 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts @@ -3,10 +3,13 @@ * SPDX-License-Identifier: AGPL-3.0-only */ +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 { Endpoint } from '@/server/api/endpoint-base.js'; +import { NoteReadService } from '@/core/NoteReadService.js'; import { NotificationEntityService } from '@/core/entities/NotificationEntityService.js'; import { NotificationService } from '@/core/NotificationService.js'; import { DI } from '@/di-symbols.js'; @@ -60,9 +63,13 @@ export default class extends Endpoint { // eslint- @Inject(DI.redis) private redisClient: Redis.Redis, + @Inject(DI.notesRepository) + private notesRepository: NotesRepository, + private idService: IdService, private notificationEntityService: NotificationEntityService, private notificationService: NotificationService, + private noteReadService: NoteReadService, ) { super(meta, paramDef, async (ps, me) => { const EXTRA_LIMIT = 100; @@ -155,8 +162,16 @@ export default class extends Endpoint { // eslint- } groupedNotifications = groupedNotifications.slice(0, ps.limit); + const noteIds = groupedNotifications + .filter((notification): notification is FilterUnionByProperty => ['mention', 'reply', 'quote'].includes(notification.type)) + .map(notification => notification.noteId!); + + if (noteIds.length > 0) { + const notes = await this.notesRepository.findBy({ id: In(noteIds) }); + this.noteReadService.read(me.id, notes); + } - return await this.notificationEntityService.packGroupedMany(groupedNotifications, me.id, true); + return await this.notificationEntityService.packGroupedMany(groupedNotifications, me.id); }); } } diff --git a/packages/backend/src/server/api/endpoints/i/notifications.ts b/packages/backend/src/server/api/endpoints/i/notifications.ts index 671bbe028bd1..320d9fdb0039 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications.ts @@ -3,10 +3,13 @@ * SPDX-License-Identifier: AGPL-3.0-only */ +import { In } from 'typeorm'; import * as Redis from 'ioredis'; import { Inject, Injectable } from '@nestjs/common'; +import type { NotesRepository } from '@/models/_.js'; import { obsoleteNotificationTypes, notificationTypes, FilterUnionByProperty } from '@/types.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; +import { NoteReadService } from '@/core/NoteReadService.js'; import { NotificationEntityService } from '@/core/entities/NotificationEntityService.js'; import { NotificationService } from '@/core/NotificationService.js'; import { DI } from '@/di-symbols.js'; @@ -60,9 +63,13 @@ export default class extends Endpoint { // eslint- @Inject(DI.redis) private redisClient: Redis.Redis, + @Inject(DI.notesRepository) + private notesRepository: NotesRepository, + private idService: IdService, private notificationEntityService: NotificationEntityService, private notificationService: NotificationService, + private noteReadService: NoteReadService, ) { super(meta, paramDef, async (ps, me) => { // includeTypes が空の場合はクエリしない @@ -105,7 +112,16 @@ export default class extends Endpoint { // eslint- this.notificationService.readAllNotification(me.id); } - return await this.notificationEntityService.packMany(notifications, me.id, true); + const noteIds = notifications + .filter((notification): notification is FilterUnionByProperty => ['mention', 'reply', 'quote'].includes(notification.type)) + .map(notification => notification.noteId); + + if (noteIds.length > 0) { + const notes = await this.notesRepository.findBy({ id: In(noteIds) }); + this.noteReadService.read(me.id, notes); + } + + return await this.notificationEntityService.packMany(notifications, me.id); }); } } From 8df636ef8dc85411d47bb195287ddb26d8f5feda Mon Sep 17 00:00:00 2001 From: taichanne30 Date: Sat, 24 Feb 2024 02:19:05 +0900 Subject: [PATCH 44/50] Clean --- packages/backend/src/core/entities/NotificationEntityService.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index ca30478350f5..4a6a4e5480d6 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -16,7 +16,6 @@ import { bindThis } from '@/decorators.js'; import { isNotNull } from '@/misc/is-not-null.js'; import { FilterUnionByProperty, groupedNotificationTypes } from '@/types.js'; import { CacheService } from '@/core/CacheService.js'; -import { NoteReadService } from '@/core/NoteReadService.js'; import { RoleEntityService } from './RoleEntityService.js'; import type { OnModuleInit } from '@nestjs/common'; import type { UserEntityService } from './UserEntityService.js'; @@ -43,7 +42,6 @@ export class NotificationEntityService implements OnModuleInit { private followRequestsRepository: FollowRequestsRepository, private cacheService: CacheService, - private noteReadService: NoteReadService, //private userEntityService: UserEntityService, //private noteEntityService: NoteEntityService, From 9f1a9e8dcc68211780c08b84602c3ef33be1eb1d Mon Sep 17 00:00:00 2001 From: syuilo <4439005+syuilo@users.noreply.github.com> Date: Wed, 28 Feb 2024 17:06:32 +0900 Subject: [PATCH 45/50] Update packages/backend/src/core/entities/NotificationEntityService.ts --- packages/backend/src/core/entities/NotificationEntityService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 4a6a4e5480d6..0186a499e289 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -105,7 +105,7 @@ export class NotificationEntityService implements OnModuleInit { }; }))).filter(r => isNotNull(r.user)); // if all users have been deleted, don't show this notification - if (!reactions.length) { + if (reactions.length === 0) { return null; } From eead5ff01d36e2039f240575c044d072e52f5850 Mon Sep 17 00:00:00 2001 From: syuilo <4439005+syuilo@users.noreply.github.com> Date: Wed, 28 Feb 2024 17:06:38 +0900 Subject: [PATCH 46/50] Update packages/backend/src/core/entities/NotificationEntityService.ts --- packages/backend/src/core/entities/NotificationEntityService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 0186a499e289..b2fdcb10da27 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -126,7 +126,7 @@ export class NotificationEntityService implements OnModuleInit { return this.userEntityService.pack(userId, { id: meId }); }))).filter(isNotNull); // if all users have been deleted, don't show this notification - if (!users.length) { + if (users.length === 0) { return null; } From 23408adb7bb55d83e540bfe9a5bf8b5cc8fdff6c Mon Sep 17 00:00:00 2001 From: syuilo <4439005+syuilo@users.noreply.github.com> Date: Wed, 28 Feb 2024 17:11:17 +0900 Subject: [PATCH 47/50] Update packages/backend/src/core/entities/NotificationEntityService.ts --- packages/backend/src/core/entities/NotificationEntityService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index b2fdcb10da27..9fd705c4d328 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -272,7 +272,7 @@ export class NotificationEntityService implements OnModuleInit { const notifier = notifiers.find(x => x.id === notification.notifierId) ?? null; - if (notifier === null) return false; + if (notifier == null) return false; if (notifier.host && userMutedInstances.has(notifier.host)) return false; if (notifier.isSuspended) return false; From bfecc0e1d4ff100cddffc715860a7837dc192014 Mon Sep 17 00:00:00 2001 From: syuilo <4439005+syuilo@users.noreply.github.com> Date: Wed, 28 Feb 2024 17:11:25 +0900 Subject: [PATCH 48/50] Update packages/backend/src/core/entities/NotificationEntityService.ts --- packages/backend/src/core/entities/NotificationEntityService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 9fd705c4d328..36a1cac31513 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -313,7 +313,7 @@ export class NotificationEntityService implements OnModuleInit { const filteredNotifications = ((await Promise.all(notifications.map(async (notification) => { const isValid = await this.#validateNotifier(notification, userIdsWhoMeMuting, userMutedInstances, notifiers); return isValid ? notification : null; - }))) as [T|null] ).filter(isNotNull); + }))) as [T | null] ).filter(isNotNull); return filteredNotifications; } From 01d67e899ee2fd0fff9649dd6a2e1ce52da3b7c8 Mon Sep 17 00:00:00 2001 From: syuilo <4439005+syuilo@users.noreply.github.com> Date: Wed, 28 Feb 2024 17:11:32 +0900 Subject: [PATCH 49/50] Update packages/backend/src/core/NotificationService.ts --- packages/backend/src/core/NotificationService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/NotificationService.ts b/packages/backend/src/core/NotificationService.ts index 0e5f7a24eb70..af5755f88bd7 100644 --- a/packages/backend/src/core/NotificationService.ts +++ b/packages/backend/src/core/NotificationService.ts @@ -163,7 +163,7 @@ export class NotificationService implements OnApplicationShutdown { const packed = await this.notificationEntityService.pack(notification, notifieeId, {}); - if (packed === null) return null; + if (packed == null) return null; // Publish notification event this.globalEventService.publishMainStream(notifieeId, 'notification', packed); From ff9df0dc5c58fd2470b535e0840e2ea480322296 Mon Sep 17 00:00:00 2001 From: taichanne30 Date: Wed, 28 Feb 2024 19:31:49 +0900 Subject: [PATCH 50/50] Clean --- .../backend/src/core/entities/NotificationEntityService.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/core/entities/NotificationEntityService.ts b/packages/backend/src/core/entities/NotificationEntityService.ts index 36a1cac31513..94d56c883bfb 100644 --- a/packages/backend/src/core/entities/NotificationEntityService.ts +++ b/packages/backend/src/core/entities/NotificationEntityService.ts @@ -261,12 +261,12 @@ export class NotificationEntityService implements OnModuleInit { /** * notifierが存在するか、ミュートされていないか、サスペンドされていないかを確認するvalidator */ - async #validateNotifier ( + #validateNotifier ( notification: T, userIdsWhoMeMuting: Set, userMutedInstances: Set, notifiers: MiUser[], - ): Promise { + ): boolean { if (!('notifierId' in notification)) return true; if (userIdsWhoMeMuting.has(notification.notifierId)) return false; @@ -311,7 +311,7 @@ export class NotificationEntityService implements OnModuleInit { }) : []; const filteredNotifications = ((await Promise.all(notifications.map(async (notification) => { - const isValid = await this.#validateNotifier(notification, userIdsWhoMeMuting, userMutedInstances, notifiers); + const isValid = this.#validateNotifier(notification, userIdsWhoMeMuting, userMutedInstances, notifiers); return isValid ? notification : null; }))) as [T | null] ).filter(isNotNull);