1. 11 Mar, 2012 2 commits
    • Fabien Potencier's avatar
      tweaked documentation · 52c5bd4b
      Fabien Potencier authored
      52c5bd4b
    • Fabien Potencier's avatar
      merged branch DrBenton/after-on-controllers (PR #272) · 3e6fe5e9
      Fabien Potencier authored
      Commits
      -------
      
      6445b9e9 Add Routes Middlewares
      
      Discussion
      ----------
      
      add "after($callback)" to Controller in order to mimic a "Route Middleware" feature in Silex
      
      Hi !
      
      As discussed in #262 and in my previous similar PR (https://github.com/fabpot/Silex/pull/270) with @GromNaN, here is another implementation of the Routes Middlewares feature, shipped with its Unit Test.
      
      The feature addition is still 100% backward compatible, but this time it may be more "Silex style" compliant - and it's "100% no performance loss" :-)
      
      With this PR the Silex ```Controller``` class has a new ```before()``` method, which registers a callback to trigger when the Route is matched, just before the route callback itself.
      This ```before()``` method can be called more than one time on a single Route definition, allowing multiple secondary callbacks (a.k.a. "Routes Middlewares") to be executed before the Route main callback.
      
      Would this feature be eligible for Silex integration, like this ?
      
      Olivier
      
      ---------------------------------------------------------------------------
      
      by DrBenton at 2012-03-01T22:16:05Z
      
      * The method is now named ```addMiddleware```, which may be more explicit for people used to "a la Sinatra" framewoks like _Node.js Express_  or _Slim_.
      * I no longer use the Silex ControllerResolver, but a dedicated Application protected service instead, fired just before the Application "before" process (I could have added a new listener on the ```Kernel```, but since we already have one for the right event for the "before" process, I used this one for simpler code).
      * I also added some checks on the ```$request``` in the Unit Test.
      
      ---------------------------------------------------------------------------
      
      by fabpot at 2012-03-02T11:40:30Z
      
      Can you add some information about this new feature in the documentation (with some typical usages)? Thanks for your work on this feature.
      
      ---------------------------------------------------------------------------
      
      by DrBenton at 2012-03-02T12:35:44Z
      
      **Done !** (the method is now named ```middleware```, and I've removed the useless comment)
      
      I've tried my best for the feature documentation, but unfortunately my english is fairly poor... :-/
      
      At this point, the Middlewares have access to the Request, but if I'm not wrong it can't be used for redirecting, for instance. Should I handle ```Response``` returns from middlewares, as in ```before``` filter ?
      
      ---------------------------------------------------------------------------
      
      by GromNaN at 2012-03-02T12:39:00Z
      
      I guess you can throw an exception (access denied for example) and manage this exception with the error handlers.
      
      ---------------------------------------------------------------------------
      
      by DrBenton at 2012-03-02T12:44:38Z
      
      Yes, in the sample usage of the documentation I've used exactly that ```Exception``` process.
      But I don't know if this is ok for everyone ?
      
      ---------------------------------------------------------------------------
      
      by igorw at 2012-03-02T15:08:29Z
      
      Very nice feature. But it should really be possible to return a response from the handler, like you can do with before() filters.
      
      ---------------------------------------------------------------------------
      
      by DrBenton at 2012-03-02T21:15:41Z
      
      All right, I think I know how to handle Responses, thanks to the code of the before() filter ; I'll push soon a new commit with the last code syle fixes and this feature !
      
      ---------------------------------------------------------------------------
      
      by DrBenton at 2012-03-03T11:34:44Z
      
      Ok, I've fixed the code style problems, and handled the Responses from middlewares.
      What do you think of it ?
      
      ---------------------------------------------------------------------------
      
      by DrBenton at 2012-03-05T13:11:56Z
      
      So... Are we OK for this feature, gentlemen ? :-)
      Do you want me to push a squashed commit ?
      
      ---------------------------------------------------------------------------
      
      by igorw at 2012-03-05T22:24:53Z
      
      Apart from the minor comments I made I am quite happy with the implementation as it is. I guess when those are fixed it can be merged.
      
      ---------------------------------------------------------------------------
      
      by DrBenton at 2012-03-05T22:58:23Z
      
      Sorry for all these "code style" errors, I'm still not enough used to Symfony 2 coding standards - but I'll get them ! :-)
      
      Well, all these errors should be fixed now. I also swapped  the order of execution with the Silex global ```before``` filters, as you asked it: the global filters are now triggered **before** the middlewares of the matched route.
      
      ---------------------------------------------------------------------------
      
      by igorw at 2012-03-05T23:10:35Z
      
      I'm not so sure about checking for `instanceof Response`. Maybe we should enforce a middleware to return either null or a Response to prevent unexpected behaviour.
      
      ---------------------------------------------------------------------------
      
      by DrBenton at 2012-03-06T14:37:42Z
      
      Well, I just copied the behaviour of the ```before``` filter :
      ```
      if ($ret instanceof Response) {
          $event->setResponse($ret);
      }```
      (in Application.php, line 237)
      
      Since route middlewares act as this ```before``` filter (in a "specific for each route" way), I thought we could copy the code of this ```before``` functionnality ?
      
      ---------------------------------------------------------------------------
      
      by DrBenton at 2012-03-08T14:34:13Z
      
      So ? Do you want me to change the ```instanceof Response```, or can we keep the current "copy of before filter" implementation ?
      
      ---------------------------------------------------------------------------
      
      by igorw at 2012-03-08T15:57:00Z
      
      IMO we should add something like this:
      
          if ($response instanceof Response) {
              ...
          } elseif (null !== $response) {
              throw new \RuntimeException('Before filter (or middleware for route foo) returned an invalid response value. Must return null or an instance of Response.');
          }
      
      ---------------------------------------------------------------------------
      
      by DrBenton at 2012-03-08T16:26:54Z
      
      Hum, I see. Sounds good to me, I'll push it soon with the documentation fixes :-)
      
      ---------------------------------------------------------------------------
      
      by igorw at 2012-03-08T16:42:48Z
      
      And tests please ;-)
      
      ---------------------------------------------------------------------------
      
      by DrBenton at 2012-03-08T22:15:11Z
      
      Here it is, sir ! :-)
      
      ---------------------------------------------------------------------------
      
      by DrBenton at 2012-03-10T19:45:59Z
      
      Shall I push in a new PR these 13 commits in a single squashed one ?
      
      ---------------------------------------------------------------------------
      
      by igorw at 2012-03-10T19:51:53Z
      
      👍
      
      ---------------------------------------------------------------------------
      
      by stof at 2012-03-10T19:52:43Z
      
      No need to create a new PR for this. Simply squash your commits and force the push to the branch
      
      ---------------------------------------------------------------------------
      
      by DrBenton at 2012-03-10T21:59:12Z
      
      All right, here is the squashed commit ! :-)
      3e6fe5e9
  2. 10 Mar, 2012 1 commit
  3. 02 Mar, 2012 4 commits
  4. 27 Feb, 2012 12 commits
  5. 26 Feb, 2012 1 commit
  6. 24 Feb, 2012 2 commits
  7. 22 Feb, 2012 1 commit
  8. 21 Feb, 2012 1 commit
    • Fabien Potencier's avatar
      merged branch igorw/composer (PR #259) · 219ba070
      Fabien Potencier authored
      Commits
      -------
      
      2a17fc96 [composer] update composer dependencies
      
      Discussion
      ----------
      
      [composer] update composer dependencies
      
      ---------------------------------------------------------------------------
      
      by igorw at 2012-02-20T20:56:36Z
      
      This is kind of urgent, since it breaks silex for everyone who is using it with composer. If you could merge this ASAP, would be great.
      
      ---------------------------------------------------------------------------
      
      by stof at 2012-02-20T22:21:26Z
      
      @igorw see my suggestion on doctrine/DoctrineBundle#24 I think it is what composer will force us to do instead of using master as main branch to be able to keep a sane versionning
      219ba070
  9. 20 Feb, 2012 1 commit
  10. 17 Feb, 2012 2 commits
  11. 16 Feb, 2012 1 commit
  12. 08 Feb, 2012 1 commit
  13. 04 Feb, 2012 6 commits
    • Fabien Potencier's avatar
      fixed typo · e002b5f0
      Fabien Potencier authored
      e002b5f0
    • Fabien Potencier's avatar
      added the cookbook to the index page · 0923a6c8
      Fabien Potencier authored
      0923a6c8
    • Fabien Potencier's avatar
      fixed CS · 1b6cf0e5
      Fabien Potencier authored
      1b6cf0e5
    • Fabien Potencier's avatar
      merged branch igorw/cookbook (PR #238) · bb2469de
      Fabien Potencier authored
      Commits
      -------
      
      9289dad7 [cookbook] implement some suggestions
      9c04345d [cookbook] initial recipe for accepting a JSON request body
      
      Discussion
      ----------
      
      [cookbook] initial recipe for accepting a JSON request body
      
      ---------------------------------------------------------------------------
      
      by igorw at 2012-01-17T13:24:48Z
      
      I implemented your suggestions.
      bb2469de
    • Fabien Potencier's avatar
      updated vendors · 6131991b
      Fabien Potencier authored
      6131991b
    • Fabien Potencier's avatar
      merged branch brtriver/twig-service-provider-bug (PR #245) · 3a3fc890
      Fabien Potencier authored
      Commits
      -------
      
      4453e9fc fixed for the latest TwigFormExtension
      
      Discussion
      ----------
      
      fixed for the latest TwigFormExtension
      
      Now TwigFormExtension requires CsrfProviderInterface as the first arg but there is no arg for this in TwigServiceProvider, so it occurs the error below:
      
      > Catchable fatal error: Argument 1 passed to Symfony\Bridge\Twig\Extension\FormExtension::__construct() must implement interface Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface, array given
      
      then I set `$app['form.csrf_provider']` as the first arg.
      3a3fc890
  14. 24 Jan, 2012 1 commit
  15. 21 Jan, 2012 4 commits
    • Fabien Potencier's avatar
      merged branch loalf/validator-tests (PR #230) · 9ac208d4
      Fabien Potencier authored
      Commits
      -------
      
      8d9bb8fb Added tests for Validator Provider
      291bac8d Added Validator Component as submodule
      
      Discussion
      ----------
      
      Added tests for Validator Provider
      
      I also had to add the Validator Component as a submodule
      9ac208d4
    • Fabien Potencier's avatar
      merged branch shieldo/patch-1 (PR #232) · e9b5d444
      Fabien Potencier authored
      Commits
      -------
      
      90f24072 added mention of SymfonyBridgesServiceProvider to example of using path() Twig function
      
      Discussion
      ----------
      
      added mention of SymfonyBridgesServiceProvider to example of using path Twig function
      
      I just spent an hour trying to figure out why using path() in Twig when using Silex wasn't working.  Turns out I hadn't included the SymfonyBridgesServiceProvider, which isn't mentioned in this piece of documentation.
      
      I don't know if this makes the comment too verbose, but if it saves someone else wasting time it'll be worth it.
      e9b5d444
    • Fabien Potencier's avatar
      merged branch tacker/patch-1 (PR #236) · 3d03340e
      Fabien Potencier authored
      Commits
      -------
      
      80c96c85 Added link to protected closure section. See #235.
      
      Discussion
      ----------
      
      Added link to protected closure section. See #235.
      3d03340e
    • Fabien Potencier's avatar
      merged branch jdreesen/patch-1 (PR #237) · 4f286d02
      Fabien Potencier authored
      Commits
      -------
      
      16599fbc fixed link to SwiftmailerServiceProvider doc
      
      Discussion
      ----------
      
      Fixed link to SwiftmailerServiceProvider doc
      4f286d02