Make sure BulkTracking skips requests for nonexistant sites so other requests will still be tracked. Includes new tests in BulkTracking plugin.

7 files changed, 77 insertions, 13 deletions

diff --git a/plugins/BulkTracking/Tracker/Handler.php b/plugins/BulkTracking/Tracker/Handler.php
index e260237093..db368a315e 100644
--- a/plugins/BulkTracking/Tracker/Handler.php
+++ b/plugins/BulkTracking/Tracker/Handler.php
@@ -9,6 +9,7 @@
 
 namespace Piwik\Plugins\BulkTracking\Tracker;
 
+use Piwik\Exception\UnexpectedWebsiteFoundException;
 use Piwik\Tracker;
 use Piwik\Tracker\RequestSet;
 use Piwik\Tracker\TrackerConfig;
@@ -37,6 +38,22 @@ class Handler extends Tracker\Handler
         // Do not run schedule task if we are importing logs or doing custom tracking (as it could slow down)
     }
 
+    public function process(Tracker $tracker, RequestSet $requestSet)
+    {
+        $invalidRequests = 0;
+        foreach ($requestSet->getRequests() as $request) {
+            try {
+                $tracker->trackRequest($request);
+            } catch (UnexpectedWebsiteFoundException $ex) {
+                $invalidRequests += 1;
+            }
+        }
+
+        /** @var Response $response */
+        $response = $this->getResponse();
+        $response->setInvalidCount($invalidRequests);
+    }
+
     public function onException(Tracker $tracker, RequestSet $requestSet, Exception $e)
     {
         $this->rollbackTransaction();
diff --git a/plugins/BulkTracking/Tracker/Response.php b/plugins/BulkTracking/Tracker/Response.php
index fd3e304a5e..816397cfd0 100644
--- a/plugins/BulkTracking/Tracker/Response.php
+++ b/plugins/BulkTracking/Tracker/Response.php
@@ -15,6 +15,11 @@ use Piwik\Tracker;
 class Response extends Tracker\Response
 {
     /**
+     * @var int
+     */
+    private $invalidRequests = 0;
+
+    /**
      * Echos an error message & other information, then exits.
      *
      * @param Tracker $tracker
@@ -55,7 +60,8 @@ class Response extends Tracker\Response
         // when doing bulk tracking we return JSON so the caller will know how many succeeded
         $result = array(
             'status'  => 'error',
-            'tracked' => $tracker->getCountOfLoggedRequests()
+            'tracked' => $tracker->getCountOfLoggedRequests(),
+            'invalid' => $this->invalidRequests,
         );
 
         // send error when in debug mode
@@ -70,8 +76,13 @@ class Response extends Tracker\Response
     {
         return array(
             'status' => 'success',
-            'tracked' => $tracker->getCountOfLoggedRequests()
+            'tracked' => $tracker->getCountOfLoggedRequests(),
+            'invalid' => $this->invalidRequests,
         );
     }
 
+    public function setInvalidCount($invalidRequests)
+    {
+        $this->invalidRequests = $invalidRequests;
+    }
 }
diff --git a/plugins/BulkTracking/tests/Integration/HandlerTest.php b/plugins/BulkTracking/tests/Integration/HandlerTest.php
index 1b2b46c8cb..a1992150ee 100644
--- a/plugins/BulkTracking/tests/Integration/HandlerTest.php
+++ b/plugins/BulkTracking/tests/Integration/HandlerTest.php
@@ -10,8 +10,8 @@ namespace Piwik\Plugins\BulkTracking\tests\Integration;
 
 use Piwik\Exception\InvalidRequestParameterException;
 use Piwik\Exception\UnexpectedWebsiteFoundException;
+use Piwik\Plugins\BulkTracking\tests\Mock\TrackerResponse;
 use Piwik\Tests\Framework\Fixture;
-use Piwik\Tests\Framework\Mock\Tracker\Response;
 use Piwik\Tests\Framework\Mock\Tracker\ScheduledTasksRunner;
 use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
 use Piwik\Tracker;
@@ -32,7 +32,7 @@ class HandlerTest extends IntegrationTestCase
     private $handler;
 
     /**
-     * @var Response
+     * @var TrackerResponse
      */
     private $response;
 
@@ -53,7 +53,7 @@ class HandlerTest extends IntegrationTestCase
         Fixture::createWebsite('2014-01-01 00:00:00');
         Tracker\Cache::deleteTrackerCache();
 
-        $this->response = new Response();
+        $this->response = new TrackerResponse();
         $this->handler  = new Handler();
         $this->handler->setResponse($this->response);
         $this->tracker  = new Tracker();
diff --git a/plugins/BulkTracking/tests/Integration/TrackerTest.php b/plugins/BulkTracking/tests/Integration/TrackerTest.php
index dd1e37ea70..32b26e4ec4 100644
--- a/plugins/BulkTracking/tests/Integration/TrackerTest.php
+++ b/plugins/BulkTracking/tests/Integration/TrackerTest.php
@@ -66,7 +66,7 @@ class TrackerTest extends BulkTrackingTestCase
     {
         $response = $this->tracker->main($this->getHandler(), $this->getEmptyRequestSet());
 
-        $this->assertSame('{"status":"success","tracked":2}', $response);
+        $this->assertSame('{"status":"success","tracked":2,"invalid":0}', $response);
     }
 
     public function test_main_shouldReturnErrorResponse_InCaseOfAnyError()
@@ -79,7 +79,7 @@ class TrackerTest extends BulkTrackingTestCase
 
         $response = $this->tracker->main($handler, $requestSet);
 
-        $this->assertSame('{"status":"error","tracked":0}', $response);
+        $this->assertSame('{"status":"error","tracked":0,"invalid":0}', $response);
     }
 
     public function test_main_shouldReturnErrorResponse_IfNotAuthorized()
@@ -91,7 +91,7 @@ class TrackerTest extends BulkTrackingTestCase
 
         $response = $this->tracker->main($handler, $this->getEmptyRequestSet());
 
-        $this->assertSame('{"status":"error","tracked":0}', $response);
+        $this->assertSame('{"status":"error","tracked":0,"invalid":0}', $response);
     }
 
     public function test_main_shouldActuallyTrack()
