Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated suggestion for JsonSerializationVisitor::addData replacement #1066

Merged
merged 1 commit into from
Mar 23, 2019
Merged

Updated suggestion for JsonSerializationVisitor::addData replacement #1066

merged 1 commit into from
Mar 23, 2019

Conversation

theoboldt
Copy link
Contributor

When using :visitProperty(new StaticPropertyMetadata('', 'name', 'value), null) the
\JMS\Serializer\GraphNavigator\SerializationGraphNavigator::accept() method is called with $data = null, therefore $type will be set to null. Therefore, value configured in StaticPropertyMetadata will not be put in place (as long as it's not null).

In order to make \JMS\Serializer\GraphNavigator\SerializationGraphNavigator::accept() extract the correct type (via gettype()), the value needs to be set into visitProperty().

In conclusion, you should use :visitProperty(new StaticPropertyMetadata('', 'name', null), 'value')

Q A
Bug fix? no
New feature? no
Doc updated yes
BC breaks? no
Deprecations? no
Fixed tickets no
License MIT

When using `:visitProperty(new StaticPropertyMetadata('', 'name', 'value), null)` the 
 `\JMS\Serializer\GraphNavigator\SerializationGraphNavigator::accept()` method is called with `$data = null`, therefore `$type` will be set to `null`. Therefore, value configured in `StaticPropertyMetadata` will not be put in place (as long as it's not `null`). 

In order to make  `\JMS\Serializer\GraphNavigator\SerializationGraphNavigator::accept()` extract the correct type (via `gettype()`), the value needs to be set into `visitProperty()`. 

In conclusion, you should use `:visitProperty(new StaticPropertyMetadata('', 'name', null), 'value')`
@goetas
Copy link
Collaborator

goetas commented Mar 22, 2019

@kunicmarko20 As I remember, you made a PR about this months ago.. any comment?

@kunicmarko20
Copy link
Contributor

kunicmarko20 commented Mar 22, 2019

Yes, and this is exactly the change I am using, forgot to make a PR. @theoboldt there is a code comment explaining this also, can you find that one and update it too.

@kunicmarko20
Copy link
Contributor

* @deprecated Use visitProperty(new StaticPropertyMetadata(null, 'name', 'value'), null) instead

@goetas goetas merged commit 5af83ae into schmittjoh:master Mar 23, 2019
@goetas
Copy link
Collaborator

goetas commented Mar 23, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants