Overwrite direct entry referrer information if campaign referrer is f…

…ound in later request. (matomo-org#14273) * Overwrite direct entry referrer information if campaign referrer is found in later request. * Update for non-campaign referrers as well. * test comment tweaks * Add current tracking URL into site urls in case website has no main url. * Fix a couple issues and start adding tests. * More tests and a fix. * Apply review feedback. * Fix couple tests. * Fix referrer tests. * try to fix random failure * Add note to README.
Charpy · May 10, 2019 · c6eb523 · c6eb523
1 parent a4c8dc9
commit c6eb523
Show file tree

Hide file tree

Showing 17 changed files with 1,548 additions and 53 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,9 @@ The Product Changelog at **[matomo.org/changelog](https://matomo.org/changelog)*
 
 ## Matomo 3.10.0
 
+### Breaking Changes
+* Website referrer URLs are now detected using domain only instead of domain and path. This means if you have two different websites on the same domain, but different paths, and a visitor visits from one to the other, it won't have a referrer website set.
+
 ### New APIs
 * A new tracker method `ping` has been added to send a ping request manually instead of using the heart beat timer.
 * Added new event `ViewDataTable.configure.end`, triggered after view configuration properties have been overwritten by saved settings and query parameters.

diff --git a/plugins/Referrers/Columns/Base.php b/plugins/Referrers/Columns/Base.php
@@ -96,7 +96,7 @@ protected function getReferrerInformation($referrerUrl, $currentUrl, $idSite, Re
         $referrerDetected = $this->detectReferrerCampaign($request, $visitor);
 
         if (!$referrerDetected) {
-            if ($this->detectReferrerDirectEntry()
+            if ($this->detectReferrerDirectEntry($request)
                 || $this->detectReferrerSearchEngine()
                 || $this->detectReferrerSocialNetwork()
             ) {
@@ -111,6 +111,8 @@ protected function getReferrerInformation($referrerUrl, $currentUrl, $idSite, Re
             $urlsByHost = $this->getCachedUrlsByHostAndIdSite();
 
             $directEntry = new SiteUrls();
+            $directEntry->addRequestUrlToSiteUrls($urlsByHost, $request);
+
             $path = $directEntry->getPathMatchingUrl($this->referrerUrlParse, $urlsByHost);
             if (!empty($path) && $path !== '/') {
                 $this->nameReferrerAnalyzed .= rtrim($path, '/');
@@ -337,7 +339,7 @@ private function getCachedUrlsByHostAndIdSite()
      * it is considered a direct entry
      * @return bool
      */
-    protected function detectReferrerDirectEntry()
+    protected function detectReferrerDirectEntry(Request $request)
     {
         if (empty($this->referrerHost)) {
             return false;
@@ -346,6 +348,8 @@ protected function detectReferrerDirectEntry()
         $urlsByHost = $this->getCachedUrlsByHostAndIdSite();
 
         $directEntry   = new SiteUrls();
+        $directEntry->addRequestUrlToSiteUrls($urlsByHost, $request);
+
         $matchingSites = $directEntry->getIdSitesMatchingUrl($this->referrerUrlParse, $urlsByHost);
 
         if (isset($matchingSites) && is_array($matchingSites) && in_array($this->idsite, $matchingSites)) {
@@ -602,4 +606,10 @@ private function truncateReferrerKeyword($refererKeyword)
     {
         return Common::mb_substr($refererKeyword, 0, 255);
     }
+
+    protected function isCurrentReferrerDirectEntry(Visitor $visitor)
+    {
+        $referrerType = $visitor->getVisitorColumn('referer_type');
+        return $referrerType == Common::REFERRER_TYPE_DIRECT_ENTRY;
+    }
 }
diff --git a/plugins/Referrers/Columns/Campaign.php b/plugins/Referrers/Columns/Campaign.php
@@ -48,8 +48,12 @@ public function shouldForceNewVisit(Request $request, Visitor $visitor, Action $
 
         $information = $this->getReferrerInformationFromRequest($request, $visitor);
 
+        // we force a new visit if the referrer is a campaign and it's different than the currently recorded referrer.
+        // if the current referrer is 'direct entry', however, we assume the referrer information was sent in a later request, and
+        // we just update the existing referrer information instead of creating a visit.
         if ($information['referer_type'] == Common::REFERRER_TYPE_CAMPAIGN
             && $this->isReferrerInformationNew($visitor, $information)
+            && !$this->isCurrentReferrerDirectEntry($visitor)
         ) {
             Common::printDebug("Existing visit detected, but creating new visit because campaign information is different than last action.");
 

diff --git a/plugins/Referrers/Columns/Keyword.php b/plugins/Referrers/Columns/Keyword.php
@@ -36,6 +36,18 @@ public function onNewVisit(Request $request, Visitor $visitor, $action)
         return $information['referer_keyword'];
     }
 
+    public function onExistingVisit(Request $request, Visitor $visitor, $action)
+    {
+        $information = $this->getReferrerInformationFromRequest($request, $visitor);
+        if ($this->isCurrentReferrerDirectEntry($visitor)
+            && $information['referer_type'] != Common::REFERRER_TYPE_DIRECT_ENTRY
+        ) {
+            return $information['referer_keyword'];
+        }
+
+        return false;
+    }
+
     /**
      * @param Request $request
      * @param Visitor $visitor

diff --git a/plugins/Referrers/Columns/ReferrerName.php b/plugins/Referrers/Columns/ReferrerName.php
@@ -37,6 +37,18 @@ public function onNewVisit(Request $request, Visitor $visitor, $action)
         return $information['referer_name'];
     }
 
+    public function onExistingVisit(Request $request, Visitor $visitor, $action)
+    {
+        $information = $this->getReferrerInformationFromRequest($request, $visitor);
+        if ($this->isCurrentReferrerDirectEntry($visitor)
+            && $information['referer_type'] != Common::REFERRER_TYPE_DIRECT_ENTRY
+        ) {
+            return $information['referer_name'];
+        }
+
+        return false;
+    }
+
     /**
      * @param Request $request
      * @param Visitor $visitor

diff --git a/plugins/Referrers/Columns/ReferrerType.php b/plugins/Referrers/Columns/ReferrerType.php
@@ -55,6 +55,18 @@ public function onNewVisit(Request $request, Visitor $visitor, $action)
         return $information['referer_type'];
     }
 
+    public function onExistingVisit(Request $request, Visitor $visitor, $action)
+    {
+        $information = $this->getReferrerInformationFromRequest($request, $visitor);
+        if ($this->isCurrentReferrerDirectEntry($visitor)
+            && $information['referer_type'] != Common::REFERRER_TYPE_DIRECT_ENTRY
+        ) {
+            return $information['referer_type'];
+        }
+
+        return false;
+    }
+
     /**
      * @param Request $request
      * @param Visitor $visitor

diff --git a/plugins/Referrers/Columns/ReferrerUrl.php b/plugins/Referrers/Columns/ReferrerUrl.php
@@ -8,6 +8,7 @@
  */
 namespace Piwik\Plugins\Referrers\Columns;
 
+use Piwik\Common;
 use Piwik\Tracker\Request;
 use Piwik\Tracker\Visitor;
 use Piwik\Tracker\Action;
@@ -35,4 +36,16 @@ public function onNewVisit(Request $request, Visitor $visitor, $action)
 
         return $information['referer_url'];
     }
+
+    public function onExistingVisit(Request $request, Visitor $visitor, $action)
+    {
+        $information = $this->getReferrerInformationFromRequest($request, $visitor);
+        if ($this->isCurrentReferrerDirectEntry($visitor)
+            && $information['referer_type'] != Common::REFERRER_TYPE_DIRECT_ENTRY
+        ) {
+            return $information['referer_url'];
+        }
+
+        return false;
+    }
 }
diff --git a/plugins/Referrers/tests/Integration/Columns/ReferrerKeywordTest.php b/plugins/Referrers/tests/Integration/Columns/ReferrerKeywordTest.php
@@ -8,6 +8,7 @@
 
 namespace Piwik\Plugins\Referrers\tests\Integration\Columns;
 
+use Piwik\Common;
 use Piwik\Plugins\Referrers\Columns\Keyword;
 use Piwik\Tests\Framework\Fixture;
 use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
@@ -29,6 +30,7 @@ class ReferrerKeywordTest extends IntegrationTestCase
     private $keyword;
     private $idSite1 = 1;
     private $idSite2 = 2;
+    private $idSite3 = 3;
 
     public function setUp()
     {
@@ -38,7 +40,8 @@ public function setUp()
         $ecommerce = false;
 
         Fixture::createWebsite($date, $ecommerce, $name = 'test1', $url = 'http://piwik.org/');
-        Fixture::createWebsite($date, $ecommerce, $name = 'test3', $url = 'http://piwik.xyz/');
+        Fixture::createWebsite($date, $ecommerce, $name = 'test2', $url = 'http://piwik.xyz/');
+        Fixture::createWebsite($date, $ecommerce, $name = 'test3', $url = null);
 
         $this->keyword = new Keyword();
     }
@@ -84,9 +87,54 @@ public function getReferrerUrls()
 
             // search engine should have keyword the search term
             array('piwik', $this->idSite2, $url, 'http://google.com/search?q=piwik'),
+
+            // site w/o url
+            array($noReferrerKeyword, $this->idSite3, $url, $directReferrer . '/'),
         );
     }
 
+    /**
+     * @dataProvider getTestDataForOnExistingVisit
+     */
+    public function test_onExistingVisit_shouldSometimesOverwriteReferrerInfo($expectedKeyword, $idSite, $url, $referrerUrl, $existingType)
+    {
+        $request = $this->getRequest(array('idsite' => $idSite, 'url' => $url, 'urlref' => $referrerUrl));
+        $visitor = $this->getNewVisitor();
+        $visitor->setVisitorColumn('referer_type', $existingType);
+        $keyword = $this->keyword->onExistingVisit($request, $visitor, $action = null);
+
+        $this->assertSame($expectedKeyword, $keyword);
+    }
+
+    public function getTestDataForOnExistingVisit()
+    {
+        return [
+            // direct entry => campaign
+            ['campaignkey1', $this->idSite2, 'http://piwik.xyz/abc?pk_campaign=testfoobar&pk_keyword=campaignkey1', 'http://piwik.org', Common::REFERRER_TYPE_DIRECT_ENTRY],
+
+            // direct entry => website
+            ['piwik2', $this->idSite2, 'http://piwik.xyz/abc', 'http://google.com/search?q=piwik2', Common::REFERRER_TYPE_DIRECT_ENTRY],
+
+            // direct entry => direct entry
+            [false, $this->idSite2, 'http://piwik.xyz/abc', 'http://piwik.xyz/def', Common::REFERRER_TYPE_DIRECT_ENTRY],
+
+            // website => direct entry
+            [false, $this->idSite2, 'http://piwik.xyz/abc', 'http://piwik.xyz/def', Common::REFERRER_TYPE_WEBSITE],
+
+            // campaign => direct entry
+            [false, $this->idSite2, 'http://piwik.xyz/abc', 'http://piwik.xyz/def', Common::REFERRER_TYPE_CAMPAIGN],
+
+            // direct entry => website (site w/o url)
+            ['piwik3', $this->idSite3, 'http://piwik.xyz/abc', 'http://google.com/search?q=piwik3', Common::REFERRER_TYPE_DIRECT_ENTRY],
+
+            // direct entry => direct entry (site w/o url)
+            [false, $this->idSite3, 'http://piwik.xyz/abc', 'http://piwik.xyz/def', Common::REFERRER_TYPE_DIRECT_ENTRY],
+
+            // website => direct entry (site w/o url)
+            [false, $this->idSite3, 'http://piwik.xyz/abc', 'http://piwik.xyz/def', Common::REFERRER_TYPE_WEBSITE],
+        ];
+    }
+
     private function getRequest($params)
     {
         return new Request($params);

diff --git a/plugins/Referrers/tests/Integration/Columns/ReferrerNameTest.php b/plugins/Referrers/tests/Integration/Columns/ReferrerNameTest.php
@@ -8,6 +8,7 @@
 
 namespace Piwik\Plugins\Referrers\tests\Integration\Columns;
 
+use Piwik\Common;
 use Piwik\Plugins\Referrers\Columns\ReferrerName;
 use Piwik\Plugins\Referrers\Columns\ReferrerType;
 use Piwik\Tests\Framework\Fixture;
@@ -32,6 +33,7 @@ class ReferrerNameTest extends IntegrationTestCase
     private $idSite1 = 1;
     private $idSite2 = 2;
     private $idSite3 = 3;
+    private $idSite4 = 4;
 
     public function setUp()
     {
@@ -45,6 +47,7 @@ public function setUp()
         Fixture::createWebsite($date, $ecommerce, $name = 'test1', $url = 'http://piwik.org/foo/bar');
         Fixture::createWebsite($date, $ecommerce, $name = 'test2', $url = 'http://piwik.org/');
         Fixture::createWebsite($date, $ecommerce, $name = 'test3', $url = 'http://piwik.xyz/');
+        Fixture::createWebsite($date, $ecommerce, $name = 'test4', $url = null);
 
         $this->referrerName = new ReferrerName();
     }
@@ -77,21 +80,21 @@ public function getReferrerUrls()
 
         return array(
             // domain matches but path does not match for idsite1
-            array('piwik.org',              $this->idSite1, $url, $referrer),
-            array('piwik.org',              $this->idSite1, $url, $referrer . '/'),
+            array(null,              $this->idSite1, $url, $referrer),
+            array(null,              $this->idSite1, $url, $referrer . '/'),
             // idSite2 matches any piwik.org path so this is a direct entry for it
             array($directEntryReferrerName, $this->idSite2, $url, $referrer),
             array($directEntryReferrerName, $this->idSite2, $url, $referrer . '/'),
             // idSite3 has different domain so it is coming from different website
-            array('piwik.org',              $this->idSite3, $url, $referrer),
-            array('piwik.org',              $this->idSite3, $url, $referrer . '/'),
+            array(null,              $this->idSite3, $url, $referrer),
+            array(null,              $this->idSite3, $url, $referrer . '/'),
 
             array($directEntryReferrerName, $this->idSite1, $url, $referrer . '/foo/bar/baz'),
             array($directEntryReferrerName, $this->idSite1, $url, $referrer . '/foo/bar/baz/'),
             array($directEntryReferrerName, $this->idSite1, $url, $referrer . '/foo/bar/baz?x=5'),
             array($directEntryReferrerName, $this->idSite1, $url, $referrer . '/fOo/BaR/baz?x=5'),
             // /not/xyz belongs to different website
-            array('piwik.org',              $this->idSite1, $url, $referrer . '/not/xyz'),
+            array(null,              $this->idSite1, $url, $referrer . '/not/xyz'),
             array($directEntryReferrerName, $this->idSite2, $url, $referrer . '/not/xyz'),
 
             // /foo/bar/baz belongs to different website
@@ -113,9 +116,54 @@ public function getReferrerUrls()
 
             // testing case where domain of referrer is not known to any site but neither is the URL, url != urlref
             array('example.com',            $this->idSite3, 'http://example.org',     'http://example.com/bar'),
+
+            // site w/o url
+            array($directEntryReferrerName, $this->idSite4, $url, $referrer . '/'),
         );
     }
 
+    /**
+     * @dataProvider getTestDataForOnExistingVisit
+     */
+    public function test_onExistingVisit_shouldSometimesOverwriteReferrerInfo($expectedName, $idSite, $url, $referrerUrl, $existingType)
+    {
+        $request = $this->getRequest(array('idsite' => $idSite, 'url' => $url, 'urlref' => $referrerUrl));
+        $visitor = $this->getNewVisitor();
+        $visitor->setVisitorColumn('referer_type', $existingType);
+        $name = $this->referrerName->onExistingVisit($request, $visitor, $action = null);
+
+        $this->assertSame($expectedName, $name);
+    }
+
+    public function getTestDataForOnExistingVisit()
+    {
+        return [
+            // direct entry => campaign
+            ['testfoobar', $this->idSite3, 'http://piwik.xyz/abc?pk_campaign=testfoobar', 'http://piwik.org', Common::REFERRER_TYPE_DIRECT_ENTRY],
+
+            // direct entry => website
+            ['piwik.org', $this->idSite3, 'http://piwik.xyz/abc', 'http://piwik.org', Common::REFERRER_TYPE_DIRECT_ENTRY],
+
+            // direct entry => direct entry
+            [false, $this->idSite3, 'http://piwik.xyz/abc', 'http://piwik.xyz/def', Common::REFERRER_TYPE_DIRECT_ENTRY],
+
+            // website => direct entry
+            [false, $this->idSite3, 'http://piwik.xyz/abc', 'http://piwik.xyz/def', Common::REFERRER_TYPE_WEBSITE],
+
+            // campaign => direct entry
+            [false, $this->idSite3, 'http://piwik.xyz/abc', 'http://piwik.xyz/def', Common::REFERRER_TYPE_CAMPAIGN],
+
+            // direct entry => website (site w/o url)
+            ['piwik.org', $this->idSite4, 'http://piwik.xyz/abc', 'http://piwik.org/', Common::REFERRER_TYPE_DIRECT_ENTRY],
+
+            // direct entry => direct entry (site w/o url)
+            [false, $this->idSite4, 'http://piwik.xyz/abc', 'http://piwik.xyz/def', Common::REFERRER_TYPE_DIRECT_ENTRY],
+
+            // website => direct entry (site w/o url)
+            [false, $this->idSite4, 'http://piwik.xyz/abc', 'http://piwik.xyz/def', Common::REFERRER_TYPE_WEBSITE],
+        ];
+    }
+
     private function getRequest($params)
     {
         return new Request($params);