From 6c7566a2f3460ef6fcc24ab842dd0d0cf8c1ebe0 Mon Sep 17 00:00:00 2001 From: alzeih Date: Tue, 30 Jul 2013 13:57:29 +1200 Subject: [PATCH 1/5] Strip invalid characters from playlist names for MPD frontend Fixes mopidy/mopidy$480 and mopidy/mopidy#474 --- mopidy/frontends/mpd/dispatcher.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/mopidy/frontends/mpd/dispatcher.py b/mopidy/frontends/mpd/dispatcher.py index 6590897d..7a4ee7d7 100644 --- a/mopidy/frontends/mpd/dispatcher.py +++ b/mopidy/frontends/mpd/dispatcher.py @@ -236,6 +236,9 @@ class MpdContext(object): #: The subsytems that we want to be notified about in idle mode. subscriptions = None + #regex for invalid characters in playlist names + _invalid_playlist_chars = re.compile(r'[\n\t\\/]') + def __init__(self, dispatcher, session=None, config=None, core=None): self.dispatcher = dispatcher self.session = session @@ -248,10 +251,11 @@ class MpdContext(object): self.refresh_playlists_mapping() def create_unique_name(self, playlist_name): - name = playlist_name + stripped_name = self._invalid_playlist_chars.sub(' ', playlist_name) + name = stripped_name i = 2 while name in self._playlist_uri_from_name: - name = '%s [%d]' % (playlist_name, i) + name = '%s [%d]' % (stripped_name, i) i += 1 return name From ac9acaabf0efb630aa746873ccaf43b09d65fc53 Mon Sep 17 00:00:00 2001 From: alzeih Date: Fri, 2 Aug 2013 01:32:07 +1200 Subject: [PATCH 2/5] typo in regex --- mopidy/frontends/mpd/dispatcher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/frontends/mpd/dispatcher.py b/mopidy/frontends/mpd/dispatcher.py index 7a4ee7d7..00e5747a 100644 --- a/mopidy/frontends/mpd/dispatcher.py +++ b/mopidy/frontends/mpd/dispatcher.py @@ -237,7 +237,7 @@ class MpdContext(object): subscriptions = None #regex for invalid characters in playlist names - _invalid_playlist_chars = re.compile(r'[\n\t\\/]') + _invalid_playlist_chars = re.compile(r'[\n\r\\/]') def __init__(self, dispatcher, session=None, config=None, core=None): self.dispatcher = dispatcher From 3f1192e95be03a226a65aa3a30a42ed34e155909 Mon Sep 17 00:00:00 2001 From: alzeih Date: Fri, 2 Aug 2013 13:38:52 +1200 Subject: [PATCH 3/5] Match MPD implementation and add tests --- mopidy/frontends/mpd/dispatcher.py | 2 +- tests/frontends/mpd/dispatcher_test.py | 30 +++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/mopidy/frontends/mpd/dispatcher.py b/mopidy/frontends/mpd/dispatcher.py index 00e5747a..52504440 100644 --- a/mopidy/frontends/mpd/dispatcher.py +++ b/mopidy/frontends/mpd/dispatcher.py @@ -237,7 +237,7 @@ class MpdContext(object): subscriptions = None #regex for invalid characters in playlist names - _invalid_playlist_chars = re.compile(r'[\n\r\\/]') + _invalid_playlist_chars = re.compile(r'[\n\r/]') def __init__(self, dispatcher, session=None, config=None, core=None): self.dispatcher = dispatcher diff --git a/tests/frontends/mpd/dispatcher_test.py b/tests/frontends/mpd/dispatcher_test.py index 9ef88e44..1543b64f 100644 --- a/tests/frontends/mpd/dispatcher_test.py +++ b/tests/frontends/mpd/dispatcher_test.py @@ -6,7 +6,7 @@ import pykka from mopidy import core from mopidy.backends import dummy -from mopidy.frontends.mpd.dispatcher import MpdDispatcher +from mopidy.frontends.mpd.dispatcher import MpdDispatcher, MpdContext from mopidy.frontends.mpd.exceptions import MpdAckError from mopidy.frontends.mpd.protocol import request_handlers, handle_request @@ -63,3 +63,31 @@ class MpdDispatcherTest(unittest.TestCase): result = self.dispatcher.handle_request('known request') self.assertIn('OK', result) self.assertIn(expected, result) + + +class MpdContextTest(unittest.TestCase): + def setUp(self): + config = { + 'mpd': { + 'password': None, + } + } + self.backend = dummy.create_dummy_backend_proxy() + self.core = core.Core.start(backends=[self.backend]).proxy() + self.dispatcher = MpdDispatcher(config=config) + self.context = self.dispatcher.context + + def tearDown(self): + pykka.ActorRegistry.stop_all() + + def test_context_create_unique_name_replaces_newlines_with_space(self): + result = self.context.create_unique_name("playlist name\n") + self.assertEqual("playlist name ", result) + + def test_context_create_unique_name_replaces_carriage_returns_with_space(self): + result = self.context.create_unique_name("playlist name\r") + self.assertEqual("playlist name ", result) + + def test_context_create_unique_name_replaces_forward_slashes_with_space(self): + result = self.context.create_unique_name("playlist name/") + self.assertEqual("playlist name ", result) From 7182f98843d76f6d98ed7a43882cc1f5b0f1d307 Mon Sep 17 00:00:00 2001 From: alzeih Date: Tue, 6 Aug 2013 21:14:23 +1200 Subject: [PATCH 4/5] cleanup tests and code - move tests to tests/frontends/mpd/stored_playlists_test.py stored_playlists_test.py: - rename test methods names to remove _config - remove unnecessary imports, setup, teardown, variables - sort remaining imports mopidy/frontends/mpd/dispatcher.py: - remove regex comment --- mopidy/frontends/mpd/dispatcher.py | 1 - tests/frontends/mpd/dispatcher_test.py | 30 +------------------- tests/frontends/mpd/stored_playlists_test.py | 28 ++++++++++++++++++ 3 files changed, 29 insertions(+), 30 deletions(-) create mode 100644 tests/frontends/mpd/stored_playlists_test.py diff --git a/mopidy/frontends/mpd/dispatcher.py b/mopidy/frontends/mpd/dispatcher.py index 52504440..a4cfb5ce 100644 --- a/mopidy/frontends/mpd/dispatcher.py +++ b/mopidy/frontends/mpd/dispatcher.py @@ -236,7 +236,6 @@ class MpdContext(object): #: The subsytems that we want to be notified about in idle mode. subscriptions = None - #regex for invalid characters in playlist names _invalid_playlist_chars = re.compile(r'[\n\r/]') def __init__(self, dispatcher, session=None, config=None, core=None): diff --git a/tests/frontends/mpd/dispatcher_test.py b/tests/frontends/mpd/dispatcher_test.py index 1543b64f..9ef88e44 100644 --- a/tests/frontends/mpd/dispatcher_test.py +++ b/tests/frontends/mpd/dispatcher_test.py @@ -6,7 +6,7 @@ import pykka from mopidy import core from mopidy.backends import dummy -from mopidy.frontends.mpd.dispatcher import MpdDispatcher, MpdContext +from mopidy.frontends.mpd.dispatcher import MpdDispatcher from mopidy.frontends.mpd.exceptions import MpdAckError from mopidy.frontends.mpd.protocol import request_handlers, handle_request @@ -63,31 +63,3 @@ class MpdDispatcherTest(unittest.TestCase): result = self.dispatcher.handle_request('known request') self.assertIn('OK', result) self.assertIn(expected, result) - - -class MpdContextTest(unittest.TestCase): - def setUp(self): - config = { - 'mpd': { - 'password': None, - } - } - self.backend = dummy.create_dummy_backend_proxy() - self.core = core.Core.start(backends=[self.backend]).proxy() - self.dispatcher = MpdDispatcher(config=config) - self.context = self.dispatcher.context - - def tearDown(self): - pykka.ActorRegistry.stop_all() - - def test_context_create_unique_name_replaces_newlines_with_space(self): - result = self.context.create_unique_name("playlist name\n") - self.assertEqual("playlist name ", result) - - def test_context_create_unique_name_replaces_carriage_returns_with_space(self): - result = self.context.create_unique_name("playlist name\r") - self.assertEqual("playlist name ", result) - - def test_context_create_unique_name_replaces_forward_slashes_with_space(self): - result = self.context.create_unique_name("playlist name/") - self.assertEqual("playlist name ", result) diff --git a/tests/frontends/mpd/stored_playlists_test.py b/tests/frontends/mpd/stored_playlists_test.py new file mode 100644 index 00000000..2c647681 --- /dev/null +++ b/tests/frontends/mpd/stored_playlists_test.py @@ -0,0 +1,28 @@ +from __future__ import unicode_literals + +import unittest + +from mopidy.frontends.mpd.dispatcher import MpdContext, MpdDispatcher + + +class MpdContextTest(unittest.TestCase): + def setUp(self): + config = { + 'mpd': { + 'password': None, + } + } + dispatcher = MpdDispatcher(config=config) + self.context = dispatcher.context + + def test_create_unique_name_replaces_newlines_with_space(self): + result = self.context.create_unique_name("playlist name\n") + self.assertEqual("playlist name ", result) + + def test_create_unique_name_replaces_carriage_returns_with_space(self): + result = self.context.create_unique_name("playlist name\r") + self.assertEqual("playlist name ", result) + + def test_create_unique_name_replaces_forward_slashes_with_space(self): + result = self.context.create_unique_name("playlist name/") + self.assertEqual("playlist name ", result) From 333f1c0dbfe0f436b05e7209e78da2daa8e9c8aa Mon Sep 17 00:00:00 2001 From: alzeih Date: Tue, 6 Aug 2013 22:22:18 +1200 Subject: [PATCH 5/5] Move tests to *protocol*/stored_playlists_test.py --- .../mpd/protocol/stored_playlists_test.py | 24 ++++++++++++++++ tests/frontends/mpd/stored_playlists_test.py | 28 ------------------- 2 files changed, 24 insertions(+), 28 deletions(-) delete mode 100644 tests/frontends/mpd/stored_playlists_test.py diff --git a/tests/frontends/mpd/protocol/stored_playlists_test.py b/tests/frontends/mpd/protocol/stored_playlists_test.py index 8199be2b..820096f4 100644 --- a/tests/frontends/mpd/protocol/stored_playlists_test.py +++ b/tests/frontends/mpd/protocol/stored_playlists_test.py @@ -107,6 +107,30 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertNotInResponse('playlist: ') self.assertInResponse('OK') + def test_listplaylists_replaces_newline_with_space(self): + self.backend.playlists.playlists = [ + Playlist(name='a\n', uri='dummy:')] + self.sendRequest('listplaylists') + self.assertInResponse('playlist: a ') + self.assertNotInResponse('playlist: a\n') + self.assertInResponse('OK') + + def test_listplaylists_replaces_carriage_return_with_space(self): + self.backend.playlists.playlists = [ + Playlist(name='a\r', uri='dummy:')] + self.sendRequest('listplaylists') + self.assertInResponse('playlist: a ') + self.assertNotInResponse('playlist: a\r') + self.assertInResponse('OK') + + def test_listplaylists_replaces_forward_slash_with_space(self): + self.backend.playlists.playlists = [ + Playlist(name='a/', uri='dummy:')] + self.sendRequest('listplaylists') + self.assertInResponse('playlist: a ') + self.assertNotInResponse('playlist: a/') + self.assertInResponse('OK') + def test_load_appends_to_tracklist(self): self.core.tracklist.add([Track(uri='a'), Track(uri='b')]) self.assertEqual(len(self.core.tracklist.tracks.get()), 2) diff --git a/tests/frontends/mpd/stored_playlists_test.py b/tests/frontends/mpd/stored_playlists_test.py deleted file mode 100644 index 2c647681..00000000 --- a/tests/frontends/mpd/stored_playlists_test.py +++ /dev/null @@ -1,28 +0,0 @@ -from __future__ import unicode_literals - -import unittest - -from mopidy.frontends.mpd.dispatcher import MpdContext, MpdDispatcher - - -class MpdContextTest(unittest.TestCase): - def setUp(self): - config = { - 'mpd': { - 'password': None, - } - } - dispatcher = MpdDispatcher(config=config) - self.context = dispatcher.context - - def test_create_unique_name_replaces_newlines_with_space(self): - result = self.context.create_unique_name("playlist name\n") - self.assertEqual("playlist name ", result) - - def test_create_unique_name_replaces_carriage_returns_with_space(self): - result = self.context.create_unique_name("playlist name\r") - self.assertEqual("playlist name ", result) - - def test_create_unique_name_replaces_forward_slashes_with_space(self): - result = self.context.create_unique_name("playlist name/") - self.assertEqual("playlist name ", result)