From 25fb2413d7dd931983d1c016df0d3895bf9434ce Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 10 Aug 2021 10:52:07 -0700 Subject: [PATCH 1/5] Fix DI regression with FactoryCallSite --- .../src/ServiceProvider.cs | 2 +- .../DependencyInjectionEventSourceTests.cs | 44 +++++++++---------- ...viceCollectionDescriptorExtensionsTests.cs | 41 ++++++++++++++++- 3 files changed, 63 insertions(+), 24 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceProvider.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceProvider.cs index cff40d1fbb1ec..83b3e4f70122d 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceProvider.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceProvider.cs @@ -167,7 +167,7 @@ private Func CreateServiceAccessor(Type serv internal void ReplaceServiceAccessor(ServiceCallSite callSite, Func accessor) { - _realizedServices[callSite.ImplementationType] = accessor; + _realizedServices[callSite.ServiceType] = accessor; } internal IServiceScope CreateScope() diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/DependencyInjectionEventSourceTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/DependencyInjectionEventSourceTests.cs index 54a282d66dd9b..b1cf5ace7d42c 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/DependencyInjectionEventSourceTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/DependencyInjectionEventSourceTests.cs @@ -17,6 +17,28 @@ public class EventSourceTests : ICollectionFixture { } + internal class TestEventListener : EventListener + { + private volatile bool _disposed; + private ConcurrentQueue _events = new ConcurrentQueue(); + + public IEnumerable EventData => _events; + + protected override void OnEventWritten(EventWrittenEventArgs eventData) + { + if (!_disposed) + { + _events.Enqueue(eventData); + } + } + + public override void Dispose() + { + _disposed = true; + base.Dispose(); + } + } + [Collection(nameof(EventSourceTests))] public class DependencyInjectionEventSourceTests: IDisposable { @@ -226,28 +248,6 @@ public void EmitsServiceRealizationFailedEvent() private T GetProperty(EventWrittenEventArgs data, string propName) => (T)data.Payload[data.PayloadNames.IndexOf(propName)]; - private class TestEventListener : EventListener - { - private volatile bool _disposed; - private ConcurrentQueue _events = new ConcurrentQueue(); - - public IEnumerable EventData => _events; - - protected override void OnEventWritten(EventWrittenEventArgs eventData) - { - if (!_disposed) - { - _events.Enqueue(eventData); - } - } - - public override void Dispose() - { - _disposed = true; - base.Dispose(); - } - } - public void Dispose() { _listener.Dispose(); diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionDescriptorExtensionsTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionDescriptorExtensionsTests.cs index 9a3588bf6a197..657c9d5a68181 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionDescriptorExtensionsTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionDescriptorExtensionsTests.cs @@ -2,15 +2,54 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Diagnostics.Tracing; +using System.Linq; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.DependencyInjection.Specification.Fakes; +using Microsoft.Extensions.DependencyInjection.Tests; using Xunit; -using Xunit.Abstractions; namespace Microsoft.Extensions.DependencyInjection { + public class ABC { } + public class ServiceCollectionDescriptorExtensionsTest { + [Fact] + internal void GetService_FactoryCallSite_Transient_DoesNotFail() + { + TestEventListener listener = new TestEventListener(); + listener.EnableEvents(DependencyInjectionEventSource.Log, EventLevel.Verbose); + IServiceProvider serviceProvider = null; + + try + { + IServiceCollection services = new ServiceCollection(); + services.Add(ServiceDescriptor.Describe(typeof(ABC), (sp) => new ABC(), ServiceLifetime.Transient)); + + var sp = services.BuildServiceProvider(ServiceProviderMode.Dynamic); + serviceProvider = sp + .CreateScope() + .ServiceProvider; + + serviceProvider.GetService(); + + for (int i = 0; i < 50; i++) + { + serviceProvider.GetService(); + System.Threading.Thread.Sleep(10); // Give the background thread time to compile + } + + Assert.Null(listener.EventData.FirstOrDefault(e => e.EventName == nameof(DependencyInjectionEventSource.Log.ServiceRealizationFailed))); + } + finally + { + ((IDisposable)serviceProvider).Dispose(); + } + } + [Fact] public void Add_AddsDescriptorToServiceDescriptors() { From d2cf1586b7fecd96e3c7e2a21b835d35ee77fb8c Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 11 Aug 2021 13:58:26 -0700 Subject: [PATCH 2/5] - Remove event listener. rely on Debug.Fail - Use existing mock types --- .../DependencyInjectionEventSourceTests.cs | 44 +++++++++---------- ...viceCollectionDescriptorExtensionsTests.cs | 40 ----------------- .../ServiceLookup/CallSiteFactoryTest.cs | 35 +++++++++++++++ 3 files changed, 57 insertions(+), 62 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/DependencyInjectionEventSourceTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/DependencyInjectionEventSourceTests.cs index b1cf5ace7d42c..54a282d66dd9b 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/DependencyInjectionEventSourceTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/DependencyInjectionEventSourceTests.cs @@ -17,28 +17,6 @@ public class EventSourceTests : ICollectionFixture { } - internal class TestEventListener : EventListener - { - private volatile bool _disposed; - private ConcurrentQueue _events = new ConcurrentQueue(); - - public IEnumerable EventData => _events; - - protected override void OnEventWritten(EventWrittenEventArgs eventData) - { - if (!_disposed) - { - _events.Enqueue(eventData); - } - } - - public override void Dispose() - { - _disposed = true; - base.Dispose(); - } - } - [Collection(nameof(EventSourceTests))] public class DependencyInjectionEventSourceTests: IDisposable { @@ -248,6 +226,28 @@ public void EmitsServiceRealizationFailedEvent() private T GetProperty(EventWrittenEventArgs data, string propName) => (T)data.Payload[data.PayloadNames.IndexOf(propName)]; + private class TestEventListener : EventListener + { + private volatile bool _disposed; + private ConcurrentQueue _events = new ConcurrentQueue(); + + public IEnumerable EventData => _events; + + protected override void OnEventWritten(EventWrittenEventArgs eventData) + { + if (!_disposed) + { + _events.Enqueue(eventData); + } + } + + public override void Dispose() + { + _disposed = true; + base.Dispose(); + } + } + public void Dispose() { _listener.Dispose(); diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionDescriptorExtensionsTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionDescriptorExtensionsTests.cs index 657c9d5a68181..6d06cbf5a2d56 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionDescriptorExtensionsTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionDescriptorExtensionsTests.cs @@ -2,54 +2,14 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Concurrent; -using System.Collections.Generic; -using System.Diagnostics.Tracing; -using System.Linq; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.DependencyInjection.Specification.Fakes; -using Microsoft.Extensions.DependencyInjection.Tests; using Xunit; namespace Microsoft.Extensions.DependencyInjection { - public class ABC { } - public class ServiceCollectionDescriptorExtensionsTest { - [Fact] - internal void GetService_FactoryCallSite_Transient_DoesNotFail() - { - TestEventListener listener = new TestEventListener(); - listener.EnableEvents(DependencyInjectionEventSource.Log, EventLevel.Verbose); - IServiceProvider serviceProvider = null; - - try - { - IServiceCollection services = new ServiceCollection(); - services.Add(ServiceDescriptor.Describe(typeof(ABC), (sp) => new ABC(), ServiceLifetime.Transient)); - - var sp = services.BuildServiceProvider(ServiceProviderMode.Dynamic); - serviceProvider = sp - .CreateScope() - .ServiceProvider; - - serviceProvider.GetService(); - - for (int i = 0; i < 50; i++) - { - serviceProvider.GetService(); - System.Threading.Thread.Sleep(10); // Give the background thread time to compile - } - - Assert.Null(listener.EventData.FirstOrDefault(e => e.EventName == nameof(DependencyInjectionEventSource.Log.ServiceRealizationFailed))); - } - finally - { - ((IDisposable)serviceProvider).Dispose(); - } - } - [Fact] public void Add_AddsDescriptorToServiceDescriptors() { diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs index da24dca4e92cf..90d6f94c7022f 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs @@ -6,16 +6,51 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection; +using System.Threading; using System.Threading.Tasks; using Microsoft.DotNet.RemoteExecutor; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.DependencyInjection.Specification.Fakes; +using Microsoft.Extensions.DependencyInjection.Tests; using Xunit; namespace Microsoft.Extensions.DependencyInjection.ServiceLookup { public class CallSiteFactoryTest { + [Fact] + public void GetService_FactoryCallSite_Transient_DoesNotFail() + { + IServiceProvider serviceProvider = null; + try + { + var collection = new ServiceCollection(); + collection.Add(ServiceDescriptor.Describe(typeof(FakeService), (sp) => new FakeService(), ServiceLifetime.Transient)); + collection.Add(ServiceDescriptor.Describe(typeof(IFakeService), (sp) => new FakeService(), ServiceLifetime.Transient)); + + serviceProvider = collection + .BuildServiceProvider(ServiceProviderMode.Dynamic) + .CreateScope() + .ServiceProvider; + + Type expectedType = typeof(FakeService); + + Assert.Equal(expectedType, serviceProvider.GetService(typeof(IFakeService)).GetType()); + Assert.Equal(expectedType, serviceProvider.GetService(typeof(FakeService)).GetType()); + + for (int i = 0; i < 50; i++) + { + serviceProvider.GetService(typeof(IFakeService)); + serviceProvider.GetService(typeof(FakeService)); + Thread.Sleep(10); // Give the background thread time to compile + } + } + finally + { + ((IDisposable)serviceProvider).Dispose(); + } + } + [Fact] public void CreateCallSite_Throws_IfTypeHasNoPublicConstructors() { From cd1801177bbcedac1908c174680f12f23008286c Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 11 Aug 2021 16:14:47 -0700 Subject: [PATCH 3/5] Apply PR feedback --- .../ServiceLookup/CallSiteFactoryTest.cs | 34 +++++++------------ 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs index 90d6f94c7022f..fe4840216f700 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs @@ -21,33 +21,23 @@ public class CallSiteFactoryTest [Fact] public void GetService_FactoryCallSite_Transient_DoesNotFail() { - IServiceProvider serviceProvider = null; - try - { - var collection = new ServiceCollection(); - collection.Add(ServiceDescriptor.Describe(typeof(FakeService), (sp) => new FakeService(), ServiceLifetime.Transient)); - collection.Add(ServiceDescriptor.Describe(typeof(IFakeService), (sp) => new FakeService(), ServiceLifetime.Transient)); + var collection = new ServiceCollection(); + collection.Add(ServiceDescriptor.Describe(typeof(FakeService), (sp) => new FakeService(), ServiceLifetime.Transient)); + collection.Add(ServiceDescriptor.Describe(typeof(IFakeService), (sp) => new FakeService(), ServiceLifetime.Transient)); - serviceProvider = collection - .BuildServiceProvider(ServiceProviderMode.Dynamic) - .CreateScope() - .ServiceProvider; + using ServiceProvider serviceProvider = collection.BuildServiceProvider(ServiceProviderMode.Dynamic); - Type expectedType = typeof(FakeService); - Assert.Equal(expectedType, serviceProvider.GetService(typeof(IFakeService)).GetType()); - Assert.Equal(expectedType, serviceProvider.GetService(typeof(FakeService)).GetType()); + Type expectedType = typeof(FakeService); - for (int i = 0; i < 50; i++) - { - serviceProvider.GetService(typeof(IFakeService)); - serviceProvider.GetService(typeof(FakeService)); - Thread.Sleep(10); // Give the background thread time to compile - } - } - finally + Assert.Equal(expectedType, serviceProvider.GetService(typeof(IFakeService)).GetType()); + Assert.Equal(expectedType, serviceProvider.GetService(typeof(FakeService)).GetType()); + + for (int i = 0; i < 50; i++) { - ((IDisposable)serviceProvider).Dispose(); + Assert.Equal(expectedType, serviceProvider.GetService(typeof(IFakeService)).GetType()); + Assert.Equal(expectedType, serviceProvider.GetService(typeof(FakeService)).GetType()); + Thread.Sleep(10); // Give the background thread time to compile } } From 2e447280c09b5cf729b65058d10753c9b3eba551 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 11 Aug 2021 16:16:54 -0700 Subject: [PATCH 4/5] Undo remove unused using --- .../tests/DI.Tests/ServiceCollectionDescriptorExtensionsTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionDescriptorExtensionsTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionDescriptorExtensionsTests.cs index 6d06cbf5a2d56..9a3588bf6a197 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionDescriptorExtensionsTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionDescriptorExtensionsTests.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.DependencyInjection.Specification.Fakes; using Xunit; +using Xunit.Abstractions; namespace Microsoft.Extensions.DependencyInjection { From bd2c21e1fad17b363d1f0d0b469e19de5f114dd9 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 11 Aug 2021 16:19:45 -0700 Subject: [PATCH 5/5] remove extra blank linke --- .../tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs index fe4840216f700..56f0982c0d1c5 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs @@ -26,8 +26,6 @@ public void GetService_FactoryCallSite_Transient_DoesNotFail() collection.Add(ServiceDescriptor.Describe(typeof(IFakeService), (sp) => new FakeService(), ServiceLifetime.Transient)); using ServiceProvider serviceProvider = collection.BuildServiceProvider(ServiceProviderMode.Dynamic); - - Type expectedType = typeof(FakeService); Assert.Equal(expectedType, serviceProvider.GetService(typeof(IFakeService)).GetType());