ComponentNotRegisteredException when type parameter name doesn't match the base class

[weelink]

Describe the Bug

Registering a component with RegisterGeneric when the service is a class where the type parameter name is different,
Autofac throws a ComponentNotRegisteredException, unless the component has been resolved explicitly first.

With_as_self_and_explicit_resolve succeeds because container.Resolve<NHibernateRepository<Task>>(); is called before container.Resolve<Repository<Task>>();.

If Repository<T> is an interface, then it succeeds.

If the type parameter in NHibernateRepository<TDomain> is renamed to match Repository<T>, it also succeeds.

Steps to Reproduce

public class ReproTest
{
    public class Task
    {
    }

    // Changing this to an interface works without problems.
    // Making it an abstract class also fails
    public class Repository<T>
    {
    }

    // Renaming TDomain to T also fixes the problem
    public class NHibernateRepository<TDomain> : Repository<TDomain>
    {
    }

    [Fact]
    public void With_as_self_and_explicit_resolve()
    {
        var builder = new ContainerBuilder();

        builder
            .RegisterGeneric(typeof(NHibernateRepository<>))
            .AsSelf()
            .As(typeof(Repository<>));

        var container = builder.Build();

        // Resolving the component explicitly fixes the problem
        container.Resolve<NHibernateRepository<Task>>();
        container.Resolve<Repository<Task>>();
    }

    [Fact]
    public void With_as_self_and_without_explicit_resolve()
    {
        var builder = new ContainerBuilder();

        builder
            .RegisterGeneric(typeof(NHibernateRepository<>))
            .AsSelf()
            .As(typeof(Repository<>));

        var container = builder.Build();

        container.Resolve<Repository<Task>>();
    }

    [Fact]
    public void Without_as_self_and_without_explicit_resolve()
    {
        var builder = new ContainerBuilder();

        builder
            .RegisterGeneric(typeof(NHibernateRepository<>))
            .As(typeof(Repository<>));

        var container = builder.Build();

        container.Resolve<Repository<Task>>();
    }
}

Expected Behavior

Resolving Repository<T> should return NHibernateRepository<TDomain> in all 3 cases.

Exception with Stack Trace

Autofac.Core.Registration.ComponentNotRegisteredException
The requested service 'Autofac.Test.ReproTest+Repository`1[[Autofac.Test.ReproTest+Task, Autofac.Test, Version=1.0.0.0, Culture=neutral, PublicKeyToken=17863af14b0044da]]' has not been registered. To avoid this exception, either register a component to provide the service, check for service registration using IsRegistered(), or use the ResolveOptional() method to resolve an optional dependency.
   at Autofac.ResolutionExtensions.ResolveService(IComponentContext context, Service service, IEnumerable`1 parameters) in C:\Projects\Autofac\src\Autofac\ResolutionExtensions.cs:line 874
   at Autofac.ResolutionExtensions.Resolve(IComponentContext context, Type serviceType, IEnumerable`1 parameters) in C:\Projects\Autofac\src\Autofac\ResolutionExtensions.cs:line 338
   at Autofac.ResolutionExtensions.Resolve[TService](IComponentContext context, IEnumerable`1 parameters) in C:\Projects\Autofac\src\Autofac\ResolutionExtensions.cs:line 290
   at Autofac.ResolutionExtensions.Resolve[TService](IComponentContext context) in C:\Projects\Autofac\src\Autofac\ResolutionExtensions.cs:line 273
   at Autofac.Test.ReproTest.Without_as_self() in C:\Projects\Autofac\test\Autofac.Test\ReproTest.cs:line 59

Dependency Versions

Autofac: 6.3.0
Autofac: The develop branch as of March 18th 2022

Additional Info

[tillig]

This is really weird and pretty interesting. I haven't dug in yet, but I admit, I'm intrigued as to why the name of the type parameter would matter.

[weelink]

In OpenGenericServiceBinder.TryFindServiceArgumentForImplementationArgumentDefinition (line 210):

var matchingRegularType = serviceArgumentDefinitionToArgument
    .Where(argdef => !argdef.Key.IsGenericType && implementationGenericArgumentDefinition.Name == argdef.Key.Name)

which can be changed to:

var matchingRegularType = serviceArgumentDefinitionToArgument
    .Where(argdef => !argdef.Key.IsGenericType && implementationGenericArgumentDefinition.GenericParameterPosition == argdef.Key.GenericParameterPosition)

will map the type parameters on position instead of name.

The good news is that all tests will pass after this change.
The bad news is (1) it might not be expected in some cases, e.g.:

public class Base<T1, T2>
{
}

public class Derived<T2, T1> : Base<T1, T2>
{
}

and (2), it doesn't explain why it works for interfaces.

OpenGenericServiceBinder.TryMapImplementationGenericArguments (the calling method) contains the following:

if (!serviceType.IsInterface)
{
    return TryFindServiceArgumentsForImplementation(implementationType, serviceGenericArguments, serviceTypeDefinition.GetGenericArguments());
}

var availableArguments = GetInterfaces(implementationType, serviceType)
    .Select(t => TryFindServiceArgumentsForImplementation(
        implementationType,
        serviceGenericArguments,
        t.GenericTypeArguments))
    .ToArray();

There are two differences between classes and interfaces.

1. For classes serviceTypeDefinition.GetGenericArguments() is used, but for interfaces it is t.GenericTypeArguments
2. For interfaces Autofac first calls GetInterfaces, which does the following:

private static Type[] GetInterfaces(Type implementationType, Type serviceType) =>
    implementationType.GetInterfaces()
        .Where(i => i.Name == serviceType.Name && i.Namespace == serviceType.Namespace)
        .ToArray();

(Not sure why the namespace should match).

[tillig]

(Not sure why the namespace should match).

I think this is to avoid a situation where you have FirstNamespace.IThing<T> and SecondNamespace.IThing<T> with a class that does public class MyThing : IThing<string>. It's probably cheaper to compare the strings than to try building a closed generic and comparing type == type.

[tillig]

The bad news is (1) it might not be expected in some cases

public class Derived<T2, T1> : Base<T1, T2>

😱 yeah, that is not great.

[tillig]

It apparently works for interfaces because the GetInterfaces() call in reflection returns the interface with the generic type parameters renamed to match what it is in the class definition, like:

[Fact]
public void InterfaceTypeParametersRenamed()
{
    var interfaces = typeof(DerivedRepository<,>).GetInterfaces();
    Assert.Single(interfaces);

    // The interface itself is IRepository<T1, T2> but
    // GetInterfaces will return IRepository<TFirst, TSecond>
    // so the names match.
    var args = interfaces[0].GetGenericArguments();
    Assert.Equal("TFirst", args[0].Name);
    Assert.Equal("TSecond", args[1].Name);
}

public interface IRepository<T1, T2>
{
}

public class Repository<T1, T2>
{
}

public class DerivedRepository<TSecond, TFirst> : Repository<TFirst, TSecond>, IRepository<TFirst, TSecond>
{
}

That explains why name matching "just works" in that case, which is news to me.

What I just figured out is that if you use BaseType for classes you'll get the names lining up, too:

        [Fact]
        public void BaseTypeParametersRenamed()
        {
            var baseType = typeof(DerivedRepository<,>).BaseType;
            var args = baseType.GetGenericArguments();
            Assert.Equal("TFirst", args[0].Name);
            Assert.Equal("TSecond", args[1].Name);
        }

So now I'm wondering if a better approach might be:

• Given the service type is a class
• Walk backwards using implementationType.BaseType until the service type generic type definition == the base type generic type definition
• Use the arguments from that call (basically from BaseType)
• Do name matching because now they should match

I think the mismatch here is that we're handed DerivedRepository<TSecond, TFirst> and Repository<T1, T2> and they're not "connected," so to get the name matching to work we need to find the connection.

I have to go do some other stuff for a bit, but that's where I'll probably start looking next.

[weelink]

I was thinking along the same lines. Or meybe fix this a bit earlier when the component is registered.
At least an interesting puzzle.

[tillig]

Super quick and dirty hack works:

        private static Type[] TryMapImplementationGenericArguments(Type implementationType, Type serviceType, Type serviceTypeDefinition, Type[] serviceGenericArguments)
        {
            if (serviceTypeDefinition == implementationType)
            {
                return serviceGenericArguments;
            }

            if (!serviceType.IsInterface)
            {
                var baseType = implementationType.BaseType;
                while (baseType != null)
                {
                    if (baseType.IsGenericType && baseType.GetGenericTypeDefinition() == serviceTypeDefinition)
                    {
                        break;
                    }

                    baseType = baseType.BaseType;
                }

                if (baseType == null)
                {
                    // The original match-by-name-only algorithm
                    return TryFindServiceArgumentsForImplementation(implementationType, serviceGenericArguments, serviceTypeDefinition.GetGenericArguments());
                }

                var matchedArguments = TryFindServiceArgumentsForImplementation(
                        implementationType,
                        serviceGenericArguments,
                        baseType.GenericTypeArguments);
                return matchedArguments;
            }

            var availableArguments = GetInterfaces(implementationType, serviceType)
                .Select(t => TryFindServiceArgumentsForImplementation(
                    implementationType,
                    serviceGenericArguments,
                    t.GenericTypeArguments))
                .ToArray();

            var exactMatch = availableArguments.FirstOrDefault(a => a.SequenceEqual(serviceGenericArguments));
            return exactMatch ?? availableArguments[0];
        }

Obviously needs to be really cleaned up but doing the thing where you walk back the inheritance chain until you find the matching base type does work and all the tests still pass. I'll see if I can clean it up and get a PR in there for folks to look at.

[weelink]

I'll see it when it is done, merged and released (no hurry obviously). For now I know how to make my code work with the latest binary. It was interesting to learn something new about generics.

Thanks for responding so quickly!

[tillig]

OK, got #1316 in for this. All the tests pass including the new ones to verify reordering the parameters works. Nice catch!