api-platform / core

The server component of API Platform: hypermedia and GraphQL APIs in minutes
https://api-platform.com
MIT License
2.43k stars 866 forks source link

Unexpected non-iterable value for to-many relation #3278

Closed pesseyjulien closed 4 years ago

pesseyjulien commented 4 years ago

Hi,

Updated to the latest version (2.5.2), and now I'm getting the following error :

Symfony\Component\Serializer\Exception\UnexpectedValueException: Unexpected non-iterable value for to-many relation.
#9 /vendor/api-platform/core/src/Serializer/AbstractItemNormalizer.php(526): ApiPlatform\Core\Serializer\AbstractItemNormalizer::getAttributeValue
#8 /vendor/symfony/symfony/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php(181): Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::normalize
#7 /vendor/api-platform/core/src/Serializer/AbstractItemNormalizer.php(151): ApiPlatform\Core\Serializer\AbstractItemNormalizer::normalize
#6 /vendor/symfony/symfony/src/Symfony/Component/Serializer/Serializer.php(152): Symfony\Component\Serializer\Serializer::normalize
#5 /vendor/symfony/symfony/src/Symfony/Component/Serializer/Serializer.php(125): Symfony\Component\Serializer\Serializer::serialize
#4 /src/AppBundle/Controller/JobController.php(90): AppBundle\Controller\JobController::createJobAction
#3 /vendor/symfony/symfony/src/Symfony/Component/HttpKernel/HttpKernel.php(151): Symfony\Component\HttpKernel\HttpKernel::handleRaw
#2 /vendor/symfony/symfony/src/Symfony/Component/HttpKernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel::handle
#1 /vendor/symfony/symfony/src/Symfony/Component/HttpKernel/Kernel.php(198): Symfony\Component\HttpKernel\Kernel::handle
#0 /public/index.php(24): null

Any idea why ?

teohhanhui commented 4 years ago

Could you share what this value is? Might be a regression introduced in #3248

pesseyjulien commented 4 years ago

No idea, here is the issue : https://sentry.io/share/issue/0062c90e3b27452ab8fb207489ee6d04/

test Data sent :

--data "{\"category\":\"v2_pastryCook\",\"end_date\":\"2019-11-28T18:00:00.000Z\",\"skills\":[{\"code\":\"cocktail-bar\",\"level\":0},{\"code\":\"bar\",\"level\":0},{\"code\":\"store\",\"level\":0},{\"code\":\"brasserie\",\"level\":0},{\"code\":\"candy\",\"level\":0},{\"code\":\"barista\",\"level\":0},{\"code\":\"vegan\",\"level\":0},{\"code\":\"english\",\"level\":0},{\"code\":\"mixologie\",\"level\":0},{\"code\":\"bread-viennoiserie\",\"level\":0},{\"code\":\"chocolat\",\"level\":0},{\"code\":\"sugar\",\"level\":0},{\"code\":\"traditional\",\"level\":0},{\"code\":\"caterer\",\"level\":0},{\"code\":\"shaved\",\"level\":0},{\"code\":\"trimmed\",\"level\":0},{\"code\":\"security-shoe\",\"level\":0},{\"code\":\"knife\",\"level\":0},{\"code\":\"light-clothes\",\"level\":0},{\"code\":\"dark-clothes\",\"level\":0},{\"code\":\"hat\",\"level\":0}],\"company\":\"f68eb899-7619-4f34-827c-fb9b86c1a38a\",\"total_worked_minutes\":420,\"rate_unit\":\"eurPerHour\",\"requested_applicant_number\":1,\"start_date\":\"2019-11-28T11:00:00.000Z\"}" \

teohhanhui commented 4 years ago

Maybe some dump calls would help in debugging this.

pesseyjulien commented 4 years ago

For now I have downgraded to the previous version, I will try some calls later and I will let you know. thanks !

ghost commented 4 years ago

Just confirming that I've seen the same issue. Downgrading to 2.5.1 fixed it. Also #followResponses

soyuka commented 4 years ago

Can you give us the related entity and if there's some additional data to reproduce (POST with data X), thanks!

ghost commented 4 years ago

I've attached the swagger docs and the v2.5.1 json output for the document thats failing.

Archive.zip

FYI We're using MongoDB (Doctrine ODM).

Output message below:

