From 1704828504f703709d92c43cf148362196ae7f35 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 30 Mar 2013 22:14:33 +0100 Subject: [PATCH 1/4] backends: Make search/find_exact take a query dict instead of kwargs --- mopidy/backends/base.py | 4 ++-- mopidy/backends/local/library.py | 8 +++++-- mopidy/backends/spotify/library.py | 6 ++--- mopidy/core/library.py | 5 +++-- tests/core/library_test.py | 36 ++++++++++++++++++++---------- 5 files changed, 38 insertions(+), 21 deletions(-) diff --git a/mopidy/backends/base.py b/mopidy/backends/base.py index f49aa89b..f7871d44 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): """ See :meth:`mopidy.core.LibraryController.find_exact`. @@ -77,7 +77,7 @@ class BaseLibraryProvider(object): """ pass - def search(self, **query): + def search(self, query=None): """ See :meth:`mopidy.core.LibraryController.search`. diff --git a/mopidy/backends/local/library.py b/mopidy/backends/local/library.py index eb328ce2..ebdd6f9d 100644 --- a/mopidy/backends/local/library.py +++ b/mopidy/backends/local/library.py @@ -35,7 +35,9 @@ class LocalLibraryProvider(base.BaseLibraryProvider): logger.debug('Failed to lookup %r', uri) return [] - def find_exact(self, **query): + def find_exact(self, query=None): + if query is None: + query = {} self._validate_query(query) result_tracks = self._uri_mapping.values() @@ -72,7 +74,9 @@ 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): + 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..0a58a79a 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): + return self.search(query=query) def lookup(self, uri): try: @@ -131,7 +131,7 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): def refresh(self, uri=None): pass # TODO - def search(self, **query): + def search(self, query=None): if not query: return self._get_all_tracks() diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 29b2e797..26216846 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -43,7 +43,8 @@ class LibraryController(object): """ query = query or kwargs futures = [ - b.library.find_exact(**query) for b in self.backends.with_library] + b.library.find_exact(query=query) + for b in self.backends.with_library] return [result for result in pykka.get_all(futures) if result] def lookup(self, uri): @@ -106,5 +107,5 @@ class LibraryController(object): """ query = query or kwargs futures = [ - b.library.search(**query) for b in self.backends.with_library] + b.library.search(query=query) for b in self.backends.with_library] 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..603a8109 100644 --- a/tests/core/library_test.py +++ b/tests/core/library_test.py @@ -87,8 +87,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'])) + self.library2.find_exact.assert_called_once_with( + query=dict(any=['a'])) def test_find_exact_filters_out_none(self): track1 = Track(uri='dummy1:a') @@ -103,8 +105,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'])) + self.library2.find_exact.assert_called_once_with( + query=dict(any=['a'])) def test_find_accepts_query_dict_instead_of_kwargs(self): track1 = Track(uri='dummy1:a') @@ -121,8 +125,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'])) + self.library2.find_exact.assert_called_once_with( + query=dict(any=['a'])) def test_search_combines_results_from_all_backends(self): track1 = Track(uri='dummy1:a') @@ -139,8 +145,10 @@ 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'])) + self.library2.search.assert_called_once_with( + query=dict(any=['a'])) def test_search_filters_out_none(self): track1 = Track(uri='dummy1:a') @@ -155,8 +163,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'])) + self.library2.search.assert_called_once_with( + query=dict(any=['a'])) def test_search_accepts_query_dict_instead_of_kwargs(self): track1 = Track(uri='dummy1:a') @@ -173,5 +183,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'])) + self.library2.search.assert_called_once_with( + query=dict(any=['a'])) From c54db3298f1958a50d0ebdf2327b0e1df4ea15b7 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 30 Mar 2013 22:19:13 +0100 Subject: [PATCH 2/4] backends: Make search/find_exact accept 'uris' kwarg --- mopidy/backends/base.py | 4 ++-- mopidy/backends/local/library.py | 8 ++++++-- mopidy/backends/spotify/library.py | 8 +++++--- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/mopidy/backends/base.py b/mopidy/backends/base.py index f7871d44..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=None): + 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=None): + 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 ebdd6f9d..f2a1a520 100644 --- a/mopidy/backends/local/library.py +++ b/mopidy/backends/local/library.py @@ -35,7 +35,9 @@ class LocalLibraryProvider(base.BaseLibraryProvider): logger.debug('Failed to lookup %r', uri) return [] - def find_exact(self, query=None): + 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) @@ -74,7 +76,9 @@ class LocalLibraryProvider(base.BaseLibraryProvider): raise LookupError('Invalid lookup field: %s' % field) return SearchResult(uri='file:search', tracks=result_tracks) - def search(self, query=None): + 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) diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index 0a58a79a..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=None): - return self.search(query=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=None): + 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() From 2abce2af38dcf3c4c7e16506cf9eb4855115bcd0 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 30 Mar 2013 22:20:12 +0100 Subject: [PATCH 3/4] core: Limit search to backends matching URI roots Fixes #337 --- mopidy/core/library.py | 61 +++++++++++++++++++++++++++++++------- tests/core/library_test.py | 58 ++++++++++++++++++++++++++++-------- 2 files changed, 96 insertions(+), 23 deletions(-) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 26216846..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,35 +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=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): @@ -80,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=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 603a8109..6e9d240a 100644 --- a/tests/core/library_test.py +++ b/tests/core/library_test.py @@ -88,9 +88,26 @@ class CoreLibraryTest(unittest.TestCase): self.assertIn(result1, result) self.assertIn(result2, result) self.library1.find_exact.assert_called_once_with( - query=dict(any=['a'])) + query=dict(any=['a']), uris=None) self.library2.find_exact.assert_called_once_with( - query=dict(any=['a'])) + 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') @@ -106,9 +123,9 @@ class CoreLibraryTest(unittest.TestCase): self.assertIn(result1, result) self.assertNotIn(None, result) self.library1.find_exact.assert_called_once_with( - query=dict(any=['a'])) + query=dict(any=['a']), uris=None) self.library2.find_exact.assert_called_once_with( - query=dict(any=['a'])) + query=dict(any=['a']), uris=None) def test_find_accepts_query_dict_instead_of_kwargs(self): track1 = Track(uri='dummy1:a') @@ -126,9 +143,9 @@ class CoreLibraryTest(unittest.TestCase): self.assertIn(result1, result) self.assertIn(result2, result) self.library1.find_exact.assert_called_once_with( - query=dict(any=['a'])) + query=dict(any=['a']), uris=None) self.library2.find_exact.assert_called_once_with( - query=dict(any=['a'])) + query=dict(any=['a']), uris=None) def test_search_combines_results_from_all_backends(self): track1 = Track(uri='dummy1:a') @@ -146,9 +163,26 @@ class CoreLibraryTest(unittest.TestCase): self.assertIn(result1, result) self.assertIn(result2, result) self.library1.search.assert_called_once_with( - query=dict(any=['a'])) + query=dict(any=['a']), uris=None) self.library2.search.assert_called_once_with( - query=dict(any=['a'])) + 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') @@ -164,9 +198,9 @@ class CoreLibraryTest(unittest.TestCase): self.assertIn(result1, result) self.assertNotIn(None, result) self.library1.search.assert_called_once_with( - query=dict(any=['a'])) + query=dict(any=['a']), uris=None) self.library2.search.assert_called_once_with( - query=dict(any=['a'])) + query=dict(any=['a']), uris=None) def test_search_accepts_query_dict_instead_of_kwargs(self): track1 = Track(uri='dummy1:a') @@ -184,6 +218,6 @@ class CoreLibraryTest(unittest.TestCase): self.assertIn(result1, result) self.assertIn(result2, result) self.library1.search.assert_called_once_with( - query=dict(any=['a'])) + query=dict(any=['a']), uris=None) self.library2.search.assert_called_once_with( - query=dict(any=['a'])) + query=dict(any=['a']), uris=None) From 8ab0f0974342275a8ec2e967264104ea03dbeba6 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 30 Mar 2013 22:27:28 +0100 Subject: [PATCH 4/4] docs: Update changelog --- docs/changes.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index fd065b34..4b81a4e1 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -24,6 +24,19 @@ v0.13.0 (in development) the Mopidy process will now always make it log tracebacks for all alive threads. +- 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: