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

VirtualProperty doesn't show up without name option set #810

Open
gnat42 opened this issue Jul 27, 2020 · 7 comments
Open

VirtualProperty doesn't show up without name option set #810

gnat42 opened this issue Jul 27, 2020 · 7 comments

Comments

@gnat42
Copy link

gnat42 commented Jul 27, 2020

Hello,

Thanks for the library, and if I could actually figure out what is wrong would submit a PR. Here's the problem.

I have a project that has been this project for a couple years - the error happens with version ^2.0 and ^3.0. I refactored one object which necessitated moving some annotations from the class properties to class methods and using the VirtualProperty annotation.

When I did, they stopped being output when serialized. I have many other objects using the VritualProperty annotation without the name option, those continue to function. I wrote a simple object and test to see if I could reproduce it, the test worked behaved the same way as the 'broken' class.

<?php

namespace NS\CoreDomain\Tests\RealEstate\Document\Pdf;

use JMS\Serializer\Annotation as JMS;

class SimpleObject
{
    /**
     * @var int
     * @JMS\SerializedName("Identity")
     */
    private $id = 1;

    /**
     * @var string
     * @JMS\SerializedName("Object Name")
     */
    private $name = 'theName';

    /** @var string|null */
    private $second;

    public function __construct(?string $second)
    {
        $this->second = $second;
    }

    public function getId(): int
    {
        return $this->id;
    }

    public function getName(): string
    {
        return $this->name;
    }

    /**
     * @JMS\VirtualProperty(name="2nd")
     * @JMS\SerializedName("2nd")
     */
    public function getSecond(): ?string
    {
        return $this->second;
    }
}

When I serialize this to an array, if I don't put name="2nd" in the VirtualProperty I get an array with keys

'second' => '', 
'Object Name' => 'theName',
'Identity' = 1

What is bizarre is I have lots of objects in this property with the following annotations that work perfectly.

/**
 * @JMS\SerializedName("Field Name")
 * @JMS\Groups({"GroupName"})
 * @JMS\VirtualProperty()
 */

So I'm really curious as to why.

In debugging a little I noticed that ClassMetadata propertyMetadata property's array keys are the name in VirtualProperty. IE in my example above, there's a 'second' entry in the propertyMetadata as a VirtualPropertyMetadata (which has a serializedName field).

In my actual project however, not specifying the name in VirtualProperty for that one object results in no corresponding field output.

Thoughts?

@goetas
Copy link
Collaborator

goetas commented Jul 31, 2020

VirtualProperty without the name param tries to infer some name automatically, that might conflict with another property that could be already present in the class. ( i can see a private prop with the same name).

the name and serializedName are two different things (one is used to match metadata to properties, the other is used to decide the output json key).

Hope it helps

@gnat42
Copy link
Author

gnat42 commented Jul 31, 2020

Yeah, the issue is I have some objects with annotations acting one way and other objects acting a different way so if I can ask. Serializing to json what should the output of the following object be?

class SimpleObject
{
    /**
     * @var int
     * @JMS\SerializedName("Identity")
     */
    private $id = 1;

    /**
     * @var string
     * @JMS\SerializedName("Object Name")
     */
    private $name = 'theName';

    /** @var string|null */
    private $second;

    public function __construct(?string $second)
    {
        $this->second = $second;
    }

    public function getId(): int
    {
        return $this->id;
    }

    public function getName(): string
    {
        return $this->name;
    }

    /**
     * @JMS\VirtualProperty()
     * @JMS\SerializedName("2nd")
     */
    public function getSecond(): ?string
    {
        return $this->second;
    }
}

vs

class SimpleObject
{
    /**
     * @var int
     * @JMS\SerializedName("Identity")
     */
    private $id = 1;

    /**
     * @var string
     * @JMS\SerializedName("Object Name")
     */
    private $name = 'theName';

    /** @var string|null */
    private $second;

    public function __construct(?string $second)
    {
        $this->second = $second;
    }

    public function getId(): int
    {
        return $this->id;
    }

    public function getName(): string
    {
        return $this->name;
    }

    /**
     * @JMS\VirtualProperty(name="2nd")
     * @JMS\SerializedName("2nd")
     */
    public function getSecond(): ?string
    {
        return $this->second;
    }
}

I'm creating a repository to illustrate what I was experiencing..

@gnat42
Copy link
Author

gnat42 commented Jul 31, 2020

Ok for example, checkout

https://github.com/gnat42/serializerTest .

composer install
./vendor/bin/phpunit

One will pass and one will fail. However since the objects both have SerializedName, I would expect the output to have '2nd' as an array key in both tests.
In my mind, because both objects

@goetas
Copy link
Collaborator

goetas commented Aug 1, 2020

https://github.com/gnat42/serializerTest/blob/c3710dae5ced2af00987d8403f221b53d935f6a1/src/SimpleObject.php#L43 will probably be just ignored since later is overridden by another property with the same name.

@goetas
Copy link
Collaborator

goetas commented Aug 1, 2020

If you look in the output of https://github.com/gnat42/serializerTest/blob/c3710dae5ced2af00987d8403f221b53d935f6a1/tests/SimpleObjectTest.php#L30 you should see also a prop named "second"

@stayeronglass
Copy link

stayeronglass commented Aug 10, 2020

by default getter names and property names are the same, but property name came after getter's name and ovverride it in

https://github.com/schmittjoh/metadata/blob/6eb35fce7142234946d58d13e1aa829e9b78b095/src/ClassHierarchyMetadata.php#L21

we need separate property name from getter name in LoadedMetadata

@gnat42
Copy link
Author

gnat42 commented Apr 15, 2022

Ok, so I've learned more and I don't know if this is a bug however it sure feels like one to me. In src/GraphNavigator/SerializationGraphNavigator.php line 219 there is this call

$metadata = $this->metadataFactory->getMetadataForClass($type['name'])

When you have a object property and a getter that 'resolve' to the same 'property' there's an issue.

In the sample repository there was a property second and a getSecond(). Which both result in a $metadata->propertyMetaData['second'] except it takes the first property and ignores the method, and none of the annotations on the getSecond method show up.

Now that I re-read your comments @goetas from August of last year, I can see that you understood the issue but I didn't. I also didn't understand @stayeronglass 's response. However this is precisely the problem.

I consider this a bug but wonder if you consider it a bug? At a minimum I would think this should be well documented and I don't mind providing a PR about it. If there's some guidance about how to fix the issue I also don't mind trying to fix it as well.

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

3 participants