Set mimetype when creating the response, don't try to fix it afterwards

Ref #76

supysonic/api/__init__.py

@@ -18,16 +18,20 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+API_VERSION = '1.8.0'
+
 import binascii
 import uuid
 
-from flask import request, json, current_app as app
+from flask import request
+from flask import current_app as app
 from pony.orm import db_session, ObjectNotFound
-from xml.dom import minidom
-from xml.etree import ElementTree
 
 from ..managers.user import UserManager
-from ..py23 import dict, strtype
+from ..py23 import dict
+
+from .formatters import make_json_response, make_jsonp_response, make_xml_response
+from .formatters import make_error_response_func
 
 @app.before_request
 def set_formatter():
@@ -35,23 +39,15 @@ def set_formatter():
         return
 
     """Return a function to create the response."""
-    (f, callback) = map(request.values.get, ['f', 'callback'])
+    f, callback = map(request.values.get, ['f', 'callback'])
     if f == 'jsonp':
-        # Some clients (MiniSub, Perisonic) set f to jsonp without callback for streamed data
+        request.formatter = lambda x, **kwargs: make_jsonp_response(x, callback, kwargs)
-        if not callback and request.endpoint not in [ 'stream_media', 'cover_art' ]:
+    elif f == 'json':
-            return ResponseHelper.responsize_json(dict(
+        request.formatter = make_json_response
-                error = dict(
-                    code = 10,
-                    message = 'Missing callback'
-                )
-            ), error = True), 400
-        request.formatter = lambda x, **kwargs: ResponseHelper.responsize_jsonp(x, callback, kwargs)
-    elif f == "json":
-        request.formatter = ResponseHelper.responsize_json
     else:
-        request.formatter = ResponseHelper.responsize_xml
+        request.formatter = make_xml_response
 
-    request.error_formatter = lambda code, msg: request.formatter(dict(error = dict(code = code, message = msg)), error = True)
+    request.error_formatter = make_error_response_func(request.formatter)
 
 def decode_password(password):
     if not password.startswith('enc:'):
@@ -110,18 +106,7 @@ def set_headers(response):
     if not request.path.startswith('/rest/'):
         return response
 
-    if response.mimetype.startswith('text'):
-        f = request.values.get('f')
-        # TODO set the mimetype when creating the response, here we could lose some info
-        if f == 'json':
-            response.headers['Content-Type'] = 'application/json'
-        elif f == 'jsonp':
-            response.headers['Content-Type'] = 'application/javascript'
-        else:
-            response.headers['Content-Type'] = 'text/xml'
-
     response.headers['Access-Control-Allow-Origin'] = '*'
-
     return response
 
 @app.errorhandler(404)
@@ -131,98 +116,6 @@ def not_found(error):
 
     return request.error_formatter(0, 'Not implemented'), 501
 
