Merge branch 'deploy-standardization' into 'master'

standardize deploy.py code to follow fdroidserver patterns

See merge request fdroid/fdroidserver!1521

fdroidserver/deploy.py

@@ -30,7 +30,6 @@ from git import Repo
 import yaml
 from argparse import ArgumentParser
 import logging
-from shlex import split
 import pathlib
 import shutil
 import git
@@ -272,7 +271,7 @@ def update_remote_storage_with_rclone(
             logging.info('Custom configuration not found.')
             logging.info(
                 'Using default configuration at {}'.format(
-                    subprocess.check_output(split("rclone config file")).decode("utf-8")
+                    subprocess.check_output(['rclone', 'config', 'file'], text=True)
                 )
             )
             configfilename = None
@@ -281,7 +280,7 @@ def update_remote_storage_with_rclone(
         logging.info('Custom configuration not found.')
         logging.info(
             'Using default configuration at {}'.format(
-                subprocess.check_output(split("rclone config file")).decode("utf-8")
+                subprocess.check_output(['rclone', 'config', 'file'], text=True)
             )
         )
         configfilename = None
@@ -299,20 +298,15 @@ def update_remote_storage_with_rclone(
     else:
         sources = [repo_section]
 
-    for source in sources:
     if isinstance(config['rclone_config'], str):
-            rclone_sync_command = (
+        rclone_config = [config['rclone_config']]
-                'rclone sync '
+    else:
-                + source
+        rclone_config = config['rclone_config']
-                + ' '
-                + config['rclone_config']
-                + ':'
-                + config['awsbucket']
-                + '/'
-                + upload_dir
-            )
 
-            rclone_sync_command = split(rclone_sync_command)
+    for source in sources:
+        for remote_config in rclone_config:
+            complete_remote_path = f'{remote_config}:{config["awsbucket"]}/{upload_dir}'
+            rclone_sync_command = ['rclone', 'sync', source, complete_remote_path]
 
             if verbose:
                 rclone_sync_command += ['--verbose']
@@ -320,45 +314,7 @@ def update_remote_storage_with_rclone(
                 rclone_sync_command += ['--quiet']
 
             if configfilename:
-                rclone_sync_command += split('--config=' + configfilename)
+                rclone_sync_command += ['--config=' + configfilename]
-
-            complete_remote_path = (
-                config['rclone_config'] + ':' + config['awsbucket'] + '/' + upload_dir
-            )
-
-            logging.debug(
-                "rclone sync all files in " + source + ' to ' + complete_remote_path
-            )
-
-            if subprocess.call(rclone_sync_command) != 0:
-                raise FDroidException()
-
-        if isinstance(config['rclone_config'], list):
-            for remote_config in config['rclone_config']:
-                rclone_sync_command = (
-                    'rclone sync '
-                    + source
-                    + ' '
-                    + remote_config
-                    + ':'
-                    + config['awsbucket']
-                    + '/'
-                    + upload_dir
-                )
-
-                rclone_sync_command = split(rclone_sync_command)
-
-                if verbose:
-                    rclone_sync_command += ['--verbose']
-                elif quiet:
-                    rclone_sync_command += ['--quiet']
-
-                if configfilename:
-                    rclone_sync_command += split('--config=' + configfilename)
-
-                complete_remote_path = (
-                    remote_config + ':' + config['awsbucket'] + '/' + upload_dir
-                )
 
             logging.debug(
                 "rclone sync all files in " + source + ' to ' + complete_remote_path

tests/deploy.TestCase

@@ -194,6 +194,31 @@ class DeployTest(unittest.TestCase):
         self.assertFalse(dest_apk.is_file())
         self.assertTrue(dest_index.is_file())
 
+    @mock.patch('subprocess.call')
+    @mock.patch('subprocess.check_output', lambda cmd, text: '/path/to/rclone.conf')
+    def test_update_remote_storage_with_rclone_mock(self, mock_call):
+        def _mock_subprocess_call(cmd):
+            self.assertEqual(
+                cmd,
+                [
+                    'rclone',
+                    'sync',
+                    'repo',
+                    'test_local_config:test_bucket_folder/fdroid/repo',
+                ],
+            )
+            return 0
+
+        mock_call.side_effect = _mock_subprocess_call
+
+        fdroidserver.deploy.config = {
+            'awsbucket': 'test_bucket_folder',
+            'rclone': True,
+            'rclone_config': 'test_local_config',
+        }
+        fdroidserver.deploy.update_remote_storage_with_rclone('repo')
+        mock_call.assert_called_once()
+
     def test_update_serverwebroot(self):
         """rsync works with file paths, so this test uses paths for the URLs"""
         os.chdir(self.testdir)