Fixes after review, more tests

stripe · Jan 6, 2018 · 7ab25b9 · 7ab25b9
1 parent ba0d1d3
commit 7ab25b9
Show file tree

Hide file tree

Showing 6 changed files with 207 additions and 31 deletions.
diff --git a/lib/Account.php b/lib/Account.php
@@ -209,7 +209,7 @@ private function serializeAdditionalOwners($legalEntity, $additionalOwners)
             $originalValue = [];
         }
         if (($originalValue) && (count($originalValue) > count($additionalOwners))) {
-            throw new InvalidArgumentException(
+            throw new \InvalidArgumentException(
                 "You cannot delete an item from an array, you must instead set a new array"
             );
         }

diff --git a/lib/ApiResource.php b/lib/ApiResource.php
@@ -11,7 +11,7 @@ abstract class ApiResource extends StripeObject
 {
     /**
      * @return Stripe\Util\Set A list of fields that can be their own type of
-     * API resource (say a nested card under an account fore xample), and if
+     * API resource (say a nested card under an account for example), and if
      * that resource is set, it should be transmitted to the API on a create or
      * update. Doing so is not the default behavior because API resources
      * should normally be persisted on their own RESTful endpoints.
@@ -45,8 +45,9 @@ public static function getSavedNestedResources()
     public function __set($k, $v)
     {
         parent::__set($k, $v);
-        $v = parent::__get($k);
-        if (static::getSavedNestedResources()->includes($k)) {
+        $v = $this->$k;
+        if ((static::getSavedNestedResources()->includes($k)) &&
+            ($v instanceof ApiResource)) {
             $v->saveWithParent = true;
         }
         return $v;

diff --git a/lib/StripeObject.php b/lib/StripeObject.php
@@ -285,8 +285,8 @@ public function serializeParamsValue($value, $original, $unsaved, $force, $key =
         //
         // Another example is that on save API calls it's sometimes desirable to
         // update a customer's default source by setting a new card (or other)
-        // object with `#source=` and then saving the customer. The
-        // `#save_with_parent` flag to override the default behavior allows us to
+        // object with `->source=` and then saving the customer. The
+        // `saveWithParent` flag to override the default behavior allows us to
         // handle these exceptions.
         //
         // We throw an error if a property was set explicitly but we can't do

diff --git a/tests/Stripe/AccountTest.php b/tests/Stripe/AccountTest.php
@@ -267,4 +267,99 @@ public function testSerializeUnsetAdditionalOwners()
         ];
         $this->assertSame($expected, $obj->serializeParameters());
     }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     */
+    public function testSerializeAdditionalOwnersDeletedItem()
+    {
+        $obj = Util\Util::convertToStripeObject([
+            'object' => 'account',
+            'legal_entity' => [
+                'additional_owners' => [
+                    StripeObject::constructFrom(['first_name' => 'Joe']),
+                    StripeObject::constructFrom(['first_name' => 'Jane']),
+                ],
+            ],
+        ], null);
+        unset($obj->legal_entity->additional_owners[0]);
+
+        $obj->serializeParameters();
+    }
+
+    public function testSerializeExternalAccountString()
+    {
+        $obj = Util\Util::convertToStripeObject([
+            'object' => 'account',
+        ], null);
+        $obj->external_account = 'btok_123';
+
+        $expected = [
+            'external_account' => 'btok_123',
+        ];
+        $this->assertSame($expected, $obj->serializeParameters());
+    }
+
+    public function testSerializeExternalAccountHash()
+    {
+        $obj = Util\Util::convertToStripeObject([
+            'object' => 'account',
+        ], null);
+        $obj->external_account = [
+            'object' => 'bank_account',
+            'routing_number' => '110000000',
+            'account_number' => '000123456789',
+            'country' => 'US',
+            'currency' => 'usd',
+        ];
+
+        $expected = [
+            'external_account' => [
+                'object' => 'bank_account',
+                'routing_number' => '110000000',
+                'account_number' => '000123456789',
+                'country' => 'US',
+                'currency' => 'usd',
+            ],
+        ];
+        $this->assertSame($expected, $obj->serializeParameters());
+    }
+
+    public function testSerializeBankAccountString()
+    {
+        $obj = Util\Util::convertToStripeObject([
+            'object' => 'account',
+        ], null);
+        $obj->bank_account = 'btok_123';
+
+        $expected = [
+            'bank_account' => 'btok_123',
+        ];
+        $this->assertSame($expected, $obj->serializeParameters());
+    }
+
+    public function testSerializeBankAccountHash()
+    {
+        $obj = Util\Util::convertToStripeObject([
+            'object' => 'account',
+        ], null);
+        $obj->bank_account = [
+            'object' => 'bank_account',
+            'routing_number' => '110000000',
+            'account_number' => '000123456789',
+            'country' => 'US',
+            'currency' => 'usd',
+        ];
+
+        $expected = [
+            'bank_account' => [
+                'object' => 'bank_account',
+                'routing_number' => '110000000',
+                'account_number' => '000123456789',
+                'country' => 'US',
+                'currency' => 'usd',
+            ],
+        ];
+        $this->assertSame($expected, $obj->serializeParameters());
+    }
 }
diff --git a/tests/Stripe/CustomerTest.php b/tests/Stripe/CustomerTest.php
@@ -230,4 +230,40 @@ public function testCanListSources()
         $resources = Customer::allSources(self::TEST_RESOURCE_ID);
         $this->assertTrue(is_array($resources->data));
     }
