From 6ccf4f75eae90837bae7e2179e26c8e8c843dab5 Mon Sep 17 00:00:00 2001 From: Dries Vints Date: Tue, 9 Mar 2021 23:32:46 +0100 Subject: [PATCH 1/4] Eager load models --- app/Helpers/HasLikes.php | 29 ++++++++++++------- app/Helpers/HasTags.php | 5 ---- app/Helpers/ReceivesReplies.php | 5 ---- app/Models/Article.php | 18 ++++++++++-- app/Models/Reply.php | 2 +- app/Models/Thread.php | 9 ++++++ app/Models/User.php | 28 +++++++++--------- resources/views/forum/_thread.blade.php | 8 ++--- .../views/livewire/like-article.blade.php | 4 +-- resources/views/livewire/like-reply.blade.php | 4 +-- .../views/livewire/like-thread.blade.php | 4 +-- .../views/livewire/show-articles.blade.php | 2 +- .../views/users/_latest_replies.blade.php | 2 +- .../views/users/_latest_threads.blade.php | 2 +- resources/views/users/_metrics.blade.php | 8 ++--- resources/views/users/articles.blade.php | 2 +- tests/Integration/Models/ArticleTest.php | 2 +- 17 files changed, 78 insertions(+), 56 deletions(-) diff --git a/app/Helpers/HasLikes.php b/app/Helpers/HasLikes.php index 04154653a..bd6fae339 100644 --- a/app/Helpers/HasLikes.php +++ b/app/Helpers/HasLikes.php @@ -8,16 +8,24 @@ trait HasLikes { + /** + * @return \Illuminate\Database\Eloquent\Collection + */ + public function likes() + { + return $this->likesRelation; + } + protected static function bootHasLikes() { static::deleting(function ($model) { - $model->likes()->delete(); + $model->likesRelation()->delete(); }); } public function likedBy(User $user) { - $this->likes()->create(['user_id' => $user->id()]); + $this->likesRelation()->create(['user_id' => $user->id()]); } public function dislikedBy(User $user) @@ -25,18 +33,19 @@ public function dislikedBy(User $user) optional($this->likes()->where('user_id', $user->id())->first())->delete(); } - public function likes(): MorphMany + /** + * It's important to name the relationship the same as the method because otherwise + * eager loading of the polymorphic relationship will fail on queued jobs. + * + * @see https://github.com/laravelio/laravel.io/issues/350 + */ + public function likesRelation(): MorphMany { - return $this->morphMany(Like::class, 'likeable'); + return $this->morphMany(Like::class, 'likesRelation', 'likeable_type', 'likeable_id'); } public function isLikedBy(User $user): bool { - return $this->likes()->where('user_id', $user->id())->exists(); - } - - public function likesCount(): int - { - return $this->likes()->count(); + return $this->likes()->where('user_id', $user->id())->isNotEmpty(); } } diff --git a/app/Helpers/HasTags.php b/app/Helpers/HasTags.php index 9c3ba2d8a..5f7262ba5 100644 --- a/app/Helpers/HasTags.php +++ b/app/Helpers/HasTags.php @@ -33,9 +33,4 @@ public function tagsRelation(): MorphToMany { return $this->morphToMany(Tag::class, 'taggable')->withTimestamps(); } - - public function hasTags(): bool - { - return $this->tagsRelation()->count() > 0; - } } diff --git a/app/Helpers/ReceivesReplies.php b/app/Helpers/ReceivesReplies.php index dcf7b4213..a95aff656 100644 --- a/app/Helpers/ReceivesReplies.php +++ b/app/Helpers/ReceivesReplies.php @@ -53,9 +53,4 @@ public function isConversationOld(): bool return $this->createdAt()->lt($sixMonthsAgo); } - - public function repliesCount(): int - { - return $this->repliesRelation()->count(); - } } diff --git a/app/Models/Article.php b/app/Models/Article.php index d06649eed..023bf6532 100644 --- a/app/Models/Article.php +++ b/app/Models/Article.php @@ -50,6 +50,20 @@ final class Article extends Model 'shared_at', ]; + /** + * {@inheritdoc} + */ + protected $with = [ + 'likesRelation', + ]; + + /** + * {@inheritdoc} + */ + protected $withCount = [ + 'likesRelation', + ]; + public function id(): int { return $this->id; @@ -242,14 +256,14 @@ public function scopeRecent(Builder $query): Builder public function scopePopular(Builder $query): Builder { - return $query->withCount('likes') + return $query->withCount('likesRelation') ->orderBy('likes_count', 'desc') ->orderBy('submitted_at', 'desc'); } public function scopeTrending(Builder $query): Builder { - return $query->withCount(['likes' => function ($query) { + return $query->withCount(['likesRelation' => function ($query) { $query->where('created_at', '>=', now()->subWeek()); }]) ->orderBy('likes_count', 'desc') diff --git a/app/Models/Reply.php b/app/Models/Reply.php index df143dbb4..a447175c7 100644 --- a/app/Models/Reply.php +++ b/app/Models/Reply.php @@ -37,7 +37,7 @@ final class Reply extends Model * {@inheritdoc} */ protected $with = [ - 'likes', + 'likesRelation', ]; public function id(): int diff --git a/app/Models/Thread.php b/app/Models/Thread.php index 67bacfcaa..604f48add 100644 --- a/app/Models/Thread.php +++ b/app/Models/Thread.php @@ -59,6 +59,14 @@ final class Thread extends Model implements ReplyAble, SubscriptionAble, Feedabl 'subject', ]; + /** + * {@inheritdoc} + */ + protected $with = [ + 'likesRelation', + 'repliesRelation', + ]; + public function id(): int { return $this->id; @@ -184,6 +192,7 @@ public static function feedQuery(): Builder { return static::with([ 'solutionReplyRelation', + 'likesRelation', 'repliesRelation', 'repliesRelation.authorRelation', 'tagsRelation', diff --git a/app/Models/User.php b/app/Models/User.php index bfcb155fb..1820d0b9b 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -214,20 +214,6 @@ public function series(): HasMany return $this->hasMany(Series::class, 'author_id'); } - /** - * @todo Make this work with Eloquent instead of a collection - */ - public function countSolutions(): int - { - return $this->replies()->filter(function (Reply $reply) { - if ($reply->replyAble() instanceof Thread) { - return $reply->replyAble()->isSolutionReply($reply); - } - - return false; - })->count(); - } - public static function findByUsername(string $username): self { return static::where('username', $username)->firstOrFail(); @@ -251,6 +237,20 @@ public function delete() parent::delete(); } + /** + * @todo Make this work with Eloquent instead of a collection + */ + public function countSolutions(): int + { + return $this->replies()->filter(function (Reply $reply) { + if ($reply->replyAble() instanceof Thread) { + return $reply->replyAble()->isSolutionReply($reply); + } + + return false; + })->count(); + } + public function scopeMostSolutions(Builder $query) { return $query->withCount(['replyAble as most_solutions' => function ($query) { diff --git a/resources/views/forum/_thread.blade.php b/resources/views/forum/_thread.blade.php index 78289e535..a10ee6dc4 100644 --- a/resources/views/forum/_thread.blade.php +++ b/resources/views/forum/_thread.blade.php @@ -8,9 +8,9 @@
- @if ($thread->hasTags()) + @if (count($tags = $thread->tags()))
- @foreach ($thread->tags() as $tag) + @foreach ($tags as $tag) {{ $tag->name() }} @@ -68,7 +68,7 @@ class="rounded-full p-1 bg-lio-100 text-lio-500"
- {{ $thread->likesCount() }} + {{ count($thread->likes()) }} Likes
@@ -76,7 +76,7 @@ class="rounded-full p-1 bg-lio-100 text-lio-500"
- {{ $thread->repliesCount() }} + {{ count($thread->replies()) }} Replies
diff --git a/resources/views/livewire/like-article.blade.php b/resources/views/livewire/like-article.blade.php index 81608be95..097b07572 100644 --- a/resources/views/livewire/like-article.blade.php +++ b/resources/views/livewire/like-article.blade.php @@ -2,12 +2,12 @@ @guest
- {{ $this->article->likesCount() }} + {{ count($this->article->likes()) }}
@else - {{ $this->article->likesCount() }} + {{ count($this->article->likes()) }} @endGuest
diff --git a/resources/views/livewire/like-reply.blade.php b/resources/views/livewire/like-reply.blade.php index 32dc33ca1..91759da46 100644 --- a/resources/views/livewire/like-reply.blade.php +++ b/resources/views/livewire/like-reply.blade.php @@ -2,12 +2,12 @@ @if (Auth::guest())
👍 - {{ $this->reply->likesCount() }} + {{ count($this->reply->likes()) }}
@else @endif
diff --git a/resources/views/livewire/like-thread.blade.php b/resources/views/livewire/like-thread.blade.php index 19a6e7e8d..69158f4fe 100644 --- a/resources/views/livewire/like-thread.blade.php +++ b/resources/views/livewire/like-thread.blade.php @@ -2,12 +2,12 @@ @if (Auth::guest())
👍 - {{ $this->thread->likesCount() }} + {{ count($this->thread->likes()) }}
@else @endif diff --git a/resources/views/livewire/show-articles.blade.php b/resources/views/livewire/show-articles.blade.php index 8df08897e..2f47c7e2c 100644 --- a/resources/views/livewire/show-articles.blade.php +++ b/resources/views/livewire/show-articles.blade.php @@ -58,7 +58,7 @@
👏 - {{ $article->likesCount() }} + {{ count($article->likes()) }}
diff --git a/resources/views/users/_latest_replies.blade.php b/resources/views/users/_latest_replies.blade.php index 22e87fe25..34cb48bf4 100644 --- a/resources/views/users/_latest_replies.blade.php +++ b/resources/views/users/_latest_replies.blade.php @@ -16,7 +16,7 @@
- {{ $reply->likesCount() }} + {{ count($reply->likes()) }}
diff --git a/resources/views/users/_latest_threads.blade.php b/resources/views/users/_latest_threads.blade.php index b2e86d0ee..b2e8a718b 100644 --- a/resources/views/users/_latest_threads.blade.php +++ b/resources/views/users/_latest_threads.blade.php @@ -21,7 +21,7 @@ - {{ $thread->likesCount() }} + {{ count($thread->likes()) }} diff --git a/resources/views/users/_metrics.blade.php b/resources/views/users/_metrics.blade.php index c5f066f64..b12eb13fe 100644 --- a/resources/views/users/_metrics.blade.php +++ b/resources/views/users/_metrics.blade.php @@ -1,19 +1,19 @@
-

{{ $user->countThreads() }} {{ Str::plural('thread', $user->countThreads()) }}

+

{{ $countedThreads = $user->countThreads() }} {{ Str::plural('thread', $countedThreads) }}

-

{{ $user->countReplies() }} {{ Str::plural('reply', $user->countReplies()) }}

+

{{ $countedReplies = $user->countReplies() }} {{ Str::plural('reply', $countedReplies) }}

-

{{ $user->countSolutions() }} {{ Str::plural('solution', $user->countSolutions()) }}

+

{{ $countedSolutions = $user->countSolutions() }} {{ Str::plural('solution', $countedSolutions) }}

-

{{ $user->countArticles() }} {{ Str::plural('article', $user->countArticles()) }}

+

{{ $countedArticles = $user->countArticles() }} {{ Str::plural('article', $countedArticles) }}

diff --git a/resources/views/users/articles.blade.php b/resources/views/users/articles.blade.php index 08f8ce58e..2b9c26bb0 100644 --- a/resources/views/users/articles.blade.php +++ b/resources/views/users/articles.blade.php @@ -106,7 +106,7 @@
👏 - {{ $article->likesCount() }} + {{ count($article->likes()) }}
diff --git a/tests/Integration/Models/ArticleTest.php b/tests/Integration/Models/ArticleTest.php index 5a715718d..f3eb2e620 100644 --- a/tests/Integration/Models/ArticleTest.php +++ b/tests/Integration/Models/ArticleTest.php @@ -43,7 +43,7 @@ public function we_can_get_trending_articles() $articles[0]->likedBy($users[1]); // Update the like timestamp outside of the trending window. - $articles[0]->likes()->update(['created_at' => now()->subWeeks(2)]); + $articles[0]->likesRelation()->update(['created_at' => now()->subWeeks(2)]); // Like the remaining articles once, but inside the trending window. $articles[1]->likedBy($users[0]); From bdd1116601f3e5e2d37845891e579cecd5392483 Mon Sep 17 00:00:00 2001 From: Dries Vints Date: Wed, 10 Mar 2021 17:20:45 +0100 Subject: [PATCH 2/4] More DB performance improvments --- app/Models/Article.php | 9 ++------- app/Models/Thread.php | 1 + ...1_03_10_161050_add_index_to_replyable_type.php | 15 +++++++++++++++ database/schema/mysql-schema.dump | 2 ++ resources/views/forum/_thread.blade.php | 1 + resources/views/livewire/show-articles.blade.php | 6 ++++-- 6 files changed, 25 insertions(+), 9 deletions(-) create mode 100644 database/migrations/2021_03_10_161050_add_index_to_replyable_type.php diff --git a/app/Models/Article.php b/app/Models/Article.php index 023bf6532..ba04be327 100644 --- a/app/Models/Article.php +++ b/app/Models/Article.php @@ -54,14 +54,9 @@ final class Article extends Model * {@inheritdoc} */ protected $with = [ + 'authorRelation', 'likesRelation', - ]; - - /** - * {@inheritdoc} - */ - protected $withCount = [ - 'likesRelation', + 'tagsRelation', ]; public function id(): int diff --git a/app/Models/Thread.php b/app/Models/Thread.php index 604f48add..ffa8bf96a 100644 --- a/app/Models/Thread.php +++ b/app/Models/Thread.php @@ -65,6 +65,7 @@ final class Thread extends Model implements ReplyAble, SubscriptionAble, Feedabl protected $with = [ 'likesRelation', 'repliesRelation', + 'tagsRelation', ]; public function id(): int diff --git a/database/migrations/2021_03_10_161050_add_index_to_replyable_type.php b/database/migrations/2021_03_10_161050_add_index_to_replyable_type.php new file mode 100644 index 000000000..51be699dc --- /dev/null +++ b/database/migrations/2021_03_10_161050_add_index_to_replyable_type.php @@ -0,0 +1,15 @@ +index('replyable_type'); + }); + } +} diff --git a/database/schema/mysql-schema.dump b/database/schema/mysql-schema.dump index 5caa97abd..643058465 100644 --- a/database/schema/mysql-schema.dump +++ b/database/schema/mysql-schema.dump @@ -145,6 +145,7 @@ CREATE TABLE `replies` ( PRIMARY KEY (`id`), KEY `replies_author_id_index` (`author_id`), KEY `replies_replyable_id_index` (`replyable_id`), + KEY `replies_replyable_type_index` (`replyable_type`), CONSTRAINT `replies_author_id_foreign` FOREIGN KEY (`author_id`) REFERENCES `users` (`id`) ON DELETE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; /*!40101 SET character_set_client = @saved_cs_client */; @@ -323,3 +324,4 @@ INSERT INTO `migrations` VALUES (55,'2020_07_16_185353_add_twitter_columns',1); INSERT INTO `migrations` VALUES (56,'2020_10_01_093001_add_email_verified_at_column_to_users',1); INSERT INTO `migrations` VALUES (57,'2020_11_03_205735_add_uuid_to_failed_jobs_table',1); INSERT INTO `migrations` VALUES (58,'2020_11_22_194212_create_schedule_monitor_tables',1); +INSERT INTO `migrations` VALUES (59,'2021_03_10_161050_add_index_to_replyable_type',2); diff --git a/resources/views/forum/_thread.blade.php b/resources/views/forum/_thread.blade.php index a10ee6dc4..e254dd122 100644 --- a/resources/views/forum/_thread.blade.php +++ b/resources/views/forum/_thread.blade.php @@ -35,6 +35,7 @@ +

diff --git a/resources/views/livewire/show-articles.blade.php b/resources/views/livewire/show-articles.blade.php index 2f47c7e2c..b4fca23aa 100644 --- a/resources/views/livewire/show-articles.blade.php +++ b/resources/views/livewire/show-articles.blade.php @@ -4,7 +4,7 @@ Loading...

- @foreach($articles as $article) + @foreach ($articles as $article)
@foreach ($article->tags() as $tag) @@ -56,6 +56,7 @@
+
👏 {{ count($article->likes()) }} @@ -65,7 +66,6 @@ @endforeach {{ $articles->links() }} -
@@ -73,9 +73,11 @@ + + From c03f5746dbe434bc295ae24b25918fc6c932b786 Mon Sep 17 00:00:00 2001 From: Dries Vints Date: Wed, 10 Mar 2021 18:12:49 +0100 Subject: [PATCH 3/4] Work on eager loading --- app/Helpers/HasAuthor.php | 2 + app/Helpers/HasLikes.php | 10 ++- app/Helpers/HasTags.php | 4 + app/Helpers/ReceivesReplies.php | 2 + app/Models/Thread.php | 1 + resources/views/forum/threads/show.blade.php | 78 ++++++++++---------- tests/Feature/ForumTest.php | 6 +- tests/Integration/Models/ArticleTest.php | 1 + 8 files changed, 60 insertions(+), 44 deletions(-) diff --git a/app/Helpers/HasAuthor.php b/app/Helpers/HasAuthor.php index c5b0bbab6..ef88c8b95 100644 --- a/app/Helpers/HasAuthor.php +++ b/app/Helpers/HasAuthor.php @@ -15,6 +15,8 @@ public function author(): User public function authoredBy(User $author) { $this->authorRelation()->associate($author); + + $this->unsetRelation('authorRelation'); } public function authorRelation(): BelongsTo diff --git a/app/Helpers/HasLikes.php b/app/Helpers/HasLikes.php index bd6fae339..1ac544b85 100644 --- a/app/Helpers/HasLikes.php +++ b/app/Helpers/HasLikes.php @@ -20,17 +20,23 @@ protected static function bootHasLikes() { static::deleting(function ($model) { $model->likesRelation()->delete(); + + $model->unsetRelation('likesRelation'); }); } public function likedBy(User $user) { $this->likesRelation()->create(['user_id' => $user->id()]); + + $this->unsetRelation('likesRelation'); } public function dislikedBy(User $user) { - optional($this->likes()->where('user_id', $user->id())->first())->delete(); + optional($this->likesRelation()->where('user_id', $user->id())->first())->delete(); + + $this->unsetRelation('likesRelation'); } /** @@ -46,6 +52,6 @@ public function likesRelation(): MorphMany public function isLikedBy(User $user): bool { - return $this->likes()->where('user_id', $user->id())->isNotEmpty(); + return $this->likesRelation()->where('user_id', $user->id())->exists(); } } diff --git a/app/Helpers/HasTags.php b/app/Helpers/HasTags.php index 5f7262ba5..fb2455384 100644 --- a/app/Helpers/HasTags.php +++ b/app/Helpers/HasTags.php @@ -22,11 +22,15 @@ public function syncTags(array $tags) { $this->save(); $this->tagsRelation()->sync($tags); + + $this->unsetRelation('tagsRelation'); } public function removeTags() { $this->tagsRelation()->detach(); + + $this->unsetRelation('tagsRelation'); } public function tagsRelation(): MorphToMany diff --git a/app/Helpers/ReceivesReplies.php b/app/Helpers/ReceivesReplies.php index a95aff656..c69e708c7 100644 --- a/app/Helpers/ReceivesReplies.php +++ b/app/Helpers/ReceivesReplies.php @@ -30,6 +30,8 @@ public function deleteReplies() foreach ($this->repliesRelation()->get() as $reply) { $reply->delete(); } + + $this->unsetRelation('repliesRelation'); } /** diff --git a/app/Models/Thread.php b/app/Models/Thread.php index ffa8bf96a..9796fa2ce 100644 --- a/app/Models/Thread.php +++ b/app/Models/Thread.php @@ -63,6 +63,7 @@ final class Thread extends Model implements ReplyAble, SubscriptionAble, Feedabl * {@inheritdoc} */ protected $with = [ + 'authorRelation', 'likesRelation', 'repliesRelation', 'tagsRelation', diff --git a/resources/views/forum/threads/show.blade.php b/resources/views/forum/threads/show.blade.php index c8e23ce10..230bd7394 100644 --- a/resources/views/forum/threads/show.blade.php +++ b/resources/views/forum/threads/show.blade.php @@ -11,6 +11,7 @@
+
@@ -32,10 +33,12 @@ class="text-lio-700 mr-2">
@include('forum.threads.info.tags')
+ @include('forum.threads._view_solution')
+
+
@@ -52,7 +56,6 @@ class="forum-content" @foreach ($thread->replies() as $reply)
- @if ($thread->isSolutionReply($reply))
Solution @@ -68,23 +71,22 @@ class="forum-content"
{{ $reply->author()->name() }} - replied - {{ $reply->createdAt()->diffForHumans() }} + replied {{ $reply->createdAt()->diffForHumans() }}
-
+
@can(App\Policies\ReplyPolicy::UPDATE, $reply) Edit + Delete @endcan @can(App\Policies\ThreadPolicy::UPDATE, $thread) - @if ($thread->isSolutionReply($reply)) Unmark As Solution @@ -108,11 +110,11 @@ class="forum-content" 'body' => '

Confirm to mark this reply as the solution for "'.e($thread->subject()).'".

', ]) @endif - @endcan
+
+
@@ -143,16 +146,13 @@ class="forum-content" @else
-
@csrf @formGroup('body') - @include('_partials._editor', [ - 'content' => old('body') - ]) + @include('_partials._editor', ['content' => old('body')]) @error('body') @endFormGroup @@ -168,7 +168,6 @@ class="forum-content"
- @endif @else @@ -187,38 +186,39 @@ class="forum-content" @endif @endcan +