Merge pull request #2551 from markstory/3.0-query-fixes

Fix issues in ORM\Query

Cake/Database/Query.php

@@ -196,9 +196,21 @@ public function connection($connection = null) {
  * Resulting statement is traversable, so it can be used in any loop as you would
  * with an array.
  *
+ * This method can be overridden in query subclasses to decorate behavior
+ * around query execution.
+ *
  * @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);
@@ -546,7 +558,9 @@ protected function _buildFromPart($parts, $generator) {
  * {{{
  *	$query->join([
  *		'a' => [
- *			'table' => 'authors', 'type' => 'LEFT', 'conditions' => 'a.id = b.author_id'
+ *			'table' => 'authors',
+ *			'type' => 'LEFT',
+ *			'conditions' => 'a.id = b.author_id'
  *		]
  *	]);
  *  // Produces LEFT JOIN authors a ON (a.id = b.author_id)
@@ -557,10 +571,14 @@ protected function _buildFromPart($parts, $generator) {
  * {{{
  *	$query->join([
  *		'a' => [
- *			'table' => 'authors', 'type' => 'LEFT', 'conditions' => 'a.id = b.author_id'
+ *			'table' => 'authors',
+ *			'type' => 'LEFT',
+ *			'conditions' => 'a.id = b.author_id'
  *		],
  *		'p' => [
- *			'table' => 'products', 'type' => 'INNER', 'conditions' => 'a.owner_id = p.id
+ *			'table' => 'products',
+ *			'type' => 'INNER',
+ *			'conditions' => 'a.owner_id = p.id
  *		]
  *	]);
  *	// LEFT JOIN authors a ON (a.id = b.author_id)

Cake/ORM/Query.php

@@ -394,36 +394,46 @@ public function setResult($results) {
  * Compiles the SQL representation of this query and executes it using the
  * provided connection object. Returns a ResultSet iterator object.
  *
+ * This method fires the `Model.beforeFind` event and processes the
+ * callback results.
+ *
  * If a result set was set using setResult() that ResultSet will be returned.
  *
  * Resulting object is traversable, so it can be used in any loop as you would
  * with an array.
  *
- * @return Cake\ORM\ResultCollectionTrait
+ * @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.
  */
 	public function execute() {
-		$table = $this->repository();
-		$event = new Event('Model.beforeFind', $table, [$this, $this->_options]);
-		$table->getEventManager()->dispatch($event);
+		if ($this->_type === 'select' || $this->_type === null) {
+			$table = $this->repository();
+			$event = new Event('Model.beforeFind', $table, [$this, $this->_options]);
+			$table->getEventManager()->dispatch($event);
+			return $this->getResults();
+		}
+		return $this->_executeStatement();
+	}
+
+/**
+ * Get the result set for this query.
+ *
+ * Will return either the results set through setResult(), or execute the underlying statement
+ * and return the ResultSet object ready for streaming of results.
+ *
+ * @return Cake\ORM\ResultCollectionTrait
+ */
+	public function getResults() {
 		if (isset($this->_results)) {
 			return $this->_results;
 		}
 		if ($this->_useBufferedResults) {
 			return $this->_results = $this->_decorateResults(
-				new BufferedResultSet($this, $this->executeStatement())
+				new BufferedResultSet($this, $this->_executeStatement())
 			);
 		}
-		return $this->_decorateResults(new ResultSet($this, $this->executeStatement()));
-	}
-
-/**
- * Compiles the SQL representation of this query and executes it using
- * the provided connection object.
- *
- * @return Cake\Database\StatementInterface
- */
-	public function executeStatement() {
-		return parent::execute();
+		return $this->_decorateResults(new ResultSet($this, $this->_executeStatement()));
 	}
 
 /**

Cake/ORM/Table.php

@@ -867,7 +867,7 @@ public function updateAll($fields, $conditions) {
 		$query->update($this->table())
 			->set($fields)
 			->where($conditions);
-		$statement = $query->executeStatement();
+		$statement = $query->execute();
 		$success = $statement->rowCount() > 0;
 		$statement->closeCursor();
 		return $success;
@@ -959,7 +959,7 @@ public function deleteAll($conditions) {
 		$query = $this->query();
 		$query->delete($this->table())
 			->where($conditions);
-		$statement = $query->executeStatement();
+		$statement = $query->execute();
 		$success = $statement->rowCount() > 0;
 		$statement->closeCursor();
 		return $success;
@@ -1190,7 +1190,7 @@ protected function _insert($entity, $data) {
 
 		$statement = $query->insert($this->table(), array_keys($data))
 			->values($data)
-			->executeStatement();
+			->execute();
 
 		$success = false;
 		if ($statement->rowCount() > 0) {
@@ -1251,7 +1251,7 @@ protected function _update($entity, $data) {
 		$statement = $query->update($this->table())
 			->set($data)
 			->where($primaryKey)
-			->executeStatement();
+			->execute();
 
 		$success = false;
 		if ($statement->rowCount() > 0) {
@@ -1340,7 +1340,7 @@ protected function _processDelete($entity, $options) {
 		$query = $this->query();
 		$statement = $query->delete($this->table())
 			->where($conditions)
-			->executeStatement();
+			->execute();
 
 		$success = $statement->rowCount() > 0;
 		if (!$success) {

Cake/Test/TestCase/ORM/QueryTest.php

@@ -860,6 +860,7 @@ public function testFilteringByBelongsToManyNoHydration() {
  */
 	public function testSetResult() {
 		$query = new Query($this->connection, $this->table);
+
 		$stmt = $this->getMock('Cake\Database\StatementInterface');
 		$results = new ResultSet($query, $stmt);
 		$query->setResult($results);
@@ -1055,15 +1056,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', ['_executeStatement'], $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('_executeStatement')
 			->will($this->returnValue($statement));
 
 		$query->mapReduce(function($k, $v, $mr) {
@@ -1416,6 +1417,8 @@ public function testCount() {
 
 /**
  * Test that count() returns correct results with group by.
+ *
+ * @return void
  */
 	public function testCountWithGroup() {
 		$table = TableRegistry::get('articles');
@@ -1425,4 +1428,43 @@ public function testCountWithGroup() {
 		$this->assertEquals(1, $result);
 	}
 
+/**
+ * Test that there is no beforeFind event with update queries.
+ *
+ * @return void
+ */
+	public function testUpdateNoBeforeFind() {
+		$table = TableRegistry::get('articles');
+		$table->getEventManager()->attach(function() {
+			$this->fail('No callback should be fired');
+		}, 'Model.beforeFind');
+
+		$query = $table->query()
+			->update($table->table())
+			->set(['title' => 'First']);
+
+		$result = $query->execute();
+		$this->assertInstanceOf('Cake\Database\StatementInterface', $result);
+		$this->assertTrue($result->rowCount() > 0);
+	}
+
+/**
+ * Test that there is no beforeFind event with delete queries.
+ *
+ * @return void
+ */
+	public function testDeleteNoBeforeFind() {
+		$table = TableRegistry::get('articles');
+		$table->getEventManager()->attach(function() {
+			$this->fail('No callback should be fired');
+		}, 'Model.beforeFind');
+
+		$query = $table->query()
+			->delete($table->table());
+
+		$result = $query->execute();
+		$this->assertInstanceOf('Cake\Database\StatementInterface', $result);
+		$this->assertTrue($result->rowCount() > 0);
+	}
+
 }

Cake/Test/TestCase/ORM/TableTest.php

@@ -491,12 +491,12 @@ public function testUpdateAllFailure() {
 			['query'],
 			[['table' => 'users']]
 		);
-		$query = $this->getMock('Cake\ORM\Query', ['executeStatement'], [$this->connection, null]);
+		$query = $this->getMock('Cake\ORM\Query', ['_executeStatement'], [$this->connection, null]);
 		$table->expects($this->once())
 			->method('query')
 			->will($this->returnValue($query));
 		$query->expects($this->once())
-			->method('executeStatement')
+			->method('_executeStatement')
 			->will($this->throwException(new \Cake\Database\Exception('Not good')));
 		$table->updateAll(['username' => 'mark'], []);
 	}
@@ -530,12 +530,12 @@ public function testDeleteAllFailure() {
 			['query'],
 			[['table' => 'users', 'connection' => $this->connection]]
 		);
-		$query = $this->getMock('Cake\ORM\Query', ['executeStatement'], [$this->connection, null]);
+		$query = $this->getMock('Cake\ORM\Query', ['_executeStatement'], [$this->connection, null]);
 		$table->expects($this->once())
 			->method('query')
 			->will($this->returnValue($query));
 		$query->expects($this->once())
-			->method('executeStatement')
+			->method('_executeStatement')
 			->will($this->throwException(new \Cake\Database\Exception('Not good')));
 		$table->deleteAll(['id >' => 4]);
 	}
@@ -1230,7 +1230,7 @@ public function testAfterSaveNotCalled() {
 		);
 		$query = $this->getMock(
 			'\Cake\ORM\Query',
-			['executeStatement', 'addDefaultTypes'],
+			['_executeStatement', 'addDefaultTypes'],
 			[null, $table]
 		);
 		$statement = $this->getMock('\Cake\Database\Statement\StatementDecorator');
@@ -1245,7 +1245,7 @@ public function testAfterSaveNotCalled() {
 		$table->expects($this->once())->method('query')
 			->will($this->returnValue($query));
 
-		$query->expects($this->once())->method('executeStatement')
+		$query->expects($this->once())->method('_executeStatement')
 			->will($this->returnValue($statement));
 
 		$statement->expects($this->once())->method('rowCount')
@@ -1311,7 +1311,7 @@ public function testAtomicSaveRollback() {
 		);
 		$query = $this->getMock(
 			'\Cake\ORM\Query',
-			['executeStatement', 'addDefaultTypes'],
+			['_executeStatement', 'addDefaultTypes'],
 			[null, $table]
 		);
 
@@ -1326,7 +1326,7 @@ public function testAtomicSaveRollback() {
 
 		$connection->expects($this->once())->method('begin');
 		$connection->expects($this->once())->method('rollback');
-		$query->expects($this->once())->method('executeStatement')
+		$query->expects($this->once())->method('_executeStatement')
 			->will($this->throwException(new \PDOException));
 
 		$data = new \Cake\ORM\Entity([
@@ -1357,7 +1357,7 @@ public function testAtomicSaveRollbackOnFailure() {
 		);
 		$query = $this->getMock(
 			'\Cake\ORM\Query',
-			['executeStatement', 'addDefaultTypes'],
+			['_executeStatement', 'addDefaultTypes'],
 			[null, $table]
 		);
 
@@ -1375,7 +1375,7 @@ public function testAtomicSaveRollbackOnFailure() {
 			->will($this->returnValue(0));
 		$connection->expects($this->once())->method('begin');
 		$connection->expects($this->once())->method('rollback');
-		$query->expects($this->once())->method('executeStatement')
+		$query->expects($this->once())->method('_executeStatement')
 			->will($this->returnValue($statement));
 
 		$data = new \Cake\ORM\Entity([
@@ -1538,7 +1538,7 @@ public function testSaveUpdatePrimaryKeyNotModified() {
 
 		$query = $this->getMock(
 			'\Cake\ORM\Query',
-			['executeStatement', 'addDefaultTypes', 'set'],
+			['_executeStatement', 'addDefaultTypes', 'set'],
 			[null, $table]
 		);
 
@@ -1549,7 +1549,7 @@ public function testSaveUpdatePrimaryKeyNotModified() {
 		$statement->expects($this->once())->method('rowCount')
 			->will($this->returnValue(1));
 
-		$query->expects($this->once())->method('executeStatement')
+		$query->expects($this->once())->method('_executeStatement')
 			->will($this->returnValue($statement));
 
 		$query->expects($this->once())->method('set')