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

Use base type for generic argument matching #1316

Merged
merged 6 commits into from
Mar 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
{
"args": [
"build",
"${workspaceFolder}/Autofac.sln",
"/property:GenerateFullPaths=true",
"/consoleloggerparameters:NoSummary"
],
Expand All @@ -17,6 +18,24 @@
},
"problemMatcher": "$msCompile",
"type": "shell"
},
{
"args": [
"test",
"${workspaceFolder}/Autofac.sln",
"/property:GenerateFullPaths=true",
"/consoleloggerparameters:NoSummary",
"--filter",
"FullyQualifiedName!~Benchmark"
],
"command": "dotnet",
"group": {
"isDefault": true,
"kind": "test"
},
"label": "test",
"problemMatcher": "$msCompile",
"type": "process"
}
],
"version": "2.0.0"
Expand Down
55 changes: 54 additions & 1 deletion src/Autofac/Features/OpenGenerics/OpenGenericServiceBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,44 @@ public static bool TryBindOpenGenericDelegateService(

if (!serviceType.IsInterface)
{
return TryFindServiceArgumentsForImplementation(implementationType, serviceGenericArguments, serviceTypeDefinition.GetGenericArguments());
/* Issue #1315: We walk backwards in the inheritance hierarchy to
* find the open generic because that's the only way to ensure the
* generic argument names match.
*
* If you have a class BaseClass<T1, T2> and a derived class
* DerivedClass<A1, A2> : BaseClass<A2, A1> then then arguments need
* to line up name-wise like...
* A2 -> T1
* A1 -> T2
*
* Having those symbols aligned means that, later, we can do a
* name-based match to populate the generic arguments.
*
* If you do a typeof(BaseClass<,>) then the generic arguments are
* named T1 and T2, which doesn't help us line up the arguments in
* the derived class because the names and order have changed.
* However, if you start at the derived class and walk backwards up
* the inheritance hierarchy then instead of getting the original
* T1/T2 naming, we get the names A1/A2 - the symbols as lined up by
* the compiler.
*
* This is the same reason why Type.GetInterfaces() "just works" -
* it's the generic in relation to the derived/implementing type
* rather than just typeof(MyInterface<,>), so the names all line
* up.
*/
var baseType = GetGenericBaseType(implementationType, serviceTypeDefinition);
tillig marked this conversation as resolved.
Show resolved Hide resolved
if (baseType == null)
{
// If it's not an interface, the implementation type MUST have derived from
// the generic service type at some point or there's no way to cast.
return Array.Empty<Type>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we test this path?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if we can unless something had gone buggy in the checks where we determine if the thing in RegisterGeneric and As are compatible but I can try walking backwards through it to see if we'd ever get there.

}

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

var availableArguments = GetInterfaces(implementationType, serviceType)
Expand All @@ -199,6 +236,22 @@ public static bool TryBindOpenGenericDelegateService(
.ToArray();
}

private static Type? GetGenericBaseType(Type implementationType, Type serviceTypeDefinition)
{
var baseType = implementationType.BaseType;
while (baseType != null)
{
if (baseType.IsGenericType && baseType.GetGenericTypeDefinition() == serviceTypeDefinition)
{
break;
}

baseType = baseType.BaseType;
}

return baseType;
}

private static Type[] GetInterfaces(Type implementationType, Type serviceType) =>
implementationType.GetInterfaces()
.Where(i => i.Name == serviceType.Name && i.Namespace == serviceType.Namespace)
Expand Down
42 changes: 41 additions & 1 deletion test/Autofac.Specification.Test/Registration/OpenGenericTests.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Autofac Project. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

using Autofac.Test.Scenarios.Graph1.GenericContraints;
using Autofac.Test.Scenarios.Graph1.GenericConstraints;

namespace Autofac.Specification.Test.Registration;

Expand Down Expand Up @@ -42,7 +42,47 @@ public void ResolveWithMultipleCandidatesLimitedByGenericConstraintsShouldSuccee
Assert.NotNull(resolved);
}

// Issue #1315: Class services fail to resolve if the names on the type
// arguments are changed.
[Fact]
public void ResolveClassWithRenamedTypeArguments()
{
var containerBuilder = new ContainerBuilder();
containerBuilder.RegisterGeneric(typeof(DerivedRepository<,>)).As(typeof(BaseRepository<,>));

var container = containerBuilder.Build();
var resolved = container.Resolve<BaseRepository<string, int>>();
Assert.IsType<DerivedRepository<int, string>>(resolved);
}

// Issue #1315: Class services fail to resolve if the names on the type
// arguments are changed. We should be able to handle a deep inheritance chain.
[Fact]
public void ResolveClassTwoLevelsDownWithRenamedTypeArguments()
{
var containerBuilder = new ContainerBuilder();
containerBuilder.RegisterGeneric(typeof(TwoLevelsDerivedRepository<,>)).As(typeof(BaseRepository<,>));

var container = containerBuilder.Build();
var resolved = container.Resolve<BaseRepository<string, int>>();
Assert.IsType<TwoLevelsDerivedRepository<int, string>>(resolved);
}

private class SelfComponent<T> : IImplementedInterface<T>
{
}

private class BaseRepository<T1, T2>
{
}

// Issue #1315: Class services fail to resolve if the names on the type
// arguments are changed.
private class DerivedRepository<TSecond, TFirst> : BaseRepository<TFirst, TSecond>
{
}

private class TwoLevelsDerivedRepository<TA, TB> : DerivedRepository<TA, TB>
{
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Autofac Project. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

namespace Autofac.Test.Scenarios.Graph1.GenericContraints;
namespace Autofac.Test.Scenarios.Graph1.GenericConstraints;

public class A : IA
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Autofac Project. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

namespace Autofac.Test.Scenarios.Graph1.GenericContraints;
namespace Autofac.Test.Scenarios.Graph1.GenericConstraints;

public class ClassWithParameterlessButNotPublicConstructor
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Autofac Project. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

namespace Autofac.Test.Scenarios.Graph1.GenericContraints;
namespace Autofac.Test.Scenarios.Graph1.GenericConstraints;

public interface IA
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Autofac Project. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

namespace Autofac.Test.Scenarios.Graph1.GenericContraints;
namespace Autofac.Test.Scenarios.Graph1.GenericConstraints;

public interface IB<TResult>
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Autofac Project. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

namespace Autofac.Test.Scenarios.Graph1.GenericContraints;
namespace Autofac.Test.Scenarios.Graph1.GenericConstraints;

public class Required : IB<ClassWithParameterlessButNotPublicConstructor>
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Autofac Project. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

namespace Autofac.Test.Scenarios.Graph1.GenericContraints;
namespace Autofac.Test.Scenarios.Graph1.GenericConstraints;

public class Unrelated<T> : IB<T>
where T : class, new()
Expand Down