-
Notifications
You must be signed in to change notification settings - Fork 1
[refactor] 유저 저장 장소 목록 조회 관련 리팩토링 #57
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
|
""" Walkthrough이 변경사항은 사용자 인증 및 권한 검증 방식을 개선하기 위해 JWT 토큰 파싱을 제거하고, Spring Security 컨텍스트에서 직접 사용자 ID를 추출하도록 컨트롤러와 서비스 로직을 리팩토링합니다. 또한, 엔티티와 DTO 구조에 일부 필드가 추가 또는 제거되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SecurityContext
participant UserPlaceController
participant UserPlaceService
Client->>UserPlaceController: POST /users/{userId}/places (Authorization 헤더 포함)
UserPlaceController->>SecurityContext: 인증 사용자 ID 조회
UserPlaceController->>UserPlaceController: userId 경로 변수와 인증 ID 비교
UserPlaceController->>UserPlaceService: saveUserPlace(userId, requestDto)
UserPlaceService-->>UserPlaceController: 저장 완료
UserPlaceController-->>Client: 응답 반환
Possibly related PRs
Suggested reviewers
Poem
""" 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 2
🧹 Nitpick comments (1)
src/main/java/com/wayble/server/wayblezone/entity/WaybleZone.java (1)
66-68: 불변성 패턴과의 일관성을 위해 setter 제거를 고려해보세요.엔티티가
@Builder(access = AccessLevel.PRIVATE)로 불변성을 지향하고 있는데, public setter가 추가되어 일관성이 깨집니다. 대표 이미지 URL 설정이 필요하다면 빌더 패턴을 통해 처리하거나, 정말 필요한 경우에만 package-private으로 제한하는 것을 권장합니다.- // 혹시 필요할수도 있어서 추가해놓음 - public void setMainImageUrl(String mainImageUrl) { - this.mainImageUrl = mainImageUrl; - } + // 패키지 내부에서만 사용 가능하도록 제한 + void setMainImageUrl(String mainImageUrl) { + this.mainImageUrl = mainImageUrl; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java(3 hunks)src/main/java/com/wayble/server/user/dto/UserPlaceRequestDto.java(0 hunks)src/main/java/com/wayble/server/user/service/UserPlaceService.java(2 hunks)src/main/java/com/wayble/server/wayblezone/entity/WaybleZone.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/wayble/server/user/dto/UserPlaceRequestDto.java
🧰 Additional context used
🧠 Learnings (1)
src/main/java/com/wayble/server/wayblezone/entity/WaybleZone.java (1)
Learnt from: seung-in-Yoo
PR: Wayble-Project/wayble-spring#37
File: src/main/java/com/wayble/server/user/entity/User.java:61-62
Timestamp: 2025-07-13T15:08:59.318Z
Learning: Java에서 같은 패키지 내의 클래스들은 import 구문 없이 서로를 참조할 수 있다. com.wayble.server.user.entity 패키지 내의 클래스들은 명시적인 import 없이 사용 가능하다.
🔇 Additional comments (4)
src/main/java/com/wayble/server/user/service/UserPlaceService.java (2)
32-32: 인증 방식 개선에 따른 올바른 메서드 시그니처 변경입니다.RequestBody에서 userId를 제거하고 별도 파라미터로 받는 것은 보안 강화에 도움이 됩니다. JWT에서 추출한 userId를 직접 전달받아 신뢰할 수 있는 값으로 처리하는 좋은 접근입니다.
75-75: N+1 쿼리 문제 해결이 효과적으로 구현되었습니다.기존의 스트림 연산을 통한 이미지 검색을
getMainImageUrl()직접 호출로 변경하여 성능이 크게 개선될 것입니다. 대표 이미지 URL을 미리 저장해두는 비정규화 접근법이 적절합니다.src/main/java/com/wayble/server/user/controller/UserPlaceController.java (2)
41-47: Spring Security 컨텍스트 기반 인증으로의 개선이 잘 구현되었습니다.JWT 토큰을 수동으로 파싱하던 방식에서 Spring Security 컨텍스트를 활용하는 방식으로 변경한 것은 보안성과 유지보수성 측면에서 훌륭한 개선입니다. 권한 검증 로직도 적절하게 구현되어 있습니다.
Also applies to: 64-67
41-41: SecurityContextHolder.getContext().getAuthentication().getPrincipal() 반환값이 Long임을 확인했습니다.
- JwtAuthentication 클래스의
getPrincipal()은 internaluserId(Long)를 반환합니다.- 따라서
Long tokenUserId = (Long) SecurityContextHolder.getContext().getAuthentication().getPrincipal();캐스팅은 안전합니다.
| public CommonResponse<String> saveUserPlace( | ||
| @PathVariable Long userId, | ||
| @RequestBody @Valid UserPlaceRequestDto request, | ||
| @RequestHeader(value = "Authorization") String authorizationHeader |
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.
🛠️ Refactor suggestion
사용하지 않는 Authorization 헤더 파라미터를 제거하세요.
SecurityContextHolder를 사용하게 되면서 Authorization 헤더를 직접 파싱하지 않으므로, 해당 파라미터는 더 이상 필요하지 않습니다. JWT 필터에서 이미 처리되었기 때문입니다.
public CommonResponse<String> saveUserPlace(
@PathVariable Long userId,
- @RequestBody @Valid UserPlaceRequestDto request,
- @RequestHeader(value = "Authorization") String authorizationHeader
+ @RequestBody @Valid UserPlaceRequestDto request
) { public CommonResponse<List<UserPlaceListResponseDto>> getUserPlaces(
- @PathVariable Long userId,
- @RequestHeader("Authorization") String authorizationHeader
+ @PathVariable Long userId
) {Also applies to: 62-62
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/user/controller/UserPlaceController.java at
lines 39 and 62, remove the unused Authorization header parameter from the
method signatures. Since SecurityContextHolder and the JWT filter handle
authorization, you no longer need to parse the Authorization header manually, so
delete the @RequestHeader(value = "Authorization") String authorizationHeader
parameters from these lines.
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.
code rabbit 말대로 Authorization 헤더는 빼는게 좋을 것 같네요!!!
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.
@KiSeungMin 알겠습니다! Authorization 헤더 파라미터와 관련된 잔여 코드(메서드 파라미터 및 불필요한 import) 제거 계획을 아래와 같이 잡았습니다.
• UserPlaceController에서 Authorization 헤더 파라미터 삭제 및 불필요한 import 정리
</plan_code_changes>
⏳ Preparing changes...
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.
A summary of the changes CodeRabbit can apply:
- Remove the Authorization header parameters from the saveUserPlace and getUserPlaces methods and delete the unused JwtTokenProvider import in the UserPlaceController.java file.
- ✅ Create PR with these edits
- 📌 Create commit in current branch
- 📋 Get copyable edits
| // 대표 이미지 필드 추가 | ||
| @Column(name = "main_image_url") | ||
| private String mainImageUrl; |
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
N+1 쿼리 문제 해결을 위한 좋은 접근입니다.
대표 이미지 URL을 별도 필드로 저장하여 스트림 연산을 통한 반복적인 이미지 조회를 방지하는 것은 성능 최적화에 효과적입니다. 데이터 정합성 유지를 위해 이미지 업데이트 시 mainImageUrl도 함께 업데이트되는 로직이 구현되어 있는지 확인해보세요.
이미지 업데이트 로직에서 mainImageUrl 동기화가 제대로 구현되어 있는지 확인해보겠습니다:
🏁 Script executed:
#!/bin/bash
# Description: mainImageUrl 필드를 업데이트하는 로직이 있는지 확인
# WaybleZoneImage 관련 서비스에서 mainImageUrl 업데이트 로직 찾기
rg -A 10 "setMainImageUrl|mainImageUrl" --type java
# 이미지 업로드/수정 관련 코드에서 mainImageUrl 처리 확인
ast-grep --pattern 'setMainImageUrl($$$)'Length of output: 724
mainImageUrl 동기화 로직 구현 검토 필요
현재 WaybleZone 엔티티에 setMainImageUrl 메서드만 정의되어 있고, 서비스 레이어나 이미지 업로드·수정 처리 과정에서 해당 메서드를 호출해 필드를 갱신하는 로직은 확인되지 않습니다.
데이터 정합성을 위해 이미지 등록/수정 시 mainImageUrl이 반드시 함께 업데이트되도록 아래 위치를 검토·보완해주세요.
- 이미지 업로드/수정을 담당하는 서비스 (예:
WaybleZoneImageService,WaybleZoneService등) - 컨트롤러나 핸들러 레이어에서 이미지 변경 후
zone.setMainImageUrl(...)호출 여부
예시:
// 이미지 저장 후 대표 이미지 URL 동기화
WaybleZone zone = waybleZoneRepository.findById(zoneId)
.orElseThrow(...);
zone.setMainImageUrl(savedImage.getUrl());
waybleZoneRepository.save(zone);🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/wayblezone/entity/WaybleZone.java around
lines 61 to 63, the mainImageUrl field has a setter but no synchronization logic
to update it during image upload or modification. Review the image upload and
update processes in services like WaybleZoneImageService or WaybleZoneService
and ensure that after saving or updating an image, the corresponding WaybleZone
entity's mainImageUrl is updated by calling setMainImageUrl with the new image
URL, followed by saving the entity to maintain data consistency.
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.
나중에 사용할수도 있어서 일단 정의 해놓은거라 뺄수도 있고 로직을 더 추가할 수도 있어서 우선 로직을 더 추가하게 된다면 그때 직접 구현할게
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.
@seung-in-Yoo 네, 충분히 이해됩니다! 현재 단계에서는 구조만 정의해두고 나중에 필요에 따라 구현하거나 제거하는 것이 합리적인 접근 방식입니다.
향후 mainImageUrl 동기화 로직을 구현하실 때는 이미지 변경 시점에 해당 필드도 함께 업데이트되도록 하시면 됩니다. 지금은 필드 정의만으로도 충분합니다.
/| /|
( :v: )
o_) (
\_) (_/
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.
오 code rabbit이랑 대화도 가능하군요...! ㅋㅋㅋㅋㅋㅋㅋㅋㅋ
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
KiSeungMin
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.
수고 많으셨어요!!!
| // 대표 이미지 필드 추가 | ||
| @Column(name = "main_image_url") | ||
| private String mainImageUrl; |
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.
오 code rabbit이랑 대화도 가능하군요...! ㅋㅋㅋㅋㅋㅋㅋㅋㅋ
| public CommonResponse<String> saveUserPlace( | ||
| @PathVariable Long userId, | ||
| @RequestBody @Valid UserPlaceRequestDto request, | ||
| @RequestHeader(value = "Authorization") String authorizationHeader |
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.
code rabbit 말대로 Authorization 헤더는 빼는게 좋을 것 같네요!!!
#️⃣ 연관된 이슈
#54
📝 작업 내용
유저 장소 저장 API 리팩토링
🖼️ 스크린샷 (선택)
대표 이미지 url 포함한 유저 장소 저장 성공
💬 리뷰 요구사항 (선택)
Summary by CodeRabbit
버그 수정
기능 개선
문서