Search code examples
asp.net-mvcaction-filter

Is this code a fit candidate for an ActionFilter?


public ActionResult Index(int ehrId, int? page)
{
    EHR ehr = ehrRepository.FindById(ehrId);
    if (ehr.UserName != User.Identity.Name)
        return View("Invalid Owner");

    var physicaltests = ehr.PhysicalTests.Where(test=>!test.IsDeleted).OrderByDescending(test => test.CreationDate);
    List<PhysicalTestListItem> physicalTestsVM = new List<PhysicalTestListItem>();
    Mapper.Map(physicaltests, physicalTestsVM);
    var paginatedTests = physicalTestsVM.ToPagedList(page ?? 0, PAGESIZE);// new PaginatedList<PhysicalTestListItem>(physicalTestsVM, page ?? 0, pageSize);
    return View(paginatedTests);
}

public ActionResult Create(int ehrId)
{
    EHR ehr = ehrRepository.FindById(ehrId);
    if (ehr.UserName != User.Identity.Name)
        return View("Invalid Owner");

    return View(new PhysicalTestForm());
}

I have absolutely all of my methods inside my PhysicalTestsController performing these three lines of code before execution. How can I refactor this to avoid so much repetition? I have only included two of my methods but there are actually six methods.


Solution

  • I would think so

    An Action Filter would be appropriate for validating input data.

    Eg

    EHR ehr = ehrRepository.FindById(ehrId);
    if (ehr.UserName != User.Identity.Name)
      return View("Invalid Owner");
    

    Would be appropriate to put in an Action Filter, I have created other Input Validation as in [Requires Login] or [RequiresSSL] in the past so that multiple actions can share the same code (onBeforeExcuting, or onAfterExecuted).

    Aspect Oriented Programming for Cross Cutting Concerns

    However other things such as Logging which is a cross cutting concern across all layers of your architecture, you may want to look into AOP (Aspect Oriented Programming) to help avoid duplication of code there.

    Update - Sample Code (Untested)

    public class CheckValidUserAttribute : ActionFilterAttribute
    {
        public override void OnActionExecuting(ActionExecutingContext filterContext)
        {
          IMyRepository myRepository = IoC.Resolve<IMyRepository>();
          Int64 userId;      
          if(Int64.TryParse(filterContext.HttpContext.Request.QueryString["id"]), out userId))
             if(!IsValidUser(userId))
                filterContext.Result = new InvalidUserResult();   
         }
    
         public bool IsValidUser(IMyRepository myRepository, Int64 userId)
         {
           EHR ehr = ehrRepository.FindById(ehrId);      
           return ehr != null && ehr.UserName == User.Identity.Name;
         }
    }
    

    Something like the above should do it, you'd need to create a named view to return your "Invalid User", however that should be trivial. There should be plenty of examples on the net on how to create and use Action Filters. Personally I learnt from Pro ASP.NET MVC Framework and