Don't remove remote message ids for smpp delivery reports

praekeltfoundation · Oct 19, 2015 · af56be0 · af56be0
1 parent c568eab
commit af56be0
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 10 deletions.
diff --git a/vumi/transports/smpp/config.py b/vumi/transports/smpp/config.py
@@ -37,6 +37,10 @@ class SmppTransportConfig(Transport.CONFIG_CLASS):
         'for matching submit_sm_resp and delivery report messages. Defaults '
         'to 1 week.',
         default=(60 * 60 * 24 * 7), static=True)
+    final_dr_third_party_id_expiry = ConfigInt(
+        'How long (in seconds) to keep 3rd party message IDs around after '
+        'receiving a success or failure delivery report for the message.',
+        default=(60 * 60), static=True)
     completed_multipart_info_expiry = ConfigInt(
         'How long (in seconds) to keep multipart message info for completed '
         'multipart messages around to avoid pending operations accidentally '

diff --git a/vumi/transports/smpp/smpp_transport.py b/vumi/transports/smpp/smpp_transport.py
@@ -240,6 +240,11 @@ def delete_remote_message_id(self, smpp_message_id):
         key = remote_message_key(smpp_message_id)
         return self.redis.delete(key)
 
+    def expire_remote_message_id(self, smpp_message_id):
+        key = remote_message_key(smpp_message_id)
+        expire = self.config.final_dr_third_party_id_expiry
+        return self.redis.expire(key, expire)
+
 
 class SmppTransceiverTransport(Transport):
 
@@ -481,7 +486,7 @@ def handle_delivery_report(self, receipted_message_id, delivery_status):
             delivery_status=delivery_status)
 
         if delivery_status in ('delivered', 'failed'):
-            yield self.message_stash.delete_remote_message_id(
+            yield self.message_stash.expire_remote_message_id(
                 receipted_message_id)
 
         returnValue(dr)

diff --git a/vumi/transports/smpp/tests/test_smpp_transport.py b/vumi/transports/smpp/tests/test_smpp_transport.py
@@ -1514,48 +1514,66 @@ def test_delivery_report_for_unknown_message(self):
 
     @inlineCallbacks
     def test_delivery_report_delivered_delete_stored_remote_id(self):
-        transport = yield self.get_transport()
+        transport = yield self.get_transport({
+            'final_dr_third_party_id_expiry': 23,
+        })
+
         yield transport.message_stash.set_remote_message_id('bar', 'foo')
 
+        self.assertNotEqual(
+            (yield transport.redis.ttl(remote_message_key('foo'))), 23)
+
         pdu = DeliverSM(sequence_number=1, esm_class=4)
         pdu.add_optional_parameter('receipted_message_id', 'foo')
         pdu.add_optional_parameter('message_state', 2)
         yield self.fake_smsc.handle_pdu(pdu)
 
         yield self.tx_helper.wait_for_dispatched_events(1)
 
-        self.assertFalse(
-            (yield transport.redis.exists(remote_message_key('foo'))))
+        self.assertEqual(
+            (yield transport.redis.ttl(remote_message_key('foo'))), 23)
 
     @inlineCallbacks
     def test_delivery_report_failed_delete_stored_remote_id(self):
-        transport = yield self.get_transport()
+        transport = yield self.get_transport({
+            'final_dr_third_party_id_expiry': 23,
+        })
+
         yield transport.message_stash.set_remote_message_id('bar', 'foo')
 
+        self.assertNotEqual(
+            (yield transport.redis.ttl(remote_message_key('foo'))), 23)
+
         pdu = DeliverSM(sequence_number=1, esm_class=4)
         pdu.add_optional_parameter('receipted_message_id', 'foo')
         pdu.add_optional_parameter('message_state', 8)
         yield self.fake_smsc.handle_pdu(pdu)
 
         yield self.tx_helper.wait_for_dispatched_events(1)
 
-        self.assertFalse(
-            (yield transport.redis.exists(remote_message_key('foo'))))
+        self.assertEqual(
+            (yield transport.redis.ttl(remote_message_key('foo'))), 23)
 
     @inlineCallbacks
     def test_delivery_report_pending_keep_stored_remote_id(self):
-        transport = yield self.get_transport()
+        transport = yield self.get_transport({
+            'final_dr_third_party_id_expiry': 23,
+        })
+
         yield transport.message_stash.set_remote_message_id('bar', 'foo')
 
+        self.assertNotEqual(
+            (yield transport.redis.ttl(remote_message_key('foo'))), 23)
+
         pdu = DeliverSM(sequence_number=1, esm_class=4)
         pdu.add_optional_parameter('receipted_message_id', 'foo')
         pdu.add_optional_parameter('message_state', 1)
         yield self.fake_smsc.handle_pdu(pdu)
 
         yield self.tx_helper.wait_for_dispatched_events(1)
 
-        self.assertTrue(
-            (yield transport.redis.exists(remote_message_key('foo'))))
+        self.assertNotEqual(
+            (yield transport.redis.ttl(remote_message_key('foo'))), 23)
 
     @inlineCallbacks
     def test_reconnect(self):