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
cakephp · May 17, 2018 · 916d025 · 916d025
1 parent ea19d0f
commit 916d025
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 13 deletions.
diff --git a/src/Database/Expression/QueryExpression.php b/src/Database/Expression/QueryExpression.php
@@ -687,36 +687,40 @@ protected function _addConditions(array $conditions, array $types)
         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;
             }
 

diff --git a/tests/TestCase/Database/Expression/QueryExpressionTest.php b/tests/TestCase/Database/Expression/QueryExpressionTest.php
@@ -222,4 +222,23 @@ public function testTypeMapUsage($method)
 
         $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);
+    }
 }