From 604689b2709ee6b9ecaad0a653933a1e247175d7 Mon Sep 17 00:00:00 2001 From: "Michael J. Rubinsky" Date: Wed, 12 Jan 2011 12:25:18 -0500 Subject: [PATCH] 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 --- kronolith/lib/FreeBusy.php | 2 +- kronolith/lib/Injector/Factory/Storage.php | 80 +++++++++++++++++++++++++++++ kronolith/lib/Storage.php | 48 ------------------ kronolith/lib/Storage/sql.php | 81 +++++++----------------------- 4 files changed, 100 insertions(+), 111 deletions(-) create mode 100644 kronolith/lib/Injector/Factory/Storage.php diff --git a/kronolith/lib/FreeBusy.php b/kronolith/lib/FreeBusy.php index 0944c4656..4a6ffca17 100644 --- 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 index 000000000..9b2bf896e --- /dev/null +++ b/kronolith/lib/Injector/Factory/Storage.php @@ -0,0 +1,80 @@ +_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 b610f4449..1f62927ce 100644 --- 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 1949a2d8c..29d6e5f64 100644 --- 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 + * @author Michael J Rubinsky * @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); } } -- 2.11.0