Skip to content

Commit

Permalink
Separate execute() into 2 methods.
Browse files Browse the repository at this point in the history 
Split execute() into 2 methods. ORM\Query::execute() now only ever
returns a StatementInterface, while Query::all() returns a ResultSet
object. This allows each method to have a consistent return value and
allows update/delete/insert queries to not trigger Model.beforeFind
events incorrectly.

I'm not sold on the method names, but I think the separation they create
is a useful change.
  • Loading branch information
@markstory
markstory committed Dec 30, 2013
1 parent 02efb76 commit c7ef7f5
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 90 deletions.
9 changes: 0 additions & 9 deletions Cake/Database/Query.php
View file
Expand Up @@ -202,15 +202,6 @@ public function connection($connection = null) {
* @return Cake\Database\StatementInterface
*/
public function execute() {
return $this->_executeStatement();
}

/**
* Internal implementation of statement execution.
*
* @return Cake\Database\StatementInterface
*/
protected function _executeStatement() {
$query = $this->_transformQuery();
$statement = $this->_connection->prepare($query->sql());
$query->_bindStatement($statement);
Expand Down
2 changes: 1 addition & 1 deletion Cake/ORM/Association/BelongsToMany.php
View file
Expand Up @@ -254,7 +254,7 @@ public function eagerLoader(array $options) {
$property = $this->target()->association($this->junction()->alias())->property();
$hydrated = $fetchQuery->hydrate();

foreach ($fetchQuery->execute() as $result) {
foreach ($fetchQuery->all() as $result) {
$result[$this->_junctionProperty] = $result[$property];
unset($result[$property]);

Expand Down
2 changes: 1 addition & 1 deletion Cake/ORM/Association/HasMany.php
View file
Expand Up @@ -87,7 +87,7 @@ public function eagerLoader(array $options) {
$fetchQuery = $this->_buildQuery($options);
$resultMap = [];
$key = $options['foreignKey'];
foreach ($fetchQuery->execute() as $result) {
foreach ($fetchQuery->all() as $result) {
$resultMap[$result[$key]][] = $result;
}

Expand Down
57 changes: 38 additions & 19 deletions Cake/ORM/Query.php
View file
Expand Up @@ -401,29 +401,43 @@ public function setResult($results) {
}

/**
* Compiles the SQL representation of this query and executes it using the
* provided connection object. Returns a ResultSet iterator object.
* Executes this query and returns a results iterator. This function is required
* for implementing the IteratorAggregate interface and allows the query to be
* iterated without having to call execute() manually, thus making it look like
* a result set instead of the query itself.
*
* This method fires the `Model.beforeFind` event and processes the
* callback results.
* @return Iterator
*/
public function getIterator() {
if (empty($this->_iterator) || $this->_dirty) {
$this->_iterator = $this->all();
}
return $this->_iterator;
}

/**
* Fetch the results for this query.
*
* If a result set was set using setResult() that ResultSet will be returned.
* Compiles the SQL representation of this query and executes it using the
* provided connection object. Returns a ResultSet iterator object.
*
* Resulting object is traversable, so it can be used in any loop as you would
* with an array.
* ResultSet is a travesable object that implements the methods found
* on Cake\Collection\Collection.
*
* @return Cake\ORM\ResultCollectionTrait|Cake\Database\StatementInterface
* A result set will be returned for select queries. For insert, delete and update
* queries a statement will be returned.
* @return Cake\ORM\ResultCollectionTrait
* @throws RuntimeException if this method is called on a non-select Query.
*/
public function execute() {
if ($this->_type === 'select') {
$table = $this->repository();
$event = new Event('Model.beforeFind', $table, [$this, $this->_options]);
$table->getEventManager()->dispatch($event);
return $this->getResults();
public function all() {
if ($this->_type !== 'select') {
throw new \RuntimeException(__d(
'cake_dev',
'You cannot call all() on a non-select query. Use execute() instead.'
));
}
return $this->_executeStatement();
$table = $this->repository();
$event = new Event('Model.beforeFind', $table, [$this, $this->_options]);
$table->getEventManager()->dispatch($event);
return $this->getResults();
}

/**
Expand All @@ -440,7 +454,7 @@ public function getResults() {
}

$this->_results = $this->_decorateResults(
new ResultSet($this, $this->_executeStatement())
new ResultSet($this, $this->execute())
);

return $this->_results;
Expand All @@ -452,7 +466,7 @@ public function getResults() {
* @return array
*/
public function toArray() {
return $this->execute()->toArray();
return $this->all()->toArray();
}

/**
Expand Down Expand Up @@ -637,8 +651,13 @@ public function first() {
$this->limit(1);
}
$this->bufferResults();
<<<<<<< HEAD
$this->_results = $this->execute();
return $this->_results->first();
=======
$this->_results = $this->all();
return $this->_results->one();
>>>>>>> Separate execute() into 2 methods.
}

/**
Expand Down
41 changes: 27 additions & 14 deletions Cake/Test/TestCase/ORM/Association/BelongsToManyTest.php
View file
Expand Up @@ -319,15 +319,19 @@ public function testEagerLoader() {
]);
$association = new BelongsToMany('Tags', $config);
$keys = [1, 2, 3, 4];
$query = $this->getMock('Cake\ORM\Query', ['execute', 'contain'], [null, null]);
$query = $this->getMock('Cake\ORM\Query', ['all', 'contain'], [null, null]);

$this->tag->expects($this->once())->method('find')->with('all')
$this->tag->expects($this->once())
->method('find')
->with('all')
->will($this->returnValue($query));

$results = [
['id' => 1, 'name' => 'foo', 'articles_tags' => ['article_id' => 1]],
['id' => 2, 'name' => 'bar', 'articles_tags' => ['article_id' => 2]]
];
$query->expects($this->once())->method('execute')
$query->expects($this->once())
->method('all')
->will($this->returnValue($results));

$query->expects($this->once())->method('contain')
Expand Down Expand Up @@ -378,18 +382,22 @@ public function testEagerLoaderWithDefaults() {
]);
$association = new BelongsToMany('Tags', $config);
$keys = [1, 2, 3, 4];
$methods = ['execute', 'contain', 'where', 'order'];
$methods = ['all', 'contain', 'where', 'order'];
$query = $this->getMock('Cake\ORM\Query', $methods, [null, null]);
$this->tag->expects($this->once())->method('find')->with('all')
$this->tag->expects($this->once())
->method('find')
->with('all')
->will($this->returnValue($query));
$results = [
['id' => 1, 'name' => 'foo', 'articles_tags' => ['article_id' => 1]],
['id' => 2, 'name' => 'bar', 'articles_tags' => ['article_id' => 2]]
];
$query->expects($this->once())->method('execute')
$query->expects($this->once())
->method('all')
->will($this->returnValue($results));

$query->expects($this->once())->method('contain')
$query->expects($this->once())
->method('contain')
->with([
'ArticlesTags' => [
'conditions' => ['ArticlesTags.article_id in' => $keys],
Expand Down Expand Up @@ -435,15 +443,15 @@ public function testEagerLoaderWithOverrides() {
]);
$association = new BelongsToMany('Tags', $config);
$keys = [1, 2, 3, 4];
$methods = ['execute', 'contain', 'where', 'order', 'select'];
$methods = ['all', 'contain', 'where', 'order', 'select'];
$query = $this->getMock('Cake\ORM\Query', $methods, [null, null]);
$this->tag->expects($this->once())->method('find')->with('all')
->will($this->returnValue($query));
$results = [
['id' => 1, 'name' => 'foo', 'articles_tags' => ['article_id' => 1]],
['id' => 2, 'name' => 'bar', 'articles_tags' => ['article_id' => 2]]
];
$query->expects($this->once())->method('execute')
$query->expects($this->once())->method('all')
->will($this->returnValue($results));

$query->expects($this->once())->method('contain')
Expand Down Expand Up @@ -508,9 +516,11 @@ public function testEagerLoaderFieldsException() {
]);
$association = new BelongsToMany('Tags', $config);
$keys = [1, 2, 3, 4];
$methods = ['execute', 'contain', 'where', 'order', 'select'];
$methods = ['all', 'contain', 'where', 'order', 'select'];
$query = $this->getMock('Cake\ORM\Query', $methods, [null, null]);
$this->tag->expects($this->once())->method('find')->with('all')
$this->tag->expects($this->once())
->method('find')
->with('all')
->will($this->returnValue($query));
$query->expects($this->any())->method('contain')->will($this->returnSelf());

Expand Down Expand Up @@ -550,18 +560,21 @@ public function testEagerLoaderSubquery() {

$query = $this->getMock(
'Cake\ORM\Query',
['execute', 'where', 'andWhere', 'order', 'select', 'contain'],
['all', 'where', 'andWhere', 'order', 'select', 'contain'],
[null, null]
);
$query->hydrate(false);

$this->tag->expects($this->once())->method('find')->with('all')
$this->tag->expects($this->once())
->method('find')
->with('all')
->will($this->returnValue($query));
$results = [
['id' => 1, 'name' => 'foo', 'articles_tags' => ['article_id' => 1]],
['id' => 2, 'name' => 'bar', 'articles_tags' => ['article_id' => 2]]
];
$query->expects($this->once())->method('execute')
$query->expects($this->once())
->method('all')
->will($this->returnValue($results));

$expected = clone $parent;
Expand Down
18 changes: 9 additions & 9 deletions Cake/Test/TestCase/ORM/Association/HasManyTest.php
View file
Expand Up @@ -119,14 +119,14 @@ public function testEagerLoader() {
];
$association = new HasMany('Articles', $config);
$keys = [1, 2, 3, 4];
$query = $this->getMock('Cake\ORM\Query', ['execute'], [null, null]);
$query = $this->getMock('Cake\ORM\Query', ['all'], [null, null]);
$this->article->expects($this->once())->method('find')->with('all')
->will($this->returnValue($query));
$results = [
['id' => 1, 'title' => 'article 1', 'author_id' => 2],
['id' => 2, 'title' => 'article 2', 'author_id' => 1]
];
$query->expects($this->once())->method('execute')
$query->expects($this->once())->method('all')
->will($this->returnValue($results));

$callable = $association->eagerLoader(compact('keys', 'query'));
Expand Down Expand Up @@ -162,7 +162,7 @@ public function testEagerLoaderWithDefaults() {
$keys = [1, 2, 3, 4];
$query = $this->getMock(
'Cake\ORM\Query',
['execute', 'where', 'andWhere', 'order'],
['all', 'where', 'andWhere', 'order'],
[null, null]
);
$this->article->expects($this->once())->method('find')->with('all')
Expand All @@ -172,7 +172,7 @@ public function testEagerLoaderWithDefaults() {
['id' => 2, 'title' => 'article 2', 'author_id' => 1]
];

$query->expects($this->once())->method('execute')
$query->expects($this->once())->method('all')
->will($this->returnValue($results));

$query->expects($this->at(0))->method('where')
Expand Down Expand Up @@ -211,7 +211,7 @@ public function testEagerLoaderWithOverrides() {
$keys = [1, 2, 3, 4];
$query = $this->getMock(
'Cake\ORM\Query',
['execute', 'where', 'andWhere', 'order', 'select', 'contain'],
['all', 'where', 'andWhere', 'order', 'select', 'contain'],
[null, null]
);
$this->article->expects($this->once())->method('find')->with('all')
Expand All @@ -221,7 +221,7 @@ public function testEagerLoaderWithOverrides() {
['id' => 2, 'title' => 'article 2', 'author_id' => 1]
];

$query->expects($this->once())->method('execute')
$query->expects($this->once())->method('all')
->will($this->returnValue($results));

$query->expects($this->at(0))->method('where')
Expand Down Expand Up @@ -281,7 +281,7 @@ public function testEagerLoaderFieldsException() {
$keys = [1, 2, 3, 4];
$query = $this->getMock(
'Cake\ORM\Query',
['execute'],
['all'],
[null, null]
);
$this->article->expects($this->once())->method('find')->with('all')
Expand Down Expand Up @@ -311,7 +311,7 @@ public function testEagerLoaderSubquery() {

$query = $this->getMock(
'Cake\ORM\Query',
['execute', 'where', 'andWhere', 'order', 'select', 'contain'],
['all', 'where', 'andWhere', 'order', 'select', 'contain'],
[null, null]
);

Expand All @@ -321,7 +321,7 @@ public function testEagerLoaderSubquery() {
['id' => 1, 'title' => 'article 1', 'author_id' => 2],
['id' => 2, 'title' => 'article 2', 'author_id' => 1]
];
$query->expects($this->once())->method('execute')
$query->expects($this->once())->method('all')
->will($this->returnValue($results));

$query->expects($this->at(0))->method('where')
Expand Down
18 changes: 9 additions & 9 deletions Cake/Test/TestCase/ORM/QueryTest.php
View file
Expand Up @@ -864,7 +864,7 @@ public function testSetResult() {
$stmt = $this->getMock('Cake\Database\StatementInterface');
$results = new ResultSet($query, $stmt);
$query->setResult($results);
$this->assertSame($results, $query->execute());
$this->assertSame($results, $query->all());
}

/**
Expand Down Expand Up @@ -1040,15 +1040,15 @@ public function testOverwriteMapReduce() {
*/
public function testResultsAreWrappedInMapReduce() {
$params = [$this->connection, $this->table];
$query = $this->getMock('\Cake\ORM\Query', ['_executeStatement'], $params);
$query = $this->getMock('\Cake\ORM\Query', ['execute'], $params);

$statement = $this->getMock('\Database\StatementInterface', ['fetch', 'closeCursor']);
$statement->expects($this->exactly(3))
->method('fetch')
->will($this->onConsecutiveCalls(['a' => 1], ['a' => 2], false));

$query->expects($this->once())
->method('_executeStatement')
->method('execute')
->will($this->returnValue($statement));

$query->mapReduce(function($k, $v, $mr) {
Expand All @@ -1063,7 +1063,7 @@ function($k, $v, $mr) {
}
);

$this->assertEquals([2, 3], iterator_to_array($query->execute()));
$this->assertEquals([2, 3], iterator_to_array($query->all()));
}

/**
Expand Down Expand Up @@ -1107,9 +1107,9 @@ public function testFirstSameResult() {
$query->select(['id'])->toArray();

$first = $query->hydrate(false)->first();
$resultSet = $query->execute();
$resultSet = $query->all();
$this->assertEquals(['id' => 1], $first);
$this->assertSame($resultSet, $query->execute());
$this->assertSame($resultSet, $query->all());
}

/**
Expand Down Expand Up @@ -1143,7 +1143,7 @@ public function testFirstMapReduce() {
public function testHydrateSimple() {
$table = TableRegistry::get('articles', ['table' => 'articles']);
$query = new Query($this->connection, $table);
$results = $query->select()->execute()->toArray();
$results = $query->select()->toArray();

$this->assertCount(3, $results);
foreach ($results as $r) {
Expand Down Expand Up @@ -1301,7 +1301,7 @@ public function testHydrateCustomObject() {
'entityClass' => '\\' . $class
]);
$query = new Query($this->connection, $table);
$results = $query->select()->execute()->toArray();
$results = $query->select()->toArray();

$this->assertCount(3, $results);
foreach ($results as $r) {
Expand Down Expand Up @@ -1395,7 +1395,7 @@ public function testCount() {
$result = $query->count();
$this->assertEquals(2, $result);

$result = $query->execute();
$result = $query->all();
$this->assertEquals(['count' => 2], $result->first());
}

Expand Down

0 comments on commit c7ef7f5

Please sign in to comment.