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

Add Keyed Services Support to Dependency Injection #87183

Merged
merged 43 commits into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
02b1c4d
Keyed services
benjaminpetit Jun 6, 2023
c26941c
Remove unecessary CompatibilitySuppressions.xml
benjaminpetit Jun 9, 2023
e165c2e
Remove some TODO
benjaminpetit Jun 9, 2023
6fc500f
Minor test cleanup
benjaminpetit Jun 9, 2023
bdc6bfb
Set error message for non keyed service provider
benjaminpetit Jun 9, 2023
b0152ac
Move tests to Specification.Tests
benjaminpetit Jun 19, 2023
57c62f7
Add open generic test
benjaminpetit Jun 19, 2023
e741b3e
dirty fix for ReplaceServiceAccessor
benjaminpetit Jun 20, 2023
3c228f1
Add enumeration tests + fixes
benjaminpetit Jun 20, 2023
f84c8a1
Add custom ToString() to KeyedService.AnyKey
benjaminpetit Jun 22, 2023
ddc7459
Fix CallSiteValidator
benjaminpetit Jun 22, 2023
18f4879
Merge ISupportRequiredKeyedService and ISupportKeyedService
benjaminpetit Jun 22, 2023
3fa39b0
Merge ISupportRequiredKeyedService and ISupportKeyedService
benjaminpetit Jun 22, 2023
8b58809
Fix ServiceCollectionDescriptorExtensions.TryAdd
benjaminpetit Jun 22, 2023
0d17ace
Fix in ServiceCollectionDescriptorExtensions
benjaminpetit Jun 22, 2023
68c3d71
Add IServiceProviderIsServiceKeyed
benjaminpetit Jun 22, 2023
38574c6
Add IServiceProviderIsServiceKeyed
benjaminpetit Jun 22, 2023
24e1e72
Some nit fixes
benjaminpetit Jun 23, 2023
8111510
Some nit fixes
benjaminpetit Jun 23, 2023
b4eb5f2
Cleanup CompatibilitySuppressions.xml
benjaminpetit Jun 23, 2023
7cd4378
WIP: ref fix tentative
benjaminpetit Jun 23, 2023
c8e397c
Fix ref/Microsoft.Extensions.DependencyInjection.Abstractions.cs
benjaminpetit Jun 23, 2023
840d3af
Fix ref/Microsoft.Extensions.DependencyInjection.cs
benjaminpetit Jun 23, 2023
8ccac9e
Segregate keyed and non keyed ServiceDescriptor member, and throw whe…
benjaminpetit Jun 23, 2023
2558c99
Enforce that the type of the parameter marked with the ServiceKey att…
benjaminpetit Jun 23, 2023
aea27cb
Add TryAddKeyed* extensions
benjaminpetit Jun 26, 2023
afeaed1
Fixes and added tests
benjaminpetit Jun 26, 2023
f2beeb2
Rename ISupportKeyedService to IKeyedServiceProvider and make it
benjaminpetit Jun 28, 2023
115c3ef
Rename IServiceProviderIsServiceKeyed to IKeyedServiceProviderIsServi…
benjaminpetit Jun 28, 2023
840d285
Merge extension methods from ServiceCollectionKeyedDescriptorExtensio…
benjaminpetit Jun 28, 2023
8f4e6f3
Merge extension methods from ServiceCollectionKeyedServiceExtensions …
benjaminpetit Jun 28, 2023
845d914
Restore wrongfully removed CompatibilitySuppressions.xml
benjaminpetit Jun 28, 2023
a5705dc
Files renaming
benjaminpetit Jul 5, 2023
460ecb4
Make serviceKey nullable where possible
benjaminpetit Jul 5, 2023
72b2b5d
IServiceProviderIsKeyedService
benjaminpetit Jul 5, 2023
17b9312
Fix ServiceDescriptor.DebuggerToString()
benjaminpetit Jul 5, 2023
ccbedc6
Fix IEnumerableWithIsKeyedServiceAlwaysReturnsTrue
benjaminpetit Jul 5, 2023
db15ad7
Fix ActivatorUtilities support of FromKeyedServicesAttribute
benjaminpetit Jul 5, 2023
ec9b7ef
Minor fixes
benjaminpetit Jul 10, 2023
3a0a306
Remove CompatibilitySuppressions.xml
benjaminpetit Jul 10, 2023
44e2fea
Rollback CompatibilitySuppressions.xml
benjaminpetit Jul 11, 2023
f7680e4
Add tests
benjaminpetit Jul 11, 2023
d9a0270
Make serviceKey nullable even when using a factory
benjaminpetit Jul 11, 2023
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

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,53 @@ private static bool TryCreateParameterMap(ParameterInfo[] constructorParameters,
return true;
}

private static object? GetService(IServiceProvider serviceProvider, ParameterInfo parameterInfo)
{
// Handle keyed service
if (TryGetServiceKey(parameterInfo, out object? key))
{
if (serviceProvider is IKeyedServiceProvider keyedServiceProvider)
{
return keyedServiceProvider.GetKeyedService(parameterInfo.ParameterType, key);
}
throw new InvalidOperationException(SR.KeyedServicesNotSupported);
}
// Try non keyed service
return serviceProvider.GetService(parameterInfo.ParameterType);
}

private static bool IsService(IServiceProviderIsService serviceProviderIsService, ParameterInfo parameterInfo)
{
// Handle keyed service
if (TryGetServiceKey(parameterInfo, out object? key))
{
if (serviceProviderIsService is IServiceProviderIsKeyedService serviceProviderIsKeyedService)
{
return serviceProviderIsKeyedService.IsKeyedService(parameterInfo.ParameterType, key);
}
throw new InvalidOperationException(SR.KeyedServicesNotSupported);
}
// Try non keyed service
return serviceProviderIsService.IsService(parameterInfo.ParameterType);
}

private static bool TryGetServiceKey(ParameterInfo parameterInfo, out object? key)
{
if (parameterInfo.CustomAttributes != null)
{
foreach (var attribute in parameterInfo.GetCustomAttributes(true))
{
if (attribute is FromKeyedServicesAttribute keyed)
{
key = keyed.Key;
return true;
}
}
}
key = null;
return false;
}

