Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Generators for ORM Query, Map, ArrayList #6518

Merged
merged 14 commits into from Mar 17, 2018
Merged
6 changes: 6 additions & 0 deletions docs/en/04_Changelogs/5.0.0.md
Expand Up @@ -39,4 +39,10 @@ guide developers in preparing existing 4.x code for compatibility with 5.0.
the returned DataList instead.
* Removed `Versioned::publish()` (previously deprecated in 4.0 and marked for removal in 5.0)

### ORM

* `Query::first()` and `Query::current()` removed. Use `Query::record()` instead.
* `Query::seek()` removed. To re-set the position, re-execute `Query::getIterator()` instead.
* `Query::next()` and other Iterator methods removed. Call `Query::getIterator()` to get an iterator instead.

<!--- Changes below this line will be automatically regenerated -->
5 changes: 3 additions & 2 deletions src/ORM/ArrayList.php
Expand Up @@ -86,10 +86,11 @@ public function getIterator()
{
foreach ($this->items as $i => $item) {
if (is_array($item)) {
$this->items[$i] = new ArrayData($item);
yield new ArrayData($item);
} else {
yield $item;
}
}
return new ArrayIterator($this->items);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/ORM/Connect/DBSchemaManager.php
Expand Up @@ -361,7 +361,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))->first();
$tableStatus = $this->query(sprintf('SHOW TABLE STATUS LIKE \'%s\'', $table))->record();
$tableOptionsChanged = ($tableStatus['Engine'] != $alteredEngine);
}
}
Expand Down
18 changes: 5 additions & 13 deletions src/ORM/Connect/MySQLQuery.php
Expand Up @@ -36,29 +36,21 @@ public function __destruct()
}
}

public function seek($row)
public function getIterator()
{
if (is_object($this->handle)) {
$this->handle->data_seek($row);
return $this->handle->fetch_assoc();
while ($data = $this->handle->fetch_assoc()) {
yield $data;
}
}
return null;
}

public function numRecords()
{
if (is_object($this->handle)) {
return $this->handle->num_rows;
}
return null;
}

public function nextRecord()
{
if (is_object($this->handle) && ($data = $this->handle->fetch_assoc())) {
return $data;
} else {
return false;
}
return null;
}
}
73 changes: 29 additions & 44 deletions src/ORM/Connect/MySQLStatement.php
Expand Up @@ -49,6 +49,26 @@ class MySQLStatement extends Query
*/
protected $boundValues = array();

/**
* 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
*/
Expand All @@ -74,55 +94,20 @@ protected function bind()
call_user_func_array(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)
{
$this->statement = $statement;
$this->metadata = $metadata;

// Immediately bind and buffer
$this->bind();
}

public function __destruct()
public function getIterator()
{
$this->statement->close();
$this->currentRecord = false;
}

public function seek($row)
{
$this->rowNum = $row - 1;
$this->statement->data_seek($row);
return $this->next();
while ($this->statement->fetch()) {
// Dereferenced row
$row = array();
foreach ($this->boundValues as $key => $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 = array();
foreach ($this->boundValues as $key => $value) {
$row[$key] = $value;
}
return $row;
}

public function rewind()
{
return $this->seek(0);
}
}
17 changes: 3 additions & 14 deletions src/ORM/Connect/PDOQuery.php
Expand Up @@ -4,6 +4,7 @@

use PDOStatement;
use PDO;
use ArrayIterator;

/**
* A result-set from a PDO database.
Expand Down Expand Up @@ -32,25 +33,13 @@ public function __construct(PDOStatement $statement)
$statement->closeCursor();
}

public function seek($row)
public function getIterator()
{
$this->rowNum = $row - 1;
return $this->nextRecord();
return new ArrayIterator($this->results);
}

public function numRecords()
{
return count($this->results);
}

public function nextRecord()
{
$index = $this->rowNum + 1;

if (isset($this->results[$index])) {
return $this->results[$index];
} else {
return false;
}
}
}
133 changes: 18 additions & 115 deletions src/ORM/Connect/Query.php
Expand Up @@ -15,30 +15,9 @@
* 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 interator.
*
* @var array
*/
protected $currentRecord = null;

/**
* The number of the current row in the interator.
*
* @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
Expand All @@ -48,9 +27,9 @@ abstract class Query implements Iterator
*/
public function column($column = null)
{
$result = array();
$result = [];

while ($record = $this->next()) {
foreach ($this as $record) {
if ($column) {
$result[] = $record[$column];
} else {
Expand All @@ -69,7 +48,8 @@ public function column($column = null)
*/
public function keyedColumn()
{
$column = array();
$column = [];

foreach ($this as $record) {
$val = $record[key($record)];
$column[$val] = $val;
Expand All @@ -94,13 +74,22 @@ public function map()
}

/**
* 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();
}

/**
* @deprecated Use record() instead
* @return array
*/
public function first()
{
return $this->record();
}

/**
Expand All @@ -110,7 +99,7 @@ public function record()
*/
public function value()
{
$record = $this->next();
$record = $this->record();
if ($record) {
return $record[key($record)];
}
Expand Down Expand Up @@ -152,103 +141,17 @@ 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 array
*/
public function rewind()
{
if ($this->queryHasBegun && $this->numRecords() > 0) {
$this->queryHasBegun = false;
$this->currentRecord = null;
return $this->seek(0);
}
return null;
}

/**
* Iterator function implementation. Return the current item of the iterator.
*
* @return array
*/
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
*/
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
*/
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
*/
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.
*
* @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);
}