heiglandreas / HybridAuth

Integrates SocialConnect as SIMPLE authentication-backend into the ZendFramework 3
https://hybridauth.heigl.org
21 stars 8 forks source link

Fixed bug for getting other Entity #15

Closed newage closed 9 years ago

newage commented 9 years ago

Because Twitter and Facebook entities only extend Hybridauth\Entity\Profile.

heiglandreas commented 9 years ago

I'm quite sorry but I can't really understand the problem. Hybridauths Google-adapter for instance also returns a Hybridauth\Entity\Profile so there are more than only Twitter and Facebook.

Perhaps you would be so kind to show an example of what is failing? That'd help me to understand you PullRequest.

Thanks

newage commented 9 years ago

Class UserWrapperFactory have a code

public function factory($userObject)
{ 
    switch (get_class($userObject))
    {
        case 'Hybridauth\\Entity\\Profile':
        ...
        default:
            throw new \UnexpectedValueException(

Class name of $userObject must be Hybridauth\\Entity\\Profile, but facebook profile class https://github.com/hybridauth/hybridauth/blob/3.0.0/src/Hybridauth/Entity/Facebook/Profile.php#L6 only extends Hybridauth\\Entity\\Profile and we get exception, because get_class() return the name of class not the name of parent class. And twitter extends Hybridauth\\Entity\\Profile, and class name has Hybridauth\Entity\Twitter\Profile https://github.com/hybridauth/hybridauth/blob/3.0.0/src/Hybridauth/Entity/Twitter/Profile.php#L6

get_class(facebookObject) == Hybridauth\Entity\Facebook\Profile get_class(twitterObject) == Hybridauth\Entity\Twitter\Profile

heiglandreas commented 9 years ago

Ah, OK. I see what you mean. get_class of course returns the current class. But instead of removing the complete switch block I'd refactor that into multiple if- statements like this:

if ( $userObject instanceof Hybridauth\Entity\Profile) {
    // Do the stuff from the first case-block
} else {
    // Do the stuff from the default case-block
}

Or you could add a further case-statement for each missing class.

That way the method has the same API as before but is capable of handling subclasses of Hybridauth\Entity\Profile. And there is no need to remove tests, on the contrary there have to be some more added.

newage commented 9 years ago

Ok. Yes, I agree with you. Code with instanceof is better. You can close this pull request. I will create new pull request. Thank you