remove extra two parameters in VisitExcluded constructor to prevent confusion (#10593)

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();
     }
 
     /**

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;
         }

tests/PHPUnit/Framework/Mock/Tracker/RequestAuthenticated.php

+<?php
+namespace Piwik\Tests\Framework\Mock\Tracker;
+
+use Piwik\Tracker\Request;
+
+class RequestAuthenticated extends Request
+{
+    protected $isAuthenticated = true;
+
+}
\ No newline at end of file

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);
         }