From f937cc1a766250772d1a6f582fab1d11594591c2 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Sun, 10 Sep 2023 23:56:23 +0200 Subject: [PATCH] Optimize & document `InterfaceMembersOnClassCollector` (#667) * Inline `IsVirtuallyImplementedInterfaceMethod` * Remove redundant call to `GetMethodOnTarget` * Add explanation for the `onlyProxyVirtual` flag --- .../InterfaceMembersOnClassCollector.cs | 37 ++++++++++++++----- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersOnClassCollector.cs b/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersOnClassCollector.cs index 27e50c098..cce4b3eb0 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersOnClassCollector.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersOnClassCollector.cs @@ -32,12 +32,35 @@ public InterfaceMembersOnClassCollector(Type type, bool onlyProxyVirtual, Interf protected override MetaMethod GetMethodToGenerate(MethodInfo method, IProxyGenerationHook hook, bool isStandalone) { - if (onlyProxyVirtual && IsVirtuallyImplementedInterfaceMethod(method)) + var methodOnTarget = GetMethodOnTarget(method); + + if (onlyProxyVirtual) { - return null; - } + // The (somewhat confusingly named) `onlyProxyVirtual` flag may need some explaining. + // + // This collector type is used in two distinct scenarios: + // + // 1. When generating a class proxy for some class `T` which implements interface `I`, + // and `I` is again specified as an additional interface to add to the proxy type. + // In this case, this collector gets invoked for `I` and `onlyProxyVirtual == true`, + // and below logic prevents `I` methods from being implemented a second time when + // the main "target" contributor already took care of them (which happens when they + // are overridable, or more specifically, when they are implicitly implemented and + // marked as `virtual`). + // + // 2. When generating an interface proxy with target for some interface `I` and target + // type `T`. In this case, `onlyProxyVirtual == false`, which forces members of `I` + // to get implemented. Unlike in (1), the target of such proxies will be separate + // objects, so it doesn't matter if & how they implement members of `I` or not; + // those `I` members still need to be implemented on the proxy type regardless. - var methodOnTarget = GetMethodOnTarget(method); + var isVirtuallyImplementedInterfaceMethod = methodOnTarget != null && methodOnTarget.IsFinal == false; + + if (isVirtuallyImplementedInterfaceMethod) + { + return null; + } + } var proxyable = AcceptMethod(method, onlyProxyVirtual, hook); return new MetaMethod(method, methodOnTarget, isStandalone, proxyable, methodOnTarget.IsPrivate == false); @@ -53,11 +76,5 @@ private MethodInfo GetMethodOnTarget(MethodInfo method) return map.TargetMethods[index]; } - - private bool IsVirtuallyImplementedInterfaceMethod(MethodInfo method) - { - var info = GetMethodOnTarget(method); - return info != null && info.IsFinal == false; - } } } \ No newline at end of file