
autofac - issue #330
Generic parameters constrained as generic interfaces fail to resolve.
What steps will reproduce the problem?
Here is the situation with a failing test.
public interface IConstraint<T> { }
public class Constraint<T> : IConstraint<T> { }
public interface IGenericConstraint<T1, T2>
where T2 : IConstraint<T1>
{
}
public class GenericConstraint<T1, T2> : IGenericConstraint<T1, T2>
where T2 : IConstraint<T1>
{
}
[Test]
public void GenericConstraints()
{
var builder = new ContainerBuilder();
builder.RegisterGeneric(typeof(Constraint<>))
.AsImplementedInterfaces();
builder.RegisterGeneric(typeof(GenericConstraint<,>))
.AsImplementedInterfaces();
var container = builder.Build();
var target = container.Resolve<IGenericConstraint<int, IConstraint<int>>>();
}
What is the expected output? What do you see instead?
This above should resolve just fine. The problem is in ParameterCompatibleWithTypeConstraint in TypeExtensions.cs
static bool ParameterCompatibleWithTypeConstraint(Type parameter, Type constraint)
{
return
constraint.IsAssignableFrom(parameter) ||
(parameter.IsClass &&
Traverse.Across(parameter, p => p.BaseType)
.Concat(parameter.GetInterfaces())
.Any(p => p.GUID == constraint.GUID));
}
If the "parameter.IsClass" condition is removed and Traverse is allowed to execute then all works correctly.
Comment #1
Posted on Jun 15, 2011 by Happy MonkeyInteresting - looks on the surface that this is a good change. I'm not 100% sure of the original reason for the .IsClass, but the generic constraint reflection APIs are quirky.
If all the tests pass with the check removed I guess by definition it is safe :) .. If anything tricky shows up we might be able to use (parameter.IsClass || parameter.IsInterface) as an alternative.
Cheers, Nick
Comment #2
Posted on Jun 19, 2011 by Quick OxThanks for the response Nick. The unit tests passed when I ran it.
After looking at the code... I think that though this fix works, the constraint compatibility checks are still not expansive enough to truly choose the correct registration. Taking the example above, the constraint checking code doesn't recurse into the generic parameter of the interface:
where T2 : IConstraint
In other words, I could have had IConstraint there, and I believe the code would report that the constraint passes, even though I truly only want to allow IConstraint. (I haven't written a test for this yet as I am not in front of the code at the moment, but I wanted to point out the possibility before you make any decisions.)
Comment #3
Posted on Jun 24, 2011 by Quick OxAnother note... Even though I pointed out that the constraint matching code in general should be more robust in its search, getting this surgical fix into the build would still help a great deal, even without adding the more complex logic.
Comment #4
Posted on Jul 3, 2011 by Happy MonkeyI've made the suggested fix - the test came down to this:
var builder = new ContainerBuilder(); builder.RegisterGeneric(typeof (Constrained<,>)); var container = builder.Build(); Assert.That(container.IsRegistered>>());
We'll need to look into this further in the future, but I'm happy to close this now and deal with unsupported cases as they arrive.
Thanks again! Nick
Status: Fixed
Labels:
Type-Defect
Priority-Medium