yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Only validate invariant properties when strictly necessary (#18729)

Browse Source
This commit is contained in:
Kenn Jacobsen
2025-03-25 12:58:01 +01:00
committed by GitHub
parent 80e092069b
commit 2711ac07ac
6 changed files with 225 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
src/Umbraco.Core/Services/PropertyValidationService.cs
View File

@@ -1,6 +1,8 @@
using System.ComponentModel.DataAnnotations;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Dictionary;
using Umbraco.Cms.Core.Models;
@@ -17,6 +19,8 @@ public class PropertyValidationService : IPropertyValidationService
private readonly PropertyEditorCollection _propertyEditors;
private readonly IValueEditorCache _valueEditorCache;
private readonly ICultureDictionary _cultureDictionary;
private readonly ILanguageService _languageService;
private readonly ContentSettings _contentSettings;
[Obsolete("Use the constructor that accepts ICultureDictionary. Will be removed in V15.")]
public PropertyValidationService(
@@ -28,18 +32,40 @@ public class PropertyValidationService : IPropertyValidationService
{
}
[Obsolete("Use the constructor that accepts ILanguageService and ContentSettings options. Will be removed in V17.")]
public PropertyValidationService(
PropertyEditorCollection propertyEditors,
IDataTypeService dataTypeService,
ILocalizedTextService textService,
IValueEditorCache valueEditorCache,
ICultureDictionary cultureDictionary)
: this(
propertyEditors,
dataTypeService,
textService,
valueEditorCache,
cultureDictionary,
StaticServiceProvider.Instance.GetRequiredService<ILanguageService>(),
StaticServiceProvider.Instance.GetRequiredService<IOptions<ContentSettings>>())
{
}
public PropertyValidationService(
PropertyEditorCollection propertyEditors,
IDataTypeService dataTypeService,
ILocalizedTextService textService,
IValueEditorCache valueEditorCache,
ICultureDictionary cultureDictionary,
ILanguageService languageService,
IOptions<ContentSettings> contentSettings)
{
_propertyEditors = propertyEditors;
_dataTypeService = dataTypeService;
_textService = textService;
_valueEditorCache = valueEditorCache;
_cultureDictionary = cultureDictionary;
_languageService = languageService;
_contentSettings = contentSettings.Value;
}
/// <inheritdoc />
@@ -66,6 +92,19 @@ public class PropertyValidationService : IPropertyValidationService
propertyType.PropertyEditorAlias);
}
// only validate culture invariant properties if
// - AllowEditInvariantFromNonDefault is true, or
// - the default language is being validated, or
// - the underlying data editor supports partial property value merging (e.g. block level variance)
var defaultCulture = _languageService.GetDefaultIsoCodeAsync().GetAwaiter().GetResult();
if (propertyType.VariesByCulture() is false
&& _contentSettings.AllowEditInvariantFromNonDefault is false
&& validationContext.CulturesBeingValidated.InvariantContains(defaultCulture) is false
&& dataEditor.CanMergePartialPropertyValues(propertyType) is false)
{
return [];
}
return ValidatePropertyValue(dataEditor, dataType, postedValue, propertyType.Mandatory, propertyType.ValidationRegExp, propertyType.MandatoryMessage, propertyType.ValidationRegExpMessage, validationContext);
}

63
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Validation.cs
View File

