From c6ad8505d48b94d7c3c093861b8eb79eac3e1bf3 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 20 Apr 2023 23:24:57 +0200 Subject: [PATCH 01/10] some easier fixes for black code format --- fdroidserver/metadata.py | 65 +++++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 24 deletions(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 4714aae9..e0e79b44 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -109,7 +109,6 @@ yaml_app_fields = [x for x in yaml_app_field_order if x != '\n'] class App(dict): - def __init__(self, copydict=None): if copydict: super().__init__(copydict) @@ -252,7 +251,6 @@ build_flags = [ class Build(dict): - def __init__(self, copydict=None): super().__init__() self.disable = '' @@ -375,7 +373,7 @@ def flagtype(name): return TYPE_STRING -class FieldValidator(): +class FieldValidator: """Designate App metadata field types and checks that it matches. 'name' - The long name of the field type @@ -495,8 +493,9 @@ def parse_yaml_srclib(metadatapath): with symlink.open("r", encoding="utf-8") as s: data = yaml.load(s) if type(data) is not dict: - raise YAMLError(_('{file} is blank or corrupt!') - .format(file=metadatapath)) + raise YAMLError( + _('{file} is blank or corrupt!').format(file=metadatapath) + ) except YAMLError as e: _warn_or_exception(_("Invalid srclib metadata: could not " "parse '{file}'") @@ -507,9 +506,11 @@ def parse_yaml_srclib(metadatapath): for key in data: if key not in thisinfo: - _warn_or_exception(_("Invalid srclib metadata: unknown key " - "'{key}' in '{file}'") - .format(key=key, file=metadatapath)) + _warn_or_exception( + _("Invalid srclib metadata: unknown key '{key}' in '{file}'").format( + key=key, file=metadatapath + ) + ) return thisinfo else: if key == 'Subdir': @@ -580,7 +581,8 @@ def read_metadata(appids={}, sort_by_time=False): metadatafiles = common.get_metadata_files(vercodes) else: metadatafiles = list(Path('metadata').glob('*.yml')) + list( - Path('.').glob('.fdroid.yml')) + Path('.').glob('.fdroid.yml') + ) if sort_by_time: entries = ((path.stat().st_mtime, path) for path in metadatafiles) @@ -594,11 +596,15 @@ def read_metadata(appids={}, sort_by_time=False): for metadatapath in metadatafiles: appid = metadatapath.stem if appid != '.fdroid' and not common.is_valid_package_name(appid): - _warn_or_exception(_("{appid} from {path} is not a valid Java Package Name!") - .format(appid=appid, path=metadatapath)) + _warn_or_exception( + _("{appid} from {path} is not a valid Java Package Name!").format( + appid=appid, path=metadatapath + ) + ) if appid in apps: - _warn_or_exception(_("Found multiple metadata files for {appid}") - .format(appid=appid)) + _warn_or_exception( + _("Found multiple metadata files for {appid}").format(appid=appid) + ) app = parse_metadata(metadatapath) check_metadata(app) apps[app.id] = app @@ -654,8 +660,9 @@ def parse_metadata(metadatapath): with metadatapath.open('r', encoding='utf-8') as mf: app.update(parse_yaml_metadata(mf)) else: - _warn_or_exception(_('Unknown metadata format: {path} (use: *.yml)') - .format(path=metadatapath)) + _warn_or_exception( + _('Unknown metadata format: {path} (use: *.yml)').format(path=metadatapath) + ) if metadatapath.name != '.fdroid.yml' and app.Repo: build_dir = common.get_build_dir(app) @@ -663,11 +670,15 @@ def parse_metadata(metadatapath): if metadata_in_repo.is_file(): try: commit_id = common.get_head_commit_id(git.Repo(build_dir)) - logging.debug(_('Including metadata from %s@%s') % (metadata_in_repo, commit_id)) + logging.debug( + _('Including metadata from %s@%s') % (metadata_in_repo, commit_id) + ) # See https://github.com/PyCQA/pylint/issues/2856 . # pylint: disable-next=no-member except git.exc.InvalidGitRepositoryError: - logging.debug(_('Including metadata from {path}').format(metadata_in_repo)) + logging.debug( + _('Including metadata from {path}').format(metadata_in_repo) + ) app_in_repo = parse_metadata(metadata_in_repo) for k, v in app_in_repo.items(): if k not in app: @@ -711,10 +722,12 @@ def parse_yaml_metadata(mf): yaml = YAML(typ='safe') yamldata = yaml.load(mf) except YAMLError as e: - _warn_or_exception(_("could not parse '{path}'") - .format(path=mf.name) + '\n' - + common.run_yamllint(mf.name, indent=4), - cause=e) + _warn_or_exception( + _("could not parse '{path}'").format(path=mf.name) + + '\n' + + common.run_yamllint(mf.name, indent=4), + cause=e, + ) if yamldata is None or yamldata == '': yamldata = dict() @@ -790,7 +803,7 @@ def post_parse_yaml_metadata(yamldata): for k, v in yamldata.items(): if fieldtype(k) == TYPE_LIST: if isinstance(v, str): - yamldata[k] = [v, ] + yamldata[k] = [v] elif v: yamldata[k] = [str(i) for i in v] elif fieldtype(k) == TYPE_INT: @@ -930,7 +943,9 @@ def write_yaml(mf, app): if hasattr(build, field): value = getattr(build, field) if field == 'gradle' and value == ['off']: - value = [ruamel.yaml.scalarstring.SingleQuotedScalarString('off')] + value = [ + ruamel.yaml.scalarstring.SingleQuotedScalarString('off') + ] typ = flagtype(field) # don't check value == True for TYPE_INT as it could be 0 if value is not None and (typ == TYPE_INT or value): @@ -960,7 +975,9 @@ def write_metadata(metadatapath, app): with metadatapath.open('w') as mf: return write_yaml(mf, app) else: - raise FDroidException(_('ruamel.yaml not installed, can not write metadata.')) + raise FDroidException( + _('ruamel.yaml not installed, can not write metadata.') + ) _warn_or_exception(_('Unknown metadata format: %s') % metadatapath) From e5fda5469373b6741deda1cb30af173d8352a5f2 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 24 Apr 2023 16:54:02 +0200 Subject: [PATCH 02/10] add test_check_metadata_AntiFeatures --- tests/metadata.TestCase | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 04da68ec..eed1baf4 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -180,6 +180,26 @@ class MetadataTest(unittest.TestCase): 'fake.app.id', ) + def test_check_metadata_AntiFeatures(self): + fdroidserver.metadata.warnings_action = 'error' + + app = fdroidserver.metadata.App() + self.assertIsNone(metadata.check_metadata(app)) + + app['AntiFeatures'] = [] + self.assertIsNone(metadata.check_metadata(app)) + + app['AntiFeatures'] = ['Ads', 'UpstreamNonFree'] + self.assertIsNone(metadata.check_metadata(app)) + + app['AntiFeatures'] = ['Ad'] + with self.assertRaises(fdroidserver.exception.MetaDataException): + metadata.check_metadata(app) + + app['AntiFeatures'] = ['Adss'] + with self.assertRaises(fdroidserver.exception.MetaDataException): + metadata.check_metadata(app) + def test_valid_funding_yml_regex(self): """Check the regex can find all the cases""" with (self.basedir / 'funding-usernames.yaml').open() as fp: From e794ccb38ca023ab05f12d7ebf33389f997ad13c Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 24 Apr 2023 14:46:03 +0200 Subject: [PATCH 03/10] work towards switching fdroidserver/metadata.py to black --- fdroidserver/metadata.py | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index e0e79b44..119a42c3 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -397,8 +397,13 @@ class FieldValidator: values = [v] for v in values: if not self.compiled.match(v): - _warn_or_exception(_("'{value}' is not a valid {field} in {appid}. Regex pattern: {pattern}") - .format(value=v, field=self.name, appid=appid, pattern=self.matching)) + _warn_or_exception( + _( + "'{value}' is not a valid {field} in {appid}. Regex pattern: {pattern}" + ).format( + value=v, field=self.name, appid=appid, pattern=self.matching + ) + ) # Generic value types @@ -469,16 +474,14 @@ def check_metadata(app): def parse_yaml_srclib(metadatapath): - - thisinfo = {'RepoType': '', - 'Repo': '', - 'Subdir': None, - 'Prepare': None} + thisinfo = {'RepoType': '', 'Repo': '', 'Subdir': None, 'Prepare': None} if not metadatapath.exists(): - _warn_or_exception(_("Invalid scrlib metadata: '{file}' " - "does not exist" - .format(file=metadatapath))) + _warn_or_exception( + _("Invalid scrlib metadata: '{file}' does not exist").format( + file=metadatapath + ) + ) return thisinfo with metadatapath.open("r", encoding="utf-8") as f: @@ -984,5 +987,9 @@ def write_metadata(metadatapath, app): def add_metadata_arguments(parser): """Add common command line flags related to metadata processing.""" - parser.add_argument("-W", choices=['error', 'warn', 'ignore'], default='error', - help=_("force metadata errors (default) to be warnings, or to be ignored.")) + parser.add_argument( + "-W", + choices=['error', 'warn', 'ignore'], + default='error', + help=_("force metadata errors (default) to be warnings, or to be ignored."), + ) From 8300ed051b8fb9e8924702b3418fead08f70dfbf Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 24 Apr 2023 22:50:30 +0200 Subject: [PATCH 04/10] ruamel.yaml is now required and the packages are all new enough --- fdroidserver/metadata.py | 40 +++++++++----------------------------- tests/rewritemeta.TestCase | 21 -------------------- 2 files changed, 9 insertions(+), 52 deletions(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 119a42c3..f5302f05 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -24,14 +24,12 @@ import platform import os import re import logging -import importlib +import ruamel.yaml from collections import OrderedDict -from ruamel.yaml import YAML, YAMLError - from . import common from . import _ -from .exception import MetaDataException, FDroidException +from .exception import MetaDataException srclibs = None warnings_action = None @@ -486,7 +484,7 @@ def parse_yaml_srclib(metadatapath): with metadatapath.open("r", encoding="utf-8") as f: try: - yaml = YAML(typ='safe') + yaml = ruamel.yaml.YAML(typ='safe') data = yaml.load(f) if type(data) is not dict: if platform.system() == 'Windows': @@ -496,10 +494,10 @@ def parse_yaml_srclib(metadatapath): with symlink.open("r", encoding="utf-8") as s: data = yaml.load(s) if type(data) is not dict: - raise YAMLError( + raise ruamel.yaml.YAMLError( _('{file} is blank or corrupt!').format(file=metadatapath) ) - except YAMLError as e: + except ruamel.yaml.YAMLError as e: _warn_or_exception(_("Invalid srclib metadata: could not " "parse '{file}'") .format(file=metadatapath) + '\n' @@ -722,9 +720,9 @@ def parse_yaml_metadata(mf): """ try: - yaml = YAML(typ='safe') + yaml = ruamel.yaml.YAML(typ='safe') yamldata = yaml.load(mf) - except YAMLError as e: + except ruamel.yaml.YAMLError as e: _warn_or_exception( _("could not parse '{path}'").format(path=mf.name) + '\n' @@ -867,21 +865,6 @@ def write_yaml(mf, app): app app metadata to written to the yaml file """ - # import rumael.yaml and check version - try: - import ruamel.yaml - except ImportError as e: - raise FDroidException('ruamel.yaml not installed, can not write metadata.') from e - if not ruamel.yaml.__version__: - raise FDroidException('ruamel.yaml.__version__ not accessible. Please make sure a ruamel.yaml >= 0.13 is installed..') - m = re.match(r'(?P[0-9]+)\.(?P[0-9]+)\.(?P[0-9]+)(-.+)?', - ruamel.yaml.__version__) - if not m: - raise FDroidException('ruamel.yaml version malfored, please install an upstream version of ruamel.yaml') - if int(m.group('major')) < 0 or int(m.group('minor')) < 13: - raise FDroidException('currently installed version of ruamel.yaml ({}) is too old, >= 1.13 required.'.format(ruamel.yaml.__version__)) - # suiteable version ruamel.yaml imported successfully - def _field_to_yaml(typ, value): """Convert data to YAML 1.2 format that keeps the right TYPE_*.""" if typ is TYPE_STRING: @@ -974,13 +957,8 @@ def write_yaml(mf, app): def write_metadata(metadatapath, app): metadatapath = Path(metadatapath) if metadatapath.suffix == '.yml': - if importlib.util.find_spec('ruamel.yaml'): - with metadatapath.open('w') as mf: - return write_yaml(mf, app) - else: - raise FDroidException( - _('ruamel.yaml not installed, can not write metadata.') - ) + with metadatapath.open('w') as mf: + return write_yaml(mf, app) _warn_or_exception(_('Unknown metadata format: %s') % metadatapath) diff --git a/tests/rewritemeta.TestCase b/tests/rewritemeta.TestCase index 2cd397d2..283e9c44 100755 --- a/tests/rewritemeta.TestCase +++ b/tests/rewritemeta.TestCase @@ -7,10 +7,8 @@ import sys import unittest import tempfile import textwrap -from unittest import mock from pathlib import Path - from testcommon import TmpCwd localmodule = Path(__file__).resolve().parent.parent @@ -20,7 +18,6 @@ if localmodule not in sys.path: from fdroidserver import common from fdroidserver import rewritemeta -from fdroidserver.exception import FDroidException class RewriteMetaTest(unittest.TestCase): @@ -71,24 +68,6 @@ class RewriteMetaTest(unittest.TestCase): ), ) - def test_rewrite_scenario_yml_no_ruamel(self): - sys.argv = ['rewritemeta', 'a'] - with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): - Path('metadata').mkdir() - with Path('metadata/a.yml').open('w') as f: - f.write('AutoName: a') - - def boom(*args): - raise FDroidException(' '.join((str(x) for x in args))) - - with mock.patch('importlib.util.find_spec', boom): - with self.assertRaises(FDroidException): - rewritemeta.main() - - self.assertEqual( - Path('metadata/a.yml').read_text(encoding='utf-8'), 'AutoName: a' - ) - if __name__ == "__main__": parser = optparse.OptionParser() From 74dddfd9fbe562304f9b152a8523e38bb9456ac4 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 2 May 2023 10:57:35 +0200 Subject: [PATCH 05/10] refactor App.get_last_build() to checkupdates This function is only used in checkupdates, and removing it from the App class moves the App class one step closer to being a plain dict, which is a more Pythonic style. --- fdroidserver/checkupdates.py | 18 +++++++++++------- fdroidserver/metadata.py | 6 ------ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/fdroidserver/checkupdates.py b/fdroidserver/checkupdates.py index b00ec572..d5a136ad 100644 --- a/fdroidserver/checkupdates.py +++ b/fdroidserver/checkupdates.py @@ -110,7 +110,7 @@ def check_tags(app, pattern): vcs.gotorevision(None) - last_build = app.get_last_build() + last_build = get_last_build_from_app(app) try_init_submodules(app, last_build, vcs) @@ -252,10 +252,7 @@ def check_repomanifest(app, branch=None): elif repotype == 'bzr': vcs.gotorevision(None) - last_build = metadata.Build() - if app.get('Builds', []): - last_build = app.get('Builds', [])[-1] - + last_build = get_last_build_from_app(app) try_init_submodules(app, last_build, vcs) hpak = None @@ -364,7 +361,7 @@ def possible_subdirs(app): else: build_dir = Path('build') / app.id - last_build = app.get_last_build() + last_build = get_last_build_from_app(app) for d in dirs_with_manifest(build_dir): m_paths = common.manifest_paths(d, last_build.gradle) @@ -399,7 +396,7 @@ def fetch_autoname(app, tag): except VCSException: return None - last_build = app.get_last_build() + last_build = get_last_build_from_app(app) logging.debug("...fetch auto name from " + str(build_dir)) new_name = None @@ -579,6 +576,13 @@ def checkupdates_app(app): raise FDroidException("Git commit failed") +def get_last_build_from_app(app): + if app.get('Builds'): + return app['Builds'][-1] + else: + return metadata.Build() + + def status_update_json(processed, failed): """Output a JSON file with metadata about this run.""" logging.debug(_('Outputting JSON')) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index f5302f05..1298f255 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -174,12 +174,6 @@ class App(dict): else: raise AttributeError("No such attribute: " + name) - def get_last_build(self): - if len(self.Builds) > 0: - return self.Builds[-1] - else: - return Build() - TYPE_STRING = 2 TYPE_BOOL = 3 From 822439dff524c852cbe67b33b9b20f476b3308fb Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 2 May 2023 11:49:17 +0200 Subject: [PATCH 06/10] remove exception for ruamel.yaml on Debian/stretch stretch is no more, and this code base specifies Python 3.9 as minimum. --- fdroidserver/metadata.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 1298f255..5f16beac 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -940,12 +940,9 @@ def write_yaml(mf, app): return builds yaml_app = _app_to_yaml(app) - try: - yaml = ruamel.yaml.YAML() - yaml.indent(mapping=4, sequence=4, offset=2) - yaml.dump(yaml_app, stream=mf) - except AttributeError: # Debian/stretch's version does not have YAML() - ruamel.yaml.round_trip_dump(yaml_app, mf, indent=4, block_seq_indent=2) + yaml = ruamel.yaml.YAML() + yaml.indent(mapping=4, sequence=4, offset=2) + yaml.dump(yaml_app, stream=mf) def write_metadata(metadatapath, app): From 0b3fd725c3fa4e92a172161f8e9253f42ff1a02b Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 24 Apr 2023 23:18:33 +0200 Subject: [PATCH 07/10] metadata.TestCase: metadata.warnings_action = 'error' as default The default state for the tests should be to fail on errors. --- tests/metadata.TestCase | 46 ++++++++++++----------------------------- 1 file changed, 13 insertions(+), 33 deletions(-) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index eed1baf4..2143d147 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -9,7 +9,6 @@ import random import shutil import sys import unittest -from unittest import mock import tempfile import textwrap from collections import OrderedDict @@ -39,6 +38,7 @@ class MetadataTest(unittest.TestCase): logging.basicConfig(level=logging.DEBUG) self.basedir = localmodule / 'tests' os.chdir(self.basedir) + fdroidserver.metadata.warnings_action = 'error' def tearDown(self): # auto-generated dirs by functions, not tests, so they are not always cleaned up @@ -67,8 +67,6 @@ class MetadataTest(unittest.TestCase): break self.assertIsNotNone(validator, "could not find 'Bitcoin address' validator") - fdroidserver.metadata.warnings_action = 'error' - # some valid addresses (P2PKH, P2SH, Bech32) self.assertIsNone( validator.check('1BrrrrErsrWetrTrnrrrrm4GFg7xJaNVN2', 'fake.app.id') @@ -126,8 +124,6 @@ class MetadataTest(unittest.TestCase): break self.assertIsNotNone(validator, "could not find 'Litecoin address' validator") - fdroidserver.metadata.warnings_action = 'error' - # some valid addresses (L, M, 3, segwit) self.assertIsNone( validator.check('LgeGrrrrJAxyXprrPrrBrrX5Qrrrrrrrrd', 'fake.app.id') @@ -326,7 +322,6 @@ class MetadataTest(unittest.TestCase): self.assertEqual('true', metadata._normalize_type_string(True)) def test_post_parse_yaml_metadata(self): - fdroidserver.metadata.warnings_action = 'error' yamldata = OrderedDict() builds = [] yamldata['Builds'] = builds @@ -425,9 +420,8 @@ class MetadataTest(unittest.TestCase): ) ) mf.name = 'mock_filename.yaml' - with mock.patch('fdroidserver.metadata.warnings_action', 'error'): - with self.assertRaises(MetaDataException): - fdroidserver.metadata.parse_yaml_metadata(mf) + with self.assertRaises(MetaDataException): + fdroidserver.metadata.parse_yaml_metadata(mf) def test_parse_yaml_metadata_unknown_build_flag(self): mf = io.StringIO( @@ -440,9 +434,8 @@ class MetadataTest(unittest.TestCase): ) ) mf.name = 'mock_filename.yaml' - with mock.patch('fdroidserver.metadata.warnings_action', 'error'): - with self.assertRaises(MetaDataException): - fdroidserver.metadata.parse_yaml_metadata(mf) + with self.assertRaises(MetaDataException): + fdroidserver.metadata.parse_yaml_metadata(mf) def test_parse_yaml_metadata_continue_on_warning(self): """When errors are disabled, parsing should provide something that can work. @@ -474,9 +467,8 @@ class MetadataTest(unittest.TestCase): """ ) ) - with mock.patch('fdroidserver.metadata.warnings_action', 'error'): - with self.assertRaises(MetaDataException): - fdroidserver.metadata.parse_yaml_srclib(srclibfile) + with self.assertRaises(MetaDataException): + fdroidserver.metadata.parse_yaml_srclib(srclibfile) def test_write_yaml_with_placeholder_values(self): mf = io.StringIO() @@ -555,8 +547,7 @@ class MetadataTest(unittest.TestCase): ) mf.name = 'mock_filename.yaml' mf.seek(0) - with mock.patch('fdroidserver.metadata.warnings_action', 'error'): - result = fdroidserver.metadata.parse_yaml_metadata(mf) + result = fdroidserver.metadata.parse_yaml_metadata(mf) self.maxDiff = None self.assertDictEqual( result, @@ -610,8 +601,7 @@ class MetadataTest(unittest.TestCase): ) mf.name = 'mock_filename.yaml' mf.seek(0) - with mock.patch('fdroidserver.metadata.warnings_action', 'error'): - result = fdroidserver.metadata.parse_yaml_metadata(mf) + result = fdroidserver.metadata.parse_yaml_metadata(mf) self.maxDiff = None self.assertDictEqual( result, @@ -660,8 +650,7 @@ class MetadataTest(unittest.TestCase): ) mf.name = 'mock_filename.yaml' mf.seek(0) - with mock.patch('fdroidserver.metadata.warnings_action', 'error'): - result = fdroidserver.metadata.parse_yaml_metadata(mf) + result = fdroidserver.metadata.parse_yaml_metadata(mf) self.assertDictEqual( result, { @@ -694,10 +683,9 @@ class MetadataTest(unittest.TestCase): ) mf.name = 'mock_filename.yaml' mf.seek(0) - with mock.patch('fdroidserver.metadata.warnings_action', 'error'): - result = fdroidserver.metadata.parse_yaml_metadata(mf) - self.assertNotIn('Provides', result) - self.assertNotIn('provides', result) + result = fdroidserver.metadata.parse_yaml_metadata(mf) + self.assertNotIn('Provides', result) + self.assertNotIn('provides', result) def test_write_yaml_1_line_scripts_as_string(self): mf = io.StringIO() @@ -923,7 +911,6 @@ class MetadataTest(unittest.TestCase): ) def test_parse_yaml_srclib_unknown_key(self): - fdroidserver.metadata.warnings_action = 'error' with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): with Path('test.yml').open('w', encoding='utf-8') as f: f.write( @@ -942,7 +929,6 @@ class MetadataTest(unittest.TestCase): fdroidserver.metadata.parse_yaml_srclib(Path('test.yml')) def test_parse_yaml_srclib_does_not_exists(self): - fdroidserver.metadata.warnings_action = 'error' with self.assertRaisesRegex( MetaDataException, "Invalid scrlib metadata: " @@ -954,7 +940,6 @@ class MetadataTest(unittest.TestCase): ) def test_parse_yaml_srclib_simple(self): - fdroidserver.metadata.warnings_action = 'error' with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): with Path('simple.yml').open('w', encoding='utf-8') as f: f.write( @@ -978,7 +963,6 @@ class MetadataTest(unittest.TestCase): ) def test_parse_yaml_srclib_simple_with_blanks(self): - fdroidserver.metadata.warnings_action = 'error' with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): with Path('simple.yml').open('w', encoding='utf-8') as f: f.write( @@ -1008,7 +992,6 @@ class MetadataTest(unittest.TestCase): ) def test_parse_yaml_srclib_Changelog_cketti(self): - fdroidserver.metadata.warnings_action = 'error' with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): with Path('Changelog-cketti.yml').open('w', encoding='utf-8') as f: f.write( @@ -1040,7 +1023,6 @@ class MetadataTest(unittest.TestCase): ) def test_read_srclibs_yml_subdir_list(self): - fdroidserver.metadata.warnings_action = 'error' fdroidserver.metadata.srclibs = None with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): Path('srclibs').mkdir() @@ -1092,7 +1074,6 @@ class MetadataTest(unittest.TestCase): ) def test_read_srclibs_yml_prepare_list(self): - fdroidserver.metadata.warnings_action = 'error' fdroidserver.metadata.srclibs = None with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): Path('srclibs').mkdir() @@ -1133,7 +1114,6 @@ class MetadataTest(unittest.TestCase): ) def test_read_srclibs(self): - fdroidserver.metadata.warnings_action = 'error' fdroidserver.metadata.srclibs = None with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): Path('srclibs').mkdir() From 28ea6cea7fe050dc47c1a844ca4062524c983c3f Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 25 Apr 2023 11:04:32 +0200 Subject: [PATCH 08/10] add tests of TYPE_LIST parsing --- tests/metadata.TestCase | 67 ++++++++++++++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 11 deletions(-) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 2143d147..a60841b2 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -14,9 +14,7 @@ import textwrap from collections import OrderedDict from pathlib import Path - from testcommon import TmpCwd - from ruamel.yaml import YAML yaml = YAML(typ='safe') @@ -31,6 +29,12 @@ from fdroidserver import metadata from fdroidserver.exception import MetaDataException +def _get_mock_mf(s): + mf = io.StringIO(s) + mf.name = 'mock_filename.yaml' + return mf + + class MetadataTest(unittest.TestCase): '''fdroidserver/metadata.py''' @@ -322,6 +326,20 @@ class MetadataTest(unittest.TestCase): self.assertEqual('true', metadata._normalize_type_string(True)) def test_post_parse_yaml_metadata(self): + yamldata = dict() + metadata.post_parse_yaml_metadata(yamldata) + + yamldata[ + 'AllowedAPKSigningKeys' + ] = 'c03dac71394d6c26766f1b04d3e31cfcac5d03b55d8aa40cc9b9fa6b74354c66' + metadata.post_parse_yaml_metadata(yamldata) + + def test_post_parse_yaml_metadata_fails(self): + yamldata = {'AllowedAPKSigningKeys': True} + with self.assertRaises(TypeError): + metadata.post_parse_yaml_metadata(yamldata) + + def test_post_parse_yaml_metadata_builds(self): yamldata = OrderedDict() builds = [] yamldata['Builds'] = builds @@ -395,17 +413,45 @@ class MetadataTest(unittest.TestCase): self.assertEqual(randomlist, allappids) def test_parse_yaml_metadata_0size_file(self): - mf = io.StringIO('') - mf.name = 'mock_filename.yaml' - self.assertEqual(fdroidserver.metadata.parse_yaml_metadata(mf), dict()) + self.assertEqual(dict(), metadata.parse_yaml_metadata(_get_mock_mf(''))) def test_parse_yaml_metadata_empty_dict_file(self): - mf = io.StringIO('{}') - mf.name = 'mock_filename.yaml' - self.assertEqual(fdroidserver.metadata.parse_yaml_metadata(mf), dict()) + self.assertEqual(dict(), metadata.parse_yaml_metadata(_get_mock_mf('{}'))) def test_parse_yaml_metadata_empty_string_file(self): - mf = io.StringIO('""') + self.assertEqual(dict(), metadata.parse_yaml_metadata(_get_mock_mf('""'))) + + def test_parse_yaml_metadata_fail_on_root_list(self): + with self.assertRaises(MetaDataException): + metadata.parse_yaml_metadata(_get_mock_mf('-')) + with self.assertRaises(MetaDataException): + metadata.parse_yaml_metadata(_get_mock_mf('[]')) + with self.assertRaises(MetaDataException): + metadata.parse_yaml_metadata(_get_mock_mf('- AutoName: fake')) + + def test_parse_yaml_metadata_type_list_str(self): + v = 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' + mf = _get_mock_mf('AllowedAPKSigningKeys: "%s"' % v) + self.assertEqual( + v, + metadata.parse_yaml_metadata(mf)['AllowedAPKSigningKeys'][0], + ) + + def test_parse_yaml_metadata_type_list_build_str(self): + mf = _get_mock_mf('Builds: [{versionCode: 1, rm: s}]') + self.assertEqual( + metadata.parse_yaml_metadata(mf), + {'Builds': [{'rm': ['s'], 'versionCode': 1}]}, + ) + + def test_parse_yaml_metadata_app_type_list_fails(self): + mf = _get_mock_mf('AllowedAPKSigningKeys: true') + with self.assertRaises(TypeError): + metadata.parse_yaml_metadata(mf) + mf = _get_mock_mf('AllowedAPKSigningKeys: 1') + with self.assertRaises(TypeError): + metadata.parse_yaml_metadata(mf) + mf.name = 'mock_filename.yaml' self.assertEqual(fdroidserver.metadata.parse_yaml_metadata(mf), dict()) @@ -449,8 +495,7 @@ class MetadataTest(unittest.TestCase): """ fdroidserver.metadata.warnings_action = None - mf = io.StringIO('[AntiFeatures: Tracking]') - mf.name = 'mock_filename.yaml' + mf = _get_mock_mf('[AntiFeatures: Tracking]') self.assertEqual(fdroidserver.metadata.parse_yaml_metadata(mf), dict()) def test_parse_yaml_srclib_corrupt_file(self): From 9a9705a667f3284c6cb7989915a31c37d744b42a Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 2 May 2023 10:59:53 +0200 Subject: [PATCH 09/10] update test_read_metadata to use ruamel.yaml and YAML 1.2 I tried to get this to indent the .yaml files properly so yamllint defaults work with tests/metadata/dump/*.yaml, but it didn't take for some reason: yaml.indent(mapping=4, sequence=4, offset=2) --- fdroidserver/metadata.py | 4 ++ tests/metadata.TestCase | 18 +++--- tests/metadata/dump/com.politedroid.yaml | 5 +- tests/metadata/dump/org.adaway.yaml | 58 +++++++++---------- .../dump/org.smssecure.smssecure.yaml | 16 ++--- tests/metadata/dump/org.videolan.vlc.yaml | 2 +- 6 files changed, 53 insertions(+), 50 deletions(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 5f16beac..a97da7ae 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -297,6 +297,10 @@ class Build(dict): else: raise AttributeError("No such attribute: " + name) + @classmethod + def to_yaml(cls, representer, node): + return representer.represent_dict(node) + def build_method(self): for f in ['maven', 'gradle']: if self.get(f): diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index a60841b2..b4301fba 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -6,6 +6,7 @@ import logging import optparse import os import random +import ruamel.yaml import shutil import sys import unittest @@ -15,9 +16,6 @@ from collections import OrderedDict from pathlib import Path from testcommon import TmpCwd -from ruamel.yaml import YAML - -yaml = YAML(typ='safe') localmodule = Path(__file__).resolve().parent.parent print('localmodule: ' + str(localmodule)) @@ -203,6 +201,7 @@ class MetadataTest(unittest.TestCase): def test_valid_funding_yml_regex(self): """Check the regex can find all the cases""" with (self.basedir / 'funding-usernames.yaml').open() as fp: + yaml = ruamel.yaml.YAML(typ='safe') data = yaml.load(fp) for k, entries in data.items(): @@ -220,10 +219,7 @@ class MetadataTest(unittest.TestCase): ) def test_read_metadata(self): - def _build_yaml_representer(dumper, data): - '''Creates a YAML representation of a Build instance''' - return dumper.represent_dict(data) - + """Read specified metadata files included in tests/, compare to stored output""" self.maxDiff = None config = dict() @@ -231,6 +227,7 @@ class MetadataTest(unittest.TestCase): fdroidserver.common.config = config fdroidserver.metadata.warnings_action = None + yaml = ruamel.yaml.YAML(typ='safe') apps = fdroidserver.metadata.read_metadata() for appid in ( 'org.smssecure.smssecure', @@ -246,9 +243,10 @@ class MetadataTest(unittest.TestCase): self.assertEqual(frommeta, from_yaml) # comment above assert and uncomment below to update test # files when new metadata fields are added - # with savepath.open('w') as f: - # yaml.add_representer(fdroidserver.metadata.Build, _build_yaml_representer) - # yaml.dump(frommeta, f, default_flow_style=False) + # with savepath.open('w') as fp: + # yaml.default_flow_style = False + # yaml.register_class(metadata.Build) + # yaml.dump(frommeta, fp) def test_rewrite_yaml_fakeotaupdate(self): with tempfile.TemporaryDirectory() as testdir: diff --git a/tests/metadata/dump/com.politedroid.yaml b/tests/metadata/dump/com.politedroid.yaml index acde9299..783308b7 100644 --- a/tests/metadata/dump/com.politedroid.yaml +++ b/tests/metadata/dump/com.politedroid.yaml @@ -1,5 +1,6 @@ AllowedAPKSigningKeys: [] -AntiFeatures: ['NonFreeNet'] +AntiFeatures: +- NonFreeNet ArchivePolicy: 4 versions AuthorEmail: null AuthorName: null @@ -130,7 +131,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null diff --git a/tests/metadata/dump/org.adaway.yaml b/tests/metadata/dump/org.adaway.yaml index 57fab70c..d9b26cb9 100644 --- a/tests/metadata/dump/org.adaway.yaml +++ b/tests/metadata/dump/org.adaway.yaml @@ -15,7 +15,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: ea5378a94ee0dc1d99d2cec95fae7e6d81afb2b9 disable: '' encoding: null @@ -51,7 +51,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: 4128e59da2eac5c2904c7c7568d298ca51e79540 disable: '' encoding: null @@ -88,7 +88,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: 0b9985398b9eef7baf6aadd0dbb12002bc199d2e disable: '' encoding: null @@ -125,7 +125,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: ab27f4dab5f3ea5e228cfb4a6b0e1fbf53695f22 disable: '' encoding: null @@ -162,7 +162,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: 695e3801e4081026c8f7213a2345fc451d5eb89c disable: '' encoding: null @@ -199,7 +199,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: 65138c11cc8b6affd28b68e125fbc1dff0886a4e disable: '' encoding: null @@ -271,7 +271,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: f811e53e1e1d2ee047b18715fd7d2072b90ae76b disable: '' encoding: null @@ -308,7 +308,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: ff97932761cdee68638dc2550751a64b2cbe18e7 disable: '' encoding: null @@ -345,7 +345,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: 33d4d80998f30bafc88c04c80cbae00b03916f99 disable: '' encoding: null @@ -382,7 +382,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: 743d25a7e287505461f33f4b8e57e4cf988fffea disable: '' encoding: null @@ -419,7 +419,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: eaa07f4 disable: '' encoding: null @@ -688,7 +688,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: v2.1 disable: '' encoding: null @@ -738,7 +738,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: v2.3 disable: '' encoding: null @@ -784,7 +784,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: v2.6 disable: '' encoding: null @@ -792,7 +792,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null @@ -822,7 +822,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: v2.7 disable: '' encoding: null @@ -830,7 +830,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null @@ -860,7 +860,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: v2.8 disable: '' encoding: null @@ -868,7 +868,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null @@ -898,7 +898,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: v2.8.1 disable: '' encoding: null @@ -906,7 +906,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null @@ -936,7 +936,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: v2.9 disable: '' encoding: null @@ -944,7 +944,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null @@ -974,7 +974,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: v2.9.1 disable: '' encoding: null @@ -982,7 +982,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null @@ -1012,7 +1012,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: v2.9.2 disable: '' encoding: null @@ -1020,7 +1020,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null @@ -1050,7 +1050,7 @@ Builds: binary: null build: '' buildjni: - - 'yes' + - yes commit: v3.0 disable: '' encoding: null @@ -1058,7 +1058,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null diff --git a/tests/metadata/dump/org.smssecure.smssecure.yaml b/tests/metadata/dump/org.smssecure.smssecure.yaml index a88a6a6a..af864923 100644 --- a/tests/metadata/dump/org.smssecure.smssecure.yaml +++ b/tests/metadata/dump/org.smssecure.smssecure.yaml @@ -22,7 +22,7 @@ Builds: forcevercode: true forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null @@ -83,7 +83,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null @@ -126,7 +126,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null @@ -167,7 +167,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null @@ -208,7 +208,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null @@ -248,7 +248,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null @@ -288,7 +288,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null @@ -328,7 +328,7 @@ Builds: forcevercode: false forceversion: false gradle: - - 'yes' + - yes gradleprops: [] init: '' maven: null diff --git a/tests/metadata/dump/org.videolan.vlc.yaml b/tests/metadata/dump/org.videolan.vlc.yaml index 290dbabe..b1679ab9 100644 --- a/tests/metadata/dump/org.videolan.vlc.yaml +++ b/tests/metadata/dump/org.videolan.vlc.yaml @@ -2640,7 +2640,7 @@ UpdateCheckIgnore: null UpdateCheckMode: Tags UpdateCheckName: null VercodeOperation: - - '%c + 5' +- '%c + 5' WebSite: http://www.videolan.org/vlc/download-android.html added: null id: org.videolan.vlc From f871df502d36954ca43675a9e4b9e714bf4959f2 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 21 Apr 2023 10:52:20 +0200 Subject: [PATCH 10/10] metadata: minor optimization: call fieldtype() once per loop I profiled this with timeit and a dict with 1000000000 items, and this is the time difference: with_equals: 0.8466835720173549 with_is: 0.8536969239939936 with_old: 1.4458542719949037 I also compared using `==` and `is`, and `==` was slightly faster. --- fdroidserver/metadata.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index a97da7ae..896c224e 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -800,15 +800,16 @@ def post_parse_yaml_metadata(yamldata): """ for k, v in yamldata.items(): - if fieldtype(k) == TYPE_LIST: + _fieldtype = fieldtype(k) + if _fieldtype == TYPE_LIST: if isinstance(v, str): yamldata[k] = [v] elif v: yamldata[k] = [str(i) for i in v] - elif fieldtype(k) == TYPE_INT: + elif _fieldtype == TYPE_INT: if v: yamldata[k] = int(v) - elif fieldtype(k) == TYPE_STRING: + elif _fieldtype == TYPE_STRING: if v or v == 0: yamldata[k] = _normalize_type_string(v) else: @@ -822,10 +823,10 @@ def post_parse_yaml_metadata(yamldata): continue _flagtype = flagtype(k) - if _flagtype is TYPE_STRING: + if _flagtype == TYPE_STRING: if v or v == 0: build[k] = _normalize_type_string(v) - elif _flagtype is TYPE_INT: + elif _flagtype == TYPE_INT: build[k] = v # versionCode must be int if not isinstance(v, int): @@ -865,16 +866,16 @@ def write_yaml(mf, app): """ def _field_to_yaml(typ, value): """Convert data to YAML 1.2 format that keeps the right TYPE_*.""" - if typ is TYPE_STRING: + if typ == TYPE_STRING: return str(value) - elif typ is TYPE_INT: + elif typ == TYPE_INT: return int(value) - elif typ is TYPE_MULTILINE: + elif typ == TYPE_MULTILINE: if '\n' in value: return ruamel.yaml.scalarstring.preserve_literal(str(value)) else: return str(value) - elif typ is TYPE_SCRIPT: + elif typ == TYPE_SCRIPT: if type(value) == list: if len(value) == 1: return value[0]