Extended Controller constructor does not have an instance of User

user_1272873 0
Viewed 184

I have a basic controller that extends from Controller, the class is working fine, but I figured that I am using a lot of times the code to get the current User from the database. So I figured I should make a constructor and move the code that I use in every function there. Basically, what I wanted to do is have the parameters ready for any of the methods in my controller.

So, this is what I have right now (and it is working fine):

public class UsersController : Controller
{
    private DBContext db = new DBContext();

    public ActionResult Info()
    {
        User user = db.Users.Where(m => m.username.Equals(User.Identity.Name)).FirstOrDefault();
        return View(user);
    }

    public ActionResult Edit(int? id){
        User user = db.Users.Where(m => m.username.Equals(User.Identity.Name)).FirstOrDefault();
        if(user.id == id){
            return View(user);
        }
    }
}

But my idea was to create something like this:

public class UsersController : Controller
{
    private DBContext db = new DBContext();
    private User _user;

    public UsersController()
    {
        _user = db.Users.Where(m => m.username.Equals(User.Identity.Name)).FirstOrDefault();
    }

    public ActionResult Info()
    {
        return View(_user);
    }

    public ActionResult Edit(int? id){
        if(_user.id == id){
            return View(_user);
        }
    }
}

When I made these changes I get the following error:

Server Error in '/' Application. Object reference not set to an instance of an object.

Description: An unhandled exception occurred during the execution of the current web request. Please review the stack trace for more information about the error and where it originated in the code.

Exception Details: System.NullReferenceException: Object reference not set to an instance of an object.

I tried debugging and I found out that the problem is that my User is null when the constructor is called, so I am guessing, some other languages can call the parent constructor before adding or after adding their own customization, for example something like this:

public function __Construct($x){
    $this->x = $x
    parent::__construct();
}

or 

public function __Construct($x){
    parent::__construct();
    $this->x = $x
}

I tried to do the same in my program, using base, but nothing seems to work and it always leads me to an error of some other nature. I am not even sure that this is the right way to do it, because all I need is to have my User (Identity) created in the constructor

extends inheritance asp.net-mvc constructor c#
2 Answers

Sounds like the user isn't found, possibly because the user identity isn't populated on the thread's principal when the constructor for the controller is called.

My suggestion would be to avoid pulling the user data in the constructor and instead grab it when you need it. To avoid duplicating code, you can write a protected or private method (not an action method) to get it:

public class UsersController : Controller
{
    private DBContext db = new DBContext();

    private User GetCurrentUser()
    {
        return db.Users.Where(m => m.username.Equals(User.Identity.Name)).FirstOrDefault();
    }

    public ActionResult Info()
    {
        var user = GetCurrentUser();
        return View(user);
    }

    public ActionResult Edit(int? id){
        var user = GetCurrentUser();
        if(user.id == id){
            return View(user);
        }
    }
}
user_27915400

As I mentioned in my question comment, inheritance is a poor choice here. Instead what you're attempting to do is give non-specific data to a View. A better choice is to use an ActionFilter.

We need a class to store User Information for the view to consume:

public class UserInfo
{
  public bool HasUser { get; set; }
  public User User { get; set; }
}

We need a place to store the data that is non-specific to views. I prefer using ViewData (because this route provides strongly typed data and an easy way to debug this storage location):

public static class ViewDataExtensions
{
   private const string UserInfoKey ="_UserInfo";

   public static void GetUserInfo(this ViewData viewData)
   {
     return viewData.ContainsKey(UserInfoKey)
       ? viewData[UserInfoKey] as UserInfo
       : null;
   }

   public static UserInfo SetUserInfo(this ViewData viewData, UserInfo userInfo)
   {
     viewData[UserInfoKey];
   }
}

Next we need a way to populate that information when needed

public class AddUserToViewDataFilterAttribute : ActionFilterAttribute
{
    private DBContext db = new DBContext();

    public void OnActionExecuting(ActionExecutingContext context)
    {
      var user = context.Controller.User;

      var userInfo = new UserInfo
      {
        HasUser = !string.IsNullOrEmpty(User.Identity?.Name),
        User = !string.IsNullOrEmpty(User.Identity?.Name)
          ? db.Users
        .Where(m => m.username.Equals(User.Identity.Name)).FirstOrDefault()
          : null;
      };

      context.ControllerContext.ViewData.SetUserInfo(userInfo);
    }
}

Populate it when needed:

public class MyController
{
  public ActionResult DoesNotNeedUserInfo()
  {
  }

  [AddUserToViewDataFilter]
  public ActionResult NeedsUserInfo()
  {
  }
}

In the view:

@model <whatever>
@if (ViewData.GetUserInfo().HasUser) {
  <div>@ViewData.GetUserInfo().User.Name</div>
}
user_2092590
Related