tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

ENH Make limit method return no results when zero is provided

Browse Source
This commit is contained in:
Maxime Rainville 2023-01-19 00:09:36 +13:00
parent 4e92d25b86
commit d1e0e1e305
10 changed files with 99 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
src/ORM/ArrayList.php
View File

@ -171,23 +171,29 @@ class ArrayList extends ViewableData implements SS_List, Filterable, Sortable, L
return $result;
}
/**
* Get a sub-range of this dataobjectset as an array
* Pass null to "remove the limit" - this is for consistency with DataList::limit(null) which itself will
* call SQLSelect::setLimit(null)
*
* @param int|null $length
* @param int $offset
* @return static
*/
public function limit($length, $offset = 0)
public function limit(?int $length, int $offset = 0): static
{
if (!$length) {
$length = count($this->items ?? []);
if ($length === null) {
// If we unset the limit, we set the length to the size of the list. We still want the offset to be picked up
$length = count($this->items);
}
if ($length < 0) {
throw new InvalidArgumentException("\$length can not be negative. $length was provided.");
}
if ($offset < 0) {
throw new InvalidArgumentException("\$offset can not be negative. $offset was provided.");
}
$list = clone $this;
if ($length === 0) {
// If we set the limit to 0, we return an empty list.
$list->items = [];
} else {
$list->items = array_slice($this->items ?? [], $offset ?? 0, $length);
}
return $list;
}

18
src/ORM/DataList.php
View File

@ -287,15 +287,19 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
/**
* Return a new DataList instance with the records returned in this query
* restricted by a limit clause.
*
* @param int $limit
* @param int $offset
* @return static
*/
public function limit($limit, $offset = 0)
public function limit(?int $length, int $offset = 0): static
{
return $this->alterDataQuery(function (DataQuery $query) use ($limit, $offset) {
$query->limit($limit, $offset);
if ($length !== null && $length < 0) {
throw new InvalidArgumentException("\$length can not be negative. $length was provided.");
}
if ($offset < 0) {
throw new InvalidArgumentException("\$offset can not be negative. $offset was provided.");
}
return $this->alterDataQuery(function (DataQuery $query) use ($length, $offset) {
$query->limit($length, $offset);
});
}

6
src/ORM/DataQuery.php
View File

@ -752,12 +752,8 @@ class DataQuery
/**
* Set the limit of this query.
*
* @param int $limit
* @param int $offset
* @return $this
*/
public function limit($limit, $offset = 0)
public function limit(?int $limit, int $offset = 0): static
{
$this->query->setLimit($limit, $offset);
return $this;

8
src/ORM/Limitable.php
View File

@ -20,9 +20,9 @@ interface Limitable extends SS_List
* If $offset is specified, then that many records at the beginning of the list will be skipped.
* This matches the behaviour of the SQL LIMIT clause.
*
* @param int $limit
* @param int $offset
* @return static
* If `$length` is null, then no limit is applied. If `$length` is 0, then an empty list is returned.
*
* @throws InvalidArgumentException if $length or offset are negative
*/
public function limit($limit, $offset = 0);
public function limit(?int $length, int $offset = 0): Limitable;
}

17
src/ORM/ListDecorator.php
View File

@ -17,21 +17,16 @@ abstract class ListDecorator extends ViewableData implements SS_List, Sortable,
/**
* @var SS_List
*/
protected $list;
protected SS_List&Sortable&Filterable&Limitable $list;
public function __construct(SS_List $list)
public function __construct(SS_List&Sortable&Filterable&Limitable $list)
{
$this->setList($list);
parent::__construct();
}
/**
* Returns the list this decorator wraps around.
*
* @return SS_List
*/
public function getList()
public function getList(): SS_List&Sortable&Filterable&Limitable
{
return $this->list;
}
@ -44,7 +39,7 @@ abstract class ListDecorator extends ViewableData implements SS_List, Sortable,
*
* @return SS_List
*/
public function setList($list)
public function setList(SS_List&Sortable&Filterable&Limitable $list): self
{
$this->list = $list;
$this->failover = $this->list;
@ -246,9 +241,9 @@ abstract class ListDecorator extends ViewableData implements SS_List, Sortable,
return $output;
}
public function limit($limit, $offset = 0)
public function limit(?int $length, int $offset = 0): SS_List&Sortable&Filterable&Limitable
{
return $this->list->limit($limit, $offset);
return $this->list->limit($length, $offset);
}
/**

2
src/ORM/Queries/SQLSelect.php
View File

@ -249,7 +249,7 @@ class SQLSelect extends SQLConditionalExpression
throw new InvalidArgumentException("SQLSelect::setLimit() only takes positive values");
}
if (is_numeric($limit) && ($limit || $offset)) {
if (is_numeric($limit)) {
$this->limit = [
'start' => (int)$offset,
'limit' => (int)$limit,

8
src/ORM/Search/SearchContext.php
View File

@ -133,7 +133,7 @@ class SearchContext
* for example "Comments__Name" instead of the filter name "Comments.Name".
* @param array|bool|string $sort Database column to sort on.
* Falls back to {@link DataObject::$default_sort} if not provided.
* @param array|bool|string $limit
* @param array|null|string $limit
* @param DataList $existingQuery
* @return DataList
* @throws Exception
@ -168,7 +168,7 @@ class SearchContext
* Prepare the query to begin searching
*
* @param array|bool|string $sort Database column to sort on.
* @param array|bool|string $limit
* @param array|null|string $limit
*/
private function prepareQuery($sort, $limit, ?DataList $existingQuery): DataList
{
@ -320,11 +320,11 @@ class SearchContext
*
* @param array $searchParams
* @param array|bool|string $sort
* @param array|bool|string $limit
* @param array|null|string $limit
* @return DataList
* @throws Exception
*/
public function getResults($searchParams, $sort = false, $limit = false)
public function getResults($searchParams, $sort = false, $limit = null)
{
$searchParams = array_filter((array)$searchParams, [$this, 'clearEmptySearchFields']);

57
tests/php/ORM/ArrayListTest.php
View File

@ -117,34 +117,69 @@ class ArrayListTest extends SapphireTest
$this->assertEquals($list->Count(), $count);
}
public function testLimit()
public function limitDataProvider(): array
{
$list = new ArrayList(
[
$all = [ ['Key' => 1], ['Key' => 2], ['Key' => 3] ];
list($one, $two, $three) = $all;
return [
'smaller limit' => [2, 0, [$one, $two]],
'limit equal to array' => [3, 0, $all],
'limit bigger than array' => [4, 0, $all],
'zero limit' => [0, 0, []],
'false limit' => [0, 0, []],
'null limit' => [null, 0, $all],
'smaller limit with offset' => [1, 1, [$two]],
'limit to end with offset' => [2, 1, [$two, $three]],
'bigger limit with offset' => [3, 1, [$two, $three]],
'offset beyond end of list' => [4, 3, []],
'zero limit with offset' => [0, 1, []],
'null limit with offset' => [null, 2, [$three]],
];
}
/**
* @dataProvider limitDataProvider
*/
public function testLimit($length, $offset, array $expected)
{
$data = [
['Key' => 1], ['Key' => 2], ['Key' => 3]
]
];
$list = new ArrayList($data);
$this->assertEquals(
$list->limit($length, $offset)->toArray(),
$expected
);
$this->assertEquals(
$list->limit(2, 1)->toArray(),
[
['Key' => 2], ['Key' => 3]
]
$list->toArray(),
$data,
'limit is immutable and does not affect the original list'
);
}
public function testLimitNull()
public function testLimitNegative()
{
$this->expectException(\InvalidArgumentException::class, 'Calling limit with a negative length throws exception');
$list = new ArrayList(
[
['Key' => 1], ['Key' => 2], ['Key' => 3]
]
);
$this->assertEquals(
$list->limit(null, 0)->toArray(),
$list->limit(-1);
}
public function testLimitNegativeOffset()
{
$this->expectException(\InvalidArgumentException::class, 'Calling limit with a negative offset throws exception');
$list = new ArrayList(
[
['Key' => 1], ['Key' => 2], ['Key' => 3]
]
);
$list->limit(1, -1);
}
public function testAddRemove()

5
tests/php/ORM/DataListTest.php
View File

@ -134,8 +134,11 @@ class DataListTest extends SapphireTest
$check = $list->limit(1, 1);
$this->assertEquals(1, $check->count());
$check = $list->limit(0);
$this->assertEquals(0, $check->count());
$check = $list->limit(false);
$this->assertEquals(3, $check->count());
$this->assertEquals(0, $check->count());
$check = $list->limit(null);
$this->assertEquals(3, $check->count());

5
tests/php/ORM/ListDecoratorTest.php
View File

@ -243,8 +243,9 @@ class ListDecoratorTest extends SapphireTest
public function testLimit()
{
$this->list->expects($this->once())->method('limit')->with(5, 3)->willReturn('mock');
$this->assertSame('mock', $this->decorator->limit(5, 3));
$mockLimitedList = $this->list;
$this->list->expects($this->once())->method('limit')->with(5, 3)->willReturn($mockLimitedList);
$this->assertSame($mockLimitedList, $this->decorator->limit(5, 3));
}
public function testTotalItems()