From 1751c911b497c37cb60bdf74eeb4073b04ef423c Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 13 Nov 2017 19:57:47 +0100 Subject: [PATCH 1/3] StripeObject: only add changed values to _unsaved_values --- stripe/stripe_object.py | 11 +++++---- .../abstract/test_updateable_api_resource.py | 24 +++++++++++++++++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/stripe/stripe_object.py b/stripe/stripe_object.py index 4f54d324b..e07f527e8 100644 --- a/stripe/stripe_object.py +++ b/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: diff --git a/tests/api_resources/abstract/test_updateable_api_resource.py b/tests/api_resources/abstract/test_updateable_api_resource.py index 2ea388eed..7b1e946d2 100644 --- a/tests/api_resources/abstract/test_updateable_api_resource.py +++ b/tests/api_resources/abstract/test_updateable_api_resource.py @@ -75,6 +75,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', From 39a40c34988b2189b43b10ceeff97b1038f015fb Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 13 Nov 2017 22:20:06 +0100 Subject: [PATCH 2/3] StripeObject.serialize: omit empty childs --- stripe/stripe_object.py | 4 +++- .../abstract/test_updateable_api_resource.py | 18 ++---------------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/stripe/stripe_object.py b/stripe/stripe_object.py index e07f527e8..738fae944 100644 --- a/stripe/stripe_object.py +++ b/stripe/stripe_object.py @@ -238,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: diff --git a/tests/api_resources/abstract/test_updateable_api_resource.py b/tests/api_resources/abstract/test_updateable_api_resource.py index 7b1e946d2..7ece53ccf 100644 --- a/tests/api_resources/abstract/test_updateable_api_resource.py +++ b/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', }, { @@ -134,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({ @@ -209,7 +201,6 @@ def test_array_none(self): '/v1/myupdateables/myid', { 'foo': 'bar', - 'legal_entity': {}, }, None ) @@ -298,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' From 3c50f68858ab9768998d471cdff9f052a159190b Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Wed, 15 Nov 2017 22:35:50 +0100 Subject: [PATCH 3/3] compare to dict with test --- stripe/stripe_object.py | 2 +- tests/test_stripe_object.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/stripe/stripe_object.py b/stripe/stripe_object.py index 738fae944..7ea9e840c 100644 --- a/stripe/stripe_object.py +++ b/stripe/stripe_object.py @@ -239,7 +239,7 @@ def serialize(self, previous): continue elif hasattr(v, 'serialize'): child = v.serialize(previous.get(k, None)) - if child: + if child != {}: params[k] = child elif k in unsaved_keys: params[k] = _compute_diff(v, previous.get(k, None)) diff --git a/tests/test_stripe_object.py b/tests/test_stripe_object.py index f0253a6c6..bafcc01c7 100644 --- a/tests/test_stripe_object.py +++ b/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': ''})