From 3e53d4caff31d4f9229572b60c9f4b87086565fe Mon Sep 17 00:00:00 2001 From: michaelbub Date: Thu, 10 Nov 2016 13:05:18 +0100 Subject: [PATCH 1/7] sanitize playlist names Make e. g. - name: $albumartist/$year-$album.m3u' work when album data includes non fs-friendly characters --- beetsplug/smartplaylist.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 416dc37f5..73c3f59eb 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -20,7 +20,7 @@ from __future__ import division, absolute_import, print_function from beets.plugins import BeetsPlugin from beets import ui -from beets.util import mkdirall, normpath, syspath, bytestring_path +from beets.util import mkdirall, normpath, sanitize_path, syspath, bytestring_path from beets.library import Item, Album, parse_query_string from beets.dbcore import OrQuery from beets.dbcore.query import MultipleSort, ParsingError @@ -186,7 +186,7 @@ class SmartPlaylistPlugin(BeetsPlugin): # As we allow tags in the m3u names, we'll need to iterate through # the items and generate the correct m3u file names. for item in items: - m3u_name = item.evaluate_template(name, True) + m3u_name = sanitize_path(item.evaluate_template(name, True), lib.replacements) if m3u_name not in m3us: m3us[m3u_name] = [] item_path = item.path From 5942adba0104e834a3760affb94e5bda2c3ac489 Mon Sep 17 00:00:00 2001 From: michaelbub Date: Fri, 11 Nov 2016 10:07:25 +0100 Subject: [PATCH 2/7] fixed/updated test to consider sanitized playlist names --- beetsplug/smartplaylist.py | 5 +++-- test/test_smartplaylist.py | 16 +++++++++------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 73c3f59eb..bde64ca3d 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -173,7 +173,8 @@ class SmartPlaylistPlugin(BeetsPlugin): for playlist in self._matched_playlists: name, (query, q_sort), (album_query, a_q_sort) = playlist - self._log.debug(u"Creating playlist {0}", name) + sanitized_name = sanitize_path(name, lib.replacements) + self._log.debug(u"Creating playlist {0}", sanitized_name) items = [] if query: @@ -186,7 +187,7 @@ class SmartPlaylistPlugin(BeetsPlugin): # As we allow tags in the m3u names, we'll need to iterate through # the items and generate the correct m3u file names. for item in items: - m3u_name = sanitize_path(item.evaluate_template(name, True), lib.replacements) + m3u_name = item.evaluate_template(sanitized_name, True) if m3u_name not in m3us: m3us[m3u_name] = [] item_path = item.path diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py index 071ff390b..4425aa172 100644 --- a/test/test_smartplaylist.py +++ b/test/test_smartplaylist.py @@ -25,7 +25,7 @@ from beetsplug.smartplaylist import SmartPlaylistPlugin from beets.library import Item, Album, parse_query_string from beets.dbcore import OrQuery from beets.dbcore.query import NullSort, MultipleSort, FixedFieldSort -from beets.util import syspath, bytestring_path, py3_path +from beets.util import syspath, bytestring_path, py3_path, CHAR_REPLACE from beets.ui import UserError from beets import config @@ -151,12 +151,15 @@ class SmartPlaylistTest(unittest.TestCase): i = Mock(path=b'/tagada.mp3') i.evaluate_template.side_effect = lambda x, _: x - q = Mock() - a_q = Mock() + lib = Mock() + lib.replacements = CHAR_REPLACE lib.items.return_value = [i] lib.albums.return_value = [] - pl = b'my_playlist.m3u', (q, None), (a_q, None) + + q = Mock() + a_q = Mock() + pl = b'.my:.m3u', (q, None), (a_q, None) spl._matched_playlists = [pl] dir = bytestring_path(mkdtemp()) @@ -171,15 +174,14 @@ class SmartPlaylistTest(unittest.TestCase): lib.items.assert_called_once_with(q, None) lib.albums.assert_called_once_with(a_q, None) - m3u_filepath = path.join(dir, pl[0]) + m3u_filepath = path.join(dir, b'_my__playlist_.m3u') self.assertTrue(path.exists(m3u_filepath)) with open(syspath(m3u_filepath), 'rb') as f: content = f.read() rmtree(dir) self.assertEqual(content, b'/tagada.mp3\n') - - + class SmartPlaylistCLITest(unittest.TestCase, TestHelper): def setUp(self): self.setup_beets() From 405a633b5a136a097d33ec54b6b08624855c1e77 Mon Sep 17 00:00:00 2001 From: michaelbub Date: Fri, 11 Nov 2016 10:51:18 +0100 Subject: [PATCH 3/7] updated changelog with info about sanitized playlist names --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index dd012285b..02054e87c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -32,6 +32,8 @@ Deprecated configuration optional removals: The are a couple of small new features: +* :doc:`/plugins/smartplaylist`: Playlist names will be sanitized to + ensure valid filenames. :bug:`2258` * :doc:`/plugins/mpdupdate`, :doc:`/plugins/mpdstats`: When the ``host`` option is not set, these plugins will now look for the ``$MPD_HOST`` environment variable before falling back to ``localhost``. Thanks to :user:`tarruda`. From 0bdc621d991c31186a835866701836258aa1a7e7 Mon Sep 17 00:00:00 2001 From: michaelbub Date: Fri, 11 Nov 2016 15:21:34 +0100 Subject: [PATCH 4/7] sanitizing playlist name AFTER values have been filled in and updated test accordingly --- beetsplug/smartplaylist.py | 5 ++--- test/test_smartplaylist.py | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index bde64ca3d..73c3f59eb 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -173,8 +173,7 @@ class SmartPlaylistPlugin(BeetsPlugin): for playlist in self._matched_playlists: name, (query, q_sort), (album_query, a_q_sort) = playlist - sanitized_name = sanitize_path(name, lib.replacements) - self._log.debug(u"Creating playlist {0}", sanitized_name) + self._log.debug(u"Creating playlist {0}", name) items = [] if query: @@ -187,7 +186,7 @@ class SmartPlaylistPlugin(BeetsPlugin): # As we allow tags in the m3u names, we'll need to iterate through # the items and generate the correct m3u file names. for item in items: - m3u_name = item.evaluate_template(sanitized_name, True) + m3u_name = sanitize_path(item.evaluate_template(name, True), lib.replacements) if m3u_name not in m3us: m3us[m3u_name] = [] item_path = item.path diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py index 4425aa172..34f5b71d1 100644 --- a/test/test_smartplaylist.py +++ b/test/test_smartplaylist.py @@ -150,7 +150,7 @@ class SmartPlaylistTest(unittest.TestCase): spl = SmartPlaylistPlugin() i = Mock(path=b'/tagada.mp3') - i.evaluate_template.side_effect = lambda x, _: x + i.evaluate_template.side_effect = lambda pl, _: pl.replace('$title', 'ta:ga:da') lib = Mock() lib.replacements = CHAR_REPLACE @@ -159,7 +159,7 @@ class SmartPlaylistTest(unittest.TestCase): q = Mock() a_q = Mock() - pl = b'.my:.m3u', (q, None), (a_q, None) + pl = b'$title-my.m3u', (q, None), (a_q, None) spl._matched_playlists = [pl] dir = bytestring_path(mkdtemp()) @@ -174,7 +174,7 @@ class SmartPlaylistTest(unittest.TestCase): lib.items.assert_called_once_with(q, None) lib.albums.assert_called_once_with(a_q, None) - m3u_filepath = path.join(dir, b'_my__playlist_.m3u') + m3u_filepath = path.join(dir, b'ta_ga_da-my_playlist_.m3u') self.assertTrue(path.exists(m3u_filepath)) with open(syspath(m3u_filepath), 'rb') as f: content = f.read() From 28d18b7c64a6c04f3e84a4f6e6f0d2a3dbee3939 Mon Sep 17 00:00:00 2001 From: michaelbub Date: Fri, 11 Nov 2016 15:53:14 +0100 Subject: [PATCH 5/7] update test to run with python 3.5.2 --- test/test_smartplaylist.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py index 34f5b71d1..f6cc04aef 100644 --- a/test/test_smartplaylist.py +++ b/test/test_smartplaylist.py @@ -150,8 +150,8 @@ class SmartPlaylistTest(unittest.TestCase): spl = SmartPlaylistPlugin() i = Mock(path=b'/tagada.mp3') - i.evaluate_template.side_effect = lambda pl, _: pl.replace('$title', 'ta:ga:da') - + i.evaluate_template.side_effect = lambda pl, _: pl.replace(b'$title', b'ta:ga:da').decode() + lib = Mock() lib.replacements = CHAR_REPLACE lib.items.return_value = [i] From 0a88338f8b2c21817b21537a0ec2162a95f6f5b8 Mon Sep 17 00:00:00 2001 From: michaelbub Date: Fri, 11 Nov 2016 16:19:33 +0100 Subject: [PATCH 6/7] obey style guide --- beetsplug/smartplaylist.py | 6 ++++-- test/test_smartplaylist.py | 8 +++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 73c3f59eb..514e74acc 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -20,7 +20,8 @@ from __future__ import division, absolute_import, print_function from beets.plugins import BeetsPlugin from beets import ui -from beets.util import mkdirall, normpath, sanitize_path, syspath, bytestring_path +from beets.util import (mkdirall, normpath, sanitize_path, syspath, + bytestring_path) from beets.library import Item, Album, parse_query_string from beets.dbcore import OrQuery from beets.dbcore.query import MultipleSort, ParsingError @@ -186,7 +187,8 @@ class SmartPlaylistPlugin(BeetsPlugin): # As we allow tags in the m3u names, we'll need to iterate through # the items and generate the correct m3u file names. for item in items: - m3u_name = sanitize_path(item.evaluate_template(name, True), lib.replacements) + m3u_name = item.evaluate_template(name, True) + m3u_name = sanitize_path(m3u_name, lib.replacements) if m3u_name not in m3us: m3us[m3u_name] = [] item_path = item.path diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py index f6cc04aef..084e0694d 100644 --- a/test/test_smartplaylist.py +++ b/test/test_smartplaylist.py @@ -150,8 +150,9 @@ class SmartPlaylistTest(unittest.TestCase): spl = SmartPlaylistPlugin() i = Mock(path=b'/tagada.mp3') - i.evaluate_template.side_effect = lambda pl, _: pl.replace(b'$title', b'ta:ga:da').decode() - + title_replacer = lambda pl, _: pl.replace(b'$title', b'ta:ga:da').decode() + i.evaluate_template.side_effect = title_replacer + lib = Mock() lib.replacements = CHAR_REPLACE lib.items.return_value = [i] @@ -181,7 +182,8 @@ class SmartPlaylistTest(unittest.TestCase): rmtree(dir) self.assertEqual(content, b'/tagada.mp3\n') - + + class SmartPlaylistCLITest(unittest.TestCase, TestHelper): def setUp(self): self.setup_beets() From 58fe63764edcac47cd20d9afdc7d8ab40d7a2c73 Mon Sep 17 00:00:00 2001 From: michaelbub Date: Fri, 11 Nov 2016 22:34:46 +0100 Subject: [PATCH 7/7] reduced line length --- test/test_smartplaylist.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py index 084e0694d..fa8658ad3 100644 --- a/test/test_smartplaylist.py +++ b/test/test_smartplaylist.py @@ -150,8 +150,8 @@ class SmartPlaylistTest(unittest.TestCase): spl = SmartPlaylistPlugin() i = Mock(path=b'/tagada.mp3') - title_replacer = lambda pl, _: pl.replace(b'$title', b'ta:ga:da').decode() - i.evaluate_template.side_effect = title_replacer + i.evaluate_template.side_effect = \ + lambda pl, _: pl.replace(b'$title', b'ta:ga:da').decode() lib = Mock() lib.replacements = CHAR_REPLACE