From 7ec23429212d819b05defb68793cf568134a9b24 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Sun, 22 Mar 2015 23:03:02 +0100
Subject: [PATCH] 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.
---
 docs/changelog.rst         |  4 ++++
 mopidy/core/library.py     | 20 ++++++++++++++++++--
 mopidy/local/search.py     |  4 ----
 tests/core/test_library.py | 10 ++++++++++
 4 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 91c83006..7261e3fb 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -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
diff --git a/mopidy/core/library.py b/mopidy/core/library.py
index b8018b16..ee0c2e64 100644
--- a/mopidy/core/library.py
+++ b/mopidy/core/library.py
@@ -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
diff --git a/mopidy/local/search.py b/mopidy/local/search.py
index 9d6edea7..fdbe871c 100644
--- a/mopidy/local/search.py
+++ b/mopidy/local/search.py
@@ -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':
diff --git a/tests/core/test_library.py b/tests/core/test_library.py
index 9eacd1a2..9a23d874 100644
--- a/tests/core/test_library.py
+++ b/tests/core/test_library.py
@@ -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)