From 5e290f4ed18cbf6d72844377ff27fd9ccec63daf Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 13 Jul 2016 17:09:46 -0700 Subject: [PATCH 1/4] Validate scope into singleton sevice injection --- .../Properties/Resources.Designer.cs | 16 +++++ .../Resources.resx | 3 + ...iceCollectionContainerBuilderExtensions.cs | 7 +- .../ServiceLookup/CallSiteValidator.cs | 68 +++++++++++++++++++ .../ServiceLookup/CallSiteValidatorState.cs | 10 +++ .../ServiceLookup/ClosedIEnumerableService.cs | 3 +- .../ServiceLookup/FactoryService.cs | 2 + .../ServiceLookup/IService.cs | 2 + .../ServiceLookup/InstanceService.cs | 2 + .../ServiceLookup/Service.cs | 2 + .../ServiceLookup/ServiceProviderService.cs | 4 +- .../ServiceLookup/ServiceScopeService.cs | 2 + .../ServiceProvider.cs | 13 +++- .../CallSiteTests.cs | 6 +- .../ServiceLookup/ServiceTest.cs | 6 +- .../ServiceProviderValidationTests.cs | 65 ++++++++++++++++++ 16 files changed, 200 insertions(+), 11 deletions(-) create mode 100644 src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs create mode 100644 src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidatorState.cs create mode 100644 test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs diff --git a/src/Microsoft.Extensions.DependencyInjection/Properties/Resources.Designer.cs b/src/Microsoft.Extensions.DependencyInjection/Properties/Resources.Designer.cs index 7750f25b..c9a8b435 100644 --- a/src/Microsoft.Extensions.DependencyInjection/Properties/Resources.Designer.cs +++ b/src/Microsoft.Extensions.DependencyInjection/Properties/Resources.Designer.cs @@ -122,6 +122,22 @@ internal static string FormatNoConstructorMatch(object p0) return string.Format(CultureInfo.CurrentCulture, GetString("NoConstructorMatch"), p0); } + /// + /// Cannot consume scoped service '{0}' from singleton '{1}'. + /// + internal static string ScopedInSingletonException + { + get { return GetString("ScopedInSingletonException"); } + } + + /// + /// Cannot consume scoped service '{0}' from singleton '{1}'. + /// + internal static string FormatScopedInSingletonException(object p0, object p1) + { + return string.Format(CultureInfo.CurrentCulture, GetString("ScopedInSingletonException"), p0, p1); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.Extensions.DependencyInjection/Resources.resx b/src/Microsoft.Extensions.DependencyInjection/Resources.resx index c7618e0c..54161294 100644 --- a/src/Microsoft.Extensions.DependencyInjection/Resources.resx +++ b/src/Microsoft.Extensions.DependencyInjection/Resources.resx @@ -141,4 +141,7 @@ A suitable constructor for type '{0}' could not be located. Ensure the type is concrete and services are registered for all parameters of a public constructor. {0} = service type + + Cannot consume scoped service '{0}' from singleton '{1}'. + \ No newline at end of file diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceCollectionContainerBuilderExtensions.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceCollectionContainerBuilderExtensions.cs index 824b7ee4..8d230b45 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceCollectionContainerBuilderExtensions.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceCollectionContainerBuilderExtensions.cs @@ -9,7 +9,12 @@ public static class ServiceCollectionContainerBuilderExtensions { public static IServiceProvider BuildServiceProvider(this IServiceCollection services) { - return new ServiceProvider(services); + return BuildServiceProvider(services, validateScopes: false); + } + + public static IServiceProvider BuildServiceProvider(this IServiceCollection services, bool validateScopes) + { + return new ServiceProvider(services, validateScopes); } } } diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs new file mode 100644 index 00000000..f2490d8c --- /dev/null +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs @@ -0,0 +1,68 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; + +namespace Microsoft.Extensions.DependencyInjection.ServiceLookup +{ + internal class CallSiteValidator: CallSiteVisitor + { + public void Validate(IServiceCallSite callSite) + { + VisitCallSite(callSite, default(CallSiteValidatorState)); + } + + protected override object VisitTransient(TransientCallSite transientCallSite, CallSiteValidatorState state) + { + VisitCallSite(transientCallSite.Service, state); + return null; + } + + protected override object VisitConstructor(ConstructorCallSite constructorCallSite, CallSiteValidatorState state) + { + foreach (var parameterCallSite in constructorCallSite.ParameterCallSites) + { + VisitCallSite(parameterCallSite, state); + } + return null; + } + + protected override object VisitSingleton(SingletonCallSite singletonCallSite, CallSiteValidatorState state) + { + state.Singleton = singletonCallSite; + VisitCallSite(singletonCallSite.ServiceCallSite, state); + return null; + } + + protected override object VisitScoped(ScopedCallSite scopedCallSite, CallSiteValidatorState state) + { + // We are fine with having ServiceScopeService requested by singletons + if (scopedCallSite.ServiceCallSite is ServiceScopeService) + { + return null; + } + if (state.Singleton != null) + { + throw new InvalidOperationException(Resources.FormatScopedInSingletonException(scopedCallSite.Key.ServiceType, state.Singleton.Key.ServiceType)); + } + VisitCallSite(scopedCallSite.ServiceCallSite, state); + return null; + } + + protected override object VisitConstant(ConstantCallSite constantCallSite, CallSiteValidatorState state) => null; + + protected override object VisitCreateInstance(CreateInstanceCallSite createInstanceCallSite, CallSiteValidatorState state) => null; + + protected override object VisitInstanceService(InstanceService instanceCallSite, CallSiteValidatorState state) => null; + + protected override object VisitServiceProviderService(ServiceProviderService serviceProviderService, CallSiteValidatorState state) => null; + + protected override object VisitEmptyIEnumerable(EmptyIEnumerableCallSite emptyIEnumerableCallSite, CallSiteValidatorState state) => null; + + protected override object VisitServiceScopeService(ServiceScopeService serviceScopeService, CallSiteValidatorState state) => null; + + protected override object VisitClosedIEnumerable(ClosedIEnumerableCallSite closedIEnumerableCallSite, CallSiteValidatorState state) => null; + + protected override object VisitFactoryService(FactoryService factoryService, CallSiteValidatorState state) => null; + } +} \ No newline at end of file diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidatorState.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidatorState.cs new file mode 100644 index 00000000..e46c5ed0 --- /dev/null +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidatorState.cs @@ -0,0 +1,10 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.Extensions.DependencyInjection.ServiceLookup +{ + internal struct CallSiteValidatorState + { + public SingletonCallSite Singleton; + } +} \ No newline at end of file diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/ClosedIEnumerableService.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/ClosedIEnumerableService.cs index 9ccc9422..d7215472 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/ClosedIEnumerableService.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/ClosedIEnumerableService.cs @@ -25,6 +25,8 @@ public ServiceLifetime Lifetime get { return ServiceLifetime.Transient; } } + public Type ServiceType => _itemType; + public IServiceCallSite CreateCallSite(ServiceProvider provider, ISet callSiteChain) { var list = new List(); @@ -36,6 +38,5 @@ public IServiceCallSite CreateCallSite(ServiceProvider provider, ISet call } return new ClosedIEnumerableCallSite(_itemType, list.ToArray()); } - } } diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/FactoryService.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/FactoryService.cs index 7e9e9e76..e65e4144 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/FactoryService.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/FactoryService.cs @@ -22,6 +22,8 @@ public ServiceLifetime Lifetime get { return Descriptor.Lifetime; } } + public Type ServiceType => Descriptor.ServiceType; + public IServiceCallSite CreateCallSite(ServiceProvider provider, ISet callSiteChain) { return this; diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/IService.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/IService.cs index fc5bcadf..d76ee067 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/IService.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/IService.cs @@ -13,5 +13,7 @@ internal interface IService ServiceLifetime Lifetime { get; } IServiceCallSite CreateCallSite(ServiceProvider provider, ISet callSiteChain); + + Type ServiceType { get; } } } diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/InstanceService.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/InstanceService.cs index 46e63a02..1d1c76ea 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/InstanceService.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/InstanceService.cs @@ -25,6 +25,8 @@ public ServiceLifetime Lifetime get { return Descriptor.Lifetime; } } + public Type ServiceType => Descriptor.ServiceType; + public IServiceCallSite CreateCallSite(ServiceProvider provider, ISet callSiteChain) { return this; diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/Service.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/Service.cs index 9c4a43e0..4cdccd06 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/Service.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/Service.cs @@ -117,6 +117,8 @@ public IServiceCallSite CreateCallSite(ServiceProvider provider, ISet call } } + public Type ServiceType => _descriptor.ServiceType; + private bool IsSuperset(IEnumerable left, IEnumerable right) { return new HashSet(left).IsSupersetOf(right); diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/ServiceProviderService.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/ServiceProviderService.cs index 60ad8f97..e894b60c 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/ServiceProviderService.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/ServiceProviderService.cs @@ -12,9 +12,11 @@ internal class ServiceProviderService : IService, IServiceCallSite public ServiceLifetime Lifetime { - get { return ServiceLifetime.Scoped; } + get { return ServiceLifetime.Transient; } } + public Type ServiceType => typeof(IServiceProvider); + public IServiceCallSite CreateCallSite(ServiceProvider provider, ISet callSiteChain) { return this; diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/ServiceScopeService.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/ServiceScopeService.cs index 012a90ad..bcb006cd 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/ServiceScopeService.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/ServiceScopeService.cs @@ -15,6 +15,8 @@ public ServiceLifetime Lifetime get { return ServiceLifetime.Scoped; } } + public Type ServiceType => typeof(IServiceScopeFactory); + public IServiceCallSite CreateCallSite(ServiceProvider provider, ISet callSiteChain) { return this; diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceProvider.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceProvider.cs index c7790d77..63506f87 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceProvider.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceProvider.cs @@ -17,6 +17,7 @@ namespace Microsoft.Extensions.DependencyInjection /// internal class ServiceProvider : IServiceProvider, IDisposable { + private readonly bool _validateScopes; private readonly ServiceTable _table; private bool _disposeCalled; private List _transientDisposables; @@ -26,12 +27,15 @@ internal class ServiceProvider : IServiceProvider, IDisposable private static readonly Func> _createServiceAccessor = CreateServiceAccessor; - // CallSiteRuntimeResolver is stateless so can be shared between all instances + // CallSiteRuntimeResolver and CallSiteValidator are stateless so can be shared between all instances private static readonly CallSiteRuntimeResolver _callSiteRuntimeResolver = new CallSiteRuntimeResolver(); + private static readonly CallSiteValidator _callSiteValidator = new CallSiteValidator(); - public ServiceProvider(IEnumerable serviceDescriptors) + public ServiceProvider(IEnumerable serviceDescriptors, bool validateScopes) { Root = this; + + _validateScopes = validateScopes; _table = new ServiceTable(serviceDescriptors); _table.Add(typeof(IServiceProvider), new ServiceProviderService()); @@ -44,6 +48,7 @@ internal ServiceProvider(ServiceProvider parent) { Root = parent.Root; _table = parent._table; + _validateScopes = parent._validateScopes; } /// @@ -62,6 +67,10 @@ private static Func CreateServiceAccessor(Type serviceT var callSite = serviceProvider.GetServiceCallSite(serviceType, new HashSet()); if (callSite != null) { + if (serviceProvider._validateScopes) + { + _callSiteValidator.Validate(callSite); + } return RealizeService(serviceProvider._table, serviceType, callSite); } diff --git a/test/Microsoft.Extensions.DependencyInjection.Tests/CallSiteTests.cs b/test/Microsoft.Extensions.DependencyInjection.Tests/CallSiteTests.cs index 1f261f11..7b7975b1 100644 --- a/test/Microsoft.Extensions.DependencyInjection.Tests/CallSiteTests.cs +++ b/test/Microsoft.Extensions.DependencyInjection.Tests/CallSiteTests.cs @@ -82,7 +82,7 @@ public static IEnumerable TestServiceDescriptors(ServiceLifetime lifet public void BuiltExpressionWillReturnResolvedServiceWhenAppropriate( ServiceDescriptor[] desciptors, Type serviceType, Func compare) { - var provider = new ServiceProvider(desciptors); + var provider = new ServiceProvider(desciptors, validateScopes: false); var callSite = provider.GetServiceCallSite(serviceType, new HashSet()); var collectionCallSite = provider.GetServiceCallSite(typeof(IEnumerable<>).MakeGenericType(serviceType), new HashSet()); @@ -111,7 +111,7 @@ public void BuiltExpressionCanResolveNestedScopedService() descriptors.AddScoped(); descriptors.AddScoped(); - var provider = new ServiceProvider(descriptors); + var provider = new ServiceProvider(descriptors, validateScopes: false); var callSite = provider.GetServiceCallSite(typeof(ServiceC), new HashSet()); var compiledCallSite = CompileCallSite(callSite); @@ -129,7 +129,7 @@ public void BuiltExpressionRethrowsOriginalExceptionFromConstructor() descriptors.AddTransient(); descriptors.AddTransient(); - var provider = new ServiceProvider(descriptors); + var provider = new ServiceProvider(descriptors, validateScopes: false); var callSite1 = provider.GetServiceCallSite(typeof(ClassWithThrowingEmptyCtor), new HashSet()); var compiledCallSite1 = CompileCallSite(callSite1); diff --git a/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceLookup/ServiceTest.cs b/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceLookup/ServiceTest.cs index 23d41ea2..6fd3a2e7 100644 --- a/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceLookup/ServiceTest.cs +++ b/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceLookup/ServiceTest.cs @@ -22,7 +22,7 @@ public void CreateCallSite_Throws_IfTypeHasNoPublicConstructors() "Ensure the type is concrete and services are registered for all parameters of a public constructor."; var descriptor = new ServiceDescriptor(type, type, ServiceLifetime.Transient); var service = new Service(descriptor); - var serviceProvider = new ServiceProvider(new[] { descriptor }); + var serviceProvider = new ServiceProvider(new[] { descriptor }, validateScopes: false); // Act and Assert var ex = Assert.Throws(() => service.CreateCallSite(serviceProvider, new HashSet())); @@ -38,7 +38,7 @@ public void CreateCallSite_CreatesInstanceCallSite_IfTypeHasDefaultOrPublicParam // Arrange var descriptor = new ServiceDescriptor(type, type, ServiceLifetime.Transient); var service = new Service(descriptor); - var serviceProvider = new ServiceProvider(new[] { descriptor }); + var serviceProvider = new ServiceProvider(new[] { descriptor }, validateScopes: false); // Act var callSite = service.CreateCallSite(serviceProvider, new HashSet()); @@ -99,7 +99,7 @@ public void CreateCallSite_UsesNullaryConstructorIfServicesCannotBeInjectedIntoO var type = typeof(TypeWithParameterizedAndNullaryConstructor); var descriptor = new ServiceDescriptor(type, type, ServiceLifetime.Transient); var service = new Service(descriptor); - var serviceProvider = new ServiceProvider(new[] { descriptor }); + var serviceProvider = new ServiceProvider(new[] { descriptor }, validateScopes: false); // Act var callSite = service.CreateCallSite(serviceProvider, new HashSet()); diff --git a/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs b/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs new file mode 100644 index 00000000..f65bbf49 --- /dev/null +++ b/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs @@ -0,0 +1,65 @@ +using System; +using Xunit; + +namespace Microsoft.Extensions.DependencyInjection.Tests +{ + public class ServiceProviderValidationTests + { + [Fact] + public void GetService_Throws_WhenScopedIsInjectedIntoSingleton() + { + // Arrange + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(); + serviceCollection.AddScoped(); + var serviceProvider = serviceCollection.BuildServiceProvider(true); + + // Act + Assert + var exception = Assert.Throws(() => serviceProvider.GetService(typeof(IFoo))); + Assert.Equal($"Cannot consume scoped service '{typeof(IBar)}' from singleton '{typeof(IFoo)}'.", exception.Message); + } + + public void GetService_DoesNotThrow_WhenScopeFactoryIsInjectedIntoSingleton() + { + // Arrange + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(); + var serviceProvider = serviceCollection.BuildServiceProvider(true); + + // Act + Assert + var result = serviceProvider.GetService(typeof(IBoo)); + Assert.NotNull(result); + } + + private interface IFoo + { + } + + private class Foo : IFoo + { + public Foo(IBar bar) + { + } + } + + private interface IBar + { + } + + private class Bar : IBar + { + } + + private interface IBoo + { + } + + private class Boo : IBoo + { + public Boo(IServiceScopeFactory scopeFactory) + { + } + } + + } +} From 89f641f46566122668706e872bb3c6a608246612 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Thu, 14 Jul 2016 09:57:13 -0700 Subject: [PATCH 2/4] Address PR comments --- .../ServiceLookup/CallSiteValidator.cs | 7 ++- .../ServiceLookup/CallSiteValidatorState.cs | 10 ---- .../CallSiteTests.cs | 6 +-- .../ServiceLookup/ServiceTest.cs | 6 +-- .../ServiceProviderValidationTests.cs | 53 +++++++++++++++++-- 5 files changed, 62 insertions(+), 20 deletions(-) delete mode 100644 src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidatorState.cs diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs index f2490d8c..b722be4b 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs @@ -5,7 +5,7 @@ namespace Microsoft.Extensions.DependencyInjection.ServiceLookup { - internal class CallSiteValidator: CallSiteVisitor + internal class CallSiteValidator: CallSiteVisitor { public void Validate(IServiceCallSite callSite) { @@ -64,5 +64,10 @@ protected override object VisitScoped(ScopedCallSite scopedCallSite, CallSiteVal protected override object VisitClosedIEnumerable(ClosedIEnumerableCallSite closedIEnumerableCallSite, CallSiteValidatorState state) => null; protected override object VisitFactoryService(FactoryService factoryService, CallSiteValidatorState state) => null; + + internal struct CallSiteValidatorState + { + public SingletonCallSite Singleton { get; set; } + } } } \ No newline at end of file diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidatorState.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidatorState.cs deleted file mode 100644 index e46c5ed0..00000000 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidatorState.cs +++ /dev/null @@ -1,10 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Microsoft.Extensions.DependencyInjection.ServiceLookup -{ - internal struct CallSiteValidatorState - { - public SingletonCallSite Singleton; - } -} \ No newline at end of file diff --git a/test/Microsoft.Extensions.DependencyInjection.Tests/CallSiteTests.cs b/test/Microsoft.Extensions.DependencyInjection.Tests/CallSiteTests.cs index 7b7975b1..0abac19b 100644 --- a/test/Microsoft.Extensions.DependencyInjection.Tests/CallSiteTests.cs +++ b/test/Microsoft.Extensions.DependencyInjection.Tests/CallSiteTests.cs @@ -82,7 +82,7 @@ public static IEnumerable TestServiceDescriptors(ServiceLifetime lifet public void BuiltExpressionWillReturnResolvedServiceWhenAppropriate( ServiceDescriptor[] desciptors, Type serviceType, Func compare) { - var provider = new ServiceProvider(desciptors, validateScopes: false); + var provider = new ServiceProvider(desciptors, validateScopes: true); var callSite = provider.GetServiceCallSite(serviceType, new HashSet()); var collectionCallSite = provider.GetServiceCallSite(typeof(IEnumerable<>).MakeGenericType(serviceType), new HashSet()); @@ -111,7 +111,7 @@ public void BuiltExpressionCanResolveNestedScopedService() descriptors.AddScoped(); descriptors.AddScoped(); - var provider = new ServiceProvider(descriptors, validateScopes: false); + var provider = new ServiceProvider(descriptors, validateScopes: true); var callSite = provider.GetServiceCallSite(typeof(ServiceC), new HashSet()); var compiledCallSite = CompileCallSite(callSite); @@ -129,7 +129,7 @@ public void BuiltExpressionRethrowsOriginalExceptionFromConstructor() descriptors.AddTransient(); descriptors.AddTransient(); - var provider = new ServiceProvider(descriptors, validateScopes: false); + var provider = new ServiceProvider(descriptors, validateScopes: true); var callSite1 = provider.GetServiceCallSite(typeof(ClassWithThrowingEmptyCtor), new HashSet()); var compiledCallSite1 = CompileCallSite(callSite1); diff --git a/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceLookup/ServiceTest.cs b/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceLookup/ServiceTest.cs index 6fd3a2e7..2036b0ac 100644 --- a/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceLookup/ServiceTest.cs +++ b/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceLookup/ServiceTest.cs @@ -22,7 +22,7 @@ public void CreateCallSite_Throws_IfTypeHasNoPublicConstructors() "Ensure the type is concrete and services are registered for all parameters of a public constructor."; var descriptor = new ServiceDescriptor(type, type, ServiceLifetime.Transient); var service = new Service(descriptor); - var serviceProvider = new ServiceProvider(new[] { descriptor }, validateScopes: false); + var serviceProvider = new ServiceProvider(new[] { descriptor }, validateScopes: true); // Act and Assert var ex = Assert.Throws(() => service.CreateCallSite(serviceProvider, new HashSet())); @@ -38,7 +38,7 @@ public void CreateCallSite_CreatesInstanceCallSite_IfTypeHasDefaultOrPublicParam // Arrange var descriptor = new ServiceDescriptor(type, type, ServiceLifetime.Transient); var service = new Service(descriptor); - var serviceProvider = new ServiceProvider(new[] { descriptor }, validateScopes: false); + var serviceProvider = new ServiceProvider(new[] { descriptor }, validateScopes: true); // Act var callSite = service.CreateCallSite(serviceProvider, new HashSet()); @@ -99,7 +99,7 @@ public void CreateCallSite_UsesNullaryConstructorIfServicesCannotBeInjectedIntoO var type = typeof(TypeWithParameterizedAndNullaryConstructor); var descriptor = new ServiceDescriptor(type, type, ServiceLifetime.Transient); var service = new Service(descriptor); - var serviceProvider = new ServiceProvider(new[] { descriptor }, validateScopes: false); + var serviceProvider = new ServiceProvider(new[] { descriptor }, validateScopes: true); // Act var callSite = service.CreateCallSite(serviceProvider, new HashSet()); diff --git a/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs b/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs index f65bbf49..b4e7733f 100644 --- a/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs +++ b/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs @@ -1,4 +1,7 @@ -using System; +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; using Xunit; namespace Microsoft.Extensions.DependencyInjection.Tests @@ -12,13 +15,43 @@ public void GetService_Throws_WhenScopedIsInjectedIntoSingleton() var serviceCollection = new ServiceCollection(); serviceCollection.AddSingleton(); serviceCollection.AddScoped(); - var serviceProvider = serviceCollection.BuildServiceProvider(true); + var serviceProvider = serviceCollection.BuildServiceProvider(validateScopes: true); // Act + Assert var exception = Assert.Throws(() => serviceProvider.GetService(typeof(IFoo))); Assert.Equal($"Cannot consume scoped service '{typeof(IBar)}' from singleton '{typeof(IFoo)}'.", exception.Message); } + [Fact] + public void GetService_Throws_WhenScopedIsInjectedIntoSingletonThroughTransient() + { + // Arrange + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(); + serviceCollection.AddTransient(); + serviceCollection.AddScoped(); + var serviceProvider = serviceCollection.BuildServiceProvider(validateScopes: true); + + // Act + Assert + var exception = Assert.Throws(() => serviceProvider.GetService(typeof(IFoo))); + Assert.Equal($"Cannot consume scoped service '{typeof(IBaz)}' from singleton '{typeof(IFoo)}'.", exception.Message); + } + + [Fact] + public void GetService_Throws_WhenScopedIsInjectedIntoSingletonThroughSingleton() + { + // Arrange + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(); + serviceCollection.AddSingleton(); + serviceCollection.AddScoped(); + var serviceProvider = serviceCollection.BuildServiceProvider(validateScopes: true); + + // Act + Assert + var exception = Assert.Throws(() => serviceProvider.GetService(typeof(IFoo))); + Assert.Equal($"Cannot consume scoped service '{typeof(IBaz)}' from singleton '{typeof(IBar)}'.", exception.Message); + } + public void GetService_DoesNotThrow_WhenScopeFactoryIsInjectedIntoSingleton() { // Arrange @@ -50,6 +83,21 @@ private class Bar : IBar { } + private class Bar2 : IBar + { + public Bar2(IBaz baz) + { + } + } + + private interface IBaz + { + } + + private class Baz : IBaz + { + } + private interface IBoo { } @@ -60,6 +108,5 @@ public Boo(IServiceScopeFactory scopeFactory) { } } - } } From 9d352ada3e02603cc548fa289bb8186ae7cf8950 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Thu, 14 Jul 2016 14:02:42 -0700 Subject: [PATCH 3/4] GetService check --- .../Properties/Resources.Designer.cs | 24 ++++- .../Resources.resx | 5 +- ...iceCollectionContainerBuilderExtensions.cs | 14 +++ .../ServiceLookup/CallSiteValidator.cs | 91 ++++++++++++++----- .../ServiceProvider.cs | 21 +++-- .../ServiceProviderValidationTests.cs | 16 ++++ 6 files changed, 132 insertions(+), 39 deletions(-) diff --git a/src/Microsoft.Extensions.DependencyInjection/Properties/Resources.Designer.cs b/src/Microsoft.Extensions.DependencyInjection/Properties/Resources.Designer.cs index c9a8b435..54d2249a 100644 --- a/src/Microsoft.Extensions.DependencyInjection/Properties/Resources.Designer.cs +++ b/src/Microsoft.Extensions.DependencyInjection/Properties/Resources.Designer.cs @@ -123,7 +123,7 @@ internal static string FormatNoConstructorMatch(object p0) } /// - /// Cannot consume scoped service '{0}' from singleton '{1}'. + /// Cannot consume {2} service '{0}' from {3} '{1}'. /// internal static string ScopedInSingletonException { @@ -131,11 +131,27 @@ internal static string ScopedInSingletonException } /// - /// Cannot consume scoped service '{0}' from singleton '{1}'. + /// Cannot consume {2} service '{0}' from {3} '{1}'. /// - internal static string FormatScopedInSingletonException(object p0, object p1) + internal static string FormatScopedInSingletonException(object p0, object p1, object p2, object p3) { - return string.Format(CultureInfo.CurrentCulture, GetString("ScopedInSingletonException"), p0, p1); + return string.Format(CultureInfo.CurrentCulture, GetString("ScopedInSingletonException"), p0, p1, p2, p3); + } + + /// + /// Cannot resolve '{0}' from root provider because it requires {2} service '{1}'. + /// + internal static string ScopedResolvedFromRootException + { + get { return GetString("ScopedResolvedFromRootException"); } + } + + /// + /// Cannot resolve '{0}' from root provider because it requires {2} service '{1}'. + /// + internal static string FormatScopedResolvedFromRootException(object p0, object p1, object p2) + { + return string.Format(CultureInfo.CurrentCulture, GetString("ScopedResolvedFromRootException"), p0, p1, p2); } private static string GetString(string name, params string[] formatterNames) diff --git a/src/Microsoft.Extensions.DependencyInjection/Resources.resx b/src/Microsoft.Extensions.DependencyInjection/Resources.resx index 54161294..c06813f0 100644 --- a/src/Microsoft.Extensions.DependencyInjection/Resources.resx +++ b/src/Microsoft.Extensions.DependencyInjection/Resources.resx @@ -142,6 +142,9 @@ {0} = service type - Cannot consume scoped service '{0}' from singleton '{1}'. + Cannot consume {2} service '{0}' from {3} '{1}'. + + + Cannot resolve '{0}' from root provider because it requires {2} service '{1}'. \ No newline at end of file diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceCollectionContainerBuilderExtensions.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceCollectionContainerBuilderExtensions.cs index 8d230b45..15a647b2 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceCollectionContainerBuilderExtensions.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceCollectionContainerBuilderExtensions.cs @@ -7,11 +7,25 @@ namespace Microsoft.Extensions.DependencyInjection { public static class ServiceCollectionContainerBuilderExtensions { + /// + /// Creates an containing services from the provided . + /// + /// The containing service descriptors. + /// An instance of providing access to services. public static IServiceProvider BuildServiceProvider(this IServiceCollection services) { return BuildServiceProvider(services, validateScopes: false); } + /// + /// Creates an containing services from the provided + /// optionaly enabling scope validation. + /// + /// The containing service descriptors. + /// + /// true to perform check verifying that scoped services never gets resolved from root provider; otherwise false. + /// + /// An instance of providing access to services. public static IServiceProvider BuildServiceProvider(this IServiceCollection services, bool validateScopes) { return new ServiceProvider(services, validateScopes); diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs index b722be4b..953664a0 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs @@ -2,39 +2,77 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; namespace Microsoft.Extensions.DependencyInjection.ServiceLookup { - internal class CallSiteValidator: CallSiteVisitor + internal class CallSiteValidator: CallSiteVisitor { - public void Validate(IServiceCallSite callSite) + private readonly Dictionary _scopedServices = new Dictionary(); + + public void ValidateCallSite(Type serviceType, IServiceCallSite callSite) { - VisitCallSite(callSite, default(CallSiteValidatorState)); + var scoped = VisitCallSite(callSite, default(CallSiteValidatorState)); + if (scoped != null) + { + _scopedServices.Add(serviceType, scoped); + } + } + + public void ValidateResolution(Type serviceType, ServiceProvider serviceProvider) + { + Type scopedService; + if (ReferenceEquals(serviceProvider, serviceProvider.Root) + && _scopedServices.TryGetValue(serviceType, out scopedService)) + { + throw new InvalidOperationException(Resources.FormatScopedResolvedFromRootException( + serviceType, + scopedService, + nameof(ServiceLifetime.Scoped).ToLowerInvariant())); + } } - protected override object VisitTransient(TransientCallSite transientCallSite, CallSiteValidatorState state) + protected override Type VisitTransient(TransientCallSite transientCallSite, CallSiteValidatorState state) { - VisitCallSite(transientCallSite.Service, state); - return null; + return VisitCallSite(transientCallSite.Service, state); } - protected override object VisitConstructor(ConstructorCallSite constructorCallSite, CallSiteValidatorState state) + protected override Type VisitConstructor(ConstructorCallSite constructorCallSite, CallSiteValidatorState state) { + Type result = null; foreach (var parameterCallSite in constructorCallSite.ParameterCallSites) { - VisitCallSite(parameterCallSite, state); + var scoped = VisitCallSite(parameterCallSite, state); + if (result == null) + { + result = scoped; + } } - return null; + return result; } - protected override object VisitSingleton(SingletonCallSite singletonCallSite, CallSiteValidatorState state) + protected override Type VisitClosedIEnumerable(ClosedIEnumerableCallSite closedIEnumerableCallSite, + CallSiteValidatorState state) + { + Type result = null; + foreach (var serviceCallSite in closedIEnumerableCallSite.ServiceCallSites) + { + var scoped = VisitCallSite(serviceCallSite, state); + if (result == null) + { + result = scoped; + } + } + return result; + } + + protected override Type VisitSingleton(SingletonCallSite singletonCallSite, CallSiteValidatorState state) { state.Singleton = singletonCallSite; - VisitCallSite(singletonCallSite.ServiceCallSite, state); - return null; + return VisitCallSite(singletonCallSite.ServiceCallSite, state); } - protected override object VisitScoped(ScopedCallSite scopedCallSite, CallSiteValidatorState state) + protected override Type VisitScoped(ScopedCallSite scopedCallSite, CallSiteValidatorState state) { // We are fine with having ServiceScopeService requested by singletons if (scopedCallSite.ServiceCallSite is ServiceScopeService) @@ -43,31 +81,34 @@ protected override object VisitScoped(ScopedCallSite scopedCallSite, CallSiteVal } if (state.Singleton != null) { - throw new InvalidOperationException(Resources.FormatScopedInSingletonException(scopedCallSite.Key.ServiceType, state.Singleton.Key.ServiceType)); + throw new InvalidOperationException(Resources.FormatScopedInSingletonException( + scopedCallSite.Key.ServiceType, + state.Singleton.Key.ServiceType, + nameof(ServiceLifetime.Scoped).ToLowerInvariant(), + nameof(ServiceLifetime.Singleton).ToLowerInvariant() + )); } - VisitCallSite(scopedCallSite.ServiceCallSite, state); - return null; + return scopedCallSite.Key.ServiceType; } - protected override object VisitConstant(ConstantCallSite constantCallSite, CallSiteValidatorState state) => null; - - protected override object VisitCreateInstance(CreateInstanceCallSite createInstanceCallSite, CallSiteValidatorState state) => null; + protected override Type VisitConstant(ConstantCallSite constantCallSite, CallSiteValidatorState state) => null; - protected override object VisitInstanceService(InstanceService instanceCallSite, CallSiteValidatorState state) => null; + protected override Type VisitCreateInstance(CreateInstanceCallSite createInstanceCallSite, CallSiteValidatorState state) => null; - protected override object VisitServiceProviderService(ServiceProviderService serviceProviderService, CallSiteValidatorState state) => null; + protected override Type VisitInstanceService(InstanceService instanceCallSite, CallSiteValidatorState state) => null; - protected override object VisitEmptyIEnumerable(EmptyIEnumerableCallSite emptyIEnumerableCallSite, CallSiteValidatorState state) => null; + protected override Type VisitServiceProviderService(ServiceProviderService serviceProviderService, CallSiteValidatorState state) => null; - protected override object VisitServiceScopeService(ServiceScopeService serviceScopeService, CallSiteValidatorState state) => null; + protected override Type VisitEmptyIEnumerable(EmptyIEnumerableCallSite emptyIEnumerableCallSite, CallSiteValidatorState state) => null; - protected override object VisitClosedIEnumerable(ClosedIEnumerableCallSite closedIEnumerableCallSite, CallSiteValidatorState state) => null; + protected override Type VisitServiceScopeService(ServiceScopeService serviceScopeService, CallSiteValidatorState state) => null; - protected override object VisitFactoryService(FactoryService factoryService, CallSiteValidatorState state) => null; + protected override Type VisitFactoryService(FactoryService factoryService, CallSiteValidatorState state) => null; internal struct CallSiteValidatorState { public SingletonCallSite Singleton { get; set; } + public List Scoped { get; set; } } } } \ No newline at end of file diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceProvider.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceProvider.cs index 63506f87..4ce8d813 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceProvider.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceProvider.cs @@ -17,7 +17,7 @@ namespace Microsoft.Extensions.DependencyInjection /// internal class ServiceProvider : IServiceProvider, IDisposable { - private readonly bool _validateScopes; + private readonly CallSiteValidator _callSiteValidator; private readonly ServiceTable _table; private bool _disposeCalled; private List _transientDisposables; @@ -27,15 +27,18 @@ internal class ServiceProvider : IServiceProvider, IDisposable private static readonly Func> _createServiceAccessor = CreateServiceAccessor; - // CallSiteRuntimeResolver and CallSiteValidator are stateless so can be shared between all instances + // CallSiteRuntimeResolver is stateless so can be shared between all instances private static readonly CallSiteRuntimeResolver _callSiteRuntimeResolver = new CallSiteRuntimeResolver(); - private static readonly CallSiteValidator _callSiteValidator = new CallSiteValidator(); public ServiceProvider(IEnumerable serviceDescriptors, bool validateScopes) { Root = this; - _validateScopes = validateScopes; + if (validateScopes) + { + _callSiteValidator = new CallSiteValidator(); + } + _table = new ServiceTable(serviceDescriptors); _table.Add(typeof(IServiceProvider), new ServiceProviderService()); @@ -48,7 +51,7 @@ internal ServiceProvider(ServiceProvider parent) { Root = parent.Root; _table = parent._table; - _validateScopes = parent._validateScopes; + _callSiteValidator = parent._callSiteValidator; } /// @@ -59,6 +62,9 @@ internal ServiceProvider(ServiceProvider parent) public object GetService(Type serviceType) { var realizedService = _table.RealizedServices.GetOrAdd(serviceType, _createServiceAccessor, this); + + _callSiteValidator?.ValidateResolution(serviceType, this); + return realizedService.Invoke(this); } @@ -67,10 +73,7 @@ private static Func CreateServiceAccessor(Type serviceT var callSite = serviceProvider.GetServiceCallSite(serviceType, new HashSet()); if (callSite != null) { - if (serviceProvider._validateScopes) - { - _callSiteValidator.Validate(callSite); - } + serviceProvider._callSiteValidator?.ValidateCallSite(serviceType, callSite); return RealizeService(serviceProvider._table, serviceType, callSite); } diff --git a/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs b/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs index b4e7733f..a2ed452f 100644 --- a/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs +++ b/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs @@ -52,6 +52,22 @@ public void GetService_Throws_WhenScopedIsInjectedIntoSingletonThroughSingleton( Assert.Equal($"Cannot consume scoped service '{typeof(IBaz)}' from singleton '{typeof(IBar)}'.", exception.Message); } + [Fact] + public void GetService_Throws_WhenGetServiceForScopedServiceIsCalledOnRoot() + { + // Arrange + var serviceCollection = new ServiceCollection(); + serviceCollection.AddTransient(); + serviceCollection.AddScoped(); + var serviceProvider = serviceCollection.BuildServiceProvider(validateScopes: true); + + // Act + Assert + var exception = Assert.Throws(() => serviceProvider.GetService(typeof(IFoo))); + Assert.Equal($"Cannot resolve '{typeof(IFoo)}' from root provider because it requires scoped service '{typeof(IBar)}'.", exception.Message); + } + + [Fact] + public void GetService_DoesNotThrow_WhenScopeFactoryIsInjectedIntoSingleton() { // Arrange From 462ff92d87011f48031d5be28fb116d0cbecb579 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Fri, 15 Jul 2016 08:37:30 -0700 Subject: [PATCH 4/4] Another test and exception message --- .../Properties/Resources.Designer.cs | 16 ++++++++++++++++ .../Resources.resx | 3 +++ ...viceCollectionContainerBuilderExtensions.cs | 5 +++-- .../ServiceLookup/CallSiteValidator.cs | 18 +++++++++++++----- .../ServiceProviderValidationTests.cs | 14 +++++++++++++- 5 files changed, 48 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Extensions.DependencyInjection/Properties/Resources.Designer.cs b/src/Microsoft.Extensions.DependencyInjection/Properties/Resources.Designer.cs index 54d2249a..2721925e 100644 --- a/src/Microsoft.Extensions.DependencyInjection/Properties/Resources.Designer.cs +++ b/src/Microsoft.Extensions.DependencyInjection/Properties/Resources.Designer.cs @@ -154,6 +154,22 @@ internal static string FormatScopedResolvedFromRootException(object p0, object p return string.Format(CultureInfo.CurrentCulture, GetString("ScopedResolvedFromRootException"), p0, p1, p2); } + /// + /// Cannot resolve {1} service '{0}' from root provider. + /// + internal static string DirectScopedResolvedFromRootException + { + get { return GetString("DirectScopedResolvedFromRootException"); } + } + + /// + /// Cannot resolve {1} service '{0}' from root provider. + /// + internal static string FormatDirectScopedResolvedFromRootException(object p0, object p1) + { + return string.Format(CultureInfo.CurrentCulture, GetString("DirectScopedResolvedFromRootException"), p0, p1); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.Extensions.DependencyInjection/Resources.resx b/src/Microsoft.Extensions.DependencyInjection/Resources.resx index c06813f0..259f794b 100644 --- a/src/Microsoft.Extensions.DependencyInjection/Resources.resx +++ b/src/Microsoft.Extensions.DependencyInjection/Resources.resx @@ -147,4 +147,7 @@ Cannot resolve '{0}' from root provider because it requires {2} service '{1}'. + + Cannot resolve {1} service '{0}' from root provider. + \ No newline at end of file diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceCollectionContainerBuilderExtensions.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceCollectionContainerBuilderExtensions.cs index 15a647b2..f5945376 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceCollectionContainerBuilderExtensions.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceCollectionContainerBuilderExtensions.cs @@ -11,7 +11,8 @@ public static class ServiceCollectionContainerBuilderExtensions /// Creates an containing services from the provided . /// /// The containing service descriptors. - /// An instance of providing access to services. + /// The. + public static IServiceProvider BuildServiceProvider(this IServiceCollection services) { return BuildServiceProvider(services, validateScopes: false); @@ -25,7 +26,7 @@ public static IServiceProvider BuildServiceProvider(this IServiceCollection serv /// /// true to perform check verifying that scoped services never gets resolved from root provider; otherwise false. /// - /// An instance of providing access to services. + /// The. public static IServiceProvider BuildServiceProvider(this IServiceCollection services, bool validateScopes) { return new ServiceProvider(services, validateScopes); diff --git a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs index 953664a0..3f6c8b22 100644 --- a/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs +++ b/src/Microsoft.Extensions.DependencyInjection/ServiceLookup/CallSiteValidator.cs @@ -8,6 +8,7 @@ namespace Microsoft.Extensions.DependencyInjection.ServiceLookup { internal class CallSiteValidator: CallSiteVisitor { + // Keys are services being resolved via GetService, values - first scoped service in their call site tree private readonly Dictionary _scopedServices = new Dictionary(); public void ValidateCallSite(Type serviceType, IServiceCallSite callSite) @@ -25,10 +26,18 @@ public void ValidateResolution(Type serviceType, ServiceProvider serviceProvider if (ReferenceEquals(serviceProvider, serviceProvider.Root) && _scopedServices.TryGetValue(serviceType, out scopedService)) { - throw new InvalidOperationException(Resources.FormatScopedResolvedFromRootException( - serviceType, - scopedService, - nameof(ServiceLifetime.Scoped).ToLowerInvariant())); + if (serviceType == scopedService) + { + throw new InvalidOperationException( + Resources.FormatDirectScopedResolvedFromRootException(serviceType, + nameof(ServiceLifetime.Scoped).ToLowerInvariant())); + } + + throw new InvalidOperationException( + Resources.FormatScopedResolvedFromRootException( + serviceType, + scopedService, + nameof(ServiceLifetime.Scoped).ToLowerInvariant())); } } @@ -108,7 +117,6 @@ protected override Type VisitScoped(ScopedCallSite scopedCallSite, CallSiteValid internal struct CallSiteValidatorState { public SingletonCallSite Singleton { get; set; } - public List Scoped { get; set; } } } } \ No newline at end of file diff --git a/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs b/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs index a2ed452f..552235cd 100644 --- a/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs +++ b/test/Microsoft.Extensions.DependencyInjection.Tests/ServiceProviderValidationTests.cs @@ -54,6 +54,19 @@ public void GetService_Throws_WhenScopedIsInjectedIntoSingletonThroughSingleton( [Fact] public void GetService_Throws_WhenGetServiceForScopedServiceIsCalledOnRoot() + { + // Arrange + var serviceCollection = new ServiceCollection(); + serviceCollection.AddScoped(); + var serviceProvider = serviceCollection.BuildServiceProvider(validateScopes: true); + + // Act + Assert + var exception = Assert.Throws(() => serviceProvider.GetService(typeof(IBar))); + Assert.Equal($"Cannot resolve scoped service '{typeof(IBar)}' from root provider.", exception.Message); + } + + [Fact] + public void GetService_Throws_WhenGetServiceForScopedServiceIsCalledOnRootViaTransient() { // Arrange var serviceCollection = new ServiceCollection(); @@ -67,7 +80,6 @@ public void GetService_Throws_WhenGetServiceForScopedServiceIsCalledOnRoot() } [Fact] - public void GetService_DoesNotThrow_WhenScopeFactoryIsInjectedIntoSingleton() { // Arrange