From 048d7788ceb345a390fe1165e255b2ee0810f3ca Mon Sep 17 00:00:00 2001 From: zYne Date: Sun, 29 Oct 2006 23:24:50 +0000 Subject: [PATCH] Many-to-Many relation handling fixed, fixes #193 --- lib/Doctrine/DataDict/Mysql.php | 202 +++++++++++++-------------- lib/Doctrine/DataDict/Pgsql.php | 2 +- lib/Doctrine/Record.php | 17 +-- lib/Doctrine/Table.php | 21 +-- tests/QueryLimitTestCase.php | 7 +- tests/RelationManyToManyTestCase.php | 126 ++++++++++++++++- tests/run.php | 5 +- 7 files changed, 253 insertions(+), 127 deletions(-) diff --git a/lib/Doctrine/DataDict/Mysql.php b/lib/Doctrine/DataDict/Mysql.php index d93b9c20f..e904aa2af 100644 --- a/lib/Doctrine/DataDict/Mysql.php +++ b/lib/Doctrine/DataDict/Mysql.php @@ -145,128 +145,122 @@ class Doctrine_DataDict_Mysql extends Doctrine_DataDict { $type = array(); $unsigned = $fixed = null; switch ($db_type) { - case 'tinyint': - $type[] = 'integer'; - $type[] = 'boolean'; - if (preg_match('/^(is|has)/', $field['name'])) { - $type = array_reverse($type); - } - $unsigned = preg_match('/ unsigned/i', $field['type']); - $length = 1; - break; - case 'smallint': - $type[] = 'integer'; - $unsigned = preg_match('/ unsigned/i', $field['type']); - $length = 2; - break; - case 'mediumint': - $type[] = 'integer'; - $unsigned = preg_match('/ unsigned/i', $field['type']); - $length = 3; - break; - case 'int': - case 'integer': - $type[] = 'integer'; - $unsigned = preg_match('/ unsigned/i', $field['type']); - $length = 4; - break; - case 'bigint': - $type[] = 'integer'; - $unsigned = preg_match('/ unsigned/i', $field['type']); - $length = 8; - break; - case 'tinytext': - case 'mediumtext': - case 'longtext': - case 'text': - case 'text': - case 'varchar': - $fixed = false; - case 'string': - case 'char': - $type[] = 'text'; - if ($length == '1') { + case 'tinyint': + $type[] = 'integer'; $type[] = 'boolean'; if (preg_match('/^(is|has)/', $field['name'])) { $type = array_reverse($type); } - } elseif (strstr($db_type, 'text')) { - $type[] = 'clob'; - if ($decimal == 'binary') { - $type[] = 'blob'; - } - } - if ($fixed !== false) { - $fixed = true; - } + $unsigned = preg_match('/ unsigned/i', $field['type']); + $length = 1; break; - case 'enum': - $type[] = 'text'; - preg_match_all('/\'.+\'/U', $field['type'], $matches); - $length = 0; - $fixed = false; - if (is_array($matches)) { - foreach ($matches[0] as $value) { - $length = max($length, strlen($value)-2); - } - if ($length == '1' && count($matches[0]) == 2) { + case 'smallint': + $type[] = 'integer'; + $unsigned = preg_match('/ unsigned/i', $field['type']); + $length = 2; + break; + case 'mediumint': + $type[] = 'integer'; + $unsigned = preg_match('/ unsigned/i', $field['type']); + $length = 3; + break; + case 'int': + case 'integer': + $type[] = 'integer'; + $unsigned = preg_match('/ unsigned/i', $field['type']); + $length = 4; + break; + case 'bigint': + $type[] = 'integer'; + $unsigned = preg_match('/ unsigned/i', $field['type']); + $length = 8; + break; + case 'tinytext': + case 'mediumtext': + case 'longtext': + case 'text': + case 'text': + case 'varchar': + $fixed = false; + case 'string': + case 'char': + $type[] = 'text'; + if ($length == '1') { $type[] = 'boolean'; if (preg_match('/^(is|has)/', $field['name'])) { $type = array_reverse($type); } + } elseif (strstr($db_type, 'text')) { + $type[] = 'clob'; + if ($decimal == 'binary') { + $type[] = 'blob'; + } + } + if ($fixed !== false) { + $fixed = true; } - } - $type[] = 'integer'; - case 'set': - $fixed = false; - $type[] = 'text'; - $type[] = 'integer'; break; - case 'date': - $type[] = 'date'; - $length = null; + case 'enum': + $type[] = 'text'; + preg_match_all('/\'.+\'/U', $field['type'], $matches); + $length = 0; + $fixed = false; + if (is_array($matches)) { + foreach ($matches[0] as $value) { + $length = max($length, strlen($value)-2); + } + if ($length == '1' && count($matches[0]) == 2) { + $type[] = 'boolean'; + if (preg_match('/^(is|has)/', $field['name'])) { + $type = array_reverse($type); + } + } + } + $type[] = 'integer'; + case 'set': + $fixed = false; + $type[] = 'text'; + $type[] = 'integer'; break; - case 'datetime': - case 'timestamp': - $type[] = 'timestamp'; - $length = null; + case 'date': + $type[] = 'date'; + $length = null; break; - case 'time': - $type[] = 'time'; - $length = null; + case 'datetime': + case 'timestamp': + $type[] = 'timestamp'; + $length = null; break; - case 'float': - case 'double': - case 'real': - $type[] = 'float'; - $unsigned = preg_match('/ unsigned/i', $field['type']); + case 'time': + $type[] = 'time'; + $length = null; break; - case 'unknown': - case 'decimal': - case 'numeric': - $type[] = 'decimal'; - $unsigned = preg_match('/ unsigned/i', $field['type']); + case 'float': + case 'double': + case 'real': + $type[] = 'float'; + $unsigned = preg_match('/ unsigned/i', $field['type']); break; - case 'tinyblob': - case 'mediumblob': - case 'longblob': - case 'blob': - $type[] = 'blob'; - $length = null; + case 'unknown': + case 'decimal': + case 'numeric': + $type[] = 'decimal'; + $unsigned = preg_match('/ unsigned/i', $field['type']); break; - case 'year': - $type[] = 'integer'; - $type[] = 'date'; - $length = null; + case 'tinyblob': + case 'mediumblob': + case 'longblob': + case 'blob': + $type[] = 'blob'; + $length = null; break; - default: - $db =& $this->getDBInstance(); - if (PEAR::isError($db)) { - return $db; - } - - return $db->raiseError(MDB2_ERROR_UNSUPPORTED, null, null, - 'unknown database attribute type: '.$db_type, __FUNCTION__); + case 'year': + $type[] = 'integer'; + $type[] = 'date'; + $length = null; + break; + default: + throw new Doctrine_DataDict_Mysql_Exception('unknown database attribute type: '.$db_type); } return array($type, $length, $unsigned, $fixed); diff --git a/lib/Doctrine/DataDict/Pgsql.php b/lib/Doctrine/DataDict/Pgsql.php index f61079ccf..4c267d48b 100644 --- a/lib/Doctrine/DataDict/Pgsql.php +++ b/lib/Doctrine/DataDict/Pgsql.php @@ -126,7 +126,7 @@ class Doctrine_DataDict_Pgsql extends Doctrine_DataDict { } $type = array(); $unsigned = $fixed = null; - switch ($field['type']) { + switch (strtolower($field['type'])) { case 'smallint': case 'int2': $type[] = 'integer'; diff --git a/lib/Doctrine/Record.php b/lib/Doctrine/Record.php index 80bf6fd56..ebb247733 100644 --- a/lib/Doctrine/Record.php +++ b/lib/Doctrine/Record.php @@ -847,11 +847,12 @@ abstract class Doctrine_Record extends Doctrine_Access implements Countable, Ite * this method is smart enough to know if any changes are made * and whether to use INSERT or UPDATE statement * - * this method also saves the related composites + * this method also saves the related components * + * @param Doctrine_Connection $conn * @return void */ - final public function save(Doctrine_Connection $conn = null) { + public function save(Doctrine_Connection $conn = null) { if ($conn === null) { $conn = $this->_table->getConnection(); } @@ -885,7 +886,7 @@ abstract class Doctrine_Record extends Doctrine_Access implements Countable, Ite * returns an array of modified fields and associated values * @return array */ - final public function getModified() { + public function getModified() { $a = array(); foreach($this->_modified as $k => $v) { @@ -1301,8 +1302,8 @@ abstract class Doctrine_Record extends Doctrine_Access implements Countable, Ite * @param string $fkField * @return void */ - final public function ownsOne($componentName,$foreignKey, $localKey = null) { - $this->_table->bind($componentName,$foreignKey,Doctrine_Relation::ONE_COMPOSITE, $localKey); + final public function ownsOne($componentName, $foreignKey, $localKey = null) { + $this->_table->bind($componentName, $foreignKey, Doctrine_Relation::ONE_COMPOSITE, $localKey); } /** * binds One-to-Many composite relation @@ -1312,7 +1313,7 @@ abstract class Doctrine_Record extends Doctrine_Access implements Countable, Ite * @return void */ final public function ownsMany($componentName,$foreignKey, $localKey = null) { - $this->_table->bind($componentName,$foreignKey,Doctrine_Relation::MANY_COMPOSITE, $localKey); + $this->_table->bind($componentName, $foreignKey, Doctrine_Relation::MANY_COMPOSITE, $localKey); } /** * binds One-to-One aggregate relation @@ -1322,7 +1323,7 @@ abstract class Doctrine_Record extends Doctrine_Access implements Countable, Ite * @return void */ final public function hasOne($componentName,$foreignKey, $localKey = null) { - $this->_table->bind($componentName,$foreignKey,Doctrine_Relation::ONE_AGGREGATE, $localKey); + $this->_table->bind($componentName, $foreignKey, Doctrine_Relation::ONE_AGGREGATE, $localKey); } /** * binds One-to-Many aggregate relation @@ -1332,7 +1333,7 @@ abstract class Doctrine_Record extends Doctrine_Access implements Countable, Ite * @return void */ final public function hasMany($componentName,$foreignKey, $localKey = null) { - $this->_table->bind($componentName,$foreignKey,Doctrine_Relation::MANY_AGGREGATE, $localKey); + $this->_table->bind($componentName, $foreignKey, Doctrine_Relation::MANY_AGGREGATE, $localKey); } /** * setPrimaryKey diff --git a/lib/Doctrine/Table.php b/lib/Doctrine/Table.php index 550233ff3..9a81d928b 100644 --- a/lib/Doctrine/Table.php +++ b/lib/Doctrine/Table.php @@ -482,6 +482,7 @@ class Doctrine_Table extends Doctrine_Configurable implements Countable { * @return array */ public function getBoundForName($name, $component) { + foreach($this->bound as $k => $bound) { $e = explode('.', $bound[0]); @@ -553,7 +554,7 @@ class Doctrine_Table extends Doctrine_Configurable implements Countable { * @param string $field * @return void */ - final public function bind($name, $field, $type, $localKey) { + public function bind($name, $field, $type, $localKey) { if(isset($this->relations[$name])) unset($this->relations[$name]); @@ -567,7 +568,7 @@ class Doctrine_Table extends Doctrine_Configurable implements Countable { $alias = $name; - $this->bound[$alias] = array($field,$type,$localKey,$name); + $this->bound[$alias] = array($field, $type, $localKey, $name); } /** * getComponentName @@ -661,8 +662,12 @@ class Doctrine_Table extends Doctrine_Configurable implements Countable { $bound = $table->getBoundForName($class, $component); break; } catch(Doctrine_Table_Exception $exc) { } - } + if( ! isset($bound)) + throw new Doctrine_Table_Exception("Couldn't map many-to-many relation for " + . $this->options['name'] . " and $name. Components use different join tables."); + + if( ! isset($local)) $local = $this->identifier; @@ -676,17 +681,17 @@ class Doctrine_Table extends Doctrine_Configurable implements Countable { if(count($fields) > 1) { // SELF-REFERENCING THROUGH JOIN TABLE - $this->relations[$e2[0]] = new Doctrine_Relation_ForeignKey($associationTable,$local,$fields[0],Doctrine_Relation::MANY_COMPOSITE, $e2[0]); + $this->relations[$e2[0]] = new Doctrine_Relation_ForeignKey($associationTable, $local, $fields[0],Doctrine_Relation::MANY_COMPOSITE, $e2[0]); - $relation = new Doctrine_Relation_Association_Self($table,$associationTable,$fields[0],$fields[1], $type, $alias); + $relation = new Doctrine_Relation_Association_Self($table, $associationTable, $fields[0], $fields[1], $type, $alias); } else { // auto initialize a new one-to-one relationship for association table - $associationTable->bind($this->getComponentName(), $associationTable->getComponentName(). '.' .$e2[1], Doctrine_Relation::ONE_AGGREGATE, 'id'); - $associationTable->bind($table->getComponentName(), $associationTable->getComponentName(). '.' .$foreign, Doctrine_Relation::ONE_AGGREGATE, 'id'); + $associationTable->bind($this->getComponentName(), $associationTable->getComponentName(). '.' .$e2[1], Doctrine_Relation::ONE_AGGREGATE, $this->getIdentifier()); + $associationTable->bind($table->getComponentName(), $associationTable->getComponentName(). '.' .$foreign, Doctrine_Relation::ONE_AGGREGATE, $table->getIdentifier()); // NORMAL MANY-TO-MANY RELATIONSHIP - $this->relations[$e2[0]] = new Doctrine_Relation_ForeignKey($associationTable,$local,$e2[1],Doctrine_Relation::MANY_COMPOSITE, $e2[0]); + $this->relations[$e2[0]] = new Doctrine_Relation_ForeignKey($associationTable, $local, $e2[1], Doctrine_Relation::MANY_COMPOSITE, $e2[0]); $relation = new Doctrine_Relation_Association($table, $associationTable, $e2[1], $foreign, $type, $alias); } diff --git a/tests/QueryLimitTestCase.php b/tests/QueryLimitTestCase.php index 6fa275ac4..5ea2a4428 100644 --- a/tests/QueryLimitTestCase.php +++ b/tests/QueryLimitTestCase.php @@ -83,7 +83,7 @@ class Doctrine_Query_Limit_TestCase extends Doctrine_UnitTestCase { public function testLimitWithOneToManyInnerJoin() { - $this->query->from("User(id):Phonenumber"); + $this->query->select('u.id')->from("User u INNER JOIN u.Phonenumber"); $this->query->limit(5); @@ -177,7 +177,8 @@ class Doctrine_Query_Limit_TestCase extends Doctrine_UnitTestCase { $q = new Doctrine_Query(); - $q->from("User")->where("User.Group.id = ?")->orderby("User.id DESC")->limit(5); + $q->from("User")->where("User.Group.id = ?")->orderby("User.id ASC")->limit(5); + $users = $q->execute(array($user->Group[1]->id)); $this->assertEqual($users->count(), 3); @@ -222,7 +223,7 @@ class Doctrine_Query_Limit_TestCase extends Doctrine_UnitTestCase { $photos = $q->execute(array(1)); $this->assertEqual($photos->count(), 3); $this->assertEqual($q->getQuery(), - "SELECT photo.id AS photo__id, photo.name AS photo__name FROM photo LEFT JOIN phototag ON photo.id = phototag.photo_id LEFT JOIN tag ON tag.id = phototag.tag_id WHERE photo.id IN (SELECT DISTINCT photo.id FROM photo LEFT JOIN phototag ON photo.id = phototag.photo_id LEFT JOIN tag ON tag.id = phototag.tag_id WHERE tag.id = ? LIMIT 100) AND tag.id = ? ORDER BY photo.id DESC"); + "SELECT photo.id AS photo__id, photo.name AS photo__name FROM photo LEFT JOIN phototag ON photo.id = phototag.photo_id LEFT JOIN tag ON tag.id = phototag.tag_id WHERE photo.id IN (SELECT DISTINCT photo.id FROM photo LEFT JOIN phototag ON photo.id = phototag.photo_id LEFT JOIN tag ON tag.id = phototag.tag_id WHERE tag.id = ? ORDER BY photo.id DESC LIMIT 100) AND tag.id = ? ORDER BY photo.id DESC"); } } ?> diff --git a/tests/RelationManyToManyTestCase.php b/tests/RelationManyToManyTestCase.php index 399fce662..3a8e3fcee 100644 --- a/tests/RelationManyToManyTestCase.php +++ b/tests/RelationManyToManyTestCase.php @@ -10,9 +10,40 @@ class M2MTest extends Doctrine_Record { $this->hasMany('RTC2 as RTC2', 'JC1.c1_id'); $this->hasMany('RTC3 as RTC3', 'JC2.c1_id'); $this->hasMany('RTC3 as RTC4', 'JC1.c1_id'); + + } +} +class RelationErrorTest extends Doctrine_Record { + public function setTableDefinition() { + $this->hasColumn('name', 'string', 200); + } + public function setUp() { + $this->hasMany('RTCUnknown', 'JC1.c1_id'); + } +} +class M2MTest2 extends Doctrine_Record { + public function setTableDefinition() { + $this->hasColumn('oid', 'integer', 11, array('autoincrement', 'primary')); + $this->hasColumn('name', 'string', 20); + } + public function setUp() { + $this->hasMany('RTC4 as RTC5', 'JC3.c1_id'); + } +} +class RTCUnknown extends Doctrine_Record { + public function setTableDefinition() { + $this->hasColumn('name', 'string', 200); + } + public function setUp() { + $this->hasMany('M2MTest', 'JC2.c2_id'); + } +} +class JC3 extends Doctrine_Record { + public function setTableDefinition() { + $this->hasColumn('c1_id', 'integer'); + $this->hasColumn('c2_id', 'integer'); } } - class JC1 extends Doctrine_Record { public function setTableDefinition() { $this->hasColumn('c1_id', 'integer'); @@ -50,12 +81,105 @@ class RTC3 extends Doctrine_Record { $this->hasMany('M2MTest as RTC4', 'JC1.c2_id'); } } +class RTC4 extends Doctrine_Record { + public function setTableDefinition() { + $this->hasColumn('oid', 'integer', 11, array('autoincrement', 'primary')); + $this->hasColumn('name', 'string', 20); + } + public function setUp() { + $this->hasMany('M2MTest2', 'JC3.c2_id'); + } +} class Doctrine_Relation_ManyToMany_TestCase extends Doctrine_UnitTestCase { public function prepareData() { } public function prepareTables() { parent::prepareTables(); } + + public function testManyToManyRelationWithAliasesAndCustomPKs() { + $component = new M2MTest2(); + + try { + $rel = $component->getTable()->getRelation('RTC5'); + $this->pass(); + } catch(Doctrine_Exception $e) { + $this->fail(); + } + $this->assertTrue($rel instanceof Doctrine_Relation_Association); + + $this->assertTrue($component->RTC5 instanceof Doctrine_Collection); + + try { + $rel = $component->getTable()->getRelation('JC3'); + $this->pass(); + } catch(Doctrine_Exception $e) { + $this->fail(); + } + $this->assertEqual($rel->getLocal(), 'oid'); + } + public function testJoinComponent() { + $component = new JC3(); + + try { + $rel = $component->getTable()->getRelation('M2MTest2'); + $this->pass(); + } catch(Doctrine_Exception $e) { + $this->fail(); + } + $this->assertEqual($rel->getForeign(), 'oid'); + try { + $rel = $component->getTable()->getRelation('RTC4'); + $this->pass(); + } catch(Doctrine_Exception $e) { + $this->fail(); + } + $this->assertEqual($rel->getForeign(), 'oid'); + } + + public function testManyToManyRelationFetchingWithAliasesAndCustomPKs() { + $q = new Doctrine_Query(); + + try { + $q->from('M2MTest2 m LEFT JOIN m.RTC5'); + $this->pass(); + } catch(Doctrine_Exception $e) { + $this->fail(); + } + try { + $q->execute(); + $this->pass(); + } catch(Doctrine_Exception $e) { + $this->fail(); + } + } + + public function testUnknownManyToManyRelation() { + $component = new RelationErrorTest(); + + try { + $rel = $component->getTable()->getRelation('RTCUnknown'); + $this->fail(); + } catch(Doctrine_Table_Exception $e) { + $this->pass(); + } + } + public function testManyToManyRelationFetchingWithAliasesAndCustomPKs2() { + $q = new Doctrine_Query(); + + try { + $q->from('M2MTest2 m INNER JOIN m.JC3'); + $this->pass(); + } catch(Doctrine_Exception $e) { + $this->fail(); + } + try { + $q->execute(); + $this->pass(); + } catch(Doctrine_Exception $e) { + $this->fail(); + } + } public function testManyToManyHasRelationWithAliases4() { $component = new M2MTest(); diff --git a/tests/run.php b/tests/run.php index 483b85fe2..b96639fb2 100644 --- a/tests/run.php +++ b/tests/run.php @@ -66,10 +66,12 @@ $test = new GroupTest('Doctrine Framework Unit Tests'); $test->addTestCase(new Doctrine_DataDict_Pgsql_TestCase()); -$test->addTestCase(new Doctrine_Relation_TestCase()); $test->addTestCase(new Doctrine_Relation_ManyToMany_TestCase()); + +$test->addTestCase(new Doctrine_Relation_TestCase()); + $test->addTestCase(new Doctrine_Record_TestCase()); $test->addTestCase(new Doctrine_Record_State_TestCase()); @@ -148,7 +150,6 @@ $test->addTestCase(new Doctrine_Query_Delete_TestCase()); $test->addTestCase(new Doctrine_Query_Update_TestCase()); - $test->addTestCase(new Doctrine_Query_Limit_TestCase()); $test->addTestCase(new Doctrine_EnumTestCase());