-
Notifications
You must be signed in to change notification settings - Fork 852
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
Fix parameter serialization logic #404
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,8 +40,24 @@ class Account extends ApiResource | |
use ApiOperations\All; | ||
use ApiOperations\Create; | ||
use ApiOperations\Delete; | ||
use ApiOperations\NestedResource; | ||
use ApiOperations\Retrieve { | ||
retrieve as protected _retrieve; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resources that have their own custom implementation of API methods but need to call the standard implementation (usually because they have some additional parameter checking) can use this syntax to "inherit" the trait methods under a different name. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's pretty cool. Thanks for the ntoe! |
||
} | ||
use ApiOperations\Update; | ||
|
||
public static function getSavedNestedResources() | ||
{ | ||
static $savedNestedResources = null; | ||
if ($savedNestedResources === null) { | ||
$savedNestedResources = new Util\Set([ | ||
'external_account', | ||
'bank_account', | ||
]); | ||
} | ||
return $savedNestedResources; | ||
} | ||
|
||
const PATH_EXTERNAL_ACCOUNTS = '/external_accounts'; | ||
const PATH_LOGIN_LINKS = '/login_links'; | ||
|
||
|
@@ -173,4 +189,45 @@ public static function createLoginLink($id, $params = null, $opts = null) | |
{ | ||
return self::_createNestedResource($id, static::PATH_LOGIN_LINKS, $params, $opts); | ||
} | ||
|
||
public function serializeParameters($force = false) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just OOC, is this mostly copied from the Ruby implementation? The test suite looks like it's pretty good in terms of vetting the new code, but just wondering how much we should expect to have to worry about it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, it's basically a straight port from the Ruby implementation. |
||
{ | ||
$update = parent::serializeParameters($force); | ||
if (isset($this->_values['legal_entity'])) { | ||
$entity = $this['legal_entity']; | ||
if (isset($entity->_values['additional_owners'])) { | ||
$owners = $entity['additional_owners']; | ||
$entityUpdate = isset($update['legal_entity']) ? $update['legal_entity'] : []; | ||
$entityUpdate['additional_owners'] = $this->serializeAdditionalOwners($entity, $owners); | ||
$update['legal_entity'] = $entityUpdate; | ||
} | ||
} | ||
return $update; | ||
} | ||
|
||
private function serializeAdditionalOwners($legalEntity, $additionalOwners) | ||
{ | ||
if (isset($legalEntity->_originalValues['additional_owners'])) { | ||
$originalValue = $legalEntity->_originalValues['additional_owners']; | ||
} else { | ||
$originalValue = []; | ||
} | ||
if (($originalValue) && (count($originalValue) > count($additionalOwners))) { | ||
throw new \InvalidArgumentException( | ||
"You cannot delete an item from an array, you must instead set a new array" | ||
); | ||
} | ||
|
||
$updateArr = []; | ||
foreach ($additionalOwners as $i => $v) { | ||
$update = ($v instanceof StripeObject) ? $v->serializeParameters() : $v; | ||
|
||
if ($update !== []) { | ||
if (!$originalValue || ($update != $legalEntity->serializeParamsValue($originalValue[$i], null, false, true))) { | ||
$updateArr[$i] = $update; | ||
} | ||
} | ||
} | ||
return $updateArr; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
<?php | ||
|
||
namespace Stripe\ApiOperations; | ||
|
||
/** | ||
* Trait for resources that have nested resources. | ||
* | ||
* This trait should only be applied to classes that derive from StripeObject. | ||
*/ | ||
trait NestedResource | ||
{ | ||
/** | ||
* @param string $method | ||
* @param string $url | ||
* @param array|null $params | ||
* @param array|string|null $options | ||
* | ||
* @return Stripe\StripeObject | ||
*/ | ||
protected static function _nestedResourceOperation($method, $url, $params = null, $options = null) | ||
{ | ||
self::_validateParams($params); | ||
|
||
list($response, $opts) = static::_staticRequest($method, $url, $params, $options); | ||
$obj = \Stripe\Util\Util::convertToStripeObject($response->json, $opts); | ||
$obj->setLastResponse($response); | ||
return $obj; | ||
} | ||
|
||
/** | ||
* @param string $id | ||
* @param string $nestedPath | ||
* @param string|null $nestedId | ||
* | ||
* @return string | ||
*/ | ||
protected static function _nestedResourceUrl($id, $nestedPath, $nestedId = null) | ||
{ | ||
$url = static::resourceUrl($id) . $nestedPath; | ||
if ($nestedId !== null) { | ||
$url .= "/$nestedId"; | ||
} | ||
return $url; | ||
} | ||
|
||
/** | ||
* @param string $id | ||
* @param string $nestedPath | ||
* @param array|null $params | ||
* @param array|string|null $options | ||
* | ||
* @return Stripe\StripeObject | ||
*/ | ||
protected static function _createNestedResource($id, $nestedPath, $params = null, $options = null) | ||
{ | ||
$url = static::_nestedResourceUrl($id, $nestedPath); | ||
return self::_nestedResourceOperation('post', $url, $params, $options); | ||
} | ||
|
||
/** | ||
* @param string $id | ||
* @param string $nestedPath | ||
* @param array|null $params | ||
* @param array|string|null $options | ||
* | ||
* @return Stripe\StripeObject | ||
*/ | ||
protected static function _retrieveNestedResource($id, $nestedPath, $nestedId, $params = null, $options = null) | ||
{ | ||
$url = static::_nestedResourceUrl($id, $nestedPath, $nestedId); | ||
return self::_nestedResourceOperation('get', $url, $params, $options); | ||
} | ||
|
||
/** | ||
* @param string $id | ||
* @param string $nestedPath | ||
* @param array|null $params | ||
* @param array|string|null $options | ||
* | ||
* @return Stripe\StripeObject | ||
*/ | ||
protected static function _updateNestedResource($id, $nestedPath, $nestedId, $params = null, $options = null) | ||
{ | ||
$url = static::_nestedResourceUrl($id, $nestedPath, $nestedId); | ||
return self::_nestedResourceOperation('post', $url, $params, $options); | ||
} | ||
|
||
/** | ||
* @param string $id | ||
* @param string $nestedPath | ||
* @param array|null $params | ||
* @param array|string|null $options | ||
* | ||
* @return Stripe\StripeObject | ||
*/ | ||
protected static function _deleteNestedResource($id, $nestedPath, $nestedId, $params = null, $options = null) | ||
{ | ||
$url = static::_nestedResourceUrl($id, $nestedPath, $nestedId); | ||
return self::_nestedResourceOperation('delete', $url, $params, $options); | ||
} | ||
|
||
/** | ||
* @param string $id | ||
* @param string $nestedPath | ||
* @param array|null $params | ||
* @param array|string|null $options | ||
* | ||
* @return Stripe\StripeObject | ||
*/ | ||
protected static function _allNestedResources($id, $nestedPath, $params = null, $options = null) | ||
{ | ||
$url = static::_nestedResourceUrl($id, $nestedPath); | ||
return self::_nestedResourceOperation('get', $url, $params, $options); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
<?php | ||
|
||
namespace Stripe\ApiOperations; | ||
|
||
/** | ||
* Trait for resources that need to make API requests. | ||
* | ||
* This trait should only be applied to classes that derive from StripeObject. | ||
*/ | ||
trait Request | ||
{ | ||
/** | ||
* @param array|null|mixed $params The list of parameters to validate | ||
* | ||
* @throws Stripe\Error\Api if $params exists and is not an array | ||
*/ | ||
protected static function _validateParams($params = null) | ||
{ | ||
if ($params && !is_array($params)) { | ||
$message = "You must pass an array as the first argument to Stripe API " | ||
. "method calls. (HINT: an example call to create a charge " | ||
. "would be: \"Stripe\\Charge::create(['amount' => 100, " | ||
. "'currency' => 'usd', 'source' => 'tok_1234'])\")"; | ||
throw new Error\Api($message); | ||
} | ||
} | ||
|
||
/** | ||
* @param string $method HTTP method ('get', 'post', etc.) | ||
* @param string $url URL for the request | ||
* @param array $params list of parameters for the request | ||
* @param array|string|null $options | ||
* | ||
* @return array tuple containing (the JSON response, $options) | ||
*/ | ||
protected function _request($method, $url, $params = [], $options = null) | ||
{ | ||
$opts = $this->_opts->merge($options); | ||
list($resp, $options) = static::_staticRequest($method, $url, $params, $opts); | ||
$this->setLastResponse($resp); | ||
return [$resp->json, $options]; | ||
} | ||
|
||
/** | ||
* @param string $method HTTP method ('get', 'post', etc.) | ||
* @param string $url URL for the request | ||
* @param array $params list of parameters for the request | ||
* @param array|string|null $options | ||
* | ||
* @return array tuple containing (the JSON response, $options) | ||
*/ | ||
protected static function _staticRequest($method, $url, $params, $options) | ||
{ | ||
$opts = \Stripe\Util\RequestOptions::parse($options); | ||
$requestor = new \Stripe\ApiRequestor($opts->apiKey, static::baseUrl()); | ||
list($response, $opts->apiKey) = $requestor->request($method, $url, $params, $opts->headers); | ||
foreach ($opts->headers as $k => $v) { | ||
if (!array_key_exists($k, self::$HEADERS_TO_PERSIST)) { | ||
unset($opts->headers[$k]); | ||
} | ||
} | ||
return [$response, $opts]; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftover from manual debugging 😨
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops. Totally missed that apparently, haha.