diff --git a/odoo/addons/base/tests/test_db_cursor.py b/odoo/addons/base/tests/test_db_cursor.py index 8492026a7dd..985ff228783 100644 --- a/odoo/addons/base/tests/test_db_cursor.py +++ b/odoo/addons/base/tests/test_db_cursor.py @@ -7,9 +7,10 @@ import psycopg2 from psycopg2.extensions import ISOLATION_LEVEL_REPEATABLE_READ import odoo -from odoo.sql_db import TestCursor +from odoo.sql_db import db_connect, TestCursor from odoo.tests import common from odoo.tests.common import BaseCase +from odoo.tools.misc import config ADMIN_USER_ID = common.ADMIN_USER_ID @@ -151,6 +152,54 @@ class TestTestCursor(common.TransactionCase): # as well as ensure the stack matches our expectations self.assertEqual(a._cursors_stack.pop(), a) + def test_borrow_connection(self): + """Tests the behavior of the postgresql connection pool recycling/borrowing""" + origin_db_port = config['db_port'] + if not origin_db_port and hasattr(self.env.cr._cnx, 'info'): + # Check the edge case of the db port set, + # which is set as an integer in our DSN/connection_info + # but as string in the DSN of psycopg2 + # The connections must be recycled/borrowed when the db_port is set + # e.g + # `connection.dsn` + # {'database': '14.0', 'port': 5432, 'sslmode': 'prefer'} + # must match + # `cr._cnx.dsn` + # 'port=5432 sslmode=prefer dbname=14.0' + config['db_port'] = self.env.cr._cnx.info.port + + cursors = [] + try: + connection = db_connect(self.cr.dbname) + + # Case #1: 2 cursors, both opened/used, do not recycle/borrow. + # The 2nd cursor must not use the connection of the 1st cursor as it's used (not closed). + cursors.append(connection.cursor()) + cursors.append(connection.cursor()) + # Ensure the port is within psycopg's dsn, as explained in an above comment, + # we want to test the behavior of the connections borrowing including the port provided in the dsn. + if config['db_port']: + self.assertTrue('port=' in cursors[0]._cnx.dsn) + # Check the connection of the 1st cursor is different than the connection of the 2nd cursor. + self.assertNotEqual(id(cursors[0]._cnx), id(cursors[1]._cnx)) + + # Case #2: Close 1st cursor, open 3rd cursor, must recycle/borrow. + # The 3rd must recycle/borrow the connection of the 1st one. + cursors[0].close() + cursors.append(connection.cursor()) + # Check the connection of this 3rd cursor uses the connection of the 1st cursor that has been closed. + self.assertEqual(id(cursors[0]._cnx), id(cursors[2]._cnx)) + + finally: + # Cleanups: + # - Close the cursors which have been left opened + # - Reset the config `db_port` + for cursor in cursors: + if not cursor.closed: + cursor.close() + config['db_port'] = origin_db_port + + class TestCursorHooks(common.TransactionCase): def setUp(self): super().setUp() diff --git a/odoo/sql_db.py b/odoo/sql_db.py index 31bf7ba2ec8..ea2feafb6b2 100644 --- a/odoo/sql_db.py +++ b/odoo/sql_db.py @@ -549,6 +549,15 @@ class PsycoConnection(psycopg2.extensions.connection): def lobject(*args, **kwargs): pass + if hasattr(psycopg2.extensions, 'ConnectionInfo'): + @property + def info(self): + class PsycoConnectionInfo(psycopg2.extensions.ConnectionInfo): + @property + def password(self): + pass + return PsycoConnectionInfo(self) + class ConnectionPool(object): """ The pool of connections to database(s) @@ -597,7 +606,7 @@ class ConnectionPool(object): _logger.info('%r: Free leaked connection to %r', self, cnx.dsn) for i, (cnx, used, _) in enumerate(self._connections): - if not used and cnx._original_dsn == connection_info: + if not used and self._dsn_equals(cnx.dsn, connection_info): try: cnx.reset() except psycopg2.OperationalError: @@ -631,7 +640,6 @@ class ConnectionPool(object): except psycopg2.Error: _logger.info('Connection to the database failed') raise - result._original_dsn = connection_info self._connections.append([result, True, 0]) self._debug('Create new connection backend PID %d', result.get_backend_pid()) return result @@ -659,25 +667,48 @@ class ConnectionPool(object): count = 0 last = None for i, (cnx, _, _) in tools.reverse_enumerate(self._connections): - if dsn is None or cnx._original_dsn == dsn: + if dsn is None or self._dsn_equals(cnx.dsn, dsn): cnx.close() last = self._connections.pop(i)[0] count += 1 _logger.info('%r: Closed %d connections %s', self, count, (dsn and last and 'to %r' % last.dsn) or '') + def _dsn_equals(self, dsn1, dsn2): + alias_keys = {'dbname': 'database'} + ignore_keys = ['password'] + dsn1, dsn2 = ({ + alias_keys.get(key, key): str(value) + for key, value in (isinstance(dsn, str) and self._dsn_to_dict(dsn) or dsn).items() + if key not in ignore_keys + } for dsn in (dsn1, dsn2)) + return dsn1 == dsn2 + + def _dsn_to_dict(self, dsn): + return dict(value.split('=', 1) for value in dsn.strip().split()) + class Connection(object): """ A lightweight instance of a connection to postgres """ def __init__(self, pool, dbname, dsn): - self.dbname = dbname - self.dsn = dsn + self.__dbname = dbname + self.__dsn = dsn self.__pool = pool + @property + def dsn(self): + dsn = dict(self.__dsn) + dsn.pop('password', None) + return dsn + + @property + def dbname(self): + return self.__dbname + def cursor(self): _logger.debug('create cursor to %r', self.dsn) - return Cursor(self.__pool, self.dbname, self.dsn) + return Cursor(self.__pool, self.__dbname, self.__dsn) def __bool__(self): raise NotImplementedError()