[volume-10] Collect, Stack, Zip#243
[volume-10] Collect, Stack, Zip#243adminhelper wants to merge 11 commits intoLoopers-dev-lab:adminhelperfrom
Conversation
[volume-3] 도메인 모델링 및 구현
# Conflicts: # apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java # apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java # apps/commerce-api/src/main/java/com/loopers/domain/brand/Brand.java # apps/commerce-api/src/main/java/com/loopers/domain/product/ProductService.java # settings.gradle.kts
Walkthrough트랜잭션 주석 제거 및 에러 분류 조정, Order/도메인 관계 개선, 랭킹(일/주/월) 기능 및 Spring Batch 기반 배치 모듈 추가, GitHub Actions 워크플로우와 관련 문서/테스트가 함께 추가되었습니다. Changes
Sequence Diagram(s)mermaid mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
.github/workflows/main.yml (1)
1-13: 중복된 PR 리뷰 도구 사용을 검토하세요.이 워크플로우는 Codium의 PR Agent를 추가합니다. 리포지토리에 이미 CodeRabbit이 구성되어 있는 것으로 보이므로, 두 개의 AI 기반 PR 리뷰 도구를 동시에 실행하면 다음과 같은 영향이 있을 수 있습니다:
- 중복되거나 상충되는 피드백
- API 비용 증가 (두 도구 모두 OpenAI API 사용)
- 개발자의 리뷰 코멘트 관리 복잡도 증가
비교 목적이 아니라면 하나의 도구를 선택하여 사용하는 것을 권장합니다. 또한
OPENAI_KEY시크릿이 올바르게 구성되어 있는지 확인하세요.apps/commerce-batch/src/main/java/com/loopers/CommerceBatchApplication.java (1)
14-18: JVM 전역 타임존 설정에 대한 문서화 또는 로깅을 추가하세요.
TimeZone.setDefault()는 JVM 전체의 기본 타임존을 변경합니다. 이는 같은 JVM에서 실행되는 다른 컴포넌트에 영향을 줄 수 있으므로, 이러한 설정 변경을 로깅하거나 문서화하는 것이 좋습니다.🔎 로깅 추가 제안
+import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + @ConfigurationPropertiesScan @SpringBootApplication public class CommerceBatchApplication { + private static final Logger log = LoggerFactory.getLogger(CommerceBatchApplication.class); @PostConstruct public void started() { // set timezone TimeZone.setDefault(TimeZone.getTimeZone("Asia/Seoul")); + log.info("Default timezone set to Asia/Seoul"); }apps/commerce-batch/src/test/java/com/loopers/job/demo/DemoJobE2ETest.java (2)
4-4: 사용하지 않는 import를 제거하세요.
RequiredArgsConstructor가 import되어 있지만 클래스에 사용되지 않습니다. 필드 주입은@Autowired를 통해 이루어지고 있습니다.🔎 제안된 수정
-import lombok.RequiredArgsConstructor; import org.junit.jupiter.api.BeforeEach;
37-40: 빈 메서드를 제거하세요.
@BeforeEach메서드가 비어있고 아무런 동작도 수행하지 않습니다. 사용하지 않는 코드는 제거하는 것이 좋습니다.🔎 제안된 수정
- @BeforeEach - void beforeEach() { - - } -apps/commerce-batch/src/main/java/com/loopers/batch/job/demo/step/DemoTasklet.java (2)
24-26: 더 구체적인 예외 타입을 사용하세요.
requestDate가 null일 때 일반적인RuntimeException대신IllegalArgumentException또는 Spring Batch의JobParametersInvalidException을 사용하는 것이 의미상 더 명확합니다.🔎 제안된 수정
if (requestDate == null) { - throw new RuntimeException("requestDate is null"); + throw new IllegalArgumentException("requestDate is required but was null"); }
27-29: 프로덕션 환경을 위해 적절한 로거를 사용하세요.
System.out.println대신 SLF4J 로거를 사용하는 것이 좋습니다. 데모 코드라도 로깅 베스트 프랙티스를 따르면 향후 프로덕션 배치 작업 개발 시 좋은 템플릿이 됩니다.🔎 제안된 수정
+import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + @StepScope @ConditionalOnProperty(name = "spring.batch.job.name", havingValue = DemoJobConfig.JOB_NAME) @RequiredArgsConstructor @Component public class DemoTasklet implements Tasklet { + private static final Logger log = LoggerFactory.getLogger(DemoTasklet.class); + @Value("#{jobParameters['requestDate']}") private String requestDate; @Override public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) throws Exception { if (requestDate == null) { throw new IllegalArgumentException("requestDate is required but was null"); } - System.out.println("Demo Tasklet 실행 (실행 일자 : " + requestDate + ")"); + log.info("Demo Tasklet 실행 (실행 일자 : {})", requestDate); Thread.sleep(1000); - System.out.println("Demo Tasklet 작업 완료"); + log.info("Demo Tasklet 작업 완료"); return RepeatStatus.FINISHED; }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
.github/workflows/main.ymlapps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.javaapps/commerce-api/src/main/java/com/loopers/domain/brand/BrandService.javaapps/commerce-api/src/main/java/com/loopers/domain/order/Order.javaapps/commerce-api/src/main/java/com/loopers/domain/point/Point.javaapps/commerce-api/src/main/java/com/loopers/domain/point/PointService.javaapps/commerce-api/src/main/java/com/loopers/domain/product/Product.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductDomainService.javaapps/commerce-api/src/main/java/com/loopers/domain/user/UserService.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/brand/BrandJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1ApiSpec.javaapps/commerce-api/src/test/java/com/loopers/domain/user/UserTest.javaapps/commerce-batch/build.gradle.ktsapps/commerce-batch/src/main/java/com/loopers/CommerceBatchApplication.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/demo/DemoJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/demo/step/DemoTasklet.javaapps/commerce-batch/src/main/java/com/loopers/batch/listener/ChunkListener.javaapps/commerce-batch/src/main/java/com/loopers/batch/listener/JobListener.javaapps/commerce-batch/src/main/java/com/loopers/batch/listener/StepMonitorListener.javaapps/commerce-batch/src/main/resources/application.ymlapps/commerce-batch/src/test/java/com/loopers/CommerceBatchApplicationTest.javaapps/commerce-batch/src/test/java/com/loopers/job/demo/DemoJobE2ETest.javadocs/10round/10round.mddocs/2round/03-class-diagram.mddocs/3round/3round.md
💤 Files with no reviewable changes (5)
- apps/commerce-api/src/main/java/com/loopers/domain/brand/BrandService.java
- apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java
- docs/2round/03-class-diagram.md
- apps/commerce-api/src/main/java/com/loopers/domain/point/Point.java
- apps/commerce-api/src/main/java/com/loopers/infrastructure/brand/BrandJpaRepository.java
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-12-19T09:30:12.459Z
Learnt from: HongChangMo
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 198
File: apps/commerce-api/src/main/java/com/loopers/application/payment/PaymentEventListener.java:0-0
Timestamp: 2025-12-19T09:30:12.459Z
Learning: In the loopers-spring-java-template repository's commerce-api module, when entities are managed within a transactional context (e.g., Transactional methods), prefer relying on JPA dirty checking for automatic persistence rather than explicit save() calls. Both Payment and Order entities in PaymentEventListener use this pattern, with state changes automatically flushed on transaction commit.
Applied to files:
apps/commerce-api/src/main/java/com/loopers/domain/order/Order.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductDomainService.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java
📚 Learning: 2025-11-27T09:09:24.961Z
Learnt from: sky980221
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 121
File: apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java:22-24
Timestamp: 2025-11-27T09:09:24.961Z
Learning: Product 엔티티 (apps/commerce-api/src/main/java/com/loopers/domain/product/Product.java)는 유즈케이스별로 의도적으로 다른 락 전략을 사용한다: 좋아요 기능에는 비관적 락(findByIdForUpdate)을, 재고 차감에는 낙관적 락(Version + 재시도)을 사용한다.
Applied to files:
apps/commerce-api/src/main/java/com/loopers/domain/product/Product.javadocs/3round/3round.mdapps/commerce-api/src/main/java/com/loopers/domain/product/ProductDomainService.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java
📚 Learning: 2025-11-17T14:23:34.490Z
Learnt from: yeonsu00
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 85
File: apps/commerce-api/src/main/java/com/loopers/domain/point/Point.java:54-60
Timestamp: 2025-11-17T14:23:34.490Z
Learning: In the Point domain entity at apps/commerce-api/src/main/java/com/loopers/domain/point/Point.java, the deduct() method does not validate balance sufficiency. The balance validation is intentionally performed at the service layer in PointService.deductPoint() using hasSufficientAmount() before calling deduct(). This is the team's design decision to handle validation at the service layer rather than within the entity.
Applied to files:
apps/commerce-api/src/main/java/com/loopers/domain/point/PointService.java
📚 Learning: 2026-01-01T14:12:04.385Z
Learnt from: ghojeong
Repo: Loopers-dev-lab/loopers-spring-kotlin-template PR: 76
File: apps/commerce-batch/src/main/kotlin/com/loopers/batch/listener/ChunkListener.kt:4-5
Timestamp: 2026-01-01T14:12:04.385Z
Learning: In Spring Batch 6.x, ChunkListener API changed from using ChunkContext to Chunk<T> parameters. The correct method signatures are: beforeChunk(Chunk<T> chunk), afterChunk(Chunk<T> chunk), and onChunkError(Exception exception, Chunk<T> chunk). The import path is org.springframework.batch.core.listener.ChunkListener.
Applied to files:
apps/commerce-batch/src/main/java/com/loopers/batch/listener/StepMonitorListener.javaapps/commerce-batch/src/main/java/com/loopers/batch/listener/JobListener.javaapps/commerce-batch/src/main/java/com/loopers/batch/listener/ChunkListener.java
📚 Learning: 2026-01-01T14:12:07.943Z
Learnt from: ghojeong
Repo: Loopers-dev-lab/loopers-spring-kotlin-template PR: 76
File: apps/commerce-batch/src/main/kotlin/com/loopers/batch/listener/JobListener.kt:4-5
Timestamp: 2026-01-01T14:12:07.943Z
Learning: Spring Batch 6.x에서 JobExecution은 org.springframework.batch.core.job 패키지에, JobExecutionListener는 org.springframework.batch.core.listener 패키지에 위치합니다. Spring Batch 5.x와 달리 하위 패키지를 사용합니다.
Applied to files:
apps/commerce-batch/src/main/java/com/loopers/batch/listener/StepMonitorListener.javaapps/commerce-batch/src/main/java/com/loopers/batch/listener/JobListener.java
📚 Learning: 2025-11-27T22:53:56.762Z
Learnt from: CR
Repo: Loopers-dev-lab/loopers-spring-kotlin-template PR: 0
File: .codeguide/loopers-1-week.md:0-0
Timestamp: 2025-11-27T22:53:56.762Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js} : Birth date must be in the format 'yyyy-MM-dd'. User object creation should fail if this format is not met.
Applied to files:
apps/commerce-api/src/test/java/com/loopers/domain/user/UserTest.java
📚 Learning: 2025-11-25T02:35:20.910Z
Learnt from: CR
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 0
File: .codeguide/loopers-1-week.md:0-0
Timestamp: 2025-11-25T02:35:20.910Z
Learning: Applies to **/*.(test|spec).{ts,tsx,js,jsx} : Birth date must be in yyyy-MM-dd format; User object creation must fail if this format is not met
Applied to files:
apps/commerce-api/src/test/java/com/loopers/domain/user/UserTest.java
📚 Learning: 2025-11-27T22:53:56.762Z
Learnt from: CR
Repo: Loopers-dev-lab/loopers-spring-kotlin-template PR: 0
File: .codeguide/loopers-1-week.md:0-0
Timestamp: 2025-11-27T22:53:56.762Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js} : Email must be in the format 'xxyy.zz'. User object creation should fail if this format is not met.
Applied to files:
apps/commerce-api/src/test/java/com/loopers/domain/user/UserTest.java
📚 Learning: 2025-11-25T02:35:20.910Z
Learnt from: CR
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 0
File: .codeguide/loopers-1-week.md:0-0
Timestamp: 2025-11-25T02:35:20.910Z
Learning: Applies to **/*.(test|spec).{ts,tsx,js,jsx} : Email must be in the format xxyy.zz; User object creation must fail if this format is not met
Applied to files:
apps/commerce-api/src/test/java/com/loopers/domain/user/UserTest.java
📚 Learning: 2025-11-21T03:38:07.494Z
Learnt from: minor7295
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 99
File: apps/commerce-api/src/main/resources/application.yml:26-30
Timestamp: 2025-11-21T03:38:07.494Z
Learning: The batch job implementation for likeCount synchronization in apps/commerce-api is temporary and intended for development environment only. It will be replaced with Event-Driven Architecture (EDA) before production deployment, so production-level configuration concerns (like profile-based initialize-schema settings) are not required.
Applied to files:
apps/commerce-batch/src/main/resources/application.yml
📚 Learning: 2025-11-09T10:41:39.297Z
Learnt from: ghojeong
Repo: Loopers-dev-lab/loopers-spring-kotlin-template PR: 25
File: apps/commerce-api/src/main/kotlin/com/loopers/domain/product/ProductRepository.kt:1-12
Timestamp: 2025-11-09T10:41:39.297Z
Learning: In this codebase, domain repository interfaces are allowed to use Spring Data's org.springframework.data.domain.Page and org.springframework.data.domain.Pageable types. This is an accepted architectural decision and should not be flagged as a DIP violation.
Applied to files:
apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java
🧬 Code graph analysis (4)
apps/commerce-batch/src/test/java/com/loopers/job/demo/DemoJobE2ETest.java (1)
apps/commerce-batch/src/test/java/com/loopers/CommerceBatchApplicationTest.java (1)
SpringBootTest(6-10)
apps/commerce-batch/src/main/java/com/loopers/batch/job/demo/step/DemoTasklet.java (1)
apps/commerce-batch/src/main/java/com/loopers/batch/job/demo/DemoJobConfig.java (1)
ConditionalOnProperty(19-48)
apps/commerce-batch/src/main/java/com/loopers/CommerceBatchApplication.java (1)
apps/pg-simulator/src/main/kotlin/com/loopers/PaymentGatewayApplication.kt (1)
main(22-24)
apps/commerce-batch/src/test/java/com/loopers/CommerceBatchApplicationTest.java (1)
apps/commerce-batch/src/test/java/com/loopers/job/demo/DemoJobE2ETest.java (1)
SpringBootTest(23-76)
🔇 Additional comments (23)
apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java (1)
22-24: LGTM! 코드가 개선되었습니다.JPA repository의 save() 결과를 직접 반환하도록 변경한 것은 좋은 개선입니다. JPA의 save() 메서드는 영속성 컨텍스트에서 관리되는 엔티티를 반환하며, 특히 새로운 엔티티의 경우 ID 생성이나 타임스탬프 등이 설정된 인스턴스를 반환하므로, 이를 그대로 반환하는 것이 올바른 패턴입니다.
apps/commerce-batch/build.gradle.kts (1)
1-21: 의존성 구조가 적절하게 구성되어 있습니다.Spring Batch 모듈을 위한 의존성 선택이 논리적이고, 모듈 조직화가 명확합니다:
- 모듈 의존성: JPA, Redis 통합 및 인프라 지원(logging, monitoring) 모두 존재하며 올바르게 구성됨
- Batch 지원: Spring Batch 및 테스트 의존성 적절히 선택
- QueryDSL: Jakarta 지원으로 올바르게 구성되어 있으며, 기존 JPA 모듈과 동일한 패턴 사용
- 테스트 픽스처: 테스트 범위로 적절히 제한
모든 참조 모듈이 존재하고 QueryDSL
::jakarta분류자 문법이 프로젝트의 기존 설정과 일치합니다.apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java (1)
18-18: 에러 메시지 개선을 확인했습니다."사용자ID"에서 "사용자 ID"로 띄어쓰기를 추가하여 가독성이 향상되었습니다.
apps/commerce-api/src/main/java/com/loopers/domain/order/Order.java (1)
57-60: 양방향 관계 설정이 올바르게 구현되었습니다.
OrderItem에 부모Order참조를 설정한 후 리스트에 추가하는 방식으로 양방향 관계를 적절히 구성했습니다.CascadeType.ALL설정과 함께 JPA가 관계를 자동으로 관리합니다.apps/commerce-api/src/main/java/com/loopers/domain/point/PointService.java (1)
38-44: 에러 타입 분류가 개선되었습니다.유효성 검증 실패(잘못된 포인트 금액, 잔액 부족)를
NOT_FOUND에서BAD_REQUEST로 변경하여 의미론적으로 올바른 HTTP 상태 코드를 반환하도록 개선했습니다.apps/commerce-api/src/test/java/com/loopers/domain/user/UserTest.java (1)
40-51: 테스트 변수명이 명확해졌습니다.생년월일 형식 검증 테스트에서
invalidEmail과birth변수를invalidBirth로 변경하여 테스트 의도를 명확하게 표현했습니다. 이제 유효한 이메일과 잘못된 생년월일 형식을 사용하여 올바르게 검증합니다.apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1ApiSpec.java (1)
25-25: API 문서 개선이 적절합니다.기존 description이 "조회할 회원 ID"로 잘못 기재되어 있었으나, "충전할 포인트 정보를 포함한 요청"으로 수정하여 chargePoint 메서드의 파라미터 설명이 정확해졌습니다.
docs/3round/3round.md (1)
27-60: 3라운드 과제 완료 상태가 잘 반영되었습니다.도메인 모델링, 테스트 작성, 아키텍처 설계 등 주요 체크리스트 항목들이 완료 상태로 업데이트되어 프로젝트 진행 상황이 명확하게 문서화되었습니다.
apps/commerce-api/src/main/java/com/loopers/domain/product/ProductDomainService.java (1)
31-40: 읽기 전용 트랜잭션 경계 추가가 적절합니다.여러 Repository(Product, Brand, Like)를 조회하는 메서드에
@Transactional(readOnly = true)를 추가하여 조회 일관성을 보장하고, 데이터베이스 수준에서의 최적화 힌트를 제공합니다.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java (2)
43-44: 에러 처리 개선이 적절합니다.상품을 찾을 수 없을 때
CoreException을 명시적으로 던지도록 변경하여, 애플리케이션 전반에 걸쳐 일관된 에러 처리가 가능하며 더 명확한 에러 메시지를 제공합니다.
50-51: 에러 처리 개선이 적절합니다.incrementLikeCount와 동일한 패턴으로 명시적인 예외 처리를 적용하여 코드 일관성을 유지합니다.
apps/commerce-api/src/main/java/com/loopers/domain/product/Product.java (2)
88-93: 가시성 변경이 적절합니다.
requireValidLikeCount는 생성자 내부에서만 사용되는 검증 메서드이므로,private으로 변경하여 캡슐화를 강화한 것이 적절합니다.
96-103: 검증 로직 개선이 적절합니다.null 체크를 음수 체크와 분리하여 더 명확한 에러 메시지를 제공하며, 다른 검증 메서드들과 일관된 패턴을 유지합니다.
apps/commerce-batch/src/main/resources/application.yml (1)
1-54: 배치 애플리케이션 설정이 적절합니다.
web-application-type: none설정과 프로파일별initialize-schema전략이 배치 애플리케이션에 적합합니다. local/test 환경에서는 스키마를 자동 생성하고, 다른 환경에서는 수동 관리하는 접근 방식이 합리적입니다.Based on learnings, 배치 작업 구현은 개발 환경용이며 프로덕션 배포 전 EDA로 전환될 예정이므로 현재의 프로파일 기반 설정이 적절합니다.
apps/commerce-batch/src/main/java/com/loopers/batch/listener/StepMonitorListener.java (1)
18-21: Step 시작 로깅이 적절합니다.Step 실행 전 로깅이 올바르게 구현되었습니다.
apps/commerce-batch/src/test/java/com/loopers/CommerceBatchApplicationTest.java (1)
1-10: 컨텍스트 로드 테스트가 적절합니다.배치 애플리케이션의 Spring 컨텍스트가 올바르게 초기화되는지 확인하는 기본적인 스모크 테스트입니다.
apps/commerce-batch/src/main/java/com/loopers/batch/listener/JobListener.java (1)
25-52: Job 실행 시간 계산 로직이 적절합니다.실행 시간을 밀리초로 저장하고, 종료 시 시/분/초로 변환하여 가독성 있게 로깅하는 구현이 우수합니다.
apps/commerce-batch/src/main/java/com/loopers/batch/listener/ChunkListener.java (1)
14-20: コードは Spring Batch 5.2.2 で正しい実装です。変更は不要です。このプロジェクトは Spring Boot 3.4.4 を使用しており、これは Spring Batch 5.2.2 をバンドルしています。Spring Batch 5.2.2 では
ChunkContextパラメータを使用する@AfterChunkメソッドが標準の API です。Spring Batch 6.x で
Chunk<T>への API 変更が行われましたが、このプロジェクトでは Spring Batch 5.2.2 を使用しているため、現在のコードは問題ありません。Likely an incorrect or invalid review comment.
apps/commerce-batch/src/main/java/com/loopers/CommerceBatchApplication.java (1)
20-23: 배치 애플리케이션에 적합한 종료 패턴입니다.
SpringApplication.exit()와System.exit()를 사용하는 패턴은 배치 애플리케이션에서 적절한 종료 코드를 반환하기 위한 표준 방식입니다. 다른 모듈(예: PaymentGatewayApplication.kt)과는 다른 패턴이지만, 배치 작업의 성공/실패를 외부 스케줄러에 전달하기 위해 필요한 방식입니다.apps/commerce-batch/src/test/java/com/loopers/job/demo/DemoJobE2ETest.java (1)
58-75: 테스트 로직이 올바릅니다.성공 케이스를 적절하게 검증하고 있으며, JobParameters를 통해 requestDate를 전달하고 COMPLETED 상태를 확인합니다.
apps/commerce-batch/src/main/java/com/loopers/batch/job/demo/step/DemoTasklet.java (1)
19-20: Job 파라미터 주입이 올바릅니다.
@StepScope빈에서 SpEL을 사용한 job 파라미터 주입이 정확하게 구현되었습니다.apps/commerce-batch/src/main/java/com/loopers/batch/job/demo/DemoJobConfig.java (2)
19-29: 설정 구조가 올바릅니다.조건부 로딩, 의존성 주입, 상수 정의가 Spring Batch 베스트 프랙티스를 잘 따르고 있습니다.
31-38: Job 빌더 구성이 적절합니다.
RunIdIncrementer, 리스너, 스텝 연결이 올바르게 구성되었습니다.
| - name: PR Agent action step | ||
| uses: Codium-ai/pr-agent@main |
There was a problem hiding this comment.
액션 버전을 특정 커밋 SHA로 고정하세요.
@main 브랜치를 사용하면 항상 최신 코드가 실행되어 공급망 보안 위험이 발생할 수 있습니다. GitHub Actions 보안 모범 사례에 따라 third-party 액션은 특정 커밋 SHA로 고정해야 합니다.
🔎 권장 수정 사항
- - name: PR Agent action step
- uses: Codium-ai/pr-agent@main
+ - name: PR Agent action step
+ uses: Codium-ai/pr-agent@<specific-commit-sha>최신 릴리스의 커밋 SHA를 확인하려면 다음 스크립트를 실행하세요:
#!/bin/bash
# Codium-ai/pr-agent 리포지토리의 최신 릴리스 태그 확인
gh api repos/Codium-ai/pr-agent/releases/latest --jq '.tag_name'Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/workflows/main.yml lines 9-10: the workflow currently uses the
third-party action via "uses: Codium-ai/pr-agent@main"; replace the branch ref
with a specific commit SHA to pin the action and mitigate supply-chain risk.
Find the commit SHA for the desired release (via the repo's latest release or
GitHub UI), then update the uses line to "uses: Codium-ai/pr-agent@<COMMIT_SHA>"
so the workflow always runs that exact commit.
| uses: Codium-ai/pr-agent@main | ||
| env: | ||
| OPENAI_KEY: ${{ secrets.OPENAI_KEY }} | ||
| GITHUB_TOKEN: ${{ secrets.G_TOKEN }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 워크플로우 파일 전체 내용 확인
cat -n .github/workflows/main.ymlRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 487
🏁 Script executed:
# 리포지토리에서 G_TOKEN 참조 검색
rg "G_TOKEN" --type yaml --type mdRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 70
🏁 Script executed:
# 워크플로우 파일에서 GITHUB_TOKEN 사용 패턴 확인
rg "GITHUB_TOKEN|secrets\." .github/workflows/ -A 2 -B 2Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 341
🌐 Web query:
Codium-ai pr-agent GitHub GITHUB_TOKEN environment variable requirements documentation
💡 Result:
Summary — GITHUB_TOKEN for Codium‑AI / pr-agent (GitHub Action)
- pr-agent expects a GitHub token (you can use the automatically-created secrets.GITHUB_TOKEN) to call the GitHub API. [1][2]
- Recommended job permissions in workflows depend on the features you enable; typical minimal permissions are:
- contents: read (to read repo files) and
- pull-requests: write (to post reviews/comments) — or issues: write for issue-based workflows. Set these with the workflow permissions key. [2][1][3]
- If your workflow needs actions that GITHUB_TOKEN cannot grant (or your organization restricts GITHUB_TOKEN write access), use a PAT or a GitHub App installation token with the appropriate repo scopes. [3][1]
Sources
- pr-agent GitHub / Qodo Merge docs (Run as GitHub Action). [1]
- pr-agent "Help Docs" example workflow showing env and permissions (contents read, issues/pull-requests write). [2]
- GitHub Actions docs on GITHUB_TOKEN and configuring permissions (how to change scopes, when to use PAT/GitHub App). [3]
🏁 Script executed:
# 워크플로우 파일의 전체 구조 확인 (숨겨진 부분이 있는지 확인)
wc -l .github/workflows/main.ymlRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 117
secrets.GITHUB_TOKEN 사용으로 변경하세요.
Codium-ai/pr-agent는 자동으로 제공되는 secrets.GITHUB_TOKEN을 사용하도록 문서화되어 있습니다. 커스텀 시크릿 G_TOKEN은 불필요하며, 표준 GitHub 토큰으로 변경하면 됩니다:
권장 수정
env:
OPENAI_KEY: ${{ secrets.OPENAI_KEY }}
- GITHUB_TOKEN: ${{ secrets.G_TOKEN }}
+ GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| GITHUB_TOKEN: ${{ secrets.G_TOKEN }} | |
| env: | |
| OPENAI_KEY: ${{ secrets.OPENAI_KEY }} | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
🤖 Prompt for AI Agents
In .github/workflows/main.yml around line 13 the workflow sets GITHUB_TOKEN from
the custom secret secrets.G_TOKEN; replace that reference with the built-in
secrets.GITHUB_TOKEN so the workflow uses the automatic GitHub token, and
update/remove any other occurrences of G_TOKEN in the workflow file to avoid
references to the custom secret.
| @JobScope | ||
| @Bean(STEP_DEMO_SIMPLE_TASK_NAME) | ||
| public Step categorySyncStep() { | ||
| return new StepBuilder(STEP_DEMO_SIMPLE_TASK_NAME, jobRepository) | ||
| .tasklet(demoTasklet, new ResourcelessTransactionManager()) | ||
| .listener(stepMonitorListener) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
메서드명과 스텝명의 불일치를 수정하세요.
메서드명 categorySyncStep()은 카테고리 동기화를 암시하지만, 실제로는 STEP_DEMO_SIMPLE_TASK_NAME ("demoSimpleTask")이라는 이름의 데모 스텝을 생성합니다. 이러한 불일치는 코드 가독성을 해치고 유지보수를 어렵게 만듭니다.
🔎 제안된 수정
@JobScope
@Bean(STEP_DEMO_SIMPLE_TASK_NAME)
- public Step categorySyncStep() {
+ public Step demoSimpleTaskStep() {
return new StepBuilder(STEP_DEMO_SIMPLE_TASK_NAME, jobRepository)
.tasklet(demoTasklet, new ResourcelessTransactionManager())
.listener(stepMonitorListener)
.build();
}그리고 Job 빈에서도 메서드명을 업데이트하세요:
@Bean(JOB_NAME)
public Job demoJob() {
return new JobBuilder(JOB_NAME, jobRepository)
.incrementer(new RunIdIncrementer())
- .start(categorySyncStep())
+ .start(demoSimpleTaskStep())
.listener(jobListener)
.build();
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/batch/job/demo/DemoJobConfig.java
around lines 40 to 47, the method name categorySyncStep() does not match the
step name STEP_DEMO_SIMPLE_TASK_NAME ("demoSimpleTask"); rename the method to
demoSimpleTaskStep (or another name that clearly matches the
STEP_DEMO_SIMPLE_TASK_NAME) and update any references (including the Job @Bean
method) to use the new method name so method names and step names are consistent
for readability and maintainability.
| log.info( | ||
| "청크 종료: readCount: ${chunkContext.stepContext.stepExecution.readCount}, " + | ||
| "writeCount: ${chunkContext.stepContext.stepExecution.writeCount}" | ||
| ); |
There was a problem hiding this comment.
Java는 문자열 보간을 지원하지 않습니다.
로그 메시지에서 ${} 구문을 사용하고 있지만, Java는 이러한 문자열 보간(string interpolation)을 지원하지 않습니다. 실제 값이 출력되지 않고 "${chunkContext.stepContext..." 문자열 그대로 로그에 기록됩니다.
🔎 올바른 로깅 방식
SLF4J의 파라미터화된 로깅을 사용하세요:
log.info(
- "청크 종료: readCount: ${chunkContext.stepContext.stepExecution.readCount}, " +
- "writeCount: ${chunkContext.stepContext.stepExecution.writeCount}"
+ "청크 종료: readCount: {}, writeCount: {}",
+ chunkContext.getStepContext().getStepExecution().getReadCount(),
+ chunkContext.getStepContext().getStepExecution().getWriteCount()
);🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/batch/listener/ChunkListener.java
around lines 16–19, the log message currently uses Java-incompatible `${...}`
interpolation so the expressions are logged literally; replace the interpolation
with SLF4J parameterized logging by using {} placeholders and pass the actual
values as separate arguments (retrieve them with the appropriate getters, e.g.,
chunkContext.getStepContext().getStepExecution().getReadCount() and
getWriteCount()), ensuring you pass the values in the same order as the
placeholders.
| @BeforeJob | ||
| void beforeJob(JobExecution jobExecution) { | ||
| log.info("Job '${jobExecution.jobInstance.jobName}' 시작"); | ||
| jobExecution.getExecutionContext().putLong("startTime", System.currentTimeMillis()); | ||
| } |
There was a problem hiding this comment.
Java는 문자열 보간을 지원하지 않습니다.
Line 21의 로그 메시지에서 ${jobExecution.jobInstance.jobName} 구문을 사용하고 있지만, Java는 이러한 문자열 보간을 지원하지 않습니다. 실제 Job 이름이 출력되지 않고 리터럴 문자열이 그대로 로그에 기록됩니다.
🔎 올바른 로깅 방식
SLF4J의 파라미터화된 로깅을 사용하세요:
- log.info("Job '${jobExecution.jobInstance.jobName}' 시작");
+ log.info("Job '{}' 시작", jobExecution.getJobInstance().getJobName());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @BeforeJob | |
| void beforeJob(JobExecution jobExecution) { | |
| log.info("Job '${jobExecution.jobInstance.jobName}' 시작"); | |
| jobExecution.getExecutionContext().putLong("startTime", System.currentTimeMillis()); | |
| } | |
| @BeforeJob | |
| void beforeJob(JobExecution jobExecution) { | |
| log.info("Job '{}' 시작", jobExecution.getJobInstance().getJobName()); | |
| jobExecution.getExecutionContext().putLong("startTime", System.currentTimeMillis()); | |
| } |
🤖 Prompt for AI Agents
In apps/commerce-batch/src/main/java/com/loopers/batch/listener/JobListener.java
around lines 19 to 23, the log uses a Java-incompatible string interpolation
("${...}") so the job name won't be printed; replace it with SLF4J parameterized
logging and the proper getter calls (e.g. log.info("Job '{}' 시작",
jobExecution.getJobInstance().getJobName());) and ensure you use
jobExecution.getJobInstance().getJobName() (or the correct accessor) rather than
an interpolated string.
| @Override | ||
| public ExitStatus afterStep(@Nonnull StepExecution stepExecution) { | ||
| if (!stepExecution.getFailureExceptions().isEmpty()) { | ||
| var jobName = stepExecution.getJobExecution().getJobInstance().getJobName(); | ||
| var exceptions = stepExecution.getFailureExceptions().stream() | ||
| .map(Throwable::getMessage) | ||
| .filter(Objects::nonNull) | ||
| .collect(Collectors.joining("\n")); | ||
| log.info( | ||
| """ | ||
| [에러 발생] | ||
| jobName: {} | ||
| exceptions: | ||
| {} | ||
| """.trim(), jobName, exceptions | ||
| ); | ||
| // error 발생 시 slack 등 다른 채널로 모니터 전송 | ||
| return ExitStatus.FAILED; | ||
| } | ||
| return ExitStatus.COMPLETED; | ||
| } |
There was a problem hiding this comment.
에러 로깅 시 적절한 로그 레벨을 사용하세요.
Line 31에서 실패 예외 정보를 log.info로 기록하고 있습니다. 에러 상황은 log.error를 사용해야 모니터링 시스템에서 적절히 감지하고 알람을 발생시킬 수 있습니다.
🔎 로그 레벨 수정
- log.info(
+ log.error(
"""
[에러 발생]
jobName: {}
exceptions:
{}
""".trim(), jobName, exceptions
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Override | |
| public ExitStatus afterStep(@Nonnull StepExecution stepExecution) { | |
| if (!stepExecution.getFailureExceptions().isEmpty()) { | |
| var jobName = stepExecution.getJobExecution().getJobInstance().getJobName(); | |
| var exceptions = stepExecution.getFailureExceptions().stream() | |
| .map(Throwable::getMessage) | |
| .filter(Objects::nonNull) | |
| .collect(Collectors.joining("\n")); | |
| log.info( | |
| """ | |
| [에러 발생] | |
| jobName: {} | |
| exceptions: | |
| {} | |
| """.trim(), jobName, exceptions | |
| ); | |
| // error 발생 시 slack 등 다른 채널로 모니터 전송 | |
| return ExitStatus.FAILED; | |
| } | |
| return ExitStatus.COMPLETED; | |
| } | |
| @Override | |
| public ExitStatus afterStep(@Nonnull StepExecution stepExecution) { | |
| if (!stepExecution.getFailureExceptions().isEmpty()) { | |
| var jobName = stepExecution.getJobExecution().getJobInstance().getJobName(); | |
| var exceptions = stepExecution.getFailureExceptions().stream() | |
| .map(Throwable::getMessage) | |
| .filter(Objects::nonNull) | |
| .collect(Collectors.joining("\n")); | |
| log.error( | |
| """ | |
| [에러 발생] | |
| jobName: {} | |
| exceptions: | |
| {} | |
| """.trim(), jobName, exceptions | |
| ); | |
| // error 발생 시 slack 등 다른 채널로 모니터 전송 | |
| return ExitStatus.FAILED; | |
| } | |
| return ExitStatus.COMPLETED; | |
| } |
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/batch/listener/StepMonitorListener.java
around lines 23 to 43, the code logs failure information with log.info; change
this to log.error so error conditions are recorded at the correct severity.
Replace the log.info call with log.error and keep the existing formatted message
and variables (jobName and exceptions); if available, also log the original
exception(s) (e.g., pass a representative Throwable or include stack traces) to
ensure the monitoring/alerting systems receive full error context.
| @DisplayName("jobParameter 중 requestDate 인자가 주어지지 않았을 때, demoJob 배치는 실패한다.") | ||
| @Test | ||
| void shouldNotSaveCategories_whenApiError() throws Exception { |
There was a problem hiding this comment.
테스트 메서드명이 실제 테스트 내용과 일치하지 않습니다.
메서드명 shouldNotSaveCategories_whenApiError는 API 오류 시 카테고리 저장 실패를 테스트하는 것처럼 보이지만, 실제로는 requestDate 파라미터가 누락되었을 때 배치가 실패하는지를 검증합니다. 테스트명을 실제 검증 내용에 맞게 수정하세요.
🔎 제안된 수정
- @DisplayName("jobParameter 중 requestDate 인자가 주어지지 않았을 때, demoJob 배치는 실패한다.")
+ @DisplayName("requestDate 파라미터가 누락되면 demoJob 배치는 실패한다.")
@Test
- void shouldNotSaveCategories_whenApiError() throws Exception {
+ void shouldFail_whenRequestDateParameterIsMissing() throws Exception {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @DisplayName("jobParameter 중 requestDate 인자가 주어지지 않았을 때, demoJob 배치는 실패한다.") | |
| @Test | |
| void shouldNotSaveCategories_whenApiError() throws Exception { | |
| @DisplayName("requestDate 파라미터가 누락되면 demoJob 배치는 실패한다.") | |
| @Test | |
| void shouldFail_whenRequestDateParameterIsMissing() throws Exception { |
🤖 Prompt for AI Agents
In apps/commerce-batch/src/test/java/com/loopers/job/demo/DemoJobE2ETest.java
around lines 42-44, the test method name shouldNotSaveCategories_whenApiError
does not match the tested behavior (batch fails when the requestDate
jobParameter is missing); rename the method to reflect that behavior (e.g.,
shouldFailBatch_whenRequestDateMissing or shouldThrowWhenRequestDateMissing) and
update any references/usages accordingly so the method name clearly indicates it
validates batch failure on a missing requestDate parameter.
| ## ✅ Checklist | ||
|
|
||
| ### 🧱 Spring Batch | ||
|
|
||
| - [x] Spring Batch Job 을 작성하고, 파라미터 기반으로 동작시킬 수 있다. | ||
| - [x] Chunk Oriented Processing (Reader/Processor/Writer or Tasklet) 기반의 배치 처리를 구현했다. | ||
| - [x] 집계 결과를 저장할 Materialized View 의 구조를 설계하고 올바르게 적재했다. | ||
|
|
||
| ### 🧩 Ranking API | ||
|
|
||
| - [x] API 가 일간, 주간, 월간 랭킹을 제공하며 조회해야 하는 형태에 따라 적절한 데이터를 기반으로 랭킹을 제공한다. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify Spring Batch implementation files exist
echo "=== Checking for Spring Batch Job configurations ==="
fd -e java -e kt | rg -l "JobBuilderFactory|StepBuilderFactory|@EnableBatchProcessing" || echo "No matches found"
echo -e "\n=== Checking for Chunk-Oriented Processing implementations ==="
fd -e java -e kt | rg -l "ItemReader|ItemProcessor|ItemWriter|Tasklet" || echo "No matches found"
echo -e "\n=== Checking for Materialized View table definitions ==="
rg "mv_product_rank_weekly|mv_product_rank_monthly" --type sql || echo "No matches found"
echo -e "\n=== Checking Ranking API implementation ==="
rg "ranking" --type java --type kt -i | head -20 || echo "No matches found"
echo -e "\n=== Listing all Java/Kotlin files ==="
fd -e java -e kt | head -30
echo -e "\n=== Listing SQL files ==="
fd -e sql | head -30Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 2967
🏁 Script executed:
#!/bin/bash
# Check RankingFacade implementation
echo "=== Checking RankingFacade.java ==="
find . -name "RankingFacade.java" -exec cat -n {} \;
echo -e "\n=== Searching for all SQL files ==="
fd -e sql -e ddl -e h2
echo -e "\n=== Searching for batch-related classes ==="
rg -i "batch|job|step" --type java | grep -i "class\|@" | head -20
echo -e "\n=== Searching for ItemReader, ItemProcessor, ItemWriter ==="
rg -i "itemreader|itemprocessor|itemwriter" --type java
echo -e "\n=== Searching for Tasklet ==="
rg -i "tasklet" --type java
echo -e "\n=== Searching for materialized view or view tables ==="
rg -i "mv_product|rank" --type sql --type java | head -20
echo -e "\n=== Checking schema files ==="
fd -e sql -path "*/schema*" -o -path "*/migration*"Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 8885
🏁 Script executed:
#!/bin/bash
# Check DemoJobConfig in detail
echo "=== DemoJobConfig.java content ==="
find . -name "DemoJobConfig.java" -exec cat -n {} \;
echo -e "\n=== Check RankingService.java ==="
find . -name "RankingService.java" -exec cat -n {} \;
echo -e "\n=== Search for all database/SQL related files ==="
fd -e sql -o -e ddl -o -e h2 2>/dev/null || echo "No SQL files found with fd"
echo -e "\n=== Search for schema initialization files ==="
fd "schema" -o "init" -o "migration"
echo -e "\n=== Check application properties/yml for batch jobs ==="
find . -name "*.properties" -o -name "*.yml" -o -name "*.yaml" | xargs grep -l "batch\|ranking" 2>/dev/null || echo "No batch config found in properties"
echo -e "\n=== Check for other batch job configurations ==="
find . -path "*/batch/job/*" -name "*Config.java"Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 5940
🏁 Script executed:
#!/bin/bash
# Check RankingKeyGenerator
echo "=== RankingKeyGenerator.java ==="
find . -name "RankingKeyGenerator.java" -exec cat -n {} \;
echo -e "\n=== Check application.yml for batch configuration ==="
cat ./apps/commerce-batch/src/main/resources/application.yml
echo -e "\n=== Search for ranking-related batch jobs ==="
rg -i "ranking.*job\|ranking.*batch" --type java
echo -e "\n=== Check all files in batch/job directory ==="
find ./apps/commerce-batch/src/main/java/com/loopers/batch/job -type f -name "*.java"
echo -e "\n=== Search for database schema or JPA entity for ranking ==="
rg "RankingEntity\|@Table.*ranking\|@Entity.*ranking" --type java -iRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 2896
체크리스트 항목 3, 4번을 재검토하기 바랍니다.
검증 결과:
- ✅ Spring Batch Job 및 파라미터 기반 실행: 구현 완료 (
DemoJobConfig) - ✅ Chunk-Oriented Processing: Tasklet 기반 구현 완료 (
DemoTasklet) - ❌ Materialized View (
mv_product_rank_weekly,mv_product_rank_monthly): SQL 스키마 파일을 찾을 수 없습니다. 현재 시스템은 Redis 기반 순위 저장소를 사용하고 있습니다. - ❌ Ranking API 일간/주간/월간 조회: 일간 조회만 구현되어 있습니다.
RankingKeyGenerator는 "yyyyMMdd" 형식(일간)만 지원하며, 주간/월간 집계 로직이 없습니다.
Materialized View 스키마를 구현하거나, Ranking API에 주간/월간 데이터 조회 기능을 추가해야 합니다.
🤖 Prompt for AI Agents
In docs/10round/10round.md around lines 43 to 53, update the checklist and
implementation to address missing materialized view schemas and incomplete
ranking API: either add SQL schema files and migration steps for
mv_product_rank_weekly and mv_product_rank_monthly (and document where they
live) or implement/describe additions to RankingKeyGenerator and aggregation
logic to support "weekly" and "monthly" keys and queries (including key formats,
aggregation schedule, and API endpoints) so the checklist items reflect actual
deliverables and the README points to the new files/functions.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (10)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankId.java (1)
10-13: @NoArgsConstructor에 access level 지정 고려관련 MV 엔티티들(
MvProductRankWeekly,MvProductRankMonthly)은@NoArgsConstructor(access = AccessLevel.PROTECTED)를 사용하고 있습니다. 일관성을 위해 동일한 패턴을 적용하는 것을 권장합니다.🔎 제안 수정
@Getter @Embeddable -@NoArgsConstructor +@NoArgsConstructor(access = AccessLevel.PROTECTED) public class ProductRankId implements Serializable {
AccessLevelimport 추가:+import lombok.AccessLevel;apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankScorePolicy.java (1)
5-13: LGTM! 가중치 설정 외부화 고려점수 계산 정책이 명확하고 단일 책임 원칙을 잘 따르고 있습니다. 가중치 합계가 1.0으로 정규화되어 있어 좋습니다.
향후 비즈니스 요구사항 변경 시 가중치 조정이 필요할 수 있으므로,
@ConfigurationProperties또는@Value를 통해 외부 설정으로 관리하는 것도 고려해 볼 수 있습니다.apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingPeriod.java (1)
49-50:RankingPeriodResolver와 중복 로직 존재
RankingPeriod(commerce-api)와RankingPeriodResolver(commerce-batch)에 동일한 날짜 파싱 및 기간 키 생성 로직이 중복되어 있습니다:parse(),toYearMonthWeek(),toYearMonth(), 동일한ZONE_ID와FORMATTER.향후 한쪽만 수정되면 불일치가 발생할 수 있습니다. 공통 유틸리티 클래스로 추출하거나, 배치 모듈에서 API 모듈의
RankingPeriod를 재사용하는 것을 고려해 주세요.Also applies to: 76-91
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingMaterializedViewWriter.java (2)
24-24: 단일 스레드 배치 처리에서는 AtomicInteger가 불필요합니다.AtomicInteger는 동시성 제어를 위한 타입이지만, Spring Batch의 ItemWriter는 단일 스레드에서 순차적으로 실행됩니다. 일반 int 필드로 충분합니다.
🔎 제안: 일반 int 필드로 변경
- private final AtomicInteger rankCounter = new AtomicInteger(1); + private int rankCounter = 1;그리고 Line 39와 Line 69도 함께 수정:
- rankCounter.set(1); + rankCounter = 1;- .addValue("rank", rankCounter.getAndIncrement()) + .addValue("rank", rankCounter++)
64-64: period.key() null 안전성을 확인하세요.period.key()가 null을 반환할 가능성이 있다면 DB 제약 조건 위반이나 데이터 무결성 문제가 발생할 수 있습니다. 생성자나 write() 초반에 period와 period.key()가 null이 아닌지 검증하는 것을 권장합니다.
🔎 제안: 생성자에서 유효성 검증 추가
public RankingMaterializedViewWriter( NamedParameterJdbcTemplate jdbcTemplate, RankingPeriod period, String tableName ) { + if (period == null || period.key() == null) { + throw new IllegalArgumentException("period and period.key() must not be null"); + } + if (tableName == null || tableName.isBlank()) { + throw new IllegalArgumentException("tableName must not be null or blank"); + } this.jdbcTemplate = jdbcTemplate; this.period = period; this.tableName = tableName; }Also applies to: 81-81
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java (1)
27-40: 기간별 라우팅 로직이 명확합니다. N+1 쿼리 문제를 검토하세요.period.isDaily()를 사용한 분기 처리가 깔끔하지만, toDto()/toSnapshotDto() 내부에서 각 아이템마다 productService.getProduct()와 brandService.getBrand()를 호출하여 N+1 쿼리 문제가 발생할 수 있습니다.
대량의 랭킹 데이터 조회 시 성능 저하가 우려되므로, 제품과 브랜드 정보를 배치로 로드하는 최적화를 고려하세요.
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingMaterializedViewService.java (1)
46-52: 매퍼 메서드의 중복을 제거할 수 있습니다.fromWeekly()와 fromMonthly()가 동일한 로직을 수행합니다. 공통 헬퍼 메서드로 추출하여 중복을 제거하는 것을 고려하세요.
🔎 제안: 공통 매퍼 메서드 추출
private ProductRankSnapshot fromWeekly(MvProductRankWeekly entity) { - return new ProductRankSnapshot(entity.getRank(), entity.getProductId(), entity.getScore()); + return toSnapshot(entity.getRank(), entity.getProductId(), entity.getScore()); } private ProductRankSnapshot fromMonthly(MvProductRankMonthly entity) { - return new ProductRankSnapshot(entity.getRank(), entity.getProductId(), entity.getScore()); + return toSnapshot(entity.getRank(), entity.getProductId(), entity.getScore()); + } + + private ProductRankSnapshot toSnapshot(int rank, Long productId, double score) { + return new ProductRankSnapshot(rank, productId, score); }apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankAggregationJobConfig.java (3)
28-32: MAX_ITEM_COUNT를 설정 가능하도록 개선을 권장합니다.현재
MAX_ITEM_COUNT = 100으로 하드코딩되어 있어 상위 100개 상품으로 제한됩니다. 이는 "top rankings" 요구사항에 맞는 것으로 보이지만, 운영 중 조정이 필요할 수 있습니다.🔎 설정 가능하도록 개선한 예시
- private static final int MAX_ITEM_COUNT = 100; + @Value("${batch.ranking.max-item-count:100}") + private int maxItemCount;그리고 line 110에서:
- .maxItemCount(MAX_ITEM_COUNT) + .maxItemCount(maxItemCount)
82-98: requestDate 파라미터 검증 추가를 권장합니다.
requestDate잡 파라미터가 null이거나 유효하지 않은 형식일 경우RankingPeriodResolver에서 런타임 오류가 발생할 수 있습니다. 파라미터 검증 로직을 추가하는 것을 권장합니다.🔎 검증 로직 추가 예시
@Bean @StepScope public RankingMaterializedViewWriter weeklyRankingWriter( @Value("#{jobParameters['requestDate']}") String requestDate ) { + if (requestDate == null || requestDate.isBlank()) { + throw new IllegalArgumentException("requestDate parameter is required"); + } RankingPeriod period = RankingPeriodResolver.weekly(requestDate); return new RankingMaterializedViewWriter(jdbcTemplate, period, "mv_product_rank_weekly"); }동일한 검증을
monthlyRankingWriter에도 적용하세요.
88-88: 테이블 명을 상수로 추출하는 것을 고려해주세요.Materialized View 테이블 명이 문자열 리터럴로 하드코딩되어 있습니다. 상수로 추출하면 유지보수성과 타입 안정성이 향상됩니다.
🔎 상수 추출 예시
+ private static final String MV_WEEKLY_TABLE = "mv_product_rank_weekly"; + private static final String MV_MONTHLY_TABLE = "mv_product_rank_monthly"; + @Bean @StepScope public RankingMaterializedViewWriter weeklyRankingWriter( @Value("#{jobParameters['requestDate']}") String requestDate ) { RankingPeriod period = RankingPeriodResolver.weekly(requestDate); - return new RankingMaterializedViewWriter(jdbcTemplate, period, "mv_product_rank_weekly"); + return new RankingMaterializedViewWriter(jdbcTemplate, period, MV_WEEKLY_TABLE); }동일한 방식으로
monthlyRankingWriter도 수정하세요.Also applies to: 97-97
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/commerce-api/src/main/java/com/loopers/application/ranking/ProductRankSnapshot.javaapps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.javaapps/commerce-api/src/main/java/com/loopers/application/ranking/RankingMaterializedViewService.javaapps/commerce-api/src/main/java/com/loopers/application/ranking/RankingPeriod.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankId.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthlyJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankWeeklyJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.javaapps/commerce-batch/src/main/java/com/loopers/batch/domain/metrics/ProductMetrics.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankAggregate.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankAggregationJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankItemProcessor.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankScorePolicy.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingMaterializedViewWriter.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingPeriod.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingPeriodResolver.javasettings.gradle.kts
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-09T10:41:39.297Z
Learnt from: ghojeong
Repo: Loopers-dev-lab/loopers-spring-kotlin-template PR: 25
File: apps/commerce-api/src/main/kotlin/com/loopers/domain/product/ProductRepository.kt:1-12
Timestamp: 2025-11-09T10:41:39.297Z
Learning: In this codebase, domain repository interfaces are allowed to use Spring Data's org.springframework.data.domain.Page and org.springframework.data.domain.Pageable types. This is an accepted architectural decision and should not be flagged as a DIP violation.
Applied to files:
apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthlyJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankWeeklyJpaRepository.java
📚 Learning: 2025-12-19T21:30:16.024Z
Learnt from: toongri
Repo: Loopers-dev-lab/loopers-spring-kotlin-template PR: 68
File: apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/outbox/OutboxEventListener.kt:0-0
Timestamp: 2025-12-19T21:30:16.024Z
Learning: In the Loopers-dev-lab/loopers-spring-kotlin-template Kafka event pipeline, Like events (LikeCreatedEventV1, LikeCanceledEventV1) intentionally use aggregateType="Like" with aggregateId=productId. The aggregateId serves as a partitioning/grouping key (not a unique Like entity identifier), ensuring all like events for the same product go to the same partition for ordering guarantees and aligning with ProductStatisticService's product-based aggregation logic. Using individual like_id would scatter events across partitions and break the statistics aggregation pattern.
Applied to files:
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankAggregate.javaapps/commerce-batch/src/main/java/com/loopers/batch/domain/metrics/ProductMetrics.java
📚 Learning: 2025-11-12T13:04:50.782Z
Learnt from: kilian-develop
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 72
File: core/domain/src/main/java/com/loopers/core/domain/productlike/vo/ProductLikeId.java:5-7
Timestamp: 2025-11-12T13:04:50.782Z
Learning: In the com.loopers codebase, domain entity ID value objects (e.g., ProductLikeId, OrderItemId, ProductId, PaymentId, OrderId, BrandId) are system-generated identifiers and do not require pattern validation (regex, length checks). They are implemented as simple records with a String value and an empty() factory method returning null for unsaved entities. This differs from UserIdentifier, which is a user-supplied login ID that requires format validation. Domain IDs should not be flagged for missing validation logic in the create() method.
<!-- [add_learning]
UserIdentifier와 같은 사용자 입력 ID와 ProductLikeId, OrderItemId 등의 도메인 ID는 검증 패턴이 다릅니다. UserIdentifier는 사용자가 입력하는 로그인 ID로서 정규식, 길이 등의 형식 검증이 필요하지만, 도메인 ID는 시스템에서 생성하는 식별자(UUID, DB 생성 ID)이므로 패턴 검증이 불필요합니다. 도메인 ID VO는 단순한 record와 empty() 팩토리 메서드만으로 충분합니다.
Applied to files:
apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankId.java
📚 Learning: 2025-11-21T03:38:07.494Z
Learnt from: minor7295
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 99
File: apps/commerce-api/src/main/resources/application.yml:26-30
Timestamp: 2025-11-21T03:38:07.494Z
Learning: The batch job implementation for likeCount synchronization in apps/commerce-api is temporary and intended for development environment only. It will be replaced with Event-Driven Architecture (EDA) before production deployment, so production-level configuration concerns (like profile-based initialize-schema settings) are not required.
Applied to files:
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankAggregationJobConfig.java
📚 Learning: 2025-11-27T09:09:24.961Z
Learnt from: sky980221
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 121
File: apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java:22-24
Timestamp: 2025-11-27T09:09:24.961Z
Learning: Product 엔티티 (apps/commerce-api/src/main/java/com/loopers/domain/product/Product.java)는 유즈케이스별로 의도적으로 다른 락 전략을 사용한다: 좋아요 기능에는 비관적 락(findByIdForUpdate)을, 재고 차감에는 낙관적 락(Version + 재시도)을 사용한다.
Applied to files:
apps/commerce-batch/src/main/java/com/loopers/batch/domain/metrics/ProductMetrics.java
🧬 Code graph analysis (6)
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankItemProcessor.java (1)
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankScorePolicy.java (1)
Component(5-14)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java (1)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Dto.java (1)
RankingV1Dto(8-33)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankId.java (2)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.java (1)
Getter(12-78)apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java (1)
Getter(12-78)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.java (2)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java (1)
Getter(12-78)apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankId.java (1)
Getter(10-46)
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankScorePolicy.java (1)
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankItemProcessor.java (1)
Component(8-27)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java (2)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.java (1)
Getter(12-78)apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankId.java (1)
Getter(10-46)
🔇 Additional comments (17)
settings.gradle.kts (1)
5-5: LGTM!새 배치 모듈 추가가 기존 프로젝트 구조와 일관성 있게 적용되었습니다.
apps/commerce-api/src/main/java/com/loopers/application/ranking/ProductRankSnapshot.java (1)
3-7: LGTM!랭킹 스냅샷 DTO로 적절하게 설계되었습니다. Record를 사용하여 불변성을 보장하고 있으며, 데이터 전달 목적에 맞는 간결한 구조입니다.
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankAggregate.java (1)
3-8: LGTM!배치 처리용 집계 데이터 레코드로 적절합니다.
ProductMetrics에서 처리된 결과를 담는 중간 객체로서의 역할이 명확합니다.apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankId.java (1)
30-45: LGTM!
equals()와hashCode()구현이 복합 키의 모든 필드를 올바르게 사용하고 있으며,Objects.equals와Objects.hash를 통해 null-safe하게 처리되어 있습니다.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingPeriod.java (1)
3-5: 두 개의 서로 다른 RankingPeriod 클래스를 혼동하고 있습니다.commerce-batch의 RankingPeriod 레코드는 단순히 기간 키를 보유하기 위한 것으로 설계된 것이 맞습니다.
from(),isDaily(),resolveStartDate(),resolveKey()메서드는 commerce-api 모듈의 RankingPeriod 열거형에 존재하며, 이는 서로 다른 모듈의 서로 다른 목적을 위한 별개의 클래스입니다. commerce-api의 RankingPeriod 열거형이 API 계층에서 기간 타입 변환과 날짜 로직을 처리하는 반면, commerce-batch의 RankingPeriod 레코드는 배치 작업에서 기간 키만 전달하는 용도로 사용됩니다.Likely an incorrect or invalid review comment.
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1ApiSpec.java (1)
20-22: period 파라미터 입력값은 안전하게 처리됨
period파라미터의 유효성 검증이RankingPeriod.from()메서드를 통해 구현되어 있습니다. 유효하지 않은 값(예:"yearly", 오타 등)이 입력되면 기본값인DAILY로 자동으로 변환되므로, 예외 발생 없이 안전하게 처리됩니다. 대소문자 구분도 없어 사용성이 좋습니다.다만, 명시적 예외 처리(invalid value에 대한 에러 반환)가 필요한 경우,
RankingPeriod.from()을 수정하여orElse(DAILY)대신 예외를 던지도록 변경할 수 있습니다.apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java (1)
20-30: LGTM!기간 파라미터 지원이 깔끔하게 구현되었습니다.
RankingPeriod.from(period)이 null과 인식되지 않는 값에 대해 DAILY로 기본값 처리하므로 안전합니다. 페이지네이션 로직도 잘 유지되어 있습니다.apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java (1)
1-78: LGTM!
MvProductRankMonthly와 일관된 패턴으로 잘 구현되었습니다. 팩토리 메서드, 임베디드 ID 사용, null-safe 편의 getter가 적절합니다.apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankWeeklyJpaRepository.java (1)
8-13: LGTM!
MvProductRankMonthlyJpaRepository와 일관된 구조입니다.deleteByIdPeriodKey메서드의 트랜잭션 처리는 월간 저장소 리뷰에서 언급한 내용과 동일하게 적용됩니다.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankItemProcessor.java (1)
8-26: LGTM! 깔끔한 Spring Batch 프로세서 구현입니다.null 처리가 적절하고, 점수 계산 로직을 ProductRankScorePolicy로 분리하여 책임이 명확합니다. Spring Batch 패턴을 잘 따르고 있습니다.
apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.java (1)
12-77: LGTM! 표준 JPA 엔티티 패턴을 잘 따르고 있습니다.복합키(ProductRankId) 사용과 정적 팩토리 메서드 패턴이 적절하며, MvProductRankWeekly와 일관된 구조를 유지하고 있습니다.
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingMaterializedViewWriter.java (2)
36-40: 배치 재시도 전략을 검증하세요.open() 메서드에서 기존 데이터를 삭제한 후 write()가 진행되므로, 배치가 중간에 실패하면 데이터가 부분적으로 손실됩니다. PR 설명에서 언급된 재시도 전략(item-level vs full job restart)과 관련하여, 현재 구조는 전체 작업 재시작을 가정합니다.
이 설계가 의도적인지 확인하고, 배치 실패 시 영향 범위를 문서화하는 것을 권장합니다.
85-90: 동적 테이블명에 대한 SQL 인젝션 위험을 검토하세요.tableName을 String.formatted()로 직접 SQL에 삽입하면 SQL 인젝션 위험이 있습니다. tableName이 설정에서 제공되고 신뢰할 수 있는 값이라면 문제없지만, 런타임에 검증되지 않은 입력에서 오는 경우 위험합니다.
tableName의 출처가 안전한지 확인하거나, 허용된 테이블명 목록과 대조하여 검증하는 것을 고려하세요.
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java (2)
43-48: LGTM! count 메서드의 기간별 분기 처리가 적절합니다.
62-81: LGTM! createRankingInfo 헬퍼 메서드 추출로 코드 중복이 제거되었습니다.toDto()와 toSnapshotDto() 모두 createRankingInfo()를 재사용하여 일관성이 향상되었습니다.
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankAggregationJobConfig.java (2)
23-26: 조건부 로딩이 적절하게 구성되었습니다.
@ConditionalOnProperty를 사용하여 특정 잡만 활성화하는 패턴이 올바르게 적용되었습니다.
42-52: 잡 정의가 올바르게 구성되었습니다.주간 랭킹 단계 후 월간 랭킹 단계가 순차적으로 실행되는 구조가 적절합니다.
| public List<ProductRankSnapshot> getRankings(RankingPeriod period, String periodKey, int page, int size) { | ||
| List<ProductRankSnapshot> snapshots = fetch(period, periodKey); | ||
| int safeSize = Math.max(size, 1); | ||
| long skip = (long) Math.max(page, 0) * safeSize; | ||
| return snapshots.stream() | ||
| .skip(skip) | ||
| .limit(safeSize) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
메모리 내 페이지네이션은 성능 문제를 유발합니다.
fetch() 메서드가 해당 기간의 모든 레코드를 가져온 후 stream().skip().limit()으로 메모리에서 페이지네이션을 수행합니다. 랭킹 데이터가 수천 개 이상일 경우 불필요한 데이터를 모두 로드하여 메모리와 DB 부하가 증가합니다.
리포지토리 레벨에서 Pageable을 사용한 DB 페이지네이션으로 변경하세요.
🔎 제안: 리포지토리에서 페이지네이션 처리
리포지토리 메서드를 다음과 같이 수정:
// MvProductRankWeeklyJpaRepository
Page<MvProductRankWeekly> findByIdPeriodKey(String periodKey, Pageable pageable);그리고 서비스 메서드를 다음과 같이 수정:
public List<ProductRankSnapshot> getRankings(RankingPeriod period, String periodKey, int page, int size) {
- List<ProductRankSnapshot> snapshots = fetch(period, periodKey);
int safeSize = Math.max(size, 1);
- long skip = (long) Math.max(page, 0) * safeSize;
- return snapshots.stream()
- .skip(skip)
- .limit(safeSize)
- .toList();
+ int safePage = Math.max(page, 0);
+ Pageable pageable = PageRequest.of(safePage, safeSize, Sort.by("rank").ascending());
+ return fetchPaged(period, periodKey, pageable);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingMaterializedViewService.java
around lines 18 to 26, the current implementation fetches all snapshots into
memory and uses stream().skip().limit() for pagination which causes high memory
and DB load; change to DB-level pagination by adding a repository method that
accepts a Pageable (e.g., findByIdPeriodKey(String periodKey, Pageable
pageable)) and modify this service method to build a PageRequest from the
incoming page and size (clamp page >= 0 and size >= 1), call the repository to
retrieve a Page<MvProductRankWeekly>, map the page content to
List<ProductRankSnapshot>, and return that list (this avoids loading all records
into memory and delegates pagination to the database).
| public long count(RankingPeriod period, String periodKey) { | ||
| return fetch(period, periodKey).size(); | ||
| } |
There was a problem hiding this comment.
count 구현이 비효율적입니다.
모든 레코드를 fetch()로 가져온 후 size()를 호출하는 것은 매우 비효율적입니다. 리포지토리에 COUNT 쿼리를 추가하여 DB 레벨에서 집계하세요.
🔎 제안: COUNT 쿼리 추가
리포지토리에 다음 메서드 추가:
// MvProductRankWeeklyJpaRepository
long countByIdPeriodKey(String periodKey);그리고 서비스 메서드를 다음과 같이 수정:
public long count(RankingPeriod period, String periodKey) {
- return fetch(period, periodKey).size();
+ if (period == RankingPeriod.WEEKLY) {
+ return weeklyRepository.countByIdPeriodKey(periodKey);
+ }
+ if (period == RankingPeriod.MONTHLY) {
+ return monthlyRepository.countByIdPeriodKey(periodKey);
+ }
+ throw new IllegalArgumentException("Unsupported period for MV: " + period);
}🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingMaterializedViewService.java
around lines 28-30, the current count implementation fetches all records then
calls size(), which is inefficient; add a DB-level count method to the
appropriate JPA repository (e.g., long countByIdPeriodKey(String periodKey) or a
signature including period if needed), then change this service method to call
that repository count method and return its result instead of fetch(). Ensure
the repository method name matches your entity's ID mapping and that the service
injects/uses the repository call directly to avoid loading full entities into
memory.
| private static String toYearMonthWeek(LocalDate target) { | ||
| WeekFields weekFields = WeekFields.of(Locale.KOREA); | ||
| int weekBasedYear = target.get(weekFields.weekBasedYear()); | ||
| int week = target.get(weekFields.weekOfWeekBasedYear()); | ||
| return String.format("%04d-W%02d", weekBasedYear, week); | ||
| } |
There was a problem hiding this comment.
Locale.KOREA와 월요일 시작 간 불일치 (RankingPeriodResolver와 동일 이슈)
WEEKLY.resolveStartDate()는 월요일을 기준으로 하지만, toYearMonthWeek()는 Locale.KOREA의 WeekFields를 사용합니다. RankingPeriodResolver에서 언급한 것과 동일한 문제입니다. 일관성을 위해 WeekFields.ISO 사용을 권장합니다.
🔎 수정 제안
private static String toYearMonthWeek(LocalDate target) {
- WeekFields weekFields = WeekFields.of(Locale.KOREA);
+ WeekFields weekFields = WeekFields.ISO;
int weekBasedYear = target.get(weekFields.weekBasedYear());
int week = target.get(weekFields.weekOfWeekBasedYear());
return String.format("%04d-W%02d", weekBasedYear, week);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static String toYearMonthWeek(LocalDate target) { | |
| WeekFields weekFields = WeekFields.of(Locale.KOREA); | |
| int weekBasedYear = target.get(weekFields.weekBasedYear()); | |
| int week = target.get(weekFields.weekOfWeekBasedYear()); | |
| return String.format("%04d-W%02d", weekBasedYear, week); | |
| } | |
| private static String toYearMonthWeek(LocalDate target) { | |
| WeekFields weekFields = WeekFields.ISO; | |
| int weekBasedYear = target.get(weekFields.weekBasedYear()); | |
| int week = target.get(weekFields.weekOfWeekBasedYear()); | |
| return String.format("%04d-W%02d", weekBasedYear, week); | |
| } |
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingPeriod.java
around lines 83 to 88, toYearMonthWeek(...) uses WeekFields.of(Locale.KOREA)
which conflicts with WEEKLY.resolveStartDate()'s Monday-based logic; change it
to use WeekFields.ISO so week-based-year and week-of-week-based-year are
computed consistently (i.e., replace WeekFields.of(Locale.KOREA) with
WeekFields.ISO and ensure the returned format remains "%04d-W%02d").
|
|
||
| List<MvProductRankMonthly> findByIdPeriodKeyOrderByRankAsc(String periodKey); | ||
|
|
||
| void deleteByIdPeriodKey(String periodKey); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the repository file and examine its structure
fd -n "MvProductRankMonthlyJpaRepository" -t fRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 318
🏁 Script executed:
#!/bin/bash
# Search for calls to deleteByIdPeriodKey to see how it's being used
rg "deleteByIdPeriodKey" --type java -C 5Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 2058
🏁 Script executed:
#!/bin/bash
# Also check the repository interface definition
rg "interface MvProductRankMonthly" --type java -A 10Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 969
🏁 Script executed:
#!/bin/bash
# Find all Java files to understand the codebase structure
find apps/commerce-api -name "*.java" -type f | grep -i "rank" | head -20Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 1247
🏁 Script executed:
#!/bin/bash
# Search for usages/invocations of deleteByIdPeriodKey
rg "deleteByIdPeriodKey\(" --type java -B 5 -A 2Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 2058
🏁 Script executed:
#!/bin/bash
# Check if there's a service layer that might call this
find apps/commerce-api -name "*RankMonthly*" -type f | grep -i serviceRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 70
🏁 Script executed:
#!/bin/bash
# Check RankingMaterializedViewService for usage
cat apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingMaterializedViewService.javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 2234
🏁 Script executed:
#!/bin/bash
# Also check RankingFacade
cat apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 2985
🏁 Script executed:
#!/bin/bash
# Search entire codebase for any invocation of deleteByIdPeriodKey
rg "deleteByIdPeriodKey" --type javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 394
🏁 Script executed:
#!/bin/bash
# Also search for "delete" patterns in ranking-related files
rg "\.delete" apps/commerce-api/src/main/java/com/loopers/application/ranking/ --type java -B 3 -A 2Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 70
🏁 Script executed:
#!/bin/bash
# Check if there are other methods with @Modifying in codebase
rg "@Modifying" --type java -B 2 -A 5Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 70
deleteByIdPeriodKey 메서드에 @Modifying 어노테이션 추가 필요
Spring Data JPA의 파생 삭제 쿼리(derived delete query)는 @Modifying 어노테이션이 필수입니다. 현재 메서드에 이 어노테이션이 없으면 DELETE 쿼리가 제대로 실행되지 않습니다.
@Modifying
void deleteByIdPeriodKey(String periodKey);추가로, 이 메서드를 호출하는 서비스 계층의 메서드는 @Transactional 어노테이션으로 감싸져야 합니다.
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthlyJpaRepository.java
around line 12, the derived delete method deleteByIdPeriodKey lacks the required
@Modifying annotation so the DELETE query won't execute; add the @Modifying
annotation to that method declaration and ensure the service method that calls
it is annotated with @Transactional (or otherwise executed within a transaction)
so the delete is applied correctly.
| @Getter | ||
| @Entity | ||
| @Table(name = "product_metrics") | ||
| public class ProductMetrics { |
There was a problem hiding this comment.
JPA 엔티티에 기본 생성자 누락
JPA 명세에 따르면 엔티티 클래스는 반드시 no-arg 생성자가 필요합니다. 현재 ProductMetrics에는 생성자가 정의되어 있지 않고, @Getter만으로는 생성자가 생성되지 않습니다. Hibernate가 결과를 매핑할 때 InstantiationException이 발생할 수 있습니다.
🔎 수정 제안
import jakarta.persistence.Table;
import lombok.Getter;
+import lombok.NoArgsConstructor;
+import lombok.AccessLevel;
@Getter
@Entity
@Table(name = "product_metrics")
+@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class ProductMetrics {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Getter | |
| @Entity | |
| @Table(name = "product_metrics") | |
| public class ProductMetrics { | |
| import jakarta.persistence.Table; | |
| import lombok.Getter; | |
| import lombok.NoArgsConstructor; | |
| import lombok.AccessLevel; | |
| @Getter | |
| @Entity | |
| @Table(name = "product_metrics") | |
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | |
| public class ProductMetrics { |
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/batch/domain/metrics/ProductMetrics.java
around lines 9 to 12, the JPA entity lacks a no-arg constructor which causes
Hibernate to throw InstantiationException; add a JPA-compatible no-argument
constructor (preferably protected) to the class so the ORM can instantiate it,
retaining any existing constructors and Lombok-generated getters.
| @Bean(STEP_WEEKLY) | ||
| public Step weeklyRankingStep( | ||
| JpaPagingItemReader<ProductMetrics> productMetricsReader, | ||
| RankingMaterializedViewWriter weeklyRankingWriter | ||
| ) { | ||
| return new StepBuilder(STEP_WEEKLY, jobRepository) | ||
| .<ProductMetrics, ProductRankAggregate>chunk(CHUNK_SIZE, transactionManager) | ||
| .reader(productMetricsReader) | ||
| .processor(itemProcessor) | ||
| .writer(weeklyRankingWriter) | ||
| .listener(stepMonitorListener) | ||
| .build(); | ||
| } | ||
|
|
||
| @Bean(STEP_MONTHLY) | ||
| public Step monthlyRankingStep( | ||
| JpaPagingItemReader<ProductMetrics> productMetricsReader, | ||
| RankingMaterializedViewWriter monthlyRankingWriter | ||
| ) { | ||
| return new StepBuilder(STEP_MONTHLY, jobRepository) | ||
| .<ProductMetrics, ProductRankAggregate>chunk(CHUNK_SIZE, transactionManager) | ||
| .reader(productMetricsReader) | ||
| .processor(itemProcessor) | ||
| .writer(monthlyRankingWriter) | ||
| .listener(stepMonitorListener) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 배치 잡에서 retry/skip/faultTolerant 패턴 검색
rg -nP --type=java -C3 '\.(faultTolerant|retry|skip|skipLimit)\s*\(' apps/commerce-batch/Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 70
🏁 Script executed:
# ProductRankAggregationJobConfig.java 전체 파일 내용 확인
cat apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankAggregationJobConfig.javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 4909
🏁 Script executed:
# productMetricsReader 정의 위치 찾기
rg -nP --type=java "productMetricsReader|@StepScope" apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 1421
재시도 및 스킵 설정 추가 검토 필요
프로젝트 내 다른 배치 잡에서도 재시도 전략을 사용하지 않는 것으로 확인되었으나, PR 목표에서 "Retry strategy for batch failures"를 검토 포인트로 지정했다면 이 스텝들에 .faultTolerant(), .retry(), .skip() 등의 설정 추가를 검토해주세요.
현재 코드(lines 54-80)에는 해당 설정이 없으며, 아이템 레벨 재시도가 필요한지 명확히 해야 합니다.
참고: productMetricsReader는 @StepScope로 인해 각 스텝이 실행될 때 새로운 인스턴스를 받으므로 상태 격리는 올바르게 동작합니다.
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankAggregationJobConfig.java
around lines 54 to 80, add fault-tolerance configuration to the Step builders:
call .faultTolerant() on each StepBuilder and configure .retry(...) with the
appropriate transient exception types and .retryLimit(n) for item-level retries,
and/or configure .skip(...) with .skipLimit(m) for non-recoverable item
failures; choose specific exceptions (e.g. transient DB/network exceptions)
rather than Exception.class, set sensible limits (commonly retryLimit=3,
skipLimit as needed), and ensure these calls are placed before .listener(...) so
the step executes with retry/skip behavior while leaving the @StepScope reader
as-is.
| @Bean | ||
| @StepScope | ||
| public JpaPagingItemReader<ProductMetrics> productMetricsReader() { | ||
| return new JpaPagingItemReaderBuilder<ProductMetrics>() | ||
| .name("productMetricsReader") | ||
| .entityManagerFactory(entityManagerFactory) | ||
| .queryString( | ||
| "SELECT p FROM ProductMetrics p ORDER BY (p.likeCount * 0.2d + p.salesCount * 0.8d) DESC" | ||
| ) | ||
| .pageSize(CHUNK_SIZE) | ||
| .maxItemCount(MAX_ITEM_COUNT) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and read ProductMetrics entity file
find . -name "ProductMetrics.java" -type f 2>/dev/null | head -5Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 262
🏁 Script executed:
#!/bin/bash
# Read ProductMetrics entity from commerce-batch module
cat -n ./apps/commerce-batch/src/main/java/com/loopers/batch/domain/metrics/ProductMetrics.javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 980
🏁 Script executed:
#!/bin/bash
# Check CHUNK_SIZE and MAX_ITEM_COUNT constants
head -120 ./apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankAggregationJobConfig.java | grep -E "CHUNK_SIZE|MAX_ITEM_COUNT"Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 454
ORDER BY 절에서 계산된 식에 인덱스가 없어 성능 저하 가능성 있습니다.
ProductMetrics 엔티티에 score 필드가 없으며, likeCount와 salesCount 컬럼에도 인덱스가 정의되어 있지 않습니다. ORDER BY 절의 (p.likeCount * 0.2d + p.salesCount * 0.8d) 계산은 인덱스 지원 없이 데이터베이스에서 수행되므로, 현재 MAX_ITEM_COUNT=100인 소규모 배치에서는 즉각적인 문제가 적지만, 데이터 증가 시 성능 저하를 초래할 수 있습니다.
다음 중 하나를 권장합니다:
ProductMetrics엔티티에 미리 계산된score필드를 추가하고 인덱스 적용- 데이터베이스 레벨에서 계산된 컬럼 또는 인덱스 생성
- 향후 데이터 규모 증가 시 성능 테스트 실시
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductRankAggregationJobConfig.java
around lines 100 to 112, the ORDER BY uses a runtime computed expression
(p.likeCount * 0.2d + p.salesCount * 0.8d) which prevents index use and will
hurt performance as data grows; change to use a persisted, indexed score column
instead: add a new score field to the ProductMetrics entity, create a DB
migration to populate score (and a trigger or scheduled task to keep it updated)
or define a computed/generated column at the DB level and add an index on it,
update any writes to maintain the score, and finally change the JPA query to
ORDER BY p.score DESC; ensure tests and any existing imports/mappings and the
batch population/migration are updated accordingly.
| public static RankingPeriod weekly(String targetDate) { | ||
| LocalDate target = parse(targetDate); | ||
| LocalDate weekStart = target.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)); | ||
| return new RankingPeriod(toYearMonthWeek(weekStart)); | ||
| } |
There was a problem hiding this comment.
주간 시작일과 주차 계산 간 불일치 가능성
weekly() 메서드는 월요일(DayOfWeek.MONDAY)을 주의 시작으로 사용하지만, toYearMonthWeek()는 WeekFields.of(Locale.KOREA)를 사용합니다. 한국 로케일은 일요일을 주의 첫째 날로 정의할 수 있어 주차 번호가 의도와 다르게 계산될 수 있습니다.
ISO-8601 표준(월요일 시작)을 일관되게 사용하려면 WeekFields.ISO를 고려해 주세요.
🔎 수정 제안
private static String toYearMonthWeek(LocalDate target) {
- WeekFields weekFields = WeekFields.of(Locale.KOREA);
+ WeekFields weekFields = WeekFields.ISO;
int weekBasedYear = target.get(weekFields.weekBasedYear());
int week = target.get(weekFields.weekOfWeekBasedYear());
return String.format("%04d-W%02d", weekBasedYear, week);
}Also applies to: 39-44
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingPeriodResolver.java
around lines 20-24 (and similarly lines 39-44), the weekly() method uses Monday
as week start but the week-number calculation uses WeekFields.of(Locale.KOREA),
causing inconsistent week numbers; update the week-number computation to use
WeekFields.ISO (or otherwise pass an explicit WeekFields.ISO) so ISO-8601
(Monday-first) week numbering is used consistently when converting dates to
year/month/week.
3 participants
📌 Summary
💬 Review Points
✅ Checklist
🧱 Spring Batch
🧩 Ranking API
📎 References
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
문서
✏️ Tip: You can customize this high-level summary in your review settings.