Constructor not called consistently

Question

I have a controller which inherits from an abstract secure controller which holds a user object as below.

public new User User
{
  get
  {
    if (this.user == null)
    {
      var id = int.Parse(base.User.Identity.Name, CultureInfo.InvariantCulture);
      this.user = this.UserRepository.FindById(id);
    }

    return this.user;
  }
}

Every time I make a call to the following function I receive a null exception for the above this.UserRepository

[UrlRoute(Path = "api/stats/events/visits/accounttype/{idList}")]
[UrlRoute(Path = "api/{idList}/stats/events/visits/accounttype")]
[UrlRouteParameterDefault(Name = "idList", Value = "")]
public virtual ActionResult Vsat(string idList, DateTime? startDate, DateTime? endDate)
{
  // get the ids from the url and retrieve a list of events for those user/s
  var ids = (from id in idList.Split(',') where !string.IsNullOrEmpty(id) select Convert.ToInt64(id)).ToList();
  var allEvents = this.eventRepository.FindForCompanyBetweenDatesForUsers(
      this.User.Company.Id, new List<EventType> { EventType.Visit }, startDate, endDate, ids).ToList();

  var groupResults = allEvents.GroupBy(x => x.Account.AccountType.Name);

  return null;
}

Even though my Api constructor calls the base constructor of Secure Controller like so

public ApiController(IUserRepository userRepository) : base(userRepository)
{
}

protected SecureController(IUserRepository userRepository)
{
  this.UserRepository = userRepository;
}

What's even more strange is that there are other functions on the page which reference this.User and none of them return null the same exception. They hit the secure constructor, then the api constructor and then the function. The above Vsat function (naming just for testing purposes) hits the functions, then breaks on the line

this.user = this.UserRepository.FindById(id);

As well as that, If I place a similar function above that, it works, but the new function then has the same problem.

Edit

Created a new class and the function works perfectly.

public class TestController : SecureController
  {
    private readonly IEventRepository eventRepository;

    public TestController(IUserRepository userRepository, IEventRepository eventRepository) : base(userRepository)
    {
      this.eventRepository = eventRepository;
    }

    [UrlRoute(Path = "test/stats/events/visits/accounttype/{idList}")]
    [UrlRoute(Path = "test/{idList}/stats/events/visits/accounttype")]
    [UrlRouteParameterDefault(Name = "idList", Value = "")]
    public virtual ActionResult Vsat(string idList, DateTime? startDate, DateTime? endDate)
    {
      // get the ids from the url and retrieve a list of events for those user/s
      var ids = (from id in idList.Split(',') where !string.IsNullOrEmpty(id) select Convert.ToInt64(id)).ToList();
      var allEvents = this.eventRepository.FindForCompanyBetweenDatesForUsers(
        this.User.Company.Id, new List<EventType> {EventType.Visit}, startDate, endDate, ids).ToList();

      var groupResults = allEvents.GroupBy(x => x.Account.AccountType.Name);

      return null;
    }
  }

Can you isolate the problem into a more simple, reproducible test case? — Travis Gockel, Aug 03 '11 at 07:52
@JLevett I believe this might just be a simple case of your base constructor being given a null reference in certain circumstances. On the `UserRespository` property setter, try testing the incoming `value` (`if (value == null) throw new Exception("null! why?");`) and throw a specific exception if it's null, just for sanity checking. — Adam Houldsworth, Aug 03 '11 at 08:13

score 3 · Answer 1 · edited May 23 '17 at 11:55

3

I'm not sure there is enough code to understand exactly where the problem is coming in, but I noticed that your User property uses the new keyword to hide the property below it. This will only work if the calling code uses a reference to ApiController (I assume this is the type with the new User property?).

If you have code that uses SecureController and tries to access User it will not touch the null checking code you have implemented around this.user.

As for inconsistent constructor calls, I can say that if you are new-ing an object, all relevant constructors on the inheritance path will be called, unless an exception is thrown.

See here for an overview about Member Hiding using the new keyword, and associated pitfalls:

http://msdn.microsoft.com/en-us/library/Aa691135

Update: I suspect Ninject is playing silly-buggers with your object life span. Either that or construction is doing something funky like this:

How does WCF deserialization instantiate objects without calling a constructor?

I'd say breakpoint your base class constructor and make sure the object provided is not null. Without seeing any more code it's hard to answer. Try creating another question, but focus more on Ninject being a possible source of the problem.

edited May 23 '17 at 11:55

Community

1
1

answered Aug 03 '11 at 07:50

Adam Houldsworth

63,413
11
150
187

Thank you for the reply. Removing the new keyword causes a warning, "The keyword new is required on user because it hides property IPrincipal System.Web.Mvc.Controller.User. It really should hit the base constructor before reaching the User property – JConstantine Aug 03 '11 at 08:04
Only if the object is constructed prior to use, if you are re-using an available object it won't. What you have done is hide a member of a class below. If your goal is to tailor the behaviour of that property, the base class should mark it `virtual` and the derived class should `override` it. – Adam Houldsworth Aug 03 '11 at 08:08
@JLevett I'd assume then that the argument you are passing into the base constructor is null, as the code looks like it should work - although the member hiding is a possible problem point. – Adam Houldsworth Aug 03 '11 at 08:09
Agreed. Lets forget the User property as that is not causing the issue here. Simply passing in 1 instead of this.User.Company.Id to the FindForCompany function stops the error from occurring. The argument passed to the constructor is provided using ninject, but as previously mentioned, neither constructor is called at any point when trying to access this function, meaning that the value will never be set in the SecureController class – JConstantine Aug 03 '11 at 08:17
@JLevett ah Ninject, have you put this object in singleton mode or caching it at all? Breakpoint the constructor and **make sure** the user repository argument is being provided not null. You have more of a problem with object lifecycle here than code structure. – Adam Houldsworth Aug 03 '11 at 08:21
So many different things working together makes it hard to keep on top of them all. Thank you for your help, it appears to be something completely different which was causing the problem. – JConstantine Aug 03 '11 at 08:34

score 2 · Accepted Answer · answered Aug 03 '11 at 08:33

2

Sorry for the confusion, it turns out the problem was cause by t4mvc templates.

After changing the ApiController the code generation templates needed to be re-ran to reflect the changes.

The constructors are now fired as expected.

answered Aug 03 '11 at 08:33

JConstantine

3,980
1
33
46

Constructor not called consistently

2 Answers2