Change session length middleware to not overwriting existing time met…

…adata or redis values

vumi/middleware/session_length.py

@@ -42,37 +42,43 @@ def setup_middleware(self):
     def teardown_middleware(self):
         yield self.redis.close_manager()
 
+    def _generate_redis_key(self, message, address):
+        return '%s:%s:%s' % (
+            message.get('transport_name'), address, 'session_created')
+
+    @inlineCallbacks
+    def _set_redis_time(self, redis_key, time):
+        if (yield self.redis.get(redis_key)) is None:
+            yield self.redis.setex(redis_key,  self.timeout, str(time))
+
     def _set_message_session_metadata(self, message, field, value):
-        if not message['helper_metadata'].get(self.field_name):
-            message['helper_metadata'][self.field_name] = {}
-        message['helper_metadata'][self.field_name][field] = value
+        metadata = message['helper_metadata'].setdefault(self.field_name, {})
+        metadata.setdefault(field, value)
 
     def _set_session_start_time(self, message, time):
+        time = float(time)
         self._set_message_session_metadata(message, 'session_start', time)
 
     def _set_session_end_time(self, message, time):
+        time = float(time)
         self._set_message_session_metadata(message, 'session_end', time)
 
     @inlineCallbacks
     def _process_message(self, message, redis_key):
         if message.get('session_event') == self.SESSION_NEW:
             start_time = self.clock.seconds()
-            yield self.redis.setex(redis_key,  self.timeout, str(start_time))
+            yield self._set_redis_time(redis_key, start_time)
             self._set_session_start_time(message, start_time)
         elif message.get('session_event') == self.SESSION_CLOSE:
             self._set_session_end_time(message, self.clock.seconds())
             created_time = yield self.redis.get(redis_key)
             if created_time:
-                self._set_session_start_time(message, float(created_time))
+                self._set_session_start_time(message, created_time)
                 yield self.redis.delete(redis_key)
         else:
             created_time = yield self.redis.get(redis_key)
             if created_time:
-                self._set_session_start_time(message, float(created_time))
-
-    def _generate_redis_key(self, message, address):
-        return '%s:%s:%s' % (
-            message.get('transport_name'), address, 'session_created')
+                self._set_session_start_time(message, created_time)
 
     @inlineCallbacks
     def handle_inbound(self, message, connector_name):

vumi/middleware/tests/test_session_length.py

@@ -7,8 +7,10 @@
 from vumi.middleware.session_length import SessionLengthMiddleware
 from vumi.tests.helpers import VumiTestCase, PersistenceHelper
 
-SESSION_NEW, SESSION_CLOSE = (
-    TransportUserMessage.SESSION_NEW, TransportUserMessage.SESSION_CLOSE)
+SESSION_NEW, SESSION_CLOSE, SESSION_NONE = (
+    TransportUserMessage.SESSION_NEW,
+    TransportUserMessage.SESSION_CLOSE,
+    TransportUserMessage.SESSION_NONE)
 
 
 class TestStaticProviderSettingMiddleware(VumiTestCase):
@@ -28,14 +30,26 @@ def mk_middleware(self, config={}):
         self.add_cleanup(mw.teardown_middleware)
         returnValue(mw)
 
-    def mk_msg(self, to_addr, from_addr, session_event=SESSION_NEW):
+    def mk_msg(self, to_addr, from_addr, session_event=SESSION_NEW,
+               session_start=None, session_end=None):
         msg = TransportUserMessage(
             to_addr=to_addr, from_addr=from_addr,
             transport_name="dummy_connector",
             transport_type="dummy_transport_type",
             session_event=session_event)
+
+        if session_start is not None:
+            self._set_metadata(msg, 'session_start', session_start)
+
+        if session_end is not None:
+            self._set_metadata(msg, 'session_end', session_end)
+
         return msg
 
+    def _set_metadata(self, msg, name, value, metadata_field_name='session'):
+        metadata = msg['helper_metadata'].setdefault(metadata_field_name, {})
+        metadata[name] = value
+
     @inlineCallbacks
     def test_incoming_message_session_start(self):
         mw = yield self.mk_middleware()
@@ -77,6 +91,59 @@ def test_incoming_message_session_start_end(self):
         self.assertEqual(
             msg['helper_metadata']['session']['session_end'], 1.0)
 
+    @inlineCallbacks
+    def test_incoming_message_session_start_no_overwrite(self):
+        mw = yield self.mk_middleware()
+
+        msg_start = self.mk_msg(
+            '+12345',
+            '+54321',
+            session_start=23,
+            session_event=SESSION_NEW)
+
+        yield self.redis.set('dummy_connector:+12345:session_created', '23')
+
+        msg = yield mw.handle_inbound(msg_start, "dummy_connector")
+        value = yield self.redis.get('dummy_connector:+12345:session_created')
+        self.assertEqual(value, '23')
+
+        self.assertEqual(
+            msg['helper_metadata']['session']['session_start'], 23)
+
+    @inlineCallbacks
+    def test_incoming_message_session_end_no_overwrite(self):
+        mw = yield self.mk_middleware()
+
+        msg_start = self.mk_msg(
+            '+12345',
+            '+54321',
+            session_start=23,
+            session_end=32,
+            session_event=SESSION_CLOSE)
+
+        msg = yield mw.handle_inbound(msg_start, "dummy_connector")
+
+        self.assertEqual(
+            msg['helper_metadata']['session']['session_start'], 23)
+
+        self.assertEqual(
+            msg['helper_metadata']['session']['session_end'], 32)
+
+    @inlineCallbacks
+    def test_incoming_message_session_none_no_overwrite(self):
+        mw = yield self.mk_middleware()
+
+        msg_start = self.mk_msg(
+            '+12345',
+            '+54321',
+            session_start=23,
+            session_event=SESSION_NONE)
+
+        msg = yield mw.handle_inbound(msg_start, "dummy_connector")
+
+        self.assertEqual(
+            msg['helper_metadata']['session']['session_start'], 23)
+
     @inlineCallbacks
     def test_outgoing_message_session_start(self):
         mw = yield self.mk_middleware()
@@ -118,6 +185,59 @@ def test_outgoing_message_session_start_end(self):
         self.assertEqual(
             msg['helper_metadata']['session']['session_end'], 1.0)
 
+    @inlineCallbacks
+    def test_outgoing_message_session_start_no_overwrite(self):
+        mw = yield self.mk_middleware()
+
+        msg_start = self.mk_msg(
+            '+12345',
+            '+54321',
+            session_start=23,
+            session_event=SESSION_NEW)
+
+        yield self.redis.set('dummy_connector:+12345:session_created', '23')
+
+        msg = yield mw.handle_outbound(msg_start, "dummy_connector")
+        value = yield self.redis.get('dummy_connector:+12345:session_created')
+        self.assertEqual(value, '23')
+
+        self.assertEqual(
+            msg['helper_metadata']['session']['session_start'], 23)
+
+    @inlineCallbacks
+    def test_outgoing_message_session_end_no_overwrite(self):
+        mw = yield self.mk_middleware()
+
+        msg_start = self.mk_msg(
+            '+12345',
+            '+54321',
+            session_start=23,
+            session_end=32,
+            session_event=SESSION_CLOSE)
+
+        msg = yield mw.handle_outbound(msg_start, "dummy_connector")
+
+        self.assertEqual(
+            msg['helper_metadata']['session']['session_start'], 23)
+
+        self.assertEqual(
+            msg['helper_metadata']['session']['session_end'], 32)
+
+    @inlineCallbacks
+    def test_outgoing_message_session_none_no_overwrite(self):
+        mw = yield self.mk_middleware()
+
+        msg_start = self.mk_msg(
+            '+12345',
+            '+54321',
+            session_start=23,
+            session_event=SESSION_NONE)
+
+        msg = yield mw.handle_outbound(msg_start, "dummy_connector")
+
+        self.assertEqual(
+            msg['helper_metadata']['session']['session_start'], 23)
+
     @inlineCallbacks
     def test_redis_key_timeout(self):
         mw = yield self.mk_middleware()