-class ResponseHelper:
-
-    @staticmethod
-    def remove_empty_lists(d):
-        if not isinstance(d, dict):
-            raise TypeError('Expecting a dict got ' + type(d).__name__)
-
-        keys_to_remove = []
-        for key, value in d.items():
-            if isinstance(value, dict):
-                d[key] = ResponseHelper.remove_empty_lists(value)
-            elif isinstance(value, list):
-                if len(value) == 0:
-                    keys_to_remove.append(key)
-                else:
-                    d[key] = [ ResponseHelper.remove_empty_lists(item) if isinstance(item, dict) else item for item in value ]
-
-        for key in keys_to_remove:
-            del d[key]
-
-        return d
-
-    @staticmethod
-    def responsize_json(ret, error = False, version = "1.8.0"):
-        ret = ResponseHelper.remove_empty_lists(ret)
-
-        # add headers to response
-        ret.update(
-            status = 'failed' if error else 'ok',
-            version = version
-        )
-        return json.dumps({ 'subsonic-response': ret }, indent = True)
-
-    @staticmethod
-    def responsize_jsonp(ret, callback, error = False, version = "1.8.0"):
-        return '{}({})'.format(callback, ResponseHelper.responsize_json(ret, error, version))
-
-    @staticmethod
-    def responsize_xml(ret, error = False, version = "1.8.0"):
-        """Return an xml response from json and replace unsupported characters."""
-        ret.update(
-            status = 'failed' if error else 'ok',
-            version = version,
-            xmlns = "http://subsonic.org/restapi"
-        )
-
-        elem = ElementTree.Element('subsonic-response')
-        ResponseHelper.dict2xml(elem, ret)
-
-        return minidom.parseString(ElementTree.tostring(elem)).toprettyxml(indent = '  ')
-
-    @staticmethod
-    def dict2xml(elem, dictionary):
-        """Convert a json structure to xml. The game is trivial. Nesting uses the [] parenthesis.
-          ex.  { 'musicFolder': {'id': 1234, 'name': "sss" } }
-            ex. { 'musicFolder': [{'id': 1234, 'name': "sss" }, {'id': 456, 'name': "aaa" }]}
-            ex. { 'musicFolders': {'musicFolder' : [{'id': 1234, 'name': "sss" }, {'id': 456, 'name': "aaa" }] } }
-            ex. { 'index': [{'name': 'A',  'artist': [{'id': '517674445', 'name': 'Antonello Venditti'}] }] }
-            ex. {"subsonic-response": { "musicFolders": {"musicFolder": [{ "id": 0,"name": "Music"}]},
-            "status": "ok","version": "1.7.0","xmlns": "http://subsonic.org/restapi"}}
-                """
-        if not isinstance(dictionary, dict):
-            raise TypeError('Expecting a dict')
-        if not all(map(lambda x: isinstance(x, strtype), dictionary)):
-            raise TypeError('Dictionary keys must be strings')
-
-        for name, value in dictionary.items():
-            if name == '_value_':
-                elem.text = ResponseHelper.value_tostring(value)
-            elif isinstance(value, dict):
-                subelem = ElementTree.SubElement(elem, name)
-                ResponseHelper.dict2xml(subelem, value)
-            elif isinstance(value, list):
-                for v in value:
-                    subelem = ElementTree.SubElement(elem, name)
-                    if isinstance(v, dict):
-                        ResponseHelper.dict2xml(subelem, v)
-                    else:
-                        subelem.text = ResponseHelper.value_tostring(v)
-            else:
-                elem.set(name, ResponseHelper.value_tostring(value))
-
-    @staticmethod
-    def value_tostring(value):
-        if value is None:
-            return None
-        if isinstance(value, strtype):
-            return value
-        if isinstance(value, bool):
-            return str(value).lower()
-        return str(value)
-
 def get_entity(req, cls, param = 'id'):
     eid = req.values.get(param)
     if not eid:

supysonic/api/formatters.py

