perf(likes): limit likes relationship results (#3781)

* perf(core,mentions): limit `mentionedBy` post relation results

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* Apply fixes from StyleCI

* chore: use a static property to allow customization

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* chore: use a static property to allow customization

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* chore: include count in show post endpoint

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* chore: consistent locale key format

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* chore: forgot to delete `FilterVisiblePosts`

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* test: `mentionedByCount` must not include invisible posts to actor

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* fix: visibility scoping on `mentionedByCount`

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* fix: `loadAggregates` conflicts with visibility scopers

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* Apply fixes from StyleCI

* chore: phpstan

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* perf(likes): limit `likes` relationship results

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* Apply fixes from StyleCI

* chore: simplify

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* test: `likesCount` is as expected

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* Apply fixes from StyleCI

---------

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
Co-authored-by: StyleCI Bot <bot@styleci.io>
Co-authored-by: IanM <16573496+imorland@users.noreply.github.com>
This commit is contained in:
Sami Mazouz 2023-04-19 09:22:41 +01:00 committed by GitHub
parent 6b8e9ce1db
commit d0669b08aa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 468 additions and 52 deletions

View File

@ -13,12 +13,15 @@ use Flarum\Api\Controller;
use Flarum\Api\Serializer\BasicUserSerializer;
use Flarum\Api\Serializer\PostSerializer;
use Flarum\Extend;
use Flarum\Likes\Api\LoadLikesRelationship;
use Flarum\Likes\Event\PostWasLiked;
use Flarum\Likes\Event\PostWasUnliked;
use Flarum\Likes\Notification\PostLikedBlueprint;
use Flarum\Likes\Query\LikedByFilter;
use Flarum\Likes\Query\LikedFilter;
use Flarum\Post\Filter\PostFilterer;
use Flarum\Post\Post;
use Flarum\User\Filter\UserFilterer;
use Flarum\User\User;
return [
@ -41,19 +44,32 @@ return [
->hasMany('likes', BasicUserSerializer::class)
->attribute('canLike', function (PostSerializer $serializer, $model) {
return (bool) $serializer->getActor()->can('like', $model);
})
->attribute('likesCount', function (PostSerializer $serializer, $model) {
return $model->getAttribute('likes_count') ?: 0;
}),
(new Extend\ApiController(Controller\ShowDiscussionController::class))
->addInclude('posts.likes'),
->addInclude('posts.likes')
->loadWhere('posts.likes', [LoadLikesRelationship::class, 'mutateRelation'])
->prepareDataForSerialization([LoadLikesRelationship::class, 'countRelation']),
(new Extend\ApiController(Controller\ListPostsController::class))
->addInclude('likes'),
->addInclude('likes')
->loadWhere('likes', [LoadLikesRelationship::class, 'mutateRelation'])
->prepareDataForSerialization([LoadLikesRelationship::class, 'countRelation']),
(new Extend\ApiController(Controller\ShowPostController::class))
->addInclude('likes'),
->addInclude('likes')
->loadWhere('likes', [LoadLikesRelationship::class, 'mutateRelation'])
->prepareDataForSerialization([LoadLikesRelationship::class, 'countRelation']),
(new Extend\ApiController(Controller\CreatePostController::class))
->addInclude('likes'),
->addInclude('likes')
->loadWhere('likes', [LoadLikesRelationship::class, 'mutateRelation'])
->prepareDataForSerialization([LoadLikesRelationship::class, 'countRelation']),
(new Extend\ApiController(Controller\UpdatePostController::class))
->addInclude('likes'),
->addInclude('likes')
->loadWhere('likes', [LoadLikesRelationship::class, 'mutateRelation'])
->prepareDataForSerialization([LoadLikesRelationship::class, 'countRelation']),
(new Extend\Event())
->listen(PostWasLiked::class, Listener\SendNotificationWhenPostIsLiked::class)
@ -63,6 +79,9 @@ return [
(new Extend\Filter(PostFilterer::class))
->addFilter(LikedByFilter::class),
(new Extend\Filter(UserFilterer::class))
->addFilter(LikedFilter::class),
(new Extend\Settings())
->default('flarum-likes.like_own_post', true),

View File

@ -0,0 +1,9 @@
import Post from 'flarum/common/models/Post';
import User from 'flarum/common/models/User';
declare module 'flarum/common/models/Post' {
export default interface Post {
likes(): User[];
likesCount(): number;
}
}

View File

@ -7,6 +7,7 @@ import username from 'flarum/common/helpers/username';
import icon from 'flarum/common/helpers/icon';
import PostLikesModal from './components/PostLikesModal';
import Button from '@flarum/core/src/common/components/Button';
export default function () {
extend(CommentPost.prototype, 'footerItems', function (items) {
@ -15,7 +16,7 @@ export default function () {
if (likes && likes.length) {
const limit = 4;
const overLimit = likes.length > limit;
const overLimit = post.likesCount() > limit;
// Construct a list of names of users who have liked this post. Make sure the
// current user is first in the list, and cap a maximum of 4 items.
@ -34,19 +35,24 @@ export default function () {
// others" name to the end of the list. Clicking on it will display a modal
// with a full list of names.
if (overLimit) {
const count = likes.length - names.length;
const count = post.likesCount() - names.length;
const label = app.translator.trans('flarum-likes.forum.post.others_link', { count });
names.push(
<a
href="#"
onclick={(e) => {
e.preventDefault();
app.modal.show(PostLikesModal, { post });
}}
>
{app.translator.trans('flarum-likes.forum.post.others_link', { count })}
</a>
);
if (app.forum.attribute('canSearchUsers')) {
names.push(
<Button
className="Button Button--ua-reset Button--text"
onclick={(e) => {
e.preventDefault();
app.modal.show(PostLikesModal, { post });
}}
>
{label}
</Button>
);
} else {
names.push(<span>{label}</span>);
}
}
items.add(

View File

@ -1,31 +0,0 @@
import app from 'flarum/forum/app';
import Modal from 'flarum/common/components/Modal';
import Link from 'flarum/common/components/Link';
import avatar from 'flarum/common/helpers/avatar';
import username from 'flarum/common/helpers/username';
export default class PostLikesModal extends Modal {
className() {
return 'PostLikesModal Modal--small';
}
title() {
return app.translator.trans('flarum-likes.forum.post_likes.title');
}
content() {
return (
<div className="Modal-body">
<ul className="PostLikesModal-list">
{this.attrs.post.likes().map((user) => (
<li>
<Link href={app.route.user(user)}>
{avatar(user)} {username(user)}
</Link>
</li>
))}
</ul>
</div>
);
}
}

View File

@ -0,0 +1,72 @@
import app from 'flarum/forum/app';
import Modal from 'flarum/common/components/Modal';
import Link from 'flarum/common/components/Link';
import avatar from 'flarum/common/helpers/avatar';
import username from 'flarum/common/helpers/username';
import type { IInternalModalAttrs } from 'flarum/common/components/Modal';
import type Post from 'flarum/common/models/Post';
import type Mithril from 'mithril';
import PostLikesModalState from '../states/PostLikesModalState';
import Button from '@flarum/core/src/common/components/Button';
import LoadingIndicator from '@flarum/core/src/common/components/LoadingIndicator';
export interface IPostLikesModalAttrs extends IInternalModalAttrs {
post: Post;
}
export default class PostLikesModal<CustomAttrs extends IPostLikesModalAttrs = IPostLikesModalAttrs> extends Modal<CustomAttrs, PostLikesModalState> {
oninit(vnode: Mithril.VnodeDOM<CustomAttrs, this>) {
super.oninit(vnode);
this.state = new PostLikesModalState({
filter: {
liked: this.attrs.post.id()!,
},
});
this.state.refresh();
}
className() {
return 'PostLikesModal Modal--small';
}
title() {
return app.translator.trans('flarum-likes.forum.post_likes.title');
}
content() {
return (
<>
<div className="Modal-body">
{this.state.isInitialLoading() ? (
<LoadingIndicator />
) : (
<ul className="PostLikesModal-list">
{this.state.getPages().map((page) =>
page.items.map((user) => (
<li>
<Link href={app.route.user(user)}>
{avatar(user)} {username(user)}
</Link>
</li>
))
)}
</ul>
)}
</div>
{this.state.hasNext() ? (
<div className="Modal-footer">
<div className="Form Form--centered">
<div className="Form-group">
<Button className="Button Button--block" onclick={() => this.state.loadNext()} loading={this.state.isLoadingNext()}>
{app.translator.trans('flarum-likes.forum.post_likes.load_more_button')}
</Button>
</div>
</div>
</div>
) : null}
</>
);
}
}

View File

@ -9,5 +9,6 @@ export default [
new Extend.Model(Post) //
.hasMany<User>('likes')
.attribute<number>('likesCount')
.attribute<boolean>('canLike'),
];

View File

@ -0,0 +1,26 @@
import PaginatedListState, { PaginatedListParams } from '@flarum/core/src/common/states/PaginatedListState';
import User from 'flarum/common/models/User';
export interface PostLikesModalListParams extends PaginatedListParams {
filter: {
liked: string;
};
page?: {
offset?: number;
limit: number;
};
}
export default class PostLikesModalState<P extends PostLikesModalListParams = PostLikesModalListParams> extends PaginatedListState<User, P> {
constructor(params: P, page: number = 1) {
const limit = 10;
params.page = { ...(params.page || {}), limit };
super(params, page, limit);
}
get type(): string {
return 'users';
}
}

View File

@ -35,6 +35,7 @@ flarum-likes:
# These translations are used by the Users Who Like This modal dialog.
post_likes:
title: Users Who Like This
load_more_button: => core.ref.load_more
# These translations are used in the Settings page.
settings:

View File

@ -0,0 +1,61 @@
<?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\Likes\Api;
use Flarum\Discussion\Discussion;
use Flarum\Http\RequestUtil;
use Flarum\Post\Post;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Database\Eloquent\Relations\BelongsToMany;
use Illuminate\Database\Query\Expression;
use Psr\Http\Message\ServerRequestInterface;
class LoadLikesRelationship
{
public static $maxLikes = 4;
public static function mutateRelation(BelongsToMany $query, ServerRequestInterface $request): BelongsToMany
{
$actor = RequestUtil::getActor($request);
$grammar = $query->getQuery()->getGrammar();
return $query
// So that we can tell if the current user has liked the post.
->orderBy(new Expression($grammar->wrap('user_id').' = '.$actor->id), 'desc')
// Limiting a relationship results is only possible because
// the Post model uses the \Staudenmeir\EloquentEagerLimit\HasEagerLimit
// trait.
->limit(self::$maxLikes);
}
/**
* Called using the @see ApiController::prepareDataForSerialization extender.
*/
public static function countRelation($controller, $data): void
{
$loadable = null;
if ($data instanceof Discussion) {
// @phpstan-ignore-next-line
$loadable = $data->newCollection($data->posts)->filter(function ($post) {
return $post instanceof Post;
});
} elseif ($data instanceof Collection) {
$loadable = $data;
} elseif ($data instanceof Post) {
$loadable = $data->newCollection([$data]);
}
if ($loadable) {
$loadable->loadCount('likes');
}
}
}

View File

@ -0,0 +1,34 @@
<?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\Likes\Query;
use Flarum\Filter\FilterInterface;
use Flarum\Filter\FilterState;
class LikedFilter implements FilterInterface
{
public function getFilterKey(): string
{
return 'liked';
}
public function filter(FilterState $filterState, string $filterValue, bool $negate)
{
$likedId = trim($filterValue, '"');
$filterState
->getQuery()
->whereIn('id', function ($query) use ($likedId) {
$query->select('user_id')
->from('post_likes')
->where('post_id', $likedId);
}, 'and', $negate);
}
}

View File

@ -0,0 +1,210 @@
<?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\Likes\Tests\integration\api\discussions;
use Carbon\Carbon;
use Flarum\Group\Group;
use Flarum\Likes\Api\LoadLikesRelationship;
use Flarum\Testing\integration\RetrievesAuthorizedUsers;
use Flarum\Testing\integration\TestCase;
use Illuminate\Support\Arr;
class ListPostsTest extends TestCase
{
use RetrievesAuthorizedUsers;
/**
* @inheritDoc
*/
protected function setUp(): void
{
parent::setUp();
$this->extension('flarum-likes');
$this->prepareDatabase([
'discussions' => [
['id' => 100, 'title' => __CLASS__, 'created_at' => Carbon::now(), 'user_id' => 1, 'first_post_id' => 101, 'comment_count' => 1],
],
'posts' => [
['id' => 101, 'discussion_id' => 100, 'created_at' => Carbon::now(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>text</p></t>'],
],
'users' => [
$this->normalUser(),
['id' => 102, 'username' => 'user102', 'email' => '102@machine.local', 'is_email_confirmed' => 1],
['id' => 103, 'username' => 'user103', 'email' => '103@machine.local', 'is_email_confirmed' => 1],
['id' => 104, 'username' => 'user104', 'email' => '104@machine.local', 'is_email_confirmed' => 1],
['id' => 105, 'username' => 'user105', 'email' => '105@machine.local', 'is_email_confirmed' => 1],
['id' => 106, 'username' => 'user106', 'email' => '106@machine.local', 'is_email_confirmed' => 1],
['id' => 107, 'username' => 'user107', 'email' => '107@machine.local', 'is_email_confirmed' => 1],
['id' => 108, 'username' => 'user108', 'email' => '108@machine.local', 'is_email_confirmed' => 1],
['id' => 109, 'username' => 'user109', 'email' => '109@machine.local', 'is_email_confirmed' => 1],
['id' => 110, 'username' => 'user110', 'email' => '110@machine.local', 'is_email_confirmed' => 1],
['id' => 111, 'username' => 'user111', 'email' => '111@machine.local', 'is_email_confirmed' => 1],
['id' => 112, 'username' => 'user112', 'email' => '112@machine.local', 'is_email_confirmed' => 1],
],
'post_likes' => [
['user_id' => 102, 'post_id' => 101],
['user_id' => 104, 'post_id' => 101],
['user_id' => 105, 'post_id' => 101],
['user_id' => 106, 'post_id' => 101],
['user_id' => 107, 'post_id' => 101],
['user_id' => 108, 'post_id' => 101],
['user_id' => 109, 'post_id' => 101],
['user_id' => 110, 'post_id' => 101],
['user_id' => 2, 'post_id' => 101],
['user_id' => 111, 'post_id' => 101],
['user_id' => 112, 'post_id' => 101],
],
'group_permission' => [
['group_id' => Group::GUEST_ID, 'permission' => 'searchUsers'],
],
]);
}
/**
* @test
*/
public function liked_filter_works()
{
$response = $this->send(
$this->request('GET', '/api/users')
->withQueryParams([
'filter' => ['liked' => 101],
])
);
$this->assertEquals(200, $response->getStatusCode());
$data = json_decode($response->getBody()->getContents(), true)['data'];
// Order-independent comparison
$ids = Arr::pluck($data, 'id');
$this->assertEqualsCanonicalizing([
102, 104, 105, 106, 107, 108, 109, 110, 2, 111, 112
], $ids, 'IDs do not match');
}
/**
* @test
*/
public function liked_filter_works_negated()
{
$response = $this->send(
$this->request('GET', '/api/users')
->withQueryParams([
'filter' => ['-liked' => 101],
])
);
$this->assertEquals(200, $response->getStatusCode());
$data = json_decode($response->getBody()->getContents(), true)['data'];
// Order-independent comparison
$ids = Arr::pluck($data, 'id');
$this->assertEqualsCanonicalizing([1, 103], $ids, 'IDs do not match');
}
/** @test */
public function likes_relation_returns_limited_results_and_shows_only_visible_posts_in_show_post_endpoint()
{
// List posts endpoint
$response = $this->send(
$this->request('GET', '/api/posts/101', [
'authenticatedAs' => 2,
])->withQueryParams([
'include' => 'likes',
])
);
$data = json_decode($response->getBody()->getContents(), true)['data'];
$this->assertEquals(200, $response->getStatusCode());
$likes = $data['relationships']['likes']['data'];
// Only displays a limited amount of likes
$this->assertCount(LoadLikesRelationship::$maxLikes, $likes);
// Displays the correct count of likes
$this->assertEquals(11, $data['attributes']['likesCount']);
// Of the limited amount of likes, the actor always appears
$this->assertEquals([2, 102, 104, 105], Arr::pluck($likes, 'id'));
}
/** @test */
public function likes_relation_returns_limited_results_and_shows_only_visible_posts_in_list_posts_endpoint()
{
// List posts endpoint
$response = $this->send(
$this->request('GET', '/api/posts', [
'authenticatedAs' => 2,
])->withQueryParams([
'filter' => ['discussion' => 100],
'include' => 'likes',
])
);
$data = json_decode($response->getBody()->getContents(), true)['data'];
$this->assertEquals(200, $response->getStatusCode());
$likes = $data[0]['relationships']['likes']['data'];
// Only displays a limited amount of likes
$this->assertCount(LoadLikesRelationship::$maxLikes, $likes);
// Displays the correct count of likes
$this->assertEquals(11, $data[0]['attributes']['likesCount']);
// Of the limited amount of likes, the actor always appears
$this->assertEquals([2, 102, 104, 105], Arr::pluck($likes, 'id'));
}
/**
* @dataProvider likesIncludeProvider
* @test
*/
public function likes_relation_returns_limited_results_and_shows_only_visible_posts_in_show_discussion_endpoint(string $include)
{
// Show discussion endpoint
$response = $this->send(
$this->request('GET', '/api/discussions/100', [
'authenticatedAs' => 2,
])->withQueryParams([
'include' => $include,
])
);
$included = json_decode($response->getBody()->getContents(), true)['included'];
$likes = collect($included)
->where('type', 'posts')
->where('id', 101)
->first()['relationships']['likes']['data'];
// Only displays a limited amount of likes
$this->assertCount(LoadLikesRelationship::$maxLikes, $likes);
// Displays the correct count of likes
$this->assertEquals(11, collect($included)
->where('type', 'posts')
->where('id', 101)
->first()['attributes']['likesCount']);
// Of the limited amount of likes, the actor always appears
$this->assertEquals([2, 102, 104, 105], Arr::pluck($likes, 'id'));
}
public function likesIncludeProvider(): array
{
return [
['posts,posts.likes'],
['posts.likes'],
[''],
];
}
}

View File

@ -1,7 +1,7 @@
import PaginatedListState, { PaginatedListParams } from 'flarum/common/states/PaginatedListState';
import Post from 'flarum/common/models/Post';
export interface MentionedByModalParams extends PaginatedListParams {
export interface MentionedByModalListParams extends PaginatedListParams {
filter: {
mentionedPost: string;
};
@ -12,7 +12,7 @@ export interface MentionedByModalParams extends PaginatedListParams {
};
}
export default class MentionedByModalState<P extends MentionedByModalParams = MentionedByModalParams> extends PaginatedListState<Post, P> {
export default class MentionedByModalState<P extends MentionedByModalListParams = MentionedByModalListParams> extends PaginatedListState<Post, P> {
constructor(params: P, page: number = 1) {
const limit = 10;

View File

@ -74,6 +74,8 @@ class CreatePostController extends AbstractCreateController
$discussion = $post->discussion;
$discussion->posts = $discussion->posts()->whereVisibleTo($actor)->orderBy('created_at')->pluck('id');
$this->loadRelations($post->newCollection([$post]), $this->extractInclude($request), $request);
return $post;
}
}

View File

@ -54,8 +54,12 @@ class UpdatePostController extends AbstractShowController
$actor = RequestUtil::getActor($request);
$data = Arr::get($request->getParsedBody(), 'data', []);
return $this->bus->dispatch(
$post = $this->bus->dispatch(
new EditPost($id, $actor, $data)
);
$this->loadRelations($post->newCollection([$post]), $this->extractInclude($request), $request);
return $post;
}
}

View File

@ -34,6 +34,7 @@ use Flarum\User\Exception\PermissionDeniedException;
use Illuminate\Contracts\Filesystem\Factory;
use Illuminate\Contracts\Hashing\Hasher;
use Illuminate\Support\Arr;
use Staudenmeir\EloquentEagerLimit\HasEagerLimit;
/**
* @property int $id
@ -55,6 +56,7 @@ class User extends AbstractModel
{
use EventGeneratorTrait;
use ScopeVisibilityTrait;
use HasEagerLimit;
/**
* The attributes that should be mutated to dates.