sfPatternRouting->getRoutes() returns serialized routes

[kaime]

Since 06b3ccf2, cached routes are unserialized on demand, when calling getRoute().

This method will check with is_string() if the requested route has been already unserialized. If it has not, it will update its routes attribute with the unserialized route. If we call getRoute() a few times, then the routes array may contain both serialized and unserialized routes.

The plural getRoutes() just returns the routes array. If no call to getRoute() has been made, then that array will contain a list of strings (serialized objects). If getRoute() has been called before, then getRoutes() may return an array of mixed serialized and unserialized routes, which is quite unuseful.

We've patched it, because there are times when we need to retrieve all routes, unserialized, and we were getting that array with unserialized and serialized routes mixed.

I'm willing to send you a PR to fix this behavior, including tests, but I've seen the following cases in sfPatternRoutingTest:

$routes = $r->getRoutes();
$t->ok(is_string($routes['test1']), '->loadConfiguration() Route objects are not serialized in cache');

According to the assertion description, the routes returned by getRoutes() (loaded from cache) should not be serialized; however, it checks one particular route returned by getRoutes() is a string, which would mean it's not serialized. That's contradictory to me.

Before preparing the PR, I need to know if it's ok with you if I change that test so it tests everything returned by getRoutes() and getRoute() is unserialized.

I'd need to change that test, which could be seen as breaking change but, indeed, it would be the original Symfony 1 behavior.

[mkopinsky]

The link to your change doesn't seem to work. I believe the correct link is habitissimo@05c281b.

My understanding is that the reason for the changes about serialization of routes was for performance. It would concern me that calling getRoutes() would be unexpectedly expensive from a performance standpoint. Perhaps it would be better to add a new getUnserializedRoutes() method to make the performance cost more transparent?