From 42f0d2b356f02de5e6d156324fa2776100254f6f Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 25 Oct 2018 15:47:25 +1100 Subject: [PATCH] Fixes dirty tracking with resetting properties --- src/Umbraco.Core/Models/Content.cs | 23 ++++++--------- src/Umbraco.Core/Models/ContentBase.cs | 29 ++++++++----------- src/Umbraco.Core/Models/ContentTypeBase.cs | 28 +++++++----------- .../Models/ContentTypeCompositionBase.cs | 19 +++++------- .../Models/DictionaryTranslation.cs | 19 ++++-------- .../Models/Entities/EntityBase.cs | 21 ++++++++++---- src/Umbraco.Core/Models/File.cs | 21 ++++---------- src/Umbraco.Core/Models/Macro.cs | 24 +++++++-------- src/Umbraco.Core/Models/Member.cs | 19 +++++------- src/Umbraco.Core/Models/Membership/User.cs | 29 ++++++++----------- src/Umbraco.Core/Models/Property.cs | 15 +++------- src/Umbraco.Core/Models/PropertyGroup.cs | 20 +++++-------- src/Umbraco.Core/Models/PropertyType.cs | 17 ++++------- src/Umbraco.Core/Models/PublicAccessEntry.cs | 18 ++++-------- 14 files changed, 117 insertions(+), 185 deletions(-) diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 3e5becf021..d46e318aea 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -499,28 +499,23 @@ namespace Umbraco.Core.Models return clone; } - public override object DeepClone() + protected override void PerformDeepClone(object clone) { - var clone = (Content) base.DeepClone(); + base.PerformDeepClone(clone); - //turn off change tracking - clone.DisableChangeTracking(); + var clonedContent = (Content)clone; //need to manually clone this since it's not settable - clone._contentType = (IContentType) ContentType.DeepClone(); + clonedContent._contentType = (IContentType) ContentType.DeepClone(); //if culture infos exist then deal with event bindings - if (clone._publishInfos != null) + if (clonedContent._publishInfos != null) { - clone._publishInfos.CollectionChanged -= PublishNamesCollectionChanged; //clear this event handler if any - clone._publishInfos = (ContentCultureInfosCollection) _publishInfos.DeepClone(); //manually deep clone - clone._publishInfos.CollectionChanged += clone.PublishNamesCollectionChanged; //re-assign correct event handler + clonedContent._publishInfos.CollectionChanged -= PublishNamesCollectionChanged; //clear this event handler if any + clonedContent._publishInfos = (ContentCultureInfosCollection) _publishInfos.DeepClone(); //manually deep clone + clonedContent._publishInfos.CollectionChanged += clonedContent.PublishNamesCollectionChanged; //re-assign correct event handler } - - //re-enable tracking - clone.EnableChangeTracking(); - - return clone; + } } } diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index 863374726d..7e70238d2f 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -475,33 +475,28 @@ namespace Umbraco.Core.Models /// /// Overriden to deal with specific object instances /// - public override object DeepClone() + protected override void PerformDeepClone(object clone) { - var clone = (ContentBase) base.DeepClone(); + base.PerformDeepClone(clone); - //turn off change tracking - clone.DisableChangeTracking(); + var clonedContent = (ContentBase)clone; //if culture infos exist then deal with event bindings - if (clone._cultureInfos != null) + if (clonedContent._cultureInfos != null) { - clone._cultureInfos.CollectionChanged -= CultureInfosCollectionChanged; //clear this event handler if any - clone._cultureInfos = (ContentCultureInfosCollection) _cultureInfos.DeepClone(); //manually deep clone - clone._cultureInfos.CollectionChanged += clone.CultureInfosCollectionChanged; //re-assign correct event handler + clonedContent._cultureInfos.CollectionChanged -= CultureInfosCollectionChanged; //clear this event handler if any + clonedContent._cultureInfos = (ContentCultureInfosCollection) _cultureInfos.DeepClone(); //manually deep clone + clonedContent._cultureInfos.CollectionChanged += clonedContent.CultureInfosCollectionChanged; //re-assign correct event handler } //if properties exist then deal with event bindings - if (clone._properties != null) + if (clonedContent._properties != null) { - clone._properties.CollectionChanged -= PropertiesChanged; //clear this event handler if any - clone._properties = (PropertyCollection) _properties.DeepClone(); //manually deep clone - clone._properties.CollectionChanged += clone.PropertiesChanged; //re-assign correct event handler + clonedContent._properties.CollectionChanged -= PropertiesChanged; //clear this event handler if any + clonedContent._properties = (PropertyCollection) _properties.DeepClone(); //manually deep clone + clonedContent._properties.CollectionChanged += clonedContent.PropertiesChanged; //re-assign correct event handler } - - //re-enable tracking - clone.EnableChangeTracking(); - - return clone; + } } } diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 9f848c6d14..caa63d7526 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -467,35 +467,29 @@ namespace Umbraco.Core.Models } } - public override object DeepClone() + protected override void PerformDeepClone(object clone) { - var clone = (ContentTypeBase) base.DeepClone(); + base.PerformDeepClone(clone); - //turn off change tracking - clone.DisableChangeTracking(); + var clonedEntity = (ContentTypeBase) clone; - if (clone._noGroupPropertyTypes != null) + if (clonedEntity._noGroupPropertyTypes != null) { //need to manually wire up the event handlers for the property type collections - we've ensured // its ignored from the auto-clone process because its return values are unions, not raw and // we end up with duplicates, see: http://issues.umbraco.org/issue/U4-4842 - clone._noGroupPropertyTypes.CollectionChanged -= PropertyTypesChanged; //clear this event handler if any - clone._noGroupPropertyTypes = (PropertyTypeCollection) _noGroupPropertyTypes.DeepClone(); //manually deep clone - clone._noGroupPropertyTypes.CollectionChanged += clone.PropertyTypesChanged; //re-assign correct event handler + clonedEntity._noGroupPropertyTypes.CollectionChanged -= PropertyTypesChanged; //clear this event handler if any + clonedEntity._noGroupPropertyTypes = (PropertyTypeCollection) _noGroupPropertyTypes.DeepClone(); //manually deep clone + clonedEntity._noGroupPropertyTypes.CollectionChanged += clonedEntity.PropertyTypesChanged; //re-assign correct event handler } - if (clone._propertyGroups != null) + if (clonedEntity._propertyGroups != null) { - clone._propertyGroups.CollectionChanged -= PropertyGroupsChanged; //clear this event handler if any - clone._propertyGroups = (PropertyGroupCollection) _propertyGroups.DeepClone(); //manually deep clone - clone._propertyGroups.CollectionChanged += clone.PropertyGroupsChanged; //re-assign correct event handler + clonedEntity._propertyGroups.CollectionChanged -= PropertyGroupsChanged; //clear this event handler if any + clonedEntity._propertyGroups = (PropertyGroupCollection) _propertyGroups.DeepClone(); //manually deep clone + clonedEntity._propertyGroups.CollectionChanged += clonedEntity.PropertyGroupsChanged; //re-assign correct event handler } - - //re-enable tracking - clone.EnableChangeTracking(); - - return clone; } public IContentType DeepCloneWithResetIdentities(string alias) diff --git a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs index 838a75b98b..cf43b661c7 100644 --- a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs @@ -290,20 +290,15 @@ namespace Umbraco.Core.Models .Union(ContentTypeComposition.SelectMany(x => x.CompositionIds())); } - public override object DeepClone() + protected override void PerformDeepClone(object clone) { - var clone = (ContentTypeCompositionBase)base.DeepClone(); - //turn off change tracking - clone.DisableChangeTracking(); - //need to manually assign since this is an internal field and will not be automatically mapped - clone.RemovedContentTypeKeyTracker = new List(); - clone._contentTypeComposition = ContentTypeComposition.Select(x => (IContentTypeComposition)x.DeepClone()).ToList(); - //this shouldn't really be needed since we're not tracking - clone.ResetDirtyProperties(false); - //re-enable tracking - clone.EnableChangeTracking(); + base.PerformDeepClone(clone); - return clone; + var clonedEntity = (ContentTypeCompositionBase)clone; + + //need to manually assign since this is an internal field and will not be automatically mapped + clonedEntity.RemovedContentTypeKeyTracker = new List(); + clonedEntity._contentTypeComposition = ContentTypeComposition.Select(x => (IContentTypeComposition)x.DeepClone()).ToList(); } } } diff --git a/src/Umbraco.Core/Models/DictionaryTranslation.cs b/src/Umbraco.Core/Models/DictionaryTranslation.cs index 2105e8057c..c3b5a8a3b2 100644 --- a/src/Umbraco.Core/Models/DictionaryTranslation.cs +++ b/src/Umbraco.Core/Models/DictionaryTranslation.cs @@ -104,23 +104,14 @@ namespace Umbraco.Core.Models set { SetPropertyValueAndDetectChanges(value, ref _value, Ps.Value.ValueSelector); } } - public override object DeepClone() + protected override void PerformDeepClone(object clone) { - var clone = (DictionaryTranslation)base.DeepClone(); + base.PerformDeepClone(clone); + + var clonedEntity = (DictionaryTranslation)clone; // clear fields that were memberwise-cloned and that we don't want to clone - clone._language = null; - - // turn off change tracking - clone.DisableChangeTracking(); - - // this shouldn't really be needed since we're not tracking - clone.ResetDirtyProperties(false); - - // re-enable tracking - clone.EnableChangeTracking(); - - return clone; + clonedEntity._language = null; } } } diff --git a/src/Umbraco.Core/Models/Entities/EntityBase.cs b/src/Umbraco.Core/Models/Entities/EntityBase.cs index 0b69586abf..5c6f943c60 100644 --- a/src/Umbraco.Core/Models/Entities/EntityBase.cs +++ b/src/Umbraco.Core/Models/Entities/EntityBase.cs @@ -159,7 +159,7 @@ namespace Umbraco.Core.Models.Entities } } - public virtual object DeepClone() + public object DeepClone() { // memberwise-clone (ie shallow clone) the entity var unused = Key; // ensure that 'this' has a key, before cloning @@ -169,20 +169,29 @@ namespace Umbraco.Core.Models.Entities clone.InstanceId = Guid.NewGuid(); #endif - // clear changes (ensures the clone has its own dictionaries) - // then disable change tracking - clone.ResetDirtyProperties(false); + //disable change tracking while we deep clone IDeepCloneable properties clone.DisableChangeTracking(); // deep clone ref properties that are IDeepCloneable DeepCloneHelper.DeepCloneRefProperties(this, clone); - // clear changes again (just to be sure, because we were not tracking) - // then enable change tracking + PerformDeepClone(clone); + + // clear changes (ensures the clone has its own dictionaries) clone.ResetDirtyProperties(false); + + //re-enable change tracking clone.EnableChangeTracking(); return clone; } + + /// + /// Used by inheritors to modify the DeepCloning logic + /// + /// + protected virtual void PerformDeepClone(object clone) + { + } } } diff --git a/src/Umbraco.Core/Models/File.cs b/src/Umbraco.Core/Models/File.cs index 2e85b13261..2f8e021f4c 100644 --- a/src/Umbraco.Core/Models/File.cs +++ b/src/Umbraco.Core/Models/File.cs @@ -156,26 +156,17 @@ namespace Umbraco.Core.Models clone._alias = Alias; } - public override object DeepClone() + protected override void PerformDeepClone(object clone) { - var clone = (File) base.DeepClone(); + base.PerformDeepClone(clone); + + var clonedFile = (File)clone; // clear fields that were memberwise-cloned and that we don't want to clone - clone._content = null; - - // turn off change tracking - clone.DisableChangeTracking(); + clonedFile._content = null; // ... - DeepCloneNameAndAlias(clone); - - // this shouldn't really be needed since we're not tracking - clone.ResetDirtyProperties(false); - - // re-enable tracking - clone.EnableChangeTracking(); - - return clone; + DeepCloneNameAndAlias(clonedFile); } } } diff --git a/src/Umbraco.Core/Models/Macro.cs b/src/Umbraco.Core/Models/Macro.cs index 6e68bda439..5ef49305ac 100644 --- a/src/Umbraco.Core/Models/Macro.cs +++ b/src/Umbraco.Core/Models/Macro.cs @@ -284,22 +284,18 @@ namespace Umbraco.Core.Models get { return _properties; } } - public override object DeepClone() + protected override void PerformDeepClone(object clone) { - var clone = (Macro)base.DeepClone(); - //turn off change tracking - clone.DisableChangeTracking(); - clone._addedProperties = new List(); - clone._removedProperties = new List(); - clone._properties = (MacroPropertyCollection)Properties.DeepClone(); - //re-assign the event handler - clone._properties.CollectionChanged += clone.PropertiesChanged; - //this shouldn't really be needed since we're not tracking - clone.ResetDirtyProperties(false); - //re-enable tracking - clone.EnableChangeTracking(); + base.PerformDeepClone(clone); - return clone; + var clonedEntity = (Macro)clone; + + clonedEntity._addedProperties = new List(); + clonedEntity._removedProperties = new List(); + clonedEntity._properties = (MacroPropertyCollection)Properties.DeepClone(); + //re-assign the event handler + clonedEntity._properties.CollectionChanged += clonedEntity.PropertiesChanged; + } } } diff --git a/src/Umbraco.Core/Models/Member.cs b/src/Umbraco.Core/Models/Member.cs index 7576f01ce0..38927898cf 100644 --- a/src/Umbraco.Core/Models/Member.cs +++ b/src/Umbraco.Core/Models/Member.cs @@ -598,20 +598,15 @@ namespace Umbraco.Core.Models return true; } - public override object DeepClone() + protected override void PerformDeepClone(object clone) { - var clone = (Member)base.DeepClone(); - //turn off change tracking - clone.DisableChangeTracking(); + base.PerformDeepClone(clone); + + var clonedEntity = (Member)clone; + //need to manually clone this since it's not settable - clone._contentType = (IMemberType)ContentType.DeepClone(); - //this shouldn't really be needed since we're not tracking - clone.ResetDirtyProperties(false); - //re-enable tracking - clone.EnableChangeTracking(); - - return clone; - + clonedEntity._contentType = (IMemberType)ContentType.DeepClone(); + } /// diff --git a/src/Umbraco.Core/Models/Membership/User.cs b/src/Umbraco.Core/Models/Membership/User.cs index 9066674193..0694194996 100644 --- a/src/Umbraco.Core/Models/Membership/User.cs +++ b/src/Umbraco.Core/Models/Membership/User.cs @@ -448,18 +448,19 @@ namespace Umbraco.Core.Models.Membership [DoNotClone] internal object AdditionalDataLock { get { return _additionalDataLock; } } - public override object DeepClone() + protected override void PerformDeepClone(object clone) { - var clone = (User)base.DeepClone(); - //turn off change tracking - clone.DisableChangeTracking(); + base.PerformDeepClone(clone); + + var clonedEntity = (User)clone; + //manually clone the start node props - clone._startContentIds = _startContentIds.ToArray(); - clone._startMediaIds = _startMediaIds.ToArray(); + clonedEntity._startContentIds = _startContentIds.ToArray(); + clonedEntity._startMediaIds = _startMediaIds.ToArray(); // this value has been cloned and points to the same object // which obviously is bad - needs to point to a new object - clone._additionalDataLock = new object(); + clonedEntity._additionalDataLock = new object(); if (_additionalData != null) { @@ -467,7 +468,7 @@ namespace Umbraco.Core.Models.Membership // changing one clone impacts all of them - so we need to reset it with a fresh // dictionary that will contain the same values - and, if some values are deep // cloneable, they should be deep-cloned too - var cloneAdditionalData = clone._additionalData = new Dictionary(); + var cloneAdditionalData = clonedEntity._additionalData = new Dictionary(); lock (_additionalDataLock) { @@ -480,15 +481,9 @@ namespace Umbraco.Core.Models.Membership } //need to create new collections otherwise they'll get copied by ref - clone._userGroups = new HashSet(_userGroups); - clone._allowedSections = _allowedSections != null ? new List(_allowedSections) : null; - //re-create the event handler - //this shouldn't really be needed since we're not tracking - clone.ResetDirtyProperties(false); - //re-enable tracking - clone.EnableChangeTracking(); - - return clone; + clonedEntity._userGroups = new HashSet(_userGroups); + clonedEntity._allowedSections = _allowedSections != null ? new List(_allowedSections) : null; + } /// diff --git a/src/Umbraco.Core/Models/Property.cs b/src/Umbraco.Core/Models/Property.cs index 8a97dc2cfc..0c71544111 100644 --- a/src/Umbraco.Core/Models/Property.cs +++ b/src/Umbraco.Core/Models/Property.cs @@ -392,21 +392,14 @@ namespace Umbraco.Core.Models return PropertyType.IsPropertyValueValid(value); } - public override object DeepClone() + protected override void PerformDeepClone(object clone) { - var clone = (Property) base.DeepClone(); + base.PerformDeepClone(clone); - //turn off change tracking - clone.DisableChangeTracking(); + var clonedEntity = (Property)clone; //need to manually assign since this is a readonly property - clone.PropertyType = (PropertyType) PropertyType.DeepClone(); - - //re-enable tracking - clone.ResetDirtyProperties(false); // not needed really, since we're not tracking - clone.EnableChangeTracking(); - - return clone; + clonedEntity.PropertyType = (PropertyType) PropertyType.DeepClone(); } } } diff --git a/src/Umbraco.Core/Models/PropertyGroup.cs b/src/Umbraco.Core/Models/PropertyGroup.cs index 6c1f2e5c61..1d0b949932 100644 --- a/src/Umbraco.Core/Models/PropertyGroup.cs +++ b/src/Umbraco.Core/Models/PropertyGroup.cs @@ -100,24 +100,18 @@ namespace Umbraco.Core.Models return baseHash ^ nameHash; } - public override object DeepClone() + protected override void PerformDeepClone(object clone) { - var clone = (PropertyGroup)base.DeepClone(); + base.PerformDeepClone(clone); - //turn off change tracking - clone.DisableChangeTracking(); + var clonedEntity = (PropertyGroup)clone; - if (clone._propertyTypes != null) + if (clonedEntity._propertyTypes != null) { - clone._propertyTypes.CollectionChanged -= PropertyTypesChanged; //clear this event handler if any - clone._propertyTypes = (PropertyTypeCollection) _propertyTypes.DeepClone(); //manually deep clone - clone._propertyTypes.CollectionChanged += clone.PropertyTypesChanged; //re-assign correct event handler + clonedEntity._propertyTypes.CollectionChanged -= PropertyTypesChanged; //clear this event handler if any + clonedEntity._propertyTypes = (PropertyTypeCollection) _propertyTypes.DeepClone(); //manually deep clone + clonedEntity._propertyTypes.CollectionChanged += clonedEntity.PropertyTypesChanged; //re-assign correct event handler } - - //re-enable tracking - clone.EnableChangeTracking(); - - return clone; } } } diff --git a/src/Umbraco.Core/Models/PropertyType.cs b/src/Umbraco.Core/Models/PropertyType.cs index a34fdb04ed..d44e7d464f 100644 --- a/src/Umbraco.Core/Models/PropertyType.cs +++ b/src/Umbraco.Core/Models/PropertyType.cs @@ -424,22 +424,17 @@ namespace Umbraco.Core.Models } /// - public override object DeepClone() + protected override void PerformDeepClone(object clone) { - var clone = (PropertyType)base.DeepClone(); - //turn off change tracking - clone.DisableChangeTracking(); + base.PerformDeepClone(clone); + + var clonedEntity = (PropertyType)clone; + //need to manually assign the Lazy value as it will not be automatically mapped if (PropertyGroupId != null) { - clone._propertyGroupId = new Lazy(() => PropertyGroupId.Value); + clonedEntity._propertyGroupId = new Lazy(() => PropertyGroupId.Value); } - //this shouldn't really be needed since we're not tracking - clone.ResetDirtyProperties(false); - //re-enable tracking - clone.EnableChangeTracking(); - - return clone; } } } diff --git a/src/Umbraco.Core/Models/PublicAccessEntry.cs b/src/Umbraco.Core/Models/PublicAccessEntry.cs index 7fd0849e27..e93dc56e35 100644 --- a/src/Umbraco.Core/Models/PublicAccessEntry.cs +++ b/src/Umbraco.Core/Models/PublicAccessEntry.cs @@ -153,23 +153,17 @@ namespace Umbraco.Core.Models } } - public override object DeepClone() + protected override void PerformDeepClone(object clone) { - var clone = (PublicAccessEntry) base.DeepClone(); + base.PerformDeepClone(clone); - //turn off change tracking - clone.DisableChangeTracking(); + var cloneEntity = (PublicAccessEntry)clone; - if (clone._ruleCollection != null) + if (cloneEntity._ruleCollection != null) { - clone._ruleCollection.CollectionChanged -= _ruleCollection_CollectionChanged; //clear this event handler if any - clone._ruleCollection.CollectionChanged += clone._ruleCollection_CollectionChanged; //re-assign correct event handler + cloneEntity._ruleCollection.CollectionChanged -= _ruleCollection_CollectionChanged; //clear this event handler if any + cloneEntity._ruleCollection.CollectionChanged += cloneEntity._ruleCollection_CollectionChanged; //re-assign correct event handler } - - //re-enable tracking - clone.EnableChangeTracking(); - - return clone; } } }