-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor/#33 장소 검색 기능 개선 #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…DB 쿼리를 한번만 요청하는 방식으로 수정
Walkthrough검색 후보 DTO 추가, PlaceService의 이미지 조회를 일괄화·비동기화하고 Google 호출을 세마포어로 제한하며 실행 시간 로깅용 애너테이션/AOP와 가상 스레드 Executor 빈을 추가했습니다. 테스트용 컨트롤러 및 응답 코드가 삭제되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PlaceService
participant GooglePlaceClient
participant PlaceImagesRepo as PlaceImagesRepository
participant Executor as ExecutorService
participant Future as CompletableFuture
Client->>PlaceService: search(keyword)
activate PlaceService
PlaceService->>GooglePlaceClient: searchPlaces(keyword)
activate GooglePlaceClient
GooglePlaceClient-->>PlaceService: List[SearchCandidateResponse]
deactivate GooglePlaceClient
rect rgba(240,248,255,0.9)
Note over PlaceService,PlaceImagesRepo: 배치 이미지 조회 (placeKeys)
PlaceService->>PlaceImagesRepo: findAllByPlaceKeyIn(placeKeys)
activate PlaceImagesRepo
PlaceImagesRepo-->>PlaceService: Map<placeKey, List<Image>>
deactivate PlaceImagesRepo
end
rect rgba(240,255,240,0.9)
Note over PlaceService,Executor: 병렬 변환 (CompletableFuture, 타임아웃/폴백)
loop 후보별
PlaceService->>Executor: submit(convertTask)
Executor->>Future: 실행 (convertToSavedPlaceInfo(imagesMap))
Future-->>Executor: SavedPlaceInfo 또는 fallback
end
end
PlaceService->>Future: joinAll() with timeout
Future-->>PlaceService: List[SavedPlaceInfo]
PlaceService-->>Client: List[SavedPlaceInfo]
deactivate PlaceService
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In
@src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java:
- Line 52: Fix the spelling typo in the inline comment inside GooglePlaceClient
(the comment "placee details -> photo reference"); change "placee" to "place" so
the comment reads "place details -> photo reference" to improve clarity in
GooglePlaceClient.java.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java (1)
88-96: 빈 결과 리스트에서 IndexOutOfBoundsException 발생 가능
response.results()가 비어있을 경우get(0)호출 시 예외가 발생합니다. null 체크 후 리스트가 비어있는지도 확인해야 합니다.🔎 안전한 결과 접근
if (response == null) { return null; } + if (response.results() == null || response.results().isEmpty()) { + return null; + } + String placeId = response.results().get(0).placeId(); if (placeId == null || placeId.isBlank()) { return null; } return placeId;
🧹 Nitpick comments (4)
src/main/java/com/campus/campus/global/annotation/stopwatch/ExecutionTimeAspect.java (1)
14-29: LGTM!AOP 기반 실행 시간 측정이 올바르게 구현되었습니다.
StopWatch를 사용한 시간 측정과toShortString()을 통한 간결한 로깅이 적절합니다.프로덕션 환경에서 로그 볼륨이 우려될 경우,
DEBUG레벨로 변경하거나 조건부 로깅을 고려해 볼 수 있습니다:🔎 조건부 로깅 제안
- log.info("⏱️ [Performance] Method: {} | Execution Time: {} ms", + log.debug("⏱️ [Performance] Method: {} | Execution Time: {} ms", joinPoint.getSignature().toShortString(), stopWatch.getTotalTimeMillis());src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java (1)
40-44: 인터럽트 플래그 복원 필요
InterruptedException처리 시 인터럽트 플래그를 복원하는 것이 좋습니다. 이는 상위 호출자가 인터럽트 상태를 확인할 수 있도록 합니다.🔎 인터럽트 플래그 복원
try{ googleApiSemaphore.acquire(); } catch (InterruptedException e){ + Thread.currentThread().interrupt(); throw new RuntimeException("Google API 대기 중 인터럽트 발생", e); }src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java (2)
86-94: 병렬 처리 패턴이 잘 구현되었습니다.
CompletableFuture를 활용한 비동기 처리, 타임아웃 및 예외 폴백 처리가 적절합니다. 다만join()호출이 중복됩니다.
allOf().join()이후 모든 future가 이미 완료되었으므로, 두 번째join()호출에서 블로킹이 발생하지 않지만 의미상 중복입니다:🔎 간결한 결과 수집
- CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])).join(); - return futures.stream().map(CompletableFuture::join).toList(); + return futures.stream() + .map(CompletableFuture::join) + .toList();
allOf().join()을 제거해도 동일하게 동작합니다. 각 future의join()이 개별적으로 완료를 기다리며 결과를 수집합니다.
140-148: 배치 이미지 조회 로직 승인캐시된 이미지가 없을 때만 Google API를 호출하는 전략이 효율적입니다.
googleClient.fetchImages()가 현재 구현상 null을 반환하지 않지만, 방어적 null 체크가 포함되어 있습니다. 일관성을 위해fetchImages의 반환 타입에@NonNull어노테이션을 추가하거나, 여기서 체크를 제거하는 것을 고려해 보세요.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/com/campus/campus/domain/place/application/dto/response/SearchCandidateResponse.javasrc/main/java/com/campus/campus/domain/place/application/service/PlaceService.javasrc/main/java/com/campus/campus/domain/place/domain/repository/PlaceImagesRepository.javasrc/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.javasrc/main/java/com/campus/campus/global/annotation/stopwatch/ExecutionTimeAspect.javasrc/main/java/com/campus/campus/global/annotation/stopwatch/LogExecutionTime.javasrc/main/java/com/campus/campus/global/config/executor/PlaceSearchExecutorConfig.java
🔇 Additional comments (5)
src/main/java/com/campus/campus/global/annotation/stopwatch/LogExecutionTime.java (1)
1-11: LGTM!메서드 실행 시간 측정을 위한 마커 어노테이션이 올바르게 정의되었습니다.
@Retention(RUNTIME)과@Target(METHOD)설정이 AOP 기반 타이밍에 적합합니다.src/main/java/com/campus/campus/domain/place/domain/repository/PlaceImagesRepository.java (1)
14-14: 배치 조회 메서드 추가 승인
findAllByPlaceKeyIn메서드를 통한 벌크 조회는 N+1 문제를 해결하고 성능을 개선합니다.대량의
placeKeys가 전달될 경우 SQL IN 절의 크기 제한에 유의하세요. 현재 사용처(search메서드)에서는 최대 5개로 제한되어 문제없지만, 향후 확장 시 청킹(chunking) 처리가 필요할 수 있습니다.src/main/java/com/campus/campus/domain/place/application/dto/response/SearchCandidateResponse.java (1)
1-12: LGTM!비동기 처리를 위한 중간 DTO로 적절합니다. 불변 record 사용으로 스레드 안전성이 보장되며, 사전 계산된 값들을 캡슐화하여 중복 연산을 방지합니다.
src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java (1)
75-84: 벌크 DB 조회 및 메모리 캐싱 최적화 승인
placeKeys를 수집하여 단일 쿼리로 조회하고Map으로 그룹화하는 패턴이 N+1 문제를 효과적으로 해결합니다.src/main/java/com/campus/campus/global/config/executor/PlaceSearchExecutorConfig.java (1)
11-14: 가상 스레드 설정이 적절합니다.I/O 바운드 작업인 Google API 호출과 DB 접근에 가상 스레드 사용이 적합합니다. Java 21에서
ExecutorService는AutoCloseable을 구현하고 있으므로destroyMethod = "close"설정으로 애플리케이션 종료 시 executor가 정상적으로 정리됩니다. 프로젝트가 Java 21을 사용하므로 가상 스레드 기능을 안전하게 활용할 수 있습니다.
src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java
Outdated
Show resolved
Hide resolved
1224kang
left a comment
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.
고생하셨습니다!! 승현님이 하신대로 비동기 처리하니 성능이 훨씬 좋아진 것 같아서 너무 좋은 것 같아요 감사합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/campus/campus/domain/place/application/service/PlaceService.javasrc/main/java/com/campus/campus/test/TestController.javasrc/main/java/com/campus/campus/test/TestResponseCode.java
💤 Files with no reviewable changes (2)
- src/main/java/com/campus/campus/test/TestResponseCode.java
- src/main/java/com/campus/campus/test/TestController.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java (3)
src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java (1)
Component(16-145)src/main/java/com/campus/campus/domain/place/application/mapper/PlaceMapper.java (2)
Component(16-70)toSavedPlaceInfo(24-36)src/main/java/com/campus/campus/domain/place/infrastructure/naver/NaverMapClient.java (1)
searchPlaces(32-54)
⏰ 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: build
🔇 Additional comments (6)
src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java (6)
6-11: 병렬 처리를 위한 필요한 import가 모두 추가되었습니다.비동기 작업, 타임아웃 처리, 스트림 수집에 필요한 import들이 적절히 추가되었습니다.
63-76: 검색 후보 준비 및 중복 제거 로직이 잘 구현되었습니다.SearchCandidateResponse로 후보를 구조화하고, distinct()로 중복 placeKey를 제거하여 불필요한 DB 조회를 방지한 점이 좋습니다.
78-82: 배치 조회로 N+1 문제를 효과적으로 해결했습니다.기존 반복문 내 개별 조회를
findAllByPlaceKeyIn으로 한 번에 조회하고 Map으로 그룹화하여 각 스레드가 DB 접근 없이 캐시된 이미지를 조회할 수 있도록 개선한 점이 우수합니다.
138-151: 헬퍼 메서드 구현이 명확하고 안전합니다.
convertToSavedPlaceInfo에서 캐시 우선 전략을 적용하고, 144번 라인의 null 체크로 방어적 프로그래밍을 구현한 점이 좋습니다. 폴백 메서드도 간결하게 구현되었습니다.
153-158: HTML 태그 제거 메서드가 적절히 문서화되었습니다.간단한 정규식으로 HTML 태그를 제거하는 로직이 명확하며, Javadoc 주석이 추가되어 가독성이 향상되었습니다.
57-57: ExecutorService 설정 및 생명주기 관리가 적절히 구성되어 있습니다.PlaceSearchExecutorConfig에서
@Bean(destroyMethod = "close")를 통해 ExecutorService의 생명주기가 올바르게 관리되고 있습니다. 가상 스레드 기반 실행자가 사용되고 있으며, PlaceService에서 비동기 작업에 적절히 활용되고 있습니다.
| List<CompletableFuture<SavedPlaceInfo>> futures = candidates.stream() | ||
| .map(response -> CompletableFuture.supplyAsync(() -> convertToSavedPlaceInfo(response, images), | ||
| executorService) | ||
| .completeOnTimeout(fallback(response), 4, TimeUnit.SECONDS) | ||
| .exceptionally(ex -> fallback(response))) | ||
| .toList(); | ||
|
|
||
| CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])).join(); | ||
| return futures.stream().map(CompletableFuture::join).toList(); |
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.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "GooglePlaceClient.java" 2>/dev/nullRepository: our-campUS/our-campUS-BE
Length of output: 159
🏁 Script executed:
# Check PlaceService.java structure and imports
head -50 src/main/java/com/campus/campus/domain/place/application/service/PlaceService.javaRepository: our-campUS/our-campUS-BE
Length of output: 2583
🏁 Script executed:
# Search for Semaphore usage in the project
rg -n "Semaphore" --type javaRepository: our-campUS/our-campUS-BE
Length of output: 608
🏁 Script executed:
# View GooglePlaceClient implementation
cat -n src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.javaRepository: our-campUS/our-campUS-BE
Length of output: 5211
🏁 Script executed:
# View PlaceService.java lines 75-100 for full context
sed -n '75,100p' src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java | cat -nRepository: our-campUS/our-campUS-BE
Length of output: 1174
🏁 Script executed:
# Check if there's another GoogleClient file
find . -type f -name "*GoogleClient.java" 2>/dev/nullRepository: our-campUS/our-campUS-BE
Length of output: 50
🏁 Script executed:
# Find convertToSavedPlaceInfo method
rg -n "convertToSavedPlaceInfo" src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java -A 15Repository: our-campUS/our-campUS-BE
Length of output: 1476
🏁 Script executed:
# Find ExecutorService configuration in PlaceService
sed -n '50,85p' src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java | cat -nRepository: our-campUS/our-campUS-BE
Length of output: 1833
🏁 Script executed:
# Check if there are any other timeout issues with CompletableFuture
rg -n "completeOnTimeout|orTimeout" src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java -B 2 -A 2Repository: our-campUS/our-campUS-BE
Length of output: 327
타임아웃된 작업이 ExecutorService에서 계속 실행되는 문제가 있습니다.
completeOnTimeout은 4초 후 CompletableFuture를 완료하지만 기존 작업을 취소하지 않습니다. convertToSavedPlaceInfo 내에서 호출되는 googleClient.fetchImages()가 4초 이상 걸리면, ExecutorService의 스레드에서 계속 실행되어 다음과 같은 문제가 발생합니다:
- HTTP 커넥션 및 네트워크 리소스 낭비
- 빈번한 호출 시 ExecutorService 스레드 누적
- 불필요한 Google API 호출로 할당량 소비
또한 타임아웃 및 에러 발생 시 로깅이 없어 모니터링 및 디버깅이 어렵습니다.
다만 PR 설명의 Semaphore(동시 실행 수 20개 제한)는 GooglePlaceClient에 정상적으로 구현되어 있으며, try-finally 블록에서 적절히 해제되고 있습니다. 따라서 Semaphore 자체는 문제가 아니며, ExecutorService 스레드의 지속 실행이 주요 이슈입니다.
🔎 로깅 추가 및 태스크 취소 개선 제안
List<CompletableFuture<SavedPlaceInfo>> futures = candidates.stream()
- .map(response -> CompletableFuture.supplyAsync(() -> convertToSavedPlaceInfo(response, images),
+ .map(response -> {
+ CompletableFuture<SavedPlaceInfo> future = CompletableFuture.supplyAsync(
+ () -> convertToSavedPlaceInfo(response, images),
executorService)
- .completeOnTimeout(fallback(response), 4, TimeUnit.SECONDS)
- .exceptionally(ex -> fallback(response)))
+ .completeOnTimeout(fallback(response), 4, TimeUnit.SECONDS)
+ .exceptionally(ex -> {
+ if (ex instanceof TimeoutException) {
+ log.warn("장소 정보 조회 타임아웃: placeKey={}, name={}",
+ response.placeKey(), response.name());
+ } else {
+ log.error("장소 정보 조회 실패: placeKey={}, name={}",
+ response.placeKey(), response.name(), ex);
+ }
+ return fallback(response);
+ });
+ return future;
+ })
.toList();참고: 완전한 태스크 취소를 위해서는 googleClient.fetchImages 내부에서 Thread.interrupted() 확인 또는 CompletableFuture 취소 신호를 전파하는 메커니즘이 필요합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In
@src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java:
- Around line 40-44: The semaphore acquire call using
googleApiSemaphore.acquire() can block indefinitely and the catch wraps
InterruptedException in a RuntimeException losing the thread interrupt status;
replace acquire() with tryAcquire(timeout, TimeUnit.SECONDS) (choose an
appropriate timeout) and if it returns false return an empty result (or
fail-fast) from the surrounding method, and if InterruptedException occurs
restore the interrupt status with Thread.currentThread().interrupt() and return
an empty result instead of throwing; also add the java.util.concurrent.TimeUnit
import and update the logic around the googleApiSemaphore.acquire() site in
GooglePlaceClient to reflect these changes.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java (2)
src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java (4)
getPlaceImgs(123-137)migrateImagesToOci(178-202)Service(38-203)getImages(142-146)src/main/java/com/campus/campus/domain/place/domain/entity/PlaceImages.java (1)
Entity(18-40)
⏰ 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: build
🔇 Additional comments (4)
src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java (4)
25-26: 동시성 제어를 위한 세마포어 추가가 적절합니다.Google API 호출을 20개로 제한하여 외부 API 부하를 관리하는 좋은 접근입니다.
46-68: try-finally를 사용한 세마포어 해제 로직이 안전합니다.예외 발생 시에도
finally블록에서 세마포어가 확실히 해제되어 리소스 누수를 방지합니다. null 체크와 빈 결과 처리도 적절합니다.
92-95: 검색 결과 0건 방어 로직이 적절합니다.커밋 메시지에 언급된
IndexOutOfBoundsException방지를 위한 null/empty 체크가 잘 추가되었습니다. 로깅도 문제 추적에 유용합니다.
85-85: PR 설명과 코드의 타임아웃 값 불일치를 확인할 수 없습니다.코드의 85번 줄과 118번 줄에서 타임아웃이 3초로 설정되어 있는 것은 확실하지만, 현재 확인 가능한 범위 내에서 PR 설명의 내용을 검증할 수 없습니다. PR 설명에서 실제로 4초 타임아웃을 명시했는지, 그리고 이것이 의도한 설정인지 확인이 필요합니다.
src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In
@src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java:
- Around line 60-64: The fetchImages method currently ignores its limit
parameter because photoRefs.stream().limit(3) is hardcoded; update the stream
call to use the method parameter (replace .limit(3) with .limit(limit)) so the
provided limit is honored, or if the parameter is truly unused remove the limit
parameter from the fetchImages signature and related callers; ensure
buildPhotoUrl is still used to map refs to URLs and adjust any tests or callers
accordingly.
🧹 Nitpick comments (2)
src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java (2)
26-26: 세마포어 permit 수를 설정 파일로 외부화하는 것을 권장합니다.현재 20으로 하드코딩되어 있어 운영 중 동시성 수준을 조정하려면 코드 수정과 재배포가 필요합니다.
@Value어노테이션을 사용해application.properties에서 주입받도록 개선하면 유연성이 향상됩니다.🔎 제안하는 개선 방안
필드 선언을 다음과 같이 수정:
- private final Semaphore googleApiSemaphore = new Semaphore(20); + private final Semaphore googleApiSemaphore;생성자에서 주입:
public GooglePlaceClient( - @Value("${map.google.places.api-key}") String apiKey + @Value("${map.google.places.api-key}") String apiKey, + @Value("${map.google.places.max-concurrent-requests:20}") int maxConcurrentRequests ) { this.apiKey = apiKey; + this.googleApiSemaphore = new Semaphore(maxConcurrentRequests); this.webClient = WebClient.builder() .baseUrl(BASE_URL) .build(); }그리고
application.properties에 추가:map.google.places.max-concurrent-requests=20
41-47: 이전 리뷰 지적사항이 올바르게 수정되었습니다.
acquire()의 무한 대기 문제가tryAcquire(5, TimeUnit.SECONDS)로 개선되어 타임아웃 시 빈 리스트를 반환하도록 처리되었습니다. 다만, 타임아웃 값(5초)이 하드코딩되어 있어 설정 파일로 외부화하면 더 유연하게 조정할 수 있습니다.🔎 선택적 개선 방안: 타임아웃 값 외부화
+ private final int semaphoreTimeoutSeconds; + public GooglePlaceClient( @Value("${map.google.places.api-key}") String apiKey, + @Value("${map.google.places.semaphore-timeout-seconds:5}") int semaphoreTimeoutSeconds ) { this.apiKey = apiKey; + this.semaphoreTimeoutSeconds = semaphoreTimeoutSeconds; // ... }그리고
tryAcquire호출 시:- acquired = googleApiSemaphore.tryAcquire(5, TimeUnit.SECONDS); + acquired = googleApiSemaphore.tryAcquire(semaphoreTimeoutSeconds, TimeUnit.SECONDS);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java
🔇 Additional comments (5)
src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java (5)
5-6: LGTM!동시성 제어를 위한 필수 import가 적절히 추가되었습니다.
49-52: LGTM!
placeId가 null일 때 빈 리스트를 반환하여 폴백을 제공하는 로직이 올바릅니다. finally 블록에서 세마포어가 해제되므로 리소스 관리도 적절합니다.
68-76: LGTM!예외 처리가 올바르게 구현되었습니다:
InterruptedException발생 시 스레드의 인터럽트 상태를 복원(Thread.currentThread().interrupt())하여 상위 호출자가 인터럽트를 인지할 수 있도록 처리- 모든 예외에 대해 빈 리스트를 반환하여 PR 목표에 명시된 폴백 정책을 준수
77-81: LGTM!finally 블록에서
acquired플래그를 확인한 후 세마포어를 해제하는 로직이 올바릅니다. 타임아웃이나 예외 발생으로 인해 permit을 획득하지 못한 경우에도 안전하게 처리됩니다.
105-108: LGTM!검색 결과가 없을 때 로그를 남기고 null을 반환하는 처리가 적절합니다. 디버깅에 유용한 정보를 제공하며 상위 메서드에서 일관되게 처리됩니다.
| //imageURL 생성 | ||
| List<String> imageUrls = photoRefs.stream() | ||
| .limit(3) | ||
| .map(this::buildPhotoUrl) | ||
| .toList(); |
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.
메서드 파라미터 limit가 사용되지 않고 있습니다.
Line 62에서 .limit(3)으로 하드코딩되어 있어 fetchImages 메서드의 limit 파라미터가 무시됩니다. 파라미터를 활용하거나 불필요하다면 시그니처에서 제거하는 것이 좋습니다.
🔎 제안하는 수정 방안
List<String> imageUrls = photoRefs.stream()
- .limit(3)
+ .limit(limit)
.map(this::buildPhotoUrl)
.toList();🤖 Prompt for AI Agents
In
@src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java
around lines 60-64, The fetchImages method currently ignores its limit parameter
because photoRefs.stream().limit(3) is hardcoded; update the stream call to use
the method parameter (replace .limit(3) with .limit(limit)) so the provided
limit is honored, or if the parameter is truly unused remove the limit parameter
from the fetchImages signature and related callers; ensure buildPhotoUrl is
still used to map refs to URLs and adjust any tests or callers accordingly.
🔀 변경 내용
✅ 작업 항목
기존 방식: 네이버 검색 결과가 5개일 경우 1번째 장소에 대한 이미지를 찾고 완료된 후 2번째 장소에 대한 이미지를 찾는 순차(직렬)처리 구조
수정 방식: CompletableFuture를 사용한 병렬 처리 구조를 통해 동시에 각 장소에 대한 작업을 동시에 진행
기존 방식: 루프를 돌며
findByPlaceKey(placeKey)를 매번 호출. 검색 결과가 5개일 경우 DB 커넥션이 5번 이루어짐.수정 방식:
findAllByPlaceKeyIn(placeKeys)를 통해 필요한 이미지 정보를 한번의 쿼리로 가져옴. 이를 Map 형태로 메모리에 올리고 각 스레드는 DB 접근 없이 메모리에서 조회.📸 스크린샷 (선택)
기존에는 api 한번 동작시 4350ms의 시간이 소요되었지만 수정했을 경우 api 한번 동작시 1243ms의 시간이 소요되어 약 71.4%의 성능 개선이 이루어졌습니다.
📎 참고 이슈
관련 이슈 번호 #33
Summary by CodeRabbit
새로운 기능
Chores
✏️ Tip: You can customize this high-level summary in your review settings.