refactor: 예약 메세지 기능 수정 #37
Conversation
- 파싱 수정 - 멤버 id db 저장 시 백틱처리
TaeyeonRoyce
added
the
refactor
There was a problem hiding this comment.
고생 많으셨습니다!
고민 끝에 더 나은 방식과 더 나은 사용자 경험을 완성 하셨네요. 덕분에 해당 기능을 자주 사용하게 될 것 같아요!
리뷰 남겼으니 확인 해주세요~
질문에 대한 답변
슬래시커멘드 + 드롭다운 + 모달 세가지 형태로 입력받는 것이 오히려 사용에 불편함을 주지 않을지
사용 하는데 불편함은 없어보여요! 오히려 단계별로 잘 구분 되어서 사용 하는 입장에서 더 쉽게 쓸 수 있을 것 같습니다.리스너를 세개로 분리했는데 이에 대해 적절한지(잘 분리되었는지) 의견이 궁금합니다!
필요에 맞게 잘 분리해주셨어요. 기존 디코봇은 slashCommand에 대한 상호작용만 처리하는 listener였는데요. 각 입력 소스에 맞추어 listener를 분리하여 구현 하는게 적절한 책임이라 생각했어요. 추가된modal 입력과드롭다운 선택을 통한 입력을 담당하는 각각의 객체로 분리하는건 좋은 것 같습니다.👍
다만, 현재 구현된ScheduledMessageModalLauncher와ScheduledMessageSubmitListener는 "해당 형태의 모든 입력을 처리" 하고 있습니다.
만약, 디코봇이 여러 드롭다운이나 모달을 지원한다면SlashCommandListenerMapper와 같은 미들웨어가 필요하게 될 거에요...!
SlashCommandListenerMapper의 경우, 입력 받은 slash 명령어에 따라 분기처리 하여 적절한 CommandListener로 요청을 보냅니다. ex) reviewer-match ->
ReviewMatchListener.onAction(), fortune-today ->FortuneTodayListener.onAction()
이처럼 모달이나 드롭다운 입력이 다양해지면 입력값에 따른 분기를 처리하는 미들웨어가 필요해집니다!
- ListenerAdapter를 전부 등록하는 방식이 맞는지, 아니면 기존 구조를 활용해야하는지 의문이 드는데 어떻게 생각하시나요?
코드에 남겨두긴 했는데, 결론적으론 전부 등록하는 것 보단 기존 방식으로 하는게 좋아보여요!
|
|
||
| List<Object> listeners = new ArrayList<>(); | ||
| listeners.add(slashCommandListenerMapper); | ||
| listeners.addAll(applicationContext.getBeansOfType(ListenerAdapter.class).values()); // 하드코딩 |
There was a problem hiding this comment.
ApplicationContext에서 빈을 조회하지 말고, jda listener로 등록할 빈을 직접 지정하여 주입 받는게 나아보여요.
ListnerAdapter를 구현한 모든 빈이 등록 되는 방식은 적용 범위가 넓어서 추후 제어하기에 불편 해보이네요.
추가된 ScheduledMessageModalLauncher, ScheduledMessageSubmitListener 모두 빈으로 등록 되어 있으니 상단의 listeners.add(slashCommandListenerMapper)처럼 직접 주입 받아서 사용 하시죠!
There was a problem hiding this comment.
JdaConfiguration 클래스 안에서
ListenerAdapter를 상속받은 모든 클래스를 한꺼번에 등록하는 방식(=기존 방식)을 사용하지 말고
return JDABuilder.createLight(token)
...
.addEventListeners(
slashCommandListenerMapper, // 기존
scheduledMessageModalLauncher, // 추가
scheduledMessageSubmitListener // 추가
)
....
이렇게 직접 클래스를 넣으라는 말로 이해했는데 맞을까요?
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Component | ||
| public class ScheduledMessageCommandListener implements AutoCompleteInteractionListener { |
There was a problem hiding this comment.
이제 채널 지정은 드롭다운으로 변경 되었으니 자동완성을 지원하는AutoCompleteInteractionListener가 아닌 SlashCommandListener로 변경 하시죠! 적합한 책임을 가지는 객체로 정의하는게 좋을 것 같아요.
하단의 AutoCompleteInteractionListener의 메서드인 onCommandAutoCompleteInteraction()도 필요 없어지겠네요!
| try { | ||
| String[] parts = event.getModalId().split(":", 3); | ||
|
|
||
| final ScheduledMessageChannel channelEnum = ScheduledMessageChannel.valueOf(parts[1]); |
There was a problem hiding this comment.
split으로 생성된 array의 각 위치별 값이 설명이 되어 있으면 좋아요!
| final ScheduledMessageChannel channelEnum = ScheduledMessageChannel.valueOf(parts[1]); | |
| String channelName = parts[1]; | |
| final ScheduledMessageChannel channelEnum = ScheduledMessageChannel.valueOf(channelName); |
| .addEventListeners(slashCommandListenerMapper) | ||
| .addEventListeners( // 수동 입력 | ||
| slashCommandListenerMapper, | ||
| scheduledMessageSubmitListener |
There was a problem hiding this comment.
빈 등록 하나가 누락 되었는데, 이것만 추가 하고 머지하시죠!
| scheduledMessageSubmitListener | |
| scheduledMessageSubmitListener, | |
| scheduledMessageModalLauncher |
There was a problem hiding this comment.
이 부분 관련해서 질문이 있습니다🤔
ScheduledMessageModalLauncher를 Listener에 등록할 경우 모달 제출 시 다음과 같은 에러가 발생했습니다:
[ErrorResponseException] 10062: Failed to acknowledge this interaction
net.dv8tion.jda.api.exceptions.ErrorResponseException: 10062: Unknown interaction
이 에러는 하나의 Interaction을 두 개의 Listener(ScheduledMessageModalLauncher, ScheduledMessageSubmitListener)가 동시에 수신하면서 중복 응답이 발생했기 때문입니다.
그래서 실제로 이벤트를 처리해야 하는 SlashCommandListenerMapper(=명령어 등록)와 ScheduledMessageSubmitListener(=모달 제출) 두 개 Bean만 등록하도록 수정했습니다.
(이렇게 수정했을 시 정상 작동은 하면서 위의 오류가 발생하지 않습니다!)
There was a problem hiding this comment.
많은 리스너들 중 왜 하필 저 두 리스너만 충돌이 일어날까?에 대해 찾아보았으나 충분한 이해가 되지 않습니다....🤯
일단 플로우를 찾아보았을 때 아래와 같다고 하네요. 🚨 표시된 곳이 문제가 일어나는 부분들인 것 같은데 알맞게 이해한 것인지 태연님 생각이 궁금합니다!
① 드롭다운 선택 → 모달 띄우기 (ScheduledMessageModalLauncher)
@Override
public void onStringSelectInteraction(@NotNull StringSelectInteractionEvent event) {
// 모달 생성
// 첫번째 ACK
event.replyModal(modal).queue();
}JDA가 Discord에 첫 번째 응답 전송
"사용자가 드롭다운을 선택 → 모달을 띄워줬으니 이벤트 응답 완료!"
② 사용자가 모달에 메시지/시간을 입력하고 제출함
ScheduledMessageSubmitListener 에서 ModalInteractionEvent 발생
→ 🚨🚨🚨 새로운 Interaction(ModalInteractionEvent)을 모든 ListenerAdapter에게 broadcast
③ ScheduledMessageSubmitListener 가 이벤트를 받아 처리
@Override
public void onModalInteraction(@NotNull ModalInteractionEvent event) {
// 올바른 모달 ID면 예약 메시지 등록 처리
// 메시지 파싱 + 유효성 검증
// 성공적으로 저장 후 Discord에 응답
event.reply("✅ 메시지가 예약되었습니다.").queue(); // 두 번째 ACK
}④ ⚠️ 하지만 ScheduledMessageModalLauncher 도 ListenerAdapter 이기 때문에
🚨🚨🚨 JDA는 이 ModalInteractionEvent 를 ScheduledMessageModalLauncher에게도 전달
-> 내부적으로
[JDA 내부 이벤트 브로드캐스트]
├── ScheduledMessageSubmitListener.onModalInteraction(event)
└── ScheduledMessageModalLauncher.onModalInteraction(event?) ← 중복
⑤ Discord 입장에서는 중복 응답
ScheduledMessageSubmitListener→event.reply("✅ 메시지가...")ScheduledMessageModalLauncher→ 내부적으로 이미event.replyModal()로 응답한 적 있음
→ Discord API는 해당 이벤트가 이미 응답된 Interaction임을 감지
→ 에러 발생
[ErrorResponseException] 10062: Failed to acknowledge this interaction
40060: Interaction already acknowledged
2 participants
작업 내용
👴🏻 기존 실행 방식
해당 형태는 다음과 같은 단점이 있습니다.
#등 마크다운 사용 불가따라서 일부 옵션 입력은 모달에서 입력하는 것으로 대체해 위와 같은 문제를 해결하고자 했습니다.
👶🏻 변경 후 실행 방식
참고 사항
기존 ScheduledCommandListener 하나로 작동하던 CommandListener를 세개로 분리했습니다.
🌊 플로우
ScheduledMessageCommandListener가 실행되어 멤버 옵션 파싱 및 채널 선택 드롭다운 표시ScheduledMessageModalLauncher가 실행되어 메시지/시간 입력 모달을 띄움ScheduledMessageSubmitListener가 실행되어 입력값 검증 후 예약 메시지를 DB에 저장⚙️ 기능
각 클래스의 기능은 다음과 같습니다
ScheduledMessageCommandListenerScheduledMessageModalLauncherScheduledMessageSubmitListener예약메세지 db채널에 저장ScheduledMessageScheduler🤔 질문사항
리스너를 세개로 분리했는데 이에 대해 적절한지(잘 분리되었는지) 의견이 궁금합니다! 또한 슬래시커멘드 + 드롭다운 + 모달 세가지 형태로 입력받는 것이 오히려 사용에 불편함을 주지 않을지또한 궁금합니다.
JdaConfiguration 클래스에 관한 질문
다만 기존의 SlashCommandListenerMapper 자체도 ListenerAdapter를 상속받고 있는데, 지금처럼 상위 클래스인 ListenerAdapter를 전부 등록하는 방식이 맞는지, 아니면 기존 구조를 활용해야하는지 의문이 드는데 어떻게 생각하시나요?
관련 이슈
PR 체크리스트