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()