Deserialize xmlKeyValuePairs #840
Conversation
fdyckhoff
commented
Nov 22, 2017
•
fdyckhoff
commented
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | yes |
| Doc updated | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #820 schmittjoh/JMSSerializerBundle#521 schmittjoh/JMSSerializerBundle#470 |
| License | Apache-2.0 |
| // handle key-value-pairs | ||
| if (null !== $metadata && $metadata->xmlKeyValuePairs) { | ||
| if (2 !== count($type['params'])) { | ||
| throw new RuntimeException(sprintf('The array type must be specified as "array<K,V>" for Key-Value-Pairs.')); |
There was a problem hiding this comment.
sprintf here is not neccessary
| { | ||
| $xml = $this->getContent('array_key_values_with_same_value_types'); | ||
| $result = $this->serializer->deserialize($xml, ObjectWithXmlKeyValuePairsWithSameValueTypes::class, 'xml'); | ||
| $this->assertEquals(new ObjectWithXmlKeyValuePairsWithSameValueTypes(), $result); |
There was a problem hiding this comment.
can you put in the xml different values than the defaults for the property $list and assert that those values are set?
the thing is that this test will be true even if the deserializer simply ignores the $list property
| @@ -151,6 +151,28 @@ public function visitDouble($data, array $type, Context $context) | |||
|
|
|||
| public function visitArray($data, array $type, Context $context) | |||
| { | |||
| $metadata = $this->currentMetadata; | |||
| if (null === $metadata && $context->getMetadataStack()->count()) { | |||
There was a problem hiding this comment.
instead of this "dirty" check, what about adding here a setCurrentMetadata call as here. Should be faster and "cleaner"
relates to schmittjoh#820 deserialize arrays with key-value pairs add test case with other values from default set current metadata from stack in serialization context
fdyckhoff
force-pushed
the
deserialize-xml-key-value-pairs
branch
from
6201e33
to
2852fe1
Compare
|
@goetas Something is still not perfect... In the project, where I integrated this feature, the exception that the array<K,V> type needs to be set is now thrown for other attributes (which do not have the key-value pair annotation set). I added a testcase for this bug but I'm not sure how to fix it. Should I add the |
|
I added the revert calls... Works again - but the solution is not pretty... Do you have any improvement ideas? |
|
I was more thinking of something as https://github.com/dyvelop/serializer/compare/deserialize-xml-key-value-pairs...goetas:dyvelop-deserialize-xml-key-value-pairs-example?expand=1 (my example should ne not working but notice the |
|
Like this? 805286b |
Not really. There should be no "visitor-specific" code in the graph navigator. Did you check my comment (and the code there)?
|
|
Happy that this will address my issue tickets. Thanks. |
