this is a very complicated one. Problem was when updating from 2.4.0 to 2.5.0-b1 the updater wanted to update all dimensions (meaning alter all columns in log_visit, link_action and conversion) to the same column type. This was happening because the system did not know those dimensions were already installed from a previous Piwik version. There was an update script that was supposed to tell Piwik those components are actually already installed since we only moved them from core to plugins but it cannot work as it is an update as well and therefore not executed before the actual update check. I tried many solutions to overcome this issue including reverting all the columns to the initial MySql Schema but even then there are problems as some plugins like DevicesDetection are not defined in MySql Schema but in the plugin. It is especially complicated since users might update from 2.4 to a future version where the column type of one of those dimension changes and we need to make sure to actually execute an alter update if one of those dimension changes. In such a case we cannot directly mark the component as successfully recorded. The conclusion was for me it is only possible to solve this problem by listing all dimensions that were moved from core to plugins including their version at that time hard coded...

2 files changed, 114 insertions, 34 deletions

diff --git a/core/Columns/Dimension.php b/core/Columns/Dimension.php
index d58284ce7c..4455480cdf 100644
--- a/core/Columns/Dimension.php
+++ b/core/Columns/Dimension.php
@@ -66,6 +66,11 @@ abstract class Dimension
         return $this->columnName;
     }
 
+    public function hasColumnType()
+    {
+        return !empty($this->columnType);
+    }
+
     abstract public function getName();
 
     /**
diff --git a/core/Columns/Updater.php b/core/Columns/Updater.php
index ed67b81702..c04779a81c 100644
--- a/core/Columns/Updater.php
+++ b/core/Columns/Updater.php
@@ -88,18 +88,7 @@ class Updater extends \Piwik\Updates
         $changingColumns = array();
 
         foreach (VisitDimension::getAllDimensions() as $dimension) {
-            $column = $dimension->getColumnName();
-
-            if (!self::hasComponentNewVersion('log_visit.' . $column)) {
-                continue;
-            }
-
-            if (array_key_exists($column, $visitColumns)) {
-                $updates = $dimension->update($visitColumns, $conversionColumns);
-            } else {
-                $updates = $dimension->install();
-            }
-
+            $updates         = self::getUpdatesForDimension($dimension, 'log_visit.', $visitColumns, $conversionColumns);
             $changingColumns = self::mixinUpdates($changingColumns, $updates);
         }
 
@@ -116,37 +105,28 @@ class Updater extends \Piwik\Updates
         return $changingColumns;
     }
 
-    private static function mixinUpdates($changingColumns, $updatesFromDimension)
-    {
-        if (!empty($updatesFromDimension)) {
-            foreach ($updatesFromDimension as $table => $col) {
-                if (empty($changingColumns[$table])) {
-                    $changingColumns[$table] = $col;
-                } else {
-                    $changingColumns[$table] = array_merge($changingColumns[$table], $col);
-                }
-            }
-        }
-
-        return $changingColumns;
-    }
-
     /**
-     * @param ActionDimension|ConversionDimension $dimension
+     * @param ActionDimension|ConversionDimension|VisitDimension $dimension
      * @param string $componentPrefix
      * @param array $existingColumnsInDb
+     * @param array $conversionColumns
      * @return array
      */
-    private static function getUpdatesForDimension($dimension, $componentPrefix, $existingColumnsInDb)
+    private static function getUpdatesForDimension($dimension, $componentPrefix, $existingColumnsInDb, $conversionColumns = array())
     {
         $column = $dimension->getColumnName();
+        $componentName = $componentPrefix . $column;
 
-        if (!self::hasComponentNewVersion($componentPrefix . $column)) {
+        if (!self::hasComponentNewVersion($componentName)) {
             return array();
         }
 
         if (array_key_exists($column, $existingColumnsInDb)) {
-            $sqlUpdates = $dimension->update($existingColumnsInDb);
+            if ($dimension instanceof VisitDimension) {
+                $sqlUpdates = $dimension->update($conversionColumns);
+            } else {
+                $sqlUpdates = $dimension->update();
+            }
         } else {
             $sqlUpdates = $dimension->install();
         }
@@ -154,6 +134,21 @@ class Updater extends \Piwik\Updates
         return $sqlUpdates;
     }
 
+    private static function mixinUpdates($changingColumns, $updatesFromDimension)
+    {
+        if (!empty($updatesFromDimension)) {
+            foreach ($updatesFromDimension as $table => $col) {
+                if (empty($changingColumns[$table])) {
+                    $changingColumns[$table] = $col;
+                } else {
+                    $changingColumns[$table] = array_merge($changingColumns[$table], $col);
+                }
+            }
+        }
+
+        return $changingColumns;
+    }
+
     public static function getAllVersions()
     {
         // to avoid having to load all dimensions on each request we check if there were any changes on the file system
@@ -170,21 +165,21 @@ class Updater extends \Piwik\Updates
 
         foreach (VisitDimension::getAllDimensions() as $dimension) {
             $columnName = $dimension->getColumnName();
-            if ($columnName) {
+            if ($columnName && $dimension->hasColumnType()) {
                 $versions['log_visit.' . $columnName] = $dimension->getVersion();
             }
         }
 
         foreach (ActionDimension::getAllDimensions() as $dimension) {
             $columnName = $dimension->getColumnName();
-            if ($columnName) {
+            if ($columnName && $dimension->hasColumnType()) {
                 $versions['log_link_visit_action.' . $columnName] = $dimension->getVersion();
             }
         }
 
         foreach (ConversionDimension::getAllDimensions() as $dimension) {
             $columnName = $dimension->getColumnName();
-            if ($columnName) {
+            if ($columnName && $dimension->hasColumnType()) {
                 $versions['log_conversion.' . $columnName] = $dimension->getVersion();
             }
         }
@@ -192,6 +187,86 @@ class Updater extends \Piwik\Updates
         return $versions;
     }
 
+    public static function isDimensionComponent($name)
+    {
+        return 0 === strpos($name, 'log_visit.')
+            || 0 === strpos($name, 'log_conversion.')
+            || 0 === strpos($name, 'log_conversion_item.')
+            || 0 === strpos($name, 'log_link_visit_action.');
+    }
+
+    public static function wasDimensionMovedFromCoreToPlugin($name, $version)
+    {
+        $dimensions = array (
+            'log_visit.config_resolution' => 'VARCHAR(9) NOT NULL',
+            'log_visit.config_device_brand' => 'VARCHAR( 100 ) CHARACTER SET utf8 COLLATE utf8_general_ci NULL DEFAULT NULL',
+            'log_visit.config_device_model' => 'VARCHAR( 100 ) CHARACTER SET utf8 COLLATE utf8_general_ci NULL DEFAULT NULL',
+            'log_visit.config_windowsmedia' => 'TINYINT(1) NOT NULL',
+            'log_visit.config_silverlight' => 'TINYINT(1) NOT NULL',
+            'log_visit.config_java' => 'TINYINT(1) NOT NULL',
+            'log_visit.config_gears' => 'TINYINT(1) NOT NULL',
+            'log_visit.config_pdf' => 'TINYINT(1) NOT NULL',
+            'log_visit.config_quicktime' => 'TINYINT(1) NOT NULL',
+            'log_visit.config_realplayer' => 'TINYINT(1) NOT NULL',
+            'log_visit.config_device_type' => 'TINYINT( 100 ) NULL DEFAULT NULL',
+            'log_visit.visitor_localtime' => 'TIME NOT NULL',
+            'log_visit.location_region' => 'char(2) DEFAULT NULL1',
+            'log_visit.visitor_days_since_last' => 'SMALLINT(5) UNSIGNED NOT NULL',
+            'log_visit.location_longitude' => 'float(10, 6) DEFAULT NULL1',
+            'log_visit.visit_total_events' => 'SMALLINT(5) UNSIGNED NOT NULL',
+            'log_visit.config_os_version' => 'VARCHAR( 100 ) CHARACTER SET utf8 COLLATE utf8_general_ci NULL DEFAULT NULL',
+            'log_visit.location_city' => 'varchar(255) DEFAULT NULL1',
+            'log_visit.location_country' => 'CHAR(3) NOT NULL1',
+            'log_visit.location_latitude' => 'float(10, 6) DEFAULT NULL1',
+            'log_visit.config_flash' => 'TINYINT(1) NOT NULL',
+            'log_visit.config_director' => 'TINYINT(1) NOT NULL',
+            'log_visit.visit_total_time' => 'SMALLINT(5) UNSIGNED NOT NULL',
+            'log_visit.visitor_count_visits' => 'SMALLINT(5) UNSIGNED NOT NULL1',
+            'log_visit.example_visit_dimension' => 'INTEGER(11) DEFAULT 0 NOT NULL',
+            'log_visit.visit_entry_idaction_name' => 'INTEGER(11) UNSIGNED NOT NULL',
+            'log_visit.visit_entry_idaction_url' => 'INTEGER(11) UNSIGNED NOT NULL',
+            'log_visit.visitor_returning' => 'TINYINT(1) NOT NULL1',
+            'log_visit.visitor_days_since_order' => 'SMALLINT(5) UNSIGNED NOT NULL1',
+            'log_visit.visit_goal_buyer' => 'TINYINT(1) NOT NULL',
+            'log_visit.visit_first_action_time' => 'DATETIME NOT NULL',
+            'log_visit.visit_goal_converted' => 'TINYINT(1) NOT NULL',
+            'log_visit.visitor_days_since_first' => 'SMALLINT(5) UNSIGNED NOT NULL1',
+            'log_visit.visit_exit_idaction_name' => 'INTEGER(11) UNSIGNED NOT NULL',
+            'log_visit.visit_exit_idaction_url' => 'INTEGER(11) UNSIGNED NULL DEFAULT 0',
+            'log_visit.config_browser_version' => 'VARCHAR(20) NOT NULL',
+            'log_visit.config_browser_name' => 'VARCHAR(10) NOT NULL',
+            'log_visit.location_browser_lang' => 'VARCHAR(20) NOT NULL',
+            'log_visit.config_os' => 'CHAR(3) NOT NULL',
+            'log_visit.config_cookie' => 'TINYINT(1) NOT NULL',
+            'log_visit.referer_visit_server_date' => 'date default NULL1',
+            'log_visit.referer_url' => 'TEXT NOT NULL',
+            'log_visit.visit_total_searches' => 'SMALLINT(5) UNSIGNED NOT NULL',
+            'log_visit.visit_total_actions' => 'SMALLINT(5) UNSIGNED NOT NULL',
+            'log_visit.referer_keyword' => 'VARCHAR(255) NULL1',
+            'log_visit.referer_name' => 'VARCHAR(70) NULL1',
+            'log_visit.referer_type' => 'TINYINT(1) UNSIGNED NULL1',
+            'log_link_visit_action.example_action_dimension' => 'VARCHAR(255) DEFAULT NULL',
+            'log_link_visit_action.idaction_name' => 'INTEGER(10) UNSIGNED',
+            'log_link_visit_action.idaction_url' => 'INTEGER(10) UNSIGNED DEFAULT NULL',
+            'log_link_visit_action.server_time' => 'DATETIME NOT NULL',
+            'log_link_visit_action.time_spent_ref_action' => 'INTEGER(10) UNSIGNED NOT NULL',
+            'log_link_visit_action.idaction_event_action' => 'INTEGER(10) UNSIGNED DEFAULT NULL',
+            'log_link_visit_action.idaction_event_category' => 'INTEGER(10) UNSIGNED DEFAULT NULL',
+            'log_conversion.example_conversion_dimension' => 'INTEGER(11) DEFAULT 0 NOT NULL',
+            'log_conversion.revenue_discount' => 'float default NULL',
+            'log_conversion.revenue' => 'float default NULL',
+            'log_conversion.revenue_shipping' => 'float default NULL',
+            'log_conversion.revenue_subtotal' => 'float default NULL',
+            'log_conversion.revenue_tax' => 'float default NULL',
+        );
+
+        if (!array_key_exists($name, $dimensions)) {
+            return false;
+        }
+
+        return strtolower($dimensions[$name]) === strtolower($version);
+    }
+
     public static function onNoUpdateAvailable($versionsThatWereChecked)
     {
         if (!empty($versionsThatWereChecked)) {