Merge pull request #5862 from umbraco/v8/bugfix/5789-PublishedContentQuery-Search

Fixes #5789 - PublishedContentQuery Search always searches on culture
This commit is contained in:
Bjarke Berg
2019-07-24 14:16:32 +02:00
committed by GitHub
8 changed files with 203 additions and 19 deletions

View File

@@ -48,6 +48,31 @@ namespace Umbraco.Examine
}
}
/// <summary>
/// Returns all index fields that are culture specific (suffixed) or invariant
/// </summary>
/// <param name="index"></param>
/// <param name="culture"></param>
/// <returns></returns>
public static IEnumerable<string> 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

View File

@@ -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
{
/// <summary>
/// Used for tests with Lucene so that each RAM directory is unique
/// </summary>
public class RandomIdRAMDirectory : RAMDirectory
{
private readonly string _lockId = Guid.NewGuid().ToString();
public override string GetLockId()
{
return _lockId;
}
}
}

View File

@@ -157,6 +157,7 @@
<Compile Include="Services\MemberGroupServiceTests.cs" />
<Compile Include="Services\MediaTypeServiceTests.cs" />
<Compile Include="Services\PropertyValidationServiceTests.cs" />
<Compile Include="TestHelpers\RandomIdRamDirectory.cs" />
<Compile Include="Testing\Objects\TestDataSource.cs" />
<Compile Include="Published\PublishedSnapshotTestObjects.cs" />
<Compile Include="Published\ModelTypeTests.cs" />
@@ -268,6 +269,7 @@
<Compile Include="Strings\StylesheetHelperTests.cs" />
<Compile Include="Strings\StringValidationTests.cs" />
<Compile Include="Web\Mvc\ValidateUmbracoFormRouteStringAttributeTests.cs" />
<Compile Include="Web\PublishedContentQueryTests.cs" />
<Compile Include="Web\UmbracoHelperTests.cs" />
<Compile Include="Membership\MembershipProviderBaseTests.cs" />
<Compile Include="Membership\UmbracoServiceMembershipProviderTests.cs" />

View File

@@ -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<string> 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<string, object>
{
[fieldNames[0]] = "Hello world, there are products here",
[UmbracoContentIndex.VariesByCultureFieldName] = "n"
}));
indexer.IndexItem(new ValueSet("2", "content", new Dictionary<string, object>
{
[fieldNames[1]] = "Hello world, there are products here",
[UmbracoContentIndex.VariesByCultureFieldName] = "y"
}));
indexer.IndexItem(new ValueSet("3", "content", new Dictionary<string, object>
{
[fieldNames[2]] = "Hello world, there are products here",
[UmbracoContentIndex.VariesByCultureFieldName] = "y"
}));
}
return indexer;
}
private PublishedContentQuery CreatePublishedContentQuery(IIndex indexer)
{
var examineManager = new Mock<IExamineManager>();
IIndex outarg = indexer;
examineManager.Setup(x => x.TryGetIndex("TestIndex", out outarg)).Returns(true);
var contentCache = new Mock<IPublishedContentCache>();
contentCache.Setup(x => x.GetById(It.IsAny<int>())).Returns((int intId) => Mock.Of<IPublishedContent>(x => x.Id == intId));
var snapshot = Mock.Of<IPublishedSnapshot>(x => x.Content == contentCache.Object);
var variationContext = new VariationContext();
var variationContextAccessor = Mock.Of<IVariationContextAccessor>(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);
}
}
}
}
}

View File

@@ -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()

View File

@@ -39,10 +39,14 @@ namespace Umbraco.Web
/// <param name="culture">Optional culture.</param>
/// <param name="indexName">Optional index name.</param>
/// <remarks>
/// <para>When the <paramref name="culture"/> is not specified, all cultures are searched.</para>
/// <para>
/// When the <paramref name="culture"/> 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.
/// </para>
/// <para>While enumerating results, the ambient culture is changed to be the searched culture.</para>
/// </remarks>
IEnumerable<PublishedSearchResult> Search(string term, string culture = null, string indexName = null);
IEnumerable<PublishedSearchResult> Search(string term, string culture = "*", string indexName = null);
/// <summary>
/// Searches content.
@@ -54,10 +58,14 @@ namespace Umbraco.Web
/// <param name="culture">Optional culture.</param>
/// <param name="indexName">Optional index name.</param>
/// <remarks>
/// <para>When the <paramref name="culture"/> is not specified, all cultures are searched.</para>
/// <para>
/// When the <paramref name="culture"/> 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.
/// </para>
/// <para>While enumerating results, the ambient culture is changed to be the searched culture.</para>
/// </remarks>
IEnumerable<PublishedSearchResult> Search(string term, int skip, int take, out long totalRecords, string culture = null, string indexName = null);
IEnumerable<PublishedSearchResult> Search(string term, int skip, int take, out long totalRecords, string culture = "*", string indexName = null);
/// <summary>
/// Executes the query and converts the results to PublishedSearchResult.

View File

@@ -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)
{
}
/// <summary>
/// Initializes a new instance of the <see cref="PublishedContentQuery"/> class.
/// </summary>
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
/// <inheritdoc />
public IEnumerable<PublishedSearchResult> Search(string term, string culture = null, string indexName = null)
public IEnumerable<PublishedSearchResult> Search(string term, string culture = "*", string indexName = null)
{
return Search(term, 0, 0, out _, culture, indexName);
}
/// <inheritdoc />
public IEnumerable<PublishedSearchResult> Search(string term, int skip, int take, out long totalRecords, string culture = null, string indexName = null)
public IEnumerable<PublishedSearchResult> 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

View File

@@ -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);