Круто!

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 — мне придется переписывать одинаковый объем кода независимо от того где я буду этот код держать — в валидаторе или в сервисе.

Сервис может понадобиться если я захочу иметь возможность свободно переключаться между Dapper и EF… но, исходя из моего опыта, это очень глупая идея: подобные общие интерфейсы обычно наследуют плохие стороны обоих решений.

Что же до вашего EntityModelBinder — он делает слишком многое. Я имел в виде простой поиск сущности по Id. Точно то же самое что вы делаете в валидаторе — только вместо Any использовать First.
UPD: значение лучше брать из ValueProvider, а не из Query
Возможно Я не понял, какую именно проблемы вы пытаетесь решить, но весь этот код не требуется если использовать FOREIGN KEY Constraints при проектировании структуры БД.
И пользователь получит SQL ошибку. Лучше вернуть человекоподобную ошибку что бы клиент знал что делать.
О какой SQL ошибке идёт речь? При правильной настройке FOREIGN KEY (с DELETE/UPDATE) описанная в статье ситуация в принципе не возможна, т.к. за гарантированным наличием/отсутствием данных в парах будет следить сама БД.
Каким образом она за этим будет следить? Разве не выкидывая ошибку в ответ на некорректные операции?
Ещё раз спрашиваю — о какой SQL ошибке идёт речь?

О вот этой:


Конфликт инструкции INSERT с ограничением FOREIGN KEY "FK_Vehicle_Request". Конфликт произошел в базе данных "RDSv9", таблица "dbo.Request", column 'RequestID'.

Так вот: пользователь такого сообщения видеть не должен.

Э-эм, а как она утечёт пользователю, если ваш код обязан проверять результат операции вставки данных в БД? Вы же наверняка про UPSERT + транзакции знаете :)

В статье ведь описана ситуация проверки связки данных двух таблиц через третью? Если данные (как у автора) уже есть в БД, то FOREIGN KEY будут автоматически обновлять/удалять зависимости и все описанные проверки в принципе не требуются.
Итак, ваш код проверил результат вставки данных в БД. Проверка показала что произошла ошибка.

Что вы скажете пользователю?
Очевидно то, что “данные не удалось сохранить”.

Лучше расскажите как вы объясните пользователю, что у вас Product в БД привязаны к несуществующим Category, а Category содержат несуществующие Product?
А откуда у меня в базе возьмутся несогласованные данные? Или вы думаете, что валидация — это замена внешним ключам, а не дополнение к ним?
Вот и мне интересно откуда у автора в БД взялись несогласованные данные, для которых требуется обязательная дополнительная валидация ключей “кочующая из одного файла в другой” :)
Э, постойте. Кто вам сказал что они у него в БД?

Валидация всеми нормальными программистами всегда делается для входных данных, а не для хранимых.

Конкретно же тот механизм, который тут обсуждается — это встроенное решение по валидации входных моделей в ASP.NET MVC.

Ну и, наконец, вы могли бы посмотреть в код. Там, вообще-то, валидируемая структура является параметром действия контроллера. Надеюсь, вы в курсе что эти параметры отражают данные из HTTP запроса?
Ага, а DbContext для красоты написан :)

Не стоит подменять контекст статьи собственными размышлениями на тему того, о чем Я не писал.
Ну разумеется. Чтобы проверить существование чего-то в базе — надо сделать запрос в базу.

Но где вы увидели в коде валидацию данных, загруженных из базы?
Простите, а вот это что? Разве не валидация данных, загруженных из базы?

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, которые являются входными данными.


В процессе валидации используется запрос к базе.

Вы сами себе противоречите :)

> Но где вы увидели в коде валидацию данных, загруженных из базы?

> В процессе валидации используется запрос к базе.
Я может чего-то не понимаю в этих ваших шарпах, но для меня этот подход попахивает нехорошим.

Тут смешаны слои. Слой UI, слой бизнес-логики и слой хранения данных. На уровне контроллера нельз провалидировать ничего, кроме формата запроса. Наличие всех обязательных полей, соответствие типам и форматам (положительные целые/UUID-строки для идентификаторов, валидный email, etc).
Валидация наличия категорий и товаров — это правило бизнес-логики и место ему в бизнес-слое.