diff --git a/plugins/BulkTracking/tests/Mock/TrackerResponse.php b/plugins/BulkTracking/tests/Mock/TrackerResponse.php
new file mode 100644
index 0000000000..15eb209bc2
--- /dev/null
+++ b/plugins/BulkTracking/tests/Mock/TrackerResponse.php
@@ -0,0 +1,21 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\BulkTracking\tests\Mock;
+
+use Piwik\Tests\Framework\Mock\Tracker\Response;
+
+class TrackerResponse extends Response
+{
+    private $invalidRequests = 0;
+
+    public function setInvalidCount($invalidRequests)
+    {
+        $this->invalidRequests = $invalidRequests;
+    }
+}
\ No newline at end of file
diff --git a/plugins/BulkTracking/tests/System/TrackerTest.php b/plugins/BulkTracking/tests/System/TrackerTest.php
index 5974a1acd5..188e87e1ca 100644
--- a/plugins/BulkTracking/tests/System/TrackerTest.php
+++ b/plugins/BulkTracking/tests/System/TrackerTest.php
@@ -46,8 +46,12 @@ class TrackerTest extends SystemTestCase
         $this->tracker->doTrackPageView('Test');
         $this->tracker->doTrackPageView('Test');
 
+        // test skipping invalid site errors
+        $this->tracker->setIdSite(5);
+        $this->tracker->doTrackPageView('Test');
+
         $response = $this->tracker->doBulkTrack();
 
-        $this->assertEquals('{"status":"success","tracked":2}', $response);
+        $this->assertEquals('{"status":"success","tracked":2,"invalid":1}', $response);
     }
 }
\ No newline at end of file
diff --git a/plugins/BulkTracking/tests/Unit/ResponseTest.php b/plugins/BulkTracking/tests/Unit/ResponseTest.php
index 9da1e9a6b3..91b3df86da 100644
--- a/plugins/BulkTracking/tests/Unit/ResponseTest.php
+++ b/plugins/BulkTracking/tests/Unit/ResponseTest.php
@@ -48,7 +48,7 @@ class ResponseTest extends UnitTestCase
         $this->response->outputException($tracker, new Exception('My Custom Message'), 400);
         $content = $this->response->getOutput();
 
-        $this->assertEquals('{"status":"error","tracked":5}', $content);
+        $this->assertEquals('{"status":"error","tracked":5,"invalid":0}', $content);
     }
 
     public function test_outputException_shouldOutputDebugMessageIfEnabled()
@@ -59,7 +59,7 @@ class ResponseTest extends UnitTestCase
         $this->response->outputException($tracker, new Exception('My Custom Message'), 400);
         $content = $this->response->getOutput();
 
-        $this->assertStringStartsWith('{"status":"error","tracked":5,"message":"My Custom Message\n', $content);
+        $this->assertStringStartsWith('{"status":"error","tracked":5,"invalid":0,"message":"My Custom Message\n', $content);
     }
 
     public function test_outputResponse_shouldOutputBulkResponse()
@@ -69,7 +69,7 @@ class ResponseTest extends UnitTestCase
         $this->response->outputResponse($tracker);
         $content = $this->response->getOutput();
 
-        $this->assertEquals('{"status":"success","tracked":5}', $content);
+        $this->assertEquals('{"status":"success","tracked":5,"invalid":0}', $content);
     }
 
     public function test_outputResponse_shouldNotOutputAnything_IfExceptionResponseAlreadySent()
@@ -80,7 +80,18 @@ class ResponseTest extends UnitTestCase
         $this->response->outputResponse($tracker);
         $content = $this->response->getOutput();
 
-        $this->assertEquals('{"status":"error","tracked":5}', $content);
+        $this->assertEquals('{"status":"error","tracked":5,"invalid":0}', $content);
+    }
+
+    public function test_outputResponse_shouldOutputInvalidRequests_IfInvalidCountSet()
+    {
+        $tracker = $this->getTrackerWithCountedRequests();
+
+        $this->response->setInvalidCount(3);
+        $this->response->outputResponse($tracker);
+        $content = $this->response->getOutput();
+
+        $this->assertEquals('{"status":"success","tracked":5,"invalid":3}', $content);
     }
 
     private function getTrackerWithCountedRequests()