-
Notifications
You must be signed in to change notification settings - Fork 34
[volume-9] Product Ranking with Redis #215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3d2ddcd
7c2114f
85a0bfb
3061196
fb6d5c0
ed75d4e
9f4f62a
86dc621
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| package com.loopers.application.ranking; | ||
|
|
||
| import com.loopers.application.product.ProductLikeSummary; | ||
| import com.loopers.ranking.RankingEntry; | ||
|
|
||
| import java.time.LocalDate; | ||
| import java.util.List; | ||
|
|
||
| public record RankingQueryResponse( | ||
| LocalDate date, | ||
| List<RankingEntry> rankingEntries, | ||
| List<ProductLikeSummary> productLikeSummary | ||
| ) { | ||
| public static RankingQueryResponse of(LocalDate date, List<RankingEntry> rankingEntries, List<ProductLikeSummary> productLikeSummary) { | ||
| return new RankingQueryResponse(date, rankingEntries, productLikeSummary); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| package com.loopers.application.ranking; | ||
|
|
||
| import com.loopers.application.like.event.ProductLikeEvent; | ||
| import com.loopers.application.product.ProductLikeSummary; | ||
| import com.loopers.application.product.ProductQueryService; | ||
| import com.loopers.domain.product.ProductSortType; | ||
| import com.loopers.ranking.DailyRankingResponse; | ||
| import com.loopers.ranking.RankingEntry; | ||
| import com.loopers.ranking.RankingZSetRepository; | ||
| import com.loopers.support.error.CoreException; | ||
| import com.loopers.support.error.ErrorType; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| import java.time.LocalDate; | ||
| import java.time.ZoneId; | ||
| import java.time.ZoneOffset; | ||
| import java.time.format.DateTimeFormatter; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.OptionalDouble; | ||
|
|
||
| @Slf4j | ||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class RankingQueryService { | ||
| private final RankingZSetRepository rankingZSetRepository; | ||
| private final ProductQueryService productQueryService; | ||
|
|
||
| @Transactional(readOnly = true) | ||
| public RankingQueryResponse getDailyPopularProducts(String date, int size) { | ||
| LocalDate target = initLocalDate(date); | ||
|
|
||
| int limit = (size <= 0) ? 20 : Math.min(size, 100); | ||
|
|
||
| List<RankingEntry> rankingEntries = rankingZSetRepository.findTopDailyAllByLimit(target, limit); | ||
| List<ProductLikeSummary> productLikeSummaries = findProductSummaryFrom(rankingEntries); | ||
|
|
||
| return new RankingQueryResponse( | ||
| target, | ||
| rankingEntries, | ||
| productLikeSummaries | ||
| ); | ||
| } | ||
|
|
||
| @Transactional(readOnly = true) | ||
| public OptionalDouble getDailyRankingScore(Long productId) { | ||
| LocalDate now = LocalDate.now(ZoneId.systemDefault()); | ||
| return rankingZSetRepository.findDailyRanking(now, productId); | ||
| } | ||
|
|
||
| private boolean hasValidDate(String date) { | ||
| return date == null || date.isBlank(); | ||
| } | ||
|
|
||
| private LocalDate initLocalDate(String date) { | ||
| return (hasValidDate(date)) | ||
| ? LocalDate.now(ZoneId.systemDefault()) | ||
| : LocalDate.parse(date, DateTimeFormatter.BASIC_ISO_DATE); | ||
|
|
||
| } | ||
|
|
||
| private List<ProductLikeSummary> findProductSummaryFrom(List<RankingEntry> rankingEntries) { | ||
| List<ProductLikeSummary> result = new ArrayList<>(); | ||
|
|
||
| for(RankingEntry rankingEntry : rankingEntries) { | ||
| ProductLikeSummary summary; | ||
| try { | ||
| summary = productQueryService.getProductLikeSummary(rankingEntry.productId()); | ||
| } catch (CoreException e) { | ||
| if(e.getErrorType() == ErrorType.NOT_FOUND) { | ||
| log.error("Could not find product like summary for {}", rankingEntry.productId()); | ||
| } | ||
| summary = null; | ||
| } | ||
| result.add(summary); | ||
| } | ||
| return result; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| package com.loopers.interfaces.api.ranking; | ||
|
|
||
| import com.loopers.application.ranking.RankingQueryResponse; | ||
| import com.loopers.application.ranking.RankingQueryService; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.web.bind.annotation.GetMapping; | ||
| import org.springframework.web.bind.annotation.RequestMapping; | ||
| import org.springframework.web.bind.annotation.RequestParam; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
|
|
||
| @RequiredArgsConstructor | ||
| @RestController | ||
| @RequestMapping("/api/v1/ranking") | ||
| public class RankingV1Controller { | ||
|
|
||
| private final RankingQueryService rankingQueryService; | ||
|
|
||
| @GetMapping | ||
| public RankingQueryResponse getDailyRanking( | ||
| @RequestParam(required = false, name = "date") String date, | ||
| @RequestParam(defaultValue = "20", name = "size") int size | ||
| ) { | ||
| return rankingQueryService.getDailyPopularProducts(date, size); | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,77 @@ | ||||||||||||||||||||||||||||||||||
| package com.loopers.application.ranking; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import com.loopers.ranking.RankingKey; | ||||||||||||||||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import org.springframework.data.redis.connection.RedisZSetCommands; | ||||||||||||||||||||||||||||||||||
| import org.springframework.data.redis.core.RedisCallback; | ||||||||||||||||||||||||||||||||||
| import org.springframework.data.redis.core.StringRedisTemplate; | ||||||||||||||||||||||||||||||||||
| import org.springframework.scheduling.annotation.Scheduled; | ||||||||||||||||||||||||||||||||||
| import org.springframework.stereotype.Service; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import java.time.*; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| @Service | ||||||||||||||||||||||||||||||||||
| @RequiredArgsConstructor | ||||||||||||||||||||||||||||||||||
| public class RankingAggregationService { | ||||||||||||||||||||||||||||||||||
| private static final double LIKE_WEIGHT = 0.2d; | ||||||||||||||||||||||||||||||||||
| private static final Duration TTL = Duration.ofDays(2); | ||||||||||||||||||||||||||||||||||
| private static final double CARRY_OVER_WEIGHT = 0.1d; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| private final StringRedisTemplate redisTemplate; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| public void applyLike(long productId, Instant occurredAt) { | ||||||||||||||||||||||||||||||||||
| LocalDate day = occurredAt.atZone(ZoneId.systemDefault()).toLocalDate(); | ||||||||||||||||||||||||||||||||||
| String key = RankingKey.dailyAll(day); | ||||||||||||||||||||||||||||||||||
| String member = String.valueOf(productId); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Score = Weight * 1 | ||||||||||||||||||||||||||||||||||
| redisTemplate.opsForZSet().incrementScore(key, member, LIKE_WEIGHT); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| setTTLOnlyOnce(key); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * μΌλ³ λνΉμ λν μ μΌμ Carry Over μ€μΌμ€λ¬ | ||||||||||||||||||||||||||||||||||
| * - λ§€μΌ 23:30λΆ μνν©λλ€. | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| @Scheduled(cron = "0 30 23 * * *") | ||||||||||||||||||||||||||||||||||
| public void carryOverDailyRanking() { | ||||||||||||||||||||||||||||||||||
| LocalDate today = LocalDate.now(ZoneId.systemDefault()); | ||||||||||||||||||||||||||||||||||
| LocalDate tomorrow = today.plusDays(1); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| String sourceKey = RankingKey.dailyAll(today); | ||||||||||||||||||||||||||||||||||
| String targetKey = RankingKey.dailyAll(tomorrow); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| carryOver(sourceKey, targetKey, CARRY_OVER_WEIGHT); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| private void carryOver(String sourceKey, String targetKey, double weight) { | ||||||||||||||||||||||||||||||||||
| redisTemplate.execute((RedisCallback<Long>) connection -> { | ||||||||||||||||||||||||||||||||||
| byte[] target = redisTemplate.getStringSerializer().serialize(targetKey); | ||||||||||||||||||||||||||||||||||
| byte[] source = redisTemplate.getStringSerializer().serialize(sourceKey); | ||||||||||||||||||||||||||||||||||
| // ZUNIONSTORE dest 1 src WEIGHTS 0.1 AGGREGATE SUM | ||||||||||||||||||||||||||||||||||
| Long result = (Long) connection.execute( | ||||||||||||||||||||||||||||||||||
| "ZUNIONSTORE", | ||||||||||||||||||||||||||||||||||
| target, | ||||||||||||||||||||||||||||||||||
| "1".getBytes(), | ||||||||||||||||||||||||||||||||||
| source, | ||||||||||||||||||||||||||||||||||
| "WEIGHTS".getBytes(), | ||||||||||||||||||||||||||||||||||
| String.valueOf(weight).getBytes(), | ||||||||||||||||||||||||||||||||||
| "AGGREGATE".getBytes(), | ||||||||||||||||||||||||||||||||||
| "SUM".getBytes() | ||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||
| // μμ μ£Όμν κ² | ||||||||||||||||||||||||||||||||||
| // ZUNIONSTORE target ... β ν€ μ¬μμ± λλ―λ‘ μ΄μ TTL μλ©Έλ¨ | ||||||||||||||||||||||||||||||||||
| connection.expire(target, TTL.getSeconds()); | ||||||||||||||||||||||||||||||||||
| return result; | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| private void setTTLOnlyOnce(String key) { | ||||||||||||||||||||||||||||||||||
| Boolean exists = redisTemplate.hasKey(key); | ||||||||||||||||||||||||||||||||||
| if (Boolean.FALSE.equals(exists)) { | ||||||||||||||||||||||||||||||||||
| redisTemplate.expire(key, TTL); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+71
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TTLμ΄ μ€μ λμ§ μλ μ¬κ°ν λ²κ·Έ
π μ μλ μμ - private void setTTLOnlyOnce(String key) {
- Boolean exists = redisTemplate.hasKey(key);
- if (Boolean.FALSE.equals(exists)) {
- redisTemplate.expire(key, TTL);
- }
- }
+ /**
+ * TTLμ΄ μ€μ λμ§ μμ κ²½μ°μλ§ TTLμ μ€μ ν©λλ€.
+ */
+ private void setTTLOnlyOnce(String key) {
+ Long currentTtl = redisTemplate.getExpire(key);
+ // TTLμ΄ -1μ΄λ©΄ λ§λ£ μκ°μ΄ μ€μ λμ§ μμ μν
+ if (currentTtl != null && currentTtl == -1) {
+ redisTemplate.expire(key, TTL);
+ }
+ }π Committable suggestion
Suggested change
π€ Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,28 +3,52 @@ | |
| import com.loopers.domain.ProductLikeMetricsModel; | ||
| import com.loopers.infrastructure.EventHandleRepository; | ||
| import com.loopers.infrastructure.ProductLikeMetricsRepository; | ||
| import com.loopers.testcontainers.KafkaTestContainersConfig; | ||
| import com.loopers.utils.KafkaCleanUp; | ||
| import org.apache.kafka.clients.consumer.ConsumerConfig; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.DisplayName; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.boot.autoconfigure.kafka.KafkaProperties; | ||
| import org.springframework.boot.test.context.SpringBootTest; | ||
| import org.springframework.context.annotation.Import; | ||
| import org.springframework.kafka.core.KafkaTemplate; | ||
| import org.testcontainers.junit.jupiter.Testcontainers; | ||
| import org.springframework.test.context.ActiveProfiles; | ||
|
|
||
| import java.util.Map; | ||
|
|
||
| import static org.assertj.core.api.AssertionsForClassTypes.assertThat; | ||
|
|
||
| @Testcontainers | ||
| @ActiveProfiles("local") | ||
| @Import(KafkaTestContainersConfig.class) | ||
| @SpringBootTest | ||
| class EventHandledServiceTest { | ||
|
|
||
| @Autowired | ||
| private KafkaCleanUp kafkaCleanUp; | ||
|
|
||
| @Autowired | ||
| KafkaTemplate<Object,Object> kafkaTemplate; | ||
| @Autowired | ||
| ProductLikeMetricsRepository metricsRepo; | ||
| @Autowired | ||
| EventHandleRepository handledRepo; | ||
|
|
||
| @Autowired | ||
| private KafkaProperties kafkaProperties; | ||
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| // @BeforeEachλ μ΄λ―Έ SpringBootTest 컨ν μ€νΈ + KafkaListener 컨ν μ΄λκ° λ€ μ¬λΌκ° λ€μ μ€ν | ||
| // kafkaCleanUp.resetAllTestTopics(); | ||
| kafkaCleanUp.resetAllConsumerGroups(); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("λ©±λ±μ± ν μ€νΈ") | ||
| void duplicate_message_should_be_applied_once() throws Exception { | ||
|
|
||
| long productId = 1L; | ||
| ProductLikeMetricsModel metrics = metricsRepo.findById(productId) | ||
| .orElseGet(() -> metricsRepo.save(ProductLikeMetricsModel.of(productId))); | ||
|
|
@@ -42,6 +66,9 @@ void duplicate_message_should_be_applied_once() throws Exception { | |
| // TODO - μΉ΄νμΉ΄ λΈλ‘컀/컨μλ¨Έ ν μ€νΈνκ²½μμ μ€μ νμ | ||
| Thread.sleep(1500); | ||
|
|
||
| Map<String, Object> consumerProps = kafkaProperties.buildConsumerProperties(); | ||
| consumerProps.put(ConsumerConfig.GROUP_ID_CONFIG, "${spring.kafka.consumer.group-id}"); | ||
|
|
||
|
Comment on lines
+69
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. consumer properties μ€μ μ΄ μ¬λ°λ₯΄μ§ μμ΅λλ€. Line 70μμ μ€μ group-id κ°μ μ¬μ©νλ €λ©΄ λ€μκ³Ό κ°μ΄ μμ νμΈμ: π μ μνλ μμ - Map<String, Object> consumerProps = kafkaProperties.buildConsumerProperties();
- consumerProps.put(ConsumerConfig.GROUP_ID_CONFIG, "${spring.kafka.consumer.group-id}");
+ // ν
μ€νΈμμ μ¬μ©ν μ€μ consumer group id
+ String actualGroupId = kafkaProperties.getConsumer().getGroupId();λ§μ½ μ΄ μ½λκ° μ€μ λ‘ νμνμ§ μλ€λ©΄ μ κ±°νλ κ²μ κΆμ₯ν©λλ€.
π€ Prompt for AI Agents |
||
|
|
||
| // then | ||
| // like μΉ΄μ΄νΈλ 1λ²λ§ μ¦κ° | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
N+1 쿼리 ν¨ν΄ λ° null μ²λ¦¬ κ°μ νμ
N+1 쿼리: κ° λνΉ νλͺ©λ§λ€ κ°λ³ DB μ‘°νκ° λ°μν©λλ€. productId λͺ©λ‘μΌλ‘ μΌκ΄ μ‘°ννλ κ²μ΄ ν¨μ¨μ μ λλ€.
Null μ²λ¦¬: μ°Ύμ§ λͺ»ν μνμ λν΄
nullμ 리μ€νΈμ μΆκ°νλ©΄ ν΄λΌμ΄μΈνΈμμ NPEκ° λ°μν μ μμ΅λλ€. νν°λ§νκ±°λ λͺ μμ μΈ "μμ λ¨" νμλ₯Ό μ¬μ©νμΈμ.μμΈ μ²λ¦¬:
NOT_FOUNDκ° μλ λ€λ₯Έ μμΈλ 무μλκ³ μμ΅λλ€.π μ μλ μμ
private List<ProductLikeSummary> findProductSummaryFrom(List<RankingEntry> rankingEntries) { - List<ProductLikeSummary> result = new ArrayList<>(); - - for(RankingEntry rankingEntry : rankingEntries) { - ProductLikeSummary summary; - try { - summary = productQueryService.getProductLikeSummary(rankingEntry.productId()); - } catch (CoreException e) { - if(e.getErrorType() == ErrorType.NOT_FOUND) { - log.error("Could not find product like summary for {}", rankingEntry.productId()); - } - summary = null; - } - result.add(summary); + List<Long> productIds = rankingEntries.stream() + .map(RankingEntry::productId) + .toList(); + + // TODO: ProductQueryServiceμ μΌκ΄ μ‘°ν λ©μλ μΆκ° κ³ λ € + List<ProductLikeSummary> result = new ArrayList<>(); + for (RankingEntry rankingEntry : rankingEntries) { + try { + ProductLikeSummary summary = productQueryService.getProductLikeSummary(rankingEntry.productId()); + result.add(summary); + } catch (CoreException e) { + if (e.getErrorType() == ErrorType.NOT_FOUND) { + log.warn("Product not found for ranking entry: {}", rankingEntry.productId()); + // null λμ 건λλ°κΈ° + } else { + throw e; // NOT_FOUNDκ° μλ μμΈλ μ ν + } + } } return result; }π€ Prompt for AI Agents