Merge pull request #9363 from piwik/9299

Fix new visit created when no referrer keyword is set + fix referrer problems with bulk tracking

11 files changed, 222 insertions, 48 deletions

diff --git a/core/Tracker/Visitor.php b/core/Tracker/Visitor.php
index 2619c98987..40e027b26f 100644
--- a/core/Tracker/Visitor.php
+++ b/core/Tracker/Visitor.php
@@ -9,7 +9,6 @@
 namespace Piwik\Tracker;
 
 use Piwik\Config;
-use Piwik\Plugin\Dimension\VisitDimension;
 use Piwik\Tracker;
 use Piwik\Tracker\Visit\VisitProperties;
 
@@ -49,6 +48,11 @@ class Visitor
         return $this->visitorKnown === true;
     }
 
+    public function isNewVisit()
+    {
+        return !$this->isVisitorKnown();
+    }
+
     private function setIsVisitorKnown($isVisitorKnown)
     {
         return $this->visitorKnown = $isVisitorKnown;
diff --git a/plugins/Referrers/Columns/Base.php b/plugins/Referrers/Columns/Base.php
index a4da79d389..655ea20449 100644
--- a/plugins/Referrers/Columns/Base.php
+++ b/plugins/Referrers/Columns/Base.php
@@ -34,12 +34,12 @@ abstract class Base extends VisitDimension
     protected $currentUrlParse;
     protected $idsite;
 
+    private static $cachedReferrerSearchEngine = array();
+
     // Used to prefix when a adsense referrer is detected
     const LABEL_PREFIX_ADWORDS_KEYWORD = '(adwords) ';
     const LABEL_ADWORDS_NAME = 'AdWords';
 
-    private static $cachedReferrer = array();
-
     /**
      * Returns an array containing the following information:
      * - referer_type
@@ -69,14 +69,8 @@ abstract class Base extends VisitDimension
      * @param int $idSite
      * @return array
      */
-    protected function getReferrerInformation($referrerUrl, $currentUrl, $idSite, Request $request)
+    protected function getReferrerInformation($referrerUrl, $currentUrl, $idSite, Request $request, Visitor $visitor)
     {
-        $cacheKey = $referrerUrl . $currentUrl . $idSite;
-
-        if (isset(self::$cachedReferrer[$cacheKey])) {
-            return self::$cachedReferrer[$cacheKey];
-        }
-
         $this->idsite = $idSite;
 
         // default values for the referer_* fields
@@ -101,7 +95,7 @@ abstract class Base extends VisitDimension
             $this->referrerHost = $this->referrerUrlParse['host'];
         }
 
-        $referrerDetected = $this->detectReferrerCampaign($request);
+        $referrerDetected = $this->detectReferrerCampaign($request, $visitor);
 
         if (!$referrerDetected) {
             if ($this->detectReferrerDirectEntry()
@@ -131,17 +125,15 @@ abstract class Base extends VisitDimension
             'referer_url'     => $this->referrerUrl,
         );
 
-        self::$cachedReferrer[$cacheKey] = $referrerInformation;
-
         return $referrerInformation;
     }
 
-    protected function getReferrerInformationFromRequest(Request $request)
+    protected function getReferrerInformationFromRequest(Request $request, Visitor $visitor)
     {
         $referrerUrl = $request->getParam('urlref');
         $currentUrl  = $request->getParam('url');
 
-        return $this->getReferrerInformation($referrerUrl, $currentUrl, $request->getIdSite(), $request);
+        return $this->getReferrerInformation($referrerUrl, $currentUrl, $request->getIdSite(), $request, $visitor);
     }
 
     /**
@@ -150,28 +142,36 @@ abstract class Base extends VisitDimension
      */
     protected function detectReferrerSearchEngine()
     {
-        $searchEngineInformation = SearchEngineDetection::getInstance()->extractInformationFromUrl($this->referrerUrl);
-
-        /**
-         * Triggered when detecting the search engine of a referrer URL.
-         *
-         * Plugins can use this event to provide custom search engine detection
-         * logic.
-         *
-         * @param array &$searchEngineInformation An array with the following information:
-         *
-         *                                        - **name**: The search engine name.
-         *                                        - **keywords**: The search keywords used.
-         *
-         *                                        This parameter is initialized to the results
-         *                                        of Piwik's default search engine detection
-         *                                        logic.
-         * @param string referrerUrl The referrer URL from the tracking request.
-         */
-        Piwik::postEvent('Tracker.detectReferrerSearchEngine', array(&$searchEngineInformation, $this->referrerUrl));
+        if (isset(self::$cachedReferrerSearchEngine[$this->referrerUrl])) {
+            $searchEngineInformation = self::$cachedReferrerSearchEngine[$this->referrerUrl];
+        } else {
+            $searchEngineInformation = SearchEngineDetection::getInstance()->extractInformationFromUrl($this->referrerUrl);
+
+            /**
+             * Triggered when detecting the search engine of a referrer URL.
+             *
+             * Plugins can use this event to provide custom search engine detection
+             * logic.
+             *
+             * @param array &$searchEngineInformation An array with the following information:
+             *
+             *                                        - **name**: The search engine name.
+             *                                        - **keywords**: The search keywords used.
+             *
+             *                                        This parameter is initialized to the results
+             *                                        of Piwik's default search engine detection
+             *                                        logic.
+             * @param string referrerUrl The referrer URL from the tracking request.
+             */
+            Piwik::postEvent('Tracker.detectReferrerSearchEngine', array(&$searchEngineInformation, $this->referrerUrl));
+
+            self::$cachedReferrerSearchEngine[$this->referrerUrl] = $searchEngineInformation;
+        }
+
         if ($searchEngineInformation === false) {
             return false;
         }
+
         $this->typeReferrerAnalyzed = Common::REFERRER_TYPE_SEARCH_ENGINE;
         $this->nameReferrerAnalyzed = $searchEngineInformation['name'];
         $this->keywordReferrerAnalyzed = $searchEngineInformation['keywords'];
@@ -354,7 +354,7 @@ abstract class Base extends VisitDimension
     /**
      * @return bool
      */
-    protected function detectReferrerCampaign(Request $request)
+    protected function detectReferrerCampaign(Request $request, Visitor $visitor)
     {
         $isCampaign = $this->detectReferrerCampaignFromTrackerParams($request);
         if (!$isCampaign) {
@@ -363,12 +363,30 @@ abstract class Base extends VisitDimension
 
         $this->detectCampaignKeywordFromReferrerUrl();
 
-        if ($this->typeReferrerAnalyzed != Common::REFERRER_TYPE_CAMPAIGN) {
-            return false;
-        }
+        $isCurrentVisitACampaignWithSameName = $visitor->getVisitorColumn('referer_name') == $this->nameReferrerAnalyzed;
+        $isCurrentVisitACampaignWithSameName = $isCurrentVisitACampaignWithSameName && $visitor->getVisitorColumn('referer_type') == Common::REFERRER_TYPE_CAMPAIGN;
+
         // if we detected a campaign but there is still no keyword set, we set the keyword to the Referrer host
         if (empty($this->keywordReferrerAnalyzed)) {
-            $this->keywordReferrerAnalyzed = $this->referrerHost;
+            if ($isCurrentVisitACampaignWithSameName) {
+                $this->keywordReferrerAnalyzed = $visitor->getVisitorColumn('referer_keyword');
+                // it is an existing visit and no referrer keyword was used initially (or a different host),
+                // we do not use the default referrer host in this case as it would create a new visit. It would create
+                // a new visit because initially the referrer keyword was not set (or from a different host) and now
+                // we would set it suddenly. The changed keyword would be recognized as a campaign change and a new
+                // visit would be forced. Why would it suddenly set a keyword but not do it initially?
+                // This happens when on the first visit when the URL was opened directly (no referrer or different host)
+                // and then the user navigates to another page where the referrer host becomes the own host
+                // (referrer = own website) see https://github.com/piwik/piwik/issues/9299
+            } else {
+                $this->keywordReferrerAnalyzed = $this->referrerHost;
+            }
+        }
+
+        if ($this->typeReferrerAnalyzed != Common::REFERRER_TYPE_CAMPAIGN) {
+            $this->keywordReferrerAnalyzed = null;
+            $this->nameReferrerAnalyzed = null;
+            return false;
         }
 
         $this->keywordReferrerAnalyzed = Common::mb_strtolower($this->keywordReferrerAnalyzed);
@@ -384,7 +402,6 @@ abstract class Base extends VisitDimension
      */
     public function getValueForRecordGoal(Request $request, Visitor $visitor)
     {
-        $referrerTimestamp       = $request->getParam('_refts');
         $referrerUrl             = $request->getParam('_ref');
         $referrerCampaignName    = $this->getReferrerCampaignQueryParam($request, '_rcn');
         $referrerCampaignKeyword = $this->getReferrerCampaignQueryParam($request, '_rck');
@@ -422,7 +439,7 @@ abstract class Base extends VisitDimension
         elseif (!empty($referrerUrl)) {
 
             $idSite   = $request->getIdSite();
-            $referrer = $this->getReferrerInformation($referrerUrl, $currentUrl = '', $idSite, $request);
+            $referrer = $this->getReferrerInformation($referrerUrl, $currentUrl = '', $idSite, $request, $visitor);
 
             // if the parsed referrer is interesting enough, ie. website or search engine
             if (in_array($referrer['referer_type'], array(Common::REFERRER_TYPE_SEARCH_ENGINE, Common::REFERRER_TYPE_WEBSITE))) {
diff --git a/plugins/Referrers/Columns/Campaign.php b/plugins/Referrers/Columns/Campaign.php
index c29b622336..be42acb34b 100644
--- a/plugins/Referrers/Columns/Campaign.php
+++ b/plugins/Referrers/Columns/Campaign.php
@@ -50,7 +50,7 @@ class Campaign extends Base
             return false;
         }
 
-        $information = $this->getReferrerInformationFromRequest($request);
+        $information = $this->getReferrerInformationFromRequest($request, $visitor);
 
         if ($information['referer_type'] == Common::REFERRER_TYPE_CAMPAIGN
             && $this->isReferrerInformationNew($visitor, $information)
diff --git a/plugins/Referrers/Columns/Keyword.php b/plugins/Referrers/Columns/Keyword.php
index b3bfc374b1..7c927d6492 100644
--- a/plugins/Referrers/Columns/Keyword.php
+++ b/plugins/Referrers/Columns/Keyword.php
@@ -42,7 +42,7 @@ class Keyword extends Base
      */
     public function onNewVisit(Request $request, Visitor $visitor, $action)
     {
-        $information = $this->getReferrerInformationFromRequest($request);
+        $information = $this->getReferrerInformationFromRequest($request, $visitor);
 
         if (!empty($information['referer_keyword'])) {
             return Common::mb_substr($information['referer_keyword'], 0, 255);
diff --git a/plugins/Referrers/Columns/ReferrerName.php b/plugins/Referrers/Columns/ReferrerName.php
index ded0a48f9e..ef21e7a277 100644
--- a/plugins/Referrers/Columns/ReferrerName.php
+++ b/plugins/Referrers/Columns/ReferrerName.php
@@ -36,7 +36,7 @@ class ReferrerName extends Base
      */
     public function onNewVisit(Request $request, Visitor $visitor, $action)
     {
-        $information = $this->getReferrerInformationFromRequest($request);
+        $information = $this->getReferrerInformationFromRequest($request, $visitor);
 
         if (!empty($information['referer_name'])) {
             return Common::mb_substr($information['referer_name'], 0, 70);
diff --git a/plugins/Referrers/Columns/ReferrerType.php b/plugins/Referrers/Columns/ReferrerType.php
index 5303a1c360..ef3a66325c 100644
--- a/plugins/Referrers/Columns/ReferrerType.php
+++ b/plugins/Referrers/Columns/ReferrerType.php
@@ -42,7 +42,7 @@ class ReferrerType extends Base
      */
     public function onNewVisit(Request $request, Visitor $visitor, $action)
     {
-        $information = $this->getReferrerInformationFromRequest($request);
+        $information = $this->getReferrerInformationFromRequest($request, $visitor);
 
         return $information['referer_type'];
     }
diff --git a/plugins/Referrers/Columns/ReferrerUrl.php b/plugins/Referrers/Columns/ReferrerUrl.php
index 21c5cd5bdc..ab523740a7 100644
--- a/plugins/Referrers/Columns/ReferrerUrl.php
+++ b/plugins/Referrers/Columns/ReferrerUrl.php
@@ -35,7 +35,7 @@ class ReferrerUrl extends Base
      */
     public function onNewVisit(Request $request, Visitor $visitor, $action)
     {
-        $information = $this->getReferrerInformationFromRequest($request);
+        $information = $this->getReferrerInformationFromRequest($request, $visitor);
 
         return $information['referer_url'];
     }
diff --git a/plugins/Referrers/Columns/Website.php b/plugins/Referrers/Columns/Website.php
index 42d05068d6..33484b6de1 100644
--- a/plugins/Referrers/Columns/Website.php
+++ b/plugins/Referrers/Columns/Website.php
@@ -41,7 +41,7 @@ class Website extends Base
             return false;
         }
 
-        $information = $this->getReferrerInformationFromRequest($request);
+        $information = $this->getReferrerInformationFromRequest($request, $visitor);
 
         if ($information['referer_type'] == Common::REFERRER_TYPE_WEBSITE
             && $this->isReferrerInformationNew($visitor, $information)
diff --git a/plugins/Referrers/tests/Integration/Columns/ReferrerKeywordTest.php b/plugins/Referrers/tests/Integration/Columns/ReferrerKeywordTest.php
new file mode 100644
index 0000000000..f5ff10e11c
--- /dev/null
+++ b/plugins/Referrers/tests/Integration/Columns/ReferrerKeywordTest.php
@@ -0,0 +1,100 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\Referrers\tests\Integration\Columns;
+
+use Piwik\Plugins\Referrers\Columns\Keyword;
+use Piwik\Tests\Framework\Fixture;
+use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+use Piwik\Tracker\Request;
+use Piwik\Tracker\Visit\VisitProperties;
+use Piwik\Tracker\Visitor;
+
+/**
+ * @group Referrers
+ * @group ReferrerTypeTest
+ * @group ReferrerType
+ * @group Plugins
+ */
+class ReferrerKeywordTest extends IntegrationTestCase
+{
+    /**
+     * @var Keyword
+     */
+    private $keyword;
+    private $idSite1 = 1;
+    private $idSite2 = 2;
+
+    public function setUp()
+    {
+        parent::setUp();
+
+        $date = '2012-01-01 00:00:00';
+        $ecommerce = false;
+
+        Fixture::createWebsite($date, $ecommerce, $name = 'test1', $url = 'http://piwik.org/');
+        Fixture::createWebsite($date, $ecommerce, $name = 'test3', $url = 'http://piwik.pro/');
+
+        $this->keyword = new Keyword();
+    }
+
+    /**
+     * @dataProvider getReferrerUrls
+     */
+    public function test_onNewVisit_shouldDetectCorrectReferrerType($expectedType, $idSite, $url, $referrerUrl)
+    {
+        $request = $this->getRequest(array('idsite' => $idSite, 'url' => $url, 'urlref' => $referrerUrl));
+        $type = $this->keyword->onNewVisit($request, $this->getNewVisitor(), $action = null);
+
+        $this->assertSame($expectedType, $type);
+    }
+
+    public function getReferrerUrls()
+    {
+        $url = 'http://piwik.org/foo/bar';
+        $noReferrer = '';
+        $directReferrer = 'http://piwik.org';
+        $externalReferrer = 'http://example.org';
+
+        $noReferrerKeyword = null;
+        $emptyReferrerKeyword = '';
+
+        return array(
+            // website referrer types usually do not have a keyword
+            array($noReferrerKeyword, $this->idSite1, $url, $externalReferrer),
+            // direct entries do usually not have a referrer keyowrd
+            array($noReferrerKeyword, $this->idSite1, $url, $directReferrer),
+
+            // it is a campaign but there is no referrer url and no keyword set specifically, we cannot detect a keyword
+            // it does not return null as it is converted to strlower(null)
+            array($emptyReferrerKeyword, $this->idSite1, $url . '?pk_campaign=test', $noReferrer),
+
+            // campaigns, coming from same domain should have a keyword
+            array('piwik.org',     $this->idSite1, $url . '?pk_campaign=test', $directReferrer),
+            // campaigns, coming from different domain should have a keyword
+            array('example.org',     $this->idSite2, $url . '?pk_campaign=test', $externalReferrer),
+            // campaign keyword is specifically set
+            array('campaignkey1',     $this->idSite2, $url . '?pk_campaign=test&pk_keyword=campaignkey1', $externalReferrer),
+            array('campaignkey2',     $this->idSite2, $url . '?pk_campaign=test&utm_term=campaignkey2', $externalReferrer),
+
+            // search engine should have keyword the search term
+            array('piwik', $this->idSite2, $url, 'http://google.com/search?q=piwik'),
+        );
+    }
+
+    private function getRequest($params)
+    {
+        return new Request($params);
+    }
+
+    private function getNewVisitor()
+    {
+        return new Visitor(new VisitProperties());
+    }
+
+}
diff --git a/plugins/Referrers/tests/Integration/Columns/ReferrerNameTest.php b/plugins/Referrers/tests/Integration/Columns/ReferrerNameTest.php
index db34a25c46..6c0bcfb1e0 100644
--- a/plugins/Referrers/tests/Integration/Columns/ReferrerNameTest.php
+++ b/plugins/Referrers/tests/Integration/Columns/ReferrerNameTest.php
@@ -74,7 +74,7 @@ class ReferrerNameTest extends IntegrationTestCase
         $url = 'http://piwik.org/foo/bar';
         $referrer = 'http://piwik.org';
 
-        $directEntryReferrerName = '';
+        $directEntryReferrerName = null;
 
         return array(
             // domain matches but path does not match for idsite1
diff --git a/plugins/Referrers/tests/System/ApiTest.php b/plugins/Referrers/tests/System/ApiTest.php
index 28aede3723..fb91e57ca2 100644
--- a/plugins/Referrers/tests/System/ApiTest.php
+++ b/plugins/Referrers/tests/System/ApiTest.php
@@ -8,7 +8,10 @@
 
 namespace Piwik\Plugins\Referrers\tests\System;
 
+use Piwik\API\Request;
+use Piwik\DataTable;
 use Piwik\Tests\Fixtures\TwoSitesManyVisitsOverSeveralDaysWithSearchEngineReferrers;
+use Piwik\Tests\Framework\Fixture;
 use Piwik\Tests\Framework\TestCase\SystemTestCase;
 
 /**
@@ -55,6 +58,56 @@ class ApiTest extends SystemTestCase
         return $apiToTest;
     }
 
+    public function test_forceNewVisit_shouldNotForceANewVisitWhenNoKeywordIsSetAndNoReferrerWasSetInitially()
+    {
+        $dateTime = '2015-01-02';
+        $idSite = self::$fixture->idSite;
+
+        $t = Fixture::getTracker($idSite, $dateTime . ' 00:01:02', $defaultInit = true);
+        // track a campaign that was opened directly (no referrer)
+        $t->setUrlReferrer('');
+        $t->setUrl('http://piwik.net/?pk_campaign=adwbuccc');
+        $t->doTrackPageView('My Title');
+
+        // navigate to next page on same page
+        $t->setUrlReferrer('http://piwik.net/?pk_campaign=adwbuccc');
+        $t->setCustomTrackingParameter('_rcn', 'adwbuccc'); // this parameter would be set by piwik.js from cookie / attributionInfo
+        $t->setCustomTrackingParameter('_rck', ''); // no keyword was used in previous tracking request
+        $t->setUrl('http://piwik.net/page1');
+        $t->doTrackPageView('Page 1');
+
+        /** @var DataTable $visits */
+        $visits = Request::processRequest('VisitsSummary.get', array('idSite' => 1, 'period' => 'day', 'date' => $dateTime));
+
+        $this->assertEquals(1, $visits->getFirstRow()->getColumn('nb_visits'));
+        $this->assertEquals(2, $visits->getFirstRow()->getColumn('nb_actions'));
+    }
+
+    public function test_forceNewVisit_shouldNotForceANewVisitWhenNoKeywordIsSetAndReferrerHostChanges()
+    {
+        $dateTime = '2015-01-03';
+        $idSite = self::$fixture->idSite;
+
+        $t = Fixture::getTracker($idSite, $dateTime . ' 00:01:02', $defaultInit = true);
+        // track a campaign that was opened directly (no referrer)
+        $t->setUrlReferrer('http://www.google.com');
+        $t->setUrl('http://piwik.net/?pk_campaign=adwbuccc');
+        $t->doTrackPageView('My Title');
+
+        // navigate to next page on same page
+        $t->setUrlReferrer('http://piwik.net/?pk_campaign=adwbuccc');
+        $t->setCustomTrackingParameter('_rcn', 'adwbuccc'); // this parameter would be set by piwik.js from cookie / attributionInfo
+        $t->setCustomTrackingParameter('_rck', ''); // no keyword was used in previous tracking request
+        $t->setUrl('http://piwik.net/page1');
+        $t->doTrackPageView('Page 1');
+
+        /** @var DataTable $visits */
+        $visits = Request::processRequest('VisitsSummary.get', array('idSite' => 1, 'period' => 'day', 'date' => $dateTime));
+
+        $this->assertEquals(1, $visits->getFirstRow()->getColumn('nb_visits'));
+        $this->assertEquals(2, $visits->getFirstRow()->getColumn('nb_actions'));
+    }
+
     public static function getOutputPrefix()
     {
         return '';