diff --git a/src/Umbraco.Examine/ExamineExtensions.cs b/src/Umbraco.Examine/ExamineExtensions.cs index 1b8033c458..d97278f31c 100644 --- a/src/Umbraco.Examine/ExamineExtensions.cs +++ b/src/Umbraco.Examine/ExamineExtensions.cs @@ -48,6 +48,31 @@ namespace Umbraco.Examine } } + /// + /// Returns all index fields that are culture specific (suffixed) or invariant + /// + /// + /// + /// + public static IEnumerable GetCultureAndInvariantFields(this IUmbracoIndex index, string culture) + { + var allFields = index.GetFields(); + // ReSharper disable once LoopCanBeConvertedToQuery + foreach (var field in allFields) + { + var match = CultureIsoCodeFieldNameMatchExpression.Match(field); + if (match.Success && match.Groups.Count == 3 && culture.InvariantEquals(match.Groups[2].Value)) + { + yield return field; //matches this culture field + } + else if (!match.Success) + { + yield return field; //matches no culture field (invariant) + } + + } + } + internal 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 diff --git a/src/Umbraco.Tests/TestHelpers/RandomIdRamDirectory.cs b/src/Umbraco.Tests/TestHelpers/RandomIdRamDirectory.cs new file mode 100644 index 0000000000..34904db1ae --- /dev/null +++ b/src/Umbraco.Tests/TestHelpers/RandomIdRamDirectory.cs @@ -0,0 +1,22 @@ +using Lucene.Net.Store; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Umbraco.Tests.TestHelpers +{ + + /// + /// Used for tests with Lucene so that each RAM directory is unique + /// + public class RandomIdRAMDirectory : RAMDirectory + { + private readonly string _lockId = Guid.NewGuid().ToString(); + public override string GetLockId() + { + return _lockId; + } + } +} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index f41ff1dd07..717006b702 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -157,6 +157,7 @@ + @@ -268,6 +269,7 @@ + diff --git a/src/Umbraco.Tests/Web/PublishedContentQueryTests.cs b/src/Umbraco.Tests/Web/PublishedContentQueryTests.cs new file mode 100644 index 0000000000..a3505aeb0e --- /dev/null +++ b/src/Umbraco.Tests/Web/PublishedContentQueryTests.cs @@ -0,0 +1,99 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Examine; +using Examine.LuceneEngine.Providers; +using Lucene.Net.Store; +using Moq; +using NUnit.Framework; +using Umbraco.Core.Models.PublishedContent; +using Umbraco.Examine; +using Umbraco.Tests.TestHelpers; +using Umbraco.Web; +using Umbraco.Web.PublishedCache; + +namespace Umbraco.Tests.Web +{ + [TestFixture] + public class PublishedContentQueryTests + { + + private class TestIndex : LuceneIndex, IUmbracoIndex + { + private readonly string[] _fieldNames; + + public TestIndex(string name, Directory luceneDirectory, string[] fieldNames) + : base(name, luceneDirectory, null, null, null, null) + { + _fieldNames = fieldNames; + } + public bool EnableDefaultEventHandler => throw new NotImplementedException(); + public bool PublishedValuesOnly => throw new NotImplementedException(); + public IEnumerable GetFields() => _fieldNames; + } + + private TestIndex CreateTestIndex(Directory luceneDirectory, string[] fieldNames) + { + var indexer = new TestIndex("TestIndex", luceneDirectory, fieldNames); + + using (indexer.ProcessNonAsync()) + { + //populate with some test data + indexer.IndexItem(new ValueSet("1", "content", new Dictionary + { + [fieldNames[0]] = "Hello world, there are products here", + [UmbracoContentIndex.VariesByCultureFieldName] = "n" + })); + indexer.IndexItem(new ValueSet("2", "content", new Dictionary + { + [fieldNames[1]] = "Hello world, there are products here", + [UmbracoContentIndex.VariesByCultureFieldName] = "y" + })); + indexer.IndexItem(new ValueSet("3", "content", new Dictionary + { + [fieldNames[2]] = "Hello world, there are products here", + [UmbracoContentIndex.VariesByCultureFieldName] = "y" + })); + } + + return indexer; + } + + private PublishedContentQuery CreatePublishedContentQuery(IIndex indexer) + { + var examineManager = new Mock(); + IIndex outarg = indexer; + examineManager.Setup(x => x.TryGetIndex("TestIndex", out outarg)).Returns(true); + + var contentCache = new Mock(); + contentCache.Setup(x => x.GetById(It.IsAny())).Returns((int intId) => Mock.Of(x => x.Id == intId)); + var snapshot = Mock.Of(x => x.Content == contentCache.Object); + var variationContext = new VariationContext(); + var variationContextAccessor = Mock.Of(x => x.VariationContext == variationContext); + + return new PublishedContentQuery(snapshot, variationContextAccessor, examineManager.Object); + } + + [TestCase("fr-fr", ExpectedResult = "1, 3", TestName = "Search Culture: fr-fr. Must return both fr-fr and invariant results")] + [TestCase("en-us", ExpectedResult = "1, 2", TestName = "Search Culture: en-us. Must return both en-us and invariant results")] + [TestCase("*", ExpectedResult = "1, 2, 3", TestName = "Search Culture: *. Must return all cultures and all invariant results")] + [TestCase(null, ExpectedResult = "1", TestName = "Search Culture: null. Must return only invariant results")] + public string Search(string culture) + { + using (var luceneDir = new RandomIdRAMDirectory()) + { + var fieldNames = new[] { "title", "title_en-us", "title_fr-fr" }; + using (var indexer = CreateTestIndex(luceneDir, fieldNames)) + { + var pcq = CreatePublishedContentQuery(indexer); + + var results = pcq.Search("Products", culture, "TestIndex"); + + var ids = results.Select(x => x.Content.Id).ToArray(); + + return string.Join(", ", ids); + } + } + } + } +} diff --git a/src/Umbraco.Tests/Web/UmbracoHelperTests.cs b/src/Umbraco.Tests/Web/UmbracoHelperTests.cs index b23b5bd6b7..26d85f60cf 100644 --- a/src/Umbraco.Tests/Web/UmbracoHelperTests.cs +++ b/src/Umbraco.Tests/Web/UmbracoHelperTests.cs @@ -1,5 +1,8 @@ using System; using System.Text; +using Examine.LuceneEngine; +using Lucene.Net.Analysis; +using Lucene.Net.Analysis.Standard; using Moq; using NUnit.Framework; using Umbraco.Core; @@ -13,18 +16,19 @@ using Umbraco.Web; namespace Umbraco.Tests.Web { + [TestFixture] public class UmbracoHelperTests - { + { [TearDown] public void TearDown() { Current.Reset(); } - - + + // ------- Int32 conversion tests [Test] public static void Converting_Boxed_34_To_An_Int_Returns_34() diff --git a/src/Umbraco.Web/IPublishedContentQuery.cs b/src/Umbraco.Web/IPublishedContentQuery.cs index 76e7be5e97..8a8d678aba 100644 --- a/src/Umbraco.Web/IPublishedContentQuery.cs +++ b/src/Umbraco.Web/IPublishedContentQuery.cs @@ -39,10 +39,14 @@ namespace Umbraco.Web /// Optional culture. /// Optional index name. /// - /// When the is not specified, 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. + /// /// While enumerating results, the ambient culture is changed to be the searched culture. /// - IEnumerable Search(string term, string culture = null, string indexName = null); + IEnumerable Search(string term, string culture = "*", string indexName = null); /// /// Searches content. @@ -54,10 +58,14 @@ namespace Umbraco.Web /// Optional culture. /// Optional index name. /// - /// When the is not specified, 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. + /// /// 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 = null, string indexName = null); + 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. diff --git a/src/Umbraco.Web/PublishedContentQuery.cs b/src/Umbraco.Web/PublishedContentQuery.cs index 61180580cb..2dbe4de4c5 100644 --- a/src/Umbraco.Web/PublishedContentQuery.cs +++ b/src/Umbraco.Web/PublishedContentQuery.cs @@ -20,14 +20,22 @@ namespace Umbraco.Web { private readonly IPublishedSnapshot _publishedSnapshot; private readonly IVariationContextAccessor _variationContextAccessor; + private readonly IExamineManager _examineManager; + + [Obsolete("Use the constructor with all parameters instead")] + public PublishedContentQuery(IPublishedSnapshot publishedSnapshot, IVariationContextAccessor variationContextAccessor) + : this (publishedSnapshot, variationContextAccessor, ExamineManager.Instance) + { + } /// /// Initializes a new instance of the class. /// - public PublishedContentQuery(IPublishedSnapshot publishedSnapshot, IVariationContextAccessor variationContextAccessor) + public PublishedContentQuery(IPublishedSnapshot publishedSnapshot, IVariationContextAccessor variationContextAccessor, IExamineManager examineManager) { _publishedSnapshot = publishedSnapshot ?? throw new ArgumentNullException(nameof(publishedSnapshot)); _variationContextAccessor = variationContextAccessor ?? throw new ArgumentNullException(nameof(variationContextAccessor)); + _examineManager = examineManager ?? throw new ArgumentNullException(nameof(examineManager)); } #region Content @@ -175,19 +183,19 @@ namespace Umbraco.Web #region Search /// - public IEnumerable Search(string term, string culture = null, string indexName = null) + public IEnumerable Search(string term, string culture = "*", string indexName = null) { return Search(term, 0, 0, out _, culture, indexName); } /// - public IEnumerable Search(string term, int skip, int take, out long totalRecords, string culture = null, string indexName = null) + 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 (!ExamineManager.Instance.TryGetIndex(indexName, out var index) || !(index is IUmbracoIndex umbIndex)) + 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(); @@ -195,20 +203,28 @@ namespace Umbraco.Web // default to max 500 results var count = skip == 0 && take == 0 ? 500 : skip + take; - //set this to the specific culture or to the culture in the request - culture = culture ?? _variationContextAccessor.VariationContext.Culture; - ISearchResults results; - if (culture.IsNullOrWhiteSpace()) + 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); + } else { + //search only the specified culture + //get all index fields suffixed with the culture name supplied - var cultureFields = umbIndex.GetCultureFields(culture); - var qry = searcher.CreateQuery().Field(UmbracoContentIndex.VariesByCultureFieldName, "y"); //must vary by culture - qry = qry.And().ManagedQuery(term, cultureFields.ToArray()); + var cultureFields = umbIndex.GetCultureAndInvariantFields(culture).ToArray(); + var qry = searcher.CreateQuery().ManagedQuery(term, cultureFields); results = qry.Execute(count); } @@ -304,7 +320,7 @@ namespace Umbraco.Web } } - + #endregion diff --git a/src/Umbraco.Web/UmbracoContextFactory.cs b/src/Umbraco.Web/UmbracoContextFactory.cs index 2a812036bf..11d8952fa6 100644 --- a/src/Umbraco.Web/UmbracoContextFactory.cs +++ b/src/Umbraco.Web/UmbracoContextFactory.cs @@ -53,7 +53,15 @@ namespace Umbraco.Web { // make sure we have a variation context if (_variationContextAccessor.VariationContext == null) + { + // TODO: By using _defaultCultureAccessor.DefaultCulture this means that the VariationContext will always return a variant culture, it will never + // return an empty string signifying that the culture is invariant. But does this matter? Are we actually expecting this to return an empty string + // for invariant routes? From what i can tell throughout the codebase is that whenever we are checking against the VariationContext.Culture we are + // also checking if the content type varies by culture or not. This is fine, however the code in the ctor of VariationContext is then misleading + // since it's assuming that the Culture can be empty (invariant) when in reality of a website this will never be empty since a real culture is always set here. _variationContextAccessor.VariationContext = new VariationContext(_defaultCultureAccessor.DefaultCulture); + } + var webSecurity = new WebSecurity(httpContext, _userService, _globalSettings);