Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.loopers.application.api.product;

import com.loopers.core.service.productlike.ProductLikeEventPublishService;
import lombok.RequiredArgsConstructor;
import org.springframework.scheduling.annotation.Scheduled;
import org.springframework.stereotype.Component;

@Component
@RequiredArgsConstructor
public class ProductLikeEventScheduler {

private final ProductLikeEventPublishService service;

@Scheduled(fixedDelay = 1000)
public void publish() {
service.publish();
}
Comment on lines +14 to +17
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

1초 간격의 스케줄링이 과도할 수 있습니다.

fixedDelay = 1000은 매우 짧은 간격으로, 프로덕션 환경에서 불필요한 부하를 초래할 수 있습니다. 이벤트 발행 빈도와 비즈니스 요구사항을 고려하여 적절한 간격(예: 5초 또는 10초)으로 조정하는 것을 권장합니다.

추가로, 분산 환경에서 여러 인스턴스가 동시에 실행될 경우 중복 발행이 발생할 수 있습니다. ShedLock과 같은 분산 락 메커니즘 도입을 고려하세요.

🔎 제안하는 수정사항
-    @Scheduled(fixedDelay = 1000)
+    @Scheduled(fixedDelay = 5000) // Consider 5-10 seconds based on business requirements
     public void publish() {
         service.publish();
     }

분산 락 적용 예시:

+    @SchedulerLock(name = "ProductLikeEventScheduler_publish", lockAtMostFor = "4s", lockAtLeastFor = "1s")
     @Scheduled(fixedDelay = 5000)
     public void publish() {
         service.publish();
     }
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/application/api/product/ProductLikeEventScheduler.java
around lines 14 to 17, the scheduled task runs every 1 second which is likely
too frequent for production and can cause unnecessary load and duplicate events
in distributed deployments; increase the fixedDelay to a safer interval (for
example 5000 or 10000 ms) according to business needs and integrate a
distributed lock (e.g., ShedLock or similar) around the publish() invocation so
only one instance executes the task at a time.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.loopers.application.api.product;

import com.loopers.core.service.product.ProductRankingCarryOverService;
import lombok.RequiredArgsConstructor;
import org.springframework.scheduling.annotation.Scheduled;
import org.springframework.stereotype.Component;

@Component
@RequiredArgsConstructor
public class ProductRankingCarryOverScheduler {

private final ProductRankingCarryOverService service;

@Scheduled(cron = "0 50 23 * * *")
public void carryOver() {
service.carryOver();
}
Comment on lines +14 to +17
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

타임존 설정 및 분산 락이 누락되었습니다.

현재 크론 표현식에 타임존이 지정되지 않아 서버의 기본 타임존을 사용합니다. 이는 다음 문제를 야기할 수 있습니다:

