Fix registering custom searchers, allow searchers without fulltext (#2755)

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;

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);
     }

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 = new GambitManager($this->container->make($fullTextGambitClass));
-                    $gambitManager->setFulltextGambit($this->container->make($fullTextGambitClass));
                     foreach (Arr::get($this->container->make('flarum.simple_search.gambits'), $searcher, []) as $gambit) {
                         $gambitManager->add($this->container->make($gambit));
                     }

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)
+    {
+    }
+}