diff --git a/pcapkit/const/reg/apptype/apptype.py b/pcapkit/const/reg/apptype/apptype.py index 1408c90257..d07c1d3bf8 100644 --- a/pcapkit/const/reg/apptype/apptype.py +++ b/pcapkit/const/reg/apptype/apptype.py @@ -15,7 +15,7 @@ from aenum import IntEnum, StrEnum, auto, extend_enum -from pcapkit.corekit.enum import EnumRegistry +from pcapkit.corekit.enum import NO_DEFAULT, EnumLookup, EnumRegistry __all__ = ['TransportProtocol', 'AppType'] @@ -25,7 +25,7 @@ from pcapkit.corekit.multidict import MultiDict -class TransportProtocol(IntEnum): +class TransportProtocol(EnumLookup, IntEnum): """Transport layer protocol.""" # mypy has no aenum plugin, so this class is a plain class to it: every @@ -87,19 +87,70 @@ class TransportProtocol(IntEnum): #: Datagram Congestion Control Protocol. dccp = cast('TransportProtocol', auto()) - @staticmethod - def get(key: 'int | str') -> 'TransportProtocol': + @classmethod + def get(cls, key: 'int | str', default: 'Any' = NO_DEFAULT) -> 'TransportProtocol': """Backport support for original codes. + Delegates to :meth:`~pcapkit.corekit.enum.EnumLookup.get` for GitHub + issue #877's re-parenting, but keeps this override rather than + dropping it -- two behaviours the base does not reproduce on its own: + + * **Case folding.** This class has always matched a name + case-insensitively (``key.lower()``); the base's own ``str`` + branch is case-sensitive. Lowering ``key`` before delegating + reproduces that: every member name here is already lower-case, so + a lowered ``key`` still hits the base's exact ``_member_map_`` + lookup. + * **The refusal.** Maintainer ruling on GitHub PR #836: "Do not + allow extension of TransportProtocol at all." The base's own miss + on a ``str`` key raises a bare :exc:`KeyError`; this class has + always raised :exc:`ValueError` naming the rejected key, which is + what every caller and test here already depends on, so a name + miss is caught and re-raised in that shape rather than left as the + base's own exception. + + The base is a :class:`classmethod` + (:meth:`~pcapkit.corekit.enum.EnumLookup.get`), so this override + moves from :class:`staticmethod` to :class:`classmethod` to + delegate at all -- the same move GitHub issue #908 and #915 made for + :meth:`~pcapkit.const.http.method.Method.get`. Grepped every call + site in this tree for GitHub issue #877: all call this method by + name, none take it as a bare callable or introspect ``__func__``, + so the switch is not caller-visible. + + ``default`` did not exist on this override before this change -- + the original had no such parameter at all, which is a genuine LSP + violation once this class' ``get`` is a :class:`classmethod` + override of one that has it: a caller holding a + :class:`~pcapkit.corekit.enum.EnumLookup`-typed reference could pass + ``default=`` and, before this, would have hit a + :exc:`TypeError` at this subclass. Forwarded verbatim to + :meth:`~pcapkit.corekit.enum.EnumLookup.get` rather than + reimplemented, so it behaves exactly as the base's own ``default`` + does: a fallback to an *already-registered* value, resolved through + ``_value2member_map_`` and never through the constructor, so + passing one still cannot mint. Every call site in this tree omits + it, so this is purely an added, backward-compatible capability, not + a change to anything this tree exercises today. + Args: key: Key to get enum item. + default: An already-registered value to fall back to when + ``key`` resolves to nothing. :data:`~pcapkit.corekit.enum. + NO_DEFAULT`, the default, means *no default* -- see + :meth:`~pcapkit.corekit.enum.EnumLookup.get`. + + Raises: + ValueError: If ``key`` names no member, by name or by value, and + there is no usable ``default``. :meta private: """ - if isinstance(key, int): - return TransportProtocol(key) - if key.lower() in TransportProtocol.__members__: - return TransportProtocol[key.lower()] # type: ignore[misc] + if isinstance(key, str): + try: + return super().get(key.lower(), default) + except KeyError: + raise ValueError(f'{key!r} is not a valid {cls.__name__}') from None # NOTE: maintainer ruling on this PR (#836): "Do not allow extension # of TransportProtocol at all." A name that is not a declared member # used to mint a brand-new one here, at ``max_val + 1`` (before that, @@ -117,7 +168,19 @@ def get(key: 'int | str') -> 'TransportProtocol': # splitting." A ``'|'``-joined name is therefore not special any # more -- it is simply not the name of a declared member, and gets # the same message as any other one that is not. - raise ValueError(f'{key!r} is not a valid {TransportProtocol.__name__}') + # + # NOTE: the delegation below is exception-compatible for the keys this + # signature admits -- an unrecognised :class:`int` still reaches the + # caller as the same plain :exc:`ValueError`. It is not compatible for + # keys outside it: ``None``, a :class:`float` and an unhashable key + # used to raise :exc:`AttributeError` from the ``key.lower()`` this + # branch no longer reaches, and now raise :exc:`ValueError` (or, for a + # :class:`float`, resolve -- ``get(1.0)`` answers ``tcp``, since + # ``cls(key)`` accepts whatever :class:`int` equality accepts). No + # caller in this tree can reach any of those: the only live call site + # passes ``proto.lower()``, always a :class:`str`. The new shape is + # what every other ``EnumLookup`` subclass already does. + return super().get(key, default) # NOTE: ``_missing_`` used to range-check ``value`` and then defer to # :mod:`aenum`'s own ``Flag._missing_``, which is what composed an diff --git a/pcapkit/corekit/infoclass.py b/pcapkit/corekit/infoclass.py index e9df2af48d..e5106198d8 100644 --- a/pcapkit/corekit/infoclass.py +++ b/pcapkit/corekit/infoclass.py @@ -16,6 +16,7 @@ import itertools from typing import TYPE_CHECKING, Generic, TypeVar +from pcapkit.corekit.enum import EnumLookup from pcapkit.utilities.compat import Mapping, final from pcapkit.utilities.exceptions import InfoError, UnsupportedCall, stacklevel from pcapkit.utilities.warnings import InfoWarning, warn @@ -31,8 +32,15 @@ ST = TypeVar('ST', bound='Type[Info]') -class FinalisedState(enum.IntEnum): - """Finalised state.""" +class FinalisedState(EnumLookup, enum.IntEnum): + """Finalised state. + + Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub + issue #877's ruling that every non-registry enumeration shares that + lookup contract -- pure re-parenting, since this class defines neither + ``get`` nor ``_missing_`` of its own to reconcile with the base. + + """ #: Not finalised. NONE = enum.auto() diff --git a/pcapkit/foundation/reassembly/data/data.py b/pcapkit/foundation/reassembly/data/data.py index 9733ca960d..c52561ecec 100644 --- a/pcapkit/foundation/reassembly/data/data.py +++ b/pcapkit/foundation/reassembly/data/data.py @@ -3,6 +3,7 @@ from typing import TYPE_CHECKING +from pcapkit.corekit.enum import EnumLookup from pcapkit.corekit.infoclass import Info, info_final from pcapkit.utilities.compat import StrEnum, auto @@ -17,9 +18,14 @@ from pcapkit.protocols.protocol import ProtocolBase -class Completion(StrEnum): +class Completion(EnumLookup, StrEnum): """How completely a datagram was reassembled, and why it stopped. + Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub + issue #877's ruling that every non-registry enumeration shares that + lookup contract -- pure re-parenting, since this class defines neither + ``get`` nor ``_missing_`` of its own to reconcile with the base. + This is the value of :attr:`Datagram.completed `. That field used to be a plain :obj:`bool`, and this enumeration is a widening diff --git a/pcapkit/protocols/application/ftp.py b/pcapkit/protocols/application/ftp.py index 2d6665eec4..5ee94f81de 100644 --- a/pcapkit/protocols/application/ftp.py +++ b/pcapkit/protocols/application/ftp.py @@ -16,6 +16,7 @@ from pcapkit.const.ftp.command import Command as Enum_Command from pcapkit.const.ftp.return_code import ReturnCode as Enum_ReturnCode +from pcapkit.corekit.enum import EnumLookup from pcapkit.protocols.application.application import Application from pcapkit.protocols.data.application.ftp import FTP as Data_FTP from pcapkit.protocols.data.application.ftp import Request as Data_Request @@ -37,8 +38,15 @@ FTP_RESPONSE = re.compile(rb'^(?P[0-9]{3})(?P\-)?( +(?P.*))?\r\n$', re.I) -class Type(StrEnum): - """FTP packet type.""" +class Type(EnumLookup, StrEnum): + """FTP packet type. + + Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub + issue #877's ruling that every non-registry enumeration shares that + lookup contract -- pure re-parenting, since this class defines neither + ``get`` nor ``_missing_`` of its own to reconcile with the base. + + """ #: Request packet. REQUEST = auto() diff --git a/pcapkit/protocols/application/httpv1.py b/pcapkit/protocols/application/httpv1.py index 1da5fd7a97..5bda637997 100644 --- a/pcapkit/protocols/application/httpv1.py +++ b/pcapkit/protocols/application/httpv1.py @@ -32,6 +32,7 @@ from pcapkit.const.http.method import Method as Enum_Method from pcapkit.const.http.status_code import StatusCode as Enum_StatusCode +from pcapkit.corekit.enum import EnumLookup from pcapkit.corekit.multidict import OrderedMultiDict from pcapkit.protocols.application.http import HTTP as HTTPBase from pcapkit.protocols.data.application.httpv1 import HTTP as Data_HTTP @@ -137,8 +138,15 @@ def _test_start_line(data: 'bytes') -> 'bool': ) -class Type(StrEnum): - """HTTP packet type.""" +class Type(EnumLookup, StrEnum): + """HTTP packet type. + + Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub + issue #877's ruling that every non-registry enumeration shares that + lookup contract -- pure re-parenting, since this class defines neither + ``get`` nor ``_missing_`` of its own to reconcile with the base. + + """ #: Request packet. REQUEST = auto() diff --git a/pcapkit/protocols/application/ngap.py b/pcapkit/protocols/application/ngap.py index 62511c4f19..92d071278f 100644 --- a/pcapkit/protocols/application/ngap.py +++ b/pcapkit/protocols/application/ngap.py @@ -104,6 +104,7 @@ from pcapkit.const.ngap.procedure_code import ProcedureCode as Enum_ProcedureCode from pcapkit.const.ngap.protocol_ie import ProtocolIE as Enum_ProtocolIE +from pcapkit.corekit.enum import NO_DEFAULT, EnumLookup from pcapkit.corekit.infoclass import Info from pcapkit.protocols.application.application import Application from pcapkit.protocols.data.application.ngap import IE as Data_IE @@ -193,12 +194,17 @@ def load_pycrate() -> 'Optional[Any]': ############################################################################## -class PDUKind(StrEnum): +class PDUKind(EnumLookup, StrEnum): """Which alternative of the ``NGAP-PDU`` ``CHOICE`` a PDU is. The values are spelled as the ASN.1 identifiers, so that a name decoded by |pycrate|_ resolves by value. + Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub + issue #877's ruling that every non-registry enumeration shares that + lookup contract -- pure re-parenting, since this class defines neither + ``get`` nor ``_missing_`` of its own to reconcile with the base. + """ #: A procedure's request, or a class 2 procedure's only message. @@ -209,7 +215,7 @@ class PDUKind(StrEnum): UNSUCCESSFUL_OUTCOME = 'unsuccessfulOutcome' -class Criticality(IntEnum): +class Criticality(EnumLookup, IntEnum): """[Criticality] What a receiver must do with an IE it does not understand. Members are named for the ASN.1 identifiers rather than upper-cased, so @@ -226,43 +232,99 @@ class Criticality(IntEnum): #: Ignore the IE, carry on, and report it. notify = 2 - @staticmethod - def get(key: 'int | str | Criticality') -> 'Criticality': + @classmethod + def get(cls, key: 'int | str | Criticality', default: 'Any' = NO_DEFAULT) -> 'Criticality': """Backport support for original codes. + Delegates to :meth:`~pcapkit.corekit.enum.EnumLookup.get` for GitHub + issue #877's re-parenting. For every key the signature admits, the + base reproduces the branch this override used to hand-roll: a + ``Criticality`` key is also an :class:`int` (this is an + :class:`~aenum.IntEnum`) and resolves through the base's non-``str`` + path, ``cls(key)``, which -- exactly like the removed + ``isinstance(key, Criticality): return key`` branch -- hands back the + identical, canonical member rather than a new one; a plain + :class:`str` resolves through the base's name lookup, the same + ``Criticality[key]`` this override used to spell directly. + + Outside that signature the two do differ, which is worth stating + rather than leaving for someone to discover. ``cls(key)`` accepts + anything :class:`int` equality accepts, so ``get(1.0)`` now returns + ``Criticality.ignore`` where the removed code raised + :exc:`ValueError`, and an unhashable key raises :exc:`ValueError` + rather than the :exc:`TypeError` the old ``Criticality[key]`` lookup + produced. Both are out of contract, no caller in this tree can reach + them -- the three live call sites pass pycrate-decoded :class:`str` + or :class:`int` -- and the new shape is what every other + :class:`~pcapkit.corekit.enum.EnumLookup` subclass already does, so + this is alignment rather than a regression. + + The one behaviour the base does not reproduce is the exception this + class has always raised for an unresolved name: a bare + :exc:`KeyError` there, versus this class's own :exc:`ValueError` + naming the rejected key -- so a name miss is still caught and + re-raised in that shape. An unresolved *value* is unaffected either + way: it already reaches the caller as :exc:`ValueError`, raised by + :meth:`_missing_` below, on both the removed code path and the + base's. + + The base is a :class:`classmethod` + (:meth:`~pcapkit.corekit.enum.EnumLookup.get`), so this override + moves from :class:`staticmethod` to :class:`classmethod` to + delegate at all -- the same move GitHub issue #908 and #915 made for + :meth:`~pcapkit.const.http.method.Method.get`. Every call site in + this tree calls this method by name; none take it as a bare + callable or introspect ``__func__``, so the switch is not + caller-visible. + Unlike :meth:`ProcedureCode.get ` and :meth:`ProtocolIE.get `, - this takes no ``default``. Those two answer an in-range value they have - not seen with a throwaway, non-registering member (see - :meth:`~pcapkit.corekit.enum.EnumRegistry._unregistered_member`), which - is the right answer for a registry 3GPP keeps assigning new codes to. - ``Criticality`` cannot grow. It is an ASN.1 ``ENUMERATED`` with no - extension marker, so a fourth value is unencodable and a lookup for one - is a bug rather than a version skew -- see :meth:`_missing_`. A - ``default`` parameter here would have to be ignored, and one that is - declared, documented and ignored is worse than one that is absent. + this still never manufactures a member for an in-range value it has + not seen -- ``Criticality`` cannot grow, unlike those two, which + answer with a throwaway, non-registering member (see + :meth:`~pcapkit.corekit.enum.EnumRegistry._unregistered_member`) for + a registry 3GPP keeps assigning new codes to. It is an ASN.1 + ``ENUMERATED`` with no extension marker, so a fourth value is + unencodable and a lookup for one is a bug rather than a version skew + -- see :meth:`_missing_`. + + ``default`` did not exist on this override before this change -- + the original had no such parameter at all, which is a genuine LSP + violation once this class' ``get`` is a :class:`classmethod` + override of one that has it (mypy's ``[override]`` check catches + exactly this shape). Forwarded verbatim to + :meth:`~pcapkit.corekit.enum.EnumLookup.get` rather than + reimplemented, so it behaves exactly as the base's own ``default`` + does: a fallback to an *already-registered* member, resolved + through ``_value2member_map_`` and never through the constructor, + so passing one still cannot mint a fourth. Every call site in this + tree omits it, so this is purely an added, backward-compatible + capability -- not the "declared, documented and ignored" shape an + earlier revision of this docstring rejected, since it is now + genuinely honoured rather than a parameter that would have to be + silently dropped. Args: key: Key to get enum item. + default: An already-registered value to fall back to when + ``key`` resolves to nothing. :data:`~pcapkit.corekit.enum. + NO_DEFAULT`, the default, means *no default*. Returns: The matching member. Raises: - ValueError: If ``key`` names no member. Raised for an unknown name as - well as an unknown value, so that the two ways of getting this - wrong do not report differently. + ValueError: If ``key`` names no member and there is no usable + ``default``. Raised for an unknown name as well as an + unknown value, so that the two ways of getting this wrong + do not report differently. :meta private: """ - if isinstance(key, Criticality): - return key - if isinstance(key, int): - return Criticality(key) try: - return Criticality[key] # type: ignore[misc] + return super().get(key, default) except KeyError: - raise ValueError('%r is not a valid %s' % (key, Criticality.__name__)) from None + raise ValueError('%r is not a valid %s' % (key, cls.__name__)) from None @classmethod def _missing_(cls, value: 'int') -> 'NoReturn': diff --git a/pcapkit/protocols/misc/pcapng.py b/pcapkit/protocols/misc/pcapng.py index 4d0a0fb171..9abcc97ee5 100644 --- a/pcapkit/protocols/misc/pcapng.py +++ b/pcapkit/protocols/misc/pcapng.py @@ -38,6 +38,7 @@ from pcapkit.const.pcapng.tls_key_label import TLSKeyLabel as Enum_TLSKeyLabel from pcapkit.const.pcapng.verdict_type import VerdictType as Enum_VerdictType from pcapkit.const.reg.linktype import LinkType as Enum_LinkType +from pcapkit.corekit.enum import EnumLookup from pcapkit.corekit.module import ModuleDescriptor from pcapkit.corekit.multidict import OrderedMultiDict from pcapkit.corekit.version import VersionInfo @@ -245,8 +246,15 @@ def _option_key(code: 'Enum_OptionType') -> 'Union[Enum_OptionType, Tuple[str, i return code -class PacketDirection(enum.IntEnum): - """Packet direction for ``epb_flags`` options.""" +class PacketDirection(EnumLookup, enum.IntEnum): + """Packet direction for ``epb_flags`` options. + + Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub + issue #877's ruling that every non-registry enumeration shares that + lookup contract -- pure re-parenting, since this class defines neither + ``get`` nor ``_missing_`` of its own to reconcile with the base. + + """ #: Information not available. UNKNOWN = 0b00 @@ -256,8 +264,15 @@ class PacketDirection(enum.IntEnum): OUTBOUND = 0b10 -class PacketReception(enum.IntEnum): - """Reception type for ``epb_flags`` options.""" +class PacketReception(EnumLookup, enum.IntEnum): + """Reception type for ``epb_flags`` options. + + Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub + issue #877's ruling that every non-registry enumeration shares that + lookup contract -- pure re-parenting, since this class defines neither + ``get`` nor ``_missing_`` of its own to reconcile with the base. + + """ #: Not specified. UNKNOWN = 0b000 @@ -294,8 +309,15 @@ class PacketReception(enum.IntEnum): TLSKeyLabel = Enum_TLSKeyLabel -class WireGuardKeyLabel(StrEnum): - """WireGuard key log label.""" +class WireGuardKeyLabel(EnumLookup, StrEnum): + """WireGuard key log label. + + Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub + issue #877's ruling that every non-registry enumeration shares that + lookup contract -- pure re-parenting, since this class defines neither + ``get`` nor ``_missing_`` of its own to reconcile with the base. + + """ LOCAL_STATIC_PRIVATE_KEY = 'LOCAL_STATIC_PRIVATE_KEY' REMOTE_STATIC_PUBLIC_KEY = 'REMOTE_STATIC_PUBLIC_KEY' diff --git a/pcapkit/protocols/schema/application/httpv2.py b/pcapkit/protocols/schema/application/httpv2.py index d044a50382..b5e6298e97 100644 --- a/pcapkit/protocols/schema/application/httpv2.py +++ b/pcapkit/protocols/schema/application/httpv2.py @@ -9,6 +9,7 @@ from pcapkit.const.http.error_code import ErrorCode as Enum_ErrorCode from pcapkit.const.http.frame import Frame as Enum_Frame from pcapkit.const.http.setting import Setting as Enum_Setting +from pcapkit.corekit.enum import EnumLookup from pcapkit.corekit.fields.collections import ListField from pcapkit.corekit.fields.misc import ConditionalField, SchemaField, SwitchField from pcapkit.corekit.fields.numbers import EnumField, NumberField, UInt8Field, UInt32Field @@ -123,8 +124,19 @@ class FrameType(EnumSchema[Enum_Frame]): __enum__ = collections.defaultdict(lambda: UnassignedFrame) - class Flags(enum.IntFlag): - """Flags enumeration for HTTP/2 frames.""" + class Flags(EnumLookup, enum.IntFlag): + """Flags enumeration for HTTP/2 frames. + + Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per + GitHub issue #877's ruling that every non-registry enumeration + shares that lookup contract. The six concrete per-frame subclasses + below each declare ``class Flags(FrameType.Flags):`` with no base + list of their own, so they inherit :class:`EnumLookup` transitively + through this one re-parent rather than needing it repeated -- + verified at runtime for GitHub issue #877 (see the session report), + not merely assumed from the MRO rules. + + """ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': """Revise ``schema`` data after unpacking process. diff --git a/pcapkit/vendor/reg/apptype/apptype.py b/pcapkit/vendor/reg/apptype/apptype.py index f1c70e3657..221ded5190 100644 --- a/pcapkit/vendor/reg/apptype/apptype.py +++ b/pcapkit/vendor/reg/apptype/apptype.py @@ -108,7 +108,7 @@ from aenum import IntEnum, StrEnum, auto, extend_enum -from pcapkit.corekit.enum import EnumRegistry +from pcapkit.corekit.enum import NO_DEFAULT, EnumLookup, EnumRegistry __all__ = ['TransportProtocol', '{NAME}'] @@ -118,7 +118,7 @@ from pcapkit.corekit.multidict import MultiDict -class TransportProtocol(IntEnum): +class TransportProtocol(EnumLookup, IntEnum): """Transport layer protocol.""" # mypy has no aenum plugin, so this class is a plain class to it: every @@ -180,19 +180,70 @@ class TransportProtocol(IntEnum): #: Datagram Congestion Control Protocol. dccp = cast('TransportProtocol', auto()) - @staticmethod - def get(key: 'int | str') -> 'TransportProtocol': + @classmethod + def get(cls, key: 'int | str', default: 'Any' = NO_DEFAULT) -> 'TransportProtocol': """Backport support for original codes. + Delegates to :meth:`~pcapkit.corekit.enum.EnumLookup.get` for GitHub + issue #877's re-parenting, but keeps this override rather than + dropping it -- two behaviours the base does not reproduce on its own: + + * **Case folding.** This class has always matched a name + case-insensitively (``key.lower()``); the base's own ``str`` + branch is case-sensitive. Lowering ``key`` before delegating + reproduces that: every member name here is already lower-case, so + a lowered ``key`` still hits the base's exact ``_member_map_`` + lookup. + * **The refusal.** Maintainer ruling on GitHub PR #836: "Do not + allow extension of TransportProtocol at all." The base's own miss + on a ``str`` key raises a bare :exc:`KeyError`; this class has + always raised :exc:`ValueError` naming the rejected key, which is + what every caller and test here already depends on, so a name + miss is caught and re-raised in that shape rather than left as the + base's own exception. + + The base is a :class:`classmethod` + (:meth:`~pcapkit.corekit.enum.EnumLookup.get`), so this override + moves from :class:`staticmethod` to :class:`classmethod` to + delegate at all -- the same move GitHub issue #908 and #915 made for + :meth:`~pcapkit.const.http.method.Method.get`. Grepped every call + site in this tree for GitHub issue #877: all call this method by + name, none take it as a bare callable or introspect ``__func__``, + so the switch is not caller-visible. + + ``default`` did not exist on this override before this change -- + the original had no such parameter at all, which is a genuine LSP + violation once this class' ``get`` is a :class:`classmethod` + override of one that has it: a caller holding a + :class:`~pcapkit.corekit.enum.EnumLookup`-typed reference could pass + ``default=`` and, before this, would have hit a + :exc:`TypeError` at this subclass. Forwarded verbatim to + :meth:`~pcapkit.corekit.enum.EnumLookup.get` rather than + reimplemented, so it behaves exactly as the base's own ``default`` + does: a fallback to an *already-registered* value, resolved through + ``_value2member_map_`` and never through the constructor, so + passing one still cannot mint. Every call site in this tree omits + it, so this is purely an added, backward-compatible capability, not + a change to anything this tree exercises today. + Args: key: Key to get enum item. + default: An already-registered value to fall back to when + ``key`` resolves to nothing. :data:`~pcapkit.corekit.enum. + NO_DEFAULT`, the default, means *no default* -- see + :meth:`~pcapkit.corekit.enum.EnumLookup.get`. + + Raises: + ValueError: If ``key`` names no member, by name or by value, and + there is no usable ``default``. :meta private: """ - if isinstance(key, int): - return TransportProtocol(key) - if key.lower() in TransportProtocol.__members__: - return TransportProtocol[key.lower()] # type: ignore[misc] + if isinstance(key, str): + try: + return super().get(key.lower(), default) + except KeyError: + raise ValueError(f'{{key!r}} is not a valid {{cls.__name__}}') from None # NOTE: maintainer ruling on this PR (#836): "Do not allow extension # of TransportProtocol at all." A name that is not a declared member # used to mint a brand-new one here, at ``max_val + 1`` (before that, @@ -210,7 +261,19 @@ def get(key: 'int | str') -> 'TransportProtocol': # splitting." A ``'|'``-joined name is therefore not special any # more -- it is simply not the name of a declared member, and gets # the same message as any other one that is not. - raise ValueError(f'{{key!r}} is not a valid {{TransportProtocol.__name__}}') + # + # NOTE: the delegation below is exception-compatible for the keys this + # signature admits -- an unrecognised :class:`int` still reaches the + # caller as the same plain :exc:`ValueError`. It is not compatible for + # keys outside it: ``None``, a :class:`float` and an unhashable key + # used to raise :exc:`AttributeError` from the ``key.lower()`` this + # branch no longer reaches, and now raise :exc:`ValueError` (or, for a + # :class:`float`, resolve -- ``get(1.0)`` answers ``tcp``, since + # ``cls(key)`` accepts whatever :class:`int` equality accepts). No + # caller in this tree can reach any of those: the only live call site + # passes ``proto.lower()``, always a :class:`str`. The new shape is + # what every other ``EnumLookup`` subclass already does. + return super().get(key, default) # NOTE: ``_missing_`` used to range-check ``value`` and then defer to # :mod:`aenum`'s own ``Flag._missing_``, which is what composed an diff --git a/tests/const/test_const_enum_get.py b/tests/const/test_const_enum_get.py index c3e4e8a735..75d79426f1 100644 --- a/tests/const/test_const_enum_get.py +++ b/tests/const/test_const_enum_get.py @@ -109,21 +109,26 @@ }) #: Enums carrying no ``get(key, default)``, so there is no ``default`` to drop. -#: The first two are helper enums describing a registry's columns rather than -#: registries themselves and have no ``get`` at all; -#: :class:`~pcapkit.const.reg.apptype.TransportProtocol` has a ``get`` whose -#: signature takes no ``default`` -- which is why the rewrite had to check the -#: signature rather than pattern-match the body. +#: These two are helper enums describing a registry's columns rather than +#: registries themselves and have no ``get`` at all -- held here rather than +#: moved by GitHub issue #877, since :mod:`pcapkit.const.ftp.command` is held +#: by #913 pending its merge. #: #: :class:`~pcapkit.const.ftp.return_code.GroupingInformation` and #: :class:`~pcapkit.const.ftp.return_code.ResponseKind` were here too until #: GitHub issue #860 step 2 brought them onto #: :class:`~pcapkit.corekit.enum.EnumRegistry` -- they inherit the base #: ``get(key, default)`` now, so they moved into the main sweep below instead. +#: :class:`~pcapkit.const.reg.apptype.TransportProtocol` was here too until +#: GitHub issue #877's re-parenting onto +#: :class:`~pcapkit.corekit.enum.EnumLookup` gave its own ``get`` override a +#: ``default`` parameter for the first time -- forwarded verbatim to the +#: base, purely to keep the override's signature a valid ``classmethod`` +#: override of one that already had it -- so it moved into the main sweep +#: below as well. EXPECTED_WITHOUT_AN_INTEGER_DEFAULT = frozenset({ 'pcapkit.const.ftp.command.CommandType', 'pcapkit.const.ftp.command.ConformanceRequirement', - 'pcapkit.const.reg.apptype.apptype.TransportProtocol', }) #: :class:`~pcapkit.const.pcapng.filter_type.FilterType` declares *no* static @@ -362,8 +367,11 @@ def test_every_integer_path_consults_the_default(self) -> None: # and into this sweep; plus 2 for GitHub issue #880's # ngap.ProcedureCode and ngap.ProtocolIE, which take the base # ``get(key, default)`` from the moment they exist under - # pcapkit.const at all. - self.assertEqual(covered, 114) + # pcapkit.const at all; plus 1 for GitHub issue #877's re-parenting of + # ``TransportProtocol`` onto ``EnumLookup``, which gave its own ``get`` + # override a forwarding ``default`` parameter and moved it out of + # ``EXPECTED_WITHOUT_AN_INTEGER_DEFAULT`` the same way. + self.assertEqual(covered, 115) def test_the_always_resolving_registries_have_nothing_to_fall_back_to(self) -> None: """The two registries excused from the sweep, and why. diff --git a/tests/corekit/test_enum_lookup_reparent_877_unit.py b/tests/corekit/test_enum_lookup_reparent_877_unit.py new file mode 100644 index 0000000000..1eef1538c6 --- /dev/null +++ b/tests/corekit/test_enum_lookup_reparent_877_unit.py @@ -0,0 +1,438 @@ +# -*- coding: utf-8 -*- +"""Phase 2 of GitHub issue #877, the unblocked half: re-parenting 11 helper +enumerations across 8 files onto :class:`~pcapkit.corekit.enum.EnumLookup`. + +The owner's ruling, verbatim: *"I still prefer to reparent all enums until a +in house base class so that they can share common contracts."* Phase 1 +(#906) split :class:`~pcapkit.corekit.enum.EnumLookup` out of +:class:`~pcapkit.corekit.enum.EnumRegistry` for exactly this; this module +pins that the split half of the tree that is not blocked by #913 or #904 +actually took the base -- :class:`TransportProtocol +`, +:class:`FinalisedState `, +:class:`Completion `, +:class:`ftp.Type `, +:class:`httpv1.Type `, +:class:`Criticality `, +:class:`PDUKind `, +:class:`PacketDirection `, +:class:`PacketReception `, +:class:`WireGuardKeyLabel `, +and :class:`FrameType.Flags +` plus its six +concrete per-frame subclasses. + +Two of those eleven, :class:`TransportProtocol` and :class:`Criticality`, +already defined their own ``get`` -- both as a :class:`staticmethod`, while +:meth:`~pcapkit.corekit.enum.EnumLookup.get` is a :class:`classmethod`, the +exact trap GitHub issue #908 hit and #915 fixed for +:meth:`~pcapkit.const.http.method.Method.get`. Both are now classmethods +that delegate, each keeping only the behaviour the base does not reproduce +on its own -- :class:`TransportProtocolGetTests` and +:class:`CriticalityGetTests` pin that each of those kept behaviours is +unchanged, not merely that the delegation compiles. + +The other nine are pure re-parenting -- no ``get`` or ``_missing_`` of their +own to reconcile -- so :class:`PureReparentGetTests` pins the one thing that +actually changes for them: ``get``/``get_all`` now exist and resolve, where +before this change the attribute did not exist at all. + +On the tree before this change every test below that calls ``.get()`` on one +of these nine fails with ``AttributeError: type object '' has no +attribute 'get'`` -- the base did not carry those methods to them yet -- and +every base-tuple assertion in :class:`ReparentedBasesTests` and +:class:`HttpV2FlagsHierarchyTests` fails since ``EnumLookup`` is not yet in +any of these seventeen classes' ``__bases__`` or MRO. + +""" +from __future__ import annotations + +import inspect +import unittest + +from pcapkit.corekit.enum import EnumLookup + +__all__ = [ + 'ReparentedBasesTests', 'HttpV2FlagsHierarchyTests', 'PureReparentGetTests', + 'TransportProtocolGetTests', 'CriticalityGetTests', 'NoMintingTests', +] + + +class ReparentedBasesTests(unittest.TestCase): + """Every one of the 11 unblocked classes gained :class:`EnumLookup` as a + base, mixed in *ahead of* its enum base so ``_member_type_`` still + resolves to :class:`int` or :class:`str`.""" + + def test_transport_protocol(self) -> None: + from aenum import IntEnum + + from pcapkit.const.reg.apptype.apptype import TransportProtocol + + self.assertEqual(TransportProtocol.__bases__, (EnumLookup, IntEnum)) + self.assertIn(EnumLookup, TransportProtocol.__mro__) + + def test_finalised_state(self) -> None: + import enum + + from pcapkit.corekit.infoclass import FinalisedState + + self.assertEqual(FinalisedState.__bases__, (EnumLookup, enum.IntEnum)) + self.assertIn(EnumLookup, FinalisedState.__mro__) + + def test_completion(self) -> None: + from pcapkit.foundation.reassembly.data.data import Completion + from pcapkit.utilities.compat import StrEnum + + self.assertEqual(Completion.__bases__, (EnumLookup, StrEnum)) + self.assertIn(EnumLookup, Completion.__mro__) + + def test_ftp_type(self) -> None: + from pcapkit.protocols.application.ftp import Type as FTPType + from pcapkit.utilities.compat import StrEnum + + self.assertEqual(FTPType.__bases__, (EnumLookup, StrEnum)) + self.assertIn(EnumLookup, FTPType.__mro__) + + def test_httpv1_type(self) -> None: + from pcapkit.protocols.application.httpv1 import Type as HTTPv1Type + from pcapkit.utilities.compat import StrEnum + + self.assertEqual(HTTPv1Type.__bases__, (EnumLookup, StrEnum)) + self.assertIn(EnumLookup, HTTPv1Type.__mro__) + + def test_criticality(self) -> None: + from aenum import IntEnum + + from pcapkit.protocols.application.ngap import Criticality + + self.assertEqual(Criticality.__bases__, (EnumLookup, IntEnum)) + self.assertIn(EnumLookup, Criticality.__mro__) + + def test_pdu_kind(self) -> None: + from pcapkit.protocols.application.ngap import PDUKind + from pcapkit.utilities.compat import StrEnum + + self.assertEqual(PDUKind.__bases__, (EnumLookup, StrEnum)) + self.assertIn(EnumLookup, PDUKind.__mro__) + + def test_packet_direction(self) -> None: + import enum + + from pcapkit.protocols.misc.pcapng import PacketDirection + + self.assertEqual(PacketDirection.__bases__, (EnumLookup, enum.IntEnum)) + self.assertIn(EnumLookup, PacketDirection.__mro__) + + def test_packet_reception(self) -> None: + import enum + + from pcapkit.protocols.misc.pcapng import PacketReception + + self.assertEqual(PacketReception.__bases__, (EnumLookup, enum.IntEnum)) + self.assertIn(EnumLookup, PacketReception.__mro__) + + def test_wireguard_key_label(self) -> None: + from pcapkit.protocols.misc.pcapng import WireGuardKeyLabel + from pcapkit.utilities.compat import StrEnum + + self.assertEqual(WireGuardKeyLabel.__bases__, (EnumLookup, StrEnum)) + self.assertIn(EnumLookup, WireGuardKeyLabel.__mro__) + + +class HttpV2FlagsHierarchyTests(unittest.TestCase): + """``FrameType.Flags`` plus its six concrete per-frame subclasses -- one + hierarchy, re-parented at the root only. + + Each concrete subclass declares ``class Flags(FrameType.Flags):`` with no + base list of its own (see :mod:`pcapkit.protocols.schema.application. + httpv2`), so re-parenting the root should carry all six rather than + needing each done individually. Verified here at runtime rather than + assumed from Python's MRO rules, per the task's own instruction to check + rather than assume. + """ + + def test_frame_type_flags_itself(self) -> None: + import enum + + from pcapkit.protocols.schema.application.httpv2 import FrameType + + self.assertEqual(FrameType.Flags.__bases__, (EnumLookup, enum.IntFlag)) + self.assertIn(EnumLookup, FrameType.Flags.__mro__) + + def test_all_six_concrete_subclasses_carry_it_transitively(self) -> None: + from pcapkit.protocols.schema.application.httpv2 import (ContinuationFrame, DataFrame, + FrameType, HeadersFrame, + PingFrame, PushPromiseFrame, + SettingsFrame) + + subclasses = { + 'DataFrame.Flags': DataFrame.Flags, + 'HeadersFrame.Flags': HeadersFrame.Flags, + 'SettingsFrame.Flags': SettingsFrame.Flags, + 'PushPromiseFrame.Flags': PushPromiseFrame.Flags, + 'PingFrame.Flags': PingFrame.Flags, + 'ContinuationFrame.Flags': ContinuationFrame.Flags, + } + for name, cls in subclasses.items(): + with self.subTest(cls=name): + # No base list of its own -- inherits solely from + # ``FrameType.Flags``, which is where ``EnumLookup`` was added. + self.assertEqual(cls.__bases__, (FrameType.Flags,)) + self.assertIn(EnumLookup, cls.__mro__) + # And the contract actually works, not merely appears in the MRO. + member = next(iter(cls)) + self.assertIs(cls.get(member.name), member) + + +class PureReparentGetTests(unittest.TestCase): + """The nine classes with no ``get``/``_missing_`` of their own: before + this change, ``.get()`` did not exist on any of them at all.""" + + def test_finalised_state_get(self) -> None: + from pcapkit.corekit.infoclass import FinalisedState + + self.assertIs(FinalisedState.get('FINAL'), FinalisedState.FINAL) + self.assertIs(FinalisedState.get(FinalisedState.NONE.value), FinalisedState.NONE) + self.assertEqual(len(FinalisedState.get_all('BASE')), 1) + + def test_completion_get(self) -> None: + from pcapkit.foundation.reassembly.data.data import Completion + + self.assertIs(Completion.get('complete'), Completion.COMPLETE) + self.assertIs(Completion.get('timeout'), Completion.TIMEOUT) + + def test_ftp_type_get(self) -> None: + from pcapkit.protocols.application.ftp import Type as FTPType + + self.assertIs(FTPType.get('request'), FTPType.REQUEST) + self.assertIs(FTPType.get('response'), FTPType.RESPONSE) + + def test_httpv1_type_get(self) -> None: + from pcapkit.protocols.application.httpv1 import Type as HTTPv1Type + + self.assertIs(HTTPv1Type.get('request'), HTTPv1Type.REQUEST) + self.assertIs(HTTPv1Type.get('response'), HTTPv1Type.RESPONSE) + + def test_pdu_kind_get(self) -> None: + from pcapkit.protocols.application.ngap import PDUKind + + self.assertIs(PDUKind.get('initiatingMessage'), PDUKind.INITIATING_MESSAGE) + + def test_packet_direction_get(self) -> None: + from pcapkit.protocols.misc.pcapng import PacketDirection + + self.assertIs(PacketDirection.get('INBOUND'), PacketDirection.INBOUND) + self.assertIs(PacketDirection.get(0b10), PacketDirection.OUTBOUND) + + def test_packet_reception_get(self) -> None: + from pcapkit.protocols.misc.pcapng import PacketReception + + self.assertIs(PacketReception.get('PROMISCUOUS'), PacketReception.PROMISCUOUS) + + def test_wireguard_key_label_get(self) -> None: + from pcapkit.protocols.misc.pcapng import WireGuardKeyLabel + + self.assertIs(WireGuardKeyLabel.get('PRESHARED_KEY'), WireGuardKeyLabel.PRESHARED_KEY) + + def test_a_pure_reparent_still_refuses_an_unknown_name(self) -> None: + """The base's own contract -- raise, never mint -- reaches these nine + for free, exactly as it does the 124 registries.""" + from pcapkit.corekit.infoclass import FinalisedState + + with self.assertRaises(KeyError): + FinalisedState.get('NOT_A_REAL_STATE') + # No member was minted answering the failed lookup. + self.assertNotIn('NOT_A_REAL_STATE', FinalisedState.__members__) + + +class TransportProtocolGetTests(unittest.TestCase): + """:class:`~pcapkit.const.reg.apptype.apptype.TransportProtocol` kept its + own ``get``, now delegating. Every behaviour the hand-rolled version had + is pinned here, not merely that the delegated version runs.""" + + def test_get_is_now_a_classmethod(self) -> None: + from pcapkit.const.reg.apptype.apptype import TransportProtocol + + self.assertIsInstance(inspect.getattr_static(TransportProtocol, 'get'), classmethod) + + def test_case_folding_is_preserved(self) -> None: + """The base's own ``str`` branch is case-sensitive; this override + still lower-cases before delegating, so an upper-cased spelling + must still resolve -- unlike a class that relies on the base alone + (see :class:`~tests.corekit.test_enum_lookup_reparent_877_unit. + PureReparentGetTests`, all of whose members are matched exactly).""" + from pcapkit.const.reg.apptype.apptype import TransportProtocol + + self.assertIs(TransportProtocol.get('TCP'), TransportProtocol.tcp) + self.assertIs(TransportProtocol.get('tcp'), TransportProtocol.tcp) + self.assertIs(TransportProtocol.get('Udp'), TransportProtocol.udp) + + def test_int_path_unchanged(self) -> None: + from pcapkit.const.reg.apptype.apptype import TransportProtocol + + self.assertIs(TransportProtocol.get(1), TransportProtocol.tcp) + + def test_unrecognised_name_still_raises_value_error_naming_the_key(self) -> None: + """Maintainer ruling on PR #836: refuse, never mint. The base's own + miss on a ``str`` key raises a bare ``KeyError``; this override still + converts it to the ``ValueError`` every caller and test here already + depends on.""" + from pcapkit.const.reg.apptype.apptype import TransportProtocol + + before = len(TransportProtocol.__members__) + with self.assertRaises(ValueError) as caught: + TransportProtocol.get('quic') + self.assertNotIsInstance(caught.exception, KeyError) + self.assertIn('quic', str(caught.exception)) + self.assertIn('is not a valid', str(caught.exception)) + self.assertEqual(len(TransportProtocol.__members__), before) + + def test_default_is_a_new_capability_not_exercised_before(self) -> None: + """``default`` did not exist on this override before this change -- + added purely because dropping an optional parameter the base + declares is a genuine ``mypy`` ``[override]`` violation. Forwarded + verbatim, so it behaves exactly as + :meth:`~pcapkit.corekit.enum.EnumLookup.get`'s own ``default``: a + fallback to an *already-registered* value, never a new mint.""" + from pcapkit.const.reg.apptype.apptype import TransportProtocol + + self.assertIs(TransportProtocol.get('bogus', default=TransportProtocol.udp), + TransportProtocol.udp) + # Omitted, as every call site in this tree omits it: raises exactly + # as before this change. + with self.assertRaises(ValueError): + TransportProtocol.get('bogus') + + +class CriticalityGetTests(unittest.TestCase): + """:class:`~pcapkit.protocols.application.ngap.Criticality` kept its own + ``get`` too, now delegating -- case-**sensitive**, unlike + ``TransportProtocol``, which is the one designed divergence between the + two overrides this issue re-parented.""" + + def test_get_is_now_a_classmethod(self) -> None: + from pcapkit.protocols.application.ngap import Criticality + + self.assertIsInstance(inspect.getattr_static(Criticality, 'get'), classmethod) + + def test_name_lookup(self) -> None: + from pcapkit.protocols.application.ngap import Criticality + + self.assertIs(Criticality.get('reject'), Criticality.reject) + self.assertIs(Criticality.get('ignore'), Criticality.ignore) + self.assertIs(Criticality.get('notify'), Criticality.notify) + + def test_value_lookup(self) -> None: + from pcapkit.protocols.application.ngap import Criticality + + self.assertIs(Criticality.get(2), Criticality.notify) + + def test_a_criticality_instance_resolves_to_itself(self) -> None: + """The removed ``isinstance(key, Criticality): return key`` branch is + absorbed by the base's own ``cls(key)`` call -- a ``Criticality`` is + also an :class:`int` (it is an :class:`~aenum.IntEnum`), so the + non-``str`` path already hands back the identical canonical member.""" + from pcapkit.protocols.application.ngap import Criticality + + self.assertIs(Criticality.get(Criticality.reject), Criticality.reject) + + def test_case_sensitivity_is_unlike_transport_protocol(self) -> None: + """Deliberately the opposite of ``TransportProtocol.get`` -- this + override never lower-cases, so an upper-cased spelling must be + refused rather than folded.""" + from pcapkit.protocols.application.ngap import Criticality + + with self.assertRaises(ValueError) as caught: + Criticality.get('REJECT') + self.assertIn('REJECT', str(caught.exception)) + + def test_unresolved_name_raises_value_error_not_key_error(self) -> None: + """The base's own miss on a ``str`` key raises a bare ``KeyError``; + this override still converts it to the ``ValueError`` its own + ``_missing_`` already uses for an unresolved value, so the two ways + of getting this wrong report identically -- exactly as before this + change.""" + from pcapkit.protocols.application.ngap import Criticality + + with self.assertRaises(ValueError) as caught: + Criticality.get('NoSuchMember') + self.assertNotIsInstance(caught.exception, KeyError) + self.assertIn('NoSuchMember', str(caught.exception)) + + def test_unresolved_value_still_raises_via_missing(self) -> None: + from pcapkit.protocols.application.ngap import Criticality + + with self.assertRaises(ValueError): + Criticality.get(3) + + def test_default_is_a_new_capability_not_exercised_before(self) -> None: + from pcapkit.protocols.application.ngap import Criticality + + self.assertIs(Criticality.get('NoSuchMember', default=Criticality.ignore), + Criticality.ignore) + with self.assertRaises(ValueError): + Criticality.get('NoSuchMember') + + +class NoMintingTests(unittest.TestCase): + """None of the eleven re-parented classes mint on a lookup -- each is a + closed set on the bare lookup tier, not the mutating + :class:`~pcapkit.corekit.enum.EnumRegistry` one. Measured per class in a + throwaway subprocess, so a lookup made by an *earlier* assertion in this + same test run can never be mistaken for growth caused by the class under + test -- the house rule for probing a minting-capable enum, applied here + even though these are the closed side of that line. + """ + + @staticmethod + def _sizes_before_and_after(import_stmt: str, cls_expr: str, lookup_expr: str) -> 'tuple[int, int, int, int]': + import subprocess + import sys + + code = ( + f'{import_stmt}\n' + f'cls = {cls_expr}\n' + f'before_names = len(cls._member_map_)\n' + f'before_values = len(cls._value2member_map_)\n' + f'{lookup_expr}\n' + f'after_names = len(cls._member_map_)\n' + f'after_values = len(cls._value2member_map_)\n' + f'print(before_names, before_values, after_names, after_values)\n' + ) + result = subprocess.run( + [sys.executable, '-c', code], + capture_output=True, text=True, check=True, + ) + before_names, before_values, after_names, after_values = map(int, result.stdout.split()) + return before_names, before_values, after_names, after_values + + def test_transport_protocol_does_not_grow(self) -> None: + before_names, before_values, after_names, after_values = self._sizes_before_and_after( + 'from pcapkit.const.reg.apptype.apptype import TransportProtocol', + 'TransportProtocol', + "cls.get('TCP')", + ) + self.assertEqual(before_names, after_names) + self.assertEqual(before_values, after_values) + + def test_criticality_does_not_grow(self) -> None: + before_names, before_values, after_names, after_values = self._sizes_before_and_after( + 'from pcapkit.protocols.application.ngap import Criticality', + 'Criticality', + "cls.get('reject')", + ) + self.assertEqual(before_names, after_names) + self.assertEqual(before_values, after_values) + + def test_finalised_state_does_not_grow(self) -> None: + before_names, before_values, after_names, after_values = self._sizes_before_and_after( + 'from pcapkit.corekit.infoclass import FinalisedState', + 'FinalisedState', + "cls.get('FINAL')", + ) + self.assertEqual(before_names, after_names) + self.assertEqual(before_values, after_values) + + +if __name__ == '__main__': + unittest.main()