-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Can't resolve some controller parameters if scanning in the registry #19
Comments
Hmm. That's really weird. I'll take a look at it when I get home from work. What happens if you set a breakpoint in |
I'm not sure what you mean by
How can that work? The wrapper registration won't get registered then. @jeremydmiller If you call |
@khellang Actually it does not lose the registrations because the OTOH, calling |
@mcliment Check your |
@jeremydmiller I don't see any major difference in any case, the results are on this gist: https://gist.github.com/mcliment/c3ae3e08c978996be1f390004fdde971 I also tried to add the |
@mcliment I need a little more context. What exactly isn't working correctly? |
@jeremydmiller If you have some time to check out this repository (https://github.com/mcliment/StructureMap.Microsoft.DependencyInjection/tree/issue-demo) and run the demo site you'll see the error live. Just go to |
Something leads me to believe that this is a StructureMap bug related to |
That's not a bug, that's behavior as designed. To keep the performance within reasonable limits with I never use TryGetInstance in my own development and constantly recommend against using it, but the clowns on the ASP.Net team insist upon it for their goofy fallback mechanism. |
Haha. Right. I think we're getting somewhere... Now we just need to figure out what we can do about this. I'm afraid this will get very hairy. |
@khellang Hmmm, I understand that but I can't see why not scanning things overcomes the problem if the same stuff is registered |
If it's scanning, can you look at the WhatDoIHave() and see if there are multiple registrations for whatever service this is? Because in that case, StructureMap doesn't automatically select the first or last one like Windsor or Autofac does. In the case of the TryGetInstance, SM just punts because there's no explicit default. |
@mcliment @khellang Not doing anything until January, but I'm gonna propose a new Might add something to the diagnostics to assert when there's multiple registrations, or maybe something that can verify that there is a default for everything expected. Or something...... |
@jeremydmiller Any new thoughts on this, or work-arounds? I'm trying out core 2.0 and deciding on a DI framework and ran into this issue. We currently use Ninject (which has basically nothing going in the core arena) and rely heavily on being able to instantiate concrete classes without explicit registration. My experience has been much the same:
Thanks for any insight! |
You can thank the ASP.Net <http://asp.net/> team for this one. If you were using StructureMap idiomatically, that would have worked just fine. The ASP.Net <http://asp.net/> Core adapter is the equivalent to StructureMap’s Container.TryGetInstance(), which will return a null because you don’t have any explicit registration. Container.GetInstance() would work as well.
If necessary, I could show you how to toss in an IFamilyPolicy that would force SM to resolve missing concrete types in the TryGetInstance() logic.
- Jeremy
… On Oct 4, 2017, at 2:41 PM, Brian ***@***.***> wrote:
@jeremydmiller <https://github.com/jeremydmiller> Any new thoughts on this, or work-arounds? I'm trying out core 2.0 and deciding on a DI framework and ran into this issue.
We currently use Ninject (which has basically nothing going in the core arena) and rely heavily on being able to instantiate concrete classes without explicit registration.
My experience has been much the same:
Setup using .UseStructureMap()
Add concrete Dummy class to my Controller's constructor
Run as-is with no config: "Unable to resolve service for type 'Dummy'"
ConfigureServices -> services.AddTransient<Dummy>(): Works
ConfigureContainer -> registry.ForConcreteType<Dummy>(): Works
container.GetInstance<Dummy>() to "prime" it (when I had it setup the IServiceProvider ConfigureServices( IServiceCollection services ) way): Works
Thanks for any insight!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#19 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAK8C7NJ8MnlXmQKXA8cBMoZ1eqdk1mjks5so99sgaJpZM4LTENG>.
|
I've seen a lot of similar complaints about how the ASP.Net team handled the DI stuff in Core :( I've been a bit hesitant in general about switching, but the tech marches forward and I figure I should at least keep somewhat abreast of the way things are headed. I'd be really curious to see how you'd implement it, if it's not too much trouble. Based on your message I started trying my hand at making an IFamilyPolicy and I got it wired in, but my very naive Build method throws exceptions :) Specifically, for:
I get:
Thanks for the insight! |
I've created an public class OptionsPolicy : IFamilyPolicy
{
public PluginFamily Build(Type type)
{
if (type.Name == "IOptions`1")
{
var family = new PluginFamily(type);
var instance = BuildInstance(type);
family.SetDefault(instance);
return family;
}
return null;
}
private Instance BuildInstance(Type type)
{
var instanceType = typeof(OptionsInstance<,>).MakeGenericType(type, type.GetGenericArguments()[0]);
return Activator.CreateInstance(instanceType) as Instance;
}
public bool AppliesToHasFamilyChecks => true;
public class OptionsInstance<T, TOptions> : LambdaInstance<T>
where T : class, IOptions<TOptions>
where TOptions : class, new()
{
public OptionsInstance() :
base($"Building {typeof(T).FullName} from options", c => c.GetInstance<OptionsManager<TOptions>>() as T)
{
}
}
} Then just use it in a Registry like this:
|
@khellang @jeremydmiller We've historically had to work around this issue with a custom fork of StructureMap.Microsoft.DependencyInjection. Ideally, we'd like to shelve that fork; but short of playing wack-a-mole with missing family policies to cover the gap between GetService vs GetRequiredService when upstream code is insistent on calling the former, I don't see a resolution. Edit: Now that I've just updated the fork, I understand a little better. Having an option to switch between the ServiceProvider calling Container.TryGetInstance vs Container.GetInstance means that calls to GetRequiredService may be left with suboptimal exception messages -- compared with StructureMap's excellent internal exceptions. |
When scanning for things in the registry, some types can't be resolved in the controller constructor (in the case of the example IOptions but I don't think it's exclusive to this type).
I have pushed some code to reproduce the issue:
https://github.com/mcliment/StructureMap.Microsoft.DependencyInjection/tree/issue-demo
To reproduce, run the sample site and go to
/Bad
. This should throw an exception because the typeIOptions<MySettings>
can't be resolved. Then go to/Good
and should work, loading the same type through a wrapper. Then "/Bad" works perfectly.The thing it that
TryGetInstance
can't resolve the type the first time (GetInstance
works properly).If the
registry.Scan
is removed fromConfigureContainer
and the typeMySettingsWrapper
is added asservices.AddTransient<IMySettingsWrapper, MySettingsWrapper>();
then/Bad
works the first time.As well, if the Scan has the clause
a.Exclude(t -> true);
instead of theInclude(...)
also works the first time.The text was updated successfully, but these errors were encountered: