From f0c76279a290902d23f5b04eb5392a8e57d0aea3 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Thu, 29 Aug 2024 15:39:01 -0700 Subject: [PATCH 1/8] Batch the process of building sitemaps --- .../LocalizedContentItemsQueryProvider.cs | 114 ++++++++++-------- ...emapUrlHrefLangExtendedMetadataProvider.cs | 8 +- .../ContentTypesSitemapSourceBuilder.cs | 77 ++++++++++-- .../DefaultContentItemsQueryProvider.cs | 48 ++++---- .../Builders/ContentItemsQueryContext.cs | 12 +- .../Builders/ContentItemsQueryResult.cs | 16 +++ .../Builders/IContentItemsQueryProvider.cs | 2 +- ...emapContentItemExtendedMetadataProvider.cs | 2 +- src/docs/releases/2.0.0.md | 15 ++- 9 files changed, 190 insertions(+), 104 deletions(-) create mode 100644 src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryResult.cs diff --git a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs index ed8b8736efc..51c1da49b6e 100644 --- a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs @@ -28,7 +28,7 @@ ILocalizationService localizationService _localizationService = localizationService; } - public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext queryContext) + public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context) { var routeableContentTypeDefinitions = await _routeableContentTypeCoordinator.ListRoutableTypeDefinitionsAsync(); @@ -37,18 +37,24 @@ public async Task GetContentItemsAsync(ContentTypesSitemapSource source, Content // Assumption here is that at least one content type will be localized. var ctdNames = routeableContentTypeDefinitions.Select(ctd => ctd.Name); - var queryResults = await _session.Query() + var results = await _session.Query() .With(x => x.Published && x.ContentType.IsIn(ctdNames)) .OrderBy(x => x.CreatedUtc) + .ThenBy(x => x.Id) + .Skip(context.Skip) + .Take(context.Take) .ListAsync(); - queryContext.ContentItems = queryResults; + return new ContentItemsQueryResult + { + ContentItems = results, - // Provide all content items with localization as reference content items. - queryContext.ReferenceContentItems = queryResults - .Where(ci => ci.Has()); + // Provide all content items with localization as reference content items. + ReferenceContentItems = results.Where(ci => ci.Has()), + }; } - else if (source.LimitItems) + + if (source.LimitItems) { // Test that content type is still valid to include in sitemap. var contentType = routeableContentTypeDefinitions @@ -56,66 +62,70 @@ public async Task GetContentItemsAsync(ContentTypesSitemapSource source, Content if (contentType == null) { - return; + return new ContentItemsQueryResult + { + ContentItems = [], + ReferenceContentItems = [], + }; } if (contentType.Parts.Any(ctd => string.Equals(ctd.Name, nameof(LocalizationPart), StringComparison.Ordinal))) { + // When limiting items Content item is valid if it is for the default culture. + var defaultCulture = await _localizationService.GetDefaultCultureAsync(); + // Get all content items here for reference. Then reduce by default culture. // We know that the content item should be localized. // If it doesn't have a localization part, the content item should have been saved. - var queryResults = await _session.Query() + var contentItems = await _session.Query() .With(ci => ci.ContentType == source.LimitedContentType.ContentTypeName && ci.Published) .OrderBy(ci => ci.CreatedUtc) - .With() + .ThenBy(ci => ci.Id) + .With(x => x.Culture == defaultCulture) + .Take(context.Take) + .Skip(context.Skip) .ListAsync(); - // When limiting items Content item is valid if it is for the default culture. - var defaultCulture = await _localizationService.GetDefaultCultureAsync(); - - // Reduce by default culture. - var items = queryResults - .Where(ci => string.Equals(ci.As().Culture, defaultCulture, StringComparison.Ordinal)) - .Skip(source.LimitedContentType.Skip) - .Take(source.LimitedContentType.Take); - - queryContext.ContentItems = items; - - // Provide all content items with localization as reference content items. - queryContext.ReferenceContentItems = queryResults - .Where(ci => ci.Has()); - } - else - { - // Content type is not localized. Produce standard results. - var queryResults = await _session.Query() - .With(x => x.ContentType == source.LimitedContentType.ContentTypeName && x.Published) - .OrderBy(x => x.CreatedUtc) - .Skip(source.LimitedContentType.Skip) - .Take(source.LimitedContentType.Take) - .ListAsync(); - - queryContext.ContentItems = queryResults; + return new ContentItemsQueryResult + { + ContentItems = contentItems, + ReferenceContentItems = contentItems.Where(ci => ci.Has()), + }; } - } - else - { - // Test that content types are still valid to include in sitemap. - var typesToIndex = routeableContentTypeDefinitions - .Where(ctd => source.ContentTypes.Any(s => string.Equals(ctd.Name, s.ContentTypeName, StringComparison.Ordinal))) - .Select(x => x.Name); - - // No advantage here in reducing with localized index. - var queryResults = await _session.Query() - .With(x => x.ContentType.IsIn(typesToIndex) && x.Published) + + // Content type is not localized. Produce standard results. + var items = await _session.Query() + .With(x => x.ContentType == source.LimitedContentType.ContentTypeName && x.Published) .OrderBy(x => x.CreatedUtc) + .Skip(context.Skip) + .Take(context.Take) .ListAsync(); - queryContext.ContentItems = queryResults; - - // Provide all content items with localization as reference content items. - queryContext.ReferenceContentItems = queryResults - .Where(ci => ci.Has()); + return new ContentItemsQueryResult + { + ContentItems = items, + ReferenceContentItems = [], + }; } + + // Test that content types are still valid to include in sitemap. + var typesToIndex = routeableContentTypeDefinitions + .Where(ctd => source.ContentTypes.Any(s => string.Equals(ctd.Name, s.ContentTypeName, StringComparison.Ordinal))) + .Select(x => x.Name); + + // No advantage here in reducing with localized index. + var queryResults = await _session.Query() + .With(x => x.ContentType.IsIn(typesToIndex) && x.Published) + .OrderBy(x => x.CreatedUtc) + .ThenBy(x => x.Id) + .Take(context.Take) + .Skip(context.Skip) + .ListAsync(); + + return new ContentItemsQueryResult + { + ContentItems = queryResults, + ReferenceContentItems = queryResults.Where(ci => ci.Has()), + }; } } diff --git a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/SitemapUrlHrefLangExtendedMetadataProvider.cs b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/SitemapUrlHrefLangExtendedMetadataProvider.cs index cc0d943f6db..785d81e8c13 100644 --- a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/SitemapUrlHrefLangExtendedMetadataProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/SitemapUrlHrefLangExtendedMetadataProvider.cs @@ -28,17 +28,19 @@ IRouteableContentTypeCoordinator routeableContentTypeCoordinator public async Task ApplyExtendedMetadataAsync( SitemapBuilderContext context, - ContentItemsQueryContext queryContext, + ContentItemsQueryResult queryResult, ContentItem contentItem, XElement url) { var part = contentItem.As(); - if (part == null) + if (part == null || + queryResult.ReferenceContentItems == null || + !queryResult.ReferenceContentItems.Any()) { return true; } - var localizedContentParts = queryContext.ReferenceContentItems + var localizedContentParts = queryResult.ReferenceContentItems .Select(ci => ci.As()) .Where(cp => cp.LocalizationSet == part.LocalizationSet); diff --git a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs index 0cee33f36ea..d8ee77b500f 100644 --- a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs +++ b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs @@ -10,6 +10,8 @@ namespace OrchardCore.Contents.Sitemaps; public class ContentTypesSitemapSourceBuilder : SitemapSourceBuilderBase { + private const int _batchSize = 500; + private static readonly XNamespace _namespace = "http://www.sitemaps.org/schemas/sitemap/0.9"; private readonly IRouteableContentTypeCoordinator _routeableContentTypeCoordinator; @@ -21,8 +23,7 @@ public ContentTypesSitemapSourceBuilder( IRouteableContentTypeCoordinator routeableContentTypeCoordinator, IContentManager contentManager, IContentItemsQueryProvider contentItemsQueryProvider, - IEnumerable sitemapContentItemExtendedMetadataProviders - ) + IEnumerable sitemapContentItemExtendedMetadataProviders) { _routeableContentTypeCoordinator = routeableContentTypeCoordinator; _contentManager = contentManager; @@ -32,33 +33,83 @@ IEnumerable sitemapContentItemExten public override async Task BuildSourceAsync(ContentTypesSitemapSource source, SitemapBuilderContext context) { - var queryContext = new ContentItemsQueryContext(); - await _contentItemsQueryProvider.GetContentItemsAsync(source, queryContext); - foreach (var sciemp in _sitemapContentItemExtendedMetadataProviders) { context.Response.ResponseElement.Add(sciemp.GetExtendedAttribute); } - foreach (var contentItem in queryContext.ContentItems) + if (source.LimitedContentType != null && source.LimitedContentType.Take > 0) + { + var result = await _contentItemsQueryProvider.GetContentItemsAsync(source, new ContentItemsQueryContext + { + Skip = source.LimitedContentType.Skip, + Take = source.LimitedContentType.Take, + }); + + if (result.ContentItems == null || !result.ContentItems.Any()) + { + return; + } + + foreach (var contentItem in result.ContentItems) + { + var url = new XElement(_namespace + "url"); + + if (await BuildUrlsetMetadataAsync(source, context, result, contentItem, url)) + { + context.Response.ResponseElement.Add(url); + } + } + + return; + } + + var queryContext = new ContentItemsQueryContext() { - var url = new XElement(_namespace + "url"); + Take = _batchSize, + }; + + if (source.LimitedContentType != null) + { + queryContext.Skip = source.LimitedContentType.Skip; + + // Make sure the 'Take' value does not exceed the batch size. + queryContext.Take = Math.Min(source.LimitedContentType.Take, _batchSize); + } + + while (true) + { + var result = await _contentItemsQueryProvider.GetContentItemsAsync(source, queryContext); - if (await BuildUrlsetMetadataAsync(source, context, queryContext, contentItem, url)) + if (result.ContentItems == null || !result.ContentItems.Any()) { - context.Response.ResponseElement.Add(url); + break; } + + foreach (var contentItem in result.ContentItems) + { + var url = new XElement(_namespace + "url"); + + if (await BuildUrlsetMetadataAsync(source, context, result, contentItem, url)) + { + context.Response.ResponseElement.Add(url); + } + } + + queryContext.Skip += queryContext.Take; } } - private async Task BuildUrlsetMetadataAsync(ContentTypesSitemapSource source, SitemapBuilderContext context, ContentItemsQueryContext queryContext, ContentItem contentItem, XElement url) + private async Task BuildUrlsetMetadataAsync(ContentTypesSitemapSource source, SitemapBuilderContext context, ContentItemsQueryResult queryResult, ContentItem contentItem, XElement url) { if (await BuildUrlAsync(context, contentItem, url)) { - if (await BuildExtendedMetadataAsync(context, queryContext, contentItem, url)) + if (await BuildExtendedMetadataAsync(context, queryResult, contentItem, url)) { PopulateLastMod(contentItem, url); + await PopulateChangeFrequencyPriority(source, contentItem, url); + return true; } @@ -68,12 +119,12 @@ private async Task BuildUrlsetMetadataAsync(ContentTypesSitemapSource sour return false; } - private async Task BuildExtendedMetadataAsync(SitemapBuilderContext context, ContentItemsQueryContext queryContext, ContentItem contentItem, XElement url) + private async Task BuildExtendedMetadataAsync(SitemapBuilderContext context, ContentItemsQueryResult queryResult, ContentItem contentItem, XElement url) { var succeeded = true; foreach (var sc in _sitemapContentItemExtendedMetadataProviders) { - if (!await sc.ApplyExtendedMetadataAsync(context, queryContext, contentItem, url)) + if (!await sc.ApplyExtendedMetadataAsync(context, queryResult, contentItem, url)) { succeeded = false; } diff --git a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs index 9cf583441d3..0977a01ac45 100644 --- a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs @@ -22,20 +22,20 @@ IRouteableContentTypeCoordinator routeableContentTypeCoordinator _routeableContentTypeCoordinator = routeableContentTypeCoordinator; } - public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context) + public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context) { + ArgumentNullException.ThrowIfNull(source); + ArgumentNullException.ThrowIfNull(context); + var routeableContentTypeDefinitions = await _routeableContentTypeCoordinator.ListRoutableTypeDefinitionsAsync(); + var query = _session.Query(); + if (source.IndexAll) { var rctdNames = routeableContentTypeDefinitions.Select(rctd => rctd.Name); - var queryResults = await _session.Query() - .With(x => x.Published && x.ContentType.IsIn(rctdNames)) - .OrderBy(x => x.CreatedUtc) - .ListAsync(); - - context.ContentItems = queryResults; + query = query.Where(x => x.Published && x.ContentType.IsIn(rctdNames)); } else if (source.LimitItems) { @@ -43,17 +43,15 @@ public async Task GetContentItemsAsync(ContentTypesSitemapSource source, Content var typeIsValid = routeableContentTypeDefinitions .Any(ctd => string.Equals(source.LimitedContentType.ContentTypeName, ctd.Name, StringComparison.Ordinal)); - if (typeIsValid) + if (!typeIsValid) { - var queryResults = await _session.Query() - .With(x => x.ContentType == source.LimitedContentType.ContentTypeName && x.Published) - .OrderBy(x => x.CreatedUtc) - .Skip(source.LimitedContentType.Skip) - .Take(source.LimitedContentType.Take) - .ListAsync(); - - context.ContentItems = queryResults; + return new ContentItemsQueryResult() + { + ContentItems = [], + }; } + + query = query.Where(x => x.ContentType == source.LimitedContentType.ContentTypeName && x.Published); } else { @@ -62,12 +60,18 @@ public async Task GetContentItemsAsync(ContentTypesSitemapSource source, Content .Where(ctd => source.ContentTypes.Any(s => string.Equals(ctd.Name, s.ContentTypeName, StringComparison.Ordinal))) .Select(x => x.Name); - var queryResults = await _session.Query() - .With(x => x.ContentType.IsIn(typesToIndex) && x.Published) - .OrderBy(x => x.CreatedUtc) - .ListAsync(); - - context.ContentItems = queryResults; + query = query.Where(x => x.ContentType.IsIn(typesToIndex) && x.Published); } + + return new ContentItemsQueryResult + { + ContentItems = await query + .OrderBy(x => x.CreatedUtc) + .ThenBy(x => x.Id) + .Take(context.Take) + .Skip(context.Skip) + .ListAsync(), + ReferenceContentItems = [], + }; } } diff --git a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryContext.cs b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryContext.cs index 7674c53bf47..cbd34021ec4 100644 --- a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryContext.cs +++ b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryContext.cs @@ -1,16 +1,8 @@ -using OrchardCore.ContentManagement; - namespace OrchardCore.Sitemaps.Builders; public class ContentItemsQueryContext { - /// - /// Content items to include in sitemap. - /// - public IEnumerable ContentItems { get; set; } = []; + public int Skip { get; set; } - /// - /// Reference content items that may be used to perform a lookup for url alternatives. - /// - public IEnumerable ReferenceContentItems { get; set; } = []; + public int Take { get; set; } = 500; } diff --git a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryResult.cs b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryResult.cs new file mode 100644 index 00000000000..0f44b5894d8 --- /dev/null +++ b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryResult.cs @@ -0,0 +1,16 @@ +using OrchardCore.ContentManagement; + +namespace OrchardCore.Sitemaps.Builders; + +public class ContentItemsQueryResult +{ + /// + /// Content items to include in sitemap. + /// + public IEnumerable ContentItems { get; set; } + + /// + /// Reference content items that may be used to perform a lookup for url alternatives. + /// + public IEnumerable ReferenceContentItems { get; set; } +} diff --git a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs index b7a9f29774b..a8f3cf2945f 100644 --- a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs +++ b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs @@ -7,5 +7,5 @@ public interface IContentItemsQueryProvider /// /// Get content items to evaluate for inclusion in a sitemap. /// - Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context); + Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context); } diff --git a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ISitemapContentItemExtendedMetadataProvider.cs b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ISitemapContentItemExtendedMetadataProvider.cs index 2d8c6746fb4..3e75dce9660 100644 --- a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ISitemapContentItemExtendedMetadataProvider.cs +++ b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ISitemapContentItemExtendedMetadataProvider.cs @@ -16,5 +16,5 @@ public interface ISitemapContentItemExtendedMetadataProvider /// /// Apply extended metadata to the url element. /// - Task ApplyExtendedMetadataAsync(SitemapBuilderContext context, ContentItemsQueryContext queryContext, ContentItem contentItem, XElement url); + Task ApplyExtendedMetadataAsync(SitemapBuilderContext context, ContentItemsQueryResult queryResult, ContentItem contentItem, XElement url); } diff --git a/src/docs/releases/2.0.0.md b/src/docs/releases/2.0.0.md index 664bdea52af..c48feff7d7c 100644 --- a/src/docs/releases/2.0.0.md +++ b/src/docs/releases/2.0.0.md @@ -398,6 +398,19 @@ Previously, the `CreateContentTask`, `RetrieveContentTask`, and `UpdateContentTa Additionally, a new workflow-scoped script function `setCorrelationId(id:string): void` was added, that you can use to update the workflow's CorrelationId. +### Sitemaps + +To enhance sitemap performance, the `IContentItemsQueryProvider` interface was updated. The `GetContentItemsAsync` method was modified to the following signature: + +```csharp +Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context) +``` + +In the `ContentItemsQueryContext` class, the `ContentItems` and `ReferenceContentItems` properties were removed and replaced by `Skip` and `Take` properties to allow batching. The `ContentItemsQueryResult` class now holds the `ContentItems` and `ReferenceContentItems` properties. + +Additionally, the `ISitemapContentItemExtendedMetadataProvider` interface was updated to accept a `ContentItemsQueryResult` object as its second parameter, replacing the previous `ContentItemsQueryContext` parameter. + + ## Change Logs ### Azure AI Search Module @@ -437,8 +450,6 @@ Additionally, if an error occurs, a new custom exception, RecipeExecutionExcepti - `UserConfirmedEvent`: this event triggers when a user successfully confirms their email address after registration. - - ### GraphQL Module When identifying content types for GraphQL exposure, we identify those without a stereotype to provide you with control over the behavior of stereotyped content types. A new option, `DiscoverableSterotypes`, has been introduced in `GraphQLContentOptions`. This allows you to specify stereotypes that should be discoverable by default. From fb824a9dde86ad539616ef93cbaa4b2fbd4b317a Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Thu, 29 Aug 2024 16:05:54 -0700 Subject: [PATCH 2/8] fix limits --- .../ContentTypesSitemapSourceBuilder.cs | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs index d8ee77b500f..02892766e09 100644 --- a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs +++ b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs @@ -1,6 +1,8 @@ using System.Globalization; using System.Xml.Linq; +using Microsoft.Extensions.Options; using OrchardCore.ContentManagement; +using OrchardCore.Data.YesSql; using OrchardCore.Sitemaps.Aspects; using OrchardCore.Sitemaps.Builders; using OrchardCore.Sitemaps.Models; @@ -10,23 +12,24 @@ namespace OrchardCore.Contents.Sitemaps; public class ContentTypesSitemapSourceBuilder : SitemapSourceBuilderBase { - private const int _batchSize = 500; - private static readonly XNamespace _namespace = "http://www.sitemaps.org/schemas/sitemap/0.9"; private readonly IRouteableContentTypeCoordinator _routeableContentTypeCoordinator; private readonly IContentManager _contentManager; + private readonly YesSqlOptions _yesSqlOptions; private readonly IContentItemsQueryProvider _contentItemsQueryProvider; private readonly IEnumerable _sitemapContentItemExtendedMetadataProviders; public ContentTypesSitemapSourceBuilder( IRouteableContentTypeCoordinator routeableContentTypeCoordinator, IContentManager contentManager, + IOptions yesSqlOptions, IContentItemsQueryProvider contentItemsQueryProvider, IEnumerable sitemapContentItemExtendedMetadataProviders) { _routeableContentTypeCoordinator = routeableContentTypeCoordinator; _contentManager = contentManager; + _yesSqlOptions = yesSqlOptions.Value; _contentItemsQueryProvider = contentItemsQueryProvider; _sitemapContentItemExtendedMetadataProviders = sitemapContentItemExtendedMetadataProviders; } @@ -38,47 +41,35 @@ public override async Task BuildSourceAsync(ContentTypesSitemapSource source, Si context.Response.ResponseElement.Add(sciemp.GetExtendedAttribute); } - if (source.LimitedContentType != null && source.LimitedContentType.Take > 0) - { - var result = await _contentItemsQueryProvider.GetContentItemsAsync(source, new ContentItemsQueryContext - { - Skip = source.LimitedContentType.Skip, - Take = source.LimitedContentType.Take, - }); - - if (result.ContentItems == null || !result.ContentItems.Any()) - { - return; - } - - foreach (var contentItem in result.ContentItems) - { - var url = new XElement(_namespace + "url"); - - if (await BuildUrlsetMetadataAsync(source, context, result, contentItem, url)) - { - context.Response.ResponseElement.Add(url); - } - } - - return; - } - var queryContext = new ContentItemsQueryContext() { - Take = _batchSize, + Take = _yesSqlOptions.CommandsPageSize, }; + var maxAllowed = int.MaxValue; + if (source.LimitedContentType != null) { queryContext.Skip = source.LimitedContentType.Skip; - // Make sure the 'Take' value does not exceed the batch size. - queryContext.Take = Math.Min(source.LimitedContentType.Take, _batchSize); + if (source.LimitedContentType.Take > 0) + { + maxAllowed = source.LimitedContentType.Take; + } } + var total = 0; + var isLastBatch = false; + while (true) { + if ((total + queryContext.Take) > maxAllowed) + { + queryContext.Take = total + queryContext.Take - maxAllowed; + + isLastBatch = true; + } + var result = await _contentItemsQueryProvider.GetContentItemsAsync(source, queryContext); if (result.ContentItems == null || !result.ContentItems.Any()) @@ -86,6 +77,10 @@ public override async Task BuildSourceAsync(ContentTypesSitemapSource source, Si break; } + var totalFound = result.ContentItems.Count(); + + total += totalFound; + foreach (var contentItem in result.ContentItems) { var url = new XElement(_namespace + "url"); @@ -96,6 +91,11 @@ public override async Task BuildSourceAsync(ContentTypesSitemapSource source, Si } } + if (isLastBatch) + { + break; + } + queryContext.Skip += queryContext.Take; } } From f65b69da6afa7b00cefb3b5e334e38d996fc02a5 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Thu, 29 Aug 2024 17:04:35 -0700 Subject: [PATCH 3/8] use readonly session and batch size of 500. --- .../LocalizedContentItemsQueryProvider.cs | 15 ++++++++------- .../Sitemaps/ContentTypesSitemapSourceBuilder.cs | 9 +++------ .../Sitemaps/DefaultContentItemsQueryProvider.cs | 13 +++++++------ .../DocumentJsonSerializerOptionsConfiguration.cs | 7 ++++++- 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs index 51c1da49b6e..d655a768901 100644 --- a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs @@ -13,17 +13,17 @@ namespace OrchardCore.ContentLocalization.Sitemaps; public class LocalizedContentItemsQueryProvider : IContentItemsQueryProvider { - private readonly ISession _session; + private readonly IStore _store; private readonly IRouteableContentTypeCoordinator _routeableContentTypeCoordinator; private readonly ILocalizationService _localizationService; public LocalizedContentItemsQueryProvider( - ISession session, + IStore store, IRouteableContentTypeCoordinator routeableContentTypeCoordinator, ILocalizationService localizationService ) { - _session = session; + _store = store; _routeableContentTypeCoordinator = routeableContentTypeCoordinator; _localizationService = localizationService; } @@ -31,13 +31,14 @@ ILocalizationService localizationService public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context) { var routeableContentTypeDefinitions = await _routeableContentTypeCoordinator.ListRoutableTypeDefinitionsAsync(); + var session = _store.CreateSession(withTracking: false); if (source.IndexAll) { // Assumption here is that at least one content type will be localized. var ctdNames = routeableContentTypeDefinitions.Select(ctd => ctd.Name); - var results = await _session.Query() + var results = await session.Query() .With(x => x.Published && x.ContentType.IsIn(ctdNames)) .OrderBy(x => x.CreatedUtc) .ThenBy(x => x.Id) @@ -77,7 +78,7 @@ public async Task GetContentItemsAsync(ContentTypesSite // Get all content items here for reference. Then reduce by default culture. // We know that the content item should be localized. // If it doesn't have a localization part, the content item should have been saved. - var contentItems = await _session.Query() + var contentItems = await session.Query() .With(ci => ci.ContentType == source.LimitedContentType.ContentTypeName && ci.Published) .OrderBy(ci => ci.CreatedUtc) .ThenBy(ci => ci.Id) @@ -94,7 +95,7 @@ public async Task GetContentItemsAsync(ContentTypesSite } // Content type is not localized. Produce standard results. - var items = await _session.Query() + var items = await session.Query() .With(x => x.ContentType == source.LimitedContentType.ContentTypeName && x.Published) .OrderBy(x => x.CreatedUtc) .Skip(context.Skip) @@ -114,7 +115,7 @@ public async Task GetContentItemsAsync(ContentTypesSite .Select(x => x.Name); // No advantage here in reducing with localized index. - var queryResults = await _session.Query() + var queryResults = await session.Query() .With(x => x.ContentType.IsIn(typesToIndex) && x.Published) .OrderBy(x => x.CreatedUtc) .ThenBy(x => x.Id) diff --git a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs index 02892766e09..a1b9241c641 100644 --- a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs +++ b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs @@ -1,8 +1,6 @@ using System.Globalization; using System.Xml.Linq; -using Microsoft.Extensions.Options; using OrchardCore.ContentManagement; -using OrchardCore.Data.YesSql; using OrchardCore.Sitemaps.Aspects; using OrchardCore.Sitemaps.Builders; using OrchardCore.Sitemaps.Models; @@ -12,24 +10,23 @@ namespace OrchardCore.Contents.Sitemaps; public class ContentTypesSitemapSourceBuilder : SitemapSourceBuilderBase { + private const int _batchSize = 500; + private static readonly XNamespace _namespace = "http://www.sitemaps.org/schemas/sitemap/0.9"; private readonly IRouteableContentTypeCoordinator _routeableContentTypeCoordinator; private readonly IContentManager _contentManager; - private readonly YesSqlOptions _yesSqlOptions; private readonly IContentItemsQueryProvider _contentItemsQueryProvider; private readonly IEnumerable _sitemapContentItemExtendedMetadataProviders; public ContentTypesSitemapSourceBuilder( IRouteableContentTypeCoordinator routeableContentTypeCoordinator, IContentManager contentManager, - IOptions yesSqlOptions, IContentItemsQueryProvider contentItemsQueryProvider, IEnumerable sitemapContentItemExtendedMetadataProviders) { _routeableContentTypeCoordinator = routeableContentTypeCoordinator; _contentManager = contentManager; - _yesSqlOptions = yesSqlOptions.Value; _contentItemsQueryProvider = contentItemsQueryProvider; _sitemapContentItemExtendedMetadataProviders = sitemapContentItemExtendedMetadataProviders; } @@ -43,7 +40,7 @@ public override async Task BuildSourceAsync(ContentTypesSitemapSource source, Si var queryContext = new ContentItemsQueryContext() { - Take = _yesSqlOptions.CommandsPageSize, + Take = _batchSize, }; var maxAllowed = int.MaxValue; diff --git a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs index 0977a01ac45..df9efe74975 100644 --- a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs @@ -10,15 +10,14 @@ namespace OrchardCore.Contents.Sitemaps; public class DefaultContentItemsQueryProvider : IContentItemsQueryProvider { - private readonly ISession _session; + private readonly IStore _store; private readonly IRouteableContentTypeCoordinator _routeableContentTypeCoordinator; public DefaultContentItemsQueryProvider( - ISession session, - IRouteableContentTypeCoordinator routeableContentTypeCoordinator - ) + IStore store, + IRouteableContentTypeCoordinator routeableContentTypeCoordinator) { - _session = session; + _store = store; _routeableContentTypeCoordinator = routeableContentTypeCoordinator; } @@ -29,7 +28,9 @@ public async Task GetContentItemsAsync(ContentTypesSite var routeableContentTypeDefinitions = await _routeableContentTypeCoordinator.ListRoutableTypeDefinitionsAsync(); - var query = _session.Query(); + var session = _store.CreateSession(withTracking: false); + + var query = session.Query(); if (source.IndexAll) { diff --git a/src/OrchardCore/OrchardCore.Abstractions/Extensions/DocumentJsonSerializerOptionsConfiguration.cs b/src/OrchardCore/OrchardCore.Abstractions/Extensions/DocumentJsonSerializerOptionsConfiguration.cs index 353700268bb..19f6dba8f94 100644 --- a/src/OrchardCore/OrchardCore.Abstractions/Extensions/DocumentJsonSerializerOptionsConfiguration.cs +++ b/src/OrchardCore/OrchardCore.Abstractions/Extensions/DocumentJsonSerializerOptionsConfiguration.cs @@ -17,7 +17,12 @@ public DocumentJsonSerializerOptionsConfiguration(IOptions Date: Thu, 29 Aug 2024 17:11:19 -0700 Subject: [PATCH 4/8] Update LocalizedContentItemsQueryProvider.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Ros --- .../Sitemaps/LocalizedContentItemsQueryProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs index d655a768901..0fe0380627e 100644 --- a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs @@ -31,7 +31,7 @@ ILocalizationService localizationService public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context) { var routeableContentTypeDefinitions = await _routeableContentTypeCoordinator.ListRoutableTypeDefinitionsAsync(); - var session = _store.CreateSession(withTracking: false); + using var session = _store.CreateSession(withTracking: false); if (source.IndexAll) { From c89313f72c4513519b1cfa7a31996e106f2ec17f Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Thu, 29 Aug 2024 17:11:25 -0700 Subject: [PATCH 5/8] Update DefaultContentItemsQueryProvider.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Ros --- .../Sitemaps/DefaultContentItemsQueryProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs index df9efe74975..a6ab136027f 100644 --- a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs @@ -28,7 +28,7 @@ public async Task GetContentItemsAsync(ContentTypesSite var routeableContentTypeDefinitions = await _routeableContentTypeCoordinator.ListRoutableTypeDefinitionsAsync(); - var session = _store.CreateSession(withTracking: false); + using var session = _store.CreateSession(withTracking: false); var query = session.Query(); From 2e4b438088c9bf3c07b557b6470ba677325863ca Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Fri, 30 Aug 2024 09:19:23 -0700 Subject: [PATCH 6/8] remove the breaking change --- .../LocalizedContentItemsQueryProvider.cs | 151 +++++++++++------- ...emapUrlHrefLangExtendedMetadataProvider.cs | 8 +- .../ContentTypesSitemapSourceBuilder.cs | 38 ++--- .../DefaultContentItemsQueryProvider.cs | 32 ++-- .../Builders/ContentItemsQueryContext.cs | 12 +- .../Builders/ContentItemsQueryResult.cs | 16 -- .../Builders/IContentItemsQueryProvider.cs | 2 +- ...emapContentItemExtendedMetadataProvider.cs | 2 +- src/docs/releases/2.0.0.md | 12 -- 9 files changed, 143 insertions(+), 130 deletions(-) delete mode 100644 src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryResult.cs diff --git a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs index d655a768901..43d57d5974f 100644 --- a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs @@ -28,34 +28,38 @@ ILocalizationService localizationService _localizationService = localizationService; } - public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context) + public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int? skip = null, int? take = null) { var routeableContentTypeDefinitions = await _routeableContentTypeCoordinator.ListRoutableTypeDefinitionsAsync(); var session = _store.CreateSession(withTracking: false); + IEnumerable contentItems = null; + if (source.IndexAll) { // Assumption here is that at least one content type will be localized. var ctdNames = routeableContentTypeDefinitions.Select(ctd => ctd.Name); - var results = await session.Query() - .With(x => x.Published && x.ContentType.IsIn(ctdNames)) - .OrderBy(x => x.CreatedUtc) - .ThenBy(x => x.Id) - .Skip(context.Skip) - .Take(context.Take) - .ListAsync(); - - return new ContentItemsQueryResult + if (take.HasValue && take > 0) { - ContentItems = results, - - // Provide all content items with localization as reference content items. - ReferenceContentItems = results.Where(ci => ci.Has()), - }; + contentItems = await session.Query() + .With(x => x.Published && x.ContentType.IsIn(ctdNames)) + .OrderBy(x => x.CreatedUtc) + .ThenBy(x => x.Id) + .Skip(skip ?? 0) + .Take(take.Value) + .ListAsync(); + } + else + { + contentItems = await session.Query() + .With(x => x.Published && x.ContentType.IsIn(ctdNames)) + .OrderBy(x => x.CreatedUtc) + .ThenBy(x => x.Id) + .ListAsync(); + } } - - if (source.LimitItems) + else if (source.LimitItems) { // Test that content type is still valid to include in sitemap. var contentType = routeableContentTypeDefinitions @@ -63,11 +67,7 @@ public async Task GetContentItemsAsync(ContentTypesSite if (contentType == null) { - return new ContentItemsQueryResult - { - ContentItems = [], - ReferenceContentItems = [], - }; + return; } if (contentType.Parts.Any(ctd => string.Equals(ctd.Name, nameof(LocalizationPart), StringComparison.Ordinal))) @@ -78,55 +78,82 @@ public async Task GetContentItemsAsync(ContentTypesSite // Get all content items here for reference. Then reduce by default culture. // We know that the content item should be localized. // If it doesn't have a localization part, the content item should have been saved. - var contentItems = await session.Query() - .With(ci => ci.ContentType == source.LimitedContentType.ContentTypeName && ci.Published) - .OrderBy(ci => ci.CreatedUtc) - .ThenBy(ci => ci.Id) - .With(x => x.Culture == defaultCulture) - .Take(context.Take) - .Skip(context.Skip) - .ListAsync(); + if (take.HasValue && take > 0) + { + contentItems = await session.Query() + .With(ci => ci.ContentType == source.LimitedContentType.ContentTypeName && ci.Published) + .OrderBy(ci => ci.CreatedUtc) + .ThenBy(ci => ci.Id) + .With(x => x.Culture == defaultCulture) + .ListAsync(); + } + else + { + contentItems = await session.Query() + .With(ci => ci.ContentType == source.LimitedContentType.ContentTypeName && ci.Published) + .OrderBy(ci => ci.CreatedUtc) + .ThenBy(ci => ci.Id) + .With(x => x.Culture == defaultCulture) + .ListAsync(); + } + } + else + { - return new ContentItemsQueryResult + if (take.HasValue && take > 0) + { + // Content type is not localized. Produce standard results. + contentItems = await session.Query() + .With(x => x.ContentType == source.LimitedContentType.ContentTypeName && x.Published) + .OrderBy(x => x.CreatedUtc) + .Skip(skip ?? 0) + .Take(take.Value) + .ListAsync(); + } + else { - ContentItems = contentItems, - ReferenceContentItems = contentItems.Where(ci => ci.Has()), - }; + contentItems = await session.Query() + .With(x => x.ContentType == source.LimitedContentType.ContentTypeName && x.Published) + .OrderBy(x => x.CreatedUtc) + .ListAsync(); + } } + } + else + { - // Content type is not localized. Produce standard results. - var items = await session.Query() - .With(x => x.ContentType == source.LimitedContentType.ContentTypeName && x.Published) + // Test that content types are still valid to include in sitemap. + var typesToIndex = routeableContentTypeDefinitions + .Where(ctd => source.ContentTypes.Any(s => string.Equals(ctd.Name, s.ContentTypeName, StringComparison.Ordinal))) + .Select(x => x.Name); + + // No advantage here in reducing with localized index. + if (take.HasValue && take > 0) + { + contentItems = await session.Query() + .With(x => x.ContentType.IsIn(typesToIndex) && x.Published) .OrderBy(x => x.CreatedUtc) - .Skip(context.Skip) - .Take(context.Take) + .ThenBy(x => x.Id) + .Skip(skip ?? 0) + .Take(take.Value) .ListAsync(); - - return new ContentItemsQueryResult + } + else { - ContentItems = items, - ReferenceContentItems = [], - }; + contentItems = await session.Query() + .With(x => x.ContentType.IsIn(typesToIndex) && x.Published) + .OrderBy(x => x.CreatedUtc) + .ThenBy(x => x.Id) + .ListAsync(); + } } - // Test that content types are still valid to include in sitemap. - var typesToIndex = routeableContentTypeDefinitions - .Where(ctd => source.ContentTypes.Any(s => string.Equals(ctd.Name, s.ContentTypeName, StringComparison.Ordinal))) - .Select(x => x.Name); - - // No advantage here in reducing with localized index. - var queryResults = await session.Query() - .With(x => x.ContentType.IsIn(typesToIndex) && x.Published) - .OrderBy(x => x.CreatedUtc) - .ThenBy(x => x.Id) - .Take(context.Take) - .Skip(context.Skip) - .ListAsync(); - - return new ContentItemsQueryResult + if (contentItems != null) { - ContentItems = queryResults, - ReferenceContentItems = queryResults.Where(ci => ci.Has()), - }; + context.ContentItems = contentItems; + + // Provide all content items with localization as reference content items. + context.ReferenceContentItems = contentItems.Where(ci => ci.Has()); + } } } diff --git a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/SitemapUrlHrefLangExtendedMetadataProvider.cs b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/SitemapUrlHrefLangExtendedMetadataProvider.cs index 785d81e8c13..77b8450d880 100644 --- a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/SitemapUrlHrefLangExtendedMetadataProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/SitemapUrlHrefLangExtendedMetadataProvider.cs @@ -28,19 +28,19 @@ IRouteableContentTypeCoordinator routeableContentTypeCoordinator public async Task ApplyExtendedMetadataAsync( SitemapBuilderContext context, - ContentItemsQueryResult queryResult, + ContentItemsQueryContext queryContext, ContentItem contentItem, XElement url) { var part = contentItem.As(); if (part == null || - queryResult.ReferenceContentItems == null || - !queryResult.ReferenceContentItems.Any()) + queryContext.ReferenceContentItems == null || + !queryContext.ReferenceContentItems.Any()) { return true; } - var localizedContentParts = queryResult.ReferenceContentItems + var localizedContentParts = queryContext.ReferenceContentItems .Select(ci => ci.As()) .Where(cp => cp.LocalizationSet == part.LocalizationSet); diff --git a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs index a1b9241c641..406d4e1c816 100644 --- a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs +++ b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/ContentTypesSitemapSourceBuilder.cs @@ -38,16 +38,12 @@ public override async Task BuildSourceAsync(ContentTypesSitemapSource source, Si context.Response.ResponseElement.Add(sciemp.GetExtendedAttribute); } - var queryContext = new ContentItemsQueryContext() - { - Take = _batchSize, - }; - var maxAllowed = int.MaxValue; + var skip = 0; if (source.LimitedContentType != null) { - queryContext.Skip = source.LimitedContentType.Skip; + skip = source.LimitedContentType.Skip; if (source.LimitedContentType.Take > 0) { @@ -56,33 +52,36 @@ public override async Task BuildSourceAsync(ContentTypesSitemapSource source, Si } var total = 0; + var take = _batchSize; var isLastBatch = false; while (true) { - if ((total + queryContext.Take) > maxAllowed) + if ((total + take) > maxAllowed) { - queryContext.Take = total + queryContext.Take - maxAllowed; + take = total + take - maxAllowed; isLastBatch = true; } - var result = await _contentItemsQueryProvider.GetContentItemsAsync(source, queryContext); + var queryContext = new ContentItemsQueryContext(); + + await _contentItemsQueryProvider.GetContentItemsAsync(source, queryContext, skip, take); - if (result.ContentItems == null || !result.ContentItems.Any()) + if (queryContext.ContentItems == null || !queryContext.ContentItems.Any()) { break; } - var totalFound = result.ContentItems.Count(); + var totalFound = queryContext.ContentItems.Count(); total += totalFound; - foreach (var contentItem in result.ContentItems) + foreach (var contentItem in queryContext.ContentItems) { var url = new XElement(_namespace + "url"); - if (await BuildUrlsetMetadataAsync(source, context, result, contentItem, url)) + if (await BuildUrlsetMetadataAsync(source, context, queryContext, contentItem, url)) { context.Response.ResponseElement.Add(url); } @@ -93,15 +92,15 @@ public override async Task BuildSourceAsync(ContentTypesSitemapSource source, Si break; } - queryContext.Skip += queryContext.Take; + skip += take; } } - private async Task BuildUrlsetMetadataAsync(ContentTypesSitemapSource source, SitemapBuilderContext context, ContentItemsQueryResult queryResult, ContentItem contentItem, XElement url) + private async Task BuildUrlsetMetadataAsync(ContentTypesSitemapSource source, SitemapBuilderContext context, ContentItemsQueryContext queryContext, ContentItem contentItem, XElement url) { if (await BuildUrlAsync(context, contentItem, url)) { - if (await BuildExtendedMetadataAsync(context, queryResult, contentItem, url)) + if (await BuildExtendedMetadataAsync(context, queryContext, contentItem, url)) { PopulateLastMod(contentItem, url); @@ -116,16 +115,18 @@ private async Task BuildUrlsetMetadataAsync(ContentTypesSitemapSource sour return false; } - private async Task BuildExtendedMetadataAsync(SitemapBuilderContext context, ContentItemsQueryResult queryResult, ContentItem contentItem, XElement url) + private async Task BuildExtendedMetadataAsync(SitemapBuilderContext context, ContentItemsQueryContext queryContext, ContentItem contentItem, XElement url) { var succeeded = true; + foreach (var sc in _sitemapContentItemExtendedMetadataProviders) { - if (!await sc.ApplyExtendedMetadataAsync(context, queryResult, contentItem, url)) + if (!await sc.ApplyExtendedMetadataAsync(context, queryContext, contentItem, url)) { succeeded = false; } } + return succeeded; } @@ -142,6 +143,7 @@ private async Task BuildUrlAsync(SitemapBuilderContext context, ContentIte var loc = new XElement(_namespace + "loc"); loc.Add(locValue); url.Add(loc); + return true; } diff --git a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs index df9efe74975..4d71f276048 100644 --- a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs @@ -21,7 +21,7 @@ public DefaultContentItemsQueryProvider( _routeableContentTypeCoordinator = routeableContentTypeCoordinator; } - public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context) + public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int? skip = null, int? take = null) { ArgumentNullException.ThrowIfNull(source); ArgumentNullException.ThrowIfNull(context); @@ -46,10 +46,7 @@ public async Task GetContentItemsAsync(ContentTypesSite if (!typeIsValid) { - return new ContentItemsQueryResult() - { - ContentItems = [], - }; + return; } query = query.Where(x => x.ContentType == source.LimitedContentType.ContentTypeName && x.Published); @@ -64,15 +61,22 @@ public async Task GetContentItemsAsync(ContentTypesSite query = query.Where(x => x.ContentType.IsIn(typesToIndex) && x.Published); } - return new ContentItemsQueryResult + query = query.OrderBy(x => x.CreatedUtc) + .ThenBy(x => x.Id); + + if (take.HasValue && take > 0) { - ContentItems = await query - .OrderBy(x => x.CreatedUtc) - .ThenBy(x => x.Id) - .Take(context.Take) - .Skip(context.Skip) - .ListAsync(), - ReferenceContentItems = [], - }; + context.ContentItems = await query + .Skip(skip ?? 0) + .Take(take.Value) + .ListAsync(); + + return; + } + + context.ContentItems = await query + .OrderBy(x => x.CreatedUtc) + .ThenBy(x => x.Id) + .ListAsync(); } } diff --git a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryContext.cs b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryContext.cs index cbd34021ec4..7674c53bf47 100644 --- a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryContext.cs +++ b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryContext.cs @@ -1,8 +1,16 @@ +using OrchardCore.ContentManagement; + namespace OrchardCore.Sitemaps.Builders; public class ContentItemsQueryContext { - public int Skip { get; set; } + /// + /// Content items to include in sitemap. + /// + public IEnumerable ContentItems { get; set; } = []; - public int Take { get; set; } = 500; + /// + /// Reference content items that may be used to perform a lookup for url alternatives. + /// + public IEnumerable ReferenceContentItems { get; set; } = []; } diff --git a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryResult.cs b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryResult.cs deleted file mode 100644 index 0f44b5894d8..00000000000 --- a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ContentItemsQueryResult.cs +++ /dev/null @@ -1,16 +0,0 @@ -using OrchardCore.ContentManagement; - -namespace OrchardCore.Sitemaps.Builders; - -public class ContentItemsQueryResult -{ - /// - /// Content items to include in sitemap. - /// - public IEnumerable ContentItems { get; set; } - - /// - /// Reference content items that may be used to perform a lookup for url alternatives. - /// - public IEnumerable ReferenceContentItems { get; set; } -} diff --git a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs index a8f3cf2945f..f6b4793ff23 100644 --- a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs +++ b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs @@ -7,5 +7,5 @@ public interface IContentItemsQueryProvider /// /// Get content items to evaluate for inclusion in a sitemap. /// - Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context); + Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int? skip = null, int? take = null); } diff --git a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ISitemapContentItemExtendedMetadataProvider.cs b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ISitemapContentItemExtendedMetadataProvider.cs index 3e75dce9660..2d8c6746fb4 100644 --- a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ISitemapContentItemExtendedMetadataProvider.cs +++ b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/ISitemapContentItemExtendedMetadataProvider.cs @@ -16,5 +16,5 @@ public interface ISitemapContentItemExtendedMetadataProvider /// /// Apply extended metadata to the url element. /// - Task ApplyExtendedMetadataAsync(SitemapBuilderContext context, ContentItemsQueryResult queryResult, ContentItem contentItem, XElement url); + Task ApplyExtendedMetadataAsync(SitemapBuilderContext context, ContentItemsQueryContext queryContext, ContentItem contentItem, XElement url); } diff --git a/src/docs/releases/2.0.0.md b/src/docs/releases/2.0.0.md index c48feff7d7c..552da36863c 100644 --- a/src/docs/releases/2.0.0.md +++ b/src/docs/releases/2.0.0.md @@ -398,18 +398,6 @@ Previously, the `CreateContentTask`, `RetrieveContentTask`, and `UpdateContentTa Additionally, a new workflow-scoped script function `setCorrelationId(id:string): void` was added, that you can use to update the workflow's CorrelationId. -### Sitemaps - -To enhance sitemap performance, the `IContentItemsQueryProvider` interface was updated. The `GetContentItemsAsync` method was modified to the following signature: - -```csharp -Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context) -``` - -In the `ContentItemsQueryContext` class, the `ContentItems` and `ReferenceContentItems` properties were removed and replaced by `Skip` and `Take` properties to allow batching. The `ContentItemsQueryResult` class now holds the `ContentItems` and `ReferenceContentItems` properties. - -Additionally, the `ISitemapContentItemExtendedMetadataProvider` interface was updated to accept a `ContentItemsQueryResult` object as its second parameter, replacing the previous `ContentItemsQueryContext` parameter. - ## Change Logs From 425dc0a10bbc721da6c65bceca75e84f60998e2a Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Fri, 30 Aug 2024 09:34:28 -0700 Subject: [PATCH 7/8] make the parameters not nullable with a default value --- .../LocalizedContentItemsQueryProvider.cs | 94 ++++++------------- .../DefaultContentItemsQueryProvider.cs | 17 +--- .../Builders/IContentItemsQueryProvider.cs | 2 +- 3 files changed, 31 insertions(+), 82 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs index c89336ea005..7b025956ada 100644 --- a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs @@ -28,7 +28,7 @@ ILocalizationService localizationService _localizationService = localizationService; } - public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int? skip = null, int? take = null) + public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int skip = 0, int take = 500) { var routeableContentTypeDefinitions = await _routeableContentTypeCoordinator.ListRoutableTypeDefinitionsAsync(); using var session = _store.CreateSession(withTracking: false); @@ -40,24 +40,14 @@ public async Task GetContentItemsAsync(ContentTypesSitemapSource source, Content // Assumption here is that at least one content type will be localized. var ctdNames = routeableContentTypeDefinitions.Select(ctd => ctd.Name); - if (take.HasValue && take > 0) - { - contentItems = await session.Query() - .With(x => x.Published && x.ContentType.IsIn(ctdNames)) - .OrderBy(x => x.CreatedUtc) - .ThenBy(x => x.Id) - .Skip(skip ?? 0) - .Take(take.Value) - .ListAsync(); - } - else - { - contentItems = await session.Query() - .With(x => x.Published && x.ContentType.IsIn(ctdNames)) - .OrderBy(x => x.CreatedUtc) - .ThenBy(x => x.Id) - .ListAsync(); - } + contentItems = await session.Query() + .With(x => x.Published && x.ContentType.IsIn(ctdNames)) + .OrderBy(x => x.CreatedUtc) + .ThenBy(x => x.Id) + .Skip(skip) + .Take(take) + .ListAsync(); + } else if (source.LimitItems) { @@ -78,74 +68,44 @@ public async Task GetContentItemsAsync(ContentTypesSitemapSource source, Content // Get all content items here for reference. Then reduce by default culture. // We know that the content item should be localized. // If it doesn't have a localization part, the content item should have been saved. - if (take.HasValue && take > 0) - { - contentItems = await session.Query() - .With(ci => ci.ContentType == source.LimitedContentType.ContentTypeName && ci.Published) - .OrderBy(ci => ci.CreatedUtc) - .ThenBy(ci => ci.Id) - .With(x => x.Culture == defaultCulture) - .ListAsync(); - } - else - { - contentItems = await session.Query() - .With(ci => ci.ContentType == source.LimitedContentType.ContentTypeName && ci.Published) - .OrderBy(ci => ci.CreatedUtc) - .ThenBy(ci => ci.Id) - .With(x => x.Culture == defaultCulture) - .ListAsync(); - } + contentItems = await session.Query() + .With(ci => ci.ContentType == source.LimitedContentType.ContentTypeName && ci.Published) + .OrderBy(ci => ci.CreatedUtc) + .ThenBy(ci => ci.Id) + .With(x => x.Culture == defaultCulture) + .Take(take) + .Skip(skip) + .ListAsync(); } else { - - if (take.HasValue && take > 0) - { - // Content type is not localized. Produce standard results. - contentItems = await session.Query() + // Content type is not localized. Produce standard results. + contentItems = await session.Query() .With(x => x.ContentType == source.LimitedContentType.ContentTypeName && x.Published) .OrderBy(x => x.CreatedUtc) - .Skip(skip ?? 0) - .Take(take.Value) + .Skip(skip) + .Take(take) .ListAsync(); - } - else - { - contentItems = await session.Query() - .With(x => x.ContentType == source.LimitedContentType.ContentTypeName && x.Published) - .OrderBy(x => x.CreatedUtc) - .ListAsync(); - } + } } else { - // Test that content types are still valid to include in sitemap. var typesToIndex = routeableContentTypeDefinitions .Where(ctd => source.ContentTypes.Any(s => string.Equals(ctd.Name, s.ContentTypeName, StringComparison.Ordinal))) .Select(x => x.Name); // No advantage here in reducing with localized index. - if (take.HasValue && take > 0) - { - contentItems = await session.Query() - .With(x => x.ContentType.IsIn(typesToIndex) && x.Published) - .OrderBy(x => x.CreatedUtc) - .ThenBy(x => x.Id) - .Skip(skip ?? 0) - .Take(take.Value) - .ListAsync(); - } - else - { - contentItems = await session.Query() + + contentItems = await session.Query() .With(x => x.ContentType.IsIn(typesToIndex) && x.Published) .OrderBy(x => x.CreatedUtc) .ThenBy(x => x.Id) + .Skip(skip) + .Take(take) .ListAsync(); - } + } if (contentItems != null) diff --git a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs index 1106b7e1db2..50f0bcc0ef6 100644 --- a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs @@ -21,7 +21,7 @@ public DefaultContentItemsQueryProvider( _routeableContentTypeCoordinator = routeableContentTypeCoordinator; } - public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int? skip = null, int? take = null) + public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int skip = 0, int take = 500) { ArgumentNullException.ThrowIfNull(source); ArgumentNullException.ThrowIfNull(context); @@ -61,22 +61,11 @@ public async Task GetContentItemsAsync(ContentTypesSitemapSource source, Content query = query.Where(x => x.ContentType.IsIn(typesToIndex) && x.Published); } - query = query.OrderBy(x => x.CreatedUtc) - .ThenBy(x => x.Id); - - if (take.HasValue && take > 0) - { - context.ContentItems = await query - .Skip(skip ?? 0) - .Take(take.Value) - .ListAsync(); - - return; - } - context.ContentItems = await query .OrderBy(x => x.CreatedUtc) .ThenBy(x => x.Id) + .Take(take) + .Skip(skip) .ListAsync(); } } diff --git a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs index f6b4793ff23..f3f8e9d8cd1 100644 --- a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs +++ b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs @@ -7,5 +7,5 @@ public interface IContentItemsQueryProvider /// /// Get content items to evaluate for inclusion in a sitemap. /// - Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int? skip = null, int? take = null); + Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int skip = 0, int take = 500); } From 22613ce9e9c17a8c01f1254c5c5b84cf037048b6 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Fri, 30 Aug 2024 14:45:16 -0700 Subject: [PATCH 8/8] address feedback --- .../LocalizedContentItemsQueryProvider.cs | 18 +++++++++--------- .../DefaultContentItemsQueryProvider.cs | 6 +++--- .../Builders/IContentItemsQueryProvider.cs | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs index 7b025956ada..122db4ba656 100644 --- a/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.ContentLocalization/Sitemaps/LocalizedContentItemsQueryProvider.cs @@ -28,7 +28,7 @@ ILocalizationService localizationService _localizationService = localizationService; } - public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int skip = 0, int take = 500) + public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int? skip = null, int? take = null) { var routeableContentTypeDefinitions = await _routeableContentTypeCoordinator.ListRoutableTypeDefinitionsAsync(); using var session = _store.CreateSession(withTracking: false); @@ -44,8 +44,8 @@ public async Task GetContentItemsAsync(ContentTypesSitemapSource source, Content .With(x => x.Published && x.ContentType.IsIn(ctdNames)) .OrderBy(x => x.CreatedUtc) .ThenBy(x => x.Id) - .Skip(skip) - .Take(take) + .Skip(skip ?? 0) + .Take(take ?? 0) .ListAsync(); } @@ -73,8 +73,8 @@ public async Task GetContentItemsAsync(ContentTypesSitemapSource source, Content .OrderBy(ci => ci.CreatedUtc) .ThenBy(ci => ci.Id) .With(x => x.Culture == defaultCulture) - .Take(take) - .Skip(skip) + .Take(take ?? 0) + .Skip(skip ?? 0) .ListAsync(); } else @@ -83,8 +83,8 @@ public async Task GetContentItemsAsync(ContentTypesSitemapSource source, Content contentItems = await session.Query() .With(x => x.ContentType == source.LimitedContentType.ContentTypeName && x.Published) .OrderBy(x => x.CreatedUtc) - .Skip(skip) - .Take(take) + .Skip(skip ?? 0) + .Take(take ?? 0) .ListAsync(); } @@ -102,8 +102,8 @@ public async Task GetContentItemsAsync(ContentTypesSitemapSource source, Content .With(x => x.ContentType.IsIn(typesToIndex) && x.Published) .OrderBy(x => x.CreatedUtc) .ThenBy(x => x.Id) - .Skip(skip) - .Take(take) + .Skip(skip ?? 0) + .Take(take ?? 0) .ListAsync(); } diff --git a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs index 50f0bcc0ef6..be23e40f8f0 100644 --- a/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.Contents/Sitemaps/DefaultContentItemsQueryProvider.cs @@ -21,7 +21,7 @@ public DefaultContentItemsQueryProvider( _routeableContentTypeCoordinator = routeableContentTypeCoordinator; } - public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int skip = 0, int take = 500) + public async Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int? skip = null, int? take = null) { ArgumentNullException.ThrowIfNull(source); ArgumentNullException.ThrowIfNull(context); @@ -64,8 +64,8 @@ public async Task GetContentItemsAsync(ContentTypesSitemapSource source, Content context.ContentItems = await query .OrderBy(x => x.CreatedUtc) .ThenBy(x => x.Id) - .Take(take) - .Skip(skip) + .Take(take ?? 0) + .Skip(skip ?? 0) .ListAsync(); } } diff --git a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs index f3f8e9d8cd1..f6b4793ff23 100644 --- a/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs +++ b/src/OrchardCore/OrchardCore.Sitemaps.Abstractions/Builders/IContentItemsQueryProvider.cs @@ -7,5 +7,5 @@ public interface IContentItemsQueryProvider /// /// Get content items to evaluate for inclusion in a sitemap. /// - Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int skip = 0, int take = 500); + Task GetContentItemsAsync(ContentTypesSitemapSource source, ContentItemsQueryContext context, int? skip = null, int? take = null); }