I have a question regarding the implementation of creating "thin controllers" in an Asp.net Mvc 5 application. I've researched this topic for the past couple of days, and I believe I'm at the point where I need a concrete example in order to connect the dots in my understanding.
So, I would like to make use of Unit tests in my application. I've looked at creating view model factories, and builders, skinny controller-fat model, but I'm not quire sure how to implement any of these design patterns that I've read about in this particular scenario.
Below you'll find 5 different actions that are found in my Manage controller. I fear that they smell and require some clean up for simplifying testing/unit testing. I understand that there is usually no "right answer" to these type of questions, so I would greatly appreciate all answers that aid in simplifying testing my application.
Here are my actions:
Action #1:
[HttpPost]
[ValidateAntiForgeryToken]
[Authorize(Roles = "DM_Admin")]
public async Task<ActionResult> Users_Create([DataSourceRequest] DataSourceRequest request, ManageUsersViewModel model)
{
if (model != null && ModelState.IsValid)
{
// instantiate new application user
var user = new ApplicationUser
{
UserName = model.Email,
Email = model.Email,
FirstName = model.FirstName,
LastName = model.LastName
};
// format the RolesList to type List<string> for entry
List<string> rolesToAssign = getRoleNameList(model);
try
{
// persist user to User Db
var createResult = await UserManager.CreateAsync(user, model.Password);
if (createResult.Succeeded)
{
// persist user roles to User Db
var rolesResult = await UserManager.AddToRolesAsync(user.Id, rolesToAssign.ToArray());
if (rolesResult.Succeeded)
{
return RedirectToAction("ManageUsers");
}
AddErrors(rolesResult);
}
else
{
AddErrors(createResult);
}
}
catch (Exception ex)
{
ErrLog.LogError(ex, "ManageController.Users_Create");
}
}
return Json(new[] { model }.AsQueryable().ToDataSourceResult(request, ModelState));
}
Action #2:
[Authorize(Roles = "DM_Admin")]
public async Task<ActionResult> Users_Read([DataSourceRequest] DataSourceRequest request)
{
List<ManageUsersViewModel> users = null;
List<ApplicationUser> allUsers = null;
try
{
// get all Data Management roles
var dmRoles = await RoleManager.Roles.Where(r => r.Name.Contains("DM_")).ToListAsync();
// find all the users for each Data Management role
foreach (var id in dmRoles.Select(r => r.Id).ToList())
{
if(allUsers == null)
{
allUsers = await UserManager.Users.Where(u => u.Roles.Any(r => r.RoleId == id)).ToListAsync();
}
else
{
allUsers.AddRange(await UserManager.Users.Where(u => u.Roles.Any(r => r.RoleId == id)).ToListAsync());
}
}
// list of users may have repeats, so remove repeated users in list
allUsers = allUsers.Distinct().ToList();
// instantiate view model with list of users and their respective roles
users = allUsers.Select(u => new ManageUsersViewModel
{
Id = u.Id,
FirstName = u.FirstName,
LastName = u.LastName,
Email = u.Email,
Password = u.PasswordHash,
ConfirmPassword = u.PasswordHash,
RolesList = dmRoles.Where(r => r.Users.Any(user => user.UserId == u.Id)).Select(r => new RoleModel
{
Id = r.Id,
Name = r.Name
}).ToList()
}).ToList();
}
catch (Exception ex)
{
ErrLog.LogError(ex, "ManageController.Users_Read");
}
return Json(users.ToDataSourceResult(request));
}
Action #3:
[HttpPost]
[ValidateAntiForgeryToken]
[Authorize(Roles = "DM_Admin")]
public async Task<ActionResult> Users_Edit([DataSourceRequest] DataSourceRequest request, ManageUsersViewModel model)
{
if (model != null && ModelState.IsValid)
{
// format the RolesList to type List<string> with name and List<string> with Id for entry and comparison
List<string> rolesToAssign = getRoleNameList(model);
List<string> modelRoleIds = getRoleIdList(model);
try
{
var currentUser = await UserManager.FindByIdAsync(model.Id);
// create list of current user's roles to determine if they have been modified
List<string> currentUserRoleIds = new List<string>();
foreach (var role in currentUser.Roles)
{
currentUserRoleIds.Add(role.RoleId);
}
// persist user roles to User Db if changes have been made
if (currentUserRoleIds.Except(modelRoleIds).Any() || modelRoleIds.Except(currentUserRoleIds).Any())
{
var updateRolesResult = await AssignRolesToUser(model.Id, rolesToAssign.ToArray());
if (updateRolesResult.Succeeded)
{
return RedirectToAction("ManageUsers", new { Message = ManageMessageId.AccountUpdateSuccess });
}
AddErrors(updateRolesResult);
}
// persist user info to User Db if changes have been made
else if(currentUser.Email != model.Email || currentUser.FirstName != model.FirstName || currentUser.LastName != model.LastName)
{
currentUser.UserName = model.Email;
currentUser.Email = model.Email;
currentUser.FirstName = model.FirstName;
currentUser.LastName = model.LastName;
var updateUserResult = await UserManager.UpdateAsync(currentUser);
if (updateUserResult.Succeeded)
{
return RedirectToAction("ManageUsers", new { Message = ManageMessageId.AccountUpdateSuccess });
}
AddErrors(updateUserResult);
}
// persist user password to User Db if changes have been made
else
{
var token = await UserManager.GeneratePasswordResetTokenAsync(currentUser.Id);
var updatePasswordResult = await UserManager.ResetPasswordAsync(currentUser.Id, token, model.Password);
if (updatePasswordResult.Succeeded)
{
return RedirectToAction("ManageUsers", new { Message = ManageMessageId.AccountUpdateSuccess });
}
AddErrors(updatePasswordResult);
}
}
catch (Exception ex)
{
ErrLog.LogError(ex, "ManageController.Users_Edit");
}
}
return Json(new[] { model }.AsQueryable().ToDataSourceResult(request, ModelState));
}
Action #4:
[HttpPost]
[ValidateAntiForgeryToken]
[Authorize(Roles = "DM_Admin")]
public async Task<ActionResult> Users_Delete([DataSourceRequest] DataSourceRequest request, ManageUsersViewModel model)
{
if (model != null && ModelState.IsValid)
{
// format the RolesList to type List<string> for removal
List<string> rolesToRemove = getRoleNameList(model);
try
{
// get the user to be deleted by id
var user = await UserManager.FindByIdAsync(model.Id);
// remove roles from user in User Db
var removeRolesResult = await UserManager.RemoveFromRolesAsync(user.Id, rolesToRemove.ToArray());
if (removeRolesResult.Succeeded)
{
// remove user from User Db
var removeUserResult = await UserManager.DeleteAsync(user);
if (removeUserResult.Succeeded)
{
return RedirectToAction("ManageUsers", new { Message = ManageMessageId.AccountDeleteSuccess });
}
AddErrors(removeUserResult);
}
else
{
AddErrors(removeRolesResult);
}
}
catch (Exception ex)
{
ErrLog.LogError(ex, "ManageController.Users_Delete");
}
}
return Json(new[] { model }.AsQueryable().ToDataSourceResult(request, ModelState));
}
Action #5:
[HttpPost]
[ValidateAntiForgeryToken]
[Authorize(Roles = "DM_Admin")]
public async Task<IdentityResult> AssignRolesToUser(string id, string[] rolesToAssign)
{
if (rolesToAssign != null)
{
try
{
// find the user to assign roles to
var user = await UserManager.FindByIdAsync(id);
if (user != null)
{
// check if the user currently has any roles
var currentRoles = await UserManager.GetRolesAsync(user.Id);
var rolesNotExist = rolesToAssign.Except(RoleManager.Roles.Select(x => x.Name)).ToArray();
if (!(rolesNotExist.Count() > 0))
{
// remove current roles from user, if any, in User Db
var removeRolesResult = await UserManager.RemoveFromRolesAsync(user.Id, currentRoles.ToArray());
if (!removeRolesResult.Succeeded)
{
AddErrors(removeRolesResult);
return removeRolesResult;
}
// assign new roles to user in User Db
var addRolesResult = await UserManager.AddToRolesAsync(user.Id, rolesToAssign);
if (!addRolesResult.Succeeded)
{
AddErrors(addRolesResult);
}
return addRolesResult;
}
else
{
ModelState.AddModelError("", string.Format("Roles '{0}' do not exist in the system", string.Join(",", rolesNotExist)));
}
}
else
{
ModelState.AddModelError("", string.Format("Unable to find user"));
}
}
catch (Exception ex)
{
ErrLog.LogError(ex, "ManageController.AssignRolesToUser");
}
}
else
{
ModelState.AddModelError("", string.Format("No roles specified"));
}
return null;
}
I kept the Identity framework the same as how Microsoft sets it up when creating a default Mvc application, and because of this I'm not really sure how to go about refactoring the code.
I understand this is a longer post, so thank you so much for taking the time to read it and I hope to hear from you all soon.
**Note: I added 5 action methods because I believe they may be necessary to show in order to thin out the controller as a whole, along with giving a better understanding of whats going on. But please don't feel the need to provide examples for all of the actions listed. **
Thanks so much guys! Snawwz
Aucun commentaire:
Enregistrer un commentaire