From cdfd14e63f84d8c3d13a6f829c48e1e7393520ed Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 31 Jan 2021 12:24:26 +0100
Subject: [PATCH] Deprecated action and endpoint to edit short URL tags

---
 .../paths/v1_short-urls_{shortCode}.json      |  7 ++++++
 .../paths/v1_short-urls_{shortCode}_tags.json |  3 ++-
 module/Core/src/Service/ShortUrlService.php   | 18 ---------------
 .../src/Service/ShortUrlServiceInterface.php  |  7 ------
 .../Core/test/Service/ShortUrlServiceTest.php | 22 -------------------
 .../ShortUrl/EditShortUrlTagsAction.php       |  7 +++++-
 .../ShortUrl/EditShortUrlTagsActionTest.php   |  5 +++--
 7 files changed, 18 insertions(+), 51 deletions(-)

diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}.json b/docs/swagger/paths/v1_short-urls_{shortCode}.json
index c7e7dc8a..546e1920 100644
--- a/docs/swagger/paths/v1_short-urls_{shortCode}.json
+++ b/docs/swagger/paths/v1_short-urls_{shortCode}.json
@@ -131,6 +131,13 @@
                             "validateUrl": {
                                 "description": "Tells if the long URL (if provided) should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config",
                                 "type": "boolean"
+                            },
+                            "tags": {
+                                "type": "array",
+                                "items": {
+                                    "type": "string"
+                                },
+                                "description": "The list of tags to set to the short URL."
                             }
                         }
                     }
diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json b/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json
index fd497380..6ea642b0 100644
--- a/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json
+++ b/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json
@@ -1,11 +1,12 @@
 {
     "put": {
+        "deprecated": true,
         "operationId": "editShortUrlTags",
         "tags": [
             "Short URLs"
         ],
         "summary": "Edit tags on short URL",
-        "description": "Edit the tags on URL identified by provided short code.",
+        "description": "Edit the tags on URL identified by provided short code.<br />This endpoint is deprecated. Use the [Edit short URL](#/Short%20URLs/editShortUrl) endpoint to edit tags.",
         "parameters": [
             {
                 "$ref": "../parameters/version.json"
diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php
index 44de59a1..70606219 100644
--- a/module/Core/src/Service/ShortUrlService.php
+++ b/module/Core/src/Service/ShortUrlService.php
@@ -16,14 +16,11 @@ use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
 use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface;
-use Shlinkio\Shlink\Core\Util\TagManagerTrait;
 use Shlinkio\Shlink\Core\Util\UrlValidatorInterface;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 class ShortUrlService implements ShortUrlServiceInterface
 {
-    use TagManagerTrait;
-
     private ORM\EntityManagerInterface $em;
     private ShortUrlResolverInterface $urlResolver;
     private UrlValidatorInterface $urlValidator;
@@ -55,21 +52,6 @@ class ShortUrlService implements ShortUrlServiceInterface
         return $paginator;
     }
 
-    /**
-     * @param string[] $tags
-     * @deprecated Use updateShortUrl instead
-     * @throws ShortUrlNotFoundException
-     */
-    public function setTagsByShortCode(ShortUrlIdentifier $identifier, array $tags, ?ApiKey $apiKey = null): ShortUrl
-    {
-        $shortUrl = $this->urlResolver->resolveShortUrl($identifier, $apiKey);
-        $shortUrl->setTags($this->tagNamesToEntities($this->em, $tags));
-
-        $this->em->flush();
-
-        return $shortUrl;
-    }
-
     /**
      * @throws ShortUrlNotFoundException
      * @throws InvalidUrlException
diff --git a/module/Core/src/Service/ShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrlServiceInterface.php
index ac9f2095..3884b55e 100644
--- a/module/Core/src/Service/ShortUrlServiceInterface.php
+++ b/module/Core/src/Service/ShortUrlServiceInterface.php
@@ -20,13 +20,6 @@ interface ShortUrlServiceInterface
      */
     public function listShortUrls(ShortUrlsParams $params, ?ApiKey $apiKey = null): Paginator;
 
-    /**
-     * @param string[] $tags
-     * @deprecated Use updateShortUrl instead
-     * @throws ShortUrlNotFoundException
-     */
-    public function setTagsByShortCode(ShortUrlIdentifier $identifier, array $tags, ?ApiKey $apiKey = null): ShortUrl;
-
     /**
      * @throws ShortUrlNotFoundException
      * @throws InvalidUrlException
diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php
index 5d873ee9..178561f0 100644
--- a/module/Core/test/Service/ShortUrlServiceTest.php
+++ b/module/Core/test/Service/ShortUrlServiceTest.php
@@ -6,13 +6,11 @@ namespace ShlinkioTest\Shlink\Core\Service;
 
 use Cake\Chronos\Chronos;
 use Doctrine\ORM\EntityManagerInterface;
-use Doctrine\ORM\EntityRepository;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
-use Shlinkio\Shlink\Core\Entity\Tag;
 use Shlinkio\Shlink\Core\Model\ShortUrlEdit;
 use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
 use Shlinkio\Shlink\Core\Model\ShortUrlsParams;
@@ -77,26 +75,6 @@ class ShortUrlServiceTest extends TestCase
         self::assertCount(4, $paginator->getCurrentPageResults());
     }
 
-    /**
-     * @test
-     * @dataProvider provideAdminApiKeys
-     */
-    public function providedTagsAreGetFromRepoAndSetToTheShortUrl(?ApiKey $apiKey): void
-    {
-        $shortUrl = ShortUrl::createEmpty();
-        $shortCode = 'abc123';
-        $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode), $apiKey)
-            ->willReturn($shortUrl)
-            ->shouldBeCalledOnce();
-
-        $tagRepo = $this->prophesize(EntityRepository::class);
-        $tagRepo->findOneBy(['name' => 'foo'])->willReturn(new Tag('foo'))->shouldBeCalledOnce();
-        $tagRepo->findOneBy(['name' => 'bar'])->willReturn(null)->shouldBeCalledOnce();
-        $this->em->getRepository(Tag::class)->willReturn($tagRepo->reveal());
-
-        $this->service->setTagsByShortCode(new ShortUrlIdentifier($shortCode), ['foo', 'bar'], $apiKey);
-    }
-
     /**
      * @test
      * @dataProvider provideShortUrlEdits
diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php
index 7d115765..d114049c 100644
--- a/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php
+++ b/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php
@@ -8,11 +8,14 @@ use Laminas\Diactoros\Response\JsonResponse;
 use Psr\Http\Message\ResponseInterface as Response;
 use Psr\Http\Message\ServerRequestInterface as Request;
 use Shlinkio\Shlink\Core\Exception\ValidationException;
+use Shlinkio\Shlink\Core\Model\ShortUrlEdit;
 use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
 use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface;
+use Shlinkio\Shlink\Core\Validation\ShortUrlInputFilter;
 use Shlinkio\Shlink\Rest\Action\AbstractRestAction;
 use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware;
 
+/** @deprecated */
 class EditShortUrlTagsAction extends AbstractRestAction
 {
     protected const ROUTE_PATH = '/short-urls/{shortCode}/tags';
@@ -38,7 +41,9 @@ class EditShortUrlTagsAction extends AbstractRestAction
         $identifier = ShortUrlIdentifier::fromApiRequest($request);
         $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request);
 
-        $shortUrl = $this->shortUrlService->setTagsByShortCode($identifier, $tags, $apiKey);
+        $shortUrl = $this->shortUrlService->updateShortUrl($identifier, ShortUrlEdit::fromRawData([
+            ShortUrlInputFilter::TAGS => $tags,
+        ]), $apiKey);
         return new JsonResponse(['tags' => $shortUrl->getTags()->toArray()]);
     }
 }
diff --git a/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php b/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php
index 60d1d093..a345046a 100644
--- a/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php
+++ b/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php
@@ -12,6 +12,7 @@ use Prophecy\Prophecy\ObjectProphecy;
 use Psr\Http\Message\ServerRequestInterface;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\ValidationException;
+use Shlinkio\Shlink\Core\Model\ShortUrlEdit;
 use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
 use Shlinkio\Shlink\Core\Service\ShortUrlService;
 use Shlinkio\Shlink\Rest\Action\ShortUrl\EditShortUrlTagsAction;
@@ -41,9 +42,9 @@ class EditShortUrlTagsActionTest extends TestCase
     public function tagsListIsReturnedIfCorrectShortCodeIsProvided(): void
     {
         $shortCode = 'abc123';
-        $this->shortUrlService->setTagsByShortCode(
+        $this->shortUrlService->updateShortUrl(
             new ShortUrlIdentifier($shortCode),
-            [],
+            Argument::type(ShortUrlEdit::class),
             Argument::type(ApiKey::class),
         )->willReturn(ShortUrl::createEmpty())
          ->shouldBeCalledOnce();