From 77c7552c3fc2c53b08cc774c3734e3243b07c4e4 Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Tue, 17 Jan 2017 15:20:22 +1300 Subject: [PATCH] =?UTF-8?q?NEW:=20ORM=E2=80=99=20Query=20is=20a=20generato?= =?UTF-8?q?r-based=20IteratorAggregate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit API: Query no longer has iterator methods current(), first(), rewind(), next() Using generators reduces the amount of boilerplate needed for this code. Turning it into an IteratorAggregate means that the iterator can be re-created for each subsequent foreach call. This means that the rewind() and seek() functionality can be discarded. --- src/ORM/Connect/DBSchemaManager.php | 2 +- src/ORM/Connect/MySQLQuery.php | 44 ++++------ src/ORM/Connect/MySQLStatement.php | 80 ++++++++---------- src/ORM/Connect/Query.php | 124 ++-------------------------- src/ORM/ManyManyList.php | 2 +- tests/php/ORM/DatabaseTest.php | 2 +- 6 files changed, 59 insertions(+), 195 deletions(-) diff --git a/src/ORM/Connect/DBSchemaManager.php b/src/ORM/Connect/DBSchemaManager.php index 7f93148cb..7a10cef8e 100644 --- a/src/ORM/Connect/DBSchemaManager.php +++ b/src/ORM/Connect/DBSchemaManager.php @@ -378,7 +378,7 @@ abstract class DBSchemaManager 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))->first(); + $tableStatus = $this->query(sprintf('SHOW TABLE STATUS LIKE \'%s\'', $table))->record(); $tableOptionsChanged = ($tableStatus['Engine'] != $alteredEngine); } } diff --git a/src/ORM/Connect/MySQLQuery.php b/src/ORM/Connect/MySQLQuery.php index c81138643..e42dddee8 100644 --- a/src/ORM/Connect/MySQLQuery.php +++ b/src/ORM/Connect/MySQLQuery.php @@ -45,16 +45,24 @@ class MySQLQuery extends Query } } - public function seek($row) + public function getIterator() { + $floatTypes = [MYSQLI_TYPE_FLOAT, MYSQLI_TYPE_DOUBLE, MYSQLI_TYPE_DECIMAL, MYSQLI_TYPE_NEWDECIMAL]; if (is_object($this->handle)) { - // 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; + while ($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; + } + yield $data; + } } - return null; } public function numRecords() @@ -62,27 +70,7 @@ class MySQLQuery extends Query 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 7bc54765f..b46390b8a 100644 --- a/src/ORM/Connect/MySQLStatement.php +++ b/src/ORM/Connect/MySQLStatement.php @@ -56,6 +56,26 @@ 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 */ @@ -82,58 +102,24 @@ class MySQLStatement extends Query call_user_func_array([$this->statement, 'bind_result'], $variables ?? []); } - /** - * 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) + public function getIterator() { - $this->statement = $statement; - $this->metadata = $metadata; - - // Immediately bind and buffer - $this->bind(); - } - - public function __destruct() - { - $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; + while($this->statement->fetch()) { + // 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; + } + yield $row; + } } 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/Query.php b/src/ORM/Connect/Query.php index af9e0c3a5..1d7497ffa 100644 --- a/src/ORM/Connect/Query.php +++ b/src/ORM/Connect/Query.php @@ -27,30 +27,9 @@ use Iterator; * on providing the specific data-access methods that are required: {@link nextRecord()}, {@link numRecords()} * and {@link seek()} */ -abstract class Query implements Iterator +abstract class Query implements \IteratorAggregate { - /** - * 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 @@ -62,7 +41,7 @@ abstract class Query implements Iterator { $result = []; - while ($record = $this->next()) { + foreach ($this as $record) { if ($column) { $result[] = $record[$column]; } else { @@ -82,6 +61,7 @@ abstract class Query implements Iterator public function keyedColumn() { $column = []; + foreach ($this as $record) { $val = $record[key($record)]; $column[$val] = $val; @@ -106,13 +86,13 @@ abstract class Query implements Iterator } /** - * Returns the next record in the iterator. + * Returns the first record in the result * * @return array */ public function record() { - return $this->next(); + return $this->getIterator()->current(); } /** @@ -122,7 +102,7 @@ abstract class Query implements Iterator */ public function value() { - $record = $this->next(); + $record = $this->record(); if ($record) { return $record[key($record)]; } @@ -164,94 +144,12 @@ abstract class Query implements Iterator 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 nextRecord(); + abstract public function getIterator(); /** * Return the total number of items in the query result. @@ -259,12 +157,4 @@ abstract class Query implements 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/ManyManyList.php b/src/ORM/ManyManyList.php index 5ad69cd8f..ff841a9dc 100644 --- a/src/ORM/ManyManyList.php +++ b/src/ORM/ManyManyList.php @@ -521,7 +521,7 @@ class ManyManyList extends RelationList $query->addWhere([ "\"{$this->joinTable}\".\"{$this->localKey}\"" => $itemID ]); - $queryResult = $query->execute()->current(); + $queryResult = $query->execute()->record(); if ($queryResult) { foreach ($queryResult as $fieldName => $value) { $result[$fieldName] = $value; diff --git a/tests/php/ORM/DatabaseTest.php b/tests/php/ORM/DatabaseTest.php index 25fa3deed..33a45caaf 100644 --- a/tests/php/ORM/DatabaseTest.php +++ b/tests/php/ORM/DatabaseTest.php @@ -74,7 +74,7 @@ class DatabaseTest extends SapphireTest 'SHOW TABLE STATUS WHERE "Name" = \'%s\'', 'DatabaseTest_MyObject' ) - )->first(); + )->record(); $this->assertEquals( $ret['Engine'], 'InnoDB',