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

[FIX] mrp: prevent bom cycle

Browse Source 
To reproduce the issue:
1. Create two products A, B
2. Create two BoMs:
   - Product: A
     - Component: B
   - Product: B
     - Component: A
3. On one BoM, open the report "Structure & Cost"

Error: an Odoo Server Error is raised: "[...] maximum recursion
depth exceeded while calling a Python object"

The error occurs because of the cycle between both BoMs. There is
already a check for that in `:MrpBom.explode`:
https://github.com/odoo/odoo/blob/2a73890d304476833d76bac9a36ef92f12f267a3/addons/mrp/models/mrp_bom.py#L329
But it's too late in the flow. We should not be able to configure
such BoMs.

OPW-3200969

closes odoo/odoo#132806

X-original-commit: ba66f6b2ad3ea226779af23cdb4ddf223dd7fe85
Signed-off-by: Adrien Widart (awt) <awt@odoo.com>
This commit is contained in:
Adrien Widart (awt)
2023-08-25 16:00:23 +02:00
parent 397d889805
commit 4cb09b80c8
3 changed files with 243 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mrp/i18n/mrp.pot
+8
View File
@@ -4886,6 +4886,14 @@ msgstr ""
#. module: mrp
#. odoo-python
#: code:addons/mrp/models/mrp_bom.py:0
#, python-format
msgid ""
"The current configuration is incorrect because it would create a cycle "
"between these products: %s."
msgstr ""
#. module: mrp
#: code:addons/mrp/models/stock_orderpoint.py:0
#, python-format
msgid "The following replenishment order has been generated"
addons/mrp/models/mrp_bom.py
+54 -32
View File
@@ -111,16 +111,60 @@ class MrpBom(models.Model):
self.operation_ids.bom_product_template_attribute_value_ids = False
self.byproduct_ids.bom_product_template_attribute_value_ids = False
@api.constrains('active', 'product_id', 'product_tmpl_id', 'bom_line_ids')
def _check_bom_cycle(self):
subcomponents_dict = dict()
def _check_cycle(components, finished_products):
"""
Check whether the components are part of the finished products (-> cycle). Then, if
these components have a BoM, repeat the operation with the subcomponents (recursion).
The method will return the list of product variants that creates the cycle
"""
products_to_find = self.env['product.product']
for component in components:
if component in finished_products:
names = finished_products.mapped('display_name')
raise ValidationError(_("The current configuration is incorrect because it would create a cycle "
"between these products: %s.") % ', '.join(names))
if component not in subcomponents_dict:
products_to_find |= component
bom_find_result = self._bom_find(products_to_find)
for component in components:
if component not in subcomponents_dict:
bom = bom_find_result[component]
subcomponents = bom.bom_line_ids.filtered(lambda l: not l._skip_bom_line(component)).product_id
subcomponents_dict[component] = subcomponents
subcomponents = subcomponents_dict[component]
if subcomponents:
_check_cycle(subcomponents, finished_products | component)
boms_to_check = self
domain = []
for product in self.bom_line_ids.product_id:
domain = OR([domain, self._bom_find_domain(product)])
if domain:
boms_to_check |= self.env['mrp.bom'].search(domain)
for bom in boms_to_check:
if not bom.active:
continue
finished_products = bom.product_id or bom.product_tmpl_id.product_variant_ids
if bom.bom_line_ids.bom_product_template_attribute_value_ids:
grouped_by_components = defaultdict(lambda: self.env['product.product'])
for finished in finished_products:
components = bom.bom_line_ids.filtered(lambda l: not l._skip_bom_line(finished)).product_id
grouped_by_components[components] |= finished
for components, finished in grouped_by_components.items():
_check_cycle(components, finished)
else:
_check_cycle(bom.bom_line_ids.product_id, finished_products)
@api.constrains('product_id', 'product_tmpl_id', 'bom_line_ids', 'byproduct_ids', 'operation_ids')
def _check_bom_lines(self):
for bom in self:
for bom_line in bom.bom_line_ids:
if bom.product_id:
same_product = bom.product_id == bom_line.product_id
else:
same_product = bom.product_tmpl_id == bom_line.product_id.product_tmpl_id
if same_product:
raise ValidationError(_("BoM line product %s should not be the same as BoM product.") % bom.display_name)
apply_variants = bom.bom_line_ids.bom_product_template_attribute_value_ids | bom.operation_ids.bom_product_template_attribute_value_ids | bom.byproduct_ids.bom_product_template_attribute_value_ids
if bom.product_id and apply_variants:
raise ValidationError(_("You cannot use the 'Apply on Variant' functionality and simultaneously create a BoM for a specific variant."))
@@ -130,7 +174,7 @@ class MrpBom(models.Model):
"The attribute value %(attribute)s set on product %(product)s does not match the BoM product %(bom_product)s.",
attribute=ptav.display_name,
product=ptav.product_tmpl_id.display_name,
bom_product=bom_line.parent_product_tmpl_id.display_name
bom_product=bom.product_tmpl_id.display_name
))
for byproduct in bom.byproduct_ids:
if bom.product_id:
@@ -216,6 +260,8 @@ class MrpBom(models.Model):
productions = self.env['mrp.production'].search(domain)
if productions:
productions.is_outdated_bom = True
if 'sequence' in vals and self and self[-1].id == self._prefetch_ids[-1]:
self.browse(self._prefetch_ids)._check_bom_cycle()
return res
def copy(self, default=None):
@@ -327,23 +373,6 @@ class MrpBom(models.Model):
Quantity describes the number of times you need the BoM: so the quantity divided by the number created by the BoM
and converted into its UoM
"""
from collections import defaultdict
graph = defaultdict(list)
V = set()
def check_cycle(v, visited, recStack, graph):
visited[v] = True
recStack[v] = True
for neighbour in graph[v]:
if visited[neighbour] == False:
if check_cycle(neighbour, visited, recStack, graph) == True:
return True
elif recStack[neighbour] == True:
return True
recStack[v] = False
return False
product_ids = set()
product_boms = {}
def update_product_boms():
@@ -356,13 +385,10 @@ class MrpBom(models.Model):
boms_done = [(self, {'qty': quantity, 'product': product, 'original_qty': quantity, 'parent_line': False})]
lines_done = []
V |= set([product.product_tmpl_id.id])
bom_lines = []
for bom_line in self.bom_line_ids:
product_id = bom_line.product_id
V |= set([product_id.product_tmpl_id.id])
graph[product.product_tmpl_id.id].append(product_id.product_tmpl_id.id)
bom_lines.append((bom_line, product, quantity, False))
product_ids.add(product_id.id)
update_product_boms()
@@ -383,10 +409,6 @@ class MrpBom(models.Model):
converted_line_quantity = current_line.product_uom_id._compute_quantity(line_quantity / bom.product_qty, bom.product_uom_id)
bom_lines += [(line, current_line.product_id, converted_line_quantity, current_line) for line in bom.bom_line_ids]
for bom_line in bom.bom_line_ids:
graph[current_line.product_id.product_tmpl_id.id].append(bom_line.product_id.product_tmpl_id.id)
if bom_line.product_id.product_tmpl_id.id in V and check_cycle(bom_line.product_id.product_tmpl_id.id, {key: False for key in V}, {key: False for key in V}, graph):
raise UserError(_('Recursion error! A product with a Bill of Material should not have itself in its BoM or child BoMs!'))
V |= set([bom_line.product_id.product_tmpl_id.id])
if not bom_line.product_id in product_boms:
product_ids.add(bom_line.product_id.id)
boms_done.append((bom, {'qty': converted_line_quantity, 'product': current_product, 'original_qty': quantity, 'parent_line': current_line}))
addons/mrp/tests/test_bom.py
+181 -25
View File
@@ -158,14 +158,12 @@ class TestBoM(TestMrpCommon):
(0, 0, {'name': 'Gift Wrap Maching', 'workcenter_id': self.workcenter_1.id, 'time_cycle': 15, 'sequence': 1}),
],
})
test_bom_1_l1 = self.env['mrp.bom.line'].create({
'bom_id': test_bom_1.id,
test_bom_1.bom_line_ids = [(0, 0, {
'product_id': self.product_3.id,
'product_qty': 3,
})
})]
test_bom_2 = self.env['mrp.bom'].create({
'product_id': self.product_7_3.id,
'product_tmpl_id': self.product_7_template.id,
'product_uom_id': self.uom_unit.id,
'product_qty': 4.0,
@@ -177,28 +175,25 @@ class TestBoM(TestMrpCommon):
(0, 0, {'name': 'Weld Machine', 'workcenter_id': self.workcenter_1.id, 'time_cycle': 18, 'sequence': 2}),
]
})
test_bom_2_l1 = self.env['mrp.bom.line'].create({
'bom_id': test_bom_2.id,
test_bom_2.bom_line_ids = [(0, 0, {
'product_id': self.product_2.id,
'product_qty': 2,
})
test_bom_2_l2 = self.env['mrp.bom.line'].create({
'bom_id': test_bom_2.id,
})]
test_bom_2.bom_line_ids = [(0, 0, {
'product_id': self.product_5.id,
'product_qty': 2,
'bom_product_template_attribute_value_ids': [(4, self.product_7_attr1_v1.id)],
})
test_bom_2_l3 = self.env['mrp.bom.line'].create({
'bom_id': test_bom_2.id,
})]
test_bom_2.bom_line_ids = [(0, 0, {
'product_id': self.product_5.id,
'product_qty': 2,
'bom_product_template_attribute_value_ids': [(4, self.product_7_attr1_v2.id)],
})
test_bom_2_l4 = self.env['mrp.bom.line'].create({
'bom_id': test_bom_2.id,
})]
test_bom_2.bom_line_ids = [(0, 0, {
'product_id': self.product_4.id,
'product_qty': 2,
})
})]
test_bom_2_l1, _test_bom_2_l2, _test_bom_2_l3, test_bom_2_l4 = test_bom_2.bom_line_ids
# check product > product_tmpl
boms, lines = test_bom_2.explode(self.product_7_1, 4)
@@ -242,18 +237,15 @@ class TestBoM(TestMrpCommon):
'consumption': 'flexible',
'type': 'phantom'
})
test_bom_3_l1 = self.env['mrp.bom.line'].create({
'bom_id': test_bom_3.id,
test_bom_3.bom_line_ids = [(0, 0, {
'product_id': self.product_10.id,
'product_qty': 1.0,
})
test_bom_4_l1 = self.env['mrp.bom.line'].create({
'bom_id': test_bom_4.id,
'product_id': self.product_9.id,
'product_qty': 1.0,
})
})]
with self.assertRaises(exceptions.UserError):
test_bom_3.explode(self.product_9, 1)
test_bom_4.bom_line_ids = [(0, 0, {
'product_id': self.product_9.id,
'product_qty': 1.0,
})]
def test_12_multi_level_variants2(self):
"""Test skip bom line with same attribute values in bom lines."""
@@ -1688,3 +1680,167 @@ class TestBoM(TestMrpCommon):
mo.button_plan()
mo.button_mark_done()
self.assertEqual(mo.state, 'done')
def test_cycle_on_line_creation(self):
bom_1_finished_product = self.bom_1.product_id
bom_2_finished_product = self.bom_2.product_id
with self.assertRaises(exceptions.ValidationError):
# finished product is one of the components:
self.bom_1.bom_line_ids = [(0, 0, {'product_id': bom_1_finished_product.id, 'product_qty': 1.0},)]
with self.assertRaises(exceptions.ValidationError):
# cycle:
self.bom_1.bom_line_ids = [(0, 0, {'product_id': bom_2_finished_product.id, 'product_qty': 1.0},)]
def test_cycle_on_line_update(self):
lines = self.bom_1.bom_line_ids
bom_2_finished_product = self.bom_2.product_id
with self.assertRaises(exceptions.ValidationError):
self.bom_1.bom_line_ids = [(1, lines[0].id, {'product_id': bom_2_finished_product.id})]
def test_cycle_on_bom_unarchive(self):
finished_product = self.bom_1.product_id
component = self.bom_1.bom_line_ids.product_id[0]
self.bom_1.active = False
self.env['mrp.bom'].create({
'product_id': component.id,
'product_tmpl_id': component.product_tmpl_id.id,
'product_uom_id': component.uom_id.id,
'product_qty': 1.0,
'type': 'normal',
'bom_line_ids': [
(0, 0, {'product_id': finished_product.id, 'product_qty': 1.0}),
],
})
with self.assertRaises(exceptions.ValidationError):
self.bom_1.active = True
def test_cycle_on_bom_creation(self):
finished_product = self.bom_4.product_id
component = self.bom_4.bom_line_ids.product_id
with self.assertRaises(exceptions.ValidationError):
self.env['mrp.bom'].create({
'product_id': component.id,
'product_tmpl_id': component.product_tmpl_id.id,
'product_uom_id': component.uom_id.id,
'product_qty': 1.0,
'type': 'normal',
'bom_line_ids': [
(0, 0, {'product_id': finished_product.id, 'product_qty': 1.0}),
],
})
def test_indirect_cycle_on_bom_creation(self):
"""
Three BoMs:
A -> D
A -> B
B -> C
Create a new BoM C -> A. At first glance, this new BoM is ok because it
does nat have a cycle (C -> A -> D). But there is an indirect cycle:
A -> B -> C -> A
Hence this new BoM should raise an error.
"""
product_A, product_B, product_C, product_D = self.env['product.product'].create([{
'name': '%s' % i
} for i in range(4)])
self.env['mrp.bom'].create([{
'product_id': finished.id,
'product_tmpl_id': finished.product_tmpl_id.id,
'product_uom_id': finished.uom_id.id,
'product_qty': 1.0,
'type': 'normal',
'bom_line_ids': [
(0, 0, {'product_id': compo.id, 'product_qty': 1.0}),
],
} for finished, compo, in [
(product_A, product_D),
(product_A, product_B),
(product_B, product_C),
]])
with self.assertRaises(exceptions.ValidationError):
self.env['mrp.bom'].create({
'product_id': product_C.id,
'product_tmpl_id': product_C.product_tmpl_id.id,
'product_uom_id': product_C.uom_id.id,
'product_qty': 1.0,
'type': 'normal',
'bom_line_ids': [
(0, 0, {'product_id': product_A.id, 'product_qty': 1.0}),
],
})
def test_cycle_on_bom_sequencing(self):
"""
Six BoMs:
A -> D
A -> B
C -> D
C -> E
B -> C
C -> A
First new sequence: we reverse C->D and C->E, this is ok as it does not
create any cycle. Change the sequence again and set C->A before C->D: it
should raise an error because C->A becomes the main BoM of C, and this
will create a cycle: A -> B -> C -> A
"""
product_A, product_B, product_C, product_D, product_E = self.env['product.product'].create([{
'name': '%s' % i
} for i in range(5)])
boms = self.env['mrp.bom'].create([{
'product_id': finished.id,
'product_tmpl_id': finished.product_tmpl_id.id,
'product_uom_id': finished.uom_id.id,
'product_qty': 1.0,
'type': 'normal',
'bom_line_ids': [
(0, 0, {'product_id': compo.id, 'product_qty': 1.0}),
],
} for finished, compo, in [
(product_A, product_D),
(product_A, product_B),
(product_C, product_D),
(product_C, product_E),
(product_B, product_C),
(product_C, product_A),
]])
# simulate resequence from UI (reverse C->D and C->E)
# (see odoo/addons/web/controllers/main.py:1352)
boms.invalidate_recordset()
for i, record in enumerate(boms[0] | boms[1] | boms[3] | boms[2] | boms[4] | boms[5]):
record.write({'sequence': i})
# simulate a second resequencing (set C->A before C->D)
with self.assertRaises(exceptions.ValidationError):
for i, record in enumerate(boms[0] | boms[1] | boms[5] | boms[3] | boms[2] | boms[4]):
record.write({'sequence': i})
def test_cycle_on_legit_apply_variants(self):
""" Should not raise anything """
self.env['mrp.bom'].create({
'product_tmpl_id': self.product_7_template.id,
'product_uom_id': self.product_7_template.uom_id.id,
'product_qty': 1.0,
'type': 'normal',
'bom_line_ids': [
(0, 0, {
'product_id': self.product_1.id,
'product_qty': 1.0
}),
(0, 0, {
'product_id': self.product_2.id,
'product_qty': 1.0,
'bom_product_template_attribute_value_ids': [(4, self.product_7_attr1_v2.id)]
}),
],
})
self.env['mrp.bom'].create({
'product_tmpl_id': self.product_2.product_tmpl_id.id,
'product_uom_id': self.product_2.uom_id.id,
'product_qty': 1.0,
'type': 'normal',
'bom_line_ids': [
(0, 0, {'product_id': self.product_7_1.id, 'product_qty': 1.0}),
],
})