Fix optional parameters handling in upload_dir

Author: const-cloudinary

cloudinary_cli/modules/upload_dir.py

@@ -51,9 +51,7 @@ def upload_dir(directory, glob_pattern, include_hidden, optional_parameter, opti
         logger.info(f"Uploading directory '{dir_to_upload}'")
         parent = dirname(dir_to_upload)
 
-    options = {
-        **{k: v for k, v in optional_parameter},
-        **{k: parse_option_value(v) for k, v in optional_parameter_parsed},
+    defaults = {
         "resource_type": "auto",
         "invalidate": True,
         "unique_filename": False,
@@ -62,6 +60,12 @@ def upload_dir(directory, glob_pattern, include_hidden, optional_parameter, opti
         "upload_preset": preset
     }
 
+    options = {
+        **defaults,
+        **{k: v for k, v in optional_parameter},
+        **{k: parse_option_value(v) for k, v in optional_parameter_parsed},
+    }
+
     uploads = []
 
     for file_path in dir_to_upload.glob(glob_pattern):
@@ -77,7 +81,7 @@ def upload_dir(directory, glob_pattern, include_hidden, optional_parameter, opti
     logger.info(style("{} resources uploaded".format(len(items)), fg="green"))
 
     if skipped:
-        logger.warn("{} items skipped".format(len(skipped)))
+        logger.warning("{} items skipped".format(len(skipped)))
         return False
 
     return True

cloudinary_cli/utils/utils.py

@@ -157,8 +157,8 @@ def write_json_list_to_csv(json_list, filename, fields_to_keep=()):
 
 
 def run_tasks_concurrently(func, tasks, concurrent_workers):
-    thread_pool = pool.ThreadPool(concurrent_workers)
-    thread_pool.starmap(func, tasks)
+    with pool.ThreadPool(concurrent_workers) as thread_pool:
+        thread_pool.starmap(func, tasks)
 
 
 def confirm_action(message="Continue? (y/N)"):

test/helper_test.py

@@ -1,5 +1,6 @@
 import os
 import random
+import re
 import time
 from functools import wraps
 from pathlib import Path
@@ -37,7 +38,45 @@ def api_response_mock():
 
 
 def uploader_response_mock():
-    return http_response_mock('{"foo":"bar"}')
+    return http_response_mock('''{
+      "public_id": "mocked_file_id.bin",
+      "resource_type": "raw",
+      "type": "upload",
+      "format":"bin",
+      "foo": "bar"
+    }''')
+
+
+def get_request_url(mocker):
+    return mocker.call_args[0][1]
+
+
+def get_params(mocker):
+    """
+    Extracts query parameters from mocked urllib3.request `fields` param.
+    Supports both list and dict values of `fields`. Returns params as dictionary.
+    Supports two list params formats:
+      - {"urls[0]": "http://host1", "urls[1]": "http://host2"}
+      - [("urls[]", "http://host1"), ("urls[]", "http://host2")]
+    In both cases the result would be {"urls": ["http://host1", "http://host2"]}
+    """
+
+    args = mocker.call_args[0]
+    if not args or not args[2]:
+        return {}
+    params = {}
+    reg = re.compile(r'^(.*)\[\d*]$')
+    fields = args[2].items() if isinstance(args[2], dict) else args[2]
+    for k, v in fields:
+        match = reg.match(k)
+        if match:
+            name = match.group(1)
+            if name not in params:
+                params[name] = []
+            params[name].append(v)
+        else:
+            params[k] = v
+    return params
 
 
 def retry_assertion(num_tries=3, delay=3):

test/test_modules/test_cli_upload_dir.py

@@ -1,11 +1,14 @@
 import time
 import unittest
+from unittest.mock import patch
 
 from click.testing import CliRunner
 
 from cloudinary_cli.cli import cli
-from test.helper_test import unique_suffix, TEST_FILES_DIR, delete_cld_folder_if_exists
+from test.helper_test import unique_suffix, TEST_FILES_DIR, delete_cld_folder_if_exists, uploader_response_mock, \
+    get_request_url, get_params
 
+UPLOAD_MOCK_RESPONSE = uploader_response_mock()
 
 class TestCLIUploadDir(unittest.TestCase):
     runner = CliRunner()
@@ -51,3 +54,16 @@ def test_upload_dir_with_exclude_dir_name_option(self):
         self.assertIn("12 resources uploaded", result.output)
         self.assertIn("as " + self.CLD_UPLOAD_DIR, result.output)
         self.assertNotIn("as " + self.CLD_UPLOAD_DIR + "/test_sync/", result.output)
+
+    @patch('urllib3.request.RequestMethods.request')
+    def test_upload_dir_override_defaults(self, mocker):
+        mocker.return_value = UPLOAD_MOCK_RESPONSE
+
+        result = self.runner.invoke(cli, ["upload_dir", TEST_FILES_DIR, "-f", self.CLD_UPLOAD_DIR,
+                                          "-o", "resource_type", "raw", "-O", "unique_filename", "True"])
+
+        self.assertEqual(0, result.exit_code)
+        self.assertIn("12 resources uploaded", result.output)
+
+        self.assertIn("raw/upload", get_request_url(mocker))
+        self.assertTrue(get_params(mocker)['unique_filename'])