First pass at implementing orderAsc/orderDesc

The current order() method has a limitation which makes it impossible to
use expression objects that also have direction. These new methods make
it easier to support directed expression objects.

I've introduced a new expression object as I felt keeping
OrderByExpression simple was worth the extra bit of cost. Additionally
this makes SQL injection in the ORM internals more difficult as we rely
on the callers to provide a safe expression. I also experimented with an
array format, but felt it left us vulnerable to the issues we've
had in the past around request data manipulation.

Refs #7163

src/Database/Expression/OrderByExpression.php

@@ -49,7 +49,7 @@ class OrderByExpression extends QueryExpression
         $order = [];
         foreach ($this->_conditions as $k => $direction) {
             if ($direction instanceof ExpressionInterface) {
-                $direction = sprintf('(%s)', $direction->sql($generator));
+                $direction = $direction->sql($generator);
             }
             $order[] = is_numeric($k) ? $direction : sprintf('%s %s', $k, $direction);
         }

src/Database/Expression/OrderClauseExpression.php

@@ -0,0 +1,75 @@
+<?php
+/**
+ * CakePHP(tm) : Rapid Development Framework (http://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ * @link          http://cakephp.org CakePHP(tm) Project
+ * @since         3.0.0
+ * @license       http://www.opensource.org/licenses/mit-license.php MIT License
+ */
+namespace Cake\Database\Expression;
+
+use Cake\Database\ExpressionInterface;
+use Cake\Database\ValueBinder;
+
+/**
+ * An expression object for complex ORDER BY clauses
+ *
+ * @internal
+ */
+class OrderClauseExpression implements ExpressionInterface
+{
+    /**
+     * The field being sorted on.
+     *
+     * @var \Cake\Database\ExpressionInterface|string
+     */
+    protected $_field;
+
+    /**
+     * The direction of sorting.
+     *
+     * @var string
+     */
+    protected $_direction;
+
+    /**
+     * Constructor
+     *
+     * @param \Cake\Database\ExpressionInterface|string $field The field to order on.
+     * @param string $direction The direction to sort on.
+     */
+    public function __construct($field, $direction)
+    {
+        $this->_field = $field;
+        $this->_direction = strtolower($direction) === 'asc' ? 'ASC' : 'DESC';
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function sql(ValueBinder $generator)
+    {
+        $field = $this->_field;
+        if ($field instanceof ExpressionInterface) {
+            $field = $field->sql($generator);
+        }
+        return sprintf("%s %s", $field, $this->_direction);
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function traverse(callable $visitor)
+    {
+        if ($this->_field instanceof ExpressionInterface) {
+            $callable($this->_field);
+            $this->_field->traverse($callable);
+        }
+    }
+}

src/Database/Query.php

@@ -16,6 +16,7 @@ namespace Cake\Database;
 
 use Cake\Database\Exception;
 use Cake\Database\Expression\OrderByExpression;
+use Cake\Database\Expression\OrderClauseExpression;
 use Cake\Database\Expression\QueryExpression;
 use Cake\Database\Expression\ValuesExpression;
 use Cake\Database\Statement\CallbackStatement;
@@ -926,6 +927,9 @@ class Query implements ExpressionInterface, IteratorAggregate
      *
      * ``ORDER BY (id %2 = 0), title ASC``
      *
+     * If you need to set complex expressions as order conditions, you
+     * should use `orderAsc()` or `orderDesc()`.
+     *
      * @param array|\Cake\Database\ExpressionInterface|string $fields fields to be added to the list
      * @param bool $overwrite whether to reset order with field list or not
      * @return $this
@@ -941,13 +945,65 @@ class Query implements ExpressionInterface, IteratorAggregate
         }
 
         if (!$this->_parts['order']) {
-            $this->_parts['order'] = new OrderByExpression;
+            $this->_parts['order'] = new OrderByExpression();
         }
         $this->_conjugate('order', $fields, '', []);
         return $this;
     }
 
     /**
+     * Add an ORDER BY clause with an ASC direction.
+     *
+     * This method allows you to set complex expressions
+     * as order conditions unlike order()
+     *
+     * @param string|\Cake\Database\QueryExpression $field The field to order on.
+     * @param bool $overwrite Whether or not to reset the order clauses.
+     * @return $this
+     */
+    public function orderAsc($field, $overwrite = false)
+    {
+        if ($overwrite) {
+            $this->_parts['order'] = null;
+        }
+        if (!$field) {
+            return $this;
+        }
+
+        if (!$this->_parts['order']) {
+            $this->_parts['order'] = new OrderByExpression();
+        }
+        $this->_parts['order']->add(new OrderClauseExpression($field, 'ASC'));
+        return $this;
+    }
+
+    /**
+     * Add an ORDER BY clause with an ASC direction.
+     *
+     * This method allows you to set complex expressions
+     * as order conditions unlike order()
+     *
+     * @param string|\Cake\Database\QueryExpression $field The field to order on.
+     * @param bool $overwrite Whether or not to reset the order clauses.
+     * @return $this
+     */
+    public function orderDesc($field, $overwrite = false)
+    {
+        if ($overwrite) {
+            $this->_parts['order'] = null;
+        }
+        if (!$field) {
+            return $this;
+        }
+
+        if (!$this->_parts['order']) {
+            $this->_parts['order'] = new OrderByExpression();
+        }
+        $this->_parts['order']->add(new OrderClauseExpression($field, 'DESC'));
+        return $this;
+    }
+
+    /**
      * Adds a single or multiple fields to be used in the GROUP BY clause for this query.
      * Fields can be passed as an array of strings, array of expression
      * objects, a single expression or a single string.

tests/TestCase/Database/QueryTest.php

@@ -1494,6 +1494,72 @@ class QueryTest extends TestCase
     }
 
     /**
+     * Test orderAsc() and its input types.
+     *
+     * @return void
+     */
+    public function testSelectOrderAsc()
+    {
+        $query = new Query($this->connection);
+        $query->select(['id'])
+            ->from('articles')
+            ->orderAsc('id');
+        $result = $query->execute()->fetchAll('assoc');
+        $expected = [
+            ['id' => 1],
+            ['id' => 2],
+            ['id' => 3],
+        ];
+        $this->assertEquals($expected, $result);
+
+        $query = new Query($this->connection);
+        $query->select(['id'])
+            ->from('articles')
+            ->orderAsc($query->func()->concat(['id' => 'literal', 'test']));
+
+        $result = $query->execute()->fetchAll('assoc');
+        $expected = [
+            ['id' => 1],
+            ['id' => 2],
+            ['id' => 3],
+        ];
+        $this->assertEquals($expected, $result);
+    }
+
+    /**
+     * Test orderDesc() and its input types.
+     *
+     * @return void
+     */
+    public function testSelectOrderDesc()
+    {
+        $query = new Query($this->connection);
+        $query->select(['id'])
+            ->from('articles')
+            ->orderDesc('id');
+        $result = $query->execute()->fetchAll('assoc');
+        $expected = [
+            ['id' => 3],
+            ['id' => 2],
+            ['id' => 1],
+        ];
+        $this->assertEquals($expected, $result);
+
+        $query = new Query($this->connection);
+        $query->select(['id'])
+            ->from('articles')
+            ->orderDesc($query->func()->concat(['id' => 'literal', 'test']));
+
+        $result = $query->execute()->fetchAll('assoc');
+        $expected = [
+            ['id' => 3],
+            ['id' => 2],
+            ['id' => 1],
+        ];
+        $this->assertEquals($expected, $result);
+    }
+
+    /**
      * Tests that group by fields can be passed similar to select fields
      * and that it sends the correct query to the database
      *