From 3a29898a9312cf4cd9b6cb71e83f06696e7b5aeb Mon Sep 17 00:00:00 2001 From: leekelleher Date: Mon, 8 Jan 2018 18:25:15 +0000 Subject: [PATCH 1/2] Proposal of a "type-cache" By moving all the object-type analysis upfront, and caching it. This will reduce the amount of time - and CPU cycles and memory allocations - needed for each object mapping. --- .../Common/DittoTypeConfig.cs | 250 ++++++++++++++++ .../Common/DittoTypeConfigCache.cs | 46 +++ .../Internal/PropertyInfoExtensions.cs | 17 +- .../Extensions/PublishedContentExtensions.cs | 283 ++++++------------ .../Our.Umbraco.Ditto.csproj | 4 +- .../PublishedContentMapping.cs | 3 + .../DefaultProcessorTests.cs | 3 + .../PropertySourceMappingTests.cs | 12 +- 8 files changed, 412 insertions(+), 206 deletions(-) create mode 100644 src/Our.Umbraco.Ditto/Common/DittoTypeConfig.cs create mode 100644 src/Our.Umbraco.Ditto/Common/DittoTypeConfigCache.cs diff --git a/src/Our.Umbraco.Ditto/Common/DittoTypeConfig.cs b/src/Our.Umbraco.Ditto/Common/DittoTypeConfig.cs new file mode 100644 index 0000000..8bdf6c7 --- /dev/null +++ b/src/Our.Umbraco.Ditto/Common/DittoTypeConfig.cs @@ -0,0 +1,250 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using Umbraco.Core.Models; + +namespace Our.Umbraco.Ditto +{ + internal sealed class DittoTypeConfig + { + public static DittoTypeConfig Create() + { + return Create(typeof(T)); + } + + public static DittoTypeConfig Create(Type type) + { + var config = new DittoTypeConfig + { + TargetType = type + }; + + // constructor + // + // Check the validity of the mapped type constructor as early as possible. + var constructorParams = type.GetConstructorParameters(); + if (constructorParams != null) + { + // Is it a PublishedContent or similar? + if (constructorParams.Length == 1 && constructorParams[0].ParameterType == typeof(IPublishedContent)) + { + config.ConstructorHasPublishedContentParameter = true; + } + + if (constructorParams.Length == 0 || config.ConstructorHasPublishedContentParameter) + { + config.ConstructorIsValid = true; + } + } + + // No valid constructor, but see if the value can be cast to the type + if (type.IsAssignableFrom(typeof(IPublishedContent))) + { + config.IsOfTypePublishedContent = true; + config.ConstructorIsValid = true; + } + + // attributes + // + config.CustomAttributes = type.GetCustomAttributes(); + + // cacheable + // + var conversionHandlers = new List(); + + foreach (var attr in config.CustomAttributes) + { + if (attr is DittoCacheAttribute) + { + config.IsCacheable = true; + config.CacheInfo = (DittoCacheAttribute)attr; + } + + // Check for class level DittoConversionHandlerAttribute + if (attr is DittoConversionHandlerAttribute) + { + conversionHandlers.Add(((DittoConversionHandlerAttribute)attr).HandlerType.GetInstance()); + } + } + + // properties (lazy & eager) + // + var lazyProperties = new List(); + var lazyPropertyNames = new List(); + var eagerProperties = new List(); + + // Collect all the properties of the given type and loop through writable ones. + foreach (var property in type.GetProperties(BindingFlags.Public | BindingFlags.Instance)) + { + if (property.CanWrite == false) + continue; + + if (property.GetSetMethod() == null) + continue; + + var attributes = new List(property.GetCustomAttributes()); + + if (attributes.Any(x => x is DittoIgnoreAttribute)) + continue; + + var propertyType = property.PropertyType; + + var propertyConfig = new DittoTypePropertyConfig + { + CustomAttributes = attributes, + PropertyInfo = property, + }; + + + // Check the property for any explicit processor attributes + var processors = attributes.Where(x => x is DittoProcessorAttribute).Cast().ToList(); + if (processors.Count == 0) + { + // Adds the default processor for this conversion + var defaultProcessor = DittoProcessorRegistry.Instance.GetDefaultProcessorFor(type); + // Forces the default processor to be the very first processor + defaultProcessor.Order = -1; + processors.Add(defaultProcessor); + } + + // Check for registered processors on the property's type + processors.AddRange(propertyType.GetCustomAttributes(true)); + + // Check any type arguments in generic enumerable types. + // This should return false against typeof(string) etc also. + if (propertyType.IsCastableEnumerableType()) + { + propertyConfig.IsEnumerable = true; + propertyConfig.EnumerableType = propertyType.GenericTypeArguments[0]; + + processors.AddRange(propertyConfig.EnumerableType.GetCustomAttributes(true)); + } + + // Sort the order of the processors + processors.Sort((x, y) => x.Order.CompareTo(y.Order)); + + // Check for globally registered processors + processors.AddRange(DittoProcessorRegistry.Instance.GetRegisteredProcessorAttributesFor(propertyType)); + + // Add any core processors onto the end + processors.AddRange(DittoProcessorRegistry.Instance.GetPostProcessorAttributes()); + + propertyConfig.Processors = processors; + + + var propertyCache = attributes.Where(x => x is DittoCacheAttribute).Cast().FirstOrDefault(); + if (propertyCache != null) + { + propertyConfig.IsCacheable = true; + propertyConfig.CacheInfo = propertyCache; + } + + // detect if the property should be lazy-loaded + if (property.ShouldAttemptLazyLoad()) + { + lazyProperties.Add(propertyConfig); + lazyPropertyNames.Add(property.Name); + } + else + { + eagerProperties.Add(propertyConfig); + } + } + + if (lazyProperties.Count > 0) + { + config.ConstructorRequiresProxyType = true; + config.HasLazyProperties = true; + config.LazyPropertyNames = lazyPropertyNames; // lazyProperties.Select(x => x.PropertyInfo.Name); + config.LazyProperties = lazyProperties; + } + + if (eagerProperties.Count > 0) + { + config.HasEagerProperties = true; + config.EagerProperties = eagerProperties; + } + + + + // conversion handlers + // + + // Check for globaly registered handlers + foreach (var handlerType in DittoConversionHandlerRegistry.Instance.GetRegisteredHandlerTypesFor(type)) + { + conversionHandlers.Add(handlerType.GetInstance()); + } + + config.ConversionHandlers = conversionHandlers; + + var before = new List(); + var after = new List(); + + // Check for method level DittoOnConvert[ing|ed]Attribute + foreach (var method in type.GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)) + { + var ing = method.GetCustomAttribute(); + var ed = method.GetCustomAttribute(); + + if (ing == null && ed == null) + continue; + + var p = method.GetParameters(); + if (p.Length == 1 && p[0].ParameterType == typeof(DittoConversionHandlerContext)) + { + if (ing != null) + before.Add(method); + + if (ed != null) + after.Add(method); + } + } + + config.ConvertingMethods = before; + config.ConvertedMethods = after; + + + return config; + } + + public Type TargetType { get; set; } + + public bool IsCacheable { get; set; } + public DittoCacheAttribute CacheInfo { get; set; } + + public bool ConstructorIsValid { get; set; } + public bool ConstructorHasPublishedContentParameter { get; set; } + public bool ConstructorRequiresProxyType { get; set; } + + public bool IsOfTypePublishedContent { get; set; } + + public IEnumerable CustomAttributes { get; set; } + + public bool HasLazyProperties { get; set; } + public IEnumerable LazyProperties { get; set; } + public IEnumerable LazyPropertyNames { get; set; } + + public bool HasEagerProperties { get; set; } + public IEnumerable EagerProperties { get; set; } + + public IEnumerable ConversionHandlers { get; set; } + public IEnumerable ConvertingMethods { get; set; } + public IEnumerable ConvertedMethods { get; set; } + + internal sealed class DittoTypePropertyConfig + { + public bool IsCacheable { get; set; } + public DittoCacheAttribute CacheInfo { get; set; } + + public IEnumerable CustomAttributes { get; set; } + + public IEnumerable Processors { get; set; } + public PropertyInfo PropertyInfo { get; set; } + + public bool IsEnumerable { get; set; } + public Type EnumerableType { get; set; } + } + } +} \ No newline at end of file diff --git a/src/Our.Umbraco.Ditto/Common/DittoTypeConfigCache.cs b/src/Our.Umbraco.Ditto/Common/DittoTypeConfigCache.cs new file mode 100644 index 0000000..343ccc3 --- /dev/null +++ b/src/Our.Umbraco.Ditto/Common/DittoTypeConfigCache.cs @@ -0,0 +1,46 @@ +using System; +using System.Collections.Concurrent; + +namespace Our.Umbraco.Ditto +{ + internal static class DittoTypeConfigCache + { + private static readonly ConcurrentDictionary _cache = new ConcurrentDictionary(); + + public static void Add() + { + Add(typeof(T)); + } + + public static void Add(Type type) + { + _cache.TryAdd(type, DittoTypeConfig.Create(type)); + } + + public static void Clear() + { + Clear(typeof(T)); + } + + public static void Clear(Type type) + { + _cache.TryRemove(type, out DittoTypeConfig config); + } + + public static DittoTypeConfig GetOrAdd() + { + return GetOrAdd(typeof(T)); + } + + public static DittoTypeConfig GetOrAdd(Type type) + { + if (_cache.TryGetValue(type, out DittoTypeConfig config) == false) + { + config = DittoTypeConfig.Create(type); + _cache.TryAdd(type, config); + } + + return config; + } + } +} \ No newline at end of file diff --git a/src/Our.Umbraco.Ditto/Extensions/Internal/PropertyInfoExtensions.cs b/src/Our.Umbraco.Ditto/Extensions/Internal/PropertyInfoExtensions.cs index 154452a..f36afd2 100644 --- a/src/Our.Umbraco.Ditto/Extensions/Internal/PropertyInfoExtensions.cs +++ b/src/Our.Umbraco.Ditto/Extensions/Internal/PropertyInfoExtensions.cs @@ -72,14 +72,21 @@ public static bool IsMappable(this PropertyInfo source) /// True if a lazy load attempt should be make public static bool ShouldAttemptLazyLoad(this PropertyInfo source) { - if (source.HasCustomAttribute()) + var hasPropertyAttribute = source.HasCustomAttribute(); + var hasClassAttribute = source.DeclaringType.HasCustomAttribute(); + var isVirtualAndOverridable = source.IsVirtualAndOverridable(); + + if (isVirtualAndOverridable && (hasPropertyAttribute || hasClassAttribute || Ditto.LazyLoadStrategy == LazyLoad.AllVirtuals)) { - return false; + return true; + } + else if (isVirtualAndOverridable == false && hasPropertyAttribute) + { + // Ensure it's a virtual property (Only relevant to property level lazy loads) + throw new InvalidOperationException($"Lazy property '{source.Name}' of type '{source.DeclaringType.AssemblyQualifiedName}' must be declared virtual in order to be lazy loadable."); } - return source.HasCustomAttribute() || - ((source.DeclaringType.HasCustomAttribute() || Ditto.LazyLoadStrategy == LazyLoad.AllVirtuals) - && source.IsVirtualAndOverridable()); + return false; } } } \ No newline at end of file diff --git a/src/Our.Umbraco.Ditto/Extensions/PublishedContentExtensions.cs b/src/Our.Umbraco.Ditto/Extensions/PublishedContentExtensions.cs index ec5a523..75c827a 100644 --- a/src/Our.Umbraco.Ditto/Extensions/PublishedContentExtensions.cs +++ b/src/Our.Umbraco.Ditto/Extensions/PublishedContentExtensions.cs @@ -1,12 +1,8 @@ using System; using System.Collections; -using System.Collections.Concurrent; using System.Collections.Generic; -using System.ComponentModel; using System.Globalization; using System.Linq; -using System.Reflection; -using Umbraco.Core; using Umbraco.Core.Models; namespace Our.Umbraco.Ditto @@ -21,12 +17,6 @@ public static class PublishedContentExtensions /// private static readonly IDittoContextAccessor ContextAccessor = Ditto.GetContextAccessor(); - /// - /// The cache for storing type property information. - /// - private static readonly ConcurrentDictionary PropertyCache - = new ConcurrentDictionary(); - /// Returns the given instance of as the specified type. /// The to convert. /// The @@ -148,21 +138,23 @@ public static object As( // Convert using (DittoDisposableTimer.DebugDuration(typeof(Ditto), $"As<{type.Name}>({content.DocumentTypeAlias} {content.Id})")) { - if (Ditto.TryGetTypeAttribute(type, out DittoCacheAttribute cacheAttr)) + var config = DittoTypeConfigCache.GetOrAdd(type); + + if (config.IsCacheable) { - var ctx = new DittoCacheContext(cacheAttr, content, type, culture); - return cacheAttr.GetCacheItem(ctx, () => ConvertContent(content, type, culture, instance, onConverting, onConverted, chainContext)); + var ctx = new DittoCacheContext(config.CacheInfo, content, type, culture); + return config.CacheInfo.GetCacheItem(ctx, () => ConvertContent(content, config, culture, instance, onConverting, onConverted, chainContext)); } else { - return ConvertContent(content, type, culture, instance, onConverting, onConverted, chainContext); + return ConvertContent(content, config, culture, instance, onConverting, onConverted, chainContext); } } } /// Returns an object representing the given . - /// The to convert. - /// The of items to return. + /// The to convert. + /// The Ditto configuration for the type. /// The /// An existing instance of T to populate /// The to fire when converting. @@ -172,78 +164,34 @@ public static object As( /// Thrown if the given type has invalid constructors. private static object ConvertContent( IPublishedContent content, - Type type, + DittoTypeConfig config, CultureInfo culture, object instance, Action onConverting, Action onConverted, DittoChainContext chainContext) { - // Collect all the properties of the given type and loop through writable ones. - PropertyCache.TryGetValue(type, out PropertyInfo[] properties); - - if (properties == null) - { - properties = type.GetProperties(BindingFlags.Public | BindingFlags.Instance) - .Where(x => x.CanWrite && x.GetSetMethod() != null).ToArray(); - - PropertyCache.TryAdd(type, properties); - } - - // Check the validity of the mpped type constructor as early as possible. - ParameterInfo[] constructorParams = type.GetConstructorParameters(); - bool validConstructor = false; - bool hasParameter = false; - bool isType = false; - bool hasLazy = false; - - if (constructorParams != null) + // If not already an instance, create an instance of the object. + if (instance == null) { - // Is it PublishedContentmModel or similar? - if (constructorParams.Length == 1 && constructorParams[0].ParameterType == typeof(IPublishedContent)) + // Check the validity of the mapped type constructor. + if (config.ConstructorIsValid == false) { - hasParameter = true; + throw new InvalidOperationException( + $"Cannot convert IPublishedContent to {config.TargetType} as it has no valid constructor. " + + "A valid constructor is either empty, or one accepting a single IPublishedContent parameter."); } - if (constructorParams.Length == 0 || hasParameter) - { - validConstructor = true; - } - } - - // No valid constructor, but see if the value can be cast to the type - if (type.IsInstanceOfType(content)) - { - isType = true; - validConstructor = true; - } - - if (validConstructor == false) - { - throw new InvalidOperationException( - $"Cannot convert IPublishedContent to {type} as it has no valid constructor. " + - "A valid constructor is either an empty one, or one accepting a single IPublishedContent parameter."); - } - - PropertyInfo[] lazyProperties = null; - - // If not already an instance, create an instance of the object - if (instance == null) - { // We can only proxy new instances. - lazyProperties = properties.Where(x => x.ShouldAttemptLazyLoad()).ToArray(); - - if (lazyProperties.Any()) + if (config.ConstructorRequiresProxyType) { - hasLazy = true; - var factory = new ProxyFactory(); - instance = hasParameter - ? factory.CreateProxy(type, lazyProperties.Select(x => x.Name), content) - : factory.CreateProxy(type, lazyProperties.Select(x => x.Name)); + instance = config.ConstructorHasPublishedContentParameter + ? factory.CreateProxy(config.TargetType, config.LazyPropertyNames, content) + : factory.CreateProxy(config.TargetType, config.LazyPropertyNames); } - else if (isType) + else if (config.IsOfTypePublishedContent) { instance = content; } @@ -251,163 +199,109 @@ private static object ConvertContent( { // 1: This extension method is about 7x faster than the native implementation. // 2: Internally this uses Activator.CreateInstance which is heavily optimized. - instance = hasParameter - ? type.GetInstance(content) // 1 - : type.GetInstance(); // 2 + instance = config.ConstructorHasPublishedContentParameter + // TODO: Review this, as we could get the Constructor metadata from the "type-cache"? + ? config.TargetType.GetInstance(content) // 1 + : config.TargetType.GetInstance(); // 2 } } // We have the instance object but haven't yet populated properties // so fire the on converting event handlers - OnConverting(content, type, culture, instance, onConverting); + OnConverting(content, config, culture, instance, onConverting); - if (hasLazy) + if (config.HasLazyProperties) { // A dictionary to store lazily invoked values. var lazyMappings = new Dictionary>(); - foreach (var propertyInfo in lazyProperties) + foreach (var lazyProperty in config.LazyProperties) { // Configure lazy properties - using (DittoDisposableTimer.DebugDuration(typeof(Ditto), $"Lazy Property ({content.Id} {propertyInfo.Name})")) - { - // Ensure it's a virtual property (Only relevant to property level lazy loads) - if (propertyInfo.IsVirtualAndOverridable() == false) - { - throw new InvalidOperationException($"Lazy property '{propertyInfo.Name}' of type '{type.AssemblyQualifiedName}' must be declared virtual in order to be lazy loadable."); - } - - lazyMappings.Add(propertyInfo.Name, new Lazy(() => GetProcessedValue(content, culture, type, propertyInfo, instance, chainContext))); - } + lazyMappings.Add(lazyProperty.PropertyInfo.Name, new Lazy(() => GetProcessedValue(content, culture, config, lazyProperty, instance, chainContext))); } ((IProxy)instance).Interceptor = new LazyInterceptor(lazyMappings); } - // Process any non lazy properties - foreach (var propertyInfo in properties.Where(x => x.ShouldAttemptLazyLoad() == false)) + // Process any eager properties + if (config.HasEagerProperties) { - // Check for the ignore attribute. - if (propertyInfo.HasCustomAttribute()) + foreach (var eagerProperty in config.EagerProperties) { - continue; - } + // Set the value normally. + var value = GetProcessedValue(content, culture, config, eagerProperty, instance, chainContext); - // Set the value normally. - var value = GetProcessedValue(content, culture, type, propertyInfo, instance, chainContext); - - // This over 4x faster as propertyInfo.SetValue(instance, value, null); - FastPropertyAccessor.SetValue(propertyInfo, instance, value); + // This over 4x faster as propertyInfo.SetValue(instance, value, null); + FastPropertyAccessor.SetValue(eagerProperty.PropertyInfo, instance, value); + } } // We have now finished populating the instance object so go ahead // and fire the on converted event handlers - OnConverted(content, type, culture, instance, onConverted); + OnConverted(content, config, culture, instance, onConverted); return instance; } /// Returns the processed value for the given type and property. /// The to convert. - /// The - /// The target type. - /// The property info associated with the type. + /// The + /// The Ditto configuration for the type. + /// Information about the mappable property. /// The instance to assign the value to. /// The for the current processor chain. /// The representing the Umbraco value. private static object GetProcessedValue( IPublishedContent content, CultureInfo culture, - Type targetType, - PropertyInfo propertyInfo, + DittoTypeConfig config, + DittoTypeConfig.DittoTypePropertyConfig mappableProperty, object instance, DittoChainContext chainContext) { - using (DittoDisposableTimer.DebugDuration(typeof(Ditto), $"Processing '{propertyInfo.Name}' ({content.Id})")) + using (DittoDisposableTimer.DebugDuration(typeof(Ditto), $"Processing '{mappableProperty.PropertyInfo.Name}' ({content.Id})")) { // Create a base processor context for this current chain level var baseProcessorContext = new DittoProcessorContext { Content = content, - TargetType = targetType, - PropertyInfo = propertyInfo, + TargetType = config.TargetType, + PropertyInfo = mappableProperty.PropertyInfo, Culture = culture }; // Check for cache attribute - var cacheAttr = propertyInfo.GetCustomAttribute(true); - if (cacheAttr != null) + if (mappableProperty.IsCacheable) { - var ctx = new DittoCacheContext(cacheAttr, content, targetType, propertyInfo, culture); - return cacheAttr.GetCacheItem(ctx, () => DoGetProcessedValue(content, propertyInfo, baseProcessorContext, chainContext)); + var ctx = new DittoCacheContext(mappableProperty.CacheInfo, content, config.TargetType, mappableProperty.PropertyInfo, culture); + return mappableProperty.CacheInfo.GetCacheItem(ctx, () => DoGetProcessedValue(content, mappableProperty, baseProcessorContext, chainContext)); } else { - return DoGetProcessedValue(content, propertyInfo, baseProcessorContext, chainContext); + return DoGetProcessedValue(content, mappableProperty, baseProcessorContext, chainContext); } } } /// Returns the processed value for the given type and property. - /// The content. - /// The property information. + /// The content. + /// Information about the mappable property. /// The base processor context. /// The for the current processor chain. /// Returns the processed value. private static object DoGetProcessedValue( IPublishedContent content, - PropertyInfo propertyInfo, + DittoTypeConfig.DittoTypePropertyConfig mappableProperty, DittoProcessorContext baseProcessorContext, DittoChainContext chainContext) { - // Check the property for any explicit processor attributes - var processorAttrs = propertyInfo.GetCustomAttributes(true) - .OrderBy(x => x.Order) - .ToList(); - - if (processorAttrs.Any() == false) - { - // Adds the default processor for this conversion - processorAttrs.Add(DittoProcessorRegistry.Instance.GetDefaultProcessorFor(baseProcessorContext.TargetType)); - } - - var propertyType = propertyInfo.PropertyType; - - // Check for type registered processors - processorAttrs.AddRange(propertyType - .GetCustomAttributes(true) - .OrderBy(x => x.Order)); - - // Check any type arguments in generic enumerable types. - // This should return false against typeof(string) etc also. - var typeInfo = propertyType.GetTypeInfo(); - bool isEnumerable = false; - Type typeArg = null; - if (propertyType.IsCastableEnumerableType()) - { - typeArg = typeInfo.GenericTypeArguments.First(); - processorAttrs.AddRange(typeInfo - .GenericTypeArguments - .First() - .GetCustomAttributes(true) - .OrderBy(x => x.Order) - .ToList()); - - isEnumerable = true; - } - - // Check for globally registered processors - processorAttrs.AddRange(DittoProcessorRegistry.Instance.GetRegisteredProcessorAttributesFor(propertyInfo.PropertyType)); - - // Add any core processors onto the end - processorAttrs.AddRange(DittoProcessorRegistry.Instance.GetPostProcessorAttributes()); - // Create holder for value as it's processed object currentValue = content; // Process attributes - foreach (var processorAttr in processorAttrs) + foreach (var processorAttr in mappableProperty.Processors) { - using (DittoDisposableTimer.DebugDuration(typeof(Ditto), $"Processor '{processorAttr.GetType().Name}' ({content.Id})")) + using (DittoDisposableTimer.DebugDuration(typeof(Ditto), $"Processor '{processorAttr}' ({content.Id})")) { // Get the right context type var ctx = chainContext.ProcessorContexts.GetOrCreate(baseProcessorContext, processorAttr.ContextType); @@ -422,35 +316,35 @@ private static object DoGetProcessedValue( } // The following has to happen after all the processors. - if (isEnumerable && currentValue != null && currentValue.Equals(Enumerable.Empty())) + if (mappableProperty.IsEnumerable && currentValue != null && currentValue.Equals(Enumerable.Empty())) { - if (propertyType.IsInterface) + if (mappableProperty.PropertyInfo.PropertyType.IsInterface) { // You cannot set an enumerable of type from an empty object array. - currentValue = EnumerableInvocations.Cast(typeArg, (IEnumerable)currentValue); + currentValue = EnumerableInvocations.Cast(mappableProperty.EnumerableType, (IEnumerable)currentValue); } else { // This should allow the casting back of IEnumerable to an empty List Collection etc. // I cant think of any that don't have an empty constructor - currentValue = propertyType.GetInstance(); + currentValue = mappableProperty.PropertyInfo.PropertyType.GetInstance(); } } - return (currentValue == null && propertyType.IsValueType) - ? propertyInfo.PropertyType.GetInstance() // Set to default instance of value type + return currentValue == null && mappableProperty.PropertyInfo.PropertyType.IsValueType + ? mappableProperty.PropertyInfo.PropertyType.GetInstance() // Set to default instance of value type : currentValue; } /// Fires off the various on converting events. - /// The to convert. - /// The instance type. + /// The to convert. + /// The Ditto configuration for the type. /// The culture. /// The instance to assign the value to. /// The to fire when converting. private static void OnConverting( IPublishedContent content, - Type type, + DittoTypeConfig config, CultureInfo culture, object instance, Action callback) @@ -458,21 +352,21 @@ private static void OnConverting( OnConvert( DittoConversionHandlerType.OnConverting, content, - type, + config, culture, instance, callback); } /// Fires off the various on converted events. - /// The to convert. - /// The instance type. + /// The to convert. + /// The Ditto configuration for the type. /// The culture. /// The instance to assign the value to. /// The to fire when converted. private static void OnConverted( IPublishedContent content, - Type type, + DittoTypeConfig config, CultureInfo culture, object instance, Action callback) @@ -480,7 +374,7 @@ private static void OnConverted( OnConvert( DittoConversionHandlerType.OnConverted, content, - type, + config, culture, instance, callback); @@ -489,15 +383,15 @@ private static void OnConverted( /// Convenience method for calling converting/converter handlers. /// The type of the attribute type. /// Type of the conversion. - /// The content. - /// The type. + /// The content. + /// The Ditto configuration for the type. /// The culture. /// The instance. /// The callback. private static void OnConvert( DittoConversionHandlerType conversionType, IPublishedContent content, - Type type, + DittoTypeConfig config, CultureInfo culture, object instance, Action callback) @@ -508,40 +402,33 @@ private static void OnConvert( { Content = content, Culture = culture, - ModelType = type, + ModelType = config.TargetType, Model = instance }; - // Check for class level DittoConversionHandlerAttribute - foreach (var attr in type.GetCustomAttributes()) + // Run the registered handlers + foreach (var handler in config.ConversionHandlers) { - ((DittoConversionHandler)attr.HandlerType.GetInstance()) - .Run(conversionCtx, conversionType); + handler.Run(conversionCtx, conversionType); } - // Check for globaly registered handlers - foreach (var handlerType in DittoConversionHandlerRegistry.Instance.GetRegisteredHandlerTypesFor(type)) - { - ((DittoConversionHandler)handlerType.GetInstance()) - .Run(conversionCtx, conversionType); - } + var methods = conversionType == DittoConversionHandlerType.OnConverting + ? config.ConvertingMethods + : config.ConvertedMethods; - // Check for method level DittoOnConvert[ing|ed]Attribute - foreach (var method in type.GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance) - .Where(x => x.GetCustomAttribute() != null)) + if (methods.Any()) { - var p = method.GetParameters(); - if (p.Length == 1 && p[0].ParameterType == typeof(DittoConversionHandlerContext)) + foreach (var method in methods) { + // TODO: Review this, `Invoke` could be CPU heavy?! method.Invoke(instance, new object[] { conversionCtx }); + // Could we use a RuntimeMethodHandle? + // https://web.archive.org/web/20150118044646/http://msdn.microsoft.com:80/en-us/magazine/cc163759.aspx#S8 } } // Check for a callback function - if (callback != null) - { - callback(conversionCtx); - } + callback?.Invoke(conversionCtx); } } } \ No newline at end of file diff --git a/src/Our.Umbraco.Ditto/Our.Umbraco.Ditto.csproj b/src/Our.Umbraco.Ditto/Our.Umbraco.Ditto.csproj index 31e3180..4f34c19 100644 --- a/src/Our.Umbraco.Ditto/Our.Umbraco.Ditto.csproj +++ b/src/Our.Umbraco.Ditto/Our.Umbraco.Ditto.csproj @@ -81,6 +81,8 @@ + + @@ -131,8 +133,8 @@ - + diff --git a/tests/Our.Umbraco.Ditto.PerformanceTests/PublishedContentMapping.cs b/tests/Our.Umbraco.Ditto.PerformanceTests/PublishedContentMapping.cs index 34150e3..2da64fc 100644 --- a/tests/Our.Umbraco.Ditto.PerformanceTests/PublishedContentMapping.cs +++ b/tests/Our.Umbraco.Ditto.PerformanceTests/PublishedContentMapping.cs @@ -48,6 +48,9 @@ public void Setup() Ditto.DeregisterPostProcessorType(); Ditto.DeregisterPostProcessorType(); Ditto.DeregisterPostProcessorType(); + + // pre-load the type config + DittoTypeConfigCache.Add(); } [Benchmark(Baseline = true)] diff --git a/tests/Our.Umbraco.Ditto.Tests/DefaultProcessorTests.cs b/tests/Our.Umbraco.Ditto.Tests/DefaultProcessorTests.cs index 84b1844..5bea7f9 100644 --- a/tests/Our.Umbraco.Ditto.Tests/DefaultProcessorTests.cs +++ b/tests/Our.Umbraco.Ditto.Tests/DefaultProcessorTests.cs @@ -70,6 +70,9 @@ public void Default_Processor_GlobalLevel_IsUsed() // Tidy-up after test, otherwise the new default processor will cause other tests to fail! Ditto.RegisterDefaultProcessorType(); + + // ...and clear the type-config cache + DittoTypeConfigCache.Clear(); } } } \ No newline at end of file diff --git a/tests/Our.Umbraco.Ditto.Tests/PropertySourceMappingTests.cs b/tests/Our.Umbraco.Ditto.Tests/PropertySourceMappingTests.cs index 5b31dd7..ad0d222 100644 --- a/tests/Our.Umbraco.Ditto.Tests/PropertySourceMappingTests.cs +++ b/tests/Our.Umbraco.Ditto.Tests/PropertySourceMappingTests.cs @@ -52,6 +52,7 @@ public void PropertySource_Properties_Map() }; Ditto.DefaultPropertySource = PropertySource.InstanceThenUmbracoProperties; + DittoTypeConfigCache.Clear(); var model = content.As(); Assert.AreEqual(instanceName, model.Name); @@ -59,6 +60,7 @@ public void PropertySource_Properties_Map() Assert.AreEqual(customProp, model.Custom); Ditto.DefaultPropertySource = PropertySource.UmbracoThenInstanceProperties; + DittoTypeConfigCache.Clear(); model = content.As(); Assert.AreEqual(customName, model.Name); @@ -66,6 +68,7 @@ public void PropertySource_Properties_Map() Assert.AreEqual(customProp, model.Custom); Ditto.DefaultPropertySource = PropertySource.InstanceProperties; + DittoTypeConfigCache.Clear(); model = content.As(); Assert.AreEqual(instanceName, model.Name); @@ -73,6 +76,7 @@ public void PropertySource_Properties_Map() Assert.IsNull(model.Custom); Ditto.DefaultPropertySource = PropertySource.UmbracoProperties; + DittoTypeConfigCache.Clear(); model = content.As(); Assert.AreEqual(customName, model.Name); @@ -81,6 +85,7 @@ public void PropertySource_Properties_Map() // Reset Ditto.DefaultPropertySource = PropertySource.InstanceThenUmbracoProperties; + DittoTypeConfigCache.Clear(); } [Test] @@ -147,9 +152,10 @@ public void PropertySource_Hidden_Properties_Warn() // Check for hidden Umbraco properties Ditto.DefaultPropertySource = PropertySource.InstanceThenUmbracoProperties; - + DittoTypeConfigCache.Clear(); + var model = content.As(); - + var logMessages = mockLogger.GetLogMessages().Where(x => x.MessageType == LogMessageType.Warn && x.CallingType == typeof(UmbracoPropertyAttribute)); Assert.NotNull(logMessages); @@ -159,6 +165,7 @@ public void PropertySource_Hidden_Properties_Warn() mockLogger.ClearLogMessages(); Ditto.DefaultPropertySource = PropertySource.UmbracoThenInstanceProperties; + DittoTypeConfigCache.Clear(); model = content.As(); @@ -169,6 +176,7 @@ public void PropertySource_Hidden_Properties_Warn() // Reset Ditto.DefaultPropertySource = PropertySource.InstanceThenUmbracoProperties; + DittoTypeConfigCache.Clear(); } } } \ No newline at end of file From 26b4b0309f40250024e3d2d924b4c7de30c3ada0 Mon Sep 17 00:00:00 2001 From: leekelleher Date: Fri, 3 Aug 2018 14:51:20 +0100 Subject: [PATCH 2/2] Renamed `DittoTypeConfig` to `DittoTypeInfo` As it contains information about the Type, it's not a "configuration". --- .../{DittoTypeConfig.cs => DittoTypeInfo.cs} | 20 +++++++++---------- ...peConfigCache.cs => DittoTypeInfoCache.cs} | 16 +++++++-------- .../Extensions/PublishedContentExtensions.cs | 16 +++++++-------- .../Our.Umbraco.Ditto.csproj | 4 ++-- .../PublishedContentMapping.cs | 2 +- .../DefaultProcessorTests.cs | 2 +- .../PropertySourceMappingTests.cs | 16 +++++++-------- 7 files changed, 38 insertions(+), 38 deletions(-) rename src/Our.Umbraco.Ditto/Common/{DittoTypeConfig.cs => DittoTypeInfo.cs} (93%) rename src/Our.Umbraco.Ditto/Common/{DittoTypeConfigCache.cs => DittoTypeInfoCache.cs} (56%) diff --git a/src/Our.Umbraco.Ditto/Common/DittoTypeConfig.cs b/src/Our.Umbraco.Ditto/Common/DittoTypeInfo.cs similarity index 93% rename from src/Our.Umbraco.Ditto/Common/DittoTypeConfig.cs rename to src/Our.Umbraco.Ditto/Common/DittoTypeInfo.cs index 8bdf6c7..5808c95 100644 --- a/src/Our.Umbraco.Ditto/Common/DittoTypeConfig.cs +++ b/src/Our.Umbraco.Ditto/Common/DittoTypeInfo.cs @@ -6,16 +6,16 @@ namespace Our.Umbraco.Ditto { - internal sealed class DittoTypeConfig + internal sealed class DittoTypeInfo { - public static DittoTypeConfig Create() + public static DittoTypeInfo Create() { return Create(typeof(T)); } - public static DittoTypeConfig Create(Type type) + public static DittoTypeInfo Create(Type type) { - var config = new DittoTypeConfig + var config = new DittoTypeInfo { TargetType = type }; @@ -70,9 +70,9 @@ public static DittoTypeConfig Create(Type type) // properties (lazy & eager) // - var lazyProperties = new List(); + var lazyProperties = new List(); var lazyPropertyNames = new List(); - var eagerProperties = new List(); + var eagerProperties = new List(); // Collect all the properties of the given type and loop through writable ones. foreach (var property in type.GetProperties(BindingFlags.Public | BindingFlags.Instance)) @@ -90,7 +90,7 @@ public static DittoTypeConfig Create(Type type) var propertyType = property.PropertyType; - var propertyConfig = new DittoTypePropertyConfig + var propertyConfig = new DittoTypePropertyInfo { CustomAttributes = attributes, PropertyInfo = property, @@ -223,17 +223,17 @@ public static DittoTypeConfig Create(Type type) public IEnumerable CustomAttributes { get; set; } public bool HasLazyProperties { get; set; } - public IEnumerable LazyProperties { get; set; } + public IEnumerable LazyProperties { get; set; } public IEnumerable LazyPropertyNames { get; set; } public bool HasEagerProperties { get; set; } - public IEnumerable EagerProperties { get; set; } + public IEnumerable EagerProperties { get; set; } public IEnumerable ConversionHandlers { get; set; } public IEnumerable ConvertingMethods { get; set; } public IEnumerable ConvertedMethods { get; set; } - internal sealed class DittoTypePropertyConfig + internal sealed class DittoTypePropertyInfo { public bool IsCacheable { get; set; } public DittoCacheAttribute CacheInfo { get; set; } diff --git a/src/Our.Umbraco.Ditto/Common/DittoTypeConfigCache.cs b/src/Our.Umbraco.Ditto/Common/DittoTypeInfoCache.cs similarity index 56% rename from src/Our.Umbraco.Ditto/Common/DittoTypeConfigCache.cs rename to src/Our.Umbraco.Ditto/Common/DittoTypeInfoCache.cs index 343ccc3..6b47386 100644 --- a/src/Our.Umbraco.Ditto/Common/DittoTypeConfigCache.cs +++ b/src/Our.Umbraco.Ditto/Common/DittoTypeInfoCache.cs @@ -3,9 +3,9 @@ namespace Our.Umbraco.Ditto { - internal static class DittoTypeConfigCache + internal static class DittoTypeInfoCache { - private static readonly ConcurrentDictionary _cache = new ConcurrentDictionary(); + private static readonly ConcurrentDictionary _cache = new ConcurrentDictionary(); public static void Add() { @@ -14,7 +14,7 @@ public static void Add() public static void Add(Type type) { - _cache.TryAdd(type, DittoTypeConfig.Create(type)); + _cache.TryAdd(type, DittoTypeInfo.Create(type)); } public static void Clear() @@ -24,19 +24,19 @@ public static void Clear() public static void Clear(Type type) { - _cache.TryRemove(type, out DittoTypeConfig config); + _cache.TryRemove(type, out DittoTypeInfo config); } - public static DittoTypeConfig GetOrAdd() + public static DittoTypeInfo GetOrAdd() { return GetOrAdd(typeof(T)); } - public static DittoTypeConfig GetOrAdd(Type type) + public static DittoTypeInfo GetOrAdd(Type type) { - if (_cache.TryGetValue(type, out DittoTypeConfig config) == false) + if (_cache.TryGetValue(type, out DittoTypeInfo config) == false) { - config = DittoTypeConfig.Create(type); + config = DittoTypeInfo.Create(type); _cache.TryAdd(type, config); } diff --git a/src/Our.Umbraco.Ditto/Extensions/PublishedContentExtensions.cs b/src/Our.Umbraco.Ditto/Extensions/PublishedContentExtensions.cs index 75c827a..6210ffa 100644 --- a/src/Our.Umbraco.Ditto/Extensions/PublishedContentExtensions.cs +++ b/src/Our.Umbraco.Ditto/Extensions/PublishedContentExtensions.cs @@ -138,7 +138,7 @@ public static object As( // Convert using (DittoDisposableTimer.DebugDuration(typeof(Ditto), $"As<{type.Name}>({content.DocumentTypeAlias} {content.Id})")) { - var config = DittoTypeConfigCache.GetOrAdd(type); + var config = DittoTypeInfoCache.GetOrAdd(type); if (config.IsCacheable) { @@ -164,7 +164,7 @@ public static object As( /// Thrown if the given type has invalid constructors. private static object ConvertContent( IPublishedContent content, - DittoTypeConfig config, + DittoTypeInfo config, CultureInfo culture, object instance, Action onConverting, @@ -254,8 +254,8 @@ private static object ConvertContent( private static object GetProcessedValue( IPublishedContent content, CultureInfo culture, - DittoTypeConfig config, - DittoTypeConfig.DittoTypePropertyConfig mappableProperty, + DittoTypeInfo config, + DittoTypeInfo.DittoTypePropertyInfo mappableProperty, object instance, DittoChainContext chainContext) { @@ -291,7 +291,7 @@ private static object GetProcessedValue( /// Returns the processed value. private static object DoGetProcessedValue( IPublishedContent content, - DittoTypeConfig.DittoTypePropertyConfig mappableProperty, + DittoTypeInfo.DittoTypePropertyInfo mappableProperty, DittoProcessorContext baseProcessorContext, DittoChainContext chainContext) { @@ -344,7 +344,7 @@ private static object DoGetProcessedValue( /// The to fire when converting. private static void OnConverting( IPublishedContent content, - DittoTypeConfig config, + DittoTypeInfo config, CultureInfo culture, object instance, Action callback) @@ -366,7 +366,7 @@ private static void OnConverting( /// The to fire when converted. private static void OnConverted( IPublishedContent content, - DittoTypeConfig config, + DittoTypeInfo config, CultureInfo culture, object instance, Action callback) @@ -391,7 +391,7 @@ private static void OnConverted( private static void OnConvert( DittoConversionHandlerType conversionType, IPublishedContent content, - DittoTypeConfig config, + DittoTypeInfo config, CultureInfo culture, object instance, Action callback) diff --git a/src/Our.Umbraco.Ditto/Our.Umbraco.Ditto.csproj b/src/Our.Umbraco.Ditto/Our.Umbraco.Ditto.csproj index 4f34c19..fce0d59 100644 --- a/src/Our.Umbraco.Ditto/Our.Umbraco.Ditto.csproj +++ b/src/Our.Umbraco.Ditto/Our.Umbraco.Ditto.csproj @@ -81,8 +81,8 @@ - - + + diff --git a/tests/Our.Umbraco.Ditto.PerformanceTests/PublishedContentMapping.cs b/tests/Our.Umbraco.Ditto.PerformanceTests/PublishedContentMapping.cs index 2da64fc..f08f12b 100644 --- a/tests/Our.Umbraco.Ditto.PerformanceTests/PublishedContentMapping.cs +++ b/tests/Our.Umbraco.Ditto.PerformanceTests/PublishedContentMapping.cs @@ -50,7 +50,7 @@ public void Setup() Ditto.DeregisterPostProcessorType(); // pre-load the type config - DittoTypeConfigCache.Add(); + DittoTypeInfoCache.Add(); } [Benchmark(Baseline = true)] diff --git a/tests/Our.Umbraco.Ditto.Tests/DefaultProcessorTests.cs b/tests/Our.Umbraco.Ditto.Tests/DefaultProcessorTests.cs index 5bea7f9..a9140bb 100644 --- a/tests/Our.Umbraco.Ditto.Tests/DefaultProcessorTests.cs +++ b/tests/Our.Umbraco.Ditto.Tests/DefaultProcessorTests.cs @@ -72,7 +72,7 @@ public void Default_Processor_GlobalLevel_IsUsed() Ditto.RegisterDefaultProcessorType(); // ...and clear the type-config cache - DittoTypeConfigCache.Clear(); + DittoTypeInfoCache.Clear(); } } } \ No newline at end of file diff --git a/tests/Our.Umbraco.Ditto.Tests/PropertySourceMappingTests.cs b/tests/Our.Umbraco.Ditto.Tests/PropertySourceMappingTests.cs index ad0d222..976b534 100644 --- a/tests/Our.Umbraco.Ditto.Tests/PropertySourceMappingTests.cs +++ b/tests/Our.Umbraco.Ditto.Tests/PropertySourceMappingTests.cs @@ -52,7 +52,7 @@ public void PropertySource_Properties_Map() }; Ditto.DefaultPropertySource = PropertySource.InstanceThenUmbracoProperties; - DittoTypeConfigCache.Clear(); + DittoTypeInfoCache.Clear(); var model = content.As(); Assert.AreEqual(instanceName, model.Name); @@ -60,7 +60,7 @@ public void PropertySource_Properties_Map() Assert.AreEqual(customProp, model.Custom); Ditto.DefaultPropertySource = PropertySource.UmbracoThenInstanceProperties; - DittoTypeConfigCache.Clear(); + DittoTypeInfoCache.Clear(); model = content.As(); Assert.AreEqual(customName, model.Name); @@ -68,7 +68,7 @@ public void PropertySource_Properties_Map() Assert.AreEqual(customProp, model.Custom); Ditto.DefaultPropertySource = PropertySource.InstanceProperties; - DittoTypeConfigCache.Clear(); + DittoTypeInfoCache.Clear(); model = content.As(); Assert.AreEqual(instanceName, model.Name); @@ -76,7 +76,7 @@ public void PropertySource_Properties_Map() Assert.IsNull(model.Custom); Ditto.DefaultPropertySource = PropertySource.UmbracoProperties; - DittoTypeConfigCache.Clear(); + DittoTypeInfoCache.Clear(); model = content.As(); Assert.AreEqual(customName, model.Name); @@ -85,7 +85,7 @@ public void PropertySource_Properties_Map() // Reset Ditto.DefaultPropertySource = PropertySource.InstanceThenUmbracoProperties; - DittoTypeConfigCache.Clear(); + DittoTypeInfoCache.Clear(); } [Test] @@ -152,7 +152,7 @@ public void PropertySource_Hidden_Properties_Warn() // Check for hidden Umbraco properties Ditto.DefaultPropertySource = PropertySource.InstanceThenUmbracoProperties; - DittoTypeConfigCache.Clear(); + DittoTypeInfoCache.Clear(); var model = content.As(); @@ -165,7 +165,7 @@ public void PropertySource_Hidden_Properties_Warn() mockLogger.ClearLogMessages(); Ditto.DefaultPropertySource = PropertySource.UmbracoThenInstanceProperties; - DittoTypeConfigCache.Clear(); + DittoTypeInfoCache.Clear(); model = content.As(); @@ -176,7 +176,7 @@ public void PropertySource_Hidden_Properties_Warn() // Reset Ditto.DefaultPropertySource = PropertySource.InstanceThenUmbracoProperties; - DittoTypeConfigCache.Clear(); + DittoTypeInfoCache.Clear(); } } } \ No newline at end of file