Browse Source

Fix empty QueryExpression objects being converted as SQL

Using the `or_()` function already produced and enpty expression object
when passed an empty array. It makes sense to do the same when using the
equivalent array notation

fixes #12081
Jose Lorenzo Rodriguez 7 years ago
parent
commit
1ca5c045e4

+ 17 - 13
src/Database/Expression/QueryExpression.php

@@ -687,36 +687,40 @@ class QueryExpression implements ExpressionInterface, Countable
         foreach ($conditions as $k => $c) {
             $numericKey = is_numeric($k);
 
-            if ($numericKey && empty($c)) {
-                continue;
-            }
-
             if ($this->isCallable($c)) {
                 $expr = new static([], $typeMap);
                 $c = $c($expr, $this);
             }
 
-            if ($numericKey && is_string($c)) {
-                $this->_conditions[] = $c;
+            if ($numericKey && empty($c)) {
                 continue;
             }
 
-            if ($numericKey && is_array($c) || in_array(strtolower($k), $operators)) {
-                $this->_conditions[] = new static($c, $typeMap, $numericKey ? 'AND' : $k);
+            $isArray = is_array($c);
+            $isOperator = in_array(strtolower($k), $operators);
+            $isNot = strtolower($k) === 'not';
+
+            if (($isOperator || $isNot) && ($isArray || $c instanceof Countable) && count($c) === 0) {
                 continue;
             }
 
-            if (strtolower($k) === 'not') {
-                $this->_conditions[] = new UnaryExpression('NOT', new static($c, $typeMap));
+            if ($numericKey && $c instanceof ExpressionInterface) {
+                $this->_conditions[] = $c;
                 continue;
             }
 
-            if ($c instanceof self && count($c) === 0) {
+            if ($numericKey && is_string($c)) {
+                $this->_conditions[] = $c;
                 continue;
             }
 
-            if ($numericKey && $c instanceof ExpressionInterface) {
-                $this->_conditions[] = $c;
+            if ($numericKey && $isArray || $isOperator) {
+                $this->_conditions[] = new static($c, $typeMap, $numericKey ? 'AND' : $k);
+                continue;
+            }
+
+            if ($isNot) {
+                $this->_conditions[] = new UnaryExpression('NOT', new static($c, $typeMap));
                 continue;
             }
 

+ 19 - 0
tests/TestCase/Database/Expression/QueryExpressionTest.php

@@ -222,4 +222,23 @@ class QueryExpressionTest extends TestCase
 
         $this->assertEquals('date', $type);
     }
+
+    /**
+     * Tests that creating query expressions with either the
+     * array notation or using the combinators will produce a
+     * zero-count expression object.
+     *
+     * @see https://github.com/cakephp/cakephp/issues/12081
+     * @return void
+     */
+    public function testEmptyOr()
+    {
+        $expr = new QueryExpression();
+        $expr = $expr->or_([]);
+        $expr = $expr->or_([]);
+        $this->assertCount(0, $expr);
+
+        $expr = new QueryExpression(['OR' => []]);
+        $this->assertCount(0, $expr);
+    }
 }