Browse Source

Remove duplicate code.

Re-use the standalone ControllerFactory in the dispatcher filter. This
allows the Dispatcher to be cleaned up a bit as exceptions for missing
controllers are raised earlier in the process.

This is a minor behavior change, but a reasonable one in my eyes.
Mark Story 10 years ago
parent
commit
cce106b283

+ 7 - 14
src/Routing/Dispatcher.php

@@ -19,7 +19,6 @@ use Cake\Event\EventDispatcherTrait;
 use Cake\Event\EventListenerInterface;
 use Cake\Network\Request;
 use Cake\Network\Response;
-use Cake\Routing\Exception\MissingControllerException;
 use LogicException;
 
 /**
@@ -55,7 +54,7 @@ class Dispatcher
      * @param \Cake\Network\Request $request Request object to dispatch.
      * @param \Cake\Network\Response $response Response object to put the results of the dispatch into.
      * @return string|null if `$request['return']` is set then it returns response body, null otherwise
-     * @throws \Cake\Routing\Exception\MissingControllerException When the controller is missing.
+     * @throws \LogicException When the controller did not get created in the Dispatcher.beforeDispatch event.
      */
     public function dispatch(Request $request, Response $response)
     {
@@ -70,19 +69,13 @@ class Dispatcher
             return null;
         }
 
-        $controller = false;
-        if (isset($beforeEvent->data['controller'])) {
-            $controller = $beforeEvent->data['controller'];
-        }
-
-        if (!($controller instanceof Controller)) {
-            throw new MissingControllerException([
-                'class' => $request->params['controller'],
-                'plugin' => empty($request->params['plugin']) ? null : $request->params['plugin'],
-                'prefix' => empty($request->params['prefix']) ? null : $request->params['prefix'],
-                '_ext' => empty($request->params['_ext']) ? null : $request->params['_ext']
-            ]);
+        if (!isset($beforeEvent->data['controller'])) {
+            throw new LogicException(
+                'The Dispatcher.beforeDispatch event did not create a controller. ' .
+                'Ensure you have added the ControllerFactoryFilter.'
+            );
         }
+        $controller = $beforeEvent->data['controller'];
 
         $response = $this->_invoke($controller);
         if (isset($request->params['return'])) {

+ 4 - 39
src/Routing/Filter/ControllerFactoryFilter.php

@@ -16,6 +16,7 @@ namespace Cake\Routing\Filter;
 
 use Cake\Core\App;
 use Cake\Event\Event;
+use Cake\Http\ControllerFactory;
 use Cake\Routing\DispatcherFilter;
 use Cake\Utility\Inflector;
 use ReflectionClass;
@@ -56,47 +57,11 @@ class ControllerFactoryFilter extends DispatcherFilter
      *
      * @param \Cake\Network\Request $request Request object
      * @param \Cake\Network\Response $response Response for the controller.
-     * @return \Cake\Controller\Controller|false Object if loaded, boolean false otherwise.
+     * @return \Cake\Controller\Controller
      */
     protected function _getController($request, $response)
     {
-        $pluginPath = $controller = null;
-        $namespace = 'Controller';
-        if (!empty($request->params['plugin'])) {
-            $pluginPath = $request->params['plugin'] . '.';
-        }
-        if (!empty($request->params['controller'])) {
-            $controller = $request->params['controller'];
-        }
-        if (!empty($request->params['prefix'])) {
-            if (strpos($request->params['prefix'], '/') === false) {
-                $namespace .= '/' . Inflector::camelize($request->params['prefix']);
-            } else {
-                $prefixes = array_map(
-                    'Cake\Utility\Inflector::camelize',
-                    explode('/', $request->params['prefix'])
-                );
-                $namespace .= '/' . implode('/', $prefixes);
-            }
-        }
-        $firstChar = substr($controller, 0, 1);
-        if (strpos($controller, '\\') !== false ||
-            strpos($controller, '.') !== false ||
-            $firstChar === strtolower($firstChar)
-        ) {
-            return false;
-        }
-        $className = false;
-        if ($pluginPath . $controller) {
-            $className = App::classname($pluginPath . $controller, $namespace, 'Controller');
-        }
-        if (!$className) {
-            return false;
-        }
-        $reflection = new ReflectionClass($className);
-        if ($reflection->isAbstract() || $reflection->isInterface()) {
-            return false;
-        }
-        return $reflection->newInstance($request, $response, $controller);
+        $factory = new ControllerFactory();
+        return $factory->create($request, $response);
     }
 }

+ 12 - 10
tests/TestCase/Routing/DispatcherTest.php

@@ -69,12 +69,6 @@ class MyPluginAppController extends Controller
 {
 }
 
-interface DispatcherTestInterfaceController
-{
-
-    public function index();
-}
-
 /**
  * MyPluginController class
  *
@@ -262,15 +256,15 @@ class DispatcherTest extends TestCase
      * testMissingControllerInterface method
      *
      * @expectedException \Cake\Routing\Exception\MissingControllerException
-     * @expectedExceptionMessage Controller class DispatcherTestInterface could not be found.
+     * @expectedExceptionMessage Controller class Interface could not be found.
      * @return void
      */
     public function testMissingControllerInterface()
     {
         $request = new Request([
-            'url' => 'dispatcher_test_interface/index',
+            'url' => 'interface/index',
             'params' => [
-                'controller' => 'DispatcherTestInterface',
+                'controller' => 'Interface',
                 'action' => 'index',
             ]
         ]);
@@ -530,7 +524,15 @@ class DispatcherTest extends TestCase
         $filter->expects($this->never())
             ->method('afterDispatch');
 
-        $request = new Request();
+        $request = new Request([
+            'url' => '/',
+            'params' => [
+                'controller' => 'Pages',
+                'action' => 'display',
+                'home',
+                'pass' => []
+            ]
+        ]);
         $res = new Response();
         $this->dispatcher->addFilter($filter);
         $this->dispatcher->dispatch($request, $res);