From d54082c4a7c1468078fb006f6ff0e4a2f5abe4e4 Mon Sep 17 00:00:00 2001 From: Asif Saifuddin Auvi Date: Sun, 5 Jun 2016 12:12:38 +0600 Subject: [PATCH 01/10] upgraded django to 1.9.7 release --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index ba500a2a0..0fc41a881 100644 --- a/tox.ini +++ b/tox.ini @@ -15,7 +15,7 @@ setenv = PYTHONWARNINGS=once deps = django18: Django==1.8.13 - django19: Django==1.9.6 + django19: Django==1.9.7 django110: Django==1.10a1 -rrequirements/requirements-testing.txt -rrequirements/requirements-optionals.txt From 2712d4e5fee3d7a573f4162a1163e2a06b33bf96 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Mon, 6 Jun 2016 11:03:56 +0100 Subject: [PATCH 02/10] Note on obtain_auth_token and throttles/permissions. Closes #4128. [ci skip] (#4173) --- docs/api-guide/authentication.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/api-guide/authentication.md b/docs/api-guide/authentication.md index 81f0e12d5..3f981c033 100644 --- a/docs/api-guide/authentication.md +++ b/docs/api-guide/authentication.md @@ -207,6 +207,10 @@ The `obtain_auth_token` view will return a JSON response when valid `username` a Note that the default `obtain_auth_token` view explicitly uses JSON requests and responses, rather than using default renderer and parser classes in your settings. If you need a customized version of the `obtain_auth_token` view, you can do so by overriding the `ObtainAuthToken` view class, and using that in your url conf instead. +By default there are no permissions or throttling applied to the `obtain_auth_token` view. If you do wish to apply throttling you'll need to override the view class, +and include them using the `throttle_classes` attribute. + + ##### With Django admin It is also possible to create Tokens manually through admin interface. In case you are using a large user base, we recommend that you monkey patch the `TokenAdmin` class to customize it to your needs, more specifically by declaring the `user` field as `raw_field`. From b1035b2a87528f7e325fdf356e15ce5456ff2324 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Mon, 6 Jun 2016 12:03:37 +0100 Subject: [PATCH 03/10] Minor docs tweaks. [ci skip] (#4174) --- docs/tutorial/quickstart.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/tutorial/quickstart.md b/docs/tutorial/quickstart.md index 5e3b522cc..0c9ddf8f2 100644 --- a/docs/tutorial/quickstart.md +++ b/docs/tutorial/quickstart.md @@ -130,7 +130,7 @@ Okay, we're done. We're now ready to test the API we've built. Let's fire up the server from the command line. - python ./manage.py runserver + python manage.py runserver We can now access our API, both from the command-line, using tools like `curl`... @@ -182,7 +182,7 @@ Or using the [httpie][httpie], command line tool... } -Or directly through the browser... +Or directly through the browser, by going to the URL `http://127.0.0.1:8000/users/`... ![Quick start image][image] From 04e5b5b20ac93186ee43f04a442d374c7bfcf57a Mon Sep 17 00:00:00 2001 From: Asif Saifuddin Auvi Date: Tue, 7 Jun 2016 17:13:35 +0600 Subject: [PATCH 04/10] removed AUTH_USER_MODEL compat property (#4176) Removed unnecessary `AUTH_USER_MODEL` compat variable. (No longer required) --- rest_framework/authtoken/models.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/rest_framework/authtoken/models.py b/rest_framework/authtoken/models.py index 50db18103..8df84105d 100644 --- a/rest_framework/authtoken/models.py +++ b/rest_framework/authtoken/models.py @@ -6,12 +6,6 @@ from django.db import models from django.utils.encoding import python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ -# Prior to Django 1.5, the AUTH_USER_MODEL setting does not exist. -# Note that we don't perform this code in the compat module due to -# bug report #1297 -# See: https://github.com/tomchristie/django-rest-framework/issues/1297 -AUTH_USER_MODEL = getattr(settings, 'AUTH_USER_MODEL', 'auth.User') - @python_2_unicode_compatible class Token(models.Model): @@ -19,8 +13,10 @@ class Token(models.Model): The default authorization token model. """ key = models.CharField(_("Key"), max_length=40, primary_key=True) - user = models.OneToOneField(AUTH_USER_MODEL, related_name='auth_token', - on_delete=models.CASCADE, verbose_name=_("User")) + user = models.OneToOneField( + settings.AUTH_USER_MODEL, related_name='auth_token', + on_delete=models.CASCADE, verbose_name=_("User") + ) created = models.DateTimeField(_("Created"), auto_now_add=True) class Meta: From a5f822d0671ce2412df666afa2b920039b302a0e Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 8 Jun 2016 15:55:09 +0100 Subject: [PATCH 05/10] Empty cases of .validated_data and .errors as lists not dicts for ListSerializer (#4180) --- rest_framework/serializers.py | 22 ++++++++++++++++++++++ tests/test_serializer_bulk_update.py | 2 ++ 2 files changed, 24 insertions(+) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 698730f53..3ec55724d 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -667,6 +667,28 @@ class ListSerializer(BaseSerializer): return self.instance + def is_valid(self, raise_exception=False): + # This implementation is the same as the default, + # except that we use lists, rather than dicts, as the empty case. + assert hasattr(self, 'initial_data'), ( + 'Cannot call `.is_valid()` as no `data=` keyword argument was ' + 'passed when instantiating the serializer instance.' + ) + + if not hasattr(self, '_validated_data'): + try: + self._validated_data = self.run_validation(self.initial_data) + except ValidationError as exc: + self._validated_data = [] + self._errors = exc.detail + else: + self._errors = [] + + if self._errors and raise_exception: + raise ValidationError(self.errors) + + return not bool(self._errors) + def __repr__(self): return unicode_to_repr(representation.list_repr(self, indent=1)) diff --git a/tests/test_serializer_bulk_update.py b/tests/test_serializer_bulk_update.py index 8d7240a7b..567a32507 100644 --- a/tests/test_serializer_bulk_update.py +++ b/tests/test_serializer_bulk_update.py @@ -46,6 +46,7 @@ class BulkCreateSerializerTests(TestCase): serializer = self.BookSerializer(data=data, many=True) self.assertEqual(serializer.is_valid(), True) self.assertEqual(serializer.validated_data, data) + self.assertEqual(serializer.errors, []) def test_bulk_create_errors(self): """ @@ -76,6 +77,7 @@ class BulkCreateSerializerTests(TestCase): serializer = self.BookSerializer(data=data, many=True) self.assertEqual(serializer.is_valid(), False) self.assertEqual(serializer.errors, expected_errors) + self.assertEqual(serializer.validated_data, []) def test_invalid_list_datatype(self): """ From bb22ab8ee7c5d484dde7112dbe5551bcfc89d7d3 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 8 Jun 2016 17:13:20 +0100 Subject: [PATCH 06/10] More robust form rendering in the browsable API (#4181) --- rest_framework/renderers.py | 50 +++++++++++--------- tests/browsable_api/test_form_rendering.py | 53 ++++++++++++++++++++++ 2 files changed, 81 insertions(+), 22 deletions(-) create mode 100644 tests/browsable_api/test_form_rendering.py diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py index 264f7ac3b..7ca680e74 100644 --- a/rest_framework/renderers.py +++ b/rest_framework/renderers.py @@ -472,31 +472,37 @@ class BrowsableAPIRenderer(BaseRenderer): return if existing_serializer is not None: - serializer = existing_serializer - else: - if has_serializer: - if method in ('PUT', 'PATCH'): - serializer = view.get_serializer(instance=instance, **kwargs) - else: - serializer = view.get_serializer(**kwargs) + try: + return self.render_form_for_serializer(existing_serializer) + except TypeError: + pass + + if has_serializer: + if method in ('PUT', 'PATCH'): + serializer = view.get_serializer(instance=instance, **kwargs) else: - # at this point we must have a serializer_class - if method in ('PUT', 'PATCH'): - serializer = self._get_serializer(view.serializer_class, view, - request, instance=instance, **kwargs) - else: - serializer = self._get_serializer(view.serializer_class, view, - request, **kwargs) + serializer = view.get_serializer(**kwargs) + else: + # at this point we must have a serializer_class + if method in ('PUT', 'PATCH'): + serializer = self._get_serializer(view.serializer_class, view, + request, instance=instance, **kwargs) + else: + serializer = self._get_serializer(view.serializer_class, view, + request, **kwargs) - if hasattr(serializer, 'initial_data'): - serializer.is_valid() + return self.render_form_for_serializer(serializer) - form_renderer = self.form_renderer_class() - return form_renderer.render( - serializer.data, - self.accepted_media_type, - {'style': {'template_pack': 'rest_framework/horizontal'}} - ) + def render_form_for_serializer(self, serializer): + if hasattr(serializer, 'initial_data'): + serializer.is_valid() + + form_renderer = self.form_renderer_class() + return form_renderer.render( + serializer.data, + self.accepted_media_type, + {'style': {'template_pack': 'rest_framework/horizontal'}} + ) def get_raw_data_form(self, data, view, method, request): """ diff --git a/tests/browsable_api/test_form_rendering.py b/tests/browsable_api/test_form_rendering.py new file mode 100644 index 000000000..5a31ae0dd --- /dev/null +++ b/tests/browsable_api/test_form_rendering.py @@ -0,0 +1,53 @@ +from django.test import TestCase + +from rest_framework import generics, renderers, serializers, status +from rest_framework.response import Response +from rest_framework.test import APIRequestFactory +from tests.models import BasicModel + +factory = APIRequestFactory() + + +class BasicSerializer(serializers.ModelSerializer): + class Meta: + model = BasicModel + + +class ManyPostView(generics.GenericAPIView): + queryset = BasicModel.objects.all() + serializer_class = BasicSerializer + renderer_classes = (renderers.BrowsableAPIRenderer, renderers.JSONRenderer) + + def post(self, request, *args, **kwargs): + serializer = self.get_serializer(self.get_queryset(), many=True) + return Response(serializer.data, status.HTTP_200_OK) + + +class TestManyPostView(TestCase): + def setUp(self): + """ + Create 3 BasicModel instances. + """ + items = ['foo', 'bar', 'baz'] + for item in items: + BasicModel(text=item).save() + self.objects = BasicModel.objects + self.data = [ + {'id': obj.id, 'text': obj.text} + for obj in self.objects.all() + ] + self.view = ManyPostView.as_view() + + def test_post_many_post_view(self): + """ + POST request to a view that returns a list of objects should + still successfully return the browsable API with a rendered form. + + Regression test for https://github.com/tomchristie/django-rest-framework/pull/3164 + """ + data = {} + request = factory.post('/', data, format='json') + with self.assertNumQueries(1): + response = self.view(request).render() + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response.data), 3) From 9bffd354327ffc99a0c50ad140a86ede94f9dfba Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Mon, 13 Jun 2016 10:41:50 +0100 Subject: [PATCH 07/10] Handle bytestrings in JSON. Closes #4185. (#4191) --- rest_framework/utils/encoders.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rest_framework/utils/encoders.py b/rest_framework/utils/encoders.py index 949c99eda..f883b4925 100644 --- a/rest_framework/utils/encoders.py +++ b/rest_framework/utils/encoders.py @@ -51,6 +51,9 @@ class JSONEncoder(json.JSONEncoder): return six.text_type(obj) elif isinstance(obj, QuerySet): return tuple(obj) + elif isinstance(obj, six.binary_type): + # Best-effort for binary blobs. See #4187. + return obj.decode('utf-8') elif hasattr(obj, 'tolist'): # Numpy arrays and array scalars. return obj.tolist() From c3b7fba918910553661095fed5d31e947a0fe2d6 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Mon, 13 Jun 2016 13:31:12 +0100 Subject: [PATCH 08/10] Exclude read_only=True fields from unique_together validation & add docs. (#4192) * Exclude read_only=True fields from unique_together validation * Test to ensure that unique_together validators can be removed * Do not add uniquness_extra_kwargs when validators are explicitly declared. * Add docs on validation in complex cases --- docs/api-guide/validators.md | 67 +++++++++++++++++++++++++++++++++- rest_framework/serializers.py | 11 +++++- tests/test_model_serializer.py | 2 - tests/test_validators.py | 39 ++++++++++++++++++++ 4 files changed, 115 insertions(+), 4 deletions(-) diff --git a/docs/api-guide/validators.md b/docs/api-guide/validators.md index e54ebfc38..edd099056 100644 --- a/docs/api-guide/validators.md +++ b/docs/api-guide/validators.md @@ -156,7 +156,7 @@ If you want the date field to be entirely hidden from the user, then use `Hidden --- -# Advanced 'default' argument usage +# Advanced field defaults Validators that are applied across multiple fields in the serializer can sometimes require a field input that should not be provided by the API client, but that *is* available as input to the validator. @@ -188,6 +188,71 @@ It takes a single argument, which is the default value or callable that should b --- +# Limitations of validators + +There are some ambiguous cases where you'll need to instead handle validation +explicitly, rather than relying on the default serializer classes that +`ModelSerializer` generates. + +In these cases you may want to disable the automatically generated validators, +by specifying an empty list for the serializer `Meta.validators` attribute. + +## Optional fields + +By default "unique together" validation enforces that all fields be +`required=True`. In some cases, you might want to explicit apply +`required=False` to one of the fields, in which case the desired behaviour +of the validation is ambiguous. + +In this case you will typically need to exclude the validator from the +serializer class, and instead write any validation logic explicitly, either +in the `.validate()` method, or else in the view. + +For example: + + class BillingRecordSerializer(serializers.ModelSerializer): + def validate(self, data): + # Apply custom validation either here, or in the view. + + class Meta: + fields = ('client', 'date', 'amount') + extra_kwargs = {'client' {'required': 'False'}} + validators = [] # Remove a default "unique together" constraint. + +## Updating nested serializers + +When applying an update to an existing instance, uniqueness validators will +exclude the current instance from the uniqueness check. The current instance +is available in the context of the uniqueness check, because it exists as +an attribute on the serializer, having initially been passed using +`instance=...` when instantiating the serializer. + +In the case of update operations on *nested* serializers there's no way of +applying this exclusion, because the instance is not available. + +Again, you'll probably want to explicitly remove the validator from the +serializer class, and write the code the for the validation constraint +explicitly, in a `.validate()` method, or in the view. + +## Debugging complex cases + +If you're not sure exactly what behavior a `ModelSerializer` class will +generate it is usually a good idea to run `manage.py shell`, and print +an instance of the serializer, so that you can inspect the fields and +validators that it automatically generates for you. + + >>> serializer = MyComplexModelSerializer() + >>> print(serializer) + class MyComplexModelSerializer: + my_fields = ... + +Also keep in mind that with complex cases it can often be better to explicitly +define your serializer classes, rather than relying on the default +`ModelSerializer` behavior. This involves a little more code, but ensures +that the resulting behavior is more transparent. + +--- + # Writing custom validators You can use any of Django's existing validators, or write your own custom validators. diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 3ec55724d..3fcc85c3b 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -1243,6 +1243,11 @@ class ModelSerializer(Serializer): read_only_fields = getattr(self.Meta, 'read_only_fields', None) if read_only_fields is not None: + if not isinstance(read_only_fields, (list, tuple)): + raise TypeError( + 'The `read_only_fields` option must be a list or tuple. ' + 'Got %s.' % type(read_only_fields).__name__ + ) for field_name in read_only_fields: kwargs = extra_kwargs.get(field_name, {}) kwargs['read_only'] = True @@ -1258,6 +1263,9 @@ class ModelSerializer(Serializer): ('dict of updated extra kwargs', 'mapping of hidden fields') """ + if getattr(self.Meta, 'validators', None) is not None: + return (extra_kwargs, {}) + model = getattr(self.Meta, 'model') model_fields = self._get_model_fields( field_names, declared_fields, extra_kwargs @@ -1308,7 +1316,7 @@ class ModelSerializer(Serializer): else: uniqueness_extra_kwargs[unique_constraint_name] = {'default': default} elif default is not empty: - # The corresponding field is not present in the, + # The corresponding field is not present in the # serializer. We have a default to use for it, so # add in a hidden field that populates it. hidden_fields[unique_constraint_name] = HiddenField(default=default) @@ -1390,6 +1398,7 @@ class ModelSerializer(Serializer): field_names = { field.source for field in self.fields.values() if (field.source != '*') and ('.' not in field.source) + and not field.read_only } # Note that we make sure to check `unique_together` both on the diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index c6f7472aa..096cbc8d6 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -521,8 +521,6 @@ class TestRelationalFieldMappings(TestCase): one_to_one = NestedSerializer(read_only=True): url = HyperlinkedIdentityField(view_name='onetoonetargetmodel-detail') name = CharField(max_length=100) - class Meta: - validators = [] """) if six.PY2: # This case is also too awkward to resolve fully across both py2 diff --git a/tests/test_validators.py b/tests/test_validators.py index 9b388951e..5858ad374 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -239,6 +239,45 @@ class TestUniquenessTogetherValidation(TestCase): """) assert repr(serializer) == expected + def test_ignore_read_only_fields(self): + """ + When serializer fields are read only, then uniqueness + validators should not be added for that field. + """ + class ReadOnlyFieldSerializer(serializers.ModelSerializer): + class Meta: + model = UniquenessTogetherModel + fields = ('id', 'race_name', 'position') + read_only_fields = ('race_name',) + + serializer = ReadOnlyFieldSerializer() + expected = dedent(""" + ReadOnlyFieldSerializer(): + id = IntegerField(label='ID', read_only=True) + race_name = CharField(read_only=True) + position = IntegerField(required=True) + """) + assert repr(serializer) == expected + + def test_allow_explict_override(self): + """ + Ensure validators can be explicitly removed.. + """ + class NoValidatorsSerializer(serializers.ModelSerializer): + class Meta: + model = UniquenessTogetherModel + fields = ('id', 'race_name', 'position') + validators = [] + + serializer = NoValidatorsSerializer() + expected = dedent(""" + NoValidatorsSerializer(): + id = IntegerField(label='ID', read_only=True) + race_name = CharField(max_length=100) + position = IntegerField() + """) + assert repr(serializer) == expected + def test_ignore_validation_for_null_fields(self): # None values that are on fields which are part of the uniqueness # constraint cause the instance to ignore uniqueness validation. From 2e7fae7698c2a0dfc3cae6efa30f5b6fb9b1e90f Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Mon, 13 Jun 2016 16:32:43 +0100 Subject: [PATCH 09/10] limit=0 should revert to default limit (#4194) --- rest_framework/pagination.py | 1 + tests/test_pagination.py | 38 ++++++++++++------------------------ 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index 153a77d33..fc20ea266 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -319,6 +319,7 @@ class LimitOffsetPagination(BasePagination): try: return _positive_int( request.query_params[self.limit_query_param], + strict=True, cutoff=self.max_limit ) except (KeyError, ValueError): diff --git a/tests/test_pagination.py b/tests/test_pagination.py index ba3cc7037..4ea706429 100644 --- a/tests/test_pagination.py +++ b/tests/test_pagination.py @@ -486,6 +486,19 @@ class TestLimitOffset: assert queryset == [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] assert content.get('next') == next_url + def test_zero_limit(self): + """ + An zero limit query param should be ignored in favor of the default. + """ + request = Request(factory.get('/', {'limit': 0, 'offset': 0})) + queryset = self.paginate_queryset(request) + content = self.get_paginated_content(queryset) + next_limit = self.pagination.default_limit + next_offset = self.pagination.default_limit + next_url = 'http://testserver/?limit={0}&offset={1}'.format(next_limit, next_offset) + assert queryset == [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] + assert content.get('next') == next_url + def test_max_limit(self): """ The limit defaults to the max_limit when there is a max_limit and the @@ -505,31 +518,6 @@ class TestLimitOffset: assert content.get('next') == next_url assert content.get('previous') == prev_url - def test_limit_zero(self): - """ - A limit of 0 should return empty results. - """ - request = Request(factory.get('/', {'limit': 0, 'offset': 10})) - queryset = self.paginate_queryset(request) - context = self.get_html_context() - content = self.get_paginated_content(queryset) - - assert context == { - 'previous_url': 'http://testserver/?limit=0&offset=10', - 'page_links': [ - PageLink( - url='http://testserver/?limit=0', - number=1, - is_active=True, - is_break=False - ) - ], - 'next_url': 'http://testserver/?limit=0&offset=10' - } - - assert queryset == [] - assert content.get('results') == [] - class TestCursorPagination: """ From 1633a0a2b1ae21bfa5d971c583039577a1e7abef Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Mon, 13 Jun 2016 16:52:45 +0100 Subject: [PATCH 10/10] Add test confirming that required=False is valid on a relational field (#4195) --- tests/test_relations_pk.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_relations_pk.py b/tests/test_relations_pk.py index ba75bd94f..7a3d45927 100644 --- a/tests/test_relations_pk.py +++ b/tests/test_relations_pk.py @@ -340,6 +340,18 @@ class PKForeignKeyTests(TestCase): serializer = NullableForeignKeySourceSerializer() self.assertEqual(serializer.data['target'], None) + def test_foreign_key_not_required(self): + """ + Let's say we wanted to fill the non-nullable model field inside + Model.save(), we would make it empty and not required. + """ + class ModelSerializer(ForeignKeySourceSerializer): + class Meta(ForeignKeySourceSerializer.Meta): + extra_kwargs = {'target': {'required': False}} + serializer = ModelSerializer(data={'name': 'test'}) + serializer.is_valid(raise_exception=True) + self.assertNotIn('target', serializer.validated_data) + class PKNullableForeignKeyTests(TestCase): def setUp(self):