Skip to content

Commit

Permalink
Fixes #3776, Fixes #3787
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mattab committed Mar 3, 2013
1 parent dcd1374 commit 12c3fbe
Show file tree
Hide file tree
Showing 5 changed files with 180 additions and 36 deletions.
2 changes: 1 addition & 1 deletion core/Tracker.php
Expand Up @@ -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))
{
Expand Down
87 changes: 55 additions & 32 deletions core/Tracker/Visit.php
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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));
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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';
Expand Down
10 changes: 8 additions & 2 deletions libs/PiwikTracker/PiwikTracker.php
Expand Up @@ -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;
}
Expand Down
115 changes: 115 additions & 0 deletions tests/PHPUnit/Integration/TrackingAPI_SetVisitorIdTest.php
@@ -0,0 +1,115 @@
<?php
/**
* Piwik - Open source web analytics
*
* @link http://piwik.org
* @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
*/

/**
* This test tests that when using &cid=, the visitor ID is enforced
*
*/
class Test_Piwik_Integration_TrackingAPI_SetVisitorId extends IntegrationTestCase
{
protected static $idSite = 1;
protected static $dateTime = '2010-03-06 11:22:33';

public static function setUpBeforeClass()
{
parent::setUpBeforeClass();
try {
self::setUpWebsitesAndGoals();
self::trackVisits();
} catch(Exception $e) {
// Skip whole test suite if an error occurs while setup
throw new PHPUnit_Framework_SkippedTestSuiteError($e->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');

This comment has been minimized.

Copy link
@benjaminpick

benjaminpick Mar 3, 2013

I don't think this test case fails if no exception is thrown ... You should add a $this->fail('No exception was thrown') after this line.

This comment has been minimized.

Copy link
@mattab

mattab Mar 4, 2013

Author Member

thanks, now fixed!

} catch(Exception $e) {
//OK
}
try {
$t->setVisitorId('61e8');
} catch(Exception $e) {
//OK
}
try {
$t->setVisitorId('61e8cc2d51fea26dabcabcabc');
} catch(Exception $e) {
//OK
}
}
}
Expand Up @@ -15,4 +15,4 @@
&lt;/script&gt;
&lt;noscript&gt;&lt;p&gt;&lt;img src=&quot;http://example.org/piwik/piwik.php?idsite=1&quot; style=&quot;border:0&quot; alt=&quot;&quot; /&gt;&lt;/p&gt;&lt;/noscript&gt;
&lt;!-- End Piwik Code --&gt;
</result>
</result>

0 comments on commit 12c3fbe

Please sign in to comment.