From c96fdb353b33234fa9e3d681ebda3365803afcb6 Mon Sep 17 00:00:00 2001
From: diosmosis <benaka@piwik.pro>
Date: Thu, 9 Jul 2015 17:52:45 -0700
Subject: [PATCH] Remove Visitor object from RequestProcessor::processRequest()
 method.

---
 core/Tracker/GoalManager.php                  | 46 ++++++++++++-------
 core/Tracker/RequestProcessor.php             |  2 +-
 core/Tracker/Visit.php                        |  5 +-
 core/Tracker/Visitor.php                      |  6 +++
 .../Tracker/ActionsRequestProcessor.php       |  3 +-
 .../Goals/Tracker/GoalsRequestProcessor.php   | 12 +----
 6 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/core/Tracker/GoalManager.php b/core/Tracker/GoalManager.php
index 87fb80ecc4..c7cde6a622 100644
--- a/core/Tracker/GoalManager.php
+++ b/core/Tracker/GoalManager.php
@@ -16,6 +16,7 @@ use Piwik\Plugin\Dimension\ConversionDimension;
 use Piwik\Plugin\Dimension\VisitDimension;
 use Piwik\Plugins\CustomVariables\CustomVariables;
 use Piwik\Tracker;
+use Piwik\Tracker\Visit\VisitProperties;
 
 /**
  */
@@ -236,9 +237,15 @@ class GoalManager
      * @param array $visitCustomVariables
      * @param Action $action
      */
-    public function recordGoals(Visitor $visitor, $visitorInformation, $visitCustomVariables, $action)
+    public function recordGoals(VisitProperties $visitProperties, Request $request)
     {
-        $goal = $this->getGoalFromVisitor($visitor, $visitorInformation, $action);
+        $visitorInformation = $visitProperties->visitorInfo;
+        $visitCustomVariables = $visitProperties->getRequestMetadata('CustomVariables', 'visitCustomVariables');
+
+        /** @var Action $action */
+        $action = $visitProperties->getRequestMetadata('Actions', 'action');
+
+        $goal = $this->getGoalFromVisitor($visitProperties, $request, $action);
 
         // Copy Custom Variables from Visit row to the Goal conversion
         // Otherwise, set the Custom Variables found in the cookie sent with this request
@@ -260,9 +267,9 @@ class GoalManager
 
         // some goals are converted, so must be ecommerce Order or Cart Update
         if ($this->requestIsEcommerce) {
-            $this->recordEcommerceGoal($goal, $visitor, $action, $visitorInformation);
+            $this->recordEcommerceGoal($visitProperties, $request, $goal, $action);
         } else {
-            $this->recordStandardGoals($goal, $visitor, $action, $visitorInformation);
+            $this->recordStandardGoals($visitProperties, $request, $goal, $action);
         }
     }
 
@@ -292,12 +299,14 @@ class GoalManager
      * @param Action $action
      * @param array $visitInformation
      */
-    protected function recordEcommerceGoal($conversion, Visitor $visitor, $action, $visitInformation)
+    protected function recordEcommerceGoal(VisitProperties $visitProperties, Request $request, $conversion, $action)
     {
         if ($this->isThereExistingCartInVisit) {
             Common::printDebug("There is an existing cart for this visit");
         }
 
+        $visitor = Visitor::makeFromVisitProperties($visitProperties, $request);
+
         if ($this->isGoalAnOrder) {
             $debugMessage = 'The conversion is an Ecommerce order';
 
@@ -335,13 +344,13 @@ class GoalManager
         $conversion['items'] = $itemsCount;
 
         if ($this->isThereExistingCartInVisit) {
-            $recorded = $this->getModel()->updateConversion($visitInformation['idvisit'], self::IDGOAL_CART, $conversion);
+            $recorded = $this->getModel()->updateConversion($visitProperties->visitorInfo['idvisit'], self::IDGOAL_CART, $conversion);
         } else {
-            $recorded = $this->insertNewConversion($conversion, $visitInformation);
+            $recorded = $this->insertNewConversion($conversion, $visitProperties->visitorInfo);
         }
 
         if ($recorded) {
-            $this->recordEcommerceItems($conversion, $items, $visitInformation);
+            $this->recordEcommerceItems($conversion, $items);
         }
 
         /**
@@ -355,7 +364,7 @@ class GoalManager
          * @param array $visitInformation The visit entity that we are tracking a conversion for. See what
          *                                information it contains [here](/guides/persistence-and-the-mysql-backend#visits).
          */
-        Piwik::postEvent('Tracker.recordEcommerceGoal', array($conversion, $visitInformation));
+        Piwik::postEvent('Tracker.recordEcommerceGoal', array($conversion, $visitProperties->visitorInfo));
     }
 
     /**
@@ -665,8 +674,10 @@ class GoalManager
      * @param Action $action
      * @param $visitorInformation
      */
-    protected function recordStandardGoals($goal, Visitor $visitor, $action, $visitorInformation)
+    protected function recordStandardGoals(VisitProperties $visitProperties, Request $request, $goal, $action)
     {
+        $visitor = Visitor::makeFromVisitProperties($visitProperties, $request);
+
         foreach ($this->convertedGoals as $convertedGoal) {
             $this->currentGoal = $convertedGoal;
             Common::printDebug("- Goal " . $convertedGoal['idgoal'] . " matched. Recording...");
@@ -682,12 +693,12 @@ class GoalManager
             // If multiple Goal conversions per visit, set a cache buster
             $conversion['buster'] = $convertedGoal['allow_multiple'] == 0
                 ? '0'
-                : $visitorInformation['visit_last_action_time'];
+                : $visitProperties->visitorInfo['visit_last_action_time'];
 
             $conversionDimensions = ConversionDimension::getAllDimensions();
             $conversion = $this->triggerHookOnDimensions($conversionDimensions, 'onGoalConversion', $visitor, $action, $conversion);
 
-            $this->insertNewConversion($conversion, $visitorInformation);
+            $this->insertNewConversion($conversion, $visitProperties->visitorInfo);
 
             /**
              * Triggered after successfully recording a non-ecommerce conversion.
@@ -805,18 +816,19 @@ class GoalManager
         return $valuesToUpdate;
     }
 
-    private function getGoalFromVisitor(Visitor $visitor, $visitorInformation, $action)
+    private function getGoalFromVisitor(VisitProperties $visitProperties, Request $request, $action)
     {
         $goal = array(
-            'idvisit'     => $visitorInformation['idvisit'],
-            'idvisitor'   => $visitorInformation['idvisitor'],
-            'server_time' => Date::getDatetimeFromTimestamp($visitorInformation['visit_last_action_time'])
+            'idvisit'     => $visitProperties->visitorInfo['idvisit'],
+            'idvisitor'   => $visitProperties->visitorInfo['idvisitor'],
+            'server_time' => Date::getDatetimeFromTimestamp($visitProperties->visitorInfo['visit_last_action_time'])
         );
 
         $visitDimensions = VisitDimension::getAllDimensions();
 
+        $visit = Visitor::makeFromVisitProperties($visitProperties, $request);
         foreach ($visitDimensions as $dimension) {
-            $value = $dimension->onAnyGoalConversion($this->request, $visitor, $action);
+            $value = $dimension->onAnyGoalConversion($this->request, $visit, $action);
             if (false !== $value) {
                 $goal[$dimension->getColumnName()] = $value;
             }
diff --git a/core/Tracker/RequestProcessor.php b/core/Tracker/RequestProcessor.php
index 3380dbe80d..f25b43f0d4 100644
--- a/core/Tracker/RequestProcessor.php
+++ b/core/Tracker/RequestProcessor.php
@@ -52,7 +52,7 @@ abstract class RequestProcessor
     /**
      * TODO
      */
-    public function processRequest(Visitor $visitor, VisitProperties $visitProperties)
+    public function processRequest(VisitProperties $visitProperties, Request $request)
     {
         // empty
     }
diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php
index 4d09a2615e..2a6ac0cbf8 100644
--- a/core/Tracker/Visit.php
+++ b/core/Tracker/Visit.php
@@ -160,7 +160,7 @@ class Visit implements VisitInterface
         $this->request->setThirdPartyCookie($this->visitProperties->visitorInfo['idvisitor']);
 
         foreach ($this->requestProcessors as $processor) {
-            $processor->processRequest($this->makeVisitorFacade(), $this->visitProperties);
+            $processor->processRequest($this->visitProperties, $this->request);
         }
 
         $this->markArchivedReportsAsInvalidIfArchiveAlreadyFinished();
@@ -568,7 +568,6 @@ class Visit implements VisitInterface
 
     private function makeVisitorFacade()
     {
-        $isKnown = $this->visitProperties->getRequestMetadata('CoreHome', 'isVisitorKnown');
-        return new Visitor($this->request, $this->visitProperties, $isKnown);
+        return Visitor::makeFromVisitProperties($this->visitProperties, $this->request);
     }
 }
diff --git a/core/Tracker/Visitor.php b/core/Tracker/Visitor.php
index 95d75ed6eb..efa88a8d96 100644
--- a/core/Tracker/Visitor.php
+++ b/core/Tracker/Visitor.php
@@ -27,6 +27,12 @@ class Visitor
         $this->setIsVisitorKnown($isVisitorKnown);
     }
 
+    public static function makeFromVisitProperties(VisitProperties $visitProperties, Request $request)
+    {
+        $isKnown = $visitProperties->getRequestMetadata('CoreHome', 'isVisitorKnown');
+        return new Visitor($request, $visitProperties, $isKnown);
+    }
+
     public function setVisitorColumn($column, $value) // TODO: remove this eventually
     {
         $this->visitProperties->visitorInfo[$column] = $value;
diff --git a/plugins/Actions/Tracker/ActionsRequestProcessor.php b/plugins/Actions/Tracker/ActionsRequestProcessor.php
index b127c68653..225f09499a 100644
--- a/plugins/Actions/Tracker/ActionsRequestProcessor.php
+++ b/plugins/Actions/Tracker/ActionsRequestProcessor.php
@@ -45,7 +45,7 @@ class ActionsRequestProcessor extends RequestProcessor
         $visitProperties->setRequestMetadata('Actions', 'idReferrerActionName', @$visitProperties->visitorInfo['visit_exit_idaction_name']);
     }
 
-    public function processRequest(Visitor $visitor, VisitProperties $visitProperties)
+    public function processRequest(VisitProperties $visitProperties, Request $request)
     {
         /** @var Action $action */
         $action = $visitProperties->getRequestMetadata('Actions', 'action');
@@ -61,6 +61,7 @@ class ActionsRequestProcessor extends RequestProcessor
                 $idReferrerActionName = $visitProperties->getRequestMetadata('Actions', 'idReferrerActionName');
             }
 
+            $visitor = Visitor::makeFromVisitProperties($visitProperties, $request);
             $action->record($visitor, $idReferrerActionUrl, $idReferrerActionName);
         }
     }
diff --git a/plugins/Goals/Tracker/GoalsRequestProcessor.php b/plugins/Goals/Tracker/GoalsRequestProcessor.php
index 9aba1375a7..5336f4fbb9 100644
--- a/plugins/Goals/Tracker/GoalsRequestProcessor.php
+++ b/plugins/Goals/Tracker/GoalsRequestProcessor.php
@@ -77,7 +77,7 @@ class GoalsRequestProcessor extends RequestProcessor
         }
     }
 
-    public function processRequest(Visitor $visitor, VisitProperties $visitProperties)
+    public function processRequest(VisitProperties $visitProperties, Request $request)
     {
         $isManualGoalConversion = self::$goalManager->isManualGoalConversion();
         $requestIsEcommerce = self::$goalManager->requestIsEcommerce;
@@ -100,15 +100,7 @@ class GoalsRequestProcessor extends RequestProcessor
 
         // record the goals if there were conversions in this request (even if the visit itself was not converted)
         if ($visitProperties->getRequestMetadata('Goals', 'someGoalsConverted')) {
-            /** @var Action $action */
-            $action = $visitProperties->getRequestMetadata('Actions', 'action');
-
-            self::$goalManager->recordGoals(
-                $visitor,
-                $visitProperties->visitorInfo,
-                $visitProperties->getRequestMetadata('CustomVariables', 'visitCustomVariables'),
-                $action
-            );
+            self::$goalManager->recordGoals($visitProperties, $request);
         }
     }
 }
-- 
GitLab