Use filterer for ListPostsController (#2479)

src/Api/Controller/ListPostsController.php

@@ -10,11 +10,10 @@
 namespace Flarum\Api\Controller;
 
 use Flarum\Api\Serializer\PostSerializer;
-use Flarum\Event\ConfigurePostsQuery;
-use Flarum\Post\PostRepository;
-use Illuminate\Database\Eloquent\Builder;
+use Flarum\Http\UrlGenerator;
+use Flarum\Post\Filter\PostFilterer;
+use Flarum\Search\SearchCriteria;
 use Illuminate\Support\Arr;
-use Illuminate\Support\Str;
 use Psr\Http\Message\ServerRequestInterface;
 use Tobscure\JsonApi\Document;
 use Tobscure\JsonApi\Exception\InvalidParameterException;
@@ -43,16 +42,23 @@ class ListPostsController extends AbstractListController
     public $sortFields = ['createdAt'];
 
     /**
-     * @var \Flarum\Post\PostRepository
+     * @var PostFilterer
      */
-    protected $posts;
+    protected $filterer;
 
     /**
-     * @param \Flarum\Post\PostRepository $posts
+     * @var UrlGenerator
      */
-    public function __construct(PostRepository $posts)
+    protected $url;
+
+    /**
+     * @param PostFilterer $filterer
+     * @param UrlGenerator $url
+     */
+    public function __construct(PostFilterer $filterer, UrlGenerator $url)
     {
-        $this->posts = $posts;
+        $this->filterer = $filterer;
+        $this->url = $url;
     }
 
     /**
@@ -61,18 +67,25 @@ class ListPostsController extends AbstractListController
     protected function data(ServerRequestInterface $request, Document $document)
     {
         $actor = $request->getAttribute('actor');
-        $filter = $this->extractFilter($request);
+
+        $filters = $this->extractFilter($request);
+        $sort = $this->extractSort($request);
+
+        $limit = $this->extractLimit($request);
+        $offset = $this->extractOffset($request);
         $include = $this->extractInclude($request);
 
-        if ($postIds = Arr::get($filter, 'id')) {
-            $postIds = explode(',', $postIds);
-        } else {
-            $postIds = $this->getPostIds($request);
-        }
+        $results = $this->filterer->filter(new SearchCriteria($actor, $filters, $sort), $limit, $offset);
 
-        $posts = $this->posts->findByIds($postIds, $actor);
+        $document->addPaginationLinks(
+            $this->url->to('api')->route('posts.index'),
+            $request->getQueryParams(),
+            $offset,
+            $limit,
+            $results->areMoreResults() ? null : 0
+        );
 
-        return $posts->load($include);
+        return $results->getResults()->load($include);
     }
 
     /**
@@ -100,55 +113,4 @@ class ListPostsController extends AbstractListController
 
         return parent::extractOffset($request);
     }
-
-    /**
-     * @param ServerRequestInterface $request
-     * @return array
-     * @throws InvalidParameterException
-     */
-    private function getPostIds(ServerRequestInterface $request)
-    {
-        $actor = $request->getAttribute('actor');
-        $filter = $this->extractFilter($request);
-        $sort = $this->extractSort($request);
-        $limit = $this->extractLimit($request);
-        $offset = $this->extractOffset($request);
-
-        $query = $this->posts->query()->whereVisibleTo($actor);
-
-        $this->applyFilters($query, $filter);
-
-        $query->skip($offset)->take($limit);
-
-        foreach ((array) $sort as $field => $order) {
-            $query->orderBy(Str::snake($field), $order);
-        }
-
-        return $query->pluck('id')->all();
-    }
-
-    /**
-     * @param Builder $query
-     * @param array $filter
-     */
-    private function applyFilters(Builder $query, array $filter)
-    {
-        if ($discussionId = Arr::get($filter, 'discussion')) {
-            $query->where('discussion_id', $discussionId);
-        }
-
-        if ($number = Arr::get($filter, 'number')) {
-            $query->where('number', $number);
-        }
-
-        if ($userId = Arr::get($filter, 'user')) {
-            $query->where('user_id', $userId);
-        }
-
-        if ($type = Arr::get($filter, 'type')) {
-            $query->where('type', $type);
-        }
-
-        event(new ConfigurePostsQuery($query, $filter));
-    }
 }

src/Event/ConfigurePostsQuery.php

@@ -11,6 +11,9 @@ namespace Flarum\Event;
 
 use Illuminate\Database\Eloquent\Builder;
 
