From 4c4dfccf5303383126c4c132d5c80ed53e7acfe9 Mon Sep 17 00:00:00 2001 From: Boris Pleshakov Date: Mon, 30 Mar 2020 18:20:42 +0300 Subject: [PATCH 01/11] f wrong argument --- rest_framework_json_api/parsers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework_json_api/parsers.py b/rest_framework_json_api/parsers.py index 7a940b6c..fe42e3c6 100644 --- a/rest_framework_json_api/parsers.py +++ b/rest_framework_json_api/parsers.py @@ -150,7 +150,7 @@ def parse(self, stream, media_type=None, parser_context=None): "The resource object's id ({data_id}) does not match url's " "lookup id ({url_id})".format( data_id=data.get('id'), - url_id=view.kwargs[view.lookup_field] + url_id=view.kwargs[lookup_url_kwarg] ) ) From fc928b019f7ea06bd1657f1659a2bdf0a2f35dbf Mon Sep 17 00:00:00 2001 From: Boris Pleshakov Date: Fri, 10 Apr 2020 00:09:20 +0300 Subject: [PATCH 02/11] f whenever lookup_field and lookup_url_kwarg doesn't exist ignore. --- rest_framework_json_api/parsers.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/rest_framework_json_api/parsers.py b/rest_framework_json_api/parsers.py index fe42e3c6..6a52115d 100644 --- a/rest_framework_json_api/parsers.py +++ b/rest_framework_json_api/parsers.py @@ -144,8 +144,13 @@ def parse(self, stream, media_type=None, parser_context=None): raise ParseError("The resource identifier object must contain an 'id' member") if request.method in ('PATCH', 'PUT'): - lookup_url_kwarg = view.lookup_url_kwarg or view.lookup_field - if str(data.get('id')) != str(view.kwargs[lookup_url_kwarg]): + # "or" syntax throws AttributeException which is silenced by DRF + lookup_url_kwarg = None + if hasattr(view, 'lookup_url_kwarg'): + lookup_url_kwarg = view.lookup_url_kwarg + elif hasattr(view, 'lookup_field'): + lookup_url_kwarg = view.lookup_field + if lookup_url_kwarg or str(data.get('id')) != str(view.kwargs[lookup_url_kwarg]): raise exceptions.Conflict( "The resource object's id ({data_id}) does not match url's " "lookup id ({url_id})".format( From f0fefc25886e98189b8f17e1077eae8beab23750 Mon Sep 17 00:00:00 2001 From: Boris Pleshakov Date: Fri, 10 Apr 2020 00:25:26 +0300 Subject: [PATCH 03/11] f whenever lookup_field and lookup_url_kwarg doesn't exist ignore. --- rest_framework_json_api/parsers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework_json_api/parsers.py b/rest_framework_json_api/parsers.py index 6a52115d..301ef496 100644 --- a/rest_framework_json_api/parsers.py +++ b/rest_framework_json_api/parsers.py @@ -150,7 +150,7 @@ def parse(self, stream, media_type=None, parser_context=None): lookup_url_kwarg = view.lookup_url_kwarg elif hasattr(view, 'lookup_field'): lookup_url_kwarg = view.lookup_field - if lookup_url_kwarg or str(data.get('id')) != str(view.kwargs[lookup_url_kwarg]): + if lookup_url_kwarg and str(data.get('id')) != str(view.kwargs[lookup_url_kwarg]): raise exceptions.Conflict( "The resource object's id ({data_id}) does not match url's " "lookup id ({url_id})".format( From 9472e6147451cbdb695bed7133650cbe53409651 Mon Sep 17 00:00:00 2001 From: Boris Pleshakov Date: Fri, 10 Apr 2020 17:40:51 +0300 Subject: [PATCH 04/11] f whenever lookup_field and lookup_url_kwarg doesn't exist ignore. --- rest_framework_json_api/parsers.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/rest_framework_json_api/parsers.py b/rest_framework_json_api/parsers.py index 301ef496..c8fe07f1 100644 --- a/rest_framework_json_api/parsers.py +++ b/rest_framework_json_api/parsers.py @@ -144,12 +144,7 @@ def parse(self, stream, media_type=None, parser_context=None): raise ParseError("The resource identifier object must contain an 'id' member") if request.method in ('PATCH', 'PUT'): - # "or" syntax throws AttributeException which is silenced by DRF - lookup_url_kwarg = None - if hasattr(view, 'lookup_url_kwarg'): - lookup_url_kwarg = view.lookup_url_kwarg - elif hasattr(view, 'lookup_field'): - lookup_url_kwarg = view.lookup_field + lookup_url_kwarg = getattr(view, 'lookup_url_kwarg', None) or getattr(view, 'lookup_field', None) if lookup_url_kwarg and str(data.get('id')) != str(view.kwargs[lookup_url_kwarg]): raise exceptions.Conflict( "The resource object's id ({data_id}) does not match url's " From 4f3f05b4089de7fd80783e6ad0cc8e1f6c1010cc Mon Sep 17 00:00:00 2001 From: Boris Pleshakov Date: Fri, 10 Apr 2020 17:57:19 +0300 Subject: [PATCH 05/11] f whenever lookup_field and lookup_url_kwarg doesn't exist ignore. --- rest_framework_json_api/parsers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rest_framework_json_api/parsers.py b/rest_framework_json_api/parsers.py index c8fe07f1..7e018938 100644 --- a/rest_framework_json_api/parsers.py +++ b/rest_framework_json_api/parsers.py @@ -144,7 +144,8 @@ def parse(self, stream, media_type=None, parser_context=None): raise ParseError("The resource identifier object must contain an 'id' member") if request.method in ('PATCH', 'PUT'): - lookup_url_kwarg = getattr(view, 'lookup_url_kwarg', None) or getattr(view, 'lookup_field', None) + lookup_url_kwarg = getattr(view, 'lookup_url_kwarg', None) or \ + getattr(view, 'lookup_field', None) if lookup_url_kwarg and str(data.get('id')) != str(view.kwargs[lookup_url_kwarg]): raise exceptions.Conflict( "The resource object's id ({data_id}) does not match url's " From 6558aa2be7476b92ed7917ae6196f55434a11b28 Mon Sep 17 00:00:00 2001 From: Boris Pleshakov Date: Tue, 14 Apr 2020 03:46:38 +0300 Subject: [PATCH 06/11] f pep8 --- rest_framework_json_api/parsers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework_json_api/parsers.py b/rest_framework_json_api/parsers.py index 7e018938..88c4f522 100644 --- a/rest_framework_json_api/parsers.py +++ b/rest_framework_json_api/parsers.py @@ -145,7 +145,7 @@ def parse(self, stream, media_type=None, parser_context=None): if request.method in ('PATCH', 'PUT'): lookup_url_kwarg = getattr(view, 'lookup_url_kwarg', None) or \ - getattr(view, 'lookup_field', None) + getattr(view, 'lookup_field', None) if lookup_url_kwarg and str(data.get('id')) != str(view.kwargs[lookup_url_kwarg]): raise exceptions.Conflict( "The resource object's id ({data_id}) does not match url's " From f6810989d183a559d08fd3df945a947633bfe250 Mon Sep 17 00:00:00 2001 From: Boris Pleshakov Date: Tue, 14 Apr 2020 15:26:15 +0300 Subject: [PATCH 07/11] f unit test --- example/tests/test_parsers.py | 51 +++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/example/tests/test_parsers.py b/example/tests/test_parsers.py index 6ff2cfa7..1d52fd3a 100644 --- a/example/tests/test_parsers.py +++ b/example/tests/test_parsers.py @@ -2,8 +2,11 @@ from io import BytesIO from django.test import TestCase, override_settings +from rest_framework import views, status from rest_framework.exceptions import ParseError +from rest_framework.response import Response +from rest_framework_json_api import serializers from rest_framework_json_api.parsers import JSONParser @@ -69,3 +72,51 @@ def test_parse_invalid_data_key(self): with self.assertRaises(ParseError): parser.parse(stream, None, self.parser_context) + + +class DummySerializer(serializers.Serializer): + body = serializers.CharField() + + +class DummyAPIView(views.APIView): + parser_classes = [JSONParser] + resource_name = 'dummy' + + def patch(self, request, *args, **kwargs): + serializer = DummySerializer(data=request.data) + serializer.is_valid(raise_exception=True) + return Response(status=status.HTTP_200_OK, data=serializer.validated_data) + + +class TestParserOnAPIView(TestCase): + + def setUp(self): + class MockRequest(object): + def __init__(self): + self.method = 'PATCH' + + request = MockRequest() + + self.parser_context = {'request': request, 'kwargs': {}, 'view': 'DummyAPIView'} + + data = { + 'data': { + 'id': 123, + 'type': 'strs', + 'attributes': { + 'body': 'hello' + }, + } + } + + self.string = json.dumps(data) + + def test_patch_doesnt_raise_attribute_error(self): + parser = JSONParser() + + stream = BytesIO(self.string.encode('utf-8')) + + data = parser.parse(stream, None, self.parser_context) + + assert data['id'] == 123 + assert data['body'] == 'hello' From 859740aab85601bacd06964cc21045db643ad9ea Mon Sep 17 00:00:00 2001 From: Boris Pleshakov Date: Tue, 14 Apr 2020 15:30:21 +0300 Subject: [PATCH 08/11] + changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2dc90b7..947c4d3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Note that in line with [Django REST Framework policy](http://www.django-rest-framework.org/topics/release-notes/), any parts of the framework not mentioned in the documentation should generally be considered private API, and may be subject to change. +### Fixed + +* Shadowed AttributeError for PUT and PATCH methods in `APIView` + ## [3.1.0] - 2020-02-08 ### Added From c123aa56ef7248285866478d8320fbfe104f7200 Mon Sep 17 00:00:00 2001 From: Boris Pleshakov Date: Tue, 14 Apr 2020 16:42:42 +0300 Subject: [PATCH 09/11] + test with actual view usage --- example/tests/test_parsers.py | 42 ++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/example/tests/test_parsers.py b/example/tests/test_parsers.py index 1d52fd3a..79597950 100644 --- a/example/tests/test_parsers.py +++ b/example/tests/test_parsers.py @@ -1,13 +1,16 @@ import json from io import BytesIO +from django.conf.urls import url from django.test import TestCase, override_settings +from django.urls import reverse from rest_framework import views, status from rest_framework.exceptions import ParseError from rest_framework.response import Response from rest_framework_json_api import serializers from rest_framework_json_api.parsers import JSONParser +from rest_framework_json_api.renderers import JSONRenderer class TestJSONParser(TestCase): @@ -74,18 +77,34 @@ def test_parse_invalid_data_key(self): parser.parse(stream, None, self.parser_context) +class DummyDTO: + def __init__(self, response_dict): + for k, v in response_dict.items(): + setattr(self, k, v) + + @property + def pk(self): + return self.id if hasattr(self, 'id') else None + + class DummySerializer(serializers.Serializer): body = serializers.CharField() + id = serializers.IntegerField() class DummyAPIView(views.APIView): parser_classes = [JSONParser] + renderer_classes = [JSONRenderer] resource_name = 'dummy' def patch(self, request, *args, **kwargs): - serializer = DummySerializer(data=request.data) - serializer.is_valid(raise_exception=True) - return Response(status=status.HTTP_200_OK, data=serializer.validated_data) + serializer = DummySerializer(DummyDTO(request.data)) + return Response(status=status.HTTP_200_OK, data=serializer.data) + + +urlpatterns = [ + url(r'repeater$', DummyAPIView.as_view(), name='repeater'), +] class TestParserOnAPIView(TestCase): @@ -96,10 +115,10 @@ def __init__(self): self.method = 'PATCH' request = MockRequest() - + # To be honest view string isn't resolved into actual view self.parser_context = {'request': request, 'kwargs': {}, 'view': 'DummyAPIView'} - data = { + self.data = { 'data': { 'id': 123, 'type': 'strs', @@ -109,7 +128,7 @@ def __init__(self): } } - self.string = json.dumps(data) + self.string = json.dumps(self.data) def test_patch_doesnt_raise_attribute_error(self): parser = JSONParser() @@ -120,3 +139,14 @@ def test_patch_doesnt_raise_attribute_error(self): assert data['id'] == 123 assert data['body'] == 'hello' + + @override_settings(ROOT_URLCONF=__name__) + def test_patch_request(self): + url = reverse('repeater') + data = self.data + data['data']['type'] = 'dummy' + response = self.client.patch(url, data, content_type='application/vnd.api+json') + data = response.json() + + assert data['data']['id'] == str(123) + assert data['data']['attributes']['body'] == 'hello' From 84f3d0ce6e5f22676b5f726f984ba886c23ca1d3 Mon Sep 17 00:00:00 2001 From: Boris Pleshakov Date: Tue, 14 Apr 2020 17:35:28 +0300 Subject: [PATCH 10/11] f test with actual view usage --- example/tests/test_parsers.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/example/tests/test_parsers.py b/example/tests/test_parsers.py index 79597950..3a2459c0 100644 --- a/example/tests/test_parsers.py +++ b/example/tests/test_parsers.py @@ -7,6 +7,7 @@ from rest_framework import views, status from rest_framework.exceptions import ParseError from rest_framework.response import Response +from rest_framework.test import APITestCase from rest_framework_json_api import serializers from rest_framework_json_api.parsers import JSONParser @@ -107,7 +108,7 @@ def patch(self, request, *args, **kwargs): ] -class TestParserOnAPIView(TestCase): +class TestParserOnAPIView(APITestCase): def setUp(self): class MockRequest(object): @@ -145,7 +146,7 @@ def test_patch_request(self): url = reverse('repeater') data = self.data data['data']['type'] = 'dummy' - response = self.client.patch(url, data, content_type='application/vnd.api+json') + response = self.client.patch(url, data=data) data = response.json() assert data['data']['id'] == str(123) From fc9c390a151b5b4da0002e98a30c0c5dcd2209da Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Sat, 18 Apr 2020 17:54:40 +0400 Subject: [PATCH 11/11] Update CHANGELOG.md --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 947c4d3e..85942770 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Note that in line with [Django REST Framework policy](http://www.django-rest-framework.org/topics/release-notes/), any parts of the framework not mentioned in the documentation should generally be considered private API, and may be subject to change. +## [Unreleased] + ### Fixed -* Shadowed AttributeError for PUT and PATCH methods in `APIView` +* Avoid `AttributeError` for PUT and PATCH methods when using `APIView` ## [3.1.0] - 2020-02-08