Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Driver extension enhancement #1787

Closed
wants to merge 11 commits into from

4 participants

@dchill42

Followup to #353, and parallel fix to #1769.

Enables mix-and-match extension of Driver library and child classes. Extend library without requiring child extensions and vice-versa.

Also allows legacy Session library extensions to be applied to new Session driver without relocation to Session subdirectory.

Session and Cache drivers updated to work with changes to valid_drivers list, which now lists child class names without parent class name included. Driver child class and file names still require parent name prefix.

dchill42 added some commits
@dchill42 dchill42 Started Driver extension changes
Signed-off-by: dchill42 <dchill42@gmail.com>
28bee3d
@dchill42 dchill42 Finished Driver extension changes
Signed-off-by: dchill42 <dchill42@gmail.com>
b9981f6
@nickl-

Round about here do something like this instead:

<?php

            if (! class_exists('CI_Driver_Library'))
            {
                // We aren't instantiating an object here, just making the base class available
                require BASEPATH.'libraries/Driver.php';
            }

Which is where it's going to matter and helps to make it understandable because the dependents are being included in the following two lines.

True - with the changes to subdirectory handling, the Driver base loading up at L914 won't work anymore. I'll have to move that - just not to where you've indicated.

@nickl-

Is there any reason why include is not sufficient?
If yes rather do if class_exists as it's got cache advantages (ie APC) which *_once misses out on.

Agreed - include is sufficient. include_once was the existing call, but it could be updated while we're here.

@nickl-

Something must be wrong, why are we going through the whole ric-ma-role again the class was included. Rather find out why it's doing double runs then to go the safe route here.

There are two different operations here. The first pass loads an extension and the base class, whereas the second pass is for identifying the regular library in the package paths if an extension wasn't found. This is how the loader works.

@nickl-

This is bad for cherry picking too much happening in one commit.

Can this not be don in 4 maybe even 8 separate commits?

I lost the plot when the cache changes started =(

@dchill42

I really don't think the number of commits is important. Just look at the diff on the pull request if you want to inspect the code changes. They are all interdependent and necessary to make this solution work.

Cache is the other library using the Driver class at this point (in addition to Session). I explained the change to Cache in the introductory comments on this pull request.

@dchill42

@narfbg and @alexbilbie - Here's my rewrite of #1769, now with Driver unit tests. I'd like to add to the Loader unit test to cover the changes there, but I'd prefer to do that after #1744 is merged, which sets the stage with basic CI_Loader::driver testing.

system/core/Loader.php
((5 lines not shown))
- // Check for match and driver base class
- if (strtolower(trim($subdir, '/')) == strtolower($class) && ! class_exists('CI_Driver_Library'))
- {
- // We aren't instantiating an object here, just making the base class available
- require BASEPATH.'libraries/Driver.php';
- }
+ // Establish subdirectories to try - if one was specified, just use that
+ $trysubs = array($subdir);
+ if ($subdir == '')
+ {
+ // Also check subdirectories matching class name
+ $dir = strtolower($class).'/';
+ $trysubs[] = ucfirst($dir);
+ $trysubs[] = $dir;
@narfbg Owner
narfbg added a note

I'm not really sure what this pull request does at all, but as a start ... indentation on this like looks weird. Additionaly - this particular element should not be added on Windows servers as they would ignore the case. Try this, with some comment added above it:

if (DIRECTORY_SEPARATOR !== '\\')
{
    $trysubs[] = $dir;
}

The change here handles three things - the lowercase "last ditch" that used to be below, the ucfirst "last, last ditch" that was added after that to pick up drivers with a library() call, and now the loading of an extension for a driver library from the main libs dir. Example - Session is now in libraries/Session/Session.php, this would allow an extension as libraries/MY_Session.php. The point is for extensions of the old Session library to be applied to the new Session driver after an upgrade without requiring relocation (drop-in replacement). The flexibility would apply to any driver library.

I'll check the indentation, but I know github is funny about it sometimes - the same tabs result in a different spacing in the diffs here. I suspect you mean L921 above, which is a classic example of that.

I agree that the case-sensitive variation is not needed on Windows, so the second addition is superfluous there. In the case of extension not found, that would skip an extra file_exists() test on that platform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
system/core/Loader.php
((66 lines not shown))
- $is_duplicate = TRUE;
- log_message('debug', $class.' class already loaded. Second attempt ignored.');
- return;
- }
+ if ( ! $found)
+ {
+ // No base matches extension - bail
+ $msg = 'Unable to load the requested class: '.$class;
+ log_message('error', $msg);
+ show_error($msg);
+ }
+
+ // Does this look like a driver?
+ $driversub = strtolower($class).'/';
+ if ((strtolower($dir) == $driversub || strtolower($basesub) == $driversub)
+ && ! class_exists('CI_Driver_Library'))
@narfbg Owner
narfbg added a note

Even though all of those changes are obviously related to the Driver library, including checks about it in another class is a bad practice and shouldn't be done.

Also, the style guide says that you must use OR instead of || and line 958 should be further indented. Using === instead of == would also be nice. :)