@@ -3,6 +3,7 @@ using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Blocks;
using Umbraco.Cms.Core.Models.ContentEditing;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Tests.Integration.Attributes;
namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.PropertyEditors;
@@ -172,7 +173,15 @@ internal partial class BlockListElementLevelVariationTests
}
[Test]
public async Task Can_Validate_Invalid_Properties_Specific_Culture_Only()
[ConfigureBuilder(ActionName = nameof(ConfigureAllowEditInvariantFromNonDefaultTrue))]
public async Task Can_Validate_Invalid_Properties_Specific_Culture_Only_With_AllowEditInvariantFromNonDefault()
=> await Can_Validate_Invalid_Properties_Specific_Culture_Only();
[Test]
public async Task Can_Validate_Invalid_Properties_Specific_Culture_Only_Without_AllowEditInvariantFromNonDefault()
=> await Can_Validate_Invalid_Properties_Specific_Culture_Only();
private async Task Can_Validate_Invalid_Properties_Specific_Culture_Only()
{
var elementType = CreateElementTypeWithValidation();
var blockListDataType = await CreateBlockListDataType(elementType);
@@ -214,6 +223,8 @@ internal partial class BlockListElementLevelVariationTests
contentType,
new[] { "en-US" });
// NOTE: since the default culture is being validated, we expect the same result regardless
// of the AllowEditInvariantFromNonDefault configuration
var errors = result.ValidationErrors.ToArray();
Assert.Multiple(() =>
{
@@ -338,7 +349,15 @@ internal partial class BlockListElementLevelVariationTests
}
[Test]
public async Task Can_Validate_Missing_Properties_Nested_Blocks_Specific_Culture_Only()
[ConfigureBuilder(ActionName = nameof(ConfigureAllowEditInvariantFromNonDefaultTrue))]
public async Task Can_Validate_Missing_Properties_Nested_Blocks_Specific_Culture_Only_With_AllowEditInvariantFromNonDefault()
=> Can_Validate_Missing_Properties_Nested_Blocks_Specific_Culture_Only(true);
[Test]
public async Task Can_Validate_Missing_Properties_Nested_Blocks_Specific_Culture_Only_Without_AllowEditInvariantFromNonDefault()
=> Can_Validate_Missing_Properties_Nested_Blocks_Specific_Culture_Only(false);
private async Task Can_Validate_Missing_Properties_Nested_Blocks_Specific_Culture_Only(bool expectedInvariantValidationErrors)
{
var (rootElementType, nestedElementType) = await CreateElementTypeWithValidationAndNestedBlocksAsync();
var rootBlockListDataType = await CreateBlockListDataType(rootElementType);
@@ -448,19 +467,39 @@ internal partial class BlockListElementLevelVariationTests
new[] { "da-DK" });
var errors = result.ValidationErrors.ToArray();
Assert.Multiple(() =>
// NOTE: since the default culture is not being validated, we expect different results depending
// on the AllowEditInvariantFromNonDefault configuration
if (expectedInvariantValidationErrors)
{
Assert.AreEqual(6, errors.Length);
Assert.IsTrue(errors.All(error => error.Alias == "blocks" && error.Culture == null && error.Segment == null));
Assert.Multiple(() =>
{
Assert.AreEqual(6, errors.Length);
Assert.IsTrue(errors.All(error => error.Alias == "blocks" && error.Culture == null && error.Segment == null));
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[0].value.contentData[0].values[?(@.alias == 'invariantText' && @.culture == null && @.segment == null)].value"));
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[0].value.contentData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value"));
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value"));
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[0].value.contentData[0].values[?(@.alias == 'invariantText' && @.culture == null && @.segment == null)].value"));
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[0].value.contentData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value"));
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value"));
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".settingsData[0].values[0].value.settingsData[0].values[?(@.alias == 'invariantText' && @.culture == null && @.segment == null)].value"));
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".settingsData[0].values[0].value.settingsData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value"));
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".settingsData[0].values[?(@.alias == 'invariantText' && @.culture == null && @.segment == null)].value"));
});
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".settingsData[0].values[0].value.settingsData[0].values[?(@.alias == 'invariantText' && @.culture == null && @.segment == null)].value"));
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".settingsData[0].values[0].value.settingsData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value"));
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".settingsData[0].values[?(@.alias == 'invariantText' && @.culture == null && @.segment == null)].value"));
});
}
else
{
Assert.Multiple(() =>
{
Assert.AreEqual(3, errors.Length);
Assert.IsTrue(errors.All(error => error.Alias == "blocks" && error.Culture == null && error.Segment == null));
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[0].value.contentData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value"));
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".contentData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value"));
Assert.IsNotNull(errors.FirstOrDefault(error => error.JsonPath == ".settingsData[0].values[0].value.settingsData[0].values[?(@.alias == 'variantText' && @.culture == 'da-DK' && @.segment == null)].value"));
});
}
}
[Test]

