Skip to content

Commit

Permalink
Make ip_address property more similar to network
Browse files Browse the repository at this point in the history
Previously, it wasn't even always a string. This makes it consistent.
  • Loading branch information
oschwald committed Jan 22, 2025
1 parent b2a27fd commit 724e605
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 28 deletions.
4 changes: 3 additions & 1 deletion HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ History
* BREAKING: The ``raw`` attribute on the model classes has been replaced
with a ``to_dict()`` method. This can be used to get a representation of
the object that is suitable for serialization.
* BREAKING: The ``ip_address`` property on the model classes now always returns
a ``ipaddress.IPv4Address`` or ``ipaddress.IPv6Address``.
* BREAKING: The model and record classes now require all arguments other than
``locales`` to be keyword arguments.
``locales`` and ``ip_address`` to be keyword arguments.
* BREAKING: ``geoip2.mixins`` has been made internal. This normally would not
have been used by external code.
* IMPORTANT: Python 3.9 or greater is required. If you are using an older
Expand Down
6 changes: 4 additions & 2 deletions geoip2/_internal.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class Model(metaclass=ABCMeta):
"""Shared methods for MaxMind model classes"""

def __eq__(self, other: Any) -> bool:
return isinstance(other, self.__class__) and self.__dict__ == other.__dict__
return isinstance(other, self.__class__) and self.to_dict() == other.to_dict()

def __ne__(self, other):
return not self.__eq__(other)
Expand Down Expand Up @@ -42,8 +42,10 @@ def to_dict(self):
elif value is not None and value is not False:
result[key] = value

# network is a property for performance reasons
# network and ip_address properties for performance reasons
# pylint: disable=no-member
if hasattr(self, "ip_address") and self.ip_address is not None:
result["ip_address"] = str(self.ip_address)
if hasattr(self, "network") and self.network is not None:
result["network"] = str(self.network)

Expand Down
2 changes: 1 addition & 1 deletion geoip2/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ def _flat_model_for(
ip_address: IPAddress,
) -> Union[ConnectionType, ISP, AnonymousIP, Domain, ASN]:
(record, prefix_len) = self._get(types, ip_address)
return model_class(ip_address=ip_address, prefix_len=prefix_len, **record)
return model_class(ip_address, prefix_len=prefix_len, **record)

def metadata(
self,
Expand Down
36 changes: 24 additions & 12 deletions geoip2/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def __init__(

def __repr__(self) -> str:
return (
f"{self.__module__}.{self.__class__.__name__}({self._locales}, "
f"{self.__module__}.{self.__class__.__name__}({repr(self._locales)}, "
f"{', '.join(f'{k}={repr(v)}' for k, v in self.to_dict().items())})"
)

Expand Down Expand Up @@ -359,13 +359,13 @@ class Enterprise(City):
class SimpleModel(Model, metaclass=ABCMeta):
"""Provides basic methods for non-location models"""

ip_address: str
_ip_address: IPAddress
_network: Optional[Union[ipaddress.IPv4Network, ipaddress.IPv6Network]]
_prefix_len: int
_prefix_len: Optional[int]

def __init__(
self,
ip_address: Optional[str],
ip_address: IPAddress,
network: Optional[str],
prefix_len: Optional[int],
) -> None:
Expand All @@ -378,14 +378,26 @@ def __init__(
# used.
self._network = None
self._prefix_len = prefix_len
self.ip_address = ip_address
self._ip_address = ip_address

def __repr__(self) -> str:
d = self.to_dict()
d.pop("ip_address", None)
return (
f"{self.__module__}.{self.__class__.__name__}"
f"({', '.join(f'{k}={repr(v)}' for k, v in self.to_dict().items())})"
f"{self.__module__}.{self.__class__.__name__}({repr(str(self._ip_address))}, "
+ ", ".join(f"{k}={repr(v)}" for k, v in d.items())
+ ")"
)

@property
def ip_address(self):
"""The IP address for the record"""
if not isinstance(
self._ip_address, (ipaddress.IPv4Address, ipaddress.IPv6Address)
):
self._ip_address = ipaddress.ip_address(self._ip_address)
return self._ip_address

@property
def network(self) -> Optional[Union[ipaddress.IPv4Network, ipaddress.IPv6Network]]:
"""The network for the record"""
Expand Down Expand Up @@ -475,14 +487,14 @@ class AnonymousIP(SimpleModel):

def __init__(
self,
ip_address: IPAddress,
*,
is_anonymous: bool = False,
is_anonymous_vpn: bool = False,
is_hosting_provider: bool = False,
is_public_proxy: bool = False,
is_residential_proxy: bool = False,
is_tor_exit_node: bool = False,
ip_address: Optional[str] = None,
network: Optional[str] = None,
prefix_len: Optional[int] = None,
**_,
Expand Down Expand Up @@ -535,10 +547,10 @@ class ASN(SimpleModel):
# pylint:disable=too-many-arguments,too-many-positional-arguments
def __init__(
self,
ip_address: IPAddress,
*,
autonomous_system_number: Optional[int] = None,
autonomous_system_organization: Optional[str] = None,
ip_address: Optional[str] = None,
network: Optional[str] = None,
prefix_len: Optional[int] = None,
**_,
Expand Down Expand Up @@ -586,9 +598,9 @@ class ConnectionType(SimpleModel):

def __init__(
self,
ip_address: IPAddress,
*,
connection_type: Optional[str] = None,
ip_address: Optional[str] = None,
network: Optional[str] = None,
prefix_len: Optional[int] = None,
**_,
Expand Down Expand Up @@ -628,9 +640,9 @@ class Domain(SimpleModel):

def __init__(
self,
ip_address: IPAddress,
*,
domain: Optional[str] = None,
ip_address: Optional[str] = None,
network: Optional[str] = None,
prefix_len: Optional[int] = None,
**_,
Expand Down Expand Up @@ -708,14 +720,14 @@ class ISP(ASN):
# pylint:disable=too-many-arguments,too-many-positional-arguments
def __init__(
self,
ip_address: IPAddress,
*,
autonomous_system_number: Optional[int] = None,
autonomous_system_organization: Optional[str] = None,
isp: Optional[str] = None,
mobile_country_code: Optional[str] = None,
mobile_network_code: Optional[str] = None,
organization: Optional[str] = None,
ip_address: Optional[str] = None,
network: Optional[str] = None,
prefix_len: Optional[int] = None,
**_,
Expand Down
13 changes: 11 additions & 2 deletions geoip2/records.py
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,7 @@ class Traits(Record):
autonomous_system_organization: Optional[str]
connection_type: Optional[str]
domain: Optional[str]
ip_address: Optional[str]
_ip_address: Optional[str]
is_anonymous: bool
is_anonymous_proxy: bool
is_anonymous_vpn: bool
Expand Down Expand Up @@ -909,7 +909,7 @@ def __init__(
self.static_ip_score = static_ip_score
self.user_type = user_type
self.user_count = user_count
self.ip_address = ip_address
self._ip_address = ip_address
if network is None:
self._network = None
else:
Expand All @@ -919,6 +919,15 @@ def __init__(
# much more performance sensitive than web service users.
self._prefix_len = prefix_len

@property
def ip_address(self):
"""The IP address for the record"""
if not isinstance(
self._ip_address, (ipaddress.IPv4Address, ipaddress.IPv6Address)
):
self._ip_address = ipaddress.ip_address(self._ip_address)
return self._ip_address

@property
def network(self) -> Optional[Union[ipaddress.IPv4Network, ipaddress.IPv6Network]]:
"""The network for the record"""
Expand Down
22 changes: 13 additions & 9 deletions tests/database_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def test_anonymous_ip(self) -> None:
self.assertEqual(record.is_public_proxy, False)
self.assertEqual(record.is_residential_proxy, False)
self.assertEqual(record.is_tor_exit_node, False)
self.assertEqual(record.ip_address, ip_address)
self.assertEqual(record.ip_address, ipaddress.ip_address(ip_address))
self.assertEqual(record.network, ipaddress.ip_network("1.2.0.0/16"))
reader.close()

Expand All @@ -99,7 +99,7 @@ def test_anonymous_ip_all_set(self) -> None:
self.assertEqual(record.is_public_proxy, True)
self.assertEqual(record.is_residential_proxy, True)
self.assertEqual(record.is_tor_exit_node, True)
self.assertEqual(record.ip_address, ip_address)
self.assertEqual(record.ip_address, ipaddress.ip_address(ip_address))
self.assertEqual(record.network, ipaddress.ip_network("81.2.69.0/24"))
reader.close()

Expand All @@ -113,7 +113,7 @@ def test_asn(self) -> None:

self.assertEqual(record.autonomous_system_number, 1221)
self.assertEqual(record.autonomous_system_organization, "Telstra Pty Ltd")
self.assertEqual(record.ip_address, ip_address)
self.assertEqual(record.ip_address, ipaddress.ip_address(ip_address))
self.assertEqual(record.network, ipaddress.ip_network("1.128.0.0/11"))

self.assertRegex(
Expand Down Expand Up @@ -156,7 +156,7 @@ def test_connection_type(self) -> None:
)

self.assertEqual(record.connection_type, "Cellular")
self.assertEqual(record.ip_address, ip_address)
self.assertEqual(record.ip_address, ipaddress.ip_address(ip_address))
self.assertEqual(record.network, ipaddress.ip_network("1.0.1.0/24"))

self.assertRegex(
Expand All @@ -171,7 +171,9 @@ def test_country(self) -> None:
reader = geoip2.database.Reader("tests/data/test-data/GeoIP2-Country-Test.mmdb")
record = reader.country("81.2.69.160")
self.assertEqual(
record.traits.ip_address, "81.2.69.160", "IP address is added to model"
record.traits.ip_address,
ipaddress.ip_address("81.2.69.160"),
"IP address is added to model",
)
self.assertEqual(record.traits.network, ipaddress.ip_network("81.2.69.160/27"))
self.assertEqual(record.country.is_in_european_union, False)
Expand All @@ -192,7 +194,7 @@ def test_domain(self) -> None:
self.assertEqual(record, eval(repr(record)), "Domain repr can be eval'd")

self.assertEqual(record.domain, "maxmind.com")
self.assertEqual(record.ip_address, ip_address)
self.assertEqual(record.ip_address, ipaddress.ip_address(ip_address))
self.assertEqual(record.network, ipaddress.ip_network("1.2.0.0/16"))

self.assertRegex(
Expand All @@ -217,7 +219,7 @@ def test_enterprise(self) -> None:
self.assertEqual(record.registered_country.is_in_european_union, False)
self.assertEqual(record.traits.connection_type, "Cable/DSL")
self.assertTrue(record.traits.is_legitimate_proxy)
self.assertEqual(record.traits.ip_address, ip_address)
self.assertEqual(record.traits.ip_address, ipaddress.ip_address(ip_address))
self.assertEqual(
record.traits.network, ipaddress.ip_network("74.209.16.0/20")
)
Expand All @@ -242,7 +244,7 @@ def test_isp(self) -> None:
self.assertEqual(record.autonomous_system_organization, "Telstra Pty Ltd")
self.assertEqual(record.isp, "Telstra Internet")
self.assertEqual(record.organization, "Telstra Internet")
self.assertEqual(record.ip_address, ip_address)
self.assertEqual(record.ip_address, ipaddress.ip_address(ip_address))
self.assertEqual(record.network, ipaddress.ip_network("1.128.0.0/11"))

self.assertRegex(
Expand All @@ -261,7 +263,9 @@ def test_context_manager(self) -> None:
"tests/data/test-data/GeoIP2-Country-Test.mmdb"
) as reader:
record = reader.country("81.2.69.160")
self.assertEqual(record.traits.ip_address, "81.2.69.160")
self.assertEqual(
record.traits.ip_address, ipaddress.ip_address("81.2.69.160")
)

@patch("maxminddb.open_database")
def test_modes(self, mock_open) -> None:
Expand Down
5 changes: 4 additions & 1 deletion tests/models_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from __future__ import unicode_literals

import sys
import ipaddress
from typing import Dict
import unittest

Expand Down Expand Up @@ -393,7 +394,9 @@ def test_unknown_keys(self) -> None:
model.unk_base # type: ignore
with self.assertRaises(AttributeError):
model.traits.invalid # type: ignore
self.assertEqual(model.traits.ip_address, "1.2.3.4", "correct ip")
self.assertEqual(
model.traits.ip_address, ipaddress.ip_address("1.2.3.4"), "correct ip"
)


class TestNames(unittest.TestCase):
Expand Down

0 comments on commit 724e605

Please sign in to comment.