Replace HttpResponse with Resource.render calls

This ensures responses are processed by the resource and consistently built.
Messages have been added to all responses in the 4xx-5xx range.

Fix #166, #167

Signed-off-by: Byron Ruth <b@devel.io>

requirements.txt

@@ -1,5 +1,5 @@
 git+git://github.com/cbmi/avocado.git
-django-preserialize>=1.0.4,<1.1.0
-restlib2>=0.3.9,<0.4
+django-preserialize>=1.0.6,<1.1.0
+restlib2>=0.4.1,<0.5.0
 python-memcached>=1.48
 django-objectset>=0.2.3

serrano/resources/__init__.py

@@ -1,5 +1,4 @@
 from urlparse import urlparse
-from django.http import HttpResponse
 from django.utils.http import is_safe_url
 from django.conf import settings as django_settings
 from django.conf.urls import patterns, url
@@ -76,15 +75,19 @@ def get(self, request):
     def post(self, request):
         username = request.data.get('username')
         password = request.data.get('password')
+
         if username and password:
             user = authenticate(username=username, password=password)
+
             if user:
                 login(request, user)
                 token = token_generator.make(user)
                 data = self.get(request)
                 data['token'] = token
                 return data
-        return HttpResponse('Invalid credentials', status=401)
+
+        return self.render(request, {'message': 'Invalid credentials'},
+                           status=codes.unauthorized)
 
 
 class Ping(Resource):

serrano/resources/concept.py

@@ -2,7 +2,6 @@
 import functools
 from django.conf.urls import patterns, url
 from django.core.urlresolvers import reverse
-from django.http import HttpResponse
 from preserialize.serialize import serialize
 from restlib2.http import codes
 from restlib2.params import Parametizer, BoolParam, StrParam, IntParam
@@ -164,10 +163,11 @@ def get(self, request, pk):
 
         if (self.checks_for_orphans and params['embed'] and
                 has_orphaned_field(instance)):
-            return HttpResponse(
-                status=codes.internal_server_error,
-                content="Could not get concept because it has one or more "
-                        "orphaned fields.")
+            data = {
+                'message': 'One or more orphaned fields exist'
+            }
+            return self.render(request, data,
+                               status=codes.internal_server_error)
 
         usage.log('read', instance=instance, request=request)
         return self.prepare(request, instance, embed=params['embed'])
@@ -183,10 +183,11 @@ def prepare(self, request, instance, template=None, **params):
         resource = FieldResource()
 
         if self.checks_for_orphans and has_orphaned_field(instance):
-            return HttpResponse(
-                status=codes.internal_server_error,
-                content="Could not get concept fields because one or more are "
-                        "linked to orphaned fields.")
+            data = {
+                'message': 'One or more orphaned fields exist'
+            }
+            return self.render(request, data,
+                               status=codes.internal_server_error)
 
         for cf in instance.concept_fields.select_related('field').iterator():
             field = resource.prepare(request, cf.field)

serrano/resources/context.py

@@ -1,7 +1,6 @@
 import functools
 import logging
 from datetime import datetime
-from django.http import HttpResponse
 from django.conf.urls import patterns, url
 from django.core.urlresolvers import reverse
 from django.views.decorators.cache import never_cache
@@ -128,7 +127,11 @@ def post(self, request):
             response = self.render(request, self.prepare(request, instance),
                                    status=codes.created)
         else:
