Skip to content

Commit d27c50a

Browse files
JohnVillalovosJohnVillalovos
committed
chore: add get() methods for GetWithoutIdMixin based classes
Add the get() methods for the GetWithoutIdMixin based classes. Update the tests/meta/test_ensure_type_hints.py tests to check to ensure that the get methods are defined with the correct return type.
1 parent 4945353 commit d27c50a

10 files changed

+169
-28
lines changed

‎gitlab/v4/objects/appearance.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,4 @@ def update(
6161
def get(
6262
self, id: Optional[Union[int, str]] = None, **kwargs: Any
6363
) -> Optional[ApplicationAppearance]:
64-
return cast(ApplicationAppearance, super().get(id=id, **kwargs))
64+
return cast(Optional[ApplicationAppearance], super().get(id=id, **kwargs))

‎gitlab/v4/objects/export_import.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class GroupExportManager(GetWithoutIdMixin, CreateMixin, RESTManager):
2727
def get(
2828
self, id: Optional[Union[int, str]] = None, **kwargs: Any
2929
) -> Optional[GroupExport]:
30-
return cast(GroupExport, super().get(id=id, **kwargs))
30+
return cast(Optional[GroupExport], super().get(id=id, **kwargs))
3131

3232

3333
class GroupImport(RESTObject):
@@ -42,7 +42,7 @@ class GroupImportManager(GetWithoutIdMixin, RESTManager):
4242
def get(
4343
self, id: Optional[Union[int, str]] = None, **kwargs: Any
4444
) -> Optional[GroupImport]:
45-
return cast(GroupImport, super().get(id=id, **kwargs))
45+
return cast(Optional[GroupImport], super().get(id=id, **kwargs))
4646

4747

4848
class ProjectExport(DownloadMixin, RefreshMixin, RESTObject):
@@ -58,7 +58,7 @@ class ProjectExportManager(GetWithoutIdMixin, CreateMixin, RESTManager):
5858
def get(
5959
self, id: Optional[Union[int, str]] = None, **kwargs: Any
6060
) -> Optional[ProjectExport]:
61-
return cast(ProjectExport, super().get(id=id, **kwargs))
61+
return cast(Optional[ProjectExport], super().get(id=id, **kwargs))
6262

6363

6464
class ProjectImport(RefreshMixin, RESTObject):
@@ -73,4 +73,4 @@ class ProjectImportManager(GetWithoutIdMixin, RESTManager):
7373
def get(
7474
self, id: Optional[Union[int, str]] = None, **kwargs: Any
7575
) -> Optional[ProjectImport]:
76-
return cast(ProjectImport, super().get(id=id, **kwargs))
76+
return cast(Optional[ProjectImport], super().get(id=id, **kwargs))

‎gitlab/v4/objects/merge_request_approvals.py

+18-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from typing import Any, Dict, List, Optional, TYPE_CHECKING
1+
from typing import Any, cast, Dict, List, Optional, TYPE_CHECKING, Union
22

33
from gitlab import exceptions as exc
44
from gitlab.base import RequiredOptional, RESTManager, RESTObject
@@ -45,6 +45,11 @@ class ProjectApprovalManager(GetWithoutIdMixin, UpdateMixin, RESTManager):
4545
)
4646
_update_uses_post = True
4747

