From 97515c8125436fc8b7694823baa4b9ffeb805e33 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Sun, 12 Apr 2015 23:59:20 +0200
Subject: [PATCH 01/10] mpd: Only short circuit 'add "uri"' case when we have a
 URI scheme

---
 mopidy/mpd/protocol/current_playlist.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py
index 38ad4017..ea815c6a 100644
--- a/mopidy/mpd/protocol/current_playlist.py
+++ b/mopidy/mpd/protocol/current_playlist.py
@@ -1,5 +1,7 @@
 from __future__ import absolute_import, unicode_literals
 
+import urlparse
+
 from mopidy.mpd import exceptions, protocol, translator
 from mopidy.utils import deprecation
 
@@ -21,8 +23,11 @@ def add(context, uri):
     if not uri.strip('/'):
         return
 
-    if context.core.tracklist.add(uris=[uri]).get():
-        return
+    # If we have an URI just try and add it directly without bothering with
+    # jumping through browse...
+    if urlparse.urlparse(uri).scheme != '':
+        if context.core.tracklist.add(uris=[uri]).get():
+            return
 
     try:
         uris = []

From 8c7a9e3f958e80ca730c757796b76db97069460b Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Mon, 13 Apr 2015 00:02:00 +0200
Subject: [PATCH 02/10] mpd: 'list "artist" ""' should not generate an invalid
 query

---
 mopidy/mpd/protocol/music_db.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py
index fc726255..541fcd6d 100644
--- a/mopidy/mpd/protocol/music_db.py
+++ b/mopidy/mpd/protocol/music_db.py
@@ -270,10 +270,12 @@ def list_(context, *args):
     if field not in _LIST_MAPPING:
         raise exceptions.MpdArgError('incorrect arguments')
 
+    query = None
     if len(params) == 1:
         if field != 'album':
             raise exceptions.MpdArgError('should be "Album" for 3 arguments')
-        query = {'artist': params}
+        if params[0].strip():
+            query = {'artist': params}
     else:
         try:
             query = _query_from_mpd_search_parameters(params, _LIST_MAPPING)

From 1b10a783d3830b381d59dfdb84c4e764460a4660 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Mon, 13 Apr 2015 00:16:09 +0200
Subject: [PATCH 03/10] mpd: Update tests to use setters and actual booleans

---
 tests/mpd/test_status.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/mpd/test_status.py b/tests/mpd/test_status.py
index 6f134df5..f6390e53 100644
--- a/tests/mpd/test_status.py
+++ b/tests/mpd/test_status.py
@@ -76,7 +76,7 @@ class StatusHandlerTest(unittest.TestCase):
         self.assertEqual(int(result['repeat']), 0)
 
     def test_status_method_contains_repeat_is_1(self):
-        self.core.tracklist.repeat = 1
+        self.core.tracklist.set_repeat(True)
         result = dict(status.status(self.context))
         self.assertIn('repeat', result)
         self.assertEqual(int(result['repeat']), 1)
@@ -87,7 +87,7 @@ class StatusHandlerTest(unittest.TestCase):
         self.assertEqual(int(result['random']), 0)
 
     def test_status_method_contains_random_is_1(self):
-        self.core.tracklist.random = 1
+        self.core.tracklist.set_random(True)
         result = dict(status.status(self.context))
         self.assertIn('random', result)
         self.assertEqual(int(result['random']), 1)
@@ -103,7 +103,7 @@ class StatusHandlerTest(unittest.TestCase):
         self.assertEqual(int(result['consume']), 0)
 
     def test_status_method_contains_consume_is_1(self):
-        self.core.tracklist.consume = 1
+        self.core.tracklist.set_consume(True)
         result = dict(status.status(self.context))
         self.assertIn('consume', result)
         self.assertEqual(int(result['consume']), 1)

From 94628b5f8232382a583d8c7bd991d0a46d9127a8 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Mon, 13 Apr 2015 00:50:33 +0200
Subject: [PATCH 04/10] local: Don't use tuple form of TlTracks in tests

---
 tests/local/test_playback.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/local/test_playback.py b/tests/local/test_playback.py
index 2bda46d3..20601d93 100644
--- a/tests/local/test_playback.py
+++ b/tests/local/test_playback.py
@@ -10,7 +10,7 @@ import pykka
 from mopidy import core
 from mopidy.core import PlaybackState
 from mopidy.local import actor
-from mopidy.models import Track
+from mopidy.models import TlTrack, Track
 from mopidy.utils import deprecation
 
 from tests import dummy_audio, path_to_data_dir
@@ -1088,4 +1088,4 @@ class LocalPlaybackProviderTest(unittest.TestCase):
     @populate_tracklist
     def test_playing_track_that_isnt_in_playlist(self):
         with self.assertRaises(AssertionError):
-            self.playback.play((17, Track()))
+            self.playback.play(TlTrack(17, Track()))

From 8646ba42527dfea5c939147710bae351c84fa939 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Tue, 14 Apr 2015 23:16:12 +0200
Subject: [PATCH 05/10] utils: Add validation helpers for verifying core APIs

---
 mopidy/exceptions.py           |   4 +
 mopidy/utils/validation.py     | 102 +++++++++++++++++++++
 tests/utils/test_validation.py | 163 +++++++++++++++++++++++++++++++++
 3 files changed, 269 insertions(+)
 create mode 100644 mopidy/utils/validation.py
 create mode 100644 tests/utils/test_validation.py

diff --git a/mopidy/exceptions.py b/mopidy/exceptions.py
index 32a2bd9a..d02a288a 100644
--- a/mopidy/exceptions.py
+++ b/mopidy/exceptions.py
@@ -46,3 +46,7 @@ class ScannerError(MopidyException):
 
 class AudioException(MopidyException):
     pass
+
+
+class ValidationError(ValueError):
+    pass
diff --git a/mopidy/utils/validation.py b/mopidy/utils/validation.py
new file mode 100644
index 00000000..a0306564
--- /dev/null
+++ b/mopidy/utils/validation.py
@@ -0,0 +1,102 @@
+from __future__ import absolute_import, unicode_literals
+
+import collections
+import urlparse
+
+from mopidy import compat, exceptions
+
+PLAYBACK_STATES = {'paused', 'stopped', 'playing'}
+
+QUERY_FIELDS = {
+    'uri', 'track_name', 'album', 'artist', 'albumartist', 'composer',
+    'performer', 'track_no', 'genre', 'date', 'comment', 'any', 'tlid', 'name'}
+
+DISTINCT_FIELDS = {
+    'artist', 'albumartist', 'album', 'composer', 'performer', 'date', 'genre'}
+
+
+# TODO: _check_iterable(check, msg, **kwargs) + [check(a) for a in arg]?
+def _check_iterable(arg, msg, **kwargs):
+    """Ensure we have an iterable which is not a string."""
+    if isinstance(arg, compat.string_types):
+        raise exceptions.ValidationError(msg.format(arg=arg, **kwargs))
+    elif not isinstance(arg, collections.Iterable):
+        raise exceptions.ValidationError(msg.format(arg=arg, **kwargs))
+
+
+def check_choice(arg, choices, msg='Expected one of {choices}, not {arg!r}'):
+    if arg not in choices:
+        raise exceptions.ValidationError(msg.format(
+            arg=arg, choices=tuple(choices)))
+
+
+def check_boolean(arg, msg='Expected a boolean, not {arg!r}'):
+    check_instance(arg, bool, msg=msg)
+
+
+def check_instance(arg, cls, msg='Expected a {name} instance, not {arg!r}'):
+    if not isinstance(arg, cls):
+        raise exceptions.ValidationError(
+            msg.format(arg=arg, name=cls.__name__))
+
+
+def check_instances(arg, cls, msg='Expected a list of {name}, not {arg!r}'):
+    _check_iterable(arg, msg, name=cls.__name__)
+    if not all(isinstance(instance, cls) for instance in arg):
+        raise exceptions.ValidationError(
+            msg.format(arg=arg, name=cls.__name__))
+
+
+def check_integer(arg, min=None, max=None):
+    if not isinstance(arg, (int, long)):
+        raise exceptions.ValidationError('Expected an integer, not %r' % arg)
+    elif min is not None and arg < min:
+        raise exceptions.ValidationError(
+            'Expected number larger or equal to %d, not %r' % (min, arg))
+    elif max is not None and arg > max:
+        raise exceptions.ValidationError(
+            'Expected number smaller or equal to %d, not %r' % (max, arg))
+
+
+def check_query(arg, list_values=True):
+    # TODO: normalize name  -> track_name
+    # TODO: normalize value -> [value]
+    # TODO: normalize blank -> [] or just remove field?
+    # TODO: normalize int -> str or remove int support?
+    # TODO: remove list_values?
+    # TODO: don't allow for instance tlid field in all queries?
+
+    if not isinstance(arg, collections.Mapping):
+        raise exceptions.ValidationError(
+            'Expected a query dictionary, not {arg!r}'.format(arg=arg))
+
+    for key, value in arg.items():
+        check_choice(key, QUERY_FIELDS, msg='Expected query field to be one '
+                     'of {choices}, not {arg!r}')
+        if list_values:
+            msg = 'Expected "{key}" values to be list of strings, not {arg!r}'
+            _check_iterable(value, msg, key=key)
+            [_check_query_value(key, v, msg) for v in value]
+        else:
+            _check_query_value(key, value, 'Expected "{key}" value to be a '
+                               'string, not {arg!r}')
+
+
+def _check_query_value(key, arg, msg):
+    if isinstance(arg, compat.string_types):
+        if not arg.strip():
+            raise exceptions.ValidationError(msg.format(arg=arg, key=key))
+    elif not isinstance(arg, (int, long)):
+        raise exceptions.ValidationError(msg.format(arg=arg, key=key))
+
+
+def check_uri(arg, msg='Expected a valid URI, not {arg!r}'):
+    if not isinstance(arg, compat.string_types):
+        raise exceptions.ValidationError(msg.format(arg=arg))
+    elif urlparse.urlparse(arg).scheme == '':
+        raise exceptions.ValidationError(msg.format(arg=arg))
+
+
+def check_uris(arg, msg='Expected a list of URIs, not {arg!r}'):
+    _check_iterable(arg, msg)
+    [check_uri(a, msg) for a in arg]
diff --git a/tests/utils/test_validation.py b/tests/utils/test_validation.py
new file mode 100644
index 00000000..d55a918e
--- /dev/null
+++ b/tests/utils/test_validation.py
@@ -0,0 +1,163 @@
+from __future__ import absolute_import, unicode_literals
+
+from pytest import raises
+
+from mopidy import compat, exceptions
+from mopidy.utils import validation
+
+
+def test_check_boolean_with_valid_values():
+    for value in (True, False):
+        validation.check_boolean(value)
+
+
+def test_check_boolean_with_truthy_values():
+    for value in 1, 0, None, '', list(), tuple():
+        with raises(exceptions.ValidationError):
+            validation.check_boolean(value)
+
+
+def test_check_boolean_error_message():
+    with raises(exceptions.ValidationError) as excinfo:
+        validation.check_boolean(1234)
+    assert 'Expected a boolean, not 1234' == str(excinfo.value)
+
+
+def test_check_choice_with_valid_values():
+    for value, choices in (2, (1, 2, 3)), ('abc', ('abc', 'def')):
+        validation.check_choice(value, choices)
+
+
+def test_check_choice_with_invalid_values():
+    for value, choices in (5, (1, 2, 3)), ('xyz', ('abc', 'def')):
+        with raises(exceptions.ValidationError):
+            validation.check_choice(value, choices)
+
+
+def test_check_choice_error_message():
+    with raises(exceptions.ValidationError) as excinfo:
+        validation.check_choice(5, (1, 2, 3))
+    assert 'Expected one of (1, 2, 3), not 5' == str(excinfo.value)
+
+
+def test_check_instance_with_valid_choices():
+    for value, cls in ((True, bool), ('a', compat.text_type), (123, int)):
+        validation.check_instance(value, cls)
+
+
+def test_check_instance_with_invalid_values():
+    for value, cls in (1, str), ('abc', int):
+        with raises(exceptions.ValidationError):
+            validation.check_instance(value, cls)
+
+
+def test_check_instance_error_message():
+    with raises(exceptions.ValidationError) as excinfo:
+        validation.check_instance(1, dict)
+    assert 'Expected a dict instance, not 1' == str(excinfo.value)
+
+
+def test_check_instances_with_valid_values():
+    validation.check_instances([], int)
+    validation.check_instances([1, 2], int)
+    validation.check_instances((1, 2), int)
+
+
+def test_check_instances_with_invalid_values():
+    with raises(exceptions.ValidationError):
+        validation.check_instances('abc', compat.string_types)
+    with raises(exceptions.ValidationError):
+        validation.check_instances(['abc', 123], compat.string_types)
+    with raises(exceptions.ValidationError):
+        validation.check_instances(None, compat.string_types)
+    with raises(exceptions.ValidationError):
+        validation.check_instances([None], compat.string_types)
+
+
+def test_check_instances_error_message():
+    with raises(exceptions.ValidationError) as excinfo:
+        validation.check_instances([1], compat.string_types)
+    assert 'Expected a list of basestring, not [1]' == str(excinfo.value)
+
+
+def test_check_query_valid_values():
+    for value in {}, {'any': []}, {'any': ['abc']}:
+        validation.check_query(value)
+
+
+def test_check_query_random_iterables():
+    for value in None, tuple(), list(), 'abc':
+        with raises(exceptions.ValidationError):
+            validation.check_query(value)
+
+
+def test_check_mapping_error_message():
+    with raises(exceptions.ValidationError) as excinfo:
+        validation.check_query([])
+    assert 'Expected a query dictionary, not []' == str(excinfo.value)
+
+
+def test_check_query_invalid_fields():
+    for value in 'wrong', 'bar', 'foo':
+        with raises(exceptions.ValidationError):
+            validation.check_query({value: []})
+
+
+def test_check_field_error_message():
+    with raises(exceptions.ValidationError) as excinfo:
+        validation.check_query({'wrong': ['abc']})
+    assert 'Expected query field to be one of ' in str(excinfo.value)
+
+
+def test_check_query_invalid_values():
+    for value in '', None, 'foo', 123, [''], [None]:
+        with raises(exceptions.ValidationError):
+            validation.check_query({'any': value})
+
+
+def test_check_values_error_message():
+    with raises(exceptions.ValidationError) as excinfo:
+        validation.check_query({'any': 'abc'})
+    assert 'Expected "any" values to be list of strings' in str(excinfo.value)
+
+
+def test_check_uri_with_valid_values():
+    for value in 'foobar:', 'http://example.com', 'git+http://example.com':
+        validation.check_uri(value)
+
+
+def test_check_uri_with_invalid_values():
+    # Note that tuple catches a potential bug with using "'foo' % arg" for
+    # formatting.
+    for value in ('foobar', 'htt p://example.com', None, 1234, tuple()):
+        with raises(exceptions.ValidationError):
+            validation.check_uri(value)
+
+
+def test_check_uri_error_message():
+    with raises(exceptions.ValidationError) as excinfo:
+        validation.check_uri('testing')
+    assert "Expected a valid URI, not u'testing'" == str(excinfo.value)
+
+
+def test_check_uris_with_valid_values():
+    validation.check_uris([])
+    validation.check_uris(['foobar:'])
+    validation.check_uris(('foobar:',))
+
+
+def test_check_uris_with_invalid_values():
+    with raises(exceptions.ValidationError):
+        validation.check_uris('foobar:')
+    with raises(exceptions.ValidationError):
+        validation.check_uris(None)
+    with raises(exceptions.ValidationError):
+        validation.check_uris([None])
+    with raises(exceptions.ValidationError):
+        validation.check_uris(['foobar:', 'foobar'])
+
+
+def test_check_uris_error_message():
+    with raises(exceptions.ValidationError) as excinfo:
+        validation.check_uris('testing')
+    assert "Expected a list of URIs, not u'testing'" == str(excinfo.value)

From 324bec1f4a28e5f808e42a9238549a3f8e2f2aea Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Tue, 14 Apr 2015 23:20:06 +0200
Subject: [PATCH 06/10] core: Validate core API calls

---
 mopidy/core/library.py         | 23 ++++++++++++--
 mopidy/core/mixer.py           |  5 +++
 mopidy/core/playback.py        |  8 ++++-
 mopidy/core/playlists.py       | 16 ++++++++--
 mopidy/core/tracklist.py       | 37 ++++++++++++++--------
 mopidy/utils/validation.py     |  6 ++--
 tests/local/test_library.py    | 56 ++++++++++++++++++----------------
 tests/local/test_playback.py   | 16 ----------
 tests/utils/test_validation.py |  2 +-
 9 files changed, 105 insertions(+), 64 deletions(-)

diff --git a/mopidy/core/library.py b/mopidy/core/library.py
index 6fc1ce38..5971ec6e 100644
--- a/mopidy/core/library.py
+++ b/mopidy/core/library.py
@@ -5,8 +5,7 @@ import logging
 import operator
 import urlparse
 
-
-from mopidy.utils import deprecation
+from mopidy.utils import deprecation, validation
 
 
 logger = logging.getLogger(__name__)
@@ -70,6 +69,9 @@ class LibraryController(object):
         """
         if uri is None:
             return self._roots()
+        elif not uri.strip():
+            return []
+        validation.check_uri(uri)
         return self._browse(uri)
 
     def _roots(self):
@@ -111,6 +113,9 @@ class LibraryController(object):
 
         .. versionadded:: 1.0
         """
