Skip to content

Commit c1c5b9a

Browse files
thomas-manginclaude
thomas-mangin
and
claude
committed
Refactor: Add singleton patterns for Neighbor, IP, and NLRI sentinels
- Add Neighbor.empty() class method for minimal neighbor in decode contexts - Add IP.no_nexthop() class method, remove _NoNextHop class - Add NLRI.invalid() class method for "treat as withdraw" semantics - Update Negotiated to use non-optional Neighbor type - Update callers to use sentinel identity checks - Remove unused type:ignore comments 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 5d26c4e commit c1c5b9a

40 files changed

+210
-177
lines changed

‎src/exabgp/bgp/message/open/capability/negotiated.py‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
if TYPE_CHECKING:
1313
from exabgp.bgp.message import Open
1414
from exabgp.bgp.message.direction import Direction
15+
from exabgp.bgp.neighbor import Neighbor
16+
from exabgp.protocol.ip import IP
1517

1618
from exabgp.bgp.message.open.asn import AS_TRANS, ASN
1719
from exabgp.bgp.message.open.capability.capability import Capability
@@ -26,8 +28,8 @@
2628
class Negotiated:
2729
FREE_SIZE: ClassVar[int] = ExtendedMessage.INITIAL_SIZE - 19 - 2 - 2
2830

29-
def __init__(self, neighbor: Any, direction: 'Direction') -> None:
30-
self.neighbor: Any = neighbor
31+
def __init__(self, neighbor: 'Neighbor', direction: 'Direction') -> None:
32+
self.neighbor: 'Neighbor' = neighbor
3133
self.direction: 'Direction' = direction
3234

3335
self.sent_open: 'Open' | None = None # Open message
@@ -190,7 +192,7 @@ def validate(self, neighbor: Any) -> Tuple[int, int, str] | None:
190192

191193
return None
192194

193-
def nexthopself(self, afi: AFI) -> Any:
195+
def nexthopself(self, afi: AFI) -> 'IP':
194196
return self.neighbor.ip_self(afi)
195197

196198
def required(self, afi: AFI, safi: SAFI) -> bool:

‎src/exabgp/bgp/message/update/__init__.py‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,13 @@ def messages(self, negotiated: Negotiated, include_withdraw: bool = True) -> Gen
126126
continue
127127

128128
add_v4 = add_v4 and nlri.action == Action.ANNOUNCE
129-
add_v4 = add_v4 and nlri.nexthop.afi == AFI.ipv4 # type: ignore[attr-defined]
129+
add_v4 = add_v4 and nlri.nexthop.afi == AFI.ipv4
130130

131131
if add_v4:
132132
nlris.append(nlri)
133133
continue
134134

135-
if nlri.nexthop.afi != AFI.undefined: # type: ignore[attr-defined]
135+
if nlri.nexthop.afi != AFI.undefined:
136136
mp_nlris.setdefault(nlri.family().afi_safi(), {}).setdefault(nlri.action, []).append(nlri)
137137
continue
138138

@@ -276,7 +276,7 @@ def unpack_message(cls, data: bytes, negotiated: Negotiated) -> Update | EOR: #
276276
if length == EOR_IPV4_UNICAST_LENGTH and data == b'\x00\x00\x00\x00':
277277
return EOR(AFI.ipv4, SAFI.unicast) # pylint: disable=E1101
278278
if length == EOR_WITH_PREFIX_LENGTH and data.startswith(EOR.NLRI.PREFIX):
279-
return EOR.unpack_message(data, negotiated) # type: ignore[no-any-return]
279+
return EOR.unpack_message(data, negotiated)
280280

281281
withdrawn, _attributes, announced = cls.split(data)
282282

@@ -312,19 +312,21 @@ def unpack_message(cls, data: bytes, negotiated: Negotiated) -> Update | EOR: #
312312
# negotiated.neighbor may be a mock or not support subscripting
313313
pass
314314

315-
nlris = []
315+
nlris: List[NLRI] = []
316316
while withdrawn:
317-
nlri, left = NLRI.unpack_nlri(AFI.ipv4, SAFI.unicast, withdrawn, Action.WITHDRAW, addpath, negotiated) # type: ignore[misc]
318-
log.debug(lazymsg('withdrawn NLRI {nlri}', nlri=nlri), 'routes') # type: ignore[has-type]
319-
withdrawn = left # type: ignore[has-type]
320-
nlris.append(nlri) # type: ignore[has-type]
317+
nlri, left = NLRI.unpack_nlri(AFI.ipv4, SAFI.unicast, withdrawn, Action.WITHDRAW, addpath, negotiated)
318+
log.debug(lazymsg('withdrawn NLRI {nlri}', nlri=nlri), 'routes')
319+
withdrawn = left
320+
if nlri is not NLRI.invalid():
321+
nlris.append(nlri)
321322

322323
while announced:
323-
nlri, left = NLRI.unpack_nlri(AFI.ipv4, SAFI.unicast, announced, Action.ANNOUNCE, addpath, negotiated) # type: ignore[misc]
324-
nlri.nexthop = nexthop # type: ignore[has-type]
325-
log.debug(lazymsg('announced NLRI {nlri}', nlri=nlri), 'routes') # type: ignore[has-type]
326-
announced = left # type: ignore[has-type]
327-
nlris.append(nlri) # type: ignore[has-type]
324+
nlri, left = NLRI.unpack_nlri(AFI.ipv4, SAFI.unicast, announced, Action.ANNOUNCE, addpath, negotiated)
325+
if nlri is not NLRI.invalid():
326+
nlri.nexthop = nexthop
327+
log.debug(lazymsg('announced NLRI {nlri}', nlri=nlri), 'routes')
328+
nlris.append(nlri)
329+
announced = left
328330

329331
unreach = attributes.pop(MPURNLRI.ID, None)
330332
reach = attributes.pop(MPRNLRI.ID, None)

‎src/exabgp/bgp/message/update/attribute/mprnlri.py‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,15 @@ def packed_attributes(
5858
for nlri in self.nlris:
5959
if nlri.family().afi_safi() != self.family().afi_safi(): # nlri is not part of specified family
6060
continue
61-
if nlri.nexthop is NoNextHop: # type: ignore[attr-defined]
61+
if nlri.nexthop is NoNextHop:
6262
# EOR and Flow may not have any next_hop
6363
nexthop = b''
6464
else:
6565
_, rd_size = Family.size.get(self.family().afi_safi(), (0, 0))
6666
nh_rd = bytes([0]) * rd_size if rd_size else b''
6767
try:
6868
# TODO: remove nlri.afi as it should be in the nexthop already
69-
nexthop = nh_rd + nlri.nexthop.ton(negotiated, nlri.afi) # type: ignore[attr-defined]
69+
nexthop = nh_rd + nlri.nexthop.ton(negotiated, nlri.afi)
7070
except TypeError:
7171
# we could not match "next-hop self" with the BGP AFI of the BGP sesion
7272
# attempting invalid IPv4 next-hop (0.0.0.0) to try to not kill the session
@@ -192,21 +192,21 @@ def unpack_attribute(cls, data: bytes, negotiated: Negotiated) -> MPRNLRI:
192192
while data:
193193
if nexthops:
194194
for nexthop in nexthops:
195-
nlri_result, left_result = NLRI.unpack_nlri(afi, safi, data, Action.ANNOUNCE, addpath, negotiated) # type: ignore[misc]
196-
# allow unpack_nlri to return none for "treat as withdraw" controlled by NLRI.unpack_nlri
197-
if nlri_result: # type: ignore[has-type]
198-
nlri_result.nexthop = NextHop.unpack_attribute(nexthop, negotiated) # type: ignore[has-type]
199-
nlris.append(nlri_result) # type: ignore[has-type]
195+
nlri_result, left_result = NLRI.unpack_nlri(afi, safi, data, Action.ANNOUNCE, addpath, negotiated)
196+
# allow unpack_nlri to return NLRI.invalid() for "treat as withdraw"
197+
if nlri_result is not NLRI.invalid():
198+
nlri_result.nexthop = NextHop.unpack_attribute(nexthop, negotiated)
199+
nlris.append(nlri_result)
200200
else:
201-
nlri_result, left_result = NLRI.unpack_nlri(afi, safi, data, Action.ANNOUNCE, addpath, negotiated) # type: ignore[misc]
202-
# allow unpack_nlri to return none for "treat as withdraw" controlled by NLRI.unpack_nlri
203-
if nlri_result: # type: ignore[has-type]
204-
nlris.append(nlri_result) # type: ignore[has-type]
201+
nlri_result, left_result = NLRI.unpack_nlri(afi, safi, data, Action.ANNOUNCE, addpath, negotiated)
202+
# allow unpack_nlri to return NLRI.invalid() for "treat as withdraw"
203+
if nlri_result is not NLRI.invalid():
204+
nlris.append(nlri_result)
205205

206-
if left_result == data: # type: ignore[has-type]
206+
if left_result == data:
207207
raise RuntimeError('sub-calls should consume data')
208208

209-
data = left_result # type: ignore[has-type]
209+
data = left_result
210210
return cls(afi, safi, nlris)
211211

212212

‎src/exabgp/bgp/message/update/attribute/mpurnlri.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,11 @@ def unpack_attribute(cls, data: bytes, negotiated: Negotiated) -> MPURNLRI:
9191
addpath = negotiated.required(afi, safi)
9292

9393
while data:
94-
nlri_result, data_result = NLRI.unpack_nlri(afi, safi, data, Action.WITHDRAW, addpath, negotiated) # type: ignore[misc]
95-
# allow unpack_nlri to return none for "treat as withdraw" controlled by NLRI.unpack_nlri
96-
if nlri_result: # type: ignore[has-type]
97-
nlris.append(nlri_result) # type: ignore[has-type]
98-
data = data_result # type: ignore[has-type]
94+
nlri_result, data_result = NLRI.unpack_nlri(afi, safi, data, Action.WITHDRAW, addpath, negotiated)
95+
# allow unpack_nlri to return NLRI.invalid() for "treat as withdraw"
96+
if nlri_result is not NLRI.invalid():
97+
nlris.append(nlri_result)
98+
data = data_result
9999

100100
return cls(afi, safi, nlris)
101101

‎src/exabgp/bgp/message/update/attribute/nexthop.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ def pack_attribute(self, negotiated: Negotiated) -> bytes:
7777
return self._attribute(negotiated.nexthopself(self.afi).ton())
7878

7979
def ton(self, negotiated: Negotiated, afi: AFI = AFI.undefined) -> bytes: # type: ignore[override]
80-
return negotiated.nexthopself(afi).ton() # type: ignore[no-any-return]
80+
return negotiated.nexthopself(afi).ton()
8181

8282
def __eq__(self, other: object) -> bool:
8383
raise RuntimeError('do not use __eq__ with NextHop')

‎src/exabgp/bgp/message/update/eor.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
from exabgp.protocol.family import AFI
1616
from exabgp.protocol.family import SAFI
17+
from exabgp.protocol.ip import NoNextHop
1718
from exabgp.bgp.message.message import Message
1819
from exabgp.bgp.message.update.attribute import Attributes
1920
from exabgp.bgp.message.update.nlri import NLRI as _NLRI
@@ -31,7 +32,7 @@ class NLRI(_NLRI):
3132
MP_LENGTH = len(PREFIX) + 1 + 2 # len(AFI) and len(SAFI)
3233
EOR = True
3334

34-
nexthop = None
35+
nexthop = NoNextHop
3536

3637
def __init__(self, afi, safi, action):
3738
_NLRI.__init__(self, afi, safi, action)
@@ -78,6 +79,9 @@ def __repr__(self):
7879
return 'EOR'
7980

8081
@classmethod
81-
def unpack_message(cls, data, negotiated):
82+
def unpack_message(cls, data: bytes, negotiated: 'Negotiated') -> 'EOR':
8283
header_length = len(EOR.NLRI.PREFIX)
83-
return cls(AFI.unpack_afi(data[header_length : header_length + 2]), SAFI.unpack_safi(data[header_length + 2]))
84+
return cls(
85+
AFI.unpack_afi(data[header_length : header_length + 2]),
86+
SAFI.unpack_safi(data[header_length + 2 : header_length + 3]),
87+
)

‎src/exabgp/bgp/message/update/nlri/bgpls/link.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
from exabgp.bgp.message.update.nlri.bgpls.nlri import BGPLS
1818
from exabgp.bgp.message.update.nlri.qualifier.rd import RouteDistinguisher
1919
from exabgp.bgp.message.update.nlri.qualifier.path import PathInfo
20-
from exabgp.protocol.ip import IP, _NoNextHop
20+
from exabgp.protocol.ip import IP, NoNextHop
2121
from exabgp.bgp.message.update.nlri.bgpls.nlri import PROTO_CODES
2222
from exabgp.bgp.message.update.nlri.bgpls.tlvs.linkid import LinkIdentifier
2323
from exabgp.bgp.message.update.nlri.bgpls.tlvs.ifaceaddr import IfaceAddr
@@ -88,7 +88,7 @@ def __init__(
8888
iface_addrs: List[IfaceAddr] | None = None,
8989
topology_ids: List[MTID] | None = None,
9090
link_ids: List[LinkIdentifier] | None = None,
91-
nexthop: IP | _NoNextHop | None = None,
91+
nexthop: IP = NoNextHop,
9292
action: Action = Action.UNSET,
9393
route_d: RouteDistinguisher | None = None,
9494
addpath: PathInfo | None = None,
@@ -103,7 +103,7 @@ def __init__(
103103
self.iface_addrs: List[IfaceAddr] = iface_addrs if iface_addrs else []
104104
self.link_ids: List[LinkIdentifier] = link_ids if link_ids else []
105105
self.topology_ids: List[MTID] = topology_ids if topology_ids else []
106-
self.nexthop: IP | _NoNextHop | None = nexthop
106+
self.nexthop = nexthop
107107
self.route_d: RouteDistinguisher | None = route_d
108108
self._packed: bytes | None = packed # type: ignore[assignment]
109109

‎src/exabgp/bgp/message/update/nlri/bgpls/nlri.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ def register(cls, klass: Type[BGPLS]) -> Type[BGPLS]: # type: ignore[override]
130130
return klass
131131

132132
@classmethod
133-
def unpack_nlri( # type: ignore[override]
133+
def unpack_nlri(
134134
cls: Type[T], afi: AFI, safi: SAFI, bgp: bytes, action: Action, addpath: PathInfo | None, negotiated
135135
) -> Tuple[T, bytes]:
136136
code, length = unpack('!HH', bgp[:4])

‎src/exabgp/bgp/message/update/nlri/bgpls/node.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
from exabgp.bgp.message.update.nlri.bgpls.tlvs.node import NodeDescriptor
2020
from exabgp.bgp.message.update.nlri.qualifier.rd import RouteDistinguisher
2121
from exabgp.bgp.message.update.nlri.qualifier.path import PathInfo
22-
from exabgp.protocol.ip import IP, _NoNextHop
22+
from exabgp.protocol.ip import IP, NoNextHop
2323

2424
# 0 1 2 3
2525
# 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
@@ -56,7 +56,7 @@ def __init__(
5656
proto_id: int,
5757
node_ids: List[NodeDescriptor],
5858
packed: bytes | None = None,
59-
nexthop: IP | _NoNextHop | None = None,
59+
nexthop: IP = NoNextHop,
6060
action: Action = Action.UNSET,
6161
route_d: RouteDistinguisher | None = None,
6262
addpath: PathInfo | None = None,
@@ -65,7 +65,7 @@ def __init__(
6565
self.domain: int = domain
6666
self.proto_id: int = proto_id
6767
self.node_ids: List[NodeDescriptor] = node_ids
68-
self.nexthop: IP | _NoNextHop | None = nexthop
68+
self.nexthop = nexthop
6969
self._pack: bytes | None = packed
7070
self.route_d: RouteDistinguisher | None = route_d
7171

‎src/exabgp/bgp/message/update/nlri/bgpls/prefixv4.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
from exabgp.bgp.message.update.nlri.bgpls.tlvs.ipreach import IpReach
2222
from exabgp.bgp.message.update.nlri.qualifier.rd import RouteDistinguisher
2323
from exabgp.bgp.message.update.nlri.qualifier.path import PathInfo
24-
from exabgp.protocol.ip import IP, _NoNextHop
24+
from exabgp.protocol.ip import IP, NoNextHop
2525
from exabgp.logger import log, lazymsg
2626

2727
# BGP-LS Prefix TLV type codes (RFC 7752)
@@ -60,7 +60,7 @@ def __init__(
6060
packed: bytes | None = None,
6161
ospf_type: OspfRoute | None = None,
6262
prefix: IpReach | None = None,
63-
nexthop: IP | _NoNextHop | None = None,
63+
nexthop: IP = NoNextHop,
6464
route_d: RouteDistinguisher | None = None,
6565
action: Action = Action.UNSET,
6666
addpath: PathInfo | None = None,
@@ -71,7 +71,7 @@ def __init__(
7171
self.proto_id: int = proto_id
7272
self.local_node: List[NodeDescriptor] = local_node
7373
self.prefix: IpReach | None = prefix
74-
self.nexthop: IP | _NoNextHop | None = nexthop
74+
self.nexthop = nexthop
7575
self._pack: bytes | None = packed
7676
self.route_d: RouteDistinguisher | None = route_d
7777

0 commit comments

Comments
 (0)