diff --git a/src/Umbraco.Core/ContentVariationExtensions.cs b/src/Umbraco.Core/ContentVariationExtensions.cs new file mode 100644 index 0000000000..275cc61425 --- /dev/null +++ b/src/Umbraco.Core/ContentVariationExtensions.cs @@ -0,0 +1,82 @@ +using Umbraco.Core.Models; + +namespace Umbraco.Core +{ + /// + /// Provides extension methods for various enumerations. + /// + public static class ContentVariationExtensions + { + /// + /// Determines whether a variation has all flags set. + /// + public static bool Has(this ContentVariation variation, ContentVariation values) + => (variation & values) == values; + + /// + /// Determines whether a variation has at least a flag set. + /// + public static bool HasAny(this ContentVariation variation, ContentVariation values) + => (variation & values) != ContentVariation.Unknown; + + /// + /// Determines whether a variation does not support culture variations + /// + /// + /// + public static bool DoesNotSupportCulture(this ContentVariation variation) + { + return !variation.HasAny(ContentVariation.CultureNeutral | ContentVariation.CultureSegment); + } + + /// + /// Determines whether a variation does support culture variations + /// + /// + /// + public static bool DoesSupportCulture(this ContentVariation variation) + { + return variation.HasAny(ContentVariation.CultureNeutral | ContentVariation.CultureSegment); + } + + /// + /// Determines whether a variation does not support invariant variations + /// + /// + /// + public static bool DoesNotSupportInvariant(this ContentVariation variation) + { + return !variation.HasAny(ContentVariation.InvariantNeutral | ContentVariation.InvariantSegment); + } + + /// + /// Determines whether a variation does support invariant variations + /// + /// + /// + public static bool DoesSupportInvariant(this ContentVariation variation) + { + return variation.HasAny(ContentVariation.InvariantNeutral | ContentVariation.InvariantSegment); + } + + /// + /// Determines whether a variation does not support segment variations + /// + /// + /// + public static bool DoesNotSupportSegment(this ContentVariation variation) + { + return !variation.HasAny(ContentVariation.InvariantSegment | ContentVariation.CultureSegment); + } + + /// + /// Determines whether a variation does not support neutral variations + /// + /// + /// + public static bool DoesNotSupportNeutral(this ContentVariation variation) + { + return !variation.HasAny(ContentVariation.InvariantNeutral | ContentVariation.CultureNeutral); + } + } +} diff --git a/src/Umbraco.Core/EnumExtensions.cs b/src/Umbraco.Core/EnumExtensions.cs deleted file mode 100644 index 58e14bcadf..0000000000 --- a/src/Umbraco.Core/EnumExtensions.cs +++ /dev/null @@ -1,22 +0,0 @@ -using Umbraco.Core.Models; - -namespace Umbraco.Core -{ - /// - /// Provides extension methods for various enumerations. - /// - public static class EnumExtensions - { - /// - /// Determines whether a variation has all flags set. - /// - public static bool Has(this ContentVariation variation, ContentVariation values) - => (variation & values) == values; - - /// - /// Determines whether a variation has at least a flag set. - /// - public static bool HasAny(this ContentVariation variation, ContentVariation values) - => (variation & values) != ContentVariation.Unknown; - } -} diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index fceaba158f..1bebdd39c5 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -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(); diff --git a/src/Umbraco.Core/Persistence/Dtos/ContentVersionDto.cs b/src/Umbraco.Core/Persistence/Dtos/ContentVersionDto.cs index ee706d2735..3ec40c74b3 100644 --- a/src/Umbraco.Core/Persistence/Dtos/ContentVersionDto.cs +++ b/src/Umbraco.Core/Persistence/Dtos/ContentVersionDto.cs @@ -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; } diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index e723ece853..81f2bde2d7 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -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 /// 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(x => x.Id, x => x.Name, x => x.LanguageId) + .From() + .InnerJoin() + .On(x => x.Id, x => x.VersionId) + .InnerJoin() + .On(x => x.NodeId, x => x.NodeId) + .Where(x => x.Current == SqlTemplate.Arg("current")) + .Where(x => x.NodeObjectType == SqlTemplate.Arg("nodeObjectType") && x.ParentId == SqlTemplate.Arg("parentId")) + .OrderBy(x => x.LanguageId)); + + var sql = template.Sql(true, NodeObjectTypeId, content.ParentId); + var names = Database.Fetch(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 } } diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index c4d7fab76c..2d9c87df64 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -303,8 +303,8 @@ + - diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs index 9675707439..6285e3cd25 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs @@ -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() diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index c99ef333b9..11f9502263 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -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] diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 2019a87f4d..a3949fcf70 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -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()); } /// diff --git a/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs b/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs index cc8523b107..e45b676177 100644 --- a/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs +++ b/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs @@ -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.");