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

Changes EnumExtensions to ContentVariationExtensions and adds more helper methods to replace a lot of the duplicate checking in our code. Adds code to enforce unique naming for culture names, adds supporting tests.

Browse Source
This commit is contained in:
Shannon
2018-06-01 15:20:16 +10:00
parent e907a085bd
commit b75cf3bc76
10 changed files with 230 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

82
src/Umbraco.Core/ContentVariationExtensions.cs Normal file
View File

@@ -0,0 +1,82 @@
using Umbraco.Core.Models;
namespace Umbraco.Core
{
/// <summary>
/// Provides extension methods for various enumerations.
/// </summary>
public static class ContentVariationExtensions
{
/// <summary>
/// Determines whether a variation has all flags set.
/// </summary>
public static bool Has(this ContentVariation variation, ContentVariation values)
=> (variation & values) == values;
/// <summary>
/// Determines whether a variation has at least a flag set.
/// </summary>
public static bool HasAny(this ContentVariation variation, ContentVariation values)
=> (variation & values) != ContentVariation.Unknown;
/// <summary>
/// Determines whether a variation does not support culture variations
/// </summary>
/// <param name="variation"></param>
/// <returns></returns>
public static bool DoesNotSupportCulture(this ContentVariation variation)
{
return !variation.HasAny(ContentVariation.CultureNeutral | ContentVariation.CultureSegment);
}
/// <summary>
/// Determines whether a variation does support culture variations
/// </summary>
/// <param name="variation"></param>
/// <returns></returns>
public static bool DoesSupportCulture(this ContentVariation variation)
{
return variation.HasAny(ContentVariation.CultureNeutral | ContentVariation.CultureSegment);
}
/// <summary>
/// Determines whether a variation does not support invariant variations
/// </summary>
/// <param name="variation"></param>
/// <returns></returns>
public static bool DoesNotSupportInvariant(this ContentVariation variation)
{
return !variation.HasAny(ContentVariation.InvariantNeutral | ContentVariation.InvariantSegment);
}
/// <summary>
/// Determines whether a variation does support invariant variations
/// </summary>
/// <param name="variation"></param>
/// <returns></returns>
public static bool DoesSupportInvariant(this ContentVariation variation)
{
return variation.HasAny(ContentVariation.InvariantNeutral | ContentVariation.InvariantSegment);
}
/// <summary>
/// Determines whether a variation does not support segment variations
/// </summary>
/// <param name="variation"></param>
/// <returns></returns>
public static bool DoesNotSupportSegment(this ContentVariation variation)
{
return !variation.HasAny(ContentVariation.InvariantSegment | ContentVariation.CultureSegment);
}
/// <summary>
/// Determines whether a variation does not support neutral variations
/// </summary>
/// <param name="variation"></param>
/// <returns></returns>
public static bool DoesNotSupportNeutral(this ContentVariation variation)
{
return !variation.HasAny(ContentVariation.InvariantNeutral | ContentVariation.CultureNeutral);
}
}
}

22
src/Umbraco.Core/EnumExtensions.cs
View File

@@ -1,22 +0,0 @@
using Umbraco.Core.Models;
namespace Umbraco.Core
{
/// <summary>
/// Provides extension methods for various enumerations.
/// </summary>
public static class EnumExtensions
{
/// <summary>
/// Determines whether a variation has all flags set.
/// </summary>
public static bool Has(this ContentVariation variation, ContentVariation values)
=> (variation & values) == values;
/// <summary>
/// Determines whether a variation has at least a flag set.
/// </summary>
public static bool HasAny(this ContentVariation variation, ContentVariation values)
=> (variation & values) != ContentVariation.Unknown;
}
}

14
src/Umbraco.Core/Models/ContentBase.cs
View File

@@ -180,7 +180,7 @@ namespace Umbraco.Core.Models
return;
}
if (!ContentTypeBase.Variations.HasAny(ContentVariation.CultureNeutral | ContentVariation.CultureSegment))
if (ContentTypeBase.Variations.DoesNotSupportCulture())
throw new NotSupportedException("Content type does not support varying name by culture.");
if (_cultureInfos == null)
@@ -210,7 +210,7 @@ namespace Umbraco.Core.Models
return;
}
if (!ContentTypeBase.Variations.HasAny(ContentVariation.CultureNeutral | ContentVariation.CultureSegment))
if (ContentTypeBase.Variations.DoesNotSupportCulture())
throw new NotSupportedException("Content type does not support varying name by culture.");
if (_cultureInfos == null) return;
@@ -224,7 +224,7 @@ namespace Umbraco.Core.Models
protected virtual void ClearNames()
{
if (!ContentTypeBase.Variations.HasAny(ContentVariation.CultureNeutral | ContentVariation.CultureSegment))
if (ContentTypeBase.Variations.DoesNotSupportCulture())
throw new NotSupportedException("Content type does not support varying name by culture.");
_cultureInfos = null;
@@ -327,13 +327,13 @@ namespace Umbraco.Core.Models
{
return Properties.Where(x =>
{
if (!culture.IsNullOrWhiteSpace() && !x.PropertyType.Variations.HasAny(ContentVariation.CultureNeutral | ContentVariation.CultureSegment))
if (!culture.IsNullOrWhiteSpace() && x.PropertyType.Variations.DoesNotSupportCulture())
return false; //has a culture, this prop is only culture invariant, ignore
if (culture.IsNullOrWhiteSpace() && !x.PropertyType.Variations.HasAny(ContentVariation.InvariantNeutral | ContentVariation.InvariantSegment))
if (culture.IsNullOrWhiteSpace() && x.PropertyType.Variations.DoesNotSupportInvariant())
return false; //no culture, this prop is only culture variant, ignore
if (!segment.IsNullOrWhiteSpace() && !x.PropertyType.Variations.HasAny(ContentVariation.InvariantSegment | ContentVariation.CultureSegment))
if (!segment.IsNullOrWhiteSpace() && x.PropertyType.Variations.DoesNotSupportSegment())
return false; //has segment, this prop is only segment neutral, ignore
if (segment.IsNullOrWhiteSpace() && !x.PropertyType.Variations.HasAny(ContentVariation.InvariantNeutral | ContentVariation.CultureNeutral))
if (segment.IsNullOrWhiteSpace() && x.PropertyType.Variations.DoesNotSupportNeutral())
return false; //no segment, this prop is only non segment neutral, ignore
return !x.IsValid(culture, segment);
}).ToArray();

1
src/Umbraco.Core/Persistence/Dtos/ContentVersionDto.cs
View File

@@ -30,6 +30,7 @@ namespace Umbraco.Core.Persistence.Dtos
[NullSetting(NullSetting = NullSettings.Null)]
public int? UserId { get => _userId == 0 ? null : _userId; set => _userId = value; } //return null if zero
//fixme - we need an index on this it is used almost always in querying and sorting
[Column("current")]
public bool Current { get; set; }

81
src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs
View File

@@ -14,6 +14,7 @@ using Umbraco.Core.Persistence.Factories;
using Umbraco.Core.Persistence.Querying;
using Umbraco.Core.Persistence.SqlSyntax;
using Umbraco.Core.Scoping;
using static Umbraco.Core.Persistence.NPocoSqlExtensions.Statics;
namespace Umbraco.Core.Persistence.Repositories.Implement
{
@@ -243,6 +244,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
// sanitize names: ensure we have an invariant name, and names are unique-ish
// (well, only invariant name is unique at the moment)
EnsureUniqueVariationNames(entity);
EnsureInvariantNameValues(entity, publishing);
entity.Name = EnsureUniqueNodeName(entity.ParentId, entity.Name);
@@ -338,7 +340,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
Database.Insert(dto);
// persist the variations
if (content.ContentType.Variations.HasAny(Models.ContentVariation.CultureNeutral | Models.ContentVariation.CultureSegment))
if (content.ContentType.Variations.DoesSupportCulture())
{
// names also impact 'edited'
foreach (var (culture, name) in content.Names)
@@ -423,6 +425,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
// sanitize names: ensure we have an invariant name, and names are unique-ish
// (well, only invariant name is unique at the moment)
EnsureUniqueVariationNames(entity);
EnsureInvariantNameValues(entity, publishing);
entity.Name = EnsureUniqueNodeName(entity.ParentId, entity.Name, entity.Id);
@@ -492,7 +495,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
if (content.PublishName != content.Name)
edited = true;
if (content.ContentType.Variations.HasAny(Models.ContentVariation.CultureNeutral | Models.ContentVariation.CultureSegment))
if (content.ContentType.Variations.DoesSupportCulture())
{
// names also impact 'edited'
foreach (var (culture, name) in content.Names)
@@ -901,7 +904,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
}
// set variations, if varying
temps = temps.Where(x => x.ContentType.Variations.HasAny(Models.ContentVariation.CultureNeutral | Models.ContentVariation.CultureSegment)).ToList();
temps = temps.Where(x => x.ContentType.Variations.DoesSupportCulture()).ToList();
if (temps.Count > 0)
{
// load all variations for all documents from database, in one query
@@ -936,7 +939,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
content.Properties = properties[dto.DocumentVersionDto.Id];
// set variations, if varying
if (contentType.Variations.HasAny(Models.ContentVariation.CultureNeutral | Models.ContentVariation.CultureSegment))
if (contentType.Variations.DoesSupportCulture())
{
var contentVariations = GetContentVariations(ltemp);
var documentVariations = GetDocumentVariations(ltemp);
@@ -1086,23 +1089,27 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
/// </summary>
private void EnsureInvariantNameValues(IContent content, bool publishing)
{
// here we have to ensure we have names and publish names,
// and to try and fix the situation if we have no name
// see also: U4-11286
// here we have to ensure we have names and publish names, and to try and fix the situation if we have no name, see also: U4-11286
// cannot save without an invariant name
if (string.IsNullOrWhiteSpace(content.Name))
if (string.IsNullOrWhiteSpace(content.Name) && content.ContentType.Variations.DoesNotSupportCulture())
throw new InvalidOperationException("Cannot save content with an empty name.");
if (content.ContentType.Variations.DoesSupportCulture())
{
// no variant name = error
if (content.Names.Count == 0)
throw new InvalidOperationException("Cannot save content with an empty name.");
// else pick the name for the default culture, or any name
var defaultCulture = LanguageRepository.GetDefaultIsoCode();
if (defaultCulture != null && content.Names.TryGetValue(defaultCulture, out var cultureName))
content.Name = cultureName;
else
content.Name = content.Names.First().Value; // only option is to take the first
// sync the invariant name to the default culture name if it's empty since we can't save with an empty invariant name.
// fixme should we always sync the invariant name with the default culture name when updating?
if (string.IsNullOrWhiteSpace(content.Name))
{
var defaultCulture = LanguageRepository.GetDefaultIsoCode();
if (defaultCulture != null && content.Names.TryGetValue(defaultCulture, out var cultureName))
content.Name = cultureName;
else
content.Name = content.Names.First().Value; // only option is to take the first
}
}
// cannot publish without an invariant name
@@ -1123,6 +1130,50 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
return EnsureUniqueNaming == false ? nodeName : base.EnsureUniqueNodeName(parentId, nodeName, id);
}
private void EnsureUniqueVariationNames(IContent content)
{
if (content.Names.Count == 0) return;
//Get all culture names at the same level
var template = SqlContext.Templates.Get("Umbraco.Core.DocumentRepository.EnsureUniqueVariationNames", tsql => tsql
.Select<ContentVersionCultureVariationDto>(x => x.Id, x => x.Name, x => x.LanguageId)
.From<ContentVersionCultureVariationDto>()
.InnerJoin<ContentVersionDto>()
.On<ContentVersionDto, ContentVersionCultureVariationDto>(x => x.Id, x => x.VersionId)
.InnerJoin<NodeDto>()
.On<NodeDto, ContentVersionDto>(x => x.NodeId, x => x.NodeId)
.Where<ContentVersionDto>(x => x.Current == SqlTemplate.Arg<bool>("current"))
.Where<NodeDto>(x => x.NodeObjectType == SqlTemplate.Arg<Guid>("nodeObjectType") && x.ParentId == SqlTemplate.Arg<int>("parentId"))
.OrderBy<ContentVersionCultureVariationDto>(x => x.LanguageId));
var sql = template.Sql(true, NodeObjectTypeId, content.ParentId);
var names = Database.Fetch<CultureNodeName>(sql)
.GroupBy(x => x.LanguageId)
.ToDictionary(x => x.Key, x => x);
if (names.Count == 0) return;
foreach(var n in content.Names)
{
var langId = LanguageRepository.GetIdByIsoCode(n.Key);
if (!langId.HasValue) continue;
if (names.TryGetValue(langId.Value, out var cultureNames))
{
var otherLangNames = cultureNames.Select(x => new SimilarNodeName { Id = x.Id, Name = x.Name });
var uniqueName = SimilarNodeName.GetUniqueName(otherLangNames, 0, n.Value);
content.SetName(uniqueName, n.Key);
}
}
}
private class CultureNodeName
{
public int Id { get; set; }
public string Name { get; set; }
public int LanguageId { get; set; }
}
#endregion
}
}

2
src/Umbraco.Core/Umbraco.Core.csproj
View File

@@ -303,8 +303,8 @@
<Compile Include="Constants-System.cs" />
<Compile Include="Constants-Web.cs" />
<Compile Include="Constants.cs" />
<Compile Include="ContentVariationExtensions.cs" />
<Compile Include="DisposableObjectSlim.cs" />
<Compile Include="EnumExtensions.cs" />
<Compile Include="Events\ExportedMemberEventArgs.cs" />
<Compile Include="Events\RolesEventArgs.cs" />
<Compile Include="Events\UserGroupWithUsers.cs" />

2
src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs
View File

@@ -406,7 +406,7 @@ namespace Umbraco.Tests.Persistence.Repositories
Assert.That(contentType.HasIdentity, Is.True);
Assert.That(textpage.HasIdentity, Is.True);
}
}
}
[Test]
public void SaveContentWithDefaultTemplate()

71
src/Umbraco.Tests/Services/ContentServiceTests.cs
View File

@@ -963,7 +963,7 @@ namespace Umbraco.Tests.Services
// Assert
Assert.That(content, Is.Not.Null);
Assert.That(content.HasIdentity, Is.False);
Assert.That(content.CreatorId, Is.EqualTo(-1)); //Default to -1 administrator
Assert.That(content.CreatorId, Is.EqualTo(0)); //Default to 0 (unknown) since we didn't explicitly set this in the Create call
}
[Test]
@@ -2499,6 +2499,75 @@ namespace Umbraco.Tests.Services
contentService.Save(content); // this is effectively a rollback?
contentService.Rollback(content); // just kill the method and offer options on values + template + name...
*/
}
[Test]
public void Ensure_Invariant_Name()
{
var languageService = ServiceContext.LocalizationService;
var langUk = new Language("en-UK") { IsDefaultVariantLanguage = true };
var langFr = new Language("fr-FR");
languageService.Save(langFr);
languageService.Save(langUk);
var contentTypeService = ServiceContext.ContentTypeService;
var contentType = contentTypeService.Get("umbTextpage");
contentType.Variations = ContentVariation.InvariantNeutral | ContentVariation.CultureNeutral;
contentType.AddPropertyType(new PropertyType(Constants.PropertyEditors.Aliases.TextBox, ValueStorageType.Nvarchar, "prop") { Variations = ContentVariation.CultureNeutral });
contentTypeService.Save(contentType);
var contentService = ServiceContext.ContentService;
var content = new Content(null, -1, contentType);
content.SetName("name-us", langUk.IsoCode);
content.SetName("name-fr", langFr.IsoCode);
contentService.Save(content);
//the name will be set to the default culture variant name
Assert.AreEqual("name-us", content.Name);
//fixme - should we always sync the invariant name even on update? see EnsureInvariantNameValues
////updating the default culture variant name should also update the invariant name so they stay in sync
//content.SetName("name-us-2", langUk.IsoCode);
//contentService.Save(content);
//Assert.AreEqual("name-us-2", content.Name);
}
[Test]
public void Ensure_Unique_Culture_Names()
{
var languageService = ServiceContext.LocalizationService;
var langUk = new Language("en-UK") { IsDefaultVariantLanguage = true };
var langFr = new Language("fr-FR");
languageService.Save(langFr);
languageService.Save(langUk);
var contentTypeService = ServiceContext.ContentTypeService;
var contentType = contentTypeService.Get("umbTextpage");
contentType.Variations = ContentVariation.InvariantNeutral | ContentVariation.CultureNeutral;
contentType.AddPropertyType(new PropertyType(Constants.PropertyEditors.Aliases.TextBox, ValueStorageType.Nvarchar, "prop") { Variations = ContentVariation.CultureNeutral });
contentTypeService.Save(contentType);
var contentService = ServiceContext.ContentService;
var content = new Content(null, -1, contentType);
content.SetName("root", langUk.IsoCode);
contentService.Save(content);
for (var i = 0; i < 5; i++)
{
var child = new Content(null, content, contentType);
child.SetName("child", langUk.IsoCode);
contentService.Save(child);
Assert.AreEqual("child" + (i == 0 ? "" : " (" + (i).ToString() + ")"), child.GetName(langUk.IsoCode));
}
}
[Test]

2
src/Umbraco.Web/Editors/ContentController.cs
View File

@@ -74,7 +74,7 @@ namespace Umbraco.Web.Editors
public bool AllowsCultureVariation()
{
var contentTypes = Services.ContentTypeService.GetAll();
return contentTypes.Any(contentType => contentType.Variations.HasAny(ContentVariation.CultureNeutral | ContentVariation.CultureSegment));
return contentTypes.Any(contentType => contentType.Variations.DoesSupportCulture());
}
/// <summary>

3
src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs
View File

@@ -58,8 +58,7 @@ namespace Umbraco.Web.WebApi.Binders
protected override bool ValidateCultureVariant(ContentItemSave postedItem, HttpActionContext actionContext)
{
var contentType = postedItem.PersistedContent.GetContentType();
if (contentType.Variations.HasAny(Core.Models.ContentVariation.CultureNeutral | Core.Models.ContentVariation.CultureSegment)
&& postedItem.Culture.IsNullOrWhiteSpace())
if (contentType.Variations.DoesSupportCulture() && postedItem.Culture.IsNullOrWhiteSpace())
{
//we cannot save a content item that is culture variant if no culture was specified in the request!
actionContext.Response = actionContext.Request.CreateValidationErrorResponse($"No 'Culture' found in request. Cannot save a content item that is of a {Core.Models.ContentVariation.CultureNeutral} content type without a specified culture.");