Start removing PEAR::DB from kronolith, refactor for H4

This commit:
  * Refactor Kronolith_Storage to use dependency injection
  * Replace PEAR::DB usage with Horde_Db_Adapter in Kronolith_Storage

Kolab storage driver wasn't touched, as I'm not familiar enough with this code,
though the code should still work, it just wasn't refactored for DI

diff --git a/kronolith/lib/FreeBusy.php b/kronolith/lib/FreeBusy.php
index 0944c46..4a6ffca 100644 (file)
--- a/kronolith/lib/FreeBusy.php
+++ b/kronolith/lib/FreeBusy.php
@@ -222,7 +222,7 @@ class Kronolith_FreeBusy
         }
 
         /* Check storage driver. */
-        $storage = Kronolith_Storage::factory();
+        $storage = $GLOBALS['injector']->getInstance('Kronolith_Injector_Factory_Storage')->create();
 
         try {
             $fb = $storage->search($email);

diff --git a/kronolith/lib/Injector/Factory/Storage.php b/kronolith/lib/Injector/Factory/Storage.php
new file mode 100644 (file)
index 0000000..9b2bf89
--- /dev/null
+++ b/kronolith/lib/Injector/Factory/Storage.php
@@ -0,0 +1,80 @@
+<?php
+
+class Kronolith_Injector_Factory_Storage
+{
+    /**
+     * Instances.
+     *
+     * @var array
+     */
+    private $_instances = array();
+
+    /**
+     * The injector.
+     *
+     * @var Horde_Injector
+     */
+    private $_injector;
+
+    /**
+     * Constructor.
+     *
+     * @param Horde_Injector $injector  The injector to use.
+     */
+    public function __construct(Horde_Injector $injector)
+    {
+        $this->_injector = $injector;
+    }
+
+    /**
+     * Return the driver instance.
+     *
+     * @return Kronolith_Storage
+     * @throws Kronolith_Exception
+     */
+    public function create($params = array())
+    {
+        if (empty($params['user'])) {
+            $user = $GLOBALS['registry']->getAuth();
+        } else {
+            $user = $params['user'];
+            unset($params['user']);
+        }
+
+        if (empty($params['driver'])) {
+            $driver = Horde_String::ucfirst($GLOBALS['conf']['storage']['driver']);
+        } else {
+            $driver = $params['driver'];
+            unset($params['driver']);
+        }
+        $driver = basename($driver);
+        $class = 'Kronolith_Storage_' . $driver;
+
+        $driver_params =  Horde::getDriverConfig('storage', 'Sql');
+        if ($driver == 'Sql') {
+            if ($driver_params != 'Horde') {
+                // Custom DB config
+                $params['db'] = $this->_injector->getInstance('Horde_Core_Factory_Db')->create('kronolith', Horde::getDriverConfig('storage', 'Sql'));
+            } else {
+                // Horde default DB config
+                $params['db'] = $this->_injector->getInstance('Horde_Core_Factory_Db')->create();
+            }
+            $params['table'] = $driver_params['table'];
+        }
+
+        if (class_exists($class)) {
+            $driver = new $class($user, $params);
+        } else {
+            throw new Kronolith_Exception(sprintf(_("Unable to load the definition of %s."), $class));
+        }
+
+        try {
+            $driver->initialize();
+        } catch (Exception $e) {
+            $driver = new Kronolith_Storage($params);
+        }
+
+        return $driver;
+    }
+
+}
\ No newline at end of file

diff --git a/kronolith/lib/Storage.php b/kronolith/lib/Storage.php
index b610f44..1f62927 100644 (file)
--- a/kronolith/lib/Storage.php
+++ b/kronolith/lib/Storage.php
@@ -15,54 +15,6 @@ abstract class Kronolith_Storage
     protected $_user = '';
 
     /**
-     * Attempts to return a concrete Kronolith_Storage instance based on
-     * $driver.
-     *
-     * @param string $driver  The type of concrete Kronolith_Storage subclass
-     *                        to return.  The is based on the storage driver
-     *                        ($driver).  The code is dynamically included.
-     * @param string $user    The name of the user who owns the free/busy
-     *                        information.
-     * @param array $params   A hash containing any additional configuration or
-     *                        connection parameters a subclass might need.
-     *
-     * @return Kronolith_Storage The newly created concrete Kronolith_Storage
-     *                           instance.
-     * @throws Kronolith_Exception
-     */
-    public static function factory($user = null, $driver = null, $params = null)
-    {
-        if (is_null($user)) {
-            $user = $GLOBALS['registry']->getAuth();
-        }
-
-        if (is_null($driver)) {
-            $driver = $GLOBALS['conf']['storage']['driver'];
-        }
-
-        $driver = basename($driver);
-
-        if (is_null($params)) {
-            $params = Horde::getDriverConfig('storage', $driver);
-        }
-
-        $class = 'Kronolith_Storage_' . $driver;
-        if (class_exists($class)) {
-            $driver = new $class($user, $params);
-        } else {
-            throw new Kronolith_Exception(sprintf(_("Unable to load the definition of %s."), $class));
-        }
-
-        try {
-            $driver->initialize();
-        } catch (Exception $e) {
-            $driver = new Kronolith_Storage($params);
-        }
-
-        return $driver;
-    }
-
-    /**
      * Stub to initiate a driver.
      * @throws Kronolith_Exception
      */

diff --git a/kronolith/lib/Storage/sql.php b/kronolith/lib/Storage/sql.php
index 1949a2d..29d6e5f 100644 (file)
--- a/kronolith/lib/Storage/sql.php
+++ b/kronolith/lib/Storage/sql.php
@@ -3,6 +3,7 @@
  * Kronolith_Storage:: defines an API for storing free/busy information.
  *
  * @author  Mike Cochrane <mike@graftonhall.co.nz>
+ * @author  Michael J Rubinsky <mrubinsk@horde.org>
  * @package Kronolith
  */
 class Kronolith_Storage_sql extends Kronolith_Storage
@@ -10,19 +11,11 @@ class Kronolith_Storage_sql extends Kronolith_Storage
     /**
      * Handle for the current database connection, used for reading.
      *
-     * @var DB
+     * @var Horde_Db_Adapter
      */
     protected $_db;
 
     /**
-     * Handle for the current database connection, used for writing. Defaults
-     * to the same handle as $_db if a separate write database is not required.
-     *
-     * @var DB
-     */
-    protected $_write_db;
-
-    /**
      * Hash containing connection parameters.
      *
      * @var array
@@ -37,28 +30,16 @@ class Kronolith_Storage_sql extends Kronolith_Storage
     public function __construct($user, $params = array())
     {
         $this->_user = $user;
+        if (empty($params['db'])) {
+            throw new InvalidArgumentException(_("Missing required db parameter"));
+        }
 
-        /* Use defaults where needed. */
+        $this->_db = $params['db'];
         $this->_params = $params;
         $this->_params['table'] = isset($params['table']) ? $params['table'] : 'kronolith_storage';
     }
 
     /**
-     * Connect to the database
-     *
-     * @throws Kronolith_Exception
-     */
-    public function initialize()
-    {
-        try {
-            $this->_db = $GLOBALS['injector']->getInstance('Horde_Core_Factory_DbPear')->create('read', 'kronolith', 'storage');
-            $this->_write_db = $GLOBALS['injector']->getInstance('Horde_Core_Factory_DbPear')->create('rw', 'kronolith', 'storage');
-        } catch (Horde_Exception $e) {
-            throw new Kronolith_Exception($e);
-        }
-    }
-
-    /**
      * Search for a user's free/busy information.
      *
      * @param string  $email        The email address to lookup
@@ -71,33 +52,26 @@ class Kronolith_Storage_sql extends Kronolith_Storage
     public function search($email, $private_only = false)
     {
         /* Build the SQL query. */
-        $query = sprintf('SELECT vfb_serialized FROM %s WHERE vfb_email=? AND (vfb_owner=?',
+        $query = sprintf('SELECT vfb_serialized FROM %s WHERE vfb_email = ? AND (vfb_owner = ?',
                          $this->_params['table']);
         $values = array($email, $this->_user);
 
         if ($private_only) {
             $query .= ')';
         } else {
-            $query .= " OR vfb_owner='')";
+            $query .= " OR vfb_owner = '')";
         }
 
-        /* Log the query at debug level. */
-        Horde::logMessage(sprintf('SQL search by %s: query = "%s"',
-                                  $GLOBALS['registry']->getAuth(), $query), 'DEBUG');
-
         /* Execute the query. */
-        $result = $this->_db->query($query, $values);
-        if (!($result instanceof PEAR_Error)) {
-            $row = $result->fetchRow(DB_GETMODE_ASSOC);
-            $result->free();
-            if (is_array($row)) {
-                /* Retrieve Freebusy object.  TODO: check for multiple
-                 * results and merge them into one and return. */
-                $vfb = Horde_Serialize::unserialize($row['vfb_serialized'], Horde_Serialize::BASIC);
-                return $vfb;
+        try {
+            $result = $this->_db->selectValue($query, $values);
+            if (empty($result)) {
+                throw new Horde_Exception_NotFound();
             }
+            return Horde_Serialize::unserialize($result, Horde_Serialize::BASIC);
+        } catch (Horde_Db_Exception $e) {
+            throw new Kronolith_Exception($e);
         }
-        throw new Horde_Exception_NotFound();
     }
 
     /**
@@ -116,28 +90,11 @@ class Kronolith_Storage_sql extends Kronolith_Storage
                          $this->_params['table']);
         $values = array($public ? '' : $this->_user, $email, Horde_Serialize::serialize($vfb, Horde_Serialize::BASIC));
 
-        /* Log the query at debug level. */
-        Horde::logMessage(sprintf('SQL insert by %s: query = "%s"',
-                                  $GLOBALS['registry']->getAuth(), $query), 'DEBUG');
-
         /* Execute the query. */
-        $result = $this->_write_db->query($query, $values);
-        $this->handleError($result);
-    }
-
-    /**
-     * Determines if the given result is a PEAR error. If it is, logs the event
-     * and throws an exception.
-     *
-     * @param mixed $result The result to check.
-     *
-     * @throws Horde_Exception
-     */
-    protected function handleError($result)
-    {
-        if ($result instanceof PEAR_Error) {
-            Horde::logMessage($result, 'ERR');
-            throw new Kronolith_Exception($result);
+        try {
+            $result = $this->_db->insert($query, $values);
+        } catch (Horde_Db_Exception $e) {
+            throw new Kronolith_Exception($e);
         }
     }