Ensured a specific exception is thrown from TagService when trying to rename a tag to the name of another tag which already exists

module/Core/src/Service/Tag/TagService.php

@@ -8,6 +8,7 @@ use Doctrine\Common\Collections\Collection;
 use Doctrine\ORM;
 use Shlinkio\Shlink\Core\Entity\Tag;
 use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
+use Shlinkio\Shlink\Core\Exception\TagConflictException;
 use Shlinkio\Shlink\Core\Repository\TagRepository;
 use Shlinkio\Shlink\Core\Util\TagManagerTrait;
 
@@ -64,17 +65,26 @@ class TagService implements TagServiceInterface
      * @param string $newName
      * @return Tag
      * @throws EntityDoesNotExistException
+     * @throws TagConflictException
      * @throws ORM\OptimisticLockException
      */
     public function renameTag($oldName, $newName): Tag
     {
+        /** @var TagRepository $repo */
+        $repo = $this->em->getRepository(Tag::class);
         $criteria = ['name' => $oldName];
+
         /** @var Tag|null $tag */
-        $tag = $this->em->getRepository(Tag::class)->findOneBy($criteria);
+        $tag = $repo->findOneBy($criteria);
         if ($tag === null) {
             throw EntityDoesNotExistException::createFromEntityAndConditions(Tag::class, $criteria);
         }
 
+        $newNameExists = $newName !== $oldName && $repo->count(['name' => $newName]) > 0;
+        if ($newNameExists) {
+            throw TagConflictException::fromExistingTag($newName);
+        }
+
         $tag->rename($newName);
 
         /** @var ORM\EntityManager $em */

module/Core/src/Service/Tag/TagServiceInterface.php

@@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Service\Tag;
 use Doctrine\Common\Collections\Collection;
 use Shlinkio\Shlink\Core\Entity\Tag;
 use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
+use Shlinkio\Shlink\Core\Exception\TagConflictException;
 
 interface TagServiceInterface
 {
@@ -34,6 +35,7 @@ interface TagServiceInterface
      * @param string $newName
      * @return Tag
      * @throws EntityDoesNotExistException
+     * @throws TagConflictException
      */
     public function renameTag($oldName, $newName): Tag;
 }

module/Core/test/Service/Tag/TagServiceTest.php

@@ -11,6 +11,7 @@ use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Entity\Tag;
 use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
+use Shlinkio\Shlink\Core\Exception\TagConflictException;
 use Shlinkio\Shlink\Core\Repository\TagRepository;
 use Shlinkio\Shlink\Core\Service\Tag\TagService;
 
@@ -88,22 +89,51 @@ class TagServiceTest extends TestCase
         $this->service->renameTag('foo', 'bar');
     }
 
-    /** @test */
+    /**
-    public function renameValidTagChangesItsName()
+     * @test
+     * @dataProvider provideValidRenames
+     */
+    public function renameValidTagChangesItsName(string $oldName, string $newName, int $count): void
     {
         $expected = new Tag('foo');
 
         $repo = $this->prophesize(TagRepository::class);
         $find = $repo->findOneBy(Argument::cetera())->willReturn($expected);
+        $countTags = $repo->count(Argument::cetera())->willReturn($count);
         $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal());
         $flush = $this->em->flush($expected)->willReturn(null);
 
-        $tag = $this->service->renameTag('foo', 'bar');
+        $tag = $this->service->renameTag($oldName, $newName);
 
         $this->assertSame($expected, $tag);
-        $this->assertEquals('bar', (string) $tag);
+        $this->assertEquals($newName, (string) $tag);
         $find->shouldHaveBeenCalled();
         $getRepo->shouldHaveBeenCalled();
         $flush->shouldHaveBeenCalled();
+        $countTags->shouldHaveBeenCalledTimes($count > 0 ? 0 : 1);
+    }
+
+    public function provideValidRenames(): iterable
+    {
+        yield 'same names' => ['foo', 'foo', 1];
+        yield 'different names names' => ['foo', 'bar', 0];
+    }
+
+    /** @test */
+    public function renameTagToAnExistingNameThrowsException(): void
+    {
+        $repo = $this->prophesize(TagRepository::class);
+        $find = $repo->findOneBy(Argument::cetera())->willReturn(new Tag('foo'));
+        $countTags = $repo->count(Argument::cetera())->willReturn(1);
+        $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal());
+        $flush = $this->em->flush(Argument::any())->willReturn(null);
+
+        $find->shouldBeCalled();
+        $getRepo->shouldBeCalled();
+        $countTags->shouldBeCalled();
+        $flush->shouldNotBeCalled();
+        $this->expectException(TagConflictException::class);
+
+        $this->service->renameTag('foo', 'bar');
     }
 }