I have a controller that has method GetSignatories(), AddMe(), RemoveMe(), AddUser(), RemoveUser() and more to come in witch I have to verify every time if the contract is existing, if the user have access to the contract and if the requested version is existing. I'm wondering where should I put this code? In a Service or extracted in an other method of my controller? my problem is that I soometime return Unathorized or NotFound and don't know what would be the best way to do this.
Here's my controller :
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);
}
}
For those who might be looking how to register the model binder in the global :
public static void RegisterModelBinders()
{
var session = (ISession)DependencyResolver.Current.GetService(typeof(ISession));
var authService = (IAuthenticationService)DependencyResolver.Current.GetService(typeof(IAut开发者_运维问答henticationService));
System.Web.Mvc.ModelBinders.Binders[typeof (Contract)] = new ContractModelBinder(session, authService);
}
Indeed you have pretty repeating code. There are many ways to refactor this code, one of which consists of writing a custom model binder for the Contract
model:
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;
}
}
Once you have registered this model binder with the Contract
model in Application_Start
your controller simply becomes:
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);
}
}
We successfully put it on a diet.
One possible solution would be to create an IContractService
interface that has two methods, one to get the contract and another to validate it:
public IContractService
{
Contract GetContract(int id);
ValidationResult ValidateContract(Contract contract);
}
ValidationResult
could be an enumeration, just to signal the caller of the method how to proceed:
public enum ValidationResult
{
Valid,
Unauthorized,
NotFound
}
A possible Implementation of 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;
}
}
Then in your controller you can still return the various HttpNotFound
, etc to the view:
[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());
}
精彩评论