-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Fix validation on build #87354
Fix validation on build #87354
Changes from 3 commits
5d8dc5a
a0c95a0
dc51a2d
09249bf
7cfab32
a020ab8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -120,9 +120,9 @@ private void OnCreate(ServiceCallSite callSite) | |
_callSiteValidator?.ValidateCallSite(callSite); | ||
} | ||
|
||
private void OnResolve(Type serviceType, IServiceScope scope) | ||
private void OnResolve(ServiceCallSite callSite, IServiceScope scope) | ||
{ | ||
_callSiteValidator?.ValidateResolution(serviceType, scope, Root); | ||
_callSiteValidator?.ValidateResolution(callSite, scope, Root); | ||
} | ||
|
||
internal object? GetService(Type serviceType, ServiceProviderEngineScope serviceProviderEngineScope) | ||
|
@@ -133,8 +133,6 @@ private void OnResolve(Type serviceType, IServiceScope scope) | |
} | ||
|
||
Func<ServiceProviderEngineScope, object?> realizedService = _realizedServices.GetOrAdd(serviceType, _createServiceAccessor); | ||
OnResolve(serviceType, serviceProviderEngineScope); | ||
DependencyInjectionEventSource.Log.ServiceResolved(this, serviceType); | ||
var result = realizedService.Invoke(serviceProviderEngineScope); | ||
System.Diagnostics.Debug.Assert(result is null || CallSiteFactory.IsService(serviceType)); | ||
return result; | ||
|
@@ -173,10 +171,20 @@ private void ValidateService(ServiceDescriptor descriptor) | |
if (callSite.Cache.Location == CallSiteResultCacheLocation.Root) | ||
{ | ||
object? value = CallSiteRuntimeResolver.Instance.Resolve(callSite, Root); | ||
return scope => value; | ||
return scope => | ||
{ | ||
DependencyInjectionEventSource.Log.ServiceResolved(this, serviceType); | ||
return value; | ||
}; | ||
} | ||
|
||
return _engine.RealizeService(callSite); | ||
Func<ServiceProviderEngineScope, object?> realizedService = _engine.RealizeService(callSite); | ||
return scope => | ||
{ | ||
OnResolve(callSite, scope); | ||
DependencyInjectionEventSource.Log.ServiceResolved(this, serviceType); | ||
return realizedService.Invoke(scope); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
}; | ||
} | ||
|
||
return _ => null; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,8 @@ | |
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using Microsoft.Extensions.DependencyInjection.Specification.Fakes; | ||
using Xunit; | ||
|
||
|
@@ -97,6 +99,49 @@ public void GetService_Throws_WhenGetServiceForScopedServiceIsCalledOnRootViaTra | |
Assert.Equal($"Cannot resolve '{typeof(IFoo)}' from root provider because it requires scoped service '{typeof(IBar)}'.", exception.Message); | ||
} | ||
|
||
[Theory] | ||
[InlineData(true)] | ||
[InlineData(false)] | ||
public void GetService_DoesNotThrow_WhenGetServiceForPolymorphicServiceIsCalledOnRoot_AndTheLastOneIsNotScoped(bool validateOnBuild) | ||
{ | ||
// Arrange | ||
var serviceCollection = new ServiceCollection(); | ||
serviceCollection.AddScoped<IBar, Bar>(); | ||
serviceCollection.AddTransient<IBar, Bar3>(); | ||
using var serviceProvider = serviceCollection.BuildServiceProvider(new ServiceProviderOptions | ||
{ | ||
ValidateScopes = true, | ||
ValidateOnBuild = validateOnBuild | ||
}); | ||
|
||
// Act | ||
var actual = serviceProvider.GetService<IBar>(); | ||
|
||
// Assert | ||
Assert.IsType<Bar3>(actual); | ||
} | ||
|
||
[Fact] | ||
public void ScopeValidation_ShouldBeAbleToDistingushGenericCollections_WhenGetServiceIsCalledOnRoot() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It appears this passes without the changes here. Is that intentional? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, It is intentional. It tests the |
||
{ | ||
// Arrange | ||
var serviceCollection = new ServiceCollection(); | ||
serviceCollection.AddTransient<IBar, Bar>(); | ||
serviceCollection.AddScoped<IBar, Bar3>(); | ||
|
||
serviceCollection.AddTransient<IBaz, Baz>(); | ||
serviceCollection.AddTransient<IBaz, Baz2>(); | ||
|
||
// Act | ||
using var serviceProvider = serviceCollection.BuildServiceProvider(validateScopes: true); | ||
Assert.Throws<InvalidOperationException>(() => serviceProvider.GetService<IEnumerable<IBar>>()); | ||
var actual = serviceProvider.GetService<IEnumerable<IBaz>>(); | ||
|
||
// Assert | ||
Assert.IsType<Baz>(actual.First()); | ||
Assert.IsType<Baz2>(actual.Last()); | ||
} | ||
|
||
[Fact] | ||
public void GetService_DoesNotThrow_WhenScopeFactoryIsInjectedIntoSingleton() | ||
{ | ||
|
@@ -206,13 +251,18 @@ private class Bar : IBar | |
{ | ||
} | ||
|
||
|
||
private class Bar2 : IBar | ||
{ | ||
public Bar2(IBaz baz) | ||
{ | ||
} | ||
} | ||
|
||
private class Bar3 : IBar | ||
{ | ||
} | ||
|
||
private interface IBaz | ||
{ | ||
} | ||
|
@@ -221,6 +271,10 @@ private class Baz : IBaz | |
{ | ||
} | ||
|
||
private class Baz2 : IBaz | ||
{ | ||
} | ||
|
||
private class BazRecursive : IBaz | ||
{ | ||
public BazRecursive(IBaz baz) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The call to
callSite.Cache.Key
should only ever occur once for perf.