[2.0] Refactored pieces of QueryBuilder, it does not use string concatenation...

[2.0] Refactored pieces of QueryBuilder, it does not use string concatenation anymore. Splitted from into from and join, allowing update and delete DQLs to be correctly built. Added missing entry set. Removed Expr\Having because it was useless. Updated docs.

lib/Doctrine/ORM/Mapping/ClassMetadata.php

@@ -384,7 +384,7 @@ final class ClassMetadata
      *
      * @var array
      */
-    //private $_tableGeneratorDefinition;
+    public $tableGeneratorDefinition;
 
     /**
      * The policy used for change-tracking on entities of this class.

lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php

@@ -194,7 +194,7 @@ class ClassMetadataFactory
                     $class->getTableGeneratorDefinition($parent->getTableGeneratorDefinition());
                 }
                 $class->setIdGeneratorType($parent->generatorType);
-                $class->setidGenerator($parent->getIdGenerator());
+                $class->setIdGenerator($parent->getIdGenerator());
             } else {
                 $this->_completeIdGeneratorMapping($class);
             }

lib/Doctrine/ORM/Query/Expr.php

@@ -73,39 +73,34 @@ class Expr
         return new Expr\GroupBy(func_get_args());
     }
 
-    public static function having($having = null)
-    {
-        return new Expr\Having(func_get_args());
-    }
-
     public static function eq($x, $y)
     {
-        return new Expr\Comparison($x, '=', $y);
+        return new Expr\Comparison($x, Expr\Comparison::EQ, $y);
     }
 
     public static function neq($x, $y)
     {
-        return new Expr\Comparison($x, '<>', $y);
+        return new Expr\Comparison($x, Expr\Comparison::NEQ, $y);
     }
 
     public static function lt($x, $y)
     {
-        return new Expr\Comparison($x, '<', $y);
+        return new Expr\Comparison($x, Expr\Comparison::LT, $y);
     }
 
     public static function lte($x, $y)
     {
-        return new Expr\Comparison($x, '<=', $y);
+        return new Expr\Comparison($x, Expr\Comparison::LTE, $y);
     }
 
     public static function gt($x, $y)
     {
-        return new Expr\Comparison($x, '>', $y);
+        return new Expr\Comparison($x, Expr\Comparison::GT, $y);
     }
 
     public static function gte($x, $y)
     {
-        return new Expr\Comparison($x, '>=', $y);
+        return new Expr\Comparison($x, Expr\Comparison::GTE, $y);
     }
 
     public static function avg($x)

lib/Doctrine/ORM/Query/Expr/Andx.php

@@ -36,7 +36,7 @@ class Andx extends Base
     protected $_separator = ') AND (';
     protected $_allowedClasses = array(
         'Doctrine\ORM\Query\Expr\Comparison',
+        'Doctrine\ORM\Query\Expr\Func',
         'Doctrine\ORM\Query\Expr\Orx',
-        'Doctrine\ORM\Query\Expr\Func'
     );
 }
\ No newline at end of file

lib/Doctrine/ORM/Query/Expr/Base.php

@@ -41,6 +41,11 @@ abstract class Base
     private $_parts = array();
 
     public function __construct($args = array())
+    {
+        $this->addMultiple($args);
+    }
+    
+    public function addMultiple($args = array())
     {
         foreach ((array) $args as $arg) {
             $this->add($arg);
@@ -70,6 +75,10 @@ abstract class Base
 
     public function __toString()
     {
+        if ($this->count() == 1) {
+            return (string) $this->_parts[0];
+        }
+        
         return $this->_preSeparator . implode($this->_separator, $this->_parts) . $this->_postSeparator;
     }
 }
\ No newline at end of file

lib/Doctrine/ORM/Query/Expr/Comparison.php

@@ -32,6 +32,13 @@ namespace Doctrine\ORM\Query\Expr;
  */
 class Comparison
 {
+    const EQ  = '=';
+    const NEQ = '<>';
+    const LT  = '<';
+    const LTE = '<=';
+    const GT  = '>';
+    const GTE = '>=';
+    
     private $_leftExpr;
     private $_operator;
     private $_rightExpr;

lib/Doctrine/ORM/Query/Expr/Having.php

-<?php
-/*
- *  $Id$
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
- * This software consists of voluntary contributions made by many individuals
- * and is licensed under the LGPL. For more information, see
- * <http://www.doctrine-project.org>.
- */
-
-namespace Doctrine\ORM\Query\Expr;
-
-/**
- * Expression class for building DQL Having parts
- *
- * @author      Jonathan H. Wage <jonwage@gmail.com>
- * @author      Guilherme Blanco <guilhermeblanco@gmail.com>
- * @license     http://www.opensource.org/licenses/lgpl-license.php LGPL
- * @link        http://www.phpdoctrine.org
- * @since       2.0
- * @version     $Revision$
- */
-class Having extends Base
-{
-    protected $_preSeparator = '';
-    protected $_postSeparator = '';
-    protected $_allowedClasses = array(
-        'Doctrine\ORM\Query\Expr\Comparison',
-        'Doctrine\ORM\Query\Expr\Orx',
-        'Doctrine\ORM\Query\Expr\Func'
-    );
-}
\ No newline at end of file

lib/Doctrine/ORM/Query/Expr/Orx.php

@@ -35,8 +35,8 @@ class Orx extends Base
 {
     protected $_separator = ') OR (';
     protected $_allowedClasses = array(
-        'Doctrine\ORM\Query\Expr\Comparison',
         'Doctrine\ORM\Query\Expr\Andx',
-        'Doctrine\ORM\Query\Expr\Func'
+        'Doctrine\ORM\Query\Expr\Comparison',
+        'Doctrine\ORM\Query\Expr\Func',
     );
 }
\ No newline at end of file

lib/Doctrine/ORM/Tools/SchemaTool.php

@@ -150,6 +150,8 @@ class SchemaTool
                     $class->getQuotedTableName($this->_platform), $columns, $options));
             $processedClasses[$class->name] = true;
 
