Necessary changes to the validation components. Further improvements and docs will follow in the next days.

Ticket: 150

lib/Doctrine/Record.php

@@ -101,7 +101,7 @@ abstract class Doctrine_Record extends Doctrine_Access implements Countable, Ite
     /**
      * @var Doctrine_Validator_ErrorStack   error stack object
      */
-    private $errorStack;
+    protected $errorStack;
     /**
      * @var integer $index                  this index is used for creating object identifiers
      */
@@ -238,14 +238,20 @@ abstract class Doctrine_Record extends Doctrine_Access implements Countable, Ite
             return true;
 
         $validator = new Doctrine_Validator();
+        // Run validators
+        $validator->validateRecord($this);
+        // Run custom validation
+        $this->validate();
         
-        if($validator->validateRecord($this))
+        return $this->errorStack->count() == 0 ? true : false;
-           return true;
+        //$this->errorStack->merge($validator->getErrorStack());
-           
-        $this->errorStack->merge($validator->getErrorStack());
-        
-        return false;
     }
+    /**
+     * Emtpy template method to provide concrete Record classes with the possibility
+     * to hook into the validation procedure, doing any custom / specialized
+     * validations that are neccessary.
+     */
+    protected function validate() {}
     /**
      * getErrorStack
      *
@@ -644,6 +650,7 @@ abstract class Doctrine_Record extends Doctrine_Access implements Countable, Ite
      * @return mixed
      */
     public function get($name, $invoke = true) {
+        
         $listener = $this->table->getAttribute(Doctrine::ATTR_LISTENER);
         $value    = self::$null;
         $lower    = strtolower($name);
@@ -834,12 +841,10 @@ abstract class Doctrine_Record extends Doctrine_Access implements Countable, Ite
 
         $saveLater = $conn->saveRelated($this);
 
-        $this->isValid();
+        if ($this->isValid()) {
-        
-        if($this->errorStack->count() > 0) {
-            $conn->getTransaction()->addInvalid($this);
-        } else {
             $conn->save($this);
+        } else {
+            $conn->getTransaction()->addInvalid($this);
         }
 
         foreach($saveLater as $fk) {

lib/Doctrine/Validator.php

@@ -27,10 +27,6 @@
  * @license     LGPL
  */
 class Doctrine_Validator {    
-    /**
-     * @var array $stack                error stack
-     */
-    private $stack              = array();
     /**
      * @var array $validators           an array of validator objects
      */
@@ -78,6 +74,8 @@ class Doctrine_Validator {
         $columns   = $record->getTable()->getColumns();
         $component = $record->getTable()->getComponentName();
 
+        $errorStack = $record->getErrorStack();
+        
         switch($record->getState()):
             case Doctrine_Record::STATE_TDIRTY:
             case Doctrine_Record::STATE_TCLEAN:
@@ -102,7 +100,7 @@ class Doctrine_Validator {
                 $value = $record->getTable()->enumIndex($key, $value);
 
                 if($value === false) {
-                    $err[$key] = 'enum';
+                    $errorStack->add($key, 'enum');
                     continue;
                 }
             }
@@ -113,7 +111,7 @@ class Doctrine_Validator {
                 $length = strlen($value);
 
             if($length > $column[1]) {
-                $err[$key] = 'length';
+                $errorStack->add($key, 'length');
                 continue;
             }
 
@@ -144,7 +142,7 @@ class Doctrine_Validator {
                 if( ! $validator->validate($record, $key, $value, $args)) {
 
 
-                    $err[$key] = $name;
+                    $errorStack->add($key, $name);
                         
                     //$err[$key] = 'not valid';
 
@@ -153,17 +151,10 @@ class Doctrine_Validator {
                 }
             }
             if( ! self::isValidType($value, $column[0])) {
-                $err[$key] = 'type';
+                $errorStack->add($key, 'type');
                 continue;
             }
         }
-
-        if( ! empty($err)) {
-            $this->stack = $err;
-            return false;
-        }
-        
-        return true;
     }
     /**
      * whether or not this validator has errors
@@ -173,14 +164,6 @@ class Doctrine_Validator {
     public function hasErrors() {
         return (count($this->stack) > 0);
     }
-    /**
-     * returns the error stack
-     *
-     * @return array
-     */
-    public function getErrorStack() {
-        return $this->stack;
-    }
     /**
      * converts a doctrine type to native php type
      *

lib/Doctrine/Validator/ErrorStack.php

@@ -18,38 +18,138 @@
  * and is licensed under the LGPL. For more information, see
  * <http://www.phpdoctrine.com>.
  */
-Doctrine::autoload('Doctrine_Access');
+
 /**
  * Doctrine_Validator_ErrorStack
  *
  * @author      Konsta Vesterinen
+ * @author      Roman Borschel
  * @license     LGPL
  * @package     Doctrine
  */
-class Doctrine_Validator_ErrorStack extends Doctrine_Access implements Countable, IteratorAggregate {
+class Doctrine_Validator_ErrorStack implements ArrayAccess, Countable, IteratorAggregate {
     
-    private $errors = array();
+    /**
+     * The errors of the error stack.
+     *
+     * @var array
+     */
+    protected $errors = array();
 
+    /**
+     * Constructor
+     *
+     */
+    public function __construct()
+    {}
+    
+    /**
+     * Adds an error to the stack.
+     *
+     * @param string $invalidFieldName
+     * @param string $errorType
+     */
+    public function add($invalidFieldName, $errorType = 'general') {
+        $this->errors[$invalidFieldName][] = array('type' => $errorType);
+    }
+    
+    /**
+     * Removes all existing errors for the specified field from the stack.
+     *
+     * @param string $fieldName
+     */
+    public function remove($fieldName) {
+        if (isset($this->errors[$fieldName])) {
+            unset($this->errors[$fieldName]);
+        }
+    }
+    
+    /**
+     * Enter description here...
+     *
+     * @param unknown_type $name
+     * @return unknown
+     */
+    public function get($name) {
+        return $this[$name];
+    }
+    
+    /** ArrayAccess implementation */
+    
+    /**
+     * Gets all errors that occured for the specified field.
+     *
+     * @param string $offset
+     * @return The array containing the errors or NULL if no errors were found.
+     */
+    public function offsetGet($offset) {
+        return isset($this->errors[$offset]) ? $this->errors[$offset] : null;
+    }
+    
+    /**
+     * Enter description here...
+     *
+     * @param string $offset
+     * @param mixed $value
+     * @throws Doctrine_Validator_ErrorStack_Exception  Always thrown since this operation is not allowed. 
+     */
+    public function offsetSet($offset, $value) {
+        throw new Doctrine_Validator_ErrorStack_Exception("Errors can only be added through
+                Doctrine_Validator_ErrorStack::add()");
+    }
+    
+    /**
+     * Enter description here...
+     *
+     * @param unknown_type $offset
+     */
+    public function offsetExists($offset) {
+        return isset($this->errors[$offset]);
+    }
+    
+    /**
+     * Enter description here...
+     *
+     * @param unknown_type $offset
+     * @throws Doctrine_Validator_ErrorStack_Exception  Always thrown since this operation is not allowed. 
+     */
+    public function offsetUnset($offset) {
+        throw new Doctrine_Validator_ErrorStack_Exception("Errors can only be removed
+                through Doctrine_Validator_ErrorStack::remove()");
+    }
+    
+    /**
+     * Enter description here...
+     *
+     * @param unknown_type $stack
+     */
+    /*
     public function merge($stack) {
         if(is_array($stack)) {
             $this->errors = array_merge($this->errors, $stack);
         }
-    }
+    }*/
     
-    public function get($name) {
-        if(isset($this->errors[$name]))
-            return $this->errors[$name];
     
-        return null;
+    /** IteratorAggregate implementation */
-    }
-
-    public function set($name, $value) {
-        $this->errors[$name] = $value;
-    }
     
+    /**
+     * Enter description here...
+     *
+     * @return unknown
+     */
     public function getIterator() {
         return new ArrayIterator($this->errors);
     }
+    
+    
+    /** Countable implementation */
+    
+    /**
+     * Enter description here...
+     *
+     * @return unknown
+     */
     public function count() {
         return count($this->errors);
     }

tests/ValidatorTestCase.php

@@ -1,10 +1,18 @@
 <?php
+/**
+ * TestCase for Doctrine's validation component.
+ * 
+ * @todo More tests to cover the full interface of Doctrine_Validator_ErrorStack.
+ */
 class Doctrine_ValidatorTestCase extends Doctrine_UnitTestCase {
     public function prepareTables() {
         $this->tables[] = "ValidatorTest";
         parent::prepareTables();
     }
 
+    /**
+     * Tests correct type detection.
+     */
     public function testIsValidType() {
         $var = "123";
         $this->assertTrue(Doctrine_Validator::isValidType($var,"string"));
@@ -70,6 +78,9 @@ class Doctrine_ValidatorTestCase extends Doctrine_UnitTestCase {
         $this->assertTrue(Doctrine_Validator::isValidType($var,"object"));
     }
 
+    /**
+     * Tests Doctrine_Validator::validateRecord()
+     */
     public function testValidate2() {
         $test = new ValidatorTest();
         $test->mymixed = "message";
@@ -79,22 +90,23 @@ class Doctrine_ValidatorTestCase extends Doctrine_UnitTestCase {
         $validator = new Doctrine_Validator();
         $validator->validateRecord($test);
 
-        $stack = $validator->getErrorStack();
+        $stack = $test->getErrorStack();
 
-        $this->assertTrue(is_array($stack));
+        $this->assertTrue($stack instanceof Doctrine_Validator_ErrorStack);
 
-        $this->assertEqual($stack['mystring'], 'notnull');
+        $this->assertTrue(in_array(array('type' => 'notnull'), $stack['mystring']));
-        $this->assertEqual($stack['myemail2'], 'notblank');
+        $this->assertTrue(in_array(array('type' => 'notblank'), $stack['myemail2']));
-        $this->assertEqual($stack['myrange'],  'range');
+        $this->assertTrue(in_array(array('type' => 'range'), $stack['myrange']));
-        $this->assertEqual($stack['myregexp'], 'regexp');
+        $this->assertTrue(in_array(array('type' => 'regexp'), $stack['myregexp']));
         $test->mystring = 'str';
 
 
         $test->save();
     }
-    public function testEmailValidation() {
-    }
 
+    /**
+     * Tests Doctrine_Validator::validateRecord()
+     */
     public function testValidate() {
         $user = $this->connection->getTable("User")->find(4); 
 
@@ -112,29 +124,27 @@ class Doctrine_ValidatorTestCase extends Doctrine_UnitTestCase {
         $validator->validateRecord($user);
 
 
-        $stack = $validator->getErrorStack();
+        $stack = $user->getErrorStack();
-
-        $this->assertTrue(is_array($stack));
-        $this->assertEqual($stack["loginname"], 'length');
-        $this->assertEqual($stack["password"],  'length');
-        $this->assertEqual($stack["created"],   'type');
 
+        $this->assertTrue($stack instanceof Doctrine_Validator_ErrorStack);
+        $this->assertTrue(in_array(array('type' => 'length'), $stack['loginname']));
+        $this->assertTrue(in_array(array('type' => 'length'), $stack['password']));
+        $this->assertTrue(in_array(array('type' => 'type'), $stack['created']));
 
         $validator->validateRecord($email);
-        $stack = $validator->getErrorStack();
+        $stack = $email->getErrorStack();
-        $this->assertEqual($stack["address"], 'email');
+        $this->assertTrue(in_array(array('type' => 'email'), $stack['address']));
         $email->address = "arnold@example.com";
 
         $validator->validateRecord($email);
-        $stack = $validator->getErrorStack();
+        $stack = $email->getErrorStack();
 
-        $this->assertEqual($stack["address"], 'unique');
+        $this->assertTrue(in_array(array('type' => 'unique'), $stack['address']));
-
-        $email->isValid();
-        
-        $this->assertTrue($email->getErrorStack() instanceof Doctrine_Validator_ErrorStack);
     }
 
+    /**
+     * Tests the Email validator. (Doctrine_Validator_Email)
+     */
     public function testIsValidEmail() {
 
         $validator = new Doctrine_Validator_Email();
@@ -153,6 +163,9 @@ class Doctrine_ValidatorTestCase extends Doctrine_UnitTestCase {
 
     }
 
+    /**
+     * Tests saving records with invalid attributes.
+     */
     public function testSave() {
         $this->manager->setAttribute(Doctrine::ATTR_VLD, true);
         $user = $this->connection->getTable("User")->find(4);
@@ -161,6 +174,10 @@ class Doctrine_ValidatorTestCase extends Doctrine_UnitTestCase {
             $user->save();
         } catch(Doctrine_Validator_Exception $e) {
             $this->assertEqual($e->count(), 1);
+            $invalidRecords = $e->getInvalidRecords();
+            $this->assertEqual(count($invalidRecords), 1);
+            $stack = $invalidRecords[0]->getErrorStack();
+            $this->assertTrue(in_array(array('type' => 'length'), $stack['name']));
         }
 
         try {
@@ -179,10 +196,32 @@ class Doctrine_ValidatorTestCase extends Doctrine_UnitTestCase {
         $emailStack = $a[array_search($user->Email, $a)]->getErrorStack();
         $userStack  = $a[array_search($user, $a)]->getErrorStack();
 
-        $this->assertEqual($emailStack["address"], 'email');
+        $this->assertTrue(in_array(array('type' => 'email'), $emailStack['address']));
-        $this->assertEqual($userStack["name"], 'length');
+        $this->assertTrue(in_array(array('type' => 'length'), $userStack['name']));
         $this->manager->setAttribute(Doctrine::ATTR_VLD, false);
     }
 
+    /**
+     * Tests whether custom validation through template methods works correctly
+     * in descendants of Doctrine_Record.
+     */
+    public function testCustomValidation() {
+        $this->manager->setAttribute(Doctrine::ATTR_VLD, true);
+        $user = $this->connection->getTable("User")->find(4);
+         try {
+            $user->name = "I'm not The Saint";
+            $user->save();
+        } catch(Doctrine_Validator_Exception $e) {
+            $this->assertEqual($e->count(), 1);
+            $invalidRecords = $e->getInvalidRecords();
+            $this->assertEqual(count($invalidRecords), 1);
+            
+            $stack = $invalidRecords[0]->getErrorStack();
+            
+            $this->assertEqual($stack->count(), 1);
+            $this->assertTrue(in_array(array('type' => 'notTheSaint'), $stack['name']));
+        }
+        $this->manager->setAttribute(Doctrine::ATTR_VLD, false);
+    }
 }
 ?>

tests/classes.php

@@ -97,6 +97,13 @@ class User extends Entity {
         $this->hasMany("Group","Groupuser.group_id");
         $this->setInheritanceMap(array("type"=>0));
     }
+    /** Custom validation */
+    public function validate() {
+        // Allow only one name!
+        if ($this->name !== 'The Saint') {
+            $this->errorStack->add('name', 'notTheSaint');
+        }
+    }
 }
 class Groupuser extends Doctrine_Record {
     public function setTableDefinition() {
@@ -541,4 +548,5 @@ class BoardWithPosition extends Doctrine_Record {
         $this->hasOne("CategoryWithPosition as Category", "BoardWithPosition.category_id");
     }
 }
+
 ?>