From 21381d695c6b6e214468e63c3a99c36d5ba1e4e9 Mon Sep 17 00:00:00 2001 From: Olivier Bellone Date: Fri, 5 Jan 2018 00:02:22 +0100 Subject: [PATCH] Get rid of ExternalAccount --- init.php | 1 - lib/Account.php | 10 +-- lib/AlipayAccount.php | 58 ++++++++++++++++- lib/BankAccount.php | 57 ++++++++++++++++- lib/BitcoinReceiver.php | 44 +++++-------- lib/Card.php | 64 ++++++++++++++++++- lib/Customer.php | 10 +-- lib/ExternalAccount.php | 83 ------------------------ tests/Stripe/AlipayAccountTest.php | 76 ++++++++++++++++++++++ tests/Stripe/BankAccountTest.php | 79 ++++++++++++++++++++++- tests/Stripe/BitcoinReceiverTest.php | 68 +++++++++++++++----- tests/Stripe/CardTest.php | 94 ++++++++++++++++++++++++++++ 12 files changed, 497 insertions(+), 147 deletions(-) delete mode 100644 lib/ExternalAccount.php create mode 100644 tests/Stripe/AlipayAccountTest.php create mode 100644 tests/Stripe/CardTest.php diff --git a/init.php b/init.php index b60d2181f..99f7a8c1b 100644 --- a/init.php +++ b/init.php @@ -50,7 +50,6 @@ require(dirname(__FILE__) . '/lib/ApiRequestor.php'); require(dirname(__FILE__) . '/lib/ApiResource.php'); require(dirname(__FILE__) . '/lib/SingletonApiResource.php'); -require(dirname(__FILE__) . '/lib/ExternalAccount.php'); // Stripe API Resources require(dirname(__FILE__) . '/lib/Account.php'); diff --git a/lib/Account.php b/lib/Account.php index e50207a66..e4a5cc4e2 100644 --- a/lib/Account.php +++ b/lib/Account.php @@ -120,7 +120,7 @@ public function deauthorize($clientId = null, $opts = null) * @param array|null $params * @param array|string|null $opts * - * @return ExternalAccount + * @return BankAccount|Card */ public static function createExternalAccount($id, $params = null, $opts = null) { @@ -133,7 +133,7 @@ public static function createExternalAccount($id, $params = null, $opts = null) * @param array|null $params * @param array|string|null $opts * - * @return ExternalAccount + * @return BankAccount|Card */ public static function retrieveExternalAccount($id, $externalAccountId, $params = null, $opts = null) { @@ -146,7 +146,7 @@ public static function retrieveExternalAccount($id, $externalAccountId, $params * @param array|null $params * @param array|string|null $opts * - * @return ExternalAccount + * @return BankAccount|Card */ public static function updateExternalAccount($id, $externalAccountId, $params = null, $opts = null) { @@ -159,7 +159,7 @@ public static function updateExternalAccount($id, $externalAccountId, $params = * @param array|null $params * @param array|string|null $opts * - * @return ExternalAccount + * @return BankAccount|Card */ public static function deleteExternalAccount($id, $externalAccountId, $params = null, $opts = null) { @@ -171,7 +171,7 @@ public static function deleteExternalAccount($id, $externalAccountId, $params = * @param array|null $params * @param array|string|null $opts * - * @return ExternalAccount + * @return BankAccount|Card */ public static function allExternalAccounts($id, $params = null, $opts = null) { diff --git a/lib/AlipayAccount.php b/lib/AlipayAccount.php index 1ba34bfa6..fb7d5bb79 100644 --- a/lib/AlipayAccount.php +++ b/lib/AlipayAccount.php @@ -6,8 +6,64 @@ * Class AlipayAccount * * @package Stripe + * + * @deprecated Alipay accounts are deprecated. Please use the sources API instead. + * @link https://stripe.com/docs/sources/alipay */ -class AlipayAccount extends ExternalAccount +class AlipayAccount extends ApiResource { + use ApiOperations\Delete; + use ApiOperations\Update; + + /** + * @return string The instance URL for this resource. It needs to be special + * cased because it doesn't fit into the standard resource pattern. + */ + public function instanceUrl() + { + if ($this['customer']) { + $base = Customer::classUrl(); + $parent = $this['customer']; + $path = 'sources'; + } else { + $msg = "Alipay accounts cannot be accessed without a customer ID."; + throw new Error\InvalidRequest($msg, null); + } + $parentExtn = urlencode(Util\Util::utf8($parent)); + $extn = urlencode(Util\Util::utf8($this['id'])); + return "$base/$parentExtn/$path/$extn"; + } + + /** + * @param array|string $_id + * @param array|string|null $_opts + * + * @throws \Stripe\Error\InvalidRequest + * + * @deprecated Alipay accounts are deprecated. Please use the sources API instead. + * @link https://stripe.com/docs/sources/alipay + */ + public static function retrieve($_id, $_opts = null) + { + $msg = "Alipay accounts cannot be accessed without a customer ID. " . + "Retrieve an Alipay account using \$customer->sources->retrieve('alipay_account_id') instead."; + throw new Error\InvalidRequest($msg, null); + } + /** + * @param string $_id + * @param array|null $_params + * @param array|string|null $_options + * + * @throws \Stripe\Error\InvalidRequest + * + * @deprecated Alipay accounts are deprecated. Please use the sources API instead. + * @link https://stripe.com/docs/sources/alipay + */ + public static function update($_id, $_params = null, $_options = null) + { + $msg = "Alipay accounts cannot be accessed without a customer ID. " . + "Call save() on \$customer->sources->retrieve('alipay_account_id') instead."; + throw new Error\InvalidRequest($msg, null); + } } diff --git a/lib/BankAccount.php b/lib/BankAccount.php index 2f11c067c..43e4498d0 100644 --- a/lib/BankAccount.php +++ b/lib/BankAccount.php @@ -7,9 +7,64 @@ * * @package Stripe */ -class BankAccount extends ExternalAccount +class BankAccount extends ApiResource { + use ApiOperations\Delete; + use ApiOperations\Update; + /** + * @return string The instance URL for this resource. It needs to be special + * cased because it doesn't fit into the standard resource pattern. + */ + public function instanceUrl() + { + if ($this['customer']) { + $base = Customer::classUrl(); + $parent = $this['customer']; + $path = 'sources'; + } elseif ($this['account']) { + $base = Account::classUrl(); + $parent = $this['account']; + $path = 'external_accounts'; + } else { + $msg = "Bank accounts cannot be accessed without a customer ID or account ID."; + throw new Error\InvalidRequest($msg, null); + } + $parentExtn = urlencode(Util\Util::utf8($parent)); + $extn = urlencode(Util\Util::utf8($this['id'])); + return "$base/$parentExtn/$path/$extn"; + } + + /** + * @param array|string $_id + * @param array|string|null $_opts + * + * @throws \Stripe\Error\InvalidRequest + */ + public static function retrieve($_id, $_opts = null) + { + $msg = "Bank accounts cannot be accessed without a customer ID or account ID. " . + "Retrieve a bank account using \$customer->sources->retrieve('bank_account_id') or " . + "\$account->external_accounts->retrieve('bank_account_id') instead."; + throw new Error\InvalidRequest($msg, null); + } + + /** + * @param string $_id + * @param array|null $_params + * @param array|string|null $_options + * + * @throws \Stripe\Error\InvalidRequest + */ + public static function update($_id, $_params = null, $_options = null) + { + $msg = "Bank accounts cannot be accessed without a customer ID or account ID. " . + "Call save() on \$customer->sources->retrieve('bank_account_id') or " . + "\$account->external_accounts->retrieve('bank_account_id') instead."; + throw new Error\InvalidRequest($msg, null); + } + + /** * @param array|null $params * @param array|string|null $options * diff --git a/lib/BitcoinReceiver.php b/lib/BitcoinReceiver.php index db7dfc5f0..8d04256b5 100644 --- a/lib/BitcoinReceiver.php +++ b/lib/BitcoinReceiver.php @@ -4,20 +4,20 @@ /** * Class BitcoinReceiver - - * @deprecated Please use sources instead. + * + * @package Stripe + * + * @deprecated Bitcoin receivers are deprecated. Please use the sources API instead. + * @link https://stripe.com/docs/sources/bitcoin */ -class BitcoinReceiver extends ExternalAccount +class BitcoinReceiver extends ApiResource { use ApiOperations\All; - use ApiOperations\Create; use ApiOperations\Retrieve; /** * @return string The class URL for this resource. It needs to be special * cased because it doesn't fit into the standard resource pattern. - * - * @deprecated Please use sources instead. */ public static function classUrl() { @@ -27,36 +27,20 @@ public static function classUrl() /** * @return string The instance URL for this resource. It needs to be special * cased because it doesn't fit into the standard resource pattern. - * - * @deprecated Please use sources instead. */ public function instanceUrl() { - $result = parent::instanceUrl(); - if ($result) { - return $result; + if ($this['customer']) { + $base = Customer::classUrl(); + $parent = $this['customer']; + $path = 'sources'; + $parentExtn = urlencode(Util\Util::utf8($parent)); + $extn = urlencode(Util\Util::utf8($this['id'])); + return "$base/$parentExtn/$path/$extn"; } else { - $id = $this['id']; - $id = Util\Util::utf8($id); - $extn = urlencode($id); $base = BitcoinReceiver::classUrl(); + $extn = urlencode(Util\Util::utf8($this['id'])); return "$base/$extn"; } } - - /** - * @param array|null $params - * @param array|string|null $options - * - * @return BitcoinReceiver The refunded Bitcoin Receiver item. - * - * @deprecated Please use sources instead. - */ - public function refund($params = null, $options = null) - { - $url = $this->instanceUrl() . '/refund'; - list($response, $opts) = $this->_request('post', $url, $params, $options); - $this->refreshFrom($response, $opts); - return $this; - } } diff --git a/lib/Card.php b/lib/Card.php index 01ad27847..0b725512f 100644 --- a/lib/Card.php +++ b/lib/Card.php @@ -19,7 +19,7 @@ * @property string $country * @property string $customer * @property string $cvc_check - * @property string $dynamic_last4": null, + * @property string $dynamic_last4 * @property int $exp_month * @property int $exp_year * @property string $fingerprint @@ -31,7 +31,67 @@ * * @package Stripe */ -class Card extends ExternalAccount +class Card extends ApiResource { + use ApiOperations\Delete; + use ApiOperations\Update; + /** + * @return string The instance URL for this resource. It needs to be special + * cased because cards are nested resources that may belong to different + * top-level resources. + */ + public function instanceUrl() + { + if ($this['customer']) { + $base = Customer::classUrl(); + $parent = $this['customer']; + $path = 'sources'; + } elseif ($this['account']) { + $base = Account::classUrl(); + $parent = $this['account']; + $path = 'external_accounts'; + } elseif ($this['recipient']) { + $base = Recipient::classUrl(); + $parent = $this['recipient']; + $path = 'cards'; + } else { + $msg = "Cards cannot be accessed without a customer ID, account ID or recipient ID."; + throw new Error\InvalidRequest($msg, null); + } + $parentExtn = urlencode(Util\Util::utf8($parent)); + $extn = urlencode(Util\Util::utf8($this['id'])); + return "$base/$parentExtn/$path/$extn"; + } + + /** + * @param array|string $_id + * @param array|string|null $_opts + * + * @throws \Stripe\Error\InvalidRequest + */ + public static function retrieve($_id, $_opts = null) + { + $msg = "Cards cannot be accessed without a customer, recipient or account ID. " . + "Retrieve a card using \$customer->sources->retrieve('card_id'), " . + "\$recipient->cards->retrieve('card_id'), or"; + "\$account->external_accounts->retrieve('card_id') instead."; + throw new Error\InvalidRequest($msg, null); + } + + /** + * @param string $_id + * @param array|null $_params + * @param array|string|null $_options + * + * @throws \Stripe\Error\InvalidRequest + */ + public static function update($_id, $_params = null, $_options = null) + { + $msg = "Cards cannot be accessed without a customer, recipient or account ID. " . + "Call save() on \$customer->sources->retrieve('card_id'), " . + "\$recipient->cards->retrieve('card_id'), or"; + "\$account->external_accounts->retrieve('card_id') instead."; + throw new Error\InvalidRequest($msg, null); + } } diff --git a/lib/Customer.php b/lib/Customer.php index e837222df..9f39699d6 100644 --- a/lib/Customer.php +++ b/lib/Customer.php @@ -139,7 +139,7 @@ public function deleteDiscount() * @param array|null $params * @param array|string|null $opts * - * @return ExternalAccount + * @return ApiResource */ public static function createSource($id, $params = null, $opts = null) { @@ -152,7 +152,7 @@ public static function createSource($id, $params = null, $opts = null) * @param array|null $params * @param array|string|null $opts * - * @return ExternalAccount + * @return ApiResource */ public static function retrieveSource($id, $sourceId, $params = null, $opts = null) { @@ -165,7 +165,7 @@ public static function retrieveSource($id, $sourceId, $params = null, $opts = nu * @param array|null $params * @param array|string|null $opts * - * @return ExternalAccount + * @return ApiResource */ public static function updateSource($id, $sourceId, $params = null, $opts = null) { @@ -178,7 +178,7 @@ public static function updateSource($id, $sourceId, $params = null, $opts = null * @param array|null $params * @param array|string|null $opts * - * @return ExternalAccount + * @return ApiResource */ public static function deleteSource($id, $sourceId, $params = null, $opts = null) { @@ -190,7 +190,7 @@ public static function deleteSource($id, $sourceId, $params = null, $opts = null * @param array|null $params * @param array|string|null $opts * - * @return ExternalAccount + * @return ApiResource */ public static function allSources($id, $params = null, $opts = null) { diff --git a/lib/ExternalAccount.php b/lib/ExternalAccount.php deleted file mode 100644 index dff99f722..000000000 --- a/lib/ExternalAccount.php +++ /dev/null @@ -1,83 +0,0 @@ -_save($opts); - } - - /** - * @param array|null $params - * @param array|string|null $opts - * - * @return ExternalAccount The verified (or not) external account. - */ - public function verify($params = null, $opts = null) - { - if ($this['customer']) { - $url = $this->instanceUrl() . '/verify'; - list($response, $options) = $this->_request('post', $url, $params, $opts); - $this->refreshFrom($response, $options); - return $this; - } else { - $message = 'Only customer external accounts can be verified in this manner.'; - throw new Error\Api($message); - } - } -} diff --git a/tests/Stripe/AlipayAccountTest.php b/tests/Stripe/AlipayAccountTest.php new file mode 100644 index 000000000..7159f5444 --- /dev/null +++ b/tests/Stripe/AlipayAccountTest.php @@ -0,0 +1,76 @@ + self::TEST_RESOURCE_ID, + 'object' => 'card', + 'metadata' => [], + ]; + return AlipayAccount::constructFrom( + array_merge($params, $base), + new Util\RequestOptions() + ); + } + + public function testHasCorrectUrlForCustomer() + { + $resource = $this->createFixture(['customer' => 'cus_123']); + $this->assertSame( + "/v1/customers/cus_123/sources/" . self::TEST_RESOURCE_ID, + $resource->instanceUrl() + ); + } + + /** + * @expectedException \Stripe\Error\InvalidRequest + */ + public function testIsNotDirectlyRetrievable() + { + AlipayAccount::retrieve(self::TEST_RESOURCE_ID); + } + + public function testIsSaveable() + { + $resource = $this->createFixture(); + $resource->metadata["key"] = "value"; + $this->expectsRequest( + 'post', + '/v1/customers/cus_123/sources/' . self::TEST_RESOURCE_ID + ); + $resource->save(); + $this->assertSame("Stripe\\AlipayAccount", get_class($resource)); + } + + /** + * @expectedException \Stripe\Error\InvalidRequest + */ + public function testIsNotDirectlyUpdatable() + { + AlipayAccount::update(self::TEST_RESOURCE_ID, [ + "metadata" => ["key" => "value"], + ]); + } + + public function testIsDeletable() + { + $resource = $this->createFixture(); + $this->expectsRequest( + 'delete', + '/v1/customers/cus_123/sources/' . self::TEST_RESOURCE_ID + ); + $resource->delete(); + $this->assertSame("Stripe\\AlipayAccount", get_class($resource)); + } +} diff --git a/tests/Stripe/BankAccountTest.php b/tests/Stripe/BankAccountTest.php index 3597c2bbf..ab1dff9d5 100644 --- a/tests/Stripe/BankAccountTest.php +++ b/tests/Stripe/BankAccountTest.php @@ -6,13 +6,86 @@ class BankAccountTest extends TestCase { const TEST_RESOURCE_ID = 'ba_123'; - public function testIsVerifiable() + // Because of the wildcard nature of sources, stripe-mock cannot currently + // reliably return sources of a given type, so we create a fixture manually + public function createFixture($params = []) { - $resource = BankAccount::constructFrom([ + if (empty($params)) { + $params['customer'] = 'cus_123'; + } + $base = [ 'id' => self::TEST_RESOURCE_ID, 'object' => 'bank_account', - 'customer' => 'cus_123', + 'metadata' => [], + ]; + return BankAccount::constructFrom( + array_merge($params, $base), + new Util\RequestOptions() + ); + } + + public function testHasCorrectUrlForCustomer() + { + $resource = $this->createFixture(['customer' => 'cus_123']); + $this->assertSame( + "/v1/customers/cus_123/sources/" . self::TEST_RESOURCE_ID, + $resource->instanceUrl() + ); + } + + public function testHasCorrectUrlForAccount() + { + $resource = $this->createFixture(['account' => 'acct_123']); + $this->assertSame( + "/v1/accounts/acct_123/external_accounts/" . self::TEST_RESOURCE_ID, + $resource->instanceUrl() + ); + } + + /** + * @expectedException \Stripe\Error\InvalidRequest + */ + public function testIsNotDirectlyRetrievable() + { + BankAccount::retrieve(self::TEST_RESOURCE_ID); + } + + public function testIsSaveable() + { + $resource = $this->createFixture(); + $resource->metadata["key"] = "value"; + $this->expectsRequest( + 'post', + '/v1/customers/cus_123/sources/' . self::TEST_RESOURCE_ID + ); + $resource->save(); + $this->assertSame("Stripe\\BankAccount", get_class($resource)); + } + + /** + * @expectedException \Stripe\Error\InvalidRequest + */ + public function testIsNotDirectlyUpdatable() + { + BankAccount::update(self::TEST_RESOURCE_ID, [ + "metadata" => ["key" => "value"], ]); + } + + public function testIsDeletable() + { + $resource = $this->createFixture(); + $this->expectsRequest( + 'delete', + '/v1/customers/cus_123/sources/' . self::TEST_RESOURCE_ID + ); + $resource->delete(); + $this->assertSame("Stripe\\BankAccount", get_class($resource)); + } + + public function testIsVerifiable() + { + $resource = $this->createFixture(); $this->expectsRequest( 'post', '/v1/customers/cus_123/sources/' . self::TEST_RESOURCE_ID . "/verify", diff --git a/tests/Stripe/BitcoinReceiverTest.php b/tests/Stripe/BitcoinReceiverTest.php index b958b1e9d..cea999fc4 100644 --- a/tests/Stripe/BitcoinReceiverTest.php +++ b/tests/Stripe/BitcoinReceiverTest.php @@ -4,23 +4,59 @@ class BitcoinReceiverTest extends TestCase { - public function testUrls() + const TEST_RESOURCE_ID = 'btcrcv_123'; + + // Because of the wildcard nature of sources, stripe-mock cannot currently + // reliably return sources of a given type, so we create a fixture manually + public function createFixture($params = []) + { + $base = [ + 'id' => self::TEST_RESOURCE_ID, + 'object' => 'bitcoin_receiver', + 'metadata' => [], + ]; + return BitcoinReceiver::constructFrom( + array_merge($params, $base), + new Util\RequestOptions() + ); + } + + public function testHasCorrectStandaloneUrl() + { + $resource = $this->createFixture(); + $this->assertSame( + "/v1/bitcoin/receivers/" . self::TEST_RESOURCE_ID, + $resource->instanceUrl() + ); + } + + public function testHasCorrectUrlForCustomer() + { + $resource = $this->createFixture(['customer' => 'cus_123']); + $this->assertSame( + "/v1/customers/cus_123/sources/" . self::TEST_RESOURCE_ID, + $resource->instanceUrl() + ); + } + + public function testIsListable() { - $classUrl = BitcoinReceiver::classUrl('Stripe_BitcoinReceiver'); - $this->assertSame($classUrl, '/v1/bitcoin/receivers'); - $receiver = new BitcoinReceiver('abcd/efgh'); - $instanceUrl = $receiver->instanceUrl(); - $this->assertSame($instanceUrl, '/v1/bitcoin/receivers/abcd%2Fefgh'); + $this->expectsRequest( + 'get', + '/v1/bitcoin/receivers' + ); + $resources = BitcoinReceiver::all(); + $this->assertTrue(is_array($resources->data)); + $this->assertSame("Stripe\\BitcoinReceiver", get_class($resources->data[0])); } - // - // Note that there are no tests of consequences in here. The Bitcoin - // endpoints have been deprecated in favor of the generic sources API. The - // BitcoinReceiver class has been left in place for some backwards - // compatibility, but all users should be migrating off of it. The tests - // have been removed because we no longer have the API endpoints required - // to run them. - // - // [1] https://stripe.com/docs/sources - // + public function testIsRetrievable() + { + $this->expectsRequest( + 'get', + '/v1/bitcoin/receivers/' . self::TEST_RESOURCE_ID + ); + $resource = BitcoinReceiver::retrieve(self::TEST_RESOURCE_ID); + $this->assertSame("Stripe\\BitcoinReceiver", get_class($resource)); + } } diff --git a/tests/Stripe/CardTest.php b/tests/Stripe/CardTest.php new file mode 100644 index 000000000..8976eff66 --- /dev/null +++ b/tests/Stripe/CardTest.php @@ -0,0 +1,94 @@ + self::TEST_RESOURCE_ID, + 'object' => 'card', + 'metadata' => [], + ]; + return Card::constructFrom( + array_merge($params, $base), + new Util\RequestOptions() + ); + } + + public function testHasCorrectUrlForCustomer() + { + $resource = $this->createFixture(['customer' => 'cus_123']); + $this->assertSame( + "/v1/customers/cus_123/sources/" . self::TEST_RESOURCE_ID, + $resource->instanceUrl() + ); + } + + public function testHasCorrectUrlForAccount() + { + $resource = $this->createFixture(['account' => 'acct_123']); + $this->assertSame( + "/v1/accounts/acct_123/external_accounts/" . self::TEST_RESOURCE_ID, + $resource->instanceUrl() + ); + } + + public function testHasCorrectUrlForRecipient() + { + $resource = $this->createFixture(['recipient' => 'rp_123']); + $this->assertSame( + "/v1/recipients/rp_123/cards/" . self::TEST_RESOURCE_ID, + $resource->instanceUrl() + ); + } + + /** + * @expectedException \Stripe\Error\InvalidRequest + */ + public function testIsNotDirectlyRetrievable() + { + Card::retrieve(self::TEST_RESOURCE_ID); + } + + public function testIsSaveable() + { + $resource = $this->createFixture(); + $resource->metadata["key"] = "value"; + $this->expectsRequest( + 'post', + '/v1/customers/cus_123/sources/' . self::TEST_RESOURCE_ID + ); + $resource->save(); + $this->assertSame("Stripe\\Card", get_class($resource)); + } + + /** + * @expectedException \Stripe\Error\InvalidRequest + */ + public function testIsNotDirectlyUpdatable() + { + Card::update(self::TEST_RESOURCE_ID, [ + "metadata" => ["key" => "value"], + ]); + } + + public function testIsDeletable() + { + $resource = $this->createFixture(); + $this->expectsRequest( + 'delete', + '/v1/customers/cus_123/sources/' . self::TEST_RESOURCE_ID + ); + $resource->delete(); + $this->assertSame("Stripe\\Card", get_class($resource)); + } +}