+            // TODO if we're reusing the sequence previously defined (in another model),
+            // it should not attempt to create a new sequence.
             if ($class->isIdGeneratorSequence() && $class->name == $class->rootEntityName) {
                 $seqDef = $class->getSequenceGeneratorDefinition();
                 $sequences[] = $this->_platform->getCreateSequenceSql(

tests/Doctrine/Tests/ORM/Query/ExprTest.php

@@ -75,7 +75,7 @@ class ExprTest extends \Doctrine\Tests\OrmTestCase
         $qb = $this->_em->createQueryBuilder();
         $qb->select('u')->from('User', 'u')->where('u.name = ?1');
         
-        $this->assertEquals('EXISTS(SELECT u FROM User u WHERE (u.name = ?1))', (string) Expr::exists($qb));
+        $this->assertEquals('EXISTS(SELECT u FROM User u WHERE u.name = ?1)', (string) Expr::exists($qb));
     }
 
     public function testAllExpr()
@@ -83,7 +83,7 @@ class ExprTest extends \Doctrine\Tests\OrmTestCase
         $qb = $this->_em->createQueryBuilder();
         $qb->select('u')->from('User', 'u')->where('u.name = ?1');
     
-        $this->assertEquals('ALL(SELECT u FROM User u WHERE (u.name = ?1))', (string) Expr::all($qb));
+        $this->assertEquals('ALL(SELECT u FROM User u WHERE u.name = ?1)', (string) Expr::all($qb));
     }
 
     public function testSomeExpr()
@@ -91,7 +91,7 @@ class ExprTest extends \Doctrine\Tests\OrmTestCase
         $qb = $this->_em->createQueryBuilder();
         $qb->select('u')->from('User', 'u')->where('u.name = ?1');
     
-        $this->assertEquals('SOME(SELECT u FROM User u WHERE (u.name = ?1))', (string) Expr::some($qb));
+        $this->assertEquals('SOME(SELECT u FROM User u WHERE u.name = ?1)', (string) Expr::some($qb));
     }
 
     public function testAnyExpr()
@@ -99,7 +99,7 @@ class ExprTest extends \Doctrine\Tests\OrmTestCase
         $qb = $this->_em->createQueryBuilder();
         $qb->select('u')->from('User', 'u')->where('u.name = ?1');
     
-        $this->assertEquals('ANY(SELECT u FROM User u WHERE (u.name = ?1))', (string) Expr::any($qb));
+        $this->assertEquals('ANY(SELECT u FROM User u WHERE u.name = ?1)', (string) Expr::any($qb));
     }
 
     public function testNotExpr()
@@ -107,7 +107,7 @@ class ExprTest extends \Doctrine\Tests\OrmTestCase
         $qb = $this->_em->createQueryBuilder();
         $qb->select('u')->from('User', 'u')->where('u.name = ?1');
     
-        $this->assertEquals('NOT(SELECT u FROM User u WHERE (u.name = ?1))', (string) Expr::not($qb));
+        $this->assertEquals('NOT(SELECT u FROM User u WHERE u.name = ?1)', (string) Expr::not($qb));
     }
 
     public function testAndExpr()
