From 811387c9090e2deb3c96fba555a2a8c8d195ab68 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Mon, 4 Jan 2021 11:27:34 +0100 Subject: [PATCH] Extract abstract base class `BaseClassProxyGenerator` (#549) `ClassProxyGenerator` and `ClassProxyWithTargetGenerator` are mostly identical; extract the common bits into a new abstract base class. This makes it easier to spot the actual differences between the two. More importantly, it fixes the semantics of these types' hierarchy: `ClassProxyGenerator` is a "without target" generator. It is wrong for a "with target" generator to inherit from such a one as this contradicts the base class' semantics. The edit sequence for showing the similarities between the two types was as follows: 1. Move methods in `ClassProxyWithTargetGenerator` further up 2. Rewrite conditional `MixinContributor` type casts 3. Extract `GetProxyInstanceContributor` methods 4. Extract `GetProxyTargetContributor` methods 5. Extract `TargetField` properties --- .../Generators/BaseClassProxyGenerator.cs | 204 ++++++++++++++++++ .../Generators/ClassProxyGenerator.cs | 167 +------------- .../ClassProxyWithTargetGenerator.cs | 188 ++-------------- 3 files changed, 233 insertions(+), 326 deletions(-) create mode 100644 src/Castle.Core/DynamicProxy/Generators/BaseClassProxyGenerator.cs diff --git a/src/Castle.Core/DynamicProxy/Generators/BaseClassProxyGenerator.cs b/src/Castle.Core/DynamicProxy/Generators/BaseClassProxyGenerator.cs new file mode 100644 index 0000000000..ae31a7ef78 --- /dev/null +++ b/src/Castle.Core/DynamicProxy/Generators/BaseClassProxyGenerator.cs @@ -0,0 +1,204 @@ +// Copyright 2004-2021 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.Generators +{ + using System; + using System.Collections.Generic; + using System.Linq; + using System.Reflection; + + using Castle.DynamicProxy.Contributors; + using Castle.DynamicProxy.Generators.Emitters.SimpleAST; + using Castle.DynamicProxy.Internal; + + internal abstract class BaseClassProxyGenerator : BaseProxyGenerator + { + protected BaseClassProxyGenerator(ModuleScope scope, Type targetType, Type[] interfaces, ProxyGenerationOptions options) + : base(scope, targetType, interfaces, options) + { + EnsureDoesNotImplementIProxyTargetAccessor(targetType, "targetType"); + } + + protected abstract FieldReference TargetField { get; } + + protected abstract ProxyInstanceContributor GetProxyInstanceContributor(List methodsToSkip); + + protected abstract CompositeTypeContributor GetProxyTargetContributor(List methodsToSkip, INamingScope namingScope); + + protected sealed override Type GenerateType(string name, INamingScope namingScope) + { + IEnumerable contributors; + var allInterfaces = GetTypeImplementerMapping(out contributors, namingScope); + + var model = new MetaType(); + // Collect methods + foreach (var contributor in contributors) + { + contributor.CollectElementsToProxy(ProxyGenerationOptions.Hook, model); + } + ProxyGenerationOptions.Hook.MethodsInspected(); + + var emitter = BuildClassEmitter(name, targetType, allInterfaces); + + CreateFields(emitter); + CreateTypeAttributes(emitter); + + // Constructor + var cctor = GenerateStaticConstructor(emitter); + + var constructorArguments = new List(); + + if (TargetField is { } targetField) + { + constructorArguments.Add(targetField); + } + + foreach (var contributor in contributors) + { + contributor.Generate(emitter); + + // TODO: redo it + if (contributor is MixinContributor mixinContributor) + { + constructorArguments.AddRange(mixinContributor.Fields); + } + } + + // constructor arguments + var interceptorsField = emitter.GetField("__interceptors"); + constructorArguments.Add(interceptorsField); + var selector = emitter.GetField("__selector"); + if (selector != null) + { + constructorArguments.Add(selector); + } + + GenerateConstructors(emitter, targetType, constructorArguments.ToArray()); + GenerateParameterlessConstructor(emitter, targetType, interceptorsField); + + // Complete type initializer code body + CompleteInitCacheMethod(cctor.CodeBuilder); + + // Crosses fingers and build type + + var proxyType = emitter.BuildType(); + InitializeStaticFields(proxyType); + return proxyType; + } + + private IEnumerable GetTypeImplementerMapping(out IEnumerable contributors, INamingScope namingScope) + { + var methodsToSkip = new List(); + var proxyInstance = GetProxyInstanceContributor(methodsToSkip); + // TODO: the trick with methodsToSkip is not very nice... + var proxyTarget = GetProxyTargetContributor(methodsToSkip, namingScope); + IDictionary typeImplementerMapping = new Dictionary(); + + // Order of interface precedence: + // 1. first target + // target is not an interface so we do nothing + + var targetInterfaces = targetType.GetAllInterfaces(); + // 2. then mixins + var mixins = new MixinContributor(namingScope, false) { Logger = Logger }; + if (ProxyGenerationOptions.HasMixins) + { + foreach (var mixinInterface in ProxyGenerationOptions.MixinData.MixinInterfaces) + { + if (targetInterfaces.Contains(mixinInterface)) + { + // OK, so the target implements this interface. We now do one of two things: + if (interfaces.Contains(mixinInterface) && + typeImplementerMapping.ContainsKey(mixinInterface) == false) + { + AddMappingNoCheck(mixinInterface, proxyTarget, typeImplementerMapping); + proxyTarget.AddInterfaceToProxy(mixinInterface); + } + // we do not intercept the interface + mixins.AddEmptyInterface(mixinInterface); + } + else + { + if (!typeImplementerMapping.ContainsKey(mixinInterface)) + { + mixins.AddInterfaceToProxy(mixinInterface); + AddMappingNoCheck(mixinInterface, mixins, typeImplementerMapping); + } + } + } + } + var additionalInterfacesContributor = new InterfaceProxyWithoutTargetContributor(namingScope, + (c, m) => NullExpression.Instance) + { Logger = Logger }; + // 3. then additional interfaces + foreach (var @interface in interfaces) + { + if (targetInterfaces.Contains(@interface)) + { + if (typeImplementerMapping.ContainsKey(@interface)) + { + continue; + } + + // we intercept the interface, and forward calls to the target type + AddMappingNoCheck(@interface, proxyTarget, typeImplementerMapping); + proxyTarget.AddInterfaceToProxy(@interface); + } + else if (ProxyGenerationOptions.MixinData.ContainsMixin(@interface) == false) + { + additionalInterfacesContributor.AddInterfaceToProxy(@interface); + AddMapping(@interface, additionalInterfacesContributor, typeImplementerMapping); + } + } + // 4. plus special interfaces +#if FEATURE_SERIALIZATION + if (targetType.IsSerializable) + { + AddMappingForISerializable(typeImplementerMapping, proxyInstance); + } +#endif + try + { + AddMappingNoCheck(typeof(IProxyTargetAccessor), proxyInstance, typeImplementerMapping); + } + catch (ArgumentException) + { + HandleExplicitlyPassedProxyTargetAccessor(targetInterfaces); + } + + contributors = new List + { + proxyTarget, + mixins, + additionalInterfacesContributor, + proxyInstance + }; + return typeImplementerMapping.Keys; + } + + private void EnsureDoesNotImplementIProxyTargetAccessor(Type type, string name) + { + if (!typeof(IProxyTargetAccessor).IsAssignableFrom(type)) + { + return; + } + var message = + string.Format( + "Target type for the proxy implements {0} which is a DynamicProxy infrastructure interface and you should never implement it yourself. Are you trying to proxy an existing proxy?", + typeof(IProxyTargetAccessor)); + throw new ArgumentException(message, name); + } + } +} diff --git a/src/Castle.Core/DynamicProxy/Generators/ClassProxyGenerator.cs b/src/Castle.Core/DynamicProxy/Generators/ClassProxyGenerator.cs index 1dd9f9d286..886c06a844 100644 --- a/src/Castle.Core/DynamicProxy/Generators/ClassProxyGenerator.cs +++ b/src/Castle.Core/DynamicProxy/Generators/ClassProxyGenerator.cs @@ -16,185 +16,34 @@ namespace Castle.DynamicProxy.Generators { using System; using System.Collections.Generic; - using System.Linq; using System.Reflection; using Castle.DynamicProxy.Contributors; - using Castle.DynamicProxy.Generators.Emitters; using Castle.DynamicProxy.Generators.Emitters.SimpleAST; - using Castle.DynamicProxy.Internal; using Castle.DynamicProxy.Serialization; - internal class ClassProxyGenerator : BaseProxyGenerator + internal sealed class ClassProxyGenerator : BaseClassProxyGenerator { public ClassProxyGenerator(ModuleScope scope, Type targetType, Type[] interfaces, ProxyGenerationOptions options) : base(scope, targetType, interfaces, options) { - EnsureDoesNotImplementIProxyTargetAccessor(targetType, "targetType"); } + protected override FieldReference TargetField => null; + protected override CacheKey GetCacheKey() { return new CacheKey(targetType, interfaces, ProxyGenerationOptions); } - protected override Type GenerateType(string name, INamingScope namingScope) + protected override ProxyInstanceContributor GetProxyInstanceContributor(List methodsToSkip) { - IEnumerable contributors; - var allInterfaces = GetTypeImplementerMapping(out contributors, namingScope); - - var model = new MetaType(); - // Collect methods - foreach (var contributor in contributors) - { - contributor.CollectElementsToProxy(ProxyGenerationOptions.Hook, model); - } - ProxyGenerationOptions.Hook.MethodsInspected(); - - var emitter = BuildClassEmitter(name, targetType, allInterfaces); - - CreateFields(emitter); - CreateTypeAttributes(emitter); - - // Constructor - var cctor = GenerateStaticConstructor(emitter); - - var constructorArguments = new List(); - foreach (var contributor in contributors) - { - contributor.Generate(emitter); - - // TODO: redo it - var mixinContributor = contributor as MixinContributor; - if (mixinContributor != null) - { - constructorArguments.AddRange(mixinContributor.Fields); - } - } - - // constructor arguments - var interceptorsField = emitter.GetField("__interceptors"); - constructorArguments.Add(interceptorsField); - var selector = emitter.GetField("__selector"); - if (selector != null) - { - constructorArguments.Add(selector); - } - - GenerateConstructors(emitter, targetType, constructorArguments.ToArray()); - GenerateParameterlessConstructor(emitter, targetType, interceptorsField); - - // Complete type initializer code body - CompleteInitCacheMethod(cctor.CodeBuilder); - - // Crosses fingers and build type - - var proxyType = emitter.BuildType(); - InitializeStaticFields(proxyType); - return proxyType; - } - - protected virtual IEnumerable GetTypeImplementerMapping(out IEnumerable contributors, - INamingScope namingScope) - { - var methodsToSkip = new List(); - var proxyInstance = new ClassProxyInstanceContributor(targetType, methodsToSkip, interfaces, ProxyTypeConstants.Class); - // TODO: the trick with methodsToSkip is not very nice... - var proxyTarget = new ClassProxyTargetContributor(targetType, methodsToSkip, namingScope) { Logger = Logger }; - IDictionary typeImplementerMapping = new Dictionary(); - - // Order of interface precedence: - // 1. first target - // target is not an interface so we do nothing - - var targetInterfaces = targetType.GetAllInterfaces(); - // 2. then mixins - var mixins = new MixinContributor(namingScope, false) { Logger = Logger }; - if (ProxyGenerationOptions.HasMixins) - { - foreach (var mixinInterface in ProxyGenerationOptions.MixinData.MixinInterfaces) - { - if (targetInterfaces.Contains(mixinInterface)) - { - // OK, so the target implements this interface. We now do one of two things: - if (interfaces.Contains(mixinInterface) && typeImplementerMapping.ContainsKey(mixinInterface) == false) - { - AddMappingNoCheck(mixinInterface, proxyTarget, typeImplementerMapping); - proxyTarget.AddInterfaceToProxy(mixinInterface); - } - // we do not intercept the interface - mixins.AddEmptyInterface(mixinInterface); - } - else - { - if (!typeImplementerMapping.ContainsKey(mixinInterface)) - { - mixins.AddInterfaceToProxy(mixinInterface); - AddMappingNoCheck(mixinInterface, mixins, typeImplementerMapping); - } - } - } - } - var additionalInterfacesContributor = new InterfaceProxyWithoutTargetContributor(namingScope, - static (c, m) => NullExpression.Instance) - { Logger = Logger }; - // 3. then additional interfaces - foreach (var @interface in interfaces) - { - if (targetInterfaces.Contains(@interface)) - { - if (typeImplementerMapping.ContainsKey(@interface)) - { - continue; - } - - // we intercept the interface, and forward calls to the target type - AddMappingNoCheck(@interface, proxyTarget, typeImplementerMapping); - proxyTarget.AddInterfaceToProxy(@interface); - } - else if (ProxyGenerationOptions.MixinData.ContainsMixin(@interface) == false) - { - additionalInterfacesContributor.AddInterfaceToProxy(@interface); - AddMapping(@interface, additionalInterfacesContributor, typeImplementerMapping); - } - } - // 4. plus special interfaces -#if FEATURE_SERIALIZATION - if (targetType.IsSerializable) - { - AddMappingForISerializable(typeImplementerMapping, proxyInstance); - } -#endif - try - { - AddMappingNoCheck(typeof(IProxyTargetAccessor), proxyInstance, typeImplementerMapping); - } - catch (ArgumentException) - { - HandleExplicitlyPassedProxyTargetAccessor(targetInterfaces); - } - - contributors = new List - { - proxyTarget, - mixins, - additionalInterfacesContributor, - proxyInstance - }; - return typeImplementerMapping.Keys; + return new ClassProxyInstanceContributor(targetType, methodsToSkip, interfaces, ProxyTypeConstants.Class); } - private void EnsureDoesNotImplementIProxyTargetAccessor(Type type, string name) + protected override CompositeTypeContributor GetProxyTargetContributor(List methodsToSkip, INamingScope namingScope) { - if (!typeof(IProxyTargetAccessor).IsAssignableFrom(type)) - { - return; - } - var message = - string.Format( - "Target type for the proxy implements {0} which is a DynamicProxy infrastructure interface and you should never implement it yourself. Are you trying to proxy an existing proxy?", - typeof(IProxyTargetAccessor)); - throw new ArgumentException(message, name); + return new ClassProxyTargetContributor(targetType, methodsToSkip, namingScope) { Logger = Logger }; } } -} \ No newline at end of file +} diff --git a/src/Castle.Core/DynamicProxy/Generators/ClassProxyWithTargetGenerator.cs b/src/Castle.Core/DynamicProxy/Generators/ClassProxyWithTargetGenerator.cs index fa5f51baf7..e4c82645d6 100644 --- a/src/Castle.Core/DynamicProxy/Generators/ClassProxyWithTargetGenerator.cs +++ b/src/Castle.Core/DynamicProxy/Generators/ClassProxyWithTargetGenerator.cs @@ -16,8 +16,8 @@ namespace Castle.DynamicProxy.Generators { using System; using System.Collections.Generic; - using System.Linq; using System.Reflection; + #if FEATURE_SERIALIZATION using System.Xml.Serialization; #endif @@ -25,193 +25,47 @@ namespace Castle.DynamicProxy.Generators using Castle.DynamicProxy.Contributors; using Castle.DynamicProxy.Generators.Emitters; using Castle.DynamicProxy.Generators.Emitters.SimpleAST; - using Castle.DynamicProxy.Internal; using Castle.DynamicProxy.Serialization; - internal class ClassProxyWithTargetGenerator : BaseProxyGenerator + internal sealed class ClassProxyWithTargetGenerator : BaseClassProxyGenerator { + private FieldReference targetField; + public ClassProxyWithTargetGenerator(ModuleScope scope, Type targetType, Type[] interfaces, ProxyGenerationOptions options) : base(scope, targetType, interfaces, options) { - EnsureDoesNotImplementIProxyTargetAccessor(targetType, "targetType"); } - protected virtual IEnumerable GetTypeImplementerMapping(out IEnumerable contributors, - INamingScope namingScope) - { - var methodsToSkip = new List(); - var proxyInstance = new ClassProxyWithTargetInstanceContributor(targetType, methodsToSkip, interfaces, - ProxyTypeConstants.ClassWithTarget); - // TODO: the trick with methodsToSkip is not very nice... - var proxyTarget = new ClassProxyWithTargetTargetContributor(targetType, methodsToSkip, namingScope) - { Logger = Logger }; - IDictionary typeImplementerMapping = new Dictionary(); - - // Order of interface precedence: - // 1. first target - // target is not an interface so we do nothing - - var targetInterfaces = targetType.GetAllInterfaces(); - // 2. then mixins - var mixins = new MixinContributor(namingScope, false) { Logger = Logger }; - if (ProxyGenerationOptions.HasMixins) - { - foreach (var mixinInterface in ProxyGenerationOptions.MixinData.MixinInterfaces) - { - if (targetInterfaces.Contains(mixinInterface)) - { - // OK, so the target implements this interface. We now do one of two things: - if (interfaces.Contains(mixinInterface) && - typeImplementerMapping.ContainsKey(mixinInterface) == false) - { - AddMappingNoCheck(mixinInterface, proxyTarget, typeImplementerMapping); - proxyTarget.AddInterfaceToProxy(mixinInterface); - } - // we do not intercept the interface - mixins.AddEmptyInterface(mixinInterface); - } - else - { - if (!typeImplementerMapping.ContainsKey(mixinInterface)) - { - mixins.AddInterfaceToProxy(mixinInterface); - AddMappingNoCheck(mixinInterface, mixins, typeImplementerMapping); - } - } - } - } - var additionalInterfacesContributor = new InterfaceProxyWithoutTargetContributor(namingScope, - (c, m) => NullExpression.Instance) - { Logger = Logger }; - // 3. then additional interfaces - foreach (var @interface in interfaces) - { - if (targetInterfaces.Contains(@interface)) - { - if (typeImplementerMapping.ContainsKey(@interface)) - { - continue; - } - - // we intercept the interface, and forward calls to the target type - AddMappingNoCheck(@interface, proxyTarget, typeImplementerMapping); - proxyTarget.AddInterfaceToProxy(@interface); - } - else if (ProxyGenerationOptions.MixinData.ContainsMixin(@interface) == false) - { - additionalInterfacesContributor.AddInterfaceToProxy(@interface); - AddMapping(@interface, additionalInterfacesContributor, typeImplementerMapping); - } - } - // 4. plus special interfaces -#if FEATURE_SERIALIZATION - if (targetType.IsSerializable) - { - AddMappingForISerializable(typeImplementerMapping, proxyInstance); - } -#endif - try - { - AddMappingNoCheck(typeof(IProxyTargetAccessor), proxyInstance, typeImplementerMapping); - } - catch (ArgumentException) - { - HandleExplicitlyPassedProxyTargetAccessor(targetInterfaces); - } + protected override FieldReference TargetField => targetField; - contributors = new List - { - proxyTarget, - mixins, - additionalInterfacesContributor, - proxyInstance - }; - return typeImplementerMapping.Keys; + protected override CacheKey GetCacheKey() + { + return new CacheKey(targetType, targetType, interfaces, ProxyGenerationOptions); } - private FieldReference CreateTargetField(ClassEmitter emitter) + protected override void CreateFields(ClassEmitter emitter) { - var targetField = emitter.CreateField("__target", targetType); -#if FEATURE_SERIALIZATION - emitter.DefineCustomAttributeFor(targetField); -#endif - return targetField; + base.CreateFields(emitter); + CreateTargetField(emitter); } - private void EnsureDoesNotImplementIProxyTargetAccessor(Type type, string name) + protected override ProxyInstanceContributor GetProxyInstanceContributor(List methodsToSkip) { - if (!typeof(IProxyTargetAccessor).IsAssignableFrom(type)) - { - return; - } - var message = - string.Format( - "Target type for the proxy implements {0} which is a DynamicProxy infrastructure interface and you should never implement it yourself. Are you trying to proxy an existing proxy?", - typeof(IProxyTargetAccessor)); - throw new ArgumentException(message, name); + return new ClassProxyWithTargetInstanceContributor(targetType, methodsToSkip, interfaces, ProxyTypeConstants.ClassWithTarget); } - protected override CacheKey GetCacheKey() + protected override CompositeTypeContributor GetProxyTargetContributor(List methodsToSkip, INamingScope namingScope) { - return new CacheKey(targetType, targetType, interfaces, ProxyGenerationOptions); + return new ClassProxyWithTargetTargetContributor(targetType, methodsToSkip, namingScope) { Logger = Logger }; } - protected override Type GenerateType(string name, INamingScope namingScope) + private void CreateTargetField(ClassEmitter emitter) { - IEnumerable contributors; - var allInterfaces = GetTypeImplementerMapping(out contributors, namingScope); - - var model = new MetaType(); - // Collect methods - foreach (var contributor in contributors) - { - contributor.CollectElementsToProxy(ProxyGenerationOptions.Hook, model); - } - ProxyGenerationOptions.Hook.MethodsInspected(); - - var emitter = BuildClassEmitter(name, targetType, allInterfaces); - - CreateFields(emitter); - CreateTypeAttributes(emitter); - - // Constructor - var cctor = GenerateStaticConstructor(emitter); - - var targetField = CreateTargetField(emitter); - var constructorArguments = new List { targetField }; - - foreach (var contributor in contributors) - { - contributor.Generate(emitter); - - // TODO: redo it - if (contributor is MixinContributor) - { - constructorArguments.AddRange((contributor as MixinContributor).Fields); - } - } - - // constructor arguments - var interceptorsField = emitter.GetField("__interceptors"); - constructorArguments.Add(interceptorsField); - var selector = emitter.GetField("__selector"); - if (selector != null) - { - constructorArguments.Add(selector); - } - - GenerateConstructors(emitter, targetType, constructorArguments.ToArray()); - GenerateParameterlessConstructor(emitter, targetType, interceptorsField); - - // Complete type initializer code body - CompleteInitCacheMethod(cctor.CodeBuilder); - - // Crosses fingers and build type - - var proxyType = emitter.BuildType(); - InitializeStaticFields(proxyType); - return proxyType; + targetField = emitter.CreateField("__target", targetType); +#if FEATURE_SERIALIZATION + emitter.DefineCustomAttributeFor(targetField); +#endif } } -} \ No newline at end of file +}