Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Zend\Authentication\Result custom result codes not possible #4452

Closed
wants to merge 2 commits into from

3 participants

@JustinBuser

What's the point of this in the Result constructor:

    if ($code < self::FAILURE_UNCATEGORIZED) {
        $code = self::FAILURE;
    } elseif ($code > self::SUCCESS ) {
        $code = 1;
    }

It pretty much nullifies the usefulness of any classes that extend result using custom codes without making a custom constructor, and seems to be there for no discernible reason.

@weierophinney weierophinney was assigned
@weierophinney weierophinney referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@weierophinney

Actually... this change would only make sense if we were using static:: instead of self:: for resolution -- and even then, the fact is that AuthenticationService is checking for the specific Result constants, which means self:: is correct.

Can you clarify what you feel needs to change, please, @JustinBuser ?

@mwillbanks
Collaborator

This portion of the code actually drives me insane also; I extend it by using the constructor right now.

I have several different codes that I use today and throw differently in the constructor. This kills the open/close principle in a bad way :(

@JustinBuser

Remove it, it serves no purpose other than to override values that are being explicitly set, ergo: shouldn't need to be overwritten. Can you cite some reason it needs to be there?

weierophinney added some commits
@weierophinney weierophinney [#4452] Use constant values, not integers
- Use constant values, to make extension possible
5717ba8
@weierophinney weierophinney Remove code munging
- Per the comments, changing the code value if it fell outside the range of the
  defined constants does not change how isValid() works, and does not allow for
  custom constants in extending classes, unless you also override the
  constructor.
6756117
@weierophinney

@mwillbanks Okay, updated -- tests still pass, so should be fine. Please review! :)

@mwillbanks mwillbanks closed this pull request from a commit
@mwillbanks mwillbanks Merge branch 'hotfix/4452'
Close #4452
f46dec7
@mwillbanks mwillbanks referenced this pull request from a commit
@mwillbanks mwillbanks Merge branch 'hotfix/4452' into develop
Forward Port #4452
acda6d9
@JustinBuser

Perfection, :+1:

@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney [#4452] Use constant values, not integers
- Use constant values, to make extension possible
bb85349
@ghost Unknown referenced this pull request from a commit
@mwillbanks mwillbanks Merge branch 'hotfix/4452'
Close #4452
559e1e9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 23, 2013
  1. @weierophinney

    [#4452] Use constant values, not integers

    weierophinney authored
    - Use constant values, to make extension possible
  2. @weierophinney

    Remove code munging

    weierophinney authored
    - Per the comments, changing the code value if it fell outside the range of the
      defined constants does not change how isValid() works, and does not allow for
      custom constants in extending classes, unless you also override the
      constructor.
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 9 deletions.
  1. +1 −9 library/Zend/Authentication/Result.php
View
10 library/Zend/Authentication/Result.php
@@ -73,15 +73,7 @@ class Result
*/
public function __construct($code, $identity, array $messages = array())
{
- $code = (int) $code;
-
- if ($code < self::FAILURE_UNCATEGORIZED) {
- $code = self::FAILURE;
- } elseif ($code > self::SUCCESS) {
- $code = 1;
- }
-
- $this->code = $code;
+ $this->code = (int) $code;
$this->identity = $identity;
$this->messages = $messages;
}
Something went wrong with that request. Please try again.