Merge pull request #44 from django-json-api/feature/errors

jsenecal · jsenecal · commit 93406e3e1921 · 2015-07-24T23:38:22.000-04:00
Added error formating and fixed the tests
diff --git a/.travis.yml b/.travis.yml
@@ -2,6 +2,8 @@ language: python
 sudo: false
 python:
   - "2.7"
+  - "3.3"
+  - "3.4"
 install:
   - pip install -e .
 script: python runtests.py
diff --git a/README.rst b/README.rst
@@ -155,7 +155,7 @@ requests and responses from the python/rest_framework's preferred underscore to
 a format of your choice. To hook this up include the following in your project
 settings::
 
-   JSON_API_FORMAT_KEYS = True
+   JSON_API_FORMAT_KEYS = 'dasherize'
 
 Note: due to the way the inflector works address_1 can camelize to address1
 on output but it cannot convert address1 back to address_1 on POST or PUT. Keep
diff --git a/example/api/resources/identity.py b/example/api/resources/identity.py
@@ -1,5 +1,5 @@
 from django.contrib.auth import models as auth_models
-from rest_framework import viewsets, generics, renderers, parsers
+from rest_framework import viewsets, generics, renderers, parsers, serializers
 from rest_framework.decorators import list_route, detail_route
 from rest_framework.response import Response
 from rest_framework_json_api import mixins, utils
@@ -41,6 +41,10 @@ def manual_resource_name(self, request, *args, **kwargs):
         self.resource_name = 'data'
         return super(Identity, self).retrieve(request, args, kwargs)
 
+    @detail_route()
+    def validation(self, request, *args, **kwargs):
+        raise serializers.ValidationError('Oh nohs!')
+
 
 class GenericIdentity(generics.GenericAPIView):
     """
@@ -63,4 +67,3 @@ def get(self, request, pk=None):
         """
         obj = self.get_object()
         return Response(IdentitySerializer(obj).data)
-
diff --git a/example/api/serializers/identity.py b/example/api/serializers/identity.py
@@ -6,8 +6,24 @@ class IdentitySerializer(serializers.ModelSerializer):
     """
     Identity Serializer
     """
+    def validate_first_name(self, data):
+        if len(data) > 10:
+            raise serializers.ValidationError(
+                    'There\'s a problem with first name')
+        return data
+
+    def validate_last_name(self, data):
+        if len(data) > 10:
+            raise serializers.ValidationError(
+                {
+                    'id': 'armageddon101',
+                    'detail': 'Hey! You need a last name!',
+                    'meta': 'something',
+                }
+            )
+        return data
+
     class Meta:
         model = auth_models.User
         fields = (
             'id', 'first_name', 'last_name', 'email', )
-
diff --git a/example/settings.py b/example/settings.py
@@ -38,6 +38,7 @@
     'PAGINATE_BY': 1,
     'PAGINATE_BY_PARAM': 'page_size',
     'MAX_PAGINATE_BY': 100,
+    'EXCEPTION_HANDLER': 'rest_framework_json_api.exceptions.exception_handler',
     # DRF v3.1+
     'DEFAULT_PAGINATION_CLASS':
         'rest_framework_json_api.pagination.PageNumberPagination',
diff --git a/example/tests/test_format_keys.py b/example/tests/test_format_keys.py
@@ -18,11 +18,11 @@ def setUp(self):
         self.detail_url = reverse('user-detail', kwargs={'pk': self.miles.pk})
 
         # Set the format keys settings.
-        setattr(settings, 'JSON_API_FORMAT_KEYS', True)
+        setattr(settings, 'JSON_API_FORMAT_KEYS', 'camelization')
 
     def tearDown(self):
         # Remove the format keys settings.
-        delattr(settings, 'JSON_API_FORMAT_KEYS')
+        setattr(settings, 'JSON_API_FORMAT_KEYS', 'dasherize')
 
 
     def test_camelization(self):
