From f1f7f5d4e3cd67730c6fb2233a5e4d6afaeae636 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 3 Oct 2012 11:50:08 +0100 Subject: [PATCH 1/9] Added failing test for m2m data --- rest_framework/renderers.py | 2 +- rest_framework/tests/serializer.py | 41 ++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py index 9484e29b9..9f0c26a58 100644 --- a/rest_framework/renderers.py +++ b/rest_framework/renderers.py @@ -262,7 +262,7 @@ class DocumentingHTMLRenderer(BaseRenderer): try: fields[k] = field_mapping[v.__class__]() except KeyError: - fields[k] = forms.CharField + fields[k] = forms.CharField() OnTheFlyForm = type("OnTheFlyForm", (forms.Form,), fields) if obj and not view.request.method == 'DELETE': # Don't fill in the form when the object is deleted diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index f7412a326..f90dce166 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -160,6 +160,47 @@ class ManyToManyTests(TestCase): self.assertEquals(instance.pk, 1) self.assertEquals(list(instance.rel.all()), [self.anchor, new_anchor]) + def test_create_empty_relationship(self): + """ + Create an instance of a model with a ManyToMany relationship, + containing no items. + """ + data = {'rel': []} + serializer = self.serializer_class(data) + self.assertEquals(serializer.is_valid(), True) + instance = serializer.save() + self.assertEquals(len(ManyToManyModel.objects.all()), 2) + self.assertEquals(instance.pk, 2) + self.assertEquals(list(instance.rel.all()), []) + + def test_update_empty_relationship(self): + """ + Update an instance of a model with a ManyToMany relationship, + containing no items. + """ + new_anchor = Anchor() + new_anchor.save() + data = {'rel': []} + serializer = self.serializer_class(data, instance=self.instance) + self.assertEquals(serializer.is_valid(), True) + instance = serializer.save() + self.assertEquals(len(ManyToManyModel.objects.all()), 1) + self.assertEquals(instance.pk, 1) + self.assertEquals(list(instance.rel.all()), []) + + def test_create_empty_relationship_flat_data(self): + """ + Create an instance of a model with a ManyToMany relationship, + containing no items, using a representation that does not support + lists (eg form data). + """ + data = {'rel': ''} + serializer = self.serializer_class(data) + self.assertEquals(serializer.is_valid(), True) + instance = serializer.save() + self.assertEquals(len(ManyToManyModel.objects.all()), 2) + self.assertEquals(instance.pk, 2) + self.assertEquals(list(instance.rel.all()), []) # def test_deserialization_for_update(self): # serializer = self.serializer_class(self.data, instance=self.instance) # expected = self.instance From cab3b2f3f8f82087cf162dd7c62f18e9d8bb208a Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 3 Oct 2012 12:07:34 +0100 Subject: [PATCH 2/9] Split out PrimaryKeyRelatedField and ManyPrimaryKeyRelatedField --- rest_framework/fields.py | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 85ee5430d..7cb2950cd 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -181,7 +181,6 @@ class RelatedField(Field): Subclass this and override `convert` to define custom behaviour when serializing related objects. """ - def field_to_native(self, obj, field_name): obj = getattr(obj, self.source or field_name) if obj.__class__.__name__ in ('RelatedManager', 'ManyRelatedManager'): @@ -202,7 +201,6 @@ class PrimaryKeyRelatedField(RelatedField): """ Serializes a model related field or related manager to a pk value. """ - # Note the we use ModelRelatedField's implementation, as we want to get the # raw database value directly, since that won't involve another # database lookup. @@ -225,23 +223,34 @@ class PrimaryKeyRelatedField(RelatedField): try: obj = obj.serializable_value(self.source or field_name) except AttributeError: - field = obj._meta.get_field_by_name(field_name)[0] + # RelatedObject (reverse relationship) obj = getattr(obj, self.source or field_name) - if obj.__class__.__name__ == 'RelatedManager': - return [self.to_native(item.pk) for item in obj.all()] - elif isinstance(field, RelatedObject): - return self.to_native(obj.pk) - raise - if obj.__class__.__name__ == 'ManyRelatedManager': - return [self.to_native(item.pk) for item in obj.all()] + return self.to_native(obj.pk) + # Forward relationship return self.to_native(obj) def field_from_native(self, data, field_name, into): value = data.get(field_name) - if hasattr(value, '__iter__'): - into[field_name] = [self.from_native(item) for item in value] - else: - into[field_name + '_id'] = self.from_native(value) + into[field_name + '_id'] = self.from_native(value) + + +class ManyPrimaryKeyRelatedField(PrimaryKeyRelatedField): + def field_to_native(self, obj, field_name): + try: + obj = obj.serializable_value(self.source or field_name) + except AttributeError: + # RelatedManager (reverse relationship) + obj = getattr(obj, self.source or field_name) + return [self.to_native(item.pk) for item in obj.all()] + # Forward relationship + return [self.to_native(item.pk) for item in obj.all()] + + def field_from_native(self, data, field_name, into): + try: + value = data.getlist(field_name) + except: + value = data.get(field_name) + into[field_name] = [self.from_native(item) for item in value] class NaturalKeyRelatedField(RelatedField): From 58c1263267e5947f8243568edb33273effdc2787 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 3 Oct 2012 12:16:30 +0100 Subject: [PATCH 3/9] Use either PrimaryKeyRelatedField or ManyPrimaryKeyRelatedField as appropriate (fixes test) --- rest_framework/fields.py | 12 ++++++------ rest_framework/serializers.py | 2 ++ rest_framework/tests/serializer.py | 1 + 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 7cb2950cd..b51d70a83 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -221,13 +221,13 @@ class PrimaryKeyRelatedField(RelatedField): def field_to_native(self, obj, field_name): try: - obj = obj.serializable_value(self.source or field_name) + pk = obj.serializable_value(self.source or field_name) except AttributeError: # RelatedObject (reverse relationship) obj = getattr(obj, self.source or field_name) return self.to_native(obj.pk) # Forward relationship - return self.to_native(obj) + return self.to_native(pk) def field_from_native(self, data, field_name, into): value = data.get(field_name) @@ -237,13 +237,13 @@ class PrimaryKeyRelatedField(RelatedField): class ManyPrimaryKeyRelatedField(PrimaryKeyRelatedField): def field_to_native(self, obj, field_name): try: - obj = obj.serializable_value(self.source or field_name) + queryset = obj.serializable_value(self.source or field_name) except AttributeError: # RelatedManager (reverse relationship) - obj = getattr(obj, self.source or field_name) - return [self.to_native(item.pk) for item in obj.all()] + queryset = getattr(obj, self.source or field_name) + return [self.to_native(item.pk) for item in queryset.all()] # Forward relationship - return [self.to_native(item.pk) for item in obj.all()] + return [self.to_native(item.pk) for item in queryset.all()] def field_from_native(self, data, field_name, into): try: diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 683b9efc8..d3ae9b8a6 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -351,6 +351,8 @@ class ModelSerializer(RelatedField, Serializer): """ Creates a default instance of a flat relational field. """ + if isinstance(model_field, models.fields.related.ManyToManyField): + return ManyPrimaryKeyRelatedField() return PrimaryKeyRelatedField() def get_field(self, model_field): diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index f90dce166..db342c9e0 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -201,6 +201,7 @@ class ManyToManyTests(TestCase): self.assertEquals(len(ManyToManyModel.objects.all()), 2) self.assertEquals(instance.pk, 2) self.assertEquals(list(instance.rel.all()), []) + # def test_deserialization_for_update(self): # serializer = self.serializer_class(self.data, instance=self.instance) # expected = self.instance From f4ad77ac04ca9563fbb402f6f212a12861e137d3 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 3 Oct 2012 12:55:53 +0100 Subject: [PATCH 4/9] Fix for empty form fields --- rest_framework/fields.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index b51d70a83..5dc055fc0 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -250,7 +250,7 @@ class ManyPrimaryKeyRelatedField(PrimaryKeyRelatedField): value = data.getlist(field_name) except: value = data.get(field_name) - into[field_name] = [self.from_native(item) for item in value] + into[field_name] = [self.from_native(item) for item in value if item] class NaturalKeyRelatedField(RelatedField): From 27c93c08d2a9b791f9a5aef5f372194cb963d63c Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 3 Oct 2012 12:56:01 +0100 Subject: [PATCH 5/9] Fix for empty form fields --- rest_framework/fields.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 5dc055fc0..8b495d6ee 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -250,7 +250,10 @@ class ManyPrimaryKeyRelatedField(PrimaryKeyRelatedField): value = data.getlist(field_name) except: value = data.get(field_name) - into[field_name] = [self.from_native(item) for item in value if item] + else: + if value == ['']: + value = [] + into[field_name] = [self.from_native(item) for item in value] class NaturalKeyRelatedField(RelatedField): From 09f22676013bd39d4c783956e719c2d1f87da927 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 3 Oct 2012 15:39:03 +0100 Subject: [PATCH 6/9] Improve comments --- rest_framework/fields.py | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 8b495d6ee..5b406703b 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -7,7 +7,6 @@ from django.core import validators from django.core.exceptions import ValidationError from django.conf import settings from django.db import DEFAULT_DB_ALIAS -from django.db.models.related import RelatedObject from django.utils.encoding import is_protected_type, smart_unicode from django.utils.translation import ugettext_lazy as _ from rest_framework.compat import parse_date, parse_datetime @@ -199,27 +198,23 @@ class RelatedField(Field): class PrimaryKeyRelatedField(RelatedField): """ - Serializes a model related field or related manager to a pk value. + Serializes a related field or related object to a pk value. """ - # Note the we use ModelRelatedField's implementation, as we want to get the - # raw database value directly, since that won't involve another - # database lookup. - # - # An alternative implementation would simply be this... - # - # class PrimaryKeyRelatedField(RelatedField): - # def to_native(self, obj): - # return obj.pk def to_native(self, pk): """ - Simply returns the object's pk. You can subclass this method to - provide different serialization behavior of the pk. - (For example returning a URL based on the model's pk.) + You can subclass this method to provide different serialization + behavior based on the pk. """ return pk def field_to_native(self, obj, field_name): + # This is only implemented for performance reasons + # + # We could leave the default `RelatedField.field_to_native()` in place, + # and inside just implement `to_native()` as `return obj.pk` + # + # That would involve an extra database lookup. try: pk = obj.serializable_value(self.source or field_name) except AttributeError: @@ -235,6 +230,10 @@ class PrimaryKeyRelatedField(RelatedField): class ManyPrimaryKeyRelatedField(PrimaryKeyRelatedField): + """ + Serializes a to-many related field or related manager to a pk value. + """ + def field_to_native(self, obj, field_name): try: queryset = obj.serializable_value(self.source or field_name) From a366d6e61261b9050c85a76d26ccf1544f165486 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 3 Oct 2012 16:08:20 +0100 Subject: [PATCH 7/9] M2M fields supported --- rest_framework/fields.py | 4 ++++ rest_framework/renderers.py | 12 ++++++++++-- rest_framework/serializers.py | 7 ++++--- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 5b406703b..edc77e1aa 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -180,6 +180,10 @@ class RelatedField(Field): Subclass this and override `convert` to define custom behaviour when serializing related objects. """ + def __init__(self, *args, **kwargs): + self.queryset = kwargs.pop('queryset', None) + super(RelatedField, self).__init__(*args, **kwargs) + def field_to_native(self, obj, field_name): obj = getattr(obj, self.source or field_name) if obj.__class__.__name__ in ('RelatedManager', 'ManyRelatedManager'): diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py index 9f0c26a58..96c5e170b 100644 --- a/rest_framework/renderers.py +++ b/rest_framework/renderers.py @@ -246,7 +246,9 @@ class DocumentingHTMLRenderer(BaseRenderer): serializers.DateField: forms.DateField, serializers.EmailField: forms.EmailField, serializers.CharField: forms.CharField, - serializers.BooleanField: forms.BooleanField + serializers.BooleanField: forms.BooleanField, + serializers.PrimaryKeyRelatedField: forms.ChoiceField, + serializers.ManyPrimaryKeyRelatedField: forms.ModelMultipleChoiceField } # Creating an on the fly form see: http://stackoverflow.com/questions/3915024/dynamically-creating-classes-python @@ -257,10 +259,16 @@ class DocumentingHTMLRenderer(BaseRenderer): serializer = view.get_serializer(instance=obj) for k, v in serializer.get_fields(True).items(): + print k, v if v.readonly: continue + + kwargs = {} + if getattr(v, 'queryset', None): + kwargs['queryset'] = getattr(v, 'queryset', None) + try: - fields[k] = field_mapping[v.__class__]() + fields[k] = field_mapping[v.__class__](**kwargs) except KeyError: fields[k] = forms.CharField() diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index d3ae9b8a6..037638249 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -351,9 +351,10 @@ class ModelSerializer(RelatedField, Serializer): """ Creates a default instance of a flat relational field. """ + queryset = model_field.rel.to._default_manager # .using(db).complex_filter(self.rel.limit_choices_to) if isinstance(model_field, models.fields.related.ManyToManyField): - return ManyPrimaryKeyRelatedField() - return PrimaryKeyRelatedField() + return ManyPrimaryKeyRelatedField(queryset=queryset) + return PrimaryKeyRelatedField(queryset=queryset) def get_field(self, model_field): """ @@ -367,7 +368,7 @@ class ModelSerializer(RelatedField, Serializer): models.EmailField: EmailField, models.CharField: CharField, models.CommaSeparatedIntegerField: CharField, - models.BooleanField: BooleanField + models.BooleanField: BooleanField, } try: return field_mapping[model_field.__class__]() From 92b091ea16e4d75a6641b1164855230dea0dae75 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 3 Oct 2012 16:19:07 +0100 Subject: [PATCH 8/9] Tweak styling of browseable API --- rest_framework/templates/rest_framework/base.html | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/rest_framework/templates/rest_framework/base.html b/rest_framework/templates/rest_framework/base.html index 867051e6b..bc6c2e4ac 100644 --- a/rest_framework/templates/rest_framework/base.html +++ b/rest_framework/templates/rest_framework/base.html @@ -122,6 +122,7 @@ {% if response.status_code != 403 %} {% if post_form %} +

POST: {{ name }}

@@ -131,7 +132,7 @@
{{ field.label_tag|add_class:"control-label" }}
- {{ field }} + {{ field|add_class:"input-xxlarge" }} {{ field.help_text }} {{ field.errors|add_class:"help-block" }}
@@ -142,9 +143,11 @@
+
{% endif %} {% if put_form %} +

PUT: {{ name }}

@@ -155,7 +158,7 @@
{{ field.label_tag|add_class:"control-label" }}
- {{ field }} + {{ field|add_class:"input-xxlarge" }} {{ field.help_text }} {{ field.errors|add_class:"help-block" }}
@@ -167,6 +170,7 @@
+
{% endif %} {% endif %} From a02707e12f750fd0d325e528f7b0fbcd7079db73 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 3 Oct 2012 21:08:32 +0100 Subject: [PATCH 9/9] Tweaks --- rest_framework/renderers.py | 2 +- rest_framework/templates/rest_framework/base.html | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py index 96c5e170b..5bc5d5f8e 100644 --- a/rest_framework/renderers.py +++ b/rest_framework/renderers.py @@ -247,7 +247,7 @@ class DocumentingHTMLRenderer(BaseRenderer): serializers.EmailField: forms.EmailField, serializers.CharField: forms.CharField, serializers.BooleanField: forms.BooleanField, - serializers.PrimaryKeyRelatedField: forms.ChoiceField, + serializers.PrimaryKeyRelatedField: forms.ModelChoiceField, serializers.ManyPrimaryKeyRelatedField: forms.ModelMultipleChoiceField } diff --git a/rest_framework/templates/rest_framework/base.html b/rest_framework/templates/rest_framework/base.html index bc6c2e4ac..a5e089429 100644 --- a/rest_framework/templates/rest_framework/base.html +++ b/rest_framework/templates/rest_framework/base.html @@ -132,7 +132,7 @@
{{ field.label_tag|add_class:"control-label" }}
- {{ field|add_class:"input-xxlarge" }} + {{ field|add_class:"input-xlarge" }} {{ field.help_text }} {{ field.errors|add_class:"help-block" }}
@@ -158,7 +158,7 @@
{{ field.label_tag|add_class:"control-label" }}
- {{ field|add_class:"input-xxlarge" }} + {{ field|add_class:"input-xlarge" }} {{ field.help_text }} {{ field.errors|add_class:"help-block" }}