refactor: 내 모든 텃밭 id 조회#161
Merged
Merged
Conversation
WalkthroughUserController의 getMyGardenIds가 서비스로 위임되었고, UserService 전반에서 에러 처리를 CustomApiException/ErrorCode로 통일, 친구 물주기 일일 제한 상수 도입, 프로필 조회 로직(정렬/필터링/N+1 회피/null 안전성)이 재작성되었습니다. 공개 API 시그니처는 변경되지 않았습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as 클라이언트
participant UC as UserController
participant US as UserService
participant UR as UserRepository
rect rgba(224,240,255,0.5)
C->>UC: GET /users/me/gardens/ids
UC->>US: getMyGardenIds(user)
US->>UR: findById(user.id) (with gardens)
UR-->>US: Managed User(+gardens)
US-->>UC: Sorted, unlocked gardenId list
UC-->>C: 200 OK [ids]
end
sequenceDiagram
autonumber
participant C as 클라이언트
participant US as UserService
participant UR as UserRepository
participant WL as WateringLogRepo
rect rgba(235,255,235,0.5)
C->>US: getUserProfile(auth, profileUserId)
US->>UR: findByUuid(auth) → currentUser
US->>UR: findById(profileUserId) → profileUser(+gardens/avatars)
US->>WL: preload today's watered gardenIds by currentUser
WL-->>US: Set<gardenId>
US-->>C: Profile DTO (image, gardens(sorted), leftWaterCount, isWateringAbleByMe)
end
note over US: 오류는 CustomApiException(ErrorCode.*)로 반환
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
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. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🔇 Additional comments (1)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
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/presentation/UserController.java (1)
46-51: PathVariable/RequestParam 불일치로 400/404 가능매핑은 /me/{avatarId}인데 파라미터는 @RequestParam입니다. @PathVariable로 교체하세요.
- @PatchMapping("/me/{avatarId}") + @PatchMapping("/me/{avatarId}") public ResponseEntity<ApiResponse<Void>> updateAvatar( @AuthenticationPrincipal User user, @RequestBody @Valid AvatarChangeRequest request, - @RequestParam("avatarId") Long avatarId) { + @PathVariable("avatarId") Long avatarId) {src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java (3)
119-137: NPE/잘못된 Principal 대비 부족authentication null/미인증, 잘못된 UUID 문자열에 대비해야 합니다.
public User getCurrentUser() { - Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + if (authentication == null || !authentication.isAuthenticated()) { + throw new CustomApiException(ErrorCode.INVALID_TOKEN, "인증 정보를 찾을 수 없습니다."); + } Object principal = authentication.getPrincipal(); @@ - if (principal instanceof String uuidString) { - UUID userUuid = UUID.fromString(uuidString); + if (principal instanceof String uuidString) { + UUID userUuid; + try { + userUuid = UUID.fromString(uuidString); + } catch (IllegalArgumentException e) { + throw new CustomApiException(ErrorCode.INVALID_TOKEN, "유효하지 않은 사용자 식별자입니다."); + } return userRepository .findByUuid(userUuid) // UUID로 최신 유저 정보를 조회합니다. .orElseThrow(() -> new CustomApiException(ErrorCode.USER_NOT_FOUND)); }
145-158: 인증 주체 null 방어 및 컨트롤러 문서와의 정합성 확인user가 null일 수 있는 경로를 방어하세요. 또한 “잠김 제외” 정책을 컨트롤러 설명과 일치시켜 주세요.
public List<Long> getMyGardenIds(User user) { + if (user == null) { + throw new CustomApiException(ErrorCode.INVALID_TOKEN, "인증된 사용자 정보가 없습니다."); + }
177-193: NPE 위험: AvatarMaster가 null일 경우avatar.getAvatarMaster()가 null이면 NPE가 발생합니다.
- avatar -> { - // AI 아바타처럼 Avatar에 직접 저장된 고유 imageUrl이 있다면 그것을 우선 사용합니다. - if (avatar.getImageUrl() != null && !avatar.getImageUrl().isBlank()) { - return avatar.getImageUrl(); - } - // 없다면, AvatarMaster에 정의된 기본 이미지를 사용합니다. - return avatar.getAvatarMaster().getDefaultImageUrl(); - }) + avatar -> { + if (avatar.getImageUrl() != null && !avatar.getImageUrl().isBlank()) { + return avatar.getImageUrl(); + } + var master = avatar.getAvatarMaster(); + return master != null ? master.getDefaultImageUrl() : null; + })
🧹 Nitpick comments (1)
src/main/java/com/example/cp_main_be/domain/member/user/presentation/UserController.java (1)
83-91: 서비스 위임은 적절. 다만 문서(“모든 텃밭”)와 실제 동작(잠금 텃밭 제외) 불일치UserService.getMyGardenIds는 잠긴 텃밭을 제외합니다. API 설명에 이를 반영하거나 서비스에서 필터링을 제거해 일관성을 맞춰주세요.
- @Operation( - summary = "내 텃밭 ID 목록 조회", - description = "현재 로그인한 유저와 연결된 모든 텃밭의 ID 목록을 조회합니다. 텃밭 슬롯 번호 순으로 정렬됩니다.") + @Operation( + summary = "내 텃밭 ID 목록 조회", + description = "현재 로그인한 유저의 잠겨있지 않은 텃밭 ID 목록을 슬롯 번호 순으로 조회합니다.")
📜 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/com/example/cp_main_be/domain/member/user/presentation/UserController.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java(10 hunks)
🧰 Additional context used
🪛 GitHub Actions: Spring Boot CI/CD with AWS
src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java
[error] 59-59: Cannot find symbol AVATAR_NOT_FOUND in ErrorCode (referenced in UserService.java:59).
🔇 Additional comments (3)
src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java (3)
34-35: 매직 넘버 제거를 위한 상수 도입 좋습니다일관된 정책 관리에 유리합니다.
168-176: 프로필 조회 시 최신 조회 로직 일원화 OK현재/대상 사용자 조회를 서비스 규약(CustomApiException)으로 통일한 점 좋습니다.
215-221: N+1 회피를 위한 선조회(Set) 도입 좋습니다워터링 가능 여부 판정의 성능 개선에 기여합니다.
Comment on lines
57
to
60
| avatarRepository | ||
| .findById(avatarId) | ||
| .orElseThrow(() -> new AvatarNotFoundException("아바타를 찾을 수 없습니다.")); | ||
| .orElseThrow(() -> new CustomApiException(ErrorCode.AVATAR_NOT_FOUND)); | ||
|
|
There was a problem hiding this comment.
빌드 실패: ErrorCode.AVATAR_NOT_FOUND 심볼 없음
CI 오류를 막기 위해 기존 에러코드로 대체하거나(ErrorCode.NOT_FOUND 등), ErrorCode에 상수를 추가하세요. 단기해결(컴파일 우선) 패치 예시:
- .orElseThrow(() -> new CustomApiException(ErrorCode.AVATAR_NOT_FOUND));
+ .orElseThrow(() -> new CustomApiException(ErrorCode.NOT_FOUND, "아바타를 찾을 수 없습니다.")); // ErrorCode.NOT_FOUND가 없다면 적절한 기존 코드로 교체권장: ErrorCode에 도메인 전용 상수 추가 후 원복.
- AVATAR_NOT_FOUND
+ AVATAR_NOT_FOUNDCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Spring Boot CI/CD with AWS
[error] 59-59: Cannot find symbol AVATAR_NOT_FOUND in ErrorCode (referenced in UserService.java:59).
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java
around lines 57 to 60, the code references ErrorCode.AVATAR_NOT_FOUND which
doesn't exist and causes build failures; fix by either (preferred) adding a new
constant AVATAR_NOT_FOUND to the ErrorCode enum/class with an appropriate
message/code and use that here, or as a short-term compile-fix replace
ErrorCode.AVATAR_NOT_FOUND with an existing constant such as
ErrorCode.NOT_FOUND; ensure the ErrorCode file is saved/compiled and imports are
correct before committing.
Comment on lines
73
to
81
| public void updateNickname(User user, String newNickname) { | ||
|
|
||
| user.updateProfile(newNickname, null); | ||
| // [수정] stale한 user 객체 대신, ID로 최신 정보를 조회해서 사용합니다. | ||
| User managedUser = | ||
| userRepository | ||
| .findById(user.getId()) | ||
| .orElseThrow(() -> new CustomApiException(ErrorCode.USER_NOT_FOUND)); | ||
| managedUser.updateProfile(newNickname, null); | ||
| userRepository.save(user); | ||
| } |
There was a problem hiding this comment.
버그: managedUser를 수정하고도 save(user) 호출
영속 엔티티가 managedUser이므로 save 대상이 잘못되었습니다.
- managedUser.updateProfile(newNickname, null);
- userRepository.save(user);
+ managedUser.updateProfile(newNickname, null);
+ userRepository.save(managedUser); // 또는 @Transactional에 의존해 생략🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java
around lines 73 to 81, the code updates the fetched managedUser but calls
userRepository.save(user) (the stale/unmanaged instance); change the save target
to the managed entity by calling userRepository.save(managedUser) (or remove the
explicit save entirely if relying on JPA dirty checking) so the persisted update
is applied to the correct entity.
Comment on lines
225
to
239
| profileUser.getGardens().stream() | ||
| .sorted(Comparator.comparing(Garden::getSlotNumber)) | ||
| .map( | ||
| garden -> { | ||
| // 현재 접속 유저가 이 정원에 오늘 물을 줄 수 있는지 여부 | ||
| boolean isWateringAbleByMe = false; | ||
| // 조건: 1) 아직 오늘 남에게 물 줄 수 있는 횟수가 남아있어야 하고 (leftWaterCountForOthers > 0) | ||
| // 2) 오늘 이 정원에 내가 물을 준 적이 없어야 한다. | ||
| if (leftWaterCountForOthers > 0) { | ||
| boolean alreadyWateredByMe = | ||
| friendWateringLogRepository | ||
| .existsByWaterGiverAndWateredGardenAndWateredAtAfter( | ||
| currentUser, garden, startOfWateringDay); | ||
| isWateringAbleByMe = !alreadyWateredByMe; | ||
| } | ||
| // DB를 반복 조회하는 대신, 미리 조회한 Set에서 확인하여 성능을 개선합니다. | ||
| boolean alreadyWateredByMe = wateredGardenIds.contains(garden.getId()); | ||
| boolean isWateringAbleByMe = leftWaterCountForOthers > 0 && !alreadyWateredByMe; | ||
|
|
||
| HomeResponseDto.AvatarInfo avatarInfoForGarden = | ||
| HomeResponseDto.AvatarInfo.builder() | ||
| .avatarId(garden.getAvatar().getId()) | ||
| // 아바타가 없는 텃밭이 있을 수 있는 예외 케이스를 방어합니다. | ||
| .avatarId(garden.getAvatar() != null ? garden.getAvatar().getId() : null) | ||
| .avatarName(garden.getAvatar().getNickname()) | ||
| .avatarImageUrl(garden.getAvatar().getAvatarMaster().getDefaultImageUrl()) | ||
| .build(); |
There was a problem hiding this comment.
NPE 위험: 아바타가 없는 텃밭 처리 미흡
avatarId만 null-세이프, 이름/이미지 접근은 그대로 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();
+ var avatar = garden.getAvatar();
+ HomeResponseDto.AvatarInfo avatarInfoForGarden =
+ HomeResponseDto.AvatarInfo.builder()
+ .avatarId(avatar != null ? avatar.getId() : null)
+ .avatarName(avatar != null ? avatar.getNickname() : null)
+ .avatarImageUrl(
+ avatar != null
+ ? (avatar.getImageUrl() != null && !avatar.getImageUrl().isBlank()
+ ? avatar.getImageUrl()
+ : (avatar.getAvatarMaster() != null
+ ? avatar.getAvatarMaster().getDefaultImageUrl()
+ : null))
+ : null)
+ .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.
Suggested change
| profileUser.getGardens().stream() | |
| .sorted(Comparator.comparing(Garden::getSlotNumber)) | |
| .map( | |
| garden -> { | |
| // 현재 접속 유저가 이 정원에 오늘 물을 줄 수 있는지 여부 | |
| boolean isWateringAbleByMe = false; | |
| // 조건: 1) 아직 오늘 남에게 물 줄 수 있는 횟수가 남아있어야 하고 (leftWaterCountForOthers > 0) | |
| // 2) 오늘 이 정원에 내가 물을 준 적이 없어야 한다. | |
| if (leftWaterCountForOthers > 0) { | |
| boolean alreadyWateredByMe = | |
| friendWateringLogRepository | |
| .existsByWaterGiverAndWateredGardenAndWateredAtAfter( | |
| currentUser, garden, startOfWateringDay); | |
| isWateringAbleByMe = !alreadyWateredByMe; | |
| } | |
| // DB를 반복 조회하는 대신, 미리 조회한 Set에서 확인하여 성능을 개선합니다. | |
| boolean alreadyWateredByMe = wateredGardenIds.contains(garden.getId()); | |
| boolean isWateringAbleByMe = leftWaterCountForOthers > 0 && !alreadyWateredByMe; | |
| HomeResponseDto.AvatarInfo avatarInfoForGarden = | |
| HomeResponseDto.AvatarInfo.builder() | |
| .avatarId(garden.getAvatar().getId()) | |
| // 아바타가 없는 텃밭이 있을 수 있는 예외 케이스를 방어합니다. | |
| .avatarId(garden.getAvatar() != null ? garden.getAvatar().getId() : null) | |
| .avatarName(garden.getAvatar().getNickname()) | |
| .avatarImageUrl(garden.getAvatar().getAvatarMaster().getDefaultImageUrl()) | |
| .build(); | |
| profileUser.getGardens().stream() | |
| .sorted(Comparator.comparing(Garden::getSlotNumber)) | |
| .map( | |
| garden -> { | |
| // DB를 반복 조회하는 대신, 미리 조회한 Set에서 확인하여 성능을 개선합니다. | |
| boolean alreadyWateredByMe = wateredGardenIds.contains(garden.getId()); | |
| boolean isWateringAbleByMe = leftWaterCountForOthers > 0 && !alreadyWateredByMe; | |
| var avatar = garden.getAvatar(); | |
| HomeResponseDto.AvatarInfo avatarInfoForGarden = | |
| HomeResponseDto.AvatarInfo.builder() | |
| .avatarId(avatar != null ? avatar.getId() : null) | |
| .avatarName(avatar != null ? avatar.getNickname() : null) | |
| .avatarImageUrl( | |
| avatar != null | |
| ? (avatar.getImageUrl() != null && !avatar.getImageUrl().isBlank() | |
| ? avatar.getImageUrl() | |
| : (avatar.getAvatarMaster() != null | |
| ? avatar.getAvatarMaster().getDefaultImageUrl() | |
| : null)) | |
| : null) | |
| .build(); | |
| // ...rest of mapping logic... | |
| }) |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java
around lines 225 to 239, the builder assumes garden.getAvatar() (and
avatar.getAvatarMaster()) are non-null which causes NPEs; make all
avatar-derived fields null-safe by checking garden.getAvatar() before accessing
nickname and avatarMaster, and also guard avatar.getAvatarMaster() before
calling getDefaultImageUrl(), populating avatarId, avatarName and avatarImageUrl
with null (or sensible defaults) when the corresponding objects are absent.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
📝 개요
이번 PR의 핵심 내용을 한 줄로 요약해 주세요.
💻 작업 내용
이번 PR에서 작업한 내용을 상세히 설명해 주세요.
작업 내용 1
작업 내용 2
...
✅ PR 체크리스트
PR을 보내기 전에 아래 체크리스트를 확인해 주세요.
커밋 메시지는 포맷에 맞게 작성했나요?
스스로 코드를 다시 한번 검토했나요?
관련 이슈를 연결했나요?
빌드 및 테스트가 로컬에서 성공했나요?
🔗 관련 이슈
스크린샷 (선택)
UI 변경 사항이 있다면 스크린샷을 첨부해 주세요.
Summary by CodeRabbit