-
Notifications
You must be signed in to change notification settings - Fork 1
[feat] 내가 저정한 장소 목록 조회 API 구현 #55
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
|
""" WalkthroughJWT 기반 인증이 사용자 장소 저장 및 조회 API에 도입되었습니다. 사용자 본인 확인을 위해 토큰에서 추출한 userId와 요청의 userId가 일치하는지 검증하며, 일치하지 않을 경우 403 FORBIDDEN 오류를 반환합니다. 또한, 사용자가 저장한 장소 목록을 조회하는 새로운 GET 엔드포인트가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant JwtTokenProvider
participant UserPlaceService
participant Repository
Client->>Controller: POST/GET /api/v1/users/{userId}/places (Authorization: Bearer ...)
Controller->>JwtTokenProvider: 토큰에서 userId 추출
Controller-->>Controller: userId 일치 여부 검증
alt userId 불일치
Controller-->>Client: 403 FORBIDDEN 반환
else userId 일치
Controller->>UserPlaceService: getUserPlaces(userId) 또는 saveUserPlace(...)
UserPlaceService->>Repository: findAllByUserPlace_User_Id(userId)
Repository-->>UserPlaceService: 매핑 목록 반환
UserPlaceService-->>Controller: 장소 목록 DTO 반환
Controller-->>Client: 응답 반환
end
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java(3 hunks)src/main/java/com/wayble/server/user/dto/UserPlaceListResponseDto.java(1 hunks)src/main/java/com/wayble/server/user/exception/UserErrorCase.java(1 hunks)src/main/java/com/wayble/server/user/repository/UserPlaceWaybleZoneMappingRepository.java(1 hunks)src/main/java/com/wayble/server/user/service/UserPlaceService.java(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
src/main/java/com/wayble/server/user/repository/UserPlaceWaybleZoneMappingRepository.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.288Z
Learning: Java에서 같은 패키지 내의 클래스들은 import 구문 없이 서로를 참조할 수 있다. com.wayble.server.user.entity 패키지 내의 클래스들은 명시적인 import 없이 사용 가능하다.
src/main/java/com/wayble/server/user/service/UserPlaceService.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.288Z
Learning: Java에서 같은 패키지 내의 클래스들은 import 구문 없이 서로를 참조할 수 있다. com.wayble.server.user.entity 패키지 내의 클래스들은 명시적인 import 없이 사용 가능하다.
🔇 Additional comments (9)
src/main/java/com/wayble/server/user/exception/UserErrorCase.java (1)
16-17: LGTM! 권한 검증을 위한 새로운 에러 케이스가 올바르게 추가되었습니다.403 상태 코드와 적절한 에러 코드(1007), 그리고 일관된 한국어 메시지로 잘 구현되었습니다.
src/main/java/com/wayble/server/user/repository/UserPlaceWaybleZoneMappingRepository.java (2)
6-6: 적절한 import 추가새로운 메서드의 반환 타입을 위해 필요한 import입니다.
10-10: Spring Data JPA 컨벤션을 따른 올바른 메서드 구현
userPlace.user.id로의 중첩 프로퍼티 접근이 올바르게 구현되었습니다.src/main/java/com/wayble/server/user/dto/UserPlaceListResponseDto.java (1)
7-22: 잘 설계된 DTO 구조Record 클래스와
@Builder패턴을 사용한 불변 DTO가 잘 구현되었습니다. 중첩된WaybleZoneDto로 응답 구조가 명확하게 정의되었습니다.참고: 필드명이 snake_case(
place_id,wayble_zone_id등)를 사용하고 있는데, 이는 API 응답 형식을 위한 의도적인 선택으로 보입니다.src/main/java/com/wayble/server/user/service/UserPlaceService.java (2)
5-5: 필요한 import 추가새로운 DTO와 List 타입을 위한 적절한 import가 추가되었습니다.
Also applies to: 20-20
63-93: 잘 구현된 사용자 장소 조회 메서드
- 사용자 존재 여부를 먼저 확인하는 방어적 프로그래밍
- Stream API를 활용한 효율적인 데이터 변환
findFirst().orElse(null)로 안전한 이미지 URL 처리- 기존 메서드들과 일관된 에러 처리 패턴
구현이 깔끔하고 논리적입니다.
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (3)
3-3: JWT 인증 구현을 위한 적절한 import 추가필요한 JWT 관련 클래스와 새로운 DTO import가 올바르게 추가되었습니다.
Also applies to: 6-6, 17-17, 25-25
38-46: JWT 기반 인증 및 권한 검증 구현토큰에서 사용자 ID를 추출하고 경로 변수와 요청 바디의 사용자 ID와 비교하는 보안 로직이 잘 구현되었습니다.
56-80: 사용자 장소 목록 조회 API 구현 완료
- JWT 토큰 기반 인증 검증
- 적절한 API 문서화 (@operation, @ApiResponses)
- 일관된 에러 처리 및 응답 형식
- 보안을 고려한 사용자 권한 검증
새로운 GET 엔드포인트가 잘 구현되었습니다.
src/main/java/com/wayble/server/user/controller/UserPlaceController.java
Outdated
Show resolved
Hide resolved
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/user/controller/UserPlaceController.java (1)
40-41: 중복된 토큰 추출 로직 리팩토링 권장두 엔드포인트에서 동일한 토큰 추출 및 검증 로직이 반복됩니다. 공통 메서드로 추출하여 코드 중복을 제거하는 것을 권장합니다.
private Long extractAndValidateTokenUserId(String authorizationHeader) { String token = authorizationHeader.replace("Bearer ", ""); if (!jwtProvider.validateToken(token)) { throw new ApplicationException(UserErrorCase.FORBIDDEN); } return jwtProvider.getUserId(token); }Also applies to: 65-67
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (1)
src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthenticationFilter.java (1)
RequiredArgsConstructor(13-43)
⏰ 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: run-docker-image-on-ec2
🔇 Additional comments (2)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (2)
3-3: 새로운 import 문들이 적절히 추가됨JWT 인증과 새로운 GET 엔드포인트 기능을 위한 필요한 import들이 올바르게 추가되었습니다.
Also applies to: 6-6, 17-17, 25-25
51-75: 새로운 GET 엔드포인트 구현이 적절함사용자가 저장한 장소 목록을 조회하는 새로운 엔드포인트가 적절히 구현되었습니다. API 문서화도 잘 되어 있고, 권한 검증 로직도 올바릅니다.
src/main/java/com/wayble/server/user/controller/UserPlaceController.java
Show resolved
Hide resolved
src/main/java/com/wayble/server/user/controller/UserPlaceController.java
Show resolved
Hide resolved
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.
수고 많으셨어요!!!
| if (!jwtProvider.validateToken(token)) { | ||
| throw new ApplicationException(UserErrorCase.FORBIDDEN); | ||
| } | ||
| Long tokenUserId = jwtProvider.getUserId(token); |
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.
Long userId = (Long) SecurityContextHolder.getContext().getAuthentication().getPrincipal(); 와 같은 형식(문법이 이게 맞는지는 모르겠네요)으로 userId를 추출할 수 있는걸로 알고 있습니다!
토큰을 전달받아 userId를 추출하는 것보다 코드도 짧고 훨씬 간결할 것 같습니다!
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.
이 부분은 SecurityContextHolder 에서 userId를 꺼내는 것이 간결하고, 유지보수에도 좋을것 같아요 감사합니다! 반영해서 리팩토링 해보겠습니다.
| if (!userId.equals(request.userId())) { | ||
| throw new ApplicationException(UserErrorCase.INVALID_USER_ID); | ||
| String token = authorizationHeader.replace("Bearer ", ""); | ||
| if (!jwtProvider.validateToken(token)) { |
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.
어짜피 요청 과정에서 jwt filter 과정을 통해 validateToken() 메서드가 자동으로 실행되는 것으로 알고 있습니다! 이 부분은 중복 검증인 것 같아요!
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.
JWT가 올바른지 직접 검증하는 핵심 로직이 컨트롤러와 서비스까지 있는게 해당 부분의 로직을 보게된다면 추적성 측면에서 좋다고 생각하였습니다. 그리고 인증 필터 이외에 컨트롤러에 검증 로직을 추가하면, 테스트, API에서도 예외를 잡을 수 있다고 생각하였고 토큰 검증은 워낙 중요하다고 생각해서 계속 검증 로직을 더 신경썻던거 같아요!
| Long tokenUserId = jwtProvider.getUserId(token); | ||
|
|
||
| // Path variable과 request body의 userId 일치 여부 확인 | ||
| if (!userId.equals(request.userId()) || !userId.equals(tokenUserId)) { |
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.
JWT에서 userId를 포함하고, pathVariable에도 userId가 포함되어 있기 때문에 request body에 userId가 꼭 필요할지 의문이에요!
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.
이것도 말씀하신것처럼 PathVariable과 JWT 토큰에서 이미 userId를 체크하고 있는데 중복이라는 단점이 존재하는것 같아요. 리뷰 주신 대로 request body에선 userId를 빼고, path variable/JWT 기반으로만 검증하도록 수정하겠습니다!
| public class UserPlaceController { | ||
|
|
||
| private final UserPlaceService userPlaceService; | ||
| private final JwtTokenProvider jwtProvider; |
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.
밑에 말씀드린 부분을 적용하면 jwtProvider 주입도 자연스럽게 빠질 것 같습니다..!
| WaybleZone waybleZone = mapping.getWaybleZone(); | ||
|
|
||
| // 웨이블존 대표 이미지 가져오기 | ||
| String imageUrl = waybleZone.getWaybleZoneImageList().stream() |
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.
waybleZone 엔티티에서 대표 이미지 url 필드가 따로 있으면 좋을 것 같아요!
매번 추출하는 것도 귀찮고, 웨이블존 상세 페이지 이외에 모든 화면들에서는 대표 이미지만 쓰이니까요...!
그리고 현재 방식처럼 쓰면 N + 1 문제가 발생할 것 같습니다!
elastic search에 웨이블존 저장할 때도 대표 이미지 url 필드만 따로 지정해놓아서 개발했는데, 값 동기화할 때도 이게 편할 것 같구요!
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.
다시 고민해보니깐 이건 지적해주신게 완전하게 맞는 말인거같아요 감사합니다! 현재는 대표 이미지를 사용할 때마다 getWaybleZoneImageList().stream()에 find로 매번 찾아야하는데 이건 웨이블존 상세가 아닌 곳에서 매번 stream 연산 필요하기때문에 N+1 문제가 발생 가능할것 같아요. 쿼리 최적화를 위해 칼럼 추가해서 리팩토링 하겠습니다. 좋은 리뷰 감사합니다!!
#️⃣ 연관된 이슈
#54
📝 작업 내용
🖼️ 스크린샷 (선택)
장소 목록 조회 성공
조회 실패 (권한이 없는 사용자)
💬 리뷰 요구사항 (선택)
Summary by CodeRabbit