From ef12e90ae7d681f4a7c99f5677194a608a0647e7 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 31 Jan 2021 13:05:21 +0100
Subject: [PATCH] Removed non-used deprecated method and added missing tests

---
 module/Core/src/Entity/ShortUrl.php           | 10 -----
 .../PersistenceShortUrlRelationResolver.php   |  9 +++-
 ...ersistenceShortUrlRelationResolverTest.php | 41 +++++++++++++++++++
 .../SimpleShortUrlRelationResolverTest.php    | 12 ++++++
 4 files changed, 60 insertions(+), 12 deletions(-)

diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php
index bf01e7a5..2919be17 100644
--- a/module/Core/src/Entity/ShortUrl.php
+++ b/module/Core/src/Entity/ShortUrl.php
@@ -128,16 +128,6 @@ class ShortUrl extends AbstractEntity
         return $this->tags;
     }
 
-    /**
-     * @param Collection|Tag[] $tags
-     * @deprecated Use ShortUrl::update to set the tags on this ShortUrl
-     */
-    public function setTags(Collection $tags): self
-    {
-        $this->tags = $tags;
-        return $this;
-    }
-
     public function update(
         ShortUrlEdit $shortUrlEdit,
         ?ShortUrlRelationResolverInterface $relationResolver = null
diff --git a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php
index 940a5a9f..fd0428bf 100644
--- a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php
+++ b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php
@@ -38,8 +38,13 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt
      */
     public function resolveTags(array $tags): Collections\Collection
     {
-        return new Collections\ArrayCollection(map($tags, function (string $tagName): Tag {
-            $tag = $this->em->getRepository(Tag::class)->findOneBy(['name' => $tagName]) ?? new Tag($tagName);
+        if (empty($tags)) {
+            return new Collections\ArrayCollection();
+        }
+
+        $repo = $this->em->getRepository(Tag::class);
+        return new Collections\ArrayCollection(map($tags, function (string $tagName) use ($repo): Tag {
+            $tag = $repo->findOneBy(['name' => $tagName]) ?? new Tag($tagName);
             $this->em->persist($tag);
 
             return $tag;
diff --git a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php
index 9cea7883..463ee1ef 100644
--- a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php
+++ b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php
@@ -7,9 +7,12 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl\Resolver;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\Persistence\ObjectRepository;
 use PHPUnit\Framework\TestCase;
+use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Entity\Domain;
+use Shlinkio\Shlink\Core\Entity\Tag;
+use Shlinkio\Shlink\Core\Repository\TagRepositoryInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver;
 
 class PersistenceShortUrlRelationResolverTest extends TestCase
@@ -62,4 +65,42 @@ class PersistenceShortUrlRelationResolverTest extends TestCase
         yield 'not found domain' => [null, $authority];
         yield 'found domain' => [new Domain($authority), $authority];
     }
+
+    /** @test */
+    public function findsAndPersistsTagsWrappedIntoCollection(): void
+    {
+        $tags = ['foo', 'bar', 'baz'];
+
+        $tagRepo = $this->prophesize(TagRepositoryInterface::class);
+        $findTag = $tagRepo->findOneBy(Argument::type('array'))->will(function (array $args): ?Tag {
+            ['name' => $name] = $args[0];
+            return $name === 'foo' ? new Tag($name) : null;
+        });
+        $getRepo = $this->em->getRepository(Tag::class)->willReturn($tagRepo->reveal());
+        $persist = $this->em->persist(Argument::type(Tag::class));
+
+        $result = $this->resolver->resolveTags($tags);
+
+        self::assertCount(3, $result);
+        self::assertEquals([new Tag('foo'), new Tag('bar'), new Tag('baz')], $result->toArray());
+        $findTag->shouldHaveBeenCalledTimes(3);
+        $getRepo->shouldHaveBeenCalledOnce();
+        $persist->shouldHaveBeenCalledTimes(3);
+    }
+
+    /** @test */
+    public function returnsEmptyCollectionWhenProvidingEmptyListOfTags(): void
+    {
+        $tagRepo = $this->prophesize(TagRepositoryInterface::class);
+        $findTag = $tagRepo->findOneBy(Argument::type('array'))->willReturn(null);
+        $getRepo = $this->em->getRepository(Tag::class)->willReturn($tagRepo->reveal());
+        $persist = $this->em->persist(Argument::type(Tag::class));
+
+        $result = $this->resolver->resolveTags([]);
+
+        self::assertEmpty($result);
+        $findTag->shouldNotHaveBeenCalled();
+        $getRepo->shouldNotHaveBeenCalled();
+        $persist->shouldNotHaveBeenCalled();
+    }
 }
diff --git a/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php
index 84d838b9..483cb67a 100644
--- a/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php
+++ b/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php
@@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl\Resolver;
 
 use PHPUnit\Framework\TestCase;
 use Shlinkio\Shlink\Core\Entity\Domain;
+use Shlinkio\Shlink\Core\Entity\Tag;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\SimpleShortUrlRelationResolver;
 
 class SimpleShortUrlRelationResolverTest extends TestCase
@@ -38,4 +39,15 @@ class SimpleShortUrlRelationResolverTest extends TestCase
         yield 'empty domain' => [null];
         yield 'non-empty domain' => ['domain.com'];
     }
+
+    /** @test */
+    public function tagsAreWrappedInEntityCollection(): void
+    {
+        $tags = ['foo', 'bar', 'baz'];
+
+        $result = $this->resolver->resolveTags($tags);
+
+        self::assertCount(3, $result);
+        self::assertEquals([new Tag('foo'), new Tag('bar'), new Tag('baz')], $result->toArray());
+    }
 }