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

Convert filters of Collection to array #796

Closed
wants to merge 1 commit into from

Conversation

baijunyao
Copy link
Contributor

@baijunyao baijunyao commented Nov 6, 2019

The default value of filters is null when used as a parameter, An error is thrown when executing the array_merge function.

$params = array_merge(
$this->filters,
['starting_after' => $lastId],
$params ?: []
);

@ob-stripe
Copy link
Contributor

Hi @baijunyao, thanks for the contribution.

filters should never be null -- it's initialized with an empty array, not null:

protected $filters = [];

That said, I just modified this class' code (for unrelated reasons) in #797 and I added a safeguard so that the array_merge calls will succeed even if filters is null for some reason:

$params = array_merge(
$this->filters ?: [],
.

Closing this, but feel free to reply if you believe there's still a bug in stripe-php!

@ob-stripe ob-stripe closed this Nov 6, 2019
@baijunyao
Copy link
Contributor Author

baijunyao commented Nov 7, 2019

Sorry, I didn't say it clearly. I mean that filters in the parameter is null by default.

public function all($params = null, $opts = null)
{
self::_validateParams($params);
list($url, $params) = $this->extractPathAndUpdateParams($params);
list($response, $opts) = $this->_request('get', $url, $params, $opts);
$obj = Util\Util::convertToStripeObject($response, $opts);
if (!($obj instanceof \Stripe\Collection)) {
throw new \Stripe\Exception\UnexpectedValueException(
'Expected type ' . \Stripe\Collection::class . ', got "' . get_class($obj) . '" instead.'
);
}
$obj->setFilters($params);
return $obj;
}

The following line of code will set $this->filters to null.

$obj->setFilters($params);

public function setFilters($filters)
{
$this->filters = $filters;
unset($this->filters['starting_after']);
unset($this->filters['ending_before']);
}

The following code will report an error array_merge() expects parameter 1 to be array, null given.

    foreach (\Stripe\Account::all()->autoPagingIterator() as $payout) {
        var_dump($payout);
    }

The $this->filters ?: [] you added two hours ago solved this issue.
But array_key_exists can't accept null values either.
So I opened a new PR #798 to fix the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants