We have moved to GitHub Issues
Created by simone chiaretta 23 Feb 2017, 16:31:18 Updated by andrei 06 Nov 2017, 21:46:06Tags: Unscheduled PR
Relates to: U4-10625
Relates to: U4-10642
I installed in my application Unity for WebAPI and registered the controller as recommended on the [https://our.umbraco.org/forum/getting-started/installing-umbraco/53839-Unable-To-Upgrade-When-Using-Unity-IOC-The-type-UmbracoContext-does-not-have-an-accessible-constructor forums]
container.RegisterType<UmbracoContext>( new ContainerControlledLifetimeManager(), new InjectionFactory(c => UmbracoContext.Current));
But when then I get an error when I try to load the developer section as the
HealthCheckController raises an error.
An error occurred when trying to create a controller of type 'HealthCheckController'. Make sure that the controller has a parameterless public constructor.
System.InvalidOperationException: An error occurred when trying to create a controller of type 'HealthCheckController'. Make sure that the controller has a parameterless public constructor.
Microsoft.Practices.Unity.ResolutionFailedException: Resolution of the dependency failed, type = "Umbraco.Web.HealthCheck.HealthCheckController", name = "(none)". Exception occurred while: while resolving. Exception is: InvalidOperationException - The current type, Umbraco.Web.HealthCheck.IHealthCheckResolver, is an interface and cannot be constructed. Are you missing a type mapping?
At the time of the exception, the container was:
Resolving Umbraco.Web.HealthCheck.HealthCheckController,(none) Resolving parameter "healthCheckResolver" of constructor Umbraco.Web.HealthCheck.HealthCheckController(Umbraco.Web.HealthCheck.IHealthCheckResolver healthCheckResolver) Resolving Umbraco.Web.HealthCheck.IHealthCheckResolver,(none)
Only way to make it work is to force Unity to construct the controller using the parameterless constructor.
This is kind of weird as the HealthCheck controller is the only one that gives this problem: all others work fine.
I think the difference is that all other controllers use the
Services class that is part of the
UmbracoApiControllerBase while the
HealthCheckController manages dependencies in its own way.
I also had to register ContentTypeController MediaController MediaTypeController to get the media section working
but everything seems to be 100% after that
According to the docs here https://our.umbraco.org/documentation/Reference/Common-Pitfalls/ UmbracoContext is request scoped and depends directly on HttpContext.
If you look into the code you can see the parameter that is required for the HealthCheckController https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/src/Umbraco.Web/HealthCheck/HealthCheckController.cs#L21
So you probably aren't registering a IHealthCheckResolver in your container.
Using the parameterless constructor sort of voids the whole point of IoC ;)
@Shandem I believe it's not registering due to the fact that the HealthCheckResolver class is marked internal.
Yes that is definitely part of the problem. Not sure how Unity works but seeing as though the error says:
The current type, Umbraco.Web.HealthCheck.IHealthCheckResolver, is an interface and cannot be constructed. Are you missing a type mapping?
It sounds like the container hasn't registered an instance of
IHealthCheckResolver, in most containers you need to specify the instance like:
Container.Register<IHealthCheckResolver, HealthCheckResolver>(); which if that had been done we probably would have already known that
HealthCheckResolver being internal was a problem.
Would be happy to accept a PR to make HealthCheckResolver public which will solve this problem.
I'll take a look this week :)
Saw your PR and have merged it :) so that will be available in 7.6.7 which (i think) should be released on Tuesday
I've set to 'fixed' since ultimately that should fix the issue but please test and report back. Cheers!
I think the point here is:
Just got into this issue myself, using SimpleInjector IOC. Is the temporary fix registering IHealthCheckResolver, like: Container.Register<IHealthCheckResolver, HealthCheckResolver>()?
[Update] - no, that doesn't work: An error occurred when trying to create a controller of type 'HealthCheckController'. Make sure that the controller has a parameterless public constructor.
The constructor of type HealthCheckResolver contains the parameter with name 'logger' and type ILogger that is not registered. Please ensure ILogger is registered, or change the constructor of HealthCheckResolver.
I'm in version 7.7.1, any ideas?
@amv Your error message says you need register an instance of ILogger in your container, so you should register one.
Yes, I would have solved by registering ILogger, but there is a second parameter to HealthCheckResolver constructor which I didn't know how to provide.
@amv right, you can see the construction of the HealthCheckResolver in the codebase here: https://github.com/umbraco/Umbraco-CMS/blob/e0025db56d52b770d2b3aedbd48a3b804fd15ef0/src/Umbraco.Web/WebBootManager.cs so you could use that same code as a delegate to register the instance.
@simonech yes i agree it's something that has been discussed for a long time and some community members had planned on making different Nuget packages for just that purpose but nothing has transpired yet.
So, just gave it another shot. Looking at source code, but also with reflector, trying to do the same, I end up trying something like this:
@aamv where/when are you creating your container? You should be creating your container on ApplicationStarted, see https://our.umbraco.org/documentation/reference/using-ioc . At this stage, PluginManager.Current will exist. Also note that when registering IHealthCheckResolver as a delegate it won't be resolved until it's needed so it seems mighty strange that your container is trying to resolve an instance before PlugingManager.Current is inititalized. You should definitely not try re-creating a new instance of PluginManager.
What you've done here is on the right path but this shouldn't be scoped, this should be a singleton and you don't need the optional parameters for ResolveTypes.
container.Register<IHealthCheckResolver>(()=> new HealthCheckResolver(LoggerResolver.Current.Logger, () => PluginManager.Current.ResolveTypes<HealthCheck>(true, null)), Lifestyle.Scoped);
Hi @Shandem, you're right, I was creating my container in ApplicationStarting. So the good news is that after moving it to ApplicationStarted, the IOC was able to initialize a HealthCheckRezolver and subsequently a HealthCheckController instance. The bad news is that HealthCheckController only seems to work when initialized with the parameter-less constructor. The second constructor that takes a IHealthCheckResolver parameter, will not initialize some private variables (healthCheckConfig and disableCheckIds) and will throw a null reference when GetAllHealthChecks() gets called in Developer section. I guess this is a bug?
Anyway, I fixed it by making my IOC to use the parameter-less constructor for HealthCheckController type. That also makes the IOC registration for IHealthCheckResolver no longer necessary.
@amv So i realized that i was wrong for registering a IHealthCheckResolver in the container, all you need to do for that is :
@Shandem that looks much cleaner. However, no reason to register it with this version (7.7.1) - as described in my previous comment, HealthCheckController only works if created with default constructor, otherwise it will throw null reference when GetAllHealthChecks() gets hit. I think this is a bug - HealthCheckController second constructor 'forgets' to initialize same variables.
@amv ouch! yes it does, i will fix up asap.
Just hit another problematic controller - LegacyTreeController. It happens under members section when trying to expand Member groups or trying to create a new member group:
SimpleInjector.ActivationException: The constructor of type LegacyTreeController contains the parameter with name >'xmlTreeNode' and type XmlTreeNode that is not registered. Please ensure XmlTreeNode is registered, or change the constructor of LegacyTreeController. (screenshot attached)
I believe the controller was intended to be used with empty constructor when automatically activated. However, the IOC will complain as it doesn't know that.
Perhaps second constructor should be internal, not public? It is used only from another internal method anyway.
Backwards Compatible: True
Due in version: 7.6.7
Sprint: Sprint 67