From 5148f34b3296b694b84e666ac778e655b5d69d67 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 11 Dec 2018 15:42:32 +1100 Subject: [PATCH] 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; }