ZF-4728: setReadOnly not recursive

Issue Type:
Bug
Created:
2008-10-28T06:24:29.000+0000
Last Updated:
2009-03-21T08:38:43.000+0000
Status:
Resolved
Fix version(s):
  • 1.8.0 (30/Apr/09)
Reporter:
Sebastian Krebs (kingcrunch)
Assignee:
Rob Allen (rob)
Tags:
  • Zend_Config
Related issues:
Attachments:

Description

I have to say, that i dont know, if this behaviour is mentioned as issue before. ;)

I thought, that the method setReadOnly() acts like the constructor-parameter allowModifications, so Im able to build up a configuration from different sources and merge it into one, but the method only locks the object itself, not the containing objects, as it will be done when setting the constructor-parameter to false.

$array = array ('key1'=>1,'key2'=>array('a'=>1,'b'=>2));
$c = new Zend_Config($array,false);
$c->key2->a = 3;

Will throw an Exception as expected.

$array = array ('key1'=>1,'key2'=>array('a'=>1,'b'=>2));
$c = new Zend_Config($array,true);
$c->setReadOnly();
$c->key2->a = 3;

Will not throw an Exception (as not expected).

Changing setReadOnly()

    public function setReadOnly()
    {
        $this->_allowModifications = false;
    }

to

    public function setReadOnly()
    {
        $this->_allowModifications = false;
        foreach (this->_data as $value) {
          if ($value instanceof Zend_Config) {
            $value->setReadOnly();
          }
        }
    }

will also lock nested objects and will make the behaviour equal to that of the constructor-parameter. And makes the method more useful ;) I dont see, why i should be able to lock a config, but are able to change deeper config settings.

Comments

Posted by Tomoaki Kosugi (noopable) on 2009-03-13T04:43:21.000+0000

I usually use this method to apply readOnly to the current iteration's elements.Because i separate readonly segment and allowModificatable segment . I prefer to add a method setReadOnlyRecursively() as follows for more flexibilty and backward compatibility.


    public function setReadOnlyRecursively()
    {
        $this->setReadOnly();
        foreach ($this as $value) {
            if ($value instanceof Zend_Config) {
                $value->setReadOnlyRecursively();
            }
        }
    }

Posted by Sebastian Krebs (kingcrunch) on 2009-03-16T15:34:45.000+0000

BC is an argument, i see. But as segment i understand that as the current element including its childs. In my eyes its seems confusing, that I can set an element as read only, but its childs are not. I dont mind to use another method instead, but it should be possible to do so. An optional parameter will also not break BC.

public function setReadOnly($recursive = false)
    {
        $this->_allowModifications = false;
        if ($recursive) {
          foreach (this->_data as $value) {
            if ($value instanceof Zend_Config) {
              $value->setReadOnly();
            }
          }
        }
    }

But keep the BC will not solve the different behaviours beetween constructor-parameter and method.

Posted by Tomoaki Kosugi (noopable) on 2009-03-16T18:07:45.000+0000

Yes , I think that it would be advisable to accord the natual code priority over BC.So, I'm fine with that an optional parameter set to true. So,like this. I corrected some point.


public function setReadOnly($recursive = true)
    {
        $this->_allowModifications = false;
        if ($recursive) {
          foreach ($this->_data as $value) {
            if ($value instanceof Zend_Config) {
              $value->setReadOnly($recursive);
            }
          }
        }
    }

By the way ,actually setting readonly partially seems confusing .By ordinary a segment is mentioned as the element including its children.

But we don't have any options to separately set readonly to the value element in the config.Therefore, in this case, this->setReadOnly's targets are that this->_data excluding objects instanceof Zend_Config. There is a difference between iterator and recursive iterator.

For instance;


admin = foo
controllers.front = index
controllers.some = bar


controllers.some = special

In this case,"$config->setReadOnly(false);" allows me to set readOnly to admin, and allowModification to controllers.


$config = Zend_Config_Ini("base.ini", null, true);
$extraConf = Zend_Config_Ini("extra.ini", null, false);
$config->setReadOnly(false);
$config->controllers->merge($extraConf->controllers);

Posted by Rob Allen (rob) on 2009-03-17T02:02:50.000+0000

I see this as a bug that should be fixed rather than B/C behaviour that should be maintained. Clearly if you call setReadOnly() you expect that the entire config object is now read only and cannot be modified.

Posted by Tomoaki Kosugi (noopable) on 2009-03-17T02:55:00.000+0000

Fair enough. I'll leave it up to you. Thanks

Posted by Rob Allen (rob) on 2009-03-21T08:38:43.000+0000

Fixed in svn r14408.