Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Oracle DBO Updates addressing persistModel and sequencing issues #123

Closed
wants to merge 3 commits into from

3 participants

Ben Murden José Lorenzo Rodríguez Mark Story
Ben Murden

These updates should address the ticket I made before #1666.

The _persist() method of Object is used to resolve the issue of sequence names not being available when persistModel is true in the controller. This was particularly a problem in the method lastInsertId(), which would only be passed a string of the table name, and would therefore have no other way of knowing the sequence name, which can be modified using a table prefix or through a model property. While caching is enabled, this update will resolve that issue, and when caching is disabled, so is persistModel, so this is no longer an issue.

Sequences now correctly take table prefixes.

A 'schema' key can be defined in the database configuration to allow faster enumeration of database tables by reducing the set to the relevant tables owner.

I've also changed the Oracle unit tests to be more inline with other DBOs, while including tests for these updates and some of the existing functionality.

Let me know if you think anything could be done better here.

Thanks

Ben

TigerDX added some commits
Ben Murden TigerDX Added logic for use with defined schema in Oracle
A "schema" key can be used in the database config when you want to access a different schema without having to always use the dot syntax. This also improves efficiency when you only use tables from one schema.
fe4c193
Ben Murden TigerDX Added tablePrefix to described sequences
This avoids conflicts with separate sequences used in test suite fixtures, for example.
f7f097b
Ben Murden TigerDX Oracle DBO bugs fixed & added features + tests
This update addresses an issue with using persistModel and sequences in Oracle. The issue is described in ticket #1666 http://cakephp.lighthouseapp.com/projects/42648-cakephp/tickets/1666

In the same ticket, the described sequence referencing bug has been addressed here.

A Schema switch has been added to the DB config. When the 'schema' key is defined, the DBO will use that to limit the table enumeration down to only those owned by the defined schema. This can greatly reduce table enumeration time, relative to the size of the database.

Added unit tests for these new features and reconfigured the Oracle test class to be consistent with other DBO unit tests. Some existing functionality will also now be tested.
4363637
Ben Murden

I can break this down into several smaller commits if that would be easier. Probably a sensible thing to do, huh?

José Lorenzo Rodríguez
Owner

Yeah, it is a hard to read pull request. That is why no one has already looked at it..

Mark Story
Owner

Thanks for the patch, but given the complexity and our inability to verify the fixes I'm going to err on the side of safety and not merge this.

Mark Story markstory closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 5, 2011
  1. Ben Murden

    Added logic for use with defined schema in Oracle

    TigerDX authored
    A "schema" key can be used in the database config when you want to access a different schema without having to always use the dot syntax. This also improves efficiency when you only use tables from one schema.
Commits on Apr 6, 2011
  1. Ben Murden

    Added tablePrefix to described sequences

    TigerDX authored
    This avoids conflicts with separate sequences used in test suite fixtures, for example.
Commits on Jun 20, 2011
  1. Ben Murden

    Oracle DBO bugs fixed & added features + tests

    TigerDX authored
    This update addresses an issue with using persistModel and sequences in Oracle. The issue is described in ticket #1666 http://cakephp.lighthouseapp.com/projects/42648-cakephp/tickets/1666
    
    In the same ticket, the described sequence referencing bug has been addressed here.
    
    A Schema switch has been added to the DB config. When the 'schema' key is defined, the DBO will use that to limit the table enumeration down to only those owned by the defined schema. This can greatly reduce table enumeration time, relative to the size of the database.
    
    Added unit tests for these new features and reconfigured the Oracle test class to be consistent with other DBO unit tests. Some existing functionality will also now be tested.
