From 1e1140099836e343f8f1db0bca08c1d425c4ed69 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 4 Nov 2026 11:16:26 -0500 Subject: [PATCH] refactor(protocols)!: replace the pcapkit.protocols.__proto__ dict with ProtocolBase.__registry__ (#2763) - ProtocolBase.__registry__ lists every built-in as a ModuleDescriptor, so the map is whole before any protocol is imported; get_protocol() resolves a name in any case, one thread at a time, as racing first imports can deadlock. - ProtocolMeta.registry is a read-only view of the entries as stored, so AnyProtocol.registry imports nothing. - A concrete Protocol subclass names itself from Protocol.__init_subclass__, after its code= is accepted; an abstract one is skipped, a clash keeps the entry and warns, and a class meeting its own descriptor is silent. - register_protocol() replaces an entry and puts a built-in's shipped descriptor back; the foundation wrapper calls it and the name leases compare through it. - Readers move to the getter; pcapkit.protocols.__proto__ goes, with no alias. - Docs and conventions record the ruling. Tests: guard, registration, getter, view, cold laziness and a thread race, mutation-proven; old readers adapted. --- .../conventions/protocol-family-grouping.rst | 9 +- .../conventions/registry-protocol.rst | 35 +- docs/source/pcapkit/protocols/index.rst | 23 +- docs/source/pcapkit/protocols/protocol.rst | 7 + pcapkit/corekit/fields/misc.py | 26 +- pcapkit/corekit/sentinels.py | 3 +- pcapkit/foundation/reassembly/reassembly.py | 7 +- .../foundation/registry/protocols/protocol.py | 112 ++-- pcapkit/foundation/traceflow/traceflow.py | 7 +- pcapkit/protocols/__init__.py | 14 +- pcapkit/protocols/protocol.py | 271 +++++++++- pcapkit/protocols/registry.py | 33 -- tests/corekit/test_fields_misc.py | 4 +- .../test_dtls_reassembly_parity_2531_unit.py | 6 +- tests/foundation/registry/test_protocols.py | 61 ++- .../test_registrar_code_check_unit.py | 6 +- .../test_registry_name_restore_unit.py | 30 +- .../registry/test_registry_symmetry_unit.py | 12 +- ...est_registry_wrapper_coverage_2511_unit.py | 22 +- .../application/test_http_id_2524_unit.py | 6 +- .../test_http_member_names_2529_unit.py | 8 +- tests/protocols/application/test_ngap_unit.py | 4 +- .../application/test_ssl_tls_dtls_unit.py | 4 +- .../protocols/application/test_sslv3_unit.py | 2 +- tests/protocols/internet/test_esp_unit.py | 2 +- tests/protocols/internet/test_igmp_unit.py | 4 +- .../test_linux_sll_arphrd_registry_unit.py | 3 +- .../link/test_linux_sll_selector_unit.py | 4 +- tests/protocols/link/test_radiotap_unit.py | 3 +- tests/protocols/test_option_roundtrip_unit.py | 8 +- .../test_protocol_code_registration_unit.py | 9 +- .../test_protocol_packages_lazy_2730_unit.py | 85 +-- .../test_protocol_registry_2763_unit.py | 484 ++++++++++++++++++ tests/test_base_class_contract.py | 2 +- 34 files changed, 1053 insertions(+), 263 deletions(-) delete mode 100644 pcapkit/protocols/registry.py create mode 100644 tests/protocols/test_protocol_registry_2763_unit.py diff --git a/docs/source/contributing/conventions/protocol-family-grouping.rst b/docs/source/contributing/conventions/protocol-family-grouping.rst index e2f9b497bd..e6871439c4 100644 --- a/docs/source/contributing/conventions/protocol-family-grouping.rst +++ b/docs/source/contributing/conventions/protocol-family-grouping.rst @@ -291,7 +291,8 @@ A parsed RARP takes ``DRARP`` as its alias for operation 5, and ``__index__`` of RARP. A family-inclusive member is selected by any sibling's registry key (a key of -``pcapkit.protocols.__proto__``), because each expands to the shared family name +:attr:`ProtocolBase.__registry__ `), +because each expands to the shared family name (:ref:`below `). A narrow member is selected by its own name and by the key of each family whose ``id()`` lists it: ``HTTPv1`` and ``HTTPv2`` by ``HTTP``, and ``AH`` and ``ESP`` by ``IPsec`` and by ``IPv6_Ext`` where IPv4 carries @@ -465,8 +466,10 @@ layer packages, :mod:`~pcapkit.protocols.misc.pcap` and package does (:ref:`const-vendor-pairing`), and :mod:`pcapkit` re-exports :mod:`pcapkit.protocols` the same way. A protocol's own package keeps the imports above, since importing its ``protocol`` module imports its ``schema`` and ``data`` -anyway. :data:`pcapkit.protocols.__proto__` names every protocol class, so -:mod:`pcapkit.protocols.registry` builds it when it is first reached. +anyway. The by-name registry, +:attr:`ProtocolBase.__registry__ `, +names each built-in protocol by a :class:`~pcapkit.corekit.module.ModuleDescriptor` +rather than by its class, so it imports none of them (:issue:`2763`). **A family's shared pieces live in the family base's package**, and its members import them (:issue:`2534`): :class:`~pcapkit.protocols.link.c_tag.protocol.C_Tag` diff --git a/docs/source/contributing/conventions/registry-protocol.rst b/docs/source/contributing/conventions/registry-protocol.rst index 58dbaddc80..94f49908b9 100644 --- a/docs/source/contributing/conventions/registry-protocol.rst +++ b/docs/source/contributing/conventions/registry-protocol.rst @@ -758,8 +758,10 @@ public wrapper in :mod:`pcapkit.foundation.registry`, carrying the validation th established ones carry. A map's registration comes in two parts: 1. **A** ``register`` **or** ``register_`` **classmethod** on the class owning - the map -- ``register`` for ``__proto__``, ``register_`` for ``____`` - -- which writes it and warns when it displaces an entry. An inherited one counts + the map -- ``register`` for ``__proto__``, ``register_`` for ``____``, + and ``register_protocol`` for the by-name registry below, named for the class it + registers (:issue:`2763`) -- which writes it and warns when it displaces an + entry. An inherited one counts only when it is typed and validated for exactly that map's keys, which the owner chose over an override on every class (:issue:`2511`). So :class:`~pcapkit.protocols.transport.tcp.protocol.TCP` and @@ -838,8 +840,30 @@ depart from it: :func:`~pcapkit.foundation.registry.protocols.protocol.register_protocol` and :func:`~pcapkit.foundation.registry.protocols.protocol.register_protocol_code` are -outside the rule: the first writes :data:`pcapkit.protocols.__proto__`, keyed by -class name, and the second whichever maps its ``code`` names. +outside the rule: the first writes the by-name registry, keyed by class name, and the +second whichever maps its ``code`` names. + +**The by-name registry is a class-level map as well**, ruled on :issue:`2763`. +:attr:`ProtocolBase.__registry__ ` +maps each upper-cased class name to its class, and lists every built-in protocol as a +:class:`~pcapkit.corekit.module.ModuleDescriptor`, as a ``__proto__`` map names its +targets, so it is whole before any protocol module is imported and +:meth:`ProtocolBase.get_protocol ` +imports a class only when it resolves its entry, while +:attr:`ProtocolMeta.registry ` is a +read-only view of the entries as stored, read as ``AnyProtocol.registry`` and importing +nothing. The map and both readers stay on the internal base, and only the hook that +fills it is on the public one: a concrete subclass of +:class:`~pcapkit.protocols.protocol.Protocol` names itself from +:meth:`Protocol.__init_subclass__ `, +as the library's other public bases register theirs, and an abstract one, or a class +deriving :class:`~pcapkit.protocols.protocol.ProtocolBase` alone, does not. A name +already held keeps its entry, and the clash is +reported as a :exc:`~pcapkit.utilities.warnings.RegistryWarning`; only +:meth:`ProtocolBase.register_protocol `, +which the wrapper calls, replaces it. A built-in meeting its own descriptor there is no +clash. :file:`tests/protocols/test_protocol_registry_2763_unit.py` fails on a shipped +protocol class with no entry, or on an entry naming another module or class. :func:`~pcapkit.foundation.registry.protocols.transport.register_sctp_chunk` is the shape for a parser map: @@ -861,7 +885,8 @@ shape for a parser map: every protocol class for such a map, and fails on one that no wrapper's tag names unless its ``EXCUSED`` table gives the reason. It fails as well on a map whose owner defines no ``register*`` classmethod of its own, unless its ``INHERITED`` table names -the one typed for that map's keys. +the one typed for that map's keys, and its ``NAMED`` table holds the classmethod named +otherwise than for its map. **A protocol's table from a code to the layout read under it is such a map as well, and each layout is a concrete schema.** Ruled in review of :pr:`2657` (:issue:`2576`): the diff --git a/docs/source/pcapkit/protocols/index.rst b/docs/source/pcapkit/protocols/index.rst index 112d21f2b5..530bd4c16f 100644 --- a/docs/source/pcapkit/protocols/index.rst +++ b/docs/source/pcapkit/protocols/index.rst @@ -196,19 +196,24 @@ from given information. The class hierarchy of :mod:`pcapkit.protocols`: Protocol Registry ----------------- -.. module:: pcapkit.protocols.registry +The registry of protocol classes by name is +:attr:`ProtocolBase.__registry__ `. +:attr:`ProtocolMeta.registry ` reads it +as ``AnyProtocol.registry``, a read-only view importing nothing, and +:meth:`ProtocolBase.get_protocol ` +looks a name up in it and returns the class. -.. autodata:: pcapkit.protocols.__proto__ - :no-value: +.. versionremoved:: 1.6.0 - :mod:`pcapkit.protocols.registry` builds it the first time it is reached, and - imports every protocol class :mod:`pcapkit.protocols` exports to do so - (:issue:`2730`). + ``pcapkit.protocols.__proto__``, with no alias: read + :attr:`ProtocolBase.__registry__ ` + through :meth:`ProtocolBase.get_protocol + ` instead (:issue:`2763`). - .. seealso:: +.. seealso:: - Please refer to :func:`pcapkit.foundation.registry.protocols.protocol.register_protocol` - for more information. + Please refer to :func:`pcapkit.foundation.registry.protocols.protocol.register_protocol` + for more information. Header Schema ------------- diff --git a/docs/source/pcapkit/protocols/protocol.rst b/docs/source/pcapkit/protocols/protocol.rst index 6830dc1ca8..aaf0239a34 100644 --- a/docs/source/pcapkit/protocols/protocol.rst +++ b/docs/source/pcapkit/protocols/protocol.rst @@ -203,6 +203,11 @@ Internal Definitions .. automethod:: __init_subclass__ + .. autoattribute:: __registry__ + :no-value: + .. automethod:: register_protocol + .. automethod:: get_protocol + :class:`Protocol` inherits the other members of this class, and they are documented under it above; each group below links to its entries. @@ -345,6 +350,8 @@ Internal Definitions :no-members: :show-inheritance: + .. autoproperty:: registry + .. autodata:: pcapkit.protocols.protocol.OUT_OF_BAND_KEYWORDS :no-value: diff --git a/pcapkit/corekit/fields/misc.py b/pcapkit/corekit/fields/misc.py index fe371b9d2b..e58a20edc9 100644 --- a/pcapkit/corekit/fields/misc.py +++ b/pcapkit/corekit/fields/misc.py @@ -264,9 +264,10 @@ def protocol(self, protocol: 'Type[_TP] | str') -> 'None': """Set payload protocol. Arguments: - protocol: Payload protocol. A :obj:`str` is resolved against the - :data:`pcapkit.protocols.__proto__` registry, case-insensitively, - and an unresolved name leaves the payload as + protocol: Payload protocol. A :obj:`str` is resolved with + :meth:`ProtocolBase.get_protocol + `, + case-insensitively, and an unresolved name leaves the payload as :class:`~pcapkit.protocols.misc.raw.protocol.Raw`. Warns: @@ -275,18 +276,17 @@ def protocol(self, protocol: 'Type[_TP] | str') -> 'None': """ if isinstance(protocol, str): - from pcapkit.protocols import __proto__ # pylint: disable=import-outside-toplevel - - # NOTE: The registry is keyed on the upper-cased class name, both - # when it is seeded (the loop over ``__all__`` in - # :mod:`pcapkit.protocols`) and when - # ``pcapkit.foundation.registry.protocols.protocol.register_protocol`` adds to - # it, so a name looked up in any other case would miss every time -- - # and a miss leaves ``_protocol`` as :obj:`None`, which the property - # above resolves to :class:`~pcapkit.protocols.misc.raw.protocol.Raw`. So + from pcapkit.protocols.protocol import \ + ProtocolBase # pylint: disable=import-outside-toplevel + + # NOTE: The registry is keyed on the upper-cased class name, and the + # getter upper-cases the name it is given, so a name in any other + # case still resolves -- where a miss leaves ``_protocol`` as + # :obj:`None`, which the property above resolves to + # :class:`~pcapkit.protocols.misc.raw.protocol.Raw`. So # ``PayloadField(protocol='http')`` would yield a raw payload instead # of HTTP, with nothing to say so (#787). - resolved = __proto__.get(protocol.upper()) + resolved = ProtocolBase.get_protocol(protocol) # NOTE: Warned rather than left silent, and warned rather than # raised. Unlike the registry lookups that dispatch on a code read diff --git a/pcapkit/corekit/sentinels.py b/pcapkit/corekit/sentinels.py index 2c9f7371ab..4ecb62f922 100644 --- a/pcapkit/corekit/sentinels.py +++ b/pcapkit/corekit/sentinels.py @@ -599,7 +599,8 @@ def __bool__(self) -> 'Literal[False]': return False -#: MissingType: Default for reading :data:`pcapkit.protocols.__proto__` in +#: MissingType: Default for reading :attr:`ProtocolBase.__registry__ +#: ` in #: :mod:`pcapkit.foundation.registry.protocols.protocol`, recorded as what a code #: registration displaced when the name was new, so that handing the code back #: removes the name rather than restoring an entry. Private by documentation diff --git a/pcapkit/foundation/reassembly/reassembly.py b/pcapkit/foundation/reassembly/reassembly.py index 0c1f268958..bd08571c79 100644 --- a/pcapkit/foundation/reassembly/reassembly.py +++ b/pcapkit/foundation/reassembly/reassembly.py @@ -16,6 +16,7 @@ from typing import TYPE_CHECKING, Generic, Type, TypeVar, cast from pcapkit.protocols.misc.raw.protocol import Raw +from pcapkit.protocols.protocol import ProtocolBase from pcapkit.utilities.exceptions import FieldValueError, UnsupportedCall from pcapkit.utilities.logging import get_logger from pcapkit.utilities.pickle import ReduceAsProtocol2 @@ -41,7 +42,6 @@ from pcapkit.corekit.infoclass import Info from pcapkit.corekit.module import ModuleDescriptor - from pcapkit.protocols.protocol import ProtocolBase CallbackFn = Callable[[List[_DT]], None] @@ -78,10 +78,7 @@ def protocol(cls) -> 'Type[ProtocolBase]': """Protocol of current reassembly object.""" if hasattr(cls, '__protocol_type__'): return cls.__protocol_type__ - # NOTE: read here, not on import, which would build it and import every protocol (#2730). - from pcapkit.protocols import \ - __proto__ as protocol_registry # pylint: disable=import-outside-toplevel - return protocol_registry.get(cls.name.upper(), Raw) + return ProtocolBase.get_protocol(cls.name, Raw) @property def registry(cls) -> 'dict[str, ModuleDescriptor[ReassemblyBase] | Type[ReassemblyBase]]': diff --git a/pcapkit/foundation/registry/protocols/protocol.py b/pcapkit/foundation/registry/protocols/protocol.py index 94fd57f4fe..41b10cbebf 100644 --- a/pcapkit/foundation/registry/protocols/protocol.py +++ b/pcapkit/foundation/registry/protocols/protocol.py @@ -6,7 +6,8 @@ This module provides the protocol registries the per-layer modules beside it build on: :func:`register_protocol` for the by-name registry -:data:`pcapkit.protocols.__proto__`, :func:`register_protocol_code` for the +:attr:`ProtocolBase.__registry__ `, +:func:`register_protocol_code` for the next-layer registries a ``code`` names, and the validation and by-name bookkeeping every registrar shares. @@ -19,16 +20,17 @@ """ import collections.abc import enum +import sys import threading -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, cast import aenum +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.corekit.sentinels import MISSING from pcapkit.protocols.protocol import ProtocolBase from pcapkit.utilities.exceptions import RegistryError from pcapkit.utilities.logging import get_logger -from pcapkit.utilities.warnings import RegistryWarning, warn if TYPE_CHECKING: from typing import Any, Iterator, Optional, Type @@ -43,19 +45,25 @@ logger = get_logger(__name__) -# NOTE: pcapkit.protocols.__proto__ +# NOTE: pcapkit.protocols.protocol.ProtocolBase.__registry__ def register_protocol(protocol: 'Type[ProtocolBase]') -> 'None': """Register a protocol class. The protocol class must be a subclass of :class:`~pcapkit.protocols.protocol.ProtocolBase`, and will be registered to - the :data:`pcapkit.protocols.__proto__` registry. + the by-name registry :attr:`ProtocolBase.__registry__ + ` through + :meth:`ProtocolBase.register_protocol + `, replacing the + entry its name holds. The registry is keyed on ``protocol.__name__.upper()``, which nothing makes unique: two protocol classes of the same name, such as a user's class - named after a shipped one, compete for one key. Registering one of them - replaces whichever was there, and the replacement is observable through - every reader of the registry, e.g. :meth:`Protocol.expand_comp + named after a shipped one, compete for one key. Defining the second as a + :class:`~pcapkit.protocols.protocol.Protocol` subclass keeps the first, and + registering it here replaces whichever was there, and the + replacement is observable through every reader of the registry, e.g. + :meth:`Protocol.expand_comp `, which resolves a bare protocol name through it. Per :issue:`675` the overwrite is reported, matching :meth:`Protocol.register @@ -80,6 +88,8 @@ class rather than supplied by a caller, and this function is the funnel filter would then hide the name collision this warning exists to surface. The sibling ``register`` methods across the package, each keyed on a caller-supplied ``code``, apply the same identity criterion (:issue:`718`). + A built-in's :class:`~pcapkit.corekit.module.ModuleDescriptor` entry is + the class it names for that criterion. Making the key itself unique would resolve the collision rather than merely reporting it, but it is a registry-format change that the bare-name @@ -116,43 +126,44 @@ class rather than supplied by a caller, and this function is the funnel :class:`~pcapkit.protocols.application.httpv2.protocol.HTTPv2`, so registering one keeps its own key and no longer displaces :class:`~pcapkit.protocols.application.http.protocol.HTTP` under ``'HTTP'`` - (:issue:`2529`). + (:issue:`2529`). The registry is + :attr:`ProtocolBase.__registry__ `, + written by :meth:`ProtocolBase.register_protocol + `, where it was + ``pcapkit.protocols.__proto__`` (:issue:`2763`). """ - # NOTE: read here, not on import, which would build it and import every protocol (#2730). - from pcapkit.protocols import \ - __proto__ as protocol_registry # pylint: disable=import-outside-toplevel - if not isinstance(protocol, type): raise RegistryError(f'protocol must be a class, not {protocol!r}') if not issubclass(protocol, ProtocolBase): raise RegistryError(f'protocol must be a Protocol subclass, not {protocol!r}') - name = protocol.__name__.upper() - incumbent = protocol_registry.get(name) - if incumbent is not None and incumbent is not protocol: - incumbent_repr, protocol_repr = repr(incumbent), repr(protocol) - if incumbent_repr == protocol_repr: - # #710: two *distinct* objects whose repr() -- for an ordinary - # class, its module plus qualname -- happens to coincide, e.g. a - # factory that builds a fresh closure-local class of the same - # name on every call. Appending __module__/__qualname__ would not - # help here: that is exactly what the coinciding repr() already - # renders, so both sides would still print identically. id() is - # the fallback that actually differs, so it is only added in this - # branch -- the common case, two genuinely different classes, - # keeps the plain repr() and stays free of the extra noise. - incumbent_repr = f'{incumbent_repr} (id={id(incumbent):#x})' - protocol_repr = f'{protocol_repr} (id={id(protocol):#x})' - warn(f'protocol {name} already registered, overwriting {incumbent_repr} ' - f'with {protocol_repr}', RegistryWarning) - - protocol_registry[name] = protocol + ProtocolBase.register_protocol(protocol) logger.debug('registered protocol: %s', protocol.__name__) +def _denotes(entry: 'Any', protocol: 'Any') -> 'bool': + """Whether a by-name registry ``entry`` is ``protocol``, or a descriptor naming it. + + A :class:`~pcapkit.corekit.module.ModuleDescriptor` names the class its + module binds under its name, read off :data:`sys.modules` rather than + imported: a class that is defined has its module loaded already. + + Args: + entry: Registry entry, a displaced one included. + protocol: Protocol class. + + """ + if entry is protocol: + return True + if isinstance(entry, ModuleDescriptor): + module = sys.modules.get(entry.module) + return module is not None and getattr(module, cast('str', entry.name), None) is protocol + return False + + class _NameLease: - """A :data:`pcapkit.protocols.__proto__` entry a code registration put there. + """A by-name registry entry a code registration put there. Each code registrar below ends by naming its class in the by-name registry, and :mod:`pcapkit.foundation.registry` has no ``unregister``: a code is @@ -172,7 +183,7 @@ class _NameLease: __slots__ = ('name', 'protocol', 'displaced', 'codes') def __init__(self, name: 'str', protocol: 'Type[ProtocolBase]', displaced: 'Any') -> 'None': - #: The key written in :data:`pcapkit.protocols.__proto__`. + #: The key written in the by-name registry. self.name = name #: The class written under it. self.protocol = protocol @@ -233,9 +244,7 @@ def _release_name(target: 'tuple[type, Any]') -> 'None': target: The ``(destination, code)`` being registered over. """ - from pcapkit.protocols import \ - __proto__ as protocol_registry # pylint: disable=import-outside-toplevel - + protocol_registry = ProtocolBase.__registry__ lease = _CODE_LEASES.pop(target, None) if lease is None: return @@ -250,11 +259,11 @@ def _release_name(target: 'tuple[type, Any]') -> 'None': del stack[index] if index < len(stack): above = stack[index] - if above.displaced is lease.protocol: + if _denotes(above.displaced, lease.protocol): above.displaced = lease.displaced - if above.displaced is above.protocol: + if _denotes(above.displaced, above.protocol): _fold_lease(stack, index) - elif protocol_registry.get(lease.name) is lease.protocol: + elif _denotes(protocol_registry.get(lease.name), lease.protocol): if lease.displaced is MISSING: protocol_registry.pop(lease.name, None) else: @@ -265,28 +274,27 @@ def _release_name(target: 'tuple[type, Any]') -> 'None': def _register_name(protocol: 'Type[ProtocolBase]', *targets: 'tuple[type, Any]') -> 'None': - """Name ``protocol`` in :data:`pcapkit.protocols.__proto__` for ``targets``. + """Name ``protocol`` in the by-name registry for ``targets``. The tail of every code registrar: :func:`register_protocol`, preceded by releasing whatever each target code held before, and followed by a lease when the write changed the name, so that registering the code back undoes - it (:issue:`1505`). A name already showing ``protocol`` -- shipped, defined - by a :class:`~pcapkit.protocols.protocol.Protocol` subclass, or - registered directly -- is not leased, so no restore takes it away; one - showing it through a lease extends that lease to ``targets``. + it (:issue:`1505`). A name already showing ``protocol`` -- shipped, named + when its concrete :class:`~pcapkit.protocols.protocol.Protocol` subclass was + defined, or registered directly -- is not + leased, so no restore takes it away; one showing it through a lease + extends that lease to ``targets``. Args: protocol: Protocol class just registered -- and so validated -- at ``targets``. *targets: The ``(destination, code)`` pairs it was registered at. """ - from pcapkit.protocols import \ - __proto__ as protocol_registry # pylint: disable=import-outside-toplevel - + protocol_registry = ProtocolBase.__registry__ name = protocol.__name__.upper() with _LEASE_LOCK: stack = _NAME_LEASES.get(name) - if stack and stack[-1].protocol is protocol and protocol_registry.get(name) is protocol: + if stack and stack[-1].protocol is protocol and _denotes(protocol_registry.get(name), protocol): current = stack[-1] # type: Optional[_NameLease] else: current = None @@ -296,11 +304,11 @@ def _register_name(protocol: 'Type[ProtocolBase]', *targets: 'tuple[type, Any]') incumbent = protocol_registry.get(name, MISSING) register_protocol(protocol) - if protocol_registry.get(name) is not protocol: + if not _denotes(protocol_registry.get(name), protocol): return stack = _NAME_LEASES.get(name) - if incumbent is not protocol: + if not _denotes(incumbent, protocol): lease = _NameLease(name, protocol, incumbent) _NAME_LEASES.setdefault(name, []).append(lease) elif stack and stack[-1].protocol is protocol: diff --git a/pcapkit/foundation/traceflow/traceflow.py b/pcapkit/foundation/traceflow/traceflow.py index b94d71f38d..024bf9da25 100644 --- a/pcapkit/foundation/traceflow/traceflow.py +++ b/pcapkit/foundation/traceflow/traceflow.py @@ -22,6 +22,7 @@ from pcapkit.corekit.module import ModuleDescriptor from pcapkit.dumpkit.common import make_dumper from pcapkit.protocols.misc.raw.protocol import Raw +from pcapkit.protocols.protocol import ProtocolBase from pcapkit.utilities.exceptions import FileExists, RegistryError, UnsupportedCall, stacklevel from pcapkit.utilities.logging import get_logger from pcapkit.utilities.warnings import FileWarning, FormatWarning, RegistryWarning, warn @@ -48,7 +49,6 @@ from typing_extensions import Literal, Self from pcapkit.corekit.infoclass import Info - from pcapkit.protocols.protocol import ProtocolBase CallbackFn = Callable[[_IT], None] @@ -79,10 +79,7 @@ def protocol(cls) -> 'Type[ProtocolBase]': """Protocol of current object.""" if hasattr(cls, '__protocol_type__'): return cls.__protocol_type__ - # NOTE: read here, not on import, which would build it and import every protocol (#2730). - from pcapkit.protocols import \ - __proto__ as protocol_registry # pylint: disable=import-outside-toplevel - return protocol_registry.get(cls.name.upper(), Raw) + return ProtocolBase.get_protocol(cls.name, Raw) @property def registry(cls) -> 'dict[str, ModuleDescriptor[TraceFlowBase] | Type[TraceFlowBase]]': diff --git a/pcapkit/protocols/__init__.py b/pcapkit/protocols/__init__.py index fe4bba1015..c34db90ca4 100644 --- a/pcapkit/protocols/__init__.py +++ b/pcapkit/protocols/__init__.py @@ -8,9 +8,9 @@ :mod:`pcapkit.protocols` is a collection of all protocol families, with detailed implementation and methods. -Each protocol class, and :data:`__proto__`, is imported the first time it is -reached rather than with this package, through -:func:`~pcapkit.utilities.importlib.lazy_exports` (:issue:`2730`). +Each protocol class is imported the first time it is reached rather than with +this package, through :func:`~pcapkit.utilities.importlib.lazy_exports` +(:issue:`2730`). """ from typing import TYPE_CHECKING @@ -31,10 +31,6 @@ from pcapkit.utilities.importlib import lazy_exports if TYPE_CHECKING: - from typing import Type - - from pcapkit.protocols.protocol import ProtocolBase - # Utility Classes for Protocols from pcapkit.protocols.misc import * @@ -44,9 +40,6 @@ from pcapkit.protocols.transport import * from pcapkit.protocols.application import * - #: dict[str, ~typing.Type[~pcapkit.protocols.protocol.ProtocolBase]]: Protocol registry. - __proto__ = {} # type: dict[str, Type[ProtocolBase]] - __all__ = [ # PCAP Headers 'Header', 'Frame', @@ -99,5 +92,4 @@ # NOTE: ``Protocol`` is bound above, and named here so that ``dir()`` lists it. __getattr__, __dir__ = lazy_exports(__name__, globals(), { 'Protocol': ('protocol', 'Protocol'), - '__proto__': ('registry', '__proto__'), }, packages=('misc', 'link', 'internet', 'transport', 'application')) diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index 156ad3aa5f..d80f0b3762 100644 --- a/pcapkit/protocols/protocol.py +++ b/pcapkit/protocols/protocol.py @@ -36,6 +36,7 @@ import string import struct import textwrap +import threading import types import urllib.parse from typing import TYPE_CHECKING, Any, Generic, Optional, Type, TypeVar, cast, overload @@ -468,10 +469,30 @@ def _sibling_model(cls: 'Type[ProtocolBase]', role: 'str') -> 'Any': class ProtocolMeta(abc.ABCMeta): """Metaclass of :class:`ProtocolBase`, and so of every protocol class. - It adds no behaviour of its own over :class:`abc.ABCMeta`. + Over :class:`abc.ABCMeta`, it adds the class property :attr:`registry`. """ + @property + def registry(cls) -> 'types.MappingProxyType[str, ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]]': + """Read-only view of the by-name protocol registry. + + It is :attr:`ProtocolBase.__registry__`, with each entry as stored: a + built-in protocol is the :class:`~pcapkit.corekit.module.ModuleDescriptor` + naming it, so reading the view imports nothing. Look a class up with + :meth:`ProtocolBase.get_protocol`, which resolves a descriptor. + + Note: + It is one map, not a per-class one, so every protocol class reads + the same entries, and the view shows each later registration. It + is defined on this metaclass, so it is a class attribute and + **not** an instance one. + + .. versionadded:: 1.6.0 + + """ + return types.MappingProxyType(ProtocolBase.__registry__) + class ProtocolBase(Generic[_PT, _ST], metaclass=ProtocolMeta): """Abstract base class for all protocol families. @@ -542,6 +563,94 @@ class ProtocolBase(Generic[_PT, _ST], metaclass=ProtocolMeta): _shipped_proto: 'types.MappingProxyType[Any, ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]]' = \ types.MappingProxyType({}) + #: Protocol registry by name, holding each protocol class under its + #: upper-cased class name, ``'IPV4'`` for :class:`~pcapkit.protocols.internet.ipv4.protocol.IPv4`. + #: Each built-in protocol is a :class:`~pcapkit.corekit.module.ModuleDescriptor` + #: here, so the map names every one before any is imported, and + #: :meth:`get_protocol` imports a class only when it resolves its entry + #: (:issue:`2763`). :meth:`Protocol.__init_subclass__` adds each concrete + #: :class:`Protocol` subclass as it is defined, :meth:`register_protocol` + #: replaces an entry, and :attr:`ProtocolMeta.registry` is a read-only view. + #: + #: .. versionadded:: 1.6.0 + __registry__: 'dict[str, ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]]' = { + 'AH': ModuleDescriptor('pcapkit.protocols.internet.ah.protocol', 'AH'), + 'ARP': ModuleDescriptor('pcapkit.protocols.link.arp.protocol', 'ARP'), + 'C_TAG': ModuleDescriptor('pcapkit.protocols.link.c_tag.protocol', 'C_Tag'), + 'DCCP': ModuleDescriptor('pcapkit.protocols.transport.dccp.protocol', 'DCCP'), + 'DRARP': ModuleDescriptor('pcapkit.protocols.application.drarp.protocol', 'DRARP'), + 'DTLS': ModuleDescriptor('pcapkit.protocols.application.dtls.protocol', 'DTLS'), + 'ESP': ModuleDescriptor('pcapkit.protocols.internet.esp.protocol', 'ESP'), + 'ETHERNET': ModuleDescriptor('pcapkit.protocols.link.ethernet.protocol', 'Ethernet'), + 'FRAD': ModuleDescriptor('pcapkit.protocols.link.frad.protocol', 'FRAD'), + 'FRAME': ModuleDescriptor('pcapkit.protocols.misc.pcap.frame.protocol', 'Frame'), + 'FTP': ModuleDescriptor('pcapkit.protocols.application.ftp.protocol', 'FTP'), + 'FTP_DATA': ModuleDescriptor('pcapkit.protocols.application.ftp_data.protocol', 'FTP_DATA'), + 'GRE': ModuleDescriptor('pcapkit.protocols.link.gre.protocol', 'GRE'), + 'HEADER': ModuleDescriptor('pcapkit.protocols.misc.pcap.header.protocol', 'Header'), + 'HIP': ModuleDescriptor('pcapkit.protocols.internet.hip.protocol', 'HIP'), + 'HOPOPT': ModuleDescriptor('pcapkit.protocols.internet.hopopt.protocol', 'HOPOPT'), + 'HTTP': ModuleDescriptor('pcapkit.protocols.application.http.protocol', 'HTTP'), + 'HTTPV1': ModuleDescriptor('pcapkit.protocols.application.httpv1.protocol', 'HTTPv1'), + 'HTTPV2': ModuleDescriptor('pcapkit.protocols.application.httpv2.protocol', 'HTTPv2'), + 'ICMP': ModuleDescriptor('pcapkit.protocols.internet.icmp.protocol', 'ICMP'), + 'ICMPV4': ModuleDescriptor('pcapkit.protocols.internet.icmpv4.protocol', 'ICMPv4'), + 'ICMPV6': ModuleDescriptor('pcapkit.protocols.internet.icmpv6.protocol', 'ICMPv6'), + 'IEEE802_11': ModuleDescriptor('pcapkit.protocols.link.ieee802_11.protocol', 'IEEE802_11'), + 'IGMP': ModuleDescriptor('pcapkit.protocols.internet.igmp.protocol', 'IGMP'), + 'IGMPV1': ModuleDescriptor('pcapkit.protocols.internet.igmpv1.protocol', 'IGMPv1'), + 'IGMPV2': ModuleDescriptor('pcapkit.protocols.internet.igmpv2.protocol', 'IGMPv2'), + 'IGMPV3': ModuleDescriptor('pcapkit.protocols.internet.igmpv3.protocol', 'IGMPv3'), + 'INARP': ModuleDescriptor('pcapkit.protocols.link.inarp.protocol', 'InARP'), + 'IP': ModuleDescriptor('pcapkit.protocols.internet.ip.protocol', 'IP'), + 'IPSEC': ModuleDescriptor('pcapkit.protocols.internet.ipsec.protocol', 'IPsec'), + 'IPV4': ModuleDescriptor('pcapkit.protocols.internet.ipv4.protocol', 'IPv4'), + 'IPV6': ModuleDescriptor('pcapkit.protocols.internet.ipv6.protocol', 'IPv6'), + 'IPV6_EXT': ModuleDescriptor('pcapkit.protocols.internet.ipv6_ext.protocol', 'IPv6_Ext'), + 'IPV6_FRAG': ModuleDescriptor('pcapkit.protocols.internet.ipv6_frag.protocol', 'IPv6_Frag'), + 'IPV6_OPTS': ModuleDescriptor('pcapkit.protocols.internet.ipv6_opts.protocol', 'IPv6_Opts'), + 'IPV6_ROUTE': ModuleDescriptor('pcapkit.protocols.internet.ipv6_route.protocol', 'IPv6_Route'), + 'IPX': ModuleDescriptor('pcapkit.protocols.internet.ipx.protocol', 'IPX'), + 'IP_PSEUDO': ModuleDescriptor('pcapkit.protocols.misc.checksum.ip_pseudo.protocol', 'IP_Pseudo'), + 'L2TP': ModuleDescriptor('pcapkit.protocols.link.l2tp.protocol', 'L2TP'), + 'L2TPV2': ModuleDescriptor('pcapkit.protocols.link.l2tpv2.protocol', 'L2TPv2'), + 'LINUXSLL': ModuleDescriptor('pcapkit.protocols.link.linux_sll.protocol', 'LinuxSLL'), + 'LINUXSLL1': ModuleDescriptor('pcapkit.protocols.link.linux_sll1.protocol', 'LinuxSLL1'), + 'LINUXSLL2': ModuleDescriptor('pcapkit.protocols.link.linux_sll2.protocol', 'LinuxSLL2'), + 'LOOPBACK': ModuleDescriptor('pcapkit.protocols.link.loopback.protocol', 'Loopback'), + 'MH': ModuleDescriptor('pcapkit.protocols.internet.mh.protocol', 'MH'), + 'NETLINK': ModuleDescriptor('pcapkit.protocols.link.netlink.protocol', 'Netlink'), + 'NGAP': ModuleDescriptor('pcapkit.protocols.application.ngap.protocol', 'NGAP'), + 'NOPAYLOAD': ModuleDescriptor('pcapkit.protocols.misc.no_payload.protocol', 'NoPayload'), + 'OSPF': ModuleDescriptor('pcapkit.protocols.application.ospf.protocol', 'OSPF'), + 'PCAPNG': ModuleDescriptor('pcapkit.protocols.misc.pcapng.protocol', 'PCAPNG'), + 'RADIOTAP': ModuleDescriptor('pcapkit.protocols.link.radiotap.protocol', 'Radiotap'), + 'RARP': ModuleDescriptor('pcapkit.protocols.application.rarp.protocol', 'RARP'), + 'RAW': ModuleDescriptor('pcapkit.protocols.misc.raw.protocol', 'Raw'), + 'SCTP': ModuleDescriptor('pcapkit.protocols.transport.sctp.protocol', 'SCTP'), + 'SSL': ModuleDescriptor('pcapkit.protocols.application.ssl.protocol', 'SSL'), + 'SSLV2': ModuleDescriptor('pcapkit.protocols.application.sslv2.protocol', 'SSLv2'), + 'SSLV3': ModuleDescriptor('pcapkit.protocols.application.sslv3.protocol', 'SSLv3'), + 'S_TAG': ModuleDescriptor('pcapkit.protocols.link.s_tag.protocol', 'S_Tag'), + 'TCP': ModuleDescriptor('pcapkit.protocols.transport.tcp.protocol', 'TCP'), + 'TLS': ModuleDescriptor('pcapkit.protocols.application.tls.protocol', 'TLS'), + 'UDP': ModuleDescriptor('pcapkit.protocols.transport.udp.protocol', 'UDP'), + 'VLAN': ModuleDescriptor('pcapkit.protocols.link.vlan.protocol', 'VLAN'), + } + + #: Read-only copy of :attr:`__registry__` as shipped, which + #: :meth:`register_protocol` reads to put a built-in's descriptor back + #: rather than its class, as :attr:`_shipped_proto` serves + #: :meth:`register` (:issue:`1504`). + _shipped_registry: 'types.MappingProxyType[str, ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]]' = \ + types.MappingProxyType(dict(__registry__)) + + #: Serialises :meth:`get_protocol`'s resolution of a descriptor. Two threads + #: importing protocol modules from different points can deadlock, one + #: holding a package and the other a module inside it, so a first import is + #: made by one thread at a time (:issue:`2763`). + _registry_lock: 'threading.RLock' = threading.RLock() + #: Construction keywords this protocol consumes out of ``**kwargs`` instead #: of declaring as a parameter, e.g. with ``kwargs.get('spam')`` in #: :meth:`read` -- as :meth:`ESP.read ` @@ -1056,8 +1165,8 @@ def expand_comp(value: 'str | ProtocolBase | Type[ProtocolBase]') -> 'tuple': protocol class itself, and the protocol names in upper case obtained from :meth:`Protocol.id `. 3. If ``value`` is :obj:`str`, the method will attempt to search for - the existing registered protocol class from - :data:`pcapkit.protocols.__proto__` and follow **step 2**; otherwise, + the existing registered protocol class with + :meth:`ProtocolBase.get_protocol` and follow **step 2**; otherwise, return the value in upper case alone. Args: @@ -1068,13 +1177,10 @@ def expand_comp(value: 'str | ProtocolBase | Type[ProtocolBase]') -> 'tuple': comp = (value, *(name.upper() for name in value.id())) elif isinstance(value, ProtocolBase): comp = (type(value), *(name.upper() for name in value.id())) + elif (proto := ProtocolBase.get_protocol(value)) is not None: + comp = (proto, *(name.upper() for name in proto.id())) else: - from pcapkit.protocols import __proto__ as protocols_registry # pylint: disable=import-outside-toplevel # isort: skip - - if (proto := protocols_registry.get(value.upper())) is not None: - comp = (proto, *(name.upper() for name in proto.id())) - else: - comp = (value.upper(),) + comp = (value.upper(),) return comp @classmethod @@ -1163,6 +1269,101 @@ class object, or a :class:`~pcapkit.corekit.module.ModuleDescriptor` f'{incumbent!r} with {entry!r}', RegistryWarning) cls.__proto__[code] = entry + @classmethod + def register_protocol(cls, protocol: 'Type[ProtocolBase]') -> 'None': + """Register a protocol class in :attr:`__registry__`, replacing what its name holds. + + The key is ``protocol.__name__.upper()``. Defining a concrete + :class:`Protocol` subclass registers it already, but keeps an entry its + name holds, so this is how to replace one, or to register a class that + derives :class:`ProtocolBase` alone. + + Arguments: + protocol: :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass + + Raises: + pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a + class, or not a :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass. + + Warns: + pcapkit.utilities.warnings.RegistryWarning: If the name holds a + different class, naming it and its replacement, and their ids + where the two read the same. A + :class:`~pcapkit.corekit.module.ModuleDescriptor` entry is + resolved to compare, so registering the class it names is silent + and leaves the descriptor in place. + + Note: + A built-in protocol is stored as the descriptor it shipped as, per + :attr:`_shipped_registry`, rather than as its class, so undoing an + override leaves the entry as shipped, to be imported on lookup. + + .. versionadded:: 1.6.0 + + """ + if not isinstance(protocol, type): + raise RegistryError(f'protocol must be a class, not {protocol!r}') + if not issubclass(protocol, ProtocolBase): + raise RegistryError(f'protocol must be a Protocol subclass, not {protocol!r}') + + name = protocol.__name__.upper() + incumbent = ProtocolBase.get_protocol(name) + if incumbent is protocol: + return + if incumbent is not None: + incumbent_repr, protocol_repr = repr(incumbent), repr(protocol) + if incumbent_repr == protocol_repr: + # NOTE: #710: two distinct classes whose repr() -- module plus + # qualname -- coincide, e.g. a factory building a fresh class of + # the same name on every call; id() is what tells them apart. + incumbent_repr = f'{incumbent_repr} (id={id(incumbent):#x})' + protocol_repr = f'{protocol_repr} (id={id(protocol):#x})' + warn(f'protocol {name} already registered, overwriting {incumbent_repr} ' + f'with {protocol_repr}', RegistryWarning) + shipped = ProtocolBase._shipped_registry.get(name) + if isinstance(shipped, ModuleDescriptor) and \ + (shipped.module, shipped.name) == (protocol.__module__, protocol.__name__): + ProtocolBase.__registry__[name] = shipped + else: + ProtocolBase.__registry__[name] = protocol + + @overload # pragma: no cover + @classmethod + def get_protocol(cls, name: 'str') -> 'Optional[Type[ProtocolBase]]': ... + @overload # pragma: no cover + @classmethod + def get_protocol(cls, name: 'str', default: '_VT') -> 'Type[ProtocolBase] | _VT': ... + + @classmethod + def get_protocol(cls, name: 'str', default: 'Any' = None) -> 'Any': + """Look a protocol class up in :attr:`__registry__` by name. + + Arguments: + name: protocol name, in any case: ``'ipv4'`` finds + :class:`~pcapkit.protocols.internet.ipv4.protocol.IPv4` + default: what to return where no entry has the name + + Returns: + The class registered under ``name.upper()``, imported where its entry + is a :class:`~pcapkit.corekit.module.ModuleDescriptor`, or + ``default``. The entry is left as it is, so a descriptor is resolved + again on each lookup and follows a reloaded module. + + Raises: + pcapkit.utilities.exceptions.ProtocolError: If a descriptor names a + class its module does not define. + + .. versionadded:: 1.6.0 + + """ + entry = ProtocolBase.__registry__.get(name.upper()) + if entry is None: + return default + if isinstance(entry, ModuleDescriptor): + with ProtocolBase._registry_lock: + return entry.klass + return entry + @classmethod def from_schema(cls, schema: '_ST | dict[str, Any]') -> 'Self': """Create protocol instance from schema. @@ -3041,7 +3242,7 @@ class Protocol(ProtocolBase, Generic[_PT, _ST]): The built-in protocol families derive from :class:`ProtocolBase`, so a built-in protocol is not an instance of this class, where every one - was; and a subclass is registered in :data:`pcapkit.protocols.__proto__` + was; and a subclass is registered in ``pcapkit.protocols.__proto__`` when it is defined. """ @@ -3078,19 +3279,47 @@ def __init_subclass__(cls, /, schema: 'Optional[Type[_ST]]' = None, :class:`~pcapkit.protocols.misc.raw.data.Raw` classes will be used. An explicit ``schema`` or ``data`` keyword is used as given. - This method also registers the subclass to the protocol registry, - i.e., :attr:`pcapkit.protocols.__proto__`. That registration is - unconditional -- it is the name-keyed identity registry, unrelated to - the ``code`` keyword -- whereas ``code``'s next-layer dispatch - registration is opt-in; see - :meth:`ProtocolBase.__init_subclass__` for the latter. + This method also names a concrete subclass in the by-name registry, + :attr:`ProtocolBase.__registry__ `, + under its upper-cased class name, once + :meth:`ProtocolBase.__init_subclass__` has accepted its ``code``, + whereas ``code``'s next-layer dispatch registration is opt-in. An + abstract subclass, as :func:`inspect.isabstract` reports it, is not + named, and nor is a class deriving :class:`ProtocolBase` without this + class, as every built-in protocol does. A name already holding another + entry keeps it, and the clash is reported as a + :exc:`~pcapkit.utilities.warnings.RegistryWarning`; only + :meth:`ProtocolBase.register_protocol + ` replaces an + entry. A class meeting the :class:`~pcapkit.corekit.module.ModuleDescriptor` + that names its own module and name is no clash, and the descriptor + stays. See Also: For more information on the registry, please refer to - :func:`pcapkit.foundation.registry.protocols.protocol.register_protocol`. + :meth:`ProtocolBase.register_protocol `. - """ - from pcapkit.foundation.registry.protocols import register_protocol - register_protocol(cls) + .. versionchanged:: 1.6.0 + + An abstract subclass is not registered, and a name already holding + an entry keeps it, where every subclass was registered over it + (:issue:`2763`). - return super().__init_subclass__(schema, data, code, *args, **kwargs) + """ + super().__init_subclass__(schema, data, code, *args, **kwargs) + + # NOTE: #2763: a concrete class names itself in the by-name registry + # once its ``code`` is accepted, so a refused one names nothing, and + # keeps off a name already held. A class meeting the descriptor naming + # its own module and name leaves it, to be resolved on lookup. + if not inspect.isabstract(cls): + name = cls.__name__.upper() + incumbent = ProtocolBase.__registry__.setdefault(name, cls) + if incumbent is not cls and not (isinstance(incumbent, ModuleDescriptor) + and incumbent == (cls.__module__, cls.__name__)): + incumbent_repr, protocol_repr = repr(incumbent), repr(cls) + if incumbent_repr == protocol_repr: # #710: tell same-named classes apart + incumbent_repr = f'{incumbent_repr} (id={id(incumbent):#x})' + protocol_repr = f'{protocol_repr} (id={id(cls):#x})' + warn(f'protocol {name} already registered, keeping {incumbent_repr} ' + f'over {protocol_repr}', RegistryWarning) diff --git a/pcapkit/protocols/registry.py b/pcapkit/protocols/registry.py deleted file mode 100644 index c89bba712e..0000000000 --- a/pcapkit/protocols/registry.py +++ /dev/null @@ -1,33 +0,0 @@ -# -*- coding: utf-8 -*- -"""Protocol Registry -======================= - -.. module:: pcapkit.protocols.registry - -:mod:`pcapkit.protocols.registry` builds :data:`pcapkit.protocols.__proto__`, the -registry of protocol classes by name, which :mod:`pcapkit.protocols` exports from -here. It names every protocol class that package exports, so importing it imports -them all; the package puts that off until the registry is first reached -(:issue:`2730`). - -.. versionadded:: 1.6.0 - -""" -from typing import TYPE_CHECKING - -from pcapkit import protocols -from pcapkit.protocols.protocol import ProtocolBase - -if TYPE_CHECKING: - from typing import Type - -__all__ = [] # type: list[str] - -#: dict[str, ~typing.Type[~pcapkit.protocols.protocol.ProtocolBase]]: Protocol -#: registry, bound before it is filled so that a read while it is being built -#: finds this object. -__proto__ = {} # type: dict[str, Type[ProtocolBase]] -for _name in protocols.__all__: - _value = getattr(protocols, _name) - if isinstance(_value, type) and issubclass(_value, ProtocolBase): - __proto__[_name.upper()] = _value diff --git a/tests/corekit/test_fields_misc.py b/tests/corekit/test_fields_misc.py index 18f6848298..c5bafb4a43 100644 --- a/tests/corekit/test_fields_misc.py +++ b/tests/corekit/test_fields_misc.py @@ -422,8 +422,8 @@ class PayloadFieldProtocolNameTests(unittest.TestCase): """Resolving :class:`~pcapkit.corekit.fields.misc.PayloadField`'s ``protocol`` from a name, per `#787 `__. - :data:`pcapkit.protocols.__proto__` is keyed on ``__name__.upper()``, both - where it is seeded and in + The by-name registry, now ``ProtocolBase.__registry__``, is keyed on + ``__name__.upper()``, both where it is seeded and in :func:`~pcapkit.foundation.registry.protocols.register_protocol`. The setter looked the name up as given, so anything not already upper-cased missed -- and a miss is indistinguishable from an unparsed payload, because diff --git a/tests/foundation/reassembly/test_dtls_reassembly_parity_2531_unit.py b/tests/foundation/reassembly/test_dtls_reassembly_parity_2531_unit.py index f573bdd898..ff41795da0 100644 --- a/tests/foundation/reassembly/test_dtls_reassembly_parity_2531_unit.py +++ b/tests/foundation/reassembly/test_dtls_reassembly_parity_2531_unit.py @@ -159,16 +159,16 @@ def test_the_engine_declares_the_dtls_protocol(self) -> None: def test_the_protocol_survives_a_registry_without_it(self) -> None: """With no ``'DTLS'`` registered, a name lookup would answer ``Raw``.""" - import pcapkit.protocols from pcapkit.protocols.application.dtls.protocol import DTLS + from pcapkit.protocols.protocol import ProtocolBase for container, write in CONTAINERS: with self.subTest(container=container): extractor = _extract(self, write([_frame(record) for record in COMPLETE]), reassembly=True, ip=True, tcp=True, dtls=True) - with mock.patch.dict(pcapkit.protocols.__proto__): + with mock.patch.dict(ProtocolBase.__registry__): for name in ('IPV4', 'TCP', 'DTLS'): - del pcapkit.protocols.__proto__[name] + del ProtocolBase.__registry__[name] for name in ('ipv4', 'tcp', 'dtls'): engine = getattr(extractor._reasm, name) self.assertIs(engine.protocol, type(engine).__protocol_type__, name) diff --git a/tests/foundation/registry/test_protocols.py b/tests/foundation/registry/test_protocols.py index e827b4f2d9..4b6e796c46 100644 --- a/tests/foundation/registry/test_protocols.py +++ b/tests/foundation/registry/test_protocols.py @@ -103,14 +103,15 @@ def test_register_protocol_warns_when_a_colliding_name_overwrites(self) -> None: """ from pcapkit.foundation.registry import protocols as registry - from pcapkit.protocols import __proto__ as protocol_registry from pcapkit.protocols.application.http import protocol as http from pcapkit.protocols.application.application import Application + from pcapkit.protocols.protocol import ProtocolBase from pcapkit.utilities.warnings import RegistryWarning + protocol_registry = ProtocolBase.__registry__ self._guard_registry(protocol_registry, 'HTTP') - # Defining a Protocol subclass registers it, so this displaces the base. + # An abstract class, so defining it names nothing. with warnings.catch_warnings(): warnings.simplefilter('ignore') custom = type('HTTP', (Application,), {'__module__': __name__}) @@ -132,7 +133,7 @@ def test_register_protocol_warns_when_a_colliding_name_overwrites(self) -> None: # The overwrite still happens. The warning reports the # collision, it does not resolve it -- resolving it is #514's # business. - self.assertIs(protocol_registry['HTTP'], replacement) + self.assertIs(ProtocolBase.get_protocol('HTTP'), replacement) messages = self._registry_warnings(caught, RegistryWarning) self.assertEqual(len(messages), 1) @@ -148,23 +149,22 @@ def test_register_protocol_warns_when_a_colliding_name_overwrites(self) -> None: def test_a_protocol_subclass_shadowing_a_builtin_name_warns(self) -> None: """The collision is reachable without calling the registrar at all. - :meth:`Protocol.__init_subclass__ - ` calls - :func:`~pcapkit.foundation.registry.protocols.register_protocol` - unconditionally, so merely *defining* a subclass of the public - :class:`~pcapkit.protocols.protocol.Protocol` under a name a built-in - already holds displaces the built-in's entry. That is the sharpest form - of #675: there is no call to any registry function anywhere in the - user's code, so without the fix nothing at all marks the moment the - dispatch table changes meaning. + :meth:`ProtocolBase.__init_subclass__ + ` names every + concrete subclass, so merely *defining* one under a name a built-in + already holds meets the built-in's entry. That is the sharpest form of + #675: there is no call to any registry function anywhere in the user's + code. The entry is kept and the clash reported, naming both, so a + bare-name lookup keeps its meaning and the moment is still marked + (#2763); only the registrar replaces it. """ - from pcapkit.foundation.registry import protocols as registry - from pcapkit.protocols import __proto__ as protocol_registry - from pcapkit.protocols.protocol import Protocol + from pcapkit.protocols.protocol import Protocol, ProtocolBase from pcapkit.utilities.warnings import RegistryWarning + protocol_registry = ProtocolBase.__registry__ incumbent = protocol_registry['HTTP'] + builtin = ProtocolBase.get_protocol('HTTP') self._guard_registry(protocol_registry, 'HTTP') with warnings.catch_warnings(record=True) as caught: @@ -173,13 +173,28 @@ def test_a_protocol_subclass_shadowing_a_builtin_name_warns(self) -> None: class HTTP(Protocol): """A user protocol that happens to be named like the built-in.""" - self.assertIs(protocol_registry['HTTP'], HTTP) - self.assertIsNot(protocol_registry['HTTP'], incumbent) + name = 'User HTTP' + length = 0 + + def read(self, length=None, **kwargs): # type: ignore[no-untyped-def] + return None + + def make(self, **kwargs): # type: ignore[no-untyped-def] + return None + + @classmethod + def __index__(cls): # type: ignore[no-untyped-def] + return None + + self.assertIs(protocol_registry['HTTP'], incumbent) + self.assertIs(ProtocolBase.get_protocol('HTTP'), builtin) + self.assertIsNot(builtin, HTTP) messages = self._registry_warnings(caught, RegistryWarning) self.assertEqual(len(messages), 1) self.assertIn(repr(incumbent), messages[0]) self.assertIn(repr(HTTP), messages[0]) + self.assertLess(messages[0].index(repr(incumbent)), messages[0].index(repr(HTTP))) def test_register_protocol_stays_quiet_when_nothing_is_displaced(self) -> None: """A fresh key and an identical re-registration are both silent. @@ -198,11 +213,12 @@ def test_register_protocol_stays_quiet_when_nothing_is_displaced(self) -> None: """ from pcapkit.foundation.registry import protocols as registry - from pcapkit.protocols import __proto__ as protocol_registry + from pcapkit.protocols.protocol import ProtocolBase from pcapkit.protocols.transport.tcp.protocol import TCP from pcapkit.protocols.transport.udp.protocol import UDP from pcapkit.utilities.warnings import RegistryWarning + protocol_registry = ProtocolBase.__registry__ UnitProtocol = self._unit_protocol() self._guard_registry(protocol_registry, 'UNITPROTOCOL') @@ -382,9 +398,10 @@ def test_register_protocol_disambiguates_classes_sharing_a_repr(self) -> None: """ from pcapkit.foundation.registry import protocols as registry - from pcapkit.protocols import __proto__ as protocol_registry + from pcapkit.protocols.protocol import ProtocolBase from pcapkit.utilities.warnings import RegistryWarning + protocol_registry = ProtocolBase.__registry__ first = self._unit_protocol() second = self._unit_protocol() @@ -419,9 +436,10 @@ def test_register_protocol_disambiguates_classes_sharing_a_repr(self) -> None: def test_register_protocol_validates_and_updates_registry(self) -> None: from pcapkit.foundation.registry import protocols as registry - from pcapkit.protocols import __proto__ as protocol_registry + from pcapkit.protocols.protocol import ProtocolBase from pcapkit.utilities.exceptions import RegistryError + protocol_registry = ProtocolBase.__registry__ UnitProtocol = self._unit_protocol() self._guard_registry(protocol_registry, 'UNITPROTOCOL') registry.register_protocol(UnitProtocol) @@ -474,9 +492,10 @@ def test_top_level_link_internet_and_transport_protocol_wrappers(self) -> None: from pcapkit.foundation.registry.protocols import misc as reg_misc from pcapkit.foundation.registry.protocols import protocol as reg_protocol from pcapkit.foundation.registry.protocols import transport as reg_transport - from pcapkit.protocols import __proto__ as protocol_registry + from pcapkit.protocols.protocol import ProtocolBase from pcapkit.utilities.exceptions import ProtocolError, RegistryError + protocol_registry = ProtocolBase.__registry__ UnitProtocol = self._unit_protocol() self._guard_registry(protocol_registry, 'UNITPROTOCOL') raw_module = ('pcapkit.protocols.misc.raw.protocol', 'Raw') diff --git a/tests/foundation/registry/test_registrar_code_check_unit.py b/tests/foundation/registry/test_registrar_code_check_unit.py index 2f24b06e55..7ed49a999a 100644 --- a/tests/foundation/registry/test_registrar_code_check_unit.py +++ b/tests/foundation/registry/test_registrar_code_check_unit.py @@ -9,7 +9,7 @@ anything. Every case snapshots each registry the registrars here write -- the dispatch and -method tables, the schema tables, :data:`pcapkit.protocols.__proto__` and the +method tables, the schema tables, ``ProtocolBase.__registry__`` and the name leases -- calls the registrar with an otherwise valid argument list, and compares every registry with the snapshot. The registrars that take a plain ``int`` (``NUMBER_REGISTRARS``) accept the ``int``, but still reject @@ -168,9 +168,9 @@ def setUp(self) -> None: def _registries(self, classes: 'tuple[type, ...]') -> 'dict[tuple[str, str], dict]': """Every dict-valued class attribute of ``classes``, plus the module-wide registries.""" from pcapkit.foundation.registry.protocols import protocol as reg_protocol - from pcapkit.protocols import __proto__ as protocol_registry + from pcapkit.protocols.protocol import ProtocolBase - found = {('pcapkit.protocols', '__proto__'): protocol_registry, + found = {('ProtocolBase', '__registry__'): ProtocolBase.__registry__, ('registrars', '_CODE_LEASES'): reg_protocol._CODE_LEASES} for cls in classes: for attr, value in vars(cls).items(): diff --git a/tests/foundation/registry/test_registry_name_restore_unit.py b/tests/foundation/registry/test_registry_name_restore_unit.py index dcf7460807..53c2419c93 100644 --- a/tests/foundation/registry/test_registry_name_restore_unit.py +++ b/tests/foundation/registry/test_registry_name_restore_unit.py @@ -3,7 +3,7 @@ Every code registrar in :mod:`pcapkit.foundation.registry.protocols` -- ``register_tcp``, ``register_linktype`` and the rest -- ends by naming the class -it registered in :data:`pcapkit.protocols.__proto__`. Guards against registering +it registered in ``ProtocolBase.__registry__``. Guards against registering the shipped entry back restoring the code but never the name: ``register_tcp(80, Custom)`` then ``register_tcp(80, shipped)`` leaving ``'CUSTOM'`` behind for good, and a custom class named like a built-in leaving @@ -36,17 +36,18 @@ @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') class TestRegistryNameRestore(unittest.TestCase): - """Restoring a code leaves :data:`pcapkit.protocols.__proto__` as it found it.""" + """Restoring a code leaves ``ProtocolBase.__registry__`` as it found it.""" def setUp(self) -> None: reimport_once_per_class(self) - from pcapkit.protocols import __proto__ as names from pcapkit.protocols.misc.pcap.frame.protocol import Frame from pcapkit.protocols.misc.pcapng.protocol import PCAPNG + from pcapkit.protocols.protocol import ProtocolBase from pcapkit.protocols.transport.tcp.protocol import TCP from pcapkit.protocols.transport.udp.protocol import UDP + names = ProtocolBase.__registry__ self.tables = (names, TCP.__proto__, UDP.__proto__, Frame.__proto__, PCAPNG.__proto__) self.saved = [dict(table) for table in self.tables] self.addCleanup(self._reset) @@ -63,6 +64,13 @@ def _custom(self, name: 'str') -> 'Any': return type(name, (Application,), {'__module__': __name__}) + @staticmethod + def _shown(entry: 'Any') -> 'Any': + """The class a by-name entry shows: a built-in's descriptor shows the class it names.""" + from pcapkit.corekit.module import ModuleDescriptor + + return entry.klass if isinstance(entry, ModuleDescriptor) else entry + def _call(self, register: 'Any', *args: 'Any') -> None: with warnings.catch_warnings(): warnings.simplefilter('ignore') @@ -113,7 +121,7 @@ def test_restoring_a_code_puts_back_the_name_it_displaced(self) -> None: self._call(register_tcp, 80, custom) self.assertIs(self.names['IPV4'], custom) self._call(register_tcp, 80, shipped) - self.assertIs(self.names['IPV4'], IPv4) + self.assertIs(self._shown(self.names['IPV4']), IPv4) self.assert_names_as_found() def test_a_name_stays_while_another_code_still_holds_it(self) -> None: @@ -157,7 +165,11 @@ def test_a_name_held_before_the_registration_is_kept(self) -> None: from pcapkit.protocols.transport.tcp.protocol import TCP shipped = TCP.__proto__[80] - defined = type('Defined', (Protocol,), {'__module__': __name__}) # named by definition + # A concrete Protocol subclass, so named by definition. + defined = type('Defined', (Protocol,), { + '__module__': __name__, 'name': 'Defined', 'length': 0, + 'read': lambda self, length=None, **kwargs: None, 'make': lambda self, **kwargs: None, + '__index__': classmethod(lambda cls: None)}) explicit = self._custom('Explicit') register_protocol(explicit) self.before = dict(self.names) @@ -187,7 +199,7 @@ def test_re_registering_the_same_override_stays_quiet(self) -> None: if issubclass(item.category, RegistryWarning)], []) self.assertIs(self.names['HTTP'], custom) self._call(register_tcp, 80, shipped) - self.assertIs(self.names['HTTP'], HTTP) + self.assertIs(self._shown(self.names['HTTP']), HTTP) self.assert_names_as_found() def test_a_built_in_over_its_own_shadow_is_undone_in_either_order(self) -> None: @@ -211,9 +223,9 @@ def test_a_built_in_over_its_own_shadow_is_undone_in_either_order(self) -> None: self._reset() self._call(register_tcp, 80, custom) self._call(register_tcp, 8080, IPv4) - self.assertIs(self.names['IPV4'], IPv4) + self.assertIs(self._shown(self.names['IPV4']), IPv4) self._call(register_tcp, order[0], {80: s80, 8080: s8080}[order[0]]) - self.assertIs(self.names['IPV4'], custom if order[0] == 8080 else IPv4) + self.assertIs(self._shown(self.names['IPV4']), custom if order[0] == 8080 else IPv4) self._call(register_tcp, order[1], {80: s80, 8080: s8080}[order[1]]) self.assert_names_as_found() @@ -282,7 +294,7 @@ def expected(held: 'dict[Any, tuple[int, Any]]', name: 'str') -> 'Any': held[op[1]] = (step, shipped[op[1]].klass) wrong = [f'step {step}, {name}: {self.names.get(name)!r}, expected ' f'{expected(held, name)!r}' for name in sorted(watched | set(self.names)) - if self.names.get(name) is not expected(held, name)] + if self._shown(self.names.get(name)) is not self._shown(expected(held, name))] if wrong: failures.append((ops[:step], wrong[0])) break diff --git a/tests/foundation/registry/test_registry_symmetry_unit.py b/tests/foundation/registry/test_registry_symmetry_unit.py index 4a11aad972..298468fbaa 100644 --- a/tests/foundation/registry/test_registry_symmetry_unit.py +++ b/tests/foundation/registry/test_registry_symmetry_unit.py @@ -25,7 +25,7 @@ and the comparison is split in two, so that two root causes cannot hide behind one another: ``dispatch`` covers the tables the registrar names -- the ``__proto__`` it targets, the option and schema registries, the extractor and -flow-tracing tables -- and ``names`` covers :data:`pcapkit.protocols.__proto__`, +flow-tracing tables -- and ``names`` covers ``ProtocolBase.__registry__``, the by-name registry the protocol registrars all funnel into. The scope is the public registrars in @@ -239,9 +239,9 @@ def _tables() -> 'dict[str, dict[Any, Any]]': """Every registry a public registrar writes, by name, as live objects.""" from pcapkit.foundation.extraction import Extractor from pcapkit.foundation.traceflow import TraceFlow - from pcapkit.protocols import __proto__ as names + from pcapkit.protocols.protocol import ProtocolBase - tables = {'names': names} # type: dict[str, dict[Any, Any]] + tables = {'names': ProtocolBase.__registry__} # type: dict[str, dict[Any, Any]] for label, path in DISPATCH.items(): tables[f'{label}.__proto__'] = resolve(path).__proto__ for registrar in METHOD_REGISTRARS: @@ -391,12 +391,12 @@ def _steps(label: 'str') -> 'list[Callable[[], None]]': head, variant, _ = label.split('/') if head == 'register_protocol': from pcapkit.foundation.registry import register_protocol - from pcapkit.protocols import __proto__ as names from pcapkit.protocols.application.application import Application + from pcapkit.protocols.protocol import ProtocolBase - shipped = names['HTTP'] + shipped = ProtocolBase.get_protocol('HTTP') # No shipped class shares HTTP's name since #2529, so a user class named - # like it displaces the entry; defining it registers it. + # like it displaces the entry; it is abstract, so defining it names nothing. custom = type('HTTP', (Application,), {'__module__': __name__}) return [lambda: register_protocol(custom), lambda: register_protocol(shipped)] name, _, member = head.partition(':') diff --git a/tests/foundation/registry/test_registry_wrapper_coverage_2511_unit.py b/tests/foundation/registry/test_registry_wrapper_coverage_2511_unit.py index 5d0569634f..677e43544b 100644 --- a/tests/foundation/registry/test_registry_wrapper_coverage_2511_unit.py +++ b/tests/foundation/registry/test_registry_wrapper_coverage_2511_unit.py @@ -16,9 +16,10 @@ stops existing, fails too. The classmethod is the owning class's own -- ``register`` for ``__proto__``, -``register_`` for ``____`` -- unless ``INHERITED`` names the one it -inherits and why that one is typed for exactly the map's keys, the owner's -option 2 on #2511. That table is checked in both directions as well. +``register_`` for ``____``, or the name ``NAMED`` gives -- unless +``INHERITED`` names the one it inherits and why that one is typed for exactly the +map's keys, the owner's option 2 on #2511. Those tables are checked in both +directions as well. The behaviour of the wrappers and classmethods #2511 added is pinned at the end: each writes the owning class's map and warns in its own key's terms, and @@ -58,6 +59,12 @@ ('pcapkit.protocols.transport.transport.Transport.register', 'typed for the port the map is keyed by'), } +#: ``map path -> (its classmethod, why the name is not register_)``. +NAMED = { + 'pcapkit.protocols.protocol.ProtocolBase.__registry__': + ('register_protocol', 'named for the protocol class it registers, ruled on #2763'), +} + #: A wrapper's tag: one or more map paths on the comment line above its ``def``. TAG = re.compile(r'^# NOTE: (?P\S.*)\ndef (?Pregister_\w+)\(', re.MULTILINE) @@ -94,6 +101,8 @@ def _maps() -> 'dict[str, type]': def _classmethod_name(path: 'str') -> 'str': """The ``register*`` classmethod writing the map at ``path``.""" + if path in NAMED: + return NAMED[path][0] kind = path.rpartition('.')[2][2:-2] return 'register' if kind == 'proto' else f'register_{kind}' @@ -165,6 +174,9 @@ def test_every_map_owner_has_its_classmethod_or_inherits_a_typed_one(self) -> No missing.append(f'{owner.__name__}.{name}') self.assertEqual(missing, [], 'a map owner inherits a register* classmethod INHERITED does not name') self.assertEqual(sorted(set(INHERITED) - set(maps)), [], 'an inherited map no longer exists') + self.assertEqual(sorted(set(NAMED) - set(maps)), [], 'a named map no longer exists') + self.assertEqual(sorted(set(NAMED) & (set(INHERITED) | set(EXCUSED))), [], + 'a named map is inherited or excused') self.assertEqual(sorted(set(INHERITED) & set(EXCUSED)), [], 'a map is both inherited and excused') def test_ipx_loopback_and_sctp_wrappers_write_the_owning_class_map(self) -> None: @@ -232,11 +244,13 @@ def test_register_ipx_and_register_loopback_dispatch_the_payload(self) -> None: from pcapkit.const.ipx.packet import Packet from pcapkit.foundation.registry import protocols as registry from pcapkit.foundation.registry.protocols import protocol as reg_protocol - from pcapkit.protocols import __proto__ as names from pcapkit.protocols.internet.ipx.protocol import IPX from pcapkit.protocols.link.loopback.protocol import Loopback + from pcapkit.protocols.protocol import ProtocolBase from pcapkit.protocols.transport.udp.protocol import UDP + names = ProtocolBase.__registry__ + udp = bytes.fromhex('0035' '0035' '0008' '0000') ipx = b'\xff\xff' + (30 + len(udp)).to_bytes(2, 'big') + b'\x00\x11' + bytes(24) + udp # Address family 99, which nothing ships, written as LINKTYPE_NULL does. diff --git a/tests/protocols/application/test_http_id_2524_unit.py b/tests/protocols/application/test_http_id_2524_unit.py index 30e9c77884..e2e163a143 100644 --- a/tests/protocols/application/test_http_id_2524_unit.py +++ b/tests/protocols/application/test_http_id_2524_unit.py @@ -93,7 +93,7 @@ def test_in_and_getitem_select_the_family_and_the_layers_own_version(self) -> No class and by its own version's name or key, and miss it by the other version's. An exact version such as ``'HTTP/1.1'`` is in no ``id()``, so only the protochain's alias match finds it.""" - from pcapkit.protocols import __proto__ as registry + from pcapkit.protocols.protocol import ProtocolBase from pcapkit.utilities.exceptions import ProtocolNotFound _, HTTPv1, HTTPv2 = self.classes() @@ -104,8 +104,8 @@ class and by its own version's name or key, and miss it by the other self.assertIs(type(layer), members[version]) for selector in SELECTORS: keys = [selector, selector.lower()] - if selector.upper() in registry: - keys.append(registry[selector.upper()]) + if (klass := ProtocolBase.get_protocol(selector)) is not None: + keys.append(klass) for key in keys: with self.subTest(version=version, selector=key): if selects(selector, version): diff --git a/tests/protocols/application/test_http_member_names_2529_unit.py b/tests/protocols/application/test_http_member_names_2529_unit.py index 99689285d5..a240191d16 100644 --- a/tests/protocols/application/test_http_member_names_2529_unit.py +++ b/tests/protocols/application/test_http_member_names_2529_unit.py @@ -125,11 +125,11 @@ def test_a_member_built_directly_reports_its_own_name(self) -> None: def test_registering_a_member_keeps_the_base_under_http(self) -> None: """Each member's own key holds it, and ``'HTTP'`` keeps the base.""" from pcapkit.foundation.registry import protocols as registry - from pcapkit.protocols import __proto__ as protocol_registry from pcapkit.protocols.application.http.protocol import HTTP + from pcapkit.protocols.protocol import ProtocolBase from pcapkit.utilities.warnings import RegistryWarning - names = protocol_registry + names = ProtocolBase.__registry__ before = dict(names) def restore() -> None: @@ -146,8 +146,8 @@ def restore() -> None: registry.register_protocol(klass) self.assertEqual([str(item.message) for item in caught if issubclass(item.category, RegistryWarning)], []) - self.assertIs(names['HTTP'], HTTP) - self.assertIs(names[class_name.upper()], klass) + self.assertIs(ProtocolBase.get_protocol('HTTP'), HTTP) + self.assertIs(ProtocolBase.get_protocol(class_name), klass) if __name__ == '__main__': diff --git a/tests/protocols/application/test_ngap_unit.py b/tests/protocols/application/test_ngap_unit.py index 2094437a02..22232f9c54 100644 --- a/tests/protocols/application/test_ngap_unit.py +++ b/tests/protocols/application/test_ngap_unit.py @@ -242,8 +242,8 @@ def test_exported_from_the_package_namespaces(self) -> None: self.assertIs(pcapkit.NGAP, NGAP) self.assertIs(ApplicationNGAP, NGAP) - self.assertIn('NGAP', pcapkit.protocols.__proto__) - self.assertIs(pcapkit.protocols.__proto__['NGAP'], NGAP) + self.assertIn('NGAP', pcapkit.protocols.protocol.ProtocolBase.__registry__) + self.assertIs(pcapkit.protocols.protocol.ProtocolBase.get_protocol('NGAP'), NGAP) self.assertIsNot(DataNGAP, NGAP) self.assertIsNot(SchemaNGAP, NGAP) diff --git a/tests/protocols/application/test_ssl_tls_dtls_unit.py b/tests/protocols/application/test_ssl_tls_dtls_unit.py index 0627f798bb..702750cae7 100644 --- a/tests/protocols/application/test_ssl_tls_dtls_unit.py +++ b/tests/protocols/application/test_ssl_tls_dtls_unit.py @@ -1272,11 +1272,11 @@ def test_exported_from_every_namespace(self) -> None: def test_registered_in_the_protocol_registry(self) -> None: import pcapkit.protocols + from pcapkit.protocols.protocol import ProtocolBase for name in self.NAMES: with self.subTest(name=name): - self.assertIs(pcapkit.protocols.__proto__[name.upper()], - getattr(pcapkit.protocols, name)) + self.assertIs(ProtocolBase.get_protocol(name), getattr(pcapkit.protocols, name)) def test_string_lookup_matches_a_parsed_record(self) -> None: """``'SSL' in pkt`` agrees with ``SSL in pkt``, the name being registered.""" diff --git a/tests/protocols/application/test_sslv3_unit.py b/tests/protocols/application/test_sslv3_unit.py index 4d8082f232..84308581dc 100644 --- a/tests/protocols/application/test_sslv3_unit.py +++ b/tests/protocols/application/test_sslv3_unit.py @@ -70,7 +70,7 @@ def test_top_level_names_are_the_new_homes_object(self) -> None: with self.subTest(package=package.__name__): self.assertIs(package.SSLv3, SSLv3) self.assertIn('SSLv3', package.__all__) - self.assertIs(pcapkit.protocols.__proto__['SSLV3'], SSLv3) + self.assertIs(pcapkit.protocols.protocol.ProtocolBase.get_protocol('SSLV3'), SSLv3) def test_parsed_tls_keeps_its_dynamic_sslv3_alias(self) -> None: """Dispatch binds TLS alone; legacy version ``0x0300`` reports ``SSLv3``.""" diff --git a/tests/protocols/internet/test_esp_unit.py b/tests/protocols/internet/test_esp_unit.py index 93abc8bda0..d260838994 100644 --- a/tests/protocols/internet/test_esp_unit.py +++ b/tests/protocols/internet/test_esp_unit.py @@ -441,7 +441,7 @@ def test_module_is_importable_and_exported(self) -> None: self.assertIn('ESP', pcapkit.__all__) self.assertIs(pcapkit.ESP, ESP) - self.assertIs(pcapkit.protocols.__proto__['ESP'], ESP) + self.assertIs(pcapkit.protocols.protocol.ProtocolBase.get_protocol('ESP'), ESP) ########################################################################## # Without SA context. diff --git a/tests/protocols/internet/test_igmp_unit.py b/tests/protocols/internet/test_igmp_unit.py index cd54f4ddcd..d76c404b88 100644 --- a/tests/protocols/internet/test_igmp_unit.py +++ b/tests/protocols/internet/test_igmp_unit.py @@ -1703,7 +1703,7 @@ def test_in_and_getitem_select_the_family_and_the_own_version(self) -> None: alike: ``'IGMP'`` reaches every version, a version key only its own -- ``'IGMPv1'`` through its registry key, since no ``id()`` carries it. The protochain also compares each alias, which here is the same name.""" - from pcapkit.protocols import __proto__ as registry + from pcapkit.protocols.protocol import ProtocolBase from pcapkit.utilities.exceptions import ProtocolNotFound for name, (packet, version) in {**IGMP_PACKETS, **IGMPV3_PACKETS}.items(): @@ -1711,7 +1711,7 @@ def test_in_and_getitem_select_the_family_and_the_own_version(self) -> None: self.assertIsInstance(layer, igmp_class(version)) for selector in self.SELECTORS: expected = selector in ('IGMP', version) - for key in (selector, registry[selector.upper()]): + for key in (selector, ProtocolBase.get_protocol(selector)): with self.subTest(packet=name, selector=key): self.assertIs(key in layer, expected) self.assertIs(key in frame, expected) diff --git a/tests/protocols/link/test_linux_sll_arphrd_registry_unit.py b/tests/protocols/link/test_linux_sll_arphrd_registry_unit.py index 0942350745..6bb75b3a98 100644 --- a/tests/protocols/link/test_linux_sll_arphrd_registry_unit.py +++ b/tests/protocols/link/test_linux_sll_arphrd_registry_unit.py @@ -61,11 +61,12 @@ def _versions(self): # type: ignore[no-untyped-def] def _snapshot(self): # type: ignore[no-untyped-def] """Every registry the registrars here write, to put back after a test.""" from pcapkit.foundation.registry.protocols import protocol as reg_protocol - from pcapkit.protocols import __proto__ as names from pcapkit.protocols.link.frad.protocol import FRAD from pcapkit.protocols.link.linux_sll.protocol import LinuxSLL from pcapkit.protocols.link.netlink.protocol import Netlink + from pcapkit.protocols.protocol import ProtocolBase + names = ProtocolBase.__registry__ tables = (LinuxSLL.__proto__, FRAD.__proto__, Netlink.__proto__, names, reg_protocol._CODE_LEASES, reg_protocol._NAME_LEASES) copies = [dict(table) for table in tables] diff --git a/tests/protocols/link/test_linux_sll_selector_unit.py b/tests/protocols/link/test_linux_sll_selector_unit.py index 97ac884e0d..34d6e9c469 100644 --- a/tests/protocols/link/test_linux_sll_selector_unit.py +++ b/tests/protocols/link/test_linux_sll_selector_unit.py @@ -85,10 +85,10 @@ def test_family_base_has_no_registry_index(self) -> None: LinuxSLL.__index__() def test_registered_in_the_name_registry(self) -> None: - from pcapkit.protocols import __proto__ as registry from pcapkit.protocols.link.linux_sll.protocol import LinuxSLL + from pcapkit.protocols.protocol import ProtocolBase - self.assertIs(registry.get('LINUXSLL'), LinuxSLL) + self.assertIs(ProtocolBase.get_protocol('LINUXSLL'), LinuxSLL) def test_family_name_selects_either_version(self) -> None: from pcapkit.protocols.link.linux_sll.protocol import LinuxSLL diff --git a/tests/protocols/link/test_radiotap_unit.py b/tests/protocols/link/test_radiotap_unit.py index 1fa612394a..012c42706c 100644 --- a/tests/protocols/link/test_radiotap_unit.py +++ b/tests/protocols/link/test_radiotap_unit.py @@ -311,9 +311,10 @@ def test_make_rejects_what_does_not_fit(self) -> None: def _snapshot(self) -> None: """Put back every registry ``register_radiotap`` writes after the test.""" from pcapkit.foundation.registry.protocols import protocol as reg_protocol - from pcapkit.protocols import __proto__ as names from pcapkit.protocols.link.radiotap.protocol import Radiotap + from pcapkit.protocols.protocol import ProtocolBase + names = ProtocolBase.__registry__ tables = (Radiotap.__proto__, names, reg_protocol._CODE_LEASES, reg_protocol._NAME_LEASES) copies = [dict(table) for table in tables] diff --git a/tests/protocols/test_option_roundtrip_unit.py b/tests/protocols/test_option_roundtrip_unit.py index 84487f3e62..ccd16a7192 100644 --- a/tests/protocols/test_option_roundtrip_unit.py +++ b/tests/protocols/test_option_roundtrip_unit.py @@ -97,8 +97,12 @@ CASE_TIMEOUT = 30 #: The non-empty option-like maps no family exercises, by ``module.Class.__map__`` -#: path, and why. Empty: every one is some family's registry (#2615). -UNCOVERED = {} # type: dict[str, str] +#: path, and why. Every other one is some family's registry (#2615). +UNCOVERED = { + 'pcapkit.protocols.protocol.ProtocolBase.__registry__': + 'the by-name protocol registry, keyed by class name rather than by an option code; ' + 'tests/protocols/test_protocol_registry_2763_unit.py covers it (#2763)', +} # type: dict[str, str] def _option_registries() -> 'dict[str, dict[Any, Any]]': diff --git a/tests/protocols/test_protocol_code_registration_unit.py b/tests/protocols/test_protocol_code_registration_unit.py index 4ccd24bf22..23fcb58645 100644 --- a/tests/protocols/test_protocol_code_registration_unit.py +++ b/tests/protocols/test_protocol_code_registration_unit.py @@ -471,13 +471,13 @@ def setUp(self) -> None: reimport_once_per_class(self) def _assert_rejected_unchanged(self, call: 'Any') -> None: - from pcapkit.protocols import __proto__ as protocol_registry from pcapkit.protocols.internet.internet import Internet from pcapkit.protocols.internet.ipx.protocol import IPX from pcapkit.protocols.link.link import Link from pcapkit.protocols.link.radiotap.protocol import Radiotap from pcapkit.protocols.misc.pcap.frame.protocol import Frame from pcapkit.protocols.misc.pcapng.protocol import PCAPNG + from pcapkit.protocols.protocol import ProtocolBase from pcapkit.protocols.transport.sctp.protocol import SCTP from pcapkit.protocols.transport.tcp.protocol import TCP from pcapkit.protocols.transport.udp.protocol import UDP @@ -485,16 +485,17 @@ def _assert_rejected_unchanged(self, call: 'Any') -> None: tables = {klass.__name__: klass.__proto__ for klass in (Link, Radiotap, Internet, IPX, Frame, PCAPNG, TCP, UDP, SCTP)} - tables['pcapkit.protocols'] = protocol_registry + tables['ProtocolBase.__registry__'] = ProtocolBase.__registry__ before = {name: dict(table) for name, table in tables.items()} try: with warnings.catch_warnings(): warnings.simplefilter('ignore') with self.assertRaises(RegistryError): call() + # NOTE: the by-name registry too: a class whose ``code`` is refused + # is never defined, so it names nothing (#2763). for name, table in tables.items(): - if name != 'pcapkit.protocols': - self.assertEqual(dict(table), before[name], f'{name}.__proto__ changed') + self.assertEqual(dict(table), before[name], f'{name} changed') finally: for name, table in tables.items(): table.clear() diff --git a/tests/protocols/test_protocol_packages_lazy_2730_unit.py b/tests/protocols/test_protocol_packages_lazy_2730_unit.py index ed40b7788c..2742397607 100644 --- a/tests/protocols/test_protocol_packages_lazy_2730_unit.py +++ b/tests/protocols/test_protocol_packages_lazy_2730_unit.py @@ -5,9 +5,10 @@ :mod:`pcapkit.protocols.misc.checksum` name each protocol class only under :data:`~typing.TYPE_CHECKING` and import it through :func:`pcapkit.utilities.importlib.lazy_exports`. :mod:`pcapkit` re-exports -:mod:`pcapkit.protocols` the same way, and :data:`pcapkit.protocols.__proto__` is -built by :mod:`pcapkit.protocols.registry` the first time it is reached. A protocol's -own package keeps its literal imports, which the #2534 layout tests pin. +:mod:`pcapkit.protocols` the same way, and the by-name registry, +``ProtocolBase.__registry__``, names each built-in by a descriptor rather than by its +class (#2763). A protocol's own package keeps its literal imports, which the #2534 +layout tests pin. * :class:`LazyPackageSweepTests` holds each such package, read off the tree, to what its eager import did: every export resolves to what its import names, ``import *`` @@ -160,19 +161,17 @@ def test_pcapkit_re_exports_the_protocols(self) -> 'None': def test_dir_lists_what_an_eager_import_bound(self) -> 'None': # NOTE: the eager import bound every submodule its imports name, and any # other one already imported; the first are listed whether imported or - # not, as they are not on a cold import. ``registry`` is where - # ``__proto__`` comes from, which ``dir()`` lists as the eager import did. + # not, as they are not on a cold import. for package in PACKAGES: module = importlib.import_module(package) named = {source[len(package) + 1:].split('.')[0] for (source, _, _) in declared(package).values() if source.startswith(f'{package}.')} submodules = named | {name for (name, value) in vars(module).items() if isinstance(value, types.ModuleType) and value.__name__ == f'{package}.{name}'} - extra = {'__proto__', 'registry'} if package == 'pcapkit.protocols' else set() with self.subTest(package=package): self.assertTrue(named) self.assertEqual(dir(module), sorted({*import_system_names(module), '__all__', *module.__all__, - *BASES.get(package, ()), *submodules, *extra})) + *BASES.get(package, ()), *submodules})) for helper in ('TYPE_CHECKING', 'lazy_exports', '__getattr__', '__dir__'): self.assertIn(helper, vars(module)) self.assertNotIn(helper, dir(module)) @@ -230,22 +229,26 @@ class ColdImportTests(unittest.TestCase): """What a cold ``import pcapkit`` leaves unimported, and what reaches it later.""" def test_import_pcapkit_binds_no_protocol_class(self) -> 'None': - """No package holds a protocol class, nor the registry, until it is reached.""" + """No package holds a protocol class until it is reached, nor the by-name registry one.""" out = run_child(f''' import importlib import pcapkit.protocols + from pcapkit.corekit.module import ModuleDescriptor + from pcapkit.protocols.protocol import ProtocolBase bound = {{}} for package in {PACKAGES!r}: module = importlib.import_module(package) bound[package] = sorted(name for name in module.__all__ if name in vars(module)) emit(bound=bound, top=sorted(name for name in pcapkit.protocols.__all__ if name in vars(pcapkit)), - registry=['__proto__' in vars(pcapkit.protocols), 'pcapkit.protocols.registry' in sys.modules], + registry=['__proto__' in vars(pcapkit.protocols), + sorted(name for name, entry in ProtocolBase.__registry__.items() + if not isinstance(entry, ModuleDescriptor))], loaded=loaded('pcapkit.protocols.link.ethernet', 'pcapkit.protocols.link.s_tag', 'pcapkit.protocols.internet.igmp', 'pcapkit.protocols.application.ngap')) ''') self.assertEqual(out['bound'], {package: sorted(EAGER.get(package, ())) for package in PACKAGES}) self.assertEqual(out['top'], []) - self.assertEqual(out['registry'], [False, False]) + self.assertEqual(out['registry'], [False, []]) self.assertEqual(out['loaded'], []) def test_reaching_a_name_imports_its_package(self) -> 'None': @@ -266,15 +269,18 @@ def test_dispatch_imports_a_next_layer_not_yet_imported(self) -> 'None': """IPv4 dispatches protocol number 2 to IGMP, which picks IGMPv2, neither imported yet.""" out = run_child(''' from pcapkit.protocols.internet.ipv4.protocol import IPv4 + from pcapkit.protocols.protocol import ProtocolBase before = loaded('pcapkit.protocols.internet.igmp', 'pcapkit.protocols.internet.igmpv2') + names = dict(ProtocolBase.__registry__) packet = IPv4(io.BytesIO(ipv4(2, IGMP)), 20 + len(IGMP)) emit(before=before, chain=str(packet.protochain), layer=type(packet.payload).__module__, - registry='pcapkit.protocols.registry' in sys.modules) + registry=sorted(name for name in set(names) | set(ProtocolBase.__registry__) + if ProtocolBase.__registry__.get(name) is not names.get(name))) ''') self.assertEqual(out['before'], []) self.assertEqual(out['chain'], 'IPv4:IGMPv2') self.assertEqual(out['layer'], 'pcapkit.protocols.internet.igmpv2.protocol') - self.assertFalse(out['registry'], 'dispatching a next layer read the name registry') + self.assertEqual(out['registry'], [], 'dispatching a next layer wrote the name registry') def test_a_registration_made_on_import_is_found_by_a_dissector(self) -> 'None': """Shim6's entry exists after a bare ``import pcapkit``, so IPv6 parses it as ``IPv6_Ext``.""" @@ -297,60 +303,77 @@ def test_a_registration_made_on_import_is_found_by_a_dissector(self) -> 'None': self.assertEqual(out['chain'], 'IPv6:IPv6-Ext:UDP:Raw') def test_a_selector_reaches_a_class_not_yet_imported(self) -> 'None': - """``'S_Tag' in`` a C-Tag frame expands through the registry, which imports S_Tag.""" + """``'S_Tag' in`` a C-Tag frame expands through the registry, which imports S_Tag alone.""" out = run_child(''' Ethernet = pcapkit.protocols.link.Ethernet tag = bytes([0x81, 0x00, 0x00, 0x01, 0x08, 0x00]) udp = struct.pack('>HHHH', 1, 2, 8, 0) raw = bytes([0xff] * 6) + bytes(range(6)) + tag + ipv4(17, udp) frame = Ethernet(io.BytesIO(raw), len(raw)) - before = loaded('pcapkit.protocols.link.s_tag', 'pcapkit.protocols.registry') + watched = ('pcapkit.protocols.link.s_tag', 'pcapkit.protocols.application.ngap') + before = loaded(*watched) emit(before=before, chain=str(frame.protochain), selected=['S_Tag' in frame, 'C_Tag' in frame], - after=loaded('pcapkit.protocols.link.s_tag', 'pcapkit.protocols.registry')) + after=loaded(*watched)) ''') self.assertEqual(out['before'], []) self.assertEqual(out['chain'], 'Ethernet:802.1Q:IPv4:UDP') self.assertEqual(out['selected'], [True, True]) - self.assertEqual(out['after'], ['pcapkit.protocols.link.s_tag', 'pcapkit.protocols.registry']) + self.assertEqual(out['after'], ['pcapkit.protocols.link.s_tag']) def test_the_registry_is_whole_when_first_read(self) -> 'None': out = run_child(''' import pcapkit.protocols as protocols from pcapkit.protocols.protocol import ProtocolBase - before = ['__proto__' in vars(protocols), loaded('pcapkit.protocols.registry')] - from pcapkit.protocols import __proto__ as registry + registry = ProtocolBase.__registry__ + keys = sorted(registry) names = [name for name in protocols.__all__ if isinstance(getattr(protocols, name), type) and issubclass(getattr(protocols, name), ProtocolBase)] - emit(before=before, keys=sorted(registry), names=sorted(name.upper() for name in names), - values=all(registry[name.upper()] is getattr(protocols, name) for name in names), - same=[registry is protocols.__proto__, registry is vars(protocols)['__proto__'], - registry is sys.modules['pcapkit.protocols.registry'].__proto__]) + emit(keys=keys, names=sorted(name.upper() for name in names), + values=all(ProtocolBase.get_protocol(name) is getattr(protocols, name) for name in names), + gone=[hasattr(protocols, '__proto__'), '__proto__' in vars(protocols)]) ''') - self.assertEqual(out['before'], [False, []]) self.assertIn('IPV4', out['keys']) self.assertEqual(out['keys'], out['names']) self.assertTrue(out['values']) - self.assertEqual(out['same'], [True] * 3) + self.assertEqual(out['gone'], [False, False]) def test_a_user_protocol_meets_the_shipped_name_before_any_read(self) -> 'None': - """Defining a :class:`~pcapkit.protocols.protocol.Protocol` subclass first still warns on a shipped name.""" + """Defining a concrete subclass first still warns on a shipped name, which it keeps.""" out = run_child(''' - from pcapkit.protocols.protocol import Protocol + from pcapkit.protocols.protocol import Protocol, ProtocolBase from pcapkit.utilities.warnings import RegistryWarning - before = loaded('pcapkit.protocols.link.ethernet', 'pcapkit.protocols.registry') + before = loaded('pcapkit.protocols.link.ethernet') with warnings.catch_warnings(record=True) as caught: warnings.simplefilter('always') class Ethernet(Protocol): """A user protocol named like the shipped one.""" - emit(before=before, messages=[str(w.message) for w in caught if issubclass(w.category, RegistryWarning)], - registered=pcapkit.protocols.__proto__['ETHERNET'] is Ethernet) + name = 'User Ethernet' + length = 0 + + def read(self, length=None, **kwargs): + return None + + def make(self, **kwargs): + return None + + @classmethod + def __index__(cls): + return None + + during = loaded('pcapkit.protocols.link.ethernet') + shipped = ProtocolBase.get_protocol('ETHERNET') + emit(before=before, during=during, + messages=[str(w.message) for w in caught if issubclass(w.category, RegistryWarning)], + kept=f'{shipped.__module__}.{shipped.__qualname__}') ''') self.assertEqual(out['before'], []) + self.assertEqual(out['during'], []) self.assertEqual(len(out['messages']), 1, out['messages']) - self.assertIn('pcapkit.protocols.link.ethernet.protocol.Ethernet', out['messages'][0]) - self.assertTrue(out['registered']) + self.assertIn("ModuleDescriptor(module='pcapkit.protocols.link.ethernet.protocol', name='Ethernet')", + out['messages'][0]) + self.assertEqual(out['kept'], 'pcapkit.protocols.link.ethernet.protocol.Ethernet') def test_a_pickle_loads_where_its_class_is_not_yet_imported(self) -> 'None': dumped = run_child(''' diff --git a/tests/protocols/test_protocol_registry_2763_unit.py b/tests/protocols/test_protocol_registry_2763_unit.py new file mode 100644 index 0000000000..70421a5b38 --- /dev/null +++ b/tests/protocols/test_protocol_registry_2763_unit.py @@ -0,0 +1,484 @@ +# -*- coding: utf-8 -*- +"""``ProtocolBase.__registry__``, the by-name protocol registry (GitHub issue #2763). + +The module-level dict ``pcapkit.protocols.__proto__`` is gone, with no alias. The +registry is a class-level map on :class:`~pcapkit.protocols.protocol.ProtocolBase`, +from an upper-cased class name to the class, read with ``get_protocol`` and written +with ``register_protocol``: + +* :class:`ShippedEntriesTests` is the guard: every shipped protocol class has its + entry, a :class:`~pcapkit.corekit.module.ModuleDescriptor` naming its own module + and class, and no entry names anything else. +* :class:`AutoRegistrationTests` pins what defining a subclass does: a concrete + :class:`~pcapkit.protocols.protocol.Protocol` subclass names itself, an abstract + one does not, nor does a class deriving ``ProtocolBase`` alone, as every built-in + does; a clash keeps the entry and warns, a class meeting its own descriptor is + silent, and only ``register_protocol`` replaces an entry, putting a built-in's + descriptor back as shipped. +* :class:`GetterTests` pins the lookup and the removal of the module dict. +* :class:`RegistryViewTests` pins ``AnyProtocol.registry``, the read-only view of the + entries as stored. +* :class:`ColdProcessTests` runs in a fresh interpreter, since this one imported + everything long ago: the view and the map import nothing, the getter imports a + class only when it resolves its entry, a reloaded built-in is followed silently, + and threads racing on a first resolution all get the one class. + +""" + +import importlib +import inspect +import json +import os +import pathlib +import pkgutil +import subprocess +import sys +import textwrap +import unittest +import warnings +from typing import TYPE_CHECKING + +from tests._support import reimport_once_per_class + +if TYPE_CHECKING: + from typing import Any + +#: Test tier (#1958): exercises ABC machinery, threads and the import system, which +#: differ by interpreter. +__tier__ = 'ALL' + +ROOT = pathlib.Path(__file__).resolve().parents[2] + + +def _import_protocols() -> 'None': + """Import every shipped protocol module, so every class is a subclass by now.""" + import pcapkit.protocols as package + + for info in pkgutil.walk_packages(package.__path__, 'pcapkit.protocols.'): + if 'NotImplemented' not in info.name: + importlib.import_module(info.name) + + +def _shipped_classes() -> 'set[type]': + """Every protocol class :mod:`pcapkit.protocols` exports, and every concrete one it defines.""" + import pcapkit.protocols + from pcapkit.protocols.protocol import ProtocolBase + + _import_protocols() + found = {value for value in (getattr(pcapkit.protocols, name) for name in pcapkit.protocols.__all__) + if isinstance(value, type) and issubclass(value, ProtocolBase)} + pending, seen = [ProtocolBase], set() # type: list[type], set[type] + while pending: + klass = pending.pop() + if klass in seen: + continue + seen.add(klass) + pending.extend(klass.__subclasses__()) + if klass.__module__.startswith('pcapkit.protocols.') and not inspect.isabstract(klass): + found.add(klass) + return found + + +def _registry_warnings(caught: 'list[warnings.WarningMessage]') -> 'list[str]': + from pcapkit.utilities.warnings import RegistryWarning + + return [str(item.message) for item in caught if issubclass(item.category, RegistryWarning)] + + +def _concrete(name: 'str', base: 'Any' = None, module: 'str' = __name__, **keywords: 'Any') -> 'Any': + """A concrete protocol class named ``name`` in ``module``, defined with the class ``keywords``.""" + if base is None: + from pcapkit.protocols.protocol import Protocol as base + + def read(self, length=None, **kwargs): # type: ignore[no-untyped-def] + return None + + def make(self, **kwargs): # type: ignore[no-untyped-def] + return None + + def index(cls): # type: ignore[no-untyped-def] + return None + + body = {'__module__': module, 'name': name, 'length': 0, 'read': read, 'make': make, + '__index__': classmethod(index)} + return type(name, (base,), body, **keywords) + + +class _RegistryTestCase(unittest.TestCase): + """Imports :mod:`pcapkit` once per class, and puts the registry back after each test.""" + + def setUp(self) -> 'None': + reimport_once_per_class(self) + from pcapkit.protocols.protocol import ProtocolBase + + registry = ProtocolBase.__registry__ + saved = dict(registry) + + def restore() -> 'None': + registry.clear() + registry.update(saved) + + self.addCleanup(restore) + self.ProtocolBase = ProtocolBase + self.registry = registry + + +class ShippedEntriesTests(_RegistryTestCase): + """The guard: each shipped protocol class has an entry naming it, and nothing else does.""" + + def test_every_shipped_protocol_class_has_its_entry(self) -> 'None': + shipped = _shipped_classes() + self.assertIn(self.ProtocolBase.get_protocol('IPv4'), shipped) # the walk still finds them + for klass in sorted(shipped, key=lambda klass: klass.__name__): + with self.subTest(protocol=f'{klass.__module__}.{klass.__qualname__}'): + entry = self.ProtocolBase._shipped_registry.get(klass.__name__.upper()) + self.assertIsNotNone(entry, 'a shipped protocol class has no __registry__ entry') + self.assertEqual(entry, (klass.__module__, klass.__name__), + 'the entry names another module or class') + + def test_no_entry_names_anything_but_a_shipped_class(self) -> 'None': + from pcapkit.corekit.module import ModuleDescriptor + + names = {klass.__name__.upper() for klass in _shipped_classes()} + self.assertEqual(sorted(set(self.ProtocolBase._shipped_registry) - names), []) + for key, entry in self.ProtocolBase._shipped_registry.items(): + with self.subTest(key=key): + self.assertIsInstance(entry, ModuleDescriptor) + klass = entry.klass + self.assertEqual((klass.__module__, klass.__name__.upper()), (entry.module, key)) + self.assertTrue(issubclass(klass, self.ProtocolBase)) + + def test_the_map_starts_as_shipped(self) -> 'None': + """Each built-in entry is the very descriptor shipped, and stays one once its class is imported.""" + _import_protocols() + shipped = self.ProtocolBase._shipped_registry + self.assertTrue(shipped) + for key, entry in shipped.items(): + with self.subTest(key=key): + self.assertIs(self.registry[key], entry) + + +class AutoRegistrationTests(_RegistryTestCase): + """What defining a subclass does to the registry.""" + + def test_a_concrete_protocol_subclass_names_itself(self) -> 'None': + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + klass = _concrete('W2763Fresh') + self.assertIs(self.registry['W2763FRESH'], klass) + self.assertIs(self.ProtocolBase.get_protocol('w2763fresh'), klass) + self.assertEqual(_registry_warnings(caught), []) + + def test_a_class_deriving_the_internal_base_alone_is_not_named(self) -> 'None': + """Only the public base registers: a built-in's subclass is named by nothing, clashing or not.""" + from pcapkit.protocols.application.application import Application + from pcapkit.protocols.misc.raw.protocol import Raw + + before = dict(self.registry) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + classes = [type('W2763Raw', (Raw,), {'__module__': __name__}), + _concrete('W2763App', base=Application), + type('IPv4', (Raw,), {'__module__': __name__})] + for klass in classes: + with self.subTest(base=klass.__mro__[1].__name__, name=klass.__name__): + self.assertFalse(inspect.isabstract(klass)) + self.assertEqual(self.registry, before) + self.assertEqual(_registry_warnings(caught), []) + + def test_an_abstract_subclass_is_skipped(self) -> 'None': + from pcapkit.protocols.protocol import Protocol + + bare = type('W2763Bare', (Protocol,), {'__module__': __name__}) + self.assertTrue(inspect.isabstract(bare)) + self.assertNotIn('W2763BARE', self.registry) + + # A concrete subclass of an abstract one is still named. + member = _concrete('W2763Member', base=bare) + self.assertFalse(inspect.isabstract(member)) + self.assertIs(self.registry['W2763MEMBER'], member) + + def test_a_clash_keeps_the_entry_and_warns(self) -> 'None': + shipped = self.registry['IPV4'] + builtin = self.ProtocolBase.get_protocol('IPV4') + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + custom = _concrete('IPv4') + self.assertIs(self.registry['IPV4'], shipped) + self.assertIs(self.ProtocolBase.get_protocol('IPV4'), builtin) + messages = _registry_warnings(caught) + self.assertEqual(len(messages), 1, messages) + self.assertIn('keeping', messages[0]) + self.assertLess(messages[0].index(repr(shipped)), messages[0].index(repr(custom))) + + def test_a_clash_between_same_named_classes_tells_them_apart(self) -> 'None': + """#710: the reprs coincide, so the message carries each id, the kept one first.""" + first = _concrete('W2763Twice') + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + second = _concrete('W2763Twice') + self.assertEqual(repr(first), repr(second)) + self.assertIs(self.registry['W2763TWICE'], first) + messages = _registry_warnings(caught) + self.assertEqual(len(messages), 1, messages) + self.assertLess(messages[0].index(f'id={id(first):#x}'), messages[0].index(f'id={id(second):#x}')) + + def test_a_class_meeting_its_own_descriptor_is_silent(self) -> 'None': + """A class defined under a built-in's module and name is no clash, and leaves the descriptor.""" + from pcapkit.protocols.application.ngap.protocol import NGAP + + shipped = self.registry['NGAP'] + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + redefined = _concrete('NGAP', module=NGAP.__module__) + self.assertFalse(inspect.isabstract(redefined)) + self.assertEqual(_registry_warnings(caught), []) + self.assertIs(self.registry['NGAP'], shipped) + self.assertIs(self.ProtocolBase.get_protocol('NGAP'), NGAP) + + def test_a_refused_code_names_nothing(self) -> 'None': + from pcapkit.const.reg.transtype import TransType + from pcapkit.protocols.misc.pcap.frame.protocol import Frame + from pcapkit.utilities.exceptions import RegistryError + + before = dict(self.registry) + with self.assertRaises(RegistryError): + _concrete('W2763Refused', code={Frame: TransType.ICMP}) + self.assertEqual(self.registry, before) + + def test_register_protocol_replaces_and_puts_the_shipped_entry_back(self) -> 'None': + from pcapkit.foundation.registry import register_protocol + + shipped = self.registry['IPV4'] + builtin = self.ProtocolBase.get_protocol('IPV4') + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + custom = _concrete('IPv4') + + for replacement, displaced, entry in ((custom, builtin, custom), (builtin, custom, shipped)): + with self.subTest(replacement=replacement.__module__): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + register_protocol(replacement) + self.assertIs(self.registry['IPV4'], entry) + self.assertIs(self.ProtocolBase.get_protocol('IPV4'), replacement) + messages = _registry_warnings(caught) + self.assertEqual(len(messages), 1, messages) + self.assertIn(f'overwriting {displaced!r} with {replacement!r}', messages[0]) + + # Handing back what the name already shows is silent, and keeps the descriptor. + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + self.ProtocolBase.register_protocol(builtin) + self.assertEqual(_registry_warnings(caught), []) + self.assertIs(self.registry['IPV4'], shipped) + + def test_register_protocol_refuses_a_non_protocol(self) -> 'None': + from pcapkit.utilities.exceptions import RegistryError + + before = dict(self.registry) + for value in (object(), 'IPv4', object, self.registry['IPV4']): + with self.subTest(value=repr(value)), self.assertRaises(RegistryError): + self.ProtocolBase.register_protocol(value) # type: ignore[arg-type] + self.assertEqual(self.registry, before) + + +class GetterTests(_RegistryTestCase): + """``get_protocol`` and the removal of the module dict.""" + + def test_the_getter_resolves_a_name_in_any_case(self) -> 'None': + from pcapkit.corekit.module import ModuleDescriptor + from pcapkit.protocols.misc.raw.protocol import Raw + from pcapkit.protocols.transport.tcp.protocol import TCP + + for name in ('TCP', 'tcp', 'Tcp'): + with self.subTest(name=name): + self.assertIs(self.ProtocolBase.get_protocol(name), TCP) + self.assertIsInstance(self.registry['TCP'], ModuleDescriptor) # resolved, not written back + self.assertIsNone(self.ProtocolBase.get_protocol('W2763-NoSuch')) + self.assertIs(self.ProtocolBase.get_protocol('W2763-NoSuch', Raw), Raw) + + def test_a_descriptor_naming_no_class_raises(self) -> 'None': + from pcapkit.corekit.module import ModuleDescriptor + from pcapkit.utilities.exceptions import CaughtByCaller, ProtocolError + + self.registry['W2763LOST'] = ModuleDescriptor('pcapkit.protocols.misc.raw.protocol', 'W2763Lost') + with CaughtByCaller(ProtocolError), self.assertRaises(ProtocolError): + self.ProtocolBase.get_protocol('W2763Lost') + + def test_the_module_dict_is_gone(self) -> 'None': + import pcapkit.protocols + + with self.assertRaises(AttributeError): + pcapkit.protocols.__proto__ # pylint: disable=pointless-statement + with self.assertRaises(ImportError): + exec('from pcapkit.protocols import __proto__', {}) # nosec: B102 + self.assertNotIn('__proto__', dir(pcapkit.protocols)) + self.assertNotIn('__proto__', vars(pcapkit.protocols)) + + +class RegistryViewTests(_RegistryTestCase): + """``AnyProtocol.registry``, the read-only view :class:`~pcapkit.protocols.protocol.ProtocolMeta` defines.""" + + def _classes(self) -> 'list[type]': + from pcapkit.protocols.internet.ipv4.protocol import IPv4 + from pcapkit.protocols.misc.raw.protocol import Raw + from pcapkit.protocols.protocol import Protocol + + return [self.ProtocolBase, Protocol, IPv4, Raw, _concrete('W2763Viewer')] + + def test_the_view_is_read_only(self) -> 'None': + import types + + view = self.ProtocolBase.registry + self.assertIsInstance(view, types.MappingProxyType) + with self.assertRaises(TypeError): + view['W2763RO'] = self.ProtocolBase # type: ignore[index] + with self.assertRaises(TypeError): + del view['IPV4'] # type: ignore[attr-defined] + with self.assertRaises(AttributeError): + self.ProtocolBase.registry = {} # type: ignore[misc] + self.assertNotIn('W2763RO', self.registry) + self.assertIn('IPV4', self.registry) + + def test_the_view_holds_the_entries_as_stored(self) -> 'None': + from pcapkit.corekit.module import ModuleDescriptor + + view = self.ProtocolBase.registry + self.assertEqual(sorted(view), sorted(self.registry)) + for key, entry in self.registry.items(): + with self.subTest(key=key): + self.assertIs(view[key], entry) + self.assertIsInstance(view['IPV4'], ModuleDescriptor) + + def test_every_protocol_class_reaches_the_same_live_view(self) -> 'None': + views = [(klass.__name__, klass.registry) for klass in self._classes()] + added = _concrete('W2763Later') + for name, view in views: + with self.subTest(through=name): + self.assertEqual(dict(view), dict(self.registry)) + self.assertIs(view['W2763LATER'], added) + + +#: What every child runs first: a cold ``import pcapkit`` from this tree. +PRELUDE = f''' +import importlib, json, sys, warnings + +def loaded(*names): + return [name for name in names if name in sys.modules] + +def emit(**kwargs): + print(json.dumps(kwargs)) + +import pcapkit +assert pcapkit.__file__.startswith({str(ROOT)!r}), pcapkit.__file__ +from pcapkit.corekit.module import ModuleDescriptor +from pcapkit.protocols.protocol import ProtocolBase +''' + + +def run_child(code: 'str') -> 'dict[str, Any]': + """Run :data:`PRELUDE` then ``code`` in a fresh interpreter; return its last ``emit`` line.""" + environ = dict(os.environ) + environ.pop('PYTHONSAFEPATH', None) + environ['PYTHONPATH'] = os.pathsep.join( + [str(ROOT), environ['PYTHONPATH']] if environ.get('PYTHONPATH') else [str(ROOT)]) + result = subprocess.run([sys.executable, '-c', PRELUDE + textwrap.dedent(code)], # nosec: B603 + cwd=str(ROOT), env=environ, capture_output=True, text=True, + timeout=300, check=False) + if result.returncode != 0: + raise AssertionError(f'child exited {result.returncode}:\n{result.stderr}') + return json.loads(result.stdout.splitlines()[-1]) + + +class ColdProcessTests(unittest.TestCase): + """What the registry imports, and when, from a cold ``import pcapkit``.""" + + def test_reading_the_view_imports_no_protocol(self) -> 'None': + out = run_child(''' + from pcapkit.protocols.protocol import Protocol + before = sorted(name for name in sys.modules if name.startswith('pcapkit.protocols.')) + views = [ProtocolBase.registry, Protocol.registry] + entries = [sorted((key, list(entry)) for key, entry in view.items()) for view in views] + after = sorted(name for name in sys.modules if name.startswith('pcapkit.protocols.')) + emit(new=sorted(set(after) - set(before)), same=entries[0] == entries[1], + descriptors=all(isinstance(entry, ModuleDescriptor) for entry in views[0].values()), + count=len(entries[0])) + ''') + self.assertEqual(out['new'], []) + self.assertTrue(out['same']) + self.assertTrue(out['descriptors']) + self.assertGreater(out['count'], 0) + + def test_the_getter_imports_a_class_only_when_resolving(self) -> 'None': + out = run_child(''' + module = 'pcapkit.protocols.application.ngap.protocol' + before = loaded(module) + entry = ProtocolBase.__registry__['NGAP'] + listed = ['NGAP' in ProtocolBase.__registry__, loaded(module)] + klass = ProtocolBase.get_protocol('ngap') + emit(before=before, listed=listed, entry=list(entry), after=loaded(module), + same=klass is sys.modules[module].NGAP, kept=ProtocolBase.__registry__['NGAP'] is entry) + ''') + module = 'pcapkit.protocols.application.ngap.protocol' + self.assertEqual(out['before'], []) + self.assertEqual(out['listed'], [True, []]) + self.assertEqual(out['entry'], [module, 'NGAP']) + self.assertEqual(out['after'], [module]) + self.assertTrue(out['same']) + self.assertTrue(out['kept']) + + def test_a_reloaded_built_in_is_silent_and_followed(self) -> 'None': + out = run_child(''' + from pcapkit.utilities.warnings import RegistryWarning + module = importlib.import_module('pcapkit.protocols.application.ngap.protocol') + old = ProtocolBase.get_protocol('NGAP') + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + importlib.reload(module) + new = ProtocolBase.get_protocol('NGAP') + emit(messages=[str(w.message) for w in caught if issubclass(w.category, RegistryWarning)], + followed=[new is not old, new is module.NGAP], + descriptor=isinstance(ProtocolBase.__registry__['NGAP'], ModuleDescriptor)) + ''') + self.assertEqual(out['messages'], []) + self.assertEqual(out['followed'], [True, True]) + self.assertTrue(out['descriptor']) + + def test_threads_racing_on_a_first_resolution_get_one_class(self) -> 'None': + """Every thread resolving a not-yet-imported protocol at once gets the class its module binds.""" + out = run_child(''' + import random, threading + sys.setswitchinterval(1e-6) + names = sorted(name for name, entry in ProtocolBase.__registry__.items() + if entry.module not in sys.modules) + barrier = threading.Barrier(16) + results, errors = {}, [] + + def worker(seed): + order = names[:] + random.Random(seed).shuffle(order) + barrier.wait() + try: + results[seed] = {name: ProtocolBase.get_protocol(name) for name in order} + except BaseException as exc: # noqa: BLE001 + errors.append(f'{type(exc).__name__}: {exc}') + + threads = [threading.Thread(target=worker, args=(seed,)) for seed in range(16)] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + wrong = sorted(name for name in names for got in results.values() + if got[name] is not getattr(sys.modules[ProtocolBase.__registry__[name].module], + ProtocolBase.__registry__[name].name)) + emit(names=names, errors=errors, threads=len(results), wrong=wrong) + ''') + self.assertGreater(len(out['names']), 1, 'no protocol was left unimported to race on') + self.assertEqual(out['errors'], []) + self.assertEqual(out['threads'], 16) + self.assertEqual(out['wrong'], []) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_base_class_contract.py b/tests/test_base_class_contract.py index adb540eda8..2e5c9888ca 100644 --- a/tests/test_base_class_contract.py +++ b/tests/test_base_class_contract.py @@ -309,7 +309,7 @@ def setUp(self) -> None: #: ``(label, base, public, keyword, registry accessor)`` per suite, #: resolved fresh in :meth:`setUp` rather than carried as a class #: attribute -- see this method's own docstring. The protocols suite is - #: absent on purpose: its name registry is ``pcapkit.protocols.__proto__`` + #: absent on purpose: its name registry is ``ProtocolBase.__registry__`` #: and its hook takes no registration keyword of this shape, so it is #: covered by :meth:`test_library_classes_are_not_descendants_of_the_public_class` #: and by ``tests/protocols/`` instead.