-
Notifications
You must be signed in to change notification settings - Fork 0
♻️ refactor/ZIP-30-kapt : 아파트 관련 기능 수정 #60
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
Changes from all commits
10eadaa
4e9b51d
483d6f1
65f4939
9cdb604
c3aca42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,11 +5,13 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.zipte.platform.core.response.pageable.PageResponse; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.zipte.platform.server.adapter.in.web.dto.response.EstateDetailResponse; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.zipte.platform.server.adapter.in.web.dto.response.EstateListResponse; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.zipte.platform.server.adapter.in.web.dto.response.EstatePriceListResponse; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.zipte.platform.server.adapter.in.web.swagger.EstateApiSpec; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.zipte.platform.server.application.in.estate.EstatePriceUseCase; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.zipte.platform.server.application.in.estate.GetEstateUseCase; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.zipte.platform.server.application.in.external.OpenAiUseCase; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.zipte.platform.server.domain.estate.Estate; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import jakarta.persistence.EntityNotFoundException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.zipte.platform.server.domain.estate.EstatePrice; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.data.domain.Page; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.data.domain.Pageable; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -25,6 +27,9 @@ public class EstateApi implements EstateApiSpec { | |||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final GetEstateUseCase getService; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// 가격 의존성 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final EstatePriceUseCase priceService; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// AI 의존성 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final OpenAiUseCase openAiService; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -37,13 +42,11 @@ public ApiResponse<EstateDetailResponse> getEstate( | |||||||||||||||||||||||||||||||||||||||||||||||||||
| Estate estate; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (code != null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| estate = getService.loadEstateByCode(code) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| .orElseThrow(() -> new EntityNotFoundException("해당하는 아파트가 존재하지 않습니다.")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| estate = getService.loadEstateByCode(code); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (name != null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| estate = getService.loadEstateByName(name) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| .orElseThrow(() -> new EntityNotFoundException("해당하는 아파트가 존재하지 않습니다.")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| estate = getService.loadEstateByName(name); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new IllegalArgumentException("최소 하나 이상의 요청 파라미터가 필요합니다."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new IllegalArgumentException(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ApiResponse.created(EstateDetailResponse.from(estate)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -74,9 +77,8 @@ public ApiResponse<List<EstateListResponse>> getEstateByLocation( | |||||||||||||||||||||||||||||||||||||||||||||||||||
| @RequestParam(value = "longitude") double longitude, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| @RequestParam(value = "latitude") double latitude, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| @RequestParam(value = "radius") double radius) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<Estate> list = getService.loadEstatesNearBy(longitude, latitude, radius); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ApiResponse.ok(EstateListResponse.from(list)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ApiResponse.ok(getService.loadEstatesNearBy(longitude, latitude, radius)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -93,6 +95,16 @@ public ApiResponse<List<EstateListResponse>> getEstateByLocationByKaptCode( | |||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| @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)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// AI 기반 특징 요약 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| @GetMapping("/ai/{kaptCode}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -101,4 +113,25 @@ public ApiResponse<String> getEstateDetail(@PathVariable String kaptCode) { | |||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ApiResponse.ok(result); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// 가격 조회 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| @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)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+119
to
+127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| @GetMapping("/price/{kaptCode}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| public ApiResponse<List<EstatePriceListResponse>> getPrice(@PathVariable String kaptCode) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<EstatePrice> list = priceService.getEstatePriceByCode(kaptCode); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ApiResponse.ok(EstatePriceListResponse.from(list)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+130
to
+136
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package com.zipte.platform.server.adapter.in.web.dto.response; | ||
|
|
||
| import com.zipte.platform.server.domain.estate.EstatePrice; | ||
| import lombok.Builder; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| @Builder | ||
| public record EstatePriceDetailResponse( | ||
| double exclusiveArea, String price, String transactionDate) { | ||
|
|
||
| public static EstatePriceDetailResponse from(Optional<EstatePrice> estatePrice) { | ||
| if (estatePrice.isEmpty()) { | ||
| return EstatePriceDetailResponse.builder() | ||
| .exclusiveArea(0.0) | ||
| .price("없음") | ||
| .transactionDate("없음") | ||
| .build(); | ||
| } | ||
|
|
||
| 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(); | ||
| } | ||
|
Comment on lines
+21
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 중복된 null 체크 로직 line 23-25에서 각 필드마다 null 체크를 하고 있는데, 이미 Optional이 존재하는지 확인했으므로 일부 중복된 체크가 있을 수 있습니다. 또한 다음과 같이 간소화할 수 있습니다: 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 |
||
|
|
||
| } | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -28,4 +28,7 @@ public interface EstateMongoRepository extends MongoRepository<EstateDocument, S | |||||||
| List<EstateDocument> findByLocation(double longitude, double latitude, double radiusInRadians); | ||||||||
|
|
||||||||
| boolean existsByKaptCode(String kaptCode); | ||||||||
|
|
||||||||
| /// 테스트용 | ||||||||
| void deleteByKaptCode(String kaptCode); | ||||||||
|
Comment on lines
+31
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 테스트 전용 메서드의 위치를 재검토해주세요. 프로덕션 레포지토리 인터페이스에 테스트 전용 메서드를 추가하는 것은 좋은 관행이 아닙니다. 다음 대안을 고려해보세요:
테스트 전용 레포지토리 인터페이스 생성 예시: // 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
Suggested change
🤖 Prompt for AI Agents |
||||||||
| } | ||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
아파트 비교 기능 구현
비교 기능이 적절히 구현되었습니다. 입력 파라미터 검증을 추가하는 것을 고려해보세요.
@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
🤖 Prompt for AI Agents