Skip to content

[Serializer] Add an option to the XmlEncoder allowing to decode tags as collection #58604

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

Open
wants to merge 8 commits into
base: 7.3
Choose a base branch
from
+48 −0
Open

[Serializer] Add an option to the XmlEncoder allowing to decode tags as collection #58604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f93c89a
feat(serializer): add option to force collection when decoding xml
AlexandreGerault Oct 19, 2024
46e9197
fix: ci and style
AlexandreGerault Oct 21, 2024
dff07d4
ci: fix suggestions by fabbot.io
AlexandreGerault Oct 21, 2024
58fc1a9
revert: remove false positive from fabbot.io
AlexandreGerault Oct 23, 2024
517cd87
refactor(serializer): remove named parameter
AlexandreGerault Oct 23, 2024
0921aae
feat(serializer): use default context when context does not supply op…
AlexandreGerault Oct 23, 2024
6f7d48f
feat(serializer): add FORCE_COLLECTION option to XmlEncoderContextBui…
AlexandreGerault Oct 30, 2024
61ee9c8
feat(serializer): add static analysis
AlexandreGerault Oct 30, 2024
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
10 changes: 10 additions & 0 deletions src/Symfony/Component/Serializer/Context/Encoder/XmlEncoderContextBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,4 +160,14 @@ public function withCdataWrappingPattern(?string $cdataWrappingPattern): static
{
return $this->with(XmlEncoder::CDATA_WRAPPING_PATTERN, $cdataWrappingPattern);
}

/**
* Configures whether to force the output of a tag to be treated as an array.
*
* @param list<string>|null $forceCollection
*/
public function withForceCollection(?array $forceCollection): static
{
return $this->with(XmlEncoder::FORCE_COLLECTION, $forceCollection);
}
}
16 changes: 16 additions & 0 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 @@ -31,6 +31,11 @@ class XmlEncoder implements EncoderInterface, DecoderInterface, NormalizationAwa

public const AS_COLLECTION = 'as_collection';

/**
* An array of XML tags who should always be treated as a collection, even when it has only one child.
*/
public const FORCE_COLLECTION = 'force_collection';

/**
* An array of ignored XML node types while decoding, each one of the DOM Predefined XML_* constants.
*/
Expand Down Expand Up @@ -72,6 +77,7 @@ class XmlEncoder implements EncoderInterface, DecoderInterface, NormalizationAwa
self::TYPE_CAST_ATTRIBUTES => true,
self::CDATA_WRAPPING => true,
self::CDATA_WRAPPING_PATTERN => '/[<>&]/',
self::FORCE_COLLECTION => [],
];

public function __construct(array $defaultContext = [])
Expand Down Expand Up @@ -224,10 +230,20 @@ final protected function isElementNameValid(string $name): bool
*/
private function parseXml(\DOMNode $node, array $context = []): array|string
{
$nodeName = $node->nodeName;

$data = $this->parseXmlAttributes($node, $context);

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

if (\is_array($value)
&& ($childNodeName = $node->firstChild?->nodeName)
&& 1 === \count($value)
&& \in_array($nodeName, $context[self::FORCE_COLLECTION] ?? $this->defaultContext[self::FORCE_COLLECTION], true)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it could be great to accept the value * as an "every tag" value, WDYT?

Copy link
Author

Choose a reason for hiding this comment

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

Maybe, however I'm still unsure how this PR would be welcome. I'm okay to continue working on it, but I'd like to get a green light before investing too much time on it 😅

But if it get a positive feedback (which doesn't seem to be the case for the moment) I can consider adding a test case for this, maybe using xpath also 🤔

) {
return [$childNodeName => [$value[$childNodeName]]];
}
Comment on lines +239 to +245
Copy link
Contributor

Choose a reason for hiding this comment

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

This condition can go after the !\is_array($value) if.
Then, you'll be able to drop the is_array check.

Copy link
Author

Choose a reason for hiding this comment

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

Actually it can't according to the test suite 👀
Looks like it actually passes the block below:

if (!\count($data)) {
    return $value;
}


if (!\count($data)) {
return $value;
}
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Component/Serializer/Tests/Context/Encoder/XmlEncoderContextBuilderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public function testWithers(array $values)
->withDecoderIgnoredNodeTypes($values[XmlEncoder::DECODER_IGNORED_NODE_TYPES])
->withEncoderIgnoredNodeTypes($values[XmlEncoder::ENCODER_IGNORED_NODE_TYPES])
->withEncoding($values[XmlEncoder::ENCODING])
->withForceCollection($values[XmlEncoder::FORCE_COLLECTION])
->withFormatOutput($values[XmlEncoder::FORMAT_OUTPUT])
->withLoadOptions($values[XmlEncoder::LOAD_OPTIONS])
->withSaveOptions($values[XmlEncoder::SAVE_OPTIONS])
Expand All @@ -59,6 +60,7 @@ public static function withersDataProvider(): iterable
XmlEncoder::DECODER_IGNORED_NODE_TYPES => [\XML_PI_NODE, \XML_COMMENT_NODE],
XmlEncoder::ENCODER_IGNORED_NODE_TYPES => [\XML_TEXT_NODE],
XmlEncoder::ENCODING => 'UTF-8',
XmlEncoder::FORCE_COLLECTION => ['order'],
XmlEncoder::FORMAT_OUTPUT => false,
XmlEncoder::LOAD_OPTIONS => \LIBXML_COMPACT,
XmlEncoder::SAVE_OPTIONS => \LIBXML_NOERROR,
Expand All @@ -76,6 +78,7 @@ public static function withersDataProvider(): iterable
XmlEncoder::DECODER_IGNORED_NODE_TYPES => null,
XmlEncoder::ENCODER_IGNORED_NODE_TYPES => null,
XmlEncoder::ENCODING => null,
XmlEncoder::FORCE_COLLECTION => null,
XmlEncoder::FORMAT_OUTPUT => null,
XmlEncoder::LOAD_OPTIONS => null,
XmlEncoder::SAVE_OPTIONS => null,
Expand Down
19 changes: 19 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 @@ -704,6 +704,25 @@ public function testDecodeAlwaysAsCollection()
$this->assertEquals($expected, $this->encoder->decode($source, 'xml', ['as_collection' => true]));
}

public function testDecodeGivenAttributeAlwaysAsCollection()
{
$source = <<<'XML'
<order_rows>
<order_row>
<id>1</id>
<price>1200</price>
</order_row>
</order_rows>
XML;
$expected = [
'order_row' => [
['id' => 1, 'price' => 1200],
],
];

$this->assertEquals($expected, $this->encoder->decode($source, 'xml', [XmlEncoder::FORCE_COLLECTION => ['order_rows']]));
}

public function testDecodeWithoutItemHash()
{
$obj = new ScalarDummy();
Expand Down