volosoft / castle-windsor-ms-adapter

Castle Windsor ASP.NET Core / Microsoft.Extensions.DependencyInjection Adapter
https://www.nuget.org/packages/Castle.Windsor.MsDependencyInjection
MIT License
85 stars 29 forks source link

Wrong constructor called #22

Open mbp opened 6 years ago

mbp commented 6 years ago

Consider the following code

public interface IMyService
{
}

public class MyService : IMyService
{
    public MyService()
    {
        Console.WriteLine("Default constructor called!");
    }

    public MyService(params string[] foo)
    {
        Console.WriteLine("Custom constructor called!");
    }
}

I register it as follows:

container.Register(Component.For<IMyService>().ImplementedBy<MyService>());

When I call container.Resolve<IMyService>() from a normal console application not involving castle-windsor-ms-adapter, the default constructor is called.

When I call container.Resolve<IMyService>() from an ASP. NET Core project using this library (return WindsorRegistrationHelper.CreateServiceProvider(container, services); in Startup.cs, the second constructor is called.

I would have expected the default constructor always be called, as by out-of-the-box Castle Windsor behaviour. Am I missing anything?

hikalkan commented 6 years ago

This library does nothing about the constructor selection. So, I could not understand why there is a difference here.

However, I believe the constructor design of this sample is very tricky and badly designed in general. You should remove the first constructor. In addition, if you use dependency injection, it will be a problem to pass string[] to the second ctor. So, I think this is not a good and realistic sample to spend time to find the problem :)

acjh commented 6 years ago

How constructor is selected by Windsor:

  • It will look at the component's constructors and see which of them it can satisfy (for which of them it can provide all required parameters).
  • It will then see how many parameters each satisfiable constructor has, and pick the one with most parameters (the greediest one).

If you want the default constructor, you can do this:

using Castle.Core;

public class MyService : IMyService
{
    public MyService()
    {
        Console.WriteLine("Default constructor called!");
    }

    [DoNotSelect]
    public MyService(params string[] foo)
    {
        Console.WriteLine("Custom constructor called!");
    }
}
mbp commented 6 years ago

@hikalkan @acjh

I provided a simplified example. The service in my real world scenario is unfortunately from a separate library where I am not able to add [DoNotSelect] or even modify the library design.

@acjh

It does not matter how much you quote from the documentation; I provided a use case where it would not follow these rules across applications. I don't see any reason why Castle.Windsor in combination with castle-windsor-ms-adapter would suddenly think it can resolve params string[] foo, because surely I cannot do container.Resolve<string>(). By the way, you can replace string with any type, even custom type, and you get the same results.

I will likely have to find a different way to solve it, for example registering the component using a factory method, or not using dependency injection for this dependency. But I wanted to report this issue since I felt it was important for others to know that your application might break or provide non-deterministic behaviour when e.g. moving from ASP.NET to ASP.NET Core. Obviously I am now already a bit worried what other types of things it might resolve differently during the runtime of my application.

hikalkan commented 6 years ago

Thank you @mbp As I said, this library does not interfere to constructor selection, it's a deeper thing inside Windsor. However, assuming you tested correctly, it seems the behaviour changes. I think it's a edge case, but I will check and think on that when I have more time. Thank you for reporting that.