api-platform / core

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

Unable to PATCH nested non API Platform resource entities when patching a resource #3438

Closed fgamess closed 1 year ago

fgamess commented 4 years ago

API Platform version(s) affected: 2.5.4

Description

Using PATCH to update an API resource with new value(s) for a nested resource, the nested resource is correctly updated. Using PATCH to update an API resource with new value(s) for a nested entity which is not an API resource, the nested entity is not updated. Instead, API Platform will create a new entity from the nested entity payload provided inside the resource and insert it in the database with a new ID.

How to reproduce
Description

I have 3 doctrine entities:

Foo has one Bar (@OneToOne, nullable=true) Foo has one Baz (@OneToOne, nullable=true)

Foo.php

<?php

namespace App\Entity;

use ApiPlatform\Core\Annotation\ApiResource;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Serializer\Annotation\Groups;

/**
 * @ApiResource(
 *     itemOperations={
 *         "patch"={
 *              "method"="PATCH",
 *              "denormalization_context"={
 *                  "groups"={"foo:write"}
 *              },
 *          },
 *          "get"
 *     },
 *     collectionOperations={
 *         "get", "post"
 *     }
 * )
 * @ORM\Entity(repositoryClass="App\Repository\FooRepository")
 */
class Foo
{
    /**
     * @ORM\Id()
     * @ORM\GeneratedValue()
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @ORM\OneToOne(targetEntity="App\Entity\Bar", cascade={"persist", "remove"})
     * @Groups({"foo:write", "foo:read"})
     */
    private $bar;

    /**
     * @ORM\OneToOne(targetEntity="App\Entity\Baz", cascade={"persist", "remove"})
     * @Groups({"foo:write", "foo:read"})
     */
    private $baz;

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

    public function getBar(): ?Bar
    {
        return $this->bar;
    }

    public function setBar(?Bar $bar): self
    {
        $this->bar = $bar;

        return $this;
    }

    public function getBaz(): ?Baz
    {
        return $this->baz;
    }

    public function setBaz(?Baz $baz): self
    {
        $this->baz = $baz;

        return $this;
    }
}

Bar.php

<?php

namespace App\Entity;

use ApiPlatform\Core\Annotation\ApiResource;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Serializer\Annotation\Groups;

/**
 * @ApiResource()
 * @ORM\Entity(repositoryClass="App\Repository\BarRepository")
 */
class Bar
{
    /**
     * @ORM\Id()
     * @ORM\GeneratedValue()
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @ORM\Column(type="string", length=255, nullable=true)
     * @Groups({"foo:write", "foo:read"})
     */
    private $fieldOne;

    /**
     * @ORM\Column(type="string", length=255, nullable=true)
     * @Groups({"foo:write", "foo:read"})
     */
    private $fieldTwo;

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

    public function getFieldOne(): ?string
    {
        return $this->fieldOne;
    }

    public function setFieldOne(?string $fieldOne): self
    {
        $this->fieldOne = $fieldOne;

        return $this;
    }

    public function getFieldTwo(): ?string
    {
        return $this->fieldTwo;
    }

    public function setFieldTwo(?string $fieldTwo): self
    {
        $this->fieldTwo = $fieldTwo;

        return $this;
    }
}

Baz.php

<?php

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Serializer\Annotation\Groups;

/**
 * @ORM\Entity(repositoryClass="App\Repository\BazRepository")
 */
class Baz
{
    /**
     * @ORM\Id()
     * @ORM\GeneratedValue()
     * @ORM\Column(type="integer")
     * @Groups({"foo:read"})
     */
    private $id;

    /**
     * @ORM\Column(type="string", length=255, nullable=true)
     * @Groups({"foo:write", "foo:read"})
     */
    private $fieldThree;

    /**
     * @ORM\Column(type="string", length=255, nullable=true)
     * @Groups({"foo:write", "foo:read"})
     */
    private $fieldFour;

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

    public function getFieldThree(): ?string
    {
        return $this->fieldThree;
    }

    public function setFieldThree(?string $fieldThree): self
    {
        $this->fieldThree = $fieldThree;

        return $this;
    }

    public function getFieldFour(): ?string
    {
        return $this->fieldFour;
    }

    public function setFieldFour(?string $fieldFour): self
    {
        $this->fieldFour = $fieldFour;

        return $this;
    }
}

Assuming I have in the database:

1 Foo record: id bar_id baz_id
1 1 1
1 Bar record: id field_one field_two
1 test1 test2
1 Baz record: id field_three field_four
1 test1 test2

attempt to patch Bar inside Foo:

{
  "bar": {
    "id": "/api/bars/1",
    "fieldOne": "patched value"
  }
}

result:

{
  "@context": "/api/contexts/Foo",
  "@id": "/api/foos/1",
  "@type": "Foo",
  "bar": {
    "@id": "/api/bars/1",
    "@type": "Bar",
    "fieldOne": "patched value",
    "fieldTwo": "test2"
  }
}

attempt to patch Baz inside Foo:

{
  "baz": {
    "id": "1",
    "fieldThree": "patched value"
  }
}

obviously, Baz does not have an IRI

result:

{
  "@context": "/api/contexts/Foo",
  "@id": "/api/foos/1",
  "@type": "Foo",
  "bar": {
    "@id": "/api/bars/1",
    "@type": "Bar",
    "fieldOne": "patched value",
    "fieldTwo": "test2"
  },
  "baz": {
    "id": 2,
    "fieldThree": "patched value"
  }
}

As you can see a new baz record has been created in the database, the record with id=1 has not been updated.

Possible Solution
Maybe we need to explore the use of PropertyMetadata as suggested by one of the member of the core team

Additional Context

fgamess commented 4 years ago

Up. Could you confirm if it is an issue or not? @dunglas @teohhanhui

tecbird commented 4 years ago

i've the same problem, if the $id property has "ORM\GeneratedValue()" it will be only read-only, as a result it is not settable for netsted update logic.

if i remove "ORM\GeneratedValue()" so it's not auto incremented, i've to set always a id for new entries - i can not set "null" to say doctrine should generate the id

i think the main problem is, that "ORM\GeneratedValue()" doesnt't mean its "read-only", it must be writeable to set existing id for update purpose.

fgamess commented 4 years ago

Hi, @dunglas @teohhanhui any thoughts about this issue? I developed a customer utility that handles the patch instead of using the default PATCH from API Platform to solve this. But I don't think I approached the problem in a good way.

Mindphreaker commented 3 years ago

i've the same problem, if the $id property has "ORM\GeneratedValue()" it will be only read-only, as a result it is not settable for netsted update logic.

if i remove "ORM\GeneratedValue()" so it's not auto incremented, i've to set always a id for new entries - i can not set "null" to say doctrine should generate the id

i think the main problem is, that "ORM\GeneratedValue()" doesnt't mean its "read-only", it must be writeable to set existing id for update purpose.

I have exactly the same issue. Need to update the reference to an object with another existing (auto-generated) id. @tecbird any chance you got this working (maybe with a workaround?)

@dunglas is this a confirmed bug? I would have thought that using auto-generated IDs e.g. ORM\GeneratedValue() is more or less a standard case. Any update on this?

fgamess commented 3 years ago

Hi @dunglas @teohhanhui I found a way to get this working by creating a custom data provider targeting specific resources. One practical use case where the issue happens is, for example, when having API resources which are MongoDB documents (using Doctrine ODM) and you want to patch inner embedded MongoDB documents.

peinz commented 2 years ago

Hi @fgamess. Could you please create a gist for your custom data provider. I have the same problem.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.