-            response = self.render(request, dict(form.errors),
+            data = {
+                'message': 'Error creating context',
+                'errors': dict(form.errors),
+            }
+            response = self.render(request, data,
                                    status=codes.unprocessable_entity)
         return response
 
@@ -159,7 +162,11 @@ def put(self, request, **kwargs):
             usage.log('update', instance=instance, request=request)
             response = self.render(request, self.prepare(request, instance))
         else:
-            response = self.render(request, dict(form.errors),
+            data = {
+                'message': 'Error updating context',
+                'errors': dict(form.errors),
+            }
+            response = self.render(request, data,
                                    status=codes.unprocessable_entity)
         return response
 
@@ -168,11 +175,11 @@ def delete(self, request, **kwargs):
 
         # Cannot delete the current session
         if instance.session:
-            return HttpResponse(status=codes.bad_request)
+            data = {'error': 'Cannot delete session context'}
+            return self.render(request, data, status=codes.bad_request)
 
         instance.delete()
         usage.log('delete', instance=instance, request=request)
-        return HttpResponse(status=codes.no_content)
 
 
 class ContextStatsResource(ContextBase):

serrano/resources/field/base.py

@@ -1,6 +1,5 @@
 import functools
 import logging
-from django.http import HttpResponse
 from django.core.urlresolvers import reverse
 from preserialize.serialize import serialize
 from restlib2.http import codes
@@ -126,9 +125,11 @@ def get(self, request, pk):
 
         # If the field is an orphan then log an error before returning an error
         if self.checks_for_orphans and is_field_orphaned(instance):
-            return HttpResponse(
-                status=codes.internal_server_error,
-                content="Error occurred when retrieving orphaned field")
+            data = {
+                'message': 'Orphaned field',
+            }
+            return self.render(request, data,
+                               status=codes.internal_server_error)
 
         return self.prepare(request, instance)
 

serrano/resources/field/dist.py

@@ -1,7 +1,5 @@
-import json
 from decimal import Decimal
 from django.db.models import Q
-from django.http import HttpResponse
 from restlib2.http import codes
 from restlib2.params import Parametizer, StrParam, BoolParam, IntParam
 from modeltree.tree import MODELTREE_DEFAULT_ALIAS, trees
@@ -106,8 +104,11 @@ def get(self, request, pk):
             return resp
 
         if length > MAXIMUM_OBSERVATIONS:
-            return HttpResponse(json.dumps({'error': 'Data too large'}),
-                                status=codes.unprocessable_entity)
+            data = {
+                'message': 'Data too large',
+            }
+            return self.render(request, data,
+                               status=codes.unprocessable_entity)
 
         # Apply ordering. If any of the fields are enumerable, ordering should
         # be relative to those fields. For continuous data, the ordering is

serrano/resources/field/values.py

@@ -124,7 +124,10 @@ def post(self, request, pk):
         params = self.get_params(request)
 
         if not request.data:
-            return self.render(request, 'Error parsing data',
+            data = {
+                'message': 'Error parsing data',
+            }
+            return self.render(request, data,
                                status=codes.unprocessable_entity)
 
         if isinstance(request.data, dict):
@@ -147,7 +150,10 @@ def post(self, request, pk):
                 array_map[i] = 'label'
                 labels.append(datum['label'])
             else:
-                return self.render(request, 'Error parsing value or label',
+                data = {
+                    'message': 'Error parsing value or lable'
+                }
+                return self.render(request, data,
                                    status=codes.unprocessable_entity)
 
         field_name = instance.field_name

serrano/resources/query.py

@@ -1,7 +1,6 @@
 import functools
 import logging
 from datetime import datetime
-from django.http import HttpResponse
 from django.conf.urls import patterns, url
 from django.core.urlresolvers import reverse
 from django.db.models import Q
@@ -146,7 +145,11 @@ def post(self, request):
             response = self.render(request, self.prepare(request, instance),
                                    status=codes.created)
         else:
-            response = self.render(request, dict(form.errors),
+            data = {
+                'message': 'Error creating query',
+                'errors': dict(form.errors),
+            }
+            response = self.render(request, data,
                                    status=codes.unprocessable_entity)
         return response
 
@@ -217,7 +220,10 @@ def get(self, request, **kwargs):
         if self._requestor_can_get_forks(request, instance):
             return self.prepare(request, self.get_queryset(request, **kwargs))
 
-        return HttpResponse(status=codes.unauthorized)
+        data = {
+            'message': 'Cannot access forks',
+        }
+        return self.render(request, data, status=codes.unauthorized)
 
     def post(self, request, **kwargs):
         instance = self.get_object(request, **kwargs)
@@ -241,7 +247,10 @@ def post(self, request, **kwargs):
 
             return self.render(request, data, status=codes.created)
 
-        return HttpResponse(status=codes.unauthorized)
+        data = {
+            'message': 'Cannot fork query',
+        }
+        return self.render(request, data, status=codes.unauthorized)
 
 
 class PublicQueriesResource(QueryBase):
@@ -290,15 +299,22 @@ def put(self, request, **kwargs):
             usage.log('update', instance=instance, request=request)
             response = self.render(request, self.prepare(request, instance))
         else:
-            response = self.render(request, dict(form.errors),
+            data = {
+                'message': 'Cannot update query',
+                'errors': dict(form.errors),
+            }
+            response = self.render(request, data,
                                    status=codes.unprocessable_entity)
         return response
 
     def delete(self, request, **kwargs):
         instance = self.get_object(request, **kwargs)
 
         if instance.session:
-            return HttpResponse(status=codes.bad_request)
+            data = {
+                'message': 'Cannot delete session query',
+            }
+            return self.render(request, data, status=codes.bad_request)
 
         utils.send_mail(instance.shared_users.values_list('email', flat=True),
                         DELETE_QUERY_EMAIL_TITLE.format(instance.name),
@@ -307,8 +323,6 @@ def delete(self, request, **kwargs):
         instance.delete()
         usage.log('delete', instance=instance, request=request)
 
-        return HttpResponse(status=codes.no_content)
-
 
 class QueryStatsResource(QueryBase):
     def is_not_found(self, request, response, **kwargs):

serrano/resources/view.py

@@ -1,7 +1,6 @@
 import functools
 import logging
 from datetime import datetime
-from django.http import HttpResponse
 from django.conf.urls import patterns, url
 from django.core.urlresolvers import reverse
 from django.views.decorators.cache import never_cache
@@ -97,7 +96,11 @@ def post(self, request):
             response = self.render(request, self.prepare(request, instance),
                                    status=codes.created)
         else:
-            response = self.render(request, dict(form.errors),
+            data = {
+                'message': 'Cannot create view',
+                'errors': dict(form.errors),
+            }
+            response = self.render(request, data,
                                    status=codes.unprocessable_entity)
         return response
 
@@ -145,19 +148,25 @@ def put(self, request, **kwargs):
             usage.log('update', instance=instance, request=request)
             response = self.render(request, self.prepare(request, instance))
         else:
-            response = self.render(request, dict(form.errors),
+            data = {
+                'message': 'Cannot update view',
+                'errors': dict(form.errors),
+            }
+            response = self.render(request, data,
                                    status=codes.unprocessable_entity)
         return response
 
     def delete(self, request, **kwargs):
         instance = self.get_object(request, **kwargs)
 
         if instance.session:
-            return HttpResponse(status=codes.bad_request)
+            data = {
+                'message': 'Cannot delete session view',
+            }
+            return self.render(request, data, status=codes.bad_request)
 
         instance.delete()
         usage.log('delete', instance=instance, request=request)
-        return HttpResponse(status=codes.no_content)
 
 
 single_resource = never_cache(ViewResource())

setup.py

@@ -3,8 +3,8 @@
 
 install_requires = [
     'avocado>=2.3,<2.4',
-    'restlib2>=0.3.9,<0.4',
-    'django-preserialize>=1.0.4,<1.1',
+    'restlib2>=0.4.1,<0.5.0',
+    'django-preserialize>=1.0.6,<1.1',
 ]
 
 if sys.version_info < (2, 7):

tests/cases/resources/tests/base.py

@@ -64,7 +64,6 @@ def test_post(self):
             content_type='application/json',
             HTTP_ACCEPT='application/json')
         self.assertEqual(response.status_code, codes.unauthorized)
-        self.assertEqual(response.content, 'Invalid credentials')
 
         response = self.client.post('/api/',
             json.dumps({'username': 'root', 'password': 'password'}),

tests/cases/resources/tests/context.py

@@ -80,7 +80,7 @@ def test_put(self):
         # Attempt to update the name via a PUT request
         response = self.client.put('/api/contexts/1/',
             data=u'{"name":"New Name"}', content_type='application/json')
-        self.assertEqual(response.status_code, codes.no_content)
+        self.assertEqual(response.status_code, codes.ok)
 
         # Make sure our changes from the PUT request are persisted
         response = self.client.get('/api/contexts/1/',

tests/cases/resources/tests/query.py

@@ -370,7 +370,7 @@ def test_put(self):
         # Attempt to update the name via a PUT request
         response = self.client.put('/api/queries/1/',
             data=u'{"name":"New Name"}', content_type='application/json')
-        self.assertEqual(response.status_code, codes.no_content)
+        self.assertEqual(response.status_code, codes.ok)
 
         # Make sure our changes from the PUT request are persisted
         response = self.client.get('/api/queries/1/',

tests/cases/resources/tests/view.py

@@ -84,7 +84,7 @@ def test_put(self):
         # Attempt to update the name via a PUT request
         response = self.client.put('/api/views/1/',
             data=u'{"name":"New Name"}', content_type='application/json')
-        self.assertEqual(response.status_code, codes.no_content)
+        self.assertEqual(response.status_code, codes.ok)
 
         # Make sure our changes from the PUT request are persisted
         response = self.client.get('/api/views/1/',