♻️ refactor/ZIP-30-kapt : 아파트 관련 기능 수정 #60
Merged
Conversation
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
- 아파트 좌표 및 최근 거래 표시
## Walkthrough
아파트 실거래가(가격) 조회 API가 기존 `EstatePriceApi`에서 `EstateApi`로 통합되었습니다. 이에 따라 가격 관련 엔드포인트가 `EstateApi`에 추가되고, 응답 DTO와 서비스 계층에 최근 거래가 정보를 포함하는 기능이 추가되었습니다. 기존 가격 전용 Swagger 및 컨트롤러 파일은 삭제되었습니다.
## Changes
| 파일/경로 요약 | 변경 내용 요약 |
|---|---|
| `.../adapter/in/web/EstateApi.java`, `.../adapter/in/web/swagger/EstateApiSpec.java` | 아파트 가격 조회 엔드포인트(`/price`, `/price/{kaptCode}`) 추가, Swagger 명세에 가격 관련 메서드 추가, 기존 위치 기반 리스트 반환 방식 소폭 리팩터링, `/compare` 엔드포인트 추가 |
| `.../adapter/in/web/EstatePriceApi.java`, `.../adapter/in/web/swagger/EstatePriceApiSpec.java` | 아파트 가격 전용 컨트롤러 및 Swagger 명세 파일 삭제 (기존 가격 API 분리 구조 제거) |
| `.../adapter/in/web/dto/response/EstateListResponse.java` | 최근 거래가를 포함하는 `price` 필드와 `from(Estate, Optional<EstatePrice>)` 팩토리 메서드 추가 |
| `.../adapter/in/web/dto/response/EstatePriceDetailResponse.java` | 최근 거래가 정보를 담는 신규 레코드 DTO 및 `from(Optional<EstatePrice>)` 팩토리 메서드 추가 |
| `.../adapter/out/EstatePricePersistenceAdapter.java`, `.../adapter/out/mongo/estate/EstatePriceMongoRepository.java`, `.../application/out/estate/EstatePricePort.java` | `kaptCode`별 최근 거래가를 조회하는 메서드(`loadRecentPriceByKaptCode`, `findFirstByKaptCodeOrderByTransactionDateDesc`) 추가 및 메서드명 오타 수정 |
| `.../application/in/estate/GetEstateUseCase.java`, `.../application/service/EstateService.java` | 위치 기반 아파트 조회 결과를 `EstateListResponse`(최근 거래가 포함)로 반환하도록 변경, 서비스 로직에서 가격 정보 통합, 비교용 아파트 조회 기능 추가, `Optional` 제거 및 예외 처리 강화 |
| `.../adapter/in/web/dto/response/EstateDetailResponse.java` | 다수 아파트 객체를 리스트로 변환하는 `from(List<Estate>)` 정적 메서드 추가 |
| `.../adapter/out/mongo/estate/EstateMongoRepository.java` | 테스트용 `deleteByKaptCode` 메서드 추가 |
| `.../test/adapter/in/web/EstateApiTest.java` | `EstateApi` 통합 테스트 추가, 가격 및 비교 API 포함 다양한 시나리오 검증 |
| `.../test/adapter/out/EstatePersistenceAdapterTest.java` | `EstatePersistenceAdapter` 통합 테스트 추가, 로드 및 존재 여부 확인 테스트 포함 |
| `.../test/domain/estate/EstateFixtures.java` | 테스트용 `Estate` 객체 생성용 고정 데이터 제공하는 Fixture 클래스 추가 |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Client
participant EstateApi
participant EstatePriceUseCase
participant EstatePricePort
participant EstatePriceMongoRepository
Client->>EstateApi: GET /api/v1/estate/price?kaptCode=...&area=...
EstateApi->>EstatePriceUseCase: getPriceByCodeAndArea(kaptCode, area)
EstatePriceUseCase->>EstatePricePort: loadEstatePricesByKaptCodeAndArea(kaptCode, area)
EstatePricePort->>EstatePriceMongoRepository: findAllByKaptCodeAndExclusiveArea(kaptCode, area)
EstatePriceMongoRepository-->>EstatePricePort: List<EstatePriceDocument>
EstatePricePort-->>EstatePriceUseCase: List<EstatePrice>
EstatePriceUseCase-->>EstateApi: List<EstatePriceListResponse>
EstateApi-->>Client: ApiResponse<List<EstatePriceListResponse>>sequenceDiagram
participant Client
participant EstateApi
participant EstateService
participant EstatePricePort
Client->>EstateApi: GET /api/v1/estate/list/location?lat=...&lng=...&radius=...
EstateApi->>EstateService: loadEstatesNearBy(lat, lng, radius)
EstateService->>EstatePricePort: loadRecentPriceByKaptCode(kaptCode) (for each estate)
EstatePricePort-->>EstateService: Optional<EstatePrice>
EstateService-->>EstateApi: List<EstateListResponse> (with price)
EstateApi-->>Client: ApiResponse<List<EstateListResponse>>
Suggested labels
Suggested reviewers
Poem
|
Test Coverage Report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (8)
src/main/java/com/zipte/platform/server/application/out/estate/EstatePricePort.java (2)
10-11: 문서화 일관성 및 매개변수 검증 고려사항새로 추가된 메서드가 적절하게 구현되었지만 몇 가지 개선사항이 있습니다:
- 주석 스타일이 다른 메서드들과 일치하지 않습니다 (triple-slash 사용 권장)
kaptCode매개변수에 대한 null 안전성 문서화가 필요합니다다음과 같이 문서화를 개선하는 것을 고려해보세요:
- /// 가장 최근의 거래 조회하기 - Optional<EstatePrice> loadRecentPriceByKaptCode(String kaptCode); + /// 코드 기반 가장 최근의 거래 조회하기 + /// @param kaptCode 아파트 코드 (null이면 안됨) + /// @return 가장 최근 거래가 정보 (없으면 Optional.empty()) + Optional<EstatePrice> loadRecentPriceByKaptCode(String kaptCode);
10-12: JavaDoc 표준을 따르는 문서화 스타일로 개선 필요메서드 구현은 적절하지만,
///주석 대신 표준 JavaDoc 형식을 사용하는 것을 권장합니다.- /// 가장 최근의 거래 조회하기 - Optional<EstatePrice> loadRecentPriceByKaptCode(String kaptCode); + /** + * 주어진 아파트 코드에 대한 가장 최근의 거래 정보를 조회합니다. + * @param kaptCode 조회할 아파트 코드 + * @return 최근 거래 정보 (존재하지 않을 경우 빈 Optional) + */ + Optional<EstatePrice> loadRecentPriceByKaptCode(String kaptCode);src/main/java/com/zipte/platform/server/adapter/out/EstatePricePersistenceAdapter.java (1)
19-23: 매개변수 검증 및 예외 처리 고려메서드 구현이 깔끔하고 함수형 프로그래밍 패턴을 잘 따르고 있습니다. 하지만 안정성을 위해 다음 사항을 고려해보세요:
kaptCode매개변수에 대한 null 검증- 잠재적인 데이터베이스 예외 처리
더 견고한 구현을 위해 다음과 같은 개선을 고려해보세요:
@Override public Optional<EstatePrice> loadRecentPriceByKaptCode(String kaptCode) { + if (kaptCode == null || kaptCode.trim().isEmpty()) { + return Optional.empty(); + } return repository.findFirstByKaptCodeOrderByTransactionDateDesc(kaptCode) .map(EstatePriceDocument::toDomain); }src/main/java/com/zipte/platform/server/application/in/estate/GetEstateUseCase.java (2)
18-18: 아키텍처 관점에서의 반환 타입 변경 검토반환 타입을
List<Estate>에서List<EstateListResponse>로 변경한 것은 기능적으로는 적절하지만, 클린 아키텍처 관점에서 고려사항이 있습니다:우려사항:
- 유즈케이스 인터페이스가 어댑터 레이어의 DTO에 의존하게 됨
- 애플리케이션 레이어가 웹 레이어에 결합되는 구조
장점:
- 부동산 정보와 가격 정보를 결합한 풍부한 데이터 제공
- API 응답 효율성 향상
다음 대안들을 고려해보세요:
- 도메인 레벨에서 Estate와 EstatePrice를 결합하는 새로운 도메인 객체 생성
- 별도의 응용 서비스 DTO 생성 (웹 DTO와 분리)
- 현재 구조 유지하되 의존성 방향에 대한 팀 차원의 합의
현재 변경이 좌표 기반 조회 개선이라는 PR 목적에 부합한다면, 이는 실용적인 결정일 수 있습니다.
18-21: UseCase 인터페이스에서 DTO 반환에 대한 아키텍처 검토UseCase 인터페이스가 웹 계층의 DTO(
EstateListResponse)를 직접 반환하도록 변경되었습니다. 클린 아키텍처 관점에서 UseCase는 도메인 객체를 반환하고, 웹 어댑터에서 DTO 변환을 처리하는 것이 일반적입니다.다음과 같은 아키텍처 개선을 고려해보세요:
- 별도의 UseCase 응답 DTO 생성: 웹 계층과 독립적인 UseCase 전용 응답 객체 사용
- 도메인 객체 유지: UseCase는 도메인 객체를 반환하고, 웹 어댑터에서 변환 처리
- Mapper 계층 도입: 도메인과 DTO 간 변환을 전담하는 매퍼 클래스 사용
현재 방식도 실용적이긴 하지만, 계층 간 의존성 관리 측면에서 검토가 필요합니다.
src/main/java/com/zipte/platform/server/adapter/in/web/dto/response/EstatePriceDetailResponse.java (2)
8-10: Record와 Builder 패턴 조합Record 클래스에
@Builder애노테이션을 사용하는 것은 일반적이지 않습니다. Record는 이미 불변 객체이고 생성자를 제공하므로, 빌더 패턴이 필요한지 재검토해보세요.더 간단한 Record 생성자 사용을 고려해보세요:
-@Builder -public record EstatePriceDetailResponse( - double exclusiveArea, String price, String transactionDate) { +public record EstatePriceDetailResponse( + double exclusiveArea, String price, String transactionDate) {그리고 팩토리 메서드에서:
- return EstatePriceDetailResponse.builder() - .exclusiveArea(0.0) - .price("없음") - .transactionDate("없음") - .build(); + return new EstatePriceDetailResponse(0.0, "없음", "없음");
12-19: Optional 빈 값 처리Optional이 비어있을 때의 기본값 설정이 적절합니다. "없음"이라는 한국어 문자열을 사용하고 있는데, 국제화를 고려한다면 상수나 메시지 코드 사용을 검토해보세요.
src/main/java/com/zipte/platform/server/adapter/in/web/swagger/EstateApiSpec.java (1)
94-103: 면적별 가격 조회 API 스펙 추가아파트 코드와 면적으로 가격 정보를 조회하는 API 스펙이 추가되었습니다.
@RequestParam을 사용하고 있는데, 일관성을 위해 첫 번째 API처럼@PathVariable을 고려해볼 수 있습니다.일관성을 위해 첫 번째 파라미터를 PathVariable로 변경하는 것을 고려해보세요:
ApiResponse<List<EstatePriceListResponse>> getPriceByCodeAndArea( - @Parameter(description = "아파트 코드", required = true, example = "A46392821") - @RequestParam String kaptCode, + @Parameter(description = "아파트 코드", required = true, example = "A46392821") + @PathVariable String kaptCode, @Parameter(description = "면적", required = true, example = "83.73") @RequestParam double area);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/com/zipte/platform/server/adapter/in/web/EstateApi.java(4 hunks)src/main/java/com/zipte/platform/server/adapter/in/web/EstatePriceApi.java(0 hunks)src/main/java/com/zipte/platform/server/adapter/in/web/dto/response/EstateListResponse.java(3 hunks)src/main/java/com/zipte/platform/server/adapter/in/web/dto/response/EstatePriceDetailResponse.java(1 hunks)src/main/java/com/zipte/platform/server/adapter/in/web/swagger/EstateApiSpec.java(4 hunks)src/main/java/com/zipte/platform/server/adapter/in/web/swagger/EstatePriceApiSpec.java(0 hunks)src/main/java/com/zipte/platform/server/adapter/out/EstatePricePersistenceAdapter.java(1 hunks)src/main/java/com/zipte/platform/server/adapter/out/mongo/estate/EstatePriceMongoRepository.java(1 hunks)src/main/java/com/zipte/platform/server/application/in/estate/GetEstateUseCase.java(1 hunks)src/main/java/com/zipte/platform/server/application/out/estate/EstatePricePort.java(1 hunks)src/main/java/com/zipte/platform/server/application/service/EstateService.java(3 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/com/zipte/platform/server/adapter/in/web/swagger/EstatePriceApiSpec.java
- src/main/java/com/zipte/platform/server/adapter/in/web/EstatePriceApi.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/com/zipte/platform/server/adapter/in/web/dto/response/EstateListResponse.java (1)
src/main/java/com/zipte/platform/server/adapter/in/web/dto/response/EstateLocationResponse.java (1)
Data(7-22)
🔇 Additional comments (18)
src/main/java/com/zipte/platform/server/adapter/out/mongo/estate/EstatePriceMongoRepository.java (2)
14-14: 새로운 리포지토리 메서드 승인새로 추가된
findFirstByKaptCodeOrderByTransactionDateDesc메서드가 올바르게 구현되었습니다. Spring Data 명명 규칙을 잘 따르고 있으며, 최신 거래 데이터 조회 목적에 적합합니다.
14-15: Spring Data JPA 네이밍 컨벤션을 잘 따른 메서드 구현
findFirstByKaptCodeOrderByTransactionDateDesc메서드명이 Spring Data JPA 규칙을 잘 따르고 있으며, 최신 거래 데이터를 조회하는 의도가 명확하게 표현되어 있습니다.src/main/java/com/zipte/platform/server/application/service/EstateService.java (3)
5-5: 새로운 의존성 추가 확인됨가격 정보 통합을 위한 새로운 도메인 클래스와 포트 인터페이스가 추가되었습니다. 변경사항이 적절합니다.
Also applies to: 9-9
11-11: 로깅 애노테이션 추가됨
@Slf4j애노테이션이 추가되었으나 실제 로깅 코드는 보이지 않습니다. 필요에 따라 로깅을 활용하시기 바랍니다.Also applies to: 20-20
29-30: 의존성 주입 방식 확인가격 포트에 대한 주석과 필드 선언이 추가되었습니다.
@RequiredArgsConstructor를 사용하므로 자동으로 생성자 주입이 됩니다.src/main/java/com/zipte/platform/server/adapter/in/web/dto/response/EstateListResponse.java (5)
5-5: 필요한 import 추가됨가격 정보 통합을 위한 새로운 import들이 적절히 추가되었습니다.
Also applies to: 10-10
14-14: JSON 직렬화 최적화
@JsonInclude(JsonInclude.Include.NON_NULL)애노테이션 추가로 NULL 값인 필드가 JSON 응답에서 제외되어 응답 크기를 최적화할 수 있습니다.
25-26: 가격 정보 필드 추가최근 거래 정보를 담는
price필드가 적절히 추가되었습니다.EstatePriceDetailResponse타입을 사용하여 타입 안전성을 확보했습니다.
51-60: 새로운 팩토리 메서드 구현Estate와 가격 정보를 함께 처리하는 새로운 팩토리 메서드가 잘 구현되었습니다.
Optional<EstatePrice>를 안전하게 처리하고 있습니다.
63-65:Details
❓ Verification inconclusive
Stream API 사용 개선
toList()메서드 사용으로 더 간결해졌습니다. Java 16+ 기능을 사용하고 있으니 프로젝트의 Java 버전이 호환되는지 확인해주세요.프로젝트의 Java 버전을 확인해주세요:
🏁 Script executed:
#!/bin/bash # 설명: 프로젝트 Java 버전 및 toList() 메서드 호환성 확인 # Maven pom.xml에서 Java 버전 확인 fd pom.xml --exec cat {} | grep -A 5 -B 5 "java.version\|maven.compiler" # Gradle build.gradle에서 Java 버전 확인 fd build.gradle --exec cat {} | grep -A 3 -B 3 "sourceCompatibility\|targetCompatibility\|JavaVersion"Length of output: 183
Stream API
toList()메서드 호환성 확인 필요
src/main/java/com/zipte/platform/server/adapter/in/web/dto/response/EstateListResponse.java
라인 63–65의return estates.stream() .map(EstateListResponse::from) .toList();에서 사용한
Stream.toList()는 Java 16 이상에서만 지원됩니다. 자동 검사로는pom.xml또는build.gradle에서 Java 버전 정보를 찾지 못했습니다.
아래를 확인해 주세요:
- 프로젝트가 Java 16 이상으로 빌드 및 실행되는지 검토
src/main/java/com/zipte/platform/server/adapter/in/web/swagger/EstateApiSpec.java (3)
8-8: 새로운 응답 DTO import 추가가격 정보 API를 위한 응답 DTO import가 적절히 추가되었습니다.
84-90: 새로운 가격 조회 API 스펙 추가아파트 코드로 가격 정보를 조회하는 API 스펙이 적절히 추가되었습니다. API 문서화도 잘 되어 있습니다.
49-49:Details
✅ Verification successful
좌표 예시값 업데이트
경도와 위도 예시값이 더 정확한 값으로 업데이트되었습니다. 이 좌표들이 실제 유효한 한국 지역을 가리키는지 확인해보세요.
좌표값이 유효한 한국 지역인지 확인해주세요:
Also applies to: 52-52, 64-64, 67-67
🌐 Web query:
위도 37.382698, 경도 127.118904 좌표가 가리키는 위치는 어디인가요?💡 Result:
위도 37.382698, 경도 127.118904 좌표는 경기도 성남시 분당구 정자동에 위치한 분당서울대학교병원을 가리킵니다.
이 위치는 분당서울대학교병원의 메인 건물 주변으로, 경기도 성남시 분당구 구미로173번길 82에 해당합니다. 분당서울대학교병원은 2003년 개원한 대형 종합병원으로, 서울대학교병원의 분당 분원이며 경기 남부 지역의 주요 의료기관으로 자리잡고 있습니다.
주변에는 정자역, 분당구청, 분당중앙공원 등이 위치해 있으며, 분당 신도시의 중심 지역에 자리하고 있습니다.
좌표 예시값 검증 완료
위도 37.382698, 경도 127.118904 좌표가 경기도 성남시 분당구 정자동 분당서울대학교병원을 정확히 가리키는 것으로 확인되었습니다. 추가 수정이 필요 없습니다.검토 위치:
- src/main/java/com/zipte/platform/server/adapter/in/web/swagger/EstateApiSpec.java: 49, 52, 64, 67
src/main/java/com/zipte/platform/server/adapter/in/web/EstateApi.java (3)
8-8: 새로운 임포트문들이 적절히 추가되었습니다.가격 관련 기능 통합을 위해 필요한 임포트문들이 정확히 추가되었습니다.
Also applies to: 10-10, 14-14
84-84: 코드 단순화가 적절합니다.서비스 호출 결과를 직접 반환하도록 리팩토링된 것이 좋습니다. 코드가 더 간결해졌습니다.
31-32:Details
✅ Verification successful
가격 서비스 의존성 주입을 확인하세요.
새로운
EstatePriceUseCase의존성이 추가되었습니다. 주석이 한국어로 작성되어 있어 일관성이 좋습니다.다음 스크립트를 실행하여
EstatePriceUseCase구현체가 올바르게 정의되어 있는지 확인하세요:
🏁 Script executed:
#!/bin/bash # Description: EstatePriceUseCase 인터페이스와 구현체가 존재하는지 확인 # Test: EstatePriceUseCase 인터페이스 찾기 rg -A 5 "interface EstatePriceUseCase" # Test: EstatePriceUseCase 구현체 찾기 rg -A 5 "implements EstatePriceUseCase"Length of output: 1365
EstatePriceUseCase DI 검증 완료 및 승인
EstatePriceUseCase인터페이스와 구현체가 정상적으로 정의되어 있으며, 의존성 주입에도 문제가 없습니다. 코드 변경을 승인합니다.
- 인터페이스:
src/main/java/com/zipte/platform/server/application/in/estate/EstatePriceUseCase.java- 구현체:
src/main/java/com/zipte/platform/server/application/service/EstatePriceService.java- 주석(
/// 가격 의존성)이 한국어로 잘 작성되어 일관성이 유지됩니다.src/main/java/com/zipte/platform/server/adapter/out/EstatePricePersistenceAdapter.java (1)
19-24:Details
✅ Verification successful
구현이 적절하며 코드 변경사항을 승인합니다
메서드 구현이 깔끔하고 Optional을 활용한 null 안전성도 잘 보장되어 있습니다. repository 메서드를 통한 도메인 매핑도 적절합니다.
다음 스크립트를 실행하여 repository 메서드가 올바르게 구현되었는지 확인해주세요:
🏁 Script executed:
#!/bin/bash # EstatePriceDocument에 transactionDate 필드가 존재하는지 확인 ast-grep --pattern 'class EstatePriceDocument { $$$ transactionDate $$$ }'Length of output: 82
다음 추가 스크립트를 실행해
EstatePriceDocument클래스 정의 위치와transactionDate필드 존재 여부를 재검증해주세요.해당 결과를 확인한 후,
loadRecentPriceByKaptCode메서드가 올바르게 작동하려면EstatePriceDocument에transactionDate필드가 필요하므로 존재 여부를 알려주세요.
🏁 Script executed:
#!/bin/bash # 1. EstatePriceDocument.java 파일 위치 확인 echo "=== EstatePriceDocument.java 파일 위치 ===" fd --extension java EstatePriceDocument.java # 2. 클래스 정의 검색 echo "=== class 정의 검색 ===" rg -n "class EstatePriceDocument" -t java # 3. transactionDate 필드 선언 검색 echo "=== transactionDate 필드 검색 ===" rg -n "transactionDate" -t java | grep EstatePriceDocument || echo "transactionDate 필드 선언을 찾을 수 없음"Length of output: 1004
구현 승인 및 검증 완료
EstatePriceDocument클래스에transactionDate필드가 정상적으로 존재함을 확인했습니다.
loadRecentPriceByKaptCode메서드 구현이 깔끔하며 Optional을 통한 null 안전성도 보장됩니다. 변경사항을 승인합니다.src/main/java/com/zipte/platform/server/application/in/estate/GetEstateUseCase.java (1)
18-18:Details
❌ Incorrect review comment
인터페이스 변경으로 인한 호환성 확인 필요
반환 타입이
List<Estate>에서List<EstateListResponse>로 변경되었습니다. 이는 중대한 변경사항이므로 모든 구현체가 업데이트되었는지 확인이 필요합니다.다음 스크립트를 실행하여 모든 구현체가 올바르게 업데이트되었는지 확인해주세요:
🏁 Script executed:
#!/bin/bash # GetEstateUseCase의 모든 구현체를 찾아 loadEstatesNearBy 메서드 시그니처 확인 ast-grep --pattern 'class $_ implements GetEstateUseCase { $$$ loadEstatesNearBy($$$) { $$$ } $$$ }' # EstateService에서 해당 메서드 구현 확인 ast-grep --pattern 'public List<EstateListResponse> loadEstatesNearBy($$$) { $$$ }'Length of output: 202
다음 스크립트를 실행하여 모든 구현체와 메서드 시그니처를 확인해주세요:
🏁 Script executed:
#!/bin/bash set -e echo "🔍 GetEstateUseCase 인터페이스를 구현하는 클래스 검색" rg "implements GetEstateUseCase" -n src echo "🔍 loadEstatesNearBy 메서드 정의(리턴 타입 포함) 검색" rg "List<.*> loadEstatesNearBy" -n srcLength of output: 1547
호환성 이슈 없음: GetEstateUseCase 구현체는 오직 EstateService뿐입니다
EstateService(src/main/java/com/zipte/platform/server/application/service/EstateService.java)만GetEstateUseCase를 구현하며,List<EstateListResponse>반환으로 올바르게 업데이트되어 있습니다.LoadEstatePort및EstatePersistenceAdapter는 출력 포트/어댑터로서 도메인 객체(List<Estate>)를 반환하는 의도된 설계를 따르고 있어 변경이 불필요합니다.따라서 인터페이스 변경으로 인한 추가 수정은 필요하지 않습니다. 원래 코멘트를 무시해 주세요.
Likely an incorrect or invalid review comment.
|
|
||
| List<EstatePriceDocument> findAllByKaptCode(String kaptCode); | ||
|
|
||
| List<EstatePriceDocument> findALlByKaptCodeAndExclusiveArea(String kaptCode, double exclusiveArea); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
기존 메서드명 오타 수정 필요
기존 메서드명에 오타가 있습니다: findALlByKaptCodeAndExclusiveArea는 findAllByKaptCodeAndExclusiveArea가 되어야 합니다.
다음과 같이 수정하세요:
- List<EstatePriceDocument> findALlByKaptCodeAndExclusiveArea(String kaptCode, double exclusiveArea);
+ List<EstatePriceDocument> findAllByKaptCodeAndExclusiveArea(String kaptCode, double exclusiveArea);이 변경사항은 해당 메서드를 사용하는 다른 코드에도 영향을 줄 수 있으므로 전체 코드베이스에서 확인이 필요합니다.
다음 스크립트로 영향받는 코드를 확인하세요:
🏁 Script executed:
#!/bin/bash
# 오타가 있는 메서드 사용 위치 찾기
rg -A 3 "findALlByKaptCodeAndExclusiveArea"Length of output: 1165
메서드명 오타 수정 및 참조 변경 필요
findALlByKaptCodeAndExclusiveArea 오타로 인해 Spring Data 쿼리 메소드가 제대로 파싱되지 않을 수 있습니다. 선언부와 사용부를 모두 아래와 같이 수정하세요.
• src/main/java/com/zipte/platform/server/adapter/out/mongo/estate/EstatePriceMongoRepository.java
• src/main/java/com/zipte/platform/server/adapter/out/EstatePricePersistenceAdapter.java
--- src/main/java/com/zipte/platform/server/adapter/out/mongo/estate/EstatePriceMongoRepository.java
- List<EstatePriceDocument> findALlByKaptCodeAndExclusiveArea(String kaptCode, double exclusiveArea);
+ List<EstatePriceDocument> findAllByKaptCodeAndExclusiveArea(String kaptCode, double exclusiveArea);
--- src/main/java/com/zipte/platform/server/adapter/out/EstatePricePersistenceAdapter.java
- return repository.findALlByKaptCodeAndExclusiveArea(kaptCode, exclusiveArea).stream()
+ return repository.findAllByKaptCodeAndExclusiveArea(kaptCode, exclusiveArea).stream()이후 전체 코드베이스에서 동일 오타가 남아있는지 재확인해주세요.
📝 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
| List<EstatePriceDocument> findALlByKaptCodeAndExclusiveArea(String kaptCode, double exclusiveArea); | |
| List<EstatePriceDocument> findAllByKaptCodeAndExclusiveArea(String kaptCode, double exclusiveArea); |
🤖 Prompt for AI Agents
In
src/main/java/com/zipte/platform/server/adapter/out/mongo/estate/EstatePriceMongoRepository.java
at line 12, correct the method name from findALlByKaptCodeAndExclusiveArea to
findAllByKaptCodeAndExclusiveArea to fix the typo. Also, update all references
to this method, including in
src/main/java/com/zipte/platform/server/adapter/out/EstatePricePersistenceAdapter.java
and any other usage across the codebase. After renaming, verify no remaining
occurrences of the typo exist by searching the entire project.
Comment on lines
+34
to
+50
| public List<EstateListResponse> loadEstatesNearBy(double latitude, double longitude, double radiusInKm) { | ||
| double radiusInRadians = radiusInKm / 6378.1; // 반경 km -> radians 변환 | ||
| return loadPort.loadEstatesNearBy(latitude, longitude, radiusInRadians); | ||
|
|
||
| /// 좌표 근처의 아파트 가져오기 | ||
| List<Estate> estates = loadPort.loadEstatesNearBy(latitude, longitude, radiusInRadians); | ||
|
|
||
| List<EstateListResponse> responses = new ArrayList<>(); | ||
|
|
||
| /// 최신의 거래 목록 가져오기 | ||
| estates.forEach(estate -> { | ||
| Optional<EstatePrice> price = pricePort.loadRecentPriceByKaptCode(estate.getKaptCode()); | ||
| EstateListResponse response = EstateListResponse.from(estate, price); | ||
| responses.add(response); | ||
| }); | ||
|
|
||
| return responses; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
메서드 시그니처 변경 및 로직 통합
메서드 반환 타입이 List<Estate>에서 List<EstateListResponse>로 변경되어 가격 정보가 통합되었습니다. 다음 사항들을 확인해주세요:
- 이 시그니처 변경이 다른 컨트롤러나 서비스에 영향을 미치지 않는지 확인
pricePort.loadRecentPriceByKaptCode()호출이 실패할 경우의 예외 처리 고려
다음 스크립트를 실행하여 메서드 사용처를 확인해주세요:
🏁 Script executed:
#!/bin/bash
# 설명: loadEstatesNearBy 메서드의 사용처를 찾아 시그니처 변경 영향도 확인
# 메서드 호출처 검색
rg -A 3 -B 3 "loadEstatesNearBy"
# GetEstateUseCase 인터페이스 구현체들 확인
ast-grep --pattern 'class $_ implements GetEstateUseCase'Length of output: 8809
시그니처 변경 영향도 확인 완료 및 예외 처리 권고
- GetEstateUseCase.loadEstatesNearBy 호출부는
- 컨트롤러:
EstateApi.getService.loadEstatesNearBy(...) - 서비스 구현:
EstateService.loadEstatesNearBy loadEstatesNearByProperty
위 3곳에만 존재하며, 모두 반환 타입 변경에 맞춰 업데이트되어 있어 추가 수정은 필요치 않습니다.
- 컨트롤러:
가격 정보 조회 예외 처리 검토
EstateService내pricePort.loadRecentPriceByKaptCode()호출 시Optional.empty()반환을EstateListResponse.from()에서 안전하게 처리하는지 확인- DB 연결 실패 등 런타임 예외 발생 시 전체 응답이 중단되지 않도록
try-catch또는 기본값(fallback) 로직 도입 검토
점검 위치:
- src/main/java/com/zipte/platform/server/application/service/EstateService.java
- src/main/java/com/zipte/platform/server/application/in/estate/EstateListResponse.java
🤖 Prompt for AI Agents
In
src/main/java/com/zipte/platform/server/application/service/EstateService.java
around lines 34 to 50, the method loadEstatesNearBy now returns
List<EstateListResponse> including price info, but the call to
pricePort.loadRecentPriceByKaptCode() may throw runtime exceptions that could
disrupt the entire response. To fix this, wrap the
pricePort.loadRecentPriceByKaptCode() call inside a try-catch block to handle
exceptions gracefully, returning Optional.empty() or a default value on failure,
ensuring the method continues processing all estates without interruption.
Comment on lines
+21
to
+27
| EstatePrice price = estatePrice.get(); | ||
| return EstatePriceDetailResponse.builder() | ||
| .exclusiveArea(price.getExclusiveArea() != 0.0 ? price.getExclusiveArea() : 0.0) | ||
| .price(price.getPrice() != null ? price.getPrice() : "없음") | ||
| .transactionDate(price.getTransactionDate() != null ? price.getTransactionDate() : "없음") | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
중복된 null 체크 로직
line 23-25에서 각 필드마다 null 체크를 하고 있는데, 이미 Optional이 존재하는지 확인했으므로 일부 중복된 체크가 있을 수 있습니다. 또한 price.getExclusiveArea() != 0.0 ? price.getExclusiveArea() : 0.0 로직은 불필요합니다.
다음과 같이 간소화할 수 있습니다:
EstatePrice price = estatePrice.get();
return EstatePriceDetailResponse.builder()
- .exclusiveArea(price.getExclusiveArea() != 0.0 ? price.getExclusiveArea() : 0.0)
+ .exclusiveArea(price.getExclusiveArea())
.price(price.getPrice() != null ? price.getPrice() : "없음")
.transactionDate(price.getTransactionDate() != null ? price.getTransactionDate() : "없음")
.build();🤖 Prompt for AI Agents
In
src/main/java/com/zipte/platform/server/adapter/in/web/dto/response/EstatePriceDetailResponse.java
lines 21 to 27, the code redundantly checks for null values on fields of the
EstatePrice object even though the Optional presence has already been confirmed.
Also, the check for exclusiveArea being not equal to 0.0 is unnecessary since it
returns the same value either way. Simplify the code by removing these redundant
null checks and directly returning the field values or default values where
appropriate without extra conditional checks.
Comment on lines
+112
to
+120
| @GetMapping("/price") | ||
| public ApiResponse<List<EstatePriceListResponse>> getPriceByCodeAndArea( | ||
| @RequestParam String kaptCode, | ||
| @RequestParam double area) { | ||
|
|
||
| List<EstatePrice> list = priceService.getEstatePriceByCode(kaptCode, area); | ||
|
|
||
| return ApiResponse.ok(EstatePriceListResponse.from(list)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
파라미터 검증 로직을 추가하는 것을 고려하세요.
새로운 가격 조회 엔드포인트가 추가되었습니다. 그러나 입력 파라미터에 대한 검증이 부족합니다.
다음과 같은 개선사항을 적용하는 것을 권장합니다:
@GetMapping("/price")
public ApiResponse<List<EstatePriceListResponse>> getPriceByCodeAndArea(
@RequestParam String kaptCode,
@RequestParam double area) {
+ if (kaptCode == null || kaptCode.trim().isEmpty()) {
+ throw new IllegalArgumentException("kaptCode는 필수 파라미터입니다.");
+ }
+ if (area <= 0) {
+ throw new IllegalArgumentException("area는 0보다 큰 값이어야 합니다.");
+ }
List<EstatePrice> list = priceService.getEstatePriceByCode(kaptCode, area);
return ApiResponse.ok(EstatePriceListResponse.from(list));
}📝 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
| @GetMapping("/price") | |
| public ApiResponse<List<EstatePriceListResponse>> getPriceByCodeAndArea( | |
| @RequestParam String kaptCode, | |
| @RequestParam double area) { | |
| List<EstatePrice> list = priceService.getEstatePriceByCode(kaptCode, area); | |
| return ApiResponse.ok(EstatePriceListResponse.from(list)); | |
| } | |
| @GetMapping("/price") | |
| public ApiResponse<List<EstatePriceListResponse>> getPriceByCodeAndArea( | |
| @RequestParam String kaptCode, | |
| @RequestParam double area) { | |
| if (kaptCode == null || kaptCode.trim().isEmpty()) { | |
| throw new IllegalArgumentException("kaptCode는 필수 파라미터입니다."); | |
| } | |
| if (area <= 0) { | |
| throw new IllegalArgumentException("area는 0보다 큰 값이어야 합니다."); | |
| } | |
| List<EstatePrice> list = priceService.getEstatePriceByCode(kaptCode, area); | |
| return ApiResponse.ok(EstatePriceListResponse.from(list)); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/zipte/platform/server/adapter/in/web/EstateApi.java around
lines 112 to 120, the getPriceByCodeAndArea method lacks validation for its
input parameters kaptCode and area. Add validation logic to check that kaptCode
is not null or empty and that area is a positive number. If validation fails,
return an appropriate error response or throw a validation exception to prevent
processing invalid inputs.
Comment on lines
+123
to
+129
| @GetMapping("/price/{kaptCode}") | ||
| public ApiResponse<List<EstatePriceListResponse>> getPrice(@PathVariable String kaptCode) { | ||
|
|
||
| List<EstatePrice> list = priceService.getEstatePriceByCode(kaptCode); | ||
|
|
||
| return ApiResponse.ok(EstatePriceListResponse.from(list)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
코드 중복을 제거하고 예외 처리를 개선하세요.
두 번째 가격 조회 엔드포인트에서도 파라미터 검증과 예외 처리가 필요합니다. 또한 두 메서드 간에 유사한 로직이 반복됩니다.
다음과 같이 개선할 수 있습니다:
@GetMapping("/price/{kaptCode}")
public ApiResponse<List<EstatePriceListResponse>> getPrice(@PathVariable String kaptCode) {
+ if (kaptCode == null || kaptCode.trim().isEmpty()) {
+ throw new IllegalArgumentException("kaptCode는 필수 파라미터입니다.");
+ }
- List<EstatePrice> list = priceService.getEstatePriceByCode(kaptCode);
-
- return ApiResponse.ok(EstatePriceListResponse.from(list));
+ try {
+ List<EstatePrice> list = priceService.getEstatePriceByCode(kaptCode);
+ return ApiResponse.ok(EstatePriceListResponse.from(list));
+ } catch (Exception e) {
+ throw new EntityNotFoundException("해당 아파트의 가격 정보를 찾을 수 없습니다.");
+ }
}또한 두 메서드를 하나로 통합하여 코드 중복을 줄일 수 있습니다:
@GetMapping("/price")
public ApiResponse<List<EstatePriceListResponse>> getPriceByCodeAndArea(
@RequestParam String kaptCode,
@RequestParam(required = false) Double area) {
if (kaptCode == null || kaptCode.trim().isEmpty()) {
throw new IllegalArgumentException("kaptCode는 필수 파라미터입니다.");
}
List<EstatePrice> list;
if (area != null) {
if (area <= 0) {
throw new IllegalArgumentException("area는 0보다 큰 값이어야 합니다.");
}
list = priceService.getEstatePriceByCode(kaptCode, area);
} else {
list = priceService.getEstatePriceByCode(kaptCode);
}
return ApiResponse.ok(EstatePriceListResponse.from(list));
}📝 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
| @GetMapping("/price/{kaptCode}") | |
| public ApiResponse<List<EstatePriceListResponse>> getPrice(@PathVariable String kaptCode) { | |
| List<EstatePrice> list = priceService.getEstatePriceByCode(kaptCode); | |
| return ApiResponse.ok(EstatePriceListResponse.from(list)); | |
| } | |
| @GetMapping("/price/{kaptCode}") | |
| public ApiResponse<List<EstatePriceListResponse>> getPrice(@PathVariable String kaptCode) { | |
| if (kaptCode == null || kaptCode.trim().isEmpty()) { | |
| throw new IllegalArgumentException("kaptCode는 필수 파라미터입니다."); | |
| } | |
| try { | |
| List<EstatePrice> list = priceService.getEstatePriceByCode(kaptCode); | |
| return ApiResponse.ok(EstatePriceListResponse.from(list)); | |
| } catch (Exception e) { | |
| throw new EntityNotFoundException("해당 아파트의 가격 정보를 찾을 수 없습니다."); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/zipte/platform/server/adapter/in/web/EstateApi.java around
lines 123 to 129, the getPrice method lacks parameter validation and exception
handling, and duplicates logic present in another price retrieval method.
Refactor by adding validation for kaptCode and proper exception handling to
manage invalid inputs or service errors. Then, consolidate this method with the
other similar price retrieval method into a single unified method to eliminate
code duplication and centralize the logic.
doup2001
changed the title
Test Coverage Report
|
doup2001
added
📬 API
Test Coverage Report
|
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (3)
src/main/java/com/zipte/platform/server/adapter/in/web/EstateApi.java (2)
119-127:⚠️ Potential issue파라미터 검증 로직을 추가하세요 (중복 리뷰)
이전 리뷰에서 지적된 파라미터 검증이 여전히 누락되어 있습니다.
130-136:⚠️ Potential issue파라미터 검증 및 예외 처리 개선 필요 (중복 리뷰)
이전 리뷰에서 지적된 파라미터 검증과 예외 처리가 여전히 누락되어 있습니다.
src/main/java/com/zipte/platform/server/application/service/EstateService.java (1)
36-52: 🛠️ Refactor suggestion가격 조회 시 예외 처리 보완 필요
메서드 반환 타입 변경과 가격 정보 통합이 적절히 구현되었으나, 가격 조회 시 예외 처리가 필요합니다.
/// 최신의 거래 목록 가져오기 estates.forEach(estate -> { - Optional<EstatePrice> price = pricePort.loadRecentPriceByKaptCode(estate.getKaptCode()); - EstateListResponse response = EstateListResponse.from(estate, price); - responses.add(response); + try { + Optional<EstatePrice> price = pricePort.loadRecentPriceByKaptCode(estate.getKaptCode()); + EstateListResponse response = EstateListResponse.from(estate, price); + responses.add(response); + } catch (Exception e) { + log.warn("가격 정보 조회 실패: kaptCode={}, error={}", estate.getKaptCode(), e.getMessage()); + EstateListResponse response = EstateListResponse.from(estate, Optional.empty()); + responses.add(response); + } });
🧹 Nitpick comments (8)
src/test/java/com/zipte/platform/server/domain/estate/EstateFixtures.java (1)
1-40: 테스트 픽스처 클래스가 잘 구현되었습니다.포괄적인 테스트 데이터를 제공하고 빌더 패턴을 적절히 활용했습니다. 다만, 테스트 데이터의 일관성과 재사용성을 위해 몇 가지 개선사항을 제안합니다.
테스트 데이터 개선 제안:
+ public static Estate stub() { + return stub("TEST-KAPT-CODE-1234"); + } + + public static Estate stub(String kaptCode) { Location location = Location.builder() .coordinates(Arrays.asList( 127.123456, 37.12345 )).build(); return Estate.builder() .id("test-estate-id") - .kaptCode("TEST-KAPT-CODE-1234") + .kaptCode(kaptCode) - .pricePerSquareMeter("test-price-1500000") + .pricePerSquareMeter("1500000") // ... 기타 필드들도 "test-" 접두사 제거 고려 .build(); + }이렇게 하면 다양한 테스트 시나리오에서 더 유연하게 사용할 수 있습니다.
src/main/java/com/zipte/platform/server/adapter/in/web/swagger/EstateApiSpec.java (1)
84-90: 가격 조회 API 명세 검토PathVariable을 사용하는 API와 RequestParam을 사용하는 API가 같은 기능을 제공하고 있습니다. REST API 설계 관점에서 일관성을 위해 하나의 방식으로 통일하는 것을 고려해보세요.
예를 들어, 다음과 같이 통합할 수 있습니다:
@Operation( - summary = "아파트 가격 정보 조회", - description = "아파트 코드를 통해 가격 정보를 조회합니다." + summary = "아파트 가격 정보 조회", + description = "아파트 코드와 선택적 평수를 통해 가격 정보를 조회합니다." ) -ApiResponse<List<EstatePriceListResponse>> getPrice( - @Parameter(description = "아파트 코드", required = true, example = "A46392821") - @PathVariable String kaptCode); +ApiResponse<List<EstatePriceListResponse>> getPrice( + @Parameter(description = "아파트 코드", required = true, example = "A46392821") + @RequestParam String kaptCode, + @Parameter(description = "면적 (선택사항)", required = false, example = "83.73") + @RequestParam(required = false) Double area);src/main/java/com/zipte/platform/server/adapter/in/web/EstateApi.java (1)
30-31: 의존성 주입 및 주석 개선가격 관련 의존성이 적절히 주입되었습니다. 주석을 더 명확하게 작성하는 것을 고려해보세요.
- /// 가격 의존성 + // 아파트 가격 조회 서비스src/main/java/com/zipte/platform/server/application/service/EstateService.java (1)
31-32: 의존성 주입 및 주석 개선가격 조회를 위한 포트가 적절히 주입되었습니다. 주석을 더 명확하게 작성하는 것을 고려해보세요.
- /// 아파트 가격 조회 + // 아파트 가격 조회 포트src/test/java/com/zipte/platform/server/adapter/out/EstatePersistenceAdapterTest.java (1)
50-67: 단위 테스트 개선 제안현재 테스트에서
repository에 직접 접근하여 검증하고 있습니다. 단위 테스트 관점에서는 SUT(System Under Test)의 결과만으로 검증하는 것이 더 적절합니다.@Test @DisplayName("[happy] 정상 조회") void load_happy() { // Given Estate stub = EstateFixtures.stub(); String kaptCode = stub.getKaptCode(); // When Optional<Estate> estate = sut.loadEstateByCode(kaptCode); - Estate result = estate.get(); // Then - Assertions.assertThat(repository.findByKaptCode(kaptCode)) - .isNotEmpty(); - - Assertions.assertThat(result.getKaptCode()).isEqualTo(stub.getKaptCode()); + Assertions.assertThat(estate).isPresent(); + Assertions.assertThat(estate.get().getKaptCode()).isEqualTo(stub.getKaptCode()); }src/test/java/com/zipte/platform/server/adapter/in/web/EstateApiTest.java (3)
32-32: 사용되지 않는 임포트를 제거하세요.
MethodArgumentNotValidException이 임포트되었지만 테스트 코드에서 사용되지 않습니다.-import org.springframework.web.bind.MethodArgumentNotValidException;
64-68: Spring Boot의 자동 설정된 ObjectMapper 사용을 고려하세요.매번 새로운 ObjectMapper를 생성하는 대신 Spring Boot가 제공하는 자동 설정된 ObjectMapper를 주입받아 사용하는 것이 좋습니다.
- private ObjectMapper objectMapper; + @Autowired + private ObjectMapper objectMapper; - @BeforeEach - public void init() { - objectMapper = new ObjectMapper(); - objectMapper.registerModule(new JavaTimeModule()); - }
148-151: 매직 넘버와 문자열을 상수로 추출하세요.테스트에서 하드코딩된 값들이 여러 곳에서 반복 사용되고 있습니다. 가독성과 유지보수성을 위해 상수로 추출하는 것이 좋습니다.
+ private static final String TEST_REGION = "서울시 강남구"; + private static final String TEST_COMPLEX_CODE = "TEST-KAPT-CODE-1234"; + private static final int TEST_PAGE = 1; + private static final int TEST_SIZE = 10; // Given - String region = "서울시 강남구"; - PageRequest pageRequest = new PageRequest(1, 10); - Pageable pageable = org.springframework.data.domain.PageRequest.of(1, 10); + String region = TEST_REGION; + PageRequest pageRequest = new PageRequest(TEST_PAGE, TEST_SIZE); + Pageable pageable = org.springframework.data.domain.PageRequest.of(TEST_PAGE, TEST_SIZE); .andExpect(jsonPath("$.success").value(true)) - .andExpect(jsonPath("$.data.dtoList[0].complexCode").value("TEST-KAPT-CODE-1234")) - .andExpected(jsonPath("$.data.pageRequest.page").value(1)) + .andExpect(jsonPath("$.data.dtoList[0].complexCode").value(TEST_COMPLEX_CODE)) + .andExpected(jsonPath("$.data.pageRequest.page").value(TEST_PAGE))Also applies to: 176-178
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/zipte/platform/server/adapter/in/web/EstateApi.java(6 hunks)src/main/java/com/zipte/platform/server/adapter/in/web/dto/response/EstateDetailResponse.java(2 hunks)src/main/java/com/zipte/platform/server/adapter/in/web/swagger/EstateApiSpec.java(4 hunks)src/main/java/com/zipte/platform/server/adapter/out/mongo/estate/EstateMongoRepository.java(1 hunks)src/main/java/com/zipte/platform/server/application/in/estate/GetEstateUseCase.java(1 hunks)src/main/java/com/zipte/platform/server/application/service/EstateService.java(3 hunks)src/test/java/com/zipte/platform/server/adapter/in/web/EstateApiTest.java(1 hunks)src/test/java/com/zipte/platform/server/adapter/out/EstatePersistenceAdapterTest.java(1 hunks)src/test/java/com/zipte/platform/server/domain/estate/EstateFixtures.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/test/java/com/zipte/platform/server/adapter/out/EstatePersistenceAdapterTest.java (1)
src/test/java/com/zipte/platform/server/domain/estate/EstateFixtures.java (1)
EstateFixtures(5-40)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (16)
src/main/java/com/zipte/platform/server/adapter/in/web/dto/response/EstateDetailResponse.java (2)
7-7: import 문 변경이 적절합니다.
java.util.*import로 변경하여 새로 추가된List타입을 지원하는 것이 적절합니다.
40-44: 컬렉션 변환 메서드가 잘 구현되었습니다.팩토리 메서드 패턴을 일관성 있게 따르고 있으며, 스트림 API를 효율적으로 활용했습니다.
toList()메서드 사용도 최신 Java 스타일에 맞습니다.src/main/java/com/zipte/platform/server/application/in/estate/GetEstateUseCase.java (2)
32-33: 아파트 비교 기능이 잘 추가되었습니다.새로운
loadEstatesByCompare메서드가 아파트 비교 기능을 지원하기 위해 적절히 추가되었습니다. 메서드명과 파라미터가 의도를 명확히 표현하고 있습니다.
18-18:Details
✅ Verification successful
반환 타입 변경을 확인해주세요.
loadEstatesNearBy메서드의 반환 타입이List<Estate>에서List<EstateListResponse>로 변경되었습니다. 이는 아키텍처 관점에서 중요한 변경사항입니다.구현체에서 이 변경사항이 올바르게 반영되었는지 확인해주세요:
🏁 Script executed:
#!/bin/bash # EstateService 구현체에서 loadEstatesNearBy 메서드 구현 확인 ast-grep --pattern $'class $_ implements GetEstateUseCase { $$$ List<EstateListResponse> loadEstatesNearBy($_, $_, $_) { $$$ } $$$ }'Length of output: 146
🏁 Script executed:
#!/bin/bash # GetEstateUseCase를 구현한 클래스와 loadEstatesNearBy 메서드 위치 검색 rg -n "implements GetEstateUseCase" rg -n "loadEstatesNearBy"Length of output: 2947
loadEstatesNearBy반환 타입 변경 정상 반영됨
EstateService#loadEstatesNearBy(double latitude, double longitude, double radiusInKm)메서드가
인터페이스와 동일하게List<EstateListResponse>를 반환하도록 구현되어 있습니다.– 파일: src/main/java/com/zipte/platform/server/application/service/EstateService.java
– 라인: 36추가 수정 사항은 없습니다.
src/main/java/com/zipte/platform/server/adapter/in/web/swagger/EstateApiSpec.java (3)
8-8: 새로운 DTO 임포트 추가 확인가격 관련 응답 DTO가 적절히 임포트되었습니다.
49-49: 좌표 예시값 업데이트 확인기존 메서드들의 경도/위도 예시값이 새로운 좌표로 업데이트되었습니다. 실제 테스트에 사용되는 좌표와 일치하는지 확인해주세요.
Also applies to: 52-52, 64-64, 67-67
106-115: 아파트 비교 API 명세 검토비교 기능의 API 명세가 적절히 정의되었습니다. 파라미터명
first와second가 직관적이며, 적절한 예시값도 제공되었습니다.src/main/java/com/zipte/platform/server/adapter/in/web/EstateApi.java (3)
8-8: 의존성 및 임포트 추가 확인가격 관련 기능을 위한 임포트들이 적절히 추가되었습니다.
Also applies to: 10-10, 14-14
45-50: 예외 처리 단순화 확인서비스 레이어에서 예외를 던지도록 변경되어 컨트롤러의 예외 처리가 단순화되었습니다. 이는 관심사의 분리 관점에서 적절한 변경입니다.
81-81: 메서드 단순화 개선
getEstateByLocation메서드가 직접 서비스 호출 결과를 반환하도록 단순화되었습니다. 코드가 더 간결해졌습니다.src/main/java/com/zipte/platform/server/application/service/EstateService.java (2)
3-3: 필요한 임포트 및 어노테이션 추가에러 코드, 가격 관련 기능, 로깅을 위한 임포트들과
@Slf4j어노테이션이 적절히 추가되었습니다.Also applies to: 6-6, 10-10, 12-12, 19-19, 22-22
75-78: 예외 처리 방식 변경 확인메서드들이
Optional을 반환하는 대신 직접Estate를 반환하고 예외를 던지도록 변경되었습니다. 이는 컨트롤러 레이어와의 일관성을 위한 적절한 변경입니다.Also applies to: 81-84
src/test/java/com/zipte/platform/server/adapter/out/EstatePersistenceAdapterTest.java (4)
19-28: 테스트 클래스 설정 확인Spring Boot 테스트 설정이 적절히 구성되었습니다.
@ActiveProfiles("test"),@Transactional어노테이션과 필요한 의존성 주입이 올바르게 설정되었습니다.
30-44: 테스트 데이터 초기화 로직 확인
@BeforeEach에서 테스트 데이터를 정리하고 새로운 테스트 데이터를 삽입하는 로직이 적절합니다. 기존 데이터 삭제 후 새 데이터 삽입으로 테스트 격리가 보장됩니다.
116-132: 지리적 근접 조회 테스트 확인좌표 기반 근처 아파트 조회 테스트가 적절히 구현되었습니다. 실제 좌표값을 사용하여 반경 내 조회 기능을 테스트하고 있습니다.
135-180: 존재 여부 확인 테스트 검토아파트 코드 존재 여부를 확인하는 테스트들이 포괄적으로 작성되었습니다. positive/negative 케이스 모두 포함되어 있어 좋습니다.
Comment on lines
+31
to
+33
|
|
||
| /// 테스트용 | ||
| void deleteByKaptCode(String kaptCode); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
테스트 전용 메서드의 위치를 재검토해주세요.
프로덕션 레포지토리 인터페이스에 테스트 전용 메서드를 추가하는 것은 좋은 관행이 아닙니다. 다음 대안을 고려해보세요:
- 테스트 프로파일 전용 구성 클래스 생성
- 테스트 패키지에 별도의 테스트 전용 레포지토리 인터페이스 생성
@TestRepository애노테이션을 사용한 테스트 전용 빈 정의
테스트 전용 레포지토리 인터페이스 생성 예시:
// src/test/java/com/zipte/platform/server/adapter/out/mongo/estate/TestEstateMongoRepository.java
+@TestRepository
+public interface TestEstateMongoRepository extends EstateMongoRepository {
+ void deleteByKaptCode(String kaptCode);
+}프로덕션 인터페이스에서 제거:
- /// 테스트용
- void deleteByKaptCode(String kaptCode);📝 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
| /// 테스트용 | |
| void deleteByKaptCode(String kaptCode); |
🤖 Prompt for AI Agents
In
src/main/java/com/zipte/platform/server/adapter/out/mongo/estate/EstateMongoRepository.java
around lines 31 to 33, the deleteByKaptCode method is a test-only method
improperly placed in the production repository interface. Remove this method
from the production interface and instead create a separate test-only repository
interface in the test package or define a test-specific bean using
@TestRepository annotation to isolate test code from production code.
|
|
||
| // 코드를 바탕으로 아파트 가져오기 | ||
| Optional<Estate> loadEstateByCode(String kaptCode); | ||
| Estate loadEstateByCode(String kaptCode); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Optional 제거로 인한 예외 처리 전략을 문서화해주세요.
loadEstateByCode와 loadEstateByName 메서드의 반환 타입에서 Optional이 제거되었습니다. 이는 존재하지 않는 엔티티에 대해 예외를 던지는 방식으로 변경되었음을 의미합니다.
메서드 문서에 예외 처리 전략을 명시해주세요:
// 코드를 바탕으로 아파트 가져오기
+ /**
+ * @throws EstateNotFoundException 해당 코드의 아파트가 존재하지 않는 경우
+ */
Estate loadEstateByCode(String kaptCode);
// 이름을 바탕으로 아파트 가져오기
+ /**
+ * @throws EstateNotFoundException 해당 이름의 아파트가 존재하지 않는 경우
+ */
Estate loadEstateByName(String kaptName);구현체에서 적절한 예외 처리가 되어있는지 확인:
Also applies to: 27-27
🏁 Script executed:
#!/bin/bash
# EstateService에서 예외 처리 확인
rg -A 10 "loadEstateByCode|loadEstateByName" --include="*Service.java"Length of output: 514
🏁 Script executed:
#!/bin/bash
# 1. GetEstateUseCase 구현체 찾기
rg -n --pcre2 "implements\s+GetEstateUseCase" -g "*.java"
# 2. Service 레이어에서 loadEstateByCode 구현부 및 예외 처리 확인
rg -n -A5 "loadEstateByCode\(" -g "*Service.java"
# 3. Service 레이어에서 loadEstateByName 구현부 및 예외 처리 확인
rg -n -A5 "loadEstateByName\(" -g "*Service.java"Length of output: 4984
Javadoc에 예외 처리 전략 명시 및 예외 타입 통일 제안
Optional 대신 예외를 던지는 방식으로 변경되었으나, 구현체별로 서로 다른 예외를 사용하고 있습니다. 이를 인터페이스 문서화하고, 가능하다면 커스텀 예외(EstateNotFoundException 등)로 통일해주세요.
- 대상 파일
- src/main/java/com/zipte/platform/server/application/in/estate/GetEstateUseCase.java
- 구현체에서 던지는 예외
- EstateService & OpenAiService →
NoSuchElementException(ErrorCode.NOT_ESTATE.getMessage()) - PropertyService →
EntityNotFoundException("해당하는 아파트가 존재하지 않습니다.")
- EstateService & OpenAiService →
제안 Javadoc 변경 예시:
public interface GetEstateUseCase {
- Estate loadEstateByCode(String kaptCode);
+ /**
+ * @throws NoSuchElementException ErrorCode.NOT_ESTATE.getMessage()와 함께
+ * 아파트가 존재하지 않는 경우
+ */
+ // TODO: 커스텀 예외 EstateNotFoundException으로 변경 시 @throws 주석도 함께 수정
Estate loadEstateByCode(String kaptCode);
- Estate loadEstateByName(String kaptName);
+ /**
+ * @throws NoSuchElementException ErrorCode.NOT_ESTATE.getMessage()와 함께
+ * 아파트가 존재하지 않는 경우
+ */
Estate loadEstateByName(String kaptName);
}- 구현체가 실제로 던지는 예외와 Javadoc이 일치하는지 재확인 부탁드립니다.
🤖 Prompt for AI Agents
In
src/main/java/com/zipte/platform/server/application/in/estate/GetEstateUseCase.java
at lines 24 and 27, the methods loadEstateByCode and loadEstateByName have
removed Optional return types and now throw exceptions when entities are not
found. Update the Javadoc for these methods to clearly document this
exception-throwing behavior, specifying the type of exception thrown.
Additionally, review the implementations to ensure they throw a consistent
custom exception, such as EstateNotFoundException, instead of varying exceptions
like NoSuchElementException or EntityNotFoundException, and update the interface
and implementations accordingly to unify the exception type.
Comment on lines
+98
to
+106
| @GetMapping("/compare") | ||
| public ApiResponse<List<EstateDetailResponse>> getEstateByCompare( | ||
| @RequestParam(value = "first") String first, | ||
| @RequestParam(value = "second") String second | ||
| ) { | ||
| List<Estate> estates = getService.loadEstatesByCompare(List.of(first, second)); | ||
|
|
||
| return ApiResponse.ok(EstateDetailResponse.from(estates)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
아파트 비교 기능 구현
비교 기능이 적절히 구현되었습니다. 입력 파라미터 검증을 추가하는 것을 고려해보세요.
@GetMapping("/compare")
public ApiResponse<List<EstateDetailResponse>> getEstateByCompare(
@RequestParam(value = "first") String first,
@RequestParam(value = "second") String second
) {
+ if (first == null || first.trim().isEmpty()) {
+ throw new IllegalArgumentException("첫 번째 아파트 코드는 필수입니다.");
+ }
+ if (second == null || second.trim().isEmpty()) {
+ throw new IllegalArgumentException("두 번째 아파트 코드는 필수입니다.");
+ }
+ if (first.equals(second)) {
+ throw new IllegalArgumentException("비교할 아파트는 서로 달라야 합니다.");
+ }
+
List<Estate> estates = getService.loadEstatesByCompare(List.of(first, second));
return ApiResponse.ok(EstateDetailResponse.from(estates));
}📝 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
| @GetMapping("/compare") | |
| public ApiResponse<List<EstateDetailResponse>> getEstateByCompare( | |
| @RequestParam(value = "first") String first, | |
| @RequestParam(value = "second") String second | |
| ) { | |
| List<Estate> estates = getService.loadEstatesByCompare(List.of(first, second)); | |
| return ApiResponse.ok(EstateDetailResponse.from(estates)); | |
| } | |
| @GetMapping("/compare") | |
| public ApiResponse<List<EstateDetailResponse>> getEstateByCompare( | |
| @RequestParam(value = "first") String first, | |
| @RequestParam(value = "second") String second | |
| ) { | |
| if (first == null || first.trim().isEmpty()) { | |
| throw new IllegalArgumentException("첫 번째 아파트 코드는 필수입니다."); | |
| } | |
| if (second == null || second.trim().isEmpty()) { | |
| throw new IllegalArgumentException("두 번째 아파트 코드는 필수입니다."); | |
| } | |
| if (first.equals(second)) { | |
| throw new IllegalArgumentException("비교할 아파트는 서로 달라야 합니다."); | |
| } | |
| List<Estate> estates = getService.loadEstatesByCompare(List.of(first, second)); | |
| return ApiResponse.ok(EstateDetailResponse.from(estates)); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/zipte/platform/server/adapter/in/web/EstateApi.java around
lines 98 to 106, the getEstateByCompare method lacks validation for the input
parameters 'first' and 'second'. Add checks to ensure these parameters are not
null or empty before proceeding, and handle invalid inputs appropriately, such
as returning a bad request response or throwing a validation exception.
Comment on lines
+92
to
+104
| @Override | ||
| public List<Estate> loadEstatesByCompare(List<String> kaptCodes) { | ||
|
|
||
| /// 예외처리를 통한 아파트 가져오기 | ||
| Estate first = loadPort.loadEstateByCode(kaptCodes.get(0)) | ||
| .orElseThrow(() -> new NoSuchElementException("first" + ErrorCode.NOT_ESTATE.getMessage())); | ||
|
|
||
| Estate second = loadPort.loadEstateByCode(kaptCodes.get(1)) | ||
| .orElseThrow(() -> new NoSuchElementException("second" + ErrorCode.NOT_ESTATE.getMessage())); | ||
|
|
||
| return List.of(first, second); | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
아파트 비교 기능 구현 검토
비교 기능이 적절히 구현되었습니다. 입력 파라미터 검증을 추가하는 것을 고려해보세요.
@Override
public List<Estate> loadEstatesByCompare(List<String> kaptCodes) {
+ if (kaptCodes == null || kaptCodes.size() != 2) {
+ throw new IllegalArgumentException("비교를 위해서는 정확히 2개의 아파트 코드가 필요합니다.");
+ }
+
+ if (kaptCodes.get(0).equals(kaptCodes.get(1))) {
+ throw new IllegalArgumentException("비교할 아파트는 서로 달라야 합니다.");
+ }
/// 예외처리를 통한 아파트 가져오기
Estate first = loadPort.loadEstateByCode(kaptCodes.get(0))
- .orElseThrow(() -> new NoSuchElementException("first" + ErrorCode.NOT_ESTATE.getMessage()));
+ .orElseThrow(() -> new NoSuchElementException("첫 번째 " + ErrorCode.NOT_ESTATE.getMessage()));
Estate second = loadPort.loadEstateByCode(kaptCodes.get(1))
- .orElseThrow(() -> new NoSuchElementException("second" + ErrorCode.NOT_ESTATE.getMessage()));
+ .orElseThrow(() -> new NoSuchElementException("두 번째 " + ErrorCode.NOT_ESTATE.getMessage()));
return List.of(first, second);
}📝 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
| @Override | |
| public List<Estate> loadEstatesByCompare(List<String> kaptCodes) { | |
| /// 예외처리를 통한 아파트 가져오기 | |
| Estate first = loadPort.loadEstateByCode(kaptCodes.get(0)) | |
| .orElseThrow(() -> new NoSuchElementException("first" + ErrorCode.NOT_ESTATE.getMessage())); | |
| Estate second = loadPort.loadEstateByCode(kaptCodes.get(1)) | |
| .orElseThrow(() -> new NoSuchElementException("second" + ErrorCode.NOT_ESTATE.getMessage())); | |
| return List.of(first, second); | |
| } | |
| @Override | |
| public List<Estate> loadEstatesByCompare(List<String> kaptCodes) { | |
| if (kaptCodes == null || kaptCodes.size() != 2) { | |
| throw new IllegalArgumentException("비교를 위해서는 정확히 2개의 아파트 코드가 필요합니다."); | |
| } | |
| if (kaptCodes.get(0).equals(kaptCodes.get(1))) { | |
| throw new IllegalArgumentException("비교할 아파트는 서로 달라야 합니다."); | |
| } | |
| /// 예외처리를 통한 아파트 가져오기 | |
| Estate first = loadPort.loadEstateByCode(kaptCodes.get(0)) | |
| .orElseThrow(() -> new NoSuchElementException("첫 번째 " + ErrorCode.NOT_ESTATE.getMessage())); | |
| Estate second = loadPort.loadEstateByCode(kaptCodes.get(1)) | |
| .orElseThrow(() -> new NoSuchElementException("두 번째 " + ErrorCode.NOT_ESTATE.getMessage())); | |
| return List.of(first, second); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/zipte/platform/server/application/service/EstateService.java
around lines 92 to 104, the method loadEstatesByCompare lacks validation for the
input list kaptCodes. Add input parameter validation to check if kaptCodes is
not null and contains at least two elements before accessing indices 0 and 1. If
the validation fails, throw an appropriate exception or handle the error
gracefully to prevent runtime exceptions.
Comment on lines
+46
to
+83
| @Nested | ||
| @DisplayName("조회 테스트") | ||
| class Load { | ||
|
|
||
| @Test | ||
| @DisplayName("[happy] 정상 조회") | ||
| void load_happy() { | ||
|
|
||
| // Given | ||
| Estate stub = EstateFixtures.stub(); | ||
| String kaptCode = stub.getKaptCode(); | ||
|
|
||
| // When | ||
| Optional<Estate> estate = sut.loadEstateByCode(kaptCode); | ||
| Estate result = estate.get(); | ||
|
|
||
| // Then | ||
| Assertions.assertThat(repository.findByKaptCode(kaptCode)) | ||
| .isNotEmpty(); | ||
|
|
||
| Assertions.assertThat(result.getKaptCode()).isEqualTo(stub.getKaptCode()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("[happy] 지역 기반 페이징 조회 성공") | ||
| void loadEstatesByRegionWithPaging_happy() { | ||
| // Given | ||
| Estate stub = EstateFixtures.stub(); | ||
| Pageable pageable = PageRequest.of(0, 10); | ||
|
|
||
| // When | ||
| Page<Estate> result = sut.loadEstatesByRegion(stub.getKaptAddr(), pageable); | ||
|
|
||
| // Then | ||
| Assertions.assertThat(result.getContent()).isNotEmpty(); | ||
| Assertions.assertThat(result.getContent().get(0).getKaptAddr()) | ||
| .contains(stub.getKaptAddr()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
지역 기반 조회 테스트 개선
페이징 및 비페이징 지역 조회 테스트가 포함되어 있습니다. 하지만 테스트에서 stub.getKaptAddr()를 직접 사용하여 조회하고 있어 실제 지역 필터링이 제대로 테스트되지 않을 수 있습니다.
@Test
@DisplayName("[happy] 지역 기반 페이징 조회 성공")
void loadEstatesByRegionWithPaging_happy() {
// Given
Estate stub = EstateFixtures.stub();
+ String region = "분당구"; // 주소에서 일부만 추출하여 검색
Pageable pageable = PageRequest.of(0, 10);
// When
- Page<Estate> result = sut.loadEstatesByRegion(stub.getKaptAddr(), pageable);
+ Page<Estate> result = sut.loadEstatesByRegion(region, pageable);
// Then
Assertions.assertThat(result.getContent()).isNotEmpty();
Assertions.assertThat(result.getContent().get(0).getKaptAddr())
- .contains(stub.getKaptAddr());
+ .contains(region);
}📝 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
| @Nested | |
| @DisplayName("조회 테스트") | |
| class Load { | |
| @Test | |
| @DisplayName("[happy] 정상 조회") | |
| void load_happy() { | |
| // Given | |
| Estate stub = EstateFixtures.stub(); | |
| String kaptCode = stub.getKaptCode(); | |
| // When | |
| Optional<Estate> estate = sut.loadEstateByCode(kaptCode); | |
| Estate result = estate.get(); | |
| // Then | |
| Assertions.assertThat(repository.findByKaptCode(kaptCode)) | |
| .isNotEmpty(); | |
| Assertions.assertThat(result.getKaptCode()).isEqualTo(stub.getKaptCode()); | |
| } | |
| @Test | |
| @DisplayName("[happy] 지역 기반 페이징 조회 성공") | |
| void loadEstatesByRegionWithPaging_happy() { | |
| // Given | |
| Estate stub = EstateFixtures.stub(); | |
| Pageable pageable = PageRequest.of(0, 10); | |
| // When | |
| Page<Estate> result = sut.loadEstatesByRegion(stub.getKaptAddr(), pageable); | |
| // Then | |
| Assertions.assertThat(result.getContent()).isNotEmpty(); | |
| Assertions.assertThat(result.getContent().get(0).getKaptAddr()) | |
| .contains(stub.getKaptAddr()); | |
| } | |
| @Test | |
| @DisplayName("[happy] 지역 기반 페이징 조회 성공") | |
| void loadEstatesByRegionWithPaging_happy() { | |
| // Given | |
| Estate stub = EstateFixtures.stub(); | |
| String region = "분당구"; // 주소에서 일부만 추출하여 검색 | |
| Pageable pageable = PageRequest.of(0, 10); | |
| // When | |
| Page<Estate> result = sut.loadEstatesByRegion(region, pageable); | |
| // Then | |
| Assertions.assertThat(result.getContent()).isNotEmpty(); | |
| Assertions.assertThat(result.getContent().get(0).getKaptAddr()) | |
| .contains(region); | |
| } |
🤖 Prompt for AI Agents
In
src/test/java/com/zipte/platform/server/adapter/out/EstatePersistenceAdapterTest.java
between lines 46 and 83, the region-based paging test uses stub.getKaptAddr()
directly for querying, which may not effectively validate the actual region
filtering logic. To fix this, modify the test to use a controlled or distinct
region value for the query that differs from the stub's address, ensuring the
filtering behavior is properly tested. Additionally, consider adding a
non-paging region-based query test with similar controlled inputs to verify both
paging and non-paging scenarios accurately.
Comment on lines
+70
to
+270
| @Nested | ||
| @DisplayName("아파트 조회") | ||
| class Get { | ||
|
|
||
| @Test | ||
| @DisplayName("[happy] Code를 바탕으로 정상적으로 아파트를 조회한다.") | ||
| void loadByCode_happy() throws Exception { | ||
|
|
||
| // Given | ||
| String code = "test_code"; | ||
| Estate stub = EstateFixtures.stub(); | ||
|
|
||
| given(getService.loadEstateByCode(code)) | ||
| .willReturn(stub); | ||
|
|
||
| ApiResponse<EstateDetailResponse> response = ApiResponse.ok(EstateDetailResponse.from(stub)); | ||
|
|
||
| // When | ||
| ResultActions resultActions = mockMvc.perform( | ||
| MockMvcRequestBuilders | ||
| .get("/api/v1/estate") | ||
| .param("code", code) | ||
| .contentType(MediaType.APPLICATION_JSON)); | ||
|
|
||
| // Then | ||
| MvcResult mvcResult = resultActions | ||
| .andExpect(status().isOk()) | ||
| .andDo(print()) | ||
| .andReturn(); | ||
|
|
||
|
|
||
| String expectResponse = objectMapper.writeValueAsString(response); | ||
| mvcResult.getResponse().setCharacterEncoding("UTF-8"); | ||
| String responseBody = mvcResult.getResponse().getContentAsString(); | ||
|
|
||
| Assertions.assertThat(response).isNotNull(); | ||
| Assertions.assertThat(responseBody).isEqualTo(expectResponse); | ||
|
|
||
|
|
||
| } | ||
|
|
||
|
|
||
| @Test | ||
| @DisplayName("[happy] 이름을 바탕으로 정상적으로 아파트를 조회한다.") | ||
| void loadByName_happy() throws Exception { | ||
|
|
||
| // Given | ||
| String name = "테스트아파트"; | ||
| Estate stub = EstateFixtures.stub(); // 고정된 stub 객체 | ||
| given(getService.loadEstateByName(name)) | ||
| .willReturn(stub); | ||
|
|
||
| ApiResponse<EstateDetailResponse> response = ApiResponse.created(EstateDetailResponse.from(stub)); | ||
|
|
||
| // When | ||
| ResultActions resultActions = mockMvc.perform( | ||
| MockMvcRequestBuilders | ||
| .get("/api/v1/estate") | ||
| .param("name", name) | ||
| .contentType(MediaType.APPLICATION_JSON)); | ||
|
|
||
| // Then | ||
| MvcResult mvcResult = resultActions | ||
| .andExpect(status().isOk()) | ||
| .andDo(print()) | ||
| .andReturn(); | ||
|
|
||
| String expectedResponse = objectMapper.writeValueAsString(response); | ||
| mvcResult.getResponse().setCharacterEncoding("UTF-8"); | ||
| String actualResponse = mvcResult.getResponse().getContentAsString(); | ||
|
|
||
| Assertions.assertThat(actualResponse).isEqualTo(expectedResponse); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("[happy] 특정 지역의 아파트 리스트를 정상적으로 조회한다.") | ||
| void getEstateByRegion_happy() throws Exception { | ||
| // Given | ||
| String region = "서울시 강남구"; | ||
| PageRequest pageRequest = new PageRequest(1, 10); | ||
| Pageable pageable = org.springframework.data.domain.PageRequest.of(1, 10); | ||
| List<Estate> content = List.of(EstateFixtures.stub()); | ||
|
|
||
| PageImpl<Estate> page = new PageImpl<>(content, pageable, 1); | ||
|
|
||
| given(getService.loadEstatesByRegion(eq(region), any(Pageable.class))) | ||
| .willReturn(page); | ||
|
|
||
|
|
||
| List<EstateListResponse> dtos = EstateListResponse.from(content); | ||
| ApiResponse<PageResponse<EstateListResponse>> response = ApiResponse.ok( | ||
| new PageResponse<>(dtos, pageRequest, page.getTotalElements()) | ||
| ); | ||
|
|
||
| // When | ||
| ResultActions resultActions = mockMvc.perform( | ||
| MockMvcRequestBuilders | ||
| .get("/api/v1/estate/list") | ||
| .param("region", region) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| ); | ||
|
|
||
| // Then | ||
| MvcResult result = resultActions | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.success").value(true)) | ||
| .andExpect(jsonPath("$.data.dtoList[0].complexCode").value("TEST-KAPT-CODE-1234")) | ||
| .andExpect(jsonPath("$.data.pageRequest.page").value(1)) | ||
| .andExpect(jsonPath("$.data.pageNumList[0]").value(1)) | ||
| .andDo(print()) | ||
| .andReturn(); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("[happy] 특정 좌표 반경 내 아파트 목록을 정상적으로 조회한다.") | ||
| void getEstateByLocation_happy() throws Exception { | ||
| // Given | ||
| double longitude = 127.0; | ||
| double latitude = 37.5; | ||
| double radius = 1.0; | ||
|
|
||
| List<EstateListResponse> expectedList = EstateListResponse.from(List.of(EstateFixtures.stub())); | ||
| given(getService.loadEstatesNearBy(longitude, latitude, radius)).willReturn(expectedList); | ||
|
|
||
| // When | ||
| ResultActions resultActions = mockMvc.perform( | ||
| MockMvcRequestBuilders | ||
| .get("/api/v1/estate/list/location") | ||
| .param("longitude", String.valueOf(longitude)) | ||
| .param("latitude", String.valueOf(latitude)) | ||
| .param("radius", String.valueOf(radius)) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| ); | ||
|
|
||
| // Then | ||
| MvcResult result = resultActions.andExpect(status().isOk()).andDo(print()).andReturn(); | ||
| String expected = objectMapper.writeValueAsString(ApiResponse.ok(expectedList)); | ||
| String actual = result.getResponse().getContentAsString(); | ||
|
|
||
| Assertions.assertThat(actual).isEqualTo(expected); | ||
| } | ||
|
|
||
|
|
||
| @Test | ||
| @DisplayName("[happy] AI를 통한 아파트 요약 정보를 정상적으로 조회한다.") | ||
| void getEstateDetailAI_happy() throws Exception { | ||
| // Given | ||
| String kaptCode = "APT1234"; | ||
| String aiSummary = "이 아파트는 좋은 입지와 편리한 교통을 갖추고 있습니다."; | ||
|
|
||
| given(openAiService.getKaptCharacteristic(kaptCode)) | ||
| .willReturn(aiSummary); | ||
|
|
||
| // When | ||
| ResultActions resultActions = mockMvc.perform( | ||
| MockMvcRequestBuilders | ||
| .get("/api/v1/estate/ai/" + kaptCode) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| ); | ||
|
|
||
| // Then | ||
| MvcResult result = resultActions.andExpect(status().isOk()).andDo(print()).andReturn(); | ||
| String expected = objectMapper.writeValueAsString(ApiResponse.ok(aiSummary)); | ||
| String actual = result.getResponse().getContentAsString(); | ||
|
|
||
| Assertions.assertThat(actual) | ||
| .isEqualTo(expected); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("[happy] 두 개의 아파트를 비교 조회한다.") | ||
| void getEstateByCompare_happy() throws Exception { | ||
| // Given | ||
| String first = "APT1"; | ||
| String second = "APT2"; | ||
| List<Estate> estates = List.of(EstateFixtures.stub(), EstateFixtures.stub()); | ||
|
|
||
| given(getService.loadEstatesByCompare(List.of(first, second))).willReturn(estates); | ||
|
|
||
| List<EstateDetailResponse> responseList = EstateDetailResponse.from(estates); | ||
|
|
||
| // When | ||
| ResultActions resultActions = mockMvc.perform( | ||
| MockMvcRequestBuilders | ||
| .get("/api/v1/estate/compare") | ||
| .param("first", first) | ||
| .param("second", second) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| ); | ||
|
|
||
| // Then | ||
| MvcResult result = resultActions.andExpect(status().isOk()).andDo(print()).andReturn(); | ||
| String expected = objectMapper.writeValueAsString(ApiResponse.ok(responseList)); | ||
| String actual = result.getResponse().getContentAsString(); | ||
|
|
||
| Assertions.assertThat(actual).isEqualTo(expected); | ||
| } | ||
|
|
||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
예외 상황과 엣지 케이스에 대한 테스트를 추가하세요.
현재 모든 테스트가 성공 케이스(happy path)만 다루고 있습니다. 실제 운영 환경에서 발생할 수 있는 다음과 같은 상황들에 대한 테스트를 추가하는 것을 권장합니다:
- 존재하지 않는 아파트 코드/이름으로 조회 시 예외 처리
- 잘못된 좌표값 입력 시 검증
- 페이지네이션 파라미터 검증 (음수, 0 등)
- 빈 문자열이나 null 입력값 처리
@Test
@DisplayName("[sad] 존재하지 않는 코드로 아파트 조회 시 예외를 반환한다.")
void loadByCode_notFound() throws Exception {
// Given
String nonExistentCode = "NON_EXISTENT";
given(getService.loadEstateByCode(nonExistentCode))
.willThrow(new EntityNotFoundException("Estate not found"));
// When & Then
mockMvc.perform(MockMvcRequestBuilders
.get("/api/v1/estate")
.param("code", nonExistentCode)
.contentType(MediaType.APPLICATION_JSON))
.andExpect(status().isNotFound())
.andDo(print());
}🤖 Prompt for AI Agents
In src/test/java/com/zipte/platform/server/adapter/in/web/EstateApiTest.java
between lines 70 and 270, the current tests only cover successful scenarios. Add
tests for edge cases and error handling such as querying with non-existent
apartment codes or names, invalid coordinate inputs, pagination parameter
validation (e.g., negative or zero values), and handling empty or null inputs.
For example, implement tests that mock service methods to throw exceptions like
EntityNotFoundException and verify the controller returns appropriate HTTP error
statuses like 404 Not Found.
| given(getService.loadEstateByName(name)) | ||
| .willReturn(stub); | ||
|
|
||
| ApiResponse<EstateDetailResponse> response = ApiResponse.created(EstateDetailResponse.from(stub)); |
There was a problem hiding this comment.
응답 생성 방식의 일관성을 맞춰주세요.
ApiResponse.created()를 사용하고 있지만 실제 테스트에서는 200 OK 상태를 기대하고 있습니다. 다른 테스트들과 일관성을 위해 ApiResponse.ok()를 사용해야 합니다.
- ApiResponse<EstateDetailResponse> response = ApiResponse.created(EstateDetailResponse.from(stub));
+ ApiResponse<EstateDetailResponse> response = ApiResponse.ok(EstateDetailResponse.from(stub));📝 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
| ApiResponse<EstateDetailResponse> response = ApiResponse.created(EstateDetailResponse.from(stub)); | |
| ApiResponse<EstateDetailResponse> response = ApiResponse.ok(EstateDetailResponse.from(stub)); |
🤖 Prompt for AI Agents
In src/test/java/com/zipte/platform/server/adapter/in/web/EstateApiTest.java at
line 122, the test uses ApiResponse.created() to generate the response but
expects a 200 OK status. To maintain consistency with other tests and match the
expected status, replace ApiResponse.created() with ApiResponse.ok() when
creating the response.
| // Given | ||
| String first = "APT1"; | ||
| String second = "APT2"; | ||
| List<Estate> estates = List.of(EstateFixtures.stub(), EstateFixtures.stub()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
비교 테스트에서 서로 다른 테스트 데이터를 사용하세요.
현재 같은 EstateFixtures.stub()을 두 번 호출하여 동일한 데이터로 비교 테스트를 수행하고 있습니다. 실제 비교 기능을 제대로 검증하려면 서로 다른 아파트 데이터를 사용해야 합니다.
- List<Estate> estates = List.of(EstateFixtures.stub(), EstateFixtures.stub());
+ Estate firstEstate = EstateFixtures.stubWithCode("APT1");
+ Estate secondEstate = EstateFixtures.stubWithCode("APT2");
+ List<Estate> estates = List.of(firstEstate, secondEstate);또는 EstateFixtures에 서로 다른 데이터를 생성하는 메서드가 있다면 활용하세요.
📝 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
| List<Estate> estates = List.of(EstateFixtures.stub(), EstateFixtures.stub()); | |
| Estate firstEstate = EstateFixtures.stubWithCode("APT1"); | |
| Estate secondEstate = EstateFixtures.stubWithCode("APT2"); | |
| List<Estate> estates = List.of(firstEstate, secondEstate); |
🤖 Prompt for AI Agents
In src/test/java/com/zipte/platform/server/adapter/in/web/EstateApiTest.java at
line 245, the test uses identical data by calling EstateFixtures.stub() twice,
which does not effectively test comparison logic. Replace the second call with a
different fixture method that generates distinct Estate data, or create a new
method in EstateFixtures to provide varied test data, ensuring the comparison
test uses two different Estate instances.
- 코드리뷰 반영
Test Coverage Report
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
📌 작업한 내용
♻️ refactor/ZIP-30-kapt : 좌표 기반 조회시, 최근 거래 나오도록 수정
✨ feat/ZIP-70-kapt : 아파트 비교 기능 구현
🔍 참고 사항
🖼️ 스크린샷
🔗 관련 이슈
ZIP-30
ZIP-70
✅ 체크리스트
Summary by CodeRabbit
Summary by CodeRabbit
신규 기능
개선 사항
문서화