From b428a6cafd18072cf457a751be2becb616c3b099 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Fri, 26 Jun 2015 10:03:17 -0400 Subject: [PATCH 1/5] Implement SUPPORT_PATCH setting --- rest_framework/settings.py | 1 + rest_framework/views.py | 9 +++++++-- tests/test_generics.py | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/rest_framework/settings.py b/rest_framework/settings.py index 4c46a4831..6dfd573a1 100644 --- a/rest_framework/settings.py +++ b/rest_framework/settings.py @@ -54,6 +54,7 @@ DEFAULTS = { # Generic view behavior 'DEFAULT_PAGINATION_CLASS': 'rest_framework.pagination.PageNumberPagination', 'DEFAULT_FILTER_BACKENDS': (), + 'SUPPORT_PATCH': True, # Throttling 'DEFAULT_THROTTLE_RATES': { diff --git a/rest_framework/views.py b/rest_framework/views.py index a709c2f6b..faa197e1e 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -129,7 +129,12 @@ class APIView(View): """ Wrap Django's private `_allowed_methods` interface in a public property. """ - return self._allowed_methods() + allowed_methods = self._allowed_methods() + + if not api_settings.SUPPORT_PATCH and 'PATCH' in allowed_methods: + allowed_methods.remove('PATCH') + + return allowed_methods @property def default_response_headers(self): @@ -451,7 +456,7 @@ class APIView(View): self.initial(request, *args, **kwargs) # Get the appropriate handler method - if request.method.lower() in self.http_method_names: + if request.method in self.allowed_methods: handler = getattr(self, request.method.lower(), self.http_method_not_allowed) else: diff --git a/tests/test_generics.py b/tests/test_generics.py index 0647119bf..d98c86dea 100644 --- a/tests/test_generics.py +++ b/tests/test_generics.py @@ -7,6 +7,7 @@ from django.test import TestCase from django.utils import six from rest_framework import generics, renderers, serializers, status +from rest_framework.settings import api_settings from rest_framework.test import APIRequestFactory from tests.models import ( BasicModel, ForeignKeySource, ForeignKeyTarget, RESTFrameworkModel @@ -213,6 +214,19 @@ class TestInstanceView(TestCase): updated = self.objects.get(id=1) self.assertEqual(updated.text, 'foobar') + def test_patch_instance_view_support_patch(self): + """ + PATCH requests with SUPPORT_PATCH=False should return 405 + """ + data = {'text': 'foobar'} + request = factory.patch('/1', data, format='json') + + api_settings.SUPPORT_PATCH = False + response = self.view(request, pk=1).render() + api_settings.SUPPORT_PATCH = True + + self.assertEqual(response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) + def test_delete_instance_view(self): """ DELETE requests to RetrieveUpdateDestroyAPIView should delete an object. From 1402f51327eaa13e96eccf2ea59e23072c3bf105 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=CC=81=20Padilla?= Date: Fri, 26 Jun 2015 10:31:50 -0400 Subject: [PATCH 2/5] Flexible implementation for SUPPORT_PATCH --- rest_framework/generics.py | 15 +++++++++------ rest_framework/views.py | 9 ++------- tests/test_generics.py | 35 +++++++++++++++++++++-------------- 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/rest_framework/generics.py b/rest_framework/generics.py index 88438e8c4..dc45bda01 100644 --- a/rest_framework/generics.py +++ b/rest_framework/generics.py @@ -229,8 +229,9 @@ class UpdateAPIView(mixins.UpdateModelMixin, def put(self, request, *args, **kwargs): return self.update(request, *args, **kwargs) - def patch(self, request, *args, **kwargs): - return self.partial_update(request, *args, **kwargs) + if api_settings.SUPPORT_PATCH: + def patch(self, request, *args, **kwargs): + return self.partial_update(request, *args, **kwargs) class ListCreateAPIView(mixins.ListModelMixin, @@ -258,8 +259,9 @@ class RetrieveUpdateAPIView(mixins.RetrieveModelMixin, def put(self, request, *args, **kwargs): return self.update(request, *args, **kwargs) - def patch(self, request, *args, **kwargs): - return self.partial_update(request, *args, **kwargs) + if api_settings.SUPPORT_PATCH: + def patch(self, request, *args, **kwargs): + return self.partial_update(request, *args, **kwargs) class RetrieveDestroyAPIView(mixins.RetrieveModelMixin, @@ -288,8 +290,9 @@ class RetrieveUpdateDestroyAPIView(mixins.RetrieveModelMixin, def put(self, request, *args, **kwargs): return self.update(request, *args, **kwargs) - def patch(self, request, *args, **kwargs): - return self.partial_update(request, *args, **kwargs) + if api_settings.SUPPORT_PATCH: + def patch(self, request, *args, **kwargs): + return self.partial_update(request, *args, **kwargs) def delete(self, request, *args, **kwargs): return self.destroy(request, *args, **kwargs) diff --git a/rest_framework/views.py b/rest_framework/views.py index faa197e1e..a709c2f6b 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -129,12 +129,7 @@ class APIView(View): """ Wrap Django's private `_allowed_methods` interface in a public property. """ - allowed_methods = self._allowed_methods() - - if not api_settings.SUPPORT_PATCH and 'PATCH' in allowed_methods: - allowed_methods.remove('PATCH') - - return allowed_methods + return self._allowed_methods() @property def default_response_headers(self): @@ -456,7 +451,7 @@ class APIView(View): self.initial(request, *args, **kwargs) # Get the appropriate handler method - if request.method in self.allowed_methods: + if request.method.lower() in self.http_method_names: handler = getattr(self, request.method.lower(), self.http_method_not_allowed) else: diff --git a/tests/test_generics.py b/tests/test_generics.py index d98c86dea..083f95f60 100644 --- a/tests/test_generics.py +++ b/tests/test_generics.py @@ -4,10 +4,11 @@ import django from django.db import models from django.shortcuts import get_object_or_404 from django.test import TestCase +from django.test.utils import override_settings from django.utils import six +from django.utils.six.moves import reload_module from rest_framework import generics, renderers, serializers, status -from rest_framework.settings import api_settings from rest_framework.test import APIRequestFactory from tests.models import ( BasicModel, ForeignKeySource, ForeignKeyTarget, RESTFrameworkModel @@ -214,19 +215,6 @@ class TestInstanceView(TestCase): updated = self.objects.get(id=1) self.assertEqual(updated.text, 'foobar') - def test_patch_instance_view_support_patch(self): - """ - PATCH requests with SUPPORT_PATCH=False should return 405 - """ - data = {'text': 'foobar'} - request = factory.patch('/1', data, format='json') - - api_settings.SUPPORT_PATCH = False - response = self.view(request, pk=1).render() - api_settings.SUPPORT_PATCH = True - - self.assertEqual(response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) - def test_delete_instance_view(self): """ DELETE requests to RetrieveUpdateDestroyAPIView should delete an object. @@ -521,3 +509,22 @@ class TestFilterBackendAppliedToViews(TestCase): response = view(request).render() self.assertContains(response, 'field_b') self.assertNotContains(response, 'field_a') + + +class TestSupportPatchSetting(TestCase): + def test_patch_instance_view_support_patch(self): + """ + PATCH requests should fail when SUPPORT_PATCH is set to False. + """ + with override_settings(REST_FRAMEWORK={'SUPPORT_PATCH': False}): + reload_module(generics) + data = {'text': 'foobar'} + request = factory.patch('/1', data, format='json') + + class InstanceView(generics.UpdateAPIView): + queryset = BasicModel.objects.all() + serializer_class = BasicSerializer + + view = InstanceView.as_view() + response = view(request, pk=1).render() + self.assertEqual(response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) From d322c3fa4b7ad9dff6d98ab31e42ea9311e3175a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Fri, 26 Jun 2015 11:40:55 -0400 Subject: [PATCH 3/5] Check for SUPPORT_PATCH in partial_update --- rest_framework/generics.py | 15 ++++++--------- rest_framework/mixins.py | 5 ++++- tests/test_generics.py | 27 +++++++++++++++------------ 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/rest_framework/generics.py b/rest_framework/generics.py index dc45bda01..88438e8c4 100644 --- a/rest_framework/generics.py +++ b/rest_framework/generics.py @@ -229,9 +229,8 @@ class UpdateAPIView(mixins.UpdateModelMixin, def put(self, request, *args, **kwargs): return self.update(request, *args, **kwargs) - if api_settings.SUPPORT_PATCH: - def patch(self, request, *args, **kwargs): - return self.partial_update(request, *args, **kwargs) + def patch(self, request, *args, **kwargs): + return self.partial_update(request, *args, **kwargs) class ListCreateAPIView(mixins.ListModelMixin, @@ -259,9 +258,8 @@ class RetrieveUpdateAPIView(mixins.RetrieveModelMixin, def put(self, request, *args, **kwargs): return self.update(request, *args, **kwargs) - if api_settings.SUPPORT_PATCH: - def patch(self, request, *args, **kwargs): - return self.partial_update(request, *args, **kwargs) + def patch(self, request, *args, **kwargs): + return self.partial_update(request, *args, **kwargs) class RetrieveDestroyAPIView(mixins.RetrieveModelMixin, @@ -290,9 +288,8 @@ class RetrieveUpdateDestroyAPIView(mixins.RetrieveModelMixin, def put(self, request, *args, **kwargs): return self.update(request, *args, **kwargs) - if api_settings.SUPPORT_PATCH: - def patch(self, request, *args, **kwargs): - return self.partial_update(request, *args, **kwargs) + def patch(self, request, *args, **kwargs): + return self.partial_update(request, *args, **kwargs) def delete(self, request, *args, **kwargs): return self.destroy(request, *args, **kwargs) diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index 1104aa29c..c338efa4f 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -6,7 +6,7 @@ which allows mixin classes to be composed in interesting ways. """ from __future__ import unicode_literals -from rest_framework import status +from rest_framework import exceptions, status from rest_framework.response import Response from rest_framework.settings import api_settings @@ -74,6 +74,9 @@ class UpdateModelMixin(object): serializer.save() def partial_update(self, request, *args, **kwargs): + if not api_settings.SUPPORT_PATCH: + raise exceptions.MethodNotAllowed(request.method) + kwargs['partial'] = True return self.update(request, *args, **kwargs) diff --git a/tests/test_generics.py b/tests/test_generics.py index 083f95f60..80a4ed66d 100644 --- a/tests/test_generics.py +++ b/tests/test_generics.py @@ -4,11 +4,10 @@ import django from django.db import models from django.shortcuts import get_object_or_404 from django.test import TestCase -from django.test.utils import override_settings from django.utils import six -from django.utils.six.moves import reload_module from rest_framework import generics, renderers, serializers, status +from rest_framework.settings import api_settings from rest_framework.test import APIRequestFactory from tests.models import ( BasicModel, ForeignKeySource, ForeignKeyTarget, RESTFrameworkModel @@ -516,15 +515,19 @@ class TestSupportPatchSetting(TestCase): """ PATCH requests should fail when SUPPORT_PATCH is set to False. """ - with override_settings(REST_FRAMEWORK={'SUPPORT_PATCH': False}): - reload_module(generics) - data = {'text': 'foobar'} - request = factory.patch('/1', data, format='json') + obj = BasicModel.objects.create(text='abc') - class InstanceView(generics.UpdateAPIView): - queryset = BasicModel.objects.all() - serializer_class = BasicSerializer + class InstanceView(generics.UpdateAPIView): + queryset = BasicModel.objects.all() + serializer_class = BasicSerializer - view = InstanceView.as_view() - response = view(request, pk=1).render() - self.assertEqual(response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) + api_settings.SUPPORT_PATCH = False + + data = {'text': 'foobar'} + request = factory.patch('/1', data, format='json') + view = InstanceView.as_view() + response = view(request, pk=obj.pk).render() + + api_settings.SUPPORT_PATCH = True + + self.assertEqual(response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) From b9dec98647e129dd14a77afbc3fe83991ce7e851 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Fri, 26 Jun 2015 11:51:24 -0400 Subject: [PATCH 4/5] Correctly report allowed methods --- rest_framework/views.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rest_framework/views.py b/rest_framework/views.py index a709c2f6b..102a8479e 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -129,7 +129,12 @@ class APIView(View): """ Wrap Django's private `_allowed_methods` interface in a public property. """ - return self._allowed_methods() + allowed_methods = self._allowed_methods() + + if not api_settings.SUPPORT_PATCH and 'PATCH' in allowed_methods: + allowed_methods.remove('PATCH') + + return allowed_methods @property def default_response_headers(self): From f0e157dcec486ae7f2ec34beea4556c98d681490 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Fri, 26 Jun 2015 13:14:07 -0400 Subject: [PATCH 5/5] Add SUPPORT_PATCH setting docs --- docs/api-guide/settings.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/api-guide/settings.md b/docs/api-guide/settings.md index 6fdb77c86..150058c1a 100644 --- a/docs/api-guide/settings.md +++ b/docs/api-guide/settings.md @@ -447,6 +447,12 @@ An integer of 0 or more, that may be used to specify the number of application p Default: `None` +#### SUPPORT_PATCH + +If set to `False` then HTTP PATCH requests will not be allowed. + +Default: `True` + [cite]: http://www.python.org/dev/peps/pep-0020/ [rfc4627]: http://www.ietf.org/rfc/rfc4627.txt [heroku-minified-json]: https://github.com/interagent/http-api-design#keep-json-minified-in-all-responses