mardi 3 octobre 2017

Thin Out Controller in Asp.Net-Mvc-5 Specific Example

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