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

Issue Type: Improvement Created: 2008-03-31T19:51:04.000+0000 Last Updated: 2008-09-02T10:39:17.000+0000 Status: Resolved Fix version(s): - 1.6.0 (02/Sep/08)

Reporter: Luke Crouch (lcrouch) Assignee: Dmitry Stogov (dmitry) Tags: - Zend_OpenId

Related issues: Attachments:


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.


Posted by Dmitry Stogov (dmitry) on 2008-04-10T09:05:46.000+0000

$_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?

Posted by Luke Crouch (lcrouch) on 2008-04-10T09:33:54.000+0000

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.

Posted by Dmitry Stogov (dmitry) on 2008-04-10T09:46:40.000+0000

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?

Posted by Luke Crouch (lcrouch) on 2008-04-10T10:43:36.000+0000

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.

Posted by Dmitry Stogov (dmitry) on 2008-04-10T10:56:37.000+0000

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.

Posted by Dmitry Stogov (dmitry) on 2008-04-14T03:09:38.000+0000

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

Posted by Luke Crouch (lcrouch) on 2008-04-14T09:33:46.000+0000

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.

Posted by Darby Felton (darby) on 2008-04-21T13:56:35.000+0000

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

Posted by Wil Sinclair (wil) on 2008-09-02T10:39:17.000+0000

Updating for the 1.6.0 release.

Have you found an issue?

See the Overview section for more details.


© 2006-2016 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.