From e0f68b3fa47d78cc980d00053c863f22f8b39afe Mon Sep 17 00:00:00 2001 From: Tofandel Date: Mon, 10 Mar 2025 14:28:42 +0100 Subject: [PATCH 1/2] fix: optimize queries in removing and adding tags --- src/TaggableTrait.php | 40 +++++++++++++++------------ tests/FunctionalTestCase.php | 11 ++++++++ tests/IlluminateTagTest.php | 13 +++++---- tests/TaggableTraitTest.php | 53 ++++++++++++++++++++---------------- 4 files changed, 70 insertions(+), 47 deletions(-) diff --git a/src/TaggableTrait.php b/src/TaggableTrait.php index 0b2046c..aca8ce0 100644 --- a/src/TaggableTrait.php +++ b/src/TaggableTrait.php @@ -214,24 +214,23 @@ public function setTags($tags, string $type = 'name'): bool */ public function addTag(string $name): void { - $tag = $this->createTagsModel()->firstOrNew([ + $tag = $this->createTagsModel()->firstOrCreate([ 'slug' => $this->generateTagSlug($name), 'namespace' => $this->getEntityClassName(), + ], [ + 'count' => 1, + 'name' => $name, ]); - if (! $tag->exists) { - $tag->name = $name; - - $tag->save(); - } - - if (! $this->tags()->get()->contains($tag->id)) { - $tag->update(['count' => $tag->count + 1]); - + if ($tag->wasRecentlyCreated || !$this->tags()->whereKey($tag)->exists()) { + if (!$tag->wasRecentlyCreated) { + $tag->update(['count' => $tag->count + 1]); + } $this->tags()->attach($tag); + if ($this->relationLoaded('tags')) { + $this->tags->add($tag); + } } - - $this->load('tags'); } /** @@ -243,8 +242,7 @@ public function removeTag(string $name): void $namespace = $this->getEntityClassName(); - $tag = $this - ->createTagsModel() + $tag = $this->tags() ->whereNamespace($namespace) ->where(function ($query) use ($name, $slug) { $query @@ -255,13 +253,19 @@ public function removeTag(string $name): void ->first() ; - if ($tag && $this->tags()->get()->contains($tag->id)) { + if ($tag) { $tag->update(['count' => $tag->count - 1]); - $this->tags()->detach($tag); - } - $this->load('tags'); + if ($this->relationLoaded('tags')) { + foreach($this->tags as $key => $loadedTag) { + if ($loadedTag->getKey() === $tag->getKey()) { + unset($this->tags[$key]); + break; + } + } + } + } } /** diff --git a/tests/FunctionalTestCase.php b/tests/FunctionalTestCase.php index 8326eba..864e12e 100644 --- a/tests/FunctionalTestCase.php +++ b/tests/FunctionalTestCase.php @@ -21,6 +21,7 @@ namespace Cartalyst\Tags\Tests; use Cartalyst\Tags\Tests\Stubs\Post; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Schema; class FunctionalTestCase extends \Orchestra\Testbench\TestCase @@ -77,4 +78,14 @@ protected function createPost() { return Post::create(['title' => 'My Test Post']); } + + protected function withQueryCount($cb): int + { + DB::enableQueryLog(); + call_user_func($cb); + $log = DB::getQueryLog(); + DB::flushQueryLog(); + DB::disableQueryLog(); + return count($log); + } } diff --git a/tests/IlluminateTagTest.php b/tests/IlluminateTagTest.php index d7922df..b3385e1 100644 --- a/tests/IlluminateTagTest.php +++ b/tests/IlluminateTagTest.php @@ -24,10 +24,11 @@ use Cartalyst\Tags\IlluminateTagged; use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\MorphTo; +use PHPUnit\Framework\Attributes\Test; class IlluminateTagTest extends FunctionalTestCase { - /** @test */ + #[Test] public function it_can_delete_a_tag_and_its_tagged_relations() { $post = $this->createPost(); @@ -45,7 +46,7 @@ public function it_can_delete_a_tag_and_its_tagged_relations() $this->assertCount(1, $post->tags); } - /** @test */ + #[Test] public function it_has_a_taggable_relationship() { $tag = new IlluminateTag(); @@ -53,7 +54,7 @@ public function it_has_a_taggable_relationship() $this->assertInstanceOf(MorphTo::class, $tag->taggable()); } - /** @test */ + #[Test] public function it_has_a_tag_relationship() { $tag = new IlluminateTag(); @@ -61,7 +62,7 @@ public function it_has_a_tag_relationship() $this->assertInstanceOf(HasMany::class, $tag->tagged()); } - /** @test */ + #[Test] public function it_has_a_name_scope() { IlluminateTag::create(['name' => 'Foo', 'slug' => 'foo', 'namespace' => 'foo']); @@ -69,7 +70,7 @@ public function it_has_a_name_scope() $this->assertCount(1, IlluminateTag::name('Foo')->get()); } - /** @test */ + #[Test] public function it_has_a_slug_scope() { IlluminateTag::create(['name' => 'Foo', 'slug' => 'foo', 'namespace' => 'foo']); @@ -77,7 +78,7 @@ public function it_has_a_slug_scope() $this->assertCount(1, IlluminateTag::slug('foo')->get()); } - /** @test */ + #[Test] public function it_can_get_and_set_the_tagged_model() { $tag = new IlluminateTag(); diff --git a/tests/TaggableTraitTest.php b/tests/TaggableTraitTest.php index 5c2d026..c361233 100644 --- a/tests/TaggableTraitTest.php +++ b/tests/TaggableTraitTest.php @@ -23,10 +23,12 @@ use Cartalyst\Tags\IlluminateTag; use Cartalyst\Tags\Tests\Stubs\Post; use Cartalyst\Tags\Tests\Stubs\Post2; +use Illuminate\Support\Facades\DB; +use PHPUnit\Framework\Attributes\Test; class TaggableTraitTest extends FunctionalTestCase { - /** @test */ + #[Test] public function it_can_add_a_single_tag() { $post1 = $this->createPost(); @@ -39,23 +41,26 @@ public function it_can_add_a_single_tag() $this->assertSame(['foo'], $post2->tags->pluck('slug')->toArray()); } - /** @test */ + #[Test] public function it_can_add_multiple_tags() { - $post1 = $this->createPost(); - $post2 = $this->createPost(); - $post3 = $this->createPost(); - - $post1->tag('foo, bar'); - $post2->tag(['foo', 'bar']); - $post3->tag(null); - + $post1 = $this->createPost(); + $post2 = $this->createPost(); + $post3 = $this->createPost(); + + $count = $this->withQueryCount(function () use ($post1, $post2, $post3) { + $post1->tag('foo, bar'); + $post2->tag(['foo', 'bar']); + $post3->tag(null); + }); + $this->assertLessThanOrEqual(14, $count); // Previously 22 queries $this->assertSame(['foo', 'bar'], $post1->tags->pluck('slug')->toArray()); $this->assertSame(['foo', 'bar'], $post2->tags->pluck('slug')->toArray()); $this->assertEmpty($post3->tags->pluck('slug')->toArray()); + } - /** @test */ + #[Test] public function it_can_untag() { $post = $this->createPost(); @@ -70,21 +75,23 @@ public function it_can_untag() $this->assertEmpty($post->tags->pluck('slug')->toArray()); } - /** @test */ + #[Test] public function it_can_remove_all_tags() { $post = $this->createPost(); - $post->tag('foo, bar, baz'); + $queryCount = $this->withQueryCount(fn () => $post->tag('foo, bar, baz')); $this->assertCount(3, $post->tags); + $this->assertLessThanOrEqual(9, $queryCount); - $post->untag(); + $queryCount = $this->withQueryCount(fn () => $post->untag()); $this->assertCount(0, $post->tags); + $this->assertLessThanOrEqual(9, $queryCount); } - /** @test */ + #[Test] public function it_can_set_tags() { $post = $this->createPost(); @@ -96,7 +103,7 @@ public function it_can_set_tags() $this->assertSame(['foo', 'bar'], $post->tags->pluck('slug')->toArray()); } - /** @test */ + #[Test] public function it_can_retrieve_tags() { $post = $this->createPost(); @@ -106,7 +113,7 @@ public function it_can_retrieve_tags() $this->assertCount(3, $post->tags); } - /** @test */ + #[Test] public function it_can_retrieve_all_tags() { $post1 = $this->createPost(); @@ -119,7 +126,7 @@ public function it_can_retrieve_all_tags() $this->assertCount(0, Post2::allTags()->get()); } - /** @test */ + #[Test] public function it_can_retrieve_by_the_given_tags() { $post1 = $this->createPost(); @@ -135,7 +142,7 @@ public function it_can_retrieve_by_the_given_tags() $this->assertcount(1, Post::withTag('bat')->get()); } - /** @test */ + #[Test] public function it_can_retrieve_without_the_given_tags() { $post1 = $this->createPost(); @@ -151,7 +158,7 @@ public function it_can_retrieve_without_the_given_tags() $this->assertcount(1, Post::withoutTag('bat')->get()); } - /** @test */ + #[Test] public function it_can_get_and_set_the_tags_delimiter() { $post = new Post(); @@ -161,7 +168,7 @@ public function it_can_get_and_set_the_tags_delimiter() $this->assertSame(',', $post->getTagsDelimiter()); } - /** @test */ + #[Test] public function it_can_get_and_set_the_tags_model() { $post = new Post(); @@ -171,7 +178,7 @@ public function it_can_get_and_set_the_tags_model() $this->assertSame(IlluminateTag::class, $post->getTagsModel()); } - /** @test */ + #[Test] public function it_can_get_and_set_the_slug_generator_as_a_string() { $post = new Post(); @@ -181,7 +188,7 @@ public function it_can_get_and_set_the_slug_generator_as_a_string() $this->assertSame('Illuminate\Support\Str::slug', $post->getSlugGenerator()); } - /** @test */ + #[Test] public function it_can_get_and_set_the_slug_generator_as_a_closure() { $post = new Post(); From 6ffc09f4e26c6ed280b503f24f5ac140f7247a87 Mon Sep 17 00:00:00 2001 From: Tofandel Date: Mon, 10 Mar 2025 14:49:37 +0100 Subject: [PATCH 2/2] Improve untag all query count and add phpdoc for properties --- src/IlluminateTag.php | 4 ++++ src/TaggableTrait.php | 21 +++++++++++++++++---- tests/TaggableTraitTest.php | 8 ++++---- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/IlluminateTag.php b/src/IlluminateTag.php index b01d10c..8c66ef3 100644 --- a/src/IlluminateTag.php +++ b/src/IlluminateTag.php @@ -25,6 +25,10 @@ use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\MorphTo; +/** @property string $name */ +/** @property string $namespace */ +/** @property string $slug */ +/** @property int $count */ class IlluminateTag extends Model { /** diff --git a/src/TaggableTrait.php b/src/TaggableTrait.php index aca8ce0..ce601f3 100644 --- a/src/TaggableTrait.php +++ b/src/TaggableTrait.php @@ -20,10 +20,12 @@ namespace Cartalyst\Tags; +use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Relations\MorphToMany; +/** @property Collection $tags */ trait TaggableTrait { /** @@ -172,10 +174,19 @@ public function tag($tags): bool */ public function untag($tags = null): bool { - $tags = $tags ?: $this->tags->pluck('name')->all(); - - foreach ($this->prepareTags($tags) as $tag) { - $this->removeTag($tag); + if (empty($tags)) { + $tags = $this->tags; + if (!empty($tags)) { + $this->tags()->detach(); + foreach ($tags as $tag) { + $tag->update(['count' => $tag->count - 1]); + } + $this->tags = new Collection(); + } + } else { + foreach ($this->prepareTags($tags) as $tag) { + $this->removeTag($tag); + } } return true; @@ -214,6 +225,7 @@ public function setTags($tags, string $type = 'name'): bool */ public function addTag(string $name): void { + /** @var IlluminateTag $tag */ $tag = $this->createTagsModel()->firstOrCreate([ 'slug' => $this->generateTagSlug($name), 'namespace' => $this->getEntityClassName(), @@ -242,6 +254,7 @@ public function removeTag(string $name): void $namespace = $this->getEntityClassName(); + /** @var IlluminateTag $tag */ $tag = $this->tags() ->whereNamespace($namespace) ->where(function ($query) use ($name, $slug) { diff --git a/tests/TaggableTraitTest.php b/tests/TaggableTraitTest.php index c361233..0d081d6 100644 --- a/tests/TaggableTraitTest.php +++ b/tests/TaggableTraitTest.php @@ -44,9 +44,9 @@ public function it_can_add_a_single_tag() #[Test] public function it_can_add_multiple_tags() { - $post1 = $this->createPost(); - $post2 = $this->createPost(); - $post3 = $this->createPost(); + $post1 = $this->createPost(); + $post2 = $this->createPost(); + $post3 = $this->createPost(); $count = $this->withQueryCount(function () use ($post1, $post2, $post3) { $post1->tag('foo, bar'); @@ -88,7 +88,7 @@ public function it_can_remove_all_tags() $queryCount = $this->withQueryCount(fn () => $post->untag()); $this->assertCount(0, $post->tags); - $this->assertLessThanOrEqual(9, $queryCount); + $this->assertLessThanOrEqual(4, $queryCount); } #[Test]