From f1c83fbd80fde9f6aea716c4f0fef76beed32f7a Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Tue, 15 Feb 2022 16:46:20 +0100 Subject: [PATCH] Only select ItemIdFieldName in PublishedContentQuery.Search to improve performance (#11950) * Fix code styling * Only select ItemIdFieldName and fix exception when variation context is null * Save hashset instead of making on on each request (indirectly) Co-authored-by: Bjarke Berg --- .../IPublishedContentQuery.cs | 21 +- .../PublishedContentQuery.cs | 241 +++++++++--------- 2 files changed, 132 insertions(+), 130 deletions(-) diff --git a/src/Umbraco.Infrastructure/IPublishedContentQuery.cs b/src/Umbraco.Infrastructure/IPublishedContentQuery.cs index 77ca873638..0f98fb2924 100644 --- a/src/Umbraco.Infrastructure/IPublishedContentQuery.cs +++ b/src/Umbraco.Infrastructure/IPublishedContentQuery.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Xml.XPath; using Examine.Search; @@ -8,31 +8,46 @@ using Umbraco.Cms.Core.Xml; namespace Umbraco.Cms.Core { /// - /// Query methods used for accessing strongly typed content in templates + /// Query methods used for accessing strongly typed content in templates. /// public interface IPublishedContentQuery { IPublishedContent Content(int id); + IPublishedContent Content(Guid id); + IPublishedContent Content(Udi id); + IPublishedContent Content(object id); + IPublishedContent ContentSingleAtXPath(string xpath, params XPathVariable[] vars); + IEnumerable Content(IEnumerable ids); + IEnumerable Content(IEnumerable ids); IEnumerable Content(IEnumerable ids); + IEnumerable ContentAtXPath(string xpath, params XPathVariable[] vars); + IEnumerable ContentAtXPath(XPathExpression xpath, params XPathVariable[] vars); + IEnumerable ContentAtRoot(); IPublishedContent Media(int id); + IPublishedContent Media(Guid id); + IPublishedContent Media(Udi id); IPublishedContent Media(object id); + IEnumerable Media(IEnumerable ids); + IEnumerable Media(IEnumerable ids); + IEnumerable Media(IEnumerable ids); + IEnumerable MediaAtRoot(); /// @@ -44,7 +59,7 @@ namespace Umbraco.Cms.Core /// The total amount of records. /// The culture (defaults to a culture insensitive search). /// The name of the index to search (defaults to ). - /// The fields to load in the results of the search (defaults to all fields loaded). + /// This parameter is no longer used, because the results are loaded from the published snapshot using the single item ID field. /// /// The search results. /// diff --git a/src/Umbraco.Infrastructure/PublishedContentQuery.cs b/src/Umbraco.Infrastructure/PublishedContentQuery.cs index d8119f919c..d0afe53540 100644 --- a/src/Umbraco.Infrastructure/PublishedContentQuery.cs +++ b/src/Umbraco.Infrastructure/PublishedContentQuery.cs @@ -17,23 +17,25 @@ using Constants = Umbraco.Cms.Core.Constants; namespace Umbraco.Cms.Infrastructure { /// - /// A class used to query for published content, media items + /// A class used to query for published content, media items /// + /// public class PublishedContentQuery : IPublishedContentQuery { private readonly IExamineManager _examineManager; private readonly IPublishedSnapshot _publishedSnapshot; private readonly IVariationContextAccessor _variationContextAccessor; + private static readonly HashSet s_itemIdFieldNameHashSet = + new HashSet() { ExamineFieldNames.ItemIdFieldName }; + /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// - public PublishedContentQuery(IPublishedSnapshot publishedSnapshot, - IVariationContextAccessor variationContextAccessor, IExamineManager examineManager) + public PublishedContentQuery(IPublishedSnapshot publishedSnapshot, IVariationContextAccessor variationContextAccessor, IExamineManager examineManager) { _publishedSnapshot = publishedSnapshot ?? throw new ArgumentNullException(nameof(publishedSnapshot)); - _variationContextAccessor = variationContextAccessor ?? - throw new ArgumentNullException(nameof(variationContextAccessor)); + _variationContextAccessor = variationContextAccessor ?? throw new ArgumentNullException(nameof(variationContextAccessor)); _examineManager = examineManager ?? throw new ArgumentNullException(nameof(examineManager)); } @@ -72,6 +74,7 @@ namespace Umbraco.Cms.Infrastructure return false; } } + private static bool ConvertIdObjectToUdi(object id, out Udi guidId) { switch (id) @@ -93,160 +96,148 @@ namespace Umbraco.Cms.Infrastructure #region Content - public IPublishedContent Content(int id) => ItemById(id, _publishedSnapshot.Content); + public IPublishedContent Content(int id) + => ItemById(id, _publishedSnapshot.Content); - public IPublishedContent Content(Guid id) => ItemById(id, _publishedSnapshot.Content); + public IPublishedContent Content(Guid id) + => ItemById(id, _publishedSnapshot.Content); public IPublishedContent Content(Udi id) - { - if (!(id is GuidUdi udi)) return null; - return ItemById(udi.Guid, _publishedSnapshot.Content); - } + => id is GuidUdi udi ? ItemById(udi.Guid, _publishedSnapshot.Content) : null; public IPublishedContent Content(object id) { if (ConvertIdObjectToInt(id, out var intId)) + { return Content(intId); + } + if (ConvertIdObjectToGuid(id, out var guidId)) + { return Content(guidId); + } + if (ConvertIdObjectToUdi(id, out var udiId)) + { return Content(udiId); + } + return null; } - public IPublishedContent ContentSingleAtXPath(string xpath, params XPathVariable[] vars) => - ItemByXPath(xpath, vars, _publishedSnapshot.Content); + public IPublishedContent ContentSingleAtXPath(string xpath, params XPathVariable[] vars) + => ItemByXPath(xpath, vars, _publishedSnapshot.Content); - public IEnumerable Content(IEnumerable ids) => - ItemsByIds(_publishedSnapshot.Content, ids); + public IEnumerable Content(IEnumerable ids) + => ItemsByIds(_publishedSnapshot.Content, ids); - public IEnumerable Content(IEnumerable ids) => - ItemsByIds(_publishedSnapshot.Content, ids); + public IEnumerable Content(IEnumerable ids) + => ItemsByIds(_publishedSnapshot.Content, ids); public IEnumerable Content(IEnumerable ids) - { - return ids.Select(Content).WhereNotNull(); - } - public IEnumerable ContentAtXPath(string xpath, params XPathVariable[] vars) => - ItemsByXPath(xpath, vars, _publishedSnapshot.Content); + => ids.Select(Content).WhereNotNull(); - public IEnumerable ContentAtXPath(XPathExpression xpath, params XPathVariable[] vars) => - ItemsByXPath(xpath, vars, _publishedSnapshot.Content); + public IEnumerable ContentAtXPath(string xpath, params XPathVariable[] vars) + => ItemsByXPath(xpath, vars, _publishedSnapshot.Content); - public IEnumerable ContentAtRoot() => ItemsAtRoot(_publishedSnapshot.Content); + public IEnumerable ContentAtXPath(XPathExpression xpath, params XPathVariable[] vars) + => ItemsByXPath(xpath, vars, _publishedSnapshot.Content); + + public IEnumerable ContentAtRoot() + => ItemsAtRoot(_publishedSnapshot.Content); #endregion #region Media - public IPublishedContent Media(int id) => ItemById(id, _publishedSnapshot.Media); + public IPublishedContent Media(int id) + => ItemById(id, _publishedSnapshot.Media); - public IPublishedContent Media(Guid id) => ItemById(id, _publishedSnapshot.Media); + public IPublishedContent Media(Guid id) + => ItemById(id, _publishedSnapshot.Media); public IPublishedContent Media(Udi id) - { - if (!(id is GuidUdi udi)) return null; - return ItemById(udi.Guid, _publishedSnapshot.Media); - } + => id is GuidUdi udi ? ItemById(udi.Guid, _publishedSnapshot.Media) : null; public IPublishedContent Media(object id) { if (ConvertIdObjectToInt(id, out var intId)) + { return Media(intId); + } + if (ConvertIdObjectToGuid(id, out var guidId)) + { return Media(guidId); + } + if (ConvertIdObjectToUdi(id, out var udiId)) + { return Media(udiId); + } + return null; } - public IEnumerable Media(IEnumerable ids) => ItemsByIds(_publishedSnapshot.Media, ids); + public IEnumerable Media(IEnumerable ids) + => ItemsByIds(_publishedSnapshot.Media, ids); + public IEnumerable Media(IEnumerable ids) - { - return ids.Select(Media).WhereNotNull(); - } + => ids.Select(Media).WhereNotNull(); - public IEnumerable Media(IEnumerable ids) => ItemsByIds(_publishedSnapshot.Media, ids); + public IEnumerable Media(IEnumerable ids) + => ItemsByIds(_publishedSnapshot.Media, ids); - public IEnumerable MediaAtRoot() => ItemsAtRoot(_publishedSnapshot.Media); + public IEnumerable MediaAtRoot() + => ItemsAtRoot(_publishedSnapshot.Media); #endregion #region Used by Content/Media private static IPublishedContent ItemById(int id, IPublishedCache cache) - { - var doc = cache.GetById(id); - return doc; - } + => cache.GetById(id); private static IPublishedContent ItemById(Guid id, IPublishedCache cache) - { - var doc = cache.GetById(id); - return doc; - } + => cache.GetById(id); private static IPublishedContent ItemByXPath(string xpath, XPathVariable[] vars, IPublishedCache cache) - { - var doc = cache.GetSingleByXPath(xpath, vars); - return doc; - } - - //NOTE: Not used? - //private IPublishedContent ItemByXPath(XPathExpression xpath, XPathVariable[] vars, IPublishedCache cache) - //{ - // var doc = cache.GetSingleByXPath(xpath, vars); - // return doc; - //} + => cache.GetSingleByXPath(xpath, vars); private static IEnumerable ItemsByIds(IPublishedCache cache, IEnumerable ids) - { - return ids.Select(eachId => ItemById(eachId, cache)).WhereNotNull(); - } + => ids.Select(eachId => ItemById(eachId, cache)).WhereNotNull(); private IEnumerable ItemsByIds(IPublishedCache cache, IEnumerable ids) - { - return ids.Select(eachId => ItemById(eachId, cache)).WhereNotNull(); - } + => ids.Select(eachId => ItemById(eachId, cache)).WhereNotNull(); - private static IEnumerable ItemsByXPath(string xpath, XPathVariable[] vars, - IPublishedCache cache) - { - var doc = cache.GetByXPath(xpath, vars); - return doc; - } + private static IEnumerable ItemsByXPath(string xpath, XPathVariable[] vars, IPublishedCache cache) + => cache.GetByXPath(xpath, vars); - private static IEnumerable ItemsByXPath(XPathExpression xpath, XPathVariable[] vars, - IPublishedCache cache) - { - var doc = cache.GetByXPath(xpath, vars); - return doc; - } + private static IEnumerable ItemsByXPath(XPathExpression xpath, XPathVariable[] vars, IPublishedCache cache) + => cache.GetByXPath(xpath, vars); - private static IEnumerable ItemsAtRoot(IPublishedCache cache) => cache.GetAtRoot(); + private static IEnumerable ItemsAtRoot(IPublishedCache cache) + => cache.GetAtRoot(); #endregion #region Search /// - public IEnumerable Search(string term, string culture = "*", - string indexName = Constants.UmbracoIndexes.ExternalIndexName) => - Search(term, 0, 0, out _, culture, indexName); + public IEnumerable Search(string term, string culture = "*", string indexName = Constants.UmbracoIndexes.ExternalIndexName) + => Search(term, 0, 0, out _, culture, indexName); /// public IEnumerable Search(string term, int skip, int take, out long totalRecords, string culture = "*", string indexName = Constants.UmbracoIndexes.ExternalIndexName, ISet loadedFields = null) { if (skip < 0) { - throw new ArgumentOutOfRangeException(nameof(skip), skip, - "The value must be greater than or equal to zero."); + throw new ArgumentOutOfRangeException(nameof(skip), skip, "The value must be greater than or equal to zero."); } if (take < 0) { - throw new ArgumentOutOfRangeException(nameof(take), take, - "The value must be greater than or equal to zero."); + throw new ArgumentOutOfRangeException(nameof(take), take, "The value must be greater than or equal to zero."); } if (string.IsNullOrEmpty(indexName)) @@ -254,68 +245,67 @@ namespace Umbraco.Cms.Infrastructure indexName = Constants.UmbracoIndexes.ExternalIndexName; } - if (!_examineManager.TryGetIndex(indexName, out var index) || !(index is IUmbracoIndex umbIndex)) + if (!_examineManager.TryGetIndex(indexName, out var index) || index is not IUmbracoIndex umbIndex) { - throw new InvalidOperationException( - $"No index found by name {indexName} or is not of type {typeof(IUmbracoIndex)}"); + throw new InvalidOperationException($"No index found by name {indexName} or is not of type {typeof(IUmbracoIndex)}"); } var query = umbIndex.Searcher.CreateQuery(IndexTypes.Content); - IQueryExecutor queryExecutor; + IOrdering ordering; if (culture == "*") { // Search everything - queryExecutor = query.ManagedQuery(term); + ordering = query.ManagedQuery(term); } else if (string.IsNullOrWhiteSpace(culture)) { // Only search invariant - queryExecutor = query + ordering = query .Field(UmbracoExamineFieldNames.VariesByCultureFieldName, "n") // Must not vary by culture .And().ManagedQuery(term); } else { // Only search the specified culture - var fields = - umbIndex.GetCultureAndInvariantFields(culture) - .ToArray(); // Get all index fields suffixed with the culture name supplied - queryExecutor = query.ManagedQuery(term, fields); - } - if (loadedFields != null && queryExecutor is IBooleanOperation booleanOperation) - { - queryExecutor = booleanOperation.SelectFields(loadedFields); + var fields = umbIndex.GetCultureAndInvariantFields(culture).ToArray(); // Get all index fields suffixed with the culture name supplied + ordering = query.ManagedQuery(term, fields); } + // Only select item ID field, because results are loaded from the published snapshot based on this single value + var queryExecutor = ordering.SelectFields(s_itemIdFieldNameHashSet); + + var results = skip == 0 && take == 0 ? queryExecutor.Execute() : queryExecutor.Execute(QueryOptions.SkipTake(skip, take)); totalRecords = results.TotalItemCount; - return new CultureContextualSearchResults( - results.ToPublishedSearchResults(_publishedSnapshot.Content), _variationContextAccessor, - culture); + return new CultureContextualSearchResults(results.ToPublishedSearchResults(_publishedSnapshot.Content), _variationContextAccessor, culture); } /// - public IEnumerable Search(IQueryExecutor query) => Search(query, 0, 0, out _); + public IEnumerable Search(IQueryExecutor query) + => Search(query, 0, 0, out _); /// - public IEnumerable Search(IQueryExecutor query, int skip, int take, - out long totalRecords) + public IEnumerable Search(IQueryExecutor query, int skip, int take, out long totalRecords) { if (skip < 0) { - throw new ArgumentOutOfRangeException(nameof(skip), skip, - "The value must be greater than or equal to zero."); + throw new ArgumentOutOfRangeException(nameof(skip), skip, "The value must be greater than or equal to zero."); } if (take < 0) { - throw new ArgumentOutOfRangeException(nameof(take), take, - "The value must be greater than or equal to zero."); + throw new ArgumentOutOfRangeException(nameof(take), take, "The value must be greater than or equal to zero."); + } + + if (query is IOrdering ordering) + { + // Only select item ID field, because results are loaded from the published snapshot based on this single value + query = ordering.SelectFields(s_itemIdFieldNameHashSet); } var results = skip == 0 && take == 0 @@ -328,8 +318,7 @@ namespace Umbraco.Cms.Infrastructure } /// - /// This is used to contextualize the values in the search results when enumerating over them so that the correct - /// culture values are used + /// This is used to contextualize the values in the search results when enumerating over them, so that the correct culture values are used. /// private class CultureContextualSearchResults : IEnumerable { @@ -337,8 +326,7 @@ namespace Umbraco.Cms.Infrastructure private readonly IVariationContextAccessor _variationContextAccessor; private readonly IEnumerable _wrapped; - public CultureContextualSearchResults(IEnumerable wrapped, - IVariationContextAccessor variationContextAccessor, string culture) + public CultureContextualSearchResults(IEnumerable wrapped, IVariationContextAccessor variationContextAccessor, string culture) { _wrapped = wrapped; _variationContextAccessor = variationContextAccessor; @@ -347,20 +335,21 @@ namespace Umbraco.Cms.Infrastructure public IEnumerator GetEnumerator() { - //We need to change the current culture to what is requested and then change it back + // We need to change the current culture to what is requested and then change it back var originalContext = _variationContextAccessor.VariationContext; - if (!_culture.IsNullOrWhiteSpace() && !_culture.InvariantEquals(originalContext.Culture)) + if (!_culture.IsNullOrWhiteSpace() && !_culture.InvariantEquals(originalContext?.Culture)) + { _variationContextAccessor.VariationContext = new VariationContext(_culture); + } - //now the IPublishedContent returned will be contextualized to the culture specified and will be reset when the enumerator is disposed - return new CultureContextualSearchResultsEnumerator(_wrapped.GetEnumerator(), _variationContextAccessor, - originalContext); + // Now the IPublishedContent returned will be contextualized to the culture specified and will be reset when the enumerator is disposed + return new CultureContextualSearchResultsEnumerator(_wrapped.GetEnumerator(), _variationContextAccessor, originalContext); } IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); /// - /// Resets the variation context when this is disposed + /// Resets the variation context when this is disposed. /// private class CultureContextualSearchResultsEnumerator : IEnumerator { @@ -368,30 +357,28 @@ namespace Umbraco.Cms.Infrastructure private readonly IVariationContextAccessor _variationContextAccessor; private readonly IEnumerator _wrapped; - public CultureContextualSearchResultsEnumerator(IEnumerator wrapped, - IVariationContextAccessor variationContextAccessor, VariationContext originalContext) + public CultureContextualSearchResultsEnumerator(IEnumerator wrapped, IVariationContextAccessor variationContextAccessor, VariationContext originalContext) { _wrapped = wrapped; _variationContextAccessor = variationContextAccessor; _originalContext = originalContext; } + public PublishedSearchResult Current => _wrapped.Current; + + object IEnumerator.Current => Current; + public void Dispose() { _wrapped.Dispose(); - //reset + + // Reset to original variation context _variationContextAccessor.VariationContext = _originalContext; } public bool MoveNext() => _wrapped.MoveNext(); - public void Reset() - { - _wrapped.Reset(); - } - - public PublishedSearchResult Current => _wrapped.Current; - object IEnumerator.Current => Current; + public void Reset() => _wrapped.Reset(); } }