fix: use the [] after key names for array variables

JohnVillalovos · JohnVillalovos · commit 59615a2fea8f · 2022-02-01T14:57:35.000-08:00
1. If a value is of type ArrayAttribute then append '[]' to the name of the value. 2. Move processing of most GitlabAttributes into the client.py:http_request() method. Now we convert our params into a list of tuples so that we can have multiple identical keys but with different values. This is step 3 in a series of steps of our goal to add full support for the GitLab API data types[1]: * array * hash * array of hashes Step one was: commit 5127b15 Step two was: commit a57334f Fixes: #1698 [1] https://docs.gitlab.com/ee/api/#encoding-api-parameters-of-array-and-hash-types
diff --git a/gitlab/client.py b/gitlab/client.py
@@ -16,6 +16,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Wrapper for the GitLab API."""
 
+import copy
 import os
 import time
 from typing import Any, cast, Dict, List, Optional, Tuple, TYPE_CHECKING, Union
@@ -27,6 +28,7 @@
 import gitlab.config
 import gitlab.const
 import gitlab.exceptions
+from gitlab import types as gl_types
 from gitlab import utils
 
 REDIRECT_MSG = (
@@ -604,6 +606,28 @@ def _prepare_send_data(
 
         return (post_data, None, "application/json")
 
+    @staticmethod
+    def _prepare_dict_for_api(*, in_dict: Dict[str, Any]) -> Dict[str, Any]:
+        result: Dict[str, Any] = {}
+        for key, value in in_dict.items():
+            if isinstance(value, gl_types.GitlabAttribute):
+                result[key] = value.get_for_api()
+            else:
+                result[key] = copy.deepcopy(in_dict[key])
+        return result
+
+    @staticmethod
+    def _param_dict_to_param_tuples(*, params: Dict[str, Any]) -> List[Tuple[str, Any]]:
+        """Convert a dict to a list of key/values. This will be used to pass
+        values to requests"""
+        result: List[Tuple[str, Any]] = []
+        for key, value in params.items():
+            if isinstance(value, gl_types.GitlabAttribute):
+                result.extend(value.get_as_tuple_list(key=key))
+            else:
+                result.append((key, value))
+        return result
+
     def http_request(
         self,
         verb: str,
@@ -663,6 +687,10 @@ def http_request(
         else:
             utils.copy_dict(params, kwargs)
 
+        tuple_params = self._param_dict_to_param_tuples(params=params)
+        if isinstance(post_data, dict):
+            post_data = self._prepare_dict_for_api(in_dict=post_data)
+
         opts = self._get_session_opts()
 
         verify = opts.pop("verify")
@@ -682,7 +710,7 @@ def http_request(
                 url=url,
                 json=json,
                 data=data,
-                params=params,
+                params=tuple_params,
                 timeout=timeout,
                 verify=verify,
                 stream=streamed,
diff --git a/gitlab/mixins.py b/gitlab/mixins.py
@@ -230,8 +230,7 @@ def list(self, **kwargs: Any) -> Union[base.RESTObjectList, List[base.RESTObject
         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()
+                    data[attr_name] = type_cls(data[attr_name])
 
         # Allow to overwrite the path, handy for custom listings
         path = data.pop("path", self.path)
@@ -307,14 +306,13 @@ def create(
             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[attr_name] = type_obj
 
         # Handle specific URL for creation
         path = kwargs.pop("path", self.path)
@@ -410,7 +408,7 @@ def update(
                         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[attr_name] = type_obj
 
         http_method = self._get_update_method()
         result = http_method(path, post_data=new_data, files=files, **kwargs)
diff --git a/gitlab/types.py b/gitlab/types.py
@@ -15,7 +15,7 @@
 # You should have received a copy of the GNU Lesser General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-from typing import Any, Optional, TYPE_CHECKING
+from typing import Any, List, Optional, Tuple, TYPE_CHECKING
 
 
 class GitlabAttribute:
@@ -31,6 +31,9 @@ def set_from_cli(self, cli_value: Any) -> None:
     def get_for_api(self) -> Any:
         return self._value
 
+    def get_as_tuple_list(self, *, key: str) -> List[Tuple[str, Any]]:
+        return [(key, self._value)]
+
 
 class _ListArrayAttribute(GitlabAttribute):
     """Helper class to support `list` / `array` types."""
@@ -55,17 +58,31 @@ class ArrayAttribute(_ListArrayAttribute):
     """To support `array` types as documented in
     https://docs.gitlab.com/ee/api/#array"""
 