diff --git a/example/tests/test_generic_validation.py b/example/tests/test_generic_validation.py
@@ -0,0 +1,33 @@
+import json
+from example.tests import TestBase
+from django.core.urlresolvers import reverse
+from django.conf import settings
+from rest_framework.serializers import ValidationError
+
+
+class GenericValidationTest(TestBase):
+    """
+    Test that a non serializer specific validation can be thrown and formatted
+    """
+    def setUp(self):
+        super(GenericValidationTest, self).setUp()
+        self.url = reverse('user-validation', kwargs={'pk': self.miles.pk})
+
+    def test_generic_validation_error(self):
+        """
+        Check error formatting
+        """
+        response = self.client.get(self.url)
+        self.assertEqual(response.status_code, 400)
+
+        result = json.loads(response.content.decode('utf8'))
+        expected = {
+            'errors': [{
+                'status': '400',
+                'source': {
+                    'pointer': '/data'
+                },
+                'detail': 'Oh nohs!'
+            }]
+        }
+        self.assertEqual(result, expected)
diff --git a/example/tests/test_generic_viewset.py b/example/tests/test_generic_viewset.py
@@ -40,12 +40,63 @@ def test_ember_expected_renderer(self):
             json.loads(response.content.decode('utf8')),
             {
                 'data': {
-                    'id': 2,
-                    'first_name': u'Miles',
-                    'last_name': u'Davis',
-                    'email': u'miles@example.com'
+                    'type': 'data',
+                    'id': '2',
+                    'attributes': {
+                        'first-name': u'Miles',
+                        'last-name': u'Davis',
+                        'email': u'miles@example.com'
+                    }
                 }
             }
         )
 
+    def test_default_validation_exceptions(self):
+        """
+        Default validation exceptions should conform to json api spec
+        """
+        expected = {
+            'errors': [
+                {
+                    'status': '400',
+                    'source': {
+                        'pointer': '/data/attributes/email',
+                    },
+                    'detail': 'Enter a valid email address.',
+                },
+                {
+                    'status': '400',
+                    'source': {
+                        'pointer': '/data/attributes/first-name',
+                    },
+                    'detail': 'There\'s a problem with first name',
+                }
+            ]
+        }
+        response = self.client.post('/identities', {
+            'email': 'bar', 'first_name': 'alajflajaljalajlfjafljalj'})
+        self.assertEqual(json.loads(response.content.decode('utf8')), expected)
 
+    def test_custom_validation_exceptions(self):
+        """
+        Exceptions should be able to be formatted manually
+        """
+        expected = {
+            'errors': [
+                {
+                    'id': 'armageddon101',
+                    'detail': 'Hey! You need a last name!',
+                    'meta': 'something',
+                },
+                {
+                    'status': '400',
+                    'source': {
+                        'pointer': '/data/attributes/email',
+                    },
+                    'detail': 'Enter a valid email address.',
+                },
+            ]
+        }
+        response = self.client.post('/identities', {
+            'email': 'bar', 'last_name': 'alajflajaljalajlfjafljalj'})
+        self.assertEqual(json.loads(response.content.decode('utf8')), expected)
diff --git a/example/tests/test_multiple_id_mixin.py b/example/tests/test_multiple_id_mixin.py
@@ -1,5 +1,6 @@
 import json
 from example.tests import TestBase
+from django.utils import encoding
 from django.contrib.auth import get_user_model
 from django.core.urlresolvers import reverse
 from django.conf import settings
@@ -23,21 +24,24 @@ def test_single_id_in_query_params(self):
         self.assertEqual(response.status_code, 200)
 
         expected = {
-            'user': [{
-                'id': self.miles.pk,
-                'first_name': self.miles.first_name,
-                'last_name': self.miles.last_name,
-                'email': self.miles.email
-            }]
+            'data': {
+                'type': 'users',
+                'id': encoding.force_text(self.miles.pk),
+                'attributes': {
+                    'first_name': self.miles.first_name,
+                    'last_name': self.miles.last_name,
+                    'email': self.miles.email
+                }
+            }
         }
 
         json_content = json.loads(response.content.decode('utf8'))
-        meta = json_content.get("meta")
+        links = json_content.get("links")
+        meta = json_content.get("meta").get('pagination')
 
         self.assertEquals(expected.get('user'), json_content.get('user'))
         self.assertEquals(meta.get('count', 0), 1)
-        self.assertEquals(meta.get("next"), None)
-        self.assertEqual(None, meta.get("next_link"))
+        self.assertEquals(links.get("next"), None)
         self.assertEqual(meta.get("page"), 1)
 
     def test_multiple_ids_in_query_params(self):