+
+    public function testSerializeSourceString()
+    {
+        $obj = Util\Util::convertToStripeObject([
+            'object' => 'customer',
+        ], null);
+        $obj->source = 'tok_visa';
+
+        $expected = [
+            'source' => 'tok_visa',
+        ];
+        $this->assertSame($expected, $obj->serializeParameters());
+    }
+
+    public function testSerializeSourceMap()
+    {
+        $obj = Util\Util::convertToStripeObject([
+            'object' => 'customer',
+        ], null);
+        $obj->source = [
+            'object' => 'card',
+            'number' => '4242424242424242',
+            'exp_month' => 12,
+            'exp_year' => 2032,
+        ];
+
+        $expected = [
+            'source' => [
+                'object' => 'card',
+                'number' => '4242424242424242',
+                'exp_month' => 12,
+                'exp_year' => 2032,
+            ],
+        ];
+        $this->assertSame($expected, $obj->serializeParameters());
+    }
 }
diff --git a/tests/Stripe/StripeObjectTest.php b/tests/Stripe/StripeObjectTest.php
@@ -4,6 +4,23 @@
 
 class StripeObjectTest extends TestCase
 {
+    /**
+     * @before
+     */
+    public function setUpReflectors()
+    {
+        // Sets up reflectors needed by some tests to access protected or
+        // private attributes.
+
+        // This is used to invoke the `deepCopy` protected function
+        $this->deepCopyReflector = new \ReflectionMethod('Stripe\\StripeObject', 'deepCopy');
+        $this->deepCopyReflector->setAccessible(true);
+
+        // This is used to access the `_opts` protected variable
+        $this->optsReflector = new \ReflectionProperty('Stripe\\StripeObject', '_opts');
+        $this->optsReflector->setAccessible(true);
+    }
+
     public function testArrayAccessorsSemantics()
     {
         $s = new StripeObject();
@@ -109,7 +126,21 @@ public function testJsonEncode()
         $s = new StripeObject();
         $s->foo = 'a';
 
-        $this->assertEquals('{"foo":"a"}', json_encode($s->__toArray()));
+        $this->assertEquals('{"foo":"a"}', json_encode($s));
+    }
+
+    public function testToString()
+    {
+        $s = new StripeObject();
+        $s->foo = 'a';
+
+        $string = $s->__toString();
+        $expected = <<<EOS
+Stripe\StripeObject JSON: {
+    "foo": "a"
+}
+EOS;
+        $this->assertEquals($expected, $string);
     }
 
     public function testReplaceNewNestedUpdatable()
@@ -122,6 +153,24 @@ public function testReplaceNewNestedUpdatable()
         $this->assertSame($s->metadata, ['baz', 'qux']);
     }
 
