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

    В большинстве примеров проверка входных данных ASP.NET MVC осуществляется следующим образом:

            [HttpPost]
            public IActionResult Test(SomeParam param)
            {
                if (!ModelState.IsValid)
                {
                    return View(param);
                    // return Json({success: false, state: ModelState});
                }
                
                dbContext.UpdateData(param);
    
                return RedirectToAction("index");
                // return Ok({success: true});
            }
    

    Этот код можно улучшить:

    1. вынести валидацию из тела метода и избавиться от дублирования if (!ModelState.IsValid)
    2. вернуть код ответа 422

    Вынесем валидацию в ActionFilter


    Авторизация в ASP.NET MVC настраивается с помощью атрибутов. Сделаем по аналогии и объявим атрибут для валидации:

    public enum ValidationResult
        {
            View,
            Json
        }
        
        public class ValidationFilterAttribute: ActionFilterAttribute
        {
            private readonly ValidationResult _result;
    
            public ValidationFilterAttribute(ValidationResult result
                = ValidationResult.Json)
            {
                _result = result;
            }
    
            public override void OnActionExecuting(ActionExecutingContext context)
            {
                if (!context.ModelState.IsValid)
                {
                    if (_result == ValidationResult.Json)
                    {
                        context.Result = new ValidationFailedResult(context.ModelState);
                    }
                    else
                    {
                        context.Result = ((Controller)context.Controller).View(
                            context.ActionArguments.Values.First());
                        ValidationFailedResult.SetStatusCodeAndHeaders(
                            context.HttpContext);
                    }
                }
            }
        }

    Добавим код ответа сервера и дополнительную информацию


    На stackoverflow обсуждался вопрос «какой код возвращать при ошибке валидации». Семейство 4** выглядит наиболее подходящим. 422 — уже используется Ruby из коробки. ASP.NET MVC не предлагает best practice на этот счет. Не вижу причин не привести в соответствие с Ruby:

        internal class ValidationFailedResult: JsonResult
        {
            public ValidationFailedResult(ModelStateDictionary modelState)
                : base(modelState.Select(x => new
                    {
                        x.Key,
                        ValidationState = x.Value.ValidationState.ToString(),
                        x.Value.Errors
                    }).ToList())        
            {
            }
    
            public override void ExecuteResult(ActionContext context)
            {
                base.ExecuteResult(context);
                SetStatusCodeAndHeaders(context.HttpContext);
            }
    
            internal static void SetStatusCodeAndHeaders(HttpContext context)
            {
                context.Response.StatusCode = 422;
                context.Response.Headers.Add("X-Status-Reason", "Validation failed");
            }
        }

    Используем атрибут


    ValidationFilterAttribute можно использовать на

    1. методе контроллера
    2. контроллере
    3. глобально

    Остается только разделить контроллеры, возвращающие View от Json. Этого можно добиться, создав два базовых класса или добавив соглашение в атрибут, например проверять наличие api в namespace контроллера.

    Примеры кода приведены для ASP.NET MVC Core. Для ASP.NET MVC придется создать два набора атрибутов для пространства имен Mvc и Http, соответственно.
    Поделиться публикацией
    Похожие публикации
    AdBlock похитил этот баннер, но баннеры не зубы — отрастут

    Подробнее
    Реклама
    Комментарии 39
    • –1
      Скажу крамольное: весь 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 динамически (при первом вызове) чему атрибуты не просто мешают — делают не возможным.
      • –2
        Ментор мой тоже не любит аттрибуты, и предпочитает выносить в базовый контроллер вот такие вот проверки. И уже в самом начале метода вызывать метод из базового класса.
        public IActionResult Post(Model model)
        {
            ThrowIfModelIsInvalid(model);
        
            return View();
        }
        

        Таким образом у нас метод становиться статическим, и не надо шаманить с аттрибутами.
        • +3
          У вас есть 3 контроллера. У первого методы get и post, у второго — get и put. У третьего — put и post. Все методы требуют валидации. Что будете в базовый класс выносить? ThrowIfModelIsInvalid? Это boilerplate, придётся везде его писать. Кроме этого, ошибка валидации — не исключительная ситуация. Нужно возвращать результат, а не кидаться эксепшнами.
          • 0
            Можно возвращать HttpMessageException со статус кодом и описанием. Сосбтвенно ради чего этот тип исключения и был создан. У нас принято все 4xx ошибки возвращать через исключение. Тут зависит от похода, и как мы относимся к ошибкам в бизнес-логике. Я лишь привел пример, метод для валидации можете назвать по другому и реализовать по своему
            Скажите, а почему бы не вынести это в базовый контроллер? Одна строчка аттрибута или одна строчка в начале метода? В обоих случая если забудете написать, то валидация работать не будет.
            • +3
              Как уже сказали, и сказали правильно, логическая ошибка не является исключительной ситуацией. Специальное исключение введено как раз для исключительных ситуаций, для случаев, когда невозможно вернуть результат, это может быть и неправильно спроектированный класс и множество других ситуаций. Но пользоваться исключениями для абсолютно нормального процесса проверки ввода пользователя — это big mistake, и недопустимо в общем случае. Нельзя так делать.
              • +3
                В обоих случая если забудете написать, то валидация работать не будет.


                Атрибут можно бросить на весь контроллер, на базовый контроллер или даже зарегистрировать глобально. Также атрибут можно навесить динамически по соглашениям и проверять различные условия. Всё это выглядит правильно и намного выгоднее, чем ThrowIfModelIsInvalid(model) — это крайне кривой и отвратительный костыль, тем более он бросает исключения, что противоречит всем best practics и рекомендациям. Обычно исключения, это то, что сломалось и это надо чинить. Тут же обычная рядовая валидация.
            • +2
              Видимо ваш ментор не знает для чего используются исключения. Даже в MSDN указано «Represents errors that occur during application execution.», а каким образом неправильно введённые данные являются ошибкой исполнения приложения?
              • –1
                AxisPod hVosttunsafePtr использование исключений — это одна из многих холиварных тем. например, никого не смущают ArgumentNullException или InvalidArgumentException и тп. исключения — это инструмент по управлению поведением приложения отличного от валидного/основного. если брать rest, при тех же ошибках в запросе обычно возвращается не 200 и {IsError=true}, а 4хх. исключения удобны, когда разные проверки скрыты в слоях сервисов и вызовов, чтобы каждый метод не анализировал результат выполнения вложенного, если и так понятно, что продолжать дальше нельзя. бросил свой тип исключения, в обработчике, например, глобальном поймал, залогировал и вернул соответствующий response клиенту. все ходы записаны ошибки анализируются и логируются в одном месте довольно компактным кодом, откуда бы они не прилетели; отдаются в том формате, в котором клиент это поймет. лепота. и, касательно валидации входных парамеров, ActionFilter выглядит лучше, чем ThrowIfModelIsInvalid. имхо.
                • 0
                  ArgumentNullException и InvalidArgumentException используются в тех случаях, если метод действительно не ожидает null или какие-то левые данные. К примеру метод открытия файла явно требует непустую строку и при этом валидный путь. И тут подразумевается ещё ваша дополнительная логика по проверке данных пользователя и вывода ему адекватного сообщения о ошибке. А вот если вы накосячили в коде или вообще напрямую кинули данные введённые пользователями, вам метод открытия файла и кинет исключение.

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

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

            >хочешь создавать контроллер из модели не через T4 а через run time генерирование
            почему не реализовать свой controller selector и свои контроллеры (со своими динамическими экшонами итд) по динамической модели данных?
            • 0
              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 контроллер может быть заменен и далее развит подобным способом.
              • +2
                Возможно вам await в реализации метода не нужен.
                • –3

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

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

                    Прочитаю статью — выношу решение.
                  • 0
                    Как жаль что на хабре нельзя изменять комментарии…

                    Поправляю.

                    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.
                    • +1
                      Ну, концепцию я понял. В моем MVC 2-3(я не на коре) можно сделать полностью динамический контроллер со своей диспечеризацией. Либо сделать Route со своим Handler'ом и там уже и контроллера нет.

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

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


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

                          • 0
                            А если выставить ConfigureAwait = false ASP.NET будет всегде брать из ThreadPool?
                            • +1
                              Да я вверху ошибся, забыл что он по дефолту захватывает контекст.
                              То что я написал вверху относится к .ConfigureAwait(false).
                              С его отсутствием или .ConfigureAwait(true) будет шедулинг в текущем SynchronizationContext либо в текущем TaskScheduler либо в дефолтном TaskScheduler(ThreadPool).

                              Получается 'await Edit();' возвращает исполнение в тот контекст в которм был до await. Вот тут я не уверен, но ASP.NET MVC на это пофиг.
                    • 0
                      Не совсем понятно, что там за проблема с T4. Если не хочется писать контроллеры, то можно переопределить фабрику и использовать HandleUnknownAction. Чем этот вариант не подходит?
                      • 0
                        Нет никакой проблемы с T4 — тоже метод, (кстати не понятно зачем и в T4 шаблон вставлять атрибут вместо вызрва прямого кода, все же генерируется).

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

                        Есть и различия.
                        В ран-тайм нет возни со сгенерированными файами исходного кода, нет возни с файлами шаблонов, нет T4 — другого языка.
                        С T4 все быстрее, цель «сгенерировать один раз», не накладывает больших обязательств на гибкость, читаемость и конфигурироемость кода. Нет притензии на новый уровень абстракции (а у программиста есть травмирующий опыт от новых уровней абстракций).
                  • +2
                    Какой тип ответа возвращать, лучше всего определять по заголовкам. Ещё можно смотреть, не является ли запрос AJAX. Как-то не очень хорошо выглядит определение по имени неймспейса, хоть это и похоже на convention, всё же это костыль :)
                    • 0
                      Заголовки — хороший вариант. К сожалению бывают противные клиенты, которые их не отправляют или отправляют не те. Я привёл пример с явным указанием, что возвращать — json или view как раз потому что не смог подобрать достаточно интуитивного и удобного соглашения. У кого-то может быть другая ситуация, поэтому упомянул и привёл пример в какую сторону можно подумать.
                    • +1
                      А что делать, когда в модели есть Select? Вот как быть, когда надо презаполнить справочник? Уже не первый раз вижу подобные решения, но не понимаю, как решить такой вопрос.
                      • 0
                        Вы что-то такое имеете в виду?
                        public class Param
                        {
                            public int SomeEntityId { get;set; }
                            // нужно проверить, что сущность с таким id существует
                        }
                        • 0

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

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

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

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


                          protected TResult IsModelStateIsValid<TModel, TResult>(Func<TModel, TResult> ifModelIsValidFunc)
                          where TResult : IActionResult
                          • 0
                            Там все достаточно оптимизировано. В 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 в любом случае будет осуществляться проверка фильтров. Можете поделиться профайлингом на сколько дополнительный фильтр замедляет выполнение?
                            • 0

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

                              • 0

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

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

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

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

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

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

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


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

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

                                    • 0
                                      Тут, к сожалению, любые новые технологии или подходы проиграют… Но и ладно!
                                      Вот у меня «ладно» и не получается, поскольку хотя я и работаю один (фрилансер, а если бы приходилось через консервативных «архитектов», «сеньеров» и «тим лидов» пробиваться было бы еще нервней) то все равно выходить к людям с новыми идеями и подходами хочется и конструктива хочется, а тебе говорят «перехочется, ты проиграл, минус».
                                      • 0
                                        Конструктив всегда на стороне бизнеса в данном случае: сколько разработчиков знает ASP.NET MVC? А сколько WebSharper? Кто будет развивать и поддерживать проект, когда ведущий поедет в отпуск?
                                        • 0
                                          Если считать за честную монету аппеляцию к бизнесу то безусловно новый подход должен делать неважным вопрос «сколько разработчиков» т.е. быть производительным на новом уровне (грубо: меньше кода в два раза, при ясной абстракции).

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

                          Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.