Looking at evolving this and integrating it into the CastleProject

[ghost]

Hi

I have been working on this issue in the castle windsor project and we identified an intended nuget id for this implementation. It does collide with your registration of Castle.Windsor.MsDependencyInjection. Are you OK for this to go back to the castle project? It does not have to happen just yet we are still figuring things out, so thought I would get the conversation going first.

As an aside, I have tried to implement this project myself and I pretty much landed on the same solution you built here @ https://github.com/fir3pho3nixx/castle-windsor-owin-deps. I could not deal with the double registrations and found out the problem was coming from here. I also had fundamental issues with the way service location was being done from scopes with fallback disposals on transients and raised this issue here. @davidfowl was kind enough to put up with me and point me to an issue that was raised with non conforming containers with a Castle Windsor implementation here. I would really like to hear your feedback on this as it seems you and I have pretty much traveled the same path to this point.

Hope you are well

Gav

[hikalkan]

Hi Gav,

I'm OK to contribute this project (or just the name) to the Castle organization.

It was hard to build Castle.Windsor.MsDependencyInjection because (as you know) Microsoft's IOC design is pretty different than Windsor. There is no 'scope' concept in castle like MS implements it as a fundamental problem. But finally my implementation become stable and used in many real applications. We can take this a base and discuss around it (instead of creating a new one). What do you think?

[jonorossi]

we identified an intended nuget id for this implementation. It does collide with your registration of Castle.Windsor.MsDependencyInjection

@fir3pho3nixx I thought we were going with Castle.Windsor.Lifestyles.MSDependencyInjection, this project is using Castle.Windsor.MsDependencyInjection.

[hikalkan]

@jonorossi I don't say that you should use Castle.Windsor.MsDependencyInjection naming, but Castle.Windsor.Lifestyles.MSDependencyInjection is not a good naming because that library will be an adapter rathen than only a new lifestyle.

[jonorossi]

@hikalkan thanks, the original plan which @fir3pho3nixx linked directly to had a lifestyle for SystemWeb and Owin too, which was the reason for the naming.

That comment also includes the following:

Unless they'll be more than just a lifestyle, i.e. the WCF integration has a per channel lifestyle but is obviously named a facility as the main part is the Windsor facility.

Knowing that this project is less about a lifestyle and more an adapter, we should instead follow the existing Windsor facilities naming convention used for all existing extensions.

[ghost]

It was hard to build Castle.Windsor.MsDependencyInjection because (as you know) Microsoft's IOC design is pretty different than Windsor. There is no 'scope' concept in castle like MS implements it as a fundamental problem.

That is a nice way of putting it, I could think of some four letter words to describe it but yes it is fundamentally different to Windsor's design and has earned Castle Windsor the name of a non-conforming container. I think this overly opinionated(rude in fact) and just plain bad sport on Microsoft's part. These containers have been around for donkeys years. Why would you move the goal posts so far and create a design with such a high barrier to integration? They clearly did not think this through and are forcing the community to a place where all the maturity and the hours of learning and maintenance on these containers has to be redesigned/hacked to work with aspnet core. Frankly I am rather annoyed with this and get sense that some of ego's in that team need a few of smack downs.

But finally my implementation become stable and used in many real applications. We can take this a base and discuss around it (instead of creating a new one). What do you think?

Yes, I can see this. This is very much on the same path I was heading and I started looking at your code and so I gave up. I want to investigate one more thing where someone has suggested implementing the container as pure OWIN middleware before we go ahead. There might be some valuable learning I would like to explore first. See below...

using System;
using Castle.MicroKernel.Lifestyle;
using Castle.MicroKernel.Registration;
using Castle.Windsor;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.ViewFeatures.Internal;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using MissingDIExtensions;

namespace SampleApplication.CastleWindsor
{
    public class Startup
    {
        private readonly WindsorContainer container = new WindsorContainer();

        public Startup(IHostingEnvironment env)
        {
            var builder = new ConfigurationBuilder()
                .SetBasePath(env.ContentRootPath)
                .AddJsonFile("appsettings.json", optional: true, reloadOnChange: true)
                .AddJsonFile($"appsettings.{env.EnvironmentName}.json", optional: true)
                .AddEnvironmentVariables();
            Configuration = builder.Build();
        }

        public IConfigurationRoot Configuration { get; }

        // This method gets called by the runtime. Use this method to add services to the container.
        public void ConfigureServices(IServiceCollection services)
        {
            // Add framework services.
            services.AddMvc();

            services.AddSingleton<IHttpContextAccessor, HttpContextAccessor>();

            services.AddRequestScopingMiddleware(container.BeginScope);
            services.AddCustomControllerActivation(container.Resolve);
        }

        // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
        public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory)
        {
            RegisterApplicationComponents(app, loggerFactory);

            // Add custom middleware
            app.Use(async (context, next) =>
            {
                await container.Resolve<CustomMiddleware>().Invoke(context, next);
            });

            app.UseStaticFiles();

            app.UseMvc(routes =>
            {
                routes.MapRoute(
                    name: "default",
                    template: "{controller=Home}/{action=Index}/{id?}");
            });
        }

        private void RegisterApplicationComponents(IApplicationBuilder app, ILoggerFactory loggerFactory)
        {
            // Register application services
            container.Register(Component.For(app.GetControllerTypes()).LifestyleScoped());
            // container.Register(Component.For(app.GetApplicationViewComponents()));

            container.Register(Component.For<IUserService>().ImplementedBy<AspNetUserService>().LifestyleScoped());
            container.Register(Component.For<CustomMiddleware>());

            // Cross-wire required framework services
            RegisterFactoryMethod(app.GetRequestService<IViewBufferScope>);
            container.Register(Component.For<ILoggerFactory>().Instance(loggerFactory));
        }

        void RegisterFactoryMethod<T>(Func<T> factory) where T : class
        {
            container.Register(Component.For<T>().UsingFactoryMethod(_ => factory()));
        }
    }

    public interface IUserService { }

    public class AspNetUserService : IUserService { }
}

Notice how this suggestion does not return and IServiceProvider from the ConfigureServices. I was wondering if you ever explored anything like this before you wrote the adapter? Look forward to your feedback.

[hikalkan]

Could not understand what's the goal here. If it's just for not returning IServiceProvider from the ConfigureServices then it's not needed because MS DI provides IServiceProviderFactory interface so you can integrate container to ASP.NET Core without modifying return value of ConfigureServices. See the approach of autofac: http://docs.autofac.org/en/latest/integration/aspnetcore.html#quick-start-with-configurecontainer (ConfigureServices(services => services.AddAutofac())). I don't think returning IServiceProvider is a problem. Actually I created the factory (WindsorServiceProviderFactory - https://github.com/volosoft/castle-windsor-ms-adapter/blob/master/src/Castle.Windsor.MsDependencyInjection/WindsorServiceProviderFactory.cs) but not created the extension method (like services.AddWindsor()). But I can do it easily.

[ghost]

Could not understand what's the goal here.

Maybe I can explain it better. The scoped registration of lifestyles is what I found interesting. This is backed on to CallContextLifetimeScope in Windsor but in dotnet core it runs on an AsyncLocal.

Scopes are created using an IStartupFilter using pure middleware callback's

Is also calls on AddRequestScopingMiddleware which gives you a tidy BeginScope callback hook using a custom IStartupFilter for using baked in middleware for each request. This happens before controller activation.

Controllers are activated using an IControllerActivator using Activator callback's

You're controllers are registered via the AddCustomControllerActivation extension which also gives you a controller activator using a custom IControllerActivator implementation. This is great because all you have to worry about is the controllers, not everything else.

In summary

It almost feels like if you go with the solution above, you would not even have a need to implement Microsoft.Extensions.DependencyInjection.Abstractions and you could just end up with something just as good!

[ghost]

Closing this for now because my focus has shifted a little. Will ping back once I have something more concrete to review. Many thanks!

[hikalkan]

OK :)