FIX: Make ArrayList::limit() consistent with DataList::limit()

This makes it easier to swap one fo the other without code breaking.
Since it’s strictly a removed API, I’ve opted to throw a deprecation
note in SS4 rather than throwing an InvalidArgumentException.

Fixes #2949
This commit is contained in:
Sam Minnee 2018-10-03 16:24:02 +13:00
parent bd6c4fdda0
commit 4740346ed8
2 changed files with 37 additions and 0 deletions

View File

@ -8,6 +8,7 @@ use SilverStripe\View\ViewableData;
use ArrayIterator;
use InvalidArgumentException;
use LogicException;
use SilverStripe\Dev\Deprecation;
/**
* A list object that wraps around an array of objects or arrays.
@ -160,7 +161,29 @@ class ArrayList extends ViewableData implements SS_List, Filterable, Sortable, L
*/
public function limit($length, $offset = 0)
{
// Type checking: designed for consistency with DataList::limit()
if (!is_numeric($length) || !is_numeric($offset)) {
Deprecation::notice(
'4.3',
'Arguments to ArrayList::limit() should be numeric'
);
}
if ($length < 0 || $offset < 0) {
Deprecation::notice(
'4.3',
'Arguments to ArrayList::limit() should be positive'
);
}
if (!$length) {
if ($length === 0) {
Deprecation::notice(
'4.3',
"limit(0) is deprecated in SS4. In SS5 a limit of 0 will instead return no records."
);
}
$length = count($this->items);
}

View File

@ -7,6 +7,7 @@ use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\Filterable;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\View\ArrayData;
use SilverStripe\Dev\Deprecation;
use stdClass;
class ArrayListTest extends SapphireTest
@ -133,6 +134,19 @@ class ArrayListTest extends SapphireTest
);
}
/**
* @expectedException PHPUnit_Framework_Error
*/
public function testZeroLimit()
{
Deprecation::notification_version('4.3.0');
$list = new ArrayList([
['Key' => 1],
['Key' => 2],
]);
$list->limit(0);
}
public function testAddRemove()
{
$list = new ArrayList(