From 7356d5c2e5043742f44792e0a1a86588c0b046f1 Mon Sep 17 00:00:00 2001 From: Jass Bagga Date: Fri, 2 Jun 2017 10:51:22 -0700 Subject: [PATCH 1/7] Add to Visited if in Cache --- .../Internal/ProxyTypeEmitter.cs | 18 ++++++++---- .../Properties/AssemblyInfo.cs | 7 +++++ .../Internal/ProxyTypeEmitterTest.cs | 29 ++++++++++++++++++- 3 files changed, 47 insertions(+), 7 deletions(-) create mode 100644 src/Microsoft.Extensions.DiagnosticAdapter/Properties/AssemblyInfo.cs diff --git a/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs b/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs index 0f27dda..2aa4775 100644 --- a/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs +++ b/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs @@ -80,7 +80,7 @@ public static Type GetProxyType(ProxyTypeCache cache, Type targetType, Type sour } else if (result.Type == null) { - // This is an identity convertion + // This is an identity conversion return null; } else @@ -89,7 +89,7 @@ public static Type GetProxyType(ProxyTypeCache cache, Type targetType, Type sour } } - private static bool VerifyProxySupport(ProxyBuilderContext context, Tuple key) + internal static bool VerifyProxySupport(ProxyBuilderContext context, Tuple key) { var sourceType = key.Item1; var targetType = key.Item2; @@ -101,10 +101,17 @@ private static bool VerifyProxySupport(ProxyBuilderContext context, Tuple -> IReadOnlyList and IReadOnlyList -> IReadOnlyList @@ -453,7 +459,7 @@ private static Type GetGenericImplementation(Type type, Type openGenericInterfac return null; } - private class ProxyBuilderContext + internal class ProxyBuilderContext { public ProxyBuilderContext(ProxyTypeCache cache, Type targetType, Type sourceType) { @@ -474,7 +480,7 @@ public ProxyBuilderContext(ProxyTypeCache cache, Type targetType, Type sourceTyp public Dictionary, VerificationResult> Visited { get; } } - private class VerificationResult + internal class VerificationResult { public ConstructorInfo Constructor { get; set; } diff --git a/src/Microsoft.Extensions.DiagnosticAdapter/Properties/AssemblyInfo.cs b/src/Microsoft.Extensions.DiagnosticAdapter/Properties/AssemblyInfo.cs new file mode 100644 index 0000000..ee47f85 --- /dev/null +++ b/src/Microsoft.Extensions.DiagnosticAdapter/Properties/AssemblyInfo.cs @@ -0,0 +1,7 @@ +// 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.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("Microsoft.Extensions.DiagnosticAdapter.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] +[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=0024000004800000940000000602000000240000525341310004000001000100c547cac37abd99c8db225ef2f6c8a3602f3b3606cc9891605d02baa56104f4cfc0734aa39b93bf7852f7d9266654753cc297e7d2edfe0bac1cdcf9f717241550e0a7b191195b7667bb4f64bcb8e2121380fd1d9d46ad2d92d2d15605093924cceaf74c4861eff62abf69b9291ed0a340e113be11e6a7d3113e92484cf7045cc7")] diff --git a/test/Microsoft.Extensions.DiagnosticAdapter.Test/Internal/ProxyTypeEmitterTest.cs b/test/Microsoft.Extensions.DiagnosticAdapter.Test/Internal/ProxyTypeEmitterTest.cs index 20b9990..73b1e08 100644 --- a/test/Microsoft.Extensions.DiagnosticAdapter.Test/Internal/ProxyTypeEmitterTest.cs +++ b/test/Microsoft.Extensions.DiagnosticAdapter.Test/Internal/ProxyTypeEmitterTest.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Linq; -using Microsoft.AspNetCore.Testing.xunit; using Xunit; namespace Microsoft.Extensions.DiagnosticAdapter.Internal @@ -66,6 +65,26 @@ public void GetProxyType_Assignable(Type sourceType, Type targetType) Assert.Null(result); } + [Fact] + public void GetProxyType_Assignable_InCache() + { + // Arrange + var sourceType = typeof(ProxyPerson); + var targetType = typeof(Person); + var key = new Tuple(sourceType, targetType); + var cache = new ProxyTypeCache(); + cache[key] = ProxyTypeCacheResult.FromType(key, sourceType, sourceType.GetConstructor(Array.Empty())); + var context = new ProxyTypeEmitter.ProxyBuilderContext(cache, targetType, sourceType); + + // Act + var result = ProxyTypeEmitter.VerifyProxySupport(context, key); + var result2 = ProxyTypeEmitter.VerifyProxySupport(context, key); + + // Assert + Assert.Single(context.Visited); + Assert.Equal(key, context.Visited.SingleOrDefault().Key); + } + [Fact] public void GetProxyType_Fails_DestinationIsNotInterface() { @@ -728,6 +747,14 @@ public class DerivedPerson : Person public double CoolnessFactor { get; set; } } + public class ProxyPerson + { + public double CoolnessFactor { get; set; } + public string FirstName { get; set; } + public string LastName { get; set; } + public Address Address { get; set; } + } + public class Address { public string City { get; set; } From c18e7aea7831d607b939331de550d4947e38f102 Mon Sep 17 00:00:00 2001 From: Jass Bagga Date: Fri, 2 Jun 2017 11:09:29 -0700 Subject: [PATCH 2/7] Change test name --- .../Internal/ProxyTypeEmitterTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Microsoft.Extensions.DiagnosticAdapter.Test/Internal/ProxyTypeEmitterTest.cs b/test/Microsoft.Extensions.DiagnosticAdapter.Test/Internal/ProxyTypeEmitterTest.cs index 73b1e08..3a0e13d 100644 --- a/test/Microsoft.Extensions.DiagnosticAdapter.Test/Internal/ProxyTypeEmitterTest.cs +++ b/test/Microsoft.Extensions.DiagnosticAdapter.Test/Internal/ProxyTypeEmitterTest.cs @@ -66,7 +66,7 @@ public void GetProxyType_Assignable(Type sourceType, Type targetType) } [Fact] - public void GetProxyType_Assignable_InCache() + public void GetProxyType_IfAlreadyInCache_AlsoAddedToVisited() { // Arrange var sourceType = typeof(ProxyPerson); @@ -82,7 +82,7 @@ public void GetProxyType_Assignable_InCache() // Assert Assert.Single(context.Visited); - Assert.Equal(key, context.Visited.SingleOrDefault().Key); + Assert.Equal(key, context.Visited.Single().Key); } [Fact] From ba54a9c89c1d59427d439ece6cd6c4bdf0217935 Mon Sep 17 00:00:00 2001 From: Jass Bagga Date: Fri, 2 Jun 2017 12:02:58 -0700 Subject: [PATCH 3/7] Remove conditions --- .../Internal/ProxyTypeEmitter.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs b/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs index 2aa4775..a453f14 100644 --- a/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs +++ b/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs @@ -104,14 +104,10 @@ internal static bool VerifyProxySupport(ProxyBuilderContext context, Tuple Date: Fri, 2 Jun 2017 12:35:46 -0700 Subject: [PATCH 4/7] Add FromError test --- .../Internal/ProxyTypeEmitterTest.cs | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/test/Microsoft.Extensions.DiagnosticAdapter.Test/Internal/ProxyTypeEmitterTest.cs b/test/Microsoft.Extensions.DiagnosticAdapter.Test/Internal/ProxyTypeEmitterTest.cs index 3a0e13d..823c950 100644 --- a/test/Microsoft.Extensions.DiagnosticAdapter.Test/Internal/ProxyTypeEmitterTest.cs +++ b/test/Microsoft.Extensions.DiagnosticAdapter.Test/Internal/ProxyTypeEmitterTest.cs @@ -66,7 +66,7 @@ public void GetProxyType_Assignable(Type sourceType, Type targetType) } [Fact] - public void GetProxyType_IfAlreadyInCache_AlsoAddedToVisited() + public void GetProxyType_IfAlreadyInCache_AlsoAddedToVisited_FromType() { // Arrange var sourceType = typeof(ProxyPerson); @@ -81,6 +81,28 @@ public void GetProxyType_IfAlreadyInCache_AlsoAddedToVisited() var result2 = ProxyTypeEmitter.VerifyProxySupport(context, key); // Assert + Assert.True(result); + Assert.True(result2); + Assert.Single(context.Visited); + Assert.Equal(key, context.Visited.Single().Key); + } + + [Fact] + public void GetProxyType_IfAlreadyInCache_AlsoAddedToVisited_FromError() + { + // Arrange + var sourceType = typeof(ProxyPerson); + var targetType = typeof(Person); + var key = new Tuple(sourceType, targetType); + var cache = new ProxyTypeCache(); + cache[key] = ProxyTypeCacheResult.FromError(key, "Test Error"); + var context = new ProxyTypeEmitter.ProxyBuilderContext(cache, targetType, sourceType); + + // Act + var result = ProxyTypeEmitter.VerifyProxySupport(context, key); + + // Assert + Assert.False(result); Assert.Single(context.Visited); Assert.Equal(key, context.Visited.Single().Key); } From 65af0d1a4e025979dd3ac515c6cefacdf4e740ea Mon Sep 17 00:00:00 2001 From: Jass Bagga Date: Mon, 5 Jun 2017 12:19:19 -0700 Subject: [PATCH 5/7] Feedback addressed --- .../Internal/ProxyTypeEmitter.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs b/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs index a453f14..fa8a9eb 100644 --- a/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs +++ b/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs @@ -66,10 +66,10 @@ public static Type GetProxyType(ProxyTypeCache cache, Type targetType, Type sour // We only want to publish the results after all of the proxies are totally generated. foreach (var verificationResult in context.Visited) { - cache[verificationResult.Key] = ProxyTypeCacheResult.FromType( + cache.TryAdd(verificationResult.Key, ProxyTypeCacheResult.FromType( verificationResult.Key, verificationResult.Value.Type, - verificationResult.Value.Constructor); + verificationResult.Value.Constructor)); } return context.Visited[context.Key].Type; @@ -104,8 +104,9 @@ internal static bool VerifyProxySupport(ProxyBuilderContext context, Tuple Date: Tue, 6 Jun 2017 12:25:42 -0700 Subject: [PATCH 6/7] Let stuff breathe --- .../Internal/ProxyTypeEmitter.cs | 4 ++++ .../Internal/ProxyTypeEmitterTest.cs | 21 +++++++------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs b/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs index fa8a9eb..6e43391 100644 --- a/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs +++ b/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs @@ -104,9 +104,13 @@ internal static bool VerifyProxySupport(ProxyBuilderContext context, Tuple(sourceType, targetType); var cache = new ProxyTypeCache(); cache[key] = ProxyTypeCacheResult.FromType(key, sourceType, sourceType.GetConstructor(Array.Empty())); + var context = new ProxyTypeEmitter.ProxyBuilderContext(cache, targetType, sourceType); // Act @@ -91,11 +93,13 @@ public void GetProxyType_IfAlreadyInCache_AlsoAddedToVisited_FromType() public void GetProxyType_IfAlreadyInCache_AlsoAddedToVisited_FromError() { // Arrange - var sourceType = typeof(ProxyPerson); - var targetType = typeof(Person); + var targetType = typeof(IPerson); + var sourceType = typeof(Person); + var key = new Tuple(sourceType, targetType); var cache = new ProxyTypeCache(); cache[key] = ProxyTypeCacheResult.FromError(key, "Test Error"); + var context = new ProxyTypeEmitter.ProxyBuilderContext(cache, targetType, sourceType); // Act @@ -103,7 +107,6 @@ public void GetProxyType_IfAlreadyInCache_AlsoAddedToVisited_FromError() // Assert Assert.False(result); - Assert.Single(context.Visited); Assert.Equal(key, context.Visited.Single().Key); } @@ -769,14 +772,6 @@ public class DerivedPerson : Person public double CoolnessFactor { get; set; } } - public class ProxyPerson - { - public double CoolnessFactor { get; set; } - public string FirstName { get; set; } - public string LastName { get; set; } - public Address Address { get; set; } - } - public class Address { public string City { get; set; } From 55345cd7f1b44d21194815224276314a03ecbffc Mon Sep 17 00:00:00 2001 From: Jass Bagga Date: Tue, 6 Jun 2017 14:37:26 -0700 Subject: [PATCH 7/7] Reword comment --- .../Internal/ProxyTypeEmitter.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs b/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs index 6e43391..148911e 100644 --- a/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs +++ b/src/Microsoft.Extensions.DiagnosticAdapter/Internal/ProxyTypeEmitter.cs @@ -104,9 +104,8 @@ internal static bool VerifyProxySupport(ProxyBuilderContext context, Tuple