private readonly struct ConstructorMatcher
{
private readonly ConstructorInfo _constructor;
Expand Down Expand Up @@ -517,7 +564,7 @@ public int Match(object[] givenParameters, IServiceProviderIsService serviceProv
for (int i = 0; i < _parameters.Length; i++)
{
if (_parameterValues[i] == null &&
!serviceProviderIsService.IsService(_parameters[i].ParameterType))
!IsService(serviceProviderIsService, _parameters[i]))
{
if (ParameterDefaultValue.TryGetDefaultValue(_parameters[i], out object? defaultValue))
{
Expand All @@ -539,7 +586,7 @@ public object CreateInstance(IServiceProvider provider)
{
if (_parameterValues[index] == null)
{
object? value = provider.GetService(_parameters[index].ParameterType);
object? value = GetService(provider, _parameters[index]);
if (value == null)
{
if (!ParameterDefaultValue.TryGetDefaultValue(_parameters[index], out object? defaultValue))
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.Extensions.DependencyInjection.Extensions
/// <summary>
/// Extension methods for adding and removing services to an <see cref="IServiceCollection" />.
/// </summary>
public static class ServiceCollectionDescriptorExtensions
public static partial class ServiceCollectionDescriptorExtensions
{
/// <summary>
/// Adds the specified <paramref name="descriptor"/> to the <paramref name="collection"/>.
Expand Down Expand Up @@ -66,7 +66,8 @@ public static void TryAdd(
int count = collection.Count;
for (int i = 0; i < count; i++)
{
if (collection[i].ServiceType == descriptor.ServiceType)
if (collection[i].ServiceType == descriptor.ServiceType
&& collection[i].ServiceKey == descriptor.ServiceKey)
{
// Already added
return;
Expand Down Expand Up @@ -411,7 +412,7 @@ public static void TryAddSingleton<TService>(this IServiceCollection collection,
ThrowHelper.ThrowIfNull(collection);
ThrowHelper.ThrowIfNull(instance);

var descriptor = ServiceDescriptor.Singleton(typeof(TService), instance);
var descriptor = ServiceDescriptor.Singleton(serviceType: typeof(TService), implementationInstance: instance);
TryAdd(collection, descriptor);
}

Expand Down Expand Up @@ -472,7 +473,8 @@ public static void TryAddEnumerable(
{
ServiceDescriptor service = services[i];
if (service.ServiceType == descriptor.ServiceType &&
service.GetImplementationType() == implementationType)
service.GetImplementationType() == implementationType &&
service.ServiceKey == descriptor.ServiceKey)
{
// Already added
return;
Expand Down Expand Up @@ -530,7 +532,7 @@ public static IServiceCollection Replace(
int count = collection.Count;
for (int i = 0; i < count; i++)
{
if (collection[i].ServiceType == descriptor.ServiceType)
if (collection[i].ServiceType == descriptor.ServiceType && collection[i].ServiceKey == descriptor.ServiceKey)
{
collection.RemoveAt(i);
break;
Expand Down Expand Up @@ -564,7 +566,7 @@ public static IServiceCollection RemoveAll(this IServiceCollection collection, T
for (int i = collection.Count - 1; i >= 0; i--)
{
ServiceDescriptor? descriptor = collection[i];
if (descriptor.ServiceType == serviceType)
if (descriptor.ServiceType == serviceType && descriptor.ServiceKey == null)
{
collection.RemoveAt(i);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;

namespace Microsoft.Extensions.DependencyInjection
{
[AttributeUsage(AttributeTargets.Parameter)]
public class FromKeyedServicesAttribute : Attribute
benjaminpetit marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

sealed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure, the developer can inherit from this attribute to use custom types for the key

Copy link
Member

Choose a reason for hiding this comment

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

Is that a good idea though? Could that interfere with future source generators? We could always shoot of a quick email to get API approval for sealing it. It seems safer since we could always unseal later.

{
public FromKeyedServicesAttribute(object key) => Key = key;

public object Key { get; }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;

namespace Microsoft.Extensions.DependencyInjection
{
public interface IKeyedServiceProvider : IServiceProvider
{
/// <summary>
/// Gets the service object of the specified type.
/// </summary>
/// <param name="serviceType">An object that specifies the type of service object to get.</param>
/// <param name="serviceKey">An object that specifies the key of service object to get.</param>
/// <returns> A service object of type serviceType. -or- null if there is no service object of type serviceType.</returns>
object? GetKeyedService(Type serviceType, object? serviceKey);

/// <summary>
/// Gets service of type <paramref name="serviceType"/> from the <see cref="IServiceProvider"/> implementing
/// this interface.
/// </summary>
/// <param name="serviceType">An object that specifies the type of service object to get.</param>
/// <param name="serviceKey">The <see cref="ServiceDescriptor.ServiceKey"/> of the service.</param>
/// <returns>A service object of type <paramref name="serviceType"/>.
/// Throws an exception if the <see cref="IServiceProvider"/> cannot create the object.</returns>
object GetRequiredKeyedService(Type serviceType, object? serviceKey);
}

public static class KeyedService
{
public static object AnyKey { get; } = new AnyKeyObj();

private sealed class AnyKeyObj
{
public override string? ToString() => "*";
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;

namespace Microsoft.Extensions.DependencyInjection
{
public interface IServiceProviderIsKeyedService : IServiceProviderIsService
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for making the API changes I suggested. As much as I like them, we should at least send an API review email to make sure any API adjustments are approved before merging. Or better yet discuss them in tomorrow's meeting.

{
/// <summary>
/// Determines if the specified service type is available from the <see cref="IServiceProvider"/>.
/// </summary>
/// <param name="serviceType">An object that specifies the type of service object to test.</param>
/// <param name="serviceKey">The <see cref="ServiceDescriptor.ServiceKey"/> of the service.</param>
/// <returns>true if the specified service is a available, false if it is not.</returns>
bool IsKeyedService(Type serviceType, object? serviceKey);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -167,4 +167,13 @@
<value>Multiple constructors accepting all given argument types have been found in type '{0}'. There should only be one applicable constructor.</value>
<comment>{0} = instance type</comment>
</data>
<data name="KeyedServicesNotSupported" xml:space="preserve">
<value>This service provider doesn't support keyed services.</value>
</data>
<data name="KeyedDescriptorMisuse" xml:space="preserve">
<value>This service descriptor is keyed. Your service provider may not support keyed services.</value>
</data>
<data name="NonKeyedDescriptorMisuse" xml:space="preserve">
<value>This service descriptor is not keyed.</value>
</data>
</root>
Loading
Loading