feature #828 Converters (fabpot)

This PR was merged into the master branch.

Discussion
----------

Converters

This PR reverts #769 (which tried to fix #768) as this is a breaks BC for the most usual use case of converters (#806).

The decision has been taken (#814) that a converter always run, independently of the available request attribute names or controller argument names.

Because of the above, running the converters as a controller event is not needed anymore, and moving the converters earlier in the process gives more power to the developer (#825 and #826).

Commits
-------

a0644877 Convert request attributes in the KernelEvents::REQUEST event instead of KernelEvents::CONTROLLER event (closes #825)
3b207399 added a note about global  converters being applied to all controllers
76776926 Revert "merged branch igorw/convert-only-existing-attribute (PR #769)"

doc/changelog.rst

@@ -4,6 +4,7 @@ Changelog
 1.2.0 (2013-XX-XX)
 ------------------
 
+* Reverted "convert attributes on the request that actually exist"
 * [BC BREAK] Routes are now always added in the order of their registration (even for mounted routes)
 * Added run() on Route to be able to define the controller code
 * Deprecated TwigCoreExtension (register the new HttpFragmentServiceProvider instead)

doc/usage.rst

@@ -501,6 +501,10 @@ the defaults for new controllers.
     mount as they have their own global configuration (read the
     :doc:`dedicated chapter<organizing_controllers>` for more information).
 
+.. warning::
+
+    The converters are run for **all** registered controllers.
+
 Error handlers
 --------------
 

src/Silex/EventListener/ConverterListener.php

@@ -13,7 +13,7 @@ namespace Silex\EventListener;
 
 use Silex\CallbackResolver;
 use Symfony\Component\HttpKernel\KernelEvents;
-use Symfony\Component\HttpKernel\Event\FilterControllerEvent;
+use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\Routing\RouteCollection;
 
@@ -42,26 +42,25 @@ class ConverterListener implements EventSubscriberInterface
     /**
      * Handles converters.
      *
-     * @param FilterControllerEvent $event The event to handle
+     * @param GetResponseEvent $event The event to handle
      */
-    public function onKernelController(FilterControllerEvent $event)
+    public function onKernelRequest(GetResponseEvent $event)
     {
         $request = $event->getRequest();
         $route = $this->routes->get($request->attributes->get('_route'));
         if ($route && $converters = $route->getOption('_converters')) {
             foreach ($converters as $name => $callback) {
-                if ($request->attributes->has($name)) {
                 $callback = $this->callbackResolver->isValid($callback) ? $this->callbackResolver->getCallback($callback) : $callback;
+
                 $request->attributes->set($name, call_user_func($callback, $request->attributes->get($name), $request));
             }
         }
     }
-    }
 
     public static function getSubscribedEvents()
     {
         return array(
-            KernelEvents::CONTROLLER => 'onKernelController',
+            KernelEvents::REQUEST => 'onKernelRequest',
         );
     }
 }

tests/Silex/Tests/ApplicationTest.php

@@ -122,6 +122,25 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals('foobar', $response->getContent());
     }
 
+    public function testConvertedAttributeIsAvailableInBeforeFilter()
+    {
+        $app = new Application();
+
+        $beforeValue = null;
+        $app
+            ->get('/foo/{foo}', function ($foo) {
+                return $foo;
+            })
+            ->convert('foo', function ($foo) { return $foo.'converted'; })
+            ->before(function (Request $request) use (&$beforeValue) {
+                $beforeValue = $request->attributes->get('foo');
+            });
+
+        $response = $app->handle(Request::create('/foo/bar'));
+        $this->assertEquals('barconverted', $beforeValue);
+        $this->assertEquals('barconverted', $response->getContent());
+    }
+
     public function testOn()
     {
         $app = new Application();

tests/Silex/Tests/ConverterTest.php

-<?php
-
-/*
- * This file is part of the Silex framework.
- *
- * (c) Fabien Potencier <fabien@symfony.com>
- *
- * This source file is subject to the MIT license that is bundled
- * with this source code in the file LICENSE.
- */
-
-namespace Silex\Tests;
-
-use Silex\Application;
-use Symfony\Component\HttpFoundation\Request;
-
-/**
- * Converter listener test cases.
- *
- * @author Igor Wiedler <igor@wiedler.ch>
- */
-class ConverterTest extends \PHPUnit_Framework_TestCase
-{
-    public function testConvertingNonExistentAttributeShouldNotCallConverter()
-    {
-        $called = false;
-        $converter = function () use (&$called) {
-            $called = true;
-        };
-
-        $app = new Application();
-        $app->get('/', function () { return 'hallo'; });
-        $app['controllers']->convert('foo', $converter);
-
-        $request = Request::create('/');
-        $app->handle($request);
-
-        $this->assertFalse($called);
-    }
-}