-
Notifications
You must be signed in to change notification settings - Fork 1
[release] 현재까지 진행상황 main으로 병합 #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
[refactor] 웨이블존 상세 조회에 위도,경도 추가 + 토큰 관련 리팩토링 + 웨이블존 목록 조회, 리뷰 작성 테스트 코드 작성
Walkthrough새로운 파라미터 주입 방식(@currentuser) 도입을 위해 커스텀 애노테이션과 ArgumentResolver, WebMvc 설정을 추가했다. ReviewController와 UserPlaceController는 보안 컨텍스트 직접 접근을 제거하고 @currentuser Long userId를 사용한다. WaybleZone 상세 응답 DTO/서비스에 위도·경도가 추가되었고, 리뷰 및 존 컨트롤러 관련 단위 테스트가 추가됐다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Dispatcher as Spring MVC
participant Resolver as CurrentUserArgumentResolver
participant Controller as *Controller
participant Security as SecurityContext
Client->>Dispatcher: HTTP Request
Dispatcher->>Resolver: Resolve @CurrentUser Long userId
Resolver->>Security: getContext().getAuthentication()
Security-->>Resolver: Authentication (principal/name)
Resolver-->>Dispatcher: userId (Long)
Dispatcher->>Controller: Invoke method(..., userId, ...)
Controller-->>Client: Response
sequenceDiagram
participant Controller as WaybleZoneService Caller
participant Service as WaybleZoneService
participant Repo as ZoneRepository
participant DTO as WaybleZoneDetailResponseDto
Controller->>Service: getDetail(zoneId)
Service->>Repo: findById(zoneId)
Repo-->>Service: Zone (address 포함)
Service->>Service: address에서 lat/lon 추출
Service->>DTO: build(..., latitude, longitude)
Service-->>Controller: DTO
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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: 1
🔭 Outside diff range comments (1)
src/main/java/com/wayble/server/wayblezone/service/WaybleZoneService.java (1)
86-94: 주소 null 시 NPE 위험: address 빌더 세팅에서 방어 필요아래에서
.address(zone.getAddress().toFullAddress())를 그대로 호출하고 있어, 주소가 null인 경우 NPE가 발생할 수 있습니다. 위도/경도처럼 주소 문자열도 null-safe로 처리해 주세요.- Double lat = zone.getAddress() != null ? zone.getAddress().getLatitude() : null; - Double lon = zone.getAddress() != null ? zone.getAddress().getLongitude() : null; + var addr = zone.getAddress(); + String address = addr != null ? addr.toFullAddress() : null; + Double lat = addr != null ? addr.getLatitude() : null; + Double lon = addr != null ? addr.getLongitude() : null; return WaybleZoneDetailResponseDto.builder() .waybleZoneId(zone.getId()) .name(zone.getZoneName()) .category(zone.getZoneType().toString()) - .address(zone.getAddress().toFullAddress()) + .address(address) .rating(zone.getRating())
🧹 Nitpick comments (9)
src/main/java/com/wayble/server/auth/resolver/CurrentUser.java (1)
1-8: 용도 명시 Javadoc 추가 제안해당 애노테이션이 Long 타입의 사용자 식별자를 주입하는 용도임을 간단한 Javadoc으로 남겨두면 이해와 검색성이 좋아집니다.
적용 예시:
package com.wayble.server.auth.resolver; import java.lang.annotation.*; +/** + * 현재 인증된 사용자의 식별자(Long)를 주입하기 위한 마커 애노테이션. + * 컨트롤러 핸들러 메서드의 파라미터에서 사용합니다. + */ @Target(ElementType.PARAMETER) @Retention(RetentionPolicy.RUNTIME) @Documented public @interface CurrentUser {}src/test/java/com/wayble/server/review/service/ReviewServiceTest.java (1)
74-106: 예외 케이스 단언 강화 및 브랜치 커버리지 보완 제안
- 현재는
ApplicationException타입만 단언하고 있어 에러 케이스 식별력이 낮습니다.assertThrows의 반환값으로errorCase혹은 메시지까지 확인하면 회귀 방지에 도움이 됩니다.- 이미지 리스트 null/빈 리스트 분기 각각에 대한 저장 호출 횟수 검증 테스트를 추가하면 브랜치 커버리지가 완성됩니다.
원하시면 위 보완 테스트 케이스를 바로 생성해드릴게요.
src/test/java/com/wayble/server/wayblezone/controller/WaybleZoneControllerTest.java (1)
64-68: 컨트롤러-서비스 위임 검증 추가 제안행위 검증을 통해 컨트롤러가 정확히 한 번 위임하는지 확인하면 테스트 의도가 더 명확해집니다.
when(waybleZoneService.getWaybleZones("서초구", "카페")) .thenReturn(List.of(item1, item2)); CommonResponse<List<WaybleZoneListResponseDto>> response = waybleZoneController.getWaybleZoneList("서초구", "카페"); + + verify(waybleZoneService, times(1)).getWaybleZones("서초구", "카페");src/main/java/com/wayble/server/review/controller/ReviewController.java (1)
53-57: sort 파라미터 값 화이트리스트 및 문서화 제안현재 service 레벨에서 switch로 기본값 처리하지만, 컨트롤러에서 허용값을 명시하면 입력 검증과 API 문서 품질이 개선됩니다.
다음과 같이 제한/문서화를 추가할 수 있습니다:
- @RequestParam(defaultValue = "latest") String sort + @RequestParam(defaultValue = "latest") + @jakarta.validation.constraints.Pattern(regexp = "latest|rating") + @io.swagger.v3.oas.annotations.Parameter( + description = "정렬 기준", + schema = @io.swagger.v3.oas.annotations.media.Schema(allowableValues = {"latest","rating"}) + ) + String sort필요 시 상단 import로 정리하려면:
// 추가 import 예시 import io.swagger.v3.oas.annotations.Parameter; import jakarta.validation.constraints.Pattern;src/main/java/com/wayble/server/auth/resolver/CurrentUserArgumentResolver.java (3)
16-19: primitive long도 지원하도록 확장하세요현재 Long 래퍼 타입만 지원합니다. 실수로 primitive long을 사용하면 주입이 실패합니다. 둘 다 지원하도록 조건을 넓히는 것을 권장합니다.
- 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()));
3-7: 예외 타입을 보안 도메인에 맞게 교체하여 올바른 HTTP 상태코드(401/403)를 유도하세요IllegalStateException은 통상 500으로 매핑되어 API 소비자에게 혼란을 줄 수 있습니다. 인증 미존재는 401, 식별자 추출 실패는 403으로 구분해 던지면 에러语义가 명확해집니다.
import org.springframework.core.MethodParameter; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.access.AccessDeniedException; +import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException; import org.springframework.stereotype.Component;- if (auth == null) { - throw new IllegalStateException("인증 정보가 없습니다."); - } + if (auth == null) { + throw new AuthenticationCredentialsNotFoundException("인증 정보가 없습니다."); + }- } catch (Exception e) { - throw new IllegalStateException("userId를 추출할 수 없습니다.", e); - } + } catch (Exception e) { + throw new AccessDeniedException("userId를 추출할 수 없습니다.", e); + }Also applies to: 26-29, 41-44
12-14: 보안 컨텍스트 직접 접근 제거 및 @currentuser 적용 제안컨트롤러 계층에서 여전히 SecurityContextHolder.getContext().getAuthentication().getPrincipal()를 직접 호출하고 있습니다. 이미 도입된 CurrentUserArgumentResolver(@currentuser)를 활용해 중복·분기 로직을 제거하는 것을 권장드립니다.
검토 및 적용 대상:
- src/main/java/com/wayble/server/wayblezone/controller/WaybleZoneController.java (61행)
- src/main/java/com/wayble/server/user/controller/UserController.java (62, 82, 100행)
- src/main/java/com/wayble/server/auth/controller/AuthController.java (107행)
- src/main/java/com/wayble/server/explore/controller/WaybleZoneRecommendController.java (28행)
다음 단계:
- 각 컨트롤러 메서드의 파라미터에
@CurrentUser Long userId를 추가- 기존 SecurityContextHolder 호출을 제거
- 서비스/필터 계층에서는 보안 컨텍스트 직접 접근이 필요한 경우 유지
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (2)
69-73: 페이징 파라미터에 Bean Validation을 추가해 입력 검증 강화page는 1 이상, size는 합리적인 상한(예: 1~100)을 두면 방어적입니다. 서비스에서 1-based를 0-based로 변환한다면 그 전제도 명시하세요.
- @RequestParam(defaultValue = "1") Integer page, - @RequestParam(defaultValue = "20") Integer size + @RequestParam(defaultValue = "1") @jakarta.validation.constraints.Min(1) Integer page, + @RequestParam(defaultValue = "20") @jakarta.validation.constraints.Min(1) @jakarta.validation.constraints.Max(100) Integer size필요 시 상단 import:
import jakarta.validation.constraints.Min; import jakarta.validation.constraints.Max;
88-94: DELETE + RequestBody는 일부 클라이언트/프록시에서 제약이 있을 수 있습니다DELETE 바디를 허용하지 않는 환경이 존재합니다. 가능하다면 아래와 같이 경로/쿼리로 옮기는 방안을 검토하세요.
예시:
- HTTP 메서드/경로: DELETE /api/v1/users/places/{placeId}/zones/{waybleZoneId}
- 컨트롤러 시그니처:
@DeleteMapping("/{placeId}/zones/{waybleZoneId}") public CommonResponse<String> removeZoneFromPlace(@CurrentUser Long userId, @PathVariable Long placeId, @PathVariable Long waybleZoneId) { ... }
📜 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 (7)
src/main/java/com/wayble/server/wayblezone/dto/WaybleZoneDetailResponseDto.java (6)
src/main/java/com/wayble/server/admin/dto/wayblezone/AdminWaybleZoneCreateDto.java (1)
AdminWaybleZoneCreateDto(9-39)src/main/java/com/wayble/server/admin/dto/wayblezone/AdminWaybleZoneDetailDto.java (1)
AdminWaybleZoneDetailDto(10-59)src/main/java/com/wayble/server/explore/dto/facility/WaybleFacilityConditionDto.java (1)
WaybleFacilityConditionDto(8-21)src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java (1)
LocationInfo(51-54)src/main/java/com/wayble/server/explore/dto/search/request/WaybleZoneSearchConditionDto.java (1)
Builder(10-29)src/main/java/com/wayble/server/direction/dto/response/WayblePathResponse.java (1)
WayblePoint(25-34)
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/service/WaybleZoneService.java (5)
src/main/java/com/wayble/server/explore/dto/recommend/WaybleZoneRecommendConditionDto.java (1)
Builder(8-20)src/main/java/com/wayble/server/explore/dto/common/WaybleZoneInfoResponseDto.java (1)
Builder(7-34)src/main/java/com/wayble/server/admin/dto/wayblezone/AdminWaybleZoneCreateDto.java (1)
AdminWaybleZoneCreateDto(9-39)src/main/java/com/wayble/server/explore/dto/search/request/WaybleZoneSearchConditionDto.java (1)
Builder(10-29)src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java (1)
LocationInfo(51-54)
src/main/java/com/wayble/server/common/config/WebMvcConfig.java (3)
src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthenticationFilter.java (1)
RequiredArgsConstructor(13-43)src/main/java/com/wayble/server/user/controller/UserController.java (1)
RestController(25-124)src/main/java/com/wayble/server/common/config/SecurityConfig.java (2)
Configuration(22-75)Bean(34-61)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (3)
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)src/main/java/com/wayble/server/user/entity/UserPlaceWaybleZoneMapping.java (1)
Entity(10-29)
src/main/java/com/wayble/server/review/controller/ReviewController.java (1)
src/main/java/com/wayble/server/review/service/ReviewService.java (1)
Service(23-81)
⏰ 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 (10)
src/main/java/com/wayble/server/auth/resolver/CurrentUser.java (1)
5-8: 마커 애노테이션 설계 적절함파라미터 타겟/런타임 보존/Documented 조합이 적절합니다. 간결한 마커 애노테이션으로 컨트롤러 시그니처가 깔끔해졌습니다.
src/main/java/com/wayble/server/wayblezone/dto/WaybleZoneDetailResponseDto.java (2)
21-23: 위도/경도 필드 추가 방향 적절Double(Nullable) 타입 채택으로 데이터 부재 시 대응이 가능해졌고, 필드명도 기존 DTO들과 일관적입니다.
22-23: 클라이언트/문서 스펙 확인 권고새 필드(latitude/longitude)가 null일 수 있으므로
- API 문서(Swagger 등)에 Nullable임을 명시
- 서버의 JSON 직렬화 정책(null 포함/제외)과 클라이언트 파싱 기대치
를 한번 점검해 주세요.src/test/java/com/wayble/server/review/service/ReviewServiceTest.java (1)
34-72: 핵심 시나리오 커버리지 우수
- 평점 가중 평균 갱신 검증(4.5), 리뷰 카운트 증가, 이미지 저장 count 검증 등 주요 부수효과를 잘 확인하고 있습니다.
src/main/java/com/wayble/server/wayblezone/service/WaybleZoneService.java (1)
86-87: lat/lon 추출 시 null 안전 처리 좋습니다주소 부재 시 null을 반환하도록 방어 코드를 둔 점 좋습니다.
src/test/java/com/wayble/server/wayblezone/controller/WaybleZoneControllerTest.java (2)
28-39: 테스트 데이터 구성 가독성 좋습니다FacilityDto와 두 개의 리스트 아이템을 명확히 구성하여 읽기 쉬운 시나리오를 제공합니다.
69-93: 필드별 단언 충실핵심 출력 필드(주소 포함 문자열, 평점, 리뷰 수, 연락처, 시설 세부 플래그 등)를 세밀하게 검증하고 있어 회귀 방지에 유용합니다.
src/main/java/com/wayble/server/review/controller/ReviewController.java (1)
39-44: @currentuser 주입으로 보안 컨텍스트 의존 제거 — 깔끔합니다메서드 시그니처와 서비스 호출이 일관되며, 인증 정보 추출 책임을 ArgumentResolver로 위임해 테스트성과 유지보수성이 좋아졌습니다.
src/main/java/com/wayble/server/common/config/WebMvcConfig.java (1)
17-20: HandlerMethodArgumentResolver 등록 적절MVC 설정에서 커스텀 Resolver를 명시적으로 추가한 방식이 표준적이며, 다른 설정과 충돌 가능성도 낮습니다. 구성의 책임이 분리되어 있어 확장성도 좋습니다.
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (1)
37-41: @currentuser 주입으로 단순화 — 서비스 계층과의 인터페이스도 명확합니다중복되는 인증 추출 코드 제거로 가독성과 테스트 용이성이 개선되었습니다.
|
|
||
| assertEquals(4.5, ratingCaptor.getValue(), 1e-6); | ||
|
|
||
| verify(zone, times(1)).addReviewCount(1L); |
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.
verify 파라미터 타입 불일치 가능성 (1L vs 1)
서비스 구현은 zone.addReviewCount(1)을 호출합니다. 엔티티 메서드가 int 혹은 Integer를 받는 경우 현재 검증(1L)은 시그니처가 달라 실패할 수 있습니다. 테스트를 구현과 동일한 타입으로 맞춰주세요.
- verify(zone, times(1)).addReviewCount(1L);
+ verify(zone, times(1)).addReviewCount(1);🤖 Prompt for AI Agents
In src/test/java/com/wayble/server/review/service/ReviewServiceTest.java around
line 69, the verify call uses 1L which may not match the entity method signature
(service calls zone.addReviewCount(1)); change the verify to pass an int (use 1)
to match the implementation and avoid type mismatch in the Mockito verification.
✔️ 연관 이슈
📝 작업 내용
스크린샷 (선택)
Summary by CodeRabbit