Improved Exception management to be more specific

This commit is contained in:
Alejandro Celaya
2017-10-12 11:28:45 +02:00
parent c422a14c5c
commit 6208f6f0d5
13 changed files with 60 additions and 55 deletions

View File

@@ -7,8 +7,10 @@ use Interop\Http\ServerMiddleware\DelegateInterface;
use Interop\Http\ServerMiddleware\MiddlewareInterface;
use Psr\Http\Message\ResponseInterface as Response;
use Psr\Http\Message\ServerRequestInterface as Request;
use Shlinkio\Shlink\Common\Exception\PreviewGenerationException;
use Shlinkio\Shlink\Common\Service\PreviewGeneratorInterface;
use Shlinkio\Shlink\Common\Util\ResponseUtilsTrait;
use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
@@ -46,14 +48,14 @@ class PreviewAction implements MiddlewareInterface
try {
$url = $this->urlShortener->shortCodeToUrl($shortCode);
if (! isset($url)) {
return $delegate->process($request);
}
$imagePath = $this->previewGenerator->generatePreview($url);
return $this->generateImageResponse($imagePath);
} catch (InvalidShortCodeException $e) {
return $delegate->process($request);
} catch (EntityDoesNotExistException $e) {
return $delegate->process($request);
} catch (PreviewGenerationException $e) {
return $delegate->process($request);
}
}
}

View File

@@ -11,6 +11,7 @@ use Psr\Http\Message\ServerRequestInterface as Request;
use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
use Shlinkio\Shlink\Common\Response\QrCodeResponse;
use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
use Zend\Expressive\Router\RouterInterface;
@@ -55,12 +56,12 @@ class QrCodeAction implements MiddlewareInterface
$shortCode = $request->getAttribute('shortCode');
try {
$shortUrl = $this->urlShortener->shortCodeToUrl($shortCode);
if ($shortUrl === null) {
return $delegate->process($request);
}
} catch (InvalidShortCodeException $e) {
$this->logger->warning('Tried to create a QR code with an invalid short code' . PHP_EOL . $e);
return $delegate->process($request);
} catch (EntityDoesNotExistException $e) {
$this->logger->warning('Tried to create a QR code with a not found short code' . PHP_EOL . $e);
return $delegate->process($request);
}
$path = $this->router->generateUri('long-url-redirect', ['shortCode' => $shortCode]);

View File

@@ -9,8 +9,11 @@ use Psr\Http\Message\ResponseInterface as Response;
use Psr\Http\Message\ServerRequestInterface as Request;
use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface;
use Zend\Diactoros\Response\HtmlResponse;
use Zend\Diactoros\Response\RedirectResponse;
class RedirectAction implements MiddlewareInterface
@@ -66,9 +69,9 @@ class RedirectAction implements MiddlewareInterface
// Return a redirect response to the long URL.
// Use a temporary redirect to make sure browsers always hit the server for analytics purposes
return new RedirectResponse($longUrl);
} catch (\Exception $e) {
// In case of error, dispatch 404 error
$this->logger->error('Error redirecting to long URL.' . PHP_EOL . $e);
} catch (InvalidShortCodeException $e) {
return $delegate->process($request);
} catch (EntityDoesNotExistException $e) {
return $delegate->process($request);
}
}

View File

@@ -11,6 +11,7 @@ use GuzzleHttp\Exception\GuzzleException;
use Psr\Http\Message\UriInterface;
use Shlinkio\Shlink\Common\Exception\RuntimeException;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
use Shlinkio\Shlink\Core\Util\TagManagerTrait;
@@ -142,10 +143,11 @@ class UrlShortener implements UrlShortenerInterface
* Tries to find the mapped URL for provided short code. Returns null if not found
*
* @param string $shortCode
* @return string|null
* @return string
* @throws InvalidShortCodeException
* @throws EntityDoesNotExistException
*/
public function shortCodeToUrl($shortCode)
public function shortCodeToUrl($shortCode): string
{
$cacheKey = sprintf('%s_longUrl', $shortCode);
// Check if the short code => URL map is already cached
@@ -158,17 +160,16 @@ class UrlShortener implements UrlShortenerInterface
throw InvalidShortCodeException::fromCharset($shortCode, $this->chars);
}
/** @var ShortUrl $shortUrl */
$shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([
'shortCode' => $shortCode,
]);
// Cache the shortcode
if (isset($shortUrl)) {
$url = $shortUrl->getOriginalUrl();
$this->cache->save($cacheKey, $url);
return $url;
$criteria = ['shortCode' => $shortCode];
/** @var ShortUrl|null $shortUrl */
$shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy($criteria);
if ($shortUrl === null) {
throw EntityDoesNotExistException::createFromEntityAndConditions(ShortUrl::class, $criteria);
}
return null;
// Cache the shortcode
$url = $shortUrl->getOriginalUrl();
$this->cache->save($cacheKey, $url);
return $url;
}
}

View File

@@ -5,6 +5,7 @@ namespace Shlinkio\Shlink\Core\Service;
use Psr\Http\Message\UriInterface;
use Shlinkio\Shlink\Common\Exception\RuntimeException;
use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
@@ -25,8 +26,9 @@ interface UrlShortenerInterface
* Tries to find the mapped URL for provided short code. Returns null if not found
*
* @param string $shortCode
* @return string|null
* @return string
* @throws InvalidShortCodeException
* @throws EntityDoesNotExistException
*/
public function shortCodeToUrl($shortCode);
public function shortCodeToUrl($shortCode): string;
}

View File

@@ -9,6 +9,7 @@ use Prophecy\Argument;
use Prophecy\Prophecy\ObjectProphecy;
use Shlinkio\Shlink\Common\Service\PreviewGenerator;
use Shlinkio\Shlink\Core\Action\PreviewAction;
use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
use Shlinkio\Shlink\Core\Service\UrlShortener;
use ShlinkioTest\Shlink\Common\Util\TestUtils;
@@ -42,7 +43,8 @@ class PreviewActionTest extends TestCase
public function invalidShortCodeFallsBackToNextMiddleware()
{
$shortCode = 'abc123';
$this->urlShortener->shortCodeToUrl($shortCode)->willReturn(null)->shouldBeCalledTimes(1);
$this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class)
->shouldBeCalledTimes(1);
$delegate = $this->prophesize(DelegateInterface::class);
$delegate->process(Argument::cetera())->shouldBeCalledTimes(1);

View File

@@ -10,6 +10,7 @@ use Prophecy\Prophecy\ObjectProphecy;
use Shlinkio\Shlink\Common\Response\QrCodeResponse;
use Shlinkio\Shlink\Core\Action\QrCodeAction;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
use Shlinkio\Shlink\Core\Service\UrlShortener;
use Zend\Diactoros\ServerRequestFactory;
@@ -42,7 +43,8 @@ class QrCodeActionTest extends TestCase
public function aNotFoundShortCodeWillDelegateIntoNextMiddleware()
{
$shortCode = 'abc123';
$this->urlShortener->shortCodeToUrl($shortCode)->willReturn(null)->shouldBeCalledTimes(1);
$this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class)
->shouldBeCalledTimes(1);
$delegate = $this->prophesize(DelegateInterface::class);
$this->action->process(
@@ -77,7 +79,7 @@ class QrCodeActionTest extends TestCase
public function aCorrectRequestReturnsTheQrCodeResponse()
{
$shortCode = 'abc123';
$this->urlShortener->shortCodeToUrl($shortCode)->willReturn(new ShortUrl())->shouldBeCalledTimes(1);
$this->urlShortener->shortCodeToUrl($shortCode)->willReturn('')->shouldBeCalledTimes(1);
$delegate = $this->prophesize(DelegateInterface::class);
$resp = $this->action->process(

View File

@@ -8,6 +8,7 @@ use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\Prophecy\ObjectProphecy;
use Shlinkio\Shlink\Core\Action\RedirectAction;
use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
use Shlinkio\Shlink\Core\Service\UrlShortener;
use Shlinkio\Shlink\Core\Service\VisitsTracker;
use ShlinkioTest\Shlink\Common\Util\TestUtils;
@@ -58,23 +59,7 @@ class RedirectActionTest extends TestCase
public function nextMiddlewareIsInvokedIfLongUrlIsNotFound()
{
$shortCode = 'abc123';
$this->urlShortener->shortCodeToUrl($shortCode)->willReturn(null)
->shouldBeCalledTimes(1);
$delegate = $this->prophesize(DelegateInterface::class);
$request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode);
$this->action->process($request, $delegate->reveal());
$delegate->process($request)->shouldHaveBeenCalledTimes(1);
}
/**
* @test
*/
public function nextMiddlewareIsInvokedIfAnExceptionIsThrown()
{
$shortCode = 'abc123';
$this->urlShortener->shortCodeToUrl($shortCode)->willThrow(\Exception::class)
$this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class)
->shouldBeCalledTimes(1);
$delegate = $this->prophesize(DelegateInterface::class);