-
Notifications
You must be signed in to change notification settings - Fork 1
[refactor] 웨이블존 상세 조회에 위도,경도 추가 + 토큰 관련 리팩토링 + 웨이블존 목록 조회, 리뷰 작성 테스트 코드 작성 #161
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새로운 @currentuser 애너테이션과 스프링 MVC ArgumentResolver를 추가해 컨트롤러 파라미터에 사용자 ID를 주입하도록 변경. WebMvcConfig에 등록. ReviewController와 UserPlaceController 시그니처를 해당 파라미터로 갱신. WaybleZone 상세 DTO에 위도/경도 필드 추가하고 서비스에서 채움. 관련 단위 테스트 2개 추가. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DispatcherServlet
participant HandlerMapping
participant Controller as Controller(@CurrentUser Long userId)
participant Resolver as CurrentUserArgumentResolver
participant Security as SecurityContextHolder
Client->>DispatcherServlet: HTTP Request
DispatcherServlet->>HandlerMapping: 핸들러 조회
HandlerMapping-->>DispatcherServlet: Controller 메서드
DispatcherServlet->>Resolver: supportsParameter(@CurrentUser Long)
Resolver-->>DispatcherServlet: true
DispatcherServlet->>Security: getContext().getAuthentication()
Security-->>DispatcherServlet: Authentication
DispatcherServlet->>Resolver: resolveArgument(Authentication)
Resolver-->>DispatcherServlet: userId (Long)
DispatcherServlet->>Controller: 메서드 호출(userId, ...)
Controller-->>Client: Response
sequenceDiagram
participant Controller
participant Service as WaybleZoneService
participant Repo as WaybleZoneRepository
participant DTO as WaybleZoneDetailResponseDto
Controller->>Service: getWaybleZoneDetail(id)
Service->>Repo: findById(id)
Repo-->>Service: WaybleZone
Service->>Service: address에서 latitude/longitude 추출(또는 null)
Service->>DTO: 빌드(위경도 포함)
DTO-->>Controller: 상세 응답
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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. ✨ 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: 0
🧹 Nitpick comments (17)
src/main/java/com/wayble/server/auth/resolver/CurrentUser.java (1)
5-8: 마커 애너테이션 정의 적절 — Javadoc 추가로 의도 명시성↑RUNTIME/PARAMETER 설정과 마커 형태 모두 적절합니다. 간단한 Javadoc을 추가해 사용 의도를 명시하면 유지보수성이 좋아집니다.
@Target(ElementType.PARAMETER) @Retention(RetentionPolicy.RUNTIME) @Documented +/** + * 현재 인증된 사용자의 ID(Long)를 컨트롤러 메서드 파라미터에 주입하기 위한 마커 애너테이션입니다. + */ public @interface CurrentUser {}src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java (3)
15-19: primitive long 파라미터도 지원하세요현재 Long만 지원합니다. 팀 컨벤션에 따라 primitive long을 사용할 수 있도록 허용하는 편이 안전합니다.
@Override public boolean supportsParameter(MethodParameter parameter) { - return parameter.hasParameterAnnotation(CurrentUser.class) - && Long.class.equals(parameter.getParameterType()); + return parameter.hasParameterAnnotation(CurrentUser.class) + && (Long.class.equals(parameter.getParameterType()) + || long.class.equals(parameter.getParameterType())); }
31-38: 단순화 가능: JwtAuthentication을 전제로 Long만 처리프로젝트에서 JwtAuthentication.getPrincipal()이 Long을 반환하도록 표준화된 상태이므로, String/Integer 파싱 분기를 제거하고 Long 외 타입은 즉시 예외 처리하는 방식으로 단순화할 수 있습니다. 코드 복잡도가 줄고 실패 시점이 빨라집니다.
3-11: 인증 예외 처리에 표준 예외 및 익명 인증 방어 적용Spring Security의 SecurityFilterChain(.anyRequest().authenticated())과 JwtAuthenticationFilter를 통해 모든
@CurrentUser사용 경로가 인증 필수로 보호되고 있으므로, 리졸버에서IllegalStateException대신AuthenticationCredentialsNotFoundException을 던지고 익명 인증 방어를 추가하세요. 이렇게 하면 인증 누락·실패 시 401 Unauthorized로 일관되게 매핑됩니다.– 파일:
src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java
– 위치:resolveArgument메서드(auth 검사 및 예외 처리 부분)@@ -import org.springframework.core.MethodParameter; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.core.MethodParameter; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.authentication.AnonymousAuthenticationToken; +import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException; import org.springframework.stereotype.Component; import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.method.support.HandlerMethodArgumentResolver; import org.springframework.web.method.support.ModelAndViewContainer; @@ - Authentication auth = SecurityContextHolder.getContext().getAuthentication(); - if (auth == null) { - throw new IllegalStateException("인증 정보가 없습니다."); - } + Authentication auth = SecurityContextHolder.getContext().getAuthentication(); + if (auth == null || !auth.isAuthenticated() || auth instanceof AnonymousAuthenticationToken) { + throw new AuthenticationCredentialsNotFoundException("인증 정보가 없습니다."); + } @@ - } catch (Exception e) { - throw new IllegalStateException("userId를 추출할 수 없습니다.", e); - } + } catch (Exception e) { + throw new AuthenticationCredentialsNotFoundException("userId를 추출할 수 없습니다.", e); + }전역 예외처리기에서
IllegalStateException을 이미 401/403으로 매핑 중이라면 그대로 두어도 되지만, Spring Security 표준 예외 사용을 권장합니다.src/test/java/com/wayble/server/review/service/ReviewServiceTest.java (2)
85-87: 실패 시 부수효과 없는지 검증 추가 권장존재하지 않는 웨이블존일 때, 저장성 호출이 전혀 일어나지 않았는지까지 검증하면 테스트 신뢰도가 높아집니다.
assertThrows(ApplicationException.class, () -> sut.registerReview(zoneId, userId, dto)); + + // 부수효과 방지 검증 + verify(reviewRepository, never()).save(any()); + verify(reviewImageRepository, never()).save(any()); + verify(waybleZoneRepository, never()).save(any(WaybleZone.class));
103-105: 유저 없음 시에도 저장성 호출 차단 검증 추가 권장유저 미존재 시 리뷰/이미지/존 저장이 호출되지 않음을 명시적으로 검증하세요.
assertThrows(ApplicationException.class, () -> sut.registerReview(zoneId, userId, dto)); + + // 부수효과 방지 검증 + verify(reviewRepository, never()).save(any()); + verify(reviewImageRepository, never()).save(any()); + verify(waybleZoneRepository, never()).save(any(WaybleZone.class));src/main/java/com/wayble/server/review/controller/ReviewController.java (2)
39-44: @currentuser 도입으로 컨트롤러 단 단순화 성공컨트롤러에서 SecurityContext 접근 로직 제거되고, 서비스로 명확히 userId를 전달하는 흐름이 좋아졌습니다. 다만 POST는 생성 작업이므로 201 Created로 응답하거나 Location 헤더/리소스 ID를 반환하는 쪽이 REST 관례에 더 가깝습니다.
예시(참고용, 변경은 본 파일 외 라인들에 걸침):
@PostMapping @ResponseStatus(HttpStatus.CREATED) public CommonResponse<String> registerReview( @PathVariable Long waybleZoneId, @CurrentUser Long userId, @RequestBody @Valid ReviewRegisterDto dto ) { reviewService.registerReview(waybleZoneId, userId, dto); return CommonResponse.success("리뷰가 등록되었습니다."); }
53-58: sort 파라미터를 Enum으로 제한하면 안전성↑"latest"/"rating" 문자열 대신 Enum(예: ReviewSort { LATEST, RATING })으로 바꾸면 유효성/스웨거 문서화/IDE 지원이 좋아집니다. 컨트롤러에서 Enum을 받고, 서비스에서 switch로 분기하세요.
src/main/java/com/wayble/server/wayblezone/dto/WaybleZoneDetailResponseDto.java (1)
21-23: 좌표 필드(Double) 추가는 합리적입니다. 다만 null 직렬화 제외와 API 호환성 점검을 권장합니다.
- latitude/longitude를 Double로 둔 것은 주소 미보유 케이스에 대응 가능해 적절합니다.
- 응답 필드가 null일 때 불필요한 키 노출을 피하려면 타입 레벨로
@JsonInclude(Include.NON_NULL)적용을 고려해 주세요.- 레코드 컴포넌트가 추가되면서 canonical constructor 시그니처가 변경되었습니다. 외부/내부에서 직접 생성자를 호출하던 사용처가 있다면 빌더로 전환되었는지 확인 필요합니다.
아래와 같이 적용할 수 있습니다(타입/임포트 추가 — 선택):
import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonInclude.Include; @JsonInclude(Include.NON_NULL) @Builder public record WaybleZoneDetailResponseDto( /* ... */ ) { /* ... */ }src/test/java/com/wayble/server/wayblezone/controller/WaybleZoneControllerTest.java (3)
28-31: 테스트 메서드명을 의도 드러나게 변경하세요.
t1대신 시나리오가 드러나는 이름이 유지보수에 유리합니다.아래와 같이 변경 제안:
- void t1() { + void getWaybleZoneList_success() {
64-68: 서비스 상호작용 검증 추가로 테스트 신뢰도를 높이세요.호출 결과만 검증 중입니다. 서비스가 올바른 파라미터로 1회 호출되었는지도 함께 검증하면 회귀에 강해집니다.
다음 변경을 제안합니다.
- Mockito 정적 임포트 보완:
-import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*;
- 컨트롤러 호출 이후 verify 추가:
CommonResponse<List<WaybleZoneListResponseDto>> response = waybleZoneController.getWaybleZoneList("서초구", "카페"); + verify(waybleZoneService, times(1)).getWaybleZones("서초구", "카페");
44-57: 입력 파라미터(서초구)와 테스트 데이터(강남구) 정합성 맞추기 권장.목적은 목록 구조 검증이지만, 주소/지명 불일치는 독자 혼란을 유발합니다. 동일 자치구로 맞추면 가독성이 좋아집니다.
예시 수정:
- .address("서울 강남구 강남대로 446 (역삼동)") + .address("서울 서초구 서초대로 446 (서초동)")그리고 단언도 일관되게:
- assertTrue(first.address().contains("강남대로")); + assertTrue(first.address().contains("서초대로"));Also applies to: 77-77
src/main/java/com/wayble/server/wayblezone/service/WaybleZoneService.java (2)
86-95: 주소 null-safe 처리 일관성 유지 권장.lat/lon은 주소 null 여부를 체크하지만, 직후
.address(zone.getAddress().toFullAddress())는 NPE 가능성이 있습니다(도메인 제약으로 주소가 항상 존재한다면 무방). 일관성을 위해 주소도 동일하게 null-safe로 처리하는 편이 안전합니다.아래와 같은 보완을 제안합니다.
- Double lat = zone.getAddress() != null ? zone.getAddress().getLatitude() : null; - Double lon = zone.getAddress() != null ? zone.getAddress().getLongitude() : null; + Double lat = zone.getAddress() != null ? zone.getAddress().getLatitude() : null; + Double lon = zone.getAddress() != null ? zone.getAddress().getLongitude() : null; + String fullAddress = zone.getAddress() != null ? zone.getAddress().toFullAddress() : null;- .address(zone.getAddress().toFullAddress()) + .address(fullAddress)주소가 항상 존재한다는 불변식이 보장된다면(예: DB 제약) 위 보완은 선택사항입니다. 보장 여부를 한 번만 확인해 주세요.
26-33: 파라미터 명칭 ‘city’는 도메인 용어와 다릅니다.컨트롤러/테스트에서 ‘구(자치구)’ 단위를 사용 중입니다. 혼란을 줄이기 위해 파라미터를 ‘district’(또는 domain에 맞는 명칭)로 맞추는 것을 권장합니다. 내부 호출부만 변경하면 영향이 작습니다.
- public List<WaybleZoneListResponseDto> getWaybleZones(String city, String category) { + public List<WaybleZoneListResponseDto> getWaybleZones(String district, String category) { WaybleZoneType zoneType = resolveType(category); - var zones = waybleZoneRepository.findSummaryByCityAndType(city, zoneType); + var zones = waybleZoneRepository.findSummaryByCityAndType(district, zoneType);src/main/java/com/wayble/server/user/controller/UserPlaceController.java (3)
37-38: Swagger 문서에서 @currentuser 파라미터를 숨겨 주세요.리졸버로 주입되는 내부 파라미터가 공개 API 스펙에 노출되지 않도록
@Parameter(hidden = true)적용을 권장합니다.변경 예시:
- @CurrentUser Long userId, + @Parameter(hidden = true) @CurrentUser Long userId,위 4곳 모두 동일하게 적용.
추가 임포트(파일 상단):
import io.swagger.v3.oas.annotations.Parameter;Also applies to: 52-54, 69-73, 89-91
71-72: page/size에 입력 유효성 제약을 추가하세요.1-based 페이지 정책을 유지하면서, 과도한 size를 방지하는 제약이 있으면 좋습니다(예: 1 ≤ page, 1 ≤ size ≤ 100).
예시:
- @RequestParam(defaultValue = "1") Integer page, - @RequestParam(defaultValue = "20") Integer size + @Min(1) @RequestParam(defaultValue = "1") Integer page, + @Min(1) @Max(100) @RequestParam(defaultValue = "20") Integer size필요 임포트:
import jakarta.validation.constraints.Min; import jakarta.validation.constraints.Max;서비스 단에서 0-based PageRequest를 사용한다면
page - 1로 보정하는 로직이 있는지(또는 컨트롤러에서 보정해 주는지) 확인 부탁드립니다.
88-94: DELETE 응답은 204 No Content도 고려해 볼 수 있습니다.메시지 바디가 꼭 필요하지 않다면 204가 더 REST 관례에 가깝습니다. 다만 프로젝트 전반의
CommonResponse일관성이 더 중요하다면 현 방식 유지도 괜찮습니다.가능한 대안(선택):
- public CommonResponse<String> removeZoneFromPlace( /* ... */ ) { - userPlaceService.removeZoneFromPlace(userId, request.placeId(), request.waybleZoneId()); - return CommonResponse.success("제거되었습니다."); - } + public ResponseEntity<Void> removeZoneFromPlace( /* ... */ ) { + userPlaceService.removeZoneFromPlace(userId, request.placeId(), request.waybleZoneId()); + return ResponseEntity.noContent().build(); + }
📜 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 (9)
src/main/java/com/wayble/server/auth/resolver/CurrentUser.java(1 hunks)src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java(1 hunks)src/main/java/com/wayble/server/common/config/WebMvcConfig.java(1 hunks)src/main/java/com/wayble/server/review/controller/ReviewController.java(2 hunks)src/main/java/com/wayble/server/user/controller/UserPlaceController.java(5 hunks)src/main/java/com/wayble/server/wayblezone/dto/WaybleZoneDetailResponseDto.java(1 hunks)src/main/java/com/wayble/server/wayblezone/service/WaybleZoneService.java(2 hunks)src/test/java/com/wayble/server/review/service/ReviewServiceTest.java(1 hunks)src/test/java/com/wayble/server/wayblezone/controller/WaybleZoneControllerTest.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
src/main/java/com/wayble/server/auth/resolver/CurrentUser.java (2)
src/main/java/com/wayble/server/user/entity/User.java (1)
Getter(14-134)src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthentication.java (1)
JwtAuthentication(7-19)
src/main/java/com/wayble/server/review/controller/ReviewController.java (3)
src/main/java/com/wayble/server/review/service/ReviewService.java (1)
Service(23-81)src/main/java/com/wayble/server/review/entity/Review.java (2)
Entity(14-65)of(56-64)src/main/java/com/wayble/server/review/repository/ReviewRepository.java (1)
ReviewRepository(9-16)
src/main/java/com/wayble/server/wayblezone/service/WaybleZoneService.java (4)
src/main/java/com/wayble/server/explore/dto/recommend/WaybleZoneRecommendConditionDto.java (1)
Builder(8-20)src/main/java/com/wayble/server/explore/dto/search/request/WaybleZoneSearchConditionDto.java (1)
Builder(10-29)src/main/java/com/wayble/server/explore/dto/common/WaybleZoneInfoResponseDto.java (1)
Builder(7-34)src/main/java/com/wayble/server/admin/dto/wayblezone/AdminWaybleZoneDetailDto.java (1)
AdminWaybleZoneDetailDto(10-59)
src/main/java/com/wayble/server/common/config/WebMvcConfig.java (2)
src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthenticationFilter.java (1)
RequiredArgsConstructor(13-43)src/main/java/com/wayble/server/common/config/SecurityConfig.java (1)
session(39-39)
src/test/java/com/wayble/server/review/service/ReviewServiceTest.java (3)
src/main/java/com/wayble/server/review/service/ReviewService.java (2)
Service(23-81)Transactional(32-54)src/main/java/com/wayble/server/review/entity/Review.java (1)
Entity(14-65)src/main/java/com/wayble/server/review/dto/ReviewRegisterDto.java (1)
Schema(8-31)
src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java (1)
src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthentication.java (1)
Override(18-18)
src/main/java/com/wayble/server/wayblezone/dto/WaybleZoneDetailResponseDto.java (8)
src/main/java/com/wayble/server/admin/dto/wayblezone/AdminWaybleZoneDetailDto.java (1)
AdminWaybleZoneDetailDto(10-59)src/main/java/com/wayble/server/wayblezone/dto/WaybleZoneListResponseDto.java (1)
Builder(6-17)src/main/java/com/wayble/server/explore/dto/recommend/WaybleZoneRecommendConditionDto.java (1)
Builder(8-20)src/main/java/com/wayble/server/explore/dto/facility/WaybleFacilityConditionDto.java (1)
WaybleFacilityConditionDto(8-21)src/main/java/com/wayble/server/explore/dto/facility/WaybleFacilityResponseDto.java (1)
Builder(8-23)src/main/java/com/wayble/server/admin/dto/wayblezone/AdminWaybleZoneCreateDto.java (1)
AdminWaybleZoneCreateDto(9-39)src/main/java/com/wayble/server/explore/dto/search/response/WaybleZoneSearchResponseDto.java (1)
Builder(8-22)src/main/java/com/wayble/server/explore/dto/search/request/WaybleZoneSearchConditionDto.java (1)
Builder(10-29)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (2)
src/main/java/com/wayble/server/user/service/UserPlaceService.java (1)
Service(27-137)src/main/java/com/wayble/server/user/dto/UserPlaceRemoveRequestDto.java (1)
UserPlaceRemoveRequestDto(5-8)
🔇 Additional comments (4)
src/test/java/com/wayble/server/review/service/ReviewServiceTest.java (1)
62-72: 성공 시나리오 검증 범위 적절평점 갱신(4.5), 카운트 증가, 이미지 저장, 영속화 호출까지 핵심 상호작용을 잘 검증했습니다. 👍
src/main/java/com/wayble/server/common/config/WebMvcConfig.java (1)
11-20: MVC 인자 리졸버 등록 LGTM전역 WebMvcConfigurer에 Resolver를 주입·등록한 방식이 간결하고 명확합니다.
src/test/java/com/wayble/server/wayblezone/controller/WaybleZoneControllerTest.java (1)
73-93: 필드 단언이 꼼꼼합니다.리스트 크기, 주요 필드, 내부 DTO까지 잘 검증하고 있습니다. 가독성과 안정성 모두 양호합니다.
src/main/java/com/wayble/server/wayblezone/service/WaybleZoneService.java (1)
108-110: 좌표 전달 로직 LGTM.DTO 변경사항을 서비스에서 정확히 반영하고 있습니다(빌더로 null 허용).
✔️ 연관 이슈
#118
📝 작업 내용
1. 웨이블존 목록 조회 관련 테스트 코드 작성
2. 웨이블존 상세 조회 Dto 리팩토링
3. 웨이블존 상세 조회 Service 리팩토링
4. 리뷰 등록 관련 테스트 코드 작성
5. 공통 리졸버 및 Config 추가
스크린샷 (선택)
Summary by CodeRabbit