• 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
Name
Last commit
Last update
..
Silex Loading commit data...