From ad5a18f1eea8daa6929b630e6041eefb4041e507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brynjar=20=C3=9Eorsteinsson?= <85184333+Brynjarth@users.noreply.github.com> Date: Tue, 1 Jul 2025 06:11:00 +0000 Subject: [PATCH] Fix pagination in Content Delivery API Index Helper (#19606) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Refactor descendant enumeration in DeliveryApiContentIndexHelper Improved loop condition to allow for processing of more than 10.000 descendants for indexing. * Add failing test for original issue. * Renamed variable for clarity. --------- Co-authored-by: Brynjar Þorsteinsson Co-authored-by: Andy Butland --- .../Examine/DeliveryApiContentIndexHelper.cs | 21 ++- .../DeliveryApiContentIndexHelperTests.cs | 120 ++++++++++++++++++ 2 files changed, 134 insertions(+), 7 deletions(-) create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexHelperTests.cs diff --git a/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexHelper.cs b/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexHelper.cs index 744502b3de..d7f2fc0c69 100644 --- a/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexHelper.cs +++ b/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexHelper.cs @@ -1,4 +1,4 @@ -using Microsoft.Extensions.Options; +using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Persistence.Querying; @@ -28,21 +28,28 @@ internal sealed class DeliveryApiContentIndexHelper : IDeliveryApiContentIndexHe public void EnumerateApplicableDescendantsForContentIndex(int rootContentId, Action actionToPerform) { const int pageSize = 10000; - var pageIndex = 0; + EnumerateApplicableDescendantsForContentIndex(rootContentId, actionToPerform, pageSize); + } + + internal void EnumerateApplicableDescendantsForContentIndex(int rootContentId, Action actionToPerform, int pageSize) + { + var itemIndex = 0; + long total; + + IQuery query = _umbracoDatabaseFactory.SqlContext.Query().Where(content => content.Trashed == false); IContent[] descendants; - IQuery query = _umbracoDatabaseFactory.SqlContext.Query().Where(content => content.Trashed == false); do { descendants = _contentService - .GetPagedDescendants(rootContentId, pageIndex, pageSize, out _, query, Ordering.By("Path")) + .GetPagedDescendants(rootContentId, itemIndex / pageSize, pageSize, out total, query, Ordering.By("Path")) .Where(descendant => _deliveryApiSettings.IsAllowedContentType(descendant.ContentType.Alias)) .ToArray(); - actionToPerform(descendants.ToArray()); + actionToPerform(descendants); - pageIndex++; + itemIndex += pageSize; } - while (descendants.Length == pageSize); + while (descendants.Length > 0 && itemIndex < total); } } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexHelperTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexHelperTests.cs new file mode 100644 index 0000000000..dc16d1ae16 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexHelperTests.cs @@ -0,0 +1,120 @@ +using Microsoft.Extensions.Options; +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Infrastructure.Examine; +using Umbraco.Cms.Infrastructure.Persistence; +using Umbraco.Cms.Tests.Common.Builders; +using Umbraco.Cms.Tests.Common.Testing; +using Umbraco.Cms.Tests.Integration.Testing; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Examine; + +[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] +[TestFixture] +public class DeliveryApiContentIndexHelperTests : UmbracoIntegrationTestWithContent +{ + public override void CreateTestData() + { + base.CreateTestData(); + + // Save an extra, published content item of a different type to those created via the base class, + // that we'll use to test filtering out disallowed content types. + var template = TemplateBuilder.CreateTextPageTemplate("textPage2"); + FileService.SaveTemplate(template); + + var contentType = ContentTypeBuilder.CreateSimpleContentType("umbTextpage2", "Textpage2", defaultTemplateId: template.Id); + contentType.Key = Guid.NewGuid(); + ContentTypeService.Save(contentType); + + ContentType.AllowedContentTypes = + [ + new ContentTypeSort(ContentType.Key, 0, "umbTextpage"), + new ContentTypeSort(contentType.Key, 1, "umbTextpage2"), + ]; + ContentTypeService.Save(ContentType); + + var subpage = ContentBuilder.CreateSimpleContent(contentType, "Alternate Text Page 4", Textpage.Id); + ContentService.Save(subpage); + + // And then add some more of the first type, so the one we'll filter out in tests isn't in the last page. + for (int i = 0; i < 5; i++) + { + subpage = ContentBuilder.CreateSimpleContent(ContentType, $"Text Page {5 + i}", Textpage.Id); + ContentService.Save(subpage); + } + } + + [Test] + public void Can_Enumerate_Descendants_For_Content_Index() + { + var sut = CreateDeliveryApiContentIndexHelper(); + + var expectedNumberOfContentItems = GetExpectedNumberOfContentItems(); + + var contentEnumerated = 0; + Action actionToPerform = content => + { + contentEnumerated += content.Length; + }; + + const int pageSize = 3; + sut.EnumerateApplicableDescendantsForContentIndex( + Cms.Core.Constants.System.Root, + actionToPerform, + pageSize); + + Assert.AreEqual(expectedNumberOfContentItems, contentEnumerated); + } + + [Test] + public void Can_Enumerate_Descendants_For_Content_Index_With_Disallowed_Content_Type() + { + var sut = CreateDeliveryApiContentIndexHelper(["umbTextPage2"]); + + var expectedNumberOfContentItems = GetExpectedNumberOfContentItems(); + + var contentEnumerated = 0; + Action actionToPerform = content => + { + contentEnumerated += content.Length; + }; + + const int pageSize = 3; + sut.EnumerateApplicableDescendantsForContentIndex( + Cms.Core.Constants.System.Root, + actionToPerform, + pageSize); + + Assert.AreEqual(expectedNumberOfContentItems - 1, contentEnumerated); + } + + private DeliveryApiContentIndexHelper CreateDeliveryApiContentIndexHelper(HashSet? disallowedContentTypeAliases = null) + { + return new DeliveryApiContentIndexHelper( + ContentService, + GetRequiredService(), + GetDeliveryApiSettings(disallowedContentTypeAliases ?? [])); + } + + private IOptionsMonitor GetDeliveryApiSettings(HashSet disallowedContentTypeAliases) + { + var deliveryApiSettings = new DeliveryApiSettings + { + DisallowedContentTypeAliases = disallowedContentTypeAliases, + }; + + var optionsMonitorMock = new Mock>(); + optionsMonitorMock.Setup(o => o.CurrentValue).Returns(deliveryApiSettings); + return optionsMonitorMock.Object; + } + + private int GetExpectedNumberOfContentItems() + { + var result = ContentService.GetAllPublished().Count(); + Assert.AreEqual(10, result); + return result; + } +}