From dd93b42e1d7d58d9fed619da7002b5e19854e4d9 Mon Sep 17 00:00:00 2001 From: Nick Steel Date: Sun, 5 Jan 2020 23:50:35 +0000 Subject: [PATCH 1/9] Run pyupgrade --py37-plus --- mopidy_iris/__init__.py | 7 ++----- mopidy_iris/core.py | 3 --- mopidy_iris/frontend.py | 4 +--- mopidy_iris/handlers.py | 2 -- mopidy_iris/system.py | 1 - 5 files changed, 3 insertions(+), 14 deletions(-) diff --git a/mopidy_iris/__init__.py b/mopidy_iris/__init__.py index acddbf8e..50dbee4d 100755 --- a/mopidy_iris/__init__.py +++ b/mopidy_iris/__init__.py @@ -1,6 +1,3 @@ - -from __future__ import unicode_literals - import logging, os, json, pathlib import tornado.web import tornado.websocket @@ -59,10 +56,10 @@ class ReactRouterHandler(tornado.web.StaticFileHandler): self.path = path self.absolute_path = path self.dirname, self.filename = os.path.split(path) - super(ReactRouterHandler, self).initialize(self.dirname) + super().initialize(self.dirname) def get(self, path=None, include_body=True): - return super(ReactRouterHandler, self).get(self.path, include_body) + return super().get(self.path, include_body) ## # Frontend factory diff --git a/mopidy_iris/core.py b/mopidy_iris/core.py index 3fb2e6cb..3169f80c 100755 --- a/mopidy_iris/core.py +++ b/mopidy_iris/core.py @@ -1,6 +1,3 @@ - -from __future__ import unicode_literals - import random, string, logging, json, pykka, urllib, os, sys, mopidy_iris, subprocess import tornado.web import tornado.ioloop diff --git a/mopidy_iris/frontend.py b/mopidy_iris/frontend.py index 831a4183..6ba3362c 100755 --- a/mopidy_iris/frontend.py +++ b/mopidy_iris/frontend.py @@ -1,5 +1,3 @@ - -from __future__ import unicode_literals from mopidy.core import CoreListener from .core import IrisCore @@ -13,7 +11,7 @@ logger = logging.getLogger(__name__) class IrisFrontend(pykka.ThreadingActor, CoreListener): def __init__(self, config, core): - super(IrisFrontend, self).__init__() + super().__init__() # Pass our Mopidy config and core to the IrisCore instance iris.config = config diff --git a/mopidy_iris/handlers.py b/mopidy_iris/handlers.py index b625f135..ecb54d67 100755 --- a/mopidy_iris/handlers.py +++ b/mopidy_iris/handlers.py @@ -1,5 +1,3 @@ - -from __future__ import unicode_literals from datetime import datetime from tornado.escape import json_encode, json_decode import tornado.ioloop, tornado.web, tornado.websocket, tornado.template diff --git a/mopidy_iris/system.py b/mopidy_iris/system.py index c9b435bf..ab1210a1 100755 --- a/mopidy_iris/system.py +++ b/mopidy_iris/system.py @@ -1,4 +1,3 @@ - from threading import Thread import os, logging, subprocess, json From d3cce38832733b56d5d1faa689ca422e638c98fd Mon Sep 17 00:00:00 2001 From: Nick Steel Date: Mon, 6 Jan 2020 00:26:55 +0000 Subject: [PATCH 2/9] Use pathlib --- mopidy_iris/__init__.py | 8 ++++---- mopidy_iris/core.py | 20 ++++++++------------ mopidy_iris/system.py | 12 ++++++------ 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/mopidy_iris/__init__.py b/mopidy_iris/__init__.py index 50dbee4d..3cbcb5ad 100755 --- a/mopidy_iris/__init__.py +++ b/mopidy_iris/__init__.py @@ -1,4 +1,4 @@ -import logging, os, json, pathlib +import logging, json, pathlib import tornado.web import tornado.websocket @@ -21,8 +21,7 @@ class Extension( ext.Extension ): ext_name = 'iris' def get_default_config(self): - conf_file = os.path.join(os.path.dirname(__file__), 'ext.conf') - return config.read(conf_file) + return config.read(pathlib.Path(__file__).parent / "ext.conf") def get_config_schema(self): schema = config.ConfigSchema(self.ext_name) @@ -55,7 +54,8 @@ class ReactRouterHandler(tornado.web.StaticFileHandler): def initialize(self, path): self.path = path self.absolute_path = path - self.dirname, self.filename = os.path.split(path) + self.dirname = path.parent + self.filename = path.name super().initialize(self.dirname) def get(self, path=None, include_body=True): diff --git a/mopidy_iris/core.py b/mopidy_iris/core.py index 3169f80c..5c7b5d1b 100755 --- a/mopidy_iris/core.py +++ b/mopidy_iris/core.py @@ -1,4 +1,4 @@ -import random, string, logging, json, pykka, urllib, os, sys, mopidy_iris, subprocess +import random, string, logging, json, pathlib, pykka, urllib, os, sys, mopidy_iris, subprocess import tornado.web import tornado.ioloop import tornado.httpclient @@ -11,6 +11,7 @@ from pkg_resources import parse_version from tornado.escape import json_encode, json_decode from tornado.httpclient import AsyncHTTPClient +from . import Extension from .system import IrisSystemThread if sys.platform == 'win32': @@ -62,15 +63,11 @@ class IrisCore(pykka.ThreadingActor): # @return void ## def save_to_file(self, dict, name): - path = self.config['iris'].get('data_dir') - - # Create the folder if it doesn't yet exist - if not os.path.exists(path): - os.makedirs(path) + file_path = Extension.get_data_dir(self.config) / ('%s.pkl' % name) # And now open the file, and drop in our dict try: - with open(path + '/' + name + '.pkl', 'wb') as f: + with file_path.open('wb') as f: pickle.dump(dict, f, pickle.HIGHEST_PROTOCOL) except Exception: return False @@ -82,10 +79,10 @@ class IrisCore(pykka.ThreadingActor): # @return Dict ## def load_from_file(self, name): - path = self.config['iris'].get('data_dir') + file_path = Extension.get_data_dir(self.config) / ('%s.pkl' % name) try: - with open(path + '/' + name + '.pkl', 'rb') as f: + with file_path.open('wb') as f: return pickle.load(f) except Exception: return {} @@ -96,10 +93,9 @@ class IrisCore(pykka.ThreadingActor): # @return String ## def load_version(self): - filepath = os.path.join(os.path.dirname(__file__), '..', 'IRIS_VERSION') + file_path = pathlib.Path(__file__).parent.parent / 'IRIS_VERSION' try: - with open(filepath, 'r') as f: - return f.read() + return file_path.read_text() except Exception: return "Unknown" diff --git a/mopidy_iris/system.py b/mopidy_iris/system.py index ab1210a1..64ee4c74 100755 --- a/mopidy_iris/system.py +++ b/mopidy_iris/system.py @@ -1,5 +1,5 @@ from threading import Thread -import os, logging, subprocess, json +import logging, pathlib, subprocess, json # import logger logger = logging.getLogger(__name__) @@ -9,7 +9,7 @@ class IrisSystemThread(Thread): Thread.__init__(self) self.action = action self.callback = callback - self.path = os.path.dirname(__file__) + self.script_path = pathlib.Path(__file__).parent / "system.sh" ## # Run the defined action @@ -31,9 +31,9 @@ class IrisSystemThread(Thread): 'error': error } - logger.debug("sudo "+ self.path +"/system.sh "+ self.action) + logger.debug("sudo %s %s", self.script_path, self.action) - proc = subprocess.Popen(["sudo", self.path+"/system.sh", self.action], + proc = subprocess.Popen(["sudo", str(self.script_path), self.action], stdout=subprocess.PIPE, stderr=subprocess.STDOUT) @@ -56,12 +56,12 @@ class IrisSystemThread(Thread): def can_run(self, *args, **kwargs): # Attempt an empty call to our system file - process = subprocess.Popen("sudo -n "+self.path+"/system.sh check", stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) + process = subprocess.Popen("sudo -n %s check" % self.script_path, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) result, error = process.communicate() exitCode = process.wait() # Some kind of failure, so we can't run any commands this way if exitCode > 0: - raise Exception("Password-less access to "+self.path+"/system.sh was refused. Check your /etc/sudoers file.") + raise Exception("Password-less access to %s was refused. Check your /etc/sudoers file." % self.script_path) else: return True From 35b3892e528971c6c60f16ca24e496121b171dcf Mon Sep 17 00:00:00 2001 From: Nick Steel Date: Mon, 6 Jan 2020 00:29:48 +0000 Subject: [PATCH 3/9] Use pkg_resources to populate __version__ --- mopidy_iris/__init__.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mopidy_iris/__init__.py b/mopidy_iris/__init__.py index 3cbcb5ad..57ad8f7c 100755 --- a/mopidy_iris/__init__.py +++ b/mopidy_iris/__init__.py @@ -2,14 +2,18 @@ import logging, json, pathlib import tornado.web import tornado.websocket +import pkg_resources from mopidy import config, ext from .frontend import IrisFrontend from .handlers import WebsocketHandler, HttpHandler from .core import IrisCore from .mem import iris +__version__ = pkg_resources.get_distribution("Mopidy-Iris").version + logger = logging.getLogger(__name__) + ## # Core extension class # @@ -19,6 +23,7 @@ class Extension( ext.Extension ): dist_name = 'Mopidy-Iris' ext_name = 'iris' + version = __version__ def get_default_config(self): return config.read(pathlib.Path(__file__).parent / "ext.conf") From e2962f67f05268d3fe0880a32f5c106b8ce5b5b4 Mon Sep 17 00:00:00 2001 From: Nick Steel Date: Tue, 7 Jan 2020 23:26:40 +0000 Subject: [PATCH 4/9] Remove LICENSE file extension --- LICENSE.md => LICENSE | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename LICENSE.md => LICENSE (100%) diff --git a/LICENSE.md b/LICENSE similarity index 100% rename from LICENSE.md rename to LICENSE From 83fbc65cfaee3ee57433ab63cc9dabdcfcfaa918 Mon Sep 17 00:00:00 2001 From: Nick Steel Date: Tue, 7 Jan 2020 23:28:34 +0000 Subject: [PATCH 5/9] Fix Extension setup and factory import. Moved ReactRouterHandler to be with the other handlers --- mopidy_iris/__init__.py | 37 +++++++++---------------------------- mopidy_iris/handlers.py | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/mopidy_iris/__init__.py b/mopidy_iris/__init__.py index 57ad8f7c..69ce9fdb 100755 --- a/mopidy_iris/__init__.py +++ b/mopidy_iris/__init__.py @@ -1,13 +1,7 @@ import logging, json, pathlib -import tornado.web -import tornado.websocket import pkg_resources from mopidy import config, ext -from .frontend import IrisFrontend -from .handlers import WebsocketHandler, HttpHandler -from .core import IrisCore -from .mem import iris __version__ = pkg_resources.get_distribution("Mopidy-Iris").version @@ -40,7 +34,7 @@ class Extension( ext.Extension ): return schema def setup(self, registry): - + from .frontend import IrisFrontend # Add web extension registry.add('http:app', { 'name': self.ext_name, @@ -50,33 +44,19 @@ class Extension( ext.Extension ): # Add our frontend registry.add('frontend', IrisFrontend) -## -# Customised handler for react router URLS -# -# This routes all URLs to the same path, so that React can handle the path etc -## -class ReactRouterHandler(tornado.web.StaticFileHandler): - def initialize(self, path): - self.path = path - self.absolute_path = path - self.dirname = path.parent - self.filename = path.name - super().initialize(self.dirname) - - def get(self, path=None, include_body=True): - return super().get(self.path, include_body) - ## # Frontend factory ## def iris_factory(config, core): + from tornado.web import StaticFileHandler + from .handlers import HttpHandler, ReactRouterHandler, WebsocketHandler path = pathlib.Path(__file__).parent / 'static' return [ ( r'/http/([^/]*)', - handlers.HttpHandler, + HttpHandler, { 'core': core, 'config': config @@ -84,7 +64,7 @@ def iris_factory(config, core): ), ( r'/ws/?', - handlers.WebsocketHandler, + WebsocketHandler, { 'core': core, 'config': config @@ -92,21 +72,22 @@ def iris_factory(config, core): ), ( r'/assets/(.*)', - tornado.web.StaticFileHandler, + StaticFileHandler, { 'path': path / 'assets' } ), ( r'/((.*)(?:css|js|json|map)$)', - tornado.web.StaticFileHandler, + StaticFileHandler, { 'path': path } ), ( r'/(.*)', - ReactRouterHandler, { + ReactRouterHandler, + { 'path': path / 'index.html' } ), diff --git a/mopidy_iris/handlers.py b/mopidy_iris/handlers.py index ecb54d67..18f327c1 100755 --- a/mopidy_iris/handlers.py +++ b/mopidy_iris/handlers.py @@ -233,4 +233,19 @@ class HttpHandler(tornado.web.RequestHandler): self.finish() +## +# Customised handler for react router URLS +# +# This routes all URLs to the same path, so that React can handle the path etc +## +class ReactRouterHandler(tornado.web.StaticFileHandler): + def initialize(self, path): + self.path = path + self.absolute_path = path + self.dirname = path.parent + self.filename = path.name + super().initialize(self.dirname) + + def get(self, path=None, include_body=True): + return super().get(self.path, include_body) From 700e09acfb02874def8898ccbff9d09f61d85106 Mon Sep 17 00:00:00 2001 From: Nick Steel Date: Wed, 8 Jan 2020 01:34:44 +0000 Subject: [PATCH 6/9] Support arbitrary path encodings and better error handling. --- mopidy_iris/system.py | 49 ++++++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/mopidy_iris/system.py b/mopidy_iris/system.py index 64ee4c74..37b40045 100755 --- a/mopidy_iris/system.py +++ b/mopidy_iris/system.py @@ -4,6 +4,27 @@ import logging, pathlib, subprocess, json # import logger logger = logging.getLogger(__name__) + +class IrisSystemError(Exception): + pass + + +class IrisSystemPermissionError(IrisSystemError): + reason = "Permission denied" + + def __init__(self, path): + message = "Password-less access to %s was refused. Check your /etc/sudoers file." % path.as_uri() + super().__init__(message) + + +class IrisSystemMissingError(IrisSystemError): + reason = "Not found" + + def __init__(self, path): + message = "Unable to access %s." % path.as_uri() + super().__init__(message) + + class IrisSystemThread(Thread): def __init__(self, action, callback): Thread.__init__(self) @@ -19,32 +40,34 @@ class IrisSystemThread(Thread): try: self.can_run() - except Exception as e: + except IrisSystemError as e: logger.error(e) error = { - 'message': "Permission denied", - 'description': str(e) + 'message': e.reason, + 'description': e.message } return { 'error': error } - logger.debug("sudo %s %s", self.script_path, self.action) + logger.debug("sudo %s %s", self.script_path.as_uri(), self.action) - proc = subprocess.Popen(["sudo", str(self.script_path), self.action], + proc = subprocess.Popen([b"sudo", bytes(self.script_path), self.action.encode()], stdout=subprocess.PIPE, - stderr=subprocess.STDOUT) + stderr=subprocess.PIPE) stdout,stderr = proc.communicate() if stderr: - logger.error(stderr.decode()) - self.callback(None, { 'error': stderr.decode() }) + error_string = os.fsdecode(stderr) + logger.error(error_string) + self.callback(None, {'error': error_string}) else: - logger.info(stdout.decode()) - self.callback({ 'output': stdout.decode() }, None) + response_string = os.fsdecode(stdout) + logger.info(response_string) + self.callback({'output': response_string}, None) @@ -54,14 +77,16 @@ class IrisSystemThread(Thread): # @return boolean or exception ## def can_run(self, *args, **kwargs): + if not self.script_path.is_file(): + raise IrisSystemMissingError(self.script_path) # Attempt an empty call to our system file - process = subprocess.Popen("sudo -n %s check" % self.script_path, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) + process = subprocess.Popen(b"sudo -n %s check" % bytes(self.script_path), stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) result, error = process.communicate() exitCode = process.wait() # Some kind of failure, so we can't run any commands this way if exitCode > 0: - raise Exception("Password-less access to %s was refused. Check your /etc/sudoers file." % self.script_path) + raise IrisSystemPermissionError(self.script_path) else: return True From 94f62b75da2193cd9be609abb74cab8de963cae8 Mon Sep 17 00:00:00 2001 From: Nick Steel Date: Wed, 8 Jan 2020 01:37:44 +0000 Subject: [PATCH 7/9] Added IrisSystemThread.get_command() helper and system tests. _USE_SUDO is to make testing easier. --- mopidy_iris/system.py | 34 +++++++++++---- tests/test_system.py | 98 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 9 deletions(-) create mode 100644 tests/test_system.py diff --git a/mopidy_iris/system.py b/mopidy_iris/system.py index 37b40045..a7bcf10f 100755 --- a/mopidy_iris/system.py +++ b/mopidy_iris/system.py @@ -1,5 +1,5 @@ from threading import Thread -import logging, pathlib, subprocess, json +import logging, os, pathlib, subprocess, json # import logger logger = logging.getLogger(__name__) @@ -14,6 +14,7 @@ class IrisSystemPermissionError(IrisSystemError): def __init__(self, path): message = "Password-less access to %s was refused. Check your /etc/sudoers file." % path.as_uri() + logger.error(message) super().__init__(message) @@ -22,16 +23,34 @@ class IrisSystemMissingError(IrisSystemError): def __init__(self, path): message = "Unable to access %s." % path.as_uri() + logger.error(message) super().__init__(message) class IrisSystemThread(Thread): + _USE_SUDO = True + def __init__(self, action, callback): Thread.__init__(self) self.action = action self.callback = callback self.script_path = pathlib.Path(__file__).parent / "system.sh" + def get_command(self, action=None, *, non_interactive=False): + if self._USE_SUDO: + if non_interactive: + args = [b'sudo -n'] + else: + args = [b'sudo'] + else: + args = [] + + if action is None: + action = self.action + + args = args + [bytes(self.script_path), action.encode()] + return args + ## # Run the defined action ## @@ -52,11 +71,9 @@ class IrisSystemThread(Thread): 'error': error } - logger.debug("sudo %s %s", self.script_path.as_uri(), self.action) - - proc = subprocess.Popen([b"sudo", bytes(self.script_path), self.action.encode()], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE) + command = self.get_command() + logger.debug("Running '%s'", os.fsdecode(b' '.join(command))) + proc = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) stdout,stderr = proc.communicate() @@ -69,8 +86,6 @@ class IrisSystemThread(Thread): logger.info(response_string) self.callback({'output': response_string}, None) - - ## # Check if we have access to the system script (system.sh) # @@ -81,7 +96,8 @@ class IrisSystemThread(Thread): raise IrisSystemMissingError(self.script_path) # Attempt an empty call to our system file - process = subprocess.Popen(b"sudo -n %s check" % bytes(self.script_path), stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) + command_bytes = b' '.join(self.get_command('check', non_interactive=True)) + process = subprocess.Popen(command_bytes, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) result, error = process.communicate() exitCode = process.wait() diff --git a/tests/test_system.py b/tests/test_system.py new file mode 100644 index 00000000..087a2280 --- /dev/null +++ b/tests/test_system.py @@ -0,0 +1,98 @@ +import pathlib, pytest, subprocess +from unittest import mock + +from mopidy_iris.system import IrisSystemThread, IrisSystemMissingError, IrisSystemPermissionError + + +def test_system_sh_path(): + iris_system = IrisSystemThread('foo', None) + assert iris_system.script_path.is_file() + assert iris_system.script_path.name == "system.sh" + + +def test_can_run(): + iris_system = IrisSystemThread('foo', None) + iris_system._USE_SUDO = False + assert iris_system.can_run() is True + +@pytest.fixture +def popen_mock(): + patcher = mock.patch("subprocess.Popen", spec=True) + yield patcher.start() + patcher.stop() + +@pytest.fixture +def process_mock(popen_mock): + mock_process = popen_mock.return_value + mock_process.communicate.return_value = ('', None) + mock_process.wait.return_value = 0 + yield mock_process + +def test_can_run_args(popen_mock, process_mock): + IrisSystemThread('foo', None).can_run() + popen_mock.assert_called_once_with( + mock.ANY, + shell=True, + stderr=subprocess.PIPE, + stdout=subprocess.PIPE + ) + +def test_can_run_uses_sudo_non_interactive(popen_mock, process_mock): + IrisSystemThread('foo', None).can_run() + + popen_mock.assert_called_once() + assert popen_mock.call_args[0][0].startswith(b"sudo -n ") + + +def test_can_run_calls_script_check(popen_mock, process_mock): + IrisSystemThread('foo', None).can_run() + + assert popen_mock.call_args[0][0].endswith(b"system.sh check") + + +def test_can_run_script_missing_raises(tmp_path, caplog): + iris_system = IrisSystemThread('foo', None) + iris_system.script_path = tmp_path + + with pytest.raises(IrisSystemMissingError) as excinfo: + iris_system.can_run() + + error_message = "Unable to access %s." % tmp_path.as_uri() + assert error_message in str(excinfo.value) + assert error_message in caplog.text + + +def test_can_run_sudo_refused_raises(popen_mock, process_mock, caplog): + process_mock.wait.return_value = 1 + iris_system = IrisSystemThread('foo', None) + + with pytest.raises(IrisSystemPermissionError) as excinfo: + iris_system.can_run() + + error_message = ( + "Password-less access to %s was refused. " + "Check your /etc/sudoers file." % iris_system.script_path.as_uri() + ) + assert error_message in str(excinfo.value) + assert error_message in caplog.text + + +def test_run_args(popen_mock, process_mock): + iris_system = IrisSystemThread('foo', mock.Mock()) + iris_system.can_run = mock.Mock(return_value = True) + iris_system.run() + + popen_mock.assert_called_once_with( + mock.ANY, + stderr=subprocess.PIPE, + stdout=subprocess.PIPE + ) + + +def test_run_uses_sudo(popen_mock, process_mock): + iris_system = IrisSystemThread('foo', mock.Mock()) + iris_system.can_run = mock.Mock(return_value = True) + iris_system.run() + + popen_mock.assert_called_once() + assert popen_mock.call_args[0][0][0] == b"sudo" From 47400bdd8c12c14c755c886fb5f39634e551b96f Mon Sep 17 00:00:00 2001 From: Nick Steel Date: Fri, 17 Jan 2020 00:59:52 +0000 Subject: [PATCH 8/9] Use AsyncHTTPClient instead of HTTPClient. Regarding HTTPClient, Tornado docs state: "Applications that are running an IOLoop must use AsyncHTTPClient instead." Also added some HTTP handler tests. --- mopidy_iris/core.py | 11 +++-- tests/test_handlers.py | 98 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 3 deletions(-) create mode 100644 tests/test_handlers.py diff --git a/mopidy_iris/core.py b/mopidy_iris/core.py index 5c7b5d1b..056701c6 100755 --- a/mopidy_iris/core.py +++ b/mopidy_iris/core.py @@ -35,6 +35,11 @@ class IrisCore(pykka.ThreadingActor): "results": [] } + @classmethod + async def do_fetch(cls, client, request): + # This wrapper function exists to ease mocking. + return await client.fetch(request) + def setup(self, config, core): self.config = config self.core = core @@ -968,7 +973,7 @@ class IrisCore(pykka.ThreadingActor): else: return response - def refresh_spotify_token(self, *args, **kwargs): + async def refresh_spotify_token(self, *args, **kwargs): callback = kwargs.get('callback', None) # Use client_id and client_secret from config @@ -981,9 +986,9 @@ class IrisCore(pykka.ThreadingActor): } try: - http_client = tornado.httpclient.HTTPClient() + http_client = tornado.httpclient.AsyncHTTPClient() request = tornado.httpclient.HTTPRequest(url, method='POST', body=urllib.parse.urlencode(data)) - response = http_client.fetch(request) + response = await self.do_fetch(http_client, request) token = json.loads(response.body) token['expires_at'] = time.time() + token['expires_in'] diff --git a/tests/test_handlers.py b/tests/test_handlers.py new file mode 100644 index 00000000..26a2cdbc --- /dev/null +++ b/tests/test_handlers.py @@ -0,0 +1,98 @@ +import pytest +from asyncio import Future +from unittest import mock + +import tornado.testing +import tornado.web +from tornado.httpclient import HTTPResponse +from tornado.escape import json_decode + +from mopidy_iris import handlers +from mopidy_iris import core +from mopidy_iris.mem import iris + + +def async_return_helper(result): + f = Future() + f.set_result(result) + return f + + +class HttpHandlerTest(tornado.testing.AsyncHTTPTestCase): + @pytest.fixture(autouse=True) + def inject_fixtures(self, caplog): + self._caplog = caplog + + def get_app(self): + http_handler = handlers.HttpHandler + # http_handler.handle_result = mock.Mock() + # self.handler_mock = http_handler.handle_result + return tornado.web.Application( + [ + ( + r"/(.*)", + http_handler, + { + 'core': None, + 'config': {}, + }, + ) + ] + ) + + def test_get_method(self): + with mock.patch("time.time", return_value=100): + response = self.fetch("/test", method="GET") + + assert 200 == response.code + result = json_decode(response.body) + assert "2.0" == result["jsonrpc"] + assert "test" == result["method"] + assert 100 == result["id"] + assert "Running test... please wait" == result["result"]["message"] + + def test_get_method_headers(self): + response = self.fetch("/test", method="GET") + + assert response.headers["Access-Control-Allow-Origin"] == "*" + assert "Origin" in response.headers["Access-Control-Allow-Headers"] + + def test_get_unknown_method_is_error(self): + response = self.fetch("/baz", method="GET") + + assert 400 == response.code + error = json_decode(response.body)["error"] + assert "Method baz does not exist" == error["message"] + + @mock.patch.object(handlers, "iris") + def test_get_method_called(self, iris_mock): + iris_mock.foo = mock.Mock() + + response = self.fetch("/foo", method="GET") + + iris_mock.foo.assert_called_once() + assert 200 == response.code + + @mock.patch.object(handlers, "iris") + def test_get_method_any_exception_handled(self, iris_mock): + iris_mock.foo = mock.Mock(side_effect=Exception("bar")) + + response = self.fetch("/foo", method="GET") + + iris_mock.foo.assert_called_once() + assert 200 == response.code + assert "bar" in self._caplog.text + + @mock.patch.object(handlers.iris, "do_fetch") + def test_get_method_with_fetch(self, fetch_mock): + iris.config = {"spotify" : {"client_id": 123, "client_secret": 456}} + result = mock.Mock(spec=HTTPResponse, body='{"expires_in":88}') + fetch_mock.return_value = async_return_helper(result) + + response = self.fetch("/refresh_spotify_token", method="GET") + + assert 200 == response.code + assert len(response.body) > 0 + result = json_decode(response.body) + assert "refresh_spotify_token" == result["method"] + assert 88 == result["result"]["spotify_token"]["expires_in"] From 985f7bb542f1736464cb7427a930439bbc31c084 Mon Sep 17 00:00:00 2001 From: Nick Steel Date: Fri, 17 Jan 2020 01:07:28 +0000 Subject: [PATCH 9/9] Removed pointless script_path check. If the file is missing, the install is bad and all bets are off. --- mopidy_iris/system.py | 12 ------------ tests/test_system.py | 14 +------------- 2 files changed, 1 insertion(+), 25 deletions(-) diff --git a/mopidy_iris/system.py b/mopidy_iris/system.py index a7bcf10f..28f52292 100755 --- a/mopidy_iris/system.py +++ b/mopidy_iris/system.py @@ -18,15 +18,6 @@ class IrisSystemPermissionError(IrisSystemError): super().__init__(message) -class IrisSystemMissingError(IrisSystemError): - reason = "Not found" - - def __init__(self, path): - message = "Unable to access %s." % path.as_uri() - logger.error(message) - super().__init__(message) - - class IrisSystemThread(Thread): _USE_SUDO = True @@ -92,9 +83,6 @@ class IrisSystemThread(Thread): # @return boolean or exception ## def can_run(self, *args, **kwargs): - if not self.script_path.is_file(): - raise IrisSystemMissingError(self.script_path) - # Attempt an empty call to our system file command_bytes = b' '.join(self.get_command('check', non_interactive=True)) process = subprocess.Popen(command_bytes, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) diff --git a/tests/test_system.py b/tests/test_system.py index 087a2280..5fd08206 100644 --- a/tests/test_system.py +++ b/tests/test_system.py @@ -1,7 +1,7 @@ import pathlib, pytest, subprocess from unittest import mock -from mopidy_iris.system import IrisSystemThread, IrisSystemMissingError, IrisSystemPermissionError +from mopidy_iris.system import IrisSystemThread, IrisSystemPermissionError def test_system_sh_path(): @@ -50,18 +50,6 @@ def test_can_run_calls_script_check(popen_mock, process_mock): assert popen_mock.call_args[0][0].endswith(b"system.sh check") -def test_can_run_script_missing_raises(tmp_path, caplog): - iris_system = IrisSystemThread('foo', None) - iris_system.script_path = tmp_path - - with pytest.raises(IrisSystemMissingError) as excinfo: - iris_system.can_run() - - error_message = "Unable to access %s." % tmp_path.as_uri() - assert error_message in str(excinfo.value) - assert error_message in caplog.text - - def test_can_run_sudo_refused_raises(popen_mock, process_mock, caplog): process_mock.wait.return_value = 1 iris_system = IrisSystemThread('foo', None)