Skip to content
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

Wrong c'tor get's choosed #136

Closed
stffabi opened this issue Jun 21, 2016 · 3 comments · Fixed by #343
Closed

Wrong c'tor get's choosed #136

stffabi opened this issue Jun 21, 2016 · 3 comments · Fixed by #343
Milestone

Comments

@stffabi
Copy link

stffabi commented Jun 21, 2016

Accordingly to https://github.com/castleproject/Windsor/blob/master/docs/how-constructor-is-selected.md the constructor with the most satisfiable parameters gets picked.

In the following example, Windsor chooses the parameterless c'tor on MyControl although the c'tor with the parameter of type IMyUtils<ArrayList> can be resolved as proofed by lResolvedFixedHelper

Registering a generic type-unbound implementation for IMyHelper<> seems to fix the problem and the expected c'tor is called on MyControl.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Castle.MicroKernel.Registration;
using Castle.Windsor;

namespace ConsoleApplication15
{
    class Program
    {
        static void Main(string[] args)
        {
            var lContainer =  new WindsorContainer();
            var lGenericUtilsReg =
                Component.For(typeof(IMyUtils<>)).
                ImplementedBy(typeof(MyGenericUtilsImpl<>));

            lContainer.Register(lGenericUtilsReg);

            var lFixedHelperReg =
               Component.For(typeof(IMyHelper<ArrayList>)).
               ImplementedBy(typeof(MyArrayListFixedHelperImpl));

            lContainer.Register(lFixedHelperReg);

            var lControlReg =
               Component.For(typeof(MyControl)).
               ImplementedBy(typeof(MyControl));

            lContainer.Register(lControlReg);

            // Commenting the following code in, let's the example run as expected.
            //var lGenericHelperReg =
            //   Component.For(typeof(IMyHelper<>)).
            //   ImplementedBy(typeof(MyGenericHelperImpl<>));

            //lContainer.Register(lGenericHelperReg);

            var lResolvedFixedHelper = lContainer.Resolve<IMyHelper<ArrayList>>();
            if (lResolvedFixedHelper == null)
            {
                throw new ApplicationException("Should not happen.");
            }

            var lResolvedControl = lContainer.Resolve<MyControl>();
            if (lResolvedControl == null)
            {
                throw new ApplicationException("Should not happen.");
            }
        }
    }

    public class MyControl
    {
        public MyControl()
        {
            throw new ApplicationException("Should not be used.");
        }

        public MyControl(IMyUtils<ArrayList> aUtils)
        {
            Console.WriteLine("MyControl: ctor " + aUtils.GetType().Name);
        }
    }

    public class MyGenericUtilsImpl<T> : IMyUtils<T>
    {
        public MyGenericUtilsImpl(IMyHelper<T> aUtils)
        {
            Console.WriteLine("MyGenericUtilsImpl: ctor " + aUtils.GetType().Name);
        }
    }

    public class MyGenericHelperImpl<T> : IMyHelper<T>
    {
        public MyGenericHelperImpl()
        {
            Console.WriteLine("MyGenericHelperImpl: ctor");
        }
    }

    public class MyArrayListFixedHelperImpl : IMyHelper<ArrayList>
    {
        public MyArrayListFixedHelperImpl()
        {
            Console.WriteLine("MyArrayListFixedHelperImpl: ctor");
        }
    }

    public interface IMyUtils<T> { }

    public interface IMyHelper<T> { }
}
@hammett hammett self-assigned this Jun 21, 2016
@ghost
Copy link

ghost commented Sep 22, 2017

@stffabi - I have been through this in detail:

The Problem

The problem is with this constructor:

public class MyGenericUtilsImpl<T> : IMyUtils<T>
{
	public MyGenericUtilsImpl(IMyHelper<T> aUtils)
	{
		Console.WriteLine("MyGenericUtilsImpl: ctor " + aUtils.GetType().Name);
	}
}

and how IMyHelper<T> is registered.

var lFixedHelperReg =
               Component.For(typeof(IMyHelper<ArrayList>)).
               ImplementedBy(typeof(MyArrayListFixedHelperImpl));

The constructor is requesting an Open Generic (meaning it is a more general type because there is no generic parameter). This technically does not match anything you have registered in the container(if this does not make sense because it resolves anyway, don't worry I will come back to this at the end). Your service registration has a higher specificity(or is in fact considered to be a new type) because it registers a Closed Generic (meaning it has a generic parameter type of ArrayList bound to IMyHelper`1).

During the the resolution process the container correctly chooses the default constructor because it technically cannot infer that the type requested IMyHelper<T> is satisfied by the IMyHelper<ArrayList> registration on the MyGenericUtilsImpl<T> constructor.

To further demonstrate my explanation, I invite you to change the constructor on MyGenericUtilsImpl to use an ArrayList instead of T like so:

public class MyGenericUtilsImpl<T> : IMyUtils<T> // <-- 2. Is not implied by consumers of this when picking constructors
{
	public MyGenericUtilsImpl(IMyHelper<ArrayList> aUtils) // <-- 1. Inference of <T> here.
	{
		Console.WriteLine("MyGenericUtilsImpl: ctor " + aUtils.GetType().Name);
	}
}

You will notice this hits your expected constructor. You also identified another approach which was to open up the generic registration which then also satisfies the requested service type on MyGenericUtilsImpl<T>'s constructor:

// Commenting the following code in, let's the example run as expected.
//var lGenericHelperReg =
//   Component.For(typeof(IMyHelper<>)).
//   ImplementedBy(typeof(MyGenericHelperImpl<>));

This was the smoking gun that pointed me in in MyGenericUtilsImpl direction.

Why does it resolve if I remove the constructor or apply the [DoNotSelect] attribute?

public class MyControl
{
	[DoNotSelect] // <-- This attribute
	public MyControl()
	{
		throw new ApplicationException("Should not be used.");
	}

	public MyControl(IMyUtils<ArrayList> aUtils)
	{
		Console.WriteLine("MyControl: ctor " + aUtils.GetType().Name);
	}
}

This is a good question. This is because the CreationContext get's recreated during the resolve process regardless of whether if handlers are still waiting for dependencies This is because open generic handlers in a state of CurrentState.WaitingDependency are converted to sub handlers with generic parameter bindings from the DefaultDependencyResolver's ResolveFromKernel method. This is what makes things a little weird here. Everything during registration time is telling you that you 'potentially' cannot do this, yet when you Resolve it works!

I am thinking some of these smarts have not been lifted up to the registration process CanResolve CanResolveFromKernel logic in DefaultDependencyResolver and that is why it could be is picking the wrong constructor.

@ghost
Copy link

ghost commented Sep 22, 2017

@stffabi - Good find! :)

@jonorossi jonorossi added this to the vNext milestone Sep 25, 2017
@jonorossi
Copy link
Member

Merged, thanks heaps @Fir3pho3nixx, great work simplifying the test case.

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

Successfully merging a pull request may close this issue.

3 participants