Suggestions for improving integration of non-conformers
dotnetjunkie opened this issue · 43 comments
During the conversation #416 we came to the conclusion that not everybody can conform to the DI abstraction that Microsoft provides when working with ASP.NET Core (commonly referred to as the Conforming Container) and that it is important to have good support for this scenario as well. We established that the integration for these 'non-conformers' is unfortunately less than optimal in ASP.NET Core 1.0 and ASP.NET Core MVC 1.0. As it currently stands, there is no adapter for Ninject, Unity, Castle Windsor and Simple Injector and Autofac only has a partly-working adapter.
NOTE: This issue assumes the reader to be familiar with what it means to be non-conforming and why developers might need or want to do this.
This issue contains suggestions for simple improvements to ASP.NET Core v1.1 (or up) and ASP.NET Core MVC 1.1 (or up). This issue is accompanied by this Github repo, which contains a set of extension methods that allow streamlining integration for non-conformers and contains sample projects for the 6 major DI libraries (and one for Pure DI) to show how those libraries can be plugged in using these extension methods. These extension methods are meant as a strarting point for further discussion.
This issue is meant as a starting point in helping to simplify integration for developers that don't conform. This issue is not meant to fuel a discussion about whether or not we should use a conforming container or not; that's up to every individual developer to decide. The fact is however that not everybody can conform, while the current integration strategy for those non-conformers is far from ideal. This issue tries to address this.
The suggested extension methods can be divided into 4 groups:
- Extension for wrapping requests in a custom scope (Core)
- Extensions for simplifying retrieval of request specific framework components (Core)
- Extensions for simplifying replacing common factory abstractions (MVC)
- Extensions for simplifying retrieval of lists of common root object types (MVC)
Note that the set of extension methods are not meant to give 100% 'coverage', and this is not needed. As the Framework Design Guidelines stated a long time ago: "A well-designed framework makes simple scenarios easy" while allowing advanced scenarios as well (with possibly a bit more work for the user). Although we might need to add some extensions as we go, the current set will already allow developers to set up an ASP.NET Core MVC application in a non-conforming way. They still might need to create custom implementations for special cases.
ASP.NET Core extensions
Extension for wrapping requests in a custom scope
A very common scenario for a developer is to require a certain scoping of objects. For this reason the old Web API framework contained an IDependencyScope abstraction. For the same reason Microsoft.Extensions.DependencyInjection contains the concept of a Scoped lifestyle. In case we let the application container manage the lifetime of instances, we need to instruct the container to create a new scope. Some containers (like Simple Injector and Castle Windsor) allow those scopes to be ambient, while other containers (Autofac, StructureMap and Unity) require the developer to resolve directly from the scope or child container.
To facilitate this, the Github project contains an AddRequestScopingMiddleware extension method that allows wrapping a request with a middleware lambda that starts and disposes such scope. Here's an example of its usage for Castle Windsor:
services.AddRequestScopingMiddleware(container.BeginScope);Under the covers this extension method registers a IStartupFilter to the IServiceCollection. When applied, this start-up filter will apply some middleware lambda to the IApplicationBuilder that will wrap the request in a using block that creates and disposes the scope.
By using a start-up filter, we ensure that the scope will be applied before all other custom middleware components that the user might register. This ensures that the user's middleware will not accidentally run outside the scope.
Extensions for simplifying retrieval of request specific framework components
An important concept for a non-conformer is the concept of cross-wiring. An application might have its own container for building up application components, but at some point in time either application components or user defined adapters require the use of framework or 3rd party components that are still built-up by the ASP.NET configuration system. Since it is possible for any party (both framework code and 3rd party components) to replace built-in framework components, an application developer can't make any assumptions of the lifestyle of such component. Since such framework or 3rd party component can be Scoped or Transient, the developer should resolve such component at runtime directly from the configuration system to prevent common problems like Captive Dependencies. ASP.NET does currently not provide an obvious method for resolving request-scoped components.
To simplify this very common scenario, the Github repository contains two extension methods that allow resolving directly from IApplicationBuilder, while still resolving request specific values. Here's an example of its usage:
app.GetRequestService<IViewBufferScope>();
app.GetRequiredRequestService<IViewBufferScope>();Internally these methods map to GetService and GetRequiredService respectivily. In other words, GetRequestService can return null, while GetRequiredRequestService will never return null, but will throw an exception instead.
Such extension method can be used to cross-wire in the application container. Here's an example using Autofac:
builder.Register(_ => app.GetRequiredRequestService<IViewBufferScope>());These extension methods work by requesting the IHttpContextAccessor from the IApplicationBuilder's ApplicationServices property and use the accessor's HttpContext property to resolve RequestServices.
ASP.NET Core MVC specific extensions
The previous extension methods where not specific to MVC and when added to the framework, they can be used for any framework that is built on top of ASP.NET Core. But on top of these methods, the integration story for MVC needs to be addressed as well.
Extensions for simplifying replacing common factory abstractions
As far as I can see, ASP.NET Core MVC contains all the required abstractions that allow plugging in at any stage in the pipeline and intercept the creation of objects. This is important, because when using a container to build up application components, the graph usually starts at a type like a controller or view component. This means that MVC contains all it takes to let non-conformers plug-in. Unfortunately this always requires the creation of custom classes.
To make the experience smoother, some extension methods need to be added that cover the most common scenario's like intercepting the creation of Controllers and View Components. Other extension methods can be added as well, but it is fine for a developer to have to do a little more work when dealing with less common interception points.
A few extension methods have been added to the repository to allow redirecting the creation of root types like Controllers and View Components to the application-container. The example shows the usage of two extension methods using Simple Injector:
services.AddCustomControllerActivation(container.GetInstance);
services.AddCustomViewComponentActivation(container.GetInstance);Under the covers these extension methods register a custom IControllerActivator or IViewComponentActivator into the IServiceCollection that wraps the supplied delegate.
Extensions for simplifying retrieval of lists of common root object types
When the user intercepts the creation of a certain family of root types (such as controllers) to the application container, this might require him to register those types in that container as well. There are several reasons why the user might want to do this:
- Some DI containers require the registration of concrete types (Autofac and Castle Windsor)
- Some DI containers will not automatically dispose concrete unregistered types at the end of the request (everything but StructureMap), which is problematic for controllers, since the Controller base class implements
IDisposable. - Some DI containers promote the registration of all root types to allow the configuration to be analysed and diagnosed (Simple Injector).
Retrieving Type objects for a family of root types however takes quite some code, while almost all containers require their registration. A simple extension method per root type will solve this problem. The following code snippet shows two extension methods while registering those types in Autofac:
builder.RegisterTypes(app.GetControllerTypes()).InstancePerRequest();
builder.RegisterTypes(app.GetViewComponentTypes());Both extension methods use the ApplicationPartManager internally to populate the family of types they require.
Code
The full code listing for the discussed extension methods can be found here.
The repository contains a set of sample applications where the concept is explained for each container. The Start-up classes that contain the container specific registrations can be found here:
Summary
Although this might not be complete set, these extension methods will simplify plug-in in any container for both ASP.NET Core and the MVC framework. This will make it easier to get started with ASP.NET Core MVC when the user can't conform, or when more flexibility is required.
/cc @davidfowl
These aren't really MVC specific but that fine I guess. Thanks for this list! We'll get back to you after looking through it.
PS: Those extension methods aren't the cleanest approach but we'll look for ways to expose the right set of primitives.
Those extension methods aren't the cleanest approach
Let's discuss how we can improve this. Whether or not we use extension methods is irrelevant to me. The only important part is to make integration as easy as possible.
Yes, I'm thinking adding decorator support to the built in DI would make this more seamless.
Adding decorator support is obviously a good idea (but possibly breaking all conformers), but I'm unsure how this would solve the problems that this issue addresses. Could you show some examples of how you think adding decorator support would streamline integration?
Adding decorator support is obviously a good idea (but possibly breaking all conformers)
The reason we didn't add it is because of that but it wouldn't be something that we would take a dependency on. User code would just use to adapt built ins.
Could you show some examples of how you think adding decorator support would streamline integration?
I was thinking it might help cross wiring but that's just gross regardless. I'm not fan of exposing top level extension methods on IApplicationBuilder either (especially by default). There's just no way to get instances or per request things because it assumes that the framework exposes singletons that you can just grab and that's not the case. So I think you're going to have to keep what you currently have.
Extension for wrapping requests in a custom scope
The technique you use here is a good one. We do this ourselves (as I'm sure you know). Not sure we need to do anything here. This would be part of an uber call to services.AddSimpleInjectorSupport() so it doesn't need to be a first class extension method that everyone sees.
Extensions for simplifying retrieval of request specific framework components
These extension methods work by requesting the IHttpContextAccessor from the IApplicationBuilder's ApplicationServices property and use the accessor's HttpContext property to resolve RequestServices.
I'm assuming you found this aspnet/HttpAbstractions#723 as a result of doing that? Unfortunately I don't think there's a workaround for this, what you're doing is the only way to make this work. In ASP.NET 4.x you'd use the HttpContext.Current to do similar things.
builder.Register(_ => app.GetRequiredRequestService<IViewBufferScope>());This is also part of your shim right? Users don't need to be exposed to app.GetRequiredRequestService. It's just something that DI containers need to do to write up into the System.
In short, these things all look good but I'm super interested in things you were 100% unable to make work. I believe you were actually able to make most of the things you mention work (albeit not 100% clean) right?
@dotnetjunkie Can you via service registration have a single entry point that allows you to configure all the required things to make simple injector work?
public class Startup
{
public void ConfigureServices(IServiceCollection services)
{
// Core simple injector support
services.AddSimpleInjector();
// MVC simple injector support
services.AddMvc()
.AddSimpleInjector();
}
}Extension for wrapping requests in a custom scope
The hosting already takes care of this through IServiceScopeFactory and IServiceScope, no? Is implementing these what you're trying to avoid?
The hosting already takes care of this through IServiceScopeFactory and IServiceScope, no? Is implementing these what you're trying to avoid?
Yes, this is the non conforming container approach so there's 2 containers living side by side. But a custom IStartupFilter is the way to go.
Not sure we need to do anything here. This would be part of an uber call to services.AddSimpleInjectorSupport() so it doesn't need to be a first class extension method that everyone sees.
Can you via service registration have a single entry point that allows you to configure all the required things to make simple injector work?
These comments assume specific integration libraries provided by the DI container. This is problematic, because it preserves the endless permutation of NuGet packages for each combination of DI library and framework that you tried so hard to fix by introducing the Conforming Container. My suggestions are meant to free us all from this pain of having to maintain integration packages (not only the conforming part of the spectrum), by instead letting framework builders define a few simple extension points. Although the extension methods might not be considered "the cleanest approach", as I showed in the Github repository, they allow plugging in any container (or Pure DI enthusiast) without any integration package. Feel free though to rearrange, rename, renamespace, or whatever is needed to make it become as clean as possible.
The final extension points can serve as blueprint for developers of other frameworks. This is important, because we already see other frameworks copy just the Conforming Container approach of MVC, without thinking about non-conformers at all.
Users don't need to be exposed to app.GetRequiredRequestService. It's just something that DI containers need to do to write up into the System.
This would be rather problematic, because not only would this force every container to have an integration package for every framework, but it also requires every possible framework supplied service to be registered into the application container using the integration package. And if we do this for every framework, we would have to do the same for the endless number of 3rd party components that will exist in the near future. And don’t forget that frameworks and 3rd party components might have complex rules that decide whether to register something or not. This means that those integration packages must copy this logic, because it can’t simply cross-wire everything, because that could trip up any verification that the user might do on his container; registrations that can’t be resolved would cause the user’s verification to break. The only way around this is to build integration packages that read the list of IServiceCollection registrations, sending us back to square one: back to the Conforming Container adapter.
Don’t forget that most users will need to cross-wire just a handful of framework components. My guess is that 90% of developers will not need more than 10 of those cross-wired dependencies. Cross-wiring those services will therefore not cause any maintenance problem and at the same time makes it really explicit what external services application code requires.
I'm super interested in things you were 100% unable to make work.
I actually haven't ran into anything that I couldn't get working and haven’t heard from any show stoppers from Simple Injector developers who are already actively building on ASP.NET Core. The main reason for this is because MVC got the right abstractions into place. The only thing I ran into was the design quirk with controllers that implement IDisposable by default. This is problematic for Simple Injector, because it will warn the user about disposable transient components (since transients aren’t tracked in Simple Injector). Because of this controller issue, the Simple Injector Startup.cs still uses the integration package, but this is something I really would like to be able to do without. My goal is to deprecate those packages once we have these simplified integration points in the framework. And I’m confident we can make this work.
These comments assume specific integration libraries provided by the DI container. This is problematic, because it preserves the endless permutation of NuGet packages for each combination of DI library and framework that you tried so hard to fix by introducing the Conforming Container. My suggestions are meant to free us all from this pain of having to maintain integration packages (not only the conforming part of the spectrum), by instead letting framework builders define a few simple extension points. Although the extension methods might not be considered "the cleanest approach", as I showed in the Github repository, they allow plugging in any container (or Pure DI enthusiast) without any integration package. Feel free though to rearrange, rename, renamespace, or whatever is needed to make it become as clean as possible.
I think the way we did that is much better than this extension method approach. Now the user has to be concerned with calling all of the right methods to wire up their container? A single entry point is easier for everyone. As a user, I don't care what the DI container has to do to wire itself to the framework, it's an implementation detail. services.AddSimpleInjectorSupport() is a much more user friendly approach. Non-conforming containers must have an integration package that wires up the extensibility points of each framework. That does not go away.
This would be rather problematic, because not only would this force every container to have an integration package for every framework, but it also requires every possible framework supplied service to be registered into the application container using the integration package. And if we do this for every framework, we would have to do the same for the endless number of 3rd party components that will exist in the near future. And don’t forget that frameworks and 3rd party components might have complex rules that decide whether to register something or not. This means that those integration packages must copy this logic, because it can’t simply cross-wire everything, because that could trip up any verification that the user might do on his container; registrations that can’t be resolved would cause the user’s verification to break. The only way around this is to integration build packages that read the list of IServiceCollection registrations, sending us back to square one: back to the Conforming Container adapter.
So instead you make the user write that boiler plate code?
I actually haven't ran into anything that I couldn't get working and haven’t heard from any show stoppers from Simple Injector developers who are already actively building on ASP.NET Core. The main reason for this is because MVC got the right abstractions into place. The only thing I ran into was the design quirk with controllers that implement IDisposable by default. This is problematic for Simple Injector, because it will warn the user about disposable transient components (since transients aren’t tracked in Simple Injector). Because of this controller issue, the Simple Injector Startup.cs still uses the integration package, but this is something I really would like to be able to do without. My goal is to deprecate those packages once we have these simplified integration points in the framework. And I’m confident we can make this work.
That's good to know. One thing I can think of that is less than ideal for a non-conforming container is middleware injection, but you can work around that as well.
I think the way we did that is much better than this extension method approach
I’m a little perplexed by this statement: I’m not attempting to prove one way is better than another, the point is that not everybody can conform; whether or not the Conforming Container model "is much better" doesn’t really seem relevant. You recently promised to make integration as smooth as it can be for non-conformers. Having spent time designing, building and testing a working solution I'm now left feeling I’ve hit a brick wall. Please be honest with me: if you're not interested in a community driven approach to finding a solution that enables non-conformers to plug in to ASP.NET then please say so. We can stop wasting our time here and focus on other things. If, however, you are truly interested in finding an answer for the members of the community who cannot conform then I hope you will consider improving my ideas in such a way that satisfies all our expectations.
Having spent time designing, building and testing a working solution I'm now left feeling I’ve hit a brick wall.
Can you tell me what brick wall you've hit? I asked you if you had anything you couldn't work around and you said no.
Please be honest with me: if you're not interested in a community driven approach to finding a solution that enables non-conformers to plug in to ASP.NET then please say so. We can stop wasting our time here and focus on other things. If, however, you are truly interested in finding an answer for the members of the community who cannot conform then I hope you will consider improving my ideas in such a way that satisfies all our expectations.
I'm 100% interested and I also don't consider the patterns in the github repository (with those public extension methods) a reasonable approach for non-conforming containers to plug into ASP.NET Core.
I'm honestly surprised you think having end users manually wire up N extensibility points with tons of extension methods on IServiceCollection and IApplicationBuilder is a viable solution.
I should have asked this before:
Are you recommending that each of the containers be wired up like the github repositories? Or is that just showing the required extensibility points?
Are you recommending that each of the containers be wired up like the github repositories?
The examples were primarily to flesh out the design, ensuring compatibility with the more mainstream containers and Pure DI. The examples are by no means meant as a recommendation of how to wire things up. Each DI library can document its integration approach to Core and MVC. If a container supports the Conforming Container, its documentation will more than likely show how to use its adapter. Non-conforming containers may benefit from the sample projects as a starting point for their documentation, but they may also find a way to make the integration more idiomatic to their container. Authors may choose to provide additional integration packages to simplify common scenarios, but the idea is that integration packages should be optional for non-conformers.
One of my thoughts around this mirrors what @davidfowl said:
I'm honestly surprised you think having end users manually wire up N extensibility points with tons of extension methods on
IServiceCollectionandIApplicationBuilderis a viable solution.
I think if you're stuck in non-conformer land (or really strongly prefer that), then your DI library is going to have to provide appropriate integration libraries to wire in correctly - even if it's just a single assembly with a single extension method in it to remove the boilerplate code. I can see how adding these hooks enables those libraries or that style of integration to be possible, but it definitely doesn't reduce the number of integration libraries you'll need; it will increase it.
Don’t forget that most users will need to cross-wire just a handful of framework components. My guess is that 90% of developers will not need more than 10 of those cross-wired dependencies.
I could be in that 10% that isn't covered, but I'm consistently in that 10%. I'm guessing the folks who don't need so many cross-wired dependencies are also the folks for whom the "File -> New Project" flow is optimized. Or possibly folks who work primarily in greenfield code and/or on small/elite teams where you can be pretty purist about the design. Plus... 10 dependencies is still > 0. Could still be painful to set up.
Regardless, interesting stuff. I think it's valuable to enable hooks like this. For folks who are interested in using them, it's priceless and doesn't seem like it'd be too difficult or impactful to support.
I'm personally more interested in reducing the constraints on the conforming container to enable users a single, easy integration point where much of this boilerplate code isn't required; but from a technical perspective, I do find this interesting as well.
Oh, and thanks to @dotnetjunkie for taking the time to look into this, spike something up, and get the conversation started. It's good and valuable work.
Regardless, interesting stuff. I think it's valuable to enable hooks like this. For folks who are interested in using them, it's priceless and doesn't seem like it'd be too difficult or impactful to support.
I'm all for fleshing out the design, I just don't like top level extension methods polluting IApplicationBuilder that a small percentage of people need to use
Agreed. That's easy enough to fix by putting them in a known namespace, though, right? If you want to use these lower-level extensions, be sure to add a using WhateverThatNamespaceIs statement to your Startup.
I don't give a rat's bottom in which namespace these extensions are placed. Newest versions of Visual Studio make it dead simpe to find these extension methods. I'm all in for not poluting the main API.
I guess I don't understand the goal. Sounds like it would be to have us own the package that contains the extension methods right? Let's say we did that:
- Microsoft.AspNetCore.NonConformingContainer.Extensions
- Microsoft.AspNetCore.Mvc.NonConformingContainer.Extensions
- Microsoft.AspNetCore.SignalR.NonConformingContainer.Extensions
Would that be enough? These packages wouldn't be there by default, users would need to add them to have the extension methods show up. To further hide them, they could be in another namespace (that would be good as well). Non-conforming container authors adapters can choose to take a dependency on these to use them to implement their non conforming container extensions. Whenever we add a new thing to a framework we would add an extension method for it here then either the app developer has to manually wire that up or the container author can hide it for them and do it on their behalf.
Best of all worlds?
[own the package that contains the extension methods right?
Rather not. Doing so would needlessly hide these integration points for a large group of developers. This would make it harder for developers to use these integration points; finding the right NuGet package (that won't be included by default) is already much more work than having the extensions in a different namespace.
Non-conforming container authors adapters can choose to take a dependency on these
This still assumes that container authors need to do anything. As my repo shows, most containers don't need to create anything when the framework contains these extensions out of the box. The more you hide those extension methods, the more container authors are forced to add integration packages to make things easier for their users while the goal should be less integration packages for everyone (not only conformers).
the container author can hide it for them and do it on their behalf.
This assumes that a container author wants to expose a single AddXXX() extension method to wire everything up. This by default pushes the creation of many framework (and third party) supplied classes back to the application container which is a problem for non-conformers.
Take tag helpers for instance. If the user plugs in a third party library that contains some tag helpers that the user wants to use, an AddMyNonConformingContainer() method will force tag helpers to be resolved by that non-conforming container. This will easily break since that non-conformer doesn’t know anything about that tag helper component and its dependencies; this will confuse developers. Instead we need a pay-for-play model where the user calls an AddCustomTagHelper(…) method in case he does have its own tag helpers that he wants to get resolved by the application container.
Whenever we […] add an extension method […] the container author can hide it for them and do it on their behalf.
A container author can’t add a call to such new extension method to its AddMyNonConformingContainer() call. It would cause the creation of a certain family of types to be forwarded to the application container. This can cause the container to throw an exception, because it might not know how to build up that object graph (remember, the non-conforming application container doesn’t have knowledge about framework and third party registrations). In other words, it’s impossible to have a AddMyNonConformingContainer() call that evolves over time, because that is a breaking change.
Since it is impossible to evolve such AddMyNonConformingContainer() call, and you don’t want to intercept all interception points by default, having such call in the first place is not helpful. The only option here is for the user to explicitly add a call to intercept a certain family of objects; trying to do this on behalf of the user (in the non-conforming world) is not possible. And without having an AddMyNonConformingContainer() method, there shouldn’t be much need to have an container-specific integration project.
Rather not. Doing so would needlessly hide these integration points for a large group of developers. This would make it harder for developers to use these integration points; finding the right NuGet package (that won't be included by default) is already much more work than having the extensions in a different namespace.
We can see what it looks like exposing something on the right types (IApplicationBuilder isn't the right place to expose MVC things for example)
This still assumes that container authors need to do anything. As my repo shows, most containers don't need to create anything when the framework contains these extensions out of the box. The more you hide those extension methods, the more container authors are forced to add integration packages to make things easier for their users while the goal should be less integration packages for everyone (not only conformers).
Your repo shows that this is possible, my personal opinion is that the code in user hostile and I don't think it's reasonable to ask developers to wire up each extensibility point.
This assumes that a container author wants to expose a single AddXXX() extension method to wire everything up. This by default pushes the creation of many framework (and third party) supplied classes back to the application container which is a problem for non-conformers.
I would hope you would do that. But to be honest its's up to you. Once again I encourage you to add that extension method but if you don't just have really good docs on how to wire up each framework's specific extensibility points with your container.
Take tag helpers for instance. If the user plugs in a third party library that contains some tag helpers that the user wants to use, an AddMyNonConformingContainer() method will force tag helpers to be resolved by that non-conforming container. This will easily break since that non-conformer doesn’t know anything about that tag helper component and its dependencies. This will confuse developers even more than having a pay-for-play model where the user calls an AddCustomTagHelper(…) method in case he does have its own tag helpers that he wants to get resolved by the application container.
Sure, if you think people need that level of granularity then you can add extension methods for each thing.
Since it is impossible to evolve such AddMyNonConformingContainer() call, and you don’t want to intercept all interception points by default, having such call in the first place is not helpful. The only option here is for the user to explicitly add a call to intercept a certain family of objects; trying to do this on behalf of the user (in the non-conforming world) is not possible. And without having an AddMyNonConformingContainer() method, there shouldn’t be much need to have an container-specific integration project.
Cool.
So we're over that, the repository you have will be the blueprint for non-conforming containers.
- We need extension methods on some types for non conforming containers to get access to. We also need to see which package they should live in.
- These are per framework so the MVC ones would likely be expose off
IMvcBuilderin a namespace that would not be included by default. - We need to figure out where to put the top level ones. A few of them I don't really like because they aren't really doing anything special, they just wrap existing concepts (like
IStartupFiler).
- These are per framework so the MVC ones would likely be expose off
- User code will wire up each extensibility point per framework.
PS: Full disclosure, I'm not a fan of how the wire up looks, it not smooth at all, in your face and extremely error prone. I'd like to see if any other container authors have similar feedback or if it's just my bias towards conforming containers clouding my vision.
/cc @khellang @ipjohnson @seesharper @alexmg @aL3891 @QuantumHive
extremely error prone.
We can work on that. Can you describe what you think is error prone?
I have been following the discussions surrounding conforming vs. non-conforming approaches since the beginning and I’m happy to see that there is progress on this point. To be honest there was a point in time when I felt I was forced to use products which did conform to this whole new idea of built-in dependency injection.
Depending on the size of my project I typically use Pure DI and when the project grows I switch to using a DI container. Because of the feedback given by the container, the built-in safety nets and therefore the implicit force to adhere to SOLID principles I use Simple Injector.
Full disclosure, I'm not a fan of how the wire up looks, it is not smooth at all, in your face and extremely error prone. I'd like to see if any other container authors have similar feedback or if it's just my bias towards conforming containers clouding my vision
Although I’m not a maintainer, I think the feedback from a user should be even more valuable, eventually we are the ones actually needed to write this code. So here is my opinion:
how the wire up looks, it is not smooth at all and in your face
Well that depends. In my opinion it is the opposite:
I really love the way how this separates the framework components (i.e. MVC) and application components. It exactly shows how the application is wired, which components are used, i.e. which components aren’t used! In other words: It makes it clear to members of my team how the application works. In earlier comments, I read that you think of this code as boilerplate code. Well it maybe is, but at least the user has to think about what he is doing, which is IMHO far better than some ‘Magic wand’ approach in which ‘it just works’.
I saw this a lot in my team in past years. Team members just calling some methods from some framework without actually knowing why it is needed and what needs to be done if there are other or new demands. Most of the time thereby violating at least two of the SOLID principles. So, this practice is a big no-no in my team.: Understand what you’re doing and don’t wire things you never need. And if they’re needed in the future, both Pure DI and Simple Injector will shout at you that have forget something where again the user has to think what he was doing in the first place.
extremely error prone
Well it maybe is. I think it is too early to give some real feedback on this, but my first reaction is: At least we get an exception (hopefully with a clear message) and we have the ability to fix it, because it is our code base.
Fortunately, MVC already had the correct integration points and is only missing the proposed simplifications. Hopefully this will guide other framework/library builders, parallel to the possibility to use the conforming container, to also provide the correct integration points and I hope to see Microsoft actively advising to do this!
I would really like to know why you think the proposed model is "extremely error prone" so we can work on this together.
Sorry, the MVP summit was this week so lots of distractions.
@TheBigRic I guess beauty is in the eye of the beholder. But if you prefer that approach then I'm glad to works out well in general. We'll continue to provide abstractions like we always have (we didn't get lucky with MVC, we just designed things that way).
I would really like to know why you think the proposed model is "extremely error prone" so we can work on this together.
The number of moving parts required to wire things up is on the per framework, per feature level and it's very hard to eyeball if you did it right. The code is extremely verbose and the extension methods unfortunately don't help with that at all. Also everything hanging off the the IApplicationBuilder isn't great. I'd prefer if MVC things were hanging off MVC etc. It's also very easy to miss "DI" enabling one of the many components you end up using along the lifetime of your application but as you said earlier that might be a feature.
I'm not sure if there's anything blocked here. Doesn't sound like it and if this pattern isn't complicated to the people that need to use it it'll just be a discovery problem and docs can help with that I guess.
Hi @davidfowl,
it's very hard to eyeball if you did it right.
Can you elaborate on this?
The code is extremely verbose and the extension methods unfortunately don't help
Can you explain why don't you think the extension methods help here?
It's also very easy to miss "DI" enabling one of the many components
This model is very different from the Conforming Container model where you can wire things up once and ‘forget’ about it. Once you start using other framework features, you must facilitate this in the Composition Root. When these extension methods are written in a consistent manner however, it will be very easy for the user to understand what it is he needs to do once he gets an exception from the framework or container about some missing component. It's just a matter of calling an AddCustomXXXActivation extension method.
Please help me out here. Tell me what it is we need to improve to be able to make it live up to your standards so we can add these simple extensions to the framework(s). This would be a massive improvement for the non-conforming part of our industry.
I can't help, what you're saying is that this is the model. Being explicit about wire up points is part of the feature. That means the user needs to be aware by design of all extensibility points.
Great. Let's move forward with this and add this feature to the framework(s).
I'm really excited to see this getting considered. I just spent hours getting the run around with SimpleInjector and finally gave up. I can't wait to see what happens.
For me, the biggest problem with both Ninject, Castle, and SimpleInjector kept boiling down to the Identity framework.
I know there are other concerns, but I was able to get most of them to do something right beyond the Identity framework. But getting that wired just didn't work. By the time I got the information to do cross wiring, it seemed pointless.
I can't pretend to understand everything going on in this thread, but I really hope something at least comes of it...
taken from the core roadmap
Encourage an active community welcoming contributions from all.
Does that philosophy extend to aspnet? If so does it extend to this contribution? If so can we have an update?
@davidfowl any news on this issue ?
For now unfortunately IMO asp.net core is not truly functional without a basic feature like DI integration.
@guy-lud stating that asp.net core doesn't have DI integration is factually inaccurate. There are plenty of containers that are integrated with asp.net core.
Wouldn't it be more factually accurate to say, "my favorite container doesn't properly support the features needed to integrate with asp.net core, therefor it's not functional to me"
Because I've use DI from a 3rd party container everyday in asp.net core and it works great. Have for more than a year.
@ipjohnson
It is great if the container of your choice is actually able to integrate with Asp.Net Core.
The problem is that because of, the much discussed, conforming container approach Microsoft took here, the containers that are able to integrate properly are rare.
Most of the popular DI containers have trouble integrating with this new conforming container approach, such as Simple Injector (see), Autofac (see), StructureMap (see), Castle Windsor (see), Unity (see) and Ninject (see).
In case your container of choice doesn't supply you with an integration point that allows replacing the built-in DI Container (thus working as an adapter on IServiceCollection), it's important that there are alternative approaches that allow integrating your DI Container with ASP.NET Core. Up to this point, integration has proven to be difficult, which is that this issue is all about.
@ipjohnson,
I totally agree with @TheBigRic response.
regarding your comment:
"my favorite container doesn't properly support the features needed to integrate with asp.net core, therefor it's not functional to me"
IMO it will be more accurate to say: "It's hard to integrate asp.net core default DI to fit my design preferences.
Let's look at IStartupFilter, ASP.Net Core DI allows you to register multiple impl of the same interface not explicitly, further more each impl can have different lifeStyle. If you'll request IStartupFilter from the DI you will get one impl (you don't know who). If you'll request all you'll get 3 (by default) two as singleton and one as Transient, Which IMO is bad design, it can lead to an unpleasant surprise.
@TheBigRic I get it, I think everyone in this thread "gets it" as it's been hashed/rehashed in a number of different issue.
What @guy-lud said wasn't any where close to what you just stated, was factually inaccurate as there are most definitely integration points and containers that integrate (I say this as an author of a container that integrates).
Everyone wants to lay blame at the feet of @davidfowl and crew but from my perspective I've had nothing but great interactions with the team and found it very easy to integrate with.
Which IMO is bad design, it can lead to an unpleasant surprise.
I'd be interested in seeing a proposal for an alternative design that will allow any number of different, unknown components to participate in composing an application 😉
@ipjohnson,
I am sorry if it's look like i want to pass blame at the feet ...... it was not my intention.
Guys, I would love to see the discussion on this thread to stay on topic. The issue is that the integration for non-conformers is not trivial and that's what this issue tries to address. If you have any feedback on my suggested extension methods or finding other problematic areas where integration is hard, what these extension methods don't address, please comment below.
We periodically close 'discussion' issues that have not been updated in a long period of time.
We apologize if this causes any inconvenience. We ask that if you are still encountering an issue, please log a new issue with updated information and we will investigate.