refactor: 패키지 구조 및 코드 스타일 개선 #1
Conversation
- 불명확한 클래스 네이밍 명확하게 수정 - 닉네임 수정을 프로필 수정으로 확장 - 코딩 컨벤션을 준수하도록 스타일 수정
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbit릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughCQRS로 명령/조회 타입을 분리하고 DTO를 command/query/result로 재배치했으며, 도메인에 유효성·행위(createRequest/accept)와 ID 기반 equals/hashCode를 추가했습니다. 전역 페이징/응답 구조가 확장되고 온보딩 상태 처리 및 Room 삭제 API가 도입되었습니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @patulus, 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은 코드베이스의 전반적인 구조와 스타일을 개선하기 위한 리팩토링 작업입니다. 주요 목표는 도메인 주도 설계 원칙을 더 잘 따르고, 코드의 가독성 및 유지보수성을 높이는 것입니다. 이를 위해 DTO를 Command, Query, Result로 분리하고, 도메인 엔티티에 비즈니스 로직을 캡슐화하며, 새로운 기능을 추가하고 기존 기능을 개선했습니다. 또한, 일관된 코딩 스타일을 적용하여 개발 효율성을 증대시키고자 합니다. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 6
Fix all issues with AI Agents 🤖
In @src/main/java/queuing/core/friend/application/query/GetListFriendQuery.java:
- Around line 11-16: The constructor GetListFriendQuery currently only checks
userSlug for null but not for an empty string; after calling userSlug =
userSlug.strip() validate that userSlug.isEmpty() (or equals("")) and if so
throw new BusinessException(ErrorCode.COMMON_INVALID_INPUT); update the
constructor to perform null-check, strip, then empty-check and assign the
stripped value back to the userSlug field.
In @src/main/java/queuing/core/room/application/command/DeleteRoomCommand.java:
- Around line 10-19: The compact constructor DeleteRoomCommand currently
null-checks and strips userSlug and roomSlug but misses validating they are
non-empty after strip(); update the constructor to, after calling userSlug =
userSlug.strip() and roomSlug = roomSlug.strip(), check for empty strings (e.g.,
userSlug.isEmpty() or roomSlug.isEmpty()) and throw new
BusinessException(ErrorCode.COMMON_INVALID_INPUT) when either is empty so all
input validation occurs in the command constructor.
In @src/main/java/queuing/core/room/presentation/controller/RoomController.java:
- Line 80: The @DeleteMapping annotation is missing the leading slash; change
@DeleteMapping("{slug}") to @DeleteMapping("/{slug}") and verify the handler
method (e.g., deleteRoom or the method annotated) uses @PathVariable("slug") on
its parameter so Spring treats slug as a path variable rather than a literal
segment.
In @src/main/java/queuing/core/user/application/service/UserWriteService.java:
- Around line 51-56: No change required: the new validateNickname(User user,
String newNickname) helper correctly centralizes nickname validation used by
updateUserProfile and completeOnboarding; ensure both methods call
validateNickname and retain the checks (newNickname != null,
!newNickname.equals(user.getNickname()),
userRepository.existsByNickname(newNickname)) and throw
BusinessException(ErrorCode.USER_NICKNAME_DUPLICATED) when duplicated.
- Around line 34-49: completeOnboarding updates the user's onboarding flag but
doesn't evict the cached AuthenticationService.isOnboardingCompleted entry
(cacheNames = "profileCompleted"), causing stale reads; add cache eviction when
the state changes by annotating UserWriteService.completeOnboarding with
@CacheEvict(cacheNames = "profileCompleted", key = "#cmd.userSlug()") or call
the appropriate cache eviction method after userRepository.saveAndFlush(user)
(ensuring the key uses cmd.userSlug()), so the
AuthenticationService.isOnboardingCompleted cache is invalidated when onboarding
completes.
♻️ Duplicate comments (2)
src/main/java/queuing/core/global/security/adapter/SpringSecurityAuthenticationAdapter.java (1)
11-12: Public API 변경 사항을 PR 설명에 문서화해 주세요.
SpringSecurityAuthenticationAdapter는 public 클래스이며AuthenticationOperations인터페이스를 구현하므로 공개 API의 일부입니다. 코딩 가이드라인에 따라 PR의 "Changes" 섹션에 다음 사항을 명시해 주세요:
- 새 public 클래스:
SpringSecurityAuthenticationAdapter- public 메서드:
void updateOnboardingStatus(boolean completed)(인터페이스로부터 상속)이러한 공개 API 변경 사항이 PR 설명에 최대 25개 항목까지 문서화되도록 해 주시기 바랍니다.
src/main/java/queuing/core/room/application/service/RoomWriteService.java (1)
68-82: 방 삭제 시 소유권 검증이 올바르게 구현되었습니다.이전 리뷰에서 지적된 소유권 검증 누락 문제가 해결되었습니다.
room.getOwner().equals(user)비교를 통해 요청한 사용자가 방의 소유자인지 확인하고, 권한이 없는 경우ErrorCode.USER_INSUFFICIENT_SCOPE예외를 발생시키고 있습니다. 트랜잭션 처리도 적절히 적용되어 있습니다.
🧹 Nitpick comments (3)
src/main/java/queuing/core/global/security/adapter/SpringSecurityAuthenticationAdapter.java (1)
16-19: 방어적 null 체크 추가를 권장합니다.
SecurityContextHolder.getContext().getAuthentication()이 null을 반환할 가능성이 있습니다. 현재 코드는 인증이 null이거나OAuth2AuthenticationToken이 아닌 경우 조용히 실패하는데, 이는 호출 컨텍스트에서 인증된 사용자를 보장하는 경우 의도된 동작일 수 있습니다.하지만 방어적 프로그래밍을 위해 명시적인 검증을 추가하는 것을 권장합니다:
🔎 제안하는 개선 방안
@Override public void updateOnboardingStatus(boolean completed) { Authentication auth = SecurityContextHolder.getContext().getAuthentication(); + + if (auth == null) { + throw new IllegalStateException("인증 정보가 존재하지 않습니다."); + } if (auth instanceof OAuth2AuthenticationToken oauth2Token && oauth2Token.getPrincipal() instanceof UserPrincipal principal) { UserPrincipal newPrincipal = principal.updateOnboardingStatus(completed); Authentication newAuth = new OAuth2AuthenticationToken( newPrincipal, newPrincipal.getAuthorities(), oauth2Token.getAuthorizedClientRegistrationId() ); SecurityContextHolder.getContext().setAuthentication(newAuth); + } else { + throw new IllegalStateException( + "예상하지 못한 인증 타입입니다: " + + (auth != null ? auth.getClass().getSimpleName() : "null") + ); } }또는 현재 silent fail 방식이 의도된 것이라면, 그 이유를 설명하는 주석을 추가해 주세요.
src/main/java/queuing/core/friend/application/query/GetListFriendQuery.java (1)
21-26:size검증 조건을 하나로 통합하는 것을 고려해 주세요.현재
size검증이 두 개의 별도if문으로 분리되어 있습니다. 가독성과 유지보수성을 위해 하나의 조건문으로 통합하는 것이 좋습니다.🔎 제안하는 수정 사항
- if (size <= 0) { - throw new BusinessException(ErrorCode.COMMON_INVALID_INPUT); - } - if (size > 100) { - throw new BusinessException(ErrorCode.COMMON_INVALID_INPUT); - } + if (size <= 0 || size > 100) { + throw new BusinessException(ErrorCode.COMMON_INVALID_INPUT); + }src/main/java/queuing/core/friend/domain/entity/Friend.java (1)
95-105:accept메서드에서 동일한ErrorCode를 사용하는 것에 대해 검토해 주세요.현재 두 가지 다른 검증 실패 상황(수신자가 아닌 경우, 상태가 PENDING이 아닌 경우)에서 동일한
FRIEND_INVALID_REQUEST에러 코드를 사용하고 있습니다. 디버깅과 클라이언트 측 에러 핸들링을 위해 더 구체적인 에러 코드를 분리하는 것을 고려해 주세요.예를 들어:
FRIEND_NOT_RECEIVER: 수신자가 아닌 사용자가 수락을 시도한 경우FRIEND_ALREADY_PROCESSED: 이미 처리된 친구 요청인 경우
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.coderabbit.yamlsrc/main/java/queuing/core/friend/application/query/GetListFriendQuery.javasrc/main/java/queuing/core/friend/application/service/FriendReadService.javasrc/main/java/queuing/core/friend/application/usecase/GetFriendListUseCase.javasrc/main/java/queuing/core/friend/domain/entity/Friend.javasrc/main/java/queuing/core/friend/presentation/controller/FriendController.javasrc/main/java/queuing/core/global/dto/PageCondition.javasrc/main/java/queuing/core/global/exception/ExceptionAdvice.javasrc/main/java/queuing/core/global/security/adapter/SpringSecurityAuthenticationAdapter.javasrc/main/java/queuing/core/room/application/command/DeleteRoomCommand.javasrc/main/java/queuing/core/room/application/query/GetListRoomQuery.javasrc/main/java/queuing/core/room/application/service/MusicTagReadService.javasrc/main/java/queuing/core/room/application/service/RoomReadService.javasrc/main/java/queuing/core/room/application/service/RoomWriteService.javasrc/main/java/queuing/core/room/domain/entity/MusicTag.javasrc/main/java/queuing/core/room/domain/entity/Room.javasrc/main/java/queuing/core/room/domain/entity/RoomMusicTag.javasrc/main/java/queuing/core/room/presentation/controller/RoomController.javasrc/main/java/queuing/core/user/application/service/UserWriteService.javasrc/main/java/queuing/core/user/domain/entity/User.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/queuing/core/global/dto/PageCondition.java
- src/main/java/queuing/core/global/exception/ExceptionAdvice.java
🧰 Additional context used
📓 Path-based instructions (7)
**/*
⚙️ CodeRabbit configuration file
**/*: - For every finding, include evidence: file path(s), line number(s), rule names (e.g., Checkstyle), and links to credible docs when relevant.
- Wrap any tokens starting with '@' (e.g.,
@JoinColumn) in backticks in PR comments.
Files:
src/main/java/queuing/core/room/application/service/RoomReadService.javasrc/main/java/queuing/core/global/security/adapter/SpringSecurityAuthenticationAdapter.javasrc/main/java/queuing/core/room/domain/entity/Room.javasrc/main/java/queuing/core/friend/application/query/GetListFriendQuery.javasrc/main/java/queuing/core/room/domain/entity/MusicTag.javasrc/main/java/queuing/core/room/application/service/RoomWriteService.javasrc/main/java/queuing/core/room/domain/entity/RoomMusicTag.javasrc/main/java/queuing/core/friend/domain/entity/Friend.javasrc/main/java/queuing/core/user/domain/entity/User.javasrc/main/java/queuing/core/user/application/service/UserWriteService.javasrc/main/java/queuing/core/friend/application/usecase/GetFriendListUseCase.javasrc/main/java/queuing/core/friend/application/service/FriendReadService.javasrc/main/java/queuing/core/room/application/query/GetListRoomQuery.javasrc/main/java/queuing/core/friend/presentation/controller/FriendController.javasrc/main/java/queuing/core/room/application/command/DeleteRoomCommand.javasrc/main/java/queuing/core/room/presentation/controller/RoomController.javasrc/main/java/queuing/core/room/application/service/MusicTagReadService.java
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: - Review against Java 21 and the rules in/.rules/checkstyle-rules.xml.
- Indentation: use spaces (no tabs), tab width = 4 spaces; files must end with Unix LF newline. (Team adaptation)
- Maximum line length: 120 characters.
- Imports: single-class imports only; allow wildcard for static imports; group imports with blank lines between sections.
- Operators: always one space before and after; on line breaks, place operators at the start of the next line (commas stay at end of line, dots at start of new line).
- Lambda expressions: omit parentheses for a single parameter; surround
->with spaces (param -> expression); use braces and explicitreturnfor multi-statement bodies; choose short, clear parameter names.- Prefer Java 21 standard APIs over Guava.
- Do not annotate immutable local variables with
finalunless required for an inner class.- Allow the
varkeyword when the value is a castnull.- For the complete NAVER Campus Hackday conventions, see: https://naver.github.io/hackday-conventions-java/
Files:
src/main/java/queuing/core/room/application/service/RoomReadService.javasrc/main/java/queuing/core/global/security/adapter/SpringSecurityAuthenticationAdapter.javasrc/main/java/queuing/core/room/domain/entity/Room.javasrc/main/java/queuing/core/friend/application/query/GetListFriendQuery.javasrc/main/java/queuing/core/room/domain/entity/MusicTag.javasrc/main/java/queuing/core/room/application/service/RoomWriteService.javasrc/main/java/queuing/core/room/domain/entity/RoomMusicTag.javasrc/main/java/queuing/core/friend/domain/entity/Friend.javasrc/main/java/queuing/core/user/domain/entity/User.javasrc/main/java/queuing/core/user/application/service/UserWriteService.javasrc/main/java/queuing/core/friend/application/usecase/GetFriendListUseCase.javasrc/main/java/queuing/core/friend/application/service/FriendReadService.javasrc/main/java/queuing/core/room/application/query/GetListRoomQuery.javasrc/main/java/queuing/core/friend/presentation/controller/FriendController.javasrc/main/java/queuing/core/room/application/command/DeleteRoomCommand.javasrc/main/java/queuing/core/room/presentation/controller/RoomController.javasrc/main/java/queuing/core/room/application/service/MusicTagReadService.java
**/main/java/**/*.java
⚙️ CodeRabbit configuration file
**/main/java/**/*.java: - This project is mature and must preserve a stable, backward-compatible public Java API.
- In the "Changes" section, list up to 25 changes to the public Java API that could affect end users; if none, explicitly state "No public Java API changes in this PR."
- Define the public Java API as public/protected methods on public classes plus
module-info.java.- Derive the list by deeply analyzing code flow, including through private methods and calls to Java 21 and Guava.
- Report:
- New or removed public API methods
- Changes to return types or parameter types
- Behavioral changes that may impact consumers
- Use
System.out.printlninstead of logging frameworks.- For performance reasons, the project prefers for-loops; do not suggest converting between loops and streams.
Files:
src/main/java/queuing/core/room/application/service/RoomReadService.javasrc/main/java/queuing/core/global/security/adapter/SpringSecurityAuthenticationAdapter.javasrc/main/java/queuing/core/room/domain/entity/Room.javasrc/main/java/queuing/core/friend/application/query/GetListFriendQuery.javasrc/main/java/queuing/core/room/domain/entity/MusicTag.javasrc/main/java/queuing/core/room/application/service/RoomWriteService.javasrc/main/java/queuing/core/room/domain/entity/RoomMusicTag.javasrc/main/java/queuing/core/friend/domain/entity/Friend.javasrc/main/java/queuing/core/user/domain/entity/User.javasrc/main/java/queuing/core/user/application/service/UserWriteService.javasrc/main/java/queuing/core/friend/application/usecase/GetFriendListUseCase.javasrc/main/java/queuing/core/friend/application/service/FriendReadService.javasrc/main/java/queuing/core/room/application/query/GetListRoomQuery.javasrc/main/java/queuing/core/friend/presentation/controller/FriendController.javasrc/main/java/queuing/core/room/application/command/DeleteRoomCommand.javasrc/main/java/queuing/core/room/presentation/controller/RoomController.javasrc/main/java/queuing/core/room/application/service/MusicTagReadService.java
**/application/service/**/*Service.java
⚙️ CodeRabbit configuration file
**/application/service/**/*Service.java: - Services must be annotated with@Serviceand implement one or more use case interfaces.
- Follow the Read/Write separation pattern: use separate classes for read operations (e.g.,
FriendReadService) and write operations (e.g.,FriendWriteService).- Write operations must be annotated with
@Transactional.- Read operations should use
@Transactional(readOnly = true)when appropriate.- Services should orchestrate domain logic, not contain it; delegate business rules to domain entities.
- Throw
BusinessExceptionwith appropriateErrorCodefor business rule violations.- Avoid direct manipulation of entities; prefer calling domain methods.
Files:
src/main/java/queuing/core/room/application/service/RoomReadService.javasrc/main/java/queuing/core/room/application/service/RoomWriteService.javasrc/main/java/queuing/core/user/application/service/UserWriteService.javasrc/main/java/queuing/core/friend/application/service/FriendReadService.javasrc/main/java/queuing/core/room/application/service/MusicTagReadService.java
**/application/query/**/*.java
⚙️ CodeRabbit configuration file
**/application/query/**/*.java: - Queries must be immutable records.
- All input validation must occur in the compact constructor.
- Validate pagination parameters (e.g.,
size > 0,size <= 100,lastId > 0).- Throw
BusinessExceptionwith appropriateErrorCodefor validation failures.- Queries should only contain parameters needed for read operations.
Files:
src/main/java/queuing/core/friend/application/query/GetListFriendQuery.javasrc/main/java/queuing/core/room/application/query/GetListRoomQuery.java
**/application/usecase/**/*.java
⚙️ CodeRabbit configuration file
**/application/usecase/**/*.java: - Use cases must be defined as interfaces with a single public method representing one business operation.
- Method names should clearly express the business intent (e.g.,
create,delete,accept,send).- Avoid multiple unrelated operations in a single use case interface.
- Keep method signatures simple and focused; use Command or Query objects for complex parameters.
Files:
src/main/java/queuing/core/friend/application/usecase/GetFriendListUseCase.java
**/application/command/**/*.java
⚙️ CodeRabbit configuration file
**/application/command/**/*.java: - Commands must be immutable records.
- All input validation must occur in the compact constructor.
- Normalize inputs (e.g.,
strip()for strings) in the compact constructor before validation.- Provide sensible defaults for optional fields (e.g.,
Set.of()for empty collections).- Throw
BusinessExceptionwith appropriateErrorCodefor validation failures.- Commands should represent write operations; avoid query-related fields unless necessary for the write operation.
Files:
src/main/java/queuing/core/room/application/command/DeleteRoomCommand.java
🧬 Code graph analysis (8)
src/main/java/queuing/core/room/application/service/RoomReadService.java (3)
src/main/java/queuing/core/friend/application/service/FriendReadService.java (1)
Service(21-45)src/main/java/queuing/core/room/application/service/MusicTagReadService.java (1)
Service(15-29)src/main/java/queuing/core/user/application/service/UserReadService.java (1)
Service(14-31)
src/main/java/queuing/core/global/security/adapter/SpringSecurityAuthenticationAdapter.java (2)
src/main/java/queuing/core/global/security/authorization/OnboardingRequiredAuthorizationManager.java (1)
Component(23-60)src/main/java/queuing/core/global/security/authorization/UserPrincipal.java (1)
UserPrincipal(18-145)
src/main/java/queuing/core/friend/application/query/GetListFriendQuery.java (1)
src/main/java/queuing/core/global/exception/BusinessException.java (1)
BusinessException(5-19)
src/main/java/queuing/core/room/application/service/RoomWriteService.java (1)
src/main/java/queuing/core/global/utils/SlugUtils.java (1)
SlugUtils(5-28)
src/main/java/queuing/core/friend/domain/entity/Friend.java (1)
src/main/java/queuing/core/global/exception/BusinessException.java (1)
BusinessException(5-19)
src/main/java/queuing/core/user/application/service/UserWriteService.java (3)
src/main/java/queuing/core/room/application/service/RoomWriteService.java (1)
Service(26-83)src/main/java/queuing/core/user/application/service/UserReadService.java (1)
Service(14-31)src/main/java/queuing/core/user/application/service/AuthenticationService.java (1)
Service(20-59)
src/main/java/queuing/core/room/application/query/GetListRoomQuery.java (1)
src/main/java/queuing/core/global/exception/BusinessException.java (1)
BusinessException(5-19)
src/main/java/queuing/core/room/application/command/DeleteRoomCommand.java (1)
src/main/java/queuing/core/global/exception/BusinessException.java (1)
BusinessException(5-19)
🔇 Additional comments (24)
src/main/java/queuing/core/global/security/adapter/SpringSecurityAuthenticationAdapter.java (2)
1-10: Import 구성이 올바릅니다.패키지 선언과 import 문이 코딩 컨벤션을 준수하고 있습니다. Spring 관련 import가 먼저 위치하고, 애플리케이션 관련 import가 그 다음에 위치하며, 모두 단일 클래스 import를 사용하고 있습니다.
14-30:OAuth2AuthenticationToken사용과 불변 접근 방식이 적절합니다.이전 리뷰 코멘트에서 제기된
UsernamePasswordAuthenticationToken사용 문제가 해결되었습니다. 현재 구현은 다음과 같은 장점이 있습니다:
- OAuth2/OIDC 컨텍스트에 적합한
OAuth2AuthenticationToken사용- 불변 방식으로 새로운 principal과 인증 토큰 생성
- Java 21의 패턴 매칭을 활용한 간결한 타입 체크
- authorities와 registration ID를 올바르게 보존
이러한 설계는 Spring Security OAuth2 모범 사례를 잘 따르고 있습니다.
.coderabbit.yaml (6)
67-67: 연산자 배치 규칙이 더 명확해졌습니다.이전 규칙에서 개선되어 연산자 간격과 줄바꿈 시 배치 규칙이 구체적으로 명시되었습니다. 특히 쉼표와 점의 위치를 명확히 한 것이 좋습니다. 이는 코드 리뷰 시 일관된 피드백을 제공하는 데 도움이 될 것입니다.
92-98: 유스케이스 인터페이스 규칙이 명확하게 정의되었습니다.단일 책임 원칙과 명확한 비즈니스 의도 표현을 강조한 가이드라인입니다. 특히 다음 사항들이 잘 정의되어 있습니다:
- 인터페이스당 하나의 public 메서드
- 비즈니스 의도를 명확히 표현하는 메서드명
- 복잡한 매개변수를 위한 Command/Query 객체 사용
이는 CQRS 패턴과 클린 아키텍처 원칙에 부합하는 좋은 지침입니다.
99-107: Command 객체에 대한 강력한 검증 규칙이 수립되었습니다.불변 레코드와 compact constructor 내 검증을 요구하는 이 가이드라인은 다음과 같은 장점이 있습니다:
- 입력 정규화(strip 등)를 검증 전에 수행하여 일관성 보장
- 옵셔널 필드에 대한 합리적인 기본값 제공 (예:
Set.of())BusinessException을 통한 일관된 오류 처리- 쓰기 작업에 집중하는 명확한 역할 정의
특히 정규화를 검증 전에 수행하도록 명시한 것은 일반적인 버그를 예방하는 좋은 관행입니다.
108-115: Query 객체 규칙이 Command와 일관성 있게 정의되었습니다.읽기 작업에 특화된 이 가이드라인은 Command 패턴과 유사한 구조를 유지하면서도 다음과 같은 Query 특성을 반영합니다:
- 페이지네이션 매개변수의 명확한 검증 규칙 (
size > 0,size <= 100,lastId > 0)- 읽기 작업에 필요한 매개변수만 포함
- 불변 레코드와 compact constructor 검증
특히
size <= 100제한은 의도치 않은 대량 쿼리를 방지하는 실용적인 가드레일입니다.
116-122: Result 객체 가이드라인이 캡슐화 원칙을 잘 반영하고 있습니다.프레젠테이션 레이어를 위한 Result 객체에 대한 이 규칙은 다음과 같은 우수한 관행을 포함합니다:
- 불변 레코드로 안전성 보장
from정적 팩토리 메서드를 통한 변환의 일관성- 도메인 내부 구조 노출 방지로 계층 간 경계 유지
- 깊은 중첩 구조보다 조합을 선호하여 유지보수성 향상
특히 "도메인 내부를 노출하지 않음"을 강조한 것은 애플리케이션 계층과 도메인 계층 간의 적절한 경계를 유지하는 데 중요합니다.
123-132: 서비스 레이어 가이드라인이 DDD 원칙을 효과적으로 구현하고 있습니다.이 규칙은 서비스 레이어의 역할을 명확히 정의하며 다음과 같은 모범 사례를 포함합니다:
- Read/Write 분리 패턴 (예:
FriendReadService,FriendWriteService)- 적절한 트랜잭션 관리 (
@Transactional,@Transactional(readOnly = true))- 도메인 엔티티에 비즈니스 로직 위임 - 서비스는 오케스트레이션만 수행
- 엔티티 직접 조작 대신 도메인 메서드 호출 선호
특히 "서비스는 도메인 로직을 포함하지 않고 오케스트레이션만 수행"한다는 원칙은 풍부한 도메인 모델을 유지하는 데 핵심적입니다. 이는 빈약한 도메인 모델(Anemic Domain Model) 안티패턴을 방지합니다.
src/main/java/queuing/core/user/domain/entity/User.java (2)
89-96:changeProfile메서드의 조용한 무시(silent ignore) 동작에 대한 확인이 필요합니다.현재 구현은
nickname이나profileImageUrl이null또는 빈 문자열일 경우 아무런 예외 없이 해당 필드를 업데이트하지 않습니다. 이는 의도된 동작일 수 있으나, 호출자 입장에서 업데이트가 실패했는지 알 수 없습니다.또한,
profileImageUrl에 대해 빈 문자열("")을 허용하지 않는 것이 의도된 것인지 확인해 주세요. 사용자가 프로필 이미지를 명시적으로 삭제하려는 경우를 처리해야 할 수도 있습니다.
98-113: JPA 엔티티의equals()/hashCode()구현이 적절합니다.
id기반의 동등성 비교와null체크가 올바르게 구현되어 있습니다.final키워드를 사용하여 하위 클래스에서의 오버라이드를 방지한 점도 좋은 설계입니다.다만,
hashCode()가id에 의존하므로, 새로 생성되어 아직 영속화되지 않은 엔티티(id == null)를HashSet이나HashMap에 넣은 후 영속화하면 해시 충돌 문제가 발생할 수 있습니다. 이 점을 인지하고 계신지 확인 부탁드립니다.src/main/java/queuing/core/friend/application/usecase/GetFriendListUseCase.java (1)
7-9: LGTM!Use Case 인터페이스가 단일 비즈니스 연산을 나타내는 하나의 공개 메서드로 정의되어 있으며, 코딩 가이드라인을 잘 준수하고 있습니다.
GetListFriendQuery를 통한 CQRS 패턴 적용도 적절합니다.src/main/java/queuing/core/friend/presentation/controller/FriendController.java (1)
45-63: LGTM!
GetListFriendQuery를 사용한 CQRS 패턴 적용이 적절하며, 컨트롤러가 비즈니스 로직을 Use Case에 올바르게 위임하고 있습니다.@RequestParam의 기본값 설정과 선택적 파라미터 처리도 적절합니다.src/main/java/queuing/core/friend/domain/entity/Friend.java (2)
84-93: 도메인 메서드를 통한 친구 요청 생성이 잘 구현되었습니다.팩토리 메서드 패턴을 사용하여 유효성 검증과 객체 생성을 캡슐화한 점이 좋습니다. 이전 리뷰에서 언급된
User.equals()구현이 완료되어, Line 85의requester.equals(receiver)비교가 올바르게 동작합니다.
107-122: JPA 엔티티의equals()/hashCode()구현이 적절합니다.
User엔티티와 동일한 패턴으로id기반 동등성 비교가 올바르게 구현되어 있습니다.src/main/java/queuing/core/friend/application/service/FriendReadService.java (1)
21-44: LGTM!서비스 구현이 코딩 가이드라인을 잘 준수하고 있습니다:
@Service어노테이션과 Use Case 인터페이스 구현- Read/Write 분리 패턴에 따른
FriendReadService명명- 클래스 레벨의
@Transactional(readOnly = true)적용- 도메인 메서드(
friend.getCounterpart(user))를 활용한 비즈니스 로직 위임
GetListFriendQuery로의 전환이 깔끔하게 이루어졌습니다.src/main/java/queuing/core/room/application/service/MusicTagReadService.java (1)
10-27: DTO에서 Result 패턴으로의 마이그레이션이 적절하게 구현되었습니다.코딩 가이드라인을 준수하여 다음과 같이 개선되었습니다:
@Transactional(readOnly = true)를 클래스 레벨로 이동하여 읽기 전용 서비스 패턴을 명확히 표현- DTO 대신 Result 타입을 사용하여 CQRS 패턴과 일관성 확보
- 메서드 시그니처와 매핑 로직이 명확하고 간결함
src/main/java/queuing/core/room/domain/entity/Room.java (1)
88-103: 이 우려사항은 코드베이스에서 실제로 발생하지 않습니다.코드 검사 결과, Room 엔티티는
Room.builder()로 생성된 후 즉시roomRepository.save(room)을 통해 영속화됩니다(RoomWriteService.java:48-56). 코드베이스에서 영속화 전에 Room 엔티티를HashSet이나HashMap과 같은 해시 기반 컬렉션에 추가하는 패턴이 발견되지 않습니다. 또한 컬렉션을 사용하여 Room을 다루는 코드도 발견되지 않습니다.현재의
equals()/hashCode()구현은 표준 JPA 엔티티 패턴을 따르고 있으며, 실제 코드 사용 패턴에서는 이론적 우려사항이 나타나지 않습니다.src/main/java/queuing/core/room/domain/entity/MusicTag.java (1)
34-49: ID 기반equals()/hashCode()구현은 사용 패턴상 문제가 없습니다.
MusicTag엔티티는 항상 저장소에서 로드된 후(이미 ID가 할당됨) 컬렉션에 추가됩니다. 실제 코드에서는 영속화 전MusicTag를 컬렉션에 추가하는 패턴이 없으므로, ID 기반equals()/hashCode()구현이 적절합니다.Likely an incorrect or invalid review comment.
src/main/java/queuing/core/room/application/query/GetListRoomQuery.java (1)
10-21: 페이지네이션 유효성 검증이 잘 구현되어 있습니다.compact constructor에서 모든 입력값에 대한 유효성 검증을 수행하고 있으며, 특히 size의 상한값(100) 검증을 추가하여 과도한 데이터 요청을 방지하고 있습니다. 코딩 가이드라인을 잘 준수하고 있습니다.
src/main/java/queuing/core/room/application/service/RoomReadService.java (1)
19-25: 읽기 전용 트랜잭션이 적절히 적용되었습니다.클래스 레벨에
@Transactional(readOnly = true)를 적용한 것은 다른 읽기 서비스(UserReadService, FriendReadService)와 일관된 패턴입니다. CQRS 패턴에 맞춰 쿼리 객체를 사용하도록 메서드 시그니처를 변경한 것도 적절합니다.src/main/java/queuing/core/user/application/service/UserWriteService.java (4)
10-11: 새로운 의존성 임포트가 적절합니다.
AuthenticationOperations와UpdateUserProfileCommand임포트가 추가되었으며, 임포트 구성이 코딩 컨벤션을 준수하고 있습니다.
20-20: 의존성 주입이 올바르게 구성되었습니다.
AuthenticationOperations필드가 final로 선언되어 생성자 주입을 통해 올바르게 관리되고 있습니다.
23-31: @transactional 어노테이션과 도메인 메서드 활용이 적절합니다.
@Transactional어노테이션이 추가되어 쓰기 작업에 대한 트랜잭션 관리가 올바르게 구성되었습니다. 또한user.changeProfile()도메인 메서드를 호출하여 비즈니스 로직을 도메인 엔티티에 위임하고 있어 서비스 계층의 역할이 명확합니다. 관리되는 엔티티에 대한 변경사항은 트랜잭션 커밋 시 자동으로 영속화되므로 명시적인 save 호출이 없어도 문제없습니다.
changeProfile메서드의 두 번째 파라미터에null을 전달하고 있는데,UpdateUserProfileCommand가 오직nickname필드만 포함하도록 설계되었으므로 이는 의도된 동작입니다.changeProfile메서드가 더 일반적이어서profileImageUrl파라미터도 지원하지만, 현재 명령은 닉네임 업데이트만 수행하는 것이 설계 의도로 보입니다.
23-35: 공개 API 변경 없음현재 코드베이스를 검토한 결과,
UpdateNicknameCommand에서UpdateUserProfileCommand로의 변경은 이 PR에서 발생하지 않습니다.UpdateUserProfileUseCase인터페이스와UserWriteService구현체의 두 메서드(updateUserProfile,completeOnboarding)는 이미UpdateUserProfileCommand를 매개변수로 사용하고 있으며, 이는UserProfileController의 호출 지점에서도 일관되게 적용되어 있습니다.따라서 이 PR에서는 공개 Java API 변경이 없습니다.
Likely an incorrect or invalid review comment.
src/main/java/queuing/core/friend/application/query/GetListFriendQuery.java
Show resolved
Hide resolved
src/main/java/queuing/core/room/application/command/DeleteRoomCommand.java
Show resolved
Hide resolved
src/main/java/queuing/core/room/presentation/controller/RoomController.java
Outdated
Show resolved
Hide resolved
src/main/java/queuing/core/user/application/service/UserWriteService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @src/main/java/queuing/core/user/application/service/UserWriteService.java:
- Around line 53-58: validateNickname currently only checks for null and
uniqueness but allows empty or whitespace-only nicknames; update the method
(validateNickname in UserWriteService) to treat blank strings as invalid by
first verifying newNickname is not blank (use String.isBlank()) before comparing
or checking uniqueness, and throw the same
BusinessException(ErrorCode.USER_NICKNAME_DUPLICATED) or a more appropriate
error code if blank input should be handled differently; ensure you still skip
validation when newNickname is null or equals user.getNickname() and keep the
userRepository.existsByNickname(newNickname) check.
♻️ Duplicate comments (2)
src/main/java/queuing/core/room/presentation/controller/RoomController.java (2)
79-94: [중요] 방 삭제 시 소유권 검증이 누락되어 있습니다.현재 구현은 사용자 인증(
isAuthenticated())만 확인하고, 요청한 사용자가 실제 방의 소유자인지 검증하지 않습니다. 이전 리뷰에서 지적된 대로RoomWriteService.delete()메서드에서 소유권 검증이 수행되지 않고 있어, 인증된 모든 사용자가 임의의 방을 삭제할 수 있는 심각한 보안 취약점이 존재합니다.
DeleteRoomUseCase또는 하위 서비스 레이어에서Room.getOwner()와 요청한User를 비교하여 권한을 검증하는 로직을 반드시 추가해야 합니다.Based on learnings from past review comments.
🤖 Prompt for AI Agents
In @src/main/java/queuing/core/room/presentation/controller/RoomController.java around lines 85-90, the deleteRoomUseCase.delete call lacks ownership validation; update the underlying DeleteRoomUseCase implementation to verify that the Room.getOwner() matches the authenticated User before allowing deletion, throwing BusinessException(ErrorCode.UNAUTHORIZED_ROOM_DELETION) if ownership check fails.
60-61: Location 헤더의 URI 경로가 불완전합니다.Line 60에서
URI.create(slug)는 슬러그 값만 전달하여 불완전한 Location 헤더를 생성합니다. RESTful API 관례에 따라 생성된 리소스의 전체 경로를 제공해야 합니다.🔎 수정 제안
- return ResponseEntity.created(URI.create(slug)) + return ResponseEntity.created(URI.create("/api/v1/rooms/" + slug)) .body(ResponseBody.success(CreateRoomResponse.from(slug)));
🧹 Nitpick comments (2)
src/main/java/queuing/core/user/application/service/UserWriteService.java (1)
24-32: 영속성 처리 패턴의 일관성을 검토해 주세요.
updateUserProfile메서드는user.changeProfile()호출 후 명시적인save()또는saveAndFlush()호출 없이 트랜잭션 커밋에 의존하고 있습니다. JPA의 자동 영속성 관리로 정상 동작하지만,completeOnboarding메서드(line 48)는saveAndFlush()를 명시적으로 호출하고 있어 패턴이 일치하지 않습니다.두 메서드 모두 동일한 영속성 처리 패턴을 따르는 것이 코드의 일관성과 유지보수성 측면에서 권장됩니다.
🔎 일관성을 위한 제안
validateNickname(user, command.nickname()); user.changeProfile(command.nickname(), null); + userRepository.saveAndFlush(user); }src/main/java/queuing/core/room/presentation/controller/RoomController.java (1)
92-93: Boolean 반환 값이 오해를 불러일으킬 수 있습니다.삭제 작업은 항상
true를 반환하거나 예외를 발생시키므로,false를 반환하는 경우가 존재하지 않습니다. REST API 설계 관점에서 다음 중 하나를 고려해 보시기 바랍니다:
204 No Content상태 코드와 빈 응답 본문 반환- 현재 구조 유지하되 문서화로 명확히 표시
🔎 204 No Content 사용 시 수정 제안
- return ResponseEntity.ok() - .body(ResponseBody.success(true)); + return ResponseEntity.noContent().build();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/queuing/core/friend/application/query/GetListFriendQuery.javasrc/main/java/queuing/core/room/application/command/DeleteRoomCommand.javasrc/main/java/queuing/core/room/presentation/controller/RoomController.javasrc/main/java/queuing/core/user/application/service/UserWriteService.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/queuing/core/friend/application/query/GetListFriendQuery.java
- src/main/java/queuing/core/room/application/command/DeleteRoomCommand.java
🧰 Additional context used
📓 Path-based instructions (4)
**/*
⚙️ CodeRabbit configuration file
**/*: - For every finding, include evidence: file path(s), line number(s), rule names (e.g., Checkstyle), and links to credible docs when relevant.
- Wrap any tokens starting with '@' (e.g.,
@JoinColumn) in backticks in PR comments.
Files:
src/main/java/queuing/core/user/application/service/UserWriteService.javasrc/main/java/queuing/core/room/presentation/controller/RoomController.java
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: - Review against Java 21 and the rules in/.rules/checkstyle-rules.xml.
- Indentation: use spaces (no tabs), tab width = 4 spaces; files must end with Unix LF newline. (Team adaptation)
- Maximum line length: 120 characters.
- Imports: single-class imports only; allow wildcard for static imports; group imports with blank lines between sections.
- Operators: always one space before and after; on line breaks, place operators at the start of the next line (commas stay at end of line, dots at start of new line).
- Lambda expressions: omit parentheses for a single parameter; surround
->with spaces (param -> expression); use braces and explicitreturnfor multi-statement bodies; choose short, clear parameter names.- Prefer Java 21 standard APIs over Guava.
- Do not annotate immutable local variables with
finalunless required for an inner class.- Allow the
varkeyword when the value is a castnull.- For the complete NAVER Campus Hackday conventions, see: https://naver.github.io/hackday-conventions-java/
Files:
src/main/java/queuing/core/user/application/service/UserWriteService.javasrc/main/java/queuing/core/room/presentation/controller/RoomController.java
**/main/java/**/*.java
⚙️ CodeRabbit configuration file
**/main/java/**/*.java: - This project is mature and must preserve a stable, backward-compatible public Java API.
- In the "Changes" section, list up to 25 changes to the public Java API that could affect end users; if none, explicitly state "No public Java API changes in this PR."
- Define the public Java API as public/protected methods on public classes plus
module-info.java.- Derive the list by deeply analyzing code flow, including through private methods and calls to Java 21 and Guava.
- Report:
- New or removed public API methods
- Changes to return types or parameter types
- Behavioral changes that may impact consumers
- Use
System.out.printlninstead of logging frameworks.- For performance reasons, the project prefers for-loops; do not suggest converting between loops and streams.
Files:
src/main/java/queuing/core/user/application/service/UserWriteService.javasrc/main/java/queuing/core/room/presentation/controller/RoomController.java
**/application/service/**/*Service.java
⚙️ CodeRabbit configuration file
**/application/service/**/*Service.java: - Services must be annotated with@Serviceand implement one or more use case interfaces.
- Follow the Read/Write separation pattern: use separate classes for read operations (e.g.,
FriendReadService) and write operations (e.g.,FriendWriteService).- Write operations must be annotated with
@Transactional.- Read operations should use
@Transactional(readOnly = true)when appropriate.- Services should orchestrate domain logic, not contain it; delegate business rules to domain entities.
- Throw
BusinessExceptionwith appropriateErrorCodefor business rule violations.- Avoid direct manipulation of entities; prefer calling domain methods.
Files:
src/main/java/queuing/core/user/application/service/UserWriteService.java
🧬 Code graph analysis (1)
src/main/java/queuing/core/user/application/service/UserWriteService.java (2)
src/main/java/queuing/core/user/application/service/UserReadService.java (1)
Service(14-31)src/main/java/queuing/core/user/application/service/AuthenticationService.java (1)
Service(20-59)
🔇 Additional comments (5)
src/main/java/queuing/core/user/application/service/UserWriteService.java (1)
34-51: 캐시 무효화 처리가 올바르게 적용되었습니다.이전 리뷰에서 지적된 캐시 일관성 문제가
@CacheEvict어노테이션 추가로 적절하게 해결되었습니다.completeOnboarding메서드 실행 시profileCompleted캐시가 무효화되어,AuthenticationService.isOnboardingCompleted메서드 호출 시 최신 온보딩 상태가 반영됩니다.또한
saveAndFlush()호출로 데이터베이스 변경사항을 즉시 반영한 후authenticationOperations.updateOnboardingStatus(true)를 호출하는 순서도 올바릅니다.src/main/java/queuing/core/room/presentation/controller/RoomController.java (4)
11-35: import 구문이 CQRS 패턴에 맞게 잘 정리되었습니다.command/query/result 패키지 구조로의 리팩터링이 import 구문에 적절하게 반영되어 있으며, 새로운 삭제 기능을 위한 필요한 타입들이 추가되었습니다. 단일 클래스 import 규칙도 잘 준수하고 있습니다.
43-43: DeleteRoomUseCase 의존성 주입이 적절합니다.생성자 주입 패턴을 일관되게 사용하고 있으며, 필드가
private final로 올바르게 선언되었습니다.
69-77: 목록 조회 로직이 CQRS 쿼리 패턴에 맞게 잘 리팩터링되었습니다.
GetListRoomQuery를 사용한 조회 로직과RoomSummaryResponse매핑이 적절하며, 응답 구조가 명확하게 개선되었습니다.
47-61: 공개 API 변경 사항 확인 필요이 PR에서 다음과 같은 공개 API 변경이 발생했습니다:
- create() 메서드: 반환 타입이
ResponseEntity<ResponseBody<Boolean>>에서ResponseEntity<ResponseBody<CreateRoomResponse>>로 변경되었습니다 (Lines 47-61).- delete() 메서드: 새로운 공개 엔드포인트
DELETE /api/v1/rooms/{slug}가 추가되었습니다 (Lines 79-94).프로젝트가 안정적이고 하위 호환성을 유지해야 하는 성숙한 단계라는 코딩 가이드라인에 따라, 이러한 변경 사항이 API 문서에 명시되어 있는지 확인해 주시기 바랍니다.
As per coding guidelines for public Java API documentation.
Also applies to: 79-94
src/main/java/queuing/core/user/application/service/UserWriteService.java
Show resolved
Hide resolved
🚩 작업 요약