@@ -0,0 +1,117 @@
+# coding: utf-8
+#
+# This file is part of Supysonic.
+# Supysonic is a Python implementation of the Subsonic server API.
+#
+# Copyright (C) 2018 Alban 'spl0k' Féron
+#
+# Distributed under terms of the GNU AGPLv3 license.
+
+from flask import json, jsonify, make_response
+from xml.dom import minidom
+from xml.etree import ElementTree
+
+from ..py23 import dict, strtype
+from . import API_VERSION
+
+def remove_empty_lists(d):
+    if not isinstance(d, dict):
+        raise TypeError('Expecting a dict got ' + type(d).__name__)
+
+    keys_to_remove = []
+    for key, value in d.items():
+        if isinstance(value, dict):
+            d[key] = remove_empty_lists(value)
+        elif isinstance(value, list):
+            if len(value) == 0:
+                keys_to_remove.append(key)
+            else:
+                d[key] = [ remove_empty_lists(item) if isinstance(item, dict) else item for item in value ]
+
+    for key in keys_to_remove:
+        del d[key]
+
+    return d
+
+def subsonicify(response, error):
+    rv = remove_empty_lists(response)
+
+    # add headers to response
+    rv.update(
+        status = 'failed' if error else 'ok',
+        version = API_VERSION
+    )
+    return { 'subsonic-response': rv }
+
+def dict2xml(elem, dictionary):
+    """Convert a json structure to xml. The game is trivial. Nesting uses the [] parenthesis.
+      ex.  { 'musicFolder': {'id': 1234, 'name': "sss" } }
+        ex. { 'musicFolder': [{'id': 1234, 'name': "sss" }, {'id': 456, 'name': "aaa" }]}
+        ex. { 'musicFolders': {'musicFolder' : [{'id': 1234, 'name': "sss" }, {'id': 456, 'name': "aaa" }] } }
+        ex. { 'index': [{'name': 'A',  'artist': [{'id': '517674445', 'name': 'Antonello Venditti'}] }] }
+        ex. {"subsonic-response": { "musicFolders": {"musicFolder": [{ "id": 0,"name": "Music"}]},
+        "status": "ok","version": "1.7.0","xmlns": "http://subsonic.org/restapi"}}
+    """
+    if not isinstance(dictionary, dict):
+        raise TypeError('Expecting a dict')
+    if not all(map(lambda x: isinstance(x, strtype), dictionary)):
+        raise TypeError('Dictionary keys must be strings')
+
+    for name, value in dictionary.items():
+        if name == '_value_':
+            elem.text = value_tostring(value)
+        elif isinstance(value, dict):
+            subelem = ElementTree.SubElement(elem, name)
+            dict2xml(subelem, value)
+        elif isinstance(value, list):
+            for v in value:
+                subelem = ElementTree.SubElement(elem, name)
+                if isinstance(v, dict):
+                    dict2xml(subelem, v)
+                else:
+                    subelem.text = value_tostring(v)
+        else:
+            elem.set(name, value_tostring(value))
+
+def value_tostring(value):
+    if value is None:
+        return None
+    if isinstance(value, strtype):
+        return value
+    if isinstance(value, bool):
+        return str(value).lower()
+    return str(value)
+
+def make_json_response(response, error = False):
+    return jsonify(subsonicify(response, error))
+
+def make_jsonp_response(response, callback, error = False):
+    if not callback:
+        return make_json_response(dict(error = dict(code = 10, message = 'Missing callback')), error = True)
+
+    rv = subsonicify(response, error)
+    rv = '{}({})'.format(callback, json.dumps(rv))
+    rv = make_response(rv)
+    rv.mimetype = 'application/javascript'
+    return rv
+
+def make_xml_response(response, error = False):
+    response.update(
+        status = 'failed' if error else 'ok',
+        version = API_VERSION,
+        xmlns = "http://subsonic.org/restapi"
+    )
+
+    elem = ElementTree.Element('subsonic-response')
+    dict2xml(elem, response)
+
+    rv = minidom.parseString(ElementTree.tostring(elem)).toprettyxml(indent = '  ')
+    rv = make_response(rv)
+    rv.mimetype = 'text/xml'
+    return rv
+
+def make_error_response_func(f):
+    def make_error_response(code, message):
+        return f(dict(error = dict(code = code, message = message)), error = True)
+    return make_error_response
+

tests/api/test_api_setup.py

@@ -111,7 +111,7 @@ class ApiSetupTestCase(TestBase):
 
         args.update({ 'f': 'jsonp' })
         rv = self.client.get('/rest/getLicense.view', query_string = args)
-        self.assertEqual(rv.mimetype, 'application/javascript')
+        self.assertEqual(rv.mimetype, 'application/json')
         json = flask.json.loads(rv.data)
         self.assertIn('subsonic-response', json)
         self.assertEqual(json['subsonic-response']['status'], 'failed')

tests/api/test_response_helper.py

@@ -22,15 +22,30 @@ class ResponseHelperBaseCase(TestBase):
     def setUp(self):
         super(ResponseHelperBaseCase, self).setUp()
 
