diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 339cb25e05..13d6bd71c9 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -1,4 +1,6 @@ using System; +using System.Collections; +using System.Collections.Generic; using System.Linq; using System.Reflection; using System.Runtime.Serialization; @@ -363,7 +365,7 @@ namespace Umbraco.Core.Models /// Returns true if only the entity properties are direty /// /// - internal bool IsEntityDirty() + public bool IsEntityDirty() { return base.IsDirty(); } @@ -372,11 +374,20 @@ namespace Umbraco.Core.Models /// Returns true if any of the properties are dirty /// /// - internal bool IsAnyUserPropertyDirty() + public bool IsAnyUserPropertyDirty() { return Properties.Any(x => x.IsDirty()); } + /// + /// Returns a list of all dirty user defined properties + /// + /// + public IEnumerable GetDirtyUserProperties() + { + return Properties.Where(x => x.IsDirty()).Select(x => x.Alias); + } + /// /// Resets dirty properties by clearing the dictionary used to track changes. /// diff --git a/src/Umbraco.Core/Models/ContentExtensions.cs b/src/Umbraco.Core/Models/ContentExtensions.cs index c12d5c34d6..3c6a892900 100644 --- a/src/Umbraco.Core/Models/ContentExtensions.cs +++ b/src/Umbraco.Core/Models/ContentExtensions.cs @@ -42,6 +42,61 @@ namespace Umbraco.Core.Models return dirty.WasPropertyDirty("Published") && entity.Published; } + /// + /// Determines if the item should be persisted at all + /// + /// + /// + /// + /// In one particular case, a content item shouldn't be persisted: + /// * The item exists and is published + /// * A call to ContentService.Save is made + /// * The item has not been modified whatsoever apart from changing it's published status from published to saved + /// + /// In this case, there is no reason to make any database changes at all + /// + internal static bool RequiresSaving(this IContent entity) + { + var publishedState = ((Content)entity).PublishedState; + return RequiresSaving(entity, publishedState); + } + + /// + /// Determines if the item should be persisted at all + /// + /// + /// + /// + /// + /// In one particular case, a content item shouldn't be persisted: + /// * The item exists and is published + /// * A call to ContentService.Save is made + /// * The item has not been modified whatsoever apart from changing it's published status from published to saved + /// + /// In this case, there is no reason to make any database changes at all + /// + internal static bool RequiresSaving(this IContent entity, PublishedState publishedState) + { + var dirtyEntity = (ICanBeDirty)entity; + var publishedChanged = dirtyEntity.IsPropertyDirty("Published") && publishedState != PublishedState.Unpublished; + //check if any user prop has changed + var propertyValueChanged = ((Content)entity).IsAnyUserPropertyDirty(); + + //We need to know if any other property apart from Published was changed here + //don't create a new version if the published state has changed to 'Save' but no data has actually been changed + if (publishedChanged && entity.Published == false && propertyValueChanged == false) + { + //at this point we need to check if any non property value has changed that wasn't the published state + var changedProps = ((TracksChangesEntityBase)entity).GetDirtyProperties(); + if (changedProps.Any(x => x != "Published") == false) + { + return false; + } + } + + return true; + } + /// /// Determines if a new version should be created /// @@ -76,9 +131,12 @@ namespace Umbraco.Core.Models var dirtyEntity = (ICanBeDirty)entity; //check if the published state has changed or the language - var contentChanged = - (dirtyEntity.IsPropertyDirty("Published") && publishedState != PublishedState.Unpublished) - || dirtyEntity.IsPropertyDirty("Language"); + var publishedChanged = dirtyEntity.IsPropertyDirty("Published") && publishedState != PublishedState.Unpublished; + var langChanged = dirtyEntity.IsPropertyDirty("Language"); + var contentChanged = publishedChanged || langChanged; + + //check if any user prop has changed + var propertyValueChanged = ((Content)entity).IsAnyUserPropertyDirty(); //return true if published or language has changed if (contentChanged) @@ -86,8 +144,6 @@ namespace Umbraco.Core.Models return true; } - //check if any user prop has changed - var propertyValueChanged = ((Content)entity).IsAnyUserPropertyDirty(); //check if any content prop has changed var contentDataChanged = ((Content)entity).IsEntityDirty(); @@ -134,7 +190,7 @@ namespace Umbraco.Core.Models //If Published state has changed then previous versions should have their publish state reset. //If state has been changed to unpublished the previous versions publish state should also be reset. - if (((ICanBeDirty)entity).IsPropertyDirty("Published") && (entity.Published || publishedState == PublishedState.Unpublished)) + if (entity.IsPropertyDirty("Published") && (entity.Published || publishedState == PublishedState.Unpublished)) { return true; } diff --git a/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs b/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs index 8e95a1a7e2..2153b8f57f 100644 --- a/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs +++ b/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs @@ -14,6 +14,12 @@ namespace Umbraco.Core.Models.EntityBase [DataContract(IsReference = true)] public abstract class TracksChangesEntityBase : IRememberBeingDirty { + //TODO: This needs to go on to ICanBeDirty http://issues.umbraco.org/issue/U4-5662 + public virtual IEnumerable GetDirtyProperties() + { + return _propertyChangedInfo.Where(x => x.Value).Select(x => x.Key); + } + /// /// Tracks the properties that have changed /// @@ -139,11 +145,12 @@ 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 bool SetPropertyValueAndDetectChanges(Func setValue, T value, PropertyInfo propertySelector) + internal virtual bool SetPropertyValueAndDetectChanges(Func setValue, T value, PropertyInfo propertySelector) { var initVal = value; var newVal = setValue(value); - if (!Equals(initVal, newVal)) + + if (Equals(initVal, newVal) == false) { OnPropertyChanged(propertySelector); return true; diff --git a/src/Umbraco.Core/Models/Rdbms/PropertyDataDto.cs b/src/Umbraco.Core/Models/Rdbms/PropertyDataDto.cs index 7c16f920bf..4e59f0275b 100644 --- a/src/Umbraco.Core/Models/Rdbms/PropertyDataDto.cs +++ b/src/Umbraco.Core/Models/Rdbms/PropertyDataDto.cs @@ -75,7 +75,7 @@ namespace Umbraco.Core.Models.Rdbms return Text; } - return string.Empty; + return null; } } diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index c51bbc27f7..2abb9fe9de 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -4,6 +4,7 @@ using System.Data; using System.Globalization; using System.Linq; using System.Linq.Expressions; +using System.Net.Http.Headers; using System.Text; using System.Xml.Linq; using Umbraco.Core.Configuration; @@ -337,13 +338,20 @@ namespace Umbraco.Core.Persistence.Repositories UpdatePropertyTags(entity, _tagRepository); } - ((ICanBeDirty)entity).ResetDirtyProperties(); + entity.ResetDirtyProperties(); } protected override void PersistUpdatedItem(IContent entity) { var publishedState = ((Content) entity).PublishedState; + //check if we need to make any database changes at all + if (entity.RequiresSaving(publishedState) == false) + { + entity.ResetDirtyProperties(); + return; + } + //check if we need to create a new version bool shouldCreateNewVersion = entity.ShouldCreateNewVersion(publishedState); if (shouldCreateNewVersion) @@ -363,7 +371,7 @@ namespace Umbraco.Core.Persistence.Repositories entity.SanitizeEntityPropertiesForXmlStorage(); //Look up parent to get and set the correct Path and update SortOrder if ParentId has changed - if (((ICanBeDirty)entity).IsPropertyDirty("ParentId")) + if (entity.IsPropertyDirty("ParentId")) { var parent = Database.First("WHERE id = @ParentId", new { ParentId = entity.ParentId }); entity.Path = string.Concat(parent.Path, ",", entity.Id); @@ -487,7 +495,7 @@ namespace Umbraco.Core.Persistence.Repositories ClearEntityTags(entity, _tagRepository); } - ((ICanBeDirty)entity).ResetDirtyProperties(); + entity.ResetDirtyProperties(); } diff --git a/src/Umbraco.Core/PropertyEditors/PropertyValueEditor.cs b/src/Umbraco.Core/PropertyEditors/PropertyValueEditor.cs index 6ba084bb8c..0e66e8d13c 100644 --- a/src/Umbraco.Core/PropertyEditors/PropertyValueEditor.cs +++ b/src/Umbraco.Core/PropertyEditors/PropertyValueEditor.cs @@ -169,7 +169,6 @@ namespace Umbraco.Core.PropertyEditors /// internal Attempt TryConvertValueToCrlType(object value) { - //this is a custom check to avoid any errors, if it's a string and it's empty just make it null var s = value as string; if (s != null) @@ -194,7 +193,15 @@ namespace Umbraco.Core.PropertyEditors // instead of int. Even though our db will not support this (will get truncated), we'll at least parse to this. valueType = typeof(long?); - break; + + //if parsing is successful, we need to return as an Int, we're only dealing with long's here because of json.net, we actually + //don't support long values and if we return a long value it will get set as a 'long' on the Property.Value (object) and then + //when we compare the values for dirty tracking we'll be comparing an int -> long and they will not match. + var result = value.TryConvertTo(valueType); + return result.Success && result.Result != null + ? Attempt.Succeed((int)(long)result.Result) + : result; + case DataTypeDatabaseType.Date: //ensure these are nullable so we can return a null if required valueType = typeof(DateTime?); diff --git a/src/Umbraco.Tests/Models/PropertyTypeTests.cs b/src/Umbraco.Tests/Models/PropertyTypeTests.cs index a623fe6444..e6a89a1717 100644 --- a/src/Umbraco.Tests/Models/PropertyTypeTests.cs +++ b/src/Umbraco.Tests/Models/PropertyTypeTests.cs @@ -2,11 +2,12 @@ using System; using NUnit.Framework; using Umbraco.Core.Models; using Umbraco.Core.Serialization; +using Umbraco.Tests.TestHelpers; namespace Umbraco.Tests.Models { [TestFixture] - public class PropertyTypeTests + public class PropertyTypeTests : BaseUmbracoConfigurationTest { [Test] public void Can_Deep_Clone()