Refactor to not use static method calls

[atrauzzi]

In many applications (especially where dependency injection is used), it's preferable to not have libraries that rely on global state.

The Stripe PHP library currently takes the approach that every call is done via a static method. While it's fine to retain this as an option, it would be nice if actual objects could be leveraged.

[GrahamCampbell]

👍

[joshhornby]

👍

[Rican7]

👍

[skyzyx]

👍

[browner12]

would it be possible to maintain both as an option on the same code? I feel like that would become unmanageable quickly. would it be better to break it off into another repo, or would that be just as difficult to maintain?

maybe the better question is, should the static approach be kept, or maybe deprecated? does someone have an example where it would be difficult to instantiate actual objects?

either way 👍

[ruudk]

👍

[yurytolochko]

👍

[phillipsnick]

👍

Having fun trying to unit test my payment service because I can't mock static methods!

[armetiz]

For people trying to unit-test payment, you should make a wrapper around Stripe.

use Acme\Payment\Stripe

class Stripe
{
  static private $instance;

  public function __construct($stripeSecretKey)
  {
        if(self::$instance) {
            throw new \RuntimeException('Stripe could not be instantiate more than once. Check PHP implementation : https://github.com/stripe/stripe-php');
        }

        self::$instance = $this;

        \Stripe\Stripe::setApiKey($stripeSecretKey);
  }

  public function payAmountWithToken($token, $amount)
  {
    \Stripe\Charge::create([]); // use correct values
  }
}

I made a singleton to avoid confusion. This is a limitation but in many case you will not need to have more than one instance.

You will be able to inject your custom wrapper / mock and make your unit-test ;)

Regards.

[bakura10]

As always, for people that do not want to use static methods, you could have a look at my library ZfrStripe. It is maintained, is based on Guzzle and is definitely easier to mock and test. Having said that it may be a bit harder to use as you definitely need to inject the client when you need it instead of relying on the staticness.

[arcanedev-maroc]

👍

[adrienbrault]

😢

[KostyaChoporov]

Also will be great to make some event solution. For example I want log all requests, and i don't know hot to implement it without wrapping original requests 😢

[lunetics]

@KostyaChoporov i think it should be psr-7 compliant for that, then you could use your own client e.g. guzzle with logging.

[robfrawley]

+1

Ping me if I can help actually make this happen in any way; glad to donate some hours toward this common good; the current reliance on static calls is...depressing. ;-)

[zarloproperty]

Interesting...

[dann95]

👍

[matjack1]

While I 👍 this request, I'm currently using as a simple workaround these mock classes:

class MockStripeCharge {

	public static $chargeData;

	public static function create() {
		return self::$chargeData;
	}

}

class MockStripe {

	public static function setApiKey() {
	}

}

that I inject with this function in my class:

public function injectTesting( $stripe, $stripeCharge ) {
	$this->stripe = $stripe;
	$this->stripeCharge = $stripeCharge;
}

as $chargeData I use a JSON with all the needed fields, like this:

$charge = json_decode( $chargeJson );
MockStripeCharge::$chargeData = $charge;

and then:

$myClass->injectTesting( 'MockStripe', 'MockStripeCharge' );

So my code doesn't use Stripe directly, but like this:

$stripeCharge = $this->stripeCharge;
$charge = $stripeCharge::create(...);

[Bilge]

Another alternative for your consideration, Stripe provider for Porter. This implementation is currently incomplete but used in production since August 2016. Adding missing API features is easy should anyone feel so inclined.

[Petah]

This would be super useful. Our main use case is we have different stripe accounts used in the same app, so we need a way to use different keys. Having to call setApiKey before every call is not ideal.

[Petah]

Also this could be backwards compatible if you just made the static class return an instance of a class and forward static methods.

pubilc class StripeInstance {
    public function someCall() {

    }
}

public class Stripe {
    private $instance;
    private static function getInstance() {
        if (!static::$instance) {
            static::$instance = new StripeInstance();
        }
        return static::$instance;
    }
    public static function someCall() {
        return static::getInstance()->someCall();
    }
}

[ob-stripe]

Having to call setApiKey before every call is not ideal.

@Petah This doesn't solve everything, but note that you can set the API key on a per-request basis by passing an "api_key" entry in the $opts array, e.g.:

$charge = \Stripe\Charge::retrieve("ch_...", array("api_key" => "sk_..."));

[timbroder]

👍

[chdeliens]

+1 to ditch the static!

[chdeliens]

@bakura10 last commit on https://github.com/zf-fr/zfr-stripe was more than a year ago, how far behind are you from the official PHP library?

[bakura10]

Hi @chdeliens .

Unfortuantely I no longer use Stripe (I've slowly moving out of tech world to tell the truth), so it's likely lacking behind and a lot of new features is not implemented. I'd be happy to give the lead to someone else would like to maintain this library.

If you are interesting please send me an email to michael at maestrooo dot com (there are three o, pay attention :)).

[GrahamCampbell]

https://github.com/cartalyst/stripe is pretty good.

[chdeliens]

Thanks, I switched to the one from Cartalyst. I can now inject it in my service classes, stub/mock it and do my unit-testing! :)

[phillips-tech]

@GrahamCampbell Be aware that cartalyst/stripe does not currently support multiple plans per subscription.

[ostrolucky]

cartalyst/stripe is unfortunately just a thin layer over submitting, receiving raw array/json data. Stripe-php will give you objects all the way and you can even call actions on them right away, in active record style.

[ostrolucky]

Here's an httplug stripe adapter I made https://gist.github.com/ostrolucky/7e2a8916283bcf5dba9829b943c6d0f0

Usage:

\Stripe\ApiRequestor::setHttpClient($stripeHttpClient);

[smknstd]

This worked for me http://docs.mockery.io/en/latest/reference/public_static_properties.html (+ this and this )

/**
 * @runInSeparateProcess
 * @preserveGlobalState disabled
 */
public function testMyStripeFeature()
{
      \Mockery::mock('alias:\Stripe\Charge')
            ->shouldReceive('create')
            ->andReturn((Object)[
                "id" => "1234"
            ]);
      ...
}

[mcordingley]

stripe-ci added the approved label on Jan 17, 2017

checks clock

Sad to say, I don't think they really care about this issue.

[prodigeris]

@mcordingley What gave it away?

[ob-stripe]

Hi everyone. We are aware that the current design of the PHP library is not ideal and could be improved by not using static method calls. We do want to add non-static calls, but at the same time we want to keep the existing methods to avoid a painful breaking change for existing users.

We have started automatically generating part of our client libraries' source code based on the OpenAPI specification file for Stripe's API. We are currently working on Ruby, Python, Node and Java, but want to tackle PHP soon after.

Once we've updated PHP to use our codegen tool, we should be able to add support for non-static methods much more easily. I don't have any ETA to provide just yet, but we will post updates in this thread.

Thanks everyone for your patience! In the meantime, I've locked this thread to avoid sending notifications to everyone every time a new message is posted.

[richardm-stripe]

Hello everybody, in version 7.33.0 (#771), we've released a "StripeClient" class that avoids static methods, that we will be shipping alongside the existing pattern.

So instead of the old-style:

<?php
\Stripe\Stripe::setApiKey('sk_test_xxx');
$customer = \Stripe\Customer::retrieve(
  'cus_xxx'
);
$customer->delete();

you can now do

<?php
$stripe = new \Stripe\StripeClient([
  'api_key' => 'sk_test_xxx',
]);
$stripe->customers->delete('cus_xxx', []);

This is now the recommended way to use stripe-php, and we will be updating the API Reference in the coming days.

Thank you to everybody on this issue thread for your thumbs, feedback and suggestions.

[Rican7]

... woah, it happened! After more than 5 years!

Thank you so much @richardm-stripe / @stripe!

[Bilge]

Thanks, Corona.

[smknstd]

Good job on this release ! How do you write this with new syntax ? https://stripe.com/docs/connect/oauth-reference#post-token

        $response = \Stripe\OAuth::token([
            'grant_type' => 'authorization_code',
            'code' => $code,
        ]);

        dd($response->stripe_user_id);

Thanx for your help !

[remi-stripe]

@smknstd We haven't migrated the OAuth methods to services just yet though it's on our list to do in the future. For now you'd keep that code as is!

[smknstd]

No problem. Thanx for your answer.