Update from 2.3.0 to 2.4.0 fails

Question

Update from 2.3.0 to 2.4.0 fails

fzieris opened this issue 7 years ago · 11 comments

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

Answer 1 · 2018-11-21T23:13:29.000Z

Hotfix in composer.json

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

Answer 2 · 2018-11-22T00:12:02.000Z

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

Answer 3 · 2018-11-22T14:16:33.000Z

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

Answer 4 · 2018-11-22T16:27:55.000Z

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!

Answer 5 · 2018-11-22T16:29:35.000Z

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

Answer 6 · 2018-11-22T18:37:04.000Z

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

Answer 7 · 2018-11-23T08:40:06.000Z

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

Answer 8 · 2018-11-23T09:53:37.000Z

Confirmed! Thanks!

Answer 9 · 2019-11-18T10:05:26.000Z

@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?

Answer 10 · 2019-11-18T18:48:41.000Z

@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?

Answer 11 · 2019-11-18T21:01:33.000Z

@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?