Skip to content

Commit 76b04a8

Browse files
JohnVillalovosJohnVillalovos
committed
fix: use the [] after key names for array variables
This is not a complete fix. We need to re-architect things so that for the 'params' parameter passed into `requests.request()` we use a list of tuples. The reason for this is then we can pass in items which contain the same key but different values. For example we want to be able to pass [('assigned_ids[]', 1), ('assigned_ids[]', 2)] which is not something we can do with a dictionary. 1. Create a new CsvStringAttribute class. This is to indicate types which are sent to the GitLab server as comma-separated-strings (CSV) but we have been allowing users to use a list-of-strings. These values are NOT array values, so adding [] to the key name breaks them. 2. Rename ListAttribute to ArrayAttribute. 3. If a value is of type ArrayAttribute then append '[]' to the name of the value.
1 parent 2cd15ac commit 76b04a8

File tree

14 files changed

+100
-48
lines changed

14 files changed

+100
-48
lines changed

‎gitlab/mixins.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,17 @@ def list(self, **kwargs: Any) -> Union[base.RESTObjectList, List[base.RESTObject
231231
for attr_name, type_cls in self._types.items():
232232
if attr_name in data.keys():
233233
type_obj = type_cls(data[attr_name])
234-
data[attr_name] = type_obj.get_for_api()
234+
if isinstance(type_obj, g_types.ArrayAttribute):
235+
# NOTE(jlvillal): Array values are supposed to be
236+
# passed in the form of 'key[]=value1&key[]=value2'
237+
# https://docs.gitlab.com/ee/api/#array
238+
# But at the moment we haven't determined an elegant
239+
# way to do that. So for now just add the '[]' to the
240+
# key we send in the request.
241+
del data[attr_name]
242+
data[f"{attr_name}[]"] = type_obj.get_for_api()
243+
else:
244+
data[attr_name] = type_obj.get_for_api()
235245

236246
# Allow to overwrite the path, handy for custom listings
237247
path = data.pop("path", self.path)
@@ -313,6 +323,15 @@ def create(
313323
if isinstance(type_obj, g_types.FileAttribute):
314324
k = type_obj.get_file_name(attr_name)
315325
files[attr_name] = (k, data.pop(attr_name))
326+
elif isinstance(type_obj, g_types.ArrayAttribute):
327+
# NOTE(jlvillal): Array values are supposed to be
328+
# passed in the form of 'key[]=value1&key[]=value2'
329+
# https://docs.gitlab.com/ee/api/#array
330+
# But at the moment we haven't determined an elegant
331+
# way to do that. So for now just add the '[]' to the
332+
# key we send in the request.
333+
del data[attr_name]
334+
data[f"{attr_name}[]"] = type_obj.get_for_api()
316335
else:
317336
data[attr_name] = type_obj.get_for_api()
318337

@@ -409,6 +428,15 @@ def update(
409428
if isinstance(type_obj, g_types.FileAttribute):
410429
k = type_obj.get_file_name(attr_name)
411430
files[attr_name] = (k, new_data.pop(attr_name))
431+
elif isinstance(type_obj, g_types.ArrayAttribute):
432+
# NOTE(jlvillal): Array values are supposed to be
433+
# passed in the form of 'key[]=value1&key[]=value2'
434+
# https://docs.gitlab.com/ee/api/#array
435+
# But at the moment we haven't determined an elegant
436+
# way to do that. So for now just add the '[]' to the
437+
# key we send in the request.
438+
del new_data[attr_name]
439+
new_data[f"{attr_name}[]"] = type_obj.get_for_api()
412440
else:
413441
new_data[attr_name] = type_obj.get_for_api()
414442

‎gitlab/types.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,31 @@ def get_for_api(self) -> Any:
3232
return self._value
3333

3434

35-
class ListAttribute(GitlabAttribute):
35+
class ArrayAttribute(GitlabAttribute):
36+
"""To support `array` types as documented in
37+
https://docs.gitlab.com/ee/api/#array"""
38+
39+
def set_from_cli(self, cli_value: str) -> None:
40+
if not cli_value.strip():
41+
self._value = []
42+
else:
43+
self._value = [item.strip() for item in cli_value.split(",")]
44+
45+
def get_for_api(self) -> str:
46+
# Do not comma-split single value passed as string
47+
if isinstance(self._value, str):
48+
return self._value
49+
50+
if TYPE_CHECKING:
51+
assert isinstance(self._value, list)
52+
return ",".join([str(x) for x in self._value])
53+
54+
55+
class CsvStringAttribute(GitlabAttribute):
56+
"""For values which are sent to the server as a Comma Separated Values
57+
(CSV) string. We allow them to be specified as a list and we convert it
58+
into a CSV"""
59+
3660
def set_from_cli(self, cli_value: str) -> None:
3761
if not cli_value.strip():
3862
self._value = []

‎gitlab/v4/objects/deploy_tokens.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class GroupDeployTokenManager(ListMixin, CreateMixin, DeleteMixin, RESTManager):
3939
"username",
4040
),
4141
)
42-
_types = {"scopes": types.ListAttribute}
42+
_types = {"scopes": types.CsvStringAttribute}
4343

4444

4545
class ProjectDeployToken(ObjectDeleteMixin, RESTObject):
@@ -60,4 +60,4 @@ class ProjectDeployTokenManager(ListMixin, CreateMixin, DeleteMixin, RESTManager
6060
"username",
6161
),
6262
)
63-
_types = {"scopes": types.ListAttribute}
63+
_types = {"scopes": types.CsvStringAttribute}

‎gitlab/v4/objects/epics.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class GroupEpicManager(CRUDMixin, RESTManager):
4040
_update_attrs = RequiredOptional(
4141
optional=("title", "labels", "description", "start_date", "end_date"),
4242
)
43-
_types = {"labels": types.ListAttribute}
43+
_types = {"labels": types.CsvStringAttribute}
4444

4545

4646
class GroupEpicIssue(ObjectDeleteMixin, SaveMixin, RESTObject):

‎gitlab/v4/objects/groups.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ class GroupManager(CRUDMixin, RESTManager):
290290
"shared_runners_setting",
291291
),
292292
)
293-
_types = {"avatar": types.ImageAttribute, "skip_groups": types.ListAttribute}
293+
_types = {"avatar": types.ImageAttribute, "skip_groups": types.ArrayAttribute}
294294