-        from supysonic.api import ResponseHelper
+        from supysonic.api.formatters import make_json_response, make_jsonp_response, make_xml_response
-        self.helper = ResponseHelper
+        self.json = self.__json_unwrapper(make_json_response)
+        self.jsonp = self.__response_unwrapper(make_jsonp_response)
+        self.xml = self.__response_unwrapper(make_xml_response)
+
+    def __json_unwrapper(self, func):
+        def execute(*args, **kwargs):
+            with self.request_context():
+                rv = func(*args, **kwargs)
+                return rv.get_data(as_text = True)
+        return execute
+
+    def __response_unwrapper(self, func):
+        def execute(*args, **kwargs):
+            rv = func(*args, **kwargs)
+            return rv.get_data(as_text = True)
+        return execute
 
 class ResponseHelperJsonTestCase(ResponseHelperBaseCase):
     def serialize_and_deserialize(self, d, error = False):
         if not isinstance(d, dict):
             raise TypeError('Invalid tested value, expecting a dict')
 
-        json = self.helper.responsize_json(d, error)
+        json = self.json(d, error)
         return flask.json.loads(json)
 
     def process_and_extract(self, d, error = False):
@@ -119,7 +134,7 @@ class ResponseHelperJsonTestCase(ResponseHelperBaseCase):
 
 class ResponseHelperJsonpTestCase(ResponseHelperBaseCase):
     def test_basic(self):
-        result = self.helper.responsize_jsonp({}, 'callback')
+        result = self.jsonp({}, 'callback')
         self.assertTrue(result.startswith('callback({'))
         self.assertTrue(result.endswith('})'))
 
@@ -128,7 +143,7 @@ class ResponseHelperJsonpTestCase(ResponseHelperBaseCase):
 
 class ResponseHelperXMLTestCase(ResponseHelperBaseCase):
     def serialize_and_deserialize(self, d, error = False):
-        xml = self.helper.responsize_xml(d, error)
+        xml = self.xml(d, error)
         xml = xml.replace('xmlns="http://subsonic.org/restapi"', '')
         root = ElementTree.fromstring(xml)
         return root
@@ -138,7 +153,7 @@ class ResponseHelperXMLTestCase(ResponseHelperBaseCase):
         self.assertDictEqual(elem.attrib, d)
 
     def test_root(self):
-        xml = self.helper.responsize_xml({ 'tag': {}})
+        xml = self.xml({ 'tag': {}})
         self.assertIn('<subsonic-response ', xml)
         self.assertIn('xmlns="http://subsonic.org/restapi"', xml)
         self.assertTrue(xml.strip().endswith('</subsonic-response>'))

tests/testbase.py

@@ -16,6 +16,8 @@ import sys
 import unittest
 import tempfile
 
+from contextlib import contextmanager
+
 from supysonic.db import init_database, release_database
 from supysonic.config import DefaultConfig
 from supysonic.managers.user import UserManager
@@ -93,11 +95,11 @@ class TestBase(unittest.TestCase):
         init_database(config.BASE['database_uri'], True)
         release_database()
 
-        app = create_application(config)
+        self.__app = create_application(config)
-        self.__ctx = app.app_context()
+        self.__ctx = self.__app.app_context()
         self.__ctx.push()
 
-        self.client = app.test_client()
+        self.client = self.__app.test_client()
 
         UserManager.add('alice', 'Alic3', 'test@example.com', True)
         UserManager.add('bob', 'B0b', 'bob@example.com', False)
@@ -106,6 +108,13 @@ class TestBase(unittest.TestCase):
         self.client.get = patch_method(self.client.get)
         self.client.post = patch_method(self.client.post)
 
+    @contextmanager
+    def request_context(self, *args, **kwargs):
+        ctx = self.__app.test_request_context(*args, **kwargs)
+        ctx.push()
+        yield
+        ctx.pop()
+
     @staticmethod
     def __should_unload_module(module):
         if module.startswith('supysonic'):