Биндинг моделей к параметрам контролера прибивает вас гвоздями к фреймворку. Возможно в мире шарпа нет альтернатив, но на пыхе мне приходилось интегрировать магазины с разными фреймворками и даже с системами без фреймворков (а давайте добавим страничку, где будем продавать товары онлайн… на бложик, накиданный на коленке 10 лет назад, ага...). Если бизнес логика не завязана на конкретный фреймворк — это сделать несложно.

зы. Ну и от dbContext коробит. А что если приложение должно уметь работать вообще без бд? Или использовать редис как хранилище? Получается мы снова жестко прибиты к EF.
А что если приложение должно уметь работать вообще без бд?

Тогда надо сменить реализацию на что-то, что не использует БД.


Нет смысла пытаться писать код который может использовать любую ORM — потому что такой код сможет работать лишь с общим подмножеством всех OPM, что лишает затею со сменой ORM какого бы то ни было смысла. Аналогично и с БД.

Просто нужно писать бизнес-логику не завязанную ни на фреймворк, ни на орм.
Пример с хранилищем. В бизнес слое есть IProductRepository. В слое инфраструктуры лежит DbProductRepository и InMemoryProductRepository. Все сервисы в бизнес-слое завязаны на IProductRepository, а уж какую реализацию им передашь — дело десятое. Более того, можно всю бизнес-логику написать до того как вообще определишься с хранилищем, фреймворком и т.д.
Можно и так делать — это не помешает показанному здесь подходу к валидации. Получится как-то так в итоге:

public class MoveProductParam
{
   [EntityId(typeof(IProductRepository))]
   public ProductId {get; set; }

   [EntityId(typeof(ICategoryRepository))]
   public CategoryId {get; set; }
}
Не совсем. Проверка существования перед перемещением — это правило бизнес-логики и должно проверяться в доменном сервисе, а не в контроллере. Вы прыгаете с UI сразу в Persistence слой, минуя бизнес логику. Собственно последний шаг к ТолстымУродливымКонтроллерам — прямо здесь же в контролере написать что-то типа

product.Category.removeProduct(product);

category.addProduct(product);
Ну и что толстого и уродливого в двух строках кода?

Дополнительный слой бизнес-логики нельзя вводить просто так, для него должна набраться некоторая крит. масса функциональности, нуждающейся в изоляции. Или крит. масса одинакового кода.

В противном случае вы заменяете ТолстымУродливыйКонтроллер на ТолстуюУродливуюБизнесЛогику.
Зачем ждать критическую массу?
Реализовать хотя бы Command не так сложно.
Это сразу дает ряд преимуществ:
1. Можно покрыть бизнес-логику юнит тестами, что гораздо проще и быстрей чем тесты через хттп запросы.
2. Разработка становиться ориентированной на use cases. Обработка каждого кейса строго изолирована. Беглого взгляда на список команд достаточно, что бы понять на что способна система.
3. Вот тут можно применить байндинг реквеста на объект команды, в котором и будет зашита валидация. Что все идентификаторы и данные на месте, что они в правильном формате. А в контроллерах будет минимум логики:
commandHandler.execute(command);

4. Вы практически за бесплатно закладываете фундамент для масштабирования в будущем. Достаточно легко можно будет перенести денормализированные данные в хранилище, оптимизированное под чтение и внедрить CQRS.

Зачем вообще нужен контроллер из одной строчки?


Вы практически за бесплатно закладываете фундамент для масштабирования в будущем. Достаточно легко можно будет перенести денормализированные данные в хранилище, оптимизированное под чтение и внедрить CQRS.

Нет, это происходит не за бесплатно. Повышается время вхождения в код, связи между классами труднее удерживать в голове. Сам код распухает: то что было одним классом, стало десятью.


А масштабирования в будущем может в итоге так и не случиться.

Можно покрыть бизнес-логику юнит тестами, что гораздо проще и быстрей чем тесты через хттп запросы.

Кстати, контроллер тоже при желании можно покрыть тестами.

Вы прыгаете с UI сразу в Persistence слой, минуя бизнес логику.

Ну и что? Какая бизнес-логика в наличие или отсутствии в БД записи? Строго говоря, откуда взялись эти id? Либо у нас ошибка и мы передаем в UI неактуальные данные, либо запрос сфабриковали. Pro100Oleh ниже приводит гораздо более убедительные аргументы и указывает на потенциальные проблемы. Кроме транзакционности добавляются потенциальные лишние запроы к БД. Вот эти проблемы действительно стоят внимания.

Вы практически за бесплатно закладываете фундамент для масштабирования в будущем. Достаточно легко можно будет перенести денормализированные данные в хранилище, оптимизированное под чтение и внедрить CQRS.

Это не так. Если у вас синхронный CRUD, но вы вместо контроллера написали запросы в квейри / командах ничего не изменилось, просто называется иначе. Тот CQRS, что асинхронный обычно требует принципиально иного подхода к проектированию системы. Грег Янг вообще не рекомендует использовать CQRS как top level architecture.
1. CQRS не обязан быть асинхронным
2. Если вам не нужен CQRS — не используйте, но бизнес логике все равно место в бизнес-слое, а не в контроллерах

зы. Я не повторял аргументы о транзакциях вполне особенно, хотя да, стоило добавить ссылку на комментатора ниже.
Тут смешаны слои. Слой 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-контейнере.
Не прибивает. В популярных ORM на .NET достаточно давно используются POCO. Т.е. — сущности — это просто классы. Они не зависят от фреймворка.


Я о байндинге параметров контролера, когда идентификаторы в хттп-запросе автоматически превращаются в сущности из слоя хранения данных.

1. UI-слою незачем знать о хранении совсем.
2. И, тут я не уверен, этот байндинг часть фреймворка или часть языка? Если часть фреймворка, то не будет работать с другим без перепиливания.

Я о байндинге параметров контролера, когда идентификаторы в хттп-запросе автоматически превращаются в сущности из слоя хранения данных.

Так превращайте их автоматически в сущности доменной модели или в сущности слоя бизнес-логики.


И, тут я не уверен, этот байндинг часть фреймворка или часть языка? Если часть фреймворка, то не будет работать с другим без перепиливания.

Если вы пишите приложение так, что в любой момент можете сменить фреймворк или вовсе обойтись без оного — то зачем вам вообще фреймворк?


Глупо не использовать возможности фреймворка только потому что вы боитесь использовать возможности выбранного вами же фреймворка.

Я о том, что БЛ не должна зависеть от фреймворка или от ОРМ или любой другой либы.
Ну условно, завтра в секьюрити компоненте фреймворка найдут критическую уязвимость или упретесь в производительность EF и нужно будет переписать на чистый sql. Вам придется во всех местах, где опираетесь на сторонние классы — переписывать.
Потому что у вас зависимость от реализации, а не интерфейса.

Я не призываю отказаться от фреймворка или ОРМ. Я призываю соблюдать границы между слоями.
То есть чтобы вдруг не пришлось переписывать завтра — надо тратить столько же усилий сразу же? :)
Cколько «столько»? Кода даже по сути будет столько же, только размещаться будет на разных слоях.
Интерфейсы, классы, защитные проверки, тесты — все это тоже код, в вашем варианте его намного больше.
Тоесть вы пишите без тестов?
В моем случае для БЛ нужно писать юнит тесты, которые пишутся быстрей и проще, чем end-to-end тесты. Их может быть больше, но все они небольшие и очень быстро гонятся на CI. TDD становится не обузой (опять тесты писать), а инструментом. Хотите верьте, хотите нет, но 90% задач я делаю не запуская написанный код вручную, только гоняя тесты.

Интерфейсы нужны на границах слоев в основном. Да, они нужны для стратегий, визиторов и других сервисов, имеющих множественные имплементации, но там их можно выделить уже тогда, когда они реально станут нужны. И это будет рефакторинг в бизнес-логике, который никак не отразится на слое UI. Там по прежнему будет commandHandler.execute(command);

Валидацию вы и так делаете. Но в моем случае валидация типов\форматов будет в конструкторе команды, а валидация бизнес-логики в хендлере этой команды, а не скопом в контроллере.
Тоесть вы пишите без тестов?

Разумеется, я не пишу тесты на несуществующие классы. И еще не пишу тесты на тривиальный код.


Но в моем случае валидация типов\форматов будет в конструкторе команды, а валидация бизнес-логики в хендлере этой команды, а не скопом в контроллере.

То есть получить все ошибки пользователь сможет только со второй попытки? Пока не исправит формат данных — ошибки в логике показаны не будут?

Вам не кажется странным пытаться совершить бизнес действие с заведомо неверными входными данными?
Это валидация разного уровня, и ошибки разного типа.
В ответ на кривой запрос веб приложение должно отдать «Bad request», а при попытке совершить некорректное бизнес действие — «Conflict», «Not found» или другой подходящий ответ в зависимости от ситуации.
Кстати, интересная тема: коды ответов. Если мы запрашиваем GET: /category/5 и такой категории нет принято возвращать 404 или 410. А вот если POST: { categoryId: 5, someData: ...} я бы возвращал 422, чтобы четко показать: такой метод существует, но шлешь ты мне хрень.

