Conversation
Walkthrough스케줄러에서 오디오 동기화 작업의 자동 실행을 비활성화했고, 도장 인기도 조회용 JPA 리포지토리 메서드 3개를 추가했으며 서비스에서 하드코딩된 인기 스팟 ID를 최근 1개월 기준/누적 기준 조건으로 동적으로 선택하도록 변경했다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant SealService
participant UserSealRepository as Repository
Client->>SealService: getPopularSealSpotIds()
activate SealService
SealService->>SealService: startDate = now - 1 month\npageable = (0,4)
SealService->>Repository: countByCollectedAtAfter(startDate)
Repository-->>SealService: recentCount
alt recentCount <= 2
SealService->>Repository: findPopularSealSpotIds(pageable)
Repository-->>SealService: topIds(all-time)
else recentCount > 2
SealService->>Repository: findPopularSealSpotIdsByDate(pageable, startDate)
Repository-->>SealService: topIds(last-month)
end
SealService-->>Client: List<Long> topIds
deactivate SealService
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/yfive/gbjs/domain/guide/scheduler/AudioDataScheduler.java (1)
30-33: 빈 catch 블록과 사용되지 않는 savedCount 처리 필요예외를 삼키면 장애 탐지가 불가합니다. savedCount도 로그로 남기면 운영 가시성이 좋아집니다.
다음 패치를 제안합니다.
+import lombok.extern.slf4j.Slf4j; @@ -@Component -@RequiredArgsConstructor +@Component +@RequiredArgsConstructor +@Slf4j public class AudioDataScheduler { @@ public void syncAudioData() { try { - int savedCount = guideService.syncGyeongbukAudioStories(); + int savedCount = guideService.syncGyeongbukAudioStories(); + log.info("Audio sync completed. savedCount={}", savedCount); } catch (Exception e) { + log.error("Audio sync failed.", e); } }또한 클래스/주석의 “프로퍼티로 활성화” 설명은 현재 동작(자동 비활성화)과 일부 불일치합니다. 주석 갱신을 권장합니다.
src/main/java/com/yfive/gbjs/domain/seal/service/SealServiceImpl.java (1)
514-519: 정렬 보장 이슈: findAllById는 입력 ID 순서를 보장하지 않습니다집계 결과의 인기 순서가 응답에서 뒤섞일 수 있습니다. ID→엔티티 맵으로 재정렬하세요.
다음 패치를 제안합니다.
- List<SealSpot> popularSealSpots = sealSpotRepository.findAllById(popularSealSpotIds); - - return popularSealSpots.stream() - .map(sealConverter::toPopularSealSpotDTO) - .collect(Collectors.toList()); + List<SealSpot> popularSealSpots = sealSpotRepository.findAllById(popularSealSpotIds); + java.util.Map<Long, SealSpot> byId = + popularSealSpots.stream().collect(Collectors.toMap(SealSpot::getId, s -> s)); + + return popularSealSpotIds.stream() + .map(byId::get) + .filter(java.util.Objects::nonNull) + .map(sealConverter::toPopularSealSpotDTO) + .collect(Collectors.toList());
🧹 Nitpick comments (6)
src/main/java/com/yfive/gbjs/domain/guide/scheduler/AudioDataScheduler.java (1)
27-27: 자동 스케줄 비활성화로 @ConditionalOnProperty 의미가 약해졌습니다.자동 실행을 끈 것이 의도라면 OK. 다만 이 클래스는 사실상 “수동 호출용 빈”만 남습니다. 향후 재활성화를 쉽게 하려면 프로퍼티로 토글 가능한 @scheduled를 유지하는 방안을 고려하세요.
다음과 같이 크론/활성화를 프로퍼티로 묶어두면 필요 시 설정만으로 복구됩니다(현재 PR에서 바로 적용 필요 없음).
+import org.springframework.scheduling.annotation.Scheduled; @@ -// @Scheduled(cron = "0 0 2 * * ?") +@Scheduled(cron = "${audio.sync.scheduler.cron:0 0 2 * * ?}")src/main/java/com/yfive/gbjs/domain/seal/service/SealServiceImpl.java (1)
498-512: 인기 기준 전환 로직의 매직 넘버(4, 1개월, ≤2) 하드코딩운영 중 조정이 잦을 값들입니다. 상수/설정화(예: application.yml)하고, 시간 기준은 Clock 주입으로 테스트 가능하게 만드는 것을 권장합니다.
예: popular.limit, popular.recent-window-months, popular.recent-threshold(≤2) 프로퍼티 도입 후 사용.
src/main/java/com/yfive/gbjs/domain/seal/repository/UserSealRepository.java (4)
88-91: 인기 집계 쿼리의 정확도/결정성 개선: 수집여부 필터와 타이브레이커 추가 권장현재는 수집 플래그와 무관하게 집계되고, 동률 시 DB가 임의 순서를 낼 수 있습니다. 운영 일관성을 위해 보완하세요.
- @Query( - "SELECT us.seal.sealSpot.id FROM UserSeal us WHERE us.seal.sealSpot.id IS NOT NULL GROUP BY us.seal.sealSpot.id ORDER BY COUNT(us.seal.sealSpot.id) DESC") + @Query( + "SELECT us.seal.sealSpot.id " + + "FROM UserSeal us " + + "WHERE us.seal.sealSpot.id IS NOT NULL " + + " AND us.collected = true " + + "GROUP BY us.seal.sealSpot.id " + + "ORDER BY COUNT(us.id) DESC, us.seal.sealSpot.id ASC") List<Long> findPopularSealSpotIds(org.springframework.data.domain.Pageable pageable);
99-104: 기간 한정 인기 집계도 동일 보완 필요수집 플래그와 타이브레이커를 동일하게 적용하세요.
- @Query( - "SELECT us.seal.sealSpot.id FROM UserSeal us WHERE us.collectedAt >= :startDate AND us.seal.sealSpot.id IS NOT NULL GROUP BY us.seal.sealSpot.id ORDER BY COUNT(us.seal.sealSpot.id) DESC") + @Query( + "SELECT us.seal.sealSpot.id " + + "FROM UserSeal us " + + "WHERE us.collectedAt >= :startDate " + + " AND us.seal.sealSpot.id IS NOT NULL " + + " AND us.collected = true " + + "GROUP BY us.seal.sealSpot.id " + + "ORDER BY COUNT(us.id) DESC, us.seal.sealSpot.id ASC") List<Long> findPopularSealSpotIdsByDate( org.springframework.data.domain.Pageable pageable, @Param("startDate") java.time.LocalDateTime startDate);
111-111: ‘After(>)’ vs ‘>=’ 불일치서비스는 countByCollectedAtAfter(startDate)로 “startDate 이후”를 계산(의미상 >)하지만, 위 기간 집계 쿼리는 ‘>=’입니다. 경계 포함 여부를 통일하세요(쿼리를 ‘>’로 바꾸거나, 메서드명을 GreaterThanEqual에 맞추기).
82-112: 집계 성능 관점 조언: 인덱스 확인 권장대량 데이터에서 GROUP BY/일자 필터 성능을 위해 아래 인덱스 존재 여부를 점검하세요.
- user_seal(collected_at)
- user_seal(seal_id)
- seal(seal_spot_id)
필요 시 복합 인덱스(user_seal.seal_id, collected_at)도 고려.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/yfive/gbjs/domain/guide/scheduler/AudioDataScheduler.java(1 hunks)src/main/java/com/yfive/gbjs/domain/seal/repository/UserSealRepository.java(1 hunks)src/main/java/com/yfive/gbjs/domain/seal/service/SealServiceImpl.java(1 hunks)
Test Results0 tests 0 ✅ 0s ⏱️ Results for commit e81c8f8. |
1 participant
Summary by CodeRabbit
신규 기능
기타