+        validation.check_choice(field, validation.DISTINCT_FIELDS)
+        query is None or validation.check_query(query)  # TODO: normalize?
+
         result = set()
         futures = {b: b.library.get_distinct(field, query)
                    for b in self.backends.with_library.values()}
@@ -137,6 +142,8 @@ class LibraryController(object):
 
         .. versionadded:: 1.0
         """
+        validation.check_uris(uris)
+
         futures = {
             backend: backend.library.get_images(backend_uris)
             for (backend, backend_uris)
@@ -187,6 +194,11 @@ class LibraryController(object):
         if none_set or both_set:
             raise ValueError("One of 'uri' or 'uris' must be set")
 
+        # TODO: validation.one_of(*args)?
+
+        uris is None or validation.check_uris(uris)
+        uri is None or validation.check_uri(uri)
+
         if uri:
             deprecation.warn('core.library.lookup:uri_arg')
 
@@ -219,6 +231,8 @@ class LibraryController(object):
         :param uri: directory or track URI
         :type uri: string
         """
+        uri is None or validation.check_uri(uri)
+
         futures = {}
         backends = {}
         uri_scheme = urlparse.urlparse(uri).scheme if uri else None
@@ -285,6 +299,10 @@ class LibraryController(object):
         """
         query = _normalize_query(query or kwargs)
 
+        uris is None or validation.check_uris(uris)
+        query is None or validation.check_query(query)
+        validation.check_boolean(exact)
+
         if kwargs:
             deprecation.warn('core.library.search:kwargs_query')
 
@@ -319,6 +337,7 @@ class LibraryController(object):
 
 def _normalize_query(query):
     broken_client = False
+    # TODO: this breaks if query is not a dictionary like object...
     for (field, values) in query.items():
         if isinstance(values, basestring):
             broken_client = True
diff --git a/mopidy/core/mixer.py b/mopidy/core/mixer.py
index 3388d706..fde7ee5a 100644
--- a/mopidy/core/mixer.py
+++ b/mopidy/core/mixer.py
@@ -2,6 +2,8 @@ from __future__ import absolute_import, unicode_literals
 
 import logging
 
+from mopidy.utils import validation
+
 
 logger = logging.getLogger(__name__)
 
@@ -31,6 +33,8 @@ class MixerController(object):
 
         Returns :class:`True` if call is successful, otherwise :class:`False`.
         """
+        validation.check_integer(volume, min=0, max=100)
+
         if self._mixer is None:
             return False
         else:
@@ -52,6 +56,7 @@ class MixerController(object):
 
         Returns :class:`True` if call is successful, otherwise :class:`False`.
         """
+        validation.check_boolean(mute)
         if self._mixer is None:
             return False
         else:
diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py
index abf9ae8a..135e1828 100644
--- a/mopidy/core/playback.py
+++ b/mopidy/core/playback.py
@@ -3,9 +3,10 @@ from __future__ import absolute_import, unicode_literals
 import logging
 import urlparse
 
+from mopidy import models
 from mopidy.audio import PlaybackState
 from mopidy.core import listener
-from mopidy.utils import deprecation
+from mopidy.utils import deprecation, validation
 
 logger = logging.getLogger(__name__)
 
@@ -96,6 +97,8 @@ class PlaybackController(object):
             "PAUSED" -> "PLAYING" [ label="resume" ]
             "PAUSED" -> "STOPPED" [ label="stop" ]
         """
+        validation.check_choice(new_state, validation.PLAYBACK_STATES)
+
         (old_state, self._state) = (self.get_state(), new_state)
         logger.debug('Changing state: %s -> %s', old_state, new_state)
 
@@ -270,6 +273,7 @@ class PlaybackController(object):
         :param tl_track: track to play
         :type tl_track: :class:`mopidy.models.TlTrack` or :class:`None`
         """
+        tl_track is None or validation.check_instance(tl_track, models.TlTrack)
         self._play(tl_track, on_error_step=1)
 
     def _play(self, tl_track=None, on_error_step=1):
@@ -360,6 +364,8 @@ class PlaybackController(object):
         :type time_position: int
         :rtype: :class:`True` if successful, else :class:`False`
         """
