Bug Report

CI_Loader::database() assumes $CI->db is in use after a returned DB load
Date:	05/11/2007	Severity:	Minor
Status:	Resolved	Reporter:	JAAulde
Version:	1.5.3
Keywords:	Libraries, Loader Class

Description

When loading a database and not having that object returned, CI_Loader::database() sets that database interaction object to the variable $CI->db. The next time CI_Loader::database() is called, if the object is not being requested for return to another variable, the method stops to avoid overwriting $CI->db. This is fine.

However, the check for a previously loaded database object is flawed in that if I have loaded a database object previously but had it returned, and then try to load a database implicitly to $CI->db, the method stops. This is because the check is looking for the existence of the class, ‘CI_DB’ rather than the presence of a previously set $CI->db.

I believe the code should be adjusted in that it would check for whether the object is to be returned and whether $CI->db is set before it makes the decision to stop.

Code Sample

$specificDB = $this->load->database('specific');
$this->load->database(); 

Expected Result

//Line one should return a database interaction object to $speciifcDB with the DB configuration options for $db[‘specific’]
//Line two should result in a database interaction object assigned to $CI->db with the DB configuration from the $active_group DB config

Actual Result

//Line one returns the specific DB as expected
//Line two does nothing because CI_Loader::database() sees that the CI_DB class already exists and returns false.

Comment on Bug Report

Page 1 of 1 pages

Posted by: JAAulde on 11 May 2007 9:26am
	the biggest problem with this bug, and the reason I only marked it as minor, is if a hook is loading a DB object into a library or something early in the system load. This is how I ran into the problem. I got around the problem by running an extended CI_Loader object which modifies CI_Loader::database() as follows: /** * Database Loader * * @access public * @param string the DB credentials * @param bool whether to return the DB object * @param bool whether to enable active record (this allows us to override the config setting) * @return object / function database($params = '', $return = FALSE, $active_record = FALSE) { //Do we need to load up the DB library? if (!class_exists('CI_DB')) { require_once(BASEPATH.'database/DB'.EXT); } // Grab the super object $CI =& get_instance(); // If the CI db database interaction object is already set and we are not returning our database object on this call, we need to stop if (isset($CI->db) AND $return == FALSE AND $active_record == FALSE) { return FALSE; } if ($return === TRUE) { return DB($params, $active_record); } // Initialize the db variable. Needed to prevent // reference errors with some configurations $CI->db = ''; // Load the DB class $CI->db =& DB($params, $active_record); // Assign the DB object to any existing models $this->_ci_assign_to_models(); } I am not 100% sure about this code, though, because I really don’t know what the check for $active_record == FALSE* has to do with anything. It was in the original method, so I left it here for now. However, I would think this would be a better version: /** * Database Loader * * @access public * @param string the DB credentials * @param bool whether to return the DB object * @param bool whether to enable active record (this allows us to override the config setting) * @return object / function database($params = '', $return = FALSE, $active_record = FALSE) { //Do we need to load up the DB library? if (!class_exists('CI_DB')) { require_once(BASEPATH.'database/DB'.EXT); } // Grab the super object $CI =& get_instance(); // If the CI db database interaction object is already set and we are not returning our database object on this call, we need to stop if (isset($CI->db) AND $return == FALSE) { return FALSE; } if ($return === TRUE) { return DB($params, $active_record); } // Initialize the db variable. Needed to prevent // reference errors with some configurations $CI->db = ''; // Load the DB class $CI->db =& DB($params, $active_record); // Assign the DB object to any existing models $this->_ci_assign_to_models(); } Notice the removed check for $active_record == FALSE* in the conditional which is making sure $CI->db doesn’t get overwritten. —Jim
Posted by: JAAulde on 11 May 2007 2:00pm
	I typo’d my example code in the original report. It should be `$specificDB = $this->load->database('specific',TRUE); $this->load->database();`