Ensures cultures set on content are correctly cased (#19290)
* Ensures cultures set on content are correctly cased and verifies with integration tests. * Improved test comments. * Move culture casing check into an extension method and use from content service. * Deduplicated test code and added more test cases * Only run invalid culture codes test on Windows --------- Co-authored-by: Kenn Jacobsen <kja@umbraco.dk>
This commit is contained in:
@@ -1575,4 +1575,22 @@ public static class StringExtensions
|
||||
// this is by far the fastest way to find string needles in a string haystack
|
||||
public static int CountOccurrences(this string haystack, string needle)
|
||||
=> haystack.Length - haystack.Replace(needle, string.Empty).Length;
|
||||
|
||||
/// <summary>
|
||||
/// Verifies the provided string is a valid culture code and returns it in a consistent casing.
|
||||
/// </summary>
|
||||
/// <param name="culture">Culture code.</param>
|
||||
/// <returns>Culture code in standard casing.</returns>
|
||||
public static string? EnsureCultureCode(this string? culture)
|
||||
{
|
||||
if (string.IsNullOrEmpty(culture) || culture == "*")
|
||||
{
|
||||
return culture;
|
||||
}
|
||||
|
||||
// Create as CultureInfo instance from provided name so we can ensure consistent casing of culture code when persisting.
|
||||
// This will accept mixed case but once created have a `Name` property that is consistently and correctly cased.
|
||||
// Will throw in an invalid culture code is provided.
|
||||
return new CultureInfo(culture).Name;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
using System.Collections.Specialized;
|
||||
using System.Collections.Specialized;
|
||||
using System.Diagnostics;
|
||||
using System.Globalization;
|
||||
using System.Runtime.ConstrainedExecution;
|
||||
using System.Runtime.Serialization;
|
||||
using Umbraco.Cms.Core.Models.Entities;
|
||||
using Umbraco.Extensions;
|
||||
@@ -288,6 +290,8 @@ public abstract class ContentBase : TreeEntityBase, IContentBase
|
||||
// set on variant content type
|
||||
if (ContentType.VariesByCulture())
|
||||
{
|
||||
culture = culture.EnsureCultureCode();
|
||||
|
||||
// invariant is ok
|
||||
if (culture.IsNullOrWhiteSpace())
|
||||
{
|
||||
@@ -297,7 +301,7 @@ public abstract class ContentBase : TreeEntityBase, IContentBase
|
||||
// clear
|
||||
else if (name.IsNullOrWhiteSpace())
|
||||
{
|
||||
ClearCultureInfo(culture!);
|
||||
ClearCultureInfo(culture);
|
||||
}
|
||||
|
||||
// set
|
||||
@@ -322,11 +326,6 @@ public abstract class ContentBase : TreeEntityBase, IContentBase
|
||||
|
||||
private void ClearCultureInfo(string culture)
|
||||
{
|
||||
if (culture == null)
|
||||
{
|
||||
throw new ArgumentNullException(nameof(culture));
|
||||
}
|
||||
|
||||
if (string.IsNullOrWhiteSpace(culture))
|
||||
{
|
||||
throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(culture));
|
||||
@@ -455,6 +454,7 @@ public abstract class ContentBase : TreeEntityBase, IContentBase
|
||||
$"No PropertyType exists with the supplied alias \"{propertyTypeAlias}\".");
|
||||
}
|
||||
|
||||
culture = culture.EnsureCultureCode();
|
||||
var updated = property.SetValue(value, culture, segment);
|
||||
if (updated)
|
||||
{
|
||||
|
||||
@@ -1184,6 +1184,8 @@ public class ContentService : RepositoryService, IContentService
|
||||
throw new ArgumentException("Cultures cannot be null or whitespace", nameof(cultures));
|
||||
}
|
||||
|
||||
cultures = cultures.Select(x => x.EnsureCultureCode()!).ToArray();
|
||||
|
||||
EventMessages evtMsgs = EventMessagesFactory.Get();
|
||||
|
||||
// we need to guard against unsaved changes before proceeding; the content will be saved, but we're not firing any saved notifications
|
||||
@@ -1263,7 +1265,7 @@ public class ContentService : RepositoryService, IContentService
|
||||
|
||||
EventMessages evtMsgs = EventMessagesFactory.Get();
|
||||
|
||||
culture = culture?.NullOrWhiteSpaceAsNull();
|
||||
culture = culture?.NullOrWhiteSpaceAsNull().EnsureCultureCode();
|
||||
|
||||
PublishedState publishedState = content.PublishedState;
|
||||
if (publishedState != PublishedState.Published && publishedState != PublishedState.Unpublished)
|
||||
@@ -2063,7 +2065,7 @@ public class ContentService : RepositoryService, IContentService
|
||||
cultures = ["*"];
|
||||
}
|
||||
|
||||
return cultures;
|
||||
return cultures.Select(x => x.EnsureCultureCode()!).ToArray();
|
||||
}
|
||||
|
||||
private static bool ProvidedCulturesIndicatePublishAll(string[] cultures) => cultures.Length == 0 || (cultures.Length == 1 && cultures[0] == "invariant");
|
||||
|
||||
@@ -0,0 +1,151 @@
|
||||
// Copyright (c) Umbraco.
|
||||
// See LICENSE for more details.
|
||||
|
||||
using NUnit.Framework;
|
||||
using Umbraco.Cms.Core;
|
||||
using Umbraco.Cms.Core.Models;
|
||||
using Umbraco.Cms.Core.Services;
|
||||
using Umbraco.Cms.Tests.Common.Builders;
|
||||
using Umbraco.Cms.Tests.Common.Builders.Extensions;
|
||||
using Umbraco.Cms.Tests.Common.Testing;
|
||||
using Umbraco.Cms.Tests.Integration.Testing;
|
||||
|
||||
// ReSharper disable CommentTypo
|
||||
// ReSharper disable StringLiteralTypo
|
||||
namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services;
|
||||
|
||||
[TestFixture]
|
||||
[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)]
|
||||
internal sealed class ContentServiceVariantTests : UmbracoIntegrationTest
|
||||
{
|
||||
private IContentService ContentService => GetRequiredService<IContentService>();
|
||||
|
||||
private IContentTypeService ContentTypeService => GetRequiredService<IContentTypeService>();
|
||||
|
||||
/// <summary>
|
||||
/// Provides both happy path with correctly cased cultures, and originally failing test cases for
|
||||
/// https://github.com/umbraco/Umbraco-CMS/issues/19287, where the culture codes are provided with inconsistent casing.
|
||||
/// </summary>
|
||||
[TestCase("en-US", "en-US", "en-US")]
|
||||
[TestCase("en-us", "en-us", "en-us")]
|
||||
[TestCase("en-US", "en-US", "en-us")]
|
||||
[TestCase("en-us", "en-US", "en-US")]
|
||||
[TestCase("en-US", "en-us", "en-US")]
|
||||
public async Task Can_Save_And_Publish_With_Inconsistent_Provision_Of_Culture_Codes(string cultureNameCultureCode, string valueCultureCode, string publishCultureCode)
|
||||
{
|
||||
var contentType = await SetupVariantTest();
|
||||
|
||||
IContent content = ContentService.Create("Test Item", Constants.System.Root, contentType);
|
||||
content.SetCultureName("Test item", cultureNameCultureCode);
|
||||
content.SetValue("title", "Title", valueCultureCode);
|
||||
ContentService.Save(content);
|
||||
|
||||
var publishResult = ContentService.Publish(content, [publishCultureCode]);
|
||||
Assert.IsTrue(publishResult.Success);
|
||||
|
||||
content = ContentService.GetById(content.Key)!;
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.IsTrue(content.Published);
|
||||
Assert.AreEqual(1, content.PublishedCultures.Count());
|
||||
Assert.AreEqual("en-US", content.PublishedCultures.FirstOrDefault());
|
||||
});
|
||||
}
|
||||
|
||||
[TestCase("en-US", "en-US", "en-US")]
|
||||
[TestCase("en-us", "en-us", "en-us")]
|
||||
[TestCase("en-US", "en-US", "en-us")]
|
||||
[TestCase("en-us", "en-US", "en-US")]
|
||||
[TestCase("en-US", "en-us", "en-US")]
|
||||
public async Task Can_Unpublish_With_Inconsistent_Provision_Of_Culture_Codes(string cultureNameCultureCode, string valueCultureCode, string unpublishCultureCode)
|
||||
{
|
||||
var contentType = await SetupVariantTest();
|
||||
|
||||
IContent content = ContentService.Create("Test Item", Constants.System.Root, contentType);
|
||||
content.SetCultureName("Test item", cultureNameCultureCode);
|
||||
content.SetValue("title", "Title", valueCultureCode);
|
||||
ContentService.Save(content);
|
||||
// use correctly cased culture code to publish
|
||||
ContentService.Publish(content, ["en-US"]);
|
||||
|
||||
var unpublishResult = ContentService.Unpublish(content, unpublishCultureCode);
|
||||
Assert.IsTrue(unpublishResult.Success);
|
||||
|
||||
content = ContentService.GetById(content.Key)!;
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.IsFalse(content.Published);
|
||||
Assert.AreEqual(0, content.PublishedCultures.Count());
|
||||
});
|
||||
}
|
||||
|
||||
[TestCase("en-US", "en-US", "en-US")]
|
||||
[TestCase("en-us", "en-us", "en-us")]
|
||||
[TestCase("en-US", "en-US", "en-us")]
|
||||
[TestCase("en-us", "en-US", "en-US")]
|
||||
[TestCase("en-US", "en-us", "en-US")]
|
||||
public async Task Can_Publish_Branch_With_Inconsistent_Provision_Of_Culture_Codes(string cultureNameCultureCode, string valueCultureCode, string publishCultureCode)
|
||||
{
|
||||
var contentType = await SetupVariantTest();
|
||||
|
||||
IContent root = ContentService.Create("Root", Constants.System.Root, contentType);
|
||||
root.SetCultureName("Root", cultureNameCultureCode);
|
||||
root.SetValue("title", "Root Title", valueCultureCode);
|
||||
ContentService.Save(root);
|
||||
|
||||
var child = ContentService.Create("Child", root.Id, contentType);
|
||||
child.SetCultureName("Child", cultureNameCultureCode);
|
||||
child.SetValue("title", "Child Title", valueCultureCode);
|
||||
ContentService.Save(child);
|
||||
|
||||
var publishResult = ContentService.PublishBranch(root, PublishBranchFilter.All, [publishCultureCode]);
|
||||
Assert.AreEqual(2, publishResult.Count());
|
||||
Assert.IsTrue(publishResult.First().Success);
|
||||
Assert.IsTrue(publishResult.Last().Success);
|
||||
|
||||
root = ContentService.GetById(root.Key)!;
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.IsTrue(root.Published);
|
||||
Assert.AreEqual(1, root.PublishedCultures.Count());
|
||||
Assert.AreEqual("en-US", root.PublishedCultures.FirstOrDefault());
|
||||
});
|
||||
|
||||
child = ContentService.GetById(child.Key)!;
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.IsTrue(child.Published);
|
||||
Assert.AreEqual(1, child.PublishedCultures.Count());
|
||||
Assert.AreEqual("en-US", child.PublishedCultures.FirstOrDefault());
|
||||
});
|
||||
}
|
||||
|
||||
private async Task<IContentType> SetupVariantTest()
|
||||
{
|
||||
var key = Guid.NewGuid();
|
||||
var contentType = new ContentTypeBuilder()
|
||||
.WithAlias("variantContent")
|
||||
.WithName("Variant Content")
|
||||
.WithKey(key)
|
||||
.WithContentVariation(ContentVariation.Culture)
|
||||
.AddPropertyGroup()
|
||||
.WithAlias("content")
|
||||
.WithName("Content")
|
||||
.WithSupportsPublishing(true)
|
||||
.AddPropertyType()
|
||||
.WithAlias("title")
|
||||
.WithName("Title")
|
||||
.WithVariations(ContentVariation.Culture)
|
||||
.Done()
|
||||
.Done()
|
||||
.Build();
|
||||
|
||||
contentType.AllowedAsRoot = true;
|
||||
await ContentTypeService.CreateAsync(contentType, Constants.Security.SuperUserKey);
|
||||
|
||||
contentType.AllowedContentTypes = [new ContentTypeSort(contentType.Key, 0, contentType.Alias)];
|
||||
await ContentTypeService.UpdateAsync(contentType, Constants.Security.SuperUserKey);
|
||||
|
||||
return contentType;
|
||||
}
|
||||
}
|
||||
@@ -3,6 +3,7 @@
|
||||
|
||||
using System.Collections.Generic;
|
||||
using System.Diagnostics;
|
||||
using System.Globalization;
|
||||
using System.IO;
|
||||
using System.Linq;
|
||||
using System.Runtime.InteropServices;
|
||||
@@ -395,4 +396,17 @@ public class StringExtensionsTests
|
||||
var ids = input.GetIdsFromPathReversed();
|
||||
Assert.AreEqual(expected, string.Join(",", ids));
|
||||
}
|
||||
|
||||
[TestCase(null, null)]
|
||||
[TestCase("", "")]
|
||||
[TestCase("*", "*")]
|
||||
[TestCase("en", "en")]
|
||||
[TestCase("EN", "en")]
|
||||
[TestCase("en-US", "en-US")]
|
||||
[TestCase("en-gb", "en-GB")]
|
||||
public void EnsureCultureCode_ReturnsExpectedResult(string? culture, string? expected) => Assert.AreEqual(expected, culture.EnsureCultureCode());
|
||||
|
||||
[Test]
|
||||
[Platform(Include = "Win")]
|
||||
public void EnsureCultureCode_ThrowsOnUnrecognisedCode() => Assert.Throws<CultureNotFoundException>(() => "xxx-xxx".EnsureCultureCode());
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user