Merge branch 'lint-categories-from-config' into 'master'

Lint: categories from config

See merge request fdroid/fdroidserver!1366

MANIFEST.in

@@ -546,6 +546,7 @@ include tests/checkupdates.TestCase
 include tests/common.TestCase
 include tests/config.py
 include tests/config/antiFeatures.yml
+include tests/config/categories.yml
 include tests/config/de/antiFeatures.yml
 include tests/config/fa/antiFeatures.yml
 include tests/config/ic_antifeature_ads.xml
@@ -662,7 +663,6 @@ include tests/org.bitbucket.tickytacky.mirrormirror_4.apk
 include tests/org.dyndns.fules.ck_20.apk
 include tests/org.sajeg.fallingblocks_3.apk
 include tests/publish.TestCase
-include tests/repo/categories.txt
 include tests/repo/com.example.test.helloworld_1.apk
 include tests/repo/com.politedroid_3.apk
 include tests/repo/com.politedroid_4.apk

fdroidserver/common.py

@@ -4042,7 +4042,6 @@ def is_repo_file(filename, for_gpg_signing=False):
     if isinstance(filename, str):
         filename = filename.encode('utf-8', errors="surrogateescape")
     ignore_files = [
-        b'categories.txt',
         b'entry.jar',
         b'index-v1.jar',
         b'index.css',

fdroidserver/lint.py

@@ -222,6 +222,7 @@ versioncode_check_pattern = re.compile(r"(\\d|\[(0-9|\\d)_?(a-fA-F)?])[+]")
 
 ANTIFEATURES_KEYS = None
 ANTIFEATURES_PATTERN = None
+CATEGORIES_KEYS = list()
 
 
 def load_antiFeatures_config():
@@ -234,6 +235,18 @@ def load_antiFeatures_config():
         ANTIFEATURES_PATTERN = ','.join(ANTIFEATURES_KEYS)
 
 
+def load_categories_config():
+    """Lazy loading, since it might read a lot of files."""
+    global CATEGORIES_KEYS
+    k = 'categories'
+    if not CATEGORIES_KEYS:
+        if config and k in config:
+            CATEGORIES_KEYS = config[k]
+        else:
+            config[k] = common.load_localized_config(k, 'repo')
+            CATEGORIES_KEYS = list(config[k].keys())
+
+
 def check_regexes(app):
     for f, checks in regex_checks.items():
         for m, r in checks:
@@ -371,32 +384,10 @@ def check_empty_fields(app):
         yield _("Categories are not set")
 
 
-all_categories = set(
-    [
-        "Connectivity",
-        "Development",
-        "Games",
-        "Graphics",
-        "Internet",
-        "Money",
-        "Multimedia",
-        "Navigation",
-        "Phone & SMS",
-        "Reading",
-        "Science & Education",
-        "Security",
-        "Sports & Health",
-        "System",
-        "Theming",
-        "Time",
-        "Writing",
-    ]
-)
-
-
 def check_categories(app):
+    """App uses 'Categories' key and parsed config uses 'categories' key."""
     for categ in app.Categories:
-        if categ not in all_categories:
+        if categ not in CATEGORIES_KEYS:
             yield _("Categories '%s' is not valid" % categ)
 
 
@@ -798,6 +789,7 @@ def main():
 
     config = common.read_config(options)
     load_antiFeatures_config()
+    load_categories_config()
 
     # Get all apps...
     allapps = metadata.read_metadata(options.appid)

fdroidserver/update.py

@@ -1846,15 +1846,6 @@ def apply_info_from_latest_apk(apps, apks):
                 app['CurrentVersionCode'] = bestver
 
 
-def make_categories_txt(repodir, categories):
-    """Write a category list in the repo to allow quick access."""
-    catdata = ''
-    for cat in sorted(categories):
-        catdata += cat + '\n'
-    with open(os.path.join(repodir, 'categories.txt'), 'w') as f:
-        f.write(catdata)
-
-
 def archive_old_apks(apps, apks, archapks, repodir, archivedir, defaultkeepversions):
     def filter_apk_list_sorted(apk_list):
         apkList = []
@@ -2246,11 +2237,6 @@ def main():
     # Get all apps...
     apps = metadata.read_metadata()
 
-    # Generate a list of categories...
-    categories = set()
-    for app in apps.values():
-        categories.update(app.Categories)
-
     # Read known apks data (will be updated and written back when we've finished)
     knownapks = common.KnownApks()
 
@@ -2363,7 +2349,6 @@ def main():
 
     # Make the index for the main repo...
     index.make(repoapps, apks, repodirs[0], False)
-    make_categories_txt(repodirs[0], categories)
 
     git_remote = config.get('binary_transparency_remote')
     if git_remote or os.path.isdir(os.path.join('binary_transparency', '.git')):

tests/common.TestCase

@@ -2509,8 +2509,7 @@ class CommonTest(unittest.TestCase):
                 'repo/index.png',
                 'repo/index.xml',
             ]
-            non_repo_files = ['repo/categories.txt']
+            for f in repo_files + index_files:
-            for f in repo_files + index_files + non_repo_files:
                 open(f, 'w').close()
 
             repo_dirs = [
@@ -2543,10 +2542,6 @@ class CommonTest(unittest.TestCase):
                 self.assertTrue(os.path.exists(d), d + ' was created')
                 self.assertFalse(is_repo_file(d), d + ' not repo file')
 
-            for f in non_repo_files:
-                self.assertTrue(os.path.exists(f), f + ' was created')
-                self.assertFalse(is_repo_file(f), f + ' not repo file')
-
     def test_get_apksigner_smartcardoptions(self):
         os.chdir(self.tmpdir)
         with open('config.yml', 'w') as fp:
@@ -2699,6 +2694,23 @@ class CommonTest(unittest.TestCase):
             p = Path(os.path.dirname(__file__) + '/repo' + v['icon']['en-US']['name'])
             self.assertTrue(p.exists())
 
+    def test_load_localized_config_categories(self):
+        """It should load"""
+        categories = fdroidserver.common.load_localized_config('categories', 'repo')
+        self.assertEqual(
+            [
+                'Time',
+                'Development',
+                'GuardianProject',
+                'Multimedia',
+                'Phone & SMS',
+                'Security',
+                'System',
+            ],
+            list(categories.keys()),
+        )
+        self.assertEqual(['en-US'], list(categories['GuardianProject']['name'].keys()))
+
 
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))

tests/config/categories.yml

@@ -0,0 +1,14 @@
+Time:
+  name: Time
+Development:
+  name: Development
+GuardianProject:
+  name: Guardian Project
+Multimedia:
+  name: Multimedia
+Phone & SMS:
+  name: Phone & SMS
+Security:
+  name: Security
+System:
+  name: System

tests/lint.TestCase

@@ -10,6 +10,7 @@ import sys
 import tempfile
 import unittest
 from pathlib import Path
+from testcommon import mkdtemp
 
 localmodule = Path(__file__).resolve().parent.parent
 print('localmodule: ' + str(localmodule))
@@ -30,6 +31,14 @@ class LintTest(unittest.TestCase):
         self.tmpdir = localmodule / '.testfiles'
         self.tmpdir.mkdir(exist_ok=True)
         os.chdir(self.basedir)
+        fdroidserver.common.config = None
+        fdroidserver.lint.config = None
+        fdroidserver.lint.CATEGORIES_KEYS = None
+        self._td = mkdtemp()
+        self.testdir = self._td.name
+
+    def tearDown(self):
+        self._td.cleanup()
 
     def test_check_for_unsupported_metadata_files(self):
         self.assertTrue(fdroidserver.lint.check_for_unsupported_metadata_files())
@@ -313,12 +322,58 @@ class LintTest(unittest.TestCase):
             logging.debug(warn)
         self.assertFalse(anywarns)
 
+    def test_check_categories_in_config(self):
+        fdroidserver.lint.config = {'categories': ['InConfig']}
+        fdroidserver.lint.load_categories_config()
+        app = fdroidserver.metadata.App({'Categories': ['InConfig']})
+        self.assertEqual(0, len(list(fdroidserver.lint.check_categories(app))))
+
+    def test_check_categories_not_in_config(self):
+        fdroidserver.lint.config = dict()
+        fdroidserver.lint.load_categories_config()
+        app = fdroidserver.metadata.App({'Categories': ['NotInConfig']})
+        self.assertEqual(1, len(list(fdroidserver.lint.check_categories(app))))
+
+    def test_check_categories_empty_is_error(self):
+        fdroidserver.lint.config = {'categories': []}
+        fdroidserver.lint.load_categories_config()
+        app = fdroidserver.metadata.App({'Categories': ['something']})
+        self.assertEqual(1, len(list(fdroidserver.lint.check_categories(app))))
+
+    def test_check_categories_old_hardcoded_not_defined(self):
+        fdroidserver.lint.config = {'categories': ['foo', 'bar']}
+        fdroidserver.lint.load_categories_config()
+        app = fdroidserver.metadata.App({'Categories': ['Writing']})
+        self.assertEqual(1, len(list(fdroidserver.lint.check_categories(app))))
+
+    def test_check_categories_from_config_yml(self):
+        """In config.yml, categories is a list."""
+        os.chdir(self.testdir)
+        Path('config.yml').write_text('categories: [foo, bar]')
+        fdroidserver.lint.config = fdroidserver.common.read_config()
+        fdroidserver.lint.load_categories_config()
+        self.assertEqual(fdroidserver.lint.CATEGORIES_KEYS, ['foo', 'bar'])
+        app = fdroidserver.metadata.App({'Categories': ['bar']})
+        self.assertEqual(0, len(list(fdroidserver.lint.check_categories(app))))
+
+    def test_check_categories_from_config_categories_yml(self):
+        """In config/categories.yml, categories is a localized STRINGMAP dict."""
+        os.chdir(self.testdir)
+        os.mkdir('config')
+        Path('config/categories.yml').write_text('{foo: {name: foo}, bar: {name: bar}}')
+        fdroidserver.lint.config = fdroidserver.common.read_config()
+        fdroidserver.lint.load_categories_config()
+        self.assertEqual(fdroidserver.lint.CATEGORIES_KEYS, ['foo', 'bar'])
+        app = fdroidserver.metadata.App({'Categories': ['bar']})
+        self.assertEqual(0, len(list(fdroidserver.lint.check_categories(app))))
+
 
 class LintAntiFeaturesTest(unittest.TestCase):
     def setUp(self):
         self.basedir = localmodule / 'tests'
         os.chdir(self.basedir)
         fdroidserver.common.config = dict()
+        fdroidserver.lint.ANTIFEATURES_KEYS = None
         fdroidserver.lint.load_antiFeatures_config()
 
     def test_check_antiFeatures_empty(self):

tests/repo/categories.txt

@@ -1,10 +0,0 @@
-1
-2.0
-Development
-GuardianProject
-Multimedia
-Phone & SMS
-Security
-System
-Time
-tests

tests/repo/entry.json

@@ -3,8 +3,8 @@
   "version": 20002,
   "index": {
     "name": "/index-v2.json",
-    "sha256": "ba000a3f5e1935d338f374c50cae529b8ce6d988ab3ed67c7a8cf437502f81ad",
+    "sha256": "7117ee6ff4ff2dd71ec3f3d3ad2ef7e9fd4afead9b1f2d39d0b224a1812e78b5",
-    "size": 52481,
+    "size": 53233,
     "numPackages": 10
   },
   "diffs": {}

tests/repo/index-v2.json

@@ -498,6 +498,43 @@
         }
       }
     },
+    "categories": {
+      "Time": {
+        "name": {
+          "en-US": "Time"
+        }
+      },
+      "Development": {
+        "name": {
+          "en-US": "Development"
+        }
+      },
+      "GuardianProject": {
+        "name": {
+          "en-US": "Guardian Project"
+        }
+      },
+      "Multimedia": {
+        "name": {
+          "en-US": "Multimedia"
+        }
+      },
+      "Phone & SMS": {
+        "name": {
+          "en-US": "Phone & SMS"
+        }
+      },
+      "Security": {
+        "name": {
+          "en-US": "Security"
+        }
+      },
+      "System": {
+        "name": {
+          "en-US": "System"
+        }
+      }
+    },
     "requests": {
       "install": [
         "org.adaway"

tests/run-tests

@@ -702,7 +702,11 @@ echo "Description: |" >> metadata/fake.yml
 echo " this is fake" >> metadata/fake.yml
 
 # fake that no JDKs are available
-echo 'java_paths: {}' > config.yml
+cat > config.yml <<EOF
+categories:
+  - Internet
+java_paths: {}
+EOF
 
 LOCAL_COPY_DIR=`create_test_dir`/fdroid
 mkdir -p $LOCAL_COPY_DIR/repo

tests/update.TestCase

@@ -22,6 +22,7 @@ import zipfile
 import textwrap
 from datetime import datetime
 from distutils.version import LooseVersion
+from pathlib import Path
 from testcommon import TmpCwd, mkdtemp
 from unittest import mock
 
@@ -1789,6 +1790,20 @@ class UpdateTest(unittest.TestCase):
                 "DEBUG:root:Checking archiving for org.smssecure.smssecure - apks:0, keepversions:6, archapks:0"
             ])
 
+    def test_categories_txt_is_removed_by_delete_unknown(self):
+        """categories.txt used to be a part of this system, now its nothing."""
+        os.chdir(self.testdir)
+        Path('config.yml').write_text('repo_pubkey: ffffffffffffffffffffffffffffffffffffffff')
+
+        categories_txt = Path('repo/categories.txt')
+        categories_txt.parent.mkdir()
+        categories_txt.write_text('placeholder')
+
+        self.assertTrue(categories_txt.exists())
+        with mock.patch('sys.argv', ['fdroid update', '--delete-unknown', '--nosign']):
+            fdroidserver.update.main()
+        self.assertFalse(categories_txt.exists())
+
 
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))