[REFACTOR] Google Place 호출 구조 최적화 및 배치 전용 모듈 도입#146
Conversation
Walkthrough장소 검색을 상세정보 기반에서 텍스트검색 기반 워크플로우로 전환하고, 새로운 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
🧪 테스트 결과147 tests 147 ✅ 31s ⏱️ Results for commit f998d90. ♻️ This comment has been updated with latest results. |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/client/GooglePlacesClient.kt (1)
210-221: currentOpeningHours가 Field Mask에서 누락됨
buildTextSearchFieldMask()에서places.currentOpeningHours가 제거되었지만,PlaceQuery.savePlacesFromTextSearch()에서는 여전히place.currentOpeningHours?.openNow를 사용하고 있습니다 (Line 62). 이로 인해openNow값이 항상null이 됩니다.🐛 Field Mask에 currentOpeningHours 추가
private fun buildTextSearchFieldMask(): String { return listOf( "places.id", "places.displayName", "places.formattedAddress", "places.rating", "places.userRatingCount", "places.photos", "places.location", - "places.types" + "places.types", + "places.currentOpeningHours" ).joinToString(",") }
🤖 Fix all issues with AI agents
In @ssolv-api-place/build.gradle.kts:
- Line 7: ssolv-api-place currently declares
implementation(project(":ssolv-batch")), which reverses the typical dependency
direction; confirm whether ssolv-api-place should depend on ssov-batch or
whether ssov-batch should depend on ssolv-api-place instead. If the batch module
should consume services/repositories from the API module, remove
implementation(project(":ssolv-batch")) from the ssov-api-place build.gradle.kts
and add implementation(project(":ssolv-api-place")) to the ssov-batch module,
then run a full build to verify no cycles and update module documentation to
reflect the intended architecture.
In
@ssolv-api-place/src/main/kotlin/org/depromeet/team3/place/application/execution/ExecutePlaceSearchService.kt:
- Around line 109-151: The mapping in ExecutePlaceSearchService's
savedEntities.mapNotNull assumes entity.address is non-null and calls
entity.address.replace(...), which can throw an NPE; update the mapping to
null-safely handle address (e.g., use entity.address?.replace("대한민국 ", "") ?: ""
or a sensible fallback) before passing into PlacesSearchResponse.PlaceItem, and
similarly ensure other fields used without safe calls (like converting
ratings/text) are guarded or defaulted; locate this logic inside the lambda that
builds PlacesSearchResponse.PlaceItem in ExecutePlaceSearchService and apply the
null-safe transformations/fallbacks.
In
@ssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/model/PlacesTextSearchResponse.kt:
- Around line 21-28: The PlacesTextSearchResponse data model declares the
location property as non-nullable but Google Places Text Search can omit
location; change the declaration of location in PlacesTextSearchResponse to be
nullable (location: Location?) and update any constructors/usages and JSON
(de)serialization handling to accept nulls; ensure the nested data class
Location remains unchanged but any code that dereferences
PlacesTextSearchResponse.location (e.g., mapping, distance calculations or
serialization code) is null-checked or guarded to avoid NPEs.
- Around line 37-41: AuthorAttribution's fields are non-nullable but the Google
Places API can omit them; change the three properties in the data class
AuthorAttribution (displayName, uri, photoUri) from String to nullable String?
so the JSON deserializer won't fail when those keys are missing, and consider
leaving them without defaults or adding explicit null defaults if you prefer
(e.g., displayName: String?).
🧹 Nitpick comments (10)
.gitignore (1)
68-68: 배치 모듈 아티팩트 무시 패턴 추가 - 승인됨새로운
*/bin패턴이 배치 모듈 및 기타 모듈에서 생성되는 bin 디렉토리를 올바르게 제외합니다. PR 목표와 완벽하게 부합합니다.선택 사항: 파일 내 다른 디렉토리 패턴들(예:
build/,out/)과의 스타일 일관성을 위해 후행 슬래시를 추가하는 것을 고려해보세요 (*/bin/). 두 형식 모두 동작하지만, 디렉토리임을 더 명확하게 나타냅니다.ssolv-batch/src/main/resources/application.yml (3)
5-9: 배치 작업을 위한 커넥션 풀 설정을 추가하세요.배치 모듈은 스케줄된 작업을 수행하며 장시간 실행될 수 있습니다. 현재 커넥션 풀 관련 설정이 없어 커넥션 고갈이나 타임아웃 문제가 발생할 수 있습니다. HikariCP 설정을 추가하여 안정적인 데이터베이스 연결 관리를 권장합니다.
♻️ 권장 커넥션 풀 설정 추가
datasource: url: jdbc:mysql://localhost:3306/${DEFAULT_SCHEMA} username: ${DB_USERNAME} password: ${DB_PASSWORD} driver-class-name: com.mysql.cj.jdbc.Driver + hikari: + maximum-pool-size: 10 + connection-timeout: 30000 + idle-timeout: 600000 + max-lifetime: 1800000
11-14: 배치 처리 최적화를 위한 JPA 설정 개선을 고려하세요.현재 설정에 몇 가지 개선 사항을 제안합니다:
show-sql: true는 배치 작업 중 대량의 SQL 로그를 생성할 수 있어 성능과 로그 관리에 영향을 줄 수 있습니다.- 배치 처리에서 일반적인 대량 작업(bulk insert/update)을 위한
batch-size설정이 누락되어 있습니다.♻️ 배치 최적화 설정 제안
jpa: hibernate: ddl-auto: none - show-sql: true + show-sql: false + properties: + hibernate: + jdbc: + batch_size: 50 + order_inserts: true + order_updates: true
16-17: 환경별 설정 관리를 위해 Spring Profile 활용을 권장합니다.
app.env값이 "local"로 하드코딩되어 있어 다른 환경(dev, staging, production)에 배포 시 혼란을 야기할 수 있습니다. Spring Profile을 활용하거나 환경 변수로 외부화하여 환경별 설정을 명확히 관리하는 것을 권장합니다.♻️ 환경 설정 개선 방안
방안 1: 환경 변수로 외부화
app: - env: local + env: ${APP_ENV:local}방안 2: Spring Profile 활용
현재 파일을
application.yml로 유지하고, 환경별 파일 추가:
application-local.ymlapplication-dev.ymlapplication-prod.yml그리고 실행 시
-Dspring.profiles.active={profile}옵션 사용ssolv-api-place/src/main/kotlin/org/depromeet/team3/place/controller/PlacesSearchController.kt (1)
48-51: OpenAPI 설명 텍스트가 불완전해 보입니다.라인 50의
description값이 "반환합니다. "로 끝나며 의미 있는 내용이 없습니다. PR 요약에서 서버 측 이미지 프록시 로직이 유지된다고 했으므로, 설명을 더 구체적으로 보완하는 것을 권장합니다. 예를 들어:
- "구글 플레이스 사진을 서버 프록시를 통해 반환합니다."
- "구글 플레이스 사진을 프록시하여 반환합니다. 응답은 30일 동안 캐시됩니다."
ssolv-infrastructure/src/test/kotlin/org/depromeet/team3/place/client/GooglePlacesClientTest.kt (1)
33-66: 테스트 커버리지 보완 권장텍스트 검색 성공 시나리오만 테스트되고 있습니다. 다음 케이스들에 대한 추가 테스트를 고려해 주세요:
- 빈 결과 반환 시 (
places = emptyList())places = null반환 시- 위치 바이어스(
locationBias) 파라미터 사용 시ssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/PlaceQuery.kt (1)
48-51: 30일 캐시 검증 로직 개선 권장
LocalDateTime.now()를 반복 호출하면 각 place마다 미세하게 다른 시간이 사용될 수 있습니다. 일관성을 위해 메서드 시작 시점에 한 번만 호출하는 것이 좋습니다.♻️ 개선 제안
): List<PlaceEntity> = withContext(Dispatchers.IO) { if (places.isEmpty()) return@withContext emptyList() + + val now = LocalDateTime.now() + val cacheThreshold = now.minusDays(30) val googlePlaceIds = places.map { it.id } val existingPlaces = placeJpaRepository.findByGooglePlaceIdIn(googlePlaceIds) .associateBy { it.googlePlaceId } val entities = places.map { place -> val existing = existingPlaces[place.id] val lastUpdated = existing?.updatedAt ?: LocalDateTime.MIN // 30일 이내 데이터가 있으면 업데이트 스킵 - if (existing != null && lastUpdated.isAfter(LocalDateTime.now().minusDays(30))) { + if (existing != null && lastUpdated.isAfter(cacheThreshold)) { return@map existing }ssolv-api-place/src/main/kotlin/org/depromeet/team3/place/application/execution/ExecutePlaceSearchService.kt (1)
499-499: 에러 메시지의 상세 정보 개선에러 상황에서
detail맵에error를 포함시키고 있습니다. 일관성을 위해 다른 예외 처리 부분(Line 479)과 동일한 키 구조를 사용하는 것이 좋습니다.에러 detail 키가 일관되게 사용되고 있는지 확인하고, 필요시 통일하는 것을 고려하세요.
ssolv-batch/src/main/kotlin/org/depromeet/team3/batch/scheduler/PlaceDataCleanupScheduler.kt (1)
22-33:@Transactional과@Scheduled조합 시 주의 필요
@Scheduled메서드에서@Transactional을 직접 사용하면 프록시 우회 문제가 발생할 수 있습니다. Spring의@Scheduled가 같은 빈 내부에서 메서드를 호출하면 프록시를 거치지 않아 트랜잭션이 적용되지 않을 수 있습니다.또한,
try-catch블록이@Transactional메서드 내부에 있어서 예외 발생 시 트랜잭션 롤백이 이루어지지 않습니다.deleteByUpdatedAtBefore가 부분 삭제 후 실패하면 일부 데이터만 삭제된 상태로 커밋될 수 있습니다.♻️ 권장 수정안
@Component class PlaceDataCleanupScheduler( private val placeJpaRepository: PlaceJpaRepository ) { private val logger = LoggerFactory.getLogger(PlaceDataCleanupScheduler::class.java) @Scheduled(cron = "0 0 3 * * *") - @Transactional fun cleanupStalePlaceData() { val thirtyDaysAgo = LocalDateTime.now().minusDays(30) try { - val deletedCount = placeJpaRepository.deleteByUpdatedAtBefore(thirtyDaysAgo) + val deletedCount = doCleanup(thirtyDaysAgo) logger.info("30일 경과 장소 데이터 삭제 완료: {}건", deletedCount) } catch (e: Exception) { logger.error("장소 데이터 정리 중 오류 발생", e) } } + + @Transactional + fun doCleanup(cutoffDate: LocalDateTime): Int { + return placeJpaRepository.deleteByUpdatedAtBefore(cutoffDate) + } }ssolv-api-place/src/test/kotlin/org/depromeet/team3/place/application/execution/ExecutePlaceSearchServiceTest.kt (1)
3-5: 사용되지 않는 import 확인 필요
Dispatchers와withContext가 import되었지만 이 테스트 파일에서 직접 사용되지 않는 것으로 보입니다. 실제로 사용되지 않는다면 제거를 고려하세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
.gitignoresettings.gradle.ktsssolv-api-core/src/main/kotlin/org/depromeet/team3/CoreApiApplication.ktssolv-api-place/build.gradle.ktsssolv-api-place/src/main/kotlin/org/depromeet/team3/place/application/execution/ExecutePlaceSearchService.ktssolv-api-place/src/main/kotlin/org/depromeet/team3/place/controller/PlacesSearchController.ktssolv-api-place/src/test/kotlin/org/depromeet/team3/place/application/execution/ExecutePlaceSearchServiceTest.ktssolv-api-place/src/test/kotlin/org/depromeet/team3/place/util/PlaceTestDataFactory.ktssolv-batch/build.gradle.ktsssolv-batch/src/main/kotlin/org/depromeet/team3/BatchApplication.ktssolv-batch/src/main/kotlin/org/depromeet/team3/batch/scheduler/MeetingPlaceSearchCleanupScheduler.ktssolv-batch/src/main/kotlin/org/depromeet/team3/batch/scheduler/PlaceDataCleanupScheduler.ktssolv-batch/src/main/resources/application.ymlssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/PlaceJpaRepository.ktssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/PlaceQuery.ktssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/client/GooglePlacesClient.ktssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/model/PlacesTextSearchResponse.ktssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/util/PlaceAddressResolver.ktssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/util/PlaceDetailsProcessor.ktssolv-infrastructure/src/test/kotlin/org/depromeet/team3/place/PlaceQueryRetryTest.ktssolv-infrastructure/src/test/kotlin/org/depromeet/team3/place/client/GooglePlacesClientRetryTest.ktssolv-infrastructure/src/test/kotlin/org/depromeet/team3/place/client/GooglePlacesClientTest.kt
💤 Files with no reviewable changes (4)
- ssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/util/PlaceDetailsProcessor.kt
- ssolv-infrastructure/src/test/kotlin/org/depromeet/team3/place/PlaceQueryRetryTest.kt
- ssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/util/PlaceAddressResolver.kt
- ssolv-api-core/src/main/kotlin/org/depromeet/team3/CoreApiApplication.kt
⏰ 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: test
🔇 Additional comments (23)
ssolv-batch/src/main/resources/application.yml (1)
1-3: 애플리케이션 이름 설정이 적절합니다.배치 모듈명과 일치하며 명확하게 구성되어 있습니다.
ssolv-api-place/src/main/kotlin/org/depromeet/team3/place/controller/PlacesSearchController.kt (2)
59-62: 캐싱 전략이 PR 요구사항과 일치합니다.30일 최대 캐시 설정이 PR 요약의 "DB에 저장된 장소 정보는 30일 유효기간 적용" 정책과 일관성 있게 구현되었습니다.
cachePublic()설정도 적절합니다.
53-57: PlacePhotoService를 통한 프록시 구현이 올바른 구조입니다.
photoName경로 매개변수는@GetMapping("/photos/{photoName:.+}")정규식으로 Google Places 리소스 이름의 전체 경로(places/ChIJ.../photos/AbCdEf형식)를 정확히 캡처하며, PlacePhotoService에서 "places/" 접두사 검증과 함께 올바르게 처리됩니다. ExecutePlaceSearchService에서 PlaceFormatter를 통해 생성된 프록시 URL은{proxyBaseUrl}/v1/places/photos/{photoName}형식으로, 컨트롤러가 추출한 photoName이 원본 Google Places 리소스 이름을 그대로 포함하므로 매핑이 정상 동작합니다. 서버 측 이미지 프록시 로직 유지 요구사항이 잘 지켜지고 있습니다.ssolv-batch/src/main/kotlin/org/depromeet/team3/batch/scheduler/MeetingPlaceSearchCleanupScheduler.kt (1)
22-33: LGTM!스케줄러 로직이 적절하게 구현되어 있습니다. 트랜잭션 처리와 예외 처리가 잘 되어 있습니다.
ssolv-batch/src/main/kotlin/org/depromeet/team3/BatchApplication.kt (1)
1-13: LGTM!배치 모듈의 진입점이 적절하게 구성되어 있습니다.
@EnableScheduling이 API 모듈에서 배치 모듈로 이동한 것이 명확합니다.ssolv-infrastructure/src/test/kotlin/org/depromeet/team3/place/client/GooglePlacesClientRetryTest.kt (2)
37-60: LGTM!500 에러 후 재시도 성공 시나리오가 올바르게 테스트되고 있습니다. POST 기반 텍스트 검색 플로우에 맞게 mock이 적절히 구성되었습니다.
62-85: LGTM!최대 재시도 횟수 초과 시나리오가 올바르게 테스트되고 있습니다.
times(3)검증이maxRetries = 3설정과 일치합니다.ssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/client/GooglePlacesClient.kt (1)
182-205: fetchPhoto 예외 처리 시 null 반환 확인
fetchPhoto에서 모든 예외를 catch하고null을 반환합니다. 이 동작이 의도된 것인지 확인해 주세요. 재시도 로직 내부에서 예외가 발생해도null이 반환되어 재시도가 무의미해질 수 있습니다.ssolv-api-place/src/test/kotlin/org/depromeet/team3/place/util/PlaceTestDataFactory.kt (1)
22-35: 테스트 데이터 확장 승인
PlacesTextSearchResponse.Place의 새로운 구조에 맞춰location과photos필드가 테스트 데이터에 추가되었습니다. 이는 Text Search API 응답 모델 변경사항과 일치합니다.테스트 데이터가 실제 API 응답 구조를 정확히 반영하고 있어 좋습니다.
settings.gradle.kts (1)
10-10:ssolv-batch모듈이 올바르게 추가되었습니다모듈 디렉토리,
build.gradle.kts, 그리고src디렉토리가 모두 존재하며settings.gradle.kts에 올바르게 등록되었습니다. PR 목표와 일치합니다.ssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/PlaceJpaRepository.kt (2)
9-10: findByGooglePlaceId 메서드 제거 마이그레이션 완료 확인됨단일 장소 조회 메서드
findByGooglePlaceId가 제거되고findByGooglePlaceIdIn(배치 조회)로 완전히 마이그레이션되었습니다. 제거된 메서드의 모든 사용처가 새로운 메서드로 전환되어 있으므로 추가 조치가 필요하지 않습니다.
5-5: 배치 정리 메서드 구현 확인 완료
deleteByUpdatedAtBefore메서드가 올바르게 구현되었습니다:
- ✓ 배치 스케줄러(
PlaceDataCleanupScheduler)에서 정확하게 호출됨 (매일 새벽 3시)- ✓
@Transactional어노테이션으로 트랜잭션 안전성 보장- ✓ 예외 처리 및 로깅으로 안정적인 운영 준비 완료
- ✓ 기존
findByGooglePlaceId메서드 제거 및findByGooglePlaceIdIn으로 완전히 마이그레이션됨PR의 "30일 유효기간 적용" 목표가 모두 충족되었습니다.
ssolv-api-place/src/main/kotlin/org/depromeet/team3/place/application/execution/ExecutePlaceSearchService.kt (5)
12-12: GooglePlacesApiProperties 의존성 추가가 올바르게 구성됨
GooglePlacesApiProperties는apiKey,baseUrl,proxyBaseUrl속성을 가진 데이터 클래스로 정의되어 있으며,proxyBaseUrl은 기본값"https://api.ssolv.site"를 갖습니다. 이 속성은PlaceFormatter.generatePhotoUrl()에서 사진 URL 생성 시 올바르게 사용되고 있습니다.PlaceDetailsProcessor제거와 함께 이 의존성 추가는 Text Search 단일 호출 방식으로의 전환에 적절합니다.
141-141: Text Search API에서 priceRange를 null로 설정하는 것은 올바른 구현입니다.
PlaceItem에서priceRange는 이미 nullable(PriceRange?)로 정의되어 있으며, Text Search API가 가격대 정보를 제공하지 않기 때문에 null로 설정하는 것이 적절합니다. 테스트 코드에서도 동일하게priceRange = null로 처리하고 있어, 클라이언트는 이를 선택적 필드로 인식합니다.
76-88: savePlacesFromTextSearch의 트랜잭션 안전성 및 중복 처리 확인됨
savePlacesFromTextSearch메서드는@Transactional어노테이션으로 보호되어 있으며, 트랜잭션 안전성이 확보되어 있습니다. 중복 저장 처리도 적절히 구현되어 있습니다:
- 기존 장소를 googlePlaceId로 조회하여 맵에 저장 (O(1) 조회)
- 30일 이내 업데이트된 데이터는 업데이트 스킵 (비용 최적화)
- 새로운 또는 오래된 데이터만 저장/업데이트
saveAll로 일괄 처리하여 데이터베이스 레벨에서 충돌 처리
122-128: 사진 프록시 URL 생성 로직은 올바르게 구현되어 있습니다.
PlaceFormatter.generatePhotoUrl이 정확한 프록시 URL 형식({proxyBaseUrl}/v1/places/photos/{photoName})을 생성하고 있으며, 프록시 엔드포인트(/photos/{photoName:.+})가 PlacesSearchController에 구현되어 있습니다. PlacePhotoService에서 photoName이 "places/"로 시작하는지 검증하고, GooglePlacesClient는 재시도 로직과 타임아웃을 포함한 안정적인 사진 조회를 수행합니다. 응답에 30일 public 캐시 제어가 설정되어 있어 이미지 노출 방지 및 성능 측면에서도 적절합니다.
47-47:keywordFetchSize증가에 대한 비용 영향 확인 완료 - 문제 없음Google Places API Text Search 가격 정책 검증 결과,
keywordFetchSize를 5에서 20으로 증가시키는 것은 비용에 영향을 주지 않습니다. API 청구는 결과 개수가 아닌 요청 단위로 이루어지므로, 5개 결과를 요청하든 20개 결과를 요청하든 동일한 요청 요금이 적용됩니다. 코드 주석의 "최대 20개까지 요금 동일"이라는 설명이 정확합니다.ssolv-batch/src/main/kotlin/org/depromeet/team3/batch/scheduler/PlaceDataCleanupScheduler.kt (1)
10-20: LGTM!Google 약관 준수를 위한 스케줄러 도입이 적절합니다. 문서화가 명확하고, 30일 데이터 보존 정책이 PR 목표에 부합합니다.
ssolv-batch/build.gradle.kts (2)
1-36: LGTM!배치 모듈의 Gradle 구성이 적절합니다. Jib을 통한 컨테이너화, JVM 플래그 설정, 의존성 구성이 잘 되어 있습니다.
25-28: 프로젝트 버전 설정 확인 완료
project.version은 루트build.gradle.kts에서"0.0.1-SNAPSHOT"으로 올바르게 설정되어 있습니다. ssolv-batch 모듈은 부모 빌드 설정으로부터 버전을 상속받으므로 Docker 이미지 태그는"latest"와"0.0.1-SNAPSHOT"으로 올바르게 생성됩니다.Likely an incorrect or invalid review comment.
ssolv-api-place/src/test/kotlin/org/depromeet/team3/place/application/execution/ExecutePlaceSearchServiceTest.kt (3)
70-125: LGTM!저장된 결과 반환 시나리오 테스트가 잘 구성되어 있습니다. 테스트 데이터와 assertion이 명확합니다.
127-178: LGTM!새로운 텍스트 검색 기반 플로우를 잘 테스트하고 있습니다.
savePlacesFromTextSearch경로를 통한 데이터 저장 및 반환 시나리오가 적절히 검증됩니다.
181-221: LGTM!
FakePlaceQuery구현이 잘 되어 있습니다. 테스트 더블 패턴을 적절히 활용하고, stub 메서드들이 유연하게 설계되어 있어 다양한 테스트 시나리오를 지원합니다.
...src/main/kotlin/org/depromeet/team3/place/application/execution/ExecutePlaceSearchService.kt
Show resolved
Hide resolved
...v-infrastructure/src/main/kotlin/org/depromeet/team3/place/model/PlacesTextSearchResponse.kt
Outdated
Show resolved
Hide resolved
...v-infrastructure/src/main/kotlin/org/depromeet/team3/place/model/PlacesTextSearchResponse.kt
Show resolved
Hide resolved
ssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/PlaceQuery.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @ssolv-batch/build.gradle.kts:
- Around line 1-3: The Jib plugin is declared but lacks configuration; add a jib
{ ... } block in build.gradle.kts configuring at minimum jib.from.image (base
image), jib.to.image (target image name/tag), and jib.container settings
(entrypoint or mainClass, jvmFlags, ports, and appRoot) so the batch module can
build a runnable container; reference the jib block and keys jib.from.image,
jib.to.image, and jib.container.mainClass/jib.container.entrypoint to locate
where to add these settings and ensure the image name uniquely identifies the
batch module (e.g., project name and version).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ssolv-batch/build.gradle.kts
⏰ 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: test
🔇 Additional comments (2)
ssolv-batch/build.gradle.kts (2)
5-10: 의존성 구조가 올바르게 설정되어 있습니다.ssolv-api-place가 ssolv-batch에 의존하고 있으며, 이는 올바른 구조입니다. ssolv-batch는 ssolv-api-place에 의존할 필요가 없습니다. 현재 빌드 파일의 의존성 구조는 PR 목표와 일치합니다.
12-20: 이 모듈의 jar/bootJar 설정은 현재 아키텍처에 맞게 올바르게 구성되어 있습니다.ssolv-batch는 라이브러리 모듈이며, ssolv-api-place에 의존성으로 포함되어 실행됩니다. 따라서 jar 활성화, bootJar 비활성화는 이 모듈의 성격에 적합합니다:
- ssolv-batch는 @SpringBootApplication 없이 @scheduled @component만 포함
- ssolv-api-place가 PlaceApiApplication을 통해 실행 가능한 jar 생성
- 배치 작업(MeetingPlaceSearchCleanupScheduler 등)은 ssolv-api-place의 Spring 컨텍스트 내에서 실행
"물리적으로 분리하여 배포"는 ssolv-api-core와 ssolv-api-place를 별도 컨테이너로 배포하는 것으로 달성되며, 현재 설정이 그 목표를 충족합니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@ssolv-api-place/src/main/kotlin/org/depromeet/team3/place/application/execution/ExecutePlaceSearchService.kt:
- Line 120: The address normalization in ExecutePlaceSearchService (assignment
to address) currently uses replace("대한민국 ", "") which only removes "대한민국"
followed by a space; update it to remove a leading "대한민국" with any following
whitespace (or the exact word alone) using a regex anchored to start, e.g.,
replace(Regex("^대한민국\\s*"), "") or trim after conditional removal, so an address
equal to "대한민국" becomes an empty string; locate the address = entity.address?
line and replace the literal-space replace with the regex-based removal and
maintain the existing null-coalescing behavior.
🧹 Nitpick comments (4)
ssolv-infrastructure/src/test/kotlin/org/depromeet/team3/place/client/GooglePlacesClientRetryTest.kt (1)
35-58: 재시도 로직이 올바르게 테스트되고 있습니다.500 에러 발생 후 재시도하여 성공하는 시나리오를 정확하게 검증합니다. Mock 체이닝과 검증 횟수(2회)가 재시도 로직과 일치합니다.
💡 선택적 개선: 테스트 데이터 강화
현재는
emptyList()를 사용하고 있지만, 실제 장소 데이터를 포함한 응답을 사용하면 통합 테스트에 더 가까워질 수 있습니다:-val mockResponse = PlacesTextSearchResponse(emptyList()) +val mockResponse = PlacesTextSearchResponse( + places = listOf( + PlacesTextSearchResponse.Place( + id = "test-place-id", + displayName = PlacesTextSearchResponse.DisplayName(text = "테스트 맛집", languageCode = "ko") + ) + ) +)다만 현재의 재시도 로직 검증 목적에는 빈 리스트로도 충분합니다.
ssolv-infrastructure/src/test/kotlin/org/depromeet/team3/place/client/GooglePlacesClientTest.kt (1)
33-65: 테스트 커버리지 보완 권장현재 성공 케이스만 테스트하고 있습니다. 다음 시나리오 추가를 고려해보세요:
- API 호출 실패 (예외 처리)
- 빈 결과 반환
- 필수 필드 누락 처리
테스트 신뢰성을 높이고 엣지 케이스에 대한 방어 로직을 검증할 수 있습니다.
ssolv-api-place/src/main/kotlin/org/depromeet/team3/place/application/execution/ExecutePlaceSearchService.kt (1)
124-129: 사진 이름 공백 처리 추가 권장
split(",")로 분리한 사진 이름에 공백이 포함될 수 있습니다.PlaceFormatter.generatePhotoUrl이 내부적으로 처리하지 않는다면 URL 생성에 실패할 수 있습니다.안전하게 처리하려면 다음과 같이 trim을 추가하는 것을 고려해보세요:
♻️ 제안하는 수정
-photos = entity.photos?.split(",")?.map { photoName -> +photos = entity.photos?.split(",")?.map { photoName -> + val trimmedName = photoName.trim() // PlaceFormatter를 사용하여 프록시 URL 생성 org.depromeet.team3.place.util.PlaceFormatter.generatePhotoUrl( - photoName, + trimmedName, googlePlacesApiProperties.proxyBaseUrl ) },ssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/PlaceQuery.kt (1)
44-51: 30일 체크 기준 시점을 반복문 밖으로 이동 권장현재
LocalDateTime.now().minusDays(30)이 각 place마다 map 내부에서 호출됩니다. 배치 처리 중 시간이 변경될 수 있어 일관성이 떨어지고, 불필요한 반복 계산이 발생합니다.♻️ 제안하는 수정
suspend fun savePlacesFromTextSearch( places: List<PlacesTextSearchResponse.Place> ): List<PlaceEntity> { if (places.isEmpty()) return emptyList() val googlePlaceIds = places.map { it.id } val existingPlaces = placeJpaRepository.findByGooglePlaceIdIn(googlePlaceIds) .associateBy { it.googlePlaceId } + + val cacheExpirationTime = LocalDateTime.now().minusDays(30) val entities = places.map { place -> val existing = existingPlaces[place.id] val lastUpdated = existing?.updatedAt ?: LocalDateTime.MIN // 30일 이내 데이터가 있으면 업데이트 스킵 (비용 최적화 및 구글 약관 준수) - if (existing != null && lastUpdated.isAfter(LocalDateTime.now().minusDays(30))) { + if (existing != null && lastUpdated.isAfter(cacheExpirationTime)) { return@map existing }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
ssolv-api-place/src/main/kotlin/org/depromeet/team3/place/application/execution/ExecutePlaceSearchService.ktssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/PlaceQuery.ktssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/model/PlacesTextSearchResponse.ktssolv-infrastructure/src/test/kotlin/org/depromeet/team3/place/client/GooglePlacesClientRetryTest.ktssolv-infrastructure/src/test/kotlin/org/depromeet/team3/place/client/GooglePlacesClientTest.kt
⏰ 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: test
🔇 Additional comments (5)
ssolv-infrastructure/src/test/kotlin/org/depromeet/team3/place/client/GooglePlacesClientRetryTest.kt (3)
1-33: 테스트 설정이 올바르게 구성되어 있습니다.Text Search API로의 전환에 맞춰 import와 mock 설정이 적절하게 변경되었습니다. RestClient mock과 GooglePlacesApiProperties 설정이 테스트 목적에 부합합니다.
60-82: 최대 재시도 횟수 초과 시나리오가 정확하게 검증됩니다.3회 호출(초기 시도 + 2회 재시도) 후
PlaceSearchException이 발생하는 것을 올바르게 검증하고 있습니다. 재시도 정책이 최대 2회임을 명확히 보여줍니다.
84-92: 헬퍼 메서드가 적절하게 구현되었습니다.
HttpClientErrorException.create()팩토리 메서드를 사용하여 다양한 HTTP 상태 코드에 대한 예외를 생성할 수 있도록 범용적으로 설계되었습니다. 이전의 PlaceDetailsResponse 전용 빌더 로직을 제거하고 더 간결한 접근 방식을 채택한 것은 좋은 개선입니다.ssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/PlaceQuery.kt (1)
34-37: @transactional과 suspend 함수 조합은 올바르게 작동합니다Spring Boot 3.4.9는 Spring Framework 6.1+의 코루틴 트랜잭션 지원을 포함하며, 프로젝트에 kotlinx-coroutines-reactor 의존성이 이미 구성되어 있어 트랜잭션 컨텍스트가 올바르게 전파됩니다. 이 패턴은 PlaceLikeQuery, MeetingPlaceQuery 등 코드베이스 전반에서 일관되게 사용되고 있으므로 추가 조치가 필요하지 않습니다.
ssolv-api-place/src/main/kotlin/org/depromeet/team3/place/application/execution/ExecutePlaceSearchService.kt (1)
518-537: 제약 조건 존재 확인 - 예외 처리 필요
MeetingPlaceEntity에(meeting_id, place_id)조합에 대한 unique constraintuk_meeting_place가 이미 정의되어 있어 데이터베이스 수준에서 중복 저장을 방지합니다.다만 현재 로직은 조회 후 저장하는 비트랜잭션 방식으로 인해 동시 요청 시 두 스레드가 같은 조합을 삽입하려 할 때
DataIntegrityViolationException이 발생할 수 있습니다. 라인 532의saveAll호출이 예외 처리 없이 진행되므로, 이 경우 에러가 호출자까지 전파됩니다.
@Transactional추가 또는saveAll주변에 예외 처리를 추가하여 중복 삽입 시도에 대한 안전한 처리를 고려하세요.
| rating = review.rating, | ||
| text = review.text | ||
| name = entity.name ?: "", | ||
| address = entity.address?.replace("대한민국 ", "") ?: "", |
There was a problem hiding this comment.
주소 문자열 처리 엣지 케이스 확인 필요
replace("대한민국 ", "") 로직은 trailing 공백이 있는 경우만 처리합니다. 만약 주소가 정확히 "대한민국" (공백 없음)인 경우 빈 문자열로 처리되지 않고 그대로 유지됩니다.
Google Places API 응답에서 이런 케이스가 발생하는지 확인하고, 필요하다면 replace(Regex("^대한민국\\s*"), "") 같은 패턴 사용을 고려해보세요.
🤖 Prompt for AI Agents
In
@ssolv-api-place/src/main/kotlin/org/depromeet/team3/place/application/execution/ExecutePlaceSearchService.kt
at line 120, The address normalization in ExecutePlaceSearchService (assignment
to address) currently uses replace("대한민국 ", "") which only removes "대한민국"
followed by a space; update it to remove a leading "대한민국" with any following
whitespace (or the exact word alone) using a regex anchored to start, e.g.,
replace(Regex("^대한민국\\s*"), "") or trim after conditional removal, so an address
equal to "대한민국" becomes an empty string; locate the address = entity.address?
line and replace the literal-space replace with the regex-based removal and
maintain the existing null-coalescing behavior.
ssolv-infrastructure/src/main/kotlin/org/depromeet/team3/place/PlaceQuery.kt
Show resolved
Hide resolved
1 participant
🎋 이슈 및 작업중인 브랜치
🔑 주요 내용
@nitronium102
Google Places API 호출 최적화
배치(Batch) 전용 모듈 신설(ssolv-api-place 모듈 의존)
캐싱 및 구글 약관(ToS) 준수
Check List
Summary by CodeRabbit
New Features
Refactor
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.