diff --git a/CHANGELOG.md b/CHANGELOG.md index 2002a898..57d95a4a 100644 --- a/CHANGELOG.md +++ b/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* diff --git a/module/CLI/src/Util/ProcessRunner.php b/module/CLI/src/Util/ProcessRunner.php index 66e94eb6..1a5471e5 100644 --- a/module/CLI/src/Util/ProcessRunner.php +++ b/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); diff --git a/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php index 765a1c4b..10a363c7 100644 --- a/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php +++ b/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); diff --git a/module/Core/src/Exception/DeleteShortUrlException.php b/module/Core/src/Exception/DeleteShortUrlException.php index 600fca57..98919b35 100644 --- a/module/Core/src/Exception/DeleteShortUrlException.php +++ b/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; } diff --git a/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php index 0732b737..e6f2e82d 100644 --- a/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php +++ b/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, ); } diff --git a/module/Core/test/Exception/DeleteShortUrlExceptionTest.php b/module/Core/test/Exception/DeleteShortUrlExceptionTest.php index 43dcc2e5..8c616ce1 100644 --- a/module/Core/test/Exception/DeleteShortUrlExceptionTest.php +++ b/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()); + } } diff --git a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php index 4c066848..6c03d7b5 100644 --- a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php +++ b/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, )); diff --git a/module/Rest/test-api/Action/DeleteShortUrlTest.php b/module/Rest/test-api/Action/DeleteShortUrlTest.php index 479527c1..bb512832 100644 --- a/module/Rest/test-api/Action/DeleteShortUrlTest.php +++ b/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);