marshinov 9 янв 2018 в 15:15

Избавляемся от boilerplate для валидации в ASP.NET MVC

2 мин

11K

.NET*C#*Веб-разработка*

+14

Комментарии 39

RomanPokrovskij 9 янв 2018 в 20:05

Скажу крамольное: весь ASP Model Binding — тупик (да и потоковый json сериалайзер тоже). Аргументирую: binding property list задается только статически (там есть вроде как теоретический путь создать его и динамически но мой вывод 1) только поверх системного кеширования, 2) массивным переписыванием кода, в общем, будем считать что нет методы, тем более что ее действительно нет как реализации кем-то раз написаной). Последствия: хочешь создавать контроллер из модели не через T4 а через run time генерирование (с исп. Execution trees) — вынужден отказываться от Model Binding'ов, брать HttpRequest и по нему сгенерированной функцией строить класс.

Это работает. Показываешь. Фидбек же будет такой — а как же мои атрибутики модел биндинга?

Есть и правильные вопросы, но из-за атрибутов Asp это такой box — что когда надо out of the box дверь находят немногие. В общем я против атрибутов, слишком часто их выставляют каким-то сверхзнанием, слишком многие люди воспринимают фокус за достижение. А «на самом деле» (простите) атрибут это наведение тумана.

Замену

 if (!ModelState.IsValid)

 return View(param);

этим

[ValidationFilter]

не считаю достижением, а считаю шагом в сторону. Шагом вперед считал бы поиск регулярного в коде и составление всего метдоа Action динамически (при первом вызове) чему атрибуты не просто мешают — делают не возможным.

unsafePtr 9 янв 2018 в 23:38

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

public IActionResult Post(Model model)
{
    ThrowIfModelIsInvalid(model);

    return View();
}

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

marshinov 9 янв 2018 в 23:55

У вас есть 3 контроллера. У первого методы get и post, у второго — get и put. У третьего — put и post. Все методы требуют валидации. Что будете в базовый класс выносить? ThrowIfModelIsInvalid? Это boilerplate, придётся везде его писать. Кроме этого, ошибка валидации — не исключительная ситуация. Нужно возвращать результат, а не кидаться эксепшнами.

unsafePtr 10 янв 2018 в 00:38

Можно возвращать HttpMessageException со статус кодом и описанием. Сосбтвенно ради чего этот тип исключения и был создан. У нас принято все 4xx ошибки возвращать через исключение. Тут зависит от похода, и как мы относимся к ошибкам в бизнес-логике. Я лишь привел пример, метод для валидации можете назвать по другому и реализовать по своему
Скажите, а почему бы не вынести это в базовый контроллер? Одна строчка аттрибута или одна строчка в начале метода? В обоих случая если забудете написать, то валидация работать не будет.

posledam 10 янв 2018 в 09:36

Как уже сказали, и сказали правильно, логическая ошибка не является исключительной ситуацией. Специальное исключение введено как раз для исключительных ситуаций, для случаев, когда невозможно вернуть результат, это может быть и неправильно спроектированный класс и множество других ситуаций. Но пользоваться исключениями для абсолютно нормального процесса проверки ввода пользователя — это big mistake, и недопустимо в общем случае. Нельзя так делать.

posledam 10 янв 2018 в 09:45

В обоих случая если забудете написать, то валидация работать не будет.

Атрибут можно бросить на весь контроллер, на базовый контроллер или даже зарегистрировать глобально. Также атрибут можно навесить динамически по соглашениям и проверять различные условия. Всё это выглядит правильно и намного выгоднее, чем ThrowIfModelIsInvalid(model) — это крайне кривой и отвратительный костыль, тем более он бросает исключения, что противоречит всем best practics и рекомендациям. Обычно исключения, это то, что сломалось и это надо чинить. Тут же обычная рядовая валидация.

AxisPod 10 янв 2018 в 10:06

Видимо ваш ментор не знает для чего используются исключения. Даже в MSDN указано «Represents errors that occur during application execution.», а каким образом неправильно введённые данные являются ошибкой исполнения приложения?

focuspocus 10 янв 2018 в 18:13

AxisPod hVostt unsafePtr использование исключений — это одна из многих холиварных тем. например, никого не смущают ArgumentNullException или InvalidArgumentException и тп. исключения — это инструмент по управлению поведением приложения отличного от валидного/основного. если брать rest, при тех же ошибках в запросе обычно возвращается не 200 и {IsError=true}, а 4хх. исключения удобны, когда разные проверки скрыты в слоях сервисов и вызовов, чтобы каждый метод не анализировал результат выполнения вложенного, если и так понятно, что продолжать дальше нельзя. бросил свой тип исключения, в обработчике, например, глобальном поймал, залогировал и вернул соответствующий response клиенту. все ~~ходы записаны~~ ошибки анализируются и логируются в одном месте довольно компактным кодом, откуда бы они не прилетели; отдаются в том формате, в котором клиент это поймет. лепота. и, касательно валидации входных парамеров, ActionFilter выглядит лучше, чем ThrowIfModelIsInvalid. имхо.

AxisPod 19 янв 2018 в 18:06

ArgumentNullException и InvalidArgumentException используются в тех случаях, если метод действительно не ожидает null или какие-то левые данные. К примеру метод открытия файла явно требует непустую строку и при этом валидный путь. И тут подразумевается ещё ваша дополнительная логика по проверке данных пользователя и вывода ему адекватного сообщения о ошибке. А вот если вы накосячили в коде или вообще напрямую кинули данные введённые пользователями, вам метод открытия файла и кинет исключение.

Я не вижу здесь пространства для холивара, вообще.

shai_hulud 9 янв 2018 в 20:25

binding property list задается только статически (там есть вроде как теоретический путь создать его и динамически но мой вывод 1) только поверх системного кеширования, 2) массивным переписыванием кода, в общем, будем считать что нет методы, тем более что ее действительно нет как реализации кем-то раз написаной). Последствия: хочешь создавать контроллер из модели не через T4 а через run time генерирование (с исп. Execution trees) — вынужден отказываться от Model Binding'ов, брать HttpRequest и по нему сгенерированной функцией строить класс.

А можно подробнее? В вашем комментарии есть несколько ключевых слов «t4, Execution trees» которые вызывают интерес. Но суть я не уловил. По этому вопросы:
> binding property list
Что это?

>хочешь создавать контроллер из модели не через T4 а через run time генерирование
почему не реализовать свой controller selector и свои контроллеры (со своими динамическими экшонами итд) по динамической модели данных?

RomanPokrovskij 9 янв 2018 в 21:59

binding property list: msdn.microsoft.com/en-us/library/system.web.mvc.bindattribute(v=vs.118).aspx это который [Bind(«Prop1,Prop2»)]

почему не реализовать свой controller selector и свои контроллеры (со своими динамическими экшонами итд) по динамической модели данных?

На сколько я понимаю IHttpControllerSelector в Core нет, есть другое. Но вообще мне (пока) не нужна его фунциональность. Я готов сжится с таким способом (на большие перестройки не хватает времени).

public class DashboardLine: Controller{
    public static meta = new Meta<DashboardLine>(....); 
    private static Task<IActionResult> Edit();
    private static Task<IActionResult> EditConfirmed();
    public DashboardLine(){
           Edit=meta.GenerateEdit(this);
           EditConfirmed=meta.GenerateEditСonfirmed(this);
   }
   
        public async Task<IActionResult> Edit()
        {
            return await Edit();
        }

        [HttpPost, ActionName(nameof(Edit)), ValidateAntiForgeryToken]
        public async Task<IActionResult> EditFormData()
        {
            return await EditConfirmed();
        }
}

Тут есть аттрибуты роутинга, пусть остаются. Но уже нет model binding. К этому есть два умных вопроса 1) как такой контроллер тестировать 2) как тут обратится к ASP IoC. Вопросы правильные на них есть ответы но я хочу просто показать что типичный T4 контроллер может быть заменен и далее развит подобным способом.

unsafePtr 9 янв 2018 в 23:54

Возможно вам await в реализации метода не нужен.

Rambalac 10 янв 2018 в 03:38

Вы свою собственную ссылку не читали?

RomanPokrovskij 10 янв 2018 в 12:36

Спасибо, я тоже так думаю. Правда это означает убрать и async у контроллера. Но по скольку я плохо представляю как там в ASP TaskSchedule устроен, вдруг он смотрит что Task создан именно что в контроллере и делает что-то специфическое то вот так взять и отказаться от async у контроллера без тестирования боязно напороться на какую-нибудь дырявую абстракцию.

Прочитаю статью — выношу решение.

RomanPokrovskij 10 янв 2018 в 12:11

Как жаль что на хабре нельзя изменять комментарии…

Поправляю.

public class DashboardLine: Controller{
    public static meta = new Meta<DashboardLine>(..здесь можно обратится к EF модели..); 
    private Func<Task<IActionResult>> Edit;
    private Func<Task<IActionResult>> EditConfirmed;
    public DashboardLine(){
           Edit=meta.GenerateEdit(this);
           EditConfirmed=meta.GenerateEditСonfirmed(this);
   }
   
        public async Task<IActionResult> Edit()
        {
            return await Edit();
        }

        [HttpPost, ActionName(nameof(Edit)), ValidateAntiForgeryToken]
        public async Task<IActionResult> EditFormData()
        {
            return await EditConfirmed();
        }
}

Те куски кода которые нужно закешировать и потом использовать уже скомпилированными компилирются при инициализации static meta.

shai_hulud 10 янв 2018 в 12:23

Ну, концепцию я понял. В моем MVC 2-3(я не на коре) можно сделать полностью динамический контроллер со своей диспечеризацией. Либо сделать Route со своим Handler'ом и там уже и контроллера нет.

btw. там реально async/await не нужен, можно прямо вернуть Task.

RomanPokrovskij 10 янв 2018 в 13:10

Спасибо за совет. Динамический роутинг хорошо — но руки не доходят. Как и проверить можно ли снять async await у контроллера (боюсь дырявых абстракций у ASP TaskSchedule) завидую тем кто не боится (потому что понимает глубже).

shai_hulud 10 янв 2018 в 13:37

А тут собственно дело обстоит так.
До return await Edit(); будет поток исполнения ASP.NET MVC (шедуллер от типа хостинга зависит).
После await будет тот поток исполнения который предоставит Awaitable объект. В случае Task это будет тот же поток в котором завершилась работа, либо ThreadPool в случае TASK_STATE_THREAD_WAS_ABORTED или Thread.CurrentThread.ThreadState == ThreadState.AbortRequested или TaskCreationOptions.RunContinuationsAsynchronously.

Если заменить это на просто return Edit() для вызывающего метода ничего не изменится (т.е. поток/контекст исполнения для него будет хаотичным и неизвестным). Скорее всего внутри он сам чекает на нужный контекст исполнения и переключается на ожидаемый.

marshinov 10 янв 2018 в 14:17

А если выставить ConfigureAwait = false ASP.NET будет всегде брать из ThreadPool?

shai_hulud 10 янв 2018 в 15:45

Да я вверху ошибся, забыл что он по дефолту захватывает контекст.
То что я написал вверху относится к .ConfigureAwait(false).
С его отсутствием или .ConfigureAwait(true) будет шедулинг в текущем SynchronizationContext либо в текущем TaskScheduler либо в дефолтном TaskScheduler(ThreadPool).

Получается 'await Edit();' возвращает исполнение в тот контекст в которм был до await. Вот тут я не уверен, но ASP.NET MVC на это пофиг.

RomanPokrovskij 10 янв 2018 в 17:21

Core: blog.stephencleary.com/2017/03/aspnetcore-synchronization-context.html
Прочитав стало яснее, вероятности какого-нибудь супер хитрого поведения TaskSchedule теперь не опасаюсь.

marshinov 10 янв 2018 в 12:36

Не совсем понятно, что там за проблема с T4. Если не хочется писать контроллеры, то можно переопределить фабрику и использовать HandleUnknownAction. Чем этот вариант не подходит?

RomanPokrovskij 10 янв 2018 в 12:52

Нет никакой проблемы с T4 — тоже метод, (кстати не понятно зачем и в T4 шаблон вставлять атрибут вместо вызрва прямого кода, все же генерируется).

Однако у генерирования ран тайм и T4 разные предназначения.
Ран тайм — можно улучшать поведение десятка однотипных контроллеров без труда.
T4 — одноразовая генерация (практика показывает только так), получили бойлерплейт остальные изменения только руками.

Есть и различия.
В ран-тайм нет возни со сгенерированными файами исходного кода, нет возни с файлами шаблонов, нет T4 — другого языка.
С T4 все быстрее, цель «сгенерировать один раз», не накладывает больших обязательств на гибкость, читаемость и конфигурироемость кода. Нет притензии на новый уровень абстракции (а у программиста есть травмирующий опыт от новых уровней абстракций).

posledam 10 янв 2018 в 09:42

Какой тип ответа возвращать, лучше всего определять по заголовкам. Ещё можно смотреть, не является ли запрос AJAX. Как-то не очень хорошо выглядит определение по имени неймспейса, хоть это и похоже на convention, всё же это костыль :)

marshinov 10 янв 2018 в 10:37

Заголовки — хороший вариант. К сожалению бывают противные клиенты, которые их не отправляют или отправляют не те. Я привёл пример с явным указанием, что возвращать — json или view как раз потому что не смог подобрать достаточно интуитивного и удобного соглашения. У кого-то может быть другая ситуация, поэтому упомянул и привёл пример в какую сторону можно подумать.

Teo_van_KOT 10 янв 2018 в 10:50

А что делать, когда в модели есть Select? Вот как быть, когда надо презаполнить справочник? Уже не первый раз вижу подобные решения, но не понимаю, как решить такой вопрос.

marshinov 10 янв 2018 в 12:38

Вы что-то такое имеете в виду?

public class Param
{
    public int SomeEntityId { get;set; }
    // нужно проверить, что сущность с таким id существует
}

focuspocus 10 янв 2018 в 18:56

Добавить в атрибут параметр — метод, заполняющий модель для вьюхи?

marshinov 10 янв 2018 в 19:05

После вашего комментария понял, что имел в виду Teo_van_KOT. Вопрос про <select>, тот что дропндаун, а не LINQ. Ваш ответ подходит, но потащит DependencyResolver.Current в тело атрибута. Есть риск организовать сильную связанность кода.

Видел еще такую альтернативу: можно <select>'ы грузить ajax'ом с фронта. В момент рендеринга View просто <input type=«hidden» class=«dropdown»> делать, а потом на фронте заменять.

marshinov 10 янв 2018 в 19:22

Поправка. Теперь есть ServiceFilter, просто пробрасываем через IOC зависимости.

Magic_B 10 янв 2018 в 14:59

Автор, конечно, молодец! НО! Не забывайте, что еще один экземпляр, в данном случае, — фильтр затормозит обработку запроса (поковыряйте исходники MVC). Я фильтры, вообще, не использую. Проще в Вашем случае, раз уж Вы пошли путем упрощения кода, сделать базовый контроллер, где определить метод (пример):

protected TResult IsModelStateIsValid<TModel, TResult>(Func<TModel, TResult> ifModelIsValidFunc)
where TResult : IActionResult

marshinov 10 янв 2018 в 15:21

Там все достаточно оптимизировано. В Core по крайней мере:

// Execute the filter factory to determine which static filters can be cached.
var filters = CreateUncachedFiltersCore(filterProviders, actionContext, allFilterItems);

// Cache the filter items based on the following criteria
// 1. Are created statically (ex: via filter attributes, added to global filter list etc.)
// 2. Are re-usable
for (var i = 0; i < staticFilterItems.Length; i++)
{
	var item = staticFilterItems[i];
	if (!item.IsReusable)
	{
		item.Filter = null;
	}
}