79
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Publish.cs
View File

@@ -2,8 +2,11 @@ using NUnit.Framework;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.ContentPublishing;
using Umbraco.Cms.Core.Models.Membership;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Services.OperationStatus;
using Umbraco.Cms.Tests.Common.Builders;
using Umbraco.Cms.Tests.Integration.Attributes;
namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services;
@@ -325,6 +328,82 @@ public partial class ContentPublishingServiceTests
Assert.AreEqual(2, content.PublishedCultures.Count());
}
[TestCase(true, "da-DK")]
[TestCase(false, "en-US")]
[TestCase(false, "en-US", "da-DK")]
public async Task Publish_Invalid_Invariant_Property_WithoutAllowEditInvariantFromNonDefault(bool expectedSuccess, params string[] culturesToRepublish)
=> await Publish_Invalid_Invariant_Property(expectedSuccess, culturesToRepublish);
[TestCase(false, "da-DK")]
[TestCase(false, "en-US")]
[TestCase(false, "en-US", "da-DK")]
[ConfigureBuilder(ActionName = nameof(ConfigureAllowEditInvariantFromNonDefaultTrue))]
public async Task Publish_Invalid_Invariant_Property_WithAllowEditInvariantFromNonDefault(bool expectedSuccess, params string[] culturesToRepublish)
=> await Publish_Invalid_Invariant_Property(expectedSuccess, culturesToRepublish);
private async Task Publish_Invalid_Invariant_Property(bool expectedSuccess, params string[] culturesToRepublish)
{
var contentType = await SetupVariantInvariantTest();
IContent content = new ContentBuilder()
.WithContentType(contentType)
.WithCultureName("en-US", "EN")
.WithCultureName("da-DK", "DA")
.Build();
content.SetValue("variantValue", "EN value", culture: "en-US");
content.SetValue("variantValue", "DA value", culture: "da-DK");
content.SetValue("invariantValue", "Invariant value");
ContentService.Save(content);
var result = await ContentPublishingService.PublishAsync(content.Key, MakeModel(new HashSet<string> { "en-US", "da-DK" }), Constants.Security.SuperUserKey);
Assert.IsTrue(result.Success);
content = ContentService.GetById(content.Key)!;
content.SetValue("variantValue", "EN value updated", culture: "en-US");
content.SetValue("variantValue", "DA value updated", culture: "da-DK");
content.SetValue("invariantValue", null);
ContentService.Save(content);
result = await ContentPublishingService.PublishAsync(content.Key, MakeModel(new HashSet<string>(culturesToRepublish)), Constants.Security.SuperUserKey);
content = ContentService.GetById(content.Key)!;
Assert.Multiple(() =>
{
Assert.AreEqual(null, content.GetValue("invariantValue", published: false));
Assert.AreEqual("EN value updated", content.GetValue("variantValue", culture: "en-US", published: false));
Assert.AreEqual("DA value updated", content.GetValue("variantValue", culture: "da-DK", published: false));
Assert.AreEqual("Invariant value", content.GetValue("invariantValue", published: true));
});
if (expectedSuccess)
{
Assert.Multiple(() =>
{
Assert.IsTrue(result.Success);
var expectedPublishedEnglishValue = culturesToRepublish.Contains("en-US")
? "EN value updated"
: "EN value";
var expectedPublishedDanishValue = culturesToRepublish.Contains("da-DK")
? "DA value updated"
: "DA value";
Assert.AreEqual(expectedPublishedEnglishValue, content.GetValue("variantValue", culture: "en-US", published: true));
Assert.AreEqual(expectedPublishedDanishValue, content.GetValue("variantValue", culture: "da-DK", published: true));
});
}
else
{
Assert.Multiple(() =>
{
Assert.IsFalse(result.Success);
Assert.AreEqual("EN value", content.GetValue("variantValue", culture: "en-US", published: true));
Assert.AreEqual("DA value", content.GetValue("variantValue", culture: "da-DK", published: true));
});
}
}
[Test]
public async Task Cannot_Publish_Non_Existing_Content()
{

42
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.cs
View File

@@ -1,5 +1,7 @@
using NUnit.Framework;
using Microsoft.Extensions.DependencyInjection;
using NUnit.Framework;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.ContentPublishing;
@@ -109,6 +111,41 @@ public partial class ContentPublishingServiceTests : UmbracoIntegrationTestWithC
return (langEn, langDa, contentType);
}
private async Task<IContentType> SetupVariantInvariantTest()
{
var langDa = new LanguageBuilder()
.WithCultureInfo("da-DK")
.Build();
await LanguageService.CreateAsync(langDa, Constants.Security.SuperUserKey);
var key = Guid.NewGuid();
var contentType = new ContentTypeBuilder()
.WithAlias("variantInvariantContent")
.WithName("Variant Invariant Content")
.WithKey(key)
.WithContentVariation(ContentVariation.Culture)
.AddAllowedContentType()
.WithKey(key)
.WithAlias("variantInvariantContent")
.Done()
.AddPropertyType()
.WithAlias("variantValue")
.WithVariations(ContentVariation.Culture)
.WithMandatory(true)
.Done()
.AddPropertyType()
.WithAlias("invariantValue")
.WithVariations(ContentVariation.Nothing)
.WithMandatory(true)
.Done()
.Build();
contentType.AllowedAsRoot = true;
await ContentTypeService.SaveAsync(contentType, Constants.Security.SuperUserKey);
return contentType;
}
protected override void CustomTestSetup(IUmbracoBuilder builder)
=> builder
.AddNotificationHandler<ContentPublishingNotification, ContentNotificationHandler>()
@@ -132,4 +169,7 @@ public partial class ContentPublishingServiceTests : UmbracoIntegrationTestWithC
public void Handle(ContentUnpublishingNotification notification) => UnpublishingContent?.Invoke(notification);
}
public static void ConfigureAllowEditInvariantFromNonDefaultTrue(IUmbracoBuilder builder)
=> builder.Services.Configure<ContentSettings>(config => config.AllowEditInvariantFromNonDefault = true);
}

