ZF-3003: make properties of Zend_OpenId_Consumer protected to enable sub-classing

Description

We have need to sub-class Zend_OpenId_Consumer, but when we do so we can't succinctly change the storage nor the dumbMode properties because they are declared private. making them protected lets us customize these without having to redefine our own constructor.

Comments

$_starage and $_dumbMode where made private by design. They can be easily set through constructor and shouldn't be changed after that. I don't see any problem to set it from overridden constructor.

class My_OpenId extends Zend_OpenId_Consumer { function __construct() { parent::__construct(new DB_Storage(), true); } }

It is not a big problem to make them protected, but I don't see any reason. Could you please explain why constructor doesn't work for you?

our problem was that calling parent::__construct from within an overridden constructor doesn't seem to work.

class Sfx_OpenId_Consumer extends Zend_OpenId_Consumer { public function __construct(Zend_OpenId_Consumer_Storage $storage = null, $dumbMode = false, $logging = false) { parent::__construct($storage, $dumbMode); $this->_logging = $logging; }

[10-Apr-2008 07:27:00] PHP Fatal error: Call to a member function getDiscoveryInfo() on a non-object in Sfx/OpenId/Consumer.php on line 210

or maybe it's because we're also overriding the _discovery method. in any case, there's a mix of protected methods using private properties which I think caused confusion for us. if the methods are going to be protected, the properties they use should probably likewise be protected so that sub-classes can use both consistently.

I see. Your constructor works fine and you set private $_storage property of Zend_OpenId_Consumer, however you probably cannot access it from Sfx_OpenId_Consumer.

Why do you need to override _discovery()? Do you implement XRI and/or Yadis discovery or some custom discovery scheme?

we actually overrode _discovery() and _associate() to fix the SHA256->SHA1 fallback behavior and to add a bunch of logging/debugging to help us discover ZF-3079.

so really, we could have, and probably should have, just modified Zend_OpenId_Consumer class directly and submitted our changes as patches, but we weren't sure at the time if we were right in changing the code, so we extended from it. honestly, this bug is not nearly as high priority as ZF-3079. just more like a convenience thing.

So could you give me more details about ZF-3079. I'm going to fix it with top priority, but for now I don't know how to reproduce the bug.

Do you still need protected properties after ZF-3079 fix?

IMO, it would still be useful, yeah ...

the protected nature of _discovery and _associate is sorta useless if the sub-class can't access the private properties to work properly anyway.

we're still overriding _associate to change it's SHA256->SHA1 fallback behavior, plus we've added some error-handling and logging to our centralized log location, so we're still sub-classing Consumer for other things, and I think others will find it useful to sub-class Consumer as well, so it will be helpful for all to be able to access and use all of Consumer's properties.

Marking as fixed for next minor release pending merge of changes to release-1.5 branch.

Updating for the 1.6.0 release.