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

JohnVillalovos · JohnVillalovos · commit a939db136573 · 2022-07-24T09:14:08.000-07: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
@@ -28,6 +29,7 @@
 import gitlab.config
 import gitlab.const
 import gitlab.exceptions
+from gitlab import types as gl_types
 from gitlab import utils
 
 REDIRECT_MSG = (
@@ -626,6 +628,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,
@@ -688,6 +712,10 @@ def http_request(
         else:
             utils.copy_dict(src=kwargs, dest=params)
 
+        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")
@@ -710,7 +738,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/types.py b/gitlab/types.py
@@ -66,6 +66,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."""
@@ -90,17 +93,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):
     @staticmethod
diff --git a/gitlab/utils.py b/gitlab/utils.py
@@ -77,12 +77,13 @@ def _transform_types(
 
         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))
+        # if the type is FileAttribute we need to pass the data as file
+        if isinstance(type_obj, types.FileAttribute):
+            if transform_files:
+                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()
+            data[attr_name] = type_obj
 
     return data, files
 
@@ -94,6 +95,8 @@ def copy_dict(
 ) -> 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
@@ -99,6 +99,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 user not in group1.members.list()
     assert group1.members_all.list()
diff --git a/tests/unit/test_gitlab_http_methods.py b/tests/unit/test_gitlab_http_methods.py
@@ -5,6 +5,7 @@
 import requests
 import responses
 
+import gitlab
 from gitlab import GitlabHttpError, GitlabList, GitlabParsingError, RedirectError
 from gitlab.client import RETRYABLE_TRANSIENT_ERROR_CODES
 from tests.unit import helpers
@@ -742,3 +743,20 @@ def test_delete_request_404(gl):
     with pytest.raises(GitlabHttpError):
         gl.http_delete("/not_there")
     assert responses.assert_call_count(url, 1) is True
+
+
+@responses.activate
+def test_array_type_request(gl):
+    url = "http://localhost/api/v4/projects"
+    params = "array_var[]=1&array_var[]=2&array_var[]=3"
+    full_url = f"{url}?array_var%5B%5D=1&array_var%5B%5D=2&array_var%5B%5D=3"
+    responses.add(
+        method=responses.GET,
+        url=url,
+        json={"name": "project1"},
+        status=200,
+        match=[responses.matchers.query_string_matcher(params)],
+    )
+
+    gl.http_get("/projects", array_var=gitlab.types.ArrayAttribute([1, 2, 3]))
+    assert responses.assert_call_count(full_url, 1) is True
diff --git a/tests/unit/test_types.py b/tests/unit/test_types.py
@@ -74,6 +74,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
@@ -113,6 +114,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"
@@ -135,7 +154,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")]
