From 76ecf456b081d2dfb9d4b5e049f54e942a72079e Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Fri, 15 May 2020 10:02:25 -0400 Subject: [PATCH 1/7] Make ->formatParams recursive --- lib/Service/AbstractService.php | 7 +++++-- tests/Stripe/Service/AbstractServiceTest.php | 12 ++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/Service/AbstractService.php b/lib/Service/AbstractService.php index abcc13ae2..f51be6023 100644 --- a/lib/Service/AbstractService.php +++ b/lib/Service/AbstractService.php @@ -40,14 +40,17 @@ public function getClient() * * @param null|array $params */ - private static function formatParams($params) + public static function formatParams($params) { if (null === $params) { return $params; } + $formatted = []; foreach ($params as $k => $v) { - if (null === $v) { + if (\is_array($v)) { + $formatted[$k] = static::formatParams($v); + } elseif (null === $v) { $formatted[$k] = ''; } else { $formatted[$k] = $v; diff --git a/tests/Stripe/Service/AbstractServiceTest.php b/tests/Stripe/Service/AbstractServiceTest.php index deee3e386..0eac886d7 100644 --- a/tests/Stripe/Service/AbstractServiceTest.php +++ b/tests/Stripe/Service/AbstractServiceTest.php @@ -57,4 +57,16 @@ public function testRetrieveThrowsIfIdNullIsWhitespace() $this->service->retrieve(' '); } + + public function testFormatParams() + { + $result = \Stripe\Service\AbstractService::formatParams(['foo' => null]); + static::assertTrue('' === $result['foo']); + static::assertTrue(null !== $result['foo']); + + $result = \Stripe\Service\AbstractService::formatParams(['foo' => ['bar' => null, 'baz' => 1]]); + static::assertTrue('' === $result['foo']['bar']); + static::assertTrue(null !== $result['foo']['bar']); + static::assertTrue(1 === $result['foo']['baz']); + } } From d5bda98a9cbed9239013d42177def7bab0ce45fc Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Fri, 15 May 2020 11:09:25 -0400 Subject: [PATCH 2/7] Works for normal arrays too --- tests/Stripe/Service/AbstractServiceTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Stripe/Service/AbstractServiceTest.php b/tests/Stripe/Service/AbstractServiceTest.php index 0eac886d7..d4e9bd6f5 100644 --- a/tests/Stripe/Service/AbstractServiceTest.php +++ b/tests/Stripe/Service/AbstractServiceTest.php @@ -68,5 +68,11 @@ public function testFormatParams() static::assertTrue('' === $result['foo']['bar']); static::assertTrue(null !== $result['foo']['bar']); static::assertTrue(1 === $result['foo']['baz']); + + $result = \Stripe\Service\AbstractService::formatParams(['foo' => ["bar", null, null, "baz"]]); + static::assertTrue('bar' === $result['foo'][0]); + static::assertTrue('' === $result['foo'][1]); + static::assertTrue('' === $result['foo'][2]); + static::assertTrue('baz' === $result['foo'][3]); } } From 5c9e1f7993e3a148fb4c6e8e9fc4a79b09eeb21e Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Fri, 15 May 2020 11:49:01 -0400 Subject: [PATCH 3/7] More sophisticated test cases --- tests/Stripe/Service/AbstractServiceTest.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/Stripe/Service/AbstractServiceTest.php b/tests/Stripe/Service/AbstractServiceTest.php index d4e9bd6f5..e32b55b85 100644 --- a/tests/Stripe/Service/AbstractServiceTest.php +++ b/tests/Stripe/Service/AbstractServiceTest.php @@ -64,15 +64,19 @@ public function testFormatParams() static::assertTrue('' === $result['foo']); static::assertTrue(null !== $result['foo']); - $result = \Stripe\Service\AbstractService::formatParams(['foo' => ['bar' => null, 'baz' => 1]]); + $result = \Stripe\Service\AbstractService::formatParams(['foo' => ['bar' => null, 'baz' => 1, 'nest' => ["triplynestednull" => null, "triplynestednonnull" => 1]]]); static::assertTrue('' === $result['foo']['bar']); static::assertTrue(null !== $result['foo']['bar']); static::assertTrue(1 === $result['foo']['baz']); + static::assertTrue('' === $result['foo']['nest']['triplynestednull']); + static::assertTrue(1 === $result['foo']['nest']['triplynestednonnull']); - $result = \Stripe\Service\AbstractService::formatParams(['foo' => ["bar", null, null, "baz"]]); - static::assertTrue('bar' === $result['foo'][0]); + $result = \Stripe\Service\AbstractService::formatParams(['foo' => ["zero", null, null, "three"], 'toplevelnull' => null, 'toplevelnonnull' => 4]); + static::assertTrue('zero' === $result['foo'][0]); static::assertTrue('' === $result['foo'][1]); static::assertTrue('' === $result['foo'][2]); - static::assertTrue('baz' === $result['foo'][3]); + static::assertTrue('three' === $result['foo'][3]); + static::assertTrue('' === $result['toplevelnull']); + static::assertTrue(4 === $result['toplevelnonnull']); } } From 217ae375dcca8923a008cf7cabcfd28bbb609cfd Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Fri, 15 May 2020 12:03:49 -0400 Subject: [PATCH 4/7] Use array_walk_recursive --- lib/Service/AbstractService.php | 18 ++++++------------ tests/Stripe/Service/AbstractServiceTest.php | 4 ++-- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/lib/Service/AbstractService.php b/lib/Service/AbstractService.php index f51be6023..e78067120 100644 --- a/lib/Service/AbstractService.php +++ b/lib/Service/AbstractService.php @@ -43,21 +43,15 @@ public function getClient() public static function formatParams($params) { if (null === $params) { - return $params; + return null; } - - $formatted = []; - foreach ($params as $k => $v) { - if (\is_array($v)) { - $formatted[$k] = static::formatParams($v); - } elseif (null === $v) { - $formatted[$k] = ''; - } else { - $formatted[$k] = $v; + \array_walk_recursive($params, function (&$value, $key) { + if (null === $value) { + $value = ''; } - } + }); - return $formatted; + return $params; } protected function request($method, $path, $params, $opts) diff --git a/tests/Stripe/Service/AbstractServiceTest.php b/tests/Stripe/Service/AbstractServiceTest.php index e32b55b85..e60eacd18 100644 --- a/tests/Stripe/Service/AbstractServiceTest.php +++ b/tests/Stripe/Service/AbstractServiceTest.php @@ -64,14 +64,14 @@ public function testFormatParams() static::assertTrue('' === $result['foo']); static::assertTrue(null !== $result['foo']); - $result = \Stripe\Service\AbstractService::formatParams(['foo' => ['bar' => null, 'baz' => 1, 'nest' => ["triplynestednull" => null, "triplynestednonnull" => 1]]]); + $result = \Stripe\Service\AbstractService::formatParams(['foo' => ['bar' => null, 'baz' => 1, 'nest' => ['triplynestednull' => null, 'triplynestednonnull' => 1]]]); static::assertTrue('' === $result['foo']['bar']); static::assertTrue(null !== $result['foo']['bar']); static::assertTrue(1 === $result['foo']['baz']); static::assertTrue('' === $result['foo']['nest']['triplynestednull']); static::assertTrue(1 === $result['foo']['nest']['triplynestednonnull']); - $result = \Stripe\Service\AbstractService::formatParams(['foo' => ["zero", null, null, "three"], 'toplevelnull' => null, 'toplevelnonnull' => 4]); + $result = \Stripe\Service\AbstractService::formatParams(['foo' => ['zero', null, null, 'three'], 'toplevelnull' => null, 'toplevelnonnull' => 4]); static::assertTrue('zero' === $result['foo'][0]); static::assertTrue('' === $result['foo'][1]); static::assertTrue('' === $result['foo'][2]); From cd2ea0dc2b443d58a8f9e9ddb363ef5725653b20 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Fri, 15 May 2020 13:20:46 -0400 Subject: [PATCH 5/7] Make private --- lib/Service/AbstractService.php | 2 +- tests/Stripe/Service/AbstractServiceTest.php | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/Service/AbstractService.php b/lib/Service/AbstractService.php index e78067120..2cc759c07 100644 --- a/lib/Service/AbstractService.php +++ b/lib/Service/AbstractService.php @@ -40,7 +40,7 @@ public function getClient() * * @param null|array $params */ - public static function formatParams($params) + private static function formatParams($params) { if (null === $params) { return null; diff --git a/tests/Stripe/Service/AbstractServiceTest.php b/tests/Stripe/Service/AbstractServiceTest.php index e60eacd18..ef8a2dbd6 100644 --- a/tests/Stripe/Service/AbstractServiceTest.php +++ b/tests/Stripe/Service/AbstractServiceTest.php @@ -26,6 +26,15 @@ public function setUpMockService() $this->service = new \Stripe\Service\CouponService($this->client); } + /** + * @before + */ + public function setUpReflectors() + { + $this->formatParamsReflector = new \ReflectionMethod(\Stripe\Service\AbstractService::class, 'formatParams'); + $this->formatParamsReflector->setAccessible(true); + } + public function testNullGetsEmptyStringified() { $this->expectException(\Stripe\Exception\InvalidRequestException::class); @@ -60,18 +69,18 @@ public function testRetrieveThrowsIfIdNullIsWhitespace() public function testFormatParams() { - $result = \Stripe\Service\AbstractService::formatParams(['foo' => null]); + $result = $this->formatParamsReflector->invoke(null, ['foo' => null]); static::assertTrue('' === $result['foo']); static::assertTrue(null !== $result['foo']); - $result = \Stripe\Service\AbstractService::formatParams(['foo' => ['bar' => null, 'baz' => 1, 'nest' => ['triplynestednull' => null, 'triplynestednonnull' => 1]]]); + $result = $this->formatParamsReflector->invoke(null, ['foo' => ['bar' => null, 'baz' => 1, 'nest' => ['triplynestednull' => null, 'triplynestednonnull' => 1]]]); static::assertTrue('' === $result['foo']['bar']); static::assertTrue(null !== $result['foo']['bar']); static::assertTrue(1 === $result['foo']['baz']); static::assertTrue('' === $result['foo']['nest']['triplynestednull']); static::assertTrue(1 === $result['foo']['nest']['triplynestednonnull']); - $result = \Stripe\Service\AbstractService::formatParams(['foo' => ['zero', null, null, 'three'], 'toplevelnull' => null, 'toplevelnonnull' => 4]); + $result = $this->formatParamsReflector->invoke(null, ['foo' => ['zero', null, null, 'three'], 'toplevelnull' => null, 'toplevelnonnull' => 4]); static::assertTrue('zero' === $result['foo'][0]); static::assertTrue('' === $result['foo'][1]); static::assertTrue('' === $result['foo'][2]); From 3fbf561c984448063376f1820021d6e905bf9485 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Fri, 15 May 2020 13:27:23 -0400 Subject: [PATCH 6/7] Fix format --- tests/Stripe/Service/AbstractServiceTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Stripe/Service/AbstractServiceTest.php b/tests/Stripe/Service/AbstractServiceTest.php index ef8a2dbd6..d8b234ac2 100644 --- a/tests/Stripe/Service/AbstractServiceTest.php +++ b/tests/Stripe/Service/AbstractServiceTest.php @@ -31,8 +31,8 @@ public function setUpMockService() */ public function setUpReflectors() { - $this->formatParamsReflector = new \ReflectionMethod(\Stripe\Service\AbstractService::class, 'formatParams'); - $this->formatParamsReflector->setAccessible(true); + $this->formatParamsReflector = new \ReflectionMethod(\Stripe\Service\AbstractService::class, 'formatParams'); + $this->formatParamsReflector->setAccessible(true); } public function testNullGetsEmptyStringified() From 29ef98b84f0a26578e8673c9e476842f4c5f142a Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Fri, 15 May 2020 13:31:07 -0400 Subject: [PATCH 7/7] Satisfy phpstan --- tests/Stripe/Service/AbstractServiceTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Stripe/Service/AbstractServiceTest.php b/tests/Stripe/Service/AbstractServiceTest.php index d8b234ac2..cf79e063d 100644 --- a/tests/Stripe/Service/AbstractServiceTest.php +++ b/tests/Stripe/Service/AbstractServiceTest.php @@ -16,6 +16,9 @@ final class AbstractServiceTest extends \PHPUnit\Framework\TestCase /** @var CouponService */ private $service; + /** @var \ReflectionMethod */ + private $formatParamsReflector; + /** * @before */