[MODM-66] Bug when persisting referenced collection Created: 31/Aug/10  Updated: 06/Sep/10  Resolved: 06/Sep/10

Status: Resolved
Project: Doctrine MongoDB ODM
Component/s: Persister
Affects Version/s: None
Fix Version/s: 1.0.0BETA2

Type: Bug Priority: Major
Reporter: Vladimir Razuvaev Assignee: Bulat Shakirzyanov
Resolution: Fixed Votes: 1
Labels: None

Issue Links:
Duplicate
duplicates MODM-70 Updates of documents after read Closed

 Description   

After adding new entry to referenced collection of existing document, ODM generates invalid update query.

Here is a test case:

/** @Document(db="tests", collection="tests") */
class a
{
    /** @Id */
    protected $id;

    /** @ReferenceMany(targetDocument="b", cascade="all") */
    protected $b;

    function __construct($b) {$this->b = new ArrayCollection($b);}

    function getB() {return $this->b;}
}

/** @Document(db="tests", collection="tests2") */
class b
{
    /** @Id */
    protected $id;

    /** @String */
    protected $tmp;

    function __construct($v) {$this->tmp = $v;}
}

$a = new a(array(new b('first')));
$dm->persist($a);
$dm->flush(); // flush causes notices
$dm->refresh($a);

$a->getB()->add(new b('second'));
$dm->persist($a);
$dm->flush();
$dm->refresh($a);

print_r($a->getB()->toArray());

Expecting following output:

Array
(
    [0] => b Object
        (
            [id:protected] => 4c7c9eea0f9d502005000000
            [tmp:protected] => first
        )

    [1] => b Object
        (
            [id:protected] => 4c7c9eea0f9d502005020000
            [tmp:protected] => second
        )

)

But getting:

Array
(
    [0] => b Object
        (
            [id:protected] => 4c7c9eea0f9d502005000000
            [tmp:protected] => first
        )

    [1] => b Object
        (
            [id:protected] => 4c7c9eea0f9d502005000000
            [tmp:protected] => first
        )

    [2] => b Object
        (
            [id:protected] => 4c7c9eea0f9d502005020000
            [tmp:protected] => second
        )

)


 Comments   
Comment by Bulat Shakirzyanov [ 03/Sep/10 ]

here is a test case for it http://pastie.org/1136303

Comment by Bulat Shakirzyanov [ 03/Sep/10 ]

Jon, fixed here http://github.com/doctrine/mongodb-odm/tree/MODM-66
Please review and resolve jira if everything is ok
As soon as the jira is resolved, I'll merge it in master

Comment by Thomas Adam [ 03/Sep/10 ]

Error is not resolved!
Wrong test case. Test case should properly be so.
are dm->refresh before the next flush. For there do it again a double entry!

$b1 = new B('first');
$a = new A(array($b1));
$this->dm->persist($a);
$this->dm->flush();
$b2 = new B('second');

$this->dm->refresh($a); // !!!!!!!!

$a->getB()->add($b2);
$this->dm->flush(); // He now adds the initial values there too. Thus, the array first, first and second!

// $this->dm->refresh($a);
$b = $a->getB()->toArray();

$this->assertEquals(2, count($b));

$this->assertEquals(array(
	$b1->getId(), $b2->getId()
), array(
	$b[0]->getId(), $b[1]->getId()
));
Comment by Bulat Shakirzyanov [ 03/Sep/10 ]

looking into it

Comment by Bulat Shakirzyanov [ 03/Sep/10 ]

should be fixed here http://github.com/doctrine/mongodb-odm/commits/MODM-66, please test and provide feedback

Comment by Thomas Adam [ 03/Sep/10 ]

So the test is working now.

But in my documents the problem persists!
Here is my test document:

namespace Documents\Avatar;

/**
 * @Document(collection="avatars")
 */
class Avatar {

	/**
	 * @Id
	 */
	protected $_id;
	/**
	 * @String(name="na")
	 * @var string
	 */
	protected $_name;
	/**
	 * @int(name="sex")
	 * @var int
	 */
	protected $_sex;
	/**
	 * @EmbedMany(
	 *	targetDocument="Documents\Avatar\AvatarPartTest",
	 *	name="aP"
	 * )
	 * @var array Documents\Avatar\AvatarPartTest
	 */
	protected $_avatarParts;

	/**
	 * Constructor
	 * @param string $name
	 * @param int $sex
	 */
	public function __construct($name, $sex, $avatarParts = null) {
		$this->_name = $name;
		$this->_sex = $sex;
		$this->_avatarParts = $avatarParts;
	}

	/**
	 * @return string
	 */
	public function getId() {
		return $this->_id;
	}

	/**
	 * @return string
	 */
	public function getName() {
		return $this->_name;
	}

	/**
	 * @param string $name
	 */
	public function setName($name) {
		$this->_name = $name;
	}

	/**
	 * @return int
	 */
	public function getSex() {
		return $this->_sex;
	}

	/**
	 * @param int $sex
	 */
	public function setSex($sex) {
		$this->_sex = $sex;
	}

	/**
	 * @return array Documents\Avatar\AvatarPartTest
	 */
	public function getAvatarParts() {
		return $this->_avatarParts;
	}

	/**
	 * @param Documents\Avatar\AvatarPartTest $part
	 */
	public function addAvatarPart($part) {
		$this->_avatarParts[] = $part;
	}

	/**
	 * @param array Documents\Avatar\AvatarPartTest $parts
	 */
	public function setAvatarParts($parts) {
		$this->_avatarParts = $parts;
	}

	/**
	 * @param Documents\Avatar\AvatarPartTest $part
	 */
	public function removeAvatarPart($part) {
		$key = array_search($this->_avatarParts, $part);
		if ($key !== false) {
			unset($this->_avatarParts[$key]);
		}
	}

}

namespace Documents\Avatar;

/**
 * @EmbeddedDocument
 */
class AvatarPartTest {
	/**
	 * @String(name="col")
	 * @var string
	 */
	protected $_color;

	/**
	 * Constructor
	 * @param string $type
	 * @param string $color
	 */
	public function __construct($color = null) {
		$this->_color = $color;
	}

	/**
	 * @return string
	 */
	public function getColor() {
		return $this->_color;
	}

	/**
	 * @param string $color
	 */
	public function setColor($color) {
		$this->_color = $color;
	}

}

And here is my test code:

$part = new Documents\Avatar\AvatarPartTest();
$avatar = new Documents\Avatar\Avatar('Test', 1, array($part));
$this->dm->persist($avatar);
$this->dm->flush();
$this->dm->refresh($avatar);

$this->dm->getUnitOfWork()->computeChangeSets();
$update = $this->dm->getUnitOfWork()->getDocumentPersister('Documents\Avatar\Avatar')->prepareUpdateData($avatar);
print_r($update);
// print :
Array
(
    [$set] => Array
        (
            [na] => Test
            [sex] => 1
        )
    [$pushAll] => Array
        (
            [aP] => Array
                (
                    [0] => Array
                        (
                        )
                )
        )
)
// That is wrong because it priced the initial values are!

$this->dm->flush();

And this is the log:

// First insert
Array
(
    [batchInsert] => 1
    [num] => 1
    [data] => Array
        (
            [0000000061fd6eaa00000000066fdfcd] => Array
                (
                    [na] => Test
                    [sex] => 1
                    [aP] => Array
                        (
                            [0] => Array
                                (
                                )
                        )
                )
        )
    [class] => Documents\Avatar\Avatar
    [db] =>test
    [collection] => avatars
)

// Refresh findOne

// The last flush after refresh
// I have made no updates yet he wants to save the entire document again!

Array
(
    [update] => 1
    [criteria] => Array
        (
            [_id] => MongoId Object
                (
                )
        )
    [newObj] => Array
        (
            [$set] => Array
                (
                    [na] => Test
                    [sex] => 1
                )
        )
    [options] => Array
        (
        )
    [class] => Documents\Avatar\Avatar
    [db] => test
    [collection] => avatars
)

Array
(
    [update] => 1
    [criteria] => Array
        (
            [_id] => MongoId Object
                (
                )
        )
    [newObj] => Array
        (
            [$pushAll] => Array
                (
                    [aP] => Array
                        (
                            [0] => Array
                                (
                                )
                        )
                )
        )
    [options] => Array
        (
        )
    [class] => Documents\Avatar\Avatar
    [db] => test
    [collection] => avatars
)
Comment by Vladimir Razuvaev [ 04/Sep/10 ]

Still getting same error with example from original post (after odm update).

Comment by Bulat Shakirzyanov [ 05/Sep/10 ]

The fix was in the MODM-66 branch, it is merged now into master, please re-try and confirm you are still getting an error

Comment by Vladimir Razuvaev [ 05/Sep/10 ]

Yes, it is fixed now. Thanks a lot!

Comment by Thomas Adam [ 06/Sep/10 ]

The upper test works, but not my test!
Please test my written test earlier in a comment. Test documents are part of it!

And:

$this->dm->getUnitOfWork()->computeChangeSets();
$update = $this->dm->getUnitOfWork()->getDocumentPersister('Documents\Avatar\Avatar')->prepareUpdateData($avatar);

still shows all the variables in it are already in Mongo!
According to flush all variables reset or push out!

Comment by Bulat Shakirzyanov [ 06/Sep/10 ]

resolved

Comment by Thomas Adam [ 06/Sep/10 ]

So I recorded all the updates, but my test document still does not work. The test for
this ticket is working, but not the test of my comment. Did they not make the mistake that he is a
push with initial values and sets makes name and gender again? Please give me an answer.
With this error is useless for me odm, because I can save anything, because everything in there twice
stands. There are also variables Increments etc.

Comment by Bulat Shakirzyanov [ 06/Sep/10 ]

Thomas, the MODM-70 ticket is still open
Jon, can you please remove related to MODM-70, so that we don't confuse Thomas.
I am working on that ticket.

Comment by Bulat Shakirzyanov [ 06/Sep/10 ]

Thomas, it would help me very much if you created a phpunit test case that fails.

Comment by Thomas Adam [ 06/Sep/10 ]

Ok. I added a PHPUnit test case in MODM-70.
I hope this helps.

Generated at Tue Sep 30 13:59:42 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.