-
Notifications
You must be signed in to change notification settings - Fork 2
feat: 응원톡 ai 필터링 비동기 처리 로직 구현 #417
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
Conversation
Summary of ChangesHello @hyobin-yang, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 응원톡에 대한 AI 기반 필터링의 비동기 처리 로직을 구현합니다. 새로운 이벤트 핸들러를 도입하여 응원톡 생성 이벤트를 비동기적으로 처리하고, 임시 AI 필터를 적용하여 부적절한 메시지를 차단합니다. 이는 실제 AI 모델을 통합하기 전에 비동기 이벤트 처리 흐름을 검증하기 위한 준비 단계입니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
안녕하세요. 응원톡 AI 필터링을 위한 비동기 처리 로직 구현 PR 잘 보았습니다. Spring의 이벤트 리스너와 @Async를 활용하여 비동기 흐름을 구성한 점이 좋습니다. 실제 모델 연동을 위한 좋은 기반이 될 것 같습니다. 코드 리뷰에서는 비동기 이벤트 핸들러의 트랜잭션 처리, 엔티티 관리 방식, 예외 처리 등 안정성을 높이기 위한 몇 가지 제안 사항을 포함했습니다. 또한, 코드 가독성을 향상시킬 수 있는 부분에 대해서도 의견을 남겼습니다.
| private void blockCheerTalk(CheerTalk cheerTalk, CheerTalkCreateEvent event) { | ||
| cheerTalk.block(); | ||
| cheerTalkRepository.save(cheerTalk); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| import com.sports.server.command.cheertalk.domain.CheerTalkCreateEvent; | ||
| import com.sports.server.command.cheertalk.domain.CheerTalkRepository; | ||
| import com.sports.server.command.cheertalk.domain.LanguageFilter; | ||
| import com.sports.server.command.cheertalk.domain.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
와일드카드(*)를 사용한 임포트는 어떤 클래스가 사용되는지 명확히 알기 어렵게 만들고, 이름 충돌의 가능성이 있어 일반적으로 권장되지 않습니다. 각 클래스를 명시적으로 임포트하여 코드 가독성을 높이는 것이 좋습니다.
| import com.sports.server.command.cheertalk.domain.*; | |
| import com.sports.server.command.cheertalk.domain.CheerTalk; | |
| import com.sports.server.command.cheertalk.domain.CheerTalkCreateEvent; | |
| import com.sports.server.command.cheertalk.domain.CheerTalkFilterResult; | |
| import com.sports.server.command.cheertalk.domain.CheerTalkRepository; | |
| import com.sports.server.command.cheertalk.domain.LanguageFilter; |
| log.debug( | ||
| "외부 봇 필터링 완료: cheerTalkId={}, result={}", | ||
| cheerTalk.getId(), | ||
| result != null ? result : "CLEAN (fallback)" |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| result != null ? result : "CLEAN (fallback)" | ||
| ); | ||
| } | ||
| } catch (Exception e) { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| */ | ||
| public CheerTalkFilterResult filter(String content){ | ||
| return CheerTalkFilterResult.CLEAN; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요거 spring filter랑 이름이 좀 혼동될 것 같아서 바꾸는 게 어떄요
이슈 보니까 우리 LanguageFilter도 두는 것 같아서 더 헷갈릴 것 같아요
| private void blockCheerTalk(CheerTalk cheerTalk, CheerTalkCreateEvent event) { | ||
| cheerTalk.block(); | ||
| cheerTalkRepository.save(cheerTalk); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전체적으로 CheerTalkBotFilterEventHandler가 좀 많은 일을 하는 것 같은데
CQRS 적용했는데 query 부분에서 command 역할도 하는 것 같아요
비즈니스 로직은 다 CheerTalkService로 옮기는 것이 어떤가요?
- CheerTalkFilterResult -> CheerTalkBotFilterResult
- CheerTalkBotFilterEventHandler: AI 필터링 및 차단 처리 - CheerTalkBlockedByBotEventHandler: 소켓 메시지 발행 전담
- blockById() 메서드 추가: EntityUtils로 최신 엔티티 조회 - block() 메서드가 blockById() 호출하도록 공통화 - JPA 변경 감지로 자동 저장되도록 개선
|
리뷰 반영하시느라 고생하셨습니다 반영한 부분은 다 동의합니다! |
| destination, | ||
| new CheerTalkResponse.ForSpectator(cheerTalk) | ||
| ); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기 소켓 발행하는 부분 CheerTalkBotFilterEventHandler에서 CheerTalk 차단하면서 발행해도 될 텐데
핸들러랑 이벤트 분리하신 이유가 궁금해요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
왜냐면 나중에 CheerTalkBotFilterEventHandler에 꽤 복잡한 재시도 로직이 포함될 것 같은데, 이 내용들을 한 핸들러에서 다 처리하면 책임이 제대로 분리가 안 될 것 같다고 생각했어요. ai 봇에 의한 필터링 관련한 로직은 CheerTalkBotFilterEventHandler에서만 관리하고, 봇에 의해 처리가 완료되어 소켓 메시지를 발행하는 건 CheerTalkBlockedByBotEventHandler에 로직을 둬서 그 책임을 명확히 하고자 했어요!
한 곳에 로직을 모아두는 게 더 적합하다 생각하시나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아하 재시도 로직 추가될 거를 생각 못 했어요
의도가 있어서 분리하신 거라면 좋습니다
Jin409
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
효빈이 PR 에서 해야되는지는 명확하진 않지만..
- CheerTalk 의 상태를 하나 더 추가하고 (bot 에 의해 블락됨)
CheerTalkBotFilterEventHandler에서 ABUSIVE 로 판단이 되면 해당 상태로 변경되는 로직이 되어야 할 것 같아요~!
수고했어용 효빈
| * @return CheerTalkBotFilterResult(CLEAN or ABUSIVE) | ||
| * 필터링 관련 서비스 별도 생성 권장 | ||
| */ | ||
| public CheerTalkBotFilterResult filter(String content){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 기존에 bot 이라는 네이밍을 사용했었는데, 일관성 있게 filterByBot 이런 식으로 봇과 관련된 요소를 넣는 건 어떨까요?
외부에서 cheerTalkService.filter() 이라고 하면, 어떤 의미인지 잘 이해가 되지 않을 것 같다는 생각이 들어용
| pendingCheerTalkRepository.save( | ||
| new PendingCheerTalk(destination, cheerTalk) | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 레포지토리에서 save 를 하는 이유는, 전송 실패한 응원톡에 대한 전송을 위함인 것으로 알고 있어요.
현재 효빈이가 앞선 코멘트 에서 이야기한 것처럼, 로직을 분리하고 있으니 재시도에 대한 것도 추후에 다시 상세하게 논의해봐야 할 것 같아요. 그래서 필터링 된 응원톡의 전송 실패에 대해서 저장하는 것 역시도 일부 구분이 필요하지 않을까 싶은데 이 부분은 현재는 빼는 게 어떨까요?
짧게 이야기하자면, 앞서서 일반적인 응원톡 전송에 대한 로직을 현재 필터링 된 응원톡 전송에 대한 로직과 분리하고 있으니 재시도에 대한 로직도 분리가 필요할 것 같다는 의견입니당
| @Component | ||
| @RequiredArgsConstructor | ||
| @Slf4j | ||
| public class CheerTalkBlockedByBotEventHandler { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 비동기로 봇 호출해서 필터링 여부 확인 - 2. 필터링이 되어야 한다면 비동기로 또 다시 이벤트 발행 - 3.
CheerTalkBlockedByBotEventHandler이 동작 인 것으로 이해했어요!
현재 Handler 의 네이밍이 봇에 의해 블락을 시키는 것으로 읽히는 것 같다는 생각이 들어요.
- BotBlockedCheerTalkSocketDispatchHandler
- BotBlockedCheerTalkSpectatorPublishHandler
요런 식으로 발행 / 디스패치 등의 맥락을 담는 건 어떠세요?
이 부분은 세현오빠가 구현할 때 추가해주면 될 것 같아용 @sese2204 |
🌍 이슈 번호
#416
📝 구현 내용
준영속(Detached) 엔티티 처리 문제 해결
비동기 이벤트 핸들러에서 이벤트로 전달받은 detached 엔티티를 직접 수정하는 방식 -> ID 기반으로 재조회하여 managed 엔티티를 사용하도록 변경
변경 전:
cheerTalkRepository.save()명시적 호출 필요변경 후:
CheerTalkService.blockById()메서드 추가메서드 네이밍 변경
CheerTalkFilterResult→CheerTalkBotFilterResult로 변경하여 Spring Filter, LanguageFilter와의 혼동 방지-> @sese2204 리뷰 반영, 이 네이밍 괜찮은지 확인 부탁!!
핸들러 분리
AI 필터링 로직과 소켓 메시지 발행 로직 분리
분리된 핸들러:
CheerTalkBotFilterEventHandler: AI 모델 필터링 및 차단 처리CheerTalkBlockedByBotEventHandler: 차단 이벤트 수신 후 소켓 메시지 발행🍀 확인해야 할 부분 / TODO
실제 AI 모델 연동
CheerTalkService.filter()메서드는 더미 구현 상태CheerTalkFilterService)로 분리하는 게 개인적으로는 '봇 필터링 로직과 응원톡 관리 로직 분리'되는 것 같아 좋을 것 같아요.프론트엔드 중복 데이터 처리 방식 제안
현재 구조상 AI 차단 시 프론트엔드에서 동일한
cheerTalkId로 2개의 메시지를 받게 됨:{ cheerTalkId: 1, content: "내용", isBlocked: false }{ cheerTalkId: 1, content: null, isBlocked: true }처리 방안:
cheerTalkId가 이미 존재하면 기존 항목을 업데이트하는 방식=> 이 방식 프론트에서 그렇게 처리가 복잡하지는 않을 거라 생각이 드는데 요 방식으로 하고 전달하는 게 어떤지?