andpp/mopidy
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

core: Make sure library can handle bad data from backends

Browse Source 
Note that None values are just ignored, while other bad data logs an error
message and is ignored.
This commit is contained in:
Thomas Adamcik 2015-05-05 22:55:53 +02:00
parent 746c3059ba
commit dd4a8f3b78
2 changed files with 181 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

93
mopidy/core/library.py
View File

@ -1,16 +1,30 @@
from __future__ import absolute_import, unicode_literals
import collections
import contextlib
import logging
import operator
import urlparse
from mopidy import compat, exceptions, models
from mopidy.utils import deprecation, validation
logger = logging.getLogger(__name__)
@contextlib.contextmanager
def _backend_error_handling(backend):
try:
yield
except exceptions.ValidationError as e:
logger.error('%s backend returned bad data: %s',
backend.actor_ref.actor_class.__name__, e)
except Exception:
logger.exception('%s backend caused an exception.',
backend.actor_ref.actor_class.__name__)
class LibraryController(object):
pykka_traversable = True
@ -79,22 +93,24 @@ class LibraryController(object):
backends = self.backends.with_library_browse.values()
futures = {b: b.library.root_directory for b in backends}
for backend, future in futures.items():
try:
directories.add(future.get())
except Exception:
logger.exception('%s backend caused an exception.',
backend.actor_ref.actor_class.__name__)
with _backend_error_handling(backend):
root = future.get()
validation.check_instance(root, models.Ref)
directories.add(root)
return sorted(directories, key=operator.attrgetter('name'))
def _browse(self, uri):
scheme = urlparse.urlparse(uri).scheme
backend = self.backends.with_library_browse.get(scheme)
try:
if backend:
return backend.library.browse(uri).get()
except Exception:
logger.exception('%s backend caused an exception.',
backend.actor_ref.actor_class.__name__)
if not backend:
return []
with _backend_error_handling(backend):
result = backend.library.browse(uri).get()
validation.check_instances(result, models.Ref)
return result
return []
def get_distinct(self, field, query=None):
@ -120,11 +136,11 @@ class LibraryController(object):
futures = {b: b.library.get_distinct(field, query)
for b in self.backends.with_library.values()}
for backend, future in futures.items():
try:
result.update(future.get())
except Exception:
logger.exception('%s backend caused an exception.',
backend.actor_ref.actor_class.__name__)
with _backend_error_handling(backend):
values = future.get()
if values is not None:
validation.check_instances(values, compat.text_type)
result.update(values)
return result
def get_images(self, uris):
@ -152,12 +168,18 @@ class LibraryController(object):
results = {uri: tuple() for uri in uris}
for backend, future in futures.items():
try:
with _backend_error_handling(backend):
if future.get() is None:
continue
validation.check_instance(future.get(), collections.Mapping)
for uri, images in future.get().items():
if uri not in uris:
name = backend.actor_ref.actor_class.__name__
logger.warning(
'%s backend returned image for URI we did not '
'ask for: %s', name, uri)
validation.check_instances(images, models.Image)
results[uri] += tuple(images)
except Exception:
logger.exception('%s backend caused an exception.',
backend.actor_ref.actor_class.__name__)
return results
def find_exact(self, query=None, uris=None, **kwargs):
@ -202,7 +224,7 @@ class LibraryController(object):
uris = [uri]
futures = {}
result = {u: [] for u in uris}
results = {u: [] for u in uris}
# TODO: lookup(uris) to backend APIs
for backend, backend_uris in self._get_backends_to_uris(uris).items():
@ -210,15 +232,15 @@ class LibraryController(object):
futures[(backend, u)] = backend.library.lookup(u)
for (backend, u), future in futures.items():
try:
result[u] = future.get()
except Exception:
logger.exception('%s backend caused an exception.',
backend.actor_ref.actor_class.__name__)
with _backend_error_handling(backend):
result = future.get()
if result is not None:
validation.check_instances(result, models.Track)
results[u] = result
if uri:
return result[uri]
return result
return results[uri]
return results
def refresh(self, uri=None):
"""
@ -241,11 +263,8 @@ class LibraryController(object):
futures[backend] = backend.library.refresh(uri)
for backend, future in futures.items():
try:
with _backend_error_handling(backend):
future.get()
except Exception:
logger.exception('%s backend caused an exception.',
backend.actor_ref.actor_class.__name__)
def search(self, query=None, uris=None, exact=False, **kwargs):
"""
@ -313,8 +332,14 @@ class LibraryController(object):
results = []
for backend, future in futures.items():
try:
results.append(future.get())
try: # TODO: fix all these cases so we can use common helper
result = future.get()
if result is not None:
validation.check_instance(result, models.SearchResult)
results.append(result)
except exceptions.ValidationError as e:
logger.error('%s backend returned bad data: %s',
backend.actor_ref.actor_class.__name__, e)
except TypeError:
backend_name = backend.actor_ref.actor_class.__name__
logger.warning(

130
tests/core/test_library.py
View File

@ -15,6 +15,7 @@ class BaseCoreLibraryTest(unittest.TestCase):
dummy1_root = Ref.directory(uri='dummy1:directory', name='dummy1')
self.backend1 = mock.Mock()
self.backend1.uri_schemes.get.return_value = ['dummy1']
self.backend1.actor_ref.actor_class.__name__ = 'DummyBackend1'
self.library1 = mock.Mock(spec=backend.LibraryProvider)
self.library1.get_images().get.return_value = {}
self.library1.get_images.reset_mock()
@ -24,6 +25,7 @@ class BaseCoreLibraryTest(unittest.TestCase):
dummy2_root = Ref.directory(uri='dummy2:directory', name='dummy2')
self.backend2 = mock.Mock()
self.backend2.uri_schemes.get.return_value = ['dummy2', 'du2']
self.backend2.actor_ref.actor_class.__name__ = 'DummyBackend2'
self.library2 = mock.Mock(spec=backend.LibraryProvider)
self.library2.get_images().get.return_value = {}
self.library2.get_images.reset_mock()
@ -33,6 +35,7 @@ class BaseCoreLibraryTest(unittest.TestCase):
# A backend without the optional library provider
self.backend3 = mock.Mock()
self.backend3.uri_schemes.get.return_value = ['dummy3']
self.backend3.actor_ref.actor_class.__name__ = 'DummyBackend3'
self.backend3.has_library().get.return_value = False
self.backend3.has_library_browse().get.return_value = False
@ -156,11 +159,14 @@ class CoreLibraryTest(BaseCoreLibraryTest):
self.core.library.lookup('dummy1:a', ['dummy2:a'])
def test_lookup_can_handle_uris(self):
self.library1.lookup().get.return_value = [1234]
self.library2.lookup().get.return_value = [5678]
track1 = Track(name='abc')
track2 = Track(name='def')
self.library1.lookup().get.return_value = [track1]
self.library2.lookup().get.return_value = [track2]
result = self.core.library.lookup(uris=['dummy1:a', 'dummy2:a'])
self.assertEqual(result, {'dummy2:a': [5678], 'dummy1:a': [1234]})
self.assertEqual(result, {'dummy2:a': [track2], 'dummy1:a': [track1]})
def test_lookup_uris_returns_empty_list_for_dummy3_track(self):
result = self.core.library.lookup(uris=['dummy3:a'])
@ -363,12 +369,14 @@ class DeprecatedLookupCoreLibraryTest(BaseCoreLibraryTest):
return super(DeprecatedLookupCoreLibraryTest, self).run(result)
def test_lookup_selects_dummy1_backend(self):
self.library1.lookup.return_value.get.return_value = []
self.core.library.lookup('dummy1:a')
self.library1.lookup.assert_called_once_with('dummy1:a')
self.assertFalse(self.library2.lookup.called)
def test_lookup_selects_dummy2_backend(self):
self.library2.lookup.return_value.get.return_value = []
self.core.library.lookup('dummy2:a')
self.assertFalse(self.library1.lookup.called)
@ -443,28 +451,124 @@ class BackendFailuresCoreLibraryTest(unittest.TestCase):
self.assertEqual([], self.core.library.browse(None))
logger.exception.assert_called_with(mock.ANY, 'DummyBackend')
def test_browse_backend_get_root_bad_value(self, logger):
self.library.root_directory.get.return_value = 123
self.assertEqual([], self.core.library.browse(None))
logger.error.assert_called_with(mock.ANY, 'DummyBackend', mock.ANY)
def test_browse_backend_get_root_none(self, logger):
self.library.root_directory.get.return_value = None
self.assertEqual([], self.core.library.browse(None))
logger.error.assert_called_with(mock.ANY, 'DummyBackend', mock.ANY)
def test_browse_backend_browse_uri_exception_gets_ignored(self, logger):
self.library.browse.return_value.get.side_effect = Exception
self.assertEqual([], self.core.library.browse('dummy:directory'))
logger.exception.assert_called_with(mock.ANY, 'DummyBackend')
def test_browse_backend_browse_uri_bad_value(self, logger):
self.library.browse.return_value.get.return_value = [123]
self.assertEqual([], self.core.library.browse('dummy:directory'))
logger.error.assert_called_with(mock.ANY, 'DummyBackend', mock.ANY)
def test_get_distinct_backend_exception_gets_ignored(self, logger):
self.library.get_distinct.return_value.get.side_effect = Exception
self.assertEqual(set(), self.core.library.get_distinct('artist'))
logger.exception.assert_called_with(mock.ANY, 'DummyBackend')
def test_get_distinct_backend_returns_string(self, logger):
self.library.get_distinct.return_value.get.return_value = 'abc'
self.assertEqual(set(), self.core.library.get_distinct('artist'))
logger.error.assert_called_with(mock.ANY, 'DummyBackend', mock.ANY)
def test_get_distinct_backend_returns_none(self, logger):
self.library.get_distinct.return_value.get.return_value = None
self.assertEqual(set(), self.core.library.get_distinct('artist'))
self.assertFalse(logger.error.called)
def test_get_distinct_backend_returns_list_if_ints(self, logger):
self.library.get_distinct.return_value.get.return_value = [1, 2, 3]
self.assertEqual(set(), self.core.library.get_distinct('artist'))
logger.error.assert_called_with(mock.ANY, 'DummyBackend', mock.ANY)
def test_get_images_backend_exception_get_ignored(self, logger):
uri = 'dummy:/1'
self.library.get_images.return_value.get.side_effect = Exception
self.assertEqual(
{'dummy:/1': tuple()}, self.core.library.get_images(['dummy:/1']))
self.assertEqual({uri: tuple()}, self.core.library.get_images([uri]))
logger.exception.assert_called_with(mock.ANY, 'DummyBackend')
def test_lookup_backend_exceptiosn_gets_ignores(self, logger):
def test_get_images_backend_returns_string(self, logger):
uri = 'dummy:/1'
self.library.get_images.return_value.get.return_value = 'abc'
self.assertEqual({uri: tuple()}, self.core.library.get_images([uri]))
logger.error.assert_called_with(mock.ANY, 'DummyBackend', mock.ANY)
def test_get_images_backend_returns_none(self, logger):
uri = 'dummy:/1'
self.library.get_images.return_value.get.return_value = None
self.assertEqual({uri: tuple()}, self.core.library.get_images([uri]))
self.assertFalse(logger.error.called)
def test_get_images_backend_returns_bad_dict(self, logger):
uri = 'dummy:/1'
self.library.get_images.return_value.get.return_value = {uri: 'abc'}
self.assertEqual({uri: tuple()}, self.core.library.get_images([uri]))
logger.error.assert_called_with(mock.ANY, 'DummyBackend', mock.ANY)
def test_get_images_backend_returns_dict_with_none(self, logger):
uri = 'dummy:/1'
self.library.get_images.return_value.get.return_value = {uri: None}
self.assertEqual({uri: tuple()}, self.core.library.get_images([uri]))
logger.error.assert_called_with(mock.ANY, 'DummyBackend', mock.ANY)
def test_get_images_backend_returns_wrong_uri(self, logger):
uri = 'dummy:/1'
self.library.get_images.return_value.get.return_value = {'foo': []}
self.assertEqual({uri: tuple()}, self.core.library.get_images([uri]))
logger.warning.assert_called_with(mock.ANY, 'DummyBackend', 'foo')
def test_lookup_backend_exceptions_gets_ignored(self, logger):
uri = 'dummy:/1'
self.library.lookup.return_value.get.side_effect = Exception
self.assertEqual(
{'dummy:/1': []}, self.core.library.lookup(uris=['dummy:/1']))
self.assertEqual({uri: []}, self.core.library.lookup(uris=[uri]))
logger.exception.assert_called_with(mock.ANY, 'DummyBackend')
def test_lookup_uris_backend_returns_string(self, logger):
uri = 'dummy:/1'
self.library.lookup.return_value.get.return_value = 'abc'
self.assertEqual({uri: []}, self.core.library.lookup(uris=[uri]))
logger.error.assert_called_with(mock.ANY, 'DummyBackend', mock.ANY)
def test_lookup_uris_backend_returns_none(self, logger):
uri = 'dummy:/1'
self.library.lookup.return_value.get.return_value = None
self.assertEqual({uri: []}, self.core.library.lookup(uris=[uri]))
self.assertFalse(logger.error.called)
def test_lookup_uris_backend_returns_bad_list(self, logger):
uri = 'dummy:/1'
self.library.lookup.return_value.get.return_value = [123]
self.assertEqual({uri: []}, self.core.library.lookup(uris=[uri]))
logger.error.assert_called_with(mock.ANY, 'DummyBackend', mock.ANY)
def test_lookup_uri_backend_returns_string(self, logger):
uri = 'dummy:/1'
self.library.lookup.return_value.get.return_value = 'abc'
self.assertEqual([], self.core.library.lookup(uri))
logger.error.assert_called_with(mock.ANY, 'DummyBackend', mock.ANY)
def test_lookup_uri_backend_returns_none(self, logger):
uri = 'dummy:/1'
self.library.lookup.return_value.get.return_value = None
self.assertEqual([], self.core.library.lookup(uri))
self.assertFalse(logger.error.called)
def test_lookup_uri_backend_returns_bad_list(self, logger):
uri = 'dummy:/1'
self.library.lookup.return_value.get.return_value = [123]
self.assertEqual([], self.core.library.lookup(uri))
logger.error.assert_called_with(mock.ANY, 'DummyBackend', mock.ANY)
def test_refresh_backend_exception_gets_ignored(self, logger):
self.library.refresh.return_value.get.side_effect = Exception
self.core.library.refresh()
@ -486,3 +590,13 @@ class BackendFailuresCoreLibraryTest(unittest.TestCase):
self.library.search.return_value.get.side_effect = LookupError
with self.assertRaises(LookupError):
self.core.library.search(query={'any': ['foo']})
def test_search_backend_returns_string(self, logger):
self.library.search.return_value.get.return_value = 'abc'
self.assertEqual([], self.core.library.search(query={'any': ['foo']}))
logger.error.assert_called_with(mock.ANY, 'DummyBackend', mock.ANY)
def test_search_backend_returns_none(self, logger):
self.library.search.return_value.get.return_value = None
self.assertEqual([], self.core.library.search(query={'any': ['foo']}))
self.assertFalse(logger.error.called)