-
Notifications
You must be signed in to change notification settings - Fork 3
refactor : 기본 언어가 저장되지 않은 유저에 대해 조회시 1번 언어로 자동 설정 및 조회되도록 설정 #183
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
WalkthroughUserService의 getUserInfo 트랜잭션을 읽기 전용에서 읽기/쓰기로 변경하고, 사용자 언어가 null인 경우 LanguageDomainService에서 기본 언어(1L)를 조회해 User에 설정한 뒤 응답을 생성합니다. User 엔티티에 공개 setter setLanguage가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant UserService
participant UserRepository as UserRepo
participant LanguageDomainService as LangSvc
Client->>UserService: getUserInfo(authUser)
UserService->>UserRepo: findById(authUser.userId)
UserRepo-->>UserService: User
alt 사용자 언어가 null
UserService->>LangSvc: getLanguage(1L)
LangSvc-->>UserService: Language(default)
UserService->>UserService: user.setLanguage(default)
else 사용자 언어가 존재
Note over UserService: 언어 유지
end
UserService-->>Client: UserInfoResponse(언어 포함)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/ezcode/codetest/application/usermanagement/user/service/UserService.java (1)
107-110: 신규 프로필 업로드 직후 “신규 이미지”가 삭제되는 치명적 버그
user.modifyProfileImage(profileImageUrl)로 새 URL을 세팅한 뒤, 삭제 시user.getProfileImageUrl()를 사용해 방금 업로드한 이미지를 지웁니다. 반드시 이전 URL(oldImageUrl)을 삭제해야 합니다.- user.modifyProfileImage(profileImageUrl); - if (oldImageUrl!=null) { - s3Uploader.delete(user.getProfileImageUrl(), "profile"); - } + user.modifyProfileImage(profileImageUrl); + if (oldImageUrl != null) { + s3Uploader.delete(oldImageUrl, "profile"); + }
🧹 Nitpick comments (3)
src/main/java/org/ezcode/codetest/domain/user/model/entity/User.java (1)
219-222: 공개 setter 추가로 엔티티 불변성 약화 + 널 가드 부재서비스 레이어 어디서든 임의 언어로 변경 가능해져 도메인 무결성이 약해집니다. 또한 null 전달에 대한 방어가 없습니다. 최소한 null 가드를 두고, 가능하면 “없을 때만 기본 언어를 적용”하는 도메인 메서드로 의도를 드러내는 편이 안전합니다.
다음처럼 방어 로직을 추가해 주세요(최소 변경):
- public void setLanguage(Language userLanguage) { - this.language = userLanguage; - } + public void setLanguage(Language userLanguage) { + if (userLanguage == null) { + throw new IllegalArgumentException("userLanguage must not be null"); + } + this.language = userLanguage; + }또는 “조회 시 기본 언어만 주입” 의도를 엔티티가 스스로 보장하도록:
+ public void applyDefaultLanguageIfAbsent(Language defaultLanguage) { + if (this.language == null) { + if (defaultLanguage == null) { + throw new IllegalArgumentException("defaultLanguage must not be null"); + } + this.language = defaultLanguage; + } + }서비스의 호출부는
user.applyDefaultLanguageIfAbsent(...)로 교체 가능합니다.src/main/java/org/ezcode/codetest/application/usermanagement/user/service/UserService.java (2)
59-59: getUserInfo가 쓰기 트랜잭션으로 변경됨 — 의도 확인 필요조회 메서드에서 DB 업데이트가 발생합니다(기본 언어 자동 설정). 재시도/캐시 계층/리드 레플리카 사용 환경에서 부작용이 될 수 있어 팀 합의가 필요합니다. 로그인 시점 또는 일회성 마이그레이션/배치로 보정하는 대안도 고려 바랍니다.
67-70: 기본 언어 ID 하드코딩(1L) 제거 및 도메인 위임 권장매직 넘버 1L 대신 상수/설정 또는 도메인 서비스 메서드로 의도를 명확히 해 주세요. 또한 null 체크 로직을 엔티티 메서드로 위임하면 재사용성과 안전성이 좋아집니다.
서비스 최소 수정안:
- if (user.getLanguage() == null) { - Language userLanguage = languageDomainService.getLanguage(1L); - user.setLanguage(userLanguage); - } + if (user.getLanguage() == null) { + Language defaultLanguage = languageDomainService.getLanguage(DEFAULT_LANGUAGE_ID); + user.setLanguage(defaultLanguage); // 또는 user.applyDefaultLanguageIfAbsent(defaultLanguage) + }클래스 상단에 상수 추가(권장):
private static final long DEFAULT_LANGUAGE_ID = 1L;또는 도메인 서비스에 전가:
Language defaultLanguage = languageDomainService.getDefaultLanguage();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/main/java/org/ezcode/codetest/application/usermanagement/user/service/UserService.java(1 hunks)src/main/java/org/ezcode/codetest/domain/user/model/entity/User.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
작업 내용
변경 사항
UserService.createUser()메서드 추가@Email유효성 검증 적용트러블 슈팅
@Transactional이 적용되지 않음this.→AopProxyUtils.사용)해결해야 할 문제
UserController에서 비즈니스 로직 일부 처리 → 서비스로 이전 고려 필요참고 사항
코드 리뷰 전 확인 체크리스트
type :)Summary by CodeRabbit