Add missing methods to PluginInterface and make it narrower

Define fewer methods to enable/disable hooks and instead use generic methods. This does mean that autocompletion won't work for hook names, but it means hooks could be extended more easily in the future. I also prefer the narrower interface.
cakephp · Feb 15, 2018 · 978a903 · 978a903
1 parent 44fa5e2
commit 978a903
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 158 deletions.
diff --git a/src/Core/Plugin.php b/src/Core/Plugin.php
@@ -309,7 +309,7 @@ public static function configPath($name)
     public static function bootstrap($name)
     {
         $plugin = static::getCollection()->get($name);
-        if (!$plugin->isBootstrapEnabled()) {
+        if (!$plugin->isEnabled('bootstrap')) {
             return false;
         }
 
@@ -339,7 +339,7 @@ public static function routes($name = null)
             return true;
         }
         $plugin = static::getCollection()->get($name);
-        if (!$plugin->isRoutesEnabled()) {
+        if (!$plugin->isEnabled('routes')) {
             return false;
         }
 

diff --git a/src/Core/PluginApp.php b/src/Core/PluginApp.php
@@ -87,7 +87,7 @@ class PluginApp implements PluginInterface
      */
     public function __construct(array $options = [])
     {
-        foreach (PluginCollection::VALID_HOOKS as $key) {
+        foreach (static::VALID_HOOKS as $key) {
             if (isset($options[$key])) {
                 $this->{"{$key}Enabled"} = (bool)$options[$key];
             }
@@ -172,113 +172,49 @@ public function getClassPath()
     /**
      * {@inheritdoc}
      */
-    public function disableRoutes()
+    public function enable($hook)
     {
-        $this->routesEnabled = false;
+        $this->checkHook($hook);
+        $this->{"{$hook}Enabled}"} = true;
 
         return $this;
     }
 
     /**
      * {@inheritdoc}
      */
-    public function enableRoutes()
+    public function disable($hook)
     {
-        $this->routesEnabled = true;
+        $this->checkHook($hook);
+        $this->{"{$hook}Enabled"} = false;
 
         return $this;
     }
 
     /**
      * {@inheritdoc}
      */
-    public function disableBootstrap()
+    public function isEnabled($hook)
     {
-        $this->bootstrapEnabled = false;
+        $this->checkHook($hook);
 
-        return $this;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function enableBootstrap()
-    {
-        $this->bootstrapEnabled = true;
-
-        return $this;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function disableMiddleware()
-    {
-        $this->middlewareEnabled = false;
-
-        return $this;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function enableMiddleware()
-    {
-        $this->middlewareEnabled = true;
-
-        return $this;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function disableConsole()
-    {
-        $this->consoleEnabled = false;
-
-        return $this;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function enableConsole()
-    {
-        $this->consoleEnabled = true;
-
-        return $this;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function isRoutesEnabled()
-    {
-        return $this->routesEnabled;
+        return $this->{"{$hook}Enabled"} === true;
     }
 
     /**
-     * {@inheritdoc}
-     */
-    public function isBootstrapEnabled()
-    {
-        return $this->bootstrapEnabled;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function isConsoleEnabled()
-    {
-        return $this->consoleEnabled;
-    }
-
-    /**
-     * {@inheritdoc}
+     * Check if a hook name is valid
+     *
+     * @param string $hook The hook name to check
+     * @throws \InvalidArgumentException on invalid hooks
+     * @return void
      */
-    public function isMiddlewareEnabled()
+    protected function checkHook($hook)
     {
-        return $this->middlewareEnabled;
+        if (!in_array($hook, static::VALID_HOOKS)) {
+            throw new InvalidArgumentException(
+                "`$hook` is not a valid hook name. Must be one of " . implode(', ', static::VALID_HOOKS)
+            );
+        }
     }
 
     /**

diff --git a/src/Core/PluginCollection.php b/src/Core/PluginCollection.php
@@ -36,8 +36,6 @@ class PluginCollection implements IteratorAggregate, Countable
      */
     protected $plugins = [];
 
-    const VALID_HOOKS = ['routes', 'bootstrap', 'console', 'middleware'];
-
     /**
      * Constructor
      *
@@ -137,13 +135,11 @@ public function count()
      */
     public function with($hook)
     {
-        if (!in_array($hook, static::VALID_HOOKS)) {
+        if (!in_array($hook, PluginInterface::VALID_HOOKS)) {
             throw new InvalidArgumentException("The `{$hook}` hook is not a known plugin hook.");
         }
-        $hook = ucfirst($hook);
-        $method = "is{$hook}Enabled";
         foreach ($this as $plugin) {
-            if ($plugin->{$method}()) {
+            if ($plugin->isEnabled($hook)) {
                 yield $plugin;
             }
         }

diff --git a/src/Core/PluginInterface.php b/src/Core/PluginInterface.php
@@ -10,14 +10,18 @@
  * @since         3.6.0
  * @license       https://opensource.org/licenses/mit-license.php MIT License
  */
-
 namespace Cake\Core;
 
 /**
  * Plugin Interface
  */
 interface PluginInterface
 {
+    /**
+     * The list of valid hooks
+     */
+    const VALID_HOOKS = ['routes', 'bootstrap', 'console', 'middleware'];
+
     /**
      * Get the name of this plugin.
      *
@@ -56,86 +60,50 @@ public function getClassPath();
     public function bootstrap();
 
     /**
-     * Disables route loading for the plugin
+     * Add a plugins console commands
      *
-     * @return $this
+     * @param \Cake\Console\CommandCollection $commands The command collection to update
+     * @return \Cake\Console\CommandCollection
      */
-    public function disableRoutes();
+    public function console($commands);
 
     /**
-     * Enables route loading for the plugin
+     * Add a plugins middleware
      *
-     * @return $this
+     * @param \Cake\Http\MiddlewareQueue $middleware The middleware queue to update.
+     * @return \Cake\Http\MiddlewareQueue
      */
-    public function enableRoutes();
+    public function middleware($middleware);
 
     /**
-     * Disables bootstrapping for the plugin
+     * Add a routes
      *
-     * @return $this
+     * @param \Cake\Routing\RouteBuilder $route The route builder to update.
+     * @return \Cake\Routing\RouteBuilder
      */
-    public function disableBootstrap();
+    public function routes($routes);
 
     /**
-     * Enables bootstrapping for the plugin
+     * Disables the named hook
      *
+     * @param string $hook The hook to disable
      * @return $this
      */
-    public function enableBootstrap();
+    public function disable($hook);
 
     /**
-     * Disables console commands for the plugin
+     * Enables the named hook
      *
+     * @param string $hook The hook to disable
      * @return $this
      */
-    public function disableConsole();
-
-    /**
-     * Enables console commands for the plugin
-     *
-     * @return $this
-     */
-    public function enableConsole();
-
-    /**
-     * Disables middleware for the plugin
-     *
-     * @return $this
-     */
-    public function disableMiddleware();
-
-    /**
-     * Enables middleware for the plugin
-     *
-     * @return $this
-     */
-    public function enableMiddleware();
-
-    /**
-     * If the routes should be loaded or not for this plugin
-     *
-     * @return bool
-     */
-    public function isRoutesEnabled();
-
-    /**
-     * If bootstrapping should be done or not for this plugin
-     *
-     * @return bool
-     */
-    public function isBootstrapEnabled();
-
-    /**
-     * If middleware is enabled
-     *
-     * @return bool
-     */
-    public function isMiddlewareEnabled();
+    public function enable($hook);
 
     /**
-     * If console is enabled
+     * Check if the named hook is enabled
      *
+     * @param string $hook The hook to check
      * @return bool
      */
-    public function isConsoleEnabled();
+    public function isEnabled($hook);
 }
diff --git a/tests/TestCase/Core/PluginAppTest.php b/tests/TestCase/Core/PluginAppTest.php
@@ -50,8 +50,10 @@ public function testConfigForRoutesAndBootstrap()
             'routes' => false
         ]);
 
-        $this->assertFalse($plugin->isBootstrapEnabled());
-        $this->assertFalse($plugin->isRoutesEnabled());
+        $this->assertFalse($plugin->isEnabled('routes'));
+        $this->assertFalse($plugin->isEnabled('bootstrap'));
+        $this->assertTrue($plugin->isEnabled('console'));
+        $this->assertTrue($plugin->isEnabled('middleware'));
     }
 
     public function testGetName()
@@ -91,10 +93,10 @@ public function testConstructorArguments()
             'console' => false,
             'middleware' => false
         ]);
-        $this->assertFalse($plugin->isRoutesEnabled());
-        $this->assertFalse($plugin->isBootstrapEnabled());
-        $this->assertFalse($plugin->isConsoleEnabled());
-        $this->assertFalse($plugin->isMiddlewareEnabled());
+        $this->assertFalse($plugin->isEnabled('routes'));
+        $this->assertFalse($plugin->isEnabled('bootstrap'));
+        $this->assertFalse($plugin->isEnabled('console'));
+        $this->assertFalse($plugin->isEnabled('middleware'));
     }
 
     public function testGetPathBaseClass()

diff --git a/tests/TestCase/Core/PluginCollectionTest.php b/tests/TestCase/Core/PluginCollectionTest.php
@@ -114,7 +114,7 @@ public function testWith()
     {
         $plugins = new PluginCollection();
         $plugin = new TestPlugin();
-        $plugin->disableRoutes();
+        $plugin->disable('routes');
 
         $pluginThree = new TestPluginThree();