From b180c3101f888b4f6a181b8f1e256977c1bbeed4 Mon Sep 17 00:00:00 2001 From: xorock Date: Tue, 6 Sep 2016 16:42:44 +0200 Subject: [PATCH 1/9] Allow array notation in sequence name for PostgreSQL adapter --- src/TableGateway/Feature/SequenceFeature.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php index 3c72fd3b6f..5e064b1946 100644 --- a/src/TableGateway/Feature/SequenceFeature.php +++ b/src/TableGateway/Feature/SequenceFeature.php @@ -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) . '\')'; break; default : return; @@ -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) . '\')'; break; default : return; From 050a3549bc8654b5c6348642f6f7fab2a84228c6 Mon Sep 17 00:00:00 2001 From: xorock Date: Tue, 6 Sep 2016 16:43:12 +0200 Subject: [PATCH 2/9] Update SequenceFeature.php --- src/TableGateway/Feature/SequenceFeature.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php index 5e064b1946..bb940bf6f8 100644 --- a/src/TableGateway/Feature/SequenceFeature.php +++ b/src/TableGateway/Feature/SequenceFeature.php @@ -21,7 +21,7 @@ class SequenceFeature extends AbstractFeature protected $primaryKeyField; /** - * @var string + * @var string|array */ protected $sequenceName; From 437aa05bd56abb4cd7ae1984091d40f2f58062e3 Mon Sep 17 00:00:00 2001 From: xorock Date: Tue, 6 Sep 2016 17:03:40 +0200 Subject: [PATCH 3/9] Update SequenceFeatureTest.php --- test/TableGateway/Feature/SequenceFeatureTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/TableGateway/Feature/SequenceFeatureTest.php b/test/TableGateway/Feature/SequenceFeatureTest.php index b3edc59c33..4ee99f0fbc 100644 --- a/test/TableGateway/Feature/SequenceFeatureTest.php +++ b/test/TableGateway/Feature/SequenceFeatureTest.php @@ -68,7 +68,7 @@ public function testNextSequenceId($platformName, $statementSql) public function nextSequenceIdProvider() { - return [['PostgreSQL', 'SELECT NEXTVAL(\'"' . $this->sequenceName . '"\')'], + return [['PostgreSQL', 'SELECT NEXTVAL(\'' . $this->sequenceName . '\')'], ['Oracle', 'SELECT ' . $this->sequenceName . '.NEXTVAL as "nextval" FROM dual']]; } } From 583772471caa4c348af17f271b44d239897a315b Mon Sep 17 00:00:00 2001 From: xorock Date: Tue, 6 Sep 2016 17:17:12 +0200 Subject: [PATCH 4/9] Update README.md From 06c773a0a54593b87daa00e1be47b12a6f3303d8 Mon Sep 17 00:00:00 2001 From: xorock Date: Tue, 6 Sep 2016 17:41:18 +0200 Subject: [PATCH 5/9] Update FeatureSetTest.php --- test/TableGateway/Feature/FeatureSetTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/TableGateway/Feature/FeatureSetTest.php b/test/TableGateway/Feature/FeatureSetTest.php index bfc6271c17..90098978db 100644 --- a/test/TableGateway/Feature/FeatureSetTest.php +++ b/test/TableGateway/Feature/FeatureSetTest.php @@ -140,7 +140,7 @@ public function testCallMagicCallSucceedsForValidMethodOfAddedFeature() $statementMock = $this->getMock('Zend\Db\Adapter\Driver\StatementInterface'); $statementMock->expects($this->any()) ->method('prepare') - ->with('SELECT CURRVAL(\'' . $sequenceName . '\')'); + ->with('SELECT CURRVAL(\'"' . $sequenceName . '"\')'); $statementMock->expects($this->any()) ->method('execute') ->will($this->returnValue($resultMock)); From 611c73d58d79397014acdb4361939d79ab42c141 Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Tue, 6 Sep 2016 21:25:43 -0400 Subject: [PATCH 6/9] Platform mocking is not sufficient enough for additional escaping function needed. Could mock yet more methods and possible return values, but there is already a facility for stubbing instead using TrustedPlatform. Also fix quoting and elaborate on test values. --- src/TableGateway/Feature/SequenceFeature.php | 4 +- test/TableGateway/Feature/FeatureSetTest.php | 13 ++--- .../Feature/SequenceFeatureTest.php | 56 +++++++++++-------- test/TestAsset/TrustingPostgresqlPlatform.php | 20 +++++++ 4 files changed, 61 insertions(+), 32 deletions(-) create mode 100644 test/TestAsset/TrustingPostgresqlPlatform.php diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php index bb940bf6f8..cb1f493291 100644 --- a/src/TableGateway/Feature/SequenceFeature.php +++ b/src/TableGateway/Feature/SequenceFeature.php @@ -89,7 +89,7 @@ public function nextSequenceId() $sql = 'SELECT ' . $platform->quoteIdentifier($this->sequenceName) . '.NEXTVAL as "nextval" FROM dual'; break; case 'PostgreSQL': - $sql = 'SELECT NEXTVAL(\'' . $platform->quoteIdentifierChain($this->sequenceName) . '\')'; + $sql = 'SELECT NEXTVAL(' . $platform->quoteIdentifierChain($this->sequenceName) . ')'; break; default : return; @@ -117,7 +117,7 @@ public function lastSequenceId() $sql = 'SELECT ' . $platform->quoteIdentifier($this->sequenceName) . '.CURRVAL as "currval" FROM dual'; break; case 'PostgreSQL': - $sql = 'SELECT CURRVAL(\'' . $platform->quoteIdentifierChain($this->sequenceName) . '\')'; + $sql = 'SELECT CURRVAL(' . $platform->quoteIdentifierChain($this->sequenceName) . ')'; break; default : return; diff --git a/test/TableGateway/Feature/FeatureSetTest.php b/test/TableGateway/Feature/FeatureSetTest.php index 90098978db..c95856c720 100644 --- a/test/TableGateway/Feature/FeatureSetTest.php +++ b/test/TableGateway/Feature/FeatureSetTest.php @@ -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 { @@ -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()) @@ -140,7 +139,7 @@ public function testCallMagicCallSucceedsForValidMethodOfAddedFeature() $statementMock = $this->getMock('Zend\Db\Adapter\Driver\StatementInterface'); $statementMock->expects($this->any()) ->method('prepare') - ->with('SELECT CURRVAL(\'"' . $sequenceName . '"\')'); + ->with('SELECT CURRVAL(' . $sequenceName . ')'); $statementMock->expects($this->any()) ->method('execute') ->will($this->returnValue($resultMock)); @@ -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)); @@ -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); diff --git a/test/TableGateway/Feature/SequenceFeatureTest.php b/test/TableGateway/Feature/SequenceFeatureTest.php index 4ee99f0fbc..25cf853180 100644 --- a/test/TableGateway/Feature/SequenceFeatureTest.php +++ b/test/TableGateway/Feature/SequenceFeatureTest.php @@ -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') @@ -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; } } diff --git a/test/TestAsset/TrustingPostgresqlPlatform.php b/test/TestAsset/TrustingPostgresqlPlatform.php new file mode 100644 index 0000000000..a5c2ddf396 --- /dev/null +++ b/test/TestAsset/TrustingPostgresqlPlatform.php @@ -0,0 +1,20 @@ +quoteTrustedValue($value); + } +} From 0b99644f1e7f7cb56323c8fbe5cbede2ce3f6064 Mon Sep 17 00:00:00 2001 From: xorock Date: Thu, 8 Sep 2016 07:16:49 +0200 Subject: [PATCH 7/9] Revert quoting --- src/TableGateway/Feature/SequenceFeature.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php index cb1f493291..bb940bf6f8 100644 --- a/src/TableGateway/Feature/SequenceFeature.php +++ b/src/TableGateway/Feature/SequenceFeature.php @@ -89,7 +89,7 @@ public function nextSequenceId() $sql = 'SELECT ' . $platform->quoteIdentifier($this->sequenceName) . '.NEXTVAL as "nextval" FROM dual'; break; case 'PostgreSQL': - $sql = 'SELECT NEXTVAL(' . $platform->quoteIdentifierChain($this->sequenceName) . ')'; + $sql = 'SELECT NEXTVAL(\'' . $platform->quoteIdentifierChain($this->sequenceName) . '\')'; break; default : return; @@ -117,7 +117,7 @@ public function lastSequenceId() $sql = 'SELECT ' . $platform->quoteIdentifier($this->sequenceName) . '.CURRVAL as "currval" FROM dual'; break; case 'PostgreSQL': - $sql = 'SELECT CURRVAL(' . $platform->quoteIdentifierChain($this->sequenceName) . ')'; + $sql = 'SELECT CURRVAL(\'' . $platform->quoteIdentifierChain($this->sequenceName) . '\')'; break; default : return; From e03b4defa435464d2fa4dbf3718cb30b1eed11f4 Mon Sep 17 00:00:00 2001 From: xorock Date: Thu, 8 Sep 2016 07:22:55 +0200 Subject: [PATCH 8/9] Update SequenceFeatureTest.php --- test/TableGateway/Feature/SequenceFeatureTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/TableGateway/Feature/SequenceFeatureTest.php b/test/TableGateway/Feature/SequenceFeatureTest.php index 25cf853180..80f501b279 100644 --- a/test/TableGateway/Feature/SequenceFeatureTest.php +++ b/test/TableGateway/Feature/SequenceFeatureTest.php @@ -57,8 +57,8 @@ public function nextSequenceIdProvider() { 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")'], + ['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'] ]; } From 5a233e22fc5a7104f582b7a535f1d6a3c2426096 Mon Sep 17 00:00:00 2001 From: xorock Date: Thu, 8 Sep 2016 07:31:30 +0200 Subject: [PATCH 9/9] Update FeatureSetTest.php --- test/TableGateway/Feature/FeatureSetTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/TableGateway/Feature/FeatureSetTest.php b/test/TableGateway/Feature/FeatureSetTest.php index c95856c720..cb19237a9d 100644 --- a/test/TableGateway/Feature/FeatureSetTest.php +++ b/test/TableGateway/Feature/FeatureSetTest.php @@ -139,7 +139,7 @@ public function testCallMagicCallSucceedsForValidMethodOfAddedFeature() $statementMock = $this->getMock('Zend\Db\Adapter\Driver\StatementInterface'); $statementMock->expects($this->any()) ->method('prepare') - ->with('SELECT CURRVAL(' . $sequenceName . ')'); + ->with('SELECT CURRVAL(\'' . $sequenceName . '\')'); $statementMock->expects($this->any()) ->method('execute') ->will($this->returnValue($resultMock));