refactor : api 경로 변경 및 필요없는 코드 정리 #97
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chat26666
requested review from
Kimminu7,
NCookies,
minjee2758,
pokerbearkr and
thezz9
## Walkthrough
채팅 관련 API, 컨트롤러, 인터셉터, 테스트 등에서 URL 경로의 단수 "room"을 복수 "rooms"로 통일하였으며, 채팅방 소유자 확인 로직을 소유자 또는 관리자 권한 확인으로 확장하였습니다. 또한, 관리 기능 클래스에 ADMIN 권한 어노테이션을 추가하고, 불필요한 findAll 메서드를 제거하였습니다. 게임 레벨업 관련 이벤트 및 리스너, 이벤트 퍼블리셔가 새로 추가되고, 이벤트 관련 패키지 구조가 일부 변경되었습니다.
## Changes
| 파일/경로 요약 | 변경 내용 요약 |
|--------------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------|
| .../application/chatting/service/ChattingUseCase.java<br>.../domain/chat/service/ChattingDomainService.java<br>.../test/java/org/ezcode/codetest/application/chatting/service/ChattingUseCaseTest.java<br>.../test/java/org/ezcode/codetest/domain/chat/service/ChattingDomainServiceTest.java | 채팅방 소유자 확인 메서드를 소유자 또는 관리자 확인으로 변경 및 테스트 코드 반영 |
| .../application/game/management/GameAdminUseCase.java | 클래스에 `@PreAuthorize("hasRole('ADMIN')")` 어노테이션 추가 |
| .../domain/chat/repository/ChatRepository.java<br>.../infrastructure/persistence/repository/chat/ChatRepositoryImpl.java | findAll() 메서드 제거 |
| .../presentation/chatting/chatting/config/ChatWebConfig.java<br>.../presentation/chatting/chatting/controller/ChatController.java<br>.../presentation/chatting/chatting/controller/ChatRoomController.java<br>.../presentation/chatting/chatting/interceptor/ChatLimitInterceptor.java<br>.../presentation/chatting/event/EventHandleController.java<br>.../presentation/chatting/view/ChatViewController.java | 패키지명 오타 수정 및 URL 경로를 단수 "room"에서 복수 "rooms"로 변경, 관련 어노테이션 수정 |
| .../resources/templates/chat-page.html | JS 내 WebSocket/REST API 경로를 "room"에서 "rooms"로 변경 |
| .../infrastructure/event/dto/GameLevelUpEvent.java | 게임 레벨업 이벤트 데이터 레코드 추가 |
| .../infrastructure/event/listener/GameLevelUpListener.java | 게임 레벨업 이벤트 리스너 클래스 추가 |
| .../infrastructure/event/publisher/ProblemEventPublisher.java | 문제 풀이 이벤트를 게임 레벨업 이벤트로 변환하여 발행하는 퍼블리셔 추가 |
| .../infrastructure/event/listener/ChatEventListener.java<br>.../infrastructure/event/listener/NotificationEventListener.java<br>.../infrastructure/event/listener/WebSocketEventListener.java<br>.../infrastructure/event/publisher/ChatEventPublisher.java<br>.../infrastructure/event/publisher/DiscordNotifier.java<br>.../infrastructure/event/publisher/NotificationEventPublisher.java<br>.../infrastructure/event/publisher/RedisJudgeQueueProducer.java<br>.../infrastructure/event/publisher/StompMessageService.java | 이벤트 관련 클래스들의 패키지 경로를 `service`에서 `publisher`로 변경 |
| .../infrastructure/event/config/CustomHandShakeHandler.java | WebSocket 연결 토큰 파라미터 처리 확장 및 예외 처리 추가 |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant ChatController
participant ChattingUseCase
participant ChattingDomainService
User->>ChatController: 채팅방 삭제 요청 (roomId, user)
ChatController->>ChattingUseCase: removeChatRoom(roomId, user)
ChattingUseCase->>ChattingDomainService: checkChatRoomOwnerOrAdmin(room, user)
ChattingDomainService-->>ChattingUseCase: 소유자 또는 관리자 권한 확인
ChattingUseCase-->>ChatController: 삭제 처리 결과 반환
ChatController-->>User: 응답 반환Possibly related PRs
Suggested reviewers
Poem
|
chat26666
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/main/java/org/ezcode/codetest/presentation/chatting/chatting/interceptor/ChatLimitInterceptor.java (1)
33-41:AuthUser캐스팅 안전성 확보 및 비인증 사용자 처리익명 사용자(
AnonymousAuthenticationToken)가 들어올 경우AuthUser로 단순 캐스팅하면ClassCastException이 발생할 수 있습니다.
타입 검증 후 처리하거나 401/403 응답을 명시적으로 내려주세요.-AuthUser authUser = (AuthUser)authentication.getPrincipal(); +if (!(authentication.getPrincipal() instanceof AuthUser authUser)) { + response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); + return false; +}
🧹 Nitpick comments (3)
src/main/java/org/ezcode/codetest/presentation/chatting/chatting/interceptor/ChatLimitInterceptor.java (3)
1-1: 패키지 경로에chatting.chatting이중 표기 – 필요성 재검토 권장
chatting.chatting처럼 동일한 디렉터리명이 두 번 반복되는 구조는 직관성이 떨어집니다.
org.ezcode.codetest.presentation.chatting.interceptor정도로 단순화해도 모듈 경계가 명확하니, 경로를 한 단계 줄이는 방안을 고려해 주세요.
24-26: 상수 선언 관례 및 오토박싱 비용 최적화
MAX_CHAT_COUNT_PER_10SEC는 상수이므로 primitivelong이 더 적합하며,static과final위치를 관례적으로 바꿔 가독성을 높일 수 있습니다.-private final static Long MAX_CHAT_COUNT_PER_10SEC = 11L; +private static final long MAX_CHAT_COUNT_PER_10SEC = 11L;
59-74:extractRoomId예외 메시지 불투명
NumberFormatException에 원본 메시지만 전달하면 어떤 PathVariable 이 잘못됐는지 로그 외에는 알기 어렵습니다.
구체적인 메시지와 함께 400 Bad Request 로 응답하도록 개선을 권장합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/main/java/org/ezcode/codetest/application/chatting/service/ChattingUseCase.java(1 hunks)src/main/java/org/ezcode/codetest/application/game/management/GameAdminUseCase.java(1 hunks)src/main/java/org/ezcode/codetest/domain/chat/repository/ChatRepository.java(0 hunks)src/main/java/org/ezcode/codetest/domain/chat/service/ChattingDomainService.java(2 hunks)src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/chat/ChatRepositoryImpl.java(0 hunks)src/main/java/org/ezcode/codetest/presentation/chatting/chatting/config/ChatWebConfig.java(2 hunks)src/main/java/org/ezcode/codetest/presentation/chatting/chatting/controller/ChatController.java(2 hunks)src/main/java/org/ezcode/codetest/presentation/chatting/chatting/controller/ChatRoomController.java(2 hunks)src/main/java/org/ezcode/codetest/presentation/chatting/chatting/interceptor/ChatLimitInterceptor.java(1 hunks)src/main/java/org/ezcode/codetest/presentation/chatting/event/EventHandleController.java(3 hunks)src/main/java/org/ezcode/codetest/presentation/chatting/view/ChatViewController.java(1 hunks)src/main/resources/templates/chat-page.html(3 hunks)src/test/java/org/ezcode/codetest/application/chatting/service/ChattingUseCaseTest.java(1 hunks)src/test/java/org/ezcode/codetest/domain/chat/service/ChattingDomainServiceTest.java(4 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/org/ezcode/codetest/domain/chat/repository/ChatRepository.java
- src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/chat/ChatRepositoryImpl.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (19)
src/main/java/org/ezcode/codetest/presentation/chatting/view/ChatViewController.java (1)
1-1: 패키지 경로가 변경되었습니다 – 레거시 참조 검사 필요
chattingmanagement.view→chatting.view로 이동하면서 다른 소스가 여전히 이전 패키지를 import 하고 있다면 컴파일이 실패합니다. 전체 코드베이스에서 레거시 경로 사용 여부를 한번 더 확인해주세요.#!/bin/bash # 변경 전 패키지 경로 사용 여부 확인 rg --line-number 'presentation\.chattingmanagement\.view' || echo "✅ 레거시 패키지 참조 없음"src/main/java/org/ezcode/codetest/presentation/chatting/chatting/interceptor/ChatLimitInterceptor.java (1)
45-55: 차단 시 클라이언트 피드백 부재현재
false만 반환하면 Spring MVC 가 단순 200 OK 로 body 없이 응답할 수 있습니다.
적절한 HTTP 상태 코드(예: 429 Too Many Requests)와 메시지를 설정해 사용자-경험을 향상시켜 주세요.src/main/java/org/ezcode/codetest/presentation/chatting/chatting/config/ChatWebConfig.java (2)
1-3: 패키지 구조 수정이 올바르게 적용되었습니다.패키지 경로가
chattingmanagement.chatting에서chatting.chatting으로 수정되어 디렉토리 구조와 일치하도록 개선되었습니다.
19-19: API 경로가 RESTful 규칙에 맞게 복수형으로 변경되었습니다.인터셉터 패턴이
/api/room/*/chat에서/api/rooms/*/chat으로 변경되어 일관된 복수형 명명 규칙을 따르고 있습니다.src/main/java/org/ezcode/codetest/presentation/chatting/chatting/controller/ChatController.java (2)
1-1: 패키지 구조 수정이 일관되게 적용되었습니다.다른 파일들과 동일하게 패키지 경로가 올바르게 수정되었습니다.
17-17: RequestMapping 경로가 RESTful 표준에 맞게 수정되었습니다.경로가
/api/room/{roomId}/chat에서/api/rooms/{roomId}/chat으로 변경되어 복수형 명명 규칙을 따르며, 인터셉터 설정과 일치합니다.src/main/java/org/ezcode/codetest/application/game/management/GameAdminUseCase.java (2)
23-23: Spring Security 어노테이션 import가 추가되었습니다.
@PreAuthorize어노테이션 사용을 위한 import가 올바르게 추가되었습니다.
31-31: 게임 관리 기능에 ADMIN 권한 제한이 올바르게 적용되었습니다.클래스 레벨에
@PreAuthorize("hasRole('ADMIN')")를 적용하여 모든 메서드에 ADMIN 권한이 필요하도록 설정한 것은 보안 강화 측면에서 적절한 구현입니다.src/test/java/org/ezcode/codetest/application/chatting/service/ChattingUseCaseTest.java (1)
170-170: 테스트가 새로운 권한 확인 로직에 맞게 올바르게 업데이트되었습니다.
isChatRoomOwner에서checkChatRoomOwnerOrAdmin으로 변경되고, 매개변수도 사용자 ID에서 전체 User 객체로 변경되어 실제 구현과 일치합니다.src/main/java/org/ezcode/codetest/application/chatting/service/ChattingUseCase.java (1)
59-59: 채팅방 권한 확인 로직이 관리자 권한을 포함하도록 확장되었습니다.
isChatRoomOwner에서checkChatRoomOwnerOrAdmin으로 변경되어 소유자뿐만 아니라 관리자도 채팅방을 삭제할 수 있도록 권한이 확장되었습니다. 전체 User 객체를 전달하여 역할 기반 검증이 가능하도록 개선되었습니다.src/main/java/org/ezcode/codetest/presentation/chatting/chatting/controller/ChatRoomController.java (2)
1-1: 패키지 선언 수정이 올바릅니다.패키지 경로가
chattingmanagement.chatting에서chatting.chatting으로 정정되어 일관성이 개선되었습니다.
24-24: RESTful API 규칙에 맞게 URL 경로가 개선되었습니다.
/api/chatrooms에서/api/rooms로 변경하여 복수형 명사 사용으로 일관성을 확보했습니다.src/main/java/org/ezcode/codetest/presentation/chatting/event/EventHandleController.java (2)
1-1: 패키지 선언 정정이 적절합니다.패키지 경로가 올바르게 수정되어 프로젝트 구조의 일관성이 향상되었습니다.
28-28: WebSocket 메시지 매핑 경로가 일관성 있게 개선되었습니다.
/room/에서/rooms/로 변경하여 RESTful 규칙에 맞게 복수형을 사용하고 있습니다.Also applies to: 38-38
src/main/resources/templates/chat-page.html (1)
311-311: 프론트엔드 엔드포인트가 백엔드 변경사항과 일치하게 업데이트되었습니다.WebSocket 및 REST API 경로가 모두 복수형 "rooms"를 사용하도록 일관성 있게 수정되었습니다.
Also applies to: 340-340, 355-355
src/main/java/org/ezcode/codetest/domain/chat/service/ChattingDomainService.java (2)
11-12: 필요한 임포트가 올바르게 추가되었습니다.새로운 메서드 시그니처와 권한 확인 로직을 위한
User와UserRole임포트가 적절히 추가되었습니다.
34-39: 권한 확인 로직이 향상되어 보안성이 개선되었습니다.메서드명이
checkChatRoomOwnerOrAdmin으로 변경되어 기능이 명확해졌고, 채팅방 소유자 또는 관리자 권한을 모두 허용하는 로직이 올바르게 구현되었습니다.src/test/java/org/ezcode/codetest/domain/chat/service/ChattingDomainServiceTest.java (2)
44-44: 테스트를 위한 추가 사용자 객체가 적절히 설정되었습니다.권한이 없는 사용자에 대한 테스트 케이스를 위해
wrongUser객체가 올바르게 추가되었습니다.Also applies to: 65-77
129-129: 테스트 메서드 호출이 서비스 변경사항에 맞게 업데이트되었습니다.
checkChatRoomOwnerOrAdmin메서드를 사용하도록 테스트가 올바르게 수정되어 새로운 권한 확인 로직을 적절히 검증하고 있습니다.Also applies to: 133-133, 199-199
thezz9
approved these changes
Jun 24, 2025
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/java/org/ezcode/codetest/infrastructure/event/publisher/ProblemEventPublisher.java (1)
16-16: 메서드명 개선 제안메서드명
publishProblemSolveEvent가 실제로GameLevelUpEvent를 발행한다는 사실을 명확히 나타내지 못합니다.메서드명을 다음과 같이 변경하는 것을 고려해보세요:
-public void publishProblemSolveEvent(UserProblemResult event) { +public void publishGameLevelUpEvent(UserProblemResult event) {또는 더 구체적으로:
-public void publishProblemSolveEvent(UserProblemResult event) { +public void publishLevelUpEventFromProblemResult(UserProblemResult event) {src/main/java/org/ezcode/codetest/infrastructure/event/listener/GameLevelUpListener.java (1)
20-22: 이벤트 데이터 검증 추가 고려도메인 서비스를 호출하기 전에 이벤트 데이터의 유효성을 검증하는 것이 좋습니다.
다음과 같은 검증 로직 추가를 고려해보세요:
public void handleGameCharacterLevelUp(GameLevelUpEvent event) { + if (event == null || event.userId() == null || event.problemCategory() == null) { + log.warn("유효하지 않은 GameLevelUpEvent: {}", event); + return; + } characterDomainService.gameCharacterLevelUp(event.userId(), event.isProblemSolved(), event.problemCategory()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/org/ezcode/codetest/infrastructure/event/dto/GameLevelUpEvent.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/listener/ChatEventListener.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/listener/GameLevelUpListener.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/listener/NotificationEventListener.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/listener/WebSocketEventListener.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/publisher/ChatEventPublisher.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/publisher/DiscordNotifier.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/publisher/NotificationEventPublisher.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/publisher/ProblemEventPublisher.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/publisher/RedisJudgeQueueProducer.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/publisher/StompMessageService.java(1 hunks)
✅ Files skipped from review due to trivial changes (9)
- src/main/java/org/ezcode/codetest/infrastructure/event/publisher/DiscordNotifier.java
- src/main/java/org/ezcode/codetest/infrastructure/event/listener/WebSocketEventListener.java
- src/main/java/org/ezcode/codetest/infrastructure/event/publisher/StompMessageService.java
- src/main/java/org/ezcode/codetest/infrastructure/event/publisher/NotificationEventPublisher.java
- src/main/java/org/ezcode/codetest/infrastructure/event/listener/ChatEventListener.java
- src/main/java/org/ezcode/codetest/infrastructure/event/listener/NotificationEventListener.java
- src/main/java/org/ezcode/codetest/infrastructure/event/publisher/ChatEventPublisher.java
- src/main/java/org/ezcode/codetest/infrastructure/event/publisher/RedisJudgeQueueProducer.java
- src/main/java/org/ezcode/codetest/infrastructure/event/dto/GameLevelUpEvent.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
Comment on lines
+16
to
+23
| public void publishProblemSolveEvent(UserProblemResult event) { | ||
|
|
||
| Long userId = event.getUser().getId(); | ||
| boolean isCorrect = event.isCorrect(); | ||
| String problemCategory = event.getProblem().getCategory().getDescription(); | ||
|
|
||
| publisher.publishEvent(new GameLevelUpEvent(userId, isCorrect, problemCategory)); | ||
| } |
There was a problem hiding this comment.
널 안전성 검증 추가 필요
현재 구현에서는 event, event.getUser(), event.getProblem(), event.getProblem().getCategory() 등의 null 체크가 누락되어 있어 NullPointerException이 발생할 수 있습니다.
다음과 같이 수정하여 null 안전성을 보장해주세요:
public void publishProblemSolveEvent(UserProblemResult event) {
+ if (event == null || event.getUser() == null || event.getProblem() == null
+ || event.getProblem().getCategory() == null) {
+ throw new IllegalArgumentException("UserProblemResult와 관련 객체들은 null일 수 없습니다");
+ }
Long userId = event.getUser().getId();
boolean isCorrect = event.isCorrect();
String problemCategory = event.getProblem().getCategory().getDescription();
publisher.publishEvent(new GameLevelUpEvent(userId, isCorrect, problemCategory));
}🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/infrastructure/event/publisher/ProblemEventPublisher.java
between lines 16 and 23, add null checks for event, event.getUser(),
event.getProblem(), and event.getProblem().getCategory() before accessing their
methods to prevent NullPointerException. Implement conditional checks that
return early or handle null cases safely to ensure null safety in the
publishProblemSolveEvent method.
Comment on lines
19
to
23
| @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) | ||
| public void handleGameCharacterLevelUp(GameLevelUpEvent event) { | ||
|
|
||
| characterDomainService.gameCharacterLevelUp(event.userId(), event.isProblemSolved(), event.problemCategory()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 처리 및 로깅 추가 필요
현재 구현에서는 도메인 서비스 호출 시 발생할 수 있는 예외에 대한 처리가 없고, 로깅 어노테이션이 있음에도 불구하고 실제 로깅이 구현되지 않았습니다.
다음과 같이 에러 처리와 로깅을 추가해주세요:
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
public void handleGameCharacterLevelUp(GameLevelUpEvent event) {
+ try {
+ log.info("게임 레벨업 이벤트 처리 시작 - userId: {}, isProblemSolved: {}, category: {}",
+ event.userId(), event.isProblemSolved(), event.problemCategory());
+
characterDomainService.gameCharacterLevelUp(event.userId(), event.isProblemSolved(), event.problemCategory());
+
+ log.info("게임 레벨업 이벤트 처리 완료 - userId: {}", event.userId());
+ } catch (Exception e) {
+ log.error("게임 레벨업 이벤트 처리 중 오류 발생 - userId: {}, error: {}",
+ event.userId(), e.getMessage(), e);
+ // 필요에 따라 재시도 로직이나 알림 로직 추가 고려
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) | |
| public void handleGameCharacterLevelUp(GameLevelUpEvent event) { | |
| characterDomainService.gameCharacterLevelUp(event.userId(), event.isProblemSolved(), event.problemCategory()); | |
| } | |
| @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) | |
| public void handleGameCharacterLevelUp(GameLevelUpEvent event) { | |
| try { | |
| log.info("게임 레벨업 이벤트 처리 시작 - userId: {}, isProblemSolved: {}, category: {}", | |
| event.userId(), event.isProblemSolved(), event.problemCategory()); | |
| characterDomainService.gameCharacterLevelUp(event.userId(), event.isProblemSolved(), event.problemCategory()); | |
| log.info("게임 레벨업 이벤트 처리 완료 - userId: {}", event.userId()); | |
| } catch (Exception e) { | |
| log.error("게임 레벨업 이벤트 처리 중 오류 발생 - userId: {}, error: {}", | |
| event.userId(), e.getMessage(), e); | |
| // 필요에 따라 재시도 로직이나 알림 로직 추가 고려 | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/infrastructure/event/listener/GameLevelUpListener.java
around lines 19 to 23, the method handleGameCharacterLevelUp lacks error
handling and does not perform any logging despite having logging annotations.
Wrap the call to characterDomainService.gameCharacterLevelUp in a try-catch
block to catch potential exceptions, log the error details using the logger, and
handle the exception appropriately to prevent it from propagating uncaught.
pokerbearkr
approved these changes
Jun 24, 2025
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/org/ezcode/codetest/infrastructure/event/listener/GameLevelUpListener.java (1)
23-37: 로깅 개선으로 디버깅 효율성을 높일 수 있습니다.이전 리뷰에서 언급된 에러 처리와 로깅이 잘 구현되어 있습니다. 다만 로그 메시지에 이벤트 컨텍스트 정보를 추가하면 디버깅과 모니터링에 더 유용할 것 같습니다.
다음과 같이 로그 메시지를 개선해보세요:
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) @Transactional(propagation = Propagation.REQUIRES_NEW) public void handleGameCharacterLevelUp(GameLevelUpEvent event) { + log.debug("게임 레벨업 이벤트 처리 시작 - userId: {}, isProblemSolved: {}, category: {}", + event.userId(), event.isProblemSolved(), event.problemCategory()); try { characterDomainService.gameCharacterLevelUp(event.userId(), event.isProblemSolved(), event.problemCategory()); + log.debug("게임 레벨업 이벤트 처리 완료 - userId: {}", event.userId()); } catch (GameException ge) { - log.info("현재 해당 사용자는 게임 캐릭터를 생성한 상태가 아닙니다. {}", ge.getMessage()); + log.info("현재 해당 사용자는 게임 캐릭터를 생성한 상태가 아닙니다. userId: {}, error: {}", + event.userId(), ge.getMessage()); TransactionAspectSupport.currentTransactionStatus().setRollbackOnly(); } catch (Exception e) { - log.warn("문제 풀이로 인한 레벨업 실패 Unknown Error 발생", e); + log.warn("문제 풀이로 인한 레벨업 실패 Unknown Error 발생 - userId: {}, category: {}", + event.userId(), event.problemCategory(), e); TransactionAspectSupport.currentTransactionStatus().setRollbackOnly(); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/org/ezcode/codetest/infrastructure/event/config/CustomHandShakeHandler.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/listener/GameLevelUpListener.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/listener/WebSocketEventListener.java(3 hunks)src/main/resources/templates/chat-page.html(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/org/ezcode/codetest/infrastructure/event/listener/WebSocketEventListener.java
- src/main/resources/templates/chat-page.html
🔇 Additional comments (3)
src/main/java/org/ezcode/codetest/infrastructure/event/listener/GameLevelUpListener.java (2)
1-14: 임포트 구조가 깔끔하게 정리되어 있습니다.필요한 의존성들이 적절히 임포트되어 있고, 패키지 구조도 명확합니다.
16-21: 클래스 선언 및 의존성 주입이 적절합니다.Spring의 표준 어노테이션들을 사용하여 컴포넌트 스캔, 로깅, 생성자 주입이 올바르게 구성되어 있습니다.
src/main/java/org/ezcode/codetest/infrastructure/event/config/CustomHandShakeHandler.java (1)
31-33: 쿼리 파라미터 null 검증 로직이 잘 추가되었습니다.WebSocket 연결 시 쿼리 파라미터가 누락된 경우를 미리 검증하여 안정성을 향상시켰습니다.
Comment on lines
+35
to
42
| if (query.startsWith("token=")) { | ||
| tokenParam = query.substring(6); | ||
| } else if (query.startsWith("chat-token=")) { | ||
| tokenParam = query.substring(11); | ||
| attributes.put("isChattingWebsocket", true); | ||
| } else { | ||
| throw new IllegalArgumentException("허용되지 않은 토큰 파라미터: " + query); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
토큰 파라미터 처리 로직의 확장성과 유지보수성을 개선해주세요.
현재 구현에서 몇 가지 개선 사항이 필요합니다:
- 매직 넘버(6, 11) 사용으로 인한 유지보수성 저하
- 문자열 리터럴 중복
- 새로운 토큰 타입 추가 시 확장성 부족
다음과 같이 리팩토링을 제안합니다:
+ private static final String TOKEN_PREFIX = "token=";
+ private static final String CHAT_TOKEN_PREFIX = "chat-token=";
+
@Override
protected Principal determineUser(
@NonNull ServerHttpRequest request,
@NonNull WebSocketHandler wsHandler,
@NonNull Map<String, Object> attributes
) {
URI uri = request.getURI();
String query = uri.getQuery();
String tokenParam = null;
if (query == null) {
throw new IllegalArgumentException("WebSocket 연결에 필요한 토큰이 없습니다.");
}
- if (query.startsWith("token=")) {
- tokenParam = query.substring(6);
- } else if (query.startsWith("chat-token=")) {
- tokenParam = query.substring(11);
+ if (query.startsWith(TOKEN_PREFIX)) {
+ tokenParam = query.substring(TOKEN_PREFIX.length());
+ } else if (query.startsWith(CHAT_TOKEN_PREFIX)) {
+ tokenParam = query.substring(CHAT_TOKEN_PREFIX.length());
attributes.put("isChattingWebsocket", true);
} else {
throw new IllegalArgumentException("허용되지 않은 토큰 파라미터: " + query);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (query.startsWith("token=")) { | |
| tokenParam = query.substring(6); | |
| } else if (query.startsWith("chat-token=")) { | |
| tokenParam = query.substring(11); | |
| attributes.put("isChattingWebsocket", true); | |
| } else { | |
| throw new IllegalArgumentException("허용되지 않은 토큰 파라미터: " + query); | |
| } | |
| // In CustomHandShakeHandler.java, add at the top of the class: | |
| private static final String TOKEN_PREFIX = "token="; | |
| private static final String CHAT_TOKEN_PREFIX = "chat-token="; | |
| @Override | |
| protected Principal determineUser( | |
| @NonNull ServerHttpRequest request, | |
| @NonNull WebSocketHandler wsHandler, | |
| @NonNull Map<String, Object> attributes | |
| ) { | |
| URI uri = request.getURI(); | |
| String query = uri.getQuery(); | |
| String tokenParam = null; | |
| if (query == null) { | |
| throw new IllegalArgumentException("WebSocket 연결에 필요한 토큰이 없습니다."); | |
| } | |
| if (query.startsWith(TOKEN_PREFIX)) { | |
| tokenParam = query.substring(TOKEN_PREFIX.length()); | |
| } else if (query.startsWith(CHAT_TOKEN_PREFIX)) { | |
| tokenParam = query.substring(CHAT_TOKEN_PREFIX.length()); | |
| attributes.put("isChattingWebsocket", true); | |
| } else { | |
| throw new IllegalArgumentException("허용되지 않은 토큰 파라미터: " + query); | |
| } | |
| // … continue processing tokenParam … | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/infrastructure/event/config/CustomHandShakeHandler.java
around lines 35 to 42, the token parameter handling uses hardcoded substring
indices and duplicated string literals, which reduces maintainability and
extensibility. Refactor by defining constants for token prefixes and their
lengths, and use these constants to extract token values. Consider using a map
or enum to associate token prefixes with their handling logic, so adding new
token types requires minimal code changes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
작업 내용
코드 리뷰 전 확인 체크리스트
type :)Summary by CodeRabbit
신규 기능
버그 수정
리팩터링/스타일
테스트