Merge branch 'archivepolicy-integer' into 'master'

Make "ArchivePolicy" always be an integer

Closes #327

See merge request fdroid/fdroidserver!920

.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_*

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 app.get('ArchivePolicy') == 0:
-    if archive_policy and archive_policy.split()[0] == "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 (
+    if (app.get('NoSourceSince') or app.get('ArchivePolicy') == 0) and not all(
-        app.get('NoSourceSince') or app.get('ArchivePolicy') == '0 versions'
-    ) 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'
         )
 
 

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'):
@@ -1227,6 +1227,10 @@ def _app_to_yaml(app):
                         cm[field] = value[0]
                     else:
                         cm[field] = value
+                elif field == 'ArchivePolicy':
+                    if value is None:
+                        continue
+                    cm[field] = value
                 elif _fieldtype == TYPE_MULTILINE:
                     v = _format_multiline(value)
                     if v:

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'):

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

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

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):
@@ -1866,7 +1885,7 @@ class MetadataTest(unittest.TestCase):
                       - NonFreeAssets
                       - UpstreamNonFree
 
-                ArchivePolicy: 4 versions
+                ArchivePolicy: 4
                 AutoUpdateMode: Version v%v
                 UpdateCheckMode: Tags
                 CurrentVersion: '1.5'
@@ -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))

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

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

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

tests/metadata/dump/org.videolan.vlc.yaml

@@ -1,6 +1,6 @@
 AllowedAPKSigningKeys: []
 AntiFeatures: {}
-ArchivePolicy: 9 versions
+ArchivePolicy: 9
 AuthorEmail: null
 AuthorName: null
 AuthorWebSite: null

tests/update.TestCase

@@ -1411,7 +1411,7 @@ class UpdateTest(unittest.TestCase):
                 self.assertDictEqual(
                     metadata_content,
                     {
-                        'ArchivePolicy': '',
+                        'ArchivePolicy': None,
                         'AuthorEmail': '',
                         'AuthorName': '',
                         'AuthorWebSite': '',