From 337974cbedf673a528cc6df8b17e0550906b3115 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gregor=20D=C3=BCster?= Date: Thu, 25 May 2023 19:05:57 +0200 Subject: [PATCH 1/3] metadata: Make ArchivePolicy an interger internally --- .gitlab-ci.yml | 1 + fdroidserver/lint.py | 20 +++++++++++++------ fdroidserver/metadata.py | 14 ++++++++----- fdroidserver/update.py | 4 ++-- .../app.with.special.build.params.yml | 2 +- .../org.fdroid.fdroid.yml | 2 +- tests/metadata.TestCase | 2 +- .../app.with.special.build.params.yml | 2 +- .../dump/app.with.special.build.params.yaml | 2 +- tests/metadata/dump/com.politedroid.yaml | 2 +- tests/metadata/dump/org.videolan.vlc.yaml | 2 +- tests/update.TestCase | 2 +- 12 files changed, 34 insertions(+), 21 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6193e6ff..636de406 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -57,6 +57,7 @@ metadata_v0: - cd fdroiddata - ../tests/dump_internal_metadata_format.py - sed -i + -e '/ArchivePolicy:/d' -e '/RequiresRoot:/d' metadata/dump_*/*.yaml - diff -uw metadata/dump_* diff --git a/fdroidserver/lint.py b/fdroidserver/lint.py index ba21295c..0fc4aa5e 100644 --- a/fdroidserver/lint.py +++ b/fdroidserver/lint.py @@ -635,6 +635,17 @@ def check_app_field_types(app): fieldtype=v.__class__.__name__, ) ) + elif t == metadata.TYPE_INT and not isinstance(v, int): + yield ( + _( + "{appid}: {field} must be a '{type}', but it is a '{fieldtype}'!" + ).format( + appid=app.id, + field=field, + type='int', + fieldtype=v.__class__.__name__, + ) + ) def check_antiFeatures(app): @@ -693,8 +704,7 @@ def check_for_unsupported_metadata_files(basedir=""): def check_current_version_code(app): """Check that the CurrentVersionCode is currently available.""" - archive_policy = app.get('ArchivePolicy') - if archive_policy and archive_policy.split()[0] == "0": + if app.get('ArchivePolicy') == 0: return cv = app.get('CurrentVersionCode') if cv is not None and cv == 0: @@ -724,13 +734,11 @@ def check_current_version_code(app): def check_updates_expected(app): """Check if update checking makes sense.""" - if ( - app.get('NoSourceSince') or app.get('ArchivePolicy') == '0 versions' - ) and not all( + if (app.get('NoSourceSince') or app.get('ArchivePolicy') == 0) and not all( app.get(key, 'None') == 'None' for key in ('AutoUpdateMode', 'UpdateCheckMode') ): yield _( - 'App has NoSourceSince or ArchivePolicy "0 versions" but AutoUpdateMode or UpdateCheckMode are not None' + 'App has NoSourceSince or ArchivePolicy "0 versions" or 0 but AutoUpdateMode or UpdateCheckMode are not None' ) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index d41b8fd9..132d2f8c 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -195,6 +195,7 @@ fieldtypes = { 'Builds': TYPE_BUILD, 'VercodeOperation': TYPE_LIST, 'CurrentVersionCode': TYPE_INT, + 'ArchivePolicy': TYPE_INT, } @@ -447,10 +448,6 @@ valuetypes = { r'^[a-fA-F0-9]{64}$', ["AllowedAPKSigningKeys"]), - FieldValidator("Archive Policy", - r'^[0-9]+ versions$', - ["ArchivePolicy"]), - FieldValidator("Auto Update Mode", r"^(Version.*|None)$", ["AutoUpdateMode"]), @@ -1017,6 +1014,9 @@ def post_parse_yaml_metadata(yamldata): if v or v == 0: yamldata[k] = _normalize_type_list(k, v) elif _fieldtype == TYPE_INT: + # ArchivePolicy used to require " versions" in the value. + if k == 'ArchivePolicy' and isinstance(v, str): + v = v.split(' ', maxsplit=1)[0] v = _normalize_type_int(k, v) if v or v == 0: yamldata[k] = v @@ -1210,7 +1210,7 @@ def _app_to_yaml(app): insert_newline = True else: value = app.get(field) - if value or field == 'Builds': + if value or field in ('Builds', 'ArchivePolicy'): _fieldtype = fieldtype(field) if field == 'Builds': if app.get('Builds'): @@ -1226,6 +1226,10 @@ def _app_to_yaml(app): if len(value) == 1: cm[field] = value[0] else: + elif field == 'ArchivePolicy': + if value is None: + continue + cm[field] = _field_to_yaml(fieldtype(field), value) cm[field] = value elif _fieldtype == TYPE_MULTILINE: v = _format_multiline(value) diff --git a/fdroidserver/update.py b/fdroidserver/update.py index 398932b4..1fb020ea 100644 --- a/fdroidserver/update.py +++ b/fdroidserver/update.py @@ -176,7 +176,7 @@ def status_update_json(apps, apks): validapks = 0 if app.get('Disabled'): output['disabled'].append(appid) - elif app.get("ArchivePolicy") and int(app["ArchivePolicy"][:-9]) == 0: + elif app["ArchivePolicy"] == 0: output['archivePolicy0'].append(appid) else: for build in app.get('Builds', []): @@ -1877,7 +1877,7 @@ def archive_old_apks(apps, apks, archapks, repodir, archivedir, defaultkeepversi for appid, app in apps.items(): if app.get('ArchivePolicy'): - keepversions = int(app['ArchivePolicy'][:-9]) + keepversions = app['ArchivePolicy'] else: keepversions = defaultkeepversions if app.get('VercodeOperation'): diff --git a/tests/metadata-rewrite-yml/app.with.special.build.params.yml b/tests/metadata-rewrite-yml/app.with.special.build.params.yml index eeb174b7..1a286a13 100644 --- a/tests/metadata-rewrite-yml/app.with.special.build.params.yml +++ b/tests/metadata-rewrite-yml/app.with.special.build.params.yml @@ -95,7 +95,7 @@ Builds: versionCode: 51 disable: Labelled as pre-release, so skipped -ArchivePolicy: 0 versions +ArchivePolicy: 0 AutoUpdateMode: None UpdateCheckMode: None CurrentVersion: 2.1.2 diff --git a/tests/metadata-rewrite-yml/org.fdroid.fdroid.yml b/tests/metadata-rewrite-yml/org.fdroid.fdroid.yml index 9471f9a6..4edb97b7 100644 --- a/tests/metadata-rewrite-yml/org.fdroid.fdroid.yml +++ b/tests/metadata-rewrite-yml/org.fdroid.fdroid.yml @@ -944,7 +944,7 @@ Builds: gradle: - yes -ArchivePolicy: 12 versions +ArchivePolicy: 12 AutoUpdateMode: None UpdateCheckMode: Static CurrentVersion: 0.102.3 diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 064c9ce8..3b1bde45 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -1866,7 +1866,7 @@ class MetadataTest(unittest.TestCase): - NonFreeAssets - UpstreamNonFree - ArchivePolicy: 4 versions + ArchivePolicy: 4 AutoUpdateMode: Version v%v UpdateCheckMode: Tags CurrentVersion: '1.5' diff --git a/tests/metadata/app.with.special.build.params.yml b/tests/metadata/app.with.special.build.params.yml index b2daee52..e07efc2d 100644 --- a/tests/metadata/app.with.special.build.params.yml +++ b/tests/metadata/app.with.special.build.params.yml @@ -94,7 +94,7 @@ Builds: versionCode: 51 disable: Labelled as pre-release, so skipped -ArchivePolicy: 0 versions +ArchivePolicy: 0 AutoUpdateMode: None UpdateCheckMode: None CurrentVersion: 2.1.2 diff --git a/tests/metadata/dump/app.with.special.build.params.yaml b/tests/metadata/dump/app.with.special.build.params.yaml index f53ce361..9698b639 100644 --- a/tests/metadata/dump/app.with.special.build.params.yaml +++ b/tests/metadata/dump/app.with.special.build.params.yaml @@ -1,7 +1,7 @@ AllowedAPKSigningKeys: [] AntiFeatures: UpstreamNonFree: {} -ArchivePolicy: 0 versions +ArchivePolicy: 0 AuthorEmail: null AuthorName: null AuthorWebSite: null diff --git a/tests/metadata/dump/com.politedroid.yaml b/tests/metadata/dump/com.politedroid.yaml index e4f24356..7a970436 100644 --- a/tests/metadata/dump/com.politedroid.yaml +++ b/tests/metadata/dump/com.politedroid.yaml @@ -3,7 +3,7 @@ AntiFeatures: NoSourceSince: en-US: '1.5' NonFreeNet: {} -ArchivePolicy: 4 versions +ArchivePolicy: 4 AuthorEmail: null AuthorName: null AuthorWebSite: null diff --git a/tests/metadata/dump/org.videolan.vlc.yaml b/tests/metadata/dump/org.videolan.vlc.yaml index f91dfb65..7bcb7dc4 100644 --- a/tests/metadata/dump/org.videolan.vlc.yaml +++ b/tests/metadata/dump/org.videolan.vlc.yaml @@ -1,6 +1,6 @@ AllowedAPKSigningKeys: [] AntiFeatures: {} -ArchivePolicy: 9 versions +ArchivePolicy: 9 AuthorEmail: null AuthorName: null AuthorWebSite: null diff --git a/tests/update.TestCase b/tests/update.TestCase index 7c4e4518..56ad81a7 100755 --- a/tests/update.TestCase +++ b/tests/update.TestCase @@ -1411,7 +1411,7 @@ class UpdateTest(unittest.TestCase): self.assertDictEqual( metadata_content, { - 'ArchivePolicy': '', + 'ArchivePolicy': None, 'AuthorEmail': '', 'AuthorName': '', 'AuthorWebSite': '', From 9ef2088aced51774c7b2675d4c8ab656ace73f14 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 25 May 2023 19:23:01 +0200 Subject: [PATCH 2/3] add unit tests --- tests/metadata.TestCase | 48 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 3b1bde45..8c606b2c 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -422,6 +422,25 @@ class MetadataTest(unittest.TestCase): ] = 'c03dac71394d6c26766f1b04d3e31cfcac5d03b55d8aa40cc9b9fa6b74354c66' metadata.post_parse_yaml_metadata(yamldata) + def test_post_parse_yaml_metadata_ArchivePolicy_int(self): + for i in range(20): + yamldata = {'ArchivePolicy': i} + metadata.post_parse_yaml_metadata(yamldata) + self.assertEqual(i, yamldata['ArchivePolicy']) + + def test_post_parse_yaml_metadata_ArchivePolicy_string(self): + for i in range(20): + yamldata = {'ArchivePolicy': '%d' % i} + metadata.post_parse_yaml_metadata(yamldata) + self.assertEqual(i, yamldata['ArchivePolicy']) + + def test_post_parse_yaml_metadata_ArchivePolicy_versions(self): + """Test that the old format still works.""" + for i in range(20): + yamldata = {'ArchivePolicy': '%d versions' % i} + metadata.post_parse_yaml_metadata(yamldata) + self.assertEqual(i, yamldata['ArchivePolicy']) + def test_post_parse_yaml_metadata_fails(self): yamldata = {'AllowedAPKSigningKeys': {'bad': 'dict-placement'}} with self.assertRaises(MetaDataException): @@ -2163,6 +2182,11 @@ class PostMetadataParseTest(unittest.TestCase): def setUp(self): fdroidserver.metadata.warnings_action = 'error' + def _post_metadata_parse_app_int(self, from_yaml, expected): + app = {'ArchivePolicy': from_yaml} + metadata.post_parse_yaml_metadata(app) + return {'ArchivePolicy': expected}, app + def _post_metadata_parse_app_list(self, from_yaml, expected): app = {'AllowedAPKSigningKeys': from_yaml} metadata.post_parse_yaml_metadata(app) @@ -2243,6 +2267,7 @@ class PostMetadataParseTest(unittest.TestCase): def test_post_metadata_parse_none(self): """Run None aka YAML null or blank through the various field and flag types.""" + self.assertEqual(*self._post_metadata_parse_app_int(None, None)) self.assertEqual(*self._post_metadata_parse_app_list(None, None)) self.assertEqual(*self._post_metadata_parse_app_string(None, None)) self.assertEqual(*self._post_metadata_parse_build_bool(None, None)) @@ -2253,6 +2278,7 @@ class PostMetadataParseTest(unittest.TestCase): def test_post_metadata_parse_int(self): """Run the int 123456 through the various field and flag types.""" + self.assertEqual(*self._post_metadata_parse_app_int(123456, 123456)) self.assertEqual(*self._post_metadata_parse_app_list(123456, ['123456'])) self.assertEqual(*self._post_metadata_parse_app_string(123456, '123456')) self.assertEqual(*self._post_metadata_parse_build_bool(123456, True)) @@ -2271,6 +2297,7 @@ class PostMetadataParseTest(unittest.TestCase): yaml = ruamel.yaml.YAML(typ='safe', pure=True) str_sha256 = '0000000000000498456908409534729834729834729834792837487293847926' sha256 = yaml.load('a: ' + str_sha256)['a'] + self.assertEqual(*self._post_metadata_parse_app_int(sha256, int(str_sha256))) self.assertEqual(*self._post_metadata_parse_app_list(sha256, [str_sha256])) self.assertEqual(*self._post_metadata_parse_app_string(sha256, str_sha256)) self.assertEqual(*self._post_metadata_parse_build_bool(sha256, True)) @@ -2281,6 +2308,7 @@ class PostMetadataParseTest(unittest.TestCase): def test_post_metadata_parse_int_0(self): """Run the int 0 through the various field and flag types.""" + self.assertEqual(*self._post_metadata_parse_app_int(0, 0)) self.assertEqual(*self._post_metadata_parse_app_list(0, ['0'])) self.assertEqual(*self._post_metadata_parse_app_string(0, '0')) self.assertEqual(*self._post_metadata_parse_build_bool(0, False)) @@ -2291,6 +2319,8 @@ class PostMetadataParseTest(unittest.TestCase): def test_post_metadata_parse_float_0_0(self): """Run the float 0.0 through the various field and flag types.""" + with self.assertRaises(MetaDataException): + self._post_metadata_parse_app_int(0.0, MetaDataException) self.assertEqual(*self._post_metadata_parse_app_list(0.0, ['0.0'])) self.assertEqual(*self._post_metadata_parse_app_string(0.0, '0.0')) self.assertEqual(*self._post_metadata_parse_build_bool(0.0, False)) @@ -2302,6 +2332,8 @@ class PostMetadataParseTest(unittest.TestCase): def test_post_metadata_parse_float_0_1(self): """Run the float 0.1 through the various field and flag types.""" + with self.assertRaises(MetaDataException): + self._post_metadata_parse_app_int(0.1, MetaDataException) self.assertEqual(*self._post_metadata_parse_app_list(0.1, ['0.1'])) self.assertEqual(*self._post_metadata_parse_app_string(0.1, '0.1')) self.assertEqual(*self._post_metadata_parse_build_bool(0.1, True)) @@ -2313,6 +2345,8 @@ class PostMetadataParseTest(unittest.TestCase): def test_post_metadata_parse_float_1_0(self): """Run the float 1.0 through the various field and flag types.""" + with self.assertRaises(MetaDataException): + self._post_metadata_parse_app_int(1.0, MetaDataException) self.assertEqual(*self._post_metadata_parse_app_list(1.0, ['1.0'])) self.assertEqual(*self._post_metadata_parse_app_string(1.0, '1.0')) self.assertEqual(*self._post_metadata_parse_build_bool(1.0, True)) @@ -2323,6 +2357,8 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_build_string(1.0, '1.0')) def test_post_metadata_parse_empty_list(self): + with self.assertRaises(MetaDataException): + self._post_metadata_parse_app_int(list(), MetaDataException) self.assertEqual(*self._post_metadata_parse_app_list(list(), list())) self.assertEqual(*self._post_metadata_parse_app_string(list(), list())) self.assertEqual(*self._post_metadata_parse_build_bool(list(), False)) @@ -2333,6 +2369,8 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_build_string(list(), list())) def test_post_metadata_parse_set_of_1(self): + with self.assertRaises(MetaDataException): + self._post_metadata_parse_app_int({1}, MetaDataException) self.assertEqual(*self._post_metadata_parse_app_list({1}, ['1'])) self.assertEqual(*self._post_metadata_parse_app_string({1}, '{1}')) self.assertEqual(*self._post_metadata_parse_build_bool({1}, True)) @@ -2343,6 +2381,8 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_build_string({1}, '{1}')) def test_post_metadata_parse_empty_dict(self): + with self.assertRaises(MetaDataException): + self._post_metadata_parse_app_int(dict(), MetaDataException) self.assertEqual(*self._post_metadata_parse_app_list(dict(), dict())) self.assertEqual(*self._post_metadata_parse_app_string(dict(), dict())) self.assertEqual(*self._post_metadata_parse_build_bool(dict(), False)) @@ -2353,6 +2393,8 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_build_string(dict(), dict())) def test_post_metadata_parse_list_int_string(self): + with self.assertRaises(MetaDataException): + self._post_metadata_parse_app_int([1, 'a'], MetaDataException) self.assertEqual(*self._post_metadata_parse_app_list([1, 'a'], ['1', 'a'])) self.assertEqual(*self._post_metadata_parse_app_string([1, 'a'], "[1, 'a']")) self.assertEqual(*self._post_metadata_parse_build_bool([1, 'a'], True)) @@ -2363,6 +2405,8 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_build_string([1, 'a'], "[1, 'a']")) def test_post_metadata_parse_dict_int_string(self): + with self.assertRaises(MetaDataException): + self._post_metadata_parse_app_int({'k': 1}, MetaDataException) with self.assertRaises(MetaDataException): self._post_metadata_parse_app_list({'k': 1}, MetaDataException) self.assertEqual(*self._post_metadata_parse_app_string({'k': 1}, "{'k': 1}")) @@ -2376,6 +2420,8 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_build_string({'k': 1}, "{'k': 1}")) def test_post_metadata_parse_false(self): + with self.assertRaises(MetaDataException): + self._post_metadata_parse_app_int(False, MetaDataException) self.assertEqual(*self._post_metadata_parse_app_list(False, ['false'])) self.assertEqual(*self._post_metadata_parse_app_string(False, 'false')) self.assertEqual(*self._post_metadata_parse_build_bool(False, False)) @@ -2386,6 +2432,8 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_build_string(False, 'false')) def test_post_metadata_parse_true(self): + with self.assertRaises(MetaDataException): + self._post_metadata_parse_app_int(True, MetaDataException) self.assertEqual(*self._post_metadata_parse_app_list(True, ['true'])) self.assertEqual(*self._post_metadata_parse_app_string(True, 'true')) self.assertEqual(*self._post_metadata_parse_build_bool(True, True)) From 31791b44f3dec27df8813a38533ab169735200cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gregor=20D=C3=BCster?= Date: Tue, 30 May 2023 23:05:59 +0200 Subject: [PATCH 3/3] fixup! metadata: Make ArchivePolicy an interger internally --- fdroidserver/metadata.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 132d2f8c..ca46bd17 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -1226,11 +1226,11 @@ def _app_to_yaml(app): if len(value) == 1: cm[field] = value[0] else: + cm[field] = value elif field == 'ArchivePolicy': if value is None: continue - cm[field] = _field_to_yaml(fieldtype(field), value) - cm[field] = value + cm[field] = value elif _fieldtype == TYPE_MULTILINE: v = _format_multiline(value) if v: