Code review сервиса подсчёта статистики по заказам клиента

32. Оптимизировать расчёт статистики по заказам клиентов

Условие задачи:
Дан Spring-сервис, который для активных пользователей рассчитывает статистику по заказам: количество заказов и их общую стоимость, после чего отправляет результат в Kafka.

Нужно провести code review: найти проблемы с Dependency Injection, работой с БД, транзакциями и производительностью, а затем предложить более эффективную реализацию, сохранив корректное поведение в том числе для пользователей без заказов.

Код:

@Component
@RequiredArgsConstructor
public class StatService {

    @Autowired
    private UserRepository userRepository;
    @Autowired
    private OrderRepository orderRepository;
    @Autowired
    private StatRepository statRepository;
    @Autowired
    private KafkaService kafkaService;

    public void calculate() {
        List<User> users = userRepository.findAll().stream().filter(u -> u.isActive()).collect(Collectors.toList());

        if (!users.isEmpty()) {
            for(User user : users) {
                List<Order> orders = orderRepository.findByUser(user.getId());

                if (orders.isEmpty()) {
                    System.out.println(""У пользователя нет заказов"");
                }

                int totalOrders = 0;
                int totalCost = 0;
                for(Order order : orders) {
                    totalOrders++;
                    totalCost += order.getCost();
                }

                save(user.getId(), totalOrders, totalCost);
            }
        }
    }
@Transactional
private void save(Long userId, int totalOrders, int totalCost) {
    kafkaService.send(userId, totalOrders, totalCost, Instant.now());
}

Спойлеры к решению

Подсказки

💡 Сейчас сначала загружаются все пользователи, а активные отбираются уже в Java.
💡 На каждого пользователя выполняется отдельный запрос заказов — проверь количество запросов при большом числе пользователей.
💡 @Transactional на private-методе, вызываемом из того же класса, не создаст транзакцию через Spring proxy.
💡 Обрати внимание на statRepository: используется ли он вообще?
💡 При оптимизации не потеряй активных пользователей, у которых нет заказов.

Решение

Проблемы:

смешаны @RequiredArgsConstructor и field injection через @Autowired;
поля зависимостей не final, поэтому @RequiredArgsConstructor здесь фактически бесполезен;
statRepository объявлен, но нигде не используется;
загружаются все пользователи, а фильтрация по active выполняется в памяти;
возникает N+1: после запроса пользователей выполняется отдельный запрос заказов для каждого пользователя;
все заказы загружаются в Java только ради COUNT и SUM;
@Transactional на private save() не сработает через Spring AOP;
более того, save() ничего не сохраняет в БД — он только отправляет сообщение в Kafka;
название save() не соответствует фактическому поведению;
System.out.println() лучше заменить логгером;
пустой if (!users.isEmpty()) не нужен — цикл и так ничего не выполнит для пустого списка;
totalCost в int может быть неподходящим для денежных сумм или больших агрегатов;
Instant.now() вызывается отдельно для каждого пользователя, хотя статистика может относиться к одному запуску расчёта;
в показанном фрагменте отсутствует закрывающая } класса StatService.

Как лучше исправить

Главная оптимизация — считать статистику сразу в БД, а не загружать пользователей и все их заказы по отдельности.

При этом важно сохранить текущее поведение: активный пользователь без заказов тоже получает статистику 0 / 0. Поэтому запрос должен использовать LEFT JOIN, а не обычный INNER JOIN.

Например, репозиторий может сразу вернуть проекцию:

public record UserOrderStat(
        Long userId,
        long totalOrders,
        long totalCost
) {
}

public interface StatRepository {

    List<UserOrderStat> calculateForActiveUsers();
}

Запрос по смыслу:

SELECT u.id,
       COUNT(o.id),
       COALESCE(SUM(o.cost), 0)
FROM users u
LEFT JOIN orders o ON o.user_id = u.id
WHERE u.active = true
GROUP BY u.id

Тогда сервис становится значительно проще:

@Service
@RequiredArgsConstructor
public class StatService {

    private final StatRepository statRepository;
    private final KafkaService kafkaService;

    public void calculate() {
        Instant calculatedAt = Instant.now();

        List<UserOrderStat> stats =
                statRepository.calculateForActiveUsers();

        for (UserOrderStat stat : stats) {
            kafkaService.send(
                    stat.userId(),
                    stat.totalOrders(),
                    stat.totalCost(),
                    calculatedAt
            );
        }
    }
}

Так вместо схемы:

1 запрос пользователей
+ N запросов заказов

можно получить статистику одним агрегирующим запросом.

@Transactional для простого вызова Kafka здесь не нужен. Если по требованиям статистику нужно одновременно сохранить в БД и отправить в Kafka, появляется другая задача: обычная DB-транзакция не делает эти две операции атомарными. Для надёжного сценария лучше сохранять статистику и outbox-запись в одной транзакции, а Kafka отправлять отдельным обработчиком.

Для стоимости стоит выбрать тип согласно модели денег: long подходит, если сумма хранится в минимальных денежных единицах, например копейках; для дробных денежных значений обычно используют BigDecimal.

Главное исправление — перенести фильтрацию и агрегацию в БД, избавиться от N+1 и не использовать фиктивную @Transactional вокруг отправки в Kafka.