From 46f5c62530744017f744cdcfec91774a0566c179 Mon Sep 17 00:00:00 2001 From: Yuri Prezument Date: Sun, 5 Jan 2014 15:16:55 +0200 Subject: [PATCH 1/6] Regression test for #1330 (Coerce None to '') --- rest_framework/tests/test_serializer.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/rest_framework/tests/test_serializer.py b/rest_framework/tests/test_serializer.py index 86f365dee..8c2c09cf8 100644 --- a/rest_framework/tests/test_serializer.py +++ b/rest_framework/tests/test_serializer.py @@ -1124,6 +1124,20 @@ class BlankFieldTests(TestCase): serializer = self.model_serializer_class(data={}) self.assertEqual(serializer.is_valid(), True) + def test_create_model_null_field_save(self): + """ + Regression test for #1330. + + https://github.com/tomchristie/django-rest-framework/pull/1330 + """ + serializer = self.model_serializer_class(data={'title': None}) + self.assertEqual(serializer.is_valid(), True) + + try: + serializer.save() + except Exception: + self.fail('Exception raised on save() after validation passes') + #test for issue #460 class SerializerPickleTests(TestCase): From e88e3c6ae163029f0fe564dd214235ab350dbfc9 Mon Sep 17 00:00:00 2001 From: Yuri Prezument Date: Sun, 5 Jan 2014 15:25:16 +0200 Subject: [PATCH 2/6] Possible fix for #1330 Coerce None to '' in CharField.to_native() --- rest_framework/fields.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 5ee752351..22f0120b7 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -452,7 +452,9 @@ class CharField(WritableField): self.validators.append(validators.MaxLengthValidator(max_length)) def from_native(self, value): - if isinstance(value, six.string_types) or value is None: + if value is None: + return '' + if isinstance(value, six.string_types): return value return smart_text(value) From 6e622d644c9b55b905e24497f0fb818d557fd970 Mon Sep 17 00:00:00 2001 From: Yuri Prezument Date: Sun, 5 Jan 2014 15:58:46 +0200 Subject: [PATCH 3/6] CharField - add allow_null argument --- docs/api-guide/fields.md | 7 ++++--- rest_framework/fields.py | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/api-guide/fields.md b/docs/api-guide/fields.md index e05c03061..83825350e 100644 --- a/docs/api-guide/fields.md +++ b/docs/api-guide/fields.md @@ -157,23 +157,24 @@ Corresponds to `django.db.models.fields.BooleanField`. ## CharField A text representation, optionally validates the text to be shorter than `max_length` and longer than `min_length`. +If `allow_none` is `False` (default), `None` values will be converted to an empty string. Corresponds to `django.db.models.fields.CharField` or `django.db.models.fields.TextField`. -**Signature:** `CharField(max_length=None, min_length=None)` +**Signature:** `CharField(max_length=None, min_length=None, allow_none=False)` ## URLField Corresponds to `django.db.models.fields.URLField`. Uses Django's `django.core.validators.URLValidator` for validation. -**Signature:** `CharField(max_length=200, min_length=None)` +**Signature:** `CharField(max_length=200, min_length=None, allow_none=False)` ## SlugField Corresponds to `django.db.models.fields.SlugField`. -**Signature:** `CharField(max_length=50, min_length=None)` +**Signature:** `CharField(max_length=50, min_length=None, allow_none=False)` ## ChoiceField diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 22f0120b7..16485b414 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -443,8 +443,9 @@ class CharField(WritableField): type_label = 'string' form_field_class = forms.CharField - def __init__(self, max_length=None, min_length=None, *args, **kwargs): + def __init__(self, max_length=None, min_length=None, allow_none=False, *args, **kwargs): self.max_length, self.min_length = max_length, min_length + self.allow_none = allow_none super(CharField, self).__init__(*args, **kwargs) if min_length is not None: self.validators.append(validators.MinLengthValidator(min_length)) @@ -452,7 +453,7 @@ class CharField(WritableField): self.validators.append(validators.MaxLengthValidator(max_length)) def from_native(self, value): - if value is None: + if value is None and not self.allow_none: return '' if isinstance(value, six.string_types): return value From e1bbe9d514c95aba596cff64292eb0f0bc7d99fa Mon Sep 17 00:00:00 2001 From: Yuri Prezument Date: Mon, 6 Jan 2014 13:56:57 +0200 Subject: [PATCH 4/6] Set `allow_none = True` for CharFields with null=True --- rest_framework/serializers.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index fa9353061..0164965cd 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -821,10 +821,15 @@ class ModelSerializer(Serializer): kwargs.update({attribute: getattr(model_field, attribute)}) try: - return self.field_mapping[model_field.__class__](**kwargs) + field_class = self.field_mapping[model_field.__class__] except KeyError: return ModelField(model_field=model_field, **kwargs) + if issubclass(field_class, CharField) and model_field.null: + kwargs['allow_none'] = True + + return field_class(**kwargs) + def get_validation_exclusions(self): """ Return a list of field names to exclude from model validation. From 0fd0454a5c1ddcf8676e23b30dfaee40fa7cb0c8 Mon Sep 17 00:00:00 2001 From: Yuri Prezument Date: Mon, 6 Jan 2014 14:02:00 +0200 Subject: [PATCH 5/6] Test for setting allow_none=True for nullable CharFields --- rest_framework/tests/test_serializer.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rest_framework/tests/test_serializer.py b/rest_framework/tests/test_serializer.py index 8c2c09cf8..6d9b85ee0 100644 --- a/rest_framework/tests/test_serializer.py +++ b/rest_framework/tests/test_serializer.py @@ -1504,6 +1504,7 @@ class AttributeMappingOnAutogeneratedFieldsTests(TestCase): image_field = models.ImageField(max_length=1024, blank=True) slug_field = models.SlugField(max_length=1024, blank=True) url_field = models.URLField(max_length=1024, blank=True) + nullable_char_field = models.CharField(max_length=1024, blank=True, null=True) class AMOAFSerializer(serializers.ModelSerializer): class Meta: @@ -1536,6 +1537,10 @@ class AttributeMappingOnAutogeneratedFieldsTests(TestCase): 'url_field': [ ('max_length', 1024), ], + 'nullable_char_field': [ + ('max_length', 1024), + ('allow_none', True), + ], } def field_test(self, field): @@ -1572,6 +1577,9 @@ class AttributeMappingOnAutogeneratedFieldsTests(TestCase): def test_url_field(self): self.field_test('url_field') + def test_nullable_char_field(self): + self.field_test('nullable_char_field') + class DefaultValuesOnAutogeneratedFieldsTests(TestCase): From cd9a4194ea4f4dc0e43a34485cd8a27eba44a39a Mon Sep 17 00:00:00 2001 From: Yuri Prezument Date: Sun, 12 Jan 2014 16:30:26 +0200 Subject: [PATCH 6/6] Check the modelfield's class instead --- rest_framework/serializers.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 0164965cd..cbf73fc30 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -804,6 +804,10 @@ class ModelSerializer(Serializer): issubclass(model_field.__class__, models.PositiveSmallIntegerField): kwargs['min_value'] = 0 + if model_field.null and \ + issubclass(model_field.__class__, (models.CharField, models.TextField)): + kwargs['allow_none'] = True + attribute_dict = { models.CharField: ['max_length'], models.CommaSeparatedIntegerField: ['max_length'], @@ -821,15 +825,10 @@ class ModelSerializer(Serializer): kwargs.update({attribute: getattr(model_field, attribute)}) try: - field_class = self.field_mapping[model_field.__class__] + return self.field_mapping[model_field.__class__](**kwargs) except KeyError: return ModelField(model_field=model_field, **kwargs) - if issubclass(field_class, CharField) and model_field.null: - kwargs['allow_none'] = True - - return field_class(**kwargs) - def get_validation_exclusions(self): """ Return a list of field names to exclude from model validation.