-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: 유저 정보 조회 수정 #162
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
WalkthroughgetUserProfile에서 프로필 소유자의 가든 목록을 구성할 때, 잠금 상태인 가든을 제외하도록 필터(.filter(g -> !g.isLocked()))가 정렬 이전에 추가됨. 공개 API 시그니처 변화 없음. 나머지 정렬, 아바타 정보, 물 주기 계산, 응답 생성 로직은 동일. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client as Client
participant UserService as UserService.getUserProfile
participant Domain as User/Garden Domain
Client->>UserService: 요청: getUserProfile(userId, viewerId)
UserService->>Domain: 사용자 및 보유 가든 조회
Note over UserService,Domain: 가든 리스트 스트림 처리
rect rgb(233, 246, 255)
Note right of UserService: 변경됨: 잠금 가든 제외<br/>filter(garden.isLocked == false)
UserService->>UserService: 정렬(슬롯 번호), 아바타 정보 구성
end
UserService->>UserService: 물 주기 가능 여부 계산
UserService-->>Client: 프로필 응답 반환
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
✨ 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java (4)
234-240: NPE 위험: avatar null 방어 누락avatarId만 null-safe이고, nickname·imageUrl 접근은 NPE 가능성이 있습니다.
- HomeResponseDto.AvatarInfo avatarInfoForGarden = - HomeResponseDto.AvatarInfo.builder() - // 아바타가 없는 텃밭이 있을 수 있는 예외 케이스를 방어합니다. - .avatarId(garden.getAvatar() != null ? garden.getAvatar().getId() : null) - .avatarName(garden.getAvatar().getNickname()) - .avatarImageUrl(garden.getAvatar().getAvatarMaster().getDefaultImageUrl()) - .build(); + Avatar avatar = garden.getAvatar(); + HomeResponseDto.AvatarInfo avatarInfoForGarden = + HomeResponseDto.AvatarInfo.builder() + .avatarId(avatar != null ? avatar.getId() : null) + .avatarName(avatar != null ? avatar.getNickname() : null) + .avatarImageUrl( + avatar == null + ? null + : (avatar.getImageUrl() != null && !avatar.getImageUrl().isBlank() + ? avatar.getImageUrl() + : avatar.getAvatarMaster().getDefaultImageUrl())) + .build();
73-81: 버그: managedUser 수정 후 잘못된 객체 저장변경은 managedUser에 했는데 저장은 user(영속 아님)로 하고 있습니다.
- managedUser.updateProfile(newNickname, null); - userRepository.save(user); + managedUser.updateProfile(newNickname, null); + // 트랜잭션 커밋 시 dirty checking으로 반영됩니다. 명시 save 불필요. + // userRepository.save(managedUser);
51-71: 권한 체크 누락: avatarId 소유권 미검증으로 임의 변경 가능avatar가 요청 유저의 소유인지 확인이 없습니다. 보안상 반드시 막아야 합니다. 또한 공백 입력 방지도 권장합니다.
Avatar avatar = avatarRepository .findById(avatarId) .orElseThrow(() -> new CustomApiException(ErrorCode.AVATAR_NOT_FOUND)); + // 소유권 검증 + if (avatar.getUser() == null || !Objects.equals(avatar.getUser().getId(), user.getId())) { + throw new CustomApiException(ErrorCode.FORBIDDEN, "자신의 아바타만 변경할 수 있습니다."); + } // [버그 수정] AvatarMaster(원본)가 아닌 Avatar(개별 인스턴스)의 imageUrl을 변경해야 합니다. // Avatar 엔티티에 imageUrl 필드가 있어야 합니다. - if (request.getNewAvatarUrl() != null) { + if (request.getNewAvatarUrl() != null && !request.getNewAvatarUrl().isBlank()) { // avatar.getAvatarMaster().setDefaultImageUrl(request.getNewAvatarUrl()); // 절대 이렇게 하면 안됩니다. avatar.setImageUrl(request.getNewAvatarUrl()); // 이렇게 수정해야 합니다. } - if (request.getNewAvatarName() != null) { + if (request.getNewAvatarName() != null && !request.getNewAvatarName().isBlank()) { avatar.setNickname(request.getNewAvatarName()); }
119-137: Null/Anonymous 인증 처리 누락으로 NPE 가능authentication 또는 principal이 null/anonymous일 수 있습니다. 방어 로직 추가를 권장합니다.
public User getCurrentUser() { - Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + if (authentication == null) { + throw new CustomApiException(ErrorCode.INVALID_TOKEN, "인증 정보를 찾을 수 없습니다."); + } Object principal = authentication.getPrincipal(); // Principal이 User 객체인 경우 (현재 JWT 필터에서 이렇게 저장함) if (principal instanceof User) { return (User) principal; } // Principal이 String(UUID)인 경우 (백업 처리) if (principal instanceof String uuidString) { UUID userUuid = UUID.fromString(uuidString); return userRepository .findByUuid(userUuid) // UUID로 최신 유저 정보를 조회합니다. .orElseThrow(() -> new CustomApiException(ErrorCode.USER_NOT_FOUND)); } - throw new CustomApiException(ErrorCode.INVALID_TOKEN, "인증 정보를 찾을 수 없습니다."); + throw new CustomApiException(ErrorCode.INVALID_TOKEN, "인증 정보를 찾을 수 없습니다."); }
🧹 Nitpick comments (5)
src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java (5)
167-167: 읽기 전용 트랜잭션으로 최적화 가능조회 전용 메서드이므로 readOnly=true로 선언하면 성능에 유리합니다.
- public UserProfileResponse getUserProfile(Long currentUserId, Long profileUserId) { + @Transactional(readOnly = true) + public UserProfileResponse getUserProfile(Long currentUserId, Long profileUserId) {
177-193: 프로필 이미지 선정 일관성: locked 고려 및 avatar.imageUrl 우선 적용 확인현재 프로필 이미지는 모든 텃밭 중 슬롯 최솟값 기준이며 locked 여부를 고려하지 않습니다. UI에서 locked 텃밭을 숨긴다면 여기서도 동일 기준 적용이 자연스럽습니다.
- profileUser.getGardens().stream() + profileUser.getGardens().stream() + .filter(g -> !g.isLocked()) // 자기 자신 노출 정책에 따라 조건부로 조정 가능 .min(Comparator.comparing(Garden::getSlotNumber)) // 슬롯 번호가 가장 낮은 텃밭 찾기
145-158: 쿼리 최적화 제안: 잠금 필터 DB 위임현재 메모리에서 isLocked 필터링합니다. fetch join 쿼리 단계에서 unlocked만 가져오면 네트워크·메모리 부담을 줄일 수 있습니다. 예: userRepository.findByIdWithUnlockedGardens(userId).
218-223: DB 인덱스 권고: (water_giver_id, watered_at) 복합 인덱스count·findWateredGardenIdsByGiverAndDate가 날짜 조건을 자주 사용하므로 복합 인덱스를 권장합니다. 대량 데이터에서 병목을 예방합니다.
262-271: 정오 기준 계산의 타임존/미래 대비 안정성Asia/Seoul은 현재 DST가 없지만, 시간 연산은 ZonedDateTime으로 계산 후 LocalDateTime으로 변환하는 편이 안전합니다. ZoneId 상수를 필드로 추출하는 것도 가독성에 좋습니다.
- private LocalDateTime getStartOfCurrentWateringDay() { - LocalDateTime now = LocalDateTime.now(ZoneId.of("Asia/Seoul")); - LocalDateTime todayNoon = now.toLocalDate().atTime(12, 0); - - if (now.isBefore(todayNoon)) { - return todayNoon.minusDays(1); - } else { - return todayNoon; - } - } + private static final ZoneId KST = ZoneId.of("Asia/Seoul"); + + private LocalDateTime getStartOfCurrentWateringDay() { + var now = java.time.ZonedDateTime.now(KST); + var todayNoon = now.toLocalDate().atTime(12, 0).atZone(KST); + return (now.isBefore(todayNoon) ? todayNoon.minusDays(1) : todayNoon).toLocalDateTime(); + }
📜 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 (1)
src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java(1 hunks)
| // 3. 프로필 주인의 정원 목록 및 물주기 가능 여부 계산 | ||
| List<UserGardenDetailResponse> userGardens = | ||
| profileUser.getGardens().stream() | ||
| .filter(garden -> !garden.isLocked()) // isLocked가 false인 텃밭만 가져옵니다. |
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.
💡 Verification agent
🧩 Analysis chain
잠금 텃밭 필터가 소유자 본인까지 숨깁니다 — 요구사항 확인 및 조건부 필터 제안
프로필 주인 본인이 자기 프로필을 볼 때도 locked 텃밭이 제외됩니다. locked 의미가 “타인에게만 비공개”라면 조건부로 필터링해야 합니다.
- .filter(garden -> !garden.isLocked()) // isLocked가 false인 텃밭만 가져옵니다.
+ .filter(garden -> Objects.equals(currentUserId, profileUserId) || !garden.isLocked())필터 조건 변경 전, currentUserId와 profileUserId가 해당 메서드 내에서 사용 가능한지 및 코드 컨텍스트를 확인합니다.
🏁 Script executed:
#!/bin/bash
# UserService.java 200~260행 출력
sed -n '200,260p' src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java
# 변수 currentUserId 사용 위치 확인
rg -n 'currentUserId' -C2 src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java
# 변수 profileUserId 사용 위치 확인
rg -n 'profileUserId' -C2 src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.javaLength of output: 4789
Objects.equals 사용을 위해 java.util.Objects가 import되어 있는지 확인합니다.
🏁 Script executed:
#!/bin/bash
rg -n 'import java.util.Objects' src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.javaLength of output: 115
java.util.Objects import 누락 & 조건부 필터 추가
Objects.equals 사용을 위해 import java.util.Objects;를 추가하고, 프로필 주인의 경우 locked 텃밭도 보여주도록 필터를 수정하세요.
+ import java.util.Objects;
...
- .filter(garden -> !garden.isLocked())
+ .filter(garden -> Objects.equals(currentUserId, profileUserId) || !garden.isLocked())📝 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.
| .filter(garden -> !garden.isLocked()) // isLocked가 false인 텃밭만 가져옵니다. | |
| // At the top of src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java, add: | |
| import java.util.Objects; | |
| ... | |
| // Then, in the stream pipeline (around line 226), replace: | |
| .filter(garden -> Objects.equals(currentUserId, profileUserId) || !garden.isLocked()) // isLocked가 false인 텃밭만 가져옵니다. |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java
around line 226, add the import "import java.util.Objects;" and change the
stream filter so locked gardens are still excluded for regular viewers but
included when the requester is the profile owner; replace the current
".filter(garden -> !garden.isLocked())" with a predicate using Objects.equals to
compare the garden owner id to the current/profile owner id (e.g.
".filter(garden -> !garden.isLocked() || Objects.equals(garden.getOwnerId(),
profileOwnerId))" or the equivalent fields/method params used in this method) to
avoid NPEs.
📝 개요
이번 PR의 핵심 내용을 한 줄로 요약해 주세요.
💻 작업 내용
이번 PR에서 작업한 내용을 상세히 설명해 주세요.
작업 내용 1
작업 내용 2
...
✅ PR 체크리스트
PR을 보내기 전에 아래 체크리스트를 확인해 주세요.
커밋 메시지는 포맷에 맞게 작성했나요?
스스로 코드를 다시 한번 검토했나요?
관련 이슈를 연결했나요?
빌드 및 테스트가 로컬에서 성공했나요?
🔗 관련 이슈
스크린샷 (선택)
UI 변경 사항이 있다면 스크린샷을 첨부해 주세요.
Summary by CodeRabbit