48+
def get(
49+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
50+
) -> Optional[ProjectApproval]:
51+
return cast(Optional[ProjectApproval], super().get(id=id, **kwargs))
52+
4853
@exc.on_http_error(exc.GitlabUpdateError)
4954
def set_approvers(
5055
self,
@@ -105,6 +110,11 @@ class ProjectMergeRequestApprovalManager(GetWithoutIdMixin, UpdateMixin, RESTMan
105110
_update_attrs = RequiredOptional(required=("approvals_required",))
106111
_update_uses_post = True
107112

113+
def get(
114+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
115+
) -> Optional[ProjectMergeRequestApproval]:
116+
return cast(Optional[ProjectMergeRequestApproval], super().get(id=id, **kwargs))
117+
108118
@exc.on_http_error(exc.GitlabUpdateError)
109119
def set_approvers(
110120
self,
@@ -241,3 +251,10 @@ class ProjectMergeRequestApprovalStateManager(GetWithoutIdMixin, RESTManager):
241251
_path = "/projects/{project_id}/merge_requests/{mr_iid}/approval_state"
242252
_obj_cls = ProjectMergeRequestApprovalState
243253
_from_parent_attrs = {"project_id": "project_id", "mr_iid": "iid"}
254+
255+
def get(
256+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
257+
) -> Optional[ProjectMergeRequestApprovalState]:
258+
return cast(
259+
Optional[ProjectMergeRequestApprovalState], super().get(id=id, **kwargs)
260+
)

‎gitlab/v4/objects/notification_settings.py

+17
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from typing import Any, cast, Optional, Union
2+
13
from gitlab.base import RequiredOptional, RESTManager, RESTObject
24
from gitlab.mixins import GetWithoutIdMixin, SaveMixin, UpdateMixin
35

@@ -36,6 +38,11 @@ class NotificationSettingsManager(GetWithoutIdMixin, UpdateMixin, RESTManager):
3638
),
3739
)
3840

41+
def get(
42+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
43+
) -> Optional[NotificationSettings]:
44+
return cast(Optional[NotificationSettings], super().get(id=id, **kwargs))
45+
3946

4047
class GroupNotificationSettings(NotificationSettings):
4148
pass
@@ -46,6 +53,11 @@ class GroupNotificationSettingsManager(NotificationSettingsManager):
4653
_obj_cls = GroupNotificationSettings
4754
_from_parent_attrs = {"group_id": "id"}
4855

56+
def get(
57+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
58+
) -> Optional[GroupNotificationSettings]:
59+
return cast(Optional[GroupNotificationSettings], super().get(id=id, **kwargs))
60+
4961

5062
class ProjectNotificationSettings(NotificationSettings):
5163
pass
@@ -55,3 +67,8 @@ class ProjectNotificationSettingsManager(NotificationSettingsManager):
5567
_path = "/projects/{project_id}/notification_settings"
5668
_obj_cls = ProjectNotificationSettings
5769
_from_parent_attrs = {"project_id": "id"}
70+
71+
def get(
72+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
73+
) -> Optional[ProjectNotificationSettings]:
74+
return cast(Optional[ProjectNotificationSettings], super().get(id=id, **kwargs))

‎gitlab/v4/objects/pipelines.py

+5
Original file line numberDiff line numberDiff line change
@@ -246,3 +246,8 @@ class ProjectPipelineTestReportManager(GetWithoutIdMixin, RESTManager):
246246
_path = "/projects/{project_id}/pipelines/{pipeline_id}/test_report"
247247
_obj_cls = ProjectPipelineTestReport
248248
_from_parent_attrs = {"project_id": "project_id", "pipeline_id": "id"}
249+
250+
def get(
251+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
252+
) -> Optional[ProjectPipelineTestReport]:
253+
return cast(Optional[ProjectPipelineTestReport], super().get(id=id, **kwargs))

‎gitlab/v4/objects/push_rules.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,4 @@ class ProjectPushRulesManager(
5454
def get(
5555
self, id: Optional[Union[int, str]] = None, **kwargs: Any
5656
) -> Optional[ProjectPushRules]:
57-
return cast(ProjectPushRules, super().get(id=id, **kwargs))
57+
return cast(Optional[ProjectPushRules], super().get(id=id, **kwargs))

‎gitlab/v4/objects/settings.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -118,4 +118,4 @@ def update(
118118
def get(
119119
self, id: Optional[Union[int, str]] = None, **kwargs: Any
120120
) -> Optional[ApplicationSettings]:
121-
return cast(ApplicationSettings, super().get(id=id, **kwargs))
121+
return cast(Optional[ApplicationSettings], super().get(id=id, **kwargs))

‎gitlab/v4/objects/statistics.py

+22
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from typing import Any, cast, Optional, Union
2+
13
from gitlab.base import RESTManager, RESTObject
24
from gitlab.mixins import GetWithoutIdMixin, RefreshMixin
35

@@ -22,6 +24,11 @@ class ProjectAdditionalStatisticsManager(GetWithoutIdMixin, RESTManager):
2224
_obj_cls = ProjectAdditionalStatistics
2325
_from_parent_attrs = {"project_id": "id"}
2426

27+
def get(
28+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
29+
) -> Optional[ProjectAdditionalStatistics]:
30+
return cast(Optional[ProjectAdditionalStatistics], super().get(id=id, **kwargs))
31+
2532

2633
class IssuesStatistics(RefreshMixin, RESTObject):
2734
_id_attr = None
@@ -31,6 +38,11 @@ class IssuesStatisticsManager(GetWithoutIdMixin, RESTManager):
3138
_path = "/issues_statistics"
3239
_obj_cls = IssuesStatistics
3340

41+
def get(
42+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
43+
) -> Optional[IssuesStatistics]:
44+
return cast(Optional[IssuesStatistics], super().get(id=id, **kwargs))
45+
3446

3547
class GroupIssuesStatistics(RefreshMixin, RESTObject):
3648
_id_attr = None
@@ -41,6 +53,11 @@ class GroupIssuesStatisticsManager(GetWithoutIdMixin, RESTManager):
4153
_obj_cls = GroupIssuesStatistics
4254
_from_parent_attrs = {"group_id": "id"}
4355

56+
def get(
57+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
58+
) -> Optional[GroupIssuesStatistics]:
59+
return cast(Optional[GroupIssuesStatistics], super().get(id=id, **kwargs))
60+
4461

4562
class ProjectIssuesStatistics(RefreshMixin, RESTObject):
4663
_id_attr = None
@@ -50,3 +67,8 @@ class ProjectIssuesStatisticsManager(GetWithoutIdMixin, RESTManager):
5067
_path = "/projects/{project_id}/issues_statistics"
5168
_obj_cls = ProjectIssuesStatistics
5269
_from_parent_attrs = {"project_id": "id"}
70+
71+
def get(
72+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
73+
) -> Optional[ProjectIssuesStatistics]:
74+
return cast(Optional[ProjectIssuesStatistics], super().get(id=id, **kwargs))

‎gitlab/v4/objects/users.py

+16-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
https://docs.gitlab.com/ee/api/users.html
44
https://docs.gitlab.com/ee/api/projects.html#list-projects-starred-by-a-user
55
"""
6-
from typing import Any, cast, Dict, List, Union
6+
from typing import Any, cast, Dict, List, Optional, Union
77

88
import requests
99

@@ -120,6 +120,11 @@ class CurrentUserStatusManager(GetWithoutIdMixin, UpdateMixin, RESTManager):
120120
_obj_cls = CurrentUserStatus
121121
_update_attrs = RequiredOptional(optional=("emoji", "message"))
122122

123+
def get(
124+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
125+
) -> Optional[CurrentUserStatus]:
126+
return cast(Optional[CurrentUserStatus], super().get(id=id, **kwargs))
127+
123128

124129
class CurrentUser(RESTObject):
125130
_id_attr = None
@@ -135,6 +140,11 @@ class CurrentUserManager(GetWithoutIdMixin, RESTManager):
135140
_path = "/user"
136141
_obj_cls = CurrentUser
137142

143+
def get(
144+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
145+
) -> Optional[CurrentUser]:
146+
return cast(Optional[CurrentUser], super().get(id=id, **kwargs))
147+
138148

139149
class User(SaveMixin, ObjectDeleteMixin, RESTObject):
140150
_short_print_attr = "username"
@@ -390,6 +400,11 @@ class UserStatusManager(GetWithoutIdMixin, RESTManager):
390400
_obj_cls = UserStatus
391401
_from_parent_attrs = {"user_id": "id"}
392402

403+
def get(
404+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
405+
) -> Optional[UserStatus]:
406+
return cast(Optional[UserStatus], super().get(id=id, **kwargs))
407+
393408

394409
class UserActivitiesManager(ListMixin, RESTManager):
395410
_path = "/user/activities"

‎tests/meta/test_ensure_type_hints.py

+84-19
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,34 @@
44
Original notes by John L. Villalovos
55
66
"""
7+
import dataclasses
8+
import functools
79
import inspect
8-
from typing import Tuple, Type
10+
from typing import Optional, Type
911

1012
import _pytest
1113

1214
import gitlab.mixins
1315
import gitlab.v4.objects
1416

1517

18+
@functools.total_ordering
19+
@dataclasses.dataclass(frozen=True)
20+
class ClassInfo:
21+
name: str
22+
type: Type
23+
24+
def __lt__(self, other: object) -> bool:
25+
if not isinstance(other, ClassInfo):
26+
return NotImplemented
27+
return (self.type.__module__, self.name) < (other.type.__module__, other.name)
28+
29+
def __eq__(self, other: object) -> bool:
30+
if not isinstance(other, ClassInfo):
31+
return NotImplemented
32+
return (self.type.__module__, self.name) == (other.type.__module__, other.name)
33+
34+
1635
def pytest_generate_tests(metafunc: _pytest.python.Metafunc) -> None:
1736
"""Find all of the classes in gitlab.v4.objects and pass them to our test
1837
function"""
@@ -35,38 +54,84 @@ def pytest_generate_tests(metafunc: _pytest.python.Metafunc) -> None:
3554
if not class_name.endswith("Manager"):
3655
continue
3756

38-
class_info_set.add((class_name, class_value))
57+
class_info_set.add(ClassInfo(name=class_name, type=class_value))
58+
59+
metafunc.parametrize("class_info", sorted(class_info_set))
3960

40-
metafunc.parametrize("class_info", class_info_set)
61+
62+
GET_ID_METHOD_TEMPLATE = """
63+
def get(
64+
self, id: Union[str, int], lazy: bool = False, **kwargs: Any
65+
) -> {obj_cls.__name__}:
66+
return cast({obj_cls.__name__}, super().get(id=id, lazy=lazy, **kwargs))
67+
68+
You may also need to add the following imports:
69+
from typing import Any, cast, Union"
70+
"""
71+
72+
GET_WITHOUT_ID_METHOD_TEMPLATE = """
73+
def get(
74+
self, id: Optional[Union[int, str]] = None, **kwargs: Any
75+
) -> Optional[{obj_cls.__name__}]:
76+
return cast(Optional[{obj_cls.__name__}], super().get(id=id, **kwargs))
77+
78+
You may also need to add the following imports:
79+
from typing import Any, cast, Optional, Union"
80+
"""
4181

4282

4383
class TestTypeHints:
44-
def test_check_get_function_type_hints(self, class_info: Tuple[str, Type]) -> None:
84+
def test_check_get_function_type_hints(self, class_info: ClassInfo) -> None:
4585
"""Ensure classes derived from GetMixin have defined a 'get()' method with
4686
correct type-hints.
4787
"""
48-
class_name, class_value = class_info
49-
if not class_name.endswith("Manager"):
50-
return
88+
self.get_check_helper(
89+
base_type=gitlab.mixins.GetMixin,
90+
class_info=class_info,
91+
method_template=GET_ID_METHOD_TEMPLATE,
92+
optional_return=False,
93+
)
5194

52-
mro = class_value.mro()
95+
def test_check_get_without_id_function_type_hints(
96+
self, class_info: ClassInfo
97+
) -> None:
98+
"""Ensure classes derived from GetMixin have defined a 'get()' method with
99+
correct type-hints.
100+
"""
101+
self.get_check_helper(
102+
base_type=gitlab.mixins.GetWithoutIdMixin,
103+
class_info=class_info,
104+
method_template=GET_WITHOUT_ID_METHOD_TEMPLATE,
105+
optional_return=True,
106+
)
107+
108+
def get_check_helper(
109+
self,
110+
*,
111+
base_type: Type,
112+
class_info: ClassInfo,
113+
method_template: str,
114+
optional_return: bool,
115+
) -> None:
116+
if not class_info.name.endswith("Manager"):
117+
return
118+
mro = class_info.type.mro()
53119
# The class needs to be derived from GetMixin or we ignore it
54-
if gitlab.mixins.GetMixin not in mro:
120+
if base_type not in mro:
55121
return
56122

57-
obj_cls = class_value._obj_cls
58-
signature = inspect.signature(class_value.get)
59-
filename = inspect.getfile(class_value)
123+
obj_cls = class_info.type._obj_cls
124+
signature = inspect.signature(class_info.type.get)
125+
filename = inspect.getfile(class_info.type)
60126

61127
fail_message = (
62-
f"class definition for {class_name!r} in file {filename!r} "
128+
f"class definition for {class_info.name!r} in file {filename!r} "
63129
f"must have defined a 'get' method with a return annotation of "
64130
f"{obj_cls} but found {signature.return_annotation}\n"
65131
f"Recommend adding the followinng method:\n"
66-
f"def get(\n"
67-
f" self, id: Union[str, int], lazy: bool = False, **kwargs: Any\n"
68-
f" ) -> {obj_cls.__name__}:\n"
69-
f" return cast({obj_cls.__name__}, super().get(id=id, lazy=lazy, "
70-
f"**kwargs))\n"
71132
)
72-
assert obj_cls == signature.return_annotation, fail_message
133+
fail_message += method_template.format(obj_cls=obj_cls)
134+
check_type = obj_cls
135+
if optional_return:
136+
check_type = Optional[obj_cls]
137+
assert check_type == signature.return_annotation, fail_message

0 commit comments

Comments
 (0)