core: Normalize search queries
This is needed as otherwise each and every backend needs to handle the fact
that some "bad" clients might send {'field': 'value'} instead of
{'field': ['value']} Though the real problem isn't the clients but our
organically grown query API.
This commit is contained in:
parent
5eebab6e18
commit
7ec2342921
@ -73,6 +73,10 @@ v1.0.0 (UNRELEASED)
|
||||
:meth:`mopidy.backend.PlaybackProvider.change_track` into account when
|
||||
determining success. (PR: :issue:`1071`)
|
||||
|
||||
- Updated :meth:`mopidy.core.LibraryController.search` and
|
||||
:meth:`mopidy.core.LibraryController.find_exact` to normalize and warn about
|
||||
bad queries from clients. (Fixes: :issue:`1067`)
|
||||
|
||||
**Backend API**
|
||||
|
||||
- Remove default implementation of
|
||||
|
||||
@ -1,11 +1,14 @@
|
||||
from __future__ import absolute_import, unicode_literals
|
||||
|
||||
import collections
|
||||
import logging
|
||||
import operator
|
||||
import urlparse
|
||||
|
||||
import pykka
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class LibraryController(object):
|
||||
pykka_traversable = True
|
||||
@ -155,7 +158,7 @@ class LibraryController(object):
|
||||
:type uris: list of strings or :class:`None`
|
||||
:rtype: list of :class:`mopidy.models.SearchResult`
|
||||
"""
|
||||
query = query or kwargs
|
||||
query = _normalize_query(query or kwargs)
|
||||
futures = [
|
||||
backend.library.find_exact(query=query, uris=backend_uris)
|
||||
for (backend, backend_uris)
|
||||
@ -263,9 +266,22 @@ class LibraryController(object):
|
||||
:type uris: list of strings or :class:`None`
|
||||
:rtype: list of :class:`mopidy.models.SearchResult`
|
||||
"""
|
||||
query = query or kwargs
|
||||
query = _normalize_query(query or kwargs)
|
||||
futures = [
|
||||
backend.library.search(query=query, uris=backend_uris)
|
||||
for (backend, backend_uris)
|
||||
in self._get_backends_to_uris(uris).items()]
|
||||
return [result for result in pykka.get_all(futures) if result]
|
||||
|
||||
|
||||
def _normalize_query(query):
|
||||
broken_client = False
|
||||
for (field, values) in query.items():
|
||||
if isinstance(values, basestring):
|
||||
broken_client = True
|
||||
query[field] = [values]
|
||||
if broken_client:
|
||||
logger.warning(
|
||||
'Client sent a broken search query, values must be lists. Please '
|
||||
'check which client sent this query and file a bug against them.')
|
||||
return query
|
||||
|
||||
@ -23,8 +23,6 @@ def find_exact(tracks, query=None, limit=100, offset=0, uris=None):
|
||||
_validate_query(query)
|
||||
|
||||
for (field, values) in query.items():
|
||||
if not hasattr(values, '__iter__'):
|
||||
values = [values]
|
||||
# FIXME this is bound to be slow for large libraries
|
||||
for value in values:
|
||||
if field == 'track_no':
|
||||
@ -134,8 +132,6 @@ def search(tracks, query=None, limit=100, offset=0, uris=None):
|
||||
_validate_query(query)
|
||||
|
||||
for (field, values) in query.items():
|
||||
if not hasattr(values, '__iter__'):
|
||||
values = [values]
|
||||
# FIXME this is bound to be slow for large libraries
|
||||
for value in values:
|
||||
if field == 'track_no':
|
||||
|
||||
@ -355,3 +355,13 @@ class CoreLibraryTest(unittest.TestCase):
|
||||
query=dict(any=['a']), uris=None)
|
||||
self.library2.search.assert_called_once_with(
|
||||
query=dict(any=['a']), uris=None)
|
||||
|
||||
def test_search_normalises_bad_queries(self):
|
||||
self.core.library.search({'any': 'foobar'})
|
||||
self.library1.search.assert_called_once_with(
|
||||
query={'any': ['foobar']}, uris=None)
|
||||
|
||||
def test_find_exact_normalises_bad_queries(self):
|
||||
self.core.library.find_exact({'any': 'foobar'})
|
||||
self.library1.find_exact.assert_called_once_with(
|
||||
query={'any': ['foobar']}, uris=None)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user