Skip to content

Updating embedded object with PATCH operation #4293

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

Closed
KableCH opened this issue Jul 5, 2020 · 25 comments
Closed

Updating embedded object with PATCH operation #4293

KableCH opened this issue Jul 5, 2020 · 25 comments

Comments

@KableCH
Copy link

KableCH commented Jul 5, 2020

API Platform version(s) affected: v2.5.6

Description
In a project I have two entities Person and Address, where Address is related to Person via ManyToOne. They look like this (shortened for brevity):

Person.php

/**
 * @ORM\Entity(repositoryClass=PersonRepository::class)
 * @ApiResource(
 *      ...
 * )
 */
class Person
{
    /**
     * @ORM\OneToMany(targetEntity=Address::class, mappedBy="person", orphanRemoval=true, cascade={"persist", "remove"})
     * @Groups({"user:read", "user:write"})
     */
    private $addresses;

    public function __construct()
    {
        $this->addresses = new ArrayCollection();
    }

    /**
     * @return Collection|Address[]
     */
    public function getAddresses(): Collection
    {
        return $this->addresses;
    }

    public function addAddress(Address $address): self
    {
        if (!$this->addresses->contains($address)) {
            $this->addresses[] = $address;
            $address->setPerson($this);
        }

        return $this;
    }

    public function removeAddress(Address $address): self
    {
        if ($this->addresses->contains($address)) {
            $this->addresses->removeElement($address);
            // set the owning side to null (unless already changed)
            if ($address->getPerson() === $this) {
                $address->setPerson(null);
            }
        }

        return $this;
    }
}

Address

/**
 * @ORM\Entity(repositoryClass=AddressRepository::class)
 * @ApiResource(
 *    ...
 * )
 */
class Address
{
    /**
     * @ORM\Column(type="string", length=255)
     * @Groups({"address:read", "address:write", "user:read", "user:write"})
     */
    private $country;

    /**
     * @ORM\ManyToOne(targetEntity=Person::class, inversedBy="addresses")
     * @ORM\JoinColumn(nullable=false)
     */
    private $person;

    public function getCountry(): ?string
    {
        return $this->country;
    }

    public function setCountry(string $country): self
    {
        $this->country = $country;

        return $this;
    }

    public function getPerson(): ?Person
    {
        return $this->person;
    }

    public function setPerson(?Person $person): self
    {
        $this->person = $person;

        return $this;
    }
}

I want to be able to create new addresses at when posting to the Person collection, and update addresses when patching/putting a Person item. POST and PUT endpoints work as expected, as in, new Address items are created when posting and Address items are updated when putting. But when using PATCH, a new Addess is created and persisted and the old one is deleted by orphan removal, even when passing @id. Here is the request body I used (same for both operations):

{
  "firstName": "John",
  "lastName": "Smith",
  "addresses": [
    {
      "@id": "/api/addresses/1",
      "city": "New York",
      "street": "736 Manor Station St.",
      "country": "USA"
    }
  ]
}

And here the responses I got:
PUT:

{
  "@context": "/api/contexts/Person",
  "@id": "/api/people/1",
  "@type": "Person",
  "firstName": "John",
  "lastName": "Smith",
  "addresses": [
    {
      "@id": "/api/addresses/1",
      "@type": "Address",
      "city": "New York",
      "street": "736 Manor Station St.",
      "country": "USA"
    }
  ]
}

PATCH

{
  "@context": "/api/contexts/Person",
  "@id": "/api/people/1",
  "@type": "Person",
  "firstName": "John",
  "lastName": "Smith",
  "addresses": [
    {
      "@id": "/api/addresses/2",
      "@type": "Address",
      "city": "New York",
      "street": "736 Manor Station St.",
      "country": "USA"
    }
  ]
}

Note the @id's aren't the same

How to reproduce
I created a fresh symfony project, installed api platform via composer require api, created the two entities (code above) and got the same behaviour when running the requests above.

Additional Context
I am not sure if this is a bug or intended behaviour, but the I couldn't find anything in the api platform docs that would indicate that PATCH doesn't support updating embedded objects.

@rugolinifr
Copy link

rugolinifr commented Jul 17, 2020

Hi,
i guess you refers to this part of the doc.
Since it mentions POST et PUT requests only, i assume PATCH is not supported with embedded objects.

As a workaround, you can still use PUT requests: on my api-platform project, sending PUT requests with partial json body works fine, the existing object is fetched from the data provider, then it is updated with supplied data only.

@MarcDiaz
Copy link

MarcDiaz commented Mar 19, 2021

Hello,

Yesterday, I asked @dunglas during a MeetUp to know if this issue will be resolved, he answered me :

updating embedded resources has been fixed recently (in the last patch release IIRC)

I have updated to 2.6.3, but the problem is still here. As I use SoftDelete, I see embedded relations to be marked as SoftDeleted and re-added instead of being updated.

Maybe the patch that @dunglas was talking about hasn't been released yet?

@soyuka
Copy link
Member

soyuka commented Mar 19, 2021

Hi, I don't understand your use case @MarcDiaz please open a new issue and provide a reproducer.

See https://github.com/api-platform/core/blob/2.6/features/main/patch.feature#L32-L60 where we do update a node.

/!\ The behavior is different on collections as specifies the Merge Patch RFC on collection nodes are added/removed and not updated !

@MarcDiaz
Copy link

MarcDiaz commented Mar 19, 2021

Hi @soyuka!

In fact, it's exactly the same problem as the issue above. As @rugolinifr said above, the doc doesn't mention PATCH for updating embedded relations in https://api-platform.com/docs/core/serialization/#denormalization.

It's also the same issue in #4061, and it continues in #759.

@mbrodala
Copy link
Contributor

@soyuka The example you mention is a 1:1 relation, the issue here talks about a 1:n relation.

I can confirm that the related object is added to the relation collection as completely new. Thus without orphanRemoval, you'll have an additional object in the collection afterwards. With orphanRemoval=true, you'll have all previously related objects dropped and replaced with the objects you sent with the PATCH request. Notice that dropped here seriously means that the previous objects are completely dropped from the database.

@mbrodala
Copy link
Contributor

I have just posted a draft PR to test this in #4263. Since the tests are vast I wasn't exactly sure where to put the relation. Also notice that I've skipped ODM since I've never used it.

@alexislefebvre
Copy link
Contributor

Automatic links to other repos don't work, here is the PR: #4263

@mbrodala
Copy link
Contributor

mbrodala commented May 3, 2021

For now we use the following normalizer as workaround:

<?php

declare(strict_types=1);

namespace App\Api\Normalizer\JsonLd;

use Symfony\Component\Serializer\Normalizer\AbstractNormalizer;
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
use Symfony\Component\Serializer\SerializerAwareInterface;
use Symfony\Component\Serializer\SerializerInterface;

final class PatchAwareItemNormalizer implements NormalizerInterface, DenormalizerInterface, SerializerAwareInterface
{
    private const OPERATION_NAME = 'patch';

    public function __construct(
        private NormalizerInterface $decorated,
    ) {
        if (!$decorated instanceof DenormalizerInterface) {
            throw new \InvalidArgumentException(sprintf('The decorated normalizer must implement the %s.', DenormalizerInterface::class));
        }
    }

    /**
     * {@inheritdoc}
     */
    public function supportsNormalization($data, string $format = null)
    {
        return $this->decorated->supportsNormalization($data, $format);
    }

    /**
     * {@inheritdoc}
     */
    public function normalize($object, string $format = null, array $context = [])
    {
        return $this->decorated->normalize($object, $format, $context);
    }

    /**
     * {@inheritdoc}
     */
    public function supportsDenormalization($data, string $type, string $format = null)
    {
        return $this->decorated->supportsDenormalization($data, $type, $format);
    }

    /**
     * {@inheritdoc}
     */
    public function denormalize($data, string $type, string $format = null, array $context = [])
    {
        if (isset($data['@id']) && $context['item_operation_name'] === self::OPERATION_NAME) {
            // Ensure the the object to populate is loaded by the JSON-LD ItemNormalizer
            unset($context[AbstractNormalizer::OBJECT_TO_POPULATE]);
        }

        return $this->decorated->denormalize($data, $type, $format, $context);
    }

    public function setSerializer(SerializerInterface $serializer)
    {
        if ($this->decorated instanceof SerializerAwareInterface) {
            $this->decorated->setSerializer($serializer);
        }
    }
}
services:
    App\Api\Normalizer\JsonLd\PatchAwareItemNormalizer:
        decorates: api_platform.jsonld.normalizer.item

