From 622e9a8e7191b239ba666dd1b2468b9ff819f19b Mon Sep 17 00:00:00 2001 From: diosmosis Date: Wed, 17 Jun 2015 15:45:11 -0700 Subject: Make sure BulkTracking skips requests for nonexistant sites so other requests will still be tracked. Includes new tests in BulkTracking plugin. --- plugins/BulkTracking/Tracker/Handler.php | 17 +++++++++++++++++ plugins/BulkTracking/Tracker/Response.php | 15 +++++++++++++-- .../BulkTracking/tests/Integration/HandlerTest.php | 6 +++--- .../BulkTracking/tests/Integration/TrackerTest.php | 6 +++--- plugins/BulkTracking/tests/Mock/TrackerResponse.php | 21 +++++++++++++++++++++ plugins/BulkTracking/tests/System/TrackerTest.php | 6 +++++- plugins/BulkTracking/tests/Unit/ResponseTest.php | 19 +++++++++++++++---- 7 files changed, 77 insertions(+), 13 deletions(-) create mode 100644 plugins/BulkTracking/tests/Mock/TrackerResponse.php (limited to 'plugins') 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 @@ -14,6 +14,11 @@ use Piwik\Tracker; class Response extends Tracker\Response { + /** + * @var int + */ + private $invalidRequests = 0; + /** * Echos an error message & other information, then exits. * @@ -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 @@ +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() -- cgit v1.2.3