Zend Framework

Add a seal() method to Zend_Config

Details

  • Type: New Feature New Feature
  • Status: Resolved Resolved
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.0.2
  • Fix Version/s: 1.0.3
  • Component/s: Zend_Config
  • Labels:
    None

Description

I tried to use the new merge() method, but had the problem, that our main config object was read-only. Adding allowModifications to the Zend_Config constructor was easy, but now it's also writable outside of our front-controller. It would be nice if Zend_Config could be sealed (setting Zend_Config::_allowModifications to false) after the init phase.

Activity

Hide
Thomas Weidner added a comment -

Assigned to Rob

Show
Thomas Weidner added a comment - Assigned to Rob
Hide
Darby Felton added a comment -

I like the idea of post-construction configurability, but I would instead recommend the solution to be adding a setAllowModifications(boolean $enabled) method or similar, though maybe there is better name for $enabled.

Show
Darby Felton added a comment - I like the idea of post-construction configurability, but I would instead recommend the solution to be adding a setAllowModifications(boolean $enabled) method or similar, though maybe there is better name for $enabled.
Hide
Rob Allen added a comment -

setAllowModifications() implies that a Zend_Config object can be made writable at will.

Maybe something like setReadOnly() so that it's a one-way operation ?

Show
Rob Allen added a comment - setAllowModifications() implies that a Zend_Config object can be made writable at will. Maybe something like setReadOnly() so that it's a one-way operation ?
Hide
Nico Edtinger added a comment -

I also think disallowing modifications should be one-way as it's something you'd do to enforce code standards. The name (seal() or setReadOnly()) doesn't matter to me.

Show
Nico Edtinger added a comment - I also think disallowing modifications should be one-way as it's something you'd do to enforce code standards. The name (seal() or setReadOnly()) doesn't matter to me.
Hide
Darby Felton added a comment -

If I understand correctly, this is trivially accomplished by extending Zend_Config:

class MyConfig extends Zend_Config
{
    public function seal()
    {
        $this->_allowModifications = false;
        return $this;
    }
}

Considering the above comments and the fact that it could also be harmful to allow only seal()/setReadOnly() - many people would have the opposite problem (needing write access to a locked object) - it would seem that this is best done in userland instead of bloating the framework component with possibly problematic functionality.

Show
Darby Felton added a comment - If I understand correctly, this is trivially accomplished by extending Zend_Config:
class MyConfig extends Zend_Config
{
    public function seal()
    {
        $this->_allowModifications = false;
        return $this;
    }
}
Considering the above comments and the fact that it could also be harmful to allow only seal()/setReadOnly() - many people would have the opposite problem (needing write access to a locked object) - it would seem that this is best done in userland instead of bloating the framework component with possibly problematic functionality.
Hide
Nico Edtinger added a comment -

It's not that easy because you need to apply the change recursively. And that's currently not possible because __construct() and __set() use new Zend_Config instead of new self and therefor all childs won't have a seal method.

Show
Nico Edtinger added a comment - It's not that easy because you need to apply the change recursively. And that's currently not possible because __construct() and __set() use new Zend_Config instead of new self and therefor all childs won't have a seal method.
Hide
Darby Felton added a comment -

Yes, of course, I missed that. What would the implementation look like, now that I committed SVN r6767?

Show
Darby Felton added a comment - Yes, of course, I missed that. What would the implementation look like, now that I committed SVN r6767?
Hide
Rob Allen added a comment -

Hi Nico,

Does Darby's change in SVN r6767 allow you to solve your problem if you write the userland class :

class MyConfig extends Zend_Config
{
    public function seal()
    {
        $this->_allowModifications = false;
        return $this;
    }
}
Show
Rob Allen added a comment - Hi Nico, Does Darby's change in SVN r6767 allow you to solve your problem if you write the userland class :
class MyConfig extends Zend_Config
{
    public function seal()
    {
        $this->_allowModifications = false;
        return $this;
    }
}
Hide
Rob Allen added a comment -

I'd like to resolve this issue one way or another if possible.

Choices are:

1. Resolve as "Won't Fix" or "Not an Issue" and users will have to create a user-land class.
2. Add a setReadOnly() function to Zend_Config.

Personally, I'm tending towards (2) as merging multiple config files into a single Zend_Config doesn't strike me as that unusual a use-case and with merge() we are implicitly endorsing this use-case anyway. May as well tidy up after ourselves!

Thoughts?

Show
Rob Allen added a comment - I'd like to resolve this issue one way or another if possible. Choices are: 1. Resolve as "Won't Fix" or "Not an Issue" and users will have to create a user-land class. 2. Add a setReadOnly() function to Zend_Config. Personally, I'm tending towards (2) as merging multiple config files into a single Zend_Config doesn't strike me as that unusual a use-case and with merge() we are implicitly endorsing this use-case anyway. May as well tidy up after ourselves! Thoughts?
Hide
Rob Allen added a comment -

Added setReadOnly() in svn 6967 on the trunk and svn 6968 on the release-1.0 branch.

Show
Rob Allen added a comment - Added setReadOnly() in svn 6967 on the trunk and svn 6968 on the release-1.0 branch.
Hide
Wil Sinclair added a comment -

Fixing Fix Version to follow issue tracker conventions.

Show
Wil Sinclair added a comment - Fixing Fix Version to follow issue tracker conventions.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: