Is it the right way to use IHttpContextAccessor in a class?

Question

I am working on an Asp.Net Core web application running on .NET Core 3.1.

For a wizard form, I am relying on a User class that holds the data stored in session. As we can access HttpContext from a controller, I had this GetUser() method inside my controller to retrieve session data when necessary :

private User GetUser()
{
    var session = HttpContext.Session.GetComplexData<User>("user");

    if (session == null)
    {
        User user = new User();
        HttpContext.Session.SetComplexData("user", user);
    }
    return (User)HttpContext.Session.GetComplexData<User>("user");
}

Then I wanted to move that GetUser() method inside the User class so I used DI to provide that class's ctor with IHttpContextAccessor :

public class User
{
    private readonly IHttpContextAccessor _contextAccessor;

    public User(IHttpContextAccessor contextAccessor)
    {
        _contextAccessor = contextAccessor;
    }

    // fields removed here to keep it short

    public User GetUser()
    {
        var session = _contextAccessor.HttpContext.Session.GetComplexData<User>("user");

        if (session == null)
        {
            User user = new User(_contextAccessor);
            _contextAccessor.HttpContext.Session.SetComplexData("user", user);
        }
        return (User)_contextAccessor.HttpContext.Session.GetComplexData<User>("user");
    }
}

But then, in order to use that GetUser() method in my controller(s), I also have to provide this controller's ctor with IHttpContextAccessor as my User needs it to be instantiated :

public class NewController : Controller
{
    private readonly IHttpContextAccessor _contextAccessor;

    public NewController(IHttpContextAccessor contextAccessor)
    {
        _contextAccessor = contextAccessor;     
    }

    [HttpGet]
    public IActionResult Step1()
    {
        // Get session data
        User user = new User(_contextAccessor);  // <--
        user = user.GetUser();

        // Other code...
        // ---
        return View("Step1");
    }
}

So my question is... Is it the right way to do it?

Or shall I just stick with my very first GetMethod() inside the controller without bothering with DI and duplicate it in other controllers if I need to access session there...?

Or perhaps you can show me something I don't know that would be more a good practice..

THanks

Answer 1

You don't want your User class to be tightly coupled to the HttpContext so your first method would be better. However, to improve readability and reusability (is that a word?) you could create users through an interface IUserManager and use dependency injection to provide the session there. The class would look something like this:

public class UserManager : IUserManager
{
     private readonly IHttpContextAccessor _httpContextAccessor;

     public UserManager(IHttpContextAccessor httpContextAccessor)
     {
         _httpContextAccessor = httpContextAccessor;
     }

     public User Create()
     {
          // create user here using session in _httpContextAccessor
     }    
}

In order for this to work, don't forget to register the necessary dependencies in your container:

public void ConfigureServices(IServiceCollection services)
{
     services.AddHttpContextAccessor();
     services.AddTransient<IUserManager, UserManager>();
}

Source material: https://docs.microsoft.com/en-us/aspnet/core/fundamentals/http-context?view=aspnetcore-3.1#use-httpcontext-from-custom-components.