From 44f0011445c6f9c66cf08739fbcc3902df1ff13f Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Thu, 18 Oct 2018 20:24:25 +0200
Subject: [PATCH] Moved logic to create a visitor from a request to the visitor
 itself

---
 .../src/Middleware/IpAddressMiddlewareFactory.php   |  5 ++---
 .../Middleware/IpAddressMiddlewareFactoryTest.php   |  3 ++-
 module/Core/src/Action/AbstractTrackingAction.php   |  7 +------
 module/Core/src/Model/Visitor.php                   | 13 +++++++++++++
 module/Core/test/Service/VisitsTrackerTest.php      |  1 -
 5 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/module/Common/src/Middleware/IpAddressMiddlewareFactory.php b/module/Common/src/Middleware/IpAddressMiddlewareFactory.php
index a8689e55..b9cd3879 100644
--- a/module/Common/src/Middleware/IpAddressMiddlewareFactory.php
+++ b/module/Common/src/Middleware/IpAddressMiddlewareFactory.php
@@ -5,14 +5,13 @@ namespace Shlinkio\Shlink\Common\Middleware;
 
 use Interop\Container\ContainerInterface;
 use RKA\Middleware\IpAddress;
+use Shlinkio\Shlink\Core\Model\Visitor;
 use Zend\ServiceManager\Exception\ServiceNotCreatedException;
 use Zend\ServiceManager\Exception\ServiceNotFoundException;
 use Zend\ServiceManager\Factory\FactoryInterface;
 
 class IpAddressMiddlewareFactory implements FactoryInterface
 {
-    public const REMOTE_ADDRESS = 'remote_address';
-
     /**
      * Create an object
      *
@@ -24,6 +23,6 @@ class IpAddressMiddlewareFactory implements FactoryInterface
      */
     public function __invoke(ContainerInterface $container, $requestedName, array $options = null): IpAddress
     {
-        return new IpAddress(true, [], self::REMOTE_ADDRESS);
+        return new IpAddress(true, [], Visitor::REMOTE_ADDRESS_ATTR);
     }
 }
diff --git a/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php b/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php
index 2087e5c3..4c744b7c 100644
--- a/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php
+++ b/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php
@@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Common\Middleware;
 use PHPUnit\Framework\TestCase;
 use ReflectionObject;
 use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory;
+use Shlinkio\Shlink\Core\Model\Visitor;
 use Zend\ServiceManager\ServiceManager;
 
 class IpAddressMiddlewareFactoryTest extends TestCase
@@ -34,6 +35,6 @@ class IpAddressMiddlewareFactoryTest extends TestCase
 
         $this->assertTrue($checkProxyHeaders->getValue($instance));
         $this->assertEquals([], $trustedProxies->getValue($instance));
-        $this->assertEquals(IpAddressMiddlewareFactory::REMOTE_ADDRESS, $attributeName->getValue($instance));
+        $this->assertEquals(Visitor::REMOTE_ADDRESS_ATTR, $attributeName->getValue($instance));
     }
 }
diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php
index 778396d9..0d118565 100644
--- a/module/Core/src/Action/AbstractTrackingAction.php
+++ b/module/Core/src/Action/AbstractTrackingAction.php
@@ -9,7 +9,6 @@ use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
-use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory;
 use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait;
 use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
 use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
@@ -71,11 +70,7 @@ abstract class AbstractTrackingAction implements MiddlewareInterface
 
             // Track visit to this short code
             if ($disableTrackParam === null || ! \array_key_exists($disableTrackParam, $query)) {
-                $this->visitTracker->track($shortCode, new Visitor(
-                    $request->getHeaderLine('User-Agent'),
-                    $request->getHeaderLine('Referer'),
-                    $request->getAttribute(IpAddressMiddlewareFactory::REMOTE_ADDRESS)
-                ));
+                $this->visitTracker->track($shortCode, Visitor::fromRequest($request));
             }
 
             return $this->createResp($url->getLongUrl());
diff --git a/module/Core/src/Model/Visitor.php b/module/Core/src/Model/Visitor.php
index c6a828f0..82cb3cae 100644
--- a/module/Core/src/Model/Visitor.php
+++ b/module/Core/src/Model/Visitor.php
@@ -3,8 +3,12 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core\Model;
 
+use Psr\Http\Message\ServerRequestInterface;
+
 final class Visitor
 {
+    public const REMOTE_ADDRESS_ATTR = 'remote_address';
+
     /**
      * @var string
      */
@@ -25,6 +29,15 @@ final class Visitor
         $this->remoteAddress = $remoteAddress;
     }
 
+    public static function fromRequest(ServerRequestInterface $request): self
+    {
+        return new self(
+            $request->getHeaderLine('User-Agent'),
+            $request->getHeaderLine('Referer'),
+            $request->getAttribute(self::REMOTE_ADDRESS_ATTR)
+        );
+    }
+
     public static function emptyInstance(): self
     {
         return new self('', '', null);
diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php
index 154d6367..fd794245 100644
--- a/module/Core/test/Service/VisitsTrackerTest.php
+++ b/module/Core/test/Service/VisitsTrackerTest.php
@@ -13,7 +13,6 @@ use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Repository\VisitRepository;
 use Shlinkio\Shlink\Core\Service\VisitsTracker;
-use Zend\Diactoros\ServerRequestFactory;
 
 class VisitsTrackerTest extends TestCase
 {