Deep cloning a document and it's embedded documents #509

Closed
vmattila opened this Issue Feb 21, 2013 · 7 comments

Projects

None yet

4 participants

@vmattila

I have recently encountered some strange behavior regarding cloning a document object that contains embedded documents. The use case here is to create a copy of the original document with new identity, but exactly same details.

It seems that $newDocument = clone $originalDocument does not clone the embedded documents. $documentManager->persist($newDocument); works find and also embedded documents are copied at that point. However, if an embedded document of $newDocument is modified before (or after persisting), the change is coupled also to the $originalDocument.

Consider the following test case (added to be a part of EmbeddedTest.php)

    public function testCloneParentAndModifyEmbeddedWithoutChangeInOriginal()
    {
        $address = new Address();
        $address->setAddress('20780 Kaarina');

        $user = new User();
        $user->setUsername('vmattila');
        $user->setAddress($address);

        $this->dm->persist($user);
        $this->dm->flush();
        $this->dm->clear();

        $user = $this->dm->createQueryBuilder('Documents\User')
            ->field('id')->equals($user->getId())
            ->getQuery()
            ->getSingleResult();
        $this->assertNotNull($user);

        // Cloning user
        $newUser = clone $user;
        $newUser->setId(null); // Nulling identity of the user
        $this->assertEquals('20780 Kaarina', $newUser->getAddress()->getAddress());

        // Changing address for the new user
        $newUser->getAddress()->setAddress('20100 Turku');
        $this->dm->persist($newUser);
        $this->dm->flush();

        $this->assertEquals('20100 Turku', $newUser->getAddress()->getAddress());
        $this->assertEquals('20780 Kaarina', $user->getAddress()->getAddress());
    }

The test fails, because $newUser->getAddress()->setAddress('20100 Turku'); will couple the change also to $user->getAddress()->setAddress('20100 Turku');.

This behaviour might be expected (is it?) - but a little bit confusing. It would be useful if Doctrine would provide helpers to make a "deep copy" of the document.

@jmikola
Doctrine member

You will need to implement __clone() in your model class, and that will need to handle any embedded objects, collections, etc. I believe the only implementation detail to keep in mind is discussed here, since Doctrine uses __clone() specially for Proxy objects.

@vmattila

Yes, custom implementation of __clone() fixes the issue. However, I would still propose that cloning of embedded documents could be done automatically by Doctrine.

In my understanding, embedded documents live always in context of the main document. If not, they should apparently be Referenced documents. When you have cloned the parent document (and not explicitly cloned and attached an Embedded document to the new clone), the change propagate to the "old" parent. This is exactly the reason behind my issue. I don't find this as logical behavior, as far as we talk about embedded documents. Also, this behavior is possible only right after the cloning. As soon as you fetch the new parent document from the database again, embedded documents are different objects.

The second reason why Doctrine could take care of cloning: I am lazy to write __clone() to every document because there already are @EmbedOne/Many annotations. ;)

@jwage
Doctrine member

A new feature to handle this might be nice. I'd be willing to review that as an enhancement. Nice idea!

@jmikola
Doctrine member

Since ODM performs no code generation other than proxy classes (which aren't really relevant here), I don't see how we could automate cloning of embedded documents to the point where you could just call clone $myDocument and be done with it.

Two alternatives might be:

  • Having a utility class or method (perhaps on the DocumentManager) to clone an object, and recursively clone its inner objects based on mapping data.
  • Have a trait for PHP 5.4+ (I assume one can define a clone implementation in a trait) that would allow clone $myDocument to work. The downside of this is that either the trait would need to be generated specifically for the class (based on mapping data), or we'd have to inject the mapping data into the class, which complicates the notion of models being simple objects.
@jwage
Doctrine member

I was thinking a long the lines of a DocumentManager::clone() method. I think this is potentially a larger discussion for the whole Doctrine project.

@malarzm
Doctrine member

#1252 aims to make situation better but is not enough given there's no Doctrine in provided scenario between reusing same embedded document and changing it's value.

@malarzm malarzm added has PR and removed feature labels Oct 29, 2015
@malarzm malarzm added this to the 1.0.3 milestone Oct 29, 2015
@malarzm
Doctrine member

#1259 is closest we can get to it automatically, as soon as document is persisted the changes to reused embedded documents are no longer shared.

@malarzm malarzm closed this in #1259 Oct 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment