From 5caced96796e6084e9aacbb4514486c06d62daf7 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 30 Oct 2026 19:39:18 +0000 Subject: [PATCH] Return the dataclass Metadata from the C reader The C extension's metadata() now returns maxminddb.reader.Metadata, the dataclass that the pure Python reader returns. Metadata from both readers is now the same type, compares equal, and works with repr() and dataclasses.asdict(). maxminddb.extension.Metadata stays as an alias of this class. As with the pure Python class, hash() raises TypeError, and the constructor takes only keyword arguments. Co-Authored-By: Claude Opus 5.5 --- HISTORY.rst | 17 ++- extension/maxminddb.c | 231 ++++++---------------------------------- maxminddb/extension.pyi | 76 +------------ tests/reader_test.py | 64 +++++------ 4 files changed, 81 insertions(+), 307 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 755aba2f..7a416cbc 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -35,8 +35,8 @@ History ``os.pread``. * C extension: - * Fixed segmentation faults from invalid use of ``Metadata``, ``Reader`` and - the internal iterator type. + * Fixed segmentation faults from invalid use of ``Reader`` and the internal + iterator type. * Fixed a segmentation fault on a database with a map key that is not a string. Such a database now raises ``InvalidDatabaseError``. * Fixed large ``uint32`` values, which came back negative on platforms @@ -44,7 +44,6 @@ History * Fixed memory leaks and a use-after-free. A second ``__init__`` on a ``Reader`` now closes the old database, and an iterator from before it raises ``ValueError``. A failed ``__init__`` keeps the old database. - Reinitializing a ``Metadata`` changes nothing. * Fixed a ``RuntimeWarning`` or ``RuntimeError`` on free-threaded Python on macOS when a ``Reader`` failed to open or was used without ``__init__``. * Fixed a deadlock on free-threaded Python when a ``Reader`` was closed @@ -57,8 +56,16 @@ History This includes an error in the data of one record, but not a failure to get the read lock. Previously, the next call returned the remaining networks. - * Added the ``node_byte_size`` and ``search_tree_size`` properties to - ``Metadata``, as the pure Python ``Metadata`` has. + * ``Reader.metadata()`` now returns the pure Python + ``maxminddb.reader.Metadata`` dataclass, as the pure Python reader does. + Metadata from the two readers compares equal with ``==``, and it works + with ``repr()`` and ``dataclasses.asdict()``. It also has the + ``node_byte_size`` and ``search_tree_size`` properties. + ``maxminddb.extension.Metadata`` is now the same class. As with the pure + Python class, ``hash()`` raises ``TypeError``, the constructor takes + only keyword arguments and raises ``TypeError`` for a missing or unknown + argument, and assignment to an attribute raises + ``dataclasses.FrozenInstanceError``. GitHub #468. * Metadata: diff --git a/extension/maxminddb.c b/extension/maxminddb.c index de5aa0ca..1abf7213 100644 --- a/extension/maxminddb.c +++ b/extension/maxminddb.c @@ -91,19 +91,6 @@ typedef struct { // Set once next() has raised StopIteration or an error. bool done; } ReaderIter_obj; - -typedef struct { - PyObject_HEAD /* no semicolon */ - PyObject *binary_format_major_version; - PyObject *binary_format_minor_version; - PyObject *build_epoch; - PyObject *database_type; - PyObject *description; - PyObject *ip_version; - PyObject *languages; - PyObject *node_count; - PyObject *record_size; -} Metadata_obj; // clang-format on // ============================================================================= @@ -139,7 +126,6 @@ static bool can_read(const char *path); static int get_record(PyObject *self, PyObject *args, PyObject **record); static PyObject *metadata_value(PyObject *map, const char *key); static void set_error_from_cause(PyObject *type, const char *message); -static PyObject *Metadata_node_byte_size(PyObject *self, void *closure); static PyObject *reader_iter_next(PyObject *self); static PyObject *corrupt_tree_error(ReaderIter_obj *ri, maxminddb_state *state, @@ -745,18 +731,32 @@ static PyObject *Reader_metadata(PyObject *self, PyObject *UNUSED(args)) { "Error decoding metadata."); } } else { - // The order of the values must match kwlist in Metadata_new. - metadata = PyObject_CallFunction(state->Metadata_Type, - "HHKOOHOIH", - binary_format_major_version, - binary_format_minor_version, - (unsigned long long)build_epoch, - database_type, - description, - ip_version, - languages, - (unsigned int)node_count, - record_size); + // Metadata takes keyword arguments only. + PyObject *kwargs = + Py_BuildValue("{s:H,s:H,s:K,s:O,s:O,s:H,s:O,s:I,s:H}", + "binary_format_major_version", + binary_format_major_version, + "binary_format_minor_version", + binary_format_minor_version, + "build_epoch", + (unsigned long long)build_epoch, + "database_type", + database_type, + "description", + description, + "ip_version", + ip_version, + "languages", + languages, + "node_count", + (unsigned int)node_count, + "record_size", + record_size); + if (kwargs != NULL) { + metadata = PyObject_VectorcallDict( + state->Metadata_Type, NULL, 0, kwargs); + Py_DECREF(kwargs); + } } Py_DECREF(map); } @@ -1158,74 +1158,6 @@ static void free_records(struct record *next) { } } -// Metadata is immutable, so tp_new sets every field and there is no tp_init. -// No object can then have a NULL field or be initialized twice. -static PyObject * -Metadata_new(PyTypeObject *type, PyObject *args, PyObject *kwds) { - PyObject *binary_format_major_version, *binary_format_minor_version, - *build_epoch, *database_type, *description, *ip_version, *languages, - *node_count, *record_size; - - // Reader_metadata passes the values in this order. - static char *kwlist[] = {"binary_format_major_version", - "binary_format_minor_version", - "build_epoch", - "database_type", - "description", - "ip_version", - "languages", - "node_count", - "record_size", - NULL}; - - if (!PyArg_ParseTupleAndKeywords(args, - kwds, - "OOOOOOOOO:Metadata", - kwlist, - &binary_format_major_version, - &binary_format_minor_version, - &build_epoch, - &database_type, - &description, - &ip_version, - &languages, - &node_count, - &record_size)) { - return NULL; - } - - Metadata_obj *obj = (Metadata_obj *)type->tp_alloc(type, 0); - if (obj == NULL) { - return NULL; - } - obj->binary_format_major_version = Py_NewRef(binary_format_major_version); - obj->binary_format_minor_version = Py_NewRef(binary_format_minor_version); - obj->build_epoch = Py_NewRef(build_epoch); - obj->database_type = Py_NewRef(database_type); - obj->description = Py_NewRef(description); - obj->ip_version = Py_NewRef(ip_version); - obj->languages = Py_NewRef(languages); - obj->node_count = Py_NewRef(node_count); - obj->record_size = Py_NewRef(record_size); - return (PyObject *)obj; -} - -static void Metadata_dealloc(PyObject *self) { - Metadata_obj *obj = (Metadata_obj *)self; - Py_DECREF(obj->binary_format_major_version); - Py_DECREF(obj->binary_format_minor_version); - Py_DECREF(obj->build_epoch); - Py_DECREF(obj->database_type); - Py_DECREF(obj->description); - Py_DECREF(obj->ip_version); - Py_DECREF(obj->languages); - Py_DECREF(obj->node_count); - Py_DECREF(obj->record_size); - PyTypeObject *type = Py_TYPE(self); - PyObject_Del(self); - Py_DECREF(type); -} - static PyObject * from_entry_data_list(maxminddb_state *state, MMDB_entry_data_list_s **entry_data_list) { @@ -1431,90 +1363,6 @@ static PyMemberDef Reader_members[] = { static PyMethodDef ReaderIter_methods[] = {{NULL, NULL, 0, NULL}}; -static PyMethodDef Metadata_methods[] = {{NULL, NULL, 0, NULL}}; - -static PyMemberDef Metadata_members[] = { - {"binary_format_major_version", - T_OBJECT, - offsetof(Metadata_obj, binary_format_major_version), - READONLY, - NULL}, - {"binary_format_minor_version", - T_OBJECT, - offsetof(Metadata_obj, binary_format_minor_version), - READONLY, - NULL}, - {"build_epoch", - T_OBJECT, - offsetof(Metadata_obj, build_epoch), - READONLY, - NULL}, - {"database_type", - T_OBJECT, - offsetof(Metadata_obj, database_type), - READONLY, - NULL}, - {"description", - T_OBJECT, - offsetof(Metadata_obj, description), - READONLY, - NULL}, - {"ip_version", - T_OBJECT, - offsetof(Metadata_obj, ip_version), - READONLY, - NULL}, - {"languages", T_OBJECT, offsetof(Metadata_obj, languages), READONLY, NULL}, - {"node_count", - T_OBJECT, - offsetof(Metadata_obj, node_count), - READONLY, - NULL}, - {"record_size", - T_OBJECT, - offsetof(Metadata_obj, record_size), - READONLY, - NULL}, - {NULL, 0, 0, 0, NULL}}; - -static PyObject *Metadata_search_tree_size(PyObject *self, - void *UNUSED(closure)) { - Metadata_obj *obj = (Metadata_obj *)self; - PyObject *node_byte_size = Metadata_node_byte_size(self, NULL); - if (node_byte_size == NULL) { - return NULL; - } - PyObject *search_tree_size = - PyNumber_Multiply(obj->node_count, node_byte_size); - Py_DECREF(node_byte_size); - return search_tree_size; -} - -static PyObject *Metadata_node_byte_size(PyObject *self, - void *UNUSED(closure)) { - Metadata_obj *obj = (Metadata_obj *)self; - PyObject *four = PyLong_FromLong(4); - if (four == NULL) { - return NULL; - } - PyObject *node_byte_size = PyNumber_FloorDivide(obj->record_size, four); - Py_DECREF(four); - return node_byte_size; -} - -// These match the properties of the pure Python Metadata class. -static PyGetSetDef Metadata_getset[] = {{"node_byte_size", - Metadata_node_byte_size, - NULL, - "The size of a node in bytes.", - NULL}, - {"search_tree_size", - Metadata_search_tree_size, - NULL, - "The size of the search tree.", - NULL}, - {NULL, NULL, NULL, NULL, NULL}}; - // ============================================================================= // Type specs for heap type conversion (PEP 489) // ============================================================================= @@ -1537,23 +1385,6 @@ static PyType_Spec Reader_Type_spec = { .slots = Reader_Type_slots, }; -static PyType_Slot Metadata_Type_slots[] = { - {Py_tp_doc, "Metadata object"}, - {Py_tp_dealloc, Metadata_dealloc}, - {Py_tp_new, Metadata_new}, - {Py_tp_methods, Metadata_methods}, - {Py_tp_members, Metadata_members}, - {Py_tp_getset, Metadata_getset}, - {0, NULL}, -}; - -static PyType_Spec Metadata_Type_spec = { - .name = "maxminddb.extension.Metadata", - .basicsize = sizeof(Metadata_obj), - .flags = Py_TPFLAGS_DEFAULT, - .slots = Metadata_Type_slots, -}; - static PyType_Slot ReaderIter_Type_slots[] = { {Py_tp_doc, "Iterator for Reader object"}, {Py_tp_dealloc, ReaderIter_dealloc}, @@ -1614,8 +1445,14 @@ static int maxminddb_exec(PyObject *module) { return -1; } - state->Metadata_Type = - PyType_FromModuleAndSpec(module, &Metadata_Type_spec, NULL); + // metadata() returns the pure Python Metadata, so both readers return + // the same type. + PyObject *reader_mod = PyImport_ImportModule("maxminddb.reader"); + if (reader_mod == NULL) { + return -1; + } + state->Metadata_Type = PyObject_GetAttrString(reader_mod, "Metadata"); + Py_DECREF(reader_mod); if (state->Metadata_Type == NULL) { return -1; } diff --git a/maxminddb/extension.pyi b/maxminddb/extension.pyi index 19c8d4fe..9ee6947d 100644 --- a/maxminddb/extension.pyi +++ b/maxminddb/extension.pyi @@ -5,6 +5,7 @@ from ipaddress import IPv4Address, IPv4Network, IPv6Address, IPv6Network from typing_extensions import Self +from maxminddb.reader import Metadata as Metadata from maxminddb.types import Record, StrOrBytesPath class Reader: @@ -58,78 +59,3 @@ class Reader: def __iter__(self) -> Iterator[tuple[IPv4Network | IPv6Network, Record]]: ... def __enter__(self) -> Self: ... def __exit__(self, *args: object) -> None: ... - -class Metadata: - """Metadata for the MaxMind DB reader.""" - - binary_format_major_version: int - """ - The major version number of the binary format used when creating the - database. - """ - - binary_format_minor_version: int - """ - The minor version number of the binary format used when creating the - database. - """ - - build_epoch: int - """ - The Unix epoch for the build time of the database. - """ - - database_type: str - """ - A string identifying the database type, e.g., "GeoIP2-City". - """ - - description: dict[str, str] - """ - A map from locales to text descriptions of the database. - """ - - ip_version: int - """ - The IP version of the data in a database. A value of "4" means the - database only supports IPv4. A database with a value of "6" may support - both IPv4 and IPv6 lookups. - """ - - languages: list[str] - """ - A list of locale codes supported by the database. - """ - - node_count: int - """ - The number of nodes in the database. - """ - - record_size: int - """ - The bit size of a record in the search tree. - """ - - def __init__( - self, - *, - binary_format_major_version: int, - binary_format_minor_version: int, - build_epoch: int, - database_type: str, - description: dict[str, str], - ip_version: int, - languages: list[str], - node_count: int, - record_size: int, - ) -> None: - """Create new Metadata object from the metadata fields in the spec.""" - - @property - def node_byte_size(self) -> int: - """The size of a node in bytes.""" - - @property - def search_tree_size(self) -> int: - """The size of the search tree.""" diff --git a/tests/reader_test.py b/tests/reader_test.py index b8c716eb..6e8d6329 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -53,18 +53,6 @@ # Directory holding the shared MaxMind DB test fixtures. _TEST_DATA_DIR = "tests/data/test-data" _DECODER_DB = f"{_TEST_DATA_DIR}/MaxMind-DB-test-decoder.mmdb" -# Valid arguments for the C extension Metadata. -_METADATA_FIELDS: dict[str, Any] = { - "binary_format_major_version": 2, - "binary_format_minor_version": 0, - "build_epoch": 1, - "database_type": "db", - "description": {}, - "ip_version": 4, - "languages": [], - "node_count": 1, - "record_size": 24, -} _PAYLOAD_TOO_LARGE = ( "^The MaxMind DB file's data section exceeds the maximum payload size$" ) @@ -1148,7 +1136,7 @@ def test_metadata_unknown_attribute(self) -> None: metadata = reader.metadata() with self.assertRaisesRegex( AttributeError, - r"'(maxminddb\.extension\.)?Metadata' object has no attribute 'blah'", + "'Metadata' object has no attribute 'blah'", ): metadata.blah # type: ignore[attr-defined] # noqa: B018 reader.close() @@ -1287,6 +1275,37 @@ def _reinitialize(self, reader: Any, path: str, mode: int) -> None: # noqa: ANN with _database_source(path, mode) as database: reader.__init__(database, mode) + def test_metadata_is_a_dataclass(self) -> None: + with open_database(_DECODER_DB, self.mode) as reader: + metadata = reader.metadata() + self.assertEqual(metadata, reader.metadata()) + with maxminddb.reader.Reader(_DECODER_DB) as reader: + expected = reader.metadata() + self.assertIs(type(metadata), maxminddb.reader.Metadata) + self.assertEqual(metadata, expected) + self.assertEqual(repr(metadata), repr(expected)) + self.assertTrue(repr(metadata).startswith("Metadata(")) + self.assertEqual( + dataclasses.asdict(metadata), + { + "binary_format_major_version": 2, + "binary_format_minor_version": 0, + "build_epoch": expected.build_epoch, + "database_type": "MaxMind DB Decoder Test", + "description": { + "en": "MaxMind DB Decoder Test database - " + "contains every MaxMind DB data type", + }, + "ip_version": 6, + "languages": ["en"], + "node_count": expected.node_count, + "record_size": 24, + }, + ) + # The description dict makes Metadata unhashable. + with self.assertRaises(TypeError): + hash(metadata) + def test_closed_metadata(self) -> None: reader = open_database( "tests/data/test-data/MaxMind-DB-test-decoder.mmdb", @@ -1524,18 +1543,8 @@ def test_record_subscript_works_at_runtime(self) -> None: class TestExtensionObjects(unittest.TestCase): """Objects in states that crashed the extension.""" - def test_new_metadata_requires_arguments(self) -> None: - metadata_class = maxminddb.extension.Metadata - with self.assertRaisesRegex(TypeError, "missing required argument"): - metadata_class.__new__(metadata_class) - - def test_metadata_missing_argument(self) -> None: - with self.assertRaisesRegex(TypeError, "missing required argument"): - maxminddb.extension.Metadata(binary_format_major_version=2) # type: ignore[call-arg] - - def test_metadata_too_many_arguments(self) -> None: - with self.assertRaisesRegex(TypeError, "at most 9"): - maxminddb.extension.Metadata(**_METADATA_FIELDS, unknown=1) # type: ignore[call-arg] + def test_metadata_is_the_pure_python_class(self) -> None: + self.assertIs(maxminddb.extension.Metadata, maxminddb.reader.Metadata) def test_iterate_uninitialized_reader(self) -> None: reader_class = maxminddb.extension.Reader @@ -1705,11 +1714,6 @@ def test_initialize_after_close_on_uninitialized_reader(self) -> None: with reader: self.assertIsNotNone(reader.get("::1.1.1.0")) - def test_reinitialize_metadata_changes_nothing(self) -> None: - metadata = maxminddb.extension.Metadata(**_METADATA_FIELDS) - metadata.__init__(**{**_METADATA_FIELDS, "record_size": 28}) # type: ignore[misc] - self.assertEqual(metadata.record_size, 24) - @unittest.skipUnless( hasattr(sys, "getrefcount") and not sysconfig.get_config_var("Py_GIL_DISABLED"), "needs CPython reference counts on a build with the GIL",