Zend\Authentication\Adapter\Http::_challengeClient() should be public #4653

Closed
wants to merge 0 commits into
from

Conversation

Projects
None yet
3 participants
@corentin-larose
Contributor

corentin-larose commented Jun 13, 2013

Zend\Authentication\Adapter\Http::_challengeClient() should be public since one could need to call it this way:

$auth->getAdapter()->challengeClient();

for instance when the authenticated user tries to access a resource for which he is not granted by ACLs.

@mwillbanks

This comment has been minimized.

Show comment Hide comment
@mwillbanks

mwillbanks Jun 19, 2013

Contributor

To preserve breaks in the API for folks that might be extending the Adapter\Http, you should provide a deprecated method in the event someone was previously overloading it. Additionally you should not modify the code already calling it. Unless @weierophinney disagrees and that this is an acceptable BC break in the API for potentially 2.3.

Only issue I really see is that it is part of the authentication component and in those cases these tend to be overwritten a bit more.

/**
 * @deprecated
 */
protected function _challengeClient()
{
    return $this->challengeClient();
}
Contributor

mwillbanks commented Jun 19, 2013

To preserve breaks in the API for folks that might be extending the Adapter\Http, you should provide a deprecated method in the event someone was previously overloading it. Additionally you should not modify the code already calling it. Unless @weierophinney disagrees and that this is an acceptable BC break in the API for potentially 2.3.

Only issue I really see is that it is part of the authentication component and in those cases these tend to be overwritten a bit more.

/**
 * @deprecated
 */
protected function _challengeClient()
{
    return $this->challengeClient();
}
@weierophinney

This comment has been minimized.

Show comment Hide comment
@weierophinney

weierophinney Jun 28, 2013

Member

@corentin-larose Agreed with @mwillbanks -- keep a stub _challengeClient() method in place, but mark it as deprecated via an annotation, and also have it raise an E_USER_DEPRECATED error indicating users should convert their code to use challengeClient() directly.

Member

weierophinney commented Jun 28, 2013

@corentin-larose Agreed with @mwillbanks -- keep a stub _challengeClient() method in place, but mark it as deprecated via an annotation, and also have it raise an E_USER_DEPRECATED error indicating users should convert their code to use challengeClient() directly.

@corentin-larose

This comment has been minimized.

Show comment Hide comment
@corentin-larose

corentin-larose Jul 11, 2013

Contributor

@weierophinney @mwillbanks, I made the changes for Zend\Authentication\Adapter\Http::_challengeClient() (#4653), I gonna a issue another pull request.

Hope it gonna do the trick.

Cheers

Contributor

corentin-larose commented Jul 11, 2013

@weierophinney @mwillbanks, I made the changes for Zend\Authentication\Adapter\Http::_challengeClient() (#4653), I gonna a issue another pull request.

Hope it gonna do the trick.

Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment