Ревью: прикладной код

Условие

Ревью: прикладной код

Класс содержит логику обработки новых заказов. Извлекает из БД заказы и позиции, после чего обращается по http к внешнему микросервису Stock, в котором бронирует товары по позициям заказа. Если все бронирования выполнены успешно, микросервис подтверждает бронь заказа в Stock и отмечает заказ обработанным. Микросервис запущен в несколько экземпляров, поэтому метод Process может выполняться параллельно. Для того, чтобы не обрабатывать одни и те же строки используется механизм пессимистичной блокировки PostgreSQL Select for update ... skip locked. Реализация IBus отправляет сообщения в RabbitMq.

public sealed class PendingOrdersProcessor
{
    public async Task PendingOrdersProcessor(ILogger logger, IBus bus, HttpStockService st
    {
        var env = Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT");
        var cs = _cs[env];

        _db = new OrdersRepository(cs);
        _bus = bus;
        logger = logger;

    _stockService = stockService;
}

private readonly ILogger _logger;
private readonly IOrdersRepository _db;
private readonly IStockService _stockService;
private readonly IBus _bus;

private static readonly Dictionary<string, string> _cs = new {
    { "Development",
      "User ID=root;Password=myPassword;Host=localhost;Port=5432;Database=myDataBase"
    },
    { "Production", "User ID=root;Password=myPassword;Host=prod-db;Port=5432;Database=
      "Pooling=true;Min Pool Size=0;Max Pool Size=1;" }
};

Ответ

В представленном коде есть несколько проблем и возможностей для улучшения:

1. Инициализация зависимостей в конструкторе

   • В коде конструктор PendingOrdersProcessor неявно инициализирует репозиторий и другие зависимости, используя переменную окружения. Лучше передавать все зависимости через параметры конструктора, чтобы повысить тестируемость и гибкость.

2. Потокобезопасность

   • Поскольку метод Process может выполняться параллельно в нескольких экземплярах, важно убедиться, что доступ к общим ресурсам (например, к базе данных) корректно синхронизирован.
   • Использование SELECT FOR UPDATE SKIP LOCKED — хорошая практика для пессимистичной блокировки и предотвращения обработки одних и тех же заказов.

3. Обработка ошибок и транзакции

   • Важно оборачивать операции с базой и вызовы внешнего сервиса в транзакции, чтобы обеспечить атомарность.
   • Если бронирование товаров не удалось, нужно корректно откатить изменения и не подтверждать заказ.

4. Логирование

   • Логирование ошибок и важных событий поможет в диагностике и мониторинге.

5. Конфигурация

   • Хранение строк подключения в словаре _cs с ключом окружения — неплохой подход, но стоит вынести конфигурацию в отдельный конфиг-файл или использовать менеджер конфигураций.

6. Пример улучшенного конструктора:

public sealed class PendingOrdersProcessor
{
    private readonly ILogger _logger;
    private readonly IOrdersRepository _db;
    private readonly IStockService _stockService;
    private readonly IBus _bus;

    public PendingOrdersProcessor(ILogger logger, IBus bus, IStockService stockService, IOrdersRepository ordersRepository)
    {
        _logger = logger ?? throw new ArgumentNullException(nameof(logger));
        _bus = bus ?? throw new ArgumentNullException(nameof(bus));
        _stockService = stockService ?? throw new ArgumentNullException(nameof(stockService));
        _db = ordersRepository ?? throw new ArgumentNullException(nameof(ordersRepository));
    }

    // Остальная логика обработки заказов
}

Таким образом, класс становится более модульным и удобным для тестирования.

Также стоит проверить, что пул подключений к БД настроен корректно (в примере для Production max pool size=1 — это может стать узким местом при параллельной обработке).