1. 08 Nov, 2012 1 commit
  2. 06 Nov, 2012 18 commits
  3. 05 Nov, 2012 8 commits
    • Fabien Potencier's avatar
      merged branch fabpot/events-refactoring (PR #525) · 0ff83c40
      Fabien Potencier authored
      This PR was merged into the master branch.
      
      Commits
      -------
      
      02a39976 moved the modularity section of the docs to its own section
      cbc769ce fixed CS
      d669a7d5 made a small refactoring for better consistency
      deb42381 refactored the exception wrapper to make the code easier to understand
      e54627f6 moved exception wrapper to its own class
      5fefc7dd fixed PHP 5.4 warning when a response is an array, dispatch the view event when an error does not return a Response
      a57f50d3 udpated docs
      8772cbca moved the before middleware to always be run after the app middlewares
      63e24c54 added the application as a third argument to the after middleware (to be consistent with the before one)
      09a61047 moved the converter and string to response listeners to proper classes
      f51a1f24 moved the middleware events to a proper event listener class
      3108bfa1 moved the locale management to its own event listener
      ca79d330 moved all subscribers registration into the dispatcher closure creation
      056fc32f refactored Silex events (refs #503, refs #504, refs #452)
      52a2fdaf removed the wrapping of the event for the exception event
      
      Discussion
      ----------
      
      [WIP] Events refactoring
      
      This PR tries to address the issues reported on the `before()` method and more generally, the issues we have with the way we manage events in Silex.
      
      Silex has special events (defined in SilexEvents), and they map with Symfony events. For instance, the `BEFORE` event maps to the `REQUEST` one from Symfony. The main difference is that the priority you set for a `BEFORE` event can only configure the priority compared to other `BEFORE` events; all the events for the `BEFORE` event are dispatched from a single priority on the `REQUEST` event. This behavior removes a lot of flexibility for no reasons. And the same goes for the other Silex events.
      
      So, this PR does is to remove the specific Silex events and replace them with regular Symfony ones. This gives you more flexibility. For instance, it means that now, you can use the `before()` method to do something before the routing (priority `Application::EARLY_EVENT`) or after it (`Application::LATE_EVENT`). And if you really need to do something after the routing and before the security, that's also possible.
      
      The biggest benefit is that with the `before()`, `after()`, `error()`, and `finish()' methods, you can pretty much change all the default Symfony behaviors. So, adding a listener directly to the dispatcher should be now pretty rare.
      
      This PR is not finished yet as the documentation is not updated (but with the added flexibility, it's going to be much easier to document exactly what you can do and how in a consistent manner), but I want to gather feedback on the philosophy first.
      
      ---------------------------------------------------------------------------
      
      by fabpot at 2012-11-03T07:56:18Z
      
      I forgot to mention that this breaks BC, but only for edge cases (and this will be documented of course).
      
      ---------------------------------------------------------------------------
      
      by yguedidi at 2012-11-03T20:00:35Z
      
      +1
      
      ---------------------------------------------------------------------------
      
      by igorw at 2012-11-04T04:34:56Z
      
      I am generally 👍 for this change.
      
      ---------------------------------------------------------------------------
      
      by fabpot at 2012-11-04T07:55:04Z
      
      I've added two commits that fix two other issues:
      
      * ca79d330: all subscribers are now registered during the creation of the dispatcher (the exception handler is not a special case anymore)
      * 3108bfa1: to avoid any problem with the local management, it is now handled in a proper listener that extends the Symfony one.
      
      ---------------------------------------------------------------------------
      
      by fabpot at 2012-11-04T08:20:36Z
      
      Another commit (f51a1f24) that moves the middleware management to an event listener (one less ugly hack).
      
      ---------------------------------------------------------------------------
      
      by fabpot at 2012-11-04T08:24:19Z
      
      At this point, I want your feedback again about another possible move I'm not sure about.
      
      Now that this PR introduces two event listener classes (`LocaleListener` and `MiddlewareListener`), what about creating two additional ones: one for the controller arguments converter and another one for the StringToResponse handling.
      
      Moving them to proper listeners makes things more consistent (and everything would be registered in the dispatcher creation closure) and more customisable (not sure you ever need to modify the default behavior though).
      
      What do you think?
      
      ---------------------------------------------------------------------------
      
      by fabpot at 2012-11-04T08:45:07Z
      
      I've just pushed a commit that implements my previous proposal (09a61047).
      
      ---------------------------------------------------------------------------
      
      by igorw at 2012-11-04T15:14:53Z
      
      👍 moving all of the subscribers out of the application class helps a lot to clean things up.
      
      * `before` and `after` should be more consistent with the `HttpKernelInterface::MASTER_REQUEST` check.
      * `error` is still a bit of a mess, if possible I'd move that to a separate class as well.
      
      ---------------------------------------------------------------------------
      
      by fabpot at 2012-11-04T15:40:41Z
      
      @igorw: The code in the error method has been moved to its own class (see 77a1878).
      
      What do you mean about the consistency of the master request check?
      
      ---------------------------------------------------------------------------
      
      by igorw at 2012-11-04T15:51:29Z
      
      Error refactoring looks good.
      
      The master request check should be return-early in both cases.
      0ff83c40
    • Fabien Potencier's avatar
    • Fabien Potencier's avatar
      fixed CS · cbc769ce
      Fabien Potencier authored
      cbc769ce
    • Fabien Potencier's avatar
      d669a7d5
    • Fabien Potencier's avatar
    • Fabien Potencier's avatar
      e54627f6
    • Fabien Potencier's avatar
      fixed PHP 5.4 warning when a response is an array, dispatch the view event... · 5fefc7dd
      Fabien Potencier authored
      fixed PHP 5.4 warning when a response is an array, dispatch the view event when an error does not return a Response
      5fefc7dd
    • Fabien Potencier's avatar
      udpated docs · a57f50d3
      Fabien Potencier authored
      Replaced the term filter by middleware (so now, we have application
      middlewares and route middlewares).
      
      Middleware documentation has been moved to its own chapter. This is not
      optimal yet as middleware are needed for some part of the usage chapter
      (which comes first).
      a57f50d3
  4. 04 Nov, 2012 11 commits
  5. 02 Nov, 2012 1 commit
  6. 31 Oct, 2012 1 commit