From 71c21b504cbe43bf31d3aa95101b8828cfc2bc6a Mon Sep 17 00:00:00 2001
From: Matthieu Aubry <mattab@users.noreply.github.com>
Date: Thu, 29 Sep 2016 10:03:58 +1300
Subject: [PATCH] remove extra two parameters in VisitExcluded constructor to
prevent confusion (#10593)
---
core/Tracker/VisitExcluded.php | 17 ++++---------
.../Tracker/VisitRequestProcessor.php | 3 +--
.../Mock/Tracker/RequestAuthenticated.php | 10 ++++++++
.../PHPUnit/Integration/Tracker/VisitTest.php | 24 ++++++++++---------
4 files changed, 28 insertions(+), 26 deletions(-)
create mode 100644 tests/PHPUnit/Framework/Mock/Tracker/RequestAuthenticated.php
diff --git a/core/Tracker/VisitExcluded.php b/core/Tracker/VisitExcluded.php
index 72972bf870..303d45c23b 100644
--- a/core/Tracker/VisitExcluded.php
+++ b/core/Tracker/VisitExcluded.php
@@ -28,25 +28,16 @@ class VisitExcluded
/**
* @param Request $request
- * @param bool|string $ip
- * @param bool|string $userAgent
*/
- public function __construct(Request $request, $ip = false, $userAgent = false)
+ public function __construct(Request $request)
{
$this->spamFilter = new ReferrerSpamFilter();
- if (false === $ip) {
- $ip = $request->getIp();
- }
-
- if (false === $userAgent) {
- $userAgent = $request->getUserAgent();
- }
-
$this->request = $request;
$this->idSite = $request->getIdSite();
- $this->userAgent = $userAgent;
- $this->ip = $ip;
+ $userAgent = $request->getUserAgent();
+ $this->userAgent = Common::unsanitizeInputValue($userAgent);
+ $this->ip = $request->getIp();
}
/**
diff --git a/plugins/CoreHome/Tracker/VisitRequestProcessor.php b/plugins/CoreHome/Tracker/VisitRequestProcessor.php
index 019965a958..033399e196 100644
--- a/plugins/CoreHome/Tracker/VisitRequestProcessor.php
+++ b/plugins/CoreHome/Tracker/VisitRequestProcessor.php
@@ -87,8 +87,7 @@ class VisitRequestProcessor extends RequestProcessor
// the IP is needed by isExcluded() and GoalManager->recordGoals()
$visitProperties->setProperty('location_ip', $request->getIp());
- // TODO: move VisitExcluded logic to here (or move to service class stored in DI)
- $excluded = new VisitExcluded($request, $visitProperties->getProperty('location_ip'));
+ $excluded = new VisitExcluded($request);
if ($excluded->isExcluded()) {
return true;
}
diff --git a/tests/PHPUnit/Framework/Mock/Tracker/RequestAuthenticated.php b/tests/PHPUnit/Framework/Mock/Tracker/RequestAuthenticated.php
new file mode 100644
index 0000000000..0b3c0a3555
--- /dev/null
+++ b/tests/PHPUnit/Framework/Mock/Tracker/RequestAuthenticated.php
@@ -0,0 +1,10 @@
+<?php
+namespace Piwik\Tests\Framework\Mock\Tracker;
+
+use Piwik\Tracker\Request;
+
+class RequestAuthenticated extends Request
+{
+ protected $isAuthenticated = true;
+
+}
\ No newline at end of file
diff --git a/tests/PHPUnit/Integration/Tracker/VisitTest.php b/tests/PHPUnit/Integration/Tracker/VisitTest.php
index f4bc1fd5ed..b1b42eb5b2 100644
--- a/tests/PHPUnit/Integration/Tracker/VisitTest.php
+++ b/tests/PHPUnit/Integration/Tracker/VisitTest.php
@@ -16,6 +16,7 @@ use Piwik\Plugin\Manager;
use Piwik\Plugins\SitesManager\API;
use Piwik\Tests\Framework\Fixture;
use Piwik\Tests\Framework\Mock\FakeAccess;
+use Piwik\Tests\Framework\Mock\Tracker\RequestAuthenticated;
use Piwik\Tracker\Request;
use Piwik\Tracker\Visit;
use Piwik\Tracker\VisitExcluded;
@@ -88,14 +89,14 @@ class VisitTest extends IntegrationTestCase
$idsite = API::getInstance()->addSite("name", "http://piwik.net/", $ecommerce = 0,
$siteSearch = 1, $searchKeywordParameters = null, $searchCategoryParameters = null, $excludedIp);
- $request = new Request(array('idsite' => $idsite));
+ $request = new RequestAuthenticated(array('idsite' => $idsite));
// test that IPs within the range, or the given IP, are excluded
foreach ($tests as $ip => $expected) {
- $testIpIsExcluded = IPUtils::stringToBinaryIP($ip);
+ $request->setParam('cip', $ip);
- $excluded = new VisitExcluded_public($request, $testIpIsExcluded);
- $this->assertSame($expected, $excluded->public_isVisitorIpExcluded($testIpIsExcluded));
+ $excluded = new VisitExcluded_public($request);
+ $this->assertSame($expected, $excluded->public_isVisitorIpExcluded($ip));
}
}
@@ -176,13 +177,12 @@ class VisitTest extends IntegrationTestCase
$idsite = API::getInstance()->addSite("name", "http://piwik.net/", $ecommerce = 0,
$siteSearch = 1, $searchKeywordParameters = null, $searchCategoryParameters = null);
- $request = new Request(array('idsite' => $idsite));
- $testIpIsExcluded = IPUtils::stringToBinaryIP($ip);
+ $request = new RequestAuthenticated(array('idsite' => $idsite, 'cip' => $ip));
$_SERVER['HTTP_VIA'] = '1.1 Chrome-Compression-Proxy';
- $excluded = new VisitExcluded_public($request, $testIpIsExcluded);
- $isBot = $excluded->public_isNonHumanBot($testIpIsExcluded);
+ $excluded = new VisitExcluded_public($request);
+ $isBot = $excluded->public_isNonHumanBot();
unset($_SERVER['HTTP_VIA']);
$this->assertSame($isNonHumanBot, $isBot);
}
@@ -239,7 +239,8 @@ class VisitTest extends IntegrationTestCase
// test that user agents that contain excluded user agent strings are excluded
foreach ($tests as $ua => $expected) {
- $excluded = new VisitExcluded_public($request, $ip = false, $ua);
+ $request->setParam('ua', $ua);
+ $excluded = new VisitExcluded_public($request);
$this->assertSame($expected, $excluded->public_isUserAgentExcluded(), "Result if isUserAgentExcluded('$ua') was not " . ($expected ? 'true' : 'false') . ".");
}
@@ -306,10 +307,11 @@ class VisitTest extends IntegrationTestCase
);
$idsite = API::getInstance()->addSite("name", "http://piwik.net/");
- $request = new Request(array('idsite' => $idsite, 'bots' => 0));
+ $request = new RequestAuthenticated(array('idsite' => $idsite, 'bots' => 0));
foreach ($isIpBot as $ip => $isBot) {
- $excluded = new VisitExcluded_public($request, IPUtils::stringToBinaryIP($ip));
+ $request->setParam('cip', $ip);
+ $excluded = new VisitExcluded_public($request);
$this->assertSame($isBot, $excluded->public_isNonHumanBot(), $ip);
}
--
GitLab