mirror of
https://github.com/inventree/InvenTree.git
synced 2026-07-28 01:17:21 +00:00
[bug] BOM checksum fix (#12455)
* Enforce ordering of BOM items * Add prefetch to getRequiredParts * Add regression test
This commit is contained in:
@@ -2068,7 +2068,14 @@ class Part(
|
|||||||
result_hash = hashlib.md5(str(self.id).encode())
|
result_hash = hashlib.md5(str(self.id).encode())
|
||||||
|
|
||||||
# List *all* BOM items (including inherited ones!)
|
# List *all* BOM items (including inherited ones!)
|
||||||
bom_items = self.get_bom_items().all().prefetch_related('part', 'sub_part')
|
# Note: We must order the BOM items in a consistent way, otherwise the hash will change if the order of the items changes
|
||||||
|
bom_items = (
|
||||||
|
self
|
||||||
|
.get_bom_items()
|
||||||
|
.all()
|
||||||
|
.prefetch_related('part', 'sub_part')
|
||||||
|
.order_by('pk')
|
||||||
|
)
|
||||||
|
|
||||||
for item in bom_items:
|
for item in bom_items:
|
||||||
result_hash.update(str(item.get_item_hash()).encode())
|
result_hash.update(str(item.get_item_hash()).encode())
|
||||||
@@ -2141,7 +2148,7 @@ class Part(
|
|||||||
if parts is None:
|
if parts is None:
|
||||||
parts = set()
|
parts = set()
|
||||||
|
|
||||||
bom_items = self.get_bom_items()
|
bom_items = self.get_bom_items().prefetch_related('sub_part')
|
||||||
|
|
||||||
for bom_item in bom_items:
|
for bom_item in bom_items:
|
||||||
sub_part = bom_item.sub_part
|
sub_part = bom_item.sub_part
|
||||||
|
|||||||
@@ -1,5 +1,8 @@
|
|||||||
"""Unit tests for the BomItem model."""
|
"""Unit tests for the BomItem model."""
|
||||||
|
|
||||||
|
import hashlib
|
||||||
|
from unittest import mock
|
||||||
|
|
||||||
import django.core.exceptions as django_exceptions
|
import django.core.exceptions as django_exceptions
|
||||||
from django.db import transaction
|
from django.db import transaction
|
||||||
from django.test import TestCase
|
from django.test import TestCase
|
||||||
@@ -460,6 +463,59 @@ class BomItemTest(TestCase):
|
|||||||
bom_item2.quantity = 99
|
bom_item2.quantity = 99
|
||||||
bom_item2.save()
|
bom_item2.save()
|
||||||
|
|
||||||
|
def test_bom_hash_order_consistency(self):
|
||||||
|
"""Regression test for BOM checksum instability due to non-deterministic item ordering.
|
||||||
|
|
||||||
|
See: https://github.com/inventree/InvenTree/issues/12445
|
||||||
|
|
||||||
|
get_bom_hash() must apply an explicit, stable ordering when iterating the BOM
|
||||||
|
items - otherwise the resulting hash can differ purely because the underlying
|
||||||
|
query returned rows in a different order (e.g. Postgres provides no ordering
|
||||||
|
guarantee unless an ORDER BY clause is specified).
|
||||||
|
"""
|
||||||
|
assembly = Part.objects.create(
|
||||||
|
name='HashOrderAssembly', description='An assembly part', assembly=True
|
||||||
|
)
|
||||||
|
|
||||||
|
for ii in range(5):
|
||||||
|
sub_part = Part.objects.create(
|
||||||
|
name=f'HashOrderPart{ii}',
|
||||||
|
description='A sub-part for hash ordering test',
|
||||||
|
component=True,
|
||||||
|
)
|
||||||
|
BomItem.objects.create(part=assembly, sub_part=sub_part, quantity=ii + 1)
|
||||||
|
|
||||||
|
# Calling get_bom_hash() repeatedly must always return the same value
|
||||||
|
h1 = assembly.get_bom_hash()
|
||||||
|
h2 = assembly.get_bom_hash()
|
||||||
|
self.assertEqual(h1, h2)
|
||||||
|
|
||||||
|
def hash_items(items) -> str:
|
||||||
|
"""Replicate the hashing logic of get_bom_hash(), for a given item order."""
|
||||||
|
result_hash = hashlib.md5(str(assembly.id).encode())
|
||||||
|
|
||||||
|
for item in items:
|
||||||
|
result_hash.update(str(item.get_item_hash()).encode())
|
||||||
|
|
||||||
|
return str(result_hash.digest())
|
||||||
|
|
||||||
|
items_forward = list(assembly.get_bom_items().order_by('pk'))
|
||||||
|
items_reverse = list(assembly.get_bom_items().order_by('-pk'))
|
||||||
|
self.assertEqual(items_forward, list(reversed(items_reverse)))
|
||||||
|
|
||||||
|
# Sanity check: hashing the *same* items in a different order produces a
|
||||||
|
# different result - so ordering genuinely matters here
|
||||||
|
self.assertNotEqual(hash_items(items_forward), hash_items(items_reverse))
|
||||||
|
|
||||||
|
# Simulate the underlying queryset returning BOM items in a non pk-ascending
|
||||||
|
# order (as could occur against a real database with no ORDER BY applied).
|
||||||
|
# get_bom_hash() must be unaffected, always normalizing to the same order.
|
||||||
|
reversed_queryset = assembly.get_bom_items().order_by('-pk')
|
||||||
|
with mock.patch.object(Part, 'get_bom_items', return_value=reversed_queryset):
|
||||||
|
hash_from_reversed_source = assembly.get_bom_hash()
|
||||||
|
|
||||||
|
self.assertEqual(hash_from_reversed_source, hash_items(items_forward))
|
||||||
|
|
||||||
def test_bom_validated(self):
|
def test_bom_validated(self):
|
||||||
"""Test for caching of 'bom_validated' property."""
|
"""Test for caching of 'bom_validated' property."""
|
||||||
from part.tasks import validate_bom
|
from part.tasks import validate_bom
|
||||||
|
|||||||
Reference in New Issue
Block a user