From 50f3435e846105e7d4ae59c8cd7760498af7e17f Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Thu, 29 Apr 2010 22:15:36 -0300 Subject: [PATCH 01/17] Optimized Query AST resultant of the parsing process phase 1 --- lib/Doctrine/ORM/Query/Parser.php | 62 +++++- lib/Doctrine/ORM/Query/SqlWalker.php | 189 +++++++++++------- lib/Doctrine/ORM/Query/TreeWalker.php | 16 ++ lib/Doctrine/ORM/Query/TreeWalkerAdapter.php | 16 ++ lib/Doctrine/ORM/Query/TreeWalkerChain.php | 26 +++ .../ORM/Functional/CustomTreeWalkersTest.php | 72 +++++-- .../ORM/Query/SelectSqlGenerationTest.php | 6 +- 7 files changed, 284 insertions(+), 103 deletions(-) diff --git a/lib/Doctrine/ORM/Query/Parser.php b/lib/Doctrine/ORM/Query/Parser.php index b15248689..241a28751 100644 --- a/lib/Doctrine/ORM/Query/Parser.php +++ b/lib/Doctrine/ORM/Query/Parser.php @@ -267,11 +267,11 @@ class Parser { $AST = $this->getAST(); - if ($customWalkers = $this->_query->getHint(Query::HINT_CUSTOM_TREE_WALKERS)) { + if (($customWalkers = $this->_query->getHint(Query::HINT_CUSTOM_TREE_WALKERS)) !== false) { $this->_customTreeWalkers = $customWalkers; } - if ($customOutputWalker = $this->_query->getHint(Query::HINT_CUSTOM_OUTPUT_WALKER)) { + if (($customOutputWalker = $this->_query->getHint(Query::HINT_CUSTOM_OUTPUT_WALKER)) !== false) { $this->_customOutputWalker = $customOutputWalker; } @@ -1786,6 +1786,12 @@ class Parser $conditionalTerms[] = $this->ConditionalTerm(); } + // Phase 1 AST optimization: Prevent AST\ConditionalExpression + // if only one AST\ConditionalTerm is defined + if (count($conditionalTerms) == 1) { + return $conditionalTerms[0]; + } + return new AST\ConditionalExpression($conditionalTerms); } @@ -1804,6 +1810,12 @@ class Parser $conditionalFactors[] = $this->ConditionalFactor(); } + // Phase 1 AST optimization: Prevent AST\ConditionalTerm + // if only one AST\ConditionalFactor is defined + if (count($conditionalFactors) == 1) { + return $conditionalFactors[0]; + } + return new AST\ConditionalTerm($conditionalFactors); } @@ -1820,11 +1832,19 @@ class Parser $this->match(Lexer::T_NOT); $not = true; } + + $conditionalPrimary = $this->ConditionalPrimary(); - $condFactor = new AST\ConditionalFactor($this->ConditionalPrimary()); - $condFactor->not = $not; + // Phase 1 AST optimization: Prevent AST\ConditionalFactor + // if only one AST\ConditionalPrimary is defined + if ( ! $not) { + return $conditionalPrimary; + } - return $condFactor; + $conditionalFactor = new AST\ConditionalFactor($conditionalPrimary); + $conditionalFactor->not = $not; + + return $conditionalFactor; } /** @@ -2104,6 +2124,12 @@ class Parser $terms[] = $this->ArithmeticTerm(); } + // Phase 1 AST optimization: Prevent AST\SimpleArithmeticExpression + // if only one AST\ArithmeticTerm is defined + if (count($terms) == 1) { + return $terms[0]; + } + return new AST\SimpleArithmeticExpression($terms); } @@ -2124,6 +2150,12 @@ class Parser $factors[] = $this->ArithmeticFactor(); } + // Phase 1 AST optimization: Prevent AST\ArithmeticTerm + // if only one AST\ArithmeticFactor is defined + if (count($factors) == 1) { + return $factors[0]; + } + return new AST\ArithmeticTerm($factors); } @@ -2134,14 +2166,22 @@ class Parser */ public function ArithmeticFactor() { - $sign = null; + $sign = null; - if (($isPlus = $this->_lexer->isNextToken(Lexer::T_PLUS)) || $this->_lexer->isNextToken(Lexer::T_MINUS)) { - $this->match(($isPlus) ? Lexer::T_PLUS : Lexer::T_MINUS); - $sign = $isPlus; - } + if (($isPlus = $this->_lexer->isNextToken(Lexer::T_PLUS)) || $this->_lexer->isNextToken(Lexer::T_MINUS)) { + $this->match(($isPlus) ? Lexer::T_PLUS : Lexer::T_MINUS); + $sign = $isPlus; + } + + $primary = $this->ArithmeticPrimary(); - return new AST\ArithmeticFactor($this->ArithmeticPrimary(), $sign); + // Phase 1 AST optimization: Prevent AST\ArithmeticFactor + // if only one AST\ArithmeticPrimary is defined + if ($sign === null) { + return $primary; + } + + return new AST\ArithmeticFactor($primary, $sign); } /** diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 78017e634..5aab37925 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -668,9 +668,7 @@ class SqlWalker implements TreeWalker */ public function walkHavingClause($havingClause) { - return ' HAVING ' . implode( - ' OR ', array_map(array($this, 'walkConditionalTerm'), $havingClause->conditionalExpression->conditionalTerms) - ); + return ' HAVING ' . $this->walkConditionalExpression($havingClause->conditionalExpression); } /** @@ -778,10 +776,10 @@ class SqlWalker implements TreeWalker } // Handle WITH clause - if ($join->conditionalExpression !== null) { - $sql .= ' AND (' . implode(' OR ', - array_map(array($this, 'walkConditionalTerm'), $join->conditionalExpression->conditionalTerms) - ). ')'; + if (($condExpr = $join->conditionalExpression) !== null) { + // Phase 2 AST optimization: Skip processment of ConditionalExpression + // if only one ConditionalTerm is defined + $sql .= ' AND (' . $this->walkConditionalExpression($condExpr) . ')'; } $discrSql = $this->_generateDiscriminatorColumnConditionSql($joinedDqlAlias); @@ -790,7 +788,7 @@ class SqlWalker implements TreeWalker $sql .= ' AND ' . $discrSql; } - //FIXME: these should either be nested or all forced to be left joins (DDC-XXX) + // FIXME: these should either be nested or all forced to be left joins (DDC-XXX) if ($targetClass->isInheritanceTypeJoined()) { $sql .= $this->_generateClassTableInheritanceJoins($targetClass, $joinedDqlAlias); } @@ -882,7 +880,12 @@ class SqlWalker implements TreeWalker $columnAlias = $this->_platform->getSQLResultCasing($columnAlias); $this->_rsm->addScalarResult($columnAlias, $resultAlias); } - else if ($expr instanceof AST\SimpleArithmeticExpression) { + else if ( + $expr instanceof AST\SimpleArithmeticExpression || + $expr instanceof AST\ArithmeticTerm || + $expr instanceof AST\ArithmeticFactor || + $expr instanceof AST\ArithmeticPrimary + ) { if ( ! $selectExpression->fieldIdentificationVariable) { $resultAlias = $this->_scalarResultCounter++; } else { @@ -1214,20 +1217,27 @@ class SqlWalker implements TreeWalker */ public function walkWhereClause($whereClause) { - $sql = ' WHERE '; - $condExpr = $whereClause->conditionalExpression; - - $sql .= implode( - ' OR ', array_map(array($this, 'walkConditionalTerm'), $condExpr->conditionalTerms) - ); - $discrSql = $this->_generateDiscriminatorColumnConditionSql($this->_currentRootAlias); + $condSql = $this->walkConditionalExpression($whereClause->conditionalExpression); - if ($discrSql) { - $sql .= ' AND ' . $discrSql; - } + return ' WHERE ' . (( ! $discrSql) ? $condSql : '(' . $condSql . ') AND ' . $discrSql); + } - return $sql; + /** + * Walk down a ConditionalExpression AST node, thereby generating the appropriate SQL. + * + * @param ConditionalExpression + * @return string The SQL. + */ + public function walkConditionalExpression($condExpr) + { + // Phase 2 AST optimization: Skip processment of ConditionalExpression + // if only one ConditionalTerm is defined + return ( ! ($condExpr instanceof AST\ConditionalExpression)) + ? $this->walkConditionalTerm($condExpr) + : implode( + ' OR ', array_map(array($this, 'walkConditionalTerm'), $condExpr->conditionalTerms) + ); } /** @@ -1238,9 +1248,13 @@ class SqlWalker implements TreeWalker */ public function walkConditionalTerm($condTerm) { - return implode( - ' AND ', array_map(array($this, 'walkConditionalFactor'), $condTerm->conditionalFactors) - ); + // Phase 2 AST optimization: Skip processment of ConditionalTerm + // if only one ConditionalFactor is defined + return ( ! ($condTerm instanceof AST\ConditionalTerm)) + ? $this->walkConditionalFactor($condTerm) + : implode( + ' AND ', array_map(array($this, 'walkConditionalFactor'), $condTerm->conditionalFactors) + ); } /** @@ -1251,21 +1265,28 @@ class SqlWalker implements TreeWalker */ public function walkConditionalFactor($factor) { - $sql = ($factor->not) ? 'NOT ' : ''; - - $primary = $factor->conditionalPrimary; + // Phase 2 AST optimization: Skip processment of ConditionalFactor + // if only one ConditionalPrimary is defined + return ( ! ($factor instanceof AST\ConditionalFactor)) + ? $this->walkConditionalPrimary($factor) + : ($factor->not ? 'NOT ' : '') . $this->walkConditionalPrimary($factor->conditionalPrimary); + } + /** + * Walks down a ConditionalPrimary AST node, thereby generating the appropriate SQL. + * + * @param ConditionalPrimary + * @return string The SQL. + */ + public function walkConditionalPrimary($primary) + { if ($primary->isSimpleConditionalExpression()) { - $sql .= $primary->simpleConditionalExpression->dispatch($this); + return $primary->simpleConditionalExpression->dispatch($this); } else if ($primary->isConditionalExpression()) { $condExpr = $primary->conditionalExpression; - $sql .= '(' . implode( - ' OR ', array_map(array($this, 'walkConditionalTerm'), $condExpr->conditionalTerms) - ) . ')'; + return '(' . $this->walkConditionalExpression($condExpr) . ')'; } - - return $sql; } /** @@ -1605,6 +1626,21 @@ class SqlWalker implements TreeWalker : $this->walkSubselect($arithmeticExpr->subselect); } + /** + * Walks down an SimpleArithmeticExpression AST node, thereby generating the appropriate SQL. + * + * @param SimpleArithmeticExpression + * @return string The SQL. + */ + public function walkSimpleArithmeticExpression($simpleArithmeticExpr) + { + return ( ! ($simpleArithmeticExpr instanceof AST\SimpleArithmeticExpression)) + ? $this->walkArithmeticTerm($simpleArithmeticExpr) + : implode( + ' ', array_map(array($this, 'walkArithmeticTerm'), $simpleArithmeticExpr->arithmeticTerms) + ); + } + /** * Walks down an ArithmeticTerm AST node, thereby generating the appropriate SQL. * @@ -1613,11 +1649,55 @@ class SqlWalker implements TreeWalker */ public function walkArithmeticTerm($term) { - if (is_string($term)) return $term; + if (is_string($term)) { + return $term; + } - return implode( - ' ', array_map(array($this, 'walkArithmeticFactor'), $term->arithmeticFactors) - ); + // Phase 2 AST optimization: Skip processment of ArithmeticTerm + // if only one ArithmeticFactor is defined + return ( ! ($term instanceof AST\ArithmeticTerm)) + ? $this->walkArithmeticFactor($term) + : implode( + ' ', array_map(array($this, 'walkArithmeticFactor'), $term->arithmeticFactors) + ); + } + + /** + * Walks down an ArithmeticFactor that represents an AST node, thereby generating the appropriate SQL. + * + * @param mixed + * @return string The SQL. + */ + public function walkArithmeticFactor($factor) + { + if (is_string($factor)) { + return $factor; + } + + // Phase 2 AST optimization: Skip processment of ArithmeticFactor + // if only one ArithmeticPrimary is defined + return ( ! ($factor instanceof AST\ArithmeticFactor)) + ? $this->walkArithmeticPrimary($factor) + : ($factor->isNegativeSigned() ? '-' : ($factor->isPositiveSigned() ? '+' : '')) + . $this->walkArithmeticPrimary($factor->arithmeticPrimary); + } + + /** + * Walks down an ArithmeticPrimary that represents an AST node, thereby generating the appropriate SQL. + * + * @param mixed + * @return string The SQL. + */ + public function walkArithmeticPrimary($primary) + { + if ($primary instanceof AST\SimpleArithmeticExpression) { + return '(' . $this->walkSimpleArithmeticExpression($primary) . ')'; + } else if ($primary instanceof AST\Node) { + return $primary->dispatch($this); + } + + // We need to deal with IdentificationVariable here + return ''; } /** @@ -1632,41 +1712,4 @@ class SqlWalker implements TreeWalker ? $this->_conn->quote($stringPrimary) : $stringPrimary->dispatch($this); } - - /** - * Walks down an ArithmeticFactor that represents an AST node, thereby generating the appropriate SQL. - * - * @param mixed - * @return string The SQL. - */ - public function walkArithmeticFactor($factor) - { - if (is_string($factor)) return $factor; - - $sql = ($factor->isNegativeSigned() ? '-' : ($factor->isPositiveSigned() ? '+' : '')); - $primary = $factor->arithmeticPrimary; - - if ($primary instanceof AST\SimpleArithmeticExpression) { - $sql .= '(' . $this->walkSimpleArithmeticExpression($primary) . ')'; - } else if ($primary instanceof AST\Node) { - $sql .= $primary->dispatch($this); - } else if (is_string($primary)) { - // We need to deal with IdentificationVariable here - } - - return $sql; - } - - /** - * Walks down an SimpleArithmeticExpression AST node, thereby generating the appropriate SQL. - * - * @param SimpleArithmeticExpression - * @return string The SQL. - */ - public function walkSimpleArithmeticExpression($simpleArithmeticExpr) - { - return implode( - ' ', array_map(array($this, 'walkArithmeticTerm'), $simpleArithmeticExpr->arithmeticTerms) - ); - } } diff --git a/lib/Doctrine/ORM/Query/TreeWalker.php b/lib/Doctrine/ORM/Query/TreeWalker.php index b76b77258..1654f2a1c 100644 --- a/lib/Doctrine/ORM/Query/TreeWalker.php +++ b/lib/Doctrine/ORM/Query/TreeWalker.php @@ -218,6 +218,14 @@ interface TreeWalker */ function walkWhereClause($whereClause); + /** + * Walks down a ConditionalExpression AST node, thereby generating the appropriate SQL. + * + * @param ConditionalExpression + * @return string The SQL. + */ + function walkConditionalExpression($condExpr); + /** * Walks down a ConditionalTerm AST node, thereby generating the appropriate SQL. * @@ -234,6 +242,14 @@ interface TreeWalker */ function walkConditionalFactor($factor); + /** + * Walks down a ConditionalPrimary AST node, thereby generating the appropriate SQL. + * + * @param ConditionalPrimary + * @return string The SQL. + */ + function walkConditionalPrimary($primary); + /** * Walks down an ExistsExpression AST node, thereby generating the appropriate SQL. * diff --git a/lib/Doctrine/ORM/Query/TreeWalkerAdapter.php b/lib/Doctrine/ORM/Query/TreeWalkerAdapter.php index 669409630..7f5f33f3c 100644 --- a/lib/Doctrine/ORM/Query/TreeWalkerAdapter.php +++ b/lib/Doctrine/ORM/Query/TreeWalkerAdapter.php @@ -252,6 +252,14 @@ abstract class TreeWalkerAdapter implements TreeWalker */ public function walkWhereClause($whereClause) {} + /** + * Walks down a ConditionalExpression AST node, thereby generating the appropriate SQL. + * + * @param ConditionalExpression + * @return string The SQL. + */ + public function walkConditionalExpression($condExpr) {} + /** * Walks down a ConditionalTerm AST node, thereby generating the appropriate SQL. * @@ -268,6 +276,14 @@ abstract class TreeWalkerAdapter implements TreeWalker */ public function walkConditionalFactor($factor) {} + /** + * Walks down a ConditionalPrimary AST node, thereby generating the appropriate SQL. + * + * @param ConditionalPrimary + * @return string The SQL. + */ + public function walkConditionalPrimary($primary) {} + /** * Walks down an ExistsExpression AST node, thereby generating the appropriate SQL. * diff --git a/lib/Doctrine/ORM/Query/TreeWalkerChain.php b/lib/Doctrine/ORM/Query/TreeWalkerChain.php index 0a83939cf..28ff54b95 100644 --- a/lib/Doctrine/ORM/Query/TreeWalkerChain.php +++ b/lib/Doctrine/ORM/Query/TreeWalkerChain.php @@ -355,6 +355,19 @@ class TreeWalkerChain implements TreeWalker } } + /** + * Walks down a ConditionalExpression AST node, thereby generating the appropriate SQL. + * + * @param ConditionalExpression + * @return string The SQL. + */ + public function walkConditionalExpression($condExpr) + { + foreach ($this->_walkers as $walker) { + $walker->walkConditionalExpression($condExpr); + } + } + /** * Walks down a ConditionalTerm AST node, thereby generating the appropriate SQL. * @@ -381,6 +394,19 @@ class TreeWalkerChain implements TreeWalker } } + /** + * Walks down a ConditionalPrimary AST node, thereby generating the appropriate SQL. + * + * @param ConditionalPrimary + * @return string The SQL. + */ + public function walkConditionalPrimary($condPrimary) + { + foreach ($this->_walkers as $walker) { + $walker->walkConditionalPrimary($condPrimary); + } + } + /** * Walks down an ExistsExpression AST node, thereby generating the appropriate SQL. * diff --git a/tests/Doctrine/Tests/ORM/Functional/CustomTreeWalkersTest.php b/tests/Doctrine/Tests/ORM/Functional/CustomTreeWalkersTest.php index 817a79eb3..d7c7a4259 100644 --- a/tests/Doctrine/Tests/ORM/Functional/CustomTreeWalkersTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/CustomTreeWalkersTest.php @@ -39,20 +39,43 @@ class CustomTreeWalkersTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->useModelSet('cms'); parent::setUp(); } - + + public function assertSqlGeneration($dqlToBeTested, $sqlToBeConfirmed) + { + try { + $query = $this->_em->createQuery($dqlToBeTested); + $query->setHint(Query::HINT_CUSTOM_TREE_WALKERS, array('Doctrine\Tests\ORM\Functional\CustomTreeWalker')) + ->useQueryCache(false); + + parent::assertEquals($sqlToBeConfirmed, $query->getSql()); + $query->free(); + } catch (\Exception $e) { + $this->fail($e->getMessage()); + } + } + public function testSupportsQueriesWithoutWhere() { - - $q = $this->_em->createQuery('select u from Doctrine\Tests\Models\CMS\CmsUser u where u.name = :name or u.name = :otherName'); - $q->setHint(Query::HINT_CUSTOM_TREE_WALKERS, array('Doctrine\Tests\ORM\Functional\CustomTreeWalker')); - - $this->assertEquals("SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE (c0_.name = ? OR c0_.name = ?) AND c0_.id = 1", $q->getSql()); - - $q->setDql('select u from Doctrine\Tests\Models\CMS\CmsUser u'); - $this->assertEquals("SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE c0_.id = 1", $q->getSql()); - - $q->setDql('select u from Doctrine\Tests\Models\CMS\CmsUser u where u.name = :name'); - $this->assertEquals("SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE c0_.name = ? AND c0_.id = 1", $q->getSql()); + $this->assertSqlGeneration( + 'select u from Doctrine\Tests\Models\CMS\CmsUser u where u.name = :name or u.name = :otherName', + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE (c0_.name = ? OR c0_.name = ?) AND c0_.id = 1" + ); + } + + public function testSupportsQueriesWithSimpleConditionalExpressions() + { + $this->assertSqlGeneration( + 'select u from Doctrine\Tests\Models\CMS\CmsUser u where u.name = :name', + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE c0_.name = ? AND c0_.id = 1" + ); + } + + public function testSupportsQueriesWithMultipleConditionalExpressions() + { + $this->assertSqlGeneration( + 'select u from Doctrine\Tests\Models\CMS\CmsUser u', + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE c0_.id = 1" + ); } } @@ -62,6 +85,7 @@ class CustomTreeWalker extends Query\TreeWalkerAdapter { // Get the DQL aliases of all the classes we want to modify $dqlAliases = array(); + foreach ($this->_getQueryComponents() as $dqlAlias => $comp) { // Hard-coded check just for demonstration: We want to modify the query if // it involves the CmsUser class. @@ -84,10 +108,19 @@ class CustomTreeWalker extends Query\TreeWalkerAdapter $factors[] = $factor; } - if ($selectStatement->whereClause !== null) { + if (($whereClause = $selectStatement->whereClause) !== null) { // There is already a WHERE clause, so append the conditions - - $existingTerms = $selectStatement->whereClause->conditionalExpression->conditionalTerms; + $condExpr = $whereClause->conditionalExpression; + + // Since Phase 1 AST optimizations were included, we need to re-add the ConditionalExpression + if ( ! ($condExpr instanceof Query\AST\ConditionalExpression)) { + $condExpr = new Query\AST\ConditionalExpression(array($condExpr)); + + $whereClause->conditionalExpression = $condExpr; + } + + $existingTerms = $whereClause->conditionalExpression->conditionalTerms; + if (count($existingTerms) > 1) { // More than one term, so we need to wrap all these terms in a single root term // i.e: "WHERE u.name = :foo or u.other = :bar" => "WHERE (u.name = :foo or u.other = :bar) AND " @@ -100,8 +133,15 @@ class CustomTreeWalker extends Query\TreeWalkerAdapter $selectStatement->whereClause->conditionalExpression->conditionalTerms = array($term); } else { // Just one term so we can simply append our factors to that term - $singleTerm = $selectStatement->whereClause->conditionalExpression->conditionalTerms[0]; + + // Since Phase 1 AST optimizations were included, we need to re-add the ConditionalExpression + if ( ! ($singleTerm instanceof Query\AST\ConditionalTerm)) { + $singleTerm = new Query\AST\ConditionalTerm(array($singleTerm)); + + $selectStatement->whereClause->conditionalExpression->conditionalTerms[0] = $singleTerm; + } + $singleTerm->conditionalFactors = array_merge($singleTerm->conditionalFactors, $factors); $selectStatement->whereClause->conditionalExpression->conditionalTerms = array($singleTerm); } diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index a85873a92..716182ac1 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -177,7 +177,7 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase { $this->assertSqlGeneration( 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE ((u.id + 5000) * u.id + 3) < 10000000', - 'SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE ((c0_.id + 5000) * c0_.id + 3) < 10000000' + 'SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE (c0_.id + 5000) * c0_.id + 3 < 10000000' ); } @@ -456,7 +456,7 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase { $this->assertSqlGeneration( "select u from Doctrine\Tests\Models\CMS\CmsUser u where u.id > 10 and u.id < 42 and ((u.id * 2) > 5)", - "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE c0_.id > 10 AND c0_.id < 42 AND ((c0_.id * 2) > 5)" + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE c0_.id > 10 AND c0_.id < 42 AND (c0_.id * 2 > 5)" ); } @@ -464,7 +464,7 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase { $this->assertSqlGeneration( "select u from Doctrine\Tests\Models\CMS\CmsUser u where (u.id > 10) and (u.id < 42 and ((u.id * 2) > 5)) or u.id <> 42", - "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE (c0_.id > 10) AND (c0_.id < 42 AND ((c0_.id * 2) > 5)) OR c0_.id <> 42" + "SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3 FROM cms_users c0_ WHERE (c0_.id > 10) AND (c0_.id < 42 AND (c0_.id * 2 > 5)) OR c0_.id <> 42" ); } From 6705d9b9ccd51fdfc83e4f68856015ea1cd78a27 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Thu, 29 Apr 2010 22:46:51 -0300 Subject: [PATCH 02/17] Introduced the concept of DBAL\Transaction and ORM\EntityTransaction. --- lib/Doctrine/DBAL/Connection.php | 203 ++-------------- lib/Doctrine/DBAL/Driver/Connection.php | 3 - .../DBAL/Driver/OCI8/OCI8Connection.php | 2 +- lib/Doctrine/DBAL/Driver/PDOConnection.php | 2 +- .../DBAL/Driver/PDOMsSql/Connection.php | 2 +- lib/Doctrine/DBAL/Driver/Transaction.php | 42 ++++ .../DBAL/Platforms/AbstractPlatform.php | 13 +- .../DBAL/Platforms/OraclePlatform.php | 8 +- .../DBAL/Platforms/SqlitePlatform.php | 8 +- lib/Doctrine/DBAL/Transaction.php | 229 ++++++++++++++++++ lib/Doctrine/ORM/EntityManager.php | 24 +- lib/Doctrine/ORM/EntityTransaction.php | 154 ++++++++++++ lib/Doctrine/ORM/UnitOfWork.php | 14 +- .../Doctrine/Tests/DBAL/DriverManagerTest.php | 5 +- .../Tests/DBAL/Functional/AllTests.php | 2 +- .../Tests/DBAL/Functional/ConnectionTest.php | 64 ----- .../Tests/DBAL/Functional/TransactionTest.php | 68 ++++++ .../DBAL/Platforms/MsSqlPlatformTest.php | 8 +- .../DBAL/Platforms/MySqlPlatformTest.php | 8 +- .../DBAL/Platforms/OraclePlatformTest.php | 8 +- .../DBAL/Platforms/PostgreSqlPlatformTest.php | 10 +- .../DBAL/Platforms/SqlitePlatformTest.php | 8 +- tests/Doctrine/Tests/Mocks/ConnectionMock.php | 5 +- 23 files changed, 589 insertions(+), 301 deletions(-) create mode 100644 lib/Doctrine/DBAL/Driver/Transaction.php create mode 100644 lib/Doctrine/DBAL/Transaction.php create mode 100644 lib/Doctrine/ORM/EntityTransaction.php delete mode 100644 tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php create mode 100644 tests/Doctrine/Tests/DBAL/Functional/TransactionTest.php diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php index 92a3480d9..5c807c697 100644 --- a/lib/Doctrine/DBAL/Connection.php +++ b/lib/Doctrine/DBAL/Connection.php @@ -44,26 +44,6 @@ use PDO, Closure, */ class Connection implements DriverConnection { - /** - * Constant for transaction isolation level READ UNCOMMITTED. - */ - const TRANSACTION_READ_UNCOMMITTED = 1; - - /** - * Constant for transaction isolation level READ COMMITTED. - */ - const TRANSACTION_READ_COMMITTED = 2; - - /** - * Constant for transaction isolation level REPEATABLE READ. - */ - const TRANSACTION_REPEATABLE_READ = 3; - - /** - * Constant for transaction isolation level SERIALIZABLE. - */ - const TRANSACTION_SERIALIZABLE = 4; - /** * The wrapped driver connection. * @@ -88,20 +68,6 @@ class Connection implements DriverConnection */ private $_isConnected = false; - /** - * The transaction nesting level. - * - * @var integer - */ - private $_transactionNestingLevel = 0; - - /** - * The currently active transaction isolation level. - * - * @var integer - */ - private $_transactionIsolationLevel; - /** * The parameters used during creation of the Connection instance. * @@ -130,14 +96,14 @@ class Connection implements DriverConnection * @var Doctrine\DBAL\Driver */ protected $_driver; - - /** - * Flag that indicates whether the current transaction is marked for rollback only. - * - * @var boolean - */ - private $_isRollbackOnly = false; + /** + * The DBAL Transaction. + * + * @var Doctrine\DBAL\Transaction + */ + protected $_transaction; + /** * Initializes a new instance of the Connection class. * @@ -168,6 +134,7 @@ class Connection implements DriverConnection $this->_config = $config; $this->_eventManager = $eventManager; + if ( ! isset($params['platform'])) { $this->_platform = $driver->getDatabasePlatform(); } else if ($params['platform'] instanceof Platforms\AbstractPlatform) { @@ -175,7 +142,8 @@ class Connection implements DriverConnection } else { throw DBALException::invalidPlatformSpecified(); } - $this->_transactionIsolationLevel = $this->_platform->getDefaultTransactionIsolationLevel(); + + $this->_transaction = new Transaction($this); } /** @@ -278,6 +246,16 @@ class Connection implements DriverConnection return $this->_platform; } + /** + * Gets the DBAL Transaction instance. + * + * @return Doctrine\DBAL\Transaction + */ + public function getTransaction() + { + return $this->_transaction; + } + /** * Establishes the connection with the database. * @@ -356,16 +334,6 @@ class Connection implements DriverConnection return $this->_isConnected; } - /** - * Checks whether a transaction is currently active. - * - * @return boolean TRUE if a transaction is currently active, FALSE otherwise. - */ - public function isTransactionActive() - { - return $this->_transactionNestingLevel > 0; - } - /** * Executes an SQL DELETE statement on a table. * @@ -400,28 +368,6 @@ class Connection implements DriverConnection $this->_isConnected = false; } - /** - * Sets the transaction isolation level. - * - * @param integer $level The level to set. - */ - public function setTransactionIsolation($level) - { - $this->_transactionIsolationLevel = $level; - - return $this->executeUpdate($this->_platform->getSetTransactionIsolationSQL($level)); - } - - /** - * Gets the currently active transaction isolation level. - * - * @return integer The current transaction isolation level. - */ - public function getTransactionIsolation() - { - return $this->_transactionIsolationLevel; - } - /** * Executes an SQL UPDATE statement on a table. * @@ -585,10 +531,10 @@ class Connection implements DriverConnection * represents a row of the result set. * @return mixed The projected result of the query. */ - public function project($query, array $params = array(), Closure $function) + public function project($query, array $params, Closure $function) { $result = array(); - $stmt = $this->executeQuery($query, $params); + $stmt = $this->executeQuery($query, $params ?: array()); while ($row = $stmt->fetch()) { $result[] = $function($row); @@ -659,16 +605,6 @@ class Connection implements DriverConnection return $this->_conn->exec($statement); } - /** - * Returns the current transaction nesting level. - * - * @return integer The nesting level. A value of 0 means there's no active transaction. - */ - public function getTransactionNestingLevel() - { - return $this->_transactionNestingLevel; - } - /** * Fetch the SQLSTATE associated with the last database operation. * @@ -708,73 +644,6 @@ class Connection implements DriverConnection return $this->_conn->lastInsertId($seqName); } - /** - * Starts a transaction by suspending auto-commit mode. - * - * @return void - */ - public function beginTransaction() - { - $this->connect(); - - if ($this->_transactionNestingLevel == 0) { - $this->_conn->beginTransaction(); - } - - ++$this->_transactionNestingLevel; - } - - /** - * Commits the current transaction. - * - * @return void - * @throws ConnectionException If the commit failed due to no active transaction or - * because the transaction was marked for rollback only. - */ - public function commit() - { - if ($this->_transactionNestingLevel == 0) { - throw ConnectionException::commitFailedNoActiveTransaction(); - } - if ($this->_isRollbackOnly) { - throw ConnectionException::commitFailedRollbackOnly(); - } - - $this->connect(); - - if ($this->_transactionNestingLevel == 1) { - $this->_conn->commit(); - } - - --$this->_transactionNestingLevel; - } - - /** - * Cancel any database changes done during the current transaction. - * - * this method can be listened with onPreTransactionRollback and onTransactionRollback - * eventlistener methods - * - * @throws ConnectionException If the rollback operation failed. - */ - public function rollback() - { - if ($this->_transactionNestingLevel == 0) { - throw ConnectionException::rollbackFailedNoActiveTransaction(); - } - - $this->connect(); - - if ($this->_transactionNestingLevel == 1) { - $this->_transactionNestingLevel = 0; - $this->_conn->rollback(); - $this->_isRollbackOnly = false; - } else { - $this->_isRollbackOnly = true; - --$this->_transactionNestingLevel; - } - } - /** * Gets the wrapped driver connection. * @@ -802,34 +671,6 @@ class Connection implements DriverConnection return $this->_schemaManager; } - /** - * Marks the current transaction so that the only possible - * outcome for the transaction to be rolled back. - * - * @throws ConnectionException If no transaction is active. - */ - public function setRollbackOnly() - { - if ($this->_transactionNestingLevel == 0) { - throw ConnectionException::noActiveTransaction(); - } - $this->_isRollbackOnly = true; - } - - /** - * Check whether the current transaction is marked for rollback only. - * - * @return boolean - * @throws ConnectionException If no transaction is active. - */ - public function getRollbackOnly() - { - if ($this->_transactionNestingLevel == 0) { - throw ConnectionException::noActiveTransaction(); - } - return $this->_isRollbackOnly; - } - /** * Converts a given value to its database representation according to the conversion * rules of a specific DBAL mapping type. diff --git a/lib/Doctrine/DBAL/Driver/Connection.php b/lib/Doctrine/DBAL/Driver/Connection.php index cee11f31a..9ad19a32f 100644 --- a/lib/Doctrine/DBAL/Driver/Connection.php +++ b/lib/Doctrine/DBAL/Driver/Connection.php @@ -36,9 +36,6 @@ interface Connection function quote($input, $type=\PDO::PARAM_STR); function exec($statement); function lastInsertId($name = null); - function beginTransaction(); - function commit(); - function rollBack(); function errorCode(); function errorInfo(); } \ No newline at end of file diff --git a/lib/Doctrine/DBAL/Driver/OCI8/OCI8Connection.php b/lib/Doctrine/DBAL/Driver/OCI8/OCI8Connection.php index 987bf6c0c..cdaa3cbd7 100644 --- a/lib/Doctrine/DBAL/Driver/OCI8/OCI8Connection.php +++ b/lib/Doctrine/DBAL/Driver/OCI8/OCI8Connection.php @@ -26,7 +26,7 @@ namespace Doctrine\DBAL\Driver\OCI8; * * @since 2.0 */ -class OCI8Connection implements \Doctrine\DBAL\Driver\Connection +class OCI8Connection implements \Doctrine\DBAL\Driver\Connection, \Doctrine\DBAL\Driver\Transaction { private $_dbh; diff --git a/lib/Doctrine/DBAL/Driver/PDOConnection.php b/lib/Doctrine/DBAL/Driver/PDOConnection.php index f0068077e..50d6865b3 100644 --- a/lib/Doctrine/DBAL/Driver/PDOConnection.php +++ b/lib/Doctrine/DBAL/Driver/PDOConnection.php @@ -29,7 +29,7 @@ use \PDO; * * @since 2.0 */ -class PDOConnection extends PDO implements Connection +class PDOConnection extends PDO implements Connection, Transaction { public function __construct($dsn, $user = null, $password = null, array $options = null) { diff --git a/lib/Doctrine/DBAL/Driver/PDOMsSql/Connection.php b/lib/Doctrine/DBAL/Driver/PDOMsSql/Connection.php index 1b5fb4efc..aa8dbd662 100644 --- a/lib/Doctrine/DBAL/Driver/PDOMsSql/Connection.php +++ b/lib/Doctrine/DBAL/Driver/PDOMsSql/Connection.php @@ -26,7 +26,7 @@ namespace Doctrine\DBAL\Driver\PDOMsSql; * * @since 2.0 */ -class Connection extends \PDO implements \Doctrine\DBAL\Driver\Connection +class Connection extends \Doctrine\DBAL\Driver\PDOConnection { /** * Performs the rollback. diff --git a/lib/Doctrine/DBAL/Driver/Transaction.php b/lib/Doctrine/DBAL/Driver/Transaction.php new file mode 100644 index 000000000..318556e2a --- /dev/null +++ b/lib/Doctrine/DBAL/Driver/Transaction.php @@ -0,0 +1,42 @@ +. + */ + +namespace Doctrine\DBAL\Driver; + +/** + * Transaction interface. + * Each Driver Connection must implement this interface. + * + * @license http://www.opensource.org/licenses/lgpl-license.php LGPL + * @link www.doctrine-project.org + * @since 2.0 + * @version $Revision$ + * @author Benjamin Eberlei + * @author Guilherme Blanco + * @author Jonathan Wage + * @author Roman Borschel + */ +interface Transaction +{ + function beginTransaction(); + function commit(); + function rollBack(); +} \ No newline at end of file diff --git a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php index 961ca0b7d..88fc848ab 100644 --- a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php @@ -23,6 +23,7 @@ namespace Doctrine\DBAL\Platforms; use Doctrine\DBAL\DBALException, Doctrine\DBAL\Connection, + Doctrine\DBAL\Transaction, Doctrine\DBAL\Types, Doctrine\DBAL\Schema\Table, Doctrine\DBAL\Schema\Index, @@ -1445,13 +1446,13 @@ abstract class AbstractPlatform protected function _getTransactionIsolationLevelSQL($level) { switch ($level) { - case Connection::TRANSACTION_READ_UNCOMMITTED: + case Transaction::READ_UNCOMMITTED: return 'READ UNCOMMITTED'; - case Connection::TRANSACTION_READ_COMMITTED: + case Transaction::READ_COMMITTED: return 'READ COMMITTED'; - case Connection::TRANSACTION_REPEATABLE_READ: + case Transaction::REPEATABLE_READ: return 'REPEATABLE READ'; - case Connection::TRANSACTION_SERIALIZABLE: + case Transaction::SERIALIZABLE: return 'SERIALIZABLE'; default: throw new \InvalidArgumentException('Invalid isolation level:' . $level); @@ -1584,11 +1585,11 @@ abstract class AbstractPlatform * Gets the default transaction isolation level of the platform. * * @return integer The default isolation level. - * @see Doctrine\DBAL\Connection\TRANSACTION_* constants. + * @see Doctrine\DBAL\Transaction constants. */ public function getDefaultTransactionIsolationLevel() { - return Connection::TRANSACTION_READ_COMMITTED; + return Transaction::READ_COMMITTED; } /* supports*() metods */ diff --git a/lib/Doctrine/DBAL/Platforms/OraclePlatform.php b/lib/Doctrine/DBAL/Platforms/OraclePlatform.php index b391045c8..74593edf1 100644 --- a/lib/Doctrine/DBAL/Platforms/OraclePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/OraclePlatform.php @@ -146,12 +146,12 @@ class OraclePlatform extends AbstractPlatform protected function _getTransactionIsolationLevelSQL($level) { switch ($level) { - case \Doctrine\DBAL\Connection::TRANSACTION_READ_UNCOMMITTED: + case \Doctrine\DBAL\Transaction::READ_UNCOMMITTED: return 'READ UNCOMMITTED'; - case \Doctrine\DBAL\Connection::TRANSACTION_READ_COMMITTED: + case \Doctrine\DBAL\Transaction::READ_COMMITTED: return 'READ COMMITTED'; - case \Doctrine\DBAL\Connection::TRANSACTION_REPEATABLE_READ: - case \Doctrine\DBAL\Connection::TRANSACTION_SERIALIZABLE: + case \Doctrine\DBAL\Transaction::REPEATABLE_READ: + case \Doctrine\DBAL\Transaction::SERIALIZABLE: return 'SERIALIZABLE'; default: return parent::_getTransactionIsolationLevelSQL($level); diff --git a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php index a1c2184b9..022b819d0 100644 --- a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php @@ -130,11 +130,11 @@ class SqlitePlatform extends AbstractPlatform protected function _getTransactionIsolationLevelSQL($level) { switch ($level) { - case \Doctrine\DBAL\Connection::TRANSACTION_READ_UNCOMMITTED: + case \Doctrine\DBAL\Transaction::READ_UNCOMMITTED: return 0; - case \Doctrine\DBAL\Connection::TRANSACTION_READ_COMMITTED: - case \Doctrine\DBAL\Connection::TRANSACTION_REPEATABLE_READ: - case \Doctrine\DBAL\Connection::TRANSACTION_SERIALIZABLE: + case \Doctrine\DBAL\Transaction::READ_COMMITTED: + case \Doctrine\DBAL\Transaction::REPEATABLE_READ: + case \Doctrine\DBAL\Transaction::SERIALIZABLE: return 1; default: return parent::_getTransactionIsolationLevelSQL($level); diff --git a/lib/Doctrine/DBAL/Transaction.php b/lib/Doctrine/DBAL/Transaction.php new file mode 100644 index 000000000..3b29f3c79 --- /dev/null +++ b/lib/Doctrine/DBAL/Transaction.php @@ -0,0 +1,229 @@ +. + */ + +namespace Doctrine\DBAL; + +/** + * The Transaction class is the central access point to DBAL Transaction functionality. + * + * @license http://www.opensource.org/licenses/lgpl-license.php LGPL + * @link www.doctrine-project.org + * @since 2.0 + * @version $Revision$ + * @author Benjamin Eberlei + * @author Guilherme Blanco + * @author Jonathan Wage + * @author Roman Borschel + */ +class Transaction +{ + /** + * Constant for transaction isolation level READ UNCOMMITTED. + */ + const READ_UNCOMMITTED = 1; + + /** + * Constant for transaction isolation level READ COMMITTED. + */ + const READ_COMMITTED = 2; + + /** + * Constant for transaction isolation level REPEATABLE READ. + */ + const REPEATABLE_READ = 3; + + /** + * Constant for transaction isolation level SERIALIZABLE. + */ + const SERIALIZABLE = 4; + + /** + * The transaction nesting level. + * + * @var integer + */ + private $_transactionNestingLevel = 0; + + /** + * The currently active transaction isolation level. + * + * @var integer + */ + private $_transactionIsolationLevel; + + /** + * Flag that indicates whether the current transaction is marked for rollback only. + * + * @var boolean + */ + private $_isRollbackOnly = false; + + /** + * Constructor + * + * @param Connection $conn The DBAL Connection + */ + public function __construct(Connection $conn) + { + $this->_conn = $conn; + + $this->_transactionIsolationLevel = $conn->getDatabasePlatform()->getDefaultTransactionIsolationLevel(); + } + + /** + * Checks whether a transaction is currently active. + * + * @return boolean TRUE if a transaction is currently active, FALSE otherwise. + */ + public function isTransactionActive() + { + return $this->_transactionNestingLevel > 0; + } + + /** + * Sets the transaction isolation level. + * + * @param integer $level The level to set. + */ + public function setTransactionIsolation($level) + { + $this->_transactionIsolationLevel = $level; + + return $this->executeUpdate($this->_conn->getDatabasePlatform()->getSetTransactionIsolationSQL($level)); + } + + /** + * Gets the currently active transaction isolation level. + * + * @return integer The current transaction isolation level. + */ + public function getTransactionIsolation() + { + return $this->_transactionIsolationLevel; + } + + /** + * Returns the current transaction nesting level. + * + * @return integer The nesting level. A value of 0 means there's no active transaction. + */ + public function getTransactionNestingLevel() + { + return $this->_transactionNestingLevel; + } + + /** + * Starts a transaction by suspending auto-commit mode. + * + * @return void + */ + public function begin() + { + $conn = $this->_conn->getWrappedConnection(); + + if ($this->_transactionNestingLevel == 0) { + $conn->beginTransaction(); + } + + ++$this->_transactionNestingLevel; + } + + /** + * Commits the current transaction. + * + * @return void + * @throws ConnectionException If the commit failed due to no active transaction or + * because the transaction was marked for rollback only. + */ + public function commit() + { + if ($this->_transactionNestingLevel == 0) { + throw ConnectionException::commitFailedNoActiveTransaction(); + } + + if ($this->_isRollbackOnly) { + throw ConnectionException::commitFailedRollbackOnly(); + } + + $conn = $this->_conn->getWrappedConnection(); + + if ($this->_transactionNestingLevel == 1) { + $conn->commit(); + } + + --$this->_transactionNestingLevel; + } + + /** + * Cancel any database changes done during the current transaction. + * + * this method can be listened with onPreTransactionRollback and onTransactionRollback + * eventlistener methods + * + * @throws ConnectionException If the rollback operation failed. + */ + public function rollback() + { + if ($this->_transactionNestingLevel == 0) { + throw ConnectionException::rollbackFailedNoActiveTransaction(); + } + + if ($this->_transactionNestingLevel == 1) { + $this->_transactionNestingLevel = 0; + $this->_conn->getWrappedConnection()->rollback(); + $this->_isRollbackOnly = false; + } else { + $this->_isRollbackOnly = true; + --$this->_transactionNestingLevel; + } + } + + /** + * Marks the current transaction so that the only possible + * outcome for the transaction to be rolled back. + * + * @throws ConnectionException If no transaction is active. + */ + public function setRollbackOnly() + { + if ($this->_transactionNestingLevel == 0) { + throw ConnectionException::noActiveTransaction(); + } + + $this->_isRollbackOnly = true; + } + + /** + * Check whether the current transaction is marked for rollback only. + * + * @return boolean + * @throws ConnectionException If no transaction is active. + */ + public function getRollbackOnly() + { + if ($this->_transactionNestingLevel == 0) { + throw ConnectionException::noActiveTransaction(); + } + + return $this->_isRollbackOnly; + } +} \ No newline at end of file diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index c4aa9bb55..843682ab0 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -107,6 +107,13 @@ class EntityManager */ private $_closed = false; + /** + * The ORM Entity Transaction. + * + * @var Doctrine\ORM\EntityTransaction + */ + protected $_transaction; + /** * Creates a new EntityManager that operates on the given database connection * and uses the given Configuration and EventManager implementations. @@ -127,6 +134,7 @@ class EntityManager $config->getProxyDir(), $config->getProxyNamespace(), $config->getAutoGenerateProxyClasses()); + $this->_transaction = new EntityTransaction($conn->getTransaction()); } /** @@ -149,6 +157,16 @@ class EntityManager return $this->_metadataFactory; } + /** + * Gets the ORM Transaction instance. + * + * @return Doctrine\ORM\EntityTransaction + */ + public function getTransaction() + { + return $this->_transaction; + } + /** * Gets an ExpressionBuilder used for object-oriented construction of query expressions. * @@ -175,7 +193,7 @@ class EntityManager */ public function beginTransaction() { - $this->_conn->beginTransaction(); + $this->getTransaction()->begin(); } /** @@ -183,7 +201,7 @@ class EntityManager */ public function commit() { - $this->_conn->commit(); + $this->getTransaction()->commit(); } /** @@ -192,7 +210,7 @@ class EntityManager */ public function rollback() { - $this->_conn->rollback(); + $this->getTransaction()->rollback(); $this->close(); } diff --git a/lib/Doctrine/ORM/EntityTransaction.php b/lib/Doctrine/ORM/EntityTransaction.php new file mode 100644 index 000000000..80695ee20 --- /dev/null +++ b/lib/Doctrine/ORM/EntityTransaction.php @@ -0,0 +1,154 @@ +. + */ + +namespace Doctrine\ORM; + +use Doctrine\DBAL\Transaction; + +/** + * The Transaction class is the central access point to ORM Transaction functionality. + * This class acts more as a delegate class to the DBAL Transaction functionality. + * + * @license http://www.opensource.org/licenses/lgpl-license.php LGPL + * @link www.doctrine-project.org + * @since 2.0 + * @version $Revision$ + * @author Benjamin Eberlei + * @author Guilherme Blanco + * @author Jonathan Wage + * @author Roman Borschel + */ +class EntityTransaction +{ + /** + * The wrapped DBAL Transaction. + * + * @var Doctrine\DBAL\Transaction + */ + protected $_wrappedTransaction; + + /** + * Constructor. + * + * @param Transaction $transaction + */ + public function __construct(Transaction $transaction) + { + $this->_wrappedTransaction = $transaction; + } + + /** + * Checks whether a transaction is currently active. + * + * @return boolean TRUE if a transaction is currently active, FALSE otherwise. + */ + public function isTransactionActive() + { + return $this->_wrappedTransaction->isTransactionActive(); + } + + /** + * Sets the transaction isolation level. + * + * @param integer $level The level to set. + */ + public function setTransactionIsolation($level) + { + return $this->_wrappedTransaction->setTransactionIsolation($level); + } + + /** + * Gets the currently active transaction isolation level. + * + * @return integer The current transaction isolation level. + */ + public function getTransactionIsolation() + { + return $this->_wrappedTransaction->getTransactionIsolation(); + } + + /** + * Returns the current transaction nesting level. + * + * @return integer The nesting level. A value of 0 means there's no active transaction. + */ + public function getTransactionNestingLevel() + { + return $this->_wrappedTransaction->getTransactionNestingLevel(); + } + + /** + * Starts a transaction by suspending auto-commit mode. + * + * @return void + */ + public function begin() + { + $this->_wrappedTransaction->begin(); + } + + /** + * Commits the current transaction. + * + * @return void + * @throws Doctrine\DBAL\ConnectionException If the commit failed due to no active transaction or + * because the transaction was marked for rollback only. + */ + public function commit() + { + $this->_wrappedTransaction->commit(); + } + + /** + * Cancel any database changes done during the current transaction. + * + * this method can be listened with onPreTransactionRollback and onTransactionRollback + * eventlistener methods + * + * @throws Doctrine\DBAL\ConnectionException If the rollback operation failed. + */ + public function rollback() + { + $this->_wrappedTransaction->rollback(); + } + + /** + * Marks the current transaction so that the only possible + * outcome for the transaction to be rolled back. + * + * @throws ConnectionException If no transaction is active. + */ + public function setRollbackOnly() + { + $this->_wrappedTransaction->setRollbackOnly(); + } + + /** + * Check whether the current transaction is marked for rollback only. + * + * @return boolean + * @throws ConnectionException If no transaction is active. + */ + public function getRollbackOnly() + { + return $this->_wrappedTransaction->getRollbackOnly(); + } +} \ No newline at end of file diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 46628c904..96cbcc99c 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -281,11 +281,12 @@ class UnitOfWork implements PropertyChangedListener // Now we need a commit order to maintain referential integrity $commitOrder = $this->_getCommitOrder(); + + $tx = $this->_em->getTransaction(); - $conn = $this->_em->getConnection(); + try { + $tx->begin(); - $conn->beginTransaction(); - try { if ($this->_entityInsertions) { foreach ($commitOrder as $class) { $this->_executeInserts($class); @@ -321,11 +322,10 @@ class UnitOfWork implements PropertyChangedListener } } - $conn->commit(); + $tx->commit(); } catch (\Exception $e) { - $conn->setRollbackOnly(); - $conn->rollback(); - $this->_em->close(); + $tx->rollback(); + throw $e; } diff --git a/tests/Doctrine/Tests/DBAL/DriverManagerTest.php b/tests/Doctrine/Tests/DBAL/DriverManagerTest.php index d7d0fb94f..a95c9ad67 100644 --- a/tests/Doctrine/Tests/DBAL/DriverManagerTest.php +++ b/tests/Doctrine/Tests/DBAL/DriverManagerTest.php @@ -56,12 +56,11 @@ class DriverManagerTest extends \Doctrine\Tests\DbalTestCase public function testCustomWrapper() { - $wrapperMock = $this->getMock('\Doctrine\DBAL\Connection', array(), array(), '', false); - $wrapperClass = get_class($wrapperMock); + $wrapperClass = 'Doctrine\Tests\Mocks\ConnectionMock'; $options = array( 'pdo' => new \PDO('sqlite::memory:'), - 'wrapperClass' => $wrapperClass + 'wrapperClass' => $wrapperClass, ); $conn = \Doctrine\DBAL\DriverManager::getConnection($options); diff --git a/tests/Doctrine/Tests/DBAL/Functional/AllTests.php b/tests/Doctrine/Tests/DBAL/Functional/AllTests.php index 31719886b..50825be9e 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/AllTests.php +++ b/tests/Doctrine/Tests/DBAL/Functional/AllTests.php @@ -25,7 +25,7 @@ class AllTests $suite->addTestSuite('Doctrine\Tests\DBAL\Functional\Schema\MySqlSchemaManagerTest'); $suite->addTestSuite('Doctrine\Tests\DBAL\Functional\Schema\PostgreSqlSchemaManagerTest'); $suite->addTestSuite('Doctrine\Tests\DBAL\Functional\Schema\OracleSchemaManagerTest'); - $suite->addTestSuite('Doctrine\Tests\DBAL\Functional\ConnectionTest'); + $suite->addTestSuite('Doctrine\Tests\DBAL\Functional\TransactionTest'); return $suite; } diff --git a/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php deleted file mode 100644 index 8ed7a57d4..000000000 --- a/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php +++ /dev/null @@ -1,64 +0,0 @@ -_conn->beginTransaction(); - $this->assertEquals(1, $this->_conn->getTransactionNestingLevel()); - - try { - $this->_conn->beginTransaction(); - $this->assertEquals(2, $this->_conn->getTransactionNestingLevel()); - throw new \Exception; - $this->_conn->commit(); // never reached - } catch (\Exception $e) { - $this->_conn->rollback(); - $this->assertEquals(1, $this->_conn->getTransactionNestingLevel()); - //no rethrow - } - $this->assertTrue($this->_conn->getRollbackOnly()); - - $this->_conn->commit(); // should throw exception - $this->fail('Transaction commit after failed nested transaction should fail.'); - } catch (ConnectionException $e) { - $this->assertEquals(1, $this->_conn->getTransactionNestingLevel()); - $this->_conn->rollback(); - $this->assertEquals(0, $this->_conn->getTransactionNestingLevel()); - } - } - - public function testTransactionBehavior() - { - try { - $this->_conn->beginTransaction(); - $this->assertEquals(1, $this->_conn->getTransactionNestingLevel()); - - throw new \Exception; - - $this->_conn->commit(); // never reached - } catch (\Exception $e) { - $this->assertEquals(1, $this->_conn->getTransactionNestingLevel()); - $this->_conn->rollback(); - $this->assertEquals(0, $this->_conn->getTransactionNestingLevel()); - } - - try { - $this->_conn->beginTransaction(); - $this->assertEquals(1, $this->_conn->getTransactionNestingLevel()); - $this->_conn->commit(); - } catch (\Exception $e) { - $this->_conn->rollback(); - $this->assertEquals(0, $this->_conn->getTransactionNestingLevel()); - } - } - -} \ No newline at end of file diff --git a/tests/Doctrine/Tests/DBAL/Functional/TransactionTest.php b/tests/Doctrine/Tests/DBAL/Functional/TransactionTest.php new file mode 100644 index 000000000..3663bbaaf --- /dev/null +++ b/tests/Doctrine/Tests/DBAL/Functional/TransactionTest.php @@ -0,0 +1,68 @@ +_conn->getTransaction(); + + try { + $tx->begin(); + $this->assertEquals(1, $tx->getTransactionNestingLevel()); + + try { + $tx->begin(); + $this->assertEquals(2, $tx->getTransactionNestingLevel()); + throw new \Exception; + $tx->commit(); // never reached + } catch (\Exception $e) { + $tx->rollback(); + $this->assertEquals(1, $tx->getTransactionNestingLevel()); + //no rethrow + } + $this->assertTrue($tx->getRollbackOnly()); + + $tx->commit(); // should throw exception + $this->fail('Transaction commit after failed nested transaction should fail.'); + } catch (ConnectionException $e) { + $this->assertEquals(1, $tx->getTransactionNestingLevel()); + $tx->rollback(); + $this->assertEquals(0, $tx->getTransactionNestingLevel()); + } + } + + public function testTransactionBehavior() + { + $tx = $this->_conn->getTransaction(); + + try { + $tx->begin(); + $this->assertEquals(1, $tx->getTransactionNestingLevel()); + + throw new \Exception; + + $tx->commit(); // never reached + } catch (\Exception $e) { + $this->assertEquals(1, $tx->getTransactionNestingLevel()); + $tx->rollback(); + $this->assertEquals(0, $tx->getTransactionNestingLevel()); + } + + try { + $tx->begin(); + $this->assertEquals(1, $tx->getTransactionNestingLevel()); + $tx->commit(); + } catch (\Exception $e) { + $tx->rollback(); + $this->assertEquals(0, $tx->getTransactionNestingLevel()); + } + } + +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/DBAL/Platforms/MsSqlPlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/MsSqlPlatformTest.php index 1302be035..e0691d74b 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/MsSqlPlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/MsSqlPlatformTest.php @@ -44,19 +44,19 @@ class MsSqlPlatformTest extends AbstractPlatformTestCase { $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_UNCOMMITTED) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_UNCOMMITTED) ); $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL READ COMMITTED', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_COMMITTED) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_COMMITTED) ); $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL REPEATABLE READ', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_REPEATABLE_READ) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::REPEATABLE_READ) ); $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL SERIALIZABLE', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_SERIALIZABLE) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::SERIALIZABLE) ); } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/MySqlPlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/MySqlPlatformTest.php index 8dbeac5db..b2c916aec 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/MySqlPlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/MySqlPlatformTest.php @@ -53,20 +53,20 @@ class MySqlPlatformTest extends AbstractPlatformTestCase { $this->assertEquals( 'SET SESSION TRANSACTION ISOLATION LEVEL READ UNCOMMITTED', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_UNCOMMITTED), + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_UNCOMMITTED), '' ); $this->assertEquals( 'SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_COMMITTED) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_COMMITTED) ); $this->assertEquals( 'SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_REPEATABLE_READ) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::REPEATABLE_READ) ); $this->assertEquals( 'SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_SERIALIZABLE) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::SERIALIZABLE) ); } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php index 8fb24abf6..c2b02d50f 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php @@ -55,19 +55,19 @@ class OraclePlatformTest extends AbstractPlatformTestCase { $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_UNCOMMITTED) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_UNCOMMITTED) ); $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL READ COMMITTED', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_COMMITTED) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_COMMITTED) ); $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL SERIALIZABLE', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_REPEATABLE_READ) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::REPEATABLE_READ) ); $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL SERIALIZABLE', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_SERIALIZABLE) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::SERIALIZABLE) ); } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/PostgreSqlPlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/PostgreSqlPlatformTest.php index 92596b021..75a8aabe8 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/PostgreSqlPlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/PostgreSqlPlatformTest.php @@ -4,7 +4,7 @@ namespace Doctrine\Tests\DBAL\Platforms; use Doctrine\DBAL\Platforms\PostgreSqlPlatform; use Doctrine\DBAL\Types\Type; -use Doctrine\DBAL\Connection; +use Doctrine\DBAL\Transaction; require_once __DIR__ . '/../../TestInit.php'; @@ -74,19 +74,19 @@ class PostgreSqlPlatformTest extends AbstractPlatformTestCase { $this->assertEquals( 'SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL READ UNCOMMITTED', - $this->_platform->getSetTransactionIsolationSQL(Connection::TRANSACTION_READ_UNCOMMITTED) + $this->_platform->getSetTransactionIsolationSQL(Transaction::READ_UNCOMMITTED) ); $this->assertEquals( 'SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL READ COMMITTED', - $this->_platform->getSetTransactionIsolationSQL(Connection::TRANSACTION_READ_COMMITTED) + $this->_platform->getSetTransactionIsolationSQL(Transaction::READ_COMMITTED) ); $this->assertEquals( 'SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL REPEATABLE READ', - $this->_platform->getSetTransactionIsolationSQL(Connection::TRANSACTION_REPEATABLE_READ) + $this->_platform->getSetTransactionIsolationSQL(Transaction::REPEATABLE_READ) ); $this->assertEquals( 'SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL SERIALIZABLE', - $this->_platform->getSetTransactionIsolationSQL(Connection::TRANSACTION_SERIALIZABLE) + $this->_platform->getSetTransactionIsolationSQL(Transaction::SERIALIZABLE) ); } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php index 4d09a2e0d..5a56b72bc 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php @@ -36,10 +36,10 @@ class SqlitePlatformTest extends AbstractPlatformTestCase public function testGeneratesTransactionCommands() { - $this->assertEquals('PRAGMA read_uncommitted = 0', $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_UNCOMMITTED)); - $this->assertEquals('PRAGMA read_uncommitted = 1', $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_COMMITTED)); - $this->assertEquals('PRAGMA read_uncommitted = 1', $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_REPEATABLE_READ)); - $this->assertEquals('PRAGMA read_uncommitted = 1', $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_SERIALIZABLE)); + $this->assertEquals('PRAGMA read_uncommitted = 0', $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_UNCOMMITTED)); + $this->assertEquals('PRAGMA read_uncommitted = 1', $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_COMMITTED)); + $this->assertEquals('PRAGMA read_uncommitted = 1', $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::REPEATABLE_READ)); + $this->assertEquals('PRAGMA read_uncommitted = 1', $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::SERIALIZABLE)); } public function testPrefersIdentityColumns() diff --git a/tests/Doctrine/Tests/Mocks/ConnectionMock.php b/tests/Doctrine/Tests/Mocks/ConnectionMock.php index 41f5ebab0..fabecf87a 100644 --- a/tests/Doctrine/Tests/Mocks/ConnectionMock.php +++ b/tests/Doctrine/Tests/Mocks/ConnectionMock.php @@ -11,8 +11,11 @@ class ConnectionMock extends \Doctrine\DBAL\Connection public function __construct(array $params, $driver, $config = null, $eventManager = null) { - parent::__construct($params, $driver, $config, $eventManager); $this->_platformMock = new DatabasePlatformMock(); + + parent::__construct($params, $driver, $config, $eventManager); + + // Override possible assignment of platform to database platform mock $this->_platform = $this->_platformMock; } From e2766ca63690516ff7272e8f251233ecf1609279 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Thu, 29 Apr 2010 22:59:51 -0300 Subject: [PATCH 03/17] Fixed double lookup on some cache instances by removing the contains() call in AnnotationReader. --- lib/Doctrine/Common/Annotations/AnnotationReader.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/Common/Annotations/AnnotationReader.php b/lib/Doctrine/Common/Annotations/AnnotationReader.php index dbbf8dff6..f7d1c3a3b 100644 --- a/lib/Doctrine/Common/Annotations/AnnotationReader.php +++ b/lib/Doctrine/Common/Annotations/AnnotationReader.php @@ -184,7 +184,7 @@ class AnnotationReader // Attempt to grab data from cache if (($data = $this->_cache->fetch($cacheKey)) !== false) { return $data; - } + } $context = 'method ' . $method->getDeclaringClass()->getName() . '::' . $method->getName() . '()'; $annotations = $this->_parser->parse($method->getDocComment(), $context); From ba6ed43afa1d972ab0743f8678140c425b540065 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Thu, 29 Apr 2010 23:02:53 -0300 Subject: [PATCH 04/17] Renamed fetchRow() to fetchAssoc() as marked as a TODO. --- lib/Doctrine/DBAL/Connection.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php index 5c807c697..454b436ae 100644 --- a/lib/Doctrine/DBAL/Connection.php +++ b/lib/Doctrine/DBAL/Connection.php @@ -290,9 +290,8 @@ class Connection implements DriverConnection * @param string $statement The SQL query. * @param array $params The query parameters. * @return array - * @todo Rename: fetchAssoc */ - public function fetchRow($statement, array $params = array()) + public function fetchAssoc($statement, array $params = array()) { return $this->executeQuery($statement, $params)->fetch(PDO::FETCH_ASSOC); } From b12b8b0041f30aed05ff87a16d3c21de135bcfc8 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Thu, 6 May 2010 18:39:19 -0300 Subject: [PATCH 05/17] Revertd partially the support to DBAL\Transaction, it was wrong. Fixed implementation of ORM\EntityTransaction and fixed tests. --- lib/Doctrine/DBAL/Connection.php | 210 ++++++++++++++-- lib/Doctrine/DBAL/Driver/.DS_Store | Bin 0 -> 6148 bytes lib/Doctrine/DBAL/Driver/Connection.php | 3 + .../DBAL/Driver/OCI8/OCI8Connection.php | 2 +- lib/Doctrine/DBAL/Driver/PDOConnection.php | 2 +- lib/Doctrine/DBAL/Driver/Transaction.php | 42 ---- .../DBAL/Platforms/AbstractPlatform.php | 15 +- .../DBAL/Platforms/OraclePlatform.php | 8 +- .../DBAL/Platforms/SqlitePlatform.php | 8 +- lib/Doctrine/DBAL/Transaction.php | 229 ------------------ lib/Doctrine/ORM/EntityManager.php | 9 +- lib/Doctrine/ORM/EntityTransaction.php | 49 ++-- lib/Doctrine/ORM/UnitOfWork.php | 2 +- .../Tests/DBAL/Functional/AllTests.php | 2 +- .../Tests/DBAL/Functional/ConnectionTest.php | 64 +++++ .../Tests/DBAL/Functional/TransactionTest.php | 68 ------ .../DBAL/Platforms/MsSqlPlatformTest.php | 8 +- .../DBAL/Platforms/MySqlPlatformTest.php | 8 +- .../DBAL/Platforms/OraclePlatformTest.php | 8 +- .../DBAL/Platforms/PostgreSqlPlatformTest.php | 9 +- .../DBAL/Platforms/SqlitePlatformTest.php | 20 +- 21 files changed, 337 insertions(+), 429 deletions(-) create mode 100644 lib/Doctrine/DBAL/Driver/.DS_Store delete mode 100644 lib/Doctrine/DBAL/Driver/Transaction.php delete mode 100644 lib/Doctrine/DBAL/Transaction.php create mode 100644 tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php delete mode 100644 tests/Doctrine/Tests/DBAL/Functional/TransactionTest.php diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php index bf1e7b0e4..9ed6292ca 100644 --- a/lib/Doctrine/DBAL/Connection.php +++ b/lib/Doctrine/DBAL/Connection.php @@ -42,6 +42,26 @@ use PDO, Closure, */ class Connection implements DriverConnection { + /** + * Constant for transaction isolation level READ UNCOMMITTED. + */ + const TRANSACTION_READ_UNCOMMITTED = 1; + + /** + * Constant for transaction isolation level READ COMMITTED. + */ + const TRANSACTION_READ_COMMITTED = 2; + + /** + * Constant for transaction isolation level REPEATABLE READ. + */ + const TRANSACTION_REPEATABLE_READ = 3; + + /** + * Constant for transaction isolation level SERIALIZABLE. + */ + const TRANSACTION_SERIALIZABLE = 4; + /** * The wrapped driver connection. * @@ -66,6 +86,20 @@ class Connection implements DriverConnection */ private $_isConnected = false; + /** + * The transaction nesting level. + * + * @var integer + */ + private $_transactionNestingLevel = 0; + + /** + * The currently active transaction isolation level. + * + * @var integer + */ + private $_transactionIsolationLevel; + /** * The parameters used during creation of the Connection instance. * @@ -94,14 +128,14 @@ class Connection implements DriverConnection * @var Doctrine\DBAL\Driver */ protected $_driver; - - /** - * The DBAL Transaction. - * - * @var Doctrine\DBAL\Transaction - */ - protected $_transaction; + /** + * Flag that indicates whether the current transaction is marked for rollback only. + * + * @var boolean + */ + private $_isRollbackOnly = false; + /** * Initializes a new instance of the Connection class. * @@ -132,7 +166,6 @@ class Connection implements DriverConnection $this->_config = $config; $this->_eventManager = $eventManager; - if ( ! isset($params['platform'])) { $this->_platform = $driver->getDatabasePlatform(); } else if ($params['platform'] instanceof Platforms\AbstractPlatform) { @@ -140,8 +173,7 @@ class Connection implements DriverConnection } else { throw DBALException::invalidPlatformSpecified(); } - - $this->_transaction = new Transaction($this); + $this->_transactionIsolationLevel = $this->_platform->getDefaultTransactionIsolationLevel(); } /** @@ -244,16 +276,6 @@ class Connection implements DriverConnection return $this->_platform; } - /** - * Gets the DBAL Transaction instance. - * - * @return Doctrine\DBAL\Transaction - */ - public function getTransaction() - { - return $this->_transaction; - } - /** * Establishes the connection with the database. * @@ -288,8 +310,9 @@ class Connection implements DriverConnection * @param string $statement The SQL query. * @param array $params The query parameters. * @return array + * @todo Rename: fetchAssoc */ - public function fetchAssoc($statement, array $params = array()) + public function fetchRow($statement, array $params = array()) { return $this->executeQuery($statement, $params)->fetch(PDO::FETCH_ASSOC); } @@ -331,6 +354,16 @@ class Connection implements DriverConnection return $this->_isConnected; } + /** + * Checks whether a transaction is currently active. + * + * @return boolean TRUE if a transaction is currently active, FALSE otherwise. + */ + public function isTransactionActive() + { + return $this->_transactionNestingLevel > 0; + } + /** * Executes an SQL DELETE statement on a table. * @@ -365,6 +398,28 @@ class Connection implements DriverConnection $this->_isConnected = false; } + /** + * Sets the transaction isolation level. + * + * @param integer $level The level to set. + */ + public function setTransactionIsolation($level) + { + $this->_transactionIsolationLevel = $level; + + return $this->executeUpdate($this->_platform->getSetTransactionIsolationSQL($level)); + } + + /** + * Gets the currently active transaction isolation level. + * + * @return integer The current transaction isolation level. + */ + public function getTransactionIsolation() + { + return $this->_transactionIsolationLevel; + } + /** * Executes an SQL UPDATE statement on a table. * @@ -528,10 +583,10 @@ class Connection implements DriverConnection * represents a row of the result set. * @return mixed The projected result of the query. */ - public function project($query, array $params, Closure $function) + public function project($query, array $params = array(), Closure $function) { $result = array(); - $stmt = $this->executeQuery($query, $params ?: array()); + $stmt = $this->executeQuery($query, $params); while ($row = $stmt->fetch()) { $result[] = $function($row); @@ -602,6 +657,16 @@ class Connection implements DriverConnection return $this->_conn->exec($statement); } + /** + * Returns the current transaction nesting level. + * + * @return integer The nesting level. A value of 0 means there's no active transaction. + */ + public function getTransactionNestingLevel() + { + return $this->_transactionNestingLevel; + } + /** * Fetch the SQLSTATE associated with the last database operation. * @@ -641,6 +706,73 @@ class Connection implements DriverConnection return $this->_conn->lastInsertId($seqName); } + /** + * Starts a transaction by suspending auto-commit mode. + * + * @return void + */ + public function beginTransaction() + { + $this->connect(); + + if ($this->_transactionNestingLevel == 0) { + $this->_conn->beginTransaction(); + } + + ++$this->_transactionNestingLevel; + } + + /** + * Commits the current transaction. + * + * @return void + * @throws ConnectionException If the commit failed due to no active transaction or + * because the transaction was marked for rollback only. + */ + public function commit() + { + if ($this->_transactionNestingLevel == 0) { + throw ConnectionException::commitFailedNoActiveTransaction(); + } + if ($this->_isRollbackOnly) { + throw ConnectionException::commitFailedRollbackOnly(); + } + + $this->connect(); + + if ($this->_transactionNestingLevel == 1) { + $this->_conn->commit(); + } + + --$this->_transactionNestingLevel; + } + + /** + * Cancel any database changes done during the current transaction. + * + * this method can be listened with onPreTransactionRollback and onTransactionRollback + * eventlistener methods + * + * @throws ConnectionException If the rollback operation failed. + */ + public function rollback() + { + if ($this->_transactionNestingLevel == 0) { + throw ConnectionException::rollbackFailedNoActiveTransaction(); + } + + $this->connect(); + + if ($this->_transactionNestingLevel == 1) { + $this->_transactionNestingLevel = 0; + $this->_conn->rollback(); + $this->_isRollbackOnly = false; + } else { + $this->_isRollbackOnly = true; + --$this->_transactionNestingLevel; + } + } + /** * Gets the wrapped driver connection. * @@ -657,7 +789,7 @@ class Connection implements DriverConnection * Gets the SchemaManager that can be used to inspect or change the * database schema through the connection. * - * @return Doctrine\DBAL\Schema\AbstractSchemaManager + * @return Doctrine\DBAL\Schema\SchemaManager */ public function getSchemaManager() { @@ -668,6 +800,34 @@ class Connection implements DriverConnection return $this->_schemaManager; } + /** + * Marks the current transaction so that the only possible + * outcome for the transaction to be rolled back. + * + * @throws ConnectionException If no transaction is active. + */ + public function setRollbackOnly() + { + if ($this->_transactionNestingLevel == 0) { + throw ConnectionException::noActiveTransaction(); + } + $this->_isRollbackOnly = true; + } + + /** + * Check whether the current transaction is marked for rollback only. + * + * @return boolean + * @throws ConnectionException If no transaction is active. + */ + public function getRollbackOnly() + { + if ($this->_transactionNestingLevel == 0) { + throw ConnectionException::noActiveTransaction(); + } + return $this->_isRollbackOnly; + } + /** * Converts a given value to its database representation according to the conversion * rules of a specific DBAL mapping type. @@ -751,4 +911,4 @@ class Connection implements DriverConnection } } } -} +} \ No newline at end of file diff --git a/lib/Doctrine/DBAL/Driver/.DS_Store b/lib/Doctrine/DBAL/Driver/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..5008ddfcf53c02e82d7eee2e57c38e5672ef89f6 GIT binary patch literal 6148 zcmeH~Jr2S!425mzP>H1@V-^m;4Wg<&0T*E43hX&L&p$$qDprKhvt+--jT7}7np#A3 zem<@ulZcFPQ@L2!n>{z**++&mCkOWA81W14cNZlEfg7;MkzE(HCqgga^y>{tEnwC%0;vJ&^%eQ zLs35+`xjp>T0. - */ - -namespace Doctrine\DBAL\Driver; - -/** - * Transaction interface. - * Each Driver Connection must implement this interface. - * - * @license http://www.opensource.org/licenses/lgpl-license.php LGPL - * @link www.doctrine-project.org - * @since 2.0 - * @version $Revision$ - * @author Benjamin Eberlei - * @author Guilherme Blanco - * @author Jonathan Wage - * @author Roman Borschel - */ -interface Transaction -{ - function beginTransaction(); - function commit(); - function rollBack(); -} \ No newline at end of file diff --git a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php index ba9c047d1..87b26f3c6 100644 --- a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php @@ -23,7 +23,6 @@ namespace Doctrine\DBAL\Platforms; use Doctrine\DBAL\DBALException, Doctrine\DBAL\Connection, - Doctrine\DBAL\Transaction, Doctrine\DBAL\Types, Doctrine\DBAL\Schema\Table, Doctrine\DBAL\Schema\Index, @@ -1457,13 +1456,13 @@ abstract class AbstractPlatform protected function _getTransactionIsolationLevelSQL($level) { switch ($level) { - case Transaction::READ_UNCOMMITTED: + case Connection::TRANSACTION_READ_UNCOMMITTED: return 'READ UNCOMMITTED'; - case Transaction::READ_COMMITTED: + case Connection::TRANSACTION_READ_COMMITTED: return 'READ COMMITTED'; - case Transaction::REPEATABLE_READ: + case Connection::TRANSACTION_REPEATABLE_READ: return 'REPEATABLE READ'; - case Transaction::SERIALIZABLE: + case Connection::TRANSACTION_SERIALIZABLE: return 'SERIALIZABLE'; default: throw new \InvalidArgumentException('Invalid isolation level:' . $level); @@ -1596,11 +1595,11 @@ abstract class AbstractPlatform * Gets the default transaction isolation level of the platform. * * @return integer The default isolation level. - * @see Doctrine\DBAL\Transaction constants. + * @see Doctrine\DBAL\Connection\TRANSACTION_* constants. */ public function getDefaultTransactionIsolationLevel() { - return Transaction::READ_COMMITTED; + return Connection::TRANSACTION_READ_COMMITTED; } /* supports*() metods */ @@ -1865,4 +1864,4 @@ abstract class AbstractPlatform { return 'TRUNCATE '.$tableName; } -} +} \ No newline at end of file diff --git a/lib/Doctrine/DBAL/Platforms/OraclePlatform.php b/lib/Doctrine/DBAL/Platforms/OraclePlatform.php index 74593edf1..b391045c8 100644 --- a/lib/Doctrine/DBAL/Platforms/OraclePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/OraclePlatform.php @@ -146,12 +146,12 @@ class OraclePlatform extends AbstractPlatform protected function _getTransactionIsolationLevelSQL($level) { switch ($level) { - case \Doctrine\DBAL\Transaction::READ_UNCOMMITTED: + case \Doctrine\DBAL\Connection::TRANSACTION_READ_UNCOMMITTED: return 'READ UNCOMMITTED'; - case \Doctrine\DBAL\Transaction::READ_COMMITTED: + case \Doctrine\DBAL\Connection::TRANSACTION_READ_COMMITTED: return 'READ COMMITTED'; - case \Doctrine\DBAL\Transaction::REPEATABLE_READ: - case \Doctrine\DBAL\Transaction::SERIALIZABLE: + case \Doctrine\DBAL\Connection::TRANSACTION_REPEATABLE_READ: + case \Doctrine\DBAL\Connection::TRANSACTION_SERIALIZABLE: return 'SERIALIZABLE'; default: return parent::_getTransactionIsolationLevelSQL($level); diff --git a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php index 022b819d0..a1c2184b9 100644 --- a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php @@ -130,11 +130,11 @@ class SqlitePlatform extends AbstractPlatform protected function _getTransactionIsolationLevelSQL($level) { switch ($level) { - case \Doctrine\DBAL\Transaction::READ_UNCOMMITTED: + case \Doctrine\DBAL\Connection::TRANSACTION_READ_UNCOMMITTED: return 0; - case \Doctrine\DBAL\Transaction::READ_COMMITTED: - case \Doctrine\DBAL\Transaction::REPEATABLE_READ: - case \Doctrine\DBAL\Transaction::SERIALIZABLE: + case \Doctrine\DBAL\Connection::TRANSACTION_READ_COMMITTED: + case \Doctrine\DBAL\Connection::TRANSACTION_REPEATABLE_READ: + case \Doctrine\DBAL\Connection::TRANSACTION_SERIALIZABLE: return 1; default: return parent::_getTransactionIsolationLevelSQL($level); diff --git a/lib/Doctrine/DBAL/Transaction.php b/lib/Doctrine/DBAL/Transaction.php deleted file mode 100644 index 3b29f3c79..000000000 --- a/lib/Doctrine/DBAL/Transaction.php +++ /dev/null @@ -1,229 +0,0 @@ -. - */ - -namespace Doctrine\DBAL; - -/** - * The Transaction class is the central access point to DBAL Transaction functionality. - * - * @license http://www.opensource.org/licenses/lgpl-license.php LGPL - * @link www.doctrine-project.org - * @since 2.0 - * @version $Revision$ - * @author Benjamin Eberlei - * @author Guilherme Blanco - * @author Jonathan Wage - * @author Roman Borschel - */ -class Transaction -{ - /** - * Constant for transaction isolation level READ UNCOMMITTED. - */ - const READ_UNCOMMITTED = 1; - - /** - * Constant for transaction isolation level READ COMMITTED. - */ - const READ_COMMITTED = 2; - - /** - * Constant for transaction isolation level REPEATABLE READ. - */ - const REPEATABLE_READ = 3; - - /** - * Constant for transaction isolation level SERIALIZABLE. - */ - const SERIALIZABLE = 4; - - /** - * The transaction nesting level. - * - * @var integer - */ - private $_transactionNestingLevel = 0; - - /** - * The currently active transaction isolation level. - * - * @var integer - */ - private $_transactionIsolationLevel; - - /** - * Flag that indicates whether the current transaction is marked for rollback only. - * - * @var boolean - */ - private $_isRollbackOnly = false; - - /** - * Constructor - * - * @param Connection $conn The DBAL Connection - */ - public function __construct(Connection $conn) - { - $this->_conn = $conn; - - $this->_transactionIsolationLevel = $conn->getDatabasePlatform()->getDefaultTransactionIsolationLevel(); - } - - /** - * Checks whether a transaction is currently active. - * - * @return boolean TRUE if a transaction is currently active, FALSE otherwise. - */ - public function isTransactionActive() - { - return $this->_transactionNestingLevel > 0; - } - - /** - * Sets the transaction isolation level. - * - * @param integer $level The level to set. - */ - public function setTransactionIsolation($level) - { - $this->_transactionIsolationLevel = $level; - - return $this->executeUpdate($this->_conn->getDatabasePlatform()->getSetTransactionIsolationSQL($level)); - } - - /** - * Gets the currently active transaction isolation level. - * - * @return integer The current transaction isolation level. - */ - public function getTransactionIsolation() - { - return $this->_transactionIsolationLevel; - } - - /** - * Returns the current transaction nesting level. - * - * @return integer The nesting level. A value of 0 means there's no active transaction. - */ - public function getTransactionNestingLevel() - { - return $this->_transactionNestingLevel; - } - - /** - * Starts a transaction by suspending auto-commit mode. - * - * @return void - */ - public function begin() - { - $conn = $this->_conn->getWrappedConnection(); - - if ($this->_transactionNestingLevel == 0) { - $conn->beginTransaction(); - } - - ++$this->_transactionNestingLevel; - } - - /** - * Commits the current transaction. - * - * @return void - * @throws ConnectionException If the commit failed due to no active transaction or - * because the transaction was marked for rollback only. - */ - public function commit() - { - if ($this->_transactionNestingLevel == 0) { - throw ConnectionException::commitFailedNoActiveTransaction(); - } - - if ($this->_isRollbackOnly) { - throw ConnectionException::commitFailedRollbackOnly(); - } - - $conn = $this->_conn->getWrappedConnection(); - - if ($this->_transactionNestingLevel == 1) { - $conn->commit(); - } - - --$this->_transactionNestingLevel; - } - - /** - * Cancel any database changes done during the current transaction. - * - * this method can be listened with onPreTransactionRollback and onTransactionRollback - * eventlistener methods - * - * @throws ConnectionException If the rollback operation failed. - */ - public function rollback() - { - if ($this->_transactionNestingLevel == 0) { - throw ConnectionException::rollbackFailedNoActiveTransaction(); - } - - if ($this->_transactionNestingLevel == 1) { - $this->_transactionNestingLevel = 0; - $this->_conn->getWrappedConnection()->rollback(); - $this->_isRollbackOnly = false; - } else { - $this->_isRollbackOnly = true; - --$this->_transactionNestingLevel; - } - } - - /** - * Marks the current transaction so that the only possible - * outcome for the transaction to be rolled back. - * - * @throws ConnectionException If no transaction is active. - */ - public function setRollbackOnly() - { - if ($this->_transactionNestingLevel == 0) { - throw ConnectionException::noActiveTransaction(); - } - - $this->_isRollbackOnly = true; - } - - /** - * Check whether the current transaction is marked for rollback only. - * - * @return boolean - * @throws ConnectionException If no transaction is active. - */ - public function getRollbackOnly() - { - if ($this->_transactionNestingLevel == 0) { - throw ConnectionException::noActiveTransaction(); - } - - return $this->_isRollbackOnly; - } -} \ No newline at end of file diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 843682ab0..a51c89e46 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -134,7 +134,7 @@ class EntityManager $config->getProxyDir(), $config->getProxyNamespace(), $config->getAutoGenerateProxyClasses()); - $this->_transaction = new EntityTransaction($conn->getTransaction()); + $this->_transaction = new EntityTransaction($this); } /** @@ -164,7 +164,7 @@ class EntityManager */ public function getTransaction() { - return $this->_transaction; + return $this->_transaction; } /** @@ -207,11 +207,12 @@ class EntityManager /** * Performs a rollback on the underlying database connection and closes the * EntityManager as it may now be in a corrupted state. + * + * @return boolean TRUE on success, FALSE on failure */ public function rollback() { - $this->getTransaction()->rollback(); - $this->close(); + return $this->getTransaction()->rollback(); } /** diff --git a/lib/Doctrine/ORM/EntityTransaction.php b/lib/Doctrine/ORM/EntityTransaction.php index 80695ee20..1257d0939 100644 --- a/lib/Doctrine/ORM/EntityTransaction.php +++ b/lib/Doctrine/ORM/EntityTransaction.php @@ -25,7 +25,6 @@ use Doctrine\DBAL\Transaction; /** * The Transaction class is the central access point to ORM Transaction functionality. - * This class acts more as a delegate class to the DBAL Transaction functionality. * * @license http://www.opensource.org/licenses/lgpl-license.php LGPL * @link www.doctrine-project.org @@ -36,23 +35,31 @@ use Doctrine\DBAL\Transaction; * @author Jonathan Wage * @author Roman Borschel */ -class EntityTransaction +final class EntityTransaction { /** - * The wrapped DBAL Transaction. + * The wrapped ORM EntityManager. * - * @var Doctrine\DBAL\Transaction + * @var Doctrine\ORM\EntityManager */ - protected $_wrappedTransaction; + private $_em; + + /** + * The database connection used by the EntityManager. + * + * @var Doctrine\DBAL\Connection + */ + private $_conn; /** * Constructor. * * @param Transaction $transaction */ - public function __construct(Transaction $transaction) + public function __construct(EntityManager $em) { - $this->_wrappedTransaction = $transaction; + $this->_em = $em; + $this->_conn = $em->getConnection(); } /** @@ -62,7 +69,7 @@ class EntityTransaction */ public function isTransactionActive() { - return $this->_wrappedTransaction->isTransactionActive(); + return $this->_conn->isTransactionActive(); } /** @@ -72,7 +79,7 @@ class EntityTransaction */ public function setTransactionIsolation($level) { - return $this->_wrappedTransaction->setTransactionIsolation($level); + return $this->_conn->setTransactionIsolation($level); } /** @@ -82,7 +89,7 @@ class EntityTransaction */ public function getTransactionIsolation() { - return $this->_wrappedTransaction->getTransactionIsolation(); + return $this->_conn->getTransactionIsolation(); } /** @@ -92,7 +99,7 @@ class EntityTransaction */ public function getTransactionNestingLevel() { - return $this->_wrappedTransaction->getTransactionNestingLevel(); + return $this->_conn->getTransactionNestingLevel(); } /** @@ -102,7 +109,7 @@ class EntityTransaction */ public function begin() { - $this->_wrappedTransaction->begin(); + $this->_conn->beginTransaction(); } /** @@ -114,41 +121,43 @@ class EntityTransaction */ public function commit() { - $this->_wrappedTransaction->commit(); + $this->_conn->commit(); } /** * Cancel any database changes done during the current transaction. * * this method can be listened with onPreTransactionRollback and onTransactionRollback - * eventlistener methods + * event listener methods * + * @return boolean TRUE on success, FALSE on failure * @throws Doctrine\DBAL\ConnectionException If the rollback operation failed. */ public function rollback() { - $this->_wrappedTransaction->rollback(); + $this->_em->close(); + return $this->_conn->rollback(); } /** * Marks the current transaction so that the only possible * outcome for the transaction to be rolled back. * - * @throws ConnectionException If no transaction is active. + * @throws Doctrine\DBAL\ConnectionException If no transaction is active. */ public function setRollbackOnly() { - $this->_wrappedTransaction->setRollbackOnly(); + $this->_conn->setRollbackOnly(); } /** * Check whether the current transaction is marked for rollback only. * * @return boolean - * @throws ConnectionException If no transaction is active. + * @throws Doctrine\DBAL\ConnectionException If no transaction is active. */ - public function getRollbackOnly() + public function isRollbackOnly() { - return $this->_wrappedTransaction->getRollbackOnly(); + return $this->_conn->getRollbackOnly(); } } \ No newline at end of file diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 95959ad38..63a99ea32 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -278,7 +278,7 @@ class UnitOfWork implements PropertyChangedListener $commitOrder = $this->_getCommitOrder(); $tx = $this->_em->getTransaction(); - + try { $tx->begin(); diff --git a/tests/Doctrine/Tests/DBAL/Functional/AllTests.php b/tests/Doctrine/Tests/DBAL/Functional/AllTests.php index f5a0b0e9f..2132fe5b4 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/AllTests.php +++ b/tests/Doctrine/Tests/DBAL/Functional/AllTests.php @@ -26,7 +26,7 @@ class AllTests $suite->addTestSuite('Doctrine\Tests\DBAL\Functional\Schema\PostgreSqlSchemaManagerTest'); $suite->addTestSuite('Doctrine\Tests\DBAL\Functional\Schema\OracleSchemaManagerTest'); $suite->addTestSuite('Doctrine\Tests\DBAL\Functional\Schema\Db2SchemaManagerTest'); - $suite->addTestSuite('Doctrine\Tests\DBAL\Functional\TransactionTest'); + $suite->addTestSuite('Doctrine\Tests\DBAL\Functional\ConnectionTest'); return $suite; } diff --git a/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php new file mode 100644 index 000000000..fa6f23dce --- /dev/null +++ b/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php @@ -0,0 +1,64 @@ +_conn->beginTransaction(); + $this->assertEquals(1, $this->_conn->getTransactionNestingLevel()); + + try { + $this->_conn->beginTransaction(); + $this->assertEquals(2, $this->_conn->getTransactionNestingLevel()); + throw new \Exception; + $this->_conn->commit(); // never reached + } catch (\Exception $e) { + $this->_conn->rollback(); + $this->assertEquals(1, $this->_conn->getTransactionNestingLevel()); + //no rethrow + } + $this->assertTrue($this->_conn->getRollbackOnly()); + + $this->_conn->commit(); // should throw exception + $this->fail('Transaction commit after failed nested transaction should fail.'); + } catch (ConnectionException $e) { + $this->assertEquals(1, $this->_conn->getTransactionNestingLevel()); + $this->_conn->rollback(); + $this->assertEquals(0, $this->_conn->getTransactionNestingLevel()); + } + } + + public function testTransactionBehavior() + { + try { + $this->_conn->beginTransaction(); + $this->assertEquals(1, $this->_conn->getTransactionNestingLevel()); + + throw new \Exception; + + $this->_connx->commit(); // never reached + } catch (\Exception $e) { + $this->assertEquals(1, $this->_conn->getTransactionNestingLevel()); + $this->_conn->rollback(); + $this->assertEquals(0, $this->_conn->getTransactionNestingLevel()); + } + + try { + $this->_conn->beginTransaction(); + $this->assertEquals(1, $this->_conn->getTransactionNestingLevel()); + $this->_conn->commit(); + } catch (\Exception $e) { + $this->_conn->rollback(); + $this->assertEquals(0, $this->_conn->getTransactionNestingLevel()); + } + } + +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/DBAL/Functional/TransactionTest.php b/tests/Doctrine/Tests/DBAL/Functional/TransactionTest.php deleted file mode 100644 index 3663bbaaf..000000000 --- a/tests/Doctrine/Tests/DBAL/Functional/TransactionTest.php +++ /dev/null @@ -1,68 +0,0 @@ -_conn->getTransaction(); - - try { - $tx->begin(); - $this->assertEquals(1, $tx->getTransactionNestingLevel()); - - try { - $tx->begin(); - $this->assertEquals(2, $tx->getTransactionNestingLevel()); - throw new \Exception; - $tx->commit(); // never reached - } catch (\Exception $e) { - $tx->rollback(); - $this->assertEquals(1, $tx->getTransactionNestingLevel()); - //no rethrow - } - $this->assertTrue($tx->getRollbackOnly()); - - $tx->commit(); // should throw exception - $this->fail('Transaction commit after failed nested transaction should fail.'); - } catch (ConnectionException $e) { - $this->assertEquals(1, $tx->getTransactionNestingLevel()); - $tx->rollback(); - $this->assertEquals(0, $tx->getTransactionNestingLevel()); - } - } - - public function testTransactionBehavior() - { - $tx = $this->_conn->getTransaction(); - - try { - $tx->begin(); - $this->assertEquals(1, $tx->getTransactionNestingLevel()); - - throw new \Exception; - - $tx->commit(); // never reached - } catch (\Exception $e) { - $this->assertEquals(1, $tx->getTransactionNestingLevel()); - $tx->rollback(); - $this->assertEquals(0, $tx->getTransactionNestingLevel()); - } - - try { - $tx->begin(); - $this->assertEquals(1, $tx->getTransactionNestingLevel()); - $tx->commit(); - } catch (\Exception $e) { - $tx->rollback(); - $this->assertEquals(0, $tx->getTransactionNestingLevel()); - } - } - -} \ No newline at end of file diff --git a/tests/Doctrine/Tests/DBAL/Platforms/MsSqlPlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/MsSqlPlatformTest.php index e0691d74b..1302be035 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/MsSqlPlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/MsSqlPlatformTest.php @@ -44,19 +44,19 @@ class MsSqlPlatformTest extends AbstractPlatformTestCase { $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_UNCOMMITTED) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_UNCOMMITTED) ); $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL READ COMMITTED', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_COMMITTED) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_COMMITTED) ); $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL REPEATABLE READ', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::REPEATABLE_READ) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_REPEATABLE_READ) ); $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL SERIALIZABLE', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::SERIALIZABLE) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_SERIALIZABLE) ); } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/MySqlPlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/MySqlPlatformTest.php index b2c916aec..8dbeac5db 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/MySqlPlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/MySqlPlatformTest.php @@ -53,20 +53,20 @@ class MySqlPlatformTest extends AbstractPlatformTestCase { $this->assertEquals( 'SET SESSION TRANSACTION ISOLATION LEVEL READ UNCOMMITTED', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_UNCOMMITTED), + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_UNCOMMITTED), '' ); $this->assertEquals( 'SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_COMMITTED) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_COMMITTED) ); $this->assertEquals( 'SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::REPEATABLE_READ) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_REPEATABLE_READ) ); $this->assertEquals( 'SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::SERIALIZABLE) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_SERIALIZABLE) ); } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php index c2b02d50f..8fb24abf6 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php @@ -55,19 +55,19 @@ class OraclePlatformTest extends AbstractPlatformTestCase { $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_UNCOMMITTED) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_UNCOMMITTED) ); $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL READ COMMITTED', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_COMMITTED) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_COMMITTED) ); $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL SERIALIZABLE', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::REPEATABLE_READ) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_REPEATABLE_READ) ); $this->assertEquals( 'SET TRANSACTION ISOLATION LEVEL SERIALIZABLE', - $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::SERIALIZABLE) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_SERIALIZABLE) ); } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/PostgreSqlPlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/PostgreSqlPlatformTest.php index 75a8aabe8..38681b3e1 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/PostgreSqlPlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/PostgreSqlPlatformTest.php @@ -4,7 +4,6 @@ namespace Doctrine\Tests\DBAL\Platforms; use Doctrine\DBAL\Platforms\PostgreSqlPlatform; use Doctrine\DBAL\Types\Type; -use Doctrine\DBAL\Transaction; require_once __DIR__ . '/../../TestInit.php'; @@ -74,19 +73,19 @@ class PostgreSqlPlatformTest extends AbstractPlatformTestCase { $this->assertEquals( 'SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL READ UNCOMMITTED', - $this->_platform->getSetTransactionIsolationSQL(Transaction::READ_UNCOMMITTED) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_UNCOMMITTED) ); $this->assertEquals( 'SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL READ COMMITTED', - $this->_platform->getSetTransactionIsolationSQL(Transaction::READ_COMMITTED) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_COMMITTED) ); $this->assertEquals( 'SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL REPEATABLE READ', - $this->_platform->getSetTransactionIsolationSQL(Transaction::REPEATABLE_READ) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_REPEATABLE_READ) ); $this->assertEquals( 'SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL SERIALIZABLE', - $this->_platform->getSetTransactionIsolationSQL(Transaction::SERIALIZABLE) + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_SERIALIZABLE) ); } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php index 5a56b72bc..fae5538e3 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php @@ -36,10 +36,22 @@ class SqlitePlatformTest extends AbstractPlatformTestCase public function testGeneratesTransactionCommands() { - $this->assertEquals('PRAGMA read_uncommitted = 0', $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_UNCOMMITTED)); - $this->assertEquals('PRAGMA read_uncommitted = 1', $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::READ_COMMITTED)); - $this->assertEquals('PRAGMA read_uncommitted = 1', $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::REPEATABLE_READ)); - $this->assertEquals('PRAGMA read_uncommitted = 1', $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Transaction::SERIALIZABLE)); + $this->assertEquals( + 'PRAGMA read_uncommitted = 0', + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_UNCOMMITTED) + ); + $this->assertEquals( + 'PRAGMA read_uncommitted = 1', + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_READ_COMMITTED) + ); + $this->assertEquals( + 'PRAGMA read_uncommitted = 1', + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_REPEATABLE_READ) + ); + $this->assertEquals( + 'PRAGMA read_uncommitted = 1', + $this->_platform->getSetTransactionIsolationSQL(\Doctrine\DBAL\Connection::TRANSACTION_SERIALIZABLE) + ); } public function testPrefersIdentityColumns() From 65fbb9f7a42b34d8211b896b1cc2b140e982867f Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Thu, 6 May 2010 18:45:18 -0300 Subject: [PATCH 06/17] Renamed fetchRow to fetchAssoc, as defined in @todo list. Renamed getRollbackOnly to isRollbackOnly, since it is more consistent to its purpose. --- lib/Doctrine/DBAL/Connection.php | 9 ++++----- lib/Doctrine/ORM/EntityManager.php | 2 +- lib/Doctrine/ORM/EntityTransaction.php | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php index 9ed6292ca..3ad6e6e0b 100644 --- a/lib/Doctrine/DBAL/Connection.php +++ b/lib/Doctrine/DBAL/Connection.php @@ -310,9 +310,8 @@ class Connection implements DriverConnection * @param string $statement The SQL query. * @param array $params The query parameters. * @return array - * @todo Rename: fetchAssoc */ - public function fetchRow($statement, array $params = array()) + public function fetchAssoc($statement, array $params = array()) { return $this->executeQuery($statement, $params)->fetch(PDO::FETCH_ASSOC); } @@ -583,10 +582,10 @@ class Connection implements DriverConnection * represents a row of the result set. * @return mixed The projected result of the query. */ - public function project($query, array $params = array(), Closure $function) + public function project($query, array $params, Closure $function) { $result = array(); - $stmt = $this->executeQuery($query, $params); + $stmt = $this->executeQuery($query, $params ?: array()); while ($row = $stmt->fetch()) { $result[] = $function($row); @@ -820,7 +819,7 @@ class Connection implements DriverConnection * @return boolean * @throws ConnectionException If no transaction is active. */ - public function getRollbackOnly() + public function isRollbackOnly() { if ($this->_transactionNestingLevel == 0) { throw ConnectionException::noActiveTransaction(); diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index a51c89e46..b9fdf98f5 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -164,7 +164,7 @@ class EntityManager */ public function getTransaction() { - return $this->_transaction; + return $this->_transaction; } /** diff --git a/lib/Doctrine/ORM/EntityTransaction.php b/lib/Doctrine/ORM/EntityTransaction.php index 1257d0939..40bcd8a13 100644 --- a/lib/Doctrine/ORM/EntityTransaction.php +++ b/lib/Doctrine/ORM/EntityTransaction.php @@ -158,6 +158,6 @@ final class EntityTransaction */ public function isRollbackOnly() { - return $this->_conn->getRollbackOnly(); + return $this->_conn->isRollbackOnly(); } } \ No newline at end of file From 8d52967fcd8a5d956fa0b7327fe1b89a690a6c79 Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Fri, 7 May 2010 13:36:25 +0200 Subject: [PATCH 07/17] Polished QueryBuilder API documentation and added another test. --- lib/Doctrine/ORM/EntityManager.php | 5 - lib/Doctrine/ORM/QueryBuilder.php | 298 ++++++++++-------- tests/Doctrine/Tests/ORM/QueryBuilderTest.php | 19 +- 3 files changed, 187 insertions(+), 135 deletions(-) diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index c4aa9bb55..871bc62fa 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -1,7 +1,5 @@ * @author Guilherme Blanco * @author Jonathan Wage diff --git a/lib/Doctrine/ORM/QueryBuilder.php b/lib/Doctrine/ORM/QueryBuilder.php index fcfe873d8..e547814ff 100644 --- a/lib/Doctrine/ORM/QueryBuilder.php +++ b/lib/Doctrine/ORM/QueryBuilder.php @@ -1,7 +1,5 @@ - * @author Jonathan Wage - * @author Roman Borschel + * @since 2.0 + * @author Guilherme Blanco + * @author Jonathan Wage + * @author Roman Borschel */ class QueryBuilder { + /* The query types. */ const SELECT = 0; const DELETE = 1; const UPDATE = 2; + /** The builder states. */ const STATE_DIRTY = 0; const STATE_CLEAN = 1; /** - * @var EntityManager $em The EntityManager used by this QueryBuilder. + * @var EntityManager The EntityManager used by this QueryBuilder. */ private $_em; /** - * @var array $dqlParts The array of DQL parts collected. + * @var array The array of DQL parts collected. */ private $_dqlParts = array( 'select' => array(), @@ -105,13 +102,17 @@ class QueryBuilder /** * Gets an ExpressionBuilder used for object-oriented construction of query expressions. - * Intended for convenient inline usage. Example: + * This producer method is intended for convenient inline usage. Example: * - * [php] + * * $qb = $em->createQueryBuilder() * ->select('u') * ->from('User', 'u') * ->where($qb->expr()->eq('u.id', 1)); + * + * + * For more complex expression construction, consider storing the expression + * builder object in a local variable. * * @return Expr */ @@ -141,16 +142,9 @@ class QueryBuilder } /** - * Get the state of this query builder instance + * Get the state of this query builder instance. * - * [php] - * if ($qb->getState() == QueryBuilder::STATE_DIRTY) { - * echo 'Query builder is dirty'; - * } else { - * echo 'Query builder is clean'; - * } - * - * @return integer + * @return integer Either QueryBuilder::STATE_DIRTY or QueryBuilder::STATE_CLEAN. */ public function getState() { @@ -158,15 +152,16 @@ class QueryBuilder } /** - * Get the complete DQL string for this query builder instance + * Get the complete DQL string formed by the current specifications of this QueryBuilder. * - * [php] + * * $qb = $em->createQueryBuilder() * ->select('u') * ->from('User', 'u') * echo $qb->getDql(); // SELECT u FROM User u + * * - * @return string The DQL string + * @return string The DQL query string. */ public function getDQL() { @@ -198,14 +193,15 @@ class QueryBuilder } /** - * Constructs a Query instance from the current configuration of the builder. + * Constructs a Query instance from the current specifications of the builder. * - * [php] + * * $qb = $em->createQueryBuilder() * ->select('u') * ->from('User', 'u'); * $q = $qb->getQuery(); * $results = $q->execute(); + * * * @return Query */ @@ -218,17 +214,19 @@ class QueryBuilder } /** - * Get the root alias for the query. This is the first entity alias involved - * in the construction of the query + * Gets the root alias of the query. This is the first entity alias involved + * in the construction of the query. * - * [php] + * * $qb = $em->createQueryBuilder() * ->select('u') * ->from('User', 'u'); * * echo $qb->getRootAlias(); // u + * * * @return string $rootAlias + * @todo Rename/Refactor: getRootAliases(), there can be multiple roots! */ public function getRootAlias() { @@ -236,14 +234,15 @@ class QueryBuilder } /** - * Sets a query parameter. + * Sets a query parameter for the query being constructed. * - * [php] + * * $qb = $em->createQueryBuilder() * ->select('u') * ->from('User', 'u') * ->where('u.id = :user_id') * ->setParameter(':user_id', 1); + * * * @param string|integer $key The parameter position or name. * @param mixed $value The parameter value. @@ -256,9 +255,9 @@ class QueryBuilder } /** - * Sets a collection of query parameters. + * Sets a collection of query parameters for the query being constructed. * - * [php] + * * $qb = $em->createQueryBuilder() * ->select('u') * ->from('User', 'u') @@ -267,8 +266,9 @@ class QueryBuilder * ':user_id1' => 1, * ':user_id2' => 2 * )); + * * - * @param array $params + * @param array $params The query parameters to set. * @return QueryBuilder This QueryBuilder instance. */ public function setParameters(array $params) @@ -278,17 +278,17 @@ class QueryBuilder } /** - * Get all defined parameters + * Gets all defined query parameters for the query being constructed. * - * @return array Defined parameters + * @return array The currently defined query parameters. */ - public function getParameters($params = array()) + public function getParameters() { return $this->_params; } - + /** - * Gets a query parameter. + * Gets a (previously set) query parameter of the query being constructed. * * @param mixed $key The key (index or name) of the bound parameter. * @return mixed The value of the bound parameter. @@ -297,7 +297,7 @@ class QueryBuilder { return isset($this->_params[$key]) ? $this->_params[$key] : null; } - + /** * Sets the position of the first result to retrieve (the "offset"). * @@ -309,10 +309,10 @@ class QueryBuilder $this->_firstResult = $firstResult; return $this; } - + /** * Gets the position of the first result the query object was set to retrieve (the "offset"). - * Returns NULL if {@link setFirstResult} was not applied to this query builder. + * Returns NULL if {@link setFirstResult} was not applied to this QueryBuilder. * * @return integer The position of the first result. */ @@ -324,7 +324,7 @@ class QueryBuilder /** * Sets the maximum number of results to retrieve (the "limit"). * - * @param integer $maxResults + * @param integer $maxResults The maximum number of results to retrieve. * @return QueryBuilder This QueryBuilder instance. */ public function setMaxResults($maxResults) @@ -345,7 +345,10 @@ class QueryBuilder } /** - * Add a single DQL query part to the array of parts + * Either appends to or replaces a single, generic query part. + * + * The available parts are: 'select', 'from', 'join', 'set', 'where', + * 'groupBy', 'having' and 'orderBy'. * * @param string $dqlPartName * @param string $dqlPart @@ -368,15 +371,17 @@ class QueryBuilder } /** - * Set the SELECT statement + * Specifies an item that is to be returned in the query result. + * Replaces any previously specified selections, if any. * - * [php] + * * $qb = $em->createQueryBuilder() * ->select('u', 'p') * ->from('User', 'u') * ->leftJoin('u.Phonenumbers', 'p'); + * * - * @param mixed $select String SELECT statement or SELECT Expr instance + * @param mixed $select The selection expressions. * @return QueryBuilder This QueryBuilder instance. */ public function select($select = null) @@ -393,16 +398,17 @@ class QueryBuilder } /** - * Add to the SELECT statement + * Adds an item that is to be returned in the query result. * - * [php] + * * $qb = $em->createQueryBuilder() * ->select('u') * ->addSelect('p') * ->from('User', 'u') * ->leftJoin('u.Phonenumbers', 'p'); + * * - * @param mixed $select String SELECT statement or SELECT Expr instance + * @param mixed $select The selection expression. * @return QueryBuilder This QueryBuilder instance. */ public function addSelect($select = null) @@ -419,16 +425,18 @@ class QueryBuilder } /** - * Construct a DQL DELETE query + * Turns the query being built into a bulk delete query that ranges over + * a certain entity type. * - * [php] + * * $qb = $em->createQueryBuilder() * ->delete('User', 'u') * ->where('u.id = :user_id'); * ->setParameter(':user_id', 1); + * * - * @param string $delete The model to delete - * @param string $alias The alias of the model + * @param string $delete The class/type whose instances are subject to the deletion. + * @param string $alias The class/type alias used in the constructed query. * @return QueryBuilder This QueryBuilder instance. */ public function delete($delete = null, $alias = null) @@ -443,16 +451,18 @@ class QueryBuilder } /** - * Construct a DQL UPDATE query + * Turns the query being built into a bulk update query that ranges over + * a certain entity type. * - * [php] + * * $qb = $em->createQueryBuilder() * ->update('User', 'u') * ->set('u.password', md5('password')) * ->where('u.id = ?'); + * * - * @param string $update The model to update - * @param string $alias The alias of the model + * @param string $update The class/type whose instances are subject to the update. + * @param string $alias The class/type alias used in the constructed query. * @return QueryBuilder This QueryBuilder instance. */ public function update($update = null, $alias = null) @@ -467,12 +477,14 @@ class QueryBuilder } /** - * Specify the FROM part when constructing a SELECT DQL query + * Create and add a query root corresponding to the entity identified by the given alias, + * forming a cartesian product with any existing query roots. * - * [php] + * * $qb = $em->createQueryBuilder() * ->select('u') * ->from('User', 'u') + * * * @param string $from The class name. * @param string $alias The alias of the class. @@ -482,20 +494,25 @@ class QueryBuilder { return $this->add('from', new Expr\From($from, $alias), true); } - + /** - * Add a INNER JOIN to an associated class. + * Creates and adds a join over an entity association to the query. * - * [php] + * The entities in the joined association will be fetched as part of the query + * result if the alias used for the joined association is placed in the select + * expressions. + * + * * $qb = $em->createQueryBuilder() * ->select('u') * ->from('User', 'u') - * ->innerJoin('u.Phonenumbers', 'p', Expr\Join::WITH, 'p.is_primary = 1'); + * ->join('u.Phonenumbers', 'p', Expr\Join::WITH, 'p.is_primary = 1'); + * * - * @param string $join The relationship to join - * @param string $alias The alias of the join - * @param string $conditionType The condition type constant. Either ON or WITH. - * @param string $condition The condition for the join + * @param string $join The relationship to join + * @param string $alias The alias of the join + * @param string $conditionType The condition type constant. Either ON or WITH. + * @param string $condition The condition for the join * @return QueryBuilder This QueryBuilder instance. */ public function join($join, $alias, $conditionType = null, $condition = null) @@ -504,7 +521,11 @@ class QueryBuilder } /** - * Add an INNER JOIN to an associated class. + * Creates and adds a join over an entity association to the query. + * + * The entities in the joined association will be fetched as part of the query + * result if the alias used for the joined association is placed in the select + * expressions. * * [php] * $qb = $em->createQueryBuilder() @@ -512,10 +533,10 @@ class QueryBuilder * ->from('User', 'u') * ->innerJoin('u.Phonenumbers', 'p', Expr\Join::WITH, 'p.is_primary = 1'); * - * @param string $join The relationship to join - * @param string $alias The alias of the join - * @param string $conditionType The condition type constant. Either ON or WITH. - * @param string $condition The condition for the join + * @param string $join The relationship to join + * @param string $alias The alias of the join + * @param string $conditionType The condition type constant. Either ON or WITH. + * @param string $condition The condition for the join * @return QueryBuilder This QueryBuilder instance. */ public function innerJoin($join, $alias, $conditionType = null, $condition = null) @@ -526,19 +547,24 @@ class QueryBuilder } /** - * Add a LEFT JOIN + * Creates and adds a left join over an entity association to the query. * - * [php] + * The entities in the joined association will be fetched as part of the query + * result if the alias used for the joined association is placed in the select + * expressions. + * + * * $qb = $em->createQueryBuilder() * ->select('u') * ->from('User', 'u') * ->leftJoin('u.Phonenumbers', 'p', Expr\Join::WITH, 'p.is_primary = 1'); + * * - * @param string $join The relationship to join - * @param string $alias The alias of the join - * @param string $conditionType The condition type constant. Either ON or WITH. - * @param string $condition The condition for the join - * @return QueryBuilder $qb + * @param string $join The relationship to join + * @param string $alias The alias of the join + * @param string $conditionType The condition type constant. Either ON or WITH. + * @param string $condition The condition for the join + * @return QueryBuilder This QueryBuilder instance. */ public function leftJoin($join, $alias, $conditionType = null, $condition = null) { @@ -548,17 +574,18 @@ class QueryBuilder } /** - * Add a SET statement for a DQL UPDATE query + * Sets a new value for a field in a bulk update query. * - * [php] + * * $qb = $em->createQueryBuilder() * ->update('User', 'u') * ->set('u.password', md5('password')) * ->where('u.id = ?'); + * * - * @param string $key The key/field to set - * @param string $value The value, expression, placeholder, etc. to use in the SET - * @return QueryBuilder $qb + * @param string $key The key/field to set. + * @param string $value The value, expression, placeholder, etc. + * @return QueryBuilder This QueryBuilder instance. */ public function set($key, $value) { @@ -566,9 +593,10 @@ class QueryBuilder } /** - * Set and override any existing WHERE statements + * Specifies one or more restrictions to the query result. + * Replaces any previously specified restrictions, if any. * - * [php] + * * $qb = $em->createQueryBuilder() * ->select('u') * ->from('User', 'u') @@ -584,9 +612,10 @@ class QueryBuilder * $qb->update('User', 'u') * ->set('u.password', md5('password')) * ->where($or); + * * - * @param mixed $predicates The predicates. - * @return QueryBuilder + * @param mixed $predicates The restriction predicates. + * @return QueryBuilder This QueryBuilder instance. */ public function where($predicates) { @@ -598,17 +627,19 @@ class QueryBuilder } /** - * Add a new WHERE statement with an AND + * Adds one or more restrictions to the query results, forming a logical + * conjunction with any previously specified restrictions. * - * [php] + * * $qb = $em->createQueryBuilder() * ->select('u') * ->from('User', 'u') * ->where('u.username LIKE ?') * ->andWhere('u.is_active = 1'); + * * - * @param mixed $where The WHERE statement - * @return QueryBuilder $qb + * @param mixed $where The query restrictions. + * @return QueryBuilder This QueryBuilder instance. * @see where() */ public function andWhere($where) @@ -627,14 +658,16 @@ class QueryBuilder } /** - * Add a new WHERE statement with an OR + * Adds one or more restrictions to the query results, forming a logical + * disjunction with any previously specified restrictions. * - * [php] + * * $qb = $em->createQueryBuilder() * ->select('u') * ->from('User', 'u') * ->where('u.id = 1') * ->orWhere('u.id = 2'); + * * * @param mixed $where The WHERE statement * @return QueryBuilder $qb @@ -656,16 +689,18 @@ class QueryBuilder } /** - * Set the GROUP BY clause + * Specifies a grouping over the results of the query. + * Replaces any previously specified groupings, if any. * - * [php] + * * $qb = $em->createQueryBuilder() * ->select('u') * ->from('User', 'u') * ->groupBy('u.id'); + * * - * @param string $groupBy The GROUP BY clause - * @return QueryBuilder $qb + * @param string $groupBy The grouping expression. + * @return QueryBuilder This QueryBuilder instance. */ public function groupBy($groupBy) { @@ -674,17 +709,18 @@ class QueryBuilder /** - * Add to the existing GROUP BY clause + * Adds a grouping expression to the query. * - * [php] + * * $qb = $em->createQueryBuilder() * ->select('u') * ->from('User', 'u') - * ->groupBy('u.last_login'); - * ->addGroupBy('u.created_at') + * ->groupBy('u.lastLogin'); + * ->addGroupBy('u.createdAt') + * * - * @param string $groupBy The GROUP BY clause - * @return QueryBuilder $qb + * @param string $groupBy The grouping expression. + * @return QueryBuilder This QueryBuilder instance. */ public function addGroupBy($groupBy) { @@ -692,10 +728,11 @@ class QueryBuilder } /** - * Set the HAVING clause + * Specifies a restriction over the groups of the query. + * Replaces any previous having restrictions, if any. * - * @param mixed $having - * @return QueryBuilder $qb + * @param mixed $having The restriction over the groups. + * @return QueryBuilder This QueryBuilder instance. */ public function having($having) { @@ -707,10 +744,11 @@ class QueryBuilder } /** - * Add to the existing HAVING clause with an AND + * Adds a restriction over the groups of the query, forming a logical + * conjunction with any existing having restrictions. * - * @param mixed $having - * @return QueryBuilder $qb + * @param mixed $having The restriction to append. + * @return QueryBuilder This QueryBuilder instance. */ public function andHaving($having) { @@ -728,10 +766,11 @@ class QueryBuilder } /** - * Add to the existing HAVING clause with an OR + * Adds a restriction over the groups of the query, forming a logical + * disjunction with any existing having restrictions. * - * @param mixed $having - * @return QueryBuilder $qb + * @param mixed $having The restriction to add. + * @return QueryBuilder This QueryBuilder instance. */ public function orHaving($having) { @@ -749,11 +788,12 @@ class QueryBuilder } /** - * Set the ORDER BY clause + * Specifies an ordering for the query results. + * Replaces any previously specified orderings, if any. * - * @param string $sort What to sort on - * @param string $order Optional: The order to sort the results. - * @return QueryBuilder $qb + * @param string $sort The ordering expression. + * @param string $order The ordering direction. + * @return QueryBuilder This QueryBuilder instance. */ public function orderBy($sort, $order = null) { @@ -762,11 +802,11 @@ class QueryBuilder } /** - * Add to the existing ORDER BY clause + * Adds an ordering to the query results. * - * @param string $sort What to sort on - * @param string $order Optional: The order to sort the results. - * @return QueryBuilder $qb + * @param string $sort The ordering expression. + * @param string $order The ordering direction. + * @return QueryBuilder This QueryBuilder instance. */ public function addOrderBy($sort, $order = null) { @@ -774,10 +814,11 @@ class QueryBuilder } /** - * Get a DQL part or parts by the part name + * Get a query part by its name. * * @param string $queryPartName * @return mixed $queryPart + * @todo Rename: getQueryPart (or remove?) */ public function getDQLPart($queryPartName) { @@ -785,9 +826,10 @@ class QueryBuilder } /** - * Get the full DQL parts array + * Get all query parts. * * @return array $dqlParts + * @todo Rename: getQueryParts (or remove?) */ public function getDQLParts() { @@ -836,6 +878,12 @@ class QueryBuilder . (isset($options['post']) ? $options['post'] : ''); } + /** + * Gets a string representation of this QueryBuilder which corresponds to + * the final DQL query being constructed. + * + * @return string The string representation of this QueryBuilder. + */ public function __toString() { return $this->getDQL(); diff --git a/tests/Doctrine/Tests/ORM/QueryBuilderTest.php b/tests/Doctrine/Tests/ORM/QueryBuilderTest.php index d4a592e65..468e0e20c 100644 --- a/tests/Doctrine/Tests/ORM/QueryBuilderTest.php +++ b/tests/Doctrine/Tests/ORM/QueryBuilderTest.php @@ -1,7 +1,5 @@ * @author Roman Borschel assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid) OR (u.id = :uid2)'); } + public function testComplexAndWhereOrWhereNesting() + { + $qb = $this->_em->createQueryBuilder(); + $qb->select('u') + ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u') + ->where('u.id = :uid') + ->orWhere('u.id = :uid2') + ->andWhere('u.id = :uid3') + ->orWhere('u.name = :name1', 'u.name = :name2') + ->andWhere('u.name <> :noname'); + + $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE ((((u.id = :uid) OR (u.id = :uid2)) AND (u.id = :uid3)) OR (u.name = :name1) OR (u.name = :name2)) AND (u.name <> :noname)'); + } + public function testAndWhereIn() { $qb = $this->_em->createQueryBuilder(); From 20c6259fa372349f7c32fe1cd32e6865aaf8ff51 Mon Sep 17 00:00:00 2001 From: Christian Heinrich Date: Wed, 5 May 2010 13:12:38 +0200 Subject: [PATCH 08/17] Corrected method names; the interface already used SQL, the files still used Sql in method names --- .../ORM/Persisters/ManyToManyPersister.php | 24 ++++++++--------- .../ORM/Persisters/OneToManyPersister.php | 26 +++++++++---------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index 7ca821930..a182a9859 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -36,7 +36,7 @@ class ManyToManyPersister extends AbstractCollectionPersister * * @override */ - protected function _getDeleteRowSql(PersistentCollection $coll) + protected function _getDeleteRowSQL(PersistentCollection $coll) { $mapping = $coll->getMapping(); $joinTable = $mapping->joinTable; @@ -51,7 +51,7 @@ class ManyToManyPersister extends AbstractCollectionPersister * @internal Order of the parameters must be the same as the order of the columns in * _getDeleteRowSql. */ - protected function _getDeleteRowSqlParameters(PersistentCollection $coll, $element) + protected function _getDeleteRowSQLParameters(PersistentCollection $coll, $element) { return $this->_collectJoinTableColumnParameters($coll, $element); } @@ -61,7 +61,7 @@ class ManyToManyPersister extends AbstractCollectionPersister * * @override */ - protected function _getUpdateRowSql(PersistentCollection $coll) + protected function _getUpdateRowSQL(PersistentCollection $coll) {} /** @@ -71,7 +71,7 @@ class ManyToManyPersister extends AbstractCollectionPersister * @internal Order of the parameters must be the same as the order of the columns in * _getInsertRowSql. */ - protected function _getInsertRowSql(PersistentCollection $coll) + protected function _getInsertRowSQL(PersistentCollection $coll) { $mapping = $coll->getMapping(); $joinTable = $mapping->joinTable; @@ -87,11 +87,11 @@ class ManyToManyPersister extends AbstractCollectionPersister * @internal Order of the parameters must be the same as the order of the columns in * _getInsertRowSql. */ - protected function _getInsertRowSqlParameters(PersistentCollection $coll, $element) + protected function _getInsertRowSQLParameters(PersistentCollection $coll, $element) { return $this->_collectJoinTableColumnParameters($coll, $element); } - + /** * Collects the parameters for inserting/deleting on the join table in the order * of the join table columns as specified in ManyToManyMapping#joinTableColumns. @@ -105,15 +105,15 @@ class ManyToManyPersister extends AbstractCollectionPersister $params = array(); $mapping = $coll->getMapping(); $isComposite = count($mapping->joinTableColumns) > 2; - + $identifier1 = $this->_uow->getEntityIdentifier($coll->getOwner()); $identifier2 = $this->_uow->getEntityIdentifier($element); - + if ($isComposite) { $class1 = $this->_em->getClassMetadata(get_class($coll->getOwner())); $class2 = $coll->getTypeClass(); } - + foreach ($mapping->joinTableColumns as $joinTableColumn) { if (isset($mapping->relationToSourceKeyColumns[$joinTableColumn])) { if ($isComposite) { @@ -138,7 +138,7 @@ class ManyToManyPersister extends AbstractCollectionPersister * * @override */ - protected function _getDeleteSql(PersistentCollection $coll) + protected function _getDeleteSQL(PersistentCollection $coll) { $mapping = $coll->getMapping(); $joinTable = $mapping->joinTable; @@ -157,7 +157,7 @@ class ManyToManyPersister extends AbstractCollectionPersister * @internal Order of the parameters must be the same as the order of the columns in * _getDeleteSql. */ - protected function _getDeleteSqlParameters(PersistentCollection $coll) + protected function _getDeleteSQLParameters(PersistentCollection $coll) { $params = array(); $mapping = $coll->getMapping(); @@ -170,7 +170,7 @@ class ManyToManyPersister extends AbstractCollectionPersister } else { $params[] = array_pop($identifier); } - + return $params; } } \ No newline at end of file diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index 2bd7925c4..efecd2a50 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -25,7 +25,7 @@ use Doctrine\ORM\PersistentCollection; /** * Persister for one-to-many collections. - * + * * IMPORTANT: * This persister is only used for uni-directional one-to-many mappings on a foreign key * (which are not yet supported). So currently this persister is not used. @@ -44,7 +44,7 @@ class OneToManyPersister extends AbstractCollectionPersister * @return string * @override */ - protected function _getDeleteRowSql(PersistentCollection $coll) + protected function _getDeleteRowSQL(PersistentCollection $coll) { $mapping = $coll->getMapping(); $targetClass = $this->_em->getClassMetadata($mapping->getTargetEntityName()); @@ -67,36 +67,36 @@ class OneToManyPersister extends AbstractCollectionPersister return array("UPDATE $table SET $setClause WHERE $whereClause", $this->_uow->getEntityIdentifier($element)); } - protected function _getInsertRowSql(PersistentCollection $coll) + protected function _getInsertRowSQL(PersistentCollection $coll) { return "UPDATE xxx SET foreign_key = yyy WHERE foreign_key = zzz"; } /* Not used for OneToManyPersister */ - protected function _getUpdateRowSql(PersistentCollection $coll) + protected function _getUpdateRowSQL(PersistentCollection $coll) { return; } - + /** * Generates the SQL UPDATE that updates all the foreign keys to null. * * @param PersistentCollection $coll */ - protected function _getDeleteSql(PersistentCollection $coll) + protected function _getDeleteSQL(PersistentCollection $coll) { - + } - + /** * Gets the SQL parameters for the corresponding SQL statement to delete * the given collection. * * @param PersistentCollection $coll */ - protected function _getDeleteSqlParameters(PersistentCollection $coll) + protected function _getDeleteSQLParameters(PersistentCollection $coll) {} - + /** * Gets the SQL parameters for the corresponding SQL statement to insert the given * element of the given collection into the database. @@ -104,9 +104,9 @@ class OneToManyPersister extends AbstractCollectionPersister * @param PersistentCollection $coll * @param mixed $element */ - protected function _getInsertRowSqlParameters(PersistentCollection $coll, $element) + protected function _getInsertRowSQLParameters(PersistentCollection $coll, $element) {} - + /** * Gets the SQL parameters for the corresponding SQL statement to delete the given * element from the given collection. @@ -114,6 +114,6 @@ class OneToManyPersister extends AbstractCollectionPersister * @param PersistentCollection $coll * @param mixed $element */ - protected function _getDeleteRowSqlParameters(PersistentCollection $coll, $element) + protected function _getDeleteRowSQLParameters(PersistentCollection $coll, $element) {} } \ No newline at end of file From 561236bd5626d0ca99d81a12d9c80b66e54d96a1 Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Sat, 8 May 2010 14:08:18 +0200 Subject: [PATCH 09/17] [DDC-576] Fixed. --- lib/Doctrine/DBAL/Driver/Connection.php | 2 - .../DBAL/Driver/PDOMsSql/Connection.php | 29 +++++++----- lib/Doctrine/ORM/Id/IdentityGenerator.php | 31 ++++++++---- .../ORM/Id/SequenceIdentityGenerator.php | 46 ------------------ .../ORM/Mapping/ClassMetadataFactory.php | 24 ++++++---- .../ORM/Mapping/ClassMetadataInfo.php | 4 +- .../PostgreSQLIdentityStrategyTest.php | 47 +++++++++++++++++++ 7 files changed, 105 insertions(+), 78 deletions(-) delete mode 100644 lib/Doctrine/ORM/Id/SequenceIdentityGenerator.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/PostgreSQLIdentityStrategyTest.php diff --git a/lib/Doctrine/DBAL/Driver/Connection.php b/lib/Doctrine/DBAL/Driver/Connection.php index cee11f31a..4cc5776a6 100644 --- a/lib/Doctrine/DBAL/Driver/Connection.php +++ b/lib/Doctrine/DBAL/Driver/Connection.php @@ -1,7 +1,5 @@ exec('BEGIN TRANSACTION'); } + + /** + * {@inheritdoc} + */ + public function lastInsertId($name = null) + { + $stmt = $this->query('SELECT SCOPE_IDENTITY()'); + $id = $stmt->fetchColumn(); + $stmt->closeCursor(); + return $id; + } } \ No newline at end of file diff --git a/lib/Doctrine/ORM/Id/IdentityGenerator.php b/lib/Doctrine/ORM/Id/IdentityGenerator.php index 96ad08add..75da2733d 100644 --- a/lib/Doctrine/ORM/Id/IdentityGenerator.php +++ b/lib/Doctrine/ORM/Id/IdentityGenerator.php @@ -21,23 +21,36 @@ namespace Doctrine\ORM\Id; use Doctrine\ORM\EntityManager; +/** + * Id generator that obtains IDs from special "identity" columns. These are columns + * that automatically get a database-generated, auto-incremented identifier on INSERT. + * This generator obtains the last insert id after such an insert. + */ class IdentityGenerator extends AbstractIdGenerator { + /** @var string The name of the sequence to pass to lastInsertId(), if any. */ + private $_seqName; + /** - * Generates an ID for the given entity. - * - * @param object $entity - * @return integer|float - * @override + * @param string $seqName The name of the sequence to pass to lastInsertId() + * to obtain the last generated identifier within the current + * database session/connection, if any. */ - public function generate(EntityManager $em, $entity) + public function __construct($seqName = null) { - return $em->getConnection()->lastInsertId(); + $this->_seqName = $seqName; } /** - * @return boolean - * @override + * {@inheritdoc} + */ + public function generate(EntityManager $em, $entity) + { + return $em->getConnection()->lastInsertId($this->_seqName); + } + + /** + * {@inheritdoc} */ public function isPostInsertGenerator() { diff --git a/lib/Doctrine/ORM/Id/SequenceIdentityGenerator.php b/lib/Doctrine/ORM/Id/SequenceIdentityGenerator.php deleted file mode 100644 index c7158bbed..000000000 --- a/lib/Doctrine/ORM/Id/SequenceIdentityGenerator.php +++ /dev/null @@ -1,46 +0,0 @@ -. - */ - -namespace Doctrine\ORM\Id; - -use Doctrine\ORM\EntityManager; - -class SequenceIdentityGenerator extends IdentityGenerator -{ - private $_sequenceName; - - public function __construct($sequenceName) - { - $this->_sequenceName = $sequenceName; - } - - public function generate(EntityManager $em, $entity) - { - return $em->getConnection()->lastInsertId($this->_sequenceName); - } - - /** - * @return boolean - * @override - */ - public function isPostInsertGenerator() - { - return true; - } -} \ No newline at end of file diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index 1dc812cd7..fb3a09906 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -19,8 +19,10 @@ namespace Doctrine\ORM\Mapping; -use Doctrine\ORM\ORMException, - Doctrine\DBAL\Platforms\AbstractPlatform, +use ReflectionException, + Doctrine\ORM\ORMException, + Doctrine\ORM\EntityManager, + Doctrine\DBAL\Platforms, Doctrine\ORM\Events; /** @@ -53,7 +55,7 @@ class ClassMetadataFactory * * @param $driver The metadata driver to use. */ - public function __construct(\Doctrine\ORM\EntityManager $em) + public function __construct(EntityManager $em) { $this->_em = $em; } @@ -94,15 +96,15 @@ class ClassMetadataFactory if ( ! $this->_initialized) { $this->_initialize(); } - + $metadata = array(); foreach ($this->_driver->getAllClassNames() as $className) { $metadata[] = $this->getMetadataFor($className); } - + return $metadata; } - + /** * Lazy initialization of this stuff, especially the metadata driver, * since these are not needed at all when a metadata cache is active. @@ -252,7 +254,7 @@ class ClassMetadataFactory // Invoke driver try { $this->_driver->loadMetadataForClass($className, $class); - } catch(\ReflectionException $e) { + } catch(ReflectionException $e) { throw MappingException::reflectionFailure($className, $e); } @@ -376,7 +378,13 @@ class ClassMetadataFactory // Create & assign an appropriate ID generator instance switch ($class->generatorType) { case ClassMetadata::GENERATOR_TYPE_IDENTITY: - $class->setIdGenerator(new \Doctrine\ORM\Id\IdentityGenerator()); + // For PostgreSQL IDENTITY (SERIAL) we need a sequence name. It defaults to + // __seq in PostgreSQL for SERIAL columns. + // Not pretty but necessary and the simplest solution that currently works. + $seqName = $this->_targetPlatform instanceof Platforms\PostgreSQLPlatform ? + $class->table['name'] . '_' . $class->columnNames[$class->identifier[0]] . '_seq' : + null; + $class->setIdGenerator(new \Doctrine\ORM\Id\IdentityGenerator($seqName)); break; case ClassMetadata::GENERATOR_TYPE_SEQUENCE: // If there is no sequence definition yet, create a default definition diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index a6100856c..b173e395d 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -317,7 +317,7 @@ class ClassMetadataInfo * READ-ONLY: The ID generator used for generating IDs for this class. * * @var AbstractIdGenerator - * @todo Remove + * @todo Remove! */ public $idGenerator; @@ -335,6 +335,7 @@ class ClassMetadataInfo * * * @var array + * @todo Merge with tableGeneratorDefinition into generic generatorDefinition */ public $sequenceGeneratorDefinition; @@ -343,6 +344,7 @@ class ClassMetadataInfo * TABLE generation strategy. * * @var array + * @todo Merge with tableGeneratorDefinition into generic generatorDefinition */ public $tableGeneratorDefinition; diff --git a/tests/Doctrine/Tests/ORM/Functional/PostgreSQLIdentityStrategyTest.php b/tests/Doctrine/Tests/ORM/Functional/PostgreSQLIdentityStrategyTest.php new file mode 100644 index 000000000..8b4a49ccf --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/PostgreSQLIdentityStrategyTest.php @@ -0,0 +1,47 @@ +_em->getConnection()->getDatabasePlatform()->getName() != 'postgresql') { + $this->markTestSkipped('This test is special to the PostgreSQL IDENTITY key generation strategy.'); + } else { + try { + $this->_schemaTool->createSchema(array( + $this->_em->getClassMetadata('Doctrine\Tests\ORM\Functional\PostgreSQLIdentityEntity'), + )); + } catch (\Exception $e) { + // Swallow all exceptions. We do not test the schema tool here. + } + } + } + + public function testPreSavePostSaveCallbacksAreInvoked() + { + $entity = new PostgreSQLIdentityEntity(); + $entity->setValue('hello'); + $this->_em->persist($entity); + $this->_em->flush(); + $this->assertTrue(is_numeric($entity->getId())); + $this->assertTrue($entity->getId() > 0); + $this->assertTrue($this->_em->contains($entity)); + } +} + +/** @Entity */ +class PostgreSQLIdentityEntity { + /** @Id @Column(type="integer") @GeneratedValue(strategy="IDENTITY") */ + private $id; + /** @Column(type="string") */ + private $value; + public function getId() {return $this->id;} + public function getValue() {return $this->value;} + public function setValue($value) {$this->value = $value;} +} From ee04b31da3dfaebebcf92c4df27db9d14353af89 Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Sat, 8 May 2010 14:20:44 +0200 Subject: [PATCH 10/17] Included new PostgreSQL IDENTITY/SERIAL test in functional suite. --- tests/Doctrine/Tests/ORM/Functional/AllTests.php | 1 + .../Tests/ORM/Functional/PostgreSQLIdentityStrategyTest.php | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/Functional/AllTests.php b/tests/Doctrine/Tests/ORM/Functional/AllTests.php index fa301beb9..c0827bcfb 100644 --- a/tests/Doctrine/Tests/ORM/Functional/AllTests.php +++ b/tests/Doctrine/Tests/ORM/Functional/AllTests.php @@ -54,6 +54,7 @@ class AllTests $suite->addTestSuite('Doctrine\Tests\ORM\Functional\EntityRepositoryTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\IdentityMapTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\DatabaseDriverTest'); + $suite->addTestSuite('Doctrine\Tests\ORM\Functional\PostgreSQLIdentityStrategyTest'); $suite->addTest(Locking\AllTests::suite()); $suite->addTest(SchemaTool\AllTests::suite()); diff --git a/tests/Doctrine/Tests/ORM/Functional/PostgreSQLIdentityStrategyTest.php b/tests/Doctrine/Tests/ORM/Functional/PostgreSQLIdentityStrategyTest.php index 8b4a49ccf..74b6ed213 100644 --- a/tests/Doctrine/Tests/ORM/Functional/PostgreSQLIdentityStrategyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/PostgreSQLIdentityStrategyTest.php @@ -22,6 +22,12 @@ class PostgreSQLIdentityStrategyTest extends \Doctrine\Tests\OrmFunctionalTestCa } } } + + protected function tearDown() { + parent::tearDown(); + // drop sequence manually due to dependency + $this->_em->getConnection()->exec('DROP SEQUENCE postgresqlidentityentity_id_seq CASCADE'); + } public function testPreSavePostSaveCallbacksAreInvoked() { From dc3844e167b129a7d3987e6c1294c3e7607b27c6 Mon Sep 17 00:00:00 2001 From: Christian Heinrich Date: Sat, 8 May 2010 00:59:21 +0200 Subject: [PATCH 11/17] Fixed #DDC-571 --- lib/Doctrine/DBAL/Types/IntegerType.php | 6 +++--- tests/Doctrine/Tests/DBAL/Types/IntegerTest.php | 8 ++++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/DBAL/Types/IntegerType.php b/lib/Doctrine/DBAL/Types/IntegerType.php index abd946c51..c790ab130 100644 --- a/lib/Doctrine/DBAL/Types/IntegerType.php +++ b/lib/Doctrine/DBAL/Types/IntegerType.php @@ -25,7 +25,7 @@ use Doctrine\DBAL\Platforms\AbstractPlatform; /** * Type that maps an SQL INT to a PHP integer. - * + * * @author Roman Borschel * @since 2.0 */ @@ -43,9 +43,9 @@ class IntegerType extends Type public function convertToPHPValue($value, AbstractPlatform $platform) { - return (int) $value; + return (null === $value) ? null : (int) $value; } - + public function getBindingType() { return \PDO::PARAM_INT; diff --git a/tests/Doctrine/Tests/DBAL/Types/IntegerTest.php b/tests/Doctrine/Tests/DBAL/Types/IntegerTest.php index 6d0efc5ed..f03e400ba 100644 --- a/tests/Doctrine/Tests/DBAL/Types/IntegerTest.php +++ b/tests/Doctrine/Tests/DBAL/Types/IntegerTest.php @@ -6,7 +6,7 @@ use Doctrine\DBAL\Types\Type; use Doctrine\Tests\DBAL\Mocks; require_once __DIR__ . '/../../TestInit.php'; - + class IntegerTest extends \Doctrine\Tests\DbalTestCase { protected @@ -19,10 +19,14 @@ class IntegerTest extends \Doctrine\Tests\DbalTestCase $this->_type = Type::getType('integer'); } - public function testDecimalConvertsToPHPValue() + public function testIntegerConvertsToPHPValue() { $this->assertTrue( is_integer($this->_type->convertToPHPValue('1', $this->_platform)) ); + + $this->assertTrue( + is_null($this->_type->convertToPHPValue(null, $this->_platform)) + ); } } \ No newline at end of file From 59d4e0c8e7744329cece96bd40ed4aa0e6773dd5 Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Sat, 8 May 2010 17:01:20 +0200 Subject: [PATCH 12/17] [DDC-481] Fixed. --- .../ORM/Mapping/ClassMetadataFactory.php | 4 ++-- .../ORM/Mapping/ClassMetadataInfo.php | 22 +++++++++++-------- .../ORM/Mapping/Driver/AnnotationDriver.php | 15 +++++-------- lib/Doctrine/ORM/Mapping/Driver/PHPDriver.php | 2 -- lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php | 9 +++++--- .../ORM/Mapping/Driver/YamlDriver.php | 22 +++++++++---------- .../ORM/Mapping/ClassMetadataFactoryTest.php | 3 +++ 7 files changed, 39 insertions(+), 38 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index fb3a09906..6429989cd 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -277,9 +277,9 @@ class ClassMetadataFactory } else { $this->_completeIdGeneratorMapping($class); } - + if ($parent && $parent->isInheritanceTypeSingleTable()) { - $class->setTableName($parent->getTableName()); + $class->setPrimaryTable($parent->table); } $class->setParentClasses($visited); diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index b173e395d..8a016fd47 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -903,8 +903,8 @@ class ClassMetadataInfo /** * Sets the name of the primary table the class is mapped to. * - * @param string $tableName The table name. - * @deprecated + * @param string $tableName The table name. + * @deprecated Use {@link setPrimaryTable}. */ public function setTableName($tableName) { @@ -912,18 +912,22 @@ class ClassMetadataInfo } /** - * Sets the primary table definition. The provided array must have the + * Sets the primary table definition. The provided array supports the * following structure: * - * name => - * schema => - * catalog => + * name => (optional, defaults to class name) + * indexes => array of indexes (optional) + * uniqueConstraints => array of constraints (optional) * - * @param array $primaryTableDefinition + * @param array $table */ - public function setPrimaryTable(array $primaryTableDefinition) + public function setPrimaryTable(array $table) { - $this->table = $primaryTableDefinition; + if (isset($table['name']) && $table['name'][0] == '`') { + $table['name'] = trim($table['name'], '`'); + $table['quoted'] = true; + } + $this->table = $table; } /** diff --git a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php index e44af01cf..6a61772a7 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php @@ -1,7 +1,5 @@ - * @author Guilherme Blanco - * @author Jonathan H. Wage - * @author Roman Borschel + * @since 2.0 + * @author Benjamin Eberlei + * @author Guilherme Blanco + * @author Jonathan H. Wage + * @author Roman Borschel */ class AnnotationDriver implements Driver { diff --git a/lib/Doctrine/ORM/Mapping/Driver/PHPDriver.php b/lib/Doctrine/ORM/Mapping/Driver/PHPDriver.php index 99b8a145a..ff86597f6 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/PHPDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/PHPDriver.php @@ -1,7 +1,5 @@ attributes + $table = array(); if (isset($xmlRoot['table'])) { - $metadata->table['name'] = (string)$xmlRoot['table']; + $table['name'] = (string)$xmlRoot['table']; } - + $metadata->setPrimaryTable($table); + + /* not implemented specially anyway. use table = schema.table if (isset($xmlRoot['schema'])) { $metadata->table['schema'] = (string)$xmlRoot['schema']; - } + }*/ if (isset($xmlRoot['inheritance-type'])) { $inheritanceType = (string)$xmlRoot['inheritance-type']; diff --git a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php index 873f9b21f..05a3fe125 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php @@ -1,7 +1,5 @@ - * @author Guilherme Blanco - * @author Jonathan H. Wage - * @author Roman Borschel + * @since 2.0 + * @author Benjamin Eberlei + * @author Guilherme Blanco + * @author Jonathan H. Wage + * @author Roman Borschel */ class YamlDriver extends AbstractFileDriver { @@ -61,13 +56,16 @@ class YamlDriver extends AbstractFileDriver } // Evaluate root level properties + $table = array(); if (isset($element['table'])) { - $metadata->table['name'] = $element['table']; + $table['name'] = $element['table']; } + $metadata->setPrimaryTable($table); + /* not implemented specially anyway. use table = schema.table if (isset($element['schema'])) { $metadata->table['schema'] = $element['schema']; - } + }*/ if (isset($element['inheritanceType'])) { $metadata->setInheritanceType(constant('Doctrine\ORM\Mapping\ClassMetadata::INHERITANCE_TYPE_' . strtoupper($element['inheritanceType']))); diff --git a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php index fcce4dff8..5236bdf91 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php @@ -26,6 +26,7 @@ class ClassMetadataFactoryTest extends \Doctrine\Tests\OrmTestCase // Self-made metadata $cm1 = new ClassMetadata('Doctrine\Tests\ORM\Mapping\TestEntity1'); + $cm1->setPrimaryTable(array('name' => '`group`')); // Add a mapped field $cm1->mapField(array('fieldName' => 'name', 'type' => 'varchar')); // Add a mapped field @@ -54,6 +55,8 @@ class ClassMetadataFactoryTest extends \Doctrine\Tests\OrmTestCase // Go $cm1 = $cmf->getMetadataFor('Doctrine\Tests\ORM\Mapping\TestEntity1'); + $this->assertEquals('group', $cm1->table['name']); + $this->assertTrue($cm1->table['quoted']); $this->assertEquals(array(), $cm1->parentClasses); $this->assertTrue($cm1->hasField('name')); $this->assertEquals(ClassMetadata::GENERATOR_TYPE_SEQUENCE, $cm1->generatorType); From f619a15a63eebb06e014bbc50fbb78c177e42476 Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Mon, 10 May 2010 23:51:56 +0200 Subject: [PATCH 13/17] Removed EntityTransaction until it has a real purpose. Added the affected entity to OptimisticLockException. Updated functional optimistic locking tests accordingly. --- lib/Doctrine/DBAL/Driver.php | 19 +- lib/Doctrine/DBAL/Driver/Connection.php | 2 +- lib/Doctrine/DBAL/Driver/IBMDB2/DB2Driver.php | 11 +- lib/Doctrine/DBAL/Driver/PDOMySql/Driver.php | 2 - lib/Doctrine/DBAL/Driver/PDOSqlite/Driver.php | 2 - lib/Doctrine/DBAL/DriverManager.php | 2 - lib/Doctrine/ORM/EntityManager.php | 39 ++--- lib/Doctrine/ORM/EntityTransaction.php | 163 ------------------ lib/Doctrine/ORM/ORMException.php | 4 +- lib/Doctrine/ORM/OptimisticLockException.php | 24 ++- .../ORM/Persisters/BasicEntityPersister.php | 2 +- lib/Doctrine/ORM/UnitOfWork.php | 29 ++-- .../Tests/DBAL/Functional/ConnectionTest.php | 2 +- .../ORM/Functional/DetachedEntityTest.php | 40 ++++- .../ORM/Functional/Locking/OptimisticTest.php | 65 ++++--- 15 files changed, 161 insertions(+), 245 deletions(-) delete mode 100644 lib/Doctrine/ORM/EntityTransaction.php diff --git a/lib/Doctrine/DBAL/Driver.php b/lib/Doctrine/DBAL/Driver.php index 535601e0f..4933b96d3 100644 --- a/lib/Doctrine/DBAL/Driver.php +++ b/lib/Doctrine/DBAL/Driver.php @@ -1,4 +1,21 @@ . + */ namespace Doctrine\DBAL; @@ -51,5 +68,5 @@ interface Driver * @param Doctrine\DBAL\Connection $conn * @return string $database */ - public function getDatabase(\Doctrine\DBAL\Connection $conn); + public function getDatabase(Connection $conn); } \ No newline at end of file diff --git a/lib/Doctrine/DBAL/Driver/Connection.php b/lib/Doctrine/DBAL/Driver/Connection.php index ef4183286..4cc5776a6 100644 --- a/lib/Doctrine/DBAL/Driver/Connection.php +++ b/lib/Doctrine/DBAL/Driver/Connection.php @@ -34,7 +34,7 @@ interface Connection function quote($input, $type=\PDO::PARAM_STR); function exec($statement); function lastInsertId($name = null); - function beginTransaction(); + function beginTransaction(); function commit(); function rollBack(); function errorCode(); diff --git a/lib/Doctrine/DBAL/Driver/IBMDB2/DB2Driver.php b/lib/Doctrine/DBAL/Driver/IBMDB2/DB2Driver.php index 6209d5ffd..b32dcbd47 100644 --- a/lib/Doctrine/DBAL/Driver/IBMDB2/DB2Driver.php +++ b/lib/Doctrine/DBAL/Driver/IBMDB2/DB2Driver.php @@ -1,7 +1,5 @@ + * @since 2.0 + * @author Benjamin Eberlei */ class DB2Driver implements Driver { diff --git a/lib/Doctrine/DBAL/Driver/PDOMySql/Driver.php b/lib/Doctrine/DBAL/Driver/PDOMySql/Driver.php index 37beb01e4..71a7f9f27 100644 --- a/lib/Doctrine/DBAL/Driver/PDOMySql/Driver.php +++ b/lib/Doctrine/DBAL/Driver/PDOMySql/Driver.php @@ -1,7 +1,5 @@ getProxyDir(), $config->getProxyNamespace(), $config->getAutoGenerateProxyClasses()); - $this->_transaction = new EntityTransaction($this); } /** @@ -152,26 +144,17 @@ class EntityManager return $this->_metadataFactory; } - /** - * Gets the ORM Transaction instance. - * - * @return Doctrine\ORM\EntityTransaction - */ - public function getTransaction() - { - return $this->_transaction; - } - /** * Gets an ExpressionBuilder used for object-oriented construction of query expressions. * * Example: * - * [php] + * * $qb = $em->createQueryBuilder(); * $expr = $em->getExpressionBuilder(); * $qb->select('u')->from('User', 'u') * ->where($expr->orX($expr->eq('u.id', 1), $expr->eq('u.id', 2))); + * * * @return ExpressionBuilder */ @@ -185,29 +168,32 @@ class EntityManager /** * Starts a transaction on the underlying database connection. + * + * @deprecated Use {@link getConnection}.beginTransaction(). */ public function beginTransaction() { - $this->getTransaction()->begin(); + $this->_conn->beginTransaction(); } /** * Commits a transaction on the underlying database connection. + * + * @deprecated Use {@link getConnection}.commit(). */ public function commit() { - $this->getTransaction()->commit(); + $this->_conn->commit(); } /** - * Performs a rollback on the underlying database connection and closes the - * EntityManager as it may now be in a corrupted state. + * Performs a rollback on the underlying database connection. * - * @return boolean TRUE on success, FALSE on failure + * @deprecated Use {@link getConnection}.rollback(). */ public function rollback() { - return $this->getTransaction()->rollback(); + $this->_conn->rollback(); } /** @@ -288,6 +274,9 @@ class EntityManager * Flushes all changes to objects that have been queued up to now to the database. * This effectively synchronizes the in-memory state of managed objects with the * database. + * + * @throws Doctrine\ORM\OptimisticLockException If a version check on an entity that + * makes use of optimistic locking fails. */ public function flush() { diff --git a/lib/Doctrine/ORM/EntityTransaction.php b/lib/Doctrine/ORM/EntityTransaction.php deleted file mode 100644 index 40bcd8a13..000000000 --- a/lib/Doctrine/ORM/EntityTransaction.php +++ /dev/null @@ -1,163 +0,0 @@ -. - */ - -namespace Doctrine\ORM; - -use Doctrine\DBAL\Transaction; - -/** - * The Transaction class is the central access point to ORM Transaction functionality. - * - * @license http://www.opensource.org/licenses/lgpl-license.php LGPL - * @link www.doctrine-project.org - * @since 2.0 - * @version $Revision$ - * @author Benjamin Eberlei - * @author Guilherme Blanco - * @author Jonathan Wage - * @author Roman Borschel - */ -final class EntityTransaction -{ - /** - * The wrapped ORM EntityManager. - * - * @var Doctrine\ORM\EntityManager - */ - private $_em; - - /** - * The database connection used by the EntityManager. - * - * @var Doctrine\DBAL\Connection - */ - private $_conn; - - /** - * Constructor. - * - * @param Transaction $transaction - */ - public function __construct(EntityManager $em) - { - $this->_em = $em; - $this->_conn = $em->getConnection(); - } - - /** - * Checks whether a transaction is currently active. - * - * @return boolean TRUE if a transaction is currently active, FALSE otherwise. - */ - public function isTransactionActive() - { - return $this->_conn->isTransactionActive(); - } - - /** - * Sets the transaction isolation level. - * - * @param integer $level The level to set. - */ - public function setTransactionIsolation($level) - { - return $this->_conn->setTransactionIsolation($level); - } - - /** - * Gets the currently active transaction isolation level. - * - * @return integer The current transaction isolation level. - */ - public function getTransactionIsolation() - { - return $this->_conn->getTransactionIsolation(); - } - - /** - * Returns the current transaction nesting level. - * - * @return integer The nesting level. A value of 0 means there's no active transaction. - */ - public function getTransactionNestingLevel() - { - return $this->_conn->getTransactionNestingLevel(); - } - - /** - * Starts a transaction by suspending auto-commit mode. - * - * @return void - */ - public function begin() - { - $this->_conn->beginTransaction(); - } - - /** - * Commits the current transaction. - * - * @return void - * @throws Doctrine\DBAL\ConnectionException If the commit failed due to no active transaction or - * because the transaction was marked for rollback only. - */ - public function commit() - { - $this->_conn->commit(); - } - - /** - * Cancel any database changes done during the current transaction. - * - * this method can be listened with onPreTransactionRollback and onTransactionRollback - * event listener methods - * - * @return boolean TRUE on success, FALSE on failure - * @throws Doctrine\DBAL\ConnectionException If the rollback operation failed. - */ - public function rollback() - { - $this->_em->close(); - return $this->_conn->rollback(); - } - - /** - * Marks the current transaction so that the only possible - * outcome for the transaction to be rolled back. - * - * @throws Doctrine\DBAL\ConnectionException If no transaction is active. - */ - public function setRollbackOnly() - { - $this->_conn->setRollbackOnly(); - } - - /** - * Check whether the current transaction is marked for rollback only. - * - * @return boolean - * @throws Doctrine\DBAL\ConnectionException If no transaction is active. - */ - public function isRollbackOnly() - { - return $this->_conn->isRollbackOnly(); - } -} \ No newline at end of file diff --git a/lib/Doctrine/ORM/ORMException.php b/lib/Doctrine/ORM/ORMException.php index abea24363..f03f8503c 100644 --- a/lib/Doctrine/ORM/ORMException.php +++ b/lib/Doctrine/ORM/ORMException.php @@ -19,13 +19,15 @@ namespace Doctrine\ORM; +use Exception; + /** * Base exception class for all ORM exceptions. * * @author Roman Borschel * @since 2.0 */ -class ORMException extends \Exception +class ORMException extends Exception { public static function missingMappingDriverImpl() { diff --git a/lib/Doctrine/ORM/OptimisticLockException.php b/lib/Doctrine/ORM/OptimisticLockException.php index ad6cde1ea..15dd61c44 100644 --- a/lib/Doctrine/ORM/OptimisticLockException.php +++ b/lib/Doctrine/ORM/OptimisticLockException.php @@ -20,15 +20,33 @@ namespace Doctrine\ORM; /** - * OptimisticLockException + * An OptimisticLockException is thrown when a version check on an object + * that uses optimistic locking through a version field fails. * * @author Roman Borschel * @since 2.0 */ class OptimisticLockException extends ORMException { - public static function lockFailed() + private $entity; + + public function __construct($msg, $entity) { - return new self("The optimistic lock failed."); + $this->entity = $entity; + } + + /** + * Gets the entity that caused the exception. + * + * @return object + */ + public function getEntity() + { + return $this->entity; + } + + public static function lockFailed($entity) + { + return new self("The optimistic lock on an entity failed.", $entity); } } \ No newline at end of file diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 54c7e5195..376572b30 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -330,7 +330,7 @@ class BasicEntityPersister $result = $this->_conn->executeUpdate($sql, $params, $types); if ($this->_class->isVersioned && ! $result) { - throw OptimisticLockException::lockFailed(); + throw OptimisticLockException::lockFailed($entity); } } diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 4c588dec4..7336150e7 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -19,7 +19,8 @@ namespace Doctrine\ORM; -use Doctrine\Common\Collections\ArrayCollection, +use Exception, + Doctrine\Common\Collections\ArrayCollection, Doctrine\Common\Collections\Collection, Doctrine\Common\NotifyPropertyChanged, Doctrine\Common\PropertyChangedListener, @@ -276,18 +277,17 @@ class UnitOfWork implements PropertyChangedListener // Now we need a commit order to maintain referential integrity $commitOrder = $this->_getCommitOrder(); - - $tx = $this->_em->getTransaction(); - - try { - $tx->begin(); + $conn = $this->_em->getConnection(); + + $conn->beginTransaction(); + try { if ($this->_entityInsertions) { foreach ($commitOrder as $class) { $this->_executeInserts($class); } } - + if ($this->_entityUpdates) { foreach ($commitOrder as $class) { $this->_executeUpdates($class); @@ -317,11 +317,10 @@ class UnitOfWork implements PropertyChangedListener } } - $tx->commit(); - } catch (\Exception $e) { - $tx->setRollbackOnly(); - $tx->rollback(); - + $conn->commit(); + } catch (Exception $e) { + $this->_em->close(); + $conn->rollback(); throw $e; } @@ -1289,6 +1288,8 @@ class UnitOfWork implements PropertyChangedListener * * @param object $entity * @return object The managed copy of the entity. + * @throws OptimisticLockException If the entity uses optimistic locking through a version + * attribute and the version check against the managed copy fails. */ public function merge($entity) { @@ -1315,7 +1316,7 @@ class UnitOfWork implements PropertyChangedListener throw new \InvalidArgumentException('New entity detected during merge.' . ' Persist the new entity before merging.'); } - + // MANAGED entities are ignored by the merge operation if ($this->getEntityState($entity, self::STATE_DETACHED) == self::STATE_MANAGED) { $managedCopy = $entity; @@ -1343,7 +1344,7 @@ class UnitOfWork implements PropertyChangedListener $entityVersion = $class->reflFields[$class->versionField]->getValue($entity); // Throw exception if versions dont match. if ($managedCopyVersion != $entityVersion) { - throw OptimisticLockException::lockFailed(); + throw OptimisticLockException::lockFailed($entity); } } diff --git a/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php index fa6f23dce..fc231f2f3 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php +++ b/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php @@ -25,7 +25,7 @@ class ConnectionTest extends \Doctrine\Tests\DbalFunctionalTestCase $this->assertEquals(1, $this->_conn->getTransactionNestingLevel()); //no rethrow } - $this->assertTrue($this->_conn->getRollbackOnly()); + $this->assertTrue($this->_conn->isRollbackOnly()); $this->_conn->commit(); // should throw exception $this->fail('Transaction commit after failed nested transaction should fail.'); diff --git a/tests/Doctrine/Tests/ORM/Functional/DetachedEntityTest.php b/tests/Doctrine/Tests/ORM/Functional/DetachedEntityTest.php index 22f1d2072..ddb3c7fec 100644 --- a/tests/Doctrine/Tests/ORM/Functional/DetachedEntityTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/DetachedEntityTest.php @@ -84,6 +84,44 @@ class DetachedEntityTest extends \Doctrine\Tests\OrmFunctionalTestCase $phonenumbers = $user->getPhonenumbers(); $this->assertTrue($this->_em->contains($phonenumbers[0])); $this->assertTrue($this->_em->contains($phonenumbers[1])); - } + } + + /** + * @group DDC-518 + */ + /*public function testMergeDetachedEntityWithNewlyPersistentOneToOneAssoc() + { + //$this->_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); + // Create a detached user + $user = new CmsUser; + $user->name = 'Roman'; + $user->username = 'romanb'; + $user->status = 'dev'; + $this->_em->persist($user); + $this->_em->flush(); + $this->_em->clear(); + + //$address = new CmsAddress; + //$address->city = 'Berlin'; + //$address->country = 'Germany'; + //$address->street = 'Sesamestreet'; + //$address->zip = 12345; + //$address->setUser($user); + + $phone = new CmsPhonenumber(); + $phone->phonenumber = '12345'; + + $user2 = $this->_em->merge($user); + + $user2->addPhonenumber($phone); + $this->_em->persist($phone); + + //$address->setUser($user2); + //$this->_em->persist($address); + + $this->_em->flush(); + + $this->assertEquals(1,1); + }*/ } diff --git a/tests/Doctrine/Tests/ORM/Functional/Locking/OptimisticTest.php b/tests/Doctrine/Tests/ORM/Functional/Locking/OptimisticTest.php index 947afdc68..73c5cdcf1 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Locking/OptimisticTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Locking/OptimisticTest.php @@ -3,6 +3,7 @@ namespace Doctrine\Tests\ORM\Functional\Locking; use Doctrine\ORM\Mapping\ClassMetadata; +use Doctrine\ORM\OptimisticLockException; use Doctrine\Common\EventManager; use Doctrine\ORM\Mapping\ClassMetadataFactory; use Doctrine\Tests\TestUtil; @@ -37,15 +38,17 @@ class OptimisticTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->_em->flush(); $this->assertEquals(1, $test->version); + + return $test; } /** - * @expectedException Doctrine\ORM\OptimisticLockException + * @depends testJoinedChildInsertSetsInitialVersionValue */ - public function testJoinedChildFailureThrowsException() + public function testJoinedChildFailureThrowsException(OptimisticJoinedChild $child) { - $q = $this->_em->createQuery('SELECT t FROM Doctrine\Tests\ORM\Functional\Locking\OptimisticJoinedChild t WHERE t.name = :name'); - $q->setParameter('name', 'child'); + $q = $this->_em->createQuery('SELECT t FROM Doctrine\Tests\ORM\Functional\Locking\OptimisticJoinedChild t WHERE t.id = :id'); + $q->setParameter('id', $child->id); $test = $q->getSingleResult(); // Manually update/increment the version so we can try and save the same @@ -55,7 +58,11 @@ class OptimisticTest extends \Doctrine\Tests\OrmFunctionalTestCase // Now lets change a property and try and save it again $test->whatever = 'ok'; - $this->_em->flush(); + try { + $this->_em->flush(); + } catch (OptimisticLockException $e) { + $this->assertSame($test, $e->getEntity()); + } } public function testJoinedParentInsertSetsInitialVersionValue() @@ -66,15 +73,17 @@ class OptimisticTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->_em->flush(); $this->assertEquals(1, $test->version); + + return $test; } /** - * @expectedException Doctrine\ORM\OptimisticLockException + * @depends testJoinedParentInsertSetsInitialVersionValue */ - public function testJoinedParentFailureThrowsException() + public function testJoinedParentFailureThrowsException(OptimisticJoinedParent $parent) { - $q = $this->_em->createQuery('SELECT t FROM Doctrine\Tests\ORM\Functional\Locking\OptimisticJoinedParent t WHERE t.name = :name'); - $q->setParameter('name', 'parent'); + $q = $this->_em->createQuery('SELECT t FROM Doctrine\Tests\ORM\Functional\Locking\OptimisticJoinedParent t WHERE t.id = :id'); + $q->setParameter('id', $parent->id); $test = $q->getSingleResult(); // Manually update/increment the version so we can try and save the same @@ -84,7 +93,11 @@ class OptimisticTest extends \Doctrine\Tests\OrmFunctionalTestCase // Now lets change a property and try and save it again $test->name = 'WHATT???'; - $this->_em->flush(); + try { + $this->_em->flush(); + } catch (OptimisticLockException $e) { + $this->assertSame($test, $e->getEntity()); + } } public function testStandardInsertSetsInitialVersionValue() @@ -95,15 +108,17 @@ class OptimisticTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->_em->flush(); $this->assertEquals(1, $test->getVersion()); + + return $test; } /** - * @expectedException Doctrine\ORM\OptimisticLockException + * @depends testStandardInsertSetsInitialVersionValue */ - public function testStandardFailureThrowsException() + public function testStandardFailureThrowsException(OptimisticStandard $entity) { - $q = $this->_em->createQuery('SELECT t FROM Doctrine\Tests\ORM\Functional\Locking\OptimisticStandard t WHERE t.name = :name'); - $q->setParameter('name', 'test'); + $q = $this->_em->createQuery('SELECT t FROM Doctrine\Tests\ORM\Functional\Locking\OptimisticStandard t WHERE t.id = :id'); + $q->setParameter('id', $entity->id); $test = $q->getSingleResult(); // Manually update/increment the version so we can try and save the same @@ -113,7 +128,11 @@ class OptimisticTest extends \Doctrine\Tests\OrmFunctionalTestCase // Now lets change a property and try and save it again $test->name = 'WHATT???'; - $this->_em->flush(); + try { + $this->_em->flush(); + } catch (OptimisticLockException $e) { + $this->assertSame($test, $e->getEntity()); + } } public function testOptimisticTimestampSetsDefaultValue() @@ -124,15 +143,17 @@ class OptimisticTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->_em->flush(); $this->assertTrue(strtotime($test->version) > 0); + + return $test; } /** - * @expectedException Doctrine\ORM\OptimisticLockException + * @depends testOptimisticTimestampSetsDefaultValue */ - public function testOptimisticTimestampFailureThrowsException() + public function testOptimisticTimestampFailureThrowsException(OptimisticTimestamp $entity) { - $q = $this->_em->createQuery('SELECT t FROM Doctrine\Tests\ORM\Functional\Locking\OptimisticTimestamp t WHERE t.name = :name'); - $q->setParameter('name', 'Testing'); + $q = $this->_em->createQuery('SELECT t FROM Doctrine\Tests\ORM\Functional\Locking\OptimisticTimestamp t WHERE t.id = :id'); + $q->setParameter('id', $entity->id); $test = $q->getSingleResult(); $this->assertType('DateTime', $test->version); @@ -143,7 +164,11 @@ class OptimisticTest extends \Doctrine\Tests\OrmFunctionalTestCase // Try and update the record and it should throw an exception $test->name = 'Testing again'; - $this->_em->flush(); + try { + $this->_em->flush(); + } catch (OptimisticLockException $e) { + $this->assertSame($test, $e->getEntity()); + } } } From f9b53c6b5c9358413edba91078fa01b9f4391d60 Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Tue, 11 May 2010 00:03:09 +0200 Subject: [PATCH 14/17] Fixed #DDC-580 Conflicts: lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php --- lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php | 93 ++++++++++--------- 1 file changed, 47 insertions(+), 46 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php index 3f095c8a8..b038973a5 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php @@ -66,6 +66,7 @@ class XmlDriver extends AbstractFileDriver if (isset($xmlRoot['table'])) { $table['name'] = (string)$xmlRoot['table']; } + $metadata->setPrimaryTable($table); /* not implemented specially anyway. use table = schema.table @@ -111,13 +112,13 @@ class XmlDriver extends AbstractFileDriver } else { $columns = $index['columns']; } - + $metadata->table['indexes'][$index['name']] = array( 'columns' => $columns ); } } - + // Evaluate if (isset($xmlRoot->{'unique-constraints'})) { foreach ($xmlRoot->{'unique-constraints'}->{'unique-constraint'} as $unique) { @@ -142,27 +143,27 @@ class XmlDriver extends AbstractFileDriver 'fieldName' => (string)$fieldMapping['name'], 'type' => (string)$fieldMapping['type'] ); - + if (isset($fieldMapping['column'])) { $mapping['columnName'] = (string)$fieldMapping['column']; } - + if (isset($fieldMapping['length'])) { $mapping['length'] = (int)$fieldMapping['length']; } - + if (isset($fieldMapping['precision'])) { $mapping['precision'] = (int)$fieldMapping['precision']; } - + if (isset($fieldMapping['scale'])) { $mapping['scale'] = (int)$fieldMapping['scale']; } - + if (isset($fieldMapping['unique'])) { $mapping['unique'] = ((string)$fieldMapping['unique'] == "false") ? false : true; } - + if (isset($fieldMapping['options'])) { $mapping['options'] = (array)$fieldMapping['options']; } @@ -170,15 +171,15 @@ class XmlDriver extends AbstractFileDriver if (isset($fieldMapping['nullable'])) { $mapping['nullable'] = ((string)$fieldMapping['nullable'] == "false") ? false : true; } - + if (isset($fieldMapping['version']) && $fieldMapping['version']) { $metadata->setVersionMapping($mapping); } - + if (isset($fieldMapping['column-definition'])) { $mapping['columnDefinition'] = (string)$fieldMapping['column-definition']; } - + $metadata->mapField($mapping); } } @@ -190,11 +191,11 @@ class XmlDriver extends AbstractFileDriver 'fieldName' => (string)$idElement['name'], 'type' => (string)$idElement['type'] ); - + if (isset($idElement['column'])) { $mapping['columnName'] = (string)$idElement['column']; } - + $metadata->mapField($mapping); if (isset($idElement->generator)) { @@ -224,11 +225,11 @@ class XmlDriver extends AbstractFileDriver 'fieldName' => (string)$oneToOneElement['field'], 'targetEntity' => (string)$oneToOneElement['target-entity'] ); - + if (isset($oneToOneElement['fetch'])) { $mapping['fetch'] = constant('Doctrine\ORM\Mapping\AssociationMapping::FETCH_' . (string)$oneToOneElement['fetch']); } - + if (isset($oneToOneElement['mapped-by'])) { $mapping['mappedBy'] = (string)$oneToOneElement['mapped-by']; } else { @@ -236,7 +237,7 @@ class XmlDriver extends AbstractFileDriver $mapping['inversedBy'] = (string)$oneToOneElement['inversed-by']; } $joinColumns = array(); - + if (isset($oneToOneElement->{'join-column'})) { $joinColumns[] = $this->_getJoinColumnMapping($oneToOneElement->{'join-column'}); } else if (isset($oneToOneElement->{'join-columns'})) { @@ -244,10 +245,10 @@ class XmlDriver extends AbstractFileDriver $joinColumns[] = $this->_getJoinColumnMapping($joinColumnElement); } } - + $mapping['joinColumns'] = $joinColumns; } - + if (isset($oneToOneElement->cascade)) { $mapping['cascade'] = $this->_getCascadeMappings($oneToOneElement->cascade); } @@ -255,7 +256,7 @@ class XmlDriver extends AbstractFileDriver if (isset($oneToOneElement->{'orphan-removal'})) { $mapping['orphanRemoval'] = (bool)$oneToOneElement->{'orphan-removal'}; } - + $metadata->mapOneToOne($mapping); } } @@ -268,15 +269,15 @@ class XmlDriver extends AbstractFileDriver 'targetEntity' => (string)$oneToManyElement['target-entity'], 'mappedBy' => (string)$oneToManyElement['mapped-by'] ); - + if (isset($oneToManyElement['fetch'])) { $mapping['fetch'] = constant('Doctrine\ORM\Mapping\AssociationMapping::FETCH_' . (string)$oneToManyElement['fetch']); } - + if (isset($oneToManyElement->cascade)) { $mapping['cascade'] = $this->_getCascadeMappings($oneToManyElement->cascade); } - + if (isset($oneToManyElement->{'orphan-removal'})) { $mapping['orphanRemoval'] = (bool)$oneToManyElement->{'orphan-removal'}; } @@ -288,11 +289,11 @@ class XmlDriver extends AbstractFileDriver } $mapping['orderBy'] = $orderBy; } - + $metadata->mapOneToMany($mapping); } } - + // Evaluate mappings if (isset($xmlRoot->{'many-to-one'})) { foreach ($xmlRoot->{'many-to-one'} as $manyToOneElement) { @@ -300,7 +301,7 @@ class XmlDriver extends AbstractFileDriver 'fieldName' => (string)$manyToOneElement['field'], 'targetEntity' => (string)$manyToOneElement['target-entity'] ); - + if (isset($manyToOneElement['fetch'])) { $mapping['fetch'] = constant('Doctrine\ORM\Mapping\AssociationMapping::FETCH_' . (string)$manyToOneElement['fetch']); } @@ -327,15 +328,15 @@ class XmlDriver extends AbstractFileDriver if (isset($manyToOneElement->cascade)) { $mapping['cascade'] = $this->_getCascadeMappings($manyToOneElement->cascade); } - + if (isset($manyToOneElement->{'orphan-removal'})) { $mapping['orphanRemoval'] = (bool)$manyToOneElement->{'orphan-removal'}; } - + $metadata->mapManyToOne($mapping); } } - + // Evaluate mappings if (isset($xmlRoot->{'many-to-many'})) { foreach ($xmlRoot->{'many-to-many'} as $manyToManyElement) { @@ -343,11 +344,11 @@ class XmlDriver extends AbstractFileDriver 'fieldName' => (string)$manyToManyElement['field'], 'targetEntity' => (string)$manyToManyElement['target-entity'] ); - + if (isset($manyToManyElement['fetch'])) { $mapping['fetch'] = constant('Doctrine\ORM\Mapping\AssociationMapping::FETCH_' . (string)$manyToManyElement['fetch']); } - + if (isset($manyToManyElement['mapped-by'])) { $mapping['mappedBy'] = (string)$manyToManyElement['mapped-by']; } else if (isset($manyToManyElement->{'join-table'})) { @@ -363,22 +364,22 @@ class XmlDriver extends AbstractFileDriver if (isset($joinTableElement['schema'])) { $joinTable['schema'] = (string)$joinTableElement['schema']; } - + foreach ($joinTableElement->{'join-columns'}->{'join-column'} as $joinColumnElement) { $joinTable['joinColumns'][] = $this->_getJoinColumnMapping($joinColumnElement); } - + foreach ($joinTableElement->{'inverse-join-columns'}->{'join-column'} as $joinColumnElement) { $joinTable['inverseJoinColumns'][] = $this->_getJoinColumnMapping($joinColumnElement); } - + $mapping['joinTable'] = $joinTable; } - + if (isset($manyToManyElement->cascade)) { $mapping['cascade'] = $this->_getCascadeMappings($manyToManyElement->cascade); } - + if (isset($manyToManyElement->{'orphan-removal'})) { $mapping['orphanRemoval'] = (bool)$manyToManyElement->{'orphan-removal'}; } @@ -390,7 +391,7 @@ class XmlDriver extends AbstractFileDriver } $mapping['orderBy'] = $orderBy; } - + $metadata->mapManyToMany($mapping); } } @@ -406,7 +407,7 @@ class XmlDriver extends AbstractFileDriver /** * Constructs a joinColumn mapping array based on the information * found in the given SimpleXMLElement. - * + * * @param $joinColumnElement The XML element. * @return array The mapping array. */ @@ -416,19 +417,19 @@ class XmlDriver extends AbstractFileDriver 'name' => (string)$joinColumnElement['name'], 'referencedColumnName' => (string)$joinColumnElement['referenced-column-name'] ); - + if (isset($joinColumnElement['unique'])) { $joinColumn['unique'] = ((string)$joinColumnElement['unique'] == "false") ? false : true; } - + if (isset($joinColumnElement['nullable'])) { $joinColumn['nullable'] = ((string)$joinColumnElement['nullable'] == "false") ? false : true; } - + if (isset($joinColumnElement['on-delete'])) { $joinColumn['onDelete'] = (string)$joinColumnElement['on-delete']; } - + if (isset($joinColumnElement['on-update'])) { $joinColumn['onUpdate'] = (string)$joinColumnElement['on-update']; } @@ -436,13 +437,13 @@ class XmlDriver extends AbstractFileDriver if (isset($joinColumnElement['column-definition'])) { $joinColumn['columnDefinition'] = (string)$joinColumnElement['column-definition']; } - + return $joinColumn; } - + /** * Gathers a list of cascade options found in the given cascade element. - * + * * @param $cascadeElement The cascade element. * @return array The list of cascade options. */ @@ -459,7 +460,7 @@ class XmlDriver extends AbstractFileDriver } return $cascades; } - + /** * {@inheritdoc} */ @@ -474,7 +475,7 @@ class XmlDriver extends AbstractFileDriver $result[$entityName] = $entityElement; } } else if (isset($xmlElement->{'mapped-superclass'})) { - foreach ($xmlElement->{'mapped-superclass'} as $mapperSuperClass) { + foreach ($xmlElement->{'mapped-superclass'} as $mappedSuperClass) { $className = (string)$mappedSuperClass['name']; $result[$className] = $mappedSuperClass; } From f2213c4d005093040fbdabc95ce85c76d69beab0 Mon Sep 17 00:00:00 2001 From: Christian Heinrich Date: Mon, 10 May 2010 16:17:17 +0200 Subject: [PATCH 15/17] Fixed #DDC-578 Also added a new testcase --- lib/Doctrine/ORM/Proxy/ProxyFactory.php | 6 +++- .../Tests/Models/Forum/ForumEntry.php | 4 +++ .../ORM/Proxy/ProxyClassGeneratorTest.php | 28 +++++++++++++------ 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/lib/Doctrine/ORM/Proxy/ProxyFactory.php b/lib/Doctrine/ORM/Proxy/ProxyFactory.php index fceab4023..94da9a7d9 100644 --- a/lib/Doctrine/ORM/Proxy/ProxyFactory.php +++ b/lib/Doctrine/ORM/Proxy/ProxyFactory.php @@ -164,7 +164,11 @@ class ProxyFactory } if ($method->isPublic() && ! $method->isFinal() && ! $method->isStatic()) { - $methods .= PHP_EOL . ' public function ' . $method->getName() . '('; + $methods .= PHP_EOL . ' public function '; + if ($method->returnsReference()) { + $methods .= '&'; + } + $methods .= $method->getName() . '('; $firstParam = true; $parameterString = $argumentString = ''; diff --git a/tests/Doctrine/Tests/Models/Forum/ForumEntry.php b/tests/Doctrine/Tests/Models/Forum/ForumEntry.php index 736c2adb6..efa359b00 100644 --- a/tests/Doctrine/Tests/Models/Forum/ForumEntry.php +++ b/tests/Doctrine/Tests/Models/Forum/ForumEntry.php @@ -18,5 +18,9 @@ class ForumEntry * @Column(type="string", length=50) */ public $topic; + + public function &getTopicByReference() { + return $this->topic; + } } diff --git a/tests/Doctrine/Tests/ORM/Proxy/ProxyClassGeneratorTest.php b/tests/Doctrine/Tests/ORM/Proxy/ProxyClassGeneratorTest.php index fcf07cfa0..6257dbe70 100644 --- a/tests/Doctrine/Tests/ORM/Proxy/ProxyClassGeneratorTest.php +++ b/tests/Doctrine/Tests/ORM/Proxy/ProxyClassGeneratorTest.php @@ -28,7 +28,7 @@ class ProxyClassGeneratorTest extends \Doctrine\Tests\OrmTestCase * @var \Doctrine\ORM\Proxy\ProxyFactory */ private $_proxyFactory; - + protected function setUp() { parent::setUp(); @@ -39,7 +39,7 @@ class ProxyClassGeneratorTest extends \Doctrine\Tests\OrmTestCase // SUT $this->_proxyFactory = new ProxyFactory($this->_emMock, __DIR__ . '/generated', 'Proxies', true); } - + protected function tearDown() { foreach (new \DirectoryIterator(__DIR__ . '/generated') as $file) { @@ -55,14 +55,14 @@ class ProxyClassGeneratorTest extends \Doctrine\Tests\OrmTestCase $proxyClass = 'Proxies\DoctrineTestsModelsECommerceECommerceFeatureProxy'; $persister = $this->_getMockPersister(); $this->_uowMock->setEntityPersister('Doctrine\Tests\Models\ECommerce\ECommerceFeature', $persister); - + $proxy = $this->_proxyFactory->getProxy('Doctrine\Tests\Models\ECommerce\ECommerceFeature', $identifier); - + $persister->expects($this->atLeastOnce()) ->method('load') ->with($this->equalTo($identifier), $this->isInstanceOf($proxyClass)) ->will($this->returnValue(new \stdClass())); // fake return of entity instance - + $proxy->getDescription(); } @@ -87,14 +87,24 @@ class ProxyClassGeneratorTest extends \Doctrine\Tests\OrmTestCase $persister = $this->_getMockPersister(); $this->_uowMock->setEntityPersister('Doctrine\Tests\Models\ECommerce\ECommerceFeature', $persister); $proxy = $this->_proxyFactory->getProxy('Doctrine\Tests\Models\ECommerce\ECommerceFeature', null); - + $method = new \ReflectionMethod(get_class($proxy), 'setProduct'); $params = $method->getParameters(); - + $this->assertEquals(1, count($params)); $this->assertEquals('Doctrine\Tests\Models\ECommerce\ECommerceProduct', $params[0]->getClass()->getName()); } + /** + * Test that the proxy behaves in regard to methods like &foo() correctly + */ + public function testProxyRespectsMethodsWhichReturnValuesByReference() { + $proxy = $this->_proxyFactory->getProxy('Doctrine\Tests\Models\Forum\ForumEntry', null); + $method = new \ReflectionMethod(get_class($proxy), 'getTopicByReference'); + + $this->assertTrue($method->returnsReference()); + } + public function testCreatesAssociationProxyAsSubclassOfTheOriginalOne() { $proxyClass = 'Proxies\DoctrineTestsModelsECommerceECommerceFeatureProxy'; @@ -122,7 +132,7 @@ class ProxyClassGeneratorTest extends \Doctrine\Tests\OrmTestCase $this->_proxyFactory->generateProxyClasses(array($classMetadata)); $classCode = file_get_contents(dirname(__FILE__)."/generated/".$proxyName.".php"); - + $this->assertNotContains("class DoctrineOrmTestEntityProxy extends \\\\DoctrineOrmTestEntity", $classCode); $this->assertContains("class DoctrineOrmTestEntityProxy extends \\DoctrineOrmTestEntity", $classCode); } @@ -153,7 +163,7 @@ class ProxyClassGeneratorTest extends \Doctrine\Tests\OrmTestCase $this->setExpectedException('Doctrine\ORM\Proxy\ProxyException'); new ProxyFactory($this->_getTestEntityManager(), __DIR__ . '/generated', null); } - + protected function _getMockPersister() { $persister = $this->getMock('Doctrine\ORM\Persisters\BasicEntityPersister', array('load'), array(), '', false); From d00f674a08e900d18891d0ed5f49dff096892f45 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Tue, 11 May 2010 23:08:36 +0200 Subject: [PATCH 16/17] DDC-515 - Enhanced Validate-Schema-Command, integrated it with CLI and besides mapping<->database checks also do consistency checks of the mapping files --- bin/doctrine.php | 1 + lib/Doctrine/DBAL/Driver/.DS_Store | Bin 6148 -> 0 bytes .../Command/SchemaValidatorCommand.php | 80 +++++++ .../Console/Command/ValidateSchemaCommand.php | 51 +++-- lib/Doctrine/ORM/Tools/SchemaValidator.php | 196 ++++++++++++++++++ tests/Doctrine/Tests/ORM/Tools/AllTests.php | 1 + .../AbstractClassMetadataExporterTest.php | 11 +- .../Tests/ORM/Tools/SchemaValidatorTest.php | 74 +++++++ tools/sandbox/doctrine.php | 1 + 9 files changed, 393 insertions(+), 22 deletions(-) delete mode 100644 lib/Doctrine/DBAL/Driver/.DS_Store create mode 100644 lib/Doctrine/ORM/Tools/Console/Command/SchemaValidatorCommand.php create mode 100644 lib/Doctrine/ORM/Tools/SchemaValidator.php create mode 100644 tests/Doctrine/Tests/ORM/Tools/SchemaValidatorTest.php diff --git a/bin/doctrine.php b/bin/doctrine.php index b1a29efeb..c307cb800 100644 --- a/bin/doctrine.php +++ b/bin/doctrine.php @@ -52,6 +52,7 @@ $cli->addCommands(array( new \Doctrine\ORM\Tools\Console\Command\GenerateProxiesCommand(), new \Doctrine\ORM\Tools\Console\Command\ConvertMappingCommand(), new \Doctrine\ORM\Tools\Console\Command\RunDqlCommand(), + new \Doctrine\ORM\Tools\Console\Command\ValidateSchemaCommand(), )); $cli->run(); \ No newline at end of file diff --git a/lib/Doctrine/DBAL/Driver/.DS_Store b/lib/Doctrine/DBAL/Driver/.DS_Store deleted file mode 100644 index 5008ddfcf53c02e82d7eee2e57c38e5672ef89f6..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeH~Jr2S!425mzP>H1@V-^m;4Wg<&0T*E43hX&L&p$$qDprKhvt+--jT7}7np#A3 zem<@ulZcFPQ@L2!n>{z**++&mCkOWA81W14cNZlEfg7;MkzE(HCqgga^y>{tEnwC%0;vJ&^%eQ zLs35+`xjp>T0. +*/ + +namespace Doctrine\ORM\Tools\Console\Command; + +use Symfony\Components\Console\Input\InputArgument, + Symfony\Components\Console\Input\InputOption, + Symfony\Components\Console; + +/** + * Schema Validator Command + * + * @license http://www.opensource.org/licenses/lgpl-license.php LGPL + * @link www.doctrine-project.com + * @since 1.0 + * @version $Revision$ + * @author Benjamin Eberlei + * @author Guilherme Blanco + * @author Jonathan Wage + * @author Roman Borschel + */ +class SchemaValidatorCommand extends Console\Command\Command +{ + /** + * @see Console\Command\Command + */ + protected function configure() + { + $this + ->setName('orm:validate-schema') + ->setDescription('Validate that the mapping files.') + ->setHelp(<<getHelper('em')->getEntityManager(); + + $validator = new \Doctrine\ORM\Tools\SchemaValidator($em); + $errors = $validator->validateMapping(); + + if ($errors) { + foreach ($errors AS $className => $errorMessages) { + $output->write("The entity-class '" . $className . "' is invalid:\n"); + foreach ($errorMessages AS $errorMessage) { + $output->write('* ' . $errorMessage . "\n"); + } + $output->write("\n"); + } + } + + if (!$validator->schemaInSyncWithMetadata()) { + $output->write('The database schema is not in sync with the current mapping file.'); + } + } +} \ No newline at end of file diff --git a/lib/Doctrine/ORM/Tools/Console/Command/ValidateSchemaCommand.php b/lib/Doctrine/ORM/Tools/Console/Command/ValidateSchemaCommand.php index f3053e08a..6b391e2b8 100644 --- a/lib/Doctrine/ORM/Tools/Console/Command/ValidateSchemaCommand.php +++ b/lib/Doctrine/ORM/Tools/Console/Command/ValidateSchemaCommand.php @@ -44,35 +44,44 @@ class ValidateSchemaCommand extends Console\Command\Command */ protected function configure() { - $this->setName('orm:validate-schema') - ->setDescription('Validate that the current metadata schema is valid.'); + $this + ->setName('orm:validate-schema') + ->setDescription('Validate that the mapping files.') + ->setHelp(<<getHelper('em'); + $em = $this->getHelper('em')->getEntityManager(); - /* @var $em \Doctrine\ORM\EntityManager */ - $em = $emHelper->getEntityManager(); + $validator = new \Doctrine\ORM\Tools\SchemaValidator($em); + $errors = $validator->validateMapping(); - $metadatas = $em->getMetadataFactory()->getAllMetadata(); - - if ( ! empty($metadatas)) { - // Create SchemaTool - $tool = new \Doctrine\ORM\Tools\SchemaTool($em); - $updateSql = $tool->getUpdateSchemaSql($metadatas, false); - - if (count($updateSql) == 0) { - $output->write("[Database] OK - Metadata schema exactly matches the database schema."); - } else { - $output->write("[Database] FAIL - There are differences between metadata and database schema."); + $exit = 0; + if ($errors) { + foreach ($errors AS $className => $errorMessages) { + $output->write("[Mapping] FAIL - The entity-class '" . $className . "' mapping is invalid:\n"); + foreach ($errorMessages AS $errorMessage) { + $output->write('* ' . $errorMessage . "\n"); + } + $output->write("\n"); } - } else { - $output->write("No metadata mappings found"); + $exit += 1; } + + if (!$validator->schemaInSyncWithMetadata()) { + $output->write('[Database] FAIL - The database schema is not in sync with the current mapping file.' . "\n"); + $exit += 2; + } else { + $output->write('[Database] OK - The database schema is in sync with the mapping files.' . "\n"); + } + + exit($exit); } } \ No newline at end of file diff --git a/lib/Doctrine/ORM/Tools/SchemaValidator.php b/lib/Doctrine/ORM/Tools/SchemaValidator.php new file mode 100644 index 000000000..12f51f1ce --- /dev/null +++ b/lib/Doctrine/ORM/Tools/SchemaValidator.php @@ -0,0 +1,196 @@ +. +*/ + +namespace Doctrine\ORM\Tools; + +use Doctrine\ORM\EntityManager; +use Doctrine\ORM\Mapping\ManyToManyMapping; +use Doctrine\ORM\Mapping\OneToOneMapping; + +/** + * Performs strict validation of the mapping schema + * + * @license http://www.opensource.org/licenses/lgpl-license.php LGPL + * @link www.doctrine-project.com + * @since 1.0 + * @version $Revision$ + * @author Benjamin Eberlei + * @author Guilherme Blanco + * @author Jonathan Wage + * @author Roman Borschel + */ +class SchemaValidator +{ + /** + * @var EntityManager + */ + private $em; + + /** + * @param EntityManager $em + */ + public function __construct(EntityManager $em) + { + $this->em = $em; + } + + /** + * Checks the internal consistency of mapping files. + * + * There are several checks that can't be done at runtime or are to expensive, which can be verified + * with this command. For example: + * + * 1. Check if a relation with "mappedBy" is actually connected to that specified field. + * 2. Check if "mappedBy" and "inversedBy" are consistent to each other. + * 3. Check if "referencedColumnName" attributes are really pointing to primary key columns. + * + * @return array + */ + public function validateMapping() + { + $errors = array(); + $cmf = $this->em->getMetadataFactory(); + $classes = $cmf->getAllMetadata(); + + foreach ($classes AS $class) { + /* @var $class ClassMetadata */ + foreach ($class->associationMappings AS $fieldName => $assoc) { + $ce = array(); + if (!$cmf->hasMetadataFor($assoc->targetEntityName)) { + $ce[] = "The target entity '" . $assoc->targetEntityName . "' specified on " . $class->name . '#' . $fieldName . ' is unknown.'; + } + + if ($assoc->mappedBy && $assoc->inversedBy) { + $ce[] = "The association " . $class . "#" . $fieldName . " cannot be defined as both inverse and owning."; + } + + $targetMetadata = $cmf->getMetadataFor($assoc->targetEntityName); + + /* @var $assoc AssociationMapping */ + if ($assoc->mappedBy) { + if ($targetMetadata->hasField($assoc->mappedBy)) { + $ce[] = "The association " . $class->name . "#" . $fieldName . " refers to the owning side ". + "field " . $assoc->targetEntityName . "#" . $assoc->mappedBy . " which is not defined as association."; + } + if (!$targetMetadata->hasAssociation($assoc->mappedBy)) { + $ce[] = "The association " . $class->name . "#" . $fieldName . " refers to the owning side ". + "field " . $assoc->targetEntityName . "#" . $assoc->mappedBy . " which does not exist."; + } + + if ($targetMetadata->associationMappings[$assoc->mappedBy]->inversedBy == null) { + $ce[] = "The field " . $class->name . "#" . $fieldName . " is on the inverse side of a ". + "bi-directional relationship, but the specified mappedBy association on the target-entity ". + $assoc->targetEntityName . "#" . $assoc->mappedBy . " does not contain the required ". + "'inversedBy' attribute."; + } else if ($targetMetadata->associationMappings[$assoc->mappedBy]->inversedBy != $fieldName) { + $ce[] = "The mappings " . $class->name . "#" . $fieldName . " and " . + $assoc->targetEntityName . "#" . $assoc->mappedBy . " are ". + "incosistent with each other."; + } + } + + if ($assoc->inversedBy) { + if ($targetMetadata->hasField($assoc->inversedBy)) { + $ce[] = "The association " . $class->name . "#" . $fieldName . " refers to the inverse side ". + "field " . $assoc->targetEntityName . "#" . $assoc->inversedBy . " which is not defined as association."; + } + if (!$targetMetadata->hasAssociation($assoc->inversedBy)) { + $ce[] = "The association " . $class->name . "#" . $fieldName . " refers to the inverse side ". + "field " . $assoc->targetEntityName . "#" . $assoc->inversedBy . " which does not exist."; + } + + if ($targetMetadata->associationMappings[$assoc->mappedBy]->mappedBy == null) { + $ce[] = "The field " . $class->name . "#" . $fieldName . " is on the inverse side of a ". + "bi-directional relationship, but the specified mappedBy association on the target-entity ". + $assoc->targetEntityName . "#" . $assoc->mappedBy . " does not contain the required ". + "'inversedBy' attribute."; + } else if ($targetMetadata->associationMappings[$assoc->inversedBy]->mappedBy != $fieldName) { + $ce[] = "The mappings " . $class->name . "#" . $fieldName . " and " . + $assoc->targetEntityName . "#" . $assoc->inversedBy . " are ". + "incosistent with each other."; + } + } + + if ($assoc instanceof ManyToManyMapping && $assoc->isOwningSide) { + foreach ($assoc->joinTable['joinColumns'] AS $joinColumn) { + if (!isset($class->fieldNames[$joinColumn['referencedColumnName']])) { + $ce[] = "The referenced column name '" . $joinColumn['referencedColumnName'] . "' does not " . + "have a corresponding field with this column name on the class '" . $class->name . "'."; + break; + } + + $fieldName = $class->fieldNames[$joinColumn['referencedColumnName']]; + if (!in_array($fieldName, $class->identifier)) { + $ce[] = "The referenced column name '" . $joinColumn['referencedColumnName'] . "' " . + "has to be a primary key column."; + } + } + foreach ($assoc->joinTable['inverseJoinColumns'] AS $inverseJoinColumn) { + if (!isset($class->fieldNames[$inverseJoinColumn['referencedColumnName']])) { + $ce[] = "The referenced column name '" . $inverseJoinColumn['referencedColumnName'] . "' does not " . + "have a corresponding field with this column name on the class '" . $class->name . "'."; + break; + } + + $fieldName = $class->fieldNames[$inverseJoinColumn['referencedColumnName']]; + if (!in_array($fieldName, $class->identifier)) { + $ce[] = "The referenced column name '" . $inverseJoinColumn['referencedColumnName'] . "' " . + "has to be a primary key column."; + } + } + } else if ($assoc instanceof OneToOneMapping) { + foreach ($assoc->joinColumns AS $joinColumn) { + if (!isset($class->fieldNames[$joinColumn['referencedColumnName']])) { + $ce[] = "The referenced column name '" . $joinColumn['referencedColumnName'] . "' does not " . + "have a corresponding field with this column name on the class '" . $class->name . "'."; + break; + } + + $fieldName = $class->fieldNames[$joinColumn['referencedColumnName']]; + if (!in_array($fieldName, $class->identifier)) { + $ce[] = "The referenced column name '" . $joinColumn['referencedColumnName'] . "' " . + "has to be a primary key column."; + } + } + } + + if ($ce) { + $errors[$class->name] = $ce; + } + } + } + + return $errors; + } + + /** + * Check if the Database Schema is in sync with the current metadata state. + * + * @return bool + */ + public function schemaInSyncWithMetadata() + { + $schemaTool = new SchemaTool($this->em); + + $allMetadata = $this->em->getMetadataFactory()->getAllMetadata(); + return (count($schemaTool->getUpdateSchemaSql($allMetadata, false)) == 0); + } +} diff --git a/tests/Doctrine/Tests/ORM/Tools/AllTests.php b/tests/Doctrine/Tests/ORM/Tools/AllTests.php index a729044df..faeeb1dd0 100644 --- a/tests/Doctrine/Tests/ORM/Tools/AllTests.php +++ b/tests/Doctrine/Tests/ORM/Tools/AllTests.php @@ -27,6 +27,7 @@ class AllTests $suite->addTestSuite('Doctrine\Tests\ORM\Tools\ConvertDoctrine1SchemaTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Tools\SchemaToolTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Tools\EntityGeneratorTest'); + $suite->addTestSuite('Doctrine\Tests\ORM\Tools\SchemaValidatorTest'); return $suite; } diff --git a/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php b/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php index 3e6d90df5..f1ba8ba92 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php @@ -67,7 +67,16 @@ abstract class AbstractClassMetadataExporterTest extends \Doctrine\Tests\OrmTest protected function _createMetadataDriver($type, $path) { - $class = 'Doctrine\ORM\Mapping\Driver\\' . ucfirst($type) . 'Driver'; + $mappingDriver = array( + 'php' => 'PHPDriver', + 'annotation' => 'AnnotationDriver', + 'xml' => 'XmlDriver', + 'yaml' => 'YamlDriver', + ); + $this->assertArrayHasKey($type, $mappingDriver, "There is no metadata driver for the type '" . $type . "'."); + $driverName = $mappingDriver[$type]; + + $class = 'Doctrine\ORM\Mapping\Driver\\' . $driverName; if ($type === 'annotation') { $driver = $class::create($path); } else { diff --git a/tests/Doctrine/Tests/ORM/Tools/SchemaValidatorTest.php b/tests/Doctrine/Tests/ORM/Tools/SchemaValidatorTest.php new file mode 100644 index 000000000..64bb03f36 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Tools/SchemaValidatorTest.php @@ -0,0 +1,74 @@ +em = $this->_getTestEntityManager(); + $this->validator = new SchemaValidator($this->em); + } + + public function testCmsModelSet() + { + $this->em->getConfiguration()->getMetadataDriverImpl()->addPaths(array( + __DIR__ . "/../../Models/CMS" + )); + $this->validator->validateMapping(); + } + + public function testCompanyModelSet() + { + $this->em->getConfiguration()->getMetadataDriverImpl()->addPaths(array( + __DIR__ . "/../../Models/Company" + )); + $this->validator->validateMapping(); + } + + public function testECommerceModelSet() + { + $this->em->getConfiguration()->getMetadataDriverImpl()->addPaths(array( + __DIR__ . "/../../Models/ECommerce" + )); + $this->validator->validateMapping(); + } + + public function testForumModelSet() + { + $this->em->getConfiguration()->getMetadataDriverImpl()->addPaths(array( + __DIR__ . "/../../Models/Forum" + )); + $this->validator->validateMapping(); + } + + public function testNavigationModelSet() + { + $this->em->getConfiguration()->getMetadataDriverImpl()->addPaths(array( + __DIR__ . "/../../Models/Navigation" + )); + $this->validator->validateMapping(); + } + + public function testRoutingModelSet() + { + $this->em->getConfiguration()->getMetadataDriverImpl()->addPaths(array( + __DIR__ . "/../../Models/Routing" + )); + $this->validator->validateMapping(); + } +} \ No newline at end of file diff --git a/tools/sandbox/doctrine.php b/tools/sandbox/doctrine.php index 3b4a856de..f325e143a 100644 --- a/tools/sandbox/doctrine.php +++ b/tools/sandbox/doctrine.php @@ -36,6 +36,7 @@ $cli->addCommands(array( new \Doctrine\ORM\Tools\Console\Command\GenerateProxiesCommand(), new \Doctrine\ORM\Tools\Console\Command\ConvertMappingCommand(), new \Doctrine\ORM\Tools\Console\Command\RunDqlCommand(), + new \Doctrine\ORM\Tools\Console\Command\ValidateSchemaCommand(), )); $cli->run(); \ No newline at end of file From 57cd2e01bb8b15436f7fc83c2ba8b66c21518ef4 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Tue, 11 May 2010 23:12:26 +0200 Subject: [PATCH 17/17] DDC-515 - Fixed a notice occuring in certain scenarios of the new Validate Schema Tool --- lib/Doctrine/ORM/Tools/SchemaValidator.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Tools/SchemaValidator.php b/lib/Doctrine/ORM/Tools/SchemaValidator.php index 12f51f1ce..aae4d37d8 100644 --- a/lib/Doctrine/ORM/Tools/SchemaValidator.php +++ b/lib/Doctrine/ORM/Tools/SchemaValidator.php @@ -117,7 +117,8 @@ class SchemaValidator "field " . $assoc->targetEntityName . "#" . $assoc->inversedBy . " which does not exist."; } - if ($targetMetadata->associationMappings[$assoc->mappedBy]->mappedBy == null) { + if (isset($targetMetadata->associationMappings[$assoc->mappedBy]) && + $targetMetadata->associationMappings[$assoc->mappedBy]->mappedBy == null) { $ce[] = "The field " . $class->name . "#" . $fieldName . " is on the inverse side of a ". "bi-directional relationship, but the specified mappedBy association on the target-entity ". $assoc->targetEntityName . "#" . $assoc->mappedBy . " does not contain the required ".