From 76680d197dcb25b3cd33ad153ee71d24f9938658 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 11 Nov 2020 17:16:56 -0500 Subject: [PATCH 1/8] Introduce RequestUtil Abstracts access to following request attributes: - actor - session - locale - route name --- src/Http/RequestUtil.php | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 src/Http/RequestUtil.php diff --git a/src/Http/RequestUtil.php b/src/Http/RequestUtil.php new file mode 100644 index 0000000000..c1055c84e3 --- /dev/null +++ b/src/Http/RequestUtil.php @@ -0,0 +1,56 @@ +getAttribute('actor'); + } + + public function withActor(Request $request, User $actor): Request + { + return $request->withAttribute('actor', $actor); + } + + public function getSession(Request $request): Session + { + return $request->getAttribute('session'); + } + + public function withSession(Request $request, Session $session): Request + { + return $request->withAttribute('session', $session); + } + + public function getLocale(Request $request): string + { + return $request->getAttribute('bypassCsrfToken'); + } + + public function withLocale(Request $request, string $locale): Request + { + return $request->withAttribute('locale', $locale); + } + + public function getRouteName(Request $request): string + { + return $request->getAttribute('routeName'); + } + + public function withRouteName(Request $request, string $routeName): Request + { + return $request->withAttribute('routeName', $routeName); + } +} From 0172008693d4454861e194a2b06effae719d0380 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Thu, 12 Nov 2020 14:20:18 -0500 Subject: [PATCH 2/8] Add Search controllers for Discussions and Users, as copies of the ListDiscussions and ListUsers controllers --- .../SearchDiscussionsController.php | 112 ++++++++++++++++++ src/Api/Controller/SearchUsersController.php | 93 +++++++++++++++ src/Api/routes.php | 14 +++ 3 files changed, 219 insertions(+) create mode 100644 src/Api/Controller/SearchDiscussionsController.php create mode 100644 src/Api/Controller/SearchUsersController.php diff --git a/src/Api/Controller/SearchDiscussionsController.php b/src/Api/Controller/SearchDiscussionsController.php new file mode 100644 index 0000000000..f2dc007744 --- /dev/null +++ b/src/Api/Controller/SearchDiscussionsController.php @@ -0,0 +1,112 @@ +searcher = $searcher; + $this->url = $url; + } + + /** + * {@inheritdoc} + */ + protected function data(ServerRequestInterface $request, Document $document) + { + $actor = $request->getAttribute('actor'); + $query = Arr::get($this->extractFilter($request), 'q'); + $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); + + $document->addPaginationLinks( + $this->url->to('api')->route('discussions.index'), + $request->getQueryParams(), + $offset, + $limit, + $results->areMoreResults() ? null : 0 + ); + + Discussion::setStateUser($actor); + + $results = $results->getResults()->load($load); + + if ($relations = array_intersect($load, ['firstPost', 'lastPost'])) { + foreach ($results as $discussion) { + foreach ($relations as $relation) { + if ($discussion->$relation) { + $discussion->$relation->discussion = $discussion; + } + } + } + } + + return $results; + } +} diff --git a/src/Api/Controller/SearchUsersController.php b/src/Api/Controller/SearchUsersController.php new file mode 100644 index 0000000000..997348db5e --- /dev/null +++ b/src/Api/Controller/SearchUsersController.php @@ -0,0 +1,93 @@ +searcher = $searcher; + $this->url = $url; + } + + /** + * {@inheritdoc} + */ + protected function data(ServerRequestInterface $request, Document $document) + { + $actor = $request->getAttribute('actor'); + + $actor->assertCan('viewUserList'); + + $query = Arr::get($this->extractFilter($request), 'q'); + $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); + + $document->addPaginationLinks( + $this->url->to('api')->route('users.index'), + $request->getQueryParams(), + $offset, + $limit, + $results->areMoreResults() ? null : 0 + ); + + return $results->getResults(); + } +} diff --git a/src/Api/routes.php b/src/Api/routes.php index 778da8bfdd..c603f35a62 100644 --- a/src/Api/routes.php +++ b/src/Api/routes.php @@ -95,6 +95,13 @@ $route->toController(Controller\SendConfirmationEmailController::class) ); + // List users + $map->get( + '/search/users', + 'users.search', + $route->toController(Controller\SearchUsersController::class) + ); + /* |-------------------------------------------------------------------------- | Notifications @@ -163,6 +170,13 @@ $route->toController(Controller\DeleteDiscussionController::class) ); + // Search discussions + $map->get( + '/search/discussions', + 'discussions.search', + $route->toController(Controller\SearchDiscussionsController::class) + ); + /* |-------------------------------------------------------------------------- | Posts From 87d2f3d246c1f3df64aa6f9e5c4633da7b4506b6 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Thu, 12 Nov 2020 14:29:00 -0500 Subject: [PATCH 3/8] If a search parameter is present (filter.q), send requests to the search endpoint instead of the filter endpoint --- js/src/common/Store.js | 2 +- js/src/forum/components/DiscussionsSearchSource.js | 2 +- js/src/forum/components/UsersSearchSource.js | 12 ++++++++---- js/src/forum/states/DiscussionListState.js | 2 +- src/Forum/Content/Index.php | 3 ++- 5 files changed, 13 insertions(+), 8 deletions(-) diff --git a/js/src/common/Store.js b/js/src/common/Store.js index 0805b8202a..42ee1433f5 100644 --- a/js/src/common/Store.js +++ b/js/src/common/Store.js @@ -83,7 +83,7 @@ export default class Store { */ find(type, id, query = {}, options = {}) { let params = query; - let url = app.forum.attribute('apiUrl') + '/' + type; + let url = app.forum.attribute('apiUrl') + (query.search ? '/search/' : '/') + type; if (id instanceof Array) { url += '?filter[id]=' + id.join(','); diff --git a/js/src/forum/components/DiscussionsSearchSource.js b/js/src/forum/components/DiscussionsSearchSource.js index 39d5a4307a..9d2302bdb6 100644 --- a/js/src/forum/components/DiscussionsSearchSource.js +++ b/js/src/forum/components/DiscussionsSearchSource.js @@ -24,7 +24,7 @@ export default class DiscussionsSearchSource { include: 'mostRelevantPost', }; - return app.store.find('discussions', params).then((results) => (this.results[query] = results)); + return app.store.find('discussions', params, { search: query }).then((results) => (this.results[query] = results)); } view(query) { diff --git a/js/src/forum/components/UsersSearchSource.js b/js/src/forum/components/UsersSearchSource.js index de0a506357..a34d59e19c 100644 --- a/js/src/forum/components/UsersSearchSource.js +++ b/js/src/forum/components/UsersSearchSource.js @@ -16,10 +16,14 @@ export default class UsersSearchResults { search(query) { return app.store - .find('users', { - filter: { q: query }, - page: { limit: 5 }, - }) + .find( + 'users', + { + filter: { q: query }, + page: { limit: 5 }, + }, + { search: query } + ) .then((results) => { this.results[query] = results; m.redraw(); diff --git a/js/src/forum/states/DiscussionListState.js b/js/src/forum/states/DiscussionListState.js index 57f4396a83..5c5f71cff0 100644 --- a/js/src/forum/states/DiscussionListState.js +++ b/js/src/forum/states/DiscussionListState.js @@ -119,7 +119,7 @@ export default class DiscussionListState { params.page = { offset }; params.include = params.include.join(','); - return this.app.store.find('discussions', params); + return this.app.store.find('discussions', params, { search: params.filter.q }); } /** diff --git a/src/Forum/Content/Index.php b/src/Forum/Content/Index.php index 2f0d506e0c..100125ef94 100644 --- a/src/Forum/Content/Index.php +++ b/src/Forum/Content/Index.php @@ -11,6 +11,7 @@ use Flarum\Api\Client; use Flarum\Api\Controller\ListDiscussionsController; +use Flarum\Api\Controller\SearchDiscussionsController; use Flarum\Frontend\Document; use Flarum\Http\UrlGenerator; use Flarum\Settings\SettingsRepositoryInterface; @@ -114,6 +115,6 @@ private function getSortMap() */ private function getApiDocument(User $actor, array $params) { - return json_decode($this->api->send(ListDiscussionsController::class, $actor, $params)->getBody()); + return json_decode($this->api->send(($params['filter']['q'] ? SearchDiscussionsController::class : ListDiscussionsController::class), $actor, $params)->getBody()); } } From c4daeeb1f2682528be2d2689e6bb42575ba79ac6 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 13 Nov 2020 01:47:15 -0500 Subject: [PATCH 4/8] Add abstract filterer system, have discussions and users controllers use that instead of default filterer system --- .../Controller/ListDiscussionsController.php | 27 +++--- src/Api/Controller/ListUsersController.php | 30 ++++--- src/Filter/Filterer.php | 84 +++++++++++++++++++ src/Filter/WrappedFilter.php | 16 ++++ 4 files changed, 135 insertions(+), 22 deletions(-) create mode 100644 src/Filter/Filterer.php create mode 100644 src/Filter/WrappedFilter.php diff --git a/src/Api/Controller/ListDiscussionsController.php b/src/Api/Controller/ListDiscussionsController.php index dc825d9cd8..574c323626 100644 --- a/src/Api/Controller/ListDiscussionsController.php +++ b/src/Api/Controller/ListDiscussionsController.php @@ -11,10 +11,10 @@ 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; use Psr\Http\Message\ServerRequestInterface; use Tobscure\JsonApi\Document; @@ -49,9 +49,14 @@ class ListDiscussionsController extends AbstractListController public $sortFields = ['lastPostedAt', 'commentCount', 'createdAt']; /** - * @var DiscussionSearcher + * @var DiscussionRepository */ - protected $searcher; + protected $discussions; + + /** + * @var Filterer + */ + protected $filterer; /** * @var UrlGenerator @@ -62,9 +67,10 @@ class ListDiscussionsController extends AbstractListController * @param DiscussionSearcher $searcher * @param UrlGenerator $url */ - public function __construct(DiscussionSearcher $searcher, UrlGenerator $url) + public function __construct(DiscussionRepository $discussions, Filterer $filterer, UrlGenerator $url) { - $this->searcher = $searcher; + $this->discussions = $discussions; + $this->filterer = $filterer; $this->url = $url; } @@ -74,16 +80,17 @@ 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'); - $sort = $this->extractSort($request); - $criteria = new SearchCriteria($actor, $query, $sort); + + $filters = $this->extractFilter($request); + $sort = $this->extractSort($request); + $query = $this->discussions->query(); $limit = $this->extractLimit($request); $offset = $this->extractOffset($request); $load = array_merge($this->extractInclude($request), ['state']); - $results = $this->searcher->search($criteria, $limit, $offset); + $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..d991771940 100644 --- a/src/Api/Controller/ListUsersController.php +++ b/src/Api/Controller/ListUsersController.php @@ -10,10 +10,9 @@ 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 Illuminate\Support\Arr; +use Flarum\User\UserRepository; use Psr\Http\Message\ServerRequestInterface; use Tobscure\JsonApi\Document; @@ -41,9 +40,9 @@ class ListUsersController extends AbstractListController ]; /** - * @var UserSearcher + * @var Filterer */ - protected $searcher; + protected $filterer; /** * @var UrlGenerator @@ -51,13 +50,20 @@ class ListUsersController extends AbstractListController protected $url; /** - * @param UserSearcher $searcher + * @var UserRepository + */ + protected $users; + + /** + * @param Filterer $filterer * @param UrlGenerator $url + * @param UserRepository $users */ - public function __construct(UserSearcher $searcher, UrlGenerator $url) + public function __construct(Filterer $filterer, UrlGenerator $url, UserRepository $users) { - $this->searcher = $searcher; + $this->filterer = $filterer; $this->url = $url; + $this->users = $users; } /** @@ -69,16 +75,16 @@ protected function data(ServerRequestInterface $request, Document $document) $actor->assertCan('viewUserList'); - $query = Arr::get($this->extractFilter($request), 'q'); - $sort = $this->extractSort($request); + $query = $this->users->query(); - $criteria = new SearchCriteria($actor, $query, $sort); + $filters = $this->extractFilter($request); + $sort = $this->extractSort($request); $limit = $this->extractLimit($request); $offset = $this->extractOffset($request); $load = $this->extractInclude($request); - $results = $this->searcher->search($criteria, $limit, $offset, $load); + $results = $this->filterer->filter($actor, $query, $filters, $sort, $limit, $offset, $load); $document->addPaginationLinks( $this->url->to('api')->route('users.index'), diff --git a/src/Filter/Filterer.php b/src/Filter/Filterer.php new file mode 100644 index 0000000000..cd844fcde4 --- /dev/null +++ b/src/Filter/Filterer.php @@ -0,0 +1,84 @@ +getModel()); + + $query->whereVisibleTo($actor); + + foreach (Arr::get(static::$filters, $resource, []) as $filterKey => $filterCallback) { + if (array_key_exists($filterKey, $filters)) { + $filterCallback($query, $filters[$filterKey]); + } + } + + $wrappedFilter = new WrappedFilter($query->getQuery(), $actor); + + $this->applySort($wrappedFilter, $sort); + $this->applyOffset($wrappedFilter, $offset); + $this->applyLimit($wrappedFilter, $limit + 1); + + foreach (Arr::get(static::$filterMutators, $resource, []) as $mutator) { + $mutator($wrappedFilter, $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 @@ + Date: Fri, 13 Nov 2020 02:55:17 -0500 Subject: [PATCH 5/8] Ensure that search AND list endpoints are tested for users and discussions --- .../integration/api/discussions/ListTest.php | 122 ++----------- .../api/discussions/SearchTest.php | 166 ++++++++++++++++++ tests/integration/api/users/SearchTest.php | 83 +++++++++ 3 files changed, 264 insertions(+), 107 deletions(-) create mode 100644 tests/integration/api/discussions/SearchTest.php create mode 100644 tests/integration/api/users/SearchTest.php diff --git a/tests/integration/api/discussions/ListTest.php b/tests/integration/api/discussions/ListTest.php index 3e56c41509..56741861ea 100644 --- a/tests/integration/api/discussions/ListTest.php +++ b/tests/integration/api/discussions/ListTest.php @@ -56,111 +56,19 @@ public function shows_index_for_guest() $this->assertEquals(1, count($data['data'])); } - /** - * @test - */ - public function can_search_for_author() - { - $response = $this->send( - $this->request('GET', '/api/discussions') - ->withQueryParams([ - 'filter' => ['q' => 'author:normal foo'], - 'include' => 'mostRelevantPost', - ]) - ); - - $this->assertEquals(200, $response->getStatusCode()); - } - - /** - * @test - */ - public function can_search_for_word_in_post() - { - $this->database()->table('discussions')->insert([ - ['id' => 2, 'title' => 'lightsail in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'comment_count' => 1], - ['id' => 3, 'title' => 'not in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'comment_count' => 1], - ]); - - $this->database()->table('posts')->insert([ - ['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

