From 30f56abc6682a3f87c193763a349fc1a041f4f96 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 25 Apr 2015 00:56:11 +0200 Subject: [PATCH 1/2] core: Add "one-of" validation for number of kwargs --- mopidy/core/library.py | 7 +------ mopidy/core/playback.py | 5 ++++- mopidy/core/tracklist.py | 7 +++---- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index ca0a159d..0f4ebac1 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -189,14 +189,9 @@ class LibraryController(object): .. deprecated:: 1.0 The ``uri`` argument. Use ``uris`` instead. """ - none_set = uri is None and uris is None - both_set = uri is not None and uris is not None - - if none_set or both_set: + if sum(o is not None for o in [uri, uris]) != 1: 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) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 2b1bbbc3..3605db0f 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -277,9 +277,12 @@ class PlaybackController(object): :param tlid: TLID of the track to play :type tlid: :class:`int` or :class:`None` """ + if sum(o is not None for o in [tl_track, tlid]) > 1: + raise ValueError('At most one of "tl_track" and "tlid" may be set') + tl_track is None or validation.check_instance(tl_track, models.TlTrack) tlid is None or validation.check_integer(tlid, min=0) - # TODO: check one of or none for args + if tl_track: deprecation.warn('core.playback.play:tl_track_kwarg', pending=True) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index dfcd503a..8fe3c612 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -406,10 +406,9 @@ class TracklistController(object): .. deprecated:: 1.0 The ``tracks`` and ``uri`` arguments. Use ``uris``. """ - 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? + if sum(o is not None for o in [tracks, uri, uris]) != 1: + raise ValueError( + 'Exactly one of tracks, uri or uris must be provided') tracks is None or validation.check_instances(tracks, Track) uri is None or validation.check_uri(uri) From 5f627984a336896ff1add1233c70ad628b8d04bb Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 26 Apr 2015 23:35:00 +0200 Subject: [PATCH 2/2] review: Address review comments for one-of-validation --- mopidy/core/library.py | 2 +- mopidy/core/tracklist.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 0f4ebac1..e6da95f1 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -190,7 +190,7 @@ class LibraryController(object): The ``uri`` argument. Use ``uris`` instead. """ if sum(o is not None for o in [uri, uris]) != 1: - raise ValueError("One of 'uri' or 'uris' must be set") + raise ValueError('Exactly one of "uri" or "uris" must be set') uris is None or validation.check_uris(uris) uri is None or validation.check_uri(uri) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 8fe3c612..692762ef 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -408,7 +408,7 @@ class TracklistController(object): """ if sum(o is not None for o in [tracks, uri, uris]) != 1: raise ValueError( - 'Exactly one of tracks, uri or uris must be provided') + 'Exactly one of "tracks", "uri" or "uris" must be set') tracks is None or validation.check_instances(tracks, Track) uri is None or validation.check_uri(uri)