@@ -115,6 +115,14 @@ class ExprTest extends \Doctrine\Tests\OrmTestCase
         $this->assertEquals('(1 = 1) AND (2 = 2)', (string) Expr::andx((string) Expr::eq(1, 1), (string) Expr::eq(2, 2)));
     }
     
+    public function testIntelligentParenthesisPreventionAndExpr()
+    {
+        $this->assertEquals(
+            '(1 = 1) AND (2 = 2)', 
+            (string) Expr::andx(Expr::orx(Expr::andx(Expr::eq(1, 1))), (string) Expr::eq(2, 2))
+        );
+    }
+
     public function testOrExpr()
     {
         $this->assertEquals('(1 = 1) OR (2 = 2)', (string) Expr::orx((string) Expr::eq(1, 1), (string) Expr::eq(2, 2)));

tests/Doctrine/Tests/ORM/QueryBuilderTest.php

@@ -156,7 +156,7 @@ class QueryBuilderTest extends \Doctrine\Tests\OrmTestCase
             ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u')
             ->where('u.id = :uid');
 
-        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid)');
+        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.id = :uid');
     }
 
     public function testAndWhere()
@@ -189,7 +189,7 @@ class QueryBuilderTest extends \Doctrine\Tests\OrmTestCase
             ->where('u.id = :uid')
             ->andWhereIn('u.id', array(1, 2, 3));
 
-        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid) AND u.id IN(1, 2, 3)');
+        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid) AND (u.id IN(1, 2, 3))');
     }
 
     public function testOrWhereIn()
@@ -200,7 +200,7 @@ class QueryBuilderTest extends \Doctrine\Tests\OrmTestCase
             ->where('u.id = :uid')
             ->orWhereIn('u.id', array(1, 2, 3));
 
-        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid) OR u.id IN(1, 2, 3)');
+        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid) OR (u.id IN(1, 2, 3))');
     }
 
     public function testAndWhereNotIn()
@@ -211,7 +211,7 @@ class QueryBuilderTest extends \Doctrine\Tests\OrmTestCase
             ->where('u.id = :uid')
             ->andWhereNotIn('u.id', array(1, 2, 3));
 
-        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid) AND u.id NOT IN(1, 2, 3)');
+        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid) AND (u.id NOT IN(1, 2, 3))');
     }
 
     public function testOrWhereNotIn()
@@ -222,7 +222,7 @@ class QueryBuilderTest extends \Doctrine\Tests\OrmTestCase
             ->where('u.id = :uid')
             ->OrWhereNotIn('u.id', array(1, 2, 3));
 
-        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid) OR u.id NOT IN(1, 2, 3)');
+        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid) OR (u.id NOT IN(1, 2, 3))');
     }
 
     public function testGroupBy()
@@ -256,7 +256,7 @@ class QueryBuilderTest extends \Doctrine\Tests\OrmTestCase
             ->having('COUNT(u.id) > 1')
             ->andHaving('COUNT(u.id) < 1');
 
-        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u GROUP BY u.id HAVING COUNT(u.id) > 1 AND COUNT(u.id) < 1');
+        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u GROUP BY u.id HAVING (COUNT(u.id) > 1) AND (COUNT(u.id) < 1)');
     }
 
     public function testOrHaving()
@@ -269,7 +269,7 @@ class QueryBuilderTest extends \Doctrine\Tests\OrmTestCase
             ->andHaving('COUNT(u.id) < 1')
             ->orHaving('COUNT(u.id) > 1');
 
-        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u GROUP BY u.id HAVING COUNT(u.id) > 1 AND COUNT(u.id) < 1 OR COUNT(u.id) > 1');
+        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u GROUP BY u.id HAVING ((COUNT(u.id) > 1) AND (COUNT(u.id) < 1)) OR (COUNT(u.id) > 1)');
     }
 
     public function testOrderBy()
@@ -319,7 +319,7 @@ class QueryBuilderTest extends \Doctrine\Tests\OrmTestCase
         $qb = $this->_em->createQueryBuilder()
             ->select('u')
             ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u')
-            ->where('u.username = :username OR u.username = :username2');
+            ->where(Expr::orx('u.username = :username', 'u.username = :username2'));
 
         $qb->setParameters(array('username' => 'jwage', 'username2' => 'jonwage'));
 
@@ -390,7 +390,7 @@ class QueryBuilderTest extends \Doctrine\Tests\OrmTestCase
             ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u')
             ->where($orExpr);
 
-        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE ((u.id = :uid3) OR (u.id IN(1)))');
+        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid3) OR (u.id IN(1))');
     }
 
     public function testGetEntityManager()