Ingenico-ePayments / connect-sdk-php

Ingenico Connect PHP Server SDK
https://docs.connect.worldline-solutions.com/documentation/sdk/server/php/
MIT License
39 stars 16 forks source link

Change autoloading from PSR-0 to PSR-4 #2

Closed erkurita closed 8 years ago

erkurita commented 8 years ago

Per http://www.php-fig.org/psr/psr-0/:

Deprecated - As of 2014-10-21 PSR-0 has been marked as deprecated. PSR-4 is now recommended as an alternative.

Given how recent version 1.0.0 is (2016-03-02), I don't suppose there'd be many implementations floating around, and thus it'd be a good moment to migrate to PSR-4.

This is a non-BC change, because all class names are changed, so according to http://semver.org/ this would push it to 2.0.0 straight away.

Caveat: unfortunately, I'm unable to run the test suite due to a missing piece of information: the base endpoint. I have the API key and secret, but I've searched through all the documentation and found nothing. No examples either. If someone would point me in the right way so I can make sure the test suite is as good as upstream, it'd be nice.

Comments and suggestions are welcome.

rob-spoor commented 8 years ago

I've checked the Developer Portal, but I can't find the base URI anywhere. The Java SDK does include it in its example configuration: https://api-sandbox.globalcollect.com/v1. Note that that's only for the sandbox environment.

erkurita commented 8 years ago

Thanks for the quick reply.

I've tested the URL and as per the config.json comments, using that endpoint generates these URI for curl https://api-sandbox.globalcollect.com/v1/v1/8897/payouts Which I assume is incorrect, as it has an extra /vi directory in it.

Even after removing it, and in master as well, I get these errors from the server:

{
   "errorId" : "0ef074f3-31a3-44f1-b3b0-6aad272e7fac",
   "errors" : [ {
      "code" : "9007",
      "message" : "ACCESS_TO_MERCHANT_NOT_ALLOWED",
      "httpStatusCode" : 403
   } ]
}

I don't suppose this has to do with the API key and secret, does it?

rob-spoor commented 8 years ago

That error indicates that either you don't have access to merchant 8897, or that the API key/secret combination is no longer valid. Can you check in the Configuration Center that this is indeed your merchant id, and that the key/secret combination has not expired yet?

erkurita commented 8 years ago

Well it's certainly not my merchant ID, and the API key/secret expire in about a year.

I can see the merchant ID is hard-coded into some tests:

        $this->defaultCommunicator->get($this->defaultResponseClassMap, '/8500/products', $clientHeaders, $findParams);

I'm gonna open a separate pull request to fix the tests before continuing here

aadmathijssen commented 8 years ago

Hi @erkurita,

Thank you very much for your contribution. You're right that we should use namespaces throughout the SDK, and that this should adhere to PSR-4. Unfortunately, we cannot directly use your pull request, because part of the library consist of generated code (all the files in the src folder), so we should update our code generation mechanism. I just created an internal ticket to pick this up.

Regarding your issues with the test suite, the integration tests are written against our internal testing environment, so these will not run in your case. We should probably make a clear separation between unit tests and integration tests, and change the integration tests such that they work for any sandbox account.

Please let us know if you run into any other issues.

Thanks!

Aad

erkurita commented 8 years ago

Hello @aadmathijssen,

Thanks for the reply. It's a shame about the PR, but that's the way it is with open source. You might want to state in the README.md that the code under src is auto-generated and should not be tinkered with directly.

Regarding the test suite, what I plan to do is add another parameter to the config.json so it can be run with an specific merchant ID. Otherwise, anyone else picking up the code will stumble upon the same issues.

I'll be closing this then.

Thank you.