diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 4ec0026c..69ba5370 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -95,7 +95,7 @@ def main(): 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): + if not ext.validate_extension_data(data): config[extension.ext_name] = {'enabled': False} config_errors[extension.ext_name] = { 'enabled': 'extension disabled by self check.'} diff --git a/mopidy/ext.py b/mopidy/ext.py index 96e5d5e1..ab35008a 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -184,47 +184,70 @@ def load_extensions(): return installed_extensions -def validate_extension(extension, entry_point): +def validate_extension_data(data): """Verify extension's dependencies and environment. :param extensions: an extension to check :returns: if extension should be run """ - logger.debug('Validating extension: %s', extension.ext_name) + logger.debug('Validating extension: %s', data.extension.ext_name) - if extension.ext_name != entry_point.name: + if data.extension.ext_name != data.entry_point.name: logger.warning( 'Disabled extension %(ep)s: entry point name (%(ep)s) ' 'does not match extension name (%(ext)s)', - {'ep': entry_point.name, 'ext': extension.ext_name}) + {'ep': data.entry_point.name, 'ext': data.extension.ext_name}) return False try: - entry_point.require() + data.entry_point.require() except pkg_resources.DistributionNotFound as ex: logger.info( 'Disabled extension %s: Dependency %s not found', - extension.ext_name, ex) + data.extension.ext_name, ex) return False except pkg_resources.VersionConflict as ex: if len(ex.args) == 2: found, required = ex.args logger.info( 'Disabled extension %s: %s required, but found %s at %s', - extension.ext_name, required, found, found.location) + data.extension.ext_name, required, found, found.location) else: logger.info( - 'Disabled extension %s: %s', extension.ext_name, ex) + 'Disabled extension %s: %s', data.extension.ext_name, ex) return False try: - extension.validate_environment() + data.extension.validate_environment() except exceptions.ExtensionError as ex: logger.info( - 'Disabled extension %s: %s', extension.ext_name, ex.message) + 'Disabled extension %s: %s', data.extension.ext_name, ex.message) return False except Exception: - return False # TODO: log + logger.exception('Validating extension %s failed with an exception.', + data.extension.ext_name) + return False + + if not data.config_schema: + logger.error('Extension %s does not have a config schema, disabling.', + data.extension.ext_name) + return False + elif not isinstance(data.config_schema.get('enabled'), config_lib.Boolean): + logger.error('Extension %s does not have the required "enabled" config' + ' option, disabling.', data.extension.ext_name) + return False + + for key, value in data.config_schema.items(): + if not isinstance(value, config_lib.ConfigValue): + logger.error('Extension %s config schema contains an invalid value' + ' for the option "%s", disabling.', + data.extension.ext_name, key) + return False + + if not data.config_defaults: + logger.error('Extension %s does not have a default config, disabling.', + data.extension.ext_name) + return False return True diff --git a/tests/test_ext.py b/tests/test_ext.py index b4fa8b9e..7fe6dba4 100644 --- a/tests/test_ext.py +++ b/tests/test_ext.py @@ -160,7 +160,7 @@ def test_load_extensions_get_command_fails(iter_entry_points_mock): get_command.assert_called_once_with() -# ext.validate_extension +# ext.validate_extension_data @pytest.fixture def ext_data(): @@ -178,18 +178,18 @@ def ext_data(): 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) + assert not ext.validate_extension_data(ext_data) 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) + assert not ext.validate_extension_data(ext_data) 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) + assert not ext.validate_extension_data(ext_data) def test_validate_extension_exception(ext_data): @@ -197,8 +197,7 @@ def test_validate_extension_exception(ext_data): # We trust that entry points are well behaved, so exception will bubble. with pytest.raises(Exception): - assert not ext.validate_extension( - ext_data.extension, ext_data.entry_point) + assert not ext.validate_extension_data(ext_data) def test_validate_extension_instance_validate_env_ext_error(ext_data): @@ -206,8 +205,7 @@ def test_validate_extension_instance_validate_env_ext_error(ext_data): with mock.patch.object(extension, 'validate_environment') as validate: validate.side_effect = exceptions.ExtensionError('error') - assert not ext.validate_extension( - ext_data.extension, ext_data.entry_point) + assert not ext.validate_extension_data(ext_data) validate.assert_called_once_with() @@ -216,6 +214,31 @@ def test_validate_extension_instance_validate_env_exception(ext_data): with mock.patch.object(extension, 'validate_environment') as validate: validate.side_effect = Exception - assert not ext.validate_extension( - ext_data.extension, ext_data.entry_point) + assert not ext.validate_extension_data(ext_data) validate.assert_called_once_with() + + +def test_validate_extension_with_missing_schema(ext_data): + ext_data = ext_data._replace(config_schema=None) + assert not ext.validate_extension_data(ext_data) + + +def test_validate_extension_with_schema_that_is_missing_enabled(ext_data): + del ext_data.config_schema['enabled'] + ext_data.config_schema['baz'] = config.String() + assert not ext.validate_extension_data(ext_data) + + +def test_validate_extension_with_schema_with_wrong_types(ext_data): + ext_data.config_schema['enabled'] = 123 + assert not ext.validate_extension_data(ext_data) + + +def test_validate_extension_with_schema_with_invalid_type(ext_data): + ext_data.config_schema['baz'] = 123 + assert not ext.validate_extension_data(ext_data) + + +def test_validate_extension_with_no_defaults(ext_data): + ext_data = ext_data._replace(config_defaults=None) + assert not ext.validate_extension_data(ext_data)