fixing some tests

matomo-org · Mar 2, 2020 · 2dd4958 · 2dd4958
1 parent f218967
commit 2dd4958
Show file tree

Hide file tree

Showing 8 changed files with 50 additions and 30 deletions.
diff --git a/core/Archive/ArchiveInvalidator.php b/core/Archive/ArchiveInvalidator.php
@@ -226,6 +226,9 @@ private function deleteOptionLike($id)
      */
     public function markArchivesAsInvalidated(array $idSites, array $dates, $period, Segment $segment = null, $cascadeDown = false)
     {
+        static $count = 0;
+        ++$count;
+        print "call count: $count\n";@ob_flush();
         $invalidationInfo = new InvalidationResult();
 
         // quick fix for #15086, if we're only invalidating today's date for a site, don't add the site to the list of sites

diff --git a/core/ArchiveProcessor/Loader.php b/core/ArchiveProcessor/Loader.php
@@ -193,11 +193,15 @@ protected function isArchivingForcedToTrigger()
     public function loadExistingArchiveIdFromDb()
     {
         if ($this->isArchivingForcedToTrigger()) {
-            return [false, false, false, false]; // no usable archive found, no existing archive
+            // TODO: lot's log here
+            // return no usable archive found, no existing archive. this will skip invalidation, which should
+            // be fine since we just force archiving. TODO: check this is true?
+            return [false, false, false, false];
         }
 
         $minDatetimeArchiveProcessedUTC = $this->getMinTimeArchiveProcessed();
-        return ArchiveSelector::getArchiveIdAndVisits($this->params, $minDatetimeArchiveProcessedUTC);
+        $result = ArchiveSelector::getArchiveIdAndVisits($this->params, $minDatetimeArchiveProcessedUTC);
+        return $result;
     }
 
     /**
@@ -279,7 +283,7 @@ private function invalidatedReportsIfNeeded()
             }
 
             try {
-                $this->invalidator->markArchivesAsInvalidated($siteIdsToActuallyInvalidate, array(Date::factory($date)), false);
+                $this->invalidator->markArchivesAsInvalidated($siteIdsToActuallyInvalidate, array(Date::factory($date)), $this->params->getPeriod()->getLabel(), $this->params->getSegment());
             } catch (\Exception $e) {
                 Site::clearCache();
                 throw $e;

diff --git a/core/DataAccess/ArchiveSelector.php b/core/DataAccess/ArchiveSelector.php
@@ -66,34 +66,34 @@ public static function getArchiveIdAndVisits(ArchiveProcessor\Parameters $params
         $plugins = array("VisitsSummary", $requestedPlugin);
 
         $doneFlags      = Rules::getDoneFlags($plugins, $segment);
+        $requestedPluginDoneFlags = Rules::getDoneFlags([$requestedPlugin], $segment);
         $doneFlagValues = Rules::getSelectableDoneFlagValues($includeInvalidated, $params);
 
         $results = self::getModel()->getArchiveIdAndVisits($numericTable, $idSite, $period, $dateStartIso, $dateEndIso, null, $doneFlags);
         if (empty($results)) { // no archive found
             return [false, false, false, false];
         }
 
-        $result = self::findArchiveDataWithLatestTsArchived($results, $requestedPlugin, $segment);
-        if (!isset($result['idarchive'])
-            || !isset($result['nb_visits'])
-        ) {
-            return [false, false, false, true];
-        }
+        $result = self::findArchiveDataWithLatestTsArchived($results, $requestedPluginDoneFlags);
 
-        if (!in_array($result['value'], $doneFlagValues)) { // the archive cannot be considered valid for this request (has wrong done flag value)
-            return [false, false, false, true];
+        $visits = isset($result['nb_visits']) ? $result['nb_visits'] : false;
+        $visitsConverted = isset($result['nb_visits_converted']) ? $result['nb_visits_converted'] : false;
+
+        if (isset($result['value'])
+            && !in_array($result['value'], $doneFlagValues)
+        ) { // the archive cannot be considered valid for this request (has wrong done flag value)
+            return [false, $visits, $visitsConverted, true];
         }
 
         // the archive is too old
         if ($minDatetimeArchiveProcessedUTC
+            && isset($result['idarchive'])
             && Date::factory($result['ts_archived'])->isEarlier(Date::factory($minDatetimeArchiveProcessedUTC))
         ) {
-            return [false, false, false, true];
+            return [false, $visits, $visitsConverted, true];
         }
 
-        $idArchive = $result['idarchive'];
-        $visits = $result['nb_visits'];
-        $visitsConverted = $result['nb_visits_converted'];
+        $idArchive = isset($result['idarchive']) ? $result['idarchive'] : false;
 
         return array($idArchive, $visits, $visitsConverted, true);
     }
@@ -344,10 +344,8 @@ private static function getNameCondition(array $plugins, Segment $segment, $incl
     }
 
     // TODO: document magic method
-    private static function findArchiveDataWithLatestTsArchived($results, $requestedPlugin, $segment)
+    private static function findArchiveDataWithLatestTsArchived($results, $requestedPluginDoneFlags)
     {
-        $namesRequestedPlugin = Rules::getDoneFlags(array($requestedPlugin), $segment);
-
         // find latest idarchive for each done flag
         $idArchives = [];
         foreach ($results as $row) {
@@ -372,10 +370,20 @@ private static function findArchiveDataWithLatestTsArchived($results, $requested
             }
         }
 
+        // if an  archive is found, but the metric data isn't found, we set the value to 0,
+        // so it won't get returned as false. // TODO: note if this is for BC or not. first check if it is for BC or not.
+        foreach ([self::NB_VISITS_RECORD_LOOKED_UP, self::NB_VISITS_CONVERTED_RECORD_LOOKED_UP] as $metric) {
+            if (!empty($idArchives)
+                && !isset($archiveData[$metric])
+            ) {
+                $archiveData[$metric] = 0;
+            }
+        }
+
         // set the idarchive & ts_archived for the archive we're looking for
         foreach ($results as $row) {
             $name = $row['name'];
-            if (in_array($name, $namesRequestedPlugin)) {
+            if (in_array($name, $requestedPluginDoneFlags)) {
                 $archiveData['idarchive'] = $row['idarchive'];
                 $archiveData['ts_archived'] = $row['ts_archived'];
                 $archiveData['value'] = $row['value'];

diff --git a/core/DataAccess/Model.php b/core/DataAccess/Model.php
@@ -232,9 +232,10 @@ public function getArchiveIdAndVisits($numericTable, $idSite, $period, $dateStar
             $bindSQL[]      = $minDatetimeIsoArchiveProcessedUTC;
         }
 
-        $limit = count($doneFlags) + 2; // total number of rows we could get w/ a single ts_archived
+        // total number of rows we could get w/ a single ts_archived (1 done flag + 2 metrics for each possible done flag)
+        // $limit = count($doneFlags) * 3;
+        // TODO :we can't predict how many segments there will be so there could be lots of nb_visits/nb_visits_converted rows... have to select everything.
 
-        // TODO: EXPLAIN it to check for performance
         $sqlQuery = "SELECT idarchive, value, name, ts_archived, date1 as startDate FROM $numericTable
                      WHERE idsite = ?
                          AND date1 = ?
@@ -244,8 +245,7 @@ public function getArchiveIdAndVisits($numericTable, $idSite, $period, $dateStar
                                OR name = '" . ArchiveSelector::NB_VISITS_RECORD_LOOKED_UP . "'
                                OR name = '" . ArchiveSelector::NB_VISITS_CONVERTED_RECORD_LOOKED_UP . "')
                          $timeStampWhere
-                     ORDER BY ts_archived DESC
-                     LIMIT $limit";
+                     ORDER BY ts_archived DESC, idarchive DESC";
         $results = Db::fetchAll($sqlQuery, $bindSQL);
 
         return $results;

diff --git a/core/Twig.php b/core/Twig.php
@@ -362,7 +362,7 @@ protected function addFilter_notification()
             if (!empty($options['raw'])) {
                 $template .= piwik_fix_lbrace($message);
             } else {
-                $template .= twig_escape_filter($twigEnv, $message, 'html');
+                $template .= piwik_escape_filter($twigEnv, $message, 'html');
             }
 
             $template .= '</div>';

diff --git a/plugins/Monolog/Processor/ExceptionToTextProcessor.php b/plugins/Monolog/Processor/ExceptionToTextProcessor.php
@@ -85,6 +85,10 @@ private function getStackTrace($exception)
 
     public static function getWholeBacktrace(\Exception $exception, $shouldPrintBacktrace = true)
     {
+        if (!$shouldPrintBacktrace) {
+            return $exception->getMessage();
+        }
+
         $message = "";
 
         $e = $exception;

diff --git a/tests/PHPUnit/Integration/ArchiveProcessor/LoaderTest.php b/tests/PHPUnit/Integration/ArchiveProcessor/LoaderTest.php
@@ -40,7 +40,7 @@ public function test_loadExistingArchiveIdFromDb_returnsFalsesIfNoArchiveFound()
 
         $archiveInfo = $loader->loadExistingArchiveIdFromDb();
 
-        $this->assertEquals([false, false, false], $archiveInfo);
+        $this->assertEquals([false, false, false, false], $archiveInfo);
     }
 
     /**
@@ -55,12 +55,12 @@ public function test_loadExistingArchiveIdFromDb_returnsFalsesPeriodIsForcedToAr
         $loader = new Loader($params);
 
         $archiveInfo = $loader->loadExistingArchiveIdFromDb();
-        $this->assertNotEquals([false, false, false], $archiveInfo);
+        $this->assertNotEquals([false, false, false, false], $archiveInfo);
 
         Config::getInstance()->Debug[$configSetting] = 1;
 
         $archiveInfo = $loader->loadExistingArchiveIdFromDb();
-        $this->assertEquals([false, false, false], $archiveInfo);
+        $this->assertEquals([false, false, false, false], $archiveInfo);
     }
 
     public function getTestDataForLoadExistingArchiveIdFromDbDebugConfig()
@@ -82,7 +82,7 @@ public function test_loadExistingArchiveIdFromDb_returnsArchiveIfArchiveInThePas
         $loader = new Loader($params);
 
         $archiveInfo = $loader->loadExistingArchiveIdFromDb();
-        $this->assertEquals(['1', '10', '0'], $archiveInfo);
+        $this->assertEquals(['1', '10', '0', true], $archiveInfo);
     }
 
     public function test_loadExistingArchiveIdFromDb_returnsArchiveIfForACurrentPeriod_AndNewEnough()
@@ -93,7 +93,7 @@ public function test_loadExistingArchiveIdFromDb_returnsArchiveIfForACurrentPeri
         $loader = new Loader($params);
 
         $archiveInfo = $loader->loadExistingArchiveIdFromDb();
-        $this->assertEquals(['1', '10', '0'], $archiveInfo);
+        $this->assertEquals(['1', '10', '0', true], $archiveInfo);
     }
 
     public function test_loadExistingArchiveIdFromDb_returnsNoArchiveIfForACurrentPeriod_AndNoneAreNewEnough()
@@ -104,7 +104,7 @@ public function test_loadExistingArchiveIdFromDb_returnsNoArchiveIfForACurrentPe
         $loader = new Loader($params);
 
         $archiveInfo = $loader->loadExistingArchiveIdFromDb();
-        $this->assertEquals([false, false, false], $archiveInfo);
+        $this->assertEquals([false, false, false, true], $archiveInfo);
     }
 
     private function insertArchive(Parameters $params, $tsArchived = null, $visits = 10)

diff --git a/tests/PHPUnit/System/OneVisitorTwoVisitsTest.php b/tests/PHPUnit/System/OneVisitorTwoVisitsTest.php
@@ -78,6 +78,7 @@ public function getApiForTesting()
         foreach ($bulkUrls as &$url) {
             $url = urlencode($url);
         }
+
         return array(
             array('all', array('idSite' => $idSite,
                                'date' => $dateTime,