@@ -50,28 +54,29 @@ def test_multiple_ids_in_query_params(self):
         self.assertEqual(response.status_code, 200)
 
         expected = {
-            'user': [{
-                'id': self.john.pk,
-                'first_name': self.john.first_name,
-                'last_name': self.john.last_name,
-                'email': self.john.email
-            }]
+            'data': {
+                'type': 'users',
+                'id': encoding.force_text(self.john.pk),
+                'attributes': {
+                    'first_name': self.john.first_name,
+                    'last_name': self.john.last_name,
+                    'email': self.john.email
+                }
+            }
         }
 
         json_content = json.loads(response.content.decode('utf8'))
-        meta = json_content.get("meta")
+        links = json_content.get("links")
+        meta = json_content.get("meta").get('pagination')
 
         self.assertEquals(expected.get('user'), json_content.get('user'))
         self.assertEquals(meta.get('count', 0), 2)
-        self.assertEquals(meta.get("next"), 2)
         self.assertEqual(
             sorted(
                 'http://testserver/identities?ids%5B%5D=2&ids%5B%5D=1&page=2'\
                 .split('?')[1].split('&')
             ),
             sorted(
-                meta.get("next_link").split('?')[1].split('&'))
+                links.get("next").split('?')[1].split('&'))
             )
         self.assertEqual(meta.get("page"), 1)
-
-
diff --git a/rest_framework_json_api/exceptions.py b/rest_framework_json_api/exceptions.py
@@ -0,0 +1,41 @@
+
+from django.utils import six, encoding
+from rest_framework.views import exception_handler as drf_exception_handler
+from rest_framework_json_api.utils import format_value
+
+
+def exception_handler(exc, context):
+    response = drf_exception_handler(exc, context)
+
+    errors = []
+    # handle generic errors. ValidationError('test') in a view for example
+    if isinstance(response.data, list):
+        for message in response.data:
+            errors.append({
+                'detail': message,
+                'source': {
+                    'pointer': '/data',
+                },
+                'status': encoding.force_text(response.status_code),
+            })
+    # 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)
+            else:
+                for message in error:
+                    errors.append({
+                        'detail': message,
+                        'source': {
+                            'pointer': pointer,
+                        },
+                        'status': encoding.force_text(response.status_code),
+                    })
+
+    context['view'].resource_name = 'errors'
+    response.data = errors
+    return response
diff --git a/rest_framework_json_api/renderers.py b/rest_framework_json_api/renderers.py
@@ -43,9 +43,10 @@ def render(self, data, accepted_media_type=None, renderer_context=None):
                 data, accepted_media_type, renderer_context
             )
 
-        # @TODO format errors correctly
         # If this is an error response, skip the rest.
         if resource_name == 'errors':
+            if len(data) > 1:
+                data.sort(key=lambda x: x.get('source', {}).get('pointer', ''))
             return super(JSONRenderer, self).render(
                 {resource_name: data}, accepted_media_type, renderer_context
             )
diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py
@@ -67,13 +67,7 @@ def get_resource_name(context):
 
             resource_name = inflection.pluralize(resource_name.lower())
 
-            format_type = getattr(settings, 'JSON_API_FORMAT_KEYS', False)
-            if format_type == 'dasherize':
-                resource_name = inflection.dasherize(resource_name)
-            elif format_type == 'camelize':
-                resource_name = inflection.camelize(resource_name)
-            elif format_type == 'underscore':
-                resource_name = inflection.underscore(resource_name)
+            resource_name = format_value(resource_name)
 
     return resource_name
 
@@ -118,6 +112,17 @@ def format_keys(obj, format_type=None):
         return obj
 
 
+def format_value(value, format_type=None):
+    format_type = getattr(settings, 'JSON_API_FORMAT_KEYS', False)
+    if format_type == 'dasherize':
+        value = inflection.dasherize(value)
+    elif format_type == 'camelize':
+        value = inflection.camelize(value)
+    elif format_type == 'underscore':
+        value = inflection.underscore(value)
+    return value
+
+
 def build_json_resource_obj(fields, resource, resource_name):
     resource_data = [
         ('type', resource_name),
