[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