+        validation.check_integer(time_position, min=0)
+
         if not self.core.tracklist.tracks:
             return False
 
diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py
index 62001517..a430e3ff 100644
--- a/mopidy/core/playlists.py
+++ b/mopidy/core/playlists.py
@@ -5,7 +5,7 @@ import urlparse
 
 from mopidy.core import listener
 from mopidy.models import Playlist
-from mopidy.utils import deprecation
+from mopidy.utils import deprecation, validation
 
 logger = logging.getLogger(__name__)
 
@@ -62,6 +62,8 @@ class PlaylistsController(object):
 
         .. versionadded:: 1.0
         """
+        validation.check_uri(uri)
+
         uri_scheme = urlparse.urlparse(uri).scheme
         backend = self.backends.with_playlists.get(uri_scheme, None)
         if backend:
@@ -139,6 +141,8 @@ class PlaylistsController(object):
         :param uri: URI of the playlist to delete
         :type uri: string
         """
+        validation.check_uri(uri)
+
         uri_scheme = urlparse.urlparse(uri).scheme
         backend = self.backends.with_playlists.get(uri_scheme, None)
         if backend:
@@ -172,6 +176,9 @@ class PlaylistsController(object):
         deprecation.warn('core.playlists.filter')
 
         criteria = criteria or kwargs
+        validation.check_query(criteria, list_values=False)
+
+        # TODO: stop using self playlists
         matches = self.playlists
         for (key, value) in criteria.iteritems():
             matches = filter(lambda p: getattr(p, key) == value, matches)
@@ -207,6 +214,8 @@ class PlaylistsController(object):
         :param uri_scheme: limit to the backend matching the URI scheme
         :type uri_scheme: string
         """
+        # TODO: check: uri_scheme is None or uri_scheme?
+
         futures = {}
         backends = {}
         playlists_loaded = False
@@ -251,8 +260,11 @@ class PlaylistsController(object):
         :type playlist: :class:`mopidy.models.Playlist`
         :rtype: :class:`mopidy.models.Playlist` or :class:`None`
         """
+        validation.check_instance(playlist, Playlist)
+
         if playlist.uri is None:
-            return
+            return  # TODO: log this problem?
+
         uri_scheme = urlparse.urlparse(playlist.uri).scheme
         backend = self.backends.with_playlists.get(uri_scheme, None)
         if backend:
diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py
index 9a251b75..359efa0c 100644
--- a/mopidy/core/tracklist.py
+++ b/mopidy/core/tracklist.py
@@ -1,13 +1,11 @@
 from __future__ import absolute_import, unicode_literals
 
-import collections
 import logging
 import random
 
-from mopidy import compat
 from mopidy.core import listener
-from mopidy.models import TlTrack
-from mopidy.utils import deprecation
+from mopidy.models import TlTrack, Track
+from mopidy.utils import deprecation, validation
 
 logger = logging.getLogger(__name__)
 
@@ -93,6 +91,7 @@ class TracklistController(object):
         :class:`False`
             Tracks are not removed from the tracklist.
         """
+        validation.check_boolean(value)
         if self.get_consume() != value:
             self._trigger_options_changed()
         return setattr(self, '_consume', value)
@@ -121,7 +120,7 @@ class TracklistController(object):
         :class:`False`
             Tracks are played in the order of the tracklist.
         """
-
+        validation.check_boolean(value)
         if self.get_random() != value:
             self._trigger_options_changed()
         if value:
@@ -157,7 +156,7 @@ class TracklistController(object):
         :class:`False`
             The tracklist is played once.
         """
-
+        validation.check_boolean(value)
         if self.get_repeat() != value:
             self._trigger_options_changed()
         return setattr(self, '_repeat', value)
@@ -188,6 +187,7 @@ class TracklistController(object):
         :class:`False`
             Playback continues after current song.
         """
+        validation.check_boolean(value)
         if self.get_single() != value:
             self._trigger_options_changed()
         return setattr(self, '_single', value)
@@ -205,9 +205,10 @@ class TracklistController(object):
         The position of the given track in the tracklist.
 
         :param tl_track: the track to find the index of
-        :type tl_track: :class:`mopidy.models.TlTrack`
+        :type tl_track: :class:`mopidy.models.TlTrack` or :class:`None`
         :rtype: :class:`int` or :class:`None`
         """
+        tl_track is None or validation.check_instance(tl_track, TlTrack)
         try:
             return self._tl_tracks.index(tl_track)
         except ValueError:
@@ -223,6 +224,7 @@ class TracklistController(object):
         :type tl_track: :class:`mopidy.models.TlTrack` or :class:`None`
         :rtype: :class:`mopidy.models.TlTrack` or :class:`None`
         """
+        tl_track is None or validation.check_instance(tl_track, TlTrack)
         if self.get_single() and self.get_repeat():
             return tl_track
         elif self.get_single():
@@ -247,6 +249,7 @@ class TracklistController(object):
         :type tl_track: :class:`mopidy.models.TlTrack` or :class:`None`
         :rtype: :class:`mopidy.models.TlTrack` or :class:`None`
         """
+        tl_track is None or validation.check_instance(tl_track, TlTrack)
 
         if not self.get_tl_tracks():
             return None
@@ -288,6 +291,8 @@ class TracklistController(object):
         :type tl_track: :class:`mopidy.models.TlTrack` or :class:`None`
         :rtype: :class:`mopidy.models.TlTrack` or :class:`None`
         """
+        tl_track is None or validation.check_instance(tl_track, TlTrack)
+
         if self.get_repeat() or self.get_consume() or self.get_random():
             return tl_track
 
@@ -330,8 +335,13 @@ class TracklistController(object):
         """
         assert tracks is not None or uri is not None or uris is not None, \
             'tracks, uri or uris must be provided'
+        # TODO: check that only one of tracks uri and uris is set...
+        # TODO: can at_position be negative?
 
-        # TODO: assert that tracks are track instances
+        tracks is None or validation.check_instances(tracks, Track)
+        uri is None or validation.check_uri(uri)
+        uris is None or validation.check_uris(uris)
+        validation.check_integer(at_position or 0)
 
         if tracks:
             deprecation.warn('core.tracklist.add:tracks_arg')
@@ -412,12 +422,11 @@ class TracklistController(object):
         :rtype: list of :class:`mopidy.models.TlTrack`
         """
         criteria = criteria or kwargs
+        validation.check_query(criteria)
+        # TODO: deprecate kwargs
+
         matches = self._tl_tracks
         for (key, values) in criteria.items():
-            if (not isinstance(values, collections.Iterable) or
-                    isinstance(values, compat.string_types)):
-                # Fail hard if anyone is using the <0.17 calling style
-                raise ValueError('Filter values must be iterable: %r' % values)
             if key == 'tlid':
                 matches = [ct for ct in matches if ct.tlid in values]
             else:
@@ -443,6 +452,7 @@ class TracklistController(object):
 
         tl_tracks = self._tl_tracks
 
+        # TODO: use validation helpers?
         assert start < end, 'start must be smaller than end'
         assert start >= 0, 'start must be at least zero'
         assert end <= len(tl_tracks), \
@@ -470,6 +480,7 @@ class TracklistController(object):
         :type criteria: dict
         :rtype: list of :class:`mopidy.models.TlTrack` that was removed
         """
+        # TODO: deprecate kwargs
         tl_tracks = self.filter(criteria, **kwargs)
         for tl_track in tl_tracks:
             position = self._tl_tracks.index(tl_track)
@@ -491,6 +502,7 @@ class TracklistController(object):
         """
         tl_tracks = self._tl_tracks
 
+        # TOOD: use validation helpers?
         if start is not None and end is not None:
             assert start < end, 'start must be smaller than end'
 
@@ -519,6 +531,7 @@ class TracklistController(object):
         :type end: int
         :rtype: :class:`mopidy.models.TlTrack`
         """
+        # TODO: validate slice?
         return self._tl_tracks[start:end]
 
     def _mark_playing(self, tl_track):
diff --git a/mopidy/utils/validation.py b/mopidy/utils/validation.py
index a0306564..25e9f227 100644
--- a/mopidy/utils/validation.py
+++ b/mopidy/utils/validation.py
@@ -74,12 +74,12 @@ def check_query(arg, list_values=True):
         check_choice(key, QUERY_FIELDS, msg='Expected query field to be one '
                      'of {choices}, not {arg!r}')
         if list_values:
-            msg = 'Expected "{key}" values to be list of strings, not {arg!r}'
+            msg = 'Expected "{key}" to be list of strings, not {arg!r}'
             _check_iterable(value, msg, key=key)
             [_check_query_value(key, v, msg) for v in value]
         else:
-            _check_query_value(key, value, 'Expected "{key}" value to be a '
-                               'string, not {arg!r}')
+            _check_query_value(
+                key, value, 'Expected "{key}" to be a string, not {arg!r}')
 
 
 def _check_query_value(key, arg, msg):
diff --git a/tests/local/test_library.py b/tests/local/test_library.py
index 0198ec9e..7763057f 100644
--- a/tests/local/test_library.py
+++ b/tests/local/test_library.py
@@ -9,7 +9,7 @@ import mock
 
 import pykka
 
-from mopidy import core
+from mopidy import core, exceptions
 from mopidy.local import actor, json
 from mopidy.models import Album, Artist, Image, Track
 
@@ -137,8 +137,8 @@ class LocalLibraryProviderTest(unittest.TestCase):
         self.assertEqual(result[uri], self.tracks[0:1])
 
     def test_lookup_unknown_track(self):
-        tracks = self.library.lookup(uris=['fake uri'])
-        self.assertEqual(tracks, {'fake uri': []})
+        tracks = self.library.lookup(uris=['fake:/uri'])
+        self.assertEqual(tracks, {'fake:/uri': []})
 
     # test backward compatibility with local libraries returning a
     # single Track
@@ -343,42 +343,44 @@ class LocalLibraryProviderTest(unittest.TestCase):
         result = self.find_exact(any=['local:track:path1'])
         self.assertEqual(list(result[0].tracks), self.tracks[:1])
 
+    # TODO: This is really just a test of the query validation code now,
+    # as this code path never even makes it to the local backend.
     def test_find_exact_wrong_type(self):
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.find_exact(wrong=['test'])
 
     def test_find_exact_with_empty_query(self):
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.find_exact(artist=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.find_exact(albumartist=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.find_exact(track_name=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.find_exact(composer=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.find_exact(performer=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.find_exact(album=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.find_exact(track_no=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.find_exact(genre=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.find_exact(date=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.find_exact(comment=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.find_exact(any=[''])
 
     def test_search_no_hits(self):
@@ -553,41 +555,41 @@ class LocalLibraryProviderTest(unittest.TestCase):
         self.assertEqual(list(result[0].tracks), self.tracks[:1])
 
     def test_search_wrong_type(self):
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.search(wrong=['test'])
 
     def test_search_with_empty_query(self):
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.search(artist=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.search(albumartist=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.search(composer=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.search(performer=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.search(track_name=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.search(album=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.search(genre=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.search(date=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.search(comment=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.search(uri=[''])
 
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.ValidationError):
             self.search(any=[''])
 
     def test_default_get_images_impl_no_images(self):
diff --git a/tests/local/test_playback.py b/tests/local/test_playback.py
index 20601d93..8aedcfbc 100644
--- a/tests/local/test_playback.py
+++ b/tests/local/test_playback.py
@@ -841,22 +841,6 @@ class LocalPlaybackProviderTest(unittest.TestCase):
         self.playback.seek(self.tracklist.tracks[-1].length * 100)
         self.assertEqual(self.playback.state, PlaybackState.STOPPED)
 
-    @unittest.SkipTest
-    @populate_tracklist
-    def test_seek_beyond_start_of_song(self):
-        # FIXME need to decide return value
-        self.playback.play()
-        result = self.playback.seek(-1000)
-        self.assert_(not result, 'Seek return value was %s' % result)
-
-    @populate_tracklist
-    def test_seek_beyond_start_of_song_update_postion(self):
-        self.playback.play()
-        self.playback.seek(-1000)
-        position = self.playback.time_position
-        self.assertGreaterEqual(position, 0)
-        self.assertEqual(self.playback.state, PlaybackState.PLAYING)
-
     @populate_tracklist
     def test_stop_when_stopped(self):
         self.playback.stop()
diff --git a/tests/utils/test_validation.py b/tests/utils/test_validation.py
index d55a918e..fdebd4d2 100644
--- a/tests/utils/test_validation.py
+++ b/tests/utils/test_validation.py
@@ -118,7 +118,7 @@ def test_check_query_invalid_values():
 def test_check_values_error_message():
     with raises(exceptions.ValidationError) as excinfo:
         validation.check_query({'any': 'abc'})
-    assert 'Expected "any" values to be list of strings' in str(excinfo.value)
+    assert 'Expected "any" to be list of strings, not' in str(excinfo.value)
 
 
 def test_check_uri_with_valid_values():

From 97235f9441bea8bc94a6d41c8b96eb4f230b4524 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Tue, 14 Apr 2015 23:46:20 +0200
Subject: [PATCH 07/10] core: Don't allow TLIDs in queries, or integers

Handle this in tracklist.filter() which is the only API that allows number
and/or TLIDs.
---
 mopidy/core/tracklist.py       | 11 ++++++-----
 mopidy/utils/validation.py     |  9 ++-------
 tests/utils/test_validation.py |  2 +-
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py
index 359efa0c..54143578 100644
--- a/mopidy/core/tracklist.py
+++ b/mopidy/core/tracklist.py
@@ -422,16 +422,17 @@ class TracklistController(object):
         :rtype: list of :class:`mopidy.models.TlTrack`
         """
         criteria = criteria or kwargs
+        tlids = criteria.pop('tlid', [])
         validation.check_query(criteria)
+        validation.check_instances(tlids, int)
         # TODO: deprecate kwargs
 
         matches = self._tl_tracks
         for (key, values) in criteria.items():
-            if key == 'tlid':
-                matches = [ct for ct in matches if ct.tlid in values]
-            else:
-                matches = [
-                    ct for ct in matches if getattr(ct.track, key) in values]
+            matches = [
+                ct for ct in matches if getattr(ct.track, key) in values]
+        if tlids:
+            matches = [ct for ct in matches if ct.tlid in tlids]
         return matches
 
     def move(self, start, end, to_position):
diff --git a/mopidy/utils/validation.py b/mopidy/utils/validation.py
index 25e9f227..c158f340 100644
--- a/mopidy/utils/validation.py
+++ b/mopidy/utils/validation.py
@@ -9,7 +9,7 @@ PLAYBACK_STATES = {'paused', 'stopped', 'playing'}
 
 QUERY_FIELDS = {
     'uri', 'track_name', 'album', 'artist', 'albumartist', 'composer',
-    'performer', 'track_no', 'genre', 'date', 'comment', 'any', 'tlid', 'name'}
+    'performer', 'track_no', 'genre', 'date', 'comment', 'any', 'name'}
 
 DISTINCT_FIELDS = {
     'artist', 'albumartist', 'album', 'composer', 'performer', 'date', 'genre'}
@@ -62,9 +62,7 @@ def check_query(arg, list_values=True):
     # TODO: normalize name  -> track_name
     # TODO: normalize value -> [value]
     # TODO: normalize blank -> [] or just remove field?
-    # TODO: normalize int -> str or remove int support?
     # TODO: remove list_values?
