diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index 763e6aaf28a..31042561142 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -470,7 +470,7 @@ class AccountMove(models.Model): """ lock_dates = self._get_violated_lock_dates(invoice_date, has_tax) today = fields.Date.today() - highest_name = self.highest_name or self._get_last_sequence(relaxed=True) + highest_name = self.highest_name or self._get_last_sequence(relaxed=True, lock=False) number_reset = self._deduce_sequence_number_reset(highest_name) if lock_dates: invoice_date = lock_dates[-1][0] + timedelta(days=1) @@ -546,7 +546,7 @@ class AccountMove(models.Model): if new_currency != self.currency_id: self.currency_id = new_currency self._onchange_currency() - if self.state == 'draft' and self.name and self.name != '/' and not self.quick_edit_mode and self._get_last_sequence(): + if self.state == 'draft' and self.name and self.name != '/' and not self.quick_edit_mode and self._get_last_sequence(lock=False): self.name = '/' @api.onchange('partner_id') @@ -1318,7 +1318,7 @@ class AccountMove(models.Model): ) ) self = self.sorted(lambda m: (m.date, m.ref or '', m.id)) - highest_name = self[0]._get_last_sequence() if self else False + highest_name = self[0]._get_last_sequence(lock=False) if self else False # Group the moves by journal and month for move in self: @@ -1381,7 +1381,7 @@ class AccountMove(models.Model): @api.depends('journal_id', 'date') def _compute_highest_name(self): for record in self: - record.highest_name = record._get_last_sequence() + record.highest_name = record._get_last_sequence(lock=False) @api.depends('journal_id.type', 'company_id.quick_edit_mode', 'quick_edit_total_amount') def _compute_quick_edit_mode(self): diff --git a/addons/account/models/sequence_mixin.py b/addons/account/models/sequence_mixin.py index 0425f34c6a0..7adf8491cb0 100644 --- a/addons/account/models/sequence_mixin.py +++ b/addons/account/models/sequence_mixin.py @@ -133,7 +133,7 @@ class SequenceMixin(models.AbstractModel): self.ensure_one() return "00000000" - def _get_last_sequence(self, relaxed=False, with_prefix=None): + def _get_last_sequence(self, relaxed=False, with_prefix=None, lock=True): """Retrieve the previous sequence. This is done by taking the number with the greatest alphabetical value within @@ -165,20 +165,22 @@ class SequenceMixin(models.AbstractModel): where_string += " AND sequence_prefix = %(with_prefix)s " param['with_prefix'] = with_prefix - query = """ - UPDATE {table} SET write_date = write_date WHERE id = ( - SELECT id FROM {table} + query = f""" + SELECT {{field}} FROM {self._table} {where_string} - AND sequence_prefix = (SELECT sequence_prefix FROM {table} {where_string} ORDER BY id DESC LIMIT 1) + AND sequence_prefix = (SELECT sequence_prefix FROM {self._table} {where_string} ORDER BY id DESC LIMIT 1) ORDER BY sequence_number DESC LIMIT 1 + """ + if lock: + query = f""" + UPDATE {self._table} SET write_date = write_date WHERE id = ( + {query.format(field='id')} ) - RETURNING {field}; - """.format( - table=self._table, - where_string=where_string, - field=self._sequence_field, - ) + RETURNING {self._sequence_field}; + """ + else: + query = query.format(field=self._sequence_field) self.flush_model([self._sequence_field, 'sequence_number', 'sequence_prefix']) self.env.cr.execute(query, param) diff --git a/addons/account/tests/test_sequence_mixin.py b/addons/account/tests/test_sequence_mixin.py index 714706e47ff..7aa582551b4 100644 --- a/addons/account/tests/test_sequence_mixin.py +++ b/addons/account/tests/test_sequence_mixin.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- from odoo.addons.account.tests.common import AccountTestInvoicingCommon from odoo.tests import tagged -from odoo.tests.common import Form +from odoo.tests.common import Form, TransactionCase from odoo import fields, api, SUPERUSER_ID from odoo.exceptions import ValidationError, UserError from odoo.tools import mute_logger @@ -349,53 +349,6 @@ class TestSequenceMixin(TestSequenceMixinCommon): reset_never = set_sequence(self.test_move.date + relativedelta(years=2), 'MISC/00100') test_date(reset_never.date, 'MISC/00101') # Keep the new prefix in 2018 - def test_sequence_concurency(self): - """Computing the same name in concurent transactions is not allowed.""" - with self.env.registry.cursor() as cr0,\ - self.env.registry.cursor() as cr1,\ - self.env.registry.cursor() as cr2: - env0 = api.Environment(cr0, SUPERUSER_ID, {}) - env1 = api.Environment(cr1, SUPERUSER_ID, {}) - env2 = api.Environment(cr2, SUPERUSER_ID, {}) - - journal = env0['account.journal'].create({ - 'name': 'concurency_test', - 'code': 'CT', - 'type': 'general', - }) - account = env0['account.account'].create({ - 'code': 'CT', - 'name': 'CT', - 'user_type_id': env0.ref('account.data_account_type_fixed_assets').id, - }) - moves = env0['account.move'].create([{ - 'journal_id': journal.id, - 'date': fields.Date.from_string('2016-01-01'), - 'line_ids': [(0, 0, {'name': 'name', 'account_id': account.id})] - }] * 3) - moves.name = '/' - moves[0].action_post() - self.assertEqual(moves.mapped('name'), ['CT/2016/01/0001', '/', '/']) - env0.cr.commit() - - # start the transactions here on cr2 to simulate concurency with cr1 - env2.cr.execute('SELECT 1') - - move = env1['account.move'].browse(moves[1].id) - move.action_post() - env1.cr.commit() - - move = env2['account.move'].browse(moves[2].id) - with self.assertRaises(psycopg2.OperationalError), env2.cr.savepoint(), mute_logger('odoo.sql_db'): - move.action_post() - - self.assertEqual(moves.mapped('name'), ['CT/2016/01/0001', 'CT/2016/01/0002', '/']) - moves.button_draft() - moves.with_context(force_delete=True).unlink() - journal.unlink() - account.unlink() - env0.cr.commit() - def test_resequence_clash(self): """Resequence doesn't clash when it uses a name set in the same batch but that will be overriden later.""" @@ -415,6 +368,20 @@ class TestSequenceMixin(TestSequenceMixinCommon): 'first_name': '2', }).resequence() + @freeze_time('2021-10-01 00:00:00') + def test_change_journal_on_first_account_move(self): + """Changing the journal on the first move is allowed""" + journal = self.env['account.journal'].create({ + 'name': 'awesome journal', + 'type': 'general', + 'code': 'AJ', + }) + move = self.env['account.move'].create({}) + self.assertEqual(move.name, 'MISC/2021/10/0001') + with Form(move) as move_form: + move_form.journal_id = journal + self.assertEqual(move.name, 'AJ/2021/10/0001') + @tagged('post_install', '-at_install') class TestSequenceMixinDeletion(TestSequenceMixinCommon): @@ -476,16 +443,97 @@ class TestSequenceMixinDeletion(TestSequenceMixinCommon): with self.assertRaises(UserError): all_moves.unlink() - @freeze_time('2021-10-01 00:00:00') - def test_change_journal_on_first_account_move(self): - """Changing the journal on the first move is allowed""" - journal = self.env['account.journal'].create({ - 'name': 'awesome journal', - 'type': 'general', - 'code': 'AJ', - }) - move = self.env['account.move'].create({}) - self.assertEqual(move.name, 'MISC/2021/10/0001') - with Form(move) as move_form: - move_form.journal_id = journal - self.assertEqual(move.name, 'AJ/2021/10/0001') + +@tagged('post_install', '-at_install') +class TestSequenceMixinConcurrency(TransactionCase): + def setUp(self): + super().setUp() + with self.env.registry.cursor() as cr: + env = api.Environment(cr, SUPERUSER_ID, {}) + journal = env['account.journal'].create({ + 'name': 'concurency_test', + 'code': 'CT', + 'type': 'general', + }) + account = env['account.account'].create({ + 'code': 'CT', + 'name': 'CT', + 'user_type_id': env.ref('account.data_account_type_fixed_assets').id, + }) + moves = env['account.move'].create([{ + 'journal_id': journal.id, + 'date': fields.Date.from_string('2016-01-01'), + 'line_ids': [(0, 0, {'name': 'name', 'account_id': account.id})] + }] * 3) + moves.name = '/' + moves[0].action_post() + self.assertEqual(moves.mapped('name'), ['CT/2016/01/0001', '/', '/']) + env.cr.commit() + self.data = { + 'move_ids': moves.ids, + 'account_id': account.id, + 'journal_id': journal.id, + 'envs': [ + api.Environment(self.env.registry.cursor(), SUPERUSER_ID, {}), + api.Environment(self.env.registry.cursor(), SUPERUSER_ID, {}), + api.Environment(self.env.registry.cursor(), SUPERUSER_ID, {}), + ], + } + self.addCleanup(self.cleanUp) + + def cleanUp(self): + with self.env.registry.cursor() as cr: + env = api.Environment(cr, SUPERUSER_ID, {}) + moves = env['account.move'].browse(self.data['move_ids']) + moves.button_draft() + moves.posted_before = False + moves.unlink() + journal = env['account.journal'].browse(self.data['journal_id']) + journal.unlink() + account = env['account.account'].browse(self.data['account_id']) + account.unlink() + env.cr.commit() + for env in self.data['envs']: + env.cr.close() + + def test_sequence_concurency(self): + """Computing the same name in concurent transactions is not allowed.""" + env0, env1, env2 = self.data['envs'] + + # start the transactions here on cr1 to simulate concurency with cr2 + env1.cr.execute('SELECT 1') + + # post in cr2 + move = env2['account.move'].browse(self.data['move_ids'][1]) + move.action_post() + env2.cr.commit() + + # try to post in cr1, should fail because this transaction started before the post in cr2 + move = env1['account.move'].browse(self.data['move_ids'][2]) + with self.assertRaises(psycopg2.OperationalError), mute_logger('odoo.sql_db'): + move.action_post() + + # check the values + moves = env0['account.move'].browse(self.data['move_ids']) + self.assertEqual(moves.mapped('name'), ['CT/2016/01/0001', 'CT/2016/01/0002', '/']) + + def test_sequence_concurency_no_useless_lock(self): + """Do not lock needlessly when the sequence is not computed""" + env0, env1, env2 = self.data['envs'] + + # start the transactions here on cr1 to simulate concurency with cr2 + env1.cr.execute('SELECT 1') + + # get the last sequence in cr1 (for instance opening a form view) + move = env2['account.move'].browse(self.data['move_ids'][1]) + move.highest_name + env2.cr.commit() + + # post in cr1, should work even though cr2 read values + move = env1['account.move'].browse(self.data['move_ids'][2]) + move.action_post() + env1.cr.commit() + + # check the values + moves = env0['account.move'].browse(self.data['move_ids']) + self.assertEqual(moves.mapped('name'), ['CT/2016/01/0001', '/', 'CT/2016/01/0002']) diff --git a/addons/l10n_ar/models/account_move.py b/addons/l10n_ar/models/account_move.py index ff748f505d3..ffc5cd45f02 100644 --- a/addons/l10n_ar/models/account_move.py +++ b/addons/l10n_ar/models/account_move.py @@ -240,7 +240,6 @@ class AccountMove(models.Model): return self._get_formatted_sequence() return super()._get_starting_sequence() - def _get_last_sequence_domain(self, relaxed=False): where_string, param = super(AccountMove, self)._get_last_sequence_domain(relaxed) if self.company_id.account_fiscal_country_id.code == "AR" and self.l10n_latam_use_documents: