Merge branch 'error-messages-and-categories' into 'master'

error messages and categories

Closes #1137

See merge request fdroid/fdroidserver!1384

fdroidserver/common.py

@@ -399,6 +399,11 @@ def read_config(opts=None):
             config = yaml.safe_load(fp)
         if not config:
             config = {}
+        if not isinstance(config, dict):
+            msg = _('{path} is not "key: value" dict, but a {datatype}!')
+            raise TypeError(
+                msg.format(path=config_file, datatype=type(config).__name__)
+            )
     elif os.path.exists(old_config_file):
         logging.warning(_("""{oldfile} is deprecated, use {newfile}""")
                         .format(oldfile=old_config_file, newfile=config_file))
@@ -528,6 +533,9 @@ def load_localized_config(name, repodir):
             locale = DEFAULT_LOCALE
         with open(f, encoding="utf-8") as fp:
             elem = yaml.safe_load(fp)
+            if not isinstance(elem, dict):
+                msg = _('{path} is not "key: value" dict, but a {datatype}!')
+                raise TypeError(msg.format(path=f, datatype=type(elem).__name__))
             for afname, field_dict in elem.items():
                 if afname not in ret:
                     ret[afname] = dict()

fdroidserver/deploy.py

@@ -277,6 +277,12 @@ def update_serverwebroot(serverwebroot, repo_section):
     has a low resolution timestamp
 
     """
+    try:
+        subprocess.run(['rsync', '--version'], capture_output=True, check=True)
+    except Exception as e:
+        raise FDroidException(
+            _('rsync is missing or broken: {error}').format(error=e)
+        ) from e
     rsyncargs = ['rsync', '--archive', '--delete-after', '--safe-links']
     if not options.no_checksum:
         rsyncargs.append('--checksum')

fdroidserver/index.py

@@ -711,6 +711,7 @@ def make_v2(apps, packages, repodir, repodict, requestsdict, fdroid_signing_key_
 
     output_packages = collections.OrderedDict()
     output['packages'] = output_packages
+    categories_used_by_apps = set()
     for package in packages:
         packageName = package['packageName']
         if packageName not in apps:
@@ -730,7 +731,9 @@ def make_v2(apps, packages, repodir, repodict, requestsdict, fdroid_signing_key_
         else:
             packagelist = {}
             output_packages[packageName] = packagelist
-            packagelist["metadata"] = package_metadata(apps[packageName], repodir)
+            app = apps[packageName]
+            categories_used_by_apps.update(app.get('Categories', []))
+            packagelist["metadata"] = package_metadata(app, repodir)
             if "signer" in package:
                 packagelist["metadata"]["preferredSigner"] = package["signer"]
 
@@ -738,6 +741,19 @@ def make_v2(apps, packages, repodir, repodict, requestsdict, fdroid_signing_key_
 
         packagelist["versions"][package["hash"]] = convert_version(package, apps[packageName], repodir)
 
+    if categories_used_by_apps and not output['repo'].get(CATEGORIES_CONFIG_NAME):
+        output['repo'][CATEGORIES_CONFIG_NAME] = dict()
+    # include definitions for "auto-defined" categories, e.g. just used in app metadata
+    for category in sorted(categories_used_by_apps):
+        if category not in output['repo'][CATEGORIES_CONFIG_NAME]:
+            output['repo'][CATEGORIES_CONFIG_NAME][category] = dict()
+    # do not include defined categories if no apps use them
+    for category in list(output['repo'].get(CATEGORIES_CONFIG_NAME, list())):
+        if category not in categories_used_by_apps:
+            del output['repo'][CATEGORIES_CONFIG_NAME][category]
+            msg = _('Category "{category}" defined but not used for any apps!')
+            logging.warning(msg.format(category=category))
+
     entry = {}
     entry["timestamp"] = repodict["timestamp"]
 

tests/common.TestCase

@@ -1919,6 +1919,18 @@ class CommonTest(unittest.TestCase):
             config = fdroidserver.common.read_config(fdroidserver.common.options)
             self.assertEqual(os.getenv('SECRET', 'fail'), config.get('keypass'))
 
+    def test_with_config_yml_is_dict(self):
+        os.chdir(self.tmpdir)
+        Path('config.yml').write_text('apksigner = /placeholder/path')
+        with self.assertRaises(TypeError):
+            fdroidserver.common.read_config(fdroidserver.common.options)
+
+    def test_with_config_yml_is_not_mixed_type(self):
+        os.chdir(self.tmpdir)
+        Path('config.yml').write_text('k: v\napksigner = /placeholder/path')
+        with self.assertRaises(yaml.scanner.ScannerError):
+            fdroidserver.common.read_config(fdroidserver.common.options)
+
     def test_with_config_py(self):
         """Make sure it is still possible to use config.py alone."""
         os.chdir(self.tmpdir)
@@ -2716,6 +2728,27 @@ class CommonTest(unittest.TestCase):
         )
         self.assertEqual(['en-US'], list(categories['GuardianProject']['name'].keys()))
 
+    def test_load_localized_config_0_file(self):
+        os.chdir(self.testdir)
+        os.mkdir('config')
+        Path('config/categories.yml').write_text('')
+        with self.assertRaises(TypeError):
+            fdroidserver.common.load_localized_config(CATEGORIES_CONFIG_NAME, 'repo')
+
+    def test_load_localized_config_string(self):
+        os.chdir(self.testdir)
+        os.mkdir('config')
+        Path('config/categories.yml').write_text('this is a string')
+        with self.assertRaises(TypeError):
+            fdroidserver.common.load_localized_config(CATEGORIES_CONFIG_NAME, 'repo')
+
+    def test_load_localized_config_list(self):
+        os.chdir(self.testdir)
+        os.mkdir('config')
+        Path('config/categories.yml').write_text('- System')
+        with self.assertRaises(TypeError):
+            fdroidserver.common.load_localized_config(CATEGORIES_CONFIG_NAME, 'repo')
+
 
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))

tests/deploy.TestCase

@@ -18,6 +18,7 @@ if localmodule not in sys.path:
 
 import fdroidserver.common
 import fdroidserver.deploy
+from fdroidserver.exception import FDroidException
 from testcommon import TmpCwd, mkdtemp
 
 
@@ -56,6 +57,13 @@ class DeployTest(unittest.TestCase):
         fdroidserver.deploy.update_serverwebroot(str(serverwebroot), 'repo')
         self.assertTrue(dest_apk.is_file())
 
+    @mock.patch.dict(os.environ, clear=True)
+    def test_update_serverwebroot_no_rsync_error(self):
+        os.environ['PATH'] = self.testdir
+        os.chdir(self.testdir)
+        with self.assertRaises(FDroidException):
+            fdroidserver.deploy.update_serverwebroot('serverwebroot', 'repo')
+
     def test_update_serverwebroot_make_cur_version_link(self):
         # setup parameters for this test run
         fdroidserver.deploy.options.no_checksum = True

tests/metadata.TestCase

@@ -1863,6 +1863,8 @@ class MetadataTest(unittest.TestCase):
                 AntiFeatures:
                   - NonFreeNet
                 Categories:
+                  - Multimedia
+                  - Security
                   - Time
                 License: GPL-3.0-only
                 SourceCode: https://github.com/miguelvps/PoliteDroid

tests/metadata/com.politedroid.yml

@@ -1,6 +1,8 @@
 AntiFeatures:
   - NonFreeNet
 Categories:
+  - Multimedia
+  - Security
   - Time
 License: GPL-3.0-only
 SourceCode: https://github.com/miguelvps/PoliteDroid

tests/metadata/dump/com.politedroid.yaml

@@ -161,6 +161,8 @@ Builds:
   versionCode: 6
   versionName: '1.5'
 Categories:
+- Multimedia
+- Security
 - Time
 Changelog: ''
 CurrentVersion: '1.5'

tests/repo/entry.json

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

tests/repo/index-v1.json

@@ -174,6 +174,8 @@
         "NonFreeNet"
       ],
       "categories": [
+        "Multimedia",
+        "Security",
         "Time"
       ],
       "suggestedVersionName": "1.5",

tests/repo/index-v2.json

@@ -533,7 +533,10 @@
         "name": {
           "en-US": "System"
         }
-      }
+      },
+      "1": {},
+      "2.0": {},
+      "tests": {}
     },
     "requests": {
       "install": [
@@ -550,6 +553,8 @@
       "metadata": {
         "added": 1498176000000,
         "categories": [
+          "Multimedia",
+          "Security",
           "Time"
         ],
         "issueTracker": "https://github.com/miguelvps/PoliteDroid/issues",

tests/repo/index.xml

@@ -311,8 +311,8 @@ APK is called F-Droid Privileged Extension.</desc>
 		<icon>com.politedroid.6.png</icon>
 		<desc>Activates silent mode during calendar events.</desc>
 		<license>GPL-3.0-only</license>
-		<categories>Time</categories>
+		<categories>Multimedia,Security,Time</categories>
-		<category>Time</category>
+		<category>Multimedia</category>
 		<web></web>
 		<source>https://github.com/miguelvps/PoliteDroid</source>
 		<tracker>https://github.com/miguelvps/PoliteDroid/issues</tracker>

tests/update.TestCase

@@ -53,6 +53,7 @@ import fdroidserver.common
 import fdroidserver.exception
 import fdroidserver.metadata
 import fdroidserver.update
+from fdroidserver.common import CATEGORIES_CONFIG_NAME
 
 
 DONATION_FIELDS = ('Donate', 'Liberapay', 'OpenCollective')
@@ -1804,6 +1805,123 @@ class UpdateTest(unittest.TestCase):
             fdroidserver.update.main()
         self.assertFalse(categories_txt.exists())
 
+    def test_no_blank_auto_defined_categories(self):
+        """When no app has Categories, there should be no definitions in the repo."""
+        os.chdir(self.testdir)
+        os.mkdir('metadata')
+        os.mkdir('repo')
+        Path('config.yml').write_text(
+            'repo_pubkey: ffffffffffffffffffffffffffffffffffffffff'
+        )
+
+        testapk = os.path.join('repo', 'com.politedroid_6.apk')
+        shutil.copy(os.path.join(self.basedir, testapk), testapk)
+        Path('metadata/com.politedroid.yml').write_text('Name: Polite')
+
+        with mock.patch('sys.argv', ['fdroid update', '--delete-unknown', '--nosign']):
+            fdroidserver.update.main()
+        with open('repo/index-v2.json') as fp:
+            index = json.load(fp)
+        self.assertFalse(CATEGORIES_CONFIG_NAME in index['repo'])
+
+    def test_auto_defined_categories(self):
+        """Repos that don't define categories in config/ should use auto-generated."""
+        os.chdir(self.testdir)
+        os.mkdir('metadata')
+        os.mkdir('repo')
+        Path('config.yml').write_text(
+            'repo_pubkey: ffffffffffffffffffffffffffffffffffffffff'
+        )
+
+        testapk = os.path.join('repo', 'com.politedroid_6.apk')
+        shutil.copy(os.path.join(self.basedir, testapk), testapk)
+        Path('metadata/com.politedroid.yml').write_text('Categories: [Time]')
+
+        with mock.patch('sys.argv', ['fdroid update', '--delete-unknown', '--nosign']):
+            fdroidserver.update.main()
+        with open('repo/index-v2.json') as fp:
+            index = json.load(fp)
+        self.assertEqual(
+            {'Time': dict()},
+            index['repo'][CATEGORIES_CONFIG_NAME],
+        )
+
+    def test_auto_defined_categories_two_apps(self):
+        """Repos that don't define categories in config/ should use auto-generated."""
+        os.chdir(self.testdir)
+        os.mkdir('metadata')
+        os.mkdir('repo')
+        Path('config.yml').write_text(
+            'repo_pubkey: ffffffffffffffffffffffffffffffffffffffff'
+        )
+
+        testapk = os.path.join('repo', 'com.politedroid_6.apk')
+        shutil.copy(os.path.join(self.basedir, testapk), testapk)
+        Path('metadata/com.politedroid.yml').write_text('Categories: [bar]')
+        testapk = os.path.join('repo', 'souch.smsbypass_9.apk')
+        shutil.copy(os.path.join(self.basedir, testapk), testapk)
+        Path('metadata/souch.smsbypass.yml').write_text('Categories: [foo, bar]')
+
+        with mock.patch('sys.argv', ['fdroid update', '--delete-unknown', '--nosign']):
+            fdroidserver.update.main()
+        with open('repo/index-v2.json') as fp:
+            index = json.load(fp)
+        self.assertEqual(
+            {'bar': dict(), 'foo': dict()},
+            index['repo'][CATEGORIES_CONFIG_NAME],
+        )
+
+    def test_auto_defined_categories_mix_into_config_categories(self):
+        """Repos that don't define all categories in config/ also use auto-generated."""
+        os.chdir(self.testdir)
+        os.mkdir('config')
+        Path('config/categories.yml').write_text('System: {name: System Apps}')
+        os.mkdir('metadata')
+        os.mkdir('repo')
+        Path('config.yml').write_text(
+            'repo_pubkey: ffffffffffffffffffffffffffffffffffffffff'
+        )
+
+        testapk = os.path.join('repo', 'com.politedroid_6.apk')
+        shutil.copy(os.path.join(self.basedir, testapk), testapk)
+        Path('metadata/com.politedroid.yml').write_text('Categories: [Time]')
+        testapk = os.path.join('repo', 'souch.smsbypass_9.apk')
+        shutil.copy(os.path.join(self.basedir, testapk), testapk)
+        Path('metadata/souch.smsbypass.yml').write_text('Categories: [System, Time]')
+
+        with mock.patch('sys.argv', ['fdroid update', '--delete-unknown', '--nosign']):
+            fdroidserver.update.main()
+        with open('repo/index-v2.json') as fp:
+            index = json.load(fp)
+        self.assertEqual(
+            {'System': {'name': {'en-US': 'System Apps'}}, 'Time': dict()},
+            index['repo'][CATEGORIES_CONFIG_NAME],
+        )
+
+    def test_empty_categories_not_in_index(self):
+        """A category with no apps should be ignored, even if defined in config."""
+        os.chdir(self.testdir)
+        os.mkdir('config')
+        Path('config/categories.yml').write_text('System: {name: S}\nTime: {name: T}\n')
+        os.mkdir('metadata')
+        os.mkdir('repo')
+        Path('config.yml').write_text(
+            'repo_pubkey: ffffffffffffffffffffffffffffffffffffffff'
+        )
+
+        testapk = os.path.join('repo', 'com.politedroid_6.apk')
+        shutil.copy(os.path.join(self.basedir, testapk), testapk)
+        Path('metadata/com.politedroid.yml').write_text('Categories: [Time]')
+
+        with mock.patch('sys.argv', ['fdroid update', '--delete-unknown', '--nosign']):
+            fdroidserver.update.main()
+        with open('repo/index-v2.json') as fp:
+            index = json.load(fp)
+        self.assertEqual(
+            {'Time': {'name': {'en-US': 'T'}}},
+            index['repo'][CATEGORIES_CONFIG_NAME],
+        )
+
 
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))