295295
def get(self, id: Union[str, int], lazy: bool = False, **kwargs: Any) -> Group:
296296
return cast(Group, super().get(id=id, lazy=lazy, **kwargs))
@@ -350,7 +350,7 @@ class GroupSubgroupManager(ListMixin, RESTManager):
350350
"with_custom_attributes",
351351
"min_access_level",
352352
)
353-
_types = {"skip_groups": types.ListAttribute}
353+
_types = {"skip_groups": types.ArrayAttribute}
354354

355355

356356
class GroupDescendantGroup(RESTObject):

‎gitlab/v4/objects/issues.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ class IssueManager(RetrieveMixin, RESTManager):
6363
"updated_after",
6464
"updated_before",
6565
)
66-
_types = {"iids": types.ListAttribute, "labels": types.ListAttribute}
66+
_types = {"iids": types.ArrayAttribute, "labels": types.CsvStringAttribute}
6767

6868

6969
class GroupIssue(RESTObject):
@@ -90,7 +90,7 @@ class GroupIssueManager(ListMixin, RESTManager):
9090
"updated_after",
9191
"updated_before",
9292
)
93-
_types = {"iids": types.ListAttribute, "labels": types.ListAttribute}
93+
_types = {"iids": types.ArrayAttribute, "labels": types.CsvStringAttribute}
9494

9595

9696
class ProjectIssue(
@@ -220,7 +220,7 @@ class ProjectIssueManager(CRUDMixin, RESTManager):
220220
"discussion_locked",
221221
),
222222
)
223-
_types = {"iids": types.ListAttribute, "labels": types.ListAttribute}
223+
_types = {"iids": types.ArrayAttribute, "labels": types.CsvStringAttribute}
224224

225225

226226
class ProjectIssueLink(ObjectDeleteMixin, RESTObject):

‎gitlab/v4/objects/members.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class GroupMemberManager(CRUDMixin, RESTManager):
3939
_update_attrs = RequiredOptional(
4040
required=("access_level",), optional=("expires_at",)
4141
)
42-
_types = {"user_ids": types.ListAttribute}
42+
_types = {"user_ids": types.ArrayAttribute}
4343

4444
def get(
4545
self, id: Union[str, int], lazy: bool = False, **kwargs: Any
@@ -95,7 +95,7 @@ class ProjectMemberManager(CRUDMixin, RESTManager):
9595
_update_attrs = RequiredOptional(
9696
required=("access_level",), optional=("expires_at",)
9797
)
98-
_types = {"user_ids": types.ListAttribute}
98+
_types = {"user_ids": types.ArrayAttribute}
9999

100100
def get(
101101
self, id: Union[str, int], lazy: bool = False, **kwargs: Any

‎gitlab/v4/objects/merge_requests.py

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,10 @@ class MergeRequestManager(ListMixin, RESTManager):
9595
"deployed_after",
9696
)
9797
_types = {
98-
"approver_ids": types.ListAttribute,
99-
"approved_by_ids": types.ListAttribute,
100-
"in": types.ListAttribute,
101-
"labels": types.ListAttribute,
98+
"approver_ids": types.ArrayAttribute,
99+
"approved_by_ids": types.ArrayAttribute,
100+
"in": types.CsvStringAttribute,
101+
"labels": types.CsvStringAttribute,
102102
}
103103

104104

@@ -133,9 +133,9 @@ class GroupMergeRequestManager(ListMixin, RESTManager):
133133
"wip",
134134
)
135135
_types = {
136-
"approver_ids": types.ListAttribute,
137-
"approved_by_ids": types.ListAttribute,
138-
"labels": types.ListAttribute,
136+
"approver_ids": types.ArrayAttribute,
137+
"approved_by_ids": types.ArrayAttribute,
138+
"labels": types.CsvStringAttribute,
139139
}
140140

141141

@@ -450,10 +450,10 @@ class ProjectMergeRequestManager(CRUDMixin, RESTManager):
450450
"wip",
451451
)
452452
_types = {
453-
"approver_ids": types.ListAttribute,
454-
"approved_by_ids": types.ListAttribute,
455-
"iids": types.ListAttribute,
456-
"labels": types.ListAttribute,
453+
"approver_ids": types.ArrayAttribute,
454+
"approved_by_ids": types.ArrayAttribute,
455+
"iids": types.ArrayAttribute,
456+
"labels": types.CsvStringAttribute,
457457
}
458458

459459
def get(

‎gitlab/v4/objects/milestones.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class GroupMilestoneManager(CRUDMixin, RESTManager):
8989
optional=("title", "description", "due_date", "start_date", "state_event"),
9090
)
9191
_list_filters = ("iids", "state", "search")
92-
_types = {"iids": types.ListAttribute}
92+
_types = {"iids": types.ArrayAttribute}
9393

9494

9595
class ProjectMilestone(PromoteMixin, SaveMixin, ObjectDeleteMixin, RESTObject):
@@ -164,4 +164,4 @@ class ProjectMilestoneManager(CRUDMixin, RESTManager):
164164
optional=("title", "description", "due_date", "start_date", "state_event"),
165165
)
166166
_list_filters = ("iids", "state", "search")
167-
_types = {"iids": types.ListAttribute}
167+
_types = {"iids": types.ArrayAttribute}

‎gitlab/v4/objects/projects.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ class ProjectManager(CRUDMixin, RESTManager):
771771
"with_merge_requests_enabled",
772772
"with_programming_language",
773773
)
774-
_types = {"avatar": types.ImageAttribute, "topic": types.ListAttribute}
774+
_types = {"avatar": types.ImageAttribute, "topic": types.CsvStringAttribute}
775775

776776
def get(self, id: Union[str, int], lazy: bool = False, **kwargs: Any) -> Project:
777777
return cast(Project, super().get(id=id, lazy=lazy, **kwargs))

‎gitlab/v4/objects/runners.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ class RunnerManager(CRUDMixin, RESTManager):
6868
),
6969
)
7070
_list_filters = ("scope", "tag_list")
71-
_types = {"tag_list": types.ListAttribute}
71+
_types = {"tag_list": types.CsvStringAttribute}
7272

7373
@cli.register_custom_action("RunnerManager", tuple(), ("scope",))
7474
@exc.on_http_error(exc.GitlabListError)
@@ -130,7 +130,7 @@ class GroupRunnerManager(ListMixin, RESTManager):
130130
_from_parent_attrs = {"group_id": "id"}
131131
_create_attrs = RequiredOptional(required=("runner_id",))
132132
_list_filters = ("scope", "tag_list")
133-
_types = {"tag_list": types.ListAttribute}
133+
_types = {"tag_list": types.CsvStringAttribute}
134134

