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

[Serializer] Xml encoder throws exception for valid data #21671

Closed
wants to merge 3 commits into from

Conversation

gr1ev0us
Copy link
Contributor

@gr1ev0us gr1ev0us commented Feb 19, 2017

Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #21617
License MIT
Doc PR None

#21617 Xml encoder throws exception for valid data

  • add tests for bool and object encoding
  • fix encoding for object in array and field

* add tests for bool and object encoding

* fix encoding for object in array and field
private $encoder;

/**
Copy link
Member

Choose a reason for hiding this comment

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

This comment is useless (the IDE will be able to infer the type using the default value).

@@ -524,4 +534,96 @@ protected function getObject()

return $obj;
}

/**
* @test
Copy link
Member

Choose a reason for hiding this comment

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

This block is useless (same below).

*/
public function testEncodeXmlWithBoolValue()
{
$expectedXml = '<?xml version="1.0"?>
Copy link
Member

Choose a reason for hiding this comment

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

You may use the heredoc syntax (the IDE will be able to highlight the snippet). (Just suggestion, not mandatory).

<response><foo>1</foo><bar>0</bar></response>
';

$actualXml = $this->encoder->encode(array('foo' => true, 'bar' => false), 'xml');
Copy link
Member

Choose a reason for hiding this comment

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

I don't get why this is expected? IMO it should be array('foo' => '1', 'bar' => '0') unless explicitly defined as bool in a XML schema.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an actual behavior of XmlEncode, to cast bool to int. I am not sure that i should fix this non explicit behaviour, so i am just cover that with test.

@@ -11,19 +11,29 @@

namespace Symfony\Component\Serializer\Tests\Encoder;

use DateTime;
Copy link
Member

Choose a reason for hiding this comment

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

For classes in the global namespace, we don't import them (use \DateTime directly).

@nicolas-grekas nicolas-grekas changed the title Issue-21617 Xml encoder throws exception for valid data [Serializer] Xml encoder throws exception for valid data Feb 25, 2017
@@ -12,18 +12,24 @@
namespace Symfony\Component\Serializer\Tests\Encoder;

use PHPUnit\Framework\TestCase;
use Symfony\Component\Serializer\Tests\Fixtures\Dummy;
Copy link
Member

Choose a reason for hiding this comment

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

please revert any change related to ordering: our policy is to add new lines in alpha order, but keep old ones as is - to help for merges between branches.

@nicolas-grekas nicolas-grekas added this to the 2.7 milestone Feb 25, 2017
@fabpot
Copy link
Member

fabpot commented Mar 1, 2017

@dunglas Is this one ok for you?

@dunglas
Copy link
Member

dunglas commented Mar 5, 2017

👍

@fabpot
Copy link
Member

fabpot commented Mar 5, 2017

Thank you @gr1ev0us.

fabpot added a commit that referenced this pull request Mar 5, 2017
…gr1ev0us)

This PR was squashed before being merged into the 2.7 branch (closes #21671).

Discussion
----------

[Serializer] Xml encoder throws exception for valid data

| Q             | A
| ------------- | ---
| Branch?       |  2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #21617
| License       | MIT
| Doc PR        | None

#21617  Xml encoder throws exception for valid data

- add tests for bool and object encoding
- fix encoding for object in array and field

Commits
-------

5c2d4c6 [Serializer] Xml encoder throws exception for valid data
@fabpot fabpot closed this Mar 5, 2017
This was referenced Mar 6, 2017
sandergo90 pushed a commit to sandergo90/symfony that referenced this pull request Jul 4, 2019
… data (gr1ev0us)

This PR was squashed before being merged into the 2.7 branch (closes symfony#21671).

Discussion
----------

[Serializer] Xml encoder throws exception for valid data

| Q             | A
| ------------- | ---
| Branch?       |  2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony#21617
| License       | MIT
| Doc PR        | None

symfony#21617  Xml encoder throws exception for valid data

- add tests for bool and object encoding
- fix encoding for object in array and field

Commits
-------

5c2d4c6 [Serializer] Xml encoder throws exception for valid data
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants