diff --git a/lib/public/appframework/db/mapper.php b/lib/public/appframework/db/mapper.php index 2022c7bfc8..4424ef3622 100644 --- 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 9cbc01883e..8e585c479b 100644 --- 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 0053b2c682..e99a7617d9 100644 --- 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++; - }