From cd38d9e918c406e2b703838d9855fb6715a97059 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Sun, 2 May 2021 01:32:56 -0700 Subject: [PATCH 1/2] Fixed issues with caching in the wrong slot - When using ValidateOnBuild we build the callsite for each ServiceDescriptor but using the wrong slot. This didn't matter before but it breaks service overrides. - Run the spec tests with ValidateOnBuild set to true when it doesn't throw. --- .../src/ServiceLookup/CallSiteFactory.cs | 4 +-- ...ollectionContainerBuilderTestExtensions.cs | 8 +++-- ...roviderDefaultContainerTestsWithOptions.cs | 31 +++++++++++++++++++ 3 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderDefaultContainerTestsWithOptions.cs diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteFactory.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteFactory.cs index 82ee191aade689..15a9b46843922b 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteFactory.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteFactory.cs @@ -481,7 +481,7 @@ public int GetSlot(ServiceDescriptor descriptor) { if (descriptor == _item) { - return 0; + return Count - 1; } if (_items != null) @@ -489,7 +489,7 @@ public int GetSlot(ServiceDescriptor descriptor) int index = _items.IndexOf(descriptor); if (index != -1) { - return index + 1; + return Count - index + 1; } } diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionContainerBuilderTestExtensions.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionContainerBuilderTestExtensions.cs index ec90be1378e56f..3a8c24e51336bd 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionContainerBuilderTestExtensions.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionContainerBuilderTestExtensions.cs @@ -8,11 +8,13 @@ namespace Microsoft.Extensions.DependencyInjection.Tests { internal static class ServiceCollectionContainerBuilderTestExtensions { - public static ServiceProvider BuildServiceProvider(this IServiceCollection services, ServiceProviderMode mode) + public static ServiceProvider BuildServiceProvider(this IServiceCollection services, ServiceProviderMode mode, ServiceProviderOptions options = null) { + options ??= ServiceProviderOptions.Default; + if (mode == ServiceProviderMode.Default) { - return services.BuildServiceProvider(); + return services.BuildServiceProvider(options); } IServiceProviderEngine engine = mode switch @@ -24,7 +26,7 @@ public static ServiceProvider BuildServiceProvider(this IServiceCollection servi _ => throw new NotSupportedException() }; - return new ServiceProvider(services, engine, ServiceProviderOptions.Default); + return new ServiceProvider(services, engine, options); } } } diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderDefaultContainerTestsWithOptions.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderDefaultContainerTestsWithOptions.cs new file mode 100644 index 00000000000000..57d0099c6abe22 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderDefaultContainerTestsWithOptions.cs @@ -0,0 +1,31 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using Microsoft.Extensions.DependencyInjection.Specification; + +namespace Microsoft.Extensions.DependencyInjection.Tests +{ + public class ServiceProviderDefaultContainerTestsWithOptions : DependencyInjectionSpecificationTests + { + protected override IServiceProvider CreateServiceProvider(IServiceCollection collection) + { + try + { + return collection.BuildServiceProvider(ServiceProviderMode.Default, new ServiceProviderOptions + { + ValidateOnBuild = true, + // Too many tests fail because they try to resolve scoped services from the root + // provider + // ValidateScopes = true + }); + } + catch + { + // This is how we "skip" tests that fail on BuildServiceProvider (broken object graphs). + // We care mainly about exercising the non-throwing code path so we fallback to the default BuildServiceProvider + return collection.BuildServiceProvider(); + } + } + } +} From dcbd4f4915e69b842d47a47ee33cd9f9a293b02d Mon Sep 17 00:00:00 2001 From: David Fowler Date: Sun, 2 May 2021 08:15:18 -0700 Subject: [PATCH 2/2] Only catch AggregateExceptions - Those are the ones that ValidateOnBuild throws --- .../DI.Tests/ServiceProviderDefaultContainerTestsWithOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderDefaultContainerTestsWithOptions.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderDefaultContainerTestsWithOptions.cs index 57d0099c6abe22..10831d1b7ff773 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderDefaultContainerTestsWithOptions.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderDefaultContainerTestsWithOptions.cs @@ -20,7 +20,7 @@ protected override IServiceProvider CreateServiceProvider(IServiceCollection col // ValidateScopes = true }); } - catch + catch (AggregateException) { // This is how we "skip" tests that fail on BuildServiceProvider (broken object graphs). // We care mainly about exercising the non-throwing code path so we fallback to the default BuildServiceProvider