[Cache] Add MarshallerInterface allowing to change the serializer, providing a default one that automatically uses igbinary when available
#27645
[Cache] Add
MarshallerInterface allowing to change the serializer, …
…providing a default one that automatically uses igbinary when available
- Loading branch information
commit ae4319a971049f67a0aa5680d7e8242372e65b10
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| <?php | ||
|
|
||
| /* | ||
| * This file is part of the Symfony package. | ||
| * | ||
| * (c) Fabien Potencier <fabien@symfony.com> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| namespace Symfony\Component\Cache\Marshaller; | ||
|
|
||
| use Symfony\Component\Cache\Exception\CacheException; | ||
|
|
||
| /** | ||
| * Serializes/unserializes values using igbinary_serialize() if available, serialize() otherwise. | ||
| * | ||
| * @author Nicolas Grekas <p@tchwork.com> | ||
| */ | ||
| class DefaultMarshaller implements MarshallerInterface | ||
| { | ||
| private $useIgbinarySerialize = true; | ||
|
|
||
| public function __construct(bool $useIgbinarySerialize = null) | ||
| { | ||
| if (null === $useIgbinarySerialize) { | ||
| $useIgbinarySerialize = \extension_loaded('igbinary'); | ||
| } elseif ($useIgbinarySerialize && !\extension_loaded('igbinary')) { | ||
| throw new CacheException('The "igbinary" PHP extension is not loaded.'); | ||
| } | ||
| $this->useIgbinarySerialize = $useIgbinarySerialize; | ||
| } | ||
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function marshall(array $values, ?array &$failed): array | ||
| { | ||
| $serialized = $failed = array(); | ||
|
|
||
| foreach ($values as $id => $value) { | ||
| try { | ||
| if ($this->useIgbinarySerialize) { | ||
| $serialized[$id] = igbinary_serialize($value); | ||
| } else { | ||
| $serialized[$id] = serialize($value); | ||
| } | ||
| } catch (\Exception $e) { | ||
| $failed[] = $id; | ||
| } | ||
| } | ||
|
|
||
| return $serialized; | ||
| } | ||
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function unmarshall(string $value) | ||
| { | ||
| if ('b:0;' === $value) { | ||
| return false; | ||
| } | ||
| if ('N;' === $value) { | ||
| return null; | ||
| } | ||
| static $igbinaryNull; | ||
| if ($value === ($igbinaryNull ?? $igbinaryNull = \extension_loaded('igbinary') ? igbinary_serialize(null) : false)) { | ||
| return null; | ||
| } | ||
| $unserializeCallbackHandler = ini_set('unserialize_callback_func', __CLASS__.'::handleUnserializeCallback'); | ||
| try { | ||
| if (':' === ($value[1] ?? ':')) { | ||
| if (false !== $value = unserialize($value)) { | ||
| return $value; | ||
| } | ||
| } elseif (false === $igbinaryNull) { | ||
| throw new \RuntimeException('Failed to unserialize values, did you forget to install the "igbinary" extension?'); | ||
| } elseif (null !== $value = igbinary_unserialize($value)) { | ||
| return $value; | ||
| } | ||
|
|
||
| throw new \DomainException(error_get_last() ? error_get_last()['message'] : 'Failed to unserialize values.'); | ||
| } catch (\Error $e) { | ||
| throw new \ErrorException($e->getMessage(), $e->getCode(), E_ERROR, $e->getFile(), $e->getLine()); | ||
| } finally { | ||
| ini_set('unserialize_callback_func', $unserializeCallbackHandler); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @internal | ||
| */ | ||
| public static function handleUnserializeCallback($class) | ||
| { | ||
| throw new \DomainException('Class not found: '.$class); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| <?php | ||
|
|
||
| /* | ||
| * This file is part of the Symfony package. | ||
| * | ||
| * (c) Fabien Potencier <fabien@symfony.com> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| namespace Symfony\Component\Cache\Marshaller; | ||
|
|
||
| /** | ||
| * Serializes/unserializes PHP values. | ||
| * | ||
| * Implementations of this interface MUST deal with errors carefully. They MUST | ||
| * also deal with forward and backward compatibility at the storage format level. | ||
| * | ||
| * @author Nicolas Grekas <p@tchwork.com> | ||
| */ | ||
| interface MarshallerInterface | ||
| { | ||
| /** | ||
| * Serializes a list of values. | ||
| * | ||
| * When serialization fails for a specific value, no exception should be | ||
| * thrown. Instead, its key should be listed in $failed. | ||
| */ | ||
| public function marshall(array $values, ?array &$failed): array; | ||
|
|
||
| /** | ||
| * Unserializes a single value and throws an exception if anything goes wrong. | ||
| * | ||
| * @return mixed | ||
| * | ||
| * @throws \Exception Whenever unserialization fails | ||
| */ | ||
| public function unmarshall(string $value); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this has been asked, but why do we have a
DefaultMarshallerwith some internal logic in all methods to differentiate between PHP and IgBinary ... instead of defining a PhpMarshaller and IgBinaryMarshaller (moreover given that we define a MarshallerInterface).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because there is only one implementation of MarshallerInterface: DefaultMarshaller, which auto-adaptatively uses serialize() or igbinary_serialize().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that looks to me like creating a
DefaultMailerwhich makesif($use_smtp) { ... } elseif ($use_gmail) { ...}inside its methods. In other parts of the framework we define 1 interface and multiple different implementation classes. Here it looks like we have multiple implementations inside a single class. It looks confusing to me, sorry.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@javiereguiluz it's not the same. Here, by dealing with both serialization methods (and there are none others for PHP objects), we provide an important feature: auto-adaptative unserialization, providing safe forward and backward compatibility at the data level. This is not possible without having something that knows about both formats.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IgBinaryMarshaller could take PhpMarshaller as a fallback marshaller (which is optional), no?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of the MarshallerInterface then if there are no other reasonable implementations?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks to the interface, the serialized string itself can still be processed, e.g compressed and/or encrypted.
One could also build a serializer that would be dedicated to and optimized for very specific data structures (eg only numbers, etc.), etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... But actually it's not true that igbinary is the only serialization format that might benefit from a fallback to PHP serialization. There are / were other contenders like msgpack (MessagePack) and Protobuf, both of which have PHP extensions.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, another reason to have the interface. Let's keep the topic: should we split the default marshaller in two. I think we would do a misfeature by doing so: things would be more complex to understand, more complex to wire, less capable, and less performant while in a critical code path. Would the individual envisioned marshallers empower our users in comparison to these downsides? Not at all in my opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're right, and those other serializer formats are not PHP-specific, so it wouldn't make sense for them to fallback to PHP serialization, actually. 😆