From 8e054c11252d27ecdadc0e307266f04d4c4428d8 Mon Sep 17 00:00:00 2001 From: jo Date: Tue, 10 Jun 2025 17:53:02 +0200 Subject: [PATCH] fix: handle string id when checking has_id_or_name The function did not handle the case of string ids (e.g. "2345"), we now handle this edge case. Co-authored-by: Lukas Metzner --- hcloud/core/domain.py | 17 ++++++++++++----- tests/unit/core/test_domain.py | 26 ++++++++++++++++---------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/hcloud/core/domain.py b/hcloud/core/domain.py index f3050301..b2ac1972 100644 --- a/hcloud/core/domain.py +++ b/hcloud/core/domain.py @@ -52,15 +52,22 @@ def has_id_or_name(self, id_or_name: int | str) -> bool: the comparison will not work as expected (e.g. the domains are the same but cannot be equal, if one provides an id and the other the name). """ - values: list[int | str] = [] + result = None + if self.id is not None: - values.append(self.id) + value = id_or_name + if isinstance(id_or_name, str) and id_or_name.isnumeric(): + value = int(id_or_name) + + result = result or self.id == value + if self.name is not None: - values.append(self.name) - if not values: + result = result or self.name == str(id_or_name) + + if result is None: raise ValueError("id or name must be set") - return id_or_name in values + return result class Pagination(BaseDomain): diff --git a/tests/unit/core/test_domain.py b/tests/unit/core/test_domain.py index 484799f6..73cab892 100644 --- a/tests/unit/core/test_domain.py +++ b/tests/unit/core/test_domain.py @@ -71,19 +71,25 @@ def test_id_or_name_exception(self): assert str(error) == "id or name must be set" @pytest.mark.parametrize( - "other, expected", + "domain, id_or_name, expected", [ - (SomeDomain(id=1), True), - (SomeDomain(name="name1"), True), - (SomeDomain(id=1, name="name1"), True), - (SomeDomain(id=2), False), - (SomeDomain(name="name2"), False), - (SomeDomain(id=2, name="name2"), False), + (SomeDomain(id=1, name="name1"), 1, True), + (SomeDomain(id=1, name="name1"), "1", True), + (SomeDomain(id=1, name="name1"), "name1", True), + (SomeDomain(id=1, name="name1"), 2, False), + (SomeDomain(id=1, name="name1"), "2", False), + (SomeDomain(id=1, name="name1"), "name2", False), + (SomeDomain(id=1, name="3"), 3, True), + (SomeDomain(id=3, name="1"), "3", True), ], ) - def test_has_id_or_name_exception(self, other, expected): - domain = SomeDomain(id=1, name="name1") - assert domain.has_id_or_name(other.id_or_name) == expected + def test_has_id_or_name( + self, + domain: SomeDomain, + id_or_name: str | int, + expected: bool, + ): + assert domain.has_id_or_name(id_or_name) == expected class ActionDomain(BaseDomain, DomainIdentityMixin):