diff --git a/plugins/BulkTracking/Tracker/Handler.php b/plugins/BulkTracking/Tracker/Handler.php index db368a315e2503c060d0cbc55f5d9bb9e803c47c..0d97a6b5b332a2c3a09c0b5bef9b9dcb0d8ab1ad 100644 --- a/plugins/BulkTracking/Tracker/Handler.php +++ b/plugins/BulkTracking/Tracker/Handler.php @@ -9,7 +9,11 @@ namespace Piwik\Plugins\BulkTracking\Tracker; +use Piwik\Archiver\Request; +use Piwik\AuthResult; +use Piwik\Container\StaticContainer; use Piwik\Exception\UnexpectedWebsiteFoundException; +use Piwik\Piwik; use Piwik\Tracker; use Piwik\Tracker\RequestSet; use Piwik\Tracker\TrackerConfig; @@ -40,18 +44,22 @@ class Handler extends Tracker\Handler public function process(Tracker $tracker, RequestSet $requestSet) { - $invalidRequests = 0; - foreach ($requestSet->getRequests() as $request) { + $isAuthenticated = $this->isBulkTrackingRequestAuthenticated($requestSet); + + /** @var Response $response */ + $response = $this->getResponse(); + $response->setIsAuthenticated($isAuthenticated); + + $invalidRequests = array(); + foreach ($requestSet->getRequests() as $index => $request) { try { $tracker->trackRequest($request); } catch (UnexpectedWebsiteFoundException $ex) { - $invalidRequests += 1; + $invalidRequests[] = $index; } } - /** @var Response $response */ - $response = $this->getResponse(); - $response->setInvalidCount($invalidRequests); + $response->setInvalidRequests($invalidRequests); } public function onException(Tracker $tracker, RequestSet $requestSet, Exception $e) @@ -96,4 +104,23 @@ class Handler extends Tracker\Handler return (bool) TrackerConfig::getConfigValue('bulk_requests_use_transaction'); } + private function isBulkTrackingRequestAuthenticated(RequestSet $requestSet) + { + $tokenAuth = $requestSet->getTokenAuth(); + if (empty($tokenAuth)) { + return false; + } + + Piwik::postEvent('Request.initAuthenticationObject'); + + /** @var \Piwik\Auth $auth */ + $auth = StaticContainer::get('Piwik\Auth'); + $auth->setTokenAuth($tokenAuth); + $auth->setLogin(null); + $auth->setPassword(null); + $auth->setPasswordHash(null); + $access = $auth->authenticate(); + + return $access->getCode() == AuthResult::SUCCESS_SUPERUSER_AUTH_CODE; + } } diff --git a/plugins/BulkTracking/Tracker/Response.php b/plugins/BulkTracking/Tracker/Response.php index 816397cfd09bd76a64351d1e39d5fc3a03156e25..e8d3667eeca0c85fbec859f1bd27b9995b1b43b3 100644 --- a/plugins/BulkTracking/Tracker/Response.php +++ b/plugins/BulkTracking/Tracker/Response.php @@ -15,9 +15,14 @@ use Piwik\Tracker; class Response extends Tracker\Response { /** - * @var int + * @var int[] */ - private $invalidRequests = 0; + private $invalidRequests = array(); + + /** + * @var bool + */ + private $isAuthenticated = false; /** * Echos an error message & other information, then exits. @@ -61,9 +66,11 @@ class Response extends Tracker\Response $result = array( 'status' => 'error', 'tracked' => $tracker->getCountOfLoggedRequests(), - 'invalid' => $this->invalidRequests, + 'invalid' => count($this->invalidRequests), ); + $this->addInvalidIndicesIfAuthenticated($result); + // send error when in debug mode if ($tracker->isDebugModeEnabled()) { $result['message'] = $this->getMessageFromException($e); @@ -74,15 +81,31 @@ class Response extends Tracker\Response private function formatResponse(Tracker $tracker) { - return array( + $result = array( 'status' => 'success', 'tracked' => $tracker->getCountOfLoggedRequests(), - 'invalid' => $this->invalidRequests, + 'invalid' => count($this->invalidRequests), ); + + $this->addInvalidIndicesIfAuthenticated($result); + + return $result; } - public function setInvalidCount($invalidRequests) + public function setInvalidRequests($invalidRequests) { $this->invalidRequests = $invalidRequests; } + + public function setIsAuthenticated($isAuthenticated) + { + $this->isAuthenticated = $isAuthenticated; + } + + private function addInvalidIndicesIfAuthenticated(&$result) + { + if ($this->isAuthenticated) { + $result['invalid_indices'] = $this->invalidRequests; + } + } } diff --git a/plugins/BulkTracking/tests/Framework/TestCase/BulkTrackingTestCase.php b/plugins/BulkTracking/tests/Framework/TestCase/BulkTrackingTestCase.php index 23b96776ae747f274093f42f8fc7214c15090246..19788b64aba1d68ab851e5bed95404cadf5d924c 100644 --- a/plugins/BulkTracking/tests/Framework/TestCase/BulkTrackingTestCase.php +++ b/plugins/BulkTracking/tests/Framework/TestCase/BulkTrackingTestCase.php @@ -80,9 +80,9 @@ class BulkTrackingTestCase extends IntegrationTestCase return $requestSet; } - protected function getDummyRequest($token = null) + protected function getDummyRequest($token = null, $idSites = array(1, 2)) { - $params = array(array('idsite' => '1', 'rec' => '1'), array('idsite' => '2', 'rec' => '1')); + $params = array(array('idsite' => $idSites[0], 'rec' => '1'), array('idsite' => $idSites[1], 'rec' => '1')); $params = array('requests' => $params); if (!is_null($token)) { diff --git a/plugins/BulkTracking/tests/Integration/TrackerTest.php b/plugins/BulkTracking/tests/Integration/TrackerTest.php index 32b26e4ec4c7ea2f5bf09cf84d693c4d2fb56ff3..a872b0524c1781ae9ad8e8b0986cb5b35aa88300 100644 --- a/plugins/BulkTracking/tests/Integration/TrackerTest.php +++ b/plugins/BulkTracking/tests/Integration/TrackerTest.php @@ -115,6 +115,30 @@ class TrackerTest extends BulkTrackingTestCase $this->assertEmpty($this->getIdVisit(3)); } + public function test_main_shouldReportInvalidIndices_IfInvalidRequestsIncluded_AndRequestAuthenticated() + { + $this->injectRawDataToBulk($this->getDummyRequest($token = Fixture::getTokenAuth(), $idSite = array(1, -100))); + + $handler = $this->getHandler(); + $handler->setResponse(new Response()); + + $response = $this->tracker->main($handler, $this->getEmptyRequestSet()); + + $this->assertEquals('{"status":"success","tracked":1,"invalid":1,"invalid_indices":[1]}', $response); + } + + public function test_main_shouldReportInvalidCount_IfInvalidRequestsIncluded_AndRequestNotAuthenticated() + { + $this->injectRawDataToBulk($this->getDummyRequest($token = null, $idSite = array(1, -100))); + + $handler = $this->getHandler(); + $handler->setResponse(new Response()); + + $response = $this->tracker->main($handler, $this->getEmptyRequestSet()); + + $this->assertEquals('{"status":"success","tracked":1,"invalid":1}', $response); + } + private function getHandler() { return Tracker\Handler\Factory::make(); @@ -130,4 +154,10 @@ class TrackerTest extends BulkTrackingTestCase return Tracker::getDatabase()->fetchRow("SELECT * FROM " . Common::prefixTable('log_visit') . " WHERE idvisit = ?", array($idVisit)); } + protected static function configureFixture($fixture) + { + parent::configureFixture($fixture); + + $fixture->createSuperUser = true; + } } \ No newline at end of file diff --git a/plugins/BulkTracking/tests/Mock/TrackerResponse.php b/plugins/BulkTracking/tests/Mock/TrackerResponse.php index 15eb209bc2a0d49511a2c5ca0c4b23afcd4c3a9b..9b6f9564d8f17d4b6c053058852294d353a29c98 100644 --- a/plugins/BulkTracking/tests/Mock/TrackerResponse.php +++ b/plugins/BulkTracking/tests/Mock/TrackerResponse.php @@ -12,10 +12,20 @@ use Piwik\Tests\Framework\Mock\Tracker\Response; class TrackerResponse extends Response { - private $invalidRequests = 0; + private $invalidRequests = array(); - public function setInvalidCount($invalidRequests) + /** + * @var bool + */ + private $isAuthenticated = false; + + public function setInvalidRequests($invalidRequests) { $this->invalidRequests = $invalidRequests; } + + public function setIsAuthenticated($isAuthenticated) + { + $this->isAuthenticated = $isAuthenticated; + } } \ No newline at end of file diff --git a/plugins/BulkTracking/tests/System/TrackerTest.php b/plugins/BulkTracking/tests/System/TrackerTest.php index 188e87e1cac2aea94527f7ae3f6f95715aa7cf21..aabef2296fa7c562726914813e76dd98d4cbc3c0 100644 --- a/plugins/BulkTracking/tests/System/TrackerTest.php +++ b/plugins/BulkTracking/tests/System/TrackerTest.php @@ -50,8 +50,15 @@ class TrackerTest extends SystemTestCase $this->tracker->setIdSite(5); $this->tracker->doTrackPageView('Test'); + $this->tracker->setIdSite(1); + $this->tracker->doTrackPageView('Test'); + + // another invalid one to further test the invalid request indices in the result + $this->tracker->setIdSite(7); + $this->tracker->doTrackPageView('Test'); + $response = $this->tracker->doBulkTrack(); - $this->assertEquals('{"status":"success","tracked":2,"invalid":1}', $response); + $this->assertEquals('{"status":"success","tracked":3,"invalid":2,"invalid_indices":[2,4]}', $response); } } \ No newline at end of file diff --git a/plugins/BulkTracking/tests/Unit/ResponseTest.php b/plugins/BulkTracking/tests/Unit/ResponseTest.php index 91b3df86da0c57a22340483964192bb94aaafff7..38c3c0cce17e25c7e442f5a75a575ef52b0bd077 100644 --- a/plugins/BulkTracking/tests/Unit/ResponseTest.php +++ b/plugins/BulkTracking/tests/Unit/ResponseTest.php @@ -83,17 +83,41 @@ class ResponseTest extends UnitTestCase $this->assertEquals('{"status":"error","tracked":5,"invalid":0}', $content); } - public function test_outputResponse_shouldOutputInvalidRequests_IfInvalidCountSet() + public function test_outputResponse_shouldIncludeInvalidIndices_IfExceptionSet_AndRequestAuthenticated() { $tracker = $this->getTrackerWithCountedRequests(); - $this->response->setInvalidCount(3); + $this->response->setInvalidRequests(array(10, 20)); + $this->response->setIsAuthenticated(true); + $this->response->outputException($tracker, new Exception('My Custom Message'), 400); + $content = $this->response->getOutput(); + + $this->assertEquals('{"status":"error","tracked":5,"invalid":2,"invalid_indices":[10,20]}', $content); + } + + public function test_outputResponse_shouldOutputInvalidRequests_IfInvalidIndicesSet_AndRequestNotAuthenticated() + { + $tracker = $this->getTrackerWithCountedRequests(); + + $this->response->setInvalidRequests(array(5, 63, 72)); $this->response->outputResponse($tracker); $content = $this->response->getOutput(); $this->assertEquals('{"status":"success","tracked":5,"invalid":3}', $content); } + public function test_outputResponse_shouldOutputInvalidRequests_IfInvalidIndicesSet_AndRequestAuthenticated() + { + $tracker = $this->getTrackerWithCountedRequests(); + + $this->response->setInvalidRequests(array(5, 63, 72)); + $this->response->setIsAuthenticated(true); + $this->response->outputResponse($tracker); + $content = $this->response->getOutput(); + + $this->assertEquals('{"status":"success","tracked":5,"invalid":3,"invalid_indices":[5,63,72]}', $content); + } + private function getTrackerWithCountedRequests() { $tracker = new Tracker(); diff --git a/tests/PHPUnit/Fixtures/ManySitesImportedLogs.php b/tests/PHPUnit/Fixtures/ManySitesImportedLogs.php index 133f4908891649217902c6ebc47822f822211d92..3a107c26bd6596ae7c247d3ece3fd8e74bda0f4a 100644 --- a/tests/PHPUnit/Fixtures/ManySitesImportedLogs.php +++ b/tests/PHPUnit/Fixtures/ManySitesImportedLogs.php @@ -192,7 +192,7 @@ class ManySitesImportedLogs extends Fixture * Logs a couple visits for the site we created and two new sites that do not * exist yet. Visits are from Aug 12, 13 & 14 of 2012. */ - public function logVisitsWithDynamicResolver() + public function logVisitsWithDynamicResolver($maxPayloadSize = 1) { $logFile = PIWIK_INCLUDE_PATH . '/tests/resources/access-logs/fake_logs_dynamic.log'; # log file @@ -201,8 +201,8 @@ class ManySitesImportedLogs extends Fixture $opts = array('--add-sites-new-hosts' => false, '--enable-testmode' => false, '--recorders' => '1', - '--recorder-max-payload-size' => '1'); - self::executeLogImporter($logFile, $opts); + '--recorder-max-payload-size' => $maxPayloadSize); + return implode("\n", self::executeLogImporter($logFile, $opts)); } /** diff --git a/tests/PHPUnit/System/ImportLogsTest.php b/tests/PHPUnit/System/ImportLogsTest.php index 952154b5353b222b5432cd0dde6d5e853b22b2aa..725b2fba1630cd1dd9ac1a0604d7e62e119b0a2b 100755 --- a/tests/PHPUnit/System/ImportLogsTest.php +++ b/tests/PHPUnit/System/ImportLogsTest.php @@ -8,11 +8,14 @@ namespace Piwik\Tests\System; use Piwik\Access; +use Piwik\Common; use Piwik\Plugins\SitesManager\API; use Piwik\Tests\Framework\Fixture; use Piwik\Tests\Framework\TestCase\SystemTestCase; use Piwik\Tests\Fixtures\ManySitesImportedLogs; use Piwik\Tests\Framework\TestingEnvironmentVariables; +use Piwik\Tracker\Request; +use Piwik\Tracker\RequestSet; /** * Tests the log importer. @@ -103,10 +106,15 @@ class ImportLogsTest extends SystemTestCase /** * NOTE: This test must be last since the new sites that get added are added in * random order. + * NOTE: This test combines two tests in order to avoid executing the log importer another time. + * If the log importer were refactored, the invalid requests test could be a unit test in + * python. */ - public function testDynamicResolverSitesCreated() + public function test_LogImporter_CreatesSitesWhenDynamicResolverUsed_AndReportsOnInvalidRequests() { - self::$fixture->logVisitsWithDynamicResolver(); + $this->simulateInvalidTrackerRequest(); + + $output = self::$fixture->logVisitsWithDynamicResolver($maxPayloadSize = 3); // reload access so new sites are viewable Access::getInstance()->setSuperUserAccess(true); @@ -120,6 +128,10 @@ class ImportLogsTest extends SystemTestCase $whateverDotCom = API::getInstance()->getSitesIdFromSiteUrl('http://whatever.com'); $this->assertEquals(1, count($whateverDotCom)); + + // make sure invalid requests are reported correctly + $this->assertContains('The Piwik tracker identified 2 invalid requests on lines: 10, 11', $output); + $this->assertContains("The following lines were not tracked by Piwik, either due to a malformed tracker request or error in the tracker:\n\n10, 11", $output); } public function test_LogImporter_RetriesWhenServerFails() @@ -163,23 +175,53 @@ class ImportLogsTest extends SystemTestCase { $testingEnvironment = new TestingEnvironmentVariables(); $testingEnvironment->_triggerTrackerFailure = null; + $testingEnvironment->_triggerInvalidRequests = null; $testingEnvironment->save(); } + private function simulateInvalidTrackerRequest() + { + $testEnvironment = new TestingEnvironmentVariables(); + $testEnvironment->_triggerInvalidRequests = true; + $testEnvironment->save(); + } + public static function provideContainerConfigBeforeClass() { $result = array(); + $observers = array(); $testingEnvironment = new TestingEnvironmentVariables(); if ($testingEnvironment->_triggerTrackerFailure) { - $result['observers.global'] = \DI\add(array( - array('Tracker.newHandler', function () { - @http_response_code(500); + $observers[] = array('Tracker.newHandler', function () { + @http_response_code(500); - throw new \Exception("injected exception"); - }) - )); + throw new \Exception("injected exception"); + }); } + + if ($testingEnvironment->_triggerInvalidRequests) { + // we trigger an invalid request by checking for triggerInvalid=1 in a request, and if found replacing the + // request w/ a request that has an nonexistent idsite + $observers[] = array('Tracker.initRequestSet', function (RequestSet $requestSet) { + $requests = $requestSet->getRequests(); + foreach ($requests as $index => $request) { + $url = $request->getParam('url'); + if (strpos($url, 'triggerInvalid=1') !== false) { + $newParams = $request->getParams(); + $newParams['idsite'] = 1000; + + $requests[$index] = new Request($newParams); + } + } + $requestSet->setRequests($requests); + }); + } + + if (!empty($observers)) { + $result['observers.global'] = \DI\add($observers); + } + return $result; } } diff --git a/tests/resources/access-logs/fake_logs_dynamic.log b/tests/resources/access-logs/fake_logs_dynamic.log index 75ef54c1d454d14a47d2072390c40c549fe2e370..04ad6c23849cd1af0c0d8c550f3984d3b23d4d8c 100644 --- a/tests/resources/access-logs/fake_logs_dynamic.log +++ b/tests/resources/access-logs/fake_logs_dynamic.log @@ -8,8 +8,8 @@ whatever.com 72.44.32.10 - - [12/Aug/2012:15:49:48 +0200] "GET /translations/ HT piwik.net 175.41.192.09 - - [12/Aug/2012:22:56:45 +0200] "GET /docs/ HTTP/1.1" 200 3574 "-" "Mozilla/5.0 (X11; Linux i686; rv:6.0) Gecko/20100101 Firefox/6.0" piwik.net 175.41.192.09 - - [12/Aug/2012:23:00:42 +0200] "GET /docs/manage-users/ HTTP/1.1" 200 3574 "-" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_0) AppleWebKit/536.3 (KHTML, like Gecko) Chrome/19.0.1063.0 Safari/536.3" whatever.com 79.125.00.21 - - [13/Aug/2012:20:03:40 +0200] "GET /newsletter/ HTTP/1.1" 200 3574 "-" "Mozilla/5.0 (compatible; MSIE 10.0; Windows NT 6.1; Trident/5.0)" -whatever.com 175.41.192.34 - - [13/Aug/2012:21:59:50 +0200] "GET /faq/how-to/ HTTP/1.1" 200 3574 "-" "Mozilla/5.0 (compatible; MSIE 10.0; Windows NT 6.1; Trident/5.0)" -anothersite.com 175.41.192.34 - - [13/Aug/2012:22:01:17 +0200] "GET /faq/ HTTP/1.1" 200 3574 "-" "Mozilla/5.0 (X11; U; Linux x86_64; fr-FR) AppleWebKit/534.7 (KHTML, like Gecko) Epiphany/2.30.6 Safari/534.7" +whatever.com 175.41.192.34 - - [13/Aug/2012:21:59:50 +0200] "GET /faq/how-to/?triggerInvalid=1 HTTP/1.1" 200 3574 "-" "Mozilla/5.0 (compatible; MSIE 10.0; Windows NT 6.1; Trident/5.0)" +anothersite.com 175.41.192.34 - - [13/Aug/2012:22:01:17 +0200] "GET /faq/?triggerInvalid=1 HTTP/1.1" 200 3574 "-" "Mozilla/5.0 (X11; U; Linux x86_64; fr-FR) AppleWebKit/534.7 (KHTML, like Gecko) Epiphany/2.30.6 Safari/534.7" anothersite.com 177.71.128.21 - - [13/Aug/2012:22:21:03 +0200] "GET /docs/manage-websites/ HTTP/1.1" 200 3574 "-" "Mozilla/5.0 (compatible; MSIE 10.6; Windows NT 6.1; Trident/5.0; InfoPath.2; SLCC1; .NET CLR 3.0.4506.2152; .NET CLR 3.5.30729; .NET CLR 2.0.50727) 3gpp-gba UNTRUSTED/1.0" anothersite.com 177.71.128.21 - - [13/Aug/2012:22:21:28 +0200] "GET /intranet-analytics/ HTTP/1.1" 200 3574 "-" "Mozilla/5.0 (X11; U; Linux x86_64; fr-FR) AppleWebKit/534.7 (KHTML, like Gecko) Epiphany/2.30.6 Safari/534.7" whatever.com 177.71.128.21 - - [13/Aug/2012:22:22:08 +0200] "GET /blog/2012/08/survey-your-opinion-matters/ HTTP/1.1" 200 3574 "-" "Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/536.6 (KHTML, like Gecko) Chrome/20.0.1092.0 Safari/536.6"