diff --git a/packages/backend/src/core/AbuseReportNotificationService.ts b/packages/backend/src/core/AbuseReportNotificationService.ts index 742e2621fd..69115e6c1e 100644 --- a/packages/backend/src/core/AbuseReportNotificationService.ts +++ b/packages/backend/src/core/AbuseReportNotificationService.ts @@ -160,22 +160,14 @@ export class AbuseReportNotificationService implements OnApplicationShutdown { }; }); - const recipientWebhookIds = await this.fetchWebhookRecipients() - .then(it => it - .filter(it => it.isActive && it.systemWebhookId && it.method === 'webhook') - .map(it => it.systemWebhookId) - .filter(x => x != null)); - for (const webhookId of recipientWebhookIds) { - await Promise.all( - convertedReports.map(it => { - return this.systemWebhookService.enqueueSystemWebhook( - webhookId, - type, - it, - ); - }), - ); - } + return Promise.all( + convertedReports.map(it => { + return this.systemWebhookService.enqueueSystemWebhook( + type, + it, + ); + }), + ); } /** diff --git a/packages/backend/src/core/SystemWebhookService.ts b/packages/backend/src/core/SystemWebhookService.ts index de00169612..bc383c1f68 100644 --- a/packages/backend/src/core/SystemWebhookService.ts +++ b/packages/backend/src/core/SystemWebhookService.ts @@ -50,7 +50,6 @@ export type SystemWebhookPayload = @Injectable() export class SystemWebhookService implements OnApplicationShutdown { - private logger: Logger; private activeSystemWebhooksFetched = false; private activeSystemWebhooks: MiSystemWebhook[] = []; @@ -62,11 +61,9 @@ export class SystemWebhookService implements OnApplicationShutdown { private idService: IdService, private queueService: QueueService, private moderationLogService: ModerationLogService, - private loggerService: LoggerService, private globalEventService: GlobalEventService, ) { this.redisForSub.on('message', this.onMessage); - this.logger = this.loggerService.getLogger('webhook'); } @bindThis @@ -193,28 +190,19 @@ export class SystemWebhookService implements OnApplicationShutdown { /** * SystemWebhook をWebhook配送キューに追加する * @see QueueService.systemWebhookDeliver - * // TODO: contentの型を厳格化する */ @bindThis public async enqueueSystemWebhook( - webhook: MiSystemWebhook | MiSystemWebhook['id'], type: T, content: SystemWebhookPayload, ) { - const webhookEntity = typeof webhook === 'string' - ? (await this.fetchActiveSystemWebhooks()).find(a => a.id === webhook) - : webhook; - if (!webhookEntity || !webhookEntity.isActive) { - this.logger.info(`SystemWebhook is not active or not found : ${webhook}`); - return; - } - - if (!webhookEntity.on.includes(type)) { - this.logger.info(`SystemWebhook ${webhookEntity.id} is not listening to ${type}`); - return; - } - - return this.queueService.systemWebhookDeliver(webhookEntity, type, content); + const webhooks = await this.fetchActiveSystemWebhooks() + .then(webhooks => webhooks.filter(webhook => webhook.on.includes(type))); + return Promise.all( + webhooks.map(webhook => { + return this.queueService.systemWebhookDeliver(webhook, type, content); + }), + ); } @bindThis diff --git a/packages/backend/src/core/UserService.ts b/packages/backend/src/core/UserService.ts index 9b1961c631..1f471513f3 100644 --- a/packages/backend/src/core/UserService.ts +++ b/packages/backend/src/core/UserService.ts @@ -63,13 +63,6 @@ export class UserService { @bindThis public async notifySystemWebhook(user: MiUser, type: 'userCreated') { const packedUser = await this.userEntityService.pack(user, null, { schema: 'UserLite' }); - const recipientWebhookIds = await this.systemWebhookService.fetchSystemWebhooks({ isActive: true, on: [type] }); - for (const webhookId of recipientWebhookIds) { - await this.systemWebhookService.enqueueSystemWebhook( - webhookId, - type, - packedUser, - ); - } + return this.systemWebhookService.enqueueSystemWebhook(type, packedUser); } } diff --git a/packages/backend/src/queue/processors/CheckModeratorsActivityProcessorService.ts b/packages/backend/src/queue/processors/CheckModeratorsActivityProcessorService.ts index 87183cb342..2e84430e72 100644 --- a/packages/backend/src/queue/processors/CheckModeratorsActivityProcessorService.ts +++ b/packages/backend/src/queue/processors/CheckModeratorsActivityProcessorService.ts @@ -231,15 +231,10 @@ export class CheckModeratorsActivityProcessorService { // -- SystemWebhook - const systemWebhooks = await this.systemWebhookService.fetchActiveSystemWebhooks() - .then(it => it.filter(it => it.on.includes('inactiveModeratorsWarning'))); - for (const systemWebhook of systemWebhooks) { - this.systemWebhookService.enqueueSystemWebhook( - systemWebhook, - 'inactiveModeratorsWarning', - { remainingTime: remainingTime }, - ); - } + return this.systemWebhookService.enqueueSystemWebhook( + 'inactiveModeratorsWarning', + { remainingTime: remainingTime }, + ); } @bindThis @@ -269,15 +264,10 @@ export class CheckModeratorsActivityProcessorService { // -- SystemWebhook - const systemWebhooks = await this.systemWebhookService.fetchActiveSystemWebhooks() - .then(it => it.filter(it => it.on.includes('inactiveModeratorsInvitationOnlyChanged'))); - for (const systemWebhook of systemWebhooks) { - this.systemWebhookService.enqueueSystemWebhook( - systemWebhook, - 'inactiveModeratorsInvitationOnlyChanged', - {}, - ); - } + return this.systemWebhookService.enqueueSystemWebhook( + 'inactiveModeratorsInvitationOnlyChanged', + {}, + ); } @bindThis diff --git a/packages/backend/test/unit/SystemWebhookService.ts b/packages/backend/test/unit/SystemWebhookService.ts index 5401dd74d8..9488e18f38 100644 --- a/packages/backend/test/unit/SystemWebhookService.ts +++ b/packages/backend/test/unit/SystemWebhookService.ts @@ -314,9 +314,10 @@ describe('SystemWebhookService', () => { isActive: true, on: ['abuseReport'], }); - await service.enqueueSystemWebhook(webhook.id, 'abuseReport', { foo: 'bar' } as any); + await service.enqueueSystemWebhook('abuseReport', { foo: 'bar' } as any); - expect(queueService.systemWebhookDeliver).toHaveBeenCalled(); + expect(queueService.systemWebhookDeliver).toHaveBeenCalledTimes(1); + expect(queueService.systemWebhookDeliver.mock.calls[0][0] as MiSystemWebhook).toEqual(webhook); }); test('非アクティブなWebhookはキューに追加されない', async () => { @@ -324,7 +325,7 @@ describe('SystemWebhookService', () => { isActive: false, on: ['abuseReport'], }); - await service.enqueueSystemWebhook(webhook.id, 'abuseReport', { foo: 'bar' } as any); + await service.enqueueSystemWebhook('abuseReport', { foo: 'bar' } as any); expect(queueService.systemWebhookDeliver).not.toHaveBeenCalled(); }); @@ -338,11 +339,33 @@ describe('SystemWebhookService', () => { isActive: true, on: ['abuseReportResolved'], }); - await service.enqueueSystemWebhook(webhook1.id, 'abuseReport', { foo: 'bar' } as any); - await service.enqueueSystemWebhook(webhook2.id, 'abuseReport', { foo: 'bar' } as any); + await service.enqueueSystemWebhook('abuseReport', { foo: 'bar' } as any); expect(queueService.systemWebhookDeliver).not.toHaveBeenCalled(); }); + + test('混在した時、有効かつ許可されたイベント種別のみ', async () => { + const webhook1 = await createWebhook({ + isActive: true, + on: ['abuseReport'], + }); + const webhook2 = await createWebhook({ + isActive: true, + on: ['abuseReportResolved'], + }); + const webhook3 = await createWebhook({ + isActive: false, + on: ['abuseReport'], + }); + const webhook4 = await createWebhook({ + isActive: false, + on: ['abuseReportResolved'], + }); + await service.enqueueSystemWebhook('abuseReport', { foo: 'bar' } as any); + + expect(queueService.systemWebhookDeliver).toHaveBeenCalledTimes(1); + expect(queueService.systemWebhookDeliver.mock.calls[0][0] as MiSystemWebhook).toEqual(webhook1); + }); }); describe('fetchActiveSystemWebhooks', () => { diff --git a/packages/backend/test/unit/UserWebhookService.ts b/packages/backend/test/unit/UserWebhookService.ts index 0e88835a02..db8f96df28 100644 --- a/packages/backend/test/unit/UserWebhookService.ts +++ b/packages/backend/test/unit/UserWebhookService.ts @@ -1,4 +1,3 @@ - /* * SPDX-FileCopyrightText: syuilo and misskey-project * SPDX-License-Identifier: AGPL-3.0-only @@ -71,7 +70,7 @@ describe('UserWebhookService', () => { LoggerService, GlobalEventService, { - provide: QueueService, useFactory: () => ({ systemWebhookDeliver: jest.fn() }), + provide: QueueService, useFactory: () => ({ userWebhookDeliver: jest.fn() }), }, ], }) @@ -242,4 +241,92 @@ describe('UserWebhookService', () => { }); }); }); + + describe('アプリを毎回作り直す必要があるグループ', () => { + beforeEach(async () => { + await beforeAllImpl(); + await beforeEachImpl(); + }); + + afterEach(async () => { + await afterEachImpl(); + await afterAllImpl(); + }); + + describe('enqueueUserWebhook', () => { + test('キューに追加成功', async () => { + const webhook = await createWebhook({ + active: true, + on: ['note'], + }); + await service.enqueueUserWebhook(webhook.userId, 'note', { foo: 'bar' } as any); + + expect(queueService.userWebhookDeliver).toHaveBeenCalledTimes(1); + expect(queueService.userWebhookDeliver.mock.calls[0][0] as MiWebhook).toEqual(webhook); + }); + + test('非アクティブなWebhookはキューに追加されない', async () => { + const webhook = await createWebhook({ + active: false, + on: ['note'], + }); + await service.enqueueUserWebhook(webhook.userId, 'note', { foo: 'bar' } as any); + + expect(queueService.userWebhookDeliver).not.toHaveBeenCalled(); + }); + + test('未許可のイベント種別が渡された場合はWebhookはキューに追加されない', async () => { + const webhook1 = await createWebhook({ + active: true, + on: [], + }); + const webhook2 = await createWebhook({ + active: true, + on: ['note'], + }); + await service.enqueueUserWebhook(webhook1.userId, 'renote', { foo: 'bar' } as any); + await service.enqueueUserWebhook(webhook2.userId, 'renote', { foo: 'bar' } as any); + + expect(queueService.userWebhookDeliver).not.toHaveBeenCalled(); + }); + + test('ユーザIDが異なるWebhookはキューに追加されない', async () => { + const webhook = await createWebhook({ + active: true, + on: ['note'], + }); + await service.enqueueUserWebhook(idService.gen(), 'note', { foo: 'bar' } as any); + + expect(queueService.userWebhookDeliver).not.toHaveBeenCalled(); + }); + + test('混在した時、有効かつ許可されたイベント種別のみ', async () => { + const userId = root.id; + const webhook1 = await createWebhook({ + userId, + active: true, + on: ['note'], + }); + const webhook2 = await createWebhook({ + userId, + active: true, + on: ['renote'], + }); + const webhook3 = await createWebhook({ + userId, + active: false, + on: ['note'], + }); + const webhook4 = await createWebhook({ + userId, + active: false, + on: ['renote'], + }); + await service.enqueueUserWebhook(userId, 'note', { foo: 'bar' } as any); + + expect(queueService.userWebhookDeliver).toHaveBeenCalledTimes(1); + expect(queueService.userWebhookDeliver.mock.calls[0][0] as MiWebhook).toEqual(webhook1); + }); + }); + }); }); diff --git a/packages/backend/test/unit/queue/processors/CheckModeratorsActivityProcessorService.ts b/packages/backend/test/unit/queue/processors/CheckModeratorsActivityProcessorService.ts index 1506283a3c..d96e6b916a 100644 --- a/packages/backend/test/unit/queue/processors/CheckModeratorsActivityProcessorService.ts +++ b/packages/backend/test/unit/queue/processors/CheckModeratorsActivityProcessorService.ts @@ -18,6 +18,7 @@ import { QueueLoggerService } from '@/queue/QueueLoggerService.js'; import { EmailService } from '@/core/EmailService.js'; import { SystemWebhookService } from '@/core/SystemWebhookService.js'; import { AnnouncementService } from '@/core/AnnouncementService.js'; +import { SystemWebhookEventType } from '@/models/SystemWebhook.js'; const baseDate = new Date(Date.UTC(2000, 11, 15, 12, 0, 0)); @@ -334,9 +335,10 @@ describe('CheckModeratorsActivityProcessorService', () => { mockModeratorRole([user1]); await service.notifyInactiveModeratorsWarning({ time: 1, asDays: 0, asHours: 0 }); - expect(systemWebhookService.enqueueSystemWebhook).toHaveBeenCalledTimes(2); - expect(systemWebhookService.enqueueSystemWebhook.mock.calls[0][0]).toEqual(systemWebhook1); - expect(systemWebhookService.enqueueSystemWebhook.mock.calls[1][0]).toEqual(systemWebhook2); + // typeとactiveによる絞り込みが機能しているかはSystemWebhookServiceのテストで確認する. + // ここでは呼び出されているか、typeが正しいかのみを確認する + expect(systemWebhookService.enqueueSystemWebhook).toHaveBeenCalledTimes(1); + expect(systemWebhookService.enqueueSystemWebhook.mock.calls[0][0] as SystemWebhookEventType).toEqual('inactiveModeratorsWarning'); }); }); @@ -372,8 +374,10 @@ describe('CheckModeratorsActivityProcessorService', () => { mockModeratorRole([user1]); await service.notifyChangeToInvitationOnly(); + // typeとactiveによる絞り込みが機能しているかはSystemWebhookServiceのテストで確認する. + // ここでは呼び出されているか、typeが正しいかのみを確認する expect(systemWebhookService.enqueueSystemWebhook).toHaveBeenCalledTimes(1); - expect(systemWebhookService.enqueueSystemWebhook.mock.calls[0][0]).toEqual(systemWebhook2); + expect(systemWebhookService.enqueueSystemWebhook.mock.calls[0][0] as SystemWebhookEventType).toEqual('inactiveModeratorsInvitationOnlyChanged'); }); }); });