diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index 9f8cc729..b83ee2e7 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -64,6 +64,8 @@ return [ ], 'not-found' => [ 'middleware' => [ + Core\ErrorHandler\NotFoundTypeResolverMiddleware::class, + Core\ErrorHandler\NotFoundTrackerMiddleware::class, Core\ErrorHandler\NotFoundRedirectHandler::class, Core\ErrorHandler\NotFoundTemplateHandler::class, ], diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 0f182b1b..50669f66 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -15,6 +15,8 @@ return [ 'dependencies' => [ 'factories' => [ + ErrorHandler\NotFoundTypeResolverMiddleware::class => ConfigAbstractFactory::class, + ErrorHandler\NotFoundTrackerMiddleware::class => ConfigAbstractFactory::class, ErrorHandler\NotFoundRedirectHandler::class => ConfigAbstractFactory::class, ErrorHandler\NotFoundTemplateHandler::class => InvokableFactory::class, @@ -58,10 +60,11 @@ return [ ], ConfigAbstractFactory::class => [ + ErrorHandler\NotFoundTypeResolverMiddleware::class => ['config.router.base_path'], + ErrorHandler\NotFoundTrackerMiddleware::class => [Visit\VisitsTracker::class], ErrorHandler\NotFoundRedirectHandler::class => [ NotFoundRedirectOptions::class, Util\RedirectResponseHelper::class, - 'config.router.base_path', ], Options\AppOptions::class => ['config.app_options'], diff --git a/module/Core/src/ErrorHandler/Model/NotFoundType.php b/module/Core/src/ErrorHandler/Model/NotFoundType.php new file mode 100644 index 00000000..7585a3ca --- /dev/null +++ b/module/Core/src/ErrorHandler/Model/NotFoundType.php @@ -0,0 +1,57 @@ +type = $type; + } + + public static function fromRequest(ServerRequestInterface $request, string $basePath): self + { + $isBaseUrl = rtrim($request->getUri()->getPath(), '/') === $basePath; + if ($isBaseUrl) { + return new self(Visit::TYPE_BASE_URL); + } + + /** @var RouteResult $routeResult */ + $routeResult = $request->getAttribute(RouteResult::class, RouteResult::fromRouteFailure(null)); + if ($routeResult->isFailure()) { + return new self(Visit::TYPE_REGULAR_404); + } + + if ($routeResult->getMatchedRouteName() === RedirectAction::class) { + return new self(Visit::TYPE_INVALID_SHORT_URL); + } + + return new self(self::class); + } + + public function isBaseUrl(): bool + { + return $this->type === Visit::TYPE_BASE_URL; + } + + public function isRegularNotFound(): bool + { + return $this->type === Visit::TYPE_REGULAR_404; + } + + public function isInvalidShortUrl(): bool + { + return $this->type === Visit::TYPE_INVALID_SHORT_URL; + } +} diff --git a/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php index a49db5bb..1f3b4fed 100644 --- a/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php +++ b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php @@ -4,67 +4,48 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ErrorHandler; -use Mezzio\Router\RouteResult; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Psr\Http\Message\UriInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; -use Shlinkio\Shlink\Core\Action\RedirectAction; +use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Options; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; -use function rtrim; - class NotFoundRedirectHandler implements MiddlewareInterface { private Options\NotFoundRedirectOptions $redirectOptions; private RedirectResponseHelperInterface $redirectResponseHelper; - private string $shlinkBasePath; public function __construct( Options\NotFoundRedirectOptions $redirectOptions, - RedirectResponseHelperInterface $redirectResponseHelper, - string $shlinkBasePath + RedirectResponseHelperInterface $redirectResponseHelper ) { $this->redirectOptions = $redirectOptions; - $this->shlinkBasePath = $shlinkBasePath; $this->redirectResponseHelper = $redirectResponseHelper; } public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { - /** @var RouteResult $routeResult */ - $routeResult = $request->getAttribute(RouteResult::class, RouteResult::fromRouteFailure(null)); - $redirectResponse = $this->createRedirectResponse($routeResult, $request->getUri()); + /** @var NotFoundType $notFoundType */ + $notFoundType = $request->getAttribute(NotFoundType::class); - return $redirectResponse ?? $handler->handle($request); - } - - private function createRedirectResponse(RouteResult $routeResult, UriInterface $uri): ?ResponseInterface - { - $isBaseUrl = rtrim($uri->getPath(), '/') === $this->shlinkBasePath; - - if ($isBaseUrl && $this->redirectOptions->hasBaseUrlRedirect()) { + if ($notFoundType->isBaseUrl() && $this->redirectOptions->hasBaseUrlRedirect()) { return $this->redirectResponseHelper->buildRedirectResponse($this->redirectOptions->getBaseUrlRedirect()); } - if (!$isBaseUrl && $routeResult->isFailure() && $this->redirectOptions->hasRegular404Redirect()) { + if ($notFoundType->isRegularNotFound() && $this->redirectOptions->hasRegular404Redirect()) { return $this->redirectResponseHelper->buildRedirectResponse( $this->redirectOptions->getRegular404Redirect(), ); } - if ( - $routeResult->isSuccess() && - $routeResult->getMatchedRouteName() === RedirectAction::class && - $this->redirectOptions->hasInvalidShortUrlRedirect() - ) { + if ($notFoundType->isInvalidShortUrl() && $this->redirectOptions->hasInvalidShortUrlRedirect()) { return $this->redirectResponseHelper->buildRedirectResponse( $this->redirectOptions->getInvalidShortUrlRedirect(), ); } - return null; + return $handler->handle($request); } } diff --git a/module/Core/src/ErrorHandler/NotFoundTrackerMiddleware.php b/module/Core/src/ErrorHandler/NotFoundTrackerMiddleware.php new file mode 100644 index 00000000..f792dd07 --- /dev/null +++ b/module/Core/src/ErrorHandler/NotFoundTrackerMiddleware.php @@ -0,0 +1,44 @@ +visitsTracker = $visitsTracker; + } + + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + /** @var NotFoundType $notFoundType */ + $notFoundType = $request->getAttribute(NotFoundType::class); + $visitor = Visitor::fromRequest($request); + + if ($notFoundType->isBaseUrl()) { + $this->visitsTracker->trackBaseUrlVisit($visitor); + } + + if ($notFoundType->isRegularNotFound()) { + $this->visitsTracker->trackRegularNotFoundVisit($visitor); + } + + if ($notFoundType->isInvalidShortUrl()) { + $this->visitsTracker->trackInvalidShortUrlVisit($visitor); + } + + return $handler->handle($request); + } +} diff --git a/module/Core/src/ErrorHandler/NotFoundTypeResolverMiddleware.php b/module/Core/src/ErrorHandler/NotFoundTypeResolverMiddleware.php new file mode 100644 index 00000000..6f13db73 --- /dev/null +++ b/module/Core/src/ErrorHandler/NotFoundTypeResolverMiddleware.php @@ -0,0 +1,27 @@ +shlinkBasePath = $shlinkBasePath; + } + + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + $notFoundType = NotFoundType::fromRequest($request, $this->shlinkBasePath); + return $handler->handle($request->withAttribute(NotFoundType::class, $notFoundType)); + } +} diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index a1df73a5..61bc50bb 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -211,8 +211,9 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo public function countVisits(?ApiKey $apiKey = null): int { - return (int) $this->matchSingleScalarResult( - Spec::countOf(new WithApiKeySpecsEnsuringJoin($apiKey, 'shortUrl')), - ); + return (int) $this->matchSingleScalarResult(Spec::countOf(Spec::andX( + Spec::isNotNull('shortUrl'), + new WithApiKeySpecsEnsuringJoin($apiKey, 'shortUrl'), + ))); } } diff --git a/module/Core/src/Visit/VisitsTracker.php b/module/Core/src/Visit/VisitsTracker.php index e4a9f304..af170cfc 100644 --- a/module/Core/src/Visit/VisitsTracker.php +++ b/module/Core/src/Visit/VisitsTracker.php @@ -29,11 +29,30 @@ class VisitsTracker implements VisitsTrackerInterface public function track(ShortUrl $shortUrl, Visitor $visitor): void { - $visit = Visit::forValidShortUrl($shortUrl, $visitor, $this->anonymizeRemoteAddr); + $visit = $this->trackVisit(Visit::forValidShortUrl($shortUrl, $visitor, $this->anonymizeRemoteAddr)); + $this->eventDispatcher->dispatch(new ShortUrlVisited($visit->getId(), $visitor->getRemoteAddress())); + } + public function trackInvalidShortUrlVisit(Visitor $visitor): void + { + $this->trackVisit(Visit::forInvalidShortUrl($visitor)); + } + + public function trackBaseUrlVisit(Visitor $visitor): void + { + $this->trackVisit(Visit::forBasePath($visitor)); + } + + public function trackRegularNotFoundVisit(Visitor $visitor): void + { + $this->trackVisit(Visit::forRegularNotFound($visitor)); + } + + private function trackVisit(Visit $visit): Visit + { $this->em->persist($visit); $this->em->flush(); - $this->eventDispatcher->dispatch(new ShortUrlVisited($visit->getId(), $visitor->getRemoteAddress())); + return $visit; } } diff --git a/module/Core/src/Visit/VisitsTrackerInterface.php b/module/Core/src/Visit/VisitsTrackerInterface.php index 75f92434..ae70d550 100644 --- a/module/Core/src/Visit/VisitsTrackerInterface.php +++ b/module/Core/src/Visit/VisitsTrackerInterface.php @@ -10,4 +10,10 @@ use Shlinkio\Shlink\Core\Model\Visitor; interface VisitsTrackerInterface { public function track(ShortUrl $shortUrl, Visitor $visitor): void; + + public function trackInvalidShortUrlVisit(Visitor $visitor): void; + + public function trackBaseUrlVisit(Visitor $visitor): void; + + public function trackRegularNotFoundVisit(Visitor $visitor): void; } diff --git a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php index 83810d22..9df49879 100644 --- a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php +++ b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php @@ -17,6 +17,7 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; +use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\ErrorHandler\NotFoundRedirectHandler; use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; @@ -33,7 +34,7 @@ class NotFoundRedirectHandlerTest extends TestCase { $this->redirectOptions = new NotFoundRedirectOptions(); $this->helper = $this->prophesize(RedirectResponseHelperInterface::class); - $this->middleware = new NotFoundRedirectHandler($this->redirectOptions, $this->helper->reveal(), ''); + $this->middleware = new NotFoundRedirectHandler($this->redirectOptions, $this->helper->reveal()); } /** @@ -64,19 +65,19 @@ class NotFoundRedirectHandlerTest extends TestCase public function provideRedirects(): iterable { yield 'base URL with trailing slash' => [ - ServerRequestFactory::fromGlobals()->withUri(new Uri('/')), + $this->withNotFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri('/'))), 'baseUrl', ]; yield 'base URL without trailing slash' => [ - ServerRequestFactory::fromGlobals()->withUri(new Uri('')), + $this->withNotFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri(''))), 'baseUrl', ]; yield 'regular 404' => [ - ServerRequestFactory::fromGlobals()->withUri(new Uri('/foo/bar')), + $this->withNotFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri('/foo/bar'))), 'regular404', ]; yield 'invalid short URL' => [ - ServerRequestFactory::fromGlobals() + $this->withNotFoundType(ServerRequestFactory::fromGlobals() ->withAttribute( RouteResult::class, RouteResult::fromRoute( @@ -88,7 +89,7 @@ class NotFoundRedirectHandlerTest extends TestCase ), ), ) - ->withUri(new Uri('/abc123')), + ->withUri(new Uri('/abc123'))), 'invalidShortUrl', ]; } @@ -96,7 +97,7 @@ class NotFoundRedirectHandlerTest extends TestCase /** @test */ public function nextMiddlewareIsInvokedWhenNotRedirectNeedsToOccur(): void { - $req = ServerRequestFactory::fromGlobals(); + $req = $this->withNotFoundType(ServerRequestFactory::fromGlobals()); $resp = new Response(); $buildResp = $this->helper->buildRedirectResponse(Argument::cetera()); @@ -110,4 +111,10 @@ class NotFoundRedirectHandlerTest extends TestCase $buildResp->shouldNotHaveBeenCalled(); $handle->shouldHaveBeenCalledOnce(); } + + private function withNotFoundType(ServerRequestInterface $req): ServerRequestInterface + { + $type = NotFoundType::fromRequest($req, ''); + return $req->withAttribute(NotFoundType::class, $type); + } }