From 4b383c1762c006492066edd28cf9ad85bc112ff9 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 20 May 2014 23:49:22 +0200 Subject: [PATCH] http: Move Mopidy request handlers to a MopidyHttpRouter --- mopidy/http/__init__.py | 3 ++- mopidy/http/actor.py | 34 +++++++++++++++++++--------------- mopidy/http/handlers.py | 37 +++++++++++++++++++++---------------- tests/http/test_events.py | 16 +++------------- tests/http/test_server.py | 6 ++++-- 5 files changed, 49 insertions(+), 47 deletions(-) diff --git a/mopidy/http/__init__.py b/mopidy/http/__init__.py index 24729a6e..d9d873d8 100644 --- a/mopidy/http/__init__.py +++ b/mopidy/http/__init__.py @@ -34,11 +34,12 @@ class Extension(ext.Extension): raise exceptions.ExtensionError('tornado library not found', e) def setup(self, registry): - from .actor import HttpFrontend + from .actor import HttpFrontend, MopidyHttpRouter HttpFrontend.routers = registry['http:router'] registry.add('frontend', HttpFrontend) + registry.add('http:router', MopidyHttpRouter) class Router(object): diff --git a/mopidy/http/actor.py b/mopidy/http/actor.py index 63f6cfd4..56f815e8 100644 --- a/mopidy/http/actor.py +++ b/mopidy/http/actor.py @@ -11,13 +11,15 @@ import tornado.ioloop import tornado.web import tornado.websocket -from mopidy import models, zeroconf +from mopidy import http, models, zeroconf from mopidy.core import CoreListener from mopidy.http import handlers logger = logging.getLogger(__name__) +mopidy_data_dir = os.path.join(os.path.dirname(__file__), 'data') + class HttpFrontend(pykka.ThreadingActor, CoreListener): routers = [] @@ -32,7 +34,6 @@ class HttpFrontend(pykka.ThreadingActor, CoreListener): self.zeroconf_name = config['http']['zeroconf'] self.zeroconf_service = None self.app = None - self.websocket_clients = set() def on_start(self): threading.Thread(target=self._startup).start() @@ -59,15 +60,13 @@ class HttpFrontend(pykka.ThreadingActor, CoreListener): event = data event['event'] = name message = json.dumps(event, cls=models.ModelJSONEncoder) - handlers.WebSocketHandler.broadcast(self.websocket_clients, message) + handlers.WebSocketHandler.broadcast(message) def _get_request_handlers(self): - mopidy_dir = os.path.join(os.path.dirname(__file__), 'data') - # Either default Mopidy or user defined path to files static_dir = self.config['http']['static_dir'] root_dir = (r'/(.*)', handlers.StaticFileHandler, { - 'path': static_dir if static_dir else mopidy_dir, + 'path': static_dir if static_dir else mopidy_data_dir, 'default_filename': 'index.html' }) @@ -76,15 +75,7 @@ class HttpFrontend(pykka.ThreadingActor, CoreListener): 'HTTP routes from extensions: %s', list((l[0], l[1]) for l in request_handlers)) - # TODO: Dynamically define all endpoints - request_handlers.extend([ - (r'/mopidy/ws/?', handlers.WebSocketHandler, {'actor': self}), - (r'/mopidy/rpc', handlers.JsonRpcHandler, {'actor': self}), - (r'/mopidy/(.*)', handlers.StaticFileHandler, { - 'path': mopidy_dir, 'default_filename': 'mopidy.html' - }), - root_dir, - ]) + request_handlers.append(root_dir) return request_handlers def _get_extension_request_handlers(self): @@ -128,3 +119,16 @@ class HttpFrontend(pykka.ThreadingActor, CoreListener): if self.zeroconf_mopidy_http_service: self.zeroconf_mopidy_http_service.unpublish() + + +class MopidyHttpRouter(http.Router): + name = 'mopidy' + + def get_request_handlers(self): + return [ + (r'/mopidy/ws/?', handlers.WebSocketHandler, {'core': self.core}), + (r'/mopidy/rpc', handlers.JsonRpcHandler, {'core': self.core}), + (r'/mopidy/(.*)', handlers.StaticFileHandler, { + 'path': mopidy_data_dir, 'default_filename': 'mopidy.html' + }), + ] diff --git a/mopidy/http/handlers.py b/mopidy/http/handlers.py index cd65bb34..267eb3a0 100644 --- a/mopidy/http/handlers.py +++ b/mopidy/http/handlers.py @@ -14,7 +14,7 @@ from mopidy.utils import jsonrpc logger = logging.getLogger(__name__) -def construct_rpc(actor): +def make_jsonrpc_wrapper(core_actor): inspector = jsonrpc.JsonRpcInspector( objects={ 'core.get_uri_schemes': core.Core.get_uri_schemes, @@ -27,12 +27,12 @@ def construct_rpc(actor): return jsonrpc.JsonRpcWrapper( objects={ 'core.describe': inspector.describe, - 'core.get_uri_schemes': actor.core.get_uri_schemes, - 'core.get_version': actor.core.get_version, - 'core.library': actor.core.library, - 'core.playback': actor.core.playback, - 'core.playlists': actor.core.playlists, - 'core.tracklist': actor.core.tracklist, + 'core.get_uri_schemes': core_actor.get_uri_schemes, + 'core.get_version': core_actor.get_version, + 'core.library': core_actor.library, + 'core.playback': core_actor.playback, + 'core.playlists': core_actor.playlists, + 'core.tracklist': core_actor.tracklist, }, decoders=[models.model_json_decoder], encoders=[models.ModelJSONEncoder] @@ -40,23 +40,28 @@ def construct_rpc(actor): class WebSocketHandler(tornado.websocket.WebSocketHandler): - def initialize(self, actor): - self.actor = actor - self.jsonrpc = construct_rpc(actor) + + # XXX This set is shared by all WebSocketHandler objects. This isn't + # optimal, but there's currently no use case for having more than one of + # these anyway. + clients = set() @classmethod - def broadcast(cls, clients, msg): - for client in clients: + def broadcast(cls, msg): + for client in cls.clients: client.write_message(msg) + def initialize(self, core): + self.jsonrpc = make_jsonrpc_wrapper(core) + def open(self): self.set_nodelay(True) - self.actor.websocket_clients.add(self) + self.clients.add(self) logger.debug( 'New WebSocket connection from %s', self.request.remote_ip) def on_close(self): - self.actor.websocket_clients.discard(self) + self.clients.discard(self) logger.debug( 'Closed WebSocket connection from %s', self.request.remote_ip) @@ -82,8 +87,8 @@ class WebSocketHandler(tornado.websocket.WebSocketHandler): class JsonRpcHandler(tornado.web.RequestHandler): - def initialize(self, actor): - self.jsonrpc = construct_rpc(actor) + def initialize(self, core): + self.jsonrpc = make_jsonrpc_wrapper(core) def head(self): self.set_extra_headers() diff --git a/tests/http/test_events.py b/tests/http/test_events.py index fae8aa85..e86d2b2e 100644 --- a/tests/http/test_events.py +++ b/tests/http/test_events.py @@ -5,17 +5,9 @@ import unittest import mock - -try: - import tornado -except ImportError: - tornado = False - -if tornado: - from mopidy.http import actor +from mopidy.http import actor -@unittest.skipUnless(tornado, 'tornado is missing') @mock.patch('mopidy.http.handlers.WebSocketHandler.broadcast') class HttpEventsTest(unittest.TestCase): def setUp(self): @@ -32,9 +24,8 @@ class HttpEventsTest(unittest.TestCase): def test_track_playback_paused_is_broadcasted(self, broadcast): broadcast.reset_mock() self.http.on_event('track_playback_paused', foo='bar') - self.assertEqual(broadcast.call_args[0][0], set([])) self.assertDictEqual( - json.loads(str(broadcast.call_args[0][1])), { + json.loads(str(broadcast.call_args[0][0])), { 'event': 'track_playback_paused', 'foo': 'bar', }) @@ -42,9 +33,8 @@ class HttpEventsTest(unittest.TestCase): def test_track_playback_resumed_is_broadcasted(self, broadcast): broadcast.reset_mock() self.http.on_event('track_playback_resumed', foo='bar') - self.assertEqual(broadcast.call_args[0][0], set([])) self.assertDictEqual( - json.loads(str(broadcast.call_args[0][1])), { + json.loads(str(broadcast.call_args[0][0])), { 'event': 'track_playback_resumed', 'foo': 'bar', }) diff --git a/tests/http/test_server.py b/tests/http/test_server.py index a80cefe5..f1dac8d6 100644 --- a/tests/http/test_server.py +++ b/tests/http/test_server.py @@ -22,8 +22,10 @@ class HttpServerTest(tornado.testing.AsyncHTTPTestCase): core.get_version = mock.MagicMock(name='get_version') core.get_version.return_value = mopidy.__version__ - actor_http = actor.HttpFrontend(config=config, core=core) - return tornado.web.Application(actor_http._get_request_handlers()) + http_frontend = actor.HttpFrontend(config=config, core=core) + http_frontend.routers = [actor.MopidyHttpRouter] + + return tornado.web.Application(http_frontend._get_request_handlers()) def test_root_should_return_index(self): response = self.fetch('/', method='GET')