Fixed where componentes (ie. MEMBER OF) that that are sensitive to pa…

…renthesis presence. Made OR and AND expressions smarter. Fixed related unit tests.

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

@@ -32,9 +32,9 @@
  * @author  Jonathan Wage <jonwage@gmail.com>
  * @author  Roman Borschel <roman@code-factory.org>
  */
-class Andx extends Base
+class Andx extends Composite
 {
-    protected $_separator = ') AND (';
+    protected $_separator = ' AND ';
     protected $_allowedClasses = array(
         'Doctrine\ORM\Query\Expr\Comparison',
         'Doctrine\ORM\Query\Expr\Func',

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

@@ -39,7 +39,7 @@ abstract class Base
     protected $_postSeparator = ')';
     protected $_allowedClasses = array();
 
-    private $_parts = array();
+    protected $_parts = array();
 
     public function __construct($args = array())
     {

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

@@ -0,0 +1,53 @@
+<?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 and parts
+ *
+ * @license http://www.opensource.org/licenses/lgpl-license.php LGPL
+ * @link    www.doctrine-project.org
+ * @since   2.0
+ * @version $Revision$
+ * @author  Guilherme Blanco <guilhermeblanco@hotmail.com>
+ * @author  Jonathan Wage <jonwage@gmail.com>
+ * @author  Roman Borschel <roman@code-factory.org>
+ */
+class Composite extends Base
+{
+    public function __toString()
+    {
+        if ($this->count() === 1) {
+            return (string) $this->_parts[0];
+        }
+
+        $components = array();
+
+        foreach ($this->_parts as $part) {
+            $components[] = (is_object($part) && $part instanceof self && $part->count() > 1)
+                ? $this->_preSeparator . ((string) $part) . $this->_postSeparator
+                : ((string) $part);
+        }
+
+        return implode($this->_separator, $components);
+    }
+}

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

@@ -32,9 +32,9 @@
  * @author  Jonathan Wage <jonwage@gmail.com>
  * @author  Roman Borschel <roman@code-factory.org>
  */
-class Orx extends Base
+class Orx extends Composite
 {
-    protected $_separator = ') OR (';
+    protected $_separator = ' OR ';
     protected $_allowedClasses = array(
         'Doctrine\ORM\Query\Expr\Andx',
         'Doctrine\ORM\Query\Expr\Comparison',

lib/Doctrine/ORM/QueryBuilder.php

@@ -629,7 +629,7 @@ public function set($key, $value)
      */
     public function where($predicates)
     {
-        if ( ! (func_num_args() == 1 && ($predicates instanceof Expr\Andx || $predicates instanceof Expr\Orx))) {
+        if ( ! (func_num_args() == 1 && $predicates instanceof Expr\Composite)) {
             $predicates = new Expr\Andx(func_get_args());
         }
 

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

@@ -111,20 +111,20 @@ public function testNotExpr()
 
     public function testAndExpr()
     {
-        $this->assertEquals('(1 = 1) AND (2 = 2)', (string) $this->_expr->andx((string) $this->_expr->eq(1, 1), (string) $this->_expr->eq(2, 2)));
+        $this->assertEquals('1 = 1 AND 2 = 2', (string) $this->_expr->andx((string) $this->_expr->eq(1, 1), (string) $this->_expr->eq(2, 2)));
     }
 
     public function testIntelligentParenthesisPreventionAndExpr()
     {
         $this->assertEquals(
-            '(1 = 1) AND (2 = 2)',
+            '1 = 1 AND 2 = 2',
             (string) $this->_expr->andx($this->_expr->orx($this->_expr->andx($this->_expr->eq(1, 1))), (string) $this->_expr->eq(2, 2))
         );
     }
 
     public function testOrExpr()
     {
-        $this->assertEquals('(1 = 1) OR (2 = 2)', (string) $this->_expr->orx((string) $this->_expr->eq(1, 1), (string) $this->_expr->eq(2, 2)));
+        $this->assertEquals('1 = 1 OR 2 = 2', (string) $this->_expr->orx((string) $this->_expr->eq(1, 1), (string) $this->_expr->eq(2, 2)));
     }
 
     public function testAbsExpr()
@@ -296,7 +296,7 @@ public function testAndxOrxExpr()
         $orExpr->add($andExpr);
         $orExpr->add($this->_expr->eq(1, 1));
 
-        $this->assertEquals('((1 = 1) AND (1 < 5)) OR (1 = 1)', (string) $orExpr);
+        $this->assertEquals('(1 = 1 AND 1 < 5) OR 1 = 1', (string) $orExpr);
     }
 
     public function testOrxExpr()
@@ -305,7 +305,7 @@ public function testOrxExpr()
         $orExpr->add($this->_expr->eq(1, 1));
         $orExpr->add($this->_expr->lt(1, 5));
 
-        $this->assertEquals('(1 = 1) OR (1 < 5)', (string) $orExpr);
+        $this->assertEquals('1 = 1 OR 1 < 5', (string) $orExpr);
     }
 
     public function testOrderByCountExpr()

tests/Doctrine/Tests/ORM/QueryBuilderTest.php

@@ -162,7 +162,7 @@ public function testAndWhere()
             ->where('u.id = :uid')
             ->andWhere('u.id = :uid2');
 
-        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid) AND (u.id = :uid2)');
+        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.id = :uid AND u.id = :uid2');
     }
 
     public function testOrWhere()
@@ -173,7 +173,7 @@ public function testOrWhere()
             ->where('u.id = :uid')
             ->orWhere('u.id = :uid2');
 
-        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid) OR (u.id = :uid2)');
+        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.id = :uid OR u.id = :uid2');
     }
 
     public function testComplexAndWhereOrWhereNesting()
@@ -187,7 +187,7 @@ public function testComplexAndWhereOrWhereNesting()
            ->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)');