This bit of code used to be up in the driver() call, but was moved down here in #353 to support loading driver libraries with library(). With this change, that check to include the driver base classes needed to move a little deeper into _ci_load_class() for the cases when we find an extension without the subdirectory for a driver lib. In short, this code has always been in Loader, it's just been relocated.

I know the style guide says OR, but I swear I've been seeing trending the other direction. I had come to assume that was a convention that had been reversed. If OR is still the true standard, OR it shall be, and yes - the explicit comparison will be better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
system/core/Loader.php
((84 lines not shown))
+ require BASEPATH.'libraries/Driver.php';
+ }
+
+ // Safety: Was the class already loaded by a previous call?
+ if (in_array($subclass, $this->_ci_loaded_files))
+ {
+ // Before we deem this to be a duplicate request, let's see
+ // if a custom object name is being supplied. If so, we'll
+ // return a new instance of the object
+ if ( ! is_null($object_name))
+ {
+ $CI =& get_instance();
+ if ( ! isset($CI->$object_name))
+ {
+ return $this->_ci_init_class($class, config_item('subclass_prefix'), $params,
+ $object_name);
@narfbg Owner
narfbg added a note

Either put it on the line above, or put it more indentation and do the same to the rest of the method parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
system/core/Loader.php
((102 lines not shown))
- include_once($baseclass);
- include_once($subclass);
- $this->_ci_loaded_files[] = $subclass;
+ $is_duplicate = TRUE;
+ log_message('debug', $class.' class already loaded. Second attempt ignored.');
+ return;
+ }
+
+ include($baseclass);
+ include($subclass);
@narfbg Owner
narfbg added a note

require_once()

OK, I'll change this if you say so, but I don't see the point in require_once for these files. The Loader already tracks what's been loaded, and we've already verified both files exist. Am I missing some reason for having PHP do the extra work of require_once over include in this scenario?

@narfbg Owner
narfbg added a note

Simple - it is way better to leave to job to PHP instead of trying to track it yourself.

That's just it, though - Loader has always tracked loaded classes internally in order to recognize duplicate requests and either skip the include to instantiate another object under a different name, or gracefully return on true duplicates after providing a log message to inform the developer of the redundant call. If we remove that legacy logic and just rely on require_once, we will lose that courtesy. So, which way do we want to go - internal tracking or require_once? I don't see any point in doing both - it's just wasteful.

@narfbg Owner
narfbg added a note

Save library names in a property, leave PHP to handle file includes.

OK, last push on this - if you insist, I'll make those require_once() even though I disagree. The difference between include() and require_once() is making sure the file exists and hasn't already been included. If the library name isn't already in the property, we know the file hasn't been included yet. If we found the file in the current directory, we already know it exists. These are steps we have to take in our loading process. So, why have PHP repeat those same checks? It's redundant and inefficient. As I said above, if you insist on that redundancy, I'll make the change and shut up about it.

@narfbg Owner
narfbg added a note

My whole point is - I don't get the whole idea of why do we have to have a _ci_loaded_files array, fill it, check against it, etc. when we can just use include_once() or require_once(). Those functions WILL always be faster than whatever tracking we implement on our own.

On a side note - I don't really care if it is require or include.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
system/libraries/Driver.php
((13 lines not shown))
if ( ! isset($this->lib_name))
{
- $this->lib_name = get_class($this);
+ // Get library name without any prefix
+ $this->subclass_prefix = (string)$CI->config->item('subclass_prefix');
@narfbg Owner
narfbg added a note

Put spaces after type casts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@narfbg
Owner

@dchill42 Could you update this one?

Edit:

I don't know how appropriate it is to support legacy MY_Session implementations. Could you also say what your thoughts are on this?

@dchill42

Sure. I'm actually on the fence about the legacy MY_Session extensions. The reason I included code here to accept them is that I've made every other effort to ensure the Session driver is a fully backwards-compatible replacement for the former library. It seemed senseless to have one outstanding exception preventing us from saying "the default Session driver will work with your existing application with no changes required."

On the other hand, this is a major release and we do have a full changelog. It is not totally unreasonable to ask the (relatively few) developers who may have extended Session to move their extension into a subdirectory. It is also noteworthy that some of the undocumented non-API functions of the former library are gone. If an extension overloaded sess_update(), for example, it would no longer be called, as that function became _sess_update() in the driver. So, extensions probably need to be inspected for compatibility as we cannot absolutely guarantee their backward compatibility.

I guess I basically just talked myself into removing support for extensions outside the driver subdirectory and replacing it with a changelog note telling devs that MY_Session will have to be relocated and possibly retooled for the new driver.

@narfbg
Owner

I don't think that we should support the old extensions. Anybody that has made changes to how CI behaves (be it by directly changing the code or by extending it) should read the upgrade notes and adjust their changes. Plus, as you noted - significant changes that could break old extensions has been made to the library anyway.

Just focus it on having the ability to extend drivers.

@narfbg narfbg commented on the diff
system/core/Loader.php
@@ -9,7 +9,7 @@
* Licensed under the Open Software License version 3.0
*
* This source file is subject to the Open Software License (OSL 3.0) that is
- * bundled with this package in the files license.txt / license.rst. It is
@narfbg Owner
narfbg added a note

Revert those, I think we already discussed them on another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dchill42

Replaced by #2029

@dchill42 dchill42 closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 11, 2012
  1. @dchill42

    Started Driver extension changes

    dchill42 authored
    Signed-off-by: dchill42 <dchill42@gmail.com>
  2. @dchill42

    Finished Driver extension changes

    dchill42 authored
    Signed-off-by: dchill42 <dchill42@gmail.com>
Commits on Sep 12, 2012
  1. @dchill42

    Tweaked Driver base autoload and switched include_once to include

    dchill42 authored
    Signed-off-by: dchill42 <dchill42@gmail.com>
  2. @dchill42

    Added Driver unit test

    dchill42 authored
    Signed-off-by: dchill42 <dchill42@gmail.com>
  3. @dchill42
Commits on Sep 14, 2012
  1. @dchill42
  2. @dchill42

    Cleanup

    dchill42 authored
    Signed-off-by: dchill42 <dchill42@gmail.com>
Commits on Oct 9, 2012
  1. @dchill42
Commits on Nov 25, 2012
  1. @dchill42

    Merge branch 'develop' of git://github.com/EllisLab/CodeIgniter into …

    dchill42 authored
    …session
    
    Signed-off-by: dchill42 <dchill42@gmail.com>
  2. @dchill42
  3. @dchill42

    Removed support for driver extensions in libraries dir and fixed sess…

    dchill42 authored
    …ion unit test
    
    Signed-off-by: dchill42 <dchill42@gmail.com>
This page is out of date. Refresh to see the latest.
View
172 system/core/Loader.php
@@ -9,7 +9,7 @@
* Licensed under the Open Software License version 3.0
*
* This source file is subject to the Open Software License (OSL 3.0) that is
- * bundled with this package in the files license.txt / license.rst. It is
@narfbg Owner
narfbg added a note

Revert those, I think we already discussed them on another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ * bundled with this package in the files license.txt / license.rst. It is
* also available through the world wide web at this URL:
* http://opensource.org/licenses/OSL-3.0
* If you did not receive a copy of the license and are unable to obtain it
@@ -136,7 +136,7 @@ class CI_Loader {
*/
public function __construct()
{
- $this->_ci_ob_level = ob_get_level();
+ $this->_ci_ob_level = ob_get_level();
$this->_ci_library_paths = array(APPPATH, BASEPATH);
$this->_ci_helper_paths = array(APPPATH, BASEPATH);
$this->_ci_model_paths = array(APPPATH);
@@ -949,109 +949,137 @@ protected function _ci_load_class($class, $params = NULL, $object_name = NULL)
// Get the filename from the path
$class = substr($class, $last_slash);
+ }
- // Check for match and driver base class
- if (strtolower(trim($subdir, '/')) == strtolower($class) && ! class_exists('CI_Driver_Library'))
+ // Establish subdirectories to try - if one was specified, just use that
+ $trysubs = array($subdir);
+ if ($subdir === '')
+ {
+ // Also check subdirectories matching class name
+ // This allows us to find libraries in self-named subdirectories
+ // (like Drivers) and extensions for those in the main libraries dir
+ $dir = strtolower($class).'/';
+ $trysubs[] = ucfirst($dir);
+
+ // If we aren't case-insensitive, check lowercase too
+ if (DIRECTORY_SEPARATOR !== '\\')
{
- // We aren't instantiating an object here, just making the base class available
- require BASEPATH.'libraries/Driver.php';
+ $trysubs[] = $dir;
}
}
+ // Get the subclass prefix
+ $prefix = config_item('subclass_prefix');
+
// We'll test for both lowercase and capitalized versions of the file name
- foreach (array(ucfirst($class), strtolower($class)) as $class)
+ $cases = array(ucfirst($class));
+ if (DIRECTORY_SEPARATOR !== '\\')
+ {
+ // Case-sensitive - try lowercase too
+ $cases[] = strtolower($class);
+ }
+ foreach ($cases as $class)
{
- $subclass = APPPATH.'libraries/'.$subdir.config_item('subclass_prefix').$class.'.php';
-
// Is this a class extension request?
- if (file_exists($subclass))
+ foreach($trysubs as $dir)
{
- $baseclass = BASEPATH.'libraries/'.ucfirst($class).'.php';
-
- if ( ! file_exists($baseclass))
+ // Check each path for an extension file
+ $subclass = APPPATH.'libraries/'.$dir.$prefix.$class.'.php';
+ if (file_exists($subclass))
{
- log_message('error', 'Unable to load the requested class: '.$class);
- show_error('Unable to load the requested class: '.$class);
- }
+ // Require the base class in the same directory
+ $baseclass = BASEPATH.'libraries/'.$dir.ucfirst($class).'.php';
+ if ( ! file_exists($baseclass))
+ {
+ // No base matches extension - bail
+ $msg = 'Unable to load the requested class: '.$class;
+ log_message('error', $msg);
+ show_error($msg);
+ }
- // Safety: Was the class already loaded by a previous call?
- if (in_array($subclass, $this->_ci_loaded_files))
- {
- // Before we deem this to be a duplicate request, let's see
- // if a custom object name is being supplied. If so, we'll
- // return a new instance of the object
- if ( ! is_null($object_name))
+ // Does this look like a driver?
+ if (strtolower($dir) === strtolower($class).'/' && ! class_exists('CI_Driver_Library'))
+ {
+ // We aren't instantiating an object here, just making the base class available
+ require BASEPATH.'libraries/Driver.php';
+ }
+
+ // Safety: Was the class already loaded by a previous call?
+ if (in_array($subclass, $this->_ci_loaded_files))
{
- $CI =& get_instance();
- if ( ! isset($CI->$object_name))
+ // Before we deem this to be a duplicate request, let's see
+ // if a custom object name is being supplied. If so, we'll
+ // return a new instance of the object
+ if ( ! is_null($object_name))
{
- return $this->_ci_init_class($class, config_item('subclass_prefix'), $params, $object_name);
+ $CI =& get_instance();
+ if ( ! isset($CI->$object_name))
+ {
+ return $this->_ci_init_class($class, $prefix, $params, $object_name);
+ }
}
- }
- $is_duplicate = TRUE;
- log_message('debug', $class.' class already loaded. Second attempt ignored.');
- return;
- }
+ $is_duplicate = TRUE;
+ log_message('debug', $class.' class already loaded. Second attempt ignored.');
+ return;
+ }
- include_once($baseclass);
- include_once($subclass);
- $this->_ci_loaded_files[] = $subclass;
+ include($baseclass);
+ include($subclass);
+ $this->_ci_loaded_files[] = $subclass;
- return $this->_ci_init_class($class, config_item('subclass_prefix'), $params, $object_name);
+ return $this->_ci_init_class($class, $prefix, $params, $object_name);
+ }
}
// Lets search for the requested library file and load it.
$is_duplicate = FALSE;
foreach ($this->_ci_library_paths as $path)
{
- $filepath = $path.'libraries/'.$subdir.$class.'.php';
-
- // Does the file exist? No? Bummer...
- if ( ! file_exists($filepath))
+ foreach ($trysubs as $dir)
{
- continue;
- }
+ $filepath = $path.'libraries/'.$dir.$class.'.php';
- // Safety: Was the class already loaded by a previous call?
- if (in_array($filepath, $this->_ci_loaded_files))
- {
- // Before we deem this to be a duplicate request, let's see
- // if a custom object name is being supplied. If so, we'll
- // return a new instance of the object
- if ( ! is_null($object_name))
+ // Does the file exist? No? Bummer...
+ if ( ! file_exists($filepath))
+ {
+ continue;
+ }
+
+ // Does this look like a driver?
+ if (strtolower($dir) === strtolower($class).'/' && ! class_exists('CI_Driver_Library'))
+ {
+ // We aren't instantiating an object here, just making the base class available
+ require BASEPATH.'libraries/Driver.php';
+ }
+
+ // Safety: Was the class already loaded by a previous call?
+ if (in_array($filepath, $this->_ci_loaded_files))
{
- $CI =& get_instance();
- if ( ! isset($CI->$object_name))
+ // Before we deem this to be a duplicate request, let's see
+ // if a custom object name is being supplied. If so, we'll
+ // return a new instance of the object
+ if ( ! is_null($object_name))
{
- return $this->_ci_init_class($class, '', $params, $object_name);
+ $CI =& get_instance();
+ if ( ! isset($CI->$object_name))
+ {
+ return $this->_ci_init_class($class, '', $params, $object_name);
+ }
}
+
+ $is_duplicate = TRUE;
+ log_message('debug', $class.' class already loaded. Second attempt ignored.');
+ return;
}
- $is_duplicate = TRUE;
- log_message('debug', $class.' class already loaded. Second attempt ignored.');
- return;
+ include($filepath);
+ $this->_ci_loaded_files[] = $filepath;
+ return $this->_ci_init_class($class, '', $params, $object_name);
}
-
- include_once($filepath);
- $this->_ci_loaded_files[] = $filepath;
- return $this->_ci_init_class($class, '', $params, $object_name);
}
} // END FOREACH
- // One last attempt. Maybe the library is in a subdirectory, but it wasn't specified?
- if ($subdir === '')
- {
- $path = strtolower($class).'/'.$class;
- return $this->_ci_load_class($path, $params, $object_name);
- }
- elseif (ucfirst($subdir) != $subdir)
- {
- // Lowercase subdir failed - retry capitalized
- $path = ucfirst($subdir).$class;
- return $this->_ci_load_class($path, $params, $object_name);
- }
-
// If we got this far we were unable to find the requested class.
// We do not issue errors if the load call failed due to a duplicate request
if ($is_duplicate === FALSE)
@@ -1324,4 +1352,4 @@ protected function _ci_prep_filename($filename, $extension)
}
/* End of file Loader.php */
-/* Location: ./system/core/Loader.php */
+/* Location: ./system/core/Loader.php */
View
14 system/libraries/Cache/Cache.php
@@ -42,13 +42,13 @@ class CI_Cache extends CI_Driver_Library {
*
* @var array
*/
- protected $valid_drivers = array(
- 'cache_apc',
- 'cache_dummy',
- 'cache_file',
- 'cache_memcached',
- 'cache_redis',
- 'cache_wincache'
+ protected $valid_drivers = array(
+ 'apc',
+ 'dummy',
+ 'file',
+ 'memcached',
+ 'redis',
+ 'wincache'
);
/**
View
136 system/libraries/Driver.php
@@ -9,7 +9,7 @@
* Licensed under the Open Software License version 3.0
*
* This source file is subject to the Open Software License (OSL 3.0) that is
- * bundled with this package in the files license.txt / license.rst. It is
+ * bundled with this package in the files license.txt / license.rst. It is
* also available through the world wide web at this URL:
* http://opensource.org/licenses/OSL-3.0
* If you did not receive a copy of the license and are unable to obtain it
@@ -55,13 +55,20 @@ class CI_Driver_Library {
protected $lib_name;
/**
+ * Subclass prefix from config
+ *
+ * @var string
+ */
+ protected $subclass_prefix = '';
+
+ /**
* Get magic method
*
* The first time a child is used it won't exist, so we instantiate it
* subsequents calls will go straight to the proper child.
*
- * @param string Child class name
- * @return object Child class
+ * @param string Child class name
+ * @return object Child class
*/
public function __get($child)
{
@@ -74,61 +81,120 @@ public function __get($child)
*
* Separate load_driver call to support explicit driver load by library or user
*
- * @param string Child class name
- * @return object Child class
+ * @param string Driver name (w/o parent prefix)
+ * @return object Child class
*/
public function load_driver($child)
{
+ // Get CodeIgniter instance
+ $CI = get_instance();
+
if ( ! isset($this->lib_name))
{
- $this->lib_name = get_class($this);
+ // Get library name without any prefix
+ $this->subclass_prefix = (string) $CI->config->item('subclass_prefix');
+ $this->lib_name = str_replace(array('CI_', $this->subclass_prefix), '', get_class($this));
}
- // The class will be prefixed with the parent lib
- $child_class = $this->lib_name.'_'.$child;
+ // The child will be prefixed with the parent lib
+ $child_name = $this->lib_name.'_'.$child;
- // Remove the CI_ prefix and lowercase
- $lib_name = ucfirst(strtolower(str_replace('CI_', '', $this->lib_name)));
- $driver_name = strtolower(str_replace('CI_', '', $child_class));
+ // See if requested child is a valid driver
+ if ( ! in_array($child, array_map('strtolower', $this->valid_drivers)))
+ {
+ // The requested driver isn't valid!
+ $msg = 'Invalid driver requested: '.$child_name;
+ log_message('error', $msg);
+ show_error($msg);
+ }
- if (in_array($driver_name, array_map('strtolower', $this->valid_drivers)))
+ // All driver files should be in a library subdirectory - capitalized
+ $subdir = ucfirst(strtolower($this->lib_name));
+
+ // Get package paths and filename case variations to search
+ $paths = $CI->load->get_package_paths(TRUE);
+ $cases = array(ucfirst($child_name), strtolower($child_name));
+
+ // Is there an extension?
+ $class_name = $this->subclass_prefix.$child_name;
+ $found = class_exists($class_name);
+ if ( ! $found)
{
- // check and see if the driver is in a separate file
- if ( ! class_exists($child_class))
+ // Check for subclass file
+ foreach ($paths as $path)
{
- // check application path first
- foreach (get_instance()->load->get_package_paths(TRUE) as $path)
+ // Extension will be in drivers subdirectory
+ $path .= 'libraries/'.$subdir.'/drivers/';
+
+ // Try filename with caps and all lowercase
+ foreach ($cases as $name)
{
- // loves me some nesting!
- foreach (array(ucfirst($driver_name), $driver_name) as $class)
+ // Does the file exist?
+ $file = $path.$this->subclass_prefix.$name.'.php';
+ if (file_exists($file))
{
- $filepath = $path.'libraries/'.$lib_name.'/drivers/'.$class.'.php';
-
- if (file_exists($filepath))
+ // Yes - require base class from last path (BASEPATH)
+ $basepath = end($paths).'libraries/'.$subdir.'/drivers/'.ucfirst($child_name).'.php';
+ if ( ! file_exists($basepath))
{
- include_once $filepath;
- break 2;
+ $msg = 'Unable to load the requested class: CI_'.$child_name;
+ log_message('error', $msg);
+ show_error($msg);
}
+
+ // Include both sources and mark found
+ include($basepath);
+ include($file);
+ $found = TRUE;
+ break 2;
}
}
+ }
+ }
- // it's a valid driver, but the file simply can't be found
- if ( ! class_exists($child_class))
+ // Do we need to search for the class?
+ if ( ! $found)
+ {
+ // Use standard class name
+ $class_name = 'CI_'.$child_name;
+ $found = class_exists($class_name);
+ if ( ! $found)
+ {
+ // Check package paths
+ foreach ($paths as $path)
{
- log_message('error', 'Unable to load the requested driver: '.$child_class);
- show_error('Unable to load the requested driver: '.$child_class);
+ // Class will be in drivers subdirectory
+ $path .= 'libraries/'.$subdir.'/drivers/';
+
+ // Try filename with caps and all lowercase
+ foreach ($cases as $name)
+ {
+ // Does the file exist?
+ $file = $path.$name.'.php';
+ if (file_exists($file))
+ {
+ // Include source
+ include $file;
+ break 2;
+ }
+ }
}
}
+ }
- $obj = new $child_class;
- $obj->decorate($this);
- $this->$child = $obj;
- return $this->$child;
+ // Did we finally find the class?
+ if ( ! class_exists($class_name))
+ {
+ $msg = 'Unable to load the requested driver: '.$class_name;
+ log_message('error', $msg);
+ show_error($msg);
}
- // The requested driver isn't valid!
- log_message('error', 'Invalid driver requested: '.$child_class);
- show_error('Invalid driver requested: '.$child_class);
+ // Instantiate, decorate, and add child
+ $obj = new $class_name;
+ $obj->decorate($this);
+ $this->$child = $obj;
+ return $this->$child;
}
}
@@ -286,4 +352,4 @@ public function __set($var, $val)
}
/* End of file Driver.php */
-/* Location: ./system/libraries/Driver.php */
+/* Location: ./system/libraries/Driver.php */
View
19 system/libraries/Session/Session.php
@@ -107,17 +107,15 @@ public function __construct(array $params = array())
// Get valid drivers list
$this->valid_drivers = array(
- 'Session_native',
- 'Session_cookie'
+ 'native',
+ 'cookie'
);
$key = 'sess_valid_drivers';
$drivers = isset($params[$key]) ? $params[$key] : $CI->config->item($key);
if ($drivers)
{
- is_array($drivers) OR $drivers = array($drivers);
-
// Add driver names to valid list
- foreach ($drivers as $driver)
+ foreach ((array)$drivers as $driver)
{
if ( ! in_array(strtolower($driver), array_map('strtolower', $this->valid_drivers)))
{
@@ -134,9 +132,9 @@ public function __construct(array $params = array())
$driver = 'cookie';
}
- if ( ! in_array('session_'.strtolower($driver), array_map('strtolower', $this->valid_drivers)))
+ if ( ! in_array(strtolower($driver), array_map('strtolower', $this->valid_drivers)))
{
- $this->valid_drivers[] = 'Session_'.$driver;
+ $this->valid_drivers[] = $driver;
}
// Save a copy of parameters in case drivers need access
@@ -178,17 +176,16 @@ public function load_driver($driver)
/**
* Select default session storage driver
*
- * @param string Driver classname
+ * @param string Driver name
* @return void
*/
public function select_driver($driver)
{
// Validate driver name
- $lowername = strtolower(str_replace('CI_', '', $driver));
- if (in_array($lowername, array_map('strtolower', $this->valid_drivers)))
+ $child = strtolower(str_replace(array('CI_', $this->subclass_prefix, $this->lib_name.'_'), '', $driver));
+ if (in_array($child, array_map('strtolower', $this->valid_drivers)))
{
// See if driver is loaded
- $child = str_replace($this->lib_name.'_', '', $driver);
if (isset($this->$child))
{
// See if driver is already current
View
269 tests/codeigniter/libraries/Driver_test.php
@@ -0,0 +1,269 @@
+<?php
+
+/**
+ * Driver library base class unit test
+ */
+class Driver_test extends CI_TestCase {
+ /**
+ * Set up test framework
+ */
+ public function set_up()
+ {
+ // Create VFS directories
+ $this->root = vfsStream::setup();
+ $this->base_root = vfsStream::newDirectory('system')->at($this->root);
+ $this->app_root = vfsStream::newDirectory('application')->at($this->root);
+ $this->base_path = vfsStream::url('system/');
+ $this->app_path = vfsStream::url('application/');
+ }
+
+ /**
+ * Test driver child loading
+ *
+ * @covers CI_Driver_Library::load_driver
+ */
+ public function test_load_driver()
+ {
+ // Create mock driver library
+ $this->_mock_library();
+
+ // Create driver file
+ $driver = 'basic';
+ $file = $this->name.'_'.$driver;
+ $class = 'CI_'.$file;
+ $prop = 'called';
+ $content = '<?php class '.$class.' extends CI_Driver { public $'.$prop.' = FALSE; '.
+ 'public function decorate($parent) { $this->'.$prop.' = TRUE; } }';
+ $this->_create_content($file, $content, $this->base_root, 'libraries/'.$this->name.'/drivers');
+
+ // Make driver valid
+ $this->lib->driver_list($driver);
+
+ // Load driver
+ $this->assertNotNull($this->lib->load_driver($driver));
+
+ // Did lib name get set?
+ $this->assertEquals($this->name, $this->lib->get_name());
+
+ // Was driver loaded?
+ $this->assertObjectHasAttribute($driver, $this->lib);
+ $this->assertAttributeInstanceOf($class, $driver, $this->lib);
+ $this->assertAttributeInstanceOf('CI_Driver', $driver, $this->lib);
+
+ // Was decorate called?
+ $this->assertObjectHasAttribute($prop, $this->lib->$driver);
+ $this->assertTrue($this->lib->$driver->$prop);
+
+ // Do we get an error for an invalid driver?
+ $driver = 'unlisted';
+ $this->setExpectedException('RuntimeException', 'CI Error: Invalid driver requested: '.$this->name.'_'.$driver);
+ $this->lib->load_driver($driver);
+ }
+
+ /**
+ * Test loading lowercase from app path
+ *
+ * @covers CI_Driver_Library::load_driver
+ */
+ public function test_load_app_driver()
+ {
+ // Create mock driver library
+ $this->_mock_library();
+
+ // Create driver file
+ $driver = 'lowpack';
+ $file = $this->name.'_'.$driver;
+ $class = 'CI_'.$file;
+ $content = '<?php class '.$class.' extends CI_Driver { }';
+ $this->_create_content(strtolower($file), $content, $this->app_root, 'libraries/'.$this->name.'/drivers');
+
+ // Make valid list
+ $nodriver = 'absent';
+ $this->lib->driver_list(array($driver, $nodriver));
+
+ // Load driver
+ $this->assertNotNull($this->lib->load_driver($driver));
+
+ // Was driver loaded?
+ $this->assertObjectHasAttribute($driver, $this->lib);
+ $this->assertAttributeInstanceOf($class, $driver, $this->lib);
+ $this->assertAttributeInstanceOf('CI_Driver', $driver, $this->lib);
+
+ // Do we get an error for a non-existent driver?
+ $this->setExpectedException('RuntimeException', 'CI Error: Unable to load the requested driver: CI_'.
+ $this->name.'_'.$nodriver);
+ $this->lib->load_driver($nodriver);
+ }
+
+ /**
+ * Test loading driver extension
+ *
+ * @covers CI_Driver_Library::load_driver
+ */
+ public function test_load_driver_ext()
+ {
+ // Create mock driver library
+ $this->_mock_library();
+
+ // Create base file
+ $driver = 'extend';
+ $base = $this->name.'_'.$driver;
+ $baseclass = 'CI_'.$base;
+ $content = '<?php class '.$baseclass.' extends CI_Driver { }';
+ $this->_create_content($base, $content, $this->base_root, 'libraries/'.$this->name.'/drivers');
+
+ // Create driver file
+ $class = $this->subclass.$base;
+ $content = '<?php class '.$class.' extends '.$baseclass.' { }';
+ $this->_create_content($class, $content, $this->app_root, 'libraries/'.$this->name.'/drivers');
+
+ // Make valid list
+ $this->lib->driver_list($driver);
+
+ // Load driver
+ $this->assertNotNull($this->lib->load_driver($driver));
+
+ // Was driver loaded?
+ $this->assertObjectHasAttribute($driver, $this->lib);
+ $this->assertAttributeInstanceOf($class, $driver, $this->lib);
+ $this->assertAttributeInstanceOf($baseclass, $driver, $this->lib);
+ $this->assertAttributeInstanceOf('CI_Driver', $driver, $this->lib);
+
+ // Create driver extension without base
+ $driver = 'baseless';
+ $base = $this->name.'_'.$driver;
+ $class = $this->subclass.$base;
+ $content = '<?php class '.$class.' extends CI_Driver { }';
+ $this->_create_content($class, $content, $this->app_root, 'libraries/'.$this->name.'/drivers');
+ $this->lib->driver_list($driver);
+
+ // Do we get an error when base class isn't found?
+ $this->setExpectedException('RuntimeException', 'CI Error: Unable to load the requested class: CI_'.$base);
+ $this->lib->load_driver($driver);
+ }
+
+ /**
+ * Test decorating driver with parent attributes
+ *
+ * @covers CI_Driver::decorate
+ */
+ public function test_decorate()
+ {
+ // Create parent with a method and property to access
+ $pclass = 'Test_Parent';
+ $prop = 'parent_prop';
+ $value = 'initial';
+ $method = 'parent_func';
+ $return = 'func return';
+ $code = 'class '.$pclass.' { public $'.$prop.' = \''.$value.'\'; '.
+ 'public function '.$method.'() { return \''.$return.'\'; } }';
+ eval($code);
+ $parent = new $pclass();
+
+ // Create child driver to decorate
+ $class = 'Test_Driver';
+ eval('class '.$class.' extends CI_Driver { }');
+ $child = new $class();
+
+ // Decorate child
+ $child->decorate($parent);
+
+ // Do we get the initial parent property value?
+ $this->assertEquals($value, $child->$prop);
+
+ // Can we change the parent property?
+ $newval = 'changed';
+ $child->$prop = $newval;
+ $this->assertEquals($newval, $parent->$prop);
+
+ // Do we get back the updated value?
+ $this->assertEquals($newval, $child->$prop);
+
+ // Can we call the parent method?
+ $this->assertEquals($return, $child->$method());
+ }
+
+ /**
+ * Create mock driver library
+ */
+ private function _mock_library()
+ {
+ // Create instance object
+ $ci = new StdClass();
+
+ // Mock up Config to return subclass prefix
+ $cfg = 'Drv_Mock_Config';
+ if ( ! class_exists($cfg))
+ {
+ $code = 'class '.$cfg.' { public $config = array(); '.
+ 'public function item($key) { return isset($this->config[$key]) ? $this->config[$key] : FALSE; } }';
+ eval($code);
+ }
+ $ci->config = new $cfg();
+ $this->subclass = 'Mock_Libraries_';
+ $ci->config->config['subclass_prefix'] = $this->subclass;
+
+ // Mock up Loader to return package paths
+ $load = 'Drv_Mock_Loader';
+ if ( ! class_exists($load))
+ {
+ $code = 'class '.$load.' { public $paths = array(); '.
+ 'public function get_package_paths($all) { return $this->paths; } }';
+ eval($code);
+ }
+ $ci->load = new $load();
+ $ci->load->paths = array($this->app_path, $this->base_path);
+
+ // Set instance
+ $this->ci_instance($ci);
+
+ // Create mock driver library
+ $this->name = 'Driver';
+ $this->lib = new Mock_Libraries_Driver();
+ }
+
+ /**
+ * Create file (and subdirectories) in VFS tree
+ *
+ * @param string File name (w/o .php)
+ * @param string File content
+ * @param object VFS root to create under
+ * @param string Subdirectory path
+ * @return void
+ */
+ private function _create_content($file, $content, $root, $path = '')
+ {
+ // Initialize tree with file
+ $tree = array($file.'.php' => $content);
+
+ // Get subdirectories
+ $subs = explode('/', $path);
+ while (($dir = array_shift($subs)))
+ {
+ // See if subdir exists under current root
+ $dir_root = $root->getChild($dir);
+ if ($dir_root)
+ {
+ // Yes - recurse into subdir
+ $root = $dir_root;
+ }
+ else
+ {
+ // No - put subdir back and quit
+ array_unshift($subs, $dir);
+ break;
+ }
+ }
+
+ // Create any remaining subdirectories
+ foreach (array_reverse($subs) as $dir)
+ {
+ // Wrap content in subdirectory for creation
+ $tree = array($dir => $tree);
+ }
+
+ // Create tree
+ vfsStream::create($tree, $root);
+ }
+}
+
View
11 tests/codeigniter/libraries/Session_test.php
@@ -39,6 +39,9 @@ public function set_up()
// Make sure string helper is available
$this->ci_vfs_clone('system/helpers/string_helper.php');
+ // Set subclass prefix to match our mock
+ $ci->config->set_item('subclass_prefix', 'Mock_Libraries_');
+
// Attach session instance locally
$config = array(
'sess_encrypt_cookie' => FALSE,
@@ -56,11 +59,7 @@ public function set_up()
'sess_time_to_update' => 300,
'time_reference' => 'local',
'cookie_prefix' => '',
- 'encryption_key' => 'foobar',
- 'sess_valid_drivers' => array(
- 'Mock_Libraries_Session_native',
- 'Mock_Libraries_Session_cookie'
- )
+ 'encryption_key' => 'foobar'
);
$this->session = new Mock_Libraries_Session($config);
}
@@ -399,4 +398,4 @@ public function test_sess_destroy()
$this->session->native->sess_destroy();
$this->assertNull($this->session->native->userdata($key));
}
-}
+}
View
28 tests/mocks/libraries/driver.php
@@ -0,0 +1,28 @@
+<?php
+
+/**
+ * Mock library to subclass Driver for testing
+ */
+class Mock_Libraries_Driver extends CI_Driver_Library {
+ /**
+ * Set valid drivers list
+ */
+ public function driver_list($drivers = NULL)
+ {
+ if (empty($drivers))
+ {
+ return $this->valid_drivers;
+ }
+
+ $this->valid_drivers = (array)$drivers;
+ }
+
+ /**
+ * Get library name
+ */
+ public function get_name()
+ {
+ return $this->lib_name;
+ }
+}
+
View
5 tests/mocks/libraries/session.php
@@ -36,8 +36,3 @@ protected function _setcookie($name, $value = '', $expire = 0, $path = '', $doma
}
}
}
-
-/**
- * Mock native driver (just for consistency in loading)
- */
-class Mock_Libraries_Session_native extends CI_Session_native { }
Something went wrong with that request. Please try again.