+/**
+ * @deprecated beta 16, remove beta 17.
+ */
 class ConfigurePostsQuery
 {
     /**

src/Filter/AbstractFilterer.php

@@ -9,6 +9,8 @@
 
 namespace Flarum\Filter;
 
+use Flarum\Event\ConfigurePostsQuery;
+use Flarum\Post\Filter\PostFilterer;
 use Flarum\Search\ApplySearchParametersTrait;
 use Flarum\Search\SearchCriteria;
 use Flarum\Search\SearchResults;
@@ -68,6 +70,12 @@ abstract class AbstractFilterer
         $this->applyOffset($filterState, $offset);
         $this->applyLimit($filterState, $limit + 1);
 
+        // DEPRECATED BC LAYER, REMOVE BETA 17
+        if (static::class === PostFilterer::class) {
+            event(new ConfigurePostsQuery($query, $criteria->query));
+        }
+        // END DEPRECATED BC LAYER
+
         foreach ($this->filterMutators as $mutator) {
             $mutator($query, $actor, $criteria->query, $criteria->sort);
         }

src/Filter/FilterServiceProvider.php

@@ -9,15 +9,13 @@
 
 namespace Flarum\Filter;
 
-use Flarum\Discussion\Filter\AuthorFilterGambit;
-use Flarum\Discussion\Filter\CreatedFilterGambit;
+use Flarum\Discussion\Filter as DiscussionFilter;
 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\Post\Filter as PostFilter;
+use Flarum\Post\Filter\PostFilterer;
+use Flarum\User\Filter as UserFilter;
 use Flarum\User\Filter\UserFilterer;
 use Illuminate\Support\Arr;
 
@@ -33,15 +31,22 @@ class FilterServiceProvider extends AbstractServiceProvider
         $this->app->singleton('flarum.filter.filters', function () {
             return [
                 DiscussionFilterer::class => [
-                    AuthorFilterGambit::class,
-                    CreatedFilterGambit::class,
-                    HiddenFilterGambit::class,
-                    UnreadFilterGambit::class,
+                    DiscussionFilter\AuthorFilterGambit::class,
+                    DiscussionFilter\CreatedFilterGambit::class,
+                    DiscussionFilter\HiddenFilterGambit::class,
+                    DiscussionFilter\UnreadFilterGambit::class,
                 ],
                 UserFilterer::class => [
-                    EmailFilterGambit::class,
-                    GroupFilterGambit::class,
-                ]
+                    UserFilter\EmailFilterGambit::class,
+                    UserFilter\GroupFilterGambit::class,
+                ],
+                PostFilterer::class => [
+                    PostFilter\AuthorFilter::class,
+                    PostFilter\DiscussionFilter::class,
+                    PostFilter\IdFilter::class,
+                    PostFilter\NumberFilter::class,
+                    PostFilter\TypeFilter::class,
+                ],
             ];
         });
 

src/Post/Filter/AuthorFilter.php

@@ -0,0 +1,45 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Post\Filter;
+
+use Flarum\Filter\FilterInterface;
+use Flarum\Filter\FilterState;
+use Flarum\User\UserRepository;
+
+class AuthorFilter implements FilterInterface
+{
+    /**
+     * @var \Flarum\User\UserRepository
+     */
+    protected $users;
+
+    /**
+     * @param \Flarum\User\UserRepository $users
+     */
+    public function __construct(UserRepository $users)
+    {
+        $this->users = $users;
+    }
+
+    public function getFilterKey(): string
+    {
+        return 'author';
+    }
+
+    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    {
+        $usernames = trim($filterValue, '"');
+        $usernames = explode(',', $usernames);
+
+        $ids = $this->users->query()->whereIn('username', $usernames)->pluck('id');
+
+        $filterState->getQuery()->whereIn('posts.user_id', $ids, 'and', $negate);
+    }
+}

src/Post/Filter/DiscussionFilter.php

@@ -0,0 +1,28 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Post\Filter;
+
+use Flarum\Filter\FilterInterface;
+use Flarum\Filter\FilterState;
+
+class DiscussionFilter implements FilterInterface
+{
+    public function getFilterKey(): string
+    {
+        return 'discussion';
+    }
+
+    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    {
+        $discussionId = trim($filterValue, '"');
+
+        $filterState->getQuery()->where('posts.discussion_id', $negate ? '!=' : '=', $discussionId);
+    }
+}

src/Post/Filter/IdFilter.php

@@ -0,0 +1,29 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Post\Filter;
+
+use Flarum\Filter\FilterInterface;
+use Flarum\Filter\FilterState;
+
+class IdFilter implements FilterInterface
+{
+    public function getFilterKey(): string
+    {
+        return 'id';
+    }
+
+    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    {
+        $idString = trim($filterValue, '"');
+        $ids = explode(',', $idString);
+
+        $filterState->getQuery()->whereIn('posts.id', $ids, 'and', $negate);
+    }
+}

src/Post/Filter/NumberFilter.php

