refactor(mixins): extract custom type transforms into utils

nejch · JohnVillalovos · commit 09b3b2225361 · 2022-05-29T08:56:19.000-07:00
diff --git a/gitlab/mixins.py b/gitlab/mixins.py
@@ -33,7 +33,6 @@
 import gitlab
 from gitlab import base, cli
 from gitlab import exceptions as exc
-from gitlab import types as g_types
 from gitlab import utils
 
 __all__ = [
@@ -214,8 +213,8 @@ def list(self, **kwargs: Any) -> Union[base.RESTObjectList, List[base.RESTObject
             GitlabListError: If the server cannot perform the request
         """
 
-        # Duplicate data to avoid messing with what the user sent us
-        data = kwargs.copy()
+        data, _ = utils._transform_types(kwargs, self._types, transform_files=False)
+
         if self.gitlab.per_page:
             data.setdefault("per_page", self.gitlab.per_page)
 
@@ -226,13 +225,6 @@ def list(self, **kwargs: Any) -> Union[base.RESTObjectList, List[base.RESTObject
         if self.gitlab.order_by:
             data.setdefault("order_by", self.gitlab.order_by)
 
-        # We get the attributes that need some special transformation
-        if self._types:
-            for attr_name, type_cls in self._types.items():
-                if attr_name in data.keys():
-                    type_obj = type_cls(data[attr_name])
-                    data[attr_name] = type_obj.get_for_api()
-
         # Allow to overwrite the path, handy for custom listings
         path = data.pop("path", self.path)
 
@@ -298,23 +290,7 @@ def create(
             data = {}
 
         self._check_missing_create_attrs(data)
-        files = {}
-
-        # We get the attributes that need some special transformation
-        if self._types:
-            # Duplicate data to avoid messing with what the user sent us
-            data = data.copy()
-            for attr_name, type_cls in self._types.items():
-                if attr_name in data.keys():
-                    type_obj = type_cls(data[attr_name])
-
-                    # if the type if FileAttribute we need to pass the data as
-                    # file
-                    if isinstance(type_obj, g_types.FileAttribute):
-                        k = type_obj.get_file_name(attr_name)
-                        files[attr_name] = (k, data.pop(attr_name))
-                    else:
-                        data[attr_name] = type_obj.get_for_api()
+        data, files = utils._transform_types(data, self._types)
 
         # Handle specific URL for creation
         path = kwargs.pop("path", self.path)
@@ -394,23 +370,7 @@ def update(
             path = f"{self.path}/{utils.EncodedId(id)}"
 
         self._check_missing_update_attrs(new_data)
-        files = {}
-
-        # We get the attributes that need some special transformation
-        if self._types:
-            # Duplicate data to avoid messing with what the user sent us
-            new_data = new_data.copy()
-            for attr_name, type_cls in self._types.items():
-                if attr_name in new_data.keys():
-                    type_obj = type_cls(new_data[attr_name])
-
-                    # if the type if FileAttribute we need to pass the data as
-                    # file
-                    if isinstance(type_obj, g_types.FileAttribute):
-                        k = type_obj.get_file_name(attr_name)
-                        files[attr_name] = (k, new_data.pop(attr_name))
-                    else:
-                        new_data[attr_name] = type_obj.get_for_api()
+        new_data, files = utils._transform_types(new_data, self._types)
 
         http_method = self._get_update_method()
         result = http_method(path, post_data=new_data, files=files, **kwargs)
diff --git a/gitlab/utils.py b/gitlab/utils.py
@@ -19,10 +19,12 @@
 import traceback
 import urllib.parse
 import warnings
-from typing import Any, Callable, Dict, Optional, Type, Union
+from typing import Any, Callable, Dict, Optional, Tuple, Type, Union
 
 import requests
 
+from gitlab import types
+
 
 class _StdoutStream:
     def __call__(self, chunk: Any) -> None:
@@ -47,6 +49,39 @@ def response_content(
     return None
 
 
+def _transform_types(
+    data: Dict[str, Any], custom_types: dict, *, transform_files: Optional[bool] = True
+) -> Tuple[dict, dict]:
+    """Copy the data dict with attributes that have custom types and transform them
+    before being sent to the server.
+
+    If ``transform_files`` is ``True`` (default), also populates the ``files`` dict for
+    FileAttribute types with tuples to prepare fields for requests' MultipartEncoder:
+    https://toolbelt.readthedocs.io/en/latest/user.html#multipart-form-data-encoder
+
+    Returns:
+        A tuple of the transformed data dict and files dict"""
+
+    # Duplicate data to avoid messing with what the user sent us
+    data = data.copy()
+    files = {}
+
+    for attr_name, type_cls in custom_types.items():
+        if attr_name not in data:
+            continue
+
+        type_obj = type_cls(data[attr_name])
+
+        # if the type if FileAttribute we need to pass the data as file
+        if transform_files and isinstance(type_obj, types.FileAttribute):
+            key = type_obj.get_file_name(attr_name)
+            files[attr_name] = (key, data.pop(attr_name))
+        else:
+            data[attr_name] = type_obj.get_for_api()
+
+    return data, files
+
+
 def copy_dict(
     *,
     src: Dict[str, Any],
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
@@ -18,7 +18,7 @@
 import json
 import warnings
 
-from gitlab import utils
+from gitlab import types, utils
 
 
 class TestEncodedId:
@@ -95,3 +95,30 @@ def test_warn(self):
         assert warn_message in str(warning.message)
         assert __file__ in str(warning.message)
         assert warn_source == warning.source
+
+
+def test_transform_types_copies_data_with_empty_files():
+    data = {"attr": "spam"}
+    new_data, files = utils._transform_types(data, {})
+
+    assert new_data is not data
+    assert new_data == data
+    assert files == {}
+
+
+def test_transform_types_with_transform_files_populates_files():
+    custom_types = {"attr": types.FileAttribute}
+    data = {"attr": "spam"}
+    new_data, files = utils._transform_types(data, custom_types)
+
+    assert new_data == {}
+    assert files["attr"] == ("attr", "spam")
+
+
+def test_transform_types_without_transform_files_populates_data_with_empty_files():
+    custom_types = {"attr": types.FileAttribute}
+    data = {"attr": "spam"}
+    new_data, files = utils._transform_types(data, custom_types, transform_files=False)
+
+    assert new_data == {"attr": "spam"}
+    assert files == {}
