feat(foundation): make subclass registration opt-in for Engine, Reassembly, TraceFlow and Dumper (#514)

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).

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

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

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

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

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

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,32 +56,75 @@ 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,
                           ext: 'Optional[str]' = None, *args: 'Any', **kwargs: 'Any') -> '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__
+        <pcapkit.protocols.schema.schema.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__()