fix(foundation): add the #718 identity guard to the five registrars it missed

pcapkit/foundation/extraction.py

@@ -373,6 +373,22 @@ def register_dumper(cls, format: 'str', dumper: 'ModuleDescriptor[Dumper] | Type
             The full qualified class name of the new dumper class
             should be as ``{dumper.module}.{dumper.name}``.
 
+            The overwrite guard fires only when the incumbent dumper differs
+            from the replacement, so re-registering the exact same object is
+            a silent no-op rather than a warning about nothing displaced --
+            the identity guard GitHub issue #718 gave the code-keyed
+            registrars, extended here by GitHub issue #739. ``__output__``
+            maps each format to a ``(dumper, ext)`` pair, so the identity
+            check compares the incumbent *dumper* (index ``0``), not the
+            pair -- a re-registration that only changes ``ext`` is still
+            identity-equal on the dumper and stays silent, since the dumper
+            is what "the same object" means here, not the pair as a whole.
+            ``__output__`` is also a :class:`collections.defaultdict`,
+            unlike the other three
+            registrars this issue touches; :meth:`dict.get` does not invoke
+            the default factory the way ``cls.__output__[format]`` would, so
+            it stays non-inserting here as well.
+
         Arguments:
             format: format name
             dumper: module descriptor or a :class:`dictdumper.dumper.Dumper` subclass
@@ -383,7 +399,9 @@ def register_dumper(cls, format: 'str', dumper: 'ModuleDescriptor[Dumper] | Type
             dumper = dumper.klass
         if not issubclass(dumper, Dumper):
             raise RegistryError(f'dumper must be a Dumper subclass, not {dumper!r}')
-        if format in cls.__output__:
+        incumbent_entry = cls.__output__.get(format)
+        incumbent = incumbent_entry[0] if incumbent_entry is not None else None
+        if incumbent is not None and incumbent is not dumper:
             warn(f'dumper {format} already registered, overwriting', RegistryWarning)
         cls.__output__[format] = (dumper, ext)
 
@@ -395,6 +413,12 @@ def register_engine(cls, name: 'str', engine: 'ModuleDescriptor[Engine] | Type[E
             The full qualified class name of the new extraction engine
             should be as ``{engine.module}.{engine.name}``.
 
+            The overwrite guard fires only when the incumbent differs from
+            the replacement, so re-registering the exact same object is a
+            silent no-op rather than a warning about nothing displaced --
+            the identity guard GitHub issue #718 gave the code-keyed
+            registrars, extended here by GitHub issue #739.
+
         Arguments:
             name: engine name
             engine: module descriptor or an
@@ -410,7 +434,8 @@ def register_engine(cls, name: 'str', engine: 'ModuleDescriptor[Engine] | Type[E
         # classes. ``Engine`` is itself an ``EngineBase``, so this only widens. See #513.
         if not issubclass(engine, EngineBase):
             raise RegistryError(f'engine must be an Engine subclass, not {engine!r}')
-        if name in cls.__engine__:
+        incumbent = cls.__engine__.get(name)
+        if incumbent is not None and incumbent is not engine:
             warn(f'engine {name} already registered, overwriting', RegistryWarning)
         cls.__engine__[name] = engine
 
@@ -422,6 +447,12 @@ def register_reassembly(cls, protocol: 'str', reassembly: 'ModuleDescriptor[Reas
             The full qualified class name of the new reassembly engine
             should be as ``{reassembly.module}.{reassembly.name}``.
 
+            The overwrite guard fires only when the incumbent differs from
+            the replacement, so re-registering the exact same object is a
+            silent no-op rather than a warning about nothing displaced --
+            the identity guard GitHub issue #718 gave the code-keyed
+            registrars, extended here by GitHub issue #739.
+
         Arguments:
             protocol: protocol name
             reassembly: module descriptor or a
@@ -434,7 +465,8 @@ def register_reassembly(cls, protocol: 'str', reassembly: 'ModuleDescriptor[Reas
         # :meth:`register_engine` above -- see #513.
         if not issubclass(reassembly, ReassemblyBase):
             raise RegistryError(f'reassembly must be a Reassembly subclass, not {reassembly!r}')
-        if protocol in cls.__reassembly__:
+        incumbent = cls.__reassembly__.get(protocol)
+        if incumbent is not None and incumbent is not reassembly:
             warn(f'reassembly {protocol} already registered, overwriting', RegistryWarning)
         cls.__reassembly__[protocol] = reassembly
 
@@ -446,6 +478,12 @@ def register_traceflow(cls, protocol: 'str', traceflow: 'ModuleDescriptor[TraceF
             The full qualified class name of the new flow tracing engine
             should be as ``{traceflow.module}.{traceflow.name}``.
 
+            The overwrite guard fires only when the incumbent differs from
+            the replacement, so re-registering the exact same object is a
+            silent no-op rather than a warning about nothing displaced --
+            the identity guard GitHub issue #718 gave the code-keyed
+            registrars, extended here by GitHub issue #739.
+
         Arguments:
             protocol: protocol name
             traceflow: module descriptor or a
@@ -458,7 +496,8 @@ def register_traceflow(cls, protocol: 'str', traceflow: 'ModuleDescriptor[TraceF
         # :meth:`register_engine` above -- see #513.
         if not issubclass(traceflow, TraceFlowBase):
             raise RegistryError(f'traceflow must be a TraceFlow subclass, not {traceflow!r}')
-        if protocol in cls.__traceflow__:
+        incumbent = cls.__traceflow__.get(protocol)
+        if incumbent is not None and incumbent is not traceflow:
             warn(f'traceflow {protocol} already registered, overwriting', RegistryWarning)
         cls.__traceflow__[protocol] = traceflow
 

pcapkit/foundation/traceflow/traceflow.py

@@ -210,6 +210,22 @@ def register_dumper(cls, format: 'str', dumper: 'ModuleDescriptor[Dumper] | Type
             The full qualified class name of the new dumper class
             should be as ``{dumper.module}.{dumper.name}``.
 
+            The overwrite guard fires only when the incumbent dumper differs
+            from the replacement, so re-registering the exact same object is
+            a silent no-op rather than a warning about nothing displaced --
+            the identity guard GitHub issue #718 gave the code-keyed
+            registrars, extended here by GitHub issue #739. ``__output__``
+            maps each format to a ``(dumper, ext)`` pair, so the identity
+            check compares the incumbent *dumper* (index ``0``), not the
+            pair -- a re-registration that only changes ``ext`` is still
+            identity-equal on the dumper and stays silent, since the dumper
+            is what "the same object" means here, not the pair as a whole.
+            ``__output__`` is also a :class:`collections.defaultdict`,
+            unlike the other three
+            registrars this issue touches; :meth:`dict.get` does not invoke
+            the default factory the way ``cls.__output__[format]`` would, so
+            it stays non-inserting here as well.
+
         Arguments:
             format: format name
             dumper: module descriptor or a :class:`dictdumper.dumper.Dumper` subclass
@@ -220,7 +236,9 @@ def register_dumper(cls, format: 'str', dumper: 'ModuleDescriptor[Dumper] | Type
             dumper = dumper.klass
         if not issubclass(dumper, Dumper):
             raise RegistryError(f'dumper must be a Dumper subclass, not {dumper!r}')
-        if format in cls.__output__:
+        incumbent_entry = cls.__output__.get(format)
+        incumbent = incumbent_entry[0] if incumbent_entry is not None else None
+        if incumbent is not None and incumbent is not dumper:
             warn(f'dumper {format} already registered, overwriting', RegistryWarning)
         cls.__output__[format] = (dumper, ext)
 

tests/foundation/test_extraction.py

@@ -246,6 +246,204 @@ def submit(self) -> tuple[object, ...]:
         Extractor.register_traceflow('unit-trace-descriptor',
                                      ModuleDescriptor('unit_extraction_trace_mod', 'UnitTraceFlow'))
 
+    def test_register_engine_identity_guard(self) -> None:
+        """GitHub issue #739: re-registering the same engine class is silent.
+
+        ``register_engine`` used a presence-only guard, so the *second* of two
+        calls registering the exact same class under the exact same name
+        warned about an overwrite that never happened. This mirrors the
+        identity guard GitHub issue #718 gave the code-keyed protocol
+        registrars (see ``test_register_stays_quiet_on_same_object_reregistration``
+        in ``tests/protocols/test_protocol_base_unit.py``): the guard now
+        fires only when the incumbent differs from the replacement, and the
+        registry write happens either way.
+
+        """
+        from pcapkit.foundation.engines.engine import Engine
+        from pcapkit.foundation.extraction import Extractor
+        from pcapkit.utilities.warnings import RegistryWarning
+
+        class UnitEngineIdentityA(Engine[str]):
+            __engine_name__ = 'UnitEngineIdentityA'
+            __engine_module__ = __name__
+
+            def run(self) -> None:
+                pass
+
+            def read_frame(self) -> str:
+                return 'frame'
+
+        class UnitEngineIdentityB(Engine[str]):
+            __engine_name__ = 'UnitEngineIdentityB'
+            __engine_module__ = __name__
+
+            def run(self) -> None:
+                pass
+
+            def read_frame(self) -> str:
+                return 'frame'
+
+        with mock.patch('pcapkit.foundation.extraction.warn'):
+            Extractor.register_engine('unit-engine-identity', UnitEngineIdentityA)
+
+        # 1. Re-registering the same object is silent.
+        with mock.patch('pcapkit.foundation.extraction.warn') as warn:
+            Extractor.register_engine('unit-engine-identity', UnitEngineIdentityA)
+        warn.assert_not_called()
+        # 3. The registry write still happens even when the guard stays quiet.
+        self.assertIs(Extractor.__engine__['unit-engine-identity'], UnitEngineIdentityA)
+
+        # 2. Replacing with a different object still warns, message unchanged.
+        with mock.patch('pcapkit.foundation.extraction.warn') as warn:
+            Extractor.register_engine('unit-engine-identity', UnitEngineIdentityB)
+        warn.assert_called_once_with(
+            'engine unit-engine-identity already registered, overwriting', RegistryWarning)
+        # 3. And the write happens on the warning path too.
+        self.assertIs(Extractor.__engine__['unit-engine-identity'], UnitEngineIdentityB)
+
+    def test_register_reassembly_identity_guard(self) -> None:
+        """GitHub issue #739: re-registering the same reassembly class is silent.
+
+        Same defect and fix as ``test_register_engine_identity_guard``, for
+        ``register_reassembly``.
+
+        """
+        from pcapkit.foundation.extraction import Extractor
+        from pcapkit.foundation.reassembly.reassembly import Reassembly
+        from pcapkit.utilities.warnings import RegistryWarning
+
+        with mock.patch('pcapkit.foundation.extraction.Extractor.register_reassembly'):
+            class UnitReassemblyIdentityA(Reassembly[object, object, tuple[str], object],
+                                           protocol='unit-reassembly-identity-a'):
+                def reassembly(self, info: object) -> None:
+                    pass
+
+                def submit(self, buf: object, **kwargs: object) -> list[object]:
+                    return []
+
+            class UnitReassemblyIdentityB(Reassembly[object, object, tuple[str], object],
+                                           protocol='unit-reassembly-identity-b'):
+                def reassembly(self, info: object) -> None:
+                    pass
+
+                def submit(self, buf: object, **kwargs: object) -> list[object]:
+                    return []
+
+        with mock.patch('pcapkit.foundation.extraction.warn'):
+            Extractor.register_reassembly('unit-reassembly-identity', UnitReassemblyIdentityA)
+
+        # 1. Re-registering the same object is silent.
+        with mock.patch('pcapkit.foundation.extraction.warn') as warn:
+            Extractor.register_reassembly('unit-reassembly-identity', UnitReassemblyIdentityA)
+        warn.assert_not_called()
+        # 3. The registry write still happens even when the guard stays quiet.
+        self.assertIs(Extractor.__reassembly__['unit-reassembly-identity'], UnitReassemblyIdentityA)
+
+        # 2. Replacing with a different object still warns, message unchanged.
+        with mock.patch('pcapkit.foundation.extraction.warn') as warn:
+            Extractor.register_reassembly('unit-reassembly-identity', UnitReassemblyIdentityB)
+        warn.assert_called_once_with(
+            'reassembly unit-reassembly-identity already registered, overwriting', RegistryWarning)
+        # 3. And the write happens on the warning path too.
+        self.assertIs(Extractor.__reassembly__['unit-reassembly-identity'], UnitReassemblyIdentityB)
+
+    def test_register_traceflow_identity_guard(self) -> None:
+        """GitHub issue #739: re-registering the same traceflow class is silent.
+
+        Same defect and fix as ``test_register_engine_identity_guard``, for
+        ``register_traceflow``.
+
+        """
+        from pcapkit.foundation.extraction import Extractor
+        from pcapkit.foundation.traceflow.traceflow import TraceFlow
+        from pcapkit.utilities.warnings import RegistryWarning
+
+        with mock.patch('pcapkit.foundation.extraction.Extractor.register_traceflow'):
+            class UnitTraceFlowIdentityA(TraceFlow[str, object, object, object],
+                                          protocol='unit-traceflow-identity-a'):
+                def dump(self, packet: object) -> None:
+                    pass
+
+                def trace(self, packet: object, *, output: bool = False):
+                    return object() if output else 'trace'
+
+                def submit(self) -> tuple[object, ...]:
+                    return ()
+
+            class UnitTraceFlowIdentityB(TraceFlow[str, object, object, object],
+                                          protocol='unit-traceflow-identity-b'):
+                def dump(self, packet: object) -> None:
+                    pass
+
+                def trace(self, packet: object, *, output: bool = False):
+                    return object() if output else 'trace'
+
+                def submit(self) -> tuple[object, ...]:
+                    return ()
+
+        with mock.patch('pcapkit.foundation.extraction.warn'):
+            Extractor.register_traceflow('unit-traceflow-identity', UnitTraceFlowIdentityA)
+
+        # 1. Re-registering the same object is silent.
+        with mock.patch('pcapkit.foundation.extraction.warn') as warn:
+            Extractor.register_traceflow('unit-traceflow-identity', UnitTraceFlowIdentityA)
+        warn.assert_not_called()
+        # 3. The registry write still happens even when the guard stays quiet.
+        self.assertIs(Extractor.__traceflow__['unit-traceflow-identity'], UnitTraceFlowIdentityA)
+
+        # 2. Replacing with a different object still warns, message unchanged.
+        with mock.patch('pcapkit.foundation.extraction.warn') as warn:
+            Extractor.register_traceflow('unit-traceflow-identity', UnitTraceFlowIdentityB)
+        warn.assert_called_once_with(
+            'traceflow unit-traceflow-identity already registered, overwriting', RegistryWarning)
+        # 3. And the write happens on the warning path too.
+        self.assertIs(Extractor.__traceflow__['unit-traceflow-identity'], UnitTraceFlowIdentityB)
+
+    def test_register_dumper_identity_guard(self) -> None:
+        """GitHub issue #739: re-registering the same dumper class is silent.
+
+        Same defect and fix as ``test_register_engine_identity_guard``, for
+        ``register_dumper``. Unlike the other three, ``__output__`` maps each
+        format to a ``(dumper, ext)`` pair and is a
+        :class:`collections.defaultdict`, so this also pins that the
+        identity check compares the incumbent *dumper* (not the pair, which
+        would never compare equal once ``ext`` changes) and that the guard's
+        ``.get()`` does not trigger the default factory the way a subscript
+        access would.
+
+        """
+        from pcapkit.dumpkit.null import NotImplementedIO
+        from pcapkit.foundation.extraction import Extractor
+        from pcapkit.utilities.warnings import RegistryWarning
+
+        class UnitDumperIdentityB(NotImplementedIO):
+            pass
+
+        with mock.patch('pcapkit.foundation.extraction.warn'):
+            Extractor.register_dumper('unit-dumper-identity', NotImplementedIO, '.unit-a')
+
+        # 1. Re-registering the same object is silent.
+        with mock.patch('pcapkit.foundation.extraction.warn') as warn:
+            Extractor.register_dumper('unit-dumper-identity', NotImplementedIO, '.unit-a')
+        warn.assert_not_called()
+        # 3. The registry write still happens even when the guard stays quiet.
+        self.assertEqual(Extractor.__output__['unit-dumper-identity'], (NotImplementedIO, '.unit-a'))
+
+        # 2. Replacing with a different object still warns, message unchanged.
+        with mock.patch('pcapkit.foundation.extraction.warn') as warn:
+            Extractor.register_dumper('unit-dumper-identity', UnitDumperIdentityB, '.unit-b')
+        warn.assert_called_once_with(
+            'dumper unit-dumper-identity already registered, overwriting', RegistryWarning)
+        # 3. And the write happens on the warning path too.
+        self.assertEqual(Extractor.__output__['unit-dumper-identity'], (UnitDumperIdentityB, '.unit-b'))
+
+        # Same dumper, only ``ext`` changes: still identity-equal on the
+        # dumper, so still silent -- but the write still updates ``ext``.
+        with mock.patch('pcapkit.foundation.extraction.warn') as warn:
+            Extractor.register_dumper('unit-dumper-identity', UnitDumperIdentityB, '.unit-c')
+        warn.assert_not_called()
+        self.assertEqual(Extractor.__output__['unit-dumper-identity'], (UnitDumperIdentityB, '.unit-c'))
+
     def test_import_test_and_make_name_paths(self) -> None:
         from pcapkit.foundation.extraction import Extractor
         from pcapkit.utilities.exceptions import FileNotFound, FormatError