From c5fc122852740e55272c6860cb14a2710fd3253a Mon Sep 17 00:00:00 2001 From: zYne Date: Tue, 26 Jun 2007 13:08:58 +0000 Subject: [PATCH] fixes #364, optimized the bulkDelete for composite primary keys, security check for circular references in cascading saves --- lib/Doctrine/Collection.php | 1 + lib/Doctrine/Connection/UnitOfWork.php | 11 ++++++- lib/Doctrine/Transaction.php | 41 ++++++++++++++++---------- tests/Query/CacheTestCase.php | 3 ++ tests/Ticket364TestCase.php | 37 +++++++++++++++++++++-- tests/classes.php | 18 +++++++---- tests/run.php | 6 +++- 7 files changed, 91 insertions(+), 26 deletions(-) diff --git a/lib/Doctrine/Collection.php b/lib/Doctrine/Collection.php index 661c24dbb..dd93969bb 100644 --- a/lib/Doctrine/Collection.php +++ b/lib/Doctrine/Collection.php @@ -635,6 +635,7 @@ class Doctrine_Collection extends Doctrine_Access implements Countable, Iterator $record->delete($conn); } + Doctrine::debug(true); $conn->commit(); $this->data = array(); diff --git a/lib/Doctrine/Connection/UnitOfWork.php b/lib/Doctrine/Connection/UnitOfWork.php index ef637dc0d..a4e54671c 100644 --- a/lib/Doctrine/Connection/UnitOfWork.php +++ b/lib/Doctrine/Connection/UnitOfWork.php @@ -139,9 +139,16 @@ class Doctrine_Connection_UnitOfWork extends Doctrine_Connection_Module */ public function saveGraph(Doctrine_Record $record) { - $conn = $this->getConnection(); + $conn = $this->getConnection(); + + if ($conn->transaction->isSaved($record)) { + return false; + } + + $conn->transaction->addSaved($record); $conn->beginTransaction(); + $saveLater = $this->saveRelated($record); if ($record->isValid()) { @@ -163,6 +170,8 @@ class Doctrine_Connection_UnitOfWork extends Doctrine_Connection_Module $this->saveAssociations($record); $conn->commit(); + + return true; } /** * saves the given record diff --git a/lib/Doctrine/Transaction.php b/lib/Doctrine/Transaction.php index a29f17e79..2b9a8ad24 100644 --- a/lib/Doctrine/Transaction.php +++ b/lib/Doctrine/Transaction.php @@ -119,7 +119,7 @@ class Doctrine_Transaction extends Doctrine_Connection_Module */ public function isSaved(Doctrine_Record $record) { - return in_array($this->_saved, $record); + return in_array($record, $this->_saved, true); } /** @@ -191,31 +191,38 @@ class Doctrine_Transaction extends Doctrine_Connection_Module */ public function bulkDelete() { + foreach ($this->delete as $name => $deletes) { $record = false; $ids = array(); if (is_array($deletes[count($deletes)-1]->getTable()->getIdentifier())) { - foreach ($deletes as $k => $record) { - $cond = array(); - $ids = $record->obtainIdentifier(); - $query = 'DELETE FROM ' - . $this->conn->quoteIdentifier($record->getTable()->getTableName()) + if (count($deletes) > 0) { + $query = 'DELETE FROM ' + . $this->conn->quoteIdentifier($deletes[0]->getTable()->getTableName()) . ' WHERE '; - - foreach (array_keys($ids) as $id){ - $cond[] = $id . ' = ? '; + + $params = array(); + $cond = array(); + foreach ($deletes as $k => $record) { + $ids = $record->obtainIdentifier(); + $tmp = array(); + foreach (array_keys($ids) as $id){ + $tmp[] = $id . ' = ? '; + } + $params = array_merge($params, array_values($ids)); + $cond[] = '(' . implode(' AND ', $tmp) . ')'; } + $query .= implode(' OR ', $cond); - $query = $query . implode(' AND ', $cond); - $this->conn->execute($query, array_values($ids)); + $this->conn->execute($query, $params); } } else { foreach ($deletes as $k => $record) { $ids[] = $record->getIncremented(); } if ($record instanceof Doctrine_Record) { - $params = substr(str_repeat('?, ', count($ids)),0,-2); + $params = substr(str_repeat('?, ', count($ids)), 0, -2); $query = 'DELETE FROM ' . $this->conn->quoteIdentifier($record->getTable()->getTableName()) @@ -340,6 +347,7 @@ class Doctrine_Transaction extends Doctrine_Connection_Module $listener->postSavepointCommit($event); } else { + if ($this->transactionLevel == 1) { $event = new Doctrine_Event($this, Doctrine_Event::TX_COMMIT); @@ -348,7 +356,7 @@ class Doctrine_Transaction extends Doctrine_Connection_Module if ( ! $event->skipOperation) { try { $this->bulkDelete(); - + } catch(Exception $e) { $this->rollback(); @@ -368,7 +376,7 @@ class Doctrine_Transaction extends Doctrine_Connection_Module $coll->takeSnapshot(); } $this->_collections = array(); - + $this->_saved = array(); $this->conn->getDbh()->commit(); //$this->conn->unitOfWork->reset(); @@ -434,7 +442,8 @@ class Doctrine_Transaction extends Doctrine_Connection_Module throw new Doctrine_Transaction_Exception($e->getMessage()); } } - + $this->_saved = array(); + $listener->postTransactionRollback($event); } @@ -506,7 +515,7 @@ class Doctrine_Transaction extends Doctrine_Connection_Module return $i; } - + /** * setIsolation * diff --git a/tests/Query/CacheTestCase.php b/tests/Query/CacheTestCase.php index fee6d3674..446e55462 100644 --- a/tests/Query/CacheTestCase.php +++ b/tests/Query/CacheTestCase.php @@ -32,6 +32,7 @@ */ class Doctrine_Query_Cache_TestCase extends Doctrine_UnitTestCase { + public function testResultSetCacheAddsResultSetsIntoCache() { $q = new Doctrine_Query(); @@ -51,6 +52,7 @@ class Doctrine_Query_Cache_TestCase extends Doctrine_UnitTestCase $this->assertTrue($coll instanceof Doctrine_Collection); $this->assertEqual($coll->count(), 8); } + public function testResultSetCacheSupportsQueriesWithJoins() { $q = new Doctrine_Query(); @@ -70,6 +72,7 @@ class Doctrine_Query_Cache_TestCase extends Doctrine_UnitTestCase $this->assertTrue($coll instanceof Doctrine_Collection); $this->assertEqual($coll->count(), 8); } + public function testResultSetCacheSupportsPreparedStatements() { $q = new Doctrine_Query(); diff --git a/tests/Ticket364TestCase.php b/tests/Ticket364TestCase.php index 6b485bac3..12495dd9d 100644 --- a/tests/Ticket364TestCase.php +++ b/tests/Ticket364TestCase.php @@ -41,7 +41,23 @@ class Doctrine_Ticket364_TestCase extends Doctrine_UnitTestCase { parent::prepareTables(); } - + + public function testMultiplePrimaryKeys() + { + $r = new Doctrine_Collection('NestReference'); + $r[0]->parent_id = 1; + $r[0]->child_id = 2; + $r[1]->parent_id = 2; + $r[1]->child_id = 3; + $r->save(); + + $r->delete(); + $this->conn->clear(); + $q = new Doctrine_Query(); + $coll = $q->from('NestReference')->execute(); + $this->assertEqual(count($coll), 0); + } + public function testCircularNonEqualSelfReferencingRelationSaving() { $n1 = new NestTest(); $n1->set('name', 'node1'); @@ -54,6 +70,18 @@ class Doctrine_Ticket364_TestCase extends Doctrine_UnitTestCase { $n1->save(); $n2->get('Children')->add($n1); $n2->save(); + + $q = new Doctrine_Query(); + $coll = $q->from('NestReference')->execute(); + + $this->assertEqual(count($coll), 2); + + $coll->delete(); + $this->conn->clear(); + + $q = new Doctrine_Query(); + $coll = $q->from('NestReference')->execute(); + $this->assertEqual(count($coll), 0); } public function testCircularEqualSelfReferencingRelationSaving() { @@ -68,6 +96,11 @@ class Doctrine_Ticket364_TestCase extends Doctrine_UnitTestCase { $n1->save(); $n2->get('Relatives')->add($n1); $n2->save(); + + $q = new Doctrine_Query(); + $coll = $q->from('NestReference')->execute(array(), Doctrine::FETCH_ARRAY); + + $this->assertEqual(count($coll), 2); } -} \ No newline at end of file +} diff --git a/tests/classes.php b/tests/classes.php index 36382a85a..fbe90fe0b 100644 --- a/tests/classes.php +++ b/tests/classes.php @@ -4,7 +4,7 @@ class Entity extends Doctrine_Record public function setUp() { $this->ownsOne('Email', array('local' => 'email_id')); - $this->ownsMany('Phonenumber', array('foreign' => 'entity_id')); + $this->hasMany('Phonenumber', array('local' => 'id', 'foreign' => 'entity_id')); $this->ownsOne('Account', array('foreign' => 'entity_id')); $this->hasMany('Entity', array('local' => 'entity1', 'refClass' => 'EntityReference', @@ -167,7 +167,9 @@ class Phonenumber extends Doctrine_Record } public function setUp() { - $this->hasOne('Entity', array('local' => 'entity_id')); + $this->hasOne('Entity', array('local' => 'entity_id', + 'foreign' => 'id', + 'onDelete' => 'CASCADE')); } } @@ -627,8 +629,10 @@ class PackageVersion extends Doctrine_Record { $this->hasMany('PackageVersionNotes as Note', 'PackageVersionNotes.package_version_id'); } } -class PackageVersionNotes extends Doctrine_Record { - public function setTableDefinition() { +class PackageVersionNotes extends Doctrine_Record +{ + public function setTableDefinition() + { $this->hasColumn('package_version_id', 'integer'); $this->hasColumn('description', 'string', 255); } @@ -639,7 +643,8 @@ class PackageVersionNotes extends Doctrine_Record { } class NestTest extends Doctrine_Record { - public function setTableDefinition() { + public function setTableDefinition() + { $this->hasColumn('name', 'string'); } public function setUp() @@ -659,7 +664,8 @@ class NestTest extends Doctrine_Record } class NestReference extends Doctrine_Record { - public function setTableDefinition() { + public function setTableDefinition() + { $this->hasColumn('parent_id', 'integer', 4, 'primary'); $this->hasColumn('child_id', 'integer', 4, 'primary'); } diff --git a/tests/run.php b/tests/run.php index d0b6f1f13..c12b9e4a9 100644 --- a/tests/run.php +++ b/tests/run.php @@ -72,7 +72,7 @@ $test = new GroupTest('Doctrine Framework Unit Tests'); $test->addTestCase(new Doctrine_Ticket330_TestCase()); */ - +/** */ // Connection drivers (not yet fully tested) $test->addTestCase(new Doctrine_Connection_Pgsql_TestCase()); $test->addTestCase(new Doctrine_Connection_Oracle_TestCase()); @@ -170,6 +170,9 @@ $test->addTestCase(new Doctrine_Relation_TestCase()); //$test->addTestCase(new Doctrine_Relation_Access_TestCase()); //$test->addTestCase(new Doctrine_Relation_ManyToMany_TestCase()); + +$test->addTestCase(new Doctrine_Relation_OneToMany_TestCase()); + $test->addTestCase(new Doctrine_Relation_Nest_TestCase()); $test->addTestCase(new Doctrine_Relation_OneToOne_TestCase()); @@ -298,6 +301,7 @@ $test->addTestCase(new Doctrine_Query_MultipleAggregateValue_TestCase()); $test->addTestCase(new Doctrine_Query_TestCase()); +$test->addTestCase(new Doctrine_Ticket364_TestCase()); //$test->addTestCase(new Doctrine_IntegrityAction_TestCase()); //$test->addTestCase(new Doctrine_AuditLog_TestCase());