From 72c04d570d167209f3f34d6d78492426f206b245 Mon Sep 17 00:00:00 2001 From: Mark Aaron Shirley Date: Fri, 4 Jan 2013 12:50:01 +0100 Subject: [PATCH 1/7] Add nested create for 1to1 reverse relationships --- rest_framework/serializers.py | 46 ++++++++++++++---- rest_framework/tests/nesting.py | 85 +++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 8 deletions(-) create mode 100644 rest_framework/tests/nesting.py diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 27458f968..a43a81d78 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -93,7 +93,7 @@ class SerializerOptions(object): self.exclude = getattr(meta, 'exclude', ()) -class BaseSerializer(Field): +class BaseSerializer(WritableField): class Meta(object): pass @@ -218,7 +218,10 @@ class BaseSerializer(Field): try: field.field_from_native(data, files, field_name, reverted_data) except ValidationError as err: - self._errors[field_name] = list(err.messages) + if hasattr(err, 'message_dict'): + self._errors[field_name] = [err.message_dict] + else: + self._errors[field_name] = list(err.messages) return reverted_data @@ -369,6 +372,25 @@ class ModelSerializer(Serializer): """ _options_class = ModelSerializerOptions + def field_from_native(self, data, files, field_name, into): + if self.read_only: + return + + try: + native = data[field_name] + except KeyError: + if self.required: + raise ValidationError(self.error_messages['required']) + return + + obj = self.from_native(native, files) + if not self._errors: + self.object = obj + into[self.source or field_name] = self + else: + # Propagate errors up to our parent + raise ValidationError(self._errors) + def get_default_fields(self): """ Return all the fields that should be serialized for the model. @@ -542,10 +564,9 @@ class ModelSerializer(Serializer): return instance - def save(self): - """ - Save the deserialized object and return it. - """ + def _save(self, parent=None, fk_field=None): + if parent and fk_field: + setattr(self.object, fk_field, parent) self.object.save() if getattr(self, 'm2m_data', None): @@ -555,9 +576,18 @@ class ModelSerializer(Serializer): if getattr(self, 'related_data', None): for accessor_name, object_list in self.related_data.items(): - setattr(self.object, accessor_name, object_list) + if isinstance(object_list, ModelSerializer): + fk_field = self.object._meta.get_field_by_name(accessor_name)[0].field.name + object_list._save(parent=self.object, fk_field=fk_field) + else: + setattr(self.object, accessor_name, object_list) self.related_data = {} - + + def save(self): + """ + Save the deserialized object and return it. + """ + self._save() return self.object diff --git a/rest_framework/tests/nesting.py b/rest_framework/tests/nesting.py new file mode 100644 index 000000000..0c130dcee --- /dev/null +++ b/rest_framework/tests/nesting.py @@ -0,0 +1,85 @@ +from django.db import models +from django.test import TestCase +from rest_framework import serializers + + +class OneToOneTarget(models.Model): + name = models.CharField(max_length=100) + + +class OneToOneTargetSource(models.Model): + name = models.CharField(max_length=100) + target = models.OneToOneField(OneToOneTarget, related_name='target_source') + + +class OneToOneSource(models.Model): + name = models.CharField(max_length=100) + target_source = models.OneToOneField(OneToOneTargetSource, related_name='source') + + +class OneToOneSourceSerializer(serializers.ModelSerializer): + class Meta: + model = OneToOneSource + exclude = ('target_source', ) + + +class OneToOneTargetSourceSerializer(serializers.ModelSerializer): + source = OneToOneSourceSerializer() + + class Meta: + model = OneToOneTargetSource + exclude = ('target', ) + +class OneToOneTargetSerializer(serializers.ModelSerializer): + target_source = OneToOneTargetSourceSerializer() + + class Meta: + model = OneToOneTarget + + +class NestedOneToOneTests(TestCase): + def setUp(self): + #import pdb ; pdb.set_trace() + for idx in range(1, 4): + target = OneToOneTarget(name='target-%d' % idx) + target.save() + target_source = OneToOneTargetSource(name='target-source-%d' % idx, target=target) + target_source.save() + source = OneToOneSource(name='source-%d' % idx, target_source=target_source) + source.save() + + def test_foreign_key_retrieve(self): + queryset = OneToOneTarget.objects.all() + serializer = OneToOneTargetSerializer(queryset) + expected = [ + {'id': 1, 'name': u'target-1', 'target_source': {'id': 1, 'name': u'target-source-1', 'source': {'id': 1, 'name': u'source-1'}}}, + {'id': 2, 'name': u'target-2', 'target_source': {'id': 2, 'name': u'target-source-2', 'source': {'id': 2, 'name': u'source-2'}}}, + {'id': 3, 'name': u'target-3', 'target_source': {'id': 3, 'name': u'target-source-3', 'source': {'id': 3, 'name': u'source-3'}}} + ] + self.assertEquals(serializer.data, expected) + + + def test_foreign_key_create(self): + data = {'id': 4, 'name': u'target-4', 'target_source': {'id': 4, 'name': u'target-source-4', 'source': {'id': 4, 'name': u'source-4'}}} + serializer = OneToOneTargetSerializer(data=data) + self.assertTrue(serializer.is_valid()) + obj = serializer.save() + self.assertEquals(serializer.data, data) + self.assertEqual(obj.name, u'target-4') + + # Ensure (source 4, target 4) is added, and everything else is as expected + queryset = OneToOneTarget.objects.all() + serializer = OneToOneTargetSerializer(queryset) + expected = [ + {'id': 1, 'name': u'target-1', 'target_source': {'id': 1, 'name': u'target-source-1', 'source': {'id': 1, 'name': u'source-1'}}}, + {'id': 2, 'name': u'target-2', 'target_source': {'id': 2, 'name': u'target-source-2', 'source': {'id': 2, 'name': u'source-2'}}}, + {'id': 3, 'name': u'target-3', 'target_source': {'id': 3, 'name': u'target-source-3', 'source': {'id': 3, 'name': u'source-3'}}}, + {'id': 4, 'name': u'target-4', 'target_source': {'id': 4, 'name': u'target-source-4', 'source': {'id': 4, 'name': u'source-4'}}} + ] + self.assertEquals(serializer.data, expected) + + def test_foreign_key_create_with_invalid_data(self): + data = {'id': 4, 'name': u'target-4', 'target_source': {'id': 4, 'name': u'target-source-4', 'source': {'id': 4}}} + serializer = OneToOneTargetSerializer(data=data) + self.assertFalse(serializer.is_valid()) + self.assertEquals(serializer.errors, {'target_source': [{'source': [{'name': [u'This field is required.']}]}]}) From e66eeb4af8611ba255274f561afb674b25a93c8a Mon Sep 17 00:00:00 2001 From: Mark Aaron Shirley Date: Fri, 4 Jan 2013 13:13:03 +0100 Subject: [PATCH 2/7] Remove commented out debug code --- rest_framework/tests/nesting.py | 1 - 1 file changed, 1 deletion(-) diff --git a/rest_framework/tests/nesting.py b/rest_framework/tests/nesting.py index 0c130dcee..d6f9237f1 100644 --- a/rest_framework/tests/nesting.py +++ b/rest_framework/tests/nesting.py @@ -39,7 +39,6 @@ class OneToOneTargetSerializer(serializers.ModelSerializer): class NestedOneToOneTests(TestCase): def setUp(self): - #import pdb ; pdb.set_trace() for idx in range(1, 4): target = OneToOneTarget(name='target-%d' % idx) target.save() From 46eea97380ab9723d747b41fab0a305dec19c738 Mon Sep 17 00:00:00 2001 From: Mark Aaron Shirley Date: Fri, 4 Jan 2013 13:48:01 +0100 Subject: [PATCH 3/7] Update one-to-one test names --- rest_framework/tests/nesting.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rest_framework/tests/nesting.py b/rest_framework/tests/nesting.py index d6f9237f1..9cc46c6c8 100644 --- a/rest_framework/tests/nesting.py +++ b/rest_framework/tests/nesting.py @@ -47,7 +47,7 @@ class NestedOneToOneTests(TestCase): source = OneToOneSource(name='source-%d' % idx, target_source=target_source) source.save() - def test_foreign_key_retrieve(self): + def test_one_to_one_retrieve(self): queryset = OneToOneTarget.objects.all() serializer = OneToOneTargetSerializer(queryset) expected = [ @@ -58,7 +58,7 @@ class NestedOneToOneTests(TestCase): self.assertEquals(serializer.data, expected) - def test_foreign_key_create(self): + def test_one_to_one_create(self): data = {'id': 4, 'name': u'target-4', 'target_source': {'id': 4, 'name': u'target-source-4', 'source': {'id': 4, 'name': u'source-4'}}} serializer = OneToOneTargetSerializer(data=data) self.assertTrue(serializer.is_valid()) @@ -77,7 +77,7 @@ class NestedOneToOneTests(TestCase): ] self.assertEquals(serializer.data, expected) - def test_foreign_key_create_with_invalid_data(self): + def test_one_to_one_create_with_invalid_data(self): data = {'id': 4, 'name': u'target-4', 'target_source': {'id': 4, 'name': u'target-source-4', 'source': {'id': 4}}} serializer = OneToOneTargetSerializer(data=data) self.assertFalse(serializer.is_valid()) From 8e5003a1f6e61664e99a376ef8c200f53c4507e1 Mon Sep 17 00:00:00 2001 From: Mark Aaron Shirley Date: Fri, 4 Jan 2013 13:54:51 +0100 Subject: [PATCH 4/7] Update errant test comment --- rest_framework/tests/nesting.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rest_framework/tests/nesting.py b/rest_framework/tests/nesting.py index 9cc46c6c8..dbc8ebc95 100644 --- a/rest_framework/tests/nesting.py +++ b/rest_framework/tests/nesting.py @@ -66,7 +66,8 @@ class NestedOneToOneTests(TestCase): self.assertEquals(serializer.data, data) self.assertEqual(obj.name, u'target-4') - # Ensure (source 4, target 4) is added, and everything else is as expected + # Ensure (target 4, target_source 4, source 4) are added, and + # everything else is as expected. queryset = OneToOneTarget.objects.all() serializer = OneToOneTargetSerializer(queryset) expected = [ From 2d62bcd5aaa6d8f25f22b3e6b89ce26c44d9dfc4 Mon Sep 17 00:00:00 2001 From: Mark Aaron Shirley Date: Sat, 5 Jan 2013 00:02:48 +0100 Subject: [PATCH 5/7] Add one-to-one nested update and delete functionality --- rest_framework/serializers.py | 14 +++++++++++ rest_framework/tests/nesting.py | 41 ++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index a43a81d78..42218e7df 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -107,6 +107,7 @@ class BaseSerializer(WritableField): self.parent = None self.root = None self.partial = partial + self.delete = False self.context = context or {} @@ -215,6 +216,15 @@ class BaseSerializer(WritableField): for field_name, field in self.fields.items(): field.initialize(parent=self, field_name=field_name) + if isinstance(field, ModelSerializer) and self.object: + # Set the serializer object if it exists + pk_field_name = field.opts.model._meta.pk.name + obj = getattr(self.object, field_name) + nested_data = data.get(field_name) + pk_val = nested_data.get(pk_field_name) if nested_data else None + if obj and (getattr(obj, pk_field_name) == pk_val): + field.object = obj + field.delete = nested_data.get('_delete') try: field.field_from_native(data, files, field_name, reverted_data) except ValidationError as err: @@ -565,6 +575,10 @@ class ModelSerializer(Serializer): return instance def _save(self, parent=None, fk_field=None): + if self.delete: + self.object.delete() + return + if parent and fk_field: setattr(self.object, fk_field, parent) self.object.save() diff --git a/rest_framework/tests/nesting.py b/rest_framework/tests/nesting.py index dbc8ebc95..10d5db996 100644 --- a/rest_framework/tests/nesting.py +++ b/rest_framework/tests/nesting.py @@ -9,7 +9,8 @@ class OneToOneTarget(models.Model): class OneToOneTargetSource(models.Model): name = models.CharField(max_length=100) - target = models.OneToOneField(OneToOneTarget, related_name='target_source') + target = models.OneToOneField(OneToOneTarget, null=True, blank=True, + related_name='target_source') class OneToOneSource(models.Model): @@ -83,3 +84,41 @@ class NestedOneToOneTests(TestCase): serializer = OneToOneTargetSerializer(data=data) self.assertFalse(serializer.is_valid()) self.assertEquals(serializer.errors, {'target_source': [{'source': [{'name': [u'This field is required.']}]}]}) + + def test_one_to_one_update(self): + data = {'id': 3, 'name': u'target-3-updated', 'target_source': {'id': 3, 'name': u'target-source-3-updated', 'source': {'id': 3, 'name': u'source-3-updated'}}} + instance = OneToOneTarget.objects.get(pk=3) + serializer = OneToOneTargetSerializer(instance, data=data) + self.assertTrue(serializer.is_valid()) + obj = serializer.save() + self.assertEquals(serializer.data, data) + self.assertEqual(obj.name, u'target-3-updated') + + # Ensure (target 3, target_source 3, source 3) are updated, + # and everything else is as expected. + queryset = OneToOneTarget.objects.all() + serializer = OneToOneTargetSerializer(queryset) + expected = [ + {'id': 1, 'name': u'target-1', 'target_source': {'id': 1, 'name': u'target-source-1', 'source': {'id': 1, 'name': u'source-1'}}}, + {'id': 2, 'name': u'target-2', 'target_source': {'id': 2, 'name': u'target-source-2', 'source': {'id': 2, 'name': u'source-2'}}}, + {'id': 3, 'name': u'target-3-updated', 'target_source': {'id': 3, 'name': u'target-source-3-updated', 'source': {'id': 3, 'name': u'source-3-updated'}}} + ] + self.assertEquals(serializer.data, expected) + + def test_one_to_one_delete(self): + data = {'id': 3, 'name': u'target-3', 'target_source': {'_delete': True, 'id': 3, 'name': u'target-source-3', 'source': {'id': 3, 'name': u'source-3'}}} + instance = OneToOneTarget.objects.get(pk=3) + serializer = OneToOneTargetSerializer(instance, data=data) + self.assertTrue(serializer.is_valid()) + obj = serializer.save() + + # Ensure (target_source 3, source 3) are deleted, + # and everything else is as expected. + queryset = OneToOneTarget.objects.all() + serializer = OneToOneTargetSerializer(queryset) + expected = [ + {'id': 1, 'name': u'target-1', 'target_source': {'id': 1, 'name': u'target-source-1', 'source': {'id': 1, 'name': u'source-1'}}}, + {'id': 2, 'name': u'target-2', 'target_source': {'id': 2, 'name': u'target-source-2', 'source': {'id': 2, 'name': u'source-2'}}}, + {'id': 3, 'name': u'target-3', 'target_source': None} + ] + self.assertEquals(serializer.data, expected) From 34e14b01e402a2b2bcaf57aab76397757e260fd6 Mon Sep 17 00:00:00 2001 From: Mark Aaron Shirley Date: Tue, 8 Jan 2013 09:38:15 -0800 Subject: [PATCH 6/7] Move nested serializer logic into .field_from_native() --- rest_framework/serializers.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 42218e7df..83bf1bc36 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -216,15 +216,6 @@ class BaseSerializer(WritableField): for field_name, field in self.fields.items(): field.initialize(parent=self, field_name=field_name) - if isinstance(field, ModelSerializer) and self.object: - # Set the serializer object if it exists - pk_field_name = field.opts.model._meta.pk.name - obj = getattr(self.object, field_name) - nested_data = data.get(field_name) - pk_val = nested_data.get(pk_field_name) if nested_data else None - if obj and (getattr(obj, pk_field_name) == pk_val): - field.object = obj - field.delete = nested_data.get('_delete') try: field.field_from_native(data, files, field_name, reverted_data) except ValidationError as err: @@ -393,6 +384,15 @@ class ModelSerializer(Serializer): raise ValidationError(self.error_messages['required']) return + if self.parent.object: + # Set the serializer object if it exists + pk_field_name = self.opts.model._meta.pk.name + pk_val = native.get(pk_field_name) + obj = getattr(self.parent.object, field_name) + if obj and (getattr(obj, pk_field_name) == pk_val): + self.object = obj + self.delete = native.get('_delete') + obj = self.from_native(native, files) if not self._errors: self.object = obj From 221f7326c7db7b6fa1a9ba2f0181ac075e3b482c Mon Sep 17 00:00:00 2001 From: Mark Aaron Shirley Date: Wed, 16 Jan 2013 16:03:59 -0800 Subject: [PATCH 7/7] Use None to delete nested object as opposed to _delete flag --- rest_framework/serializers.py | 27 ++++++++++++++------------- rest_framework/tests/nesting.py | 2 +- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 83bf1bc36..a84370e93 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -107,7 +107,6 @@ class BaseSerializer(WritableField): self.parent = None self.root = None self.partial = partial - self.delete = False self.context = context or {} @@ -119,6 +118,7 @@ class BaseSerializer(WritableField): self._data = None self._files = None self._errors = None + self._delete = False ##### # Methods to determine which fields to use when (de)serializing objects. @@ -378,7 +378,7 @@ class ModelSerializer(Serializer): return try: - native = data[field_name] + value = data[field_name] except KeyError: if self.required: raise ValidationError(self.error_messages['required']) @@ -387,19 +387,20 @@ class ModelSerializer(Serializer): if self.parent.object: # Set the serializer object if it exists pk_field_name = self.opts.model._meta.pk.name - pk_val = native.get(pk_field_name) obj = getattr(self.parent.object, field_name) - if obj and (getattr(obj, pk_field_name) == pk_val): - self.object = obj - self.delete = native.get('_delete') - - obj = self.from_native(native, files) - if not self._errors: self.object = obj - into[self.source or field_name] = self + + if value in (None, ''): + self._delete = True + into[(self.source or field_name)] = self else: - # Propagate errors up to our parent - raise ValidationError(self._errors) + obj = self.from_native(value, files) + if not self._errors: + self.object = obj + into[self.source or field_name] = self + else: + # Propagate errors up to our parent + raise ValidationError(self._errors) def get_default_fields(self): """ @@ -575,7 +576,7 @@ class ModelSerializer(Serializer): return instance def _save(self, parent=None, fk_field=None): - if self.delete: + if self._delete: self.object.delete() return diff --git a/rest_framework/tests/nesting.py b/rest_framework/tests/nesting.py index 10d5db996..e4e326676 100644 --- a/rest_framework/tests/nesting.py +++ b/rest_framework/tests/nesting.py @@ -106,7 +106,7 @@ class NestedOneToOneTests(TestCase): self.assertEquals(serializer.data, expected) def test_one_to_one_delete(self): - data = {'id': 3, 'name': u'target-3', 'target_source': {'_delete': True, 'id': 3, 'name': u'target-source-3', 'source': {'id': 3, 'name': u'source-3'}}} + data = {'id': 3, 'name': u'target-3', 'target_source': None} instance = OneToOneTarget.objects.get(pk=3) serializer = OneToOneTargetSerializer(instance, data=data) self.assertTrue(serializer.is_valid())