fix(serializer): put replaces embed collection

[soyuka]

fixes #5587 #5603

[soyuka]

failure is unrelated!

[mdieudonne]

This works for me! Thanks.

[jelovac]

Hi, when can we expect a release for this fix?

[jonag]

Hello @soyuka, I have updated API Platform to the last version (v3.1.13) and I think that this PR introduced a BC Break.

In order to get arround this issue I wrote a custom Normalizer to unset the OBJECT_TO_POPULATE and thus allowing me to update a collection operation while PATCHing a resource.

Here is my custom Normalizer:

<?php

namespace App\Normalizer;

use ApiPlatform\Metadata\Patch;
use Doctrine\Common\Collections\Collection;
use Symfony\Component\DependencyInjection\Attribute\AsDecorator;
use Symfony\Component\DependencyInjection\Attribute\AutowireDecorated;
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;

#[AsDecorator('api_platform.jsonld.normalizer.item')]
final readonly class AllowPatchEmbeddedCollectionRelationNormalizer implements NormalizerInterface, DenormalizerInterface, SerializerAwareInterface
{
    public function __construct(
        #[AutowireDecorated] private NormalizerInterface&DenormalizerInterface $decorated,
    ) {
    }

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

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

    /**
     * {@inheritdoc}
     */
    public function denormalize($data, $type, $format = null, array $context = []): mixed
    {
        if (
            ($context['operation'] ?? null) instanceof Patch
            && ($context[AbstractNormalizer::OBJECT_TO_POPULATE] ?? null) instanceof Collection
        ) {
            unset($context[AbstractNormalizer::OBJECT_TO_POPULATE]);
        }

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

    /**
     * {@inheritdoc}
     */
    public function supportsDenormalization($data, $type, $format = null, array $context = []): bool
    {
        return $this->decorated->supportsDenormalization($data, $type, $format);
    }

    /**
     * {@inheritdoc}
     */
    public function setSerializer(SerializerInterface $serializer): void
    {
        if ($this->decorated instanceof SerializerAwareInterface) {
            $this->decorated->setSerializer($serializer);
        }
    }
}

However this line replaces the original operation with a Get operation and therefore breaks my workaround.

Please tell me if I should rather open a new issue to discuss it (I can also upload a repo with my failing test if needed).

[Aerendir]

@soyuka , I confirm this change is a BC change: it broke my tests where:

• PUT Resource with POST SubResource
• PUT Resource with PUT SubResource
• PUT Resource with DELETE SubResource

More, it may also be wrong: the problem reported by @mdieudonne seems to relate to the name of placeholders: all his placeholders are named "id": this causes a conflict.

Removing the child context removes the conflict, but also breaks my PR #5546 : the child context (I'm not 100% sure, but the facts seems to confirm this hypotesys) is required to get the subresources.

So, maybe that simply changing the name of the placeholders, avoiding using the same name for different placeholders, would have resolved the problem reported by @mdieudonne .

Anyway, this PR makes impossible to use, for resources IDs, placeholders that have a different name than "id" (that is exactly what I fixed with my PR - and that was something known as there was a todo comment that remembered to fix it).

WDYT?

[soyuka]

I noted both of your comments they don't seem to have the same concerns... I'll handle these asap, please stay on the previous 3.1.12 for now if you can.

[soyuka]

this looks like this should be reverter

[soyuka]

@jonag can you use root_operation ?

[Aerendir]

I noted both of your comments they don't seem to have the same concerns... I'll handle these asap, please stay on the previous 3.1.12 for now if you can.

Yes, they are two different problems...

[jonag]

@jonag can you use root_operation ?

I should find this key in the context passed to the normalizer? Because this key is currently not set when I dump the context on my custom normalizer.

[soyuka]

core/src/Hydra/Serializer/CollectionNormalizer.php

Line 92 in 92a81f0

$context['root_operation'] = $context['operation'];

it should be there when using Hydra, I may add this to the SerializerContextBuilder but I'd like for you to try it first if it's possible?

[nxtpge]

Hello @jonag and @soyuka, is the fix still ongoing?

[jonag]

Hello @nxtpge, if you are talking about the property root_operation, I forgot to answer to @soyuka but it's indeed already available in the context passed to the normalizer.