Wrong attributes serialized when embedding multiple different inherited entities

[nik-lampe]

When serializing an related entity which hast some kind of entity with inheritance embedded (doesn't actually matter if it's single table or joined).
The properties of the parent class are serialized, and ONLY the properties of one of the child classes. But for all entites, regardless of their type.

This is a sample setup:

/**
 * @ApiResource()
 * @ORM\Entity
 * @ORM\InheritanceType(value="JOINED")
 */
class ParentClass
{
    /**
     * @var int The entity Id
     *
     * @ORM\Id
     * @ORM\GeneratedValue
     * @ORM\Column(type="integer")
     * @Groups({"get"})
     */
    protected $id;

    /**
     * @var string Something else
     *
     * @ORM\Column
     * @Assert\NotBlank
     * @Groups({"get"})
     */
    protected $baz = '';
}

/**
 * @ApiResource
 * @ORM\Entity
 */
class BarChild extends ParentClass
{
    /**
     * @var int The entity Id
     *
     * @ORM\Id
     * @ORM\GeneratedValue
     * @ORM\Column(type="integer")
     */
    protected $id;

    /**
     * @var string Something else
     *
     * @ORM\Column
     * @Assert\NotBlank
     * @Groups({"get"})
     */
    private $bar = '';
}

/**
 * @ApiResource
 * @ORM\Entity
 */
class FooChild extends ParentClass
{
    /**
     * @var int The entity Id
     *
     * @ORM\Id
     * @ORM\GeneratedValue
     * @ORM\Column(type="integer")
     */
    protected $id;

    /**
     * @var string Something else
     *
     * @ORM\Column
     * @Assert\NotBlank
     * @Groups({"get"})
     */
    private $foo = '';

/**
 * @ApiResource(attributes={"normalization_context"={"groups"={"get"}}})
 * @ORM\Entity()
 */
class RelatedEntity
{
    /**
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="AUTO")
     * @ORM\Column(type="integer")
     * @Groups({"get"})
     */
    private $id;

    /**
     * @ORM\Column(type="string")
     * @Groups({"get"})
     */
    private $attribute;

    /**
     * @ORM\OneToMany(targetEntity="AppBundle\Entity\ParentClass", mappedBy="relatedEntity")
     * @Groups({"get"})
     */
    private $relatedEntites;
}

When retrieving a RelatedEntity which has a relation to one FooChild and one BarChild the result looks like this:

{
  "@context": "\/app_dev.php\/contexts\/RelatedEntity",
  "@id": "\/app_dev.php\/related_entities\/1",
  "@type": "RelatedEntity",
  "id": 1,
  "attribute": "test",
  "relatedEntites": [
    {
      "@id": "\/app_dev.php\/foo_children\/1",
      "@type": "FooChild",
      "id": 1,
      "baz": "test2",
      "bar": null
    },
    {
      "@id": "\/app_dev.php\/bar_children\/2",
      "@type": "BarChild",
      "id": 2,
      "baz": "testbaz",
      "bar": ""
    }
  ]
}

But the FooChild Entity should not have the property bar, but foo instead. And in my testing date the value of baron the BarChild entity is actually not an empty string...

[nik-lampe]

After Debugging a little I see that the method getAllowedAttributes of api-platform/core/src/Serializer/AbstractItemNormalizer.php returns the right attributes only for the first related entity, for the second it returns the same attributes as the first one.

The CachedPropertyMetadataFactory returns in method create in line 39 the propertyMetadata from Cache instead of creating new, because it uses the parent class as part of the hash key for the cache.

Changing line 130 of AbstractItemNormalizer so that it uses get_class($classOrObject) instead of $context['resource_class']seems to fix it. But I didn't run any tests yet to see if anything else breaks. I just needed a quick fix.

[soyuka]

I think I had the same issue #810 I then managed my groups with yaml. It is more configurable IMO.

nvm #263

[dunglas]

@soyuka do you have any idea how to fix this? It's a major bug.
@meyerbaptiste can you take a look at this one and the solution suggested by @nik-lampe?

[nik-lampe]

I have found a few more places in the code which might have to be changed. In my current project it works fine now, but I found another bug. I will post more information tomorrow.

[dunglas]

Thanks! Don't hesitate to post a WIP Pull Request, it will allow us to review early.

[soyuka]

@dunglas never noticed this when I'm using parent classes I don't usually need a Resource on the parent class... Indeed this is a major bug and I have no clue why it does this, it has to do with how caching works (keep properties of first class, because instanceof Parent === true on the second, no changes are made).

[dunglas]

Yes, I usually avoid to use inheritance for entities too. Maybe should be just remove the ability to use inheritance with API Platform, IMO it's a bad practice, it makes the code complexer and leads to bugs...

[soyuka]

Sometimes it's inevitable. Though, having a Resource on the parent entity feels weird. IMHO an Item in a Collection of Resources should always have the same Type. To support this properly, it should serialize only the properties that are the same in every child entity or only the one from the Parent.

Anyway, we can maybe find an easy fix for this and still keep this "feature".

[nik-lampe]

While building my data model I didn't feel very good about it either.
But actually I can think of some cases where you need inheritance for entities.
For example https://symfony.com/legacy/doc/more-with-symfony/1_4/en/09-Doctrine-Form-Inheritance
I understand that this is legacy documentation, but the cases at the beginning seem valid to me...

Or in my project I have an entity that consists of many different components in one Collection. These componentes can be of different type and thus have different properties. It wouldn't make much sense to only serialize the shared properties and the parent properties are serialized, like @soyuka suggests.

But I am open for suggestions how I can solve this problem without inheritance. Perhaps I started wrong from the beginning on.

The work in progress PR is coming tomorrow morning CET.

[nik-lampe]

I am currently busy with a project which has the deadline tomorrow. I decided to remove the inheritance in this case just to get it over with.

After that I will create a PR.
I came across some more issues with the inheritance and have written down my observations so I hope I can also create example projects where we can see the bugs I came across.

I noticed strange behaviour with deserializing properties dependent on whether Doctrines eager loading is enabled or not.

[nik-lampe]

Here is what I mean with the last sentence:

api-platform/api-platform#272

[nik-lampe]

It seems like this is not only occuring with inheritance.
I stumbled across the same problem here #1043 and my changes in the PR fixed it for me

[bwegrzyn]

I'm running in to this issue as well, and would like to see it fixed instead of the feature being removed. Especially since Doctrine is basically a first class citizen for this project.

There are good reasons to have mixed types in a single collection. For example, I have a Document entity that has bunch of ChangeEvents attached to it. These events are all similar, but contain slightly different data depending on whether they are SubjectChangeEvent or AuthorChangeEvent, etc. It really doesn't make sense for these child entities to be completely separate.

[dunglas]

@bwegrzyn there are better solutions now to handle mixed types, namely JSON columns (with https://github.com/dunglas/doctrine-json-odm).

If someone want to work on a fix, I'll be glad to merge it. But I was against supporting abstract classes in API Platform from the very beginning, the author of Doctrine himself discourages to use this feature of the ORM and since today this feature has always been buggy. If we cannot find a proper fix, I suggest to deprecate this feature to prevent people from being trapped.

[bwegrzyn]

@dunglas Thanks for the link. I will take a look at it. I'm already using something similar in my project, but the downfall of this approach is that you cannot have relations. In my case, the status of a Document might change, so the StatusChangeEvent entity has a relation to the Status entity.

I'm definitely open to refactoring my implementation of the ChangeEvent entity I have. I'm just not sure how to do it effectively without using inheritance and having them be properly searchable.