diff options
author | Thomas Steur <thomas.steur@gmail.com> | 2015-08-29 14:02:01 +0300 |
---|---|---|
committer | Thomas Steur <thomas.steur@gmail.com> | 2015-08-29 14:02:01 +0300 |
commit | 1196447c73918b41a61bb38e17044067ca9d88a6 (patch) | |
tree | c0a43ded9671c115f53c5436a44021684c8888d1 /plugins/CustomVariables | |
parent | 96ae06bdeff514e579237776bd148794003af262 (diff) |
return min amount of available custom variables across tables instead of maximum
Diffstat (limited to 'plugins/CustomVariables')
5 files changed, 100 insertions, 25 deletions
diff --git a/plugins/CustomVariables/Archiver.php b/plugins/CustomVariables/Archiver.php index b613f65d02..bc43cbc8dd 100644 --- a/plugins/CustomVariables/Archiver.php +++ b/plugins/CustomVariables/Archiver.php @@ -66,7 +66,7 @@ class Archiver extends \Piwik\Plugin\Archiver { $this->dataArray = new DataArray(); - $maxCustomVariables = CustomVariables::getMaxCustomVariables(); + $maxCustomVariables = CustomVariables::getNumUsableCustomVariables(); for ($i = 1; $i <= $maxCustomVariables; $i++) { $this->aggregateCustomVariable($i); } diff --git a/plugins/CustomVariables/Commands/Info.php b/plugins/CustomVariables/Commands/Info.php index bf300e357a..e5dfce5d5b 100644 --- a/plugins/CustomVariables/Commands/Info.php +++ b/plugins/CustomVariables/Commands/Info.php @@ -28,7 +28,7 @@ class Info extends ConsoleCommand protected function execute(InputInterface $input, OutputInterface $output) { - $maxVars = CustomVariables::getMaxCustomVariables(); + $maxVars = CustomVariables::getNumUsableCustomVariables(); if ($this->hasEverywhereSameAmountOfVariables()) { $this->writeSuccessMessage($output, array( diff --git a/plugins/CustomVariables/CustomVariables.php b/plugins/CustomVariables/CustomVariables.php index 64a7c9ea05..7a37808909 100644 --- a/plugins/CustomVariables/CustomVariables.php +++ b/plugins/CustomVariables/CustomVariables.php @@ -40,7 +40,7 @@ class CustomVariables extends \Piwik\Plugin { $customVariables = array(); - $maxCustomVariables = self::getMaxCustomVariables(); + $maxCustomVariables = self::getNumUsableCustomVariables(); for ($i = 1; $i <= $maxCustomVariables; $i++) { if (!empty($details['custom_var_k' . $i])) { @@ -63,25 +63,41 @@ class CustomVariables extends \Piwik\Plugin return 200; } - public static function getMaxCustomVariables() + /** + * Returns the number of available custom variables that can be used. + * + * "Can be used" is identifed by the minimum number of available custom variables across all relevant tables. Eg + * if there are 6 custom variables installed in log_visit but only 5 in log_conversion, we consider only 5 custom + * variables as usable. + * @return int + */ + public static function getNumUsableCustomVariables() { $cache = Cache::getCacheGeneral(); - $cacheKey = 'CustomVariables.MaxNumCustomVariables'; + $cacheKey = 'CustomVariables.NumUsableCustomVariables'; if (!array_key_exists($cacheKey, $cache)) { - $maxCustomVar = 0; + $minCustomVar = null; foreach (Model::getScopes() as $scope) { $model = new Model($scope); $highestIndex = $model->getHighestCustomVarIndex(); - if ($highestIndex > $maxCustomVar) { - $maxCustomVar = $highestIndex; + if (!isset($minCustomVar)) { + $minCustomVar = $highestIndex; } + + if ($highestIndex < $minCustomVar) { + $minCustomVar = $highestIndex; + } + } + + if (!isset($minCustomVar)) { + $minCustomVar = 0; } - $cache[$cacheKey] = $maxCustomVar; + $cache[$cacheKey] = $minCustomVar; Cache::setCacheGeneral($cache); } @@ -90,7 +106,7 @@ class CustomVariables extends \Piwik\Plugin public function getSegmentsMetadata(&$segments) { - $maxCustomVariables = self::getMaxCustomVariables(); + $maxCustomVariables = self::getNumUsableCustomVariables(); for ($i = 1; $i <= $maxCustomVariables; $i++) { $segments[] = array( diff --git a/plugins/CustomVariables/tests/Commands/SetNumberOfCustomVariablesTest.php b/plugins/CustomVariables/tests/Commands/SetNumberOfCustomVariablesTest.php index e70c3515bb..a17a0683a5 100644 --- a/plugins/CustomVariables/tests/Commands/SetNumberOfCustomVariablesTest.php +++ b/plugins/CustomVariables/tests/Commands/SetNumberOfCustomVariablesTest.php @@ -65,7 +65,7 @@ class SetNumberOfCustomVariablesTest extends IntegrationTestCase public function testExecute_ShouldAddMaxCustomVars_IfNumberIsHigherThanActual() { - $this->assertEquals(5, CustomVariables::getMaxCustomVariables()); + $this->assertEquals(5, CustomVariables::getNumUsableCustomVariables()); $result = $this->executeCommand(6); @@ -77,13 +77,13 @@ class SetNumberOfCustomVariablesTest extends IntegrationTestCase $this->assertContains('Added a variable in scope "Conversion" having the index 6', $result); $this->assertContains('Your Piwik is now configured for 6 custom variables.', $result); - $this->assertEquals(6, CustomVariables::getMaxCustomVariables()); + $this->assertEquals(6, CustomVariables::getNumUsableCustomVariables()); } public function testExecute_ShouldRemoveMaxCustomVars_IfNumberIsLessThanActual() { $this->executeCommand(6, true); - $this->assertEquals(6, CustomVariables::getMaxCustomVariables()); + $this->assertEquals(6, CustomVariables::getNumUsableCustomVariables()); $result = $this->executeCommand(5); @@ -95,18 +95,18 @@ class SetNumberOfCustomVariablesTest extends IntegrationTestCase $this->assertContains('Removed a variable in scope "Conversion" having the index 6', $result); $this->assertContains('Your Piwik is now configured for 5 custom variables.', $result); - $this->assertEquals(5, CustomVariables::getMaxCustomVariables()); + $this->assertEquals(5, CustomVariables::getNumUsableCustomVariables()); } public function testExecute_AddMultiple_RemoveMultiple() { - $this->assertEquals(5, CustomVariables::getMaxCustomVariables()); + $this->assertEquals(5, CustomVariables::getNumUsableCustomVariables()); $this->executeCommand(9); - $this->assertEquals(9, CustomVariables::getMaxCustomVariables()); + $this->assertEquals(9, CustomVariables::getNumUsableCustomVariables()); $this->executeCommand(6); - $this->assertEquals(6, CustomVariables::getMaxCustomVariables()); + $this->assertEquals(6, CustomVariables::getNumUsableCustomVariables()); } /** diff --git a/plugins/CustomVariables/tests/Integration/CustomVariablesTest.php b/plugins/CustomVariables/tests/Integration/CustomVariablesTest.php index 75832efe37..e24bbf0090 100644 --- a/plugins/CustomVariables/tests/Integration/CustomVariablesTest.php +++ b/plugins/CustomVariables/tests/Integration/CustomVariablesTest.php @@ -8,6 +8,7 @@ namespace Piwik\Plugins\CustomVariables\tests; use Piwik\Plugins\CustomVariables\CustomVariables; +use Piwik\Plugins\CustomVariables\Model; use Piwik\Tracker\Cache; use Piwik\Tests\Framework\TestCase\IntegrationTestCase; @@ -18,27 +19,85 @@ use Piwik\Tests\Framework\TestCase\IntegrationTestCase; */ class CustomVariablesTest extends IntegrationTestCase { - public function testGetMaxCustomVariables_ShouldDetectCorrectNumberOfVariables() + public function test_getNumUsableCustomVariables_ShouldDetectCorrectNumberOfVariables() { Cache::clearCacheGeneral(); - $this->assertSame(5, CustomVariables::getMaxCustomVariables()); + $this->assertSame(5, CustomVariables::getNumUsableCustomVariables()); } - public function testGetMaxCustomVariables_ShouldCacheTheResult() + public function test_getNumUsableCustomVariables_ShouldCacheTheResult() { - CustomVariables::getMaxCustomVariables(); + CustomVariables::getNumUsableCustomVariables(); $cache = Cache::getCacheGeneral(); - $this->assertSame(5, $cache['CustomVariables.MaxNumCustomVariables']); + $this->assertSame(5, $cache['CustomVariables.NumUsableCustomVariables']); } - public function testGetMaxCustomVariables_ShouldReadFromCacheIfPossible() + public function test_getNumUsableCustomVariables_ShouldReadFromCacheIfPossible() { $cache = Cache::getCacheGeneral(); - $cache['CustomVariables.MaxNumCustomVariables'] = 10; + $cache['CustomVariables.NumUsableCustomVariables'] = 10; Cache::setCacheGeneral($cache); - $this->assertSame(10, CustomVariables::getMaxCustomVariables()); + $this->assertSame(10, CustomVariables::getNumUsableCustomVariables()); } + public function test_getNumUsableCustomVariables_ShouldReturnMinVariables_IfOneTableHasLessEntriesThanOthers() + { + $this->assertEquals(5, CustomVariables::getNumUsableCustomVariables()); + + $scopes = Model::getScopes(); + + // removing custom vars step by step... as soon as one custom var is removed, + // it should return the min count of available variables + for ($i = 4; $i != -1; $i--) { + foreach ($scopes as $scope) { + $this->dropCustomVar($scope); + $this->assertSame($i, CustomVariables::getNumUsableCustomVariables()); + } + } + + $this->assertEquals(0, CustomVariables::getNumUsableCustomVariables()); + + // add custom var, only once all custom vars are written it should write return a higher custom var number + for ($i = 1; $i != 7; $i++) { + foreach ($scopes as $index => $scope) { + $isLastIndex = $index === (count($scopes) - 1); + + $this->addCustomVar($scope); + + if ($isLastIndex) { + $this->assertSame($i, CustomVariables::getNumUsableCustomVariables()); + // all scopes have been added, it should consider all custom var counts + } else { + $this->assertSame($i - 1, CustomVariables::getNumUsableCustomVariables()); + // at least one scope is not added and should therefore return the old custom var count until all + // tables have been updated + } + } + } + + $this->assertEquals(6, CustomVariables::getNumUsableCustomVariables()); + } + + private function dropCustomVar($scope) + { + $this->clearCache(); + $model = new Model($scope); + $model->removeCustomVariable(); + } + + private function addCustomVar($scope) + { + $this->clearCache(); + $model = new Model($scope); + $model->addCustomVariable(); + } + + private function clearCache() + { + Cache::clearCacheGeneral(); + } + + } |