merged branch Crell/route-serialize (PR #4755)
Commits
-------
a6d44bd
When serializing a Route, don't serialize the compiled route.
Discussion
----------
When serializing a Route, don't serialize the compiled route.
The compiled route typically has a reference to the Route. The Route of course has a reference to the compiled route. This is a circular reference. That can sometimes cause issues, say for var_export(). I ran into this issue while serializing route objects, but it could happen in other cases, too.
In any event, since the compiled route object is simply derived data I see no reason to keep it around. If we serialize a Route, we just want the basic route information. We can recompile later if needs be. As a nice side-effect, this makes the serialized string much smaller.
---------------------------------------------------------------------------
by stof at 2012-07-04T21:09:34Z
I would implement the Serializable interface instead. It would be more consistent with what Symfony does elsewhere, and would avoid breaking things when you extend the class (Silex does it). Private properties cannot be accessed from the child class, and this is what PHP tries to do when you use ``__sleep`` as you only return the property name.
---------------------------------------------------------------------------
by Tobion at 2012-07-04T22:52:00Z
I also saw this circular reference. Maybe it's time to remove the reference from compiled route to the original route?!
If a developer needs the original route, he should keep the reference himself. No need to do it for him. It's like offering a "decompiling" reference that shouldnt be there.
---------------------------------------------------------------------------
by Crell at 2012-07-05T00:08:51Z
With Serializable, I'd have to reimplement serialize() myself, wouldn't I? I just want to exclude the one value, but otherwise serialize normally. Seems like that would be a non-small amount of work compared to a one-liner.
---------------------------------------------------------------------------
by stof at 2012-07-05T00:09:44Z
@Crell you have to reimplement it in the child class only if you want to add more stuff in the serialized data.
and breaking Silex just to be lazy here is not a good idea
---------------------------------------------------------------------------
by Crell at 2012-07-05T00:24:52Z
I'm not trying to break Silex. :-P
I think this is what you mean, but I'm not sure that completely changing the serialized form (which this does) is wise. Is that not also a BC break of sorts?
Once we settle on one we can cherry-pick out the commits we want.
---------------------------------------------------------------------------
by stof at 2012-07-05T01:00:29Z
changing the serialization format is not an issue: the only place where serialized routes are likely to be stored is in the cache, and the cache does not need to be BC between 2.0 and 2.1. The user already have to delete it due to other changes.
---------------------------------------------------------------------------
by fabpot at 2012-07-09T15:02:59Z
+1 for implementing `Serializable`. Can you squash your commits and fix the CS (we need 4 spaces)? Thanks.
---------------------------------------------------------------------------
by Crell at 2012-07-10T03:25:04Z
Squshed, Spaced, and Rebased.
This commit is contained in:
commit
c2a6e37ad7
@ -18,7 +18,7 @@ namespace Symfony\Component\Routing;
|
||||
*
|
||||
* @api
|
||||
*/
|
||||
class Route
|
||||
class Route implements \Serializable
|
||||
{
|
||||
private $pattern;
|
||||
private $defaults;
|
||||
@ -55,6 +55,25 @@ class Route
|
||||
$this->compiled = null;
|
||||
}
|
||||
|
||||
public function serialize()
|
||||
{
|
||||
return serialize(array(
|
||||
'pattern' => $this->pattern,
|
||||
'default' => $this->default,
|
||||
'requirements' => $this->requirements,
|
||||
'options' => $this->options,
|
||||
));
|
||||
}
|
||||
|
||||
public function unserialize($data)
|
||||
{
|
||||
$data = unserialize($data);
|
||||
$this->pattern = $data['pattern'];
|
||||
$this->default = $data['default'];
|
||||
$this->requirements = $data['requirements'];
|
||||
$this->options = $data['options'];
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the pattern.
|
||||
*
|
||||
|
Reference in New Issue
Block a user