@@ -0,0 +1,28 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Post\Filter;
+
+use Flarum\Filter\FilterInterface;
+use Flarum\Filter\FilterState;
+
+class NumberFilter implements FilterInterface
+{
+    public function getFilterKey(): string
+    {
+        return 'number';
+    }
+
+    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    {
+        $number = trim($filterValue, '"');
+
+        $filterState->getQuery()->where('posts.number', $negate ? '!=' : '=', $number);
+    }
+}

src/Post/Filter/PostFilterer.php

@@ -0,0 +1,40 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Post\Filter;
+
+use Flarum\Filter\AbstractFilterer;
+use Flarum\Post\PostRepository;
+use Flarum\User\User;
+use Illuminate\Database\Eloquent\Builder;
+
+class PostFilterer extends AbstractFilterer
+{
+    /**
+     * @var PostRepository
+     */
+    protected $posts;
+
+    /**
+     * @param PostRepository $posts
+     * @param array $filters
+     * @param array $filterMutators
+     */
+    public function __construct(PostRepository $posts, array $filters, array $filterMutators)
+    {
+        parent::__construct($filters, $filterMutators);
+
+        $this->posts = $posts;
+    }
+
+    protected function getQuery(User $actor): Builder
+    {
+        return $this->posts->query()->whereVisibleTo($actor);
+    }
+}

src/Post/Filter/TypeFilter.php

@@ -0,0 +1,28 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Post\Filter;
+
+use Flarum\Filter\FilterInterface;
+use Flarum\Filter\FilterState;
+
+class TypeFilter implements FilterInterface
+{
+    public function getFilterKey(): string
+    {
+        return 'type';
+    }
+
+    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    {
+        $type = trim($filterValue, '"');
+
+        $filterState->getQuery()->where('posts.type', $negate ? '!=' : '=', $type);
+    }
+}

src/Post/PostRepository.php

@@ -80,32 +80,6 @@ class PostRepository
         return $query->get();
     }
 
