From 764b3b80bef6e54c08b4875c97034fcfacd1d55c Mon Sep 17 00:00:00 2001
From: Thomas Steur <thomas.steur@gmail.com>
Date: Tue, 21 Jul 2015 08:51:16 +0000
Subject: [PATCH] refs #8304 make sure to not uninstall a dimension if another
plugin defines the same, make sure to correctly mark the dimension as
uninstalled
---
core/Columns/Dimension.php | 2 +-
core/Columns/Updater.php | 6 +--
core/Plugin/Dimension/ActionDimension.php | 2 +
core/Plugin/Dimension/ConversionDimension.php | 2 +
core/Plugin/Dimension/VisitDimension.php | 2 +
core/Plugin/Manager.php | 49 ++++++++++++++++---
core/Updater.php | 15 ++++++
7 files changed, 66 insertions(+), 12 deletions(-)
diff --git a/core/Columns/Dimension.php b/core/Columns/Dimension.php
index 8da3822fa9..c89ae637a5 100644
--- a/core/Columns/Dimension.php
+++ b/core/Columns/Dimension.php
@@ -142,7 +142,7 @@ abstract class Dimension
/**
* Returns a unique string ID for this dimension. The ID is built using the namespaced class name
- * of the dimension, but is modified to be more human readable
+ * of the dimension, but is modified to be more human readable.
*
* @return string eg, `"Referrers.Keywords"`
* @throws Exception if the plugin and simple class name of this instance cannot be determined.
diff --git a/core/Columns/Updater.php b/core/Columns/Updater.php
index f6121165bb..a29ed16330 100644
--- a/core/Columns/Updater.php
+++ b/core/Columns/Updater.php
@@ -163,15 +163,15 @@ class Updater extends \Piwik\Updates
$conversionColumns = DbHelper::getTableColumns(Common::prefixTable('log_conversion'));
foreach ($this->visitDimensions as $dimension) {
- $versions = $this->mixinVersions($updater, $dimension, 'log_visit.', $visitColumns, $versions);
+ $versions = $this->mixinVersions($updater, $dimension, VisitDimension::INSTALLER_PREFIX, $visitColumns, $versions);
}
foreach ($this->actionDimensions as $dimension) {
- $versions = $this->mixinVersions($updater, $dimension, 'log_link_visit_action.', $actionColumns, $versions);
+ $versions = $this->mixinVersions($updater, $dimension, ActionDimension::INSTALLER_PREFIX, $actionColumns, $versions);
}
foreach ($this->conversionDimensions as $dimension) {
- $versions = $this->mixinVersions($updater, $dimension, 'log_conversion.', $conversionColumns, $versions);
+ $versions = $this->mixinVersions($updater, $dimension, ConversionDimension::INSTALLER_PREFIX, $conversionColumns, $versions);
}
return $versions;
diff --git a/core/Plugin/Dimension/ActionDimension.php b/core/Plugin/Dimension/ActionDimension.php
index 3c697641a3..ce8a1eeda2 100644
--- a/core/Plugin/Dimension/ActionDimension.php
+++ b/core/Plugin/Dimension/ActionDimension.php
@@ -36,6 +36,8 @@ use Exception;
*/
abstract class ActionDimension extends Dimension
{
+ const INSTALLER_PREFIX = 'log_link_visit_action.';
+
private $tableName = 'log_link_visit_action';
/**
diff --git a/core/Plugin/Dimension/ConversionDimension.php b/core/Plugin/Dimension/ConversionDimension.php
index e20f1eeeaf..ff1bff55ae 100644
--- a/core/Plugin/Dimension/ConversionDimension.php
+++ b/core/Plugin/Dimension/ConversionDimension.php
@@ -38,6 +38,8 @@ use Exception;
*/
abstract class ConversionDimension extends Dimension
{
+ const INSTALLER_PREFIX = 'log_conversion.';
+
private $tableName = 'log_conversion';
/**
diff --git a/core/Plugin/Dimension/VisitDimension.php b/core/Plugin/Dimension/VisitDimension.php
index b93ca041b6..48bcf70516 100644
--- a/core/Plugin/Dimension/VisitDimension.php
+++ b/core/Plugin/Dimension/VisitDimension.php
@@ -37,6 +37,8 @@ use Exception;
*/
abstract class VisitDimension extends Dimension
{
+ const INSTALLER_PREFIX = 'log_visit.';
+
private $tableName = 'log_visit';
/**
diff --git a/core/Plugin/Manager.php b/core/Plugin/Manager.php
index 25434468d1..ff4c3553dc 100644
--- a/core/Plugin/Manager.php
+++ b/core/Plugin/Manager.php
@@ -1219,34 +1219,66 @@ class Manager
}
try {
+ $visitDimensions = VisitDimension::getAllDimensions();
+
foreach (VisitDimension::getDimensions($plugin) as $dimension) {
- $this->uninstallDimension($dimension);
+ $this->uninstallDimension(VisitDimension::INSTALLER_PREFIX, $dimension, $visitDimensions);
}
} catch (\Exception $e) {
}
try {
+ $actionDimensions = ActionDimension::getAllDimensions();
+
foreach (ActionDimension::getDimensions($plugin) as $dimension) {
- $this->uninstallDimension($dimension);
+ $this->uninstallDimension(ActionDimension::INSTALLER_PREFIX, $dimension, $actionDimensions);
}
} catch (\Exception $e) {
}
try {
+ $conversionDimensions = ConversionDimension::getAllDimensions();
+
foreach (ConversionDimension::getDimensions($plugin) as $dimension) {
- $this->uninstallDimension($dimension);
+ $this->uninstallDimension(ConversionDimension::INSTALLER_PREFIX, $dimension, $conversionDimensions);
}
} catch (\Exception $e) {
}
}
/**
+ * @param VisitDimension|ActionDimension|ConversionDimension $dimension
+ * @param VisitDimension[]|ActionDimension[]|ConversionDimension[] $allDimensions
+ * @return bool
+ */
+ private function doesAnotherPluginDefineSameColumnWithDbEntry($dimension, $allDimensions)
+ {
+ $module = $dimension->getModule();
+ $columnName = $dimension->getColumnName();
+
+ foreach ($allDimensions as $dim) {
+ if ($dim->getColumnName() === $columnName &&
+ $dim->hasColumnType() &&
+ $dim->getModule() !== $module) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ /**
+ * @param string $prefix column installer prefix
* @param ConversionDimension|VisitDimension|ActionDimension $dimension
+ * @param VisitDimension[]|ActionDimension[]|ConversionDimension[] $allDimensions
*/
- private function uninstallDimension(Dimension $dimension)
+ private function uninstallDimension($prefix, Dimension $dimension, $allDimensions)
{
- $dimension->uninstall();
- Option::delete('version_' . $dimension->getVersion());
+ if (!$this->doesAnotherPluginDefineSameColumnWithDbEntry($dimension, $allDimensions)) {
+ $dimension->uninstall();
+
+ $this->removeInstalledVersionFromOptionTable($prefix . $dimension->getColumnName());
+ }
}
/**
@@ -1259,9 +1291,10 @@ class Manager
return in_array($pluginName, $pluginsInstalled);
}
- private function removeInstalledVersionFromOptionTable($version)
+ private function removeInstalledVersionFromOptionTable($name)
{
- Option::delete('version_' . $version);
+ $updater = new Updater();
+ $updater->markComponentSuccessfullyUninstalled($name);
}
private function makeSureOnlyActivatedPluginsAreLoaded()
diff --git a/core/Updater.php b/core/Updater.php
index 9190320fa7..90c8c6839c 100644
--- a/core/Updater.php
+++ b/core/Updater.php
@@ -90,6 +90,21 @@ class Updater
}
}
+ /**
+ * Marks a component as successfully uninstalled. Deletes an option
+ * that looks like `"version_$componentName"`.
+ *
+ * @param string $name The component name. Eg, a plugin name, `'core'` or dimension column name.
+ */
+ public function markComponentSuccessfullyUninstalled($name)
+ {
+ try {
+ Option::delete(self::getNameInOptionTable($name));
+ } catch (\Exception $e) {
+ // case when the option table is not yet created (before 0.2.10)
+ }
+ }
+
/**
* Returns the currently installed version of a Piwik component.
*
--
GitLab