diff --git a/CHANGELOG.md b/CHANGELOG.md index 946afc9a7..f03570a46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The largest release since 1.0, and the first recorded here as it happened rather - **Changed** -- conflicting TCP overlaps resolve first-write-wins, per [RFC 9293](https://datatracker.ietf.org/doc/html/rfc9293) section 3.10, where they had silently resolved last-write-wins (#443, #478). A deliberate behaviour break, and a narrow one: a conforming retransmission carries identical bytes, so nothing changes for it. IP fragment reassembly keeps last-write-wins, because [RFC 791](https://datatracker.ietf.org/doc/html/rfc791) specifies the opposite resolution, and records the disagreement instead (#482). - **Changed** -- `Probe`, `CipherSuite` and `IntegritySuite` are `Info` subclasses rather than `typing.NamedTuple`, and no `NamedTuple` remains in the package. They are Mappings now, so `len()` and iteration yield field names rather than values. - **Changed** -- renames with no compatibility alias left behind: `HoleDiscriptor` is spelled `HoleDescriptor` and its package alias `TCP_HoleDiscriptor` is `TCP_HoleDescriptor` (#350); PCAP-NG `Option` subclasses spell the namespace class keyword `ns=` instead of `namespace=` (#439); and `examples/sample` and `examples/samples` -- one letter apart, holding different things -- are now `examples/captures` and `examples/generators`. +- **Changed** -- subclass registration is **opt-in** for `Engine`, `Reassembly`, `TraceFlow` and `dumpkit`'s `Dumper` (#514). Each registers if and only if its registry keyword is given -- `name=` for `Engine`, `protocol=` for `Reassembly` and `TraceFlow`, `fmt=` for `Dumper`. Previously an absent keyword fell back to the class' own name, so *every* subclass of the public class was registered, and declining meant subclassing the parallel `*Base` class under an alias -- which is what every built-in does, and why the public classes had **0** subclasses between them against the `*Base` classes' 9, 5, 2 and 3. **This breaks out-of-tree code that subclasses one of the four and relies on the derived key**; pass the keyword, or call the matching `register_*` function. Nothing the library ships is affected, and the `*Base` classes remain importable. Two things that were silent are now loud: an unrecognised class keyword raises `UnsupportedCall` instead of being swallowed by `**kwargs` -- which used to register the class under its own name, so passing `name=` to a `Reassembly` subclass silently ignored the key it was given, `protocol=` being the real one -- and `Dumper`'s `ext=` without `fmt=` likewise. A class attribute is not an opt-in: `__engine_name__` and `__protocol_name__` still set the name a class reports, registered or not. Each metaclass also gained a class-level `registry` property mirroring `EnumSchema.registry`. As a side effect a `Dumper` subclass no longer touches the filesystem while its `class` statement runs: inferring `fmt` from the `kind` property meant instantiating the class against a `NamedTemporaryFile` mid-definition. **One constraint specific to Python 3.10**, where this matters because the keyword is now the only class-definition path: `Engine`'s keyword is literally `name`, and `mcls`, `name`, `bases` and `namespace` collide with `abc.ABCMeta.__new__`'s own parameters, which are positional-or-keyword before 3.11 and positional-only from 3.11. So on 3.10 a class statement passing `name=` raises `TypeError` from the metaclass before the hook is reached, and an engine has to be registered with `Extractor.register_engine` instead -- which works on every version. `protocol=` and `fmt=` do not collide and are unaffected. Measured on 3.10.21, 3.11.15 and 3.14.7. - **Changed** -- extraction is around 46% faster on a 1,117-frame HTTP capture, with byte-identical output (#420). A reassembled datagram's payload is now analysed on first read rather than eagerly, which cuts IP reassembly's own cost by 90.7% and TCP's by 23.7% -- IP reassembly submits a datagram for every frame, fragmented or not (#424). Flow tracing over the same capture went from 1416.6 ms to 744.0 ms, because the flow dumper had been handing each record to a `Frame` constructor that re-dissected the whole protocol stack to return bytes it had just been given; options are no longer parsed twice either (#427). All output compared byte-for-byte across the sample captures in each case. - **Fixed** -- next-layer, option, chunk, block and parameter dispatch all read `defaultdict` registries, so a lookup miss inserted the key into class-level state shared by every later instance, after which a legitimate `register_*` call warned that the code was already registered. Every read now goes through a lookup that does not grow the table, and `IPv4.__option__` and `HIP.__parameter__` became inspectable class attributes rather than names assembled at call time (#426, #428, #429, #434). One break comes with it: a tuple-registered handler pair written to the documented `OptionParser`/`OptionConstructor` signature now works where it could previously never be called at all, and a pair written with an explicit leading `self` -- the only shape that used to work -- now does not. - **Fixed** -- on Python 3.10 and older, no `Schema` subclass got its own `_abc_impl`: all of them fell through to `collections.abc.Mapping`'s, so a single `isinstance` or `issubclass` answer poisoned every later question about that class for the rest of the process. A terminating PCAP-NG `EndRecord` tested `True` as an `IPv4Record` (#439). diff --git a/docs/source/changelog/1.5.0.rst b/docs/source/changelog/1.5.0.rst index 9851e1d86..f019d022b 100644 --- a/docs/source/changelog/1.5.0.rst +++ b/docs/source/changelog/1.5.0.rst @@ -135,6 +135,38 @@ pull requests between #326 and #509. (#439); and ``examples/sample`` and ``examples/samples`` -- one letter apart, holding different things -- are now ``examples/captures`` and ``examples/generators``. +* **Changed** -- subclass registration is **opt-in** for ``Engine``, + ``Reassembly``, ``TraceFlow`` and ``dumpkit``'s ``Dumper`` (#514). Each + registers if and only if its registry keyword is given -- ``name=`` for + ``Engine``, ``protocol=`` for ``Reassembly`` and ``TraceFlow``, ``fmt=`` for + ``Dumper``. Previously an absent keyword fell back to the class' own name, so + *every* subclass of the public class was registered, and declining meant + subclassing the parallel ``*Base`` class under an alias -- which is what every + built-in does, and why the public classes had **0** subclasses between them + against the ``*Base`` classes' 9, 5, 2 and 3. **This breaks out-of-tree code + that subclasses one of the four and relies on the derived key**; pass the + keyword, or call the matching ``register_*`` function. Nothing the library + ships is affected, and the ``*Base`` classes remain importable. Two things + that were silent are now loud: an unrecognised class keyword raises + ``UnsupportedCall`` instead of being swallowed by ``**kwargs`` -- which used + to register the class under its own name, so passing ``name=`` to a + ``Reassembly`` subclass silently ignored the key it was given, ``protocol=`` + being the real one -- and ``Dumper``'s ``ext=`` without ``fmt=`` likewise. A class + attribute is not an opt-in: ``__engine_name__`` and ``__protocol_name__`` still + set the name a class reports, registered or not. Each metaclass also gained a + class-level ``registry`` property mirroring ``EnumSchema.registry``. As a side + effect a ``Dumper`` subclass no longer touches the filesystem while its + ``class`` statement runs: inferring ``fmt`` from the ``kind`` property meant + instantiating the class against a ``NamedTemporaryFile`` mid-definition. + **One constraint specific to Python 3.10**, where this matters because the + keyword is now the only class-definition path: ``Engine``'s keyword is literally + ``name``, and ``mcls``, ``name``, ``bases`` and ``namespace`` collide with + ``abc.ABCMeta.__new__``'s own parameters, which are positional-or-keyword + before 3.11 and positional-only from 3.11. So on 3.10 a class statement passing + ``name=`` raises ``TypeError`` from the metaclass before the hook is reached, + and an engine has to be registered with ``Extractor.register_engine`` instead + -- which works on every version. ``protocol=`` and ``fmt=`` do not collide and + are unaffected. Measured on 3.10.21, 3.11.15 and 3.14.7. * **Changed** -- extraction is around 46% faster on a 1,117-frame HTTP capture, with byte-identical output (#420). A reassembled datagram's payload is now analysed on first read rather than eagerly, which cuts IP reassembly's own diff --git a/docs/source/ext.rst b/docs/source/ext.rst index 1eee18895..c8afbe06b 100644 --- a/docs/source/ext.rst +++ b/docs/source/ext.rst @@ -384,7 +384,13 @@ The following code snippet shows how to create a new engine class: from scapy.packet import Packet - class MyScapy(Engine['Packet']): + # NOTE: The ``name`` keyword is what registers the engine with the Extractor, + # and it is the key ``Extractor(engine=...)`` will look it up under. It is + # required: registration is opt-in, so omitting it defines a perfectly usable + # class that is simply not selectable by name. Note that __engine_name__ is + # *not* an opt-in -- it sets the name the engine reports about itself, which + # it does whether or not the engine is registered. + class MyScapy(Engine['Packet'], name='scapy'): __engine_name__ = 'Scapy' # friendly name of the engine __engine_module__ = 'scapy' # module name that the engine is based on @@ -737,7 +743,15 @@ The following code snippet shows how to create a new reassembly class: # a subclass of the base class, i.e., Reassembly, and implement the core # methods, i.e., reassembly and submit, for reassembling the fragmented # packets and submitting the reassembled datagram, respectively. - class MyReassembly(Reassembly[Packet, Datagram, BufferID, Buffer]): + # + # The ``protocol`` keyword is what registers the class, and it is the key it + # will be looked up under. It is required: registration is opt-in, so omitting + # it defines a perfectly usable class that is simply not selectable by name. + # Note that __protocol_name__ is *not* an opt-in -- it sets the name the class + # reports about itself, registered or not. Note also that the keyword is + # spelled ``protocol`` here and ``name`` on Engine above. + class MyReassembly(Reassembly[Packet, Datagram, BufferID, Buffer], + protocol='ipv4'): __protocol_name__ = 'IPv4' # name of the protocol __protocol_type__ = IPv4 # type of the protocol diff --git a/docs/source/pcapkit/foundation/engines/engine.rst b/docs/source/pcapkit/foundation/engines/engine.rst index de7ef7107..ed551ca9a 100644 --- a/docs/source/pcapkit/foundation/engines/engine.rst +++ b/docs/source/pcapkit/foundation/engines/engine.rst @@ -36,6 +36,19 @@ all engine support functionality. This property is also available as a class variable. Its value can be set by :attr:`__engine_module__` class attribute. + .. property:: registry + :type: dict[str, ModuleDescriptor[EngineBase] | Type[EngineBase]] + + Mapping of engine names to engine classes. + + .. note:: + + This property is only available as a class variable, since it is + defined on :class:`EngineMeta`. It reads + :attr:`~pcapkit.foundation.extraction.Extractor.__engine__`, the + single table every engine registration lands in, so it is not a + per-class mapping. + .. autoproperty:: extractor .. automethod:: unsupported_reason diff --git a/docs/source/pcapkit/foundation/reassembly/reassembly.rst b/docs/source/pcapkit/foundation/reassembly/reassembly.rst index b8e3d9c5a..23c52179f 100644 --- a/docs/source/pcapkit/foundation/reassembly/reassembly.rst +++ b/docs/source/pcapkit/foundation/reassembly/reassembly.rst @@ -39,6 +39,19 @@ implements datagram reassembly of IP and TCP packets. This property is also available as a class variable. Its value can be set by :attr:`__protocol_type__` class attribute. + .. property:: registry + :type: dict[str, ModuleDescriptor[ReassemblyBase] | Type[ReassemblyBase]] + + Mapping of protocol names to reassembly classes. + + .. note:: + + This property is only available as a class variable, since it is + defined on :class:`ReassemblyMeta`. It reads + :attr:`~pcapkit.foundation.extraction.Extractor.__reassembly__`, the + single table every reassembly registration lands in, so it is not a + per-class mapping. + .. autoproperty:: count .. autoproperty:: datagram .. autoproperty:: timeout diff --git a/docs/source/pcapkit/foundation/traceflow/traceflow.rst b/docs/source/pcapkit/foundation/traceflow/traceflow.rst index 47aca4946..fd1b70944 100644 --- a/docs/source/pcapkit/foundation/traceflow/traceflow.rst +++ b/docs/source/pcapkit/foundation/traceflow/traceflow.rst @@ -36,6 +36,20 @@ which is an abstract base class for all flow tracing classes. This property is also available as a class variable. Its value can be set by :attr:`__protocol_type__` class attribute. + .. property:: registry + :type: dict[str, ModuleDescriptor[TraceFlowBase] | Type[TraceFlowBase]] + + Mapping of protocol names to flow tracing classes. + + .. note:: + + This property is only available as a class variable, since it is + defined on :class:`TraceFlowMeta`. It reads + :attr:`~pcapkit.foundation.extraction.Extractor.__traceflow__`, the + single table every flow tracing registration lands in, so it is not a + per-class mapping. It is *not* :attr:`__output__`, which is the + separate output-dumper table this class also owns. + .. autoproperty:: index .. automethod:: register_dumper diff --git a/pcapkit/dumpkit/common.py b/pcapkit/dumpkit/common.py index c86c62ba9..af8183b31 100644 --- a/pcapkit/dumpkit/common.py +++ b/pcapkit/dumpkit/common.py @@ -15,7 +15,6 @@ import decimal import enum import ipaddress -import tempfile from typing import TYPE_CHECKING import aenum @@ -24,6 +23,7 @@ from pcapkit.corekit.infoclass import Info from pcapkit.corekit.multidict import MultiDict, OrderedMultiDict from pcapkit.protocols.schema.schema import Schema +from pcapkit.utilities.exceptions import UnsupportedCall from pcapkit.utilities.logging import get_logger __all__ = ['make_dumper'] @@ -56,11 +56,38 @@ class Dumper(DumperBase): This class is a customised :class:`~dictdumper.dumper.Dumper` for the :mod:`pcapkit.dumpkit` implementation, which is generally customised - for automatic registration to the + for opt-in registration to the :class:`~pcapkit.foundation.extraction.Extractor` and :class:`~pcapkit.foundation.traceflow.traceflow.TraceFlow` output dumper registries. + Example: + + Registration is opt-in. Pass keyword argument ``fmt`` at class + definition to register the dumper under that output format: + + .. code-block:: python + + class MyDumper(Dumper, fmt='my_format', ext='.mine'): + ... + + Omit it and the subclass is *not* registered: + + .. code-block:: python + + class MyMixin(Dumper): # not registered + ... + + Such a class can still be registered later, on demand. Note this hook + writes *both* output registries, so the equivalent manual call is the + module-level one that does the same, not either class' own method: + + .. code-block:: python + + from pcapkit.foundation.registry.foundation import register_dumper + + register_dumper('my_mixin', MyMixin, '.mine') + """ def __init_subclass__(cls, /, fmt: 'Optional[str]' = None, @@ -68,20 +95,36 @@ def __init_subclass__(cls, /, fmt: 'Optional[str]' = None, """Initialise subclass. This method is used to register the subclass to the - :class:`~pcapkit.foundation.extraction.Extraction` and + :class:`~pcapkit.foundation.extraction.Extractor` and :class:`~pcapkit.foundation.traceflow.traceflow.TraceFlow` output dumper registries. Args: - fmt: Output format to register. - ext: Output file extension. + fmt: Output format to register the subclass under, lowercased. + :data:`None` (the default) skips registration entirely. + ext: Output file extension; :data:`None` infers it from ``fmt``. + Only meaningful alongside ``fmt``. *args: Arbitrary positional arguments. **kwargs: Arbitrary keyword arguments. - If the ``fmt`` is not provided, we will try to get it from the - :attr:`~dictdumper.dumper.Dumper.kind` property of the subclass. - And if the ``ext`` is not provided, we will infer it from the - ``fmt``. + Raises: + UnsupportedCall: If ``ext`` is given without ``fmt``, or if any + unrecognised class keyword is given. + + Registration is **opt-in**: the subclass is registered if and only if + ``fmt`` is given. This is what lets a subclass decline registration + rather than having to inherit :class:`DumperBase` to avoid it, and it + matches :meth:`EnumSchema.__init_subclass__ + `, which + has guarded on its own ``code`` keyword all along. + + Note: + The previous behaviour inferred ``fmt`` from the subclass' + :attr:`~dictdumper.dumper.Dumper.kind` property, which it could + only read off an *instance* -- so it constructed one against a + :func:`tempfile.NamedTemporaryFile` while the ``class`` statement + was still executing. Guarding on ``fmt`` removes that: a class + definition no longer touches the filesystem. See Also: - :func:`pcapkit.foundation.registry.foundation.register_dumper` @@ -91,18 +134,32 @@ def __init_subclass__(cls, /, fmt: 'Optional[str]' = None, - :meth:`pcapkit.foundation.traceflow.traceflow.TraceFlow.register_dumper` """ + # NOTE: as in the four sibling hooks, an unrecognised class keyword would + # otherwise land in ``**kwargs`` and be dropped by the bare + # ``super().__init_subclass__()`` below, silently skipping registration. + if args or kwargs: + unexpected = ', '.join([*map(repr, args), *sorted(kwargs)]) + raise UnsupportedCall(f'{cls.__name__}: unexpected class keyword(s): {unexpected}') + + # NOTE: ``ext`` alone cannot register anything -- there is no format to + # register it against -- so it would silently do nothing. Say so instead. if fmt is None: - with tempfile.NamedTemporaryFile() as temp: - fmt = cls(temp.name).kind - fmt = fmt.lower() + if ext is not None: + raise UnsupportedCall(f'{cls.__name__}: ext={ext!r} given without fmt') + return super().__init_subclass__() + fmt = fmt.lower() if ext is None: ext = f'.{fmt}' - from pcapkit.foundation.extraction import Extractor + from pcapkit.foundation.extraction import \ + Extractor # pylint: disable=import-outside-toplevel + Extractor.register_dumper(fmt, cls, ext) - from pcapkit.foundation.traceflow.traceflow import TraceFlow + from pcapkit.foundation.traceflow.traceflow import \ + TraceFlow # pylint: disable=import-outside-toplevel + TraceFlow.register_dumper(fmt, cls, ext) return super().__init_subclass__() diff --git a/pcapkit/foundation/engines/engine.py b/pcapkit/foundation/engines/engine.py index 2ae64c8bb..5f38a036e 100644 --- a/pcapkit/foundation/engines/engine.py +++ b/pcapkit/foundation/engines/engine.py @@ -9,13 +9,16 @@ """ import abc -from typing import TYPE_CHECKING, Generic, TypeVar, cast +from typing import TYPE_CHECKING, Generic, TypeVar + +from pcapkit.utilities.exceptions import UnsupportedCall __all__ = ['Engine'] if TYPE_CHECKING: - from typing import Any, Optional + from typing import Any, Optional, Type + from pcapkit.corekit.module import ModuleDescriptor from pcapkit.foundation.extraction import Extractor _T = TypeVar('_T') @@ -49,6 +52,31 @@ def module(cls) -> 'str': return cls.__engine_module__ return cls.__module__ + @property + def registry(cls) -> 'dict[str, ModuleDescriptor[Engine] | Type[Engine]]': + """Mapping of engine names to engine classes. + + Note: + Unlike :attr:`EnumSchema.registry + `, this is not + a per-class mapping: every engine registration lands in the single + :attr:`Extractor.__engine__ + ` table, so + reading it through any subclass returns that same object. The + property exists so ``MyEngine.registry`` is spelled the same way + here as it is for schemas. + + Note also that :class:`EnumSchema` carries *two* ``registry`` + properties, one on its metaclass and one on the class body, so it + answers on an instance as well. This one is on the metaclass only, + so it is available as a class attribute and **not** on an instance. + + """ + from pcapkit.foundation.extraction import \ + Extractor # pylint: disable=import-outside-toplevel + + return Extractor.__engine__ + class EngineBase(Generic[_T], metaclass=EngineMeta): """Base class for engine support. @@ -187,14 +215,28 @@ class Engine(EngineBase[_T], Generic[_T]): Example: - Use keyword argument ``name`` to specify the engine name at - class definition: + Registration is opt-in. Pass keyword argument ``name`` at class + definition to register the engine under that name: .. code-block:: python class MyEngine(Engine, name='my_engine'): ... + Omit it and the subclass is *not* registered, which is how a class + that is not meant to be selectable by name declines: + + .. code-block:: python + + class MyMixin(Engine): # not registered + ... + + Such a class can still be registered later, on demand: + + .. code-block:: python + + Extractor.register_engine('my_mixin', MyMixin) + Args: extractor: :class:`~pcapkit.foundation.extraction.Extractor` instance. @@ -207,19 +249,75 @@ def __init_subclass__(cls, /, name: 'Optional[str]' = None, *args: 'Any', **kwar :class:`~pcapkit.foundation.extraction.Extractor` class. Args: - name: Engine name, default to class name. + name: Engine name to register the subclass under, lowercased. + :data:`None` (the default) skips registration entirely. *args: Arbitrary positional arguments. **kwargs: Arbitrary keyword arguments. + Raises: + UnsupportedCall: If any unrecognised class keyword is given. + + Registration is **opt-in**: the subclass is registered if and only if + ``name`` is given. This is what lets a subclass decline registration + rather than having to inherit :class:`EngineBase` to avoid it, and it + matches :meth:`EnumSchema.__init_subclass__ + `, which + has guarded on its own ``code`` keyword all along. + + Note: + :attr:`__engine_name__` is *not* an opt-in. It supplies the + :attr:`name ` + the engine reports, which it does whether or not the engine is + registered; only the keyword decides registration. + + Warning: + **On Python 3.10 the ``name`` keyword cannot be passed at all.** + :meth:`abc.ABCMeta.__new__` takes ``mcls``, ``name``, ``bases`` and + ``namespace`` as positional-*or-keyword* parameters before 3.11, so a + class keyword named ``name`` collides with one of them and the class + statement raises :exc:`TypeError` -- ``ABCMeta.__new__() got multiple + values for argument 'name'`` -- from the metaclass, before this method + is reached. From 3.11 those parameters are positional-only and the + keyword arrives here normally. Measured on 3.10.21, 3.11.15 and + 3.14.7. + + The consequence on 3.10 is that an engine cannot be registered at + class definition; register it explicitly instead, which works on every + version:: + + class MyEngine(Engine): # no keyword, so not registered + ... + + Extractor.register_engine('my_engine', MyEngine) + + The sibling hooks are unaffected, their keywords being ``protocol`` + and ``fmt``. + See Also: For more details, please refer to :meth:`pcapkit.foundation.extraction.Extractor.register_engine`. """ - if name is None: - name = cast('str', cls.name) - - from pcapkit.foundation.extraction import Extractor - Extractor.register_engine(name.lower(), cls) + # NOTE: an unrecognised class keyword lands in ``**kwargs`` and is then + # dropped by the bare ``super().__init_subclass__()`` below, since + # ``object.__init_subclass__`` takes none. Silently swallowing it is how + # ``class MyEngine(Engine, nmae='x')`` used to register under its class + # name instead -- no exception, no warning. Now that a missing keyword + # means "do not register", the same typo would silently skip + # registration altogether, which is quieter still. So reject it. + # + # ``args`` is checked alongside ``kwargs`` for completeness rather than + # because a ``class`` statement can fill it -- class creation passes + # keywords only. It is reachable through a direct + # ``__init_subclass__(...)`` call, which the declared signature permits. + if args or kwargs: + unexpected = ', '.join([*map(repr, args), *sorted(kwargs)]) + raise UnsupportedCall(f'{cls.__name__}: unexpected class keyword(s): {unexpected}') + + if name is not None: + from pcapkit.foundation.extraction import \ + Extractor # pylint: disable=import-outside-toplevel + + Extractor.register_engine(name.lower(), cls) return super().__init_subclass__() diff --git a/pcapkit/foundation/reassembly/reassembly.py b/pcapkit/foundation/reassembly/reassembly.py index ddb3df7c3..9ff532501 100644 --- a/pcapkit/foundation/reassembly/reassembly.py +++ b/pcapkit/foundation/reassembly/reassembly.py @@ -40,6 +40,7 @@ from typing_extensions import Self from pcapkit.corekit.infoclass import Info + from pcapkit.corekit.module import ModuleDescriptor from pcapkit.protocols.protocol import ProtocolBase as Protocol CallbackFn = Callable[[list[_DT]], None] @@ -79,6 +80,31 @@ def protocol(cls) -> 'Type[Protocol]': return cls.__protocol_type__ return protocol_registry.get(cls.name.upper(), Raw) + @property + def registry(cls) -> 'dict[str, ModuleDescriptor[Reassembly] | Type[Reassembly]]': + """Mapping of protocol names to reassembly classes. + + Note: + Unlike :attr:`EnumSchema.registry + `, this is not + a per-class mapping: every reassembly registration lands in the + single :attr:`Extractor.__reassembly__ + ` table, so + reading it through any subclass returns that same object. The + property exists so ``MyReassembly.registry`` is spelled the same way + here as it is for schemas. + + Note also that :class:`EnumSchema` carries *two* ``registry`` + properties, one on its metaclass and one on the class body, so it + answers on an instance as well. This one is on the metaclass only, + so it is available as a class attribute and **not** on an instance. + + """ + from pcapkit.foundation.extraction import \ + Extractor # pylint: disable=import-outside-toplevel + + return Extractor.__reassembly__ + class ReassemblyBase(Generic[_PT, _DT, _IT, _BT], metaclass=ReassemblyMeta): """Base class for reassembly procedure. @@ -95,7 +121,7 @@ class ReassemblyBase(Generic[_PT, _DT, _IT, _BT], metaclass=ReassemblyMeta): Note: This class is for internal use only. For customisation, please use - :class:`TraceFlow` instead. + :class:`Reassembly` instead. """ if TYPE_CHECKING: @@ -473,18 +499,32 @@ def __init_subclass__(cls) -> 'None': class Reassembly(ReassemblyBase[_PT, _DT, _IT, _BT], Generic[_PT, _DT, _IT, _BT]): - """Base flow tracing class. + """Base reassembly class. Example: - Use keyword argument ``protocol`` to specify the protocol - name at class definition: + Registration is opt-in. Pass keyword argument ``protocol`` at class + definition to register the reassembly under that protocol name: .. code-block:: python class MyProtocol(Reassembly, protocol='my_protocol'): ... + Omit it and the subclass is *not* registered, which is how a class + that is not meant to be selectable by name declines: + + .. code-block:: python + + class MyMixin(Reassembly): # not registered + ... + + Such a class can still be registered later, on demand: + + .. code-block:: python + + Extractor.register_reassembly('my_mixin', MyMixin) + Arguments: strict: if return all datagrams (including those not implemented) when submit @@ -499,23 +539,50 @@ class MyProtocol(Reassembly, protocol='my_protocol'): def __init_subclass__(cls, /, protocol: 'Optional[str]' = None, *args: 'Any', **kwargs: 'Any') -> 'None': """Initialise subclass. - This method is to be used for registering the engine class to + This method is to be used for registering the reassembly class to :class:`~pcapkit.foundation.extraction.Extractor` class. Args: - name: Protocol name, default to class name. + protocol: Protocol name to register the subclass under, lowercased. + :data:`None` (the default) skips registration entirely. *args: Arbitrary positional arguments. **kwargs: Arbitrary keyword arguments. + Raises: + UnsupportedCall: If any unrecognised class keyword is given. + + Registration is **opt-in**: the subclass is registered if and only if + ``protocol`` is given. This is what lets a subclass decline registration + rather than having to inherit :class:`ReassemblyBase` to avoid it, and it + matches :meth:`EnumSchema.__init_subclass__ + `, which + has guarded on its own ``code`` keyword all along. + + Note: + :attr:`__protocol_name__` is *not* an opt-in. It supplies the + :attr:`name ` + the reassembly reports, which it does whether or not the class is + registered; only the keyword decides registration. + See Also: For more details, please refer to :meth:`pcapkit.foundation.extraction.Extractor.register_reassembly`. """ - if protocol is None: - protocol = cast('str', cls.name) - - from pcapkit.foundation.extraction import Extractor - Extractor.register_reassembly(protocol.lower(), cls) + # NOTE: the keyword here is ``protocol``, but ``Engine`` spells the same + # idea ``name`` -- so guessing ``name=`` by analogy is the expected + # mistake, not a careless one. It used to land in ``**kwargs``, get + # dropped by the bare ``super().__init_subclass__()`` below, and leave + # the class registered under its own class name instead: no exception, no + # warning. See the sibling note in ``Engine.__init_subclass__``. + if args or kwargs: + unexpected = ', '.join([*map(repr, args), *sorted(kwargs)]) + raise UnsupportedCall(f'{cls.__name__}: unexpected class keyword(s): {unexpected}') + + if protocol is not None: + from pcapkit.foundation.extraction import \ + Extractor # pylint: disable=import-outside-toplevel + + Extractor.register_reassembly(protocol.lower(), cls) return super().__init_subclass__() diff --git a/pcapkit/foundation/traceflow/traceflow.py b/pcapkit/foundation/traceflow/traceflow.py index 8c4abfa60..5474d6d13 100644 --- a/pcapkit/foundation/traceflow/traceflow.py +++ b/pcapkit/foundation/traceflow/traceflow.py @@ -14,7 +14,7 @@ import collections import os import sys -from typing import TYPE_CHECKING, Generic, TypeVar, cast, overload +from typing import TYPE_CHECKING, Generic, TypeVar, overload from dictdumper.dumper import Dumper @@ -22,7 +22,7 @@ from pcapkit.dumpkit.common import make_dumper from pcapkit.protocols import __proto__ as protocol_registry from pcapkit.protocols.misc.raw import Raw -from pcapkit.utilities.exceptions import FileExists, RegistryError, stacklevel +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 @@ -81,6 +81,35 @@ def protocol(cls) -> 'Type[Protocol]': return cls.__protocol_type__ return protocol_registry.get(cls.name.upper(), Raw) + @property + def registry(cls) -> 'dict[str, ModuleDescriptor[TraceFlow] | Type[TraceFlow]]': + """Mapping of protocol names to flow tracing classes. + + Note: + Unlike :attr:`EnumSchema.registry + `, this is not + a per-class mapping: every flow tracing registration lands in the + single :attr:`Extractor.__traceflow__ + ` table, so + reading it through any subclass returns that same object. The + property exists so ``MyTraceFlow.registry`` is spelled the same way + here as it is for schemas. + + Note also that :class:`EnumSchema` carries *two* ``registry`` + properties, one on its metaclass and one on the class body, so it + answers on an instance as well. This one is on the metaclass only, + so it is available as a class attribute and **not** on an instance. + + This is *not* :attr:`TraceFlow.__output__ + `, + which is the separate output-dumper table this class also owns. + + """ + from pcapkit.foundation.extraction import \ + Extractor # pylint: disable=import-outside-toplevel + + return Extractor.__traceflow__ + class TraceFlowBase(Generic[_DT, _BT, _IT, _PT], metaclass=TraceFlowMeta): """Base flow tracing class. @@ -409,14 +438,28 @@ class TraceFlow(TraceFlowBase[_DT, _BT, _IT, _PT], Generic[_DT, _BT, _IT, _PT]): Example: - Use keyword argument ``protocol`` to specify the protocol - name at class definition: + Registration is opt-in. Pass keyword argument ``protocol`` at class + definition to register the flow tracing class under that protocol name: .. code-block:: python class MyProtocol(TraceFlow, protocol='my_protocol'): ... + Omit it and the subclass is *not* registered, which is how a class + that is not meant to be selectable by name declines: + + .. code-block:: python + + class MyMixin(TraceFlow): # not registered + ... + + Such a class can still be registered later, on demand: + + .. code-block:: python + + Extractor.register_traceflow('my_mixin', MyMixin) + Arguments: fout: output path format: output format @@ -430,23 +473,50 @@ class MyProtocol(TraceFlow, protocol='my_protocol'): def __init_subclass__(cls, /, protocol: 'Optional[str]' = None, *args: 'Any', **kwargs: 'Any') -> 'None': """Initialise subclass. - This method is to be used for registering the engine class to + This method is to be used for registering the flow tracing class to :class:`~pcapkit.foundation.extraction.Extractor` class. Args: - name: Protocol name, default to class name. + protocol: Protocol name to register the subclass under, lowercased. + :data:`None` (the default) skips registration entirely. *args: Arbitrary positional arguments. **kwargs: Arbitrary keyword arguments. + Raises: + UnsupportedCall: If any unrecognised class keyword is given. + + Registration is **opt-in**: the subclass is registered if and only if + ``protocol`` is given. This is what lets a subclass decline registration + rather than having to inherit :class:`TraceFlowBase` to avoid it, and it + matches :meth:`EnumSchema.__init_subclass__ + `, which + has guarded on its own ``code`` keyword all along. + + Note: + :attr:`__protocol_name__` is *not* an opt-in. It supplies the + :attr:`name ` + the class reports, which it does whether or not the class is + registered; only the keyword decides registration. + See Also: For more details, please refer to :meth:`pcapkit.foundation.extraction.Extractor.register_traceflow`. """ - if protocol is None: - protocol = cast('str', cls.name) - - from pcapkit.foundation.extraction import Extractor - Extractor.register_traceflow(protocol.lower(), cls) + # NOTE: the keyword here is ``protocol``, but ``Engine`` spells the same + # idea ``name`` -- so guessing ``name=`` by analogy is the expected + # mistake, not a careless one. It used to land in ``**kwargs``, get + # dropped by the bare ``super().__init_subclass__()`` below, and leave + # the class registered under its own class name instead: no exception, no + # warning. See the sibling note in ``Engine.__init_subclass__``. + if args or kwargs: + unexpected = ', '.join([*map(repr, args), *sorted(kwargs)]) + raise UnsupportedCall(f'{cls.__name__}: unexpected class keyword(s): {unexpected}') + + if protocol is not None: + from pcapkit.foundation.extraction import \ + Extractor # pylint: disable=import-outside-toplevel + + Extractor.register_traceflow(protocol.lower(), cls) return super().__init_subclass__() diff --git a/tests/dumpkit/test_common_unit.py b/tests/dumpkit/test_common_unit.py index b1dc04203..0a79a33a3 100644 --- a/tests/dumpkit/test_common_unit.py +++ b/tests/dumpkit/test_common_unit.py @@ -54,7 +54,17 @@ class DumpkitCommonTests(unittest.TestCase): def setUp(self) -> None: purge_modules(['pcapkit']) - def test_dumper_subclass_registration_explicit_and_inferred(self) -> None: + def test_dumper_subclass_registration_is_opt_in(self) -> None: + """Registration happens if and only if ``fmt`` is given. + + This is the #514 opt-in contract, extended to the fifth pair. Before it, + an absent ``fmt`` was *inferred* from the subclass' + :attr:`~dictdumper.dumper.Dumper.kind` property -- which is an instance + property, so the old code constructed an instance against a + :func:`tempfile.NamedTemporaryFile` while the ``class`` statement was + still executing. A class definition no longer touches the filesystem. + + """ from pcapkit.dumpkit.common import Dumper with mock.patch('pcapkit.foundation.extraction.Extractor.register_dumper') as extractor: @@ -65,15 +75,59 @@ class ExplicitDumper(Dumper, fmt='CUSTOM', ext='.custom'): extractor.assert_called_once_with('custom', ExplicitDumper, '.custom') traceflow.assert_called_once_with('custom', ExplicitDumper, '.custom') + # ``ext`` is still inferred from ``fmt`` -- that is a default for a format + # that *was* given, not a decision to register. with mock.patch('pcapkit.foundation.extraction.Extractor.register_dumper') as extractor: with mock.patch('pcapkit.foundation.traceflow.traceflow.TraceFlow.register_dumper') as traceflow: - class InferredDumper(Dumper): - @property - def kind(self): - return 'AUTO' + class FormatOnlyDumper(Dumper, fmt='FMTONLY'): + pass + + extractor.assert_called_once_with('fmtonly', FormatOnlyDumper, '.fmtonly') + traceflow.assert_called_once_with('fmtonly', FormatOnlyDumper, '.fmtonly') + + with mock.patch('pcapkit.foundation.extraction.Extractor.register_dumper') as extractor: + with mock.patch('pcapkit.foundation.traceflow.traceflow.TraceFlow.register_dumper') as traceflow: + with mock.patch('tempfile.NamedTemporaryFile') as named_temp: + class InferredDumper(Dumper): + @property + def kind(self): + return 'AUTO' + + extractor.assert_not_called() + traceflow.assert_not_called() + # the old inference path is gone, not merely unused + named_temp.assert_not_called() + + def test_dumper_subclass_rejects_ext_without_fmt(self) -> None: + """``ext`` alone cannot register anything, so it raises rather than no-op. + + With registration keyed on ``fmt``, an ``ext`` on its own has no format + to attach to. Accepting it would silently discard the caller's intent. + + """ + from pcapkit.dumpkit.common import Dumper + from pcapkit.utilities.exceptions import UnsupportedCall + + with mock.patch('pcapkit.foundation.extraction.Extractor.register_dumper') as extractor: + with self.assertRaises(UnsupportedCall) as caught: + class ExtOnly(Dumper, ext='.lonely'): + pass + + extractor.assert_not_called() + self.assertIn('.lonely', str(caught.exception)) + + def test_dumper_subclass_rejects_unrecognised_keyword(self) -> None: + """A misspelled class keyword raises instead of being swallowed.""" + from pcapkit.dumpkit.common import Dumper + from pcapkit.utilities.exceptions import UnsupportedCall + + with mock.patch('pcapkit.foundation.extraction.Extractor.register_dumper') as extractor: + with self.assertRaises(UnsupportedCall) as caught: + class Typo(Dumper, format='wrong-keyword-for-dumper'): + pass - extractor.assert_called_once_with('auto', InferredDumper, '.auto') - traceflow.assert_called_once_with('auto', InferredDumper, '.auto') + extractor.assert_not_called() + self.assertIn('format', str(caught.exception)) def test_make_dumper_object_hook_conversions_and_fallbacks(self) -> None: from pcapkit.corekit.infoclass import Info diff --git a/tests/foundation/engines/test_engine_base.py b/tests/foundation/engines/test_engine_base.py index bb8f97e91..bebf42f97 100644 --- a/tests/foundation/engines/test_engine_base.py +++ b/tests/foundation/engines/test_engine_base.py @@ -63,7 +63,32 @@ def read_frame(self) -> str: self.assertEqual(named.name, 'instance') self.assertEqual(named.module, 'instance.module') - def test_engine_subclass_registration_explicit_and_default(self) -> None: + def test_engine_subclass_registration_is_opt_in(self) -> None: + """Registration happens if and only if ``name`` is given. + + This is the #514 opt-in contract. Before it, ``__init_subclass__`` fell + back to ``cls.name`` when the keyword was absent, so *every* subclass of + the public class was registered -- which is why no built-in could + subclass it and every one of them inherited :class:`EngineBase` under an + alias instead. + + The ``Default`` case is the load-bearing half: it sets + ``__engine_name__``, so under the old fallback it registered under + ``'defaultengine'``. A class attribute is not a registry key, and now it + registers nothing. + + Note on the version guard below, which predates this change: it is + *not* about ``Engine`` being generic. ``Engine``'s registry keyword is + literally ``name``, and ``mcls``/``name``/``bases``/``namespace`` collide + with :meth:`abc.ABCMeta.__new__`'s own parameters, which are + positional-or-keyword on Python 3.10 and positional-only from 3.11. So on + 3.10 ``class MyEngine(Engine, name='x')`` -- the documented way to + register an engine -- raises :exc:`TypeError` from the metaclass before + ``__init_subclass__`` runs. Measured against this tree on 3.10.21. A + non-colliding keyword needs no guard, which is why the sibling test above + has none. + + """ from pcapkit.foundation.engines.engine import Engine if sys.version_info >= (3, 11): @@ -87,7 +112,115 @@ def run(self) -> None: def read_frame(self) -> str: return 'frame' - register.assert_called_once_with('defaultengine', Default) + register.assert_not_called() + + # ... and ``__engine_name__`` keeps doing its own job regardless, which is + # why it is not an opt-in: it names the engine, registered or not. + self.assertEqual(Default.name, 'DefaultEngine') + + def test_engine_subclass_rejects_unrecognised_keyword(self) -> None: + """A misspelled class keyword raises instead of being swallowed. + + ``Engine`` spells the registry key ``name`` while ``Reassembly`` and + ``TraceFlow`` spell the same idea ``protocol``, so guessing the wrong one + is the expected mistake. It used to land in ``**kwargs``, get dropped by + the bare ``super().__init_subclass__()``, and leave the class registered + under its own class name. With registration now opt-in the same typo + would instead skip registration silently, which is quieter still. + + Unguarded by version on purpose: ``protocol`` is not one of the four + names that collide with :meth:`abc.ABCMeta.__new__` on Python 3.10, so + the guard is reached on every supported version. Verified against this + tree on 3.10.21, where it raises ``UnsupportedCall`` as it does on 3.14. + + """ + from pcapkit.foundation.engines.engine import Engine + from pcapkit.utilities.exceptions import UnsupportedCall + + with mock.patch('pcapkit.foundation.extraction.Extractor.register_engine') as register: + with self.assertRaises(UnsupportedCall) as caught: + class Typo(Engine[str], protocol='wrong-keyword-for-engine'): + def run(self) -> None: + pass + + def read_frame(self) -> str: + return 'frame' + + register.assert_not_called() + self.assertIn('protocol', str(caught.exception)) + + @unittest.skipIf(sys.version_info < (3, 11), + "Engine's registry keyword is literally `name`, which collides " + 'with ABCMeta.__new__ on 3.10 -- see the note in the docstring') + def test_registration_is_not_inherited_by_a_subclass(self) -> None: + """A subclass of a *registered* class does not inherit its registration. + + This is #514's answer to what the ``*Base`` hierarchy was standing in for. + The built-ins inherit the non-registering base as a substitute for a + ``final`` marker -- so that subclassing them again cannot register the + subclass under a name nobody chose. That worked, but it is a property of + *which base you inherited*, so it cannot be varied per subclass, and + under the old fallback a subclass of a registered class re-registered + itself under its own class name. + + Keying registration on the keyword instead gives both properties at once: + the parent stays registered, the subclass does not re-register, and a + subclass that *wants* registering can still ask for it. Which is why + ``HTTP``, ``L2TP`` and ``IP`` can be open to inheritance without the + anti-re-registration guarantee being weakened. + + """ + from pcapkit.foundation.engines.engine import Engine + + with mock.patch('pcapkit.foundation.extraction.Extractor.register_engine') as register: + class Parent(Engine[str], name='ParentEngine'): + def run(self) -> None: + pass + + def read_frame(self) -> str: + return 'frame' + + register.assert_called_once_with('parentengine', Parent) + + # the substitute for ``final``: inheriting a registered class registers + # nothing, where the old fallback registered it as 'derived' + with mock.patch('pcapkit.foundation.extraction.Extractor.register_engine') as register: + class Derived(Parent): + pass + + register.assert_not_called() + + # ... and inheritance is still open to a subclass that asks to register + with mock.patch('pcapkit.foundation.extraction.Extractor.register_engine') as register: + class DerivedOptIn(Parent, name='DerivedEngine'): + pass + + register.assert_called_once_with('derivedengine', DerivedOptIn) + self.assertTrue(issubclass(DerivedOptIn, Parent)) + + def test_engine_registry_property_reads_the_extractor_table(self) -> None: + """``Engine.registry`` is a class-level accessor, as on ``EnumSchema``. + + It has to live on the metaclass: a ``property`` in the class body would + be an instance property, so ``Engine.registry`` would return the + property object rather than the mapping. + + """ + from pcapkit.foundation.engines.engine import Engine + from pcapkit.foundation.extraction import Extractor + + self.assertIs(Engine.registry, Extractor.__engine__) + + # read through a subclass too, which is the spelling the property exists + # for. No class keyword here, so this works on every supported version. + class Subclass(Engine[str]): + def run(self) -> None: + pass + + def read_frame(self) -> str: + return 'frame' + + self.assertIs(Subclass.registry, Extractor.__engine__) if __name__ == '__main__': diff --git a/tests/foundation/reassembly/test_reassembly_base.py b/tests/foundation/reassembly/test_reassembly_base.py index 5e14921f6..a0eee7536 100644 --- a/tests/foundation/reassembly/test_reassembly_base.py +++ b/tests/foundation/reassembly/test_reassembly_base.py @@ -1,6 +1,7 @@ from __future__ import annotations import importlib.util +import sys import unittest from unittest import mock @@ -122,7 +123,13 @@ class InstanceProtocol: with self.assertRaises(UnsupportedCall): _ = reasm.datagram - def test_reassembly_subclass_registration_uses_explicit_and_default_protocols(self) -> None: + def test_reassembly_subclass_registration_is_opt_in(self) -> None: + """Registration happens if and only if ``protocol`` is given. + + This is the #514 opt-in contract; see the sibling test in + ``tests/foundation/engines/test_engine_base.py`` for the full rationale. + + """ from pcapkit.corekit.infoclass import Info, info_final from pcapkit.foundation.reassembly.reassembly import Reassembly @@ -160,7 +167,82 @@ def reassembly(self, info: DummyPacket) -> None: def submit(self, buf: DummyBuffer, **kwargs: object) -> list[DummyDatagram]: return [] - register.assert_called_once_with('defaultproto', Default) + # #514: registration is opt-in. Before it, the absent keyword fell back to + # ``cls.name`` -- i.e. ``__protocol_name__`` here -- and this registered + # under ``'defaultproto'``. A class attribute is not a registry key. + register.assert_not_called() + self.assertEqual(Default.name, 'DefaultProto') + self.assertEqual(Default.__callback_fn__, []) + + def test_reassembly_subclass_rejects_unrecognised_keyword(self) -> None: + """A misspelled class keyword raises instead of being swallowed. + + It used to land in ``**kwargs``, get dropped by the bare + ``super().__init_subclass__()``, and leave the class registered under its + own class name -- no exception, no warning. + + The keyword used here is deliberately *not* ``name``. Four class keyword + names -- ``mcls``, ``name``, ``bases`` and ``namespace`` -- collide with + :meth:`abc.ABCMeta.__new__`'s own parameters, which are + positional-or-keyword on Python 3.10 and positional-only from 3.11. On + 3.10 the collision therefore raises :exc:`TypeError` from the metaclass + *before* ``__init_subclass__`` is reached, so testing the guard with + ``name=`` would only exercise it on 3.11+. Measured on 3.10.21, 3.11.15 + and 3.14.7. The colliding case is pinned separately below. + + """ + from pcapkit.corekit.infoclass import Info, info_final + from pcapkit.foundation.reassembly.reassembly import Reassembly + from pcapkit.utilities.exceptions import UnsupportedCall + + @info_final + class DummyPacket(Info): + value: int + + @info_final + class DummyDatagram(Info): + index: tuple[int, ...] + + @info_final + class DummyBuffer(Info): + entries: list[int] + + with mock.patch('pcapkit.foundation.extraction.Extractor.register_reassembly') as register: + with self.assertRaises(UnsupportedCall) as caught: + class Typo(Reassembly[DummyPacket, DummyDatagram, tuple[str], DummyBuffer], + reassembly='wrong-keyword-for-reassembly'): + def reassembly(self, info: DummyPacket) -> None: + super().reassembly(info) + + def submit(self, buf: DummyBuffer, **kwargs: object) -> list[DummyDatagram]: + return [] + + register.assert_not_called() + self.assertIn('reassembly', str(caught.exception)) + + # ``name=`` is the mistake a user actually makes, by analogy with + # ``Engine``, and it is one of the four colliding names -- so which + # exception surfaces is version-dependent. Pinned rather than skipped, so + # the 3.10 behaviour is recorded rather than merely untested. + expected = UnsupportedCall if sys.version_info >= (3, 11) else TypeError + with mock.patch('pcapkit.foundation.extraction.Extractor.register_reassembly') as register: + with self.assertRaises(expected): + class Collides(Reassembly[DummyPacket, DummyDatagram, tuple[str], DummyBuffer], + name='collides-with-ABCMeta-on-3.10'): + def reassembly(self, info: DummyPacket) -> None: + super().reassembly(info) + + def submit(self, buf: DummyBuffer, **kwargs: object) -> list[DummyDatagram]: + return [] + + register.assert_not_called() + + def test_reassembly_registry_property_reads_the_extractor_table(self) -> None: + """``Reassembly.registry`` is a class-level accessor, as on ``EnumSchema``.""" + from pcapkit.foundation.extraction import Extractor + from pcapkit.foundation.reassembly.reassembly import Reassembly + + self.assertIs(Reassembly.registry, Extractor.__reassembly__) if __name__ == '__main__': diff --git a/tests/foundation/traceflow/test_traceflow_base.py b/tests/foundation/traceflow/test_traceflow_base.py index fdb302687..34ca2afab 100644 --- a/tests/foundation/traceflow/test_traceflow_base.py +++ b/tests/foundation/traceflow/test_traceflow_base.py @@ -2,6 +2,7 @@ import importlib.util import pathlib +import sys import tempfile import unittest from unittest import mock @@ -177,7 +178,89 @@ def trace(self, packet: DummyPacket, *, output: bool = False): def submit(self) -> tuple[DummyIndex, ...]: return () - register.assert_called_once_with('defaulttrace', Default) + # #514: registration is opt-in. Before it, the absent keyword fell back to + # ``cls.name`` -- i.e. ``__protocol_name__`` here -- and this registered + # under ``'defaulttrace'``. A class attribute is not a registry key. + register.assert_not_called() + self.assertEqual(Default.name, 'DefaultTrace') + + def test_traceflow_subclass_rejects_unrecognised_keyword(self) -> None: + """A misspelled class keyword raises instead of being swallowed. + + It used to land in ``**kwargs``, get dropped by the bare + ``super().__init_subclass__()``, and leave the class registered under its + own class name -- no exception, no warning. + + The keyword used here is deliberately *not* ``name``; see the sibling + test in ``tests/foundation/reassembly/test_reassembly_base.py`` for why + four class keyword names collide with :meth:`abc.ABCMeta.__new__` on + Python 3.10. The colliding case is pinned separately below. + + """ + from pcapkit.corekit.infoclass import Info, info_final + from pcapkit.foundation.traceflow.traceflow import TraceFlow + from pcapkit.utilities.exceptions import UnsupportedCall + + @info_final + class DummyPacket(Info): + index: int + + @info_final + class DummyIndex(Info): + index: tuple[int, ...] + + @info_final + class DummyBuffer(Info): + index: list[int] + + with mock.patch('pcapkit.foundation.extraction.Extractor.register_traceflow') as register: + with self.assertRaises(UnsupportedCall) as caught: + class Typo(TraceFlow[str, DummyBuffer, DummyIndex, DummyPacket], + traceflow='wrong-keyword-for-traceflow'): + def dump(self, packet: DummyPacket) -> None: + self.trace(packet) + + def trace(self, packet: DummyPacket, *, output: bool = False): + return object() if output else 'flow' + + def submit(self) -> tuple[DummyIndex, ...]: + return () + + register.assert_not_called() + self.assertIn('traceflow', str(caught.exception)) + + # ``name=`` is the mistake a user actually makes, by analogy with + # ``Engine``, and it is one of the four colliding names -- so which + # exception surfaces is version-dependent. Pinned rather than skipped. + expected = UnsupportedCall if sys.version_info >= (3, 11) else TypeError + with mock.patch('pcapkit.foundation.extraction.Extractor.register_traceflow') as register: + with self.assertRaises(expected): + class Collides(TraceFlow[str, DummyBuffer, DummyIndex, DummyPacket], + name='collides-with-ABCMeta-on-3.10'): + def dump(self, packet: DummyPacket) -> None: + self.trace(packet) + + def trace(self, packet: DummyPacket, *, output: bool = False): + return object() if output else 'flow' + + def submit(self) -> tuple[DummyIndex, ...]: + return () + + register.assert_not_called() + + def test_traceflow_registry_property_reads_the_extractor_table(self) -> None: + """``TraceFlow.registry`` is a class-level accessor, as on ``EnumSchema``. + + Note it is the *flow tracing* registry, not + :attr:`TraceFlow.__output__`, which is the separate output-dumper table + this same class owns. + + """ + from pcapkit.foundation.extraction import Extractor + from pcapkit.foundation.traceflow.traceflow import TraceFlow + + self.assertIs(TraceFlow.registry, Extractor.__traceflow__) + self.assertIsNot(TraceFlow.registry, TraceFlow.__output__) if __name__ == '__main__':