From c0ae09e0dfd128f82836db09e86c2d48667dc28e Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 20 Apr 2023 17:43:56 +0200 Subject: [PATCH] metadata: remove strange app arg construct from parse_yaml_metadata() My guess is that this is some kind of vestige of the old code structure, back when there was .txt and .yml formats. This makes it a normal Python function: input as arg, return value is the result. --- fdroidserver/metadata.py | 37 +++++++++++++++++++++++++------------ tests/metadata.TestCase | 31 +++++++++++++++++++++---------- 2 files changed, 46 insertions(+), 22 deletions(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index b4328c04..efd2422b 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -675,23 +675,31 @@ def post_metadata_parse(app): def parse_metadata(metadatapath): """Parse metadata file, also checking the source repo for .fdroid.yml. + This function finds the relevant files, gets them parsed, converts + dicts into App and Build instances, and combines the results into + a single App instance. + If this is a metadata file from fdroiddata, it will first load the source repo type and URL from fdroiddata, then read .fdroid.yml if it exists, then include the rest of the metadata as specified in fdroiddata, so that fdroiddata has precedence over the metadata in the source code. + .fdroid.yml is embedded in the app's source repo, so it is + "user-generated". That means that it can have weird things in it + that need to be removed so they don't break the overall process, + e.g. if the upstream developer includes some broken field, it can + be overridden in the metadata file. + """ metadatapath = Path(metadatapath) app = App() app.metadatapath = metadatapath.as_posix() - name = metadatapath.stem - if name != '.fdroid': - app.id = name - + if metadatapath.stem != '.fdroid': + app.id = metadatapath.stem if metadatapath.suffix == '.yml': with metadatapath.open('r', encoding='utf-8') as mf: - parse_yaml_metadata(mf, app) + app.update(parse_yaml_metadata(mf)) else: _warn_or_exception(_('Unknown metadata format: {path} (use: *.yml)') .format(path=metadatapath)) @@ -727,15 +735,18 @@ def parse_metadata(metadatapath): return app -def parse_yaml_metadata(mf, app): +def parse_yaml_metadata(mf): """Parse the .yml file and post-process it. + This function handles parsing a metadata YAML file and converting + all the various data types into a consistent internal + representation. The results are meant to update an existing App + instance or used as a plain dict. + Clean metadata .yml files can be used directly, but in order to make a better user experience for people editing .yml files, there - is post processing. .fdroid.yml is embedded in the app's source - repo, so it is "user-generated". That means that it can have - weird things in it that need to be removed so they don't break the - overall process. + is post processing. That makes the parsing perform something like + Strict YAML. """ try: @@ -747,6 +758,9 @@ def parse_yaml_metadata(mf, app): + common.run_yamllint(mf.name, indent=4), cause=e) + if yamldata is None or yamldata == '': + return dict() + deprecated_in_yaml = ['Provides'] if yamldata: @@ -775,8 +789,7 @@ def parse_yaml_metadata(mf, app): _warn_or_exception(msg.format(build_flag=build_flag, path=mf.name)) post_parse_yaml_metadata(yamldata) - app.update(yamldata) - return app + return yamldata def post_parse_yaml_metadata(yamldata): diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index d2a42180..bdb71f3d 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -362,6 +362,21 @@ class MetadataTest(unittest.TestCase): allappids.append(appid) 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()) + + 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()) + + def test_parse_yaml_metadata_empty_string_file(self): + mf = io.StringIO('""') + mf.name = 'mock_filename.yaml' + self.assertEqual(fdroidserver.metadata.parse_yaml_metadata(mf), dict()) + def test_parse_yaml_metadata_unknown_app_field(self): mf = io.StringIO( textwrap.dedent( @@ -375,7 +390,7 @@ 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, {}) + fdroidserver.metadata.parse_yaml_metadata(mf) def test_parse_yaml_metadata_unknown_build_flag(self): mf = io.StringIO( @@ -390,7 +405,7 @@ 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, {}) + fdroidserver.metadata.parse_yaml_metadata(mf) def test_parse_yaml_srclib_corrupt_file(self): with tempfile.TemporaryDirectory() as testdir: @@ -487,9 +502,8 @@ class MetadataTest(unittest.TestCase): ) mf.name = 'mock_filename.yaml' mf.seek(0) - result = {} with mock.patch('fdroidserver.metadata.warnings_action', 'error'): - fdroidserver.metadata.parse_yaml_metadata(mf, result) + result = fdroidserver.metadata.parse_yaml_metadata(mf) self.maxDiff = None self.assertDictEqual( result, @@ -543,9 +557,8 @@ class MetadataTest(unittest.TestCase): ) mf.name = 'mock_filename.yaml' mf.seek(0) - result = {} with mock.patch('fdroidserver.metadata.warnings_action', 'error'): - fdroidserver.metadata.parse_yaml_metadata(mf, result) + result = fdroidserver.metadata.parse_yaml_metadata(mf) self.maxDiff = None self.assertDictEqual( result, @@ -594,9 +607,8 @@ class MetadataTest(unittest.TestCase): ) mf.name = 'mock_filename.yaml' mf.seek(0) - result = {} with mock.patch('fdroidserver.metadata.warnings_action', 'error'): - fdroidserver.metadata.parse_yaml_metadata(mf, result) + result = fdroidserver.metadata.parse_yaml_metadata(mf) self.assertDictEqual( result, { @@ -629,9 +641,8 @@ class MetadataTest(unittest.TestCase): ) mf.name = 'mock_filename.yaml' mf.seek(0) - result = {} with mock.patch('fdroidserver.metadata.warnings_action', 'error'): - fdroidserver.metadata.parse_yaml_metadata(mf, result) + result = fdroidserver.metadata.parse_yaml_metadata(mf) self.assertNotIn('Provides', result) self.assertNotIn('provides', result)