From 3047d04f02eeb9d08f3e4772955932897e7e5a39 Mon Sep 17 00:00:00 2001 From: "Morten@Thinkpad-X220" Date: Wed, 10 Oct 2012 13:18:14 -0200 Subject: [PATCH] Some improvements around validation of content/media properties --- src/Umbraco.Core/Models/ContentBase.cs | 9 ++ src/Umbraco.Core/Models/IContentBase.cs | 6 ++ src/Umbraco.Core/Models/Property.cs | 41 +++++--- src/Umbraco.Tests/App.config | 12 +++ .../Services/ContentServiceTests.cs | 99 +++++++++++++++++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + src/Umbraco.Web/Services/ContentService.cs | 25 ++++- 7 files changed, 174 insertions(+), 19 deletions(-) create mode 100644 src/Umbraco.Tests/Services/ContentServiceTests.cs diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index cb18a7693f..43dcf69768 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -258,5 +258,14 @@ namespace Umbraco.Core.Models } Properties.Add(propertyType.CreatePropertyFromValue(value)); } + + /// + /// Boolean indicating whether the content and its properties are valid + /// + /// True if content is valid otherwise false + public virtual bool IsValid() + { + return Properties.Any(property => !property.IsValid()) == false; + } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Models/IContentBase.cs b/src/Umbraco.Core/Models/IContentBase.cs index ee9e43c5eb..db7d469adb 100644 --- a/src/Umbraco.Core/Models/IContentBase.cs +++ b/src/Umbraco.Core/Models/IContentBase.cs @@ -107,5 +107,11 @@ namespace Umbraco.Core.Models /// Alias of the PropertyType /// Value to set for the Property void SetValue(string propertyTypeAlias, object value); + + /// + /// Boolean indicating whether the content and its properties are valid + /// + /// True if content is valid otherwise false + bool IsValid(); } } \ No newline at end of file diff --git a/src/Umbraco.Core/Models/Property.cs b/src/Umbraco.Core/Models/Property.cs index e47fc9c47a..9a2c5e95a2 100644 --- a/src/Umbraco.Core/Models/Property.cs +++ b/src/Umbraco.Core/Models/Property.cs @@ -83,7 +83,10 @@ namespace Umbraco.Core.Models /// /// Gets or Sets the value of the Property /// - /// Setting the value will trigger a type and value validation + /// + /// Setting the value will trigger a type validation. + /// The type of the value has to be valid in order to be saved. + /// [DataMember] public object Value { @@ -91,13 +94,6 @@ namespace Umbraco.Core.Models set { bool typeValidation = _propertyType.IsPropertyTypeValid(value); - bool valueValidation = _propertyType.IsPropertyValueValid(value); - - if (!typeValidation && !valueValidation) - throw new Exception( - string.Format( - "Both Type and Value validation failed. The value type: {0} does not match the DataType in PropertyType with alias: {1}", - value.GetType(), Alias)); if (!typeValidation) throw new Exception( @@ -105,15 +101,32 @@ namespace Umbraco.Core.Models "Type validation failed. The value type: {0} does not match the DataType in PropertyType with alias: {1}", value.GetType(), Alias)); - if (!valueValidation) - throw new Exception( - string.Format( - "Validation failed for the Value, because it does not conform to the validation rules set for the PropertyType with alias: {0}", - Alias)); - _value = value; OnPropertyChanged(ValueSelector); } } + + /// + /// Boolean indicating whether the current value is valid + /// + /// + /// A valid value implies that it is ready for publishing. + /// Invalid property values can be saved, but not published. + /// + /// True is property value is valid, otherwise false + public bool IsValid() + { + return IsValid(Value); + } + + /// + /// Boolean indicating whether the passed in value is valid + /// + /// + /// True is property value is valid, otherwise false + public bool IsValid(object value) + { + return _propertyType.IsPropertyValueValid(value); + } } } \ No newline at end of file diff --git a/src/Umbraco.Tests/App.config b/src/Umbraco.Tests/App.config index c6642b945c..5231460cde 100644 --- a/src/Umbraco.Tests/App.config +++ b/src/Umbraco.Tests/App.config @@ -13,4 +13,16 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs new file mode 100644 index 0000000000..12c3a44225 --- /dev/null +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -0,0 +1,99 @@ +using System; +using NUnit.Framework; +using Umbraco.Core.Models; +using Umbraco.Tests.TestHelpers; +using Umbraco.Web.Services; + +namespace Umbraco.Tests.Services +{ + [TestFixture] + public class ContentServiceTests : BaseWebTest + { + /*[Test]*/ + public void Can_Create_Content() + { + // Arrange + var contentService = new ContentService(); + + // Act + IContent content = contentService.CreateContent(-1, "umbTextpage"); + + // Assert + Assert.That(content, Is.Not.Null); + Assert.That(content.HasIdentity, Is.False); + } + + /*[Test]*/ + public void Cannot_Create_Content_With_Non_Existing_ContentType_Alias() + { + // Arrange + var contentService = new ContentService(); + + // Act & Assert + Assert.Throws(() => contentService.CreateContent(-1, "umbAliasDoesntExist")); + } + + public void Can_Get_Content_By_Id() + { } + + public void Can_Get_Content_By_Level() + { } + + public void Can_Get_Children_Of_Content_Id() + { } + + public void Can_Get_All_Versions_Of_Content() + { } + + public void Can_Get_Root_Content() + { } + + public void Can_Get_Content_For_Expiration() + { } + + public void Can_Get_Content_For_Release() + { } + + public void Can_Get_Content_In_RecycleBin() + { } + + public void Can_RePublish_All_Content() + { } + + public void Can_Publish_Content() + { } + + public void Can_Publish_Content_Children() + { } + + public void Can_Save_And_Publish_Content() + { } + + public void Can_Save_Content() + { } + + public void Can_Bulk_Save_Content() + { } + + public void Can_Delete_Content_Of_Specific_ContentType() + { } + + public void Can_Delete_Content() + { } + + public void Can_Move_Content_To_RecycleBin() + { } + + public void Can_Move_Content() + { } + + public void Can_Copy_Content() + { } + + public void Can_Send_To_Publication() + { } + + public void Can_Rollback_Version_On_Content() + { } + } +} \ No newline at end of file diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 0b8cc4e532..c74168e018 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -80,6 +80,7 @@ + diff --git a/src/Umbraco.Web/Services/ContentService.cs b/src/Umbraco.Web/Services/ContentService.cs index b6d848907c..f36b2a2bbe 100644 --- a/src/Umbraco.Web/Services/ContentService.cs +++ b/src/Umbraco.Web/Services/ContentService.cs @@ -185,6 +185,8 @@ namespace Umbraco.Web.Services var list = new List(); + //Consider creating a Path query instead of recursive method + //var query = Query.Builder.Where(x => x.Path.StartsWith("-1")); var rootContent = GetRootContent(); foreach (var content in rootContent) { @@ -193,8 +195,12 @@ namespace Umbraco.Web.Services foreach (var item in list) { - ((Content)item).ChangePublishedState(true); - repository.AddOrUpdate(item); + //Only publish valid content - Might need to change the flat list as it could pose problems for children of invalid content + if(item.IsValid()) + { + ((Content)item).ChangePublishedState(true); + repository.AddOrUpdate(item); + } } unitOfWork.Commit(); @@ -224,13 +230,19 @@ namespace Umbraco.Web.Services var unitOfWork = _provider.GetUnitOfWork(); var repository = RepositoryResolver.ResolveByType(unitOfWork); + //Consider creating a Path query instead of recursive method + //var query = Query.Builder.Where(x => x.Path.StartsWith(content.Path)); var list = GetChildrenDeep(content.Id); list.Add(content); foreach (var item in list) { - ((Content)item).ChangePublishedState(true); - repository.AddOrUpdate(item); + //Only publish valid content - Might need to change the flat list as it could pose problems for children of invalid content + if (item.IsValid()) + { + ((Content) item).ChangePublishedState(true); + repository.AddOrUpdate(item); + } } unitOfWork.Commit(); @@ -283,7 +295,10 @@ namespace Umbraco.Web.Services { var unitOfWork = _provider.GetUnitOfWork(); var repository = RepositoryResolver.ResolveByType(unitOfWork); - + + if (!content.IsValid()) + return false;//Content contains invalid property values and can therefore not be published + ((Content)content).ChangePublishedState(true); repository.AddOrUpdate(content); unitOfWork.Commit();