Combine the request wrappers in rest/media/v1 and http/server into a single wrapper decorator

4 files changed, 62 insertions, 131 deletions

diff --git a/synapse/rest/media/v1/base_resource.py b/synapse/rest/media/v1/base_resource.py
index b10cbddb81..fc078fca82 100644
--- a/synapse/rest/media/v1/base_resource.py
+++ b/synapse/rest/media/v1/base_resource.py
@@ -18,7 +18,7 @@ from .thumbnailer import Thumbnailer
 from synapse.http.server import respond_with_json
 from synapse.util.stringutils import random_string
 from synapse.api.errors import (
-    cs_exception, CodeMessageException, cs_error, Codes, SynapseError
+    cs_error, Codes, SynapseError
 )
 
 from twisted.internet import defer
@@ -32,6 +32,18 @@ import logging
 logger = logging.getLogger(__name__)
 
 
+def parse_media_id(request):
+    try:
+        server_name, media_id = request.postpath
+        return (server_name, media_id)
+    except:
+        raise SynapseError(
+            404,
+            "Invalid media id token %r" % (request.postpath,),
+            Codes.UNKNOWN,
+        )
+
+
 class BaseMediaResource(Resource):
     isLeaf = True
 
@@ -47,72 +59,6 @@ class BaseMediaResource(Resource):
         self.filepaths = filepaths
         self.downloads = {}
 
-    @staticmethod
-    def catch_errors(request_handler):
-        @defer.inlineCallbacks
-        def wrapped_request_handler(self, request):
-            try:
-                yield request_handler(self, request)
-            except CodeMessageException as e:
-                logger.info("Responding with error: %r", e)
-                respond_with_json(
-                    request, e.code, cs_exception(e), send_cors=True
-                )
-            except:
-                logger.exception(
-                    "Failed handle request %s.%s on %r",
-                    request_handler.__module__,
-                    request_handler.__name__,
-                    self,
-                )
-                respond_with_json(
-                    request,
-                    500,
-                    {"error": "Internal server error"},
-                    send_cors=True
-                )
-        return wrapped_request_handler
-
-    @staticmethod
-    def _parse_media_id(request):
-        try:
-            server_name, media_id = request.postpath
-            return (server_name, media_id)
-        except:
-            raise SynapseError(
-                404,
-                "Invalid media id token %r" % (request.postpath,),
-                Codes.UNKNOWN,
-            )
-
-    @staticmethod
-    def _parse_integer(request, arg_name, default=None):
-        try:
-            if default is None:
-                return int(request.args[arg_name][0])
-            else:
-                return int(request.args.get(arg_name, [default])[0])
-        except:
-            raise SynapseError(
-                400,
-                "Missing integer argument %r" % (arg_name,),
-                Codes.UNKNOWN,
-            )
-
-    @staticmethod
-    def _parse_string(request, arg_name, default=None):
-        try:
-            if default is None:
-                return request.args[arg_name][0]
-            else:
-                return request.args.get(arg_name, [default])[0]
-        except:
-            raise SynapseError(
-                400,
-                "Missing string argument %r" % (arg_name,),
-                Codes.UNKNOWN,
-            )
-
     def _respond_404(self, request):
         respond_with_json(
             request, 404,
diff --git a/synapse/rest/media/v1/download_resource.py b/synapse/rest/media/v1/download_resource.py
index c585bb11f7..0fe6abf647 100644
--- a/synapse/rest/media/v1/download_resource.py
+++ b/synapse/rest/media/v1/download_resource.py
@@ -13,7 +13,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from .base_resource import BaseMediaResource
+from .base_resource import BaseMediaResource, parse_media_id
+from synapse.http.server import request_handler
 
 from twisted.web.server import NOT_DONE_YET
 from twisted.internet import defer
@@ -28,15 +29,10 @@ class DownloadResource(BaseMediaResource):
         self._async_render_GET(request)
         return NOT_DONE_YET
 
-    @BaseMediaResource.catch_errors
+    @request_handler
     @defer.inlineCallbacks
     def _async_render_GET(self, request):
-        try:
-            server_name, media_id = request.postpath
-        except:
-            self._respond_404(request)
-            return
-
+        server_name, media_id = parse_media_id(request)
         if server_name == self.server_name:
             yield self._respond_local_file(request, media_id)
         else:
diff --git a/synapse/rest/media/v1/thumbnail_resource.py b/synapse/rest/media/v1/thumbnail_resource.py
index 84f5e3463c..1dadd880b2 100644
--- a/synapse/rest/media/v1/thumbnail_resource.py
+++ b/synapse/rest/media/v1/thumbnail_resource.py
@@ -14,7 +14,9 @@
 # limitations under the License.
 
 
-from .base_resource import BaseMediaResource
+from .base_resource import BaseMediaResource, parse_media_id
+from synapse.http.servlet import parse_string, parse_integer
+from synapse.http.server import request_handler
 
 from twisted.web.server import NOT_DONE_YET
 from twisted.internet import defer
@@ -31,14 +33,14 @@ class ThumbnailResource(BaseMediaResource):
         self._async_render_GET(request)
         return NOT_DONE_YET
 
-    @BaseMediaResource.catch_errors
+    @request_handler
     @defer.inlineCallbacks
     def _async_render_GET(self, request):
-        server_name, media_id = self._parse_media_id(request)
-        width = self._parse_integer(request, "width")
-        height = self._parse_integer(request, "height")
-        method = self._parse_string(request, "method", "scale")
-        m_type = self._parse_string(request, "type", "image/png")
+        server_name, media_id = parse_media_id(request)
+        width = parse_integer(request, "width")
+        height = parse_integer(request, "height")
+        method = parse_string(request, "method", "scale")
+        m_type = parse_string(request, "type", "image/png")
 
         if server_name == self.server_name:
             yield self._respond_local_thumbnail(
diff --git a/synapse/rest/media/v1/upload_resource.py b/synapse/rest/media/v1/upload_resource.py
index e5aba3af4c..cc571976a5 100644
--- a/synapse/rest/media/v1/upload_resource.py
+++ b/synapse/rest/media/v1/upload_resource.py
@@ -13,12 +13,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from synapse.http.server import respond_with_json
+from synapse.http.server import respond_with_json, request_handler
 
 from synapse.util.stringutils import random_string
-from synapse.api.errors import (
-    cs_exception, SynapseError, CodeMessageException
-)
+from synapse.api.errors import SynapseError
 
 from twisted.web.server import NOT_DONE_YET
 from twisted.internet import defer
@@ -69,53 +67,42 @@ class UploadResource(BaseMediaResource):
 
         defer.returnValue("mxc://%s/%s" % (self.server_name, media_id))
 
+    @request_handler
     @defer.inlineCallbacks
     def _async_render_POST(self, request):
-        try:
-            auth_user, client = yield self.auth.get_user_by_req(request)
-            # TODO: The checks here are a bit late. The content will have
-            # already been uploaded to a tmp file at this point
-            content_length = request.getHeader("Content-Length")
-            if content_length is None:
-                raise SynapseError(
-                    msg="Request must specify a Content-Length", code=400
-                )
-            if int(content_length) > self.max_upload_size:
-                raise SynapseError(
-                    msg="Upload request body is too large",
-                    code=413,
-                )
-
-            headers = request.requestHeaders
-
-            if headers.hasHeader("Content-Type"):
-                media_type = headers.getRawHeaders("Content-Type")[0]
-            else:
-                raise SynapseError(
-                    msg="Upload request missing 'Content-Type'",
-                    code=400,
-                )
-
-            # if headers.hasHeader("Content-Disposition"):
-            #     disposition = headers.getRawHeaders("Content-Disposition")[0]
-            # TODO(markjh): parse content-dispostion
-
-            content_uri = yield self.create_content(
-                media_type, None, request.content.read(),
-                content_length, auth_user
+        auth_user, client = yield self.auth.get_user_by_req(request)
+        # TODO: The checks here are a bit late. The content will have
+        # already been uploaded to a tmp file at this point
+        content_length = request.getHeader("Content-Length")
+        if content_length is None:
+            raise SynapseError(
+                msg="Request must specify a Content-Length", code=400
             )
-
-            respond_with_json(
-                request, 200, {"content_uri": content_uri}, send_cors=True
+        if int(content_length) > self.max_upload_size:
+            raise SynapseError(
+                msg="Upload request body is too large",
+                code=413,
             )
-        except CodeMessageException as e:
-            logger.exception(e)
-            respond_with_json(request, e.code, cs_exception(e), send_cors=True)
-        except:
-            logger.exception("Failed to store file")
-            respond_with_json(
-                request,
-                500,
-                {"error": "Internal server error"},
-                send_cors=True
+
+        headers = request.requestHeaders
+
+        if headers.hasHeader("Content-Type"):
+            media_type = headers.getRawHeaders("Content-Type")[0]
+        else:
+            raise SynapseError(
+                msg="Upload request missing 'Content-Type'",
+                code=400,
             )
+
+        # if headers.hasHeader("Content-Disposition"):
+        #     disposition = headers.getRawHeaders("Content-Disposition")[0]
+        # TODO(markjh): parse content-dispostion
+
+        content_uri = yield self.create_content(
+            media_type, None, request.content.read(),
+            content_length, auth_user
+        )
+
+        respond_with_json(
+            request, 200, {"content_uri": content_uri}, send_cors=True
+        )