diff --git a/module/Core/src/Service/Tag/TagService.php b/module/Core/src/Service/Tag/TagService.php index fd32efe5..24856188 100644 --- a/module/Core/src/Service/Tag/TagService.php +++ b/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 */ diff --git a/module/Core/src/Service/Tag/TagServiceInterface.php b/module/Core/src/Service/Tag/TagServiceInterface.php index 1eb11112..942da3a6 100644 --- a/module/Core/src/Service/Tag/TagServiceInterface.php +++ b/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; } diff --git a/module/Core/test/Service/Tag/TagServiceTest.php b/module/Core/test/Service/Tag/TagServiceTest.php index 7670eafd..6b904fe5 100644 --- a/module/Core/test/Service/Tag/TagServiceTest.php +++ b/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'); } }