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.

fixed PHP 5.4 warning when a response is an array, dispatch the view event when an error does not return a Response

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).

This PR was merged into the master branch.

Commits
-------

ff7bac34 Make SecurityServiceProvider::addFakeRoute less confusing

Discussion
----------

Make SecurityServiceProvider::addFakeRoute less confusing

This PR was merged into the master branch.

Commits
-------

dc8abdd8 Add note about defining the login route, closes #447

Discussion
----------

Add note about defining the login route, closes #447

added the application as a third argument to the after middleware (to be consistent with the before one)

This is mainly useful when working with Traits.

That removes the ugly route_*_middlewares_trigger services.

That's ensure that the locale is managed in only one place and always at
the right time.

This removes yet another special case that should not exist. Now, all
listeners/subscribers registration are done in one place.

If you want to disable the exception_handler, you now should call the
disable() method on it. You can still unset it but be careful to do it
as early as possible (which is anyway always the best idea).

Removed the SilexEvents class. Using the standard Symfony events allows
us to be more flexible (the priorities are the same as the one from
Symfony, so you can hook at any place into the HTTP handling).

Updated documentation to make it accurate with the current behavior, and
extended it to explain the flexibility the developer has.

It is indeed a good thing to do but for consistency, it must be done for
all events. As this is not possible now, I prefer to revert it for now.

This PR was merged into the master branch.

Commits
-------

5c5dae19 Flipped condition
9f4a122f Log a little more information about exceptions

Discussion
----------

Log a little more information about exceptions

This takes a little from Symfony's ExceptionListener, logging a little more appropriate information. Without this code, 404's etc are logged with a blank message.

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

by MarcinKleczek at 2012-10-29T11:53:41Z

+1

This PR was merged into the master branch.

Commits
-------

33d802ec updated FormTrait documentation

Discussion
----------

Updated doc/providers/form.rst

Hello,

Only one line changed. FormTrait adds ```function form($data, $options)``` to \Silex\Application, not ```form($name, $data, $options)``` like createBuilder; this example in docs was throwing exception.

