Rewrite of ASP.NET MVC’s controller (part 4)

Hi everyone,

In this series of articles I made a rant about recommended software templates and techniques (A rant about Microsoft’s ASP.NET MVC templates). In the last article (How to manage permissions on actions in ASP.NET MVC (Part 3)) I proposed how to make proper permissions/actions mechanism to protect views and controller actions against misuse of actions by people without authorization and only in Order states which allow certain actions.

In this article I’m gonna address another requirement:

How to create a proper ASP.NET MVC controller ??

Concepts in this article apply to all kinds of controllers, especially ASP.NET MVC,  ASP.Net Web API and ASP.NET Core. After some modifications of course. Only ASP.NET MVC is the scope of current article.

Let’s start from the beginning. Let’s start from standard ASP.NET MVC’s scaffolding. Let’s create a model class of an order:

   
public class Order
    {
        public enum Statuses { Open, Assigned, Sent, Cancelled }

        public int OrderId { get; set; }

        [Required]
        public String OrderCode { get; set; }
      
        [DisplayFormat(DataFormatString = "{0:yyyy-MM-dd HH:mm}", ApplyFormatInEditMode = true)]
        public DateTime? SentDate { get; set; }
     
        [Required]
        public String Status { get; set; }

        [Required]
        public int CustomerId { get; set; }
        public virtual Customer Customer { get; set; }

    }

Now let’s create a controller automatically, using built in VS2015 features.
blog1

And let’s see what dear collegues from Microsoft created for us. Let’s read from the top:

    public class OrdersController : Controller
    {
        private ApplicationDbContext db = new ApplicationDbContext();

        // GET: Orders
        public ActionResult Index()
        {
            var orders = db.Orders.Include(o => o.Customer);
            return View(orders.ToList());
        }

Wait, what ? Context is created directly without any repository or business layer class ? Index view is generated directly from database query ? Let’s see other actions:

        public ActionResult Details(int? id)
        {
            if (id == null)
            {
                return new HttpStatusCodeResult(HttpStatusCode.BadRequest);
            }
            Order order = db.Orders.Find(id);
            if (order == null)
            {
                return HttpNotFound();
            }
            return View(order);
        }
        public ActionResult Edit(int? id)
        {
            if (id == null)
            {
                return new HttpStatusCodeResult(HttpStatusCode.BadRequest);
            }
            Order order = db.Orders.Find(id);
            if (order == null)
            {
                return HttpNotFound();
            }
            ViewBag.CustomerId = new SelectList(db.Customers, "CustomerId", "FirstName", order.CustomerId);
            return View(order);
        }
        public ActionResult Delete(int? id)
        {
            if (id == null)
            {
                return new HttpStatusCodeResult(HttpStatusCode.BadRequest);
            }
            Order order = db.Orders.Find(id);
            if (order == null)
            {
                return HttpNotFound();
            }
            return View(order);
        }

Well, it’s repeated code. Three method’s almost identical. Against DRY principle. What’s ViewBag doing here ? You don’t use ViewBag in real system, unless it’s some workaround for technical difficulties or just some proof of concept. A list of customers should be part of viewmodel, which means that this action should not be built around Order entity but rather some viewmodel which contains customer list. (If you don’t know what viewmodel is refer to this article). Let’s see some POST actions (above presented are GET actions):

        [HttpPost]
        [ValidateAntiForgeryToken]
        public ActionResult Edit([Bind(Include = "OrderId,OrderCode,SentDate,Status,CustomerId")] Order order)
        {
            if (ModelState.IsValid)
            {
                db.Entry(order).State = EntityState.Modified;
                db.SaveChanges();
                return RedirectToAction("Index");
            }
            ViewBag.CustomerId = new SelectList(db.Customers, "CustomerId", "FirstName", order.CustomerId);
            return View(order);
        }

Ok, so allowed fields are written as string literals. So when any time they change you have to update all actions that use Bind and Include in entire application. Also when SaveChanges throws an exception you are in deep sh..t: no try catch, no common nice error page, no event log. Checking ModelState for irregularities is fine, but doesn’t catch all problems that may arise while saving to database. As mentioned in my earlier posts, some errors throw exceptions while saving so you have to catch them and convert into meaningful ModelState error messages (like for example: if a user breaks unique index constraint by entering non-unique id for some product).
Again we find usage of Viewbag, which is really a bad way to add something that you forgot at design time and want to fix with a cheap glue. Look at the automatically generated view code from this Viewbag property: @Html.DropDownList("CustomerId", null, htmlAttributes: new { @class = "form-control" }). It accesses Viewbag’s property using string literal, so any time you change it’s name this view will throw an exception. Also VS is not able to help you in any way because it’s a dynamic property so VS does not check it’s type in any way. Another problem is SelectList, which is almost the same for Edit and Create views (both GET and POST http modes). Also this code again uses direct access to database without any business layer. It’s like coding entire business logic in WinForms click event. BAD. DESIGN.

Ok, so we got repeated code and common behaviour in actions. Also we need to separate roles of each component: controller has to control what views display and should not do business processes, nor it should write and read from/to database. Repository object should not be initiated directly in controller, that is bad design (no loose coupling). It should be injected with some dependency injection container. It makes it easier to write unit tests then  and change database technology in the future if needed.

For the purpose of this article I used Unity MVC dependency injection container, but you may use any container. So constructor of our OrderController should look like:

       public OrdersController(IOrderRepo repo)
        {
            this.repo = repo;
        }

Where IOrderRepo contains some basic repository (database) access functions. In my case it’s:

    public interface IOrderRepo 
    {
        void Create(Order Order);
        void Update(Order order);
        void Delete(int id);
        IQueryable GetCustomers();
        IQueryable GetOrders();
        bool IsOrderCodeAlreadyTaken(String OrderCode);
    }

And then we have to configure Unity to inject Order repository into controller, automatically:

        public static void RegisterTypes(IUnityContainer container)
        {
            container.RegisterType< AccountController >(new InjectionConstructor());
            container.RegisterType< ManageController >(new InjectionConstructor());
            container.RegisterType< IOrderRepo, OrderRepo >(new PerRequestLifetimeManager());
            container.RegisterType< IContext, ApplicationDbContext >(new PerRequestLifetimeManager());
        }

IContext is an interface of the context class (db access class, in template application it’s called ApplicationDbContext). It allows repository classes to be initialized automatically with injected database context objects like so:

    
public class OrderRepo : IOrderRepo
    {
     
        private readonly IContext db;

        public OrderRepo(IContext Db)
        {
            this.db = Db;
        }

Next, we need to standardize: permissions to actions, setting up additional view controls (if needed) and processing business functions. Most of things we need in each action is shared across all actions:

  • set action name (Index, Create etc)
  • set http mode (get, post etc)
  • set businesss entity id (if needed)
  • load view model
  • set special view controls (if needed)
  • check for permissions to this action and permission to perform action for business entity in it’s current state
  • set redirect action where we send user after performing action (if needed)

It seems we need a separate class to handle all of this. I called it ViewBuilder. In previous article I made first attempt to it, but I wasn’t happy about it. Last attempt to view builder was a controller method called ProcessGetAction . It had arguments of all things that were mentioned above. My lack of happiness was because the code was not very readable and it wanted to do too many things:

 public ActionResult Edit(int? id)
        {
            return ProcessGetAction(
                ()=>IsActionAvailable(id, Order.Actions.Edit), 
                Order.Actions.Edit.ToString(), 
                ()=>repo.Get(id),
                () => SetUpCreateEditViewControls(id));
        }

Even when you look at definition and input arguments of this method you have to be very concentrated to understand what is this function about and what are these parameters doing. It’s very easy to do something wrong. We need a different approach to initialization of a view. I decided to use so called fluent api, which allows more readable creation of a complex objects. Let’s see how it would look for GET actions:

        public ActionResult Create()
        {
            var viewBuilder = ViewBuilder.Initialize(this)
             .SetActionName(Order.Actions.Create.ToString())
             .SetMode(ViewBuilder.HttpModes.Get)
             .SetRolePermission(IsActionAvailable)
             .InitView(SetUpCreateViewControls)
             .Build();
            return viewBuilder.View;
        }
        public ActionResult Details(int? id)
        {
            var viewBuilder = ViewBuilder.Initialize(this)
                .SetActionName(Order.Actions.Details.ToString())
                .SetMode(ViewBuilder.HttpModes.Get)
                .SetItemId(id)
                .SetGetModel(()=>repo.Get(id))
                .SetItemPermission(IsActionAvailable)
                .Build();
            return viewBuilder.View;
        }

Much more readable, isn’t it. But how does ViewBuilder class look like ?

    public class ViewBuilder
    {
        public enum BuilderStates { Initialized, Built, GeneralError, AuthorizationError, ValidationError }
        public enum HttpModes { Get, Post, Unknown}

        public Func< String, bool > CheckIfRoleAllowed { get; internal set; }
        public Func< int?, String, bool > CheckIfItemAllowed { get; internal set; }
        public String ActionName { get; internal set; }
        public Func< object > GetModelData { get; internal set; }
        public Action< String > BusinessFunc { get; internal set; }
        public Action SetUpView { get; internal set; }
        public int? ItemId { get; internal set; }
        public object resultModel { get; set; }
        public IMyController controller { get; set; }
        public BuilderStates BuilderState { get; internal set; }
        public HttpModes HttpMode { get; internal set; } = HttpModes.Unknown;
        public String RedirectAction { get; internal set; }

        public static ViewBuilder Initialize(IMyController Controller)
        {
            var builder = new ViewBuilder();
            builder.resultModel = null;
            builder.controller = Controller;
            builder.ItemId = null;
            builder.BuilderState = BuilderStates.Initialized;
            return builder;
        }

        public ActionResult View {
            get {

                if (this.BuilderState== BuilderStates.ValidationError)
                {
                    if (resultModel == null) return controller.MyView();
                    else
                        return controller.MyView(resultModel);
                }

                if (this.BuilderState!= BuilderStates.Built)
                {
                    Logger.Logging.LogError("View is not built sucessfully, can't show");
                    return controller.MyRedirectToAction("Error", "Home");
                }

                if (!String.IsNullOrEmpty(RedirectAction))
                    return controller.MyRedirectToAction(RedirectAction);

                if (resultModel == null) return controller.MyView();
                else
                    return controller.MyView(resultModel);
            }
        }
    }

And the build method:

       public static ViewBuilder Build(this ViewBuilder builder)
        {
            try
            {
                if (builder.HttpMode == ViewBuilder.HttpModes.Unknown)
                    throw new BuildViewException("HttpMode not set");

                if (String.IsNullOrEmpty(builder.ActionName))
                    throw new BuildViewException("ActionName was not set");

                if (builder.ItemId == null && builder.CheckIfItemAllowed != null)
                    throw new BuildViewException("ItemId not set can't run CheckIfItemAllowed");

                if (builder.CheckIfRoleAllowed != null && !builder.CheckIfRoleAllowed.Invoke(builder.ActionName))
                    throw new MyActionNotAllowedException(builder.ActionName, builder.controller.GetType().Name);

                if (builder.CheckIfItemAllowed != null && builder.ItemId!=null && !builder.CheckIfItemAllowed.Invoke(builder.ItemId, builder.ActionName))
                    throw new MyActionNotAllowedException(builder.ActionName, (int)builder.ItemId, builder.controller.GetType().Name);

                if (builder.GetModelData != null)
                    builder.resultModel = builder.GetModelData.Invoke();
                builder.BusinessFunc?.Invoke(builder.ActionName);
                if (String.IsNullOrEmpty(builder.RedirectAction))
                    builder.SetUpView?.Invoke();
                builder.BuilderState = ViewBuilder.BuilderStates.Built;
                return builder;
            }
            catch (MyValidationException mve)
            {
                builder.BuilderState = ViewBuilder.BuilderStates.ValidationError;
                builder.controller.AddValidationErrorsToModelState(mve);
                builder.SetUpView?.Invoke();
                return builder;
            }
            catch (MyActionNotAllowedException )
            {
                builder.BuilderState = ViewBuilder.BuilderStates.AuthorizationError ;
                return builder;
            }
            catch (BuildViewException )
            {
                builder.BuilderState = ViewBuilder.BuilderStates.GeneralError;
                return builder;
            }
         
        }

Now you have one, common way to check permissions, load a model, set up view, do business function and redirect to other view.
In the next article I’m going to show you what’s inside business layer, what is SetBusinessFunc(orderBL.PerformAction) and how to to separate business functions from database access:

        [HttpPost, ActionName(CancelActionName)]
        [ValidateAntiForgeryToken]
        public ActionResult CancelConfirmed(int id)
        {
            Order order = repo.Get(id);
            BusinessLayer.OrderBL orderBL = new BusinessLayer.OrderBL(repo, order, Helpers.UserHelper.GetRoles(HttpContext).ToList());

            var viewBuilder = ViewBuilder.Initialize(this)
               .SetActionName(Order.Actions.Cancel.ToString())
               .SetMode(ViewBuilder.HttpModes.Post)
               .SetItemId(order.OrderId)
               .SetItemPermission(IsActionAvailable)
               .SetBusinessFunc(orderBL.PerformAction)
               .SetGetModel(() => repo.Get(order.OrderId))
               .SetRedirectAction(Order.Actions.Index.ToString())
               .Build();
            return viewBuilder.View;
        }

Thanks for reading 🙂

Dominik Steinhauf

CEO, .Net developer, software architect at Creative Yellow Solutions (formerly Indesys)

If you need help with your software project, or need customized software for your company, contact me at: dominik.steinhauf ( at) cys.biz.pl

Leave a Reply

Your email address will not be published. Required fields are marked *