Merge pull request #1244 from acelaya-forks/feature/missing-domain-in-error

Added domain to DeleteShortUrlException

CHANGELOG.md

@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 ### Changed
 * [#1218](https://github.com/shlinkio/shlink/issues/1218) Updated to symfony/mercure 0.6.
 * [#1223](https://github.com/shlinkio/shlink/issues/1223) Updated to phpstan 1.0.
+* Added `domain` field to `DeleteShortUrlException` exception.
 
 ### Deprecated
 * *Nothing*

module/CLI/src/Util/ProcessRunner.php

@@ -34,7 +34,7 @@ class ProcessRunner implements ProcessRunnerInterface
         }
 
         /** @var DebugFormatterHelper $formatter */
-        $formatter = $this->helper->getHelperSet()->get('debug_formatter');
+        $formatter = $this->helper->getHelperSet()?->get('debug_formatter') ?? new DebugFormatterHelper();
         /** @var Process $process */
         $process = ($this->createProcess)($cmd);
 

module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php

@@ -83,7 +83,10 @@ class DeleteShortUrlCommandTest extends TestCase
                 $ignoreThreshold = array_pop($args);
 
                 if (!$ignoreThreshold) {
-                    throw Exception\DeleteShortUrlException::fromVisitsThreshold(10, $shortCode);
+                    throw Exception\DeleteShortUrlException::fromVisitsThreshold(
+                        10,
+                        ShortUrlIdentifier::fromShortCodeAndDomain($shortCode),
+                    );
                 }
             },
         );
@@ -93,7 +96,7 @@ class DeleteShortUrlCommandTest extends TestCase
         $output = $this->commandTester->getDisplay();
 
         self::assertStringContainsString(sprintf(
-            'Impossible to delete short URL with short code "%s" since it has more than "10" visits.',
+            'Impossible to delete short URL with short code "%s", since it has more than "10" visits.',
             $shortCode,
         ), $output);
         self::assertStringContainsString($expectedMessage, $output);
@@ -112,7 +115,10 @@ class DeleteShortUrlCommandTest extends TestCase
     {
         $shortCode = 'abc123';
         $deleteByShortCode = $this->service->deleteByShortCode(new ShortUrlIdentifier($shortCode), false)->willThrow(
-            Exception\DeleteShortUrlException::fromVisitsThreshold(10, $shortCode),
+            Exception\DeleteShortUrlException::fromVisitsThreshold(
+                10,
+                ShortUrlIdentifier::fromShortCodeAndDomain($shortCode),
+            ),
         );
         $this->commandTester->setInputs(['no']);
 
@@ -120,7 +126,7 @@ class DeleteShortUrlCommandTest extends TestCase
         $output = $this->commandTester->getDisplay();
 
         self::assertStringContainsString(sprintf(
-            'Impossible to delete short URL with short code "%s" since it has more than "10" visits.',
+            'Impossible to delete short URL with short code "%s", since it has more than "10" visits.',
             $shortCode,
         ), $output);
         self::assertStringContainsString('Short URL was not deleted.', $output);

module/Core/src/Exception/DeleteShortUrlException.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\Model\ShortUrlIdentifier;
 
 use function sprintf;
 
@@ -17,11 +18,15 @@ class DeleteShortUrlException extends DomainException implements ProblemDetailsE
     private const TITLE = 'Cannot delete short URL';
     private const TYPE = 'INVALID_SHORTCODE_DELETION'; // FIXME Deprecated: Should be INVALID_SHORT_URL_DELETION
 
-    public static function fromVisitsThreshold(int $threshold, string $shortCode): self
+    public static function fromVisitsThreshold(int $threshold, ShortUrlIdentifier $identifier): self
     {
+        $shortCode = $identifier->shortCode();
+        $domain = $identifier->domain();
+        $suffix = $domain === null ? '' : sprintf(' for domain "%s"', $domain);
         $e = new self(sprintf(
-            'Impossible to delete short URL with short code "%s" since it has more than "%s" visits.',
+            'Impossible to delete short URL with short code "%s"%s, since it has more than "%s" visits.',
             $shortCode,
+            $suffix,
             $threshold,
         ));
 
@@ -34,6 +39,10 @@ class DeleteShortUrlException extends DomainException implements ProblemDetailsE
             'threshold' => $threshold,
         ];
 
+        if ($domain !== null) {
+            $e->additional['domain'] = $domain;
+        }
+
         return $e;
     }
 

module/Core/src/Service/ShortUrl/DeleteShortUrlService.php

@@ -33,7 +33,7 @@ class DeleteShortUrlService implements DeleteShortUrlServiceInterface
         if (! $ignoreThreshold && $this->isThresholdReached($shortUrl)) {
             throw Exception\DeleteShortUrlException::fromVisitsThreshold(
                 $this->deleteShortUrlsOptions->getVisitsThreshold(),
-                $shortUrl->getShortCode(),
+                $identifier,
             );
         }
 

module/Core/test/Exception/DeleteShortUrlExceptionTest.php

@@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Core\Exception;
 
 use PHPUnit\Framework\TestCase;
 use Shlinkio\Shlink\Core\Exception\DeleteShortUrlException;
+use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
 
 use function Functional\map;
 use function range;
@@ -23,7 +24,10 @@ class DeleteShortUrlExceptionTest extends TestCase
         string $shortCode,
         string $expectedMessage,
     ): void {
-        $e = DeleteShortUrlException::fromVisitsThreshold($threshold, $shortCode);
+        $e = DeleteShortUrlException::fromVisitsThreshold(
+            $threshold,
+            ShortUrlIdentifier::fromShortCodeAndDomain($shortCode),
+        );
 
         self::assertEquals($threshold, $e->getVisitsThreshold());
         self::assertEquals($expectedMessage, $e->getMessage());
@@ -41,10 +45,29 @@ class DeleteShortUrlExceptionTest extends TestCase
     {
         return map(range(5, 50, 5), function (int $number) {
             return [$number, $shortCode = generateRandomShortCode(6), sprintf(
-                'Impossible to delete short URL with short code "%s" since it has more than "%s" visits.',
+                'Impossible to delete short URL with short code "%s", since it has more than "%s" visits.',
                 $shortCode,
                 $number,
             )];
         });
     }
+
+    /** @test */
+    public function domainIsPartOfAdditionalWhenProvidedInIdentifier(): void
+    {
+        $e = DeleteShortUrlException::fromVisitsThreshold(
+            10,
+            ShortUrlIdentifier::fromShortCodeAndDomain('abc123', 'doma.in'),
+        );
+        $expectedMessage = 'Impossible to delete short URL with short code "abc123" for domain "doma.in", since it '
+            . 'has more than "10" visits.';
+
+        self::assertEquals([
+            'shortCode' => 'abc123',
+            'domain' => 'doma.in',
+            'threshold' => 10,
+        ], $e->getAdditionalData());
+        self::assertEquals($expectedMessage, $e->getMessage());
+        self::assertEquals($expectedMessage, $e->getDetail());
+    }
 }

module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php

@@ -51,7 +51,7 @@ class DeleteShortUrlServiceTest extends TestCase
 
         $this->expectException(DeleteShortUrlException::class);
         $this->expectExceptionMessage(sprintf(
-            'Impossible to delete short URL with short code "%s" since it has more than "5" visits.',
+            'Impossible to delete short URL with short code "%s", since it has more than "5" visits.',
             $this->shortCode,
         ));
 

module/Rest/test-api/Action/DeleteShortUrlTest.php

@@ -40,7 +40,8 @@ class DeleteShortUrlTest extends ApiTestCase
         for ($i = 0; $i < 20; $i++) {
             self::assertEquals(self::STATUS_FOUND, $this->callShortUrl('abc123')->getStatusCode());
         }
-        $expectedDetail = 'Impossible to delete short URL with short code "abc123" since it has more than "15" visits.';
+        $expectedDetail = 'Impossible to delete short URL with short code "abc123", since it has more than "15" '
+            . 'visits.';
 
         $resp = $this->callApiWithKey(self::METHOD_DELETE, '/short-urls/abc123');
         $payload = $this->getJsonResponsePayload($resp);