Merge pull request #61 from xsnippet/rest-resource

Make Resource class more "RESTful"

tests/test_resource.py

@@ -19,11 +19,11 @@
 class _TestResource(resource.Resource):
 
     async def get(self):
-        return self.make_response({'who': 'batman'}, status=299)
+        return {'who': 'batman'}, 299
 
     async def post(self):
-        data = await self.read_request()
-        return self.make_response(data, status=298)
+        data = await self.request.get_data()
+        return data, 298
 
 
 @pytest.fixture(scope='function')

xsnippet/api/resource.py

@@ -10,6 +10,7 @@
     :license: MIT, see LICENSE for details
 """
 
+import asyncio
 import json
 import datetime
 import fnmatch
@@ -18,6 +19,8 @@
 from aiohttp import web, hdrs
 from werkzeug import http
 
+from . import exceptions
+
 
 class _JSONEncoder(json.JSONEncoder):
     """Advanced JSON encoder for extra data types.
@@ -35,7 +38,7 @@ def default(self, instance):
 
 
 class Resource(web.View):
-    """Resource wrapper around :class:`aiohttp.web.View`.
+    """Class-based view to represent RESTful resource.
 
     The class provides basic facilities for building RESTful API, such as
     encoding responses according to ``Accept`` and decoding requests
@@ -58,7 +61,37 @@ def __init__(self, *args, **kwargs):
         #: an application database alias to make code a bit readable
         self.db = self.request.app['db']
 
-    def make_response(self, data, headers=None, status=200):
+    @asyncio.coroutine
+    def __iter__(self):
+        # So far (Jan 5, 2018) aiohttp doesn't support custom request classes,
+        # but we would like to provide a better user experience for consumers
+        # of this middleware. Hence, we monkey patch the instance and add a new
+        # async method that would de-serialize income payload according to HTTP
+        # content negotiation rules.
+        setattr(self.request.__class__, 'get_data', self._get_data())
+
+        try:
+            response = yield from super(Resource, self).__iter__()
+
+        except exceptions.SnippetNotFound as exc:
+            error = {'message': str(exc)}
+            return self._make_response(error, None, 404)
+
+        except web.HTTPError as exc:
+            error = {'message': str(exc)}
+            return self._make_response(error, None, exc.status_code)
+
+        status_code = 200
+        headers = None
+
+        if isinstance(response, tuple):
+            response, status_code, *rest = response
+            if rest:
+                headers = rest[0]
+
+        return self._make_response(response, headers, status_code)
+
+    def _make_response(self, data, headers=None, status=200):
         """Return an HTTP response object.
 
         The method serializes given data according to 'Accept' HTTP header,
@@ -109,19 +142,13 @@ def make_response(self, data, headers=None, status=200):
 
         raise web.HTTPNotAcceptable()
 
-    async def read_request(self):
-        """Return parsed request data.
-
-        The method reads request's payload and tries to deserialize it
-        according to 'Content-Type' header.
+    def _get_data(self):
+        decoders = self._decoders
 
