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

Allow using @XmlValue together with @Accessor/@AccessType #1083

Closed
localheinz opened this issue May 8, 2019 · 7 comments
Closed

Allow using @XmlValue together with @Accessor/@AccessType #1083

localheinz opened this issue May 8, 2019 · 7 comments

Comments

@localheinz
Copy link
Contributor

localheinz commented May 8, 2019

Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? no

Steps required to reproduce the problem

  1. Use jms/serializer:1.5.0.

  2. Have XML which cannot be controlled

<foo bar="40" baz="20">
                    
    https://placekitten.com/200/300
</foo>
  1. Have class
<?php

use JMS\Serializer\Annotation as JMS;

class Foo
{
    /**
     * @JMS\Accessor(
     *     getter="getUrl",
     *     setter="setUrl"
     * )
     * @JMS\Type("string")
     * @JMS\XmlValue
     *
     * @var string|null
     */
    private $url;

    public function setUrl(string $url): void
    {
        $this->url = trim($url);
    }

    public function getUrl(): ?string
    {
        return $this->url;
    }
}
  1. Deserialize XML into object
$foo = $serializer->deserialize(
    $xml,
    Foo::class,
    'xml'
);

var_dump($foo->getUrl());

Expected Result

string(31) "https://placekitten.com/200/300"

Actual Result

string(58) "

    https://placekitten.com/200/300
"

💡 It's of course possible to solve this using something like

diff --git a/Foo.php b/Foo.php
index f4afea402..7329e3bf2 100644
--- a/backend/Foo.php
+++ b/backend/Foo.php
@@ -25,4 +25,16 @@ class Foo
     {
         return $this->url;
     }
+
+    /**
+     * @JMS\PostDeserialize
+     */
+    public function trimUrl(): void
+    {
+        if (!\is_string($this->url)) {
+            return;
+        }
+
+        $this->url = \trim($this->url);
+    }
 }

but I think it would be nice not having to add another method for this.

What do you think?

@goetas
Copy link
Collaborator

goetas commented May 8, 2019

Well, in XML all those spaces have a meaning, so trimming them would be a HUGE bc break, and also not compliant to XML specs

@goetas
Copy link
Collaborator

goetas commented May 8, 2019

adding an option @XmlValue(trim=true) could be an acceptable solution

@localheinz
Copy link
Contributor Author

@goetas

I'm not asking to trim the values, I'm wondering whether it would be possible to use the @JMS\XmlValue annotation together with @JMS\Accessor or @JMS\AccessType - then modifications can be made in userland code.

@goetas
Copy link
Collaborator

goetas commented May 8, 2019

So you are saying that @JMS\XmlValue can not be used with @JMS\Accessor? If is to, to me sounds more like a bug than a feature request.... Can you submit a failing test case?

@localheinz localheinz changed the title Allow using @XmlAttribute together with @Accessor/@AccessType Allow using @XmlValue together with @Accessor/@AccessType May 10, 2019
@localheinz
Copy link
Contributor Author

Looks like this is already covered by tests:

  • public function testAccessorSetterDeserialization()
    {
    /** @var AccessorSetter $object */
    $object = $this->deserialize(
    '<?xml version="1.0"?>
    <AccessorSetter>
    <element attribute="attribute">element</element>
    <collection>
    <entry>collectionEntry</entry>
    </collection>
    </AccessorSetter>',
    'JMS\Serializer\Tests\Fixtures\AccessorSetter'
    );
    self::assertInstanceOf('stdClass', $object->getElement());
    self::assertInstanceOf('JMS\Serializer\Tests\Fixtures\AccessorSetterElement', $object->getElement()->element);
    self::assertEquals('attribute-different', $object->getElement()->element->getAttribute());
    self::assertEquals('element-different', $object->getElement()->element->getElement());
    self::assertEquals(['collectionEntry' => 'collectionEntry'], $object->getCollection());
    }
  • /**
    * @Serializer\Type("string")
    * @Serializer\Accessor(setter="setElementDifferent")
    * @Serializer\XmlValue
    *
    * @var string
    */
    protected $element;

I'll take another look, closing for now!

@localheinz
Copy link
Contributor Author

Works like a charm, not sure what I was doing wrong!

@goetas
Copy link
Collaborator

goetas commented May 10, 2019

👍

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

No branches or pull requests

2 participants