Added new integration test for archive.php cron script.

Notes:
  - Modified ManySitesImportedLogs fixture to add segments if desired.
  - Switch to synchronous execution of curl requests until deadlocking issue in archive processing is dealt with.
  - Fixed bug in LogAggregator where if a segment required a subquery, archiving would failed.

core/DataAccess/LogAggregator.php

@@ -229,14 +229,30 @@ class Piwik_DataAccess_LogAggregator
             if ($selectAsString == $field
                 && $isKnownField
             ) {
-                $field = "$tableName.$field";
+                $field = $this->prefixColumn($field, $tableName);
             }
             if ($appendSelectAs && $selectAsString) {
-                $field = $field . $this->getSelectAliasAs($selectAsString);
+                $field = $this->prefixColumn($field, $tableName) . $this->getSelectAliasAs($selectAsString);
             }
         }
         return $dimensions;
     }
+    
+    /**
+     * Prefixes a column name with a table name if not already done.
+     * 
+     * @param string $column eg, 'location_provider'
+     * @param string $tableName eg, 'log_visit'
+     * @return string eg, 'log_visit.location_provider'
+     */
+    private function prefixColumn($column, $tableName)
+    {
+        if (strpos($column, '.') === false) {
+            return $tableName . '.' . $column;
+        } else {
+            return $column;
+        }
+    }
 
     protected function isFieldFunctionOrComplexExpression($field)
     {
@@ -486,4 +502,4 @@ class Piwik_DataAccess_LogAggregator
     {
         return Zend_Registry::get('db');
     }
-}
\ No newline at end of file
+}

core/Piwik.php

@@ -35,6 +35,13 @@ class Piwik
         'year'  => 4,
         'range' => 5,
     );
+    
+    /**
+     * @see getKnownSegmentsToArchive
+     * 
+     * @var array
+     */
+    public static $cachedKnownSegmentsToArchive = null;
 
     const LABEL_ID_GOAL_IS_ECOMMERCE_CART = 'ecommerceAbandonedCart';
     const LABEL_ID_GOAL_IS_ECOMMERCE_ORDER = 'ecommerceOrder';
@@ -1527,18 +1534,16 @@ class Piwik
      */
     static public function getKnownSegmentsToArchive()
     {
-        static $cachedResult = null;
-
-        if (is_null($cachedResult)) {
+        if (self::$cachedKnownSegmentsToArchive === null) {
             $segments = Piwik_Config::getInstance()->Segments;
             $cachedResult = isset($segments['Segments']) ? $segments['Segments'] : array();
 
             Piwik_PostEvent('Piwik.getKnownSegmentsToArchiveAllSites', array(&$cachedResult));
 
-            $cachedResult = array_unique($cachedResult);
+            self::$cachedKnownSegmentsToArchive = array_unique($cachedResult);
         }
 
-        return $cachedResult;
+        return self::$cachedKnownSegmentsToArchive;
     }
 
     static public function getKnownSegmentsToArchiveForSite($idSite)
@@ -2520,4 +2525,16 @@ class Piwik
         return false; // not NFS, or we can't run a program to find out
     }
 
+    /**
+     * Returns the option name of the option that stores the time the archive.php
+     * script was last run.
+     * 
+     * @param string $period
+     * @param string $idSite
+     * @return string
+     */
+    public static function getArchiveCronLastRunOptionName($period, $idSite)
+    {
+        return "lastRunArchive" . $period . "_" . $idSite;
+    }
 }

misc/cron/archive.php

@@ -46,12 +46,14 @@ Ideas for improvements:
 	- Core: check that on first day of month, if request last month from UI, 
 	  it returns last temporary monthly report generated, if the last month haven't yet been processed / finalized
  */
+
 define('PIWIK_INCLUDE_PATH', realpath(dirname(__FILE__) . "/../.."));
 define('PIWIK_USER_PATH', PIWIK_INCLUDE_PATH);
 define('PIWIK_ENABLE_DISPATCH', false);
 define('PIWIK_ENABLE_ERROR_HANDLER', false);
 define('PIWIK_ENABLE_SESSION_START', false);
 define('PIWIK_MODE_ARCHIVE', true);
+
 require_once PIWIK_INCLUDE_PATH . "/index.php";
 require_once PIWIK_INCLUDE_PATH . "/core/API/Request.php";
 
@@ -165,7 +167,7 @@ class Archiving
 
     private function lastRunKey($idsite, $period)
     {
-        return "lastRunArchive" . $period . "_" . $idsite;
+        return Piwik::getArchiveCronLastRunOptionName($period, $idsite);
     }
 
     /**
@@ -417,7 +419,6 @@ class Archiving
         // already processed above for "day"
         if ($period != "day") {
             $ch = $this->getNewCurlHandle($url);
-            $this->addCurlHandleToMulti($mh, $ch);
             $aCurl[$url] = $ch;
             $this->requests++;
         }
@@ -425,7 +426,6 @@ class Archiving
         foreach ($this->getSegmentsForSite($idsite) as $segment) {
             $segmentUrl = $url . '&segment=' . urlencode($segment);
             $ch = $this->getNewCurlHandle($segmentUrl);
-            $this->addCurlHandleToMulti($mh, $ch);
             $aCurl[$segmentUrl] = $ch;
             $this->requests++;
         }
@@ -434,14 +434,13 @@ class Archiving
         $visitsAllDaysInPeriod = false;
 
         if (!empty($aCurl)) {
-            $running = null;
-            do {
-                usleep(10000);
-                curl_multi_exec($mh, $running);
-            } while ($running > 0);
-
+            // FIXME: This code used to execute multiple curl requests asynchronously. This caused
+            // deadlocks since archive tables are locked for the entire archiving process. Moving back
+            // to synchronous requests is a quick fix, but the locking mechanism can be changed to
+            // only lock when getting the new archive ID. When that is done, this code should be changed
+            // back to use asnychronous requests.
             foreach ($aCurl as $url => $ch) {
-                $content = curl_multi_getcontent($ch);
+                $content = curl_exec($ch);
                 $successResponse = $this->checkResponse($content, $url);
                 $success = $successResponse && $success;
                 if ($url == $urlNoSegment
@@ -453,12 +452,8 @@ class Archiving
                     }
                     $visitsAllDaysInPeriod = @array_sum($stats);
                 }
+                curl_close($ch);
             }
-
-            foreach ($aCurl as $ch) {
-                curl_multi_remove_handle($mh, $ch);
-            }
-            curl_multi_close($mh);
         }
 
         $this->log("Archived website id = $idsite, period = $period, "

tests/PHPUnit/Fixtures/ManySitesImportedLogs.php

@@ -15,6 +15,9 @@ class Test_Piwik_Fixture_ManySitesImportedLogs extends Test_Piwik_BaseFixture
     public $idSite = 1;
     public $idSite2 = 2;
     public $idGoal = 1;
+    public $segments = null; // should be array mapping segment name => segment definition
+    
+    public $addSegments = false;
 
     public function setUp()
     {
@@ -26,6 +29,7 @@ class Test_Piwik_Fixture_ManySitesImportedLogs extends Test_Piwik_BaseFixture
         Piwik_UserCountry_LocationProvider::setCurrentProvider('geoip_php');
 
         $this->trackVisits();
+        $this->setupSegments();
     }
 
     public function tearDown()
@@ -43,6 +47,24 @@ class Test_Piwik_Fixture_ManySitesImportedLogs extends Test_Piwik_BaseFixture
         self::createWebsite($this->dateTime, $ecommerce = 0, $siteName = 'Piwik test two',
             $siteUrl = 'http://example-site-two.com');
     }
+    
+    public function getDefaultSegments()
+    {
+        return array(
+            'segmentOnlyOneSite'   => array('definition'      => 'browserCode==IE',
+                                            'idSite'          => $this->idSite,
+                                            'autoArchive'     => true,
+                                            'enabledAllUsers' => true),
+            'segmentNoAutoArchive' => array('definition'      => 'customVariableName1==Not-bot',
+                                            'idSite'          => false,
+                                            'autoArchive'     => false,
+                                            'enabledAllUsers' => true),
+            'segmentOnlySuperuser' => array('definition'      => 'customVariablePageName1==HTTP-code',
+                                            'idSite'          => false,
+                                            'autoArchive'     => true,
+                                            'enabledAllUsers' => false),
+        );
+    }
 
     private function trackVisits()
     {
@@ -51,6 +73,37 @@ class Test_Piwik_Fixture_ManySitesImportedLogs extends Test_Piwik_BaseFixture
         $this->replayLogFile();
         $this->logCustomFormat();
     }
+    
+    private function setupSegments()
+    {
+        if (!$this->addSegments) {
+            return;
+        }
+        
+        if ($this->segments === null) {
+            $this->segments = $this->getDefaultSegments();
+        }
+        
+        foreach ($this->segments as $segmentName => $info) {
+            $idSite = false;
+            if (isset($info['idSite'])) {
+                $idSite = $info['idSite'];
+            }
+            
+            $autoArchive = true;
+            if (isset($info['autoArchive'])) {
+                $autoArchive = $info['autoArchive'];
+            }
+            
+            $enabledAllUsers = true;
+            if (isset($info['enabledAllUsers'])) {
+                $enabledAllUsers = $info['enabledAllUsers'];
+            }
+            
+            Piwik_SegmentEditor_API::getInstance()->add(
+                $segmentName, $info['definition'], $idSite, $autoArchive, $enabledAllUsers);
+        }
+    }
 
     /**
      * Logs a couple visits for Aug 9, Aug 10, Aug 11 of 2012, for site we create.
@@ -135,5 +188,4 @@ class Test_Piwik_Fixture_ManySitesImportedLogs extends Test_Piwik_BaseFixture
 
         self::executeLogImporter($logFile, $opts);
     }
-
 }

tests/PHPUnit/Integration/ArchiveCronTest.php

+<?php
+/**
+ * Piwik - Open source web analytics
+ *
+ * @link    http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+/**
+ * Tests the archive.php cron script.
+ */
+class Test_Piwik_Integration_ArchiveCronTest extends IntegrationTestCase
+{
+    public static $fixture = null; // initialized below class definition
+    
+    public function getApiForTesting()
+    {
+        $results = array();
+        $results[] = array('VisitsSummary.get', array('idSite'  => 'all',
+                                                      'date'    => '2012-08-09',
+                                                      'periods' => array('day', 'week', 'month', 'year')));
+
+        foreach (self::$fixture->getDefaultSegments() as $segmentName => $info) {
+            $results[] = array('VisitsSummary.get', array('idSite'     => 'all',
+                                                          'date'       => '2012-08-09',
+                                                          'periods'    => array('day', 'week', 'month', 'year'),
+                                                          'segment'    => $info['definition'],
+                                                          'testSuffix' => '_' . $segmentName));
+        }
+        
+        $results[] = array('VisitsSummary.get', array('idSite'     => 'all',
+                                                      'date'       => '2012-08-09',
+                                                      'periods'    => array('day', 'week', 'month', 'year'),
+                                                      'segment'    => 'browserCode==EP',
+                                                      'testSuffix' => '_nonPreArchivedSegment'));
+        
+        return $results;
+    }
+    
+    public function getArchivePhpCronOptionsToTest()
+    {
+        return array(
+            array('noOptions', array()),
+            // segment archiving makes calling the script more than once impractical. if all 4 are
+            // called, this test can take up to 13min to complete.
+            /*array('forceAllWebsites', array('--force-all-websites' => false)),
+            array('forceAllPeriods_lastDay', array('--force-all-periods' => '86400')),
+            array('forceAllPeriods_allTime', array('--force-all-periods' => false)),*/
+        );
+    }
+    
+    /**
+     * @dataProvider getArchivePhpCronOptionsToTest
+     * @group        Integration
+     * @group        ImportLogs
+     */
+    public function testArchivePhpCron($optionGroupName, $archivePhpOptions)
+    {
+        self::deleteArchiveTables();
+        $this->setLastRunArchiveOptions();
+        $this->runArchivePhpCron($archivePhpOptions);
+        
+        foreach ($this->getApiForTesting() as $testInfo) {
+            list($api, $params) = $testInfo;
+            
+            if (!isset($params['testSuffix'])) {
+                $params['testSuffix'] = '';
+            }
+            $params['testSuffix'] .= '_' . $optionGroupName;
+            $params['disableArchiving'] = true;
+            
+            // only do month for the last 3 option groups
+            if ($optionGroupName != 'noOptions') {
+                $params['periods'] = array('day');
+            }
+            
+            $this->runApiTests($api, $params);
+        }
+    }
+    
+    private function setLastRunArchiveOptions()
+    {
+        $periodTypes = array('day', 'periods');
+        $idSites = Piwik_SitesManager_API::getInstance()->getAllSitesId();
+        
+        $time = Piwik_Date::factory(self::$fixture->dateTime)->subDay(1)->getTimestamp();
+        
+        foreach ($periodTypes as $period) {
+            foreach ($idSites as $idSite) {
+                $lastRunArchiveOption = Piwik::getArchiveCronLastRunOptionName($period, $idSite);
+                
+                Piwik_SetOption($lastRunArchiveOption, $time);
+            }
+        }
+    }
+    
+    private function runArchivePhpCron($options)
+    {
+        $archivePhpScript = PIWIK_INCLUDE_PATH . '/tests/PHPUnit/proxy/archive.php';
+        $urlToProxy = Test_Piwik_BaseFixture::getRootUrl() . 'tests/PHPUnit/proxy/index.php';
+        
+        // create the command
+        $cmd = "php \"$archivePhpScript\" --url=\"$urlToProxy\" ";
+        foreach ($options as $name => $value) {
+            $cmd .= $name;
+            if ($value !== false) {
+                $cmd .= '="' . $value . '"';
+            }
+            $cmd .= ' ';
+        }
+        $cmd .= '2>&1';
+        
+        // run the command
+        exec($cmd, $output, $result);
+        if ($result !== 0) {
+            throw new Exception("log importer failed: " . implode("\n", $output) . "\n\ncommand used: $cmd");
+        }
+
+        return $output;
+    }
+}
+
+Test_Piwik_Integration_ArchiveCronTest::$fixture = new Test_Piwik_Fixture_ManySitesImportedLogs();
+Test_Piwik_Integration_ArchiveCronTest::$fixture->addSegments = true;