-    # TODO: don't allow for instance tlid field in all queries?
 
     if not isinstance(arg, collections.Mapping):
         raise exceptions.ValidationError(
@@ -83,10 +81,7 @@ def check_query(arg, list_values=True):
 
 
 def _check_query_value(key, arg, msg):
-    if isinstance(arg, compat.string_types):
-        if not arg.strip():
-            raise exceptions.ValidationError(msg.format(arg=arg, key=key))
-    elif not isinstance(arg, (int, long)):
+    if not isinstance(arg, compat.string_types) or not arg.strip():
         raise exceptions.ValidationError(msg.format(arg=arg, key=key))
 
 
diff --git a/tests/utils/test_validation.py b/tests/utils/test_validation.py
index fdebd4d2..d9413686 100644
--- a/tests/utils/test_validation.py
+++ b/tests/utils/test_validation.py
@@ -98,7 +98,7 @@ def test_check_mapping_error_message():
 
 
 def test_check_query_invalid_fields():
-    for value in 'wrong', 'bar', 'foo':
+    for value in 'wrong', 'bar', 'foo', 'tlid':
         with raises(exceptions.ValidationError):
             validation.check_query({value: []})
 

From 2c31dbe47c1f24b9ed061a7a2f2c7beb21b3dc6a Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Wed, 15 Apr 2015 23:42:45 +0200
Subject: [PATCH 08/10] core: Check correct query fields in core

---
 mopidy/core/playlists.py   |  3 ++-
 mopidy/core/tracklist.py   |  3 ++-
 mopidy/utils/validation.py | 15 ++++++++++-----
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py
index a430e3ff..b470fa56 100644
--- a/mopidy/core/playlists.py
+++ b/mopidy/core/playlists.py
@@ -176,7 +176,8 @@ class PlaylistsController(object):
         deprecation.warn('core.playlists.filter')
 
         criteria = criteria or kwargs
-        validation.check_query(criteria, list_values=False)
+        validation.check_query(
+            criteria, validation.PLAYLIST_FIELDS, list_values=False)
 
         # TODO: stop using self playlists
         matches = self.playlists
diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py
index 54143578..21c6d86a 100644
--- a/mopidy/core/tracklist.py
+++ b/mopidy/core/tracklist.py
@@ -423,9 +423,10 @@ class TracklistController(object):
         """
         criteria = criteria or kwargs
         tlids = criteria.pop('tlid', [])
-        validation.check_query(criteria)
+        validation.check_query(criteria, validation.TRACKLIST_FIELDS)
         validation.check_instances(tlids, int)
         # TODO: deprecate kwargs
+        # TODO: id=[1, 2, 3] filtering can't possibly be working
 
         matches = self._tl_tracks
         for (key, values) in criteria.items():
diff --git a/mopidy/utils/validation.py b/mopidy/utils/validation.py
index c158f340..4897f513 100644
--- a/mopidy/utils/validation.py
+++ b/mopidy/utils/validation.py
@@ -7,9 +7,14 @@ from mopidy import compat, exceptions
 
 PLAYBACK_STATES = {'paused', 'stopped', 'playing'}
 
-QUERY_FIELDS = {
+SEARCH_FIELDS = {
     'uri', 'track_name', 'album', 'artist', 'albumartist', 'composer',
-    'performer', 'track_no', 'genre', 'date', 'comment', 'any', 'name'}
+    'performer', 'track_no', 'genre', 'date', 'comment', 'any'}
+
+PLAYLIST_FIELDS = {'uri', 'name'}  # TODO: add length and last_modified?
+
+TRACKLIST_FIELDS = {  # TODO: add bitrate, length, disc_no, track_no, modified?
+    'uri', 'name', 'genre', 'date', 'comment', 'musicbrainz_id'}
 
 DISTINCT_FIELDS = {
     'artist', 'albumartist', 'album', 'composer', 'performer', 'date', 'genre'}
@@ -58,7 +63,7 @@ def check_integer(arg, min=None, max=None):
             'Expected number smaller or equal to %d, not %r' % (max, arg))
 
 
-def check_query(arg, list_values=True):
+def check_query(arg, fields=SEARCH_FIELDS, list_values=True):
     # TODO: normalize name  -> track_name
     # TODO: normalize value -> [value]
     # TODO: normalize blank -> [] or just remove field?
@@ -69,8 +74,8 @@ def check_query(arg, list_values=True):
             'Expected a query dictionary, not {arg!r}'.format(arg=arg))
 
     for key, value in arg.items():
-        check_choice(key, QUERY_FIELDS, msg='Expected query field to be one '
-                     'of {choices}, not {arg!r}')
+        check_choice(key, fields, msg='Expected query field to be one of '
+                     '{choices}, not {arg!r}')
         if list_values:
             msg = 'Expected "{key}" to be list of strings, not {arg!r}'
             _check_iterable(value, msg, key=key)

From 98587f50986fa32a87994fa366904369608dd9ff Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Wed, 15 Apr 2015 23:48:44 +0200
Subject: [PATCH 09/10] review: Fix test name

---
 tests/utils/test_validation.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/utils/test_validation.py b/tests/utils/test_validation.py
index d9413686..67b42a4c 100644
--- a/tests/utils/test_validation.py
+++ b/tests/utils/test_validation.py
@@ -11,7 +11,7 @@ def test_check_boolean_with_valid_values():
         validation.check_boolean(value)
 
 
-def test_check_boolean_with_truthy_values():
+def test_check_boolean_with_other_values():
     for value in 1, 0, None, '', list(), tuple():
         with raises(exceptions.ValidationError):
             validation.check_boolean(value)

From 0b928e787672c6f86c5449566e1e4ecae7ae9dd8 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Wed, 15 Apr 2015 23:51:33 +0200
Subject: [PATCH 10/10] docs: Add core input validation to changelog

---
 docs/changelog.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index a730af21..5ea9ef09 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -13,9 +13,11 @@ Core API
 - Calling :meth:`mopidy.core.library.LibraryController.search`` with ``kwargs``
   as the query is no longer supported (PR: :issue:`1090`)
 
-- Update core controllers to handle backend exceptions in all calls that rely
+- Updated core controllers to handle backend exceptions in all calls that rely
   on multiple backends. (Issue: :issue:`667`)
 
+- Update core methods to do strict input checking. (Fixes: :issue:`#700`)
+
 Models
 ------