Move 'is new visit' detection to CoreHome RequestProcessor and out of core\Tracker\Visit.php.

3 files changed, 126 insertions, 95 deletions

diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php
index d8877aa637..4992e3cfcd 100644
--- a/core/Tracker/Visit.php
+++ b/core/Tracker/Visit.php
@@ -71,8 +71,6 @@ class Visit implements VisitInterface
      */
     private $visitorRecognizer;
 
-    private $visitorId = null;
-
     public function __construct()
     {
         $this->requestProcessors = StaticContainer::get('tracker.request.processors');
@@ -121,19 +119,17 @@ class Visit implements VisitInterface
             }
         }
 
-        /***
-         * Visitor recognition
-         */
-        $this->visitorId = $this->getSettingsObject()->getConfigId($this->request, $this->getVisitorIp());
-
-        $isKnown = $this->visitorRecognizer->findKnownVisitor($this->visitorId, $this->visitProperties, $this->request);
-
-        $visitor = new Visitor($this->request, $this->visitorId, $this->visitProperties, $isKnown);
+        $isKnown = $this->visitProperties->getRequestMetadata('CoreHome', 'isVisitorKnown');
+        $visitor = new Visitor($this->request, $this->visitProperties, $isKnown);
 
         /** @var Action $action */
         $action = $this->visitProperties->getRequestMetadata('Actions', 'action');
 
-        $isNewVisit = $this->isVisitNew($visitor, $action);
+        $isNewVisit = $this->visitProperties->getRequestMetadata('CoreHome', 'isNewVisit');
+        if (!$isNewVisit) {
+            $isNewVisit = $this->triggerPredicateHookOnDimensions($this->getAllVisitDimensions(), 'shouldForceNewVisit',
+                $visitor, $action);
+        }
 
         $goalManager = GoalsRequestProcessor::$goalManager;
 
@@ -369,46 +365,6 @@ class Visit implements VisitInterface
         return $this->userSettings;
     }
 
-    /**
-     * Returns true if the last action was done during the last 30 minutes
-     * @return bool
-     */
-    protected function isLastActionInTheSameVisit(Visitor $visitor)
-    {
-        $lastActionTime = $visitor->getVisitorColumn('visit_last_action_time');
-
-        return isset($lastActionTime)
-        && false !== $lastActionTime
-        && ($lastActionTime > ($this->request->getCurrentTimestamp() - Config::getInstance()->Tracker['visit_standard_length']));
-    }
-
-    /**
-     * Returns true if the last action was not today.
-     * @param Visitor $visitor
-     * @return bool
-     */
-    private function wasLastActionNotToday(Visitor $visitor)
-    {
-        $lastActionTime = $visitor->getVisitorColumn('visit_last_action_time');
-
-        if (empty($lastActionTime)) {
-            return false;
-        }
-
-        $idSite = $this->request->getIdSite();
-        $timezone = $this->getTimezoneForSite($idSite);
-
-        if (empty($timezone)) {
-            throw new UnexpectedWebsiteFoundException("An unexpected website was found: idsite was set to '$idSite'");
-        }
-
-        $date = Date::factory((int)$lastActionTime, $timezone);
-        $now = $this->request->getCurrentTimestamp();
-        $now = Date::factory((int)$now, $timezone);
-
-        return $date->toString() !== $now->toString();
-    }
-
     // is the referrer host any of the registered URLs for this website?
     public static function isHostKnownAliasHost($urlHost, $idSite)
     {
@@ -455,7 +411,7 @@ class Visit implements VisitInterface
             Common::printDebug('Updated existing visit: ' . var_export($valuesToUpdate, true));
         } else {
             throw new VisitorNotFoundInDb(
-                "The visitor with idvisitor=" . bin2hex($this->visitProperties->visitorInfo['idvisitor']) . " and idvisit=" . $this->visitProperties->visitorInfo['idvisit']
+                "The visitor with idvisitor=" . bin2hex($this->visitProperties->visitorInfo['idvisitor']) . " and idvisit=" . @$this->visitProperties->visitorInfo['idvisit']
                 . " wasn't found in the DB, we fallback to a new visitor");
         }
     }
@@ -479,7 +435,7 @@ class Visit implements VisitInterface
     {
         $idVisitor = $this->getVisitorIdcookie($visitor);
         $visitorIp = $this->getVisitorIp();
-        $configId = $this->visitorId;
+        $configId = $this->visitProperties->getRequestMetadata('CoreHome', 'visitorId');
 
         $this->visitProperties->visitorInfo = array();
 
@@ -610,44 +566,6 @@ class Visit implements VisitInterface
         return $this->getModel()->createVisit($visit);
     }
 
-    /**
-     * Determines if the tracker if the current action should be treated as the start of a new visit or
-     * an action in an existing visit.
-     *
-     * @param Visitor $visitor The current visit/visitor information.
-     * @param Action|null $action The current action being tracked.
-     * @return bool
-     */
-    public function isVisitNew(Visitor $visitor, Action $action = null)
-    {
-        if (!$visitor->isVisitorKnown()) {
-            return true;
-        }
-
-        $isLastActionInTheSameVisit = $this->isLastActionInTheSameVisit($visitor);
-
-        if (!$isLastActionInTheSameVisit) {
-            Common::printDebug("Visitor detected, but last action was more than 30 minutes ago...");
-
-            return true;
-        }
-
-        $wasLastActionYesterday = $this->wasLastActionNotToday($visitor);
-        if ($wasLastActionYesterday) {
-            Common::printDebug("Visitor detected, but last action was yesterday...");
-
-            return true;
-        }
-
-        $shouldForceNewVisit = $this->triggerPredicateHookOnDimensions($this->getAllVisitDimensions(),
-            'shouldForceNewVisit', $visitor, $action);
-        if ($shouldForceNewVisit) {
-            return true;
-        }
-
-        return false;
-    }
-
     private function markArchivedReportsAsInvalidIfArchiveAlreadyFinished()
     {
         $idSite = (int)$this->request->getIdSite();
diff --git a/core/Tracker/Visitor.php b/core/Tracker/Visitor.php
index 3db2a54568..95d75ed6eb 100644
--- a/core/Tracker/Visitor.php
+++ b/core/Tracker/Visitor.php
@@ -19,12 +19,10 @@ class Visitor
     private $visitorKnown = false;
     private $request;
     private $visitProperties;
-    private $configId;
 
-    public function __construct(Request $request, $configId, VisitProperties $visitProperties, $isVisitorKnown = false)
+    public function __construct(Request $request, VisitProperties $visitProperties, $isVisitorKnown = false)
     {
         $this->request = $request;
-        $this->configId = $configId;
         $this->visitProperties = $visitProperties;
         $this->setIsVisitorKnown($isVisitorKnown);
     }
diff --git a/plugins/CoreHome/Tracker/VisitRequestProcessor.php b/plugins/CoreHome/Tracker/VisitRequestProcessor.php
index 596d0b926f..f531015424 100644
--- a/plugins/CoreHome/Tracker/VisitRequestProcessor.php
+++ b/plugins/CoreHome/Tracker/VisitRequestProcessor.php
@@ -8,11 +8,17 @@
 
 namespace Piwik\Plugins\CoreHome\Tracker;
 
+use Piwik\Common;
+use Piwik\Date;
 use Piwik\EventDispatcher;
+use Piwik\Exception\UnexpectedWebsiteFoundException;
+use Piwik\Tracker\Cache;
 use Piwik\Tracker\Request;
 use Piwik\Tracker\RequestProcessor;
+use Piwik\Tracker\Settings;
 use Piwik\Tracker\Visit\VisitProperties;
 use Piwik\Tracker\VisitExcluded;
+use Piwik\Tracker\VisitorRecognizer;
 
 /**
  * Encapsulates core tracking logic related to visits.
@@ -24,9 +30,21 @@ class VisitRequestProcessor extends RequestProcessor
      */
     private $eventDispatcher;
 
-    public function __construct(EventDispatcher $eventDispatcher)
+    /**
+     * @var VisitorRecognizer
+     */
+    private $visitorRecognizer;
+
+    /**
+     * @var Settings
+     */
+    private $userSettings;
+
+    public function __construct(EventDispatcher $eventDispatcher, VisitorRecognizer $visitorRecognizer, Settings $userSettings)
     {
         $this->eventDispatcher = $eventDispatcher;
+        $this->visitorRecognizer = $visitorRecognizer;
+        $this->userSettings = $userSettings;
     }
 
     public function processRequestParams(VisitProperties $visitProperties, Request $request)
@@ -40,6 +58,16 @@ class VisitRequestProcessor extends RequestProcessor
             return true;
         }
 
+        // visitor recognition
+        $visitorId = $this->userSettings->getConfigId($request, $visitProperties->visitorInfo['location_ip']);
+        $visitProperties->setRequestMetadata('CoreHome', 'visitorId', $visitorId);
+
+        $isKnown = $this->visitorRecognizer->findKnownVisitor($visitorId, $visitProperties, $request);
+        $visitProperties->setRequestMetadata('CoreHome', 'isVisitorKnown', $isKnown);
+
+        $isNewVisit = $this->isVisitNew($visitProperties, $request);
+        $visitProperties->setRequestMetadata('CoreHome', 'isNewVisit', $isNewVisit);
+
         return false;
     }
 
@@ -55,4 +83,91 @@ class VisitRequestProcessor extends RequestProcessor
          */
         $this->eventDispatcher->postEvent('Tracker.setVisitorIp', array(&$visitProperties->visitorInfo['location_ip']));
     }
+
+    /**
+     * Determines if the tracker if the current action should be treated as the start of a new visit or
+     * an action in an existing visit.
+     *
+     * @param VisitProperties $visitProperties The current visit/visitor information.
+     * @param Request $request
+     * @return bool
+     */
+    private function isVisitNew(VisitProperties $visitProperties, Request $request)
+    {
+        $isKnown = $visitProperties->getRequestMetadata('CoreHome', 'isVisitorKnown');
+        if (!$isKnown) {
+            return true;
+        }
+
+        $isLastActionInTheSameVisit = $this->isLastActionInTheSameVisit($visitProperties, $request);
+        if (!$isLastActionInTheSameVisit) {
+            Common::printDebug("Visitor detected, but last action was more than 30 minutes ago...");
+
+            return true;
+        }
+
+        $wasLastActionYesterday = $this->wasLastActionNotToday($visitProperties, $request);
+        if ($wasLastActionYesterday) {
+            Common::printDebug("Visitor detected, but last action was yesterday...");
+
+            return true;
+        }
+
+        return false;
+    }
+
+    /**
+     * Returns true if the last action was done during the last 30 minutes
+     * @return bool
+     */
+    protected function isLastActionInTheSameVisit(VisitProperties $visitProperties, Request $request)
+    {
+        $lastActionTime = $visitProperties->visitorInfo['visit_last_action_time'];
+
+        return isset($lastActionTime)
+            && false !== $lastActionTime
+            && ($lastActionTime > ($request->getCurrentTimestamp() - \Piwik\Config::getInstance()->Tracker['visit_standard_length'])); // TODO: move to DI
+    }
+
+    /**
+     * Returns true if the last action was not today.
+     * @param VisitProperties $visitor
+     * @return bool
+     */
+    private function wasLastActionNotToday(VisitProperties $visitProperties, Request $request)
+    {
+        $lastActionTime = $visitProperties->visitorInfo['visit_last_action_time'];
+
+        if (empty($lastActionTime)) {
+            return false;
+        }
+
+        $idSite = $request->getIdSite();
+        $timezone = $this->getTimezoneForSite($idSite);
+
+        if (empty($timezone)) {
+            throw new UnexpectedWebsiteFoundException('An unexpected website was found, check idSite in the request');
+        }
+
+        $date = Date::factory((int)$lastActionTime, $timezone);
+        $now = $request->getCurrentTimestamp();
+        $now = Date::factory((int)$now, $timezone);
+
+        return $date->toString() !== $now->toString();
+    }
+
+    private function getTimezoneForSite($idSite) // TODO: duplicate function in Visit
+    {
+        try {
+            $site = Cache::getCacheWebsiteAttributes($idSite);
+        } catch (UnexpectedWebsiteFoundException $e) {
+            return null;
+        }
+
+        if (!empty($site['timezone'])) {
+            return $site['timezone'];
+        }
+
+        return null;
+    }
 }