+    def get_as_tuple_list(self, *, key: str) -> List[Tuple[str, str]]:
+        if isinstance(self._value, str):
+            return [(f"{key}[]", self._value)]
+
+        if TYPE_CHECKING:
+            assert isinstance(self._value, list)
+        return [(f"{key}[]", str(value)) for value in self._value]
+
 
 class CommaSeparatedListAttribute(_ListArrayAttribute):
     """For values which are sent to the server as a Comma Separated Values
     (CSV) string.  We allow them to be specified as a list and we convert it
     into a CSV"""
 
+    def get_as_tuple_list(self, *, key: str) -> List[Tuple[str, str]]:
+        return [(key, self.get_for_api())]
+
 
 class LowercaseStringAttribute(GitlabAttribute):
     def get_for_api(self) -> str:
         return str(self._value).lower()
 
+    def get_as_tuple_list(self, *, key: str) -> List[Tuple[str, str]]:
+        return [(key, self.get_for_api())]
+
 
 class FileAttribute(GitlabAttribute):
     def get_file_name(self, attr_name: Optional[str] = None) -> Optional[str]:
diff --git a/gitlab/utils.py b/gitlab/utils.py
@@ -47,6 +47,8 @@ def response_content(
 def copy_dict(dest: Dict[str, Any], src: Dict[str, Any]) -> None:
     for k, v in src.items():
         if isinstance(v, dict):
+            # NOTE(jlvillal): This provides some support for the `hash` type
+            # https://docs.gitlab.com/ee/api/#hash
             # Transform dict values to new attributes. For example:
             # custom_attributes: {'foo', 'bar'} =>
             #   "custom_attributes['foo']": "bar"
diff --git a/tests/functional/api/test_groups.py b/tests/functional/api/test_groups.py
@@ -91,6 +91,11 @@ def test_groups(gl):
     assert len(group1.members.list()) == 3
     assert len(group2.members.list()) == 2
 
+    # Test `user_ids` array
+    result = group1.members.list(user_ids=[user.id, 99999])
+    assert len(result) == 1
+    assert result[0].id == user.id
+
     group1.members.delete(user.id)
     assert len(group1.members.list()) == 2
     assert len(group1.members_all.list())
diff --git a/tests/unit/test_types.py b/tests/unit/test_types.py
@@ -25,6 +25,7 @@ def test_gitlab_attribute_get():
     o.set_from_cli("whatever2")
     assert o.get() == "whatever2"
     assert o.get_for_api() == "whatever2"
+    assert o.get_as_tuple_list(key="foo") == [("foo", "whatever2")]
 
     o = types.GitlabAttribute()
     assert o._value is None
@@ -64,6 +65,24 @@ def test_array_attribute_get_for_api_from_int_list():
     assert o.get_for_api() == "1,9,7"
 
 
+def test_array_attribute_get_as_tuple_list_from_list():
+    o = types.ArrayAttribute(["foo", "bar", "baz"])
+    assert o.get_as_tuple_list(key="identifier") == [
+        ("identifier[]", "foo"),
+        ("identifier[]", "bar"),
+        ("identifier[]", "baz"),
+    ]
+
+
+def test_array_attribute_get_as_tuple_list_from_int_list():
+    o = types.ArrayAttribute([1, 9, 7])
+    assert o.get_as_tuple_list(key="identifier") == [
+        ("identifier[]", "1"),
+        ("identifier[]", "9"),
+        ("identifier[]", "7"),
+    ]
+
+
 def test_array_attribute_does_not_split_string():
     o = types.ArrayAttribute("foo")
     assert o.get_for_api() == "foo"
@@ -86,7 +105,22 @@ def test_csv_string_attribute_get_for_api_from_int_list():
     assert o.get_for_api() == "1,9,7"
 
 
+def test_csv_string_attribute_get_as_tuple_list_from_list():
+    o = types.CommaSeparatedListAttribute(["foo", "bar", "baz"])
+    assert o.get_as_tuple_list(key="identifier") == [("identifier", "foo,bar,baz")]
+
+
+def test_csv_string_attribute_get_as_tuple_list_from_int_list():
+    o = types.CommaSeparatedListAttribute([1, 9, 7])
+    assert o.get_as_tuple_list(key="identifier") == [("identifier", "1,9,7")]
+
+
 # LowercaseStringAttribute tests
 def test_lowercase_string_attribute_get_for_api():
     o = types.LowercaseStringAttribute("FOO")
     assert o.get_for_api() == "foo"
+
+
+def test_lowercase_string_attribute_get_as_tuple():
+    o = types.LowercaseStringAttribute("FOO")
+    assert o.get_as_tuple_list(key="user_name") == [("user_name", "foo")]
