From 014a33dfa74c846a33bf77cde33b33ae167e3e5f Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 1 Nov 2026 02:48:58 -0500 Subject: [PATCH] refactor(application)!: move DRARP into its own module (#2529) A module holds one protocol class, named after it (owner ruling on #2529). BREAKING CHANGE: DRARP moves from pcapkit.protocols.application.rarp to pcapkit.protocols.application.drarp, and the old path keeps no re-export (#1715, #1744). pcapkit.DRARP and pcapkit.protocols.DRARP are unchanged. - rarp.py keeps RARP alone; RARP's dynamic DRARP alias is unchanged. - Docs: new drarp.rst, versionremoved:: 1.6.0 on rarp, refs retargeted; the VLAN, C/S-Tag and L2TP docs and tests drop the per-index rule. - Tests: the #1744 ledger takes a removal version; the reachability audit accepts a class inheriting its family base's __index__, not a shared module; new test_drarp_unit.py fails before the move. --- .../conventions/protocol-family-grouping.rst | 2 +- docs/source/ext.rst | 2 +- .../pcapkit/protocols/application/drarp.rst | 30 +++++++ .../protocols/application/ftp_data.rst | 6 +- .../pcapkit/protocols/application/index.rst | 1 + .../pcapkit/protocols/application/rarp.rst | 27 +++--- docs/source/pcapkit/protocols/index.rst | 2 +- docs/source/pcapkit/protocols/link/c_tag.rst | 7 +- docs/source/pcapkit/protocols/link/l2tp.rst | 3 +- docs/source/pcapkit/protocols/link/s_tag.rst | 7 +- docs/source/pcapkit/protocols/link/vlan.rst | 12 ++- pcapkit/protocols/application/__init__.py | 3 +- pcapkit/protocols/application/drarp.py | 54 ++++++++++++ pcapkit/protocols/application/ftp_data.py | 6 +- pcapkit/protocols/application/rarp.py | 42 ++++------ pcapkit/protocols/link/arp.py | 2 +- pcapkit/protocols/link/c_tag.py | 10 +-- pcapkit/protocols/link/l2tp.py | 4 +- pcapkit/protocols/link/s_tag.py | 7 +- pcapkit/protocols/link/vlan.py | 22 ++--- ...test_no_moved_name_re_exports_1744_unit.py | 32 ++++--- .../protocols/application/test_drarp_unit.py | 83 +++++++++++++++++++ .../application/test_layer_placement_unit.py | 12 ++- tests/protocols/application/test_rarp_unit.py | 3 +- .../protocols/link/test_arp_roundtrip_unit.py | 6 +- tests/protocols/link/test_link_unit.py | 9 +- .../protocols/test_dispatch_bindings_unit.py | 2 +- .../test_dispatch_reachability_unit.py | 24 ++++-- 28 files changed, 290 insertions(+), 130 deletions(-) create mode 100644 docs/source/pcapkit/protocols/application/drarp.rst create mode 100644 pcapkit/protocols/application/drarp.py create mode 100644 tests/protocols/application/test_drarp_unit.py diff --git a/docs/source/contributing/conventions/protocol-family-grouping.rst b/docs/source/contributing/conventions/protocol-family-grouping.rst index 76bdbc98c3..175b095e1f 100644 --- a/docs/source/contributing/conventions/protocol-family-grouping.rst +++ b/docs/source/contributing/conventions/protocol-family-grouping.rst @@ -92,7 +92,7 @@ Only :class:`~pcapkit.protocols.application.rarp.RARP` ``('RARP', 'DRARP')`` is to EtherType ``0x8035`` (:attr:`~pcapkit.const.reg.ethertype.EtherType.Reverse_Address_Resolution_Protocol`). A parsed RARP takes ``DRARP`` as its alias for operation 5, and -:class:`~pcapkit.protocols.application.rarp.DRARP` ``('DRARP',)`` inherits the +:class:`~pcapkit.protocols.application.drarp.DRARP` ``('DRARP',)`` inherits the ``__index__`` of RARP. A family-inclusive member is selected by any sibling's registry key (a key of diff --git a/docs/source/ext.rst b/docs/source/ext.rst index cde3dd6e60..e1752fe863 100644 --- a/docs/source/ext.rst +++ b/docs/source/ext.rst @@ -108,7 +108,7 @@ The protocol classes :mod:`pcapkit` ships: + +----------------+-----------------------+-------------------------------------------------------------+ | | | :class:`pcapkit.protocols.application.rarp.RARP` | + + RARP Family +-----------------------+-------------------------------------------------------------+ -| | | :class:`pcapkit.protocols.application.rarp.DRARP` | +| | | :class:`pcapkit.protocols.application.drarp.DRARP` | + +----------------+-----------------------+-------------------------------------------------------------+ | | | :class:`pcapkit.protocols.application.ssl.SSL` | + + +-----------------------+-------------------------------------------------------------+ diff --git a/docs/source/pcapkit/protocols/application/drarp.rst b/docs/source/pcapkit/protocols/application/drarp.rst new file mode 100644 index 0000000000..85e2dc3430 --- /dev/null +++ b/docs/source/pcapkit/protocols/application/drarp.rst @@ -0,0 +1,30 @@ +DRARP - Dynamic Reverse Address Resolution Protocol +=================================================== + +.. module:: pcapkit.protocols.application.drarp + +:mod:`pcapkit.protocols.application.drarp` contains +:class:`~pcapkit.protocols.application.drarp.DRARP` only, +which implements an extractor for Dynamic Reverse Address +Resolution Protocol (DRARP) [*]_, whose frame is the one +:mod:`pcapkit.protocols.application.rarp` describes. + +Nothing dispatches to :class:`~pcapkit.protocols.application.drarp.DRARP`: +under EtherType ``0x8035``, a DRARP operation parses as +:class:`~pcapkit.protocols.application.rarp.RARP` with alias ``DRARP``. + +.. versionadded:: 1.6.0 + + Moved here from :mod:`pcapkit.protocols.application.rarp`, which keeps + :class:`~pcapkit.protocols.application.rarp.RARP` alone, since a module + holds one protocol class, named after it (:issue:`2529`). + +.. autoclass:: pcapkit.protocols.application.drarp.DRARP + :no-members: + :show-inheritance: + + .. automethod:: id + +.. rubric:: Footnotes + +.. [*] https://datatracker.ietf.org/doc/html/rfc1931 diff --git a/docs/source/pcapkit/protocols/application/ftp_data.rst b/docs/source/pcapkit/protocols/application/ftp_data.rst index 2288d5083a..423fe3a7b5 100644 --- a/docs/source/pcapkit/protocols/application/ftp_data.rst +++ b/docs/source/pcapkit/protocols/application/ftp_data.rst @@ -13,10 +13,8 @@ Transfer Protocol (FTP) [*]_, whose octets it keeps undissected. Moved here from :mod:`pcapkit.protocols.application.ftp`, which keeps :class:`~pcapkit.protocols.application.ftp.FTP` alone. The control and data channels are dispatched separately -- FTP at TCP port 21, the data channel - at port 20 -- so each is a protocol of its own and gets its own module, - unlike :class:`~pcapkit.protocols.application.rarp.RARP` and - :class:`~pcapkit.protocols.application.rarp.DRARP`, which one dispatch - entry reaches (:issue:`2285`). + at port 20 -- so each is a protocol of its own and gets its own module + (:issue:`2285`). .. autoclass:: pcapkit.protocols.application.ftp_data.FTP_DATA :no-members: diff --git a/docs/source/pcapkit/protocols/application/index.rst b/docs/source/pcapkit/protocols/application/index.rst index 3ffb8fc841..54fd274b2b 100644 --- a/docs/source/pcapkit/protocols/application/index.rst +++ b/docs/source/pcapkit/protocols/application/index.rst @@ -22,6 +22,7 @@ function rather than encapsulation, see ngap ospf rarp + drarp ssl sslv2 tls diff --git a/docs/source/pcapkit/protocols/application/rarp.rst b/docs/source/pcapkit/protocols/application/rarp.rst index 6c3b39a2fe..7ec30535c9 100644 --- a/docs/source/pcapkit/protocols/application/rarp.rst +++ b/docs/source/pcapkit/protocols/application/rarp.rst @@ -1,14 +1,13 @@ -RARP/DRARP - (Dynamic) Reverse Address Resolution Protocol -========================================================== +RARP - Reverse Address Resolution Protocol +========================================== .. module:: pcapkit.protocols.application.rarp :mod:`pcapkit.protocols.application.rarp` contains -:class:`~pcapkit.protocols.application.rarp.RARP` and -:class:`~pcapkit.protocols.application.rarp.DRARP`, -which implement extractors for (Dynamic) Reverse -Address Resolution Protocol (RARP/DRARP) [*]_, -whose structure is described as below: +:class:`~pcapkit.protocols.application.rarp.RARP` only, +which implements an extractor for Reverse Address +Resolution Protocol (RARP) [*]_, whose structure is +described as below: ====== ========= ========================= ========================= Octets Bits Name Description @@ -28,7 +27,7 @@ Octets Bits Name Description Moved here from ``pcapkit.protocols.link.rarp``. :class:`~pcapkit.protocols.application.rarp.RARP` and - :class:`~pcapkit.protocols.application.rarp.DRARP` derive from + :class:`~pcapkit.protocols.application.drarp.DRARP` derive from :class:`~pcapkit.protocols.application.application.Application` as well as :class:`~pcapkit.protocols.link.arp.ARP`, so their layer is ``'Application'`` (:issue:`719`). @@ -38,6 +37,12 @@ Octets Bits Name Description ``pcapkit.protocols.link.rarp``, and the ``RARP`` and ``DRARP`` names that :mod:`pcapkit.protocols.link` exported, with no alias (:issue:`719`). +.. versionremoved:: 1.6.0 + + ``pcapkit.protocols.application.rarp.DRARP``, with no alias. The class + moved to :mod:`pcapkit.protocols.application.drarp`, since a module holds + one protocol class, named after it (:issue:`2529`). + .. autoclass:: pcapkit.protocols.application.rarp.RARP :no-members: :show-inheritance: @@ -46,12 +51,6 @@ Octets Bits Name Description .. automethod:: __index__ -.. autoclass:: pcapkit.protocols.application.rarp.DRARP - :no-members: - :show-inheritance: - - .. automethod:: id - .. rubric:: Footnotes .. [*] http://en.wikipedia.org/wiki/Address_Resolution_Protocol diff --git a/docs/source/pcapkit/protocols/index.rst b/docs/source/pcapkit/protocols/index.rst index 3bbe5f0fe7..dea39428b0 100644 --- a/docs/source/pcapkit/protocols/index.rst +++ b/docs/source/pcapkit/protocols/index.rst @@ -181,7 +181,7 @@ from given information. The class hierarchy of :mod:`pcapkit.protocols`: click NGAP "application/ngap.html#pcapkit.protocols.application.ngap.NGAP" click OSPF "application/ospf.html#pcapkit.protocols.application.ospf.OSPF" click RARP "application/rarp.html#pcapkit.protocols.application.rarp.RARP" - click DRARP "application/rarp.html#pcapkit.protocols.application.rarp.DRARP" + click DRARP "application/drarp.html#pcapkit.protocols.application.drarp.DRARP" click SSL "application/ssl.html#pcapkit.protocols.application.ssl.SSL" click SSLv2 "application/sslv2.html#pcapkit.protocols.application.sslv2.SSLv2" click TLS "application/tls.html#pcapkit.protocols.application.tls.TLS" diff --git a/docs/source/pcapkit/protocols/link/c_tag.rst b/docs/source/pcapkit/protocols/link/c_tag.rst index 54bcdb6eac..c24765a551 100644 --- a/docs/source/pcapkit/protocols/link/c_tag.rst +++ b/docs/source/pcapkit/protocols/link/c_tag.rst @@ -16,9 +16,10 @@ identity -- :attr:`~pcapkit.protocols.link.c_tag.C_Tag.name`, :meth:`~pcapkit.protocols.link.c_tag.C_Tag.id` -- and its registry index. It has a module of its own, apart from -:class:`~pcapkit.protocols.link.s_tag.S_Tag`, because the two are reached through -*different* registry indices, ``0x8100`` against ``0x88A8``; see -:mod:`pcapkit.protocols.link.vlan` for the rule. +:class:`~pcapkit.protocols.link.s_tag.S_Tag`, since a module holds one protocol +class, named after it, with only the framework bases in +:mod:`pcapkit.protocols.protocol` exempt (:issue:`2529`). The two tags are +reached through different registry indices, ``0x8100`` against ``0x88A8``. .. autoclass:: pcapkit.protocols.link.c_tag.C_Tag :no-members: diff --git a/docs/source/pcapkit/protocols/link/l2tp.rst b/docs/source/pcapkit/protocols/link/l2tp.rst index 35fc86c44b..1c240c4845 100644 --- a/docs/source/pcapkit/protocols/link/l2tp.rst +++ b/docs/source/pcapkit/protocols/link/l2tp.rst @@ -45,8 +45,7 @@ directly over IP as **protocol number 115**. That second route is why :attr:`Internet.__proto__ ` leaves 115 unbound: the binding waits on an ``L2TPv3`` class. It would also be the first member of this family with a real -:meth:`~pcapkit.protocols.protocol.Protocol.__index__`, and so the first that -needs a module of its own under the project's one-module-per-index rule. +:meth:`~pcapkit.protocols.protocol.Protocol.__index__`. **L2F** [:rfc:`2341`] is reached when the version nibble reads ``1``. It is *not* an earlier version of L2TP: :rfc:`2661` ยง3.1 requires ``Ver`` to be 2 and reserves diff --git a/docs/source/pcapkit/protocols/link/s_tag.rst b/docs/source/pcapkit/protocols/link/s_tag.rst index 0c3b7d6593..49c6a635b8 100644 --- a/docs/source/pcapkit/protocols/link/s_tag.rst +++ b/docs/source/pcapkit/protocols/link/s_tag.rst @@ -15,9 +15,10 @@ identity -- :attr:`~pcapkit.protocols.link.s_tag.S_Tag.name`, :meth:`~pcapkit.protocols.link.s_tag.S_Tag.id` -- and its registry index. It has a module of its own, apart from -:class:`~pcapkit.protocols.link.c_tag.C_Tag`, because the two are reached through -*different* registry indices, ``0x88A8`` against ``0x8100``; see -:mod:`pcapkit.protocols.link.vlan` for the rule. +:class:`~pcapkit.protocols.link.c_tag.C_Tag`, since a module holds one protocol +class, named after it, with only the framework bases in +:mod:`pcapkit.protocols.protocol` exempt (:issue:`2529`). The two tags are +reached through different registry indices, ``0x88A8`` against ``0x8100``. .. autoclass:: pcapkit.protocols.link.s_tag.S_Tag :no-members: diff --git a/docs/source/pcapkit/protocols/link/vlan.rst b/docs/source/pcapkit/protocols/link/vlan.rst index 74ef356735..23e9c5a110 100644 --- a/docs/source/pcapkit/protocols/link/vlan.rst +++ b/docs/source/pcapkit/protocols/link/vlan.rst @@ -56,13 +56,11 @@ next-EtherType is ``0x8100``, which selects a customer tag in turn: would nest a ``c_tag`` inside a ``c_tag``, leaving nothing to say which was the service tag. -Two EtherTypes also mean two :meth:`~pcapkit.protocols.protocol.Protocol.__index__` -values, which is the project's rule for separate modules: siblings that *share* an -index may share a module, as :class:`~pcapkit.protocols.link.arp.InARP` shares -:mod:`~pcapkit.protocols.link.arp` and -:class:`~pcapkit.protocols.application.rarp.DRARP` shares -:mod:`~pcapkit.protocols.application.rarp`. This base is abstract, nothing -dispatches to it, and it declares no index, so its ``__index__`` raises. +Each tag has a module of its own because a module holds one protocol class, +named after it, with only the framework bases in :mod:`pcapkit.protocols.protocol` +exempt (:issue:`2529`). This base is abstract, nothing dispatches to it, and it +declares no :meth:`~pcapkit.protocols.protocol.Protocol.__index__`, so its +``__index__`` raises. .. autoclass:: pcapkit.protocols.link.vlan.VLAN :no-members: diff --git a/pcapkit/protocols/application/__init__.py b/pcapkit/protocols/application/__init__.py index f24bc233de..a63291800f 100644 --- a/pcapkit/protocols/application/__init__.py +++ b/pcapkit/protocols/application/__init__.py @@ -27,7 +27,8 @@ from pcapkit.protocols.application.httpv2 import HTTP as HTTPv2 from pcapkit.protocols.application.ngap import NGAP from pcapkit.protocols.application.ospf import OSPF -from pcapkit.protocols.application.rarp import RARP, DRARP +from pcapkit.protocols.application.rarp import RARP +from pcapkit.protocols.application.drarp import DRARP from pcapkit.protocols.application.sslv2 import SSLv2 from pcapkit.protocols.application.tls import TLS, SSLv3 from pcapkit.protocols.application.dtls import DTLS diff --git a/pcapkit/protocols/application/drarp.py b/pcapkit/protocols/application/drarp.py new file mode 100644 index 0000000000..6330e7b930 --- /dev/null +++ b/pcapkit/protocols/application/drarp.py @@ -0,0 +1,54 @@ +# -*- coding: utf-8 -*- +"""DRARP - Dynamic Reverse Address Resolution Protocol +========================================================= + +.. module:: pcapkit.protocols.application.drarp + +:mod:`pcapkit.protocols.application.drarp` contains +:class:`~pcapkit.protocols.application.drarp.DRARP` only, +which implements an extractor for Dynamic Reverse Address +Resolution Protocol (DRARP) [*]_, whose frame is the one +:mod:`pcapkit.protocols.application.rarp` describes. + +Nothing dispatches to :class:`~pcapkit.protocols.application.drarp.DRARP`: +under EtherType ``0x8035``, a DRARP operation parses as +:class:`~pcapkit.protocols.application.rarp.RARP` with alias ``DRARP``. + +.. [*] https://datatracker.ietf.org/doc/html/rfc1931 + +.. versionadded:: 1.6.0 + + Moved here from :mod:`pcapkit.protocols.application.rarp`, which keeps + :class:`~pcapkit.protocols.application.rarp.RARP` alone, since a module + holds one protocol class, named after it (:issue:`2529`). + +""" +from typing import TYPE_CHECKING + +from pcapkit.protocols.application.rarp import RARP +from pcapkit.protocols.data.link.arp import ARP as Data_ARP +from pcapkit.protocols.schema.link.arp import ARP as Schema_ARP + +if TYPE_CHECKING: + from typing_extensions import Literal + +__all__ = ['DRARP'] + + +class DRARP(RARP, schema=Schema_ARP, data=Data_ARP): + """This class implements Dynamic Reverse Address Resolution Protocol. + + Inherits RARP's bases unchanged, so ``layer == 'Application'`` here too -- + :rfc:`1931` makes it RARP with dynamic allocation, the same function and so + the same layer. + + """ + + ########################################################################## + # Methods. + ########################################################################## + + @classmethod + def id(cls) -> 'tuple[Literal["DRARP"]]': # type: ignore[override] + """Index ID of the protocol.""" + return ('DRARP',) diff --git a/pcapkit/protocols/application/ftp_data.py b/pcapkit/protocols/application/ftp_data.py index 60ae65bcbe..0f3e599e2b 100644 --- a/pcapkit/protocols/application/ftp_data.py +++ b/pcapkit/protocols/application/ftp_data.py @@ -16,10 +16,8 @@ Moved here from :mod:`pcapkit.protocols.application.ftp`, which keeps :class:`~pcapkit.protocols.application.ftp.FTP` alone. The control and data channels are dispatched separately -- FTP at TCP port 21, the data channel - at port 20 -- so each is a protocol of its own and gets its own module, - unlike :class:`~pcapkit.protocols.application.rarp.RARP` and - :class:`~pcapkit.protocols.application.rarp.DRARP`, which one dispatch - entry reaches (:issue:`2285`). + at port 20 -- so each is a protocol of its own and gets its own module + (:issue:`2285`). """ from typing import TYPE_CHECKING diff --git a/pcapkit/protocols/application/rarp.py b/pcapkit/protocols/application/rarp.py index 37a55136da..50203d5b91 100644 --- a/pcapkit/protocols/application/rarp.py +++ b/pcapkit/protocols/application/rarp.py @@ -1,15 +1,14 @@ # -*- coding: utf-8 -*- -"""RARP/DRARP - (Dynamic) Reverse Address Resolution Protocol -================================================================ +"""RARP - Reverse Address Resolution Protocol +================================================ .. module:: pcapkit.protocols.application.rarp :mod:`pcapkit.protocols.application.rarp` contains -:class:`~pcapkit.protocols.application.rarp.RARP` and -:class:`~pcapkit.protocols.application.rarp.DRARP`, -which implement extractors for (Dynamic) Reverse -Address Resolution Protocol (RARP/DRARP) [*]_, -whose structure is described as below: +:class:`~pcapkit.protocols.application.rarp.RARP` only, +which implements an extractor for Reverse Address +Resolution Protocol (RARP) [*]_, whose structure is +described as below: ====== ========= ========================= ========================= Octets Bits Name Description @@ -31,7 +30,7 @@ Moved here from ``pcapkit.protocols.link.rarp``. :class:`~pcapkit.protocols.application.rarp.RARP` and - :class:`~pcapkit.protocols.application.rarp.DRARP` derive from + :class:`~pcapkit.protocols.application.drarp.DRARP` derive from :class:`~pcapkit.protocols.application.application.Application` as well as :class:`~pcapkit.protocols.link.arp.ARP`, so their layer is ``'Application'`` (:issue:`719`). @@ -41,6 +40,12 @@ ``pcapkit.protocols.link.rarp``, and the ``RARP`` and ``DRARP`` names that :mod:`pcapkit.protocols.link` exported, with no alias (:issue:`719`). +.. versionremoved:: 1.6.0 + + ``pcapkit.protocols.application.rarp.DRARP``, with no alias. The class + moved to :mod:`pcapkit.protocols.application.drarp`, since a module holds + one protocol class, named after it (:issue:`2529`). + """ from typing import TYPE_CHECKING @@ -53,7 +58,7 @@ if TYPE_CHECKING: from typing_extensions import Literal -__all__ = ['RARP', 'DRARP'] +__all__ = ['RARP'] class RARP(Application, ARP, schema=Schema_ARP, data=Data_ARP): # pylint: disable=abstract-method @@ -106,22 +111,3 @@ def __index__(cls) -> 'Enum_EtherType': # pylint: disable=invalid-index-returne """ return Enum_EtherType.Reverse_Address_Resolution_Protocol # type: ignore[return-value] - - -class DRARP(RARP, schema=Schema_ARP, data=Data_ARP): - """This class implements Dynamic Reverse Address Resolution Protocol. - - Inherits RARP's bases unchanged, so ``layer == 'Application'`` here too -- - :rfc:`1931` makes it RARP with dynamic allocation, the same function and so - the same layer. - - """ - - ########################################################################## - # Methods. - ########################################################################## - - @classmethod - def id(cls) -> 'tuple[Literal["DRARP"]]': # type: ignore[override] - """Index ID of the protocol.""" - return ('DRARP',) diff --git a/pcapkit/protocols/link/arp.py b/pcapkit/protocols/link/arp.py index 4d67155c6d..4cd1a5a582 100644 --- a/pcapkit/protocols/link/arp.py +++ b/pcapkit/protocols/link/arp.py @@ -77,7 +77,7 @@ class ARP(Link[Data_ARP, Schema_ARP], - Address Resolution Protocol (:class:`~pcapkit.protocols.link.arp.ARP`) [:rfc:`826`] - Reverse Address Resolution Protocol (:class:`~pcapkit.protocols.application.rarp.RARP`) [:rfc:`903`] - - Dynamic Reverse Address Resolution Protocol (:class:`~pcapkit.protocols.application.rarp.DRARP`) [:rfc:`1931`] + - Dynamic Reverse Address Resolution Protocol (:class:`~pcapkit.protocols.application.drarp.DRARP`) [:rfc:`1931`] - Inverse Address Resolution Protocol (:class:`~pcapkit.protocols.link.arp.InARP`) [:rfc:`2390`] .. versionchanged:: 1.5.0 diff --git a/pcapkit/protocols/link/c_tag.py b/pcapkit/protocols/link/c_tag.py index 0d94745fe1..fcf0842f9e 100644 --- a/pcapkit/protocols/link/c_tag.py +++ b/pcapkit/protocols/link/c_tag.py @@ -12,12 +12,10 @@ identity and its registry index. It lives in a module of its own rather than beside -:class:`~pcapkit.protocols.link.s_tag.S_Tag` because the two are reached through -*different* registry indices -- ``0x8100`` against ``0x88A8`` -- which is the -project's rule for when protocols share a module. Contrast -:class:`~pcapkit.protocols.link.arp.InARP`, which shares -:mod:`~pcapkit.protocols.link.arp` with :class:`~pcapkit.protocols.link.arp.ARP` -precisely because it inherits its index. +:class:`~pcapkit.protocols.link.s_tag.S_Tag`, since a module holds one protocol +class, named after it, with only the framework bases in +:mod:`pcapkit.protocols.protocol` exempt (:issue:`2529`). The two tags are +reached through different registry indices, ``0x8100`` against ``0x88A8``. .. [*] https://en.wikipedia.org/wiki/IEEE_802.1Q diff --git a/pcapkit/protocols/link/l2tp.py b/pcapkit/protocols/link/l2tp.py index b946c78c2e..ffa90c08a6 100644 --- a/pcapkit/protocols/link/l2tp.py +++ b/pcapkit/protocols/link/l2tp.py @@ -176,9 +176,7 @@ def __index__(cls) -> 'NoReturn': # pylint: disable=invalid-index-returned Note: An abstract base is reached by nothing, so it has no index of its - own. That is also the project's rule for module layout -- a distinct - ``__index__`` means a distinct module, and a base carrying none - claims no module of its own beyond holding the family together. + own. """ raise UnsupportedCall(f'{cls.__name__!r} object cannot be interpreted as an integer') diff --git a/pcapkit/protocols/link/s_tag.py b/pcapkit/protocols/link/s_tag.py index 68c17f4dea..7b74e39726 100644 --- a/pcapkit/protocols/link/s_tag.py +++ b/pcapkit/protocols/link/s_tag.py @@ -12,9 +12,10 @@ identity and its registry index. It lives in a module of its own rather than beside -:class:`~pcapkit.protocols.link.c_tag.C_Tag` because the two are reached through -*different* registry indices -- ``0x88A8`` against ``0x8100`` -- which is the -project's rule for when protocols share a module. +:class:`~pcapkit.protocols.link.c_tag.C_Tag`, since a module holds one protocol +class, named after it, with only the framework bases in +:mod:`pcapkit.protocols.protocol` exempt (:issue:`2529`). The two tags are +reached through different registry indices, ``0x88A8`` against ``0x8100``. .. [*] https://en.wikipedia.org/wiki/IEEE_802.1ad diff --git a/pcapkit/protocols/link/vlan.py b/pcapkit/protocols/link/vlan.py index 1787cffc54..c5444fe285 100644 --- a/pcapkit/protocols/link/vlan.py +++ b/pcapkit/protocols/link/vlan.py @@ -7,11 +7,10 @@ :mod:`pcapkit.protocols.link.vlan` contains :class:`~pcapkit.protocols.link.vlan.VLAN` only, an abstract base class holding the tag layout shared by every VLAN tag [*]_. The concrete tags live in -modules of their own, since they are reached through different registry indices --- :class:`~pcapkit.protocols.link.c_tag.C_Tag` for the 802.1Q customer tag -(``0x8100``) and :class:`~pcapkit.protocols.link.s_tag.S_Tag` for the 802.1ad -service tag (``0x88A8``). The tag structure is described as below, keyed as for -a customer tag; a service tag's keys start ``s_tag`` instead: +modules of their own: :class:`~pcapkit.protocols.link.c_tag.C_Tag` for the +802.1Q customer tag (``0x8100``) and :class:`~pcapkit.protocols.link.s_tag.S_Tag` +for the 802.1ad service tag (``0x88A8``). The tag structure is described as +below, keyed as for a customer tag; a service tag's keys start ``s_tag`` instead: ======= ========= ====================== ============================= Octets Bits Name Description @@ -39,14 +38,11 @@ would nest one ``c_tag`` inside another, leaving nothing in the output to say which of the two was the service tag. -Two distinct EtherTypes also mean two distinct -:meth:`~pcapkit.protocols.protocol.Protocol.__index__` values, which is the -project's rule for when protocols get separate modules: siblings that *share* an -index may share a module, as :class:`~pcapkit.protocols.link.arp.InARP` shares -:mod:`~pcapkit.protocols.link.arp` and -:class:`~pcapkit.protocols.application.rarp.DRARP` shares -:mod:`~pcapkit.protocols.application.rarp`. This base declares no index of its own -- -it is abstract and nothing dispatches to it -- so its ``__index__`` raises. +Each tag has a module of its own because a module holds one protocol class, +named after it, with only the framework bases in :mod:`pcapkit.protocols.protocol` +exempt (:issue:`2529`). This base declares no +:meth:`~pcapkit.protocols.protocol.Protocol.__index__` of its own -- it is +abstract and nothing dispatches to it -- so its ``__index__`` raises. .. [*] https://en.wikipedia.org/wiki/IEEE_802.1Q diff --git a/tests/project/test_no_moved_name_re_exports_1744_unit.py b/tests/project/test_no_moved_name_re_exports_1744_unit.py index 475dbcb4ad..7aaf643d4d 100644 --- a/tests/project/test_no_moved_name_re_exports_1744_unit.py +++ b/tests/project/test_no_moved_name_re_exports_1744_unit.py @@ -21,9 +21,10 @@ * :class:`TestNoReExportProse` -- no docstring under :mod:`pcapkit` and no page under ``docs/`` says a removed name is importable, re-exported or available at its old path; -* :class:`TestVersionRemoved` -- the paths that shipped in 1.4.x carry - ``versionremoved:: 1.5.0`` in the module docstring and on the docs page, per - #1103; the beta-only ones are deleted with no directive. +* :class:`TestVersionRemoved` -- the paths that shipped in a release carry + ``versionremoved`` for the version that removed them, in the module docstring + and on the docs page, per #1103; the beta-only ones are deleted with no + directive. This module is unit-tier and reads no capture. @@ -55,6 +56,7 @@ ('pcapkit.foundation.reassembly.data.data', 'DeferredPacket'), ('pcapkit.foundation.traceflow.data.data', 'DeferredPacket'), ('pcapkit.protocols.application.ftp', 'FTP_DATA'), + ('pcapkit.protocols.application.rarp', 'DRARP'), ('pcapkit.protocols.application.ngap', 'ProcedureCode'), ('pcapkit.protocols.application.ngap', 'ProtocolIE'), ('pcapkit.protocols.internet.mh', 'NTPTimestamp'), @@ -73,17 +75,22 @@ #: Every removed ``(old module, name)`` pair. REMOVED = PURE + tuple((module, name) for module, name, _ in USED) -#: The paths that shipped in 1.4.x, as ``(old module, name, source file, docs page)``. +#: The paths that shipped in a release, as ``(old module, name, source file, docs page, +#: the version that removed it)``. SHIPPED = ( ('pcapkit.protocols.misc.pcapng', 'TLSKeyLabel', - 'pcapkit/protocols/misc/pcapng.py', 'pcapkit/protocols/misc/pcapng.rst'), + 'pcapkit/protocols/misc/pcapng.py', 'pcapkit/protocols/misc/pcapng.rst', '1.5.0'), ('pcapkit.corekit.fields.field', 'NoValueType', - 'pcapkit/corekit/fields/field.py', 'pcapkit/corekit/fields/field.rst'), + 'pcapkit/corekit/fields/field.py', 'pcapkit/corekit/fields/field.rst', '1.5.0'), ('pcapkit.protocols.internet.mh', 'NTPTimestamp', - 'pcapkit/protocols/internet/mh.py', 'pcapkit/protocols/internet/mh.rst'), + 'pcapkit/protocols/internet/mh.py', 'pcapkit/protocols/internet/mh.rst', '1.5.0'), ('pcapkit.protocols.application.ftp', 'FTP_DATA', 'pcapkit/protocols/application/ftp.py', - 'pcapkit/protocols/application/ftp.rst'), + 'pcapkit/protocols/application/ftp.rst', '1.5.0'), + # #2529: shipped in 1.5.x, moved to its own module. + ('pcapkit.protocols.application.rarp', 'DRARP', + 'pcapkit/protocols/application/rarp.py', + 'pcapkit/protocols/application/rarp.rst', '1.6.0'), ) @@ -270,18 +277,19 @@ def test_no_docs_page_claims_an_old_path(self) -> 'None': class TestVersionRemoved(unittest.TestCase): - """A path that shipped in 1.4.x is recorded as removed in 1.5.0, twice.""" + """A path that shipped in a release is recorded as removed, twice.""" def test_docstring_and_docs_page_carry_the_directive(self) -> 'None': - for module_name, name, source, page in SHIPPED: + for module_name, name, source, page, version in SHIPPED: old_path = f'``{module_name}.{name}``' module = importlib.import_module(module_name) for label, text in (('docstring', module.__doc__ or ''), ('docs page', (_docs_dir() / page).read_text(encoding='utf-8'))): with self.subTest(path=f'{module_name}.{name}', where=label): - blocks = re.findall(r'(?ms)^\.\. versionremoved:: 1\.5\.0\n\n(.*?)(?=^\S|\Z)', text) + blocks = re.findall(rf'(?ms)^\.\. versionremoved:: {re.escape(version)}\n\n(.*?)(?=^\S|\Z)', + text) self.assertTrue(any(old_path in block for block in blocks), - f'no versionremoved:: 1.5.0 block names {old_path} in {label}') + f'no versionremoved:: {version} block names {old_path} in {label}') self.assertTrue((ROOT / source).is_file()) diff --git a/tests/protocols/application/test_drarp_unit.py b/tests/protocols/application/test_drarp_unit.py new file mode 100644 index 0000000000..b9a9308440 --- /dev/null +++ b/tests/protocols/application/test_drarp_unit.py @@ -0,0 +1,83 @@ +# -*- coding: utf-8 -*- +"""Unit tests for :mod:`pcapkit.protocols.application.drarp`. + +#2529: a module holds one protocol class, named after it, so ``DRARP`` left +:mod:`pcapkit.protocols.application.rarp` for a module of its own, with no +re-export at the old path. Every case builds its own octets. + +""" +from __future__ import annotations + +import importlib +import importlib.util +import unittest + +from tests._support import reimport_once_per_class + +RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) + +#: A DRARP-Request (operation 5) over Ethernet and IPv4, :rfc:`1931`. +DRARP_REQUEST = bytes.fromhex('000108000604' '0005' '001122334455' '0a000001' + '66778899aabb' '0a000002') + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class DRARPModuleTests(unittest.TestCase): + """``DRARP`` lives in its own module and keeps RARP's behaviour.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + def test_drarp_is_defined_in_its_own_module(self) -> None: + from pcapkit.protocols.application.drarp import DRARP + from pcapkit.protocols.application.rarp import RARP + + drarp = importlib.import_module('pcapkit.protocols.application.drarp') + self.assertEqual(DRARP.__module__, 'pcapkit.protocols.application.drarp') + self.assertEqual(drarp.__all__, ['DRARP']) + self.assertEqual(DRARP.__bases__, (RARP,)) + + def test_rarp_module_holds_rarp_alone(self) -> None: + """#2529: no re-export at the old path (#1715, #1744).""" + rarp = importlib.import_module('pcapkit.protocols.application.rarp') + + self.assertEqual(rarp.__all__, ['RARP']) + self.assertFalse(hasattr(rarp, 'DRARP')) + + def test_id_and_index(self) -> None: + """``DRARP`` narrows ``id()`` to itself and inherits RARP's ``__index__``.""" + from pcapkit.const.reg.ethertype import EtherType + from pcapkit.protocols.application.drarp import DRARP + from pcapkit.protocols.application.rarp import RARP + + self.assertEqual(DRARP.id(), ('DRARP',)) + self.assertEqual(RARP.id(), ('RARP', 'DRARP')) + self.assertNotIn('__index__', vars(DRARP)) + self.assertEqual(DRARP.__index__(), EtherType.Reverse_Address_Resolution_Protocol) + + def test_top_level_names_are_the_new_homes_object(self) -> None: + import pcapkit + import pcapkit.protocols + import pcapkit.protocols.application + from pcapkit.protocols.application.drarp import DRARP + + for package in (pcapkit, pcapkit.protocols, pcapkit.protocols.application): + with self.subTest(package=package.__name__): + self.assertIs(package.DRARP, DRARP) + self.assertIn('DRARP', package.__all__) + + def test_parsed_rarp_keeps_its_dynamic_drarp_alias(self) -> None: + """EtherType ``0x8035`` binds RARP alone; operation 5 reports ``DRARP``.""" + from pcapkit.protocols.application.drarp import DRARP + from pcapkit.protocols.application.rarp import RARP + + parsed = RARP(DRARP_REQUEST) + self.assertIs(type(parsed), RARP) + self.assertEqual(parsed.alias, 'DRARP') + self.assertEqual(DRARP(DRARP_REQUEST).alias, 'DRARP') + self.assertEqual(DRARP.from_data(DRARP(DRARP_REQUEST).info).data, DRARP_REQUEST) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/application/test_layer_placement_unit.py b/tests/protocols/application/test_layer_placement_unit.py index d9acd264bc..d34f45142d 100644 --- a/tests/protocols/application/test_layer_placement_unit.py +++ b/tests/protocols/application/test_layer_placement_unit.py @@ -89,8 +89,9 @@ def setUp(self) -> None: ########################################################################## def test_ospf_and_rarp_report_the_application_layer(self) -> None: + from pcapkit.protocols.application.drarp import DRARP from pcapkit.protocols.application.ospf import OSPF - from pcapkit.protocols.application.rarp import DRARP, RARP + from pcapkit.protocols.application.rarp import RARP for cls in (OSPF, RARP, DRARP): with self.subTest(cls=cls.__name__): @@ -118,7 +119,8 @@ def test_ospf_names_application_as_its_only_base(self) -> None: def test_rarp_subclasses_both_application_and_arp_in_that_order(self) -> None: """The order is load-bearing: ``Link`` owns ``__layer__``, ``Raw`` does not.""" from pcapkit.protocols.application.application import Application - from pcapkit.protocols.application.rarp import DRARP, RARP + from pcapkit.protocols.application.drarp import DRARP + from pcapkit.protocols.application.rarp import RARP from pcapkit.protocols.link.arp import ARP from pcapkit.protocols.link.link import Link @@ -176,8 +178,9 @@ def test_link_owns_layer_but_raw_does_not(self) -> None: def test_the_top_level_packages_re_export_the_classes(self) -> None: import pcapkit import pcapkit.protocols + from pcapkit.protocols.application.drarp import DRARP from pcapkit.protocols.application.ospf import OSPF - from pcapkit.protocols.application.rarp import DRARP, RARP + from pcapkit.protocols.application.rarp import RARP for name, cls in (('OSPF', OSPF), ('RARP', RARP), ('DRARP', DRARP)): with self.subTest(name=name): @@ -273,8 +276,9 @@ def test_ospf_has_none_of_links_registry_and_rarp_shares_it(self) -> None: def test_ospf_and_rarp_override_none_of_the_next_layer_hooks(self) -> None: """Both inherit the hooks from ``Application`` rather than re-pointing them.""" from pcapkit.protocols.application.application import Application + from pcapkit.protocols.application.drarp import DRARP from pcapkit.protocols.application.ospf import OSPF - from pcapkit.protocols.application.rarp import DRARP, RARP + from pcapkit.protocols.application.rarp import RARP hooks = ('__post_init__', '_decode_next_layer', '_import_next_layer') for cls in (OSPF, RARP, DRARP): diff --git a/tests/protocols/application/test_rarp_unit.py b/tests/protocols/application/test_rarp_unit.py index d9995ea044..eccc318701 100644 --- a/tests/protocols/application/test_rarp_unit.py +++ b/tests/protocols/application/test_rarp_unit.py @@ -19,10 +19,9 @@ def setUp(self) -> None: def test_rarp_ids_and_index_are_stable(self) -> None: from pcapkit.const.reg.ethertype import EtherType - from pcapkit.protocols.application.rarp import DRARP, RARP + from pcapkit.protocols.application.rarp import RARP self.assertEqual(RARP.id(), ('RARP', 'DRARP')) - self.assertEqual(DRARP.id(), ('DRARP',)) self.assertEqual(RARP.__index__(), EtherType.Reverse_Address_Resolution_Protocol) if __name__ == '__main__': diff --git a/tests/protocols/link/test_arp_roundtrip_unit.py b/tests/protocols/link/test_arp_roundtrip_unit.py index c13deb3e9c..a80562fb5c 100644 --- a/tests/protocols/link/test_arp_roundtrip_unit.py +++ b/tests/protocols/link/test_arp_roundtrip_unit.py @@ -4,7 +4,7 @@ GitHub issues: * #1204: :class:`~pcapkit.protocols.link.arp.InARP` and - :class:`~pcapkit.protocols.application.rarp.DRARP` did not pass + :class:`~pcapkit.protocols.application.drarp.DRARP` did not pass ``schema=``/``data=``, so they fell back to the ``Raw`` schema and could not parse any input. * #1205: an Ethernet hardware address whose ``hlen`` is not 6 was read as @@ -36,7 +36,7 @@ ('pcapkit.protocols.link.arp', 'ARP'), ('pcapkit.protocols.link.arp', 'InARP'), ('pcapkit.protocols.application.rarp', 'RARP'), - ('pcapkit.protocols.application.rarp', 'DRARP'), + ('pcapkit.protocols.application.drarp', 'DRARP'), ) @@ -73,7 +73,7 @@ def test_subclasses_use_arp_schema(self) -> None: def test_subclasses_round_trip(self) -> None: """#1204: the issue's repro parses and rebuilds for InARP and DRARP.""" for module, name, oper in (('pcapkit.protocols.link.arp', 'InARP', '0008'), - ('pcapkit.protocols.application.rarp', 'DRARP', '0005')): + ('pcapkit.protocols.application.drarp', 'DRARP', '0005')): with self.subTest(cls=name): cls = self._cls(module, name) data = bytes.fromhex('000108000604' + oper + '001122334455' '0a000001' diff --git a/tests/protocols/link/test_link_unit.py b/tests/protocols/link/test_link_unit.py index 7b86e6137e..4154760984 100644 --- a/tests/protocols/link/test_link_unit.py +++ b/tests/protocols/link/test_link_unit.py @@ -25,12 +25,11 @@ class LinkProtocolUnitTests(unittest.TestCase): def setUp(self) -> None: reimport_once_per_class(self) - def test_vlan_indices_follow_the_one_module_per_index_rule(self) -> None: + def test_vlan_tags_declare_their_ethertypes_in_modules_of_their_own(self) -> None: """Each tag declares the EtherType it is reached by; the base declares none. - The two indices differing is *why* the tags live in separate modules -- - the project's rule is that a shared index may share a module (as - ``InARP`` shares ``ARP``'s), and a distinct one may not. + Each class is in a module of its own, since a module holds one protocol + class, named after it (#2529). """ from pcapkit.const.reg.ethertype import EtherType @@ -50,7 +49,7 @@ def test_vlan_indices_follow_the_one_module_per_index_rule(self) -> None: with self.assertRaises(UnsupportedCall): VLAN.__index__() - # ... and each tag is in a module of its own, keyed on that difference. + # ... and each class is in a module named after it. self.assertEqual(C_Tag.__module__, 'pcapkit.protocols.link.c_tag') self.assertEqual(S_Tag.__module__, 'pcapkit.protocols.link.s_tag') self.assertEqual(VLAN.__module__, 'pcapkit.protocols.link.vlan') diff --git a/tests/protocols/test_dispatch_bindings_unit.py b/tests/protocols/test_dispatch_bindings_unit.py index 30f555ab31..ea6302f2ae 100644 --- a/tests/protocols/test_dispatch_bindings_unit.py +++ b/tests/protocols/test_dispatch_bindings_unit.py @@ -232,7 +232,7 @@ def test_l2tp_over_ip_waits_on_an_l2tpv3_class(self) -> None: :rfc:`2661` framing :class:`~pcapkit.protocols.link.l2tpv2.L2TPv2` implements, so the binding waits on an ``L2TPv3`` class rather than on a framing decision. 115 is also the first index anything in this family - would carry, which is why v3 gets a module of its own when written. + would carry. """ from pcapkit.const.reg.transtype import TransType diff --git a/tests/protocols/test_dispatch_reachability_unit.py b/tests/protocols/test_dispatch_reachability_unit.py index b140c5cdb0..48d4b879a1 100644 --- a/tests/protocols/test_dispatch_reachability_unit.py +++ b/tests/protocols/test_dispatch_reachability_unit.py @@ -70,6 +70,16 @@ def _resolve(entry: 'Any') -> 'tuple[Any, Any]': return getattr(entry, '__module__', None), getattr(entry, '__name__', None) +def _inherits_index(cls: 'type', module: 'Any', name: 'Any') -> 'bool': + """Whether ``cls`` inherits its ``__index__`` from the class ``module.name``.""" + if module is None or name is None: + return False + target = getattr(importlib.import_module(module), name, None) + if not isinstance(target, type) or not issubclass(cls, target): + return False + return getattr(cls.__index__, '__func__', None) is getattr(target.__index__, '__func__', None) + + def _descendants(cls: 'type') -> 'Any': """Every subclass of ``cls``, transitively.""" for sub in cls.__subclasses__(): @@ -121,13 +131,11 @@ def audit() -> 'tuple[list[Gap], int]': # re-export rather than the defining module -- ``Frame.__proto__`` # holds ``pcapkit.protocols.link:Ethernet``, not # ``pcapkit.protocols.link.ethernet:Ethernet``; - # * the same *module*, because the project's rule is that a shared - # index may share a module -- ``InARP`` shares ``ARP``'s, and - # ``DRARP`` shares ``RARP``'s -- and a distinct one may not, per - # ``test_vlan_indices_follow_the_one_module_per_index_rule``. Both - # classes answer to the code and the module they share is what - # dispatches between them, so the sibling's name is not a mismatch. - if name == cls.__name__ or module == cls.__module__: + # * a class this one inherits its ``__index__`` from, because a + # member sharing its family base's index is parsed by the base and + # reported through its alias -- ``InARP`` by ``ARP``, ``DRARP`` by + # ``RARP`` (#2529). + if name == cls.__name__ or _inherits_index(cls, module, name): checked += 1 continue @@ -155,7 +163,7 @@ def test_every_indexed_class_is_reachable_under_the_code_it_declares(self) -> No # A walk that found nothing to check would also report no gaps, so pin # a floor of 23 claims (measured: the 21 distinct classes plus ``InARP`` - # and ``DRARP`` reaching their siblings' entries, and ``Ethernet`` + # and ``DRARP`` reaching their family bases' entries, and ``Ethernet`` # counted once for ``Frame`` and once for ``PCAPNG``). self.assertGreaterEqual(checked, 23)