[volume-10] Collect, Stack, Zip#229
[volume-10] Collect, Stack, Zip#229kilian-develop wants to merge 13 commits intoLoopers-dev-lab:kilian-developfrom
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough상품 메트릭 추적을 일일/주간/월간 단위로 분리하고, 배치 처리로 주간/월간 집계 데이터를 생성하며, 랭킹 조회 API에 타입 파라미터를 추가하여 전략 기반 랭킹 선택을 지원합니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant API as ProductV1Api
participant Service as GetProductRankingService
participant Selector as GetProductRankingsStrategySelector
participant Strategy as GetProductRankingsStrategy
participant Repository as Metric Repository
participant Cache as Cache Repository
User->>API: getProductRankings(date, "WEEKLY")
API->>Service: getRanking(GetProductRankingQuery)
Service->>Selector: select("WEEKLY")
Selector->>Strategy: [filter by support]
Strategy-->>Selector: GetWeeklyProductRankingsStrategy
Selector-->>Service: WeeklyStrategy
Service->>Strategy: getRankings(date, page, size)
Strategy->>Repository: findRankingsBy(date, weights, pageable)
Repository->>Repository: Query aggregated metrics
Repository-->>Strategy: ProductRankings
Strategy-->>Service: ProductRankings
Service-->>API: ApiResponse
API-->>User: Rankings JSON
sequenceDiagram
actor Scheduler
participant MonthlyScheduler as MonthlyProductMetricScheduler
participant JobLauncher
participant Job as monthlyProductMetricJob
participant Partitioner as MonthlyProductMetricBatchPartitioner
participant Reader as MonthlyProductMetricBatchReader
participant Writer as MonthlyProductMetricBatchWriter
participant DailyRepo as DailyProductMetricRepository
participant MonthlyRepo as MonthlyProductMetricRepository
rect rgb(220, 240, 255)
Note over MonthlyScheduler: 매월 1일 오전 2시
Scheduler->>MonthlyScheduler: 스케줄 실행
end
MonthlyScheduler->>MonthlyScheduler: 지난달 startDate, endDate 계산
MonthlyScheduler->>JobLauncher: JobParameters 전달 (startDate, endDate)
JobLauncher->>Job: 월간 배치 작업 시작
rect rgb(240, 255, 240)
Note over Job: 파티셔닝 단계
Job->>Partitioner: partition(gridSize)
Partitioner->>DailyRepo: countDistinctProductIdsBy(date range)
DailyRepo-->>Partitioner: 총 상품 수
Partitioner-->>Job: 파티션 맵 (partition0...partitionN)
end
par Partition Processing
rect rgb(255, 240, 220)
Note over Reader: 각 파티션 병렬 처리
Job->>Reader: open(ExecutionContext)
Reader->>DailyRepo: findAggregatedBy(date, offset, limit)
DailyRepo-->>Reader: ProductMetricAggregation 리스트
end
loop 각 집계 항목
Job->>Reader: read()
Reader-->>Job: ProductMetricAggregation
Job->>Writer: write(chunk)
Writer->>Writer: to(YearMonth) 변환
Writer->>MonthlyRepo: bulkUpsert(MonthlyProductMetric)
end
end
Job-->>JobLauncher: 작업 완료
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/entity/DailyProductMetricEntity.java (1)
62-72:to()메서드에서id가 null일 경우 NPE 발생 가능성.
from()메서드에서는Optional.ofNullable()로 null ID를 처리하지만,to()메서드의this.id.toString()(Line 64)은 저장되지 않은 엔티티의 경우NullPointerException을 발생시킬 수 있습니다.🔎 제안된 수정 사항
public DailyProductMetric to() { return DailyProductMetric.mappedBy( - new DailyProductMetricId(this.id.toString()), + new DailyProductMetricId(Optional.ofNullable(this.id).map(Object::toString).orElse(null)), new ProductId(this.productId.toString()), new ProductLikeCount(this.likeCount), new ProductTotalSalesCount(this.totalSalesCount), new ProductDetailViewCount(this.viewCount), new CreatedAt(this.createdAt), new UpdatedAt(this.updatedAt) ); }
🟡 Minor comments (3)
core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/MonthlyProductMetricBulkRepository.java-9-9 (1)
9-9: 인터페이스 수준에서 null/empty 처리 계약을 명시적으로 문서화하세요구현체에서 null 또는 빈 리스트에 대해 적절히 처리하고 있으나(라인 22-24), 인터페이스 메서드에
@NonNull어노테이션이나 JavaDoc이 없어 호출자가 이 동작을 인식하기 어렵습니다. 메서드 파라미터에@NonNull을 추가하거나 JavaDoc으로 null/empty 리스트 시 동작을 명시하는 것을 권장합니다.core/service/src/main/java/com/loopers/core/service/product/IncreaseProductTotalSalesService.java-40-43 (1)
40-43: 루프 외부에서 타임스탬프를 추출하세요.
forEach루프 내에서 매 iteration마다new CreatedAt(LocalDateTime.now())를 호출하면, 동일한 주문의 여러 상품들이 서로 다른 일자의 DailyProductMetric에 집계될 수 있습니다. 타임스탬프를 루프 전에 한 번 생성하고 재사용하세요:LocalDateTime now = LocalDateTime.now(); orderItemRepository.findAllByOrderId(order.getId()).forEach(orderItem -> { DailyProductMetric metric = dailyProductMetricRepository.findByWithLock(orderItem.getProductId(), new CreatedAt(now)) .orElse(DailyProductMetric.init(orderItem.getProductId())); dailyProductMetricRepository.save(metric.increaseSalesCount(orderItem.getQuantity())); });또는 이미 존재하는
CreatedAt.now()헬퍼 메서드를 사용할 수 있습니다.core/domain/src/main/java/com/loopers/core/domain/product/DailyProductMetric.java-62-66 (1)
62-66:increaseLikeCount()에서updatedAt갱신 누락
increaseViewCount()와increaseSalesCount()는updatedAt(UpdatedAt.now())를 호출하여 수정 시간을 갱신하지만,increaseLikeCount()는 이를 수행하지 않습니다. 일관성을 위해 동일하게 갱신해야 합니다.🔎 제안된 수정
public DailyProductMetric increaseLikeCount() { return this.toBuilder() .likeCount(this.likeCount.increase()) + .updatedAt(UpdatedAt.now()) .build(); }
🧹 Nitpick comments (32)
settings.gradle.kts (1)
43-43: 모듈 선언을 기존 include 블록에 통합하세요.새로운
commerce-batch모듈이 별도의include()문으로 추가되었습니다. 일관성과 유지보수성을 위해 3-20번째 줄의 기존include()블록에 함께 선언하는 것이 좋습니다.🔎 권장하는 수정안
include( ":apps:commerce-api", ":apps:commerce-streamer", + ":apps:commerce-batch", ":supports:jackson", ":supports:logging", ":supports:monitoring", ":core:domain", ":core:service", ":core:infra:database:mysql:mysql-config", ":core:infra:database:mysql", ":core:infra:database:redis:redis-config", ":core:infra:database:redis", ":core:infra:http-client:http-client-config", ":core:infra:http-client", ":core:infra:event", ":core:infra:event:kafka-config", ":core:common" ) // configurations pluginManagement { val springBootVersion: String by settings val springDependencyManagementVersion: String by settings repositories { maven { url = uri("https://repo.spring.io/milestone") } maven { url = uri("https://repo.spring.io/snapshot") } gradlePluginPortal() } resolutionStrategy { eachPlugin { when (requested.id.id) { "org.springframework.boot" -> useVersion(springBootVersion) "io.spring.dependency-management" -> useVersion(springDependencyManagementVersion) } } } } - -include("apps:commerce-batch")apps/commerce-batch/src/main/resources/application.yml (2)
64-86: 환경별 설정 추가를 권장합니다.현재 프로필 섹션들이 대부분 비어있습니다. 각 환경(dev, qa, prd)에 맞는 데이터베이스 URL, 외부 서비스 엔드포인트, 배치 파티션 크기 등 환경별 설정을 추가하는 것을 권장합니다.
예를 들어:
- 데이터베이스 연결 정보
- 외부 API 엔드포인트 (위에서 언급한 PG 시뮬레이터, 콜백 URL)
- 배치 청크 크기 및 파티션 그리드 크기 (환경 리소스에 따라 조정)
- 로깅 레벨 조정
88-90: SpringDoc 설정 위치를 명확히 하세요.
springdoc설정이 프로덕션 프로필 섹션 뒤에 위치하여 혼란을 줄 수 있습니다. 현재 들여쓰기 기준으로는 전역 설정이지만, 위치상 prd 프로필과 연관된 것처럼 보입니다.API 문서를 모든 환경에서 비활성화하려면 전역 설정 영역(line 13-35 부근)으로 이동하거나, 특정 프로필에서만 비활성화하려면 해당 프로필 섹션 내부로 이동하세요.
🔎 전역 설정으로 이동 제안
batch: jdbc: initialize-schema: always job: enabled: false + +springdoc: + api-docs: + enabled: false + http-client: pg-simulator:그리고 현재 위치에서 제거:
spring: config: activate: on-profile: prd - -springdoc: - api-docs: - enabled: falsecore/domain/src/testFixtures/java/com/loopers/core/domain/product/DailyProductMetricFixture.java (1)
14-66: 코드 중복 제거를 통한 유지보수성 개선 권장네 개의 팩토리 메서드가 유사한 Instancio 설정을 반복하고 있습니다. 공통 로직을 private 헬퍼 메서드로 추출하면 유지보수성이 향상되고 향후 필드 변경 시 수정 범위를 줄일 수 있습니다.
🔎 리팩토링 제안
public class DailyProductMetricFixture { + + private static DailyProductMetric buildBase( + String productId, + long likeCount, + long viewCount, + long totalSalesCount, + LocalDateTime createdAt + ) { + return Instancio.of(DailyProductMetric.class) + .set(field(DailyProductMetric::getId), DailyProductMetricId.empty()) + .set(field(DailyProductMetric::getProductId), new ProductId(productId)) + .set(field(DailyProductMetric::getLikeCount), new ProductLikeCount(likeCount)) + .set(field(DailyProductMetric::getTotalSalesCount), new ProductTotalSalesCount(totalSalesCount)) + .set(field(DailyProductMetric::getViewCount), new ProductDetailViewCount(viewCount)) + .set(field(DailyProductMetric::getCreatedAt), new CreatedAt(createdAt)) + .set(field(DailyProductMetric::getUpdatedAt), new UpdatedAt(createdAt)) + .create(); + } public static DailyProductMetric create() { - return Instancio.of(DailyProductMetric.class) - .set(field(DailyProductMetric::getId), DailyProductMetricId.empty()) - .set(field(DailyProductMetric::getProductId), new ProductId("1")) - .set(field(DailyProductMetric::getLikeCount), new ProductLikeCount(0L)) - .set(field(DailyProductMetric::getTotalSalesCount), new ProductTotalSalesCount(0L)) - .set(field(DailyProductMetric::getViewCount), new ProductDetailViewCount(0L)) - .set(field(DailyProductMetric::getCreatedAt), new CreatedAt(LocalDateTime.now())) - .set(field(DailyProductMetric::getUpdatedAt), new UpdatedAt(LocalDateTime.now())) - .create(); + return buildBase("1", 0L, 0L, 0L, LocalDateTime.now()); } public static DailyProductMetric createWith(String productId) { - return Instancio.of(DailyProductMetric.class) - .set(field(DailyProductMetric::getId), DailyProductMetricId.empty()) - .set(field(DailyProductMetric::getProductId), new ProductId(productId)) - .set(field(DailyProductMetric::getLikeCount), new ProductLikeCount(0L)) - .set(field(DailyProductMetric::getTotalSalesCount), new ProductTotalSalesCount(0L)) - .set(field(DailyProductMetric::getViewCount), new ProductDetailViewCount(0L)) - .set(field(DailyProductMetric::getCreatedAt), new CreatedAt(LocalDateTime.now())) - .set(field(DailyProductMetric::getUpdatedAt), new UpdatedAt(LocalDateTime.now())) - .create(); + return buildBase(productId, 0L, 0L, 0L, LocalDateTime.now()); } public static DailyProductMetric createWith(String productId, LocalDateTime createdAt) { - return Instancio.of(DailyProductMetric.class) - .set(field(DailyProductMetric::getId), DailyProductMetricId.empty()) - .set(field(DailyProductMetric::getProductId), new ProductId(productId)) - .set(field(DailyProductMetric::getLikeCount), new ProductLikeCount(0L)) - .set(field(DailyProductMetric::getTotalSalesCount), new ProductTotalSalesCount(0L)) - .set(field(DailyProductMetric::getViewCount), new ProductDetailViewCount(0L)) - .set(field(DailyProductMetric::getCreatedAt), new CreatedAt(createdAt)) - .set(field(DailyProductMetric::getUpdatedAt), new UpdatedAt(createdAt)) - .create(); + return buildBase(productId, 0L, 0L, 0L, createdAt); } public static DailyProductMetric createWith( String productId, long likeCount, long viewCount, long totalSalesCount, LocalDateTime createdAt ) { - return Instancio.of(DailyProductMetric.class) - .set(field(DailyProductMetric::getId), DailyProductMetricId.empty()) - .set(field(DailyProductMetric::getProductId), new ProductId(productId)) - .set(field(DailyProductMetric::getLikeCount), new ProductLikeCount(likeCount)) - .set(field(DailyProductMetric::getTotalSalesCount), new ProductTotalSalesCount(totalSalesCount)) - .set(field(DailyProductMetric::getViewCount), new ProductDetailViewCount(viewCount)) - .set(field(DailyProductMetric::getCreatedAt), new CreatedAt(createdAt)) - .set(field(DailyProductMetric::getUpdatedAt), new UpdatedAt(createdAt)) - .create(); + return buildBase(productId, likeCount, viewCount, totalSalesCount, createdAt); } }core/service/src/main/java/com/loopers/core/service/product/command/IncreaseProductLikeMetricCommand.java (1)
3-7: eventId와 productId에 대한 null 검증 고려레코드가 null 값을 허용하면 서비스 레이어에서 NullPointerException이 발생할 수 있습니다. eventId는 멱등성 보장에 사용되고 productId는 메트릭 식별에 필수적이므로, compact 생성자에서 null 검증을 추가하는 것이 안전합니다.
🔎 null 검증 추가 제안
public record IncreaseProductLikeMetricCommand( String eventId, String productId ) { + public IncreaseProductLikeMetricCommand { + if (eventId == null || eventId.isBlank()) { + throw new IllegalArgumentException("eventId must not be null or blank"); + } + if (productId == null || productId.isBlank()) { + throw new IllegalArgumentException("productId must not be null or blank"); + } + } }apps/commerce-batch/src/test/java/com/loopers/application/batch/IntegrationTest.java (1)
10-12: 베이스 클래스로 사용될 경우 abstract 선언 고려IntegrationTest가 다른 통합 테스트 클래스들(예: MonthlyProductMetricSchedulerTest, WeeklyProductMetricSchedulerTest)의 베이스 클래스로 사용될 경우, abstract로 선언하여 직접 인스턴스화되는 것을 방지하는 것이 좋습니다.
🔎 abstract 클래스로 변경 제안
@SpringBootTest @ExtendWith(MySqlTestContainersExtension.class) -public class IntegrationTest { +public abstract class IntegrationTest {apps/commerce-streamer/src/main/java/com/loopers/applications/streamer/consumer/product/dto/IncreaseProductLikeMetricEvent.java (1)
5-8: 커맨드와 동일한 null 검증 고려IncreaseProductLikeMetricCommand에 검증을 추가하는 경우, 이벤트 레코드에서도 일관성을 위해 동일한 검증을 수행하는 것이 좋습니다. Kafka에서 역직렬화된 데이터의 무결성을 보장할 수 있습니다.
core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/WeeklyProductMetricBulkRepository.java (1)
7-10: 반환 타입 및 문서화 개선 고려현재 void 반환 타입은 upsert 작업의 결과(예: 영향받은 행 수, 성공/실패 여부)를 확인할 수 없습니다. 다음 개선 사항을 고려해보세요:
- 메서드 JavaDoc 추가 (빈 리스트 처리, 트랜잭션 동작, 예외 발생 조건 등)
- 필요시 영향받은 행 수를 반환하도록 반환 타입 변경 (예:
int또는BulkUpsertResult)🔎 문서화 예시
public interface WeeklyProductMetricBulkRepository { + /** + * 주간 상품 메트릭 목록을 일괄 삽입 또는 업데이트합니다. + * + * @param weeklyProductMetrics upsert할 주간 메트릭 엔티티 목록 (빈 리스트의 경우 아무 작업도 수행하지 않음) + * @throws org.springframework.dao.DataAccessException 데이터베이스 오류 발생 시 + */ void bulkUpsert(List<WeeklyProductMetricEntity> weeklyProductMetrics); }core/service/src/main/java/com/loopers/core/service/product/query/GetProductRankingQuery.java (1)
7-7: type 파라미터의 유효성 검증 또는 문서화를 고려하세요.type 파라미터는 전략 선택에 사용되지만 현재 검증이나 제약이 없습니다. 다음 중 하나를 고려해보세요:
- 유효한 타입 값들을 enum으로 정의 (타입 안정성)
- 생성자에서 유효성 검증 로직 추가
- 최소한 Javadoc으로 유효한 값들을 명시 (예: "daily", "weekly", "monthly")
현재는 잘못된 type 값이 전달되어도 쿼리 생성 시점에 감지되지 않습니다.
🔎 Enum 기반 리팩토링 제안
package com.loopers.core.service.product.query; import java.time.LocalDate; +public enum RankingType { + DAILY, WEEKLY, MONTHLY +} + public record GetProductRankingQuery( LocalDate date, - String type, + RankingType type, int pageNo, int pageSize ) { }core/service/src/main/java/com/loopers/core/service/product/component/GetProductRankingsStrategy.java (1)
7-12: LGTM: 전략 패턴이 깔끔하게 구현되었습니다.인터페이스 설계가 명확하고 단일 책임 원칙을 잘 따르고 있습니다.
supports()와getRankings()의 분리가 전략 선택과 실행을 명확히 구분합니다.선택적으로, 다음을 문서화하면 더 좋습니다:
supports()메서드가 어떤 type 값들을 지원해야 하는지getRankings()의 반환값인 ProductRankings의 구조- 여러 전략이 동일한 type을 지원할 경우의 우선순위
apps/commerce-batch/src/test/resources/application-test.yml (1)
53-61: 배치 설정이 적절합니다.chunk 크기 100과 partition grid-size 4는 테스트 환경에 적합한 값입니다.
선택적으로, weekly와 monthly의 설정이 동일하므로 공통 설정으로 추출하여 중복을 제거할 수 있습니다.
apps/commerce-batch/src/main/java/com/loopers/application/batch/product/MonthlyProductMetricScheduler.java (2)
27-29: 타임존 처리를 명시적으로 지정하는 것을 고려하세요.
YearMonth.now()는 시스템 기본 타임존을 사용하므로, 서버 환경에 따라 다른 결과를 초래할 수 있습니다. 배치 작업의 일관성을 위해 명시적인 타임존(예: UTC)을 사용하는 것이 좋습니다.🔎 명시적 타임존 사용 제안
+import java.time.ZoneId; + @Scheduled(cron = "0 0 2 1 * ?") public void run() { - YearMonth lastMonth = YearMonth.now().minusMonths(1); + YearMonth lastMonth = YearMonth.now(ZoneId.of("Asia/Seoul")).minusMonths(1); LocalDate startDate = lastMonth.atDay(1); LocalDate endDate = lastMonth.atEndOfMonth();또는 UTC를 사용하려면:
+import java.time.ZoneOffset; + - YearMonth lastMonth = YearMonth.now().minusMonths(1); + YearMonth lastMonth = YearMonth.now(ZoneOffset.UTC).minusMonths(1);
25-25: 스케줄 실행 타임존을 명시하는 것을 고려하세요.cron 표현식에 타임존이 지정되지 않아 서버의 기본 타임존을 사용합니다. 일관된 실행을 보장하기 위해
zone속성을 추가하는 것을 권장합니다.🔎 타임존 지정 제안
- @Scheduled(cron = "0 0 2 1 * ?") + @Scheduled(cron = "0 0 2 1 * ?", zone = "Asia/Seoul") public void run() {core/service/src/main/java/com/loopers/core/service/product/component/GetProductRankingsStrategySelector.java (1)
14-19: null 안전성과 예외 처리를 개선하세요.현재 구현에 다음 개선이 필요합니다:
- Null 파라미터 처리:
type이 null인 경우supports()호출 시 NPE가 발생할 수 있습니다.- 예외 타입: 클라이언트 오류에 대해
IllegalArgumentException대신 도메인별 예외를 사용하는 것이 좋습니다.- 관찰성: 전략 선택 실패 시 로깅이 없어 디버깅이 어렵습니다.
🔎 개선 사항 제안
+import lombok.extern.slf4j.Slf4j; +import java.util.Objects; + +@Slf4j @Component @RequiredArgsConstructor public class GetProductRankingsStrategySelector { private final List<GetProductRankingsStrategy> strategies; public GetProductRankingsStrategy select(String type) { + Objects.requireNonNull(type, "랭킹 타입은 null일 수 없습니다."); + return strategies.stream() .filter(strategy -> strategy.supports(type)) .findFirst() - .orElseThrow(() -> new IllegalArgumentException("지원하지 않는 타입의 랭킹입니다.")); + .orElseThrow(() -> { + log.error("Unsupported ranking type requested: {}", type); + return new IllegalArgumentException("지원하지 않는 타입의 랭킹입니다: " + type); + }); } }더 나은 방법으로는, 프로젝트에 도메인 예외 클래스(예:
UnsupportedRankingTypeException)를 정의하여 사용하는 것을 고려하세요.apps/commerce-api/src/main/java/com/loopers/application/api/product/ProductV1ApiSpec.java (1)
44-44:type파라미터에 대한 Swagger 문서를 추가하고, API 수준에서의 유효성 검증을 고려하세요.
type파라미터는 허용되는 값("DAILY", "WEEKLY", "MONTHLY")에 대한 Swagger 문서가 없습니다. 또한 컨트롤러 레벨에서 유효성 검증 어노테이션이 없어, API 사용자가 유효한 값을 사전에 파악하기 어렵습니다. 서비스 레이어에서 런타임 검증이 존재하지만, API 문서 개선과 입력값 검증 강화는 API 사용성을 크게 개선할 수 있습니다.🔎 개선 방안
@Operation( summary = "상품 랭킹 조회", - description = "상품 랭킹을 조회합니다." + description = "상품 랭킹을 조회합니다. type은 'DAILY', 'WEEKLY', 'MONTHLY' 중 하나를 지정할 수 있습니다." ) ApiResponse<GetProductRankingsResponse> getProductRankings(LocalDate date, String type, int pageNo, int pageSize);실제 구현체(
ProductV1Api)에서@RequestParam에@Pattern어노테이션을 추가하거나 enum을 사용하여 컴파일 타임에 검증하는 것을 고려하세요.core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/MonthlyProductMetricBulkRepositoryImpl.java (1)
21-52: 벌크 업서트 구현이 올바릅니다.ON DUPLICATE KEY UPDATE를 사용한 벌크 업서트 로직이 정확하며,
createdAt은 보존하고 메트릭 필드들과updatedAt만 갱신하는 것이 적절합니다.다만
WeeklyProductMetricBulkRepositoryImpl과 거의 동일한 구조로 코드 중복이 발생하고 있습니다. 다음과 같이 공통 추상 클래스나 템플릿 메서드 패턴을 고려해보세요:🔎 제안: 추상 클래스를 통한 중복 제거
공통 로직을 추상화하는 베이스 클래스를 만들 수 있습니다:
public abstract class AbstractProductMetricBulkRepository<T> { protected final NamedParameterJdbcTemplate jdbcTemplate; protected AbstractProductMetricBulkRepository(NamedParameterJdbcTemplate jdbcTemplate) { this.jdbcTemplate = jdbcTemplate; } protected void executeBulkUpsert(List<T> metrics, String sql, Function<T, SqlParameterSource> mapper) { if (Objects.isNull(metrics) || metrics.isEmpty()) { return; } SqlParameterSource[] batch = metrics.stream() .map(mapper) .toArray(SqlParameterSource[]::new); jdbcTemplate.batchUpdate(sql, batch); } protected abstract String getUpsertSql(); protected abstract SqlParameterSource toParameterSource(T metric); }그런 다음 각 구현체에서 SQL과 매핑 로직만 제공하면 됩니다.
core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/WeeklyProductMetricBulkRepositoryImpl.java (1)
21-53: 벌크 업서트 로직이 올바르게 구현되었습니다.ON DUPLICATE KEY UPDATE를 사용하여 메트릭을 효율적으로 업서트하고 있으며,
createdAt보존과 메트릭 필드 갱신 로직이 적절합니다.
MonthlyProductMetricBulkRepositoryImpl과 동일한 구조의 코드 중복이 발생하고 있습니다. 월간 메트릭 리뷰 코멘트에서 제안한 것처럼, 공통 추상 클래스를 통해 중복을 제거하는 것을 고려하세요.apps/commerce-batch/src/main/java/com/loopers/application/batch/product/WeeklyProductMetricBatchReader.java (1)
22-30: ExecutionContext 파라미터 검증을 고려하세요.
startDate,endDate,partitionOffset,partitionLimit파라미터를 ExecutionContext에서 읽을 때 null 체크나 예외 처리가 없습니다. 배치 설정 오류 시 명확하지 않은 예외가 발생할 수 있습니다. 필수 파라미터 검증을 추가하면 배치 실패 시 원인 파악이 더 쉬워집니다.🔎 방어적 검증 추가 제안
@Override public void open(@NonNull ExecutionContext executionContext) { + Objects.requireNonNull(executionContext.getString("startDate"), "startDate is required"); + Objects.requireNonNull(executionContext.getString("endDate"), "endDate is required"); LocalDate startDate = LocalDate.parse(executionContext.getString("startDate")); LocalDate endDate = LocalDate.parse(executionContext.getString("endDate")); long partitionOffset = executionContext.getLong("partitionOffset"); long partitionLimit = executionContext.getLong("partitionLimit"); + + if (partitionLimit <= 0) { + throw new IllegalArgumentException("partitionLimit must be positive"); + } this.iterator = dailyProductMetricRepository.findAggregatedBy(startDate, endDate, partitionOffset, partitionLimit) .iterator(); }apps/commerce-batch/src/test/java/com/loopers/application/batch/product/WeeklyProductMetricSchedulerTest.java (1)
142-142: 불필요한 주석 코드 정리 필요주석 처리된 코드가 남아있습니다. 정리해 주세요.
🔎 수정 제안
-// // 상품 1: 7일 동안 좋아요 10씩 (총 70) + // 상품 1: 7일 동안 좋아요 10씩 (총 70)core/domain/src/main/java/com/loopers/core/domain/product/MonthlyProductMetric.java (1)
8-45:WeeklyProductMetric과의 일관성 개선 제안
WeeklyProductMetric은@AllArgsConstructor(access = AccessLevel.PRIVATE)를 사용하여 생성자를 자동 생성하는 반면, 이 클래스는 수동으로 private 생성자를 정의했습니다. 기능상 동일하지만, 코드베이스 일관성을 위해 Lombok 어노테이션 사용을 고려해 보세요.🔎 수정 제안
import com.loopers.core.domain.common.vo.CreatedAt; import com.loopers.core.domain.common.vo.UpdatedAt; import com.loopers.core.domain.product.vo.*; +import lombok.AccessLevel; +import lombok.AllArgsConstructor; import lombok.Getter; @Getter +@AllArgsConstructor(access = AccessLevel.PRIVATE) public class MonthlyProductMetric { private final MonthlyProductMetricId id; - private final ProductId productId; - private final ProductLikeCount likeCount; - private final ProductDetailViewCount viewCount; - private final ProductTotalSalesCount totalSalesCount; - private final YearMonth yearMonth; - private final CreatedAt createdAt; - private final UpdatedAt updatedAt; - private MonthlyProductMetric( - MonthlyProductMetricId id, - ProductId productId, - ProductLikeCount likeCount, - ProductDetailViewCount viewCount, - ProductTotalSalesCount totalSalesCount, - YearMonth yearMonth, - CreatedAt createdAt, - UpdatedAt updatedAt - ) { - this.id = id; - this.productId = productId; - this.likeCount = likeCount; - this.viewCount = viewCount; - this.totalSalesCount = totalSalesCount; - this.yearMonth = yearMonth; - this.createdAt = createdAt; - this.updatedAt = updatedAt; - } - public static MonthlyProductMetric create(apps/commerce-batch/src/main/java/com/loopers/application/batch/product/MonthlyProductMetricBatchReader.java (1)
31-38:@Nullable어노테이션 누락
WeeklyProductMetricBatchReader의read()메서드에는@Nullable어노테이션이 있지만, 이 클래스에는 누락되어 있습니다. 일관성을 위해 추가하는 것이 좋습니다.🔎 제안된 수정
+import org.springframework.lang.Nullable; + @RequiredArgsConstructor public class MonthlyProductMetricBatchReader implements ItemStreamReader<ProductMetricAggregation> { // ... + @Nullable @Override public ProductMetricAggregation read() { if (Objects.isNull(iterator) || !iterator.hasNext()) { return null; } return iterator.next(); } }core/service/src/main/java/com/loopers/core/service/product/component/GetMonthlyProductRankingsStrategy.java (1)
26-29:supports()메서드의 NullPointerException 가능성
type파라미터가null로 전달될 경우type.equals("MONTHLY")에서 NPE가 발생할 수 있습니다.GetWeeklyProductRankingsStrategy도 동일한 패턴을 사용하고 있지만, 방어적 코딩을 고려해볼 수 있습니다.🔎 제안된 수정
@Override public boolean supports(String type) { - return type.equals("MONTHLY"); + return "MONTHLY".equals(type); }core/service/src/main/java/com/loopers/core/service/product/component/GetWeeklyProductRankingsStrategy.java (1)
26-29:type이 null인 경우 NPE 발생 가능
supports()메서드에서type.equals("WEEKLY")호출 시type이 null이면 NullPointerException이 발생합니다. 다른 전략들도 동일한 패턴을 사용하고 있으나, 방어적 코딩을 위해 수정을 권장합니다.🔎 제안된 수정
@Override public boolean supports(String type) { - return type.equals("WEEKLY"); + return "WEEKLY".equals(type); }apps/commerce-batch/src/main/java/com/loopers/application/batch/product/MonthlyProductMetricBatchPartitioner.java (1)
42-51: 불필요한 빈 파티션 생성 가능성
totalCount가gridSize보다 작은 경우에도 모든gridSize개의 파티션을 생성합니다. 이로 인해partitionOffset이totalCount를 초과하는 빈 파티션이 생성될 수 있습니다.🔎 제안된 수정
+ int actualGridSize = (int) Math.min(gridSize, totalCount); + long targetSize = (totalCount / actualGridSize) + 1; Map<String, ExecutionContext> partitions = new HashMap<>(); - for (int i = 0; i < gridSize; i++) { + for (int i = 0; i < actualGridSize; i++) { ExecutionContext context = new ExecutionContext(); context.putLong("partitionOffset", i * targetSize); context.putLong("partitionLimit", targetSize); context.putString("startDate", startDateParam); context.putString("endDate", endDateParam); partitions.put("partition" + i, context); }core/domain/src/main/java/com/loopers/core/domain/common/vo/YearMonthWeek.java (1)
48-50:getWeekEndDateTime()에서 마지막 나노초 누락
atTime(23, 59, 59)는 마지막 1초(999,999,999 나노초)를 포함하지 않습니다. 범위 쿼리에 사용 시 미세한 데이터 누락이 발생할 수 있습니다.🔎 제안된 수정
public LocalDateTime getWeekEndDateTime() { - return this.getWeekEndDate().atTime(23, 59, 59); + return this.getWeekEndDate().atTime(23, 59, 59, 999_999_999); }또는
LocalTime.MAX를 사용:return this.getWeekEndDate().atTime(LocalTime.MAX);core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/impl/MonthlyProductMetricRepositoryImpl.java (1)
25-30: Null 리스트 입력 시 NPE 가능성
bulkUpsert에서monthlyProductMetrics가 null인 경우stream()호출 시 NPE가 발생합니다.MonthlyProductMetricBulkRepositoryImpl에서 null 체크를 수행하지만, entity 변환 전에 발생합니다.🔎 제안된 수정
@Override public void bulkUpsert(List<MonthlyProductMetric> monthlyProductMetrics) { + if (monthlyProductMetrics == null || monthlyProductMetrics.isEmpty()) { + return; + } bulkRepository.bulkUpsert(monthlyProductMetrics.stream() .map(MonthlyProductMetricEntity::from) .toList()); }apps/commerce-batch/src/main/java/com/loopers/application/batch/product/config/WeeklyProductMetricBatchConfig.java (3)
38-49: Step 이름이 실제 기능과 불일치합니다.
partitionDailyMetricStep과collectDailyMetricStep은 주간(Weekly) 메트릭 집계 작업을 수행하지만, 이름에 "Daily"가 포함되어 있어 혼란을 줄 수 있습니다.partitionWeeklyMetricStep,aggregateWeeklyMetricStep등으로 변경을 권장합니다.
80-86: TaskExecutor Bean 이름 충돌 가능성
asyncTaskExecutor()라는 일반적인 이름은 다른 배치 설정(예: MonthlyProductMetricBatchConfig의monthlyAsyncTaskExecutor)과 충돌하거나 의도치 않은 주입이 발생할 수 있습니다.🔎 수정 제안
@Bean -public TaskExecutor asyncTaskExecutor() { +public TaskExecutor weeklyAsyncTaskExecutor() { SimpleAsyncTaskExecutor executor = new SimpleAsyncTaskExecutor("weekly-product-metric-batch-"); executor.setVirtualThreads(true); executor.setConcurrencyLimit(20); return executor; }그리고
collectDailyMetricStep에서도 해당 Bean을 명시적으로 참조:-.taskExecutor(asyncTaskExecutor()) +.taskExecutor(weeklyAsyncTaskExecutor())
75-77: 재시도 횟수(retryLimit) 명시 권장
retry(DataAccessException.class)만 설정되어 있고retryLimit가 명시되지 않았습니다. 기본값에 의존하기보다 명시적으로 재시도 횟수를 설정하는 것이 운영 시 예측 가능한 동작을 보장합니다.🔎 수정 제안
.faultTolerant() .retry(DataAccessException.class) +.retryLimit(3) .build();core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/DailyProductMetricJpaRepository.java (2)
30-46: JPQL에서 LIMIT/OFFSET 직접 사용은 Hibernate 전용 문법입니다.표준 JPQL은
LIMIT/OFFSET을 지원하지 않으며, 이는 Hibernate(HQL) 확장 기능입니다. Hibernate 외 JPA 구현체로 전환 시 호환성 문제가 발생할 수 있습니다.Spring Data JPA의
Pageable파라미터를 활용하면 구현체에 독립적인 페이지네이션이 가능합니다.
22-28:CAST(createdAt AS date)사용 시 인덱스 활용 확인 필요
cast(pme.createdAt as date)함수 호출은createdAt컬럼의 인덱스를 활용하지 못하게 할 수 있습니다(Index Scan → Full Table Scan). 데이터 규모가 클 경우 쿼리 성능에 영향을 줄 수 있습니다.대안으로 날짜 범위 조건을 사용하거나, 별도의
date컬럼을 추가하여 인덱싱하는 방법을 고려해 보세요.-- 대안: 범위 조건 사용 WHERE pme.createdAt >= :startDate AND pme.createdAt < :endDate + 1 dayAlso applies to: 36-37
apps/commerce-batch/src/main/java/com/loopers/application/batch/product/config/MonthlyProductMetricBatchConfig.java (1)
75-77: 재시도 횟수 제한 명시를 권장합니다.
.retry(DataAccessException.class)만 설정하면 Spring Batch 기본값(1회)이 적용됩니다. 명시적으로.retryLimit()을 지정하면 의도가 더 명확해지고 유지보수가 용이합니다.🔎 제안된 수정 사항
.taskExecutor(monthlyAsyncTaskExecutor()) .faultTolerant() .retry(DataAccessException.class) + .retryLimit(3) .build();
| dependencies { | ||
| implementation(project(":core:infra:database:mysql:mysql-config")) | ||
| implementation(project(":supports:jackson")) | ||
| implementation(project(":supports:logging")) | ||
| implementation(project(":supports:monitoring")) | ||
|
|
||
| //service | ||
| implementation(project(":core:service")) | ||
|
|
||
| //domain | ||
| implementation(project(":core:domain")) | ||
|
|
||
| // web | ||
| implementation("org.springframework.boot:spring-boot-starter-web") | ||
|
|
||
| //batch | ||
| implementation("org.springframework.boot:spring-boot-starter-batch") | ||
|
|
||
| implementation("org.springframework:spring-tx") | ||
| implementation("org.springframework.boot:spring-boot-starter-actuator") | ||
|
|
||
| // test-fixtures | ||
| testImplementation(project(":core:infra:database:mysql:mysql-config")) | ||
| testImplementation(testFixtures(project(":core:domain"))) | ||
| testImplementation(testFixtures(project(":core:infra:database:mysql"))) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for web component usage in commerce-batch module
fd -e java -e kt --path-separator / | grep -E "apps/commerce-batch" | head -20Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 1574
🏁 Script executed:
# Search for web annotations in commerce-batch source files
rg -i '(@(RestController|Controller|RequestMapping|GetMapping|PostMapping|Service|Component)|WebController|RestEndpoint)' apps/commerce-batch --type java --type kotlin -nRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 828
🏁 Script executed:
# Check the actual directory structure of commerce-batch
find apps/commerce-batch -type f -name "*.kt" -o -name "*.java" -o -name "*.kts" | grep -v ".gradle" | head -30Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 1574
🏁 Script executed:
# Check CommerceBatchApplication main class
cat -n apps/commerce-batch/src/main/java/com/loopers/CommerceBatchApplication.javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 599
🏁 Script executed:
# Check one of the scheduler files to see if web exposure is used
cat -n apps/commerce-batch/src/main/java/com/loopers/application/batch/product/MonthlyProductMetricScheduler.javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 2033
🏁 Script executed:
# Search for any Actuator endpoint configuration or web exposure in the batch module
rg -i '(actuator|endpoint|web|servlet)' apps/commerce-batch --type java --type kotlin -A 2 -B 2Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 788
spring-boot-starter-web 제거 및 중복된 mysql-config 의존성 정리
-
spring-boot-starter-web 제거 (Line 14): 배치 애플리케이션에서 웹 스타터가 사용되지 않습니다. 모듈 전체에서 HTTP 엔드포인트나 웹 컨트롤러가 없으며, 모든 배치 작업은
@Scheduled어노테이션을 통해 스케줄링으로 실행됩니다. 이 의존성을 제거하세요. -
중복된 mysql-config 제거 (Line 23):
mysql-config는 이미implementation스코프(Line 2)에 선언되어 있으므로,testImplementation에서의 선언은 제거하세요. Gradle에서 테스트 의존성은 자동으로 구현 의존성을 상속받습니다.
🤖 Prompt for AI Agents
In apps/commerce-batch/build.gradle.kts around lines 1 to 26, remove the
unnecessary "org.springframework.boot:spring-boot-starter-web" dependency (line
~14) because this is a batch-only module with no web controllers, and delete the
duplicate testImplementation(project(":core:infra:database:mysql:mysql-config"))
(line ~23) since mysql-config is already declared as an implementation
dependency and test scope inherits implementation dependencies; update the file
to keep only the batch- and required support dependencies.
...tch/src/main/java/com/loopers/application/batch/product/MonthlyProductMetricBatchWriter.java
Show resolved
Hide resolved
| @Scheduled(cron = "0 0 2 1 * ?") | ||
| public void run() throws JobInstanceAlreadyCompleteException, JobExecutionAlreadyRunningException, JobParametersInvalidException, JobRestartException { |
There was a problem hiding this comment.
배치 작업 실행 실패 시 관찰성과 오류 처리를 개선하세요.
스케줄러 메서드가 4개의 체크 예외를 던지지만 로깅이나 예외 처리가 없어, 배치 작업 실패가 조용히 무시될 수 있습니다. 운영 환경에서 문제 진단을 어렵게 만듭니다.
🔎 로깅과 예외 처리 추가 제안
@Scheduled(cron = "0 0 2 1 * ?")
- public void run() throws JobInstanceAlreadyCompleteException, JobExecutionAlreadyRunningException, JobParametersInvalidException, JobRestartException {
+ public void run() {
+ try {
+ log.info("Starting monthly product metric aggregation batch job");
YearMonth lastMonth = YearMonth.now().minusMonths(1);
LocalDate startDate = lastMonth.atDay(1);
LocalDate endDate = lastMonth.atEndOfMonth();
JobParameters jobParameters = new JobParametersBuilder()
.addString("startDate", startDate.toString())
.addString("endDate", endDate.toString())
.addLong("timestamp", System.currentTimeMillis())
.toJobParameters();
jobLauncher.run(monthlyProductMetricJob, jobParameters);
+ log.info("Monthly product metric aggregation batch job completed successfully");
+ } catch (JobExecutionAlreadyRunningException e) {
+ log.warn("Monthly product metric batch job is already running", e);
+ } catch (JobInstanceAlreadyCompleteException | JobRestartException | JobParametersInvalidException e) {
+ log.error("Failed to execute monthly product metric batch job", e);
+ }
}Lombok의 @Slf4j 어노테이션을 클래스에 추가하세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/application/batch/product/MonthlyProductMetricScheduler.java
around lines 25-26, the scheduled run() method currently declares four checked
exceptions but has no logging or handling; add Lombok's @Slf4j to the class,
wrap the job execution in a try/catch that catches those specific exceptions (or
Exception) inside the method instead of letting them propagate silently, call
log.error(...) with a clear contextual message and the exception instance for
each failure, and optionally emit a metric or rethrow a runtime exception if you
need the scheduler to mark the job as failed.
| @Component | ||
| @RequiredArgsConstructor | ||
| public class WeeklyProductMetricBatchWriter implements ItemStreamWriter<ProductMetricAggregation> { | ||
|
|
||
| private final WeeklyProductMetricRepository repository; | ||
| private YearMonthWeek yearMonthWeek; | ||
|
|
||
| @Override | ||
| public void open(@NonNull ExecutionContext executionContext) { | ||
| LocalDate startDate = LocalDate.parse(executionContext.getString("startDate")); | ||
| this.yearMonthWeek = YearMonthWeek.from(startDate); | ||
| } |
There was a problem hiding this comment.
싱글톤 Bean에서 가변 상태로 인한 동시성 문제
@Component는 싱글톤 스코프이지만, yearMonthWeek 필드는 가변 상태입니다. 파티션이 병렬로 실행될 경우, 여러 스레드가 동일한 인스턴스를 공유하면서 open() 메서드에서 yearMonthWeek를 덮어쓸 수 있어 데이터 정합성 문제가 발생할 수 있습니다.
@StepScope를 추가하여 각 파티션이 독립적인 Writer 인스턴스를 갖도록 해야 합니다.
🔎 제안된 수정
import org.springframework.batch.item.Chunk;
import org.springframework.batch.item.ExecutionContext;
import org.springframework.batch.item.ItemStreamWriter;
+import org.springframework.batch.core.configuration.annotation.StepScope;
import org.springframework.stereotype.Component;
@Component
+@StepScope
@RequiredArgsConstructor
public class WeeklyProductMetricBatchWriter implements ItemStreamWriter<ProductMetricAggregation> {🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/application/batch/product/WeeklyProductMetricBatchWriter.java
around lines 17 to 28, the class is a singleton component but holds mutable
state in the yearMonthWeek field which can be overwritten concurrently by
parallel partitions; make the writer step-scoped so each partition gets its own
instance: annotate the class with Spring Batch's @StepScope (and add the
necessary import) so dependency injection still works but a new writer instance
is created per step execution, keep yearMonthWeek as an instance field
(non-static) and leave open() logic unchanged; verify no other shared mutable
fields exist.
| - monitoring.yml | ||
| batch: | ||
| jdbc: | ||
| initialize-schema: always |
There was a problem hiding this comment.
프로덕션 환경에서 데이터 손실 위험이 있습니다.
initialize-schema: always 설정은 애플리케이션이 시작될 때마다 스키마를 재생성합니다. 이는 개발 환경에서는 유용하지만, 프로덕션 환경에서는 데이터 손실을 초래할 수 있습니다. 프로필별로 다르게 설정해야 합니다.
🔎 프로필별 설정 제안
글로벌 설정을 제거하고 각 프로필 섹션에 다음과 같이 추가하세요:
- batch:
- jdbc:
- initialize-schema: always
- job:
- enabled: false
+ batch:
+ job:
+ enabled: false그리고 local/test 프로필 섹션에만 추가:
spring:
config:
activate:
on-profile: local, test
+ batch:
+ jdbc:
+ initialize-schema: always프로덕션 환경(dev, qa, prd)에서는 Flyway나 Liquibase 같은 마이그레이션 도구를 사용하거나 never로 설정하세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/commerce-batch/src/main/resources/application.yml around line 33, the
global setting "initialize-schema: always" causes schema recreation on every
startup (risking production data loss); remove this global key and instead add
profile-specific settings: set initialize-schema: always (or create-drop) only
under local/test profiles, and set initialize-schema: never for production
profiles (dev/qa/prd) while relying on Flyway/Liquibase for migrations; update
the YAML to place these keys under appropriate spring.profiles sections so
production never recreates the schema.
| Page<MonthlyProductRankingProjection> findMonthlyProductRanking( | ||
| LocalDate date, | ||
| Double payWeight, | ||
| Double viewWeight, | ||
| Double likeWeight, | ||
| Pageable pageable | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the implementation file
find . -type f -name "*MonthlyProductMetricQuerydslRepositoryImpl*" -o -name "*MonthlyProductMetric*" | head -20Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 1964
🏁 Script executed:
#!/bin/bash
# Search for the implementation class definition
rg "class.*MonthlyProductMetricQuerydslRepository" --type javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 329
🏁 Script executed:
#!/bin/bash
# Search for findMonthlyProductRanking implementation
rg "findMonthlyProductRanking" --type java -A 30Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 9995
🏁 Script executed:
#!/bin/bash
# Read the complete implementation file
cat -n core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/MonthlyProductMetricQuerydslRepositoryImpl.javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 3286
🏁 Script executed:
#!/bin/bash
# Check the caller class for parameter validation
cat -n core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/impl/MonthlyProductMetricRepositoryImpl.java | head -80Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 2708
🏁 Script executed:
#!/bin/bash
# Search for any @Nullable or @NonNull annotations on the interface
rg "@Nullable|@NonNull|Objects.requireNonNull" core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/MonthlyProductMetricQuerydslRepository.java -B 2 -A 2Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 70
🏁 Script executed:
#!/bin/bash
# Check the interface definition for annotations
cat -n core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/MonthlyProductMetricQuerydslRepository.javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 770
🏁 Script executed:
#!/bin/bash
# Search for any validation or use of these weight parameters in domain layer
rg "payWeight|viewWeight|likeWeight" --type java -B 2 -A 2Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 18074
🏁 Script executed:
#!/bin/bash
# Check if there's validation at service or controller level
find . -type f -name "*.java" | xargs grep -l "findRankingsBy" | head -10Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 786
🏁 Script executed:
#!/bin/bash
# Check the strategy component to see weight handling
cat -n core/service/src/main/java/com/loopers/core/service/product/component/GetMonthlyProductRankingsStrategy.javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 1493
date 파라미터의 null 체크와 가중치 값 검증 필요
구현체(MonthlyProductMetricQuerydslRepositoryImpl)에서 date 파라미터에 대한 null 체크가 없어 date.getYear() 호출 시 NullPointerException이 발생할 수 있습니다. 또한 payWeight, viewWeight, likeWeight 파라미터도 null일 경우 .multiply() 연산에서 NPE가 발생하며, 음수 또는 유효하지 않은 가중치 값에 대한 검증이 없어 부정확한 순위 계산이 발생할 수 있습니다.
권장 수정 사항
메서드 시작 부분에서 다음 검증 로직 추가:
Objects.requireNonNull(date, "date must not be null")Objects.requireNonNull(payWeight, "payWeight must not be null")- 가중치 값이 음수 또는 0 이하인 경우에 대한 검증 로직
🤖 Prompt for AI Agents
In
core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/MonthlyProductMetricQuerydslRepository.java
around lines 11-17, add input validation at the start of the implementation:
require non-null for date, payWeight, viewWeight, likeWeight (e.g.
Objects.requireNonNull(..., "X must not be null")), and validate that numeric
weights are > 0 (else throw IllegalArgumentException with a clear message);
these checks prevent date.getYear() and weight.multiply(...)
NullPointerExceptions and guard against zero/negative weights affecting ranking.
| List<MonthlyProductRankingProjection> content = queryFactory | ||
| .select(new QMonthlyProductRankingProjection( | ||
| metric.productId, | ||
| Expressions.numberTemplate(Long.class, | ||
| "ROW_NUMBER() OVER (ORDER BY {0} DESC)", | ||
| scoreCalculation), | ||
| scoreCalculation.as("score") | ||
| )) | ||
| .from(metric) | ||
| .where(metric.year.eq(year) | ||
| .and(metric.month.eq(month))) | ||
| .orderBy(scoreCalculation.desc()) | ||
| .offset(pageable.getOffset()) | ||
| .limit(pageable.getPageSize()) | ||
| .fetch(); |
There was a problem hiding this comment.
ROW_NUMBER()와 페이지네이션 조합 시 랭킹 값이 잘못됩니다.
ROW_NUMBER() OVER (ORDER BY score DESC)는 쿼리 결과 전체가 아닌 페이지네이션 적용 후의 결과 집합에서 계산됩니다. 예를 들어, pageNo=1, pageSize=10일 때 실제 순위 1120위인 데이터가 ROW_NUMBER 110으로 반환됩니다.
수정 방안:
- 서브쿼리에서 먼저 ROW_NUMBER를 계산한 후 외부 쿼리에서 페이지네이션 적용
- 또는 ROW_NUMBER 대신
offset + index + 1로 클라이언트에서 랭킹 계산
🔎 수정 제안: 클라이언트 측 랭킹 계산
ROW_NUMBER 컬럼을 제거하고, 서비스 레이어에서 offset + index + 1로 랭킹을 계산하는 방식을 권장합니다:
// Service layer에서 처리
long offset = pageable.getOffset();
for (int i = 0; i < content.size(); i++) {
content.get(i).setRanking(offset + i + 1);
}🤖 Prompt for AI Agents
In
core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/MonthlyProductMetricQuerydslRepositoryImpl.java
around lines 42 to 56, the current use of ROW_NUMBER() in the same query that
applies offset/limit produces incorrect rankings for paginated results; fix by
either 1) moving the ROW_NUMBER() calculation into a subquery (compute
ROW_NUMBER() OVER (ORDER BY score DESC) in an inner query and then apply
WHERE/ORDER/OFFSET/LIMIT in an outer query selecting the precomputed ranking),
or 2) remove the ROW_NUMBER projection from this repository and compute ranking
in the service layer using offset + index + 1 after fetch; choose one approach
and implement it consistently (if you pick client-side, strip ROW_NUMBER from
the select and set ranking in service using pageable.getOffset() + i + 1).
| Page<WeeklyProductRankingProjection> findWeeklyProductRanking( | ||
| LocalDate date, | ||
| Double payWeight, | ||
| Double viewWeight, | ||
| Double likeWeight, | ||
| Pageable pageable | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the implementation class
find . -name "*WeeklyProductMetricQuerydslRepository*" -type fRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 353
🏁 Script executed:
#!/bin/bash
# Search for the implementation class
rg "class WeeklyProductMetricQuerydslRepositoryImpl" -A 30Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 5599
🏁 Script executed:
#!/bin/bash
# Also search for the interface file to confirm its location
rg "interface WeeklyProductMetricQuerydslRepository" -A 20Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 1661
🏁 Script executed:
#!/bin/bash
# Get complete implementation file
wc -l ./core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/WeeklyProductMetricQuerydslRepositoryImpl.javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 225
🏁 Script executed:
#!/bin/bash
# Read the complete implementation
cat -n ./core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/WeeklyProductMetricQuerydslRepositoryImpl.javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 3626
🏁 Script executed:
#!/bin/bash
# Check for @NonNull or @NotNull annotations in both files
rg "(@NonNull|@NotNull|@Nullable)" ./core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/WeeklyProductMetric*Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 70
🏁 Script executed:
#!/bin/bash
# Search for callers of findWeeklyProductRanking method
rg "findWeeklyProductRanking" --type java -B 2 -A 5Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 3995
🏁 Script executed:
#!/bin/bash
# Check if there are any validation annotations or constraints in the imports
head -20 ./core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/WeeklyProductMetricQuerydslRepository.javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 640
가중치 파라미터에 null 안전성을 추가하세요.
구현체의 42-44줄에서 payWeight, viewWeight, likeWeight 파라미터가 null 체크 없이 .multiply() 연산에 직접 사용됩니다. null 값이 전달되면 NullPointerException이 발생합니다. @NonNull 어노테이션을 파라미터에 추가하거나 primitive double 타입으로 변경하세요.
🤖 Prompt for AI Agents
In
core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/WeeklyProductMetricQuerydslRepository.java
around lines 11 to 17 (implementation uses lines ~42-44), the weight parameters
payWeight, viewWeight, likeWeight are nullable and are used with .multiply()
without null checks which can cause NPEs; fix by making these parameters
null-safe—either change their types from Double to primitive double or add
@NonNull to each parameter in the method signature, and ensure the
implementation assumes non-null (or add explicit null-to-default fallback before
multiply if you prefer to keep boxed types).
| List<WeeklyProductRankingProjection> content = queryFactory | ||
| .select(new QWeeklyProductRankingProjection( | ||
| metric.productId, | ||
| Expressions.numberTemplate(Long.class, | ||
| "ROW_NUMBER() OVER (ORDER BY {0} DESC)", | ||
| scoreCalculation), | ||
| scoreCalculation.as("score") | ||
| )) | ||
| .from(metric) | ||
| .where(metric.year.eq(year) | ||
| .and(metric.month.eq(month)) | ||
| .and(metric.weekOfYear.eq(weekOfYear))) | ||
| .orderBy(scoreCalculation.desc()) | ||
| .offset(pageable.getOffset()) | ||
| .limit(pageable.getPageSize()) | ||
| .fetch(); |
There was a problem hiding this comment.
ROW_NUMBER()와 페이지네이션 조합 문제 (MonthlyProductMetricQuerydslRepositoryImpl과 동일)
위 Monthly 구현과 동일하게, ROW_NUMBER가 페이지네이션 적용 후 결과에 대해 계산되어 잘못된 랭킹 값이 반환됩니다.
🤖 Prompt for AI Agents
In
core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/WeeklyProductMetricQuerydslRepositoryImpl.java
around lines 46-61, the ROW_NUMBER() window is being applied after offset/limit
which yields wrong ranks; instead compute ROW_NUMBER() in an inner/subquery
without pagination and then apply pagination in an outer query. Change the
Querydsl logic to build a subquery that selects productId, ROW_NUMBER() OVER
(ORDER BY score DESC) as rank and score (no offset/limit), then create an outer
query that selects from that subquery, orders by score (or rank) and applies
pageable.getOffset()/getPageSize() so pagination does not affect rank
calculation.
| .where(metric.year.eq(year) | ||
| .and(metric.month.eq(month)) | ||
| .and(metric.weekOfYear.eq(weekOfYear))) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "YearMonthWeek.java" --exec cat {} \;Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 1657
🏁 Script executed:
fd "WeeklyProductMetricBatchWriter.java" -x head -100 {}Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 1517
🏁 Script executed:
rg "class WeeklyProductMetric" --type java -A 10Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 18443
🏁 Script executed:
cat > /tmp/test_week.java << 'EOF'
import java.time.LocalDate;
import java.time.temporal.WeekFields;
import java.util.Locale;
public class test_week {
public static void main(String[] args) {
// Test cross-month week scenario
// Week that spans Dec 31 2024 to Jan 6 2025
LocalDate dec31 = LocalDate.of(2024, 12, 31);
LocalDate jan1 = LocalDate.of(2025, 1, 1);
LocalDate jan6 = LocalDate.of(2025, 1, 6);
WeekFields wf = WeekFields.of(Locale.getDefault());
System.out.println("Dec 31, 2024: year=" + dec31.getYear() + ", month=" + dec31.getMonthValue() + ", weekOfYear=" + dec31.get(wf.weekOfYear()));
System.out.println("Jan 1, 2025: year=" + jan1.getYear() + ", month=" + jan1.getMonthValue() + ", weekOfYear=" + jan1.get(wf.weekOfYear()));
System.out.println("Jan 6, 2025: year=" + jan6.getYear() + ", month=" + jan6.getMonthValue() + ", weekOfYear=" + jan6.get(wf.weekOfYear()));
}
}
EOF
javac /tmp/test_week.java 2>/dev/null && java -cp /tmp test_weekRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 72
🏁 Script executed:
# Check if there are any query methods that might use date ranges instead of the three-field filter
rg "findWeeklyProductRanking|findByYearMonthWeek" --type java -A 5Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 3129
🏁 Script executed:
# Look at the complete query method to understand the context
cat core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/WeeklyProductMetricQuerydslRepositoryImpl.java | head -80Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 3122
🏁 Script executed:
# Verify the exact locale used in YearMonthWeek vs the query
rg "Locale\.(KOREA|getDefault)" --type java -B 2 -A 2Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 2720
🏁 Script executed:
# Check if there are any tests that cover cross-month week scenarios
fd "test" -t f --name "*Weekly*" | head -5 | xargs grep -l "Dec\|Jan\|month" 2>/dev/null || echo "No specific cross-month tests found"Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 391
🏁 Script executed:
# Verify the impact by checking if Locale.KOREA behavior is different from Locale.getDefault()
# and understand what the default locale typically is
rg "server.servlet.locale|application.properties|locale" core/infra/database/mysql/src/main/resources/ 2>/dev/null || echo "No locale config found in infra module"Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 126
🏁 Script executed:
# Check the batch config to see if there's a default locale set
fd "application*.properties" -type f | xargs grep -i "locale" 2>/dev/null || echo "No locale property found"Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 300
🏁 Script executed:
# Look for any TODO or comments about the locale inconsistency
rg "TODO|FIXME|locale|Locale\.(KOREA|getDefault)" core/domain/src/main/java/com/loopers/core/domain/common/vo/YearMonthWeek.java core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/WeeklyProductMetricQuerydslRepositoryImpl.javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 722
Locale.KOREA와 Locale.getDefault() 불일치로 인한 쿼리 불일치 문제
YearMonthWeek.java는 데이터 저장 시 Locale.getDefault()를 사용하여 weekOfYear를 계산하지만, WeeklyProductMetricQuerydslRepositoryImpl.java의 쿼리는 Locale.KOREA를 사용하여 weekOfYear를 계산합니다.
서버의 기본 로케일이 한국이 아닌 경우, 저장된 데이터와 쿼리의 weekOfYear 값이 일치하지 않아 쿼리 결과가 반환되지 않을 수 있습니다. 데이터 저장과 조회 로직에서 동일한 로케일(Locale.KOREA)을 사용하여 일관성을 보장하세요.
현재 코드
// 저장 시 - YearMonthWeek.java
WeekFields weekFields = WeekFields.of(Locale.getDefault());
// 쿼리 시 - WeeklyProductMetricQuerydslRepositoryImpl.java
WeekFields weekFields = WeekFields.of(Locale.KOREA);🤖 Prompt for AI Agents
In
core/infra/database/mysql/src/main/java/com/loopers/core/infra/database/mysql/product/WeeklyProductMetricQuerydslRepositoryImpl.java
around lines 55-57, the query uses WeekFields.of(Locale.KOREA) while
YearMonthWeek.java (storage) uses WeekFields.of(Locale.getDefault()), causing
weekOfYear mismatches; make the locale usage consistent by changing the storage
logic (YearMonthWeek.java) to use WeekFields.of(Locale.KOREA) instead of
Locale.getDefault() so both save and query use Locale.KOREA, and run/update
tests to verify week-based lookups still return expected results.
1 participant
📌 Summary
구현 기능 목록
메트릭 집계
메트릭은 Spring Batch를 통해 구현했습니다.
기존에 있던 api, streamer안에 같이 구현하면 실시간 고객 경험을 담당하는 다른 서비스에 배포 시, 혹은 문제 발생 시 영향을 줄 수 있을 거라고 판단하여 Spring Batch를 별도 어플리케이션(commerce-batch)으로 구성했습니다.
집계 Job은 두 Step으로 구분됩니다.
Step1. 집계할 전체 상품 ID를 여러 Grid로 구분합니다.
Step2. 집계 데이터를 읽고, 주간 집계, 월간 집계 데이터를 생성하여 저장합니다.
위와 같이 두 Step으로 구분지은 이유는 다음과 같습니다.
일일 집계 데이터가 수십만건이 넘어가는 경우, Scaling을 통해 Step을 나누어 독립적으로 처리할 필요가 있어보였습니다.
이 Scailing 방법 중
partitioner를 사용하여 구현했습니다. 참고문서두 Step은 아래와 같이 설정했습니다.
Step1. 파티셔닝
각 설정은 아래와 같습니다.
partitioner: 하위 Step을 분리할 파티셔너를 정의합니다.step: 하위 Step을 정의합니다.taskExecutor: 하위 Step들을 실행 시킬 Executor를 정의합니다.gridSize: 하위 스텝을 나눌 gridSize를 정의합니다. (이 값이 4인 경우, 4개의 Step으로 동작)Step2. 주간 집계
processor가 집계에 큰 의미가 없다고 판단하여 reader와 writer로만 구현했습니다.SynchronizedItemStreamReader를 사용하여 구현했습니다.TaskExecutor는 수십만건의 상품을 동기로 Step을 동기적으로 실행하면 성능문제가 발생할 것이라고 생각하여,
Virtual Thread 멀티쓰레드 기반으로 구현했습니다.
Partitioner 구현은 아래와 같습니다.
ExecutionContext에 put합니다.만약 총 2000개라면 500개씩 나뉘어 동작합니다.
ItemReader는 아래와 같이 구현했습니다.
open: Reader가 시작될 때, 파티셔너로부터 ExecutionContext을 받아서 처리할 데이터를 조회합니다.read: 전체 조회한 데이터를 읽습니다.ItemWriter는 아래와 같이 구현합니다.
open: 시작일로부터 몇년, 몇월, 몇주차인지를 계산합니다.상품 랭킹 조회
상품 랭킹은 type을 받아 일일, 주간, 월간을 조회할 수 있도록 했습니다.
일간 랭킹은 기존과 동이하게 실시간으로 redis를 통해 받아오며 주간, 월간은 metric을 통해 계산합니다.
이 부분은 전략패턴을 통해 type별로 다른 전략을 사용하도록 했습니다.
전략 인터페이스
일일 랭킹 조회
주간 랭킹 조회
월간 랭킹 조회
전략 선택자
💬 Review Points
processor를 단건씩 주간 메트릭을 만드는 것보다writer에서 일괄로 만드는 것이 더 효율적이라 판단하여 구현하지 않았습니다. 이 부분에 대해서 어떻게 생각하시나요?✅ Checklist
🧱 Spring Batch
🧩 Ranking API
📎 References
Summary by CodeRabbit
릴리스 노트
새 기능
변경 사항
✏️ Tip: You can customize this high-level summary in your review settings.