From 49f7c92f5e105bb88249bf3a716e1c522db2bb3d Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 13 Nov 2020 17:18:16 -0500 Subject: [PATCH 01/36] Introduce extensible filterer system, make ListUsersController and ListDiscussionsController use that instead of searcher if a "q" filter parameter is provided --- .../Controller/ListDiscussionsController.php | 32 ++++- src/Api/Controller/ListUsersController.php | 32 ++++- src/Extend/Filter.php | 63 ++++++++ src/Filter/FilterInterface.php | 26 ++++ src/Filter/Filterer.php | 88 ++++++++++++ src/Filter/WrappedFilter.php | 16 +++ tests/integration/extenders/FilterTest.php | 135 ++++++++++++++++++ 7 files changed, 382 insertions(+), 10 deletions(-) create mode 100644 src/Extend/Filter.php create mode 100644 src/Filter/FilterInterface.php create mode 100644 src/Filter/Filterer.php create mode 100644 src/Filter/WrappedFilter.php create mode 100644 tests/integration/extenders/FilterTest.php diff --git a/src/Api/Controller/ListDiscussionsController.php b/src/Api/Controller/ListDiscussionsController.php index dc825d9cd8..a030c185bb 100644 --- a/src/Api/Controller/ListDiscussionsController.php +++ b/src/Api/Controller/ListDiscussionsController.php @@ -11,7 +11,9 @@ use Flarum\Api\Serializer\DiscussionSerializer; use Flarum\Discussion\Discussion; +use Flarum\Discussion\DiscussionRepository; use Flarum\Discussion\Search\DiscussionSearcher; +use Flarum\Filter\Filterer; use Flarum\Http\UrlGenerator; use Flarum\Search\SearchCriteria; use Illuminate\Support\Arr; @@ -48,6 +50,16 @@ class ListDiscussionsController extends AbstractListController */ public $sortFields = ['lastPostedAt', 'commentCount', 'createdAt']; + /** + * @var DiscussionRepository + */ + protected $discussions; + + /** + * @var Filterer + */ + protected $filterer; + /** * @var DiscussionSearcher */ @@ -59,11 +71,15 @@ class ListDiscussionsController extends AbstractListController protected $url; /** + * @param DiscussionRepository $discussions + * @param Filterer $filterer * @param DiscussionSearcher $searcher * @param UrlGenerator $url */ - public function __construct(DiscussionSearcher $searcher, UrlGenerator $url) + public function __construct(DiscussionRepository $discussions, Filterer $filterer, DiscussionSearcher $searcher, UrlGenerator $url) { + $this->discussions = $discussions; + $this->filterer = $filterer; $this->searcher = $searcher; $this->url = $url; } @@ -74,16 +90,22 @@ public function __construct(DiscussionSearcher $searcher, UrlGenerator $url) protected function data(ServerRequestInterface $request, Document $document) { $actor = $request->getAttribute('actor'); - $query = Arr::get($this->extractFilter($request), 'q'); + $filters = $this->extractFilter($request); $sort = $this->extractSort($request); - $criteria = new SearchCriteria($actor, $query, $sort); - $limit = $this->extractLimit($request); $offset = $this->extractOffset($request); $load = array_merge($this->extractInclude($request), ['state']); - $results = $this->searcher->search($criteria, $limit, $offset); + if (array_key_exists('q', $filters)) { + $criteria = new SearchCriteria($actor, $filters['q'], $sort); + + $results = $this->searcher->search($criteria, $limit, $offset, $load); + } else { + $query = $this->discussions->query(); + + $results = $this->filterer->filter($actor, $query, $filters, $sort, $limit, $offset, $load); + } $document->addPaginationLinks( $this->url->to('api')->route('discussions.index'), diff --git a/src/Api/Controller/ListUsersController.php b/src/Api/Controller/ListUsersController.php index 5f44c0984a..4fe4cd3c60 100644 --- a/src/Api/Controller/ListUsersController.php +++ b/src/Api/Controller/ListUsersController.php @@ -10,9 +10,11 @@ namespace Flarum\Api\Controller; use Flarum\Api\Serializer\UserSerializer; +use Flarum\Filter\Filterer; use Flarum\Http\UrlGenerator; use Flarum\Search\SearchCriteria; use Flarum\User\Search\UserSearcher; +use Flarum\User\UserRepository; use Illuminate\Support\Arr; use Psr\Http\Message\ServerRequestInterface; use Tobscure\JsonApi\Document; @@ -40,6 +42,11 @@ class ListUsersController extends AbstractListController 'joinedAt' ]; + /** + * @var Filterer + */ + protected $filterer; + /** * @var UserSearcher */ @@ -51,13 +58,22 @@ class ListUsersController extends AbstractListController protected $url; /** + * @var UserRepository + */ + protected $users; + + /** + * @param Filterer $filterer * @param UserSearcher $searcher * @param UrlGenerator $url + * @param UserRepository $users */ - public function __construct(UserSearcher $searcher, UrlGenerator $url) + public function __construct(Filterer $filterer, UserSearcher $searcher, UrlGenerator $url, UserRepository $users) { + $this->filterer = $filterer; $this->searcher = $searcher; $this->url = $url; + $this->users = $users; } /** @@ -69,16 +85,22 @@ protected function data(ServerRequestInterface $request, Document $document) $actor->assertCan('viewUserList'); - $query = Arr::get($this->extractFilter($request), 'q'); + $filters = $this->extractFilter($request); $sort = $this->extractSort($request); - $criteria = new SearchCriteria($actor, $query, $sort); - $limit = $this->extractLimit($request); $offset = $this->extractOffset($request); $load = $this->extractInclude($request); - $results = $this->searcher->search($criteria, $limit, $offset, $load); + if (array_key_exists('q', $filters)) { + $criteria = new SearchCriteria($actor, $filters['q'], $sort); + + $results = $this->searcher->search($criteria, $limit, $offset, $load); + } else { + $query = $this->users->query(); + + $results = $this->filterer->filter($actor, $query, $filters, $sort, $limit, $offset, $load); + } $document->addPaginationLinks( $this->url->to('api')->route('users.index'), diff --git a/src/Extend/Filter.php b/src/Extend/Filter.php new file mode 100644 index 0000000000..50f040d3d9 --- /dev/null +++ b/src/Extend/Filter.php @@ -0,0 +1,63 @@ +resource = $resource; + } + + /** + * Add a filter to run when the resource is filtered. + * + * @param string $filterClass: The ::class attribute of the filter you are adding. + */ + public function addFilter(string $filterClass) + { + $this->filters[] = $filterClass; + + return $this; + } + + /** + * Add a callback through which to run all filter queries after filters have been applied. + */ + public function addFilterMutator($callback) + { + $this->filterMutators[] = $callback; + + return $this; + } + + public function extend(Container $container, Extension $extension = null) + { + foreach ($this->filters as $filter) { + Filterer::addFilter($this->resource, $container->make($filter)); + } + + foreach ($this->filterMutators as $mutator) { + Filterer::addFilterMutator($this->resource, ContainerUtil::wrapCallback($mutator, $container)); + } + } +} diff --git a/src/Filter/FilterInterface.php b/src/Filter/FilterInterface.php new file mode 100644 index 0000000000..73b5b29d70 --- /dev/null +++ b/src/Filter/FilterInterface.php @@ -0,0 +1,26 @@ +getKey(), static::$filters[$resource])) { + static::$filters[$resource][$filter->getKey()] = []; + } + + static::$filters[$resource][$filter->getKey()][] = $filter; + } + + public static function addFilterMutator($resource, $mutator) + { + if (! array_key_exists($resource, static::$filterMutators)) { + static::$filterMutators[$resource] = []; + } + + static::$filterMutators[$resource][] = $mutator; + } + + /** + * @param FilterCriteria $criteria + * @param int|null $limit + * @param int $offset + * + * @return FilterResults + */ + public function filter($actor, $query, $filters, $sort = null, $limit = null, $offset = 0, array $load = []) + { + $resource = get_class($query->getModel()); + + $query->whereVisibleTo($actor); + + $wrappedFilter = new WrappedFilter($query->getQuery(), $actor); + + foreach ($filters as $filterKey => $filterValue) { + foreach (Arr::get(static::$filters, "$resource.$filterKey", []) as $filter) { + $filter->apply($wrappedFilter, $filterValue); + } + } + + $this->applySort($wrappedFilter, $sort); + $this->applyOffset($wrappedFilter, $offset); + $this->applyLimit($wrappedFilter, $limit + 1); + + foreach (Arr::get(static::$filterMutators, $resource, []) as $mutator) { + $mutator($query, $actor, $filters, $sort); + } + + // Execute the filter 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/src/Filter/WrappedFilter.php b/src/Filter/WrappedFilter.php new file mode 100644 index 0000000000..a1f9be79e6 --- /dev/null +++ b/src/Filter/WrappedFilter.php @@ -0,0 +1,16 @@ +prepareDatabase([ + 'discussions' => [ + ['id' => 1, 'title' => 'DISCUSSION 1', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'first_post_id' => 1, 'comment_count' => 1], + ['id' => 2, 'title' => 'DISCUSSION 2', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'first_post_id' => 2, 'comment_count' => 1], + ], + 'posts' => [ + ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

foo bar

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

foo bar not the same

'], + ], + 'users' => [ + $this->adminUser(), + $this->normalUser(), + ], + ]); + } + + public function filterDiscussions($filters, $limit = null) + { + $response = $this->send( + $this->request('GET', '/api/discussions', [ + 'authenticatedAs' => 1, + ])->withQueryParams([ + 'filter' => $filters, + 'include' => 'mostRelevantPost', + ]) + ); + + return json_decode($response->getBody()->getContents(), true)['data']; + } + + /** + * @test + */ + public function works_as_expected_with_no_modifications() + { + $this->prepDb(); + + $searchForAll = json_encode($this->filterDiscussions([], 5)); + $this->assertContains('DISCUSSION 1', $searchForAll); + $this->assertContains('DISCUSSION 2', $searchForAll); + } + + /** + * @test + */ + public function custom_filter_gambit_has_effect_if_added() + { + $this->extend((new Extend\Filter(Discussion::class))->addFilter(NoResultFilter::class)); + + $this->prepDb(); + + $withResultSearch = json_encode($this->filterDiscussions(['noResult' => 0], 5)); + $this->assertContains('DISCUSSION 1', $withResultSearch); + $this->assertContains('DISCUSSION 2', $withResultSearch); + $this->assertEquals([], $this->filterDiscussions(['noResult' => 1], 5)); + } + + /** + * @test + */ + public function filter_mutator_has_effect_if_added() + { + $this->extend((new Extend\Filter(Discussion::class))->addFilterMutator(function ($query, $actor, $filters, $sort) { + $query->getQuery()->whereRaw('1=0'); + })); + + $this->prepDb(); + + $this->assertEquals([], $this->filterDiscussions([], 5)); + } + + /** + * @test + */ + public function filter_mutator_has_effect_if_added_with_invokable_class() + { + $this->extend((new Extend\Filter(Discussion::class))->addFilterMutator(CustomFilterMutator::class)); + + $this->prepDb(); + + $this->assertEquals([], $this->filterDiscussions([], 5)); + } +} + +class NoResultFilter implements FilterInterface +{ + public function getFilterKey(): string + { + return 'noResult'; + } + + /** + * {@inheritdoc} + */ + public function applyFilter(WrappedFilter $wrappedFilter, $filterValue) + { + if ($filterValue) { + $wrappedFilter->getQuery() + ->whereRaw('0=1'); + } + } +} + +class CustomFilterMutator +{ + public function __invoke($query, $actor, $filters, $sort) + { + $query->getQuery()->whereRaw('1=0'); + } +} From f76de0b10d7e3b9ccab7b2fe5b48bc3bebe24830 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 13 Nov 2020 19:06:20 -0500 Subject: [PATCH 02/36] Add filter counterparts to builtin gambits --- src/Discussion/Filter/AuthorFilter.php | 49 +++++++++++++++++++++++ src/Discussion/Filter/CreatedFilter.php | 39 ++++++++++++++++++ src/Discussion/Filter/HiddenFilter.php | 32 +++++++++++++++ src/Discussion/Filter/UnreadFilter.php | 53 +++++++++++++++++++++++++ src/User/Filter/EmailFilter.php | 32 +++++++++++++++ src/User/Filter/GroupFilter.php | 43 ++++++++++++++++++++ 6 files changed, 248 insertions(+) create mode 100644 src/Discussion/Filter/AuthorFilter.php create mode 100644 src/Discussion/Filter/CreatedFilter.php create mode 100644 src/Discussion/Filter/HiddenFilter.php create mode 100644 src/Discussion/Filter/UnreadFilter.php create mode 100644 src/User/Filter/EmailFilter.php create mode 100644 src/User/Filter/GroupFilter.php diff --git a/src/Discussion/Filter/AuthorFilter.php b/src/Discussion/Filter/AuthorFilter.php new file mode 100644 index 0000000000..9fea602dcf --- /dev/null +++ b/src/Discussion/Filter/AuthorFilter.php @@ -0,0 +1,49 @@ +users = $users; + } + + public function getFilterKey(): string + { + return 'author'; + } + + public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) + { + $usernames = trim($filterValue, '"'); + $usernames = explode(',', $usernames); + + $ids = []; + foreach ($usernames as $username) { + $ids[] = $this->users->getIdForUsername($username); + } + + $wrappedFilter->getQuery()->whereIn('discussions.user_id', $ids, 'and', $negate); + + } +} diff --git a/src/Discussion/Filter/CreatedFilter.php b/src/Discussion/Filter/CreatedFilter.php new file mode 100644 index 0000000000..3f40ce6453 --- /dev/null +++ b/src/Discussion/Filter/CreatedFilter.php @@ -0,0 +1,39 @@ +getQuery()->whereDate('created_at', $negate ? '!=' : '=', $matches[0]); + } else { + $wrappedFilter->getQuery()->whereBetween('created_at', [$matches[0], $matches[2]], 'and', $negate); + } + + } +} diff --git a/src/Discussion/Filter/HiddenFilter.php b/src/Discussion/Filter/HiddenFilter.php new file mode 100644 index 0000000000..450ec425ed --- /dev/null +++ b/src/Discussion/Filter/HiddenFilter.php @@ -0,0 +1,32 @@ +getQuery()->where(function ($query) use ($negate) { + if ($negate) { + $query->whereNull('hidden_at')->where('comment_count', '>', 0); + } else { + $query->whereNotNull('hidden_at')->orWhere('comment_count', 0); + } + }); + } +} diff --git a/src/Discussion/Filter/UnreadFilter.php b/src/Discussion/Filter/UnreadFilter.php new file mode 100644 index 0000000000..ec192c6893 --- /dev/null +++ b/src/Discussion/Filter/UnreadFilter.php @@ -0,0 +1,53 @@ +discussions = $discussions; + } + + + public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) + { + $actor = $wrappedFilter->getActor(); + + if ($actor->exists) { + $readIds = $this->discussions->getReadIds($actor); + + $wrappedFilter->getQuery()->where(function ($query) use ($readIds, $negate, $actor) { + if (!$negate) { + $query->whereNotIn('id', $readIds)->where('last_posted_at', '>', $actor->marked_all_as_read_at ?: 0); + } else { + $query->whereIn('id', $readIds)->orWhere('last_posted_at', '<=', $actor->marked_all_as_read_at ?: 0); + } + }); + } + } +} diff --git a/src/User/Filter/EmailFilter.php b/src/User/Filter/EmailFilter.php new file mode 100644 index 0000000000..9fe8128f72 --- /dev/null +++ b/src/User/Filter/EmailFilter.php @@ -0,0 +1,32 @@ +getActor()->hasPermission('user.edit')) { + return; + } + + $email = trim($filterValue, '"'); + + $wrappedFilter->getQuery()->where('email', $negate ? '!=' : '=', $email); + } +} diff --git a/src/User/Filter/GroupFilter.php b/src/User/Filter/GroupFilter.php new file mode 100644 index 0000000000..85cbe61a55 --- /dev/null +++ b/src/User/Filter/GroupFilter.php @@ -0,0 +1,43 @@ +getActor()); + + foreach ($groupIdentifiers as $identifier) { + if (is_numeric($identifier)) { + $groupQuery->orWhere('id', $identifier); + } else { + $groupQuery->orWhere('name_singular', $identifier)->orWhere('name_plural', $identifier); + } + } + + $userIds = $groupQuery->join('group_user', 'groups.id', 'group_user.group_id') + ->pluck('group_user.user_id') + ->all(); + + $wrappedFilter->getQuery()->whereIn('id', $userIds, 'and', $negate); + } +} From 7c99276010c69fb5a82c1f1b2b2545a40af392f9 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 13 Nov 2020 19:07:13 -0500 Subject: [PATCH 03/36] Properly register filters, support negation (passed into filter interface) --- src/Extend/Filter.php | 10 ++-- src/Filter/FilterInterface.php | 2 +- src/Filter/FilterServiceProvider.php | 59 ++++++++++++++++++++++ src/Filter/Filterer.php | 13 +++-- src/Foundation/InstalledSite.php | 2 + tests/integration/extenders/FilterTest.php | 2 +- 6 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 src/Filter/FilterServiceProvider.php diff --git a/src/Extend/Filter.php b/src/Extend/Filter.php index 50f040d3d9..adc25ab74c 100644 --- a/src/Extend/Filter.php +++ b/src/Extend/Filter.php @@ -52,9 +52,13 @@ public function addFilterMutator($callback) public function extend(Container $container, Extension $extension = null) { - foreach ($this->filters as $filter) { - Filterer::addFilter($this->resource, $container->make($filter)); - } + $container->extend('flarum.filter.filters', function ($originalFilters) { + foreach ($this->filters as $filter) { + $originalFilters[$this->resource][] = $filter; + } + + return $originalFilters; + }); foreach ($this->filterMutators as $mutator) { Filterer::addFilterMutator($this->resource, ContainerUtil::wrapCallback($mutator, $container)); diff --git a/src/Filter/FilterInterface.php b/src/Filter/FilterInterface.php index 73b5b29d70..95d96d8c24 100644 --- a/src/Filter/FilterInterface.php +++ b/src/Filter/FilterInterface.php @@ -22,5 +22,5 @@ public function getFilterKey(): string; * @param WrappedFilter $filter * @param string $value The value of the requested filter */ - public function filter(WrappedFilter $wrappedFilter, $filterValue); + public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate); } diff --git a/src/Filter/FilterServiceProvider.php b/src/Filter/FilterServiceProvider.php new file mode 100644 index 0000000000..5f22ed522b --- /dev/null +++ b/src/Filter/FilterServiceProvider.php @@ -0,0 +1,59 @@ +app->singleton('flarum.filter.filters', function() { + return [ + Discussion::class => [ + AuthorFilter::class, + CreatedFilter::class, + HiddenFilter::class, + UnreadFilter::class, + ], + User::class => [ + EmailFilter::class, + GroupFilter::class, + ] + ]; + }); + } + + public function boot() + { + $allFilters = $this->app->make('flarum.filter.filters'); + + foreach ($allFilters as $resource => $resourceFilters) { + foreach ($resourceFilters as $filter) { + $filter = $this->app->make($filter); + Filterer::addFilter($resource, $filter); + } + } + } +} diff --git a/src/Filter/Filterer.php b/src/Filter/Filterer.php index cf67227c71..b3f5775b8a 100644 --- a/src/Filter/Filterer.php +++ b/src/Filter/Filterer.php @@ -27,11 +27,11 @@ public static function addFilter($resource, FilterInterface $filter) static::$filters[$resource] = []; } - if (! array_key_exists($filter->getKey(), static::$filters[$resource])) { - static::$filters[$resource][$filter->getKey()] = []; + if (! array_key_exists($filter->getFilterKey(), static::$filters[$resource])) { + static::$filters[$resource][$filter->getFilterKey()] = []; } - static::$filters[$resource][$filter->getKey()][] = $filter; + static::$filters[$resource][$filter->getFilterKey()][] = $filter; } public static function addFilterMutator($resource, $mutator) @@ -59,8 +59,13 @@ public function filter($actor, $query, $filters, $sort = null, $limit = null, $o $wrappedFilter = new WrappedFilter($query->getQuery(), $actor); foreach ($filters as $filterKey => $filterValue) { + $negate = false; + if (substr($filterKey, 0, 1) == '-') { + $negate = true; + $filterKey = substr($filterKey, 1); + } foreach (Arr::get(static::$filters, "$resource.$filterKey", []) as $filter) { - $filter->apply($wrappedFilter, $filterValue); + $filter->filter($wrappedFilter, $filterValue, $negate); } } diff --git a/src/Foundation/InstalledSite.php b/src/Foundation/InstalledSite.php index 8e944e8277..ec52fb9cc3 100644 --- a/src/Foundation/InstalledSite.php +++ b/src/Foundation/InstalledSite.php @@ -16,6 +16,7 @@ use Flarum\Database\DatabaseServiceProvider; use Flarum\Discussion\DiscussionServiceProvider; use Flarum\Extension\ExtensionServiceProvider; +use Flarum\Filter\FilterServiceProvider; use Flarum\Formatter\FormatterServiceProvider; use Flarum\Forum\ForumServiceProvider; use Flarum\Frontend\FrontendServiceProvider; @@ -117,6 +118,7 @@ protected function bootLaravel(): Container $laravel->register(ExtensionServiceProvider::class); $laravel->register(ErrorServiceProvider::class); $laravel->register(FilesystemServiceProvider::class); + $laravel->register(FilterServiceProvider::class); $laravel->register(FormatterServiceProvider::class); $laravel->register(ForumServiceProvider::class); $laravel->register(FrontendServiceProvider::class); diff --git a/tests/integration/extenders/FilterTest.php b/tests/integration/extenders/FilterTest.php index 7022283335..f9c24d8e29 100644 --- a/tests/integration/extenders/FilterTest.php +++ b/tests/integration/extenders/FilterTest.php @@ -117,7 +117,7 @@ public function getFilterKey(): string /** * {@inheritdoc} */ - public function applyFilter(WrappedFilter $wrappedFilter, $filterValue) + public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) { if ($filterValue) { $wrappedFilter->getQuery() From 8fd3f11e7d3ce10594401555bf096ac8a6f0107d Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 13 Nov 2020 19:07:33 -0500 Subject: [PATCH 04/36] Add tests for all user filters and gambits --- tests/integration/api/users/ListTest.php | 197 +++++++++++++++++++++++ 1 file changed, 197 insertions(+) diff --git a/tests/integration/api/users/ListTest.php b/tests/integration/api/users/ListTest.php index c2b05a9419..91635126c0 100644 --- a/tests/integration/api/users/ListTest.php +++ b/tests/integration/api/users/ListTest.php @@ -11,6 +11,7 @@ use Flarum\Tests\integration\RetrievesAuthorizedUsers; use Flarum\Tests\integration\TestCase; +use Illuminate\Support\Arr; class ListTest extends TestCase { @@ -59,4 +60,200 @@ public function shows_index_for_admin() $this->assertEquals(200, $response->getStatusCode()); } + + /** + * @test + */ + public function shows_full_results_without_search_or_filter() + { + $response = $this->send( + $this->request('GET', '/api/users', [ + 'authenticatedAs' => 1, + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true)['data']; + $this->assertEquals(['1', '2'], Arr::pluck($data, 'id')); + } + + /** + * @test + */ + public function group_filter_works() + { + $response = $this->send( + $this->request('GET', '/api/users', [ + 'authenticatedAs' => 1, + ])->withQueryParams([ + 'filter' => ['group' => '1'], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true)['data']; + $this->assertEquals(['1'], Arr::pluck($data, 'id')); + } + + /** + * @test + */ + public function group_filter_works_negated() + { + $response = $this->send( + $this->request('GET', '/api/users', [ + 'authenticatedAs' => 1, + ])->withQueryParams([ + 'filter' => ['-group' => '1'], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true)['data']; + $this->assertEquals(['2'], Arr::pluck($data, 'id')); + } + + /** + * @test + */ + public function email_filter_works() + { + $response = $this->send( + $this->request('GET', '/api/users', [ + 'authenticatedAs' => 1, + ])->withQueryParams([ + 'filter' => ['email' => 'admin@machine.local'], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true)['data']; + $this->assertEquals(['1'], Arr::pluck($data, 'id')); + } + + /** + * @test + */ + public function email_filter_works_negated() + { + $response = $this->send( + $this->request('GET', '/api/users', [ + 'authenticatedAs' => 1, + ])->withQueryParams([ + 'filter' => ['-email' => 'admin@machine.local'], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true)['data']; + $this->assertEquals(['2'], Arr::pluck($data, 'id')); + } + + /** + * @test + */ + public function email_filter_only_works_for_admin() + { + $response = $this->send( + $this->request('GET', '/api/users', [ + 'authenticatedAs' => 2, + ])->withQueryParams([ + 'filter' => ['email' => 'admin@machine.local'], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true)['data']; + $this->assertEquals(['1', '2'], Arr::pluck($data, 'id')); + } + + /** + * @test + */ + public function group_gambit_works() + { + $response = $this->send( + $this->request('GET', '/api/users', [ + 'authenticatedAs' => 1, + ])->withQueryParams([ + 'filter' => ['q' => 'group:1'], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true)['data']; + $this->assertEquals(['1'], Arr::pluck($data, 'id')); + } + + /** + * @test + */ + public function group_gambit_works_negated() + { + $response = $this->send( + $this->request('GET', '/api/users', [ + 'authenticatedAs' => 1, + ])->withQueryParams([ + 'filter' => ['q' => '-group:1'], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true)['data']; + $this->assertEquals(['2'], Arr::pluck($data, 'id')); + } + + /** + * @test + */ + public function email_gambit_works() + { + $response = $this->send( + $this->request('GET', '/api/users', [ + 'authenticatedAs' => 1, + ])->withQueryParams([ + 'filter' => ['q' => 'email:admin@machine.local'], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true)['data']; + $this->assertEquals(['1'], Arr::pluck($data, 'id')); + } + + /** + * @test + */ + public function email_gambit_works_negated() + { + $response = $this->send( + $this->request('GET', '/api/users', [ + 'authenticatedAs' => 1, + ])->withQueryParams([ + 'filter' => ['q' => '-email:admin@machine.local'], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true)['data']; + $this->assertEquals(['2'], Arr::pluck($data, 'id')); + } + + /** + * @test + */ + public function email_gambit_only_works_for_admin() + { + $response = $this->send( + $this->request('GET', '/api/users', [ + 'authenticatedAs' => 2, + ])->withQueryParams([ + 'filter' => ['q' => 'email:admin@machine.local'], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true)['data']; + $this->assertEquals([], Arr::pluck($data, 'id')); + } } From 34594fcce87130273f9925780fc839de550906e6 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Sat, 14 Nov 2020 02:48:53 -0500 Subject: [PATCH 05/36] Fix created filter --- src/Discussion/Filter/CreatedFilter.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Discussion/Filter/CreatedFilter.php b/src/Discussion/Filter/CreatedFilter.php index 3f40ce6453..fb207644e7 100644 --- a/src/Discussion/Filter/CreatedFilter.php +++ b/src/Discussion/Filter/CreatedFilter.php @@ -23,16 +23,16 @@ public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $ { $pattern = '(\d{4}\-\d\d\-\d\d)(\.\.(\d{4}\-\d\d\-\d\d))?'; - preg_match('/^(-?)'.$pattern.'$/i', $filterValue, $matches); + preg_match('/^'.$pattern.'$/i', $filterValue, $matches); // If we've just been provided with a single YYYY-MM-DD date, then find // discussions that were started on that exact date. But if we've been // provided with a YYYY-MM-DD..YYYY-MM-DD range, then find discussions // that were started during that period. if (empty($matches[2])) { - $wrappedFilter->getQuery()->whereDate('created_at', $negate ? '!=' : '=', $matches[0]); + $wrappedFilter->getQuery()->whereDate('created_at', $negate ? '!=' : '=', $matches[1]); } else { - $wrappedFilter->getQuery()->whereBetween('created_at', [$matches[0], $matches[2]], 'and', $negate); + $wrappedFilter->getQuery()->whereBetween('created_at', [$matches[1], $matches[3]], 'and', $negate); } } From 619913c6d18d458a6e7ba28ea836204bfe41e207 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Sat, 14 Nov 2020 02:49:18 -0500 Subject: [PATCH 06/36] Add tests for all discussion filters and gambits --- .../integration/api/discussions/ListTest.php | 457 +++++++++++++++++- 1 file changed, 451 insertions(+), 6 deletions(-) diff --git a/tests/integration/api/discussions/ListTest.php b/tests/integration/api/discussions/ListTest.php index 48e1ee949f..78aae5aeda 100644 --- a/tests/integration/api/discussions/ListTest.php +++ b/tests/integration/api/discussions/ListTest.php @@ -10,8 +10,12 @@ namespace Flarum\Tests\integration\api\discussions; use Carbon\Carbon; +use Flarum\Discussion\Discussion; +use Flarum\Discussion\UserState; use Flarum\Tests\integration\RetrievesAuthorizedUsers; use Flarum\Tests\integration\TestCase; +use Flarum\User\User; +use Illuminate\Support\Arr; class ListTest extends TestCase { @@ -26,15 +30,26 @@ protected function setUp(): void $this->prepareDatabase([ 'discussions' => [ - ['id' => 1, 'title' => __CLASS__, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'first_post_id' => 1, 'comment_count' => 1], + ['id' => 1, 'title' => __CLASS__, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'last_posted_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'first_post_id' => 1, 'comment_count' => 1], + ['id' => 2, 'title' => 'lightsail in title', 'created_at' => Carbon::createFromDate(1985, 5, 21)->toDateTimeString(), 'last_posted_at' => Carbon::createFromDate(1985, 5, 21)->toDateTimeString(), 'user_id' => 2, 'comment_count' => 1], + ['id' => 3, 'title' => 'not in title', 'created_at' => Carbon::createFromDate(1995, 5, 21)->toDateTimeString(), 'last_posted_at' => Carbon::createFromDate(1995, 5, 21)->toDateTimeString(), 'user_id' => 2, 'comment_count' => 1], + ['id' => 4, 'title' => 'hidden', 'created_at' => Carbon::createFromDate(2005, 5, 21)->toDateTimeString(), 'last_posted_at' => Carbon::createFromDate(2005, 5, 21)->toDateTimeString(), 'hidden_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1], ], 'posts' => [ - ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

foo bar

'], + ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

foo bar

'], + ['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::createFromDate(1985, 5, 21)->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

not in text

'], + ['id' => 3, 'discussion_id' => 3, 'created_at' => Carbon::createFromDate(1995, 5, 21)->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

lightsail in text

'], + ['id' => 4, 'discussion_id' => 4, 'created_at' => Carbon::createFromDate(2005, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

lightsail in text

'], ], 'users' => [ + $this->adminUser(), $this->normalUser(), ] ]); + + $user = User::find(2); + $user->marked_all_as_read_at = Carbon::createFromDate(1990, 0, 0)->toDateTimeString(); + $user->save(); } /** @@ -49,22 +64,452 @@ public function shows_index_for_guest() $this->assertEquals(200, $response->getStatusCode()); $data = json_decode($response->getBody()->getContents(), true); - $this->assertEquals(1, count($data['data'])); + $this->assertEquals(3, count($data['data'])); } /** * @test */ - public function can_search_for_author() + public function can_search_for_word_in_post() { $response = $this->send( $this->request('GET', '/api/discussions') ->withQueryParams([ - 'filter' => ['q' => 'author:normal foo'], + 'filter' => ['q' => 'lightsail'], 'include' => 'mostRelevantPost', ]) ); - $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['3'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function ignores_non_word_characters_when_searching() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['q' => 'lightsail+'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['3'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function search_for_special_characters_gives_empty_result() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['q' => '*'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true); + $this->assertEquals([], $data['data']); + + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['q' => '@'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true); + $this->assertEquals([], $data['data']); + } + + /** + * @test + */ + public function author_filter_works() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['author' => 'normal'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['2', '3'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function author_filter_works_negated() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['-author' => 'normal'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['1'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function created_filter_works_with_date() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['created' => '1995-05-21'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['3'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function created_filter_works_negated_with_date() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['-created' => '1995-05-21'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['1', '2'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function created_filter_works_with_range() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['created' => '1980-05-21..2000-05-21'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['2', '3'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function created_filter_works_negated_with_range() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['-created' => '1980-05-21..2000-05-21'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['1'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function hidden_filter_works() + { + $response = $this->send( + $this->request('GET', '/api/discussions', ['authenticatedAs' => 1]) + ->withQueryParams([ + 'filter' => ['hidden' => ''], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['4'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function hidden_filter_works_negated() + { + $response = $this->send( + $this->request('GET','/api/discussions', ['authenticatedAs' => 1]) + ->withQueryParams([ + 'filter' => ['-hidden' => ''], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['1', '2', '3'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function unread_filter_works() + { + $response = $this->send( + $this->request('GET', '/api/discussions', ['authenticatedAs' => 2]) + ->withQueryParams([ + 'filter' => ['unread' => ''], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['3'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function unread_filter_works_when_negated() + { + $response = $this->send( + $this->request('GET', '/api/discussions', ['authenticatedAs' => 2]) + ->withQueryParams([ + 'filter' => ['-unread' => ''], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['1', '2'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function author_gambit_works() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['q' => 'author:normal'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['2', '3'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function author_gambit_works_negated() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['q' => '-author:normal'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['1'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function created_gambit_works_with_date() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['q' => 'created:1995-05-21'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['3'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function created_gambit_works_negated_with_date() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['q' => '-created:1995-05-21'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['1', '2'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function created_gambit_works_with_range() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['q' => 'created:1980-05-21..2000-05-21'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['2', '3'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function created_gambit_works_negated_with_range() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['q' => '-created:1980-05-21..2000-05-21'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['1'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function hidden_gambit_works() + { + $response = $this->send( + $this->request('GET', '/api/discussions', ['authenticatedAs' => 1]) + ->withQueryParams([ + 'filter' => ['q' => 'is:hidden'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['4'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function hidden_gambit_works_negated() + { + $response = $this->send( + $this->request('GET', '/api/discussions', ['authenticatedAs' => 1]) + ->withQueryParams([ + 'filter' => ['q' => '-is:hidden'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['1', '2', '3'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function unread_gambit_works() + { + $response = $this->send( + $this->request('GET', '/api/discussions', ['authenticatedAs' => 2]) + ->withQueryParams([ + 'filter' => ['q' => 'is:unread'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['3'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function unread_gambit_works_when_negated() + { + $response = $this->send( + $this->request('GET', '/api/discussions', ['authenticatedAs' => 2]) + ->withQueryParams([ + 'filter' => ['q' => '-is:unread'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true)['data']; + + // Order-independent comparison + $this->assertEquals(['1', '2'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); } } From dd61e4404fd998b8fac117c77213bd108351d8cf Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Sat, 14 Nov 2020 07:49:41 +0000 Subject: [PATCH 07/36] Apply fixes from StyleCI [ci skip] [skip ci] --- src/Api/Controller/ListDiscussionsController.php | 1 - src/Api/Controller/ListUsersController.php | 1 - src/Discussion/Filter/AuthorFilter.php | 1 - src/Discussion/Filter/CreatedFilter.php | 1 - src/Discussion/Filter/UnreadFilter.php | 3 +-- src/Filter/FilterServiceProvider.php | 3 +-- src/User/Filter/EmailFilter.php | 2 +- tests/integration/api/discussions/ListTest.php | 4 +--- 8 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/Api/Controller/ListDiscussionsController.php b/src/Api/Controller/ListDiscussionsController.php index a030c185bb..ab7a3d3c50 100644 --- a/src/Api/Controller/ListDiscussionsController.php +++ b/src/Api/Controller/ListDiscussionsController.php @@ -16,7 +16,6 @@ use Flarum\Filter\Filterer; use Flarum\Http\UrlGenerator; use Flarum\Search\SearchCriteria; -use Illuminate\Support\Arr; use Psr\Http\Message\ServerRequestInterface; use Tobscure\JsonApi\Document; diff --git a/src/Api/Controller/ListUsersController.php b/src/Api/Controller/ListUsersController.php index 4fe4cd3c60..ff09cfb358 100644 --- a/src/Api/Controller/ListUsersController.php +++ b/src/Api/Controller/ListUsersController.php @@ -15,7 +15,6 @@ use Flarum\Search\SearchCriteria; use Flarum\User\Search\UserSearcher; use Flarum\User\UserRepository; -use Illuminate\Support\Arr; use Psr\Http\Message\ServerRequestInterface; use Tobscure\JsonApi\Document; diff --git a/src/Discussion/Filter/AuthorFilter.php b/src/Discussion/Filter/AuthorFilter.php index 9fea602dcf..762b781cc3 100644 --- a/src/Discussion/Filter/AuthorFilter.php +++ b/src/Discussion/Filter/AuthorFilter.php @@ -44,6 +44,5 @@ public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $ } $wrappedFilter->getQuery()->whereIn('discussions.user_id', $ids, 'and', $negate); - } } diff --git a/src/Discussion/Filter/CreatedFilter.php b/src/Discussion/Filter/CreatedFilter.php index fb207644e7..d56268a528 100644 --- a/src/Discussion/Filter/CreatedFilter.php +++ b/src/Discussion/Filter/CreatedFilter.php @@ -34,6 +34,5 @@ public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $ } else { $wrappedFilter->getQuery()->whereBetween('created_at', [$matches[1], $matches[3]], 'and', $negate); } - } } diff --git a/src/Discussion/Filter/UnreadFilter.php b/src/Discussion/Filter/UnreadFilter.php index ec192c6893..4986bd160a 100644 --- a/src/Discussion/Filter/UnreadFilter.php +++ b/src/Discussion/Filter/UnreadFilter.php @@ -33,7 +33,6 @@ public function __construct(DiscussionRepository $discussions) $this->discussions = $discussions; } - public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) { $actor = $wrappedFilter->getActor(); @@ -42,7 +41,7 @@ public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $ $readIds = $this->discussions->getReadIds($actor); $wrappedFilter->getQuery()->where(function ($query) use ($readIds, $negate, $actor) { - if (!$negate) { + if (! $negate) { $query->whereNotIn('id', $readIds)->where('last_posted_at', '>', $actor->marked_all_as_read_at ?: 0); } else { $query->whereIn('id', $readIds)->orWhere('last_posted_at', '<=', $actor->marked_all_as_read_at ?: 0); diff --git a/src/Filter/FilterServiceProvider.php b/src/Filter/FilterServiceProvider.php index 5f22ed522b..6aff4e485b 100644 --- a/src/Filter/FilterServiceProvider.php +++ b/src/Filter/FilterServiceProvider.php @@ -14,7 +14,6 @@ use Flarum\Discussion\Filter\CreatedFilter; use Flarum\Discussion\Filter\HiddenFilter; use Flarum\Discussion\Filter\UnreadFilter; -use Flarum\Filter\Filterer; use Flarum\Foundation\AbstractServiceProvider; use Flarum\User\Filter\EmailFilter; use Flarum\User\Filter\GroupFilter; @@ -29,7 +28,7 @@ class FilterServiceProvider extends AbstractServiceProvider */ public function register() { - $this->app->singleton('flarum.filter.filters', function() { + $this->app->singleton('flarum.filter.filters', function () { return [ Discussion::class => [ AuthorFilter::class, diff --git a/src/User/Filter/EmailFilter.php b/src/User/Filter/EmailFilter.php index 9fe8128f72..7d2e81692c 100644 --- a/src/User/Filter/EmailFilter.php +++ b/src/User/Filter/EmailFilter.php @@ -21,7 +21,7 @@ public function getFilterKey(): string public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) { - if (!$wrappedFilter->getActor()->hasPermission('user.edit')) { + if (! $wrappedFilter->getActor()->hasPermission('user.edit')) { return; } diff --git a/tests/integration/api/discussions/ListTest.php b/tests/integration/api/discussions/ListTest.php index 78aae5aeda..e3d983c065 100644 --- a/tests/integration/api/discussions/ListTest.php +++ b/tests/integration/api/discussions/ListTest.php @@ -10,8 +10,6 @@ namespace Flarum\Tests\integration\api\discussions; use Carbon\Carbon; -use Flarum\Discussion\Discussion; -use Flarum\Discussion\UserState; use Flarum\Tests\integration\RetrievesAuthorizedUsers; use Flarum\Tests\integration\TestCase; use Flarum\User\User; @@ -272,7 +270,7 @@ public function hidden_filter_works() public function hidden_filter_works_negated() { $response = $this->send( - $this->request('GET','/api/discussions', ['authenticatedAs' => 1]) + $this->request('GET', '/api/discussions', ['authenticatedAs' => 1]) ->withQueryParams([ 'filter' => ['-hidden' => ''], 'include' => 'mostRelevantPost', From 44e74316d0155f64a6666bdd0b1cc0d52a799e03 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Tue, 19 Jan 2021 18:41:15 -0500 Subject: [PATCH 08/36] Fix failing tests --- .../integration/api/discussions/ListTest.php | 84 ++++--------------- tests/integration/api/users/ListTest.php | 14 ++++ tests/integration/extenders/FilterTest.php | 1 - 3 files changed, 31 insertions(+), 68 deletions(-) diff --git a/tests/integration/api/discussions/ListTest.php b/tests/integration/api/discussions/ListTest.php index e3d983c065..b97a79b955 100644 --- a/tests/integration/api/discussions/ListTest.php +++ b/tests/integration/api/discussions/ListTest.php @@ -40,11 +40,15 @@ protected function setUp(): void ['id' => 4, 'discussion_id' => 4, 'created_at' => Carbon::createFromDate(2005, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

lightsail in text

'], ], 'users' => [ - $this->adminUser(), $this->normalUser(), ] ]); + } + /** + * Mark some discussions, but not others, as read to test that filter/gambit. + */ + protected function read() { $user = User::find(2); $user->marked_all_as_read_at = Carbon::createFromDate(1990, 0, 0)->toDateTimeString(); $user->save(); @@ -65,72 +69,6 @@ public function shows_index_for_guest() $this->assertEquals(3, count($data['data'])); } - /** - * @test - */ - public function can_search_for_word_in_post() - { - $response = $this->send( - $this->request('GET', '/api/discussions') - ->withQueryParams([ - 'filter' => ['q' => 'lightsail'], - 'include' => 'mostRelevantPost', - ]) - ); - - $data = json_decode($response->getBody()->getContents(), true)['data']; - - // Order-independent comparison - $this->assertEquals(['3'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); - } - - /** - * @test - */ - public function ignores_non_word_characters_when_searching() - { - $response = $this->send( - $this->request('GET', '/api/discussions') - ->withQueryParams([ - 'filter' => ['q' => 'lightsail+'], - 'include' => 'mostRelevantPost', - ]) - ); - - $data = json_decode($response->getBody()->getContents(), true)['data']; - - // Order-independent comparison - $this->assertEquals(['3'], Arr::pluck($data, 'id'), 'IDs do not match', 0.0, 10, true); - } - - /** - * @test - */ - public function search_for_special_characters_gives_empty_result() - { - $response = $this->send( - $this->request('GET', '/api/discussions') - ->withQueryParams([ - 'filter' => ['q' => '*'], - 'include' => 'mostRelevantPost', - ]) - ); - - $data = json_decode($response->getBody()->getContents(), true); - $this->assertEquals([], $data['data']); - - $response = $this->send( - $this->request('GET', '/api/discussions') - ->withQueryParams([ - 'filter' => ['q' => '@'], - 'include' => 'mostRelevantPost', - ]) - ); - - $data = json_decode($response->getBody()->getContents(), true); - $this->assertEquals([], $data['data']); - } - /** * @test */ @@ -288,6 +226,9 @@ public function hidden_filter_works_negated() */ public function unread_filter_works() { + $this->app(); + $this->read(); + $response = $this->send( $this->request('GET', '/api/discussions', ['authenticatedAs' => 2]) ->withQueryParams([ @@ -307,6 +248,9 @@ public function unread_filter_works() */ public function unread_filter_works_when_negated() { + $this->app(); + $this->read(); + $response = $this->send( $this->request('GET', '/api/discussions', ['authenticatedAs' => 2]) ->withQueryParams([ @@ -478,6 +422,9 @@ public function hidden_gambit_works_negated() */ public function unread_gambit_works() { + $this->app(); + $this->read(); + $response = $this->send( $this->request('GET', '/api/discussions', ['authenticatedAs' => 2]) ->withQueryParams([ @@ -497,6 +444,9 @@ public function unread_gambit_works() */ public function unread_gambit_works_when_negated() { + $this->app(); + $this->read(); + $response = $this->send( $this->request('GET', '/api/discussions', ['authenticatedAs' => 2]) ->withQueryParams([ diff --git a/tests/integration/api/users/ListTest.php b/tests/integration/api/users/ListTest.php index 91635126c0..72199e3b48 100644 --- a/tests/integration/api/users/ListTest.php +++ b/tests/integration/api/users/ListTest.php @@ -17,6 +17,20 @@ class ListTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ + protected function setUp(): void + { + parent::setUp(); + + $this->prepareDatabase([ + 'users' => [ + $this->normalUser(), + ], + ]); + } + /** * @test */ diff --git a/tests/integration/extenders/FilterTest.php b/tests/integration/extenders/FilterTest.php index f9c24d8e29..678bd475ce 100644 --- a/tests/integration/extenders/FilterTest.php +++ b/tests/integration/extenders/FilterTest.php @@ -33,7 +33,6 @@ public function prepDb() ['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

foo bar not the same

'], ], 'users' => [ - $this->adminUser(), $this->normalUser(), ], ]); From f644c698548c1872cbca1cba2be7c79cbdad0f38 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Tue, 19 Jan 2021 23:41:32 +0000 Subject: [PATCH 09/36] Apply fixes from StyleCI [ci skip] [skip ci] --- tests/integration/api/discussions/ListTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/api/discussions/ListTest.php b/tests/integration/api/discussions/ListTest.php index b97a79b955..9a16f411f3 100644 --- a/tests/integration/api/discussions/ListTest.php +++ b/tests/integration/api/discussions/ListTest.php @@ -48,7 +48,8 @@ protected function setUp(): void /** * Mark some discussions, but not others, as read to test that filter/gambit. */ - protected function read() { + protected function read() + { $user = User::find(2); $user->marked_all_as_read_at = Carbon::createFromDate(1990, 0, 0)->toDateTimeString(); $user->save(); From 7b409cb1ec8c1cea181de8f6eb22c17d49851254 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 27 Jan 2021 23:02:21 -0500 Subject: [PATCH 10/36] Add types --- src/Filter/Filterer.php | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Filter/Filterer.php b/src/Filter/Filterer.php index b3f5775b8a..b75fca583c 100644 --- a/src/Filter/Filterer.php +++ b/src/Filter/Filterer.php @@ -11,7 +11,10 @@ use Flarum\Search\ApplySearchParametersTrait; use Flarum\Search\SearchResults; +use Flarum\User\User; +use Illuminate\Database\Query\Builder; use Illuminate\Support\Arr; +use InvalidArgumentException; class Filterer { @@ -44,13 +47,17 @@ public static function addFilterMutator($resource, $mutator) } /** - * @param FilterCriteria $criteria - * @param int|null $limit + * @param User $actor + * @param Builder $query + * @param array $filters + * @param mixed|null $sort + * @param mixed|null $limit * @param int $offset - * - * @return FilterResults + * @param array $load + * @return SearchResults + * @throws InvalidArgumentException */ - public function filter($actor, $query, $filters, $sort = null, $limit = null, $offset = 0, array $load = []) + public function filter(User $actor, Builder $query, array $filters, array $sort = null, int $limit = null, int $offset = 0, array $load = []): SearchResults { $resource = get_class($query->getModel()); From e9702828be2c1b916e1252c116968decab56222c Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Mon, 1 Feb 2021 13:05:19 -0500 Subject: [PATCH 11/36] Expect eloquent builder --- src/Filter/Filterer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Filter/Filterer.php b/src/Filter/Filterer.php index b75fca583c..692ad75661 100644 --- a/src/Filter/Filterer.php +++ b/src/Filter/Filterer.php @@ -12,7 +12,7 @@ use Flarum\Search\ApplySearchParametersTrait; use Flarum\Search\SearchResults; use Flarum\User\User; -use Illuminate\Database\Query\Builder; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Arr; use InvalidArgumentException; From 6ccd7a560a7b4b50297c16525c591d1094f8f9f0 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Mon, 1 Feb 2021 13:50:11 -0500 Subject: [PATCH 12/36] Merge gambits into "FilterGambit" --- ...uthorFilter.php => AuthorFilterGambit.php} | 27 ++++++- src/Discussion/Filter/CreatedFilter.php | 38 --------- src/Discussion/Filter/CreatedFilterGambit.php | 57 ++++++++++++++ ...iddenFilter.php => HiddenFilterGambit.php} | 25 +++++- ...nreadFilter.php => UnreadFilterGambit.php} | 36 +++++++-- src/Discussion/Search/Gambit/AuthorGambit.php | 57 -------------- .../Search/Gambit/CreatedGambit.php | 43 ----------- src/Discussion/Search/Gambit/HiddenGambit.php | 41 ---------- src/Discussion/Search/Gambit/UnreadGambit.php | 61 --------------- src/Filter/FilterServiceProvider.php | 24 +++--- src/Search/SearchServiceProvider.php | 24 +++--- src/User/Filter/EmailFilter.php | 32 -------- src/User/Filter/EmailFilterGambit.php | 65 ++++++++++++++++ src/User/Filter/GroupFilter.php | 43 ----------- src/User/Filter/GroupFilterGambit.php | 64 +++++++++++++++ src/User/Search/Gambit/EmailGambit.php | 63 --------------- src/User/Search/Gambit/GroupGambit.php | 77 ------------------- 17 files changed, 285 insertions(+), 492 deletions(-) rename src/Discussion/Filter/{AuthorFilter.php => AuthorFilterGambit.php} (55%) delete mode 100644 src/Discussion/Filter/CreatedFilter.php create mode 100644 src/Discussion/Filter/CreatedFilterGambit.php rename src/Discussion/Filter/{HiddenFilter.php => HiddenFilterGambit.php} (51%) rename src/Discussion/Filter/{UnreadFilter.php => UnreadFilterGambit.php} (62%) delete mode 100644 src/Discussion/Search/Gambit/AuthorGambit.php delete mode 100644 src/Discussion/Search/Gambit/CreatedGambit.php delete mode 100644 src/Discussion/Search/Gambit/HiddenGambit.php delete mode 100644 src/Discussion/Search/Gambit/UnreadGambit.php delete mode 100644 src/User/Filter/EmailFilter.php create mode 100644 src/User/Filter/EmailFilterGambit.php delete mode 100644 src/User/Filter/GroupFilter.php create mode 100644 src/User/Filter/GroupFilterGambit.php delete mode 100644 src/User/Search/Gambit/EmailGambit.php delete mode 100644 src/User/Search/Gambit/GroupGambit.php diff --git a/src/Discussion/Filter/AuthorFilter.php b/src/Discussion/Filter/AuthorFilterGambit.php similarity index 55% rename from src/Discussion/Filter/AuthorFilter.php rename to src/Discussion/Filter/AuthorFilterGambit.php index 762b781cc3..f2662985e7 100644 --- a/src/Discussion/Filter/AuthorFilter.php +++ b/src/Discussion/Filter/AuthorFilterGambit.php @@ -11,9 +11,12 @@ use Flarum\Filter\FilterInterface; use Flarum\Filter\WrappedFilter; +use Flarum\Search\AbstractRegexGambit; +use Flarum\Search\AbstractSearch; use Flarum\User\UserRepository; +use Illuminate\Database\Query\Builder; -class AuthorFilter implements FilterInterface +class AuthorFilterGambit extends AbstractRegexGambit implements FilterInterface { /** * @var \Flarum\User\UserRepository @@ -28,6 +31,19 @@ public function __construct(UserRepository $users) $this->users = $users; } + /** + * {@inheritdoc} + */ + protected $pattern = 'author:(.+)'; + + /** + * {@inheritdoc} + */ + protected function conditions(AbstractSearch $search, array $matches, $negate) + { + $this->constrain($search->getQuery(), $matches[1], $negate); + } + public function getFilterKey(): string { return 'author'; @@ -35,7 +51,12 @@ public function getFilterKey(): string public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) { - $usernames = trim($filterValue, '"'); + $this->constrain($wrappedFilter->getQuery(), $filterValue, $negate); + } + + protected function constrain(Builder $query, $rawUsernames, $negate) + { + $usernames = trim($rawUsernames, '"'); $usernames = explode(',', $usernames); $ids = []; @@ -43,6 +64,6 @@ public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $ $ids[] = $this->users->getIdForUsername($username); } - $wrappedFilter->getQuery()->whereIn('discussions.user_id', $ids, 'and', $negate); + $query->whereIn('discussions.user_id', $ids, 'and', $negate); } } diff --git a/src/Discussion/Filter/CreatedFilter.php b/src/Discussion/Filter/CreatedFilter.php deleted file mode 100644 index d56268a528..0000000000 --- a/src/Discussion/Filter/CreatedFilter.php +++ /dev/null @@ -1,38 +0,0 @@ -getQuery()->whereDate('created_at', $negate ? '!=' : '=', $matches[1]); - } else { - $wrappedFilter->getQuery()->whereBetween('created_at', [$matches[1], $matches[3]], 'and', $negate); - } - } -} diff --git a/src/Discussion/Filter/CreatedFilterGambit.php b/src/Discussion/Filter/CreatedFilterGambit.php new file mode 100644 index 0000000000..3de9f8a74d --- /dev/null +++ b/src/Discussion/Filter/CreatedFilterGambit.php @@ -0,0 +1,57 @@ +constrain($search->getQuery(), $matches[1], $matches[3], $negate); + } + + public function getFilterKey(): string + { + return 'created'; + } + + public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) + { + preg_match('/^'.$this->pattern.'$/i', 'created:'.$filterValue, $matches); + + $this->constrain($wrappedFilter->getQuery(), $matches[1], $matches[3], $negate); + } + + public function constrain(Builder $query, $firstDate, $secondDate, $negate) + { + // If we've just been provided with a single YYYY-MM-DD date, then find + // discussions that were started on that exact date. But if we've been + // provided with a YYYY-MM-DD..YYYY-MM-DD range, then find discussions + // that were started during that period. + if (empty($secondDate)) { + $query->whereDate('created_at', $negate ? '!=' : '=', $firstDate); + } else { + $query->whereBetween('created_at', [$firstDate, $secondDate], 'and', $negate); + } + } +} diff --git a/src/Discussion/Filter/HiddenFilter.php b/src/Discussion/Filter/HiddenFilterGambit.php similarity index 51% rename from src/Discussion/Filter/HiddenFilter.php rename to src/Discussion/Filter/HiddenFilterGambit.php index 450ec425ed..3ff2c59372 100644 --- a/src/Discussion/Filter/HiddenFilter.php +++ b/src/Discussion/Filter/HiddenFilterGambit.php @@ -11,9 +11,25 @@ use Flarum\Filter\FilterInterface; use Flarum\Filter\WrappedFilter; +use Flarum\Search\AbstractRegexGambit; +use Flarum\Search\AbstractSearch; +use Illuminate\Database\Query\Builder; -class HiddenFilter implements FilterInterface +class HiddenFilterGambit extends AbstractRegexGambit implements FilterInterface { + /** + * {@inheritdoc} + */ + protected $pattern = 'is:hidden'; + + /** + * {@inheritdoc} + */ + protected function conditions(AbstractSearch $search, array $matches, $negate) + { + $this->constrain($search->getQuery(), $negate); + } + public function getFilterKey(): string { return 'hidden'; @@ -21,7 +37,12 @@ public function getFilterKey(): string public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) { - $wrappedFilter->getQuery()->where(function ($query) use ($negate) { + $this->constrain($wrappedFilter->getQuery(), $negate); + } + + protected function constrain(Builder $query, bool $negate) + { + $query->where(function ($query) use ($negate) { if ($negate) { $query->whereNull('hidden_at')->where('comment_count', '>', 0); } else { diff --git a/src/Discussion/Filter/UnreadFilter.php b/src/Discussion/Filter/UnreadFilterGambit.php similarity index 62% rename from src/Discussion/Filter/UnreadFilter.php rename to src/Discussion/Filter/UnreadFilterGambit.php index 4986bd160a..906c3d70fa 100644 --- a/src/Discussion/Filter/UnreadFilter.php +++ b/src/Discussion/Filter/UnreadFilterGambit.php @@ -12,14 +12,13 @@ use Flarum\Discussion\DiscussionRepository; use Flarum\Filter\FilterInterface; use Flarum\Filter\WrappedFilter; +use Flarum\Search\AbstractRegexGambit; +use Flarum\Search\AbstractSearch; +use Flarum\User\User; +use Illuminate\Database\Query\Builder; -class UnreadFilter implements FilterInterface +class UnreadFilterGambit extends AbstractRegexGambit implements FilterInterface { - public function getFilterKey(): string - { - return 'unread'; - } - /** * @var \Flarum\Discussion\DiscussionRepository */ @@ -33,14 +32,35 @@ public function __construct(DiscussionRepository $discussions) $this->discussions = $discussions; } + /** + * {@inheritdoc} + */ + protected $pattern = 'is:unread'; + + /** + * {@inheritdoc} + */ + protected function conditions(AbstractSearch $search, array $matches, $negate) + { + $this->constrain($search->getQuery(), $search->getActor(), $negate); + } + + public function getFilterKey(): string + { + return 'unread'; + } + public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) { - $actor = $wrappedFilter->getActor(); + $this->constrain($wrappedFilter->getQuery(), $wrappedFilter->getActor(), $negate); + } + protected function constrain(Builder $query, User $actor, bool $negate) + { if ($actor->exists) { $readIds = $this->discussions->getReadIds($actor); - $wrappedFilter->getQuery()->where(function ($query) use ($readIds, $negate, $actor) { + $query->where(function ($query) use ($readIds, $negate, $actor) { if (! $negate) { $query->whereNotIn('id', $readIds)->where('last_posted_at', '>', $actor->marked_all_as_read_at ?: 0); } else { diff --git a/src/Discussion/Search/Gambit/AuthorGambit.php b/src/Discussion/Search/Gambit/AuthorGambit.php deleted file mode 100644 index 819a8885bb..0000000000 --- a/src/Discussion/Search/Gambit/AuthorGambit.php +++ /dev/null @@ -1,57 +0,0 @@ -users = $users; - } - - /** - * {@inheritdoc} - */ - protected function conditions(AbstractSearch $search, array $matches, $negate) - { - if (! $search instanceof DiscussionSearch) { - throw new LogicException('This gambit can only be applied on a DiscussionSearch'); - } - - $usernames = trim($matches[1], '"'); - $usernames = explode(',', $usernames); - - $ids = []; - foreach ($usernames as $username) { - $ids[] = $this->users->getIdForUsername($username); - } - - $search->getQuery()->whereIn('discussions.user_id', $ids, 'and', $negate); - } -} diff --git a/src/Discussion/Search/Gambit/CreatedGambit.php b/src/Discussion/Search/Gambit/CreatedGambit.php deleted file mode 100644 index 3d24d5f0f5..0000000000 --- a/src/Discussion/Search/Gambit/CreatedGambit.php +++ /dev/null @@ -1,43 +0,0 @@ -getQuery()->whereDate('created_at', $negate ? '!=' : '=', $matches[1]); - } else { - $search->getQuery()->whereBetween('created_at', [$matches[1], $matches[3]], 'and', $negate); - } - } -} diff --git a/src/Discussion/Search/Gambit/HiddenGambit.php b/src/Discussion/Search/Gambit/HiddenGambit.php deleted file mode 100644 index e658ba0e6e..0000000000 --- a/src/Discussion/Search/Gambit/HiddenGambit.php +++ /dev/null @@ -1,41 +0,0 @@ -getQuery()->where(function ($query) use ($negate) { - if ($negate) { - $query->whereNull('hidden_at')->where('comment_count', '>', 0); - } else { - $query->whereNotNull('hidden_at')->orWhere('comment_count', 0); - } - }); - } -} diff --git a/src/Discussion/Search/Gambit/UnreadGambit.php b/src/Discussion/Search/Gambit/UnreadGambit.php deleted file mode 100644 index 33aac7c91b..0000000000 --- a/src/Discussion/Search/Gambit/UnreadGambit.php +++ /dev/null @@ -1,61 +0,0 @@ -discussions = $discussions; - } - - /** - * {@inheritdoc} - */ - protected function conditions(AbstractSearch $search, array $matches, $negate) - { - if (! $search instanceof DiscussionSearch) { - throw new LogicException('This gambit can only be applied on a DiscussionSearch'); - } - - $actor = $search->getActor(); - - if ($actor->exists) { - $readIds = $this->discussions->getReadIds($actor); - - $search->getQuery()->where(function ($query) use ($readIds, $negate, $actor) { - if (! $negate) { - $query->whereNotIn('id', $readIds)->where('last_posted_at', '>', $actor->marked_all_as_read_at ?: 0); - } else { - $query->whereIn('id', $readIds)->orWhere('last_posted_at', '<=', $actor->marked_all_as_read_at ?: 0); - } - }); - } - } -} diff --git a/src/Filter/FilterServiceProvider.php b/src/Filter/FilterServiceProvider.php index 6aff4e485b..c594c0fe41 100644 --- a/src/Filter/FilterServiceProvider.php +++ b/src/Filter/FilterServiceProvider.php @@ -10,13 +10,13 @@ namespace Flarum\Filter; use Flarum\Discussion\Discussion; -use Flarum\Discussion\Filter\AuthorFilter; -use Flarum\Discussion\Filter\CreatedFilter; -use Flarum\Discussion\Filter\HiddenFilter; -use Flarum\Discussion\Filter\UnreadFilter; +use Flarum\Discussion\Filter\AuthorFilterGambit; +use Flarum\Discussion\Filter\CreatedFilterGambit; +use Flarum\Discussion\Filter\HiddenFilterGambit; +use Flarum\Discussion\Filter\UnreadFilterGambit; use Flarum\Foundation\AbstractServiceProvider; -use Flarum\User\Filter\EmailFilter; -use Flarum\User\Filter\GroupFilter; +use Flarum\User\Filter\EmailFilterGambit; +use Flarum\User\Filter\GroupFilterGambit; use Flarum\User\User; class FilterServiceProvider extends AbstractServiceProvider @@ -31,14 +31,14 @@ public function register() $this->app->singleton('flarum.filter.filters', function () { return [ Discussion::class => [ - AuthorFilter::class, - CreatedFilter::class, - HiddenFilter::class, - UnreadFilter::class, + AuthorFilterGambit::class, + CreatedFilterGambit::class, + HiddenFilterGambit::class, + UnreadFilterGambit::class, ], User::class => [ - EmailFilter::class, - GroupFilter::class, + EmailFilterGambit::class, + GroupFilterGambit::class, ] ]; }); diff --git a/src/Search/SearchServiceProvider.php b/src/Search/SearchServiceProvider.php index fee38bf955..264958f92b 100644 --- a/src/Search/SearchServiceProvider.php +++ b/src/Search/SearchServiceProvider.php @@ -9,19 +9,19 @@ namespace Flarum\Search; +use Flarum\Discussion\Filter\AuthorFilterGambit; +use Flarum\Discussion\Filter\CreatedFilterGambit; +use Flarum\Discussion\Filter\HiddenFilterGambit; +use Flarum\Discussion\Filter\UnreadFilterGambit; use Flarum\Discussion\Search\DiscussionSearcher; -use Flarum\Discussion\Search\Gambit\AuthorGambit; -use Flarum\Discussion\Search\Gambit\CreatedGambit; use Flarum\Discussion\Search\Gambit\FulltextGambit as DiscussionFulltextGambit; -use Flarum\Discussion\Search\Gambit\HiddenGambit; -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\Filter\EmailFilterGambit; +use Flarum\User\Filter\GroupFilterGambit; use Flarum\User\Search\Gambit\FulltextGambit as UserFulltextGambit; -use Flarum\User\Search\Gambit\GroupGambit; use Flarum\User\Search\UserSearcher; use Illuminate\Support\Arr; @@ -42,14 +42,14 @@ public function register() $this->app->singleton('flarum.simple_search.gambits', function () { return [ DiscussionSearcher::class => [ - AuthorGambit::class, - CreatedGambit::class, - HiddenGambit::class, - UnreadGambit::class + AuthorFilterGambit::class, + CreatedFilterGambit::class, + HiddenFilterGambit::class, + UnreadFilterGambit::class ], UserSearcher::class => [ - EmailGambit::class, - GroupGambit::class + EmailFilterGambit::class, + GroupFilterGambit::class ] ]; }); diff --git a/src/User/Filter/EmailFilter.php b/src/User/Filter/EmailFilter.php deleted file mode 100644 index 7d2e81692c..0000000000 --- a/src/User/Filter/EmailFilter.php +++ /dev/null @@ -1,32 +0,0 @@ -getActor()->hasPermission('user.edit')) { - return; - } - - $email = trim($filterValue, '"'); - - $wrappedFilter->getQuery()->where('email', $negate ? '!=' : '=', $email); - } -} diff --git a/src/User/Filter/EmailFilterGambit.php b/src/User/Filter/EmailFilterGambit.php new file mode 100644 index 0000000000..666f40466d --- /dev/null +++ b/src/User/Filter/EmailFilterGambit.php @@ -0,0 +1,65 @@ +getActor()->hasPermission('user.edit')) { + return false; + } + + return parent::apply($search, $bit); + } + + /** + * {@inheritdoc} + */ + protected function conditions(AbstractSearch $search, array $matches, $negate) + { + $this->constrain($search->getQuery(), $matches[1], $negate); + } + + public function getFilterKey(): string + { + return 'email'; + } + + public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) + { + if (! $wrappedFilter->getActor()->hasPermission('user.edit')) { + return; + } + + $this->constrain($wrappedFilter->getQuery(), $filterValue, $negate); + } + + protected function constrain(Builder $query, $rawEmail, bool $negate) + { + $email = trim($rawEmail, '"'); + + $query->where('email', $negate ? '!=' : '=', $email); + } +} diff --git a/src/User/Filter/GroupFilter.php b/src/User/Filter/GroupFilter.php deleted file mode 100644 index 85cbe61a55..0000000000 --- a/src/User/Filter/GroupFilter.php +++ /dev/null @@ -1,43 +0,0 @@ -getActor()); - - foreach ($groupIdentifiers as $identifier) { - if (is_numeric($identifier)) { - $groupQuery->orWhere('id', $identifier); - } else { - $groupQuery->orWhere('name_singular', $identifier)->orWhere('name_plural', $identifier); - } - } - - $userIds = $groupQuery->join('group_user', 'groups.id', 'group_user.group_id') - ->pluck('group_user.user_id') - ->all(); - - $wrappedFilter->getQuery()->whereIn('id', $userIds, 'and', $negate); - } -} diff --git a/src/User/Filter/GroupFilterGambit.php b/src/User/Filter/GroupFilterGambit.php new file mode 100644 index 0000000000..ed03dae4fd --- /dev/null +++ b/src/User/Filter/GroupFilterGambit.php @@ -0,0 +1,64 @@ +constrain($search->getQuery(), $search->getActor(), $matches[1], $negate); + } + + public function getFilterKey(): string + { + return 'group'; + } + + public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) + { + $this->constrain($wrappedFilter->getQuery(), $wrappedFilter->getActor(), $filterValue, $negate); + } + + protected function constrain(Builder $query, User $actor, string $rawQuery, bool $negate) + { + $groupIdentifiers = explode(',', trim($rawQuery, '"')); + + $groupQuery = Group::whereVisibleTo($actor); + + foreach ($groupIdentifiers as $identifier) { + if (is_numeric($identifier)) { + $groupQuery->orWhere('id', $identifier); + } else { + $groupQuery->orWhere('name_singular', $identifier)->orWhere('name_plural', $identifier); + } + } + + $userIds = $groupQuery->join('group_user', 'groups.id', 'group_user.group_id') + ->pluck('group_user.user_id') + ->all(); + + $query->whereIn('id', $userIds, 'and', $negate); + } +} diff --git a/src/User/Search/Gambit/EmailGambit.php b/src/User/Search/Gambit/EmailGambit.php deleted file mode 100644 index f3ca088b6e..0000000000 --- a/src/User/Search/Gambit/EmailGambit.php +++ /dev/null @@ -1,63 +0,0 @@ -users = $users; - } - - /** - * {@inheritdoc} - */ - public function apply(AbstractSearch $search, $bit) - { - if (! $search->getActor()->hasPermission('user.edit')) { - return false; - } - - return parent::apply($search, $bit); - } - - /** - * {@inheritdoc} - */ - protected function conditions(AbstractSearch $search, array $matches, $negate) - { - if (! $search instanceof UserSearch) { - throw new LogicException('This gambit can only be applied on a UserSearch'); - } - - $email = trim($matches[1], '"'); - - $search->getQuery()->where('email', $negate ? '!=' : '=', $email); - } -} diff --git a/src/User/Search/Gambit/GroupGambit.php b/src/User/Search/Gambit/GroupGambit.php deleted file mode 100644 index 5e113e6dfa..0000000000 --- a/src/User/Search/Gambit/GroupGambit.php +++ /dev/null @@ -1,77 +0,0 @@ -groups = $groups; - } - - /** - * {@inheritdoc} - */ - protected function conditions(AbstractSearch $search, array $matches, $negate) - { - if (! $search instanceof UserSearch) { - throw new LogicException('This gambit can only be applied on a UserSearch'); - } - - $groupIdentifiers = $this->extractGroupIdentifiers($matches); - - $groupQuery = Group::whereVisibleTo($search->getActor()); - - foreach ($groupIdentifiers as $identifier) { - if (is_numeric($identifier)) { - $groupQuery->orWhere('id', $identifier); - } else { - $groupQuery->orWhere('name_singular', $identifier)->orWhere('name_plural', $identifier); - } - } - - $userIds = $groupQuery->join('group_user', 'groups.id', 'group_user.group_id') - ->pluck('group_user.user_id') - ->all(); - - $search->getQuery()->whereIn('id', $userIds, 'and', $negate); - } - - /** - * Extract the group names from the pattern match. - * - * @param array $matches - * @return array - */ - protected function extractGroupIdentifiers(array $matches) - { - return explode(',', trim($matches[1], '"')); - } -} From a8c9ae814a7335bca63643f88e56716ec6a43941 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Mon, 1 Feb 2021 13:59:26 -0500 Subject: [PATCH 13/36] Use Arr::get in CreatedFilterGambit --- src/Discussion/Filter/CreatedFilterGambit.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Discussion/Filter/CreatedFilterGambit.php b/src/Discussion/Filter/CreatedFilterGambit.php index 3de9f8a74d..707ea412d2 100644 --- a/src/Discussion/Filter/CreatedFilterGambit.php +++ b/src/Discussion/Filter/CreatedFilterGambit.php @@ -14,6 +14,7 @@ use Flarum\Search\AbstractRegexGambit; use Flarum\Search\AbstractSearch; use Illuminate\Database\Query\Builder; +use Illuminate\Support\Arr; class CreatedFilterGambit extends AbstractRegexGambit implements FilterInterface { @@ -27,7 +28,7 @@ class CreatedFilterGambit extends AbstractRegexGambit implements FilterInterface */ protected function conditions(AbstractSearch $search, array $matches, $negate) { - $this->constrain($search->getQuery(), $matches[1], $matches[3], $negate); + $this->constrain($search->getQuery(), Arr::get($matches, 1), Arr::get($matches, 3), $negate); } public function getFilterKey(): string @@ -39,10 +40,10 @@ public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $ { preg_match('/^'.$this->pattern.'$/i', 'created:'.$filterValue, $matches); - $this->constrain($wrappedFilter->getQuery(), $matches[1], $matches[3], $negate); + $this->constrain($wrappedFilter->getQuery(), Arr::get($matches, 1), Arr::get($matches, 3), $negate); } - public function constrain(Builder $query, $firstDate, $secondDate, $negate) + public function constrain(Builder $query, ?string $firstDate, ?string $secondDate, $negate) { // If we've just been provided with a single YYYY-MM-DD date, then find // discussions that were started on that exact date. But if we've been From 837253d942b1826843355e88aabf60df14baefbf Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Mon, 1 Feb 2021 15:42:59 -0500 Subject: [PATCH 14/36] Inject filters and filter mutators instead of static --- src/Extend/Filter.php | 9 ++++--- src/Filter/FilterServiceProvider.php | 40 ++++++++++++++++++++++------ src/Filter/Filterer.php | 34 ++++++++--------------- 3 files changed, 49 insertions(+), 34 deletions(-) diff --git a/src/Extend/Filter.php b/src/Extend/Filter.php index adc25ab74c..3ebe6a0b05 100644 --- a/src/Extend/Filter.php +++ b/src/Extend/Filter.php @@ -59,9 +59,12 @@ public function extend(Container $container, Extension $extension = null) return $originalFilters; }); + $container->extend('flarum.filter.filter_mutators', function ($originalMutators) { + foreach ($this->filterMutators as $mutator) { + $originalMutators[$this->resource][] = $mutator; + } - foreach ($this->filterMutators as $mutator) { - Filterer::addFilterMutator($this->resource, ContainerUtil::wrapCallback($mutator, $container)); - } + return $originalMutators; + }); } } diff --git a/src/Filter/FilterServiceProvider.php b/src/Filter/FilterServiceProvider.php index c594c0fe41..7ac9432b57 100644 --- a/src/Filter/FilterServiceProvider.php +++ b/src/Filter/FilterServiceProvider.php @@ -15,6 +15,7 @@ use Flarum\Discussion\Filter\HiddenFilterGambit; use Flarum\Discussion\Filter\UnreadFilterGambit; use Flarum\Foundation\AbstractServiceProvider; +use Flarum\Foundation\ContainerUtil; use Flarum\User\Filter\EmailFilterGambit; use Flarum\User\Filter\GroupFilterGambit; use Flarum\User\User; @@ -42,17 +43,40 @@ public function register() ] ]; }); + + $this->app->singleton('flarum.filter.filter_mutators', function () { + return []; + }); } public function boot() { - $allFilters = $this->app->make('flarum.filter.filters'); - - foreach ($allFilters as $resource => $resourceFilters) { - foreach ($resourceFilters as $filter) { - $filter = $this->app->make($filter); - Filterer::addFilter($resource, $filter); - } - } + $this->app + ->when(Filterer::class) + ->needs('$filters') + ->give(function () { + $compiled = []; + + foreach ($this->app->make('flarum.filter.filters') as $resourceClass => $filters) { + $compiled[$resourceClass] = []; + foreach ($filters as $filter) { + $filter = $this->app->make($filter); + $compiled[$resourceClass][$filter->getFilterKey()][] = $filter; + } + } + + return $compiled; + }); + + $this->app + ->when(Filterer::class) + ->needs('$filterMutators') + ->give(function () { + return array_map(function($resourceFilters) { + return array_map(function ($filterClass) { + return ContainerUtil::wrapCallback($filterClass, $this->app); + }, $resourceFilters); + }, $this->app->make('flarum.filter.filter_mutators')); + }); } } diff --git a/src/Filter/Filterer.php b/src/Filter/Filterer.php index 692ad75661..170b5199d8 100644 --- a/src/Filter/Filterer.php +++ b/src/Filter/Filterer.php @@ -20,30 +20,18 @@ class Filterer { use ApplySearchParametersTrait; - protected static $filters = []; + protected $filters; - protected static $filterMutators = []; + protected $filterMutators; - public static function addFilter($resource, FilterInterface $filter) - { - if (! array_key_exists($resource, static::$filters)) { - static::$filters[$resource] = []; - } - - if (! array_key_exists($filter->getFilterKey(), static::$filters[$resource])) { - static::$filters[$resource][$filter->getFilterKey()] = []; - } - - static::$filters[$resource][$filter->getFilterKey()][] = $filter; - } - - public static function addFilterMutator($resource, $mutator) + /** + * @param array $filters + * @param array $filterMutators + */ + public function __construct(array $filters, array $filterMutators) { - if (! array_key_exists($resource, static::$filterMutators)) { - static::$filterMutators[$resource] = []; - } - - static::$filterMutators[$resource][] = $mutator; + $this->filters = $filters; + $this->filterMutators = $filterMutators; } /** @@ -71,7 +59,7 @@ public function filter(User $actor, Builder $query, array $filters, array $sort $negate = true; $filterKey = substr($filterKey, 1); } - foreach (Arr::get(static::$filters, "$resource.$filterKey", []) as $filter) { + foreach (Arr::get($this->filters, "$resource.$filterKey", []) as $filter) { $filter->filter($wrappedFilter, $filterValue, $negate); } } @@ -80,7 +68,7 @@ public function filter(User $actor, Builder $query, array $filters, array $sort $this->applyOffset($wrappedFilter, $offset); $this->applyLimit($wrappedFilter, $limit + 1); - foreach (Arr::get(static::$filterMutators, $resource, []) as $mutator) { + foreach (Arr::get($this->filterMutators, $resource, []) as $mutator) { $mutator($query, $actor, $filters, $sort); } From 60a1a2b54bfbbe6cbfcffb51b1e10a980fb92741 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Mon, 1 Feb 2021 20:43:17 +0000 Subject: [PATCH 15/36] Apply fixes from StyleCI [ci skip] [skip ci] --- src/Extend/Filter.php | 2 -- src/Filter/FilterServiceProvider.php | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Extend/Filter.php b/src/Extend/Filter.php index 3ebe6a0b05..4268321d28 100644 --- a/src/Extend/Filter.php +++ b/src/Extend/Filter.php @@ -10,8 +10,6 @@ namespace Flarum\Extend; use Flarum\Extension\Extension; -use Flarum\Filter\Filterer; -use Flarum\Foundation\ContainerUtil; use Illuminate\Contracts\Container\Container; class Filter implements ExtenderInterface diff --git a/src/Filter/FilterServiceProvider.php b/src/Filter/FilterServiceProvider.php index 7ac9432b57..021b4908c7 100644 --- a/src/Filter/FilterServiceProvider.php +++ b/src/Filter/FilterServiceProvider.php @@ -72,7 +72,7 @@ public function boot() ->when(Filterer::class) ->needs('$filterMutators') ->give(function () { - return array_map(function($resourceFilters) { + return array_map(function ($resourceFilters) { return array_map(function ($filterClass) { return ContainerUtil::wrapCallback($filterClass, $this->app); }, $resourceFilters); From 94823ba5a15408b8a6800cd9bd341d924ca486ca Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 10 Feb 2021 10:46:18 -0500 Subject: [PATCH 16/36] Per-resource filter classes --- .../Controller/ListDiscussionsController.php | 13 +++-- src/Api/Controller/ListUsersController.php | 13 +++-- src/Discussion/Filter/DiscussionFilterer.php | 40 +++++++++++++ src/Extend/Filter.php | 14 ++--- .../{Filterer.php => AbstractFilterer.php} | 25 ++++---- src/Filter/FilterCriteria.php | 57 +++++++++++++++++++ src/Filter/FilterServiceProvider.php | 57 ++++++++++--------- src/User/Filter/UserFilterer.php | 40 +++++++++++++ tests/integration/extenders/FilterTest.php | 6 +- 9 files changed, 203 insertions(+), 62 deletions(-) create mode 100644 src/Discussion/Filter/DiscussionFilterer.php rename src/Filter/{Filterer.php => AbstractFilterer.php} (72%) create mode 100644 src/Filter/FilterCriteria.php create mode 100644 src/User/Filter/UserFilterer.php diff --git a/src/Api/Controller/ListDiscussionsController.php b/src/Api/Controller/ListDiscussionsController.php index ab7a3d3c50..6b333e9efa 100644 --- a/src/Api/Controller/ListDiscussionsController.php +++ b/src/Api/Controller/ListDiscussionsController.php @@ -12,8 +12,9 @@ use Flarum\Api\Serializer\DiscussionSerializer; use Flarum\Discussion\Discussion; use Flarum\Discussion\DiscussionRepository; +use Flarum\Discussion\Filter\DiscussionFilterer; use Flarum\Discussion\Search\DiscussionSearcher; -use Flarum\Filter\Filterer; +use Flarum\Filter\FilterCriteria; use Flarum\Http\UrlGenerator; use Flarum\Search\SearchCriteria; use Psr\Http\Message\ServerRequestInterface; @@ -55,7 +56,7 @@ class ListDiscussionsController extends AbstractListController protected $discussions; /** - * @var Filterer + * @var DiscussionFilterer */ protected $filterer; @@ -71,11 +72,11 @@ class ListDiscussionsController extends AbstractListController /** * @param DiscussionRepository $discussions - * @param Filterer $filterer + * @param DiscussionFilterer $filterer * @param DiscussionSearcher $searcher * @param UrlGenerator $url */ - public function __construct(DiscussionRepository $discussions, Filterer $filterer, DiscussionSearcher $searcher, UrlGenerator $url) + public function __construct(DiscussionRepository $discussions, DiscussionFilterer $filterer, DiscussionSearcher $searcher, UrlGenerator $url) { $this->discussions = $discussions; $this->filterer = $filterer; @@ -101,9 +102,9 @@ protected function data(ServerRequestInterface $request, Document $document) $results = $this->searcher->search($criteria, $limit, $offset, $load); } else { - $query = $this->discussions->query(); + $criteria = new FilterCriteria($actor, $filters, $sort); - $results = $this->filterer->filter($actor, $query, $filters, $sort, $limit, $offset, $load); + $results = $this->filterer->filter($criteria, $limit, $offset, $load); } $document->addPaginationLinks( diff --git a/src/Api/Controller/ListUsersController.php b/src/Api/Controller/ListUsersController.php index ff09cfb358..bf25fde089 100644 --- a/src/Api/Controller/ListUsersController.php +++ b/src/Api/Controller/ListUsersController.php @@ -10,9 +10,10 @@ namespace Flarum\Api\Controller; use Flarum\Api\Serializer\UserSerializer; -use Flarum\Filter\Filterer; +use Flarum\Filter\FilterCriteria; use Flarum\Http\UrlGenerator; use Flarum\Search\SearchCriteria; +use Flarum\User\Filter\UserFilterer; use Flarum\User\Search\UserSearcher; use Flarum\User\UserRepository; use Psr\Http\Message\ServerRequestInterface; @@ -42,7 +43,7 @@ class ListUsersController extends AbstractListController ]; /** - * @var Filterer + * @var UserFilterer */ protected $filterer; @@ -62,12 +63,12 @@ class ListUsersController extends AbstractListController protected $users; /** - * @param Filterer $filterer + * @param UserFilterer $filterer * @param UserSearcher $searcher * @param UrlGenerator $url * @param UserRepository $users */ - public function __construct(Filterer $filterer, UserSearcher $searcher, UrlGenerator $url, UserRepository $users) + public function __construct(UserFilterer $filterer, UserSearcher $searcher, UrlGenerator $url, UserRepository $users) { $this->filterer = $filterer; $this->searcher = $searcher; @@ -96,9 +97,9 @@ protected function data(ServerRequestInterface $request, Document $document) $results = $this->searcher->search($criteria, $limit, $offset, $load); } else { - $query = $this->users->query(); + $criteria = new FilterCriteria($actor, $filters, $sort); - $results = $this->filterer->filter($actor, $query, $filters, $sort, $limit, $offset, $load); + $results = $this->filterer->filter($criteria, $limit, $offset, $load); } $document->addPaginationLinks( diff --git a/src/Discussion/Filter/DiscussionFilterer.php b/src/Discussion/Filter/DiscussionFilterer.php new file mode 100644 index 0000000000..8ada10502a --- /dev/null +++ b/src/Discussion/Filter/DiscussionFilterer.php @@ -0,0 +1,40 @@ +discussions = $discussions; + } + + protected function getQuery(User $actor): Builder + { + return $this->discussions->query()->select('discussions.*')->whereVisibleTo($actor); + } +} diff --git a/src/Extend/Filter.php b/src/Extend/Filter.php index 4268321d28..5551c88e06 100644 --- a/src/Extend/Filter.php +++ b/src/Extend/Filter.php @@ -14,20 +14,20 @@ class Filter implements ExtenderInterface { - private $resource; + private $filtererClass; private $filters = []; private $filterMutators = []; /** - * @param string $resource: The ::class attribute of the resource this applies to, which is typically an Eloquent model. + * @param string $filtererclass: The ::class attribute of the filterer to extend */ - public function __construct($resource) + public function __construct($filtererClass) { - $this->resource = $resource; + $this->filtererClass = $filtererClass; } /** - * Add a filter to run when the resource is filtered. + * Add a filter to run when the filtererclass is filtered. * * @param string $filterClass: The ::class attribute of the filter you are adding. */ @@ -52,14 +52,14 @@ public function extend(Container $container, Extension $extension = null) { $container->extend('flarum.filter.filters', function ($originalFilters) { foreach ($this->filters as $filter) { - $originalFilters[$this->resource][] = $filter; + $originalFilters[$this->filtererClass][] = $filter; } return $originalFilters; }); $container->extend('flarum.filter.filter_mutators', function ($originalMutators) { foreach ($this->filterMutators as $mutator) { - $originalMutators[$this->resource][] = $mutator; + $originalMutators[$this->filtererClass][] = $mutator; } return $originalMutators; diff --git a/src/Filter/Filterer.php b/src/Filter/AbstractFilterer.php similarity index 72% rename from src/Filter/Filterer.php rename to src/Filter/AbstractFilterer.php index 170b5199d8..bfcf0e564f 100644 --- a/src/Filter/Filterer.php +++ b/src/Filter/AbstractFilterer.php @@ -16,7 +16,7 @@ use Illuminate\Support\Arr; use InvalidArgumentException; -class Filterer +abstract class AbstractFilterer { use ApplySearchParametersTrait; @@ -34,42 +34,41 @@ public function __construct(array $filters, array $filterMutators) $this->filterMutators = $filterMutators; } + abstract protected function getQuery(User $actor): Builder; + /** - * @param User $actor - * @param Builder $query - * @param array $filters - * @param mixed|null $sort + * @param FilterCriteria $criteria * @param mixed|null $limit * @param int $offset * @param array $load * @return SearchResults * @throws InvalidArgumentException */ - public function filter(User $actor, Builder $query, array $filters, array $sort = null, int $limit = null, int $offset = 0, array $load = []): SearchResults + public function filter(FilterCriteria $criteria, int $limit = null, int $offset = 0, array $load = []): SearchResults { - $resource = get_class($query->getModel()); + $actor = $criteria->actor; - $query->whereVisibleTo($actor); + $query = $this->getQuery($actor); $wrappedFilter = new WrappedFilter($query->getQuery(), $actor); - foreach ($filters as $filterKey => $filterValue) { + foreach ($criteria->queryParams as $filterKey => $filterValue) { $negate = false; if (substr($filterKey, 0, 1) == '-') { $negate = true; $filterKey = substr($filterKey, 1); } - foreach (Arr::get($this->filters, "$resource.$filterKey", []) as $filter) { + foreach (Arr::get($this->filters, $filterKey, []) as $filter) { $filter->filter($wrappedFilter, $filterValue, $negate); } } - $this->applySort($wrappedFilter, $sort); + $this->applySort($wrappedFilter, $criteria->sort); $this->applyOffset($wrappedFilter, $offset); $this->applyLimit($wrappedFilter, $limit + 1); - foreach (Arr::get($this->filterMutators, $resource, []) as $mutator) { - $mutator($query, $actor, $filters, $sort); + foreach ($this->filterMutators as $mutator) { + $mutator($query, $actor, $criteria->queryParams, $criteria->sort); } // Execute the filter query and retrieve the results. We get one more diff --git a/src/Filter/FilterCriteria.php b/src/Filter/FilterCriteria.php new file mode 100644 index 0000000000..87b98e07d1 --- /dev/null +++ b/src/Filter/FilterCriteria.php @@ -0,0 +1,57 @@ +actor = $actor; + $this->queryParams = $queryParams; + $this->sort = $sort; + } +} diff --git a/src/Filter/FilterServiceProvider.php b/src/Filter/FilterServiceProvider.php index 021b4908c7..477ea7a30d 100644 --- a/src/Filter/FilterServiceProvider.php +++ b/src/Filter/FilterServiceProvider.php @@ -9,16 +9,17 @@ namespace Flarum\Filter; -use Flarum\Discussion\Discussion; use Flarum\Discussion\Filter\AuthorFilterGambit; use Flarum\Discussion\Filter\CreatedFilterGambit; +use Flarum\Discussion\Filter\DiscussionFilterer; use Flarum\Discussion\Filter\HiddenFilterGambit; use Flarum\Discussion\Filter\UnreadFilterGambit; use Flarum\Foundation\AbstractServiceProvider; use Flarum\Foundation\ContainerUtil; use Flarum\User\Filter\EmailFilterGambit; use Flarum\User\Filter\GroupFilterGambit; -use Flarum\User\User; +use Flarum\User\Filter\UserFilterer; +use Illuminate\Support\Arr; class FilterServiceProvider extends AbstractServiceProvider { @@ -31,13 +32,13 @@ public function register() { $this->app->singleton('flarum.filter.filters', function () { return [ - Discussion::class => [ + DiscussionFilterer::class => [ AuthorFilterGambit::class, CreatedFilterGambit::class, HiddenFilterGambit::class, UnreadFilterGambit::class, ], - User::class => [ + UserFilterer::class => [ EmailFilterGambit::class, GroupFilterGambit::class, ] @@ -51,32 +52,34 @@ public function register() public function boot() { - $this->app - ->when(Filterer::class) - ->needs('$filters') - ->give(function () { - $compiled = []; + // We can resolve the filter mutators in the when->needs->give callback, + // but we need to resolve at least one regardless so we know which + // filterers we need to register filters for. + $filters = $this->app->make('flarum.filter.filters'); - foreach ($this->app->make('flarum.filter.filters') as $resourceClass => $filters) { - $compiled[$resourceClass] = []; - foreach ($filters as $filter) { - $filter = $this->app->make($filter); - $compiled[$resourceClass][$filter->getFilterKey()][] = $filter; + foreach ($filters as $filterer => $filterClasses) { + $this->app + ->when($filterer) + ->needs('$filters') + ->give(function () use ($filterClasses) { + $compiled = []; + + foreach ($filterClasses as $filterClass) { + $filter = $this->app->make($filterClass); + $compiled[$filter->getFilterKey()][] = $filter; } - } - return $compiled; - }); + return $compiled; + }); - $this->app - ->when(Filterer::class) - ->needs('$filterMutators') - ->give(function () { - return array_map(function ($resourceFilters) { - return array_map(function ($filterClass) { - return ContainerUtil::wrapCallback($filterClass, $this->app); - }, $resourceFilters); - }, $this->app->make('flarum.filter.filter_mutators')); - }); + $this->app + ->when($filterer) + ->needs('$filterMutators') + ->give(function () use ($filterer) { + return array_map(function ($filterMutatorClass) { + return ContainerUtil::wrapCallback($filterMutatorClass, $this->app); + }, Arr::get($this->app->make('flarum.filter.filter_mutators'), $filterer, [])); + }); + } } } diff --git a/src/User/Filter/UserFilterer.php b/src/User/Filter/UserFilterer.php new file mode 100644 index 0000000000..418dca3794 --- /dev/null +++ b/src/User/Filter/UserFilterer.php @@ -0,0 +1,40 @@ +users = $users; + } + + protected function getQuery(User $actor): Builder + { + return $this->users->query()->whereVisibleTo($actor); + } +} diff --git a/tests/integration/extenders/FilterTest.php b/tests/integration/extenders/FilterTest.php index 678bd475ce..77204fe535 100644 --- a/tests/integration/extenders/FilterTest.php +++ b/tests/integration/extenders/FilterTest.php @@ -69,7 +69,7 @@ public function works_as_expected_with_no_modifications() */ public function custom_filter_gambit_has_effect_if_added() { - $this->extend((new Extend\Filter(Discussion::class))->addFilter(NoResultFilter::class)); + $this->extend((new Extend\Filter(DiscussionFilterer::class))->addFilter(NoResultFilter::class)); $this->prepDb(); @@ -84,7 +84,7 @@ public function custom_filter_gambit_has_effect_if_added() */ public function filter_mutator_has_effect_if_added() { - $this->extend((new Extend\Filter(Discussion::class))->addFilterMutator(function ($query, $actor, $filters, $sort) { + $this->extend((new Extend\Filter(DiscussionFilterer::class))->addFilterMutator(function ($query, $actor, $filters, $sort) { $query->getQuery()->whereRaw('1=0'); })); @@ -98,7 +98,7 @@ public function filter_mutator_has_effect_if_added() */ public function filter_mutator_has_effect_if_added_with_invokable_class() { - $this->extend((new Extend\Filter(Discussion::class))->addFilterMutator(CustomFilterMutator::class)); + $this->extend((new Extend\Filter(DiscussionFilterer::class))->addFilterMutator(CustomFilterMutator::class)); $this->prepDb(); From 870395618a731ec9fd0db5e723b18c5b2324595c Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 10 Feb 2021 15:47:34 +0000 Subject: [PATCH 17/36] Apply fixes from StyleCI [ci skip] [skip ci] --- tests/integration/extenders/FilterTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/extenders/FilterTest.php b/tests/integration/extenders/FilterTest.php index 77204fe535..1bec593b7f 100644 --- a/tests/integration/extenders/FilterTest.php +++ b/tests/integration/extenders/FilterTest.php @@ -10,7 +10,6 @@ namespace Flarum\Tests\integration\extenders; use Carbon\Carbon; -use Flarum\Discussion\Discussion; use Flarum\Extend; use Flarum\Filter\FilterInterface; use Flarum\Filter\WrappedFilter; From 66976fcaa4b1ff96555b07cf3b8111f2525b57ec Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 10 Feb 2021 10:49:58 -0500 Subject: [PATCH 18/36] List controllers no longer need repositories --- src/Api/Controller/ListDiscussionsController.php | 10 +--------- src/Api/Controller/ListUsersController.php | 10 +--------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/src/Api/Controller/ListDiscussionsController.php b/src/Api/Controller/ListDiscussionsController.php index 6b333e9efa..2c01f41dd3 100644 --- a/src/Api/Controller/ListDiscussionsController.php +++ b/src/Api/Controller/ListDiscussionsController.php @@ -11,7 +11,6 @@ use Flarum\Api\Serializer\DiscussionSerializer; use Flarum\Discussion\Discussion; -use Flarum\Discussion\DiscussionRepository; use Flarum\Discussion\Filter\DiscussionFilterer; use Flarum\Discussion\Search\DiscussionSearcher; use Flarum\Filter\FilterCriteria; @@ -50,11 +49,6 @@ class ListDiscussionsController extends AbstractListController */ public $sortFields = ['lastPostedAt', 'commentCount', 'createdAt']; - /** - * @var DiscussionRepository - */ - protected $discussions; - /** * @var DiscussionFilterer */ @@ -71,14 +65,12 @@ class ListDiscussionsController extends AbstractListController protected $url; /** - * @param DiscussionRepository $discussions * @param DiscussionFilterer $filterer * @param DiscussionSearcher $searcher * @param UrlGenerator $url */ - public function __construct(DiscussionRepository $discussions, DiscussionFilterer $filterer, DiscussionSearcher $searcher, UrlGenerator $url) + public function __construct(DiscussionFilterer $filterer, DiscussionSearcher $searcher, UrlGenerator $url) { - $this->discussions = $discussions; $this->filterer = $filterer; $this->searcher = $searcher; $this->url = $url; diff --git a/src/Api/Controller/ListUsersController.php b/src/Api/Controller/ListUsersController.php index bf25fde089..f0a87dbd9a 100644 --- a/src/Api/Controller/ListUsersController.php +++ b/src/Api/Controller/ListUsersController.php @@ -15,7 +15,6 @@ use Flarum\Search\SearchCriteria; use Flarum\User\Filter\UserFilterer; use Flarum\User\Search\UserSearcher; -use Flarum\User\UserRepository; use Psr\Http\Message\ServerRequestInterface; use Tobscure\JsonApi\Document; @@ -57,23 +56,16 @@ class ListUsersController extends AbstractListController */ protected $url; - /** - * @var UserRepository - */ - protected $users; - /** * @param UserFilterer $filterer * @param UserSearcher $searcher * @param UrlGenerator $url - * @param UserRepository $users */ - public function __construct(UserFilterer $filterer, UserSearcher $searcher, UrlGenerator $url, UserRepository $users) + public function __construct(UserFilterer $filterer, UserSearcher $searcher, UrlGenerator $url) { $this->filterer = $filterer; $this->searcher = $searcher; $this->url = $url; - $this->users = $users; } /** From 6b9645787b49da716aeaede922609d2baa6635c5 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 10 Feb 2021 11:01:03 -0500 Subject: [PATCH 19/36] Add missing imports --- tests/integration/extenders/FilterTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/extenders/FilterTest.php b/tests/integration/extenders/FilterTest.php index 1bec593b7f..a3a5b04341 100644 --- a/tests/integration/extenders/FilterTest.php +++ b/tests/integration/extenders/FilterTest.php @@ -10,6 +10,7 @@ namespace Flarum\Tests\integration\extenders; use Carbon\Carbon; +use Flarum\Discussion\Filter\DiscussionFilterer; use Flarum\Extend; use Flarum\Filter\FilterInterface; use Flarum\Filter\WrappedFilter; @@ -66,7 +67,7 @@ public function works_as_expected_with_no_modifications() /** * @test */ - public function custom_filter_gambit_has_effect_if_added() + public function custom_filter_has_effect_if_added() { $this->extend((new Extend\Filter(DiscussionFilterer::class))->addFilter(NoResultFilter::class)); From 6f276905455c61a5f5c963e66255a53d176829ca Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Sat, 13 Feb 2021 01:15:15 -0500 Subject: [PATCH 20/36] Move gambit pattern to method instead of attribute for consistency --- src/Discussion/Filter/AuthorFilterGambit.php | 5 ++++- src/Discussion/Filter/CreatedFilterGambit.php | 7 +++++-- src/Discussion/Filter/HiddenFilterGambit.php | 5 ++++- src/Discussion/Filter/UnreadFilterGambit.php | 5 ++++- src/Search/AbstractRegexGambit.php | 7 +++---- src/User/Filter/EmailFilterGambit.php | 13 ++++++++----- src/User/Filter/GroupFilterGambit.php | 8 ++++++-- .../extenders/SimpleFlarumSearchTest.php | 7 ++++++- 8 files changed, 40 insertions(+), 17 deletions(-) diff --git a/src/Discussion/Filter/AuthorFilterGambit.php b/src/Discussion/Filter/AuthorFilterGambit.php index f2662985e7..cad9be0d0d 100644 --- a/src/Discussion/Filter/AuthorFilterGambit.php +++ b/src/Discussion/Filter/AuthorFilterGambit.php @@ -34,7 +34,10 @@ public function __construct(UserRepository $users) /** * {@inheritdoc} */ - protected $pattern = 'author:(.+)'; + public function getGambitPattern() + { + return 'author:(.+)'; + } /** * {@inheritdoc} diff --git a/src/Discussion/Filter/CreatedFilterGambit.php b/src/Discussion/Filter/CreatedFilterGambit.php index 707ea412d2..4cfbe81dda 100644 --- a/src/Discussion/Filter/CreatedFilterGambit.php +++ b/src/Discussion/Filter/CreatedFilterGambit.php @@ -21,7 +21,10 @@ class CreatedFilterGambit extends AbstractRegexGambit implements FilterInterface /** * {@inheritdoc} */ - protected $pattern = 'created:(\d{4}\-\d\d\-\d\d)(\.\.(\d{4}\-\d\d\-\d\d))?'; + public function getGambitPattern() + { + return 'created:(\d{4}\-\d\d\-\d\d)(\.\.(\d{4}\-\d\d\-\d\d))?'; + } /** * {@inheritdoc} @@ -38,7 +41,7 @@ public function getFilterKey(): string public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) { - preg_match('/^'.$this->pattern.'$/i', 'created:'.$filterValue, $matches); + preg_match('/^'.$this->getGambitPattern().'$/i', 'created:'.$filterValue, $matches); $this->constrain($wrappedFilter->getQuery(), Arr::get($matches, 1), Arr::get($matches, 3), $negate); } diff --git a/src/Discussion/Filter/HiddenFilterGambit.php b/src/Discussion/Filter/HiddenFilterGambit.php index 3ff2c59372..f25ff5fd88 100644 --- a/src/Discussion/Filter/HiddenFilterGambit.php +++ b/src/Discussion/Filter/HiddenFilterGambit.php @@ -20,7 +20,10 @@ class HiddenFilterGambit extends AbstractRegexGambit implements FilterInterface /** * {@inheritdoc} */ - protected $pattern = 'is:hidden'; + public function getGambitPattern() + { + return 'is:hidden'; + } /** * {@inheritdoc} diff --git a/src/Discussion/Filter/UnreadFilterGambit.php b/src/Discussion/Filter/UnreadFilterGambit.php index 906c3d70fa..b28a8a02ec 100644 --- a/src/Discussion/Filter/UnreadFilterGambit.php +++ b/src/Discussion/Filter/UnreadFilterGambit.php @@ -35,7 +35,10 @@ public function __construct(DiscussionRepository $discussions) /** * {@inheritdoc} */ - protected $pattern = 'is:unread'; + public function getGambitPattern() + { + return 'is:unread'; + } /** * {@inheritdoc} diff --git a/src/Search/AbstractRegexGambit.php b/src/Search/AbstractRegexGambit.php index 204b05c327..886f6fe28d 100644 --- a/src/Search/AbstractRegexGambit.php +++ b/src/Search/AbstractRegexGambit.php @@ -13,10 +13,8 @@ abstract class AbstractRegexGambit implements GambitInterface { /** * The regex pattern to match the bit against. - * - * @var string */ - protected $pattern; + protected function getGambitPattern() {} /** * {@inheritdoc} @@ -40,7 +38,8 @@ public function apply(AbstractSearch $search, $bit) */ protected function match($bit) { - if (preg_match('/^(-?)'.$this->pattern.'$/i', $bit, $matches)) { + // @deprecated, remove use of $this->pattern during beta 17. + if (preg_match('/^(-?)'.($this->pattern ?: $this->getGambitPattern()).'$/i', $bit, $matches)) { return $matches; } } diff --git a/src/User/Filter/EmailFilterGambit.php b/src/User/Filter/EmailFilterGambit.php index 666f40466d..ca6853bb7a 100644 --- a/src/User/Filter/EmailFilterGambit.php +++ b/src/User/Filter/EmailFilterGambit.php @@ -17,11 +17,6 @@ class EmailFilterGambit extends AbstractRegexGambit implements FilterInterface { - /** - * {@inheritdoc} - */ - protected $pattern = 'email:(.+)'; - /** * {@inheritdoc} */ @@ -34,6 +29,14 @@ public function apply(AbstractSearch $search, $bit) return parent::apply($search, $bit); } + /** + * {@inheritdoc} + */ + public function getGambitPattern() + { + return 'email:(.+)'; + } + /** * {@inheritdoc} */ diff --git a/src/User/Filter/GroupFilterGambit.php b/src/User/Filter/GroupFilterGambit.php index ed03dae4fd..8c67d5a602 100644 --- a/src/User/Filter/GroupFilterGambit.php +++ b/src/User/Filter/GroupFilterGambit.php @@ -18,10 +18,14 @@ use Illuminate\Database\Query\Builder; class GroupFilterGambit extends AbstractRegexGambit implements FilterInterface -{/** +{ + /** * {@inheritdoc} */ - protected $pattern = 'group:(.+)'; + public function getGambitPattern() + { + return 'group:(.+)'; + } /** * {@inheritdoc} diff --git a/tests/integration/extenders/SimpleFlarumSearchTest.php b/tests/integration/extenders/SimpleFlarumSearchTest.php index 2a874cf763..1ae8c118d4 100644 --- a/tests/integration/extenders/SimpleFlarumSearchTest.php +++ b/tests/integration/extenders/SimpleFlarumSearchTest.php @@ -151,7 +151,12 @@ public function apply(AbstractSearch $search, $searchValue) class NoResultFilterGambit extends AbstractRegexGambit { - protected $pattern = 'noResult:(.+)'; + /** + * {@inheritdoc} + */ + public function getGambitPattern() { + return 'noResult:(.+)'; + } /** * {@inheritdoc} From 2838f110861be547a37e5175e7cf605022b07cfb Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Sat, 13 Feb 2021 16:49:59 -0500 Subject: [PATCH 21/36] Combine search and filter criteria classes --- .../Controller/ListDiscussionsController.php | 6 +- src/Api/Controller/ListUsersController.php | 6 +- src/Filter/AbstractFilterer.php | 9 +-- src/Filter/FilterCriteria.php | 57 ------------------- src/Search/AbstractSearcher.php | 2 +- src/Search/SearchCriteria.php | 12 ++-- .../extenders/SimpleFlarumSearchTest.php | 2 +- 7 files changed, 15 insertions(+), 79 deletions(-) delete mode 100644 src/Filter/FilterCriteria.php diff --git a/src/Api/Controller/ListDiscussionsController.php b/src/Api/Controller/ListDiscussionsController.php index 2c01f41dd3..21614199ed 100644 --- a/src/Api/Controller/ListDiscussionsController.php +++ b/src/Api/Controller/ListDiscussionsController.php @@ -13,7 +13,6 @@ use Flarum\Discussion\Discussion; use Flarum\Discussion\Filter\DiscussionFilterer; use Flarum\Discussion\Search\DiscussionSearcher; -use Flarum\Filter\FilterCriteria; use Flarum\Http\UrlGenerator; use Flarum\Search\SearchCriteria; use Psr\Http\Message\ServerRequestInterface; @@ -89,13 +88,10 @@ protected function data(ServerRequestInterface $request, Document $document) $offset = $this->extractOffset($request); $load = array_merge($this->extractInclude($request), ['state']); + $criteria = new SearchCriteria($actor, $filters, $sort); if (array_key_exists('q', $filters)) { - $criteria = new SearchCriteria($actor, $filters['q'], $sort); - $results = $this->searcher->search($criteria, $limit, $offset, $load); } else { - $criteria = new FilterCriteria($actor, $filters, $sort); - $results = $this->filterer->filter($criteria, $limit, $offset, $load); } diff --git a/src/Api/Controller/ListUsersController.php b/src/Api/Controller/ListUsersController.php index f0a87dbd9a..850ce99581 100644 --- a/src/Api/Controller/ListUsersController.php +++ b/src/Api/Controller/ListUsersController.php @@ -10,7 +10,6 @@ namespace Flarum\Api\Controller; use Flarum\Api\Serializer\UserSerializer; -use Flarum\Filter\FilterCriteria; use Flarum\Http\UrlGenerator; use Flarum\Search\SearchCriteria; use Flarum\User\Filter\UserFilterer; @@ -84,13 +83,10 @@ protected function data(ServerRequestInterface $request, Document $document) $offset = $this->extractOffset($request); $load = $this->extractInclude($request); + $criteria = new SearchCriteria($actor, $filters, $sort); if (array_key_exists('q', $filters)) { - $criteria = new SearchCriteria($actor, $filters['q'], $sort); - $results = $this->searcher->search($criteria, $limit, $offset, $load); } else { - $criteria = new FilterCriteria($actor, $filters, $sort); - $results = $this->filterer->filter($criteria, $limit, $offset, $load); } diff --git a/src/Filter/AbstractFilterer.php b/src/Filter/AbstractFilterer.php index bfcf0e564f..91128b17f6 100644 --- a/src/Filter/AbstractFilterer.php +++ b/src/Filter/AbstractFilterer.php @@ -10,6 +10,7 @@ namespace Flarum\Filter; use Flarum\Search\ApplySearchParametersTrait; +use Flarum\Search\SearchCriteria; use Flarum\Search\SearchResults; use Flarum\User\User; use Illuminate\Database\Eloquent\Builder; @@ -37,14 +38,14 @@ public function __construct(array $filters, array $filterMutators) abstract protected function getQuery(User $actor): Builder; /** - * @param FilterCriteria $criteria + * @param SearchCriteria $criteria * @param mixed|null $limit * @param int $offset * @param array $load * @return SearchResults * @throws InvalidArgumentException */ - public function filter(FilterCriteria $criteria, int $limit = null, int $offset = 0, array $load = []): SearchResults + public function filter(SearchCriteria $criteria, int $limit = null, int $offset = 0, array $load = []): SearchResults { $actor = $criteria->actor; @@ -52,7 +53,7 @@ public function filter(FilterCriteria $criteria, int $limit = null, int $offset $wrappedFilter = new WrappedFilter($query->getQuery(), $actor); - foreach ($criteria->queryParams as $filterKey => $filterValue) { + foreach ($criteria->query as $filterKey => $filterValue) { $negate = false; if (substr($filterKey, 0, 1) == '-') { $negate = true; @@ -68,7 +69,7 @@ public function filter(FilterCriteria $criteria, int $limit = null, int $offset $this->applyLimit($wrappedFilter, $limit + 1); foreach ($this->filterMutators as $mutator) { - $mutator($query, $actor, $criteria->queryParams, $criteria->sort); + $mutator($query, $actor, $criteria->query, $criteria->sort); } // Execute the filter query and retrieve the results. We get one more diff --git a/src/Filter/FilterCriteria.php b/src/Filter/FilterCriteria.php deleted file mode 100644 index 87b98e07d1..0000000000 --- a/src/Filter/FilterCriteria.php +++ /dev/null @@ -1,57 +0,0 @@ -actor = $actor; - $this->queryParams = $queryParams; - $this->sort = $sort; - } -} diff --git a/src/Search/AbstractSearcher.php b/src/Search/AbstractSearcher.php index 9ccb33d59f..914cf73d18 100644 --- a/src/Search/AbstractSearcher.php +++ b/src/Search/AbstractSearcher.php @@ -58,7 +58,7 @@ public function search(SearchCriteria $criteria, $limit = null, $offset = 0, arr $search = $this->getSearch($query, $actor); - $this->gambits->apply($search, $criteria->query); + $this->gambits->apply($search, $criteria->query['q']); $this->applySort($search, $criteria->sort); $this->applyOffset($search, $offset); $this->applyLimit($search, $limit + 1); diff --git a/src/Search/SearchCriteria.php b/src/Search/SearchCriteria.php index 6e8dd6e36f..30820dd670 100644 --- a/src/Search/SearchCriteria.php +++ b/src/Search/SearchCriteria.php @@ -13,22 +13,22 @@ /** * Represents the criteria that will determine the entire result set of a - * search. The limit and offset are not included because they only determine + * query. The limit and offset are not included because they only determine * which part of the entire result set will be returned. */ class SearchCriteria { /** - * The user performing the search. + * The user performing the query. * * @var User */ public $actor; /** - * The search query. + * Query params. * - * @var string + * @var array */ public $query; @@ -42,8 +42,8 @@ class SearchCriteria public $sort; /** - * @param User $actor The user performing the search. - * @param string $query The search query. + * @param User $actor The user performing the query. + * @param array $query The query params. * @param array $sort An array of sort-order pairs, where the column is the * key, and the order is the value. The order may be 'asc', 'desc', or * an array of IDs to order by. diff --git a/tests/integration/extenders/SimpleFlarumSearchTest.php b/tests/integration/extenders/SimpleFlarumSearchTest.php index 1ae8c118d4..6a09ec47e5 100644 --- a/tests/integration/extenders/SimpleFlarumSearchTest.php +++ b/tests/integration/extenders/SimpleFlarumSearchTest.php @@ -64,7 +64,7 @@ public function searchDiscussions($query, $limit = null) $actor = User::find(1); - $criteria = new SearchCriteria($actor, $query); + $criteria = new SearchCriteria($actor, ['q' => $query]); return $this->app()->getContainer()->make(DiscussionSearcher::class)->search($criteria, $limit)->getResults(); } From ad38f79f074cc13a8b566ee4505721b8ba5e88c6 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Sat, 13 Feb 2021 21:50:18 +0000 Subject: [PATCH 22/36] Apply fixes from StyleCI [ci skip] [skip ci] --- src/Search/AbstractRegexGambit.php | 4 +++- tests/integration/extenders/SimpleFlarumSearchTest.php | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Search/AbstractRegexGambit.php b/src/Search/AbstractRegexGambit.php index 886f6fe28d..db64d28803 100644 --- a/src/Search/AbstractRegexGambit.php +++ b/src/Search/AbstractRegexGambit.php @@ -14,7 +14,9 @@ abstract class AbstractRegexGambit implements GambitInterface /** * The regex pattern to match the bit against. */ - protected function getGambitPattern() {} + protected function getGambitPattern() + { + } /** * {@inheritdoc} diff --git a/tests/integration/extenders/SimpleFlarumSearchTest.php b/tests/integration/extenders/SimpleFlarumSearchTest.php index 6a09ec47e5..0b12884296 100644 --- a/tests/integration/extenders/SimpleFlarumSearchTest.php +++ b/tests/integration/extenders/SimpleFlarumSearchTest.php @@ -154,7 +154,8 @@ class NoResultFilterGambit extends AbstractRegexGambit /** * {@inheritdoc} */ - public function getGambitPattern() { + public function getGambitPattern() + { return 'noResult:(.+)'; } From 9fcc7cdfe6fb45d6a7334e7152c1d8af2cb30b7f Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 17 Feb 2021 13:18:46 -0500 Subject: [PATCH 23/36] Use null coalesce instead of shorthand ternary --- src/Search/AbstractRegexGambit.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Search/AbstractRegexGambit.php b/src/Search/AbstractRegexGambit.php index db64d28803..a1c8ed52ac 100644 --- a/src/Search/AbstractRegexGambit.php +++ b/src/Search/AbstractRegexGambit.php @@ -41,7 +41,7 @@ public function apply(AbstractSearch $search, $bit) protected function match($bit) { // @deprecated, remove use of $this->pattern during beta 17. - if (preg_match('/^(-?)'.($this->pattern ?: $this->getGambitPattern()).'$/i', $bit, $matches)) { + if (preg_match('/^(-?)'.($this->pattern ?? $this->getGambitPattern()).'$/i', $bit, $matches)) { return $matches; } } From 70ebed2ad9d350d892739018f4a6ead5a16d765f Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 17 Feb 2021 14:50:03 -0500 Subject: [PATCH 24/36] Replace WrappedFilter and AbstractSearch with FilterState and SearchState --- .../Controller/ListDiscussionsController.php | 5 ++ src/Discussion/Filter/AuthorFilterGambit.php | 10 ++-- src/Discussion/Filter/CreatedFilterGambit.php | 10 ++-- src/Discussion/Filter/HiddenFilterGambit.php | 10 ++-- src/Discussion/Filter/UnreadFilterGambit.php | 10 ++-- src/Discussion/Search/DiscussionSearch.php | 51 ------------------- src/Discussion/Search/DiscussionSearcher.php | 10 ++-- .../Search/Gambit/FulltextGambit.php | 4 +- src/Extend/SimpleFlarumSearch.php | 2 +- src/Filter/AbstractFilterer.php | 10 ++-- src/Filter/FilterInterface.php | 4 +- .../FilterState.php} | 41 +++------------ src/Filter/WrappedFilter.php | 16 ------ src/Search/AbstractRegexGambit.php | 6 +-- src/Search/AbstractSearcher.php | 6 +-- src/Search/ApplySearchParametersTrait.php | 12 ++--- src/Search/GambitInterface.php | 4 +- src/Search/GambitManager.php | 12 ++--- src/Search/SearchState.php | 41 +++++++++++++++ src/User/Filter/EmailFilterGambit.php | 14 ++--- src/User/Filter/GroupFilterGambit.php | 10 ++-- src/User/Search/Gambit/FulltextGambit.php | 4 +- src/User/Search/UserSearch.php | 16 ------ src/User/Search/UserSearcher.php | 10 ++-- tests/integration/extenders/FilterTest.php | 6 +-- .../extenders/SimpleFlarumSearchTest.php | 6 +-- 26 files changed, 128 insertions(+), 202 deletions(-) delete mode 100644 src/Discussion/Search/DiscussionSearch.php rename src/{Search/AbstractSearch.php => Filter/FilterState.php} (64%) delete mode 100644 src/Filter/WrappedFilter.php create mode 100644 src/Search/SearchState.php delete mode 100644 src/User/Search/UserSearch.php diff --git a/src/Api/Controller/ListDiscussionsController.php b/src/Api/Controller/ListDiscussionsController.php index 21614199ed..d35d55b1c0 100644 --- a/src/Api/Controller/ListDiscussionsController.php +++ b/src/Api/Controller/ListDiscussionsController.php @@ -58,6 +58,11 @@ class ListDiscussionsController extends AbstractListController */ protected $searcher; + /** + * {@inheritDoc} + */ + protected $sort = ['lastPostedAt' => 'desc']; + /** * @var UrlGenerator */ diff --git a/src/Discussion/Filter/AuthorFilterGambit.php b/src/Discussion/Filter/AuthorFilterGambit.php index cad9be0d0d..616b8be385 100644 --- a/src/Discussion/Filter/AuthorFilterGambit.php +++ b/src/Discussion/Filter/AuthorFilterGambit.php @@ -10,9 +10,9 @@ namespace Flarum\Discussion\Filter; use Flarum\Filter\FilterInterface; -use Flarum\Filter\WrappedFilter; +use Flarum\Filter\FilterState; use Flarum\Search\AbstractRegexGambit; -use Flarum\Search\AbstractSearch; +use Flarum\Search\SearchState; use Flarum\User\UserRepository; use Illuminate\Database\Query\Builder; @@ -42,7 +42,7 @@ public function getGambitPattern() /** * {@inheritdoc} */ - protected function conditions(AbstractSearch $search, array $matches, $negate) + protected function conditions(SearchState $search, array $matches, $negate) { $this->constrain($search->getQuery(), $matches[1], $negate); } @@ -52,9 +52,9 @@ public function getFilterKey(): string return 'author'; } - public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) + public function filter(FilterState $filterState, string $filterValue, bool $negate) { - $this->constrain($wrappedFilter->getQuery(), $filterValue, $negate); + $this->constrain($filterState->getQuery(), $filterValue, $negate); } protected function constrain(Builder $query, $rawUsernames, $negate) diff --git a/src/Discussion/Filter/CreatedFilterGambit.php b/src/Discussion/Filter/CreatedFilterGambit.php index 4cfbe81dda..638d848845 100644 --- a/src/Discussion/Filter/CreatedFilterGambit.php +++ b/src/Discussion/Filter/CreatedFilterGambit.php @@ -10,9 +10,9 @@ namespace Flarum\Discussion\Filter; use Flarum\Filter\FilterInterface; -use Flarum\Filter\WrappedFilter; +use Flarum\Filter\FilterState; use Flarum\Search\AbstractRegexGambit; -use Flarum\Search\AbstractSearch; +use Flarum\Search\SearchState; use Illuminate\Database\Query\Builder; use Illuminate\Support\Arr; @@ -29,7 +29,7 @@ public function getGambitPattern() /** * {@inheritdoc} */ - protected function conditions(AbstractSearch $search, array $matches, $negate) + protected function conditions(SearchState $search, array $matches, $negate) { $this->constrain($search->getQuery(), Arr::get($matches, 1), Arr::get($matches, 3), $negate); } @@ -39,11 +39,11 @@ public function getFilterKey(): string return 'created'; } - public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) + public function filter(FilterState $filterState, string $filterValue, bool $negate) { preg_match('/^'.$this->getGambitPattern().'$/i', 'created:'.$filterValue, $matches); - $this->constrain($wrappedFilter->getQuery(), Arr::get($matches, 1), Arr::get($matches, 3), $negate); + $this->constrain($filterState->getQuery(), Arr::get($matches, 1), Arr::get($matches, 3), $negate); } public function constrain(Builder $query, ?string $firstDate, ?string $secondDate, $negate) diff --git a/src/Discussion/Filter/HiddenFilterGambit.php b/src/Discussion/Filter/HiddenFilterGambit.php index f25ff5fd88..133167a360 100644 --- a/src/Discussion/Filter/HiddenFilterGambit.php +++ b/src/Discussion/Filter/HiddenFilterGambit.php @@ -10,9 +10,9 @@ namespace Flarum\Discussion\Filter; use Flarum\Filter\FilterInterface; -use Flarum\Filter\WrappedFilter; +use Flarum\Filter\FilterState; use Flarum\Search\AbstractRegexGambit; -use Flarum\Search\AbstractSearch; +use Flarum\Search\SearchState; use Illuminate\Database\Query\Builder; class HiddenFilterGambit extends AbstractRegexGambit implements FilterInterface @@ -28,7 +28,7 @@ public function getGambitPattern() /** * {@inheritdoc} */ - protected function conditions(AbstractSearch $search, array $matches, $negate) + protected function conditions(SearchState $search, array $matches, $negate) { $this->constrain($search->getQuery(), $negate); } @@ -38,9 +38,9 @@ public function getFilterKey(): string return 'hidden'; } - public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) + public function filter(FilterState $filterState, string $filterValue, bool $negate) { - $this->constrain($wrappedFilter->getQuery(), $negate); + $this->constrain($filterState->getQuery(), $negate); } protected function constrain(Builder $query, bool $negate) diff --git a/src/Discussion/Filter/UnreadFilterGambit.php b/src/Discussion/Filter/UnreadFilterGambit.php index b28a8a02ec..06e6efb8e9 100644 --- a/src/Discussion/Filter/UnreadFilterGambit.php +++ b/src/Discussion/Filter/UnreadFilterGambit.php @@ -11,9 +11,9 @@ use Flarum\Discussion\DiscussionRepository; use Flarum\Filter\FilterInterface; -use Flarum\Filter\WrappedFilter; +use Flarum\Filter\FilterState; use Flarum\Search\AbstractRegexGambit; -use Flarum\Search\AbstractSearch; +use Flarum\Search\SearchState; use Flarum\User\User; use Illuminate\Database\Query\Builder; @@ -43,7 +43,7 @@ public function getGambitPattern() /** * {@inheritdoc} */ - protected function conditions(AbstractSearch $search, array $matches, $negate) + protected function conditions(SearchState $search, array $matches, $negate) { $this->constrain($search->getQuery(), $search->getActor(), $negate); } @@ -53,9 +53,9 @@ public function getFilterKey(): string return 'unread'; } - public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) + public function filter(FilterState $filterState, string $filterValue, bool $negate) { - $this->constrain($wrappedFilter->getQuery(), $wrappedFilter->getActor(), $negate); + $this->constrain($filterState->getQuery(), $filterState->getActor(), $negate); } protected function constrain(Builder $query, User $actor, bool $negate) diff --git a/src/Discussion/Search/DiscussionSearch.php b/src/Discussion/Search/DiscussionSearch.php deleted file mode 100644 index 4a07492b25..0000000000 --- a/src/Discussion/Search/DiscussionSearch.php +++ /dev/null @@ -1,51 +0,0 @@ - 'desc']; - - /** - * @var array - */ - protected $relevantPostIds = []; - - /** - * Get the related IDs for each result. - * - * @return int[] - */ - public function getRelevantPostIds() - { - return $this->relevantPostIds; - } - - /** - * Set the relevant post IDs for the results. - * - * @param array $relevantPostIds - * @return void - */ - public function setRelevantPostIds(array $relevantPostIds) - { - $this->relevantPostIds = $relevantPostIds; - } -} diff --git a/src/Discussion/Search/DiscussionSearcher.php b/src/Discussion/Search/DiscussionSearcher.php index edc755b8c8..dac0d53bc1 100644 --- a/src/Discussion/Search/DiscussionSearcher.php +++ b/src/Discussion/Search/DiscussionSearcher.php @@ -11,10 +11,11 @@ use Flarum\Discussion\DiscussionRepository; use Flarum\Discussion\Event\Searching; -use Flarum\Search\AbstractSearch; +use Flarum\Search\SearchState; use Flarum\Search\AbstractSearcher; use Flarum\Search\GambitManager; use Flarum\Search\SearchCriteria; +use Flarum\Search\SearchState; use Flarum\User\User; use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Database\Eloquent\Builder; @@ -50,15 +51,10 @@ protected function getQuery(User $actor): Builder return $this->discussions->query()->select('discussions.*')->whereVisibleTo($actor); } - protected function getSearch(Builder $query, User $actor): AbstractSearch - { - return new DiscussionSearch($query->getQuery(), $actor); - } - /** * @deprecated along with the Searching event, remove in Beta 17. */ - protected function mutateSearch(AbstractSearch $search, SearchCriteria $criteria) + protected function mutateSearch(SearchState $search, SearchCriteria $criteria) { parent::mutateSearch($search, $criteria); diff --git a/src/Discussion/Search/Gambit/FulltextGambit.php b/src/Discussion/Search/Gambit/FulltextGambit.php index ea3fe3ede6..a79a1d5495 100644 --- a/src/Discussion/Search/Gambit/FulltextGambit.php +++ b/src/Discussion/Search/Gambit/FulltextGambit.php @@ -11,7 +11,7 @@ use Flarum\Discussion\Search\DiscussionSearch; use Flarum\Post\Post; -use Flarum\Search\AbstractSearch; +use Flarum\Search\SearchState; use Flarum\Search\GambitInterface; use Illuminate\Database\Query\Expression; use LogicException; @@ -21,7 +21,7 @@ class FulltextGambit implements GambitInterface /** * {@inheritdoc} */ - public function apply(AbstractSearch $search, $bit) + public function apply(SearchState $search, $bit) { if (! $search instanceof DiscussionSearch) { throw new LogicException('This gambit can only be applied on a DiscussionSearch'); diff --git a/src/Extend/SimpleFlarumSearch.php b/src/Extend/SimpleFlarumSearch.php index 6f308a5067..678ed67541 100644 --- a/src/Extend/SimpleFlarumSearch.php +++ b/src/Extend/SimpleFlarumSearch.php @@ -60,7 +60,7 @@ public function setFullTextGambit($gambitClass) * @param callable|string $callback * * The callback can be a closure or an invokable class, and should accept: - * - Flarum\Search\AbstractSearch $search + * - Flarum\Search\SearchState $search * - Flarum\Search\SearchCriteria $criteria */ public function addSearchMutator($callback) diff --git a/src/Filter/AbstractFilterer.php b/src/Filter/AbstractFilterer.php index 91128b17f6..4c2d932c06 100644 --- a/src/Filter/AbstractFilterer.php +++ b/src/Filter/AbstractFilterer.php @@ -51,7 +51,7 @@ public function filter(SearchCriteria $criteria, int $limit = null, int $offset $query = $this->getQuery($actor); - $wrappedFilter = new WrappedFilter($query->getQuery(), $actor); + $filterState = new FilterState($query->getQuery(), $actor); foreach ($criteria->query as $filterKey => $filterValue) { $negate = false; @@ -60,13 +60,13 @@ public function filter(SearchCriteria $criteria, int $limit = null, int $offset $filterKey = substr($filterKey, 1); } foreach (Arr::get($this->filters, $filterKey, []) as $filter) { - $filter->filter($wrappedFilter, $filterValue, $negate); + $filter->filter($filterState, $filterValue, $negate); } } - $this->applySort($wrappedFilter, $criteria->sort); - $this->applyOffset($wrappedFilter, $offset); - $this->applyLimit($wrappedFilter, $limit + 1); + $this->applySort($filterState, $criteria->sort); + $this->applyOffset($filterState, $offset); + $this->applyLimit($filterState, $limit + 1); foreach ($this->filterMutators as $mutator) { $mutator($query, $actor, $criteria->query, $criteria->sort); diff --git a/src/Filter/FilterInterface.php b/src/Filter/FilterInterface.php index 95d96d8c24..a5853c4bd7 100644 --- a/src/Filter/FilterInterface.php +++ b/src/Filter/FilterInterface.php @@ -19,8 +19,8 @@ public function getFilterKey(): string; /** * Filters a query. * - * @param WrappedFilter $filter + * @param FilterState $filter * @param string $value The value of the requested filter */ - public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate); + public function filter(FilterState $filterState, string $filterValue, bool $negate); } diff --git a/src/Search/AbstractSearch.php b/src/Filter/FilterState.php similarity index 64% rename from src/Search/AbstractSearch.php rename to src/Filter/FilterState.php index 1de7dcac9c..cc5a4a2595 100644 --- a/src/Search/AbstractSearch.php +++ b/src/Filter/FilterState.php @@ -7,17 +7,12 @@ * LICENSE file that was distributed with this source code. */ -namespace Flarum\Search; +namespace Flarum\Filter; use Flarum\User\User; use Illuminate\Database\Query\Builder; -/** - * An object which represents the internal state of a generic search: - * the search query, the user performing the search, the fallback sort order, - * and a log of which gambits have been used. - */ -abstract class AbstractSearch +class FilterState { /** * @var Builder @@ -30,23 +25,19 @@ abstract class AbstractSearch protected $actor; /** - * @var array + * @var mixed */ protected $defaultSort = []; - /** - * @var GambitInterface[] - */ - protected $activeGambits = []; - /** * @param Builder $query * @param User $actor */ - public function __construct(Builder $query, User $actor) + public function __construct(Builder $query, User $actor, $defaultSort = []) { $this->query = $query; $this->actor = $actor; + $this->defaultSort = $defaultSort; } /** @@ -86,31 +77,11 @@ public function getDefaultSort() * @param mixed $defaultSort An array of sort-order pairs, where the column * is the key, and the order is the value. The order may be 'asc', * 'desc', or an array of IDs to order by. + * Alternatively, a callable may be used. * @return mixed */ public function setDefaultSort($defaultSort) { $this->defaultSort = $defaultSort; } - - /** - * Get a list of the gambits that are active in this search. - * - * @return GambitInterface[] - */ - public function getActiveGambits() - { - return $this->activeGambits; - } - - /** - * Add a gambit as being active in this search. - * - * @param GambitInterface $gambit - * @return void - */ - public function addActiveGambit(GambitInterface $gambit) - { - $this->activeGambits[] = $gambit; - } } diff --git a/src/Filter/WrappedFilter.php b/src/Filter/WrappedFilter.php deleted file mode 100644 index a1f9be79e6..0000000000 --- a/src/Filter/WrappedFilter.php +++ /dev/null @@ -1,16 +0,0 @@ -match($bit)) { list($negate) = array_splice($matches, 1, 1); @@ -49,11 +49,11 @@ protected function match($bit) /** * Apply conditions to the search, given that the gambit was matched. * - * @param AbstractSearch $search The search object. + * @param SearchState $search The search object. * @param array $matches An array of matches from the search bit. * @param bool $negate Whether or not the bit was negated, and thus whether * or not the conditions should be negated. * @return mixed */ - abstract protected function conditions(AbstractSearch $search, array $matches, $negate); + abstract protected function conditions(SearchState $search, array $matches, $negate); } diff --git a/src/Search/AbstractSearcher.php b/src/Search/AbstractSearcher.php index 914cf73d18..9a11d13a1a 100644 --- a/src/Search/AbstractSearcher.php +++ b/src/Search/AbstractSearcher.php @@ -34,9 +34,9 @@ public function __construct(GambitManager $gambits, array $searchMutators) abstract protected function getQuery(User $actor): Builder; - abstract protected function getSearch(Builder $query, User $actor): AbstractSearch; + abstract protected function getSearch(Builder $query, User $actor): SearchState; - protected function mutateSearch(AbstractSearch $search, SearchCriteria $criteria) + protected function mutateSearch(SearchState $search, SearchCriteria $criteria) { foreach ($this->searchMutators as $mutator) { $mutator($search, $criteria); @@ -56,7 +56,7 @@ public function search(SearchCriteria $criteria, $limit = null, $offset = 0, arr $query = $this->getQuery($actor); - $search = $this->getSearch($query, $actor); + $search = new SearchState($query->getQuery(), $actor); $this->gambits->apply($search, $criteria->query['q']); $this->applySort($search, $criteria->sort); diff --git a/src/Search/ApplySearchParametersTrait.php b/src/Search/ApplySearchParametersTrait.php index 0e5798743c..e1172b0e9a 100644 --- a/src/Search/ApplySearchParametersTrait.php +++ b/src/Search/ApplySearchParametersTrait.php @@ -16,10 +16,10 @@ trait ApplySearchParametersTrait /** * Apply sort criteria to a discussion search. * - * @param AbstractSearch $search + * @param SearchState $search * @param array $sort */ - protected function applySort(AbstractSearch $search, array $sort = null) + protected function applySort(SearchState $search, array $sort = null) { $sort = $sort ?: $search->getDefaultSort(); @@ -39,10 +39,10 @@ protected function applySort(AbstractSearch $search, array $sort = null) } /** - * @param AbstractSearch $search + * @param SearchState $search * @param int $offset */ - protected function applyOffset(AbstractSearch $search, $offset) + protected function applyOffset(SearchState $search, $offset) { if ($offset > 0) { $search->getQuery()->skip($offset); @@ -50,10 +50,10 @@ protected function applyOffset(AbstractSearch $search, $offset) } /** - * @param AbstractSearch $search + * @param SearchState $search * @param int|null $limit */ - protected function applyLimit(AbstractSearch $search, $limit) + protected function applyLimit(SearchState $search, $limit) { if ($limit > 0) { $search->getQuery()->take($limit); diff --git a/src/Search/GambitInterface.php b/src/Search/GambitInterface.php index 4a190a2c8d..cbb691e630 100644 --- a/src/Search/GambitInterface.php +++ b/src/Search/GambitInterface.php @@ -14,9 +14,9 @@ interface GambitInterface /** * Apply conditions to the searcher for a bit of the search string. * - * @param AbstractSearch $search + * @param SearchState $search * @param string $bit The piece of the search string. * @return bool Whether or not the gambit was active for this bit. */ - public function apply(AbstractSearch $search, $bit); + public function apply(SearchState $search, $bit); } diff --git a/src/Search/GambitManager.php b/src/Search/GambitManager.php index 775af42b60..b9e5b0b0f0 100644 --- a/src/Search/GambitManager.php +++ b/src/Search/GambitManager.php @@ -55,10 +55,10 @@ public function getGambits() /** * Apply gambits to a search, given a search query. * - * @param AbstractSearch $search + * @param SearchState $search * @param string $query */ - public function apply(AbstractSearch $search, $query) + public function apply(SearchState $search, $query) { $query = $this->applyGambits($search, $query); @@ -89,11 +89,11 @@ protected function explode($query) } /** - * @param AbstractSearch $search + * @param SearchState $search * @param string $query * @return string */ - protected function applyGambits(AbstractSearch $search, $query) + protected function applyGambits(SearchState $search, $query) { $bits = $this->explode($query); @@ -121,10 +121,10 @@ protected function applyGambits(AbstractSearch $search, $query) } /** - * @param AbstractSearch $search + * @param SearchState $search * @param string $query */ - protected function applyFulltext(AbstractSearch $search, $query) + protected function applyFulltext(SearchState $search, $query) { if (! $this->fulltextGambit) { return; diff --git a/src/Search/SearchState.php b/src/Search/SearchState.php new file mode 100644 index 0000000000..1d13e4529a --- /dev/null +++ b/src/Search/SearchState.php @@ -0,0 +1,41 @@ +activeGambits; + } + + /** + * Add a gambit as being active in this search. + * + * @param GambitInterface $gambit + * @return void + */ + public function addActiveGambit(GambitInterface $gambit) + { + $this->activeGambits[] = $gambit; + } +} diff --git a/src/User/Filter/EmailFilterGambit.php b/src/User/Filter/EmailFilterGambit.php index ca6853bb7a..3fc4ae9abf 100644 --- a/src/User/Filter/EmailFilterGambit.php +++ b/src/User/Filter/EmailFilterGambit.php @@ -10,9 +10,9 @@ namespace Flarum\User\Filter; use Flarum\Filter\FilterInterface; -use Flarum\Filter\WrappedFilter; +use Flarum\Filter\FilterState; use Flarum\Search\AbstractRegexGambit; -use Flarum\Search\AbstractSearch; +use Flarum\Search\SearchState; use Illuminate\Database\Query\Builder; class EmailFilterGambit extends AbstractRegexGambit implements FilterInterface @@ -20,7 +20,7 @@ class EmailFilterGambit extends AbstractRegexGambit implements FilterInterface /** * {@inheritdoc} */ - public function apply(AbstractSearch $search, $bit) + public function apply(SearchState $search, $bit) { if (! $search->getActor()->hasPermission('user.edit')) { return false; @@ -40,7 +40,7 @@ public function getGambitPattern() /** * {@inheritdoc} */ - protected function conditions(AbstractSearch $search, array $matches, $negate) + protected function conditions(SearchState $search, array $matches, $negate) { $this->constrain($search->getQuery(), $matches[1], $negate); } @@ -50,13 +50,13 @@ public function getFilterKey(): string return 'email'; } - public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) + public function filter(FilterState $filterState, string $filterValue, bool $negate) { - if (! $wrappedFilter->getActor()->hasPermission('user.edit')) { + if (! $filterState->getActor()->hasPermission('user.edit')) { return; } - $this->constrain($wrappedFilter->getQuery(), $filterValue, $negate); + $this->constrain($filterState->getQuery(), $filterValue, $negate); } protected function constrain(Builder $query, $rawEmail, bool $negate) diff --git a/src/User/Filter/GroupFilterGambit.php b/src/User/Filter/GroupFilterGambit.php index 8c67d5a602..4d1c445f8c 100644 --- a/src/User/Filter/GroupFilterGambit.php +++ b/src/User/Filter/GroupFilterGambit.php @@ -10,10 +10,10 @@ namespace Flarum\User\Filter; use Flarum\Filter\FilterInterface; -use Flarum\Filter\WrappedFilter; +use Flarum\Filter\FilterState; use Flarum\Group\Group; use Flarum\Search\AbstractRegexGambit; -use Flarum\Search\AbstractSearch; +use Flarum\Search\SearchState; use Flarum\User\User; use Illuminate\Database\Query\Builder; @@ -30,7 +30,7 @@ public function getGambitPattern() /** * {@inheritdoc} */ - protected function conditions(AbstractSearch $search, array $matches, $negate) + protected function conditions(SearchState $search, array $matches, $negate) { $this->constrain($search->getQuery(), $search->getActor(), $matches[1], $negate); } @@ -40,9 +40,9 @@ public function getFilterKey(): string return 'group'; } - public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) + public function filter(FilterState $filterState, string $filterValue, bool $negate) { - $this->constrain($wrappedFilter->getQuery(), $wrappedFilter->getActor(), $filterValue, $negate); + $this->constrain($filterState->getQuery(), $filterState->getActor(), $filterValue, $negate); } protected function constrain(Builder $query, User $actor, string $rawQuery, bool $negate) diff --git a/src/User/Search/Gambit/FulltextGambit.php b/src/User/Search/Gambit/FulltextGambit.php index 410d74810a..63eac123cc 100644 --- a/src/User/Search/Gambit/FulltextGambit.php +++ b/src/User/Search/Gambit/FulltextGambit.php @@ -9,7 +9,7 @@ namespace Flarum\User\Search\Gambit; -use Flarum\Search\AbstractSearch; +use Flarum\Search\SearchState; use Flarum\Search\GambitInterface; use Flarum\User\UserRepository; @@ -43,7 +43,7 @@ private function getUserSearchSubQuery($searchValue) /** * {@inheritdoc} */ - public function apply(AbstractSearch $search, $searchValue) + public function apply(SearchState $search, $searchValue) { $search->getQuery() ->whereIn( diff --git a/src/User/Search/UserSearch.php b/src/User/Search/UserSearch.php deleted file mode 100644 index 37106e5384..0000000000 --- a/src/User/Search/UserSearch.php +++ /dev/null @@ -1,16 +0,0 @@ -users->query()->whereVisibleTo($actor); } - protected function getSearch(Builder $query, User $actor): AbstractSearch - { - return new UserSearch($query->getQuery(), $actor); - } - /** * @deprecated along with the Searching event, remove in Beta 17. */ - protected function mutateSearch(AbstractSearch $search, SearchCriteria $criteria) + protected function mutateSearch(SearchState $search, SearchCriteria $criteria) { parent::mutateSearch($search, $criteria); diff --git a/tests/integration/extenders/FilterTest.php b/tests/integration/extenders/FilterTest.php index a3a5b04341..68b81bd7e4 100644 --- a/tests/integration/extenders/FilterTest.php +++ b/tests/integration/extenders/FilterTest.php @@ -13,7 +13,7 @@ use Flarum\Discussion\Filter\DiscussionFilterer; use Flarum\Extend; use Flarum\Filter\FilterInterface; -use Flarum\Filter\WrappedFilter; +use Flarum\Filter\FilterState; use Flarum\Tests\integration\RetrievesAuthorizedUsers; use Flarum\Tests\integration\TestCase; @@ -116,10 +116,10 @@ public function getFilterKey(): string /** * {@inheritdoc} */ - public function filter(WrappedFilter $wrappedFilter, string $filterValue, bool $negate) + public function filter(FilterState $filterState, string $filterValue, bool $negate) { if ($filterValue) { - $wrappedFilter->getQuery() + $filterState->getQuery() ->whereRaw('0=1'); } } diff --git a/tests/integration/extenders/SimpleFlarumSearchTest.php b/tests/integration/extenders/SimpleFlarumSearchTest.php index 0b12884296..28865742f1 100644 --- a/tests/integration/extenders/SimpleFlarumSearchTest.php +++ b/tests/integration/extenders/SimpleFlarumSearchTest.php @@ -13,7 +13,7 @@ use Flarum\Discussion\Search\DiscussionSearcher; use Flarum\Extend; use Flarum\Search\AbstractRegexGambit; -use Flarum\Search\AbstractSearch; +use Flarum\Search\SearchState; use Flarum\Search\GambitInterface; use Flarum\Search\SearchCriteria; use Flarum\Tests\integration\RetrievesAuthorizedUsers; @@ -142,7 +142,7 @@ class NoResultFullTextGambit implements GambitInterface /** * {@inheritdoc} */ - public function apply(AbstractSearch $search, $searchValue) + public function apply(SearchState $search, $searchValue) { $search->getQuery() ->whereRaw('0=1'); @@ -162,7 +162,7 @@ public function getGambitPattern() /** * {@inheritdoc} */ - public function conditions(AbstractSearch $search, array $matches, $negate) + public function conditions(SearchState $search, array $matches, $negate) { $noResults = trim($matches[1], ' '); if ($noResults == '1') { From a21796811e90dc11945ebf4ea7c1d70314ab0aba Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 17 Feb 2021 19:50:21 +0000 Subject: [PATCH 25/36] Apply fixes from StyleCI [ci skip] [skip ci] --- src/Discussion/Search/DiscussionSearcher.php | 2 +- src/Discussion/Search/Gambit/FulltextGambit.php | 2 +- src/User/Search/Gambit/FulltextGambit.php | 2 +- src/User/Search/UserSearcher.php | 2 +- tests/integration/extenders/SimpleFlarumSearchTest.php | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Discussion/Search/DiscussionSearcher.php b/src/Discussion/Search/DiscussionSearcher.php index dac0d53bc1..8755dc8839 100644 --- a/src/Discussion/Search/DiscussionSearcher.php +++ b/src/Discussion/Search/DiscussionSearcher.php @@ -11,11 +11,11 @@ use Flarum\Discussion\DiscussionRepository; use Flarum\Discussion\Event\Searching; -use Flarum\Search\SearchState; use Flarum\Search\AbstractSearcher; use Flarum\Search\GambitManager; use Flarum\Search\SearchCriteria; use Flarum\Search\SearchState; +use Flarum\Search\SearchState; use Flarum\User\User; use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Database\Eloquent\Builder; diff --git a/src/Discussion/Search/Gambit/FulltextGambit.php b/src/Discussion/Search/Gambit/FulltextGambit.php index a79a1d5495..62ff5baee6 100644 --- a/src/Discussion/Search/Gambit/FulltextGambit.php +++ b/src/Discussion/Search/Gambit/FulltextGambit.php @@ -11,8 +11,8 @@ use Flarum\Discussion\Search\DiscussionSearch; use Flarum\Post\Post; -use Flarum\Search\SearchState; use Flarum\Search\GambitInterface; +use Flarum\Search\SearchState; use Illuminate\Database\Query\Expression; use LogicException; diff --git a/src/User/Search/Gambit/FulltextGambit.php b/src/User/Search/Gambit/FulltextGambit.php index 63eac123cc..0522e76d26 100644 --- a/src/User/Search/Gambit/FulltextGambit.php +++ b/src/User/Search/Gambit/FulltextGambit.php @@ -9,8 +9,8 @@ namespace Flarum\User\Search\Gambit; -use Flarum\Search\SearchState; use Flarum\Search\GambitInterface; +use Flarum\Search\SearchState; use Flarum\User\UserRepository; class FulltextGambit implements GambitInterface diff --git a/src/User/Search/UserSearcher.php b/src/User/Search/UserSearcher.php index 09c94172a6..850c31b95a 100644 --- a/src/User/Search/UserSearcher.php +++ b/src/User/Search/UserSearcher.php @@ -9,11 +9,11 @@ namespace Flarum\User\Search; -use Flarum\Search\SearchState; use Flarum\Search\AbstractSearcher; use Flarum\Search\GambitManager; use Flarum\Search\SearchCriteria; use Flarum\Search\SearchState; +use Flarum\Search\SearchState; use Flarum\User\Event\Searching; use Flarum\User\User; use Flarum\User\UserRepository; diff --git a/tests/integration/extenders/SimpleFlarumSearchTest.php b/tests/integration/extenders/SimpleFlarumSearchTest.php index 28865742f1..52d8dfa9a6 100644 --- a/tests/integration/extenders/SimpleFlarumSearchTest.php +++ b/tests/integration/extenders/SimpleFlarumSearchTest.php @@ -13,9 +13,9 @@ use Flarum\Discussion\Search\DiscussionSearcher; use Flarum\Extend; use Flarum\Search\AbstractRegexGambit; -use Flarum\Search\SearchState; use Flarum\Search\GambitInterface; use Flarum\Search\SearchCriteria; +use Flarum\Search\SearchState; use Flarum\Tests\integration\RetrievesAuthorizedUsers; use Flarum\Tests\integration\TestCase; use Flarum\User\User; From 41835a5b00478365b502293af6b6daececc10e94 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 17 Feb 2021 14:56:58 -0500 Subject: [PATCH 26/36] Remove unnecessary load This is already done in the searcher --- src/Api/Controller/ListDiscussionsController.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Api/Controller/ListDiscussionsController.php b/src/Api/Controller/ListDiscussionsController.php index d35d55b1c0..657aae0600 100644 --- a/src/Api/Controller/ListDiscussionsController.php +++ b/src/Api/Controller/ListDiscussionsController.php @@ -110,8 +110,6 @@ protected function data(ServerRequestInterface $request, Document $document) Discussion::setStateUser($actor); - $results = $results->getResults()->load($load); - if ($relations = array_intersect($load, ['firstPost', 'lastPost'])) { foreach ($results as $discussion) { foreach ($relations as $relation) { From 40da823ef4b465be57c9f7852e953d9caf69f1df Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 17 Feb 2021 15:00:29 -0500 Subject: [PATCH 27/36] Rename $load to $include $load is the mechanism, $include is the actual important bit. --- src/Api/Controller/ListDiscussionsController.php | 8 ++++---- src/Api/Controller/ListUsersController.php | 6 +++--- src/Filter/AbstractFilterer.php | 7 ++++--- src/Search/AbstractSearcher.php | 6 ++++-- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/Api/Controller/ListDiscussionsController.php b/src/Api/Controller/ListDiscussionsController.php index 657aae0600..aba3085496 100644 --- a/src/Api/Controller/ListDiscussionsController.php +++ b/src/Api/Controller/ListDiscussionsController.php @@ -91,13 +91,13 @@ protected function data(ServerRequestInterface $request, Document $document) $limit = $this->extractLimit($request); $offset = $this->extractOffset($request); - $load = array_merge($this->extractInclude($request), ['state']); + $include = array_merge($this->extractInclude($request), ['state']); $criteria = new SearchCriteria($actor, $filters, $sort); if (array_key_exists('q', $filters)) { - $results = $this->searcher->search($criteria, $limit, $offset, $load); + $results = $this->searcher->search($criteria, $limit, $offset, $include); } else { - $results = $this->filterer->filter($criteria, $limit, $offset, $load); + $results = $this->filterer->filter($criteria, $limit, $offset, $include); } $document->addPaginationLinks( @@ -110,7 +110,7 @@ protected function data(ServerRequestInterface $request, Document $document) Discussion::setStateUser($actor); - if ($relations = array_intersect($load, ['firstPost', 'lastPost'])) { + if ($relations = array_intersect($include, ['firstPost', 'lastPost'])) { foreach ($results as $discussion) { foreach ($relations as $relation) { if ($discussion->$relation) { diff --git a/src/Api/Controller/ListUsersController.php b/src/Api/Controller/ListUsersController.php index 850ce99581..cc8ff06039 100644 --- a/src/Api/Controller/ListUsersController.php +++ b/src/Api/Controller/ListUsersController.php @@ -81,13 +81,13 @@ protected function data(ServerRequestInterface $request, Document $document) $limit = $this->extractLimit($request); $offset = $this->extractOffset($request); - $load = $this->extractInclude($request); + $include = $this->extractInclude($request); $criteria = new SearchCriteria($actor, $filters, $sort); if (array_key_exists('q', $filters)) { - $results = $this->searcher->search($criteria, $limit, $offset, $load); + $results = $this->searcher->search($criteria, $limit, $offset, $include); } else { - $results = $this->filterer->filter($criteria, $limit, $offset, $load); + $results = $this->filterer->filter($criteria, $limit, $offset, $include); } $document->addPaginationLinks( diff --git a/src/Filter/AbstractFilterer.php b/src/Filter/AbstractFilterer.php index 4c2d932c06..04e8a5f5d8 100644 --- a/src/Filter/AbstractFilterer.php +++ b/src/Filter/AbstractFilterer.php @@ -41,11 +41,12 @@ abstract protected function getQuery(User $actor): Builder; * @param SearchCriteria $criteria * @param mixed|null $limit * @param int $offset - * @param array $load + * @param array $include + * * @return SearchResults * @throws InvalidArgumentException */ - public function filter(SearchCriteria $criteria, int $limit = null, int $offset = 0, array $load = []): SearchResults + public function filter(SearchCriteria $criteria, int $limit = null, int $offset = 0, array $include = []): SearchResults { $actor = $criteria->actor; @@ -81,7 +82,7 @@ public function filter(SearchCriteria $criteria, int $limit = null, int $offset $results->pop(); } - $results->load($load); + $results->load($include); return new SearchResults($results, $areMoreResults); } diff --git a/src/Search/AbstractSearcher.php b/src/Search/AbstractSearcher.php index 9a11d13a1a..f88df558af 100644 --- a/src/Search/AbstractSearcher.php +++ b/src/Search/AbstractSearcher.php @@ -47,10 +47,12 @@ protected function mutateSearch(SearchState $search, SearchCriteria $criteria) * @param SearchCriteria $criteria * @param int|null $limit * @param int $offset + * @param array $include * * @return SearchResults + * @throws InvalidArgumentException */ - public function search(SearchCriteria $criteria, $limit = null, $offset = 0, array $load = []) + public function search(SearchCriteria $criteria, $limit = null, $offset = 0, array $include = []) { $actor = $criteria->actor; @@ -74,7 +76,7 @@ public function search(SearchCriteria $criteria, $limit = null, $offset = 0, arr $results->pop(); } - $results->load($load); + $results->load($include); return new SearchResults($results, $areMoreResults); } From 2b5b14d39015179bb6558004476753f77ca96888 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 17 Feb 2021 15:00:58 -0500 Subject: [PATCH 28/36] Typehint AbstractSearcher return value --- src/Search/AbstractSearcher.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Search/AbstractSearcher.php b/src/Search/AbstractSearcher.php index f88df558af..bd355efe4e 100644 --- a/src/Search/AbstractSearcher.php +++ b/src/Search/AbstractSearcher.php @@ -52,7 +52,7 @@ protected function mutateSearch(SearchState $search, SearchCriteria $criteria) * @return SearchResults * @throws InvalidArgumentException */ - public function search(SearchCriteria $criteria, $limit = null, $offset = 0, array $include = []) + public function search(SearchCriteria $criteria, $limit = null, $offset = 0, array $include = []): SearchResults { $actor = $criteria->actor; From ba3994b94087e8dec97a43ae3b8e4bf949ba425e Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 17 Feb 2021 15:08:25 -0500 Subject: [PATCH 29/36] Move $include out of searching / filtering On second thought, the searcher / filterer shouldn't be responsible for loading related items. It centralizes code, but isn't proper (and breaks `Discussion::setStateUser()`) --- src/Api/Controller/ListDiscussionsController.php | 2 ++ src/Api/Controller/ListUsersController.php | 2 ++ src/Filter/AbstractFilterer.php | 5 +---- src/Search/AbstractSearcher.php | 1 - 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Api/Controller/ListDiscussionsController.php b/src/Api/Controller/ListDiscussionsController.php index aba3085496..ae98bc8fad 100644 --- a/src/Api/Controller/ListDiscussionsController.php +++ b/src/Api/Controller/ListDiscussionsController.php @@ -110,6 +110,8 @@ protected function data(ServerRequestInterface $request, Document $document) Discussion::setStateUser($actor); + $results = $results->getResults()->load($include); + if ($relations = array_intersect($include, ['firstPost', 'lastPost'])) { foreach ($results as $discussion) { foreach ($relations as $relation) { diff --git a/src/Api/Controller/ListUsersController.php b/src/Api/Controller/ListUsersController.php index cc8ff06039..65f160a036 100644 --- a/src/Api/Controller/ListUsersController.php +++ b/src/Api/Controller/ListUsersController.php @@ -98,6 +98,8 @@ protected function data(ServerRequestInterface $request, Document $document) $results->areMoreResults() ? null : 0 ); + $results = $results->getResults()->load($include); + return $results->getResults(); } } diff --git a/src/Filter/AbstractFilterer.php b/src/Filter/AbstractFilterer.php index 04e8a5f5d8..c7ddc05424 100644 --- a/src/Filter/AbstractFilterer.php +++ b/src/Filter/AbstractFilterer.php @@ -41,12 +41,11 @@ abstract protected function getQuery(User $actor): Builder; * @param SearchCriteria $criteria * @param mixed|null $limit * @param int $offset - * @param array $include * * @return SearchResults * @throws InvalidArgumentException */ - public function filter(SearchCriteria $criteria, int $limit = null, int $offset = 0, array $include = []): SearchResults + public function filter(SearchCriteria $criteria, int $limit = null, int $offset = 0): SearchResults { $actor = $criteria->actor; @@ -82,8 +81,6 @@ public function filter(SearchCriteria $criteria, int $limit = null, int $offset $results->pop(); } - $results->load($include); - return new SearchResults($results, $areMoreResults); } } diff --git a/src/Search/AbstractSearcher.php b/src/Search/AbstractSearcher.php index bd355efe4e..81ac3d3414 100644 --- a/src/Search/AbstractSearcher.php +++ b/src/Search/AbstractSearcher.php @@ -47,7 +47,6 @@ protected function mutateSearch(SearchState $search, SearchCriteria $criteria) * @param SearchCriteria $criteria * @param int|null $limit * @param int $offset - * @param array $include * * @return SearchResults * @throws InvalidArgumentException From 3d0d93134ff7bed65472cb6c6006dbc9fcb6126e Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 17 Feb 2021 15:26:52 -0500 Subject: [PATCH 30/36] Fix stupid mistakes --- src/Api/Controller/ListDiscussionsController.php | 10 +++++----- src/Discussion/Search/DiscussionSearcher.php | 1 - src/Search/AbstractSearcher.php | 2 -- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/Api/Controller/ListDiscussionsController.php b/src/Api/Controller/ListDiscussionsController.php index ae98bc8fad..12c34988fc 100644 --- a/src/Api/Controller/ListDiscussionsController.php +++ b/src/Api/Controller/ListDiscussionsController.php @@ -43,6 +43,11 @@ class ListDiscussionsController extends AbstractListController 'lastPost' ]; + /** + * {@inheritDoc} + */ + public $sort = ['lastPostedAt' => 'desc']; + /** * {@inheritdoc} */ @@ -58,11 +63,6 @@ class ListDiscussionsController extends AbstractListController */ protected $searcher; - /** - * {@inheritDoc} - */ - protected $sort = ['lastPostedAt' => 'desc']; - /** * @var UrlGenerator */ diff --git a/src/Discussion/Search/DiscussionSearcher.php b/src/Discussion/Search/DiscussionSearcher.php index 8755dc8839..05a16a3bb8 100644 --- a/src/Discussion/Search/DiscussionSearcher.php +++ b/src/Discussion/Search/DiscussionSearcher.php @@ -15,7 +15,6 @@ use Flarum\Search\GambitManager; use Flarum\Search\SearchCriteria; use Flarum\Search\SearchState; -use Flarum\Search\SearchState; use Flarum\User\User; use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Database\Eloquent\Builder; diff --git a/src/Search/AbstractSearcher.php b/src/Search/AbstractSearcher.php index 81ac3d3414..1e872ef07d 100644 --- a/src/Search/AbstractSearcher.php +++ b/src/Search/AbstractSearcher.php @@ -34,8 +34,6 @@ public function __construct(GambitManager $gambits, array $searchMutators) abstract protected function getQuery(User $actor): Builder; - abstract protected function getSearch(Builder $query, User $actor): SearchState; - protected function mutateSearch(SearchState $search, SearchCriteria $criteria) { foreach ($this->searchMutators as $mutator) { From 947e5daa057b8f4f57bb53946d6ad47fff1e691f Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 17 Feb 2021 15:40:35 -0500 Subject: [PATCH 31/36] More fixes for stupid stuff, also temporarily loosen interface to avoid BC breaks --- src/Api/Controller/ListUsersController.php | 4 +- src/Discussion/Event/Searching.php | 8 ++-- .../Search/Gambit/FulltextGambit.php | 6 --- src/Search/AbstractRegexGambit.php | 3 +- src/Search/AbstractSearch.php | 45 +++++++++++++++++++ src/Search/ApplySearchParametersTrait.php | 13 +++--- src/Search/SearchState.php | 31 +------------ src/User/Event/Searching.php | 8 ++-- src/User/Search/UserSearcher.php | 1 - 9 files changed, 64 insertions(+), 55 deletions(-) create mode 100644 src/Search/AbstractSearch.php diff --git a/src/Api/Controller/ListUsersController.php b/src/Api/Controller/ListUsersController.php index 65f160a036..a511f2c40e 100644 --- a/src/Api/Controller/ListUsersController.php +++ b/src/Api/Controller/ListUsersController.php @@ -98,8 +98,6 @@ protected function data(ServerRequestInterface $request, Document $document) $results->areMoreResults() ? null : 0 ); - $results = $results->getResults()->load($include); - - return $results->getResults(); + return $results->getResults()->load($include); } } diff --git a/src/Discussion/Event/Searching.php b/src/Discussion/Event/Searching.php index 138a4f4699..dde2b47b93 100644 --- a/src/Discussion/Event/Searching.php +++ b/src/Discussion/Event/Searching.php @@ -9,8 +9,8 @@ namespace Flarum\Discussion\Event; -use Flarum\Discussion\Search\DiscussionSearch; use Flarum\Search\SearchCriteria; +use Flarum\Search\SearchState; /** * @deprecated beta 16, remove beta 17 @@ -18,7 +18,7 @@ class Searching { /** - * @var DiscussionSearch + * @var SearchState */ public $search; @@ -28,10 +28,10 @@ class Searching public $criteria; /** - * @param DiscussionSearch $search + * @param SearchState $search * @param \Flarum\Search\SearchCriteria $criteria */ - public function __construct(DiscussionSearch $search, SearchCriteria $criteria) + public function __construct(SearchState $search, SearchCriteria $criteria) { $this->search = $search; $this->criteria = $criteria; diff --git a/src/Discussion/Search/Gambit/FulltextGambit.php b/src/Discussion/Search/Gambit/FulltextGambit.php index 62ff5baee6..f57e13c763 100644 --- a/src/Discussion/Search/Gambit/FulltextGambit.php +++ b/src/Discussion/Search/Gambit/FulltextGambit.php @@ -9,12 +9,10 @@ namespace Flarum\Discussion\Search\Gambit; -use Flarum\Discussion\Search\DiscussionSearch; use Flarum\Post\Post; use Flarum\Search\GambitInterface; use Flarum\Search\SearchState; use Illuminate\Database\Query\Expression; -use LogicException; class FulltextGambit implements GambitInterface { @@ -23,10 +21,6 @@ class FulltextGambit implements GambitInterface */ public function apply(SearchState $search, $bit) { - if (! $search instanceof DiscussionSearch) { - throw new LogicException('This gambit can only be applied on a DiscussionSearch'); - } - // Replace all non-word characters with spaces. // We do this to prevent MySQL fulltext search boolean mode from taking // effect: https://dev.mysql.com/doc/refman/5.7/en/fulltext-boolean.html diff --git a/src/Search/AbstractRegexGambit.php b/src/Search/AbstractRegexGambit.php index 5100a03506..c1dd0fc2c6 100644 --- a/src/Search/AbstractRegexGambit.php +++ b/src/Search/AbstractRegexGambit.php @@ -55,5 +55,6 @@ protected function match($bit) * or not the conditions should be negated. * @return mixed */ - abstract protected function conditions(SearchState $search, array $matches, $negate); + // Uncomment for beta 17 + // abstract protected function conditions(SearchState $search, array $matches, $negate); } diff --git a/src/Search/AbstractSearch.php b/src/Search/AbstractSearch.php new file mode 100644 index 0000000000..adac8ceb23 --- /dev/null +++ b/src/Search/AbstractSearch.php @@ -0,0 +1,45 @@ +activeGambits; + } + + /** + * Add a gambit as being active in this search. + * + * @param GambitInterface $gambit + * @return void + */ + public function addActiveGambit(GambitInterface $gambit) + { + $this->activeGambits[] = $gambit; + } +} diff --git a/src/Search/ApplySearchParametersTrait.php b/src/Search/ApplySearchParametersTrait.php index e1172b0e9a..2b77a5bd8a 100644 --- a/src/Search/ApplySearchParametersTrait.php +++ b/src/Search/ApplySearchParametersTrait.php @@ -9,6 +9,7 @@ namespace Flarum\Search; +use Flarum\Filter\FilterState; use Illuminate\Support\Str; trait ApplySearchParametersTrait @@ -16,10 +17,10 @@ trait ApplySearchParametersTrait /** * Apply sort criteria to a discussion search. * - * @param SearchState $search + * @param FilterState $search * @param array $sort */ - protected function applySort(SearchState $search, array $sort = null) + protected function applySort(FilterState $search, array $sort = null) { $sort = $sort ?: $search->getDefaultSort(); @@ -39,10 +40,10 @@ protected function applySort(SearchState $search, array $sort = null) } /** - * @param SearchState $search + * @param FilterState $search * @param int $offset */ - protected function applyOffset(SearchState $search, $offset) + protected function applyOffset(FilterState $search, $offset) { if ($offset > 0) { $search->getQuery()->skip($offset); @@ -50,10 +51,10 @@ protected function applyOffset(SearchState $search, $offset) } /** - * @param SearchState $search + * @param FilterState $search * @param int|null $limit */ - protected function applyLimit(SearchState $search, $limit) + protected function applyLimit(FilterState $search, $limit) { if ($limit > 0) { $search->getQuery()->take($limit); diff --git a/src/Search/SearchState.php b/src/Search/SearchState.php index 1d13e4529a..26fe32ec4d 100644 --- a/src/Search/SearchState.php +++ b/src/Search/SearchState.php @@ -9,33 +9,4 @@ namespace Flarum\Search; -use Flarum\Filter\FilterState; - -abstract class SearchState extends FilterState -{ - /** - * @var GambitInterface[] - */ - protected $activeGambits = []; - - /** - * Get a list of the gambits that are active in this search. - * - * @return GambitInterface[] - */ - public function getActiveGambits() - { - return $this->activeGambits; - } - - /** - * Add a gambit as being active in this search. - * - * @param GambitInterface $gambit - * @return void - */ - public function addActiveGambit(GambitInterface $gambit) - { - $this->activeGambits[] = $gambit; - } -} +class SearchState extends AbstractSearch {} diff --git a/src/User/Event/Searching.php b/src/User/Event/Searching.php index ffa0db36cf..ff6debac7c 100644 --- a/src/User/Event/Searching.php +++ b/src/User/Event/Searching.php @@ -10,7 +10,7 @@ namespace Flarum\User\Event; use Flarum\Search\SearchCriteria; -use Flarum\User\Search\UserSearch; +use Flarum\Search\SearchState; /** * @deprecated beta 16, remove beta 17 @@ -18,7 +18,7 @@ class Searching { /** - * @var \Flarum\User\Search\UserSearch + * @var \Flarum\User\Search\SearchState */ public $search; @@ -28,10 +28,10 @@ class Searching public $criteria; /** - * @param UserSearch $search + * @param SearchState $search * @param SearchCriteria $criteria */ - public function __construct(UserSearch $search, SearchCriteria $criteria) + public function __construct(SearchState $search, SearchCriteria $criteria) { $this->search = $search; $this->criteria = $criteria; diff --git a/src/User/Search/UserSearcher.php b/src/User/Search/UserSearcher.php index 850c31b95a..d0ac09970c 100644 --- a/src/User/Search/UserSearcher.php +++ b/src/User/Search/UserSearcher.php @@ -13,7 +13,6 @@ use Flarum\Search\GambitManager; use Flarum\Search\SearchCriteria; use Flarum\Search\SearchState; -use Flarum\Search\SearchState; use Flarum\User\Event\Searching; use Flarum\User\User; use Flarum\User\UserRepository; From 993fbeda591a3e6111aad885f6ef565f26c6c5b5 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 17 Feb 2021 20:41:27 +0000 Subject: [PATCH 32/36] Apply fixes from StyleCI [ci skip] [skip ci] --- src/Search/SearchState.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Search/SearchState.php b/src/Search/SearchState.php index 26fe32ec4d..b57fbeee26 100644 --- a/src/Search/SearchState.php +++ b/src/Search/SearchState.php @@ -9,4 +9,6 @@ namespace Flarum\Search; -class SearchState extends AbstractSearch {} +class SearchState extends AbstractSearch +{ +} From ff4f227cb738b5d5f96a328c020497dfe14822df Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 17 Feb 2021 16:00:34 -0500 Subject: [PATCH 33/36] Don't pass unused $include to filterer/searcher --- src/Api/Controller/ListDiscussionsController.php | 4 ++-- src/Api/Controller/ListUsersController.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Api/Controller/ListDiscussionsController.php b/src/Api/Controller/ListDiscussionsController.php index 12c34988fc..c2b4af6d9f 100644 --- a/src/Api/Controller/ListDiscussionsController.php +++ b/src/Api/Controller/ListDiscussionsController.php @@ -95,9 +95,9 @@ protected function data(ServerRequestInterface $request, Document $document) $criteria = new SearchCriteria($actor, $filters, $sort); if (array_key_exists('q', $filters)) { - $results = $this->searcher->search($criteria, $limit, $offset, $include); + $results = $this->searcher->search($criteria, $limit, $offset); } else { - $results = $this->filterer->filter($criteria, $limit, $offset, $include); + $results = $this->filterer->filter($criteria, $limit, $offset); } $document->addPaginationLinks( diff --git a/src/Api/Controller/ListUsersController.php b/src/Api/Controller/ListUsersController.php index a511f2c40e..6e7bb4f846 100644 --- a/src/Api/Controller/ListUsersController.php +++ b/src/Api/Controller/ListUsersController.php @@ -85,9 +85,9 @@ protected function data(ServerRequestInterface $request, Document $document) $criteria = new SearchCriteria($actor, $filters, $sort); if (array_key_exists('q', $filters)) { - $results = $this->searcher->search($criteria, $limit, $offset, $include); + $results = $this->searcher->search($criteria, $limit, $offset); } else { - $results = $this->filterer->filter($criteria, $limit, $offset, $include); + $results = $this->filterer->filter($criteria, $limit, $offset); } $document->addPaginationLinks( From 5fb3f29291edb53a88a4cd3bb8e9b0e893c5f18a Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 17 Feb 2021 16:07:30 -0500 Subject: [PATCH 34/36] Remove includes param from AbstractSearcher --- src/Search/AbstractSearcher.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Search/AbstractSearcher.php b/src/Search/AbstractSearcher.php index 1e872ef07d..58de86243e 100644 --- a/src/Search/AbstractSearcher.php +++ b/src/Search/AbstractSearcher.php @@ -49,7 +49,7 @@ protected function mutateSearch(SearchState $search, SearchCriteria $criteria) * @return SearchResults * @throws InvalidArgumentException */ - public function search(SearchCriteria $criteria, $limit = null, $offset = 0, array $include = []): SearchResults + public function search(SearchCriteria $criteria, $limit = null, $offset = 0): SearchResults { $actor = $criteria->actor; @@ -73,8 +73,6 @@ public function search(SearchCriteria $criteria, $limit = null, $offset = 0, arr $results->pop(); } - $results->load($include); - return new SearchResults($results, $areMoreResults); } } From 5a4bc5ae724de8c292bcbd544c63476e3be72a27 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 17 Feb 2021 16:11:15 -0500 Subject: [PATCH 35/36] Add filter mutator params to docblock --- src/Extend/Filter.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Extend/Filter.php b/src/Extend/Filter.php index 5551c88e06..2cec235749 100644 --- a/src/Extend/Filter.php +++ b/src/Extend/Filter.php @@ -40,6 +40,12 @@ public function addFilter(string $filterClass) /** * Add a callback through which to run all filter queries after filters have been applied. + * + * @param callable|string $callback + * + * The callback can be a closure or an invokable class, and should accept: + * - Flarum\Filter\FilterState $filter + * - Flarum\Search\SearchCriteria $criteria */ public function addFilterMutator($callback) { From 97ac81abb77f42676e09fcf3412e73f1241cc69f Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 17 Feb 2021 19:33:42 -0500 Subject: [PATCH 36/36] Capitalization fix --- src/Extend/Filter.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Extend/Filter.php b/src/Extend/Filter.php index 2cec235749..49f54c5419 100644 --- a/src/Extend/Filter.php +++ b/src/Extend/Filter.php @@ -19,7 +19,7 @@ class Filter implements ExtenderInterface private $filterMutators = []; /** - * @param string $filtererclass: The ::class attribute of the filterer to extend + * @param string $filtererClass: The ::class attribute of the filterer to extend */ public function __construct($filtererClass) { @@ -27,7 +27,7 @@ public function __construct($filtererClass) } /** - * Add a filter to run when the filtererclass is filtered. + * Add a filter to run when the filtererClass is filtered. * * @param string $filterClass: The ::class attribute of the filter you are adding. */