четверг, 14 июля 2011 г.

Как правильнее осуществить проверку бизнес правил в ASP.NET MVC приложении?

            Данный пост является скорее всего темой для обсуждения, а не готовым решением хотелось бы выяснить у кого какие мнения и решения на данный счет.
При разработки ASP.NET MVC приложения столкнулся со следующей задачей опишу  вкратце.
Приложение ASP.NET MVC имеет архитектуру из 3 слоев.
1. Data Access (Repositories)
2. Business logic (Services)
3. Application layer (Controllers)
            Имеется класс Learner который представляет собой сущность ученика, ученики могут сдавать экзамены, при сдаче экзамена, создается заказ (Order  класс), после сдачи экзамена, для каждого ученика необходимо выставить результирующие оценки. При выставлении результатов необходимо проверить следующие правила
1.      Результаты не должны быть уже выставлены
2.      Все ученики которые имеют статус Present (присутствовал на экзамене) должны иметь оценку
3.      Оценочная шкала должна быть подтверждена.

Когда пользователь выполняет выставление оценок все эти правила должны проверятся, и если одна из проверок не прошла пользователю должно выводится соответствующее сообщение об ошибке.
            Я решил что логика по проверке бизнес правил должна быть Service классе и если одна из проверок не прошла то генерировать специфическое исключение с описанием ошибки.
Вот  метод класса сервиса
public void ReleaseResults(long orderId)
{
    var order = orderRepository.Get<ExamOrder>(orderId);

    Check.Require(order != null, "Order was not found");

    if (IsOrderReleased(order))
    {
        throw new ReleaseResultsException("The results has been already released", order.OrderNo);
    }

    if (AllLearnersHasStatusPresentAndMark(order))
    {
        throw new ReleaseResultsException("One or more learners unmarked", order.OrderNo);
    }
    if (!GradingBoundaryConfirmed(order))
    {
        throw new ReleaseResultsException("The Grading boundary needs to be confirmed", order.OrderNo);
    }

    foreach (var learnerDetail in order.LearnerDetails)
    {
        if (HasNotStatusPresent(learnerDetail))
        {
            continue;
        }
        learnerDetail.SetReleasedResults();

    }

    orderRepository.SaveOrUpdate(order);
}


И вот метод класса Controller

        public ActionResult Release(EncryptedId orderId)
        {
            Check.Require(orderId != null, "The parameter orderId was null");

            try
            {
                orderReleaseResultsService.ReleaseResults(orderId);
            }
            catch (ReleaseResultsException e)
            {
                return Content(string.Format("Error: {0}", e.Message));
            }

            return Content(MUI.TheResultsHasBeenReleased);
        }

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

