flarum / framework

Simple forum software for building great communities.
http://flarum.org/
6.27k stars 826 forks source link

Search drivers #3884

Closed SychO9 closed 10 months ago

SychO9 commented 11 months ago

This is a breakdown of the current implementation of searching/filtering, and a plan to implement search drivers. The work done prior to stable 1.0 presents a good base to build on top. But there is some refactoring to be done first.

Current Implementation

The current system in place separates the concept of searching & filtering. So we have Gambits and Filters, which are in practice both filtering mechanisms that do the the same thing. This adds some complexity on the backend side as well as the frontend side where applying a filter (like when clicking on the following sidebar link to show subscribed to discussions) requires doing it in two formats by checking if a search is active or not.

The choice between filtering and searching rests on whether a q filter is active. From there we either apply basic database filtering, or searching.

The current AbstractSearchers are a hardcoded database search driver implementation.

What should not change:

Current search extensions

search-drivers-2--1

Brainstorming

Gambits Vs Filters

Dropping gambits on the backend should be the first step. Gambits are just filters apply in a different format. We can transform gambits to API request filters from the frontend. The additional advantage of that is the frontend becomes aware of gambits which allows us to improve the search UI/UX after the work here is done.

Filterers VS Searchers

Removing gambits still means we need to have filters per searcher. We could simply replace the idea of gambits in searchers with filters, but that leaves some unnecessary duplication between Filterers Vs Searchers.

It seems clear that we can simply do without the separate concept of a Filterer. If a search driver implements its own filtering (like Blomstra's elastic implementation) then it can do the filtering. If a driver does not (like Clark's scout) then it can fallback to the database driver to apply filters (like Clark's scout impl). The difference lies in what the driver does. So it needs to be able to fallback to the database driver after it has the results it needs (the same line of thinking applies to visibility scoping).

search-drivers-2--2

Implementing drivers

The rest should be fairly straightforward to implement after the above is taken care of.

What we need is:

I envision the use from the controller to be something along the lines of

        // Before
        $criteria = new QueryCriteria($actor, $filters, $sort, $sortIsDefault);
        if (array_key_exists('q', $filters)) {
            $results = $this->searcher->search($criteria, $limit, $offset);
        } else {
            $results = $this->filterer->filter($criteria, $limit, $offset);
        }

        // After
        $results = $this->search->for(Discussion::class)->search(
            new QueryCriteria($actor, $filters, $sort, $limit, $offset, $sortIsDefault);
        );
SychO9 commented 11 months ago

Update

Filtering vs Searching

My initial line of thought for having no separation at all between simple filtering and searching was incorrect. It would be sane to keep on relying on the database implementation when filtering only, and in the future we might easily introduce the ability to change that behavior per driver if it makes sense to do so.

An upside is that this only means w default to the database search driver for that.

search-drivers-2--2

Indexing

I initially wanted to leave the logic for listening to events and indexing models to the driver extension implementation. But fter looking into a POC it is clear to me that there is enough repetitive boilerplate involved that it would make sense to expose an API for it that makes it more seamless.

The only ting I'm unsure of at the moment is if we should also have a built in index building command or whether to leave that to extensions. Since we would have the indexer classes, then we could build the command, but extensions might require custom commands with custom options regardless so I'm leaning towards leaving it up to extensions.

(new Extend\SearchIndex)
    ->indexer(Discussion::class, DiscussionIndexer::class)
    ->indexer(Post::class, PostIndexer::class),
interface IndexerInterface
{
    static function index(): string;

    /**
     * @param AbstractModel[] $models
     */
    function save(array $models): void;

    /**
     * @param AbstractModel[] $models
     */
    function delete(array $models): void;
}
class ModelObserver
{
    public function __construct(
        protected SearchManager $search,
        protected Queue $queue
    ) {
    }

    public function created(AbstractModel $model): void
    {
        $this->runIndexJob($model, 'save');
    }

    public function updated(AbstractModel $model): void
    {
        $this->runIndexJob($model, 'save');
    }

    public function hidden(AbstractModel $model): void
    {
        $this->runIndexJob($model, 'delete');
    }

    public function deleted(AbstractModel $model): void
    {
        $this->runIndexJob($model, 'delete');
    }

    public function forceDeleted(AbstractModel $model): void
    {
        $this->runIndexJob($model, 'delete');
    }

    public function restored(AbstractModel $model): void
    {
        $this->runIndexJob($model, 'save');
    }

    private function runIndexJob(AbstractModel $model, string $operation): void
    {
        if ($this->search->indexable($model)) {
            foreach ($this->search->indexers($model) as $indexerClass) {
                $this->queue->push(new IndexJob($indexerClass, [$model], $operation));
            }
        }
    }
}
SychO9 commented 10 months ago

Now that we have the backend search drivers API in place, we can think of certain follow up tasks:

Admin settings to change drivers

We now need to build the UI on the admin side to allow selecting a driver per-model. An advanced page that is hidden by default makes sense here, if you don't have alternative search drivers, the page will not make much sense. It will be toggleable through an option under the cache clearing option.

This page will in the future store this type of advanced settings and drivers.

(Will follow up with a PR).

Displaying gambits on the frontend

This is part of the search UI/UX task so we won't tackle it here, but the way we would go about it is since gambits are now just a layer that translates a string to a filter, we would use information from the backend about what filters are supported by the current search driver and go from there. (We'll brainstorm the details for that when working on that task).

Improvements to the current backend implementation.

While working on the POC and a couple other cases for searching in Flarum. I noticed two limitations that need to be addressed:

(Will follow up with a PR).


edit 29th Nov (search ui/ux preview) screen-capture (18).webm ezgif-1-57ca234ac3