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

Fix registering custom searchers, allow searchers without fulltext #2755

Merged
merged 11 commits into from
Apr 19, 2021
2 changes: 1 addition & 1 deletion src/Extend/SimpleFlarumSearch.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public function addSearchMutator($callback)
public function extend(Container $container, Extension $extension = null)
{
if (! is_null($this->fullTextGambit)) {
$container->resolving('flarum.simple_search.fulltext_gambits', function ($oldFulltextGambits) {
$container->extend('flarum.simple_search.fulltext_gambits', function ($oldFulltextGambits) {
$oldFulltextGambits[$this->searcher] = $this->fullTextGambit;

return $oldFulltextGambits;
Expand Down
26 changes: 10 additions & 16 deletions src/Search/GambitManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
use LogicException;

/**
* @todo This whole gambits thing needs way better documentation.
* @internal
*/
class GambitManager
{
Expand All @@ -26,12 +26,20 @@ class GambitManager
*/
protected $fulltextGambit;

/**
* @param GambitInterface $gambit
*/
public function __construct(GambitInterface $fulltextGambit)
{
$this->fulltextGambit = $fulltextGambit;
}

/**
* Add a gambit.
*
* @param GambitInterface $gambit
*/
public function add($gambit)
public function add(GambitInterface $gambit)
{
$this->gambits[] = $gambit;
}
Expand All @@ -51,16 +59,6 @@ public function apply(SearchState $search, $query)
}
}

/**
* Set the gambit to handle fulltext searching.
*
* @param GambitInterface $gambit
*/
public function setFulltextGambit($gambit)
{
$this->fulltextGambit = $gambit;
}

/**
* Explode a search query into an array of bits.
*
Expand Down Expand Up @@ -110,10 +108,6 @@ protected function applyGambits(SearchState $search, $query)
*/
protected function applyFulltext(SearchState $search, $query)
{
if (! $this->fulltextGambit) {
return;
}

$search->addActiveGambit($this->fulltextGambit);
$this->fulltextGambit->apply($search, $query);
}
Expand Down
6 changes: 1 addition & 5 deletions src/Search/SearchServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,14 @@ public function register()
*/
public function boot()
{
// The rest of these we can resolve in the when->needs->give callback,
// but we need to resolve at least one regardless so we know which
// searchers we need to register gambits for.
$fullTextGambits = $this->container->make('flarum.simple_search.fulltext_gambits');

foreach ($fullTextGambits as $searcher => $fullTextGambitClass) {
$this->container
->when($searcher)
->needs(GambitManager::class)
->give(function () use ($searcher, $fullTextGambitClass) {
$gambitManager = new GambitManager();
$gambitManager->setFulltextGambit($this->container->make($fullTextGambitClass));
$gambitManager = new GambitManager($this->container->make($fullTextGambitClass));
foreach (Arr::get($this->container->make('flarum.simple_search.gambits'), $searcher, []) as $gambit) {
$gambitManager->add($this->container->make($gambit));
}
Expand Down
50 changes: 50 additions & 0 deletions tests/integration/extenders/SimpleFlarumSearchTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,17 @@
use Carbon\Carbon;
use Flarum\Discussion\Search\DiscussionSearcher;
use Flarum\Extend;
use Flarum\Group\Group;
use Flarum\Query\QueryCriteria;
use Flarum\Search\AbstractRegexGambit;
use Flarum\Search\AbstractSearcher;
use Flarum\Search\GambitInterface;
use Flarum\Search\SearchState;
use Flarum\Testing\integration\RetrievesAuthorizedUsers;
use Flarum\Testing\integration\TestCase;
use Flarum\User\User;
use Illuminate\Contracts\Container\BindingResolutionException;
use Illuminate\Database\Eloquent\Builder;

class SimpleFlarumSearchTest extends TestCase
{
Expand Down Expand Up @@ -135,6 +139,36 @@ public function search_mutator_has_effect_if_added_with_invokable_class()

$this->assertEquals('[]', json_encode($this->searchDiscussions('in text', 5)));
}

/**
* @test
*/
public function cant_resolve_custom_searcher_without_fulltext_gambit()
{
$this->expectException(BindingResolutionException::class);

$this->app()->getContainer()->make(CustomSearcher::class);
}

/**
* @test
*/
public function can_resolve_custom_searcher_with_fulltext_gambit()
{
$this->extend(
(new Extend\SimpleFlarumSearch(CustomSearcher::class))->setFullTextGambit(CustomFullTextGambit::class)
);

$anExceptionWasThrown = false;

try {
$this->app()->getContainer()->make(CustomSearcher::class);
} catch (BindingResolutionException $e) {
$anExceptionWasThrown = true;
}

$this->assertFalse($anExceptionWasThrown);
}
}

class NoResultFullTextGambit implements GambitInterface
Expand Down Expand Up @@ -179,3 +213,19 @@ public function __invoke($search, $criteria)
$search->getQuery()->whereRaw('1=0');
}
}

class CustomSearcher extends AbstractSearcher
{
// This isn't actually used, we just need it to implement the abstract method.
protected function getQuery(User $actor): Builder
{
return Group::query();
}
}

class CustomFullTextGambit implements GambitInterface
{
public function apply(SearchState $search, $bit)
{
}
}