Skip to content

Xml encoder optional type cast #23122

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Xml encoder optional type cast #23122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Symfony/Component/Serializer/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ CHANGELOG
* [DEPRECATION] the `Exception` interface has been renamed to `ExceptionInterface`
* added `ObjectNormalizer` leveraging the `PropertyAccess` component to normalize
objects containing both properties and getters / setters / issers / hassers methods.
* added `xml_type_cast_attributes` context option for allowing users to opt-out of typecasting
xml attributes.

2.6.0
-----
Expand Down
33 changes: 24 additions & 9 deletions src/Symfony/Component/Serializer/Encoder/XmlEncoder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,10 @@ public function decode($data, $format, array $context = array())
unset($data['@xmlns:xml']);

if (empty($data)) {
return $this->parseXml($rootNode);
return $this->parseXml($rootNode, $context);
}

return array_merge($data, (array) $this->parseXml($rootNode));
return array_merge($data, (array) $this->parseXml($rootNode, $context));
}

if (!$rootNode->hasAttributes()) {
Expand Down Expand Up @@ -256,11 +256,11 @@ final protected function isElementNameValid($name)
*
* @return array|string
*/
private function parseXml(\DOMNode $node)
private function parseXml(\DOMNode $node, array $context = array())
{
$data = $this->parseXmlAttributes($node);
$data = $this->parseXmlAttributes($node, $context);

$value = $this->parseXmlValue($node);
$value = $this->parseXmlValue($node, $context);

if (!count($data)) {
return $value;
Expand Down Expand Up @@ -292,16 +292,17 @@ private function parseXml(\DOMNode $node)
*
* @return array
*/
private function parseXmlAttributes(\DOMNode $node)
private function parseXmlAttributes(\DOMNode $node, array $context = array())
{
if (!$node->hasAttributes()) {
return array();
}

$data = array();
$typeCastAttributes = $this->resolveXmlTypeCastAttributes($context);

foreach ($node->attributes as $attr) {
if (!is_numeric($attr->nodeValue)) {
if (!is_numeric($attr->nodeValue) || !$typeCastAttributes) {
Copy link
Member

@nicolas-grekas nicolas-grekas Jun 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I propose excluding numbers that start with zero:
if (!is_numeric($attr->nodeValue) || (isset($attr->nodeValue[1]) && '0' === $attr->nodeValue[0])) {

Copy link
Member

Choose a reason for hiding this comment

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

I agree for 2.7. Having this flag in 3.4 would be nice too.

Copy link
Contributor Author

@ragboyjr ragboyjr Jun 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@nicolas-grekas That sounds good. That would fix my specific use case, not sure if it fixes all of the issues others might have.

I might just submit another MR then with the bug fix like @nicolas-grekas shows into 2.7. And rebase this off of 3.4 @dunglas

Copy link
Member

Choose a reason for hiding this comment

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

Good idea, we can even do better:

  • submit the fix proposed by Nicolas into 2.7
  • rebase this PR against master and make this feature optin (document the BC break in the CHANGELOG)

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dunglas @nicolas-grekas Sounds great. You guys are the boss! just let me know :).

Copy link
Member

Choose a reason for hiding this comment

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

@ragboyjr I'm going to merge this one in 3.4 as a new feature. Can you create another PR on 2.7 for the bug fix? Thanks.

$data['@'.$attr->nodeName] = $attr->nodeValue;

continue;
Expand All @@ -326,7 +327,7 @@ private function parseXmlAttributes(\DOMNode $node)
*
* @return array|string
*/
private function parseXmlValue(\DOMNode $node)
private function parseXmlValue(\DOMNode $node, array $context = array())
{
if (!$node->hasChildNodes()) {
return $node->nodeValue;
Expand All @@ -343,7 +344,7 @@ private function parseXmlValue(\DOMNode $node)
continue;
}

$val = $this->parseXml($subnode);
$val = $this->parseXml($subnode, $context);

if ('item' === $subnode->nodeName && isset($val['@key'])) {
if (isset($val['#'])) {
Expand Down Expand Up @@ -522,6 +523,20 @@ private function resolveXmlRootName(array $context = array())
: $this->rootNodeName;
}

/**
* Get XML option for type casting attributes Defaults to true.
*
* @param array $context
*
* @return bool
*/
private function resolveXmlTypeCastAttributes(array $context = array())
{
return isset($context['xml_type_cast_attributes'])
? (bool) $context['xml_type_cast_attributes']
: true;
}

/**
* Create a DOM document, taking serializer options into account.
*
Expand Down
22 changes: 22 additions & 0 deletions src/Symfony/Component/Serializer/Tests/Encoder/XmlEncoderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,28 @@ public function testDecodeNegativeFloatAttribute()
$this->assertSame(array('@index' => -12.11, '#' => 'Name'), $this->encoder->decode($source, 'xml'));
}

public function testNoTypeCastAttribute()
{
$source = <<<XML
<?xml version="1.0"?>
<document a="018" b="-12.11">
<node a="018" b="-12.11"/>
</document>
XML;

$data = $this->encoder->decode($source, 'xml', array('xml_type_cast_attributes' => false));
$expected = array(
'@a' => '018',
'@b' => '-12.11',
'node' => array(
'@a' => '018',
'@b' => '-12.11',
'#' => '',
),
);
$this->assertSame($expected, $data);
}

public function testEncode()
{
$source = $this->getXmlSource();
Expand Down