+    /**
+     * @expectedException \InvalidArgumentException
+     */
+    public function testSetPermanentAttribute()
+    {
+        $s = new StripeObject();
+        $s->id = 'abc_123';
+    }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     */
+    public function testSetEmptyStringValue()
+    {
+        $s = new StripeObject();
+        $s->foo = '';
+    }
+
     public function testSerializeParametersOnEmptyObject()
     {
         $obj = StripeObject::constructFrom([]);
@@ -329,14 +378,6 @@ public function testDirty()
 
     public function testDeepCopy()
     {
-        // This is used to invoke the `deepCopy` protected function
-        $deepCopyReflector = new \ReflectionMethod('Stripe\\StripeObject', 'deepCopy');
-        $deepCopyReflector->setAccessible(true);
-
-        // This is used to access the `_opts` protected variable
-        $optsReflector = new \ReflectionProperty('Stripe\\StripeObject', '_opts');
-        $optsReflector->setAccessible(true);
-
         $opts = [
             "api_base" => Stripe::$apiBase,
             "api_key" => "apikey",
@@ -355,55 +396,58 @@ public function testDeepCopy()
                 "2" => 2
             ],
         ];
-        $copyValues = $deepCopyReflector->invoke(null, $values);
+
+        $copyValues = $this->deepCopyReflector->invoke(null, $values);
+
         // we can't compare the hashes directly because they have embedded
         // objects which are different from each other
         $this->assertEquals($values["id"], $copyValues["id"]);
         $this->assertEquals($values["name"], $copyValues["name"]);
         $this->assertEquals(count($values["arr"]), count($copyValues["arr"]));
+
         // internal values of the copied StripeObject should be the same,
         // but the object itself should be new (hence the assertNotSame)
         $this->assertEquals($values["arr"][0]["id"], $copyValues["arr"][0]["id"]);
         $this->assertNotSame($values["arr"][0], $copyValues["arr"][0]);
+
         // likewise, the Util\RequestOptions instance in _opts should have
         // copied values but be a new instance
         $this->assertEquals(
-            $optsReflector->getValue($values["arr"][0]),
-            $optsReflector->getValue($copyValues["arr"][0])
+            $this->optsReflector->getValue($values["arr"][0]),
+            $this->optsReflector->getValue($copyValues["arr"][0])
         );
         $this->assertNotSame(
-            $optsReflector->getValue($values["arr"][0]),
-            $optsReflector->getValue($copyValues["arr"][0])
+            $this->optsReflector->getValue($values["arr"][0]),
+            $this->optsReflector->getValue($copyValues["arr"][0])
         );
+
         // scalars however, can be compared
         $this->assertEquals($values["arr"][1], $copyValues["arr"][1]);
         $this->assertEquals($values["arr"][2], $copyValues["arr"][2]);
+
         // and a similar story with the hash
         $this->assertEquals($values["map"]["0"]["id"], $copyValues["map"]["0"]["id"]);
         $this->assertNotSame($values["map"]["0"], $copyValues["map"]["0"]);
         $this->assertNotSame(
-            $optsReflector->getValue($values["arr"][0]),
-            $optsReflector->getValue($copyValues["arr"][0])
+            $this->optsReflector->getValue($values["arr"][0]),
+            $this->optsReflector->getValue($copyValues["arr"][0])
         );
         $this->assertEquals(
-            $optsReflector->getValue($values["map"]["0"]),
-            $optsReflector->getValue($copyValues["map"]["0"])
+            $this->optsReflector->getValue($values["map"]["0"]),
+            $this->optsReflector->getValue($copyValues["map"]["0"])
         );
         $this->assertNotSame(
-            $optsReflector->getValue($values["map"]["0"]),
-            $optsReflector->getValue($copyValues["map"]["0"])
+            $this->optsReflector->getValue($values["map"]["0"]),
+            $this->optsReflector->getValue($copyValues["map"]["0"])
         );
         $this->assertEquals($values["map"]["1"], $copyValues["map"]["1"]);
         $this->assertEquals($values["map"]["2"], $copyValues["map"]["2"]);
     }
+
     public function testDeepCopyMaintainClass()
     {
-        // This is used to invoke the `deepCopy` protected function
-        $deepCopyReflector = new \ReflectionMethod('Stripe\\StripeObject', 'deepCopy');
-        $deepCopyReflector->setAccessible(true);
-
         $charge = Charge::constructFrom(["id" => 1], null);
-        $copyCharge = $deepCopyReflector->invoke(null, $charge);
+        $copyCharge = $this->deepCopyReflector->invoke(null, $charge);
         $this->assertEquals(get_class($charge), get_class($copyCharge));
     }
 }