Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[VarExporter] a new component to serialize values to plain PHP code#28231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
335dcc6 to2bc8313Comparetolry commentedAug 20, 2018
since the naming is similar/identical it might be a good idea to distinguish this from the php core functionhttp://php.net/manual/de/function.var-export.php ? |
6d05c08 to8668202Comparenicolas-grekas commentedAug 20, 2018
@tolry we have the |
77d9f7a toe07f116Comparetolry commentedAug 20, 2018
@nicolas-grekas I guessed so, just thought it might be easier to grasp, if the explanation mentioned var_export explicitely. Ah, I see it's in there now - did you add it or did I miss it before? 😁 |
6114d8f to81606f9Comparenicolas-grekas commentedAug 20, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@tolry yes, I updated the description. PR is ready IMHO. |
sroze left a comment
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.
Except these two comments, it looks good to me 👌
Note that it's very likely it will require more maintenance than when it was internal.
| useSymfony\Component\VarExporter\Internal\Values; | ||
| /** | ||
| * @author Nicolas Grekas <p@tchwork.com> |
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.
We usually put that at the end of the comment.
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.
moved down
| "name":"symfony/var-exporter", | ||
| "type":"library", | ||
| "description":"A blend of var_export() + serialize() to turn any serializable data structure to plain PHP code", | ||
| "keywords": ["export","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.
I'd addsymfony andvar-exporter as well, they are the component 💃
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.
this is already in the name of the component, no need to duplicate
| 4.2.0 | ||
| ----- | ||
| * added the component |
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.
- * added the component+ * Created the component (extracted `PhpMarshaller` from the `Cache` component)
Could be valuable to give a hint of where it comes from.
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.
PhpMarshaller has never been released, better not give it any existence except in internal history
I'd keep as is :)
nicolas-grekas commentedAug 24, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
That's why I added more tests for more cases (and fixes) ;) Comments addressed, thanks for the review. |
| ===================== | ||
| The VarExporter component allows exporting any serializable PHP data structure to | ||
| plain PHP code. While doing so, it preserves all the semantics associated to the |
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.
associated with
| The VarExporter component allows exporting any serializable PHP data structure to | ||
| plain PHP code. While doing so, it preserves all the semantics associated to the | ||
| serialization mechanism of PHP (`__wakeup`,`__sleep`,`Serializable` esp.) |
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.
"esp" can be removed IMHO
| Unlike`var_export()`, this works on any serializable PHP value. | ||
| It also provides a few improvements over`var_export()`/`serialize()`: | ||
| * the output is PSR-2 compatible |
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.
missing empty line before the list, and missing; at the end of each line but the last.
| instances are preserved | ||
| *`Reflection*`,`IteratorIterator` and`RecursiveIteratorIterator` classes | ||
| throw an exception when being serialized (their unserialized version is broken | ||
| anyway, seehttps://bugs.php.net/76737.) |
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.
dot outside the parenthesis
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.
extra space after "see"
| *PhpMarshaller allows serializing PHP data structuresusingvar_export() | ||
| * while preserving all the semantics associated to serialize(). | ||
| *VarExporter allows serializing PHP data structuresto plain PHP code (likevar_export()) | ||
| * while preserving all the semantics associated to serialize() (unlike var_export()). |
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.
associated with
| * | ||
| * @return string The value exported as PHP code | ||
| * | ||
| * @throw \Exception When the provided value cannot be serialized |
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.
nicolas-grekas commentedAug 27, 2018
Comments addressed thanks. |
fabpot commentedAug 27, 2018
Thank you@nicolas-grekas. |
…lain PHP code (nicolas-grekas)This PR was merged into the 4.2-dev branch.Discussion----------[VarExporter] a new component to serialize values to plain PHP code| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -This PR proposes moving what is currently the `PhpMarshaller` class in the Cache component to a separate component.This component would provide only one public static method:`VarExporter::export($value, bool &$isStaticValue = null): string`.This method returns `$value` serialized as plain PHP code. Running this code creates the same exact data structure that `$value` contained. This is exactly like `serialize()` and `unserialize()`, from which all semantics are preserved (`__sleep`, `__wakeup` and `Serializable`).The reason to use this method *vs* `serialize()` or even igbinary is performance: thanks to OPcache, the resulting code is significantly faster and more memory efficient than using `unserialize()` or `igbinary_unserialize()`.Unlike `var_export()`, this works on any serializable PHP value.It also provides a few improvements over `var_export()`/`serialize()`:- the output is PSR-2 compatible- the output can be re-indented without messing up with any `\r` or `\n` in the data- missing classes throw a `ReflectionException` instead of being unserialized to a `PHP_Incomplete_Class` object- references involving `SplObjectStorage`, `ArrayObject` or `ArrayIterator` instances are preserved- `Reflection*`, `IteratorIterator` and `RecursiveIteratorIterator` classes throw an exception when being serialized (their unserialized version is broken anyway, seehttps://bugs.php.net/76737.)Commits-------7831ad7 [VarExporter] a new component to serialize values to plain PHP code
Uh oh!
There was an error while loading.Please reload this page.
This PR proposes moving what is currently the
PhpMarshallerclass in the Cache component to a separate component.This component would provide only one public static method:
VarExporter::export($value, bool &$isStaticValue = null): string.This method returns
$valueserialized as plain PHP code. Running this code creates the same exact data structure that$valuecontained. This is exactly likeserialize()andunserialize(), from which all semantics are preserved (__sleep,__wakeupandSerializable).The reason to use this methodvs
serialize()or even igbinary is performance: thanks to OPcache, the resulting code is significantly faster and more memory efficient than usingunserialize()origbinary_unserialize().Unlike
var_export(), this works on any serializable PHP value.It also provides a few improvements over
var_export()/serialize():\ror\nin the dataReflectionExceptioninstead of being unserialized to aPHP_Incomplete_ClassobjectSplObjectStorage,ArrayObjectorArrayIteratorinstances are preservedReflection*,IteratorIteratorandRecursiveIteratorIteratorclasses throw an exception when being serialized (their unserialized version is broken anyway, seehttps://bugs.php.net/76737.)