[release/9.0-preview2] #98551 - Regression in DI scope validation (#9…

…9010)

* Add additional test cases for regression

* Make sure to always check for scoped-in-singleton

* Remove duplicate line

---------

Co-authored-by: Christiaan de Ridder <christiaan@channelengine.com>

src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteValidator.cs

@@ -41,18 +41,27 @@ public void ValidateResolution(ServiceCallSite callSite, IServiceScope scope, IS
             // First, check if we have encountered this call site before to prevent visiting call site trees that have already been visited
             // If firstScopedServiceInCallSiteTree is null there are no scoped dependencies in this service's call site tree
             // If firstScopedServiceInCallSiteTree has a value, it contains the first scoped service in this service's call site tree
-            if (_scopedServices.TryGetValue(callSite.Cache.Key, out Type? firstScopedServiceInCallSiteTree))
+            if (!_scopedServices.TryGetValue(callSite.Cache.Key, out Type? firstScopedServiceInCallSiteTree))
             {
-                return firstScopedServiceInCallSiteTree;
-            }
+                // This call site wasn't cached yet, walk the tree
+                firstScopedServiceInCallSiteTree = base.VisitCallSite(callSite, argument);
 
-            // Walk the tree
-            Type? scoped = base.VisitCallSite(callSite, argument);
+                // Cache the result
+                _scopedServices[callSite.Cache.Key] = firstScopedServiceInCallSiteTree;
+            }
 
-            // Store the result for each visited service
-            _scopedServices[callSite.Cache.Key] = scoped;
+            // If there is a scoped service in the call site tree, make sure we are not resolving it from a singleton
+            if (firstScopedServiceInCallSiteTree != null && argument.Singleton != null)
+            {
+                throw new InvalidOperationException(SR.Format(SR.ScopedInSingletonException,
+                    callSite.ServiceType,
+                    argument.Singleton.ServiceType,
+                    nameof(ServiceLifetime.Scoped).ToLowerInvariant(),
+                    nameof(ServiceLifetime.Singleton).ToLowerInvariant()
+                    ));
+            }
 
-            return scoped;
+            return firstScopedServiceInCallSiteTree;
         }
 
         protected override Type? VisitConstructor(ConstructorCallSite constructorCallSite, CallSiteValidatorState state)
@@ -91,15 +100,6 @@ public void ValidateResolution(ServiceCallSite callSite, IServiceScope scope, IS
             {
                 return null;
             }
-            if (state.Singleton != null)
-            {
-                throw new InvalidOperationException(SR.Format(SR.ScopedInSingletonException,
-                    scopedCallSite.ServiceType,
-                    state.Singleton.ServiceType,
-                    nameof(ServiceLifetime.Scoped).ToLowerInvariant(),
-                    nameof(ServiceLifetime.Singleton).ToLowerInvariant()
-                    ));
-            }
 
             VisitCallSiteMain(scopedCallSite, state);
             return scopedCallSite.ServiceType;

src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderValidationTests.cs

@@ -239,6 +239,84 @@ public void GetService_DoesNotThrow_WhenGetServiceForNonScopedImplementationWith
             Assert.NotNull(result);
         }
 
+        [Fact]
+        public void BuildServiceProvider_ValidateOnBuild_Throws_WhenScopedIsInjectedIntoSingleton()
+        {
+            // Arrange
+            var serviceCollection = new ServiceCollection();
+            serviceCollection.AddScoped<IBar, Bar>();
+            serviceCollection.AddSingleton<IFoo, Foo>();
+
+            // Act + Assert
+            var aggregateException = Assert.Throws<AggregateException>(() => serviceCollection.BuildServiceProvider(new ServiceProviderOptions() { ValidateOnBuild = true, ValidateScopes = true }));
+            Assert.StartsWith("Some services are not able to be constructed", aggregateException.Message);
+            Assert.Equal(1, aggregateException.InnerExceptions.Count);
+            Assert.Equal("Error while validating the service descriptor 'ServiceType: Microsoft.Extensions.DependencyInjection.Tests.ServiceProviderValidationTests+IFoo Lifetime: Singleton ImplementationType: Microsoft.Extensions.DependencyInjection.Tests.ServiceProviderValidationTests+Foo': " +
+                         "Cannot consume scoped service 'Microsoft.Extensions.DependencyInjection.Tests.ServiceProviderValidationTests+IBar' from singleton 'Microsoft.Extensions.DependencyInjection.Tests.ServiceProviderValidationTests+IFoo'."
+                , aggregateException.InnerExceptions[0].Message);
+        }
+
+        [Fact]
+        public void BuildServiceProvider_ValidateOnBuild_Throws_WhenScopedIsInjectedIntoSingleton_ReverseRegistrationOrder()
+        {
+            // Arrange
+            var serviceCollection = new ServiceCollection();
+            serviceCollection.AddSingleton<IFoo, Foo>();
+            serviceCollection.AddScoped<IBar, Bar>();
+
+            // Act + Assert
+            var aggregateException = Assert.Throws<AggregateException>(() => serviceCollection.BuildServiceProvider(new ServiceProviderOptions() { ValidateOnBuild = true, ValidateScopes = true }));
+            Assert.StartsWith("Some services are not able to be constructed", aggregateException.Message);
+            Assert.Equal(1, aggregateException.InnerExceptions.Count);
+            Assert.Equal("Error while validating the service descriptor 'ServiceType: Microsoft.Extensions.DependencyInjection.Tests.ServiceProviderValidationTests+IFoo Lifetime: Singleton ImplementationType: Microsoft.Extensions.DependencyInjection.Tests.ServiceProviderValidationTests+Foo': " +
+                         "Cannot consume scoped service 'Microsoft.Extensions.DependencyInjection.Tests.ServiceProviderValidationTests+IBar' from singleton 'Microsoft.Extensions.DependencyInjection.Tests.ServiceProviderValidationTests+IFoo'."
+                , aggregateException.InnerExceptions[0].Message);
+        }
+
+        [Fact]
+        public void BuildServiceProvider_ValidateOnBuild_DoesNotThrow_WhenScopeFactoryIsInjectedIntoSingleton()
+        {
+            // Arrange
+            var serviceCollection = new ServiceCollection();
+            serviceCollection.AddSingleton<IBoo, Boo>();
+
+            // Act + Assert
+            serviceCollection.BuildServiceProvider(new ServiceProviderOptions() { ValidateOnBuild = true, ValidateScopes = true });
+        }
+
+        [Fact]
+        public void BuildServiceProvider_ValidateOnBuild_Throws_WhenScopedIsInjectedIntoSingleton_CachedCallSites()
+        {
+            // Arrange
+            var serviceCollection = new ServiceCollection();
+            serviceCollection.AddScoped<Foo>();
+            serviceCollection.AddSingleton<Foo2>();
+            serviceCollection.AddScoped<IBar, Bar2>();
+            serviceCollection.AddScoped<IBaz, Baz>();
+
+            // Act + Assert
+            var aggregateException = Assert.Throws<AggregateException>(() => serviceCollection.BuildServiceProvider(new ServiceProviderOptions() { ValidateOnBuild = true, ValidateScopes = true }));
+            Assert.StartsWith("Some services are not able to be constructed", aggregateException.Message);
+            Assert.Equal(1, aggregateException.InnerExceptions.Count);
+            Assert.Equal("Error while validating the service descriptor 'ServiceType: Microsoft.Extensions.DependencyInjection.Tests.ServiceProviderValidationTests+Foo2 Lifetime: Singleton ImplementationType: Microsoft.Extensions.DependencyInjection.Tests.ServiceProviderValidationTests+Foo2': " +
+                         "Cannot consume scoped service 'Microsoft.Extensions.DependencyInjection.Tests.ServiceProviderValidationTests+IBar' from singleton 'Microsoft.Extensions.DependencyInjection.Tests.ServiceProviderValidationTests+Foo2'."
+                , aggregateException.InnerExceptions[0].Message);
+        }
+
+        [Fact]
+        public void BuildServiceProvider_ValidateOnBuild_DoesNotThrow_CachedCallSites()
+        {
+            // Arrange
+            var serviceCollection = new ServiceCollection();
+            serviceCollection.AddScoped<Foo>();
+            serviceCollection.AddScoped<Foo2>();
+            serviceCollection.AddScoped<IBar, Bar2>();
+            serviceCollection.AddScoped<IBaz, Baz>();
+
+            // Act + Assert
+            serviceCollection.BuildServiceProvider(new ServiceProviderOptions() { ValidateOnBuild = true, ValidateScopes = true });
+        }
+
         [Fact]
         public void BuildServiceProvider_ValidateOnBuild_ThrowsForUnresolvableServices()
         {
@@ -327,6 +405,13 @@ public Foo(IBar bar)
             }
         }
 
+        private class Foo2 : IFoo
+        {
+            public Foo2(IBar bar)
+            {
+            }
+        }
+
         private interface IBar
         {
         }