Код-ревью Middle EF Core обновлено
Код-ревью: эндпоинт заказов на EF Core
Вопрос
Эндпоинт возвращает последние заказы клиента с позициями. Найдите проблемы с производительностью, корректностью и безопасностью.
app.MapGet("/customers/{id}/orders", async (string id, AppDbContext db) =>
{
var orders = db.Orders.ToList()
.Where(o => o.CustomerId.ToString() == id)
.OrderByDescending(o => o.CreatedAt);
var result = new List<OrderDto>();
foreach (var o in orders)
{
var items = db.OrderItems.Where(i => i.OrderId == o.Id).ToList();
result.Add(new OrderDto(o.Id, o.Total, items.Count));
}
var raw = db.Orders.FromSqlRaw($"SELECT * FROM orders WHERE note LIKE '%{id}%'").ToList();
return Results.Ok(new { result, raw });
});
Коротко
Главное в этом коде: работа с данными идёт в памяти, а не в базе. Ранний ToList() тянет всю таблицу, цикл с запросом позиций даёт N+1, а синхронные вызовы занимают потоки.
Что должен найти ревьюер
- Производительностьстрока 3
ToList()доWhereзагружает все заказы всех клиентов в память, фильтр и сортировка идут в приложении. Фильтр должен уйти в SQL до материализации. - Производительностьстрока 4Сравнение через
ToString()не переведётся в SQL с индексом. Параметр надо типизировать (int idилиGuid id) и сравнивать ключи напрямую. - Производительностьстрока 10Запрос позиций в цикле: классическая проблема N+1. Нужен один запрос с проекцией
Select(o => new OrderDto(o.Id, o.Total, o.Items.Count)). - Производительностьстрока 3Чтение без изменений, но запросы отслеживаемые: лишние снимки в трекере. Для чтения
AsNoTracking()или проекция в DTO. - Ошибкастрока 5Нет ограничения выборки (
Take), хотя нужны «последние» заказы: у старых клиентов ответ будет огромным. - Безопасностьстрока 14
FromSqlRawс интерполяцией строки: пользовательскийidпопадает прямо в SQL, это SQL-инъекция. НуженFromSql($"...")с параметром или LINQEF.Functions.Like. - Производительностьстрока 14Синхронные
ToList()в async-обработчике блокируют поток пула: нужныToListAsync()сCancellationToken. - Безопасностьстрока 15Наружу отдаются сущности целиком (
raw), включая служебные поля, а проверки, что клиент запрашивает свои заказы, нет.
Разбор
Главное в этом коде: работа с данными идёт в памяти, а не в базе. Ранний ToList() тянет всю таблицу, цикл с запросом позиций даёт N+1, а синхронные вызовы занимают потоки. Правильная версия одним запросом:
var result = await db.Orders.AsNoTracking()
.Where(o => o.CustomerId == id)
.OrderByDescending(o => o.CreatedAt)
.Take(50)
.Select(o => new OrderDto(o.Id, o.Total, o.Items.Count))
.ToListAsync(ct);
Отдельно критична инъекция через FromSqlRaw с интерполированной строкой и отсутствие авторизации на чужие заказы.
Ответьте сами, а не прочитайте
В тренажёре вопрос попадёт в повторения и вернётся, когда начнёт забываться. Плюс билеты с зачётом: 20 вопросов, не больше 2 ошибок.