[DBAL-278] Implement OCI8Connection::lastInsertId() when a sequence is passed.

lib/Doctrine/DBAL/Driver/OCI8/OCI8Connection.php

@@ -19,6 +19,8 @@
 
 namespace Doctrine\DBAL\Driver\OCI8;
 
+use Doctrine\DBAL\Platforms\OraclePlatform;
+
 /**
  * OCI8 implementation of the Connection interface.
  *
@@ -26,9 +28,15 @@ namespace Doctrine\DBAL\Driver\OCI8;
  */
 class OCI8Connection implements \Doctrine\DBAL\Driver\Connection
 {
-    protected $_dbh;
+    /**
+     * @var resource
+     */
+    protected $dbh;
 
-    protected $_executeMode = OCI_COMMIT_ON_SUCCESS;
+    /**
+     * @var int
+     */
+    protected $executeMode = OCI_COMMIT_ON_SUCCESS;
 
     /**
      * Create a Connection to an Oracle Database using oci8 extension.
@@ -43,11 +51,11 @@ class OCI8Connection implements \Doctrine\DBAL\Driver\Connection
             define('OCI_NO_AUTO_COMMIT', 0);
         }
 
-        $this->_dbh = $persistent
+        $this->dbh = $persistent
             ? @oci_pconnect($username, $password, $db, $charset, $sessionMode)
             : @oci_connect($username, $password, $db, $charset, $sessionMode);
 
-        if ( ! $this->_dbh) {
+        if ( ! $this->dbh) {
             throw OCI8Exception::fromErrorInfo(oci_error());
         }
     }
@@ -60,7 +68,7 @@ class OCI8Connection implements \Doctrine\DBAL\Driver\Connection
      */
     public function prepare($prepareString)
     {
-        return new OCI8Statement($this->_dbh, $prepareString, $this);
+        return new OCI8Statement($this->dbh, $prepareString, $this);
     }
 
     /**
@@ -105,9 +113,26 @@ class OCI8Connection implements \Doctrine\DBAL\Driver\Connection
         return $stmt->rowCount();
     }
 
+    /**
+     * {@inheritDoc}
+     */
     public function lastInsertId($name = null)
     {
-        //TODO: throw exception or support sequences?
+        if ($name === null) {
+            return false;
+        }
+
+        OraclePlatform::assertValidIdentifier($name);
+
+        $sql    = 'SELECT ' . $name . '.CURRVAL FROM DUAL';
+        $stmt   = $this->query($sql);
+        $result = $stmt->fetch(\PDO::FETCH_ASSOC);
+
+        if ($result === false || !isset($result['CURRVAL'])) {
+            throw new OCI8Exception("lastInsertId failed: Query was executed but no result was returned.");
+        }
+
+        return (int) $result['CURRVAL'];
     }
 
     /**
@@ -115,7 +140,7 @@ class OCI8Connection implements \Doctrine\DBAL\Driver\Connection
      */
     public function getExecuteMode()
     {
-        return $this->_executeMode;
+        return $this->executeMode;
     }
 
     /**
@@ -129,7 +154,7 @@ class OCI8Connection implements \Doctrine\DBAL\Driver\Connection
      */
     public function beginTransaction()
     {
-        $this->_executeMode = OCI_NO_AUTO_COMMIT;
+        $this->executeMode = OCI_NO_AUTO_COMMIT;
         return true;
     }
 
@@ -139,10 +164,10 @@ class OCI8Connection implements \Doctrine\DBAL\Driver\Connection
      */
     public function commit()
     {
-        if (!oci_commit($this->_dbh)) {
+        if (!oci_commit($this->dbh)) {
             throw OCI8Exception::fromErrorInfo($this->errorInfo());
         }
-        $this->_executeMode = OCI_COMMIT_ON_SUCCESS;
+        $this->executeMode = OCI_COMMIT_ON_SUCCESS;
         return true;
     }
 
@@ -152,16 +177,16 @@ class OCI8Connection implements \Doctrine\DBAL\Driver\Connection
      */
     public function rollBack()
     {
-        if (!oci_rollback($this->_dbh)) {
+        if (!oci_rollback($this->dbh)) {
             throw OCI8Exception::fromErrorInfo($this->errorInfo());
         }
-        $this->_executeMode = OCI_COMMIT_ON_SUCCESS;
+        $this->executeMode = OCI_COMMIT_ON_SUCCESS;
         return true;
     }
 
     public function errorCode()
     {
-        $error = oci_error($this->_dbh);
+        $error = oci_error($this->dbh);
         if ($error !== false) {
             $error = $error['code'];
         }
@@ -170,6 +195,6 @@ class OCI8Connection implements \Doctrine\DBAL\Driver\Connection
 
     public function errorInfo()
     {
-        return oci_error($this->_dbh);
+        return oci_error($this->dbh);
     }
 }

lib/Doctrine/DBAL/Platforms/OraclePlatform.php

@@ -20,6 +20,7 @@
 namespace Doctrine\DBAL\Platforms;
 
 use Doctrine\DBAL\Schema\TableDiff;
+use Doctrine\DBAL\DBALException;
 
 /**
  * OraclePlatform.
@@ -31,6 +32,20 @@ use Doctrine\DBAL\Schema\TableDiff;
  */
 class OraclePlatform extends AbstractPlatform
 {
+    /**
+     * Assertion for Oracle identifiers
+     *
+     * @link http://docs.oracle.com/cd/B19306_01/server.102/b14200/sql_elements008.htm
+     * @param string
+     * @throws DBALException
+     */
+    static public function assertValidIdentifier($identifier)
+    {
+        if ( ! preg_match('(^(([a-zA-Z]{1}[a-zA-Z0-9_$#]{0,})|("[^"]+"))$)', $identifier)) {
+            throw new DBALException("Invalid Oracle identifier");
+        }
+    }
+
     /**
      * return string to call a function to get a substring inside an SQL statement
      *

tests/Doctrine/Tests/DBAL/Functional/WriteTest.php

@@ -146,4 +146,35 @@ class WriteTest extends \Doctrine\Tests\DbalFunctionalTestCase
         $this->assertNotNull($num, "LastInsertId() should not be null.");
         $this->assertTrue($num > 0, "LastInsertId() should be non-negative number.");
     }
+
+    public function testLastInsertIdSequence()
+    {
+        if ( ! $this->_conn->getDatabasePlatform()->supportsSequences()) {
+            $this->markTestSkipped('Test only works on platforms with sequences.');
+        }
+
+        $sequence = new \Doctrine\DBAL\Schema\Sequence('write_table_seq');
+        try {
+            $this->_conn->getSchemaManager()->dropSequence($sequence);
+        } catch(\Exception $e) {}
+        $this->_conn->getSchemaManager()->createSequence($sequence);
+
+        $stmt = $this->_conn->query($this->_conn->getDatabasePlatform()->getSequenceNextValSQL('write_table_seq'));
+        $nextSequenceVal = $stmt->fetchColumn();
+
+        $lastInsertId = $this->_conn->lastInsertId('write_table_seq');
+
+        $this->assertTrue($lastInsertId > 0);
+        $this->assertEquals($nextSequenceVal, $lastInsertId);
+    }
+
+    public function testLastInsertIdNoSequenceGiven()
+    {
+        if ( ! $this->_conn->getDatabasePlatform()->supportsSequences()) {
+            $this->markTestSkipped('Test only works on platforms with sequences.');
+        }
+
+        $this->assertFalse($this->_conn->lastInsertId( null ));
+
+    }
 }

tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php

@@ -9,6 +9,51 @@ require_once __DIR__ . '/../../TestInit.php';
 
 class OraclePlatformTest extends AbstractPlatformTestCase
 {
+    static public function dataValidIdentifiers()
+    {
+        return array(
+            array('a'),
+            array('foo'),
+            array('Foo'),
+            array('Foo123'),
+            array('Foo#bar_baz$'),
+            array('"a"'),
+            array('"1"'),
+            array('"foo_bar"'),
+            array('"@$%&!"'),
+        );
+    }
+
+    /**
+     * @dataProvider dataValidIdentifiers
+     */
+    public function testValidIdentifiers($identifier)
+    {
+        $platform = $this->createPlatform();
+        $platform->assertValidIdentifier($identifier);
+    }
+
+    static public function dataInvalidIdentifiers()
+    {
+        return array(
+            array('1'),
+            array('abc&'),
+            array('abc-def'),
+            array('"'),
+            array('"foo"bar"'),
+        );
+    }
+
+    /**
+     * @dataProvider dataInvalidIdentifiers
+     */
+    public function testInvalidIdentifiers($identifier)
+    {
+        $this->setExpectedException('Doctrine\DBAL\DBALException');
+        $platform = $this->createPlatform();
+        $platform->assertValidIdentifier($identifier);
+    }
+
     public function createPlatform()
     {
         return new OraclePlatform;
@@ -224,4 +269,4 @@ class OraclePlatformTest extends AbstractPlatformTestCase
                 $this->getBitAndComparisonExpressionSql($value1, $value2)
                 . '+' . $value2 . ')';
     }
-}
+}
\ No newline at end of file