Merge pull request #372 from blueyed/unsaved_values-changed_only

StripeObject: only add changed values to _unsaved_values

stripe/stripe_object.py

@@ -86,13 +86,14 @@ def __setitem__(self, k, v):
                 "You may set %s.%s = None to delete the property" % (
                     k, str(self), k))
 
-        super(StripeObject, self).__setitem__(k, v)
+        if not hasattr(self, k) or v != getattr(self, k):
+            # Allows for unpickling in Python 3.x
+            if not hasattr(self, '_unsaved_values'):
+                self._unsaved_values = set()
 
-        # Allows for unpickling in Python 3.x
-        if not hasattr(self, '_unsaved_values'):
-            self._unsaved_values = set()
+            self._unsaved_values.add(k)
 
-        self._unsaved_values.add(k)
+        super(StripeObject, self).__setitem__(k, v)
 
     def __getitem__(self, k):
         try:
@@ -237,7 +238,9 @@ def serialize(self, previous):
             elif isinstance(v, stripe.api_resources.abstract.APIResource):
                 continue
             elif hasattr(v, 'serialize'):
-                params[k] = v.serialize(previous.get(k, None))
+                child = v.serialize(previous.get(k, None))
+                if child != {}:
+                    params[k] = child
             elif k in unsaved_keys:
                 params[k] = _compute_diff(v, previous.get(k, None))
             elif k == 'additional_owners' and v is not None:

tests/api_resources/abstract/test_updateable_api_resource.py

@@ -43,7 +43,6 @@ def test_idempotent_save(self):
             'post',
             '/v1/myupdateables/myid',
             {
-                'metadata': {},
                 'baz': 'updated',
             },
             {
@@ -75,6 +74,30 @@ def test_save(self):
             None
         )
 
+        # Saving again should not cause any request.
+        self.requestor_mock.request.reset_mock()
+        self.checkSave()
+        self.assertFalse(self.requestor_mock.request.called)
+
+        # Setting the same value should not cause any request.
+        self.obj.thats = 'it'
+        self.checkSave()
+        self.assertFalse(self.requestor_mock.request.called)
+
+        # Changing the value should cause a request.
+        self.obj.id = 'myid'
+        self.obj.thats = 'updated'
+        self.checkSave()
+
+        self.requestor_mock.request.assert_called_with(
+            'post',
+            '/v1/myupdateables/myid',
+            {
+                'thats': 'updated',
+            },
+            None
+        )
+
     def test_add_key_to_nested_object(self):
         acct = MyUpdateable.construct_from({
             'id': 'myid',
@@ -110,14 +133,7 @@ def test_save_nothing(self):
 
         self.assertTrue(acct is acct.save())
 
-        # Note: ideally, we'd want the library to NOT issue requests in this
-        # case (i.e. the assert should actually be `assert_not_called()`).
-        self.requestor_mock.request.assert_called_with(
-            'post',
-            '/v1/myupdateables/myid',
-            {'metadata': {}},
-            None
-        )
+        self.requestor_mock.request.assert_not_called()
 
     def test_replace_nested_object(self):
         acct = MyUpdateable.construct_from({
@@ -185,7 +201,6 @@ def test_array_none(self):
             '/v1/myupdateables/myid',
             {
                 'foo': 'bar',
-                'legal_entity': {},
             },
             None
         )
@@ -274,12 +289,7 @@ def test_hash_noop(self):
 
         self.assertTrue(acct is acct.save())
 
-        self.requestor_mock.request.assert_called_with(
-            'post',
-            '/v1/myupdateables/myid',
-            {'legal_entity': {'address': {}}},
-            None
-        )
+        self.requestor_mock.request.assert_not_called()
 
     def test_save_replace_metadata_with_number(self):
         self.obj.baz = 'updated'

tests/test_stripe_object.py

@@ -267,3 +267,17 @@ def test_deepcopy(self):
 
         # Verify that we're actually deep copying nested values.
         self.assertNotEqual(id(nested), id(copied.nested))
+
+    def test_serialize_empty_string_unsets(self):
+        class SerializeToEmptyString(stripe.stripe_object.StripeObject):
+            def serialize(self, previous):
+                return ''
+
+        nested = SerializeToEmptyString.construct_from({
+            'value': 'bar',
+        }, 'mykey')
+        obj = stripe.stripe_object.StripeObject.construct_from({
+            'nested': nested,
+        }, 'mykey')
+
+        self.assertEqual(obj.serialize(None), {'nested': ''})