lint: Anti-Features validator uses names from config

fdroidserver/lint.py

@@ -220,6 +220,19 @@ locale_pattern = re.compile(r"[a-z]{2,3}(-([A-Z][a-zA-Z]+|\d+|[a-z]+))*")
 
 versioncode_check_pattern = re.compile(r"(\\d|\[(0-9|\\d)_?(a-fA-F)?])[+]")
 
+ANTIFEATURES_KEYS = None
+ANTIFEATURES_PATTERN = None
+
+
+def load_antiFeatures_config():
+    """Lazy loading, since it might read a lot of files."""
+    global ANTIFEATURES_KEYS, ANTIFEATURES_PATTERN
+    k = 'antiFeatures'  # internal dict uses camelCase key name
+    if not ANTIFEATURES_KEYS or k not in common.config:
+        common.config[k] = common.load_localized_config(k, 'repo')
+        ANTIFEATURES_KEYS = sorted(common.config[k].keys())
+        ANTIFEATURES_PATTERN = ','.join(ANTIFEATURES_KEYS)
+
 
 def check_regexes(app):
     for f, checks in regex_checks.items():
@@ -613,6 +626,26 @@ def check_app_field_types(app):
             )
 
 
+def check_antiFeatures(app):
+    """Check the Anti-Features keys match those declared in the config."""
+    pattern = ANTIFEATURES_PATTERN
+    msg = _("'{value}' is not a valid {field} in {appid}. Regex pattern: {pattern}")
+
+    field = 'AntiFeatures'  # App entries use capitalized CamelCase
+    for value in app.get(field, []):
+        if value not in ANTIFEATURES_KEYS:
+            yield msg.format(value=value, field=field, appid=app.id, pattern=pattern)
+
+    field = 'antifeatures'  # Build entries use all lowercase
+    for build in app.get('Builds', []):
+        build_antiFeatures = build.get(field, [])
+        for value in build_antiFeatures:
+            if value not in ANTIFEATURES_KEYS:
+                yield msg.format(
+                    value=value, field=field, appid=app.id, pattern=pattern
+                )
+
+
 def check_for_unsupported_metadata_files(basedir=""):
     """Check whether any non-metadata files are in metadata/."""
     basedir = Path(basedir)
@@ -745,6 +778,7 @@ def main():
     metadata.warnings_action = options.W
 
     config = common.read_config(options)
+    load_antiFeatures_config()
 
     # Get all apps...
     allapps = metadata.read_metadata(options.appid)
@@ -801,6 +835,7 @@ def main():
 
         app_check_funcs = [
             check_app_field_types,
+            check_antiFeatures,
             check_regexes,
             check_update_check_data_url,
             check_update_check_data_int,

fdroidserver/metadata.py

@@ -448,10 +448,6 @@ valuetypes = {
                    r'^[0-9]+ versions$',
                    ["ArchivePolicy"]),
 
-    FieldValidator("Anti-Feature",
-                   r'^(Ads|Tracking|NonFreeNet|NonFreeDep|NonFreeAdd|UpstreamNonFree|NonFreeAssets|KnownVuln|ApplicationDebuggable|NoSourceSince|NSFW)$',
-                   ["AntiFeatures"]),
-
     FieldValidator("Auto Update Mode",
                    r"^(Version.*|None)$",
                    ["AutoUpdateMode"]),

tests/lint.TestCase

@@ -132,23 +132,9 @@ class LintTest(unittest.TestCase):
         app.Description = 'These are some back'
 
         fields = {
-            'AntiFeatures': {
-                'good': [
-                    [
-                        'KnownVuln',
-                    ],
-                    ['NonFreeNet', 'KnownVuln'],
-                ],
-                'bad': [
-                    'KnownVuln',
-                    'NonFreeNet,KnownVuln',
-                ],
-            },
             'Categories': {
                 'good': [
-                    [
+                    ['Sports & Health'],
-                        'Sports & Health',
-                    ],
                     ['Multimedia', 'Graphics'],
                 ],
                 'bad': [
@@ -328,6 +314,53 @@ class LintTest(unittest.TestCase):
         self.assertFalse(anywarns)
 
 
+class LintAntiFeaturesTest(unittest.TestCase):
+    def setUp(self):
+        self.basedir = localmodule / 'tests'
+        os.chdir(self.basedir)
+        fdroidserver.common.config = dict()
+        fdroidserver.lint.load_antiFeatures_config()
+
+    def test_check_antiFeatures_empty(self):
+        app = fdroidserver.metadata.App()
+        self.assertEqual([], list(fdroidserver.lint.check_antiFeatures(app)))
+
+    def test_check_antiFeatures_empty_AntiFeatures(self):
+        app = fdroidserver.metadata.App()
+        app['AntiFeatures'] = []
+        self.assertEqual([], list(fdroidserver.lint.check_antiFeatures(app)))
+
+    def test_check_antiFeatures(self):
+        app = fdroidserver.metadata.App()
+        app['AntiFeatures'] = ['Ads', 'UpstreamNonFree']
+        self.assertEqual([], list(fdroidserver.lint.check_antiFeatures(app)))
+
+    def test_check_antiFeatures_fails_one(self):
+        app = fdroidserver.metadata.App()
+        app['AntiFeatures'] = ['Ad']
+        self.assertEqual(1, len(list(fdroidserver.lint.check_antiFeatures(app))))
+
+    def test_check_antiFeatures_fails_many(self):
+        app = fdroidserver.metadata.App()
+        app['AntiFeatures'] = ['Adss', 'Tracker', 'NoSourceSince', 'FAKE', 'NonFree']
+        self.assertEqual(4, len(list(fdroidserver.lint.check_antiFeatures(app))))
+
+    def test_check_antiFeatures_build_empty(self):
+        app = fdroidserver.metadata.App()
+        app['Builds'] = [{'antifeatures': []}]
+        self.assertEqual([], list(fdroidserver.lint.check_antiFeatures(app)))
+
+    def test_check_antiFeatures_build(self):
+        app = fdroidserver.metadata.App()
+        app['Builds'] = [{'antifeatures': ['Tracking']}]
+        self.assertEqual(0, len(list(fdroidserver.lint.check_antiFeatures(app))))
+
+    def test_check_antiFeatures_build_fail(self):
+        app = fdroidserver.metadata.App()
+        app['Builds'] = [{'antifeatures': ['Ads', 'Tracker']}]
+        self.assertEqual(1, len(list(fdroidserver.lint.check_antiFeatures(app))))
+
+
 if __name__ == "__main__":
     parser = optparse.OptionParser()
     parser.add_option(

tests/metadata.TestCase

@@ -178,26 +178,6 @@ 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: