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

Fix stdClass inconsistencies when serializing to JSON #730

Merged
merged 4 commits into from
Apr 21, 2017

Conversation

goetas
Copy link
Collaborator

@goetas goetas commented Mar 13, 2017

Fixes #61
Closes #59
Closes #242

@goetas goetas added this to the v1.7 milestone Mar 13, 2017
@@ -263,7 +265,85 @@ public function testSerializeWithNonUtf8EncodingWhenDisplayErrorsOn()

public function testSerializeArrayWithEmptyObject()
{
$this->assertEquals('{"0":{}}', $this->serialize(array(new \stdClass())));
$this->assertEquals('[{}]', $this->serialize(array(new \stdClass())));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this to me sounds a wrong test case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@schmittjoh What do you think?

@goetas goetas changed the title Fix stdClass inconsistencies Fix stdClass inconsistencies when serializing to JSON Apr 21, 2017
@goetas goetas merged commit f547e35 into master Apr 21, 2017
@goetas goetas deleted the std-class-and-cleanup branch April 21, 2017 14:01
@robwasripped
Copy link

With the class GenericSerializationVisitor now deprecated, what is the recommended way to ensure that the visitor given in the serializer.post_serialize event contains the setData method?

@goetas
Copy link
Collaborator Author

goetas commented Aug 10, 2017

I really do not suggest to use setData. when possible is better IMO to use vistiProperty(new StaticMedadataProperty(....)).

But to answer to your question, using directly JsonSerializationVisitor should work well.
(BTW with the previous implementation, setData was fully relying on the internal json_encode logic)

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

Successfully merging this pull request may close these issues.

2 participants