Skip to content
This repository was archived by the owner on Jan 29, 2020. It is now read-only.

Fixed invalid quoting for postgresql adapter #162

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/TableGateway/Feature/SequenceFeature.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class SequenceFeature extends AbstractFeature
protected $primaryKeyField;

/**
* @var string
* @var string|array
*/
protected $sequenceName;

Expand Down Expand Up @@ -89,7 +89,7 @@ public function nextSequenceId()
$sql = 'SELECT ' . $platform->quoteIdentifier($this->sequenceName) . '.NEXTVAL as "nextval" FROM dual';
break;
case 'PostgreSQL':
$sql = 'SELECT NEXTVAL(\'"' . $this->sequenceName . '"\')';
$sql = 'SELECT NEXTVAL(\'' . $platform->quoteIdentifierChain($this->sequenceName) . '\')';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to:

$sql = 'SELECT NEXTVAL(' . $platform->quoteValue($platform->quoteIdentifierChain($this->sequenceName)) . ')';

break;
default :
return;
Expand Down Expand Up @@ -117,7 +117,7 @@ public function lastSequenceId()
$sql = 'SELECT ' . $platform->quoteIdentifier($this->sequenceName) . '.CURRVAL as "currval" FROM dual';
break;
case 'PostgreSQL':
$sql = 'SELECT CURRVAL(\'' . $this->sequenceName . '\')';
$sql = 'SELECT CURRVAL(\'' . $platform->quoteIdentifierChain($this->sequenceName) . '\')';
Copy link

@andrey-mokhov andrey-mokhov Sep 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to:

$sql = 'SELECT CURRVAL(' . $platform->quoteValue($platform->quoteIdentifierChain($this->sequenceName)) . ')';

break;
default :
return;
Expand Down
11 changes: 5 additions & 6 deletions test/TableGateway/Feature/FeatureSetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Zend\Db\TableGateway\Feature\SequenceFeature;
use Zend\Db\TableGateway\Feature\MetadataFeature;
use Zend\Db\Metadata\Object\ConstraintObject;
use ZendTest\Db\TestAsset\TrustingPostgresqlPlatform;

