diff --git a/core/Columns/Dimension.php b/core/Columns/Dimension.php index 26ede26f723ceeb7662933724f33bea9ede8cbbc..9f897b968e21fccbccf3c777ac45e2aca0cc97d4 100644 --- a/core/Columns/Dimension.php +++ b/core/Columns/Dimension.php @@ -27,14 +27,6 @@ abstract class Dimension } - public function getVersion() - { - $reflectionClass = new \ReflectionObject($this); - $file = $reflectionClass->getFileName(); - - return filemtime($file); - } - public function hasImplementedEvent($method) { $reflectionObject = new \ReflectionObject($this); diff --git a/core/Columns/Updates.php b/core/Columns/Updates.php index 30cfa017cdbf71e98ac8a16cf120c2a80d33a195..85b27ee0ff3377c2121ce4a62c58a6c816383fe9 100644 --- a/core/Columns/Updates.php +++ b/core/Columns/Updates.php @@ -19,6 +19,11 @@ use Piwik\Updater; */ class Updates extends \Piwik\Updates { + /** + * @var Updater + */ + private static $updater; + /** * Return SQL to be executed in this update * @@ -35,7 +40,7 @@ class Updates extends \Piwik\Updates $changingColumns = self::getUpdates(); foreach ($changingColumns as $table => $columns) { - $sqls["ALTER TABLE `" . $table . "` " . implode(', ', $columns)] = false; + $sqls["ALTER TABLE `" . Common::prefixTable($table) . "` " . implode(', ', $columns)] = false; } return $sqls; @@ -57,6 +62,16 @@ class Updates extends \Piwik\Updates } } + public static function setUpdater($updater) + { + self::$updater = $updater; + } + + private static function hasComponentNewVersion($component) + { + return empty(self::$updater) || self::$updater->hasNewVersion($component); + } + private static function getUpdates() { $visitColumns = DbHelper::getTableColumns(Common::prefixTable('log_visit')); @@ -66,7 +81,17 @@ class Updates extends \Piwik\Updates $changingColumns = array(); foreach (VisitDimension::getAllDimensions() as $dimension) { - $columns = $dimension->install($visitColumns, $conversionColumns); + $column = $dimension->getColumnName(); + + if (!self::hasComponentNewVersion('log_visit.' . $column)) { + continue; + } + + if (array_key_exists($column, $visitColumns)) { + $columns = $dimension->update($visitColumns, $conversionColumns); + } else { + $columns = $dimension->install(); + } if (!empty($columns)) { foreach ($columns as $table => $col) { if (empty($changingColumns[$table])) { @@ -79,7 +104,18 @@ class Updates extends \Piwik\Updates } foreach (ActionDimension::getAllDimensions() as $dimension) { - $columns = $dimension->install($actionColumns); + $column = $dimension->getColumnName(); + + if (!self::hasComponentNewVersion('log_link_visit_action.' . $column)) { + continue; + } + + if (array_key_exists($column, $actionColumns)) { + $columns = $dimension->update($actionColumns); + } else { + $columns = $dimension->install(); + } + if (!empty($columns)) { foreach ($columns as $table => $col) { if (empty($changingColumns[$table])) { diff --git a/core/Plugin/ActionDimension.php b/core/Plugin/ActionDimension.php index ca0e7bfc3e690ed42aec1459e3f2d917d982cee9..f662155144ba98738ae03e506e3f2696ab7c7494 100644 --- a/core/Plugin/ActionDimension.php +++ b/core/Plugin/ActionDimension.php @@ -10,6 +10,7 @@ namespace Piwik\Plugin; use Piwik\Cache\PluginAwareStaticCache; use Piwik\Columns\Dimension; +use Piwik\DbHelper; use Piwik\Plugin\Manager as PluginManager; use Piwik\Common; use Piwik\Db; @@ -26,35 +27,47 @@ abstract class ActionDimension extends Dimension { private $tableName = 'log_link_visit_action'; - public function install($actionColumns) + public function install() { if (empty($this->columnName) || empty($this->columnType)) { return array(); } - $columnExists = array_key_exists($this->columnName, $actionColumns); + return array( + $this->tableName => array("ADD COLUMN `$this->columnName` $this->columnType") + ); + } - if (!$columnExists) { - return array( - Common::prefixTable($this->tableName) => array("ADD COLUMN `$this->columnName` $this->columnType") - ); + public function update() + { + if (empty($this->columnName) || empty($this->columnType)) { + return array(); } - return array(); + return array( + $this->tableName => array("MODIFY COLUMN `$this->columnName` $this->columnType") + ); } - public function uninstall($actionColumns) + public function uninstall() { - if (!empty($this->columnName) - && !empty($this->columnType) - && array_key_exists($this->columnName, $actionColumns)) { + if (empty($this->columnName) || empty($this->columnType)) { + return; + } - return array( - Common::prefixTable($this->tableName) => array("DROP COLUMN `$this->columnName`") - ); + try { + $sql = "ALTER TABLE `" . Common::prefixTable($this->tableName) . "` DROP COLUMN `$this->columnName`"; + Db::exec($sql); + } catch (\Exception $e) { + if (!Db::get()->isErrNo($e, '1091')) { + throw $e; + } } + } - return array(); + public function getVersion() + { + return $this->columnType; } /** diff --git a/core/Plugin/Manager.php b/core/Plugin/Manager.php index 3bbbf0084f453195265743c231768ca2105900ee..d77d6d07a3152298a378a4f7130792cc5b811b84 100644 --- a/core/Plugin/Manager.php +++ b/core/Plugin/Manager.php @@ -1278,13 +1278,31 @@ class Manager extends Singleton /** * @param $pluginName */ - public function executePluginUninstall($pluginName) + private function executePluginUninstall($pluginName) { try { $plugin = $this->getLoadedPlugin($pluginName); $plugin->uninstall(); } catch (\Exception $e) { } + + if (empty($plugin)) { + return; + } + + try { + foreach (VisitDimension::getDimensions($plugin) as $dimension) { + $dimension->uninstall(); + } + } catch (\Exception $e) { + } + + try { + foreach (ActionDimension::getDimensions($plugin) as $dimension) { + $dimension->uninstall(); + } + } catch (\Exception $e) { + } } /** diff --git a/core/Plugin/VisitDimension.php b/core/Plugin/VisitDimension.php index 33899b211b473936ac1de8090fdb22107fed5bc4..49fa40ad5217329197aad6d701e37e87f9620e9a 100644 --- a/core/Plugin/VisitDimension.php +++ b/core/Plugin/VisitDimension.php @@ -26,43 +26,50 @@ abstract class VisitDimension extends Dimension { private $tableName = 'log_visit'; - public function install($visitColumns, $conversionColumns) + public function install() { if (!$this->columnType) { return array(); } - $changes = array(); + $changes = array( + $this->tableName => array("ADD COLUMN `$this->columnName` $this->columnType") + ); + + if ($this->isHandlingLogConversion()) { + $changes['log_conversion'] = array("ADD COLUMN `$this->columnName` $this->columnType"); + } - $hasVisitColumn = array_key_exists($this->columnName, $visitColumns); + return $changes; + } - if (!$hasVisitColumn) { - $tableVisit = Common::prefixTable($this->tableName); - $changes[$tableVisit] = array("ADD COLUMN `$this->columnName` $this->columnType"); + public function update($visitColumns, $conversionColumns) + { + if (!$this->columnType) { + return array(); } - $handlingLogConversion = $this->isHandlingLogConversion(); - $hasConversionColumn = array_key_exists($this->columnName, $conversionColumns); - $tableConversion = Common::prefixTable("log_conversion"); + $changes = array(); + + $changes[$this->tableName] = array("MODIFY COLUMN `$this->columnName` $this->columnType"); - if (!$hasConversionColumn && $handlingLogConversion) { - $changes[$tableConversion] = array("ADD COLUMN `$this->columnName` $this->columnType"); - } elseif ($hasConversionColumn && !$handlingLogConversion) { - $changes[$tableConversion] = array("DROP COLUMN `$this->columnName`"); + $handlingConversion = $this->isHandlingLogConversion(); + $hasConversionColumn = array_key_exists($this->columnName, $conversionColumns); + + if ($hasConversionColumn && $handlingConversion) { + $changes['log_conversion'] = array("MODIFY COLUMN `$this->columnName` $this->columnType"); + } elseif (!$hasConversionColumn && $handlingConversion) { + $changes['log_conversion'] = array("ADD COLUMN `$this->columnName` $this->columnType"); + } elseif ($hasConversionColumn && !$handlingConversion) { + $changes['log_conversion'] = array("DROP COLUMN `$this->columnName`"); } return $changes; } - private function isHandlingLogVisit() + public function getVersion() { - if (empty($this->columnName) || empty($this->columnType)) { - return false; - } - - return $this->hasImplementedEvent('onNewVisit') - || $this->hasImplementedEvent('onExistingVisit') - || $this->hasImplementedEvent('onConvertedVisit'); + return $this->columnType . $this->isHandlingLogConversion(); } private function isHandlingLogConversion() @@ -74,25 +81,29 @@ abstract class VisitDimension extends Dimension return $this->hasImplementedEvent('onRecordGoal'); } - public function uninstall($visitColumns, $conversionColumns) + public function uninstall() { if (empty($this->columnName) || empty($this->columnType)) { - return array(); + return; } - $columnsToDrop = array(); - - if (array_key_exists($this->columnName, $visitColumns) && $this->isHandlingLogVisit()) { - $tableVisit = Common::prefixTable($this->tableName); - $columnsToDrop[$tableVisit] = array("DROP COLUMN `$this->columnName`"); + try { + $sql = "ALTER TABLE `" . Common::prefixTable($this->tableName) . "` DROP COLUMN `$this->columnName`"; + Db::exec($sql); + } catch (\Exception $e) { + if (!Db::get()->isErrNo($e, '1091')) { + throw $e; + } } - if (array_key_exists($this->columnName, $conversionColumns) && $this->isHandlingLogConversion()) { - $tableConversion = Common::prefixTable("log_conversion"); - $columnsToDrop[$tableConversion] = array("DROP COLUMN `$this->columnName`"); + try { + $sql = "ALTER TABLE `" . Common::prefixTable('log_conversion') . "` DROP COLUMN `$this->columnName`"; + Db::exec($sql); + } catch (\Exception $e) { + if (!Db::get()->isErrNo($e, '1091')) { + throw $e; + } } - - return $columnsToDrop; } protected function addSegment(Segment $segment) diff --git a/core/Updater.php b/core/Updater.php index d0b2fd9ad3ea7bb6299e99aa18d66ee9df392108..9e58572900f9415185635a0183142fb02bac1270 100644 --- a/core/Updater.php +++ b/core/Updater.php @@ -210,6 +210,8 @@ class Updater $componentsWithUpdateFile = array(); $hasDimensionUpdate = null; + ColumnUpdates::setUpdater($this); + foreach ($this->componentsWithNewVersion as $name => $versions) { $currentVersion = $versions[self::INDEX_CURRENT_VERSION]; $newVersion = $versions[self::INDEX_NEW_VERSION]; @@ -217,7 +219,7 @@ class Updater if ($name == 'core') { $pathToUpdates = $this->pathUpdateFileCore . '*.php'; } elseif ($this->isDimensionComponent($name)) { - if (is_null($hasDimensionUpdate)) { + if (!$hasDimensionUpdate) { $hasDimensionUpdate = ColumnUpdates::hasUpdates(); } if ($hasDimensionUpdate) { @@ -295,10 +297,14 @@ class Updater self::recordComponentSuccessfullyUpdated($name, $currentVersion); } - // note: when versionCompare == 1, the version in the DB is newer, we choose to ignore - $currentVersionIsOutdated = version_compare($currentVersion, $version) == -1; - $isComponentOutdated = $currentVersion === false || $currentVersionIsOutdated; - if ($isComponentOutdated) { + if ($this->isDimensionComponent($name)) { + $isComponentOutdated = $currentVersion !== $version; + } else { + // note: when versionCompare == 1, the version in the DB is newer, we choose to ignore + $isComponentOutdated = version_compare($currentVersion, $version) == -1; + } + + if ($isComponentOutdated || $currentVersion === false) { $componentsToUpdate[$name] = array( self::INDEX_CURRENT_VERSION => $currentVersion, self::INDEX_NEW_VERSION => $version diff --git a/plugins/Actions/Columns/ServerTime.php b/plugins/Actions/Columns/ServerTime.php index 9240fba414ee68d6e991683f8f9810f2987f763d..8c13bbde633fd4b9fef2870ae5bef0e02fdcdcb4 100644 --- a/plugins/Actions/Columns/ServerTime.php +++ b/plugins/Actions/Columns/ServerTime.php @@ -21,18 +21,12 @@ class ServerTime extends ActionDimension protected $columnName = 'server_time'; protected $columnType = 'DATETIME NOT NULL'; - public function install($actionColumns) + public function install() { - if (array_key_exists($this->columnName, $actionColumns)) { - return array(); - } + $changes = parent::install(); + $changes['log_link_visit_action'][] = "ADD INDEX index_idsite_servertime ( idsite, server_time )"; - return array( - Common::prefixTable("log_link_visit_action") => array( - "ADD COLUMN server_time DATETIME NOT NULL", - "ADD INDEX index_idsite_servertime ( idsite, server_time )" - ) - ); + return $changes; } public function getName() diff --git a/tests/PHPUnit/Fixture.php b/tests/PHPUnit/Fixture.php index c609341c64331accd3b4a19cb31603bfcec79c44..b454abab370c84a328d58c08a075f96a9fda1a20 100644 --- a/tests/PHPUnit/Fixture.php +++ b/tests/PHPUnit/Fixture.php @@ -29,7 +29,8 @@ use Piwik\Site; use Piwik\Tracker\Cache; use Piwik\Translate; use Piwik\Url; -use Piwik\Columns\Updates as ColumnsUpdates; +use Piwik\Updater; +use Piwik\Plugins\CoreUpdater\CoreUpdater; /** * Base type for all integration test fixtures. Integration test fixtures @@ -179,6 +180,13 @@ class Fixture extends PHPUnit_Framework_Assert static::loadAllPlugins($this->getTestEnvironment(), $this->testCaseClass); + $updater = new Updater(); + $componentsWithUpdateFile = CoreUpdater::getComponentUpdates($updater); + + if (!empty($componentsWithUpdateFile)) { + CoreUpdater::updateComponents($updater, $componentsWithUpdateFile); + } + $_GET = $_REQUEST = array(); $_SERVER['HTTP_REFERER'] = ''; @@ -302,8 +310,6 @@ class Fixture extends PHPUnit_Framework_Assert Log::info("Plugin loading messages: %s", implode(" --- ", $messages)); } - ColumnsUpdates::update(); - // Activate them foreach($plugins as $name) { if (!$pluginsManager->isPluginActivated($name)) { @@ -318,7 +324,7 @@ class Fixture extends PHPUnit_Framework_Assert $manager = \Piwik\Plugin\Manager::getInstance(); $plugins = $manager->getLoadedPlugins(); foreach ($plugins AS $plugin) { - $manager->executePluginUninstall($plugin->getPluginName()); + $plugin->uninstall(); } \Piwik\Plugin\Manager::getInstance()->unloadPlugins(); } catch (Exception $e) {