(this is my first pull request and now I see I should have done it in separate branch; sorry. I hope I didn't broke anything)

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

by igorw at 2012-10-29T02:25:44Z

👍

form function takes only 2 parameters: $data and $options

This PR was merged into the master branch.

Commits
-------

26c50d4a Grammar tweak
44c11ca2 Aren't parameters set after the provider is registered ?

Discussion
----------

Aren't parameters set after the provider is registered ?

fabpot/Silex@9766faf0616a67cd0ff7437950d5453deebac9c4

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

by igorw at 2012-10-01T14:19:19Z

👍

This PR was merged into the master branch.

Commits
-------

03bccc83 Add a `SerializerServiceProvider`.

Discussion
----------

Add a SerializerServiceProvider

Usage
-----

```php
<?php

use Silex\Application;
use Silex\Provider\SerializerServiceProvider;
use Symfony\Component\HttpFoundation\Response;

$app = new Application();

$app->register(new SerializerServiceProvider);

// only accept content types supported by the serializer via the assert method.
$app->get("/pages/{id}.{_format}", function ($id) use ($app) {
    // assume a page_repository service exists that returns Page objects. The
    // object returned has getters and setters exposing the state.
    $page = $app['page_repository']->find($id);
    $format = $app['request']->getFormat();

    if (!$page instanceof Page) {
        $this->abort("No page found for id: $id");
    }

    return new Response($app['serializer']->serialize($page, $format), 200, array(
        "Content-Type" => $app['request']->getMimeType($format)
    ));
})->assert("_format", "xml|json")
  ->assert("id", "\d+");
```

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

by GromNaN at 2012-06-28T14:29:50Z

You can create this ServiceProvider in its own repository and put a link on the wiki :
https://github.com/fabpot/Silex/wiki/Third-Party-ServiceProviders

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

by fabpot at 2012-06-28T14:38:42Z

I think it makes sense to have this in core.

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

by marijn at 2012-06-28T14:41:03Z

Ok. Haven't tested in "in the wild" yet (working on that as we speak). Any pointers, concerns or other comments are more than welcome 😄

Should I add more documentation or is this enough?

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

by igorw at 2012-06-28T14:47:03Z

@fabpot I agree. 👍

Please add some more tests.

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

by marijn at 2012-06-28T14:50:13Z

@igorw in regards to the tests: what would you like to see added? I figured we should only test if the serializer is configured properly, not if it actually serializes.

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

by igorw at 2012-06-28T15:11:06Z

You're right, no need to test serialization. Looks good. 👍

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

by marijn at 2012-06-28T16:02:20Z

Somehow I have trouble running the full test suite. The swift mailer configuration doesn't seem to work. Is this a known issue?

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

by GromNaN at 2012-06-28T20:33:14Z

A new trait can also be added with the method `serialize()`.

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

by marijn at 2012-06-28T21:34:58Z

At the moment I haven't got a working version of PHP 5.4 so I cannot test traits.

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

by marijn at 2012-06-29T19:01:21Z

It seems to me that adding a `trait` for the serializer doesn't really make any sense: the API for the `Serializer` component is pretty extensive, it would add a lot of methods to the `Application`. What do you think?

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

by stof at 2012-06-30T10:32:07Z

@marijn the trait would cover the simple use of the serializer, i.e. a ``serialize()`` method. It is not intended to cover all possible use cases.

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

by fabpot at 2012-06-30T18:31:32Z

Can you squash your commits before I merge? Thanks.

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

by marijn at 2012-07-10T10:58:58Z

@fabpot this has been squashed. I think it's mergeable 😄

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

by marijn at 2012-08-17T13:39:17Z

Have I missed something here? To the best of my knowledge, this is mergeable. Please let me know if there is anything I still need to do 😄

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

by alanbem at 2012-10-27T20:39:02Z

What is status of this PR? Is it going to be merged?

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

by marijn at 2012-10-27T21:14:12Z

I have no clue, @alanbem. If you're in need of a serializer for Silex you can either merge this in your own fork of Silex or use the [`jms/serializer-service-provider`][1] that I released.

[1]: https://github.com/pink-tie/JMSSerializerServiceProvider

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

by igorw at 2012-10-27T23:15:53Z

@fabpot this looks good to me.

This PR was merged into the master branch.

Commits
-------

5e83c757 Update logout_path with pattern info per #423

Discussion
----------

Update logout_path with pattern info per #423

Because logout_path must exist inside the existing pattern, the example
in the docs would not work by default.

This PR was merged into the master branch.

Commits
-------

49cde048 Change 'security.authentication_listener.form._proto' definition to the same signature all other _protos have.

Discussion
----------

Change 'security.authentication_listener.form._proto' definition to the ...

...same signature all other _protos have.

This is a fix to remove the unussed (third) $class parameter from the 'security.authentication_listener.form._proto' definition and allow to customize the class via $options['listener_class'] instead.

Fixes https://github.com/fabpot/Silex/issues/459

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

by sli-systems at 2012-08-29T23:20:15Z

Any feedback on this? I'd love to get this sorted so if there are issues let me know and I'll address them.

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

by sli-systems at 2012-09-11T20:55:26Z

bump again - feedback either way would be appreciated.

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

by sli-systems at 2012-09-30T21:54:04Z

bump again (again) - it would be helpful to know if there is interest in this - it seems pointless to keep rebasing without any way of knowing if it is worthwhile...

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

by sli-systems at 2012-10-23T21:54:12Z

Just did another rebase - how long does it usually take for a one line PR to get accepted or rejected? Sorry, but I do not know what else to do other than bumping this thread every now and then...

If this PR is not good, please just reject it so I do not have to waste any more time on it.

Thanks, mano

This PR was merged into the master branch.

Commits
-------

ac9cdded Update doc/providers/twig.rst

Discussion
----------

Update doc/providers/twig.rst

corrected the grammatical possessiveness of the word services 

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

by igorw at 2012-10-21T18:20:54Z

👍

corrected the grammatical possessiveness of the services 

Because logout_path must exist inside the existing pattern, the example
in the docs would not work by default.

Change 'security.authentication_listener.form._proto' definition to the same signature all other _protos have.

This is a fix to remove the unussed (third) $class parameter from the 'security.authentication_listener.form._proto' definition and allow to customize the class via $options['listener_class'] instead.

This PR was merged into the master branch.

Commits
-------

f23f1589 [Doc] Added more sample to url_url_generator provider

Discussion
----------

[Doc] Added more sample to url_url_generator provider

fixed #508

fixed #508