Complete testing.

diff --git a/framework/History/lib/Horde/History.php b/framework/History/lib/Horde/History.php
index aa246a0..a871dd0 100644 (file)
--- a/framework/History/lib/Horde/History.php
+++ b/framework/History/lib/Horde/History.php
@@ -135,6 +135,10 @@ class Horde_History
     public function log($guid, array $attributes = array(),
                         $replaceAction = false)
     {
+        if (!is_string($guid)) {
+            throw new InvalidArgumentException('The guid needs to be a string!');
+        }
+
         $history = $this->getHistory($guid);
 
         if (!isset($attributes['who'])) {
@@ -181,6 +185,24 @@ class Horde_History
      */
     public function getHistory($guid)
     {
+        if (!is_string($guid)) {
+            throw new InvalidArgumentException('The guid needs to be a string!');
+        }
+        return $this->_getHistory($guid);
+    }
+
+    /**
+     * 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_Exception
+     */
+    public function _getHistory($guid)
+    {
         throw new Horde_Exception('Not implemented!');
     }
 
@@ -211,6 +233,42 @@ class Horde_History
     public function getByTimestamp($cmp, $ts, array $filters = array(),
                                    $parent = null)
     {
+        if (!is_string($cmp)) {
+            throw new InvalidArgumentException('The comparison operator needs to be a string!');
+        }
+        if (!is_integer($ts)) {
+            throw new InvalidArgumentException('The timestamp needs to be an integer!');
+        }
+        return $this->_getByTimestamp($cmp, $ts, $filters, $parent);
+    }
+
+    /**
+     * Finds history objects by timestamp, and optionally filter on other
+     * fields as well.
+     *
+     * @param string  $cmp     The comparison operator (<, >, <=, >=, or =) to
+     *                         check the timestamps with.
+     * @param integer $ts      The timestamp to compare against.
+     * @param array   $filters An array of additional (ANDed) criteria.
+     *                         Each array value should be an array with 3
+     *                         entries:
+     * <pre>
+     * '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').
+     * </pre>
+     * @param string  $parent  The parent history to start searching at. If
+     *                         non-empty, will be searched for with a LIKE
+     *                         '$parent:%' clause.
+     *
+     * @return array  An array of history object ids, or an empty array if
+     *                none matched the criteria.
+     *
+     * @throws Horde_Exception
+     */
+    public function _getByTimestamp($cmp, $ts, array $filters = array(),
+                                    $parent = null)
+    {
         throw new Horde_Exception('Not implemented!');
     }
 
@@ -222,10 +280,16 @@ class Horde_History
      *
      * @return integer  The timestamp, or 0 if no matching entry is found.
      *
+     * @throws InvalidArgumentException If the input parameters are not of
+     *                                  type string.
      * @throws Horde_Exception
      */
     public function getActionTimestamp($guid, $action)
     {
+        if (!is_string($guid) || !is_string($action)) {
+            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. */

diff --git a/framework/History/lib/Horde/History/Factory.php b/framework/History/lib/Horde/History/Factory.php
index cdfa8e6..963fa18 100644 (file)
--- a/framework/History/lib/Horde/History/Factory.php
+++ b/framework/History/lib/Horde/History/Factory.php
@@ -148,8 +148,6 @@ class Horde_History_Factory
                 $portability |= DB_PORTABILITY_RTRIM;
             }
             $db->setOption('portability', $portability);
-        } else if ($db instanceOf PEAR_Error) {
-            throw new Horde_Exception($db->getMessage());
         }
         return $db;
     }

diff --git a/framework/History/lib/Horde/History/Mock.php b/framework/History/lib/Horde/History/Mock.php
index dde9254..71f2f40 100644 (file)
--- a/framework/History/lib/Horde/History/Mock.php
+++ b/framework/History/lib/Horde/History/Mock.php
@@ -110,7 +110,7 @@ class Horde_History_Mock extends Horde_History
      *
      * @return Horde_HistoryObject  A Horde_HistoryObject
      */
-    public function getHistory($guid)
+    public function _getHistory($guid)
     {
         $result= array();
         foreach ($this->_data as $id => $element) {
@@ -146,8 +146,8 @@ class Horde_History_Mock extends Horde_History
      *
      * @throws Horde_Exception
      */
-    public function getByTimestamp($cmp, $ts, $filters = array(),
-                                   $parent = null)
+    public function _getByTimestamp($cmp, $ts, $filters = array(),
+                                    $parent = null)
     {
         $result = array();
 

diff --git a/framework/History/lib/Horde/History/Sql.php b/framework/History/lib/Horde/History/Sql.php
index e3cc548..3df2470 100644 (file)
--- a/framework/History/lib/Horde/History/Sql.php
+++ b/framework/History/lib/Horde/History/Sql.php
@@ -167,7 +167,7 @@ class Horde_History_Sql extends Horde_History
      *
      * @return Horde_HistoryObject  A Horde_HistoryObject
      */
-    public function getHistory($guid)
+    public function _getHistory($guid)
     {
         $rows = $this->_db->getAll('SELECT * FROM horde_histories WHERE object_uid = ?', array($guid), DB_FETCHMODE_ASSOC);
         $this->handleError($rows);
@@ -198,8 +198,8 @@ class Horde_History_Sql extends Horde_History
      *
      * @throws Horde_Exception
      */
-    public function getByTimestamp($cmp, $ts, $filters = array(),
-                                   $parent = null)
+    public function _getByTimestamp($cmp, $ts, $filters = array(),
+                                    $parent = null)
     {
         /* Build the timestamp test. */
         $where = array("history_ts $cmp $ts");

diff --git a/framework/History/test/Horde/History/InterfaceTest.php b/framework/History/test/Horde/History/InterfaceTest.php
index 829cd43..eadb1fe 100644 (file)
--- a/framework/History/test/Horde/History/InterfaceTest.php
+++ b/framework/History/test/Horde/History/InterfaceTest.php
@@ -147,6 +147,7 @@ EOL;
         }
         return $injector;
     }
+
     /**
      * Return the history handler for the specified environment.
      *
@@ -177,7 +178,6 @@ EOL;
 
     public function testMethodLogHasPostConditionThatTimestampAndActorAreAlwaysRecorded()
     {
-        //@todo: More complex
         foreach ($this->getEnvironments() as $environment) {
             $history = $this->getHistory($environment);
             $history->log('test', array('action' => 'test'));
@@ -189,7 +189,6 @@ EOL;
 
     public function testMethodLogHasPostConditionThatTheGivenEventHasBeenRecorded()
     {
-        //@todo: More complex
         foreach ($this->getEnvironments() as $environment) {
             $history = $this->getHistory($environment);
             $history->log('test', array('who' => 'me', 'ts' => 1000, 'action' => 'test'));
@@ -199,12 +198,14 @@ EOL;
 
     public function testMethodLogHasParameterStringGuid()
     {
-        //@todo: Add a check for the type.
-    }
-
-    public function testMethodLogHasArrayParameterAttributes()
-    {
-        //@todo: type hint the array
+        foreach ($this->getEnvironments() as $environment) {
+            $history = $this->getHistory($environment);
+            try {
+                $history->log(array());
+                $this->fail('No exception!');
+            } catch (InvalidArgumentException $e) {
+            }
+        }
     }
 
     public function testMethodLogHasArrayParameterBooleanReplaceaction()
@@ -245,7 +246,14 @@ EOL;
 
     public function testMethodGethistoryHasParameterStringGuid()
     {
-        //@todo: Add a check for the type.
+        foreach ($this->getEnvironments() as $environment) {
+            $history = $this->getHistory($environment);
+            try {
+                $history->getHistory(array());
+                $this->fail('No exception!');
+            } catch (InvalidArgumentException $e) {
+            }
+        }
     }
 
     public function testMethodGethistoryHasResultHordehistoryobjectRepresentingTheHistoryLogMatchingTheGivenGuid()
@@ -278,10 +286,26 @@ EOL;
 
     public function testMethodGetbytimestampHasParameterStringCmp()
     {
+        foreach ($this->getEnvironments() as $environment) {
+            $history = $this->getHistory($environment);
+            try {
+                $history->getByTimestamp(array(), 1);
+                $this->fail('No exception!');
+            } catch (InvalidArgumentException $e) {
+            }
+        }
     }
 
     public function testMethodGetbytimestampHasParameterIntegerTs()
     {
+        foreach ($this->getEnvironments() as $environment) {
+            $history = $this->getHistory($environment);
+            try {
+                $history->getByTimestamp('>', 'hello');
+                $this->fail('No exception!');
+            } catch (InvalidArgumentException $e) {
+            }
+        }
     }
 
     public function testMethodGetbytimestampHasParameterArrayFilters()
@@ -309,30 +333,57 @@ EOL;
 
     public function testMethodGetbytimestampHasResultArrayContainingTheMatchingEventIds()
     {
-        //@todo: More complex
         foreach ($this->getEnvironments() as $environment) {
             $history = $this->getHistory($environment);
             $history->log('test', array('who' => 'me', 'ts' => 1000, 'action' => 'test'));
             $history->log('test', array('who' => 'me', 'ts' => 1000, 'action' => 'test'));
             $history->log('test', array('who' => 'you', 'ts' => 2000, 'action' => 'yours', 'extra' => array('a' => 'a')));
+            $result = $history->getByTimestamp('<=', 1000);
+            $this->assertEquals(array('test' => 2), $result);
             $result = $history->getByTimestamp('<', 1001);
             $this->assertEquals(array('test' => 2), $result);
+            $result = $history->getByTimestamp('>', 1001);
+            $this->assertEquals(array('test' => 3), $result);
+            $result = $history->getByTimestamp('>=', 2000);
+            $this->assertEquals(array('test' => 3), $result);
+            $result = $history->getByTimestamp('=', 2000);
+            $this->assertEquals(array('test' => 3), $result);
+            $result = $history->getByTimestamp('>', 2000);
+            $this->assertEquals(array(), $result);
         }
     }
 
     public function testMethodGetactiontimestampHasParameterStringGuid()
     {
-        //@todo: Add a check for the type.
+        foreach ($this->getEnvironments() as $environment) {
+            $history = $this->getHistory($environment);
+            try {
+                $history->getActionTimestamp(array(), 'test');
+                $this->fail('No exception!');
+            } catch (InvalidArgumentException $e) {
+            }
+        }
     }
 
     public function testMethodGetactiontimestampHasParameterStringAction()
     {
-        //@todo: Add a check for the type.
+        foreach ($this->getEnvironments() as $environment) {
+            $history = $this->getHistory($environment);
+            try {
+                $history->getActionTimestamp('test', array());
+                $this->fail('No exception!');
+            } catch (InvalidArgumentException $e) {
+            }
+        }
     }
 
     public function testMethodGetactiontimestampHasResultIntegerZeroIfGethistoryReturnsAnError()
     {
-        //@todo: test
+        $injector = $this->initializeEnvironment(self::ENVIRONMENT_DB);
+        $mock = new Dummy_Db();
+        $injector->setInstance('DB_common_write', $mock);
+        $history = $injector->getInstance('Horde_History');
+        $this->assertEquals(0, $history->getActionTimestamp('test', 'test'));
     }
 
     public function testMethodGetactiontimestampHasResultIntegerZeroIfThereIsNoMatchingRecord()