From d413ce5d1a87b6989be4fd186234d9cd6aed2a3d Mon Sep 17 00:00:00 2001 From: Anton Martyniuk Date: Sun, 29 Jun 2014 15:51:12 +0200 Subject: [PATCH 1/7] fixes fields behaviour when null value is assigned --- rest_framework/fields.py | 10 ++++++---- rest_framework/relations.py | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 6caae9242..8e30e77a3 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -429,6 +429,11 @@ class ModelField(WritableField): "type": self.model_field.get_internal_type() } + def validate(self, value): + super(ModelField, self).validate(value) + if value is None and not self.model_field.null: + raise ValidationError(self.error_messages['invalid']) + ##### Typed Fields ##### @@ -474,10 +479,7 @@ class CharField(WritableField): self.validators.append(validators.MaxLengthValidator(max_length)) def from_native(self, value): - if isinstance(value, six.string_types): - return value - - if value is None: + if value in validators.EMPTY_VALUES: return '' return smart_text(value) diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 3463954dc..df17a8606 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -33,7 +33,7 @@ class RelatedField(WritableField): many_widget = widgets.SelectMultiple form_field_class = forms.ChoiceField many_form_field_class = forms.MultipleChoiceField - null_values = (None, '', 'None') + null_values = (None, '', 'None', [], (), {}) cache_choices = False empty_label = None @@ -182,7 +182,7 @@ class RelatedField(WritableField): if value in self.null_values: if self.required: raise ValidationError(self.error_messages['required']) - into[(self.source or field_name)] = None + into[(self.source or field_name)] = [] if self.many else None elif self.many: into[(self.source or field_name)] = [self.from_native(item) for item in value] else: From 2f73e3f704e1dde13762d35260c80b49a873de88 Mon Sep 17 00:00:00 2001 From: Anton Martyniuk Date: Sun, 29 Jun 2014 16:30:58 +0200 Subject: [PATCH 2/7] updates test model definition due to RelationField changes --- rest_framework/tests/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/tests/models.py b/rest_framework/tests/models.py index fba3f8f7c..664fe8d1a 100644 --- a/rest_framework/tests/models.py +++ b/rest_framework/tests/models.py @@ -52,7 +52,7 @@ class CallableDefaultValueModel(RESTFrameworkModel): class ManyToManyModel(RESTFrameworkModel): - rel = models.ManyToManyField(Anchor, help_text='Some help text.') + rel = models.ManyToManyField(Anchor, help_text='Some help text.', blank=True) class ReadOnlyManyToManyModel(RESTFrameworkModel): From 9cb4e4e00ea6d821acf3c3a1743137127ad56952 Mon Sep 17 00:00:00 2001 From: Anton Martyniuk Date: Sun, 29 Jun 2014 16:31:25 +0200 Subject: [PATCH 3/7] adds test for missing but required many-to-many relationship value --- rest_framework/tests/test_relations.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/rest_framework/tests/test_relations.py b/rest_framework/tests/test_relations.py index 37ac826b2..a2d494529 100644 --- a/rest_framework/tests/test_relations.py +++ b/rest_framework/tests/test_relations.py @@ -3,6 +3,7 @@ General tests for relational fields. """ from __future__ import unicode_literals from django import get_version +from django.core.exceptions import ValidationError from django.db import models from django.test import TestCase from django.utils import unittest @@ -43,12 +44,28 @@ class TestManyRelatedMixin(TestCase): https://github.com/tomchristie/django-rest-framework/pull/632 ''' - field = serializers.RelatedField(many=True, read_only=False) + field = serializers.RelatedField(many=True, read_only=False, required=False) into = {} field.field_from_native({}, None, 'field_name', into) self.assertEqual(into['field_name'], []) + def test_missing_required_many_to_many_related_field(self): + ''' + Missing but required many-to-many relationship should cause + ValidationError + ''' + field = serializers.RelatedField(many=True, read_only=False) + + into = {} + self.assertRaisesMessage( + ValidationError, + "This field is required.", + field.field_from_native, + {}, None, 'field_name', into + ) + self.assertDictEqual(into, {}) + # Regression tests for #694 (`source` attribute on related fields) From 0ff2acdd51a0aa93a1c2642b84dcb066f19584f6 Mon Sep 17 00:00:00 2001 From: Anton Martyniuk Date: Sun, 29 Jun 2014 17:09:21 +0200 Subject: [PATCH 4/7] should fix 1.3.7 test --- rest_framework/tests/test_relations.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rest_framework/tests/test_relations.py b/rest_framework/tests/test_relations.py index a2d494529..8b753f5ca 100644 --- a/rest_framework/tests/test_relations.py +++ b/rest_framework/tests/test_relations.py @@ -58,9 +58,8 @@ class TestManyRelatedMixin(TestCase): field = serializers.RelatedField(many=True, read_only=False) into = {} - self.assertRaisesMessage( + self.assertRaises( ValidationError, - "This field is required.", field.field_from_native, {}, None, 'field_name', into ) From 5d148207bbe474f81446ca41c6f88b12220722a3 Mon Sep 17 00:00:00 2001 From: Anton Martyniuk Date: Sun, 29 Jun 2014 17:33:17 +0200 Subject: [PATCH 5/7] None as empty value should be enough for m2m field --- rest_framework/relations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/relations.py b/rest_framework/relations.py index df17a8606..8f35b1741 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -182,7 +182,7 @@ class RelatedField(WritableField): if value in self.null_values: if self.required: raise ValidationError(self.error_messages['required']) - into[(self.source or field_name)] = [] if self.many else None + into[(self.source or field_name)] = None elif self.many: into[(self.source or field_name)] = [self.from_native(item) for item in value] else: From 8522f906bbccd00ad6f43f1886eabbb841a972e7 Mon Sep 17 00:00:00 2001 From: Anton Martyniuk Date: Sun, 29 Jun 2014 16:30:58 +0200 Subject: [PATCH 6/7] updates test model definition due to RelationField changes --- rest_framework/serializers.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index c2b414d7a..25c45d923 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -18,6 +18,7 @@ import types from decimal import Decimal from django.contrib.contenttypes.generic import GenericForeignKey from django.core.paginator import Page +from django.core.exceptions import ObjectDoesNotExist from django.db import models from django.forms import widgets from django.utils.datastructures import SortedDict @@ -912,9 +913,19 @@ class ModelSerializer(Serializer): for field_name, field in self.fields.items(): field_name = field.source or field_name + + # PY3 compat problem with ManyToManyField & OneToOneField + # hasattr in PY2 catches all exceptions, but in PY3 it only looks + # for AttributeError + # @see: https://code.djangoproject.com/ticket/22839 + try: + field_exists = hasattr(instance, field_name) + except (AttributeError, ValueError, ObjectDoesNotExist): + field_exists = False + if field_name in exclusions \ and not field.read_only \ - and (field.required or hasattr(instance, field_name)) \ + and (field.required or field_exists) \ and not isinstance(field, Serializer): exclusions.remove(field_name) return exclusions From 5edf19e91982abc4c5298ef4da9f7e0e171025f6 Mon Sep 17 00:00:00 2001 From: Anton Martyniuk Date: Sun, 29 Jun 2014 16:30:58 +0200 Subject: [PATCH 7/7] updates test model definition due to RelationField changes --- rest_framework/relations.py | 2 +- rest_framework/serializers.py | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 8f35b1741..df17a8606 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -182,7 +182,7 @@ class RelatedField(WritableField): if value in self.null_values: if self.required: raise ValidationError(self.error_messages['required']) - into[(self.source or field_name)] = None + into[(self.source or field_name)] = [] if self.many else None elif self.many: into[(self.source or field_name)] = [self.from_native(item) for item in value] else: diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index c2b414d7a..25c45d923 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -18,6 +18,7 @@ import types from decimal import Decimal from django.contrib.contenttypes.generic import GenericForeignKey from django.core.paginator import Page +from django.core.exceptions import ObjectDoesNotExist from django.db import models from django.forms import widgets from django.utils.datastructures import SortedDict @@ -912,9 +913,19 @@ class ModelSerializer(Serializer): for field_name, field in self.fields.items(): field_name = field.source or field_name + + # PY3 compat problem with ManyToManyField & OneToOneField + # hasattr in PY2 catches all exceptions, but in PY3 it only looks + # for AttributeError + # @see: https://code.djangoproject.com/ticket/22839 + try: + field_exists = hasattr(instance, field_name) + except (AttributeError, ValueError, ObjectDoesNotExist): + field_exists = False + if field_name in exclusions \ and not field.read_only \ - and (field.required or hasattr(instance, field_name)) \ + and (field.required or field_exists) \ and not isinstance(field, Serializer): exclusions.remove(field_name) return exclusions