diff --git a/src/backend/InvenTree/part/models.py b/src/backend/InvenTree/part/models.py index 7cdcb879eb..c7c53258cd 100644 --- a/src/backend/InvenTree/part/models.py +++ b/src/backend/InvenTree/part/models.py @@ -540,6 +540,12 @@ class Part( # For legacy reasons the 'variant_of' field is used to indicate the MPTT parent parent_attr = 'variant_of' + # Do not order the tree by name (overrides InvenTreeTree.MPTTMeta). + # Ordered insertion of a new root node shifts the tree_id of every following + # tree in the database, which is expensive (O(N) row updates per new Part), + # and leaves the tree_id of any Part instances held in memory stale. + order_insertion_by = [] + @staticmethod def get_api_url(): """Return the list API endpoint URL associated with the Part model.""" @@ -686,7 +692,16 @@ class Part( ) }) - if self.tree_id == parent.tree_id: + # Read the tree_id values from the database, as in-memory values may be stale + tree_ids = dict( + Part.objects.filter(pk__in=[self.pk, parent.pk]).values_list( + 'pk', 'tree_id' + ) + ) + + if tree_ids.get(self.pk, self.tree_id) == tree_ids.get( + parent.pk, parent.tree_id + ): raise ValidationError({ 'sub_part': _( f"Part '{self}' cannot be used in BOM for '{parent}' (recursive)" diff --git a/src/backend/InvenTree/part/test_bom_item.py b/src/backend/InvenTree/part/test_bom_item.py index f114063420..a894c4870c 100644 --- a/src/backend/InvenTree/part/test_bom_item.py +++ b/src/backend/InvenTree/part/test_bom_item.py @@ -378,6 +378,59 @@ class BomItemTest(TestCase): with self.assertRaises(django_exceptions.ValidationError): BomItem.objects.create(part=part_v, sub_part=part_a, quantity=10) + def test_bom_unrelated_root_parts(self): + """Unrelated top-level parts created in succession can be added to each other's BOM. + + Regression test: ordered (by name) insertion of a new root Part shifted the tree_id + of following trees in the database, leaving instances held in memory with a stale + tree_id - which could match the new part's tree_id, falsely flagging the BOM as recursive. + """ + # Part 'Y' is inserted directly before Part 'Z' when ordered by name + part_z = Part.objects.create( + name='zzzz assembly', description='Z', assembly=True, component=True + ) + part_y = Part.objects.create( + name='zzzy assembly', description='Y', assembly=True, component=True + ) + + # In-memory tree_id values match the database + self.assertEqual(part_z.tree_id, Part.objects.get(pk=part_z.pk).tree_id) + self.assertNotEqual(part_z.tree_id, part_y.tree_id) + + # No refresh_from_db() required + BomItem.objects.create(part=part_y, sub_part=part_z, quantity=1) + + def test_root_part_create_preserves_tree_ids(self): + """Creating a new top-level part does not modify the tree_id of any other part.""" + tree_ids = dict(Part.objects.values_list('pk', 'tree_id')) + + # Name sorts before every existing part + Part.objects.create(name='!!! first part', description='first') + + self.assertEqual( + dict(Part.objects.filter(pk__in=tree_ids).values_list('pk', 'tree_id')), + tree_ids, + ) + + def test_check_add_to_bom_stale_tree_id(self): + """check_add_to_bom() uses database tree_id values, not stale in-memory values.""" + part_a = Part.objects.create( + name='Stale A', description='A', assembly=True, is_template=True + ) + part_b = Part.objects.create(name='Stale B', description='B', component=True) + + # Simulate a stale in-memory tree_id which matches the other part + part_b.tree_id = part_a.tree_id + + self.assertTrue(part_b.check_add_to_bom(part_a, raise_error=True)) + + # A genuine variant relationship is still detected + part_v = Part.objects.create( + name='Stale V', description='V', variant_of=part_a, component=True + ) + + self.assertFalse(part_v.check_add_to_bom(part_a)) + def test_locked_assembly(self): """Test that BomItem objects work correctly for a 'locked' assembly.""" assembly = Part.objects.create(