If you're going to be separating the responsibilities, then you should separate all responsibilities.
Your UdpateUserProfile
method should be agnostic of where it was called from. If you want to add a WPF client down the line, you shouldn't have to change this class at all. In that situation, you won't be returning an IHttpActionResult
, you'll be doing something else.
Therefore, remove that dependency from your method. Have it notify that its task was successful, or not. In that situation, a bool
would probably be a better fit for a return value. If you want to return additional information, you can create a simple model to encapsulate any other data you want to return.
public class AuthorizationResult
{
public bool Result { get; set; }
public string Message { get; set; }
public AuthorizationResult()
{
Result = true;
}
public AuthorizationResult(string errorMessage)
{
Result = false;
Message = errorMessage;
}
}
Then inside your service.
public async Task<AuthorizationResult> UpdateUserProfile(UserProfile profile)
{
try
{
var ups = ApiServiceFactory.GetUserProfileService();
var result = ups.UpdateUserProfile(profile);
return new AuthorizationResult();
}
catch (Exception ex)
{
// Just an example of how to get a message.
// Depending on your implementation, you might be returning a
// message from UpdateUserProfile(profile).
return new AuthorizationResult(ex.Message);
}
}
Then, inside your API controller, that is when you tightly couple it to the technology, because it is being directly used there. Your check to see if the user is authenticated should also be included here, as your service won't know anything about the mechanics of authenticating the user.
var result = HttpContext.Current.Request.IsAuthenticated ?
separateClass.UpdatedUserProfile(profile) :
new AuthorizationResult("User is not authenticated");
return result.Result ? Ok() : Unauthorized();
Judging by the return type of your Profile Service, it sounds like you need to refactor that UpdateUserProfile()
method as well to remove the dependency there also.
For best security, you should not display any specific reason why the user was unable to be updated. However, that should definitely be logged somewhere so you can keep track of any unauthorized access to your system.