Normally the object to populate in the context would be already filled with the ArrayCollection of the relation leading to the JSON-LD ItemNormalizer skipping any incoming IRI in @id. By dropping the object from the context here, it is then loaded and put into the context by the ItemNormalizer..

To use this custom normalizer, API resources must specify jsonld with application/merge-patch+json as input format for the patch operation:

/**
 * @ApiResource(
 *     collectionOperations={
 *         "get",
 *         "post",
 *     },
 *     itemOperations={
 *         "get",
 *         "put",
 *         "patch"={
 *             "input_formats"={
 *                 "jsonld"={
 *                     "application/merge-patch+json",
 *                 },
 *             },
 *         },
 *         "delete",
 *     },
 * )
 */

@soyuka soyuka transferred this issue from api-platform/api-platform May 26, 2021
@soyuka
Copy link
Member

soyuka commented May 26, 2021

I can confirm that the related object is added to the relation collection as completely new. Thus without orphanRemoval, you'll have an additional object in the collection afterwards. With orphanRemoval=true, you'll have all previously related objects dropped and replaced with the objects you sent with the PATCH request. Notice that dropped here seriously means that the previous objects are completely dropped from the database.

Please understand that this is how the Merge PATCH format works. I think that you'd need this implementation instead: #759. Let me know if I'm wrong and I'll reopen this issue.

@soyuka soyuka closed this as completed May 26, 2021
@mbrodala
Copy link
Contributor

@soyuka I am open for anything which works and tries to stick to standards as much as possible. The less workarounds and hacks we have, the better. So I'll have an eye on the mentioned issue.

@ahmed-bhs
Copy link
Contributor

In case where we have an embed OneToMany relation, event with PUT method, the elements of the array collection will not be merged! And non specified elements going to be deleted I tried with Patch same behavior :/ @MarcDiaz did you find any solution how to patch relation properties !?

@MarcDiaz
Copy link

@MarcDiaz did you find any solution how to patch relation properties !?

@ahmed-bhs Unfortunately no, so I have used PUT instead of PATCH to update embedded relations without remplacements. And when you update a property that is a relation, yes you must send all related objects, else they are removed.

In API Platform 3 (and 2.7), you'll be able to add an object without resending all the elements of the collection, if I have well understood, thanks to the addition of POST/PUT/DELETE on subresouces.

@MarcDiaz
Copy link

Hi everyone,

I've seen that an update of the doc related to this issue has been made some time ago by @alanpoulain: api-platform/docs#1393.
I thought that it meant that this issue was fixed, but it's not still the case.

On API Platform 2.6.8, I tried to update an embedded relation in a PATCH operation filling the @id, and it still deletes then creates a new one.
@alanpoulain, as you updated the doc for this, can you give an example how to proceed? @soyuka seems to say in https://github.com/soyuka that's normal, contrary to PUT.

@erikkubica
Copy link

@mbrodala thank you for the workaround. For those interested in a version for PHP 8.1+ and later api-platform version as of June 2023:

<?php

declare(strict_types=1);

namespace App\Api\Normalizer\JsonLd;

use Symfony\Component\Serializer\Normalizer\AbstractNormalizer;
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
use Symfony\Component\Serializer\SerializerAwareInterface;
use Symfony\Component\Serializer\SerializerInterface;

final class PatchAwareItemNormalizer implements NormalizerInterface, DenormalizerInterface, SerializerAwareInterface
{
    private const OPERATION_NAME = "patch";

    public function __construct(
        private NormalizerInterface $decorated,
    ) {
        if (!$decorated instanceof DenormalizerInterface) {
            throw new \InvalidArgumentException(
                sprintf('The decorated normalizer must implement the %s.', DenormalizerInterface::class)
            );
        }
    }

    /**
     * {@inheritdoc}
     */
    public function supportsNormalization($data, string $format = null)
    {
        return $this->decorated->supportsNormalization($data, $format);
    }

    /**
     * {@inheritdoc}
     */
    public function normalize($object, string $format = null, array $context = [])
    {
        return $this->decorated->normalize($object, $format, $context);
    }

    /**
     * {@inheritdoc}
     */
    public function supportsDenormalization($data, string $type, string $format = null)
    {
        return $this->decorated->supportsDenormalization($data, $type, $format);
    }

    /**
     * {@inheritdoc}
     */
    public function denormalize($data, string $type, string $format = null, array $context = [])
    {
        if (isset($data['@id']) && str_contains($context['operation_name'], self::OPERATION_NAME)) {
            // Ensure the the object to populate is loaded by the JSON-LD ItemNormalizer

            unset($context[AbstractNormalizer::OBJECT_TO_POPULATE]);
        }

        return $this->decorated->denormalize($data, $type, $format, $context);
    }

    public function setSerializer(SerializerInterface $serializer)
    {
        if ($this->decorated instanceof SerializerAwareInterface) {
            $this->decorated->setSerializer($serializer);
        }
    }
}

Use the services as mbrodala provided then on entity:

#[Patch(
    inputFormats: [
        'jsonld' => ['application/merge-patch+json'],
    ],
    controller: OrderUpdateAction::class,
    normalizationContext: [
        'groups' => [
            OrderContext::PATCH,
        ],
    ],
    denormalizationContext: [
        'groups' => [
            OrderContext::PATCH,
        ],
    ],
    security: "is_granted('ROLE_OA2_MANAGE_ALL') or is_granted('ROLE_OA2_ORDER_MANAGE')",
    validationContext: [
        'groups' => [
            OrderContext::PATCH,
        ],
    ],
)]

@mbrodala
Copy link
Contributor

mbrodala commented Jun 19, 2023

Thanks for the hint. For understanding: before $context['item_operation_name'] did contain 'patch'. Now it is empty, which breaks the original check I suggested.

But $context['operation_name'] exists and contains '_api_/<resource>/{id}.{_format}_patch' which can be checked against. Either with str_contains() or better with str_ends_with(). However, given that $context['operation'] is also available and contains the Operation instance I'd suggest using that instead:

<?php

declare(strict_types=1);

namespace App\Api\Normalizer\JsonLd;

use ApiPlatform\Metadata\HttpOperation;
use Symfony\Component\Serializer\Normalizer\AbstractNormalizer;
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
use Symfony\Component\Serializer\SerializerAwareInterface;
use Symfony\Component\Serializer\SerializerInterface;

final class PatchAwareItemNormalizer implements NormalizerInterface, DenormalizerInterface, SerializerAwareInterface
{
    public function __construct(
        private readonly NormalizerInterface $decorated,
    ) {
        if (!$decorated instanceof DenormalizerInterface) {
            throw new \InvalidArgumentException(sprintf('The decorated normalizer must implement the %s.', DenormalizerInterface::class));
        }
    }

    public function supportsNormalization($data, string $format = null): bool
    {
        return $this->decorated->supportsNormalization($data, $format);
    }

    public function normalize($object, string $format = null, array $context = []): array|bool|string|int|float|null|\ArrayObject
    {
        return $this->decorated->normalize($object, $format, $context);
    }

    public function supportsDenormalization($data, string $type, string $format = null): bool
    {
        if (!$this->decorated instanceof DenormalizerInterface) {
            throw new \InvalidArgumentException(sprintf('The decorated normalizer must implement the %s.', DenormalizerInterface::class));
        }

        return $this->decorated->supportsDenormalization($data, $type, $format);
    }

    public function denormalize($data, string $type, string $format = null, array $context = [])
    {
        if (!$this->decorated instanceof DenormalizerInterface) {
            throw new \InvalidArgumentException(sprintf('The decorated normalizer must implement the %s.', DenormalizerInterface::class));
        }

        if (isset($data['@id']) && $context['operation']?->getMethod() === HttpOperation::METHOD_PATCH) {
            // Ensure the the object to populate is loaded by the JSON-LD ItemNormalizer
            unset($context[AbstractNormalizer::OBJECT_TO_POPULATE]);
        }

        return $this->decorated->denormalize($data, $type, $format, $context);
    }

    public function setSerializer(SerializerInterface $serializer)
    {
        if ($this->decorated instanceof SerializerAwareInterface) {
            $this->decorated->setSerializer($serializer);
        }
    }
}
Diff for the curious
--- a/src/Api/Normalizer/JsonLd/PatchAwareItemNormalizer.php
+++ b/src/Api/Normalizer/JsonLd/PatchAwareItemNormalizer.php
@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace App\Api\Normalizer\JsonLd;
 
