From 423a06bffd54aa7e96ab31f31afeb76a10d4a82a Mon Sep 17 00:00:00 2001 From: Jan Schneider Date: Fri, 19 Mar 2010 16:24:54 +0100 Subject: [PATCH] Make Horde_History an abstract class. Add Horde_History_Exception. phpdoc --- framework/History/lib/Horde/History.php | 121 +++++++++------------ framework/History/lib/Horde/History/Exception.php | 16 +++ framework/History/lib/Horde/History/Factory.php | 54 +++++---- framework/History/lib/Horde/History/Mock.php | 51 ++++----- framework/History/lib/Horde/History/Sql.php | 83 +++++++------- framework/History/lib/Horde/HistoryObject.php | 2 +- .../History/test/Horde/History/InterfaceTest.php | 63 ++++------- 7 files changed, 180 insertions(+), 210 deletions(-) create mode 100644 framework/History/lib/Horde/History/Exception.php diff --git a/framework/History/lib/Horde/History.php b/framework/History/lib/Horde/History.php index ebf9020fc..04fba81b9 100644 --- a/framework/History/lib/Horde/History.php +++ b/framework/History/lib/Horde/History.php @@ -33,7 +33,7 @@ require_once 'Horde/Autoloader.php'; * @license http://www.fsf.org/copyleft/lgpl.html LGPL * @link http://pear.horde.org/index.php?package=History */ -class Horde_History +abstract class Horde_History { /** * Instance cache. @@ -59,7 +59,7 @@ class Horde_History * @param string $driver The driver to use. * * @return Horde_History The concrete Horde_History reference. - * @throws Horde_Exception + * @throws Horde_History_Exception */ static public function singleton($driver = null) { @@ -72,7 +72,7 @@ class Horde_History if ($driver == 'Sql') { if (empty($conf['sql']['phptype']) || ($conf['sql']['phptype'] == 'none')) { - throw new Horde_Exception(_("The History system is disabled.")); + throw new Horde_History_Exception(_("The History system is disabled.")); } $params = $conf['sql']; } else { @@ -109,28 +109,25 @@ class Horde_History } /** - * Logs an event to an item's history log. The item must be uniquely - * identified by $guid. Any other details about the event are passed in - * $attributes. Standard suggested attributes are: + * Logs an event to an item's history log. * - * 'who' => The id of the user that performed the action (will be added - * automatically if not present). + * The item must be uniquely identified by $guid. Any other details about + * the event are passed in $attributes. Standard suggested attributes are: + * - who: The id of the user that performed the action (will be added + * automatically if not present). + * - ts: Timestamp of the action (this will be added automatically if it + * is not present). * - * 'ts' => Timestamp of the action (this will be added automatically if - * it is not present). - * - * @param string $guid The unique identifier of the entry to - * add to. + * @param string $guid The unique identifier of the entry to add + * to. * @param array $attributes The hash of name => value entries that * describe this event. * @param boolean $replaceAction If $attributes['action'] is already - * present in the item's history log, - * update that entry instead of creating a - * new one. - * - * @return boolean True if the operation succeeded. + * present in the item's history log, update + * that entry instead of creating a new one. * - * @throws Horde_Exception + * @throws Horde_History_Exception + * @throws InvalidArgumentException */ public function log($guid, array $attributes = array(), $replaceAction = false) @@ -148,30 +145,26 @@ class Horde_History $attributes['ts'] = time(); } - return $this->_log($history, $attributes, $replaceAction); + $this->_log($history, $attributes, $replaceAction); } /** - * Logs an event to an item's history log. Any other details about the event - * are passed in $attributes. + * Logs an event to an item's history log. Any other details about the + * event are passed in $attributes. * * @param Horde_HistoryObject $history The history item to add to. - * @param array $attributes The hash of name => value entries - * that describe this event. + * @param array $attributes The hash of name => value + * entries that describe this + * event. * @param boolean $replaceAction If $attributes['action'] is * already present in the item's * history log, update that entry * instead of creating a new one. * - * @return boolean True if the operation succeeded. - * - * @throws Horde_Exception + * @throws Horde_History_Exception */ - protected function _log(Horde_HistoryObject $history, array $attributes, - $replaceAction = false) - { - throw new Horde_Exception('Not implemented!'); - } + abstract protected function _log(Horde_HistoryObject $history, + array $attributes, $replaceAction = false); /** * Returns a Horde_HistoryObject corresponding to the named history @@ -181,7 +174,8 @@ class Horde_History * * @return Horde_HistoryObject A Horde_HistoryObject * - * @throws Horde_Exception + * @throws Horde_History_Exception + * @throws InvalidArgumentException */ public function getHistory($guid) { @@ -199,12 +193,9 @@ class Horde_History * * @return Horde_HistoryObject A Horde_HistoryObject * - * @throws Horde_Exception + * @throws Horde_History_Exception */ - public function _getHistory($guid) - { - throw new Horde_Exception('Not implemented!'); - } + abstract public function _getHistory($guid); /** * Finds history objects by timestamp, and optionally filter on other @@ -216,11 +207,10 @@ class Horde_History * @param array $filters An array of additional (ANDed) criteria. * Each array value should be an array with 3 * entries: - * 
-     * 'field' - the history field being compared (i.e. 'action').
-     * 'op'    - the operator to compare this field with.
-     * 'value' - the value to check for (i.e. 'add').
-     *
+ * - field: the history field being compared (i.e. + * 'action'). + * - op: the operator to compare this field with. + * - value: the value to check for (i.e. 'add'). * @param string $parent The parent history to start searching at. If * non-empty, will be searched for with a LIKE * '$parent:%' clause. @@ -228,7 +218,8 @@ class Horde_History * @return array An array of history object ids, or an empty array if * none matched the criteria. * - * @throws Horde_Exception + * @throws Horde_History_Exception + * @throws InvalidArgumentException */ public function getByTimestamp($cmp, $ts, array $filters = array(), $parent = null) @@ -252,11 +243,10 @@ class Horde_History * @param array $filters An array of additional (ANDed) criteria. * Each array value should be an array with 3 * entries: - * 
-     * 'field' - the history field being compared (i.e. 'action').
-     * 'op'    - the operator to compare this field with.
-     * 'value' - the value to check for (i.e. 'add').
-     *
+ * - field: the history field being compared (i.e. + * 'action'). + * - op: the operator to compare this field with. + * - value: the value to check for (i.e. 'add'). * @param string $parent The parent history to start searching at. If * non-empty, will be searched for with a LIKE * '$parent:%' clause. @@ -264,13 +254,11 @@ class Horde_History * @return array An array of history object ids, or an empty array if * none matched the criteria. * - * @throws Horde_Exception + * @throws Horde_History_Exception */ - public function _getByTimestamp($cmp, $ts, array $filters = array(), - $parent = null) - { - throw new Horde_Exception('Not implemented!'); - } + abstract public function _getByTimestamp($cmp, $ts, + array $filters = array(), + $parent = null); /** * Gets the timestamp of the most recent change to $guid. @@ -282,7 +270,6 @@ class Horde_History * * @throws InvalidArgumentException If the input parameters are not of * type string. - * @throws Horde_Exception */ public function getActionTimestamp($guid, $action) { @@ -290,12 +277,11 @@ class Horde_History throw new InvalidArgumentException('$guid and $action need to be strings!'); } - /* This implementation still works, but we should be able to - * get much faster now with a SELECT MAX(history_ts) - * ... query. */ + /* This implementation still works, but we should be able to get much + * faster now with a SELECT MAX(history_ts) ... query. */ try { $history = $this->getHistory($guid); - } catch (Horde_Exception $e) { + } catch (Horde_History_Exception $e) { return 0; } @@ -317,7 +303,7 @@ class Horde_History * * @param string $parent The parent name to remove. * - * @throws Horde_Exception + * @throws Horde_History_Exception */ public function removeByParent($parent) { @@ -330,17 +316,12 @@ class Horde_History } /** - * Remove one or more history entries by name. + * Removes one or more history entries by name. * - * @param array $names The history entries to remove. + * @param array $names The history entries to remove. * - * @return boolean True if the operation succeeded. - * - * @throws Horde_Exception + * @throws Horde_History_Exception */ - public function removeByNames(array $names) - { - throw new Horde_Exception('Not implemented!'); - } + abstract public function removeByNames(array $names); } diff --git a/framework/History/lib/Horde/History/Exception.php b/framework/History/lib/Horde/History/Exception.php new file mode 100644 index 000000000..370adccb8 --- /dev/null +++ b/framework/History/lib/Horde/History/Exception.php @@ -0,0 +1,16 @@ + + * @category Horde + * @package Horde_History + */ +class Horde_History_Exception extends Horde_Exception_Prior +{ +} diff --git a/framework/History/lib/Horde/History/Factory.php b/framework/History/lib/Horde/History/Factory.php index 6c0f82d84..1051f79f1 100644 --- a/framework/History/lib/Horde/History/Factory.php +++ b/framework/History/lib/Horde/History/Factory.php @@ -36,20 +36,22 @@ require_once 'Horde/Autoloader.php'; class Horde_History_Factory { /** - * Create a concrete Horde_History instance. + * Creates a concrete Horde_History instance. * - * @param Horde_Injector $injector The environment for creating the instance. + * @param Horde_Injector $injector The environment for creating the + * instance. * * @return Horde_History The new Horde_History instance. * - * @throws Horde_Exception If the injector provides no configuration. + * @throws Horde_History_Exception If the injector provides no + * configuration. */ static public function getHistory(Horde_Injector $injector) { try { $config = $injector->getInstance('Horde_History_Config'); } catch (ReflectionException $e) { - throw new Horde_Exception( + throw new Horde_History_Exception( sprintf( 'The configuration for the History driver is missing: %s', $e->getMessage() @@ -63,37 +65,39 @@ class Horde_History_Factory case 'Mock': return Horde_History_Factory::getHistoryMock($config->params); default: - throw new Horde_Exception(sprintf("Driver %s not supported!", $config->driver)); + throw new Horde_History_Exception(sprintf("Driver %s not supported!", $config->driver)); } } /** - * Create a concrete Horde_History_Sql instance. + * Creates a concrete Horde_History_Sql instance. * - * @param Horde_Injector $injector The environment for creating the instance. - * @param array $params The db connection parameters if the - * environment does not already provide a - * connection. + * @param Horde_Injector $injector The environment for creating the + * instance. + * @param array $params The db connection parameters if the + * environment does not already provide a + * connection. * * @return Horde_History_Sql The new Horde_History_Sql instance. * - * @throws Horde_Exception If the injector provides no configuration or - * creating the database connection failed. + * @throws Horde_History_Exception If the injector provides no + * configuration or creating the database + * connection failed. */ static protected function getHistorySql(Horde_Injector $injector, array $params) { try { - /** See if there is a specific write db instance available */ + /* See if there is a specific write db instance available */ $write_db = $injector->getInstance('DB_common_write'); $history = new Horde_History_Sql($write_db); try { - /** See if there is a specific read db instance available */ + /* See if there is a specific read db instance available */ $read_db = $injector->getInstance('DB_common_read'); $history->setReadDb($read_db); } catch (ReflectionException $e) { } } catch (ReflectionException $e) { - /** No DB instances. Use the configuration. */ + /* No DB instances. Use the configuration. */ $write_db = Horde_History_Factory::getDb($params); $history = new Horde_History_Sql($write_db); @@ -110,17 +114,19 @@ class Horde_History_Factory } /** - * Create a concrete Horde_History_Mock instance. + * Creates a concrete Horde_History_Mock instance. * - * @param Horde_Injector $injector The environment for creating the instance. - * @param array $params The db connection parameters if the - * environment does not already provide a - * connection. + * @param Horde_Injector $injector The environment for creating the + * instance. + * @param array $params The db connection parameters if the + * environment does not already provide a + * connection. * * @return Horde_History_Mock The new Horde_History_Mock instance. * - * @throws Horde_Exception If the injector provides no configuration or - * creating the database connection failed. + * @throws Horde_History_Exception If the injector provides no + * configuration or creating the database + * connection failed. */ static protected function getHistoryMock(array $params) { @@ -128,13 +134,13 @@ class Horde_History_Factory } /** - * Create a database connection. + * Creates a database connection. * * @param array $params The database connection parameters. * * @return DB_common * - * @throws Horde_Exception In case the database connection failed. + * @throws Horde_History_Exception In case the database connection failed. */ static protected function getDb(array $params) { diff --git a/framework/History/lib/Horde/History/Mock.php b/framework/History/lib/Horde/History/Mock.php index dc9a7db86..d4dacb5b1 100644 --- a/framework/History/lib/Horde/History/Mock.php +++ b/framework/History/lib/Horde/History/Mock.php @@ -12,8 +12,8 @@ */ /** - * The Horde_History_Mock:: class provides a method of tracking changes in Horde - * objects, stored in memory. + * The Horde_History_Mock:: class provides a method of tracking changes in + * Horde objects, stored in memory. * * Copyright 2009-2010 The Horde Project (http://www.horde.org/) * @@ -43,20 +43,19 @@ class Horde_History_Mock extends Horde_History private $_id = 1; /** - * Logs an event to an item's history log. Any other details about the event - * are passed in $attributes. + * Logs an event to an item's history log. Any other details about the + * event are passed in $attributes. * * @param Horde_HistoryObject $history The history item to add to. - * @param array $attributes The hash of name => value entries - * that describe this event. + * @param array $attributes The hash of name => value + * entries that describe this + * event. * @param boolean $replaceAction If $attributes['action'] is * already present in the item's * history log, update that entry * instead of creating a new one. * - * @return boolean True if the operation succeeded. - * - * @throws Horde_Exception + * @throws Horde_History_Exception */ protected function _log(Horde_HistoryObject $history, array $attributes, @@ -98,15 +97,13 @@ class Horde_History_Mock extends Horde_History $this->_data[$this->_id] = $values; $this->_id++; } - - return true; } /** - * Returns a Horde_HistoryObject corresponding to the named history - * entry, with the data retrieved appropriately. + * Returns a Horde_HistoryObject corresponding to the named history entry, + * with the data retrieved appropriately. * - * @param string $guid The name of the history entry to retrieve. + * @param string $guid The name of the history entry to retrieve. * * @return Horde_HistoryObject A Horde_HistoryObject */ @@ -132,11 +129,10 @@ class Horde_History_Mock extends Horde_History * @param array $filters An array of additional (ANDed) criteria. * Each array value should be an array with 3 * entries: - * 
-     * 'field' - the history field being compared (i.e. 'action').
-     * 'op'    - the operator to compare this field with.
-     * 'value' - the value to check for (i.e. 'add').
-     *
+ * - field: the history field being compared (i.e. + * 'action'). + * - op: the operator to compare this field with. + * - value: the value to check for (i.e. 'add'). * @param string $parent The parent history to start searching at. If * non-empty, will be searched for with a LIKE * '$parent:%' clause. @@ -144,7 +140,7 @@ class Horde_History_Mock extends Horde_History * @return array An array of history object ids, or an empty array if * none matched the criteria. * - * @throws Horde_Exception + * @throws Horde_History_Exception */ public function _getByTimestamp($cmp, $ts, array $filters = array(), $parent = null) @@ -184,7 +180,7 @@ class Horde_History_Mock extends Horde_History }; break; default: - throw new Horde_Exception(sprintf("Comparison %s not implemented!", $cmp)); + throw new Horde_History_Exception(sprintf("Comparison %s not implemented!", $cmp)); } if ($ignore) { @@ -195,7 +191,7 @@ class Horde_History_Mock extends Horde_History if ($filters) { foreach ($filters as $filter) { if ($filter['op'] != '=') { - throw new Horde_Exception(sprintf("Comparison %s not implemented!", $filter['op'])); + throw new Horde_History_Exception(sprintf("Comparison %s not implemented!", $filter['op'])); } if ($element['history_' . $filter['field']] != $filter['value']) { $ignore = true; @@ -219,18 +215,16 @@ class Horde_History_Mock extends Horde_History } /** - * Remove one or more history entries by name. - * - * @param array $names The history entries to remove. + * Removes one or more history entries by name. * - * @return boolean True if the operation succeeded. + * @param array $names The history entries to remove. * - * @throws Horde_Exception + * @throws Horde_History_Exception */ public function removeByNames(array $names) { if (!count($names)) { - return true; + return; } $ids = array(); @@ -243,6 +237,5 @@ class Horde_History_Mock extends Horde_History foreach ($ids as $id) { unset($this->_data[$id]); } - return true; } } diff --git a/framework/History/lib/Horde/History/Sql.php b/framework/History/lib/Horde/History/Sql.php index d2b2def20..4537973a0 100644 --- a/framework/History/lib/Horde/History/Sql.php +++ b/framework/History/lib/Horde/History/Sql.php @@ -1,6 +1,6 @@ value entries - * that describe this event. + * @param array $attributes The hash of name => value + * entries that describe this + * event. * @param boolean $replaceAction If $attributes['action'] is * already present in the item's * history log, update that entry * instead of creating a new one. * - * @return boolean True if the operation succeeded. - * - * @throws Horde_Exception + * @throws Horde_History_Exception */ protected function _log(Horde_HistoryObject $history, array $attributes, @@ -155,17 +153,17 @@ class Horde_History_Sql extends Horde_History ); $this->handleError($r); } - - return true; } /** - * Returns a Horde_HistoryObject corresponding to the named history - * entry, with the data retrieved appropriately. + * Returns a Horde_HistoryObject corresponding to the named history entry, + * with the data retrieved appropriately. * * @param string $guid The name of the history entry to retrieve. * * @return Horde_HistoryObject A Horde_HistoryObject + * + * @throws Horde_History_Exception */ public function _getHistory($guid) { @@ -184,11 +182,10 @@ class Horde_History_Sql extends Horde_History * @param array $filters An array of additional (ANDed) criteria. * Each array value should be an array with 3 * entries: - * 
-     * 'field' - the history field being compared (i.e. 'action').
-     * 'op'    - the operator to compare this field with.
-     * 'value' - the value to check for (i.e. 'add').
-     *
+ * - field: the history field being compared (i.e. + * 'action'). + * - op: the operator to compare this field with. + * - value: the value to check for (i.e. 'add'). * @param string $parent The parent history to start searching at. If * non-empty, will be searched for with a LIKE * '$parent:%' clause. @@ -196,9 +193,9 @@ class Horde_History_Sql extends Horde_History * @return array An array of history object ids, or an empty array if * none matched the criteria. * - * @throws Horde_Exception + * @throws Horde_History_Exception */ - public function _getByTimestamp($cmp, $ts, $filters = array(), + public function _getByTimestamp($cmp, $ts, array $filters = array(), $parent = null) { /* Build the timestamp test. */ @@ -221,18 +218,16 @@ class Horde_History_Sql extends Horde_History } /** - * Remove one or more history entries by name. + * Removes one or more history entries by name. * - * @param array $names The history entries to remove. + * @param array $names The history entries to remove. * - * @return boolean True if the operation succeeded. - * - * @throws Horde_Exception + * @throws Horde_History_Exception */ - public function removeByNames($names) + public function removeByNames(array $names) { if (!count($names)) { - return true; + return; } $ids = array(); @@ -240,20 +235,16 @@ class Horde_History_Sql extends Horde_History $ids[] = $this->_write_db->quote($name); } - $result = $this->_write_db->query('DELETE FROM horde_histories WHERE object_uid IN (' . implode(',', $ids) . ')'); - $this->handleError($result); - return $result; + $this->handleError($this->_write_db->query('DELETE FROM horde_histories WHERE object_uid IN (' . implode(',', $ids) . ')')); } /** - * Determine if the given result is a PEAR error. If it is, log the event - * and throw an exception. - * - * @param mixed $result The result to check. + * Determines if the given result is a PEAR error. If it is, logs the event + * and throws an exception. * - * @return NULL + * @param mixed $result The result to check. * - * @throws Horde_Exception + * @throws Horde_History_Exception */ protected function handleError($result) { @@ -263,7 +254,7 @@ class Horde_History_Sql extends Horde_History } else { Horde::logMessage($result, 'ERR'); } - throw new Horde_Exception($result->getMessage()); + throw new Horde_History_Exception($result->getMessage()); } } } diff --git a/framework/History/lib/Horde/HistoryObject.php b/framework/History/lib/Horde/HistoryObject.php index ad9ed3ba6..dd54ce202 100644 --- a/framework/History/lib/Horde/HistoryObject.php +++ b/framework/History/lib/Horde/HistoryObject.php @@ -8,7 +8,7 @@ * did not receive this file, see http://www.fsf.org/copyleft/lgpl.html. * * @author Chuck Hagenbuch - * @package Horde_History + * @package History */ class Horde_HistoryObject { diff --git a/framework/History/test/Horde/History/InterfaceTest.php b/framework/History/test/Horde/History/InterfaceTest.php index 56ba76879..7804d79a9 100644 --- a/framework/History/test/Horde/History/InterfaceTest.php +++ b/framework/History/test/Horde/History/InterfaceTest.php @@ -45,10 +45,24 @@ class Horde_History_InterfaceTest extends PHPUnit_Framework_TestCase private $_db_file; /** + * Test setup. + */ + public function setUp() + { + if (in_array(self::ENVIRONMENT_DB, $this->getEnvironments())) { + /* PEAR DB is not E_STRICT safe. */ + $this->_errorReporting = error_reporting(E_ALL & ~E_STRICT); + } + } + + /** * Test cleanup. */ public function tearDown() { + if (in_array(self::ENVIRONMENT_DB, $this->getEnvironments())) { + error_reporting($this->_errorReporting); + } if (!empty($this->_db_file)) { unlink($this->_db_file); } @@ -62,10 +76,8 @@ class Horde_History_InterfaceTest extends PHPUnit_Framework_TestCase public function getEnvironments() { if (empty($this->_environments)) { - /** - * The db environment provides our only test scenario before - * refactoring. - */ + /* The db environment provides our only test scenario before + * refactoring. */ $this->_environments = array( self::ENVIRONMENT_MOCK, /** Uncomment if you want to run a sqlity based test */ @@ -454,7 +466,7 @@ EOL; } } - public function testMethodRemovebynamesHasResultBooleanTrueIfParameterNamesIsEmpty() + public function testMethodRemovebynamesSucceedsIfParameterNamesIsEmpty() { foreach ($this->getEnvironments() as $environment) { $history = $this->getHistory($environment); @@ -462,39 +474,7 @@ EOL; $history->log('test', array('who' => 'me', 'ts' => 1000, 'action' => 'test'), false); $history->log('test', array('who' => 'you', 'ts' => 2000, 'action' => 'yours')); $history->log('test', array('who' => 'you', 'ts' => 2000, 'action' => 'yours'), true); - $this->assertEquals(true, $data = $history->removeByNames(array())); - } - } - - public function testBaseHordehistoryClassProvidesIncompleteImplementation() - { - global $conf; - - try { - $history = new Horde_History(); - $history->getHistory(''); - $this->fail('No exception was thrown!'); - } catch (Horde_Exception $e) { - } - try { - $history = new Horde_History(); - $history->getByTimestamp('=', 1); - $this->fail('No exception was thrown!'); - } catch (Horde_Exception $e) { - } - try { - $history = new Horde_History(); $history->removeByNames(array()); - $this->fail('No exception was thrown!'); - } catch (Horde_Exception $e) { - } - - unset($conf['sql']); - - try { - $history = Horde_History::singleton(); - $this->fail('No exception was thrown!'); - } catch (Horde_Exception $e) { } } @@ -556,7 +536,8 @@ class Dummy_Db extends DB_common { public function &query($query, $params = array()) { - return new PEAR_Error('Error'); + $e = new PEAR_Error('Error'); + return $e; } public function nextId($seq_name, $ondemand = true) @@ -567,12 +548,14 @@ class Dummy_Db extends DB_common public function &getAll($query, $params = array(), $fetchmode = DB_FETCHMODE_DEFAULT) { - return new PEAR_Error('Error'); + $e = new PEAR_Error('Error'); + return $e; } public function &getAssoc($query, $force_array = false, $params = array(), $fetchmode = DB_FETCHMODE_DEFAULT, $group = false) { - return new PEAR_Error('Error'); + $e = new PEAR_Error('Error'); + return $e; } } \ No newline at end of file -- 2.11.0