remove unreachable code and add test

matomo-org · Jan 24, 2025 · 407d1a8 · 407d1a8
1 parent 1fd9e83
commit 407d1a8
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 45 deletions.
diff --git a/core/DataAccess/Model.php b/core/DataAccess/Model.php
@@ -713,58 +713,16 @@ public function startArchive($invalidation)
         $table = Common::prefixTable('archive_invalidations');
 
         // set archive value to in progress if not set already
-        $statement = Db::query("UPDATE `$table` SET `status` = ?, processing_host = ?, process_id = ?, ts_started = NOW() WHERE idinvalidation = ? AND status = ?", [
+        $statement = Db::query("UPDATE `$table` SET `status` = ?, `processing_host` = ?, `process_id` = ?, `ts_started` = NOW() WHERE `idinvalidation` = ? AND `status` = ?", [
             ArchiveInvalidator::INVALIDATION_STATUS_IN_PROGRESS,
             gethostname() ?: null,
             Common::getProcessId(),
             $invalidation['idinvalidation'],
             ArchiveInvalidator::INVALIDATION_STATUS_QUEUED,
         ]);
 
-        if ($statement->rowCount() > 0) { // if we updated, then we've marked the archive as started
-            return true;
-        }
-
-        // archive was not originally started or was started within the expected time, so we assume it's ongoing and another process
-        // (on this machine or another) is actively archiving it.
-        $archiveFailureRecoveryTimeout = GeneralConfig::getConfigValue('archive_failure_recovery_timeout', $invalidation['idsite']);
-        if (
-            empty($invalidation['ts_started'])
-            || $invalidation['ts_started'] > Date::now()->subSeconds($archiveFailureRecoveryTimeout)->getTimestamp()
-        ) {
-            return false;
-        }
-
-        // archive was started over 24 hours ago, we assume it failed and take it over
-        Db::query("UPDATE `$table` SET `status` = ?, processing_host = ?, process_id = ?, ts_started = NOW() WHERE idinvalidation = ?", [
-            ArchiveInvalidator::INVALIDATION_STATUS_IN_PROGRESS,
-            gethostname() ?: null,
-            Common::getProcessId(),
-            $invalidation['idinvalidation'],
-        ]);
-
-        // remove similar invalidations w/ lesser idinvalidation values
-        $bind = [
-            $invalidation['idsite'],
-            $invalidation['period'],
-            $invalidation['date1'],
-            $invalidation['date2'],
-            $invalidation['name'],
-            ArchiveInvalidator::INVALIDATION_STATUS_IN_PROGRESS,
-        ];
-
-        if (empty($invalidation['report'])) {
-            $reportClause = "(report IS NULL OR report = '')";
-        } else {
-            $reportClause = "report = ?";
-            $bind[] = $invalidation['report'];
-        }
-
-        $sql = "DELETE FROM " . Common::prefixTable('archive_invalidations') . " WHERE idinvalidation < ? AND idsite = ? AND "
-            . "date1 = ? AND date2 = ? AND `period` = ? AND `name` = ? AND $reportClause";
-        Db::query($sql, $bind);
-
-        return true;
+        // if we updated, then we've marked the archive as started
+        return $statement->rowCount() > 0;
     }
 
     public function isSimilarArchiveInProgress($invalidation)

diff --git a/tests/PHPUnit/Integration/DataAccess/ModelTest.php b/tests/PHPUnit/Integration/DataAccess/ModelTest.php
@@ -9,6 +9,7 @@
 
 namespace Piwik\Tests\Integration\DataAccess;
 
+use Piwik\Archive\ArchiveInvalidator;
 use Piwik\Common;
 use Piwik\Config;
 use Piwik\DataAccess\ArchiveTableCreator;
@@ -737,6 +738,38 @@ public function testDeleteInvalidationsForDeletedSites()
         ], $invalidations);
     }
 
+    public function testStartArchiveEnrichesRecordWithHostnameAndProcessId()
+    {
+        Fixture::createWebsite('2014-01-01 00:00:00');
+
+        $this->insertInvalidations([
+            ['idsite' => 1, 'date1' => '2014-02-04', 'date2' => '2014-02-04', 'period' => 1, 'name' => 'done', 'ts_started' => Date::now()->getDatetime(), 'status' => ArchiveInvalidator::INVALIDATION_STATUS_IN_PROGRESS],
+            ['idsite' => 1, 'date1' => '2014-02-01', 'date2' => '2014-02-28', 'period' => 2, 'name' => 'done', 'status' => ArchiveInvalidator::INVALIDATION_STATUS_QUEUED],
+        ]);
+
+        $invalidations = Db::fetchAll("SELECT * FROM " . Common::prefixTable('archive_invalidations') . " ORDER BY idinvalidation ASC");
+
+        self::assertEmpty($invalidations[0]['processing_host']);
+        self::assertEmpty($invalidations[0]['process_id']);
+        self::assertEmpty($invalidations[1]['processing_host']);
+        self::assertEmpty($invalidations[1]['process_id']);
+
+        self::assertCount(2, $invalidations);
+
+        $this->model->startArchive($invalidations[0]);
+        $this->model->startArchive($invalidations[1]);
+
+        $invalidations = Db::fetchAll("SELECT * FROM " . Common::prefixTable('archive_invalidations') . " ORDER BY idinvalidation ASC");
+
+        self::assertCount(2, $invalidations);
+
+        // first one should not be updated as it's already running
+        self::assertEmpty($invalidations[0]['processing_host']);
+        self::assertEmpty($invalidations[0]['process_id']);
+        self::assertNotEmpty($invalidations[1]['processing_host']);
+        self::assertNotEmpty($invalidations[1]['process_id']);
+    }
+
     private function insertArchiveData($archivesToInsert)
     {
         $idarchive = 1;