ZF-9607: Zend_Form_Element belongsTo Issues

Description

This Task is for belongsTo related issues in several Zend_Form methods mainly isValid isValidPartial getValues getValidValues populate ...

This first patch is fixing isValid to work with belongsTo, and also make sure that validated ElementValues are stripped from data to validate what the issue ZF-9679 was about.

For this i added a method _dissolveArrayUnsetKey which will unset the key in a given array after dissolving the given arrayPath.

[#patch1] isValid fix for belongsTo [#patch2] testIsValidDiscardsValidatedValues [#patch3] isValidPartial fix for belongsTo [#patch4] populate setDefaults fix for belongsTo [#patch5] testSetDefaultsDiscardsPopulatedValues [#patch6] getValues fix for belongsTo [#patch7] getValidValues fix for belongsTo

Changed parts in isValid, before Patch


$valid      = true;

        if ($this->isArray()) {
            $data = $this->_dissolveArrayValue($data, $this->getElementsBelongTo());
        }
 
        foreach ($this->getElements() as $key => $element) {
            if (null !== $translator && !$element->hasTranslator()) {
                $element->setTranslator($translator);
            }
            if (!isset($data[$key])) {
                $valid = $element->isValid(null, $data) && $valid;
            } else {
                $valid = $element->isValid($data[$key], $data) && $valid;
            }
        }

After Patch


$valid      = true;
        $eBelongTo  = null;

        if ($this->isArray()) {
            $eBelongTo = $this->getElementsBelongTo();
            $data = $this->_dissolveArrayValue($data, $eBelongTo);
        }
 
        foreach ($this->getElements() as $key => $element) {
            if (null !== $translator && !$element->hasTranslator()) {
                $element->setTranslator($translator);
            }
            $check = $data;
            if (($belongsTo = $element->getBelongsTo()) !== $eBelongTo) {
                $check = $this->_dissolveArrayValue($data, $belongsTo);
            }
            if (!isset($check[$key])) {
                $valid = $element->isValid(null, $check) && $valid;
            } else {
                $valid = $element->isValid($check[$key], $check) && $valid;
                $data = $this->_dissolveArrayUnsetKey($data, $belongsTo, $key);
            }
        }

{anchor:patch1} h4.Patch 1 isValid fix for belongsTo The first patch is for isValid, Unit Test included, this also obsoletes ZF-9679.


Index: tests/Zend/Form/FormTest.php
===================================================================
--- tests/Zend/Form/FormTest.php        (Revision 21863)
+++ tests/Zend/Form/FormTest.php        (Arbeitskopie)
@@ -1579,7 +1579,27 @@
         $this->assertSame($this->form->getValidValues($data['invalid']), $data['partial']);
     }
 
+    public function _setup9607()
+    {
+        $this->form->addElement('text', 'foo')
+                   ->foo->setBelongsTo('bar[quo]')
+                        ->setRequired(true)
+                        ->addValidator('Identical',
+                                       false,
+                                       'foo Value');
 
+        $data = array('valid' => array('bar' =>
+                                       array('quo' =>
+                                             array('foo' => 'foo Value'))));
+        return $data;
+    }
+
+    public function testIsValidWithBelongsTo()
+    {
+        $data = $this->_setup9607();
+        $this->assertTrue($this->form->isValid($data['valid']));
+    }
+
     // Display groups
 
     public function testCanAddAndRetrieveSingleDisplayGroups()
Index: library/Zend/Form.php
===================================================================
--- library/Zend/Form.php       (Revision 21863)
+++ library/Zend/Form.php       (Arbeitskopie)
@@ -2005,6 +2005,34 @@
     }
 
     /**
+     * Given an array, an optional arrayPath and a key this method
+     * dissolves the arrayPath and unsets the key within the array
+     * if it exists.
+     * 
+     * @param array $array 
+     * @param string|null $arrayPath
+     * @param string $key
+     * @return array
+     */
+    protected function _dissolveArrayUnsetKey($array, $arrayPath, $key)
+    {
+        $unset =& $array;
+        $path  = trim(strtr((string)$arrayPath, array('[' => '/', ']' => '')), '/');
+        $segs  = ('' !== $path) ? explode('/', $path) : array();
+        
+        foreach ($segs as $seg) {
+            if (!array_key_exists($seg, (array)$unset)) {
+                return $array;
+            }
+            $unset =& $unset[$seg];
+        }
+        if (array_key_exists($key, (array)$unset)) {
+            unset($unset[$key]);
+        }
+        return $array;
+    }
+
+    /**
      * Converts given arrayPath to an array and attaches given value at the end of it.
      *
      * @param  mixed $value The value to attach
@@ -2044,19 +2072,26 @@
         }
         $translator = $this->getTranslator();
         $valid      = true;
+        $eBelongTo  = null;
 
         if ($this->isArray()) {
-            $data = $this->_dissolveArrayValue($data, $this->getElementsBelongTo());
+            $eBelongTo = $this->getElementsBelongTo();
+            $data = $this->_dissolveArrayValue($data, $eBelongTo);
         }
 
         foreach ($this->getElements() as $key => $element) {
             if (null !== $translator && !$element->hasTranslator()) {
                 $element->setTranslator($translator);
             }
-            if (!isset($data[$key])) {
-                $valid = $element->isValid(null, $data) && $valid;
+            $check = $data;
+            if (($belongsTo = $element->getBelongsTo()) !== $eBelongTo) {
+                $check = $this->_dissolveArrayValue($data, $belongsTo);
+            }
+            if (!isset($check[$key])) {
+                $valid = $element->isValid(null, $check) && $valid;
             } else {
-                $valid = $element->isValid($data[$key], $data) && $valid;
+                $valid = $element->isValid($check[$key], $check) && $valid;
+                $data = $this->_dissolveArrayUnsetKey($data, $belongsTo, $key);
             }
         }
         foreach ($this->getSubForms() as $key => $form) {

{anchor:patch2} h4.Patch 2 testIsValidDiscardsValidatedValues

This second patch is an Unit Test only to ensure that isValid unsets already validated ElementValues and does not carry them to SubForms, like issued in ZF-9679


Index: tests/Zend/Form/FormTest.php
===================================================================
--- tests/Zend/Form/FormTest.php    (Revision 21873)
+++ tests/Zend/Form/FormTest.php    (Arbeitskopie)
@@ -1528,6 +1528,20 @@
         $this->assertRegexp('/value=.foo Value./', $this->form->render());
     }
 
+    /**
+     * @group ZF-9679
+     */
+    public function testIsValidDiscardsValidatedValues()
+    {
+        $this->form->addElement('text', 'foo');
+        $this->form->addSubForm(new Zend_Form_SubForm(), 'bar')
+                   ->bar->addElement('text', 'foo')
+                        ->foo->setAllowEmpty(true)
+                             ->addValidator('Identical', true, '');
+
+        $this->assertTrue($this->form->isValid(array('foo' => 'foo Value')));
+    }
+
     public function _setup9350()
     {
         $this->form->addSubForm(new Zend_Form_SubForm(), 'foo')

{anchor:patch3} h4.Patch 3 isValidPartial fix for belongsTo This is the same patch as Patch 1 only for isValidPartial


Index: tests/Zend/Form/FormTest.php
===================================================================
--- tests/Zend/Form/FormTest.php        (Revision 21875)
+++ tests/Zend/Form/FormTest.php        (Arbeitskopie)
@@ -1579,7 +1579,14 @@
         $this->assertSame($this->form->getValidValues($data['invalid']), $data['partial']);
     }
 
+    public function testIsValidPartialWithBelongsTo()
+    {
+        $data = $this->_setup9607();
+        $this->assertTrue($this->form->isValidPartial($data['valid']));
+        $this->assertSame('foo Value', $this->form->foo->getValue());
+    }
 
+
     // Display groups
 
     public function testCanAddAndRetrieveSingleDisplayGroups()
Index: library/Zend/Form.php
===================================================================
--- library/Zend/Form.php       (Revision 21875)
+++ library/Zend/Form.php       (Arbeitskopie)
@@ -2090,19 +2090,27 @@
      */
     public function isValidPartial(array $data)
     {
+        $eBelongTo  = null;
+
         if ($this->isArray()) {
-            $data = $this->_dissolveArrayValue($data, $this->getElementsBelongTo());
+            $eBelongTo = $this->getElementsBelongTo();
+            $data = $this->_dissolveArrayValue($data, $eBelongTo);
         }
 
         $translator        = $this->getTranslator();
         $valid             = true;
 
         foreach ($this->getElements() as $key => $element) {
-            if (isset($data[$key])) {
+            $check = $data;
+            if (($belongsTo = $element->getBelongsTo()) !== $eBelongTo) {
+                $check = $this->_dissolveArrayValue($data, $belongsTo);
+            }
+            if (isset($check[$key])) {
                 if (null !== $translator && !$element->hasTranslator()) {
                     $element->setTranslator($translator);
                 }
-                $valid = $element->isValid($data[$key], $data) && $valid;
+                $valid = $element->isValid($check[$key], $check) && $valid;
+                $data = $this->_dissolveArrayUnsetKey($data, $belongsTo, $key);
             }
         }
         foreach ($this->getSubForms() as $key => $form) {

{anchor:patch4} h4.Patch 4 fix for setDefaults to work with belongsTo

this obsoletes ZF-9666


Index: tests/Zend/Form/FormTest.php
===================================================================
--- tests/Zend/Form/FormTest.php        (Revision 21877)
+++ tests/Zend/Form/FormTest.php        (Arbeitskopie)
@@ -1579,7 +1579,14 @@
         $this->assertSame($this->form->getValidValues($data['invalid']), $data['partial']);
     }
 
+    public function testPopulateWithBelongsTo()
+    {
+        $data = $this->_setup9607();
+        $this->form->populate($data['valid']);
+        $this->assertSame('foo Value', $this->form->foo->getValue());
+    }
 
+
     // Display groups
 
     public function testCanAddAndRetrieveSingleDisplayGroups()
Index: library/Zend/Form.php
===================================================================
--- library/Zend/Form.php       (Revision 21877)
+++ library/Zend/Form.php       (Arbeitskopie)
@@ -1226,13 +1226,21 @@
      */
     public function setDefaults(array $defaults)
     {
+        $eBelongTo = null;
+
         if ($this->isArray()) {
-            $defaults = $this->_dissolveArrayValue($defaults, $this->getElementsBelongTo());
+            $eBelongTo = $this->getElementsBelongTo();
+            $defaults = $this->_dissolveArrayValue($defaults, $eBelongTo);
         }
         foreach ($this->getElements() as $name => $element) {
-            if (array_key_exists($name, $defaults)) {
-                $this->setDefault($name, $defaults[$name]);
+            $check = $defaults;
+            if (($belongsTo = $element->getBelongsTo()) !== $eBelongTo) {
+                $check = $this->_dissolveArrayValue($defaults, $belongsTo);
             }
+            if (array_key_exists($name, $check)) {
+                $this->setDefault($name, $check[$name]);
+                $defaults = $this->_dissolveArrayUnsetKey($defaults, $belongsTo, $name);
+            }
         }
         foreach ($this->getSubForms() as $name => $form) {
             if (!$form->isArray() && array_key_exists($name, $defaults)) {

{anchor:patch5} h4.Patch 5 make sure populated Values are not carried to SubForms


Index: tests/Zend/Form/FormTest.php
===================================================================
--- tests/Zend/Form/FormTest.php    (Revision 21879)
+++ tests/Zend/Form/FormTest.php    (Arbeitskopie)
@@ -1528,6 +1528,21 @@
         $this->assertRegexp('/value=.foo Value./', $this->form->render());
     }
 
+    /**
+     * @group ZF-9666
+     */
+    public function testSetDefaultsDiscardsPopulatedValues()
+    {
+        $this->form->addElement('text', 'foo');
+        $this->form->addSubForm(new Zend_Form_SubForm(), 'bar')
+                   ->bar->addElement('text', 'foo');
+
+        $this->form->populate(array('foo' => 'foo Value'));
+        $html = $this->form->setView($this->getView())
+                           ->render();
+        $this->assertEquals(1, preg_match_all('/foo Value/', $html, $matches));
+    }
+
     public function _setup9350()
     {
         $this->form->addSubForm(new Zend_Form_SubForm(), 'foo')

{anchor:patch6} h4.Patch 6 getValues fix for belongsTo


Index: tests/Zend/Form/FormTest.php
===================================================================
--- tests/Zend/Form/FormTest.php        (Revision 21883)
+++ tests/Zend/Form/FormTest.php        (Arbeitskopie)
@@ -1579,7 +1579,14 @@
         $this->assertSame($this->form->getValidValues($data['invalid']), $data['partial']);
     }
 
+    public function testGetValuesWithBelongsTo()
+    {
+        $data = $this->_setup9607();
+        $this->form->populate($data['valid']);
+        $this->assertSame($data['valid'], $this->form->getValues());
+    }
 
+
     // Display groups
 
     public function testCanAddAndRetrieveSingleDisplayGroups()
Index: library/Zend/Form.php
===================================================================
--- library/Zend/Form.php       (Revision 21883)
+++ library/Zend/Form.php       (Arbeitskopie)
@@ -1301,9 +1301,22 @@
     public function getValues($suppressArrayNotation = false)
     {
         $values = array();
+        $eBelongTo = null;
+        
+        if ($this->isArray()) {
+            $eBelongTo = $this->getElementsBelongTo();
+        }
+        
         foreach ($this->getElements() as $key => $element) {
             if (!$element->getIgnore()) {
-                $values[$key] = $element->getValue();
+                $merge = array();
+                if (($belongsTo = $element->getBelongsTo()) !== $eBelongTo) {
+                    if ('' !== (string)$belongsTo) {
+                        $key = $belongsTo . '[' . $key . ']';
+                    }
+                }
+                $merge = $this->_attachToArray($element->getValue(), $key);
+                $values = array_merge_recursive($values, $merge);
             }
         }
         foreach ($this->getSubForms() as $key => $subForm) {

{anchor:patch7} h4.Patch 7 getValidValues with belongsTo

Index: tests/Zend/Form/FormTest.php
===================================================================
--- tests/Zend/Form/FormTest.php        (Revision 21885)
+++ tests/Zend/Form/FormTest.php        (Arbeitskopie)
@@ -1579,7 +1579,13 @@
         $this->assertSame($this->form->getValidValues($data['invalid']), $data['partial']);
     }
 
+    public function testGetValidValuesWithBelongsTo()
+    {
+        $data = $this->_setup9607();
+        $this->assertSame($data['partial'], $this->form->getValidValues($data['invalid']));
+    }
 
+
     // Display groups
 
     public function testCanAddAndRetrieveSingleDisplayGroups()
Index: library/Zend/Form.php
===================================================================
--- library/Zend/Form.php       (Revision 21885)
+++ library/Zend/Form.php       (Arbeitskopie)
@@ -1331,15 +1331,29 @@
      */
     public function getValidValues($data, $suppressArrayNotation = false)
     {
+        $values = array();
+        $eBelongTo = null;
+
         if ($this->isArray()) {
-            $data = $this->_dissolveArrayValue($data, $this->getElementsBelongTo());
+            $eBelongTo = $this->getElementsBelongTo();
+            $data = $this->_dissolveArrayValue($data, $eBelongTo);
         }
-        $values = array();
+        
         foreach ($this->getElements() as $key => $element) {
-            if (isset($data[$key])) {
-                if($element->isValid($data[$key], $data)) {
-                    $values[$key] = $element->getValue();
+            $check = $data;
+            if (($belongsTo = $element->getBelongsTo()) !== $eBelongTo) {
+                $check = $this->_dissolveArrayValue($data, $belongsTo);
+            }
+            if (isset($check[$key])) {
+                if($element->isValid($check[$key], $check)) {
+                    $merge = array();
+                    if ($belongsTo !== $eBelongTo && '' !== (string)$belongsTo) {
+                            $key = $belongsTo . '[' . $key . ']';
+                    }
+                    $merge = $this->_attachToArray($element->getValue(), $key);
+                    $values = array_merge_recursive($values, $merge);
                 }
+                $data = $this->_dissolveArrayUnsetKey($data, $belongsTo, $key);
             }
         }
         foreach ($this->getSubForms() as $key => $form) {

Comments

Added patch for isValid

Change Type to Patch.

Fixed whitespaces within patch.

added Unit Test to respect ZF-9679

Added Patch and Unit Test for isValidPartial fix belongsTo

Added Patch 4 including Unit Test as fix for belongsTo within setDefaults. This obsoletes the patch from ZF-9666

Added patch 5, an Unit Test to make sure ZF-9666 is satisfied.

Added Patch 6 for getValues fix for belongsTo including Unit Test

Added Patch and Unit Test for getValidValues with belongsTo

This is missing in Patch 7


    public function _setup9607()
    {
        $this->form->addElement('text', 'foo')
                   ->foo->setBelongsTo('bar[quo]')
                        ->setRequired(true)
                        ->addValidator('Identical',
                                       false,
                                       'foo Value');

        $this->form->addElement('text', 'quo')
                   ->quo->setBelongsTo('bar[quo]')
                        ->addValidator('Identical',
                                       false,
                                       'quo Value');

        $data = array('valid' => array('bar' =>
                                       array('quo' =>
                                             array('foo' => 'foo Value',
                                                   'quo' => 'quo Value'))),
                      'invalid' => array('bar' =>
                                         array('quo' =>
                                               array('foo' => 'foo Invalid',
                                                     'quo' => 'quo Value'))),
                      'partial' => array('bar' =>
                                         array('quo' =>
                                               array('quo' => 'quo Value'))));
        return $data;
    }

Patches applied in trunk and 1.10 release branch.

Applied missing TestSetup in trunk and 1.10 release branch.

This update resulted in changes of $context variable in methods isValid() in all of the specified custom validators. When we call method isValid() method of Zend_Form class, every element of the form is being deleted from the context: one-by-one, in order. Thus, I can't use $context in isValid() method for those elements that are specified before the current element. For instance, if I have 2 fields: "password" and "confirm_password" - I can't use $context in my custom validator, in the method isValid() for the field "password" because $context['password'] is unset (!) - it is not longer in the context.

This happens due to these lines in Zend_Form class, since $key is always specified and $unset refers to $array which is our current context:

if (array_key_exists($key, (array)$unset)) { unset($unset[$key]); }

So, I had to overwrite _dissolveArrayUnsetKey() method this way in my Form Class:

protected function _dissolveArrayUnsetKey($array, $arrayPath, $key) { return $array; }

Lex you are right that was not intended, could you check if this patch solves the problem


Index: library/Zend/Form.php
===================================================================
--- library/Zend/Form.php       (Revision 21922)
+++ library/Zend/Form.php       (Arbeitskopie)
@@ -2120,7 +2120,7 @@
             $eBelongTo = $this->getElementsBelongTo();
             $data = $this->_dissolveArrayValue($data, $eBelongTo);
         }
-
+        $context = $data;
         foreach ($this->getElements() as $key => $element) {
             if (null !== $translator && !$element->hasTranslator()) {
                 $element->setTranslator($translator);
@@ -2130,9 +2130,9 @@
                 $check = $this->_dissolveArrayValue($data, $belongsTo);
             }
             if (!isset($check[$key])) {
-                $valid = $element->isValid(null, $check) && $valid;
+                $valid = $element->isValid(null, $context) && $valid;
             } else {
-                $valid = $element->isValid($check[$key], $check) && $valid;
+                $valid = $element->isValid($check[$key], $context) && $valid;
                 $data = $this->_dissolveArrayUnsetKey($data, $belongsTo, $key);
             }
         }

(my name was Lex in the prev. post. Sorry, for this confusion)

Christian, this patch works like a charm and solves the problem. I don't need to overwrite "_dissolveArrayUnsetKey" anymore.

Thank you.

Additional patches applied to trunk and 1.10 release for isValid isValidPartial and getValidValues to make sure context data is kept.