Skip to content

Commit

Permalink
Merge pull request #8132 from piwik/bulk_track_ignore_errors
Browse files Browse the repository at this point in the history
Make sure BulkTracking skips requests for nonexistant sites.
  • Loading branch information
diosmosis committed Jun 22, 2015
2 parents cb8b4d4 + f7a0c26 commit e4a75a3
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 14 deletions.
17 changes: 17 additions & 0 deletions plugins/BulkTracking/Tracker/Handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace Piwik\Plugins\BulkTracking\Tracker;

use Piwik\Exception\UnexpectedWebsiteFoundException;
use Piwik\Tracker;
use Piwik\Tracker\RequestSet;
use Piwik\Tracker\TrackerConfig;
Expand Down Expand Up @@ -37,6 +38,22 @@ public function onAllRequestsTracked(Tracker $tracker, RequestSet $requestSet)
// 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();
Expand Down
15 changes: 13 additions & 2 deletions plugins/BulkTracking/Tracker/Response.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@

class Response extends Tracker\Response
{
/**
* @var int
*/
private $invalidRequests = 0;

/**
* Echos an error message & other information, then exits.
*
Expand Down Expand Up @@ -55,7 +60,8 @@ private function formatException(Tracker $tracker, Exception $e)
// 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
Expand All @@ -70,8 +76,13 @@ private function formatResponse(Tracker $tracker)
{
return array(
'status' => 'success',
'tracked' => $tracker->getCountOfLoggedRequests()
'tracked' => $tracker->getCountOfLoggedRequests(),
'invalid' => $this->invalidRequests,
);
}

public function setInvalidCount($invalidRequests)
{
$this->invalidRequests = $invalidRequests;
}
}
6 changes: 3 additions & 3 deletions plugins/BulkTracking/tests/Integration/HandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@

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;
Expand All @@ -32,7 +32,7 @@ class HandlerTest extends IntegrationTestCase
private $handler;

/**
* @var Response
* @var TrackerResponse
*/
private $response;

Expand All @@ -53,7 +53,7 @@ public function setUp()
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();
Expand Down
6 changes: 3 additions & 3 deletions plugins/BulkTracking/tests/Integration/TrackerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public function test_main_shouldReturnBulkTrackingResponse()
{
$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()
Expand All @@ -79,7 +79,7 @@ public function test_main_shouldReturnErrorResponse_InCaseOfAnyError()

$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()
Expand All @@ -91,7 +91,7 @@ public function test_main_shouldReturnErrorResponse_IfNotAuthorized()

$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()
Expand Down
21 changes: 21 additions & 0 deletions plugins/BulkTracking/tests/Mock/TrackerResponse.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
6 changes: 5 additions & 1 deletion plugins/BulkTracking/tests/System/TrackerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,12 @@ public function test_response_ShouldContainBulkTrackingApiResponse()
$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);
}
}
19 changes: 15 additions & 4 deletions plugins/BulkTracking/tests/Unit/ResponseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public function test_outputException_shouldOutputBulkResponse()
$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()
Expand All @@ -59,7 +59,7 @@ public function test_outputException_shouldOutputDebugMessageIfEnabled()
$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()
Expand All @@ -69,7 +69,7 @@ public function test_outputResponse_shouldOutputBulkResponse()
$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()
Expand All @@ -80,7 +80,18 @@ public function test_outputResponse_shouldNotOutputAnything_IfExceptionResponseA
$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()
Expand Down
3 changes: 2 additions & 1 deletion tests/PHPUnit/Framework/TestCase/UnitTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
use Piwik\Tests\Framework\Mock\File;

/**
* Base class for Unit tests.
* Base class for Unit tests. Use this if you need to use the DI container in tests. It will be created fresh
* before each test.
*
* @deprecated Unit tests don't need no environment.
*
Expand Down

0 comments on commit e4a75a3

Please sign in to comment.