return new FilterFactoryResult(staticFilterItems, filters);

Если хочется перформанса тогда можно выкинуть MVC из пайплайна, заменить на Handler / Middleware. В MVC в любом случае будет осуществляться проверка фильтров. Можете поделиться профайлингом на сколько дополнительный фильтр замедляет выполнение?

Magic_B 10 янв 2018 в 15:23

Дело, даже не в оптимизации, а дополнительном коде (вызовы и экземпляры). Я для себя открыл AspRazorPages! MVC забыт! :)

Magic_B 10 янв 2018 в 15:37

Насчет, бенчмарка. Тут смысла мерить нету. Разница будет в десяток-другой вызовов и нескольких экземплярах. Но т.к. мы говорим о веб-приложении, то (моя позиция) необходимо минимизировать любые расходы… И когда вся совокупность будет учтена, получится разница в производительности. А, вообще, пустой запрос в ASP.NET MVC (Release режим) занимает 5 мс, тогда как в AspRazorPages или при обработке через Handler / Middleware, естественно < 1 мс. Таким образом, избавившись от MVC, мы сэкономим 5 мс на каждом запросе. Но, не будем, забывать, что если остальной код не оптимизирован, то и разговаривать не о чем...

marshinov 10 янв 2018 в 15:52

Я пишу:

Если хочется перформанса тогда можно выкинуть MVC из пайплайна, заменить на Handler / Middleware

Вы пишите:

AspRazorPages или при обработке через Handler / Middleware, естественно < 1 мс.

Здесь мы согласны.

Но т.к. мы говорим о веб-приложении, то (моя позиция) необходимо минимизировать любые расходы…

Здесь позволю не согласиться. Перформанс — действительно важный критерий качества ПО, но есть еще удобство сопровождения и стоимость разработки / поддержки. Возможно вы и бизнес по разному оцениваете стоимость этих 4мс. Кроме того кроме tfb нужно еще смотреть на throughput. По этой логике нужно срочно все переписать на Netty :) Я бы не был так категоричен в высказываниях.

Magic_B 10 янв 2018 в 16:02

В свое время смотрел эти бенчмарки, и Netty смотрел тоже! :) Вы не менее категорично восприняли. :)

но есть еще удобство сопровождения и стоимость разработки / поддержки

Тут, к сожалению, любые новые технологии или подходы проиграют… И я со своими наработками проиграю. Но и ладно! Когда-нибудь же эти технологии и патерны зайдут в продакшн.

RomanPokrovskij 10 янв 2018 в 18:00

Тут, к сожалению, любые новые технологии или подходы проиграют… Но и ладно!

Вот у меня «ладно» и не получается, поскольку хотя я и работаю один (фрилансер, а если бы приходилось через консервативных «архитектов», «сеньеров» и «тим лидов» пробиваться было бы еще нервней) то все равно выходить к людям с новыми идеями и подходами хочется и конструктива хочется, а тебе говорят «перехочется, ты проиграл, минус».

marshinov 10 янв 2018 в 18:02

Конструктив всегда на стороне бизнеса в данном случае: сколько разработчиков знает ASP.NET MVC? А сколько WebSharper? Кто будет развивать и поддерживать проект, когда ведущий поедет в отпуск?

RomanPokrovskij 10 янв 2018 в 18:40

Если считать за честную монету аппеляцию к бизнесу то безусловно новый подход должен делать неважным вопрос «сколько разработчиков» т.е. быть производительным на новом уровне (грубо: меньше кода в два раза, при ясной абстракции).

Но мой опыт говорит, что «бизнес патриотизм» прибежище черти знает кого. Да и бесконечно длинна очередь желающих доказать что они сильнее всех любят бизнес на словах. Люби бизнес на деле — будь фрилансером :)

Зарегистрируйтесь на Хабре, чтобы оставить комментарий

Избавляемся от boilerplate для валидации в ASP.NET MVC

Комментарии 39

Публикации

Истории