Zend Framework

setReadOnly not recursive

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.7 Preview Release
  • Fix Version/s: 1.8.0
  • Component/s: Zend_Config
  • Labels:
    None

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.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Tomoaki Kosugi added a comment -

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();
            }
        }
    }
Show
Tomoaki Kosugi added a comment - 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();
            }
        }
    }
Hide
Permalink
Sebastian Krebs added a comment - - edited

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.

Show
Sebastian Krebs added a comment - - edited 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.
Hide
Permalink
Tomoaki Kosugi added a comment -

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;

base.ini
admin = foo
controllers.front = index
controllers.some = bar
extra.ini
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);
Show
Tomoaki Kosugi added a comment - 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;
base.ini
admin = foo
controllers.front = index
controllers.some = bar
extra.ini
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);
Hide
Permalink
Rob Allen added a comment -

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.

Show
Rob Allen added a comment - 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.
Hide
Permalink
Tomoaki Kosugi added a comment -

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

Show
Tomoaki Kosugi added a comment - Fair enough. I'll leave it up to you. Thanks
Hide
Permalink
Rob Allen added a comment -

Fixed in svn r14408.

Show
Rob Allen added a comment - Fixed in svn r14408.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Zend Framework. Try JIRA - bug tracking software for your team.