feat: 장소 및 경로 벌크 조회 API 추가 및 비동기 가상 스레드 최적화#132
Merged
Merged
Conversation
- POST /places/photo-names/batch API 신규 구현 - PlacePhotoNameService에 대표 사진 이름 목록 병렬 벌크 조회 로직 추가 (taskExecutor 활용, 중복 ID 제거 및 요청 순서 유지, 부분 실패 대응) - HttpRateLimitPolicyResolver에 벌크 조회 경로를 rate limit 대상으로 추가 - 관련 컨트롤러 테스트, 서비스/통합 테스트 작성 및 docs/ai/features.md 명세 업데이트
|
Warning Review limit reached
More reviews will be available in 31 minutes and 46 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughPR은 비동기 실행 채널을 AI/Google API별로 분리하고, 레이트 리밋 정책을 단일/배치 및 사진 전용으로 세분화하며, Redis 벌크 캐시를 도입하고 Places/Bookmarks/Schedules에 배치 조회·선택적 로딩 기능과 관련 테스트를 추가합니다. Changes인프라 및 설정 계층
Places API 배치 조회 및 캐싱
Bookmarks API 카테고리 필터
Schedules API 아이템 포함 및 배치 라우트
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
|
![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/howaboutus/backend/common/ratelimit/HttpRateLimitPolicyResolver.java (1)
32-36: ⚡ Quick winPLACE_BATCH_READ_PATHS 리스트의 일관성을 확인하세요.
PLACE_BATCH_READ_PATHS에/places/photos/batch가 포함되어 있는데, 이 경로는 Line 90-93에서 별도로places-photo-bulk정책을 적용받습니다. 따라서 Line 97의PLACE_BATCH_READ_PATHS.contains(path)조건에서는/places/photos/batch가 매칭되지 않도록 해야 합니다.현재 로직은 Line 85-94에서 early return을 사용하므로 Line 97에 도달하지 않지만, 코드 가독성과 유지보수성을 위해
PLACE_BATCH_READ_PATHS에서/places/photos/batch를 제거하는 것을 고려하세요.♻️ PLACE_BATCH_READ_PATHS 정리 제안
private static final List<String> PLACE_BATCH_READ_PATHS = List.of( "/places/previews/batch", - "/places/photo-names/batch", - "/places/photos/batch" + "/places/photo-names/batch" );또는 주석으로 의도를 명확히:
private static final List<String> PLACE_BATCH_READ_PATHS = List.of( "/places/previews/batch", - "/places/photo-names/batch", - "/places/photos/batch" + "/places/photo-names/batch" + // Note: /places/photos/batch is handled separately in addPlacesPolicies );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/howaboutus/backend/common/ratelimit/HttpRateLimitPolicyResolver.java` around lines 32 - 36, PLACE_BATCH_READ_PATHS currently includes "/places/photos/batch" which conflicts with the dedicated "places-photo-bulk" policy check in HttpRateLimitPolicyResolver; remove "/places/photos/batch" from the PLACE_BATCH_READ_PATHS constant (or alternatively add a clear comment next to the constant explaining that "/places/photos/batch" is intentionally excluded because it is handled by the places-photo-bulk policy) so that PLACE_BATCH_READ_PATHS.contains(path) cannot incorrectly match that route and the explicit places-photo-bulk branch (lines handling that policy) remains the single source of truth.docs/ai/erd.md (1)
193-193: 💤 Low value이동 정보 응답 규칙이 명확하게 정의되었습니다.
단건 조회(204)와 벌크 조회(200 + status/errorCode)의 응답 방식이 잘 구분되어 있습니다. 다만 Google Routes API의 "경로를 찾지 못한" 경우와 "마지막 장소" 경우를 동일하게 204로 처리하는데, 클라이언트 입장에서 이 두 상황을 구분할 필요가 있는지 검토하시기 바랍니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ai/erd.md` at line 193, The docs currently collapse two different cases—"last place (no subsequent leg)" and "Google Routes returned no route"—into the same 204 response for single lookups, which makes it impossible for clients to distinguish them; update the API spec and docs to explicitly define how to differentiate these cases (either change single-lookup behavior to return a distinct per-route status/errorCode instead of 204, or keep 204 but add a deterministic machine-readable indicator such as a specific errorCode/value in the response or an HTTP header), and reflect that change in areas mentioning travelMode, route::{origin}:{dest}:{mode} cache behavior, and the route response shape (route.status / route.errorCode) so clients can reliably detect "last place" versus "no route" in both single and bulk flows.src/main/java/com/howaboutus/backend/places/controller/dto/PlacePreviewBatchItemResponse.java (1)
30-30: ⚖️ Poor tradeoffController DTO가 service layer 타입을 직접 참조하는 구조 검토 권장.
PlacePreviewResult.Location타입을 controller DTO에서 직접 사용하고 있어 계층 간 결합이 발생합니다.Location이 단순 좌표 값 객체라 공유가 의도된 것일 수 있으나, controller layer 전용Locationrecord를 정의하면 계층 분리가 명확해지고 향후 controller/service 응답 형식이 독립적으로 변경 가능합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/howaboutus/backend/places/controller/dto/PlacePreviewBatchItemResponse.java` at line 30, The controller DTO PlacePreviewBatchItemResponse currently references the service-layer type PlacePreviewResult.Location causing tight coupling; define a new controller-layer record (e.g., PlacePreviewBatchItemResponse.Location or ControllerLocation) that contains the same coordinate fields, replace the PlacePreviewResult.Location usage in PlacePreviewBatchItemResponse with this new record, and add a simple mapping in the controller or a mapper method to convert PlacePreviewResult.Location -> the new controller Location when constructing PlacePreviewBatchItemResponse.src/test/java/com/howaboutus/backend/places/service/PlaceDetailCachingTest.java (1)
103-154: ⚡ Quick win캐시된 preview의 photo name 조회 실패 케이스 테스트 추가 권장.
배치 조회 테스트에서 preview 자체 조회 실패(Line 137-154)와 photo name 조회 실패(Line 180-199)는 검증되지만, 캐시 히트된 preview의 photo name 갱신 실패는 테스트되지 않습니다.
PlacePreviewService.fetchCachedWithFreshPhotoNames(Line 84-88)에서getFirstPhotoName이 예외를 던질 경우의 동작을 검증하는 테스트를 추가하면, 부분 실패 처리가 일관되게 작동하는지 확인할 수 있습니다.📝 테스트 케이스 예시
`@Test` `@DisplayName`("캐시된 미리보기의 사진 이름 갱신 실패 시 항목별 실패 또는 null 처리") void handlesPhotoNameFetchFailureForCachedPreviews() { // 첫 번째 호출로 preview 캐싱 given(googlePlaceDetailClient.getPreview("ChIJ123")) .willReturn(detailResponse("places/ChIJ123/photos/a")); placePreviewService.getPreview("ChIJ123"); // 두 번째 배치 호출에서 photo name 조회 실패 given(googlePlaceDetailClient.getPhotoNames("ChIJ123")) .willThrow(new ExternalApiException(new RuntimeException("timeout"))); List<PlacePreviewBatchItemResult> results = placePreviewService.getPreviews( List.of("ChIJ123") ); // 기대 동작: preview는 유효하므로 photoName=null 또는 항목별 실패 assertThat(results).hasSize(1); // 구현에 따라 적절한 assertion 추가 }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/howaboutus/backend/places/service/PlaceDetailCachingTest.java` around lines 103 - 154, Add a test that seeds the cache with a successful preview (use given(googlePlaceDetailClient.getPreview("ChIJ123")).willReturn(...); call placePreviewService.getPreview("ChIJ123")) then exercise fetchCachedWithFreshPhotoNames by invoking placePreviewService.getPreviews(List.of("ChIJ123")) after stubbing googlePlaceDetailClient.getPhotoNames("ChIJ123") to throw an ExternalApiException; assert the batch result for "ChIJ123" matches the service's expected behavior (either photoName==null or status/errorCode==EXTERNAL_API_ERROR) and verify interactions: getPreview was used only for the initial cache seed and getPhotoNames was called once and resulted in the failure path.src/main/java/com/howaboutus/backend/common/cache/RedisBulkCacheAccessor.java (1)
67-77: ⚡ Quick win매 요청마다 serializer 인스턴스를 재생성하는 비효율 개선 권장.
serializer()메서드가multiGet/put호출마다 새로운GenericJacksonJsonRedisSerializer인스턴스를 생성합니다. 설정이 불변이므로 이를private final필드로 생성자에서 한 번만 초기화하면 불필요한 객체 생성을 방지할 수 있습니다.♻️ 필드로 리팩토링 제안
public class RedisBulkCacheAccessor { private final RedisConnectionFactory connectionFactory; - private final ObjectMapper objectMapper; + private final GenericJacksonJsonRedisSerializer serializer; - public PlacePreviewService( + public RedisBulkCacheAccessor( RedisConnectionFactory connectionFactory, ObjectMapper objectMapper) { this.connectionFactory = connectionFactory; - this.objectMapper = objectMapper; + this.serializer = GenericJacksonJsonRedisSerializer.builder(objectMapper::rebuild) + .enableDefaultTyping( + BasicPolymorphicTypeValidator.builder() + .allowIfSubType("com.howaboutus.backend") + .allowIfBaseType(Map.class) + .allowIfBaseType(Collection.class) + .build() + ) + .build(); } public <T> Map<String, T> multiGet(...) { ... - GenericJacksonJsonRedisSerializer serializer = serializer(); for (int index = 0; index < distinctKeys.size(); index++) { ... } ... } public void put(...) { ... - serializer().serialize(value), + serializer.serialize(value), ... } - - private GenericJacksonJsonRedisSerializer serializer() { - return GenericJacksonJsonRedisSerializer.builder(objectMapper::rebuild) - ... - .build(); - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/howaboutus/backend/common/cache/RedisBulkCacheAccessor.java` around lines 67 - 77, The serializer() method in RedisBulkCacheAccessor recreates a GenericJacksonJsonRedisSerializer on every call; make it a single private final field (e.g., private final GenericJacksonJsonRedisSerializer serializer) initialized once in the RedisBulkCacheAccessor constructor using the same objectMapper::rebuild and BasicPolymorphicTypeValidator setup (preserve allowIfSubType("com.howaboutus.backend") and allowIfBaseType for Map/Collection), then update methods like multiGet and put to use that field instead of calling serializer().src/main/java/com/howaboutus/backend/places/controller/PlaceController.java (1)
162-171: 💤 Low value응답 DTO 생성 패턴의 일관성을 고려하세요.
Line 168에서
PlacePhotoResponse를 직접 생성하는 반면, 다른 엔드포인트들(예: Line 116, 132, 149)은 정적 팩토리 메서드from()을 사용합니다.PlacePhotoResponse가 단순한 wrapper라면 현재 방식도 괜찮지만, 일관성을 위해 정적 팩토리 메서드 사용을 고려할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/howaboutus/backend/places/controller/PlaceController.java` around lines 162 - 171, The getPhotoUrl method in PlaceController currently constructs the response with new PlacePhotoResponse(placePhotoService.getPhotoUrl(photoName)) which is inconsistent with other endpoints that use the static factory method from(); update this method to return ResponseEntity.ok(PlacePhotoResponse.from(placePhotoService.getPhotoUrl(photoName))) when photoEnabled is true, and if PlacePhotoResponse lacks a from(...) factory, add a static from(String url) factory in the PlacePhotoResponse class that wraps the URL to preserve consistency with other endpoints like the ones using from().src/main/java/com/howaboutus/backend/places/service/dto/PlacePhotoBatchItemResult.java (1)
14-16: 💤 Low valuestatus 필드가 errorCode와 중복되는 값을 가지는 설계를 검토하세요.
failure()팩토리에서status와errorCode에 동일한errorCode값을 설정하고 있습니다. 이는ok()가status="OK"로 상태 리터럴을 사용하는 것과 일관성이 떨어지며, 클라이언트가 성공/실패를 판단할 때 두 필드를 모두 확인해야 하는 모호함을 만듭니다.status를"FAILED"또는"ERROR"같은 표준 상태로 통일하고errorCode는 실패 원인 코드만 담도록 분리하는 것을 고려해보세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/howaboutus/backend/places/service/dto/PlacePhotoBatchItemResult.java` around lines 14 - 16, The failure() factory in PlacePhotoBatchItemResult sets both status and errorCode to the same value, causing ambiguity; change failure(String photoName, String errorCode) so it constructs a PlacePhotoBatchItemResult with a standard failure status literal (e.g., "FAILED" or "ERROR") for the status field and keep errorCode only for the error reason; update any usages expecting the old behavior if necessary and ensure ok() remains using status="OK" while errorCode stays null on success.src/main/java/com/howaboutus/backend/schedules/service/dto/RouteBatchItemResult.java (1)
32-36: 💤 Low value실패 케이스에서
status와errorCode가 중복됩니다.
failure팩토리 메서드에서status필드와errorCode필드에 모두 동일한errorCode값을 할당하고 있습니다. 이는 클라이언트가 두 필드 중 하나를 선택적으로 확인할 수 있도록 하는 의도로 보이지만, 데이터 중복이 발생합니다.더 명확한 구분을 위해 성공/실패를 나타내는 일반 상태(
status="SUCCESS"/"ERROR")와 구체적인 에러 코드(errorCode="NO_ROUTE"등)로 분리하는 것을 고려해볼 수 있습니다. 다만 현재 설계가 의도적이라면 유지해도 무방합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/howaboutus/backend/schedules/service/dto/RouteBatchItemResult.java` around lines 32 - 36, The failure factory method RouteBatchItemResult.failure currently assigns the same errorCode string to both the status and errorCode fields, causing duplication; update RouteBatchItemResult.failure to set a clear general status value (e.g., "ERROR" or "FAILURE") for the status parameter and pass the specific errorCode (e.g., "NO_ROUTE") only to the errorCode field so status and errorCode are distinct while keeping the other constructor arguments the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/ai/features.md`:
- Around line 39-40: Update the rate-limit table entry for "Places 상세/미리보기/사진
이름/사진 URL 조회 및 벌크 조회" in docs/ai/features.md to match the more granular values
defined in application.yaml by splitting the single `30/min/user` row into four
rows (or a single row with four sub-items) referencing the policy keys:
places-read-single = 30/min/user, places-read-bulk = 10/min/user,
places-photo-single = 45/min/user, and places-photo-bulk = 15/min/user so the
doc reflects each operation's specific limit.
In
`@src/main/java/com/howaboutus/backend/bookmarks/repository/BookmarkRepository.java`:
- Around line 30-31: 두 조회 메서드의 정렬 기준이 일치하지 않아 동일 엔드포인트에서 categoryId 유무로 결과 정렬이
달라집니다; findAllByRoom_IdAndCategory_IdOrderByCreatedAtDesc와
findAllByRoom_IdOrderByCategory_CreatedAtAscCategory_IdAscCreatedAtDescIdDesc 중
하나로 정렬 규칙을 통일하거나(예: 양쪽 모두 category.createdAt ASC, category.id ASC,
bookmark.createdAt DESC, bookmark.id DESC 또는 반대로), 의도된 차이라면 API 문서에 명확히 명시하세요;
구현상 수정은 해당 Repository 인터페이스의 메서드 시그니처(함수명:
findAllByRoom_IdAndCategory_IdOrderByCreatedAtDesc 및
findAllByRoom_IdOrderByCategory_CreatedAtAscCategory_IdAscCreatedAtDescIdDesc)를
찾아 동일한 OrderBy 절로 변경하거나 별도 엔드포인트/문서화를 추가하면 됩니다.
In `@src/main/java/com/howaboutus/backend/common/config/CachePolicy.java`:
- Line 20: 현재 CachePolicy의 PLACE_PHOTO_URI
(PLACE_PHOTO_URI(Keys.PLACE_PHOTO_URI, Duration.ofDays(1)))는 Google Places에서
제공하는 photo URI/식별자가 단기 만료될 수 있는 특성을 반영하지 않아 만료된 URI를 캐시할 위험이 있습니다; 수정 방법:
CachePolicy 클래스에서 PLACE_PHOTO_URI 엔트리의 TTL을 대폭 단축(예: 몇 분 단위)하거나 제거하고 대신 placeId
중심 캐싱을 우선하도록 변경하며(예: placeId 키의 캐시 유지), 사진 URI를 반환하는 로직(사진 조회/서버 코드)에서는 URI를 캐시된
placeId로부터 필요 시(또는 4xx 실패 시) 재조회/갱신하도록 구현해 Keys.PLACE_PHOTO_URI와 placeId 반환/조회
흐름을 함께 업데이트/검토하세요.
In
`@src/main/java/com/howaboutus/backend/places/service/PlacePreviewService.java`:
- Around line 84-88: In fetchCachedWithFreshPhotoNames, the CompletableFuture
lambda that calls placePhotoNameService.getFirstPhotoName(googlePlaceId) can
throw non-CustomException exceptions which bubble up and fail the entire batch;
wrap that call inside the lambda with a try/catch that mirrors
fetchMissingPreviews behavior: catch ExternalApiException (and any other runtime
exceptions you expect) and either return
PlacePreviewBatchItemResult.ok(preview.withPhotoName(null)) or return
PlacePreviewBatchItemResult.failed(preview.getId(), EXTERNAL_API_ERROR) for
per-item failure consistency; update the lambda in
fetchCachedWithFreshPhotoNames to handle exceptions and return a safe
PlacePreviewBatchItemResult instead of letting the exception escape.
In
`@src/test/java/com/howaboutus/backend/places/service/PlacePhotoServiceTest.java`:
- Around line 17-24: Replace manual Mockito.mock(...) construction in
PlacePhotoServiceTest with annotation-based setup: add
`@ExtendWith`(MockitoExtension.class) on the test class, annotate
googlePlacePhotoClient and redisBulkCacheAccessor fields with `@Mock`, replace the
explicit new PlacePhotoService(...) with an `@InjectMocks` PlacePhotoService
placePhotoService field so Mockito injects the mocks, and leave the Executor
taskExecutor as a real instance field but either annotate it with `@Spy` or set it
into the injected placePhotoService in a `@BeforeEach` to ensure the real
Runnable::run executor is used.
---
Nitpick comments:
In `@docs/ai/erd.md`:
- Line 193: The docs currently collapse two different cases—"last place (no
subsequent leg)" and "Google Routes returned no route"—into the same 204
response for single lookups, which makes it impossible for clients to
distinguish them; update the API spec and docs to explicitly define how to
differentiate these cases (either change single-lookup behavior to return a
distinct per-route status/errorCode instead of 204, or keep 204 but add a
deterministic machine-readable indicator such as a specific errorCode/value in
the response or an HTTP header), and reflect that change in areas mentioning
travelMode, route::{origin}:{dest}:{mode} cache behavior, and the route response
shape (route.status / route.errorCode) so clients can reliably detect "last
place" versus "no route" in both single and bulk flows.
In
`@src/main/java/com/howaboutus/backend/common/cache/RedisBulkCacheAccessor.java`:
- Around line 67-77: The serializer() method in RedisBulkCacheAccessor recreates
a GenericJacksonJsonRedisSerializer on every call; make it a single private
final field (e.g., private final GenericJacksonJsonRedisSerializer serializer)
initialized once in the RedisBulkCacheAccessor constructor using the same
objectMapper::rebuild and BasicPolymorphicTypeValidator setup (preserve
allowIfSubType("com.howaboutus.backend") and allowIfBaseType for
Map/Collection), then update methods like multiGet and put to use that field
instead of calling serializer().
In
`@src/main/java/com/howaboutus/backend/common/ratelimit/HttpRateLimitPolicyResolver.java`:
- Around line 32-36: PLACE_BATCH_READ_PATHS currently includes
"/places/photos/batch" which conflicts with the dedicated "places-photo-bulk"
policy check in HttpRateLimitPolicyResolver; remove "/places/photos/batch" from
the PLACE_BATCH_READ_PATHS constant (or alternatively add a clear comment next
to the constant explaining that "/places/photos/batch" is intentionally excluded
because it is handled by the places-photo-bulk policy) so that
PLACE_BATCH_READ_PATHS.contains(path) cannot incorrectly match that route and
the explicit places-photo-bulk branch (lines handling that policy) remains the
single source of truth.
In
`@src/main/java/com/howaboutus/backend/places/controller/dto/PlacePreviewBatchItemResponse.java`:
- Line 30: The controller DTO PlacePreviewBatchItemResponse currently references
the service-layer type PlacePreviewResult.Location causing tight coupling;
define a new controller-layer record (e.g.,
PlacePreviewBatchItemResponse.Location or ControllerLocation) that contains the
same coordinate fields, replace the PlacePreviewResult.Location usage in
PlacePreviewBatchItemResponse with this new record, and add a simple mapping in
the controller or a mapper method to convert PlacePreviewResult.Location -> the
new controller Location when constructing PlacePreviewBatchItemResponse.
In `@src/main/java/com/howaboutus/backend/places/controller/PlaceController.java`:
- Around line 162-171: The getPhotoUrl method in PlaceController currently
constructs the response with new
PlacePhotoResponse(placePhotoService.getPhotoUrl(photoName)) which is
inconsistent with other endpoints that use the static factory method from();
update this method to return
ResponseEntity.ok(PlacePhotoResponse.from(placePhotoService.getPhotoUrl(photoName)))
when photoEnabled is true, and if PlacePhotoResponse lacks a from(...) factory,
add a static from(String url) factory in the PlacePhotoResponse class that wraps
the URL to preserve consistency with other endpoints like the ones using from().
In
`@src/main/java/com/howaboutus/backend/places/service/dto/PlacePhotoBatchItemResult.java`:
- Around line 14-16: The failure() factory in PlacePhotoBatchItemResult sets
both status and errorCode to the same value, causing ambiguity; change
failure(String photoName, String errorCode) so it constructs a
PlacePhotoBatchItemResult with a standard failure status literal (e.g., "FAILED"
or "ERROR") for the status field and keep errorCode only for the error reason;
update any usages expecting the old behavior if necessary and ensure ok()
remains using status="OK" while errorCode stays null on success.
In
`@src/main/java/com/howaboutus/backend/schedules/service/dto/RouteBatchItemResult.java`:
- Around line 32-36: The failure factory method RouteBatchItemResult.failure
currently assigns the same errorCode string to both the status and errorCode
fields, causing duplication; update RouteBatchItemResult.failure to set a clear
general status value (e.g., "ERROR" or "FAILURE") for the status parameter and
pass the specific errorCode (e.g., "NO_ROUTE") only to the errorCode field so
status and errorCode are distinct while keeping the other constructor arguments
the same.
In
`@src/test/java/com/howaboutus/backend/places/service/PlaceDetailCachingTest.java`:
- Around line 103-154: Add a test that seeds the cache with a successful preview
(use given(googlePlaceDetailClient.getPreview("ChIJ123")).willReturn(...); call
placePreviewService.getPreview("ChIJ123")) then exercise
fetchCachedWithFreshPhotoNames by invoking
placePreviewService.getPreviews(List.of("ChIJ123")) after stubbing
googlePlaceDetailClient.getPhotoNames("ChIJ123") to throw an
ExternalApiException; assert the batch result for "ChIJ123" matches the
service's expected behavior (either photoName==null or
status/errorCode==EXTERNAL_API_ERROR) and verify interactions: getPreview was
used only for the initial cache seed and getPhotoNames was called once and
resulted in the failure path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e1714a7b-c06b-4cd8-8482-ec4bc0cfd275
📒 Files selected for processing (61)
docs/ai/erd.mddocs/ai/features.mddocs/superpowers/plans/2026-06-07-rate-limit-photo-split.mddocs/superpowers/plans/2026-06-07-rate-limit-redesign.mdsrc/main/java/com/howaboutus/backend/ai/listener/AiSummaryTriggerListener.javasrc/main/java/com/howaboutus/backend/ai/service/AiRequestQueueWorker.javasrc/main/java/com/howaboutus/backend/bookmarks/controller/BookmarkController.javasrc/main/java/com/howaboutus/backend/bookmarks/repository/BookmarkRepository.javasrc/main/java/com/howaboutus/backend/bookmarks/service/BookmarkService.javasrc/main/java/com/howaboutus/backend/common/cache/RedisBulkCacheAccessor.javasrc/main/java/com/howaboutus/backend/common/config/AsyncConfig.javasrc/main/java/com/howaboutus/backend/common/config/CachePolicy.javasrc/main/java/com/howaboutus/backend/common/config/properties/AsyncExecutorProperties.javasrc/main/java/com/howaboutus/backend/common/config/properties/RateLimitProperties.javasrc/main/java/com/howaboutus/backend/common/ratelimit/HttpRateLimitPolicyResolver.javasrc/main/java/com/howaboutus/backend/places/controller/PlaceController.javasrc/main/java/com/howaboutus/backend/places/controller/dto/PlacePhotoBatchItemResponse.javasrc/main/java/com/howaboutus/backend/places/controller/dto/PlacePhotoBatchRequest.javasrc/main/java/com/howaboutus/backend/places/controller/dto/PlacePhotoBatchResponse.javasrc/main/java/com/howaboutus/backend/places/controller/dto/PlacePhotoNameBatchItemResponse.javasrc/main/java/com/howaboutus/backend/places/controller/dto/PlacePhotoNameBatchRequest.javasrc/main/java/com/howaboutus/backend/places/controller/dto/PlacePhotoNameBatchResponse.javasrc/main/java/com/howaboutus/backend/places/controller/dto/PlacePreviewBatchItemResponse.javasrc/main/java/com/howaboutus/backend/places/controller/dto/PlacePreviewBatchRequest.javasrc/main/java/com/howaboutus/backend/places/controller/dto/PlacePreviewBatchResponse.javasrc/main/java/com/howaboutus/backend/places/service/PlacePhotoNameService.javasrc/main/java/com/howaboutus/backend/places/service/PlacePhotoService.javasrc/main/java/com/howaboutus/backend/places/service/PlacePreviewService.javasrc/main/java/com/howaboutus/backend/places/service/dto/PlacePhotoBatchItemResult.javasrc/main/java/com/howaboutus/backend/places/service/dto/PlacePhotoNameBatchItemResult.javasrc/main/java/com/howaboutus/backend/places/service/dto/PlacePreviewBatchItemResult.javasrc/main/java/com/howaboutus/backend/schedules/controller/ScheduleController.javasrc/main/java/com/howaboutus/backend/schedules/controller/ScheduleRouteController.javasrc/main/java/com/howaboutus/backend/schedules/controller/dto/BatchRouteItemRequest.javasrc/main/java/com/howaboutus/backend/schedules/controller/dto/BatchRouteItemResponse.javasrc/main/java/com/howaboutus/backend/schedules/controller/dto/BatchRouteRequest.javasrc/main/java/com/howaboutus/backend/schedules/controller/dto/BatchRouteResponse.javasrc/main/java/com/howaboutus/backend/schedules/controller/dto/ScheduleResponse.javasrc/main/java/com/howaboutus/backend/schedules/repository/ScheduleItemRepository.javasrc/main/java/com/howaboutus/backend/schedules/repository/ScheduleRepository.javasrc/main/java/com/howaboutus/backend/schedules/service/ScheduleItemService.javasrc/main/java/com/howaboutus/backend/schedules/service/ScheduleService.javasrc/main/java/com/howaboutus/backend/schedules/service/dto/RouteBatchItemCommand.javasrc/main/java/com/howaboutus/backend/schedules/service/dto/RouteBatchItemResult.javasrc/main/java/com/howaboutus/backend/schedules/service/dto/ScheduleWithItemsResult.javasrc/main/resources/application.yamlsrc/test/java/com/howaboutus/backend/bookmarks/BookmarkIntegrationTest.javasrc/test/java/com/howaboutus/backend/bookmarks/controller/BookmarkControllerTest.javasrc/test/java/com/howaboutus/backend/bookmarks/service/BookmarkServiceTest.javasrc/test/java/com/howaboutus/backend/common/config/AsyncConfigTest.javasrc/test/java/com/howaboutus/backend/common/ratelimit/HttpRateLimitPolicyResolverTest.javasrc/test/java/com/howaboutus/backend/messages/service/MessageRateLimiterTest.javasrc/test/java/com/howaboutus/backend/places/controller/PlaceControllerTest.javasrc/test/java/com/howaboutus/backend/places/service/PlaceDetailCachingTest.javasrc/test/java/com/howaboutus/backend/places/service/PlacePhotoCachingTest.javasrc/test/java/com/howaboutus/backend/places/service/PlacePhotoServiceTest.javasrc/test/java/com/howaboutus/backend/schedules/ScheduleIntegrationTest.javasrc/test/java/com/howaboutus/backend/schedules/controller/ScheduleControllerTest.javasrc/test/java/com/howaboutus/backend/schedules/controller/ScheduleRouteControllerTest.javasrc/test/java/com/howaboutus/backend/schedules/service/ScheduleItemServiceTest.javasrc/test/java/com/howaboutus/backend/schedules/service/ScheduleServiceTest.java
- RedisBulkCacheAccessor Jackson Serializer 캐싱 최적화 - DTO status/errorCode 스펙 정비 및 결합도 완화 - PlacePhotoService 등 Executor Lombok RequiredArgsConstructor 주입 적용 - Rate Limit 정책 예외 조건 보강 및 테스트 코드 최신화
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/com/howaboutus/backend/places/controller/PlaceControllerTest.java (1)
767-769:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win실패 항목의
status기대값이 잘못됐습니다.여기서는
errorCode가 아니라status를 검증하고 있어서"EXTERNAL_API_ERROR"대신"FAILED"를 기대해야 합니다. 지금 값이면 응답 계약 회귀를 제대로 못 잡습니다.수정 예시
- .andExpect(jsonPath("$.photoNames[1].status").value("EXTERNAL_API_ERROR")) + .andExpect(jsonPath("$.photoNames[1].status").value("FAILED"))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/howaboutus/backend/places/controller/PlaceControllerTest.java` around lines 767 - 769, The test in PlaceControllerTest incorrectly expects the photoNames[1].status to be "EXTERNAL_API_ERROR"; update the assertion for jsonPath("$.photoNames[1].status") to expect "FAILED" (leave jsonPath("$.photoNames[1].errorCode") as "EXTERNAL_API_ERROR") so the test verifies status vs errorCode correctly; locate the expectations block that contains .andExpect(jsonPath("$.photoNames[1].googlePlaceId")...) and change only the .andExpect for status to "FAILED".
🧹 Nitpick comments (2)
docs/ai/features.md (1)
91-93: 💤 Low value마크다운 린트 경고: blockquote 내부 빈 줄
92번 라인에 blockquote 내부에 빈 줄이 있어 markdownlint MD028 규칙 위반이 발생합니다. 의도적인 가독성 개선이라면 무시해도 되지만, 린트 정책을 따르려면 빈 줄을 제거하세요.
♻️ 제안 수정
> **Schedule 구조 변경 이벤트:** Schedule 생성/배치 생성/삭제/이동, `dayNumber` 지정 중간 삽입, Room `startDate`/`endDate` 실제 변경으로 인한 Schedule 재동기화는 `/topic/rooms/{roomId}/schedules` 채널에 `type: ROOM_SCHEDULES_RESYNCED`로 발행된다. 단, Schedule 이동에서 source와 target 일차가 같으면 이벤트를 발행하지 않는다. payload의 `scheduleId`, `itemId`, `affectedRouteItemIds`, `scheduleIds`는 모두 `null`이다. 클라이언트는 이 이벤트를 diff/patch로 반영하지 않고, 수신 시 `GET /rooms/{roomId}/schedules`를 호출해 schedule 목록을 재조회한다. title/destination만 변경된 경우에는 이 이벤트가 발행되지 않는다. ScheduleItem 생성/수정/삭제/같은 일자 내 순서 변경/다른 일자로 이동은 기존 상세 이벤트와 식별자 payload를 유지한다. 다른 일자로 이동할 때는 `SCHEDULE_ITEM_MOVED`를 발행하고, 클라이언트는 source/target schedule만 갱신하거나 필요한 구간 route를 재조회한다. - > **방 기간 변경 시 Schedule 자동 재동기화:** 방의 `startDate`/`endDate`가 변경되면 새 trip 일수에 맞춰 Schedule이 자동으로 재동기화된다. 새 trip 일수보다 `dayNumber`가 큰 Schedule은 자동 삭제되고, 자식 ScheduleItem은 DB FK `ON DELETE CASCADE`로 함께 정리된다. 부족한 일자에는 빈 Schedule이 자동 생성된다. 새 trip 일수가 30일(`SchedulePolicy.MAX_SCHEDULES_PER_ROOM`)을 초과하면 `SCHEDULE_LIMIT_EXCEEDED`로 거부된다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ai/features.md` around lines 91 - 93, Remove the empty blank line inside the blockquote that contains the "Schedule 구조 변경 이벤트" and "방 기간 변경 시 Schedule 자동 재동기화" paragraphs so the blockquote no longer violates markdownlint MD028; edit the quoted section in docs/ai/features.md by deleting the stray blank line between those two paragraphs (the one noted at line 92) to keep the content continuous and satisfy the linter.Source: Linters/SAST tools
src/main/java/com/howaboutus/backend/schedules/service/ScheduleItemService.java (1)
253-279: ⚡ Quick win배치 라우트 조회가
items × commands선형탐색으로 커집니다.현재 구현은 command마다 전체 items를 다시 순회합니다.
itemId -> index맵을 한 번 만들면 대량 요청에서 비용을 크게 줄일 수 있습니다.♻️ 제안 diff
+import java.util.HashMap; +import java.util.Map; @@ public List<RouteBatchItemResult> getRoutesForItems(UUID roomId, Long scheduleId, List<RouteBatchItemCommand> commands, Long userId) { @@ List<ScheduleItem> items = scheduleItemRepository.findAllBySchedule_IdOrderByOrderIndexAsc(scheduleId); + Map<Long, Integer> indexByItemId = new HashMap<>(items.size()); + for (int i = 0; i < items.size(); i++) { + indexByItemId.put(items.get(i).getId(), i); + } List<RouteBatchItemResult> results = new ArrayList<>(); for (RouteBatchItemCommand command : commands) { - results.add(getRouteForBatchItem(items, command)); + results.add(getRouteForBatchItem(items, indexByItemId, command)); } return results; } - private RouteBatchItemResult getRouteForBatchItem(List<ScheduleItem> items, RouteBatchItemCommand command) { + private RouteBatchItemResult getRouteForBatchItem(List<ScheduleItem> items, + Map<Long, Integer> indexByItemId, + RouteBatchItemCommand command) { String mode = TravelMode.normalize(command.travelMode()); - for (int i = 0; i < items.size(); i++) { - ScheduleItem current = items.get(i); - if (!current.getId().equals(command.itemId())) { - continue; - } - if (i + 1 >= items.size()) { - return RouteBatchItemResult.lastItem(command.itemId(), current.getId(), mode); - } - ScheduleItem next = items.get(i + 1); - return computeBatchRoute(command.itemId(), current, next, mode); + Integer idx = indexByItemId.get(command.itemId()); + if (idx == null) { + return RouteBatchItemResult.itemNotFound(command.itemId(), mode); } - return RouteBatchItemResult.itemNotFound(command.itemId(), mode); + ScheduleItem current = items.get(idx); + if (idx + 1 >= items.size()) { + return RouteBatchItemResult.lastItem(command.itemId(), current.getId(), mode); + } + ScheduleItem next = items.get(idx + 1); + return computeBatchRoute(command.itemId(), current, next, mode); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/howaboutus/backend/schedules/service/ScheduleItemService.java` around lines 253 - 279, The loop over commands currently calls getRouteForBatchItem which linearly scans items each time; change getRoutesForItems to build a one-time Map<UUID,Integer> (or Map<UUID,ScheduleItem>) from items (using scheduleItemRepository result) and then, for each RouteBatchItemCommand, look up the item index (or item) in that map and call a new/overloaded helper (e.g., computeBatchRoute or getRouteForBatchItem(Map...)) that uses the direct lookup to get current and next ScheduleItem — this removes the items×commands nested scan and preserves existing behavior in getRouteForBatchItem/computeBatchRoute.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/howaboutus/backend/places/service/PlacePreviewService.java`:
- Around line 73-79: In the lambda inside PlacePreviewService (the supplier that
calls placePhotoNameService.getFirstPhotoName and returns
PlacePreviewBatchItemResult), stop catching all RuntimeException; instead catch
only the specific external-error sentinel (CustomException) and convert that to
PlacePreviewBatchItemResult.failure(googlePlaceId, "EXTERNAL_API_ERROR"), and
rethrow any other exceptions (e.g., NPE) so they propagate unchanged to the
fetchMissingPreviews/join() path; keep the success path returning
PlacePreviewBatchItemResult.ok(preview.withPhotoName(photoName)).
---
Outside diff comments:
In
`@src/test/java/com/howaboutus/backend/places/controller/PlaceControllerTest.java`:
- Around line 767-769: The test in PlaceControllerTest incorrectly expects the
photoNames[1].status to be "EXTERNAL_API_ERROR"; update the assertion for
jsonPath("$.photoNames[1].status") to expect "FAILED" (leave
jsonPath("$.photoNames[1].errorCode") as "EXTERNAL_API_ERROR") so the test
verifies status vs errorCode correctly; locate the expectations block that
contains .andExpect(jsonPath("$.photoNames[1].googlePlaceId")...) and change
only the .andExpect for status to "FAILED".
---
Nitpick comments:
In `@docs/ai/features.md`:
- Around line 91-93: Remove the empty blank line inside the blockquote that
contains the "Schedule 구조 변경 이벤트" and "방 기간 변경 시 Schedule 자동 재동기화" paragraphs so
the blockquote no longer violates markdownlint MD028; edit the quoted section in
docs/ai/features.md by deleting the stray blank line between those two
paragraphs (the one noted at line 92) to keep the content continuous and satisfy
the linter.
In
`@src/main/java/com/howaboutus/backend/schedules/service/ScheduleItemService.java`:
- Around line 253-279: The loop over commands currently calls
getRouteForBatchItem which linearly scans items each time; change
getRoutesForItems to build a one-time Map<UUID,Integer> (or
Map<UUID,ScheduleItem>) from items (using scheduleItemRepository result) and
then, for each RouteBatchItemCommand, look up the item index (or item) in that
map and call a new/overloaded helper (e.g., computeBatchRoute or
getRouteForBatchItem(Map...)) that uses the direct lookup to get current and
next ScheduleItem — this removes the items×commands nested scan and preserves
existing behavior in getRouteForBatchItem/computeBatchRoute.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a85f6baa-545f-49c3-b258-c6e3ecb22b9a
📒 Files selected for processing (32)
docs/ai/erd.mddocs/ai/features.mddocs/superpowers/plans/2026-06-05-schedule-structure-events.mddocs/superpowers/plans/2026-06-07-pr-review-refactoring.mddocs/superpowers/specs/2026-06-05-room-dates-schedule-resync-design.mdsrc/main/java/com/howaboutus/backend/common/cache/RedisBulkCacheAccessor.javasrc/main/java/com/howaboutus/backend/common/ratelimit/HttpRateLimitPolicyResolver.javasrc/main/java/com/howaboutus/backend/places/controller/PlaceController.javasrc/main/java/com/howaboutus/backend/places/controller/dto/PlacePhotoBatchItemResponse.javasrc/main/java/com/howaboutus/backend/places/controller/dto/PlacePhotoResponse.javasrc/main/java/com/howaboutus/backend/places/controller/dto/PlacePreviewBatchItemResponse.javasrc/main/java/com/howaboutus/backend/places/service/PlacePhotoNameService.javasrc/main/java/com/howaboutus/backend/places/service/PlacePhotoService.javasrc/main/java/com/howaboutus/backend/places/service/PlacePreviewService.javasrc/main/java/com/howaboutus/backend/places/service/dto/PlacePhotoBatchItemResult.javasrc/main/java/com/howaboutus/backend/places/service/dto/PlacePreviewBatchItemResult.javasrc/main/java/com/howaboutus/backend/schedules/controller/ScheduleController.javasrc/main/java/com/howaboutus/backend/schedules/controller/dto/BatchRouteItemResponse.javasrc/main/java/com/howaboutus/backend/schedules/repository/ScheduleRepository.javasrc/main/java/com/howaboutus/backend/schedules/service/ScheduleItemService.javasrc/main/java/com/howaboutus/backend/schedules/service/ScheduleService.javasrc/main/java/com/howaboutus/backend/schedules/service/dto/RouteBatchItemResult.javasrc/main/resources/application.yamlsrc/test/java/com/howaboutus/backend/places/controller/PlaceControllerTest.javasrc/test/java/com/howaboutus/backend/places/service/PlaceDetailCachingTest.javasrc/test/java/com/howaboutus/backend/places/service/PlacePhotoCachingTest.javasrc/test/java/com/howaboutus/backend/places/service/PlacePhotoServiceTest.javasrc/test/java/com/howaboutus/backend/schedules/ScheduleIntegrationTest.javasrc/test/java/com/howaboutus/backend/schedules/controller/ScheduleControllerTest.javasrc/test/java/com/howaboutus/backend/schedules/controller/ScheduleRouteControllerTest.javasrc/test/java/com/howaboutus/backend/schedules/service/ScheduleItemServiceTest.javasrc/test/java/com/howaboutus/backend/schedules/service/ScheduleServiceTest.java
💤 Files with no reviewable changes (1)
- src/main/resources/application.yaml
✅ Files skipped from review due to trivial changes (3)
- src/main/java/com/howaboutus/backend/places/controller/dto/PlacePhotoResponse.java
- docs/superpowers/plans/2026-06-05-schedule-structure-events.md
- docs/ai/erd.md
🚧 Files skipped from review as they are similar to previous changes (16)
- src/main/java/com/howaboutus/backend/schedules/controller/dto/BatchRouteItemResponse.java
- src/main/java/com/howaboutus/backend/places/service/dto/PlacePhotoBatchItemResult.java
- src/main/java/com/howaboutus/backend/places/service/dto/PlacePreviewBatchItemResult.java
- src/main/java/com/howaboutus/backend/schedules/service/dto/RouteBatchItemResult.java
- src/main/java/com/howaboutus/backend/places/controller/dto/PlacePhotoBatchItemResponse.java
- src/test/java/com/howaboutus/backend/places/service/PlacePhotoServiceTest.java
- src/main/java/com/howaboutus/backend/places/controller/dto/PlacePreviewBatchItemResponse.java
- src/main/java/com/howaboutus/backend/common/ratelimit/HttpRateLimitPolicyResolver.java
- src/test/java/com/howaboutus/backend/schedules/service/ScheduleItemServiceTest.java
- src/main/java/com/howaboutus/backend/schedules/controller/ScheduleController.java
- src/test/java/com/howaboutus/backend/places/service/PlaceDetailCachingTest.java
- src/main/java/com/howaboutus/backend/common/cache/RedisBulkCacheAccessor.java
- src/main/java/com/howaboutus/backend/places/service/PlacePhotoService.java
- src/test/java/com/howaboutus/backend/places/service/PlacePhotoCachingTest.java
- src/main/java/com/howaboutus/backend/places/controller/PlaceController.java
- src/test/java/com/howaboutus/backend/schedules/controller/ScheduleRouteControllerTest.java
- 비동기 CompletableFuture join 시 외부 API 에러(EXTERNAL_API_ERROR) 복구 로직을 AsyncHelper로 공통화 - PlacePreviewService, PlacePhotoNameService, PlacePhotoService의 중복 예외 처리 및 join 헬퍼 메서드 제거 - Lombok 어노테이션(@NoArgsConstructor)을 활용한 AsyncHelper 인스턴스화 방지 처리
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
변경 내용
POST /rooms/{roomId}/schedules/{scheduleId}/routes/batch) 추가변경 이유
테스트
./gradlew build/review-code-against-docs스킬로 검증체크리스트
하네스 변경 체크리스트
Summary by CodeRabbit
새로운 기능
개선
문서