BOTイベントを追加

[H1rono]

close #1884
USER_GROUP_CREATED, USER_GROUP_UPDATED, USER_GROUP_DELETED, USER_GROUP_MEMBER_ADDED, USER_GROUP_MEMBER_UPDATED, USER_GROUP_MEMBER_REMOVED, USER_GROUP_ADMIN_ADDED, USER_GROUP_ADMIN_REMOVEDを追加しました

[logica0419]

ちょっと悩んだ点はあったけど、多分可能な範囲で最大限綺麗な実装にはなっていると思います
ありがとう

余裕があれば、テストケースとしてsubscribeしてないbotに配信されないのを確かめるの欲しいな

[logica0419]

あと、1つ疑問点として、このイベントはどのようなユースケースを想定しているのか教えてほしいです

[logica0419]

それを知らずに入れてしまうのは、流石に無責任だなと思ったので

[H1rono]

余裕があれば、テストケースとしてsubscribeしてないbotに配信されないのを確かめるの欲しいな

考えてみます、同じようなテストをしている箇所があったら教えて欲しいです

[H1rono]

あと、1つ疑問点として、このイベントはどのようなユースケースを想定しているのか教えてほしいです

BOTで作成したもの(TODOリストなど)の管理権をユーザーグループに割り当てるとき、とかを想定してます
このような状況ではこのイベントがないとtraQへの無駄なリクエストが大量に発生すると考えました

[logica0419]

余裕があれば、テストケースとしてsubscribeしてないbotに配信されないのを確かめるの欲しいな

考えてみます、同じようなテストをしている箇所があったら教えて欲しいです

TAG_ADDEDとか

[logica0419]

あと、1つ疑問点として、このイベントはどのようなユースケースを想定しているのか教えてほしいです

BOTで作成したもの(TODOリストなど)の管理権をユーザーグループに割り当てるとき、とかを想定してます

このような状況ではこのイベントがないとtraQへの無駄なリクエストが大量に発生すると考えました

なるほど！
とするとグループのメンバー関係のイベントって作らなくていいの？

[H1rono]

USER_GROUP_MEMBER_ADDEDみたいな感じですか？USER_GROUP_UPDATEDが包含してると思ってたんですが...
(イベント発火の条件を詳しく調べてないので良くわかってないです:pray:)

[logica0419]

ここに全てがあります
https://github.com/traPtitech/traQ/blob/7f3ffa1f93372dbfca823a11c972f8c66b54a10d/event/topic.go

[logica0419]

BOTイベントは、traQ全体としてのイベントを受け取って、その中でBOTにも通知すべきものをBOTに流しているという感じなので、traQのイベントの区分がそのまま適応されます

[logica0419]

リポジトリ内で、event.UserCreatedのようにevent/topic.goに書いてあるイベント名を検索すると、どこの部分が実行されたらイベントが発生するかがわかるので、それで確かめてみて下さい

[H1rono]

traQ/event/topic.go

Lines 54 to 57 in 7f3ffa1

	// UserGroupUpdated ユーザーグループが更新された
	// Fields:
	// group_id: uuid.UUID
	UserGroupUpdated = "user_group.updated"

USER_GROUP_UPDATEDのペイロード勘違いしてるのに気づいたので直します、あとグループメンバー/アドミン系のイベント足します

[ikura-hamu]

一応見て、一個だけ気づいたことがあったので、お願いします。

[ikura-hamu]

あと、これがマージされたら、bot-consoleの書き換えは必須になると思います。
traq-botとtraq-ws-botは必須ではないだろうけど、やってくれると嬉しい。

[H1rono]

関係ないテスト落ちた...

--- FAIL: Test_fetchTwitterSyndicationAPI (0.33s)
    --- FAIL: Test_fetchTwitterSyndicationAPI/success (0.30s)
        domain_twitter_test.go:21: 
            	Error Trace:	/home/runner/work/traQ/traQ/service/ogp/parser/domain_twitter_test.go:21
            	            				/home/runner/work/traQ/traQ/service/ogp/parser/domain_twitter_test.go:41
            	Error:      	Not equal: 
            	            	expected: "@_winnie_on は?情緒不安定か?ﾏﾝｺﾞｰうまいぜ"
            	            	actual  : ""
            	            	
            	            	Diff:
            	            	--- Expected
            	            	+++ Actual
            	            	@@ -1 +1 @@
            	            	-@_winnie_on は?情緒不安定か?ﾏﾝｺﾞｰうまいぜ
            	            	+
            	Test:       	Test_fetchTwitterSyndicationAPI/success
    --- FAIL: Test_fetchTwitterSyndicationAPI/not_found (0.03s)
        domain_twitter_test.go:31: 
            	Error Trace:	/home/runner/work/traQ/traQ/service/ogp/parser/domain_twitter_test.go:31
            	            				/home/runner/work/traQ/traQ/service/ogp/parser/domain_twitter_test.go:38
            	Error:      	Target error should be in err chain:
            	            	expected: "network error (client)"
            	            	in chain: 
            	Test:       	Test_fetchTwitterSyndicationAPI/not_found
            	Messages:   	fetchTwitterSyndicationAPI(990696508403040257)
FAIL
	github.com/traPtitech/traQ/service/ogp/parser	coverage: 35.1% of statements
FAIL	github.com/traPtitech/traQ/service/ogp/parser	0.5[82](https://github.com/traPtitech/traQ/actions/runs/5862942891/job/15895577605?pr=1894#step:6:83)s

[H1rono]

approveもらったらbot-consoleとtraq-botとtraq-ws-botにもPR出す予定です

[ikura-hamu]

1個だけお願いします。

テストについては、 #t/S/t/Serverにある通り、 t.SkipNow()がいいと思いますが、このプルリクでやることではない気がするのでちょっと待ってください。

[logica0419]

テストはmainブランチではもう直ってるので、mainをrebaseかmergeして解決してください

[H1rono]

masterをmergeしました

[ikura-hamu]

いいと思います。ありがとう！

[H1rono]

https://github.com/H1rono/traQ/blob/4523aee8ca8cf21dccc87294910ffe7747bcad68/service/bot/event/payload/ev_user_group_created.go#L10-L13

bot-consoleのドキュメント書こうとして気づいたんですが、model.UserGroupそのまま返すのってもしかしてやばいですか？(ペイロードが肥大化して見せてはいけない情報が入ってしまう可能性がある)

[H1rono]

あとUSER_GROUP_UPDATEDとUSER_GROUP_DELETEDのペイロード型がちぐはぐなので直します

https://github.com/H1rono/traQ/blob/4523aee8ca8cf21dccc87294910ffe7747bcad68/service/bot/event/payload/ev_user_group_updated.go#L10-L13
https://github.com/H1rono/traQ/blob/4523aee8ca8cf21dccc87294910ffe7747bcad68/service/bot/event/payload/ev_user_group_deleted.go#L11-L16

[ikura-hamu]

確かにmodelそのまま返すのは良くなかったかも。
いいと思います。引き続きお願いします

[H1rono]

typo修正

[ikura-hamu]

さんざん「よさそう」って言っておいて申し訳ないんですが、UserGroupMemberUpdatedとUserGroupMemberAddedの違いは何でしょうか。
名前的にはupdatedの方はメンバーが削除されたときも発行されそうですが、今はremovedだけ発行されていそうです。

もしupdatedをメンバー削除のときも発行するなら、グループメンバー全員を乗せるのがいいのかな

[H1rono]

対応するtraQ内イベントは↓で定義されています。

traQ/event/topic.go

Lines 62 to 76 in 4306a93

	// UserGroupMemberAdded ユーザーがグループに追加された
	// Fields:
	// group_id: uuid.UUID
	// user_id: uuid.UUID
	UserGroupMemberAdded = "user_group.member.added"
	// UserGroupMemberUpdated ユーザーグループメンバーが更新された
	// Fields:
	// group_id: uuid.UUID
	// user_id: uuid.UUID
	UserGroupMemberUpdated = "user_group.member.updated"
	// UserGroupMemberRemoved ユーザーがグループから削除された
	// Fields:
	// group_id: uuid.UUID
	// user_id: uuid.UUID
	UserGroupMemberRemoved = "user_group.member.removed"

コメントに従えば、イベント発行のタイミングは次のようになっているはずです。

• UserGroupMemberUpdated: グループメンバーの中で一人が更新(=役割変更)されたとき
• UserGroupMemberRemoved: グループメンバーの中で一人がグループから削除されたとき

[ikura-hamu]

repository/user_group.go の実装を読み違っていました。
この実装であれば、UserGroupMemberUpdatedは、GroupMemberではなくメンバーのロールが含まれているUserGroupMemberを送るのが親切そうです。

[H1rono]

traQ/event/topic.go

Lines 67 to 71 in 0642dc8

	// UserGroupMemberUpdated ユーザーグループメンバーが更新された
	// Fields:
	// group_id: uuid.UUID
	// user_id: uuid.UUID
	UserGroupMemberUpdated = "user_group.member.updated"

ここのコメントを見れば分かる通り、イベントのフィールドにrole情報が含まれていません。なのでこのイベントにrole情報も載せようとなるとイベント発行側の実装もいじる必要が出てきて面倒になります

traQ/service/bot/handler/ev_user_group_member_updated.go

Lines 14 to 16 in 0642dc8

	func UserGroupMemberUpdated(ctx Context, datetime time.Time, _ string, fields hub.Fields) error {
	groupID := fields["group_id"].(uuid.UUID)
	userID := fields["user_id"].(uuid.UUID)

[ikura-hamu]

あれ、まだ実装読み違ってるかな。インターン終わったらじっくり見ます。長引いてごめん:pray:

botへのイベント発行の流れをざっくりでいいから書いておいてもらえると助かる。

[H1rono]

1: hubでイベントがpublishされます

traQ/repository/gorm/user_group.go

Lines 221 to 227 in 4306a93

	repo.hub.Publish(hub.Message{
	Name: event.UserGroupMemberUpdated,
	Fields: hub.Fields{
	"group_id": groupID,
	"user_id": userID,
	},
	})

2: 対応するイベントハンドラが呼ばれます

traQ/service/bot/handlers.go

Line 32 in 0642dc8

intevent.UserGroupMemberUpdated: handler.UserGroupMemberUpdated,

traQ/service/bot/handler/ev_user_group_member_updated.go

Lines 14 to 33 in 0642dc8

	func UserGroupMemberUpdated(ctx Context, datetime time.Time, _ string, fields hub.Fields) error {
	groupID := fields["group_id"].(uuid.UUID)
	userID := fields["user_id"].(uuid.UUID)
	bots, err := ctx.GetBots(event.UserGroupMemberUpdated)
	if err != nil {
	return fmt.Errorf("failed to GetBots: %w", err)
	}
	if len(bots) == 0 {
	return nil
	}
	if err := ctx.Multicast(
	event.UserGroupMemberUpdated,
	payload.MakeUserGroupMemberUpdated(datetime, groupID, userID),
	bots,
	); err != nil {
	return fmt.Errorf("failed to multicast: %w", err)
	}
	return nil
	}

3: handler.Context.Multicastからevent.Multicastが呼ばれます

traQ/service/bot/service_impl.go

Lines 135 to 137 in 0642dc8

	func (p *serviceImpl) Multicast(ev model.BotEventType, payload interface{}, targets []*model.Bot) error {
	return event.Multicast(p.dispatcher, ev, payload, targets)
	}

traQ/service/bot/event/dispatcher.go

Lines 28 to 53 in 0642dc8

	func Multicast(d Dispatcher, ev model.BotEventType, payload interface{}, targets []*model.Bot) error {
	if len(targets) == 0 {
	return nil
	}
	buf, release, err := makePayloadJSON(&payload)
	if err != nil {
	return err
	}
	defer release()
	var wg sync.WaitGroup
	done := make(map[uuid.UUID]bool, len(targets))
	for _, bot := range targets {
	if !done[bot.ID] {
	done[bot.ID] = true
	bot := bot
	wg.Add(1)
	go func() {
	defer wg.Done()
	d.Send(bot, ev, buf)
	}()
	}
	}
	wg.Wait()
	return nil
	}

4: event.Dispatcher.SendでBOTにペイロードが送信されます

traQ/service/bot/event/dispatcher_impl.go

Lines 42 to 57 in 0642dc8

	func (d *dispatcherImpl) Send(b *model.Bot, event model.BotEventType, body []byte) (ok bool) {
	reqID := uuid.Must(uuid.NewV4())
	var log *model.BotEventLog
	switch b.Mode {
	case model.BotModeHTTP:
	ok, log = d.http.send(b, event, reqID, body)
	case model.BotModeWebSocket:
	ok, log = d.ws.send(b, event, reqID, body)
	default:
	return false
	}
	d.writeLog(log)
	return ok
	}

[ikura-hamu]

ありがとう、理解しました。今回いじってない repository/gorm/user_group を変えなきゃいけないってことね。

全体に発行するイベントの変更はこのPRでやることではないし、イベント受け取ったらAPIを叩くようにすれば新しいroleは取れるので、roleの情報を乗せるのは要望が出てからでもいいと思ってきました。

approveしておきます。

[ikura-hamu]

長引いてごめん。bot-consoleとbot系ライブラリも見ます。