+use ApiPlatform\Metadata\HttpOperation;
 use Symfony\Component\Serializer\Normalizer\AbstractNormalizer;
 use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
 use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
@@ -12,8 +13,6 @@ use Symfony\Component\Serializer\SerializerInterface;
 
 final class PatchAwareItemNormalizer implements NormalizerInterface, DenormalizerInterface, SerializerAwareInterface
 {
-    private const OPERATION_NAME = 'patch';
-
     public function __construct(
         private readonly NormalizerInterface $decorated,
     ) {
@@ -47,7 +46,7 @@ final class PatchAwareItemNormalizer implements NormalizerInterface, Denormalize
             throw new \InvalidArgumentException(sprintf('The decorated normalizer must implement the %s.', DenormalizerInterface::class));
         }
 
-        if (isset($data['@id']) && ($context['item_operation_name'] ?? null) === self::OPERATION_NAME) {
+        if (isset($data['@id']) && $context['operation']?->getMethod() === HttpOperation::METHOD_PATCH) {
             // Ensure the the object to populate is loaded by the JSON-LD ItemNormalizer
             unset($context[AbstractNormalizer::OBJECT_TO_POPULATE]);
         }

@soyuka
Copy link
Member

soyuka commented Jun 20, 2023

better check $context['operation']->getMethod() === 'PATCH'

@mbrodala
Copy link
Contributor

mbrodala commented Jun 20, 2023

Or HttpOperation::PATCH to avoid magic strings, indeed. I've updated my snippet accordingly.

@erikkubica
Copy link

erikkubica commented Sep 8, 2023

It's me again, not sure if it works after all.

Using API Platform Core 3.1.17

I am doing PATCH /api/v1/ambulances/1

{
    "name": "Test ambulance"
    "availability": [
        {
            "@id": "/api/v1/ambulance_availabilities/22",
            "ambulance": "/api/v1/ambulances/1",
            "range": {
                "from": "2023-08-01T00:00:00+00:00",
                "to": "2023-08-15T23:59:59+00:00"
            },
            "availabilityRecords": [
                {
                    "@id": "/api/v1/ambulance_availability_records/33",
                    "range": {
                        "from": "09:00",
                        "to": "17:00"
                    },
                    "procedures": [],
                    "dayOfWeek": 1,
                    "doctor": "/api/v1/doctors/1",
                    "public": true
                }
            ]
        }
    ]
}

The ID of my availability is getting iterated and the data is getting duplicated in the database recreating all the child relationships as well.

If I try to be more specific and add under "@id" of my availability:

"id": 22,

So it becomes:

{
    "name": "Test ambulance"
    "availability": [
        {
            "@id": "/api/v1/ambulance_availabilities/22",
            "id": 22,
            "ambulance": "/api/v1/ambulances/1",
            "range": {
                "from": "2023-08-01T00:00:00+00:00",
                "to": "2023-08-15T23:59:59+00:00"
            },
            "availabilityRecords": [
                {
                    "@id": "/api/v1/ambulance_availability_records/33",
                    "range": {
                        "from": "09:00",
                        "to": "17:00"
                    },
                    "procedures": [],
                    "dayOfWeek": 1,
                    "doctor": "/api/v1/doctors/1",
                    "public": true
                }
            ]
        }
    ]
}

then the response tells me that:

"availability": [
    "/api/v1/ambulance_availabilities/1"
],

How does availability with ID 22 and @id "/api/v1/ambulance_availabilities/22", become ID 1 /api/v1/ambulance_availabilities/1 ?

It's super duper messed up, it almost looks like the ID of the ambulance is used as ID for the availability.

final class PatchAwareItemNormalizer implements NormalizerInterface, DenormalizerInterface, SerializerAwareInterface
{
    public function __construct(
        private readonly NormalizerInterface $decorated,
    ) {
        if (!$decorated instanceof DenormalizerInterface) {
            throw new \InvalidArgumentException(
                sprintf('The decorated normalizer must implement the %s.', DenormalizerInterface::class)
            );
        }
    }

    public function supportsNormalization($data, string $format = null): bool
    {
        return $this->decorated->supportsNormalization($data, $format);
    }

    public function normalize(
        $object,
        string $format = null,
        array $context = []
    ): array|bool|string|int|float|null|\ArrayObject {
        return $this->decorated->normalize($object, $format, $context);
    }

    public function supportsDenormalization($data, string $type, string $format = null): bool
    {
        if (!$this->decorated instanceof DenormalizerInterface) {
            throw new \InvalidArgumentException(
                sprintf('The decorated normalizer must implement the %s.', DenormalizerInterface::class)
            );
        }

        return $this->decorated->supportsDenormalization($data, $type, $format);
    }

    public function denormalize($data, string $type, string $format = null, array $context = [])
    {
        if (!$this->decorated instanceof DenormalizerInterface) {
            throw new \InvalidArgumentException(
                sprintf('The decorated normalizer must implement the %s.', DenormalizerInterface::class)
            );
        }

        $iri = $data['@id'] ?? null;
        $operation = $context['operation'] ?? null;

        //echo(sprintf("Operation: %s\nIRI: %s\n\n", $operation?->getMethod() ?? "-", $iri ?? "-"));

        if ($iri && is_a($operation, Patch::class)) {
            // Ensure the object to populate is loaded by the JSON-LD ItemNormalizer
            unset($context[AbstractNormalizer::OBJECT_TO_POPULATE]);
        }

        return $this->decorated->denormalize($data, $type, $format, $context);
    }

    public function setSerializer(SerializerInterface $serializer)
    {
        if ($this->decorated instanceof SerializerAwareInterface) {
            $this->decorated->setSerializer($serializer);
        }
    }
}
#[Patch(
    inputFormats: [
        'jsonld' => ['application/merge-patch+json'],
    ],
    denormalizationContext: [
        'groups' => [AmbulanceContext::PATCH],
    ],
    security: "is_granted('ROLE_OA2_MANAGE_ALL') or is_granted('ROLE_OA2_AMBULANCE_MANAGE')",
    securityMessage: 'Access denied',
    validationContext: [
        'groups' => [AmbulanceContext::PATCH],
    ],
)]

@MonkeyKiki
Copy link

MonkeyKiki commented Sep 14, 2023

I don't know if it's a good idea but I had to modify code like this to make it works for the subelements :

        if (
            isset($data['@id']) && (
                $context['operation']?->getMethod() === HttpOperation::METHOD_PATCH ||
                $context['root_operation']?->getMethod() === HttpOperation::METHOD_PATCH
            )
        ) {
            unset($context[AbstractNormalizer::OBJECT_TO_POPULATE]);
        }

@erikkubica
Copy link

@MonkeyKiki thanks man, in my case operation is an undefined array key, however after adding some issets, it looks promising. its late at night, will test it in more detail tomorrow and let you know!

@erikkubica
Copy link

It seems to work but not deep enough, in our use-case we have an Ambulance that has AvailabilityGroups and those have AvailabilityRecords (yep a nice big form). When I PATCH the ambulance with the groups and records, groups are no longer re-created which is the expected behavior, but the availability records inside the availability group are getting recreated, which means relations targeting the records themselves are being unset which might cause issues. I will set up xdebug to see what's going on.

@mbrodala
Copy link
Contributor

You can use ($context['root_operation'] ?? $context['operation'])?->getMethod() as a shorter version but yeah, we had to do the same update due to #5743

@soyuka
Copy link
Member

soyuka commented Oct 17, 2023

Is there an issue here? please open a new one if needed an note that merge patch doesn't update embeded collections.

@mdieudonne
Copy link

mdieudonne commented Oct 21, 2023

Author of: #5587
I am still encoutering the problem with the latest release, updating embedded object with PATCH operation does not work for me.
Sticking to PUT (legacy PATCH behavior) when manytomany relation is involved.

I have to roll back to 3.1.19, in ItemNormalizer:

        if (
            isset($data['@id']) && (
                $context['operation']?->getMethod() === HttpOperation::METHOD_PATCH ||
                $context['root_operation']?->getMethod() === HttpOperation::METHOD_PATCH
            )
        ) {
            unset($context[AbstractNormalizer::OBJECT_TO_POPULATE]);
        }


now triggers an error "Warning: Undefined array key "operation"".

@mskoric
Copy link

mskoric commented Aug 5, 2024

The Problem is still present in v3.3.11 . I traced the problem down to the ObjectToPopulateTrait.
context[AbstractNormalizer::OBJECT_TO_POPULATE] is always the PersistentCollection, so the calling normalizer

AbstractItemNormalizer:L233 wont get the correct object, thus interpreting it as "this must be a new resource".

My workaround is to use the above code with a slight change.


<?php

declare(strict_types=1);
namespace App\Serializer\Normalizer;

use ApiPlatform\Metadata\HttpOperation;
use ApiPlatform\Metadata\Patch;
use Doctrine\ORM\PersistentCollection;
use Symfony\Component\Serializer\Normalizer\AbstractNormalizer;
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
use Symfony\Component\Serializer\SerializerAwareInterface;
use Symfony\Component\Serializer\SerializerInterface;

final class PatchAwareItemNormalizer implements NormalizerInterface, DenormalizerInterface, SerializerAwareInterface
{
    public function __construct(
        private readonly NormalizerInterface $decorated,
    ) {
        if (!$decorated instanceof DenormalizerInterface) {
            throw new \InvalidArgumentException(
                sprintf('The decorated normalizer must implement the %s.', DenormalizerInterface::class)
            );
        }
    }

    public function supportsNormalization($data, string $format = null, array $context = []): bool
    {
        return $this->decorated->supportsNormalization($data, $format, $context);
    }

    public function normalize(
        $object,
        string $format = null,
        array $context = []
    ): array|bool|string|int|float|null|\ArrayObject {
        return $this->decorated->normalize($object, $format, $context);
    }

    public function supportsDenormalization($data, string $type, string $format = null, array $context = []): bool
    {
        if (!$this->decorated instanceof DenormalizerInterface) {
            throw new \InvalidArgumentException(
                sprintf('The decorated normalizer must implement the %s.', DenormalizerInterface::class)
            );
        }

        return $this->decorated->supportsDenormalization($data, $type, $format, $context);
    }

    public function denormalize($data, string $type, string $format = null, array $context = []): mixed
    {
        if (!$this->decorated instanceof DenormalizerInterface) {
            throw new \InvalidArgumentException(
                sprintf('The decorated normalizer must implement the %s.', DenormalizerInterface::class)
            );
        }

        $iri = $data['@id'] ?? null;
        $operation = $context['operation'] ?? null;
        $rootOps = $context['root_operation'] ?? null;


        if (is_a($operation, Patch::class) || is_a($rootOps, Patch::class)) {
            if ($context[AbstractNormalizer::OBJECT_TO_POPULATE] instanceof PersistentCollection) {
                if (null !== $iri) {
                    $id = explode('/', $iri);
                    $id = (int)array_pop($id);

                    foreach ($context[AbstractNormalizer::OBJECT_TO_POPULATE] as $object) {
                        if ($object->getId() === $id) {
                            $context[AbstractNormalizer::OBJECT_TO_POPULATE] = $object;
                        }
                    }
                }
                else {
                    unset($context[AbstractNormalizer::OBJECT_TO_POPULATE]);
                }
            }
        }

        return $this->decorated->denormalize($data, $type, $format, $context);
    }

    public function setSerializer(SerializerInterface $serializer): void
    {
        if ($this->decorated instanceof SerializerAwareInterface) {
            $this->decorated->setSerializer($serializer);
        }
    }
    public function getSupportedTypes(?string $format): array
    {
        return [
            ... yourclasses
        ];
    }
}

The crucial part:


 if (is_a($operation, Patch::class) || is_a($rootOps, Patch::class)) {
            if ($context[AbstractNormalizer::OBJECT_TO_POPULATE] instanceof PersistentCollection) {
                if (null !== $iri) {
                    $id = explode('/', $iri);
                    $id = (int)array_pop($id);

                    foreach ($context[AbstractNormalizer::OBJECT_TO_POPULATE] as $object) {
                        if ($object->getId() === $id) {
                            $context[AbstractNormalizer::OBJECT_TO_POPULATE] = $object;
                        }
                    }
                }
                else {
                    unset($context[AbstractNormalizer::OBJECT_TO_POPULATE]);
                }
            }
        }

The idea behind that is to provide the correct object to the "context". So the later processing get the correct Object from the context. If there is no matching id, we need to remove the collection from the context, to get the desired result (create a new record).

@mdieudonne maybe you can try to use the above code and confirm that it its working with your relations

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