From ac2e413ec03c68b39819292a3ecf28dadd61d470 Mon Sep 17 00:00:00 2001 From: David Eisner Date: Fri, 1 Nov 2013 09:27:31 +0000 Subject: [PATCH 01/32] Advertise MPD with Avahi A rudimentary implementation to resolve #39, ignoring dbus errors (just restart), name collisions (choose a fresh name), etc. --- mopidy/frontends/mpd/__init__.py | 2 ++ mopidy/frontends/mpd/actor.py | 26 ++++++++++++++++++++ mopidy/frontends/mpd/ext.conf | 2 ++ mopidy/frontends/mpd/zeroconf.py | 42 ++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+) create mode 100644 mopidy/frontends/mpd/zeroconf.py diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 276be450..28f9c951 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -23,6 +23,8 @@ class Extension(ext.Extension): schema['password'] = config.Secret(optional=True) schema['max_connections'] = config.Integer(minimum=1) schema['connection_timeout'] = config.Integer(minimum=1) + schema['zeroconf_enabled'] = config.Boolean() + schema['zeroconf_name'] = config.String() return schema def validate_environment(self): diff --git a/mopidy/frontends/mpd/actor.py b/mopidy/frontends/mpd/actor.py index 4d983b73..b48b2b65 100644 --- a/mopidy/frontends/mpd/actor.py +++ b/mopidy/frontends/mpd/actor.py @@ -17,6 +17,9 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): super(MpdFrontend, self).__init__() hostname = network.format_hostname(config['mpd']['hostname']) port = config['mpd']['port'] + self.config = config + self.hostname = hostname + self.port = port try: network.Server( @@ -36,8 +39,31 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): logger.info('MPD server running at [%s]:%s', hostname, port) + def on_start(self): + try: + if self.config['mpd']['zeroconf_enabled']: + name = self.config['mpd']['zeroconf_name'] + import re + lo = re.search('(? Date: Tue, 5 Nov 2013 10:33:04 +0000 Subject: [PATCH 02/32] Avahi python compatibility fix --- mopidy/frontends/mpd/actor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/frontends/mpd/actor.py b/mopidy/frontends/mpd/actor.py index b48b2b65..f2e203c6 100644 --- a/mopidy/frontends/mpd/actor.py +++ b/mopidy/frontends/mpd/actor.py @@ -47,7 +47,7 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): lo = re.search('(? Date: Tue, 5 Nov 2013 10:37:54 +0000 Subject: [PATCH 03/32] Avahi wrapper moved to utils --- mopidy/frontends/mpd/actor.py | 2 +- mopidy/{frontends/mpd => utils}/zeroconf.py | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename mopidy/{frontends/mpd => utils}/zeroconf.py (100%) diff --git a/mopidy/frontends/mpd/actor.py b/mopidy/frontends/mpd/actor.py index f2e203c6..ab5bd35d 100644 --- a/mopidy/frontends/mpd/actor.py +++ b/mopidy/frontends/mpd/actor.py @@ -47,7 +47,7 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): lo = re.search('(? Date: Tue, 5 Nov 2013 11:06:51 +0000 Subject: [PATCH 04/32] Avahi constants named --- mopidy/utils/zeroconf.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/mopidy/utils/zeroconf.py b/mopidy/utils/zeroconf.py index db92877a..750f6731 100644 --- a/mopidy/utils/zeroconf.py +++ b/mopidy/utils/zeroconf.py @@ -2,6 +2,10 @@ import dbus __all__ = ["Zeroconf"] +avahi_IF_UNSPEC = -1 +avahi_PROTO_UNSPEC = -1 +avahi_PublishFlags_None = 0 + class Zeroconf: """A simple class to publish a network service with zeroconf using @@ -31,7 +35,8 @@ class Zeroconf: server.EntryGroupNew()), "org.freedesktop.Avahi.EntryGroup") - g.AddService(-1, -1, dbus.UInt32(0), + g.AddService(avahi_IF_UNSPEC, avahi_PROTO_UNSPEC, + dbus.UInt32(avahi_PublishFlags_None), self.name, self.stype, self.domain, self.host, dbus.UInt16(self.port), self.text) From c4281339b6401d5c83f6231b4b5ade3e8a2334cd Mon Sep 17 00:00:00 2001 From: David Eisner Date: Tue, 5 Nov 2013 11:07:35 +0000 Subject: [PATCH 05/32] Avahi hostname choice extracted for reuse --- mopidy/frontends/mpd/actor.py | 5 +---- mopidy/utils/zeroconf.py | 6 +++++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/mopidy/frontends/mpd/actor.py b/mopidy/frontends/mpd/actor.py index ab5bd35d..66218593 100644 --- a/mopidy/frontends/mpd/actor.py +++ b/mopidy/frontends/mpd/actor.py @@ -43,14 +43,11 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): try: if self.config['mpd']['zeroconf_enabled']: name = self.config['mpd']['zeroconf_name'] - import re - lo = re.search('(? Date: Tue, 5 Nov 2013 13:05:05 +0100 Subject: [PATCH 06/32] core: Letting filter() accept lists --- mopidy/core/tracklist.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index dbc81945..9f7d40ee 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -318,10 +318,10 @@ class TracklistController(object): matches = self._tl_tracks for (key, value) in criteria.iteritems(): if key == 'tlid': - matches = filter(lambda ct: ct.tlid == value, matches) + matches = filter(lambda ct: value in ct.tlid, matches) else: matches = filter( - lambda ct: getattr(ct.track, key) == value, matches) + lambda ct: value in getattr(ct.track, key), matches) return matches def move(self, start, end, to_position): From 32b01f4e4aba65ef7e11fe27378e13d04d65f3d8 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Tue, 5 Nov 2013 13:17:55 +0100 Subject: [PATCH 07/32] Adding a 'to list' conversion and correcting horrible mistake in comparison (put it reverse) --- mopidy/core/tracklist.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 9f7d40ee..002b0d0d 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -317,11 +317,13 @@ class TracklistController(object): criteria = criteria or kwargs matches = self._tl_tracks for (key, value) in criteria.iteritems(): + if not type(value) is list: + value = [value] if key == 'tlid': - matches = filter(lambda ct: value in ct.tlid, matches) + matches = filter(lambda ct: ct.tlid in value, matches) else: matches = filter( - lambda ct: value in getattr(ct.track, key), matches) + lambda ct: getattr(ct.track, key) in value, matches) return matches def move(self, start, end, to_position): From 697bff81cdad59c7c51f54237f076d1d690ea737 Mon Sep 17 00:00:00 2001 From: David Eisner Date: Tue, 5 Nov 2013 16:23:58 +0000 Subject: [PATCH 08/32] Advertise HTTP with Avahi --- mopidy/frontends/http/__init__.py | 2 ++ mopidy/frontends/http/actor.py | 26 ++++++++++++++++++++++++-- mopidy/frontends/http/ext.conf | 2 ++ mopidy/frontends/mpd/actor.py | 6 +++--- mopidy/frontends/mpd/ext.conf | 2 +- mopidy/utils/zeroconf.py | 5 ++++- 6 files changed, 36 insertions(+), 7 deletions(-) diff --git a/mopidy/frontends/http/__init__.py b/mopidy/frontends/http/__init__.py index 6d84b25b..d5f8f1bc 100644 --- a/mopidy/frontends/http/__init__.py +++ b/mopidy/frontends/http/__init__.py @@ -21,6 +21,8 @@ class Extension(ext.Extension): schema['hostname'] = config.Hostname() schema['port'] = config.Port() schema['static_dir'] = config.Path(optional=True) + schema['zeroconf_enabled'] = config.Boolean() + schema['zeroconf_name'] = config.String() return schema def validate_environment(self): diff --git a/mopidy/frontends/http/actor.py b/mopidy/frontends/http/actor.py index 5e49d2cd..47602b33 100644 --- a/mopidy/frontends/http/actor.py +++ b/mopidy/frontends/http/actor.py @@ -28,10 +28,13 @@ class HttpFrontend(pykka.ThreadingActor, CoreListener): self._setup_logging(app) def _setup_server(self): + self.config_section = self.config['http'] + self.hostname = self.config_section['hostname'] + self.port = self.config_section['port'] cherrypy.config.update({ 'engine.autoreload_on': False, - 'server.socket_host': self.config['http']['hostname'], - 'server.socket_port': self.config['http']['port'], + 'server.socket_host': self.hostname, + 'server.socket_port': self.port, }) def _setup_websocket_plugin(self): @@ -87,11 +90,30 @@ class HttpFrontend(pykka.ThreadingActor, CoreListener): logger.debug('Starting HTTP server') cherrypy.engine.start() logger.info('HTTP server running at %s', cherrypy.server.base()) + try: + if self.config_section['zeroconf_enabled']: + name = self.config_section['zeroconf_name'] + + from mopidy.utils.zeroconf import Zeroconf + self.service = Zeroconf( + stype="_http._tcp", + name=name, port=self.port, host=self.hostname, + text=["path=/"]) + self.service.publish() + + logger.info('Registered with Avahi as %s', name) + except Exception as e: + logger.warning('Avahi registration failed (%s)', e) def on_stop(self): logger.debug('Stopping HTTP server') cherrypy.engine.exit() logger.info('Stopped HTTP server') + try: + if self.service: + self.service.unpublish() + except Exception as e: + logger.warning('Avahi unregistration failed (%s)', e) def on_event(self, name, **data): event = data diff --git a/mopidy/frontends/http/ext.conf b/mopidy/frontends/http/ext.conf index 04fb1aae..f3df5f1a 100644 --- a/mopidy/frontends/http/ext.conf +++ b/mopidy/frontends/http/ext.conf @@ -3,6 +3,8 @@ enabled = true hostname = 127.0.0.1 port = 6680 static_dir = +zeroconf_enabled = true +zeroconf_name = Mopidy [loglevels] cherrypy = warning diff --git a/mopidy/frontends/mpd/actor.py b/mopidy/frontends/mpd/actor.py index 66218593..b252ee2d 100644 --- a/mopidy/frontends/mpd/actor.py +++ b/mopidy/frontends/mpd/actor.py @@ -17,7 +17,7 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): super(MpdFrontend, self).__init__() hostname = network.format_hostname(config['mpd']['hostname']) port = config['mpd']['port'] - self.config = config + self.config_section = config['mpd'] self.hostname = hostname self.port = port @@ -41,8 +41,8 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): def on_start(self): try: - if self.config['mpd']['zeroconf_enabled']: - name = self.config['mpd']['zeroconf_name'] + if self.config_section['zeroconf_enabled']: + name = self.config_section['zeroconf_name'] from mopidy.utils.zeroconf import Zeroconf self.service = Zeroconf( diff --git a/mopidy/frontends/mpd/ext.conf b/mopidy/frontends/mpd/ext.conf index 667aff24..d51c04f6 100644 --- a/mopidy/frontends/mpd/ext.conf +++ b/mopidy/frontends/mpd/ext.conf @@ -6,4 +6,4 @@ password = max_connections = 20 connection_timeout = 60 zeroconf_enabled = true -zeroconf_name = Mopidy +zeroconf_name = Mopidy (MPD) diff --git a/mopidy/utils/zeroconf.py b/mopidy/utils/zeroconf.py index d77738b4..4e802f7c 100644 --- a/mopidy/utils/zeroconf.py +++ b/mopidy/utils/zeroconf.py @@ -14,7 +14,7 @@ class Zeroconf: """ def __init__(self, name, port, stype="_http._tcp", - domain="", host="", text=""): + domain="", host="", text=[]): self.name = name self.stype = stype self.domain = domain @@ -39,6 +39,9 @@ class Zeroconf: server.EntryGroupNew()), "org.freedesktop.Avahi.EntryGroup") + if self.text: + self.text = [[dbus.Byte(ord(c)) for c in s] for s in self.text] + g.AddService(avahi_IF_UNSPEC, avahi_PROTO_UNSPEC, dbus.UInt32(avahi_PublishFlags_None), self.name, self.stype, self.domain, self.host, From 8b5f30e5ffe736ef95e64a8a0ec5bd91883ed28c Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Thu, 7 Nov 2013 09:34:40 +0100 Subject: [PATCH 09/32] Adding test case --- tests/backends/local/tracklist_test.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/backends/local/tracklist_test.py b/tests/backends/local/tracklist_test.py index 8310ce1a..21c06673 100644 --- a/tests/backends/local/tracklist_test.py +++ b/tests/backends/local/tracklist_test.py @@ -239,6 +239,18 @@ class LocalTracklistProviderTest(unittest.TestCase): def test_removing_from_empty_playlist_does_nothing(self): self.controller.remove(uri='/nonexistant') + @populate_tracklist + def test_remove_lists(self): + track1 = self.controller.tracks[1] + track2 = self.controller.tracks[2] + track3 = self.controller.tracks[3] + version = self.controller.version + self.controller.remove(uri=[track1.uri, track3.uri]) + self.assertLess(version, self.controller.version) + self.assertNotIn(track1, self.controller.tracks) + self.assertNotIn(track3, self.controller.tracks) + self.assertEqual(track2, self.controller.tracks[1]) + @populate_tracklist def test_shuffle(self): random.seed(1) From 6721a59b266b9f3209e792910255ba3565c07469 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Thu, 7 Nov 2013 09:46:34 +0100 Subject: [PATCH 10/32] tests: Fixing self confusion mistake docs: Documenting tracklist's new filter() feature --- mopidy/core/tracklist.py | 16 ++++++++++++++++ tests/backends/local/tracklist_test.py | 10 +++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 002b0d0d..5c2d91e0 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -298,18 +298,34 @@ class TracklistController(object): filter({'tlid': 7}) filter(tlid=7) + # Returns tracks with TLIDs 1, 2, 3 and 4 + filter({'tlid': [1, 2, 3, 4]}) + filter(tlid=[1, 2, 3, 4]) + # Returns track with ID 1 filter({'id': 1}) filter(id=1) + # Returns track with IDs 1 5 and 7 + filter({'id': [1, 5, 7]}) + filter(id=[1, 5, 7]) + # Returns track with URI 'xyz' filter({'uri': 'xyz'}) filter(uri='xyz') + # Returns track with URIs 'xyz' and 'abc' + filter({'uri': ['xyz', 'abc']}) + filter(uri=['xyz', 'abc']) + # Returns track with ID 1 and URI 'xyz' filter({'id': 1, 'uri': 'xyz'}) filter(id=1, uri='xyz') + # Returns track with IDs (1, 3 or 6) and URIs ('xyz' or 'abc') + filter({'id': [1, 3, 6], 'uri': ['xyz', 'abc']}) + filter(id=[1, 3, 6], uri=['xyz', 'abc']) + :param criteria: on or more criteria to match by :type criteria: dict :rtype: list of :class:`mopidy.models.TlTrack` diff --git a/tests/backends/local/tracklist_test.py b/tests/backends/local/tracklist_test.py index 21c06673..90eadeb9 100644 --- a/tests/backends/local/tracklist_test.py +++ b/tests/backends/local/tracklist_test.py @@ -241,15 +241,15 @@ class LocalTracklistProviderTest(unittest.TestCase): @populate_tracklist def test_remove_lists(self): + track0 = self.controller.tracks[0] track1 = self.controller.tracks[1] track2 = self.controller.tracks[2] - track3 = self.controller.tracks[3] version = self.controller.version - self.controller.remove(uri=[track1.uri, track3.uri]) + self.controller.remove(uri=[track0.uri, track2.uri]) self.assertLess(version, self.controller.version) - self.assertNotIn(track1, self.controller.tracks) - self.assertNotIn(track3, self.controller.tracks) - self.assertEqual(track2, self.controller.tracks[1]) + self.assertNotIn(track0, self.controller.tracks) + self.assertNotIn(track2, self.controller.tracks) + self.assertEqual(track1, self.controller.tracks[0]) @populate_tracklist def test_shuffle(self): From 45a38cdaf1510c1b17c8ecb1f830b9792de7b071 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Thu, 7 Nov 2013 10:51:29 +0100 Subject: [PATCH 11/32] core: Changing input to accept also sets, as might be also used. --- mopidy/core/tracklist.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 5c2d91e0..584a8bb9 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -290,7 +290,8 @@ class TracklistController(object): def filter(self, criteria=None, **kwargs): """ - Filter the tracklist by the given criterias. + Filter the tracklist by the given criterias. The value of the field to + check can be a list or a set. Examples:: @@ -333,7 +334,7 @@ class TracklistController(object): criteria = criteria or kwargs matches = self._tl_tracks for (key, value) in criteria.iteritems(): - if not type(value) is list: + if not type(value) in [list, set]: value = [value] if key == 'tlid': matches = filter(lambda ct: ct.tlid in value, matches) From 71bae709ef3143ff00f1b0f7b11385984bc11cc2 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Thu, 7 Nov 2013 11:05:33 +0100 Subject: [PATCH 12/32] core: filter() to accept also tuples --- mopidy/core/tracklist.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 584a8bb9..012dd796 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -291,7 +291,7 @@ class TracklistController(object): def filter(self, criteria=None, **kwargs): """ Filter the tracklist by the given criterias. The value of the field to - check can be a list or a set. + check can be a list, a tuple or a set. Examples:: @@ -334,7 +334,7 @@ class TracklistController(object): criteria = criteria or kwargs matches = self._tl_tracks for (key, value) in criteria.iteritems(): - if not type(value) in [list, set]: + if not type(value) in [list, tuple, set]: value = [value] if key == 'tlid': matches = filter(lambda ct: ct.tlid in value, matches) From 03750a8bf22251ec96060b070e7bcf113b9f13f8 Mon Sep 17 00:00:00 2001 From: Lasse Bigum Date: Fri, 8 Nov 2013 21:25:21 +0100 Subject: [PATCH 13/32] Fix scan with multiple track artists and add tests --- mopidy/audio/scan.py | 8 +++++++- tests/audio/scan_test.py | 24 ++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index 82803379..1d5f85f6 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -144,6 +144,12 @@ def audio_data_to_track(data): track_kwargs['last_modified'] = int(data['mtime']) track_kwargs['length'] = data[gst.TAG_DURATION] // gst.MSECOND track_kwargs['album'] = Album(**album_kwargs) - track_kwargs['artists'] = [Artist(**artist_kwargs)] + + if ('name' in artist_kwargs and + type(artist_kwargs['name']) is list): + track_kwargs['artists'] = [Artist(**{'name': artist}) + for artist in artist_kwargs['name']] + else: + track_kwargs['artists'] = [Artist(**artist_kwargs)] return Track(**track_kwargs) diff --git a/tests/audio/scan_test.py b/tests/audio/scan_test.py index b53b0b57..6fda7f74 100644 --- a/tests/audio/scan_test.py +++ b/tests/audio/scan_test.py @@ -46,11 +46,18 @@ class TranslatorTest(unittest.TestCase): 'musicbrainz_id': 'mbalbumid', } - self.artist = { + self.artist_single = { 'name': 'name', 'musicbrainz_id': 'mbartistid', } + self.artist_multiple = { + 'name': ['name1', 'name2'], + 'musicbrainz_id': 'mbartistid', + } + + self.artist = self.artist_single + self.albumartist = { 'name': 'albumartistname', 'musicbrainz_id': 'mbalbumartistid', @@ -71,7 +78,14 @@ class TranslatorTest(unittest.TestCase): if self.albumartist: self.album['artists'] = [Artist(**self.albumartist)] self.track['album'] = Album(**self.album) - self.track['artists'] = [Artist(**self.artist)] + + if ('name' in self.artist and + type(self.artist['name']) is list): + self.track['artists'] = [Artist(**{'name': artist}) + for artist in self.artist['name']] + else: + self.track['artists'] = [Artist(**self.artist)] + return Track(**self.track) def check(self): @@ -122,6 +136,12 @@ class TranslatorTest(unittest.TestCase): del self.artist['musicbrainz_id'] self.check() + def test_multiple_track_artists(self): + self.data['artist'] = ['name1', 'name2'] + self.data['musicbrainz-artistid'] = 'mbartistid' + self.artist = self.artist_multiple + self.check() + def test_missing_album_artist(self): del self.data['album-artist'] del self.albumartist['name'] From 093c0400a9ff28d0fe6a3cadeb7b2bd283afc112 Mon Sep 17 00:00:00 2001 From: Lasse Bigum Date: Fri, 8 Nov 2013 21:43:31 +0100 Subject: [PATCH 14/32] Fix review comments --- mopidy/audio/scan.py | 2 +- tests/audio/scan_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index 1d5f85f6..52885000 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -147,7 +147,7 @@ def audio_data_to_track(data): if ('name' in artist_kwargs and type(artist_kwargs['name']) is list): - track_kwargs['artists'] = [Artist(**{'name': artist}) + track_kwargs['artists'] = [Artist(name=artist) for artist in artist_kwargs['name']] else: track_kwargs['artists'] = [Artist(**artist_kwargs)] diff --git a/tests/audio/scan_test.py b/tests/audio/scan_test.py index 6fda7f74..2a669bd6 100644 --- a/tests/audio/scan_test.py +++ b/tests/audio/scan_test.py @@ -81,7 +81,7 @@ class TranslatorTest(unittest.TestCase): if ('name' in self.artist and type(self.artist['name']) is list): - self.track['artists'] = [Artist(**{'name': artist}) + self.track['artists'] = [Artist(name=artist) for artist in self.artist['name']] else: self.track['artists'] = [Artist(**self.artist)] From c776565f656dd1a3a3812e9f8ddb388a1835e1c3 Mon Sep 17 00:00:00 2001 From: Lasse Bigum Date: Sat, 9 Nov 2013 01:22:27 +0100 Subject: [PATCH 15/32] Another round of review comments --- mopidy/audio/scan.py | 4 ++-- tests/audio/scan_test.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index 52885000..071857df 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -145,8 +145,8 @@ def audio_data_to_track(data): track_kwargs['length'] = data[gst.TAG_DURATION] // gst.MSECOND track_kwargs['album'] = Album(**album_kwargs) - if ('name' in artist_kwargs and - type(artist_kwargs['name']) is list): + if ('name' in artist_kwargs + and not isinstance(artist_kwargs['name'], basestring)): track_kwargs['artists'] = [Artist(name=artist) for artist in artist_kwargs['name']] else: diff --git a/tests/audio/scan_test.py b/tests/audio/scan_test.py index 2a669bd6..efeb3877 100644 --- a/tests/audio/scan_test.py +++ b/tests/audio/scan_test.py @@ -79,8 +79,8 @@ class TranslatorTest(unittest.TestCase): self.album['artists'] = [Artist(**self.albumartist)] self.track['album'] = Album(**self.album) - if ('name' in self.artist and - type(self.artist['name']) is list): + if ('name' in self.artist + and not isinstance(self.artist['name'], basestring)): self.track['artists'] = [Artist(name=artist) for artist in self.artist['name']] else: From 484efab28e5e8cc1cbe8335ab989bb14e833e33b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 10 Nov 2013 21:37:48 +0100 Subject: [PATCH 16/32] utils: Remove Python 2.6 workaround --- mopidy/utils/log.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/mopidy/utils/log.py b/mopidy/utils/log.py index 715aca1a..dee0fb96 100644 --- a/mopidy/utils/log.py +++ b/mopidy/utils/log.py @@ -12,9 +12,8 @@ def setup_logging(config, verbosity_level, save_debug_log): setup_console_logging(config, verbosity_level) if save_debug_log: setup_debug_logging_to_file(config) - if hasattr(logging, 'captureWarnings'): - # New in Python 2.7 - logging.captureWarnings(True) + + logging.captureWarnings(True) if config['logging']['config_file']: logging.config.fileConfig(config['logging']['config_file']) From cf5589b4eba1315ce1931cd3b4bcb7930f25dfd5 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 9 Nov 2013 13:56:05 +0100 Subject: [PATCH 17/32] avahi: Style and code cleanups in zeroconf module. - Prepare for handling missing dbus directly in module. - Constants should always be all caps. - Extracted helpers from class to convey intent via their naming. - Moved imports out of class, imports should always be on the top. - Made sure calling publish mutiple times does not re-convert text. - Made sure calling unpublish without publish does not break. --- mopidy/utils/zeroconf.py | 69 +++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/mopidy/utils/zeroconf.py b/mopidy/utils/zeroconf.py index 4e802f7c..c69fb950 100644 --- a/mopidy/utils/zeroconf.py +++ b/mopidy/utils/zeroconf.py @@ -1,17 +1,30 @@ -import dbus +from __future__ import unicode_literals -__all__ = ["Zeroconf"] +try: + import dbus +except ImportError: + dbus = None -avahi_IF_UNSPEC = -1 -avahi_PROTO_UNSPEC = -1 -avahi_PublishFlags_None = 0 +import re + +_AVAHI_IF_UNSPEC = -1 +_AVAHI_PROTO_UNSPEC = -1 +_AVAHI_PUBLISHFLAGS_NONE = 0 + + +def _filter_loopback_and_meta_addresses(host): + # TODO: see if we can find a cleaner way of handling this. + if re.search(r'(? Date: Sat, 9 Nov 2013 14:04:38 +0100 Subject: [PATCH 18/32] avahi: Improve error handling for dbus errors. - Handle dbus not being installed. - Handle not finding the system bus. - Handle not finding the avahi service. - Return if publish succeeded. --- mopidy/utils/zeroconf.py | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/mopidy/utils/zeroconf.py b/mopidy/utils/zeroconf.py index c69fb950..62545e27 100644 --- a/mopidy/utils/zeroconf.py +++ b/mopidy/utils/zeroconf.py @@ -1,17 +1,21 @@ from __future__ import unicode_literals +import logging +import re + +logger = logging.getLogger('mopidy.utils.zerconf') + try: import dbus except ImportError: dbus = None -import re - _AVAHI_IF_UNSPEC = -1 _AVAHI_PROTO_UNSPEC = -1 _AVAHI_PUBLISHFLAGS_NONE = 0 + def _filter_loopback_and_meta_addresses(host): # TODO: see if we can find a cleaner way of handling this. if re.search(r'(? Date: Sat, 9 Nov 2013 14:21:41 +0100 Subject: [PATCH 19/32] avahi: Convert MPD and HTTP to use cleaned up versions - Move imports to top as they should be. - Report state based on publish return value. - Remove overly broad except clauses. - Unpublish before stopping servers. --- mopidy/frontends/http/actor.py | 31 ++++++++++++++----------------- mopidy/frontends/mpd/actor.py | 32 ++++++++++++++------------------ 2 files changed, 28 insertions(+), 35 deletions(-) diff --git a/mopidy/frontends/http/actor.py b/mopidy/frontends/http/actor.py index 47602b33..d5c37f09 100644 --- a/mopidy/frontends/http/actor.py +++ b/mopidy/frontends/http/actor.py @@ -11,6 +11,7 @@ from ws4py.server.cherrypyserver import WebSocketPlugin, WebSocketTool from mopidy import models from mopidy.core import CoreListener +from mopidy.utils import zeroconf from . import ws @@ -22,6 +23,7 @@ class HttpFrontend(pykka.ThreadingActor, CoreListener): super(HttpFrontend, self).__init__() self.config = config self.core = core + self.zeroconf_service = None self._setup_server() self._setup_websocket_plugin() app = self._create_app() @@ -90,30 +92,25 @@ class HttpFrontend(pykka.ThreadingActor, CoreListener): logger.debug('Starting HTTP server') cherrypy.engine.start() logger.info('HTTP server running at %s', cherrypy.server.base()) - try: - if self.config_section['zeroconf_enabled']: - name = self.config_section['zeroconf_name'] - from mopidy.utils.zeroconf import Zeroconf - self.service = Zeroconf( - stype="_http._tcp", - name=name, port=self.port, host=self.hostname, - text=["path=/"]) - self.service.publish() + if self.config_section['zeroconf_enabled']: + name = self.config_section['zeroconf_name'] + self.zeroconf_service = zeroconf.Zeroconf( + stype="_http._tcp", name=name, port=self.port, + host=self.hostname) - logger.info('Registered with Avahi as %s', name) - except Exception as e: - logger.warning('Avahi registration failed (%s)', e) + if self.zeroconf_service.publish(): + logger.info('Registered HTTP with zeroconf as %s', name) + else: + logger.warning('Registering HTTP with zeroconf failed.') def on_stop(self): + if self.zeroconf_service: + self.zeroconf_service.unpublish() + logger.debug('Stopping HTTP server') cherrypy.engine.exit() logger.info('Stopped HTTP server') - try: - if self.service: - self.service.unpublish() - except Exception as e: - logger.warning('Avahi unregistration failed (%s)', e) def on_event(self, name, **data): event = data diff --git a/mopidy/frontends/mpd/actor.py b/mopidy/frontends/mpd/actor.py index b252ee2d..51735837 100644 --- a/mopidy/frontends/mpd/actor.py +++ b/mopidy/frontends/mpd/actor.py @@ -7,7 +7,7 @@ import pykka from mopidy.core import CoreListener from mopidy.frontends.mpd import session -from mopidy.utils import encoding, network, process +from mopidy.utils import encoding, network, process, zeroconf logger = logging.getLogger('mopidy.frontends.mpd') @@ -20,6 +20,7 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): self.config_section = config['mpd'] self.hostname = hostname self.port = port + self.zeroconf_service = None try: network.Server( @@ -40,27 +41,22 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): logger.info('MPD server running at [%s]:%s', hostname, port) def on_start(self): - try: - if self.config_section['zeroconf_enabled']: - name = self.config_section['zeroconf_name'] + if self.config_section['zeroconf_enabled']: + name = self.config_section['zeroconf_name'] + self.zeroconf_service = zeroconf.Zeroconf( + stype="_mpd._tcp", name=name, port=self.port, + host=self.hostname) - from mopidy.utils.zeroconf import Zeroconf - self.service = Zeroconf( - stype="_mpd._tcp", - name=name, port=self.port, host=self.hostname) - self.service.publish() - - logger.info('Registered with Avahi as %s', name) - except Exception as e: - logger.warning('Avahi registration failed (%s)', e) + if self.zeroconf_service.publish(): + logger.info('Registered MPD with zeroconf as %s', name) + else: + logger.warning('Registering MPD with zeroconf failed.') def on_stop(self): + if self.zeroconf_service: + self.zeroconf_service.unpublish() + process.stop_actors_by_class(session.MpdSession) - try: - if self.service: - self.service.unpublish() - except Exception as e: - logger.warning('Avahi unregistration failed (%s)', e) def send_idle(self, subsystem): listeners = pykka.ActorRegistry.get_by_class(session.MpdSession) From 6c6932c374d5be7de6b69d50ae40a09f423218b0 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 9 Nov 2013 17:22:37 +0100 Subject: [PATCH 20/32] avahi: Only convert single string at a time in helper --- mopidy/utils/zeroconf.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/utils/zeroconf.py b/mopidy/utils/zeroconf.py index 62545e27..8af63777 100644 --- a/mopidy/utils/zeroconf.py +++ b/mopidy/utils/zeroconf.py @@ -24,7 +24,7 @@ def _filter_loopback_and_meta_addresses(host): def _convert_text_to_dbus_bytes(text): - return [[dbus.Byte(ord(c)) for c in s] for s in text] + return [dbus.Byte(ord(c)) for c in text] class Zeroconf: @@ -62,7 +62,7 @@ class Zeroconf: bus.get_object("org.freedesktop.Avahi", server.EntryGroupNew()), "org.freedesktop.Avahi.EntryGroup") - text = _convert_text_to_dbus_bytes(self.text) + text = [_convert_text_to_dbus_bytes(t) for t in self.text] self.group.AddService(_AVAHI_IF_UNSPEC, _AVAHI_PROTO_UNSPEC, dbus.UInt32(_AVAHI_PUBLISHFLAGS_NONE), self.name, self.stype, self.domain, self.host, From fb31193ed057ea87d5477ab302e0cd61c9b79dcf Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 11 Nov 2013 21:26:11 +0100 Subject: [PATCH 21/32] avahi: Fixes from review comments --- mopidy/frontends/http/actor.py | 8 ++++---- mopidy/frontends/mpd/actor.py | 8 ++++---- mopidy/utils/zeroconf.py | 5 ++--- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/mopidy/frontends/http/actor.py b/mopidy/frontends/http/actor.py index d5c37f09..ad5441c3 100644 --- a/mopidy/frontends/http/actor.py +++ b/mopidy/frontends/http/actor.py @@ -96,13 +96,13 @@ class HttpFrontend(pykka.ThreadingActor, CoreListener): if self.config_section['zeroconf_enabled']: name = self.config_section['zeroconf_name'] self.zeroconf_service = zeroconf.Zeroconf( - stype="_http._tcp", name=name, port=self.port, - host=self.hostname) + stype='_http._tcp', name=name, + host=self.hostname, port=self.port) if self.zeroconf_service.publish(): - logger.info('Registered HTTP with zeroconf as %s', name) + logger.info('Registered HTTP with Zeroconf as %s', name) else: - logger.warning('Registering HTTP with zeroconf failed.') + logger.warning('Registering HTTP with Zeroconf failed.') def on_stop(self): if self.zeroconf_service: diff --git a/mopidy/frontends/mpd/actor.py b/mopidy/frontends/mpd/actor.py index 51735837..575f4159 100644 --- a/mopidy/frontends/mpd/actor.py +++ b/mopidy/frontends/mpd/actor.py @@ -44,13 +44,13 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): if self.config_section['zeroconf_enabled']: name = self.config_section['zeroconf_name'] self.zeroconf_service = zeroconf.Zeroconf( - stype="_mpd._tcp", name=name, port=self.port, - host=self.hostname) + stype="_mpd._tcp", name=name, + port=self.port, host=self.hostname) if self.zeroconf_service.publish(): - logger.info('Registered MPD with zeroconf as %s', name) + logger.info('Registered MPD with Zeroconf as %s', name) else: - logger.warning('Registering MPD with zeroconf failed.') + logger.warning('Registering MPD with Zeroconf failed.') def on_stop(self): if self.zeroconf_service: diff --git a/mopidy/utils/zeroconf.py b/mopidy/utils/zeroconf.py index 8af63777..34e9a182 100644 --- a/mopidy/utils/zeroconf.py +++ b/mopidy/utils/zeroconf.py @@ -15,7 +15,6 @@ _AVAHI_PROTO_UNSPEC = -1 _AVAHI_PUBLISHFLAGS_NONE = 0 - def _filter_loopback_and_meta_addresses(host): # TODO: see if we can find a cleaner way of handling this. if re.search(r'(? Date: Mon, 11 Nov 2013 21:39:14 +0100 Subject: [PATCH 22/32] avahi: Add hostname and port template values to names --- mopidy/frontends/http/actor.py | 3 ++- mopidy/frontends/http/ext.conf | 2 +- mopidy/frontends/mpd/actor.py | 3 ++- mopidy/frontends/mpd/ext.conf | 2 +- mopidy/utils/zeroconf.py | 9 +++++++-- 5 files changed, 13 insertions(+), 6 deletions(-) diff --git a/mopidy/frontends/http/actor.py b/mopidy/frontends/http/actor.py index ad5441c3..1cd0c5f4 100644 --- a/mopidy/frontends/http/actor.py +++ b/mopidy/frontends/http/actor.py @@ -100,7 +100,8 @@ class HttpFrontend(pykka.ThreadingActor, CoreListener): host=self.hostname, port=self.port) if self.zeroconf_service.publish(): - logger.info('Registered HTTP with Zeroconf as %s', name) + logger.info('Registered HTTP with Zeroconf as "%s"', + self.zeroconf_service.name) else: logger.warning('Registering HTTP with Zeroconf failed.') diff --git a/mopidy/frontends/http/ext.conf b/mopidy/frontends/http/ext.conf index f3df5f1a..891aeb5b 100644 --- a/mopidy/frontends/http/ext.conf +++ b/mopidy/frontends/http/ext.conf @@ -4,7 +4,7 @@ hostname = 127.0.0.1 port = 6680 static_dir = zeroconf_enabled = true -zeroconf_name = Mopidy +zeroconf_name = Mopidy HTTP server on $hostname [loglevels] cherrypy = warning diff --git a/mopidy/frontends/mpd/actor.py b/mopidy/frontends/mpd/actor.py index 575f4159..a40834b2 100644 --- a/mopidy/frontends/mpd/actor.py +++ b/mopidy/frontends/mpd/actor.py @@ -48,7 +48,8 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): port=self.port, host=self.hostname) if self.zeroconf_service.publish(): - logger.info('Registered MPD with Zeroconf as %s', name) + logger.info('Registered MPD with Zeroconf as "%s"', + self.zeroconf_service.name) else: logger.warning('Registering MPD with Zeroconf failed.') diff --git a/mopidy/frontends/mpd/ext.conf b/mopidy/frontends/mpd/ext.conf index d51c04f6..ad3b333a 100644 --- a/mopidy/frontends/mpd/ext.conf +++ b/mopidy/frontends/mpd/ext.conf @@ -6,4 +6,4 @@ password = max_connections = 20 connection_timeout = 60 zeroconf_enabled = true -zeroconf_name = Mopidy (MPD) +zeroconf_name = Mopidy MPD server on $hostname diff --git a/mopidy/utils/zeroconf.py b/mopidy/utils/zeroconf.py index 34e9a182..ba4e327d 100644 --- a/mopidy/utils/zeroconf.py +++ b/mopidy/utils/zeroconf.py @@ -2,6 +2,8 @@ from __future__ import unicode_literals import logging import re +import socket +import string logger = logging.getLogger('mopidy.utils.zerconf') @@ -31,13 +33,16 @@ class Zeroconf: def __init__(self, name, port, stype="_http._tcp", domain="", host="", text=[]): - self.name = name + self.group = None self.stype = stype self.domain = domain self.port = port self.text = text self.host = _filter_loopback_and_meta_addresses(host) - self.group = None + + template = string.Template(name) + self.name = template.safe_substitute( + hostname=self.host or socket.getfqdn(), port=self.port) def publish(self): if not dbus: From c964d15ac46adc1994aeab93629d07bf39d245fb Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 11 Nov 2013 21:42:23 +0100 Subject: [PATCH 23/32] avahi: Simplify config to single value --- mopidy/frontends/http/__init__.py | 3 +-- mopidy/frontends/http/actor.py | 5 ++--- mopidy/frontends/http/ext.conf | 3 +-- mopidy/frontends/mpd/__init__.py | 3 +-- mopidy/frontends/mpd/actor.py | 5 ++--- mopidy/frontends/mpd/ext.conf | 3 +-- 6 files changed, 8 insertions(+), 14 deletions(-) diff --git a/mopidy/frontends/http/__init__.py b/mopidy/frontends/http/__init__.py index d5f8f1bc..64cb88f9 100644 --- a/mopidy/frontends/http/__init__.py +++ b/mopidy/frontends/http/__init__.py @@ -21,8 +21,7 @@ class Extension(ext.Extension): schema['hostname'] = config.Hostname() schema['port'] = config.Port() schema['static_dir'] = config.Path(optional=True) - schema['zeroconf_enabled'] = config.Boolean() - schema['zeroconf_name'] = config.String() + schema['zeroconf'] = config.String(optional=True) return schema def validate_environment(self): diff --git a/mopidy/frontends/http/actor.py b/mopidy/frontends/http/actor.py index 1cd0c5f4..3e46dc63 100644 --- a/mopidy/frontends/http/actor.py +++ b/mopidy/frontends/http/actor.py @@ -93,10 +93,9 @@ class HttpFrontend(pykka.ThreadingActor, CoreListener): cherrypy.engine.start() logger.info('HTTP server running at %s', cherrypy.server.base()) - if self.config_section['zeroconf_enabled']: - name = self.config_section['zeroconf_name'] + if self.config_section['zeroconf']: self.zeroconf_service = zeroconf.Zeroconf( - stype='_http._tcp', name=name, + stype='_http._tcp', name=self.config_section['zeroconf'], host=self.hostname, port=self.port) if self.zeroconf_service.publish(): diff --git a/mopidy/frontends/http/ext.conf b/mopidy/frontends/http/ext.conf index 891aeb5b..fc239230 100644 --- a/mopidy/frontends/http/ext.conf +++ b/mopidy/frontends/http/ext.conf @@ -3,8 +3,7 @@ enabled = true hostname = 127.0.0.1 port = 6680 static_dir = -zeroconf_enabled = true -zeroconf_name = Mopidy HTTP server on $hostname +zeroconf = Mopidy HTTP server on $hostname [loglevels] cherrypy = warning diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 28f9c951..571d6455 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -23,8 +23,7 @@ class Extension(ext.Extension): schema['password'] = config.Secret(optional=True) schema['max_connections'] = config.Integer(minimum=1) schema['connection_timeout'] = config.Integer(minimum=1) - schema['zeroconf_enabled'] = config.Boolean() - schema['zeroconf_name'] = config.String() + schema['zeroconf'] = config.String(optional=True) return schema def validate_environment(self): diff --git a/mopidy/frontends/mpd/actor.py b/mopidy/frontends/mpd/actor.py index a40834b2..6e277078 100644 --- a/mopidy/frontends/mpd/actor.py +++ b/mopidy/frontends/mpd/actor.py @@ -41,10 +41,9 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): logger.info('MPD server running at [%s]:%s', hostname, port) def on_start(self): - if self.config_section['zeroconf_enabled']: - name = self.config_section['zeroconf_name'] + if self.config_section['zeroconf']: self.zeroconf_service = zeroconf.Zeroconf( - stype="_mpd._tcp", name=name, + stype="_mpd._tcp", name=self.config_section['zeroconf'], port=self.port, host=self.hostname) if self.zeroconf_service.publish(): diff --git a/mopidy/frontends/mpd/ext.conf b/mopidy/frontends/mpd/ext.conf index ad3b333a..c62c37ef 100644 --- a/mopidy/frontends/mpd/ext.conf +++ b/mopidy/frontends/mpd/ext.conf @@ -5,5 +5,4 @@ port = 6600 password = max_connections = 20 connection_timeout = 60 -zeroconf_enabled = true -zeroconf_name = Mopidy MPD server on $hostname +zeroconf = Mopidy MPD server on $hostname From f03c049485a22d0df63c88a455e3ff78c3f05618 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 11 Nov 2013 22:04:13 +0100 Subject: [PATCH 24/32] avahi: More review comments and some other fixes. - Switched to newstyle class - Switched to safe values in kwargs --- mopidy/frontends/mpd/actor.py | 4 ++-- mopidy/utils/zeroconf.py | 24 ++++++++++++------------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/mopidy/frontends/mpd/actor.py b/mopidy/frontends/mpd/actor.py index 6e277078..87c1a571 100644 --- a/mopidy/frontends/mpd/actor.py +++ b/mopidy/frontends/mpd/actor.py @@ -43,8 +43,8 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): def on_start(self): if self.config_section['zeroconf']: self.zeroconf_service = zeroconf.Zeroconf( - stype="_mpd._tcp", name=self.config_section['zeroconf'], - port=self.port, host=self.hostname) + stype='_mpd._tcp', name=self.config_section['zeroconf'], + host=self.hostname, port=self.port) if self.zeroconf_service.publish(): logger.info('Registered MPD with Zeroconf as "%s"', diff --git a/mopidy/utils/zeroconf.py b/mopidy/utils/zeroconf.py index ba4e327d..c1781867 100644 --- a/mopidy/utils/zeroconf.py +++ b/mopidy/utils/zeroconf.py @@ -28,17 +28,17 @@ def _convert_text_to_dbus_bytes(text): return [dbus.Byte(ord(c)) for c in text] -class Zeroconf: - """Publish a network service with zeroconf using Avahi.""" +class Zeroconf(object): + """Publish a network service with Zeroconf using Avahi.""" - def __init__(self, name, port, stype="_http._tcp", - domain="", host="", text=[]): + def __init__(self, name, port, stype=None, domain=None, + host=None, text=None): self.group = None - self.stype = stype - self.domain = domain + self.stype = stype or '_http._tcp' + self.domain = domain or '' self.port = port - self.text = text - self.host = _filter_loopback_and_meta_addresses(host) + self.text = text or [] + self.host = _filter_loopback_and_meta_addresses(host or '') template = string.Template(name) self.name = template.safe_substitute( @@ -59,12 +59,12 @@ class Zeroconf: logger.debug('Zeroconf publish failed: Avahi service not running.') return False - server = dbus.Interface(bus.get_object("org.freedesktop.Avahi", "/"), - "org.freedesktop.Avahi.Server") + server = dbus.Interface(bus.get_object('org.freedesktop.Avahi', '/'), + 'org.freedesktop.Avahi.Server') self.group = dbus.Interface( - bus.get_object("org.freedesktop.Avahi", server.EntryGroupNew()), - "org.freedesktop.Avahi.EntryGroup") + bus.get_object('org.freedesktop.Avahi', server.EntryGroupNew()), + 'org.freedesktop.Avahi.EntryGroup') text = [_convert_text_to_dbus_bytes(t) for t in self.text] self.group.AddService(_AVAHI_IF_UNSPEC, _AVAHI_PROTO_UNSPEC, From 660a1b738290afa45d9c2f0fbcabf0b171e34b9d Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 11 Nov 2013 22:32:46 +0100 Subject: [PATCH 25/32] avahi: Remove use of config_section --- mopidy/frontends/http/actor.py | 12 +++++++----- mopidy/frontends/mpd/actor.py | 14 +++++++------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/mopidy/frontends/http/actor.py b/mopidy/frontends/http/actor.py index 3e46dc63..4e3493d4 100644 --- a/mopidy/frontends/http/actor.py +++ b/mopidy/frontends/http/actor.py @@ -23,16 +23,18 @@ class HttpFrontend(pykka.ThreadingActor, CoreListener): super(HttpFrontend, self).__init__() self.config = config self.core = core + + self.hostname = config['http']['hostname'] + self.port = config['http']['port'] + self.zeroconf_name = config['http']['zeroconf'] self.zeroconf_service = None + self._setup_server() self._setup_websocket_plugin() app = self._create_app() self._setup_logging(app) def _setup_server(self): - self.config_section = self.config['http'] - self.hostname = self.config_section['hostname'] - self.port = self.config_section['port'] cherrypy.config.update({ 'engine.autoreload_on': False, 'server.socket_host': self.hostname, @@ -93,9 +95,9 @@ class HttpFrontend(pykka.ThreadingActor, CoreListener): cherrypy.engine.start() logger.info('HTTP server running at %s', cherrypy.server.base()) - if self.config_section['zeroconf']: + if self.zeroconf_name: self.zeroconf_service = zeroconf.Zeroconf( - stype='_http._tcp', name=self.config_section['zeroconf'], + stype='_http._tcp', name=self.zeroconf_name, host=self.hostname, port=self.port) if self.zeroconf_service.publish(): diff --git a/mopidy/frontends/mpd/actor.py b/mopidy/frontends/mpd/actor.py index 87c1a571..9df7ba07 100644 --- a/mopidy/frontends/mpd/actor.py +++ b/mopidy/frontends/mpd/actor.py @@ -15,16 +15,16 @@ logger = logging.getLogger('mopidy.frontends.mpd') class MpdFrontend(pykka.ThreadingActor, CoreListener): def __init__(self, config, core): super(MpdFrontend, self).__init__() + hostname = network.format_hostname(config['mpd']['hostname']) - port = config['mpd']['port'] - self.config_section = config['mpd'] self.hostname = hostname - self.port = port + self.port = config['mpd']['port'] + self.zeroconf_name = config['mpd']['zeroconf'] self.zeroconf_service = None try: network.Server( - hostname, port, + self.hostname, self.port, protocol=session.MpdSession, protocol_kwargs={ 'config': config, @@ -38,12 +38,12 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): encoding.locale_decode(error)) sys.exit(1) - logger.info('MPD server running at [%s]:%s', hostname, port) + logger.info('MPD server running at [%s]:%s', self.hostname, self.port) def on_start(self): - if self.config_section['zeroconf']: + if self.zeroconf_name: self.zeroconf_service = zeroconf.Zeroconf( - stype='_mpd._tcp', name=self.config_section['zeroconf'], + stype='_mpd._tcp', name=self.zeroconf_name, host=self.hostname, port=self.port) if self.zeroconf_service.publish(): From 924553b62efdc979e797e5ce107698045145a673 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 11 Nov 2013 22:40:45 +0100 Subject: [PATCH 26/32] http: Fix tests --- tests/frontends/http/events_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/frontends/http/events_test.py b/tests/frontends/http/events_test.py index 2c6b241e..5150db9b 100644 --- a/tests/frontends/http/events_test.py +++ b/tests/frontends/http/events_test.py @@ -28,6 +28,7 @@ class HttpEventsTest(unittest.TestCase): 'hostname': '127.0.0.1', 'port': 6680, 'static_dir': None, + 'zeroconf': '', } } self.http = actor.HttpFrontend(config=config, core=mock.Mock()) From df4f99cf7b59ba19d8d981ecb799ca6649af85a1 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 11 Nov 2013 22:46:21 +0100 Subject: [PATCH 27/32] docs: Update changelog --- docs/changelog.rst | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index f9ccbb3e..c20715bc 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -18,10 +18,25 @@ v0.17.0 (UNRELEASED) - When scanning, we no longer default the album artist to be the same as the track artist. Album artist is now only populated if the scanned file got an explicit album artist set. + - Library scanning has been switched back to custom code due to various issues - with GStreamer's built in scanner in 0.10. This also fixes the scanner slowdown. - (Fixes: :issue:`565`) -- Fix scanner so that mtime is respected when deciding which files can be skipped. + with GStreamer's built in scanner in 0.10. This also fixes the scanner + slowdown. (Fixes: :issue:`565`) + +- Fix scanner so that mtime is respected when deciding which files can be + skipped. + +**MPD frontend** + +- The MPD service is now published as a Zeroconf service if avahi-deamon is + running on the system. Some MPD clients will use this to present Mopidy as an + available server on the local network without needing any configuration. + +**HTTP frontend** + +- The HTTP service is now published as a Zeroconf service if avahi-deamon is + running on the system. Some browsers will present HTTP Zeroconf services on + the local network as "local sites" bookmarks. v0.16.1 (2013-11-02) From f7c3272045899eb409539c6f694c15be82e3b77f Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 11 Nov 2013 22:46:32 +0100 Subject: [PATCH 28/32] docs: Update authors --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 3794a267..8269452d 100644 --- a/AUTHORS +++ b/AUTHORS @@ -28,3 +28,4 @@ - Pavol Babincak - Javier Domingo - Lasse Bigum +- David Eisner From d2280d9a86e59be861008fac290a05596b749ef2 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 11 Nov 2013 22:52:36 +0100 Subject: [PATCH 29/32] docs: Add {mpd,http}/zeroconf descriptions --- docs/changelog.rst | 8 ++++++-- docs/ext/http.rst | 7 +++++++ docs/ext/mpd.rst | 7 +++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index c20715bc..5e01bd47 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -30,13 +30,17 @@ v0.17.0 (UNRELEASED) - The MPD service is now published as a Zeroconf service if avahi-deamon is running on the system. Some MPD clients will use this to present Mopidy as an - available server on the local network without needing any configuration. + available server on the local network without needing any configuration. See + the :confval:`mpd/zeroconf` config value to change the service name or + disable the service. (Fixes: :issue:`39`) **HTTP frontend** - The HTTP service is now published as a Zeroconf service if avahi-deamon is running on the system. Some browsers will present HTTP Zeroconf services on - the local network as "local sites" bookmarks. + the local network as "local sites" bookmarks. See the + :confval:`http/zeroconf` config value to change the service name or disable + the service. (Fixes: :issue:`39`) v0.16.1 (2013-11-02) diff --git a/docs/ext/http.rst b/docs/ext/http.rst index 65bddb73..ce79588e 100644 --- a/docs/ext/http.rst +++ b/docs/ext/http.rst @@ -59,6 +59,13 @@ Configuration values Change this to have Mopidy serve e.g. files for your JavaScript client. "/mopidy" will continue to work as usual even if you change this setting. +.. confval:: http/zeroconf + + Name of the HTTP service when published through Zeroconf. The variables + ``$hostname`` and ``$port`` can be used in the name. + + Set to an empty string to disable Zeroconf for HTTP. + Usage ===== diff --git a/docs/ext/mpd.rst b/docs/ext/mpd.rst index 9dbcbe11..52cb8ef2 100644 --- a/docs/ext/mpd.rst +++ b/docs/ext/mpd.rst @@ -97,6 +97,13 @@ Configuration values Number of seconds an MPD client can stay inactive before the connection is closed by the server. +.. confval:: mpd/zeroconf + + Name of the MPD service when published through Zeroconf. The variables + ``$hostname`` and ``$port`` can be used in the name. + + Set to an empty string to disable Zeroconf for MPD. + Usage ===== From aa7b360d0d73d5ed80795ee171f766c7520a2d7f Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 11 Nov 2013 22:59:45 +0100 Subject: [PATCH 30/32] docs: Fix typos --- docs/changelog.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 5e01bd47..de9bccc7 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -28,7 +28,7 @@ v0.17.0 (UNRELEASED) **MPD frontend** -- The MPD service is now published as a Zeroconf service if avahi-deamon is +- The MPD service is now published as a Zeroconf service if avahi-daemon is running on the system. Some MPD clients will use this to present Mopidy as an available server on the local network without needing any configuration. See the :confval:`mpd/zeroconf` config value to change the service name or @@ -36,7 +36,7 @@ v0.17.0 (UNRELEASED) **HTTP frontend** -- The HTTP service is now published as a Zeroconf service if avahi-deamon is +- The HTTP service is now published as a Zeroconf service if avahi-daemon is running on the system. Some browsers will present HTTP Zeroconf services on the local network as "local sites" bookmarks. See the :confval:`http/zeroconf` config value to change the service name or disable From 50be4a531664db3df06c8a9484d11c44455103e6 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 11 Nov 2013 23:13:25 +0100 Subject: [PATCH 31/32] docs: Update changelog --- docs/changelog.rst | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index de9bccc7..727c7885 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -15,16 +15,19 @@ v0.17.0 (UNRELEASED) **Local backend** -- When scanning, we no longer default the album artist to be the same as the - track artist. Album artist is now only populated if the scanned file got an - explicit album artist set. - - Library scanning has been switched back to custom code due to various issues with GStreamer's built in scanner in 0.10. This also fixes the scanner slowdown. (Fixes: :issue:`565`) -- Fix scanner so that mtime is respected when deciding which files can be - skipped. +- When scanning, we no longer default the album artist to be the same as the + track artist. Album artist is now only populated if the scanned file got an + explicit album artist set. + +- The scanner will now extract multiple artists from files with multiple artist + tags. + +- Fix scanner so that time of last modification is respected when deciding + which files can be skipped. **MPD frontend** From 04788abaac1e9165e73a5c84817505d54a08978f Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 12 Nov 2013 00:00:22 +0100 Subject: [PATCH 32/32] core: Change tracklist.{filter,remove} usage The criterias are now a mapping between field names and one or more values. This aligns tracklist.{filter,remove} with the API of library.{find_exact,search}, and allows for e.g. batch removals. An exception is raised immediately if the API is used in the old way to ease migration and debugging. --- docs/changelog.rst | 11 ++++ mopidy/core/tracklist.py | 53 +++++++++---------- .../mpd/protocol/current_playlist.py | 16 +++--- mopidy/frontends/mpd/protocol/playback.py | 2 +- tests/backends/local/tracklist_test.py | 28 +++++----- tests/core/events_test.py | 2 +- tests/core/tracklist_test.py | 14 +++-- 7 files changed, 70 insertions(+), 56 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 727c7885..a157e417 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -13,6 +13,17 @@ v0.17.0 (UNRELEASED) - The search field ``track`` has been renamed to ``track_name`` to avoid confusion with ``track_no``. (Fixes: :issue:`535`) +- The signature of the tracklist's + :meth:`~mopidy.core.TracklistController.filter` and + :meth:`~mopidy.core.TracklistController.remove` methods have changed. + Previously, they expected e.g. ``tracklist.filter(tlid=17)``. Now, the value + must always be a list, e.g. ``tracklist.filter(tlid=[17])``. This change + allows you to get or remove multiple tracks with a single call, e.g. + ``tracklist.remove(tlid=[1, 2, 7])``. This is especially useful for web + clients, as requests can be batched. This also brings the interface closer to + the library's :meth:`~mopidy.core.LibraryController.find_exact` and + :meth:`~mopidy.core.LibraryController.search` methods. + **Local backend** - Library scanning has been switched back to custom code due to various issues diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 012dd796..d3cc0d75 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +import collections import logging import random @@ -290,57 +291,53 @@ class TracklistController(object): def filter(self, criteria=None, **kwargs): """ - Filter the tracklist by the given criterias. The value of the field to - check can be a list, a tuple or a set. + Filter the tracklist by the given criterias. + + A criteria consists of a model field to check and a list of values to + compare it against. If the model field matches one of the values, it + may be returned. + + Only tracks that matches all the given criterias are returned. Examples:: - # Returns track with TLID 7 (tracklist ID) - filter({'tlid': 7}) - filter(tlid=7) - - # Returns tracks with TLIDs 1, 2, 3 and 4 + # Returns tracks with TLIDs 1, 2, 3, or 4 (tracklist ID) filter({'tlid': [1, 2, 3, 4]}) filter(tlid=[1, 2, 3, 4]) - # Returns track with ID 1 - filter({'id': 1}) - filter(id=1) - - # Returns track with IDs 1 5 and 7 + # Returns track with IDs 1, 5, or 7 filter({'id': [1, 5, 7]}) filter(id=[1, 5, 7]) - # Returns track with URI 'xyz' - filter({'uri': 'xyz'}) - filter(uri='xyz') - - # Returns track with URIs 'xyz' and 'abc' + # Returns track with URIs 'xyz' or 'abc' filter({'uri': ['xyz', 'abc']}) filter(uri=['xyz', 'abc']) - # Returns track with ID 1 and URI 'xyz' - filter({'id': 1, 'uri': 'xyz'}) - filter(id=1, uri='xyz') + # Returns tracks with ID 1 and URI 'xyz' + filter({'id': [1], 'uri': ['xyz']}) + filter(id=[1], uri=['xyz']) - # Returns track with IDs (1, 3 or 6) and URIs ('xyz' or 'abc') + # Returns track with a matching ID (1, 3 or 6) and a matching URI + # ('xyz' or 'abc') filter({'id': [1, 3, 6], 'uri': ['xyz', 'abc']}) filter(id=[1, 3, 6], uri=['xyz', 'abc']) :param criteria: on or more criteria to match by - :type criteria: dict + :type criteria: dict, of (string, list) pairs :rtype: list of :class:`mopidy.models.TlTrack` """ criteria = criteria or kwargs matches = self._tl_tracks - for (key, value) in criteria.iteritems(): - if not type(value) in [list, tuple, set]: - value = [value] + for (key, values) in criteria.iteritems(): + if (not isinstance(values, collections.Iterable) + or isinstance(values, basestring)): + # Fail hard if anyone is using the <0.17 calling style + raise ValueError('Filter values must be iterable: %r' % values) if key == 'tlid': - matches = filter(lambda ct: ct.tlid in value, matches) + matches = filter(lambda ct: ct.tlid in values, matches) else: matches = filter( - lambda ct: getattr(ct.track, key) in value, matches) + lambda ct: getattr(ct.track, key) in values, matches) return matches def move(self, start, end, to_position): @@ -454,7 +451,7 @@ class TracklistController(object): """Private method used by :class:`mopidy.core.PlaybackController`.""" if not self.consume: return False - self.remove(tlid=tl_track.tlid) + self.remove(tlid=[tl_track.tlid]) return True def _trigger_tracklist_changed(self): diff --git a/mopidy/frontends/mpd/protocol/current_playlist.py b/mopidy/frontends/mpd/protocol/current_playlist.py index 20452203..bc040067 100644 --- a/mopidy/frontends/mpd/protocol/current_playlist.py +++ b/mopidy/frontends/mpd/protocol/current_playlist.py @@ -76,7 +76,7 @@ def delete_range(context, start, end=None): if not tl_tracks: raise MpdArgError('Bad song index', command='delete') for (tlid, _) in tl_tracks: - context.core.tracklist.remove(tlid=tlid) + context.core.tracklist.remove(tlid=[tlid]) @handle_request(r'^delete "(?P\d+)"$') @@ -86,7 +86,7 @@ def delete_songpos(context, songpos): songpos = int(songpos) (tlid, _) = context.core.tracklist.slice( songpos, songpos + 1).get()[0] - context.core.tracklist.remove(tlid=tlid) + context.core.tracklist.remove(tlid=[tlid]) except IndexError: raise MpdArgError('Bad song index', command='delete') @@ -101,7 +101,7 @@ def deleteid(context, tlid): Deletes the song ``SONGID`` from the playlist """ tlid = int(tlid) - tl_tracks = context.core.tracklist.remove(tlid=tlid).get() + tl_tracks = context.core.tracklist.remove(tlid=[tlid]).get() if not tl_tracks: raise MpdNoExistError('No such song', command='deleteid') @@ -157,7 +157,7 @@ def moveid(context, tlid, to): """ tlid = int(tlid) to = int(to) - tl_tracks = context.core.tracklist.filter(tlid=tlid).get() + tl_tracks = context.core.tracklist.filter(tlid=[tlid]).get() if not tl_tracks: raise MpdNoExistError('No such song', command='moveid') position = context.core.tracklist.index(tl_tracks[0]).get() @@ -195,7 +195,7 @@ def playlistfind(context, tag, needle): - does not add quotes around the tag. """ if tag == 'filename': - tl_tracks = context.core.tracklist.filter(uri=needle).get() + tl_tracks = context.core.tracklist.filter(uri=[needle]).get() if not tl_tracks: return None position = context.core.tracklist.index(tl_tracks[0]).get() @@ -215,7 +215,7 @@ def playlistid(context, tlid=None): """ if tlid is not None: tlid = int(tlid) - tl_tracks = context.core.tracklist.filter(tlid=tlid).get() + tl_tracks = context.core.tracklist.filter(tlid=[tlid]).get() if not tl_tracks: raise MpdNoExistError('No such song', command='playlistid') position = context.core.tracklist.index(tl_tracks[0]).get() @@ -380,8 +380,8 @@ def swapid(context, tlid1, tlid2): """ tlid1 = int(tlid1) tlid2 = int(tlid2) - tl_tracks1 = context.core.tracklist.filter(tlid=tlid1).get() - tl_tracks2 = context.core.tracklist.filter(tlid=tlid2).get() + tl_tracks1 = context.core.tracklist.filter(tlid=[tlid1]).get() + tl_tracks2 = context.core.tracklist.filter(tlid=[tlid2]).get() if not tl_tracks1 or not tl_tracks2: raise MpdNoExistError('No such song', command='swapid') position1 = context.core.tracklist.index(tl_tracks1[0]).get() diff --git a/mopidy/frontends/mpd/protocol/playback.py b/mopidy/frontends/mpd/protocol/playback.py index b9289d8a..0d6bfe75 100644 --- a/mopidy/frontends/mpd/protocol/playback.py +++ b/mopidy/frontends/mpd/protocol/playback.py @@ -151,7 +151,7 @@ def playid(context, tlid): tlid = int(tlid) if tlid == -1: return _play_minus_one(context) - tl_tracks = context.core.tracklist.filter(tlid=tlid).get() + tl_tracks = context.core.tracklist.filter(tlid=[tlid]).get() if not tl_tracks: raise MpdNoExistError('No such song', command='playid') return context.core.playback.play(tl_tracks[0]).get() diff --git a/tests/backends/local/tracklist_test.py b/tests/backends/local/tracklist_test.py index 90eadeb9..ac135a25 100644 --- a/tests/backends/local/tracklist_test.py +++ b/tests/backends/local/tracklist_test.py @@ -71,34 +71,34 @@ class LocalTracklistProviderTest(unittest.TestCase): def test_filter_by_tlid(self): tl_track = self.controller.tl_tracks[1] self.assertEqual( - [tl_track], self.controller.filter(tlid=tl_track.tlid)) + [tl_track], self.controller.filter(tlid=[tl_track.tlid])) @populate_tracklist def test_filter_by_uri(self): tl_track = self.controller.tl_tracks[1] self.assertEqual( - [tl_track], self.controller.filter(uri=tl_track.track.uri)) + [tl_track], self.controller.filter(uri=[tl_track.track.uri])) @populate_tracklist def test_filter_by_uri_returns_nothing_for_invalid_uri(self): - self.assertEqual([], self.controller.filter(uri='foobar')) + self.assertEqual([], self.controller.filter(uri=['foobar'])) def test_filter_by_uri_returns_single_match(self): track = Track(uri='a') self.controller.add([Track(uri='z'), track, Track(uri='y')]) - self.assertEqual(track, self.controller.filter(uri='a')[0].track) + self.assertEqual(track, self.controller.filter(uri=['a'])[0].track) def test_filter_by_uri_returns_multiple_matches(self): track = Track(uri='a') self.controller.add([Track(uri='z'), track, track]) - tl_tracks = self.controller.filter(uri='a') + tl_tracks = self.controller.filter(uri=['a']) self.assertEqual(track, tl_tracks[0].track) self.assertEqual(track, tl_tracks[1].track) def test_filter_by_uri_returns_nothing_if_no_match(self): self.controller.playlist = Playlist( - tracks=[Track(uri='z'), Track(uri='y')]) - self.assertEqual([], self.controller.filter(uri='a')) + tracks=[Track(uri=['z']), Track(uri=['y'])]) + self.assertEqual([], self.controller.filter(uri=['a'])) def test_filter_by_multiple_criteria_returns_elements_matching_all(self): track1 = Track(uri='a', name='x') @@ -106,18 +106,18 @@ class LocalTracklistProviderTest(unittest.TestCase): track3 = Track(uri='b', name='y') self.controller.add([track1, track2, track3]) self.assertEqual( - track1, self.controller.filter(uri='a', name='x')[0].track) + track1, self.controller.filter(uri=['a'], name=['x'])[0].track) self.assertEqual( - track2, self.controller.filter(uri='b', name='x')[0].track) + track2, self.controller.filter(uri=['b'], name=['x'])[0].track) self.assertEqual( - track3, self.controller.filter(uri='b', name='y')[0].track) + track3, self.controller.filter(uri=['b'], name=['y'])[0].track) def test_filter_by_criteria_that_is_not_present_in_all_elements(self): track1 = Track() track2 = Track(uri='b') track3 = Track() self.controller.add([track1, track2, track3]) - self.assertEqual(track2, self.controller.filter(uri='b')[0].track) + self.assertEqual(track2, self.controller.filter(uri=['b'])[0].track) @populate_tracklist def test_clear(self): @@ -227,17 +227,17 @@ class LocalTracklistProviderTest(unittest.TestCase): track1 = self.controller.tracks[1] track2 = self.controller.tracks[2] version = self.controller.version - self.controller.remove(uri=track1.uri) + self.controller.remove(uri=[track1.uri]) self.assertLess(version, self.controller.version) self.assertNotIn(track1, self.controller.tracks) self.assertEqual(track2, self.controller.tracks[1]) @populate_tracklist def test_removing_track_that_does_not_exist_does_nothing(self): - self.controller.remove(uri='/nonexistant') + self.controller.remove(uri=['/nonexistant']) def test_removing_from_empty_playlist_does_nothing(self): - self.controller.remove(uri='/nonexistant') + self.controller.remove(uri=['/nonexistant']) @populate_tracklist def test_remove_lists(self): diff --git a/tests/core/events_test.py b/tests/core/events_test.py index 6d192b87..5d646840 100644 --- a/tests/core/events_test.py +++ b/tests/core/events_test.py @@ -105,7 +105,7 @@ class BackendEventsTest(unittest.TestCase): self.core.tracklist.add([Track(uri='dummy:a')]).get() send.reset_mock() - self.core.tracklist.remove(uri='dummy:a').get() + self.core.tracklist.remove(uri=['dummy:a']).get() self.assertEqual(send.call_args[0][0], 'tracklist_changed') diff --git a/tests/core/tracklist_test.py b/tests/core/tracklist_test.py index 9f17f6de..596a20a6 100644 --- a/tests/core/tracklist_test.py +++ b/tests/core/tracklist_test.py @@ -37,7 +37,7 @@ class TracklistTest(unittest.TestCase): self.assertEqual(tl_tracks, self.core.tracklist.tl_tracks[-1:]) def test_remove_removes_tl_tracks_matching_query(self): - tl_tracks = self.core.tracklist.remove(name='foo') + tl_tracks = self.core.tracklist.remove(name=['foo']) self.assertEqual(2, len(tl_tracks)) self.assertListEqual(self.tl_tracks[:2], tl_tracks) @@ -46,7 +46,7 @@ class TracklistTest(unittest.TestCase): self.assertListEqual(self.tl_tracks[2:], self.core.tracklist.tl_tracks) def test_remove_works_with_dict_instead_of_kwargs(self): - tl_tracks = self.core.tracklist.remove({'name': 'foo'}) + tl_tracks = self.core.tracklist.remove({'name': ['foo']}) self.assertEqual(2, len(tl_tracks)) self.assertListEqual(self.tl_tracks[:2], tl_tracks) @@ -55,15 +55,21 @@ class TracklistTest(unittest.TestCase): self.assertListEqual(self.tl_tracks[2:], self.core.tracklist.tl_tracks) def test_filter_returns_tl_tracks_matching_query(self): - tl_tracks = self.core.tracklist.filter(name='foo') + tl_tracks = self.core.tracklist.filter(name=['foo']) self.assertEqual(2, len(tl_tracks)) self.assertListEqual(self.tl_tracks[:2], tl_tracks) def test_filter_works_with_dict_instead_of_kwargs(self): - tl_tracks = self.core.tracklist.filter({'name': 'foo'}) + tl_tracks = self.core.tracklist.filter({'name': ['foo']}) self.assertEqual(2, len(tl_tracks)) self.assertListEqual(self.tl_tracks[:2], tl_tracks) + def test_filter_fails_if_values_isnt_iterable(self): + self.assertRaises(ValueError, self.core.tracklist.filter, tlid=3) + + def test_filter_fails_if_values_is_a_string(self): + self.assertRaises(ValueError, self.core.tracklist.filter, uri='a') + # TODO Extract tracklist tests from the base backend tests