Handling PropertyTypeExtractorInterface with multiple types

[JumpIfBelow]

Hi,

To first start with: thanks for all the work you are doing. It is able to achieve many things, even if it is sometimes a bit hard to understand.

I'm using API Platform 2.1 (I'm locked to Symfony 3.4 here) and I have an issue with the property_info service always using Doctrine to performs the type inference.
The issue there is that I have Doctrine fields that does not allows null while the property itself allows it. It then uses validation to says if null is allowed or not.

It could works great but that's where the property_info is working there to say something like The type of the "configuration" attribute must be "string", "NULL" given..
From now on, I was trying to use PhpDocs first to infer the type and then fallback to Doctrine type extractor.
I adds this in the services.yaml:

services:
#...
    php_doc_extractor:
        decorates: 'property_info.php_doc_extractor'
        class: 'Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor'
        tags:
            - { name: 'property_info.description_extractor', priority: -1000 }
            - { name: 'property_info.type_extractor', priority: -998 }

as Doctrine has a priority of -999 for type extractor, now the PhpDocs is triggered first. And it is!

But that's where the bug takes place: I have many types which are declared like this:

<?php

namespace Baz;

use Doctrine\Common\Collections\Collection;

class Foo {
  /**
   * @var Collection|Bar[] $bars
   */
  private $bars;
}

(I'm droping API Platform and Doctrine configuration here because it has nothing to do with the actual issue)

When the IRI generator will try to understands which type I'm trying to show, it will first hits the Collection type. The result is a cryptic bug like this: Could not denormalize object of type Doctrine\Common\Collections\Collection, no supporting normalizer found.
Sometimes, I have another one even more cryptic which says Uncaught PHP Exception Symfony\Component\Config\Exception\FileLoaderLoadException: "Resource "Doctrine\Common\Collections\Collection" not found in . (which is being imported from "/srv/app/config/routes/api_platform.yaml"). Make sure there is a loader supporting the "api_platform" type." at /srv/app/vendor/symfony/config/Loader/FileLoader.php line 168.

The bugs looks like when it hits an unhandled type, it simply crashes instead of testing other types.
The PropertyTypeExtractorInterface::getTypes() returns null if nothing is found, or an array. We should iterate through this array until a handled type is found. If none, then an error should be thrown.

I think this issue happens because no one ever tried to change this behavior and Doctrine always returns null or an array with one element inside. Handling many values could be better.

The workaround for me is to make the class above looking like this:

<?php

namespace Baz;

use Doctrine\Common\Collections\Collection;

class Foo {
  /**
   * @var Bar[]|Collection $bars
   */
  private $bars;
}

Then it will first hits Bar[], which is handled. Then everything works.

It really is an edge case and I understand it. If someone is able to fix it, it is cool. If not, this issue may be useful for anyone coming to the same case.

[dunglas]

Hi, thanks for the kind works and the detailed report.

[Growiel]

I have the same issue.

The PhpDocExtractor is the only extractor that retrns multiple types (as per the Symfony documentation) and it seems that when the PropertyInfoPropertyMetadataFactory class from Api-platform was created, it omitted that fact, you can see the following code line 52:

if (null === $propertyMetadata->getType()) {
            $types = $this->propertyInfo->getTypes($resourceClass, $name, $options);
            if (isset($types[0])) {
                $propertyMetadata = $propertyMetadata->withType($types[0]);
            }
 }

The class only checks for one type and uses it instead of being able to handle an array of types.

But that's only part of the issue. The real issue lies in the fact that the entire PropertyMetadata class and eco-system has been designed to only handle ONE type (see the getType(): ?Type method, the withType(Type $type) method and so on.

That leads to another issue, when the AbstractItemNormalizer class tries to validate the type of the object (method validateType, it only checks for that one type and thus will fail to handle a PHPDoc that would like @var int|string $answer.

The course of action should be:

• Change PropertyMetadata and the associated classes (such as PropertyInfoPropertyMetadataFactory) to handle an array of Types.
• Change AbstractItemNormalizer's validateType method so it goes through all the types and only fails if none of the types are correct.
• BC breaks ?

If you think that's the correct way to approach the issue and fix, I'd be willing to work on it and propose a first PR that you could then finalize. This issue is quite blocking for us.

[teohhanhui]

I disagree that we should support multiple types.

As for the OP's issue:

  /**
   * @var Collection|Bar[] $bars
   */
  private $bars;

While technically incorrect, it's indeed a very common way of writing PHPDoc. As a matter of fact, phpstan supports this and treats it as Collection<Bar>: phpstan/phpstan#1790 (comment)

I think this should be patched in Symfony PropertyInfo, to add special handling for this syntax.

[celinederoland]

Hello, the problem is still present nowadays,

I need to define a resource with a property that can be oneOf(integer,boolean). So I define it this way :

class MyResource {
  public int|bool $myAttribute = 0;
}

During normalization, the instances are denormalized correctly, with a boolean or integer value of $myAttribute.
But during denormalization, when I receive a PUT or POST request, I can't handle a boolean value (The type of the "myAttribute" attribute must be "int", "boolean" given.), so I debugged it and it is exactly because of the same piece of code :

 if (null === $propertyMetadata->getType()) {
            $types = $this->propertyInfo->getTypes($resourceClass, $name, $options);
            if (isset($types[0])) {
                $propertyMetadata = $propertyMetadata->withType($types[0]);
            }
 }

You can see in the screenshot bellow that the 2 possible types of my attributes are stored in the $types variable, and we only keep $types[0], so a few instructions later we have the error : The type of the "myAttribute" attribute must be "int", "boolean" given.

@teohhanhui I don't understand your comment : generally speaking, multiple types are part of the openapi standard, so in my opinion it should be supported by api platform. The case of Doctrine annotations like @var Collection|Bar[] is a particular case, but multiple types exist in many other situations and is supported.
https://swagger.io/docs/specification/data-models/data-types/ ==> Mixed types

[soyuka]

has been implemented in 3.2

[ThyMakra]

has been implemented in 3.2

@soyuka May I get the reference to where this has been implemented?

[soyuka]

at #5470