-        :return: a deserialized content
-        :rtype: python object
-
-        :raise: :class:`aiohttp.web.HTTPUnsupportedMediaType`
-        """
-        if self.request.content_type in self._decoders:
-            decode = self._decoders[self.request.content_type]
-            return decode(await self.request.text())
+        async def impl(self):
+            if self.content_type in decoders:
+                decode = decoders[self.content_type]
+                return decode(await self.text())
 
-        raise web.HTTPUnsupportedMediaType()
+            raise web.HTTPUnsupportedMediaType()
+        return impl

xsnippet/api/resources/snippets.py

@@ -12,13 +12,10 @@
 import functools
 
 import cerberus
+import aiohttp.web as web
 
 from .misc import cerberus_errors_to_str, try_int
-from .. import exceptions, resource, services
-
-
-class InvalidId(Exception):
-    pass
+from .. import resource, services
 
 
 def _get_id(resource):
@@ -27,7 +24,8 @@ def _get_id(resource):
     })
 
     if not v.validate(dict(resource.request.match_info)):
-        raise InvalidId('%s.' % cerberus_errors_to_str(v.errors))
+        reason = '%s.' % cerberus_errors_to_str(v.errors)
+        raise web.HTTPBadRequest(reason=reason)
 
     return int(resource.request.match_info['id'])
 
@@ -44,7 +42,7 @@ async def _write(resource, service_fn, *, status):
         'updated_at': {'type': 'datetime', 'readonly': True},
     })
 
-    snippet = await resource.read_request()
+    snippet = await resource.request.get_data()
     conf = resource.request.app['conf']
     syntaxes = conf.getlist('snippet', 'syntaxes', fallback=None)
 
@@ -60,29 +58,15 @@ async def _write(resource, service_fn, *, status):
 
     if not v.validate(snippet, update=is_patch):
         error = cerberus_errors_to_str(v.errors)
-        return resource.make_response({'message': '%s.' % error}, status=400)
-
-    try:
-        written = await service_fn(snippet)
+        raise web.HTTPBadRequest(reason='%s.' % error)
 
-    except InvalidId as exc:
-        return resource.make_response({'message': str(exc)}, status=400)
-    except exceptions.SnippetNotFound as exc:
-        return resource.make_response({'message': str(exc)}, status=404)
-
-    return resource.make_response(written, status=status)
+    written = await service_fn(snippet)
+    return written, status
 
 
 async def _read(resource, service_fn, *, status):
-    try:
-        read = await service_fn(_get_id(resource))
-
-    except InvalidId as exc:
-        return resource.make_response({'message': str(exc)}, status=400)
-    except exceptions.SnippetNotFound as exc:
-        return resource.make_response({'message': str(exc)}, status=404)
-
-    return resource.make_response(read, status=status)
+    read = await service_fn(_get_id(resource))
+    return read, status
 
 
 class Snippet(resource.Resource):
@@ -98,11 +82,7 @@ async def _wrapper(self, *args, **kwargs):
             # functionality.
             conf = self.request.app['conf']
             if not conf.getboolean('test', 'sudo', fallback=False):
-                return self.make_response(
-                    {
-                        'message': 'Not yet. :)'
-                    },
-                    status=403)
+                raise web.HTTPForbidden(reason='Not yet. :)')
             return await fn(self, *args, **kwargs)
         return _wrapper
 
@@ -218,7 +198,7 @@ async def get(self):
 
         if not v.validate(dict(self.request.GET)):
             error = '%s.' % cerberus_errors_to_str(v.errors)
-            return self.make_response({'message': error}, status=400)
+            raise web.HTTPBadRequest(reason=error)
 
         # It's safe to have type cast here since those query parameters
         # are guaranteed to be integer, thanks to validation above.
@@ -228,41 +208,38 @@ async def get(self):
         tag = self.request.GET.get('tag')
         syntax = self.request.GET.get('syntax')
 
-        try:
-            # actual snippets to be returned
-            current_page = await services.Snippet(self.db).get(
+        # actual snippets to be returned
+        current_page = await services.Snippet(self.db).get(
+            title=title, tag=tag, syntax=syntax,
+            # read one more to know if there is next page
+            limit=limit + 1,
+            marker=marker,
+        )
+
+        # only needed to render a link to the previous page
+        if marker:
+            previous_page = await services.Snippet(self.db).get(
                 title=title, tag=tag, syntax=syntax,
-                # read one more to know if there is next page
+                # read one more to know if there is prev page
                 limit=limit + 1,
                 marker=marker,
+                direction='backward'
             )
+        else:
+            # we are at the very beginning of the list - no prev page
+            previous_page = []
 
-            # only needed to render a link to the previous page
-            if marker:
-                previous_page = await services.Snippet(self.db).get(
-                    title=title, tag=tag, syntax=syntax,
-                    # read one more to know if there is prev page
-                    limit=limit + 1,
-                    marker=marker,
-                    direction='backward'
-                )
-            else:
-                # we are at the very beginning of the list - no prev page
-                previous_page = []
-        except exceptions.SnippetNotFound as exc:
-            return self.make_response({'message': str(exc)}, status=404)
-
-        return self.make_response(
+        return (
             # return no more than $limit snippets (if we read $limit + 1)
             current_page[:limit],
-            headers={
+            200,
+            {
                 'Link': _build_link_header(
                     self.request,
                     current_page, previous_page,
                     limit=limit,
                 )
             },
-            status=200
         )
 
     async def post(self):

xsnippet/api/resources/syntaxes.py

@@ -16,6 +16,4 @@ class Syntaxes(resource.Resource):
 
     async def get(self):
         conf = self.request.app['conf']
-        syntaxes = conf.getlist('snippet', 'syntaxes', fallback=None) or []
-
-        return self.make_response(syntaxes, status=200)
+        return conf.getlist('snippet', 'syntaxes', fallback=[])