merged branch Seldaek/post_response (PR #2791)

Commits
-------

7c2f11f Merge pull request #1 from pminnieur/post_response
9f4391f [HttpKernel] fixed DocBlocks
2a61714 [HttpKernel] added PostResponseEvent dispatching to HttpKernel
915f440 [HttpKernel] removed BC breaks, introduced new TerminableInterface
7efe4bc [HttpKernel] Add Kernel::terminate() and HttpKernel::terminate() for post-response logic

Discussion
----------

[HttpKernel] Add Kernel::terminate() and HttpKernel::terminate() for post-response logic

This came out of a discussion on IRC about doing stuff post-response, and the fact that right now there is no best practice, and it basically requires adding code after the `->send()` call.

It's an attempt at fixing it in an official way. Of course terminate() would need to be called explicitly, and added to the front controllers, but then it offers a standard way for everyone to listen on that event and do things without slowing down the user response.

---------------------------------------------------------------------------

by stof at 2011/12/06 02:41:26 -0800

We discussed it on IRC and I suggested a way to avoid the BC break of the interface: adding a new interface (``TerminableInterface`` or whatever better name you find) containing this method.
HttpKernel, Kernel and HttpCache can then implement it without breaking the existing apps using the component (Kernel and HttpCache would need an instanceof check to see if the inner kernel implements the method)

For Symfony2 users it will mean they have to change their front controller to benefit from the new event of course, but this is easy to do.

Btw, Silex can then be able to use it without *any* change for the end users as it can be done inside ``Application::run()``

---------------------------------------------------------------------------

by pminnieur at 2011/12/06 11:47:03 -0800

@Seldaek: I opened a pull request so that the discussion on IRC is fulfilled and no BC breaks exist: https://github.com/Seldaek/symfony/pull/1/files

---------------------------------------------------------------------------

by fabpot at 2011/12/07 07:59:49 -0800

Any real-world use case for this?

---------------------------------------------------------------------------

by Seldaek at 2011/12/07 08:10:31 -0800

Doing slow stuff after the user got his response back without having to implement a message queue. I believe @pminnieur wanted to use it to send logs to loggly?

---------------------------------------------------------------------------

by pminnieur at 2011/12/07 09:08:41 -0800

Its a good practice to defer code execution without the introduction of a new software layer (like gearman, amqp, whatever tools people use to defer code execution) which may be way too much just for the goal of having fast responses, whatever my code does.

My real world use case which made me miss this feature the first time:

 > I have a calendar with a scheduled Event. For a given period of time, several Event entities will be created, coupled to the scheduled event (the schedule Event just keeps track of `startDate`, `endDate` and the `dateInterval`). Let's say we want this scheduled Event to be on every Monday-Friday, on a weekly basis, for the next 10 years.

This means I have to create `10*52*5` Event entities before I could even think about sending a simple redirect response. If I could defer code execution, I'd only save the scheduled Event, send the redirect response and after that, I create the `10*52*5` entities.

The other use case was loggly, yes. Sending logging data over the wire before the response is send doesn't make sense in my eyes, so it could be deferred after the response is send (this especially sucks if loggly fails and i get a 500 --the frontend/public user is not interested in a working logging facility, he wants his responses).

---------------------------------------------------------------------------

by mvrhov at 2011/12/07 10:07:03 -0800

This would help significantly, but the real problem, that your process is busy and unavailable for the next request, is still there.

---------------------------------------------------------------------------

by fabpot at 2011/12/07 10:15:18 -0800

I think this is the wrong solution for a real problem.

Saying "Its a good practice to defer code execution without the introduction of a new software layer" is just wrong.

It is definitely a good practice to defer code execution, but you should use the right tool for the job.

I'm -1.

---------------------------------------------------------------------------

by pminnieur at 2011/12/07 10:25:44 -0800

It should just give a possibility to put unimportant but heavy lifting code behind the send request with ease. With little effort people could benefit from the usage of `fastcgi_finish_request` without introducing new software, using `register_shutdown_function` or using `__destruct `(which works for simple things, but may act weird with dependencies).

It should not simulate node.js ;-) I agree that the real problem is not solved, but small problems could be solved easily. I personally don't want to setup RabbitMQ or whatever, maintain my crontab or any other software that may allow me to defer code execution.

---------------------------------------------------------------------------

by Seldaek at 2011/12/08 01:08:32 -0800

@fabpot: one could say that on shared hostings it is still useful because they generally don't give you gearman or \*MQs. Anyway I think it'd be nice to really complete the HttpKernel event cycle.

---------------------------------------------------------------------------

by pminnieur at 2011/12/08 01:48:57 -0800

not only on shared hostings, sometimes teams/projects just don't have the resources or knowledge or time to setup such an infrastructure.

---------------------------------------------------------------------------

by videlalvaro at 2011/12/08 01:53:06 -0800

I can say we used `fastcgi_finish_request` quite a lot at poppen with symfony 1.x. It certainly helped us to send data to Graphite, save XHProf runs, send data to RabbitMQ, and so on.

For example we used to connect to RabbitMQ and send the messages _after_ calling `fastcgi_finish_request` so the user never had to wait for stuff like that.

Also keep in mind that if you are using Gearman or RabbitMQ or whatever tool you use to defer code execution… you are not deferring the network connection handling, sending data over the wire and what not. I know this is obvious but is often overlooked.

So it would be nice to have an standard way of doing this.

---------------------------------------------------------------------------

by henrikbjorn at 2011/12/13 01:42:23 -0800

This could have been useful recently while implementing a "Poor mans cronjob" system. The solution was to do a custom Response object and do the stuff after send have been called with a Connection: Close header and ignore_user_abort(); (Yes very ugly)

Author: fabpot

src/Symfony/Component/HttpKernel/Event/PostResponseEvent.php

@@ -0,0 +1,44 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\HttpKernel\Event;
+
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\EventDispatcher\Event;
+
+/**
+ * Allows to execute logic after a response was sent
+ *
+ * @author Jordi Boggiano <[email protected]>
+ */
+class PostResponseEvent extends Event
+{
+    /**
+     * The kernel in which this event was thrown
+     * @var HttpKernelInterface
+     */
+    private $kernel;
+
+    public function __construct(HttpKernelInterface $kernel)
+    {
+        $this->kernel = $kernel;
+    }
+
+    /**
+     * Returns the kernel in which this event was thrown
+     *
+     * @return HttpKernelInterface
+     */
+    public function getKernel()
+    {
+        return $this->kernel;
+    }
+}

src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php

@@ -16,6 +16,7 @@
 namespace Symfony\Component\HttpKernel\HttpCache;
 
 use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\HttpKernel\TerminableInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 
@@ -26,7 +27,7 @@
  *
  * @api
  */
-class HttpCache implements HttpKernelInterface
+class HttpCache implements HttpKernelInterface, TerminableInterface
 {
     private $kernel;
     private $store;
@@ -215,6 +216,18 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ
         return $response;
     }
 
+    /**
+     * {@inheritdoc}
+     *
+     * @api
+     */
+    public function terminate()
+    {
+        if ($this->getKernel() instanceof TerminableInterface) {
+            $this->getKernel()->terminate();
+        }
+    }
+
     /**
      * Forwards the Request to the backend without storing the Response in the cache.
      *

src/Symfony/Component/HttpKernel/HttpKernel.php

@@ -18,6 +18,7 @@
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent;
 use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
+use Symfony\Component\HttpKernel\Event\PostResponseEvent;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
@@ -29,7 +30,7 @@
  *
  * @api
  */
-class HttpKernel implements HttpKernelInterface
+class HttpKernel implements HttpKernelInterface, TerminableInterface
 {
     private $dispatcher;
     private $resolver;
@@ -78,6 +79,17 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ
         }
     }
 
+    /**
+     * {@inheritdoc}
+     *
+     * @api
+     */
+    public function terminate()
+    {
+        $event = new PostResponseEvent($this);
+        $this->dispatcher->dispatch(KernelEvents::TERMINATE, $event);
+    }
+
     /**
      * Handles a request to convert it to a response.
      *

src/Symfony/Component/HttpKernel/Kernel.php

@@ -44,7 +44,7 @@
  *
  * @api
  */
-abstract class Kernel implements KernelInterface
+abstract class Kernel implements KernelInterface, TerminableInterface
 {
     protected $bundles;
     protected $bundleMap;
@@ -134,6 +134,22 @@ public function boot()
         $this->booted = true;
     }
 
+    /**
+     * {@inheritdoc}
+     *
+     * @api
+     */
+    public function terminate()
+    {
+        if (false === $this->booted) {
+            return;
+        }
+
+        if ($this->getHttpKernel() instanceof TerminableInterface) {
+            $this->getHttpKernel()->terminate();
+        }
+    }
+
     /**
      * Shutdowns the kernel.
      *

src/Symfony/Component/HttpKernel/KernelEvents.php

@@ -91,4 +91,15 @@ final class KernelEvents
      * @api
      */
     const RESPONSE = 'kernel.response';
+
+    /**
+     * The TERMINATE event occurs once a reponse was sent
+     *
+     * This event allows you to run expensive post-response jobs.
+     * The event listener method receives a
+     * Symfony\Component\HttpKernel\Event\PostResponseEvent instance.
+     *
+     * @var string
+     */
+    const TERMINATE = 'kernel.terminate';
 }

src/Symfony/Component/HttpKernel/TerminableInterface.php

@@ -0,0 +1,36 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\HttpKernel;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+
+/**
+ * Terminable extends the Kernel request/response cycle with dispatching a post
+ * response event after sending the response and before shutting down the kernel.
+ *
+ * @author Jordi Boggiano <[email protected]>
+ * @author Pierre Minnieur <[email protected]>
+ *
+ * @api
+ */
+interface TerminableInterface
+{
+    /**
+     * Terminates a request/response cycle.
+     *
+     * Should be called after sending the response and before shutting down the kernel.
+     *
+     * @api
+     */
+    function terminate();
+}

tests/Symfony/Tests/Component/HttpKernel/EventListener/ExceptionListenerTest.php

@@ -114,6 +114,7 @@ public function handle(Request $request, $type = self::MASTER_REQUEST, $catch =
         return new Response('foo');
     }
 
+    public function terminate() {}
 }
 
 class TestKernelThatThrowsException implements HttpKernelInterface
@@ -122,4 +123,6 @@ public function handle(Request $request, $type = self::MASTER_REQUEST, $catch =
     {
         throw new \Exception('bar');
     }
+
+    public function terminate() {}
 }

tests/Symfony/Tests/Component/HttpKernel/HttpCache/HttpCacheTest.php

@@ -11,10 +11,43 @@
 
 namespace Symfony\Tests\Component\HttpKernel\HttpCache;
 
