From dd250a65cb04748b53b128899de6a684957f4c2e Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Thu, 31 Aug 2023 23:43:14 +0200 Subject: [PATCH 01/17] `DefaultInterfaceMembersTestCase`: use strings ... instead of `MethodBase`s to identify which methods get invoked. This should make tests easier to read & understand. --- .../DefaultInterfaceMembersTestCase.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs index 8df2d01834..7aff2db241 100644 --- a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs @@ -33,15 +33,16 @@ public void Can_proxy_interface_with_sealed_method() public void Can_invoke_sealed_method_in_proxied_interface() { var proxy = generator.CreateInterfaceProxyWithoutTarget(); - var invokedMethod = proxy.SealedMethod(); - Assert.AreEqual(typeof(IHaveSealedMethod).GetMethod(nameof(IHaveSealedMethod.SealedMethod)), invokedMethod); + var expected = "default implementation"; + var actual = proxy.SealedMethod(); + Assert.AreEqual(expected, actual); } public interface IHaveSealedMethod { - sealed MethodBase SealedMethod() + sealed string SealedMethod() { - return MethodBase.GetCurrentMethod(); + return "default implementation"; } } } From 5e7cfc47c14f7a5bd6b6801953ec1f0d44795922 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Fri, 1 Sep 2023 00:03:14 +0200 Subject: [PATCH 02/17] Add more tests for default interface method impls Two of these tests are failing: * Class proxies cannot intercept interface methods with a default implementation if the proxied class type does not override it. * Interface proxies cannot proceed to default implementations. --- .../DefaultInterfaceMembersTestCase.cs | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs index 7aff2db241..0be67095e9 100644 --- a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs @@ -16,6 +16,8 @@ using System.Reflection; +using Castle.DynamicProxy.Tests.Interceptors; + using NUnit.Framework; namespace Castle.DynamicProxy.Tests @@ -23,6 +25,78 @@ namespace Castle.DynamicProxy.Tests [TestFixture] public class DefaultInterfaceMembersTestCase : BasePEVerifyTestCase { + [Test] + public void Can_proxy_class_that_inherits_method_with_default_implementation_from_interface() + { + _ = generator.CreateClassProxy(); + } + + [Test] + public void Can_proxy_interface_with_method_with_default_implementation() + { + _ = generator.CreateInterfaceProxyWithoutTarget(); + } + + [Test] + public void Default_implementation_gets_called_when_method_not_intercepted_in_proxied_class() + { + var options = new ProxyGenerationOptions(new ProxyNothingHook()); + var proxy = generator.CreateClassProxy(options); + var expected = "default implementation"; + var actual = ((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Default_implementation_gets_called_when_method_not_intercepted_in_proxied_interface() + { + var options = new ProxyGenerationOptions(new ProxyNothingHook()); + var proxy = generator.CreateInterfaceProxyWithoutTarget(options); + var expected = "default implementation"; + var actual = proxy.MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_intercept_method_with_default_implementation_in_proxied_class() + { + var expected = "intercepted"; + var interceptor = new WithCallbackInterceptor(invocation => invocation.ReturnValue = expected); + var proxy = generator.CreateClassProxy(interceptor); + var actual = ((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_intercept_method_with_default_implementation_in_proxied_interface() + { + var expected = "intercepted"; + var interceptor = new WithCallbackInterceptor(invocation => invocation.ReturnValue = expected); + var proxy = generator.CreateInterfaceProxyWithoutTarget(interceptor); + var actual = proxy.MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_proceed_to_default_implementation_in_proxied_class() + { + var interceptor = new WithCallbackInterceptor(invocation => invocation.Proceed()); + var proxy = generator.CreateClassProxy(interceptor); + var expected = "default implementation"; + var actual = ((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_proceed_to_default_implementation_in_proxied_interface() + { + var interceptor = new WithCallbackInterceptor(invocation => invocation.Proceed()); + var proxy = generator.CreateInterfaceProxyWithoutTarget(interceptor); + var expected = "default implementation"; + var actual = proxy.MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + [Test] public void Can_proxy_interface_with_sealed_method() { @@ -38,6 +112,14 @@ public void Can_invoke_sealed_method_in_proxied_interface() Assert.AreEqual(expected, actual); } + public interface IHaveMethodWithDefaultImplementation + { + string MethodWithDefaultImplementation() + { + return "default implementation"; + } + } + public interface IHaveSealedMethod { sealed string SealedMethod() @@ -45,6 +127,8 @@ sealed string SealedMethod() return "default implementation"; } } + + public class InheritsMethodWithDefaultImplementation : IHaveMethodWithDefaultImplementation { } } } From 501be7527238fdb5871ebf5db1c3beaf11ebc1d6 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Thu, 31 Aug 2023 22:38:50 +0200 Subject: [PATCH 03/17] Fix proceeding to default impls in interface proxies --- .../InterfaceProxyWithoutTargetContributor.cs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs index 739abac32b..3d330beedf 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs @@ -17,9 +17,11 @@ namespace Castle.DynamicProxy.Contributors using System; using System.Collections.Generic; using System.Diagnostics; + using System.Reflection; using Castle.DynamicProxy.Generators; using Castle.DynamicProxy.Generators.Emitters; + using Castle.DynamicProxy.Generators.Emitters.SimpleAST; using Castle.DynamicProxy.Internal; internal class InterfaceProxyWithoutTargetContributor : CompositeTypeContributor @@ -70,6 +72,16 @@ private Type GetInvocationType(MetaMethod method, ClassEmitter emitter) return typeof(InterfaceMethodWithoutTargetInvocation); } + if (canChangeTarget == false && methodInfo.IsAbstract == false) + { + // This allows proceeding to a interface method's default implementation. + // The code has been copied over from `ClassProxyTargetContributor`. + var callback = CreateCallbackMethod(emitter, methodInfo, method.MethodOnTarget); + return new InheritanceInvocationTypeGenerator(callback.DeclaringType, method, callback, null) + .Generate(emitter, namingScope) + .BuildType(); + } + var scope = emitter.ModuleScope; Type[] invocationInterfaces; if (canChangeTarget) @@ -93,5 +105,24 @@ private Type GetInvocationType(MetaMethod method, ClassEmitter emitter) .Generate(emitter, namingScope) .BuildType()); } + + private MethodInfo CreateCallbackMethod(ClassEmitter emitter, MethodInfo methodInfo, MethodInfo methodOnTarget) + { + var targetMethod = methodOnTarget ?? methodInfo; + var callBackMethod = emitter.CreateMethod(namingScope.GetUniqueName(methodInfo.Name + "_callback"), targetMethod); + + if (targetMethod.IsGenericMethod) + { + targetMethod = targetMethod.MakeGenericMethod(callBackMethod.GenericTypeParams.AsTypeArray()); + } + + // invocation on base interface + + callBackMethod.CodeBuilder.AddStatement( + new ReturnStatement( + new MethodInvocationExpression(SelfReference.Self, targetMethod, callBackMethod.Arguments))); + + return callBackMethod.MethodBuilder; + } } } \ No newline at end of file From 65475c5098160d03900bcbaed765b4b241b996a3 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Thu, 31 Aug 2023 22:55:13 +0200 Subject: [PATCH 04/17] Polish & optimize the code changes just made --- .../Contributors/InterfaceMembersCollector.cs | 2 +- .../InterfaceProxyWithoutTargetContributor.cs | 43 +++++++++---------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersCollector.cs b/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersCollector.cs index 81d998dbd0..ba11dd0370 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersCollector.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersCollector.cs @@ -35,7 +35,7 @@ protected override MetaMethod GetMethodToGenerate(MethodInfo method, IProxyGener return null; } - return new MetaMethod(method, method, isStandalone, proxyable, false); + return new MetaMethod(method, method, isStandalone, proxyable, hasTarget: !method.IsAbstract); } } } \ No newline at end of file diff --git a/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs index 3d330beedf..d450b39c4b 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs @@ -65,33 +65,32 @@ private Type GetInvocationType(MetaMethod method, ClassEmitter emitter) { var methodInfo = method.Method; - if (canChangeTarget == false && methodInfo.IsAbstract) + if (canChangeTarget == false) { - // We do not need to generate a custom invocation type because no custom implementation - // for `InvokeMethodOnTarget` will be needed (proceeding to target isn't possible here): - return typeof(InterfaceMethodWithoutTargetInvocation); + if (!method.HasTarget) + { + // We do not need to generate a custom invocation type because no custom implementation + // for `InvokeMethodOnTarget` will be needed (proceeding to target isn't possible here): + return typeof(InterfaceMethodWithoutTargetInvocation); + } + else + { + // We end up here for interface methods with a default implementation: + Debug.Assert(methodInfo.DeclaringType.IsInterface && methodInfo.IsAbstract == false); + + // This allows proceeding to a interface method's default implementation. + // The code has been copied over from `ClassProxyTargetContributor`. + var callback = CreateCallbackMethod(emitter, methodInfo, method.MethodOnTarget); + return new InheritanceInvocationTypeGenerator(callback.DeclaringType, method, callback, null) + .Generate(emitter, namingScope) + .BuildType(); + } } - if (canChangeTarget == false && methodInfo.IsAbstract == false) - { - // This allows proceeding to a interface method's default implementation. - // The code has been copied over from `ClassProxyTargetContributor`. - var callback = CreateCallbackMethod(emitter, methodInfo, method.MethodOnTarget); - return new InheritanceInvocationTypeGenerator(callback.DeclaringType, method, callback, null) - .Generate(emitter, namingScope) - .BuildType(); - } + Debug.Assert(canChangeTarget); var scope = emitter.ModuleScope; - Type[] invocationInterfaces; - if (canChangeTarget) - { - invocationInterfaces = new[] { typeof(IInvocation), typeof(IChangeProxyTarget) }; - } - else - { - invocationInterfaces = new[] { typeof(IInvocation) }; - } + Type[] invocationInterfaces = new[] { typeof(IInvocation), typeof(IChangeProxyTarget) }; var key = new CacheKey(methodInfo, CompositionInvocationTypeGenerator.BaseType, invocationInterfaces, null); // no locking required as we're already within a lock From ce4be78aa0571393a6258ba1481a712f7239eff1 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Sat, 21 May 2022 14:28:06 +0200 Subject: [PATCH 05/17] Fix interception of inherited default impls in class proxies A class implementing an interface with default method implementations will not necessarily override those methods; in such cases, the methods are currently "invisible" to DynamicProxy and thus won't get proxied. This can be solved by an extra pass over all interfaces to collect such methods, as is done in this commit. Ideally we wouldn't have to iterate over all interfaces more than once, so we should next make an effort to limit the runtime impact of this extra pass. --- .../ClassProxyTargetContributor.cs | 5 ++ ...mbersWithDefaultImplementationCollector.cs | 61 +++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs diff --git a/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs index a7621b8c53..81770a1a17 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs @@ -42,6 +42,11 @@ protected override IEnumerable GetCollectors() var targetItem = new ClassMembersCollector(targetType) { Logger = Logger }; yield return targetItem; + foreach (var @interface in targetType.GetAllInterfaces()) + { + yield return new InterfaceMembersWithDefaultImplementationCollector(@interface, targetType); + } + foreach (var @interface in interfaces) { var item = new InterfaceMembersOnClassCollector(@interface, true, diff --git a/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs b/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs new file mode 100644 index 0000000000..cd26407acd --- /dev/null +++ b/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs @@ -0,0 +1,61 @@ +// Copyright 2004-2023 Castle Project - http://www.castleproject.org/ +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +namespace Castle.DynamicProxy.Contributors +{ + using System; + using System.Diagnostics; + using System.Reflection; + + using Castle.DynamicProxy.Generators; + + internal sealed class InterfaceMembersWithDefaultImplementationCollector : MembersCollector + { + private readonly InterfaceMapping map; + + public InterfaceMembersWithDefaultImplementationCollector(Type interfaceType, Type classToProxy) + : base(interfaceType) + { + Debug.Assert(interfaceType != null); + Debug.Assert(interfaceType.IsInterface); + + Debug.Assert(classToProxy != null); + Debug.Assert(classToProxy.IsClass); + + map = classToProxy.GetInterfaceMap(interfaceType); + } + + protected override MetaMethod GetMethodToGenerate(MethodInfo method, IProxyGenerationHook hook, bool isStandalone) + { + var index = Array.IndexOf(map.InterfaceMethods, method); + Debug.Assert(index >= 0); + + var methodOnTarget = map.TargetMethods[index]; + if (methodOnTarget.DeclaringType.IsInterface == false) + { + // An interface method can have its default implementation "overridden" in the class, + // in which case this collector isn't interested in it and another should deal with it. + return null; + } + + var proxyable = AcceptMethod(method, true, hook); + if (!proxyable) + { + return null; + } + + return new MetaMethod(method, methodOnTarget, true, proxyable, !method.IsAbstract); + } + } +} From 9e7a15ee57822948e5f8eaadeda3f8ee6774a2f5 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Fri, 1 Sep 2023 00:40:04 +0200 Subject: [PATCH 06/17] Reduce impact of extra collection pass Assuming that default interface methods are relatively rare, if we keep track of which interfaces contain any of them at all, we can then limit our extra pass during class proxy generation to just those interfaces and exclude the rest (i.e. hopefully the majority). Additionally, our collector may skip some of the more expensive checks by bailing out for methods it isn't actually interested in. --- .../ClassProxyTargetContributor.cs | 5 ++++ ...mbersWithDefaultImplementationCollector.cs | 7 +++++ .../DynamicProxy/Internal/TypeUtil.cs | 27 +++++++++++++++++++ 3 files changed, 39 insertions(+) diff --git a/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs index 81770a1a17..fb376b025e 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs @@ -44,6 +44,11 @@ protected override IEnumerable GetCollectors() foreach (var @interface in targetType.GetAllInterfaces()) { + if (@interface.HasAnyOverridableDefaultImplementations() == false) + { + continue; + } + yield return new InterfaceMembersWithDefaultImplementationCollector(@interface, targetType); } diff --git a/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs b/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs index cd26407acd..e183bffe0e 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs @@ -38,6 +38,13 @@ public InterfaceMembersWithDefaultImplementationCollector(Type interfaceType, Ty protected override MetaMethod GetMethodToGenerate(MethodInfo method, IProxyGenerationHook hook, bool isStandalone) { + if (method.IsAbstract) + { + // This collector is only interested in methods with default implementations. + // All other interface methods should be dealt with in other contributors. + return null; + } + var index = Array.IndexOf(map.InterfaceMethods, method); Debug.Assert(index >= 0); diff --git a/src/Castle.Core/DynamicProxy/Internal/TypeUtil.cs b/src/Castle.Core/DynamicProxy/Internal/TypeUtil.cs index 1074b69bbc..4abb205cd5 100644 --- a/src/Castle.Core/DynamicProxy/Internal/TypeUtil.cs +++ b/src/Castle.Core/DynamicProxy/Internal/TypeUtil.cs @@ -27,6 +27,7 @@ namespace Castle.DynamicProxy.Internal public static class TypeUtil { private static readonly Dictionary instanceMethodsCache = new Dictionary(); + private static readonly Dictionary hasAnyOverridableDefaultImplementationsCache = new Dictionary(); internal static bool IsNullableType(this Type type) { @@ -134,6 +135,32 @@ public static Type GetTypeOrNull(object target) return target.GetType(); } + internal static bool HasAnyOverridableDefaultImplementations(this Type interfaceType) + { + Debug.Assert(interfaceType != null); + Debug.Assert(interfaceType.IsInterface); + + var cache = hasAnyOverridableDefaultImplementationsCache; + lock (cache) + { + if (!cache.TryGetValue(interfaceType, out var result)) + { + foreach (var method in interfaceType.GetAllInstanceMethods()) + { + if (method.IsAbstract == false && method.IsFinal == false && method.IsVirtual) + { + result = true; + break; + } + } + + cache[interfaceType] = result; + } + + return result; + } + } + internal static Type[] AsTypeArray(this GenericTypeParameterBuilder[] typeInfos) { Type[] types = new Type[typeInfos.Length]; From c542f13515f06b123f486ece88971dab8cbcfd04 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Fri, 1 Sep 2023 00:59:06 +0200 Subject: [PATCH 07/17] Add more tests for default interface property impls --- .../DefaultInterfaceMembersTestCase.cs | 89 ++++++++++++++++++- 1 file changed, 87 insertions(+), 2 deletions(-) diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs index 0be67095e9..f623425da5 100644 --- a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs @@ -78,7 +78,7 @@ public void Can_intercept_method_with_default_implementation_in_proxied_interfac } [Test] - public void Can_proceed_to_default_implementation_in_proxied_class() + public void Can_proceed_to_method_default_implementation_in_proxied_class() { var interceptor = new WithCallbackInterceptor(invocation => invocation.Proceed()); var proxy = generator.CreateClassProxy(interceptor); @@ -88,7 +88,7 @@ public void Can_proceed_to_default_implementation_in_proxied_class() } [Test] - public void Can_proceed_to_default_implementation_in_proxied_interface() + public void Can_proceed_to_method_default_implementation_in_proxied_interface() { var interceptor = new WithCallbackInterceptor(invocation => invocation.Proceed()); var proxy = generator.CreateInterfaceProxyWithoutTarget(interceptor); @@ -97,6 +97,78 @@ public void Can_proceed_to_default_implementation_in_proxied_interface() Assert.AreEqual(expected, actual); } + [Test] + public void Can_proxy_class_that_inherits_property_with_default_implementation_from_interface() + { + _ = generator.CreateClassProxy(); + } + + [Test] + public void Can_proxy_interface_with_property_with_default_implementation() + { + _ = generator.CreateInterfaceProxyWithoutTarget(); + } + + [Test] + public void Default_implementation_gets_called_when_property_not_intercepted_in_proxied_class() + { + var options = new ProxyGenerationOptions(new ProxyNothingHook()); + var proxy = generator.CreateClassProxy(options); + var expected = "default implementation"; + var actual = ((IHavePropertyWithDefaultImplementation)proxy).PropertyWithDefaultImplementation; + Assert.AreEqual(expected, actual); + } + + [Test] + public void Default_implementation_gets_called_when_property_not_intercepted_in_proxied_interface() + { + var options = new ProxyGenerationOptions(new ProxyNothingHook()); + var proxy = generator.CreateInterfaceProxyWithoutTarget(options); + var expected = "default implementation"; + var actual = proxy.PropertyWithDefaultImplementation; + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_intercept_property_with_default_implementation_in_proxied_class() + { + var expected = "intercepted"; + var interceptor = new WithCallbackInterceptor(invocation => invocation.ReturnValue = expected); + var proxy = generator.CreateClassProxy(interceptor); + var actual = ((IHavePropertyWithDefaultImplementation)proxy).PropertyWithDefaultImplementation; + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_intercept_property_with_default_implementation_in_proxied_interface() + { + var expected = "intercepted"; + var interceptor = new WithCallbackInterceptor(invocation => invocation.ReturnValue = expected); + var proxy = generator.CreateInterfaceProxyWithoutTarget(interceptor); + var actual = proxy.PropertyWithDefaultImplementation; + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_proceed_to_property_default_implementation_in_proxied_class() + { + var interceptor = new WithCallbackInterceptor(invocation => invocation.Proceed()); + var proxy = generator.CreateClassProxy(interceptor); + var expected = "default implementation"; + var actual = ((IHavePropertyWithDefaultImplementation)proxy).PropertyWithDefaultImplementation; + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_proceed_to_property_default_implementation_in_proxied_interface() + { + var interceptor = new WithCallbackInterceptor(invocation => invocation.Proceed()); + var proxy = generator.CreateInterfaceProxyWithoutTarget(interceptor); + var expected = "default implementation"; + var actual = proxy.PropertyWithDefaultImplementation; + Assert.AreEqual(expected, actual); + } + [Test] public void Can_proxy_interface_with_sealed_method() { @@ -120,6 +192,17 @@ string MethodWithDefaultImplementation() } } + public interface IHavePropertyWithDefaultImplementation + { + string PropertyWithDefaultImplementation + { + get + { + return "default implementation"; + } + } + } + public interface IHaveSealedMethod { sealed string SealedMethod() @@ -129,6 +212,8 @@ sealed string SealedMethod() } public class InheritsMethodWithDefaultImplementation : IHaveMethodWithDefaultImplementation { } + + public class InheritsPropertyWithDefaultImplementation : IHavePropertyWithDefaultImplementation { } } } From 840748aa63819a7db0094160ac660a4eaa9947ba Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Fri, 1 Sep 2023 01:28:35 +0200 Subject: [PATCH 08/17] Fix tests for properties with default impls ... by properly forwarding the "standalone" flag in the newly added collector. --- .../InterfaceMembersWithDefaultImplementationCollector.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs b/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs index e183bffe0e..d365a78ffb 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs @@ -62,7 +62,7 @@ protected override MetaMethod GetMethodToGenerate(MethodInfo method, IProxyGener return null; } - return new MetaMethod(method, methodOnTarget, true, proxyable, !method.IsAbstract); + return new MetaMethod(method, methodOnTarget, isStandalone, proxyable, !method.IsAbstract); } } } From a7b6a7c1660623192b16978f0c76407bf64a8bf9 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Fri, 1 Sep 2023 10:37:17 +0200 Subject: [PATCH 09/17] Add more tests re: methods with default impls Those essentially target "mixed" scenarios where default implementations show up next to "regular" methods. This is to better ensure that the new code paths will not interfere with pre-existing logic. --- .../DefaultInterfaceMembersTestCase.cs | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs index f623425da5..04e15d5bcb 100644 --- a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs @@ -97,6 +97,46 @@ public void Can_proceed_to_method_default_implementation_in_proxied_interface() Assert.AreEqual(expected, actual); } + [Test] + public void Can_intercept_method_with_overridden_default_implementation_in_proxied_class() + { + var expected = "intercepted"; + var interceptor = new WithCallbackInterceptor(invocation => invocation.ReturnValue = expected); + var proxy = generator.CreateClassProxy(interceptor); + var actual = ((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_intercept_method_that_is_sibling_of_method_with_default_implementation_in_proxied_class() + { + var expected = "intercepted"; + var interceptor = new WithCallbackInterceptor(invocation => invocation.ReturnValue = expected); + var proxy = generator.CreateClassProxy(interceptor); + var actual = proxy.Method(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_intercept_method_that_is_sibling_of_method_with_default_implementation_in_proxied_interface() + { + var expected = "intercepted"; + var interceptor = new WithCallbackInterceptor(invocation => invocation.ReturnValue = expected); + var proxy = generator.CreateInterfaceProxyWithoutTarget(interceptor); + var actual = proxy.Method(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_proceed_to_base_implementation_of_method_that_is_sibling_of_method_with_default_implementation_in_proxied_class() + { + var expected = "class implementation"; + var interceptor = new WithCallbackInterceptor(invocation => invocation.Proceed()); + var proxy = generator.CreateClassProxy(interceptor); + var actual = proxy.Method(); + Assert.AreEqual(expected, actual); + } + [Test] public void Can_proxy_class_that_inherits_property_with_default_implementation_from_interface() { @@ -192,6 +232,11 @@ string MethodWithDefaultImplementation() } } + public interface IHaveMethodWithDefaultImplementationAmongOtherThings : IHaveMethodWithDefaultImplementation + { + string Method(); + } + public interface IHavePropertyWithDefaultImplementation { string PropertyWithDefaultImplementation @@ -213,7 +258,23 @@ sealed string SealedMethod() public class InheritsMethodWithDefaultImplementation : IHaveMethodWithDefaultImplementation { } + public class InheritsMethodWithDefaultImplementationAmongOtherThings : IHaveMethodWithDefaultImplementationAmongOtherThings + { + public virtual string Method() + { + return "class implementation"; + } + } + public class InheritsPropertyWithDefaultImplementation : IHavePropertyWithDefaultImplementation { } + + public class OverridesMethodWithDefaultImplementation : IHaveMethodWithDefaultImplementation + { + public virtual string MethodWithDefaultImplementation() + { + return "class implementation"; + } + } } } From 0bb493cfc8bd361312124bbc3dab274c385a6784 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Fri, 1 Sep 2023 11:25:08 +0200 Subject: [PATCH 10/17] Add test for generic method with default impl The code changes we made do not suggest that pre-existing logic dealing with generics might somehow no longer work, however we did add a bit of new code for generics which we should target with at least one test. --- .../DefaultInterfaceMembersTestCase.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs index 04e15d5bcb..fc9df43a2a 100644 --- a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs @@ -97,6 +97,18 @@ public void Can_proceed_to_method_default_implementation_in_proxied_interface() Assert.AreEqual(expected, actual); } + [Test] + public void Can_proceed_to_generic_method_default_implementation_in_proxied_interface() + { + // (This test targets the code regarding generics inside `InterfaceProxyWithoutTargetContributor.CreateCallbackMethod`.) + + var interceptor = new WithCallbackInterceptor(invocation => invocation.Proceed()); + var proxy = generator.CreateInterfaceProxyWithoutTarget(interceptor); + var expected = "default implementation for Int32"; + var actual = proxy.GenericMethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + [Test] public void Can_intercept_method_with_overridden_default_implementation_in_proxied_class() { @@ -224,6 +236,14 @@ public void Can_invoke_sealed_method_in_proxied_interface() Assert.AreEqual(expected, actual); } + public interface IHaveGenericMethodWithDefaultImplementation + { + string GenericMethodWithDefaultImplementation() + { + return "default implementation for " + typeof(T).Name; + } + } + public interface IHaveMethodWithDefaultImplementation { string MethodWithDefaultImplementation() From fe406347d490ba8d3d91fb1f0f2e76070ee0869c Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Fri, 1 Sep 2023 11:52:41 +0200 Subject: [PATCH 11/17] Add tests for static interface methods This includes a test for `static abstract` methods that remains ignored for now, since we haven't implemented support for this C# 11 language feature, nor do we target any runtimes that support it. (The test is being added as a future reminder, once we start targeting .NET 7+.) --- buildscripts/common.props | 2 +- .../DefaultInterfaceMembersTestCase.cs | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/buildscripts/common.props b/buildscripts/common.props index 157f3f4433..e7f541ce4d 100644 --- a/buildscripts/common.props +++ b/buildscripts/common.props @@ -1,7 +1,7 @@ - 9.0 + 11.0 $(NoWarn);CS1591;CS3014;CS3003;CS3001;CS3021 $(NoWarn);CS0612;CS0618 git diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs index fc9df43a2a..aab7ea6b14 100644 --- a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs @@ -236,6 +236,21 @@ public void Can_invoke_sealed_method_in_proxied_interface() Assert.AreEqual(expected, actual); } + [Test] + public void Can_proxy_interface_with_static_method() + { + _ = generator.CreateInterfaceProxyWithoutTarget(); + } + +#if NET7_0_OR_GREATER + [Test] + [Ignore("Support for static abstract interface members has not yet been implemented.")] + public void Can_proxy_interface_with_static_abstract_method() + { + _ = generator.CreateInterfaceProxyWithoutTarget(); + } +#endif + public interface IHaveGenericMethodWithDefaultImplementation { string GenericMethodWithDefaultImplementation() @@ -276,6 +291,21 @@ sealed string SealedMethod() } } + public interface IHaveStaticMethod + { + static string StaticMethod() + { + return "default implementation"; + } + } + +#if NET7_0_OR_GREATER + public interface IHaveStaticAbstractMethod + { + static abstract string StaticAbstractMethod(); + } +#endif + public class InheritsMethodWithDefaultImplementation : IHaveMethodWithDefaultImplementation { } public class InheritsMethodWithDefaultImplementationAmongOtherThings : IHaveMethodWithDefaultImplementationAmongOtherThings From 27828bd3c4b2856ff78b2ea60b6cf3a8d62c3f9c Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Fri, 1 Sep 2023 12:32:42 +0200 Subject: [PATCH 12/17] Add tests for method default impls in additional interfaces --- .../DefaultInterfaceMembersTestCase.cs | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs index aab7ea6b14..4de731d6c8 100644 --- a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs @@ -17,6 +17,7 @@ using System.Reflection; using Castle.DynamicProxy.Tests.Interceptors; +using Castle.DynamicProxy.Tests.Interfaces; using NUnit.Framework; @@ -221,6 +222,58 @@ public void Can_proceed_to_property_default_implementation_in_proxied_interface( Assert.AreEqual(expected, actual); } + [Test] + public void Can_proxy_class_that_inherits_method_with_default_implementation_from_additional_interface() + { + _ = generator.CreateClassProxy(typeof(object), new[] { typeof(IHaveMethodWithDefaultImplementation) }); + } + + [Test] + public void Can_proxy_interface_with_method_with_default_implementation_from_additional_interface() + { + _ = generator.CreateInterfaceProxyWithoutTarget(typeof(IEmpty), new[] { typeof(IHaveMethodWithDefaultImplementation) }); + } + + [Test] + public void Can_intercept_method_with_default_implementation_from_additional_interface_in_proxied_class() + { + var expected = "intercepted"; + var interceptor = new WithCallbackInterceptor(invocation => invocation.ReturnValue = expected); + var proxy = generator.CreateClassProxy(typeof(object), new[] { typeof(IHaveMethodWithDefaultImplementation) },interceptor); + var actual = ((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_intercept_method_with_default_implementation_from_additional_interface_in_proxied_interface() + { + var expected = "intercepted"; + var interceptor = new WithCallbackInterceptor(invocation => invocation.ReturnValue = expected); + var proxy = generator.CreateInterfaceProxyWithoutTarget(typeof(IEmpty), new[] { typeof(IHaveMethodWithDefaultImplementation) },interceptor); + var actual = ((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_proceed_to_method_default_implementation_from_additional_interface_in_proxied_class() + { + var interceptor = new WithCallbackInterceptor(invocation => invocation.Proceed()); + var proxy = generator.CreateClassProxy(typeof(object), new[] { typeof(IHaveMethodWithDefaultImplementation) }, interceptor); + var expected = "default implementation"; + var actual = ((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_proceed_to_method_default_implementation_from_additional_interface_in_proxied_interface() + { + var interceptor = new WithCallbackInterceptor(invocation => invocation.Proceed()); + var proxy = generator.CreateInterfaceProxyWithoutTarget(typeof(IEmpty), new[] { typeof(IHaveMethodWithDefaultImplementation) }, interceptor); + var expected = "default implementation"; + var actual = ((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + [Test] public void Can_proxy_interface_with_sealed_method() { From 09d4c15bb0bf7e580c7bbc793d76b7308950ae6e Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Fri, 1 Sep 2023 12:38:17 +0200 Subject: [PATCH 13/17] Group tests using `#region`s --- .../DefaultInterfaceMembersTestCase.cs | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs index 4de731d6c8..046952ad35 100644 --- a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs @@ -26,6 +26,8 @@ namespace Castle.DynamicProxy.Tests [TestFixture] public class DefaultInterfaceMembersTestCase : BasePEVerifyTestCase { + #region Methods with default implementation + [Test] public void Can_proxy_class_that_inherits_method_with_default_implementation_from_interface() { @@ -98,6 +100,10 @@ public void Can_proceed_to_method_default_implementation_in_proxied_interface() Assert.AreEqual(expected, actual); } + #endregion + + #region Generic methods with default implementation + [Test] public void Can_proceed_to_generic_method_default_implementation_in_proxied_interface() { @@ -110,6 +116,10 @@ public void Can_proceed_to_generic_method_default_implementation_in_proxied_inte Assert.AreEqual(expected, actual); } + #endregion + + #region Methods with "overridden" default implementation + [Test] public void Can_intercept_method_with_overridden_default_implementation_in_proxied_class() { @@ -120,6 +130,10 @@ public void Can_intercept_method_with_overridden_default_implementation_in_proxi Assert.AreEqual(expected, actual); } + #endregion + + #region Siblings of methods with default implementation + [Test] public void Can_intercept_method_that_is_sibling_of_method_with_default_implementation_in_proxied_class() { @@ -150,6 +164,10 @@ public void Can_proceed_to_base_implementation_of_method_that_is_sibling_of_meth Assert.AreEqual(expected, actual); } + #endregion + + #region Properties with default implementation + [Test] public void Can_proxy_class_that_inherits_property_with_default_implementation_from_interface() { @@ -222,6 +240,10 @@ public void Can_proceed_to_property_default_implementation_in_proxied_interface( Assert.AreEqual(expected, actual); } + #endregion + + #region Methods with default implementation in additional interfaces + [Test] public void Can_proxy_class_that_inherits_method_with_default_implementation_from_additional_interface() { @@ -274,6 +296,10 @@ public void Can_proceed_to_method_default_implementation_from_additional_interfa Assert.AreEqual(expected, actual); } + #endregion + + #region Sealed members + [Test] public void Can_proxy_interface_with_sealed_method() { @@ -289,6 +315,10 @@ public void Can_invoke_sealed_method_in_proxied_interface() Assert.AreEqual(expected, actual); } + #endregion + + #region Static members + [Test] public void Can_proxy_interface_with_static_method() { @@ -304,6 +334,8 @@ public void Can_proxy_interface_with_static_abstract_method() } #endif + #endregion + public interface IHaveGenericMethodWithDefaultImplementation { string GenericMethodWithDefaultImplementation() From 982f5ff86bc1280f7ff35bb364446f26a1db8e9e Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Fri, 1 Sep 2023 12:50:49 +0200 Subject: [PATCH 14/17] Add tests for protected methods with default impls A few of these tests fail due to `Debug.Assert` failures in `Interface- MembersWithDefaultImplementationCollector`. --- .../DefaultInterfaceMembersTestCase.cs | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs index 046952ad35..77ffab73cb 100644 --- a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs @@ -298,6 +298,77 @@ public void Can_proceed_to_method_default_implementation_from_additional_interfa #endregion + #region Non-public methods with default implementation + + public void Can_proxy_class_that_inherits_protected_method_with_default_implementation_from_interface() + { + _ = generator.CreateClassProxy(); + } + + [Test] + public void Can_proxy_interface_with_protected_method_with_default_implementation() + { + _ = generator.CreateInterfaceProxyWithoutTarget(); + } + + [Test] + public void Can_intercept_protected_method_with_default_implementation_in_proxied_class() + { + var expected = "intercepted"; + var interceptor = new WithCallbackInterceptor(invocation => + { + Assume.That(invocation.Method.Name == "ProtectedMethodWithDefaultImplementation"); + invocation.ReturnValue = expected; + }); + var proxy = generator.CreateClassProxy(interceptor); + var actual = ((IHaveProtectedMethodWithDefaultImplementation)proxy).InvokeProtectedMethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_intercept_protected_method_with_default_implementation_in_proxied_interface() + { + var expected = "intercepted"; + var interceptor = new WithCallbackInterceptor(invocation => + { + Assume.That(invocation.Method.Name == "ProtectedMethodWithDefaultImplementation"); + invocation.ReturnValue = expected; + }); + var proxy = generator.CreateInterfaceProxyWithoutTarget(interceptor); + var actual = proxy.InvokeProtectedMethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_proceed_to_protected_method_default_implementation_in_proxied_class() + { + var interceptor = new WithCallbackInterceptor(invocation => + { + Assume.That(invocation.Method.Name == "ProtectedMethodWithDefaultImplementation"); + invocation.Proceed(); + }); + var proxy = generator.CreateClassProxy(interceptor); + var expected = "default implementation"; + var actual = ((IHaveProtectedMethodWithDefaultImplementation)proxy).InvokeProtectedMethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_proceed_to_protected_method_default_implementation_in_proxied_interface() + { + var interceptor = new WithCallbackInterceptor(invocation => + { + Assume.That(invocation.Method.Name == "ProtectedMethodWithDefaultImplementation"); + invocation.Proceed(); + }); + var proxy = generator.CreateInterfaceProxyWithoutTarget(interceptor); + var expected = "default implementation"; + var actual = proxy.InvokeProtectedMethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + #endregion + #region Sealed members [Test] @@ -357,6 +428,19 @@ public interface IHaveMethodWithDefaultImplementationAmongOtherThings : IHaveMet string Method(); } + public interface IHaveProtectedMethodWithDefaultImplementation + { + sealed string InvokeProtectedMethodWithDefaultImplementation() + { + return ProtectedMethodWithDefaultImplementation(); + } + + protected string ProtectedMethodWithDefaultImplementation() + { + return "default implementation"; + } + } + public interface IHavePropertyWithDefaultImplementation { string PropertyWithDefaultImplementation @@ -401,6 +485,9 @@ public virtual string Method() } } + public class InheritsProtectedMethodWithDefaultImplementation : IHaveProtectedMethodWithDefaultImplementation { } + + public class InheritsPropertyWithDefaultImplementation : IHavePropertyWithDefaultImplementation { } public class OverridesMethodWithDefaultImplementation : IHaveMethodWithDefaultImplementation From e41315c43eef96fe7d4c78205be67c65fc45a8ed Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Fri, 1 Sep 2023 12:53:50 +0200 Subject: [PATCH 15/17] Exclude non-overridable methods from collection `sealed` methods do not show up in interface mappings, so make sure `InterfaceMembersWithDefaultImplementationCollector` can deal with them anyway in case of interfaces that contain both `sealed` methods as well as overridable ones with a default implementation. --- .../InterfaceMembersWithDefaultImplementationCollector.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs b/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs index d365a78ffb..70293a5c9b 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersWithDefaultImplementationCollector.cs @@ -38,10 +38,13 @@ public InterfaceMembersWithDefaultImplementationCollector(Type interfaceType, Ty protected override MetaMethod GetMethodToGenerate(MethodInfo method, IProxyGenerationHook hook, bool isStandalone) { - if (method.IsAbstract) + if (method.IsAbstract || method.IsFinal || method.IsVirtual == false) { - // This collector is only interested in methods with default implementations. + // This collector is only interested in overridable methods with default implementations. // All other interface methods should be dealt with in other contributors. + // + // (Note this is the opposite check of that in `TypeUtil.HasAnyOverridableDefaultImplementations`, + // which is the method used to filter out whole interfaces before this collector gets run for them.) return null; } From ae9139fa2232b1da42233319eb2344c80f1fd983 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Fri, 1 Sep 2023 13:03:43 +0200 Subject: [PATCH 16/17] Add tests for method default impls in mixins --- .../DefaultInterfaceMembersTestCase.cs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs index 77ffab73cb..cc4838a593 100644 --- a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs @@ -407,6 +407,57 @@ public void Can_proxy_interface_with_static_abstract_method() #endregion + #region Mixins + + [Test] + public void Can_intercept_method_with_default_implementation_from_mixin_in_proxied_class() + { + var options = new ProxyGenerationOptions(); + options.AddMixinInstance(new InheritsMethodWithDefaultImplementation()); + var expected = "intercepted"; + var interceptor = new WithCallbackInterceptor(invocation => invocation.ReturnValue = expected); + var proxy = generator.CreateClassProxy(typeof(object), options, interceptor); + var actual = ((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_intercept_method_with_default_implementation_from_mixin_in_proxied_interface() + { + var options = new ProxyGenerationOptions(); + options.AddMixinInstance(new InheritsMethodWithDefaultImplementation()); + var expected = "intercepted"; + var interceptor = new WithCallbackInterceptor(invocation => invocation.ReturnValue = expected); + var proxy = generator.CreateInterfaceProxyWithoutTarget(typeof(IEmpty), options, interceptor); + var actual = ((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_proceed_to_method_default_implementation_from_mixin_in_proxied_class() + { + var options = new ProxyGenerationOptions(); + options.AddMixinInstance(new InheritsMethodWithDefaultImplementation()); + var interceptor = new WithCallbackInterceptor(invocation => invocation.Proceed()); + var proxy = generator.CreateClassProxy(typeof(object), options, interceptor); + var expected = "default implementation"; + var actual = ((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + + [Test] + public void Can_proceed_to_method_default_implementation_from_mixin_in_proxied_interface() + { + var options = new ProxyGenerationOptions(); + options.AddMixinInstance(new InheritsMethodWithDefaultImplementation()); + var interceptor = new WithCallbackInterceptor(invocation => invocation.Proceed()); + var proxy = generator.CreateInterfaceProxyWithoutTarget(typeof(IEmpty), options, interceptor); + var expected = "default implementation"; + var actual = ((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation(); + Assert.AreEqual(expected, actual); + } + #endregion + public interface IHaveGenericMethodWithDefaultImplementation { string GenericMethodWithDefaultImplementation() From 51112db2c11efc4d1ed65fa8567b8afca09f0c94 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Fri, 1 Sep 2023 12:07:40 +0200 Subject: [PATCH 17/17] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7c205052b..d737add665 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Enhancements: - Two new generic method overloads `proxyGenerator.CreateClassProxy([options], constructorArguments, interceptors)` (@backstromjoel, #636) - Allow specifying which attributes should always be copied to proxy class by adding attribute type to `AttributesToAlwaysReplicate`. Previously only non-inherited, with `Inherited=false`, attributes were copied. (@shoaibshakeel381, #633) +- Support for C# 8+ [default interface methods](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/default-interface-methods) in interface and class proxies without target (@stakx, #661) Bugfixes: - `ArgumentException`: "Could not find method overriding method" with overridden class method having generic by-ref parameter (@stakx, #657)