Tests for field choices

6 files changed, 149 insertions, 102 deletions

diff --git a/rest_framework/fields.py b/rest_framework/fields.py
index 48a3e1ab..f5bae734 100644
--- a/rest_framework/fields.py
+++ b/rest_framework/fields.py
@@ -102,6 +102,7 @@ class Field(object):
         'null': _('This field may not be null.')
     }
     default_validators = []
+    default_empty_html = None
 
     def __init__(self, read_only=False, write_only=False,
                  required=None, default=empty, initial=None, source=None,
@@ -185,6 +186,11 @@ class Field(object):
         Given the *incoming* primative data, return the value for this field
         that should be validated and transformed to a native value.
         """
+        if html.is_html_input(dictionary):
+            # HTML forms will represent empty fields as '', and cannot
+            # represent None or False values directly.
+            ret = dictionary.get(self.field_name, '')
+            return self.default_empty_html if (ret == '') else ret
         return dictionary.get(self.field_name, empty)
 
     def get_attribute(self, instance):
@@ -236,9 +242,6 @@ class Field(object):
         Test the given value against all the validators on the field,
         and either raise a `ValidationError` or simply return.
         """
-        if value in (None, '', [], (), {}):
-            return
-
         errors = []
         for validator in self.validators:
             try:
@@ -282,16 +285,10 @@ class BooleanField(Field):
     default_error_messages = {
         'invalid': _('`{input}` is not a valid boolean.')
     }
+    default_empty_html = False
     TRUE_VALUES = set(('t', 'T', 'true', 'True', 'TRUE', '1', 1, True))
     FALSE_VALUES = set(('f', 'F', 'false', 'False', 'FALSE', '0', 0, 0.0, False))
 
-    def get_value(self, dictionary):
-        if html.is_html_input(dictionary):
-            # HTML forms do not send a `False` value on an empty checkbox,
-            # so we override the default empty value to be False.
-            return dictionary.get(self.field_name, False)
-        return dictionary.get(self.field_name, empty)
-
     def to_internal_value(self, data):
         if data in self.TRUE_VALUES:
             return True
@@ -315,6 +312,7 @@ class CharField(Field):
     default_error_messages = {
         'blank': _('This field may not be blank.')
     }
+    default_empty_html = ''
 
     def __init__(self, **kwargs):
         self.allow_blank = kwargs.pop('allow_blank', False)
@@ -323,6 +321,9 @@ class CharField(Field):
         super(CharField, self).__init__(**kwargs)
 
     def run_validation(self, data=empty):
+        # Test for the empty string here so that it does not get validated,
+        # and so that subclasses do not need to handle it explicitly
+        # inside the `to_internal_value()` method.
         if data == '':
             if not self.allow_blank:
                 self.fail('blank')
diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py
index d9f9c8cb..949f5915 100644
--- a/rest_framework/serializers.py
+++ b/rest_framework/serializers.py
@@ -411,6 +411,9 @@ class ModelSerializer(Serializer):
                     # `ModelField`, which is used when no other typed field
                     # matched to the model field.
                     kwargs.pop('model_field', None)
+                if not issubclass(field_cls, CharField):
+                    # `allow_blank` is only valid for textual fields.
+                    kwargs.pop('allow_blank', None)
 
             elif field_name in info.relations:
                 # Create forward and reverse relationships.
diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py
index be72e444..1c718ccb 100644
--- a/rest_framework/utils/field_mapping.py
+++ b/rest_framework/utils/field_mapping.py
@@ -49,8 +49,9 @@ def get_field_kwargs(field_name, model_field):
     kwargs = {}
     validator_kwarg = model_field.validators
 
-    if model_field.null or model_field.blank:
-        kwargs['required'] = False
+    # The following will only be used by ModelField classes.
+    # Gets removed for everything else.
+    kwargs['model_field'] = model_field
 
     if model_field.verbose_name and needs_label(model_field, field_name):
         kwargs['label'] = capfirst(model_field.verbose_name)
@@ -59,23 +60,26 @@ def get_field_kwargs(field_name, model_field):
         kwargs['help_text'] = model_field.help_text
 
     if isinstance(model_field, models.AutoField) or not model_field.editable:
+        # If this field is read-only, then return early.
+        # Further keyword arguments are not valid.
         kwargs['read_only'] = True
-        # Read only implies that the field is not required.
-        # We have a cleaner repr on the instance if we don't set it.
-        kwargs.pop('required', None)
+        return kwargs
 
     if model_field.has_default():
-        kwargs['default'] = model_field.get_default()
-        # Having a default implies that the field is not required.
-        # We have a cleaner repr on the instance if we don't set it.
-        kwargs.pop('required', None)
+        kwargs['required'] = False
 
     if model_field.flatchoices:
-        # If this model field contains choices, then return now,
-        # any further keyword arguments are not valid.
+        # If this model field contains choices, then return early.
+        # Further keyword arguments are not valid.
         kwargs['choices'] = model_field.flatchoices
         return kwargs
 
+    if model_field.null:
+        kwargs['allow_null'] = True
+
+    if model_field.blank:
+        kwargs['allow_blank'] = True
+
     # Ensure that max_length is passed explicitly as a keyword arg,
     # rather than as a validator.
     max_length = getattr(model_field, 'max_length', None)
@@ -88,7 +92,10 @@ def get_field_kwargs(field_name, model_field):
 
     # Ensure that min_length is passed explicitly as a keyword arg,
     # rather than as a validator.
-    min_length = getattr(model_field, 'min_length', None)
+    min_length = next((
+        validator.limit_value for validator in validator_kwarg
+        if isinstance(validator, validators.MinLengthValidator)
+    ), None)
     if min_length is not None:
         kwargs['min_length'] = min_length
         validator_kwarg = [
@@ -153,20 +160,9 @@ def get_field_kwargs(field_name, model_field):
     if decimal_places is not None:
         kwargs['decimal_places'] = decimal_places
 
-    if isinstance(model_field, models.BooleanField):
-        # models.BooleanField has `blank=True`, but *is* actually
-        # required *unless* a default is provided.
-        # Also note that Django<1.6 uses `default=False` for
-        # models.BooleanField, but Django>=1.6 uses `default=None`.
-        kwargs.pop('required', None)
-
     if validator_kwarg:
         kwargs['validators'] = validator_kwarg
 
-    # The following will only be used by ModelField classes.
-    # Gets removed for everything else.
-    kwargs['model_field'] = model_field
-
     return kwargs
 
 
@@ -188,16 +184,22 @@ def get_relation_kwargs(field_name, relation_info):
         kwargs.pop('queryset', None)
 
     if model_field:
-        if model_field.null or model_field.blank:
-            kwargs['required'] = False
         if model_field.verbose_name and needs_label(model_field, field_name):
             kwargs['label'] = capfirst(model_field.verbose_name)
-        if not model_field.editable:
-            kwargs['read_only'] = True
-            kwargs.pop('queryset', None)
         help_text = clean_manytomany_helptext(model_field.help_text)
         if help_text:
             kwargs['help_text'] = help_text
+        if not model_field.editable:
+            kwargs['read_only'] = True
+            kwargs.pop('queryset', None)
+        if kwargs.get('read_only', False):
+            # If this field is read-only, then return early.
+            # No further keyword arguments are valid.
+            return kwargs
+        if model_field.has_default():
+            kwargs['required'] = False
+        if model_field.null:
+            kwargs['allow_null'] = True
 
     return kwargs
 
diff --git a/tests/test_field_options.py b/tests/test_field_options.py
deleted file mode 100644
index 444bd424..00000000
--- a/tests/test_field_options.py
+++ /dev/null
@@ -1,55 +0,0 @@
-from rest_framework import fields
-import pytest
-
-
-class TestFieldOptions:
-    def test_required(self):
-        """
-        By default a field must be included in the input.
-        """
-        field = fields.IntegerField()
-        with pytest.raises(fields.ValidationError) as exc_info:
-            field.run_validation()
-        assert exc_info.value.messages == ['This field is required.']
-
-    def test_not_required(self):
-        """
-        If `required=False` then a field may be omitted from the input.
-        """
-        field = fields.IntegerField(required=False)
-        with pytest.raises(fields.SkipField):
-            field.run_validation()
-
-    def test_disallow_null(self):
-        """
-        By default `None` is not a valid input.
-        """
-        field = fields.IntegerField()
-        with pytest.raises(fields.ValidationError) as exc_info:
-            field.run_validation(None)
-        assert exc_info.value.messages == ['This field may not be null.']
-
-    def test_allow_null(self):
-        """
-        If `allow_null=True` then `None` is a valid input.
-        """
-        field = fields.IntegerField(allow_null=True)
-        output = field.run_validation(None)
-        assert output is None
-
-    def test_disallow_blank(self):
-        """
-        By default '' is not a valid input.
-        """
-        field = fields.CharField()
-        with pytest.raises(fields.ValidationError) as exc_info:
-            field.run_validation('')
-        assert exc_info.value.messages == ['This field may not be blank.']
-
-    def test_allow_blank(self):
-        """
-        If `allow_blank=True` then '' is a valid input.
-        """
-        field = fields.CharField(allow_blank=True)
-        output = field.run_validation('')
-        assert output is ''
diff --git a/tests/test_field_values.py b/tests/test_fields.py
index bac50f0b..6bf9aed4 100644
--- a/tests/test_field_values.py
+++ b/tests/test_fields.py
@@ -6,6 +6,73 @@ import django
 import pytest
 
 
+# Tests for field keyword arguments and core functionality.
+# ---------------------------------------------------------
+
+class TestFieldOptions:
+    def test_required(self):
+        """
+        By default a field must be included in the input.
+        """
+        field = fields.IntegerField()
+        with pytest.raises(fields.ValidationError) as exc_info:
+            field.run_validation()
+        assert exc_info.value.messages == ['This field is required.']
+
+    def test_not_required(self):
+        """
+        If `required=False` then a field may be omitted from the input.
+        """
+        field = fields.IntegerField(required=False)
+        with pytest.raises(fields.SkipField):
+            field.run_validation()
+
+    def test_disallow_null(self):
+        """
+        By default `None` is not a valid input.
+        """
+        field = fields.IntegerField()
+        with pytest.raises(fields.ValidationError) as exc_info:
+            field.run_validation(None)
+        assert exc_info.value.messages == ['This field may not be null.']
+
+    def test_allow_null(self):
+        """
+        If `allow_null=True` then `None` is a valid input.
+        """
+        field = fields.IntegerField(allow_null=True)
+        output = field.run_validation(None)
+        assert output is None
+
+    def test_disallow_blank(self):
+        """
+        By default '' is not a valid input.
+        """
+        field = fields.CharField()
+        with pytest.raises(fields.ValidationError) as exc_info:
+            field.run_validation('')
+        assert exc_info.value.messages == ['This field may not be blank.']
+
+    def test_allow_blank(self):
+        """
+        If `allow_blank=True` then '' is a valid input.
+        """
+        field = fields.CharField(allow_blank=True)
+        output = field.run_validation('')
+        assert output is ''
+
+    def test_default(self):
+        """
+        If `default` is set, then omitted values get the default input.
+        """
+        field = fields.IntegerField(default=123)
+        output = field.run_validation()
+        assert output is 123
+
+
+# Tests for field input and output values.
+# ----------------------------------------
+
 def get_items(mapping_or_list_of_two_tuples):
     # Tests accept either lists of two tuples, or dictionaries.
     if isinstance(mapping_or_list_of_two_tuples, dict):
diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py
index d9f9efbe..731ed2fb 100644
--- a/tests/test_model_serializer.py
+++ b/tests/test_model_serializer.py
@@ -6,6 +6,7 @@ These tests deal with ensuring that we correctly map the model fields onto
 an appropriate set of serializer fields for each case.
 """
 from django.core.exceptions import ImproperlyConfigured
+from django.core.validators import MaxValueValidator, MinValueValidator, MinLengthValidator
 from django.db import models
 from django.test import TestCase
 from rest_framework import serializers
@@ -15,7 +16,8 @@ def dedent(blocktext):
     return '\n'.join([line[12:] for line in blocktext.splitlines()[1:-1]])
 
 
-# Testing regular field mappings
+# Tests for regular field mappings.
+# ---------------------------------
 
 class CustomField(models.Field):
     """
@@ -24,9 +26,6 @@ class CustomField(models.Field):
     pass
 
 
-COLOR_CHOICES = (('red', 'Red'), ('blue', 'Blue'), ('green', 'Green'))
-
-
 class RegularFieldsModel(models.Model):
     """
     A model class for testing regular flat fields.
@@ -35,7 +34,6 @@ class RegularFieldsModel(models.Model):
     big_integer_field = models.BigIntegerField()
     boolean_field = models.BooleanField(default=False)
     char_field = models.CharField(max_length=100)
-    choices_field = models.CharField(max_length=100, choices=COLOR_CHOICES)
     comma_seperated_integer_field = models.CommaSeparatedIntegerField(max_length=100)
     date_field = models.DateField()
     datetime_field = models.DateTimeField()
@@ -57,6 +55,19 @@ class RegularFieldsModel(models.Model):
         return 'method'
 
 
+COLOR_CHOICES = (('red', 'Red'), ('blue', 'Blue'), ('green', 'Green'))
+
+
+class FieldOptionsModel(models.Model):
+    value_limit_field = models.IntegerField(validators=[MinValueValidator(1), MaxValueValidator(10)])
+    length_limit_field = models.CharField(validators=[MinLengthValidator(3)], max_length=12)
+    blank_field = models.CharField(blank=True, max_length=10)
+    null_field = models.IntegerField(null=True)
+    default_field = models.IntegerField(default=0)
+    descriptive_field = models.IntegerField(help_text='Some help text', verbose_name='A label')
+    choices_field = models.CharField(max_length=100, choices=COLOR_CHOICES)
+
+
 class TestRegularFieldMappings(TestCase):
     def test_regular_fields(self):
         """
@@ -70,9 +81,8 @@ class TestRegularFieldMappings(TestCase):
             TestSerializer():
                 auto_field = IntegerField(read_only=True)
                 big_integer_field = IntegerField()
-                boolean_field = BooleanField(default=False)
+                boolean_field = BooleanField(required=False)
                 char_field = CharField(max_length=100)
-                choices_field = ChoiceField(choices=[('red', 'Red'), ('blue', 'Blue'), ('green', 'Green')])
                 comma_seperated_integer_field = CharField(max_length=100, validators=[<django.core.validators.RegexValidator object>])
                 date_field = DateField()
                 datetime_field = DateTimeField()
@@ -80,7 +90,7 @@ class TestRegularFieldMappings(TestCase):
                 email_field = EmailField(max_length=100)
                 float_field = FloatField()
                 integer_field = IntegerField()
-                null_boolean_field = BooleanField(required=False)
+                null_boolean_field = BooleanField(allow_null=True)
                 positive_integer_field = IntegerField()
                 positive_small_integer_field = IntegerField()
                 slug_field = SlugField(max_length=100)
@@ -92,6 +102,24 @@ class TestRegularFieldMappings(TestCase):
         """)
         self.assertEqual(repr(TestSerializer()), expected)
 
+    def test_field_options(self):
+        class TestSerializer(serializers.ModelSerializer):
+            class Meta:
+                model = FieldOptionsModel
+
+        expected = dedent("""
+            TestSerializer():
+                id = IntegerField(label='ID', read_only=True)
+                value_limit_field = IntegerField(max_value=10, min_value=1)
+                length_limit_field = CharField(max_length=12, min_length=3)
+                blank_field = CharField(allow_blank=True, max_length=10)
+                null_field = IntegerField(allow_null=True)
+                default_field = IntegerField(required=False)
+                descriptive_field = IntegerField(help_text='Some help text', label='A label')
+                choices_field = ChoiceField(choices=[('red', 'Red'), ('blue', 'Blue'), ('green', 'Green')])
+        """)
+        self.assertEqual(repr(TestSerializer()), expected)
+
     def test_method_field(self):
         """
         Properties and methods on the model should be allowed as `Meta.fields`
@@ -178,7 +206,8 @@ class TestRegularFieldMappings(TestCase):
         assert str(excinfo.exception) == expected
 
 
-# Testing relational field mappings
+# Tests for relational field mappings.
+# ------------------------------------
 
 class ForeignKeyTargetModel(models.Model):
     name = models.CharField(max_length=100)