Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[refactor] UserWebhookの配信処理をService化 #15003

Open
1 task done
samunohito opened this issue Nov 19, 2024 · 1 comment · May be fixed by #15035
Open
1 task done

[refactor] UserWebhookの配信処理をService化 #15003

samunohito opened this issue Nov 19, 2024 · 1 comment · May be fixed by #15035
Assignees
Labels
packages/backend Server side specific issue/PR 💚Refactor Rewriting code without changing behavior

Comments

@samunohito
Copy link
Member

samunohito commented Nov 19, 2024

Summary

UserWebhookを送出用のキューに積む処理が随所でむき出しになっているので、これをUserWebhookServiceにまとめたいです。
↓こんなの

const webhooks = (await this.webhookService.getActiveWebhooks()).filter(x => x.userId === data.renote!.userId && x.on.includes('renote'));
for (const webhook of webhooks) {
this.queueService.userWebhookDeliver(webhook, 'renote', {
note: noteObj,
});
}

具体的なイメージとしては、SystemWebhookServiceの下記処理と同等の構造にします。に近い形となります。

public async enqueueSystemWebhook<T extends SystemWebhookEventType>(
webhook: MiSystemWebhook | MiSystemWebhook['id'],
type: T,
content: unknown,
) {
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);
}

アクティブなWebhookを取得し、それら全件に向かって発信するのはどこも同じなので、以下のように出来ると良いと思っています

	public async enqueueUserWebhook<T extends WebhookEventTypes>(
		type: T,
		content: UserWebhookPayload<T>,
	) {
		const webhooks = await this.getActiveWebhooks()
			.then(webhooks => webhooks.filter(webhook => webhook.on.includes(type)));
		return Promise.all(
			webhooks.map(webhook => {
				return this.queueService.userWebhookDeliver(webhook, type, content);
			}),
		);
	}

Purpose

  • 積む処理周辺の見通しがよくなる(特にNoteCreateService)
  • UserWebhook絡みの影響範囲調査をするときに影響範囲調査が楽になる(UserWebhookServiceにまとまるので)

Do you want to implement this feature yourself?

  • Yes, I will implement this by myself and send a pull request
@samunohito samunohito added packages/backend Server side specific issue/PR 💚Refactor Rewriting code without changing behavior labels Nov 19, 2024
@samunohito samunohito self-assigned this Nov 19, 2024
@samunohito
Copy link
Member Author

(SystemWebhookServiceも改良の余地あるのでそっちもやろうと思います)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR 💚Refactor Rewriting code without changing behavior
Projects
Development

Successfully merging a pull request may close this issue.

1 participant