deploy: update_serverwebroots() for testable logic

This moves all of the serverwebroot: logic into a function, and adds tests.
I did this because I ran into issues in the logic in main():

Traceback (most recent call last):
  File "/builds/eighthave/fdroidserver/fdroid", line 22, in <module>
    fdroidserver.__main__.main()
  File "/builds/eighthave/fdroidserver/fdroidserver/__main__.py", line 230, in main
    raise e
  File "/builds/eighthave/fdroidserver/fdroidserver/__main__.py", line 211, in main
    mod.main()
  File "/builds/eighthave/fdroidserver/fdroidserver/deploy.py", line 753, in main
    s = serverwebroot.rstrip('/').split(':')
AttributeError: 'dict' object has no attribute 'rstrip'

fdroidserver/__init__.py

@@ -55,7 +55,9 @@ scan_apk  # NOQA: B101
 scan_repo_files  # NOQA: B101
 from fdroidserver.deploy import (update_awsbucket,
                                  update_servergitmirrors,
+                                 update_serverwebroots,
                                  update_serverwebroot)  # NOQA: E402
 update_awsbucket  # NOQA: B101
 update_servergitmirrors  # NOQA: B101
+update_serverwebroots  # NOQA: B101
 update_serverwebroot  # NOQA: B101

fdroidserver/deploy.py

@@ -313,6 +313,34 @@ def update_serverwebroot(serverwebroot, repo_section):
                 raise FDroidException()
 
 
+def update_serverwebroots(serverwebroots, repo_section, standardwebroot=True):
+    for d in serverwebroots:
+        # this supports both an ssh host:path and just a path
+        serverwebroot = d['url']
+        s = serverwebroot.rstrip('/').split(':')
+        if len(s) == 1:
+            fdroiddir = s[0]
+        elif len(s) == 2:
+            host, fdroiddir = s
+        else:
+            logging.error(_('Malformed serverwebroot line:') + ' ' + serverwebroot)
+            sys.exit(1)
+        repobase = os.path.basename(fdroiddir)
+        if standardwebroot and repobase != 'fdroid':
+            logging.error(
+                _(
+                    'serverwebroot: path does not end with "fdroid", perhaps you meant one of these:'
+                )
+                + '\n\t'
+                + serverwebroot.rstrip('/')
+                + '/fdroid\n\t'
+                + serverwebroot.rstrip('/').rstrip(repobase)
+                + 'fdroid'
+            )
+            sys.exit(1)
+        update_serverwebroot(d, repo_section)
+
+
 def sync_from_localcopy(repo_section, local_copy_dir):
     """Sync the repo from "local copy dir" filesystem to this box.
 
@@ -748,24 +776,6 @@ def main():
     else:
         standardwebroot = True
 
-    for serverwebroot in config.get('serverwebroot', []):
-        # this supports both an ssh host:path and just a path
-        s = serverwebroot.rstrip('/').split(':')
-        if len(s) == 1:
-            fdroiddir = s[0]
-        elif len(s) == 2:
-            host, fdroiddir = s
-        else:
-            logging.error(_('Malformed serverwebroot line:') + ' ' + serverwebroot)
-            sys.exit(1)
-        repobase = os.path.basename(fdroiddir)
-        if standardwebroot and repobase != 'fdroid':
-            logging.error('serverwebroot path does not end with "fdroid", '
-                          + 'perhaps you meant one of these:\n\t'
-                          + serverwebroot.rstrip('/') + '/fdroid\n\t'
-                          + serverwebroot.rstrip('/').rstrip(repobase) + 'fdroid')
-            sys.exit(1)
-
     if options.local_copy_dir is not None:
         local_copy_dir = options.local_copy_dir
     elif config.get('local_copy_dir'):
@@ -826,8 +836,10 @@ def main():
                 sync_from_localcopy(repo_section, local_copy_dir)
             else:
                 update_localcopy(repo_section, local_copy_dir)
-        for serverwebroot in config.get('serverwebroot', []):
+        if config.get('serverwebroot'):
-            update_serverwebroot(serverwebroot, repo_section)
+            update_serverwebroots(
+                config['serverwebroot'], repo_section, standardwebroot
+            )
         if config.get('servergitmirrors', []):
             # update_servergitmirrors will take care of multiple mirrors so don't need a foreach
             servergitmirrors = config.get('servergitmirrors', [])

tests/deploy.TestCase

@@ -35,6 +35,59 @@ class DeployTest(unittest.TestCase):
     def tearDown(self):
         self._td.cleanup()
 
+    def test_update_serverwebroots_bad_None(self):
+        with self.assertRaises(TypeError):
+            fdroidserver.deploy.update_serverwebroots(None, 'repo')
+
+    def test_update_serverwebroots_bad_int(self):
+        with self.assertRaises(TypeError):
+            fdroidserver.deploy.update_serverwebroots(9, 'repo')
+
+    def test_update_serverwebroots_bad_float(self):
+        with self.assertRaises(TypeError):
+            fdroidserver.deploy.update_serverwebroots(1.0, 'repo')
+
+    def test_update_serverwebroots(self):
+        """rsync works with file paths, so this test uses paths for the URLs"""
+        os.chdir(self.testdir)
+        repo = Path('repo')
+        repo.mkdir()
+        fake_apk = repo / 'fake.apk'
+        with fake_apk.open('w') as fp:
+            fp.write('not an APK, but has the right filename')
+        url0 = Path('url0/fdroid')
+        url0.mkdir(parents=True)
+        url1 = Path('url1/fdroid')
+        url1.mkdir(parents=True)
+
+        dest_apk0 = url0 / fake_apk
+        dest_apk1 = url1 / fake_apk
+        self.assertFalse(dest_apk0.is_file())
+        self.assertFalse(dest_apk1.is_file())
+        fdroidserver.deploy.update_serverwebroots(
+            [
+                {'url': str(url0)},
+                {'url': str(url1)},
+            ],
+            str(repo),
+        )
+        self.assertTrue(dest_apk0.is_file())
+        self.assertTrue(dest_apk1.is_file())
+
+    def test_update_serverwebroots_url_does_not_end_with_fdroid(self):
+        with self.assertRaises(SystemExit):
+            fdroidserver.deploy.update_serverwebroots([{'url': 'url'}], 'repo')
+
+    def test_update_serverwebroots_bad_ssh_url(self):
+        with self.assertRaises(SystemExit):
+            fdroidserver.deploy.update_serverwebroots(
+                [{'url': 'f@b.ar::/path/to/fdroid'}], 'repo'
+            )
+
+    def test_update_serverwebroots_unsupported_ssh_url(self):
+        with self.assertRaises(SystemExit):
+            fdroidserver.deploy.update_serverwebroots([{'url': 'ssh://nope'}], 'repo')
+
     def test_update_serverwebroot(self):
         """rsync works with file paths, so this test uses paths for the URLs"""
         os.chdir(self.testdir)