Fixing build + getting the logic right in ArchiveProcessor.preProcessReport

5 files changed, 164 insertions, 104 deletions

diff --git a/core/API/ResponseBuilder.php b/core/API/ResponseBuilder.php
index 1f04edb8b3..be5639d1c8 100644
--- a/core/API/ResponseBuilder.php
+++ b/core/API/ResponseBuilder.php
@@ -133,6 +133,8 @@ class Piwik_API_ResponseBuilder
         return $renderer->renderException();
     }
 
+    const DISPLAY_BACKTRACE_DEBUG = false;
+
     /**
      * @param Exception $e
      * @return Exception
@@ -141,13 +143,10 @@ class Piwik_API_ResponseBuilder
     {
         // If we are in tests, show full backtrace
         if( defined('PIWIK_PATH_TEST_TO_ROOT')) {
-            // Set this to true to see the light!
-            $SHOW_ME_BACKTRACE = false;
-
-            if($SHOW_ME_BACKTRACE) {
+            if(self::DISPLAY_BACKTRACE_DEBUG) {
                 $message = $e->getMessage() . " in \n " . $e->getFile() . ":" . $e->getLine() . " \n " . $e->getTraceAsString();
             } else {
-                $message = $e->getMessage() . "\n \n --> To temporarily debug this error further, set \$SHOW_ME_BACKTRACE=true; in " . basename(__FILE__) ;
+                $message = $e->getMessage() . "\n \n --> To temporarily debug this error further, set const DISPLAY_BACKTRACE_DEBUG=true; in " . basename(__FILE__) ;
             }
             return new Exception($message);
         }
diff --git a/core/ArchiveProcessor.php b/core/ArchiveProcessor.php
index 438355751f..1f7cd71309 100644
--- a/core/ArchiveProcessor.php
+++ b/core/ArchiveProcessor.php
@@ -88,7 +88,6 @@ abstract class Piwik_ArchiveProcessor
      */
     protected $temporaryArchive;
 
-
     /**
      * This methods reads the subperiods if necessary,
      * and computes the archive of the current period.
@@ -134,10 +133,11 @@ abstract class Piwik_ArchiveProcessor
 
         $isArchivingDisabled = Piwik_ArchiveProcessor_Rules::isArchivingDisabledFor($segment, $period->getLabel());
 
-        if($isArchivingDisabled) {
+        if ($isArchivingDisabled) {
             if ($endTimestampUTC > $now
                 && $period->getNumberOfSubperiods() == 0
-                && $dateStart->getTimestamp() <= $now) {
+                && $dateStart->getTimestamp() <= $now
+            ) {
                 $minimumArchiveTime = false;
             } else {
                 // However, if archiving is disabled for this request, we shall
@@ -154,28 +154,48 @@ abstract class Piwik_ArchiveProcessor
     {
         $period = $this->getPeriod()->getLabel();
         $debugSetting = 'always_archive_data_period'; // default
-        if($period == 'day') {
+        if ($period == 'day') {
             $debugSetting = 'always_archive_data_day';
-        } elseif($period == 'range') {
+        } elseif ($period == 'range') {
             $debugSetting = 'always_archive_data_range';
         }
         return Piwik_Config::getInstance()->Debug[$debugSetting];
     }
 
-    public function preProcessArchive($requestedPlugin)
+    public function preProcessArchive($requestedPlugin, $enforceProcessCoreMetricsOnly = false)
     {
         $this->idArchive = false;
 
         $this->setRequestedPlugin($requestedPlugin);
 
-        $idArchive = $this->loadExistingArchiveIdFromDb($requestedPlugin);
-        if ($this->isArchivingForcedToTrigger()) {
-            $idArchive = false;
-        }
+        $processAllReportsIncludingVisitsSummary = Piwik_ArchiveProcessor_Rules::shouldProcessReportsAllPlugins($this->getSegment(), $this->getPeriod()->getLabel());
+
+        $doesRequestedPluginIncludeVisitsSummary = $processAllReportsIncludingVisitsSummary || $requestedPlugin == 'VisitsSummary';
+
+        if( !$enforceProcessCoreMetricsOnly ) {
+            $this->idArchive = $this->loadExistingArchiveIdFromDb($requestedPlugin);
+            if ($this->isArchivingForcedToTrigger()) {
+                $this->idArchive = false;
+                $this->setNumberOfVisits(false);
+            }
+            if (!empty($this->idArchive)) {
+                return $this->idArchive;
+            }
+
+            $visitsNotKnownYet = $this->getNumberOfVisits() === false;
+
+            $createAnotherArchiveForVisitsSummary = !$doesRequestedPluginIncludeVisitsSummary && $visitsNotKnownYet;
 
-        if(!empty($idArchive)) {
-            $this->idArchive = $idArchive;
-            return $idArchive;
+            if ($createAnotherArchiveForVisitsSummary) {
+                // recursive archive creation in case we create another separate one, for VisitsSummary core metrics
+                // We query VisitsSummary here, as it is needed in the call below ($this->getNumberOfVisits() > 0)
+                $requestedPlugin = $this->getRequestedPlugin();
+                $this->preProcessArchive('VisitsSummary', $pleaseProcessCoreMetricsOnly = true);
+                $this->setRequestedPlugin($requestedPlugin);
+                if($this->getNumberOfVisits() === false) {
+                    throw new Exception("preProcessArchive() is expected to set number of visits to a numeric value.");
+                }
+            }
         }
 
         if (!Piwik_DataAccess_Archiver::getArchiveProcessorLock($this->getSite()->getId(), $this->getPeriod(), $this->getSegment())) {
@@ -185,35 +205,26 @@ abstract class Piwik_ArchiveProcessor
             return;
         }
 
-        // Visits will be stored in this archive
-        $visitsSummaryWillBeProcessed = Piwik_ArchiveProcessor_Rules::shouldProcessReportsAllPlugins($this->getSegment(), $this->getPeriod()->getLabel());
-
-        // If visits were not stored in this archive and will not be archived as part of it, we create a new one
-//        if($this->getNumberOfVisits() === false
-//            && !$visitsSummaryWillBeProcessed) {
-//            if($requestedPlugin != 'VisitsSummary') {
-//                // creates an archive to store visits
-//                $requestedPlugin = $this->getRequestedPlugin();
-//                $this->preProcessArchive('VisitsSummary');
-//                $this->setRequestedPlugin($requestedPlugin);
-//            }
-//        }
-//
-        $idArchive = Piwik_DataAccess_Archiver::allocateNewArchiveId($this->getTableArchiveNumericName(), $this->getSite()->getId());
-        $this->idArchive = $idArchive;
+        $this->idArchive = Piwik_DataAccess_Archiver::allocateNewArchiveId($this->getTableArchiveNumericName(), $this->getSite()->getId());
 
         $doneFlag = Piwik_ArchiveProcessor_Rules::getDoneStringFlagFor($this->getSegment(), $this->getPeriod()->getLabel(), $requestedPlugin);
+//        var_dump($requestedPlugin); var_dump($doneFlag); var_dump($this->getSegment()->getString());
         $this->insertNumericRecord($doneFlag, Piwik_ArchiveProcessor::DONE_ERROR);
 
-//        if($visitsSummaryWillBeProcessed ) {
-        $metrics = $this->aggregateCoreVisitsMetrics();
+        $visitsNotKnownYet = $this->getNumberOfVisits() === false;
 
-        if(empty($metrics)) {
-            $this->setNumberOfVisits(false);
-        } else {
-            $this->setNumberOfVisits($metrics['nb_visits'], $metrics['nb_visits_converted']);
+        if ($visitsNotKnownYet
+            || $doesRequestedPluginIncludeVisitsSummary
+            || $enforceProcessCoreMetricsOnly) {
+            $metrics = $this->aggregateCoreVisitsMetrics();
+
+            if (empty($metrics)) {
+                $this->setNumberOfVisits(false);
+            } else {
+                $this->setNumberOfVisits($metrics['nb_visits'], $metrics['nb_visits_converted']);
+            }
         }
-//        }
+
         $temporary = 'definitive archive';
         if ($this->isArchiveTemporary()) {
             $temporary = 'temporary archive';
@@ -228,7 +239,8 @@ abstract class Piwik_ArchiveProcessor
             $this->getDateEnd()->getDateEndUTC()
         ));
 
-        if ($this->getNumberOfVisits() > 0) {
+        if ($this->getNumberOfVisits() > 0
+            && !$enforceProcessCoreMetricsOnly) {
             $this->compute();
         }
 
@@ -242,8 +254,7 @@ abstract class Piwik_ArchiveProcessor
 
         Piwik_DataAccess_Archiver::releaseArchiveProcessorLock($this->getSite()->getId(), $this->getPeriod(), $this->getSegment());
 
-        return $idArchive;
-
+        return $this->idArchive;
     }
 
     /**
@@ -253,7 +264,7 @@ abstract class Piwik_ArchiveProcessor
      */
     public function getTableArchiveNumericName()
     {
-        if(empty($this->tableArchiveNumeric)) {
+        if (empty($this->tableArchiveNumeric)) {
             $this->tableArchiveNumeric = new Piwik_TablePartitioning_Monthly('archive_numeric');
             $this->tableArchiveNumeric->setTimestamp($this->getPeriod()->getDateStart()->getTimestamp());
         }
@@ -267,14 +278,13 @@ abstract class Piwik_ArchiveProcessor
      */
     public function getTableArchiveBlobName()
     {
-        if(empty($this->tableArchiveBlob)) {
+        if (empty($this->tableArchiveBlob)) {
             $this->tableArchiveBlob = new Piwik_TablePartitioning_Monthly('archive_blob');
             $this->tableArchiveBlob->setTimestamp($this->getPeriod()->getDateStart()->getTimestamp());
         }
         return $this->tableArchiveBlob->getTableName();
     }
 
-
     public function setRequestedPlugin($plugin)
     {
         $this->requestedPlugin = $plugin;
@@ -295,10 +305,10 @@ abstract class Piwik_ArchiveProcessor
      */
     protected function setNumberOfVisits($visitsMetricCached, $convertedVisitsMetricCached = false)
     {
-        if(empty($visitsMetricCached)) {
+        if (empty($visitsMetricCached)) {
             $visitsMetricCached = 0;
         }
-        if(empty($convertedVisitsMetricCached)) {
+        if (empty($convertedVisitsMetricCached)) {
             $convertedVisitsMetricCached = 0;
         }
         $this->visitsMetricCached = (int)$visitsMetricCached;
@@ -315,7 +325,6 @@ abstract class Piwik_ArchiveProcessor
         return $this->convertedVisitsMetricCached;
     }
 
-
     /**
      * @param string $name
      * @param int|float $value
@@ -364,9 +373,9 @@ abstract class Piwik_ArchiveProcessor
         return array($name => $values);
     }
 
-    protected function compress( $data)
+    protected function compress($data)
     {
-        if(Zend_Registry::get('db')->hasBlobDataType()) {
+        if (Zend_Registry::get('db')->hasBlobDataType()) {
             return gzcompress($data);
         }
         return $data;
@@ -424,18 +433,17 @@ abstract class Piwik_ArchiveProcessor
     /**
      * Inserts a record in the right table (either NUMERIC or BLOB)
      *
-     * @param string  $name
-     * @param mixed   $value
+     * @param string $name
+     * @param mixed $value
      *
      * @return void
      */
     protected function insertRecord($name, $value)
     {
-        // We choose not to record records with a value of 0
-        if ($value === 0
-            && $name != 'nb_visits') {
+        if($this->isRecordZero($name, $value)) {
             return;
         }
+
         $tableName = $this->getTableNameToInsert($value);
 
         // duplicate idarchives are Ignored, see http://dev.piwik.org/trac/ticket/987
@@ -449,6 +457,16 @@ abstract class Piwik_ArchiveProcessor
         Piwik_Query($query, $bindSql);
     }
 
+    /**
+     * @param $name
+     * @param $value
+     * @return bool
+     */
+    protected function isRecordZero($name, $value)
+    {
+        return ($value === '0' || $value === false || $value === 0 || $value === 0.0);
+    }
+
     protected function getTableNameToInsert($value)
     {
         if (is_numeric($value)) {
@@ -474,11 +492,11 @@ abstract class Piwik_ArchiveProcessor
         $numericTableName = $this->getTableArchiveNumericName();
 
         $idAndVisits = Piwik_DataAccess_Archiver::getArchiveIdAndVisits($numericTableName, $site, $period, $segment, $minDatetimeArchiveProcessedUTC, $requestedPlugin);
-        if(!$idAndVisits) {
+        if (!$idAndVisits) {
             return false;
         }
-        list($idArchive, $visits) = $idAndVisits;
-        $this->setNumberOfVisits( $visits );
+        list($idArchive, $visits, $visitsConverted) = $idAndVisits;
+        $this->setNumberOfVisits($visits, $visitsConverted);
         return $idArchive;
     }
 
@@ -494,16 +512,15 @@ abstract class Piwik_ArchiveProcessor
         }
         // If any other segment, only process if the requested report belong to this plugin
         $pluginBeingProcessed = $this->getRequestedPlugin();
-        if($pluginBeingProcessed == $pluginName) {
+        if ($pluginBeingProcessed == $pluginName) {
             return true;
         }
-        if(!Piwik_PluginsManager::getInstance()->isPluginLoaded($pluginBeingProcessed)) {
+        if (!Piwik_PluginsManager::getInstance()->isPluginLoaded($pluginBeingProcessed)) {
             return true;
         }
         return false;
     }
 
-
     /**
      * Site of the current archive
      * Can be accessed by plugins (that is why it's public)
@@ -512,7 +529,6 @@ abstract class Piwik_ArchiveProcessor
      */
     private $site = null;
 
-
     /**
      * @var Piwik_Period
      */
@@ -569,5 +585,4 @@ abstract class Piwik_ArchiveProcessor
     {
         return $this->idArchive;
     }
-
 }
diff --git a/core/ArchiveProcessor/Period.php b/core/ArchiveProcessor/Period.php
index 776155a01a..a2f6632e67 100644
--- a/core/ArchiveProcessor/Period.php
+++ b/core/ArchiveProcessor/Period.php
@@ -77,7 +77,6 @@ class Piwik_ArchiveProcessor_Period extends Piwik_ArchiveProcessor
     {
         // We clean up below all tables created during this function call (and recursive calls)
         $latestUsedTableId = Piwik_DataTable_Manager::getInstance()->getMostRecentTableId();
-
         if (!is_array($recordNames)) {
             $recordNames = array($recordNames);
         }
@@ -116,8 +115,12 @@ class Piwik_ArchiveProcessor_Period extends Piwik_ArchiveProcessor
         }
 
         $data = $this->archiver->getDataTableExpanded($name, $idSubTable = null, $addMetadataSubtableId = false);
-        foreach ($data->getArray() as $date => $tableToSum) {
-            $table->addDataTable($tableToSum);
+        if($data instanceof Piwik_DataTable_Array) {
+            foreach ($data->getArray() as $date => $tableToSum) {
+                $table->addDataTable($tableToSum);
+            }
+        } else {
+            $table->addDataTable($data);
         }
 
         if (is_null($invalidSummedColumnNameToRenamedName)) {
@@ -229,8 +232,11 @@ class Piwik_ArchiveProcessor_Period extends Piwik_ArchiveProcessor
     {
         $results = array();
         foreach ($data as $row) {
+            if(!is_array($row)) {
+                // this is not a data array to aggregate
+                return $data;
+            }
             foreach ($row as $name => $value) {
-
                 $operation = $operationForColumn[$name];
                 switch ($operation) {
                     case 'sum':
diff --git a/core/ArchiveProcessor/Rules.php b/core/ArchiveProcessor/Rules.php
index e78e35441a..5817dc5519 100644
--- a/core/ArchiveProcessor/Rules.php
+++ b/core/ArchiveProcessor/Rules.php
@@ -112,17 +112,34 @@ class Piwik_ArchiveProcessor_Rules
         return self::getDoneFlagArchiveContainsAllPlugins($segment);
     }
 
-    public static function getDoneFlagArchiveContainsAllPlugins($segment)
+    private static function getDoneFlagArchiveContainsAllPlugins($segment)
     {
         return 'done' . $segment->getHash();
     }
 
-    public static function getDoneFlagArchiveContainsOnePlugin($segment, $plugin)
+    private static function getDoneFlagArchiveContainsOnePlugin($segment, $plugin)
     {
         return 'done' . $segment->getHash() . '.' . $plugin;
     }
 
     /**
+     * @param array $plugins
+     * @param $segment
+     * @return array
+     */
+    public static function getDoneFlags(array $plugins, $segment)
+    {
+        $doneFlags = array();
+        $doneAllPlugins = self::getDoneFlagArchiveContainsAllPlugins($segment);
+        $doneFlags[$doneAllPlugins] = $doneAllPlugins;
+        foreach ($plugins as $plugin) {
+            $doneOnePlugin = self::getDoneFlagArchiveContainsOnePlugin($segment, $plugin);
+            $doneFlags[$plugin] = $doneOnePlugin;
+        }
+        return $doneFlags;
+    }
+
+    /**
      * Given a monthly archive table, will delete all reports that are now outdated,
      * or reports that ended with an error
      */
diff --git a/core/DataAccess/Archiver.php b/core/DataAccess/Archiver.php
index a3003e1ec1..c1aac9c6e7 100644
--- a/core/DataAccess/Archiver.php
+++ b/core/DataAccess/Archiver.php
@@ -14,6 +14,9 @@
  */
 class Piwik_DataAccess_Archiver
 {
+    const NB_VISITS_RECORD_LOOKED_UP = "nb_visits";
+    const NB_VISITS_CONVERTED_RECORD_LOOKED_UP = "nb_visits_converted";
+
     public static function deletePreviousArchiveStatus($numericTable, $requestedPlugin, $segment, $period, $idArchive)
     {
         $done = Piwik_ArchiveProcessor_Rules::getDoneStringFlagFor($segment, $period->getLabel(), $requestedPlugin);
@@ -132,18 +135,19 @@ class Piwik_DataAccess_Archiver
             $bindSQL[] = Piwik_Date::factory($minDatetimeArchiveProcessedUTC)->getDatetime();
         }
 
-        $done = Piwik_ArchiveProcessor_Rules::getDoneFlagArchiveContainsOnePlugin($segment, $requestedPlugin);
-        $doneAllPluginsProcessed = Piwik_ArchiveProcessor_Rules::getDoneFlagArchiveContainsAllPlugins($segment);
+        $pluginOrVisitsSummary = array("VisitsSummary", $requestedPlugin);
+        $pluginOrVisitsSummary = array_unique($pluginOrVisitsSummary);
+        $sqlWhereArchiveName = self::getNameCondition($pluginOrVisitsSummary, $segment);
 
-        $doneFlagSelect = self::getNameCondition(array($requestedPlugin), $segment);
         $sqlQuery = "	SELECT idarchive, value, name, date1 as startDate
 						FROM " . $numericTableName . "``
 						WHERE idsite = ?
 							AND date1 = ?
 							AND date2 = ?
 							AND period = ?
-							AND ( $doneFlagSelect
-								  OR name = 'nb_visits')
+							AND ( ($sqlWhereArchiveName)
+								  OR name = '" . self::NB_VISITS_RECORD_LOOKED_UP . "'
+								  OR name = '" . self::NB_VISITS_CONVERTED_RECORD_LOOKED_UP . "')
 							$timeStampWhere
 						ORDER BY idarchive DESC";
         $results = Piwik_FetchAll($sqlQuery, $bindSQL);
@@ -151,28 +155,54 @@ class Piwik_DataAccess_Archiver
             return false;
         }
 
-        $idArchive = false;
-        // we look for the more recent idarchive
-        foreach ($results as $result) {
-            if ( in_array($result['name'], array($done, $doneAllPluginsProcessed)) ) {
-                $idArchive = $result['idarchive'];
-                break;
-            }
-        }
+        $idArchive = self::getMostRecentIdArchiveFromResults($segment, $requestedPlugin, $results);
+        $idArchiveVisitsSummary = self::getMostRecentIdArchiveFromResults($segment, "VisitsSummary", $results);
 
-        if(!$idArchive) {
+        list($visits, $visitsConverted) = self::getVisitsMetricsFromResults($idArchive, $idArchiveVisitsSummary, $results);
+
+        if($visits === false
+            && $idArchive === false) {
             return false;
         }
 
-        $visits = 0;
-        foreach($results as $result) {
-            if($result['idarchive'] == $idArchive
-                && $result['name'] == 'nb_visits') {
-                $visits = (int)$result['value'];
+        return array($idArchive, $visits, $visitsConverted);
+    }
+
+    protected static function getVisitsMetricsFromResults($idArchive, $idArchiveVisitsSummary, $results)
+    {
+        $visits = $visitsConverted = false;
+        if($idArchiveVisitsSummary !== false) {
+            $visits = $visitsConverted = 0;
+        }
+        foreach ($results as $result) {
+            if (in_array($result['idarchive'], array($idArchive, $idArchiveVisitsSummary))) {
+                $value = (int)$result['value'];
+                if (empty($visits)
+                    && $result['name'] == self::NB_VISITS_RECORD_LOOKED_UP) {
+                    $visits = $value;
+                }
+                if (empty($visitsConverted)
+                    && $result['name'] == self::NB_VISITS_CONVERTED_RECORD_LOOKED_UP) {
+                    $visitsConverted = $value;
+                }
+            }
+        }
+        return array($visits, $visitsConverted);
+    }
+
+    protected static function getMostRecentIdArchiveFromResults(Piwik_Segment $segment, $requestedPlugin, $results)
+    {
+        $idArchive = false;
+        $namesRequestedPlugin = Piwik_ArchiveProcessor_Rules::getDoneFlags(array($requestedPlugin), $segment);
+        foreach ($results as $result) {
+            if ($idArchive === false
+                && in_array($result['name'], $namesRequestedPlugin)
+            ) {
+                $idArchive = $result['idarchive'];
                 break;
             }
         }
-        return array($idArchive, $visits);
+        return $idArchive;
     }
 
     /**
@@ -195,7 +225,7 @@ class Piwik_DataAccess_Archiver
                                FROM %s
                               WHERE period = ?
                                 AND %s
-                                AND ".self::getNameCondition($plugins, $segment)."
+                                AND ". self::getNameCondition($plugins, $segment) ."
                                 AND idsite IN (".implode(',', $siteIds).")
                            GROUP BY idsite, date1, date2";
         
@@ -307,23 +337,16 @@ class Piwik_DataAccess_Archiver
     {
         // the flags used to tell how the archiving process for a specific archive was completed,
         // if it was completed
-        $doneFlags = array();
-        foreach ($plugins as $plugin) {
-            $doneAllPlugins = Piwik_ArchiveProcessor_Rules::getDoneFlagArchiveContainsAllPlugins($segment);
-            $doneOnePlugin = Piwik_ArchiveProcessor_Rules::getDoneFlagArchiveContainsOnePlugin($segment, $plugin);
-            
-            $doneFlags[$doneAllPlugins] = $doneAllPlugins;
-            $doneFlags[$doneOnePlugin] = $doneOnePlugin;
-        }
+        $doneFlags = Piwik_ArchiveProcessor_Rules::getDoneFlags($plugins, $segment);
 
         $allDoneFlags = "'".implode("','", $doneFlags)."'";
         
         // create the SQL to find archives that are DONE
-        return "(name IN ($allDoneFlags)) AND
-                (value = '".Piwik_ArchiveProcessor::DONE_OK."' OR
-                 value = '".Piwik_ArchiveProcessor::DONE_OK_TEMPORARY."')";
+        return "(name IN ($allDoneFlags)) AND ".
+                " (value = '".Piwik_ArchiveProcessor::DONE_OK."' OR ".
+                " value = '".Piwik_ArchiveProcessor::DONE_OK_TEMPORARY."')";
     }
-    
+
     /**
      * Returns the periods of the archives this instance is querying for grouped by
      * by year & month.