Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update from 2.3.0 to 2.4.0 fails #752

Closed
fzieris opened this issue Nov 21, 2018 · 11 comments
Closed

Update from 2.3.0 to 2.4.0 fails #752

fzieris opened this issue Nov 21, 2018 · 11 comments

Comments

@fzieris
Copy link

fzieris commented Nov 21, 2018

Context: Symfony project with dependency "league/oauth2-google": "^2.2", which in turn depends on oauth2-client.

So far, I had oauth2-client installed in version 2.3.0, but upon composer update I get the following error message, after the upgrade to 2.4.0:

Fatal error: Declaration of League\OAuth2\Client\Provider\Google::getResourceOwnerDetailsUrl(League\OAuth2\Client\Token\AccessToken $token) must be compatible with League\OAuth2\Client\Provider\AbstractProvider::getResourceOwnerDetailsUrl(League\OAuth2\Client\Token\AccessTokenInterface $token) in G:\XXX\vendor\league\oauth2-google\src\Provider\Google.php on line 11

@fzieris
Copy link
Author

fzieris commented Nov 21, 2018

Hotfix in composer.json

    "conflict": {
+        "league/oauth2-client": "2.4.0"
    },

@ramsey
Copy link
Contributor

ramsey commented Nov 22, 2018

Gah! I didn’t think that would break BC. I was wrong. I’ll get a fix out within 24 hours. Thanks!

@senyahnoj
Copy link

Thanks for your prompt action on resolving this. I have the exact same issue with pviojo/oauth2-keycloak

@ramsey
Copy link
Contributor

ramsey commented Nov 22, 2018

I am trying to reproduce this issue so I can write a test that shows this fatal error.

So far, I have this:

namespace League\OAuth2\Client\Test\Provider\Fake;

use League\OAuth2\Client\Token\AccessToken;

class ProviderWithAccessTokenHints extends AccessToken
{
    public function getResourceOwnerDetailsUrl(AccessToken $token)
    {
        return 'https://api.example.com/owner/' . $token->getResourceOwnerId();
    }
}

But I'm able to run the test without any problems, and I see no fatal errors. I'm running the test with PHP 7.2.11.

What version of PHP are you using, and at what point do you see the error? Is it during the composer upgrade that the error occurs?

Thanks!

@ramsey
Copy link
Contributor

ramsey commented Nov 22, 2018

Nevermind. I am an idiot. I was extending the wrong class. 🤦‍♂️

@ramsey ramsey closed this as completed in bb12ba5 Nov 22, 2018
@ramsey
Copy link
Contributor

ramsey commented Nov 22, 2018

Please check release 2.4.1 and confirm that it fixes this issue. Thanks!

@senyahnoj
Copy link

I confirm that it's fixed it for me. Thanks again

@holtkamp
Copy link
Contributor

Confirmed! Thanks!

@holtkamp
Copy link
Contributor

holtkamp commented Nov 18, 2019

@ramsey since it breaks compatibility, a new major release is "required" to be able to use the AccessTokenInterface instead of the AccessToken. Are there concrete plans for such a major release already?

@ramsey
Copy link
Contributor

ramsey commented Nov 18, 2019

@holtkamp I missed that comment from a year ago. So, version 2.4.1 introduced a BC break, and it should have, instead, been released in a new major version? Since it's been a year, is this still a concern?

@holtkamp
Copy link
Contributor

@ramsey ooh, no worries, the comment was from today when I was navigating some code and saw a AccessToken being passed around instead of an AccessTokenInterface, that is why I "revived" this. It is not an issue, but it would be more elegant to only work with an AccessTokenInterface, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants