Merge pull request #1551 from ArthurHoaro/fix/plugin-save-metadata

Plugins: do not save metadata along plugin parameters

5 files changed, 51 insertions, 31 deletions

diff --git a/application/front/controller/admin/PluginsController.php b/application/front/controller/admin/PluginsController.php
index 0e09116e..8e059681 100644
--- a/application/front/controller/admin/PluginsController.php
+++ b/application/front/controller/admin/PluginsController.php
@@ -62,6 +62,7 @@ class PluginsController extends ShaarliAdminController
 
             if (isset($parameters['parameters_form'])) {
                 unset($parameters['parameters_form']);
+                unset($parameters['token']);
                 foreach ($parameters as $param => $value) {
                     $this->container->conf->set('plugins.'. $param, escape($value));
                 }
diff --git a/application/plugin/PluginManager.php b/application/plugin/PluginManager.php
index 7881e3be..1b2197c9 100644
--- a/application/plugin/PluginManager.php
+++ b/application/plugin/PluginManager.php
@@ -100,20 +100,17 @@ class PluginManager
      */
     public function executeHooks($hook, &$data, $params = array())
     {
-        if (!empty($params['target'])) {
-            $data['_PAGE_'] = $params['target'];
-        }
-
-        if (isset($params['loggedin'])) {
-            $data['_LOGGEDIN_'] = $params['loggedin'];
-        }
-
-        if (isset($params['basePath'])) {
-            $data['_BASE_PATH_'] = $params['basePath'];
-        }
-
-        if (isset($params['bookmarkService'])) {
-            $data['_BOOKMARK_SERVICE_'] = $params['bookmarkService'];
+        $metadataParameters = [
+            'target' => '_PAGE_',
+            'loggedin' => '_LOGGEDIN_',
+            'basePath' => '_BASE_PATH_',
+            'bookmarkService' => '_BOOKMARK_SERVICE_',
+        ];
+
+        foreach ($metadataParameters as $parameter => $metaKey) {
+            if (array_key_exists($parameter, $params)) {
+                $data[$metaKey] = $params[$parameter];
+            }
         }
 
         foreach ($this->loadedPlugins as $plugin) {
@@ -128,6 +125,10 @@ class PluginManager
                 }
             }
         }
+
+        foreach ($metadataParameters as $metaKey) {
+            unset($data[$metaKey]);
+        }
     }
 
     /**
diff --git a/tests/PluginManagerTest.php b/tests/PluginManagerTest.php
index a5d5dbe9..3018999c 100644
--- a/tests/PluginManagerTest.php
+++ b/tests/PluginManagerTest.php
@@ -41,17 +41,31 @@ class PluginManagerTest extends \PHPUnit\Framework\TestCase
 
         $this->assertTrue(function_exists('hook_test_random'));
 
-        $data = array(0 => 'woot');
+        $data = [0 => 'woot'];
         $this->pluginManager->executeHooks('random', $data);
-        $this->assertEquals('woot', $data[1]);
 
-        $data = array(0 => 'woot');
+        static::assertCount(2, $data);
+        static::assertSame('woot', $data[1]);
+
+        $data = [0 => 'woot'];
         $this->pluginManager->executeHooks('random', $data, array('target' => 'test'));
-        $this->assertEquals('page test', $data[1]);
 
-        $data = array(0 => 'woot');
+        static::assertCount(2, $data);
+        static::assertSame('page test', $data[1]);
+
+        $data = [0 => 'woot'];
         $this->pluginManager->executeHooks('random', $data, array('loggedin' => true));
-        $this->assertEquals('loggedin', $data[1]);
+
+        static::assertCount(2, $data);
+        static::assertEquals('loggedin', $data[1]);
+
+        $data = [0 => 'woot'];
+        $this->pluginManager->executeHooks('random', $data, array('loggedin' => null));
+
+        static::assertCount(3, $data);
+        static::assertEquals('loggedin', $data[1]);
+        static::assertArrayHasKey(2, $data);
+        static::assertNull($data[2]);
     }
 
     /**
@@ -78,8 +92,8 @@ class PluginManagerTest extends \PHPUnit\Framework\TestCase
      */
     public function testPluginNotFound(): void
     {
-        $this->pluginManager->load(array());
-        $this->pluginManager->load(array('nope', 'renope'));
+        $this->pluginManager->load([]);
+        $this->pluginManager->load(['nope', 'renope']);
         $this->addToAssertionCount(1);
     }
 
@@ -89,18 +103,18 @@ class PluginManagerTest extends \PHPUnit\Framework\TestCase
     public function testGetPluginsMeta(): void
     {
         PluginManager::$PLUGINS_PATH = self::$pluginPath;
-        $this->pluginManager->load(array(self::$pluginName));
+        $this->pluginManager->load([self::$pluginName]);
 
-        $expectedParameters = array(
-            'pop' => array(
+        $expectedParameters = [
+            'pop' => [
                 'value' => '',
                 'desc'  => 'pop description',
-            ),
-            'hip' => array(
+            ],
+            'hip' => [
                 'value' => '',
                 'desc' => '',
-            ),
-        );
+            ],
+        ];
         $meta = $this->pluginManager->getPluginsMeta();
         $this->assertEquals('test plugin', $meta[self::$pluginName]['description']);
         $this->assertEquals($expectedParameters, $meta[self::$pluginName]['parameters']);
diff --git a/tests/front/controller/admin/PluginsControllerTest.php b/tests/front/controller/admin/PluginsControllerTest.php
index 5b59285c..9526474c 100644
--- a/tests/front/controller/admin/PluginsControllerTest.php
+++ b/tests/front/controller/admin/PluginsControllerTest.php
@@ -32,7 +32,7 @@ class PluginsControllerTest extends TestCase
         array_map(function (string $plugin) use ($path) { touch($path . '/' . $plugin); }, static::PLUGIN_NAMES);
     }
 
-    public function tearDown()
+    public function tearDown(): void
     {
         $path = __DIR__ . '/folder';
         array_map(function (string $plugin) use ($path) { unlink($path . '/' . $plugin); }, static::PLUGIN_NAMES);
@@ -125,6 +125,7 @@ class PluginsControllerTest extends TestCase
             'parameters_form' => true,
             'parameter1' => 'blip',
             'parameter2' => 'blop',
+            'token' => 'this parameter should not be saved'
         ];
 
         $request = $this->createMock(Request::class);
@@ -143,7 +144,7 @@ class PluginsControllerTest extends TestCase
             ->with('save_plugin_parameters', $parameters)
         ;
         $this->container->conf
-            ->expects(static::atLeastOnce())
+            ->expects(static::exactly(2))
             ->method('set')
             ->withConsecutive(['plugins.parameter1', 'blip'], ['plugins.parameter2', 'blop'])
         ;
diff --git a/tests/plugins/test/test.php b/tests/plugins/test/test.php
index ae5032dd..03be4f4e 100644
--- a/tests/plugins/test/test.php
+++ b/tests/plugins/test/test.php
@@ -13,6 +13,9 @@ function hook_test_random($data)
         $data[1] = 'page test';
     } elseif (isset($data['_LOGGEDIN_']) && $data['_LOGGEDIN_'] === true) {
         $data[1] = 'loggedin';
+    } elseif (array_key_exists('_LOGGEDIN_', $data)) {
+        $data[1] = 'loggedin';
+        $data[2] = $data['_LOGGEDIN_'];
     } else {
         $data[1] = $data[0];
     }