Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IPublishedContentQuery.Search needs to support culture #3828

Closed
Shazwazza opened this issue Dec 5, 2018 · 5 comments
Closed

IPublishedContentQuery.Search needs to support culture #3828

Shazwazza opened this issue Dec 5, 2018 · 5 comments

Comments

@Shazwazza
Copy link
Contributor

Shazwazza commented Dec 5, 2018

child of #3530
requires #3535

Be sure to review #3535 first!

IPublishedContentQuery.Search needs to support culture so that searching can be contextual to a language automatically

PR: #3892

... see notes below

@Shazwazza
Copy link
Contributor Author

There are many Examine API changes made to support a much cleaner/simpler search interface that isn't a totally broken abstraction.

Umbraco's own IPublishedContentQuery had 4 search methods:

IEnumerable<IPublishedContent> TypedSearch(string term, bool useWildCards = true, string searchProvider = null);

IEnumerable<IPublishedContent> TypedSearch(int skip, int take, out int totalRecords, string term, bool useWildCards = true, string searchProvider = null);

IEnumerable<IPublishedContent> TypedSearch(Examine.SearchCriteria.ISearchCriteria criteria, Examine.Providers.BaseSearchProvider searchProvider = null);

IEnumerable<IPublishedContent> TypedSearch(int skip, int take, out int totalrecords, Examine.SearchCriteria.ISearchCriteria criteria, Examine.Providers.BaseSearchProvider searchProvider = null);

There are several problems with these methods:

  • The return result does not provide a score so it's not easy to know how the results are ordered
  • The latter 2 methods are a broken abstraction - which is now what is also fixed in Examine - this is because you cannot pass in an ISearchCriteria instance to a BaseSearchProvider that didn't create it whereas these methods allow you to do just that

The updated 4 methods look like:

IEnumerable<PublishedSearchResult> Search(string term, string indexName = null);

IEnumerable<PublishedSearchResult> Search(string term, int skip, int take, out long totalRecords, string indexName = null);

IEnumerable<PublishedSearchResult> Search(IQueryExecutor query);

IEnumerable<PublishedSearchResult> Search(IQueryExecutor query, int skip, int take, out long totalRecords);

The result is now a wrapper of IPublishedContent that includes the Score. There is now 'useWildcards' because examine no longer has this parameter. If you pass a single term to Examine it will use it's underying IIndexValueType to create the most appropriate query for each field, which for any full text fields will use wildcards and boost accordingly. The method parameters are also ordered and make more sense.

The latter 2 methods now only accept a IQueryExecutor which is a new interface in Examine and it's how a fluent API query is executed. This means there are no broken abstractions. These 2 methods essentially just convert a search result to the output model with the correct paging logic.

@Shazwazza
Copy link
Contributor Author

Have updated the first 2 methods to accept a culture:

IEnumerable<PublishedSearchResult> Search(string term, string culture = null, string indexName = null);

IEnumerable<PublishedSearchResult> Search(string term, int skip, int take, out long totalRecords, string culture = null, string indexName = null);

When a culture is specified it will go find all culture specific names for the culture specified (i.e. myField_en-us if the culture was en-us or en-US), then it will perform a managed search across all of these culture fields for the term specified, it will also add a mandatory query argument for "varies by culture" since if the user is searching on a culture, the document must have this flag.

The 2 methods accepting IQueryExecutor cannot have culture specified, since these methods are based on the fluent search API which the developer builds up themselves.

@zpqrtbnk
Copy link
Contributor

code review is ok

Testing, having issues. I have a content item which is variant, has different names on two cultures. A Search("Home") or Search("Maison"), with en-US being the default lang, works. But a Search("Home", "en-US") does not. And Search("Maison", "fr-FR") does not either.

Checking the index, I can see that Name, Name_en-us, etc are all set correctly. Querying the External Index from the dashboard, works. Have republished the content item.

I... have tried to look into PublishedContentQuery but cannot figure out what's wrong.

I have tried to rebuild the index but then it becomes weirder. I can instantly rebuild the Internal Index, but the External Index rebuild never completes (keeps doing PostCheckRebuildIndex forever).

@zpqrtbnk
Copy link
Contributor

more details: I have physically deleted the indexes, and restarted the site - indexes are rebuilt, but it does not change a thing (search fails, cannot rebuild external index)

@zpqrtbnk
Copy link
Contributor

Problem with search is now issue #3920

Rest is OK, merging the PR now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants