From b0d834668d0631d14bd580ae615e08616ed33f07 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Fri, 1 Nov 2019 10:49:45 +0100 Subject: [PATCH 1/8] Fix search not skipping results --- src/Umbraco.Web/PublishedContentQuery.cs | 57 +++++++++++++----------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/src/Umbraco.Web/PublishedContentQuery.cs b/src/Umbraco.Web/PublishedContentQuery.cs index 2dbe4de4c5..72aec94e58 100644 --- a/src/Umbraco.Web/PublishedContentQuery.cs +++ b/src/Umbraco.Web/PublishedContentQuery.cs @@ -191,46 +191,52 @@ namespace Umbraco.Web /// public IEnumerable Search(string term, int skip, int take, out long totalRecords, string culture = "*", string indexName = null) { - indexName = string.IsNullOrEmpty(indexName) - ? Constants.UmbracoIndexes.ExternalIndexName - : indexName; + if (string.IsNullOrEmpty(indexName)) + { + indexName = Constants.UmbracoIndexes.ExternalIndexName; + } if (!_examineManager.TryGetIndex(indexName, out var index) || !(index is IUmbracoIndex umbIndex)) + { throw new InvalidOperationException($"No index found by name {indexName} or is not of type {typeof(IUmbracoIndex)}"); + } var searcher = umbIndex.GetSearcher(); - // default to max 500 results - var count = skip == 0 && take == 0 ? 500 : skip + take; - ISearchResults results; if (culture == "*") { - //search everything - - results = searcher.Search(term, count); - } - else if (culture.IsNullOrWhiteSpace()) - { - //only search invariant - - var qry = searcher.CreateQuery().Field(UmbracoContentIndex.VariesByCultureFieldName, "n"); //must not vary by culture - qry = qry.And().ManagedQuery(term); - results = qry.Execute(count); + // Search everything + results = skip == 0 && take == 0 + ? searcher.Search(term) + : searcher.Search(term, skip + take); } else { - //search only the specified culture + IBooleanOperation query; + if (string.IsNullOrWhiteSpace(culture)) + { + // Only search invariant + query = searcher.CreateQuery() + .Field(UmbracoContentIndex.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 + query = searcher.CreateQuery() + .ManagedQuery(term, fields); + } - //get all index fields suffixed with the culture name supplied - var cultureFields = umbIndex.GetCultureAndInvariantFields(culture).ToArray(); - var qry = searcher.CreateQuery().ManagedQuery(term, cultureFields); - results = qry.Execute(count); + results = skip == 0 && take == 0 + ? query.Execute() + : query.Execute(skip + take); } totalRecords = results.TotalItemCount; - return new CultureContextualSearchResults(results.ToPublishedSearchResults(_publishedSnapshot.Content), _variationContextAccessor, culture); + return new CultureContextualSearchResults(results.Skip(skip).ToPublishedSearchResults(_publishedSnapshot.Content), _variationContextAccessor, culture); } /// @@ -244,10 +250,11 @@ namespace Umbraco.Web { var results = skip == 0 && take == 0 ? query.Execute() - : query.Execute(maxResults: skip + take); + : query.Execute(skip + take); totalRecords = results.TotalItemCount; - return results.ToPublishedSearchResults(_publishedSnapshot.Content); + + return results.Skip(skip).ToPublishedSearchResults(_publishedSnapshot.Content); } /// From 9b87b65537d92ca954db116f9948561fd55ea411 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Wed, 13 Nov 2019 14:39:11 +0100 Subject: [PATCH 2/8] Create Examine query that only searches content --- src/Umbraco.Web/PublishedContentQuery.cs | 40 ++++++++++-------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/src/Umbraco.Web/PublishedContentQuery.cs b/src/Umbraco.Web/PublishedContentQuery.cs index 72aec94e58..8f4242c180 100644 --- a/src/Umbraco.Web/PublishedContentQuery.cs +++ b/src/Umbraco.Web/PublishedContentQuery.cs @@ -201,39 +201,31 @@ namespace Umbraco.Web throw new InvalidOperationException($"No index found by name {indexName} or is not of type {typeof(IUmbracoIndex)}"); } - var searcher = umbIndex.GetSearcher(); + var query = umbIndex.GetSearcher().CreateQuery(IndexTypes.Content); - ISearchResults results; + IQueryExecutor queryExecutor; if (culture == "*") { // Search everything - results = skip == 0 && take == 0 - ? searcher.Search(term) - : searcher.Search(term, skip + take); + queryExecutor = query.ManagedQuery(term); + } + else if (string.IsNullOrWhiteSpace(culture)) + { + // Only search invariant + queryExecutor = query.Field(UmbracoContentIndex.VariesByCultureFieldName, "n") // Must not vary by culture + .And().ManagedQuery(term); } else { - IBooleanOperation query; - if (string.IsNullOrWhiteSpace(culture)) - { - // Only search invariant - query = searcher.CreateQuery() - .Field(UmbracoContentIndex.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 - query = searcher.CreateQuery() - .ManagedQuery(term, fields); - } - - results = skip == 0 && take == 0 - ? query.Execute() - : query.Execute(skip + take); + // 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); } + var results = skip == 0 && take == 0 + ? queryExecutor.Execute() + : queryExecutor.Execute(skip + take); + totalRecords = results.TotalItemCount; return new CultureContextualSearchResults(results.Skip(skip).ToPublishedSearchResults(_publishedSnapshot.Content), _variationContextAccessor, culture); From b64eb05e792abb9a3c44dda14f173541644deac1 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Wed, 13 Nov 2019 14:39:52 +0100 Subject: [PATCH 3/8] Update documentation of Search overloads --- src/Umbraco.Web/IPublishedContentQuery.cs | 48 +++++++++++++++-------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/src/Umbraco.Web/IPublishedContentQuery.cs b/src/Umbraco.Web/IPublishedContentQuery.cs index 8a8d678aba..351847da73 100644 --- a/src/Umbraco.Web/IPublishedContentQuery.cs +++ b/src/Umbraco.Web/IPublishedContentQuery.cs @@ -35,12 +35,15 @@ namespace Umbraco.Web /// /// Searches content. /// - /// Term to search. - /// Optional culture. - /// Optional index name. + /// The term to search. + /// The culture (defaults to a culture insensitive search). + /// The name of the index to search (defaults to ). + /// + /// The search results. + /// /// /// - /// When the is not specified or is *, all cultures are searched. + /// When the is not specified or is *, all cultures are searched. /// To search for only invariant documents and fields use null. /// When searching on a specific culture, all culture specific fields are searched for the provided culture and all invariant fields for all documents. /// @@ -51,15 +54,18 @@ namespace Umbraco.Web /// /// Searches content. /// - /// Term to search. - /// Numbers of items to skip. - /// Numbers of items to return. - /// Total number of matching items. - /// Optional culture. - /// Optional index name. + /// The term to search. + /// The amount of results to skip. + /// The amount of results to take/return. + /// The total amount of records. + /// The culture (defaults to a culture insensitive search). + /// The name of the index to search (defaults to ). + /// + /// The search results. + /// /// /// - /// When the is not specified or is *, all cultures are searched. + /// When the is not specified or is *, all cultures are searched. /// To search for only invariant documents and fields use null. /// When searching on a specific culture, all culture specific fields are searched for the provided culture and all invariant fields for all documents. /// @@ -68,18 +74,26 @@ namespace Umbraco.Web IEnumerable Search(string term, int skip, int take, out long totalRecords, string culture = "*", string indexName = null); /// - /// Executes the query and converts the results to PublishedSearchResult. + /// Executes the query and converts the results to using the . /// - /// - /// While enumerating results, the ambient culture is changed to be the searched culture. - /// + /// The query. + /// + /// The search results. + /// IEnumerable Search(IQueryExecutor query); /// - /// Executes the query and converts the results to PublishedSearchResult. + /// Executes the query and converts the results to using the . /// + /// The query. + /// The amount of results to skip. + /// The amount of results to take/return. + /// The total amount of records. + /// + /// The search results. + /// /// - /// While enumerating results, the ambient culture is changed to be the searched culture. + /// Make sure only content is searched (searcher.CreateQuery(IndexTypes.Content)), as this only returns content, but the could otherwise also include media records. /// IEnumerable Search(IQueryExecutor query, int skip, int take, out long totalRecords); } From 634f64851753f612b938475238aa687a0dc3c749 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Thu, 14 Nov 2019 09:20:18 +0100 Subject: [PATCH 4/8] Require Examine 1.0.2 --- build/NuSpecs/UmbracoCms.Web.nuspec | 2 +- src/Umbraco.Examine/Umbraco.Examine.csproj | 4 ++-- src/Umbraco.Tests/Umbraco.Tests.csproj | 2 +- src/Umbraco.Web.UI/Umbraco.Web.UI.csproj | 4 ++-- src/Umbraco.Web/Umbraco.Web.csproj | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/build/NuSpecs/UmbracoCms.Web.nuspec b/build/NuSpecs/UmbracoCms.Web.nuspec index c8374bc2f7..72b4df2ecb 100644 --- a/build/NuSpecs/UmbracoCms.Web.nuspec +++ b/build/NuSpecs/UmbracoCms.Web.nuspec @@ -28,7 +28,7 @@ - + diff --git a/src/Umbraco.Examine/Umbraco.Examine.csproj b/src/Umbraco.Examine/Umbraco.Examine.csproj index db623ecddd..0e0ee62139 100644 --- a/src/Umbraco.Examine/Umbraco.Examine.csproj +++ b/src/Umbraco.Examine/Umbraco.Examine.csproj @@ -49,7 +49,7 @@ - + 1.0.0-beta2-19324-01 runtime; build; native; contentfiles; analyzers; buildtransitive @@ -112,4 +112,4 @@ - + \ No newline at end of file diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 39826fcc38..47542484d8 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -79,7 +79,7 @@ - + 1.8.14 diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index 6e766f578e..70039488ed 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -87,7 +87,7 @@ - + @@ -431,4 +431,4 @@ - + \ No newline at end of file diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 56feaf52e7..9947a5e1de 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -63,7 +63,7 @@ - + 2.7.0.100 @@ -1279,4 +1279,4 @@ - + \ No newline at end of file From c206382fa105ebe4140fc724c5c987a4bf5bba22 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Thu, 14 Nov 2019 09:23:18 +0100 Subject: [PATCH 5/8] Use snapshot to convert search results into content, media or members --- src/Umbraco.Web/ExamineExtensions.cs | 83 ++++++++++++++++++++--- src/Umbraco.Web/IPublishedContentQuery.cs | 7 +- src/Umbraco.Web/PublishedContentQuery.cs | 2 +- 3 files changed, 76 insertions(+), 16 deletions(-) diff --git a/src/Umbraco.Web/ExamineExtensions.cs b/src/Umbraco.Web/ExamineExtensions.cs index 9a9fa98d95..8fead91194 100644 --- a/src/Umbraco.Web/ExamineExtensions.cs +++ b/src/Umbraco.Web/ExamineExtensions.cs @@ -2,32 +2,95 @@ using System.Collections.Generic; using System.Linq; using Examine; -using Umbraco.Core; +using Examine.LuceneEngine.Providers; using Umbraco.Core.Models.PublishedContent; +using Umbraco.Examine; using Umbraco.Web.PublishedCache; namespace Umbraco.Web { /// - /// Extension methods for Examine + /// Extension methods for Examine. /// public static class ExamineExtensions { + /// + /// Creates an containing all content from the . + /// + /// The search results. + /// The cache to fetch the content from. + /// + /// An containing all content. + /// + /// cache + /// + /// Search results are skipped if it can't be fetched from the by its integer id. + /// public static IEnumerable ToPublishedSearchResults(this IEnumerable results, IPublishedCache cache) { - var list = new List(); + if (cache == null) throw new ArgumentNullException(nameof(cache)); + + var publishedSearchResults = new List(); foreach (var result in results.OrderByDescending(x => x.Score)) { - if (!int.TryParse(result.Id, out var intId)) continue; //invalid - var content = cache.GetById(intId); - if (content == null) continue; // skip if this doesn't exist in the cache - - list.Add(new PublishedSearchResult(content, result.Score)); - + if (int.TryParse(result.Id, out var contentId) && + cache.GetById(contentId) is IPublishedContent content) + { + publishedSearchResults.Add(new PublishedSearchResult(content, result.Score)); + } } - return list; + return publishedSearchResults; + } + + /// + /// Creates an containing all content, media or members from the . + /// + /// The search results. + /// The snapshot. + /// + /// An containing all content, media or members. + /// + /// snapshot + /// + /// Search results are skipped if it can't be fetched from the respective cache by its integer id. + /// + public static IEnumerable ToPublishedSearchResults(this IEnumerable results, IPublishedSnapshot snapshot) + { + if (snapshot == null) throw new ArgumentNullException(nameof(snapshot)); + + var publishedSearchResults = new List(); + + foreach (var result in results.OrderByDescending(x => x.Score)) + { + if (int.TryParse(result.Id, out var contentId) && + result.Values.TryGetValue(LuceneIndex.CategoryFieldName, out var indexType)) + { + IPublishedContent content; + switch (indexType) + { + case IndexTypes.Content: + content = snapshot.Content.GetById(contentId); + break; + case IndexTypes.Media: + content = snapshot.Media.GetById(contentId); + break; + case IndexTypes.Member: + content = snapshot.Members.GetById(contentId); + break; + default: + continue; + } + + if (content != null) + { + publishedSearchResults.Add(new PublishedSearchResult(content, result.Score)); + } + } + } + + return publishedSearchResults; } } } diff --git a/src/Umbraco.Web/IPublishedContentQuery.cs b/src/Umbraco.Web/IPublishedContentQuery.cs index 351847da73..c4d829968f 100644 --- a/src/Umbraco.Web/IPublishedContentQuery.cs +++ b/src/Umbraco.Web/IPublishedContentQuery.cs @@ -74,7 +74,7 @@ namespace Umbraco.Web IEnumerable Search(string term, int skip, int take, out long totalRecords, string culture = "*", string indexName = null); /// - /// Executes the query and converts the results to using the . + /// Executes the query and converts the results to . /// /// The query. /// @@ -83,7 +83,7 @@ namespace Umbraco.Web IEnumerable Search(IQueryExecutor query); /// - /// Executes the query and converts the results to using the . + /// Executes the query and converts the results to . /// /// The query. /// The amount of results to skip. @@ -92,9 +92,6 @@ namespace Umbraco.Web /// /// The search results. /// - /// - /// Make sure only content is searched (searcher.CreateQuery(IndexTypes.Content)), as this only returns content, but the could otherwise also include media records. - /// IEnumerable Search(IQueryExecutor query, int skip, int take, out long totalRecords); } } diff --git a/src/Umbraco.Web/PublishedContentQuery.cs b/src/Umbraco.Web/PublishedContentQuery.cs index 8f4242c180..833df9971b 100644 --- a/src/Umbraco.Web/PublishedContentQuery.cs +++ b/src/Umbraco.Web/PublishedContentQuery.cs @@ -246,7 +246,7 @@ namespace Umbraco.Web totalRecords = results.TotalItemCount; - return results.Skip(skip).ToPublishedSearchResults(_publishedSnapshot.Content); + return results.Skip(skip).ToPublishedSearchResults(_publishedSnapshot); } /// From c355e8f5f0938b2ef9eb5a7247df43728a7f61f0 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Sun, 5 Jan 2020 22:26:13 +0100 Subject: [PATCH 6/8] Use ExternalIndexName as default parameter value --- src/Umbraco.Web/IPublishedContentQuery.cs | 4 ++-- src/Umbraco.Web/PublishedContentQuery.cs | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Umbraco.Web/IPublishedContentQuery.cs b/src/Umbraco.Web/IPublishedContentQuery.cs index c4d829968f..7066475dc9 100644 --- a/src/Umbraco.Web/IPublishedContentQuery.cs +++ b/src/Umbraco.Web/IPublishedContentQuery.cs @@ -49,7 +49,7 @@ namespace Umbraco.Web /// /// While enumerating results, the ambient culture is changed to be the searched culture. /// - IEnumerable Search(string term, string culture = "*", string indexName = null); + IEnumerable Search(string term, string culture = "*", string indexName = Constants.UmbracoIndexes.ExternalIndexName); /// /// Searches content. @@ -71,7 +71,7 @@ namespace Umbraco.Web /// /// While enumerating results, the ambient culture is changed to be the searched culture. /// - IEnumerable Search(string term, int skip, int take, out long totalRecords, string culture = "*", string indexName = null); + IEnumerable Search(string term, int skip, int take, out long totalRecords, string culture = "*", string indexName = Constants.UmbracoIndexes.ExternalIndexName); /// /// Executes the query and converts the results to . diff --git a/src/Umbraco.Web/PublishedContentQuery.cs b/src/Umbraco.Web/PublishedContentQuery.cs index 833df9971b..44a3208956 100644 --- a/src/Umbraco.Web/PublishedContentQuery.cs +++ b/src/Umbraco.Web/PublishedContentQuery.cs @@ -183,13 +183,13 @@ namespace Umbraco.Web #region Search /// - public IEnumerable Search(string term, string culture = "*", string indexName = null) + public IEnumerable Search(string term, string culture = "*", string indexName = Constants.UmbracoIndexes.ExternalIndexName) { return Search(term, 0, 0, out _, culture, indexName); } /// - public IEnumerable Search(string term, int skip, int take, out long totalRecords, string culture = "*", string indexName = null) + public IEnumerable Search(string term, int skip, int take, out long totalRecords, string culture = "*", string indexName = Constants.UmbracoIndexes.ExternalIndexName) { if (string.IsNullOrEmpty(indexName)) { @@ -319,9 +319,6 @@ namespace Umbraco.Web } } - - - #endregion } } From 3bd7718c38e03212a7d9061be7b40e1061611786 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Sun, 5 Jan 2020 22:27:56 +0100 Subject: [PATCH 7/8] Remove ordering by score when converting to PublishedSearchResult --- src/Umbraco.Web/ExamineExtensions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web/ExamineExtensions.cs b/src/Umbraco.Web/ExamineExtensions.cs index 8fead91194..421993f8fd 100644 --- a/src/Umbraco.Web/ExamineExtensions.cs +++ b/src/Umbraco.Web/ExamineExtensions.cs @@ -32,7 +32,7 @@ namespace Umbraco.Web var publishedSearchResults = new List(); - foreach (var result in results.OrderByDescending(x => x.Score)) + foreach (var result in results) { if (int.TryParse(result.Id, out var contentId) && cache.GetById(contentId) is IPublishedContent content) @@ -62,7 +62,7 @@ namespace Umbraco.Web var publishedSearchResults = new List(); - foreach (var result in results.OrderByDescending(x => x.Score)) + foreach (var result in results) { if (int.TryParse(result.Id, out var contentId) && result.Values.TryGetValue(LuceneIndex.CategoryFieldName, out var indexType)) From fa23b50d1ee3248499591ffb781d2486d6fa06fe Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Sun, 5 Jan 2020 22:55:55 +0100 Subject: [PATCH 8/8] Add skip and take argument validation --- src/Umbraco.Web/PublishedContentQuery.cs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/Umbraco.Web/PublishedContentQuery.cs b/src/Umbraco.Web/PublishedContentQuery.cs index 44a3208956..d697898f33 100644 --- a/src/Umbraco.Web/PublishedContentQuery.cs +++ b/src/Umbraco.Web/PublishedContentQuery.cs @@ -191,6 +191,16 @@ namespace Umbraco.Web /// public IEnumerable Search(string term, int skip, int take, out long totalRecords, string culture = "*", string indexName = Constants.UmbracoIndexes.ExternalIndexName) { + if (skip < 0) + { + 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."); + } + if (string.IsNullOrEmpty(indexName)) { indexName = Constants.UmbracoIndexes.ExternalIndexName; @@ -240,6 +250,16 @@ namespace Umbraco.Web /// 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."); + } + + if (take < 0) + { + throw new ArgumentOutOfRangeException(nameof(take), take, "The value must be greater than or equal to zero."); + } + var results = skip == 0 && take == 0 ? query.Execute() : query.Execute(skip + take);