Skip to content
Permalink
Browse files

feature #30286 Drop more usages of Serializable (nicolas-grekas)

This PR was merged into the 4.3-dev branch.

Discussion
----------

Drop more usages of Serializable

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

When the serialized payload of some class is used for ephemeral needs, my proposal here is to drop implementing `Serializable` and provide a deprecation layer based on `__sleep` to still call the `serialize`/`unserialize` methods. This means that at the behavior level, we provide BC, and at the payload level, we don't.

That leaves us with only two "not ephemeral" use cases:
- `Route` and `CompilerRoute` instances are serialized by Drupal. We'd better not break such existing payloads without being sure it's ok for Drupal (ping @alexpott). My proposal in this PR is to schedule the BC break for Symfony 5, and ask Drupal (and others) to add a check in their unserialization logic so that they are able to recompile the cached list of routes if unserialize fails due to this change. They could alternatively implement `Serializable` on their own, without calling `parent::(un)serialize()`.
- `TokenInterface` from `Security` - for this one, I have no idea yet, except either plan for breaking BC at the payload level (which would mean invalidating all sessions when moving to SF5) - or create a new interface if we think breaking sessions is not reasonable. For now, I've kept using `Serializable`, which could be also fine keeping if we mark the corresponding method `final` to force

WDYT?

Commits
-------

05d6475 Drop more usages of Serializable
  • Loading branch information...
fabpot committed Mar 4, 2019
2 parents ec8033f + 05d6475 commit 7951ea16e55da49501d254ba9659587bb3d8cbff
Showing with 216 additions and 114 deletions.
  1. +2 −0 UPGRADE-4.3.md
  2. +2 −0 UPGRADE-5.0.md
  3. +4 −5 src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/AppKernel.php
  4. +1 −1 src/Symfony/Bundle/FrameworkBundle/composer.json
  5. +2 −7 src/Symfony/Component/Cache/Tests/Adapter/AdapterTestCase.php
  6. +2 −7 src/Symfony/Component/Cache/Tests/Psr16CacheTest.php
  7. +2 −7 src/Symfony/Component/Cache/Tests/Simple/CacheTestCase.php
  8. +2 −2 src/Symfony/Component/Debug/Tests/phpt/fatal_with_nested_handlers.phpt
  9. +1 −0 src/Symfony/Component/Form/CHANGELOG.md
  10. +9 −2 src/Symfony/Component/Form/Extension/DataCollector/FormDataCollector.php
  11. +2 −2 src/Symfony/Component/Form/composer.json
  12. +5 −0 src/Symfony/Component/HttpKernel/CHANGELOG.md
  13. +29 −1 src/Symfony/Component/HttpKernel/DataCollector/DataCollector.php
  14. +31 −8 src/Symfony/Component/HttpKernel/DataCollector/DumpDataCollector.php
  15. +33 −0 src/Symfony/Component/HttpKernel/Kernel.php
  16. +1 −1 src/Symfony/Component/HttpKernel/KernelInterface.php
  17. +5 −5 src/Symfony/Component/HttpKernel/Tests/DataCollector/DumpDataCollectorTest.php
  18. +2 −3 src/Symfony/Component/HttpKernel/Tests/KernelTest.php
  19. +2 −0 src/Symfony/Component/Routing/CHANGELOG.md
  20. +2 −2 src/Symfony/Component/Routing/CompiledRoute.php
  21. +2 −2 src/Symfony/Component/Routing/Route.php
  22. +21 −20 src/Symfony/Component/Security/CHANGELOG.md
  23. +8 −14 src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php
  24. +33 −15 src/Symfony/Component/Security/Core/Exception/AuthenticationException.php
  25. +5 −5 src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php
  26. +5 −5 src/Symfony/Component/Security/Core/Tests/Exception/CustomUserMessageAuthenticationExceptionTest.php
  27. +3 −0 src/Symfony/Component/Security/Core/composer.json
@@ -55,6 +55,8 @@ Routing

* The `generator_base_class`, `generator_cache_class`, `matcher_base_class`, and `matcher_cache_class` router
options have been deprecated.
* Implementing `Serializable` for `Route` and `CompiledRoute` is deprecated; if you serialize them, please
ensure your unserialization logic can recover from a failure related to an updated serialization format

