From 60585dfd71c72ac762b0102af7f1bed1eac0f1ac Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Wed, 9 Nov 2016 09:20:57 -0500 Subject: [PATCH 01/18] #186 FeatureSet to handle more than one feature of a type --- src/TableGateway/Feature/FeatureSet.php | 38 +++++---- src/TableGateway/Feature/SequenceFeature.php | 4 +- test/TableGateway/Feature/FeatureSetTest.php | 81 ++++++++++++++++++-- 3 files changed, 102 insertions(+), 21 deletions(-) diff --git a/src/TableGateway/Feature/FeatureSet.php b/src/TableGateway/Feature/FeatureSet.php index f557dfc7d8..92cad68aa0 100644 --- a/src/TableGateway/Feature/FeatureSet.php +++ b/src/TableGateway/Feature/FeatureSet.php @@ -19,7 +19,7 @@ class FeatureSet protected $tableGateway = null; /** - * @var AbstractFeature[] + * @var string[]AbstractFeature[] */ protected $features = []; @@ -46,14 +46,14 @@ public function setTableGateway(AbstractTableGateway $tableGateway) public function getFeatureByClassName($featureClassName) { - $feature = false; - foreach ($this->features as $potentialFeature) { - if ($potentialFeature instanceof $featureClassName) { - $feature = $potentialFeature; - break; - } + if(!array_key_exists($featureClassName, $this->features)) return false; + + $featuresByType = $this->features[$featureClassName]; + if(count($featuresByType) == 1) { + return $featuresByType[0]; + } else { + return $featuresByType; } - return $feature; } public function addFeatures(array $features) @@ -69,7 +69,7 @@ public function addFeature(AbstractFeature $feature) if ($this->tableGateway instanceof TableGatewayInterface) { $feature->setTableGateway($this->tableGateway); } - $this->features[] = $feature; + $this->features[get_class($feature)][] = $feature; return $this; } @@ -132,8 +132,8 @@ public function callMagicSet($property, $value) public function canCallMagicCall($method) { if (!empty($this->features)) { - foreach ($this->features as $feature) { - if (method_exists($feature, $method)) { + foreach ($this->features as $featureClass => $featuresSet) { + if (method_exists($featureClass, $method)) { return true; } } @@ -149,9 +149,19 @@ public function canCallMagicCall($method) */ public function callMagicCall($method, $arguments) { - foreach ($this->features as $feature) { - if (method_exists($feature, $method)) { - return $feature->$method($arguments); + foreach ($this->features as $featureClass => $featuresSet) { + if (method_exists($featureClass, $method)) { + + // iterator management instead of foreach to avoid extra conditions and indentations + reset($featuresSet); + $featureReturn = null; + while($featureReturn === null) { + $current = current($featuresSet); + $featureReturn = $current->$method($arguments); + next($featuresSet); + } + + return $featureReturn; } } diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php index 3c72fd3b6f..a192a47f8b 100644 --- a/src/TableGateway/Feature/SequenceFeature.php +++ b/src/TableGateway/Feature/SequenceFeature.php @@ -107,8 +107,10 @@ public function nextSequenceId() * Return the most recent value from the specified sequence in the database. * @return int */ - public function lastSequenceId() + public function lastSequenceId($sequenceName = null) { + if($sequenceName !== null && strcmp($sequenceName, $this->sequenceName) !== 0) return null; + $platform = $this->tableGateway->adapter->getPlatform(); $platformName = $platform->getName(); diff --git a/test/TableGateway/Feature/FeatureSetTest.php b/test/TableGateway/Feature/FeatureSetTest.php index bfc6271c17..c3b8cb2e03 100644 --- a/test/TableGateway/Feature/FeatureSetTest.php +++ b/test/TableGateway/Feature/FeatureSetTest.php @@ -10,6 +10,7 @@ namespace ZendTest\Db\TableGateway\Feature; use ReflectionClass; +use Zend\Db\TableGateway\Feature\AbstractFeature; use Zend\Db\TableGateway\Feature\FeatureSet; use Zend\Db\TableGateway\Feature\MasterSlaveFeature; use Zend\Db\TableGateway\Feature\SequenceFeature; @@ -80,6 +81,55 @@ public function testAddFeatureThatFeatureHasTableGatewayButFeatureSetDoesNotHave $this->assertInstanceOf('Zend\Db\TableGateway\Feature\FeatureSet', $featureSet->addFeature($feature)); } + /** + * @covers Zend\Db\TableGateway\Feature\FeatureSet::getFeatureByClassName + */ + public function testGetSingleFeatureByClassName() { + $featureMock = $this->getMock(AbstractFeature::class); + + $featureSet = new FeatureSet(); + $featureSet->addFeature($featureMock); + + $this->assertInstanceOf( + get_class($featureMock), + $featureSet->getFeatureByClassName(get_class($featureMock)), + "When only one feature of its type is added to FeatureSet, getFeatureByClassName() should return that single instance" + ); + } + + /** + * @covers Zend\Db\TableGateway\Feature\FeatureSet::getFeatureByClassName + */ + public function testGetAllFeaturesOfSameTypeByClassName() { + $featureMock1 = $this->getMock(AbstractFeature::class); + $featureMock2 = $this->getMock(AbstractFeature::class); + + $featureSet = new FeatureSet(); + $featureSet->addFeature($featureMock1); + $featureSet->addFeature($featureMock2); + + $features = $featureSet->getFeatureByClassName(get_class($featureMock1)); + + $this->assertTrue(is_array($features), "When multiple features of same type are added, they all should be return in array"); + + $this->assertInstanceOf(get_class($featureMock1), $features[0]); + $this->assertInstanceOf(get_class($featureMock2), $features[1]); + } + + /** + * @covers Zend\Db\TableGateway\Feature\FeatureSet::getFeatureByClassName + */ + public function testGetFeatureByClassNameReturnsFalseIfNotAdded() { + $featureMock = $this->getMock(AbstractFeature::class); + + $featureSet = new FeatureSet(); + + $this->assertFalse( + $featureSet->getFeatureByClassName(get_class($featureMock)), + "Requesting unregistered feature should return false" + ); + } + /** * @covers Zend\Db\TableGateway\Feature\FeatureSet::canCallMagicCall */ @@ -126,8 +176,28 @@ public function testCanCallMagicCallReturnsFalseWhenNoFeaturesHaveBeenAdded() */ public function testCallMagicCallSucceedsForValidMethodOfAddedFeature() { - $sequenceName = 'table_sequence'; + $featureSet = new FeatureSet; + $featureSet->addFeature($this->getMockSequence('table_sequence', 1)); + $this->assertEquals(1, $featureSet->callMagicCall('lastSequenceId', null)); + } + + /** + * @covers Zend\Db\TableGateway\Feature\FeatureSet::callMagicCall + */ + public function testCallMagicMethodAllSimilarFeaturesUntilNotNull() { + $featureSet = new FeatureSet(); + $featureSet->addFeature($this->getMockSequence('seq_1', 1)); + $featureSet->addFeature($this->getMockSequence('seq_2', 2)); + $featureSet->addFeature($this->getMockSequence('seq_3', 3)); + + $result = $featureSet->callMagicCall('lastSequenceId','seq_2'); + + $this->assertEquals(2, $result); + } + + // FeatureSet uses method_exists which does not work on mock objects. Therefore, need a real object. + private function getMockSequence($sequenceName, $expectedCurrVal) { $platformMock = $this->getMock('Zend\Db\Adapter\Platform\Postgresql'); $platformMock->expects($this->any()) ->method('getName')->will($this->returnValue('PostgreSQL')); @@ -135,7 +205,7 @@ public function testCallMagicCallSucceedsForValidMethodOfAddedFeature() $resultMock = $this->getMock('Zend\Db\Adapter\Driver\Pgsql\Result'); $resultMock->expects($this->any()) ->method('current') - ->will($this->returnValue(['currval' => 1])); + ->will($this->returnValue(['currval' => $expectedCurrVal])); $statementMock = $this->getMock('Zend\Db\Adapter\Driver\StatementInterface'); $statementMock->expects($this->any()) @@ -162,10 +232,9 @@ public function testCallMagicCallSucceedsForValidMethodOfAddedFeature() $reflectionProperty->setAccessible(true); $reflectionProperty->setValue($tableGatewayMock, $adapterMock); - $feature = new SequenceFeature('id', 'table_sequence'); + $feature = new SequenceFeature('id', $sequenceName); $feature->setTableGateway($tableGatewayMock); - $featureSet = new FeatureSet; - $featureSet->addFeature($feature); - $this->assertEquals(1, $featureSet->callMagicCall('lastSequenceId', null)); + + return $feature; } } From 99344c67d076759f6021f9a928c190fd7e540d90 Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Wed, 9 Nov 2016 09:34:03 -0500 Subject: [PATCH 02/18] #186 setTableGateway to be aware of multidimensional featureSet --- src/TableGateway/Feature/FeatureSet.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/TableGateway/Feature/FeatureSet.php b/src/TableGateway/Feature/FeatureSet.php index 92cad68aa0..03010838ce 100644 --- a/src/TableGateway/Feature/FeatureSet.php +++ b/src/TableGateway/Feature/FeatureSet.php @@ -38,8 +38,10 @@ public function __construct(array $features = []) public function setTableGateway(AbstractTableGateway $tableGateway) { $this->tableGateway = $tableGateway; - foreach ($this->features as $feature) { - $feature->setTableGateway($this->tableGateway); + foreach ($this->features as $featureClass => $featureSet) { + foreach($featureSet as $feature) { + $feature->setTableGateway($this->tableGateway); + } } return $this; } From 38a2d5dd3e964b24d3774d6fa88dbc23c50896d5 Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Wed, 9 Nov 2016 09:35:52 -0500 Subject: [PATCH 03/18] #186 apply to be aware of multidimensional featureSet --- src/TableGateway/Feature/FeatureSet.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/TableGateway/Feature/FeatureSet.php b/src/TableGateway/Feature/FeatureSet.php index 03010838ce..4ed1e51968 100644 --- a/src/TableGateway/Feature/FeatureSet.php +++ b/src/TableGateway/Feature/FeatureSet.php @@ -77,11 +77,13 @@ public function addFeature(AbstractFeature $feature) public function apply($method, $args) { - foreach ($this->features as $feature) { - if (method_exists($feature, $method)) { - $return = call_user_func_array([$feature, $method], $args); - if ($return === self::APPLY_HALT) { - break; + foreach ($this->features as $featureClass => $featureSet) { + foreach($featureSet as $feature) { + if (method_exists($feature, $method)) { + $return = call_user_func_array([$feature, $method], $args); + if ($return === self::APPLY_HALT) { + break; + } } } } From 7239dff3ad564487b4e31e77c20d3cf50279c619 Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Wed, 9 Nov 2016 10:01:39 -0500 Subject: [PATCH 04/18] cs rules --- src/TableGateway/Feature/FeatureSet.php | 12 +++++++----- src/TableGateway/Feature/SequenceFeature.php | 4 +++- test/TableGateway/Feature/FeatureSetTest.php | 15 ++++++++++----- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/TableGateway/Feature/FeatureSet.php b/src/TableGateway/Feature/FeatureSet.php index 4ed1e51968..19155c9dce 100644 --- a/src/TableGateway/Feature/FeatureSet.php +++ b/src/TableGateway/Feature/FeatureSet.php @@ -39,7 +39,7 @@ public function setTableGateway(AbstractTableGateway $tableGateway) { $this->tableGateway = $tableGateway; foreach ($this->features as $featureClass => $featureSet) { - foreach($featureSet as $feature) { + foreach ($featureSet as $feature) { $feature->setTableGateway($this->tableGateway); } } @@ -48,10 +48,12 @@ public function setTableGateway(AbstractTableGateway $tableGateway) public function getFeatureByClassName($featureClassName) { - if(!array_key_exists($featureClassName, $this->features)) return false; + if (!array_key_exists($featureClassName, $this->features)) { + return false; + } $featuresByType = $this->features[$featureClassName]; - if(count($featuresByType) == 1) { + if (count($featuresByType) == 1) { return $featuresByType[0]; } else { return $featuresByType; @@ -78,7 +80,7 @@ public function addFeature(AbstractFeature $feature) public function apply($method, $args) { foreach ($this->features as $featureClass => $featureSet) { - foreach($featureSet as $feature) { + foreach ($featureSet as $feature) { if (method_exists($feature, $method)) { $return = call_user_func_array([$feature, $method], $args); if ($return === self::APPLY_HALT) { @@ -159,7 +161,7 @@ public function callMagicCall($method, $arguments) // iterator management instead of foreach to avoid extra conditions and indentations reset($featuresSet); $featureReturn = null; - while($featureReturn === null) { + while ($featureReturn === null) { $current = current($featuresSet); $featureReturn = $current->$method($arguments); next($featuresSet); diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php index a192a47f8b..a1c1e99771 100644 --- a/src/TableGateway/Feature/SequenceFeature.php +++ b/src/TableGateway/Feature/SequenceFeature.php @@ -109,7 +109,9 @@ public function nextSequenceId() */ public function lastSequenceId($sequenceName = null) { - if($sequenceName !== null && strcmp($sequenceName, $this->sequenceName) !== 0) return null; + if($sequenceName !== null && strcmp($sequenceName, $this->sequenceName) !== 0) { + return null; + } $platform = $this->tableGateway->adapter->getPlatform(); $platformName = $platform->getName(); diff --git a/test/TableGateway/Feature/FeatureSetTest.php b/test/TableGateway/Feature/FeatureSetTest.php index c3b8cb2e03..7ac79d93ed 100644 --- a/test/TableGateway/Feature/FeatureSetTest.php +++ b/test/TableGateway/Feature/FeatureSetTest.php @@ -84,7 +84,8 @@ public function testAddFeatureThatFeatureHasTableGatewayButFeatureSetDoesNotHave /** * @covers Zend\Db\TableGateway\Feature\FeatureSet::getFeatureByClassName */ - public function testGetSingleFeatureByClassName() { + public function testGetSingleFeatureByClassName() + { $featureMock = $this->getMock(AbstractFeature::class); $featureSet = new FeatureSet(); @@ -100,7 +101,8 @@ public function testGetSingleFeatureByClassName() { /** * @covers Zend\Db\TableGateway\Feature\FeatureSet::getFeatureByClassName */ - public function testGetAllFeaturesOfSameTypeByClassName() { + public function testGetAllFeaturesOfSameTypeByClassName() + { $featureMock1 = $this->getMock(AbstractFeature::class); $featureMock2 = $this->getMock(AbstractFeature::class); @@ -119,7 +121,8 @@ public function testGetAllFeaturesOfSameTypeByClassName() { /** * @covers Zend\Db\TableGateway\Feature\FeatureSet::getFeatureByClassName */ - public function testGetFeatureByClassNameReturnsFalseIfNotAdded() { + public function testGetFeatureByClassNameReturnsFalseIfNotAdded() + { $featureMock = $this->getMock(AbstractFeature::class); $featureSet = new FeatureSet(); @@ -184,7 +187,8 @@ public function testCallMagicCallSucceedsForValidMethodOfAddedFeature() /** * @covers Zend\Db\TableGateway\Feature\FeatureSet::callMagicCall */ - public function testCallMagicMethodAllSimilarFeaturesUntilNotNull() { + public function testCallMagicMethodAllSimilarFeaturesUntilNotNull() + { $featureSet = new FeatureSet(); $featureSet->addFeature($this->getMockSequence('seq_1', 1)); @@ -197,7 +201,8 @@ public function testCallMagicMethodAllSimilarFeaturesUntilNotNull() { } // FeatureSet uses method_exists which does not work on mock objects. Therefore, need a real object. - private function getMockSequence($sequenceName, $expectedCurrVal) { + private function getMockSequence($sequenceName, $expectedCurrVal) + { $platformMock = $this->getMock('Zend\Db\Adapter\Platform\Postgresql'); $platformMock->expects($this->any()) ->method('getName')->will($this->returnValue('PostgreSQL')); From 82e311b817ebdfc2b3d9fc91b0d9ac32ba515f49 Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Wed, 9 Nov 2016 10:04:35 -0500 Subject: [PATCH 05/18] cs rules --- src/TableGateway/Feature/SequenceFeature.php | 4 ++-- test/TableGateway/Feature/FeatureSetTest.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php index a1c1e99771..f10ebafb49 100644 --- a/src/TableGateway/Feature/SequenceFeature.php +++ b/src/TableGateway/Feature/SequenceFeature.php @@ -109,8 +109,8 @@ public function nextSequenceId() */ public function lastSequenceId($sequenceName = null) { - if($sequenceName !== null && strcmp($sequenceName, $this->sequenceName) !== 0) { - return null; + if ($sequenceName !== null && strcmp($sequenceName, $this->sequenceName) !== 0) { + return; } $platform = $this->tableGateway->adapter->getPlatform(); diff --git a/test/TableGateway/Feature/FeatureSetTest.php b/test/TableGateway/Feature/FeatureSetTest.php index 7ac79d93ed..351bb6cb43 100644 --- a/test/TableGateway/Feature/FeatureSetTest.php +++ b/test/TableGateway/Feature/FeatureSetTest.php @@ -195,7 +195,7 @@ public function testCallMagicMethodAllSimilarFeaturesUntilNotNull() $featureSet->addFeature($this->getMockSequence('seq_2', 2)); $featureSet->addFeature($this->getMockSequence('seq_3', 3)); - $result = $featureSet->callMagicCall('lastSequenceId','seq_2'); + $result = $featureSet->callMagicCall('lastSequenceId', 'seq_2'); $this->assertEquals(2, $result); } From 07b5e3857bf320a761a208ca34525eb642aba362 Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Mon, 19 Dec 2016 13:52:06 -0500 Subject: [PATCH 06/18] #186 Add SERIAL column type to support autoincrement -style columns in PostgreSQL --- src/Sql/Ddl/Column/Serial.php | 23 ++++++++++++++++++ test/Sql/Ddl/Column/SerialTest.php | 38 ++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 src/Sql/Ddl/Column/Serial.php create mode 100644 test/Sql/Ddl/Column/SerialTest.php diff --git a/src/Sql/Ddl/Column/Serial.php b/src/Sql/Ddl/Column/Serial.php new file mode 100644 index 0000000000..c98f31fb0d --- /dev/null +++ b/src/Sql/Ddl/Column/Serial.php @@ -0,0 +1,23 @@ +assertEquals( + [['%s %s NOT NULL', ['id', 'SERIAL'], [$column::TYPE_IDENTIFIER, $column::TYPE_LITERAL]]], + $column->getExpressionData() + ); + + $column = new Serial('id'); + $column->addConstraint(new PrimaryKey()); + $this->assertEquals( + [ + ['%s %s NOT NULL', ['id', 'SERIAL'], [$column::TYPE_IDENTIFIER, $column::TYPE_LITERAL]], + ' ', + ['PRIMARY KEY', [], []], + ], + $column->getExpressionData() + ); + } +} From 98ede10df1d5f592b577799338082820a734503c Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Mon, 19 Dec 2016 23:04:00 -0500 Subject: [PATCH 07/18] #186 Generate sequence name for names up to 63 chars. --- src/TableGateway/AbstractTableGateway.php | 4 +- src/TableGateway/Feature/SequenceFeature.php | 33 +++++++++- .../Feature/SequenceFeatureTest.php | 65 ++++++++++++++++--- test/TestAsset/TrustingPostgresqlPlatform.php | 20 ++++++ 4 files changed, 108 insertions(+), 14 deletions(-) create mode 100644 test/TestAsset/TrustingPostgresqlPlatform.php diff --git a/src/TableGateway/AbstractTableGateway.php b/src/TableGateway/AbstractTableGateway.php index a27d50756f..1eddf56f31 100644 --- a/src/TableGateway/AbstractTableGateway.php +++ b/src/TableGateway/AbstractTableGateway.php @@ -26,7 +26,6 @@ * * @property AdapterInterface $adapter * @property int $lastInsertValue - * @property string $table */ abstract class AbstractTableGateway implements TableGatewayInterface { @@ -83,7 +82,6 @@ public function isInitialized() * Initialize * * @throws Exception\RuntimeException - * @return null */ public function initialize() { @@ -122,7 +120,7 @@ public function initialize() /** * Get table name * - * @return string + * @return string|array|TableIdentifier */ public function getTable() { diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php index f10ebafb49..286d68806b 100644 --- a/src/TableGateway/Feature/SequenceFeature.php +++ b/src/TableGateway/Feature/SequenceFeature.php @@ -12,6 +12,7 @@ use Zend\Db\Sql\Insert; use Zend\Db\Adapter\Driver\ResultInterface; use Zend\Db\Adapter\Driver\StatementInterface; +use Zend\Db\Sql\TableIdentifier; class SequenceFeature extends AbstractFeature { @@ -33,14 +34,42 @@ class SequenceFeature extends AbstractFeature /** * @param string $primaryKeyField - * @param string $sequenceName + * @param string|array|null $sequenceName */ - public function __construct($primaryKeyField, $sequenceName) + public function __construct($primaryKeyField, $sequenceName = null) { $this->primaryKeyField = $primaryKeyField; $this->sequenceName = $sequenceName; } + /** + * @return string + */ + public function getSequenceName() { + if ($this->sequenceName !== null) { + return $this->sequenceName; + } + + $platform = $this->tableGateway->getAdapter()->getPlatform(); + $table = $this->tableGateway->getTable(); + + $sequenceSuffix = '_' . $this->primaryKeyField . '_seq'; + + if(is_string($table)) { + $this->sequenceName = $table . $sequenceSuffix; + } elseif(is_array($table)) { + // assuming key 0 is schema name + $table[1] .= $sequenceSuffix; + $this->sequenceName = $table; + } elseif($table instanceof TableIdentifier) { + $this->sequenceName = $table->hasSchema() ? [$table->getSchema(), $table->getTable().$sequenceSuffix] : $table->getTable().$sequenceSuffix; + } + + $this->sequenceName = $platform->quoteIdentifierChain($this->sequenceName); + + return $this->sequenceName; + } + /** * @param Insert $insert * @return Insert diff --git a/test/TableGateway/Feature/SequenceFeatureTest.php b/test/TableGateway/Feature/SequenceFeatureTest.php index b3edc59c33..ccee82e847 100644 --- a/test/TableGateway/Feature/SequenceFeatureTest.php +++ b/test/TableGateway/Feature/SequenceFeatureTest.php @@ -10,13 +10,13 @@ namespace ZendTest\Db\TableGateway\Feature; use PHPUnit_Framework_TestCase; +use Zend\Db\Adapter\Platform\Postgresql; +use Zend\Db\Sql\TableIdentifier; use Zend\Db\TableGateway\Feature\SequenceFeature; +use ZendTest\Db\TestAsset\TrustingPostgresqlPlatform; class SequenceFeatureTest extends PHPUnit_Framework_TestCase { - /** @var SequenceFeature */ - protected $feature = null; - /** @var \Zend\Db\TableGateway\TableGateway */ protected $tableGateway = null; @@ -26,15 +26,51 @@ class SequenceFeatureTest extends PHPUnit_Framework_TestCase /** @var string sequence name */ protected $sequenceName = 'table_sequence'; - public function setup() + /** + * @dataProvider nextSequenceIdProvider + */ + public function testNextSequenceIdForNamedSequence($platformName, $statementSql) { - $this->feature = new SequenceFeature($this->primaryKeyField, $this->sequenceName); + $feature = new SequenceFeature($this->primaryKeyField, $this->sequenceName); + $feature->setTableGateway($this->tableGateway); + $feature->nextSequenceId(); } /** - * @dataProvider nextSequenceIdProvider + * @dataProvider tableIdentifierProvider */ - public function testNextSequenceId($platformName, $statementSql) + public function testSequenceNameGenerated($tableIdentifier, $sequenceName) + { + $adapter = $this->getMock('Zend\Db\Adapter\Adapter', ['getPlatform', 'createStatement'], [], '', false); + $adapter->expects($this->any()) + ->method('getPlatform') + ->will($this->returnValue(new TrustingPostgresqlPlatform())); + + $this->tableGateway = $this->getMockForAbstractClass('Zend\Db\TableGateway\TableGateway', [$tableIdentifier, $adapter], '', true); + + $sequence = new SequenceFeature('serial_column'); + $sequence->setTableGateway($this->tableGateway); + $sequence->getSequenceName(); + } + + public function testSequenceNameQueriedWhenTooLong() + { + + } + + /** + * Sequences for SERIAL columns start with no name which eventually gets filled. + * Ensure null value is replaced with actual on first call + * so that repeated calls to getSequenceName() do not make extra database calls (for long name case) + * + * Also test do not try to generate when name is manually supplied in constructor. + */ + public function testCacheSequenceName() + { + + } + + public function testNextSequenceIdForSerialColumn($platformName, $statementSql) { $platform = $this->getMockForAbstractClass('Zend\Db\Adapter\Platform\PlatformInterface', ['getName']); $platform->expects($this->any()) @@ -62,8 +98,10 @@ 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 = new SequenceFeature($this->primaryKeyField); + $feature->setTableGateway($this->tableGateway); + $feature->nextSequenceId(); } public function nextSequenceIdProvider() @@ -71,4 +109,13 @@ public function nextSequenceIdProvider() return [['PostgreSQL', 'SELECT NEXTVAL(\'"' . $this->sequenceName . '"\')'], ['Oracle', 'SELECT ' . $this->sequenceName . '.NEXTVAL as "nextval" FROM dual']]; } + + public function tableIdentifierProvider() + { + return [ + ['table', 'table_serial_column_seq'], + [['schema', 'table'], '"schema"."table_serial_column_seq"'], + [new TableIdentifier('table', 'schema'), '"schema"."table_serial_column_seq"'] + ]; + } } diff --git a/test/TestAsset/TrustingPostgresqlPlatform.php b/test/TestAsset/TrustingPostgresqlPlatform.php new file mode 100644 index 0000000000..858ab42afd --- /dev/null +++ b/test/TestAsset/TrustingPostgresqlPlatform.php @@ -0,0 +1,20 @@ +quoteTrustedValue($value); + } +} From 6d17e8f5909090ce7694f963eda30271bf9f81b3 Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Wed, 21 Dec 2016 23:00:34 -0500 Subject: [PATCH 08/18] #186 #165 Amendment to how SequenceFeatures are filtered from FeatureSet to match documentation. Since sequence names are not always known + more natural to interact with TableGateway using column names, filter function call by column names. Open to suggestion on how to improve this and filter somehow from FeatureSet *before* attempt to execute method on the feature (challenge to do so that does not break other feature types). Also, since SequenceFeature is more involved and requires long description, can consider moving to own docs page. --- doc/book/table-gateway.md | 100 ++++++++++++++++++ src/TableGateway/Feature/SequenceFeature.php | 14 ++- .../Feature/SequenceFeatureTest.php | 11 +- 3 files changed, 121 insertions(+), 4 deletions(-) diff --git a/doc/book/table-gateway.md b/doc/book/table-gateway.md index 9bf745f810..ea7cf8a39b 100644 --- a/doc/book/table-gateway.md +++ b/doc/book/table-gateway.md @@ -215,3 +215,103 @@ There are a number of features built-in and shipped with zend-db: $artistRow->name = 'New Name'; $artistRow->save(); ``` +- `SequenceFeature`: the ability to integrate with Oracle, PostgreSQL, + (TODO: and SqlServer 2016) sequence objects. Sequences are used for generating sequential integers. + These are usually used for automatically generating Primary Key IDs (similar to MySQL's + `AUTO_INCREMENT` but without performance penalties), or ensuring uniqueness of entity IDs + across multiple tables following some business rule (for example, unique invoice numbers across + multiple order systems). Sequence objects are + exclusively used for incrementing an integer and are not tied to table values. Therefore, they + need to be created manually prior to inserting data into tables requiring PKs and `UNIQUE` columns + using DDL + ```sql + CREATE SEQUENCE album_id; + ``` + + Sequence's `NEXTVAL` SQL construct can be used either as a default value for a column specified in + table's `CREATE` DDL, + or ran manually at every `insert` operation to have next available integer captured and inserted + in a table along with the rest of the values. + + Unless need to guarantee uniqueness across all tables, thus calling `sequence_name.nextval` on every `insert` + query across entire codebase, usually a separate sequence is created per table. Every `insert` + statement would have `album_id.nextval`, `artist_id.nextval` etc. as one of the values along with + the actual data. + + To be able to do these operations at the DB abstraction level, `TableGateway` needs to be informed + what columns should be managed by what sequence. + + If developer chooses to manually create a sequence for each table's autoincremented column (in Oracle + prior to *12c* this was the only way), then the name of sequence responsible for particular table + would known and can be applied to `TableGateway` right away. + + ```php + $table = new TableGateway('artist', $adapter, new Feature\SequenceFeature('id', 'artist_id_sequence')); + + $table->insert(['name'] => 'New Name'; + $nextId = $table->nextSequenceId('id'); + $lastInsertId = $table->lastSequenceId('id'); + ``` + + However, PostgreSQL (TODO: and Oracle since *12c*) allows automatic creation of sequences during `CREATE TABLE` + or `ALTER TABLE` operation by specifying column type `SERIAL`: + + ```sql + CREATE TABLE artist + { + id SERIAL, + name CHARACTER VARYING (30) + }; + ```` + + Or using Zend's `Db\Sql\DDL` + + ```php + $table = new CreateTable('artist'); + + $idColumn = new Serial('id'); + $nameColumn = new Char('name'); + + $table->addColumn($idColumn); + $table->addColumn($nameColumn); + ``` + + In this case, sequence is created automatically. `TableGateway` still has to be aware of what column + is getting autoincrement treatment but without knowing exactly what the sequence name is, second parameter + should be left blank: + + ```php + $table = new TableGateway('artist' $adapter, new Feature\SequenceFeature('id'); + ``` + + With second parameter left null, `TableGateway` will generate sequence name based on same rule + PostgreSQL uses (*tablename_columnname_seq* but if the resultant name is determined to be greater than + 63 characters, an additional query will be made to database schema to find what PostgreSQL has created + instead, since transaction rules are more complex. + + This is important to know if you have long table and column names, and do not want + to run an extra metadata query on every `TableGateway` object construction. If that is the case, + take note of what PostgreSQL created using + ```sql + SELECT 'column_default' FROM information_schema.columns WHERE + table_schema = 'public' + AND table_name = 'artist' + AND column_name = 'id' + AND column_default LIKE 'nextval%'; + ``` + + take note of what `nextval` is reading from, and add it to `SequenceFeature` constructor. + + There could be complex business rules needing multiple sequences in a table. `TableGateway` can have + multiple sequences added in an array: + + ```php + $table = new TableGateway('wholesales', $adapter, [ + new Feature\Sequence('id', 'sale_id_sequence'), + new Feature\Sequence('invoice_id', 'invoice_id_sequence)' + ]); + ``` + + Then calls to `$table->lastSequenceId('invoice_id')` will find the appropriate sequence instance to + get ID from. + \ No newline at end of file diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php index 286d68806b..869a09fbd6 100644 --- a/src/TableGateway/Feature/SequenceFeature.php +++ b/src/TableGateway/Feature/SequenceFeature.php @@ -106,10 +106,16 @@ public function postInsert(StatementInterface $statement, ResultInterface $resul /** * Generate a new value from the specified sequence in the database, and return it. + * @param $columnName string Column name which this sequence instance is expected to manage. + * If expectation does not match, ignore the call. * @return int */ - public function nextSequenceId() + public function nextSequenceId($columnName = null) { + if ($columnName !== null && strcmp($columnName, $this->primaryKeyField) !== 0) { + return; + } + $platform = $this->tableGateway->adapter->getPlatform(); $platformName = $platform->getName(); @@ -134,11 +140,13 @@ public function nextSequenceId() /** * Return the most recent value from the specified sequence in the database. + * @param $columnName string Column name which this sequence instance is expected to manage. + * If expectation does not match, ignore the call. * @return int */ - public function lastSequenceId($sequenceName = null) + public function lastSequenceId($columnName = null) { - if ($sequenceName !== null && strcmp($sequenceName, $this->sequenceName) !== 0) { + if ($columnName !== null && strcmp($columnName, $this->primaryKeyField) !== 0) { return; } diff --git a/test/TableGateway/Feature/SequenceFeatureTest.php b/test/TableGateway/Feature/SequenceFeatureTest.php index ccee82e847..5350e008fd 100644 --- a/test/TableGateway/Feature/SequenceFeatureTest.php +++ b/test/TableGateway/Feature/SequenceFeatureTest.php @@ -70,6 +70,9 @@ public function testCacheSequenceName() } + /** + * @dataProvider nextSequenceIdProvider + */ public function testNextSequenceIdForSerialColumn($platformName, $statementSql) { $platform = $this->getMockForAbstractClass('Zend\Db\Adapter\Platform\PlatformInterface', ['getName']); @@ -99,11 +102,17 @@ public function testNextSequenceIdForSerialColumn($platformName, $statementSql) ->will($this->returnValue($statement)); $this->tableGateway = $this->getMockForAbstractClass('Zend\Db\TableGateway\TableGateway', ['table', $adapter], '', true); - $feature = new SequenceFeature($this->primaryKeyField); + $feature = new SequenceFeature($this->primaryKeyField, $this->sequenceName); $feature->setTableGateway($this->tableGateway); $feature->nextSequenceId(); } + public function testDoNotReactToDifferentColumnName() { + $sequence1 = new SequenceFeature('col_1', 'seq_1'); + $this->assertEquals($sequence1->lastSequenceId('col_2'), null, 'Sequence should not react to foreign column name'); + $this->assertEquals($sequence1->nextSequenceId('col_2'), null, 'Sequence should not react to foreign column name'); + } + public function nextSequenceIdProvider() { return [['PostgreSQL', 'SELECT NEXTVAL(\'"' . $this->sequenceName . '"\')'], From 99cfd74317415f6eb2b9bf841bfec8d632cd41ea Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Wed, 21 Dec 2016 23:49:44 -0500 Subject: [PATCH 09/18] #186 #165 Docs SQL alignment --- doc/book/table-gateway.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/doc/book/table-gateway.md b/doc/book/table-gateway.md index ea7cf8a39b..d054f193d7 100644 --- a/doc/book/table-gateway.md +++ b/doc/book/table-gateway.md @@ -293,11 +293,12 @@ There are a number of features built-in and shipped with zend-db: to run an extra metadata query on every `TableGateway` object construction. If that is the case, take note of what PostgreSQL created using ```sql - SELECT 'column_default' FROM information_schema.columns WHERE - table_schema = 'public' - AND table_name = 'artist' - AND column_name = 'id' - AND column_default LIKE 'nextval%'; + SELECT 'column_default' + FROM information_schema.columns + WHERE table_schema = 'public' + AND table_name = 'artist' + AND column_name = 'id' + AND column_default LIKE 'nextval%'; ``` take note of what `nextval` is reading from, and add it to `SequenceFeature` constructor. From ce3b28cbaebf7ae73655ec4402dc91197525275c Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Sat, 24 Dec 2016 23:46:35 -0500 Subject: [PATCH 10/18] #186 #165 truncate based on length. If longer than 63 bytes for postgresql, query the actual name. --- doc/book/table-gateway.md | 9 +-- src/TableGateway/Feature/SequenceFeature.php | 74 ++++++++++++++----- test/TableGateway/Feature/FeatureSetTest.php | 17 ++--- .../Feature/SequenceFeatureTest.php | 54 ++++++++------ 4 files changed, 98 insertions(+), 56 deletions(-) diff --git a/doc/book/table-gateway.md b/doc/book/table-gateway.md index d054f193d7..75ef2f7d1c 100644 --- a/doc/book/table-gateway.md +++ b/doc/book/table-gateway.md @@ -286,19 +286,14 @@ There are a number of features built-in and shipped with zend-db: With second parameter left null, `TableGateway` will generate sequence name based on same rule PostgreSQL uses (*tablename_columnname_seq* but if the resultant name is determined to be greater than - 63 characters, an additional query will be made to database schema to find what PostgreSQL has created + 63 bytes, an additional query will be made to database schema to find what PostgreSQL has created instead, since transaction rules are more complex. This is important to know if you have long table and column names, and do not want to run an extra metadata query on every `TableGateway` object construction. If that is the case, take note of what PostgreSQL created using ```sql - SELECT 'column_default' - FROM information_schema.columns - WHERE table_schema = 'public' - AND table_name = 'artist' - AND column_name = 'id' - AND column_default LIKE 'nextval%'; + pg_get_serial_sequence('artist', 'id'); ``` take note of what `nextval` is reading from, and add it to `SequenceFeature` constructor. diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php index 869a09fbd6..1c3b1b28e1 100644 --- a/src/TableGateway/Feature/SequenceFeature.php +++ b/src/TableGateway/Feature/SequenceFeature.php @@ -9,6 +9,7 @@ namespace Zend\Db\TableGateway\Feature; +use Zend\Db\Adapter\Adapter; use Zend\Db\Sql\Insert; use Zend\Db\Adapter\Driver\ResultInterface; use Zend\Db\Adapter\Driver\StatementInterface; @@ -50,22 +51,50 @@ public function getSequenceName() { return $this->sequenceName; } - $platform = $this->tableGateway->getAdapter()->getPlatform(); - $table = $this->tableGateway->getTable(); + //@TODO move to PostgreSQL specific class (possibly decorator) + /** @var Adapter $adapter */ + $adapter = $this->tableGateway->getAdapter(); + $platform = $adapter->getPlatform(); + $tableIdentifier = $this->tableGateway->getTable(); + // need to preserve table name in case have to query postgres metadata + // (case for large resultant identifier names) + $tableName = ''; $sequenceSuffix = '_' . $this->primaryKeyField . '_seq'; + // To find whether exceed identifier length, need to keep track of combination of + // table name ane suffix but not including schema name. + // Since schema has to be appended in the end, + $sequenceObjectName = ''; - if(is_string($table)) { - $this->sequenceName = $table . $sequenceSuffix; - } elseif(is_array($table)) { + if(is_string($tableIdentifier)) { + $tableName = $tableIdentifier; + + $sequenceObjectName = $this->sequenceName = $tableIdentifier . $sequenceSuffix; + } elseif(is_array($tableIdentifier)) { // assuming key 0 is schema name - $table[1] .= $sequenceSuffix; - $this->sequenceName = $table; - } elseif($table instanceof TableIdentifier) { - $this->sequenceName = $table->hasSchema() ? [$table->getSchema(), $table->getTable().$sequenceSuffix] : $table->getTable().$sequenceSuffix; + $tableName = $tableIdentifier[1]; + + $this->sequenceName = $tableIdentifier; + $this->sequenceName[1] = $tableName.$sequenceSuffix; + $sequenceObjectName = $this->sequenceName[1]; + } elseif($tableIdentifier instanceof TableIdentifier) { + $tableName = $tableIdentifier->getTable(); + $sequenceObjectName = $tableName.$sequenceSuffix; + $this->sequenceName = $tableIdentifier->hasSchema() ? [$tableIdentifier->getSchema(), $sequenceObjectName] : $sequenceObjectName; + } + + if(strlen($sequenceObjectName) < 64 ) { + $this->sequenceName = $platform->quoteIdentifierChain($this->sequenceName); + return $this->sequenceName; } - $this->sequenceName = $platform->quoteIdentifierChain($this->sequenceName); + $statement = $adapter->createStatement(); + $statement->prepare('SELECT pg_get_serial_sequence(:table, :column)'); + $result = $statement->execute(['table' => $tableIdentifier, 'column' => $this->primaryKeyField]); + $this->sequenceName = $result->current()['pg_get_serial_sequence']; + + // there could be a benefit porting this algorithm here instead of extra query call + // https://github.com/postgres/postgres/blob/f0e44751d7175fa3394da2c8f85e3ceb3cdbfe63/src/backend/commands/indexcmds.c#L1485 return $this->sequenceName; } @@ -116,23 +145,27 @@ public function nextSequenceId($columnName = null) return; } - $platform = $this->tableGateway->adapter->getPlatform(); + /** @var Adapter $adapter */ + $adapter = $this->tableGateway->adapter; + $platform = $adapter->getPlatform(); $platformName = $platform->getName(); switch ($platformName) { case 'Oracle': $sql = 'SELECT ' . $platform->quoteIdentifier($this->sequenceName) . '.NEXTVAL as "nextval" FROM dual'; + $param = []; break; case 'PostgreSQL': - $sql = 'SELECT NEXTVAL(\'"' . $this->sequenceName . '"\')'; + $sql = 'SELECT NEXTVAL( :sequence_name )'; + $param = ['sequence_name' => $this->getSequenceName()]; break; default : return; } - $statement = $this->tableGateway->adapter->createStatement(); + $statement = $adapter->createStatement(); $statement->prepare($sql); - $result = $statement->execute(); + $result = $statement->execute($param); $sequence = $result->current(); unset($statement, $result); return $sequence['nextval']; @@ -150,23 +183,28 @@ public function lastSequenceId($columnName = null) return; } - $platform = $this->tableGateway->adapter->getPlatform(); + /** @var Adapter $adapter */ + $adapter = $this->tableGateway->adapter; + $platform = $adapter->getPlatform(); $platformName = $platform->getName(); switch ($platformName) { case 'Oracle': $sql = 'SELECT ' . $platform->quoteIdentifier($this->sequenceName) . '.CURRVAL as "currval" FROM dual'; + $param = []; break; case 'PostgreSQL': - $sql = 'SELECT CURRVAL(\'' . $this->sequenceName . '\')'; + $sql = 'SELECT CURRVAL( :sequence_name )'; + $param = ['sequence_name' => $this->getSequenceName()]; break; + //@TODO add SQLServer2016 default : return; } - $statement = $this->tableGateway->adapter->createStatement(); + $statement = $adapter->createStatement(); $statement->prepare($sql); - $result = $statement->execute(); + $result = $statement->execute($param); $sequence = $result->current(); unset($statement, $result); return $sequence['currval']; diff --git a/test/TableGateway/Feature/FeatureSetTest.php b/test/TableGateway/Feature/FeatureSetTest.php index 351bb6cb43..77d09d73ec 100644 --- a/test/TableGateway/Feature/FeatureSetTest.php +++ b/test/TableGateway/Feature/FeatureSetTest.php @@ -180,7 +180,7 @@ public function testCanCallMagicCallReturnsFalseWhenNoFeaturesHaveBeenAdded() public function testCallMagicCallSucceedsForValidMethodOfAddedFeature() { $featureSet = new FeatureSet; - $featureSet->addFeature($this->getMockSequence('table_sequence', 1)); + $featureSet->addFeature($this->getMockSequence('table_name', 'sequence_name', 1)); $this->assertEquals(1, $featureSet->callMagicCall('lastSequenceId', null)); } @@ -191,17 +191,17 @@ public function testCallMagicMethodAllSimilarFeaturesUntilNotNull() { $featureSet = new FeatureSet(); - $featureSet->addFeature($this->getMockSequence('seq_1', 1)); - $featureSet->addFeature($this->getMockSequence('seq_2', 2)); - $featureSet->addFeature($this->getMockSequence('seq_3', 3)); + $featureSet->addFeature($this->getMockSequence('col_1', 'seq_1', 1)); + $featureSet->addFeature($this->getMockSequence('col_2', 'seq_2', 2)); + $featureSet->addFeature($this->getMockSequence('col_3', 'seq_3', 3)); - $result = $featureSet->callMagicCall('lastSequenceId', 'seq_2'); + $result = $featureSet->callMagicCall('lastSequenceId', 'col_2'); $this->assertEquals(2, $result); } // FeatureSet uses method_exists which does not work on mock objects. Therefore, need a real object. - private function getMockSequence($sequenceName, $expectedCurrVal) + private function getMockSequence($columnName, $sequenceName, $expectedCurrVal) { $platformMock = $this->getMock('Zend\Db\Adapter\Platform\Postgresql'); $platformMock->expects($this->any()) @@ -214,8 +214,7 @@ private function getMockSequence($sequenceName, $expectedCurrVal) $statementMock = $this->getMock('Zend\Db\Adapter\Driver\StatementInterface'); $statementMock->expects($this->any()) - ->method('prepare') - ->with('SELECT CURRVAL(\'' . $sequenceName . '\')'); + ->method('prepare'); $statementMock->expects($this->any()) ->method('execute') ->will($this->returnValue($resultMock)); @@ -237,7 +236,7 @@ private function getMockSequence($sequenceName, $expectedCurrVal) $reflectionProperty->setAccessible(true); $reflectionProperty->setValue($tableGatewayMock, $adapterMock); - $feature = new SequenceFeature('id', $sequenceName); + $feature = new SequenceFeature($columnName, $sequenceName); $feature->setTableGateway($tableGatewayMock); return $feature; diff --git a/test/TableGateway/Feature/SequenceFeatureTest.php b/test/TableGateway/Feature/SequenceFeatureTest.php index 5350e008fd..b75ea14910 100644 --- a/test/TableGateway/Feature/SequenceFeatureTest.php +++ b/test/TableGateway/Feature/SequenceFeatureTest.php @@ -10,9 +10,9 @@ namespace ZendTest\Db\TableGateway\Feature; use PHPUnit_Framework_TestCase; -use Zend\Db\Adapter\Platform\Postgresql; use Zend\Db\Sql\TableIdentifier; use Zend\Db\TableGateway\Feature\SequenceFeature; +use ZendTest\Db\TestAsset\TrustingOraclePlatform; use ZendTest\Db\TestAsset\TrustingPostgresqlPlatform; class SequenceFeatureTest extends PHPUnit_Framework_TestCase @@ -24,17 +24,7 @@ class SequenceFeatureTest extends PHPUnit_Framework_TestCase protected $primaryKeyField = 'id'; /** @var string sequence name */ - protected $sequenceName = 'table_sequence'; - - /** - * @dataProvider nextSequenceIdProvider - */ - public function testNextSequenceIdForNamedSequence($platformName, $statementSql) - { - $feature = new SequenceFeature($this->primaryKeyField, $this->sequenceName); - $feature->setTableGateway($this->tableGateway); - $feature->nextSequenceId(); - } + protected $sequenceName = 'sequence_name'; /** * @dataProvider tableIdentifierProvider @@ -55,7 +45,31 @@ public function testSequenceNameGenerated($tableIdentifier, $sequenceName) public function testSequenceNameQueriedWhenTooLong() { + $adapter = $this->getMock('Zend\Db\Adapter\Adapter', ['getPlatform', 'createStatement'], [], '', false); + $adapter->expects($this->any()) + ->method('getPlatform') + ->will($this->returnValue(new TrustingPostgresqlPlatform())); + $result = $this->getMockForAbstractClass('Zend\Db\Adapter\Driver\ResultInterface', [], '', false, true, true, ['current']); + $result->expects($this->any()) + ->method('current') + ->will($this->returnValue(['pg_get_serial_sequence' => 'table_name_column_very_long_name_causing_postgresql_to_trun_seq'])); + $statement = $this->getMockForAbstractClass('Zend\Db\Adapter\Driver\StatementInterface', [], '', false, true, true, ['prepare', 'execute']); + $statement->expects($this->any()) + ->method('execute') + ->with(['table' => 'table_name', 'column' => 'column_very_long_name_causing_postgresql_to_truncate']) + ->will($this->returnValue($result)); + $statement->expects($this->any()) + ->method('prepare') + ->with('SELECT pg_get_serial_sequence(:table, :column)'); + $adapter->expects($this->once()) + ->method('createStatement') + ->will($this->returnValue($statement)); + $this->tableGateway = $this->getMockForAbstractClass('Zend\Db\TableGateway\TableGateway', ['table_name', $adapter], '', true); + $sequence = new SequenceFeature('column_very_long_name_causing_postgresql_to_truncate'); + $sequence->setTableGateway($this->tableGateway); + + $this->assertEquals('table_name_column_very_long_name_causing_postgresql_to_trun_seq', $sequence->getSequenceName()); } /** @@ -73,15 +87,8 @@ public function testCacheSequenceName() /** * @dataProvider nextSequenceIdProvider */ - public function testNextSequenceIdForSerialColumn($platformName, $statementSql) + public function testNextSequenceIdByPlatform($platform, $statementSql, $statementParameter) { - $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)); $adapter = $this->getMock('Zend\Db\Adapter\Adapter', ['getPlatform', 'createStatement'], [], '', false); $adapter->expects($this->any()) ->method('getPlatform') @@ -93,6 +100,7 @@ public function testNextSequenceIdForSerialColumn($platformName, $statementSql) $statement = $this->getMockForAbstractClass('Zend\Db\Adapter\Driver\StatementInterface', [], '', false, true, true, ['prepare', 'execute']); $statement->expects($this->any()) ->method('execute') + ->with($statementParameter) ->will($this->returnValue($result)); $statement->expects($this->any()) ->method('prepare') @@ -115,8 +123,10 @@ public function testDoNotReactToDifferentColumnName() { public function nextSequenceIdProvider() { - return [['PostgreSQL', 'SELECT NEXTVAL(\'"' . $this->sequenceName . '"\')'], - ['Oracle', 'SELECT ' . $this->sequenceName . '.NEXTVAL as "nextval" FROM dual']]; + return [ + [new TrustingPostgresqlPlatform(), 'SELECT NEXTVAL( :sequence_name )', ['sequence_name' => $this->sequenceName]], + [new TrustingOraclePlatform(), 'SELECT "' . $this->sequenceName . '".NEXTVAL as "nextval" FROM dual', []] + ]; } public function tableIdentifierProvider() From 7e7d3df26f267de3a99c294bca553ea2dae37d53 Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Sat, 24 Dec 2016 23:50:06 -0500 Subject: [PATCH 11/18] #186 missing last sequence id test. --- .../Feature/SequenceFeatureTest.php | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/TableGateway/Feature/SequenceFeatureTest.php b/test/TableGateway/Feature/SequenceFeatureTest.php index b75ea14910..c106a122b2 100644 --- a/test/TableGateway/Feature/SequenceFeatureTest.php +++ b/test/TableGateway/Feature/SequenceFeatureTest.php @@ -115,6 +115,38 @@ public function testNextSequenceIdByPlatform($platform, $statementSql, $statemen $feature->nextSequenceId(); } + + /** + * @dataProvider lastSequenceIdProvider + */ + public function testLastSequenceIdByPlatform($platform, $statementSql, $statementParameter) + { + $adapter = $this->getMock('Zend\Db\Adapter\Adapter', ['getPlatform', 'createStatement'], [], '', false); + $adapter->expects($this->any()) + ->method('getPlatform') + ->will($this->returnValue($platform)); + $result = $this->getMockForAbstractClass('Zend\Db\Adapter\Driver\ResultInterface', [], '', false, true, true, ['current']); + $result->expects($this->any()) + ->method('current') + ->will($this->returnValue(['currval' => 1])); + $statement = $this->getMockForAbstractClass('Zend\Db\Adapter\Driver\StatementInterface', [], '', false, true, true, ['prepare', 'execute']); + $statement->expects($this->any()) + ->method('execute') + ->with($statementParameter) + ->will($this->returnValue($result)); + $statement->expects($this->any()) + ->method('prepare') + ->with($statementSql); + $adapter->expects($this->once()) + ->method('createStatement') + ->will($this->returnValue($statement)); + $this->tableGateway = $this->getMockForAbstractClass('Zend\Db\TableGateway\TableGateway', ['table', $adapter], '', true); + + $feature = new SequenceFeature($this->primaryKeyField, $this->sequenceName); + $feature->setTableGateway($this->tableGateway); + $feature->lastSequenceId(); + } + public function testDoNotReactToDifferentColumnName() { $sequence1 = new SequenceFeature('col_1', 'seq_1'); $this->assertEquals($sequence1->lastSequenceId('col_2'), null, 'Sequence should not react to foreign column name'); @@ -129,6 +161,14 @@ public function nextSequenceIdProvider() ]; } + public function lastSequenceIdProvider() + { + return [ + [new TrustingPostgresqlPlatform(), 'SELECT CURRVAL( :sequence_name )', ['sequence_name' => $this->sequenceName]], + [new TrustingOraclePlatform(), 'SELECT "' . $this->sequenceName . '".CURRVAL as "currval" FROM dual', []] + ]; + } + public function tableIdentifierProvider() { return [ From ca1757569a1f12e81e489f164f6235f7042f89a2 Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Sat, 24 Dec 2016 23:53:59 -0500 Subject: [PATCH 12/18] #186 change misnomer name primary key to just column. Sequence does not imply primary key. --- src/TableGateway/Feature/SequenceFeature.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php index 1c3b1b28e1..066e09a00b 100644 --- a/src/TableGateway/Feature/SequenceFeature.php +++ b/src/TableGateway/Feature/SequenceFeature.php @@ -20,7 +20,7 @@ class SequenceFeature extends AbstractFeature /** * @var string */ - protected $primaryKeyField; + protected $sequencedColumn; /** * @var string @@ -34,12 +34,12 @@ class SequenceFeature extends AbstractFeature /** - * @param string $primaryKeyField + * @param string $sequencedColumn * @param string|array|null $sequenceName */ - public function __construct($primaryKeyField, $sequenceName = null) + public function __construct($sequencedColumn, $sequenceName = null) { - $this->primaryKeyField = $primaryKeyField; + $this->sequencedColumn = $sequencedColumn; $this->sequenceName = $sequenceName; } @@ -60,7 +60,7 @@ public function getSequenceName() { // (case for large resultant identifier names) $tableName = ''; - $sequenceSuffix = '_' . $this->primaryKeyField . '_seq'; + $sequenceSuffix = '_' . $this->sequencedColumn . '_seq'; // To find whether exceed identifier length, need to keep track of combination of // table name ane suffix but not including schema name. // Since schema has to be appended in the end, @@ -90,7 +90,7 @@ public function getSequenceName() { $statement = $adapter->createStatement(); $statement->prepare('SELECT pg_get_serial_sequence(:table, :column)'); - $result = $statement->execute(['table' => $tableIdentifier, 'column' => $this->primaryKeyField]); + $result = $statement->execute(['table' => $tableIdentifier, 'column' => $this->sequencedColumn]); $this->sequenceName = $result->current()['pg_get_serial_sequence']; // there could be a benefit porting this algorithm here instead of extra query call @@ -107,7 +107,7 @@ public function preInsert(Insert $insert) { $columns = $insert->getRawState('columns'); $values = $insert->getRawState('values'); - $key = array_search($this->primaryKeyField, $columns); + $key = array_search($this->sequencedColumn, $columns); if ($key !== false) { $this->sequenceValue = $values[$key]; return $insert; @@ -118,7 +118,7 @@ public function preInsert(Insert $insert) return $insert; } - $insert->values([$this->primaryKeyField => $this->sequenceValue], Insert::VALUES_MERGE); + $insert->values([$this->sequencedColumn => $this->sequenceValue], Insert::VALUES_MERGE); return $insert; } @@ -141,7 +141,7 @@ public function postInsert(StatementInterface $statement, ResultInterface $resul */ public function nextSequenceId($columnName = null) { - if ($columnName !== null && strcmp($columnName, $this->primaryKeyField) !== 0) { + if ($columnName !== null && strcmp($columnName, $this->sequencedColumn) !== 0) { return; } @@ -179,7 +179,7 @@ public function nextSequenceId($columnName = null) */ public function lastSequenceId($columnName = null) { - if ($columnName !== null && strcmp($columnName, $this->primaryKeyField) !== 0) { + if ($columnName !== null && strcmp($columnName, $this->sequencedColumn) !== 0) { return; } From a4b00d3006ef673f26eaa74af233ab0f66064d91 Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Sun, 25 Dec 2016 00:01:43 -0500 Subject: [PATCH 13/18] #186 filled test for reusing implicit sequence name generation --- .../Feature/SequenceFeatureTest.php | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/TableGateway/Feature/SequenceFeatureTest.php b/test/TableGateway/Feature/SequenceFeatureTest.php index c106a122b2..ba938a63d6 100644 --- a/test/TableGateway/Feature/SequenceFeatureTest.php +++ b/test/TableGateway/Feature/SequenceFeatureTest.php @@ -81,7 +81,32 @@ public function testSequenceNameQueriedWhenTooLong() */ public function testCacheSequenceName() { + $adapter = $this->getMock('Zend\Db\Adapter\Adapter', ['getPlatform', 'createStatement'], [], '', false); + $adapter->expects($this->any()) + ->method('getPlatform') + ->will($this->returnValue(new TrustingPostgresqlPlatform())); + $result = $this->getMockForAbstractClass('Zend\Db\Adapter\Driver\ResultInterface', [], '', false, true, true, ['current']); + $result->expects($this->once()) + ->method('current') + ->will($this->returnValue(['pg_get_serial_sequence' => 'table_name_column_very_long_name_causing_postgresql_to_trun_seq'])); + $statement = $this->getMockForAbstractClass('Zend\Db\Adapter\Driver\StatementInterface', [], '', false, true, true, ['prepare', 'execute']); + $statement->expects($this->once()) + ->method('execute') + ->with(['table' => 'table_name', 'column' => 'column_very_long_name_causing_postgresql_to_truncate']) + ->will($this->returnValue($result)); + $statement->expects($this->once()) + ->method('prepare') + ->with('SELECT pg_get_serial_sequence(:table, :column)'); + $adapter->expects($this->once()) + ->method('createStatement') + ->will($this->returnValue($statement)); + $this->tableGateway = $this->getMockForAbstractClass('Zend\Db\TableGateway\TableGateway', ['table_name', $adapter], '', true); + $sequence = new SequenceFeature('column_very_long_name_causing_postgresql_to_truncate'); + $sequence->setTableGateway($this->tableGateway); + + $this->assertEquals('table_name_column_very_long_name_causing_postgresql_to_trun_seq', $sequence->getSequenceName()); + $this->assertEquals('table_name_column_very_long_name_causing_postgresql_to_trun_seq', $sequence->getSequenceName()); } /** From 57585a8fad1db7a32f37773681a7c2237be309ad Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Sun, 25 Dec 2016 00:05:08 -0500 Subject: [PATCH 14/18] #186 in docs let know that sequence may have different increment interval. --- doc/book/table-gateway.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/book/table-gateway.md b/doc/book/table-gateway.md index 75ef2f7d1c..8814b2078b 100644 --- a/doc/book/table-gateway.md +++ b/doc/book/table-gateway.md @@ -220,10 +220,10 @@ There are a number of features built-in and shipped with zend-db: These are usually used for automatically generating Primary Key IDs (similar to MySQL's `AUTO_INCREMENT` but without performance penalties), or ensuring uniqueness of entity IDs across multiple tables following some business rule (for example, unique invoice numbers across - multiple order systems). Sequence objects are - exclusively used for incrementing an integer and are not tied to table values. Therefore, they - need to be created manually prior to inserting data into tables requiring PKs and `UNIQUE` columns - using DDL + multiple order systems). Sequence objects are exclusively used for incrementing an integer + by specified interval (1 by default) and are not tied to table values. Therefore, + they need to be created manually prior to inserting data into tables requiring PKs and `UNIQUE` + columns using DDL ```sql CREATE SEQUENCE album_id; ``` From 4ed6ff29a5e8481ed5e349e9e797fcf53e8f9217 Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Sun, 25 Dec 2016 00:14:17 -0500 Subject: [PATCH 15/18] #186 pass through CS fixer --- src/TableGateway/Feature/SequenceFeature.php | 38 ++++++++++++------- test/TableGateway/Feature/FeatureSetTest.php | 26 +++++++------ .../Feature/SequenceFeatureTest.php | 16 ++++---- test/TestAsset/TrustingPostgresqlPlatform.php | 6 ++- 4 files changed, 52 insertions(+), 34 deletions(-) diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php index 066e09a00b..c3d1779355 100644 --- a/src/TableGateway/Feature/SequenceFeature.php +++ b/src/TableGateway/Feature/SequenceFeature.php @@ -1,8 +1,10 @@ sequenceName !== null) { return $this->sequenceName; } @@ -60,31 +62,32 @@ public function getSequenceName() { // (case for large resultant identifier names) $tableName = ''; - $sequenceSuffix = '_' . $this->sequencedColumn . '_seq'; + $sequenceSuffix = '_'.$this->sequencedColumn.'_seq'; // To find whether exceed identifier length, need to keep track of combination of // table name ane suffix but not including schema name. // Since schema has to be appended in the end, $sequenceObjectName = ''; - if(is_string($tableIdentifier)) { + if (is_string($tableIdentifier)) { $tableName = $tableIdentifier; - $sequenceObjectName = $this->sequenceName = $tableIdentifier . $sequenceSuffix; - } elseif(is_array($tableIdentifier)) { + $sequenceObjectName = $this->sequenceName = $tableIdentifier.$sequenceSuffix; + } elseif (is_array($tableIdentifier)) { // assuming key 0 is schema name $tableName = $tableIdentifier[1]; $this->sequenceName = $tableIdentifier; $this->sequenceName[1] = $tableName.$sequenceSuffix; $sequenceObjectName = $this->sequenceName[1]; - } elseif($tableIdentifier instanceof TableIdentifier) { + } elseif ($tableIdentifier instanceof TableIdentifier) { $tableName = $tableIdentifier->getTable(); $sequenceObjectName = $tableName.$sequenceSuffix; $this->sequenceName = $tableIdentifier->hasSchema() ? [$tableIdentifier->getSchema(), $sequenceObjectName] : $sequenceObjectName; } - if(strlen($sequenceObjectName) < 64 ) { + if (strlen($sequenceObjectName) < 64) { $this->sequenceName = $platform->quoteIdentifierChain($this->sequenceName); + return $this->sequenceName; } @@ -101,6 +104,7 @@ public function getSequenceName() { /** * @param Insert $insert + * * @return Insert */ public function preInsert(Insert $insert) @@ -110,6 +114,7 @@ public function preInsert(Insert $insert) $key = array_search($this->sequencedColumn, $columns); if ($key !== false) { $this->sequenceValue = $values[$key]; + return $insert; } @@ -119,12 +124,13 @@ public function preInsert(Insert $insert) } $insert->values([$this->sequencedColumn => $this->sequenceValue], Insert::VALUES_MERGE); + return $insert; } /** * @param StatementInterface $statement - * @param ResultInterface $result + * @param ResultInterface $result */ public function postInsert(StatementInterface $statement, ResultInterface $result) { @@ -135,8 +141,10 @@ public function postInsert(StatementInterface $statement, ResultInterface $resul /** * Generate a new value from the specified sequence in the database, and return it. + * * @param $columnName string Column name which this sequence instance is expected to manage. * If expectation does not match, ignore the call. + * * @return int */ public function nextSequenceId($columnName = null) @@ -152,7 +160,7 @@ public function nextSequenceId($columnName = null) switch ($platformName) { case 'Oracle': - $sql = 'SELECT ' . $platform->quoteIdentifier($this->sequenceName) . '.NEXTVAL as "nextval" FROM dual'; + $sql = 'SELECT '.$platform->quoteIdentifier($this->sequenceName).'.NEXTVAL as "nextval" FROM dual'; $param = []; break; case 'PostgreSQL': @@ -168,13 +176,16 @@ public function nextSequenceId($columnName = null) $result = $statement->execute($param); $sequence = $result->current(); unset($statement, $result); + return $sequence['nextval']; } /** * Return the most recent value from the specified sequence in the database. + * * @param $columnName string Column name which this sequence instance is expected to manage. * If expectation does not match, ignore the call. + * * @return int */ public function lastSequenceId($columnName = null) @@ -190,7 +201,7 @@ public function lastSequenceId($columnName = null) switch ($platformName) { case 'Oracle': - $sql = 'SELECT ' . $platform->quoteIdentifier($this->sequenceName) . '.CURRVAL as "currval" FROM dual'; + $sql = 'SELECT '.$platform->quoteIdentifier($this->sequenceName).'.CURRVAL as "currval" FROM dual'; $param = []; break; case 'PostgreSQL': @@ -207,6 +218,7 @@ public function lastSequenceId($columnName = null) $result = $statement->execute($param); $sequence = $result->current(); unset($statement, $result); + return $sequence['currval']; } } diff --git a/test/TableGateway/Feature/FeatureSetTest.php b/test/TableGateway/Feature/FeatureSetTest.php index 77d09d73ec..38083c3578 100644 --- a/test/TableGateway/Feature/FeatureSetTest.php +++ b/test/TableGateway/Feature/FeatureSetTest.php @@ -1,8 +1,10 @@ setTableGateway($tableGatewayMock); $this->assertInstanceOf('Zend\Db\TableGateway\Feature\FeatureSet', $featureSet->addFeature($feature)); @@ -77,7 +79,7 @@ public function testAddFeatureThatFeatureHasTableGatewayButFeatureSetDoesNotHave $feature = new MetadataFeature($metadataMock); $feature->setTableGateway($tableGatewayMock); - $featureSet = new FeatureSet; + $featureSet = new FeatureSet(); $this->assertInstanceOf('Zend\Db\TableGateway\Feature\FeatureSet', $featureSet->addFeature($feature)); } @@ -94,7 +96,7 @@ public function testGetSingleFeatureByClassName() $this->assertInstanceOf( get_class($featureMock), $featureSet->getFeatureByClassName(get_class($featureMock)), - "When only one feature of its type is added to FeatureSet, getFeatureByClassName() should return that single instance" + 'When only one feature of its type is added to FeatureSet, getFeatureByClassName() should return that single instance' ); } @@ -112,7 +114,7 @@ public function testGetAllFeaturesOfSameTypeByClassName() $features = $featureSet->getFeatureByClassName(get_class($featureMock1)); - $this->assertTrue(is_array($features), "When multiple features of same type are added, they all should be return in array"); + $this->assertTrue(is_array($features), 'When multiple features of same type are added, they all should be return in array'); $this->assertInstanceOf(get_class($featureMock1), $features[0]); $this->assertInstanceOf(get_class($featureMock2), $features[1]); @@ -129,7 +131,7 @@ public function testGetFeatureByClassNameReturnsFalseIfNotAdded() $this->assertFalse( $featureSet->getFeatureByClassName(get_class($featureMock)), - "Requesting unregistered feature should return false" + 'Requesting unregistered feature should return false' ); } @@ -139,12 +141,12 @@ public function testGetFeatureByClassNameReturnsFalseIfNotAdded() public function testCanCallMagicCallReturnsTrueForAddedMethodOfAddedFeature() { $feature = new SequenceFeature('id', 'table_sequence'); - $featureSet = new FeatureSet; + $featureSet = new FeatureSet(); $featureSet->addFeature($feature); $this->assertTrue( $featureSet->canCallMagicCall('lastSequenceId'), - "Should have been able to call lastSequenceId from the Sequence Feature" + 'Should have been able to call lastSequenceId from the Sequence Feature' ); } @@ -154,12 +156,12 @@ public function testCanCallMagicCallReturnsTrueForAddedMethodOfAddedFeature() public function testCanCallMagicCallReturnsFalseForAddedMethodOfAddedFeature() { $feature = new SequenceFeature('id', 'table_sequence'); - $featureSet = new FeatureSet; + $featureSet = new FeatureSet(); $featureSet->addFeature($feature); $this->assertFalse( $featureSet->canCallMagicCall('postInitialize'), - "Should have been able to call postInitialize from the MetaData Feature" + 'Should have been able to call postInitialize from the MetaData Feature' ); } @@ -168,7 +170,7 @@ public function testCanCallMagicCallReturnsFalseForAddedMethodOfAddedFeature() */ public function testCanCallMagicCallReturnsFalseWhenNoFeaturesHaveBeenAdded() { - $featureSet = new FeatureSet; + $featureSet = new FeatureSet(); $this->assertFalse( $featureSet->canCallMagicCall('lastSequenceId') ); @@ -179,7 +181,7 @@ public function testCanCallMagicCallReturnsFalseWhenNoFeaturesHaveBeenAdded() */ public function testCallMagicCallSucceedsForValidMethodOfAddedFeature() { - $featureSet = new FeatureSet; + $featureSet = new FeatureSet(); $featureSet->addFeature($this->getMockSequence('table_name', 'sequence_name', 1)); $this->assertEquals(1, $featureSet->callMagicCall('lastSequenceId', null)); } diff --git a/test/TableGateway/Feature/SequenceFeatureTest.php b/test/TableGateway/Feature/SequenceFeatureTest.php index ba938a63d6..aef9900521 100644 --- a/test/TableGateway/Feature/SequenceFeatureTest.php +++ b/test/TableGateway/Feature/SequenceFeatureTest.php @@ -1,8 +1,10 @@ nextSequenceId(); } - /** * @dataProvider lastSequenceIdProvider */ @@ -172,7 +173,8 @@ public function testLastSequenceIdByPlatform($platform, $statementSql, $statemen $feature->lastSequenceId(); } - public function testDoNotReactToDifferentColumnName() { + public function testDoNotReactToDifferentColumnName() + { $sequence1 = new SequenceFeature('col_1', 'seq_1'); $this->assertEquals($sequence1->lastSequenceId('col_2'), null, 'Sequence should not react to foreign column name'); $this->assertEquals($sequence1->nextSequenceId('col_2'), null, 'Sequence should not react to foreign column name'); @@ -182,7 +184,7 @@ public function nextSequenceIdProvider() { return [ [new TrustingPostgresqlPlatform(), 'SELECT NEXTVAL( :sequence_name )', ['sequence_name' => $this->sequenceName]], - [new TrustingOraclePlatform(), 'SELECT "' . $this->sequenceName . '".NEXTVAL as "nextval" FROM dual', []] + [new TrustingOraclePlatform(), 'SELECT "'.$this->sequenceName.'".NEXTVAL as "nextval" FROM dual', []], ]; } @@ -190,7 +192,7 @@ public function lastSequenceIdProvider() { return [ [new TrustingPostgresqlPlatform(), 'SELECT CURRVAL( :sequence_name )', ['sequence_name' => $this->sequenceName]], - [new TrustingOraclePlatform(), 'SELECT "' . $this->sequenceName . '".CURRVAL as "currval" FROM dual', []] + [new TrustingOraclePlatform(), 'SELECT "'.$this->sequenceName.'".CURRVAL as "currval" FROM dual', []], ]; } @@ -199,7 +201,7 @@ public function tableIdentifierProvider() return [ ['table', 'table_serial_column_seq'], [['schema', 'table'], '"schema"."table_serial_column_seq"'], - [new TableIdentifier('table', 'schema'), '"schema"."table_serial_column_seq"'] + [new TableIdentifier('table', 'schema'), '"schema"."table_serial_column_seq"'], ]; } } diff --git a/test/TestAsset/TrustingPostgresqlPlatform.php b/test/TestAsset/TrustingPostgresqlPlatform.php index 858ab42afd..c9308cc466 100644 --- a/test/TestAsset/TrustingPostgresqlPlatform.php +++ b/test/TestAsset/TrustingPostgresqlPlatform.php @@ -1,9 +1,10 @@ quoteTrustedValue($value); } } From daa9c33fe2bfae642008dd5238908cfe584f6bd0 Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Sun, 25 Dec 2016 13:52:12 -0500 Subject: [PATCH 16/18] #186 Take into account array parameter for sequence name to specify schema. --- src/TableGateway/Feature/SequenceFeature.php | 13 +++-- .../Feature/SequenceFeatureTest.php | 56 ++++++++++--------- 2 files changed, 39 insertions(+), 30 deletions(-) diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php index c3d1779355..ca7c8d6c82 100644 --- a/src/TableGateway/Feature/SequenceFeature.php +++ b/src/TableGateway/Feature/SequenceFeature.php @@ -49,14 +49,19 @@ public function __construct($sequencedColumn, $sequenceName = null) */ public function getSequenceName() { - if ($this->sequenceName !== null) { - return $this->sequenceName; - } - //@TODO move to PostgreSQL specific class (possibly decorator) /** @var Adapter $adapter */ $adapter = $this->tableGateway->getAdapter(); $platform = $adapter->getPlatform(); + + if ($this->sequenceName !== null) { + if (is_array($this->sequenceName)) { + $this->sequenceName = $platform->quoteIdentifierChain($this->sequenceName); + } + + return $this->sequenceName; + } + $tableIdentifier = $this->tableGateway->getTable(); // need to preserve table name in case have to query postgres metadata // (case for large resultant identifier names) diff --git a/test/TableGateway/Feature/SequenceFeatureTest.php b/test/TableGateway/Feature/SequenceFeatureTest.php index aef9900521..8436e72831 100644 --- a/test/TableGateway/Feature/SequenceFeatureTest.php +++ b/test/TableGateway/Feature/SequenceFeatureTest.php @@ -29,20 +29,34 @@ class SequenceFeatureTest extends PHPUnit_Framework_TestCase protected $sequenceName = 'sequence_name'; /** - * @dataProvider tableIdentifierProvider + * @dataProvider identifierProvider */ - public function testSequenceNameGenerated($tableIdentifier, $sequenceName) + public function testSequenceNameGenerated($platform, $tableIdentifier, $sequenceName, $expectedSequenceName) { $adapter = $this->getMock('Zend\Db\Adapter\Adapter', ['getPlatform', 'createStatement'], [], '', false); $adapter->expects($this->any()) ->method('getPlatform') - ->will($this->returnValue(new TrustingPostgresqlPlatform())); + ->will($this->returnValue($platform)); $this->tableGateway = $this->getMockForAbstractClass('Zend\Db\TableGateway\TableGateway', [$tableIdentifier, $adapter], '', true); - $sequence = new SequenceFeature('serial_column'); + $sequence = new SequenceFeature('serial_column', $sequenceName); $sequence->setTableGateway($this->tableGateway); - $sequence->getSequenceName(); + $this->assertEquals($expectedSequenceName, $sequence->getSequenceName()); + } + + public function identifierProvider() + { + return [ + [new TrustingPostgresqlPlatform(), + 'table', null, '"table_serial_column_seq"', ], + [new TrustingPostgresqlPlatform(), + ['schema', 'table'], null, '"schema"."table_serial_column_seq"', ], + [new TrustingPostgresqlPlatform(), + new TableIdentifier('table', 'schema'), null, '"schema"."table_serial_column_seq"', ], + [new TrustingPostgresqlPlatform(), + new TableIdentifier('table', 'schema'), ['schema', 'sequence_name'], '"schema"."sequence_name"', ], + ]; } public function testSequenceNameQueriedWhenTooLong() @@ -142,6 +156,13 @@ public function testNextSequenceIdByPlatform($platform, $statementSql, $statemen $feature->nextSequenceId(); } + public function nextSequenceIdProvider() + { + return [ + [new TrustingPostgresqlPlatform(), 'SELECT NEXTVAL( :sequence_name )', ['sequence_name' => $this->sequenceName]], + [new TrustingOraclePlatform(), 'SELECT "'.$this->sequenceName.'".NEXTVAL as "nextval" FROM dual', []], + ]; + } /** * @dataProvider lastSequenceIdProvider */ @@ -173,21 +194,6 @@ public function testLastSequenceIdByPlatform($platform, $statementSql, $statemen $feature->lastSequenceId(); } - public function testDoNotReactToDifferentColumnName() - { - $sequence1 = new SequenceFeature('col_1', 'seq_1'); - $this->assertEquals($sequence1->lastSequenceId('col_2'), null, 'Sequence should not react to foreign column name'); - $this->assertEquals($sequence1->nextSequenceId('col_2'), null, 'Sequence should not react to foreign column name'); - } - - public function nextSequenceIdProvider() - { - return [ - [new TrustingPostgresqlPlatform(), 'SELECT NEXTVAL( :sequence_name )', ['sequence_name' => $this->sequenceName]], - [new TrustingOraclePlatform(), 'SELECT "'.$this->sequenceName.'".NEXTVAL as "nextval" FROM dual', []], - ]; - } - public function lastSequenceIdProvider() { return [ @@ -196,12 +202,10 @@ public function lastSequenceIdProvider() ]; } - public function tableIdentifierProvider() + public function testDoNotReactToDifferentColumnName() { - return [ - ['table', 'table_serial_column_seq'], - [['schema', 'table'], '"schema"."table_serial_column_seq"'], - [new TableIdentifier('table', 'schema'), '"schema"."table_serial_column_seq"'], - ]; + $sequence1 = new SequenceFeature('col_1', 'seq_1'); + $this->assertEquals($sequence1->lastSequenceId('col_2'), null, 'Sequence should not react to foreign column name'); + $this->assertEquals($sequence1->nextSequenceId('col_2'), null, 'Sequence should not react to foreign column name'); } } From 21c45b717ec1b33b7f9910eb37bf9675a0f0d49f Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Sun, 25 Dec 2016 14:07:08 -0500 Subject: [PATCH 17/18] #186 Update TableGateway table mock to supply needed platform and adapter objects --- test/TableGateway/Feature/FeatureSetTest.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/TableGateway/Feature/FeatureSetTest.php b/test/TableGateway/Feature/FeatureSetTest.php index 38083c3578..a24918a683 100644 --- a/test/TableGateway/Feature/FeatureSetTest.php +++ b/test/TableGateway/Feature/FeatureSetTest.php @@ -12,12 +12,13 @@ namespace ZendTest\Db\TableGateway\Feature; use ReflectionClass; +use Zend\Db\Metadata\Object\ConstraintObject; use Zend\Db\TableGateway\Feature\AbstractFeature; use Zend\Db\TableGateway\Feature\FeatureSet; use Zend\Db\TableGateway\Feature\MasterSlaveFeature; -use Zend\Db\TableGateway\Feature\SequenceFeature; use Zend\Db\TableGateway\Feature\MetadataFeature; -use Zend\Db\Metadata\Object\ConstraintObject; +use Zend\Db\TableGateway\Feature\SequenceFeature; +use ZendTest\Db\TestAsset\TrustingPostgresqlPlatform; class FeatureSetTest extends \PHPUnit_Framework_TestCase { @@ -221,17 +222,16 @@ private function getMockSequence($columnName, $sequenceName, $expectedCurrVal) ->method('execute') ->will($this->returnValue($resultMock)); - $adapterMock = $this->getMockBuilder('Zend\Db\Adapter\Adapter') - ->disableOriginalConstructor() - ->getMock(); + $adapterMock = $this->getMock('Zend\Db\Adapter\Adapter', ['getPlatform', 'createStatement'], [], '', false); + $adapterMock->expects($this->any()) + ->method('getPlatform') + ->will($this->returnValue(new TrustingPostgresqlPlatform())); $adapterMock->expects($this->any()) ->method('getPlatform')->will($this->returnValue($platformMock)); $adapterMock->expects($this->any()) ->method('createStatement')->will($this->returnValue($statementMock)); - $tableGatewayMock = $this->getMockBuilder('Zend\Db\TableGateway\AbstractTableGateway') - ->disableOriginalConstructor() - ->getMock(); + $tableGatewayMock = $this->getMockForAbstractClass('Zend\Db\TableGateway\TableGateway', ['table', $adapterMock], '', true); $reflectionClass = new ReflectionClass('Zend\Db\TableGateway\AbstractTableGateway'); $reflectionProperty = $reflectionClass->getProperty('adapter'); From 8fb1ab19321692bc997aa6ee149d05747eba01c7 Mon Sep 17 00:00:00 2001 From: Sasha Alex Romanenko Date: Sun, 25 Dec 2016 14:16:59 -0500 Subject: [PATCH 18/18] #165 leftover docs from old query --- doc/book/table-gateway.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/book/table-gateway.md b/doc/book/table-gateway.md index 8814b2078b..9dd2bf6564 100644 --- a/doc/book/table-gateway.md +++ b/doc/book/table-gateway.md @@ -296,7 +296,7 @@ There are a number of features built-in and shipped with zend-db: pg_get_serial_sequence('artist', 'id'); ``` - take note of what `nextval` is reading from, and add it to `SequenceFeature` constructor. + and add it to `SequenceFeature` constructor. There could be complex business rules needing multiple sequences in a table. `TableGateway` can have multiple sequences added in an array: