From d290a21d02d7ecace36cb095425d9242ad4bf375 Mon Sep 17 00:00:00 2001 From: Igor Ponomarev Date: Thu, 6 Feb 2025 11:30:33 +0000 Subject: [PATCH] feat(api)!: Make RESTObjectList typing generic BREAKING CHANGE: Type narrowing of `list()` methods return objects from RESTObject to a concrete subclass (for example `MergeRequest`) can become redundant. Currently the RESTObjectList type hints yielded objects as base RESTObject. However, the ListMixin is now generic and will return the RESTObject subclass based on the RESTManager typing. Using `typing.Generic` it is possible to make RESTObjectList type hint a specific subclass of the RESTObject. Iterating over `list()` call the ListMixin will now yield the same object class because both `list` and `RESTObjectList` will have the same type hinted subclass. Signed-off-by: Igor Ponomarev --- gitlab/base.py | 16 ++++++++-------- gitlab/mixins.py | 4 +++- gitlab/v4/cli.py | 7 ++++++- gitlab/v4/objects/ldap.py | 2 +- gitlab/v4/objects/merge_requests.py | 6 +++--- gitlab/v4/objects/milestones.py | 22 +++++++++++++--------- gitlab/v4/objects/snippets.py | 6 +++--- gitlab/v4/objects/users.py | 2 +- tests/functional/conftest.py | 12 ++++++------ 9 files changed, 44 insertions(+), 33 deletions(-) diff --git a/gitlab/base.py b/gitlab/base.py index 5b868761e..1ee0051c9 100644 --- a/gitlab/base.py +++ b/gitlab/base.py @@ -252,7 +252,10 @@ def encoded_id(self) -> int | str | None: return obj_id -class RESTObjectList: +TObjCls = TypeVar("TObjCls", bound=RESTObject) + + +class RESTObjectList(Generic[TObjCls]): """Generator object representing a list of RESTObject's. This generator uses the Gitlab pagination system to fetch new data when @@ -268,7 +271,7 @@ class RESTObjectList: """ def __init__( - self, manager: RESTManager[Any], obj_cls: type[RESTObject], _list: GitlabList + self, manager: RESTManager[TObjCls], obj_cls: type[TObjCls], _list: GitlabList ) -> None: """Creates an objects list from a GitlabList. @@ -284,16 +287,16 @@ def __init__( self._obj_cls = obj_cls self._list = _list - def __iter__(self) -> RESTObjectList: + def __iter__(self) -> RESTObjectList[TObjCls]: return self def __len__(self) -> int: return len(self._list) - def __next__(self) -> RESTObject: + def __next__(self) -> TObjCls: return self.next() - def next(self) -> RESTObject: + def next(self) -> TObjCls: data = self._list.next() return self._obj_cls(self.manager, data, created_from_list=True) @@ -334,9 +337,6 @@ def total(self) -> int | None: return self._list.total -TObjCls = TypeVar("TObjCls", bound=RESTObject) - - class RESTManager(Generic[TObjCls]): """Base class for CRUD operations on objects. diff --git a/gitlab/mixins.py b/gitlab/mixins.py index 8e0c3075d..4d9d37f66 100644 --- a/gitlab/mixins.py +++ b/gitlab/mixins.py @@ -162,7 +162,9 @@ class ListMixin(HeadMixin[base.TObjCls]): _list_filters: tuple[str, ...] = () @exc.on_http_error(exc.GitlabListError) - def list(self, **kwargs: Any) -> base.RESTObjectList | list[base.TObjCls]: + def list( + self, **kwargs: Any + ) -> base.RESTObjectList[base.TObjCls] | list[base.TObjCls]: """Retrieve a list of objects. Args: diff --git a/gitlab/v4/cli.py b/gitlab/v4/cli.py index 974b3c395..69ebf734b 100644 --- a/gitlab/v4/cli.py +++ b/gitlab/v4/cli.py @@ -133,7 +133,12 @@ def do_create(self) -> gitlab.base.RESTObject: cli.die("Impossible to create object", e) return result - def do_list(self) -> gitlab.base.RESTObjectList | list[gitlab.base.RESTObject]: + def do_list( + self, + ) -> ( + gitlab.base.RESTObjectList[gitlab.base.RESTObject] + | list[gitlab.base.RESTObject] + ): if TYPE_CHECKING: assert isinstance(self.mgr, gitlab.mixins.ListMixin) message_details = gitlab.utils.WarnMessageData( diff --git a/gitlab/v4/objects/ldap.py b/gitlab/v4/objects/ldap.py index a00705893..5f11de59a 100644 --- a/gitlab/v4/objects/ldap.py +++ b/gitlab/v4/objects/ldap.py @@ -18,7 +18,7 @@ class LDAPGroupManager(RESTManager[LDAPGroup]): _list_filters = ("search", "provider") @exc.on_http_error(exc.GitlabListError) - def list(self, **kwargs: Any) -> list[LDAPGroup] | RESTObjectList: + def list(self, **kwargs: Any) -> list[LDAPGroup] | RESTObjectList[LDAPGroup]: """Retrieve a list of objects. Args: diff --git a/gitlab/v4/objects/merge_requests.py b/gitlab/v4/objects/merge_requests.py index 8da50922f..b73ef0e7b 100644 --- a/gitlab/v4/objects/merge_requests.py +++ b/gitlab/v4/objects/merge_requests.py @@ -203,7 +203,7 @@ def cancel_merge_when_pipeline_succeeds(self, **kwargs: Any) -> dict[str, str]: @cli.register_custom_action(cls_names="ProjectMergeRequest") @exc.on_http_error(exc.GitlabListError) - def related_issues(self, **kwargs: Any) -> RESTObjectList: + def related_issues(self, **kwargs: Any) -> RESTObjectList[ProjectIssue]: """List issues related to this merge request." Args: @@ -232,7 +232,7 @@ def related_issues(self, **kwargs: Any) -> RESTObjectList: @cli.register_custom_action(cls_names="ProjectMergeRequest") @exc.on_http_error(exc.GitlabListError) - def closes_issues(self, **kwargs: Any) -> RESTObjectList: + def closes_issues(self, **kwargs: Any) -> RESTObjectList[ProjectIssue]: """List issues that will close on merge." Args: @@ -257,7 +257,7 @@ def closes_issues(self, **kwargs: Any) -> RESTObjectList: @cli.register_custom_action(cls_names="ProjectMergeRequest") @exc.on_http_error(exc.GitlabListError) - def commits(self, **kwargs: Any) -> RESTObjectList: + def commits(self, **kwargs: Any) -> RESTObjectList[ProjectCommit]: """List the merge request commits. Args: diff --git a/gitlab/v4/objects/milestones.py b/gitlab/v4/objects/milestones.py index d6669796f..7cbb6e6eb 100644 --- a/gitlab/v4/objects/milestones.py +++ b/gitlab/v4/objects/milestones.py @@ -4,6 +4,7 @@ from gitlab import exceptions as exc from gitlab import types from gitlab.base import RESTObject, RESTObjectList +from gitlab.client import GitlabList from gitlab.mixins import ( CRUDMixin, ObjectDeleteMixin, @@ -16,6 +17,7 @@ from .issues import GroupIssue, GroupIssueManager, ProjectIssue, ProjectIssueManager from .merge_requests import ( GroupMergeRequest, + GroupMergeRequestManager, ProjectMergeRequest, ProjectMergeRequestManager, ) @@ -33,7 +35,7 @@ class GroupMilestone(SaveMixin, ObjectDeleteMixin, RESTObject): @cli.register_custom_action(cls_names="GroupMilestone") @exc.on_http_error(exc.GitlabListError) - def issues(self, **kwargs: Any) -> RESTObjectList: + def issues(self, **kwargs: Any) -> RESTObjectList[GroupIssue]: """List issues related to this milestone. Args: @@ -53,14 +55,14 @@ def issues(self, **kwargs: Any) -> RESTObjectList: path = f"{self.manager.path}/{self.encoded_id}/issues" data_list = self.manager.gitlab.http_list(path, iterator=True, **kwargs) if TYPE_CHECKING: - assert isinstance(data_list, RESTObjectList) + assert isinstance(data_list, GitlabList) manager = GroupIssueManager(self.manager.gitlab, parent=self.manager._parent) # FIXME(gpocentek): the computed manager path is not correct return RESTObjectList(manager, GroupIssue, data_list) @cli.register_custom_action(cls_names="GroupMilestone") @exc.on_http_error(exc.GitlabListError) - def merge_requests(self, **kwargs: Any) -> RESTObjectList: + def merge_requests(self, **kwargs: Any) -> RESTObjectList[GroupMergeRequest]: """List the merge requests related to this milestone. Args: @@ -79,8 +81,10 @@ def merge_requests(self, **kwargs: Any) -> RESTObjectList: path = f"{self.manager.path}/{self.encoded_id}/merge_requests" data_list = self.manager.gitlab.http_list(path, iterator=True, **kwargs) if TYPE_CHECKING: - assert isinstance(data_list, RESTObjectList) - manager = GroupIssueManager(self.manager.gitlab, parent=self.manager._parent) + assert isinstance(data_list, GitlabList) + manager = GroupMergeRequestManager( + self.manager.gitlab, parent=self.manager._parent + ) # FIXME(gpocentek): the computed manager path is not correct return RESTObjectList(manager, GroupMergeRequest, data_list) @@ -105,7 +109,7 @@ class ProjectMilestone(PromoteMixin, SaveMixin, ObjectDeleteMixin, RESTObject): @cli.register_custom_action(cls_names="ProjectMilestone") @exc.on_http_error(exc.GitlabListError) - def issues(self, **kwargs: Any) -> RESTObjectList: + def issues(self, **kwargs: Any) -> RESTObjectList[ProjectIssue]: """List issues related to this milestone. Args: @@ -125,14 +129,14 @@ def issues(self, **kwargs: Any) -> RESTObjectList: path = f"{self.manager.path}/{self.encoded_id}/issues" data_list = self.manager.gitlab.http_list(path, iterator=True, **kwargs) if TYPE_CHECKING: - assert isinstance(data_list, RESTObjectList) + assert isinstance(data_list, GitlabList) manager = ProjectIssueManager(self.manager.gitlab, parent=self.manager._parent) # FIXME(gpocentek): the computed manager path is not correct return RESTObjectList(manager, ProjectIssue, data_list) @cli.register_custom_action(cls_names="ProjectMilestone") @exc.on_http_error(exc.GitlabListError) - def merge_requests(self, **kwargs: Any) -> RESTObjectList: + def merge_requests(self, **kwargs: Any) -> RESTObjectList[ProjectMergeRequest]: """List the merge requests related to this milestone. Args: @@ -151,7 +155,7 @@ def merge_requests(self, **kwargs: Any) -> RESTObjectList: path = f"{self.manager.path}/{self.encoded_id}/merge_requests" data_list = self.manager.gitlab.http_list(path, iterator=True, **kwargs) if TYPE_CHECKING: - assert isinstance(data_list, RESTObjectList) + assert isinstance(data_list, GitlabList) manager = ProjectMergeRequestManager( self.manager.gitlab, parent=self.manager._parent ) diff --git a/gitlab/v4/objects/snippets.py b/gitlab/v4/objects/snippets.py index fb01720f3..ffada66d6 100644 --- a/gitlab/v4/objects/snippets.py +++ b/gitlab/v4/objects/snippets.py @@ -109,7 +109,7 @@ class SnippetManager(CRUDMixin[Snippet]): ) @cli.register_custom_action(cls_names="SnippetManager") - def list_public(self, **kwargs: Any) -> RESTObjectList | list[Snippet]: + def list_public(self, **kwargs: Any) -> RESTObjectList[Snippet] | list[Snippet]: """List all public snippets. Args: @@ -129,7 +129,7 @@ def list_public(self, **kwargs: Any) -> RESTObjectList | list[Snippet]: return self.list(path="/snippets/public", **kwargs) @cli.register_custom_action(cls_names="SnippetManager") - def list_all(self, **kwargs: Any) -> RESTObjectList | list[Snippet]: + def list_all(self, **kwargs: Any) -> RESTObjectList[Snippet] | list[Snippet]: """List all snippets. Args: @@ -148,7 +148,7 @@ def list_all(self, **kwargs: Any) -> RESTObjectList | list[Snippet]: """ return self.list(path="/snippets/all", **kwargs) - def public(self, **kwargs: Any) -> RESTObjectList | list[Snippet]: + def public(self, **kwargs: Any) -> RESTObjectList[Snippet] | list[Snippet]: """List all public snippets. Args: diff --git a/gitlab/v4/objects/users.py b/gitlab/v4/objects/users.py index 9dd7648da..775b00599 100644 --- a/gitlab/v4/objects/users.py +++ b/gitlab/v4/objects/users.py @@ -623,7 +623,7 @@ class UserProjectManager(ListMixin[UserProject], CreateMixin[UserProject]): "id_before", ) - def list(self, **kwargs: Any) -> RESTObjectList | list[UserProject]: + def list(self, **kwargs: Any) -> RESTObjectList[UserProject] | list[UserProject]: """Retrieve a list of objects. Args: diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py index 234796045..3ea2768ab 100644 --- a/tests/functional/conftest.py +++ b/tests/functional/conftest.py @@ -87,12 +87,12 @@ def reset_gitlab(gl: gitlab.Gitlab) -> None: settings.save() for project in gl.projects.list(): - for deploy_token in project.deploytokens.list(): + for project_deploy_token in project.deploytokens.list(): logging.info( - f"Deleting deploy token: {deploy_token.username!r} in " + f"Deleting deploy token: {project_deploy_token.username!r} in " f"project: {project.path_with_namespace!r}" ) - helpers.safe_delete(deploy_token) + helpers.safe_delete(project_deploy_token) logging.info(f"Deleting project: {project.path_with_namespace!r}") helpers.safe_delete(project) @@ -106,12 +106,12 @@ def reset_gitlab(gl: gitlab.Gitlab) -> None: ) continue - for deploy_token in group.deploytokens.list(): + for group_deploy_token in group.deploytokens.list(): logging.info( - f"Deleting deploy token: {deploy_token.username!r} in " + f"Deleting deploy token: {group_deploy_token.username!r} in " f"group: {group.path_with_namespace!r}" ) - helpers.safe_delete(deploy_token) + helpers.safe_delete(group_deploy_token) logging.info(f"Deleting group: {group.full_path!r}") helpers.safe_delete(group) for topic in gl.topics.list():