+        $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()
@@ -198,7 +198,7 @@ public function testAndWhereIn()
            ->where('u.id = :uid')
            ->andWhere($qb->expr()->in('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()
@@ -209,7 +209,7 @@ public function testOrWhereIn()
            ->where('u.id = :uid')
            ->orWhere($qb->expr()->in('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()
@@ -220,7 +220,7 @@ public function testAndWhereNotIn()
            ->where('u.id = :uid')
            ->andWhere($qb->expr()->notIn('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()
@@ -231,7 +231,7 @@ public function testOrWhereNotIn()
            ->where('u.id = :uid')
            ->orWhere($qb->expr()->notIn('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()
@@ -265,7 +265,7 @@ public function testAndHaving()
             ->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()
@@ -278,7 +278,7 @@ public function testOrHaving()
             ->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()
@@ -375,7 +375,7 @@ public function testMultipleWhere()
             ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u')
             ->where('u.id = :uid', 'u.id = :uid2');
 
-        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid) AND (u.id = :uid2)');
+        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.id = :uid AND u.id = :uid2');
     }
 
     public function testMultipleAndWhere()
@@ -385,7 +385,7 @@ public function testMultipleAndWhere()
             ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u')
             ->andWhere('u.id = :uid', 'u.id = :uid2');
 
-        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid) AND (u.id = :uid2)');
+        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.id = :uid AND u.id = :uid2');
     }
 
     public function testMultipleOrWhere()
@@ -395,7 +395,7 @@ public function testMultipleOrWhere()
            ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u')
            ->orWhere('u.id = :uid', $qb->expr()->eq('u.id', ':uid2'));
 
-        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.id = :uid) OR (u.id = :uid2)');
+        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.id = :uid OR u.id = :uid2');
     }
 
     public function testComplexWhere()
@@ -409,7 +409,7 @@ public function testComplexWhere()
            ->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 testWhereInWithStringLiterals()
@@ -453,7 +453,7 @@ public function testNegation()
            ->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 (NOT(u.id IN(1)))');
+        $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.id = :uid3 OR NOT(u.id IN(1))');
     }
 
     public function testSomeAllAny()
@@ -490,7 +490,7 @@ public function testMultipleIsolatedQueryConstruction()
 
         $q2 = $qb->getQuery();
 
-        $this->assertEquals('SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE (u.name = :name) AND (u.id = :id)', $q2->getDql());
+        $this->assertEquals('SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.name = :name AND u.id = :id', $q2->getDql());
         $this->assertTrue($q1 !== $q2); // two different, independent queries
         $this->assertEquals(2, count($q2->getParameters()));
         $this->assertEquals(1, count($q1->getParameters())); // $q1 unaffected