moving getMaxCustomVariables to CustomVariables class, a model should not really care about caching etc

5 files changed, 85 insertions, 44 deletions

diff --git a/plugins/CustomVariables/Archiver.php b/plugins/CustomVariables/Archiver.php
index d1f31f9c59..f2bdd4d6da 100644
--- a/plugins/CustomVariables/Archiver.php
+++ b/plugins/CustomVariables/Archiver.php
@@ -15,7 +15,6 @@ use Piwik\DataArray;
 use Piwik\Metrics;
 use Piwik\Tracker;
 use Piwik\Tracker\GoalManager;
-use Piwik\Plugins\CustomVariables\Model as CustomVariablesModel;
 
 require_once PIWIK_INCLUDE_PATH . '/libs/PiwikTracker/PiwikTracker.php';
 
@@ -60,7 +59,7 @@ class Archiver extends \Piwik\Plugin\Archiver
     {
         $this->dataArray = new DataArray();
 
-        for ($i = 1; $i <= CustomVariablesModel::getMaxCustomVariables(); $i++) {
+        for ($i = 1; $i <= CustomVariables::getMaxCustomVariables(); $i++) {
             $this->aggregateCustomVariable($i);
         }
 
diff --git a/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php b/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php
index 57fa3f174d..acd7233a1e 100644
--- a/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php
+++ b/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php
@@ -10,6 +10,7 @@
 namespace Piwik\Plugins\CustomVariables\Commands;
 
 use Piwik\Plugin\ConsoleCommand;
+use Piwik\Tracker\Cache;
 use Piwik\Plugins\CustomVariables\Model;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
@@ -65,6 +66,7 @@ class SetNumberOfCustomVariables extends ConsoleCommand
             $this->performChange($scope, $numVarsToSet, $output);
         }
 
+        Cache::clearCacheGeneral();
         $this->progress->finish();
 
         $this->writeSuccessMessage($output, array(
diff --git a/plugins/CustomVariables/CustomVariables.php b/plugins/CustomVariables/CustomVariables.php
index 396d2ef382..6a7ae6618e 100644
--- a/plugins/CustomVariables/CustomVariables.php
+++ b/plugins/CustomVariables/CustomVariables.php
@@ -14,6 +14,7 @@ use Piwik\Piwik;
 use Piwik\Plugin\ViewDataTable;
 use Piwik\Tracker;
 use Piwik\WidgetsList;
+use Piwik\Tracker\Cache;
 
 /**
  */
@@ -58,6 +59,40 @@ class CustomVariables extends \Piwik\Plugin
         Model::install();
     }
 
+    /**
+     * There are also some hardcoded places in JavaScript
+     * @return int
+     */
+    public static function getMaxLengthCustomVariables()
+    {
+        return 200;
+    }
+
+    public static function getMaxCustomVariables()
+    {
+        $cache    = Cache::getCacheGeneral();
+        $cacheKey = 'CustomVariables.MaxNumCustomVariables';
+
+        if (!array_key_exists($cacheKey, $cache)) {
+
+            $maxCustomVar = 0;
+
+            foreach (Model::getScopes() as $scope) {
+                $model = new Model($scope);
+                $highestIndex = $model->getHighestCustomVarIndex();
+
+                if ($highestIndex > $maxCustomVar) {
+                    $maxCustomVar = $highestIndex;
+                }
+            }
+
+            $cache[$cacheKey] = $maxCustomVar;
+            Cache::setCacheGeneral($cache);
+        }
+
+        return $cache[$cacheKey];
+    }
+
     public function addConsoleCommands(&$commands)
     {
         $commands[] = __NAMESPACE__ . '\\Commands\\SetNumberOfCustomVariables';
@@ -92,7 +127,7 @@ class CustomVariables extends \Piwik\Plugin
 
     public function getSegmentsMetadata(&$segments)
     {
-        for ($i = 1; $i <= Model::getMaxCustomVariables(); $i++) {
+        for ($i = 1; $i <= self::getMaxCustomVariables(); $i++) {
             $segments[] = array(
                 'type'       => 'dimension',
                 'category'   => 'CustomVariables_CustomVariables',
diff --git a/plugins/CustomVariables/Model.php b/plugins/CustomVariables/Model.php
index d33fe3353c..252718f87a 100644
--- a/plugins/CustomVariables/Model.php
+++ b/plugins/CustomVariables/Model.php
@@ -12,7 +12,6 @@ use Piwik\Common;
 use Piwik\DataTable;
 use Piwik\Db;
 use Piwik\Log;
-use Piwik\Tracker\Cache;
 
 class Model
 {
@@ -92,8 +91,6 @@ class Model
         Db::exec(sprintf('ALTER TABLE %s DROP COLUMN custom_var_k%d', $dbTable, $index));
         Db::exec(sprintf('ALTER TABLE %s DROP COLUMN custom_var_v%d', $dbTable, $index));
 
-        Cache::clearCacheGeneral();
-
         return $index;
     }
 
@@ -101,11 +98,10 @@ class Model
     {
         $dbTable = Common::prefixTable($this->scope);
         $index   = $this->getHighestCustomVarIndex() + 1;
+        $maxLen  = CustomVariables::getMaxLengthCustomVariables();
 
-        Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_k%d VARCHAR(%d) DEFAULT NULL', $dbTable, $index, self::getMaxLengthCustomVariables()));
-        Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_v%d VARCHAR(%d) DEFAULT NULL', $dbTable, $index, self::getMaxLengthCustomVariables()));
-
-        Cache::clearCacheGeneral();
+        Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_k%d VARCHAR(%d) DEFAULT NULL', $dbTable, $index, $maxLen));
+        Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_v%d VARCHAR(%d) DEFAULT NULL', $dbTable, $index, $maxLen));
 
         return $index;
     }
@@ -124,40 +120,6 @@ class Model
         return array(self::SCOPE_PAGE, self::SCOPE_VISIT, self::SCOPE_CONVERSION);
     }
 
-    public static function getMaxCustomVariables()
-    {
-        $cache    = Cache::getCacheGeneral();
-        $cacheKey = 'CustomVariables.MaxNumCustomVariables';
-
-        if (!array_key_exists($cacheKey, $cache)) {
-
-            $maxCustomVar = 0;
-
-            foreach (self::getScopes() as $scope) {
-                $model = new Model($scope);
-                $highestIndex = $model->getHighestCustomVarIndex();
-
-                if ($highestIndex > $maxCustomVar) {
-                    $maxCustomVar = $highestIndex;
-                }
-            }
-
-            $cache[$cacheKey] = $maxCustomVar;
-            Cache::setCacheGeneral($cache);
-        }
-
-        return $cache[$cacheKey];
-    }
-
-    /**
-     * There are also some hardcoded places in JavaScript
-     * @return int
-     */
-    public static function getMaxLengthCustomVariables()
-    {
-        return 200;
-    }
-
     public static function install()
     {
         foreach (self::getScopes() as $scope) {
diff --git a/plugins/CustomVariables/tests/CustomVariablesTest.php b/plugins/CustomVariables/tests/CustomVariablesTest.php
new file mode 100644
index 0000000000..980401d608
--- /dev/null
+++ b/plugins/CustomVariables/tests/CustomVariablesTest.php
@@ -0,0 +1,43 @@
+<?php
+/**
+ * Piwik - Open source web analytics
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\CustomVariables\tests;
+use Piwik\Plugins\CustomVariables\CustomVariables;
+use Piwik\Tracker\Cache;
+
+/**
+ * @group CustomVariables
+ * @group CustomVariablesTest
+ * @group Database
+ */
+class CustomVariablesTest extends \DatabaseTestCase
+{
+    public function testGetMaxCustomVariables_ShouldDetectCorrectNumberOfVariables()
+    {
+        Cache::clearCacheGeneral();
+        $this->assertSame(5, CustomVariables::getMaxCustomVariables());
+    }
+
+    public function testGetMaxCustomVariables_ShouldCacheTheResult()
+    {
+        CustomVariables::getMaxCustomVariables();
+        $cache = Cache::getCacheGeneral();
+
+        $this->assertSame(5, $cache['CustomVariables.MaxNumCustomVariables']);
+    }
+
+    public function testGetMaxCustomVariables_ShouldReadFromCacheIfPossible()
+    {
+        $cache = Cache::getCacheGeneral();
+        $cache['CustomVariables.MaxNumCustomVariables'] = 10;
+        Cache::setCacheGeneral($cache);
+
+        $this->assertSame(10, CustomVariables::getMaxCustomVariables());
+    }
+
+}