21 комментарий:

  1. ИМХО очень правильно так писать. Проверки являются предусловиями к действию, поэтому и должны проверяться в действии и не где-либо еще.

    ОтветитьУдалить
  2. @gandjustas то есть Вы имеете ввиду, что генерирование исклюений при прохождении проверок, а потом их обработка в action методе в контроллере это нормально. Правильно я понял?

    ОтветитьУдалить
  3. Рекомендую посмотреть FluentValidation. Я им пользуюсь и выношу всю валидацию в отдельные классы, где декларативно описываются правила.

    ОтветитьУдалить
  4. Да, забыл написать - естественно вызов метода сервиса (слоя бизнес логики) завернут в обработчик исключений типа ValidationException. Такие исключения обрабатываются и информация из них кладется в ModelState.ModelErrors. Это позволяет на автомате подствечивать неверно заполненные поля в форме.

    ОтветитьУдалить
  5. Исключения генерировать как раз правильно. А вот использовать их для валидации - не очень. Имхо.

    Я пришел к таким возможным вариантам:
    1) Проверять все входные данные в контроллере и выводить ошибки валидации пользователю. В бизнес-логике снова проверять данные, уже генерируя исключения. Но тут получается дублирование проверок.
    2) Можно из ReleaseResults возвращать не void, а некой объект с результатом (enum, например), который бы говорил о том, что все прошло успешно, либо что-то не верно.

    Тоже интересен данный вопрос, буду рад услышать ваши мнения.

    ОтветитьУдалить
  6. Этот комментарий был удален автором.

    ОтветитьУдалить
  7. @Хэлкар да Вы правильно говорите я знаю про FluentValidation, но я говорю не про валидацию пользовательского ввода, а про проверку некоторых бизнес правил, которые могут быть довольно сложными не знаю можно ли в данном случае обойтись FluentValidation, и получатся он тоже генерирует исключения так? Правильно я Вас понял, может пример где покажете?

    ОтветитьУдалить
  8. @ginfag я тоже думал о том чтобы возврашать некий результат прохождения данной операции возможно так правильнее.
    Первая часть Вашего ответа относится опять же к проверке пользовательского ввода. В данном случае задача другая как написал выше.

    ОтветитьУдалить
  9. @Сергей. FluentValidator'у вообще без разницы что проверять. Он просто реализует возможность создавать набор правил. Бизнес-правила не исключение. Механизм задания правил позволяет написать почти все что угодно. Например там есть правило Must, которое просто описывается функцией возвращающей bool. Да, FluentValidator кидает исключения типа ValidationException. В доках у них все доавольно просто - http://fluentvalidation.codeplex.com/documentation

    ОтветитьУдалить
  10. А вот делать проверку бизнес-логики в контроллере - это как раз неправильно (ИМХО).

    ОтветитьУдалить
  11. Этот комментарий был удален автором.

    ОтветитьУдалить
  12. Business Logic отвечает и за проверку данных в том числе. Поэтому полностью согласен с автором статьи насчет подхода к валидации. В своих проектах обычно использую типизированные Exeption вместо бросания одного типа, но с различными сообщениями.
    Сам Application layer при этом выглядит, например, так:

    try
    {
    orderReleaseResultsService.ReleaseResults(orderId);
    }
    catch (CustomType1Exception e)
    {
    return ModelState.ModelErrors.Add(..."Some error1"...);
    }
    catch (CustomType2Exception e)
    {
    return ModelState.ModelErrors.Add(..."Some error2"...);
    }

    Это позволяет выносить строку с ошибкой в ресурсы и локализовать их.

    ОтветитьУдалить
  13. Непонятно почему использование одного типа исключений мешает локализации? Утверждаю что не мешает.

    ОтветитьУдалить
  14. Не мешает, но тогда сами ресурсы нужно будет хранить в Business logic, что есть не совсем правильно, на мой взгляд.

    ОтветитьУдалить
  15. Выброс исключений при обработке бизнес логики имеет смысл. В конечном счете, исключение может быть выброшено на уровне базы данных (unique key violation и т.п.), что тоже является частью бизнес контекста.
    Я стараюсь для каждой сущности завожить спаренный класс вроде EntityFacility, в котором проверяются бизнес правила перед отправкой в БД или другой persistence context. Проверка бизнес правил выдает результат в виде экхемпляра класса ValidationResult или вроде того, а слой инфраструктуры приложения решает, что делать с этим объектом - заворачивать все сообщения в исключение и пробрасывать дальше или просто писать лог...

    ОтветитьУдалить
  16. @yurets спасибо за ответ над локализацией сообщений тоже думал так как для нашего приложения это важно.
    @Хэлкар вот только не понятно мне тоже как локализовать
    исключение одного типа так как у нас в слое бизнес логики не используются ресурсы. Может Вы по другому имеете ввиду?

    ОтветитьУдалить
  17. @yurets - просто если проверок много то получается и много надо дополнительных классов исключений.

    ОтветитьУдалить
  18. @yurets - Не мешает, но тогда сами ресурсы нужно будет хранить в Business logic, что есть не совсем правильно, на мой взгляд.
    У нас в проекте как раз так.

    ОтветитьУдалить
  19. @Хэлкар - А вот делать проверку бизнес-логики в контроллере - это как раз неправильно (ИМХО)

    Полностью согласен

    ОтветитьУдалить
  20. @Сергей, честно говоря у нас нет такого ограничения - ресурсы выделены в отдельную сборку и ею может пользоваться любая часть системы.

    ОтветитьУдалить
  21. @Хэлкар да наверное так тоже будет правильно.

    ОтветитьУдалить