refactor/simplify session (#53)

ovos_bus_client/client/client.py

@@ -176,7 +176,7 @@ def emit(self, message: Message):
             sess = SessionManager.sessions.get(self.session_id) or \
                    SessionManager.default_session
             message.context["session"] = sess.serialize()
-            sess.update_history(message)
+
             sess.touch()
 
         if not self.connected_event.wait(10):

ovos_bus_client/session.py

@@ -1,14 +1,14 @@
 import enum
-import json
 import time
 from threading import Lock
 from typing import Optional, List, Tuple, Union, Iterable
 from uuid import uuid4
 
-from ovos_bus_client.message import dig_for_message, Message
 from ovos_config.config import Configuration
 from ovos_config.locale import get_default_lang
-from ovos_utils.log import LOG, log_deprecation
+from ovos_utils.log import LOG
+
+from ovos_bus_client.message import dig_for_message, Message
 
 
 class UtteranceState(str, enum.Enum):
@@ -268,8 +268,7 @@ def get_context(self, max_frames: int = None,
 class Session:
     def __init__(self, session_id: str = None, expiration_seconds: int = None,
                  active_skills: List[List[Union[str, float]]] = None,
-                 history: List[Tuple[Message, float]] = None,
-                 max_time: int = 5, max_messages: int = 5,
+                 history=None, max_time=None, max_messages=None,
                  utterance_states: dict = None, lang: str = None,
                  context: IntentContextManager = None,
                  valid_langs: List[str] = None,
@@ -280,9 +279,9 @@ def __init__(self, session_id: str = None, expiration_seconds: int = None,
         @param session_id: string UUID for the session
         @param expiration_seconds: TTL for session (-1 for no expiration)
         @param active_skills: List of list skill_id, last reference
-        @param history: List of tuple Message, timestamp
-        @param max_time: maximum time for history in minutes
-        @param max_messages: maximum number of messages to retain
+        @param history: DEPRECATED
+        @param max_time: DEPRECATED
+        @param max_messages: DEPRECATED
         @param utterance_states: dict of skill_id to UtteranceState
         @param lang: language associated with this Session
         @param context: IntentContextManager for this Session
@@ -293,12 +292,9 @@ def __init__(self, session_id: str = None, expiration_seconds: int = None,
         self.site_id = site_id or "unknown"  # indoors placement info
 
         self.valid_languages = valid_langs or _get_valid_langs()
-        self.active_skills = active_skills or []  # [skill_id , timestamp]
-        # TODO: Can active_skills be a list of tuples like history?
-        self.history = history or []  # (Message , timestamp)
+        self.active_skills = active_skills or []  # [skill_id , timestamp]# (Message , timestamp)
         self.utterance_states = utterance_states or {}  # {skill_id: UtteranceState}
-        self.max_time = max_time  # minutes
-        self.max_messages = max_messages
+
         self.touch_time = int(time.time())
         self.expiration_seconds = expiration_seconds or \
                                   Configuration().get('session', {}).get("ttl", -1)
@@ -315,6 +311,13 @@ def __init__(self, session_id: str = None, expiration_seconds: int = None,
         ]
         self.context = context or IntentContextManager(timeout=self.touch_time + self.expiration_seconds)
 
+        # deprecated - TODO remove 0.0.8
+        if history is not None or max_time is not None or max_messages is not None:
+            LOG.warning("history, max_time and max_messages have been deprecated")
+        self.history = []  # (Message , timestamp)
+        self.max_time = 5  # minutes
+        self.max_messages = 5
+
     @property
     def active(self) -> bool:
         """
@@ -348,15 +351,15 @@ def enable_response_mode(self, skill_id: str):
         @param skill_id: ID of skill expecting a response
         """
         self.utterance_states[skill_id] = UtteranceState.RESPONSE.value
-        SessionManager.update(self)
+        self.touch()
 
     def disable_response_mode(self, skill_id: str):
         """
         Mark a skill as not expecting a response (handling intents normally)
         @param skill_id: ID of skill expecting a response
         """
         self.utterance_states[skill_id] = UtteranceState.INTENT.value
-        SessionManager.update(self)
+        self.touch()
 
     def activate_skill(self, skill_id: str):
         """
@@ -367,7 +370,7 @@ def activate_skill(self, skill_id: str):
         self.deactivate_skill(skill_id)
         # add skill with timestamp to start of active list
         self.active_skills.insert(0, [skill_id, time.time()])
-        SessionManager.update(self)
+        self.touch()
 
     def deactivate_skill(self, skill_id: str):
         """
@@ -378,37 +381,23 @@ def deactivate_skill(self, skill_id: str):
         if skill_id in active_ids:
             idx = active_ids.index(skill_id)
             self.active_skills.pop(idx)
-        SessionManager.update(self)
+        self.touch()
 
     def is_active(self, skill_id: str) -> bool:
         """
         Check if a skill is active
         @param skill_id: ID of skill to check
         @return: True if the requested skill is active
         """
-        self._prune_history()  # TODO: Is this necessary or useful?
         active_ids = [s[0] for s in self.active_skills]
         return skill_id in active_ids
 
-    def _prune_history(self):
-        """
-        Remove messages from history that are too old or exceed max_messages
-        """
-        # filter old messages from history
-        now = time.time()
-        self.history = [m for m in self.history
-                        if now - m[1] < 60 * self.max_time]
-        # keep only self.max_messages
-        if len(self.history) > self.max_messages:
-            self.history = self.history[self.max_messages * -1:]
-
     def clear(self):
         """
-        Clear history and active_skills
+        Clear active_skills
         """
         self.active_skills = []
-        self.history = []
-        SessionManager.update(self)
+        self.touch()
 
     def serialize(self) -> dict:
         """
@@ -419,7 +408,6 @@ def serialize(self) -> dict:
             "active_skills": self.active_skills,
             "utterance_states": self.utterance_states,
             "session_id": self.session_id,
-            "history": self.history,
             "lang": self.lang,
             "valid_languages": self.valid_languages,
             "context": self.context.serialize(),
@@ -432,23 +420,8 @@ def update_history(self, message: Message = None):
         Add a message to history and then prune history
         @param message: Message to append to history
         """
-        message = message or dig_for_message()
-        if message:
-            try:
-                m = message.as_dict
-            except AttributeError:
-                log_deprecation("mycroft-bus-client has been deprecated, please"
-                                " update your imports to use ovos-bus-client",
-                                "0.0.4",
-                                excluded_package_refs=["ovos_bus_client"])
-                m = json.loads(message.serialize())
-            except Exception as e:
-                LOG.exception(e)
-                m = json.loads(message.serialize())
-            m["context"] = {}  # clear personal data
-            self.history.append((m, time.time()))
-        self._prune_history()
-        SessionManager.update(self)
+        LOG.warning("update_history has been deprecated, "
+                    "session no longer has a message history")
 
     @staticmethod
     def deserialize(data: dict):
@@ -459,9 +432,6 @@ def deserialize(data: dict):
         """
         uid = data.get("session_id")
         active = data.get("active_skills") or []
-        history = data.get("history") or []
-        max_time = data.get("max_time") or 5
-        max_messages = data.get("max_messages") or 5
         states = data.get("utterance_states") or {}
         lang = data.get("lang")
         valid_langs = data.get("valid_languages") or _get_valid_langs()
@@ -471,11 +441,8 @@ def deserialize(data: dict):
         return Session(uid,
                        active_skills=active,
                        utterance_states=states,
-                       history=history,
-                       max_time=max_time,
                        lang=lang,
                        valid_langs=valid_langs,
-                       max_messages=max_messages,
                        context=context,
                        pipeline=pipeline,
                        site_id=site_id)
@@ -569,8 +536,6 @@ def update(sess: Session, make_default: bool = False):
         if make_default:
             sess.session_id = "default"
             LOG.debug(f"replacing default session with: {sess.serialize()}")
-        else:
-            LOG.debug(f"session updated: {sess.session_id}")
 
         if sess.session_id == "default":
             SessionManager.default_session = sess

test/unittests/test_message.py

@@ -39,9 +39,7 @@ def test_serialize_deserialize(self):
     def test_session_serialize_deserialize(self):
         """Assert that a serized message is recreated when deserialized."""
         s = Session()
-        for i in range(3):
-            s.update_history(Message(f'test_type_{i}',
-                                     context={"session": s.serialize()}))
+
         SessionManager.update(s, make_default=True)
 
         source = Message('test_type',

test/unittests/test_session.py

@@ -140,10 +140,7 @@ def test_init(self):
         self.assertIsInstance(session.lang, str)
         self.assertIsInstance(session.valid_languages, list)
         self.assertEqual(session.active_skills, list())
-        self.assertEqual(session.history, list())
         self.assertEqual(session.utterance_states, dict())
-        self.assertIsInstance(session.max_time, int)
-        self.assertIsInstance(session.max_messages, int)
         self.assertIsInstance(session.touch_time, int)
         self.assertIsInstance(session.expiration_seconds, int)
         self.assertIsInstance(session.context, IntentContextManager)
@@ -194,10 +191,6 @@ def test_is_active(self):
         # TODO
         pass
 
-    def test_prune_history(self):
-        # TODO
-        pass
-
     def test_clear(self):
         # TODO
         pass
@@ -225,20 +218,14 @@ def test_serialize_deserialize(self):
         self.assertIsInstance(test_session.lang, str)
         self.assertIsInstance(test_session.valid_languages, list)
         self.assertIsInstance(test_session.active_skills, list)
-        self.assertIsInstance(test_session.history, list)
         self.assertIsInstance(test_session.utterance_states, dict)
-        self.assertIsInstance(test_session.max_time, int)
         self.assertIsInstance(test_session.touch_time, int)
         self.assertIsInstance(test_session.expiration_seconds, int)
         self.assertIsInstance(test_session.context, IntentContextManager)
         serialized = test_session.serialize()
         self.assertIsInstance(serialized, dict)
         self.assertIsInstance(serialized['context'], dict)
 
-    def test_update_history(self):
-        # TODO
-        pass
-
     def test_from_message(self):
         # TODO
         pass