Add PSR-7 to Symfony Response View listener

1. +++ b/core/lib/Drupal/Core/EventSubscriber/PsrResponseSubscriber.php
   @@ -0,0 +1,49 @@
   +        if (!$controllerResult instanceof ResponseInterface) {
   +            return;
   +        }
   +        $event->setResponse($this->httpFoundationFactory->createResponse($controllerResult));
   

   This could be simplified. if it is an instance of ResponseInterface, then do the setResponse() call. Else do nothing.

2. +++ b/core/lib/Drupal/Core/EventSubscriber/PsrResponseSubscriber.php
   @@ -0,0 +1,49 @@
   +        $events[KernelEvents::TERMINATE][] = array('onKernelView');
   

   This should be KernelEvents::VIEW. TERMINATE is a different event.

+++ b/core/core.services.yml
@@ -852,6 +863,13 @@ services:
+  route_converts_psr_response_to_httpfoundation:

That's a very long and unweidly service name. Please check the other event subscribers and follow their style. If all else fails, the class name makes a good starting point.

Please merge both patches into a single patch file. It doesn't work if each file is in its own diff.

We also need a test to verify that it works. One simple test should be sufficient. Have a controller in a test module that returns a ResponseInterface object, and make sure that when requesting that path everything works correctly. (Stop by #drupal-contribute if you need help setting that up.)

Also note that you won't be able to finish this issue until #2497691: Include Symfony PSR-7 bridge library is in, since that's how you'd have a PSR-7 response to return. :-) It's a bit jumping the gun to start with this issue, but thank you for doing so!

Also, check your whitespace. There are a number of lines that have trailing whitespace, and there should be one blank line between a method and the start of the docblock after it.

Thanks!

1. +++ b/core/core.services.yml
   @@ -852,6 +863,13 @@ services:
   +      - { name: event_subscriber }
   +      ¶
   +  ¶
   

   core.services.yml and RouterTest.php still have trailing whitespace. I suggest setting your IDE to auto-trim whitespace on save. It makes life a lot easier. :-)

2. +++ b/core/lib/Drupal/Core/EventSubscriber/PsrResponseSubscriber.php
   @@ -0,0 +1,62 @@
   +  /**
   +   * The httpFoundation factory.
   +   *
   +   * @var HttpFoundationFactoryInterface
   +   */
   +  private $httpFoundationFactory;
   

   Drupal convention is to use protected, not private, for object member variables. There are a few exceptions, but this isn't one of them.

3. +++ b/core/lib/Drupal/Core/EventSubscriber/PsrResponseSubscriber.php
   @@ -0,0 +1,62 @@
   +  public static function getSubscribedEvents() {
   +    $events[KernelEvents::VIEW][] = array('onKernelView');
   +    return $events;
   +  }
   

   This is a new class, so let's be consistent and use new-style [] syntax everywhere. I'm not sure if we need to specify a priority on this listener.

   Obviously none of this will work until the library is available. :-)

The blocking issue is in, so this can move forward.

Lookin' great! Just a few minor points, all of them nitpicks:

1. +++ b/core/lib/Drupal/Core/EventSubscriber/PsrResponseSubscriber.php
   @@ -0,0 +1,62 @@
   +    $events[KernelEvents::VIEW][] = ['onKernelView'];
   

   Note that not setting a priority means that it gets a priority of 0, which is pretty low. However, in this case we want that because this is a likely-rare subscriber, so only initializing it late (after the more common render-based listeners have a try) makes sense to me.

   No change needed here, just calling it out.

2. +++ b/core/modules/system/tests/modules/router_test_directory/src/TestControllers.php
   @@ -12,6 +12,9 @@
   +use Zend\Diactoros\Response as PSRResponse;
   

   Nit: Alias to PsrResponse, not PSRResponse.

3. +++ b/core/modules/system/tests/modules/router_test_directory/src/TestControllers.php
   @@ -102,6 +105,13 @@ public function test21() {
   +    return $response;
   

   I believe the version of Diactoros we included has utility subclasses available, so you can just use return new HtmlResponse('test123'); and it will handle creating the stream for you. It should still be a valid test of this patch.

   (And that would mean no need to alias the class above, invalidating my previous comment.)

4. +++ b/core/tests/Drupal/Tests/Core/EventSubscriber/PsrResponseSubscriberTest.php
   @@ -0,0 +1,102 @@
   +    $event->expects($this->once())->method('setResponse')->with($this->isInstanceOf('Symfony\Component\HttpFoundation\Response'));
   

   Nit: Please align the method calls vertically to make it easier to read. To wit:

   $event
     ->expects(...)
     ->method(...)
     ->with(...);
   

   (Same in a few other places.)

5. +++ b/core/tests/Drupal/Tests/Core/EventSubscriber/PsrResponseSubscriberTest.php
   @@ -0,0 +1,102 @@
   +    $event = $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent', array(), array(), '', NULL);
   

   As long as we're going to need another reroll anyway, let's use new-style arrays exclusively.

I was more indicating that the current code was fine. The order is not critical as there's no dependencies, and the default puts it after most of our common stuff anyway I think.

So, RTBC! (Darling it's better with those four letters...)

Looks like bot being silly.