diff --git a/framework/core/src/Discussion/Event/Searching.php b/framework/core/src/Discussion/Event/Searching.php index 51471b689..138a4f469 100644 --- a/framework/core/src/Discussion/Event/Searching.php +++ b/framework/core/src/Discussion/Event/Searching.php @@ -12,6 +12,9 @@ namespace Flarum\Discussion\Event; use Flarum\Discussion\Search\DiscussionSearch; use Flarum\Search\SearchCriteria; +/** + * @deprecated beta 16, remove beta 17 + */ class Searching { /** diff --git a/framework/core/src/Discussion/Search/DiscussionSearcher.php b/framework/core/src/Discussion/Search/DiscussionSearcher.php index 6f4dc5839..edc755b8c 100644 --- a/framework/core/src/Discussion/Search/DiscussionSearcher.php +++ b/framework/core/src/Discussion/Search/DiscussionSearcher.php @@ -11,21 +11,16 @@ namespace Flarum\Discussion\Search; use Flarum\Discussion\DiscussionRepository; use Flarum\Discussion\Event\Searching; -use Flarum\Search\ApplySearchParametersTrait; +use Flarum\Search\AbstractSearch; +use Flarum\Search\AbstractSearcher; use Flarum\Search\GambitManager; use Flarum\Search\SearchCriteria; -use Flarum\Search\SearchResults; +use Flarum\User\User; use Illuminate\Contracts\Events\Dispatcher; +use Illuminate\Database\Eloquent\Builder; -class DiscussionSearcher +class DiscussionSearcher extends AbstractSearcher { - use ApplySearchParametersTrait; - - /** - * @var GambitManager - */ - protected $gambits; - /** * @var DiscussionRepository */ @@ -37,53 +32,36 @@ class DiscussionSearcher protected $events; /** - * @param GambitManager $gambits * @param DiscussionRepository $discussions * @param Dispatcher $events + * @param GambitManager $gambits + * @param array $searchMutators */ - public function __construct(GambitManager $gambits, DiscussionRepository $discussions, Dispatcher $events) + public function __construct(DiscussionRepository $discussions, Dispatcher $events, GambitManager $gambits, array $searchMutators) { - $this->gambits = $gambits; + parent::__construct($gambits, $searchMutators); + $this->discussions = $discussions; $this->events = $events; } - /** - * @param SearchCriteria $criteria - * @param int|null $limit - * @param int $offset - * - * @return SearchResults - */ - public function search(SearchCriteria $criteria, $limit = null, $offset = 0) + protected function getQuery(User $actor): Builder { - $actor = $criteria->actor; + return $this->discussions->query()->select('discussions.*')->whereVisibleTo($actor); + } - $query = $this->discussions->query()->select('discussions.*')->whereVisibleTo($actor); + protected function getSearch(Builder $query, User $actor): AbstractSearch + { + return new DiscussionSearch($query->getQuery(), $actor); + } - // Construct an object which represents this search for discussions. - // Apply gambits to it, sort, and paging criteria. Also give extensions - // an opportunity to modify it. - $search = new DiscussionSearch($query->getQuery(), $actor); - - $this->gambits->apply($search, $criteria->query); - $this->applySort($search, $criteria->sort); - $this->applyOffset($search, $offset); - $this->applyLimit($search, $limit + 1); + /** + * @deprecated along with the Searching event, remove in Beta 17. + */ + protected function mutateSearch(AbstractSearch $search, SearchCriteria $criteria) + { + parent::mutateSearch($search, $criteria); $this->events->dispatch(new Searching($search, $criteria)); - - // Execute the search query and retrieve the results. We get one more - // results than the user asked for, so that we can say if there are more - // results. If there are, we will get rid of that extra result. - $discussions = $query->get(); - - $areMoreResults = $limit > 0 && $discussions->count() > $limit; - - if ($areMoreResults) { - $discussions->pop(); - } - - return new SearchResults($discussions, $areMoreResults); } } diff --git a/framework/core/src/Event/AbstractConfigureGambits.php b/framework/core/src/Event/AbstractConfigureGambits.php index a2d437272..62b75d8ce 100644 --- a/framework/core/src/Event/AbstractConfigureGambits.php +++ b/framework/core/src/Event/AbstractConfigureGambits.php @@ -11,6 +11,9 @@ namespace Flarum\Event; use Flarum\Search\GambitManager; +/** + * @deprecated beta 16, removed in beta 17 + */ abstract class AbstractConfigureGambits { /** diff --git a/framework/core/src/Event/ConfigureDiscussionGambits.php b/framework/core/src/Event/ConfigureDiscussionGambits.php index e13fdaa4f..ba47e15c7 100644 --- a/framework/core/src/Event/ConfigureDiscussionGambits.php +++ b/framework/core/src/Event/ConfigureDiscussionGambits.php @@ -9,6 +9,9 @@ namespace Flarum\Event; +/** + * @deprecated beta 16, removed in beta 17 + */ class ConfigureDiscussionGambits extends AbstractConfigureGambits { } diff --git a/framework/core/src/Event/ConfigureUserGambits.php b/framework/core/src/Event/ConfigureUserGambits.php index 2dc20bbf7..80e14ed48 100644 --- a/framework/core/src/Event/ConfigureUserGambits.php +++ b/framework/core/src/Event/ConfigureUserGambits.php @@ -9,6 +9,9 @@ namespace Flarum\Event; +/** + * @deprecated beta 16, removed in beta 17 + */ class ConfigureUserGambits extends AbstractConfigureGambits { } diff --git a/framework/core/src/Extend/SimpleFlarumSearch.php b/framework/core/src/Extend/SimpleFlarumSearch.php new file mode 100644 index 000000000..6f308a506 --- /dev/null +++ b/framework/core/src/Extend/SimpleFlarumSearch.php @@ -0,0 +1,99 @@ +searcher = $searcherClass; + } + + /** + * Add a gambit to this searcher. Gambits are used to filter search queries. + * + * @param string $gambitClass: The ::class attribute of the gambit you are adding. + * This gambit must extend \Flarum\Search\AbstractRegexGambit + */ + public function addGambit($gambitClass) + { + $this->gambits[] = $gambitClass; + + return $this; + } + + /** + * Set the full text gambit for this searcher. The full text gambit actually executes the search. + * + * @param string $gambitClass: The ::class attribute of the full test gambit you are adding. + * This gambit must implement \Flarum\Search\GambitInterface + */ + public function setFullTextGambit($gambitClass) + { + $this->fullTextGambit = $gambitClass; + + return $this; + } + + /** + * Add a callback through which to run all search queries after gambits have been applied. + * + * @param callable|string $callback + * + * The callback can be a closure or an invokable class, and should accept: + * - Flarum\Search\AbstractSearch $search + * - Flarum\Search\SearchCriteria $criteria + */ + public function addSearchMutator($callback) + { + $this->searchMutators[] = $callback; + + return $this; + } + + public function extend(Container $container, Extension $extension = null) + { + if (! is_null($this->fullTextGambit)) { + $container->resolving('flarum.simple_search.fulltext_gambits', function ($oldFulltextGambits) { + $oldFulltextGambits[$this->searcher] = $this->fullTextGambit; + + return $oldFulltextGambits; + }); + } + + $container->extend('flarum.simple_search.gambits', function ($oldGambits) { + foreach ($this->gambits as $gambit) { + $oldGambits[$this->searcher][] = $gambit; + } + + return $oldGambits; + }); + + $container->extend('flarum.simple_search.search_mutators', function ($oldMutators) { + foreach ($this->searchMutators as $mutator) { + $oldMutators[$this->searcher][] = $mutator; + } + + return $oldMutators; + }); + } +} diff --git a/framework/core/src/Search/AbstractSearcher.php b/framework/core/src/Search/AbstractSearcher.php new file mode 100644 index 000000000..9ccb33d59 --- /dev/null +++ b/framework/core/src/Search/AbstractSearcher.php @@ -0,0 +1,81 @@ +gambits = $gambits; + $this->searchMutators = $searchMutators; + } + + abstract protected function getQuery(User $actor): Builder; + + abstract protected function getSearch(Builder $query, User $actor): AbstractSearch; + + protected function mutateSearch(AbstractSearch $search, SearchCriteria $criteria) + { + foreach ($this->searchMutators as $mutator) { + $mutator($search, $criteria); + } + } + + /** + * @param SearchCriteria $criteria + * @param int|null $limit + * @param int $offset + * + * @return SearchResults + */ + public function search(SearchCriteria $criteria, $limit = null, $offset = 0, array $load = []) + { + $actor = $criteria->actor; + + $query = $this->getQuery($actor); + + $search = $this->getSearch($query, $actor); + + $this->gambits->apply($search, $criteria->query); + $this->applySort($search, $criteria->sort); + $this->applyOffset($search, $offset); + $this->applyLimit($search, $limit + 1); + + $this->mutateSearch($search, $criteria); + + // Execute the search query and retrieve the results. We get one more + // results than the user asked for, so that we can say if there are more + // results. If there are, we will get rid of that extra result. + $results = $query->get(); + + if ($areMoreResults = $limit > 0 && $results->count() > $limit) { + $results->pop(); + } + + $results->load($load); + + return new SearchResults($results, $areMoreResults); + } +} diff --git a/framework/core/src/Search/GambitManager.php b/framework/core/src/Search/GambitManager.php index 0092dafb1..775af42b6 100644 --- a/framework/core/src/Search/GambitManager.php +++ b/framework/core/src/Search/GambitManager.php @@ -9,7 +9,6 @@ namespace Flarum\Search; -use Illuminate\Contracts\Container\Container; use LogicException; /** @@ -23,33 +22,36 @@ class GambitManager protected $gambits = []; /** - * @var string + * @var GambitInterface */ protected $fulltextGambit; - /** - * @var Container - */ - protected $container; - - /** - * @param Container $container - */ - public function __construct(Container $container) - { - $this->container = $container; - } - /** * Add a gambit. * - * @param string $gambit + * @param GambitInterface $gambit */ public function add($gambit) { $this->gambits[] = $gambit; } + /** + * @deprecated Do not use. Added temporarily to provide support for ConfigureUserGambits and ConfigureDiscussionGambits until they are removed in beta 17. + */ + public function getFullTextGambit() + { + return $this->fulltextGambit; + } + + /** + * @deprecated Do not use. Added temporarily to provide support for ConfigureUserGambits and ConfigureDiscussionGambits until they are removed in beta 17. + */ + public function getGambits() + { + return $this->gambits; + } + /** * Apply gambits to a search, given a search query. * @@ -68,7 +70,7 @@ class GambitManager /** * Set the gambit to handle fulltext searching. * - * @param string $gambit + * @param GambitInterface $gambit */ public function setFulltextGambit($gambit) { @@ -99,10 +101,8 @@ class GambitManager return ''; } - $gambits = array_map([$this->container, 'make'], $this->gambits); - foreach ($bits as $k => $bit) { - foreach ($gambits as $gambit) { + foreach ($this->gambits as $gambit) { if (! $gambit instanceof GambitInterface) { throw new LogicException( 'Gambit '.get_class($gambit).' does not implement '.GambitInterface::class @@ -130,9 +130,7 @@ class GambitManager return; } - $gambit = $this->container->make($this->fulltextGambit); - - $search->addActiveGambit($gambit); - $gambit->apply($search, $query); + $search->addActiveGambit($this->fulltextGambit); + $this->fulltextGambit->apply($search, $query); } } diff --git a/framework/core/src/Search/SearchServiceProvider.php b/framework/core/src/Search/SearchServiceProvider.php index e61de63b1..fee38bf95 100644 --- a/framework/core/src/Search/SearchServiceProvider.php +++ b/framework/core/src/Search/SearchServiceProvider.php @@ -18,63 +18,108 @@ use Flarum\Discussion\Search\Gambit\UnreadGambit; use Flarum\Event\ConfigureDiscussionGambits; use Flarum\Event\ConfigureUserGambits; use Flarum\Foundation\AbstractServiceProvider; +use Flarum\Foundation\ContainerUtil; use Flarum\User\Search\Gambit\EmailGambit; use Flarum\User\Search\Gambit\FulltextGambit as UserFulltextGambit; use Flarum\User\Search\Gambit\GroupGambit; use Flarum\User\Search\UserSearcher; -use Illuminate\Contracts\Container\Container; +use Illuminate\Support\Arr; class SearchServiceProvider extends AbstractServiceProvider { /** - * Register the service provider. - * - * @return void + * @inheritDoc */ public function register() { - $this->registerDiscussionGambits(); + $this->app->singleton('flarum.simple_search.fulltext_gambits', function () { + return [ + DiscussionSearcher::class => DiscussionFulltextGambit::class, + UserSearcher::class => UserFulltextGambit::class + ]; + }); - $this->registerUserGambits(); + $this->app->singleton('flarum.simple_search.gambits', function () { + return [ + DiscussionSearcher::class => [ + AuthorGambit::class, + CreatedGambit::class, + HiddenGambit::class, + UnreadGambit::class + ], + UserSearcher::class => [ + EmailGambit::class, + GroupGambit::class + ] + ]; + }); + + $this->app->singleton('flarum.simple_search.search_mutators', function () { + return []; + }); } - public function registerUserGambits() + /** + * {@inheritdoc} + */ + public function boot() { - $this->app->when(UserSearcher::class) - ->needs(GambitManager::class) - ->give(function (Container $app) { - $gambits = new GambitManager($app); + // 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->app->make('flarum.simple_search.fulltext_gambits'); - $gambits->setFulltextGambit(UserFulltextGambit::class); - $gambits->add(EmailGambit::class); - $gambits->add(GroupGambit::class); + foreach ($fullTextGambits as $searcher => $fullTextGambitClass) { + $this->app + ->when($searcher) + ->needs(GambitManager::class) + ->give(function () use ($searcher, $fullTextGambitClass) { + $gambitManager = new GambitManager(); + $gambitManager->setFulltextGambit($this->app->make($fullTextGambitClass)); + foreach (Arr::get($this->app->make('flarum.simple_search.gambits'), $searcher, []) as $gambit) { + $gambitManager->add($this->app->make($gambit)); + } - $app->make('events')->dispatch( - new ConfigureUserGambits($gambits) - ); + // Temporary BC Layer + // @deprecated beta 16, remove beta 17. - return $gambits; - }); - } + $oldEvents = [ + DiscussionSearcher::class => ConfigureDiscussionGambits::class, + UserSearcher::class => ConfigureUserGambits::class + ]; - public function registerDiscussionGambits() - { - $this->app->when(DiscussionSearcher::class) - ->needs(GambitManager::class) - ->give(function (Container $app) { - $gambits = new GambitManager($app); + foreach ($oldEvents as $oldSearcher => $event) { + if ($searcher === $oldSearcher) { + $tempGambits = new GambitManager; + $this->app->make('events')->dispatch( + new $event($tempGambits) + ); - $gambits->setFulltextGambit(DiscussionFulltextGambit::class); - $gambits->add(AuthorGambit::class); - $gambits->add(CreatedGambit::class); - $gambits->add(HiddenGambit::class); - $gambits->add(UnreadGambit::class); + if (! is_null($fullTextGambit = $tempGambits->getFullTextGambit())) { + $gambitManager->setFullTextGambit($this->app->make($fullTextGambit)); + } - $app->make('events')->dispatch( - new ConfigureDiscussionGambits($gambits) - ); + foreach ($tempGambits->getGambits() as $gambit) { + $gambitManager->add($this->app->make($gambit)); + } + } + } - return $gambits; - }); + // End BC Layer + + return $gambitManager; + }); + + $this->app + ->when($searcher) + ->needs('$searchMutators') + ->give(function () use ($searcher) { + $searchMutators = Arr::get($this->app->make('flarum.simple_search.search_mutators'), $searcher, []); + + return array_map(function ($mutator) { + return ContainerUtil::wrapCallback($mutator, $this->app); + }, $searchMutators); + }); + } } } diff --git a/framework/core/src/User/Event/Searching.php b/framework/core/src/User/Event/Searching.php index 7e6ce893f..ffa0db36c 100644 --- a/framework/core/src/User/Event/Searching.php +++ b/framework/core/src/User/Event/Searching.php @@ -12,6 +12,9 @@ namespace Flarum\User\Event; use Flarum\Search\SearchCriteria; use Flarum\User\Search\UserSearch; +/** + * @deprecated beta 16, remove beta 17 + */ class Searching { /** diff --git a/framework/core/src/User/Search/UserSearcher.php b/framework/core/src/User/Search/UserSearcher.php index 9a14a560a..cecd75651 100644 --- a/framework/core/src/User/Search/UserSearcher.php +++ b/framework/core/src/User/Search/UserSearcher.php @@ -9,25 +9,26 @@ namespace Flarum\User\Search; -use Flarum\Search\ApplySearchParametersTrait; +use Flarum\Search\AbstractSearch; +use Flarum\Search\AbstractSearcher; use Flarum\Search\GambitManager; use Flarum\Search\SearchCriteria; -use Flarum\Search\SearchResults; use Flarum\User\Event\Searching; +use Flarum\User\User; use Flarum\User\UserRepository; +use Illuminate\Contracts\Events\Dispatcher; +use Illuminate\Database\Eloquent\Builder; /** * Takes a UserSearchCriteria object, performs a search using gambits, * and spits out a UserSearchResults object. */ -class UserSearcher +class UserSearcher extends AbstractSearcher { - use ApplySearchParametersTrait; - /** - * @var GambitManager + * @var Dispatcher */ - protected $gambits; + protected $events; /** * @var UserRepository @@ -35,51 +36,36 @@ class UserSearcher protected $users; /** + * @param UserRepository $users + * @param Dispatcher $events * @param GambitManager $gambits - * @param \Flarum\User\UserRepository $users + * @param array $searchMutators */ - public function __construct(GambitManager $gambits, UserRepository $users) + public function __construct(UserRepository $users, Dispatcher $events, GambitManager $gambits, array $searchMutators) { - $this->gambits = $gambits; + parent::__construct($gambits, $searchMutators); + + $this->events = $events; $this->users = $users; } - /** - * @param SearchCriteria $criteria - * @param int|null $limit - * @param int $offset - * @param array $load An array of relationships to load on the results. - * @return SearchResults - */ - public function search(SearchCriteria $criteria, $limit = null, $offset = 0, array $load = []) + protected function getQuery(User $actor): Builder { - $actor = $criteria->actor; + return $this->users->query()->whereVisibleTo($actor); + } - $query = $this->users->query()->whereVisibleTo($actor); + protected function getSearch(Builder $query, User $actor): AbstractSearch + { + return new UserSearch($query->getQuery(), $actor); + } - // Construct an object which represents this search for users. - // Apply gambits to it, sort, and paging criteria. Also give extensions - // an opportunity to modify it. - $search = new UserSearch($query->getQuery(), $actor); + /** + * @deprecated along with the Searching event, remove in Beta 17. + */ + protected function mutateSearch(AbstractSearch $search, SearchCriteria $criteria) + { + parent::mutateSearch($search, $criteria); - $this->gambits->apply($search, $criteria->query); - $this->applySort($search, $criteria->sort); - $this->applyOffset($search, $offset); - $this->applyLimit($search, $limit + 1); - - event(new Searching($search, $criteria)); - - // Execute the search query and retrieve the results. We get one more - // results than the user asked for, so that we can say if there are more - // results. If there are, we will get rid of that extra result. - $users = $query->get(); - - if ($areMoreResults = ($limit > 0 && $users->count() > $limit)) { - $users->pop(); - } - - $users->load($load); - - return new SearchResults($users, $areMoreResults); + $this->events->dispatch(new Searching($search, $criteria)); } } diff --git a/framework/core/tests/integration/extenders/SimpleFlarumSearchTest.php b/framework/core/tests/integration/extenders/SimpleFlarumSearchTest.php new file mode 100644 index 000000000..2a874cf76 --- /dev/null +++ b/framework/core/tests/integration/extenders/SimpleFlarumSearchTest.php @@ -0,0 +1,175 @@ +database()->rollBack(); + + // We need to insert these outside of a transaction, because FULLTEXT indexing, + // which is needed for search, doesn't happen in transactions. + // We clean it up explcitly at the end. + $this->database()->table('discussions')->insert([ + ['id' => 1, 'title' => 'DISCUSSION 1', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1], + ['id' => 2, 'title' => 'DISCUSSION 2', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1], + ]); + + $this->database()->table('posts')->insert([ + ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

not in text

'], + ['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

lightsail in text

'], + ]); + + // We need to call these again, since we rolled back the transaction started by `::app()`. + $this->database()->beginTransaction(); + + $this->populateDatabase(); + } + + /** + * @inheritDoc + */ + protected function tearDown(): void + { + parent::tearDown(); + + $this->database()->table('discussions')->whereIn('id', [1, 2])->delete(); + $this->database()->table('posts')->whereIn('id', [1, 2])->delete(); + } + + public function searchDiscussions($query, $limit = null) + { + $this->app(); + + $actor = User::find(1); + + $criteria = new SearchCriteria($actor, $query); + + return $this->app()->getContainer()->make(DiscussionSearcher::class)->search($criteria, $limit)->getResults(); + } + + /** + * @test + */ + public function works_as_expected_with_no_modifications() + { + $this->prepDb(); + + $searchForAll = json_encode($this->searchDiscussions('in text', 5)); + $this->assertContains('DISCUSSION 1', $searchForAll); + $this->assertContains('DISCUSSION 2', $searchForAll); + + $searchForSecond = json_encode($this->searchDiscussions('lightsail', 5)); + $this->assertNotContains('DISCUSSION 1', $searchForSecond); + $this->assertContains('DISCUSSION 2', $searchForSecond); + } + + /** + * @test + */ + public function custom_full_text_gambit_has_effect_if_added() + { + $this->extend((new Extend\SimpleFlarumSearch(DiscussionSearcher::class))->setFullTextGambit(NoResultFullTextGambit::class)); + + $this->assertEquals('[]', json_encode($this->searchDiscussions('in text', 5))); + } + + /** + * @test + */ + public function custom_filter_gambit_has_effect_if_added() + { + $this->extend((new Extend\SimpleFlarumSearch(DiscussionSearcher::class))->addGambit(NoResultFilterGambit::class)); + + $this->prepDb(); + + $withResultSearch = json_encode($this->searchDiscussions('noResult:0', 5)); + $this->assertContains('DISCUSSION 1', $withResultSearch); + $this->assertContains('DISCUSSION 2', $withResultSearch); + $this->assertEquals('[]', json_encode($this->searchDiscussions('noResult:1', 5))); + } + + /** + * @test + */ + public function search_mutator_has_effect_if_added() + { + $this->extend((new Extend\SimpleFlarumSearch(DiscussionSearcher::class))->addSearchMutator(function ($search, $criteria) { + $search->getquery()->whereRaw('1=0'); + })); + + $this->prepDb(); + + $this->assertEquals('[]', json_encode($this->searchDiscussions('in text', 5))); + } + + /** + * @test + */ + public function search_mutator_has_effect_if_added_with_invokable_class() + { + $this->extend((new Extend\SimpleFlarumSearch(DiscussionSearcher::class))->addSearchMutator(CustomSearchMutator::class)); + + $this->prepDb(); + + $this->assertEquals('[]', json_encode($this->searchDiscussions('in text', 5))); + } +} + +class NoResultFullTextGambit implements GambitInterface +{ + /** + * {@inheritdoc} + */ + public function apply(AbstractSearch $search, $searchValue) + { + $search->getQuery() + ->whereRaw('0=1'); + } +} + +class NoResultFilterGambit extends AbstractRegexGambit +{ + protected $pattern = 'noResult:(.+)'; + + /** + * {@inheritdoc} + */ + public function conditions(AbstractSearch $search, array $matches, $negate) + { + $noResults = trim($matches[1], ' '); + if ($noResults == '1') { + $search->getQuery() + ->whereRaw('0=1'); + } + } +} + +class CustomSearchMutator +{ + public function __invoke($search, $criteria) + { + $search->getQuery()->whereRaw('1=0'); + } +}