Skip to content

Conflict between original_data cloning and application cloning in WriteListener #6362

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
ZZromanZZ opened this issue May 9, 2024 · 4 comments
Labels

Comments

@ZZromanZZ
Copy link

ZZromanZZ commented May 9, 2024

API Platform version(s) affected: 3.3.2

Description

I noticed that in PR #6102 was added feature to clone original data in Symfony WriteListener. This orignal data are further used to generate Content-Location header.
However when someone already use __clone() on Entity (original data object) to provide some other bussiness functionality and resets id property (for example):

public function __clone()
{
    $this->id = null;
}

Generating Content Location header fails with Metadata RuntimeException No identifier value found, did you forget to persist the entity?

image

How to reproduce
Simple GET /api/entity/{id} with __clone() method setting ID property to null results to RuntimeException
Of course, this issue applies only for Symfony (use_symfony_listeners: true)

Possible Solution

I'm not sure why is cloning original data even needed.

@amyAMeQ
Copy link

amyAMeQ commented May 10, 2024

This is definitely a problem for us. My API uses entities from a shared library, and several entities have __clone functions that intentionally do not copy over the id. Things would be ok if the IRI wasn't generated based on previous data, but on current data.

@amyAMeQ
Copy link

amyAMeQ commented May 10, 2024

OK. I have a work around for those of us who stubbornly wanted to update. You can decorate the WriteListener Here is my quick implementation. (I've just banged it out and not put it through any significant tests yet)

<?php

namespace App\EventListener;

use Symfony\Component\DependencyInjection\Attribute\AsDecorator;
use Symfony\Component\HttpKernel\Event\ViewEvent;
use Throwable;

#[AsDecorator('api_platform.listener.view.write')]
class WriteListener
{
    public function __construct(private \ApiPlatform\Symfony\EventListener\WriteListener $decorated)
    {
    }

    public function onKernelView(ViewEvent $event): void
    {
        $this->decorated->onKernelView($event);
        $data = $event->getControllerResult();
        $request = $event->getRequest();
        $method = $request->getMethod();
        if ('GET' === $method) {
            try {
                $originalData = $request->attributes->get('original_data');
                if ($data->getId() && !$originalData->getId()) {
                    $request->attributes->set('original_data', $data);
                }
            } catch (Throwable) {
            }
        }
    }
}

@soyuka soyuka added the bug label May 13, 2024
soyuka added a commit to soyuka/core that referenced this issue May 13, 2024
@soyuka
Copy link
Member

soyuka commented May 13, 2024

I'm not sure why is cloning original data even needed.

Thanks for the detailed report I'll fix this, indeed it looks like cloning is not needed there, can you confirm the patch solves your issue? Thanks.

@ZZromanZZ
Copy link
Author

@soyuka Works, thank you.

soyuka added a commit to soyuka/core that referenced this issue May 13, 2024
@soyuka soyuka closed this as completed May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants