Skip to content
Permalink
Browse files

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
  • Loading branch information...
markstory committed Aug 7, 2015
1 parent 8f09723 commit 05b11a4a0ccb01c3b3aeb6ca49a1e912453f331a
@@ -49,7 +49,7 @@ public function sql(ValueBinder $generator)
$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);
}
@@ -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);
}
}
}
@@ -16,6 +16,7 @@
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 @@ public function orWhere($conditions, $types = [])
*
* ``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,12 +945,64 @@ public function order($fields, $overwrite = false)
}
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
@@ -1493,6 +1493,72 @@ public function testSelectOrderByString()
$this->assertEquals(['id' => 3], $result->fetch('assoc'));
}
/**
* 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

0 comments on commit 05b11a4

Please sign in to comment.
You can’t perform that action at this time.