From e351acdf440d065591fd6bc54de04899a63d33fd Mon Sep 17 00:00:00 2001 From: mattpiwik <matthieu.aubry@gmail.com> Date: Fri, 8 Apr 2011 04:44:20 +0000 Subject: [PATCH] Refs #2222 Implementing Visitor ID forced request, so that we can log, after the fact, a page/goal/action to the latest visit of a given visitor ID (useful for example to trigger goal conversions that were missed by the JS beacon...) Had to add another index on the log_visit table unfortunately, to ensure reasonnably fast lookup on the idvisitor, even though this index will very rarely get used (never if you don't use the tracking API with setVisitorId()...) git-svn-id: http://dev.piwik.org/svn/trunk@4369 59fd770c-687e-43c8-a1e3-f5a4ff64c105 --- config/global.ini.php | 12 ++-- core/Db/Schema/Myisam.php | 3 +- core/Tracker.php | 18 +++++- core/Tracker/Visit.php | 62 ++++++++++++++----- core/Updates/1.2.5-rc7.php | 32 ++++++++++ core/Version.php | 2 +- libs/PiwikTracker/PiwikTracker.php | 6 +- tests/integration/Main.test.php | 35 +++++++++++ ...eadOfHeuristics__VisitsSummary.get_day.xml | 13 ++++ tests/integration/proxy-piwik.php | 7 +++ 10 files changed, 163 insertions(+), 27 deletions(-) create mode 100644 core/Updates/1.2.5-rc7.php create mode 100644 tests/integration/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics__VisitsSummary.get_day.xml diff --git a/config/global.ini.php b/config/global.ini.php index f0bf948fb2..a0ba449dbe 100644 --- a/config/global.ini.php +++ b/config/global.ini.php @@ -245,6 +245,12 @@ api_service_url = http://api.piwik.org ; this is useful when you want to do cross websites analysis use_third_party_id_cookie = 0 +; By default, Piwik does not trust the idcookie as accurate and will always check that if the visitor visited +; the website earlier by looking for a visitor with the same IP and user configuration (to avoid abuse or misbehaviour) +; This setting should only be set to 1 in an intranet setting, where most users have the same configuration (browsers, OS) +; and the same IP. If left to 0 in this setting, all visitors will be counted as one single visitor. +trust_visitors_cookies = 0 + ; name of the cookie used to store the visitor information ; This is used only if use_third_party_id_cookie = 1 cookie_name = piwik_visitor @@ -268,12 +274,6 @@ visit_standard_length = 1800 ; visitors that stay on the website and view only one page will be considered as time on site of 0 second default_time_one_page_visit = 0 -; By default, Piwik does not trust the idcookie as accurate and will always check that if the visitor visited -; the website earlier by looking for a visitor with the same IP and user configuration (to avoid abuse or misbehaviour) -; This setting should only be set to 1 in an intranet setting, where most users have the same configuration (browsers, OS) -; and the same IP. If left to 0 in this setting, all visitors will be counted as one single visitor. -trust_visitors_cookies = 0 - ; if set to 1, Piwik attempts a "best guess" at the visitor's country of ; origin when the preferred language tag omits region information. ; The mapping is defined in core/DataFiles/LanguageToCountry.php, diff --git a/core/Db/Schema/Myisam.php b/core/Db/Schema/Myisam.php index c3f08c0764..9c529d39bc 100644 --- a/core/Db/Schema/Myisam.php +++ b/core/Db/Schema/Myisam.php @@ -221,7 +221,8 @@ class Piwik_Db_Schema_Myisam implements Piwik_Db_Schema_Interface custom_var_v5 VARCHAR(50) DEFAULT NULL, PRIMARY KEY(idvisit), INDEX index_idsite_config_datetime (idsite, config_id, visit_last_action_time), - INDEX index_idsite_datetime (idsite, visit_last_action_time) + INDEX index_idsite_datetime (idsite, visit_last_action_time), + INDEX index_idsite_idvisitor (idsite, idvisitor) ) DEFAULT CHARSET=utf8 ", diff --git a/core/Tracker.php b/core/Tracker.php index 8511b14a37..b639d653b4 100644 --- a/core/Tracker.php +++ b/core/Tracker.php @@ -43,6 +43,7 @@ class Piwik_Tracker static protected $forcedDateTime = null; static protected $forcedIpString = null; + static protected $forcedVisitorId = null; static protected $pluginsNotToLoad = array(); @@ -59,6 +60,11 @@ class Piwik_Tracker self::$forcedDateTime = $dateTime; } + public static function setForceVisitorId($visitorId) + { + self::$forcedVisitorId = $visitorId; + } + public function getCurrentTimestamp() { if(!is_null(self::$forcedDateTime)) @@ -263,6 +269,7 @@ class Piwik_Tracker if(is_null($visit)) { $visit = new Piwik_Tracker_Visit( self::$forcedIpString, self::$forcedDateTime ); + $visit->setForcedVisitorId(self::$forcedVisitorId); } elseif(!($visit instanceof Piwik_Tracker_Visit_Interface )) { @@ -373,19 +380,24 @@ class Piwik_Tracker // Custom IP to use for this visitor $customIp = Piwik_Common::getRequestVar('cip', false); - if(!empty($customIp)) { $this->setForceIp($customIp); } - + // Custom server date time to use $customDatetime = Piwik_Common::getRequestVar('cdt', false); - if(!empty($customDatetime)) { $this->setForceDateTime($customDatetime); } + + // Forced Visitor ID to record the visit / action + $customVisitorId = Piwik_Common::getRequestVar('cid', false); + if(!empty($customVisitorId)) + { + $this->setForceVisitorId($customVisitorId); + } } } diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php index 802adcf30a..5ce28a5d41 100644 --- a/core/Tracker/Visit.php +++ b/core/Tracker/Visit.php @@ -49,6 +49,8 @@ class Piwik_Tracker_Visit implements Piwik_Tracker_Visit_Interface // can be overwritten in constructor protected $timestamp; protected $ipString; + // via setForcedVisitorId() + protected $forcedVisitorId; const TIME_IN_PAST_TO_SEARCH_FOR_VISITOR = 86400; @@ -68,6 +70,11 @@ class Piwik_Tracker_Visit implements Piwik_Tracker_Visit_Interface $this->ipString = Piwik_Common::getIp($ipString); } + function setForcedVisitorId($visitorId) + { + $this->forcedVisitorId = $visitorId; + } + function setRequest($requestArray) { $this->request = $requestArray; @@ -772,19 +779,35 @@ class Piwik_Tracker_Visit implements Piwik_Tracker_Visit_Interface $this->printCookie(); - // Read ID Visitor - $found = false; - // - If set to use 3rd party cookies for Visit ID, read the cookies - // - By default, reads the first party cookie ID - $useThirdPartyCookie = $this->shouldUseThirdPartyCookie(); - if($useThirdPartyCookie) + $found = $forcedVisitorId = false; + + // Was a Visitor ID "forced" (@see Tracking API setVisitorId()) for this request? + $idVisitor = $this->forcedVisitorId; + 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"); + } + printDebug("Request will be forced to record for this idvisitor = ".$idVisitor); + $forcedVisitorId = true; + $found = true; + } + + if(!$found) { - $idVisitor = $this->cookie->get(0); - if($idVisitor !== false - && strlen($idVisitor) == Piwik_Tracker::LENGTH_HEX_ID_STRING) + // - If set to use 3rd party cookies for Visit ID, read the cookies + // - By default, reads the first party cookie ID + $useThirdPartyCookie = $this->shouldUseThirdPartyCookie(); + if($useThirdPartyCookie) + { + $idVisitor = $this->cookie->get(0); + if($idVisitor !== false + && strlen($idVisitor) == Piwik_Tracker::LENGTH_HEX_ID_STRING) { $found = true; } + } } // If a third party cookie was not found, we default to the first party cookie if(!$found) @@ -809,16 +832,25 @@ class Piwik_Tracker_Visit implements Piwik_Tracker_Visit_Interface $timeLookBack = date('Y-m-d H:i:s', $this->getCurrentTimestamp() - self::TIME_IN_PAST_TO_SEARCH_FOR_VISITOR); $where = "visit_last_action_time >= ? - AND idsite = ? - AND config_id = ?"; - $bindSql = array( $timeLookBack, $this->idsite, $configId); + AND idsite = ?"; + $bindSql = array( $timeLookBack, $this->idsite); - if(Piwik_Tracker_Config::getInstance()->Tracker['trust_visitors_cookies'] - && !empty($this->visitorInfo['idvisitor'])) + // we always match on the config_id, except if the current request forces the visitor id + if(!$forcedVisitorId) + { + $where .= ' AND config_id = ? '; + $bindSql[] = $configId; + } + + // We force to match a visitor ID + // 1) If the visitor cookies should be trusted (ie. intranet) - config file setting + // 2) or if the Visitor ID was forced via the Tracking API setVisitorId() + if(!empty($this->visitorInfo['idvisitor']) + && ( Piwik_Tracker_Config::getInstance()->Tracker['trust_visitors_cookies'] + || $forcedVisitorId )) { printDebug("Matching the visitor based on his idcookie: ".bin2hex($this->visitorInfo['idvisitor']) ."..."); - // If the visitor cookies should be trusted (ie. intranet) we add this condition $where .= ' AND idvisitor = ?'; $bindSql[] = $this->visitorInfo['idvisitor']; } diff --git a/core/Updates/1.2.5-rc7.php b/core/Updates/1.2.5-rc7.php new file mode 100644 index 0000000000..cee6496307 --- /dev/null +++ b/core/Updates/1.2.5-rc7.php @@ -0,0 +1,32 @@ +<?php +/** + * Piwik - Open source web analytics + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + * @version $Id$ + * + * @category Piwik + * @package Updates + */ + +/** + * @package Updates + */ +class Piwik_Updates_1_2_5_rc7 extends Piwik_Updates +{ + static function getSql($schema = 'Myisam') + { + return array( + 'ALTER TABLE `'. Piwik_Common::prefixTable('log_visit') .'` + ADD INDEX index_idsite_idvisitor (idsite, idvisitor)' => false, + ); + } + + static function update() + { + Piwik_Updater::updateDatabase(__FILE__, self::getSql()); + } +} + + diff --git a/core/Version.php b/core/Version.php index ef16837da2..bc5e3590d4 100644 --- a/core/Version.php +++ b/core/Version.php @@ -17,5 +17,5 @@ */ final class Piwik_Version { - const VERSION = '1.2.5-rc6'; + const VERSION = '1.2.5-rc7'; } diff --git a/libs/PiwikTracker/PiwikTracker.php b/libs/PiwikTracker/PiwikTracker.php index f4568514fe..dd15c2effc 100644 --- a/libs/PiwikTracker/PiwikTracker.php +++ b/libs/PiwikTracker/PiwikTracker.php @@ -268,7 +268,7 @@ class PiwikTracker { throw new Exception("setVisitorId() expects a 16 characters ID"); } - $this->visitorId = $visitorId; + $this->forcedVisitorId = $visitorId; } /** @@ -281,6 +281,10 @@ class PiwikTracker */ public function getVisitorId() { + if(!empty($this->forcedVisitorId)) + { + return $this->forcedVisitorId; + } return $this->visitorId; } diff --git a/tests/integration/Main.test.php b/tests/integration/Main.test.php index 8bd6c6b774..1cc6b4f655 100644 --- a/tests/integration/Main.test.php +++ b/tests/integration/Main.test.php @@ -639,4 +639,39 @@ class Test_Piwik_Integration_Main extends Test_Integration $this->visitorId ); } + + function test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics() + { + $this->setApiToCall( 'VisitsSummary.get' ); + $dateTime = '2009-01-04 00:11:42'; + $idSite = $this->createWebsite($dateTime); + $idGoal = Piwik_Goals_API::getInstance()->addGoal($idSite, 'triggered js', 'manually', '', ''); + + $t = $this->getTracker($idSite, $dateTime, $defaultInit = true); + + // Record 1st page view + $t->setUrl( 'http://example.org/index.htm' ); + $this->checkResponse($t->doTrackPageView( 'incredible title!')); + + $visitorId = $t->getVisitorId(); + $this->assertTrue(strlen($visitorId) == 16); + + // Create a new Tracker object, with different attributes + $t2 = $this->getTracker($idSite, $dateTime, $defaultInit = false); + + // Make sure the ID is different at first + $visitorId2 = $t2->getVisitorId(); + $this->assertTrue($visitorId != $visitorId2); + + // Then force the visitor ID + $t2->setVisitorId($visitorId); + + // And Record a Goal: The previous visit should be updated rather than a new visit Created + $t2->setForceVisitDateTime(Piwik_Date::factory($dateTime)->addHour(0.3)->getDatetime()); + $this->checkResponse($t2->doTrackGoal($idGoal, $revenue = 42.256)); + + // TOTAL should be: 1 visit, 1 converted goal, 1 page view + $this->callGetApiCompareOutput(__FUNCTION__, 'xml', $idSite, $dateTime); + } + } diff --git a/tests/integration/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics__VisitsSummary.get_day.xml b/tests/integration/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics__VisitsSummary.get_day.xml new file mode 100644 index 0000000000..21459e9535 --- /dev/null +++ b/tests/integration/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics__VisitsSummary.get_day.xml @@ -0,0 +1,13 @@ +<?xml version="1.0" encoding="utf-8" ?> +<result> + <nb_visits>1</nb_visits> + <nb_uniq_visitors>1</nb_uniq_visitors> + <nb_actions>1</nb_actions> + <nb_visits_converted>1</nb_visits_converted> + <bounce_count>1</bounce_count> + <sum_visit_length>1080</sum_visit_length> + <max_actions>1</max_actions> + <bounce_rate>100%</bounce_rate> + <nb_actions_per_visit>1</nb_actions_per_visit> + <avg_time_on_site>1080</avg_time_on_site> +</result> \ No newline at end of file diff --git a/tests/integration/proxy-piwik.php b/tests/integration/proxy-piwik.php index 3921c3312f..da2fb84520 100644 --- a/tests/integration/proxy-piwik.php +++ b/tests/integration/proxy-piwik.php @@ -46,6 +46,13 @@ if(!empty($customDatetime)) Piwik_Tracker::setForceDateTime($customDatetime); } +// Custom server date time to use +$customVisitorId = Piwik_Common::getRequestVar('cid', false); +if(!empty($customVisitorId)) +{ + Piwik_Tracker::setForceVisitorId($customVisitorId); +} + // Disable provider plugin, because it is so slow to do reverse ip lookup in dev environment somehow Piwik_Tracker::setPluginsNotToLoad(array('Provider')); include '../../piwik.php'; -- GitLab