  1. 타임존 불일치: 여러 인스턴스가 다른 타임존에서 실행될 경우 예상과 다른 시간에 실행됩니다.
  2. 중복 실행: 분산 환경에서 모든 인스턴스가 동시에 carry-over를 실행하여 데이터 불일치가 발생할 수 있습니다.

23:50에 실행되는 일일 처리 작업이므로, 명확한 타임존 지정과 분산 락이 필수적입니다.

🔎 제안하는 수정사항
-    @Scheduled(cron = "0 50 23 * * *")
+    @SchedulerLock(name = "ProductRankingCarryOverScheduler_carryOver", 
+                   lockAtMostFor = "10m", lockAtLeastFor = "1m")
+    @Scheduled(cron = "0 50 23 * * *", zone = "Asia/Seoul")
     public void carryOver() {
         service.carryOver();
     }

Committable suggestion skipped: line range outside the PR's diff.

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,30 @@
import com.loopers.core.domain.product.Product;
import com.loopers.core.domain.product.ProductDetail;
import com.loopers.core.domain.product.ProductListView;
import com.loopers.core.domain.product.ProductRankingList;
import com.loopers.core.service.product.GetProductRankingService;
import com.loopers.core.service.product.ProductQueryService;
import com.loopers.core.service.product.query.GetProductDetailQuery;
import com.loopers.core.service.product.query.GetProductListQuery;
import com.loopers.core.service.product.query.GetProductQuery;
import com.loopers.core.service.product.query.GetProductRankingQuery;
import lombok.RequiredArgsConstructor;
import org.springframework.format.annotation.DateTimeFormat;
import org.springframework.web.bind.annotation.*;

import java.time.LocalDate;

import static com.loopers.application.api.product.ProductV1Dto.*;
import static com.loopers.application.api.product.ProductV1Dto.GetProductDetailResponse.GetProductRankingsResponse;
import static com.loopers.application.api.product.ProductV1Dto.GetProductDetailResponse.from;

@RestController
@RequiredArgsConstructor
@RequestMapping("/api/v1/products")
public class ProductV1Api implements ProductV1ApiSpec {

private final ProductQueryService queryService;
private final GetProductRankingService getProductRankingService;

@Override
@GetMapping("/{productId}")
Expand Down Expand Up @@ -50,6 +59,18 @@ public ApiResponse<GetProductDetailResponse> getProductDetail(
) {
ProductDetail productDetail = queryService.getProductDetail(new GetProductDetailQuery(productId));

return ApiResponse.success(GetProductDetailResponse.from(productDetail));
return ApiResponse.success(from(productDetail));
}

@Override
@GetMapping("/rankings")
public ApiResponse<GetProductRankingsResponse> getProductRankings(
@RequestParam @DateTimeFormat(pattern = "yyyy-MM-dd") LocalDate date,
@RequestParam(required = false, defaultValue = "0") int pageNo,
@RequestParam(required = false, defaultValue = "10") int pageSize
) {
ProductRankingList ranking = getProductRankingService.getRanking(new GetProductRankingQuery(date, pageNo, pageSize));

return ApiResponse.success(GetProductRankingsResponse.from(ranking));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,25 @@
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag;

import java.time.LocalDate;

import static com.loopers.application.api.product.ProductV1Dto.*;
import static com.loopers.application.api.product.ProductV1Dto.GetProductDetailResponse.GetProductRankingsResponse;

@Tag(name = "Product V1 API", description = "상품 API 입니다.")
public interface ProductV1ApiSpec {

@Operation(
summary = "상품 정보 조회",
description = "상품 정보를 조회합니다."
)
ApiResponse<ProductV1Dto.GetProductResponse> getProduct(String productId);
ApiResponse<GetProductResponse> getProduct(String productId);

@Operation(
summary = "상품 목록 조회",
description = "상품 목록을 조회합니다."
)
ApiResponse<ProductV1Dto.GetProductListResponse> getProductList(
ApiResponse<GetProductListResponse> getProductList(
String brandId,
String createdAtSort,
String priceSort,
Expand All @@ -30,5 +35,11 @@ ApiResponse<ProductV1Dto.GetProductListResponse> getProductList(
summary = "상품 상세 조회",
description = "상품 상세 정보를 조회합니다."
)
ApiResponse<ProductV1Dto.GetProductDetailResponse> getProductDetail(String productId);
ApiResponse<GetProductDetailResponse> getProductDetail(String productId);

@Operation(
summary = "상품 랭킹 조회",
description = "상품 랭킹을 조회합니다."
)
ApiResponse<GetProductRankingsResponse> getProductRankings(LocalDate date, int pageNo, int pageSize);
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package com.loopers.application.api.product;

import com.loopers.core.domain.brand.Brand;
import com.loopers.core.domain.product.Product;
import com.loopers.core.domain.product.ProductDetail;
import com.loopers.core.domain.product.ProductListItem;
import com.loopers.core.domain.product.ProductListView;
import com.loopers.core.domain.product.*;
import com.loopers.core.domain.product.ProductRankingList.ProductRankingItem;
import com.loopers.core.domain.product.vo.ProductRanking;

import java.math.BigDecimal;
import java.util.List;
import java.util.Optional;

public class ProductV1Dto {

Expand Down Expand Up @@ -77,7 +77,8 @@ public record GetProductDetailResponse(
String name,
BigDecimal price,
Long stock,
Long likeCount
Long likeCount,
GetProductDetailRanking ranking
) {

public static GetProductDetailResponse from(ProductDetail detail) {
Expand All @@ -87,10 +88,22 @@ public static GetProductDetailResponse from(ProductDetail detail) {
detail.getProduct().getName().value(),
detail.getProduct().getPrice().value(),
detail.getProduct().getStock().value(),
detail.getProduct().getLikeCount().value()
detail.getProduct().getLikeCount().value(),
Optional.ofNullable(detail.getRanking())
.map(GetProductDetailRanking::from)
.orElse(null)
);
}

public record GetProductDetailRanking(
Long ranking,
Double score
) {
public static GetProductDetailRanking from(ProductRanking ranking) {
return new GetProductDetailRanking(ranking.ranking(), ranking.score());
}
}

public record GetProductDetailBrand(
String id,
String name,
Expand All @@ -105,5 +118,48 @@ public static GetProductDetailBrand from(Brand brand) {
);
}
}

public record GetProductRankingsResponse(
List<ProductRankingResponse> products,
long totalElements,
int totalPages,
boolean hasNext,
boolean hasPrevious
) {

public static GetProductRankingsResponse from(ProductRankingList list) {
return new GetProductRankingsResponse(
list.products().stream()
.map(ProductRankingResponse::from)
.toList(),
list.totalElements(),
list.totalPages(),
list.hasNext(),
list.hasPrevious()
);
}

public record ProductRankingResponse(
String id,
Long ranking,
String brandName,
String name,
BigDecimal price,
Long likeCount,
Double score
) {
public static ProductRankingResponse from(ProductRankingItem item) {
return new ProductRankingResponse(
item.id().value(),
item.ranking(),
item.brandName().value(),
item.name().value(),
item.price().value(),
item.likeCount().value(),
item.score()
);
}
}
}
}
}
10 changes: 10 additions & 0 deletions apps/commerce-api/src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ spring:
product-detail-viewed: product-detail-viewed.v1
product-out-of-stock: product-out-of-stock.v1
payment-completed: payment-completed.v1
product-like: product-like.v1

product:
ranking:
score:
weight:
like: 0.2
view: 0.1
pay: 0.7
carryOver: 0.1

springdoc:
use-fqn: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.loopers.core.domain.brand.repository.BrandRepository;
import com.loopers.core.domain.product.Product;
import com.loopers.core.domain.product.ProductFixture;
import com.loopers.core.domain.product.repository.ProductRankingCacheRepository;
import com.loopers.core.domain.product.repository.ProductRepository;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
Expand All @@ -19,6 +20,9 @@
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;

import java.time.LocalDate;

import static com.loopers.application.api.product.ProductV1Dto.GetProductDetailResponse.GetProductRankingsResponse;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the definition of GetProductRankingsResponse
rg -A 10 "record GetProductRankingsResponse" --type java

Repository: Loopers-dev-lab/loopers-spring-java-template

Length of output: 1477


🏁 Script executed:

#!/bin/bash
# Also search for GetProductDetailResponse to see the full structure
rg -B 5 -A 20 "record GetProductDetailResponse" --type java

Repository: Loopers-dev-lab/loopers-spring-java-template

Length of output: 3333


🏁 Script executed:

#!/bin/bash
# Check all usages of GetProductRankingsResponse across the codebase
rg "GetProductRankingsResponse" --type java

Repository: Loopers-dev-lab/loopers-spring-java-template

Length of output: 2634


🏁 Script executed:

#!/bin/bash
# Get the full structure around GetProductDetailResponse and GetProductRankingsResponse
sed -n '90,180p' apps/commerce-api/src/main/java/com/loopers/application/api/product/ProductV1Dto.java

Repository: Loopers-dev-lab/loopers-spring-java-template

Length of output: 2799


🏁 Script executed:

#!/bin/bash
# Count the record definitions in ProductV1Dto to understand the structure
rg "record Get" apps/commerce-api/src/main/java/com/loopers/application/api/product/ProductV1Dto.java --type java

Repository: Loopers-dev-lab/loopers-spring-java-template

Length of output: 403


GetProductRankingsResponseProductV1Dto의 최상위 레벨로 이동해야 합니다.

현재 GetProductRankingsResponseGetProductDetailResponse 내부에 중첩되어 있으나, 이는 설계 오류입니다. GetProductRankingsResponse는 상품 랭킹 목록 조회 API(getProductRankings)의 응답으로 사용되며, 상품 상세 조회 API(getProductDetail)의 응답인 GetProductDetailResponse와 전혀 다른 목적을 가집니다. 두 응답 구조가 독립적이므로 GetProductRankingsResponseProductRankingResponseProductV1Dto의 최상위 레벨로 옮기기를 권장합니다.

🤖 Prompt for AI Agents
In
apps/commerce-api/src/test/java/com/loopers/application/api/product/ProductV1ApiIntegrationTest.java
around line 25, the test imports a nested class GetProductRankingsResponse from
ProductV1Dto.GetProductDetailResponse which is incorrect; move
GetProductRankingsResponse (and ProductRankingResponse) out of the
GetProductDetailResponse inner class and promote them to top-level static inner
classes (or separate classes) directly under ProductV1Dto, update the class
declarations and constructors accordingly, then update all imports/usages
(including this test) to import
com.loopers.application.api.product.ProductV1Dto.GetProductRankingsResponse (and
ProductRankingResponse) from the new top-level location and run compilation to
fix any reference errors.

import static com.loopers.application.api.product.ProductV1Dto.GetProductListResponse;
import static com.loopers.application.api.product.ProductV1Dto.GetProductResponse;
import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -31,6 +35,9 @@ class ProductV1ApiIntegrationTest extends ApiIntegrationTest {
@Autowired
private ProductRepository productRepository;

@Autowired
private ProductRankingCacheRepository productRankingCacheRepository;

@Nested
@DisplayName("상품 목록 조회")
class 상품_목록_조회 {
Expand Down Expand Up @@ -136,4 +143,74 @@ void status404() {
}
}
}

@Nested
@DisplayName("상품 랭킹 조회")
class 상품_랭킹_조회 {

@Nested
@DisplayName("정상 요청인 경우")
class 정상_요청인_경우 {

@BeforeEach
void setUp() {
Brand brand = brandRepository.save(
BrandFixture.create()
);

Product product1 = productRepository.save(
ProductFixture.createWith(brand.getId())
);
productRankingCacheRepository.increaseDaily(product1.getId(), LocalDate.now(), 0.7);

Product product2 = productRepository.save(
ProductFixture.createWith(brand.getId())
);
productRankingCacheRepository.increaseDaily(product2.getId(), LocalDate.now(), 0.9);
}

@Test
@DisplayName("Status 200")
void status200() {
// When
LocalDate today = LocalDate.now();
String endPoint = "/api/v1/products/rankings?date=" + today + "&pageNo=0&pageSize=10";

ParameterizedTypeReference<ApiResponse<GetProductRankingsResponse>> responseType =
new ParameterizedTypeReference<>() {
};

ResponseEntity<ApiResponse<GetProductRankingsResponse>> response =
testRestTemplate.exchange(endPoint, HttpMethod.GET, HttpEntity.EMPTY, responseType);

// Then
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK);
assertThat(response.getBody()).isNotNull();
assertThat(response.getBody().data()).isNotNull();
assertThat(response.getBody().data().products()).isNotEmpty();
}

@Test
@DisplayName("페이지네이션 정보가 포함되어 있다")
void pagination_information_included() {
// When
LocalDate today = LocalDate.now();
String endPoint = "/api/v1/products/rankings?date=" + today + "&pageNo=0&pageSize=10";

ParameterizedTypeReference<ApiResponse<GetProductRankingsResponse>> responseType =
new ParameterizedTypeReference<>() {
};

ResponseEntity<ApiResponse<GetProductRankingsResponse>> response =
testRestTemplate.exchange(endPoint, HttpMethod.GET, HttpEntity.EMPTY, responseType);

// Then
GetProductRankingsResponse rankingResponse = response.getBody().data();
assertThat(rankingResponse.totalElements()).isGreaterThanOrEqualTo(0);
assertThat(rankingResponse.totalPages()).isGreaterThanOrEqualTo(0);
assertThat(rankingResponse.hasNext()).isFalse();
assertThat(rankingResponse.hasPrevious()).isFalse();
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package com.loopers.applications.streamer.consumer.product;

import com.loopers.JacksonUtil;
import com.loopers.applications.streamer.consumer.product.dto.IncreaseProductLikeRankingScoreEvent;
import com.loopers.core.infra.event.kafka.config.KafkaConfig;
import com.loopers.core.service.productlike.IncreaseProductLikeRankingScoreService;
import lombok.RequiredArgsConstructor;
import org.apache.kafka.clients.consumer.ConsumerRecord;
import org.springframework.kafka.annotation.KafkaListener;
import org.springframework.kafka.support.Acknowledgment;
import org.springframework.stereotype.Component;

import java.util.List;

@Component
@RequiredArgsConstructor
public class IncreaseProductLikeRankingScoreKafkaConsumer {

private final IncreaseProductLikeRankingScoreService service;

@KafkaListener(
topics = {"${spring.kafka.topic.product-like}"},
containerFactory = KafkaConfig.BATCH_LISTENER,
groupId = "increase-product-like-ranking-score"
)
public void listen(
List<ConsumerRecord<Object, String>> records,
Acknowledgment acknowledgment
) {
records.stream()
.map(event -> JacksonUtil.convertToObject(event.value(), IncreaseProductLikeRankingScoreEvent.class))
.map(IncreaseProductLikeRankingScoreEvent::toCommand)
.forEach(service::increase);
acknowledgment.acknowledge();
}
Comment on lines +26 to +35
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

에러 처리 및 중복 방지 로직이 누락되었습니다.

현재 구현에는 두 가지 중요한 문제가 있습니다:

  1. 에러 처리 부재: 배치 처리 중 하나의 이벤트라도 실패하면 전체 배치가 손실됩니다. 학습된 패턴에 따르면, service.increase() 메서드에 @InboxEvent 어노테이션을 적용하여 EventInboxAspect를 통한 중앙화된 에러 처리를 사용해야 합니다.

  2. 멱등성 보장 부재: eventId가 중복 이벤트를 방지하는 데 사용되지 않습니다. 재처리 시 동일한 이벤트가 여러 번 처리될 수 있습니다.

Based on learnings, centralized error handling via EventInboxAspect should be adopted.

🔎 제안하는 수정사항

서비스 메서드에 @InboxEvent 어노테이션 추가:

// In IncreaseProductLikeRankingScoreService.java
@InboxEvent
public void increase(IncreaseProductLikeRankingScoreCommand command) {
    // existing logic
}

또는 컨슈머에서 각 이벤트별 에러 처리:

     public void listen(
             List<ConsumerRecord<Object, String>> records,
             Acknowledgment acknowledgment
     ) {
-        records.stream()
-                .map(event -> JacksonUtil.convertToObject(event.value(), IncreaseProductLikeRankingScoreEvent.class))
-                .map(IncreaseProductLikeRankingScoreEvent::toCommand)
-                .forEach(service::increase);
+        records.forEach(record -> {
+            try {
+                IncreaseProductLikeRankingScoreEvent event = 
+                    JacksonUtil.convertToObject(record.value(), IncreaseProductLikeRankingScoreEvent.class);
+                service.increase(event.toCommand());
+            } catch (Exception e) {
+                // Log error but continue processing other events
+                log.error("Failed to process event: {}", record.value(), e);
+            }
+        });
         acknowledgment.acknowledge();
     }
🤖 Prompt for AI Agents
In
apps/commerce-streamer/src/main/java/com/loopers/applications/streamer/consumer/product/IncreaseProductLikeRankingScoreKafkaConsumer.java
around lines 26-35, the consumer lacks per-event error handling and idempotency:
update the service method (IncreaseProductLikeRankingScoreService.increase) to
be annotated with @InboxEvent so EventInboxAspect provides centralized error
handling and duplicate protection, ensure the command object carries the eventId
when mapping from the event so the inbox can deduplicate, and adjust the
consumer to process records individually (apply the mapping to commands
one-by-one and invoke service.increase for each) while only acknowledging the
Kafka batch after all records processed successfully (or consider acknowledging
per-record after successful handling) so failed events don’t cause silent loss.

}
Loading