-    /**
-     * Find posts by their IDs, optionally making sure they are visible to a
-     * certain user.
-     *
-     * @param array $ids
-     * @param \Flarum\User\User|null $actor
-     * @return \Illuminate\Database\Eloquent\Collection
-     */
-    public function findByIds(array $ids, User $actor = null)
-    {
-        $posts = $this->queryIds($ids, $actor)->get();
-
-        $posts = $posts->sort(function ($a, $b) use ($ids) {
-            $aPos = array_search($a->id, $ids);
-            $bPos = array_search($b->id, $ids);
-
-            if ($aPos === $bPos) {
-                return 0;
-            }
-
-            return $aPos < $bPos ? -1 : 1;
-        });
-
-        return $posts;
-    }
-
     /**
      * Filter a list of post IDs to only include posts that are visible to a
      * certain user.

tests/integration/api/posts/ListTest.php

@@ -0,0 +1,226 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Tests\integration\api\posts;
+
+use Carbon\Carbon;
+use Flarum\Event\ConfigurePostsQuery;
+use Flarum\Tests\integration\RetrievesAuthorizedUsers;
+use Flarum\Tests\integration\TestCase;
+use Illuminate\Support\Arr;
+
+class ListTests extends TestCase
+{
+    use RetrievesAuthorizedUsers;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->prepareDatabase([
+            'discussions' => [
+                ['id' => 1, 'title' => __CLASS__, 'created_at' => Carbon::now(), 'last_posted_at' => Carbon::now(), 'user_id' => 1, 'first_post_id' => 1, 'comment_count' => 2],
+                ['id' => 2, 'title' => __CLASS__, 'created_at' => Carbon::now(), 'last_posted_at' => Carbon::now(), 'user_id' => 2, 'first_post_id' => 1, 'comment_count' => 2],
+            ],
+            'posts' => [
+                ['id' => 1, 'number' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>something</p></t>'],
+                ['id' => 2, 'number' => 1, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>something</p></t>'],
+                ['id' => 3, 'number' => 2, 'discussion_id' => 1, 'created_at' => Carbon::now(), 'user_id' => 2, 'type' => 'comment', 'content' => '<t><p>something</p></t>'],
+                ['id' => 4, 'number' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 2, 'type' => 'comment', 'content' => '<t><p>something</p></t>'],
+                ['id' => 5, 'number' => 3, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 2, 'type' => 'discussionRenamed', 'content' => '<t><p>something</p></t>'],
+            ],
+            'users' => [
+                $this->normalUser(),
+            ],
+        ]);
+    }
+
+    private function forbidGuestsFromSeeingForum()
+    {
+        $this->database()->table('group_permission')->where('permission', 'viewDiscussions')->where('group_id', 2)->delete();
+    }
+
+    /**
+     * @test
+     */
+    public function guests_cant_see_anything_if_not_allowed()
+    {
+        $this->forbidGuestsFromSeeingForum();
+
+        $response = $this->send(
+            $this->request('GET', '/api/posts')
+        );
+
+        $this->assertEquals(200, $response->getStatusCode());
+        $data = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertEquals([], $data['data']);
+    }
+
+    /**
+     * @test
+     */
+    public function authorized_users_can_see_posts()
+    {
+        $response = $this->send(
+            $this->request('GET', '/api/posts', ['authenticatedAs' => 1])
+        );
+
+        $this->assertEquals(200, $response->getStatusCode());
+        $data = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertEquals(5, count($data['data']));
+    }
+
+    /**
+     * @test
+     */
+    public function author_filter_works()
+    {
+        $response = $this->send(
+            $this->request('GET', '/api/posts', ['authenticatedAs' => 1])
+                ->withQueryParams([
+                    'filter' => ['author' => 'admin'],
+                ])
+        );
+
+        $this->assertEquals(200, $response->getStatusCode());
+        $data = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertEquals(['1', '2'], Arr::pluck($data['data'], 'id'));
+    }
+
+    /**
+     * @test
+     */
+    public function author_filter_works_with_multiple_values()
+    {
+        $response = $this->send(
+            $this->request('GET', '/api/posts', ['authenticatedAs' => 1])
+                ->withQueryParams([
+                    'filter' => ['author' => 'admin,normal'],
+                ])
+        );
+
+        $this->assertEquals(200, $response->getStatusCode());
+        $data = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertEquals(['1', '2', '3', '4', '5'], Arr::pluck($data['data'], 'id'));
+    }
+
+    /**
+     * @test
+     */
+    public function discussion_filter_works()
+    {
+        $response = $this->send(
+            $this->request('GET', '/api/posts', ['authenticatedAs' => 1])
+            ->withQueryParams([
+                'filter' => ['discussion' => '1'],
+            ])
+        );
+
+        $this->assertEquals(200, $response->getStatusCode());
+        $data = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertEquals(['1', '3'], Arr::pluck($data['data'], 'id'));
+    }
+
+    /**
+     * @test
+     */
+    public function type_filter_works()
+    {
+        $response = $this->send(
+            $this->request('GET', '/api/posts', ['authenticatedAs' => 1])
+                ->withQueryParams([
+                    'filter' => ['type' => 'discussionRenamed'],
+                ])
+        );
+
+        $this->assertEquals(200, $response->getStatusCode());
+        $data = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertEquals(['5'], Arr::pluck($data['data'], 'id'));
+    }
+
+    /**
+     * @test
+     */
+    public function number_filter_works()
+    {
+        $response = $this->send(
+            $this->request('GET', '/api/posts', ['authenticatedAs' => 1])
+                ->withQueryParams([
+                    'filter' => ['number' => '2'],
+                ])
+        );
+
+        $this->assertEquals(200, $response->getStatusCode());
+        $data = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertEquals(['3', '4'], Arr::pluck($data['data'], 'id'));
+    }
+
+    /**
+     * @test
+     */
+    public function id_filter_works()
+    {
+        $response = $this->send(
+            $this->request('GET', '/api/posts', ['authenticatedAs' => 1])
+                ->withQueryParams([
+                    'filter' => ['id' => '4'],
+                ])
+        );
+
+        $this->assertEquals(200, $response->getStatusCode());
+        $data = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertEquals(['4'], Arr::pluck($data['data'], 'id'));
+    }
+
+    /**
+     * @test
+     */
+    public function id_filter_works_with_multiple_ids()
+    {
+        $response = $this->send(
+            $this->request('GET', '/api/posts', ['authenticatedAs' => 1])
+                ->withQueryParams([
+                    'filter' => ['id' => '1,3,5'],
+                ])
+        );
+
+        $this->assertEquals(200, $response->getStatusCode());
+        $data = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertEquals(['1', '3', '5'], Arr::pluck($data['data'], 'id'));
+    }
+
+    /**
+     * @deprecated beta 16, remove beta 17
+     * @test
+     */
+    public function deprecated_configure_posts_query_extension_still_works()
+    {
+        $this->app()->getContainer()->make('events')->listen(ConfigurePostsQuery::class, function (ConfigurePostsQuery $event) {
+            $event->query->where('id', '1');
+        });
+
+        $response = $this->send(
+            $this->request('GET', '/api/posts', ['authenticatedAs' => 1])
+        );
+
+        $this->assertEquals(200, $response->getStatusCode());
+        $data = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertEquals(['1'], Arr::pluck($data['data'], 'id'));
+    }
+}