From ca48be15ab7faed2b1e2387985c823709e49cd57 Mon Sep 17 00:00:00 2001 From: Matthieu Dietrich Date: Thu, 26 Mar 2015 18:54:21 +0100 Subject: [PATCH 1/8] Fix SQL error when an user logs in twice during a transaction --- base_login_date_improvement/__init__.py | 22 +++++ base_login_date_improvement/__openerp__.py | 38 ++++++++ base_login_date_improvement/res_users.py | 108 +++++++++++++++++++++ 3 files changed, 168 insertions(+) create mode 100644 base_login_date_improvement/__init__.py create mode 100644 base_login_date_improvement/__openerp__.py create mode 100644 base_login_date_improvement/res_users.py diff --git a/base_login_date_improvement/__init__.py b/base_login_date_improvement/__init__.py new file mode 100644 index 000000000..364b1d0b3 --- /dev/null +++ b/base_login_date_improvement/__init__.py @@ -0,0 +1,22 @@ +# -*- coding: utf-8 -*- +############################################################################## +# +# Author: Matthieu Dietrich +# Copyright 2015 Camptocamp SA +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +# +############################################################################## + +from . import res_users diff --git a/base_login_date_improvement/__openerp__.py b/base_login_date_improvement/__openerp__.py new file mode 100644 index 000000000..d9b637750 --- /dev/null +++ b/base_login_date_improvement/__openerp__.py @@ -0,0 +1,38 @@ +# -*- coding: utf-8 -*- +############################################################################## +# +# Author: Matthieu Dietrich +# Copyright 2015 Camptocamp SA +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +# +############################################################################## + +{"name": "Base Login Date Improvement", + "version": "1.0", + "author": "Camptocamp,Odoo Community Association (OCA)", + "category": "Specific Module", + "description": """ +Module to separate the login date from res.users; on long transactions, +"re-logging" by opening a new tab changes the current res.user row, +which creates concurrency issues with PostgreSQL in the first transaction. + +This creates a new table and a function field to avoid this. +""", + "website": "http://camptocamp.com", + "depends": ['base'], + "data": [], + "auto_install": False, + "installable": True + } diff --git a/base_login_date_improvement/res_users.py b/base_login_date_improvement/res_users.py new file mode 100644 index 000000000..08117fada --- /dev/null +++ b/base_login_date_improvement/res_users.py @@ -0,0 +1,108 @@ +# -*- coding: utf-8 -*- +############################################################################## +# +# Author: Matthieu Dietrich +# Copyright 2015 Camptocamp SA +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +# +############################################################################## +import logging +import openerp.exceptions +from openerp import pooler, SUPERUSER_ID +from openerp.osv import orm, fields + +_logger = logging.getLogger(__name__) + + +# New class to store the login date +class ResUsersLogin(orm.Model): + + _name = 'res.users.login' + _columns = { + 'user_id': fields.many2one('res.users', 'User', required=True), + 'login_dt': fields.date('Latest connection'), + } + + +class ResUsers(orm.Model): + + _inherit = 'res.users' + + # Function to retrieve the login date from the res.users object + # (used in some functions, and the user state) + def _get_login_date(self, cr, uid, ids, name, args, context=None): + res = {} + user_login_obj = self.pool['res.users.login'] + for user_id in ids: + login_ids = user_login_obj.search( + cr, uid, [('user_id', '=', user_id)], limit=1, + context=context) + if len(login_ids) == 0: + res[user_id] = False + else: + login = user_login_obj.browse(cr, uid, login_ids[0], + context=context) + res[user_id] = login.login_dt + return res + + _columns = { + 'login_date': fields.function(_get_login_date, + string='Latest connection', + type='date', select=1, + readonly=True, store=False), + } + + # Re-defining the login function in order to use the new table + def login(self, db, login, password): + if not password: + return False + user_id = False + cr = pooler.get_db(db).cursor() + try: + cr.autocommit(True) + # check if user exists + res = self.search(cr, SUPERUSER_ID, [('login', '=', login)]) + if res: + user_id = res[0] + # check credentials + self.check_credentials(cr, user_id, password) + try: + update_clause = ('NO KEY UPDATE' + if cr._cnx.server_version >= 90300 + else 'UPDATE') + cr.execute("SELECT login_dt " + "FROM res_users_login " + "WHERE user_id=%%s " + "FOR %s NOWAIT" % update_clause, + (user_id,), log_exceptions=False) + # create login line if not existing + result = cr.fetchone() + if not result: + cr.execute("INSERT INTO res_users_login " + "(user_id) VALUES (%s)", (user_id,)) + cr.execute("UPDATE res_users_login " + "SET login_dt = now() AT TIME ZONE 'UTC' " + "WHERE user_id=%s", (user_id,)) + except Exception: + _logger.debug("Failed to update last_login " + "for db:%s login:%s", + db, login, exc_info=True) + except openerp.exceptions.AccessDenied: + _logger.info("Login failed for db:%s login:%s", db, login) + user_id = False + finally: + cr.close() + + return user_id From d1359b4f712b7d31d5dfa8934807182b5f6b43fb Mon Sep 17 00:00:00 2001 From: Matthieu Dietrich Date: Fri, 27 Mar 2015 09:42:45 +0100 Subject: [PATCH 2/8] Add security rules for new object --- base_login_date_improvement/security/ir.model.access.csv | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 base_login_date_improvement/security/ir.model.access.csv diff --git a/base_login_date_improvement/security/ir.model.access.csv b/base_login_date_improvement/security/ir.model.access.csv new file mode 100644 index 000000000..922391f56 --- /dev/null +++ b/base_login_date_improvement/security/ir.model.access.csv @@ -0,0 +1,2 @@ +id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink +"access_res_users_login_all","res_users_login all","model_res_users_login",,1,0,0,0 From 74d5872c09b98cf1cbef01cd107fbf1a6b25e8be Mon Sep 17 00:00:00 2001 From: Matthieu Dietrich Date: Fri, 27 Mar 2015 13:57:15 +0100 Subject: [PATCH 3/8] Add security file to __openerp__ + nodrop on login_date --- base_login_date_improvement/__openerp__.py | 3 ++- base_login_date_improvement/res_users.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/base_login_date_improvement/__openerp__.py b/base_login_date_improvement/__openerp__.py index d9b637750..860264bd0 100644 --- a/base_login_date_improvement/__openerp__.py +++ b/base_login_date_improvement/__openerp__.py @@ -32,7 +32,8 @@ This creates a new table and a function field to avoid this. """, "website": "http://camptocamp.com", "depends": ['base'], - "data": [], + "data": ['security/ir.model.access.csv', + ], "auto_install": False, "installable": True } diff --git a/base_login_date_improvement/res_users.py b/base_login_date_improvement/res_users.py index 08117fada..393017270 100644 --- a/base_login_date_improvement/res_users.py +++ b/base_login_date_improvement/res_users.py @@ -61,7 +61,8 @@ class ResUsers(orm.Model): 'login_date': fields.function(_get_login_date, string='Latest connection', type='date', select=1, - readonly=True, store=False), + readonly=True, store=False, + nodrop=True), } # Re-defining the login function in order to use the new table From 78ffc1c518ef3d9f92cfbcb44689bd5eec0af05f Mon Sep 17 00:00:00 2001 From: Matthieu Dietrich Date: Fri, 10 Apr 2015 14:53:14 +0200 Subject: [PATCH 4/8] Rework login method --- base_login_date_improvement/res_users.py | 65 ++++++++++++++---------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/base_login_date_improvement/res_users.py b/base_login_date_improvement/res_users.py index 393017270..6cb4ce4e5 100644 --- a/base_login_date_improvement/res_users.py +++ b/base_login_date_improvement/res_users.py @@ -19,6 +19,7 @@ # ############################################################################## import logging +import psycopg2 import openerp.exceptions from openerp import pooler, SUPERUSER_ID from openerp.osv import orm, fields @@ -35,6 +36,12 @@ class ResUsersLogin(orm.Model): 'login_dt': fields.date('Latest connection'), } + _sql_constraints = [ + ('user_id_unique', + 'unique(user_id)', + 'The user can only have one login line !') + ] + class ResUsers(orm.Model): @@ -72,37 +79,43 @@ class ResUsers(orm.Model): user_id = False cr = pooler.get_db(db).cursor() try: - cr.autocommit(True) # check if user exists res = self.search(cr, SUPERUSER_ID, [('login', '=', login)]) if res: user_id = res[0] - # check credentials - self.check_credentials(cr, user_id, password) try: - update_clause = ('NO KEY UPDATE' - if cr._cnx.server_version >= 90300 - else 'UPDATE') - cr.execute("SELECT login_dt " - "FROM res_users_login " - "WHERE user_id=%%s " - "FOR %s NOWAIT" % update_clause, - (user_id,), log_exceptions=False) - # create login line if not existing - result = cr.fetchone() - if not result: - cr.execute("INSERT INTO res_users_login " - "(user_id) VALUES (%s)", (user_id,)) - cr.execute("UPDATE res_users_login " - "SET login_dt = now() AT TIME ZONE 'UTC' " - "WHERE user_id=%s", (user_id,)) - except Exception: - _logger.debug("Failed to update last_login " - "for db:%s login:%s", - db, login, exc_info=True) - except openerp.exceptions.AccessDenied: - _logger.info("Login failed for db:%s login:%s", db, login) - user_id = False + # check credentials + self.check_credentials(cr, user_id, password) + except openerp.exceptions.AccessDenied: + _logger.info("Login failed for db:%s login:%s", db, login) + user_id = False + + if user_id: + try: + update_clause = ('NO KEY UPDATE' + if cr._cnx.server_version >= 90300 + else 'UPDATE') + cr.execute("SELECT login_dt " + "FROM res_users_login " + "WHERE user_id=%%s " + "FOR %s NOWAIT" % update_clause, + (user_id,), log_exceptions=False) + # create login line if not existing + result = cr.fetchone() + if not result: + cr.execute("INSERT INTO res_users_login " + "(user_id) VALUES (%s)", (user_id,)) + cr.execute("UPDATE res_users_login " + "SET login_dt = now() AT TIME ZONE 'UTC' " + "WHERE user_id=%s", (user_id,)) + cr.commit() + except psycopg2.OperationalError: + _logger.warning("Failed to update last_login " + "for db:%s login:%s", + db, login, exc_info=True) + cr.rollback() + except Exception: + pass finally: cr.close() From e66fab88e6a2711efd28563557f42df762f4c1d1 Mon Sep 17 00:00:00 2001 From: Matthieu Dietrich Date: Fri, 10 Apr 2015 15:35:48 +0200 Subject: [PATCH 5/8] Add cron to sync data to old column + improve exception logging --- base_login_date_improvement/__openerp__.py | 6 ++++-- base_login_date_improvement/cron.xml | 24 ++++++++++++++++++++++ base_login_date_improvement/res_users.py | 21 ++++++++++++++++--- 3 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 base_login_date_improvement/cron.xml diff --git a/base_login_date_improvement/__openerp__.py b/base_login_date_improvement/__openerp__.py index 860264bd0..e351f2bdd 100644 --- a/base_login_date_improvement/__openerp__.py +++ b/base_login_date_improvement/__openerp__.py @@ -28,12 +28,14 @@ Module to separate the login date from res.users; on long transactions, "re-logging" by opening a new tab changes the current res.user row, which creates concurrency issues with PostgreSQL in the first transaction. -This creates a new table and a function field to avoid this. +This creates a new table and a function field to avoid this. In order to +avoid breaking modules which access via SQL the login_date column, a cron +(inactive by default) can be used to sync data. """, "website": "http://camptocamp.com", "depends": ['base'], "data": ['security/ir.model.access.csv', - ], + 'cron.xml'], "auto_install": False, "installable": True } diff --git a/base_login_date_improvement/cron.xml b/base_login_date_improvement/cron.xml new file mode 100644 index 000000000..a134f7387 --- /dev/null +++ b/base_login_date_improvement/cron.xml @@ -0,0 +1,24 @@ + + + + + + Synchronize login dates in res.users + 1 + + + 1 + days + -1 + + + + + + + diff --git a/base_login_date_improvement/res_users.py b/base_login_date_improvement/res_users.py index 6cb4ce4e5..603ea9c6e 100644 --- a/base_login_date_improvement/res_users.py +++ b/base_login_date_improvement/res_users.py @@ -39,9 +39,23 @@ class ResUsersLogin(orm.Model): _sql_constraints = [ ('user_id_unique', 'unique(user_id)', - 'The user can only have one login line !') + 'The user can only have one login line!') ] + # Cron method + def cron_sync_login_date(self, cr, uid, context=None): + # Simple SQL query to update the original login_date column. + try: + cr.execute("UPDATE res_users SET login_date = " + "(SELECT login_dt FROM res_users_login " + "WHERE res_users_login.user_id = res_users.id)") + cr.commit() + except Exception as e: + cr.rollback() + _logger.exception('Could not synchronize login dates: %s', e) + + return True + class ResUsers(orm.Model): @@ -114,8 +128,9 @@ class ResUsers(orm.Model): "for db:%s login:%s", db, login, exc_info=True) cr.rollback() - except Exception: - pass + except Exception as e: + _logger.exception('Login exception: %s', e) + user_id = False finally: cr.close() From 727e0a1025fc02123b0730f92c5ac11a8b0babd8 Mon Sep 17 00:00:00 2001 From: Matthieu Dietrich Date: Mon, 13 Apr 2015 14:53:44 +0200 Subject: [PATCH 6/8] Rename module as base_concurrency --- .../__init__.py | 0 .../__openerp__.py | 7 +++++-- {base_login_date_improvement => base_concurrency}/cron.xml | 0 .../res_users.py | 0 .../security/ir.model.access.csv | 0 5 files changed, 5 insertions(+), 2 deletions(-) rename {base_login_date_improvement => base_concurrency}/__init__.py (100%) rename {base_login_date_improvement => base_concurrency}/__openerp__.py (88%) rename {base_login_date_improvement => base_concurrency}/cron.xml (100%) rename {base_login_date_improvement => base_concurrency}/res_users.py (100%) rename {base_login_date_improvement => base_concurrency}/security/ir.model.access.csv (100%) diff --git a/base_login_date_improvement/__init__.py b/base_concurrency/__init__.py similarity index 100% rename from base_login_date_improvement/__init__.py rename to base_concurrency/__init__.py diff --git a/base_login_date_improvement/__openerp__.py b/base_concurrency/__openerp__.py similarity index 88% rename from base_login_date_improvement/__openerp__.py rename to base_concurrency/__openerp__.py index e351f2bdd..90f08f946 100644 --- a/base_login_date_improvement/__openerp__.py +++ b/base_concurrency/__openerp__.py @@ -19,12 +19,15 @@ # ############################################################################## -{"name": "Base Login Date Improvement", +{"name": "Base Concurrency", "version": "1.0", "author": "Camptocamp,Odoo Community Association (OCA)", "category": "Specific Module", "description": """ -Module to separate the login date from res.users; on long transactions, +Module to regroup all workarounds/fixes to avoid concurrency issues in SQL. + +* res.users login_date: +the login date is now separated from res.users; on long transactions, "re-logging" by opening a new tab changes the current res.user row, which creates concurrency issues with PostgreSQL in the first transaction. diff --git a/base_login_date_improvement/cron.xml b/base_concurrency/cron.xml similarity index 100% rename from base_login_date_improvement/cron.xml rename to base_concurrency/cron.xml diff --git a/base_login_date_improvement/res_users.py b/base_concurrency/res_users.py similarity index 100% rename from base_login_date_improvement/res_users.py rename to base_concurrency/res_users.py diff --git a/base_login_date_improvement/security/ir.model.access.csv b/base_concurrency/security/ir.model.access.csv similarity index 100% rename from base_login_date_improvement/security/ir.model.access.csv rename to base_concurrency/security/ir.model.access.csv From dafdd160b27aca1916778b46b4ddb139d4faf072 Mon Sep 17 00:00:00 2001 From: Matthieu Dietrich Date: Mon, 13 Apr 2015 14:54:07 +0200 Subject: [PATCH 7/8] Improve the UPSERT mechanism --- base_concurrency/res_users.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/base_concurrency/res_users.py b/base_concurrency/res_users.py index 603ea9c6e..36ffbcfe6 100644 --- a/base_concurrency/res_users.py +++ b/base_concurrency/res_users.py @@ -106,22 +106,22 @@ class ResUsers(orm.Model): if user_id: try: - update_clause = ('NO KEY UPDATE' - if cr._cnx.server_version >= 90300 - else 'UPDATE') cr.execute("SELECT login_dt " "FROM res_users_login " "WHERE user_id=%%s " - "FOR %s NOWAIT" % update_clause, - (user_id,), log_exceptions=False) + "FOR UPDATE NOWAIT", (user_id,), + log_exceptions=False) # create login line if not existing result = cr.fetchone() - if not result: + if result: + cr.execute("UPDATE res_users_login " + "SET login_dt = now() " + "AT TIME ZONE 'UTC' " + "WHERE user_id=%s", (user_id,)) + else: cr.execute("INSERT INTO res_users_login " - "(user_id) VALUES (%s)", (user_id,)) - cr.execute("UPDATE res_users_login " - "SET login_dt = now() AT TIME ZONE 'UTC' " - "WHERE user_id=%s", (user_id,)) + "(user_id, login_dt) " + "VALUES (%s, now())", (user_id,)) cr.commit() except psycopg2.OperationalError: _logger.warning("Failed to update last_login " From 2c9e53108c674c44985fab35e0846d81b0df00af Mon Sep 17 00:00:00 2001 From: Matthieu Dietrich Date: Mon, 13 Apr 2015 16:28:59 +0200 Subject: [PATCH 8/8] Remove extra %s --- base_concurrency/res_users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base_concurrency/res_users.py b/base_concurrency/res_users.py index 36ffbcfe6..905af8566 100644 --- a/base_concurrency/res_users.py +++ b/base_concurrency/res_users.py @@ -108,7 +108,7 @@ class ResUsers(orm.Model): try: cr.execute("SELECT login_dt " "FROM res_users_login " - "WHERE user_id=%%s " + "WHERE user_id=%s " "FOR UPDATE NOWAIT", (user_id,), log_exceptions=False) # create login line if not existing