Minor code amends

leekelleher · leekelleher · commit 6dbf87822a37 · 2018-01-23T17:40:32.000Z
- DittoChainContext - renamed parameter vars
- DittoCacheableAttribute - references the local `cacheKeyBuilderType` as this could be different than the instance property value.
- UmbracoPropertiesAttribute - corrected a mistake with the `Inherited` bool value.
- UmbracoPropertyAttribute - removed the dependency on Umbraco.Core `IsNullOrWhiteSpace()` extension method. This incurred extra allocations.
- MockPublishedContent - switched from `SingleOrDefault` to `FirstOrDefault` - to reduce the number of iterations of the lookup.
diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Attributes/DittoCacheableAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Attributes/DittoCacheableAttribute.cs
@@ -49,9 +49,12 @@ protected DittoCacheableAttribute()
         /// <param name="cacheContext">The cache context.</param>
         /// <param name="refresher">The refresher.</param>
         /// <returns>Returns the output type.</returns>
-        /// <exception cref="System.ApplicationException">Expected a cache key builder of type  + typeof(DittoProcessorCacheKeyBuilder) +  but got  + CacheKeyBuilderType</exception>
+        /// <exception cref="ApplicationException">Expected a cache key builder of type DittoProcessorCacheKeyBuilder but got CacheKeyBuilderType.</exception>
         internal TOuputType GetCacheItem<TOuputType>(DittoCacheContext cacheContext, Func<TOuputType> refresher)
         {
+            // TODO: [LK:2018-01-18] Review this, does `cacheContext` need to be passed in?
+            // Given that the values are available on the instance.
+
             // If no cache duration set, (and in debug mode AND NOT a unit-test), then just run the refresher
             if (this.CacheDuration == 0 || (Ditto.IsDebuggingEnabled && Ditto.IsRunningInUnitTest == false))
             {
@@ -64,7 +67,7 @@ internal TOuputType GetCacheItem<TOuputType>(DittoCacheContext cacheContext, Fun
             // Check the cache key builder type
             if (typeof(DittoCacheKeyBuilder).IsAssignableFrom(cacheKeyBuilderType) == false)
             {
-                throw new ApplicationException($"Expected a cache key builder of type {typeof(DittoCacheKeyBuilder)} but got {this.CacheKeyBuilderType}");
+                throw new ApplicationException($"Expected a cache key builder of type {typeof(DittoCacheKeyBuilder)} but got {cacheKeyBuilderType}");
             }
 
             // Construct the cache key builder
diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/Contexts/DittoChainContext.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/Contexts/DittoChainContext.cs
@@ -48,26 +48,26 @@ internal ProcessorContextsCollection(IEnumerable<DittoProcessorContext> processo
         /// Adds a range of processor contexts to the collection chain.
         /// </summary>
         /// <param name="ctxs">An enumerable of processor contexts.</param>
-        public void AddRange(IEnumerable<DittoProcessorContext> ctxs)
+        public void AddRange(IEnumerable<DittoProcessorContext> contexts)
         {
-            if (ctxs == null)
+            if (contexts == null)
             {
                 return;
             }
 
-            foreach (var ctx in ctxs)
+            foreach (var context in contexts)
             {
-                Add(ctx);
+                Add(context);
             }
         }
 
         /// <summary>
         /// Adds a processor context to the collection chain.
         /// </summary>
         /// <param name="ctx">The processor context.</param>
-        public void Add(DittoProcessorContext ctx)
+        public void Add(DittoProcessorContext context)
         {
-            _processorContexts.AddOrUpdate(ctx.GetType(), ctx, (type, ctx2) => ctx2); // Don't override if already exists
+            _processorContexts.AddOrUpdate(context.GetType(), context, (type, ctx) => ctx); // Don't override if already exists
         }
 
         /// <summary>
diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/UmbracoPropertiesAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/UmbracoPropertiesAttribute.cs
@@ -5,7 +5,7 @@ namespace Our.Umbraco.Ditto
     /// <summary>
     /// The Umbraco properties attribute.
     /// </summary>
-    [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = false)]
+    [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)]
     public sealed class UmbracoPropertiesAttribute : Attribute
     {
         /// <summary>
diff --git a/src/Our.Umbraco.Ditto/ComponentModel/Processors/UmbracoPropertyAttribute.cs b/src/Our.Umbraco.Ditto/ComponentModel/Processors/UmbracoPropertyAttribute.cs
@@ -1,7 +1,5 @@
 ﻿using System;
 using System.Linq;
-using System.Reflection;
-using Umbraco.Core;
 using Umbraco.Core.Logging;
 using Umbraco.Core.Models;
 using Umbraco.Web;
@@ -133,14 +131,14 @@ public override object ProcessValue()
             }
 
             // Try fetching the alt value.
-            if ((propertyValue == null || propertyValue.ToString().IsNullOrWhiteSpace())
+            if ((propertyValue == null || (propertyValue is string tmp && string.IsNullOrWhiteSpace(tmp)))
                 && string.IsNullOrWhiteSpace(altUmbracoPropertyName) == false)
             {
                 propertyValue = GetPropertyValue(content, altUmbracoPropertyName, recursive);
             }
 
             // Try setting the default value.
-            if ((propertyValue == null || propertyValue.ToString().IsNullOrWhiteSpace())
+            if ((propertyValue == null || (propertyValue is string tmp2 && string.IsNullOrWhiteSpace(tmp2)))
                 && defaultValue != null)
             {
                 propertyValue = defaultValue;
@@ -165,13 +163,13 @@ protected object GetPropertyValue(IPublishedContent content, string umbracoPrope
                 propertyValue = GetClassPropertyValue(content, umbracoPropertyName);
             }
 
-            if ((propertyValue == null || propertyValue.ToString().IsNullOrWhiteSpace())
+            if ((propertyValue == null || (propertyValue is string tmp && string.IsNullOrWhiteSpace(tmp)))
                 && (PropertySource != PropertySource.InstanceProperties))
             {
                 propertyValue = GetUmbracoPropertyValue(content, umbracoPropertyName, recursive);
             }
 
-            if ((propertyValue == null || propertyValue.ToString().IsNullOrWhiteSpace())
+            if ((propertyValue == null || (propertyValue is string tmp2 && string.IsNullOrWhiteSpace(tmp2)))
                 && PropertySource == PropertySource.UmbracoThenInstanceProperties)
             {
                 propertyValue = GetClassPropertyValue(content, umbracoPropertyName);
@@ -194,10 +192,10 @@ private object GetClassPropertyValue(IPublishedContent content, string umbracoPr
             {
                 if (Ditto.IsDebuggingEnabled
                     && PropertySource == PropertySource.InstanceThenUmbracoProperties
-                    && Ditto.IPublishedContentProperties.Any(x => x.Name.InvariantEquals(umbracoPropertyName))
+                    && Ditto.IPublishedContentProperties.Any(x => string.Equals(x.Name, umbracoPropertyName, StringComparison.InvariantCultureIgnoreCase))
                     && content.HasProperty(umbracoPropertyName))
                 {
-                    // Property is an IPublishedContent property and an umbraco property exists so warn the user
+                    // Property is an IPublishedContent property and an Umbraco property exists so warn the user
                     LogHelper.Warn<UmbracoPropertyAttribute>($"The property {umbracoPropertyName} being mapped from content type {contentType.Name}'s instance properties hides a property in the Umbraco properties collection of the same name. It is recommended that you avoid using Umbraco property aliases that conflict with IPublishedContent instance property names, but if you can't avoid this and you require access to the hidden property you can use the PropertySource parameter of the processors attribute to override the order in which properties are checked.");
                 }
 
@@ -219,11 +217,11 @@ private object GetUmbracoPropertyValue(IPublishedContent content, string umbraco
         {
             if (Ditto.IsDebuggingEnabled
                 && PropertySource == PropertySource.UmbracoThenInstanceProperties
-                && Ditto.IPublishedContentProperties.Any(x => x.Name.InvariantEquals(umbracoPropertyName))
+                && Ditto.IPublishedContentProperties.Any(x => string.Equals(x.Name, umbracoPropertyName, StringComparison.InvariantCultureIgnoreCase))
                 && content.HasProperty(umbracoPropertyName))
             {
-                // Property is an IPublishedContent property and an umbraco property exists so warn the user
-                LogHelper.Warn<UmbracoPropertyAttribute>($"The property {umbracoPropertyName} being mapped from the Umbraco properties collection hides an instance property of the same name on content type {content.GetType().Name}. It is recommended that you avoid using Umbraco property aliases that conflict with IPublishedContent instance property names, but if you can't avoid this and you require access to the hidden property you can use the PropertySource parameter of the processors attribute to override the order in which properties are checked.");
+                // Property is an IPublishedContent property and an Umbraco property exists so warn the user
+                LogHelper.Warn<UmbracoPropertyAttribute>($"The property {umbracoPropertyName} being mapped from the Umbraco properties collection hides an instance property of the same name on content type {content}. It is recommended that you avoid using Umbraco property aliases that conflict with IPublishedContent instance property names, but if you can't avoid this and you require access to the hidden property you can use the PropertySource parameter of the processors attribute to override the order in which properties are checked.");
             }
 
             return content.GetPropertyValue(umbracoPropertyName, recursive);
diff --git a/tests/Our.Umbraco.Ditto.Tests/Mocks/MockPublishedContent.cs b/tests/Our.Umbraco.Ditto.Tests/Mocks/MockPublishedContent.cs
@@ -3,7 +3,6 @@
 using System.Collections.ObjectModel;
 using System.Linq;
 using Moq;
-using Umbraco.Core;
 using Umbraco.Core.Logging;
 using Umbraco.Core.Models;
 using Umbraco.Core.Models.PublishedContent;
@@ -50,7 +49,7 @@ public IPublishedProperty GetProperty(string alias)
 
         public IPublishedProperty GetProperty(string alias, bool recurse)
         {
-            var prop = Properties.SingleOrDefault(p => p.PropertyTypeAlias.InvariantEquals(alias));
+            var prop = Properties.FirstOrDefault(p => string.Equals(p.PropertyTypeAlias, alias, StringComparison.InvariantCultureIgnoreCase));
 
             if (prop == null && recurse && Parent != null)
             {

Original file line number	Diff line number	Diff line change
`@@ -49,9 +49,12 @@ protected DittoCacheableAttribute()`
`49`	`49`	`/// <param name="cacheContext">The cache context.</param>`
`50`	`50`	`/// <param name="refresher">The refresher.</param>`
`51`	`51`	`/// <returns>Returns the output type.</returns>`
`52`		`- /// <exception cref="System.ApplicationException">Expected a cache key builder of type + typeof(DittoProcessorCacheKeyBuilder) + but got + CacheKeyBuilderType</exception>`
	`52`	`+ /// <exception cref="ApplicationException">Expected a cache key builder of type DittoProcessorCacheKeyBuilder but got CacheKeyBuilderType.</exception>`
`53`	`53`	`internal TOuputType GetCacheItem<TOuputType>(DittoCacheContext cacheContext, Func<TOuputType> refresher)`
`54`	`54`	`{`
	`55`	+ // TODO: [LK:2018-01-18] Review this, does `cacheContext` need to be passed in?
	`56`	`+ // Given that the values are available on the instance.`
	`57`	`+`
`55`	`58`	`// If no cache duration set, (and in debug mode AND NOT a unit-test), then just run the refresher`
`56`	`59`	`if (this.CacheDuration == 0 \|\| (Ditto.IsDebuggingEnabled && Ditto.IsRunningInUnitTest == false))`
`57`	`60`	`{`
`@@ -64,7 +67,7 @@ internal TOuputType GetCacheItem<TOuputType>(DittoCacheContext cacheContext, Fun`
`64`	`67`	`// Check the cache key builder type`
`65`	`68`	`if (typeof(DittoCacheKeyBuilder).IsAssignableFrom(cacheKeyBuilderType) == false)`
`66`	`69`	`{`
`67`		`- throw new ApplicationException($"Expected a cache key builder of type {typeof(DittoCacheKeyBuilder)} but got {this.CacheKeyBuilderType}");`
	`70`	`+ throw new ApplicationException($"Expected a cache key builder of type {typeof(DittoCacheKeyBuilder)} but got {cacheKeyBuilderType}");`
`68`	`71`	`}`
`69`	`72`
`70`	`73`	`// Construct the cache key builder`
Original file line number	Diff line number	Diff line change
`@@ -48,26 +48,26 @@ internal ProcessorContextsCollection(IEnumerable<DittoProcessorContext> processo`
`48`	`48`	`/// Adds a range of processor contexts to the collection chain.`
`49`	`49`	`/// </summary>`
`50`	`50`	`/// <param name="ctxs">An enumerable of processor contexts.</param>`
`51`		`- public void AddRange(IEnumerable<DittoProcessorContext> ctxs)`
	`51`	`+ public void AddRange(IEnumerable<DittoProcessorContext> contexts)`
`52`	`52`	`{`
`53`		`- if (ctxs == null)`
	`53`	`+ if (contexts == null)`
`54`	`54`	`{`
`55`	`55`	`return;`
`56`	`56`	`}`
`57`	`57`
`58`		`- foreach (var ctx in ctxs)`
	`58`	`+ foreach (var context in contexts)`
`59`	`59`	`{`
`60`		`- Add(ctx);`
	`60`	`+ Add(context);`
`61`	`61`	`}`
`62`	`62`	`}`
`63`	`63`
`64`	`64`	`/// <summary>`
`65`	`65`	`/// Adds a processor context to the collection chain.`
`66`	`66`	`/// </summary>`
`67`	`67`	`/// <param name="ctx">The processor context.</param>`
`68`		`- public void Add(DittoProcessorContext ctx)`
	`68`	`+ public void Add(DittoProcessorContext context)`
`69`	`69`	`{`
`70`		`- _processorContexts.AddOrUpdate(ctx.GetType(), ctx, (type, ctx2) => ctx2); // Don't override if already exists`
	`70`	`+ _processorContexts.AddOrUpdate(context.GetType(), context, (type, ctx) => ctx); // Don't override if already exists`
`71`	`71`	`}`
`72`	`72`
`73`	`73`	`/// <summary>`
Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,7 @@ namespace Our.Umbraco.Ditto`
`5`	`5`	`/// <summary>`
`6`	`6`	`/// The Umbraco properties attribute.`
`7`	`7`	`/// </summary>`
`8`		`- [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = false)]`
	`8`	`+ [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)]`
`9`	`9`	`public sealed class UmbracoPropertiesAttribute : Attribute`
`10`	`10`	`{`
`11`	`11`	`/// <summary>`
Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,5 @@`
`1`	`1`	`using System;`
`2`	`2`	`using System.Linq;`
`3`		`-using System.Reflection;`
`4`		`-using Umbraco.Core;`
`5`	`3`	`using Umbraco.Core.Logging;`
`6`	`4`	`using Umbraco.Core.Models;`
`7`	`5`	`using Umbraco.Web;`
`@@ -133,14 +131,14 @@ public override object ProcessValue()`
`133`	`131`	`}`
`134`	`132`
`135`	`133`	`// Try fetching the alt value.`
`136`		`- if ((propertyValue == null \|\| propertyValue.ToString().IsNullOrWhiteSpace())`
	`134`	`+ if ((propertyValue == null \|\| (propertyValue is string tmp && string.IsNullOrWhiteSpace(tmp)))`
`137`	`135`	`&& string.IsNullOrWhiteSpace(altUmbracoPropertyName) == false)`
`138`	`136`	`{`
`139`	`137`	`propertyValue = GetPropertyValue(content, altUmbracoPropertyName, recursive);`
`140`	`138`	`}`
`141`	`139`
`142`	`140`	`// Try setting the default value.`
`143`		`- if ((propertyValue == null \|\| propertyValue.ToString().IsNullOrWhiteSpace())`
	`141`	`+ if ((propertyValue == null \|\| (propertyValue is string tmp2 && string.IsNullOrWhiteSpace(tmp2)))`
`144`	`142`	`&& defaultValue != null)`
`145`	`143`	`{`
`146`	`144`	`propertyValue = defaultValue;`
`@@ -165,13 +163,13 @@ protected object GetPropertyValue(IPublishedContent content, string umbracoPrope`
`165`	`163`	`propertyValue = GetClassPropertyValue(content, umbracoPropertyName);`
`166`	`164`	`}`
`167`	`165`
`168`		`- if ((propertyValue == null \|\| propertyValue.ToString().IsNullOrWhiteSpace())`
	`166`	`+ if ((propertyValue == null \|\| (propertyValue is string tmp && string.IsNullOrWhiteSpace(tmp)))`
`169`	`167`	`&& (PropertySource != PropertySource.InstanceProperties))`
`170`	`168`	`{`
`171`	`169`	`propertyValue = GetUmbracoPropertyValue(content, umbracoPropertyName, recursive);`
`172`	`170`	`}`
`173`	`171`
`174`		`- if ((propertyValue == null \|\| propertyValue.ToString().IsNullOrWhiteSpace())`
	`172`	`+ if ((propertyValue == null \|\| (propertyValue is string tmp2 && string.IsNullOrWhiteSpace(tmp2)))`
`175`	`173`	`&& PropertySource == PropertySource.UmbracoThenInstanceProperties)`
`176`	`174`	`{`
`177`	`175`	`propertyValue = GetClassPropertyValue(content, umbracoPropertyName);`
`@@ -194,10 +192,10 @@ private object GetClassPropertyValue(IPublishedContent content, string umbracoPr`
`194`	`192`	`{`
`195`	`193`	`if (Ditto.IsDebuggingEnabled`
`196`	`194`	`&& PropertySource == PropertySource.InstanceThenUmbracoProperties`
`197`		`- && Ditto.IPublishedContentProperties.Any(x => x.Name.InvariantEquals(umbracoPropertyName))`
	`195`	`+ && Ditto.IPublishedContentProperties.Any(x => string.Equals(x.Name, umbracoPropertyName, StringComparison.InvariantCultureIgnoreCase))`
`198`	`196`	`&& content.HasProperty(umbracoPropertyName))`
`199`	`197`	`{`
`200`		`- // Property is an IPublishedContent property and an umbraco property exists so warn the user`
	`198`	`+ // Property is an IPublishedContent property and an Umbraco property exists so warn the user`
`201`	`199`	LogHelper.Warn<UmbracoPropertyAttribute>($"The property {umbracoPropertyName} being mapped from content type {contentType.Name}'s instance properties hides a property in the Umbraco properties collection of the same name. It is recommended that you avoid using Umbraco property aliases that conflict with IPublishedContent instance property names, but if you can't avoid this and you require access to the hidden property you can use the PropertySource parameter of the processors attribute to override the order in which properties are checked.");
`202`	`200`	`}`
`203`	`201`
`@@ -219,11 +217,11 @@ private object GetUmbracoPropertyValue(IPublishedContent content, string umbraco`
`219`	`217`	`{`
`220`	`218`	`if (Ditto.IsDebuggingEnabled`
`221`	`219`	`&& PropertySource == PropertySource.UmbracoThenInstanceProperties`
`222`		`- && Ditto.IPublishedContentProperties.Any(x => x.Name.InvariantEquals(umbracoPropertyName))`
	`220`	`+ && Ditto.IPublishedContentProperties.Any(x => string.Equals(x.Name, umbracoPropertyName, StringComparison.InvariantCultureIgnoreCase))`
`223`	`221`	`&& content.HasProperty(umbracoPropertyName))`
`224`	`222`	`{`
`225`		`- // Property is an IPublishedContent property and an umbraco property exists so warn the user`
`226`		- LogHelper.Warn<UmbracoPropertyAttribute>($"The property {umbracoPropertyName} being mapped from the Umbraco properties collection hides an instance property of the same name on content type {content.GetType().Name}. It is recommended that you avoid using Umbraco property aliases that conflict with IPublishedContent instance property names, but if you can't avoid this and you require access to the hidden property you can use the PropertySource parameter of the processors attribute to override the order in which properties are checked.");
	`223`	`+ // Property is an IPublishedContent property and an Umbraco property exists so warn the user`
	`224`	+ LogHelper.Warn<UmbracoPropertyAttribute>($"The property {umbracoPropertyName} being mapped from the Umbraco properties collection hides an instance property of the same name on content type {content}. It is recommended that you avoid using Umbraco property aliases that conflict with IPublishedContent instance property names, but if you can't avoid this and you require access to the hidden property you can use the PropertySource parameter of the processors attribute to override the order in which properties are checked.");
`227`	`225`	`}`
`228`	`226`
`229`	`227`	`return content.GetPropertyValue(umbracoPropertyName, recursive);`
Original file line number	Diff line number	Diff line change
`@@ -3,7 +3,6 @@`
`3`	`3`	`using System.Collections.ObjectModel;`
`4`	`4`	`using System.Linq;`
`5`	`5`	`using Moq;`
`6`		`-using Umbraco.Core;`
`7`	`6`	`using Umbraco.Core.Logging;`
`8`	`7`	`using Umbraco.Core.Models;`
`9`	`8`	`using Umbraco.Core.Models.PublishedContent;`
`@@ -50,7 +49,7 @@ public IPublishedProperty GetProperty(string alias)`
`50`	`49`
`51`	`50`	`public IPublishedProperty GetProperty(string alias, bool recurse)`
`52`	`51`	`{`
`53`		`- var prop = Properties.SingleOrDefault(p => p.PropertyTypeAlias.InvariantEquals(alias));`
	`52`	`+ var prop = Properties.FirstOrDefault(p => string.Equals(p.PropertyTypeAlias, alias, StringComparison.InvariantCultureIgnoreCase));`
`54`	`53`
`55`	`54`	`if (prop == null && recurse && Parent != null)`
`56`	`55`	`{`