use IDBConnection and close cursors after insert/update/delete

lib/public/appframework/db/mapper.php

@@ -70,7 +70,8 @@ abstract class Mapper {
 	 */
 	public function delete(Entity $entity){
 		$sql = 'DELETE FROM `' . $this->tableName . '` WHERE `id` = ?';
-		$this->execute($sql, [$entity->getId()]);
+		$stmt = $this->execute($sql, [$entity->getId()]);
+		$stmt->closeCursor();
 		return $entity;
 	}
 
@@ -103,7 +104,7 @@ abstract class Mapper {
 				$values .= ',';
 			}
 
-			array_push($params, $entity->$getter());
+			$params[] = $entity->$getter();
 			$i++;
 
 		}
@@ -111,10 +112,12 @@ abstract class Mapper {
 		$sql = 'INSERT INTO `' . $this->tableName . '`(' .
 				$columns . ') VALUES(' . $values . ')';
 
-		$this->execute($sql, $params);
+		$stmt = $this->execute($sql, $params);
 
 		$entity->setId((int) $this->db->lastInsertId($this->tableName));
 
+		$stmt->closeCursor();
+
 		return $entity;
 	}
 
@@ -162,15 +165,16 @@ abstract class Mapper {
 				$columns .= ',';
 			}
 
-			array_push($params, $entity->$getter());
+			$params[] = $entity->$getter();
 			$i++;
 		}
 
 		$sql = 'UPDATE `' . $this->tableName . '` SET ' .
 				$columns . ' WHERE `id` = ?';
-		array_push($params, $id);
+		$params[] = $id;
 
-		$this->execute($sql, $params);
+		$stmt = $this->execute($sql, $params);
+		$stmt->closeCursor();
 
 		return $entity;
 	}

tests/lib/appframework/db/mappertest.php

@@ -24,7 +24,7 @@
 
 namespace OCP\AppFramework\Db;
 
-use \OCP\IDb;
+use \OCP\IDBConnection;
 use Test\AppFramework\Db\MapperTestUtility;
 
 /**
@@ -42,7 +42,7 @@ class Example extends Entity {
 
 
 class ExampleMapper extends Mapper {
-	public function __construct(IDb $db){ parent::__construct($db, 'table'); }
+	public function __construct(IDBConnection $db){ parent::__construct($db, 'table'); }
 	public function find($table, $id){ return $this->findOneQuery($table, $id); }
 	public function findOneEntity($table, $id){ return $this->findEntity($table, $id); }
 	public function findAllEntities($table){ return $this->findEntities($table); }
@@ -137,7 +137,7 @@ class MapperTest extends MapperTestUtility {
 		$sql = 'DELETE FROM `*PREFIX*table` WHERE `id` = ?';
 		$params = array(2);
 
-		$this->setMapperResult($sql, $params);
+		$this->setMapperResult($sql, $params, [], null, null, true);
 		$entity = new Example();
 		$entity->setId($params[0]);
 
@@ -147,7 +147,7 @@ class MapperTest extends MapperTestUtility {
 
 	public function testCreate(){
 		$this->db->expects($this->once())
-			->method('getInsertId')
+			->method('lastInsertId')
 			->with($this->equalTo('*PREFIX*table'))
 			->will($this->returnValue(3));
 		$this->mapper = new ExampleMapper($this->db);
@@ -159,7 +159,7 @@ class MapperTest extends MapperTestUtility {
 		$entity->setPreName($params[0]);
 		$entity->setEmail($params[1]);
 
-		$this->setMapperResult($sql, $params);
+		$this->setMapperResult($sql, $params, [], null, null, true);
 
 		$this->mapper->insert($entity);
 	}
@@ -167,7 +167,7 @@ class MapperTest extends MapperTestUtility {
 
 	public function testCreateShouldReturnItemWithCorrectInsertId(){
 		$this->db->expects($this->once())
-			->method('getInsertId')
+			->method('lastInsertId')
 			->with($this->equalTo('*PREFIX*table'))
 			->will($this->returnValue(3));
 		$this->mapper = new ExampleMapper($this->db);
@@ -200,7 +200,7 @@ class MapperTest extends MapperTestUtility {
 		$entity->setEmail($params[1]);
 		$entity->setId($params[2]);
 
-		$this->setMapperResult($sql, $params);
+		$this->setMapperResult($sql, $params, [], null, null, true);
 
 		$this->mapper->update($entity);
 	}

tests/lib/appframework/db/mappertestutility.php

@@ -31,7 +31,6 @@ namespace Test\AppFramework\Db;
 abstract class MapperTestUtility extends \Test\TestCase {
 	protected $db;
 	private $query;
-	private $pdoResult;
 	private $queryAt;
 	private $prepareAt;
 	private $fetchAt;
@@ -46,15 +45,14 @@ abstract class MapperTestUtility extends \Test\TestCase {
 		parent::setUp();
 
 		$this->db = $this->getMockBuilder(
-			'\OCP\IDb')
+			'\OCP\IDBConnection')
 			->disableOriginalConstructor()
 			->getMock();
 
-		$this->query = $this->getMock('Query', array('execute', 'bindValue'));
-		$this->pdoResult = $this->getMock('Result', array('fetch', 'closeCursor'));
+		$this->query = $this->getMock('\PDOStatement');
 		$this->queryAt = 0;
 		$this->prepareAt = 0;
-		$this->iterators = array();
+		$this->iterators = [];
 		$this->fetchAt = 0;
 	}
 
@@ -70,13 +68,38 @@ abstract class MapperTestUtility extends \Test\TestCase {
 	 */
 	protected function setMapperResult($sql, $arguments=array(), $returnRows=array(),
 		$limit=null, $offset=null, $expectClose=false){
+		if($limit === null && $offset === null) {
+			$this->db->expects($this->at($this->prepareAt))
+				->method('prepare')
+				->with($this->equalTo($sql))
+				->will(($this->returnValue($this->query)));
+		} elseif($limit !== null && $offset === null) {
+			$this->db->expects($this->at($this->prepareAt))
+				->method('prepare')
+				->with($this->equalTo($sql), $this->equalTo($limit))
+				->will(($this->returnValue($this->query)));
+		} elseif($limit === null && $offset !== null) {
+			$this->db->expects($this->at($this->prepareAt))
+				->method('prepare')
+				->with($this->equalTo($sql),
+					$this->equalTo(null),
+					$this->equalTo($offset))
+				->will(($this->returnValue($this->query)));
+		} else  {
+			$this->db->expects($this->at($this->prepareAt))
+				->method('prepare')
+				->with($this->equalTo($sql),
+					$this->equalTo($limit),
+					$this->equalTo($offset))
+				->will(($this->returnValue($this->query)));
+		}
 
 		$this->iterators[] = new ArgumentIterator($returnRows);
 
 		$iterators = $this->iterators;
 		$fetchAt = $this->fetchAt;
 
-		$this->pdoResult->expects($this->any())
+		$this->query->expects($this->any())
 			->method('fetch')
 			->will($this->returnCallback(
 				function() use ($iterators, $fetchAt){
@@ -87,15 +110,11 @@ abstract class MapperTestUtility extends \Test\TestCase {
 						$fetchAt++;
 					}
 
+					$this->queryAt++;
+
 					return $result;
 				}
 			));
-		if ($expectClose) {
-			$closing = $this->once();
-		} else {
-			$closing = $this->any();
-		}
-		$this->pdoResult->expects($closing)->method('closeCursor');
 
 		$index = 1;
 		foreach($arguments as $argument) {
@@ -126,39 +145,21 @@ abstract class MapperTestUtility extends \Test\TestCase {
 		}
 
 		$this->query->expects($this->at($this->queryAt))
-			->method('execute')
-			->with()
-			->will($this->returnValue($this->pdoResult));
+			->method('execute');
 		$this->queryAt++;
 
-		if($limit === null && $offset === null) {
-			$this->db->expects($this->at($this->prepareAt))
-				->method('prepareQuery')
-				->with($this->equalTo($sql))
-				->will(($this->returnValue($this->query)));
-		} elseif($limit !== null && $offset === null) {
-			$this->db->expects($this->at($this->prepareAt))
-				->method('prepareQuery')
-				->with($this->equalTo($sql), $this->equalTo($limit))
-				->will(($this->returnValue($this->query)));
-		} elseif($limit === null && $offset !== null) {
-			$this->db->expects($this->at($this->prepareAt))
-				->method('prepareQuery')
-				->with($this->equalTo($sql),
-					$this->equalTo(null),
-					$this->equalTo($offset))
-				->will(($this->returnValue($this->query)));
-		} else  {
-			$this->db->expects($this->at($this->prepareAt))
-				->method('prepareQuery')
-				->with($this->equalTo($sql),
-					$this->equalTo($limit),
-					$this->equalTo($offset))
-				->will(($this->returnValue($this->query)));
+
+
+		if ($expectClose) {
+			$closing = $this->once();
+		} else {
+			$closing = $this->any();
 		}
+		$this->query->expects($closing)->method('closeCursor');
+		$this->queryAt++;
+
 		$this->prepareAt++;
 		$this->fetchAt++;
-
 	}