diff --git a/docs/changes.rst b/docs/changes.rst index e3664013..93b8d366 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -30,6 +30,19 @@ v0.13.0 (in development) to the server just to add it to the tracklist when the server already got all the needed information easily available. (Fixes: :issue:`325`) +- Change the following methods to accept an ``uris`` keyword argument: + + - :meth:`mopidy.core.LibraryController.find_exact` + - :meth:`mopidy.core.LibraryController.search` + + Search queries will only be forwarded to backends handling the given URI + roots, and the backends may use the URI roots to further limit what results + are returned. For example, a search with ``uris=['file:']`` will only be + processed by the local backend. A search with + ``uris=['file:///media/music']`` will only be processed by the local backend, + and, if such filtering is supported by the backend, will only return results + with URIs within the given URI root. + **Audio sub-system** - Make audio error logging handle log messages with non-ASCII chars. (Fixes: diff --git a/mopidy/backends/base.py b/mopidy/backends/base.py index f49aa89b..415ef2a5 100644 --- a/mopidy/backends/base.py +++ b/mopidy/backends/base.py @@ -53,7 +53,7 @@ class BaseLibraryProvider(object): def __init__(self, backend): self.backend = backend - def find_exact(self, **query): + def find_exact(self, query=None, uris=None): """ See :meth:`mopidy.core.LibraryController.find_exact`. @@ -77,7 +77,7 @@ class BaseLibraryProvider(object): """ pass - def search(self, **query): + def search(self, query=None, uris=None): """ See :meth:`mopidy.core.LibraryController.search`. diff --git a/mopidy/backends/local/library.py b/mopidy/backends/local/library.py index eb328ce2..f2a1a520 100644 --- a/mopidy/backends/local/library.py +++ b/mopidy/backends/local/library.py @@ -35,7 +35,11 @@ class LocalLibraryProvider(base.BaseLibraryProvider): logger.debug('Failed to lookup %r', uri) return [] - def find_exact(self, **query): + def find_exact(self, query=None, uris=None): + # TODO Only return results within URI roots given by ``uris`` + + if query is None: + query = {} self._validate_query(query) result_tracks = self._uri_mapping.values() @@ -72,7 +76,11 @@ class LocalLibraryProvider(base.BaseLibraryProvider): raise LookupError('Invalid lookup field: %s' % field) return SearchResult(uri='file:search', tracks=result_tracks) - def search(self, **query): + def search(self, query=None, uris=None): + # TODO Only return results within URI roots given by ``uris`` + + if query is None: + query = {} self._validate_query(query) result_tracks = self._uri_mapping.values() diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index 96e5f616..7afde913 100644 --- a/mopidy/backends/spotify/library.py +++ b/mopidy/backends/spotify/library.py @@ -62,8 +62,8 @@ class SpotifyTrack(Track): class SpotifyLibraryProvider(base.BaseLibraryProvider): - def find_exact(self, **query): - return self.search(**query) + def find_exact(self, query=None, uris=None): + return self.search(query=query, uris=uris) def lookup(self, uri): try: @@ -131,7 +131,9 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): def refresh(self, uri=None): pass # TODO - def search(self, **query): + def search(self, query=None, uris=None): + # TODO Only return results within URI roots given by ``uris`` + if not query: return self._get_all_tracks() diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 29b2e797..50d75144 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +from collections import defaultdict import urlparse import pykka @@ -16,34 +17,60 @@ class LibraryController(object): uri_scheme = urlparse.urlparse(uri).scheme return self.backends.with_library_by_uri_scheme.get(uri_scheme, None) - def find_exact(self, query=None, **kwargs): + def _get_backends_to_uris(self, uris): + if uris: + backends_to_uris = defaultdict(list) + for uri in uris: + backend = self._get_backend(uri) + if backend is not None: + backends_to_uris[backend].append(uri) + else: + backends_to_uris = dict([ + (b, None) for b in self.backends.with_library]) + return backends_to_uris + + def find_exact(self, query=None, uris=None, **kwargs): """ Search the library for tracks where ``field`` is ``values``. If the query is empty, and the backend can support it, all available tracks are returned. + If ``uris`` is given, the search is limited to results from within the + URI roots. For example passing ``uris=['file:']`` will limit the search + to the local backend. + Examples:: - # Returns results matching 'a' + # Returns results matching 'a' from any backend find_exact({'any': ['a']}) find_exact(any=['a']) - # Returns results matching artist 'xyz' + # Returns results matching artist 'xyz' from any backend find_exact({'artist': ['xyz']}) find_exact(artist=['xyz']) - # Returns results matching 'a' and 'b' and artist 'xyz' + # Returns results matching 'a' and 'b' and artist 'xyz' from any + # backend find_exact({'any': ['a', 'b'], 'artist': ['xyz']}) find_exact(any=['a', 'b'], artist=['xyz']) + # Returns results matching 'a' if within the given URI roots + # "file:///media/music" and "spotify:" + find_exact( + {'any': ['a']}, uris=['file:///media/music', 'spotify:']) + find_exact(any=['a'], uris=['file:///media/music', 'spotify:']) + :param query: one or more queries to search for :type query: dict + :param uris: zero or more URI roots to limit the search to + :type uris: list of strings or :class:`None` :rtype: list of :class:`mopidy.models.SearchResult` """ query = query or kwargs futures = [ - b.library.find_exact(**query) for b in self.backends.with_library] + backend.library.find_exact(query=query, uris=uris) + for (backend, uris) in self._get_backends_to_uris(uris).items()] return [result for result in pykka.get_all(futures) if result] def lookup(self, uri): @@ -79,32 +106,45 @@ class LibraryController(object): b.library.refresh(uri) for b in self.backends.with_library] pykka.get_all(futures) - def search(self, query=None, **kwargs): + def search(self, query=None, uris=None, **kwargs): """ Search the library for tracks where ``field`` contains ``values``. If the query is empty, and the backend can support it, all available tracks are returned. + If ``uris`` is given, the search is limited to results from within the + URI roots. For example passing ``uris=['file:']`` will limit the search + to the local backend. + Examples:: - # Returns results matching 'a' + # Returns results matching 'a' in any backend search({'any': ['a']}) search(any=['a']) - # Returns results matching artist 'xyz' + # Returns results matching artist 'xyz' in any backend search({'artist': ['xyz']}) search(artist=['xyz']) - # Returns results matching 'a' and 'b' and artist 'xyz' + # Returns results matching 'a' and 'b' and artist 'xyz' in any + # backend search({'any': ['a', 'b'], 'artist': ['xyz']}) search(any=['a', 'b'], artist=['xyz']) + # Returns results matching 'a' if within the given URI roots + # "file:///media/music" and "spotify:" + search({'any': ['a']}, uris=['file:///media/music', 'spotify:']) + search(any=['a'], uris=['file:///media/music', 'spotify:']) + :param query: one or more queries to search for :type query: dict + :param uris: zero or more URI roots to limit the search to + :type uris: list of strings or :class:`None` :rtype: list of :class:`mopidy.models.SearchResult` """ query = query or kwargs futures = [ - b.library.search(**query) for b in self.backends.with_library] + backend.library.search(query=query, uris=uris) + for (backend, uris) in self._get_backends_to_uris(uris).items()] return [result for result in pykka.get_all(futures) if result] diff --git a/tests/core/library_test.py b/tests/core/library_test.py index e01696c7..6e9d240a 100644 --- a/tests/core/library_test.py +++ b/tests/core/library_test.py @@ -87,8 +87,27 @@ class CoreLibraryTest(unittest.TestCase): self.assertIn(result1, result) self.assertIn(result2, result) - self.library1.find_exact.assert_called_once_with(any=['a']) - self.library2.find_exact.assert_called_once_with(any=['a']) + self.library1.find_exact.assert_called_once_with( + query=dict(any=['a']), uris=None) + self.library2.find_exact.assert_called_once_with( + query=dict(any=['a']), uris=None) + + def test_find_exact_with_uris_selects_dummy1_backend(self): + self.core.library.find_exact( + any=['a'], uris=['dummy1:', 'dummy1:foo', 'dummy3:']) + + self.library1.find_exact.assert_called_once_with( + query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo']) + self.assertFalse(self.library2.find_exact.called) + + def test_find_exact_with_uris_selects_both_backends(self): + self.core.library.find_exact( + any=['a'], uris=['dummy1:', 'dummy1:foo', 'dummy2:']) + + self.library1.find_exact.assert_called_once_with( + query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo']) + self.library2.find_exact.assert_called_once_with( + query=dict(any=['a']), uris=['dummy2:']) def test_find_exact_filters_out_none(self): track1 = Track(uri='dummy1:a') @@ -103,8 +122,10 @@ class CoreLibraryTest(unittest.TestCase): self.assertIn(result1, result) self.assertNotIn(None, result) - self.library1.find_exact.assert_called_once_with(any=['a']) - self.library2.find_exact.assert_called_once_with(any=['a']) + self.library1.find_exact.assert_called_once_with( + query=dict(any=['a']), uris=None) + self.library2.find_exact.assert_called_once_with( + query=dict(any=['a']), uris=None) def test_find_accepts_query_dict_instead_of_kwargs(self): track1 = Track(uri='dummy1:a') @@ -121,8 +142,10 @@ class CoreLibraryTest(unittest.TestCase): self.assertIn(result1, result) self.assertIn(result2, result) - self.library1.find_exact.assert_called_once_with(any=['a']) - self.library2.find_exact.assert_called_once_with(any=['a']) + self.library1.find_exact.assert_called_once_with( + query=dict(any=['a']), uris=None) + self.library2.find_exact.assert_called_once_with( + query=dict(any=['a']), uris=None) def test_search_combines_results_from_all_backends(self): track1 = Track(uri='dummy1:a') @@ -139,8 +162,27 @@ class CoreLibraryTest(unittest.TestCase): self.assertIn(result1, result) self.assertIn(result2, result) - self.library1.search.assert_called_once_with(any=['a']) - self.library2.search.assert_called_once_with(any=['a']) + self.library1.search.assert_called_once_with( + query=dict(any=['a']), uris=None) + self.library2.search.assert_called_once_with( + query=dict(any=['a']), uris=None) + + def test_search_with_uris_selects_dummy1_backend(self): + self.core.library.search( + query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo', 'dummy3:']) + + self.library1.search.assert_called_once_with( + query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo']) + self.assertFalse(self.library2.search.called) + + def test_search_with_uris_selects_both_backends(self): + self.core.library.search( + query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo', 'dummy2:']) + + self.library1.search.assert_called_once_with( + query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo']) + self.library2.search.assert_called_once_with( + query=dict(any=['a']), uris=['dummy2:']) def test_search_filters_out_none(self): track1 = Track(uri='dummy1:a') @@ -155,8 +197,10 @@ class CoreLibraryTest(unittest.TestCase): self.assertIn(result1, result) self.assertNotIn(None, result) - self.library1.search.assert_called_once_with(any=['a']) - self.library2.search.assert_called_once_with(any=['a']) + self.library1.search.assert_called_once_with( + query=dict(any=['a']), uris=None) + self.library2.search.assert_called_once_with( + query=dict(any=['a']), uris=None) def test_search_accepts_query_dict_instead_of_kwargs(self): track1 = Track(uri='dummy1:a') @@ -173,5 +217,7 @@ class CoreLibraryTest(unittest.TestCase): self.assertIn(result1, result) self.assertIn(result2, result) - self.library1.search.assert_called_once_with(any=['a']) - self.library2.search.assert_called_once_with(any=['a']) + self.library1.search.assert_called_once_with( + query=dict(any=['a']), uris=None) + self.library2.search.assert_called_once_with( + query=dict(any=['a']), uris=None)