breaking: started removing Examine from core depedencies

[bielu]

HI @bergmania,
As started looking into #13620 and it seemed like it will be equally bit change as abstracting out examine, I started working on it.
To do:

• Remove examine dependency
• Create base abstractions
• Get Umbraco to boot
• Restructure headless API to use search provider
• Implement filter and sort logic
• Reintroduce configuration for examine provider
• Reintroduce tests
• update docs

[github-actions]

Hi there @bielu, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

• It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
• The automated tests all pass (see "Checks" tab on this PR)
• The level of security for this contribution is the same or improved
• The level of performance for this contribution is the same or improved
• Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
• If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
• 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[bergmania]

Hi @bielu..

It looks pretty good already.
IMO it makes good sense to abstract out examine completely, as long as it can still be implemented using examine (and implementations of examine).

To make the abstractions even better it would make sense to try to implement that using a non-lucene based search engine. E.g. Algolia

[bielu]

@bergmania I am doing full abstraction, so result validation and specific configuration are going to be provider based (or by abstraction layer). So I am doing default implementation of Lucene Examine (leaving space for other Examine providers - potentially space for other Examine providers)

[bergmania]

@bielu, yep, I see and that's great :)

[bielu]

@bergmania I was thinking about it and it might make sense to move search providers to separate repo at some point, also I am consider preparing Elastic Search provider(it is main search provider which I am using in projects). Having separate repo will make it easier to maintain it outside of Umbraco live cycle and deliver updates / new features in much easier way, what you think about it?

[bergmania]

@bielu having the abstractions outside of Umbraco is almost identical to using Examine. Examine is also just abstractions.
So I think it makes most sense to keep the abstractions in Umbraco and version it together with Umbraco.

[bielu]

@bergmania you misunderstood me here 😓, I mean just implementation for search provider, not abstractions!

[bielu]

@bergmania I found few things, which I am not sure if we want to reintroduce, FriendlyPublishedContentExtensions have SearchDescendants method, which I think should not be part of Umbraco.Common package, if we want to reindrouce it we should move it to specific providers, also it use managed query which will be really expensive as it will query every field separately.

[bergmania]

@bielu, if possible I think we should try to keep an extension method with the same signature as in FriendlyPublishedContentExtensions. It make sense to move such an extension to a search project instead.

[bielu]

@bergmania as abstraction will be mostly for backoffice I think it should be moved to specific providers if we reintroduce it after changes, what you think? than people can use native DSL for each provider without us maintaining full blown abstraction, which I done for example here: https://github.com/bielu/SImpl.SearchModule, it allows to switch providers without rewriting search queries, but as every project of this type you paying with little of performance as queries are not optimaze as well as native DSL

[bielu]

@bergmania I think abstraction will not change much from this stage, so we can start discussing them now. I noticed in new management api libraries there was few services which were created in relation to search, do you know if there any works now to do anything else related to search management?
In meantime I will be focusing on restoring functionality (right now search management works and index is getting updated but searches are not working

[bergmania]

@bielu it is still under development and still subject to change alot, so it is too early to present any code

[bielu]

@bergmania I am going to hang it for while, to have indexes from 12 in state where I can look and validate abstraction, as covering now with testing wouldn't make sense if I need rewrite abstraction tu support headless index :)

[bergmania]

@bielu I'm sorry, but I agree this make sense. I'll try to mention you when we have something.

[bielu]

@bergmania any news on Headless index for v12 or not yet?

[bergmania]

@bielu

It was merged into v12/dev yesterday.
#14051

[bielu]

@bergmania I am working on aligning my pr with changes related to DeliveryApi my question there:

1. I see you introduced query handlers, but they are they design to deal with search query or any content query?
2. I see you introduced sort handlers, so fart they seemed really search specific, I think it they should be in search abstraction package, but let me know if I am wrong
3. I looked into filters and they seem to be simple and or filters, without grouping am I right? (Examples of grouping filtering: (A || B) && (C || D), as based on code I expect it to be always A || B || C || D or A && B && C && D)
4. all implementation of IContentIndexHandler are internal are you sure of this choice?
5. IContentIndexHandler seems like should belong to search abstraction am I right?

[bielu]

hey @bergmania,
I am going to have some spare time soon, if you could respond to question above would be great as it would unclock me with resolving conflicts from v12 :)

[bergmania]

Hi @bielu..

I'll just get some internal help to answer the questions, and return when I have answers..

The abstractions are only relevant for the content delivery api, and not search in general.

[kjac]

Hi there @bielu 👋

I'm not sure you need to be overly concerned with the Delivery API implementation at this point.

The search/query related parts of the Delivery API are built to be technology agnostic. Thus you won't find any Examine dependencies in the selectors (ISelectorHandler), filters (IFilterHandler), sorters (ISortHandler), content indexers (IContentIndexHandler) or query handlers (IQueryHandler).

The same goes for the query service (IApiContentQueryService), which is pretty much "just" an orchestrator for all (or most) of the above.

Things get technology (Examine) specific in two places:

• The core implementation of IApiContentQueryProvider, which is responsible for executing the queries built by the query service.
• The examine index creation and population, which is mostly located in the Umbraco.Cms.Infrastructure.Examine namespace (look for anything called DeliveryApi) and in the DeliveryApiContentIndexingNotificationHandler.

We have an ambition of making the notification handler more technology agnostic (provider based somehow), but it's not going to happen for 12.0 --- if at all. The handler performs a lot of assumptions that are mostly related to how the Examine index is built, so it might be impossible to abstract it into something that yields any value for alternative technology providers to leverage.

With all of this in mind, let me try to answer your concrete questions 😆

I see you introduced query handlers, but they are they design to deal with search query or any content query?

They are for querying content items through the Delivery API only.

I see you introduced sort handlers, so fart they seemed really search specific, I think it they should be in search abstraction package, but let me know if I am wrong

These are the concrete implementations of the sorting options specified in the Delivery API specification

I looked into filters and they seem to be simple and or filters, without grouping am I right? (Examples of grouping filtering: (A || B) && (C || D), as based on code I expect it to be always A || B || C || D or A && B && C && D)

The Delivery API query "language" is kept quite rudimentary at this point. Filters can be chained and will always be applied as AND. However, depending on the individual filter implementations, a single filter might perform a grouped OR on multiple values.

Ultimately, all of this is up to the IApiContentQueryProvider to handle.

all implementation of IContentIndexHandler are internal are you sure of this choice?

Absolutely. These are built to power the individual query options of the Delivery API specification, and again - they are entirely technology agnostic.

IContentIndexHandler seems like should belong to search abstraction am I right?

This has only to do with procuring data for specific parts of the query options. One option (e.g. a selector or a filter) is backed by a one content indexer to provide the data necessary to utilise said option in a search.

[bielu]

so my understanding based @kjac comments, only bits for me take care based on v12 changes:

• ensure the new delivery index is using new search providers instead of Examine directly
• ensure the top apis are working with underhood abstraction, without moving anything around from delivery project
• base filter implementation is and only (which is cool and easy to implement

and based on your "We have an ambition of making the notification handler more technology agnostic (provider based somehow), but it's not going to happen for 12.0" isnt it what is happening in this pr? :)

[kjac]

@bielu correct 👍

and based on your "We have an ambition of making the notification handler more technology agnostic (provider based somehow), but it's not going to happen for 12.0" isnt it what is happening in this pr? :)

Well yes 😄 🤞

[bielu]

@kjac managed to get all changes from v12 included in this pr, working now on making tests working, but it would be great if someone would go through and make some comments around code, as there is a lot bits which I moved. I also have on list making filters and sorts working (as didnt implement it yet), but in general it would be great to get some feedback around if anything should be added / changed

[bergmania]

Hi @bielu

Thank you for all the work.

Sadly we won't have any one that can review it the next couple of weeks due to vacation time. 🌴

I will create a task for @kjac, so he can take a look when he is back 😓

[bielu]

@bergmania than he might have all code done by then :), as hopefully manage to port all tests and missing small bits

[bergmania]

Sorry, the PR got closed because the v13/dev branch have been deleted and recreated from v12, due to new backoffice being targeting v14. Please open as new PR or if you can change the base branch, feel free do to that

[bielu]

@bergmania will move stuff around from scratch as it will limit amount of code changes and conflicts, so will open new pr most likely in next few days