Issue #1938980 by Crell, rootatwc, tim.plunkett: Move controller derivation to route enhancers.

core/lib/Drupal/Core/CoreBundle.php

@@ -230,15 +230,10 @@ public function build(ContainerBuilder $container) {
     $container->register('mime_type_matcher', 'Drupal\Core\Routing\MimeTypeMatcher')
       ->addTag('route_filter');
 
-    $container->register('paramconverter_manager', 'Drupal\Core\ParamConverter\ParamConverterManager')
-      ->addTag('route_enhancer');
     $container->register('paramconverter.entity', 'Drupal\Core\ParamConverter\EntityConverter')
       ->addArgument(new Reference('plugin.manager.entity'))
       ->addTag('paramconverter');
 
-    $container->register('router_processor_subscriber', 'Drupal\Core\EventSubscriber\RouteProcessorSubscriber')
-      ->addArgument(new Reference('content_negotiation'))
-      ->addTag('event_subscriber');
     $container->register('router_listener', 'Symfony\Component\HttpKernel\EventListener\RouterListener')
       ->addArgument(new Reference('router'))
       ->addTag('event_subscriber');
@@ -322,7 +317,6 @@ public function build(ContainerBuilder $container) {
     $container->addCompilerPass(new RegisterAccessChecksPass());
     // Add a compiler pass for upcasting of entity route parameters.
     $container->addCompilerPass(new RegisterParamConvertersPass());
-    $container->addCompilerPass(new RegisterRouteEnhancersPass());
     // Add a compiler pass for registering services needing destruction.
     $container->addCompilerPass(new RegisterServicesForDestructionPass());
   }
@@ -380,6 +374,23 @@ protected function registerRouting(ContainerBuilder $container) {
       ->addMethodCall('setContext', array(new Reference('router.request_context')))
       ->addMethodCall('add', array(new Reference('router.dynamic')))
       ->addMethodCall('add', array(new Reference('legacy_router')));
+
+    // Add a route enhancer to upcast parameters to objects if possible.
+    $container->register('paramconverter_manager', 'Drupal\Core\ParamConverter\ParamConverterManager')
+      ->addTag('route_enhancer', array('priority' => 50));
+
+    // Add core route enhancers to dynamically derive the _controller
+    $container->register('route_enhancer.ajax', 'Drupal\Core\Routing\Enhancer\AjaxEnhancer')
+      ->addArgument(new Reference('content_negotiation'))
+      ->addTag('route_enhancer', array('priority' => 20));
+    $container->register('route_enhancer.form', 'Drupal\Core\Routing\Enhancer\FormEnhancer')
+      ->addArgument(new Reference('content_negotiation'))
+      ->addTag('route_enhancer', array('priority' => 10));
+    $container->register('route_enhancer.page', 'Drupal\Core\Routing\Enhancer\PageEnhancer')
+      ->addArgument(new Reference('content_negotiation'))
+      ->addTag('route_enhancer', array('priority' => 0));
+
+    $container->addCompilerPass(new RegisterRouteEnhancersPass());
   }
 
   /**

core/lib/Drupal/Core/EventSubscriber/RouteProcessorSubscriber.php

-<?php
-
-/**
- * @file
- * Definition of Drupal\Core\EventSubscriber\RouteProcessorSubscriber.
- */
-
-namespace Drupal\Core\EventSubscriber;
-
-use Symfony\Component\EventDispatcher\EventSubscriberInterface;
-use Symfony\Component\HttpKernel\HttpKernelInterface;
-use Symfony\Component\HttpKernel\Event\GetResponseEvent;
-use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
-use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException;
-use Symfony\Component\HttpKernel\Log\LoggerInterface;
-use Symfony\Component\HttpKernel\KernelEvents;
-use Symfony\Component\Routing\Matcher\UrlMatcherInterface;
-use Symfony\Component\Routing\Exception\ResourceNotFoundException;
-use Drupal\Core\ContentNegotiation;
-
-/**
- * Listener to process request controller information.
- */
-class RouteProcessorSubscriber implements EventSubscriberInterface {
-
-  protected $negotiation;
-
-  public function __construct(ContentNegotiation $negotiation) {
-    $this->negotiation = $negotiation;
-  }
-
-  /**
-   * Sets a default controller for a route if one was not specified.
-   *
-   * @param Symfony\Component\HttpKernel\Event\GetResponseEvent $event
-   *   Event that is created to create a response for a request.
-   */
-  public function onRequestSetController(GetResponseEvent $event) {
-    $request = $event->getRequest();
-
-    // @todo This should all get converted into one or more RouteEnhancers.
-    if (!$request->attributes->has('_content') && $this->negotiation->getContentType($request) == 'drupal_ajax') {
-      $request->attributes->set('_content', $request->attributes->get('_controller'));
-      $request->attributes->set('_controller', '\Drupal\Core\AjaxController::content');
-    }
-    elseif (!$request->attributes->has('_controller') && $this->negotiation->getContentType($request) === 'html') {
-      if ($request->attributes->has('_form')) {
-        $request->attributes->set('_controller', '\Drupal\Core\HtmlFormController::content');
-      }
-      elseif ($request->attributes->has('_content')) {
-        $request->attributes->set('_controller', '\Drupal\Core\HtmlPageController::content');
-      }
-      else {
-        throw new \InvalidArgumentException('No valid subcontroller key was found');
-      }
-    }
-  }
-
-  /**
-   * Registers the methods in this class that should be listeners.
-   *
-   * @return array
-   *   An array of event listener definitions.
-   */
-  static function getSubscribedEvents() {
-    // The RouterListener has priority 32, and we need to run after that.
-    $events[KernelEvents::REQUEST][] = array('onRequestSetController', 30);
-
-    return $events;
-  }
-
-}

core/lib/Drupal/Core/Routing/Enhancer/AjaxEnhancer.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Routing\Enhancer\AjaxEnhancer.
+ */
+
+namespace Drupal\Core\Routing\Enhancer;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Cmf\Component\Routing\Enhancer\RouteEnhancerInterface;
+use Drupal\Core\ContentNegotiation;
+
+/**
+ * Enhances an ajax route with the appropriate controller.
+ */
+class AjaxEnhancer implements RouteEnhancerInterface {
+
+  /**
+   * Content negotiation library.
+   *
+   * @var \Drupal\CoreContentNegotiation
+   */
+  protected $negotiation;
+
+  /**
+   * Constructs a new \Drupal\Core\Routing\Enhancer\AjaxEnhancer object.
+   */
+  public function __construct(ContentNegotiation $negotiation) {
+    $this->negotiation = $negotiation;
+  }
+
+  /**
+   * Implements \Symfony\Cmf\Component\Routing\Enhancer\RouteEnhancerInterface::enhance()
+   */
+  public function enhance(array $defaults, Request $request) {
+    // Old-style routes work differently, since they are their own controller.
+    if ($request->attributes->get('_legacy') == TRUE) {
+      if (empty($defaults['_content']) && $this->negotiation->getContentType($request) == 'drupal_ajax') {
+        $defaults['_content'] = $defaults['_controller'];
+        $defaults['_controller'] = '\Drupal\Core\AjaxController::content';
+      }
+    }
+    else {
+      if (empty($defaults['_controller']) && !empty($defaults['_content']) && $this->negotiation->getContentType($request) === 'drupal_ajax') {
+        $defaults['_controller'] = '\Drupal\Core\AjaxController::content';
+      }
+    }
+    return $defaults;
+  }
+
+}

core/lib/Drupal/Core/Routing/Enhancer/FormEnhancer.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Routing\Enhancer\FormEnhancer.
+ */
+
+namespace Drupal\Core\Routing\Enhancer;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Cmf\Component\Routing\Enhancer\RouteEnhancerInterface;
+use Drupal\Core\ContentNegotiation;
+
+/**
+ * Enhances a form route with the appropriate controller.
+ */
+class FormEnhancer implements RouteEnhancerInterface {
+
+  /**
+   * Content negotiation library.
+   *
+   * @var \Drupal\CoreContentNegotiation
+   */
+  protected $negotiation;
+
+  /**
+   * Constructs a new \Drupal\Core\Routing\Enhancer\FormEnhancer object.
+   */
+  public function __construct(ContentNegotiation $negotiation) {
+    $this->negotiation = $negotiation;
+  }
+
+  /**
+   * Implements \Symfony\Cmf\Component\Routing\Enhancer\RouteEnhancerInterface::enhance()
+   */
+  public function enhance(array $defaults, Request $request) {
+    if (empty($defaults['_controller']) && !empty($defaults['_form']) && $this->negotiation->getContentType($request) === 'html') {
+      $defaults['_controller'] = '\Drupal\Core\HtmlFormController::content';
+    }
+    return $defaults;
+  }
+
+}

core/lib/Drupal/Core/Routing/Enhancer/PageEnhancer.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Routing\Enhancer\PageEnhancer.
+ */
+
+namespace Drupal\Core\Routing\Enhancer;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Cmf\Component\Routing\Enhancer\RouteEnhancerInterface;
+use Drupal\Core\ContentNegotiation;
+
+/**
+ * Enhances a page route with the appropriate controller.
+ */
+class PageEnhancer implements RouteEnhancerInterface {
+
+  /**
+   * Content negotiation library.
+   *
+   * @var \Drupal\CoreContentNegotiation
+   */
+  protected $negotiation;
+
+  /**
+   * Constructs a new \Drupal\Core\Routing\Enhancer\PageEnhancer object.
+   */
+  public function __construct(ContentNegotiation $negotiation) {
+    $this->negotiation = $negotiation;
+  }
+
+  /**
+   * Implements \Symfony\Cmf\Component\Routing\Enhancer\RouteEnhancerInterface::enhance()
+   */
+  public function enhance(array $defaults, Request $request) {
+    if (empty($defaults['_controller']) && !empty($defaults['_content']) && $this->negotiation->getContentType($request) === 'html') {
+      $defaults['_controller'] = '\Drupal\Core\HtmlPageController::content';
+    }
+    return $defaults;
+  }
+
+}

core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php

@@ -106,4 +106,35 @@ public function testDynamicRoutes() {
     $this->assertResponse(200);
     $this->assertRaw('test5', 'The correct string was returned because the route was successful.');
   }
+
+  /**
+   * Checks that a request with text/html response gets rendered as a page.
+   */
+  public function testControllerResolutionPage() {
+    $this->drupalGet('/router_test/test10');
+
+    $this->assertRaw('abcde', 'Correct body was found.');
+
+    // Confirm that the page wrapping is being added, so we're not getting a
+    // raw body returned.
+    $this->assertRaw('</html>', 'Page markup was found.');
+
+    // In some instances, the subrequest handling may get confused and render
+    // a page inception style.  This test verifies that is not happening.
+    $this->assertNoPattern('#</body>.*</body>#s', 'There was no double-page effect from a misrendered subrequest.');
+  }
+
+  /**
+   * Checks that an ajax request gets rendered as an Ajax response, by mime.
+   */
+  public function testControllerResolutionAjax() {
+    // This will fail with a JSON parse error if the request is not routed to
+    // The correct controller.
+    $this->drupalGetAJAX('/router_test/test10', array(), array('Accept: application/vnd.drupal-ajax'));
+
+    $this->assertEqual($this->drupalGetHeader('Content-Type'), 'application/json', 'Correct mime content type was returned');
+
+    $this->assertRaw('abcde', 'Correct body was found.');
+  }
+
 }

core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestContent.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\router_test\TestContent.
+ */
+
+namespace Drupal\router_test;
+
+/**
+ * Test controllers that are intended to be wrapped in a main controller.
+ */
+class TestContent {
+
+  public function test1() {
+    return 'abcde';
+  }
+
+}

core/modules/system/tests/modules/router_test/router_test.routing.yml

@@ -53,3 +53,10 @@ router_test_9:
   requirements:
     _permission: 'access test7'
     _access_router_test: 'TRUE'
+
+router_test_10:
+  pattern: '/router_test/test10'
+  defaults:
+    _content: '\Drupal\router_test\TestContent::test1'
+  requirements:
+    _access: 'TRUE'