Семантика валидации БЛ одинаковая: нет такой сущности. А вот код ответа — разный. Если вы возвращаете результат валидации из бизнес-слоя, скажем NotFoundResult, дополнительно обрабатываете в контроллере, зная контекст запроса?
Не совсем. У меня есть ряд общих ошибок: UserNotFound / ProductNotFound, которые летят из репы. OrderCantBePaid, ProductOutOfStock, которые могут летать из определенных сервисов. Но эти ошибки не вылетают в контроллер, потому что отсутсвие юзера может быть как 404, так и 422 в разных запросах. Так что все потенциальные ошибки ловятся хендлером команды и в случае возникновения заворачиваются в новую. Например при попытке поместить в корзину несуществующий товар или в корзину несуществующего юзера вылетит в контроллер AddToBasketError, которая в свою очередь уже сконвертится в код 422 и боди типа такого
{
    "message": "Product can't be added to basket.",
    "logref": "...",
    "_embedded": {
        "errors": [{
                "message": "User #123 doesn't exist.",
        }]
    }
}
UserNotFound, ProductNotFound, OrderCantBePaid, ProductOutOfStock — это типы исключений или возвращаемых из CommandHandler значений?
Это исключения. CommandHandler всегда возвращает успешный результат.
А вот эти аргументы можете прокомментировать? Считаете, что ничего страшного: exceptions for control flow?
Я не использую эксепшены для control flow.

Есть условно хепи-пас. И есть разные ситуации, когда действие совершить невозможно.
Простой пример ошибка при попытке купить товаров больше, чем доступно на складе. Бизнес-слой отреагирует эксепшеном, так как это недопустимая операция. Бизнес слой не знает словит кто его или не словит. Его задача либо оформить заказ на товары, либо выбросить ошибку.
А вот UI слой как раз и занимается тем, что формирует представление для юзера.
Да, теоретически можно было бы всегда возвращать FailedResult, но никто не гарантирует, что результат будет проверен.
Стоит заметить что ваше решение плохо работает когда надо прикрутить транзакции. А имя вашего класса MoveProductParam как бы подразумевает что транзакция бы не помешала. Можно бы усложнить велосипед и прикрутить транзакцию к веб контексту, но в голове уже возникает троллейбус из хлеба. Но что делать когда нужно несколько транзакций на один реквест?
Правильно заметили выше — намешаны слои. Если уж так хочеться — делайте авто-валидацию или модел-байдинг по ид из БД уже в бизнес слое не завязываясь на веб АПИ. Тут тоже можно прикрутить свой велосипед через рефлекшн и атрибуты.
Стоит заметить что ваше решение плохо работает когда надо прикрутить транзакции. А имя вашего класса MoveProductParam как бы подразумевает что транзакция бы не помешала.

Вы имеете в виду случай, когда валидация проходит, а за оставшиеся 100мс кто-то успевает удалить продукт / категорию с таким id? Если да, то эта тема отдельной большой статьи.

Правильно заметили выше — намешаны слои. Если уж так хочеться — делайте авто-валидацию или модел-байдинг по ид из БД уже в бизнес слое не завязываясь на веб АПИ. Тут тоже можно прикрутить свой велосипед через рефлекшн и атрибуты.

Да прикрутил уже. Бывают простые случаи, когда нужно сделать много CRUD'а и не хочится делать DTO ради двух-трех полей, которые мапятся на foreign-ключи. Хотя, может их проще сразу гнать на Update в базу через Dapper. В любом случае, вариант с атрибутом скорее экспериментальный и не пертендует на истину в последней инстанции.
А не лучше было бы просто перехватывать ошибку от БД и уже разбирая ее выводить какой-то юзер меседж, если уж так хочется. Хотя не понятно зачем. Юзер обычно в таких случаях должен получить какой-то дженерик меседж да и все.
Мне — не лучше. Предпочитаю возвращать осмысленные коды ответов. Возвращение осмысленных кодов имеет преимущества по usability, соответствию стандартам и простоте отладки.
Только полноправные пользователи могут оставлять комментарии.
Войдите, пожалуйста.