Skip to content
This repository has been archived by the owner on Jul 20, 2023. It is now read-only.

Reinstate Windsor #81

Closed
jonorossi opened this issue Jul 12, 2017 · 21 comments
Closed

Reinstate Windsor #81

jonorossi opened this issue Jul 12, 2017 · 21 comments

Comments

@jonorossi
Copy link

Hey guys, I've just released Castle Windsor 4.0.0 which supports .NET Core and Castle Core 4.0.0.

https://github.com/castleproject/Windsor/releases/tag/v4.0.0

@danielpalme
Copy link
Owner

I have updated the benchmark with the latest Windosor release.

Any hint for me how I can add tests for Net Core?
See these containers for comparsion:
https://github.com/danielpalme/IocPerformance/blob/master/IocPerformance/Adapters/GraceContainerAdapter.cs#L95
https://github.com/danielpalme/IocPerformance/blob/master/IocPerformance/Adapters/DryIocAdapter.cs#L80

@jonorossi
Copy link
Author

We don't yet have support for ASP.NET Core's Microsoft.Extensions.DependencyInjection. It is currently a work in progress to bring built-in support.

@hikalkan wrote the Castle.Windsor.MsDependencyInjection package which is well maintained, however the .NET Standard target still depends on a Windsor fork for .NET Core support so can't be used yet (see volosoft/castle-windsor-ms-adapter#15).

We'll keep you in the loop as we get this sorted, there are a few Windsor GitHub issues surrounding this, OWIN support and moving the existing System.Web lifestyle out of the core.

/cc @fir3pho3nixx @hikalkan

@hikalkan
Copy link

Just released v2.1 for Castle.Windsor.MsDependencyInjection: https://www.nuget.org/packages/Castle.Windsor.MsDependencyInjection

It depends on Castle.Windsor anymore. Thank you @fir3pho3nixx and @jonorossi.

@ghost
Copy link

ghost commented Oct 6, 2017

Any hint for me how I can add tests for Net Core?

We are still working on this by the way, once we have made an in-road into how we achieve this we will definitely get back to you.

I do have a question about ContainerAdapterBase->CreateServiceCollection. Would you be partial to creating a new method that supports non conforming containers that prefer to use their native registration API?

Please see aspnet/Mvc#5403 if you have heaps and heaps of time. The summary for us right now is that we are taking things away from our users in terms of our Registration API richness, introducing performance issues and just generally have to munge our lifecycles to support this.

Would even be happy to work with you to help raise PR's to help you make this work across the board. I do believe the non-conformers would also be very grateful.

Would also open the gates for:

  • Autofac
  • Ninject
  • Simple Injector
  • StructureMap
  • Unity
  • Pure DI

Look forward to your thoughts on this.

@ipjohnson
Copy link
Contributor

@fir3pho3nixx I wrote the test for asp.net core so I can speak to the test and while I'm not the least bit interested in rehashing the topic in that issue there are a few things I'd like to point out.

  • Autofac - is a "conforming" container so there isn't a reason to use the non conforming route
  • Ninject - is no longer being developed and doesn't support .netstandard
  • StructureMap - has a conforming adapter so not sure the non conforming make sense
  • Unity - is no longer being developed and doesn't support .netstandard

Originally I didn't travel down the non conforming road because it was suggested it might not make sense issue #66 by the author of Simple Injector. I also didn't go down this road because of the complexity and the fact that to do it correctly you have to setup a root container (MS container) that has the controllers registered and the services in the child container (container being tested). It seemed like a lot of work to support containers that either didn't want it or don't support .netstandard.

It seems like if you are willing to go the length of doing the root container idea it would be a fair test.

@ghost
Copy link

ghost commented Oct 7, 2017

@ipjohnson - Sorry if I am resurrecting old things, was not my intention. I was merely quoting the issue I referenced. I appreciate things might have moved on, so thanks for the update.

After reading the issue you posted I can see why supporting this test might give the wrong impression. What I am really after is the AspNetCore metric (which we want to support using facilities) but potentially without ServiceCollection's/Adaptation in one instance and then perhaps also include a metric where it is supported. Just thinking out loud here.

I also didn't go down this road because of the complexity and the fact that to do it correctly you have to setup a root container (MS container) that has the controllers registered and the services in the child container (container being tested)

Why do you say the controllers have to be registered in the root container? I read the issue and was not clear on why this would be a requirement? Could the controllers be registered in the "container being tested"?

Also thanks for taking the time to talk to me, I really appreciate your input.

@dadhi
Copy link
Contributor

dadhi commented Oct 7, 2017

Small (or not so) correction that both Ninject and Unity are active now and releasing the versions with .Net Standard support.

@ipjohnson
Copy link
Contributor

@dadhi my mistake as of august both projects hadn't released in quite some time and had open issues questioning if they were still being developed with no response from the developers. That said let's wait for when/if official releases happen to see if they are conforming or non-conforming.

@dotnetjunkie please correct me where I'm wrong as I don't use the non conforming route. The non conforming pattern uses microsoft's container for scoping, locating some services, and controller creation/disposal. The non conforming container is used to satisfy dependency registered by the user.

Don't get me wrong I think the idea of adding non conforming support for the benchmark makes sense, it should just be as close to the real thing as possible.

@dotnetjunkie
Copy link
Contributor

@ipjohnson,

I don't think there is really "one way" to be non-conforming, and I'm not sure I completely follow what you are saying, but in general, I would say that non-conforming means:

  • That the built-in MS DI Container [let's call this the "framework's configuration system"] is used for all built-in ASP.NET components and 3rd party components that require registration of their components.
  • A 3rd party container (or Pure DI) is used to build up application components by hooking into the available abstractions, e.g. IControllerActivator and IViewComponentActivator. [Let's call this 3rd party container the "application container"].
  • When it comes to scoping one could use the ASP.NET infrastructure. Using the infrastructure makes most sense when practicing Pure DI (although it might not always be needed), while using your application container's scoping facility would IMO keep you closest to your application container's development model. ASP.NET Core 1.0 and 1.1 for instance disposes instances in the wrong order, and using your application container's facilities prevents these kinds of problems.
  • In the end your code needs to interact with ASP.NET Core or 3rd party components, so at some point you will have to resolve stuff from the framework's configuration system. Either you do this by hand (at runtime inside an adapter for instance) or you inject them into your application components, which means you need to "cross-wire" them. This is the part that is sometimes confusing for users, and here competing or incompatible container models could collide. This is the part that needs research and improvement by means of integration points and documentation. Preferably this means Microsoft provided integration points and documentation, but in absence of that, it probably means this needs to be provided by container developers or others.

I hope this helps.

@ipjohnson
Copy link
Contributor

@dotnetjunkie yes thank you most helpful. Quick question just for clarification, a new scope will be created for each request in both the "framework configuration system" and the "application container" correct?

@fir3pho3nixx after reading steven's description you are correct the controllers would be in the application container not the "framework configuration system". That said the way the benchmark is written currently it's not very conducive to this type of setup.

Now if you change the benchmark to mimic the owin pipeline and controller activator you could get an apples to apples comparison of conforming and non conforming.

@dotnetjunkie
Copy link
Contributor

Quick question just for clarification, a new scope will be created for each request in both the "framework configuration system" and the "application container" correct?

Correct. You could have two scopes that start and end at the same time, but since each has its own set of components you would typically not notice this.

That changes however when you let both containers compose the same components (a bad idea), because that could lead to Torn Lifestyles.

@ghost
Copy link

ghost commented Oct 8, 2017

I would just like to deliver a mega huge thank you to @dotnetjunkie, @danielpalme for participating in this discussion.

Are we saying we can support metrics for conformers/non-conformers separately though? Or should we try and merge this into one metric?

PS: @ipjohnson congrats for setting the bar for conformers using Grace, I am going to be trawling through your code to figure out how the hell you managed to achieve this. Well done.

@dotnetjunkie
Copy link
Contributor

I think @ipjohnson is lucky enough that his container design matches the MS Abstraction close enough to be able to adapt. But container authors must also be aware that adapting to the abstraction can also severely limit the evolution of their container. New features can only be added when they don't cause any compatibility issues that cause the adapter to break. An easy example is when Johnson wants to add a feature to Grace that is similar to the verification facilities that both Castle Windsor and Simple Injector contain. Such feature easily breaks the contract, because you would be verifying framework and 3rd party registrations as well, that are not designed with that particular container in mind.

@ipjohnson
Copy link
Contributor

@dotnetjunkie Grace actually has a feature that allows the user to get a list of possible missing dependencies allowing the validator to ignore 3rd party registrations if they want.

@fir3pho3nixx thanks for the compliment.

@dotnetjunkie
Copy link
Contributor

@ipjohnson, that's great, but verification in Castle and especially Simple Injector go much further than that. It much more than simply checking whether an object graph can be created. It's about detecting whether a certain configuration can cause problems, even if it can be created.

This has proven to be an incredible powerful feature and is IMO the main reason for SI's popularity.

But once you try to add such feature and start to prevent certain shapes of object graphs from being created, you will violate the MS contract and break your adapter.

@ipjohnson
Copy link
Contributor

@dotnetjunkie thanks I'll keep your thoughts in mind ;)

@dotnetjunkie
Copy link
Contributor

StructureMap - has a conforming adapter so not sure the non conforming make sense

Perhaps I'm a bit late, but I would like to point out that StructureMap actually isn't fully conforming. Although they do have an adapter, their adapter fails several of the spec tests as defined by Microsoft. Once developers run into issues concerning incompatibility, using StructureMap as a non-conforming container probably is your only way around this. So yes, using StructureMap in a non-conforming way actually does make a lot of sense.

@ghost
Copy link

ghost commented Jan 24, 2018

Update: I hope to raise the PR for the new Castle Windsor AspNet Core facility by the end of the week, just to allow some time for everyone on this issue to provide some community feedback.

https://github.com/fir3pho3nixx/Windsor.AspNetCore

Will help out with the benchmark after that.

@ghost
Copy link

ghost commented Apr 18, 2018

@danielpalme, Crikey, This took a while.

We now have a production ready version of our ASP.NET.Core facility here: castleproject/Windsor#394

Do we need to release it before we can benchmark? Or can I help submit a PR before hand?

Thanks to @generik0 for helping out.

@danielpalme
Copy link
Owner

danielpalme commented Apr 18, 2018

A (pre-release) package on Nuget is the preferred way to get the latest release.
A PR is also very welcome.

@ghost
Copy link

ghost commented Jul 12, 2018

A PR is also very welcome.

We are getting close! :)

Thanks for being so patient.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants