From 0d64b4a704eed2fb483de66a7e7271ac83144530 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Mon, 9 Jan 2012 15:56:41 +0100 Subject: [PATCH 1/2] Make a nested if flat This is a possible fix for issue #73. The problem occurs when the first if-statement is true, but the second is not. This results into the variable obj not being set. This commit solves it by removing that branch. --- djangorestframework/serializer.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/djangorestframework/serializer.py b/djangorestframework/serializer.py index 429adea2c..4e1c6b80e 100644 --- a/djangorestframework/serializer.py +++ b/djangorestframework/serializer.py @@ -230,11 +230,10 @@ class Serializer(object): # serialize each required field for fname in fields: try: - if hasattr(self, smart_str(fname)): + if inspect.ismethod(getattr(self, fname, None)) and \ + len(inspect.getargspec(getattr(self, fname))[0]) == 2: # check first for a method 'fname' on self first - meth = getattr(self, fname) - if inspect.ismethod(meth) and len(inspect.getargspec(meth)[0]) == 2: - obj = meth(instance) + obj = meth(instance) elif hasattr(instance, '__contains__') and fname in instance: # check for a key 'fname' on the instance obj = instance[fname] From 792bc4d60814af026f926a4c92a9d4a8e3124341 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Tue, 10 Jan 2012 20:38:01 +0200 Subject: [PATCH 2/2] fixed issue#73 and added a test --- djangorestframework/serializer.py | 10 ++++++---- djangorestframework/tests/serializer.py | 15 ++++++++++++--- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/djangorestframework/serializer.py b/djangorestframework/serializer.py index 4e1c6b80e..43d32b293 100644 --- a/djangorestframework/serializer.py +++ b/djangorestframework/serializer.py @@ -230,12 +230,14 @@ class Serializer(object): # serialize each required field for fname in fields: try: - if inspect.ismethod(getattr(self, fname, None)) and \ - len(inspect.getargspec(getattr(self, fname))[0]) == 2: - # check first for a method 'fname' on self first + # we first check for a method 'fname' on self, + # 'fname's signature must be 'def fname(self, instance)' + meth = getattr(self, fname, None) + if (inspect.ismethod(meth) and + len(inspect.getargspec(meth)[0]) == 2): obj = meth(instance) elif hasattr(instance, '__contains__') and fname in instance: - # check for a key 'fname' on the instance + # then check for a key 'fname' on the instance obj = instance[fname] elif hasattr(instance, smart_str(fname)): # finally check for an attribute 'fname' on the instance diff --git a/djangorestframework/tests/serializer.py b/djangorestframework/tests/serializer.py index 8e370fa13..7cf9325b2 100644 --- a/djangorestframework/tests/serializer.py +++ b/djangorestframework/tests/serializer.py @@ -34,9 +34,7 @@ class TestObjectToData(TestCase): self.assertEquals(self.serialize(Foo().foo), 1) def test_datetime(self): - """ - datetime objects are left as-is. - """ + """datetime objects are left as-is.""" now = datetime.datetime.now() self.assertEquals(self.serialize(now), now) @@ -121,3 +119,14 @@ class TestFieldNesting(TestCase): self.assertEqual(SerializerM2().serialize(self.m2), {'field': {'field1': u'foo'}}) self.assertEqual(SerializerM3().serialize(self.m3), {'field': {'field2': u'bar'}}) + + def test_serializer_unvalid_hook_method(self): + """ + Test serializing a model instance with an unvalid hook method on the serializer. + """ + class SerializerM2(Serializer): + fields = ('unvalid_hook', ) + def unvalid_hook(self): + return + self.m2.unvalid_hook = 'bla' + self.assertEqual(SerializerM2().serialize_model(self.m2), {'unvalid_hook': 'bla'})