Merge pull request #13777 from owncloud/close-cursor
Close cursor for appframework and manipulation queries if applicable
This commit is contained in:
commit
fcc5f5a4f4
4 changed files with 51 additions and 37 deletions
|
@ -69,6 +69,7 @@ class OC_DB_StatementWrapper {
|
|||
return false;
|
||||
}
|
||||
if ($this->isManipulation) {
|
||||
$this->statement->closeCursor();
|
||||
return $this->statement->rowCount();
|
||||
} else {
|
||||
return $this;
|
||||
|
|
|
@ -41,7 +41,7 @@ abstract class Mapper {
|
|||
|
||||
/**
|
||||
* @param IDb $db Instance of the Db abstraction layer
|
||||
* @param string $tableName the name of the table. set this to allow entity
|
||||
* @param string $tableName the name of the table. set this to allow entity
|
||||
* @param string $entityClass the name of the entity that the sql should be
|
||||
* mapped to queries without using sql
|
||||
*/
|
||||
|
@ -70,10 +70,12 @@ abstract class Mapper {
|
|||
/**
|
||||
* Deletes an entity from the table
|
||||
* @param Entity $entity the entity that should be deleted
|
||||
* @return Entity the deleted entity
|
||||
*/
|
||||
public function delete(Entity $entity){
|
||||
$sql = 'DELETE FROM `' . $this->tableName . '` WHERE `id` = ?';
|
||||
$this->execute($sql, array($entity->getId()));
|
||||
$this->execute($sql, [$entity->getId()]);
|
||||
return $entity;
|
||||
}
|
||||
|
||||
|
||||
|
@ -88,14 +90,14 @@ abstract class Mapper {
|
|||
$properties = $entity->getUpdatedFields();
|
||||
$values = '';
|
||||
$columns = '';
|
||||
$params = array();
|
||||
$params = [];
|
||||
|
||||
// build the fields
|
||||
$i = 0;
|
||||
foreach($properties as $property => $updated) {
|
||||
$column = $entity->propertyToColumn($property);
|
||||
$getter = 'get' . ucfirst($property);
|
||||
|
||||
|
||||
$columns .= '`' . $column . '`';
|
||||
$values .= '?';
|
||||
|
||||
|
@ -112,10 +114,11 @@ abstract class Mapper {
|
|||
|
||||
$sql = 'INSERT INTO `' . $this->tableName . '`(' .
|
||||
$columns . ') VALUES(' . $values . ')';
|
||||
|
||||
|
||||
$this->execute($sql, $params);
|
||||
|
||||
$entity->setId((int) $this->db->getInsertId($this->tableName));
|
||||
|
||||
return $entity;
|
||||
}
|
||||
|
||||
|
@ -147,7 +150,7 @@ abstract class Mapper {
|
|||
unset($properties['id']);
|
||||
|
||||
$columns = '';
|
||||
$params = array();
|
||||
$params = [];
|
||||
|
||||
// build the fields
|
||||
$i = 0;
|
||||
|
@ -155,7 +158,7 @@ abstract class Mapper {
|
|||
|
||||
$column = $entity->propertyToColumn($property);
|
||||
$getter = 'get' . ucfirst($property);
|
||||
|
||||
|
||||
$columns .= '`' . $column . '` = ?';
|
||||
|
||||
// only append colon if there are more entries
|
||||
|
@ -167,7 +170,7 @@ abstract class Mapper {
|
|||
$i++;
|
||||
}
|
||||
|
||||
$sql = 'UPDATE `' . $this->tableName . '` SET ' .
|
||||
$sql = 'UPDATE `' . $this->tableName . '` SET ' .
|
||||
$columns . ' WHERE `id` = ?';
|
||||
array_push($params, $id);
|
||||
|
||||
|
@ -185,7 +188,7 @@ abstract class Mapper {
|
|||
* @param int $offset from which row we want to start
|
||||
* @return \PDOStatement the database query result
|
||||
*/
|
||||
protected function execute($sql, array $params=array(), $limit=null, $offset=null){
|
||||
protected function execute($sql, array $params=[], $limit=null, $offset=null){
|
||||
$query = $this->db->prepareQuery($sql, $limit, $offset);
|
||||
|
||||
$index = 1; // bindParam is 1 indexed
|
||||
|
@ -199,12 +202,12 @@ abstract class Mapper {
|
|||
case 'boolean':
|
||||
$pdoConstant = \PDO::PARAM_BOOL;
|
||||
break;
|
||||
|
||||
|
||||
default:
|
||||
$pdoConstant = \PDO::PARAM_STR;
|
||||
break;
|
||||
}
|
||||
|
||||
|
||||
$query->bindValue($index, $param, $pdoConstant);
|
||||
|
||||
$index++;
|
||||
|
@ -226,14 +229,16 @@ abstract class Mapper {
|
|||
* @throws MultipleObjectsReturnedException if more than one item exist
|
||||
* @return array the result as row
|
||||
*/
|
||||
protected function findOneQuery($sql, array $params=array(), $limit=null, $offset=null){
|
||||
$result = $this->execute($sql, $params, $limit, $offset);
|
||||
$row = $result->fetch();
|
||||
protected function findOneQuery($sql, array $params=[], $limit=null, $offset=null){
|
||||
$stmt = $this->execute($sql, $params, $limit, $offset);
|
||||
$row = $stmt->fetch();
|
||||
|
||||
if($row === false || $row === null){
|
||||
$stmt->closeCursor();
|
||||
throw new DoesNotExistException('No matching entry found');
|
||||
}
|
||||
$row2 = $result->fetch();
|
||||
$row2 = $stmt->fetch();
|
||||
$stmt->closeCursor();
|
||||
//MDB2 returns null, PDO and doctrine false when no row is available
|
||||
if( ! ($row2 === false || $row2 === null )) {
|
||||
throw new MultipleObjectsReturnedException('More than one result');
|
||||
|
@ -262,15 +267,17 @@ abstract class Mapper {
|
|||
* @param int $offset from which row we want to start
|
||||
* @return array all fetched entities
|
||||
*/
|
||||
protected function findEntities($sql, array $params=array(), $limit=null, $offset=null) {
|
||||
$result = $this->execute($sql, $params, $limit, $offset);
|
||||
protected function findEntities($sql, array $params=[], $limit=null, $offset=null) {
|
||||
$stmt = $this->execute($sql, $params, $limit, $offset);
|
||||
|
||||
$entities = array();
|
||||
|
||||
while($row = $result->fetch()){
|
||||
$entities = [];
|
||||
|
||||
while($row = $stmt->fetch()){
|
||||
$entities[] = $this->mapRowToEntity($row);
|
||||
}
|
||||
|
||||
$stmt->closeCursor();
|
||||
|
||||
return $entities;
|
||||
}
|
||||
|
||||
|
@ -286,7 +293,7 @@ abstract class Mapper {
|
|||
* @throws MultipleObjectsReturnedException if more than one item exist
|
||||
* @return Entity the entity
|
||||
*/
|
||||
protected function findEntity($sql, array $params=array(), $limit=null, $offset=null){
|
||||
protected function findEntity($sql, array $params=[], $limit=null, $offset=null){
|
||||
return $this->mapRowToEntity($this->findOneQuery($sql, $params, $limit, $offset));
|
||||
}
|
||||
|
||||
|
|
|
@ -74,7 +74,7 @@ class MapperTest extends MapperTestUtility {
|
|||
$rows = array(
|
||||
array('hi')
|
||||
);
|
||||
$this->setMapperResult($sql, $params, $rows);
|
||||
$this->setMapperResult($sql, $params, $rows);
|
||||
$this->mapper->find($sql, $params);
|
||||
}
|
||||
|
||||
|
@ -84,7 +84,7 @@ class MapperTest extends MapperTestUtility {
|
|||
$rows = array(
|
||||
array('pre_name' => 'hi')
|
||||
);
|
||||
$this->setMapperResult($sql, $params, $rows);
|
||||
$this->setMapperResult($sql, $params, $rows, null, null, true);
|
||||
$this->mapper->findOneEntity($sql, $params);
|
||||
}
|
||||
|
||||
|
@ -92,7 +92,7 @@ class MapperTest extends MapperTestUtility {
|
|||
$sql = 'hi';
|
||||
$params = array('jo');
|
||||
$rows = array();
|
||||
$this->setMapperResult($sql, $params, $rows);
|
||||
$this->setMapperResult($sql, $params, $rows);
|
||||
$this->setExpectedException(
|
||||
'\OCP\AppFramework\Db\DoesNotExistException');
|
||||
$this->mapper->find($sql, $params);
|
||||
|
@ -102,7 +102,7 @@ class MapperTest extends MapperTestUtility {
|
|||
$sql = 'hi';
|
||||
$params = array('jo');
|
||||
$rows = array();
|
||||
$this->setMapperResult($sql, $params, $rows);
|
||||
$this->setMapperResult($sql, $params, $rows, null, null, true);
|
||||
$this->setExpectedException(
|
||||
'\OCP\AppFramework\Db\DoesNotExistException');
|
||||
$this->mapper->findOneEntity($sql, $params);
|
||||
|
@ -114,7 +114,7 @@ class MapperTest extends MapperTestUtility {
|
|||
$rows = array(
|
||||
array('jo'), array('ho')
|
||||
);
|
||||
$this->setMapperResult($sql, $params, $rows);
|
||||
$this->setMapperResult($sql, $params, $rows, null, null, true);
|
||||
$this->setExpectedException(
|
||||
'\OCP\AppFramework\Db\MultipleObjectsReturnedException');
|
||||
$this->mapper->find($sql, $params);
|
||||
|
@ -126,7 +126,7 @@ class MapperTest extends MapperTestUtility {
|
|||
$rows = array(
|
||||
array('jo'), array('ho')
|
||||
);
|
||||
$this->setMapperResult($sql, $params, $rows);
|
||||
$this->setMapperResult($sql, $params, $rows, null, null, true);
|
||||
$this->setExpectedException(
|
||||
'\OCP\AppFramework\Db\MultipleObjectsReturnedException');
|
||||
$this->mapper->findOneEntity($sql, $params);
|
||||
|
@ -249,7 +249,7 @@ class MapperTest extends MapperTestUtility {
|
|||
$entity = new Example();
|
||||
$entity->setPreName('hi');
|
||||
$entity->resetUpdatedFields();
|
||||
$this->setMapperResult($sql, array(), $rows);
|
||||
$this->setMapperResult($sql, array(), $rows, null, null, true);
|
||||
$result = $this->mapper->findAllEntities($sql);
|
||||
$this->assertEquals(array($entity), $result);
|
||||
}
|
||||
|
|
|
@ -36,7 +36,7 @@ abstract class MapperTestUtility extends \Test\TestCase {
|
|||
private $prepareAt;
|
||||
private $fetchAt;
|
||||
private $iterators;
|
||||
|
||||
|
||||
|
||||
/**
|
||||
* Run this function before the actual test to either set or initialize the
|
||||
|
@ -51,7 +51,7 @@ abstract class MapperTestUtility extends \Test\TestCase {
|
|||
->getMock();
|
||||
|
||||
$this->query = $this->getMock('Query', array('execute', 'bindValue'));
|
||||
$this->pdoResult = $this->getMock('Result', array('fetch'));
|
||||
$this->pdoResult = $this->getMock('Result', array('fetch', 'closeCursor'));
|
||||
$this->queryAt = 0;
|
||||
$this->prepareAt = 0;
|
||||
$this->iterators = array();
|
||||
|
@ -69,7 +69,7 @@ abstract class MapperTestUtility extends \Test\TestCase {
|
|||
* will be called on the result
|
||||
*/
|
||||
protected function setMapperResult($sql, $arguments=array(), $returnRows=array(),
|
||||
$limit=null, $offset=null){
|
||||
$limit=null, $offset=null, $expectClose=false){
|
||||
|
||||
$this->iterators[] = new ArgumentIterator($returnRows);
|
||||
|
||||
|
@ -88,8 +88,14 @@ abstract class MapperTestUtility extends \Test\TestCase {
|
|||
}
|
||||
|
||||
return $result;
|
||||
}
|
||||
}
|
||||
));
|
||||
if ($expectClose) {
|
||||
$closing = $this->once();
|
||||
} else {
|
||||
$closing = $this->any();
|
||||
}
|
||||
$this->pdoResult->expects($closing)->method('closeCursor');
|
||||
|
||||
$index = 1;
|
||||
foreach($arguments as $argument) {
|
||||
|
@ -105,7 +111,7 @@ abstract class MapperTestUtility extends \Test\TestCase {
|
|||
case 'boolean':
|
||||
$pdoConstant = \PDO::PARAM_BOOL;
|
||||
break;
|
||||
|
||||
|
||||
default:
|
||||
$pdoConstant = \PDO::PARAM_STR;
|
||||
break;
|
||||
|
@ -138,14 +144,14 @@ abstract class MapperTestUtility extends \Test\TestCase {
|
|||
} elseif($limit === null && $offset !== null) {
|
||||
$this->db->expects($this->at($this->prepareAt))
|
||||
->method('prepareQuery')
|
||||
->with($this->equalTo($sql),
|
||||
->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),
|
||||
->with($this->equalTo($sql),
|
||||
$this->equalTo($limit),
|
||||
$this->equalTo($offset))
|
||||
->will(($this->returnValue($this->query)));
|
||||
|
@ -162,11 +168,11 @@ abstract class MapperTestUtility extends \Test\TestCase {
|
|||
class ArgumentIterator {
|
||||
|
||||
private $arguments;
|
||||
|
||||
|
||||
public function __construct($arguments){
|
||||
$this->arguments = $arguments;
|
||||
}
|
||||
|
||||
|
||||
public function next(){
|
||||
$result = array_shift($this->arguments);
|
||||
if($result === null){
|
||||
|
|
Loading…
Reference in a new issue