pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] account: unique constraint in sequence mixin

Browse Source 
Change the way the uniqueness of the sequence numbers is ensured.
Instead of using a `SELECT FOR UPDATE` approach inspired from the
`ir.sequence`, or even a true UPDATE, we can use a constraint approach.

SELECT FOR UPDATE
=================

By doing a FOR UPDATE NO WAIT, we are throwing an exception when another
transaction is creating a move using the same sequence as this one, by
locking the row that holds the current greatest sequence.
Since the row doesn't change, the lock is released and the following
UPDATE is allowed. Because of this, a "useless" UPDATE was always done
on the previous row to ensure a SerializationFailureError if two
concurrent transactions were trying to assign the same number.
This means that in a database with a lot of concurrent transactions
(typically with an online shop), many transactions were
aborted/rollbacked.
This approach also has the drawback of having the constraint implemented
python side, which is less robust.

UNIQUE CONSTRAINT
=================

Using a constraint on the database level means that the database knows
when some transactions will try to do something similar. It will
therefore make the concurrent transactions wait instead of aborting the
transaction.
It is however harder to have the constraint customized by different
modules/localization because it is now done directly in the database and
not in python. Changing the constraint means deleting/recreating the
index. For now, this only happens for Latin America l10n, so a simple
hardcoded approach is implemented.
In order to achieve this, the business code is trying to use the
sequence incrementally until it gets a number it can use for when
multiple transactions are concurrent.

closes odoo/odoo#106325

X-original-commit: 4b430f8e30efb15f06982e4673cd73531a2119b3
Related: odoo/enterprise#34297
Signed-off-by: Quentin De Paoli <qdp@odoo.com>
Signed-off-by: William André (wan) <wan@odoo.com>
This commit is contained in:
william-andre
2022-11-23 16:14:45 +01:00
parent 15beec2324
commit c45998c836
11 changed files with 177 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/account/models/account_move.py
+7 -64
View File
@@ -78,7 +78,7 @@ class AccountMove(models.Model):
compute='_compute_name', readonly=False, store=True,
copy=False,
tracking=True,
index='btree', # We need the btree index for unicity constraint (`_check_unique_sequence_number`)
index='trigram',
)
ref = fields.Char(string='Reference', copy=False, tracking=True)
date = fields.Date(
@@ -549,10 +549,6 @@ class AccountMove(models.Model):
def _auto_init(self):
super()._auto_init()
if self.pool.has_trigram:
# This index is for human searches
sql.create_index(self._cr, 'account_move_name_trigram_index', self._table, ['"name" gin_trgm_ops'], 'gin')
self.env.cr.execute("""
CREATE INDEX IF NOT EXISTS account_move_to_check_idx
ON account_move(journal_id) WHERE to_check = true;
@@ -663,81 +659,29 @@ class AccountMove(models.Model):
@api.depends('posted_before', 'state', 'journal_id', 'date')
def _compute_name(self):
def journal_key(move):
return (move.journal_id, move.journal_id.refund_sequence and move.move_type)
def date_key(move):
return (move.date.year, move.date.month)
grouped = defaultdict( # key: journal_id, move_type
lambda: defaultdict( # key: first adjacent (date.year, date.month)
lambda: {
'records': self.env['account.move'],
'format': False,
'format_values': False,
'reset': False
}
)
)
self = self.sorted(lambda m: (m.date, m.ref or '', m.id))
highest_name = self[0]._get_last_sequence(lock=False) if self else False
# Group the moves by journal and month
for move in self:
if not highest_name and move == self[0] and not move.posted_before and move.date:
# In the form view, we need to compute a default sequence so that the user can edit
# it. We only check the first move as an approximation (enough for new in form view)
pass
move._set_next_sequence()
elif move.quick_edit_mode and not move.posted_before:
# We always suggest the next sequence as the default name of the new move
pass
move._set_next_sequence()
elif (move.name and move.name != '/') or move.state != 'posted':
try:
move._constrains_date_sequence()
# Has already a name or is not posted, we don't add to a batch
continue
# The name matches the date: we don't recompute
except ValidationError:
# Has never been posted and the name doesn't match the date: recompute it
pass
group = grouped[journal_key(move)][date_key(move)]
if not group['records']:
# Compute all the values needed to sequence this whole group
move._set_next_sequence()
else:
# The name is not set yet and it is posted
move._set_next_sequence()
group['format'], group['format_values'] = move._get_sequence_format_param(move.name)
group['reset'] = move._deduce_sequence_number_reset(move.name)
group['records'] += move
# Fusion the groups depending on the sequence reset and the format used because `seq` is
# the same counter for multiple groups that might be spread in multiple months.
final_batches = []
for journal_group in grouped.values():
journal_group_changed = True
for date_group in journal_group.values():
if (
journal_group_changed
or final_batches[-1]['format'] != date_group['format']
or dict(final_batches[-1]['format_values'], seq=0) != dict(date_group['format_values'], seq=0)
):
final_batches += [date_group]
journal_group_changed = False
elif date_group['reset'] == 'never':
final_batches[-1]['records'] += date_group['records']
elif (
date_group['reset'] == 'year'
and final_batches[-1]['records'][0].date.year == date_group['records'][0].date.year
):
final_batches[-1]['records'] += date_group['records']
else:
final_batches += [date_group]
# Give the name based on previously computed values
for batch in final_batches:
for move in batch['records']:
move.name = batch['format'].format(**batch['format_values'])
batch['format_values']['seq'] += 1
self.filtered(lambda m: not m.name).name = '/'
self._compute_split_sequence()
@api.depends('journal_id', 'date')
def _compute_highest_name(self):
@@ -1710,7 +1654,6 @@ class AccountMove(models.Model):
raise UserError(error_msg)
def _get_unbalanced_moves(self, container):
moves = container['records'].filtered(lambda move: move.line_ids)
if not moves:
return
addons/account/tests/test_sequence_mixin.py
+5 -5
View File
@@ -280,7 +280,7 @@ class TestSequenceMixin(TestSequenceMixinCommon):
self.assertEqual(copies[5].name, 'XMISC/2019/00004')
# Can't have twice the same name
with self.assertRaises(ValidationError):
with self.assertRaises(psycopg2.errors.IntegrityError), mute_logger('odoo.sql_db'), self.env.cr.savepoint():
copies[0].name = 'XMISC/2019/00001'
# Lets remove the order by date
@@ -508,14 +508,14 @@ class TestSequenceMixinConcurrency(TransactionCase):
move.action_post()
env2.cr.commit()
# try to post in cr1, should fail because this transaction started before the post in cr2
# try to post in cr1, the retry sould find the right number
move = env1['account.move'].browse(self.data['move_ids'][2])
with self.assertRaises(psycopg2.OperationalError), mute_logger('odoo.sql_db'):
move.action_post()
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', '/'])
self.assertEqual(moves.mapped('name'), ['CT/2016/01/0001', 'CT/2016/01/0002', 'CT/2016/01/0003'])
def test_sequence_concurency_no_useless_lock(self):
"""Do not lock needlessly when the sequence is not computed"""
addons/account_sequence/__init__.py
+1
View File
@@ -0,0 +1 @@
from . import models
addons/account_sequence/__manifest__.py
+16
View File
@@ -0,0 +1,16 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
# Copyright (c) 2011 CCI Connect asbl (http://www.cciconnect.be) All Rights Reserved.
# Philmer <philmer@cciconnect.be>
{
'name': 'Accounting Sequence',
'version': '1.0',
'category': 'Hidden',
'description': "Change the way `sequence.mixin` works to reduce concurrency errors",
'depends': ['account'],
'installable': True,
'auto_install': True,
'license': 'LGPL-3',
}
addons/account_sequence/models/__init__.py
+2
View File
@@ -0,0 +1,2 @@
from . import account_move
from . import sequence_mixin
addons/account_sequence/models/account_move.py
+54
View File
@@ -0,0 +1,54 @@
from odoo import models
from odoo.tools import index_exists
class AccountMove(models.Model):
_inherit = 'account.move'
_sql_constraints = [(
'unique_name', "", "Another entry with the same name already exists.",
)]
def _auto_init(self):
super()._auto_init()
if not index_exists(self.env.cr, 'account_move_unique_name'):
# Make all values of `name` different (naming them `name (1)`, `name (2)`...) so that we can add the following UNIQUE INDEX
self.env.cr.execute("""
WITH duplicated_sequence AS (
SELECT name, journal_id, state
FROM account_move
WHERE state = 'posted'
AND name != '/'
GROUP BY journal_id, name, state
HAVING COUNT(*) > 1
),
to_update AS (
SELECT move.id,
move.name,
move.journal_id,
move.state,
move.date,
row_number() OVER(PARTITION BY move.name, move.journal_id ORDER BY move.name, move.journal_id, move.date) AS row_seq
FROM duplicated_sequence
JOIN account_move move ON move.name = duplicated_sequence.name
AND move.journal_id = duplicated_sequence.journal_id
AND move.state = duplicated_sequence.state
),
new_vals AS (
SELECT id,
name || ' (' || (row_seq-1)::text || ')' AS name
FROM to_update
WHERE row_seq > 1
)
UPDATE account_move
SET name = new_vals.name
FROM new_vals
WHERE account_move.id = new_vals.id;
""")
self.env.cr.execute("""
CREATE UNIQUE INDEX account_move_unique_name
ON account_move(name, journal_id) WHERE (state = 'posted' AND name != '/');
""")
def _check_unique_sequence_number(self):
return
addons/account_sequence/models/sequence_mixin.py
+42
View File
@@ -0,0 +1,42 @@
from psycopg2 import errors
from odoo import models
from odoo.tools import mute_logger
class SequenceMixin(models.AbstractModel):
_inherit = 'sequence.mixin'
def _get_last_sequence(self, relaxed=False, with_prefix=None, lock=True):
return super()._get_last_sequence(relaxed, with_prefix, False)
def _set_next_sequence(self):
# OVERRIDE
self.ensure_one()
last_sequence = self._get_last_sequence()
new = not last_sequence
if new:
last_sequence = self._get_last_sequence(relaxed=True) or self._get_starting_sequence()
format_string, format_values = self._get_sequence_format_param(last_sequence)
if new:
format_values['seq'] = 0
format_values['year'] = self[self._sequence_date_field].year % (10 ** format_values['year_length'])
format_values['month'] = self[self._sequence_date_field].month
# before flushing inside the savepoint (which may be rolled back!), make sure everything
# is already flushed, otherwise we could lose non-sequence fields values, as the ORM believes
# them to be flushed.
self.flush_recordset()
while True:
format_values['seq'] = format_values['seq'] + 1
sequence = format_string.format(**format_values)
try:
with self.env.cr.savepoint(flush=False), mute_logger('odoo.sql_db'):
self[self._sequence_field] = sequence
self.flush_recordset([self._sequence_field])
break
except (errors.ExclusionViolation, errors.UniqueViolation):
pass
self._compute_split_sequence()
self.flush_recordset(['sequence_prefix', 'sequence_number'])
addons/l10n_latam_account_sequence/__init__.py
+1
View File
@@ -0,0 +1 @@
from . import models
addons/l10n_latam_account_sequence/__manifest__.py
+16
View File
@@ -0,0 +1,16 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
# Copyright (c) 2011 CCI Connect asbl (http://www.cciconnect.be) All Rights Reserved.
# Philmer <philmer@cciconnect.be>
{
'name': 'Accounting Sequence - Latam Documents',
'version': '1.0',
'category': 'Hidden',
'description': "Change the way `sequence.mixin` works to reduce concurrency errors",
'depends': ['l10n_latam_invoice_document', 'account_sequence'],
'installable': True,
'auto_install': True,
'license': 'LGPL-3',
}
addons/l10n_latam_account_sequence/models/__init__.py
+1
View File
@@ -0,0 +1 @@
from . import account_move
addons/l10n_latam_account_sequence/models/account_move.py
+32
View File
@@ -0,0 +1,32 @@
from odoo import models
from odoo.tools.sql import index_exists, drop_index
class AccountMove(models.Model):
_inherit = 'account.move'
_sql_constraints = [(
'unique_name', "", "Another entry with the same name already exists.",
), (
'unique_name_latam', "", "Another entry with the same name already exists.",
)]
def _auto_init(self):
super()._auto_init()
# Update the generic unique name constraint to not consider the purchases in latam companies.
# The name should be unique by partner for those documents.
if not index_exists(self.env.cr, "account_move_unique_name_latam"):
drop_index(self.env.cr, "account_move_unique_name", self._table)
self.env.cr.execute("""
CREATE UNIQUE INDEX account_move_unique_name
ON account_move(name, journal_id)
WHERE (state = 'posted' AND name != '/'
AND (l10n_latam_document_type_id IS NULL OR move_type NOT IN ('in_invoice', 'in_refund', 'in_receipt')));
CREATE UNIQUE INDEX account_move_unique_name_latam
ON account_move(name, commercial_partner_id, l10n_latam_document_type_id, company_id)
WHERE (state = 'posted' AND name != '/'
AND (l10n_latam_document_type_id IS NOT NULL AND move_type IN ('in_invoice', 'in_refund', 'in_receipt')));
""")
def _check_unique_vendor_number(self):
return