From 5e2340bf101a46caff921989c6ad19b80338c863 Mon Sep 17 00:00:00 2001
From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com>
Date: Mon, 19 Apr 2021 16:59:53 -0400
Subject: [PATCH] Fix registering custom searchers, allow searchers without
 fulltext  (#2755)

---
 src/Extend/SimpleFlarumSearch.php             |  2 +-
 src/Search/GambitManager.php                  | 26 ++++------
 src/Search/SearchServiceProvider.php          |  6 +--
 .../extenders/SimpleFlarumSearchTest.php      | 50 +++++++++++++++++++
 4 files changed, 62 insertions(+), 22 deletions(-)

diff --git a/src/Extend/SimpleFlarumSearch.php b/src/Extend/SimpleFlarumSearch.php
index cce557715..57e441a8e 100644
--- a/src/Extend/SimpleFlarumSearch.php
+++ b/src/Extend/SimpleFlarumSearch.php
@@ -73,7 +73,7 @@ class SimpleFlarumSearch implements ExtenderInterface
     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;
diff --git a/src/Search/GambitManager.php b/src/Search/GambitManager.php
index ca625dfd0..928bd1257 100644
--- a/src/Search/GambitManager.php
+++ b/src/Search/GambitManager.php
@@ -12,7 +12,7 @@ namespace Flarum\Search;
 use LogicException;
 
 /**
- * @todo This whole gambits thing needs way better documentation.
+ * @internal
  */
 class GambitManager
 {
@@ -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;
     }
@@ -51,16 +59,6 @@ class GambitManager
         }
     }
 
-    /**
-     * 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.
      *
@@ -110,10 +108,6 @@ class GambitManager
      */
     protected function applyFulltext(SearchState $search, $query)
     {
-        if (! $this->fulltextGambit) {
-            return;
-        }
-
         $search->addActiveGambit($this->fulltextGambit);
         $this->fulltextGambit->apply($search, $query);
     }
diff --git a/src/Search/SearchServiceProvider.php b/src/Search/SearchServiceProvider.php
index 7db1c7e4a..0547bdd51 100644
--- a/src/Search/SearchServiceProvider.php
+++ b/src/Search/SearchServiceProvider.php
@@ -58,9 +58,6 @@ class SearchServiceProvider extends AbstractServiceProvider
      */
     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) {
@@ -68,8 +65,7 @@ class SearchServiceProvider extends AbstractServiceProvider
                 ->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));
                     }
diff --git a/tests/integration/extenders/SimpleFlarumSearchTest.php b/tests/integration/extenders/SimpleFlarumSearchTest.php
index 5fb32a262..c582ec24a 100644
--- a/tests/integration/extenders/SimpleFlarumSearchTest.php
+++ b/tests/integration/extenders/SimpleFlarumSearchTest.php
@@ -12,13 +12,17 @@ namespace Flarum\Tests\integration\extenders;
 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
 {
@@ -135,6 +139,36 @@ class SimpleFlarumSearchTest extends TestCase
 
         $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
@@ -179,3 +213,19 @@ class CustomSearchMutator
         $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)
+    {
+    }
+}