From 90efbb6be74d39640ded918230497b316d784ead Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 5 Apr 2013 22:49:33 +0200 Subject: [PATCH 1/3] config: Add a Path config value and an ExpandedPath wrapper. Allows us to easily use expanded paths, without losing the original value for display and storage. In theory we could be using same trick for passwords. --- mopidy/utils/config.py | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/mopidy/utils/config.py b/mopidy/utils/config.py index 1a3127b5..675d92af 100644 --- a/mopidy/utils/config.py +++ b/mopidy/utils/config.py @@ -5,6 +5,7 @@ import re import socket from mopidy import exceptions +from mopidy.utils import path def validate_required(value, required): @@ -93,7 +94,7 @@ class ConfigValue(object): class String(ConfigValue): """String values. - Supports: optional choices and secret. + Supports: optional, choices and secret. """ def deserialize(self, value): value = value.strip() @@ -209,6 +210,34 @@ class Port(Integer): self.maximum = 2 ** 16 - 1 +class ExpandedPath(bytes): + def __new__(self, value): + expanded = path.expand_path(value) + return super(ExpandedPath, self).__new__(self, expanded) + + def __init__(self, value): + self.original = value + + +class Path(ConfigValue): + """File system path that will be expanded with mopidy.utils.path.expand_path + + Supports: optional, choices and secret. + """ + def deserialize(self, value): + value = value.strip() + validate_required(value, not self.optional) + validate_choice(value, self.choices) + if not value: + return None + return ExpandedPath(value) + + def serialize(self, value): + if isinstance(value, ExpandedPath): + return value.original + return value + + class ConfigSchema(object): """Logical group of config values that correspond to a config section. @@ -231,6 +260,8 @@ class ConfigSchema(object): return self._schema[key] def format(self, name, values): + # TODO: should the output be encoded utf-8 since we use that in + # serialize for strings? lines = ['[%s]' % name] for key in self._order: value = values.get(key) From 36266064d324dd6c99a1610084ecb3c165045dc0 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 5 Apr 2013 22:50:31 +0200 Subject: [PATCH 2/3] config: Switch all paths and files to use the Path in schemas. Also renames static_dir to static_path for better consistency. --- mopidy/backends/local/__init__.py | 6 +++--- mopidy/backends/spotify/__init__.py | 2 +- mopidy/config.py | 2 +- mopidy/frontends/http/__init__.py | 4 ++-- mopidy/frontends/mpris/__init__.py | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/mopidy/backends/local/__init__.py b/mopidy/backends/local/__init__.py index 17fd659e..54a1c7a4 100644 --- a/mopidy/backends/local/__init__.py +++ b/mopidy/backends/local/__init__.py @@ -55,9 +55,9 @@ class Extension(ext.Extension): def get_config_schema(self): schema = config.ExtensionConfigSchema() - schema['music_path'] = config.String() - schema['playlist_path'] = config.String() - schema['tag_cache_file'] = config.String() + schema['music_path'] = config.Path() + schema['playlist_path'] = config.Path() + schema['tag_cache_file'] = config.Path() return schema def validate_environment(self): diff --git a/mopidy/backends/spotify/__init__.py b/mopidy/backends/spotify/__init__.py index b03849eb..6de15aab 100644 --- a/mopidy/backends/spotify/__init__.py +++ b/mopidy/backends/spotify/__init__.py @@ -80,7 +80,7 @@ class Extension(ext.Extension): schema['password'] = config.String(secret=True) schema['bitrate'] = config.Integer(choices=(96, 160, 320)) schema['timeout'] = config.Integer(minimum=0) - schema['cache_path'] = config.String() + schema['cache_path'] = config.Path() schema['proxy_hostname'] = config.Hostname(optional=True) schema['proxy_username'] = config.String(optional=True) schema['proxy_password'] = config.String(optional=True, secret=True) diff --git a/mopidy/config.py b/mopidy/config.py index 85feffcc..4335f877 100644 --- a/mopidy/config.py +++ b/mopidy/config.py @@ -22,7 +22,7 @@ config_schemas = {} # TODO: use ordered dict? config_schemas['logging'] = config.ConfigSchema() config_schemas['logging']['console_format'] = config.String() config_schemas['logging']['debug_format'] = config.String() -config_schemas['logging']['debug_file'] = config.String() +config_schemas['logging']['debug_file'] = config.Path() config_schemas['logging.levels'] = config.LogLevelConfigSchema() diff --git a/mopidy/frontends/http/__init__.py b/mopidy/frontends/http/__init__.py index d588a376..4ca1d9b4 100644 --- a/mopidy/frontends/http/__init__.py +++ b/mopidy/frontends/http/__init__.py @@ -31,7 +31,7 @@ port = 6680 # 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. # -static_dir = +static_path = [logging.levels] cherrypy = warning @@ -533,7 +533,7 @@ class Extension(ext.Extension): schema = config.ExtensionConfigSchema() schema['hostname'] = config.Hostname() schema['port'] = config.Port() - schema['static_dir'] = config.String(optional=True) + schema['static_path'] = config.Path(optional=True) return schema def validate_environment(self): diff --git a/mopidy/frontends/mpris/__init__.py b/mopidy/frontends/mpris/__init__.py index 79806c47..82d15e9d 100644 --- a/mopidy/frontends/mpris/__init__.py +++ b/mopidy/frontends/mpris/__init__.py @@ -79,7 +79,7 @@ class Extension(ext.Extension): def get_config_schema(self): schema = config.ExtensionConfigSchema() - schema['desktop_file'] = config.String() + schema['desktop_file'] = config.Path() return schema def validate_environment(self): From 4f0e1e448cced6cf83d568320c08b77b6d554848 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 7 Apr 2013 22:01:34 +0000 Subject: [PATCH 3/3] config: Add path config value tests --- tests/utils/config_test.py | 49 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/utils/config_test.py b/tests/utils/config_test.py index 77c846df..5439dcc7 100644 --- a/tests/utils/config_test.py +++ b/tests/utils/config_test.py @@ -298,6 +298,55 @@ class PortTest(unittest.TestCase): self.assertRaises(ValueError, value.deserialize, '') +class ExpandedPathTest(unittest.TestCase): + def test_is_bytes(self): + self.assertIsInstance(config.ExpandedPath('/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', config.ExpandedPath('~')) + + @mock.patch('mopidy.utils.path.expand_path') + def test_orginal_stores_unexpanded(self, expand_path_mock): + self.assertEqual('~', config.ExpandedPath('~').original) + + +class PathTest(unittest.TestCase): + def test_deserialize_conversion_success(self): + result = config.Path().deserialize('/foo') + self.assertEqual('/foo', result) + self.assertIsInstance(result, config.ExpandedPath) + self.assertIsInstance(result, bytes) + + def test_deserialize_enforces_choices(self): + value = config.Path(choices=['/foo', '/bar', '/baz']) + self.assertEqual('/foo', value.deserialize('/foo')) + self.assertRaises(ValueError, value.deserialize, '/foobar') + + def test_deserialize_enforces_required(self): + value = config.Path() + self.assertRaises(ValueError, value.deserialize, '') + self.assertRaises(ValueError, value.deserialize, ' ') + + def test_deserialize_respects_optional(self): + value = config.Path(optional=True) + self.assertIsNone(value.deserialize('')) + self.assertIsNone(value.deserialize(' ')) + + @mock.patch('mopidy.utils.path.expand_path') + def test_serialize_uses_original(self, expand_path_mock): + expand_path_mock.return_value = 'expanded_path' + path = config.ExpandedPath('original_path') + value = config.Path() + self.assertEqual('expanded_path', path) + self.assertEqual('original_path', value.serialize(path)) + + def test_serialize_plain_string(self): + value = config.Path() + self.assertEqual('path', value.serialize('path')) + + class ConfigSchemaTest(unittest.TestCase): def setUp(self): self.schema = config.ConfigSchema()