Merge pull request #364 from jodal/feature/limit-search-by-uri-root

Limit search to only return matches within given URI roots

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:

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`.
 

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

mopidy/backends/spotify/library.py

@@ -62,8 +62,8 @@ class SpotifyTrack(Track):
 
 
 class SpotifyLibraryProvider(base.BaseLibraryProvider):
-    def find_exact(self, **query):
+    def find_exact(self, query=None, uris=None):
-        return self.search(**query)
+        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()
 

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]

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.library1.find_exact.assert_called_once_with(
-        self.library2.find_exact.assert_called_once_with(any=['a'])
+            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.library1.find_exact.assert_called_once_with(
-        self.library2.find_exact.assert_called_once_with(any=['a'])
+            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.library1.find_exact.assert_called_once_with(
-        self.library2.find_exact.assert_called_once_with(any=['a'])
+            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.library1.search.assert_called_once_with(
-        self.library2.search.assert_called_once_with(any=['a'])
+            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.library1.search.assert_called_once_with(
-        self.library2.search.assert_called_once_with(any=['a'])
+            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.library1.search.assert_called_once_with(
-        self.library2.search.assert_called_once_with(any=['a'])
+            query=dict(any=['a']), uris=None)
+        self.library2.search.assert_called_once_with(
+            query=dict(any=['a']), uris=None)