From 8cf1297e2c939729a01f463cbda8772b9593e684 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 6 Mar 2025 13:15:21 +0100 Subject: [PATCH] clarify config data types and structures --- fdroidserver/common.py | 66 +++++++++++++++++++++++++++++++----------- fdroidserver/lint.py | 2 +- fdroidserver/net.py | 2 +- tests/test_common.py | 24 +++++++-------- tests/test_lint.py | 53 +++++++++++++++++++++++++++++++++ 5 files changed, 116 insertions(+), 31 deletions(-) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 06ac9cf9..b2386396 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -25,8 +25,32 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -# common.py is imported by all modules, so do not import third-party -# libraries here as they will become a requirement for all commands. + +"""Collection of functions shared by subcommands. + +This is basically the "shared library" for all the fdroid subcommands. +The contains core functionality and a number of utility functions. +This is imported by all modules, so do not import third-party +libraries here as they will become a requirement for all commands. + +Config +------ + +Parsing and using the configuration settings from config.yml is +handled here. The data format is YAML 1.2. The config has its own +supported data types: + +* Boolean (e.g. deploy_process_logs:) +* Integer (e.g. archive_older:, repo_maxage:) +* String-only (e.g. repo_name:, sdk_path:) +* Multi-String (string, list of strings, or list of dicts with + strings, e.g. serverwebroot:, mirrors:) + +String-only fields can also use a special value {env: varname}, which +is a dict with a single key 'env' and a value that is the name of the +environment variable to include. + +""" import copy import difflib @@ -586,12 +610,15 @@ def read_config(): fill_config_defaults(config) if 'serverwebroot' in config: - roots = parse_mirrors_config(config['serverwebroot']) + roots = parse_list_of_dicts(config['serverwebroot']) rootlist = [] for d in roots: # since this is used with rsync, where trailing slashes have # meaning, ensure there is always a trailing slash - rootstr = d['url'] + rootstr = d.get('url') + if not rootstr: + logging.error('serverwebroot: has blank value!') + continue if rootstr[-1] != '/': rootstr += '/' d['url'] = rootstr.replace('//', '/') @@ -599,7 +626,7 @@ def read_config(): config['serverwebroot'] = rootlist if 'servergitmirrors' in config: - config['servergitmirrors'] = parse_mirrors_config(config['servergitmirrors']) + config['servergitmirrors'] = parse_list_of_dicts(config['servergitmirrors']) limit = config['git_mirror_size_limit'] config['git_mirror_size_limit'] = parse_human_readable_size(limit) @@ -666,18 +693,23 @@ def expand_env_dict(s): return os.path.expanduser(s) -def parse_mirrors_config(mirrors): - """Mirrors can be specified as a string, list of strings, or dictionary map.""" - if isinstance(mirrors, str): - return [{"url": expand_env_dict(mirrors)}] - if isinstance(mirrors, dict): - return [{"url": expand_env_dict(mirrors)}] - if all(isinstance(item, str) for item in mirrors): - return [{'url': expand_env_dict(i)} for i in mirrors] - if all(isinstance(item, dict) for item in mirrors): - for item in mirrors: +def parse_list_of_dicts(l_of_d): + """Parse config data structure that is a list of dicts of strings. + + The value can be specified as a string, list of strings, or list of dictionary maps + where the values are strings. + + """ + if isinstance(l_of_d, str): + return [{"url": expand_env_dict(l_of_d)}] + if isinstance(l_of_d, dict): + return [{"url": expand_env_dict(l_of_d)}] + if all(isinstance(item, str) for item in l_of_d): + return [{'url': expand_env_dict(i)} for i in l_of_d] + if all(isinstance(item, dict) for item in l_of_d): + for item in l_of_d: item['url'] = expand_env_dict(item['url']) - return mirrors + return l_of_d raise TypeError(_('only accepts strings, lists, and tuples')) @@ -690,7 +722,7 @@ def get_mirrors(url, filename=None): if url.netloc == 'f-droid.org': mirrors = FDROIDORG_MIRRORS else: - mirrors = parse_mirrors_config(url.geturl()) + mirrors = parse_list_of_dicts(url.geturl()) if filename: return append_filename_to_mirrors(filename, mirrors) diff --git a/fdroidserver/lint.py b/fdroidserver/lint.py index 4e62a404..ce541c4d 100644 --- a/fdroidserver/lint.py +++ b/fdroidserver/lint.py @@ -899,7 +899,7 @@ def lint_config(arg): show_error = False if t is str: - if type(data[key]) not in (str, dict): + if type(data[key]) not in (str, list, dict): passed = False show_error = True elif type(data[key]) != t: diff --git a/fdroidserver/net.py b/fdroidserver/net.py index 5c6e0144..1ec7d096 100644 --- a/fdroidserver/net.py +++ b/fdroidserver/net.py @@ -92,7 +92,7 @@ def download_using_mirrors(mirrors, local_filename=None): logic will try it twice: first without SNI, then again with SNI. """ - mirrors = common.parse_mirrors_config(mirrors) + mirrors = common.parse_list_of_dicts(mirrors) mirror_configs_to_try = [] for mirror in mirrors: mirror_configs_to_try.append(mirror) diff --git a/tests/test_common.py b/tests/test_common.py index bbdaa016..c6f90890 100755 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -2903,46 +2903,46 @@ class CommonTest(unittest.TestCase): with self.assertRaises(TypeError): fdroidserver.common.expand_env_dict({'env': 'foo', 'foo': 'bar'}) - def test_parse_mirrors_config_str(self): + def test_parse_list_of_dicts_str(self): s = 'foo@example.com:/var/www' mirrors = yaml.load("""'%s'""" % s) self.assertEqual( - [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors) + [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors) ) - def test_parse_mirrors_config_list(self): + def test_parse_list_of_dicts_list(self): s = 'foo@example.com:/var/www' mirrors = yaml.load("""- '%s'""" % s) self.assertEqual( - [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors) + [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors) ) - def test_parse_mirrors_config_dict(self): + def test_parse_list_of_dicts_dict(self): s = 'foo@example.com:/var/www' mirrors = yaml.load("""- url: '%s'""" % s) self.assertEqual( - [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors) + [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors) ) @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH'), 'foo': 'bar'}, clear=True) - def test_parse_mirrors_config_env_str(self): + def test_parse_list_of_dicts_env_str(self): mirrors = yaml.load('{env: foo}') self.assertEqual( - [{'url': 'bar'}], fdroidserver.common.parse_mirrors_config(mirrors) + [{'url': 'bar'}], fdroidserver.common.parse_list_of_dicts(mirrors) ) - def test_parse_mirrors_config_env_list(self): + def test_parse_list_of_dicts_env_list(self): s = 'foo@example.com:/var/www' mirrors = yaml.load("""- '%s'""" % s) self.assertEqual( - [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors) + [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors) ) - def test_parse_mirrors_config_env_dict(self): + def test_parse_list_of_dicts_env_dict(self): s = 'foo@example.com:/var/www' mirrors = yaml.load("""- url: '%s'""" % s) self.assertEqual( - [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors) + [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors) ) def test_KnownApks_recordapk(self): diff --git a/tests/test_lint.py b/tests/test_lint.py index 95752cb9..5a6a1001 100755 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -4,6 +4,7 @@ import logging import os import shutil import tempfile +import textwrap import unittest from pathlib import Path @@ -534,6 +535,13 @@ class LintAntiFeaturesTest(unittest.TestCase): class ConfigYmlTest(LintTest): + """Test data formats used in config.yml. + + lint.py uses print() and not logging so hacks are used to control + the output when running in the test runner. + + """ + def setUp(self): super().setUp() self.config_yml = Path(self.testdir) / fdroidserver.common.CONFIG_FILE @@ -550,6 +558,22 @@ class ConfigYmlTest(LintTest): self.config_yml.write_text('sdk_path: /opt/android-sdk\n') self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) + def test_config_yml_str_list(self): + self.config_yml.write_text('serverwebroot: [server1, server2]\n') + self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) + + def test_config_yml_str_list_of_dicts(self): + self.config_yml.write_text( + textwrap.dedent( + """\ + serverwebroot: + - url: 'me@b.az:/srv/fdroid' + index_only: true + """ + ) + ) + self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) + def test_config_yml_str_list_of_dicts_env(self): """serverwebroot can be str, list of str, or list of dicts.""" self.config_yml.write_text('serverwebroot: {env: ANDROID_HOME}\n') @@ -595,3 +619,32 @@ class ConfigYmlTest(LintTest): fdroidserver.lint.lint_config(self.config_yml), f'{key} should fail on value of "{value}"', ) + + def test_config_yml_keyaliases(self): + self.config_yml.write_text( + textwrap.dedent( + """\ + keyaliases: + com.example: myalias + com.foo: '@com.example' + """ + ) + ) + self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) + + def test_config_yml_keyaliases_bad_str(self): + """The keyaliases: value is a dict not a str.""" + self.config_yml.write_text("keyaliases: '@com.example'\n") + self.assertFalse(fdroidserver.lint.lint_config(self.config_yml)) + + def test_config_yml_keyaliases_bad_list(self): + """The keyaliases: value is a dict not a list.""" + self.config_yml.write_text( + textwrap.dedent( + """\ + keyaliases: + - com.example: myalias + """ + ) + ) + self.assertFalse(fdroidserver.lint.lint_config(self.config_yml))