{
  "message": "Unexpected non-iterable value for to-many relation.",
  "status": 400,
  "title": "Bad Request",
  "trace": [
    "Line 176: \\ApiPlatform\\Core\\Serializer\\AbstractItemNormalizer::getAttributeValue",
    "Line 151: \\Symfony\\Component\\Serializer\\Normalizer\\AbstractObjectNormalizer::normalize",
    "Line 146: \\ApiPlatform\\Core\\Serializer\\AbstractItemNormalizer::normalize",
    "Line 119: \\Symfony\\Component\\Serializer\\Serializer::normalize",
    "Line 95: \\Symfony\\Component\\Serializer\\Serializer::serialize",
    "Line 126: \\ApiPlatform\\Core\\EventListener\\SerializeListener::onKernelView",
    "Line 264: \\Symfony\\Component\\EventDispatcher\\Debug\\WrappedListener::__invoke",
    "Line 239: \\Symfony\\Component\\EventDispatcher\\EventDispatcher::doDispatch",
    "Line 73: \\Symfony\\Component\\EventDispatcher\\EventDispatcher::callListeners",
    "Line 168: \\Symfony\\Component\\EventDispatcher\\EventDispatcher::dispatch",
    "Line 151: \\Symfony\\Component\\EventDispatcher\\Debug\\TraceableEventDispatcher::dispatch",
    "Line 68: \\Symfony\\Component\\HttpKernel\\HttpKernel::handleRaw",
    "Line 201: \\Symfony\\Component\\HttpKernel\\HttpKernel::handle",
    "Line 28: \\Symfony\\Component\\HttpKernel\\Kernel::handle"
  ]
}
bastnic commented 4 years ago

Hmm, just checking in on this error.

class MyEntity
{
    /**
     * @var Collection|Workspace[]
     *
     * @ORM\ManyToMany(targetEntity="Workspace", inversedBy="XXXX")
     */
    private $workspaces;

    public function getWorkspace()
    {
        return $this->workspaces->first();
    }

    public function getWorkspaces()
    {
        return $this->workspaces;
    }

} 
App\Entity\MyEntity:
  attributes:
    workspace:
      groups: ['myEntity:read']
    workspaces:
      groups: ['myEntity:read']

If I rename my method to firstWorkspace and change it in the serialization, it works. I find it to suspect that I tried with another collection, just creating a method that is the singular of the collection name and bam.

Here $type is seen as an array for workspace but $attributeValue is a Workspace as it should be. So not iterable, and it crashes with Unexpected non-iterable value for to-many relation..

teohhanhui commented 4 years ago

@bastnic In your case it doesn't seem like a bug in our code. Symfony PropertyAccess / PropertyInfo (I haven't investigated which one) is confused because you don't follow the naming conventions.

bastnic commented 4 years ago

@teohhanhui you can consider it's a regression so.

It worked before as expected (returning a Workspace) and now throw an error. Maybe, if not a collection, just bypass that piece of code ?

teohhanhui commented 4 years ago

It's a bug fix, not a regression. It was never supposed to work. Getting a resource object when we're expecting a collection just makes no sense, and trying to bypass things just makes our code more complicated for no reason.

bastnic commented 4 years ago

Still, it's not a collection, and before it was returning something correct and now not. I understand that this is an underlying bug (or feature, I will check) in Symfony not an APIP one.

I'm in a position where I can investigate and fix thing. So it's already old news and it's fixed since a long time on my side. But 2.5.2 is a minor release, at least 3 persons experience it and the only information we got is Unexpected non-iterable value for to-many relation. Maybe we should specify in the error where it happens property X of entity Y is supposed to be a collection but it's not

bastnic commented 4 years ago

FYI, for the one who got the error, just add dd($attributeValue, $type, $attribute, get_class($object) ligne 527 de vendor/api-platform/core/src/Serializer/AbstractItemNormalizer.php before the throw. Il will show you whch property/method give you the error and the Type it beilieves it is.

soyuka commented 4 years ago

@pesseyjulien Is your error related to a bad property name? @im-jackhansard we need the entity file, not sure to understand how outputing the documentation may break through this error.

bastnic commented 4 years ago

it seems that on another of our projects, it breaks because the property (supposed to be an array) is null, so not iterable.

teohhanhui commented 4 years ago

the property (supposed to be an array) is null, so not iterable.

Yeah, a to-many relation cannot have a null value. That makes no sense, so it was a bug in your code. :smile:

teohhanhui commented 4 years ago

I agree we can improve the exception message.

pesseyjulien commented 4 years ago

@soyuka no must be 'an array cannot be null' situation for me

bastnic commented 4 years ago

Yeah, a to-many relation cannot have a null value. That makes no sense, so it was a bug in your code. smile

In this case, it was not a to-many relation but a computed array in an entity, which is totally allowed to be null. We defaulted it to an empty array, but we needed my line to just begin to see where the problem is. So +1 on exception message and even maybe a not null check before?

teohhanhui commented 4 years ago

@bastnic That's incorrect. Look at the code here: https://github.com/api-platform/core/blob/fe38de4ee480d1542a858220701d9d2dece0fd43/src/Serializer/AbstractItemNormalizer.php#L518-L524

We already check that it's a to-many resource collection.

teohhanhui commented 4 years ago

@pesseyjulien Have you figured out what's going on in your case? If there's an actual bug, I'd love to fix it and unblock your upgrade path. :smile:

nferrand commented 4 years ago

I'm having the same problem with Api Platform. I think the problem in my case is with the doctrine entity inheritance I have a base entity Account, and an AdminAccount that inherits from Account.
AdminAccount has a manytomany relationship.
I made a little repository to reproduce the bug. https://github.com/nferrand/apiplatform-inheritance-issue

The exception occurs on the list of account entities but not on the list of AdminAccount entities.

bastnic commented 4 years ago

@teohhanhui the type is an array, that IS a collection and the target entity is a resource, just a computed one, not from a "doctrine related" collection. So not a bug at all.

Something like that:

class MyEntity
{
    /**
     * @var Collection|Workspace[]
     *
     * @ORM\ManyToMany(targetEntity="Workspace", inversedBy="XXXX")
     */
    private $workspaces;

    /**
     * @var Workspace[]
     */
    private $filteredWorkspaces;

    public function getWorkspace()
    {
        return $this->workspaces->first();
    }

    public function getWorkspaces()
    {
        return $this->workspaces;
    }

    public function getFilteredWorkspaces()
    {
        // do something that finished like that, null or array.
        return random() ? null : [$workspace1, $workspace2];
    }
} 

Maybe we should have return an empty array, but it's still a regression.

pesseyjulien commented 4 years ago

@teohhanhui So yeah, on my side, it's because the following field is null :

    /**
     * @ORM\OneToMany(targetEntity="AppBundle\Entity\Candidate", mappedBy="offer", cascade={"persist", "remove"})
     * @ORM\JoinColumn(nullable=true, onDelete="CASCADE")
     */
    protected $candidates;

If in the construct method of the entity I do the following, then it works :

$this->candidates = new ArrayCollection();

teohhanhui commented 4 years ago

Closing as this is not a bug.

regisnew commented 4 years ago

I'm having the same problem with Api Platform. I think the problem in my case is with the doctrine entity inheritance I have a base entity Account, and an AdminAccount that inherits from Account. AdminAccount has a manytomany relationship. I made a little repository to reproduce the bug. https://github.com/nferrand/apiplatform-inheritance-issue

The exception occurs on the list of account entities but not on the list of AdminAccount entities.

same problem here

lucasgranberg commented 4 years ago

I think the problem is here and here. The arguments for is_subclass_of are the wrong way around.

edit: The error comes from trying to serialize a parent class but the properties from the child class are included. This code should work the other way around. When the Child class is serialized the parent class properties should be included.

lucasgranberg commented 4 years ago

Turns out only InheritedPropertyNameCollectionFactory causes trouble. I fixed it in my project by copying the class and flippling the arguments for is_subclass_of

    App\Metadata\InheritedPropertyNameCollectionFactory:
        decorates: api_platform.metadata.property.name_collection_factory
        arguments:
            - "@api_platform.metadata.resource.name_collection_factory"
            - "@api_platform.metadata.property.name_collection_factory.inherited.inner"
Enrac-Nocilihc commented 4 years ago

That solution works perfectly. Personally, I needed to clear my cache after doing it :)

soyuka commented 4 years ago

interesting, could you open a pr with that change?

jdeniau commented 3 years ago

For the record, I had the same problem when upgrading to api-platform 2.5 and 2.6. I did not had the problem with 2.4. My Symfony property info version is 5.2.4 .

For my case, it is a "problem" in Symfony PropertyInfo. The problem is that my class has a "adder" that has the same name that my getter:

class Order {
  public function getInvoiceList() {
    // …
  }
  public function addInvoice(Invoice $invoice) {
    // …
  }
  public function getInvoice(): Invoice { // this method is deprecated but kept for retro-compatibility
    // …
  }
}

Renaming the method addInvoice to addInvoiceList did fix the problem.

It is reproductible with the following snippet :

<?php

require 'vendor/autoload.php';

use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
use Symfony\Component\PropertyInfo\PropertyInfoExtractor;

class Invoice {}

class Order
{
    public function getInvoiceList(): void
    {
        // …
    }

    public function addInvoice(Invoice $invoice): void
    {
        // …
    }

    public function getInvoice(): Invoice
    { // this method is deprecated but kept for retro-compatibility
      // …
      return new Invoice();
    }
}

class Order2
{
    public function getInvoiceList(): void
    {
        // …
    }

    public function addInvoiceList(Invoice $invoice): void
    {
        // …
    }

    public function getInvoice(): Invoice
    { // this method is deprecated but kept for retro-compatibility
      // …
      return new Invoice();
    }
}

$reflectionExtractor = new ReflectionExtractor();

$propertyInfo = new PropertyInfoExtractor(
    [$reflectionExtractor],
    [$reflectionExtractor],
    [],
    [$reflectionExtractor],
    [$reflectionExtractor]
);

// see below for more examples
dump($propertyInfo->getTypes(Order::class, 'invoice')[0]); // builtinType is "array"
dump($propertyInfo->getTypes(Order2::class, 'invoice')[0]); // builtinType is "object"