diff --git a/CHANGELOG.md b/CHANGELOG.md index 85942770..abb46967 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,12 @@ 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] +## [3.2.0] - pending -### Fixed +### Added -* Avoid `AttributeError` for PUT and PATCH methods when using `APIView` +* Added support for serializiing complex structures as attributes. For details please reffer to #769 +* Avoid `AttributeError` for PUT and PATCH methods when using `APIView` ## [3.1.0] - 2020-02-08 diff --git a/example/tests/test_errors.py b/example/tests/test_errors.py new file mode 100644 index 00000000..0c334e91 --- /dev/null +++ b/example/tests/test_errors.py @@ -0,0 +1,238 @@ +import pytest +from django.test import override_settings +from django.urls import reverse, path + +from example.models import Blog +from rest_framework_json_api import serializers +from rest_framework import views + + +# serializers +class CommentAttachmentSerializer(serializers.Serializer): + data = serializers.CharField(allow_null=False, required=True) + + def validate_data(self, value): + if value and len(value) < 10: + raise serializers.ValidationError('Too short data') + + +class CommentSerializer(serializers.Serializer): + attachments = CommentAttachmentSerializer(many=True, required=False) + attachment = CommentAttachmentSerializer(required=False) + one_more_attachment = CommentAttachmentSerializer(required=False) + body = serializers.CharField(allow_null=False, required=True) + + +class EntrySerializer(serializers.Serializer): + blog = serializers.IntegerField() + comments = CommentSerializer(many=True, required=False) + comment = CommentSerializer(required=False) + headline = serializers.CharField(allow_null=True, required=True) + body_text = serializers.CharField() + + def validate(self, attrs): + body_text = attrs['body_text'] + if len(body_text) < 5: + raise serializers.ValidationError({'body_text': 'Too short'}) + + +# view +class DummyTestView(views.APIView): + serializer_class = EntrySerializer + resource_name = 'entries' + + def post(self, request, *args, **kwargs): + serializer = self.serializer_class(data=request.data) + serializer.is_valid(raise_exception=True) + + +urlpatterns = [ + path(r'^entries-nested/$', DummyTestView.as_view(), + name='entries-nested-list') +] + + +@pytest.fixture(scope='function') +def some_blog(db): + return Blog.objects.create(name='Some Blog', tagline="It's a blog") + + +def perform_error_test(client, data, expected_pointer, errors_count=1): + with override_settings( + JSON_API_SERIALIZE_NESTED_SERIALIZERS_AS_ATTRIBUTE=True, + ROOT_URLCONF=__name__ + ): + url = reverse('entries-nested-list') + response = client.post(url, data=data) + + errors = response.data + + assert len(errors) == errors_count + assert errors[0]['source']['pointer'] == expected_pointer + + +def test_first_level_attribute_error(client, some_blog): + data = { + 'data': { + 'type': 'entries', + 'attributes': { + 'blog': some_blog.pk, + 'body_text': 'body_text', + } + } + } + perform_error_test(client, data, '/data/attributes/headline') + + +def test_first_level_custom_attribute_error(client, some_blog): + data = { + 'data': { + 'type': 'entries', + 'attributes': { + 'blog': some_blog.pk, + 'body_text': 'body', + 'headline': 'headline' + } + } + } + with override_settings(JSON_API_FORMAT_FIELD_NAMES='underscore'): + perform_error_test(client, data, '/data/attributes/body_text') + + +def test_second_level_array_error(client, some_blog): + data = { + 'data': { + 'type': 'entries', + 'attributes': { + 'blog': some_blog.pk, + 'body_text': 'body_text', + 'headline': 'headline', + 'comments': [ + { + } + ] + } + } + } + + perform_error_test(client, data, '/data/attributes/comments/0/body') + + +def test_second_level_dict_error(client, some_blog): + data = { + 'data': { + 'type': 'entries', + 'attributes': { + 'blog': some_blog.pk, + 'body_text': 'body_text', + 'headline': 'headline', + 'comment': {} + } + } + } + + perform_error_test(client, data, '/data/attributes/comment/body') + + +def test_third_level_array_error(client, some_blog): + data = { + 'data': { + 'type': 'entries', + 'attributes': { + 'blog': some_blog.pk, + 'body_text': 'body_text', + 'headline': 'headline', + 'comments': [ + { + 'body': 'test comment', + 'attachments': [ + { + } + ] + } + ] + } + } + } + + perform_error_test(client, data, '/data/attributes/comments/0/attachments/0/data') + + +def test_third_level_custom_array_error(client, some_blog): + data = { + 'data': { + 'type': 'entries', + 'attributes': { + 'blog': some_blog.pk, + 'body_text': 'body_text', + 'headline': 'headline', + 'comments': [ + { + 'body': 'test comment', + 'attachments': [ + { + 'data': 'text' + } + ] + } + ] + } + } + } + + perform_error_test(client, data, '/data/attributes/comments/0/attachments/0/data') + + +def test_third_level_dict_error(client, some_blog): + data = { + 'data': { + 'type': 'entries', + 'attributes': { + 'blog': some_blog.pk, + 'body_text': 'body_text', + 'headline': 'headline', + 'comments': [ + { + 'body': 'test comment', + 'attachment': {} + } + ] + } + } + } + + perform_error_test(client, data, '/data/attributes/comments/0/attachment/data') + + +def test_many_third_level_dict_errors(client, some_blog): + data = { + 'data': { + 'type': 'entries', + 'attributes': { + 'blog': some_blog.pk, + 'body_text': 'body_text', + 'headline': 'headline', + 'comments': [ + { + 'attachment': {} + } + ] + } + } + } + + perform_error_test(client, data, '/data/attributes/comments/0/body', 2) + + +@pytest.mark.filterwarning('default::DeprecationWarning:rest_framework_json_api.serializers') +def test_deprecation_warning(recwarn): + class DummyNestedSerializer(serializers.Serializer): + field = serializers.CharField() + + class DummySerializer(serializers.Serializer): + nested = DummyNestedSerializer(many=True) + + assert len(recwarn) == 1 + warning = recwarn.pop(DeprecationWarning) + assert warning + assert str(warning.message).startswith('Rendering') diff --git a/example/tests/unit/test_renderers.py b/example/tests/unit/test_renderers.py index e432704d..7aa805ee 100644 --- a/example/tests/unit/test_renderers.py +++ b/example/tests/unit/test_renderers.py @@ -1,11 +1,13 @@ import json import pytest +from django.test import override_settings +from django.utils import timezone from rest_framework_json_api import serializers, views from rest_framework_json_api.renderers import JSONRenderer -from example.models import Author, Comment, Entry +from example.models import Author, Comment, Entry, Blog # serializers @@ -38,6 +40,31 @@ class JSONAPIMeta: included_resources = ('related_models',) +class EntryDRFSerializers(serializers.ModelSerializer): + + class Meta: + model = Entry + fields = ('headline', 'body_text') + read_only_fields = ('tags',) + + +class CommentWithNestedFieldsSerializer(serializers.ModelSerializer): + entry = EntryDRFSerializers() + + class Meta: + model = Comment + exclude = ('created_at', 'modified_at', 'author') + # fields = ('entry', 'body', 'author',) + + +class AuthorWithNestedFieldsSerializer(serializers.ModelSerializer): + comments = CommentWithNestedFieldsSerializer(many=True) + + class Meta: + model = Author + fields = ('name', 'email', 'comments') + + # views class DummyTestViewSet(views.ModelViewSet): queryset = Entry.objects.all() @@ -49,6 +76,12 @@ class ReadOnlyDummyTestViewSet(views.ReadOnlyModelViewSet): serializer_class = DummyTestSerializer +class AuthorWithNestedFieldsViewSet(views.ModelViewSet): + queryset = Author.objects.all() + serializer_class = AuthorWithNestedFieldsSerializer + resource_name = 'authors' + + def render_dummy_test_serialized_view(view_class, instance): serializer = view_class.serializer_class(instance=instance) renderer = JSONRenderer() @@ -138,3 +171,54 @@ def test_extract_relation_instance(comment): field=serializer.fields['blog'], resource_instance=comment ) assert got == comment.entry.blog + + +def test_attribute_rendering_strategy(db): + # setting up + blog = Blog.objects.create(name='Some Blog', tagline="It's a blog") + entry = Entry.objects.create( + blog=blog, + headline='headline', + body_text='body_text', + pub_date=timezone.now(), + mod_date=timezone.now(), + n_comments=0, + n_pingbacks=0, + rating=3 + ) + + author = Author.objects.create(name='some_author', email='some_author@example.org') + entry.authors.add(author) + + Comment.objects.create( + entry=entry, + body='testing one two three', + author=Author.objects.first() + ) + + with override_settings( + JSON_API_SERIALIZE_NESTED_SERIALIZERS_AS_ATTRIBUTE=True): + rendered = render_dummy_test_serialized_view(AuthorWithNestedFieldsViewSet, author) + result = json.loads(rendered.decode()) + + expected = { + "data": { + "type": "authors", + "id": "1", + "attributes": { + "name": "some_author", + "email": "some_author@example.org", + "comments": [ + { + "id": 1, + "entry": { + 'headline': 'headline', + 'body_text': 'body_text', + }, + "body": "testing one two three" + } + ] + } + } + } + assert expected == result diff --git a/example/views.py b/example/views.py index 33393be9..90272bee 100644 --- a/example/views.py +++ b/example/views.py @@ -23,8 +23,7 @@ EntryDRFSerializers, EntrySerializer, ProjectSerializer, - ProjectTypeSerializer -) + ProjectTypeSerializer) HTTP_422_UNPROCESSABLE_ENTITY = 422 diff --git a/pytest.ini b/pytest.ini index 2c69372d..ebf0e544 100644 --- a/pytest.ini +++ b/pytest.ini @@ -3,3 +3,4 @@ DJANGO_SETTINGS_MODULE=example.settings.test filterwarnings = error::DeprecationWarning error::PendingDeprecationWarning + ignore::DeprecationWarning:rest_framework_json_api.serializers \ No newline at end of file diff --git a/rest_framework_json_api/renderers.py b/rest_framework_json_api/renderers.py index 8da333ed..8872e392 100644 --- a/rest_framework_json_api/renderers.py +++ b/rest_framework_json_api/renderers.py @@ -13,6 +13,7 @@ from rest_framework.relations import PKOnlyObject from rest_framework.serializers import BaseSerializer, ListSerializer, Serializer from rest_framework.settings import api_settings +from .settings import json_api_settings import rest_framework_json_api from rest_framework_json_api import utils @@ -52,6 +53,7 @@ def extract_attributes(cls, fields, resource): Builds the `attributes` object of the JSON API resource object. """ data = OrderedDict() + render_nested_as_attribute = json_api_settings.SERIALIZE_NESTED_SERIALIZERS_AS_ATTRIBUTE for field_name, field in iter(fields.items()): # ID is always provided in the root of JSON API so remove it from attributes if field_name == 'id': @@ -61,10 +63,13 @@ def extract_attributes(cls, fields, resource): continue # Skip fields with relations if isinstance( - field, (relations.RelatedField, relations.ManyRelatedField, BaseSerializer) + field, (relations.RelatedField, relations.ManyRelatedField) ): continue + if isinstance(field, BaseSerializer) and not render_nested_as_attribute: + continue + # Skip read_only attribute fields when `resource` is an empty # serializer. Prevents the "Raw Data" form of the browsable API # from rendering `"foo": null` for read only fields @@ -89,6 +94,7 @@ def extract_relationships(cls, fields, resource, resource_instance): from rest_framework_json_api.relations import ResourceRelatedField data = OrderedDict() + render_nested_as_attribute = json_api_settings.SERIALIZE_NESTED_SERIALIZERS_AS_ATTRIBUTE # Don't try to extract relationships from a non-existent resource if resource_instance is None: @@ -109,6 +115,9 @@ def extract_relationships(cls, fields, resource, resource_instance): ): continue + if isinstance(field, BaseSerializer) and render_nested_as_attribute: + continue + source = field.source relation_type = utils.get_related_resource_type(field) @@ -327,18 +336,22 @@ def extract_included(cls, fields, resource, resource_instance, included_resource included_serializers = utils.get_included_serializers(current_serializer) included_resources = copy.copy(included_resources) included_resources = [inflection.underscore(value) for value in included_resources] + render_nested_as_attribute = json_api_settings.SERIALIZE_NESTED_SERIALIZERS_AS_ATTRIBUTE for field_name, field in iter(fields.items()): # Skip URL field if field_name == api_settings.URL_FIELD_NAME: continue - # Skip fields without relations or serialized data + # Skip fields without relations if not isinstance( - field, (relations.RelatedField, relations.ManyRelatedField, BaseSerializer) + field, (relations.RelatedField, relations.ManyRelatedField, BaseSerializer) ): continue + if isinstance(field, BaseSerializer) and render_nested_as_attribute: + continue + try: included_resources.remove(field_name) except ValueError: diff --git a/rest_framework_json_api/serializers.py b/rest_framework_json_api/serializers.py index 56688bd9..4de984f9 100644 --- a/rest_framework_json_api/serializers.py +++ b/rest_framework_json_api/serializers.py @@ -1,3 +1,5 @@ +import warnings + import inflection from django.core.exceptions import ObjectDoesNotExist from django.db.models.query import QuerySet @@ -15,6 +17,8 @@ get_resource_type_from_serializer ) +from rest_framework_json_api.settings import json_api_settings + class ResourceIdentifierObjectSerializer(BaseSerializer): default_error_messages = { @@ -115,8 +119,41 @@ def validate_path(serializer_class, field_path, path): super(IncludedResourcesValidationMixin, self).__init__(*args, **kwargs) +class SerializerMetaclass(SerializerMetaclass): + + @classmethod + def _get_declared_fields(cls, bases, attrs): + fields = super()._get_declared_fields(bases, attrs) + for field_name, field in fields.items(): + if isinstance(field, BaseSerializer) and \ + not json_api_settings.SERIALIZE_NESTED_SERIALIZERS_AS_ATTRIBUTE: + clazz = '{}.{}'.format(attrs['__module__'], attrs['__qualname__']) + if isinstance(field, ListSerializer): + nested_class = type(field.child).__name__ + else: + nested_class = type(field).__name__ + + warnings.warn(DeprecationWarning( + "Rendering nested serializer as relationship is deprecated. " + "Use `ResourceRelatedField` instead if {} in serializer {} should remain " + "a relationship. Otherwise set " + "JSON_API_SERIALIZE_NESTED_SERIALIZERS_AS_ATTRIBUTE to True to render nested " + "serializer as nested json attribute".format(nested_class, clazz))) + return fields + + +# If user imports serializer from here we can catch class definition and check +# nested serializers for depricated use. +class Serializer( + IncludedResourcesValidationMixin, SparseFieldsetsMixin, Serializer, + metaclass=SerializerMetaclass +): + pass + + class HyperlinkedModelSerializer( - IncludedResourcesValidationMixin, SparseFieldsetsMixin, HyperlinkedModelSerializer + IncludedResourcesValidationMixin, SparseFieldsetsMixin, HyperlinkedModelSerializer, + metaclass=SerializerMetaclass ): """ A type of `ModelSerializer` that uses hyperlinked relationships instead @@ -132,7 +169,8 @@ class HyperlinkedModelSerializer( """ -class ModelSerializer(IncludedResourcesValidationMixin, SparseFieldsetsMixin, ModelSerializer): +class ModelSerializer(IncludedResourcesValidationMixin, SparseFieldsetsMixin, ModelSerializer, + metaclass=SerializerMetaclass): """ A `ModelSerializer` is just a regular `Serializer`, except that: @@ -193,9 +231,11 @@ def to_representation(self, instance): def _get_field_representation(self, field, instance): request = self.context.get('request') is_included = field.source in get_included_resources(request) + render_nested_as_attribute = json_api_settings.SERIALIZE_NESTED_SERIALIZERS_AS_ATTRIBUTE if not is_included and \ isinstance(field, ModelSerializer) and \ - hasattr(instance, field.source + '_id'): + hasattr(instance, field.source + '_id') and \ + not render_nested_as_attribute: attribute = getattr(instance, field.source + '_id') if attribute is None: diff --git a/rest_framework_json_api/settings.py b/rest_framework_json_api/settings.py index 1385630c..74e4e8d3 100644 --- a/rest_framework_json_api/settings.py +++ b/rest_framework_json_api/settings.py @@ -14,6 +14,7 @@ 'FORMAT_TYPES': False, 'PLURALIZE_TYPES': False, 'UNIFORM_EXCEPTIONS': False, + 'SERIALIZE_NESTED_SERIALIZERS_AS_ATTRIBUTE': False } diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index b3932651..c626d1cc 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -312,29 +312,25 @@ def format_drf_errors(response, context, exc): # handle generic errors. ValidationError('test') in a view for example if isinstance(response.data, list): for message in response.data: - errors.append(format_error_object(message, '/data', response)) + errors.extend(format_error_object(message, '/data', response)) # handle all errors thrown from serializers else: for field, error in response.data.items(): field = format_value(field) pointer = '/data/attributes/{}'.format(field) - # see if they passed a dictionary to ValidationError manually - if isinstance(error, dict): - errors.append(error) - elif isinstance(exc, Http404) and isinstance(error, str): + if isinstance(exc, Http404) and isinstance(error, str): # 404 errors don't have a pointer - errors.append(format_error_object(error, None, response)) + errors.extend(format_error_object(error, None, response)) elif isinstance(error, str): classes = inspect.getmembers(exceptions, inspect.isclass) # DRF sets the `field` to 'detail' for its own exceptions if isinstance(exc, tuple(x[1] for x in classes)): pointer = '/data' - errors.append(format_error_object(error, pointer, response)) + errors.extend(format_error_object(error, pointer, response)) elif isinstance(error, list): - for message in error: - errors.append(format_error_object(message, pointer, response)) + errors.extend(format_error_object(error, pointer, response)) else: - errors.append(format_error_object(error, pointer, response)) + errors.extend(format_error_object(error, pointer, response)) context['view'].resource_name = 'errors' response.data = errors @@ -343,18 +339,42 @@ def format_drf_errors(response, context, exc): def format_error_object(message, pointer, response): - error_obj = { - 'detail': message, - 'status': encoding.force_str(response.status_code), - } - if pointer is not None: - error_obj['source'] = { - 'pointer': pointer, + errors = [] + if isinstance(message, dict): + links = message.pop('links', None) + source = message.pop('source', None) + is_custom_error = all([isinstance(x, str) for x in message.values()]) + if links is not None: + message['links'] = links + if source is not None: + message['source'] = source + if is_custom_error: + errors.append(message) + else: + for k, v in message.items(): + errors.extend(format_error_object(v, pointer + '/{}'.format(k), response)) + elif isinstance(message, list): + for num, error in enumerate(message): + if isinstance(error, (list, dict)): + new_pointer = pointer + '/{}'.format(num) + else: + new_pointer = pointer + if error: + errors.extend(format_error_object(error, new_pointer, response)) + else: + error_obj = { + 'detail': message, + 'status': encoding.force_str(response.status_code), } - code = getattr(message, "code", None) - if code is not None: - error_obj['code'] = code - return error_obj + if pointer is not None: + error_obj['source'] = { + 'pointer': pointer, + } + code = getattr(message, "code", None) + if code is not None: + error_obj['code'] = code + errors.append(error_obj) + return errors def format_errors(data):