andpp/mopidy
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Merge pull request #427 from adamcik/feature/path-as-bytes

Browse Source 
Enforce that we only want bytes in path related code.
This commit is contained in:
Stein Magnus Jodal 2013-04-27 14:41:13 -07:00
commit 1bd8f9e793
14 changed files with 95 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
mopidy/__main__.py
View File

@ -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):

14
mopidy/backends/local/actor.py
View File

@ -1,6 +1,7 @@
from __future__ import unicode_literals
import logging
import os
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,13 +28,10 @@ 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'])

2
mopidy/backends/local/translator.py
View File

@ -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'

19
mopidy/config/types.py
View File

@ -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):

2
mopidy/frontends/mpd/translator.py
View File

@ -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')

8
mopidy/scanner.py
View File

@ -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('Config value local/media_dir is not set.')
return
# TODO: missing error checking and other default setup code.
tracks = []

16
mopidy/utils/path.py
View File

@ -22,8 +22,13 @@ 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):
raise ValueError('Path is not a bytestring.')
dir_path = expand_path(dir_path)
if os.path.isfile(dir_path):
raise OSError(
@ -36,6 +41,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,8 +100,13 @@ def split_path(path):
def expand_path(path):
# TODO: expandvars as well?
path = string.Template(path).safe_substitute(XDG_DIRS)
# TODO: document as we want people to use this.
if not isinstance(path, bytes):
raise ValueError('Path is not a bytestring.')
try:
path = string.Template(path).substitute(XDG_DIRS)
except KeyError:
return None
path = os.path.expanduser(path)
path = os.path.abspath(path)
return path

4
tests/__init__.py
View File

@ -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)

2
tests/backends/local/events_test.py
View File

@ -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'),
}
}

2
tests/backends/local/library_test.py
View File

@ -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'),
}
}

2
tests/backends/local/playback_test.py
View File

@ -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'),
}
}

2
tests/backends/local/tracklist_test.py
View File

@ -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'),
}
}

36
tests/config/types_test.py
View File

@ -324,47 +324,43 @@ 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', b'foo'), 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('~'))
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('~').original)
original = b'~'
expanded = b'expanded_path'
result = types.ExpandedPath(original, expanded)
self.assertEqual(original, result.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')
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')
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))
def test_serialize_plain_string(self):
value = types.Path()
self.assertEqual('path', value.serialize('path'))
self.assertEqual('path', value.serialize(b'path'))

39
tests/utils/path_test.py
View File

@ -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):
@ -219,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'))