diff --git a/session_db/pg_session_store.py b/session_db/pg_session_store.py index 849e431de..cf2dd607d 100644 --- a/session_db/pg_session_store.py +++ b/session_db/pg_session_store.py @@ -45,12 +45,16 @@ def with_cursor(func): while True: tries += 1 try: + self._ensure_connection() return func(self, *args, **kwargs) - except (psycopg2.InterfaceError, psycopg2.OperationalError) as e: - _logger.info("Session in DB connection Retry %s/5" % tries) + except (psycopg2.InterfaceError, psycopg2.OperationalError): + self._close_connection() if tries > 4: - raise e - self._open_connection() + _logger.warning( + "session_db operation try %s/5 failed, aborting", tries + ) + raise + _logger.info("session_db operation try %s/5 failed, retrying", tries) return wrapper @@ -64,21 +68,29 @@ class PGSessionStore(sessions.SessionStore): self._setup_db() def __del__(self): - if self._cr is not None: - self._cr.close() + self._close_connection() + + @with_lock + def _ensure_connection(self): + if self._cr is None: + self._open_connection() @with_lock def _open_connection(self): - # return cursor to the pool + self._close_connection() + cnx = odoo.sql_db.db_connect(self._uri, allow_uri=True) + self._cr = cnx.cursor() + self._cr._cnx.autocommit = True + + @with_lock + def _close_connection(self): + """Return cursor to the pool.""" if self._cr is not None: try: self._cr.close() except Exception: # pylint: disable=except-pass pass self._cr = None - cnx = odoo.sql_db.db_connect(self._uri, allow_uri=True) - self._cr = cnx.cursor() - self._cr._cnx.autocommit = True @with_lock @with_cursor diff --git a/session_db/tests/__init__.py b/session_db/tests/__init__.py new file mode 100644 index 000000000..22a56c889 --- /dev/null +++ b/session_db/tests/__init__.py @@ -0,0 +1 @@ +from . import test_pg_session_store diff --git a/session_db/tests/test_pg_session_store.py b/session_db/tests/test_pg_session_store.py new file mode 100644 index 000000000..61e7500ec --- /dev/null +++ b/session_db/tests/test_pg_session_store.py @@ -0,0 +1,82 @@ +from unittest import mock + +import psycopg2 + +from odoo import http +from odoo.sql_db import connection_info_for +from odoo.tests.common import TransactionCase +from odoo.tools import config + +from odoo.addons.session_db.pg_session_store import PGSessionStore + + +def _make_postgres_uri( + login=None, password=None, host=None, port=None, database=None, **kwargs +): + uri = ["postgres://"] + if login: + uri.append(login) + if password: + uri.append(f":{password}") + uri.append("@") + if host: + uri.append(host) + if port: + uri.append(f":{port}") + uri.append("/") + if database: + uri.append(database) + return "".join(uri) + + +class TestPGSessionStore(TransactionCase): + def setUp(self): + super().setUp() + _, connection_info = connection_info_for(config["db_name"]) + self.session_store = PGSessionStore( + _make_postgres_uri(**connection_info), session_class=http.Session + ) + + def test_session_crud(self): + session = self.session_store.new() + session["test"] = "test" + self.session_store.save(session) + assert session.sid is not None + assert self.session_store.get(session.sid)["test"] == "test" + self.session_store.delete(session) + assert self.session_store.get(session.sid).get("test") is None + + def test_retry(self): + """Test that session operations are retried before failing""" + with mock.patch("odoo.sql_db.Cursor.execute") as mock_execute: + mock_execute.side_effect = psycopg2.OperationalError() + try: + self.session_store.get("abc") + except psycopg2.OperationalError: # pylint: disable=except-pass + pass + else: + # We don't use self.assertRaises because Odoo is overriding + # in a way that interferes with the Cursor.execute mock + raise AssertionError("expected psycopg2.OperationalError") + assert mock_execute.call_count == 5 + # when the error is resolved, it works again + self.session_store.get("abc") + + def test_retry_connect_fail(self): + with mock.patch("odoo.sql_db.Cursor.execute") as mock_execute, mock.patch( + "odoo.sql_db.db_connect" + ) as mock_db_connect: + mock_execute.side_effect = psycopg2.OperationalError() + mock_db_connect.side_effect = RuntimeError("connection failed") + # get fails, and a RuntimeError is raised when trying to reconnect + try: + self.session_store.get("abc") + except RuntimeError: # pylint: disable=except-pass + pass + else: + # We don't use self.assertRaises because Odoo is overriding + # in a way that interferes with the Cursor.execute mock + raise AssertionError("expected RuntimeError") + assert mock_execute.call_count == 1 + # when the error is resolved, it works again + self.session_store.get("abc")