From 5148f34b3296b694b84e666ac778e655b5d69d67 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 11 Dec 2018 15:42:32 +1100 Subject: [PATCH 1/5] Fixes ISearchableTree lifetime registration, fixes routing that clears required query strings, fixes lucene search string, gets URls in search results working with default lang/variants --- src/Umbraco.Core/Constants-Examine.cs | 24 ---------- src/Umbraco.Core/Umbraco.Core.csproj | 1 - .../PublishedMediaCacheTests.cs | 2 - src/Umbraco.Web.UI.Client/src/init.js | 46 +++++++++++++------ .../DictionaryPublishedContent.cs | 2 +- .../XmlPublishedCache/PublishedMediaCache.cs | 4 +- src/Umbraco.Web/PublishedContentExtensions.cs | 8 ++-- src/Umbraco.Web/PublishedContentQuery.cs | 6 +-- .../Search/SearchableTreeCollectionBuilder.cs | 3 ++ src/Umbraco.Web/Search/UmbracoTreeSearcher.cs | 42 +++++++++++------ .../Trees/ContentTreeController.cs | 9 ++-- 11 files changed, 78 insertions(+), 69 deletions(-) delete mode 100644 src/Umbraco.Core/Constants-Examine.cs diff --git a/src/Umbraco.Core/Constants-Examine.cs b/src/Umbraco.Core/Constants-Examine.cs deleted file mode 100644 index ddc3500066..0000000000 --- a/src/Umbraco.Core/Constants-Examine.cs +++ /dev/null @@ -1,24 +0,0 @@ -namespace Umbraco.Core -{ - public static partial class Constants - { - public static class Examine - { - /// - /// The alias of the internal member indexer - /// - public const string InternalMemberIndexer = "InternalMemberIndexer"; - - /// - /// The alias of the internal content indexer - /// - public const string InternalIndexer = "InternalIndexer"; - - /// - /// The alias of the external content indexer - /// - public const string ExternalIndexer = "ExternalIndexer"; - - } - } -} diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index a8361a237a..a015d8d4f8 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -304,7 +304,6 @@ - diff --git a/src/Umbraco.Tests/Cache/PublishedCache/PublishedMediaCacheTests.cs b/src/Umbraco.Tests/Cache/PublishedCache/PublishedMediaCacheTests.cs index 64194ebb47..aeda2eaca2 100644 --- a/src/Umbraco.Tests/Cache/PublishedCache/PublishedMediaCacheTests.cs +++ b/src/Umbraco.Tests/Cache/PublishedCache/PublishedMediaCacheTests.cs @@ -111,7 +111,6 @@ namespace Umbraco.Tests.Cache.PublishedCache } [TestCase("id")] - [TestCase("nodeId")] [TestCase("__NodeId")] public void DictionaryDocument_Id_Keys(string key) { @@ -128,7 +127,6 @@ namespace Umbraco.Tests.Cache.PublishedCache } [TestCase("nodeName")] - [TestCase("__nodeName")] public void DictionaryDocument_NodeName_Keys(string key) { var dicDoc = GetDictionaryDocument(nodeNameKey: key); diff --git a/src/Umbraco.Web.UI.Client/src/init.js b/src/Umbraco.Web.UI.Client/src/init.js index 3f44638096..eaa2fe7b31 100644 --- a/src/Umbraco.Web.UI.Client/src/init.js +++ b/src/Umbraco.Web.UI.Client/src/init.js @@ -47,7 +47,17 @@ app.run(['userService', '$q', '$log', '$rootScope', '$route', '$location', 'urlH /** execute code on each successful route */ $rootScope.$on('$routeChangeSuccess', function (event, current, previous) { - currentRouteParams = angular.copy(current.params); //store this so we can reference it in $routeUpdate + var toRetain = currentRouteParams ? navigationService.retainQueryStrings(currentRouteParams, current.params) : null; + + //if toRetain is not null it means that there are missing query strings and we need to update the current params + if (toRetain) { + $route.updateParams(toRetain); + currentRouteParams = toRetain; + } + else { + currentRouteParams = angular.copy(current.params); + } + var deployConfig = Umbraco.Sys.ServerVariables.deploy; var deployEnv, deployEnvTitle; @@ -122,25 +132,33 @@ app.run(['userService', '$q', '$log', '$rootScope', '$route', '$location', 'urlH $route.reload(); } else { - + + var toRetain = navigationService.retainQueryStrings(currentRouteParams, next.params); + + //if toRetain is not null it means that there are missing query strings and we need to update the current params + if (toRetain) { + $route.updateParams(toRetain); + } + //check if the location being changed is only due to global/state query strings which means the location change //isn't actually going to cause a route change. - if (navigationService.isRouteChangingNavigation(currentRouteParams, next.params)) { - //The location change will cause a route change. We need to ensure that the global/state - //query strings have not been stripped out. If they have, we'll re-add them and re-route. + if (!toRetain && navigationService.isRouteChangingNavigation(currentRouteParams, next.params)) { + + //The location change will cause a route change, continue the route if the query strings haven't been updated. + $route.reload(); - var toRetain = navigationService.retainQueryStrings(currentRouteParams, next.params); - if (toRetain) { - $route.updateParams(toRetain); - } - else { - //continue the route - $route.reload(); - } } else { + //navigation is not changing but we should update the currentRouteParams to include all current parameters - currentRouteParams = angular.copy(next.params); + + if (toRetain) { + currentRouteParams = toRetain; + } + else { + currentRouteParams = angular.copy(next.params); + } + } } }); diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/DictionaryPublishedContent.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/DictionaryPublishedContent.cs index 7a8ce65ae3..c9d3c79ff5 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/DictionaryPublishedContent.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/DictionaryPublishedContent.cs @@ -55,7 +55,7 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache ValidateAndSetProperty(valueDictionary, val => _key = Guid.Parse(val), "key"); //ValidateAndSetProperty(valueDictionary, val => _templateId = int.Parse(val), "template", "templateId"); ValidateAndSetProperty(valueDictionary, val => _sortOrder = Int32.Parse(val), "sortOrder"); - ValidateAndSetProperty(valueDictionary, val => _name = val, "nodeName", "__nodeName"); + ValidateAndSetProperty(valueDictionary, val => _name = val, "nodeName"); ValidateAndSetProperty(valueDictionary, val => _urlName = val, "urlName"); ValidateAndSetProperty(valueDictionary, val => _documentTypeAlias = val, "nodeTypeAlias", LuceneIndex.ItemTypeFieldName); ValidateAndSetProperty(valueDictionary, val => _documentTypeId = Int32.Parse(val), "nodeType"); diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedMediaCache.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedMediaCache.cs index ac6b425e27..f203d5d2c9 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedMediaCache.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedMediaCache.cs @@ -240,9 +240,9 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache try { - if (eMgr.TryGetIndex(Constants.Examine.InternalIndexer, out var index)) + if (eMgr.TryGetIndex(Constants.UmbracoIndexes.InternalIndexName, out var index)) return index.GetSearcher(); - throw new InvalidOperationException($"No index found by name {Constants.Examine.InternalIndexer}"); + throw new InvalidOperationException($"No index found by name {Constants.UmbracoIndexes.InternalIndexName}"); } catch (FileNotFoundException) { diff --git a/src/Umbraco.Web/PublishedContentExtensions.cs b/src/Umbraco.Web/PublishedContentExtensions.cs index 2bc0d7be3f..e3291f9ad5 100644 --- a/src/Umbraco.Web/PublishedContentExtensions.cs +++ b/src/Umbraco.Web/PublishedContentExtensions.cs @@ -286,7 +286,7 @@ namespace Umbraco.Web { //fixme: pass in the IExamineManager - indexName = string.IsNullOrEmpty(indexName) ? Constants.Examine.ExternalIndexer : indexName; + indexName = string.IsNullOrEmpty(indexName) ? Constants.UmbracoIndexes.ExternalIndexName : indexName; if (!ExamineManager.Instance.TryGetIndex(indexName, out var index)) throw new InvalidOperationException("No index found with name " + indexName); @@ -306,7 +306,7 @@ namespace Umbraco.Web { //fixme: pass in the IExamineManager - indexName = string.IsNullOrEmpty(indexName) ? Constants.Examine.ExternalIndexer : indexName; + indexName = string.IsNullOrEmpty(indexName) ? Constants.UmbracoIndexes.ExternalIndexName : indexName; if (!ExamineManager.Instance.TryGetIndex(indexName, out var index)) throw new InvalidOperationException("No index found with name " + indexName); @@ -328,8 +328,8 @@ namespace Umbraco.Web if (searchProvider == null) { - if (!ExamineManager.Instance.TryGetIndex(Constants.Examine.ExternalIndexer, out var index)) - throw new InvalidOperationException("No index found with name " + Constants.Examine.ExternalIndexer); + if (!ExamineManager.Instance.TryGetIndex(Constants.UmbracoIndexes.ExternalIndexName, out var index)) + throw new InvalidOperationException("No index found with name " + Constants.UmbracoIndexes.ExternalIndexName); searchProvider = index.GetSearcher(); } var results = searchProvider.Search(criteria); diff --git a/src/Umbraco.Web/PublishedContentQuery.cs b/src/Umbraco.Web/PublishedContentQuery.cs index c2fb84a3da..67b54330c5 100644 --- a/src/Umbraco.Web/PublishedContentQuery.cs +++ b/src/Umbraco.Web/PublishedContentQuery.cs @@ -234,7 +234,7 @@ namespace Umbraco.Web if (_query != null) return _query.Search(skip, take, out totalRecords, term, useWildCards, indexName); indexName = string.IsNullOrEmpty(indexName) - ? Constants.Examine.ExternalIndexer + ? Constants.UmbracoIndexes.ExternalIndexName : indexName; if (!ExamineManager.Instance.TryGetIndex(indexName, out var index)) @@ -264,8 +264,8 @@ namespace Umbraco.Web //fixme: inject IExamineManager if (searcher == null) { - if (!ExamineManager.Instance.TryGetIndex(Constants.Examine.ExternalIndexer, out var index)) - throw new InvalidOperationException($"No index found by name {Constants.Examine.ExternalIndexer}"); + if (!ExamineManager.Instance.TryGetIndex(Constants.UmbracoIndexes.ExternalIndexName, out var index)) + throw new InvalidOperationException($"No index found by name {Constants.UmbracoIndexes.ExternalIndexName}"); searcher = index.GetSearcher(); } diff --git a/src/Umbraco.Web/Search/SearchableTreeCollectionBuilder.cs b/src/Umbraco.Web/Search/SearchableTreeCollectionBuilder.cs index ae83cc5eab..22db27b1fb 100644 --- a/src/Umbraco.Web/Search/SearchableTreeCollectionBuilder.cs +++ b/src/Umbraco.Web/Search/SearchableTreeCollectionBuilder.cs @@ -21,5 +21,8 @@ namespace Umbraco.Web.Search { return new SearchableTreeCollection(CreateItems(), _treeService); } + + //per request because generally an instance of ISearchableTree is a controller + protected override ILifetime CollectionLifetime => new PerRequestLifeTime(); } } diff --git a/src/Umbraco.Web/Search/UmbracoTreeSearcher.cs b/src/Umbraco.Web/Search/UmbracoTreeSearcher.cs index 9aab30edae..af2db4f7ac 100644 --- a/src/Umbraco.Web/Search/UmbracoTreeSearcher.cs +++ b/src/Umbraco.Web/Search/UmbracoTreeSearcher.cs @@ -10,6 +10,7 @@ using Umbraco.Core; using Umbraco.Core.Composing; using Umbraco.Core.Models; using Umbraco.Core.Services; +using Umbraco.Examine; using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Trees; using SearchResult = Examine.SearchResult; @@ -23,11 +24,13 @@ namespace Umbraco.Web.Search { private readonly IExamineManager _examineManager; private readonly UmbracoHelper _umbracoHelper; + private readonly ILocalizationService _languageService; - public UmbracoTreeSearcher(IExamineManager examineManager, UmbracoHelper umbracoHelper) + public UmbracoTreeSearcher(IExamineManager examineManager, UmbracoHelper umbracoHelper, ILocalizationService languageService) { _examineManager = examineManager ?? throw new ArgumentNullException(nameof(examineManager)); _umbracoHelper = umbracoHelper ?? throw new ArgumentNullException(nameof(umbracoHelper)); + _languageService = languageService; } /// @@ -51,7 +54,7 @@ namespace Umbraco.Web.Search var sb = new StringBuilder(); string type; - var indexName = Constants.Examine.InternalIndexer; + var indexName = Constants.UmbracoIndexes.InternalIndexName; var fields = new[] { "id", "__NodeId" }; var umbracoContext = _umbracoHelper.UmbracoContext; @@ -60,7 +63,7 @@ namespace Umbraco.Web.Search switch (entityType) { case UmbracoEntityTypes.Member: - indexName = Constants.Examine.InternalMemberIndexer; + indexName = Constants.UmbracoIndexes.MembersIndexName; type = "member"; fields = new[] { "id", "__NodeId", "email", "loginName" }; if (searchFrom != null && searchFrom != Constants.Conventions.MemberTypes.AllMembersListId && searchFrom.Trim() != "-1") @@ -90,8 +93,8 @@ namespace Umbraco.Web.Search var internalSearcher = index.GetSearcher(); //build a lucene query: - // the __nodeName will be boosted 10x without wildcards - // then __nodeName will be matched normally with wildcards + // the nodeName will be boosted 10x without wildcards + // then nodeName will be matched normally with wildcards // the rest will be normal without wildcards @@ -120,7 +123,7 @@ namespace Umbraco.Web.Search query = string.Format("{0}{1}{0}", "\"", query); //node name exactly boost x 10 - sb.Append("+(__nodeName: ("); + sb.Append("+(nodeName: ("); sb.Append(query.ToLower()); sb.Append(")^10.0 "); @@ -155,14 +158,14 @@ namespace Umbraco.Web.Search var querywords = query.Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries); //node name exactly boost x 10 - sb.Append("+(__nodeName:"); + sb.Append("+(nodeName:"); sb.Append("\""); sb.Append(query.ToLower()); sb.Append("\""); sb.Append("^10.0 "); //node name normally with wildcards - sb.Append(" __nodeName:"); + sb.Append(" nodeName:"); sb.Append("("); foreach (var w in querywords) { @@ -333,17 +336,28 @@ namespace Umbraco.Web.Search /// private IEnumerable ContentFromSearchResults(IEnumerable results) { - var mapped = Mapper.Map>(results).ToArray(); - //add additional data - foreach (var m in mapped) + var defaultLang = _languageService.GetDefaultLanguageIsoCode(); + + foreach (var result in results) { - var intId = m.Id.TryConvertTo(); + var entity = Mapper.Map(result); + + var intId = entity.Id.TryConvertTo(); if (intId.Success) { - m.AdditionalData["Url"] = _umbracoHelper.Url(intId.Result); + //TODO: Here we need to figure out how to get the URL based on variant, etc... + if (result.Values.TryGetValue(UmbracoContentIndex.VariesByCultureFieldName, out var varies) && varies == "1") + { + entity.AdditionalData["Url"] = _umbracoHelper.Url(intId.Result, defaultLang); + } + else + { + entity.AdditionalData["Url"] = _umbracoHelper.Url(intId.Result); + } } + + yield return entity; } - return mapped; } } diff --git a/src/Umbraco.Web/Trees/ContentTreeController.cs b/src/Umbraco.Web/Trees/ContentTreeController.cs index d2b94c815b..bb38b8c578 100644 --- a/src/Umbraco.Web/Trees/ContentTreeController.cs +++ b/src/Umbraco.Web/Trees/ContentTreeController.cs @@ -36,10 +36,12 @@ namespace Umbraco.Web.Trees public class ContentTreeController : ContentTreeControllerBase, ISearchableTree { private readonly UmbracoTreeSearcher _treeSearcher; + private readonly ActionCollection _actions; - public ContentTreeController(UmbracoTreeSearcher treeSearcher) + public ContentTreeController(UmbracoTreeSearcher treeSearcher, ActionCollection actions) { _treeSearcher = treeSearcher; + _actions = actions; } protected override int RecycleBinId => Constants.System.RecycleBinContent; @@ -127,7 +129,7 @@ namespace Umbraco.Web.Trees // we need to get the default permissions as you can't set permissions on the very root node var permission = Services.UserService.GetPermissions(Security.CurrentUser, Constants.System.Root).First(); - var nodeActions = Current.Actions.FromEntityPermission(permission) + var nodeActions = _actions.FromEntityPermission(permission) .Select(x => new MenuItem(x)); //these two are the standard items @@ -313,8 +315,7 @@ namespace Umbraco.Web.Trees private void AddActionNode(IUmbracoEntity item, MenuItemCollection menu, bool hasSeparator = false, bool convert = false, bool opensDialog = false) where TAction : IAction { - //fixme: Inject - var menuItem = menu.Items.Add(Services.TextService.Localize("actions", Current.Actions.GetAction().Alias), hasSeparator); + var menuItem = menu.Items.Add(Services.TextService.Localize("actions", _actions.GetAction().Alias), hasSeparator); if (convert) menuItem.ConvertLegacyMenuItem(item, "content", "content"); menuItem.OpensDialog = opensDialog; } From eb3841fc0a5332f0bd73935cae4fffa5286a803c Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 11 Dec 2018 17:51:47 +1100 Subject: [PATCH 2/5] Searches on variant nodeName, orders ISearchableTree results by the corresponding tree SortOrder, fixes assembly scanning ISearchableTree --- src/Umbraco.Examine/ExamineExtensions.cs | 26 +++ .../Properties/AssemblyInfo.cs | 1 + .../Editors/ExamineManagementController.cs | 25 +-- .../Runtime/WebRuntimeComponent.cs | 6 +- src/Umbraco.Web/Search/ExamineComponent.cs | 3 + .../Search/SearchableTreeCollection.cs | 10 +- src/Umbraco.Web/Search/UmbracoTreeSearcher.cs | 196 ++++++++++++------ 7 files changed, 177 insertions(+), 90 deletions(-) diff --git a/src/Umbraco.Examine/ExamineExtensions.cs b/src/Umbraco.Examine/ExamineExtensions.cs index 3681979267..9fdf5e9123 100644 --- a/src/Umbraco.Examine/ExamineExtensions.cs +++ b/src/Umbraco.Examine/ExamineExtensions.cs @@ -1,8 +1,11 @@ using System; using Examine.LuceneEngine.Providers; +using Lucene.Net.Analysis; using Lucene.Net.Index; +using Lucene.Net.QueryParsers; using Lucene.Net.Search; using Lucene.Net.Store; +using Version = Lucene.Net.Util.Version; namespace Umbraco.Examine { @@ -11,6 +14,29 @@ namespace Umbraco.Examine /// internal static class ExamineExtensions { + public static bool TryParseLuceneQuery(string query) + { + //TODO: I'd assume there would be a more strict way to parse the query but not that i can find yet, for now we'll + // also do this rudimentary check + if (!query.Contains(":")) + return false; + + try + { + //This will pass with a plain old string without any fields, need to figure out a way to have it properly parse + var parsed = new QueryParser(Version.LUCENE_30, "nodeName", new KeywordAnalyzer()).Parse(query); + return true; + } + catch (ParseException) + { + return false; + } + catch (Exception) + { + return false; + } + } + /// /// Checks if the index can be read/opened /// diff --git a/src/Umbraco.Examine/Properties/AssemblyInfo.cs b/src/Umbraco.Examine/Properties/AssemblyInfo.cs index 6713111968..5c42a236f4 100644 --- a/src/Umbraco.Examine/Properties/AssemblyInfo.cs +++ b/src/Umbraco.Examine/Properties/AssemblyInfo.cs @@ -8,6 +8,7 @@ using System.Runtime.CompilerServices; // Umbraco Cms [assembly: InternalsVisibleTo("Umbraco.Tests")] +[assembly: InternalsVisibleTo("Umbraco.Web")] // code analysis // IDE1006 is broken, wants _value syntax for consts, etc - and it's even confusing ppl at MS, kill it diff --git a/src/Umbraco.Web/Editors/ExamineManagementController.cs b/src/Umbraco.Web/Editors/ExamineManagementController.cs index 67209c91bd..2f96ee3d45 100644 --- a/src/Umbraco.Web/Editors/ExamineManagementController.cs +++ b/src/Umbraco.Web/Editors/ExamineManagementController.cs @@ -73,7 +73,7 @@ namespace Umbraco.Web.Editors if (!msg.IsSuccessStatusCode) throw new HttpResponseException(msg); - var results = TryParseLuceneQuery(query) + var results = Examine.ExamineExtensions.TryParseLuceneQuery(query) ? searcher.Search(searcher.CreateCriteria().RawQuery(query), maxResults: pageSize * (pageIndex + 1)) : searcher.Search(query, true, maxResults: pageSize * (pageIndex + 1)); @@ -92,28 +92,7 @@ namespace Umbraco.Web.Editors }; } - private bool TryParseLuceneQuery(string query) - { - //TODO: I'd assume there would be a more strict way to parse the query but not that i can find yet, for now we'll - // also do this rudimentary check - if (!query.Contains(":")) - return false; - - try - { - //This will pass with a plain old string without any fields, need to figure out a way to have it properly parse - var parsed = new QueryParser(Version.LUCENE_30, "nodeName", new KeywordAnalyzer()).Parse(query); - return true; - } - catch (ParseException) - { - return false; - } - catch (Exception) - { - return false; - } - } + /// /// Check if the index has been rebuilt diff --git a/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs b/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs index 2e1c934bc0..ccd9a7ef7d 100644 --- a/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs +++ b/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs @@ -127,8 +127,12 @@ namespace Umbraco.Web.Runtime composition.Container.RegisterUmbracoControllers(typeLoader, GetType().Assembly); composition.Container.EnableWebApi(GlobalConfiguration.Configuration); + //we aren't scanning for ISearchableTree since that is not IDiscoverable, instead we'll just filter what we've + //already scanned since all of our ISearchableTree is of type UmbracoApiController and in most cases any developers' + //own trees they want searched will also be of type UmbracoApiController. If a developer wants to replace one of ours + //then they will have to manually register/replace. composition.Container.RegisterCollectionBuilder() - .Add(() => typeLoader.GetTypes()); // fixme which searchable trees?! + .Add(() => typeLoader.GetTypes().Where(x => x.Implements())); composition.Container.Register(new PerRequestLifeTime()); diff --git a/src/Umbraco.Web/Search/ExamineComponent.cs b/src/Umbraco.Web/Search/ExamineComponent.cs index d8c1016c3e..65264d0308 100644 --- a/src/Umbraco.Web/Search/ExamineComponent.cs +++ b/src/Umbraco.Web/Search/ExamineComponent.cs @@ -26,6 +26,8 @@ using Examine.LuceneEngine.Directories; using LightInject; using Umbraco.Core.Composing; using Umbraco.Core.Strings; +using Umbraco.Web.Models.ContentEditing; +using Umbraco.Web.Trees; namespace Umbraco.Web.Search { @@ -53,6 +55,7 @@ namespace Umbraco.Web.Search // but greater that SafeXmlReaderWriter priority which is 60 private const int EnlistPriority = 80; + public override void Compose(Composition composition) { base.Compose(composition); diff --git a/src/Umbraco.Web/Search/SearchableTreeCollection.cs b/src/Umbraco.Web/Search/SearchableTreeCollection.cs index 86f4494353..81ee0a2898 100644 --- a/src/Umbraco.Web/Search/SearchableTreeCollection.cs +++ b/src/Umbraco.Web/Search/SearchableTreeCollection.cs @@ -19,15 +19,17 @@ namespace Umbraco.Web.Search private Dictionary CreateDictionary(IApplicationTreeService treeService) { - var appTrees = treeService.GetAll().ToArray(); + var appTrees = treeService.GetAll() + .OrderBy(x => x.SortOrder) + .ToArray(); var dictionary = new Dictionary(); var searchableTrees = this.ToArray(); - foreach (var searchableTree in searchableTrees) + foreach (var appTree in appTrees) { - var found = appTrees.FirstOrDefault(x => x.Alias == searchableTree.TreeAlias); + var found = searchableTrees.FirstOrDefault(x => x.TreeAlias == appTree.Alias); if (found != null) { - dictionary[searchableTree.TreeAlias] = new SearchableApplicationTree(found.ApplicationAlias, found.Alias, searchableTree); + dictionary[found.TreeAlias] = new SearchableApplicationTree(appTree.ApplicationAlias, appTree.Alias, found); } } return dictionary; diff --git a/src/Umbraco.Web/Search/UmbracoTreeSearcher.cs b/src/Umbraco.Web/Search/UmbracoTreeSearcher.cs index af2db4f7ac..8f977aab77 100644 --- a/src/Umbraco.Web/Search/UmbracoTreeSearcher.cs +++ b/src/Umbraco.Web/Search/UmbracoTreeSearcher.cs @@ -25,12 +25,17 @@ namespace Umbraco.Web.Search private readonly IExamineManager _examineManager; private readonly UmbracoHelper _umbracoHelper; private readonly ILocalizationService _languageService; + private readonly IEntityService _entityService; - public UmbracoTreeSearcher(IExamineManager examineManager, UmbracoHelper umbracoHelper, ILocalizationService languageService) + public UmbracoTreeSearcher(IExamineManager examineManager, + UmbracoHelper umbracoHelper, + ILocalizationService languageService, + IEntityService entityService) { _examineManager = examineManager ?? throw new ArgumentNullException(nameof(examineManager)); _umbracoHelper = umbracoHelper ?? throw new ArgumentNullException(nameof(umbracoHelper)); _languageService = languageService; + _entityService = entityService; } /// @@ -59,7 +64,13 @@ namespace Umbraco.Web.Search var umbracoContext = _umbracoHelper.UmbracoContext; - //TODO: WE should really just allow passing in a lucene raw query + //TODO: WE should try to allow passing in a lucene raw query, however we will still need to do some manual string + // manipulation for things like start paths, member types, etc... + //if (Examine.ExamineExtensions.TryParseLuceneQuery(query)) + //{ + + //} + switch (entityType) { case UmbracoEntityTypes.Member: @@ -75,13 +86,13 @@ namespace Umbraco.Web.Search break; case UmbracoEntityTypes.Media: type = "media"; - var allMediaStartNodes = umbracoContext.Security.CurrentUser.CalculateMediaStartNodeIds(Current.Services.EntityService); - AppendPath(sb, UmbracoObjectTypes.Media, allMediaStartNodes, searchFrom, Current.Services.EntityService); + var allMediaStartNodes = umbracoContext.Security.CurrentUser.CalculateMediaStartNodeIds(_entityService); + AppendPath(sb, UmbracoObjectTypes.Media, allMediaStartNodes, searchFrom, _entityService); break; case UmbracoEntityTypes.Document: type = "content"; - var allContentStartNodes = umbracoContext.Security.CurrentUser.CalculateContentStartNodeIds(Current.Services.EntityService); - AppendPath(sb, UmbracoObjectTypes.Document, allContentStartNodes, searchFrom, Current.Services.EntityService); + var allContentStartNodes = umbracoContext.Security.CurrentUser.CalculateContentStartNodeIds(_entityService); + AppendPath(sb, UmbracoObjectTypes.Document, allContentStartNodes, searchFrom, _entityService); break; default: throw new NotSupportedException("The " + typeof(UmbracoTreeSearcher) + " currently does not support searching against object type " + entityType); @@ -92,11 +103,43 @@ namespace Umbraco.Web.Search var internalSearcher = index.GetSearcher(); + if (!BuildQuery(sb, query, searchFrom, fields, type)) + { + totalFound = 0; + return Enumerable.Empty(); + } + + var raw = internalSearcher.CreateCriteria().RawQuery(sb.ToString()); + + var result = internalSearcher + //only return the number of items specified to read up to the amount of records to fill from 0 -> the number of items on the page requested + .Search(raw, Convert.ToInt32(pageSize * (pageIndex + 1))); + + totalFound = result.TotalItemCount; + + var pagedResult = result.Skip(Convert.ToInt32(pageIndex)); + + switch (entityType) + { + case UmbracoEntityTypes.Member: + return MemberFromSearchResults(pagedResult.ToArray()); + case UmbracoEntityTypes.Media: + return MediaFromSearchResults(pagedResult); + case UmbracoEntityTypes.Document: + return ContentFromSearchResults(pagedResult); + default: + throw new NotSupportedException("The " + typeof(UmbracoTreeSearcher) + " currently does not support searching against object type " + entityType); + } + } + + private bool BuildQuery(StringBuilder sb, string query, string searchFrom, string[] fields, string type) + { //build a lucene query: // the nodeName will be boosted 10x without wildcards // then nodeName will be matched normally with wildcards // the rest will be normal without wildcards + var allLangs = _languageService.GetAllLanguages().Select(x => x.IsoCode).ToList(); //check if text is surrounded by single or double quotes, if so, then exact match var surroundedByQuotes = Regex.IsMatch(query, "^\".*?\"$") @@ -105,15 +148,14 @@ namespace Umbraco.Web.Search if (surroundedByQuotes) { //strip quotes, escape string, the replace again - query = query.Trim(new[] { '\"', '\'' }); + query = query.Trim('\"', '\''); query = Lucene.Net.QueryParsers.QueryParser.Escape(query); //nothing to search if (searchFrom.IsNullOrWhiteSpace() && query.IsNullOrWhiteSpace()) { - totalFound = 0; - return new List(); + return false; } //update the query with the query term @@ -122,10 +164,9 @@ namespace Umbraco.Web.Search //add back the surrounding quotes query = string.Format("{0}{1}{0}", "\"", query); - //node name exactly boost x 10 - sb.Append("+(nodeName: ("); - sb.Append(query.ToLower()); - sb.Append(")^10.0 "); + sb.Append("+("); + + AppendNodeNamePhraseWithBoost(sb, query, allLangs); foreach (var f in fields) { @@ -146,8 +187,7 @@ namespace Umbraco.Web.Search //nothing to search if (searchFrom.IsNullOrWhiteSpace() && trimmed.IsNullOrWhiteSpace()) { - totalFound = 0; - return new List(); + return false; } //update the query with the query term @@ -157,24 +197,12 @@ namespace Umbraco.Web.Search var querywords = query.Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries); - //node name exactly boost x 10 - sb.Append("+(nodeName:"); - sb.Append("\""); - sb.Append(query.ToLower()); - sb.Append("\""); - sb.Append("^10.0 "); - - //node name normally with wildcards - sb.Append(" nodeName:"); - sb.Append("("); - foreach (var w in querywords) - { - sb.Append(w.ToLower()); - sb.Append("* "); - } - sb.Append(") "); + sb.Append("+("); + AppendNodeNameExactWithBoost(sb, query, allLangs); + AppendNodeNameWithWildcards(sb, querywords, allLangs); + foreach (var f in fields) { //additional fields normally @@ -198,26 +226,69 @@ namespace Umbraco.Web.Search sb.Append("+__IndexType:"); sb.Append(type); - var raw = internalSearcher.CreateCriteria().RawQuery(sb.ToString()); + return true; + } - var result = internalSearcher - //only return the number of items specified to read up to the amount of records to fill from 0 -> the number of items on the page requested - .Search(raw, Convert.ToInt32(pageSize * (pageIndex + 1))); + private void AppendNodeNamePhraseWithBoost(StringBuilder sb, string query, IEnumerable allLangs) + { + //node name exactly boost x 10 + sb.Append("nodeName: ("); + sb.Append(query.ToLower()); + sb.Append(")^10.0 "); - totalFound = result.TotalItemCount; - - var pagedResult = result.Skip(Convert.ToInt32(pageIndex)); - - switch (entityType) + //also search on all variant node names + foreach (var lang in allLangs) { - case UmbracoEntityTypes.Member: - return MemberFromSearchResults(pagedResult.ToArray()); - case UmbracoEntityTypes.Media: - return MediaFromSearchResults(pagedResult); - case UmbracoEntityTypes.Document: - return ContentFromSearchResults(pagedResult); - default: - throw new NotSupportedException("The " + typeof(UmbracoTreeSearcher) + " currently does not support searching against object type " + entityType); + //node name exactly boost x 10 + sb.Append($"nodeName_{lang}: ("); + sb.Append(query.ToLower()); + sb.Append(")^10.0 "); + } + } + + private void AppendNodeNameExactWithBoost(StringBuilder sb, string query, IEnumerable allLangs) + { + //node name exactly boost x 10 + sb.Append("nodeName:"); + sb.Append("\""); + sb.Append(query.ToLower()); + sb.Append("\""); + sb.Append("^10.0 "); + //also search on all variant node names + foreach (var lang in allLangs) + { + //node name exactly boost x 10 + sb.Append($"nodeName_{lang}:"); + sb.Append("\""); + sb.Append(query.ToLower()); + sb.Append("\""); + sb.Append("^10.0 "); + } + } + + private void AppendNodeNameWithWildcards(StringBuilder sb, string[] querywords, IEnumerable allLangs) + { + //node name normally with wildcards + sb.Append("nodeName:"); + sb.Append("("); + foreach (var w in querywords) + { + sb.Append(w.ToLower()); + sb.Append("* "); + } + sb.Append(") "); + //also search on all variant node names + foreach (var lang in allLangs) + { + //node name normally with wildcards + sb.Append($"nodeName_{lang}:"); + sb.Append("("); + foreach (var w in querywords) + { + sb.Append(w.ToLower()); + sb.Append("* "); + } + sb.Append(") "); } } @@ -281,32 +352,33 @@ namespace Umbraco.Web.Search /// /// /// - private IEnumerable MemberFromSearchResults(ISearchResult[] results) + private IEnumerable MemberFromSearchResults(IEnumerable results) { - var mapped = Mapper.Map>(results).ToArray(); //add additional data - foreach (var m in mapped) + foreach (var result in results) { + var m = Mapper.Map(result); + //if no icon could be mapped, it will be set to document, so change it to picture if (m.Icon == "icon-document") { m.Icon = "icon-user"; } - - var searchResult = results.First(x => x.Id == m.Id.ToString()); - if (searchResult.Values.ContainsKey("email") && searchResult.Values["email"] != null) + + if (result.Values.ContainsKey("email") && result.Values["email"] != null) { - m.AdditionalData["Email"] = results.First(x => x.Id == m.Id.ToString()).Values["email"]; + m.AdditionalData["Email"] = result.Values["email"]; } - if (searchResult.Values.ContainsKey("__key") && searchResult.Values["__key"] != null) + if (result.Values.ContainsKey("__key") && result.Values["__key"] != null) { - if (Guid.TryParse(searchResult.Values["__key"], out var key)) + if (Guid.TryParse(result.Values["__key"], out var key)) { m.Key = key; } } + + yield return m; } - return mapped; } /// @@ -316,17 +388,17 @@ namespace Umbraco.Web.Search /// private IEnumerable MediaFromSearchResults(IEnumerable results) { - var mapped = Mapper.Map>(results).ToArray(); //add additional data - foreach (var m in mapped) + foreach (var result in results) { + var m = Mapper.Map(result); //if no icon could be mapped, it will be set to document, so change it to picture if (m.Icon == "icon-document") { m.Icon = "icon-picture"; } + yield return m; } - return mapped; } /// @@ -345,7 +417,7 @@ namespace Umbraco.Web.Search var intId = entity.Id.TryConvertTo(); if (intId.Success) { - //TODO: Here we need to figure out how to get the URL based on variant, etc... + //if it varies by culture, return the default language URL if (result.Values.TryGetValue(UmbracoContentIndex.VariesByCultureFieldName, out var varies) && varies == "1") { entity.AdditionalData["Url"] = _umbracoHelper.Url(intId.Result, defaultLang); From 1fe3045b4c912a1ed01ef301f51fd687ee9cc474 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 11 Dec 2018 17:54:10 +1100 Subject: [PATCH 3/5] Adds TODO notes --- src/Umbraco.Web/Models/Mapping/EntityMapperProfile.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Umbraco.Web/Models/Mapping/EntityMapperProfile.cs b/src/Umbraco.Web/Models/Mapping/EntityMapperProfile.cs index ea76293df5..178027857c 100644 --- a/src/Umbraco.Web/Models/Mapping/EntityMapperProfile.cs +++ b/src/Umbraco.Web/Models/Mapping/EntityMapperProfile.cs @@ -122,6 +122,8 @@ namespace Umbraco.Web.Models.Mapping .ForMember(dest => dest.AdditionalData, opt => opt.Ignore()) .AfterMap((src, dest) => { + //TODO: Properly map this (not aftermap) + //get the icon if there is one dest.Icon = src.Values.ContainsKey(UmbracoExamineIndex.IconFieldName) ? src.Values[UmbracoExamineIndex.IconFieldName] From 8048d57ffe062da230151f841e771d7cddc8e93a Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 11 Dec 2018 17:57:40 +1100 Subject: [PATCH 4/5] ensures query fields contained the lowercased iso code --- src/Umbraco.Web/Search/UmbracoTreeSearcher.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web/Search/UmbracoTreeSearcher.cs b/src/Umbraco.Web/Search/UmbracoTreeSearcher.cs index 8f977aab77..c3ab7318a0 100644 --- a/src/Umbraco.Web/Search/UmbracoTreeSearcher.cs +++ b/src/Umbraco.Web/Search/UmbracoTreeSearcher.cs @@ -139,7 +139,7 @@ namespace Umbraco.Web.Search // then nodeName will be matched normally with wildcards // the rest will be normal without wildcards - var allLangs = _languageService.GetAllLanguages().Select(x => x.IsoCode).ToList(); + var allLangs = _languageService.GetAllLanguages().Select(x => x.IsoCode.ToLowerInvariant()).ToList(); //check if text is surrounded by single or double quotes, if so, then exact match var surroundedByQuotes = Regex.IsMatch(query, "^\".*?\"$") From 5593d49719437f3ce4ccb3e4b3bc97da94139ff1 Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 19 Dec 2018 16:00:29 +0100 Subject: [PATCH 5/5] Minor fixes as per PR#3852 discussion --- src/Umbraco.Web/Runtime/WebRuntimeComponent.cs | 6 +----- src/Umbraco.Web/Search/SearchableTreeCollection.cs | 8 +++++--- src/Umbraco.Web/Trees/ISearchableTree.cs | 3 ++- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs b/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs index ccd9a7ef7d..0d27253466 100644 --- a/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs +++ b/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs @@ -127,12 +127,8 @@ namespace Umbraco.Web.Runtime composition.Container.RegisterUmbracoControllers(typeLoader, GetType().Assembly); composition.Container.EnableWebApi(GlobalConfiguration.Configuration); - //we aren't scanning for ISearchableTree since that is not IDiscoverable, instead we'll just filter what we've - //already scanned since all of our ISearchableTree is of type UmbracoApiController and in most cases any developers' - //own trees they want searched will also be of type UmbracoApiController. If a developer wants to replace one of ours - //then they will have to manually register/replace. composition.Container.RegisterCollectionBuilder() - .Add(() => typeLoader.GetTypes().Where(x => x.Implements())); + .Add(() => typeLoader.GetTypes()); composition.Container.Register(new PerRequestLifeTime()); diff --git a/src/Umbraco.Web/Search/SearchableTreeCollection.cs b/src/Umbraco.Web/Search/SearchableTreeCollection.cs index 81ee0a2898..38c329cafa 100644 --- a/src/Umbraco.Web/Search/SearchableTreeCollection.cs +++ b/src/Umbraco.Web/Search/SearchableTreeCollection.cs @@ -1,6 +1,8 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Collections.ObjectModel; using System.Linq; +using Umbraco.Core; using Umbraco.Core.Composing; using Umbraco.Core.Services; using Umbraco.Web.Trees; @@ -22,11 +24,11 @@ namespace Umbraco.Web.Search var appTrees = treeService.GetAll() .OrderBy(x => x.SortOrder) .ToArray(); - var dictionary = new Dictionary(); + var dictionary = new Dictionary(StringComparer.OrdinalIgnoreCase); var searchableTrees = this.ToArray(); foreach (var appTree in appTrees) { - var found = searchableTrees.FirstOrDefault(x => x.TreeAlias == appTree.Alias); + var found = searchableTrees.FirstOrDefault(x => x.TreeAlias.InvariantEquals(appTree.Alias)); if (found != null) { dictionary[found.TreeAlias] = new SearchableApplicationTree(appTree.ApplicationAlias, appTree.Alias, found); diff --git a/src/Umbraco.Web/Trees/ISearchableTree.cs b/src/Umbraco.Web/Trees/ISearchableTree.cs index 4146bfaf45..3d82d548c8 100644 --- a/src/Umbraco.Web/Trees/ISearchableTree.cs +++ b/src/Umbraco.Web/Trees/ISearchableTree.cs @@ -1,9 +1,10 @@ using System.Collections.Generic; +using Umbraco.Core.Composing; using Umbraco.Web.Models.ContentEditing; namespace Umbraco.Web.Trees { - public interface ISearchableTree + public interface ISearchableTree : IDiscoverable { /// /// The alias of the tree that the belongs to