From 2efc9437ab6dac668607af9d43f84208c1e27cb6 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 25 May 2023 16:52:14 +0200 Subject: [PATCH 1/9] gitlab-ci: purge stale removals from metadata_v0 job. --- .gitlab-ci.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 7a9dd403..6193e6ff 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -58,9 +58,6 @@ metadata_v0: - ../tests/dump_internal_metadata_format.py - sed -i -e '/RequiresRoot:/d' - -e "/buildozer/d" - -e '/^comments\W /d' - -e 's,maven\(\W\) false,maven\1 null,' metadata/dump_*/*.yaml - diff -uw metadata/dump_* From d3521d7374d92b804fbdc06bd495e259412738eb Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 24 May 2023 14:57:32 +0200 Subject: [PATCH 2/9] metadata: case-insensitive sort for AntiFeatures Categories --- fdroidserver/metadata.py | 4 +++- tests/metadata.TestCase | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index a4203b08..484b9b51 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -1127,7 +1127,7 @@ def _format_stringmap(appid, field, stringmap, versionCode=None): make_list = False break if make_list: - return outlist + return sorted(outlist, key=str.lower) return stringmap @@ -1208,6 +1208,8 @@ def _app_to_yaml(app): if field == 'Builds': if app.get('Builds'): cm.update({field: _builds_to_yaml(app)}) + elif field == 'Categories': + cm[field] = sorted(value, key=str.lower) elif field == 'CurrentVersionCode': cm[field] = _field_to_yaml(TYPE_INT, value) elif field == 'AntiFeatures': diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 777002bc..631f0d9f 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -2019,6 +2019,20 @@ class MetadataTest(unittest.TestCase): appid, field, {afname: {'uz': 'a', locale: 'b', 'zh': 'c'}}, versionCode ) + def test_app_to_yaml_one_category(self): + """Categories does not get simplified to string when outputting YAML.""" + self.assertEqual( + metadata._app_to_yaml({'Categories': ['one']}), + {'Categories': ['one']}, + ) + + def test_app_to_yaml_categories(self): + """Sort case-insensitive before outputting YAML.""" + self.assertEqual( + metadata._app_to_yaml({'Categories': ['c', 'a', 'B']}), + {'Categories': ['a', 'B', 'c']}, + ) + class PostMetadataParseTest(unittest.TestCase): """Test the functions that post process the YAML input. From 26b2cffdcc5d6bd768755fc0f93eea804d2f8b1f Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 24 May 2023 16:44:34 +0200 Subject: [PATCH 3/9] metadata: tests for converting Builds: entries for writing out * The metadata.Builds() class initializes all possible flags, then the flags with init values are filtered out when writing out YAML. * TYPE_SCRIPT flags with one entry will be converted to a string. --- tests/metadata.TestCase | 61 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 631f0d9f..fd15c5e7 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -1894,6 +1894,10 @@ class MetadataTest(unittest.TestCase): app = metadata.App({'Builds': [metadata.Build({'rm': ['one']})]}) self.assertEqual({'rm': ['one']}, metadata._app_to_yaml(app)['Builds'][0]) + def test_app_to_yaml_build_list_two(self): + app = metadata.App({'Builds': [metadata.Build({'rm': ['1', '2']})]}) + self.assertEqual({'rm': ['1', '2']}, metadata._app_to_yaml(app)['Builds'][0]) + def test_app_to_yaml_build_list(self): app = metadata.App({'Builds': [metadata.Build({'rm': ['b2', 'NO1']})]}) self.assertEqual({'rm': ['b2', 'NO1']}, metadata._app_to_yaml(app)['Builds'][0]) @@ -2033,6 +2037,63 @@ class MetadataTest(unittest.TestCase): {'Categories': ['a', 'B', 'c']}, ) + def test_builds_to_yaml_gradle_yes(self): + app = {'Builds': [{'versionCode': 0, 'gradle': ['yes']}]} + self.assertEqual( + metadata._builds_to_yaml(app), [{'versionCode': 0, 'gradle': ['yes']}] + ) + + def test_builds_to_yaml_gradle_off(self): + app = {'Builds': [{'versionCode': 0, 'gradle': ['off']}]} + self.assertEqual( + metadata._builds_to_yaml(app), [{'versionCode': 0, 'gradle': ['off']}] + ) + + def test_builds_to_yaml_gradle_true(self): + app = {'Builds': [{'versionCode': 0, 'gradle': ['true']}]} + self.assertEqual( + metadata._builds_to_yaml(app), [{'versionCode': 0, 'gradle': ['true']}] + ) + + def test_builds_to_yaml_gradle_false(self): + app = {'Builds': [{'versionCode': 0, 'gradle': ['false']}]} + self.assertEqual( + metadata._builds_to_yaml(app), [{'versionCode': 0, 'gradle': ['false']}] + ) + + def test_builds_to_yaml(self): + """Include one of each flag type with a valid value.""" + app = { + 'Builds': [ + metadata.Build( + { + 'versionCode': 0, + 'gradle': ['free'], + 'rm': ['0', '2'], + 'submodules': True, + 'timeout': 0, + 'init': ['false', 'two'], + } + ) + ] + } + # check that metadata.Build() inited flag values + self.assertEqual(app['Builds'][0]['scanignore'], list()) + # then unchanged values should be removed by _builds_to_yaml + self.assertEqual( + metadata._builds_to_yaml(app), + [ + { + 'versionCode': 0, + 'gradle': ['free'], + 'rm': ['0', '2'], + 'submodules': True, + 'timeout': 0, + 'init': ['false', 'two'], + } + ], + ) + class PostMetadataParseTest(unittest.TestCase): """Test the functions that post process the YAML input. From e64f121c0c4011bf98ecd05e4209b0bf0be08120 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 24 May 2023 21:44:36 +0200 Subject: [PATCH 4/9] metadata: type conversion happens at parsing, not at writing These test cases were writing assuming they had to transform the data format. That is no longer the case. Going forward, the parsing process converts everything to a standardized format. That will hopefully be enforceable by the JSON Schema in the future. --- tests/metadata.TestCase | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index fd15c5e7..343d2b1c 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -720,7 +720,7 @@ class MetadataTest(unittest.TestCase): build.versionCode = '0' # taken from fdroidserver/import.py build.disable = 'Generated by import.py ...' build.commit = 'Unknown' - build.gradle = [True] + build.gradle = ['yes'] app['Builds'] = [build] fdroidserver.metadata.write_yaml(mf, app) @@ -745,7 +745,7 @@ class MetadataTest(unittest.TestCase): disable: Generated by import.py ... commit: Unknown gradle: - - true + - yes AutoUpdateMode: None UpdateCheckMode: Tags @@ -1886,10 +1886,6 @@ class MetadataTest(unittest.TestCase): app = metadata.App({'Builds': [metadata.Build({'rm': []})]}) self.assertEqual(dict(), metadata._app_to_yaml(app)['Builds'][0]) - def test_app_to_yaml_build_list_string(self): - app = metadata.App({'Builds': [metadata.Build({'rm': 'one'})]}) - self.assertEqual({'rm': 'one'}, metadata._app_to_yaml(app)['Builds'][0]) - def test_app_to_yaml_build_list_one(self): app = metadata.App({'Builds': [metadata.Build({'rm': ['one']})]}) self.assertEqual({'rm': ['one']}, metadata._app_to_yaml(app)['Builds'][0]) From 070dae14316dac858069b1299b609317bb3f1be6 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 24 May 2023 16:18:24 +0200 Subject: [PATCH 5/9] versionCode is an int everywhere since !1176 fixed #332 --- tests/metadata.TestCase | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 343d2b1c..52f1bf6e 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -717,7 +717,7 @@ class MetadataTest(unittest.TestCase): app.UpdateCheckMode = 'Tags' build = fdroidserver.metadata.Build() build.versionName = 'Unknown' # taken from fdroidserver/import.py - build.versionCode = '0' # taken from fdroidserver/import.py + build.versionCode = 0 # taken from fdroidserver/import.py build.disable = 'Generated by import.py ...' build.commit = 'Unknown' build.gradle = ['yes'] From 1bc9b41a2be80502ced56dac0443e5c965ec59a4 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 24 May 2023 16:46:41 +0200 Subject: [PATCH 6/9] metadata: YAML 1.2 handles `gradle: off` now, "off" isn't a boolean Before switching to YAML 1.2, there needed to be special handling of values that YAML parsed as booleans. --- fdroidserver/metadata.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 484b9b51..9688ea6c 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -1172,10 +1172,6 @@ def _builds_to_yaml(app): for field in build_flags: if hasattr(build, field): value = getattr(build, field) - if field == 'gradle' and value == ['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 and typ == TYPE_STRINGMAP: From b055559df70be140162ac299208111e31c2d4889 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 24 May 2023 15:28:25 +0200 Subject: [PATCH 7/9] metadata: remove STRING/INT conversion on output The type conversion should all happen in post_parse_yaml_metadata whenever possible. Also, when `if` blocks end in `return`, it is clearer if no `elif` or `else` is used. --- fdroidserver/metadata.py | 55 ++++++++++++++++++++-------------------- tests/metadata.TestCase | 29 +++++++++++++++++++++ 2 files changed, 57 insertions(+), 27 deletions(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 9688ea6c..29ac8b96 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -1070,6 +1070,20 @@ def post_parse_yaml_metadata(yamldata): } +def _format_multiline(value): + """TYPE_MULTILINE with newlines in them are saved as YAML literal strings.""" + if '\n' in value: + return ruamel.yaml.scalarstring.preserve_literal(str(value)) + return str(value) + + +def _format_script(value): + """TYPE_SCRIPT with one value are converted to YAML string values.""" + if len(value) == 1: + return value[0] + return value + + def _format_stringmap(appid, field, stringmap, versionCode=None): """Format TYPE_STRINGMAP taking into account localized files in the metadata dir. @@ -1142,27 +1156,6 @@ def _del_duplicated_NoSourceSince(app): del app['AntiFeatures'][key] -def _field_to_yaml(typ, value): - """Convert data to YAML 1.2 format that keeps the right TYPE_*.""" - if typ == TYPE_STRING: - return str(value) - elif typ == TYPE_INT: - return int(value) - elif typ == TYPE_MULTILINE: - if '\n' in value: - return ruamel.yaml.scalarstring.preserve_literal(str(value)) - else: - return str(value) - elif typ == TYPE_SCRIPT: - if type(value) == list: - if len(value) == 1: - return value[0] - else: - return value - else: - return value - - def _builds_to_yaml(app): builds = ruamel.yaml.comments.CommentedSeq() for build in app.get('Builds', []): @@ -1179,7 +1172,7 @@ def _builds_to_yaml(app): if v: b[field] = v elif value is not None and (typ == TYPE_INT or value): - b.update({field: _field_to_yaml(typ, value)}) + b[field] = value builds.append(b) @@ -1201,13 +1194,12 @@ def _app_to_yaml(app): else: value = app.get(field) if value or field == 'Builds': + _fieldtype = fieldtype(field) if field == 'Builds': if app.get('Builds'): cm.update({field: _builds_to_yaml(app)}) elif field == 'Categories': cm[field] = sorted(value, key=str.lower) - elif field == 'CurrentVersionCode': - cm[field] = _field_to_yaml(TYPE_INT, value) elif field == 'AntiFeatures': v = _format_stringmap(app['id'], field, value) if v: @@ -1215,11 +1207,20 @@ def _app_to_yaml(app): elif field == 'AllowedAPKSigningKeys': value = [str(i).lower() for i in value] if len(value) == 1: - cm[field] = _field_to_yaml(TYPE_STRING, value[0]) + cm[field] = value[0] else: - cm[field] = _field_to_yaml(TYPE_LIST, value) + cm[field] = value + elif _fieldtype == TYPE_MULTILINE: + v = _format_multiline(value) + if v: + cm[field] = v + elif _fieldtype == TYPE_SCRIPT: + v = _format_script(value) + if v: + cm[field] = v else: - cm[field] = _field_to_yaml(fieldtype(field), value) + if value: + cm[field] = value if insert_newline: # we need to prepend a newline in front of this field diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 52f1bf6e..f430fede 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -1920,6 +1920,35 @@ class MetadataTest(unittest.TestCase): cm = metadata._app_to_yaml(metadata.App({'CurrentVersionCode': 0})) self.assertFalse('CurrentVersionCode' in cm) + def test_format_multiline(self): + self.assertEqual(metadata._format_multiline('description'), 'description') + + def test_format_multiline_empty(self): + self.assertEqual(metadata._format_multiline(''), '') + + def test_format_multiline_newline_char(self): + self.assertEqual(metadata._format_multiline('one\\ntwo'), 'one\\ntwo') + + def test_format_multiline_newlines(self): + self.assertEqual( + metadata._format_multiline( + textwrap.dedent( + """ + one + two + three + """ + ) + ), + '\none\ntwo\nthree\n', + ) + + def test_format_script_newline(self): + self.assertEqual(metadata._format_script(['one\ntwo']), 'one\ntwo') + + def test_format_script_newline_char(self): + self.assertEqual(metadata._format_script(['one\\ntwo']), 'one\\ntwo') + def test_format_stringmap_empty(self): self.assertEqual( metadata._format_stringmap('🔥', 'test', dict()), From 689786eea43bfdea46cc29b1acf63fb93e44f323 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 24 May 2023 18:43:48 +0200 Subject: [PATCH 8/9] metadata: refactor _builds_to_yaml to use dicts and _format functions _builds_to_yaml does not use any features of the metadata.Build class, so it can operate on plain dicts as well. It also does not need to output Build instances because those are converted to plain dicts when writing out to YAML. --- fdroidserver/metadata.py | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 29ac8b96..3692e7cb 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -1157,22 +1157,31 @@ def _del_duplicated_NoSourceSince(app): def _builds_to_yaml(app): + """Reformat Builds: flags for output to YAML 1.2. + + This will strip any flag/value that is not set or is empty. + TYPE_BOOL fields are removed when they are false. 0 is valid + value, it should not be stripped, so there are special cases to + handle that. + + """ builds = ruamel.yaml.comments.CommentedSeq() for build in app.get('Builds', []): - if not isinstance(build, Build): - build = Build(build) b = ruamel.yaml.comments.CommentedMap() for field in build_flags: - if hasattr(build, field): - value = getattr(build, field) - typ = flagtype(field) - # don't check value == True for TYPE_INT as it could be 0 - if value and typ == TYPE_STRINGMAP: - v = _format_stringmap(app['id'], field, value, build['versionCode']) - if v: - b[field] = v - elif value is not None and (typ == TYPE_INT or value): - b[field] = value + v = build.get(field) + if v is None or v is False or v == '' or v == dict() or v == list(): + continue + _flagtype = flagtype(field) + if _flagtype == TYPE_MULTILINE: + v = _format_multiline(v) + elif _flagtype == TYPE_SCRIPT: + v = _format_script(v) + elif _flagtype == TYPE_STRINGMAP: + v = _format_stringmap(app['id'], field, v, build['versionCode']) + + if v or v == 0: + b[field] = v builds.append(b) From fac7ceffe387279a1e19965afcd282939dc3a464 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 24 May 2023 21:54:29 +0200 Subject: [PATCH 9/9] metadata: remove non-values from Builds: entries --- fdroidserver/metadata.py | 8 ++++++++ tests/metadata.TestCase | 30 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 3692e7cb..d41b8fd9 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -1077,8 +1077,14 @@ def _format_multiline(value): return str(value) +def _format_list(value): + """TYPE_LIST should not contain null values.""" + return [v for v in value if v] + + def _format_script(value): """TYPE_SCRIPT with one value are converted to YAML string values.""" + value = [v for v in value if v] if len(value) == 1: return value[0] return value @@ -1175,6 +1181,8 @@ def _builds_to_yaml(app): _flagtype = flagtype(field) if _flagtype == TYPE_MULTILINE: v = _format_multiline(v) + elif _flagtype == TYPE_LIST: + v = _format_list(v) elif _flagtype == TYPE_SCRIPT: v = _format_script(v) elif _flagtype == TYPE_STRINGMAP: diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index f430fede..064c9ce8 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -1943,6 +1943,24 @@ class MetadataTest(unittest.TestCase): '\none\ntwo\nthree\n', ) + def test_format_list_empty(self): + self.assertEqual(metadata._format_list(['', None]), list()) + + def test_format_list_one_empty(self): + self.assertEqual(metadata._format_list(['foo', None]), ['foo']) + + def test_format_list_two(self): + self.assertEqual(metadata._format_list(['2', '1']), ['2', '1']) + + def test_format_list_newline(self): + self.assertEqual(metadata._format_list(['one\ntwo']), ['one\ntwo']) + + def test_format_list_newline_char(self): + self.assertEqual(metadata._format_list(['one\\ntwo']), ['one\\ntwo']) + + def test_format_script_empty(self): + self.assertEqual(metadata._format_script(['', None]), list()) + def test_format_script_newline(self): self.assertEqual(metadata._format_script(['one\ntwo']), 'one\ntwo') @@ -2086,6 +2104,18 @@ class MetadataTest(unittest.TestCase): metadata._builds_to_yaml(app), [{'versionCode': 0, 'gradle': ['false']}] ) + def test_builds_to_yaml_stripped(self): + self.assertEqual( + metadata._builds_to_yaml( + { + 'Builds': [ + metadata.Build({'versionCode': 0, 'rm': [None], 'init': ['']}) + ] + } + ), + [{'versionCode': 0}], + ) + def test_builds_to_yaml(self): """Include one of each flag type with a valid value.""" app = {