diff options
| -rw-r--r-- | rest_framework/fields.py | 21 | ||||
| -rw-r--r-- | rest_framework/serializers.py | 3 | ||||
| -rw-r--r-- | rest_framework/utils/field_mapping.py | 58 | ||||
| -rw-r--r-- | tests/test_field_options.py | 55 | ||||
| -rw-r--r-- | tests/test_fields.py (renamed from tests/test_field_values.py) | 67 | ||||
| -rw-r--r-- | tests/test_model_serializer.py | 47 |
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) |