From 12c3fbeb845c42d493df4440e35ccf95c3dcae45 Mon Sep 17 00:00:00 2001 From: mattab Date: Sun, 3 Mar 2013 13:37:03 +1300 Subject: [PATCH] Fixes #3776, Fixes #3787 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding integration test to test this code. I also updated [ tracking api reference] to explain "cid" parameter. cid — (requires token_auth to be set) defines the visitor ID for this request. You must set this value to exactly a 16 character hexadecimal string (containing only characters 01234567890abcdefABCDEF). When specified, the Visitor ID will be “enforced”. This means that if there is no recent visit with this visitor ID, a new one will be created. If a visit is found in the last 30 minutes with your specified Visitor Id, then the new action will be recorded to this existing visit. --- core/Tracker.php | 2 +- core/Tracker/Visit.php | 87 ++++++++----- libs/PiwikTracker/PiwikTracker.php | 10 +- .../TrackingAPI_SetVisitorIdTest.php | 115 ++++++++++++++++++ ...ta_year__SitesManager.getJavascriptTag.xml | 2 +- 5 files changed, 180 insertions(+), 36 deletions(-) create mode 100644 tests/PHPUnit/Integration/TrackingAPI_SetVisitorIdTest.php diff --git a/core/Tracker.php b/core/Tracker.php index 3145abbc6b3..ea51577520c 100644 --- a/core/Tracker.php +++ b/core/Tracker.php @@ -807,7 +807,7 @@ public static function setTestEnvironment( $args = null, $requestMethod = null ) self::setForceDateTime($customDatetime); } - // Custom server date time to use + // Custom visitor id $customVisitorId = Piwik_Common::getRequestVar('cid', false, null, $args); if(!empty($customVisitorId)) { diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php index f89d19aa6fa..c5c3032bd6c 100644 --- a/core/Tracker/Visit.php +++ b/core/Tracker/Visit.php @@ -40,18 +40,19 @@ class Piwik_Tracker_Visit implements Piwik_Tracker_Visit_Interface * @var Piwik_Cookie */ protected $cookie = null; - protected $visitorInfo = array(); - protected $userSettingsInformation = null; - protected $visitorCustomVariables = array(); - protected $idsite; - protected $visitorKnown; - protected $request; + protected $visitorInfo = array(); + protected $userSettingsInformation = null; + protected $visitorCustomVariables = array(); + protected $idsite; + protected $visitorKnown; + protected $request; + protected $forcedVisitorId = null; + + // can be overwritten in constructor + protected $timestamp; + protected $ip; + protected $authenticated = false; - // can be overwritten in constructor - protected $timestamp; - protected $ip; - protected $authenticated = false; - // Set to true when we set some custom variables from the cookie protected $customVariablesSetFromRequest = false; @@ -755,13 +756,19 @@ protected function getVisitorIdcookie() { return $this->visitorInfo['idvisitor']; } + return Piwik_Common::hex2bin($this->generateUniqueVisitorId()); + } - // Return Random UUID - $uniqueId = substr($this->getVisitorUniqueId(), 0, Piwik_Tracker::LENGTH_HEX_ID_STRING); - return Piwik_Common::hex2bin($uniqueId); - } + /** + * @return string returns random 16 chars hex string + */ + static public function generateUniqueVisitorId() + { + $uniqueId = substr(Piwik_Common::generateUniqId(), 0, Piwik_Tracker::LENGTH_HEX_ID_STRING); + return $uniqueId; + } - /** + /** * Returns the visitor's IP address * * @return long @@ -1045,12 +1052,12 @@ protected function assignVisitorIdFromRequest() $found = false; // Was a Visitor ID "forced" (@see Tracking API setVisitorId()) for this request? - $idVisitor = $this->forcedVisitorId; + $idVisitor = $this->getForcedVisitorId(); if(!empty($idVisitor)) { if(strlen($idVisitor) != Piwik_Tracker::LENGTH_HEX_ID_STRING) { - throw new Exception("Visitor ID (cid) must be ".Piwik_Tracker::LENGTH_HEX_ID_STRING." characters long"); + throw new Exception("Visitor ID (cid) $idVisitor must be ".Piwik_Tracker::LENGTH_HEX_ID_STRING." characters long"); } printDebug("Request will be recorded for this idvisitor = ".$idVisitor); $found = true; @@ -1090,7 +1097,12 @@ protected function assignVisitorIdFromRequest() } } - /** + protected function getForcedVisitorId() + { + return $this->forcedVisitorId; + } + + /** * This methods tries to see if the visitor has visited the website before. * * We have to split the visitor into one of the category @@ -1110,9 +1122,9 @@ protected function recognizeTheVisitor() $configId = $userInfo['config_id']; $this->assignVisitorIdFromRequest(); - $matchVisitorId = !empty($this->visitorInfo['idvisitor']); + $isVisitorIdToLookup = !empty($this->visitorInfo['idvisitor']); - if($matchVisitorId) + if($isVisitorIdToLookup) { printDebug("Matching visitors with: visitorId=".bin2hex($this->visitorInfo['idvisitor'])." OR configId=".bin2hex($configId)); } @@ -1156,29 +1168,25 @@ protected function recognizeTheVisitor() "; $from = "FROM ".Piwik_Common::prefixTable('log_visit'); - $bindSql = array(); $timeLookBack = date('Y-m-d H:i:s', $this->getCurrentTimestamp() - Piwik_Config::getInstance()->Tracker['visit_standard_length']); - // This setting would be enabled for Intranet websites, to ensure that visitors using all the same computer config, same IP - // are not counted as 1 visitor. In this case, we want to enforce and trust the visitor ID from the cookie. - $trustCookiesOnly = Piwik_Config::getInstance()->Tracker['trust_visitors_cookies']; - - $shouldMatchOneFieldOnly = ($matchVisitorId && $trustCookiesOnly) || !$matchVisitorId; - - // Two use cases: + $shouldMatchOneFieldOnly = $this->shouldLookupOneVisitorFieldOnly($isVisitorIdToLookup); + + // Two use cases: // 1) there is no visitor ID so we try to match only on config_id (heuristics) // Possible causes of no visitor ID: no browser cookie support, direct Tracking API request without visitor ID passed, etc. // We can use config_id heuristics to try find the visitor in the past, there is a risk to assign // this page view to the wrong visitor, but this is better than creating artificial visits. - // 2) there is a visitor ID and we trust it (config setting trust_visitors_cookies), so we force to look up this visitor id + // 2) there is a visitor ID and we trust it (config setting trust_visitors_cookies, OR it was set using &cid= in tracking API), + // and in these cases, we force to look up this visitor id if($shouldMatchOneFieldOnly) { $where = "visit_last_action_time >= ? AND idsite = ?"; $bindSql[] = $timeLookBack; $bindSql[] = $this->idsite; - if(!$matchVisitorId) + if(!$isVisitorIdToLookup) { $where .= ' AND config_id = ?'; $bindSql[] = $configId; @@ -1305,7 +1313,22 @@ protected function recognizeTheVisitor() } } - static public function getCustomVariables($scope, $request) + protected function shouldLookupOneVisitorFieldOnly($isVisitorIdToLookup) + { + // This setting would be enabled for Intranet websites, to ensure that visitors using all the same computer config, same IP + // are not counted as 1 visitor. In this case, we want to enforce and trust the visitor ID from the cookie. + $trustCookiesOnly = Piwik_Config::getInstance()->Tracker['trust_visitors_cookies']; + + // If a &cid= was set, we force to select this visitor (or create a new one) + $isForcedVisitorIdMustMatch = ($this->getForcedVisitorId() != null); + + $shouldMatchOneFieldOnly = (($isVisitorIdToLookup && $trustCookiesOnly) + || $isForcedVisitorIdMustMatch + || !$isVisitorIdToLookup); + return $shouldMatchOneFieldOnly; + } + + static public function getCustomVariables($scope, $request) { if($scope == 'visit') { $parameter = '_cvar'; diff --git a/libs/PiwikTracker/PiwikTracker.php b/libs/PiwikTracker/PiwikTracker.php index 2f05a57fa96..69808cab85e 100644 --- a/libs/PiwikTracker/PiwikTracker.php +++ b/libs/PiwikTracker/PiwikTracker.php @@ -708,9 +708,15 @@ public function setIp($ip) */ public function setVisitorId($visitorId) { - if(strlen($visitorId) != self::LENGTH_VISITOR_ID) + $hexChars = '01234567890abcdefABCDEF'; + if(strlen($visitorId) != self::LENGTH_VISITOR_ID + || strspn($visitorId, $hexChars) !== strlen($visitorId)) { - throw new Exception("setVisitorId() expects a ".self::LENGTH_VISITOR_ID." characters ID"); + throw new Exception("setVisitorId() expects a " + .self::LENGTH_VISITOR_ID + ." characters hexadecimal string (containing only the following: " + .$hexChars + .")"); } $this->forcedVisitorId = $visitorId; } diff --git a/tests/PHPUnit/Integration/TrackingAPI_SetVisitorIdTest.php b/tests/PHPUnit/Integration/TrackingAPI_SetVisitorIdTest.php new file mode 100644 index 00000000000..611d612c17f --- /dev/null +++ b/tests/PHPUnit/Integration/TrackingAPI_SetVisitorIdTest.php @@ -0,0 +1,115 @@ +getMessage()); + } + } + + public function setUp() + { + Piwik_API_Proxy::getInstance()->setHideIgnoredFunctions(false); + } + + public function tearDown() + { + Piwik_API_Proxy::getInstance()->setHideIgnoredFunctions(true); + } + + /** + * @dataProvider getApiForTesting + * @group Integration + * @group OneVisitorTwoVisits + */ + public function testApi($api, $params) + { + $this->runApiTests($api, $params); + } + + public function getApiForTesting() + { + return array( + // test hideColumns && showColumns parameters + array('VisitsSummary.get', array('idSite' => self::$idSite, 'date' => self::$dateTime, + 'periods' => 'day', + 'testSuffix' => '', + )) + ); + } + + protected static function setUpWebsitesAndGoals() + { + // tests run in UTC, the Tracker in UTC + self::createWebsite(self::$dateTime); + } + + protected static function trackVisits() + { + $dateTime = self::$dateTime; + $idSite = self::$idSite; + $t = self::getTracker($idSite, $dateTime, $defaultInit = true); + + // First, some basic tests + self::settingInvalidVisitorIdShouldThrow($t); + + // We create VISITOR A + $t->setUrl('http://example.org/index.htm'); + $t->setVisitorId(Piwik_Tracker_Visit::generateUniqueVisitorId()); + self::checkResponse($t->doTrackPageView('incredible title!')); + + // VISITOR B: few minutes later, we trigger the same tracker but with a custom visitor ID, + // => this will create a new visit B + $t->setForceVisitDateTime(Piwik_Date::factory($dateTime)->addHour(0.05)->getDatetime()); + $t->setUrl('http://example.org/index2.htm'); + $t->setVisitorId(Piwik_Tracker_Visit::generateUniqueVisitorId()); + self::checkResponse($t->doTrackPageView('incredible title!')); + + // This new visit B will have 2 page views + $t->setForceVisitDateTime(Piwik_Date::factory($dateTime)->addHour(0.1)->getDatetime()); + $t->setUrl('http://example.org/index3.htm'); + self::checkResponse($t->doTrackPageView('incredible title!')); + + // total = 2 visitors, 3 page views + + } + + static protected function settingInvalidVisitorIdShouldThrow(PiwikTracker $t) + { + try { + $t->setVisitorId('test'); + } catch(Exception $e) { + //OK + } + try { + $t->setVisitorId('61e8'); + } catch(Exception $e) { + //OK + } + try { + $t->setVisitorId('61e8cc2d51fea26dabcabcabc'); + } catch(Exception $e) { + //OK + } + } +} diff --git a/tests/PHPUnit/Integration/expected/test_apiGetReportMetadata_year__SitesManager.getJavascriptTag.xml b/tests/PHPUnit/Integration/expected/test_apiGetReportMetadata_year__SitesManager.getJavascriptTag.xml index 91ba03727ac..bc73182494a 100644 --- a/tests/PHPUnit/Integration/expected/test_apiGetReportMetadata_year__SitesManager.getJavascriptTag.xml +++ b/tests/PHPUnit/Integration/expected/test_apiGetReportMetadata_year__SitesManager.getJavascriptTag.xml @@ -15,4 +15,4 @@ </script> <noscript><p><img src="http://example.org/piwik/piwik.php?idsite=1" style="border:0" alt="" /></p></noscript> <!-- End Piwik Code --> - + \ No newline at end of file