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

Fatal when using MonetaryAccountBank class #38

Closed
OGKevin opened this issue Aug 22, 2017 · 11 comments
Closed

Fatal when using MonetaryAccountBank class #38

OGKevin opened this issue Aug 22, 2017 · 11 comments
Assignees
Labels
Milestone

Comments

@OGKevin
Copy link
Contributor

OGKevin commented Aug 22, 2017

Steps to reproduce:

  1. Run https://github.com/bunq/sdk_php/blob/c672ea6467bd94778cbb49bc034f2afb3e37fdef/tests/Model/Generated/MonetaryAccountBankTest.php on php5.6

What happens:

  1. You will get the following fatal error:
Fatal error: Cannot use bunq\Model\Generated\Object\Avatar as Avatar because the name is already in use in /sdk_php/src/Model/Generated/MonetaryAccountBank.php on line 8

What should happen:

  1. No fatal error

Extra info:

@OGKevin
Copy link
Contributor Author

OGKevin commented Sep 7, 2017

Closing in favor of #44

@OGKevin OGKevin closed this as completed Sep 7, 2017
@WouterFlorijn
Copy link
Contributor

@OGKevin I'm getting the same issue with php 7.0.10, so it doesn't have to do with a lower PHP version. Is there a possibility that this will get fixed, or is this not an issue anymore in the next release?

@OGKevin
Copy link
Contributor Author

OGKevin commented Oct 8, 2017

@WouterFlorijn Could you please paste the output of php --version and which php

Also, is this happing when you run the tests mentioned above? In other words how can we reproduce this ?

@WouterFlorijn
Copy link
Contributor

@OGKevin php --version outputs:

PHP 7.0.10 (cli) (built: Aug 18 2016 09:48:53) ( ZTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2016 Zend Technologies

I'm not exactly sure about which php. I'm on Windows and don't have the command. However, phpinfo() displays version 7.0.10 as well.

I haven't ran the tests yet (will post the result when I have), but I run into this issue whenever I try to use the MonetaryAccountBank class (whether directly or indirectly). My setup:

  1. Install Laravel.
  2. Install bunq sdk (0.11.0) through composer.
  3. Create sandbox .conf file using the SDK (works perfectly fine).
  4. Load .conf file and request data using the SDK with the following code (Laravel controller):
<?php

namespace App\Http\Controllers;

use Illuminate\Http\Request;

use bunq\Context\ApiContext;
use bunq\Util\BunqEnumApiEnvironmentType;
use bunq\Http\Pagination;
use bunq\Model\Generated\User;
use bunq\Model\Generated\Payment;
use bunq\Model\Generated\MonetaryAccount;

class BunqController extends Controller
{
    public function payments()
    {
        $apiContext = ApiContext::restore(storage_path('app/bunq.conf'));

        $users  = User::listing($apiContext)->getValue();
        $user   = $users[0]->getUserCompany();
        $userId = $user->getId();

        $monetaryAccounts  = MonetaryAccount::listing($apiContext, $userId)->getValue(); // Error on this line
        $monetaryAccount   = $monetaryAccounts[0]->getMonetaryAccountBank();
        $monetaryAccountId = $monetaryAccount->getId();

        $paginationCountOnly = new Pagination();
        $paginationCountOnly->setCount(10);

        $paymentsResponse = Payment::listing(
            $apiContext,
            $userId,
            $monetaryAccountId,
            $paginationCountOnly->getUrlParamsCountOnly()
        );

        $payments = $paymentsResponse->getValue();

        dd($payments);
    }
}

The error is caused by the use bunq\Model\Generated\Object\Avatar; line in Model/Generated/MonetaryAccountBank.php. When I uncomment this line, everything works. It seems that the Avatar class in the bunq\Model\Generated namespace is already present, as the one in bunq\Model\Generated\Object tries to 'replace' it.

@OGKevin OGKevin changed the title Fatal on php 5.6 Fatal when using MonetaryAccountBank class Oct 9, 2017
@OGKevin OGKevin self-assigned this Oct 9, 2017
@OGKevin OGKevin removed this from the v0.11.0 milestone Oct 9, 2017
@OGKevin OGKevin reopened this Oct 9, 2017
@OGKevin
Copy link
Contributor Author

OGKevin commented Nov 28, 2017

@WouterFlorijn Are you still running into this issue ?

@WouterFlorijn
Copy link
Contributor

@OGKevin Yes, I'm still having the same issue with version 0.12.3 of the sdk.

I think simply replacing the use bunq\Model\Generated\Object\Avatar; with use bunq\Model\Generated\Object\Avatar as AvatarObject; and then replacing occurrences of Avatar with AvatarObject in Model/Generated/EndPoint/MonetaryAccountBank.php should do the trick right? This should then also be done for other files, since I had the same error occur in Model/Generated/EndPoint/UserPerson.php for example (also on the Avatar).

@OGKevin
Copy link
Contributor Author

OGKevin commented Dec 2, 2017

@WouterFlorijn I see, I cant reproduce this on:

PHP 7.0.25 (cli) (built: Nov 17 2017 23:06:15) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2017 Zend Technologies
    with Xdebug v2.5.5, Copyright (c) 2002-2017, by Derick Rethans

🤔 Do you mind updating your PHP 7.0.10 to PHP 7.0.25 and try again. If this doesn't solve your problem or for some reason you cant upgrade then we shall look into a fix. But if this is indeed fixed on PHP 7.0.25 then there is no need to need to provide a fix as it works on the latests php70.

@WouterFlorijn
Copy link
Contributor

Strange, I don't know enough about the core of PHP to understand where this could be coming from. Could it perhaps be because of Composer autoloading that happens in Laravel (just a guess)?

I'll test with 7.0.25 as soon as I get the chance!

@OGKevin
Copy link
Contributor Author

OGKevin commented Dec 3, 2017

I think it has to do with this https://bugs.php.net/bug.php?id=66773 which has been fixed in php 7.0.13. So i think if you indeed upgrade your php this problem should be solved! Please let me know if this is indeed the case 👍

If this is indeed the case then I'll make sure to put php 7.0.13 as requirement in composer.json.

@WouterFlorijn
Copy link
Contributor

@OGKevin I'm now on 7.0.23 and I don't get the error anymore! Thanks!

@OGKevin
Copy link
Contributor Author

OGKevin commented Dec 3, 2017

Nice, i will update the composer.json requirement.

dnl-blkv added a commit that referenced this issue Dec 6, 2017
…composer_bunq/sdk_php#38

Add specific php version to composer. (#38)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants