2011-04-09 50 views
4

我有一个控制器,它具有方法GetSignatories(),AddMe(),RemoveMe(),AddUser(),RemoveUser()等等,以便我必须每次验证如果合同存在,用户是否有权访问合同以及是否存在请求的版本。我想知道我应该在哪里放这个代码?在一个服务中或者用我的控制器的其他方法提取?我的问题是,我soometime返回Unathorized或NotFound,不知道什么是最好的方式来做到这一点。在控制器中重复授权代码的最佳做法

这里是我的控制器:

public partial class ContractController : Controller 
    { 

     private ISession _session; 
     private IAuthenticationService _authService; 

     public V1Controller(ISession session, IAuthenticationService authService) 
     { 
      _session = session; 
      _authService = authService; 
     } 

     [HttpPost] 
     [Authorize(Roles = "User")] 
     [ValidateAntiForgeryToken] 
     public virtual ActionResult GetSignatories(string contractId, int version) 
     { 
      //NOTE Should be extracted 
      var contract = _session.Single<Contract>(contractId); 
      if (contract == null) return HttpNotFound(); 
      var user = _authService.LoggedUser(); 
      if (contract.CreatedBy == null || !contract.CreatedBy.Id.HasValue || contract.CreatedBy.Id.Value != user.Id) return new HttpUnauthorizedResult(); 
      if (contract.Versions.Count < version) return HttpNotFound(); 
      /*NOTE END Should be extracted */ 

      return Json(contract.CurrentVersion.Tokens.Select(x => x.User).ToList()); 
     } 

     [HttpPost] 
     [Authorize(Roles = "User")] 
     [ValidateAntiForgeryToken] 
     public virtual ActionResult AddMe(string contractId, int version) 
     { 
      //NOTE Should be extracted 
      var contract = _session.Single<Contract>(contractId); 
      if (contract == null) return HttpNotFound(); 
      var user = _authService.LoggedUser(); 
      if (contract.CreatedBy == null || !contract.CreatedBy.Id.HasValue || contract.CreatedBy.Id.Value != user.Id) return new HttpUnauthorizedResult(); 
      if (contract.Versions.Count < version) return HttpNotFound(); 
      /*NOTE END Should be extracted */ 

      return AddUserToContract(contract, new UserSummary(user)); 
     } 

     [HttpPost] 
     [Authorize(Roles = "User")] 
     [ValidateAntiForgeryToken] 
     public virtual ActionResult RemoveMe(string contractId, int version) 
     { 
      //NOTE Should be extracted 
      var contract = _session.Single<Contract>(contractId); 
      if (contract == null) return HttpNotFound(); 
      var user = _authService.LoggedUser(); 
      if (contract.CreatedBy == null || !contract.CreatedBy.Id.HasValue || contract.CreatedBy.Id.Value != user.Id) return new HttpUnauthorizedResult(); 
      if (contract.Versions.Count < version) return HttpNotFound(); 
      /*NOTE END Should be extracted */ 

      return RemoveUserFromContract(contract, new UserSummary(user)); 
     } 

     [HttpPost] 
     [Authorize(Roles = "User")] 
     [ValidateAntiForgeryToken] 
     public virtual ActionResult AddUser(string contractId, int version, UserSummary userSummary) 
     { 
      //NOTE Should be extracted 
      var contract = _session.Single<Contract>(contractId); 
      if (contract == null) return HttpNotFound(); 
      var user = _authService.LoggedUser(); 
      if (contract.CreatedBy == null || !contract.CreatedBy.Id.HasValue || contract.CreatedBy.Id.Value != user.Id) return new HttpUnauthorizedResult(); 
      if (contract.Versions.Count < version) return HttpNotFound(); 
      /*NOTE END Should be extracted */ 


      return AddUserToContract(contract, userSummary); 
     } 

     [HttpPost] 
     [Authorize(Roles = "User")] 
     [ValidateAntiForgeryToken] 
     public virtual ActionResult RemoveUser(string contractId, int version, UserSummary userSummary) 
     { 
      //NOTE Should be extracted 
      var contract = _session.Single<Contract>(contractId); 
      if (contract == null) return HttpNotFound(); 
      var user = _authService.LoggedUser(); 
      if (contract.CreatedBy == null || !contract.CreatedBy.Id.HasValue || contract.CreatedBy.Id.Value != user.Id) return new HttpUnauthorizedResult(); 
      if (contract.Versions.Count < version) return HttpNotFound(); 
      /*NOTE END Should be extracted */ 


      return RemoveUserFromContract(contract, userSummary); 
     } 
} 

对于那些谁可能会寻求如何在全球注册模型绑定:

public static void RegisterModelBinders() 
    { 
     var session = (ISession)DependencyResolver.Current.GetService(typeof(ISession)); 
     var authService = (IAuthenticationService)DependencyResolver.Current.GetService(typeof(IAuthenticationService)); 
     System.Web.Mvc.ModelBinders.Binders[typeof (Contract)] = new ContractModelBinder(session, authService); 
    } 
+0

对于那些希望在这里ModelBinder的如何进行单元测试是一个良好的开端:http://stackoverflow.com/questions/1992629/unit-testing-custom-model-binder-in-asp-net-mvc-2 – VinnyG 2011-04-09 18:57:06

回答

2

你的确有相当的重复代码。有许多方法来重构这个代码,其中一个由编写自定义模型绑定的Contract型号:

public class ContractModelBinder : DefaultModelBinder 
{ 
    private readonly ISession _session; 
    private readonly IAuthenticationService _authService; 
    public ContractModelBinder(ISession session, IAuthenticationService authService) 
    { 
     _session = session; 
     _authService = authService; 
    } 

    public override object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext) 
    { 
     string contractId = null; 
     int version = 0; 
     var contractIdValue = bindingContext.ValueProvider.GetValue("contractId"); 
     var versionValue = bindingContext.ValueProvider.GetValue("version"); 
     if (versionValue != null) 
     { 
      version = int.Parse(versionValue.AttemptedValue); 
     } 
     if (contractIdValue != null) 
     { 
      contractId = contractIdValue.AttemptedValue; 
     } 

     var contract = _session.Single<Contract>(contractId); 
     if (contract == null) 
     { 
      throw new HttpException(404, "Not found"); 
     } 
     var user = _authService.LoggedUser(); 
     if (contract.CreatedBy == null || 
      !contract.CreatedBy.Id.HasValue || 
      contract.CreatedBy.Id.Value != user.Id 
     ) 
     { 
      throw new HttpException(401, "Unauthorized"); 
     } 

     if (contract.Versions.Count < version) 
     { 
      throw new HttpException(404, "Not found"); 
     } 
     return contract; 
    } 
} 

一旦在Application_Start注册这个模型绑定与Contract模式控制器简单地变为:

public class ContractController : Controller 
{ 
    [HttpPost] 
    [Authorize(Roles = "User")] 
    [ValidateAntiForgeryToken] 
    public ActionResult GetSignatories(Contract contract) 
    { 
     return Json(contract.CurrentVersion.Tokens.Select(x => x.User).ToList()); 
    } 

    [HttpPost] 
    [Authorize(Roles = "User")] 
    [ValidateAntiForgeryToken] 
    public ActionResult AddMe(Contract contract) 
    { 
     var user = contract.CreatedBy; 
     return AddUserToContract(contract, new UserSummary(user)); 
    } 

    [HttpPost] 
    [Authorize(Roles = "User")] 
    [ValidateAntiForgeryToken] 
    public ActionResult RemoveMe(Contract contract) 
    { 
     var user = contract.CreatedBy; 
     return RemoveUserFromContract(contract, new UserSummary(user)); 
    } 