tests/PHPUnit/Integration/ImportLogsTest.php

@@ -12,7 +12,7 @@
 class Test_Piwik_Integration_ImportLogs extends IntegrationTestCase
 {
     public static $fixture = null; // initialized below class definition
-
+    
     /**
      * @dataProvider getApiForTesting
      * @group        Integration
@@ -45,6 +45,9 @@ class Test_Piwik_Integration_ImportLogs extends IntegrationTestCase
     /**
      * @group        Integration
      * @group        ImportLogs
+     * 
+     * NOTE: This test must be last since the new sites that get added are added in
+     *       random order.
      */
     public function testDynamicResolverSitesCreated()
     {

tests/PHPUnit/Integration/expected/test_ArchiveCronTest_noOptions__VisitsSummary.get_day.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<results>
+	<result idSite="1">
+		<nb_uniq_visitors>9</nb_uniq_visitors>
+		<nb_visits>9</nb_visits>
+		<nb_actions>9</nb_actions>
+		<nb_visits_converted>9</nb_visits_converted>
+		<bounce_count>9</bounce_count>
+		<sum_visit_length>0</sum_visit_length>
+		<max_actions>1</max_actions>
+		<bounce_rate>100%</bounce_rate>
+		<nb_actions_per_visit>1</nb_actions_per_visit>
+		<avg_time_on_site>0</avg_time_on_site>
+	</result>
+	<result idSite="2" />
+</results>
\ No newline at end of file

tests/PHPUnit/Integration/expected/test_ArchiveCronTest_noOptions__VisitsSummary.get_month.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<results>
+	<result idSite="1">
+		<nb_uniq_visitors>26</nb_uniq_visitors>
+		<nb_visits>26</nb_visits>
+		<nb_actions>29</nb_actions>
+		<nb_visits_converted>24</nb_visits_converted>
+		<bounce_count>24</bounce_count>
+		<sum_visit_length>305</sum_visit_length>
+		<max_actions>3</max_actions>
+		<bounce_rate>92%</bounce_rate>
+		<nb_actions_per_visit>1.1</nb_actions_per_visit>
+		<avg_time_on_site>12</avg_time_on_site>
+	</result>
+	<result idSite="2">
+		<nb_uniq_visitors>1</nb_uniq_visitors>
+		<nb_visits>1</nb_visits>
+		<nb_actions>1</nb_actions>
+		<nb_visits_converted>0</nb_visits_converted>
+		<bounce_count>1</bounce_count>
+		<sum_visit_length>0</sum_visit_length>
+		<max_actions>1</max_actions>
+		<bounce_rate>100%</bounce_rate>
+		<nb_actions_per_visit>1</nb_actions_per_visit>
+		<avg_time_on_site>0</avg_time_on_site>
+	</result>
+</results>
\ No newline at end of file

tests/PHPUnit/Integration/expected/test_ArchiveCronTest_noOptions__VisitsSummary.get_week.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<results>
+	<result idSite="1">
+		<nb_uniq_visitors>24</nb_uniq_visitors>
+		<nb_visits>24</nb_visits>
+		<nb_actions>27</nb_actions>
+		<nb_visits_converted>22</nb_visits_converted>
+		<bounce_count>22</bounce_count>
+		<sum_visit_length>305</sum_visit_length>
+		<max_actions>3</max_actions>
+		<bounce_rate>92%</bounce_rate>
+		<nb_actions_per_visit>1.1</nb_actions_per_visit>
+		<avg_time_on_site>13</avg_time_on_site>
+	</result>
+	<result idSite="2" />
+</results>
\ No newline at end of file

tests/PHPUnit/Integration/expected/test_ArchiveCronTest_noOptions__VisitsSummary.get_year.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<results>
+	<result idSite="1">
+		<nb_visits>27</nb_visits>
+		<nb_actions>39</nb_actions>
+		<nb_visits_converted>25</nb_visits_converted>
+		<bounce_count>24</bounce_count>
+		<sum_visit_length>359</sum_visit_length>
+		<max_actions>10</max_actions>
+		<bounce_rate>89%</bounce_rate>
+		<nb_actions_per_visit>1.4</nb_actions_per_visit>
+		<avg_time_on_site>13</avg_time_on_site>
+	</result>
+	<result idSite="2">
+		<nb_visits>1</nb_visits>
+		<nb_actions>1</nb_actions>
+		<nb_visits_converted>0</nb_visits_converted>
+		<bounce_count>1</bounce_count>
+		<sum_visit_length>0</sum_visit_length>
+		<max_actions>1</max_actions>
+		<bounce_rate>100%</bounce_rate>
+		<nb_actions_per_visit>1</nb_actions_per_visit>
+		<avg_time_on_site>0</avg_time_on_site>
+	</result>
+</results>
\ No newline at end of file

tests/PHPUnit/Integration/expected/test_ArchiveCronTest_nonPreArchivedSegment_noOptions__VisitsSummary.get_day.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<results>
+	<result idSite="1" />
+	<result idSite="2" />
+</results>
\ No newline at end of file

tests/PHPUnit/Integration/expected/test_ArchiveCronTest_nonPreArchivedSegment_noOptions__VisitsSummary.get_month.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<results>
+	<result idSite="1" />
+	<result idSite="2" />
+</results>
\ No newline at end of file

tests/PHPUnit/Integration/expected/test_ArchiveCronTest_nonPreArchivedSegment_noOptions__VisitsSummary.get_week.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<results>
+	<result idSite="1" />
+	<result idSite="2" />
+</results>
\ No newline at end of file

tests/PHPUnit/Integration/expected/test_ArchiveCronTest_nonPreArchivedSegment_noOptions__VisitsSummary.get_year.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<results>
+	<result idSite="1" />
+	<result idSite="2" />
+</results>
\ No newline at end of file

tests/PHPUnit/Integration/expected/test_ArchiveCronTest_segmentNoAutoArchive_noOptions__VisitsSummary.get_day.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<results>
+	<result idSite="1" />
+	<result idSite="2" />
+</results>
\ No newline at end of file

tests/PHPUnit/Integration/expected/test_ArchiveCronTest_segmentNoAutoArchive_noOptions__VisitsSummary.get_month.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<results>
+	<result idSite="1" />
+	<result idSite="2" />
+</results>
\ No newline at end of file

tests/PHPUnit/Integration/expected/test_ArchiveCronTest_segmentNoAutoArchive_noOptions__VisitsSummary.get_week.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<results>
+	<result idSite="1" />
+	<result idSite="2" />
+</results>
\ No newline at end of file

tests/PHPUnit/Integration/expected/test_ArchiveCronTest_segmentNoAutoArchive_noOptions__VisitsSummary.get_year.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<results>
+	<result idSite="1" />
+	<result idSite="2" />
+</results>
\ No newline at end of file

tests/PHPUnit/Integration/expected/test_ArchiveCronTest_segmentOnlyOneSite_noOptions__VisitsSummary.get_day.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<results>
+	<result idSite="1">
+		<nb_uniq_visitors>1</nb_uniq_visitors>
+		<nb_visits>1</nb_visits>
+		<nb_actions>1</nb_actions>
+		<nb_visits_converted>1</nb_visits_converted>
+		<bounce_count>1</bounce_count>
+		<sum_visit_length>0</sum_visit_length>
+		<max_actions>1</max_actions>
+		<bounce_rate>100%</bounce_rate>
+		<nb_actions_per_visit>1</nb_actions_per_visit>
+		<avg_time_on_site>0</avg_time_on_site>
+	</result>
+	<result idSite="2" />
+</results>
\ No newline at end of file

tests/PHPUnit/Integration/expected/test_ArchiveCronTest_segmentOnlyOneSite_noOptions__VisitsSummary.get_month.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<results>
+	<result idSite="1">
+		<nb_uniq_visitors>6</nb_uniq_visitors>
+		<nb_visits>6</nb_visits>
+		<nb_actions>6</nb_actions>
+		<nb_visits_converted>6</nb_visits_converted>
+		<bounce_count>6</bounce_count>
+		<sum_visit_length>0</sum_visit_length>
+		<max_actions>1</max_actions>
+		<bounce_rate>100%</bounce_rate>
+		<nb_actions_per_visit>1</nb_actions_per_visit>
+		<avg_time_on_site>0</avg_time_on_site>
+	</result>
+	<result idSite="2" />
+</results>
\ No newline at end of file