From b5710f87e27cc59f03e86079bfae9e8ab0856b7a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 6 Jan 2021 13:11:28 +0100 Subject: [PATCH] Created value object to wrap the renaming of a tag --- .../CLI/src/Command/Tag/RenameTagCommand.php | 3 +- .../test/Command/Tag/RenameTagCommandTest.php | 9 ++- .../src/Exception/TagConflictException.php | 10 ++- module/Core/src/Tag/Model/TagRenaming.php | 68 +++++++++++++++++++ module/Core/src/Tag/TagService.php | 13 ++-- module/Core/src/Tag/TagServiceInterface.php | 3 +- .../Exception/TagConflictExceptionTest.php | 7 +- .../Core/test/Service/Tag/TagServiceTest.php | 7 +- .../Rest/src/Action/Tag/UpdateTagAction.php | 11 +-- .../test/Action/Tag/UpdateTagActionTest.php | 3 +- 10 files changed, 102 insertions(+), 32 deletions(-) create mode 100644 module/Core/src/Tag/Model/TagRenaming.php diff --git a/module/CLI/src/Command/Tag/RenameTagCommand.php b/module/CLI/src/Command/Tag/RenameTagCommand.php index fe42a832..8bfb0242 100644 --- a/module/CLI/src/Command/Tag/RenameTagCommand.php +++ b/module/CLI/src/Command/Tag/RenameTagCommand.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\CLI\Command\Tag; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; +use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -42,7 +43,7 @@ class RenameTagCommand extends Command $newName = $input->getArgument('newName'); try { - $this->tagService->renameTag($oldName, $newName); + $this->tagService->renameTag(TagRenaming::fromNames($oldName, $newName)); $io->success('Tag properly renamed.'); return ExitCodes::EXIT_SUCCESS; } catch (TagNotFoundException | TagConflictException $e) { diff --git a/module/CLI/test/Command/Tag/RenameTagCommandTest.php b/module/CLI/test/Command/Tag/RenameTagCommandTest.php index 9764a111..d457c25d 100644 --- a/module/CLI/test/Command/Tag/RenameTagCommandTest.php +++ b/module/CLI/test/Command/Tag/RenameTagCommandTest.php @@ -10,6 +10,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Tag\RenameTagCommand; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; +use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -37,7 +38,9 @@ class RenameTagCommandTest extends TestCase { $oldName = 'foo'; $newName = 'bar'; - $renameTag = $this->tagService->renameTag($oldName, $newName)->willThrow(TagNotFoundException::fromTag('foo')); + $renameTag = $this->tagService->renameTag(TagRenaming::fromNames($oldName, $newName))->willThrow( + TagNotFoundException::fromTag('foo'), + ); $this->commandTester->execute([ 'oldName' => $oldName, @@ -54,7 +57,9 @@ class RenameTagCommandTest extends TestCase { $oldName = 'foo'; $newName = 'bar'; - $renameTag = $this->tagService->renameTag($oldName, $newName)->willReturn(new Tag($newName)); + $renameTag = $this->tagService->renameTag(TagRenaming::fromNames($oldName, $newName))->willReturn( + new Tag($newName), + ); $this->commandTester->execute([ 'oldName' => $oldName, diff --git a/module/Core/src/Exception/TagConflictException.php b/module/Core/src/Exception/TagConflictException.php index 7362f76b..d551ec19 100644 --- a/module/Core/src/Exception/TagConflictException.php +++ b/module/Core/src/Exception/TagConflictException.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Exception; use Fig\Http\Message\StatusCodeInterface; use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; +use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; use function sprintf; @@ -17,18 +18,15 @@ class TagConflictException extends RuntimeException implements ProblemDetailsExc private const TITLE = 'Tag conflict'; private const TYPE = 'TAG_CONFLICT'; - public static function fromExistingTag(string $oldName, string $newName): self + public static function forExistingTag(TagRenaming $renaming): self { - $e = new self(sprintf('You cannot rename tag %s to %s, because it already exists', $oldName, $newName)); + $e = new self(sprintf('You cannot rename tag %s, because it already exists', $renaming->toString())); $e->detail = $e->getMessage(); $e->title = self::TITLE; $e->type = self::TYPE; $e->status = StatusCodeInterface::STATUS_CONFLICT; - $e->additional = [ - 'oldName' => $oldName, - 'newName' => $newName, - ]; + $e->additional = $renaming->toArray(); return $e; } diff --git a/module/Core/src/Tag/Model/TagRenaming.php b/module/Core/src/Tag/Model/TagRenaming.php new file mode 100644 index 00000000..1f677376 --- /dev/null +++ b/module/Core/src/Tag/Model/TagRenaming.php @@ -0,0 +1,68 @@ +oldName = $oldName; + $o->newName = $newName; + + return $o; + } + + public static function fromArray(array $payload): self + { + if (! isset($payload['oldName'], $payload['newName'])) { + throw ValidationException::fromArray([ + 'oldName' => 'oldName is required', + 'newName' => 'newName is required', + ]); + } + + return self::fromNames($payload['oldName'], $payload['newName']); + } + + public function oldName(): string + { + return $this->oldName; + } + + public function newName(): string + { + return $this->newName; + } + + public function nameChanged(): bool + { + return $this->oldName !== $this->newName; + } + + public function toString(): string + { + return sprintf('%s to %s', $this->oldName, $this->newName); + } + + public function toArray(): array + { + return [ + 'oldName' => $this->oldName, + 'newName' => $this->newName, + ]; + } +} diff --git a/module/Core/src/Tag/TagService.php b/module/Core/src/Tag/TagService.php index 342c5d31..5ce2be0f 100644 --- a/module/Core/src/Tag/TagService.php +++ b/module/Core/src/Tag/TagService.php @@ -13,6 +13,7 @@ use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\Core\Repository\TagRepositoryInterface; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; +use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; use Shlinkio\Shlink\Core\Util\TagManagerTrait; use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -82,23 +83,23 @@ class TagService implements TagServiceInterface * @throws TagNotFoundException * @throws TagConflictException */ - public function renameTag(string $oldName, string $newName): Tag + public function renameTag(TagRenaming $renaming): Tag { /** @var TagRepository $repo */ $repo = $this->em->getRepository(Tag::class); /** @var Tag|null $tag */ - $tag = $repo->findOneBy(['name' => $oldName]); + $tag = $repo->findOneBy(['name' => $renaming->oldName()]); if ($tag === null) { - throw TagNotFoundException::fromTag($oldName); + throw TagNotFoundException::fromTag($renaming->oldName()); } - $newNameExists = $newName !== $oldName && $repo->count(['name' => $newName]) > 0; + $newNameExists = $renaming->nameChanged() && $repo->count(['name' => $renaming->newName()]) > 0; if ($newNameExists) { - throw TagConflictException::fromExistingTag($oldName, $newName); + throw TagConflictException::forExistingTag($renaming); } - $tag->rename($newName); + $tag->rename($renaming->newName()); $this->em->flush(); return $tag; diff --git a/module/Core/src/Tag/TagServiceInterface.php b/module/Core/src/Tag/TagServiceInterface.php index 698736a6..7bcf2ace 100644 --- a/module/Core/src/Tag/TagServiceInterface.php +++ b/module/Core/src/Tag/TagServiceInterface.php @@ -9,6 +9,7 @@ use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; +use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; use Shlinkio\Shlink\Rest\Entity\ApiKey; interface TagServiceInterface @@ -39,5 +40,5 @@ interface TagServiceInterface * @throws TagNotFoundException * @throws TagConflictException */ - public function renameTag(string $oldName, string $newName): Tag; + public function renameTag(TagRenaming $renaming): Tag; } diff --git a/module/Core/test/Exception/TagConflictExceptionTest.php b/module/Core/test/Exception/TagConflictExceptionTest.php index 156fd500..4427eb40 100644 --- a/module/Core/test/Exception/TagConflictExceptionTest.php +++ b/module/Core/test/Exception/TagConflictExceptionTest.php @@ -2,22 +2,23 @@ declare(strict_types=1); -namespace ShlinkioTest\Shlink\Rest\Exception; +namespace ShlinkioTest\Shlink\Core\Exception; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Exception\TagConflictException; +use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; use function sprintf; class TagConflictExceptionTest extends TestCase { /** @test */ - public function properlyCreatesExceptionFromNotFoundTag(): void + public function properlyCreatesExceptionForExistingTag(): void { $oldName = 'foo'; $newName = 'bar'; $expectedMessage = sprintf('You cannot rename tag %s to %s, because it already exists', $oldName, $newName); - $e = TagConflictException::fromExistingTag($oldName, $newName); + $e = TagConflictException::forExistingTag(TagRenaming::fromNames($oldName, $newName)); self::assertEquals($expectedMessage, $e->getMessage()); self::assertEquals($expectedMessage, $e->getDetail()); diff --git a/module/Core/test/Service/Tag/TagServiceTest.php b/module/Core/test/Service/Tag/TagServiceTest.php index a66fdc6f..842ce9a2 100644 --- a/module/Core/test/Service/Tag/TagServiceTest.php +++ b/module/Core/test/Service/Tag/TagServiceTest.php @@ -14,6 +14,7 @@ use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; +use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; use Shlinkio\Shlink\Core\Tag\TagService; class TagServiceTest extends TestCase @@ -92,7 +93,7 @@ class TagServiceTest extends TestCase $find->shouldBeCalled(); $this->expectException(TagNotFoundException::class); - $this->service->renameTag('foo', 'bar'); + $this->service->renameTag(TagRenaming::fromNames('foo', 'bar')); } /** @@ -107,7 +108,7 @@ class TagServiceTest extends TestCase $countTags = $this->repo->count(Argument::cetera())->willReturn($count); $flush = $this->em->flush()->willReturn(null); - $tag = $this->service->renameTag($oldName, $newName); + $tag = $this->service->renameTag(TagRenaming::fromNames($oldName, $newName)); self::assertSame($expected, $tag); self::assertEquals($newName, (string) $tag); @@ -134,6 +135,6 @@ class TagServiceTest extends TestCase $flush->shouldNotBeCalled(); $this->expectException(TagConflictException::class); - $this->service->renameTag('foo', 'bar'); + $this->service->renameTag(TagRenaming::fromNames('foo', 'bar')); } } diff --git a/module/Rest/src/Action/Tag/UpdateTagAction.php b/module/Rest/src/Action/Tag/UpdateTagAction.php index fbf93f50..77431798 100644 --- a/module/Rest/src/Action/Tag/UpdateTagAction.php +++ b/module/Rest/src/Action/Tag/UpdateTagAction.php @@ -7,7 +7,7 @@ namespace Shlinkio\Shlink\Rest\Action\Tag; use Laminas\Diactoros\Response\EmptyResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Core\Exception\ValidationException; +use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -33,14 +33,7 @@ class UpdateTagAction extends AbstractRestAction public function handle(ServerRequestInterface $request): ResponseInterface { $body = $request->getParsedBody(); - if (! isset($body['oldName'], $body['newName'])) { - throw ValidationException::fromArray([ - 'oldName' => 'oldName is required', - 'newName' => 'newName is required', - ]); - } - - $this->tagService->renameTag($body['oldName'], $body['newName']); + $this->tagService->renameTag(TagRenaming::fromArray($body)); return new EmptyResponse(); } } diff --git a/module/Rest/test/Action/Tag/UpdateTagActionTest.php b/module/Rest/test/Action/Tag/UpdateTagActionTest.php index b82c8c2e..8546312f 100644 --- a/module/Rest/test/Action/Tag/UpdateTagActionTest.php +++ b/module/Rest/test/Action/Tag/UpdateTagActionTest.php @@ -10,6 +10,7 @@ use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\ValidationException; +use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\Tag\UpdateTagAction; @@ -53,7 +54,7 @@ class UpdateTagActionTest extends TestCase 'oldName' => 'foo', 'newName' => 'bar', ]); - $rename = $this->tagService->renameTag('foo', 'bar')->willReturn(new Tag('bar')); + $rename = $this->tagService->renameTag(TagRenaming::fromNames('foo', 'bar'))->willReturn(new Tag('bar')); $resp = $this->action->handle($request);