From 06b13e0fa621be58c7a9f43ffdc86c071d262fe4 Mon Sep 17 00:00:00 2001
From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com>
Date: Fri, 2 Sep 2022 10:58:37 +1200
Subject: [PATCH] Revert "Merge pull request #10450 from
creative-commoners/pulls/5/rescue-master-generators" (#10483)
This reverts commit 9edf3a5ca635f9687179143373fe25bddf10c5e3, reversing
changes made to 934fafd29d07a248e00b3055a2ae6d2edab243f6.
---
src/Forms/GridField/GridFieldExportButton.php | 4 +
src/ORM/ArrayList.php | 23 +-
src/ORM/Connect/DBSchemaManager.php | 2 +-
src/ORM/Connect/MySQLQuery.php | 35 ++-
src/ORM/Connect/MySQLStatement.php | 77 ++++---
src/ORM/Connect/PDOQuery.php | 23 +-
src/ORM/Connect/Query.php | 135 ++++++++++--
src/ORM/DataList.php | 67 +++---
src/ORM/ListDecorator.php | 4 +-
src/ORM/ManyManyList.php | 2 +-
src/ORM/Map.php | 57 ++---
src/ORM/Map_Iterator.php | 200 ++++++++++++++++++
src/ORM/PaginatedList.php | 26 +--
src/ORM/UnsavedRelationList.php | 6 +-
src/View/SSViewer_Scope.php | 20 +-
src/View/ViewableData.php | 6 +-
tests/php/ORM/DatabaseTest.php | 2 +-
tests/php/ORM/ListDecoratorTest.php | 6 +-
tests/php/ORM/MySQLDatabaseTest.php | 57 +++--
tests/php/ORM/PDODatabaseTest.php | 57 +++--
tests/php/ORM/PaginatedListTest.php | 20 +-
21 files changed, 567 insertions(+), 262 deletions(-)
create mode 100644 src/ORM/Map_Iterator.php
diff --git a/src/Forms/GridField/GridFieldExportButton.php b/src/Forms/GridField/GridFieldExportButton.php
index 9fc3ad3f36a..9e56b76aad2 100644
--- a/src/Forms/GridField/GridFieldExportButton.php
+++ b/src/Forms/GridField/GridFieldExportButton.php
@@ -223,6 +223,10 @@ public function generateExportFileData($gridField)
// Remove limit as the list may be paginated, we want the full list for the export
$items = $items->limit(null);
+ // Use Generator in applicable cases to reduce memory consumption
+ $items = $items instanceof DataList
+ ? $items->getGenerator()
+ : $items;
/** @var DataObject $item */
foreach ($items as $item) {
diff --git a/src/ORM/ArrayList.php b/src/ORM/ArrayList.php
index d8dca3195fe..74a7cd600ea 100644
--- a/src/ORM/ArrayList.php
+++ b/src/ORM/ArrayList.php
@@ -2,8 +2,8 @@
namespace SilverStripe\ORM;
+use ArrayIterator;
use InvalidArgumentException;
-use Iterator;
use LogicException;
use SilverStripe\Dev\Debug;
use SilverStripe\Dev\Deprecation;
@@ -103,16 +103,19 @@ public function exists()
/**
* Returns an Iterator for this ArrayList.
* This function allows you to use ArrayList in foreach loops
+ *
+ * @return ArrayIterator
*/
- public function getIterator(): Iterator
- {
- foreach ($this->items as $i => $item) {
- if (is_array($item)) {
- yield new ArrayData($item);
- } else {
- yield $item;
- }
- }
+ #[\ReturnTypeWillChange]
+ public function getIterator()
+ {
+ $items = array_map(
+ function ($item) {
+ return is_array($item) ? new ArrayData($item) : $item;
+ },
+ $this->items ?? []
+ );
+ return new ArrayIterator($items);
}
/**
diff --git a/src/ORM/Connect/DBSchemaManager.php b/src/ORM/Connect/DBSchemaManager.php
index 7a10cef8e93..7f93148cbd6 100644
--- a/src/ORM/Connect/DBSchemaManager.php
+++ b/src/ORM/Connect/DBSchemaManager.php
@@ -378,7 +378,7 @@ public function requireTable(
if ($dbID && isset($options[$dbID])) {
if (preg_match('/ENGINE=([^\s]*)/', $options[$dbID] ?? '', $alteredEngineMatches)) {
$alteredEngine = $alteredEngineMatches[1];
- $tableStatus = $this->query(sprintf('SHOW TABLE STATUS LIKE \'%s\'', $table))->record();
+ $tableStatus = $this->query(sprintf('SHOW TABLE STATUS LIKE \'%s\'', $table))->first();
$tableOptionsChanged = ($tableStatus['Engine'] != $alteredEngine);
}
}
diff --git a/src/ORM/Connect/MySQLQuery.php b/src/ORM/Connect/MySQLQuery.php
index d019167014b..c811386430a 100644
--- a/src/ORM/Connect/MySQLQuery.php
+++ b/src/ORM/Connect/MySQLQuery.php
@@ -2,8 +2,6 @@
namespace SilverStripe\ORM\Connect;
-use Iterator;
-
/**
* A result-set from a MySQL database (using MySQLiConnector)
* Note that this class is only used for the results of non-prepared statements
@@ -47,13 +45,16 @@ public function __destruct()
}
}
- public function getIterator(): Iterator
+ public function seek($row)
{
if (is_object($this->handle)) {
- while ($data = $this->handle->fetch_assoc()) {
- yield $data;
- }
+ // Fix for https://github.com/silverstripe/silverstripe-framework/issues/9097 without breaking the seek() API
+ $this->handle->data_seek($row);
+ $result = $this->nextRecord();
+ $this->handle->data_seek($row);
+ return $result;
}
+ return null;
}
public function numRecords()
@@ -61,7 +62,27 @@ public function numRecords()
if (is_object($this->handle)) {
return $this->handle->num_rows;
}
-
return null;
}
+
+ public function nextRecord()
+ {
+ $floatTypes = [MYSQLI_TYPE_FLOAT, MYSQLI_TYPE_DOUBLE, MYSQLI_TYPE_DECIMAL, MYSQLI_TYPE_NEWDECIMAL];
+
+ if (is_object($this->handle) && ($row = $this->handle->fetch_array(MYSQLI_NUM))) {
+ $data = [];
+ foreach ($row as $i => $value) {
+ if (!isset($this->columns[$i])) {
+ throw new DatabaseException("Can't get metadata for column $i");
+ }
+ if (in_array($this->columns[$i]->type, $floatTypes ?? [])) {
+ $value = (float)$value;
+ }
+ $data[$this->columns[$i]->name] = $value;
+ }
+ return $data;
+ } else {
+ return false;
+ }
+ }
}
diff --git a/src/ORM/Connect/MySQLStatement.php b/src/ORM/Connect/MySQLStatement.php
index eada8ad407b..7bc54765fcd 100644
--- a/src/ORM/Connect/MySQLStatement.php
+++ b/src/ORM/Connect/MySQLStatement.php
@@ -2,7 +2,6 @@
namespace SilverStripe\ORM\Connect;
-use Iterator;
use mysqli_result;
use mysqli_stmt;
@@ -57,26 +56,6 @@ class MySQLStatement extends Query
*/
protected $boundValues = [];
- /**
- * Hook the result-set given into a Query class, suitable for use by SilverStripe.
- * @param mysqli_stmt $statement The related statement, if present
- * @param mysqli_result $metadata The metadata for this statement
- */
- public function __construct($statement, $metadata)
- {
- $this->statement = $statement;
- $this->metadata = $metadata;
-
- // Immediately bind and buffer
- $this->bind();
- }
-
- public function __destruct()
- {
- $this->statement->close();
- $this->currentRecord = false;
- }
-
/**
* Binds this statement to the variables
*/
@@ -103,20 +82,58 @@ protected function bind()
call_user_func_array([$this->statement, 'bind_result'], $variables ?? []);
}
- public function getIterator(): Iterator
+ /**
+ * Hook the result-set given into a Query class, suitable for use by SilverStripe.
+ * @param mysqli_stmt $statement The related statement, if present
+ * @param mysqli_result $metadata The metadata for this statement
+ */
+ public function __construct($statement, $metadata)
+ {
+ $this->statement = $statement;
+ $this->metadata = $metadata;
+
+ // Immediately bind and buffer
+ $this->bind();
+ }
+
+ public function __destruct()
{
- while ($this->statement->fetch()) {
- // Dereferenced row
- $row = [];
- foreach ($this->boundValues as $key => $value) {
- $row[$key] = $value;
- }
- yield $row;
- }
+ $this->statement->close();
+ $this->currentRecord = false;
+ }
+
+ public function seek($row)
+ {
+ $this->rowNum = $row - 1;
+
+ // Fix for https://github.com/silverstripe/silverstripe-framework/issues/9097 without breaking the seek() API
+ $this->statement->data_seek($row);
+ $result = $this->next();
+ $this->statement->data_seek($row);
+ return $result;
}
public function numRecords()
{
return $this->statement->num_rows();
}
+
+ public function nextRecord()
+ {
+ // Skip data if out of data
+ if (!$this->statement->fetch()) {
+ return false;
+ }
+
+ // Dereferenced row
+ $row = [];
+ foreach ($this->boundValues as $key => $value) {
+ $floatTypes = [MYSQLI_TYPE_FLOAT, MYSQLI_TYPE_DOUBLE, MYSQLI_TYPE_DECIMAL, MYSQLI_TYPE_NEWDECIMAL];
+ if (in_array($this->types[$key], $floatTypes ?? [])) {
+ $value = (float)$value;
+ }
+ $row[$key] = $value;
+ }
+ return $row;
+ }
}
diff --git a/src/ORM/Connect/PDOQuery.php b/src/ORM/Connect/PDOQuery.php
index 24f9712eb8a..7180de28d0e 100644
--- a/src/ORM/Connect/PDOQuery.php
+++ b/src/ORM/Connect/PDOQuery.php
@@ -2,9 +2,6 @@
namespace SilverStripe\ORM\Connect;
-use ArrayIterator;
-use Iterator;
-
/**
* A result-set from a PDO database.
*/
@@ -17,7 +14,7 @@ class PDOQuery extends Query
/**
* Hook the result-set given into a Query class, suitable for use by SilverStripe.
- * @param PDOStatementHandle $statement The internal PDOStatement containing the results
+ * @param PDOStatement $statement The internal PDOStatement containing the results
*/
public function __construct(PDOStatementHandle $statement)
{
@@ -29,13 +26,25 @@ public function __construct(PDOStatementHandle $statement)
$statement->closeCursor();
}
- public function getIterator(): Iterator
+ public function seek($row)
{
- return new ArrayIterator($this->results);
+ $this->rowNum = $row - 1;
+ return $this->nextRecord();
}
public function numRecords()
{
- return count($this->results);
+ return count($this->results ?? []);
+ }
+
+ public function nextRecord()
+ {
+ $index = $this->rowNum + 1;
+
+ if (isset($this->results[$index])) {
+ return $this->results[$index];
+ } else {
+ return false;
+ }
}
}
diff --git a/src/ORM/Connect/Query.php b/src/ORM/Connect/Query.php
index 8aac231b2b5..af9e0c3a540 100644
--- a/src/ORM/Connect/Query.php
+++ b/src/ORM/Connect/Query.php
@@ -27,9 +27,30 @@
* on providing the specific data-access methods that are required: {@link nextRecord()}, {@link numRecords()}
* and {@link seek()}
*/
-abstract class Query implements \IteratorAggregate
+abstract class Query implements Iterator
{
+ /**
+ * The current record in the iterator.
+ *
+ * @var array
+ */
+ protected $currentRecord = null;
+
+ /**
+ * The number of the current row in the iterator.
+ *
+ * @var int
+ */
+ protected $rowNum = -1;
+
+ /**
+ * Flag to keep track of whether iteration has begun, to prevent unnecessary seeks
+ *
+ * @var bool
+ */
+ protected $queryHasBegun = false;
+
/**
* Return an array containing all the values from a specific column. If no column is set, then the first will be
* returned
@@ -41,7 +62,7 @@ public function column($column = null)
{
$result = [];
- foreach ($this as $record) {
+ while ($record = $this->next()) {
if ($column) {
$result[] = $record[$column];
} else {
@@ -61,7 +82,6 @@ public function column($column = null)
public function keyedColumn()
{
$column = [];
-
foreach ($this as $record) {
$val = $record[key($record)];
$column[$val] = $val;
@@ -86,22 +106,13 @@ public function map()
}
/**
- * Returns the first record in the result
+ * Returns the next record in the iterator.
*
* @return array
*/
public function record()
{
- return $this->getIterator()->current();
- }
-
- /**
- * @deprecated Use record() instead
- * @return array
- */
- public function first()
- {
- return $this->record();
+ return $this->next();
}
/**
@@ -111,7 +122,7 @@ public function first()
*/
public function value()
{
- $record = $this->record();
+ $record = $this->next();
if ($record) {
return $record[key($record)];
}
@@ -153,10 +164,94 @@ public function table()
return $result;
}
+ /**
+ * Iterator function implementation. Rewind the iterator to the first item and return it.
+ * Makes use of {@link seek()} and {@link numRecords()}, takes care of the plumbing.
+ *
+ * @return void
+ */
+ #[\ReturnTypeWillChange]
+ public function rewind()
+ {
+ if ($this->queryHasBegun && $this->numRecords() > 0) {
+ $this->queryHasBegun = false;
+ $this->currentRecord = null;
+ $this->seek(0);
+ }
+ }
+
+ /**
+ * Iterator function implementation. Return the current item of the iterator.
+ *
+ * @return array
+ */
+ #[\ReturnTypeWillChange]
+ public function current()
+ {
+ if (!$this->currentRecord) {
+ return $this->next();
+ } else {
+ return $this->currentRecord;
+ }
+ }
+
+ /**
+ * Iterator function implementation. Return the first item of this iterator.
+ *
+ * @return array
+ */
+ public function first()
+ {
+ $this->rewind();
+ return $this->current();
+ }
+
+ /**
+ * Iterator function implementation. Return the row number of the current item.
+ *
+ * @return int
+ */
+ #[\ReturnTypeWillChange]
+ public function key()
+ {
+ return $this->rowNum;
+ }
+
+ /**
+ * Iterator function implementation. Return the next record in the iterator.
+ * Makes use of {@link nextRecord()}, takes care of the plumbing.
+ *
+ * @return array
+ */
+ #[\ReturnTypeWillChange]
+ public function next()
+ {
+ $this->queryHasBegun = true;
+ $this->currentRecord = $this->nextRecord();
+ $this->rowNum++;
+ return $this->currentRecord;
+ }
+
+ /**
+ * Iterator function implementation. Check if the iterator is pointing to a valid item.
+ *
+ * @return bool
+ */
+ #[\ReturnTypeWillChange]
+ public function valid()
+ {
+ if (!$this->queryHasBegun) {
+ $this->next();
+ }
+ return $this->currentRecord !== false;
+ }
+
/**
* Return the next record in the query result.
+ *
+ * @return array
*/
- abstract public function getIterator(): Iterator;
+ abstract public function nextRecord();
/**
* Return the total number of items in the query result.
@@ -164,4 +259,12 @@ abstract public function getIterator(): Iterator;
* @return int
*/
abstract public function numRecords();
+
+ /**
+ * Go to a specific row number in the query result and return the record.
+ *
+ * @param int $rowNum Row number to go to.
+ * @return array
+ */
+ abstract public function seek($rowNum);
}
diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php
index 8dffc1dfe2b..72dfaf02ef2 100644
--- a/src/ORM/DataList.php
+++ b/src/ORM/DataList.php
@@ -6,12 +6,10 @@
use SilverStripe\Dev\Debug;
use SilverStripe\ORM\Filters\SearchFilter;
use SilverStripe\ORM\Queries\SQLConditionGroup;
-use SilverStripe\View\TemplateIterator;
use SilverStripe\View\ViewableData;
use ArrayIterator;
use Exception;
use InvalidArgumentException;
-use Iterator;
use LogicException;
/**
@@ -51,13 +49,6 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
*/
protected $dataQuery;
- /**
- * A cached Query to save repeated database calls. {@see DataList::getTemplateIteratorCount()}
- *
- * @var SilverStripe\ORM\Connect\Query
- */
- protected $finalisedQuery;
-
/**
* Create a new DataList.
* No querying is done on construction, but the initial query schema is set up.
@@ -88,7 +79,6 @@ public function dataClass()
public function __clone()
{
$this->dataQuery = clone $this->dataQuery;
- $this->finalisedQuery = null;
}
/**
@@ -791,6 +781,20 @@ public function each($callback)
return $this;
}
+ /**
+ * Returns a generator for this DataList
+ *
+ * @return \Generator&DataObject[]
+ */
+ public function getGenerator()
+ {
+ $query = $this->dataQuery->query()->execute();
+
+ while ($row = $query->record()) {
+ yield $this->createDataObject($row);
+ }
+ }
+
public function debug()
{
$val = "
" . static::class . "
";
@@ -859,32 +863,13 @@ public function getQueryParams()
/**
* Returns an Iterator for this DataList.
* This function allows you to use DataLists in foreach loops
- */
- public function getIterator(): Iterator
- {
- foreach ($this->getFinalisedQuery() as $row) {
- yield $this->createDataObject($row);
- }
-
- // Re-set the finaliseQuery so that it can be re-executed
- $this->finalisedQuery = null;
- }
-
- /**
- * Returns the Query result for this DataList. Repeated calls will return
- * a cached result, unless the DataQuery underlying this list has been
- * modified
*
- * @return SilverStripe\ORM\Connect\Query
- * @internal This API may change in minor releases
+ * @return ArrayIterator
*/
- protected function getFinalisedQuery()
+ #[\ReturnTypeWillChange]
+ public function getIterator()
{
- if (!$this->finalisedQuery) {
- $this->finalisedQuery = $this->dataQuery->query()->execute();
- }
-
- return $this->finalisedQuery;
+ return new ArrayIterator($this->toArray());
}
/**
@@ -895,10 +880,6 @@ protected function getFinalisedQuery()
#[\ReturnTypeWillChange]
public function count()
{
- if ($this->finalisedQuery) {
- return $this->finalisedQuery->numRecords();
- }
-
return $this->dataQuery->count();
}
@@ -1046,7 +1027,8 @@ public function byID($id)
*/
public function column($colName = "ID")
{
- return $this->dataQuery->distinct(false)->column($colName);
+ $dataQuery = clone $this->dataQuery;
+ return $dataQuery->distinct(false)->column($colName);
}
/**
@@ -1192,7 +1174,7 @@ public function shuffle()
*/
public function removeAll()
{
- foreach ($this as $item) {
+ foreach ($this->getGenerator() as $item) {
$this->remove($item);
}
return $this;
@@ -1335,15 +1317,14 @@ public function chunkedFetch(int $chunkSize = 1000): iterable
$currentChunk = 0;
// Keep looping until we run out of chunks
- while ($chunk = $this->limit($chunkSize, $chunkSize * $currentChunk)) {
+ while ($chunk = $this->limit($chunkSize, $chunkSize * $currentChunk)->getIterator()) {
// Loop over all the item in our chunk
- $count = 0;
foreach ($chunk as $item) {
- $count++;
yield $item;
}
- if ($count < $chunkSize) {
+
+ if ($chunk->count() < $chunkSize) {
// If our last chunk had less item than our chunkSize, we've reach the end.
break;
}
diff --git a/src/ORM/ListDecorator.php b/src/ORM/ListDecorator.php
index f34e5ea5a91..d3f006e64b5 100644
--- a/src/ORM/ListDecorator.php
+++ b/src/ORM/ListDecorator.php
@@ -2,7 +2,6 @@
namespace SilverStripe\ORM;
-use Iterator;
use SilverStripe\View\ViewableData;
use LogicException;
@@ -97,7 +96,8 @@ public function remove($itemObject)
$this->list->remove($itemObject);
}
- public function getIterator(): Iterator
+ #[\ReturnTypeWillChange]
+ public function getIterator()
{
return $this->list->getIterator();
}
diff --git a/src/ORM/ManyManyList.php b/src/ORM/ManyManyList.php
index ff841a9dc65..5ad69cd8f46 100644
--- a/src/ORM/ManyManyList.php
+++ b/src/ORM/ManyManyList.php
@@ -521,7 +521,7 @@ public function getExtraData($componentName, $itemID)
$query->addWhere([
"\"{$this->joinTable}\".\"{$this->localKey}\"" => $itemID
]);
- $queryResult = $query->execute()->record();
+ $queryResult = $query->execute()->current();
if ($queryResult) {
foreach ($queryResult as $fieldName => $value) {
$result[$fieldName] = $value;
diff --git a/src/ORM/Map.php b/src/ORM/Map.php
index a29116c539b..6bcd68494db 100644
--- a/src/ORM/Map.php
+++ b/src/ORM/Map.php
@@ -4,7 +4,6 @@
use ArrayAccess;
use Countable;
-use Iterator;
use IteratorAggregate;
/**
@@ -251,55 +250,23 @@ public function offsetUnset($key)
}
/**
- * Returns an Iterator for iterating over the complete set
+ * Returns an Map_Iterator instance for iterating over the complete set
* of items in the map.
*
- * Satisfies the IteratorAggregate interface.
- */
- public function getIterator(): Iterator
- {
- $keyField = $this->keyField;
- $valueField = $this->valueField;
-
- foreach ($this->firstItems as $k => $v) {
- yield $k => $v;
- }
-
- foreach ($this->list as $record) {
- if (isset($this->firstItems[$record->$keyField])) {
- continue;
- }
- if (isset($this->lastItems[$record->$keyField])) {
- continue;
- }
- yield $this->extractValue($record, $this->keyField) => $this->extractValue($record, $this->valueField);
- }
-
- foreach ($this->lastItems as $k => $v) {
- yield $k => $v;
- }
- }
-
- /**
- * Extracts a value from an item in the list, where the item is either an
- * object or array.
+ * Satisfies the IteratorAggreagte interface.
*
- * @param array|object $item
- * @param string $key
- * @return mixed
+ * @return Map_Iterator
*/
- protected function extractValue($item, $key)
+ #[\ReturnTypeWillChange]
+ public function getIterator()
{
- if (is_object($item)) {
- if (method_exists($item, 'hasMethod') && $item->hasMethod($key)) {
- return $item->{$key}();
- }
- return $item->{$key};
- } else {
- if (array_key_exists($key, $item)) {
- return $item[$key];
- }
- }
+ return new Map_Iterator(
+ $this->list->getIterator(),
+ $this->keyField,
+ $this->valueField,
+ $this->firstItems,
+ $this->lastItems
+ );
}
/**
diff --git a/src/ORM/Map_Iterator.php b/src/ORM/Map_Iterator.php
new file mode 100644
index 00000000000..d645ca07df8
--- /dev/null
+++ b/src/ORM/Map_Iterator.php
@@ -0,0 +1,200 @@
+items = $items;
+ $this->keyField = $keyField;
+ $this->titleField = $titleField;
+ $this->endItemIdx = null;
+
+ if ($firstItems) {
+ foreach ($firstItems as $k => $v) {
+ $this->firstItems[] = [$k, $v];
+ $this->excludedItems[] = $k;
+ }
+ }
+
+ if ($lastItems) {
+ foreach ($lastItems as $k => $v) {
+ $this->lastItems[] = [$k, $v];
+ $this->excludedItems[] = $k;
+ }
+ }
+ }
+
+ /**
+ * Rewind the Iterator to the first element.
+ *
+ * @return mixed
+ */
+ #[\ReturnTypeWillChange]
+ public function rewind()
+ {
+ $this->firstItemIdx = 0;
+ $this->endItemIdx = null;
+
+ $rewoundItem = $this->items->rewind();
+
+ if (isset($this->firstItems[$this->firstItemIdx])) {
+ return $this->firstItems[$this->firstItemIdx][1];
+ } else {
+ if ($rewoundItem) {
+ return $this->extractValue($rewoundItem, $this->titleField);
+ } else {
+ if (!$this->items->valid() && $this->lastItems) {
+ $this->endItemIdx = 0;
+
+ return $this->lastItems[0][1];
+ }
+ }
+ }
+ }
+
+ /**
+ * Return the current element.
+ *
+ * @return mixed
+ */
+ #[\ReturnTypeWillChange]
+ public function current()
+ {
+ if (($this->endItemIdx !== null) && isset($this->lastItems[$this->endItemIdx])) {
+ return $this->lastItems[$this->endItemIdx][1];
+ } else {
+ if (isset($this->firstItems[$this->firstItemIdx])) {
+ return $this->firstItems[$this->firstItemIdx][1];
+ }
+ }
+ return $this->extractValue($this->items->current(), $this->titleField);
+ }
+
+ /**
+ * Extracts a value from an item in the list, where the item is either an
+ * object or array.
+ *
+ * @param array|object $item
+ * @param string $key
+ * @return mixed
+ */
+ protected function extractValue($item, $key)
+ {
+ if (is_object($item)) {
+ if (method_exists($item, 'hasMethod') && $item->hasMethod($key)) {
+ return $item->{$key}();
+ }
+ return $item->{$key};
+ } else {
+ if (array_key_exists($key, $item ?? [])) {
+ return $item[$key];
+ }
+ }
+ }
+
+ /**
+ * Return the key of the current element.
+ *
+ * @return string
+ */
+ #[\ReturnTypeWillChange]
+ public function key()
+ {
+ if (($this->endItemIdx !== null) && isset($this->lastItems[$this->endItemIdx])) {
+ return $this->lastItems[$this->endItemIdx][0];
+ } else {
+ if (isset($this->firstItems[$this->firstItemIdx])) {
+ return $this->firstItems[$this->firstItemIdx][0];
+ } else {
+ return $this->extractValue($this->items->current(), $this->keyField);
+ }
+ }
+ }
+
+ /**
+ * Move forward to next element.
+ *
+ * @return mixed
+ */
+ #[\ReturnTypeWillChange]
+ public function next()
+ {
+ $this->firstItemIdx++;
+
+ if (isset($this->firstItems[$this->firstItemIdx])) {
+ return $this->firstItems[$this->firstItemIdx][1];
+ } else {
+ if (!isset($this->firstItems[$this->firstItemIdx - 1])) {
+ $this->items->next();
+ }
+
+ if ($this->excludedItems) {
+ while (($c = $this->items->current()) && in_array($c->{$this->keyField}, $this->excludedItems ?? [], true)) {
+ $this->items->next();
+ }
+ }
+ }
+
+ if (!$this->items->valid()) {
+ // iterator has passed the preface items, off the end of the items
+ // list. Track through the end items to go through to the next
+ if ($this->endItemIdx === null) {
+ $this->endItemIdx = -1;
+ }
+
+ $this->endItemIdx++;
+
+ if (isset($this->lastItems[$this->endItemIdx])) {
+ return $this->lastItems[$this->endItemIdx];
+ }
+
+ return false;
+ }
+ }
+
+ /**
+ * Checks if current position is valid.
+ *
+ * @return boolean
+ */
+ #[\ReturnTypeWillChange]
+ public function valid()
+ {
+ return (
+ (isset($this->firstItems[$this->firstItemIdx])) ||
+ (($this->endItemIdx !== null) && isset($this->lastItems[$this->endItemIdx])) ||
+ $this->items->valid()
+ );
+ }
+}
diff --git a/src/ORM/PaginatedList.php b/src/ORM/PaginatedList.php
index b962c0e546a..f7289e9a8bd 100644
--- a/src/ORM/PaginatedList.php
+++ b/src/ORM/PaginatedList.php
@@ -8,7 +8,6 @@
use SilverStripe\View\ArrayData;
use ArrayAccess;
use Exception;
-use Iterator;
use IteratorIterator;
/**
@@ -209,7 +208,11 @@ public function setLimitItems($limit)
return $this;
}
- public function getIterator(): Iterator
+ /**
+ * @return IteratorIterator
+ */
+ #[\ReturnTypeWillChange]
+ public function getIterator()
{
$pageLength = $this->getPageLength();
if ($this->limitItems && $pageLength) {
@@ -222,21 +225,6 @@ public function getIterator(): Iterator
}
}
- /**
- * @return array
- */
- public function toArray()
- {
- $result = [];
-
- // Use getIterator()
- foreach ($this as $record) {
- $result[] = $record;
- }
-
- return $result;
- }
-
/**
* Returns a set of links to all the pages in the list. This is useful for
* basic pagination.
@@ -356,8 +344,8 @@ public function PaginationSummary($context = 4)
$num = $i + 1;
$emptyRange = $num != 1 && $num != $total && (
- $num == $left - 1 || $num == $right + 1
- );
+ $num == $left - 1 || $num == $right + 1
+ );
if ($emptyRange) {
$result->push(new ArrayData([
diff --git a/src/ORM/UnsavedRelationList.php b/src/ORM/UnsavedRelationList.php
index 0206ef19edb..51cde920fd9 100644
--- a/src/ORM/UnsavedRelationList.php
+++ b/src/ORM/UnsavedRelationList.php
@@ -4,7 +4,6 @@
use InvalidArgumentException;
use ArrayIterator;
-use Iterator;
use SilverStripe\ORM\FieldType\DBField;
/**
@@ -121,8 +120,11 @@ public function dataClass()
/**
* Returns an Iterator for this relation.
+ *
+ * @return ArrayIterator
*/
- public function getIterator(): Iterator
+ #[\ReturnTypeWillChange]
+ public function getIterator()
{
return new ArrayIterator($this->toArray());
}
diff --git a/src/View/SSViewer_Scope.php b/src/View/SSViewer_Scope.php
index 28d1b541f0d..224fd07530a 100644
--- a/src/View/SSViewer_Scope.php
+++ b/src/View/SSViewer_Scope.php
@@ -3,7 +3,6 @@
namespace SilverStripe\View;
use ArrayIterator;
-use Countable;
use Iterator;
/**
@@ -290,31 +289,16 @@ public function next()
}
if (!$this->itemIterator) {
- // Note: it is important that getIterator() is called before count() as implemenations may rely on
- // this to efficiency get both the number of records and an iterator (e.g. DataList does this)
-
- // Item may be an array or a regular IteratorAggregate
if (is_array($this->item)) {
$this->itemIterator = new ArrayIterator($this->item);
} else {
$this->itemIterator = $this->item->getIterator();
-
- // This will execute code in a generator up to the first yield. For example, this ensures that
- // DataList::getIterator() is called before Datalist::count()
- $this->itemIterator->rewind();
- }
-
- // If the item implements Countable, use that to fetch the count, otherwise we have to inspect the
- // iterator and then rewind it.
- if ($this->item instanceof Countable) {
- $this->itemIteratorTotal = count($this->item);
- } else {
- $this->itemIteratorTotal = iterator_count($this->itemIterator);
- $this->itemIterator->rewind();
}
$this->itemStack[$this->localIndex][SSViewer_Scope::ITEM_ITERATOR] = $this->itemIterator;
+ $this->itemIteratorTotal = iterator_count($this->itemIterator); // Count the total number of items
$this->itemStack[$this->localIndex][SSViewer_Scope::ITEM_ITERATOR_TOTAL] = $this->itemIteratorTotal;
+ $this->itemIterator->rewind();
} else {
$this->itemIterator->next();
}
diff --git a/src/View/ViewableData.php b/src/View/ViewableData.php
index 4fd3051c206..2755b1da4a8 100644
--- a/src/View/ViewableData.php
+++ b/src/View/ViewableData.php
@@ -5,7 +5,6 @@
use ArrayIterator;
use Exception;
use InvalidArgumentException;
-use Iterator;
use IteratorAggregate;
use LogicException;
use SilverStripe\Core\ClassInfo;
@@ -574,8 +573,11 @@ public function getXMLValues($fields)
*
* This is useful so you can use a single record inside a <% control %> block in a template - and then use
* to access individual fields on this object.
+ *
+ * @return ArrayIterator
*/
- public function getIterator(): Iterator
+ #[\ReturnTypeWillChange]
+ public function getIterator()
{
return new ArrayIterator([$this]);
}
diff --git a/tests/php/ORM/DatabaseTest.php b/tests/php/ORM/DatabaseTest.php
index 33a45caaf38..25fa3deed8e 100644
--- a/tests/php/ORM/DatabaseTest.php
+++ b/tests/php/ORM/DatabaseTest.php
@@ -74,7 +74,7 @@ public function testMySQLCreateTableOptions()
'SHOW TABLE STATUS WHERE "Name" = \'%s\'',
'DatabaseTest_MyObject'
)
- )->record();
+ )->first();
$this->assertEquals(
$ret['Engine'],
'InnoDB',
diff --git a/tests/php/ORM/ListDecoratorTest.php b/tests/php/ORM/ListDecoratorTest.php
index 0410bba7653..2e85f50c970 100644
--- a/tests/php/ORM/ListDecoratorTest.php
+++ b/tests/php/ORM/ListDecoratorTest.php
@@ -2,7 +2,6 @@
namespace SilverStripe\ORM\Tests;
-use ArrayIterator;
use LogicException;
use PHPUnit\Framework\MockObject\MockObject;
use SilverStripe\Dev\SapphireTest;
@@ -35,9 +34,8 @@ protected function setUp(): void
public function testGetIterator()
{
- $iterator = new ArrayIterator();
- $this->list->expects($this->once())->method('getIterator')->willReturn($iterator);
- $this->assertSame($iterator, $this->decorator->getIterator());
+ $this->list->expects($this->once())->method('getIterator')->willReturn('mock');
+ $this->assertSame('mock', $this->decorator->getIterator());
}
public function testCanSortBy()
diff --git a/tests/php/ORM/MySQLDatabaseTest.php b/tests/php/ORM/MySQLDatabaseTest.php
index e4ffdfd2fd6..7b084b04522 100644
--- a/tests/php/ORM/MySQLDatabaseTest.php
+++ b/tests/php/ORM/MySQLDatabaseTest.php
@@ -45,47 +45,58 @@ public function testPreparedStatements()
$this->assertInstanceOf(MySQLQuery::class, $result3);
// Iterating one level should not buffer, but return the right result
- $result1Array = [];
- foreach ($result1 as $record) {
- $result1Array[] = $record;
- }
$this->assertEquals(
[
- [ 'Sort' => 1, 'Title' => 'First Item' ],
- [ 'Sort' => 2, 'Title' => 'Second Item' ],
- [ 'Sort' => 3, 'Title' => 'Third Item' ],
- [ 'Sort' => 4, 'Title' => 'Last Item' ],
+ 'Sort' => 1,
+ 'Title' => 'First Item'
+ ],
+ $result1->next()
+ );
+ $this->assertEquals(
+ [
+ 'Sort' => 2,
+ 'Title' => 'Second Item'
],
- $result1Array
+ $result1->next()
+ );
+
+ // Test first
+ $this->assertEquals(
+ [
+ 'Sort' => 1,
+ 'Title' => 'First Item'
+ ],
+ $result1->first()
+ );
+
+ // Test seek
+ $this->assertEquals(
+ [
+ 'Sort' => 2,
+ 'Title' => 'Second Item'
+ ],
+ $result1->seek(1)
);
// Test count
$this->assertEquals(4, $result1->numRecords());
// Test second statement
- $result2Array = [];
- foreach ($result2 as $record) {
- $result2Array[] = $record;
- break;
- }
$this->assertEquals(
[
- [ 'Sort' => 3, 'Title' => 'Third Item' ],
+ 'Sort' => 3,
+ 'Title' => 'Third Item'
],
- $result2Array
+ $result2->next()
);
// Test non-prepared query
- $result3Array = [];
- foreach ($result3 as $record) {
- $result3Array[] = $record;
- break;
- }
$this->assertEquals(
[
- [ 'Sort' => 1, 'Title' => 'First Item' ],
+ 'Sort' => 1,
+ 'Title' => 'First Item'
],
- $result3Array
+ $result3->next()
);
}
diff --git a/tests/php/ORM/PDODatabaseTest.php b/tests/php/ORM/PDODatabaseTest.php
index 5462056711a..fa91676c536 100644
--- a/tests/php/ORM/PDODatabaseTest.php
+++ b/tests/php/ORM/PDODatabaseTest.php
@@ -43,47 +43,58 @@ public function testPreparedStatements()
$this->assertInstanceOf(PDOQuery::class, $result3);
// Iterating one level should not buffer, but return the right result
- $result1Array = [];
- foreach ($result1 as $record) {
- $result1Array[] = $record;
- }
$this->assertEquals(
[
- [ 'Sort' => 1, 'Title' => 'First Item' ],
- [ 'Sort' => 2, 'Title' => 'Second Item' ],
- [ 'Sort' => 3, 'Title' => 'Third Item' ],
- [ 'Sort' => 4, 'Title' => 'Last Item' ],
+ 'Sort' => 1,
+ 'Title' => 'First Item'
+ ],
+ $result1->next()
+ );
+ $this->assertEquals(
+ [
+ 'Sort' => 2,
+ 'Title' => 'Second Item'
],
- $result1Array
+ $result1->next()
+ );
+
+ // Test first
+ $this->assertEquals(
+ [
+ 'Sort' => 1,
+ 'Title' => 'First Item'
+ ],
+ $result1->first()
+ );
+
+ // Test seek
+ $this->assertEquals(
+ [
+ 'Sort' => 2,
+ 'Title' => 'Second Item'
+ ],
+ $result1->seek(1)
);
// Test count
$this->assertEquals(4, $result1->numRecords());
// Test second statement
- $result2Array = [];
- foreach ($result2 as $record) {
- $result2Array[] = $record;
- break;
- }
$this->assertEquals(
[
- [ 'Sort' => 3, 'Title' => 'Third Item' ],
+ 'Sort' => 3,
+ 'Title' => 'Third Item'
],
- $result2Array
+ $result2->next()
);
// Test non-prepared query
- $result3Array = [];
- foreach ($result3 as $record) {
- $result3Array[] = $record;
- break;
- }
$this->assertEquals(
[
- [ 'Sort' => 1, 'Title' => 'First Item' ],
+ 'Sort' => 1,
+ 'Title' => 'First Item'
],
- $result3Array
+ $result3->next()
);
}
diff --git a/tests/php/ORM/PaginatedListTest.php b/tests/php/ORM/PaginatedListTest.php
index f8a1e2c9772..fb63403d6ec 100644
--- a/tests/php/ORM/PaginatedListTest.php
+++ b/tests/php/ORM/PaginatedListTest.php
@@ -90,7 +90,7 @@ public function testSetCurrentPage()
$this->assertEquals(1, $list->CurrentPage());
}
- public function testIteration()
+ public function testGetIterator()
{
$list = new PaginatedList(
new ArrayList([
@@ -105,23 +105,26 @@ public function testIteration()
$this->assertListEquals(
[['Num' => 1], ['Num' => 2]],
- $list
+ ArrayList::create($list->getIterator()->getInnerIterator()->getArrayCopy())
);
$list->setCurrentPage(2);
$this->assertListEquals(
[['Num' => 3], ['Num' => 4]],
- $list
+ ArrayList::create($list->getIterator()->getInnerIterator()->getArrayCopy())
);
$list->setCurrentPage(3);
$this->assertListEquals(
[['Num' => 5]],
- $list
+ ArrayList::create($list->getIterator()->getInnerIterator()->getArrayCopy())
);
$list->setCurrentPage(999);
- $this->assertListEquals([], $list);
+ $this->assertListEquals(
+ [],
+ ArrayList::create($list->getIterator()->getInnerIterator()->getArrayCopy())
+ );
// Test disabled paging
$list->setPageLength(0);
@@ -134,13 +137,14 @@ public function testIteration()
['Num' => 4],
['Num' => 5],
],
- $list
+ ArrayList::create($list->getIterator()->getInnerIterator()->getArrayCopy())
);
// Test with dataobjectset
$players = Player::get();
$list = new PaginatedList($players);
$list->setPageLength(1);
+ $list->getIterator();
$this->assertEquals(
4,
$list->getTotalItems(),
@@ -219,10 +223,10 @@ public function testLimitItems()
$list = new PaginatedList($list);
$list->setCurrentPage(3);
- $this->assertEquals(10, count($list->toArray()));
+ $this->assertCount(10, $list->getIterator()->getInnerIterator());
$list->setLimitItems(false);
- $this->assertEquals(50, count($list->toArray()));
+ $this->assertCount(50, $list->getIterator()->getInnerIterator());
}
public function testCurrentPage()