    [HttpPost] 
    [Authorize(Roles = "User")] 
    [ValidateAntiForgeryToken] 
    public ActionResult AddUser(Contract contract, UserSummary userSummary) 
    { 
     return AddUserToContract(contract, userSummary); 
    } 

    [HttpPost] 
    [Authorize(Roles = "User")] 
    [ValidateAntiForgeryToken] 
    public ActionResult RemoveUser(Contract contract, UserSummary userSummary) 
    { 
     return RemoveUserFromContract(contract, userSummary); 
    } 
} 

我们成功地put it on a diet

+1

确实很短的代码,但是你确定模型绑定器是否适合这个?毕竟有很多业务逻辑。把它放在一个服务类似乎是一种更常见的方法... – 2011-04-09 16:11:50

+0

@Adrian Grigore,严格来说是0的商业逻辑。这是授权逻辑,实际上授权过滤器可能是另一种可能的解决方案,但模型绑定器的优点是您可以在控制器操作中获得结果,以便您可以直接使用它们。如果您使用服务,那么您仍然会在这些操作中拥有大量重复逻辑,并且它们不会太干。 – 2011-04-09 16:13:48

+0

它似乎是最好的解决方案,你将如何处理HttpException?项目文件夹中的活页夹在哪里? – VinnyG 2011-04-09 16:14:19

2

一个可能的解决方案是创建一个IContractService接口,有两种方法,一种获得合同,另一个对其进行验证:

public IContractService 
{ 
    Contract GetContract(int id); 
    ValidationResult ValidateContract(Contract contract); 
} 

ValidationResult可能是一个枚举,只是信号的呼叫者方法如何进行:

public enum ValidationResult 
{ 
    Valid, 
    Unauthorized, 
    NotFound 
} 

一种可能实现的IContractService

public class ContractService : IContractService 
{ 
    private readonly ISession _session; 
    private readonly IAuthenticationService _authService; 

    // Use Dependency Injection for this! 
    public ContractService(ISession session, IAuthenticationService authService) 
    { 
     _session = session; 
     _authService = authService; 
    } 

    public Contract GetContract(int id) 
    { 
     var contract = _session.Single<Contract>(contractId); 

     // hanlde somwhere else whether it's null or not 
     return contract; 
    } 

    public ValidationResult ValidateContract(Contract contract) 
    { 
     var user = _authService.LoggedUser(); 
     if (contract.CreatedBy == null || !contract.CreatedBy.Id.HasValue || 
      contract.CreatedBy.Id.Value != user.Id) 
       return ValidationResult.Unauthorized; 

     if (contract.Versions.Count < version) 
      return ValidationResult.NotFound; 

     return ValidationResult.Valid; 
    } 
} 

然后在您的控制器仍然可以返回各种HttpNotFound等的观点:

[HttpPost, Authorize(Roles = "User"), ValidateAntiForgeryToken] 
public virtual ActionResult GetSignatories(string contractId, int version) 
{ 
    //NOTE Should be extracted 
    var contract = _contractService.GetContract(contractId); 

    if (contract == null) 
     return HttpNotFound(); 

    var result = _contractService.ValidateContract(contract); 

    if (result == ValidationResult.Unauthorized) 
     return new HttpUnauthorizedResult(); 

    if (result == ValidationResult.NotFound) 
     return HttpNotFound(); 

    return Json(contract.CurrentVersion.Tokens.Select(x => x.User).ToList()); 
} 
+1

你会如何在我的控制器中执行GetSignatories()方法?如果我明白了,该服务只会取代最后3行,而我的控制器中会有尽可能多的代码? – VinnyG 2011-04-09 16:24:50

+0

实际上这是正确的。这主要将验证逻辑移出控制器,它不属于该控制器。将ActionResult'返回到视图仍然是控制器的工作。我会更新我的答案,以包含“GetSignatoires”操作。 – 2011-04-09 16:32:30

+0

我认为这两种解决方案都很好,但我更喜欢Darin,因为我的代码较少。感谢您的帮助! – VinnyG 2011-04-09 17:10:05