[Serializer] XML deserialization with MetadataAwareNameConverter and optional xml attributes fails if attributes are not present

[jlhernandez]

Symfony version(s) affected: ^4.3

Description
I am using Symfony Serializer to deserialize complex XML documents into custom objects. Some XML nodes have optional attributes that may be present or not.

For example, the "price" node can have a "currency" attribute or not:
<price currency="EUR">66</price>
or
<price>55</price>
are both valid.

I am using MetadataAwareNameConverter to map our class attributes to xml attributes.
In the first case, the Price object is correctly created and both attributes are filled in, but in the second case the Price object is created by both attributes are left NULL.

How to reproduce
Our Price class is this one:

use Symfony\Component\Serializer\Annotation\SerializedName;

class Price {

    /**
     * @SerializedName("@currency")
     * @var string
     */
    private $currency;

    /**
     * @SerializedName("#")
     * @var string
     */
    private $amount;


    public function getCurrency() {
        return $this->currency;
    }
    public function setCurrency(string $currency) {
        $this->currency = $currency;
    }

    public function getAmount() {
        return $this->amount;
    }
    public function setAmount(string $amount) {
        $this->amount = $amount;
    }
}

This test case shows the correct behaviour when "currency" attribute is present:

    function testWorks() {

        $classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
        $metadataAwareNameConverter = new MetadataAwareNameConverter($classMetadataFactory);

        $encoders = [new XmlEncoder()];
        $normalizers = [new ObjectNormalizer($classMetadataFactory, $metadataAwareNameConverter)];  
        $serializer = new Serializer($normalizers, $encoders);
    
        $xml = '<price currency="EUR">55</price>';

        $object = $serializer->deserialize($xml, Price::class, 'xml');

        $this->assertEquals('EUR', $object->getCurrency());
        $this->assertEquals(55, $object->getAmount());
    }

but this test case fails (Amount is also NULL!):

    function testFails() {

        $classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
        $metadataAwareNameConverter = new MetadataAwareNameConverter($classMetadataFactory);

        $encoders = [new XmlEncoder()];
        $normalizers = [new ObjectNormalizer($classMetadataFactory, $metadataAwareNameConverter)];  
        $serializer = new Serializer($normalizers, $encoders);
    
        $xml = '<price>55</price>';

        $object = $serializer->deserialize($xml, Price::class, 'xml');

        $this->assertNull($object->getCurrency());
        $this->assertEquals(55, $object->getAmount());
    }

Possible Solution

I have found that when denormalizing the attributes, the name converter receives an array with key '0' as property name. I have implemented a CustomMetadataAwareNameConverter that replaces the property name '0' by '#', and it works now..

This is the CustomMetadataAwareNameConverter class:

class CustomMetadataAwareNameConverter implements AdvancedNameConverterInterface {

    /**
     * The real MetadataAwareNameConverter
     * 
     * @var MetadataAwareNameConverter
     */
    private $converter;

    public function __construct(ClassMetadataFactoryInterface $metadataFactory, NameConverterInterface $fallbackNameConverter = null) {
        $this->converter = new MetadataAwareNameConverter($metadataFactory, $fallbackNameConverter);
    }

    public function normalize($propertyName, string $class = null, string $format = null, array $context = []) {
        return $this->converter->normalize($propertyName, $class, $format, $context);
    }

    public function denormalize($propertyName, string $class = null, string $format = null, array $context = []) {
        return $this->converter->denormalize($propertyName == '0' ? '#' : $propertyName, $class, $format, $context);
    }
}

and the test that previously failed works now:

    function testFixed() {

        $classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
        $metadataAwareNameConverter = new CustomMetadataAwareNameConverter($classMetadataFactory);

        $encoders = [new XmlEncoder()];
        $normalizers = [new ObjectNormalizer($classMetadataFactory, $metadataAwareNameConverter)];  
        $serializer = new Serializer($normalizers, $encoders);
    
        $xml = '<price>55</price>';

        $object = $serializer->deserialize($xml, Price::class, 'xml');

        $this->assertNull($object->getCurrency());
        $this->assertEquals(55, $object->getAmount());
    }

[mkrauser]

I can confirm this exact issue

[monteiro]

I have create an issue to cover both cases without using the SerializedName annotation. The solution is just a name converter that is passed to the ObjectNormalizer. The name converter converts extra attributes (that are prefixed with @) and node value ( using #) returned from the XmlEncoder.

Can this be useful for you?

[tacman]

I think this is related, but perhaps not. I'm trying to deserialize XML into a Property entity with a name and value, e.g.

<property name="length">4110</property>

serialization.yaml is configured so that the attribute and value are generated properly.

App\Entity\Property:
  attributes:
    name:
      serialized_name: '@name'
      groups: ['xml']
    value:
      serialized_name: '#'
      groups: ['xml']

And works as expected.

        $property = (new Property())->setName('length')->setValue(4110);
        $context = [
            'xml_root_node_name' => 'property',
            'groups' => ['xml']
        ];

        $xml = $serializer->serialize($property, 'xml', $context);   
       //         $xml is now '<property name="length">4110</property>' as expected

        $xml = '<property name="length">4110</property>';
    
       // but serializing to Property doesn't work.
        $data = $serializer->deserialize($xml, Property::class, 'xml');
        // neither name or value is set.
        dd($data);

Does the serialized_name property (or @SerializedName annotation) work in the deserialization process? Or should I be calling this differently?

[nicolas-grekas]

Help wanted.

[carsonbot]

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

[tacman]

My workaround was to use a regex to rewrite the attributes as node elements, then rewrite my parsing accordingly. Of course, my solution is a classic definition of "hack" -- functional, but certainly not the "right" way.

[monteiro]

Did something related here: #35087

[carsonbot]

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

[carsonbot]

Could I get a reply or should I close this?

[tacman]

@nicolas-grekas Do you know if any tests have been added for this?

[nicolas-grekas]

I don't remember sorry. Please have a look if you can.

[tacman]

I can confirm that I still have the issue.

I've create a repo that shows example, trivial (2 entities, serialization.yaml, and a Command).

git clone git@github.com:tacman/xml-serializer-demo.git
cd xml-serializer-demo && composer install
bin/console app:bug

The serializations works, but not the deserialization:

$product = (new Product())
    ->setDescription("Wool sweater");
$product
    ->addProperty((new Property())->setName('color')->setValue('blue'))
    ->addProperty((new Property())->setName('size')->setValue('small'))
    ;

$context = [
    'xml_root_node_name' => 'product',
    'groups' => ['xml'],
    'xml_standalone' => false,
    'xml_format_output' => true,
];
$xml = $this->serializer->serialize($product, 'xml', $context);

// works!

        /** @var Product $product2 */
        $product2 = $this->serializer->deserialize($xml, Product::class, 'xml');

        dump($product2); // nothing is set :-(

[carsonbot]

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

[carsonbot]

Could I get a reply or should I close this?

[tacman]

I'll try again.

[tacman]

I bumped the example repo to Symfony 6.1 and it still fails.

git clone git@github.com:tacman/xml-serializer-demo.git
cd xml-serializer-demo && composer install
bin/console app:bug

[carsonbot]

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

[carsonbot]

Could I get a reply or should I close this?

[tacman]

I guess not many people use XML deserialization. It continues to be an issue.

[carsonbot]

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

[carsonbot]

Could I get a reply or should I close this?

[tacman]

I think this PR will fix it: b70032c

but shouldn't it be on all Symfony releases, not just 5.4?

[xabbuh]

@tacman We regularly merge the 5.4 up into all other maintained branches. So if the linked PR is merged into the 5.4 branch, the patch will eventually land in all maintained Symfony versions.