Skip to content
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

fix: ensure clone method creates a deep clone #6743

Open
wants to merge 4 commits into
base: 4.0
Choose a base branch
from

Conversation

Kyzegs
Copy link
Contributor

@Kyzegs Kyzegs commented Oct 23, 2024

Q A
Branch? 4.0
Tickets N/A
License MIT
Doc PR api-platform/docs#...

The CloneTrait clones only the top-level object. Updating nested object(s) on the clone will also update the nested object(s) in
the original object.

Imagine the following case with the current implementation:

/** @implements ProcessorInterface<AccountInput, AccountOutput> */
final class AccountUpdateProcessor implements ProcessorInterface
{
    /** @inheritDoc */
    public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): ?AccountOutput
    {
        $data->getTwoFactorAuthentication()->getMethod(); // totp
        $context['previous_data']->getTwoFactorAuthentication()->getMethod(); // totp
        
        $context['previous_data']->getTwoFactorAuthentication()->setMethod('sms');
        
        // Any mutations on a nested object will reflect on the original object as the nested object uses the same reference as the original
        $data->getTwoFactorAuthentication()->getMethod(); // sms
        $context['previous_data']->getTwoFactorAuthentication()->getMethod(); // sms
    }
}

@Kyzegs
Copy link
Contributor Author

Kyzegs commented Oct 23, 2024

I'll still have to add tests but I'd like to know if this is something that might be merged before I put any more time into it.

@Kyzegs Kyzegs changed the title Feature/deep clone fix: ensure clone method creates a deep clone Oct 23, 2024
@soyuka
Copy link
Member

soyuka commented Oct 28, 2024

can you explain your use case or where it fails?

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

Successfully merging this pull request may close these issues.

2 participants