From 5550785146aeb0e9426936c2fc69360f18c76118 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 11 May 2015 00:28:18 +0200 Subject: [PATCH] ext: Wrap extension state in a ExtensionData tuple This allows us to do more of the data loading that might fail safely in the mopidy.ext module instead of having things spread all over the place. Note that only minimal changes have been made to __main__ to make things work. Further refactoring should follow. --- mopidy/__main__.py | 23 ++++++---- mopidy/ext.py | 34 ++++++++++---- tests/test_ext.py | 107 +++++++++++++++++++-------------------------- 3 files changed, 86 insertions(+), 78 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 6584146f..4ec0026c 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -68,19 +68,20 @@ def main(): installed_extensions = ext.load_extensions() - for extension in installed_extensions: - ext_cmd = extension.get_command() - if ext_cmd: - ext_cmd.set(extension=extension) - root_cmd.add_child(extension.ext_name, ext_cmd) + for data in installed_extensions: + if data.command: + data.command.set(extension=data.command) + root_cmd.add_child(data.extension.ext_name, data.command) args = root_cmd.parse(mopidy_args) create_file_structures_and_config(args, installed_extensions) check_old_locations() + # TODO: make config.load use extension data? or just pass in schema+def config, config_errors = config_lib.load( - args.config_files, installed_extensions, args.config_overrides) + args.config_files, [d.extension for d in installed_extensions], + args.config_overrides) verbosity_level = args.base_verbosity_level if args.verbosity_level: @@ -90,8 +91,11 @@ def main(): extensions = { 'validate': [], 'config': [], 'disabled': [], 'enabled': []} - for extension in installed_extensions: - if not ext.validate_extension(extension): + for data in installed_extensions: + extension = data.extension + + # TODO: factor out all of this to a helper that can be tested + if not ext.validate_extension(data.extension, data.entry_point): config[extension.ext_name] = {'enabled': False} config_errors[extension.ext_name] = { 'enabled': 'extension disabled by self check.'} @@ -109,6 +113,9 @@ def main(): else: extensions['enabled'].append(extension) + # TODO: convert rest of code to use new ExtensionData + installed_extensions = [d.extension for d in installed_extensions] + log_extension_info(installed_extensions, extensions['enabled']) # Config and deps commands are simply special cased for now. diff --git a/mopidy/ext.py b/mopidy/ext.py index 625c27a2..32e06d1c 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -11,6 +11,12 @@ from mopidy import config as config_lib, exceptions logger = logging.getLogger(__name__) +_extension_data_fields = ['extension', 'entry_point', 'config_schema', + 'config_defaults', 'command'] + +ExtensionData = collections.namedtuple('ExtensionData', _extension_data_fields) + + class Extension(object): """Base class for Mopidy extensions""" @@ -149,6 +155,8 @@ def load_extensions(): logger.debug('Loading entry point: %s', entry_point) extension_class = entry_point.load(require=False) + # TODO: start using _extension_error_handling(...) pattern + try: if not issubclass(extension_class, Extension): continue # TODO: log this @@ -160,19 +168,26 @@ def load_extensions(): except Exception: continue # TODO: log this - extension.entry_point = entry_point + # TODO: handle exceptions and validate result... + config_schema = extension.get_config_schema() + default_config = extension.get_default_config() + command = extension.get_command() + + installed_extensions.append(ExtensionData( + extension, entry_point, config_schema, default_config, command)) + + # TODO: call validate_extension here? + # TODO: do basic config tests like schema contains enabled? - # TODO: store: (instance, entry_point, command, schema, defaults) - installed_extensions.append(extension) logger.debug( 'Loaded extension: %s %s', extension.dist_name, extension.version) - names = (e.ext_name for e in installed_extensions) + names = (ed.extension.ext_name for ed in installed_extensions) logger.debug('Discovered extensions: %s', ', '.join(names)) return installed_extensions -def validate_extension(extension): +def validate_extension(extension, entry_point): """Verify extension's dependencies and environment. :param extensions: an extension to check @@ -181,15 +196,15 @@ def validate_extension(extension): logger.debug('Validating extension: %s', extension.ext_name) - if extension.ext_name != extension.entry_point.name: + if extension.ext_name != entry_point.name: logger.warning( 'Disabled extension %(ep)s: entry point name (%(ep)s) ' 'does not match extension name (%(ext)s)', - {'ep': extension.entry_point.name, 'ext': extension.ext_name}) + {'ep': entry_point.name, 'ext': extension.ext_name}) return False try: - extension.entry_point.require() + entry_point.require() except pkg_resources.DistributionNotFound as ex: logger.info( 'Disabled extension %s: Dependency %s not found', @@ -202,7 +217,8 @@ def validate_extension(extension): 'Disabled extension %s: %s required, but found %s at %s', extension.ext_name, required, found, found.location) else: - logger.info('Disabled extension %s: %s', extension.ext_name, ex) + logger.info( + 'Disabled extension %s: %s', extension.ext_name, ex) return False try: diff --git a/tests/test_ext.py b/tests/test_ext.py index 72e1e141..d5dad4b1 100644 --- a/tests/test_ext.py +++ b/tests/test_ext.py @@ -8,12 +8,20 @@ import pytest from mopidy import config, exceptions, ext +from tests import IsA, any_unicode + class TestExtension(ext.Extension): dist_name = 'Mopidy-Foobar' ext_name = 'foobar' version = '1.2.3' + def get_default_config(self): + return '[foobar]\nenabled = true' + + +any_testextension = IsA(TestExtension) + # ext.Extension @@ -69,9 +77,11 @@ def test_load_extensions(mock_entry_points): mock_entry_points.return_value = [mock_entry_point] - extensions = ext.load_extensions() - assert len(extensions) == 1 - assert isinstance(extensions[0], TestExtension) + expected = ext.ExtensionData( + any_testextension, mock_entry_point, IsA(config.ConfigSchema), + any_unicode, None) + + assert ext.load_extensions() == [expected] @mock.patch('pkg_resources.iter_entry_points') @@ -110,87 +120,62 @@ def test_load_extensions_creating_instance_fails(mock_entry_points): assert [] == ext.load_extensions() -@mock.patch('pkg_resources.iter_entry_points') -def test_load_extensions_store_entry_point(mock_entry_points): - mock_entry_point = mock.Mock() - mock_entry_point.load.return_value = TestExtension - mock_entry_points.return_value = [mock_entry_point] - - extensions = ext.load_extensions() - assert len(extensions) == 1 - assert extensions[0].entry_point == mock_entry_point - - # ext.validate_extension -def test_validate_extension_name_mismatch(): - ep = mock.Mock() - ep.name = 'barfoo' - +@pytest.fixture +def ext_data(): extension = TestExtension() - extension.entry_point = ep - assert not ext.validate_extension(extension) + entry_point = mock.Mock() + entry_point.name = extension.ext_name + + schema = extension.get_config_schema() + defaults = extension.get_default_config() + command = extension.get_command() + + return ext.ExtensionData(extension, entry_point, schema, defaults, command) -def test_validate_extension_distribution_not_found(): - ep = mock.Mock() - ep.name = 'foobar' - ep.require.side_effect = pkg_resources.DistributionNotFound - - extension = TestExtension() - extension.entry_point = ep - - assert not ext.validate_extension(extension) +def test_validate_extension_name_mismatch(ext_data): + ext_data.entry_point.name = 'barfoo' + assert not ext.validate_extension(ext_data.extension, ext_data.entry_point) -def test_validate_extension_version_conflict(): - ep = mock.Mock() - ep.name = 'foobar' - ep.require.side_effect = pkg_resources.VersionConflict - - extension = TestExtension() - extension.entry_point = ep - - assert not ext.validate_extension(extension) +def test_validate_extension_distribution_not_found(ext_data): + error = pkg_resources.DistributionNotFound + ext_data.entry_point.require.side_effect = error + assert not ext.validate_extension(ext_data.extension, ext_data.entry_point) -def test_validate_extension_exception(): - ep = mock.Mock() - ep.name = 'foobar' - ep.require.side_effect = Exception +def test_validate_extension_version_conflict(ext_data): + ext_data.entry_point.require.side_effect = pkg_resources.VersionConflict + assert not ext.validate_extension(ext_data.extension, ext_data.entry_point) - extension = TestExtension() - extension.entry_point = ep + +def test_validate_extension_exception(ext_data): + ext_data.entry_point.require.side_effect = Exception # We trust that entry points are well behaved, so exception will bubble. with pytest.raises(Exception): - assert not ext.validate_extension(extension) + assert not ext.validate_extension( + ext_data.extension, ext_data.entry_point) -def test_validate_extension_instance_validate_env_ext_error(): - ep = mock.Mock() - ep.name = 'foobar' - - extension = TestExtension() - extension.entry_point = ep - +def test_validate_extension_instance_validate_env_ext_error(ext_data): + extension = ext_data.extension with mock.patch.object(extension, 'validate_environment') as validate: validate.side_effect = exceptions.ExtensionError('error') - assert not ext.validate_extension(extension) + assert not ext.validate_extension( + ext_data.extension, ext_data.entry_point) validate.assert_called_once_with() -def test_validate_extension_instance_validate_env_exception(): - ep = mock.Mock() - ep.name = 'foobar' - - extension = TestExtension() - extension.entry_point = ep - +def test_validate_extension_instance_validate_env_exception(ext_data): + extension = ext_data.extension with mock.patch.object(extension, 'validate_environment') as validate: validate.side_effect = Exception - assert not ext.validate_extension(extension) + assert not ext.validate_extension( + ext_data.extension, ext_data.entry_point) validate.assert_called_once_with()