From ef4e84f20e59649a05240118de5ecedb56fd6996 Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Wed, 22 Oct 2014 12:55:56 +1300
Subject: [PATCH] Partial cleanup after PR...
---
core/API/Proxy.php | 87 +++++++++----------
plugins/CoreAdminHome/CoreAdminHome.php | 11 +--
.../Core/API/DocumentationGeneratorTest.php | 34 ++++++--
3 files changed, 70 insertions(+), 62 deletions(-)
diff --git a/core/API/Proxy.php b/core/API/Proxy.php
index ddcc68cb46..f4904bf349 100644
--- a/core/API/Proxy.php
+++ b/core/API/Proxy.php
@@ -78,7 +78,7 @@ class Proxy extends Singleton
$this->checkClassIsSingleton($className);
$rClass = new ReflectionClass($className);
- if(!$this->checkIfDocContainsHideAnnotation($rClass->getDocComment())) {
+ if(!$this->shouldHideAPIMethod($rClass->getDocComment())) {
foreach ($rClass->getMethods() as $method) {
$this->loadMethodMetadata($className, $method);
}
@@ -430,24 +430,25 @@ class Proxy extends Singleton
*/
private function loadMethodMetadata($class, $method)
{
- if ($this->checkIfMethodIsAvailable($method)) {
- $name = $method->getName();
- $parameters = $method->getParameters();
+ if (!$this->checkIfMethodIsAvailable($method)) {
+ return;
+ }
+ $name = $method->getName();
+ $parameters = $method->getParameters();
- $aParameters = array();
- foreach ($parameters as $parameter) {
- $nameVariable = $parameter->getName();
+ $aParameters = array();
+ foreach ($parameters as $parameter) {
+ $nameVariable = $parameter->getName();
- $defaultValue = $this->noDefaultValue;
- if ($parameter->isDefaultValueAvailable()) {
- $defaultValue = $parameter->getDefaultValue();
- }
-
- $aParameters[$nameVariable] = $defaultValue;
+ $defaultValue = $this->noDefaultValue;
+ if ($parameter->isDefaultValueAvailable()) {
+ $defaultValue = $parameter->getDefaultValue();
}
- $this->metadataArray[$class][$name]['parameters'] = $aParameters;
- $this->metadataArray[$class][$name]['numberOfRequiredParameters'] = $method->getNumberOfRequiredParameters();
+
+ $aParameters[$nameVariable] = $defaultValue;
}
+ $this->metadataArray[$class][$name]['parameters'] = $aParameters;
+ $this->metadataArray[$class][$name]['numberOfRequiredParameters'] = $method->getNumberOfRequiredParameters();
}
/**
@@ -468,40 +469,32 @@ class Proxy extends Singleton
* @param $docComment
* @return bool
*/
- public function checkIfDocContainsHideAnnotation($docComment)
+ public function shouldHideAPIMethod($docComment)
{
$hideLine = strstr($docComment, '@hide');
- if ($hideLine) {
- $hideString = trim(str_replace("@", "", strtok($hideLine, "\n")));
- $response = true;
- if (!empty($hideString)) {
- /**
- * Triggered to check if plugin should be hidden from the API for the current user.
- *
- * This event exists for checking if the user should be able to see the plugin API.
- * If &$response is set to false then the user will be able to see the plugin API.
- * If &$response is set to true then the plugin API will be hidden for the user.
- *
- * **Example**
- *
- * public function checkIfNotSuperUser(&$response)
- * {
- * try {
- * Piwik::checkUserHasSuperUserAccess();
- * $response = false;
- * } catch (\Exception $e) {
- * $response = true;
- * }
- * }
- *
- * @param bool &$response Boolean value containing information
- * if the plugin API should be hidden from the current user.
- */
- Piwik::postEvent(sprintf('API.DocumentationGenerator.%s', $hideString), array(&$response));
- }
- return $response;
+
+ if ($hideLine === false) {
+ return false;
}
- return false;
+
+ $hideLine = trim($hideLine);
+ $hideLine .= ' ';
+
+ $token = strtok($hideLine, " ");
+
+ $hide = false;
+
+ if (!empty($token)) {
+ /**
+ * This event exists for checking whether a Plugin API class or a Plugin API method tagged
+ * with a `@hideXYZ` should be hidden in the API listing.
+ *
+ * @param bool &$hide whether to hide APIs tagged with $token should be displayed.
+ */
+ Piwik::postEvent(sprintf('API.DocumentationGenerator.%s', $token), array(&$hide));
+ }
+
+ return $hide;
}
/**
@@ -522,7 +515,7 @@ class Proxy extends Singleton
return false;
}
- if ($this->checkIfDocContainsHideAnnotation($method->getDocComment())) {
+ if ($this->shouldHideAPIMethod($method->getDocComment())) {
return false;
}
diff --git a/plugins/CoreAdminHome/CoreAdminHome.php b/plugins/CoreAdminHome/CoreAdminHome.php
index deeff2e143..20ae4c4179 100644
--- a/plugins/CoreAdminHome/CoreAdminHome.php
+++ b/plugins/CoreAdminHome/CoreAdminHome.php
@@ -26,7 +26,7 @@ class CoreAdminHome extends \Piwik\Plugin
'AssetManager.getStylesheetFiles' => 'getStylesheetFiles',
'AssetManager.getJavaScriptFiles' => 'getJsFiles',
'UsersManager.deleteUser' => 'cleanupUser',
- 'API.DocumentationGenerator.hideExceptForSuperUser' => 'checkIfNotSuperUser'
+ 'API.DocumentationGenerator.@hideExceptForSuperUser' => 'displayOnlyForSuperUser'
);
}
@@ -62,13 +62,8 @@ class CoreAdminHome extends \Piwik\Plugin
$jsFiles[] = "plugins/CoreAdminHome/javascripts/pluginSettings.js";
}
- public function checkIfNotSuperUser(&$response)
+ public function displayOnlyForSuperUser(&$hide)
{
- try {
- Piwik::checkUserHasSuperUserAccess();
- $response = false;
- } catch (\Exception $e) {
- $response = true;
- }
+ $hide = !Piwik::hasUserSuperUserAccess();
}
}
diff --git a/tests/PHPUnit/Core/API/DocumentationGeneratorTest.php b/tests/PHPUnit/Core/API/DocumentationGeneratorTest.php
index 9d1ad4faa4..5fb119f6a0 100644
--- a/tests/PHPUnit/Core/API/DocumentationGeneratorTest.php
+++ b/tests/PHPUnit/Core/API/DocumentationGeneratorTest.php
@@ -17,7 +17,7 @@ use Piwik\Plugin\Manager as PluginManager;
*/
class DocumentationGeneratorTest extends PHPUnit_Framework_TestCase
{
- public function testCheckIfModuleContainsHideAnnotation()
+ public function test_CheckIfModule_ContainsHideAnnotation()
{
$annotation = '@hideExceptForSuperUser test test';
$mock = $this->getMockBuilder('ReflectionClass')
@@ -29,7 +29,7 @@ class DocumentationGeneratorTest extends PHPUnit_Framework_TestCase
$this->assertTrue($documentationGenerator->checkIfClassCommentContainsHideAnnotation($mock));
}
- public function testCheckDocumentation()
+ public function test_CheckDocumentation()
{
$moduleToCheck = 'this is documentation which contains @hideExceptForSuperUser';
$documentationAfterCheck = 'this is documentation which contains ';
@@ -37,13 +37,33 @@ class DocumentationGeneratorTest extends PHPUnit_Framework_TestCase
$this->assertEquals($documentationGenerator->checkDocumentation($moduleToCheck), $documentationAfterCheck);
}
- public function testCheckIfMethodCommentContainsHideAnnotation()
+ public function test_CheckIfMethodComment_ContainsHideAnnotation_andText()
{
$annotation = '@hideForAll test test';
- EventDispatcher::getInstance()->addObserver('API.DocumentationGenerator.hideForAll',
- function (&$response) {
- $response = true;
+ EventDispatcher::getInstance()->addObserver('API.DocumentationGenerator.@hideForAll',
+ function (&$hide) {
+ $hide = true;
});
- $this->assertEquals(Proxy::getInstance()->checkIfDocContainsHideAnnotation($annotation), true);
+ $this->assertEquals(Proxy::getInstance()->shouldHideAPIMethod($annotation), true);
+ }
+
+ public function test_CheckIfMethodComment_ContainsHideAnnotation_only()
+ {
+ $annotation = '@hideForAll';
+ EventDispatcher::getInstance()->addObserver('API.DocumentationGenerator.@hideForAll',
+ function (&$hide) {
+ $hide = true;
+ });
+ $this->assertEquals(Proxy::getInstance()->shouldHideAPIMethod($annotation), true);
+ }
+
+ public function test_CheckIfMethodComment_DoesNotContainHideAnnotation()
+ {
+ $annotation = '@not found here';
+ EventDispatcher::getInstance()->addObserver('API.DocumentationGenerator.@hello',
+ function (&$hide) {
+ $hide = true;
+ });
+ $this->assertEquals(Proxy::getInstance()->shouldHideAPIMethod($annotation), false);
}
}
\ No newline at end of file
--
GitLab