not in text

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

lightsail in text

'], - ]); - - $response = $this->send( - $this->request('GET', '/api/discussions') - ->withQueryParams([ - 'filter' => ['q' => 'lightsail'], - 'include' => 'mostRelevantPost', - ]) - ); - - $data = json_decode($response->getBody()->getContents(), true); - $ids = array_map(function ($row) { - return $row['id']; - }, $data['data']); - - // Order-independent comparison - $this->assertEquals(['3'], $ids, 'IDs do not match', 0.0, 10, true); - } - - /** - * @test - */ - public function ignores_non_word_characters_when_searching() - { - $this->database()->table('discussions')->insert([ - ['id' => 2, 'title' => 'lightsail in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'comment_count' => 1], - ['id' => 3, 'title' => 'not in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'comment_count' => 1], - ]); - - $this->database()->table('posts')->insert([ - ['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

not in text

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

lightsail in text

'], - ]); - - $response = $this->send( - $this->request('GET', '/api/discussions') - ->withQueryParams([ - 'filter' => ['q' => 'lightsail+'], - 'include' => 'mostRelevantPost', - ]) - ); - - $data = json_decode($response->getBody()->getContents(), true); - $ids = array_map(function ($row) { - return $row['id']; - }, $data['data']); - - // Order-independent comparison - $this->assertEquals(['3'], $ids, '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 can_search_for_author() + // { + // $response = $this->send( + // $this->request('GET', '/api/search/discussions') + // ->withQueryParams([ + // 'filter' => ['q' => 'author:normal foo'], + // 'include' => 'mostRelevantPost', + // ]) + // ); + + // $this->assertEquals(200, $response->getStatusCode()); + // } } diff --git a/tests/integration/api/discussions/SearchTest.php b/tests/integration/api/discussions/SearchTest.php new file mode 100644 index 0000000000..5919c3c73f --- /dev/null +++ b/tests/integration/api/discussions/SearchTest.php @@ -0,0 +1,166 @@ +prepareDatabase([ + 'discussions' => [ + ['id' => 1, 'title' => __CLASS__, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'first_post_id' => 1, 'comment_count' => 1], + ], + 'posts' => [ + ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

foo bar

'], + ], + 'users' => [ + $this->normalUser(), + ], + 'groups' => [ + $this->memberGroup(), + $this->guestGroup(), + ], + 'group_permission' => [ + ['permission' => 'viewDiscussions', 'group_id' => 2], + ] + ]); + } + + /** + * @test + */ + public function shows_index_for_guest() + { + $response = $this->send( + $this->request('GET', '/api/search/discussions') + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true); + + $this->assertEquals(1, count($data['data'])); + } + + /** + * @test + */ + public function can_search_for_author() + { + $response = $this->send( + $this->request('GET', '/api/search/discussions') + ->withQueryParams([ + 'filter' => ['q' => 'author:normal foo'], + 'include' => 'mostRelevantPost', + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + } + + /** + * @test + */ + public function can_search_for_word_in_post() + { + $this->database()->table('discussions')->insert([ + ['id' => 2, 'title' => 'lightsail in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'comment_count' => 1], + ['id' => 3, 'title' => 'not in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'comment_count' => 1], + ]); + + $this->database()->table('posts')->insert([ + ['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

not in text

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

lightsail in text

'], + ]); + + $response = $this->send( + $this->request('GET', '/api/search/discussions') + ->withQueryParams([ + 'filter' => ['q' => 'lightsail'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true); + $ids = array_map(function ($row) { + return $row['id']; + }, $data['data']); + + // Order-independent comparison + $this->assertEquals(['3'], $ids, 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function ignores_non_word_characters_when_searching() + { + $this->database()->table('discussions')->insert([ + ['id' => 2, 'title' => 'lightsail in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'comment_count' => 1], + ['id' => 3, 'title' => 'not in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'comment_count' => 1], + ]); + + $this->database()->table('posts')->insert([ + ['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

not in text

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

lightsail in text

'], + ]); + + $response = $this->send( + $this->request('GET', '/api/search/discussions') + ->withQueryParams([ + 'filter' => ['q' => 'lightsail+'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true); + $ids = array_map(function ($row) { + return $row['id']; + }, $data['data']); + + // Order-independent comparison + $this->assertEquals(['3'], $ids, '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/search/discussions') + ->withQueryParams([ + 'filter' => ['q' => '*'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true); + $this->assertEquals([], $data['data']); + + $response = $this->send( + $this->request('GET', '/api/search/discussions') + ->withQueryParams([ + 'filter' => ['q' => '@'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true); + $this->assertEquals([], $data['data']); + } +} diff --git a/tests/integration/api/users/SearchTest.php b/tests/integration/api/users/SearchTest.php new file mode 100644 index 0000000000..9a33728e19 --- /dev/null +++ b/tests/integration/api/users/SearchTest.php @@ -0,0 +1,83 @@ +prepareDatabase([ + 'users' => [ + $this->adminUser(), + ], + 'groups' => [ + $this->adminGroup(), + $this->guestGroup(), + ], + 'group_permission' => [], + 'group_user' => [ + ['user_id' => 1, 'group_id' => 1], + ], + ]); + } + + /** + * @test + */ + public function disallows_index_for_guest() + { + $response = $this->send( + $this->request('GET', '/api/search/users') + ); + + $this->assertEquals(403, $response->getStatusCode()); + } + + /** + * @test + */ + public function shows_index_for_guest_when_they_have_permission() + { + Permission::unguarded(function () { + Permission::create([ + 'permission' => 'viewUserList', + 'group_id' => 2, + ]); + }); + + $response = $this->send( + $this->request('GET', '/api/search/users') + ); + + $this->assertEquals(200, $response->getStatusCode()); + } + + /** + * @test + */ + public function shows_index_for_admin() + { + $response = $this->send( + $this->request('GET', '/api/search/users', [ + 'authenticatedAs' => 1, + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + } +} From 0b2d01b75f5f0e6b3bb6bfd97217f8d294e56f30 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 13 Nov 2020 02:55:58 -0500 Subject: [PATCH 6/8] Define FilterInterface --- src/Filter/FilterInterface.php | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 src/Filter/FilterInterface.php diff --git a/src/Filter/FilterInterface.php b/src/Filter/FilterInterface.php new file mode 100644 index 0000000000..87cd60cf99 --- /dev/null +++ b/src/Filter/FilterInterface.php @@ -0,0 +1,26 @@ + Date: Fri, 13 Nov 2020 02:56:13 -0500 Subject: [PATCH 7/8] Add extender and tests for filter --- src/Extend/Filter.php | 65 ++++++++++ src/Filter/Filterer.php | 20 +-- tests/integration/extenders/FilterTest.php | 138 +++++++++++++++++++++ 3 files changed, 215 insertions(+), 8 deletions(-) create mode 100644 src/Extend/Filter.php create mode 100644 tests/integration/extenders/FilterTest.php diff --git a/src/Extend/Filter.php b/src/Extend/Filter.php new file mode 100644 index 0000000000..65c3156abf --- /dev/null +++ b/src/Extend/Filter.php @@ -0,0 +1,65 @@ +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/Filterer.php b/src/Filter/Filterer.php index cd844fcde4..0d77efca38 100644 --- a/src/Filter/Filterer.php +++ b/src/Filter/Filterer.php @@ -21,13 +21,17 @@ class Filterer protected static $filterMutators = []; - public static function addFilter($resource, $filterKey, $filter) + public static function addFilter($resource, FilterInterface $filter) { if (!array_key_exists($resource, static::$filters)) { static::$filters[$resource] = []; } - static::$filters[$resource][$filterKey] = $filter; + if (!array_key_exists($filter->getKey(), static::$filters[$resource])) { + static::$filters[$resource][$filter->getKey()] = []; + } + + static::$filters[$resource][$filter->getKey()][] = $filter; } public static function addFilterMutator($resource, $mutator) @@ -52,20 +56,20 @@ public function filter($actor, $query, $filters, $sort = null, $limit = null, $o $query->whereVisibleTo($actor); - foreach (Arr::get(static::$filters, $resource, []) as $filterKey => $filterCallback) { - if (array_key_exists($filterKey, $filters)) { - $filterCallback($query, $filters[$filterKey]); + $wrappedFilter = new WrappedFilter($query->getQuery(), $actor); + + foreach ($filters as $filterKey => $filterValue) { + foreach (Arr::get(static::$filters, "$resource.$filterKey", []) as $filter) { + $filter->apply($wrappedFilter, $filterValue); } } - $wrappedFilter = new WrappedFilter($query->getQuery(), $actor); - $this->applySort($wrappedFilter, $sort); $this->applyOffset($wrappedFilter, $offset); $this->applyLimit($wrappedFilter, $limit + 1); foreach (Arr::get(static::$filterMutators, $resource, []) as $mutator) { - $mutator($wrappedFilter, $filters, $sort); + $mutator($query, $actor, $filters, $sort); } // Execute the filter query and retrieve the results. We get one more diff --git a/tests/integration/extenders/FilterTest.php b/tests/integration/extenders/FilterTest.php new file mode 100644 index 0000000000..5ac67ba7d1 --- /dev/null +++ b/tests/integration/extenders/FilterTest.php @@ -0,0 +1,138 @@ +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 getKey(): string + { + return 'noResult'; + } + + /** + * {@inheritdoc} + */ + public function apply(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 f2aa804753371df8cc14410129d902c8a0735d22 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 13 Nov 2020 07:56:37 +0000 Subject: [PATCH 8/8] Apply fixes from StyleCI [ci skip] [skip ci] --- src/Api/Controller/ListDiscussionsController.php | 1 - src/Extend/Filter.php | 4 +--- src/Filter/Filterer.php | 6 +++--- src/Http/RequestUtil.php | 3 ++- tests/integration/extenders/FilterTest.php | 3 --- 5 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/Api/Controller/ListDiscussionsController.php b/src/Api/Controller/ListDiscussionsController.php index 574c323626..335a62c4f2 100644 --- a/src/Api/Controller/ListDiscussionsController.php +++ b/src/Api/Controller/ListDiscussionsController.php @@ -81,7 +81,6 @@ protected function data(ServerRequestInterface $request, Document $document) { $actor = $request->getAttribute('actor'); - $filters = $this->extractFilter($request); $sort = $this->extractSort($request); $query = $this->discussions->query(); diff --git a/src/Extend/Filter.php b/src/Extend/Filter.php index 65c3156abf..50f040d3d9 100644 --- a/src/Extend/Filter.php +++ b/src/Extend/Filter.php @@ -11,7 +11,6 @@ use Flarum\Extension\Extension; use Flarum\Filter\Filterer; -use Flarum\Filter\FilterInterface; use Flarum\Foundation\ContainerUtil; use Illuminate\Contracts\Container\Container; @@ -30,7 +29,7 @@ public function __construct($resource) } /** - * Add a filter to run when the resource is filtered + * Add a filter to run when the resource is filtered. * * @param string $filterClass: The ::class attribute of the filter you are adding. */ @@ -53,7 +52,6 @@ public function addFilterMutator($callback) public function extend(Container $container, Extension $extension = null) { - foreach ($this->filters as $filter) { Filterer::addFilter($this->resource, $container->make($filter)); } diff --git a/src/Filter/Filterer.php b/src/Filter/Filterer.php index 0d77efca38..cf67227c71 100644 --- a/src/Filter/Filterer.php +++ b/src/Filter/Filterer.php @@ -23,11 +23,11 @@ class Filterer public static function addFilter($resource, FilterInterface $filter) { - if (!array_key_exists($resource, static::$filters)) { + if (! array_key_exists($resource, static::$filters)) { static::$filters[$resource] = []; } - if (!array_key_exists($filter->getKey(), static::$filters[$resource])) { + if (! array_key_exists($filter->getKey(), static::$filters[$resource])) { static::$filters[$resource][$filter->getKey()] = []; } @@ -36,7 +36,7 @@ public static function addFilter($resource, FilterInterface $filter) public static function addFilterMutator($resource, $mutator) { - if (!array_key_exists($resource, static::$filterMutators)) { + if (! array_key_exists($resource, static::$filterMutators)) { static::$filterMutators[$resource] = []; } diff --git a/src/Http/RequestUtil.php b/src/Http/RequestUtil.php index c1055c84e3..05e19761b7 100644 --- a/src/Http/RequestUtil.php +++ b/src/Http/RequestUtil.php @@ -15,7 +15,8 @@ class RequestUtil { - public static function getActor(Request $request): User { + public static function getActor(Request $request): User + { return $request->getAttribute('actor'); } diff --git a/tests/integration/extenders/FilterTest.php b/tests/integration/extenders/FilterTest.php index 5ac67ba7d1..8293e0eb30 100644 --- a/tests/integration/extenders/FilterTest.php +++ b/tests/integration/extenders/FilterTest.php @@ -11,14 +11,11 @@ use Carbon\Carbon; use Flarum\Discussion\Discussion; -use Flarum\Discussion\Search\DiscussionSearcher; use Flarum\Extend; use Flarum\Filter\FilterInterface; use Flarum\Filter\WrappedFilter; -use Flarum\Search\AbstractSearch; use Flarum\Tests\integration\RetrievesAuthorizedUsers; use Flarum\Tests\integration\TestCase; -use Flarum\User\User; class FilterTest extends TestCase {