This page is out of date. Refresh to see the latest.
99 cake/libs/model/datasources/dbo/dbo_oracle.php
View
@@ -140,7 +140,7 @@ class DboOracle extends DboSource {
var $_error;
/**
- * Base configuration settings for MySQL driver
+ * Base configuration settings for Oracle driver
*
* @var array
*/
@@ -187,6 +187,9 @@ function connect() {
if (!empty($config['nls_comp'])) {
$this->execute('ALTER SESSION SET NLS_COMP='.$config['nls_comp']);
}
+ if (!empty($config['schema'])) {
+ $this->execute('ALTER SESSION SET CURRENT_SCHEMA='.$config['schema']);
+ }
$this->execute("ALTER SESSION SET NLS_DATE_FORMAT='YYYY-MM-DD HH24:MI:SS'");
} else {
$this->connected = false;
@@ -456,6 +459,76 @@ function createTrigger($table) {
}
/**
+ * Cache the sequence details
+ *
+ * @param object $model
+ * @access private
+ * @return mixed
+ *
+ */
+ function _cacheSequence(&$model) {
+ if ($this->cacheSources === false) {
+ return null;
+ }
+
+ $cache = null;
+
+ $table = $this->fullTableName($model, false);
+ $key = ConnectionManager::getSourceName($this) . '_' . $table . '_seq';
+ $cached = $this->_persist($key, null, $cache);
+
+ if ($cached === false) {
+ $cache = $this->_getSequenceName($model);
+ $this->_persist($key, true, $cache);
+ } else {
+ $this->_persist($key, true, $cache);
+ $cache = $this->{$key};
+ }
+
+ return $cache;
+ }
+
+/**
+ * Get cached sequence details from a table name only.
+ * Required for proper functionality of lastInsertId when models are cached.
+ *
+ * @param String table
+ * @access private
+ * @return mixed
+ */
+ function _getCachedSequence($table) {
+ if ($this->cacheSources === false) {
+ return null;
+ }
+
+ $cache = null;
+
+ $key = ConnectionManager::getSourceName($this) . '_' . $table . '_seq';
+ $this->_persist($key, true, $cache);
+ $cache = $this->{$key};
+
+ return $cache;
+ }
+
+/**
+ * Get the sequence name for a model
+ *
+ * @param object model
+ * @access private
+ * @return String
+ *
+ */
+ function _getSequenceName(&$model) {
+ if (!empty($model->sequence)) {
+ $seq = $model->sequence;
+ } elseif (!empty($model->table)) {
+ $seq = $this->fullTableName($model, false) . '_seq';
+ }
+
+ return $seq;
+ }
+
+/**
* Returns an array of tables in the database. If there are no tables, an error is
* raised and the application exits.
*
@@ -463,11 +536,17 @@ function createTrigger($table) {
* @access public
*/
function listSources() {
+ $config = $this->config;
$cache = parent::listSources();
if ($cache != null) {
return $cache;
}
- $sql = 'SELECT view_name AS name FROM all_views UNION SELECT table_name AS name FROM all_tables';
+
+ if (!empty($config['schema'])) {
+ $sql = 'SELECT view_name AS name FROM all_views WHERE owner = \'' . $config['schema'] . '\' UNION SELECT table_name AS name FROM all_tables WHERE owner = \'' . $config['schema'] . '\'';
+ } else {
+ $sql = 'SELECT view_name AS name FROM all_views UNION SELECT table_name AS name FROM all_tables';
+ }
if (!$this->execute($sql)) {
return false;
@@ -490,13 +569,9 @@ function listSources() {
*/
function describe(&$model) {
$table = $this->fullTableName($model, false);
-
- if (!empty($model->sequence)) {
- $this->_sequenceMap[$table] = $model->sequence;
- } elseif (!empty($model->table)) {
- $this->_sequenceMap[$table] = $model->table . '_seq';
- }
-
+
+ $this->_sequenceMap[$table] = $this->_cacheSequence($model);
+
$cache = parent::describe($model);
if ($cache != null) {
@@ -897,7 +972,11 @@ function value($data, $column = null, $safe = false) {
* @access public
*/
function lastInsertId($source) {
- $sequence = $this->_sequenceMap[$source];
+ if (isset($this->_sequenceMap[$source])) {
+ $sequence = $this->_sequenceMap[$source];
+ } else {
+ $sequence = $this->_getCachedSequence($source);
+ }
$sql = "SELECT $sequence.currval FROM dual";
if (!$this->execute($sql)) {
262 cake/tests/cases/libs/model/datasources/dbo/dbo_oracle.test.php
View
@@ -17,11 +17,144 @@
* @since CakePHP(tm) v 1.2.0
* @license MIT License (http://www.opensource.org/licenses/mit-license.php)
*/
-if (!defined('CAKEPHP_UNIT_TEST_EXECUTION')) {
- define('CAKEPHP_UNIT_TEST_EXECUTION', 1);
+
+App::import('Core', array('Model', 'DataSource', 'DboSource', 'DboOracle'));
+
+/**
+ * DboOracleTestDb class
+ *
+ * @package cake
+ * @subpackage cake.tests.cases.libs.model.datasources
+ */
+class DboOracleTestDb extends DboOracle {
+
+/**
+ * simulated property
+ *
+ * @var array
+ * @access public
+ */
+ var $simulated = array();
+
+/**
+ * execute method
+ *
+ * @param mixed $sql
+ * @access protected
+ * @return void
+ */
+ function _execute($sql) {
+ $this->simulated[] = $sql;
+ $this->_statementId = null;
+ return null;
+ }
+
+/**
+ * getLastQuery method
+ *
+ * @access public
+ * @return void
+ */
+ function getLastQuery() {
+ return $this->simulated[count($this->simulated) - 1];
+ }
+
+/**
+ * getHistoricalQuery method
+ * Get a query from the passed steps $ago.
+ * E.g. getHistoricalQuery(1) is the same as getLastQuery().
+ * getHistoricalQuery(3) is the query executed 3 times ago.
+ *
+ * @param int $ago
+ * @access public
+ * @return String
+ */
+ function getHistoricalQuery($ago) {
+ return $this->simulated[count($this->simulated) - $ago];
+ }
+}
+
+/**
+ * OracleTestModel class
+ *
+ * @package cake
+ * @subpackage cake.tests.cases.libs.model.datasources
+ */
+class OracleTestModel extends Model {
+
+/**
+ * name property
+ *
+ * @var string 'OracleTestModel'
+ * @access public
+ */
+ var $name = 'OracleTestModel';
+
+/**
+ * useTable property
+ *
+ * @var bool false
+ * @access public
+ */
+ var $useTable = false;
+
+/**
+ * find method
+ *
+ * @param mixed $conditions
+ * @param mixed $fields
+ * @param mixed $order
+ * @param mixed $recursive
+ * @access public
+ * @return void
+ */
+ function find($conditions = null, $fields = null, $order = null, $recursive = null) {
+ return $conditions;
+ }
+
+/**
+ * findAll method
+ *
+ * @param mixed $conditions
+ * @param mixed $fields
+ * @param mixed $order
+ * @param mixed $recursive
+ * @access public
+ * @return void
+ */
+ function findAll($conditions = null, $fields = null, $order = null, $recursive = null) {
+ return $conditions;
+ }
+
+/**
+ * schema method
+ *
+ * @access public
+ * @return void
+ */
+ function schema() {
+ return array(
+ 'id' => array('type' => 'integer', 'null' => '', 'default' => '', 'length' => '8'),
+ 'client_id' => array('type' => 'integer', 'null' => '', 'default' => '0', 'length' => '11'),
+ 'name' => array('type' => 'string', 'null' => '', 'default' => '', 'length' => '255'),
+ 'login' => array('type' => 'string', 'null' => '', 'default' => '', 'length' => '255'),
+ 'passwd' => array('type' => 'string', 'null' => '1', 'default' => '', 'length' => '255'),
+ 'addr_1' => array('type' => 'string', 'null' => '1', 'default' => '', 'length' => '255'),
+ 'addr_2' => array('type' => 'string', 'null' => '1', 'default' => '', 'length' => '25'),
+ 'zip_code' => array('type' => 'string', 'null' => '1', 'default' => '', 'length' => '155'),
+ 'city' => array('type' => 'string', 'null' => '1', 'default' => '', 'length' => '155'),
+ 'country' => array('type' => 'string', 'null' => '1', 'default' => '', 'length' => '155'),
+ 'phone' => array('type' => 'string', 'null' => '1', 'default' => '', 'length' => '155'),
+ 'fax' => array('type' => 'string', 'null' => '1', 'default' => '', 'length' => '155'),
+ 'url' => array('type' => 'string', 'null' => '1', 'default' => '', 'length' => '255'),
+ 'email' => array('type' => 'string', 'null' => '1', 'default' => '', 'length' => '155'),
+ 'comments' => array('type' => 'text', 'null' => '1', 'default' => '', 'length' => ''),
+ 'last_login'=> array('type' => 'datetime', 'null' => '1', 'default' => '', 'length' => ''),
+ 'created' => array('type' => 'date', 'null' => '1', 'default' => '', 'length' => ''),
+ 'updated' => array('type' => 'datetime', 'null' => '1', 'default' => '', 'length' => null)
+ );
+ }
}
-require_once LIBS . 'model' . DS . 'datasources' . DS . 'dbo_source.php';
-require_once LIBS . 'model' . DS . 'datasources' . DS . 'dbo' . DS . 'dbo_oracle.php';
/**
* DboOracleTest class
@@ -37,26 +170,137 @@ class DboOracleTest extends CakeTestCase {
var $fixtures = array('core.oracle_user');
/**
+ * Actual DB connection used in testing
+ *
+ * @var DboSource
+ * @access public
+ */
+ var $db = null;
+
+/**
+ * Simulated DB connection used in testing
+ *
+ * @var DboSource
+ * @access public
+ */
+ var $simDb = null;
+
+/**
+ * Testing model
+ *
+ * @var Model
+ * @access public
+ */
+ var $model = null;
+
+/**
+ * Set up test suite database connection
+ *
+ * @access public
+ */
+ function startTest() {
+ $this->_initDb();
+ }
+
+/**
* setup method
*
* @access public
* @return void
*/
function setUp() {
- $this->_initDb();
+ Configure::write('Cache.disable', true);
+ $this->startTest();
+ $this->db =& ConnectionManager::getDataSource('test_suite');
+ $this->simDb = new DboOracleTestDb($this->db->config, false);
+ $this->model = new OracleTestModel();
}
/**
+ * Tears down the Dbo class instance
+ *
+ * @access public
+ */
+ function tearDown() {
+ Configure::write('Cache.disable', false);
+ unset($this->db);
+ unset($this->simDb);
+ unset($this->model);
+ }
+
+/**
* skip method
*
* @access public
* @return void
*/
- function skip() {
- $this->_initDb();
- $this->skipUnless($this->db->config['driver'] == 'oracle', '%s Oracle connection not available');
- }
+ function skip() {
+ $this->_initDb();
+ $this->skipUnless($this->db->config['driver'] == 'oracle', '%s Oracle connection not available');
+ }
+
+/**
+ * testConnect method
+ *
+ * @access public
+ * @return void
+ */
+ function testConnect() {
+ $result = $this->db->connect();
+ $this->assertTrue($result);
+
+ $this->simDb->config['schema'] = 'SAIBOT';
+ $this->simDb->connect();
+ $result = $this->simDb->getLastQuery();
+ $expected = "ALTER SESSION SET NLS_DATE_FORMAT='YYYY-MM-DD HH24:MI:SS'";
+ $this->assertEqual($expected, $result);
+
+ $result = $this->simDb->getHistoricalQuery(2);
+ $expected = 'ALTER SESSION SET CURRENT_SCHEMA=SAIBOT';
+ $this->assertEqual($expected, $result);
+ }
+
+/**
+ * testDescribe method
+ *
+ * @access public
+ * @return void
+ */
+ function testDescribe() {
+ $this->model->table = 'test_table';
+ $this->simDb->describe($this->model);
+ $expected = 'test_table_seq';
+ $result = $this->simDb->_sequenceMap['test_table'];
+ $this->assertEqual($expected, $result);
+
+ $this->model->tablePrefix = 'ultimate_';
+ $this->simDb->describe($this->model);
+ $expected = 'ultimate_test_table_seq';
+ $result = $this->simDb->_sequenceMap['ultimate_test_table'];
+ $this->assertEqual($expected, $result);
+ $this->model->tablePrefix = null;
+ $this->model->sequence = 'test_sequence_dude';
+ $this->simDb->describe($this->model);
+ $expected = 'test_sequence_dude';
+ $result = $this->simDb->_sequenceMap['test_table'];
+ $this->assertEqual($expected, $result);
+ }
+
+/**
+ * testLastInsertId method
+ *
+ * @access public
+ * @return void
+ */
+ function testLastInsertId() {
+ $this->model->table = 'test_table';
+ $this->simDb->describe($this->model);
+ $this->simDb->lastInsertId('test_table');
+ $expected = "SELECT test_table_seq.currval FROM dual";
+ $result = $this->simDb->getLastQuery();
+ $this->assertEqual($expected, $result);
+ }
/**
* testLastErrorStatement method
*
Something went wrong with that request. Please try again.