Skip to content

Commit e8e1b05

Browse files
author
dantleech
committed
Improved orderBy logic in query builder
- Order order always sanitized to uppercase -- should it be? - Validation to check that given value is one of "ASC" or "DESC" - orderBy reuses existing addOrderBy rather than copying the logic
1 parent c3f2531 commit e8e1b05

File tree

2 files changed

+29
-8
lines changed

2 files changed

+29
-8
lines changed

src/PHPCR/Util/QOM/QueryBuilder.php

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,12 @@ public function getOrderings()
193193
*/
194194
public function addOrderBy(DynamicOperandInterface $sort, $order = 'ASC')
195195
{
196+
$order = strtoupper($order);
197+
198+
if (!in_array($order, array('ASC', 'DESC'))) {
199+
throw new \InvalidArgumentException('Order must be one of "ASC" or "DESC"');
200+
}
201+
196202
$this->state = self::STATE_DIRTY;
197203
if ($order == 'DESC') {
198204
$ordering = $this->qomFactory->descending($sort);
@@ -213,15 +219,10 @@ public function addOrderBy(DynamicOperandInterface $sort, $order = 'ASC')
213219
*
214220
* @return QueryBuilder This QueryBuilder instance.
215221
*/
216-
public function orderBy(DynamicOperandInterface $sort, $order = null)
222+
public function orderBy(DynamicOperandInterface $sort, $order = 'ASC')
217223
{
218-
$this->state = self::STATE_DIRTY;
219-
if ($order == 'DESC') {
220-
$ordering = $this->qomFactory->descending($sort);
221-
} else {
222-
$ordering = $this->qomFactory->ascending($sort);
223-
}
224-
$this->orderings = array($ordering);
224+
$this->orderings = array();
225+
$this->addOrderBy($sort, $order);
225226

226227
return $this;
227228
}

tests/PHPCR/Tests/Util/QOM/QueryBuilderTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,26 @@ public function testAddOrderBy()
3737
$orderings = $qb->getOrderings();
3838
}
3939

40+
public function testAddOrderByLowercase()
41+
{
42+
$dynamicOperand = $this->getMock('PHPCR\Query\QOM\DynamicOperandInterface', array(), array());
43+
$qb = new QueryBuilder($this->qf);
44+
$qb->addOrderBy($dynamicOperand, 'asc');
45+
$qb->addOrderBy($dynamicOperand, 'desc');
46+
$this->assertEquals(2, count($qb->getOrderings()));
47+
$orderings = $qb->getOrderings();
48+
}
49+
50+
/**
51+
* @expectedException \InvalidArgumentException
52+
*/
53+
public function testAddOrderByInvalid()
54+
{
55+
$dynamicOperand = $this->getMock('PHPCR\Query\QOM\DynamicOperandInterface', array(), array());
56+
$qb = new QueryBuilder($this->qf);
57+
$qb->addOrderBy($dynamicOperand, 'FOO');
58+
}
59+
4060
public function testOrderBy()
4161
{
4262
$dynamicOperand = $this->getMock('PHPCR\Query\QOM\DynamicOperandInterface', array(), array());

0 commit comments

Comments
 (0)