+use Symfony\Component\HttpKernel\HttpCache\HttpCache;
+use Symfony\Component\HttpKernel\HttpCache\StoreInterface;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
 require_once __DIR__.'/HttpCacheTestCase.php';
 
 class HttpCacheTest extends HttpCacheTestCase
 {
+    public function testTerminateDelegatesTerminationOnlyForTerminableInterface()
+    {
+        $storeMock = $this->getMockBuilder('Symfony\\Component\\HttpKernel\\HttpCache\\StoreInterface')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        // does not implement TerminableInterface
+        $kernelMock = $this->getMockBuilder('Symfony\\Component\\HttpKernel\\HttpKernelInterface')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $kernelMock->expects($this->never())
+            ->method('terminate');
+
+        $kernel = new HttpCache($kernelMock, $storeMock);
+        $kernel->terminate();
+
+        // implements TerminableInterface
+        $kernelMock = $this->getMockBuilder('Symfony\\Component\\HttpKernel\\Kernel')
+            ->disableOriginalConstructor()
+            ->setMethods(array('terminate', 'registerBundles', 'registerContainerConfiguration'))
+            ->getMock();
+
+        $kernelMock->expects($this->once())
+            ->method('terminate');
+
+        $kernel = new HttpCache($kernelMock, $storeMock);
+        $kernel->terminate();
+    }
+
     public function testPassesOnNonGetHeadRequests()
     {
         $this->setNextResponse(200);

tests/Symfony/Tests/Component/HttpKernel/HttpKernelTest.php

@@ -164,6 +164,20 @@ public function testHandleWithAResponseListener()
         $this->assertEquals('foo', $kernel->handle(new Request())->getContent());
     }
 
+    public function testTerminate()
+    {
+        $dispatcher = new EventDispatcher();
+        $kernel = new HttpKernel($dispatcher, $this->getResolver());
+        $dispatcher->addListener(KernelEvents::TERMINATE, function ($event) use (&$called, &$capturedKernel) {
+            $called = true;
+            $capturedKernel = $event->getKernel();
+        });
+
+        $kernel->terminate();
+        $this->assertTrue($called);
+        $this->assertEquals($kernel, $capturedKernel);
+    }
+
     protected function getResolver($controller = null)
     {
         if (null === $controller) {

tests/Symfony/Tests/Component/HttpKernel/KernelTest.php

@@ -652,6 +652,66 @@ public function testInitializeBundleThrowsExceptionWhenABundleExtendsItself()
         $kernel->initializeBundles();
     }
 
+    public function testTerminateReturnsSilentlyIfKernelIsNotBooted()
+    {
+        $kernel = $this->getMockBuilder('Symfony\Tests\Component\HttpKernel\KernelForTest')
+            ->disableOriginalConstructor()
+            ->setMethods(array('getHttpKernel'))
+            ->getMock();
+
+        $kernel->expects($this->never())
+            ->method('getHttpKernel');
+
+        $kernel->setIsBooted(false);
+        $kernel->terminate();
+    }
+
+    public function testTerminateDelegatesTerminationOnlyForTerminableInterface()
+    {
+        // does not implement TerminableInterface
+        $httpKernelMock = $this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $httpKernelMock
+            ->expects($this->never())
+            ->method('terminate');
+
+        $kernel = $this->getMockBuilder('Symfony\Tests\Component\HttpKernel\KernelForTest')
+            ->disableOriginalConstructor()
+            ->setMethods(array('getHttpKernel'))
+            ->getMock();
+
+        $kernel->expects($this->once())
+            ->method('getHttpKernel')
+            ->will($this->returnValue($httpKernelMock));
+
+        $kernel->setIsBooted(true);
+        $kernel->terminate();
+
+        // implements TerminableInterface
+        $httpKernelMock = $this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernel')
+            ->disableOriginalConstructor()
+            ->setMethods(array('terminate'))
+            ->getMock();
+
+        $httpKernelMock
+            ->expects($this->once())
+            ->method('terminate');
+
+        $kernel = $this->getMockBuilder('Symfony\Tests\Component\HttpKernel\KernelForTest')
+            ->disableOriginalConstructor()
+            ->setMethods(array('getHttpKernel'))
+            ->getMock();
+
+        $kernel->expects($this->exactly(2))
+            ->method('getHttpKernel')
+            ->will($this->returnValue($httpKernelMock));
+
+        $kernel->setIsBooted(true);
+        $kernel->terminate();
+    }
+
     protected function getBundle($dir = null, $parent = null, $className = null, $bundleName = null)
     {
         $bundle = $this