From 160a70e6dfcd6e7b9e98e6c9bc3d1ea9a0c7fb28 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 25 Apr 2013 21:08:33 +0200 Subject: [PATCH 1/7] path: Only accept bytes as paths --- mopidy/utils/path.py | 6 +++++ tests/__init__.py | 4 ++- tests/backends/local/events_test.py | 2 +- tests/backends/local/library_test.py | 2 +- tests/backends/local/playback_test.py | 2 +- tests/backends/local/tracklist_test.py | 2 +- tests/config/types_test.py | 26 +++++++++---------- tests/utils/path_test.py | 36 +++++++++++++++++++------- 8 files changed, 53 insertions(+), 27 deletions(-) diff --git a/mopidy/utils/path.py b/mopidy/utils/path.py index 164b114c..3d8bacf8 100644 --- a/mopidy/utils/path.py +++ b/mopidy/utils/path.py @@ -24,6 +24,8 @@ XDG_DIRS = { def get_or_create_dir(dir_path): + if not isinstance(dir_path, bytes): + raise ValueError('Path is not a bytestring.') dir_path = expand_path(dir_path) if os.path.isfile(dir_path): raise OSError( @@ -36,6 +38,8 @@ def get_or_create_dir(dir_path): def get_or_create_file(file_path): + if not isinstance(file_path, bytes): + raise ValueError('Path is not a bytestring.') file_path = expand_path(file_path) get_or_create_dir(os.path.dirname(file_path)) if not os.path.isfile(file_path): @@ -93,6 +97,8 @@ def split_path(path): def expand_path(path): + if not isinstance(path, bytes): + raise ValueError('Path is not a bytestring.') # TODO: expandvars as well? path = string.Template(path).safe_substitute(XDG_DIRS) path = os.path.expanduser(path) diff --git a/tests/__init__.py b/tests/__init__.py index b4e1d283..98df0370 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -10,8 +10,10 @@ else: def path_to_data_dir(name): + if not isinstance(name, bytes): + name = name.encode(sys.getfilesystemencoding()) path = os.path.dirname(__file__) - path = os.path.join(path, 'data') + path = os.path.join(path, b'data') path = os.path.abspath(path) return os.path.join(path, name) diff --git a/tests/backends/local/events_test.py b/tests/backends/local/events_test.py index e09bf4b9..1218afc4 100644 --- a/tests/backends/local/events_test.py +++ b/tests/backends/local/events_test.py @@ -11,7 +11,7 @@ class LocalBackendEventsTest(events.BackendEventsTest, unittest.TestCase): config = { 'local': { 'media_dir': path_to_data_dir(''), - 'playlists_dir': '', + 'playlists_dir': b'', 'tag_cache_file': path_to_data_dir('empty_tag_cache'), } } diff --git a/tests/backends/local/library_test.py b/tests/backends/local/library_test.py index 74635b3e..1cb933ce 100644 --- a/tests/backends/local/library_test.py +++ b/tests/backends/local/library_test.py @@ -11,7 +11,7 @@ class LocalLibraryControllerTest(LibraryControllerTest, unittest.TestCase): config = { 'local': { 'media_dir': path_to_data_dir(''), - 'playlists_dir': '', + 'playlists_dir': b'', 'tag_cache_file': path_to_data_dir('library_tag_cache'), } } diff --git a/tests/backends/local/playback_test.py b/tests/backends/local/playback_test.py index 16592539..7381113a 100644 --- a/tests/backends/local/playback_test.py +++ b/tests/backends/local/playback_test.py @@ -15,7 +15,7 @@ class LocalPlaybackControllerTest(PlaybackControllerTest, unittest.TestCase): config = { 'local': { 'media_dir': path_to_data_dir(''), - 'playlists_dir': '', + 'playlists_dir': b'', 'tag_cache_file': path_to_data_dir('empty_tag_cache'), } } diff --git a/tests/backends/local/tracklist_test.py b/tests/backends/local/tracklist_test.py index 2d8e87b6..44e4d004 100644 --- a/tests/backends/local/tracklist_test.py +++ b/tests/backends/local/tracklist_test.py @@ -13,7 +13,7 @@ class LocalTracklistControllerTest(TracklistControllerTest, unittest.TestCase): config = { 'local': { 'media_dir': path_to_data_dir(''), - 'playlists_dir': '', + 'playlists_dir': b'', 'tag_cache_file': path_to_data_dir('empty_tag_cache'), } } diff --git a/tests/config/types_test.py b/tests/config/types_test.py index de800e98..6351a0b2 100644 --- a/tests/config/types_test.py +++ b/tests/config/types_test.py @@ -324,47 +324,47 @@ class PortTest(unittest.TestCase): class ExpandedPathTest(unittest.TestCase): def test_is_bytes(self): - self.assertIsInstance(types.ExpandedPath('/tmp'), bytes) + self.assertIsInstance(types.ExpandedPath(b'/tmp'), bytes) @mock.patch('mopidy.utils.path.expand_path') def test_defaults_to_expanded(self, expand_path_mock): - expand_path_mock.return_value = 'expanded_path' - self.assertEqual('expanded_path', types.ExpandedPath('~')) + expand_path_mock.return_value = b'expanded_path' + self.assertEqual(b'expanded_path', types.ExpandedPath(b'~')) @mock.patch('mopidy.utils.path.expand_path') def test_orginal_stores_unexpanded(self, expand_path_mock): - self.assertEqual('~', types.ExpandedPath('~').original) + self.assertEqual('~', types.ExpandedPath(b'~').original) class PathTest(unittest.TestCase): def test_deserialize_conversion_success(self): - result = types.Path().deserialize('/foo') + result = types.Path().deserialize(b'/foo') self.assertEqual('/foo', result) self.assertIsInstance(result, types.ExpandedPath) self.assertIsInstance(result, bytes) def test_deserialize_enforces_choices(self): value = types.Path(choices=['/foo', '/bar', '/baz']) - self.assertEqual('/foo', value.deserialize('/foo')) - self.assertRaises(ValueError, value.deserialize, '/foobar') + self.assertEqual('/foo', value.deserialize(b'/foo')) + self.assertRaises(ValueError, value.deserialize, b'/foobar') def test_deserialize_enforces_required(self): value = types.Path() - self.assertRaises(ValueError, value.deserialize, '') + self.assertRaises(ValueError, value.deserialize, b'') def test_deserialize_respects_optional(self): value = types.Path(optional=True) - self.assertIsNone(value.deserialize('')) - self.assertIsNone(value.deserialize(' ')) + self.assertIsNone(value.deserialize(b'')) + self.assertIsNone(value.deserialize(b' ')) @mock.patch('mopidy.utils.path.expand_path') def test_serialize_uses_original(self, expand_path_mock): - expand_path_mock.return_value = 'expanded_path' - path = types.ExpandedPath('original_path') + expand_path_mock.return_value = b'expanded_path' + path = types.ExpandedPath(b'original_path') value = types.Path() self.assertEqual('expanded_path', path) self.assertEqual('original_path', value.serialize(path)) def test_serialize_plain_string(self): value = types.Path() - self.assertEqual('path', value.serialize('path')) + self.assertEqual('path', value.serialize(b'path')) diff --git a/tests/utils/path_test.py b/tests/utils/path_test.py index bd2e794b..13e541fc 100644 --- a/tests/utils/path_test.py +++ b/tests/utils/path_test.py @@ -22,7 +22,7 @@ class GetOrCreateDirTest(unittest.TestCase): shutil.rmtree(self.parent) def test_creating_dir(self): - dir_path = os.path.join(self.parent, 'test') + dir_path = os.path.join(self.parent, b'test') self.assert_(not os.path.exists(dir_path)) created = path.get_or_create_dir(dir_path) self.assert_(os.path.exists(dir_path)) @@ -30,8 +30,8 @@ class GetOrCreateDirTest(unittest.TestCase): self.assertEqual(created, dir_path) def test_creating_nested_dirs(self): - level2_dir = os.path.join(self.parent, 'test') - level3_dir = os.path.join(self.parent, 'test', 'test') + level2_dir = os.path.join(self.parent, b'test') + level3_dir = os.path.join(self.parent, b'test', b'test') self.assert_(not os.path.exists(level2_dir)) self.assert_(not os.path.exists(level3_dir)) created = path.get_or_create_dir(level3_dir) @@ -48,11 +48,20 @@ class GetOrCreateDirTest(unittest.TestCase): self.assertEqual(created, self.parent) def test_create_dir_with_name_of_existing_file_throws_oserror(self): - conflicting_file = os.path.join(self.parent, 'test') + conflicting_file = os.path.join(self.parent, b'test') open(conflicting_file, 'w').close() - dir_path = os.path.join(self.parent, 'test') + dir_path = os.path.join(self.parent, b'test') self.assertRaises(OSError, path.get_or_create_dir, dir_path) + def test_create_dir_with_unicode(self): + with self.assertRaises(ValueError): + dir_path = unicode(os.path.join(self.parent, b'test')) + path.get_or_create_dir(dir_path) + + def test_create_dir_with_none(self): + with self.assertRaises(ValueError): + path.get_or_create_dir(None) + class GetOrCreateFileTest(unittest.TestCase): def setUp(self): @@ -63,7 +72,7 @@ class GetOrCreateFileTest(unittest.TestCase): shutil.rmtree(self.parent) def test_creating_file(self): - file_path = os.path.join(self.parent, 'test') + file_path = os.path.join(self.parent, b'test') self.assert_(not os.path.exists(file_path)) created = path.get_or_create_file(file_path) self.assert_(os.path.exists(file_path)) @@ -71,8 +80,8 @@ class GetOrCreateFileTest(unittest.TestCase): self.assertEqual(created, file_path) def test_creating_nested_file(self): - level2_dir = os.path.join(self.parent, 'test') - file_path = os.path.join(self.parent, 'test', 'test') + level2_dir = os.path.join(self.parent, b'test') + file_path = os.path.join(self.parent, b'test', b'test') self.assert_(not os.path.exists(level2_dir)) self.assert_(not os.path.exists(file_path)) created = path.get_or_create_file(file_path) @@ -83,7 +92,7 @@ class GetOrCreateFileTest(unittest.TestCase): self.assertEqual(created, file_path) def test_creating_existing_file(self): - file_path = os.path.join(self.parent, 'test') + file_path = os.path.join(self.parent, b'test') path.get_or_create_file(file_path) created = path.get_or_create_file(file_path) self.assert_(os.path.exists(file_path)) @@ -94,6 +103,15 @@ class GetOrCreateFileTest(unittest.TestCase): conflicting_dir = os.path.join(self.parent) self.assertRaises(IOError, path.get_or_create_file, conflicting_dir) + def test_create_dir_with_unicode(self): + with self.assertRaises(ValueError): + file_path = unicode(os.path.join(self.parent, b'test')) + path.get_or_create_file(file_path) + + def test_create_dir_with_none(self): + with self.assertRaises(ValueError): + path.get_or_create_file(None) + class PathToFileURITest(unittest.TestCase): def test_simple_path(self): From 3409ca99d14d302a81a9deaad9229470a1d50a5a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 25 Apr 2013 21:24:02 +0200 Subject: [PATCH 2/7] path: Ensure mopidy.__main__ uses bytes for paths --- mopidy/__main__.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index a52c7ccc..cb9e2df8 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -39,7 +39,7 @@ def main(): loop = gobject.MainLoop() options = parse_options() - config_files = options.config.split(':') + config_files = options.config.split(b':') config_overrides = options.overrides enabled_extensions = [] # Make sure it is defined before the finally block @@ -162,7 +162,7 @@ def parse_options(): parser.add_option( b'--config', action='store', dest='config', - default='/etc/mopidy/mopidy.conf:$XDG_CONFIG_DIR/mopidy/mopidy.conf', + default=b'/etc/mopidy/mopidy.conf:$XDG_CONFIG_DIR/mopidy/mopidy.conf', help='config files to use, colon seperated, later files override') parser.add_option( b'-o', b'--option', @@ -174,7 +174,7 @@ def parse_options(): def show_config_callback(option, opt, value, parser): # TODO: don't use callback for this as --config or -o set after # --show-config will be ignored. - files = getattr(parser.values, 'config', '').split(':') + files = getattr(parser.values, 'config', b'').split(b':') overrides = getattr(parser.values, 'overrides', []) extensions = ext.load_extensions() @@ -196,14 +196,14 @@ def show_config_callback(option, opt, value, parser): def check_old_locations(): - dot_mopidy_dir = path.expand_path('~/.mopidy') + dot_mopidy_dir = path.expand_path(b'~/.mopidy') if os.path.isdir(dot_mopidy_dir): logger.warning( 'Old Mopidy dot dir found at %s. Please migrate your config to ' 'the ini-file based config format. See release notes for further ' 'instructions.', dot_mopidy_dir) - old_settings_file = path.expand_path('$XDG_CONFIG_DIR/mopidy/settings.py') + old_settings_file = path.expand_path(b'$XDG_CONFIG_DIR/mopidy/settings.py') if os.path.isfile(old_settings_file): logger.warning( 'Old Mopidy settings file found at %s. Please migrate your ' @@ -212,8 +212,8 @@ def check_old_locations(): def create_file_structures(): - path.get_or_create_dir('$XDG_DATA_DIR/mopidy') - path.get_or_create_file('$XDG_CONFIG_DIR/mopidy/mopidy.conf') + path.get_or_create_dir(b'$XDG_DATA_DIR/mopidy') + path.get_or_create_file(b'$XDG_CONFIG_DIR/mopidy/mopidy.conf') def setup_audio(config): From ac7edad86d6903856c6b7e87ebcaf2835230988f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 25 Apr 2013 22:02:09 +0200 Subject: [PATCH 3/7] config/path: Pass in expanded value in expanded path type --- mopidy/config/types.py | 19 ++++++++----------- tests/config/types_test.py | 26 +++++++++++--------------- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/mopidy/config/types.py b/mopidy/config/types.py index 29c41ae2..726d81f7 100644 --- a/mopidy/config/types.py +++ b/mopidy/config/types.py @@ -24,12 +24,11 @@ def encode(value): class ExpandedPath(bytes): - def __new__(self, value): - expanded = path.expand_path(value) + def __new__(self, original, expanded): return super(ExpandedPath, self).__new__(self, expanded) - def __init__(self, value): - self.original = value + def __init__(self, original, expanded): + self.original = original class ConfigValue(object): @@ -241,20 +240,18 @@ class Path(ConfigValue): - ``$XDG_DATA_DIR`` according to the XDG spec - ``$XDG_MUSIC_DIR`` according to the XDG spec - - Supported kwargs: ``optional``, ``choices``, and ``secret`` """ - def __init__(self, optional=False, choices=None): + def __init__(self, optional=False): self._required = not optional - self._choices = choices def deserialize(self, value): value = value.strip() + expanded = path.expand_path(value) validators.validate_required(value, self._required) - validators.validate_choice(value, self._choices) - if not value: + validators.validate_required(expanded, self._required) + if not value or expanded is None: return None - return ExpandedPath(value) + return ExpandedPath(value, expanded) def serialize(self, value, display=False): if isinstance(value, ExpandedPath): diff --git a/tests/config/types_test.py b/tests/config/types_test.py index 6351a0b2..ea07d5f5 100644 --- a/tests/config/types_test.py +++ b/tests/config/types_test.py @@ -324,16 +324,19 @@ class PortTest(unittest.TestCase): class ExpandedPathTest(unittest.TestCase): def test_is_bytes(self): - self.assertIsInstance(types.ExpandedPath(b'/tmp'), bytes) + self.assertIsInstance(types.ExpandedPath(b'/tmp', b'foo'), bytes) - @mock.patch('mopidy.utils.path.expand_path') - def test_defaults_to_expanded(self, expand_path_mock): - expand_path_mock.return_value = b'expanded_path' - self.assertEqual(b'expanded_path', types.ExpandedPath(b'~')) + def test_defaults_to_expanded(self,): + original = b'~' + expanded = b'expanded_path' + self.assertEqual(expanded, types.ExpandedPath(original, expanded)) @mock.patch('mopidy.utils.path.expand_path') def test_orginal_stores_unexpanded(self, expand_path_mock): - self.assertEqual('~', types.ExpandedPath(b'~').original) + original = b'~' + expanded = b'expanded_path' + result = types.ExpandedPath(original, expanded) + self.assertEqual(original, result.original) class PathTest(unittest.TestCase): @@ -343,11 +346,6 @@ class PathTest(unittest.TestCase): self.assertIsInstance(result, types.ExpandedPath) self.assertIsInstance(result, bytes) - def test_deserialize_enforces_choices(self): - value = types.Path(choices=['/foo', '/bar', '/baz']) - self.assertEqual('/foo', value.deserialize(b'/foo')) - self.assertRaises(ValueError, value.deserialize, b'/foobar') - def test_deserialize_enforces_required(self): value = types.Path() self.assertRaises(ValueError, value.deserialize, b'') @@ -357,10 +355,8 @@ class PathTest(unittest.TestCase): self.assertIsNone(value.deserialize(b'')) self.assertIsNone(value.deserialize(b' ')) - @mock.patch('mopidy.utils.path.expand_path') - def test_serialize_uses_original(self, expand_path_mock): - expand_path_mock.return_value = b'expanded_path' - path = types.ExpandedPath(b'original_path') + def test_serialize_uses_original(self): + path = types.ExpandedPath(b'original_path', b'expanded_path') value = types.Path() self.assertEqual('expanded_path', path) self.assertEqual('original_path', value.serialize(path)) From a7b6ff7b18a41f04084fb09c1ecff24afcb52062 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 25 Apr 2013 22:27:45 +0200 Subject: [PATCH 4/7] path: Disallow unknown substitutions in expand_path --- mopidy/utils/path.py | 10 ++++++++-- tests/utils/path_test.py | 3 +-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/mopidy/utils/path.py b/mopidy/utils/path.py index 3d8bacf8..8ad9b31a 100644 --- a/mopidy/utils/path.py +++ b/mopidy/utils/path.py @@ -22,6 +22,9 @@ XDG_DIRS = { 'XDG_MUSIC_DIR': glib.get_user_special_dir(glib.USER_DIRECTORY_MUSIC), } +# XDG_MUSIC_DIR can be none, so filter out any bad data. +XDG_DIRS = dict((k, v) for k, v in XDG_DIRS.items() if v is not None) + def get_or_create_dir(dir_path): if not isinstance(dir_path, bytes): @@ -97,10 +100,13 @@ def split_path(path): def expand_path(path): + # TODO: document as we want people to use this. if not isinstance(path, bytes): raise ValueError('Path is not a bytestring.') - # TODO: expandvars as well? - path = string.Template(path).safe_substitute(XDG_DIRS) + try: + path = string.Template(path).substitute(XDG_DIRS) + except KeyError: + return None path = os.path.expanduser(path) path = os.path.abspath(path) return path diff --git a/tests/utils/path_test.py b/tests/utils/path_test.py index 13e541fc..f7306f3d 100644 --- a/tests/utils/path_test.py +++ b/tests/utils/path_test.py @@ -237,8 +237,7 @@ class ExpandPathTest(unittest.TestCase): path.expand_path(b'$XDG_DATA_DIR/foo')) def test_xdg_subsititution_unknown(self): - self.assertEqual( - b'/tmp/$XDG_INVALID_DIR/foo', + self.assertIsNone( path.expand_path(b'/tmp/$XDG_INVALID_DIR/foo')) From 69ddfe4eb0541fd784811becc10a778efd4a860c Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 25 Apr 2013 22:28:24 +0200 Subject: [PATCH 5/7] local: Do not create missing media/playlists dir --- mopidy/backends/local/actor.py | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/mopidy/backends/local/actor.py b/mopidy/backends/local/actor.py index 1817e133..75cc485f 100644 --- a/mopidy/backends/local/actor.py +++ b/mopidy/backends/local/actor.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals import logging +import os.path import pykka @@ -19,7 +20,7 @@ class LocalBackend(pykka.ThreadingActor, base.Backend): self.config = config - self.create_dirs_and_files() + self.check_dirs_and_files() self.library = LocalLibraryProvider(backend=self) self.playback = base.BasePlaybackProvider(audio=audio, backend=self) @@ -27,20 +28,14 @@ class LocalBackend(pykka.ThreadingActor, base.Backend): self.uri_schemes = ['file'] - def create_dirs_and_files(self): - try: - path.get_or_create_dir(self.config['local']['media_dir']) - except EnvironmentError as error: - logger.warning( - 'Could not create local media dir: %s', - encoding.locale_decode(error)) + def check_dirs_and_files(self): + if not os.path.isdir(self.config['local']['media_dir']): + logger.warning('Local media dir %s does not exist.' % + self.config['local']['media_dir']) - try: - path.get_or_create_dir(self.config['local']['playlists_dir']) - except EnvironmentError as error: - logger.warning( - 'Could not create local playlists dir: %s', - encoding.locale_decode(error)) + if not os.path.isdir(self.config['local']['playlists_dir']): + logger.warning('Local playlists dir %s does not exist.' % + self.config['local']['playlists_dir']) try: path.get_or_create_file(self.config['local']['tag_cache_file']) From ba40c0032be2cb1c5bcf8b915d2d661c533deb34 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 25 Apr 2013 22:44:55 +0200 Subject: [PATCH 6/7] scanner: Handle missing media_dir --- mopidy/scanner.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/mopidy/scanner.py b/mopidy/scanner.py index cb19c5fd..d86568b3 100644 --- a/mopidy/scanner.py +++ b/mopidy/scanner.py @@ -35,8 +35,8 @@ from mopidy.utils import log, path, versioning def main(): options = parse_options() # TODO: support config files and overrides (shared from main?) - config_files = ['/etc/mopidy/mopidy.conf', - '$XDG_CONFIG_DIR/mopidy/mopidy.conf'] + config_files = [b'/etc/mopidy/mopidy.conf', + b'$XDG_CONFIG_DIR/mopidy/mopidy.conf'] config_overrides = [] # TODO: decide if we want to avoid this boilerplate some how. @@ -50,6 +50,10 @@ def main(): config_files, extensions, config_overrides) log.setup_log_levels(config) + if not config['local']['media_dir']: + logging.warning('local/media_dir is not set.') + return + # TODO: missing error checking and other default setup code. tracks = [] From 085b44e52fc7620403212d0a189d70ff03b4bf9a Mon Sep 17 00:00:00 2001 From: Thomas Adacmik Date: Sat, 27 Apr 2013 02:21:27 +0200 Subject: [PATCH 7/7] path: Update with respect to review comments in #427 --- mopidy/backends/local/actor.py | 11 +++++++---- mopidy/backends/local/translator.py | 2 ++ mopidy/frontends/mpd/translator.py | 2 +- mopidy/scanner.py | 2 +- tests/config/types_test.py | 2 +- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/mopidy/backends/local/actor.py b/mopidy/backends/local/actor.py index 75cc485f..8f53af4d 100644 --- a/mopidy/backends/local/actor.py +++ b/mopidy/backends/local/actor.py @@ -1,7 +1,7 @@ from __future__ import unicode_literals import logging -import os.path +import os import pykka @@ -33,9 +33,12 @@ class LocalBackend(pykka.ThreadingActor, base.Backend): logger.warning('Local media dir %s does not exist.' % self.config['local']['media_dir']) - if not os.path.isdir(self.config['local']['playlists_dir']): - logger.warning('Local playlists dir %s does not exist.' % - self.config['local']['playlists_dir']) + try: + path.get_or_create_dir(self.config['local']['playlists_dir']) + except EnvironmentError as error: + logger.warning( + 'Could not create local playlists dir: %s', + encoding.locale_decode(error)) try: path.get_or_create_file(self.config['local']['tag_cache_file']) diff --git a/mopidy/backends/local/translator.py b/mopidy/backends/local/translator.py index 683ad6b4..c53011f6 100644 --- a/mopidy/backends/local/translator.py +++ b/mopidy/backends/local/translator.py @@ -31,6 +31,7 @@ def parse_m3u(file_path, media_dir): - This function does not bother with Extended M3U directives. """ + # TODO: uris as bytes uris = [] try: with open(file_path) as m3u: @@ -71,6 +72,7 @@ def parse_mpd_tag_cache(tag_cache, music_dir=''): current = {} state = None + # TODO: uris as bytes for line in contents.split(b'\n'): if line == b'songList begin': state = 'songs' diff --git a/mopidy/frontends/mpd/translator.py b/mopidy/frontends/mpd/translator.py index 7cf5b0c6..e164883d 100644 --- a/mopidy/frontends/mpd/translator.py +++ b/mopidy/frontends/mpd/translator.py @@ -236,7 +236,7 @@ def tracks_to_tag_cache_format(tracks, media_dir): _add_to_tag_cache(result, dirs, files, media_dir) return result - +# TODO: bytes only def _add_to_tag_cache(result, dirs, files, media_dir): base_path = media_dir.encode('utf-8') diff --git a/mopidy/scanner.py b/mopidy/scanner.py index d86568b3..fc80f95b 100644 --- a/mopidy/scanner.py +++ b/mopidy/scanner.py @@ -51,7 +51,7 @@ def main(): log.setup_log_levels(config) if not config['local']['media_dir']: - logging.warning('local/media_dir is not set.') + logging.warning('Config value local/media_dir is not set.') return # TODO: missing error checking and other default setup code. diff --git a/tests/config/types_test.py b/tests/config/types_test.py index ea07d5f5..65732f56 100644 --- a/tests/config/types_test.py +++ b/tests/config/types_test.py @@ -326,7 +326,7 @@ class ExpandedPathTest(unittest.TestCase): def test_is_bytes(self): self.assertIsInstance(types.ExpandedPath(b'/tmp', b'foo'), bytes) - def test_defaults_to_expanded(self,): + def test_defaults_to_expanded(self): original = b'~' expanded = b'expanded_path' self.assertEqual(expanded, types.ExpandedPath(original, expanded))