From 39f9449dc76d21bcd981117dc263bc12656b0c97 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 1 May 2018 13:30:25 +1000 Subject: [PATCH] Adds overloads for content ctors to pass in culture so we can more easily set the name in the API nicely, adds a unit test for paging queries with variant data. --- src/Umbraco.Core/Models/Content.cs | 16 ++-- src/Umbraco.Core/Models/ContentBase.cs | 15 ++-- .../Repositories/ContentRepositoryTest.cs | 77 +++++++++++++++++++ .../TestHelpers/Entities/MockedContent.cs | 7 +- .../Testing/ContentBaseExtensions.cs | 6 +- 5 files changed, 101 insertions(+), 20 deletions(-) diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 8bfb5718a2..57ad512616 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -31,8 +31,8 @@ namespace Umbraco.Core.Models /// Name of the content /// Parent object /// ContentType for the current Content object - public Content(string name, IContent parent, IContentType contentType) - : this(name, parent, contentType, new PropertyCollection()) + public Content(string name, IContent parent, IContentType contentType, string culture = null) + : this(name, parent, contentType, new PropertyCollection(), culture) { } /// @@ -42,8 +42,8 @@ namespace Umbraco.Core.Models /// Parent object /// ContentType for the current Content object /// Collection of properties - public Content(string name, IContent parent, IContentType contentType, PropertyCollection properties) - : base(name, parent, contentType, properties) + public Content(string name, IContent parent, IContentType contentType, PropertyCollection properties, string culture = null) + : base(name, parent, contentType, properties, culture) { _contentType = contentType ?? throw new ArgumentNullException(nameof(contentType)); _publishedState = PublishedState.Unpublished; @@ -56,8 +56,8 @@ namespace Umbraco.Core.Models /// Name of the content /// Id of the Parent content /// ContentType for the current Content object - public Content(string name, int parentId, IContentType contentType) - : this(name, parentId, contentType, new PropertyCollection()) + public Content(string name, int parentId, IContentType contentType, string culture = null) + : this(name, parentId, contentType, new PropertyCollection(), culture) { } /// @@ -67,8 +67,8 @@ namespace Umbraco.Core.Models /// Id of the Parent content /// ContentType for the current Content object /// Collection of properties - public Content(string name, int parentId, IContentType contentType, PropertyCollection properties) - : base(name, parentId, contentType, properties) + public Content(string name, int parentId, IContentType contentType, PropertyCollection properties, string culture = null) + : base(name, parentId, contentType, properties, culture) { _contentType = contentType ?? throw new ArgumentNullException(nameof(contentType)); _publishedState = PublishedState.Unpublished; diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index 7bc327c2d0..f0cb8c0574 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -30,8 +30,8 @@ namespace Umbraco.Core.Models /// /// Initializes a new instance of the class. /// - protected ContentBase(string name, int parentId, IContentTypeComposition contentType, PropertyCollection properties) - : this(name, contentType, properties) + protected ContentBase(string name, int parentId, IContentTypeComposition contentType, PropertyCollection properties, string culture = null) + : this(name, contentType, properties, culture) { if (parentId == 0) throw new ArgumentOutOfRangeException(nameof(parentId)); ParentId = parentId; @@ -40,22 +40,25 @@ namespace Umbraco.Core.Models /// /// Initializes a new instance of the class. /// - protected ContentBase(string name, IContentBase parent, IContentTypeComposition contentType, PropertyCollection properties) - : this(name, contentType, properties) + protected ContentBase(string name, IContentBase parent, IContentTypeComposition contentType, PropertyCollection properties, string culture = null) + : this(name, contentType, properties, culture) { if (parent == null) throw new ArgumentNullException(nameof(parent)); SetParent(parent); } - private ContentBase(string name, IContentTypeComposition contentType, PropertyCollection properties) + private ContentBase(string name, IContentTypeComposition contentType, PropertyCollection properties, string culture = null) { ContentTypeBase = contentType ?? throw new ArgumentNullException(nameof(contentType)); // initially, all new instances have Id = 0; // no identity VersionId = 0; // no versions + + //fixme we always need to set the invariant name else an exception will throw if we try to persist + Name = name; + SetName(culture, name); - Name = name; _contentTypeId = contentType.Id; _properties = properties ?? throw new ArgumentNullException(nameof(properties)); _properties.EnsurePropertyTypes(PropertyTypes); diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs index caa2453aed..552eea81da 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs @@ -679,6 +679,83 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.AreEqual("[table].[column2]", matches[1].Groups[1].Value); Assert.AreEqual("[alias2]", matches[1].Groups[2].Value); } + + [Test] + public void GetPagedResultsByQuery_With_Variant_Names() + { + //2x content types, one invariant, one variant + + var invariantCT = MockedContentTypes.CreateSimpleContentType("umbInvariantTextpage", "Invariant Textpage"); + invariantCT.Variations = ContentVariation.InvariantNeutral; + foreach (var p in invariantCT.PropertyTypes) p.Variations = ContentVariation.InvariantNeutral; + ServiceContext.FileService.SaveTemplate(invariantCT.DefaultTemplate); // else, FK violation on contentType! + ServiceContext.ContentTypeService.Save(invariantCT); + + var variantCT = MockedContentTypes.CreateSimpleContentType("umbVariantTextpage", "Variant Textpage"); + variantCT.Variations = ContentVariation.CultureNeutral; + foreach (var p in variantCT.PropertyTypes) p.Variations = ContentVariation.CultureNeutral; + ServiceContext.FileService.SaveTemplate(variantCT.DefaultTemplate); // else, FK violation on contentType! + ServiceContext.ContentTypeService.Save(variantCT); + + invariantCT.AllowedContentTypes = new[] { new ContentTypeSort(invariantCT.Id, 0), new ContentTypeSort(variantCT.Id, 1) }; + ServiceContext.ContentTypeService.Save(invariantCT); + + //create content + + var root = MockedContent.CreateSimpleContent(invariantCT); + ServiceContext.ContentService.Save(root); + + for (int i = 0; i < 25; i++) + { + var isInvariant = i % 2 == 0; + var child = MockedContent.CreateSimpleContent( + isInvariant ? invariantCT : variantCT, + (isInvariant ? "INV" : "VAR") + "_" + Guid.NewGuid().ToString(), + root, + culture: isInvariant ? null : "en-US"); + + ServiceContext.ContentService.Save(child); + } + + var provider = TestObjects.GetScopeProvider(Logger); + using (var scope = provider.CreateScope()) + { + var repository = CreateRepository((IScopeAccessor)provider, out _); + + var query = scope.SqlContext.Query().Where(x => x.ParentId == root.Id); + + try + { + scope.Database.AsUmbracoDatabase().EnableSqlTrace = true; + scope.Database.AsUmbracoDatabase().EnableSqlCount = true; + + var result = repository.GetPage(query, 0, 20, out var totalRecords, "UpdateDate", Direction.Ascending, true); + + Assert.AreEqual(25, totalRecords); + foreach(var r in result) + { + var isInvariant = r.ContentType.Alias == "umbInvariantTextpage"; + var name = isInvariant ? r.Name : r.Names["en-US"]; + var namePrefix = (isInvariant ? "INV" : "VAR"); + + //ensure the correct name (invariant vs variant) is in the result + Assert.IsTrue(name.StartsWith(namePrefix)); + + foreach (var p in r.Properties) + { + //ensure there is a value for the correct variant/invariant property + var value = p.GetValue(isInvariant ? null : "en-US"); + Assert.IsNotNull(value); + } + } + } + finally + { + scope.Database.AsUmbracoDatabase().EnableSqlTrace = false; + scope.Database.AsUmbracoDatabase().EnableSqlCount = false; + } + } + } [Test] public void GetPagedResultsByQuery_CustomPropertySort() diff --git a/src/Umbraco.Tests/TestHelpers/Entities/MockedContent.cs b/src/Umbraco.Tests/TestHelpers/Entities/MockedContent.cs index 5e69eff83a..e1c56d8d54 100644 --- a/src/Umbraco.Tests/TestHelpers/Entities/MockedContent.cs +++ b/src/Umbraco.Tests/TestHelpers/Entities/MockedContent.cs @@ -53,9 +53,10 @@ namespace Umbraco.Tests.TestHelpers.Entities return content; } - public static Content CreateSimpleContent(IContentType contentType, string name, IContent parent) + public static Content CreateSimpleContent(IContentType contentType, string name, IContent parent, string culture = null, string segment = null) { - var content = new Content(name, parent, contentType) { CreatorId = 0, WriterId = 0 }; + var content = new Content(name, parent, contentType, culture) { CreatorId = 0, WriterId = 0 }; + object obj = new { @@ -64,7 +65,7 @@ namespace Umbraco.Tests.TestHelpers.Entities author = "John Doe" }; - content.PropertyValues(obj); + content.PropertyValues(obj, culture, segment); content.ResetDirtyProperties(false); diff --git a/src/Umbraco.Tests/Testing/ContentBaseExtensions.cs b/src/Umbraco.Tests/Testing/ContentBaseExtensions.cs index 1a94b8770a..58d4dfbd7f 100644 --- a/src/Umbraco.Tests/Testing/ContentBaseExtensions.cs +++ b/src/Umbraco.Tests/Testing/ContentBaseExtensions.cs @@ -10,7 +10,7 @@ namespace Umbraco.Tests.Testing /// Set property values by alias with an annonymous object. /// /// Does not support variants. - public static void PropertyValues(this IContentBase content, object value) + public static void PropertyValues(this IContentBase content, object value, string culture = null, string segment = null) { if (value == null) throw new Exception("No properties has been passed in"); @@ -27,7 +27,7 @@ namespace Umbraco.Tests.Testing var item = content.Properties.FirstOrDefault(x => x.Alias == propertyInfo.Name); if (item != null) { - item.SetValue(propertyInfo.GetValue(value, null)); + item.SetValue(propertyInfo.GetValue(value, null), culture, segment); //Update item with newly added value content.Properties.Add(item); } @@ -35,7 +35,7 @@ namespace Umbraco.Tests.Testing { //Create new Property to add to collection var property = propertyType.CreateProperty(); - property.SetValue(propertyInfo.GetValue(value, null)); + property.SetValue(propertyInfo.GetValue(value, null), culture, segment); content.Properties.Add(property); } }