135135

136136
class ProjectRunner(ObjectDeleteMixin, RESTObject):
@@ -143,4 +143,4 @@ class ProjectRunnerManager(CreateMixin, DeleteMixin, ListMixin, RESTManager):
143143
_from_parent_attrs = {"project_id": "id"}
144144
_create_attrs = RequiredOptional(required=("runner_id",))
145145
_list_filters = ("scope", "tag_list")
146-
_types = {"tag_list": types.ListAttribute}
146+
_types = {"tag_list": types.CsvStringAttribute}

‎gitlab/v4/objects/settings.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,12 @@ class ApplicationSettingsManager(GetWithoutIdMixin, UpdateMixin, RESTManager):
8080
),
8181
)
8282
_types = {
83-
"asset_proxy_allowlist": types.ListAttribute,
84-
"disabled_oauth_sign_in_sources": types.ListAttribute,
85-
"domain_allowlist": types.ListAttribute,
86-
"domain_denylist": types.ListAttribute,
87-
"import_sources": types.ListAttribute,
88-
"restricted_visibility_levels": types.ListAttribute,
83+
"asset_proxy_allowlist": types.ArrayAttribute,
84+
"disabled_oauth_sign_in_sources": types.ArrayAttribute,
85+
"domain_allowlist": types.ArrayAttribute,
86+
"domain_denylist": types.ArrayAttribute,
87+
"import_sources": types.ArrayAttribute,
88+
"restricted_visibility_levels": types.ArrayAttribute,
8989
}
9090

9191
@exc.on_http_error(exc.GitlabUpdateError)

‎gitlab/v4/objects/users.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ class ProjectUserManager(ListMixin, RESTManager):
359359
_obj_cls = ProjectUser
360360
_from_parent_attrs = {"project_id": "id"}
361361
_list_filters = ("search", "skip_users")
362-
_types = {"skip_users": types.ListAttribute}
362+
_types = {"skip_users": types.ArrayAttribute}
363363

364364

365365
class UserEmail(ObjectDeleteMixin, RESTObject):

‎tests/unit/test_types.py

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,42 +30,42 @@ def test_gitlab_attribute_get():
3030
assert o._value is None
3131

3232

33-
def test_list_attribute_input():
34-
o = types.ListAttribute()
33+
def test_array_attribute_input():
34+
o = types.ArrayAttribute()
3535
o.set_from_cli("foo,bar,baz")
3636
assert o.get() == ["foo", "bar", "baz"]
3737

3838
o.set_from_cli("foo")
3939
assert o.get() == ["foo"]
4040

4141

42-
def test_list_attribute_empty_input():
43-
o = types.ListAttribute()
42+
def test_array_attribute_empty_input():
43+
o = types.ArrayAttribute()
4444
o.set_from_cli("")
4545
assert o.get() == []
4646

4747
o.set_from_cli(" ")
4848
assert o.get() == []
4949

5050

51-
def test_list_attribute_get_for_api_from_cli():
52-
o = types.ListAttribute()
51+
def test_array_attribute_get_for_api_from_cli():
52+
o = types.ArrayAttribute()
5353
o.set_from_cli("foo,bar,baz")
5454
assert o.get_for_api() == "foo,bar,baz"
5555

5656

57-
def test_list_attribute_get_for_api_from_list():
58-
o = types.ListAttribute(["foo", "bar", "baz"])
57+
def test_array_attribute_get_for_api_from_list():
58+
o = types.ArrayAttribute(["foo", "bar", "baz"])
5959
assert o.get_for_api() == "foo,bar,baz"
6060

6161

62-
def test_list_attribute_get_for_api_from_int_list():
63-
o = types.ListAttribute([1, 9, 7])
62+
def test_array_attribute_get_for_api_from_int_list():
63+
o = types.ArrayAttribute([1, 9, 7])
6464
assert o.get_for_api() == "1,9,7"
6565

6666

67-
def test_list_attribute_does_not_split_string():
68-
o = types.ListAttribute("foo")
67+
def test_array_attribute_does_not_split_string():
68+
o = types.ArrayAttribute("foo")
6969
assert o.get_for_api() == "foo"
7070

7171

0 commit comments

Comments
 (0)