refactor: simplify client class inheritance complexity (#272)

- Instead of extensively overriding a parent method, use a private helper method.
- Prevent having get_actions methods for clients that do not support it.
- Make sure we document all the clients methods by not forgetting to override a parent method.
- Do not implement inheritance with different method signature.

* refactor: rename ClientEntityBase _get_all to _iter_pages

* refactor: move GetEntityByNameMixin.get_by_name to ClientEntityBase._get_first_by

Remove GetEntityByNameMixin and use _get_first_by method.

* refactor: remove ClientEntityBase get_* methods

* fix: always pass args before kwargs in _iter_pages

* refactor: reuse _get_first_by for all get_by_* methods

Author: jooola

hcloud/actions/client.py

@@ -101,4 +101,4 @@ def get_all(
                Specify how the results are sorted. Choices: `id` `command` `status` `progress`  `started` `finished` . You can add one of ":asc", ":desc" to modify sort order. ( ":asc" is default)
         :return: List[:class:`BoundAction <hcloud.actions.client.BoundAction>`]
         """
-        return super().get_all(status=status, sort=sort)
+        return self._iter_pages(self.get_list, status=status, sort=sort)

hcloud/certificates/client.py

@@ -3,7 +3,7 @@
 from typing import TYPE_CHECKING, Any, NamedTuple
 
 from ..actions import ActionsPageResult, BoundAction
-from ..core import BoundModelBase, ClientEntityBase, GetEntityByNameMixin, Meta
+from ..core import BoundModelBase, ClientEntityBase, Meta
 from .domain import (
     Certificate,
     CreateManagedCertificateResponse,
@@ -103,7 +103,7 @@ class CertificatesPageResult(NamedTuple):
     meta: Meta | None
 
 
-class CertificatesClient(ClientEntityBase, GetEntityByNameMixin):
+class CertificatesClient(ClientEntityBase):
     _client: Client
 
     def get_by_id(self, id: int) -> BoundCertificate:
@@ -171,7 +171,7 @@ def get_all(
                Can be used to filter certificates by labels. The response will only contain certificates matching the label selector.
         :return: List[:class:`BoundCertificate <hcloud.certificates.client.BoundCertificate>`]
         """
-        return super().get_all(name=name, label_selector=label_selector)
+        return self._iter_pages(self.get_list, name=name, label_selector=label_selector)
 
     def get_by_name(self, name: str) -> BoundCertificate | None:
         """Get certificate by name
@@ -180,7 +180,7 @@ def get_by_name(self, name: str) -> BoundCertificate | None:
                Used to get certificate by name.
         :return: :class:`BoundCertificate <hcloud.certificates.client.BoundCertificate>`
         """
-        return super().get_by_name(name)
+        return self._get_first_by(name=name)
 
     def create(
         self,
@@ -338,7 +338,12 @@ def get_actions(
                Specify how the results are sorted. Choices: `id` `id:asc` `id:desc` `command` `command:asc` `command:desc` `status` `status:asc` `status:desc` `progress` `progress:asc` `progress:desc` `started` `started:asc` `started:desc` `finished` `finished:asc` `finished:desc`
         :return: List[:class:`BoundAction <hcloud.actions.client.BoundAction>`]
         """
-        return super().get_actions(certificate, status=status, sort=sort)
+        return self._iter_pages(
+            self.get_actions_list,
+            certificate,
+            status=status,
+            sort=sort,
+        )
 
     def retry_issuance(
         self,

hcloud/core/__init__.py

@@ -1,4 +1,4 @@
 from __future__ import annotations
 
-from .client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin  # noqa: F401
+from .client import BoundModelBase, ClientEntityBase  # noqa: F401
 from .domain import BaseDomain, DomainIdentityMixin, Meta, Pagination  # noqa: F401

hcloud/core/client.py

@@ -4,7 +4,6 @@
 
 if TYPE_CHECKING:
     from .._client import Client
-    from ..actions import BoundAction
 
 
 class ClientEntityBase:
@@ -19,7 +18,7 @@ def __init__(self, client: Client):
         """
         self._client = client
 
-    def _get_all(  # type: ignore[no-untyped-def]
+    def _iter_pages(  # type: ignore[no-untyped-def]
         self,
         list_function: Callable,
         *args,
@@ -32,42 +31,21 @@ def _get_all(  # type: ignore[no-untyped-def]
             # The *PageResult tuples MUST have the following structure
             # `(result: List[Bound*], meta: Meta)`
             result, meta = list_function(
-                page=page, per_page=self.max_per_page, *args, **kwargs
+                *args, page=page, per_page=self.max_per_page, **kwargs
             )
             if result:
                 results.extend(result)
 
-            if (
-                meta
-                and meta.pagination
-                and meta.pagination.next_page
-                and meta.pagination.next_page
-            ):
+            if meta and meta.pagination and meta.pagination.next_page:
                 page = meta.pagination.next_page
             else:
                 page = 0
 
         return results
 
-    def get_all(self, *args, **kwargs) -> list:  # type: ignore[no-untyped-def]
+    def _get_first_by(self, **kwargs):  # type: ignore[no-untyped-def]
         assert hasattr(self, "get_list")
-        return self._get_all(self.get_list, *args, **kwargs)
-
-    def get_actions(self, *args, **kwargs) -> list[BoundAction]:  # type: ignore[no-untyped-def]
-        if not hasattr(self, "get_actions_list"):
-            raise ValueError("this endpoint does not support get_actions method")
-
-        return self._get_all(self.get_actions_list, *args, **kwargs)
-
-
-class GetEntityByNameMixin:
-    """
-    Use as a mixin for ClientEntityBase classes
-    """
-
-    def get_by_name(self, name: str):  # type: ignore[no-untyped-def]
-        assert hasattr(self, "get_list")
-        entities, _ = self.get_list(name=name)
+        entities, _ = self.get_list(**kwargs)
         return entities[0] if entities else None
 
 

hcloud/datacenters/client.py

@@ -2,7 +2,7 @@
 
 from typing import TYPE_CHECKING, Any, NamedTuple
 
-from ..core import BoundModelBase, ClientEntityBase, GetEntityByNameMixin, Meta
+from ..core import BoundModelBase, ClientEntityBase, Meta
 from ..locations import BoundLocation
 from ..server_types import BoundServerType
 from .domain import Datacenter, DatacenterServerTypes
@@ -55,7 +55,7 @@ class DatacentersPageResult(NamedTuple):
     meta: Meta | None
 
 
-class DatacentersClient(ClientEntityBase, GetEntityByNameMixin):
+class DatacentersClient(ClientEntityBase):
     _client: Client
 
     def get_by_id(self, id: int) -> BoundDatacenter:
@@ -109,7 +109,7 @@ def get_all(self, name: str | None = None) -> list[BoundDatacenter]:
                Can be used to filter datacenters by their name.
         :return: List[:class:`BoundDatacenter <hcloud.datacenters.client.BoundDatacenter>`]
         """
-        return super().get_all(name=name)
+        return self._iter_pages(self.get_list, name=name)
 
     def get_by_name(self, name: str) -> BoundDatacenter | None:
         """Get datacenter by name
@@ -118,4 +118,4 @@ def get_by_name(self, name: str) -> BoundDatacenter | None:
                Used to get datacenter by name.
         :return: :class:`BoundDatacenter <hcloud.datacenters.client.BoundDatacenter>`
         """
-        return super().get_by_name(name)
+        return self._get_first_by(name=name)

hcloud/firewalls/client.py

@@ -3,7 +3,7 @@
 from typing import TYPE_CHECKING, Any, NamedTuple
 
 from ..actions import ActionsPageResult, BoundAction
-from ..core import BoundModelBase, ClientEntityBase, GetEntityByNameMixin, Meta
+from ..core import BoundModelBase, ClientEntityBase, Meta
 from .domain import (
     CreateFirewallResponse,
     Firewall,
@@ -158,7 +158,7 @@ class FirewallsPageResult(NamedTuple):
     meta: Meta | None
 
 
-class FirewallsClient(ClientEntityBase, GetEntityByNameMixin):
+class FirewallsClient(ClientEntityBase):
     _client: Client
 
     def get_actions_list(
@@ -218,7 +218,12 @@ def get_actions(
 
         :return: List[:class:`BoundAction <hcloud.actions.client.BoundAction>`]
         """
-        return super().get_actions(firewall, status=status, sort=sort)
+        return self._iter_pages(
+            self.get_actions_list,
+            firewall,
+            status=status,
+            sort=sort,
+        )
 
     def get_by_id(self, id: int) -> BoundFirewall:
         """Returns a specific Firewall object.
@@ -287,7 +292,12 @@ def get_all(
                Choices: id name created (You can add one of ":asc", ":desc" to modify sort order. ( ":asc" is default))
         :return: List[:class:`BoundFirewall <hcloud.firewalls.client.BoundFirewall>`]
         """
-        return super().get_all(label_selector=label_selector, name=name, sort=sort)
+        return self._iter_pages(
+            self.get_list,
+            label_selector=label_selector,
+            name=name,
+            sort=sort,
+        )
 
     def get_by_name(self, name: str) -> BoundFirewall | None:
         """Get Firewall by name
@@ -296,7 +306,7 @@ def get_by_name(self, name: str) -> BoundFirewall | None:
                Used to get Firewall by name.
         :return: :class:`BoundFirewall <hcloud.firewalls.client.BoundFirewall>`
         """
-        return super().get_by_name(name)
+        return self._get_first_by(name=name)
 
     def create(
         self,

hcloud/floating_ips/client.py

@@ -3,7 +3,7 @@
 from typing import TYPE_CHECKING, Any, NamedTuple
 
 from ..actions import ActionsPageResult, BoundAction
-from ..core import BoundModelBase, ClientEntityBase, GetEntityByNameMixin, Meta
+from ..core import BoundModelBase, ClientEntityBase, Meta
 from ..locations import BoundLocation
 from .domain import CreateFloatingIPResponse, FloatingIP
 
@@ -139,7 +139,7 @@ class FloatingIPsPageResult(NamedTuple):
     meta: Meta | None
 
 
-class FloatingIPsClient(ClientEntityBase, GetEntityByNameMixin):
+class FloatingIPsClient(ClientEntityBase):
     _client: Client
 
     def get_actions_list(
@@ -199,7 +199,12 @@ def get_actions(
 
         :return: List[:class:`BoundAction <hcloud.actions.client.BoundAction>`]
         """
-        return super().get_actions(floating_ip, status=status, sort=sort)
+        return self._iter_pages(
+            self.get_actions_list,
+            floating_ip,
+            status=status,
+            sort=sort,
+        )
 
     def get_by_id(self, id: int) -> BoundFloatingIP:
         """Returns a specific Floating IP object.
@@ -263,7 +268,7 @@ def get_all(
                Can be used to filter networks by their name.
         :return: List[:class:`BoundFloatingIP <hcloud.floating_ips.client.BoundFloatingIP>`]
         """
-        return super().get_all(label_selector=label_selector, name=name)
+        return self._iter_pages(self.get_list, label_selector=label_selector, name=name)
 
     def get_by_name(self, name: str) -> BoundFloatingIP | None:
         """Get Floating IP by name
@@ -272,7 +277,7 @@ def get_by_name(self, name: str) -> BoundFloatingIP | None:
                Used to get Floating IP by name.
         :return: :class:`BoundFloatingIP <hcloud.floating_ips.client.BoundFloatingIP>`
         """
-        return super().get_by_name(name)
+        return self._get_first_by(name=name)
 
     def create(
         self,

hcloud/images/client.py

@@ -3,7 +3,7 @@
 from typing import TYPE_CHECKING, Any, NamedTuple
 
 from ..actions import ActionsPageResult, BoundAction
-from ..core import BoundModelBase, ClientEntityBase, GetEntityByNameMixin, Meta
+from ..core import BoundModelBase, ClientEntityBase, Meta
 from .domain import Image
 
 if TYPE_CHECKING:
@@ -110,7 +110,7 @@ class ImagesPageResult(NamedTuple):
     meta: Meta | None
 
 
-class ImagesClient(ClientEntityBase, GetEntityByNameMixin):
+class ImagesClient(ClientEntityBase):
     _client: Client
 
     def get_actions_list(
@@ -169,7 +169,12 @@ def get_actions(
                Specify how the results are sorted. Choices: `id` `command` `status` `progress`  `started` `finished` . You can add one of ":asc", ":desc" to modify sort order. ( ":asc" is default)
         :return: List[:class:`BoundAction <hcloud.actions.client.BoundAction>`]
         """
-        return super().get_actions(image, sort=sort, status=status)
+        return self._iter_pages(
+            self.get_actions_list,
+            image,
+            sort=sort,
+            status=status,
+        )
 
     def get_by_id(self, id: int) -> BoundImage:
         """Get a specific Image
@@ -274,7 +279,8 @@ def get_all(
                Include deprecated images in the response. Default: False
         :return: List[:class:`BoundImage <hcloud.images.client.BoundImage>`]
         """
-        return super().get_all(
+        return self._iter_pages(
+            self.get_list,
             name=name,
             label_selector=label_selector,
             bound_to=bound_to,
@@ -294,7 +300,7 @@ def get_by_name(self, name: str) -> BoundImage | None:
                Used to get image by name.
         :return: :class:`BoundImage <hcloud.images.client.BoundImage>`
         """
-        return super().get_by_name(name)
+        return self._get_first_by(name=name)
 
     def get_by_name_and_architecture(
         self,
@@ -309,9 +315,7 @@ def get_by_name_and_architecture(
                Used to identify the image.
         :return: :class:`BoundImage <hcloud.images.client.BoundImage>`
         """
-        entities, _ = self.get_list(name=name, architecture=[architecture])
-        entity = entities[0] if entities else None
-        return entity
+        return self._get_first_by(name=name, architecture=[architecture])
 
     def update(
         self,

hcloud/isos/client.py

@@ -3,7 +3,7 @@
 from typing import TYPE_CHECKING, Any, NamedTuple
 from warnings import warn
 
-from ..core import BoundModelBase, ClientEntityBase, GetEntityByNameMixin, Meta
+from ..core import BoundModelBase, ClientEntityBase, Meta
 from .domain import Iso
 
 if TYPE_CHECKING:
@@ -21,7 +21,7 @@ class IsosPageResult(NamedTuple):
     meta: Meta | None
 
 
-class IsosClient(ClientEntityBase, GetEntityByNameMixin):
+class IsosClient(ClientEntityBase):
     _client: Client
 
     def get_by_id(self, id: int) -> BoundIso:
@@ -111,7 +111,8 @@ def get_all(
             )
             include_architecture_wildcard = include_wildcard_architecture
 
-        return super().get_all(
+        return self._iter_pages(
+            self.get_list,
             name=name,
             architecture=architecture,
             include_architecture_wildcard=include_architecture_wildcard,
@@ -124,4 +125,4 @@ def get_by_name(self, name: str) -> BoundIso | None:
                Used to get iso by name.
         :return: :class:`BoundIso <hcloud.isos.client.BoundIso>`
         """
-        return super().get_by_name(name)
+        return self._get_first_by(name=name)

hcloud/load_balancer_types/client.py

@@ -2,7 +2,7 @@
 
 from typing import TYPE_CHECKING, Any, NamedTuple
 
-from ..core import BoundModelBase, ClientEntityBase, GetEntityByNameMixin, Meta
+from ..core import BoundModelBase, ClientEntityBase, Meta
 from .domain import LoadBalancerType
 
 if TYPE_CHECKING:
@@ -20,7 +20,7 @@ class LoadBalancerTypesPageResult(NamedTuple):
     meta: Meta | None
 
 
-class LoadBalancerTypesClient(ClientEntityBase, GetEntityByNameMixin):
+class LoadBalancerTypesClient(ClientEntityBase):
     _client: Client
 
     def get_by_id(self, id: int) -> BoundLoadBalancerType:
@@ -77,7 +77,7 @@ def get_all(self, name: str | None = None) -> list[BoundLoadBalancerType]:
                Can be used to filter Load Balancer type by their name.
         :return: List[:class:`BoundLoadBalancerType <hcloud.load_balancer_types.client.BoundLoadBalancerType>`]
         """
-        return super().get_all(name=name)
+        return self._iter_pages(self.get_list, name=name)
 
     def get_by_name(self, name: str) -> BoundLoadBalancerType | None:
         """Get Load Balancer type by name
@@ -86,4 +86,4 @@ def get_by_name(self, name: str) -> BoundLoadBalancerType | None:
                Used to get Load Balancer type by name.
         :return: :class:`BoundLoadBalancerType <hcloud.load_balancer_types.client.BoundLoadBalancerType>`
         """
-        return super().get_by_name(name)
+        return self._get_first_by(name=name)