diff --git a/app/src/main/java/com/example/app/config/AppConfig.java b/app/src/main/java/com/example/app/config/AppConfig.java index 90300d9..263e942 100644 --- a/app/src/main/java/com/example/app/config/AppConfig.java +++ b/app/src/main/java/com/example/app/config/AppConfig.java @@ -7,12 +7,14 @@ import com.example.common.db.UserRepository; import com.example.common.db.UserStore; import com.example.orderapi.client.DeliveryClient; -import com.example.orderapi.service.NoDiscountPolicy; -import com.example.orderapi.service.OrderService; -import com.example.orderapi.service.PriceService; import com.example.loyalty.service.BatchRunner; import com.example.loyalty.service.LoyaltyCalculator; +import com.example.loyalty.service.LoyaltyConfig; +import com.example.orderapi.client.TimeoutDeliveryClient; +import com.example.orderapi.service.OrderService; +import com.example.orderapi.service.PriceService; import java.math.BigDecimal; +import java.time.Duration; import javax.sql.DataSource; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -37,12 +39,12 @@ public OrderStore orderStore(DataSource dataSource) { @Bean public DeliveryClient deliveryClient() { - return new StubDeliveryClient(); + return new TimeoutDeliveryClient(new StubDeliveryClient(), Duration.ofMillis(500)); } @Bean - public PriceService priceService(DeliveryClient deliveryClient) { - return new PriceService(deliveryClient, new NoDiscountPolicy(), "USD"); + public PriceService priceService(DeliveryClient deliveryClient, DiscountStore discountStore) { + return new PriceService(deliveryClient, discountStore, "USD"); } @Bean @@ -52,11 +54,24 @@ public OrderService orderService(OrderStore orderStore, UserStore userStore, Dis @Bean public LoyaltyCalculator loyaltyCalculator() { - return new LoyaltyCalculator(new BigDecimal("0.05")); + return new LoyaltyCalculator(loyaltyConfig()); + } + + @Bean + public LoyaltyConfig loyaltyConfig() { + return new LoyaltyConfig( + new BigDecimal("0.05"), + new BigDecimal("0.02"), + new BigDecimal("100.00"), + new BigDecimal("50.00"), + new BigDecimal("1.00"), + new BigDecimal("0.01"), + 30 + ); } @Bean - public BatchRunner batchRunner(LoyaltyCalculator calculator, DiscountStore discountStore) { - return new BatchRunner(calculator, discountStore); + public BatchRunner batchRunner(LoyaltyCalculator calculator, DiscountStore discountStore, LoyaltyConfig config) { + return new BatchRunner(calculator, config, discountStore); } } diff --git a/libs/common/src/main/java/com/example/common/ErrorCodes.java b/libs/common/src/main/java/com/example/common/ErrorCodes.java index e72e76d..caa5512 100644 --- a/libs/common/src/main/java/com/example/common/ErrorCodes.java +++ b/libs/common/src/main/java/com/example/common/ErrorCodes.java @@ -3,6 +3,7 @@ public final class ErrorCodes { public static final String DELIVERY_UNAVAILABLE = "DELIVERY_UNAVAILABLE"; public static final String INVALID_REQUEST = "INVALID_REQUEST"; + public static final String ORDER_TOO_SMALL = "ORDER_TOO_SMALL"; public static final String INTERNAL_ERROR = "INTERNAL_ERROR"; public static final String LOYALTY_INVALID_INPUT = "LOYALTY_INVALID_INPUT"; diff --git a/libs/common/src/main/java/com/example/common/model/Item.java b/libs/common/src/main/java/com/example/common/model/Item.java index b6c5720..5a4fc44 100644 --- a/libs/common/src/main/java/com/example/common/model/Item.java +++ b/libs/common/src/main/java/com/example/common/model/Item.java @@ -15,6 +15,9 @@ public Item(String sku, int quantity, BigDecimal unitPrice) { } this.quantity = quantity; this.unitPrice = Objects.requireNonNull(unitPrice, "unitPrice"); + if (unitPrice.compareTo(BigDecimal.ZERO) < 0) { + throw new IllegalArgumentException("unitPrice must be non-negative"); + } } public String getSku() { @@ -29,4 +32,3 @@ public BigDecimal getUnitPrice() { return unitPrice; } } - diff --git a/libs/common/src/test/java/com/example/common/model/ModelSmokeTest.java b/libs/common/src/test/java/com/example/common/model/ModelSmokeTest.java index b96d567..e34d0d4 100644 --- a/libs/common/src/test/java/com/example/common/model/ModelSmokeTest.java +++ b/libs/common/src/test/java/com/example/common/model/ModelSmokeTest.java @@ -18,6 +18,7 @@ void itemValidatesQuantityAndNulls() { assertThrows(NullPointerException.class, () -> new Item(null, 1, new BigDecimal("1.00"))); assertThrows(NullPointerException.class, () -> new Item("sku-1", 1, null)); assertThrows(IllegalArgumentException.class, () -> new Item("sku-1", 0, new BigDecimal("1.00"))); + assertThrows(IllegalArgumentException.class, () -> new Item("sku-1", 1, new BigDecimal("-1.00"))); Item item = new Item("sku-1", 2, new BigDecimal("10.00")); assertEquals("sku-1", item.getSku()); @@ -25,4 +26,3 @@ void itemValidatesQuantityAndNulls() { assertEquals(new BigDecimal("10.00"), item.getUnitPrice()); } } - diff --git a/longread.md b/longread.md index 0abad76..f7712a9 100644 --- a/longread.md +++ b/longread.md @@ -1,261 +1,208 @@ +# Code Review в промышленной разработке (Java) -# Инженерное код-ревью в Java: полный теоретический лонгрид +## Зачем вообще нужен код‑ревью -## Зачем вообще нужно код-ревью +Код‑ревью — это **инженерный механизм управления качеством**, а не формальность и не поиск виноватых. Его задачи: -Код-ревью — это не проверка стиля и не поиск «красивого» кода. -Это инженерная практика управления рисками и улучшения code health кодовой базы со временем. +* снизить вероятность дефектов; +* выровнять стиль и уровень кода в команде; +* передать знания и контекст; +* защитить продукт от технического долга; +* сделать код поддерживаемым **другими людьми**. -Код-ревью отвечает не на вопрос «нравится ли мне этот код», а на вопросы: -- будет ли он корректно работать во всех разумных сценариях; -- можно ли его безопасно эксплуатировать в продакшене; -- сможет ли другой разработчик понять и поддерживать его через полгода; -- не создаёт ли он скрытых архитектурных, эксплуатационных или продуктовых рисков. - -Эта трактовка совпадает с общемировыми практиками Google, Microsoft, Mozilla. +Важно: ревью — это не про «красиво» и не про «я бы написал иначе». Это про **понимание, корректность и последствия**. --- -## Базовый принцип +## Модель мышления ревьюера + +Хороший ревьюер читает код не как автор, а как: -Code smell — это не ошибка. -Это сигнал возможного риска. +1. **Читатель** — смогу ли я понять код через полгода? +2. **Инженер сопровождения** — смогу ли я это безопасно менять? +3. **Система** — что будет при ошибках, нагрузке, отказах? +4. **Команда** — соответствует ли код нашим договорённостям? -Задача ревьюера — не «устранить запах», а: -1. распознать риск; -2. оценить его серьёзность; -3. решить, блокирует ли он merge или может быть принят осознанно. +Ключевой сдвиг: ревью — это **симуляция будущего использования кода**. --- -## Алгоритм код-ревью (фиксированный) +## Пирамида код‑ревью -Ревью всегда проводится в одном и том же порядке: +Проверка должна идти сверху вниз: -1. Прочитать описание PR: что меняется и зачем. -2. Прочитать тесты: что считается правильным поведением. -3. Проверить correctness. -4. Проверить контракты и API. -5. Проверить архитектуру и границы. -6. Проверить состояние и жизненный цикл. -7. Оценить тестируемость и намерение. -8. Проверить эксплуатационную готовность. -9. Проверить безопасность и зависимости. -10. Только в конце — читаемость и стиль. +1. **Смысл и требования** +2. **Архитектура и границы** +3. **Контракты и инварианты** +4. **Читаемость и структура** +5. **Тесты** +6. **Мелкие дефекты и стиль** -Любое ревью, начинающееся со стиля, считается методологически неверным. +Если верхние уровни не в порядке — нижние не имеют значения. --- -## Correctness: корректность выполнения +## 1. Смысл и соответствие задаче -Correctness отвечает на вопрос: «А этот код всегда работает правильно?» +Вопросы ревьюера: -### Что проверяется: -- null и Optional; -- исключения и их обработка; -- граничные случаи; -- многопоточность; -- числовые и временные границы. +* Решает ли код именно ту задачу, которая описана? +* Нет ли скрытого изменения требований? +* Понятно ли из кода **что он делает**, без чтения ТЗ? -### Типовые smells: -- проглатывание исключений; -- возврат null как часть неявного протокола; -- catch(Exception); -- boolean-параметры. +Антипаттерны: -### Ключевой вопрос ревьюера: -В каком сценарии этот код сломается первым? +* «Код работает, но непонятно зачем» +* Магические значения без доменной интерпретации +* Логика, не отражённая в именах --- -## Contracts & API: явные обещания кода +## 2. Архитектура и границы ответственности -Контракты — это то, что код обещает вызывающему. +### Принцип -### Что проверяется: -- входные параметры; -- возвращаемые значения; -- гарантии; -- ответственность сторон; -- ошибки. +Каждый модуль должен иметь **одну понятную причину для изменения**. -### Типовые smells: -- неявные предположения; -- слишком широкие исключения; -- возврат mutable-объектов. +### Проверяем: -### Ключевой вопрос: -Поймёт ли другой разработчик, как правильно использовать этот код? +* Разделены ли слои (API / domain / infrastructure)? +* Нет ли протекания деталей реализации? +* Явные ли зависимости? +* Можно ли заменить компонент без переписывания всего? ---- +### Частые проблемы -## Architecture & Boundaries: где должна жить логика +* God‑классы +* Сервисы, которые делают «всё сразу» +* Доменные решения внутри контроллеров -Архитектурные ошибки редко ломают код сразу, но почти всегда ломают систему со временем. +--- -### Что проверяется: -- разделение слоёв; -- направление зависимостей; -- размещение бизнес-логики. +## 3. Контракты и инварианты -### Типовые smells: -- god service; -- бизнес-логика в контроллерах; -- прямые зависимости Controller → Repository. +Код — это система контрактов. -### Ключевой вопрос: -Можно ли изменить один слой, не переписывая остальные? +Ревьюер обязан видеть: ---- +* Что считается валидным входом? +* Что гарантируется на выходе? +* Где проверяются инварианты? +* Где возможны ошибки? -## State & Lifecycle: владение состоянием +В Java это выражается через: -Состояние — один из главных источников нестабильности. +* типы; +* исключения; +* null‑контракты; +* Optional; +* defensive checks. -### Что проверяется: -- mutable state; -- shared state; -- потокобезопасность; -- инициализация и освобождение ресурсов. +Плохой признак — необходимость «догадываться». -### Типовые smells: -- shared mutable state; -- lazy init без синхронизации; -- неявный жизненный цикл. +--- -### Ключевой вопрос: -Что произойдёт при параллельном использовании? +## 4. Читаемость и структура ---- +### Код читают чаще, чем пишут -## Testability & Intent: тесты как документация +Правила: -Тесты — это форма спецификации. +* Имена важнее комментариев +* Функция делает одну вещь +* Уровни абстракции не смешиваются -Хорошие тесты: -- объясняют намерение; -- фиксируют контракты; -- показывают границы допустимого поведения. +### Code smells -Плохие тесты: -- проверяют реализацию, а не поведение; -- требуют сложной настройки; -- не читаются без прод-кода. +* длинные методы; +* вложенные условия; +* флаги поведения; +* неявные зависимости; +* временные переменные без смысла. -Ключевой вопрос: -Можно ли понять код, читая только тесты? +Если код нельзя объяснить голосом — он плох. --- -## Operability: эксплуатационная готовность +## 5. Тесты как часть ревью + +Тесты — это **исполняемая спецификация**. -Код может быть корректным, но неэксплуатируемым. +Ревьюер смотрит: -### Что проверяется: -- логирование; -- сообщения об ошибках; -- метрики и наблюдаемость; -- таймауты и ретраи; -- rollback и обратная совместимость. +* Что именно проверяется? +* Покрыты ли граничные случаи? +* Читаются ли тесты как сценарии? -### Типовые smells: -- отсутствие логов при ошибках; -- отсутствие таймаутов; -- шумные логи; -- отсутствие плана отката. +Плохие тесты: -### Ключевой вопрос: -Что мы увидим в продакшене, если это сломается? +* повторяют код; +* проверяют реализацию, а не поведение; +* ломаются при рефакторинге. ---- +Хороший признак — по тестам можно понять систему. -## Security & Dependencies: безопасность +--- -Безопасность — обязательная часть ревью в зрелых командах. +## 6. Ошибки, крайние случаи, отказоустойчивость -### Что проверяется: -- авторизация и права доступа; -- работа с PII; -- обработка входных данных; -- зависимости и лицензии. +Вопросы: -### Типовые smells: -- проверки прав в одном месте; -- логирование чувствительных данных; -- непроверенные зависимости. +* Что будет при null? +* При пустом списке? +* При исключении? +* При повторном вызове? -### Ключевой вопрос: -Можно ли использовать это во вред? +Отсутствие обработки ошибок — это архитектурное решение, и его нужно осознавать. --- -## Maintainability & Readability: код для других людей +## 7. Производительность и ресурсы -Код пишется для следующего разработчика, а не для автора. +Ревью не про микрооптимизации, но про: -### Что проверяется: -- читаемость намерения; -- имена сущностей; -- когнитивная сложность; -- структура кода. +* асимптотику; +* лишние аллокации; +* IO в циклах; +* блокировки; +* синхронизацию. -Комментарии не должны компенсировать плохую структуру. - -Ключевой вопрос: -Поймёт ли это человек без контекста? +Важно понимать **где код будет выполняться и под какой нагрузкой**. --- -## Как писать ревью-комментарии +## Формат хорошего комментария на ревью -Каждый комментарий должен содержать: -1. Что вы видите. -2. Какой риск вы видите. -3. Почему это важно. +Плохой комментарий: -Комментарии маркируются как: -- BLOCKER; -- NON-BLOCKING; -- QUESTION; -- SUGGESTION. +> Сделай нормально -Запрещены: -- вкусовые замечания; -- «я бы сделал по-другому»; -- архитектурные войны без аргумента риска. +Хороший комментарий: ---- +> Здесь метод делает две логически разные операции — валидацию и сохранение. Это усложняет тестирование и повторное использование. -## Уровни ревьюеров +Ревью — это обучение, а не приговор. -Junior: -- ищет баги и очевидные ошибки. +--- -Middle: -- проверяет контракты, тесты, границы. +## Чеклист ревьюера -Senior: -- оценивает архитектуру, эксплуатацию и последствия. +* Я понимаю, что делает код? +* Я понимаю, почему он так устроен? +* Я смогу его изменить? +* Я знаю, где он сломается? +* Я доверяю этому коду? ---- +Если на любой вопрос ответ «нет» — это повод для комментария. -## Definition of Done для PR +--- -PR готов к merge, если: -- цель изменений описана; -- контракты понятны; -- ошибки обрабатываются явно; -- тесты отражают намерение; -- нет скрытого состояния; -- есть базовая observability; -- учтены security и зависимости; -- код читаем без комментариев; -- все выявленные риски проговорены. +## Главное ---- +Код‑ревью — это: -## Итог +* инженерная практика; +* социальный процесс; +* инструмент качества. -Хорошее код-ревью — это систематическая работа по снижению рисков. -Это инженерная практика, а не вкусовая дискуссия. +Цель — не идеальный код, а **поддерживаемая система, понятная команде**. diff --git a/services/loyalty-batch/src/main/java/com/example/loyalty/service/BatchRunner.java b/services/loyalty-batch/src/main/java/com/example/loyalty/service/BatchRunner.java index a7a53db..57c7229 100644 --- a/services/loyalty-batch/src/main/java/com/example/loyalty/service/BatchRunner.java +++ b/services/loyalty-batch/src/main/java/com/example/loyalty/service/BatchRunner.java @@ -4,6 +4,8 @@ import com.example.common.db.DiscountStore; import com.example.loyalty.model.OrderRecord; import java.math.BigDecimal; +import java.time.Clock; +import java.time.LocalDate; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -12,27 +14,73 @@ public class BatchRunner { private final LoyaltyCalculator calculator; + private final LoyaltyConfig config; + private final Clock clock; private final DiscountStore discountStore; - public BatchRunner(LoyaltyCalculator calculator, DiscountStore discountStore) { + public BatchRunner(LoyaltyCalculator calculator, LoyaltyConfig config, DiscountStore discountStore) { + this(calculator, config, discountStore, Clock.systemDefaultZone()); + } + + public BatchRunner(LoyaltyCalculator calculator, LoyaltyConfig config, DiscountStore discountStore, Clock clock) { this.calculator = Objects.requireNonNull(calculator, "calculator"); + this.config = Objects.requireNonNull(config, "config"); this.discountStore = Objects.requireNonNull(discountStore, "discountStore"); + this.clock = Objects.requireNonNull(clock, "clock"); } public BatchResult run(List orders) { List errors = new ArrayList<>(); - List safeOrders = new ArrayList<>(); + List valid = new ArrayList<>(); + LocalDate cutoff = LocalDate.now(clock).minusDays(config.windowDays() - 1L); for (OrderRecord order : orders) { - if (order.getAmount().compareTo(BigDecimal.ZERO) < 0) { - errors.add(ErrorCodes.LOYALTY_INVALID_INPUT + ":negative-amount:" + order.getUserId()); - } else { - safeOrders.add(order); + if (order == null) { + errors.add(ErrorCodes.LOYALTY_INVALID_INPUT + ":null-order"); + continue; + } + if (order.getUserId() == null || order.getUserId().isBlank()) { + errors.add(ErrorCodes.LOYALTY_INVALID_INPUT + ":missing-user"); + continue; + } + if (order.getAmount() == null || order.getAmount().compareTo(BigDecimal.ZERO) <= 0) { + errors.add(ErrorCodes.LOYALTY_INVALID_INPUT + ":invalid-amount:" + order.getUserId()); + continue; + } + if (order.getAmount().compareTo(config.minOrderAmount()) < 0) { + errors.add(ErrorCodes.LOYALTY_INVALID_INPUT + ":too-small:" + order.getUserId()); + continue; + } + if (order.getCreatedAt() == null) { + errors.add(ErrorCodes.LOYALTY_INVALID_INPUT + ":missing-date:" + order.getUserId()); + continue; + } + if (order.getCreatedAt().isBefore(cutoff)) { + errors.add(ErrorCodes.LOYALTY_INVALID_INPUT + ":out-of-window:" + order.getUserId()); + continue; } + valid.add(order); } - Map discounts = calculator.calculateDiscounts(safeOrders); + + LoyaltyCalculator.CalculationResult calcResult = calculator.calculateDiscounts(valid); + Map discounts = calcResult.discounts(); discountStore.saveAll(discounts); - return new BatchResult(discounts, errors); + Stats stats = new Stats(valid.size(), orders.size() - valid.size(), errors.size()); + return new BatchResult(discounts, errors, calcResult.warnings(), stats); + } + + public record BatchResult(Map discounts, List errors, List warnings, Stats stats) { + public BatchResult { + discounts = discounts == null ? Map.of() : Map.copyOf(discounts); + errors = errors == null ? List.of() : List.copyOf(errors); + warnings = warnings == null ? List.of() : List.copyOf(warnings); + } } - public record BatchResult(Map discounts, List errors) {} + public record Stats(int processed, int skipped, int errors) { + public Stats { + if (processed < 0 || skipped < 0 || errors < 0) { + throw new IllegalArgumentException("stats cannot be negative"); + } + } + } } diff --git a/services/loyalty-batch/src/main/java/com/example/loyalty/service/LoyaltyCalculator.java b/services/loyalty-batch/src/main/java/com/example/loyalty/service/LoyaltyCalculator.java index 29f02e4..737bd84 100644 --- a/services/loyalty-batch/src/main/java/com/example/loyalty/service/LoyaltyCalculator.java +++ b/services/loyalty-batch/src/main/java/com/example/loyalty/service/LoyaltyCalculator.java @@ -3,6 +3,10 @@ import com.example.common.MoneyUtils; import com.example.loyalty.model.OrderRecord; import java.math.BigDecimal; +import java.time.Clock; +import java.time.DayOfWeek; +import java.time.LocalDate; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -10,19 +14,64 @@ public class LoyaltyCalculator { - private final BigDecimal baseRate; + private final LoyaltyConfig config; + private final Clock clock; + private static final String WARNING_CAP_APPLIED = "cap-applied"; - public LoyaltyCalculator(BigDecimal baseRate) { - this.baseRate = Objects.requireNonNull(baseRate, "baseRate"); + public LoyaltyCalculator(LoyaltyConfig config) { + this(config, Clock.systemDefaultZone()); } - public Map calculateDiscounts(List orders) { + public LoyaltyCalculator(LoyaltyConfig config, Clock clock) { + this.config = Objects.requireNonNull(config, "config"); + this.clock = Objects.requireNonNull(clock, "clock"); + } + + public CalculationResult calculateDiscounts(List orders) { Objects.requireNonNull(orders, "orders"); Map result = new HashMap<>(); + List warnings = new ArrayList<>(); + LocalDate cutoff = LocalDate.now(clock).minusDays(config.windowDays() - 1L); + for (OrderRecord order : orders) { - BigDecimal discount = MoneyUtils.roundToCents(order.getAmount().multiply(baseRate)); + if (order == null || order.getUserId() == null || order.getUserId().isBlank()) { + continue; + } + if (order.getAmount() == null || order.getCreatedAt() == null) { + continue; + } + if (order.getCreatedAt().isBefore(cutoff)) { + continue; + } + if (order.getAmount().compareTo(config.minOrderAmount()) < 0) { + continue; + } + + BigDecimal discount = MoneyUtils.roundToCents(order.getAmount().multiply(config.baseRate())); + if (order.getAmount().compareTo(config.highValueThreshold()) >= 0) { + discount = discount.add(MoneyUtils.roundToCents(order.getAmount().multiply(config.bonusRate()))); + } + if (isWeekend(order.getCreatedAt())) { + discount = discount.add(MoneyUtils.roundToCents(order.getAmount().multiply(config.weekendBonusRate()))); + } result.merge(order.getUserId(), discount, BigDecimal::add); } - return result; + + for (Map.Entry entry : new ArrayList<>(result.entrySet())) { + BigDecimal capped = entry.getValue().min(config.userCap()); + if (capped.compareTo(entry.getValue()) < 0) { + warnings.add(WARNING_CAP_APPLIED + ":" + entry.getKey()); + result.put(entry.getKey(), MoneyUtils.roundToCents(capped)); + } + } + + return new CalculationResult(Map.copyOf(result), List.copyOf(warnings)); } + + private boolean isWeekend(LocalDate date) { + DayOfWeek dow = date.getDayOfWeek(); + return dow == DayOfWeek.SATURDAY || dow == DayOfWeek.SUNDAY; + } + + public record CalculationResult(Map discounts, List warnings) {} } diff --git a/services/loyalty-batch/src/main/java/com/example/loyalty/service/LoyaltyConfig.java b/services/loyalty-batch/src/main/java/com/example/loyalty/service/LoyaltyConfig.java new file mode 100644 index 0000000..6ca1cef --- /dev/null +++ b/services/loyalty-batch/src/main/java/com/example/loyalty/service/LoyaltyConfig.java @@ -0,0 +1,26 @@ +package com.example.loyalty.service; + +import java.math.BigDecimal; +import java.util.Objects; + +public record LoyaltyConfig( + BigDecimal baseRate, + BigDecimal bonusRate, + BigDecimal highValueThreshold, + BigDecimal userCap, + BigDecimal minOrderAmount, + BigDecimal weekendBonusRate, + int windowDays +) { + public LoyaltyConfig { + Objects.requireNonNull(baseRate, "baseRate"); + Objects.requireNonNull(bonusRate, "bonusRate"); + Objects.requireNonNull(highValueThreshold, "highValueThreshold"); + Objects.requireNonNull(userCap, "userCap"); + Objects.requireNonNull(minOrderAmount, "minOrderAmount"); + Objects.requireNonNull(weekendBonusRate, "weekendBonusRate"); + if (windowDays <= 0) { + throw new IllegalArgumentException("windowDays must be positive"); + } + } +} diff --git a/services/loyalty-batch/src/test/java/com/example/loyalty/BatchRunnerTest.java b/services/loyalty-batch/src/test/java/com/example/loyalty/BatchRunnerTest.java index 7f94ca4..1230119 100644 --- a/services/loyalty-batch/src/test/java/com/example/loyalty/BatchRunnerTest.java +++ b/services/loyalty-batch/src/test/java/com/example/loyalty/BatchRunnerTest.java @@ -7,8 +7,11 @@ import com.example.loyalty.model.OrderRecord; import com.example.loyalty.service.BatchRunner; import com.example.loyalty.service.LoyaltyCalculator; +import com.example.loyalty.service.LoyaltyConfig; import java.math.BigDecimal; +import java.time.Clock; import java.time.LocalDate; +import java.time.ZoneOffset; import java.util.List; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -24,10 +27,34 @@ void setUp() { @Test void savesDiscountsToRepository() { - BatchRunner runner = new BatchRunner(new LoyaltyCalculator(new BigDecimal("0.1")), repository); + LoyaltyConfig config = new LoyaltyConfig(new BigDecimal("0.1"), new BigDecimal("0.02"), + new BigDecimal("100.00"), new BigDecimal("50.00"), new BigDecimal("1.00"), new BigDecimal("0.01"), 30); + Clock clock = Clock.fixed(LocalDate.of(2025, 1, 31).atStartOfDay().toInstant(ZoneOffset.UTC), ZoneOffset.UTC); + BatchRunner runner = new BatchRunner(new LoyaltyCalculator(config, clock), config, repository, clock); - runner.run(List.of(new OrderRecord("u1", new BigDecimal("100.00"), LocalDate.now()))); + runner.run(List.of(new OrderRecord("u1", new BigDecimal("100.00"), LocalDate.of(2025, 1, 26)))); - assertEquals(new BigDecimal("10.00"), repository.findByUserId("u1").orElse(BigDecimal.ZERO)); + assertEquals(new BigDecimal("13.00"), repository.findByUserId("u1").orElse(BigDecimal.ZERO)); // 10% + 1% weekend bonus + 2% bonus + } + + @Test + void skipsInvalidAndOutOfWindowOrdersWithErrors() { + LoyaltyConfig config = new LoyaltyConfig(new BigDecimal("0.1"), new BigDecimal("0.02"), + new BigDecimal("100.00"), new BigDecimal("50.00"), new BigDecimal("1.00"), new BigDecimal("0.01"), 30); + Clock clock = Clock.fixed(LocalDate.of(2025, 1, 31).atStartOfDay().toInstant(ZoneOffset.UTC), ZoneOffset.UTC); + BatchRunner runner = new BatchRunner(new LoyaltyCalculator(config, clock), config, repository, clock); + + List orders = List.of( + new OrderRecord("u1", new BigDecimal("0.50"), LocalDate.of(2025, 1, 26)), // too small + new OrderRecord("u2", new BigDecimal("10.00"), LocalDate.of(2024, 12, 10)), // out of window + new OrderRecord("u3", new BigDecimal("20.00"), LocalDate.of(2025, 1, 26)) // ok + ); + + BatchRunner.BatchResult result = runner.run(orders); + + assertEquals(1, result.discounts().size()); + assertEquals(1, result.stats().processed()); + assertEquals(2, result.stats().skipped()); + assertEquals(2, result.errors().size()); } } diff --git a/services/loyalty-batch/src/test/java/com/example/loyalty/LoyaltyCalculatorTest.java b/services/loyalty-batch/src/test/java/com/example/loyalty/LoyaltyCalculatorTest.java index 5aef996..25b5849 100644 --- a/services/loyalty-batch/src/test/java/com/example/loyalty/LoyaltyCalculatorTest.java +++ b/services/loyalty-batch/src/test/java/com/example/loyalty/LoyaltyCalculatorTest.java @@ -2,13 +2,13 @@ import static org.junit.jupiter.api.Assertions.*; -import com.example.common.db.DiscountRepository; -import com.example.common.db.InMemoryDataSourceFactory; import com.example.loyalty.model.OrderRecord; -import com.example.loyalty.service.BatchRunner; import com.example.loyalty.service.LoyaltyCalculator; +import com.example.loyalty.service.LoyaltyConfig; import java.math.BigDecimal; +import java.time.Clock; import java.time.LocalDate; +import java.time.ZoneOffset; import java.util.List; import java.util.Map; import org.junit.jupiter.api.Test; @@ -17,37 +17,40 @@ class LoyaltyCalculatorTest { @Test void calculatesDiscountsPerUser() { - LoyaltyCalculator calculator = new LoyaltyCalculator(new BigDecimal("0.05")); - BatchRunner runner = new BatchRunner(calculator, new DiscountRepository(InMemoryDataSourceFactory.createAndInit())); + var config = new LoyaltyConfig(new BigDecimal("0.05"), new BigDecimal("0.02"), new BigDecimal("100.00"), + new BigDecimal("50.00"), new BigDecimal("1.00"), new BigDecimal("0.01"), 30); + Clock clock = Clock.fixed(LocalDate.of(2025, 1, 31).atStartOfDay().toInstant(ZoneOffset.UTC), ZoneOffset.UTC); + LoyaltyCalculator calculator = new LoyaltyCalculator(config, clock); List orders = List.of( - new OrderRecord("u1", new BigDecimal("100.00"), LocalDate.now()), - new OrderRecord("u1", new BigDecimal("50.00"), LocalDate.now()), - new OrderRecord("u2", new BigDecimal("200.00"), LocalDate.now()) + new OrderRecord("u1", new BigDecimal("100.00"), LocalDate.of(2025, 1, 26)), // воскресенье + new OrderRecord("u1", new BigDecimal("50.00"), LocalDate.of(2025, 1, 26)), + new OrderRecord("u2", new BigDecimal("200.00"), LocalDate.of(2025, 1, 25)) // суббота ); - BatchRunner.BatchResult result = runner.run(orders); + LoyaltyCalculator.CalculationResult result = calculator.calculateDiscounts(orders); Map discounts = result.discounts(); - assertEquals(new BigDecimal("7.50"), discounts.get("u1")); - assertEquals(new BigDecimal("10.00"), discounts.get("u2")); - assertTrue(result.errors().isEmpty()); + // u1: 100*(0.05+0.02+0.01)=8 + 50*(0.05+0.01)=3 => 11.00 + assertEquals(new BigDecimal("11.00"), discounts.get("u1")); + // u2: 200*(0.05+0.02+0.01)=16 + assertEquals(new BigDecimal("16.00"), discounts.get("u2")); + assertTrue(result.warnings().isEmpty()); } @Test - void skipsNegativeAmountsAndReportsErrors() { - LoyaltyCalculator calculator = new LoyaltyCalculator(new BigDecimal("0.05")); - BatchRunner runner = new BatchRunner(calculator, new DiscountRepository(InMemoryDataSourceFactory.createAndInit())); + void appliesCapAndAddsWarning() { + var config = new LoyaltyConfig(new BigDecimal("0.10"), new BigDecimal("0.05"), new BigDecimal("50.00"), + new BigDecimal("20.00"), new BigDecimal("1.00"), new BigDecimal("0.01"), 30); + LoyaltyCalculator calculator = new LoyaltyCalculator(config); List orders = List.of( - new OrderRecord("u1", new BigDecimal("-10.00"), LocalDate.now()), - new OrderRecord("u2", new BigDecimal("100.00"), LocalDate.now()) + new OrderRecord("u1", new BigDecimal("200.00"), LocalDate.now()) ); - BatchRunner.BatchResult result = runner.run(orders); - - assertEquals(new BigDecimal("5.00"), result.discounts().get("u2")); - assertEquals(1, result.errors().size()); - assertTrue(result.errors().get(0).contains("negative-amount")); + LoyaltyCalculator.CalculationResult result = calculator.calculateDiscounts(orders); + // 200*(0.10+0.05+0.01) = 32, но cap=20 + assertEquals(new BigDecimal("20.00"), result.discounts().get("u1")); + assertTrue(result.warnings().contains("cap-applied:u1")); } } diff --git a/services/order-api/src/main/java/com/example/orderapi/client/TimeoutDeliveryClient.java b/services/order-api/src/main/java/com/example/orderapi/client/TimeoutDeliveryClient.java new file mode 100644 index 0000000..e8790e3 --- /dev/null +++ b/services/order-api/src/main/java/com/example/orderapi/client/TimeoutDeliveryClient.java @@ -0,0 +1,39 @@ +package com.example.orderapi.client; + +import com.example.common.model.Address; +import com.example.common.model.Item; +import com.example.orderapi.model.DeliveryQuote; +import java.time.Duration; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; + +/** + * Оборачивает DeliveryClient и гарантирует таймаут + безопасное падение в Optional.empty(). + */ +public class TimeoutDeliveryClient implements DeliveryClient { + + private final DeliveryClient delegate; + private final Duration timeout; + + public TimeoutDeliveryClient(DeliveryClient delegate, Duration timeout) { + this.delegate = Objects.requireNonNull(delegate, "delegate"); + this.timeout = Objects.requireNonNull(timeout, "timeout"); + } + + @Override + public Optional getQuote(Address address, List items) { + try { + return CompletableFuture + .supplyAsync(() -> delegate.getQuote(address, items)) + .orTimeout(timeout.toMillis(), TimeUnit.MILLISECONDS) + .exceptionally(ex -> Optional.empty()) + .get(); + } catch (Exception e) { + return Optional.empty(); + } + } +} + diff --git a/services/order-api/src/main/java/com/example/orderapi/model/OrderPriceRequest.java b/services/order-api/src/main/java/com/example/orderapi/model/OrderPriceRequest.java index c41e918..b8f765d 100644 --- a/services/order-api/src/main/java/com/example/orderapi/model/OrderPriceRequest.java +++ b/services/order-api/src/main/java/com/example/orderapi/model/OrderPriceRequest.java @@ -9,11 +9,17 @@ public class OrderPriceRequest { private final List items; private final Address address; private final String userId; + private final String currency; public OrderPriceRequest(List items, Address address, String userId) { + this(items, address, userId, "USD"); + } + + public OrderPriceRequest(List items, Address address, String userId, String currency) { this.items = Objects.requireNonNull(items, "items"); this.address = Objects.requireNonNull(address, "address"); this.userId = Objects.requireNonNull(userId, "userId"); + this.currency = Objects.requireNonNull(currency, "currency"); } public List getItems() { @@ -27,4 +33,8 @@ public Address getAddress() { public String getUserId() { return userId; } + + public String getCurrency() { + return currency; + } } diff --git a/services/order-api/src/main/java/com/example/orderapi/model/OrderPriceResponse.java b/services/order-api/src/main/java/com/example/orderapi/model/OrderPriceResponse.java index c293c63..253ef0e 100644 --- a/services/order-api/src/main/java/com/example/orderapi/model/OrderPriceResponse.java +++ b/services/order-api/src/main/java/com/example/orderapi/model/OrderPriceResponse.java @@ -1,6 +1,7 @@ package com.example.orderapi.model; import java.math.BigDecimal; +import java.util.List; import java.util.Optional; public class OrderPriceResponse { @@ -10,15 +11,22 @@ public class OrderPriceResponse { private final BigDecimal total; private final String currency; private final Error error; + private final List warnings; public OrderPriceResponse(BigDecimal itemsTotal, BigDecimal deliveryFee, BigDecimal discount, BigDecimal total, String currency, Error error) { + this(itemsTotal, deliveryFee, discount, total, currency, error, List.of()); + } + + public OrderPriceResponse(BigDecimal itemsTotal, BigDecimal deliveryFee, BigDecimal discount, + BigDecimal total, String currency, Error error, List warnings) { this.itemsTotal = itemsTotal; this.deliveryFee = deliveryFee; this.discount = discount; this.total = total; this.currency = currency; this.error = error; + this.warnings = warnings == null ? List.of() : List.copyOf(warnings); } public BigDecimal getItemsTotal() { @@ -45,5 +53,9 @@ public Optional getError() { return Optional.ofNullable(error); } + public List getWarnings() { + return warnings; + } + public record Error(String code, String message) {} } diff --git a/services/order-api/src/main/java/com/example/orderapi/service/PriceService.java b/services/order-api/src/main/java/com/example/orderapi/service/PriceService.java index 506911b..2df22ec 100644 --- a/services/order-api/src/main/java/com/example/orderapi/service/PriceService.java +++ b/services/order-api/src/main/java/com/example/orderapi/service/PriceService.java @@ -2,63 +2,153 @@ import com.example.common.ErrorCodes; import com.example.common.MoneyUtils; -import com.example.orderapi.client.DeliveryClient; +import com.example.common.db.DiscountStore; import com.example.common.model.Address; -import com.example.orderapi.model.DeliveryQuote; import com.example.common.model.Item; +import com.example.orderapi.client.DeliveryClient; +import com.example.orderapi.model.DeliveryQuote; import com.example.orderapi.model.OrderPriceRequest; import com.example.orderapi.model.OrderPriceResponse; import java.math.BigDecimal; +import java.math.RoundingMode; +import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; public class PriceService { private final DeliveryClient deliveryClient; - private final DiscountPolicy discountPolicy; - private final String currency; + private final DiscountStore discountStore; + private final String defaultCurrency; - public PriceService(DeliveryClient deliveryClient, DiscountPolicy discountPolicy, String currency) { + private static final BigDecimal MAX_DISCOUNT_RATE = new BigDecimal("0.20"); + private static final BigDecimal SMALL_ORDER_THRESHOLD = new BigDecimal("20.00"); + private static final BigDecimal SMALL_ORDER_FEE = new BigDecimal("2.50"); + private static final BigDecimal FREE_DELIVERY_THRESHOLD = new BigDecimal("100.00"); + private static final BigDecimal MIN_ORDER_TOTAL = new BigDecimal("1.00"); + + public PriceService(DeliveryClient deliveryClient, DiscountStore discountStore, String defaultCurrency) { this.deliveryClient = Objects.requireNonNull(deliveryClient, "deliveryClient"); - this.discountPolicy = Objects.requireNonNull(discountPolicy, "discountPolicy"); - this.currency = Objects.requireNonNull(currency, "currency"); + this.discountStore = Objects.requireNonNull(discountStore, "discountStore"); + this.defaultCurrency = Objects.requireNonNull(defaultCurrency, "defaultCurrency"); } public OrderPriceResponse calculatePrice(OrderPriceRequest request) { - validateRequest(request); + List normalizedItems = normalizeItems(request.getItems()); + BigDecimal itemsTotal = sumItems(normalizedItems); - BigDecimal itemsTotal = request.getItems().stream() - .map(item -> item.getUnitPrice().multiply(BigDecimal.valueOf(item.getQuantity()))) - .map(MoneyUtils::roundToCents) - .reduce(BigDecimal.ZERO, BigDecimal::add); + if (itemsTotal.compareTo(MIN_ORDER_TOTAL) < 0) { + return new OrderPriceResponse( + itemsTotal, + MoneyUtils.roundToCents(BigDecimal.ZERO), + MoneyUtils.roundToCents(BigDecimal.ZERO), + MoneyUtils.roundToCents(BigDecimal.ZERO), + resolveCurrency(request), + new OrderPriceResponse.Error(ErrorCodes.ORDER_TOO_SMALL, "Order total below minimum"), + List.of("order-too-small") + ); + } - BigDecimal discount = MoneyUtils.roundToCents(discountPolicy.calculateDiscount(request, itemsTotal)); - DeliveryResult deliveryResult = quoteDelivery(request.getAddress(), request.getItems()); + List warnings = new ArrayList<>(); + DiscountResult discountResult = calculateDiscount(request, itemsTotal); + if (discountResult.capped()) { + warnings.add("discount-capped"); + } + DeliveryResult deliveryResult = quoteDelivery(request.getAddress(), normalizedItems, itemsTotal, warnings); BigDecimal deliveryFee = deliveryResult.fee; - BigDecimal total = MoneyUtils.roundToCents(itemsTotal.add(deliveryFee).subtract(discount)); + + BigDecimal total = MoneyUtils.roundToCents( + itemsTotal.add(deliveryFee).subtract(discountResult.amount()).max(BigDecimal.ZERO)); OrderPriceResponse.Error error = deliveryResult.errorCode == null ? null : new OrderPriceResponse.Error(deliveryResult.errorCode, deliveryResult.errorMessage); - return new OrderPriceResponse(itemsTotal, deliveryFee, discount, total, currency, error); + return new OrderPriceResponse(itemsTotal, deliveryFee, discountResult.amount(), total, resolveCurrency(request), error, warnings); + } + + private String resolveCurrency(OrderPriceRequest request) { + String cur = request.getCurrency(); + return cur == null || cur.isBlank() ? defaultCurrency : cur; } - private void validateRequest(OrderPriceRequest request) { - if (request.getItems().isEmpty()) { + private List normalizeItems(List items) { + if (items.isEmpty()) { throw new IllegalArgumentException("items must not be empty"); } + Map aggregated = new HashMap<>(); + for (Item item : items) { + if (item.getQuantity() <= 0) { + throw new IllegalArgumentException("quantity must be positive"); + } + if (item.getUnitPrice().compareTo(BigDecimal.ZERO) < 0) { + throw new IllegalArgumentException("unitPrice must be non-negative"); + } + aggregated.compute(item.getSku(), (sku, acc) -> { + ItemAccumulator next = acc == null ? new ItemAccumulator() : acc; + next.add(item.getQuantity(), item.getUnitPrice()); + return next; + }); + } + List result = new ArrayList<>(); + for (Map.Entry entry : aggregated.entrySet()) { + ItemAccumulator acc = entry.getValue(); + BigDecimal averagePrice = acc.amount.divide(BigDecimal.valueOf(acc.qty), 2, RoundingMode.HALF_UP); + result.add(new Item(entry.getKey(), acc.qty, averagePrice)); + } + return result; + } + + private BigDecimal sumItems(List items) { + return items.stream() + .map(item -> item.getUnitPrice().multiply(BigDecimal.valueOf(item.getQuantity()))) + .map(MoneyUtils::roundToCents) + .reduce(BigDecimal.ZERO, BigDecimal::add); } - private DeliveryResult quoteDelivery(Address address, List items) { - Optional quote = deliveryClient.getQuote(address, items); - if (quote.isEmpty()) { + private DiscountResult calculateDiscount(OrderPriceRequest request, BigDecimal itemsTotal) { + BigDecimal stored = discountStore.findByUserId(request.getUserId()).orElse(BigDecimal.ZERO); + BigDecimal cap = MoneyUtils.roundToCents(itemsTotal.multiply(MAX_DISCOUNT_RATE)); + BigDecimal applied = MoneyUtils.roundToCents(stored.min(cap)); + boolean capped = stored.compareTo(cap) > 0; + return new DiscountResult(applied, capped); + } + + private DeliveryResult quoteDelivery(Address address, List items, BigDecimal itemsTotal, List warnings) { + try { + if (itemsTotal.compareTo(FREE_DELIVERY_THRESHOLD) >= 0) { + return new DeliveryResult(MoneyUtils.roundToCents(BigDecimal.ZERO), null, null); + } + Optional quote = deliveryClient.getQuote(address, items); + if (quote.isEmpty()) { + warnings.add("delivery-unavailable"); + return new DeliveryResult(MoneyUtils.roundToCents(BigDecimal.ZERO), ErrorCodes.DELIVERY_UNAVAILABLE, "Delivery unavailable"); + } + BigDecimal fee = MoneyUtils.roundToCents(quote.get().getFee()); + if (itemsTotal.compareTo(SMALL_ORDER_THRESHOLD) < 0) { + fee = MoneyUtils.roundToCents(fee.add(SMALL_ORDER_FEE)); + warnings.add("small-order-fee"); + } + return new DeliveryResult(fee, null, null); + } catch (Exception ex) { + warnings.add("delivery-error"); return new DeliveryResult(MoneyUtils.roundToCents(BigDecimal.ZERO), ErrorCodes.DELIVERY_UNAVAILABLE, "Delivery unavailable"); } - DeliveryQuote q = quote.get(); - return new DeliveryResult(MoneyUtils.roundToCents(q.getFee()), null, null); } + private record DiscountResult(BigDecimal amount, boolean capped) {} private record DeliveryResult(BigDecimal fee, String errorCode, String errorMessage) {} + + private static final class ItemAccumulator { + int qty = 0; + BigDecimal amount = BigDecimal.ZERO; + + void add(int q, BigDecimal price) { + qty += q; + amount = amount.add(price.multiply(BigDecimal.valueOf(q))); + } + } } diff --git a/services/order-api/src/main/java/com/example/orderapi/service/UserDiscountPolicy.java b/services/order-api/src/main/java/com/example/orderapi/service/UserDiscountPolicy.java new file mode 100644 index 0000000..08807ff --- /dev/null +++ b/services/order-api/src/main/java/com/example/orderapi/service/UserDiscountPolicy.java @@ -0,0 +1,33 @@ +package com.example.orderapi.service; + +import com.example.common.MoneyUtils; +import com.example.common.db.DiscountStore; +import com.example.orderapi.model.OrderPriceRequest; +import java.math.BigDecimal; +import java.util.Objects; + +/** + * Скидка из хранилища пользователя с ограничением по максимальной ставке. + */ +public class UserDiscountPolicy implements DiscountPolicy { + + private final DiscountStore discountStore; + private final BigDecimal maxRate; + + public UserDiscountPolicy(DiscountStore discountStore, BigDecimal maxRate) { + this.discountStore = Objects.requireNonNull(discountStore, "discountStore"); + this.maxRate = Objects.requireNonNull(maxRate, "maxRate"); + } + + @Override + public BigDecimal calculateDiscount(OrderPriceRequest request, BigDecimal itemsTotal) { + if (itemsTotal.compareTo(BigDecimal.ZERO) <= 0) { + return BigDecimal.ZERO; + } + var stored = discountStore.findByUserId(request.getUserId()).orElse(BigDecimal.ZERO); + BigDecimal cap = itemsTotal.multiply(maxRate); + BigDecimal applied = stored.min(cap); + return MoneyUtils.roundToCents(applied); + } +} + diff --git a/services/order-api/src/test/java/com/example/orderapi/OrderServiceTest.java b/services/order-api/src/test/java/com/example/orderapi/OrderServiceTest.java index fba7356..d4c7528 100644 --- a/services/order-api/src/test/java/com/example/orderapi/OrderServiceTest.java +++ b/services/order-api/src/test/java/com/example/orderapi/OrderServiceTest.java @@ -15,8 +15,6 @@ import com.example.orderapi.model.OrderPriceRequest; import com.example.orderapi.model.OrderPriceResponse; import com.example.common.model.OrderStatus; -import com.example.orderapi.service.DiscountPolicy; -import com.example.orderapi.service.NoDiscountPolicy; import com.example.orderapi.service.OrderService; import com.example.orderapi.service.PriceService; import java.math.BigDecimal; @@ -39,7 +37,7 @@ void setUp() { discountRepository = new DiscountRepository(ds); userRepository = new UserRepository(ds); userRepository.upsert(new UserRepository.User("u1", true, "EU")); - PriceService priceService = new PriceService(deliveryClient, new NoDiscountPolicy(), "USD"); + PriceService priceService = new PriceService(deliveryClient, discountRepository, "USD"); orderService = new OrderService(new OrderRepository(ds), userRepository, discountRepository, priceService); } @@ -63,11 +61,11 @@ void appliesDiscountFromRepoIfPresent() { when(deliveryClient.getQuote(any(Address.class), any())).thenReturn(Optional.of(new DeliveryQuote(new BigDecimal("0.00"), "USD"))); Order order = orderService.createOrder("u1", - List.of(new Item("sku", 1, new BigDecimal("10.00"))), + List.of(new Item("sku", 1, new BigDecimal("20.00"))), new Address("Main st"), "USD"); - assertEquals(new BigDecimal("7.00"), order.getTotal()); + assertEquals(new BigDecimal("17.00"), order.getTotal()); } @Test diff --git a/services/order-api/src/test/java/com/example/orderapi/PriceServiceTest.java b/services/order-api/src/test/java/com/example/orderapi/PriceServiceTest.java index 1ad11e9..01cc42c 100644 --- a/services/order-api/src/test/java/com/example/orderapi/PriceServiceTest.java +++ b/services/order-api/src/test/java/com/example/orderapi/PriceServiceTest.java @@ -1,17 +1,20 @@ package com.example.orderapi; import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.Mockito.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import com.example.common.ErrorCodes; -import com.example.orderapi.client.DeliveryClient; +import com.example.common.db.DiscountRepository; +import com.example.common.db.DiscountStore; +import com.example.common.db.InMemoryDataSourceFactory; import com.example.common.model.Address; -import com.example.orderapi.model.DeliveryQuote; import com.example.common.model.Item; +import com.example.orderapi.client.DeliveryClient; +import com.example.orderapi.model.DeliveryQuote; import com.example.orderapi.model.OrderPriceRequest; import com.example.orderapi.model.OrderPriceResponse; -import com.example.orderapi.service.DiscountPolicy; -import com.example.orderapi.service.NoDiscountPolicy; import com.example.orderapi.service.PriceService; import java.math.BigDecimal; import java.util.List; @@ -22,30 +25,70 @@ class PriceServiceTest { private DeliveryClient deliveryClient; - private DiscountPolicy discountPolicy; + private DiscountStore discountStore; @BeforeEach void setUp() { deliveryClient = mock(DeliveryClient.class); - discountPolicy = new NoDiscountPolicy(); + discountStore = new DiscountRepository(InMemoryDataSourceFactory.createAndInit()); } @Test - void calculatesTotalWithDeliveryAndNoDiscount() { + void appliesDiscountCapAndDeliveryFee() { + discountStore.saveAll(java.util.Map.of("user-1", new BigDecimal("50.00"))); // заведомо больше 20% от суммы + OrderPriceRequest request = new OrderPriceRequest( - List.of(new Item("sku-1", 2, new BigDecimal("10.00"))), + List.of(new Item("sku-1", 2, new BigDecimal("100.00"))), new Address("Main st"), - "user-1"); + "user-1" + ); when(deliveryClient.getQuote(any(), any())).thenReturn(Optional.of(new DeliveryQuote(new BigDecimal("5.00"), "USD"))); - PriceService service = new PriceService(deliveryClient, discountPolicy, "USD"); + PriceService service = new PriceService(deliveryClient, discountStore, "USD"); + OrderPriceResponse response = service.calculatePrice(request); + + assertEquals(new BigDecimal("200.00"), response.getItemsTotal()); + // скидка урезана до 20% = 40.00 + assertEquals(new BigDecimal("40.00"), response.getDiscount()); + assertTrue(response.getWarnings().contains("discount-capped")); + assertEquals(new BigDecimal("0.00"), response.getDeliveryFee()); // бесплатная доставка по порогу + assertEquals(new BigDecimal("160.00"), response.getTotal()); + } + + @Test + void addsSmallOrderFee() { + OrderPriceRequest request = new OrderPriceRequest( + List.of(new Item("sku-1", 1, new BigDecimal("10.00"))), + new Address("Main st"), + "user-1" + ); + + when(deliveryClient.getQuote(any(), any())).thenReturn(Optional.of(new DeliveryQuote(new BigDecimal("1.00"), "USD"))); + + PriceService service = new PriceService(deliveryClient, discountStore, "USD"); + OrderPriceResponse response = service.calculatePrice(request); + + assertEquals(new BigDecimal("10.00"), response.getItemsTotal()); + assertEquals(new BigDecimal("3.50"), response.getDeliveryFee()); // 1.00 + small order fee 2.50 + assertTrue(response.getWarnings().contains("small-order-fee")); + } + + @Test + void providesFreeDeliveryOnThreshold() { + OrderPriceRequest request = new OrderPriceRequest( + List.of(new Item("sku-1", 1, new BigDecimal("100.00"))), + new Address("Main st"), + "user-1" + ); + + // даже если сервис вернёт стоимость, она должна быть обнулена из-за порога + when(deliveryClient.getQuote(any(), any())).thenReturn(Optional.of(new DeliveryQuote(new BigDecimal("9.00"), "USD"))); + + PriceService service = new PriceService(deliveryClient, discountStore, "USD"); OrderPriceResponse response = service.calculatePrice(request); - assertEquals(new BigDecimal("20.00"), response.getItemsTotal()); - assertEquals(new BigDecimal("5.00"), response.getDeliveryFee()); - assertEquals(new BigDecimal("0.00"), response.getDiscount()); - assertEquals(new BigDecimal("25.00"), response.getTotal()); + assertEquals(new BigDecimal("0.00"), response.getDeliveryFee()); assertTrue(response.getError().isEmpty()); } @@ -54,22 +97,40 @@ void marksErrorWhenDeliveryUnavailable() { OrderPriceRequest request = new OrderPriceRequest( List.of(new Item("sku-1", 1, new BigDecimal("12.50"))), new Address("Main st"), - "user-1"); + "user-1" + ); when(deliveryClient.getQuote(any(), any())).thenReturn(Optional.empty()); - PriceService service = new PriceService(deliveryClient, discountPolicy, "USD"); + PriceService service = new PriceService(deliveryClient, discountStore, "USD"); OrderPriceResponse response = service.calculatePrice(request); - assertEquals(new BigDecimal("12.50"), response.getItemsTotal()); - assertEquals(new BigDecimal("0.00"), response.getDeliveryFee()); assertEquals(ErrorCodes.DELIVERY_UNAVAILABLE, response.getError().map(OrderPriceResponse.Error::code).orElse("")); + assertTrue(response.getWarnings().contains("delivery-unavailable")); } @Test void rejectsEmptyItems() { OrderPriceRequest request = new OrderPriceRequest(List.of(), new Address("Main st"), "user-1"); - PriceService service = new PriceService(deliveryClient, discountPolicy, "USD"); + PriceService service = new PriceService(deliveryClient, discountStore, "USD"); assertThrows(IllegalArgumentException.class, () -> service.calculatePrice(request)); } + + @Test + void failsIfTotalTooSmall() { + OrderPriceRequest request = new OrderPriceRequest( + List.of(new Item("sku-1", 1, new BigDecimal("0.50"))), + new Address("Main st"), + "user-1" + ); + + when(deliveryClient.getQuote(any(), any())).thenReturn(Optional.of(new DeliveryQuote(new BigDecimal("1.00"), "USD"))); + + PriceService service = new PriceService(deliveryClient, discountStore, "USD"); + OrderPriceResponse response = service.calculatePrice(request); + + assertEquals(ErrorCodes.ORDER_TOO_SMALL, response.getError().map(OrderPriceResponse.Error::code).orElse("")); + assertEquals(new BigDecimal("0.50"), response.getItemsTotal()); + assertEquals(new BigDecimal("0.00"), response.getTotal()); + } } diff --git a/services/order-api/src/test/java/com/example/orderapi/client/TimeoutDeliveryClientTest.java b/services/order-api/src/test/java/com/example/orderapi/client/TimeoutDeliveryClientTest.java new file mode 100644 index 0000000..4aac6ef --- /dev/null +++ b/services/order-api/src/test/java/com/example/orderapi/client/TimeoutDeliveryClientTest.java @@ -0,0 +1,59 @@ +package com.example.orderapi.client; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.example.common.model.Address; +import com.example.common.model.Item; +import com.example.orderapi.model.DeliveryQuote; +import java.math.BigDecimal; +import java.time.Duration; +import java.util.List; +import java.util.Optional; +import org.junit.jupiter.api.Test; + +class TimeoutDeliveryClientTest { + + @Test + void returnsDelegateResult() { + DeliveryClient delegate = (addr, items) -> Optional.of(new DeliveryQuote(new BigDecimal("5.00"), "USD")); + TimeoutDeliveryClient client = new TimeoutDeliveryClient(delegate, Duration.ofMillis(200)); + + Optional quote = client.getQuote(new Address("Main"), List.of(sampleItem())); + + assertTrue(quote.isPresent()); + assertEquals(new BigDecimal("5.00"), quote.get().getFee()); + } + + @Test + void returnsEmptyOnTimeout() { + DeliveryClient slow = (addr, items) -> { + try { + Thread.sleep(300); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + return Optional.of(new DeliveryQuote(new BigDecimal("1.00"), "USD")); + }; + TimeoutDeliveryClient client = new TimeoutDeliveryClient(slow, Duration.ofMillis(100)); + + Optional quote = client.getQuote(new Address("Main"), List.of(sampleItem())); + + assertTrue(quote.isEmpty()); + } + + @Test + void returnsEmptyOnException() { + DeliveryClient failing = (addr, items) -> { throw new IllegalStateException("boom"); }; + TimeoutDeliveryClient client = new TimeoutDeliveryClient(failing, Duration.ofMillis(100)); + + Optional quote = client.getQuote(new Address("Main"), List.of(sampleItem())); + + assertTrue(quote.isEmpty()); + } + + private static Item sampleItem() { + return new Item("sku-1", 1, new BigDecimal("10.00")); + } +} + diff --git a/specs/feature-1.md b/specs/feature-1.md index 864de39..a8349a6 100644 --- a/specs/feature-1.md +++ b/specs/feature-1.md @@ -1,11 +1,11 @@ -# Feature‑1: Order API — расчёт цены заказа с доставкой +# Feature‑1: Order API — расчёт цены заказа с доставкой + расширенные правила Эта спецификация относится к ветке `feature-1`. ## Контекст Есть сервис заказов. Нужно уметь посчитать итоговую цену до оформления заказа: сумма товаров + доставка − скидка. -Стоимость доставки приходит из внешнего сервиса доставки. +Стоимость доставки приходит из внешнего сервиса доставки. Добавляем слой бизнес-правил (скидки из БД, пороги доставки, предупреждения), чтобы PR был содержательным и покрывал больше кейсов. ## User story @@ -22,6 +22,7 @@ - `items[]`: `{ sku: string, qty: int, unitPrice: decimal }` - `address`: строка/структура (как в базе) - `userId`: string +- `currency` (опционально): string, если нет — берём дефолт. ### Response (логический контракт) @@ -30,6 +31,7 @@ - `discount`: decimal - `total`: decimal - `currency`: string +- `warnings[]`: optional string list (например, «доставка недоступна, считаем без неё», «скидка урезана до 20%») Если расчёт невозможен (например, недоступна доставка), ответ должен это **явно** отражать: статус/ошибка/сообщение по принятому в проекте контракту. @@ -38,17 +40,29 @@ - Внешний вызов доставки должен иметь **таймаут** и предсказуемое поведение при сбоях. - В логах нельзя писать PII (например, полный адрес). - Денежные расчёты должны быть через `BigDecimal` и корректное округление. +- Валидация без вывода PII: ошибки/лог не содержат полный адрес, только userId/sku/агрегированные поля. -## Граничные случаи (обязательно продумать на ревью) +## Бизнес-правила расчёта -- `items` пустой. -- `qty <= 0`. -- `unitPrice < 0`. -- доставка недоступна/таймаут. +1. **Минимальная сумма заказа**: если `itemsTotal < 1.00` — ошибка `ORDER_TOO_SMALL`. +2. **Агрегация позиций**: дубликаты SKU суммируются (qty складываются), отрицательные qty/цены → ошибка `INVALID_ITEM`. +3. **Индивидуальная скидка**: берём из таблицы `discounts` по `userId`, но не больше `MAX_DISCOUNT_RATE` от `itemsTotal` (по умолчанию 20%). Скидка не может сделать `total < 0`. Если обрезали — warning. +4. **Малая корзина**: если `itemsTotal < SMALL_ORDER_THRESHOLD` (20.00) — добавляем сервисный сбор `smallOrderFee` (фиксированная сумма) в доставку, кладём warning. +5. **Бесплатная доставка**: если `itemsTotal >= FREE_DELIVERY_THRESHOLD` (100.00) — доставка 0 при условии, что сервис доставки отвечает. Если доставка недоступна — ошибка `DELIVERY_UNAVAILABLE`. +6. **Доставка**: вызов внешнего сервиса с таймаутом. Если он недоступен, возвращаем `DELIVERY_UNAVAILABLE`, deliveryFee=0, total=itemsTotal−discount. В warnings фиксируем факт фола. +7. **Валюта**: если в запросе не передана, используем дефолт (например, `USD`). В ответе всегда единая валюта. + +## Граничные случаи (обязательно продумать/протестировать) + +- `items` пустой → ошибка. +- `qty <= 0` или `unitPrice < 0` → ошибка. +- Дубликаты SKU → агрегируются. +- `itemsTotal` ровно на порогах: `<1`, `=1`, `<20`, `=20`, `<100`, `=100`. +- Доставка недоступна/таймаут. +- Скидка больше 20% → должна быть обрезана, warning. ## Done criteria для PR -- Контракт понятен (включая ошибки). -- Тесты описывают поведение (happy path + хотя бы 1–2 границы). +- Контракт понятен (включая ошибки и warnings). +- Тесты описывают happy path + границы (таймаут доставки, маленький заказ, бесплатная доставка, обрезанная скидка). - CI объясним: если что-то падает, понятно почему и что это значит для прод‑риска. - diff --git a/specs/feature-2.md b/specs/feature-2.md index 91bb4de..5202398 100644 --- a/specs/feature-2.md +++ b/specs/feature-2.md @@ -4,7 +4,7 @@ ## Контекст -Есть ночная batch‑джоба, которая пересчитывает скидки/бонусы для пользователей на основе истории заказов. +Ночная batch‑джоба пересчитывает скидки/бонусы для пользователей на основе истории заказов. Нужны чёткие правила расчёта и диагностики, чтобы студенты могли найти ошибки и границы. ## User story @@ -15,28 +15,44 @@ Функция/сервис принимает список заказов и возвращает скидку по `userId`: - Вход: `List` (у заказа есть `userId`, `amount`, `createdAt`). -- Выход: `Map` - -Контракт должен быть **явным** по таким пунктам: -- что делать с `null`/пустыми полями (`amount == null`, `userId == null`); -- что делать с некорректными строками/записями (пропускать? падать? агрегировать ошибки?); -- политика округления (копейки, `HALF_UP`). +- Выход: `Map` + `errors[]` (код+сообщение) + `warnings[]` + `stats` (`processed`, `skipped`, `errorsCount`). + +Правила контракта: +- `userId == null/blank` → запись пропускается, ошибка `MISSING_USER`. +- `amount == null` или `amount <= 0` → пропускается, ошибка `INVALID_AMOUNT`. +- `createdAt == null` → пропускается, ошибка `MISSING_DATE`. +- Заказ старше окна расчёта (30 дней от «сейчас») → пропускается, ошибка `OUT_OF_WINDOW`. +- Сумма < `MIN_ORDER_AMOUNT` (1.00) → пропускается, ошибка `TOO_SMALL`. +- Округление денежных значений — `HALF_UP` до копеек. + +## Бизнес-правила расчёта + +1. **Базовая ставка**: `baseRate` (например, 5%) от суммы заказа. +2. **Бонус за крупный заказ**: если `amount >= HIGH_VALUE_THRESHOLD` (100.00) — доп. ставка `bonusRate` (например, 2%) на всю сумму заказа. +3. **Бонус выходного дня**: если заказ сделан в субботу/воскресенье — добавить `weekendBonusRate` (например, 1%) на сумму заказа. +4. **Кап на пользователя**: суммарная скидка на пользователя не должна превышать `USER_CAP` (например, 50.00). При обрезании добавляем warning `cap-applied:{userId}`. +5. **Минимальная сумма заказа**: `< MIN_ORDER_AMOUNT` (1.00) не учитываются (ошибка `TOO_SMALL`). +6. **Окно расчёта**: учитываем только заказы за последние `windowDays` (30) от `now()`, остальные в ошибки `OUT_OF_WINDOW`. +7. **Диагностика**: возвращаем `errors[]`, `warnings[]`, `stats` (`processed`, `skipped`, `errorsCount`). ## Нефункциональные требования -- Результат не должен мутировать входные данные (и не должен отдавать наружу mutable структуры, которые легко сломать). -- Джоба должна быть диагностируемой: сколько записей обработали, сколько ошибок, что пошло не так. +- Не мутировать входные коллекции; наружу отдавать неизменяемые структуры. +- Джоба должна быть диагностируемой: ошибки с кодами, счётчики обработанных/пропущенных записей. +- Логи/ошибки не должны содержать PII кроме `userId`. -## Граничные случаи (обязательно продумать на ревью) +## Граничные случаи (протестировать) - Пустой список заказов. -- `amount == null`. -- `amount < 0`. -- Граничные даты (например, конец года) — если есть правила по датам. +- `amount <= 0`, `amount < MIN_ORDER_AMOUNT`. +- `userId` пустой/`null`. +- Дата `null` или заказ вне окна 30 дней. +- Заказ ровно на порогах: 1.00, 100.00, окно даты = сегодня / вчера / 31 день назад. +- Применение капа пользователя (большая сумма заказов) и фиксация warning. +- Заказ в выходные (должен добавить weekend-бонус). ## Done criteria для PR -- Контракт понятен и зафиксирован (хотя бы в описании PR/тестах). -- Тесты описывают поведение (happy path + хотя бы 1–2 границы). -- Решение о merge основано на рисках, а не на стиле. - +- Контракт и правила зафиксированы (тесты/описание). +- Тесты покрывают happy path + ошибки/границы (окно дат, кап, минимальная сумма, пустой userId, выходные). +- CI зелёный/понятен; ошибки интерпретируемы.