From e6fee3c4f8aa6427b41b166da9f6191dddb60e13 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Wed, 23 Oct 2024 17:25:28 +1300 Subject: [PATCH] API Combine Sortable, Filterable and Limitable into SS_List --- src/Forms/GridField/GridField.php | 11 +- src/Forms/GridField/GridFieldFilterHeader.php | 7 +- src/Forms/GridField/GridFieldLazyLoader.php | 3 +- src/Forms/GridField/GridFieldPaginator.php | 7 +- .../GridField/GridFieldSortableHeader.php | 7 +- src/Model/List/ArrayList.php | 97 +++----- src/Model/List/Filterable.php | 107 --------- src/Model/List/Limitable.php | 32 --- src/Model/List/ListDecorator.php | 150 +++++++----- src/Model/List/PaginatedList.php | 2 +- src/Model/List/SS_List.php | 217 +++++++++++++++--- src/Model/List/Sortable.php | 51 ---- src/ORM/DataList.php | 109 ++++----- src/ORM/EagerLoadedList.php | 48 ++-- src/ORM/HasManyList.php | 4 +- src/ORM/ManyManyList.php | 8 +- src/ORM/ManyManyThroughList.php | 10 +- src/ORM/PolymorphicHasManyList.php | 2 +- src/ORM/Relation.php | 8 +- src/ORM/Search/BasicSearchContext.php | 14 +- src/ORM/UnsavedRelationList.php | 21 +- src/Security/Member_GroupSet.php | 2 +- tests/php/Model/List/ArrayListTest.php | 4 +- tests/php/Model/List/ListDecoratorTest.php | 69 +++--- tests/php/ORM/DataListTest.php | 11 +- tests/php/ORM/EagerLoadedListTest.php | 11 +- 26 files changed, 473 insertions(+), 539 deletions(-) delete mode 100644 src/Model/List/Filterable.php delete mode 100644 src/Model/List/Limitable.php delete mode 100644 src/Model/List/Sortable.php diff --git a/src/Forms/GridField/GridField.php b/src/Forms/GridField/GridField.php index 12b2522c087..a00aab521cf 100644 --- a/src/Forms/GridField/GridField.php +++ b/src/Forms/GridField/GridField.php @@ -22,9 +22,6 @@ use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataObjectInterface; use SilverStripe\ORM\FieldType\DBField; -use SilverStripe\Model\List\Filterable; -use SilverStripe\Model\List\Limitable; -use SilverStripe\Model\List\Sortable; use SilverStripe\Model\List\SS_List; use SilverStripe\View\HTML; use SilverStripe\Model\ModelData; @@ -86,7 +83,7 @@ class GridField extends FormField /** * Data source. * - * @var SS_List&Filterable&Sortable&Limitable + * @var SS_List */ protected $list = null; @@ -397,7 +394,7 @@ public function getCastedValue($value, $castingDefinition) /** * Set the data source. * - * @param SS_List&Filterable&Sortable&Limitable $list + * @param SS_List $list * * @return $this */ @@ -411,7 +408,7 @@ public function setList(SS_List $list) /** * Get the data source. * - * @return SS_List&Filterable&Sortable&Limitable + * @return SS_List */ public function getList() { @@ -421,7 +418,7 @@ public function getList() /** * Get the data source after applying every {@link GridField_DataManipulator} to it. * - * @return SS_List&Filterable&Sortable&Limitable + * @return SS_List */ public function getManipulatedList() { diff --git a/src/Forms/GridField/GridFieldFilterHeader.php b/src/Forms/GridField/GridFieldFilterHeader.php index 7a9b0cbd658..eb8b84a4d50 100755 --- a/src/Forms/GridField/GridFieldFilterHeader.php +++ b/src/Forms/GridField/GridFieldFilterHeader.php @@ -11,7 +11,6 @@ use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; use SilverStripe\Forms\Schema\FormSchema; -use SilverStripe\Model\List\Filterable; use SilverStripe\ORM\Search\SearchContext; use SilverStripe\Model\List\SS_List; use SilverStripe\Model\ArrayData; @@ -109,13 +108,13 @@ public function setSearchField(string $field): GridFieldFilterHeader */ protected function checkDataType($dataList) { - if ($dataList instanceof Filterable) { + if ($dataList instanceof SS_List) { return true; } else { // This will be changed to always throw an exception in a future major release. if ($this->throwExceptionOnBadDataType) { throw new LogicException( - static::class . " expects an SS_Filterable list to be passed to the GridField." + static::class . " expects an SS_List list to be passed to the GridField." ); } return false; @@ -209,7 +208,7 @@ public function getManipulatedData(GridField $gridField, SS_List $dataList) public function canFilterAnyColumns($gridField) { $list = $gridField->getList(); - if (!($list instanceof Filterable) || !$this->checkDataType($list)) { + if (!($list instanceof SS_List) || !$this->checkDataType($list)) { return false; } $modelClass = $gridField->getModelClass(); diff --git a/src/Forms/GridField/GridFieldLazyLoader.php b/src/Forms/GridField/GridFieldLazyLoader.php index 59844114e82..a6e97405e93 100755 --- a/src/Forms/GridField/GridFieldLazyLoader.php +++ b/src/Forms/GridField/GridFieldLazyLoader.php @@ -5,7 +5,6 @@ use SilverStripe\Forms\FormField; use SilverStripe\Forms\TabSet; use SilverStripe\Model\List\ArrayList; -use SilverStripe\Model\List\Filterable; use SilverStripe\Model\List\SS_List; /** @@ -28,7 +27,7 @@ public function getManipulatedData(GridField $gridField, SS_List $dataList) { // If we are lazy loading an empty the list if ($this->isLazy($gridField)) { - if ($dataList instanceof Filterable) { + if ($dataList instanceof SS_List) { // If our original list can be filtered, filter out all results. $dataList = $dataList->byIDs([-1]); } else { diff --git a/src/Forms/GridField/GridFieldPaginator.php b/src/Forms/GridField/GridFieldPaginator.php index fcda7a645e3..dd5b7497048 100755 --- a/src/Forms/GridField/GridFieldPaginator.php +++ b/src/Forms/GridField/GridFieldPaginator.php @@ -3,7 +3,6 @@ namespace SilverStripe\Forms\GridField; use SilverStripe\Core\Config\Configurable; -use SilverStripe\Model\List\Limitable; use SilverStripe\Model\List\SS_List; use SilverStripe\ORM\UnsavedRelationList; use SilverStripe\Model\ArrayData; @@ -89,13 +88,13 @@ public function getThrowExceptionOnBadDataType() */ protected function checkDataType($dataList) { - if ($dataList instanceof Limitable) { + if ($dataList instanceof SS_List) { return true; } else { // This will be changed to always throw an exception in a future major release. if ($this->throwExceptionOnBadDataType) { throw new LogicException( - static::class . " expects an SS_Limitable list to be passed to the GridField." + static::class . " expects an SS_List list to be passed to the GridField." ); } return false; @@ -183,7 +182,7 @@ public function getManipulatedData(GridField $gridField, SS_List $dataList) $startRow = 0; } - if (!($dataList instanceof Limitable) || ($dataList instanceof UnsavedRelationList)) { + if (!($dataList instanceof SS_List) || ($dataList instanceof UnsavedRelationList)) { return $dataList; } diff --git a/src/Forms/GridField/GridFieldSortableHeader.php b/src/Forms/GridField/GridFieldSortableHeader.php index b3705495ccc..fe09fb854a6 100644 --- a/src/Forms/GridField/GridFieldSortableHeader.php +++ b/src/Forms/GridField/GridFieldSortableHeader.php @@ -4,7 +4,6 @@ use SilverStripe\Forms\LiteralField; use SilverStripe\ORM\DataObjectSchema; -use SilverStripe\Model\List\Sortable; use SilverStripe\Model\List\ArrayList; use SilverStripe\Model\List\SS_List; use SilverStripe\ORM\DataObject; @@ -78,13 +77,13 @@ public function getThrowExceptionOnBadDataType() */ protected function checkDataType($dataList) { - if ($dataList instanceof Sortable) { + if ($dataList instanceof SS_List) { return true; } else { // This will be changed to always throw an exception in a future major release. if ($this->throwExceptionOnBadDataType) { throw new LogicException( - static::class . " expects an SS_Sortable list to be passed to the GridField." + static::class . " expects an SS_List list to be passed to the GridField." ); } return false; @@ -246,7 +245,7 @@ public function handleAction(GridField $gridField, $actionName, $arguments, $dat * {@link DataQuery} first. * * @param GridField $gridField - * @param SS_List&Sortable $dataList + * @param SS_List $dataList * @return SS_List */ public function getManipulatedData(GridField $gridField, SS_List $dataList) diff --git a/src/Model/List/ArrayList.php b/src/Model/List/ArrayList.php index 45f93456511..a0e5c2c4cc7 100644 --- a/src/Model/List/ArrayList.php +++ b/src/Model/List/ArrayList.php @@ -15,8 +15,8 @@ /** * A list object that wraps around an array of objects or arrays. * - * Note that (like DataLists), the implementations of the methods from SS_Filterable, SS_Sortable and - * SS_Limitable return a new instance of ArrayList, rather than modifying the existing instance. + * Note that (like DataLists), the implementations of the methods from SS_List return a new instance of ArrayList, + * rather than modifying the existing instance. * * For easy reference, methods that operate in this way are: * @@ -28,11 +28,8 @@ * * @template T * @implements SS_List - * @implements Filterable - * @implements Sortable - * @implements Limitable */ -class ArrayList extends ModelData implements SS_List, Filterable, Sortable, Limitable +class ArrayList extends ModelData implements SS_List { use SearchFilterable; @@ -134,18 +131,15 @@ public function getIterator(): Traversable * * @return array */ - public function toArray() + public function toArray(): array { return $this->items; } /** * Walks the list using the specified callback - * - * @param callable $callback - * @return $this */ - public function each($callback) + public function each(callable $callback): static { foreach ($this as $item) { $callback($item); @@ -166,7 +160,7 @@ public function debug(): string /** * Return this list as an array and every object it as an sub array as well */ - public function toNestedArray() + public function toNestedArray(): array { $result = []; @@ -214,20 +208,16 @@ public function limit(?int $length, int $offset = 0): static /** * Add this $item into this list - * - * @param mixed $item */ - public function add($item) + public function add(mixed $item): void { $this->push($item); } /** * Remove this item from this list - * - * @param mixed $item */ - public function remove($item) + public function remove(mixed $item) { $renumberKeys = false; foreach ($this->items as $key => $value) { @@ -341,7 +331,7 @@ public function shift() return array_shift($this->items); } - public function first() + public function first(): mixed { if (empty($this->items)) { return null; @@ -350,7 +340,7 @@ public function first() return reset($this->items); } - public function last() + public function last(): mixed { if (empty($this->items)) { return null; @@ -364,9 +354,8 @@ public function last() * * @param string $keyfield The 'key' field of the result array * @param string $titlefield The value field of the result array - * @return Map */ - public function map($keyfield = 'ID', $titlefield = 'Title') + public function map(string $keyfield = 'ID', string $titlefield = 'Title'): Map { $list = clone $this; return new Map($list, $keyfield, $titlefield); @@ -374,11 +363,8 @@ public function map($keyfield = 'ID', $titlefield = 'Title') /** * Returns an array of a single field value for all items in the list. - * - * @param string $colName - * @return array */ - public function column($colName = 'ID') + public function column(string $colName = 'ID'): array { $result = []; @@ -391,22 +377,16 @@ public function column($colName = 'ID') /** * Returns a unique array of a single field value for all the items in the list - * - * @param string $colName - * @return array */ - public function columnUnique($colName = 'ID') + public function columnUnique(string $colName = 'ID'): array { return array_unique($this->column($colName) ?? []); } /** * You can always sort a ArrayList - * - * @param string $by - * @return bool */ - public function canSortBy($by) + public function canSortBy(string $by): bool { return true; } @@ -416,7 +396,7 @@ public function canSortBy($by) * * @return static */ - public function reverse() + public function reverse(): static { $list = clone $this; $list->items = array_reverse($this->items ?? []); @@ -476,10 +456,8 @@ protected function parseSortColumn($column, $direction = null) * * @return static */ - public function sort() + public function sort(...$args): static { - $args = func_get_args(); - if (count($args ?? [])==0) { return $this; } @@ -559,11 +537,8 @@ public function shuffle() * Returns true if the given column can be used to filter the records. * * It works by checking the fields available in the first record of the list. - * - * @param string $by - * @return bool */ - public function canFilterBy($by) + public function canFilterBy(string $by): bool { if (empty($this->items)) { return false; @@ -585,11 +560,9 @@ public function canFilterBy($by) /** * Find the first item of this list where the given key = value * - * @param string $key - * @param mixed $value * @return T|null */ - public function find($key, $value) + public function find(string $key, string $value): mixed { return $this->filter($key, $value)->first(); } @@ -597,7 +570,7 @@ public function find($key, $value) /** * Filter the list to include items with these characteristics * - * @see Filterable::filter() + * @see SS_List::filter() * @example $list->filter('Name', 'bob'); // only bob in the list * @example $list->filter('Name', array('aziz', 'bob'); // aziz and bob in list * @example $list->filter(array('Name'=>'bob, 'Age'=>21)); // bob with the Age 21 in list @@ -611,9 +584,9 @@ public function find($key, $value) * * @return static */ - public function filter() + public function filter(...$args): static { - $filters = $this->normaliseFilterArgs(...func_get_args()); + $filters = $this->normaliseFilterArgs(...$args); return $this->filterOrExclude($filters); } @@ -638,9 +611,9 @@ public function filter() * @param string|array See {@link filter()} * @return static */ - public function filterAny() + public function filterAny(...$args): static { - $filters = $this->normaliseFilterArgs(...func_get_args()); + $filters = $this->normaliseFilterArgs(...$args); return $this->filterOrExclude($filters, true, true); } @@ -661,9 +634,9 @@ public function filterAny() * * @return static */ - public function exclude() + public function exclude(...$args): static { - $filters = $this->normaliseFilterArgs(...func_get_args()); + $filters = $this->normaliseFilterArgs(...$args); return $this->filterOrExclude($filters, false); } @@ -688,9 +661,9 @@ public function exclude() * @param string|array See {@link filter()} * @return static */ - public function excludeAny(): static + public function excludeAny(...$args): static { - $filters = $this->normaliseFilterArgs(...func_get_args()); + $filters = $this->normaliseFilterArgs(...$args); return $this->filterOrExclude($filters, false, true); } @@ -836,13 +809,13 @@ protected function normaliseFilterArgs($column, $value = null) * * @return static */ - public function byIDs($ids) + public function byIDs(array $ids): static { $ids = array_map('intval', $ids ?? []); // sanitize return $this->filter('ID', $ids); } - public function byID($id) + public function byID(int|string|null $id): mixed { $firstElement = $this->filter("ID", $id)->first(); @@ -854,21 +827,13 @@ public function byID($id) } /** - * @see Filterable::filterByCallback() + * @see SS_List::filterByCallback() * * @example $list = $list->filterByCallback(function($item, $list) { return $item->Age == 9; }) - * @param callable $callback * @return static */ - public function filterByCallback($callback) + public function filterByCallback(callable $callback): static { - if (!is_callable($callback)) { - throw new LogicException(sprintf( - "SS_Filterable::filterByCallback() passed callback must be callable, '%s' given", - gettype($callback) - )); - } - $output = static::create(); foreach ($this as $item) { diff --git a/src/Model/List/Filterable.php b/src/Model/List/Filterable.php deleted file mode 100644 index 3ede8179faa..00000000000 --- a/src/Model/List/Filterable.php +++ /dev/null @@ -1,107 +0,0 @@ - - */ -interface Filterable extends SS_List -{ - - /** - * Returns TRUE if the list can be filtered by a given field expression. - * - * @param string $by - * @return bool - */ - public function canFilterBy($by); - - /** - * Return a new instance of this list that only includes items with these characteristics - * - * @example $list = $list->filter('Name', 'bob'); // only bob in the list - * @example $list = $list->filter('Name', array('aziz', 'bob'); // aziz and bob in list - * @example $list = $list->filter(array('Name'=>'bob, 'Age'=>21)); // bob with the age 21 - * @example $list = $list->filter(array('Name'=>'bob, 'Age'=>array(21, 43))); // bob with the Age 21 or 43 - * @example $list = $list->filter(array('Name'=>array('aziz','bob'), 'Age'=>array(21, 43))); - * // aziz with the age 21 or 43 and bob with the Age 21 or 43 - * - * @return static - */ - public function filter(); - - /** - * Return a copy of this list which contains items matching any of these characteristics. - * - * @example // only bob in the list - * $list = $list->filterAny('Name', 'bob'); - * // SQL: WHERE "Name" = 'bob' - * @example // azis or bob in the list - * $list = $list->filterAny('Name', array('aziz', 'bob'); - * // SQL: WHERE ("Name" IN ('aziz','bob')) - * @example // bob or anyone aged 21 in the list - * $list = $list->filterAny(array('Name'=>'bob, 'Age'=>21)); - * // SQL: WHERE ("Name" = 'bob' OR "Age" = '21') - * @example // bob or anyone aged 21 or 43 in the list - * $list = $list->filterAny(array('Name'=>'bob, 'Age'=>array(21, 43))); - * // SQL: WHERE ("Name" = 'bob' OR ("Age" IN ('21', '43')) - * @example // all bobs, phils or anyone aged 21 or 43 in the list - * $list = $list->filterAny(array('Name'=>array('bob','phil'), 'Age'=>array(21, 43))); - * // SQL: WHERE (("Name" IN ('bob', 'phil')) OR ("Age" IN ('21', '43')) - * - * @param string|array See {@link filter()} - * @return static - */ - public function filterAny(); - - /** - * Return a new instance of this list that excludes any items with these characteristics - * - * @example $list = $list->exclude('Name', 'bob'); // exclude bob from list - * @example $list = $list->exclude('Name', array('aziz', 'bob'); // exclude aziz and bob from list - * @example $list = $list->exclude(array('Name'=>'bob, 'Age'=>21)); // exclude bob that has Age 21 - * @example $list = $list->exclude(array('Name'=>'bob, 'Age'=>array(21, 43))); // exclude bob with Age 21 or 43 - * @example $list = $list->exclude(array('Name'=>array('bob','phil'), 'Age'=>array(21, 43))); - * // bob age 21 or 43, phil age 21 or 43 would be excluded - * - * @return static - */ - public function exclude(); - - /** - * Return a new instance of this list that excludes any items with these characteristics - * Filter this List by a callback function. The function will be passed each record of the List in turn, - * and must return true for the record to be included. Returns the filtered list. - * - * @example $list = $list->filterByCallback(function($item, $list) { return $item->Age == 9; }) - * @param callable $callback - * @return SS_List - */ - public function filterByCallback($callback); - - /** - * Return the first item with the given ID - * - * @param int $id - * @return T|null - */ - public function byID($id); - - /** - * Filter this list to only contain the given Primary IDs - * - * @param array $ids Array of integers - * @return static - */ - public function byIDs($ids); -} diff --git a/src/Model/List/Limitable.php b/src/Model/List/Limitable.php deleted file mode 100644 index 27a2fe8bee7..00000000000 --- a/src/Model/List/Limitable.php +++ /dev/null @@ -1,32 +0,0 @@ - - */ -interface Limitable extends SS_List -{ - - /** - * Returns a new instance of this list where no more than $limit records are included. - * 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. - * - * 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 - * @return static - */ - public function limit(?int $length, int $offset = 0): Limitable; -} diff --git a/src/Model/List/ListDecorator.php b/src/Model/List/ListDecorator.php index 6cfc963b425..3ef29dc4132 100644 --- a/src/Model/List/ListDecorator.php +++ b/src/Model/List/ListDecorator.php @@ -5,39 +5,36 @@ use SilverStripe\Model\ModelData; use LogicException; use Traversable; +use BadMethodCallException; /** * A base class for decorators that wrap around a list to provide additional * functionality. It passes through list methods to the underlying list * implementation. * - * @template TList of SS_List&Sortable&Filterable&Limitable * @template T * @implements SS_List - * @implements Sortable - * @implements Filterable - * @implements Limitable */ -abstract class ListDecorator extends ModelData implements SS_List, Sortable, Filterable, Limitable +abstract class ListDecorator extends ModelData implements SS_List { /** - * @var TList + * @var SS_List */ - protected SS_List&Sortable&Filterable&Limitable $list; + protected SS_List $list; /** - * @param TList $list + * @param SS_List $list */ - public function __construct(SS_List&Sortable&Filterable&Limitable $list) + public function __construct(SS_List $list) { $this->setList($list); parent::__construct(); } /** - * @return TList + * @return SS_List */ - public function getList(): SS_List&Sortable&Filterable&Limitable + public function getList(): SS_List { return $this->list; } @@ -48,12 +45,12 @@ public function getList(): SS_List&Sortable&Filterable&Limitable * Useful for keeping a decorator/paginated list configuration intact while modifying * the underlying list. * - * @template TListA + * @template SS_ListA * @template TA - * @param TListA $list - * @return static + * @param SS_ListA $list + * @return static */ - public function setList(SS_List&Sortable&Filterable&Limitable $list): ListDecorator + public function setList(SS_List $list): ListDecorator { $this->list = $list; $this->failover = $this->list; @@ -83,22 +80,22 @@ public function offsetUnset(mixed $key): void $this->list->offsetUnset($key); } - public function toArray() + public function toArray(): array { return $this->list->toArray(); } - public function toNestedArray() + public function toNestedArray(): array { return $this->list->toNestedArray(); } - public function add($item) + public function add(mixed $item): void { $this->list->add($item); } - public function remove($itemObject) + public function remove(mixed $itemObject) { $this->list->remove($itemObject); } @@ -113,15 +110,18 @@ public function getIterator(): Traversable public function exists(): bool { - return $this->list->exists(); + if (method_exists($this->list, 'exists')) { + return call_user_func([$this->list, 'exists']); + } + return false; } - public function first() + public function first(): mixed { return $this->list->first(); } - public function last() + public function last(): mixed { return $this->list->last(); } @@ -134,50 +134,62 @@ public function TotalItems() return $this->list->count(); } - public function Count(): int + public function count(): int { return $this->list->count(); } public function forTemplate(): string { - return $this->list->forTemplate(); + if (method_exists($this->list, 'forTemplate')) { + return call_user_func([$this->list, 'forTemplate']); + } + throw new BadMethodCallException(sprintf( + "Method 'forTemplate' not found on class '%s'", + get_class($this->list) + )); } - public function map($index = 'ID', $titleField = 'Title') + public function map(string $index = 'ID', string $titleField = 'Title'): Map { return $this->list->map($index, $titleField); } - public function find($key, $value) + public function find(string $key, string $value): mixed { return $this->list->find($key, $value); } - public function column($value = 'ID') + public function column(string $value = 'ID'): array { return $this->list->column($value); } - public function columnUnique($value = "ID") + public function columnUnique(string $value = "ID"): array { - return $this->list->columnUnique($value); + if (method_exists($this->list, 'columnUnique')) { + return call_user_func([$this->list, 'columnUnique'], $value); + } + throw new BadMethodCallException(sprintf( + "Method 'columnUnique' not found on class '%s'", + get_class($this->list) + )); } /** - * @return TList + * @return SS_List */ - public function each($callback) + public function each(callable $callback): SS_List { return $this->list->each($callback); } - public function canSortBy($by) + public function canSortBy(string $by): bool { return $this->list->canSortBy($by); } - public function reverse() + public function reverse(): SS_List { return $this->list->reverse(); } @@ -191,14 +203,14 @@ public function reverse() * @example $list->sort('Name', 'ASC'); * @example $list->sort(array('Name'=>'ASC,'Age'=>'DESC')); * - * @return TList + * @return SS_List */ - public function sort() + public function sort(...$args): SS_List { - return $this->list->sort(...func_get_args()); + return $this->list->sort(...$args); } - public function canFilterBy($by) + public function canFilterBy(string $by): bool { return $this->list->canFilterBy($by); } @@ -211,11 +223,11 @@ public function canFilterBy($by) * @example $list->filter(array('Name'=>'bob, 'Age'=>21)); // bob or someone with Age 21 * @example $list->filter(array('Name'=>'bob, 'Age'=>array(21, 43))); // bob or anyone with Age 21 or 43 * - * @return TList + * @return SS_List */ - public function filter() + public function filter(...$args): SS_List { - return $this->list->filter(...func_get_args()); + return $this->list->filter(...$args); } /** @@ -239,30 +251,23 @@ public function filter() * * @param string|array See {@link filter()} * - * @return TList + * @return SS_List */ - public function filterAny() + public function filterAny(...$args): SS_List { - return $this->list->filterAny(...func_get_args()); + return $this->list->filterAny(...$args); } /** * Note that, in the current implementation, the filtered list will be an ArrayList, but this may change in a * future implementation. - * @see Filterable::filterByCallback() + * @see SS_List::filterByCallback() * * @example $list = $list->filterByCallback(function($item, $list) { return $item->Age == 9; }) - * @param callable $callback * @return ArrayList */ - public function filterByCallback($callback) + public function filterByCallback(callable $callback): ArrayList { - if (!is_callable($callback)) { - throw new LogicException(sprintf( - "SS_Filterable::filterByCallback() passed callback must be callable, '%s' given", - gettype($callback) - )); - } $output = ArrayList::create(); foreach ($this->list as $item) { if ($callback($item, $this->list)) { @@ -273,14 +278,14 @@ public function filterByCallback($callback) } /** - * @return TList + * @return SS_List */ - public function limit(?int $length, int $offset = 0): SS_List&Sortable&Filterable&Limitable + public function limit(?int $length, int $offset = 0): SS_List { return $this->list->limit($length, $offset); } - public function byID($id) + public function byID(int|string|null $id): mixed { return $this->list->byID($id); } @@ -290,9 +295,9 @@ public function byID($id) * * @param array $ids Array of integers * - * @return TList + * @return SS_List */ - public function byIDs($ids) + public function byIDs(array $ids): SS_List { return $this->list->byIDs($ids); } @@ -305,15 +310,38 @@ public function byIDs($ids) * @example $list->exclude(array('Name'=>'bob, 'Age'=>21)); // exclude bob or someone with Age 21 * @example $list->exclude(array('Name'=>'bob, 'Age'=>array(21, 43))); // exclude bob or anyone with Age 21 or 43 * - * @return TList + * @return SS_List + */ + public function exclude(...$args): SS_List + { + return $this->list->exclude(...$args); + } + + /** + * Return a copy of this list which does not contain any items with any of these params + * + * @example $list = $list->excludeAny('Name', 'bob'); // exclude bob from list + * @example $list = $list->excludeAny('Name', array('aziz', 'bob'); // exclude aziz and bob from list + * @example $list = $list->excludeAny(array('Name'=>'bob, 'Age'=>21)); // exclude bob or Age 21 + * @example $list = $list->excludeAny(array('Name'=>'bob, 'Age'=>array(21, 43))); // exclude bob or Age 21 or 43 + * @example $list = $list->excludeAny(array('Name'=>array('bob','phil'), 'Age'=>array(21, 43))); + * // bob, phil, 21 or 43 would be excluded + * + * @return SS_List */ - public function exclude() + public function excludeAny(...$args): SS_List { - return $this->list->exclude(...func_get_args()); + return $this->list->excludeAny(...$args); } public function debug(): string { - return $this->list->debug(); + if (method_exists($this->list, 'debug')) { + return call_user_func([$this->list, 'debug']); + } + throw new BadMethodCallException(sprintf( + "Method 'debug' not found on class '%s'", + get_class($this->list) + )); } } diff --git a/src/Model/List/PaginatedList.php b/src/Model/List/PaginatedList.php index 3df3dfbc024..4d5006a350a 100644 --- a/src/Model/List/PaginatedList.php +++ b/src/Model/List/PaginatedList.php @@ -228,7 +228,7 @@ public function getIterator(): Traversable /** * @return array */ - public function toArray() + public function toArray(): array { $result = []; diff --git a/src/Model/List/SS_List.php b/src/Model/List/SS_List.php index 3b788193f5d..f0ddebf9b86 100644 --- a/src/Model/List/SS_List.php +++ b/src/Model/List/SS_List.php @@ -5,6 +5,7 @@ use ArrayAccess; use Countable; use IteratorAggregate; +use Traversable; /** * An interface that a class can implement to be treated as a list container. @@ -15,83 +16,243 @@ */ interface SS_List extends ArrayAccess, Countable, IteratorAggregate { - /** * Returns all the items in the list in an array. * * @return array */ - public function toArray(); + public function toArray(): array; /** * Returns the contents of the list as an array of maps. - * - * @return array */ - public function toNestedArray(); + public function toNestedArray(): array; /** * Adds an item to the list, making no guarantees about where it will * appear. - * - * @param mixed $item */ - public function add($item); + public function add(mixed $item): void; /** * Removes an item from the list. - * - * @param mixed $item */ - public function remove($item); + public function remove(mixed $item); + + /** + * Return the number of items in this list + */ + public function count(): int; /** * Returns the first item in the list. * * @return T|null */ - public function first(); + public function first(): mixed; /** * Returns the last item in the list. * * @return T|null */ - public function last(); + public function last(): mixed; /** * Returns a map of a key field to a value field of all the items in the * list. - * - * @param string $keyfield - * @param string $titlefield - * @return Map */ - public function map($keyfield = 'ID', $titlefield = 'Title'); + public function map(string $keyfield = 'ID', string $titlefield = 'Title'): Map; /** * Returns the first item in the list where the key field is equal to the * value. * - * @param string $key - * @param mixed $value * @return T|null */ - public function find($key, $value); + public function find(string $key, string $value): mixed; /** * Returns an array of a single field value for all items in the list. - * - * @param string $colName - * @return array */ - public function column($colName = "ID"); + public function column(string $colName = "ID"): array; + + /** + * Returns a unique array of a single field value for all items in the list. + */ + public function columnUnique(string $colName = 'ID'): array; /** * Walks the list using the specified callback * - * @param callable $callback - * @return static + * @return SS_List + */ + public function each(callable $callback): SS_List; + + /** + * Returns TRUE if the list can be filtered by a given field expression. + */ + public function canFilterBy(string $by): bool; + + /** + * Returns true if this list has items + */ + public function exists(): bool; + + /** + * Return a new instance of this list that only includes items with these characteristics + * + * @example $list = $list->filter('Name', 'bob'); // only bob in the list + * @example $list = $list->filter('Name', array('aziz', 'bob'); // aziz and bob in list + * @example $list = $list->filter(array('Name'=>'bob, 'Age'=>21)); // bob with the age 21 + * @example $list = $list->filter(array('Name'=>'bob, 'Age'=>array(21, 43))); // bob with the Age 21 or 43 + * @example $list = $list->filter(array('Name'=>array('aziz','bob'), 'Age'=>array(21, 43))); + * // aziz with the age 21 or 43 and bob with the Age 21 or 43 + * + * @return SS_List + */ + public function filter(...$args): SS_List; + + /** + * Return a copy of this list which contains items matching any of these characteristics. + * + * @example // only bob in the list + * $list = $list->filterAny('Name', 'bob'); + * // SQL: WHERE "Name" = 'bob' + * @example // azis or bob in the list + * $list = $list->filterAny('Name', array('aziz', 'bob'); + * // SQL: WHERE ("Name" IN ('aziz','bob')) + * @example // bob or anyone aged 21 in the list + * $list = $list->filterAny(array('Name'=>'bob, 'Age'=>21)); + * // SQL: WHERE ("Name" = 'bob' OR "Age" = '21') + * @example // bob or anyone aged 21 or 43 in the list + * $list = $list->filterAny(array('Name'=>'bob, 'Age'=>array(21, 43))); + * // SQL: WHERE ("Name" = 'bob' OR ("Age" IN ('21', '43')) + * @example // all bobs, phils or anyone aged 21 or 43 in the list + * $list = $list->filterAny(array('Name'=>array('bob','phil'), 'Age'=>array(21, 43))); + * // SQL: WHERE (("Name" IN ('bob', 'phil')) OR ("Age" IN ('21', '43')) + * + * @param string|array See {@link filter()} + * @return SS_List + */ + public function filterAny(...$args): SS_List; + + /** + * Return a new instance of this list that excludes any items with these characteristics + * + * @example $list = $list->exclude('Name', 'bob'); // exclude bob from list + * @example $list = $list->exclude('Name', array('aziz', 'bob'); // exclude aziz and bob from list + * @example $list = $list->exclude(array('Name'=>'bob, 'Age'=>21)); // exclude bob that has Age 21 + * @example $list = $list->exclude(array('Name'=>'bob, 'Age'=>array(21, 43))); // exclude bob with Age 21 or 43 + * @example $list = $list->exclude(array('Name'=>array('bob','phil'), 'Age'=>array(21, 43))); + * // bob age 21 or 43, phil age 21 or 43 would be excluded + * + * @return SS_List + */ + public function exclude(...$args): SS_List; + + /** + * Return a copy of this list which does not contain any items with any of these params + * + * @return SS_List + */ + public function excludeAny(...$args): SS_List; + + /** + * Return a new instance of this list that excludes any items with these characteristics + * Filter this List by a callback function. The function will be passed each record of the List in turn, + * and must return true for the record to be included. Returns the filtered list. + * + * @example $list = $list->filterByCallback(function($item, $list) { return $item->Age == 9; }) + * @return SS_List + */ + public function filterByCallback(callable $callback): SS_List; + + /** + * Return the first item with the given ID + * + * @param int $id + * @return T|null + */ + public function byID(int|string|null $id): mixed; + // Note that string ID's and null values should be handled as things like + // form submissions and controller params will often be string IDs, + // and null is often unintentionally passed in a lot of instances. + // If this is changed in the future to only accept int, ensure that all other + // filtering methods are updated at the same time for consistency + + /** + * Filter this list to only contain the given Primary IDs + * + * @param array $ids Array of integers + * @return SS_List + */ + public function byIDs(array $ids): SS_List; + + /** + * Returns TRUE if the list can be sorted by a field. + */ + public function canSortBy(string $by): bool; + + /** + * Return a new instance of this list that is sorted by one or more fields. You can either pass in a single + * field name and direction, or a map of field names to sort directions. + * + * @example $list = $list->sort('Name'); // default ASC sorting + * @example $list = $list->sort('Name DESC'); // DESC sorting + * @example $list = $list->sort('Name', 'ASC'); + * @example $list = $list->sort(array('Name'=>'ASC,'Age'=>'DESC')); + * + * @return SS_List + */ + public function sort(...$args): SS_List; + + + /** + * Return a new instance of this list based on reversing the current sort. + * + * @example $list = $list->reverse(); + * + * @return SS_List + */ + public function reverse(): SS_List; + + /** + * Returns a new instance of this list where no more than $limit records are included. + * 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. + * + * 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 + * @return SS_List + */ + public function limit(?int $length, int $offset = 0): SS_List; + + /** + * Returns an Iterator for this list + * + * @return Traversable + */ + public function getIterator(): Traversable; + + /** + * Returns whether an item with $key exists + */ + public function offsetExists(mixed $key): bool; + + /** + * Returns item stored in list with index $key + */ + public function offsetGet(mixed $key): mixed; + + /** + * Set an item with the key in $key + */ + public function offsetSet(mixed $key, mixed $value): void; + + /** + * Unset an item with the key in $key */ - public function each($callback); + public function offsetUnset(mixed $key): void; } diff --git a/src/Model/List/Sortable.php b/src/Model/List/Sortable.php deleted file mode 100644 index c818cc5644e..00000000000 --- a/src/Model/List/Sortable.php +++ /dev/null @@ -1,51 +0,0 @@ - - */ -interface Sortable extends SS_List -{ - - /** - * Returns TRUE if the list can be sorted by a field. - * - * @param string $by - * @return bool - */ - public function canSortBy($by); - - /** - * Return a new instance of this list that is sorted by one or more fields. You can either pass in a single - * field name and direction, or a map of field names to sort directions. - * - * @example $list = $list->sort('Name'); // default ASC sorting - * @example $list = $list->sort('Name DESC'); // DESC sorting - * @example $list = $list->sort('Name', 'ASC'); - * @example $list = $list->sort(array('Name'=>'ASC,'Age'=>'DESC')); - * - * @return static - */ - public function sort(); - - - /** - * Return a new instance of this list based on reversing the current sort. - * - * @example $list = $list->reverse(); - * - * @return static - */ - public function reverse(); -} diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php index a65f5b161ef..b99324cf7be 100644 --- a/src/ORM/DataList.php +++ b/src/ORM/DataList.php @@ -10,14 +10,12 @@ use InvalidArgumentException; use LogicException; use BadMethodCallException; +use Dflydev\DotAccessData\Data; use SilverStripe\ORM\Connect\Query; use Traversable; use SilverStripe\ORM\DataQuery; use SilverStripe\Model\List\ArrayList; -use SilverStripe\Model\List\Filterable; -use SilverStripe\Model\List\Limitable; use SilverStripe\Model\List\Map; -use SilverStripe\Model\List\Sortable; use SilverStripe\Model\List\SS_List; use SilverStripe\ORM\Filters\SearchFilterable; @@ -43,11 +41,8 @@ * * @template T of DataObject * @implements SS_List - * @implements Filterable - * @implements Sortable - * @implements Limitable */ -class DataList extends ModelData implements SS_List, Filterable, Sortable, Limitable +class DataList extends ModelData implements SS_List { use SearchFilterable; @@ -286,11 +281,8 @@ public function whereAny($filter) /** * Returns true if this DataList can be sorted by the given field. - * - * @param string $fieldName - * @return boolean */ - public function canSortBy($fieldName) + public function canSortBy(string $fieldName): bool { return $this->dataQuery()->query()->canSortBy($fieldName); } @@ -299,9 +291,8 @@ public function canSortBy($fieldName) * Returns true if this DataList can be filtered by the given field. * * @param string $fieldName (May be a related field in dot notation like Member.FirstName) - * @return boolean */ - public function canFilterBy($fieldName) + public function canFilterBy(string $fieldName): bool { $model = singleton($this->dataClass); $relations = explode(".", $fieldName ?? ''); @@ -474,7 +465,7 @@ public function orderBy(string $orderBy): static * * Raw SQL is not accepted, only actual field names can be passed * - * @see Filterable::filter() + * @see SS_List::filter() * * @example $list = $list->filter('Name', 'bob'); // only bob in the list * @example $list = $list->filter('Name', array('aziz', 'bob'); // aziz and bob in list @@ -490,17 +481,16 @@ public function orderBy(string $orderBy): static * @param string|array Escaped SQL statement. If passed as array, all keys and values will be escaped internally * @return static */ - public function filter() + public function filter(...$args): static { // Validate and process arguments - $arguments = func_get_args(); - switch (sizeof($arguments ?? [])) { + switch (sizeof($args ?? [])) { case 1: - $filters = $arguments[0]; + $filters = $args[0]; break; case 2: - $filters = [$arguments[0] => $arguments[1]]; + $filters = [$args[0] => $args[1]]; break; default: @@ -552,15 +542,15 @@ public function addFilter($filterArray) * @param string|array See {@link filter()} * @return static */ - public function filterAny() + public function filterAny(...$args): static { - $numberFuncArgs = count(func_get_args()); + $numberFuncArgs = count($args); $whereArguments = []; - if ($numberFuncArgs == 1 && is_array(func_get_arg(0))) { - $whereArguments = func_get_arg(0); + if ($numberFuncArgs == 1 && is_array($args[0])) { + $whereArguments = $args[0]; } elseif ($numberFuncArgs == 2) { - $whereArguments[func_get_arg(0)] = func_get_arg(1); + $whereArguments[$args[0]] = $args[1]; } else { throw new InvalidArgumentException('Incorrect number of arguments passed to filterAny()'); } @@ -591,20 +581,13 @@ private function getFilterAnySubquery(DataQuery $query, array $whereArguments): /** * Note that, in the current implementation, the filtered list will be an ArrayList, but this may change in a * future implementation. - * @see Filterable::filterByCallback() + * @see SS_List::filterByCallback() * * @example $list = $list->filterByCallback(function($item, $list) { return $item->Age == 9; }) - * @param callable $callback * @return ArrayList */ - public function filterByCallback($callback) + public function filterByCallback(callable $callback): ArrayList { - if (!is_callable($callback)) { - throw new LogicException(sprintf( - "SS_Filterable::filterByCallback() passed callback must be callable, '%s' given", - gettype($callback) - )); - } $output = ArrayList::create(); foreach ($this as $item) { if (call_user_func($callback, $item, $this)) { @@ -693,15 +676,15 @@ protected function isValidRelationName($field) * @param string [optional] * @return static */ - public function exclude() + public function exclude(...$args): static { - $numberFuncArgs = count(func_get_args()); + $numberFuncArgs = count($args); $whereArguments = []; - if ($numberFuncArgs == 1 && is_array(func_get_arg(0))) { - $whereArguments = func_get_arg(0); + if ($numberFuncArgs == 1 && is_array($args[0])) { + $whereArguments = $args[0]; } elseif ($numberFuncArgs == 2) { - $whereArguments[func_get_arg(0)] = func_get_arg(1); + $whereArguments[$args[0]] = $args[1]; } else { throw new InvalidArgumentException('Incorrect number of arguments passed to exclude()'); } @@ -733,15 +716,15 @@ public function exclude() * * @return static */ - public function excludeAny() + public function excludeAny(...$args): static { - $numberFuncArgs = count(func_get_args()); + $numberFuncArgs = count($args); $whereArguments = []; - if ($numberFuncArgs == 1 && is_array(func_get_arg(0))) { - $whereArguments = func_get_arg(0); + if ($numberFuncArgs == 1 && is_array($args[0])) { + $whereArguments = $args[0]; } elseif ($numberFuncArgs == 2) { - $whereArguments[func_get_arg(0)] = func_get_arg(1); + $whereArguments[$args[0]] = $args[1]; } else { throw new InvalidArgumentException('Incorrect number of arguments passed to excludeAny()'); } @@ -837,7 +820,7 @@ public function rightJoin($table, $onClause, $alias = null, $order = 20, $parame * This is when the query is actually executed. * @return array */ - public function toArray() + public function toArray(): array { $results = []; @@ -850,10 +833,8 @@ public function toArray() /** * Return this list as an array and every object it as an sub array as well - * - * @return array */ - public function toNestedArray() + public function toNestedArray(): array { $result = []; @@ -864,7 +845,7 @@ public function toNestedArray() return $result; } - public function each($callback) + public function each(callable $callback): static { foreach ($this as $row) { $callback($row); @@ -888,9 +869,8 @@ public function debug(): string * * @param string $keyField - the 'key' field of the result array * @param string $titleField - the value field of the result array - * @return Map */ - public function map($keyField = 'ID', $titleField = 'Title') + public function map(string $keyField = 'ID', string $titleField = 'Title'): Map { return new Map($this, $keyField, $titleField); } @@ -1672,7 +1652,7 @@ public function sum($fieldName) * The object returned is not cached, unlike {@link DataObject::get_one()} * @return T|null */ - public function first() + public function first(): ?DataObject { // We need to trigger eager loading by iterating over the list, rather than just fetching // the first row from the dataQuery. @@ -1691,7 +1671,7 @@ public function first() * The object returned is not cached, unlike {@link DataObject::get_one()} * @return T|null */ - public function last() + public function last(): mixed { // We need to trigger eager loading by iterating over the list, rather than just fetching // the last row from the dataQuery. @@ -1718,11 +1698,9 @@ public function exists(): bool * * The object returned is not cached, unlike {@link DataObject::get_one()} * - * @param string $key - * @param string $value * @return T|null */ - public function find($key, $value) + public function find(string $key, string $value): mixed { return $this->filter($key, $value)->first(); } @@ -1740,7 +1718,7 @@ public function setQueriedColumns($queriedColumns) }); } - public function byIDs($ids) + public function byIDs(array $ids): static { return $this->filter('ID', $ids); } @@ -1749,20 +1727,18 @@ public function byIDs($ids) * Return the first DataObject with the given ID * * The object returned is not cached, unlike {@link DataObject::get_by_id()} + * * @return T|null */ - public function byID($id) + public function byID(int|string|null $id): ?DataObject { return $this->filter('ID', $id)->first(); } /** * Returns an array of a single field value for all items in the list. - * - * @param string $colName - * @return array */ - public function column($colName = "ID") + public function column(string $colName = "ID"): array { if ($this->finalisedQuery) { $finalisedQuery = clone $this->finalisedQuery; @@ -1779,7 +1755,7 @@ public function column($colName = "ID") * @param string $colName * @return array */ - public function columnUnique($colName = "ID") + public function columnUnique(string $colName = "ID"): array { return $this->dataQuery->distinct(true)->column($colName); } @@ -1928,7 +1904,7 @@ public function removeAll() * * @param DataObject|int $item */ - public function add($item) + public function add(mixed $item): void { // Nothing needs to happen by default // TO DO: If a filter is given to this data list then @@ -1951,8 +1927,11 @@ public function newObject($initialFields = null) * * @param DataObject $item */ - public function remove($item) + public function remove(mixed $item) { + if (!is_a($item, $this->dataClass)) { + throw new InvalidArgumentException('Item must be an instance of ' . $this->dataClass); + } // By default, we remove an item from a DataList by deleting it. $this->removeByID($item->ID); } @@ -1976,7 +1955,7 @@ public function removeByID($itemID) * * @return static */ - public function reverse() + public function reverse(): static { return $this->alterDataQuery(function (DataQuery $query) { $query->reverseSort(); diff --git a/src/ORM/EagerLoadedList.php b/src/ORM/EagerLoadedList.php index d65a49d3767..db4138a58b6 100644 --- a/src/ORM/EagerLoadedList.php +++ b/src/ORM/EagerLoadedList.php @@ -11,10 +11,7 @@ use LogicException; use SilverStripe\Core\ArrayLib; use SilverStripe\Model\List\ArrayList; -use SilverStripe\Model\List\Filterable; -use SilverStripe\Model\List\Limitable; use SilverStripe\Model\List\Map; -use SilverStripe\Model\List\Sortable; use SilverStripe\Model\List\SS_List; use SilverStripe\ORM\Filters\SearchFilterable; use Traversable; @@ -32,11 +29,8 @@ * @template T of DataObject * @implements Relation * @implements SS_List - * @implements Filterable - * @implements Sortable - * @implements Limitable */ -class EagerLoadedList extends ModelData implements Relation, SS_List, Filterable, Sortable, Limitable +class EagerLoadedList extends ModelData implements Relation, SS_List { use SearchFilterable; @@ -268,7 +262,7 @@ public function addRows(array $rows): static /** * Not implemented - use addRow instead. */ - public function add($item) + public function add(mixed $item): void { throw new BadMethodCallException('Cannot add a DataObject record to EagerLoadedList. Use addRow() to add database rows.'); } @@ -276,9 +270,14 @@ public function add($item) /** * Removes a record from the list. Note that the record will not be removed from the * database - this list is read-only. + * + * @param DataObject $item */ - public function remove($item): static + public function remove(mixed $item) { + if (!is_a($item, $this->dataClass)) { + throw new InvalidArgumentException('Item must be an instance of ' . $this->dataClass); + } $id = $item->ID; if (array_key_exists($id, $this->rows)) { unset($this->rows[$id]); @@ -304,12 +303,12 @@ public function last(): ?DataObject return $this->byID(array_key_last($rows)); } - public function map($keyField = 'ID', $titleField = 'Title'): Map + public function map(string $keyField = 'ID', string $titleField = 'Title'): Map { return new Map($this, $keyField, $titleField); } - public function column($colName = 'ID'): array + public function column(string $colName = 'ID'): array { $rows = $this->getFinalisedRows(); @@ -332,15 +331,13 @@ public function column($colName = 'ID'): array /** * Returns a unique array of a single field value for all the items in the list - * - * @param string $colName */ - public function columnUnique($colName = 'ID'): array + public function columnUnique(string $colName = 'ID'): array { return array_unique($this->column($colName)); } - public function each($callback): static + public function each(callable $callback): static { foreach ($this as $row) { $callback($row); @@ -470,7 +467,7 @@ public function exists(): bool return $this->count() !== 0; } - public function canFilterBy($fieldName): bool + public function canFilterBy(string $fieldName): bool { if (!is_string($fieldName) || empty($this->rows)) { return false; @@ -480,12 +477,12 @@ public function canFilterBy($fieldName): bool return array_key_exists($fieldName, $this->rows[$id]); } - public function canSortBy($fieldName): bool + public function canSortBy(string $fieldName): bool { return $this->canFilterBy($fieldName); } - public function find($key, $value): ?DataObject + public function find(string $key, string $value): ?DataObject { return $this->filter($key, $value)->first(); } @@ -644,15 +641,8 @@ private function extractValue(array $row, $key): mixed /** * @return ArrayList */ - public function filterByCallback($callback): ArrayList + public function filterByCallback(callable $callback): ArrayList { - if (!is_callable($callback)) { - throw new LogicException(sprintf( - "SS_Filterable::filterByCallback() passed callback must be callable, '%s' given", - gettype($callback) - )); - } - $output = ArrayList::create(); foreach ($this as $item) { if (call_user_func($callback, $item, $this)) { @@ -662,7 +652,7 @@ public function filterByCallback($callback): ArrayList return $output; } - public function byID($id): ?DataObject + public function byID(int|string|null $id): ?DataObject { $rows = $this->getFinalisedRows(); if (!array_key_exists($id, $rows)) { @@ -671,10 +661,10 @@ public function byID($id): ?DataObject return $this->createDataObject($rows[$id]); } - public function byIDs($ids): static + public function byIDs(array $ids): static { $list = clone $this; - $ids = array_map('intval', (array) $ids); + $ids = array_map('intval', $ids); $list->rows = ArrayLib::filter_keys($list->rows, $ids); return $list; } diff --git a/src/ORM/HasManyList.php b/src/ORM/HasManyList.php index 88a5674d90b..7d2dd8e6393 100644 --- a/src/ORM/HasManyList.php +++ b/src/ORM/HasManyList.php @@ -73,7 +73,7 @@ protected function foreignIDFilter($id = null) * * @param DataObject|int $item The DataObject to be added, or its ID */ - public function add($item) + public function add(mixed $item): void { if (is_numeric($item)) { $item = DataObject::get_by_id($this->dataClass, $item); @@ -123,7 +123,7 @@ public function removeByID($itemID) * * @param DataObject $item The DataObject to be removed */ - public function remove($item) + public function remove(mixed $item) { if (!($item instanceof $this->dataClass)) { throw new InvalidArgumentException("HasManyList::remove() expecting a $this->dataClass object, or ID"); diff --git a/src/ORM/ManyManyList.php b/src/ORM/ManyManyList.php index 52fdfa2c011..96c7cac316b 100644 --- a/src/ORM/ManyManyList.php +++ b/src/ORM/ManyManyList.php @@ -211,14 +211,14 @@ protected function foreignIDWriteFilter($id = null) * @throws Exception * * @param DataObject|int $item - * @param array $extraFields A map of additional columns to insert into the joinTable. + * @param null|array $extraFields A map of additional columns to insert into the joinTable. * Column names should be ANSI quoted. * @throws Exception */ - public function add($item, $extraFields = []) + public function add(mixed $item, ?array $extraFields = []): void { - // Ensure nulls or empty strings are correctly treated as empty arrays - if (empty($extraFields)) { + // Ensure nulls are correctly treated as empty arrays + if (is_null($extraFields)) { $extraFields = []; } diff --git a/src/ORM/ManyManyThroughList.php b/src/ORM/ManyManyThroughList.php index 0553de98c5e..438bb34490d 100644 --- a/src/ORM/ManyManyThroughList.php +++ b/src/ORM/ManyManyThroughList.php @@ -169,14 +169,10 @@ public function removeAll() } } - /** - * @param mixed $item - * @param array $extraFields - */ - public function add($item, $extraFields = []) + public function add(mixed $item, ?array $extraFields = []): void { - // Ensure nulls or empty strings are correctly treated as empty arrays - if (empty($extraFields)) { + // Ensure nulls are correctly treated as empty arrays + if (is_null($extraFields)) { $extraFields = []; } diff --git a/src/ORM/PolymorphicHasManyList.php b/src/ORM/PolymorphicHasManyList.php index 6f1908441db..2eac5a2f4e0 100644 --- a/src/ORM/PolymorphicHasManyList.php +++ b/src/ORM/PolymorphicHasManyList.php @@ -110,7 +110,7 @@ public function __construct($dataClass, $foreignField, $foreignClass) )); } - public function add($item) + public function add(mixed $item): void { if (is_numeric($item)) { $item = DataObject::get_by_id($this->dataClass, $item); diff --git a/src/ORM/Relation.php b/src/ORM/Relation.php index 62b2b266cb2..0bc0d3b02fc 100644 --- a/src/ORM/Relation.php +++ b/src/ORM/Relation.php @@ -2,9 +2,6 @@ namespace SilverStripe\ORM; -use SilverStripe\Model\List\Filterable; -use SilverStripe\Model\List\Limitable; -use SilverStripe\Model\List\Sortable; use SilverStripe\Model\List\SS_List; use SilverStripe\ORM\FieldType\DBField; @@ -19,11 +16,8 @@ * * @template T * @extends SS_List - * @extends Filterable - * @extends Sortable - * @extends Limitable */ -interface Relation extends SS_List, Filterable, Sortable, Limitable +interface Relation extends SS_List { /** diff --git a/src/ORM/Search/BasicSearchContext.php b/src/ORM/Search/BasicSearchContext.php index c7b10685810..5f33be22efd 100644 --- a/src/ORM/Search/BasicSearchContext.php +++ b/src/ORM/Search/BasicSearchContext.php @@ -8,11 +8,9 @@ use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\Deprecation; -use SilverStripe\Model\List\Filterable; use SilverStripe\ORM\Filters\PartialMatchFilter; use SilverStripe\ORM\Filters\SearchFilter; -use SilverStripe\Model\List\Limitable; -use SilverStripe\Model\List\Sortable; +use SilverStripe\Model\List\SS_List; /** * A SearchContext that can be used with non-ORM data. @@ -37,12 +35,12 @@ class BasicSearchContext extends SearchContext * for example "Comments__Name" instead of the filter name "Comments.Name". * @param array|bool|string $sort Field to sort on. * @param array|null|string $limit - * @param Filterable&Sortable&Limitable $existingQuery + * @param SS_List $existingQuery */ - public function getQuery($searchParams, $sort = false, $limit = false, $existingQuery = null): Filterable&Sortable&Limitable + public function getQuery($searchParams, $sort = false, $limit = false, $existingQuery = null): SS_List { - if (!$existingQuery || !($existingQuery instanceof Filterable) || !($existingQuery instanceof Sortable) || !($existingQuery instanceof Limitable)) { - throw new InvalidArgumentException('getQuery requires a pre-existing filterable/sortable/limitable list to be passed as $existingQuery.'); + if (!$existingQuery || !is_a($existingQuery, SS_List::class)) { + throw new InvalidArgumentException('getQuery requires a pre-existing SS_List list to be passed as $existingQuery.'); } if ((count(func_get_args()) >= 3) && (!in_array(gettype($limit), ['array', 'NULL', 'string']))) { @@ -98,7 +96,7 @@ private function applySearchFilters(array $searchParams): array return $applied; } - private function applyGeneralSearchField(array &$searchParams, Filterable $existingQuery): Filterable + private function applyGeneralSearchField(array &$searchParams, SS_List $existingQuery): SS_List { $generalFieldName = static::config()->get('general_search_field_name'); if (array_key_exists($generalFieldName, $searchParams)) { diff --git a/src/ORM/UnsavedRelationList.php b/src/ORM/UnsavedRelationList.php index e01ff241e17..2eadc95d315 100644 --- a/src/ORM/UnsavedRelationList.php +++ b/src/ORM/UnsavedRelationList.php @@ -72,10 +72,9 @@ public function __construct($baseClass, $relationName, $dataClass) /** * Add an item to this relationship * - * @param mixed $item - * @param array $extraFields A map of additional columns to insert into the joinTable in the case of a many_many relation + * @param null|array $extraFields A map of additional columns to insert into the joinTable in the case of a many_many relation */ - public function add($item, $extraFields = null) + public function add(mixed $item, ?array $extraFields = null): void { $this->push($item, $extraFields); } @@ -136,7 +135,7 @@ public function getIterator(): Traversable * Return an array of the actual items that this relation contains at this stage. * This is when the query is actually executed. */ - public function toArray() + public function toArray(): array { $items = []; foreach ($this->items as $key => $item) { @@ -231,7 +230,7 @@ public function getIDList() return $ids; } - public function first() + public function first(): ?DataObject { $item = reset($this->items) ?: null; if (is_numeric($item)) { @@ -243,7 +242,7 @@ public function first() return $item; } - public function last() + public function last(): ?DataObject { $item = end($this->items) ?: null; if (is_numeric($item)) { @@ -257,11 +256,8 @@ public function last() /** * Returns an array of a single field value for all items in the list. - * - * @param string $colName - * @return array */ - public function column($colName = 'ID') + public function column(string $colName = 'ID'): array { $list = new ArrayList($this->toArray()); return $list->column($colName); @@ -269,11 +265,8 @@ public function column($colName = 'ID') /** * Returns a unique array of a single field value for all items in the list. - * - * @param string $colName - * @return array */ - public function columnUnique($colName = "ID") + public function columnUnique(string $colName = "ID"): array { $list = new ArrayList($this->toArray()); return $list->columnUnique($colName); diff --git a/src/Security/Member_GroupSet.php b/src/Security/Member_GroupSet.php index c50afc1bdf6..9c911872e56 100644 --- a/src/Security/Member_GroupSet.php +++ b/src/Security/Member_GroupSet.php @@ -71,7 +71,7 @@ public function foreignIDWriteFilter($id = null) return parent::foreignIDFilter($id); } - public function add($item, $extraFields = null) + public function add(mixed $item, ?array $extraFields = null): void { // Get Group.ID $itemID = null; diff --git a/tests/php/Model/List/ArrayListTest.php b/tests/php/Model/List/ArrayListTest.php index 6aca22cf737..c5ec7ee0886 100644 --- a/tests/php/Model/List/ArrayListTest.php +++ b/tests/php/Model/List/ArrayListTest.php @@ -5,7 +5,7 @@ use SilverStripe\Dev\SapphireTest; use SilverStripe\Model\List\ArrayList; use SilverStripe\ORM\DataObject; -use SilverStripe\Model\List\Filterable; +use SilverStripe\Model\List\SS_List; use SilverStripe\Model\ArrayData; use SilverStripe\Model\List\Map; use stdClass; @@ -1455,7 +1455,7 @@ function ($item, $list) { $this->assertEquals(2, $list->count()); $this->assertEquals($steve, $list[0]->toMap(), 'List should only contain Steve and Clair'); $this->assertEquals($clair, $list[1]->toMap(), 'List should only contain Steve and Clair'); - $this->assertTrue($list instanceof Filterable, 'The List should be of type SS_Filterable'); + $this->assertTrue($list instanceof SS_List, 'The List should be of type SS_List'); } /** diff --git a/tests/php/Model/List/ListDecoratorTest.php b/tests/php/Model/List/ListDecoratorTest.php index b6c3d75e5bd..84c1750f509 100644 --- a/tests/php/Model/List/ListDecoratorTest.php +++ b/tests/php/Model/List/ListDecoratorTest.php @@ -10,6 +10,7 @@ use SilverStripe\Model\List\ListDecorator; use SilverStripe\Model\List\SS_List; use PHPUnit\Framework\Attributes\DataProvider; +use SilverStripe\Model\List\Map; /** * This test class is testing that ListDecorator correctly proxies its calls through to the underlying SS_List @@ -61,8 +62,20 @@ public function testRemove() #[DataProvider('filterProvider')] public function testExclude($input) { - $this->list->expects($this->once())->method('exclude')->with($input)->willReturn('mock'); - $this->assertSame('mock', $this->decorator->exclude($input)); + $mock = $this->createMock(ArrayList::class); + $this->list->expects($this->once())->method('exclude')->with($input)->willReturn($mock); + $this->assertSame($mock, $this->decorator->exclude($input)); + } + + /** + * @param array $input + */ + #[DataProvider('filterProvider')] + public function testExcludeAny($input) + { + $mock = $this->createMock(ArrayList::class); + $this->list->expects($this->once())->method('excludeAny')->with($input)->willReturn($mock); + $this->assertSame($mock, $this->decorator->excludeAny($input)); } /** @@ -71,8 +84,9 @@ public function testExclude($input) #[DataProvider('filterProvider')] public function testFilter($input) { - $this->list->expects($this->once())->method('filter')->with($input)->willReturn('mock'); - $this->assertSame('mock', $this->decorator->filter($input)); + $mock = $this->createMock(ArrayList::class); + $this->list->expects($this->once())->method('filter')->with($input)->willReturn($mock); + $this->assertSame($mock, $this->decorator->filter($input)); } /** @@ -81,8 +95,9 @@ public function testFilter($input) #[DataProvider('filterProvider')] public function testFilterAny($input) { - $this->list->expects($this->once())->method('filterAny')->with($input)->willReturn('mock'); - $this->assertSame('mock', $this->decorator->filterAny($input)); + $mock = $this->createMock(ArrayList::class); + $this->list->expects($this->once())->method('filterAny')->with($input)->willReturn($mock); + $this->assertSame($mock, $this->decorator->filterAny($input)); } /** @@ -91,8 +106,9 @@ public function testFilterAny($input) #[DataProvider('filterProvider')] public function testSort($input) { - $this->list->expects($this->once())->method('sort')->with($input)->willReturn('mock'); - $this->assertSame('mock', $this->decorator->sort($input)); + $mock = $this->createMock(ArrayList::class); + $this->list->expects($this->once())->method('sort')->with($input)->willReturn($mock); + $this->assertSame($mock, $this->decorator->sort($input)); } /** @@ -114,13 +130,6 @@ public function testCanFilterBy() $this->assertFalse($this->decorator->canFilterBy('Title')); } - public function testFilterByCallbackThrowsExceptionWhenGivenNonCallable() - { - $this->expectException(\LogicException::class); - $this->expectExceptionMessage("SS_Filterable::filterByCallback() passed callback must be callable, 'boolean' given"); - $this->decorator->filterByCallback(true); - } - public function testFilterByCallback() { $input = new ArrayList([ @@ -163,8 +172,9 @@ public function testEach() $callable = function () { // noop }; - $this->list->expects($this->once())->method('each')->with($callable)->willReturn('mock'); - $this->assertSame('mock', $this->decorator->each($callable)); + $mock = $this->createMock(ArrayList::class); + $this->list->expects($this->once())->method('each')->with($callable)->willReturn($mock); + $this->assertSame($mock, $this->decorator->each($callable)); } public function testOffsetExists() @@ -180,20 +190,22 @@ public function testGetList() public function testColumnUnique() { - $this->list->expects($this->once())->method('columnUnique')->with('ID')->willReturn('mock'); - $this->assertSame('mock', $this->decorator->columnUnique('ID')); + $this->list->expects($this->once())->method('columnUnique')->with('ID')->willReturn(['foo']); + $this->assertSame(['foo'], $this->decorator->columnUnique('ID')); } public function testMap() { - $this->list->expects($this->once())->method('map')->with('ID', 'Title')->willReturn('mock'); - $this->assertSame('mock', $this->decorator->map('ID', 'Title')); + $return = new Map(new ArrayList()); + $this->list->expects($this->once())->method('map')->with('ID', 'Title')->willReturn($return); + $this->assertSame($return, $this->decorator->map('ID', 'Title')); } public function testReverse() { - $this->list->expects($this->once())->method('reverse')->willReturn('mock'); - $this->assertSame('mock', $this->decorator->reverse()); + $mock = $this->createMock(ArrayList::class); + $this->list->expects($this->once())->method('reverse')->willReturn($mock); + $this->assertSame($mock, $this->decorator->reverse()); } public function testOffsetGet() @@ -216,8 +228,9 @@ public function testByID() public function testByIDs() { - $this->list->expects($this->once())->method('byIDs')->with([1, 2])->willReturn('mock'); - $this->assertSame('mock', $this->decorator->byIDs([1, 2])); + $mock = $this->createMock(ArrayList::class); + $this->list->expects($this->once())->method('byIDs')->with([1, 2])->willReturn($mock); + $this->assertSame($mock, $this->decorator->byIDs([1, 2])); } public function testToArray() @@ -259,7 +272,7 @@ public function testTotalItems() public function testAdd() { - $this->list->expects($this->once())->method('add')->with('foo')->willReturn('mock'); + $this->list->expects($this->once())->method('add')->with('foo'); $this->decorator->add('foo'); } @@ -277,7 +290,7 @@ public function testLast() public function testColumn() { - $this->list->expects($this->once())->method('column')->with('DOB')->willReturn('mock'); - $this->assertSame('mock', $this->decorator->column('DOB')); + $this->list->expects($this->once())->method('column')->with('DOB')->willReturn(['foo']); + $this->assertSame(['foo'], $this->decorator->column('DOB')); } } diff --git a/tests/php/ORM/DataListTest.php b/tests/php/ORM/DataListTest.php index bb67daa199e..7b608096656 100755 --- a/tests/php/ORM/DataListTest.php +++ b/tests/php/ORM/DataListTest.php @@ -12,7 +12,7 @@ use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataQuery; use SilverStripe\ORM\DB; -use SilverStripe\Model\List\Filterable; +use SilverStripe\Model\List\SS_List; use SilverStripe\ORM\Filters\ExactMatchFilter; use SilverStripe\ORM\Tests\DataObjectTest\DataListQueryCounter; use SilverStripe\ORM\Tests\DataObjectTest\Fixture; @@ -521,6 +521,13 @@ public function testRemove() $this->assertNull($list->byID($obj->ID)); } + public function testRemoveWrongDataClass() + { + $this->expectException(InvalidArgumentException::class); + $list = Team::get(); + $list->remove(Player::get()->first()); + } + /** * Test DataList->removeByID() */ @@ -1591,7 +1598,7 @@ function ($item, $list) use ($team1ID) { $this->assertEquals(2, $list->count()); $this->assertEquals($expected, $result, 'List should only contain comments from Team 1 (Joe and Bob)'); - $this->assertTrue($list instanceof Filterable, 'The List should be of type SS_Filterable'); + $this->assertTrue($list instanceof SS_List, 'The List should be of type SS_List'); } /** diff --git a/tests/php/ORM/EagerLoadedListTest.php b/tests/php/ORM/EagerLoadedListTest.php index 3eca2f184ee..17859a7ff9b 100644 --- a/tests/php/ORM/EagerLoadedListTest.php +++ b/tests/php/ORM/EagerLoadedListTest.php @@ -10,7 +10,7 @@ use SilverStripe\ORM\Connect\MySQLiConnector; use SilverStripe\ORM\EagerLoadedList; use SilverStripe\ORM\DB; -use SilverStripe\Model\List\Filterable; +use SilverStripe\Model\List\SS_List; use SilverStripe\ORM\Tests\DataObjectTest\EquipmentCompany; use SilverStripe\ORM\Tests\DataObjectTest\Fan; use SilverStripe\ORM\Tests\DataObjectTest\Player; @@ -923,6 +923,13 @@ public function testRemove() $this->assertFalse($list->hasID($obj->ID)); } + public function testRemoveWrongDataClass() + { + $this->expectException(InvalidArgumentException::class); + $list = Team::get(); + $list->remove(Player::get()->first()); + } + public function testCanSortBy() { // Basic check @@ -1690,7 +1697,7 @@ function ($item, $list) use ($team1ID) { $this->assertEquals(2, $list->count()); $this->assertEquals($expected, $result, 'List should only contain comments from Team 1 (Joe and Bob)'); - $this->assertTrue($list instanceof Filterable, 'The List should be of type SS_Filterable'); + $this->assertTrue($list instanceof SS_List, 'The List should be of type SS_List'); } public function testSimpleExclude()