Merge pull request #233 from leekelleher/dev/deprecate-propertydescri…

…ptor Deprecate PropertyDescriptor in Contexts
umco · Jan 9, 2018 · 6cd3d9a · 6cd3d9a
2 parents 1a5b163 + b5e5761
commit 6cd3d9a
Show file tree

Hide file tree

Showing 18 changed files with 75 additions and 42 deletions.
diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Attributes/DittoProcessorAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Attributes/DittoProcessorAttribute.cs
@@ -196,7 +196,7 @@ protected DatabaseContext DatabaseContext
             this.Context = context;
             this.ChainContext = chainContext;
 
-            var ctx = new DittoCacheContext(this, context.Content, context.TargetType, context.PropertyDescriptor, context.Culture);
+            var ctx = new DittoCacheContext(this, context.Content, context.TargetType, context.PropertyInfo, context.Culture);
             return this.GetCacheItem(ctx, this.ProcessValue);
         }
     }

diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Caching/DittoCacheContext.cs b/src/Our.Umbraco.Ditto/ComponentModel/Caching/DittoCacheContext.cs
@@ -1,6 +1,7 @@
 using System;
 using System.ComponentModel;
 using System.Globalization;
+using System.Reflection;
 using Umbraco.Core.Models;
 
 namespace Our.Umbraco.Ditto
@@ -30,21 +31,21 @@ public class DittoCacheContext
         /// </summary>
         /// <param name="attribute">The attribute.</param>
         /// <param name="content">The content.</param>
-        /// <param name="targetType">Type of the target.</param>
-        /// <param name="propertyDescriptor">The property descriptor.</param>
+        /// <param name="targetType">Type of the target.</param> 
+        /// <param name="propertyInfo">The property info.</param>
         /// <param name="culture">The culture.</param>
         internal DittoCacheContext(
             DittoCacheableAttribute attribute,
             IPublishedContent content,
             Type targetType,
-            PropertyDescriptor propertyDescriptor,
+            PropertyInfo propertyInfo,
             CultureInfo culture)
         {
             this.Attribute = attribute;
             this.Content = content;
             this.TargetType = targetType;
             this.Culture = culture;
-            this.PropertyDescriptor = propertyDescriptor;
+            this.PropertyInfo = propertyInfo;
         }
 
         /// <summary>
@@ -69,7 +70,24 @@ public class DittoCacheContext
         /// <value>
         /// The property descriptor.
         /// </value>
-        public PropertyDescriptor PropertyDescriptor { get; internal set; }
+        [Obsolete("PropertyDescriptor has been deprecated, please use PropertyInfo instead. This property will be removed in a future Ditto version.", false)]
+        public PropertyDescriptor PropertyDescriptor
+        {
+            get
+            {
+                return this.TargetType != null && this.PropertyInfo != null
+                    ? TypeDescriptor.GetProperties(this.TargetType)[this.PropertyInfo.Name]
+                    : null;
+            }
+        }
+
+        /// <summary>
+        /// Gets the property info.
+        /// </summary>
+        /// <value>
+        /// The property info.
+        /// </value>
+        public PropertyInfo PropertyInfo { get; set; }
 
         /// <summary>
         /// Gets the culture.

diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Caching/DittoDefaultCacheKeyBuilder.cs b/src/Our.Umbraco.Ditto/ComponentModel/Caching/DittoDefaultCacheKeyBuilder.cs
@@ -26,9 +26,9 @@ public override string BuildCacheKey(DittoCacheContext context)
                 cacheKey.Add(context.Content.Version);
             }
 
-            if (context.PropertyDescriptor != null && (context.Attribute.CacheBy & DittoCacheBy.PropertyName) == DittoCacheBy.PropertyName)
+            if (context.PropertyInfo != null && (context.Attribute.CacheBy & DittoCacheBy.PropertyName) == DittoCacheBy.PropertyName)
             {
-                cacheKey.Add(context.PropertyDescriptor.Name);
+                cacheKey.Add(context.PropertyInfo.Name);
             }
 
             if ((context.Attribute.CacheBy & DittoCacheBy.TargetType) == DittoCacheBy.TargetType)

diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/AppSettingAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/AppSettingAttribute.cs
@@ -31,7 +31,7 @@ public AppSettingAttribute(string appSettingKey)
         /// </returns>
         public override object ProcessValue()
         {
-            var appSettingKey = this.AppSettingKey ?? (this.Context.PropertyDescriptor != null ? this.Context.PropertyDescriptor.Name : string.Empty);
+            var appSettingKey = this.AppSettingKey ?? (this.Context.PropertyInfo != null ? this.Context.PropertyInfo.Name : string.Empty);
 
             if (string.IsNullOrWhiteSpace(appSettingKey))
             {

diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/Contexts/DittoProcessorContext.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/Contexts/DittoProcessorContext.cs
@@ -1,6 +1,7 @@
 using System;
 using System.ComponentModel;
 using System.Globalization;
+using System.Reflection;
 using Umbraco.Core.Models;
 
 namespace Our.Umbraco.Ditto
@@ -32,7 +33,24 @@ public class DittoProcessorContext
         /// <value>
         /// The property descriptor.
         /// </value>
-        public PropertyDescriptor PropertyDescriptor { get; internal set; }
+        [Obsolete("PropertyDescriptor has been deprecated, please use PropertyInfo instead. This property will be removed in a future Ditto version.", false)]
+        public PropertyDescriptor PropertyDescriptor
+        {
+            get
+            {
+                return this.TargetType != null && this.PropertyInfo != null
+                    ? TypeDescriptor.GetProperties(this.TargetType)[this.PropertyInfo.Name]
+                    : null;
+            }
+        }
+
+        /// <summary>
+        /// Gets the property info.
+        /// </summary>
+        /// <value>
+        /// The property info.
+        /// </value>
+        public PropertyInfo PropertyInfo { get; set; }
 
         /// <summary>
         /// Gets the culture.
@@ -51,7 +69,7 @@ internal DittoProcessorContext Populate(DittoProcessorContext baseProcessorConte
         {
             Content = baseProcessorContext.Content;
             TargetType = baseProcessorContext.TargetType;
-            PropertyDescriptor = baseProcessorContext.PropertyDescriptor;
+            PropertyInfo = baseProcessorContext.PropertyInfo;
             Culture = baseProcessorContext.Culture;
 
             return this;

diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/CurrentContentAsAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/CurrentContentAsAttribute.cs
@@ -19,9 +19,9 @@ public override object ProcessValue()
         {
             // NOTE: [LK] In order to prevent an infinite loop / stack-overflow, we check if the
             // property's type matches the containing model's type, then we throw an exception.
-            if (this.Context.PropertyDescriptor.PropertyType == this.Context.PropertyDescriptor.ComponentType)
+            if (this.Context.PropertyInfo.PropertyType == this.Context.TargetType)
             {
-                throw new InvalidOperationException($"Unable to process property type '{this.Context.PropertyDescriptor.PropertyType.Name}', it is the same as the containing model type.");
+                throw new InvalidOperationException($"Unable to process property type '{this.Context.TargetType.Name}', it is the same as the containing model type.");
             }
 
             return this.Context.Content;

diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/DittoFactoryAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/DittoFactoryAttribute.cs
@@ -48,7 +48,7 @@ protected DittoFactoryAttribute(Type[] allowedTypes)
         /// </returns>
         public override object ProcessValue()
         {
-            var propType = this.Context.PropertyDescriptor.PropertyType;
+            var propType = this.Context.PropertyInfo.PropertyType;
             var propTypeIsEnumerable = propType.IsEnumerableType();
             var baseType = propTypeIsEnumerable ? propType.GetEnumerableType() : propType;
 

diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/EnumAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/EnumAttribute.cs
@@ -19,12 +19,12 @@ public class EnumAttribute : DittoProcessorAttribute
         public override object ProcessValue()
         {
             // ReSharper disable once ConditionIsAlwaysTrueOrFalse
-            if (this.Context == null || this.Context.PropertyDescriptor == null)
+            if (this.Context == null || this.Context.PropertyInfo == null)
             {
                 return null;
             }
 
-            var propertyType = this.Context.PropertyDescriptor.PropertyType;
+            var propertyType = this.Context.PropertyInfo.PropertyType;
 
             if (this.Value.IsNullOrEmptyString())
             {

diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/EnumerableConverterAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/EnumerableConverterAttribute.cs
@@ -35,7 +35,7 @@ public EnumerableConverterAttribute()
         public override object ProcessValue()
         {
             object result = this.Value;
-            var propertyType = this.Context.PropertyDescriptor.PropertyType;
+            var propertyType = this.Context.PropertyInfo.PropertyType;
             var propertyIsEnumerableType = Direction == EnumerableConvertionDirection.Automatic
                 ? propertyType.IsEnumerableType()
                     && (propertyType == typeof(string)) == false

diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/HtmlStringAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/HtmlStringAttribute.cs
@@ -17,7 +17,7 @@ public class HtmlStringAttribute : DittoProcessorAttribute
         /// </returns>
         public override object ProcessValue()
         {
-            if (typeof(IHtmlString).IsAssignableFrom(this.Context.PropertyDescriptor.PropertyType))
+            if (typeof(IHtmlString).IsAssignableFrom(this.Context.PropertyInfo.PropertyType))
             {
                 if (this.Value.IsNullOrEmptyString())
                 {

diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/RecursiveDittoAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/RecursiveDittoAttribute.cs
@@ -25,25 +25,25 @@ public override object ProcessValue()
             var result = this.Value;
 
             // If we aren't already the right type, try recursing if the type is IPublishedContent
-            if (this.Value != null && !this.Context.PropertyDescriptor.PropertyType.IsInstanceOfType(this.Value))
+            if (this.Value != null && !this.Context.PropertyInfo.PropertyType.IsInstanceOfType(this.Value))
             {
-                if (this.Value is IPublishedContent && this.Context.PropertyDescriptor.PropertyType.IsClass)
+                if (this.Value is IPublishedContent && this.Context.PropertyInfo.PropertyType.IsClass)
                 {
                     // If the property value is an IPublishedContent, then we can use Ditto to map to the target type.
                     result = ((IPublishedContent)this.Value).As(
-                        this.Context.PropertyDescriptor.PropertyType,
+                        this.Context.PropertyInfo.PropertyType,
                         this.Context.Culture,
                         null,
                         chainContext: ChainContext);
                 }
                 else if (this.Value != null && this.Value.GetType().IsEnumerableOfType(typeof(IPublishedContent))
-                    && this.Context.PropertyDescriptor.PropertyType.IsEnumerable()
-                    && this.Context.PropertyDescriptor.PropertyType.GetEnumerableType() != null
-                    && this.Context.PropertyDescriptor.PropertyType.GetEnumerableType().IsClass)
+                    && this.Context.PropertyInfo.PropertyType.IsEnumerable()
+                    && this.Context.PropertyInfo.PropertyType.GetEnumerableType() != null
+                    && this.Context.PropertyInfo.PropertyType.GetEnumerableType().IsClass)
                 {
                     // If the property value is IEnumerable<IPublishedContent>, then we can use Ditto to map to the target type.
                     result = ((IEnumerable<IPublishedContent>)this.Value).As(
-                        this.Context.PropertyDescriptor.PropertyType.GetEnumerableType(),
+                        this.Context.PropertyInfo.PropertyType.GetEnumerableType(),
                         this.Context.Culture,
                         chainContext: ChainContext);
                 }

diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/TryConvertToAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/TryConvertToAttribute.cs
@@ -17,12 +17,12 @@ public override object ProcessValue()
         {
             var result = this.Value;
 
-            if (this.Value != null && !this.Context.PropertyDescriptor.PropertyType.IsInstanceOfType(this.Value))
+            if (this.Value != null && !this.Context.PropertyInfo.PropertyType.IsInstanceOfType(this.Value))
             {
                 // TODO: Maybe support enumerables?
-                using (DittoDisposableTimer.DebugDuration<TryConvertToAttribute>($"TryConvertTo '{this.Context.PropertyDescriptor.Name}' ({this.Context.Content.Id})"))
+                using (DittoDisposableTimer.DebugDuration<TryConvertToAttribute>($"TryConvertTo '{this.Context.PropertyInfo.Name}' ({this.Context.Content.Id})"))
                 {
-                    var convert = this.Value.TryConvertTo(this.Context.PropertyDescriptor.PropertyType);
+                    var convert = this.Value.TryConvertTo(this.Context.PropertyInfo.PropertyType);
                     if (convert.Success)
                     {
                         result = convert.Result;

diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/UmbracoDictionaryAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/UmbracoDictionaryAttribute.cs
@@ -39,7 +39,7 @@ public UmbracoDictionaryAttribute(string dictionaryKey)
         public override object ProcessValue()
         {
             var dictionaryKey = this.DictionaryKey
-                ?? (this.Context.PropertyDescriptor != null ? this.Context.PropertyDescriptor.Name : string.Empty);
+                ?? (this.Context.PropertyInfo != null ? this.Context.PropertyInfo.Name : string.Empty);
 
             if (string.IsNullOrWhiteSpace(dictionaryKey))
             {

diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/UmbracoPickerAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/UmbracoPickerAttribute.cs
@@ -22,7 +22,7 @@ public class UmbracoPickerAttribute : DittoProcessorAttribute
         public override object ProcessValue()
         {
             // ReSharper disable once ConditionIsAlwaysTrueOrFalse
-            if (this.Context == null || this.Context.PropertyDescriptor == null || this.Value.IsNullOrEmptyString())
+            if (this.Context == null || this.Context.PropertyInfo == null || this.Value.IsNullOrEmptyString())
             {
                 return Enumerable.Empty<IPublishedContent>();
             }

diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/UmbracoPropertyAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/UmbracoPropertyAttribute.cs
@@ -89,7 +89,7 @@ public override object ProcessValue()
             var defaultValue = this.DefaultValue;
 
             var recursive = this.Recursive;
-            var propName = this.Context.PropertyDescriptor != null ? this.Context.PropertyDescriptor.Name : string.Empty;
+            var propName = this.Context.PropertyInfo != null ? this.Context.PropertyInfo.Name : string.Empty;
             var altPropName = string.Empty;
 
             // Check for Umbraco properties attribute on class

diff --git a/src/Our.Umbraco.Ditto/Extensions/PublishedContentExtensions.cs b/src/Our.Umbraco.Ditto/Extensions/PublishedContentExtensions.cs
@@ -328,23 +328,20 @@ public static class PublishedContentExtensions
         {
             using (DittoDisposableTimer.DebugDuration(typeof(Ditto), $"Processing '{propertyInfo.Name}' ({content.Id})"))
             {
-                // Get the target property description
-                var propertyDescriptor = TypeDescriptor.GetProperties(instance)[propertyInfo.Name];
-
                 // Create a base processor context for this current chain level
                 var baseProcessorContext = new DittoProcessorContext
                 {
                     Content = content,
                     TargetType = targetType,
-                    PropertyDescriptor = propertyDescriptor,
+                    PropertyInfo = propertyInfo,
                     Culture = culture
                 };
 
                 // Check for cache attribute
                 var cacheAttr = propertyInfo.GetCustomAttribute<DittoCacheAttribute>(true);
                 if (cacheAttr != null)
                 {
-                    var ctx = new DittoCacheContext(cacheAttr, content, targetType, propertyDescriptor, culture);
+                    var ctx = new DittoCacheContext(cacheAttr, content, targetType, propertyInfo, culture);
                     return cacheAttr.GetCacheItem(ctx, () => DoGetProcessedValue(content, propertyInfo, contextAccessor, baseProcessorContext, chainContext));
                 }
                 else

diff --git a/tests/Our.Umbraco.Ditto.Tests/EnumerableConverterTests.cs b/tests/Our.Umbraco.Ditto.Tests/EnumerableConverterTests.cs
@@ -62,7 +62,7 @@ public void EnumerableConverter_EnumerableModel_Test()
         {
             var context = new DittoProcessorContext
             {
-                PropertyDescriptor = TypeDescriptor.GetProperties(new WrappedModel())["MyProperty1"]
+                PropertyInfo = typeof(WrappedModel).GetProperty("MyProperty1")
             };
 
             var processor = new EnumerableConverterAttribute();
@@ -76,7 +76,7 @@ public void EnumerableConverter_InheritedEnumerableModel_Test()
         {
             var context = new DittoProcessorContext
             {
-                PropertyDescriptor = TypeDescriptor.GetProperties(new WrappedModel())["MyProperty2"]
+                PropertyInfo = typeof(WrappedModel).GetProperty("MyProperty2")
             };
 
             var processor = new EnumerableConverterAttribute();
@@ -91,7 +91,7 @@ public void EnumerableConverter_EnumerableModelEmptyList_Test()
         {
             var context = new DittoProcessorContext
             {
-                PropertyDescriptor = TypeDescriptor.GetProperties(new WrappedModel())["MyProperty3"]
+                PropertyInfo = typeof(WrappedModel).GetProperty("MyProperty3")
             };
 
             var processor = new EnumerableConverterAttribute();
@@ -105,7 +105,7 @@ public void EnumerableConverter_EnumerableModelEmptyDictionary_Test()
         {
             var context = new DittoProcessorContext
             {
-                PropertyDescriptor = TypeDescriptor.GetProperties(new WrappedModel())["MyProperty4"]
+                PropertyInfo = typeof(WrappedModel).GetProperty("MyProperty4")
             };
 
             var processor = new EnumerableConverterAttribute();

diff --git a/tests/Our.Umbraco.Ditto.Tests/ExtremeChainedContextTests.cs b/tests/Our.Umbraco.Ditto.Tests/ExtremeChainedContextTests.cs
@@ -39,15 +39,15 @@ public class MyProcessorAttribute : DittoProcessorAttribute
         {
             public override object ProcessValue()
             {
-                Assert.That(this.Context.PropertyDescriptor.Name, Is.EqualTo("MyProperty"));
+                Assert.That(this.Context.PropertyInfo.Name, Is.EqualTo("MyProperty"));
 
                 var items = Value as IEnumerable;
                 foreach (var item in items)
                 {
                     // do nothing, we just need the for-loop so that the collection is evaluated
                 }
 
-                Assert.That(this.Context.PropertyDescriptor.Name, Is.EqualTo("MyProperty"));
+                Assert.That(this.Context.PropertyInfo.Name, Is.EqualTo("MyProperty"));
 
                 return Value;
             }