From 0d14e68114d31c2d70ed7e6fdf9da00ba4310645 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 13 Nov 2023 13:47:28 +1300 Subject: [PATCH 1/2] Filter out segments for disabled plugins or features --- plugins/SegmentEditor/API.php | 26 +++ .../Segment/SegmentUnavailableTest.php | 186 ++++++++++++++++++ 2 files changed, 212 insertions(+) create mode 100644 tests/PHPUnit/Integration/Segment/SegmentUnavailableTest.php diff --git a/plugins/SegmentEditor/API.php b/plugins/SegmentEditor/API.php index e5f0771b5f2..aa151e61683 100644 --- a/plugins/SegmentEditor/API.php +++ b/plugins/SegmentEditor/API.php @@ -395,11 +395,37 @@ public function getAll($idSite = false) } } + $segments = $this->filterSegmentsWithDisabledElements($segments, $idSite); $segments = $this->sortSegmentsCreatedByUserFirst($segments); return $segments; } + /** + * Filter out any segments which cannot be initialized due to disable plugins or features + * + * @param array $segments + * @param $idSite + * + * @return array + */ + private function filterSegmentsWithDisabledElements(array $segments, $idSite = false): array + { + $sites = []; + if (!empty($idSite)) { + $sites = [$idSite]; + } + + foreach ($segments as $k => $segment) { + try { + new Segment($segment['definition'], $sites); + } catch (Exception $e) { + unset($segments[$k]); + } + } + return $segments; + } + /** * Sorts segment in a particular order: * diff --git a/tests/PHPUnit/Integration/Segment/SegmentUnavailableTest.php b/tests/PHPUnit/Integration/Segment/SegmentUnavailableTest.php new file mode 100644 index 00000000000..cd0d0f469d0 --- /dev/null +++ b/tests/PHPUnit/Integration/Segment/SegmentUnavailableTest.php @@ -0,0 +1,186 @@ +addSite('test', 'http://example.org'); + + Option::set(Rules::OPTION_BROWSER_TRIGGER_ARCHIVING, 0); + } + + public function tearDown() : void + { + parent::tearDown(); + Option::set(Rules::OPTION_BROWSER_TRIGGER_ARCHIVING, 1); + } + + public function testvisitorIdSegmentIsDisabledWhenVisitorProfileUnavailable() + { + // Create a new segment that uses the visitorId property + $definition = 'visitorId==4b3d9389bae51466'; + $name = 'visitorId'; + $visitorSegmentId = API::getInstance()->add($name, $definition, $this->idSite, $autoArchive = 1, $enabledAllUsers = 1); + + // Check it is returned by the API get all segment call + $this->checkSegmentAvailable($definition, $name, true); + + // Disable the visitor progile + $this->disableVisitorProfile(true); + + // Check that the new segment is no longer returned by the API + $this->checkSegmentAvailable($definition, $name, false); + + // Clean up + API::getInstance()->delete($visitorSegmentId); + $this->disableVisitorProfile(false); + } + + public function testUserIdSegmentIsDisabledWhenVisitorProfileUnavailable() + { + // Create a new segment that uses the userId property + $definition = 'userId==4b3d9389bae51466'; + $name = 'userId'; + $userSegmentId = API::getInstance()->add($name, $definition, $this->idSite, $autoArchive = 1, $enabledAllUsers = 1); + + // Check it is returned by the API get all segment call + $this->checkSegmentAvailable($definition, $name, true); + + // Disable the visitor progile + $this->disableVisitorProfile(true); + + // Check that the new segment is no longer returned by the API + $this->checkSegmentAvailable($definition, $name, false); + + // Clean up + API::getInstance()->delete($userSegmentId); + $this->disableVisitorProfile(false); + } + + public function testPluginSegmentIsDisabledWhenPluginUnavailable() + { + + // Create a new segment that uses the userId property + $definition = 'browserName==Chrome'; + $name = 'browserName'; + $browserNameSegmentId = API::getInstance()->add($name, $definition, $this->idSite, $autoArchive = 1, $enabledAllUsers = 1); + + // Check it is returned by the API get all segment call + $this->checkSegmentAvailable($definition, $name, true); + + // Disable the devices detection plugin so the browser name metric will be unavailable + \Piwik\Plugin\Manager::getInstance()->deactivatePlugin('DevicesDetection'); + \Piwik\Plugin\Manager::getInstance()->unloadPlugin('DevicesDetection'); + $this->flushCaches(); + + // Check that the new segment is no longer returned by the API + $this->checkSegmentAvailable($definition, $name, false); + + // Clean up + \Piwik\Plugin\Manager::getInstance()->loadPlugin('DevicesDetection'); + \Piwik\Plugin\Manager::getInstance()->installLoadedPlugins(); + \Piwik\Plugin\Manager::getInstance()->activatePlugin('DevicesDetection'); + API::getInstance()->delete($browserNameSegmentId); + $this->flushCaches(); + } + + /** + * Disable the visitorlog feature + * + * @param bool $status + * + * @return void + * @throws \Exception + */ + private function disableVisitorProfile(bool $status): void + { + $settings = new SystemSettings(); + $settings->disableVisitorLog->setValue($status); + $settings->disableVisitorProfile->setValue($status); + $settings->save(); + + $this->flushCaches(); + } + + /** + * Flush the caches that contain cached segment data + * + * @return void + */ + private function flushCaches(): void + { + Cache::getLazyCache()->flushAll(); + Cache::getEagerCache()->flushAll(); + Cache::getTransientCache()->flushAll(); + } + + /** + * Check if a segment is available + * + * @param string $definition + * @param string $name + * @param bool $shouldBeEnabled + * + * @return void + */ + private function checkSegmentAvailable(string $definition, string $name, bool $shouldBeEnabled): void + { + $expected = []; + if ($shouldBeEnabled) { + $expected = [0 => + [ + 'idsegment' => 1, + 'name' => $name, + 'definition' => $definition, + 'hash' => md5($definition), + 'login' => 'super user was set', + 'enable_all_users' => 1, + 'enable_only_idsite' => 1, + 'auto_archive' => 1, + 'ts_last_edit' => null, + 'deleted' => 0, + ] + ]; + } + + $segments = API::getInstance()->getAll($this->idSite); + if (isset($segments[0]['ts_created'])) { + unset($segments[0]['ts_created']); + } + + $this->assertEquals($expected, $segments); + } +} From 0d0af35fd18f04bac36d841b11a0edd2a7663532 Mon Sep 17 00:00:00 2001 From: Ben Burgess <88810029+bx80@users.noreply.github.com> Date: Mon, 13 Nov 2023 17:37:56 +1300 Subject: [PATCH 2/2] Update plugins/SegmentEditor/API.php Co-authored-by: Michal Kleiner --- plugins/SegmentEditor/API.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/SegmentEditor/API.php b/plugins/SegmentEditor/API.php index aa151e61683..bf6d88a2601 100644 --- a/plugins/SegmentEditor/API.php +++ b/plugins/SegmentEditor/API.php @@ -413,7 +413,7 @@ private function filterSegmentsWithDisabledElements(array $segments, $idSite = f { $sites = []; if (!empty($idSite)) { - $sites = [$idSite]; + $sites = is_array($idSite) ? $idSite : [$idSite]; } foreach ($segments as $k => $segment) {