From 46212904ef797f398aba16cd3ed6f97c2745b77a Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 5 Mar 2015 17:08:58 +1100 Subject: [PATCH] Changes the SetPropertyValueAndDetectChanges to throw an exception if an Enumerable is passed in without using the overload the specifies an IEqualityComparer. Updates all entities that use this method with IEnumerables to specify the correct Equality comparer which now uses the new UnsortedSequenceEqual method. Added unit tests for new EnumerableExtensions and makes the ContainsAll work faster. --- src/Umbraco.Core/EnumerableExtensions.cs | 43 ++++++++++++++----- src/Umbraco.Core/Models/ContentType.cs | 6 ++- src/Umbraco.Core/Models/ContentTypeBase.cs | 6 ++- src/Umbraco.Core/Models/DictionaryItem.cs | 6 ++- .../EntityBase/TracksChangesEntityBase.cs | 34 ++++++++++++++- .../Models/Membership/UserType.cs | 7 ++- src/Umbraco.Core/Models/Property.cs | 21 ++++++++- .../EnumerableExtensionsTests.cs | 29 +++++++++++++ 8 files changed, 134 insertions(+), 18 deletions(-) diff --git a/src/Umbraco.Core/EnumerableExtensions.cs b/src/Umbraco.Core/EnumerableExtensions.cs index 8055b33ab8..79b48af3ab 100644 --- a/src/Umbraco.Core/EnumerableExtensions.cs +++ b/src/Umbraco.Core/EnumerableExtensions.cs @@ -1,4 +1,5 @@ using System; +using System.Collections; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; @@ -114,13 +115,7 @@ namespace Umbraco.Core /// public static bool ContainsAll(this IEnumerable source, IEnumerable other) { - var matches = true; - foreach (var i in other) - { - matches = source.Contains(i); - if (!matches) break; - } - return matches; + return other.Except(source).Any() == false; } /// @@ -132,7 +127,7 @@ namespace Umbraco.Core /// public static bool ContainsAny(this IEnumerable source, IEnumerable other) { - return other.Any(i => source.Contains(i)); + return other.Any(source.Contains); } /// @@ -224,7 +219,7 @@ namespace Umbraco.Core return sequence.Select( x => { - if (typeof(TActual).IsAssignableFrom(x.GetType())) + if (x is TActual) { var casted = x as TActual; projection.Invoke(casted); @@ -276,6 +271,34 @@ namespace Umbraco.Core ///The enumerable to search. ///The item to find. ///The index of the first matching item, or -1 if the item was not found. - public static int IndexOf(this IEnumerable items, T item) { return items.FindIndex(i => EqualityComparer.Default.Equals(item, i)); } + public static int IndexOf(this IEnumerable items, T item) + { + return items.FindIndex(i => EqualityComparer.Default.Equals(item, i)); + } + + /// + /// Determines if 2 lists have equal elements within them regardless of how they are sorted + /// + /// + /// + /// + /// + /// + /// The logic for this is taken from: + /// http://stackoverflow.com/questions/4576723/test-whether-two-ienumerablet-have-the-same-values-with-the-same-frequencies + /// + /// There's a few answers, this one seems the best for it's simplicity and based on the comment of Eamon + /// + public static bool UnsortedSequenceEqual(this IEnumerable source, IEnumerable other) + { + if (source == null && other == null) return true; + if (source == null || other == null) return false; + + var list1Groups = source.ToLookup(i => i); + var list2Groups = other.ToLookup(i => i); + return list1Groups.Count == list2Groups.Count + && list1Groups.All(g => g.Count() == list2Groups[g.Key].Count()); + } + } } diff --git a/src/Umbraco.Core/Models/ContentType.cs b/src/Umbraco.Core/Models/ContentType.cs index fc53a21c3f..355d724fbe 100644 --- a/src/Umbraco.Core/Models/ContentType.cs +++ b/src/Umbraco.Core/Models/ContentType.cs @@ -90,7 +90,11 @@ namespace Umbraco.Core.Models { _allowedTemplates = value; return _allowedTemplates; - }, _allowedTemplates, AllowedTemplatesSelector); + }, _allowedTemplates, AllowedTemplatesSelector, + //Custom comparer for enumerable + new DelegateEqualityComparer>( + (templates, enumerable) => templates.UnsortedSequenceEqual(enumerable), + templates => templates.GetHashCode())); } } diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 07c2067022..bee9ed9b6e 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -351,7 +351,11 @@ namespace Umbraco.Core.Models { _allowedContentTypes = value; return _allowedContentTypes; - }, _allowedContentTypes, AllowedContentTypesSelector); + }, _allowedContentTypes, AllowedContentTypesSelector, + //Custom comparer for enumerable + new DelegateEqualityComparer>( + (sorts, enumerable) => sorts.UnsortedSequenceEqual(enumerable), + sorts => sorts.GetHashCode())); } } diff --git a/src/Umbraco.Core/Models/DictionaryItem.cs b/src/Umbraco.Core/Models/DictionaryItem.cs index 6c2ee47714..17cc744bd5 100644 --- a/src/Umbraco.Core/Models/DictionaryItem.cs +++ b/src/Umbraco.Core/Models/DictionaryItem.cs @@ -80,7 +80,11 @@ namespace Umbraco.Core.Models { _translations = value; return _translations; - }, _translations, TranslationsSelector); + }, _translations, TranslationsSelector, + //Custom comparer for enumerable + new DelegateEqualityComparer>( + (enumerable, translations) => enumerable.UnsortedSequenceEqual(translations), + enumerable => enumerable.GetHashCode())); } } diff --git a/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs b/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs index 7ab1b47ebb..cd5b762f84 100644 --- a/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs +++ b/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs @@ -1,4 +1,5 @@ using System; +using System.Collections; using System.Collections.Generic; using System.ComponentModel; using System.Linq; @@ -166,7 +167,36 @@ namespace Umbraco.Core.Models.EntityBase /// save a document type, nearly all properties are flagged as dirty just because we've 'reset' them, but they are all set /// to the same value, so it's really not dirty. /// - internal virtual bool SetPropertyValueAndDetectChanges(Func setValue, T value, PropertyInfo propertySelector) + internal bool SetPropertyValueAndDetectChanges(Func setValue, T value, PropertyInfo propertySelector) + { + if ((typeof(T) == typeof(string) == false) && TypeHelper.IsTypeAssignableFrom(typeof(T))) + { + throw new InvalidOperationException("This method does not support IEnumerable instances. For IEnumerable instances a manual custom equality check will be required"); + } + + return SetPropertyValueAndDetectChanges(setValue, value, propertySelector, + new DelegateEqualityComparer( + //Standard Equals comparison + (arg1, arg2) => Equals(arg1, arg2), + arg => arg.GetHashCode())); + + } + + /// + /// Used by inheritors to set the value of properties, this will detect if the property value actually changed and if it did + /// it will ensure that the property has a dirty flag set. + /// + /// + /// + /// + /// The equality comparer to use + /// returns true if the value changed + /// + /// This is required because we don't want a property to show up as "dirty" if the value is the same. For example, when we + /// save a document type, nearly all properties are flagged as dirty just because we've 'reset' them, but they are all set + /// to the same value, so it's really not dirty. + /// + internal bool SetPropertyValueAndDetectChanges(Func setValue, T value, PropertyInfo propertySelector, IEqualityComparer comparer) { var initVal = value; var newVal = setValue(value); @@ -174,7 +204,7 @@ namespace Umbraco.Core.Models.EntityBase //don't track changes, just set the value (above) if (_changeTrackingEnabled == false) return false; - if (Equals(initVal, newVal) == false) + if (comparer.Equals(initVal, newVal) == false) { OnPropertyChanged(propertySelector); return true; diff --git a/src/Umbraco.Core/Models/Membership/UserType.cs b/src/Umbraco.Core/Models/Membership/UserType.cs index 915943be43..5c32e57f38 100644 --- a/src/Umbraco.Core/Models/Membership/UserType.cs +++ b/src/Umbraco.Core/Models/Membership/UserType.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Reflection; using System.Runtime.Serialization; using Umbraco.Core.Models.EntityBase; @@ -67,7 +68,11 @@ namespace Umbraco.Core.Models.Membership { _permissions = value; return _permissions; - }, _permissions, PermissionsSelector); + }, _permissions, PermissionsSelector, + //Custom comparer for enumerable + new DelegateEqualityComparer>( + (enum1, enum2) => enum1.UnsortedSequenceEqual(enum2), + enum1 => enum1.GetHashCode())); } } } diff --git a/src/Umbraco.Core/Models/Property.cs b/src/Umbraco.Core/Models/Property.cs index 4f8fa3deb2..8ba30665d0 100644 --- a/src/Umbraco.Core/Models/Property.cs +++ b/src/Umbraco.Core/Models/Property.cs @@ -1,4 +1,7 @@ using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; using System.Reflection; using System.Runtime.Serialization; using Umbraco.Core.Models.EntityBase; @@ -120,7 +123,7 @@ namespace Umbraco.Core.Models { bool typeValidation = _propertyType.IsPropertyTypeValid(value); - if (!typeValidation) + if (typeValidation == false) throw new Exception( string.Format( "Type validation failed. The value type: '{0}' does not match the DataType in PropertyType with alias: '{1}'", @@ -130,7 +133,21 @@ namespace Umbraco.Core.Models { _value = value; return _value; - }, _value, ValueSelector); + }, _value, ValueSelector, + new DelegateEqualityComparer( + (o, o1) => + { + //Custom comparer for enumerable if it is enumerable + if (o == null && o1 == null) return true; + if (o == null || o1 == null) return false; + var enum1 = o as IEnumerable; + var enum2 = o1 as IEnumerable; + if (enum1 != null && enum2 != null) + { + return enum1.Cast().UnsortedSequenceEqual(enum2.Cast()); + } + return o.Equals(o1); + }, o => o.GetHashCode())); } } diff --git a/src/Umbraco.Tests/EnumerableExtensionsTests.cs b/src/Umbraco.Tests/EnumerableExtensionsTests.cs index 0fd436fcea..bbc0420ccc 100644 --- a/src/Umbraco.Tests/EnumerableExtensionsTests.cs +++ b/src/Umbraco.Tests/EnumerableExtensionsTests.cs @@ -11,6 +11,35 @@ namespace Umbraco.Tests public class EnumerableExtensionsTests { + [Test] + public void Unsorted_Sequence_Equal() + { + var list1 = new[] { 1, 2, 3, 4, 5, 6 }; + var list2 = new[] { 6, 5, 3, 2, 1, 4 }; + var list3 = new[] { 6, 5, 4, 3, 2, 2 }; + + Assert.IsTrue(list1.UnsortedSequenceEqual(list2)); + Assert.IsTrue(list2.UnsortedSequenceEqual(list1)); + Assert.IsFalse(list1.UnsortedSequenceEqual(list3)); + + Assert.IsTrue(((IEnumerable)null).UnsortedSequenceEqual(null)); + Assert.IsFalse(((IEnumerable)null).UnsortedSequenceEqual(list1)); + Assert.IsFalse(list1.UnsortedSequenceEqual(null)); + } + + [Test] + public void Contains_All() + { + var list1 = new[] {1, 2, 3, 4, 5, 6}; + var list2 = new[] {6, 5, 3, 2, 1, 4}; + var list3 = new[] {6, 5, 4, 3}; + + Assert.IsTrue(list1.ContainsAll(list2)); + Assert.IsTrue(list2.ContainsAll(list1)); + Assert.IsTrue(list1.ContainsAll(list3)); + Assert.IsFalse(list3.ContainsAll(list1)); + } + [Test] public void Flatten_List_2() {