Doctrine MongoDB ODM
  1. Doctrine MongoDB ODM
  2. MODM-66

Bug when persisting referenced collection

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.0.0BETA2
    • Component/s: Persister
    • Labels:
      None

      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
              )
      
      )
      

        Issue Links

          Activity

          Hide
          Bulat Shakirzyanov added a comment -

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

          Show
          Bulat Shakirzyanov added a comment - here is a test case for it http://pastie.org/1136303
          Hide
          Bulat Shakirzyanov added a comment -

          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

          Show
          Bulat Shakirzyanov added a comment - 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
          Hide
          Thomas Adam added a comment -

          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()
          ));
          
          Show
          Thomas Adam added a comment - 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() ));
          Hide
          Bulat Shakirzyanov added a comment -

          looking into it

          Show
          Bulat Shakirzyanov added a comment - looking into it
          Hide
          Bulat Shakirzyanov added a comment -

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

          Show
          Bulat Shakirzyanov added a comment - should be fixed here http://github.com/doctrine/mongodb-odm/commits/MODM-66 , please test and provide feedback
          Hide
          Thomas Adam added a comment - - edited

          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
          )
          
          Show
          Thomas Adam added a comment - - edited 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 )
          Hide
          Vladimir Razuvaev added a comment -

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

          Show
          Vladimir Razuvaev added a comment - Still getting same error with example from original post (after odm update).
          Hide
          Bulat Shakirzyanov added a comment -

          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

          Show
          Bulat Shakirzyanov added a comment - 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
          Hide
          Vladimir Razuvaev added a comment -

          Yes, it is fixed now. Thanks a lot!

          Show
          Vladimir Razuvaev added a comment - Yes, it is fixed now. Thanks a lot!
          Hide
          Thomas Adam added a comment -

          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!

          Show
          Thomas Adam added a comment - 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!
          Hide
          Bulat Shakirzyanov added a comment -

          resolved

          Show
          Bulat Shakirzyanov added a comment - resolved
          Hide
          Thomas Adam added a comment -

          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.

          Show
          Thomas Adam added a comment - 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.
          Hide
          Bulat Shakirzyanov added a comment -

          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.

          Show
          Bulat Shakirzyanov added a comment - 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.
          Hide
          Bulat Shakirzyanov added a comment -

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

          Show
          Bulat Shakirzyanov added a comment - Thomas, it would help me very much if you created a phpunit test case that fails.
          Hide
          Thomas Adam added a comment -

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

          Show
          Thomas Adam added a comment - Ok. I added a PHPUnit test case in MODM-70 . I hope this helps.

            People

            • Assignee:
              Bulat Shakirzyanov
              Reporter:
              Vladimir Razuvaev
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: