Reorganise Matomo 4 updates for better performance (#16284)

4 files changed, 75 insertions, 12 deletions

diff --git a/core/Updater/Migration/Db/DropColumns.php b/core/Updater/Migration/Db/DropColumns.php
new file mode 100644
index 0000000000..5df61ddf08
--- /dev/null
+++ b/core/Updater/Migration/Db/DropColumns.php
@@ -0,0 +1,43 @@
+<?php
+/**
+ * Matomo - free/libre analytics platform
+ *
+ * @link https://matomo.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+namespace Piwik\Updater\Migration\Db;
+
+use Piwik\Db;
+use Piwik\DbHelper;
+
+/**
+ * @see Factory::dropColumn()
+ * @ignore
+ */
+class DropColumns extends Sql
+{
+    public function __construct($tableName, $columnNames)
+    {
+        $table = DbHelper::getTableColumns($tableName);
+
+        // we need to remove all not existing columns. Otherwise if only one of the columns doesn't exist, all of
+        // the columns wouldn't be removed
+        $columnNames = array_filter($columnNames, function ($columnName) use ($table) {
+            return isset($table[$columnName]);
+        });
+
+        if (empty($columnNames)) {
+            parent::__construct('', static::ERROR_CODE_COLUMN_NOT_EXISTS);
+        } else {
+            $columnNames = array_unique($columnNames);
+            $dropColumns = array_map(function ($columnName) {
+                return sprintf('DROP COLUMN `%s`', $columnName);
+            }, $columnNames);
+
+            $sql = sprintf("ALTER TABLE `%s` %s", $tableName, implode(', ', $dropColumns));
+            parent::__construct($sql, static::ERROR_CODE_COLUMN_NOT_EXISTS);
+        }
+
+    }
+
+}
diff --git a/core/Updater/Migration/Db/Factory.php b/core/Updater/Migration/Db/Factory.php
index a49ed58c92..e7de868c0d 100644
--- a/core/Updater/Migration/Db/Factory.php
+++ b/core/Updater/Migration/Db/Factory.php
@@ -177,6 +177,22 @@ class Factory
     }
 
     /**
+     * Drops an existing database table column.
+     *
+     * @param string $table  Unprefixed database table name, eg 'log_visit'.
+     * @param array $columnName  An array of column names that should be dropped eg ['column1', 'column2'].
+     * @return DropColumns
+     */
+    public function dropColumns($table, $columnNames)
+    {
+        $table = $this->prefixTable($table);
+
+        return $this->container->make('Piwik\Updater\Migration\Db\DropColumns', array(
+            'tableName' => $table, 'columnNames' => $columnNames
+        ));
+    }
+
+    /**
      * Changes the column name and column type of an existing database table column.
      *
      * @param string $table  Unprefixed database table name, eg 'log_visit'.
diff --git a/core/Updater/Migration/Db/Sql.php b/core/Updater/Migration/Db/Sql.php
index ca2becb461..0ad435c0b5 100644
--- a/core/Updater/Migration/Db/Sql.php
+++ b/core/Updater/Migration/Db/Sql.php
@@ -83,7 +83,7 @@ class Sql extends DbMigration
     {
         $sql = $this->sql;
 
-        if (!Common::stringEndsWith($sql, ';')) {
+        if (!empty($sql) && !Common::stringEndsWith($sql, ';')) {
             $sql .= ';';
         }
 
@@ -92,6 +92,8 @@ class Sql extends DbMigration
 
     public function exec()
     {
-        Db::exec($this->sql);
+        if (!empty($this->sql)) {
+            Db::exec($this->sql);
+        }
     }
 }
diff --git a/core/Updates/4.0.0-b1.php b/core/Updates/4.0.0-b1.php
index 4b192cf1db..be1bc7bcfb 100644
--- a/core/Updates/4.0.0-b1.php
+++ b/core/Updates/4.0.0-b1.php
@@ -49,8 +49,6 @@ class Updates_4_0_0_b1 extends PiwikUpdates
         $migrations = [];
         $migrations[] = $this->migration->db->changeColumnType('log_action', 'name', 'VARCHAR(4096)');
         $migrations[] = $this->migration->db->changeColumnType('log_conversion', 'url', 'VARCHAR(4096)');
-        $migrations[] = $this->migration->db->dropColumn('log_visit', 'config_gears');
-        $migrations[] = $this->migration->db->dropColumn('log_visit', 'config_director');
         $migrations[] = $this->migration->db->changeColumn('log_link_visit_action', 'interaction_position', 'pageview_position', 'MEDIUMINT UNSIGNED DEFAULT NULL');
 
         /** APP SPECIFIC TOKEN START */
@@ -151,8 +149,7 @@ class Updates_4_0_0_b1 extends PiwikUpdates
 
         if (Manager::getInstance()->isPluginInstalled('CustomVariables')) {
             $visitActionTable = Common::prefixTable('log_link_visit_action');
-            $migrations[]     = $this->migration->db->sql("UPDATE $visitActionTable SET search_cat = custom_var_v4 WHERE custom_var_k4 = '_pk_scat'");
-            $migrations[]     = $this->migration->db->sql("UPDATE $visitActionTable SET search_count = custom_var_v5 WHERE custom_var_k5 = '_pk_scount'");
+            $migrations[]     = $this->migration->db->sql("UPDATE $visitActionTable SET search_cat = if(custom_var_k4 = '_pk_scat', custom_var_v4, search_cat), search_count = if(custom_var_k5 = '_pk_scount', custom_var_v5, search_count) WHERE custom_var_k4 = '_pk_scat' or custom_var_k5 = '_pk_scount'");
         }
 
         if ($this->usesGeoIpLegacyLocationProvider()) {
@@ -184,12 +181,17 @@ class Updates_4_0_0_b1 extends PiwikUpdates
         }
 
         // remove old days_to_... columns
-        $migrations[] = $this->migration->db->dropColumn('log_visit', 'visitor_days_since_first');
-        $migrations[] = $this->migration->db->dropColumn('log_visit', 'visitor_days_since_order');
-        $migrations[] = $this->migration->db->dropColumn('log_visit', 'visitor_days_since_last');
-
-        $migrations[] = $this->migration->db->dropColumn('log_conversion', 'visitor_days_since_first');
-        $migrations[] = $this->migration->db->dropColumn('log_conversion', 'visitor_days_since_order');
+        $migrations[] = $this->migration->db->dropColumns('log_visit', [
+            'config_gears',
+            'config_director',
+            'visitor_days_since_first',
+            'visitor_days_since_order',
+            'visitor_days_since_last',
+        ]);
+        $migrations[] = $this->migration->db->dropColumns('log_conversion', [
+            'visitor_days_since_first',
+            'visitor_days_since_order',
+        ]);
 
         $config = Config::getInstance();