6
tests/Umbraco.Tests.UnitTests/Umbraco.Core/Models/VariationTests.cs
View File

@@ -1,10 +1,12 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.
using Microsoft.Extensions.Options;
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Dictionary;
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.Models;
@@ -654,6 +656,8 @@ public class VariationTests
dataTypeService,
Mock.Of<ILocalizedTextService>(),
new ValueEditorCache(),
Mock.Of<ICultureDictionary>());
Mock.Of<ICultureDictionary>(),
Mock.Of<ILanguageService>(),
Mock.Of<IOptions<ContentSettings>>());
}
}

11
tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Services/PropertyValidationServiceTests.cs
View File

@@ -1,10 +1,12 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.
using Microsoft.Extensions.Options;
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Dictionary;
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.Models;
@@ -44,7 +46,14 @@ public class PropertyValidationServiceTests
var propEditors = new PropertyEditorCollection(new DataEditorCollection(() => new[] { dataEditor }));
validationService = new PropertyValidationService(propEditors, dataTypeService.Object, Mock.Of<ILocalizedTextService>(),new ValueEditorCache(), Mock.Of<ICultureDictionary>());
validationService = new PropertyValidationService(
propEditors,
dataTypeService.Object,
Mock.Of<ILocalizedTextService>(),
new ValueEditorCache(),
Mock.Of<ICultureDictionary>(),
Mock.Of<ILanguageService>(),
Mock.Of<IOptions<ContentSettings>>());
}
[Test]