added optional expression parameter to the ReadOnly annotation #1178
Conversation
|
Hi. Thanks for your contribution. |
| /** | ||
| * {@inheritDoc} | ||
| */ | ||
| public function shouldSkipPropertyOnDeserialize(PropertyMetadata $property, Context $navigatorContext): bool |
There was a problem hiding this comment.
I believe that this method can be moved into shouldSkipProperty and use the $navigatorContext to understand if we are in "deserialization". In this way we do not need a new public method in this class.
There was a problem hiding this comment.
good idea, I've changed that
optimzed shouldSkipProperty and removed shouldSkipPropertyOnDeserialize
|
Hey, I've now also added this functionality to the XML and YML drivers. |
There was a problem hiding this comment.
Hi! Great work so far! Can you also add tests please? (and update the XML/YML reference in https://github.com/schmittjoh/serializer/tree/master/doc/reference ?)
src/Metadata/Driver/YamlDriver.php
Outdated
| } | ||
| } | ||
|
|
||
| /* |
There was a problem hiding this comment.
can you avoid commenting code please?
There was a problem hiding this comment.
oops sorry, forgot to remove that block
|
See https://github.com/schmittjoh/serializer/pull/1178/files#r399651332 that suggests a bit different approach. |
|
to fix automatically CS issues, you can run |
…or read-only parameter
| @@ -68,6 +69,7 @@ protected function loadMetadataFromFile(\ReflectionClass $class, string $path): | |||
| $exclude = $elem->attributes()->exclude; | |||
| $excludeAll = null !== $exclude ? 'true' === strtolower((string) $exclude) : false; | |||
| $classAccessType = (string) ($elem->attributes()->{'access-type'} ?: PropertyMetadata::ACCESS_TYPE_PROPERTY); | |||
| $readOnlyClass = $elem->attributes()->{'read-only'}; | |||
There was a problem hiding this comment.
for some weird reason I really cannot find out, I cannot do parseExpression() here, because if re-use the variable $readOnlyClass in the property-loop it is only working for the first call. On the second call then, I'm getting error on unserializing the cache. But if I do parseExpression() inside the property-loop it works fine.
Makes really no sense to me.
There was a problem hiding this comment.
you should cast to string the xml node
There was a problem hiding this comment.
If you look into my previous commit, I'm doing that there.
I've noticed that it was actually only working for the caching-call. The second call fails with error:
Return value of JMS\Serializer\Metadata\PropertyMetadata::unserializeProperties() must be of the type string, null returned [] []
It's really weird because if I do the exact same thing inside of the property-loop, it is working.
So putting the block on line 90-96 (previous commit) into the loop starting at line 174, it works..
It's just confusing me and I wanted to find out how this happens.
But it does not really matter because doing parseExpression on every porperty will maybe only affect the performance on when creating cache
src/Metadata/Driver/XmlDriver.php
Outdated
| $pMetadata->readOnlyIf = null; | ||
| $readOnly = null === $pElem->attributes()->{'read-only'} ? $readOnlyClass : $pElem->attributes()->{'read-only'}; | ||
| if(null !== $readOnly) { | ||
| if ($readOnly === 'true' || $readOnly === 'false') { |
There was a problem hiding this comment.
Thought of also parsing the true/false values as expression. But I guess it's not good for performance.
There was a problem hiding this comment.
here performance are not a concern as this is done only once, then data are cached
There was a problem hiding this comment.
as I saw it is only caching the expression-string and not the result, which makes sense because the expression should be evaluated on deserialize to be dynamically.
And then it would always evalute expressions with "true" and "false" on deserialize.
Not sure if this will produce impact at all though.
|
Removed read-only-if tag in yml and xml. read-only is now also parsing expressions. |
|
hey, |
|
when this will be merged? |
Hello,
for my project I needed some way to dynamically define if a property is ReadOnly or not.
So I implemented the ExpressionParser the same way as the @exclude annotation does.
Similar features has also been asked here some years ago. It's not possible with groups though.
schmittjoh/JMSSerializerBundle#628