[DDC-758] When merging many to many entites back into the repository changes to the associations are not respected Created: 19/Aug/10  Updated: 31/Oct/10  Resolved: 31/Oct/10

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: ORM
Affects Version/s: None
Fix Version/s: 2.0-RC1
Security Level: All

Type: Bug Priority: Major
Reporter: Dave Keen Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 1
Labels: None

Attachments: Text File 0147-DDC-758.patch     File DDC758Test.php    

 Description   

When merging a DETACHED entity into the repository with ManyToMany associations that have changed since the last flush, the join table is not updated with the new associations.

Many Movies have many Artists with cascade merge:

class Movie {
	
	/** @Id @Column(type="integer") @GeneratedValue(strategy="IDENTITY") */
	public $id;
	
	/** @Column(length=50, type="string") */
	public $title;
	
	/** 
	 * @ManyToMany(targetEntity="Artist", cascade={"merge"})
	 */
	public $artists;
	
	public function __construct() {
		$this->artists = new ArrayCollection();
	}	

}
<?php
class Artist {
	
	/** @Id @Column(type="integer") @GeneratedValue(strategy="IDENTITY") */
	public $id;
	
	/** @Column(length=50, type="string") */
	public $name;
	
	/** @ManyToMany(targetEntity="Movie", mappedBy="artists", cascade={"merge"}) */
	public $movies;
	
	public function __construct() {
		$this->movies = new ArrayCollection();
	}
	
}

Assume that the database contains:
Movie: id=1, title="Movie 1"
Artist: id=1, name="Artist 1"
Artist: id=2, name="Artist 2"

and that there is a entry (1, 1) in movie_artist so that there is a many-many relationship between Movie 1 and Artist 1.

I now want to merge a detched version of a movie into the repository, with an extra relationship between Movie 1 and Artist 2:

$m1 = new \vo\Movie();
$m1->id = 1;
$m1->title = "Movie 1";

$a1 = new \vo\Artist();
$a1->id = 1;
$a1->name = "Artist 1";

$a2 = new \vo\Artist();
$a2->id = 2;
$a2->name = "Artist 2";

$m1->artists->add($a1); $a1->movies->add($m1);

// Add the new association
$m1->artists->add($a2); $a2->movies->add($m1);

// This is a cascade merge so it will merge the entities down the tree
$m1 = $em->merge($m1);

At this point $m1 should contains merged entities reflecting the same as what is in the database, plus the extra association between Movie 1 and Artist 2, so after a flush() movie_artist should contain (1,1),(1,2)

I now run:

$em->flush();

but movie_artist remains unchanged (still just (1,1)) and $em->getUnitOfWork()->computeChangeSets() is empty before the flush().

The same behaviour is exhibited when merging $m1 after deleting entities from its many-to-many associations; movie_artist does not change.



 Comments   
Comment by Dave Keen [ 13/Sep/10 ]

Here is a failing test case for this issue.

Comment by Dave Keen [ 13/Sep/10 ]

The problem area seems to be in the first lines of UnitOfWork::computeAssociationChanges:

if ($value instanceof PersistentCollection && $value->isDirty()) {
            if ($assoc['isOwningSide']) {
                $this->collectionUpdates[] = $value;
            }
            $this->visitedCollections[] = $value;
        }

If I comment out the check for $value->isDirty() then the test case passes so it looks like this is failing because the ManyToMany PersistentCollection isn't getting marked as dirty during computeChangeSets. I've actually fiddled around and found various ways to fix it, but computeChangeSets is pretty complicated and I'm worried that any changes I make might affect performance without me realizing. However, if you can give me a few pointers I'm happy to have another go at patching the issue.

Comment by Dave Keen [ 21/Sep/10 ]

As it turns out commenting out $value->isDirty() only half fixes the problem; it fixes it in the case of adding many to many associations (as in the example about), but it doesn't fix the case of removing many to many associations.

I have updated the attached test to include a test case which creates some many to many associations, then merges in equivalent detached entities but which have empty association attributes. On a cascade merge and flush I would expect this to empty out the associations table; in fact it has no effect.

Comment by Benjamin Eberlei [ 21/Sep/10 ]

removing $value->isDirty() is no option as it is really necessary at that position. The question is, why does merge not mark the collection as dirty.

Comment by Dave Keen [ 21/Sep/10 ]

Agreed. I am hard at work on this right now, and have discovered something that may be another clue. It actually seems the problem is twofold. As well as the collection not getting marked dirty, the reason that the ManyToManyPersister is not removing rows is that PersistentCollection::getDeleteDiff() isn't returning any differences, and the reason for that is that $snapshot isn't get set to anything during the merge process. When merging a collection I think we need to set $snapshot to whatever is currently in the database, and set $coll to whatever is in the collection being merged. I'm not quite sure, but perhaps this could also be the reason why the collection isn't getting set as dirty?

Comment by Dave Keen [ 21/Sep/10 ]

This is an attempt to patch the issue. After each cascade merge this fills in $snapshot from the database.

Comment by Dave Keen [ 21/Sep/10 ]

Slightly changed the patch - this uses setDirty() instead of changed() - without this a Flextrine test fails.

Comment by Benjamin Eberlei [ 31/Oct/10 ]

Fixed, can you verify Dave?

Generated at Tue Jul 29 19:10:45 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.