use IDBConnection and close cursors after insert/update/delete

diff --git a/lib/public/appframework/db/mapper.php b/lib/public/appframework/db/mapper.php
index 2022c7bfc88c02460e5e2b02dc70255c199c1022..4424ef3622b16d8a7b54e05e31b53a34f2a5cf79 100644 (file)
--- a/lib/public/appframework/db/mapper.php
+++ b/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;
        }

diff --git a/tests/lib/appframework/db/mappertest.php b/tests/lib/appframework/db/mappertest.php
index 9cbc01883e3e83511d81107669c69928ba8a66c1..8e585c479bb38ef56c46066b72ce6b72cd730d1e 100644 (file)
--- a/tests/lib/appframework/db/mappertest.php
+++ b/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);
        }

diff --git a/tests/lib/appframework/db/mappertestutility.php b/tests/lib/appframework/db/mappertestutility.php
index 0053b2c682d9e831ce5fe9efafcbbb6f987aa26b..e99a7617d9f53c6b09e6f606beb11aa9552b4713 100644 (file)
--- a/tests/lib/appframework/db/mappertestutility.php
+++ b/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++;
-
        }