diff --git a/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs index 00a4aed84f..5011df7779 100644 --- a/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs +++ b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs @@ -179,11 +179,9 @@ namespace Umbraco.Core.Models if (!content.ContentType.SupportsPropertyVariation(culture.Culture, "*", true)) throw new NotSupportedException($"Culture \"{culture}\" is not supported by content type \"{content.ContentType.Alias}\" with variation \"{content.ContentType.Variations}\"."); - var alsoInvariant = false; - switch (culture.CultureBehavior) { - case CultureType.Behavior.All: + case CultureType.Behavior.AllCultures: { foreach (var c in content.AvailableCultures) { @@ -194,31 +192,28 @@ namespace Umbraco.Core.Models } break; } - case CultureType.Behavior.Invariant: + case CultureType.Behavior.InvariantCulture: { if (string.IsNullOrWhiteSpace(content.Name)) return false; // PublishName set by repository - nothing to do here break; } - case var behavior when behavior.HasFlag(CultureType.Behavior.Explicit): + case CultureType.Behavior.ExplicitCulture: { var name = content.GetCultureName(culture.Culture); if (string.IsNullOrWhiteSpace(name)) return false; content.SetPublishInfo(culture.Culture, name, DateTime.Now); - alsoInvariant = behavior.HasFlag(CultureType.Behavior.Invariant); // we also want to publish invariant values break; } - default: - throw new ArgumentOutOfRangeException(); } // property.PublishValues only publishes what is valid, variation-wise foreach (var property in content.Properties) { property.PublishValues(culture.Culture); - if (alsoInvariant) + if (culture.CultureBehavior.HasFlag(CultureType.Behavior.InvariantProperties)) property.PublishValues(null); } diff --git a/src/Umbraco.Core/Models/CultureType.cs b/src/Umbraco.Core/Models/CultureType.cs index 8c7fcac052..e273cbdc3a 100644 --- a/src/Umbraco.Core/Models/CultureType.cs +++ b/src/Umbraco.Core/Models/CultureType.cs @@ -23,11 +23,46 @@ namespace Umbraco.Core.Models /// /// /// - public static CultureType Single(string culture, bool isDefault) + public static CultureType Explicit(string culture, bool isDefault) { return new CultureType(culture, isDefault); } + /// + /// Creates a based on a item + /// + /// + /// + /// + /// + public static CultureType Create(IContent content, string culture, bool isDefault) + { + if (!TryCreate(content.ContentType.Variations, culture, isDefault, out var cultureType)) + throw new InvalidOperationException($"The null value for culture is reserved for invariant content but the content type {content.ContentType.Alias} is variant"); + + return cultureType; + } + + internal static bool TryCreate(ContentVariation variation, string culture, bool isDefault, out CultureType cultureType) + { + cultureType = null; + if (culture == null || culture == "*") + { + if (culture == null && variation.VariesByCulture()) + return false; + + //we support * for invariant since it means ALL but we need to explicitly translate it so it's behavior is Invariant + if (culture == "*" && !variation.VariesByCulture()) + { + cultureType = new CultureType(null, isDefault); + return true; + } + } + + cultureType = new CultureType(culture, isDefault); + return true; + } + private CultureType(string culture, bool isDefault = false) { Culture = culture; @@ -39,14 +74,18 @@ namespace Umbraco.Core.Models { get { - if (Culture == "*") return Behavior.All; - if (Culture == null) return Behavior.Invariant; + //null can only be invariant + if (Culture == null) return Behavior.InvariantCulture | Behavior.InvariantProperties; - var result = Behavior.Explicit; + // * is All which means its also invariant properties since this will include the default language + if (Culture == "*") return (Behavior.AllCultures | Behavior.InvariantProperties); - //if the explicit culture is the default, then the behavior is also Invariant + //else it's explicit + var result = Behavior.ExplicitCulture; + + //if the explicit culture is the default, then the behavior is also InvariantProperties if (IsDefaultCulture) - result |= Behavior.Invariant; + result |= Behavior.InvariantProperties; return result; } @@ -57,9 +96,10 @@ namespace Umbraco.Core.Models [Flags] public enum Behavior : byte { - All = 0, - Invariant = 1, - Explicit = 2 + AllCultures = 1, + InvariantCulture = 2, + ExplicitCulture = 4, + InvariantProperties = 8 } } } diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 05eb32d57d..e7563fac45 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -882,7 +882,7 @@ namespace Umbraco.Core.Services.Implement // if culture is '*', then publish them all (including variants) //this will create the correct culture type even if culture is * or null - var cultureType = CultureType.Single(culture, _languageRepository.IsDefault(culture)); + var cultureType = CultureType.Create(content, culture, _languageRepository.IsDefault(culture)); // publish the culture(s) // we don't care about the response here, this response will be rechecked below but we need to set the culture info values now. @@ -919,9 +919,10 @@ namespace Umbraco.Core.Services.Implement : new PublishResult(PublishResultType.FailedPublishNothingToPublish, evtMsgs, content); } - //fixme: Shouldn't we makes ure that all string cultures here are valid? i.e. no * or null is allowed when using this method + if (cultures.Any(x => x == null || x == "*")) + throw new InvalidOperationException("Only valid cultures are allowed to be used in this method, wildcards or nulls are not allowed"); - var cultureTypes = cultures.Select(x => CultureType.Single(x, _languageRepository.IsDefault(x))); + var cultureTypes = cultures.Select(x => CultureType.Explicit(x, _languageRepository.IsDefault(x))); // publish the culture(s) // we don't care about the response here, this response will be rechecked below but we need to set the culture info values now. @@ -1307,7 +1308,7 @@ namespace Umbraco.Core.Services.Implement //publish the culture values and validate the property values, if validation fails, log the invalid properties so the develeper has an idea of what has failed Property[] invalidProperties = null; - var cultureType = CultureType.Single(culture, _languageRepository.IsDefault(culture)); + var cultureType = CultureType.Explicit(culture, _languageRepository.IsDefault(culture)); var tryPublish = d.PublishCulture(cultureType) && _propertyValidationService.Value.IsPropertyDataValid(d, out invalidProperties, cultureType); if (invalidProperties != null && invalidProperties.Length > 0) Logger.Warn("Scheduled publishing will fail for document {DocumentId} and culture {Culture} because of invalid properties {InvalidProperties}", @@ -1408,7 +1409,7 @@ namespace Umbraco.Core.Services.Implement { return culturesToPublish.All(culture => { - var cultureType = CultureType.Single(culture, _languageRepository.IsDefault(culture)); + var cultureType = CultureType.Explicit(culture, _languageRepository.IsDefault(culture)); return content.PublishCulture(cultureType) && _propertyValidationService.Value.IsPropertyDataValid(content, out _, cultureType); }); } @@ -2486,15 +2487,17 @@ namespace Umbraco.Core.Services.Implement var variesByCulture = content.ContentType.VariesByCulture(); - var cultureTypesToPublish = culturesPublishing.ToDictionary(x => x, x => CultureType.Single(x, _languageRepository.IsDefault(x))); + var cultureTypesToPublish = culturesPublishing == null + ? new[] {CultureType.Invariant} //if it's null it's invariant + : culturesPublishing.Select(x => CultureType.Explicit(x, _languageRepository.IsDefault(x))).ToArray(); // publish the culture(s) - if (!cultureTypesToPublish.All(x => content.PublishCulture(x.Value))) + if (!cultureTypesToPublish.All(content.PublishCulture)) return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content); //validate the property values Property[] invalidProperties = null; - if (!cultureTypesToPublish.All(x => _propertyValidationService.Value.IsPropertyDataValid(content, out invalidProperties, x.Value))) + if (!cultureTypesToPublish.All(x => _propertyValidationService.Value.IsPropertyDataValid(content, out invalidProperties, x))) return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content) { InvalidProperties = invalidProperties @@ -2504,6 +2507,8 @@ namespace Umbraco.Core.Services.Implement // be changed to Unpublished and any culture currently published will not be visible. if (variesByCulture) { + //fixme: culturesPublishing can be null here, it shouldn't be at this point since that would indicate its invariant but we should check + if (content.Published && culturesPublishing.Count == 0 && culturesUnpublishing.Count == 0) // no published cultures = cannot be published return new PublishResult(PublishResultType.FailedPublishNothingToPublish, evtMsgs, content); diff --git a/src/Umbraco.Core/Services/PropertyValidationService.cs b/src/Umbraco.Core/Services/PropertyValidationService.cs index 3882ab0cb4..800f4126f0 100644 --- a/src/Umbraco.Core/Services/PropertyValidationService.cs +++ b/src/Umbraco.Core/Services/PropertyValidationService.cs @@ -42,34 +42,25 @@ namespace Umbraco.Core.Services var propertyTypeVaries = x.PropertyType.VariesByCulture(); - switch (culture.CultureBehavior) - { - case CultureType.Behavior.Invariant: - return !(propertyTypeVaries || IsPropertyValid(x, null)); // validate invariant property, invariant culture - case CultureType.Behavior.All: - return !IsPropertyValid(x, culture.Culture); // validate property, all cultures - case var behavior when behavior.HasFlag(CultureType.Behavior.Explicit): - if (propertyTypeVaries) - { - return !IsPropertyValid(x, culture.Culture); // validate variant property, explicit culture - } - else - { - //We only want to validate the invariant property against an explicit culture if: - // * The culture is the default (which means it will have the Behavior.Invariant flag) OR - // * The content item isn't published + if (culture.CultureBehavior.HasFlag(CultureType.Behavior.InvariantCulture)) + return !(propertyTypeVaries || IsPropertyValid(x, null)); // validate invariant property, invariant culture - //This is because an invariant property is only edited on the default culture, but if the - //content item isn't published, we can't allow publishing of the specific non default culture - //if the invariant property data is invalid. + if (culture.CultureBehavior.HasFlag(CultureType.Behavior.AllCultures)) + return !IsPropertyValid(x, culture.Culture); // validate property, all cultures + + //we're dealing with Behavior.Explicit now... + + if (propertyTypeVaries) + return !IsPropertyValid(x, culture.Culture); // validate variant property, explicit culture + + //Validate invariant properties on the default language (will have the Behavior.InvariantProperties) + // or if there is no published version of the content, in which case it doesn't matter which culture is publishing + + var shouldValidate = culture.CultureBehavior.HasFlag(CultureType.Behavior.InvariantProperties) //default language + || !content.Published; //non-default language but no published version + + return shouldValidate && !IsPropertyValid(x, null); // validate invariant property, explicit culture - return (behavior.HasFlag(CultureType.Behavior.Invariant) || !content.Published) - && !IsPropertyValid(x, null); // validate invariant property, explicit culture - } - default: - throw new ArgumentOutOfRangeException(); - } - }).ToArray(); return invalidProperties.Length == 0; diff --git a/src/Umbraco.Tests/Models/ContentTests.cs b/src/Umbraco.Tests/Models/ContentTests.cs index 54934a339d..643ccfb788 100644 --- a/src/Umbraco.Tests/Models/ContentTests.cs +++ b/src/Umbraco.Tests/Models/ContentTests.cs @@ -104,7 +104,7 @@ namespace Umbraco.Tests.Models Thread.Sleep(500); //The "Date" wont be dirty if the test runs too fast since it will be the same date content.SetCultureName("name-fr", langFr); - content.PublishCulture(CultureType.Single(langFr, false)); //we've set the name, now we're publishing it + content.PublishCulture(CultureType.Explicit(langFr, false)); //we've set the name, now we're publishing it Assert.IsTrue(content.IsPropertyDirty("PublishCultureInfos")); //now it will be changed since the collection has changed var frCultureName = content.PublishCultureInfos[langFr]; Assert.IsTrue(frCultureName.IsPropertyDirty("Date")); @@ -116,7 +116,7 @@ namespace Umbraco.Tests.Models Thread.Sleep(500); //The "Date" wont be dirty if the test runs too fast since it will be the same date content.SetCultureName("name-fr", langFr); - content.PublishCulture(CultureType.Single(langFr, false)); //we've set the name, now we're publishing it + content.PublishCulture(CultureType.Explicit(langFr, false)); //we've set the name, now we're publishing it Assert.IsTrue(frCultureName.IsPropertyDirty("Date")); Assert.IsTrue(content.IsPropertyDirty("PublishCultureInfos")); //it's true now since we've updated a name } diff --git a/src/Umbraco.Tests/Models/CultureTypeTests.cs b/src/Umbraco.Tests/Models/CultureTypeTests.cs new file mode 100644 index 0000000000..9b6c0149da --- /dev/null +++ b/src/Umbraco.Tests/Models/CultureTypeTests.cs @@ -0,0 +1,138 @@ +using NUnit.Framework; +using Umbraco.Core.Models; + +namespace Umbraco.Tests.Models +{ + [TestFixture] + public class CultureTypeTests + { + [Test] + public void All() + { + var ct = CultureType.All; + + Assert.AreEqual(ct.Culture, "*"); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantProperties)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantCulture)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.ExplicitCulture)); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.AllCultures)); + Assert.IsFalse(ct.IsDefaultCulture); + } + + [Test] + public void Invariant() + { + var ct = CultureType.Invariant; + + Assert.AreEqual(ct.Culture, null); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantProperties)); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantCulture)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.ExplicitCulture)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.AllCultures)); + Assert.IsFalse(ct.IsDefaultCulture); + } + + [Test] + public void Single_Default_Culture() + { + var ct = CultureType.Explicit("en-US", true); + + Assert.AreEqual(ct.Culture, "en-US"); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantProperties)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantCulture)); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.ExplicitCulture)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.AllCultures)); + Assert.IsTrue(ct.IsDefaultCulture); + } + + [Test] + public void Single_Non_Default_Culture() + { + var ct = CultureType.Explicit("en-US", false); + + Assert.AreEqual(ct.Culture, "en-US"); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantProperties)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantCulture)); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.ExplicitCulture)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.AllCultures)); + Assert.IsFalse(ct.IsDefaultCulture); + } + + [Test] + public void Try_Create_Explicit_Default_Culture() + { + var success = CultureType.TryCreate(ContentVariation.Culture, "en-US", true, out var ct); + Assert.IsTrue(success); + + Assert.AreEqual(ct.Culture, "en-US"); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantProperties)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantCulture)); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.ExplicitCulture)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.AllCultures)); + Assert.IsTrue(ct.IsDefaultCulture); + } + + [Test] + public void Try_Create_Explicit_Non_Default_Culture() + { + var success = CultureType.TryCreate(ContentVariation.Culture, "en-US", false, out var ct); + Assert.IsTrue(success); + + Assert.AreEqual(ct.Culture, "en-US"); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantProperties)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantCulture)); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.ExplicitCulture)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.AllCultures)); + Assert.IsFalse(ct.IsDefaultCulture); + } + + [Test] + public void Try_Create_Wildcard_Invariant() + { + var success = CultureType.TryCreate(ContentVariation.Nothing, "*", false, out var ct); + Assert.IsTrue(success); + + Assert.AreEqual(ct.Culture, null); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantProperties)); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantCulture)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.ExplicitCulture)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.AllCultures)); + Assert.IsFalse(ct.IsDefaultCulture); + } + + [Test] + public void Try_Create_Wildcard_Variant() + { + var success = CultureType.TryCreate(ContentVariation.Culture, "*", false, out var ct); + Assert.IsTrue(success); + + Assert.AreEqual(ct.Culture, "*"); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantProperties)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantCulture)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.ExplicitCulture)); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.AllCultures)); + Assert.IsFalse(ct.IsDefaultCulture); + } + + [Test] + public void Try_Create_Null_Variant() + { + var success = CultureType.TryCreate(ContentVariation.Culture, null, false, out var ct); + Assert.IsFalse(success); + } + + [Test] + public void Try_Create_Null_Invariant() + { + var success = CultureType.TryCreate(ContentVariation.Nothing, null, false, out var ct); + Assert.IsTrue(success); + + Assert.AreEqual(ct.Culture, null); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantProperties)); + Assert.IsTrue(ct.CultureBehavior.HasFlag(CultureType.Behavior.InvariantCulture)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.ExplicitCulture)); + Assert.IsFalse(ct.CultureBehavior.HasFlag(CultureType.Behavior.AllCultures)); + Assert.IsFalse(ct.IsDefaultCulture); + } + } +} diff --git a/src/Umbraco.Tests/Models/VariationTests.cs b/src/Umbraco.Tests/Models/VariationTests.cs index 6f68f8ae7a..8d86db5236 100644 --- a/src/Umbraco.Tests/Models/VariationTests.cs +++ b/src/Umbraco.Tests/Models/VariationTests.cs @@ -305,9 +305,9 @@ namespace Umbraco.Tests.Models // can publish value // and get edited and published values - Assert.IsFalse(content.PublishCulture(CultureType.Single(langFr, false))); // no name + Assert.IsFalse(content.PublishCulture(CultureType.Explicit(langFr, false))); // no name content.SetCultureName("name-fr", langFr); - Assert.IsTrue(content.PublishCulture(CultureType.Single(langFr, false))); + Assert.IsTrue(content.PublishCulture(CultureType.Explicit(langFr, false))); Assert.IsNull(content.GetValue("prop")); Assert.IsNull(content.GetValue("prop", published: true)); Assert.AreEqual("c", content.GetValue("prop", langFr)); @@ -331,7 +331,7 @@ namespace Umbraco.Tests.Models content.UnpublishCulture(langFr); Assert.AreEqual("c", content.GetValue("prop", langFr)); Assert.IsNull(content.GetValue("prop", langFr, published: true)); - Assert.IsTrue(content.PublishCulture(CultureType.Single(langFr, false))); + Assert.IsTrue(content.PublishCulture(CultureType.Explicit(langFr, false))); Assert.AreEqual("c", content.GetValue("prop", langFr)); Assert.AreEqual("c", content.GetValue("prop", langFr, published: true)); @@ -385,7 +385,7 @@ namespace Umbraco.Tests.Models content.SetCultureName("hello", langFr); //for this test we'll make the french culture the default one - this is needed for publishing invariant property values - var langFrCultureType = CultureType.Single(langFr, true); + var langFrCultureType = CultureType.Explicit(langFr, true); Assert.IsTrue(content.PublishCulture(langFrCultureType)); // succeeds because names are ok (not validating properties here) Assert.IsFalse(propertyValidationService.IsPropertyDataValid(content, out _, langFrCultureType));// fails because prop1 is mandatory @@ -428,12 +428,12 @@ namespace Umbraco.Tests.Models content.SetValue("prop", "a-es", langEs); // cannot publish without a name - Assert.IsFalse(content.PublishCulture(CultureType.Single(langFr, false))); + Assert.IsFalse(content.PublishCulture(CultureType.Explicit(langFr, false))); // works with a name // and then FR is available, and published content.SetCultureName("name-fr", langFr); - Assert.IsTrue(content.PublishCulture(CultureType.Single(langFr, false))); + Assert.IsTrue(content.PublishCulture(CultureType.Explicit(langFr, false))); // now UK is available too content.SetCultureName("name-uk", langUk); diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 8a43f2de4a..0d906dc1f3 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -732,8 +732,8 @@ namespace Umbraco.Tests.Services IContent content = new Content("content", Constants.System.Root, contentType); content.SetCultureName("content-fr", langFr.IsoCode); content.SetCultureName("content-en", langUk.IsoCode); - content.PublishCulture(CultureType.Single(langFr.IsoCode, langFr.IsDefault)); - content.PublishCulture(CultureType.Single(langUk.IsoCode, langUk.IsDefault)); + content.PublishCulture(CultureType.Explicit(langFr.IsoCode, langFr.IsDefault)); + content.PublishCulture(CultureType.Explicit(langUk.IsoCode, langUk.IsDefault)); Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); @@ -963,6 +963,18 @@ namespace Umbraco.Tests.Services Assert.AreEqual("Home", e.Name); } + [Test] + public void Can_Not_Publish_Invalid_Cultures() + { + var contentService = ServiceContext.ContentService; + var content = Mock.Of(c => c.ContentType == Mock.Of(s => s.Variations == ContentVariation.Culture)); + + Assert.Throws(() => contentService.SaveAndPublish(content, new[] {"*"})); + Assert.Throws(() => contentService.SaveAndPublish(content, new string[] { null })); + Assert.Throws(() => contentService.SaveAndPublish(content, new[] { "*", null })); + Assert.Throws(() => contentService.SaveAndPublish(content, new[] { "en-US", "*", "es-ES" })); + } + [Test] public void Can_Publish_Only_Valid_Content() { @@ -973,10 +985,7 @@ namespace Umbraco.Tests.Services const int parentId = NodeDto.NodeIdSeed + 2; var contentService = ServiceContext.ContentService; - var content = MockedContent.CreateSimpleContent(contentType, "Invalid Content", parentId); - content.SetValue("author", string.Empty); - contentService.Save(content); - + var parent = contentService.GetById(parentId); var parentPublished = contentService.SaveAndPublish(parent); @@ -986,6 +995,10 @@ namespace Umbraco.Tests.Services Assert.IsTrue(parentPublished.Success); Assert.IsTrue(parent.Published); + var content = MockedContent.CreateSimpleContent(contentType, "Invalid Content", parentId); + content.SetValue("author", string.Empty); + Assert.IsFalse(content.HasIdentity); + // content cannot publish values because they are invalid var propertyValidationService = new PropertyValidationService(Factory.GetInstance(), ServiceContext.DataTypeService); var isValid = propertyValidationService.IsPropertyDataValid(content, out var invalidProperties, CultureType.Invariant); @@ -998,7 +1011,12 @@ namespace Umbraco.Tests.Services Assert.IsFalse(contentPublished.Success); Assert.AreEqual(PublishResultType.FailedPublishContentInvalid, contentPublished.Result); Assert.IsFalse(content.Published); + + //Ensure it saved though + Assert.Greater(content.Id, 0); + Assert.IsTrue(content.HasIdentity); } + [Test] public void Can_Publish_And_Unpublish_Cultures_In_Single_Operation() @@ -1018,7 +1036,7 @@ namespace Umbraco.Tests.Services content.SetCultureName("name-fr", langFr.IsoCode); content.SetCultureName("name-da", langDa.IsoCode); - content.PublishCulture(CultureType.Single(langFr.IsoCode, langFr.IsDefault)); + content.PublishCulture(CultureType.Explicit(langFr.IsoCode, langFr.IsDefault)); var result = ((ContentService)ServiceContext.ContentService).CommitDocumentChanges(content); Assert.IsTrue(result.Success); content = ServiceContext.ContentService.GetById(content.Id); @@ -1026,7 +1044,7 @@ namespace Umbraco.Tests.Services Assert.IsFalse(content.IsCulturePublished(langDa.IsoCode)); content.UnpublishCulture(langFr.IsoCode); - content.PublishCulture(CultureType.Single(langDa.IsoCode, langDa.IsDefault)); + content.PublishCulture(CultureType.Explicit(langDa.IsoCode, langDa.IsDefault)); result = ((ContentService)ServiceContext.ContentService).CommitDocumentChanges(content); Assert.IsTrue(result.Success); diff --git a/src/Umbraco.Tests/Services/PropertyValidationServiceTests.cs b/src/Umbraco.Tests/Services/PropertyValidationServiceTests.cs new file mode 100644 index 0000000000..6c3f308509 --- /dev/null +++ b/src/Umbraco.Tests/Services/PropertyValidationServiceTests.cs @@ -0,0 +1,177 @@ +using System.Threading; +using Moq; +using NUnit.Framework; +using Umbraco.Core; +using Umbraco.Core.Models; +using Umbraco.Core.PropertyEditors; +using Umbraco.Core.PropertyEditors.Validators; +using Umbraco.Core.Services; +using Umbraco.Tests.Testing; +using Umbraco.Web.PropertyEditors; + +namespace Umbraco.Tests.Services +{ + [TestFixture] + public class PropertyValidationServiceTests : UmbracoTestBase + { + private void MockObjects(out PropertyValidationService validationService, out IDataType dt) + { + var textService = new Mock(); + textService.Setup(x => x.Localize(It.IsAny(), Thread.CurrentThread.CurrentCulture, null)).Returns("Localized text"); + + var dataTypeService = new Mock(); + var dataType = Mock.Of( + x => x.Configuration == (object)string.Empty //irrelevant but needs a value + && x.DatabaseType == ValueStorageType.Nvarchar + && x.EditorAlias == Constants.PropertyEditors.Aliases.TextBox); + dataTypeService.Setup(x => x.GetDataType(It.IsAny())).Returns(() => dataType); + dt = dataType; + + //new data editor that returns a TextOnlyValueEditor which will do the validation for the properties + var dataEditor = Mock.Of( + x => x.Type == EditorType.PropertyValue + && x.Alias == Constants.PropertyEditors.Aliases.TextBox); + Mock.Get(dataEditor).Setup(x => x.GetValueEditor(It.IsAny())) + .Returns(new CustomTextOnlyValueEditor(new DataEditorAttribute(Constants.PropertyEditors.Aliases.TextBox, "Test Textbox", "textbox"), textService.Object)); + + var propEditors = new PropertyEditorCollection(new DataEditorCollection(new[] { dataEditor })); + + validationService = new PropertyValidationService(propEditors, dataTypeService.Object); + } + + [Test] + public void Validate_Invariant_Properties_On_Variant_Default_Culture() + { + MockObjects(out var validationService, out var dataType); + + var p1 = new Property(new PropertyType(dataType, "test1") { Mandatory = true, Variations = ContentVariation.Culture}); + p1.SetValue("Hello", "en-US"); + var p2 = new Property(new PropertyType(dataType, "test2") { Mandatory = true, Variations = ContentVariation.Nothing }); + p2.SetValue("Hello", null); + var p3 = new Property(new PropertyType(dataType, "test3") { Mandatory = true, Variations = ContentVariation.Culture }); + p3.SetValue(null, "en-US"); //invalid + var p4 = new Property(new PropertyType(dataType, "test4") { Mandatory = true, Variations = ContentVariation.Nothing }); + p4.SetValue(null, null); //invalid + + var content = Mock.Of( + x => x.Published == true //set to published, the default culture will validate invariant anyways + && x.Properties == new PropertyCollection(new[] { p1, p2, p3, p4 })); + + var result = validationService.IsPropertyDataValid(content, out var invalid, CultureType.Explicit("en-US", true)); + + Assert.IsFalse(result); + Assert.AreEqual(2, invalid.Length); + } + + [Test] + public void Validate_Invariant_Properties_On_Variant_Non_Default_Culture() + { + MockObjects(out var validationService, out var dataType); + + var p1 = new Property(new PropertyType(dataType, "test1") { Mandatory = true, Variations = ContentVariation.Culture }); + p1.SetValue("Hello", "en-US"); + var p2 = new Property(new PropertyType(dataType, "test2") { Mandatory = true, Variations = ContentVariation.Nothing }); + p2.SetValue("Hello", null); + var p3 = new Property(new PropertyType(dataType, "test3") { Mandatory = true, Variations = ContentVariation.Culture }); + p3.SetValue(null, "en-US"); //invalid + var p4 = new Property(new PropertyType(dataType, "test4") { Mandatory = true, Variations = ContentVariation.Nothing }); + p4.SetValue(null, null); //invalid + + var content = Mock.Of( + x => x.Published == false //set to not published, the non default culture will need to validate invariant too + && x.Properties == new PropertyCollection(new[] { p1, p2, p3, p4 })); + + var result = validationService.IsPropertyDataValid(content, out var invalid, CultureType.Explicit("en-US", false)); + + Assert.IsFalse(result); + Assert.AreEqual(2, invalid.Length); + } + + [Test] + public void Validate_Variant_Properties_On_Variant() + { + MockObjects(out var validationService, out var dataType); + + var p1 = new Property(new PropertyType(dataType, "test1") { Mandatory = true, Variations = ContentVariation.Culture }); + p1.SetValue(null, "en-US"); //invalid + var p2 = new Property(new PropertyType(dataType, "test2") { Mandatory = true, Variations = ContentVariation.Nothing }); + p2.SetValue(null, null); //invalid + var p3 = new Property(new PropertyType(dataType, "test3") { Mandatory = true, Variations = ContentVariation.Culture }); + p3.SetValue(null, "en-US"); //ignored because the CultureType isn't the default lang + the content is published + var p4 = new Property(new PropertyType(dataType, "test4") { Mandatory = true, Variations = ContentVariation.Nothing }); + p4.SetValue(null, null); //ignored because the CultureType isn't the default lang + the content is published + + var content = Mock.Of( + x => x.Published == true //set to published + && x.Properties == new PropertyCollection(new[] { p1, p2, p3, p4 })); + + var result = validationService.IsPropertyDataValid(content, out var invalid, CultureType.Explicit("en-US", false)); + + Assert.IsFalse(result); + Assert.AreEqual(2, invalid.Length); + } + + [Test] + public void Validate_Invariant_Properties_On_Invariant() + { + MockObjects(out var validationService, out var dataType); + + var p1 = new Property(new PropertyType(dataType, "test1") { Mandatory = true, Variations = ContentVariation.Culture }); + p1.SetValue(null, "en-US"); //ignored since this is variant + var p2 = new Property(new PropertyType(dataType, "test2") { Mandatory = true, Variations = ContentVariation.Nothing }); + p2.SetValue(null, null); //invalid + var p3 = new Property(new PropertyType(dataType, "test3") { Mandatory = true, Variations = ContentVariation.Culture }); + p3.SetValue("Hello", "en-US"); //ignored since this is variant + var p4 = new Property(new PropertyType(dataType, "test4") { Mandatory = true, Variations = ContentVariation.Nothing }); + p4.SetValue(null, null); //invalid + + var content = Mock.Of( + x => x.Properties == new PropertyCollection(new[] { p1, p2, p3, p4 })); + + var result = validationService.IsPropertyDataValid(content, out var invalid, CultureType.Invariant); + + Assert.IsFalse(result); + Assert.AreEqual(2, invalid.Length); + } + + [Test] + public void Validate_Properties_On_All() + { + MockObjects(out var validationService, out var dataType); + + var p1 = new Property(new PropertyType(dataType, "test1") { Mandatory = true, Variations = ContentVariation.Culture }); + p1.SetValue(null, "en-US"); //invalid + var p2 = new Property(new PropertyType(dataType, "test2") { Mandatory = true, Variations = ContentVariation.Nothing }); + p2.SetValue(null, null); //invalid + var p3 = new Property(new PropertyType(dataType, "test3") { Mandatory = true, Variations = ContentVariation.Culture }); + p3.SetValue(null, "en-US"); //invalid + var p4 = new Property(new PropertyType(dataType, "test4") { Mandatory = true, Variations = ContentVariation.Nothing }); + p4.SetValue(null, null); //invalid + + var content = Mock.Of( + x => x.Properties == new PropertyCollection(new[] { p1, p2, p3, p4 })); + + var result = validationService.IsPropertyDataValid(content, out var invalid, CultureType.All); + + Assert.IsFalse(result); + Assert.AreEqual(4, invalid.Length); + } + + + //used so we can inject a mock - we should fix the base class DataValueEditor to be able to have the ILocalizedTextField passed + // in to create the Requried and Regex validators so we aren't using singletons + private class CustomTextOnlyValueEditor : TextOnlyValueEditor + { + private readonly ILocalizedTextService _textService; + + public CustomTextOnlyValueEditor(DataEditorAttribute attribute, ILocalizedTextService textService) : base(attribute) + { + _textService = textService; + } + + public override IValueRequiredValidator RequiredValidator => new RequiredValidator(_textService); + + public override IValueFormatValidator FormatValidator => new RegexValidator(_textService, null); + } + } +} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 0acde69e08..c555211d59 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -134,6 +134,7 @@ + @@ -152,6 +153,7 @@ + diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index f9c701c1b0..a1129e5583 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -1341,7 +1341,7 @@ namespace Umbraco.Web.Editors foreach (var variant in cultureVariants.Where(x => x.Publish)) { // publishing any culture, implies the invariant culture - var valid = persistentContent.PublishCulture(CultureType.Single(variant.Culture, IsDefaultCulture(variant.Culture))); + var valid = persistentContent.PublishCulture(CultureType.Explicit(variant.Culture, IsDefaultCulture(variant.Culture))); if (!valid) { AddCultureValidationError(variant.Culture, "speechBubbles/contentCultureValidationError");