class FeatureSetTest extends \PHPUnit_Framework_TestCase
{
Expand Down Expand Up @@ -126,11 +127,9 @@ public function testCanCallMagicCallReturnsFalseWhenNoFeaturesHaveBeenAdded()
*/
public function testCallMagicCallSucceedsForValidMethodOfAddedFeature()
{
$sequenceName = 'table_sequence';
$sequenceName = '"schema"."table_sequence"';

$platformMock = $this->getMock('Zend\Db\Adapter\Platform\Postgresql');
$platformMock->expects($this->any())
->method('getName')->will($this->returnValue('PostgreSQL'));
$platformStub = new TrustingPostgresqlPlatform();

$resultMock = $this->getMock('Zend\Db\Adapter\Driver\Pgsql\Result');
$resultMock->expects($this->any())
Expand All @@ -149,7 +148,7 @@ public function testCallMagicCallSucceedsForValidMethodOfAddedFeature()
->disableOriginalConstructor()
->getMock();
$adapterMock->expects($this->any())
->method('getPlatform')->will($this->returnValue($platformMock));
->method('getPlatform')->will($this->returnValue($platformStub));
$adapterMock->expects($this->any())
->method('createStatement')->will($this->returnValue($statementMock));

Expand All @@ -162,7 +161,7 @@ public function testCallMagicCallSucceedsForValidMethodOfAddedFeature()
$reflectionProperty->setAccessible(true);
$reflectionProperty->setValue($tableGatewayMock, $adapterMock);

$feature = new SequenceFeature('id', 'table_sequence');
$feature = new SequenceFeature('id', ['schema', 'table_sequence']);
$feature->setTableGateway($tableGatewayMock);
$featureSet = new FeatureSet;
$featureSet->addFeature($feature);
Expand Down
56 changes: 33 additions & 23 deletions test/TableGateway/Feature/SequenceFeatureTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,39 +10,26 @@
namespace ZendTest\Db\TableGateway\Feature;

use PHPUnit_Framework_TestCase;
use Zend\Db\Adapter\Platform\PlatformInterface;
use ZendTest\Db\TestAsset;
use Zend\Db\TableGateway\Feature\SequenceFeature;

class SequenceFeatureTest extends PHPUnit_Framework_TestCase
{
/** @var SequenceFeature */
protected $feature = null;

/** @var \Zend\Db\TableGateway\TableGateway */
protected $tableGateway = null;

/** @var string primary key name */
protected $primaryKeyField = 'id';

/** @var string sequence name */
protected $sequenceName = 'table_sequence';

public function setup()
{
$this->feature = new SequenceFeature($this->primaryKeyField, $this->sequenceName);
}

/**
* @dataProvider nextSequenceIdProvider
*/
public function testNextSequenceId($platformName, $statementSql)
public function testNextSequenceId($platformName, $sequenceName, $statementSql)
{
$platform = $this->getMockForAbstractClass('Zend\Db\Adapter\Platform\PlatformInterface', ['getName']);
$platform->expects($this->any())
->method('getName')
->will($this->returnValue($platformName));
$platform->expects($this->any())
->method('quoteIdentifier')
->will($this->returnValue($this->sequenceName));
$feature = new SequenceFeature($this->primaryKeyField, $sequenceName);

$platform = $this->getPlatformStub($platformName);
$adapter = $this->getMock('Zend\Db\Adapter\Adapter', ['getPlatform', 'createStatement'], [], '', false);
$adapter->expects($this->any())
->method('getPlatform')
Expand All @@ -62,13 +49,36 @@ public function testNextSequenceId($platformName, $statementSql)
->method('createStatement')
->will($this->returnValue($statement));
$this->tableGateway = $this->getMockForAbstractClass('Zend\Db\TableGateway\TableGateway', ['table', $adapter], '', true);
$this->feature->setTableGateway($this->tableGateway);
$this->feature->nextSequenceId();
$feature->setTableGateway($this->tableGateway);
$feature->nextSequenceId();
}

public function nextSequenceIdProvider()
{
return [['PostgreSQL', 'SELECT NEXTVAL(\'"' . $this->sequenceName . '"\')'],
['Oracle', 'SELECT ' . $this->sequenceName . '.NEXTVAL as "nextval" FROM dual']];
return [
//@TODO MS SQL SERVER 2016 now supports sequences too
['PostgreSQL', 'table_sequence', 'SELECT NEXTVAL(\'"table_sequence"\')'],
['PostgreSQL', ['schema','table_sequence'], 'SELECT NEXTVAL(\'"schema"."table_sequence"\')'],
['Oracle', 'table_sequence', 'SELECT "table_sequence".NEXTVAL as "nextval" FROM dual']
];
}

/**
* Data provider
* @TODO this method is replicated in a several tests. Seems common enough to put in common utility, trait or abstract test class
*
* @param string $platform
*
* @return PlatformInterface
*/
protected function getPlatformStub($platform)
{
switch ($platform) {
case 'Oracle' : $platform = new TestAsset\TrustingOraclePlatform(); break;
case 'PostgreSQL' : $platform = new TestAsset\TrustingPostgresqlPlatform(); break;
default : $platform = null;
}

return $platform;
}
}
20 changes: 20 additions & 0 deletions test/TestAsset/TrustingPostgresqlPlatform.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php
/**
* Zend Framework (http://framework.zend.com/)
*
* @link http://github.com/zendframework/zf2 for the canonical source repository
* @copyright Copyright (c) 2005-2016 Zend Technologies USA Inc. (http://www.zend.com)
* @license http://framework.zend.com/license/new-bsd New BSD License
*/

namespace ZendTest\Db\TestAsset;

use Zend\Db\Adapter\Platform\Postgresql;

class TrustingPostgresqlPlatform extends Postgresql
{
public function quoteValue($value)
{
return $this->quoteTrustedValue($value);
}
}