From d5a1439457607e66a3513e89b0734322a5c57c80 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 24 Apr 2023 20:10:17 +0200 Subject: [PATCH] lint: Anti-Features validator uses names from config --- fdroidserver/lint.py | 35 ++++++++++++++++++++++ fdroidserver/metadata.py | 4 --- tests/lint.TestCase | 63 ++++++++++++++++++++++++++++++---------- tests/metadata.TestCase | 20 ------------- 4 files changed, 83 insertions(+), 39 deletions(-) diff --git a/fdroidserver/lint.py b/fdroidserver/lint.py index fb94e258..1283409b 100644 --- a/fdroidserver/lint.py +++ b/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, diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 896c224e..5341a3a7 100644 --- a/fdroidserver/metadata.py +++ b/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"]), diff --git a/tests/lint.TestCase b/tests/lint.TestCase index 6fbe76cf..544b18c8 100755 --- a/tests/lint.TestCase +++ b/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( diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index b4301fba..82d5c3e5 100755 --- a/tests/metadata.TestCase +++ b/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: