Doctrine 1
  1. Doctrine 1
  2. DC-300

synchronizeWithArray deletes Entries in RefTable when updating related Entries

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.2.0-RC1
    • Fix Version/s: 1.2.2
    • Component/s: Record, Relations
    • Labels:
      None

      Description

      You have Users and Groups with a ManyToMany relation (refClass = UserGroup).
      Then you do a synchronizeWithArray() on a user:

       
      $user->synchronizeWithArray(array(
          'Groups' => array(
              array('name' => 'updated Group')
          )
      ));
      

      When you update a record with synchronizeWithArray() and you update ManyToMany related entries, the RefTable entries gets deleted.

      I think the problem is located near:
      Doctrine_Record line 2024:

       
              // Eliminate relationships missing in the $array
              foreach ($this->_references as $name => $relation) {
      	        $rel = $this->getTable()->getRelation($name);
      	
      // PROBLEM: RefClass "UserGroup" will never be set in the sync-Array (just "Groups"), so all entries in UserGroup-Table will be deleted
      // only new Groups that are synced will be created with a link in the refTable
      		if ( ! isset($array[$name]) && ( ! $rel->isOneToOne() || ! isset($array[$rel->getLocalFieldName()]))) {
                          unset($this->$name);
                      }
              }
      

      TestCase follows.

        Activity

        Hide
        Marcus Häußler added a comment -

        TestCase

        Show
        Marcus Häußler added a comment - TestCase
        Hide
        Jonathan H. Wage added a comment -

        This one is proving to be very tricky to fix. Any suggestions or ideas for solutions would be appreciated.

        Show
        Jonathan H. Wage added a comment - This one is proving to be very tricky to fix. Any suggestions or ideas for solutions would be appreciated.
        Hide
        Marcus Häußler added a comment - - edited

        Is there any possibilty to check wether a relation is used as "refClass"?
        In setUp() you say hasMany(array([..], 'refClass' => 'UserGroup')), but i could not find this information in the relation-object.

        Then you could do something like this

        if ( ! isset($array[$name]) && ( (!$rel->isRefClass() && ! $rel->isOneToOne()) || ! isset($array[$rel->getLocalFieldName()]))) {
            unset($this->$name);
        }
        

        The unlinking works anyway.

        Show
        Marcus Häußler added a comment - - edited Is there any possibilty to check wether a relation is used as "refClass"? In setUp() you say hasMany(array( [..] , 'refClass' => 'UserGroup')), but i could not find this information in the relation-object. Then you could do something like this if ( ! isset($array[$name]) && ( (!$rel->isRefClass() && ! $rel->isOneToOne()) || ! isset($array[$rel->getLocalFieldName()]))) { unset($this->$name); } The unlinking works anyway.
        Hide
        Jonathan H. Wage added a comment -

        This fixes your test case but I want to test it before we include it in a release(1.2.2)

        Index: lib/Doctrine/Relation.php
        ===================================================================
        --- lib/Doctrine/Relation.php	(revision 6882)
        +++ lib/Doctrine/Relation.php	(working copy)
        @@ -74,6 +74,8 @@
                                           'orderBy' => null
                                           );
         
        +    protected $_isRefClass = null;
        +
             /**
              * constructor
              *
        @@ -416,6 +418,24 @@
                 }
             }
         
        +    public function isRefClass()
        +    {
        +        if ($this->_isRefClass === null) {
        +            $this->_isRefClass = false;
        +            $table = $this->getTable();
        +            foreach ($table->getRelations() as $name => $relation) {
        +                foreach ($relation['table']->getRelations() as $relation) {
        +                    if (isset($relation['refTable']) && $relation['refTable'] === $table) {
        +                        $this->_isRefClass = true;
        +                        break(2);
        +                    }
        +                }
        +            }
        +        }
        +
        +        return $this->_isRefClass;
        +    }
        +
             /**
              * __toString
              *
        Index: lib/Doctrine/Record.php
        ===================================================================
        --- lib/Doctrine/Record.php	(revision 6882)
        +++ lib/Doctrine/Record.php	(working copy)
        @@ -2024,8 +2024,8 @@
                 // Eliminate relationships missing in the $array
                 foreach ($this->_references as $name => $relation) {
         	        $rel = $this->getTable()->getRelation($name);
        -	
        -			if ( ! isset($array[$name]) && ( ! $rel->isOneToOne() || ! isset($array[$rel->getLocalFieldName()]))) {
        +
        +            if ( ! $rel->isRefClass() && ! isset($array[$name]) && ( ! $rel->isOneToOne() || ! isset($array[$rel->getLocalFieldName()]))) {
                         unset($this->$name);
                     }
                 }
        Index: tests/Ticket/DC300TestCase.php
        ===================================================================
        --- tests/Ticket/DC300TestCase.php	(revision 0)
        +++ tests/Ticket/DC300TestCase.php	(revision 0)
        @@ -0,0 +1,127 @@
        +<?php
        +/*
        + *  $Id$
        + *
        + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
        + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
        + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
        + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
        + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
        + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
        + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
        + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
        + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
        + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
        + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
        + *
        + * This software consists of voluntary contributions made by many individuals
        + * and is licensed under the LGPL. For more information, see
        + * <http://www.phpdoctrine.org>.
        + */
        +
        +/**
        + * Doctrine_Ticket_DC300_TestCase
        + *
        + * @package     Doctrine
        + * @author      Konsta Vesterinen <kvesteri@cc.hut.fi>
        + * @license     http://www.opensource.org/licenses/lgpl-license.php LGPL
        + * @category    Object Relational Mapping
        + * @link        www.phpdoctrine.org
        + * @since       1.0
        + * @version     $Revision$
        + */
        +class Doctrine_Ticket_DC300_TestCase extends Doctrine_UnitTestCase
        +{
        +    public function prepareData()
        +    {
        +        $g1 = new Ticket_DC300_Group();
        +        $g1['name'] = 'group1';
        +        $g1->save();
        +
        +        $g2 = new Ticket_DC300_Group();
        +        $g2['name'] = 'group2';
        +        $g2->save();
        +
        +        $g3 = new Ticket_DC300_Group();
        +        $g3['name'] = 'group3';
        +        $g3->save();
        +
        +        $u1 = new Ticket_DC300_User();
        +        $u1['name'] = 'user1';
        +        $u1['Groups']->add($g1);
        +        $u1['Groups']->add($g2);
        +        $u1->save();
        +    }
        +
        +    public function prepareTables()
        +    {
        +        $this->tables[] = 'Ticket_DC300_Group';
        +        $this->tables[] = 'Ticket_DC300_User';
        +        $this->tables[] = 'Ticket_DC300_UserGroup';
        +        parent::prepareTables();
        +    }
        +
        +    public function testRefTableEntriesOnManyToManyRelationsWithSynchronizeWithArray()
        +    {
        +		$u1 = Doctrine::getTable('Ticket_DC300_User')->find(1);
        +
        +		// update the groups user (id 1) is linked to
        +		$u1->synchronizeWithArray(array(
        +			'Groups' => array(
        +				array('name' => 'group1 update'),
        +				array('name' => 'group2 update')
        +			)
        +		));
        +		$u1->save();
        +
        +		// update the user-objects with real data from database
        +		$u1->loadReference('Groups');
        +
        +		// check wether the two database-entries in RefTable exists
        +		$this->assertEqual(count($u1->Groups), 2);
        +    }
        +   
        +}
        +
        +class Ticket_DC300_Group extends Doctrine_Record
        +{
        +	public function setTableDefinition()
        +	{
        +		$this->hasColumn('name', 'string', 255);
        +	}
        +	
        +	public function setUp()
        +	{
        +		$this->hasMany('Ticket_DC300_User as Users', array(
        +			'local' => 'group_id',
        +			'foreign' => 'user_id',
        +			'refClass' => 'Ticket_DC300_UserGroup'
        +		));
        +	}
        +}
        +
        +class Ticket_DC300_User extends Doctrine_Record
        +{
        +	public function setTableDefinition()
        +	{
        +		$this->hasColumn('name', 'string', 255);
        +	}
        +
        +	public function setUp()
        +	{
        +		$this->hasMany('Ticket_DC300_Group as Groups', array(
        +			'local' => 'user_id',
        +			'foreign' => 'group_id',
        +			'refClass' => 'Ticket_DC300_UserGroup'
        +		));
        +	}
        +}
        +
        +class Ticket_DC300_UserGroup extends Doctrine_Record
        +{
        +	public function setTableDefinition()
        +	{
        +		$this->hasColumn('user_id', 'integer');
        +		$this->hasColumn('group_id', 'integer');
        +	}
        +}
        \ No newline at end of file
        
        Show
        Jonathan H. Wage added a comment - This fixes your test case but I want to test it before we include it in a release(1.2.2) Index: lib/Doctrine/Relation.php =================================================================== --- lib/Doctrine/Relation.php (revision 6882) +++ lib/Doctrine/Relation.php (working copy) @@ -74,6 +74,8 @@ 'orderBy' => null ); + protected $_isRefClass = null ; + /** * constructor * @@ -416,6 +418,24 @@ } } + public function isRefClass() + { + if ($ this ->_isRefClass === null ) { + $ this ->_isRefClass = false ; + $table = $ this ->getTable(); + foreach ($table->getRelations() as $name => $relation) { + foreach ($relation['table']->getRelations() as $relation) { + if (isset($relation['refTable']) && $relation['refTable'] === $table) { + $ this ->_isRefClass = true ; + break (2); + } + } + } + } + + return $ this ->_isRefClass; + } + /** * __toString * Index: lib/Doctrine/Record.php =================================================================== --- lib/Doctrine/Record.php (revision 6882) +++ lib/Doctrine/Record.php (working copy) @@ -2024,8 +2024,8 @@ // Eliminate relationships missing in the $array foreach ($ this ->_references as $name => $relation) { $rel = $ this ->getTable()->getRelation($name); - - if ( ! isset($array[$name]) && ( ! $rel->isOneToOne() || ! isset($array[$rel->getLocalFieldName()]))) { + + if ( ! $rel->isRefClass() && ! isset($array[$name]) && ( ! $rel->isOneToOne() || ! isset($array[$rel->getLocalFieldName()]))) { unset($ this ->$name); } } Index: tests/Ticket/DC300TestCase.php =================================================================== --- tests/Ticket/DC300TestCase.php (revision 0) +++ tests/Ticket/DC300TestCase.php (revision 0) @@ -0,0 +1,127 @@ +<?php +/* + * $Id$ + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * This software consists of voluntary contributions made by many individuals + * and is licensed under the LGPL. For more information, see + * <http: //www.phpdoctrine.org>. + */ + +/** + * Doctrine_Ticket_DC300_TestCase + * + * @ package Doctrine + * @author Konsta Vesterinen <kvesteri@cc.hut.fi> + * @license http: //www.opensource.org/licenses/lgpl-license.php LGPL + * @category Object Relational Mapping + * @link www.phpdoctrine.org + * @since 1.0 + * @version $Revision$ + */ +class Doctrine_Ticket_DC300_TestCase extends Doctrine_UnitTestCase +{ + public function prepareData() + { + $g1 = new Ticket_DC300_Group(); + $g1['name'] = 'group1'; + $g1->save(); + + $g2 = new Ticket_DC300_Group(); + $g2['name'] = 'group2'; + $g2->save(); + + $g3 = new Ticket_DC300_Group(); + $g3['name'] = 'group3'; + $g3->save(); + + $u1 = new Ticket_DC300_User(); + $u1['name'] = 'user1'; + $u1['Groups']->add($g1); + $u1['Groups']->add($g2); + $u1->save(); + } + + public function prepareTables() + { + $ this ->tables[] = 'Ticket_DC300_Group'; + $ this ->tables[] = 'Ticket_DC300_User'; + $ this ->tables[] = 'Ticket_DC300_UserGroup'; + parent::prepareTables(); + } + + public function testRefTableEntriesOnManyToManyRelationsWithSynchronizeWithArray() + { + $u1 = Doctrine::getTable('Ticket_DC300_User')->find(1); + + // update the groups user (id 1) is linked to + $u1->synchronizeWithArray(array( + 'Groups' => array( + array('name' => 'group1 update'), + array('name' => 'group2 update') + ) + )); + $u1->save(); + + // update the user-objects with real data from database + $u1->loadReference('Groups'); + + // check wether the two database-entries in RefTable exists + $ this ->assertEqual(count($u1->Groups), 2); + } + +} + +class Ticket_DC300_Group extends Doctrine_Record +{ + public function setTableDefinition() + { + $ this ->hasColumn('name', 'string', 255); + } + + public function setUp() + { + $ this ->hasMany('Ticket_DC300_User as Users', array( + 'local' => 'group_id', + 'foreign' => 'user_id', + 'refClass' => 'Ticket_DC300_UserGroup' + )); + } +} + +class Ticket_DC300_User extends Doctrine_Record +{ + public function setTableDefinition() + { + $ this ->hasColumn('name', 'string', 255); + } + + public function setUp() + { + $ this ->hasMany('Ticket_DC300_Group as Groups', array( + 'local' => 'user_id', + 'foreign' => 'group_id', + 'refClass' => 'Ticket_DC300_UserGroup' + )); + } +} + +class Ticket_DC300_UserGroup extends Doctrine_Record +{ + public function setTableDefinition() + { + $ this ->hasColumn('user_id', 'integer'); + $ this ->hasColumn('group_id', 'integer'); + } +} \ No newline at end of file
        Hide
        Marcus Häußler added a comment -

        I tested your patch in our project and it seems to work very well.

        Thank you and your team for the good work !

        Show
        Marcus Häußler added a comment - I tested your patch in our project and it seems to work very well. Thank you and your team for the good work !

          People

          • Assignee:
            Jonathan H. Wage
            Reporter:
            Marcus Häußler
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: