-
Notifications
You must be signed in to change notification settings - Fork 0
[FEAT] 사용자 정보 수정 API #167
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
[FEAT] 사용자 정보 수정 API #167
Conversation
📝 WalkthroughWalkthrough사용자 정보 수정 기능이 추가되었습니다: PATCH /api/v1/user/me 엔드포인트와 이에 따른 Request/Response DTO, 서비스/엔티티 변경 및 성공 코드가 도입되었고, 일부 사용자 조회·진행중 챌린지 처리 로직이 정리·확장되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as 클라이언트
participant Controller as UserController
participant Auth as Authentication (CustomUserDetails)
participant Service as UserServiceImpl
participant Repo as UserRepository
participant Storage as S3Service/URLResolver
Note over Client,Controller: PATCH /api/v1/user/me (부분 업데이트 가능)
Client->>Controller: PATCH /me {nickname?, profileImageKey?, isPublic?}
Controller->>Auth: 인증정보 확인 (`@AuthenticationPrincipal`)
Controller->>Service: updateUserInfo(userId, requestDto)
Service->>Repo: 사용자 조회 (by userId)
alt 닉네임 변경 요청
Service->>Repo: 닉네임 중복 검사 (existsByNickname)
end
Service->>Repo: 사용자 엔티티 수정 및 저장 (nickname/profileImageKey/isPublic)
Service->>Storage: profileImageKey -> full URL 해석 (필요 시)
Service-->>Controller: UpdateUserInfoResponseDto (nickname, profileImageUrl, isPublic, updatedAt)
Controller-->>Client: 200 USER_UPDATE_OK + 응답 DTO
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
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 |
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: 1
🧹 Nitpick comments (1)
src/main/java/com/hrr/backend/domain/user/service/UserService.java (1)
3-3: 와일드카드 import 사용에 대한 선택적 제안입니다.현재
import com.hrr.backend.domain.user.dto.*로 변경되었습니다. 일부 코딩 스타일 가이드는 명시적 import를 권장하지만, 같은 패키지의 여러 DTO를 사용하는 경우 와일드카드 import도 합리적입니다. 팀의 코딩 컨벤션에 따라 선택하세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/com/hrr/backend/domain/user/controller/UserController.javasrc/main/java/com/hrr/backend/domain/user/dto/UpdateUserInfoRequestDto.javasrc/main/java/com/hrr/backend/domain/user/dto/UpdateUserInfoResponseDto.javasrc/main/java/com/hrr/backend/domain/user/entity/User.javasrc/main/java/com/hrr/backend/domain/user/service/UserService.javasrc/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.javasrc/main/java/com/hrr/backend/global/response/SuccessCode.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-06T08:06:38.145Z
Learnt from: yerinchun
Repo: Hrr-HabbitRoutineResult/hrr-server PR: 26
File: src/main/java/com/hrr/backend/domain/user/entity/User.java:42-44
Timestamp: 2025-11-06T08:06:38.145Z
Learning: In the hrr-server project User entity (src/main/java/com/hrr/backend/domain/user/entity/User.java), the password field is nullable because the application uses social login (OAuth), and social login users do not have passwords stored in the database.
Applied to files:
src/main/java/com/hrr/backend/domain/user/controller/UserController.javasrc/main/java/com/hrr/backend/domain/user/service/UserService.java
🧬 Code graph analysis (4)
src/main/java/com/hrr/backend/domain/user/entity/User.java (2)
src/main/java/com/hrr/backend/domain/user/entity/UserTerm.java (1)
updateIsAgreed(45-47)src/main/java/com/hrr/backend/domain/user/service/UserTermServiceImpl.java (1)
ut(62-62)
src/main/java/com/hrr/backend/domain/user/dto/UpdateUserInfoResponseDto.java (2)
src/main/java/com/hrr/backend/domain/user/dto/UpdateUserInfoRequestDto.java (1)
Getter(9-24)src/main/java/com/hrr/backend/domain/user/dto/UserResponseDto.java (4)
Getter(16-56)UserResponseDto(14-162)Getter(58-88)Builder(90-161)
src/main/java/com/hrr/backend/domain/user/dto/UpdateUserInfoRequestDto.java (2)
src/main/java/com/hrr/backend/domain/user/dto/UpdateUserInfoResponseDto.java (1)
Getter(12-39)src/main/java/com/hrr/backend/domain/user/dto/UserNicknameRequestDto.java (1)
Getter(10-18)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
src/main/java/com/hrr/backend/domain/auth/service/SocialUserService.java (1)
socialAuth(134-159)
🔇 Additional comments (10)
src/main/java/com/hrr/backend/domain/user/entity/User.java (1)
133-136: LGTM! 기존 패턴과 일관된 업데이트 메서드입니다.다른 업데이트 메서드들(
updateNickname,updateProfileImage)과 동일한 패턴을 따르고 있어 일관성이 좋습니다.Boolean래퍼 타입 사용도 적절합니다.src/main/java/com/hrr/backend/global/response/SuccessCode.java (1)
27-28: 성공 코드가 올바르게 추가되었습니다.코드 네이밍(
USER2001)과 메시지가 기존 패턴과 일치하며,HttpStatus.OK도 적절합니다.src/main/java/com/hrr/backend/domain/user/service/UserService.java (1)
44-47: 새로운 서비스 메서드 시그니처가 명확합니다.메서드명과 파라미터가 의도를 잘 드러내고 있으며, 반환 타입도 적절합니다.
src/main/java/com/hrr/backend/domain/user/controller/UserController.java (1)
144-155: PATCH 엔드포인트가 RESTful 원칙에 맞게 구현되었습니다.
@Valid를 통한 요청 검증 ✓@AuthenticationPrincipal로 인증된 사용자 정보 활용 ✓- 적절한 성공 코드(
USER_UPDATE_OK) 사용 ✓- PATCH 메서드로 부분 업데이트 의도 명확 ✓
src/main/java/com/hrr/backend/domain/user/dto/UpdateUserInfoRequestDto.java (2)
19-23: 선택적 필드들이 부분 업데이트 요구사항을 잘 충족합니다.모든 필드를 optional로 설정하여 부분 업데이트(Partial Update)를 지원하는 것이 요구사항과 잘 맞습니다.
@Size검증이 null일 때는 실행되지 않으므로 의도대로 동작합니다.
15-17: 닉네임 길이 제약의 불일치 검증이 필요합니다.리뷰에서 언급된
UserNicknameRequestDto와의 불일치를 확인하려면 두 DTO의 검증 규칙과 User 엔티티의 컬럼 길이를 직접 비교해야 합니다. 또한 실제로 다른 제약이 있는지, 그리고 비즈니스 요구사항상 의도적인 차이인지 확인이 필요합니다.src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (3)
142-145: 프로필 이미지 업데이트 로직이 간결합니다.S3 Key를 직접 저장하는 방식이 명확하며, URL 변환은 응답 생성 시에만 수행하는 것이 효율적입니다.
147-150: 공개 여부 업데이트가 올바르게 구현되었습니다.새로 추가된
updateIsPublic메서드를 적절히 활용하고 있습니다.
152-155: S3 URL 변환 메서드의 null 안전성을 확인하세요.
user.getProfileImage()가 null을 반환할 수 있는 경우,s3UrlUtil.toFullUrl()메서드가 이를 어떻게 처리하는지 명확히 해야 합니다. 다음 사항을 확인하세요:
toFullUrl()메서드의 null 입력 처리 로직- null 입력 시 반환 값 (null, 빈 문자열, 기본 이미지 URL 등)
- 클라이언트에서 이 응답값을 안전하게 처리할 수 있는지 여부
필요시 null 체크를 추가하거나
Optional을 활용한 안전한 처리를 고려하세요.src/main/java/com/hrr/backend/domain/user/dto/UpdateUserInfoResponseDto.java (1)
31-38:profileImageUrlnull 안전성을 확인하고 명시적으로 처리하세요.팩토리 메서드가 깔끔하게 구성되었습니다. 다만
profileImageUrl이 null일 가능성을 고려해야 합니다. 서비스 레이어에서s3UrlUtil.toFullUrl(user.getProfileImage())를 호출할 때user.getProfileImage()가 null인 경우:
- S3UrlUtil.toFullUrl()이 null을 어떻게 처리하는지 확인하세요 — null을 그대로 반환하는지, 기본값을 반환하는지, 예외를 발생시키는지 확인 필요
- 필요시 service 레이어에서 null 처리를 명시적으로 수행하세요 — 예를 들어
user.getProfileImage() != null ? s3UrlUtil.toFullUrl(...) : null또는 기본 이미지 URL 지정- from() 메서드에
@Nullable또는@NotNull어노테이션을 추가하여 계약을 명시하세요 — 호출자가 null 가능성을 인지하도록이를 통해 null 안전성을 보장하고 예상치 못한 버그를 방지할 수 있습니다.
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 (2)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (2)
76-115: 컴파일 오류를 발생시키는 미완성 코드를 제거해야 합니다.Lines 76-115의 인증 확인 로직이 다음 문제를 일으키고 있습니다:
정의되지 않은 타입 및 변수 참조:
Challenge,ChallengeDays,VerificationStatus타입 미정의challengeRepository,verificationRepository필드 미선언LocalDate,LocalDateTime,LocalTime임포트 누락Pipeline 빌드 실패: 9개의 컴파일 에러가 발생하여 PR 머지가 차단되고 있습니다.
이 코드 블록을 완전히 제거하거나, 필요한 경우 별도 이슈로 분리하여 필수 의존성과 임포트를 추가한 후 구현하세요.
🔎 해당 코드 블록 제거 방안
// URL 변환 로직 교체 slice.getContent().forEach(dto -> dto.setThumbnailUrl(s3UrlUtil.toFullUrl(dto.getThumbnailUrl())) ); - // 인증 완료 여부 추가 - slice.getContent().forEach(dto -> { - Long challengeId = dto.getChallengeId(); - - // 챌린지 요일/인증 시간 로딩 - Challenge challenge = challengeRepository.findByIdWithDays(challengeId) - .orElseThrow(() -> new GlobalException(ErrorCode.CHALLENGE_NOT_FOUND)); - - // java의 DayOfWeek를 ChallengeDays로 변환 - DayOfWeek todayDayOfWeek = LocalDate.now().getDayOfWeek(); - ChallengeDays todayChallengeDay = ChallengeDays.from(todayDayOfWeek); - - // 오늘이 인증하는 날인지 체크 - boolean isRegistrationDay = challenge.getChallengeDays().stream() - .anyMatch(day -> day.getDayOfWeek() == todayChallengeDay); - - boolean verified = false; - - if (isRegistrationDay) { - // 유저-챌린지 매핑 (userChallengeId 필요) - Long userChallengeId = userChallengeRepository - .findByUserIdAndChallengeId(userId, challengeId) - .orElseThrow(() -> new GlobalException(ErrorCode.CHALLENGE_NOT_FOUND)) - .getId(); - - // 오늘 00:00 ~ 23:59 범위 설정 - LocalDateTime startOfDay = LocalDate.now().atStartOfDay(); - LocalDateTime endOfDay = LocalDate.now().atTime(LocalTime.MAX); - - - verified = verificationRepository.existsTodayVerification( // 오늘이 인증날이고, 인증을 완료했을 떄에만 true - userChallengeId, - VerificationStatus.COMPLETED, - startOfDay, - endOfDay - ); - } - - // 결과를 DTO 반영 - dto.setVerified(verified); - }); - // SliceResponseDto로 변환하여 반환 return new SliceResponseDto<>(slice);
33-33: 인터페이스 계약 위반으로 인한 컴파일 오류입니다.
UserService인터페이스는searchChallengers(User, String, int, int)메서드를 선언하고 있으나,UserServiceImpl에서 이를 구현하지 않았습니다. 이로 인해 컴파일이 실패합니다.해결 방안 (둘 중 하나):
- 해당 기능이 더 이상 필요 없다면
UserService인터페이스에서 메서드 선언 제거- 기능이 필요하다면
UserServiceImpl에 구현 추가 (슬라이싱 페이징으로 닉네임 검색 결과 반환)인터페이스-구현 클래스 간의 메서드 일치는 컴파일 타임에 강제되는 중요한 계약입니다. 이를 통해 런타임 오류를 사전에 방지할 수 있습니다.
♻️ Duplicate comments (1)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
192-196: 이전 리뷰에서 지적된 닉네임 정규화 누락 문제가 여전히 남아있습니다.Lines 192-196에서
requestDto.getNickname()을 정규화하지 않고 사용하여, 이전 리뷰에서 지적한 것과 동일한 문제가 발생합니다:문제점:
- Line 192:
"test "!="test"→ 변경된 것으로 잘못 판단- Line 193:
"test "로 중복 검사 → 실제"test"중복을 탐지 못함- Line 196: 정규화되지 않은 닉네임이 저장됨
Line 161의
setNickname메서드에서는normalize()를 올바르게 사용하고 있으므로, 동일한 패턴을 적용하세요.🔎 정규화를 추가하는 수정안
// 닉네임이 제공된 경우 중복 검사 및 업데이트 - if (requestDto.getNickname() != null && !requestDto.getNickname().equals(user.getNickname())) { - if (userRepository.existsByNickname(requestDto.getNickname())) { + if (requestDto.getNickname() != null) { + String normalizedNickname = normalize(requestDto.getNickname()); + if (!normalizedNickname.equals(user.getNickname())) { + if (userRepository.existsByNickname(normalizedNickname)) { throw new GlobalException(ErrorCode.NICKNAME_DUPLICATED); } - user.updateNickname(requestDto.getNickname()); + user.updateNickname(normalizedNickname); + } }참고:
normalize()메서드의 동작 원리와 문자열 정규화의 중요성에 대해서는 Java String API 문서를 참고하세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/hrr/backend/domain/user/entity/User.javasrc/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/hrr/backend/domain/user/entity/User.java
🧰 Additional context used
🪛 GitHub Actions: CI
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
[error] 33-33: UserServiceImpl is not abstract and does not override abstract method searchChallengers(User,String,int,int) in UserService
[error] 80-80: cannot find symbol: Challenge; method findByIdWithDays(challengeId) references a type or symbol that is undefined in this context
[error] 80-80: cannot find symbol: challengeRepository; variable or field not declared in UserServiceImpl
[error] 84-84: cannot find symbol: LocalDate; LocalDate.now() used but import missing or class not found
[error] 85-85: cannot find symbol: ChallengeDays; ChallengeDays.from(todayDayOfWeek) references an undefined type or symbol
[error] 101-101: cannot find symbol: LocalDateTime; LocalDate.now().atStartOfDay() used but import missing or class not found
[error] 102-102: cannot find symbol: LocalDate; LocalDate.now() used but import missing or class not found
[error] 102-102: cannot find symbol: LocalTime; LocalTime.MAX used but import missing or class not found
[error] 107-107: cannot find symbol: VerificationStatus; VERIFICATION status enum not found or not imported
[error] 105-105: cannot find symbol: verificationRepository; variable or field not declared in UserServiceImpl
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
♻️ Duplicate comments (1)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
293-299: 🚨 닉네임 정규화가 누락되어 중복 검증이 우회될 수 있습니다.이 이슈는 이전 리뷰에서 이미 지적되었으나 아직 수정되지 않았습니다.
문제점:
- Line 294:
requestDto.getNickname()을 정규화하지 않고 직접 비교- Line 295:
existsByNickname()에 정규화되지 않은 닉네임 전달- Line 298:
updateNickname()에 정규화되지 않은 닉네임 전달발생 가능한 시나리오:
1. DB에 "test" 닉네임 존재 2. 사용자가 "test " (뒤에 공백)로 변경 시도 3. Line 294: "test " != "test" → 변경으로 간주 4. Line 295: existsByNickname("test ") → false 반환 (실제로 "test"는 존재함) 5. Line 298: "test "가 DB에 저장됨 6. 결과: 정규화 후 중복인 닉네임이 허용됨다른 메서드들(
isNicknameAvailableLine 157,setNicknameLine 179,searchChallengersLine 213)에서는 모두normalize()메서드를 사용하고 있으므로, 일관성을 위해서라도 반드시 수정이 필요합니다.🔎 수정안
// 닉네임이 제공된 경우 중복 검사 및 업데이트 - if (requestDto.getNickname() != null && !requestDto.getNickname().equals(user.getNickname())) { - if (userRepository.existsByNickname(requestDto.getNickname())) { + if (requestDto.getNickname() != null) { + String normalizedNickname = normalize(requestDto.getNickname()); + if (!normalizedNickname.equals(user.getNickname())) { + if (userRepository.existsByNickname(normalizedNickname)) { throw new GlobalException(ErrorCode.NICKNAME_DUPLICATED); } - user.updateNickname(requestDto.getNickname()); + user.updateNickname(normalizedNickname); + } }
🧹 Nitpick comments (3)
src/main/java/com/hrr/backend/domain/user/service/UserService.java (1)
3-3: 와일드카드 import는 지양하는 것이 좋습니다.Java 코딩 컨벤션에서는 명시적인 import를 권장합니다. 와일드카드(
*) import는 네임스페이스를 오염시키고 실제 의존성을 불명확하게 만들 수 있습니다.다만 코드베이스 전반에 걸쳐 이 패턴이 사용되고 있다면, 일관성을 위해 현재 상태를 유지하되 향후 리팩토링 시 개선을 고려해보세요.
🔎 명시적 import로 변경하는 예시
-import com.hrr.backend.domain.user.dto.*; +import com.hrr.backend.domain.user.dto.UpdateUserInfoRequestDto; +import com.hrr.backend.domain.user.dto.UpdateUserInfoResponseDto; +import com.hrr.backend.domain.user.dto.UserResponseDto; +import com.hrr.backend.domain.user.dto.UserNicknameRequestDto; +import com.hrr.backend.domain.user.dto.UserNicknameResponseDto;src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (2)
91-132: 인증 상태 확인 로직이 기능적으로는 올바르나 복잡도가 높습니다.오늘 날짜가 인증 요일인지 확인하고 인증 완료 여부를 체크하는 로직이 정확하게 구현되었습니다. 다만
forEach내부의 로직이 복잡하여 가독성이 떨어집니다.향후 유지보수를 위해 이 로직을 별도의 헬퍼 메서드로 추출하는 것을 고려해보세요.
🔎 리팩토링 제안: 헬퍼 메서드 추출
private boolean checkTodayVerificationStatus(Long userId, Long challengeId) { // 챌린지 요일/인증 시간 로딩 Challenge challenge = challengeRepository.findByIdWithDays(challengeId) .orElseThrow(() -> new GlobalException(ErrorCode.CHALLENGE_NOT_FOUND)); // java의 DayOfWeek를 ChallengeDays로 변환 DayOfWeek todayDayOfWeek = LocalDate.now().getDayOfWeek(); ChallengeDays todayChallengeDay = ChallengeDays.from(todayDayOfWeek); // 오늘이 인증하는 날인지 체크 boolean isRegistrationDay = challenge.getChallengeDays().stream() .anyMatch(day -> day.getDayOfWeek() == todayChallengeDay); if (!isRegistrationDay) { return false; } // 유저-챌린지 매핑 Long userChallengeId = userChallengeRepository .findByUserIdAndChallengeId(userId, challengeId) .orElseThrow(() -> new GlobalException(ErrorCode.CHALLENGE_NOT_FOUND)) .getId(); // 오늘 00:00 ~ 23:59 범위 설정 LocalDateTime startOfDay = LocalDate.now().atStartOfDay(); LocalDateTime endOfDay = LocalDate.now().atTime(LocalTime.MAX); return verificationRepository.existsTodayVerification( userChallengeId, VerificationStatus.COMPLETED, startOfDay, endOfDay ); }그리고 forEach에서는 단순하게:
slice.getContent().forEach(dto -> { boolean verified = checkTodayVerificationStatus(userId, dto.getChallengeId()); dto.setVerified(verified); });
260-263: 정규화 메서드를 static으로 선언하는 것을 고려해보세요.
normalize메서드는 인스턴스 필드를 사용하지 않으므로private static메서드로 선언하는 것이 더 적절합니다.🔎 제안 수정
- private String normalize(String raw) { + private static String normalize(String raw) { if (raw == null) return ""; return raw.trim(); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/hrr/backend/domain/user/controller/UserController.javasrc/main/java/com/hrr/backend/domain/user/service/UserService.javasrc/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-06T08:06:38.145Z
Learnt from: yerinchun
Repo: Hrr-HabbitRoutineResult/hrr-server PR: 26
File: src/main/java/com/hrr/backend/domain/user/entity/User.java:42-44
Timestamp: 2025-11-06T08:06:38.145Z
Learning: In the hrr-server project User entity (src/main/java/com/hrr/backend/domain/user/entity/User.java), the password field is nullable because the application uses social login (OAuth), and social login users do not have passwords stored in the database.
Applied to files:
src/main/java/com/hrr/backend/domain/user/service/UserService.javasrc/main/java/com/hrr/backend/domain/user/controller/UserController.java
🔇 Additional comments (4)
src/main/java/com/hrr/backend/domain/user/service/UserService.java (1)
44-50: 새로운 메서드 시그니처가 잘 정의되었습니다!부분 업데이트(Partial Update) 패턴을 지원하는 API 시그니처가 명확하게 정의되었습니다. Javadoc도 적절히 작성되어 있어 메서드의 의도를 잘 전달하고 있습니다.
src/main/java/com/hrr/backend/domain/user/controller/UserController.java (1)
164-175: PATCH 엔드포인트가 RESTful 원칙에 맞게 잘 구현되었습니다!부분 업데이트를 위한 PATCH 메서드 사용,
@Valid를 통한 요청 검증, 그리고 인증된 사용자 정보 추출까지 모두 적절하게 처리되었습니다. Swagger 문서화도 명확합니다.src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (2)
198-258: N+1 문제를 잘 해결한 구현입니다!팔로우 관계를 bulk로 조회하고
HashSet을 활용하여 O(1) 시간 복잡도로 팔로우 여부를 확인하는 방식이 훌륭합니다. 성능을 고려한 좋은 구현입니다.
301-314: 프로필 이미지 및 공개 여부 업데이트 로직이 적절합니다.부분 업데이트를 위한 null 체크가 올바르게 구현되어 있고, S3 URL 변환도 적절하게 처리되었습니다. 응답 DTO 생성도 명확합니다.
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: 2
🧹 Nitpick comments (1)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
11-11: 와일드카드 import 대신 명시적 import를 고려하세요.
com.hrr.backend.domain.user.dto.*와일드카드 import는 네임스페이스 충돌 가능성과 코드 가독성을 저하시킬 수 있습니다. 사용하는 DTO 클래스들을 명시적으로 import하면 코드의 의존성이 더 명확해집니다.다만, 팀 코딩 컨벤션이 와일드카드를 허용한다면 무시하셔도 됩니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
src/main/java/com/hrr/backend/domain/user/dto/UserResponseDto.java (1)
UserResponseDto(14-162)
🔇 Additional comments (4)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (4)
241-242: 팔로워/팔로잉 수를 null로 설정한 의도를 확인하세요.검색 결과에서
followerCount와followingCount를 명시적으로null로 설정하고 있습니다. 이는 페이로드 최적화를 위한 의도적인 설계일 수 있지만,UserResponseDto.ProfileDto.from()정적 팩토리 메서드와는 다른 동작입니다.검색 결과에서 팔로워/팔로잉 수를 제외하는 것이 요구사항이 맞는지 확인해주세요. 만약 필요하다면, 추가 쿼리 없이 User 엔티티에서 직접 가져올 수 있습니다.
260-263: 깔끔한 정규화 헬퍼 메서드입니다! 👍닉네임 처리의 일관성을 위해 전역 헬퍼 메서드로 잘 구현되었습니다. null 처리와 trim 로직이 적절합니다.
286-319: 부분 업데이트 패턴이 올바르게 구현되었습니다.각 필드를 null 체크한 후 선택적으로 업데이트하는 로직이 PR 요구사항과 일치합니다. S3 이미지 Key 저장(Line 307)과 응답 시 Full URL 변환(Line 316) 패턴도 적절하게 적용되었습니다.
@Transactional어노테이션도 잘 적용되어 있어 데이터 일관성이 보장됩니다.
293-303: 이전 정규화 문제가 정확히 수정되었고, 데이터 일관성도 안전합니다! ✓Line 295의
normalize()적용이 제대로 되어 있으며, 더 중요한 것은 전체 시스템에서 모든 닉네임 저장 지점이 정규화를 일관되게 적용하고 있다는 점입니다:
setNickname()(line 179): 저장 전 정규화 ✓updateUserInfo()(line 295): 저장 전 정규화 ✓searchUsers()(line 213): 검색 전 정규화 ✓따라서 DB에 저장되는 모든 닉네임이 이미 정규화된 상태이므로, Line 297의
nickname.equals(user.getNickname())비교도 안전합니다. 데이터 일관성이 코드 레벨에서 강제되고 있으며,updateIsPublic()메서드도 정상 작동합니다. 좋은 구현입니다!
| // 인증 완료 여부 추가 | ||
| slice.getContent().forEach(dto -> { | ||
| Long challengeId = dto.getChallengeId(); | ||
|
|
||
| // 챌린지 요일/인증 시간 로딩 | ||
| Challenge challenge = challengeRepository.findByIdWithDays(challengeId) | ||
| .orElseThrow(() -> new GlobalException(ErrorCode.CHALLENGE_NOT_FOUND)); | ||
|
|
||
| // java의 DayOfWeek를 ChallengeDays로 변환 | ||
| DayOfWeek todayDayOfWeek = LocalDate.now().getDayOfWeek(); | ||
| ChallengeDays todayChallengeDay = ChallengeDays.from(todayDayOfWeek); | ||
|
|
||
| // 오늘이 인증하는 날인지 체크 | ||
| boolean isRegistrationDay = challenge.getChallengeDays().stream() | ||
| .anyMatch(day -> day.getDayOfWeek() == todayChallengeDay); | ||
|
|
||
| boolean verified = false; | ||
|
|
||
| if (isRegistrationDay) { | ||
| // 유저-챌린지 매핑 (userChallengeId 필요) | ||
| Long userChallengeId = userChallengeRepository | ||
| .findByUserIdAndChallengeId(userId, challengeId) | ||
| .orElseThrow(() -> new GlobalException(ErrorCode.CHALLENGE_NOT_FOUND)) | ||
| .getId(); | ||
|
|
||
| // 오늘 00:00 ~ 23:59 범위 설정 | ||
| LocalDateTime startOfDay = LocalDate.now().atStartOfDay(); | ||
| LocalDateTime endOfDay = LocalDate.now().atTime(LocalTime.MAX); | ||
|
|
||
|
|
||
| verified = verificationRepository.existsTodayVerification( // 오늘이 인증날이고, 인증을 완료했을 떄에만 true | ||
| userChallengeId, | ||
| VerificationStatus.COMPLETED, | ||
| startOfDay, | ||
| endOfDay | ||
| ); | ||
| } | ||
|
|
||
| // 인증 완료 여부 추가 | ||
| slice.getContent().forEach(dto -> { | ||
| Long challengeId = dto.getChallengeId(); | ||
|
|
||
| // 챌린지 요일/인증 시간 로딩 | ||
| Challenge challenge = challengeRepository.findByIdWithDays(challengeId) | ||
| .orElseThrow(() -> new GlobalException(ErrorCode.CHALLENGE_NOT_FOUND)); | ||
|
|
||
| // java의 DayOfWeek를 ChallengeDays로 변환 | ||
| DayOfWeek todayDayOfWeek = LocalDate.now().getDayOfWeek(); | ||
| ChallengeDays todayChallengeDay = ChallengeDays.from(todayDayOfWeek); | ||
|
|
||
| // 오늘이 인증하는 날인지 체크 | ||
| boolean isRegistrationDay = challenge.getChallengeDays().stream() | ||
| .anyMatch(day -> day.getDayOfWeek() == todayChallengeDay); | ||
|
|
||
| boolean verified = false; | ||
|
|
||
| if (isRegistrationDay) { | ||
| // 유저-챌린지 매핑 (userChallengeId 필요) | ||
| Long userChallengeId = userChallengeRepository | ||
| .findByUserIdAndChallengeId(userId, challengeId) | ||
| .orElseThrow(() -> new GlobalException(ErrorCode.CHALLENGE_NOT_FOUND)) | ||
| .getId(); | ||
|
|
||
| // 오늘 00:00 ~ 23:59 범위 설정 | ||
| LocalDateTime startOfDay = LocalDate.now().atStartOfDay(); | ||
| LocalDateTime endOfDay = LocalDate.now().atTime(LocalTime.MAX); | ||
|
|
||
|
|
||
| verified = verificationRepository.existsTodayVerification( // 오늘이 인증날이고, 인증을 완료했을 떄에만 true | ||
| userChallengeId, | ||
| VerificationStatus.COMPLETED, | ||
| startOfDay, | ||
| endOfDay | ||
| ); | ||
| } | ||
|
|
||
| // 결과를 DTO 반영 | ||
| dto.setVerified(verified); | ||
| }); | ||
| // 결과를 DTO 반영 | ||
| dto.setVerified(verified); | ||
| }); |
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.
N+1 쿼리 문제로 인한 성능 저하가 예상됩니다.
현재 구현은 각 진행 중인 챌린지마다 다음 쿼리들을 개별적으로 실행합니다:
challengeRepository.findByIdWithDays()(Line 97-98)userChallengeRepository.findByUserIdAndChallengeId()(Line 112-115)verificationRepository.existsTodayVerification()(Line 122-127)
사용자가 참여 중인 챌린지 수가 20개라면, 최대 60개 이상의 쿼리가 실행될 수 있습니다.
개선 방안:
- 챌린지 ID 리스트를 수집하여
IN절을 사용한 bulk 조회 Map<Long, Challenge>,Map<Long, UserChallenge>구조로 변환- 모든 userChallengeId를 한 번에 조회하여 인증 여부 확인
🔎 Bulk 조회 구조 예시
// 1. 모든 challengeId 수집
List<Long> challengeIds = slice.getContent().stream()
.map(UserResponseDto.OngoingChallengeDto::getChallengeId)
.collect(Collectors.toList());
// 2. Bulk 조회
List<Challenge> challenges = challengeRepository.findByIdInWithDays(challengeIds);
Map<Long, Challenge> challengeMap = challenges.stream()
.collect(Collectors.toMap(Challenge::getId, c -> c));
// 3. UserChallenge bulk 조회
List<UserChallenge> userChallenges = userChallengeRepository
.findByUserIdAndChallengeIdIn(userId, challengeIds);
Map<Long, UserChallenge> userChallengeMap = userChallenges.stream()
.collect(Collectors.toMap(uc -> uc.getChallenge().getId(), uc -> uc));
// 4. 인증 여부 bulk 조회
List<Long> userChallengeIds = userChallenges.stream()
.map(UserChallenge::getId)
.collect(Collectors.toList());
LocalDateTime startOfDay = LocalDate.now().atStartOfDay();
LocalDateTime endOfDay = LocalDate.now().atTime(LocalTime.MAX);
Set<Long> verifiedUserChallengeIds = verificationRepository
.findVerifiedUserChallengeIds(userChallengeIds, VerificationStatus.COMPLETED, startOfDay, endOfDay);
// 5. DTO에 반영 (단일 루프)
slice.getContent().forEach(dto -> {
Challenge challenge = challengeMap.get(dto.getChallengeId());
UserChallenge uc = userChallengeMap.get(dto.getChallengeId());
// ... 로직 적용
});Repository에 bulk 조회 메서드 추가가 필요하며, 이는 데이터베이스 부하를 크게 줄이고 응답 속도를 개선합니다.
참고 자료:
🤖 Prompt for AI Agents
In src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java around
lines 92-132, fix the N+1 query issue by collecting all challengeIds from
slice.getContent(), bulk-loading challenges with days (add
challengeRepository.findByIdInWithDays), bulk-loading UserChallenge entities for
the user with those challengeIds (add
userChallengeRepository.findByUserIdAndChallengeIdIn), extracting their ids and
bulk-checking verifications for today (add
verificationRepository.findVerifiedUserChallengeIds returning a Set of verified
userChallengeIds for the given status and time range), build maps
(challengeId->Challenge, challengeId->UserChallenge) and then in a single loop
set dto.setVerified(...) by checking if the corresponding userChallenge id is in
the verified set; remove per-item repository calls and use the preloaded maps
and verified id set.
| List<UserResponseDto.ProfileDto> profileDtos = targetUsers.stream() | ||
| .map(target -> { | ||
| // Set을 사용하여 O(1) 복잡도로 팔로우 여부 확인 | ||
| boolean isFollowing = followingIdSet.contains(target.getId()); | ||
|
|
||
| return UserResponseDto.ProfileDto.builder() | ||
| .userId(target.getId()) | ||
| .profileImage(target.getProfileImage()) | ||
| .nickname(target.getNickname()) | ||
| .followerCount(null) | ||
| .followingCount(null) | ||
| .level(target.getUserLevel()) | ||
| .isFollowing(isFollowing) // Set에서 가져온 값 사용 | ||
| .build(); | ||
| }) | ||
| .collect(Collectors.toList()); |
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.
프로필 이미지 URL 변환이 누락되었습니다.
Line 239에서 target.getProfileImage()를 그대로 사용하고 있는데, 다른 메서드들(예: getOngoingChallenges의 Line 88-89, updateUserInfo의 Line 316)에서는 s3UrlUtil.toFullUrl()을 사용하여 S3 Key를 전체 URL로 변환합니다.
일관성을 위해 여기서도 동일한 변환을 적용하는 것이 좋습니다.
🔎 수정 제안
return UserResponseDto.ProfileDto.builder()
.userId(target.getId())
- .profileImage(target.getProfileImage())
+ .profileImage(s3UrlUtil.toFullUrl(target.getProfileImage()))
.nickname(target.getNickname())
.followerCount(null)
.followingCount(null)
.level(target.getUserLevel())
.isFollowing(isFollowing)
.build();📝 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.
| List<UserResponseDto.ProfileDto> profileDtos = targetUsers.stream() | |
| .map(target -> { | |
| // Set을 사용하여 O(1) 복잡도로 팔로우 여부 확인 | |
| boolean isFollowing = followingIdSet.contains(target.getId()); | |
| return UserResponseDto.ProfileDto.builder() | |
| .userId(target.getId()) | |
| .profileImage(target.getProfileImage()) | |
| .nickname(target.getNickname()) | |
| .followerCount(null) | |
| .followingCount(null) | |
| .level(target.getUserLevel()) | |
| .isFollowing(isFollowing) // Set에서 가져온 값 사용 | |
| .build(); | |
| }) | |
| .collect(Collectors.toList()); | |
| List<UserResponseDto.ProfileDto> profileDtos = targetUsers.stream() | |
| .map(target -> { | |
| // Set을 사용하여 O(1) 복잡도로 팔로우 여부 확인 | |
| boolean isFollowing = followingIdSet.contains(target.getId()); | |
| return UserResponseDto.ProfileDto.builder() | |
| .userId(target.getId()) | |
| .profileImage(s3UrlUtil.toFullUrl(target.getProfileImage())) | |
| .nickname(target.getNickname()) | |
| .followerCount(null) | |
| .followingCount(null) | |
| .level(target.getUserLevel()) | |
| .isFollowing(isFollowing) // Set에서 가져온 값 사용 | |
| .build(); | |
| }) | |
| .collect(Collectors.toList()); |
🤖 Prompt for AI Agents
In src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java around
lines 232-247, the code uses target.getProfileImage() directly when building
ProfileDto; change this to convert the S3 key to a full URL by calling
s3UrlUtil.toFullUrl(target.getProfileImage()) (guarding for null/empty if
needed) and pass the converted URL into profileImage; ensure you use the
existing s3UrlUtil instance available in the service.
yc3697
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.
확인했습니다 수고하셨어요~
#️⃣ 연관된 이슈
✨ 작업 내용 (Summary)
사용자가 자신의 프로필 정보(닉네임, 프로필 이미지, 공개여부)를 수정할 수 있는 PATCH API를 구현했습니다.
✅ 변경 사항 체크리스트
🧪 테스트 결과
📸 스크린샷
💬 리뷰 요구사항
📎 참고 자료
Summary by CodeRabbit
새로운 기능
기타
✏️ Tip: You can customize this high-level summary in your review settings.