[Asterisk-code-review] channels/pjsip/subscriptions/rls: Refactor RLSTest to inheri... (testsuite[master])
Anonymous Coward
asteriskteam at digium.com
Tue Nov 24 08:26:37 CST 2015
Anonymous Coward #1000019 has submitted this change and it was merged.
Change subject: channels/pjsip/subscriptions/rls: Refactor RLSTest to inherit from VOIPProxy
......................................................................
channels/pjsip/subscriptions/rls: Refactor RLSTest to inherit from VOIPProxy
This patch refactors the RLSTest class to inherit from VOIPProxy. For the most
part, this consists of merely making the class inherit from VOIPProxy as
opposed to VOIPListener. However, a few other modifications were made in
addition to this change:
(1) Printing out a dictionary of an instance of a class prints out a lot more
information than is warranted (such as the class's pydoc strings, etc.)
In reality, all we need to see is the contents of the packet. As such, we
now just print out a string representation of the packet. This defers
the string representation of the instance to the derived classes of
Packet.
(2) Because we spend a lot of time in verification, Asterisk can retransmit a
NOTIFY request. That's perfectly acceptable; however, it breaks the logic
introduced in de4b8fb that erroneously assumes we will not receive
retransmits. Since it is a good idea to make sure that we don't receive
more expected NOTIFY requests than we are checking for, we now extract
the CSeq from the SIPPacket and keep track of the last known CSeq. That
handles the case of re-transmits.
(3) For some subscriptions, Asterisk will transmit NOTIFY requests on shutdown
due to the subscription being terminated. This also breaks the logic
introduced in de4b8fb around expected NOTIFY requests. However, in this
case, there isn't a good way to prevent Asterisk from doing this or even
knowing at that point that we're shutting down. A stop observer on the
test_object will not necessarily be called quickly enough, as Asterisk
can be a tad aggressive on shutting itself down. As such, we merely check
the _stopping variable on the test_object, and refuse to look at packets
after Asterisk starts shutting down.
ASTERISK-25430
Change-Id: I02258ed1e4850d2c249cbf08944e5f5338689ff0
---
M tests/channels/pjsip/subscriptions/rls/rls_test.py
1 file changed, 24 insertions(+), 31 deletions(-)
Approvals:
Anonymous Coward #1000019: Verified
Joshua Colp: Looks good to me, approved
diff --git a/tests/channels/pjsip/subscriptions/rls/rls_test.py b/tests/channels/pjsip/subscriptions/rls/rls_test.py
index 707de37..3364ee2 100755
--- a/tests/channels/pjsip/subscriptions/rls/rls_test.py
+++ b/tests/channels/pjsip/subscriptions/rls/rls_test.py
@@ -14,7 +14,7 @@
sys.path.append("lib/python")
sys.path.append("tests/channels/pjsip/subscriptions/rls")
-from pcap import VOIPListener
+from pcap import VOIPProxy
from rls_element import RLSPacket
from rls_validation import ValidationInfo
from twisted.internet import reactor
@@ -25,7 +25,7 @@
"""Determines if a packet is an RLS multipart packet.
Keyword Arguments:
- packet -- A yappcap.PcapPacket
+ packet -- A SIPPacket
Returns:
True if this is a multipart NOTIFY packet. Otherwise, returns False.
@@ -54,38 +54,13 @@
write_packet_contents -- Whether or not to dump the contents of the
packet to the log.
"""
-
- message = "Received SIP packet:\n"
-
if not write_packet_contents:
return
-
- for attr in dir(packet):
- message += "%s = %s\n" % (attr, getattr(packet, attr))
- if attr == "body":
- message += "body:\n"
- body = getattr(packet, attr)
- for body_attr in dir(body):
- value = getattr(body, body_attr)
- message += "\t%s = %s\n" % (body_attr, value)
- message += "%s = %s\n" % (attr, getattr(packet, attr))
-
+ message = "Processing SIP packet:\n{0}".format(packet)
LOGGER.debug(message)
-def set_pcap_defaults(module_config):
- """Sets default values for the PcapListener."""
- if not module_config.get("bpf-filter"):
- module_config["bpf-filter"] = "udp port 5061"
- if not module_config.get("register-observer"):
- module_config["register-observer"] = True
- if not module_config.get("debug-packets"):
- module_config["debug-packets"] = True
- if not module_config.get("device"):
- module_config["device"] = "lo"
-
-
-class RLSTest(VOIPListener):
+class RLSTest(VOIPProxy):
"""Verifies that SIP notifies contain expected updates.
A test module that observes incoming SIP notifies and compares them
@@ -102,8 +77,6 @@
test_object -- Used to manipulate reactor and set/remove
failure tokens.
"""
-
- set_pcap_defaults(module_config)
super(RLSTest, self).__init__(module_config, test_object)
self.test_object = test_object
@@ -116,6 +89,7 @@
self.ami = None
self.packets_idx = 0
+ self.cseq = None
self.list_name = module_config["list-name"]
self.log_packets = module_config.get("log-packets", False)
self.packets = module_config["packets"]
@@ -184,11 +158,30 @@
packet -- Incoming SIP Packet
"""
+ # We are accessing a pseudo-private member here publicly, which
+ # shouldn't really be done. (Granted, read-only, hence why we can
+ # do this and not break the world.)
+ # We can't register a stop observer, as Asterisk will already be
+ # stopping before our observer is fired, and we need to ignore
+ # any packets that are received once the shutdown sequence starts.
+ # Asterisk will send new NOTIFY requests during shutdown for
+ # Presence types, which will muck up our "too-many packet" checks.
+ if self.test_object._stopping:
+ LOGGER.debug('Test is stopping; ignoring packet')
+ return
+
log_packet(packet, self.log_packets)
if not filter_multipart_packet(packet):
return
+ cseq = int(packet.headers['CSeq'].strip().split()[0])
+ if self.cseq:
+ if cseq <= self.cseq:
+ LOGGER.debug('Out of order or retransmission; dropping')
+ return
+ self.cseq = cseq
+
if self.packets_idx >= len(self.packets):
message = (
"Received more packets ({0}) than expected ({1}). "
--
To view, visit https://gerrit.asterisk.org/1686
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I02258ed1e4850d2c249cbf08944e5f5338689ff0
Gerrit-PatchSet: 3
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
More information about the asterisk-code-review
mailing list