Comments 53
Func<Type, object, bool>
— ужасный тип для сервиса. Надо либо напрямую обращаться к DbContext в атрибуте — либо заводить именованный делегат или интерфейс для сервиса.
Кстати, я бы в таких случаях использовал не столько валидатор, сколько биндер. Потому что раз уж все равно обращения к базе не избежать — почему бы не вытащить из нее сразу всю сущность и не поместить в модель?
Как-то так в идеале должно выглядеть:
public class MoveProductParam
{
[ResolveEntity]
public Product Product {get; set; }
[ResolveEntity]
public Category Category {get; set; }
}
Func<Type, object, bool> — ужасный тип для сервиса. Надо либо напрямую обращаться к DbContext в атрибуте — либо заводить именованный делегат или интерфейс для сервиса.
Не факт, что вы не захотите делать это через Dapper или еще как-то. Завист от проекта.
Func<Type, object, bool>
— пример. Какую зависимость использовать — вам решать.Байндер такой у меня тоже завалялся. Он сильно старый с reflection'ом и его еще допилить нужно. Только в виде идеи:
public class EntityModelBinder : DefaultModelBinder
{
public override object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
{
var model = base.BindModel(controllerContext, bindingContext);
if (model == null || !(model is EntityBase)) return null;
var props = ((EntityBase) model).GetDependentProperties();
var dbContext = DependencyResolver.Current.GetService<DbContext>();
foreach (var propertyInfo in props)
{
var id = controllerContext.HttpContext.Request[propertyInfo.Name];
if (string.IsNullOrEmpty(id)) continue;
var fkValue = dbContext.Set(propertyInfo.PropertyType).Find(int.Parse(id));
if (fkValue == null) continue;
propertyInfo.SetValue(model, fkValue, null);
if (bindingContext.ModelState[propertyInfo.Name].Errors.Any())
{
bindingContext.ModelState[propertyInfo.Name].Errors.Clear();
}
}
return model;
}
}
Сервис может понадобиться если я захочу иметь возможность свободно переключаться между Dapper и EF… но, исходя из моего опыта, это очень глупая идея: подобные общие интерфейсы обычно наследуют плохие стороны обоих решений.
Что же до вашего EntityModelBinder — он делает слишком многое. Я имел в виде простой поиск сущности по Id. Точно то же самое что вы делаете в валидаторе — только вместо Any использовать First.
О вот этой:
Конфликт инструкции INSERT с ограничением FOREIGN KEY "FK_Vehicle_Request". Конфликт произошел в базе данных "RDSv9", таблица "dbo.Request", column 'RequestID'.
Так вот: пользователь такого сообщения видеть не должен.
В статье ведь описана ситуация проверки связки данных двух таблиц через третью? Если данные (как у автора) уже есть в БД, то FOREIGN KEY будут автоматически обновлять/удалять зависимости и все описанные проверки в принципе не требуются.
Что вы скажете пользователю?
Лучше расскажите как вы объясните пользователю, что у вас Product в БД привязаны к несуществующим Category, а Category содержат несуществующие Product?
Валидация всеми нормальными программистами всегда делается для входных данных, а не для хранимых.
Конкретно же тот механизм, который тут обсуждается — это встроенное решение по валидации входных моделей в ASP.NET MVC.
Ну и, наконец, вы могли бы посмотреть в код. Там, вообще-то, валидируемая структура является параметром действия контроллера. Надеюсь, вы в курсе что эти параметры отражают данные из HTTP запроса?
Не стоит подменять контекст статьи собственными размышлениями на тему того, о чем Я не писал.
Но где вы увидели в коде валидацию данных, загруженных из базы?
if(!dbContext.Products.Any(x => x.Id == par.ProductId))
return BadRequest("Product not found");
if(!dbContext.Categories.Any(x => x.Id == par.CategoryId ))
return BadRequest("Category not found");
Лол, нет. Это валидация свойств par.ProductId
и par.CategoryId
, которые являются входными данными.
В процессе валидации используется запрос к базе.
А что если приложение должно уметь работать вообще без бд?
Тогда надо сменить реализацию на что-то, что не использует БД.
Нет смысла пытаться писать код который может использовать любую ORM — потому что такой код сможет работать лишь с общим подмножеством всех OPM, что лишает затею со сменой ORM какого бы то ни было смысла. Аналогично и с БД.
public class MoveProductParam
{
[EntityId(typeof(IProductRepository))]
public ProductId {get; set; }
[EntityId(typeof(ICategoryRepository))]
public CategoryId {get; set; }
}
Дополнительный слой бизнес-логики нельзя вводить просто так, для него должна набраться некоторая крит. масса функциональности, нуждающейся в изоляции. Или крит. масса одинакового кода.
В противном случае вы заменяете ТолстымУродливыйКонтроллер на ТолстуюУродливуюБизнесЛогику.
Зачем вообще нужен контроллер из одной строчки?
Вы практически за бесплатно закладываете фундамент для масштабирования в будущем. Достаточно легко можно будет перенести денормализированные данные в хранилище, оптимизированное под чтение и внедрить CQRS.
Нет, это происходит не за бесплатно. Повышается время вхождения в код, связи между классами труднее удерживать в голове. Сам код распухает: то что было одним классом, стало десятью.
А масштабирования в будущем может в итоге так и не случиться.
Можно покрыть бизнес-логику юнит тестами, что гораздо проще и быстрей чем тесты через хттп запросы.
Кстати, контроллер тоже при желании можно покрыть тестами.
Вы прыгаете с UI сразу в Persistence слой, минуя бизнес логику.
Ну и что? Какая бизнес-логика в наличие или отсутствии в БД записи? Строго говоря, откуда взялись эти id? Либо у нас ошибка и мы передаем в UI неактуальные данные, либо запрос сфабриковали. Pro100Oleh ниже приводит гораздо более убедительные аргументы и указывает на потенциальные проблемы. Кроме транзакционности добавляются потенциальные лишние запроы к БД. Вот эти проблемы действительно стоят внимания.
Вы практически за бесплатно закладываете фундамент для масштабирования в будущем. Достаточно легко можно будет перенести денормализированные данные в хранилище, оптимизированное под чтение и внедрить CQRS.
Это не так. Если у вас синхронный CRUD, но вы вместо контроллера написали запросы в квейри / командах ничего не изменилось, просто называется иначе. Тот CQRS, что асинхронный обычно требует принципиально иного подхода к проектированию системы. Грег Янг вообще не рекомендует использовать CQRS как top level architecture.
Тут смешаны слои. Слой UI, слой бизнес-логики и слой хранения данных. На уровне контроллера нельз провалидировать ничего, кроме формата запроса. Наличие всех обязательных полей, соответствие типам и форматам (положительные целые/UUID-строки для идентификаторов, валидный email, etc).
Валидация наличия категорий и товаров — это правило бизнес-логики и место ему в бизнес-слое.
Здесь вы правы. Но если кроме валидации изменить модел-байндинг, то все получается логично: раз в контроллер приходят не DTO, а сразу Entity, то и валидацию следует произвести там же. Конкретно в данном случае я вижу опасность не в перекрытии слоев, а в не стандартности решения. Ну и ошибки придется искать в двух местах.
Биндинг моделей к параметрам контролера прибивает вас гвоздями к фреймворку
Не прибивает.
Func<Type, object, bool>
и Func<Type, object, object>
соответствуют следующему php-коду:function(Type $type, object obj): bool {
//...
}
function(Type $type, object obj): obj {
//...
}
Вместо них можно инжектировать сервисы / репозитории / что угодно. Для примера я использовал просто делегат (функцию). Кстати,
dbContext
уже реализует репозиторий, поэтому на практике эта абстракция в .NET смысловой нагрузки не несет: IQueryable
— протекающая абстракция by design, потому что все выражения может разобрать только in-memory реализация, которая убивает весь смысл интерфейса.Биндинг моделей к параметрам контролера прибивает вас гвоздями к фреймворку
Не прибивает. В популярных ORM на .NET достаточно давно используются POCO. Т.е. — сущности — это просто классы. Они не зависят от фреймворка.
зы. Ну и от dbContext коробит. А что если приложение должно уметь работать вообще без бд? Или использовать редис как хранилище? Получается мы снова жестко прибиты к EF.
Обратите внимание,
dbContext
не инжектится в атрибут. Я работаю с делегатом. Конкретная релизация настривается в IOC-контейнере.Я о байндинге параметров контролера, когда идентификаторы в хттп-запросе автоматически превращаются в сущности из слоя хранения данных.
Так превращайте их автоматически в сущности доменной модели или в сущности слоя бизнес-логики.
И, тут я не уверен, этот байндинг часть фреймворка или часть языка? Если часть фреймворка, то не будет работать с другим без перепиливания.
Если вы пишите приложение так, что в любой момент можете сменить фреймворк или вовсе обойтись без оного — то зачем вам вообще фреймворк?
Глупо не использовать возможности фреймворка только потому что вы боитесь использовать возможности выбранного вами же фреймворка.
Тоесть вы пишите без тестов?
Разумеется, я не пишу тесты на несуществующие классы. И еще не пишу тесты на тривиальный код.
Но в моем случае валидация типов\форматов будет в конструкторе команды, а валидация бизнес-логики в хендлере этой команды, а не скопом в контроллере.
То есть получить все ошибки пользователь сможет только со второй попытки? Пока не исправит формат данных — ошибки в логике показаны не будут?
GET: /category/5
и такой категории нет принято возвращать 404 или 410. А вот если POST: { categoryId: 5, someData: ...}
я бы возвращал 422, чтобы четко показать: такой метод существует, но шлешь ты мне хрень. Семантика валидации БЛ одинаковая: нет такой сущности. А вот код ответа — разный. Если вы возвращаете результат валидации из бизнес-слоя, скажем
NotFoundResult
, дополнительно обрабатываете в контроллере, зная контекст запроса?UserNotFound, ProductNotFound, OrderCantBePaid, ProductOutOfStock
— это типы исключений или возвращаемых из CommandHandler
значений?Правильно заметили выше — намешаны слои. Если уж так хочеться — делайте авто-валидацию или модел-байдинг по ид из БД уже в бизнес слое не завязываясь на веб АПИ. Тут тоже можно прикрутить свой велосипед через рефлекшн и атрибуты.
Стоит заметить что ваше решение плохо работает когда надо прикрутить транзакции. А имя вашего класса MoveProductParam как бы подразумевает что транзакция бы не помешала.
Вы имеете в виду случай, когда валидация проходит, а за оставшиеся 100мс кто-то успевает удалить продукт / категорию с таким id? Если да, то эта тема отдельной большой статьи.
Правильно заметили выше — намешаны слои. Если уж так хочеться — делайте авто-валидацию или модел-байдинг по ид из БД уже в бизнес слое не завязываясь на веб АПИ. Тут тоже можно прикрутить свой велосипед через рефлекшн и атрибуты.
Да прикрутил уже. Бывают простые случаи, когда нужно сделать много CRUD'а и не хочится делать DTO ради двух-трех полей, которые мапятся на foreign-ключи. Хотя, может их проще сразу гнать на Update в базу через Dapper. В любом случае, вариант с атрибутом скорее экспериментальный и не пертендует на истину в последней инстанции.
Еще немного о валидации в ASP.NET