Security
--------
@@ -234,6 +234,8 @@ Routing

* The `generator_base_class`, `generator_cache_class`, `matcher_base_class`, and `matcher_cache_class` router
options have been removed.
* `Route` and `CompiledRoute` don't implement `Serializable` anymore; if you serialize them, please
ensure your unserialization logic can recover from a failure related to an updated serialization format

Security
--------
@@ -79,15 +79,14 @@ protected function build(ContainerBuilder $container)
$container->register('logger', NullLogger::class);
}
public function serialize()
public function __sleep()
{
return serialize([$this->varDir, $this->testCase, $this->rootConfig, $this->getEnvironment(), $this->isDebug()]);
return ['varDir', 'testCase', 'rootConfig', 'environment', 'debug'];
}
public function unserialize($str)
public function __wakeup()
{
$a = unserialize($str);
$this->__construct($a[0], $a[1], $a[2], $a[3], $a[4]);
$this->__construct($this->varDir, $this->testCase, $this->rootConfig, $this->environment, $this->debug);
}
protected function getKernelParameters()
@@ -24,7 +24,7 @@
"symfony/dependency-injection": "^4.3",
"symfony/event-dispatcher": "^4.1",
"symfony/http-foundation": "^4.3",
"symfony/http-kernel": "^4.2",
"symfony/http-kernel": "^4.3",
"symfony/polyfill-mbstring": "~1.0",
"symfony/filesystem": "~3.4|~4.0",
"symfony/finder": "~3.4|~4.0",
@@ -215,14 +215,9 @@ public function testPrune()
}
}
class NotUnserializable implements \Serializable
class NotUnserializable
{
public function serialize()
{
return serialize(123);
}
public function unserialize($ser)
public function __wakeup()
{
throw new \Exception(__CLASS__);
}
@@ -159,14 +159,9 @@ protected function isPruned($cache, $name)
}
}
class NotUnserializable implements \Serializable
class NotUnserializable
{
public function serialize()
{
return serialize(123);
}
public function unserialize($ser)
public function __wakeup()
{
throw new \Exception(__CLASS__);
}
@@ -132,14 +132,9 @@ public function testPrune()
}
}
class NotUnserializable implements \Serializable
class NotUnserializable
{
public function serialize()
{
return serialize(123);
}
public function unserialize($ser)
public function __wakeup()
{
throw new \Exception(__CLASS__);
}
@@ -24,7 +24,7 @@ var_dump([
$eHandler[0]->setExceptionHandler('print_r');
if (true) {
class Broken implements \Serializable
class Broken implements \JsonSerializable
{
}
}
@@ -37,6 +37,6 @@ array(1) {
}
object(Symfony\Component\Debug\Exception\FatalErrorException)#%d (%d) {
["message":protected]=>
string(199) "Error: Class Symfony\Component\Debug\Broken contains 2 abstract methods and must therefore be declared abstract or implement the remaining methods (Serializable::serialize, Serializable::unserialize)"
string(179) "Error: Class Symfony\Component\Debug\Broken contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (JsonSerializable::jsonSerialize)"
%a
}
@@ -15,6 +15,7 @@ CHANGELOG
* added `block_prefix` option to `BaseType`.
* added `help_html` option to display the `help` text as HTML.
* `FormError` doesn't implement `Serializable` anymore
* `FormDataCollector` has been marked as `final`
* added `label_translation_parameters`, `attr_translation_parameters`, `help_translation_parameters` options
to `FormType` to pass translation parameters to form labels, attributes (`placeholder` and `title`) and help text respectively.
The passed parameters will replace placeholders in translation messages.
@@ -27,6 +27,8 @@
*
* @author Robert Schönthal <robert.schoenthal@gmail.com>
* @author Bernhard Schussek <bschussek@gmail.com>
*
* @final since Symfony 4.3
*/
class FormDataCollector extends DataCollector implements FormDataCollectorInterface
{
@@ -229,15 +231,20 @@ public function getData()
return $this->data;
}
public function serialize()
/**
* @internal
*/
public function __sleep()
{
foreach ($this->data['forms_by_hash'] as &$form) {
if (isset($form['type_class']) && !$form['type_class'] instanceof ClassStub) {
$form['type_class'] = new ClassStub($form['type_class']);
}
}
return serialize($this->cloneVar($this->data));
$this->data = $this->cloneVar($this->data);
return parent::__sleep();
}
/**
@@ -31,7 +31,7 @@
"symfony/config": "~3.4|~4.0",
"symfony/console": "~3.4|~4.0",
"symfony/http-foundation": "~3.4|~4.0",
"symfony/http-kernel": "~3.4|~4.0",
"symfony/http-kernel": "~4.3",
"symfony/security-csrf": "~3.4|~4.0",
"symfony/translation": "~4.2",
"symfony/var-dumper": "~3.4|~4.0"
@@ -41,7 +41,7 @@
"symfony/dependency-injection": "<3.4",
"symfony/doctrine-bridge": "<3.4",
"symfony/framework-bundle": "<3.4",
"symfony/http-kernel": "<3.4",
"symfony/http-kernel": "<4.3",
"symfony/translation": "<4.2",
"symfony/twig-bridge": "<3.4.5|<4.0.5,>=4.0"
},
@@ -4,9 +4,14 @@ CHANGELOG
4.3.0
-----

* `KernelInterface` doesn't extend `Serializable` anymore
* deprecated the `Kernel::serialize()` and `unserialize()` methods
* increased the priority of `Symfony\Component\HttpKernel\EventListener\AddRequestFormatsListener`
* made `Symfony\Component\HttpKernel\EventListenerLocaleListener` set the default locale early
* made `FileLinkFormatter` final and not implement `Serializable` anymore
* the base `DataCollector` doesn't implement `Serializable` anymore, you should
store all the serialized state in the data property instead
* `DumpDataCollector` has been marked as `final`

4.2.0
-----
@@ -26,7 +26,7 @@
* @author Fabien Potencier <fabien@symfony.com>
* @author Bernhard Schussek <bschussek@symfony.com>
*/
abstract class DataCollector implements DataCollectorInterface, \Serializable
abstract class DataCollector implements DataCollectorInterface
{
protected $data = [];
@@ -35,16 +35,26 @@ abstract class DataCollector implements DataCollectorInterface, \Serializable
*/
private $cloner;
/**
* @deprecated since Symfony 4.3, store all the serialized state in the data property instead
*/
public function serialize()
{
@trigger_error(sprintf('The "%s" method is deprecated since Symfony 4.3, store all the serialized state in the data property instead.', __METHOD__), E_USER_DEPRECATED);
$trace = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT, 2);
$isCalledFromOverridingMethod = isset($trace[1]['function'], $trace[1]['object']) && 'serialize' === $trace[1]['function'] && $this === $trace[1]['object'];
return $isCalledFromOverridingMethod ? $this->data : serialize($this->data);
}
/**
* @deprecated since Symfony 4.3, store all the serialized state in the data property instead
*/
public function unserialize($data)
{
@trigger_error(sprintf('The "%s" method is deprecated since Symfony 4.3, store all the serialized state in the data property instead.', __METHOD__), E_USER_DEPRECATED);
$this->data = \is_array($data) ? $data : unserialize($data);
}
@@ -100,4 +110,22 @@ protected function getCasters()
return $casters;
}
public function __sleep()
{
if (__CLASS__ !== $c = (new \ReflectionMethod($this, 'serialize'))->getDeclaringClass()->name) {
@trigger_error(sprintf('Implementing the "%s::serialize()" method is deprecated since Symfony 4.3, store all the serialized state in the "data" property instead.', $c), E_USER_DEPRECATED);
$this->data = $this->serialize();
}
return ['data'];
}
public function __wakeup()
{
if (__CLASS__ !== $c = (new \ReflectionMethod($this, 'unserialize'))->getDeclaringClass()->name) {
@trigger_error(sprintf('Implementing the "%s::unserialize()" method is deprecated since Symfony 4.3, store all the serialized state in the "data" property instead.', $c), E_USER_DEPRECATED);
$this->unserialize($this->data);
}
}
}
@@ -25,6 +25,8 @@
/**
* @author Nicolas Grekas <p@tchwork.com>
*
* @final since Symfony 4.3
*/
class DumpDataCollector extends DataCollector implements DataDumperInterface
{
@@ -85,6 +87,9 @@ public function dump(Data $data)
$this->isCollected = false;
}
if (!$this->dataCount) {
$this->data = [];
}
$this->data[] = compact('data', 'name', 'file', 'line', 'fileExcerpt');
++$this->dataCount;
@@ -95,6 +100,10 @@ public function dump(Data $data)
public function collect(Request $request, Response $response, \Exception $exception = null)
{
if (!$this->dataCount) {
$this->data = [];
}
// Sub-requests and programmatic calls stay in the collected profile.
if ($this->dumper || ($this->requestStack && $this->requestStack->getMasterRequest() !== $request) || $request->isXmlHttpRequest() || $request->headers->has('Origin')) {
return;
@@ -136,28 +145,38 @@ public function reset()
$this->clonesIndex = 0;
}
public function serialize()
/**
* @internal
*/
public function __sleep()
{
if (!$this->dataCount) {
$this->data = [];
}
if ($this->clonesCount !== $this->clonesIndex) {
return 'a:0:{}';
return [];
}
$this->data[] = $this->fileLinkFormat;
$this->data[] = $this->charset;
$ser = serialize($this->data);
$this->data = [];
$this->dataCount = 0;
$this->isCollected = true;
return $ser;
return parent::__sleep();
}
public function unserialize($data)
/**
* @internal
*/
public function __wakeup()
{
$this->data = unserialize($data);
parent::__wakeup();
$charset = array_pop($this->data);
$fileLinkFormat = array_pop($this->data);
$this->dataCount = \count($this->data);
self::__construct($this->stopwatch, $fileLinkFormat, $charset);
}
@@ -178,6 +197,10 @@ public function getDumps($format, $maxDepthLimit = -1, $maxItemsPerDepth = -1)
}
$dumps = [];
if (!$this->dataCount) {
return $this->data = [];
}
foreach ($this->data as $dump) {
$dumper->dump($dump['data']->withMaxDepth($maxDepthLimit)->withMaxItemsPerDepth($maxItemsPerDepth));
$dump['data'] = stream_get_contents($data, -1, 0);
@@ -196,7 +219,7 @@ public function getName()
public function __destruct()
{
if (0 === $this->clonesCount-- && !$this->isCollected && $this->data) {
if (0 === $this->clonesCount-- && !$this->isCollected && $this->dataCount) {
$this->clonesCount = 0;
$this->isCollected = true;
@@ -833,15 +833,48 @@ public static function stripComments($source)
return $output;
}
/**
* @deprecated since Symfony 4.3
*/
public function serialize()
{
@trigger_error(sprintf('The "%s" method is deprecated since Symfony 4.3.', __METHOD__), E_USER_DEPRECATED);
return serialize([$this->environment, $this->debug]);
}
/**
* @deprecated since Symfony 4.3
*/
public function unserialize($data)
{
@trigger_error(sprintf('The "%s" method is deprecated since Symfony 4.3.', __METHOD__), E_USER_DEPRECATED);
list($environment, $debug) = unserialize($data, ['allowed_classes' => false]);
$this->__construct($environment, $debug);
}
public function __sleep()
{
if (__CLASS__ !== $c = (new \ReflectionMethod($this, 'serialize'))->getDeclaringClass()->name) {
@trigger_error(sprintf('Implementing the "%s::serialize()" method is deprecated since Symfony 4.3.', $c), E_USER_DEPRECATED);
$this->serialized = $this->serialize();
return ['serialized'];
}
return ['environment', 'debug'];
}
public function __wakeup()
{
if (__CLASS__ !== $c = (new \ReflectionMethod($this, 'serialize'))->getDeclaringClass()->name) {
@trigger_error(sprintf('Implementing the "%s::serialize()" method is deprecated since Symfony 4.3.', $c), E_USER_DEPRECATED);
$this->unserialize($this->serialized);
unset($this->serialized);
return;
}
$this->__construct($this->environment, $this->debug);
}
}
Oops, something went wrong.

0 comments on commit 7951ea1

Please sign in to comment.
You can’t perform that action at this time.