From d8c9c8d10b7d61c13eb4ea93f3982923c7c017a4 Mon Sep 17 00:00:00 2001 From: Yannick Tivisse Date: Wed, 8 Mar 2023 10:32:17 +0000 Subject: [PATCH] [FIX] base: Handle portal user deletion separately Purpose ======= Improve the portal user deletion by preventing timeouts and avoiding several rollbacks causes. Specifications ============== - Move the portal users deletions from autovacuum to its own cron. On big databases such as odoo.com, deleting a res.users takes more than 1 minute. Move the whole process into its own scheduled tasks. - Commit the deletion at each user. As said previously the deletion can be expensive, so commit what has been done to avoid having to delete the same user again in case of rollbacks or timeout. - Delete the user and the partner separately. If the partner is used in a sales order for instance, the unlink is possible for the user and not the parner. It allows to unlink the user and keep the partner in case it cannot be deleted. - Re-call the cron into another transaction in case there are too many users to delete, instead of waiting next call, that is supposed to occur the day after. Taskid: 3222941 Part-of: odoo/odoo#114656 --- odoo/addons/base/data/ir_cron_data.xml | 10 +++ odoo/addons/base/models/res_users_deletion.py | 62 ++++++++++++++----- odoo/addons/base/tests/test_res_users.py | 2 +- 3 files changed, 59 insertions(+), 15 deletions(-) diff --git a/odoo/addons/base/data/ir_cron_data.xml b/odoo/addons/base/data/ir_cron_data.xml index fb9cd881dd0..662b6db3607 100644 --- a/odoo/addons/base/data/ir_cron_data.xml +++ b/odoo/addons/base/data/ir_cron_data.xml @@ -9,4 +9,14 @@ days -1 + + + Base: Portal Users Deletion + + code + model._gc_portal_users() + 1 + days + -1 + diff --git a/odoo/addons/base/models/res_users_deletion.py b/odoo/addons/base/models/res_users_deletion.py index 4cccf48502f..f61198b967a 100644 --- a/odoo/addons/base/models/res_users_deletion.py +++ b/odoo/addons/base/models/res_users_deletion.py @@ -2,6 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. import logging +import threading from odoo import api, fields, models @@ -35,8 +36,8 @@ class ResUsersDeletion(models.Model): if user_deletion.user_id: user_deletion.user_id_int = user_deletion.user_id.id - @api.autovacuum - def _gc_portal_users(self): + @api.model + def _gc_portal_users(self, batch_size=10): """Remove the portal users that asked to deactivate their account. (see ::_deactivate_portal_user) @@ -45,22 +46,55 @@ class ResUsersDeletion(models.Model): create_uid, write_uid on each models, which are not always indexed). Because of that, this operation is done in a CRON. """ - delete_requests = self.search([('state', '=', 'todo')]) + delete_requests = self.search([("state", "=", "todo")]) # filter the requests related to a deleted user done_requests = delete_requests.filtered(lambda request: not request.user_id) - done_requests.state = 'done' + done_requests.state = "done" - for delete_request in (delete_requests - done_requests): + todo_requests = delete_requests - done_requests + batch_requests = todo_requests[:batch_size] + + auto_commit = not getattr(threading.current_thread(), "testing", False) + + for delete_request in batch_requests: user = delete_request.user_id user_name = user.name + requester_name = delete_request.create_uid.name + # Step 1: Delete User try: - with self.env.cr.savepoint(): - partner = user.partner_id - user.unlink() - partner.unlink() - _logger.info('User #%i %r, deleted. Original request from %r.', - user.id, user_name, delete_request.create_uid.name) - delete_request.state = 'done' - except Exception: - delete_request.state = 'fail' + self.env.cr.execute("SAVEPOINT delete_user") + partner = user.partner_id + user.unlink() + _logger.info("User #%i %r, deleted. Original request from %r.", + user.id, user_name, delete_request.create_uid.name) + self.env.cr.execute("RELEASE SAVEPOINT delete_user") + delete_request.state = 'done' + except Exception as e: + _logger.error("User #%i %r could not be deleted. Original request from %r. Related error: %s", + user.id, user_name, requester_name, e) + self.env.cr.execute("ROLLBACK TO SAVEPOINT delete_user") + delete_request.state = "fail" + # make sure we never rollback the work we've done, this can take a long time + if auto_commit: + self.env.cr.commit() + if delete_request.state == "fail": + continue + + # Step 2: Delete Linked Partner + # Could be impossible if the partner is linked to a SO for example + try: + self.env.cr.execute("SAVEPOINT delete_partner") + partner.unlink() + _logger.info("Partner #%i %r, deleted. Original request from %r.", + partner.id, user_name, delete_request.create_uid.name) + self.env.cr.execute("RELEASE SAVEPOINT delete_partner") + except Exception as e: + _logger.warning("Partner #%i %r could not be deleted. Original request from %r. Related error: %s", + partner.id, user_name, requester_name, e) + self.env.cr.execute("ROLLBACK TO SAVEPOINT delete_partner") + # make sure we never rollback the work we've done, this can take a long time + if auto_commit: + self.env.cr.commit() + if len(todo_requests) > batch_size: + self.env.ref("base.ir_cron_res_users_deletion")._trigger() diff --git a/odoo/addons/base/tests/test_res_users.py b/odoo/addons/base/tests/test_res_users.py index cd4a472de11..ac6dd21a07d 100644 --- a/odoo/addons/base/tests/test_res_users.py +++ b/odoo/addons/base/tests/test_res_users.py @@ -133,7 +133,7 @@ class TestUsers(TransactionCase): with self.assertRaises(UserError, msg='Internal users should not be able to deactivate their account'): user_internal._deactivate_portal_user() - @mute_logger('odoo.sql_db') + @mute_logger('odoo.sql_db', 'odoo.addons.base.models.res_users_deletion') def test_deactivate_portal_users_archive_and_remove(self): """Test that if the account can not be removed, it's archived instead and sensitive information are removed.