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

Segments: Only validate segment values for cultures they are defined for (closes #21029) (#21033)

Browse Source 
* Only validate segment values for cultures they are defined for.

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Integration test suppressions.

* Remove previous implementation using ISegmentService and rely on values provided in the model to determine segments with cultures.

* Omit null culture and remove passing but unrealistic tests.

* Fixed nullability error.

* Apply suggestions from code review

Co-authored-by: Kenn Jacobsen <kja@umbraco.dk>

* Relocated function following code review.

* Reset unchanged files.

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Kenn Jacobsen <kja@umbraco.dk>
This commit is contained in:
Andy Butland
2025-12-04 12:54:44 +01:00
committed by GitHub
parent effccef81d
commit e6c7ef8904
4 changed files with 290 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

47
src/Umbraco.Core/Services/ContentValidationServiceBase.cs
View File

@@ -50,7 +50,7 @@ internal abstract class ContentValidationServiceBase<TContentType>
cultures = await GetCultureCodes();
}
// we don't have any managed segments, so we have to make do with the ones passed in the model
// We don't have managed segments, so we have to make do with the ones passed in the model.
var segments =
new string?[] { null }
.Union(contentEditingModelBase.Variants
@@ -105,15 +105,32 @@ internal abstract class ContentValidationServiceBase<TContentType>
}
}
if (cultureAndSegmentVariantPropertyTypes.Length > 0)
{
// Get a mapping of segments to their associated cultures based on the variants and properties provided in the model.
// Without managed segments again we need to rely on the model data.
Dictionary<string, HashSet<string>> segmentCultures = GetPopulatedSegmentCultures(contentEditingModelBase, cultures);
foreach (IPropertyType propertyType in cultureAndSegmentVariantPropertyTypes)
{
foreach (var culture in cultures)
{
foreach (var segment in segments.DefaultIfEmpty(null))
{
// Skip validation if the segment has cultures defined and the current culture is not included.
if (segment is not null &&
segmentCultures.TryGetValue(segment, out HashSet<string>? associatedCultures) &&
associatedCultures.Contains(culture) is false)
{
continue;
}
var validationContext = new PropertyValidationContext
{
Culture = culture, Segment = segment, CulturesBeingValidated = cultures, SegmentsBeingValidated = segments
Culture = culture,
Segment = segment,
CulturesBeingValidated = cultures,
SegmentsBeingValidated = segments,
};
PropertyValueModel? propertyValueModel = contentEditingModelBase
@@ -123,6 +140,7 @@ internal abstract class ContentValidationServiceBase<TContentType>
}
}
}
}
return new ContentValidationResult { ValidationErrors = validationErrors };
}
@@ -140,6 +158,31 @@ internal abstract class ContentValidationServiceBase<TContentType>
private async Task<string[]> GetCultureCodes() => (await _languageService.GetAllIsoCodesAsync()).ToArray();
/// <summary>
/// Gets a dictionary of segments along with the cultures they are associated with.
/// </summary>
/// <param name="contentEditingModel">The content editing model.</param>
/// <param name="cultures">The cultures to consider when finding associated cultures for each segment.</param>
/// <returns>
/// A dictionary where the key is a unique segment from <see cref="ContentEditingModelBase.Variants"/> and the value is
/// the set of cultures that have at least one property defined for that segment in <see cref="ContentEditingModelBase.Properties"/>.
/// </returns>
/// <remarks>
/// Internal to support unit testing.
/// </remarks>
internal static Dictionary<string, HashSet<string>> GetPopulatedSegmentCultures(ContentEditingModelBase contentEditingModel, string[] cultures)
{
IEnumerable<string> uniqueSegments = contentEditingModel.Variants.Select(variant => variant.Segment).WhereNotNull().Distinct();
return uniqueSegments.ToDictionary(
segment => segment,
segment => contentEditingModel.Properties
.Where(property => property.Segment.InvariantEquals(segment))
.Where(property => property.Culture is not null && cultures.Contains(property.Culture))
.Select(property => property.Culture!)
.ToHashSet());
}
private IEnumerable<PropertyValidationError> ValidateProperty(IPropertyType propertyType, PropertyValueModel? propertyValueModel, PropertyValidationContext validationContext)
{
ValidationResult[] validationResults = _propertyValidationService

16
tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml
View File

@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<!-- https://learn.microsoft.com/dotnet/fundamentals/package-validation/diagnostic-ids -->
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
<Suppression>
@@ -85,6 +85,13 @@
<Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.ContentEditingServiceTests.Can_Validate_Valid_Variant_Content</Target>
<Left>lib/net10.0/Umbraco.Tests.Integration.dll</Left>
<Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.ContentEditingServiceTests.Relate(Umbraco.Cms.Core.Models.IContent,Umbraco.Cms.Core.Models.IContent)</Target>
@@ -92,4 +99,11 @@
<Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.ContentEditingServiceTests.Will_Fail_Invalid_Variant_Content</Target>
<Left>lib/net10.0/Umbraco.Tests.Integration.dll</Left>
<Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
</Suppressions>

71
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.Validate.cs
View File

@@ -3,9 +3,11 @@ using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.ContentEditing;
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.Common.Builders.Extensions;
using Umbraco.Cms.Tests.Integration.Attributes;
namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services;
@@ -60,7 +62,7 @@ public partial class ContentEditingServiceTests
}
[Test]
public async Task Can_Validate_Valid_Variant_Content()
public async Task Can_Validate_Valid_Culture_Variant_Content()
{
var content = await CreateCultureVariantContent();
@@ -85,7 +87,7 @@ public partial class ContentEditingServiceTests
}
[Test]
public async Task Will_Fail_Invalid_Variant_Content()
public async Task Will_Fail_Invalid_Culture_Variant_Content()
{
var content = await CreateCultureVariantContent();
@@ -111,6 +113,71 @@ public partial class ContentEditingServiceTests
Assert.AreEqual("#validation_invalidNull", result.Result.ValidationErrors.Single(x => x.Alias == "variantTitle" && x.Culture == "da-DK").ErrorMessages[0]);
}
[Test]
public async Task Can_Validate_Valid_Culture_And_Segment_Variant_Content()
{
var content = await CreateCultureAndSegmentVariantContent(ContentVariation.Culture);
var validateContentUpdateModel = new ValidateContentUpdateModel
{
Properties =
[
new PropertyValueModel { Alias = "invariantTitle", Value = "The updated invariant title" },
new PropertyValueModel { Alias = "variantTitle", Value = "The updated English default segment title", Culture = "en-US" },
new PropertyValueModel { Alias = "variantTitle", Value = "The updated Danish default segment title", Culture = "da-DK" },
new PropertyValueModel { Alias = "variantTitle", Value = "The updated English segment 1 title", Culture = "en-US", Segment = "seg-1" },
new PropertyValueModel { Alias = "variantTitle", Value = "The updated Danish segment 1 title", Culture = "da-DK", Segment = "seg-1" },
new PropertyValueModel { Alias = "variantTitle", Value = "The updated English segment 2 title", Culture = "en-US", Segment = "seg-2" },
new PropertyValueModel { Alias = "variantTitle", Value = "The updated Danish segment 2 title", Culture = "da-DK", Segment = "seg-2" }
],
Variants =
[
new VariantModel { Culture = "en-US", Segment = "seg-1", Name = "Updated English segment 1 Name" },
new VariantModel { Culture = "da-DK", Segment = "seg-1", Name = "Updated Danish segment 1 Name" },
new VariantModel { Culture = "en-US", Segment = "seg-2", Name = "Updated English segment 2 Name" },
new VariantModel { Culture = "da-DK", Segment = "seg-2", Name = "Updated Danish segment 2 Name" }
],
};
Attempt<ContentValidationResult, ContentEditingOperationStatus> result = await ContentEditingService.ValidateUpdateAsync(content.Key, validateContentUpdateModel, Constants.Security.SuperUserKey);
Assert.IsTrue(result.Success);
Assert.AreEqual(ContentEditingOperationStatus.Success, result.Status);
}
[Test]
public async Task Will_Fail_Invalid_Culture_And_Segment_Variant_Content()
{
var content = await CreateCultureAndSegmentVariantContent(ContentVariation.Culture);
var validateContentUpdateModel = new ValidateContentUpdateModel
{
Properties =
[
new PropertyValueModel { Alias = "invariantTitle", Value = "The updated invariant title" },
new PropertyValueModel { Alias = "variantTitle", Value = "The updated English default segment title", Culture = "en-US" },
new PropertyValueModel { Alias = "variantTitle", Value = "The updated Danish default segment title", Culture = "da-DK" },
new PropertyValueModel { Alias = "variantTitle", Value = "The updated English segment 1 title", Culture = "en-US", Segment = "seg-1" },
new PropertyValueModel { Alias = "variantTitle", Value = "The updated Danish segment 1 title", Culture = "da-DK", Segment = "seg-1" },
new PropertyValueModel { Alias = "variantTitle", Value = "The updated English segment 2 title", Culture = "en-US", Segment = "seg-2" },
new PropertyValueModel { Alias = "variantTitle", Value = null, Culture = "da-DK", Segment = "seg-2" }
],
Variants =
[
new VariantModel { Culture = "en-US", Segment = "seg-1", Name = "Updated English segment 1 Name" },
new VariantModel { Culture = "da-DK", Segment = "seg-1", Name = "Updated Danish segment 1 Name" },
new VariantModel { Culture = "en-US", Segment = "seg-2", Name = "Updated English segment 2 Name" },
new VariantModel { Culture = "da-DK", Segment = "seg-2", Name = "Updated Danish segment 2 Name" }
],
};
Attempt<ContentValidationResult, ContentEditingOperationStatus> result = await ContentEditingService.ValidateUpdateAsync(content.Key, validateContentUpdateModel, Constants.Security.SuperUserKey);
Assert.IsFalse(result.Success);
Assert.AreEqual(ContentEditingOperationStatus.PropertyValidationError, result.Status);
Assert.AreEqual(1, result.Result.ValidationErrors.Count());
Assert.AreEqual("#validation_invalidNull", result.Result.ValidationErrors.Single(x => x.Alias == "variantTitle" && x.Culture == "da-DK" && x.Segment == "seg-2").ErrorMessages[0]);
}
[Test]
public async Task Will_Succeed_For_Invalid_Variant_Content_Without_Access_To_Edited_Culture()
{

148
tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/ContentValidationServiceTests.cs Normal file
View File

@@ -0,0 +1,148 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.
using NUnit.Framework;
using Umbraco.Cms.Core.Models.ContentEditing;
using Umbraco.Cms.Core.Services;
namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Models.ContentEditing;
[TestFixture]
public class ContentValidationServiceTests
{
// Concrete implementation for testing the abstract base class.
private class TestContentEditingModel : ContentEditingModelBase
{
}
[Test]
public void GetPopulatedSegmentCultures_ReturnsEmptyDictionary_WhenNoSegmentsInVariants()
{
var model = new TestContentEditingModel
{
Variants =
[
new VariantModel { Name = "English", Culture = "en-US", Segment = null },
new VariantModel { Name = "Danish", Culture = "da-DK", Segment = null }
],
Properties =
[
new PropertyValueModel { Alias = "title", Value = "Hello", Culture = "en-US", Segment = null }
],
};
var result = ContentValidationService.GetPopulatedSegmentCultures(model, ["en-US", "da-DK"]);
Assert.That(result, Is.Empty);
}
[Test]
public void GetPopulatedSegmentCultures_ReturnsSegmentWithCultures_WhenPropertiesExistForSegment()
{
var model = new TestContentEditingModel
{
Variants =
[
new VariantModel { Name = "English Default", Culture = "en-US", Segment = null },
new VariantModel { Name = "English Segment 1", Culture = "en-US", Segment = "segment-1" },
new VariantModel { Name = "Danish Segment 1", Culture = "da-DK", Segment = "segment-1" }
],
Properties =
[
new PropertyValueModel { Alias = "title", Value = "Hello", Culture = "en-US", Segment = null },
new PropertyValueModel { Alias = "title", Value = "Hello Segment 1", Culture = "en-US", Segment = "segment-1" },
new PropertyValueModel { Alias = "title", Value = "Hej Segment 1", Culture = "da-DK", Segment = "segment-1" }
],
};
var result = ContentValidationService.GetPopulatedSegmentCultures(model, ["en-US", "da-DK"]);
Assert.That(result, Has.Count.EqualTo(1));
Assert.That(result.ContainsKey("segment-1"), Is.True);
Assert.That(result["segment-1"], Does.Contain("en-US"));
Assert.That(result["segment-1"], Does.Contain("da-DK"));
}
[Test]
public void GetPopulatedSegmentCultures_ReturnsOnlyCulturesWithProperties_WhenSomeSegmentCultureCombinationsHaveNoProperties()
{
var model = new TestContentEditingModel
{
Variants =
[
new VariantModel { Name = "English Segment 1", Culture = "en-US", Segment = "segment-1" },
new VariantModel { Name = "Danish Segment 1", Culture = "da-DK", Segment = "segment-1" }
],
Properties =
[
// Only en-US has a property for the mobile segment
new PropertyValueModel { Alias = "title", Value = "Hello Segment 1", Culture = "en-US", Segment = "segment-1" }
],
};
var result = ContentValidationService.GetPopulatedSegmentCultures(model, ["en-US", "da-DK"]);
Assert.That(result, Has.Count.EqualTo(1));
Assert.That(result.ContainsKey("segment-1"), Is.True);
Assert.That(result["segment-1"], Does.Contain("en-US"));
Assert.That(result["segment-1"], Does.Not.Contain("da-DK"));
}
[Test]
public void GetPopulatedSegmentCultures_ExcludesCulturesNotInCulturesParameter()
{
var model = new TestContentEditingModel
{
Variants =
[
new VariantModel { Name = "English Segment 1", Culture = "en-US", Segment = "segment-1" },
new VariantModel { Name = "Danish Segment 1", Culture = "da-DK", Segment = "segment-1" },
new VariantModel { Name = "German Segment 1", Culture = "de-DE", Segment = "segment-1" }
],
Properties =
[
new PropertyValueModel { Alias = "title", Value = "Hello Segment 1", Culture = "en-US", Segment = "segment-1" },
new PropertyValueModel { Alias = "title", Value = "Hej Segment 1", Culture = "da-DK", Segment = "segment-1" },
new PropertyValueModel { Alias = "title", Value = "Hallo Segment 1", Culture = "de-DE", Segment = "segment-1" }
],
};
// Only validating en-US and da-DK, not de-DE
var result = ContentValidationService.GetPopulatedSegmentCultures(model, ["en-US", "da-DK"]);
Assert.That(result, Has.Count.EqualTo(1));
Assert.That(result["segment-1"], Does.Contain("en-US"));
Assert.That(result["segment-1"], Does.Contain("da-DK"));
Assert.That(result["segment-1"], Does.Not.Contain("de-DE"));
}
[Test]
public void GetPopulatedSegmentCultures_HandlesMultipleSegments()
{
var model = new TestContentEditingModel
{
Variants =
[
new VariantModel { Name = "English Segment 1", Culture = "en-US", Segment = "segment-1" },
new VariantModel { Name = "English Segment 2", Culture = "en-US", Segment = "segment-2" },
new VariantModel { Name = "Danish Segment 1", Culture = "da-DK", Segment = "segment-1" }
],
Properties =
[
new PropertyValueModel { Alias = "title", Value = "Hello Segment 1", Culture = "en-US", Segment = "segment-1" },
new PropertyValueModel { Alias = "title", Value = "Hello Segment 2", Culture = "en-US", Segment = "segment-2" },
new PropertyValueModel { Alias = "title", Value = "Hej Segment 1", Culture = "da-DK", Segment = "segment-1" }
],
};
var result = ContentValidationService.GetPopulatedSegmentCultures(model, ["en-US", "da-DK"]);
Assert.That(result, Has.Count.EqualTo(2));
Assert.That(result.ContainsKey("segment-1"), Is.True);
Assert.That(result.ContainsKey("segment-2"), Is.True);
Assert.That(result["segment-1"], Does.Contain("en-US"));
Assert.That(result["segment-1"], Does.Contain("da-DK"));
Assert.That(result["segment-2"], Does.Contain("en-US"));
Assert.That(result["segment-2"], Does.Not.Contain("da-DK"));
}
}