From b75cf3bc764b0faaf5147b9e04f5c0022492f370 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 1 Jun 2018 15:20:16 +1000 Subject: [PATCH 1/5] 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. --- .../ContentVariationExtensions.cs | 82 +++++++++++++++++++ src/Umbraco.Core/EnumExtensions.cs | 22 ----- src/Umbraco.Core/Models/ContentBase.cs | 14 ++-- .../Persistence/Dtos/ContentVersionDto.cs | 1 + .../Implement/DocumentRepository.cs | 81 ++++++++++++++---- src/Umbraco.Core/Umbraco.Core.csproj | 2 +- .../Repositories/ContentRepositoryTest.cs | 2 +- .../Services/ContentServiceTests.cs | 71 +++++++++++++++- src/Umbraco.Web/Editors/ContentController.cs | 2 +- .../WebApi/Binders/ContentItemBinder.cs | 3 +- 10 files changed, 230 insertions(+), 50 deletions(-) create mode 100644 src/Umbraco.Core/ContentVariationExtensions.cs delete mode 100644 src/Umbraco.Core/EnumExtensions.cs 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."); From 3cfd9c5b00806e6460fd7692f18b1bb3c2580fa6 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 1 Jun 2018 15:51:45 +1000 Subject: [PATCH 2/5] ensures the unique name check is not performed against itself --- .../Repositories/Implement/DocumentRepository.cs | 6 ++++-- src/Umbraco.Tests/Services/ContentServiceTests.cs | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 81f2bde2d7..02299556ac 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -1144,10 +1144,12 @@ namespace Umbraco.Core.Persistence.Repositories.Implement .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")) + .Where(x => x.NodeObjectType == SqlTemplate.Arg("nodeObjectType") + && x.ParentId == SqlTemplate.Arg("parentId") + && x.NodeId != SqlTemplate.Arg($"{Constants.DatabaseSchema.Tables.Node}.id")) .OrderBy(x => x.LanguageId)); - var sql = template.Sql(true, NodeObjectTypeId, content.ParentId); + var sql = template.Sql(true, NodeObjectTypeId, content.ParentId, content.Id); var names = Database.Fetch(sql) .GroupBy(x => x.LanguageId) .ToDictionary(x => x.Key, x => x); diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 11f9502263..34cab4f970 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -2565,7 +2565,10 @@ namespace Umbraco.Tests.Services 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)); + //Save it again to ensure that the unique check is not performed again against it's own name + contentService.Save(child); Assert.AreEqual("child" + (i == 0 ? "" : " (" + (i).ToString() + ")"), child.GetName(langUk.IsoCode)); } } From fd3e6f4d9c4519c95f26fe19a2d3b00e45de3479 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 1 Jun 2018 15:52:55 +1000 Subject: [PATCH 3/5] turns out there's no need for the full table name, the sql generator figures this out --- .../Persistence/Repositories/Implement/DocumentRepository.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 02299556ac..7e9697d544 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -1146,7 +1146,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement .Where(x => x.Current == SqlTemplate.Arg("current")) .Where(x => x.NodeObjectType == SqlTemplate.Arg("nodeObjectType") && x.ParentId == SqlTemplate.Arg("parentId") - && x.NodeId != SqlTemplate.Arg($"{Constants.DatabaseSchema.Tables.Node}.id")) + && x.NodeId != SqlTemplate.Arg("id")) .OrderBy(x => x.LanguageId)); var sql = template.Sql(true, NodeObjectTypeId, content.ParentId, content.Id); From 90e6769169003a4ba7839ec2c1937fcafa6448f2 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 1 Jun 2018 16:04:20 +1000 Subject: [PATCH 4/5] adds check for EnsureUniqueNaming --- .../Persistence/Repositories/Implement/DocumentRepository.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 7e9697d544..1fec78954b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -1132,7 +1132,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement private void EnsureUniqueVariationNames(IContent content) { - if (content.Names.Count == 0) return; + if (!EnsureUniqueNaming || content.Names.Count == 0) return; //Get all culture names at the same level From 504b6cb079a6df8e94c0ea87c2bf4475c4b100f3 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 1 Jun 2018 16:15:46 +1000 Subject: [PATCH 5/5] Changes Content.Names to Content.CultureNames and Content.PublishNames to Content.PublishCultureNames --- src/Umbraco.Core/Models/Content.cs | 10 ++++----- src/Umbraco.Core/Models/ContentBase.cs | 4 ++-- src/Umbraco.Core/Models/IContent.cs | 2 +- src/Umbraco.Core/Models/IContentBase.cs | 2 +- .../Implement/DocumentRepository.cs | 22 +++++++++---------- src/Umbraco.Tests/Models/VariationTests.cs | 10 ++++----- .../Repositories/ContentRepositoryTest.cs | 2 +- .../Models/Mapping/MemberMapperProfile.cs | 2 +- .../NuCache/PublishedSnapshotService.cs | 6 ++--- src/Umbraco.Web/umbraco.presentation/page.cs | 2 +- 10 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 343310dd4e..2861ad4b26 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -232,7 +232,7 @@ namespace Umbraco.Core.Models /// [IgnoreDataMember] - public IReadOnlyDictionary PublishNames => _publishInfos?.ToDictionary(x => x.Key, x => x.Value.Name, StringComparer.OrdinalIgnoreCase) ?? NoNames; + public IReadOnlyDictionary PublishCultureNames => _publishInfos?.ToDictionary(x => x.Key, x => x.Value.Name, StringComparer.OrdinalIgnoreCase) ?? NoNames; /// public string GetPublishName(string culture) @@ -300,10 +300,10 @@ namespace Umbraco.Core.Models } /// - public IEnumerable EditedCultures => Names.Keys.Where(IsCultureEdited); + public IEnumerable EditedCultures => CultureNames.Keys.Where(IsCultureEdited); /// - public IEnumerable AvailableCultures => Names.Keys; + public IEnumerable AvailableCultures => CultureNames.Keys; [IgnoreDataMember] public int PublishedVersionId { get; internal set; } @@ -324,7 +324,7 @@ namespace Umbraco.Core.Models throw new InvalidOperationException($"Cannot publish invariant culture without a name."); PublishName = Name; var now = DateTime.Now; - foreach (var (culture, name) in Names) + foreach (var (culture, name) in CultureNames) { if (string.IsNullOrWhiteSpace(name)) return false; //fixme this should return an attempt with error results @@ -479,7 +479,7 @@ namespace Umbraco.Core.Models // copy names ClearNames(); - foreach (var (culture, name) in other.Names) + foreach (var (culture, name) in other.CultureNames) SetName(name, culture); Name = other.Name; } diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index 1bebdd39c5..d10b2e770d 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -69,7 +69,7 @@ namespace Umbraco.Core.Models public readonly PropertyInfo DefaultContentTypeIdSelector = ExpressionHelper.GetPropertyInfo(x => x.ContentTypeId); public readonly PropertyInfo PropertyCollectionSelector = ExpressionHelper.GetPropertyInfo(x => x.Properties); public readonly PropertyInfo WriterSelector = ExpressionHelper.GetPropertyInfo(x => x.WriterId); - public readonly PropertyInfo NamesSelector = ExpressionHelper.GetPropertyInfo>(x => x.Names); + public readonly PropertyInfo NamesSelector = ExpressionHelper.GetPropertyInfo>(x => x.CultureNames); } protected void PropertiesChanged(object sender, NotifyCollectionChangedEventArgs e) @@ -141,7 +141,7 @@ namespace Umbraco.Core.Models /// [DataMember] - public virtual IReadOnlyDictionary Names => _cultureInfos?.ToDictionary(x => x.Key, x => x.Value.Name, StringComparer.OrdinalIgnoreCase) ?? NoNames; + public virtual IReadOnlyDictionary CultureNames => _cultureInfos?.ToDictionary(x => x.Key, x => x.Value.Name, StringComparer.OrdinalIgnoreCase) ?? NoNames; // sets culture infos // internal for repositories diff --git a/src/Umbraco.Core/Models/IContent.cs b/src/Umbraco.Core/Models/IContent.cs index 51cc3e42ff..7bf52ea56b 100644 --- a/src/Umbraco.Core/Models/IContent.cs +++ b/src/Umbraco.Core/Models/IContent.cs @@ -121,7 +121,7 @@ namespace Umbraco.Core.Models /// Because a dictionary key cannot be null this cannot get the invariant /// name, which must be get via the property. /// - IReadOnlyDictionary PublishNames { get; } + IReadOnlyDictionary PublishCultureNames { get; } /// /// Gets the available cultures. diff --git a/src/Umbraco.Core/Models/IContentBase.cs b/src/Umbraco.Core/Models/IContentBase.cs index e127b42fe9..527b4455ce 100644 --- a/src/Umbraco.Core/Models/IContentBase.cs +++ b/src/Umbraco.Core/Models/IContentBase.cs @@ -53,7 +53,7 @@ namespace Umbraco.Core.Models /// Because a dictionary key cannot be null this cannot get the invariant /// name, which must be get or set via the property. /// - IReadOnlyDictionary Names { get; } + IReadOnlyDictionary CultureNames { get; } /// /// Gets a value indicating whether a given culture is available. diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 1fec78954b..905e2dc910 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -343,7 +343,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (content.ContentType.Variations.DoesSupportCulture()) { // names also impact 'edited' - foreach (var (culture, name) in content.Names) + foreach (var (culture, name) in content.CultureNames) if (name != content.GetPublishName(culture)) (editedCultures ?? (editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase))).Add(culture); @@ -498,7 +498,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (content.ContentType.Variations.DoesSupportCulture()) { // names also impact 'edited' - foreach (var (culture, name) in content.Names) + foreach (var (culture, name) in content.CultureNames) if (name != content.GetPublishName(culture)) (editedCultures ?? (editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase))).Add(culture); @@ -1031,7 +1031,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement private IEnumerable GetContentVariationDtos(IContent content, bool publishing) { // create dtos for the 'current' (non-published) version, all cultures - foreach (var (culture, name) in content.Names) + foreach (var (culture, name) in content.CultureNames) yield return new ContentVersionCultureVariationDto { VersionId = content.VersionId, @@ -1046,7 +1046,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (!publishing) yield break; // create dtos for the 'published' version, for published cultures (those having a name) - foreach (var (culture, name) in content.PublishNames) + foreach (var (culture, name) in content.PublishCultureNames) yield return new ContentVersionCultureVariationDto { VersionId = content.PublishedVersionId, @@ -1059,7 +1059,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement private IEnumerable GetDocumentVariationDtos(IContent content, bool publishing, HashSet editedCultures) { - foreach (var (culture, name) in content.Names) + foreach (var (culture, name) in content.CultureNames) yield return new DocumentCultureVariationDto { NodeId = content.Id, @@ -1097,7 +1097,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (content.ContentType.Variations.DoesSupportCulture()) { // no variant name = error - if (content.Names.Count == 0) + if (content.CultureNames.Count == 0) throw new InvalidOperationException("Cannot save content with an empty name."); // sync the invariant name to the default culture name if it's empty since we can't save with an empty invariant name. @@ -1105,10 +1105,10 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (string.IsNullOrWhiteSpace(content.Name)) { var defaultCulture = LanguageRepository.GetDefaultIsoCode(); - if (defaultCulture != null && content.Names.TryGetValue(defaultCulture, out var cultureName)) + if (defaultCulture != null && content.CultureNames.TryGetValue(defaultCulture, out var cultureName)) content.Name = cultureName; else - content.Name = content.Names.First().Value; // only option is to take the first + content.Name = content.CultureNames.First().Value; // only option is to take the first } } @@ -1116,7 +1116,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (publishing && string.IsNullOrWhiteSpace(content.PublishName)) { // no variant name = error - if (content.PublishNames.Count == 0) + if (content.PublishCultureNames.Count == 0) throw new InvalidOperationException("Cannot publish content with an empty name."); // else... we cannot deal with it here because PublishName is readonly, so in reality, PublishName @@ -1132,7 +1132,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement private void EnsureUniqueVariationNames(IContent content) { - if (!EnsureUniqueNaming || content.Names.Count == 0) return; + if (!EnsureUniqueNaming || content.CultureNames.Count == 0) return; //Get all culture names at the same level @@ -1156,7 +1156,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (names.Count == 0) return; - foreach(var n in content.Names) + foreach(var n in content.CultureNames) { var langId = LanguageRepository.GetIdByIsoCode(n.Key); if (!langId.HasValue) continue; diff --git a/src/Umbraco.Tests/Models/VariationTests.cs b/src/Umbraco.Tests/Models/VariationTests.cs index 9f124b65fc..044f8fa0cd 100644 --- a/src/Umbraco.Tests/Models/VariationTests.cs +++ b/src/Umbraco.Tests/Models/VariationTests.cs @@ -184,11 +184,11 @@ namespace Umbraco.Tests.Models Assert.AreEqual("name-uk", content.GetName(langUk)); // variant dictionary of names work - Assert.AreEqual(2, content.Names.Count); - Assert.IsTrue(content.Names.ContainsKey(langFr)); - Assert.AreEqual("name-fr", content.Names[langFr]); - Assert.IsTrue(content.Names.ContainsKey(langUk)); - Assert.AreEqual("name-uk", content.Names[langUk]); + Assert.AreEqual(2, content.CultureNames.Count); + Assert.IsTrue(content.CultureNames.ContainsKey(langFr)); + Assert.AreEqual("name-fr", content.CultureNames[langFr]); + Assert.IsTrue(content.CultureNames.ContainsKey(langUk)); + Assert.AreEqual("name-uk", content.CultureNames[langUk]); } [Test] diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs index 6285e3cd25..884c4f94d9 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs @@ -752,7 +752,7 @@ namespace Umbraco.Tests.Persistence.Repositories foreach (var r in result) { var isInvariant = r.ContentType.Alias == "umbInvariantTextpage"; - var name = isInvariant ? r.Name : r.Names["en-US"]; + var name = isInvariant ? r.Name : r.CultureNames["en-US"]; var namePrefix = isInvariant ? "INV" : "VAR"; //ensure the correct name (invariant vs variant) is in the result diff --git a/src/Umbraco.Web/Models/Mapping/MemberMapperProfile.cs b/src/Umbraco.Web/Models/Mapping/MemberMapperProfile.cs index 621fa0b95f..d0ab4099a9 100644 --- a/src/Umbraco.Web/Models/Mapping/MemberMapperProfile.cs +++ b/src/Umbraco.Web/Models/Mapping/MemberMapperProfile.cs @@ -45,7 +45,7 @@ namespace Umbraco.Web.Models.Mapping .ForMember(dest => dest.CreatorId, opt => opt.Ignore()) .ForMember(dest => dest.Level, opt => opt.Ignore()) .ForMember(dest => dest.Name, opt => opt.Ignore()) - .ForMember(dest => dest.Names, opt => opt.Ignore()) + .ForMember(dest => dest.CultureNames, opt => opt.Ignore()) .ForMember(dest => dest.ParentId, opt => opt.Ignore()) .ForMember(dest => dest.Path, opt => opt.Ignore()) .ForMember(dest => dest.SortOrder, opt => opt.Ignore()) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index 062a696ffc..0af0f3e6cc 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -1188,9 +1188,9 @@ namespace Umbraco.Web.PublishedCache.NuCache var names = content is IContent document ? (published - ? document.PublishNames - : document.Names) - : content.Names; + ? document.PublishCultureNames + : document.CultureNames) + : content.CultureNames; foreach (var (culture, name) in names) { diff --git a/src/Umbraco.Web/umbraco.presentation/page.cs b/src/Umbraco.Web/umbraco.presentation/page.cs index 2a2f125ece..4730d1b501 100644 --- a/src/Umbraco.Web/umbraco.presentation/page.cs +++ b/src/Umbraco.Web/umbraco.presentation/page.cs @@ -484,7 +484,7 @@ namespace umbraco if (_cultureInfos != null) return _cultureInfos; - return _cultureInfos = _inner.PublishNames + return _cultureInfos = _inner.PublishCultureNames .ToDictionary(x => x.Key, x => new PublishedCultureInfo(x.Key, x.Value, _inner.GetCulturePublishDate(x.Key))); } }