mirror of
https://github.com/inventree/InvenTree.git
synced 2026-07-17 20:23:50 +00:00
- Ensure database values are used for delta updates
- Regression tests
(cherry picked from commit 431a6e49a0)
Co-authored-by: Oliver <oliver.henry.walters@gmail.com>
This commit is contained in:
co-authored by
Oliver
parent
ae0765cebd
commit
983916cb5a
@@ -1197,9 +1197,11 @@ class Build(
|
|||||||
trigger_event(BuildEvents.OUTPUT_COMPLETED, id=output.pk, build_id=self.pk)
|
trigger_event(BuildEvents.OUTPUT_COMPLETED, id=output.pk, build_id=self.pk)
|
||||||
|
|
||||||
# Increase the completed quantity for this build
|
# Increase the completed quantity for this build
|
||||||
self.completed += output.quantity
|
# Increment at the database level to prevent lost updates
|
||||||
|
# (multiple outputs may be completed concurrently)
|
||||||
self.save()
|
self.completed = F('completed') + output.quantity
|
||||||
|
self.save(update_fields=['completed'])
|
||||||
|
self.refresh_from_db(fields=['completed'])
|
||||||
|
|
||||||
@transaction.atomic
|
@transaction.atomic
|
||||||
def auto_allocate_stock(
|
def auto_allocate_stock(
|
||||||
@@ -2025,8 +2027,11 @@ class BuildItem(InvenTree.models.InvenTreeMetadataModel):
|
|||||||
)
|
)
|
||||||
|
|
||||||
# Increase the "consumed" count for the associated BuildLine
|
# Increase the "consumed" count for the associated BuildLine
|
||||||
self.build_line.consumed += quantity
|
# Increment at the database level to prevent lost updates
|
||||||
self.build_line.save()
|
# (multiple allocations against the same BuildLine may complete concurrently)
|
||||||
|
self.build_line.consumed = F('consumed') + quantity
|
||||||
|
self.build_line.save(update_fields=['consumed'])
|
||||||
|
self.build_line.refresh_from_db(fields=['consumed'])
|
||||||
|
|
||||||
# Decrease the allocated quantity
|
# Decrease the allocated quantity
|
||||||
self.quantity = max(0, self.quantity - quantity)
|
self.quantity = max(0, self.quantity - quantity)
|
||||||
|
|||||||
@@ -993,7 +993,11 @@ class BuildAllocationSerializer(serializers.Serializer):
|
|||||||
}
|
}
|
||||||
|
|
||||||
try:
|
try:
|
||||||
if build_item := BuildItem.objects.filter(**params).first():
|
# Lock the row, so concurrent allocations cannot both read
|
||||||
|
# the same starting quantity (lost update)
|
||||||
|
if build_item := (
|
||||||
|
BuildItem.objects.select_for_update().filter(**params).first()
|
||||||
|
):
|
||||||
# Find an existing BuildItem for this stock item
|
# Find an existing BuildItem for this stock item
|
||||||
# If it exists, increase the quantity
|
# If it exists, increase the quantity
|
||||||
build_item.quantity += quantity
|
build_item.quantity += quantity
|
||||||
|
|||||||
@@ -643,6 +643,70 @@ class BuildTest(BuildTestBase):
|
|||||||
for output in outputs:
|
for output in outputs:
|
||||||
self.assertFalse(output.is_building)
|
self.assertFalse(output.is_building)
|
||||||
|
|
||||||
|
def test_complete_output_stale_build_instance(self):
|
||||||
|
"""The 'completed' count is incremented atomically at the database level.
|
||||||
|
|
||||||
|
Simulates two concurrent processes completing different outputs of the
|
||||||
|
same build, each holding its own (stale) copy of the Build instance.
|
||||||
|
"""
|
||||||
|
self.stock_1_1.quantity = 1000
|
||||||
|
self.stock_1_1.save()
|
||||||
|
|
||||||
|
self.stock_2_1.quantity = 30
|
||||||
|
self.stock_2_1.save()
|
||||||
|
|
||||||
|
self.build.issue_build()
|
||||||
|
|
||||||
|
# Allocate non-tracked parts
|
||||||
|
self.allocate_stock(
|
||||||
|
None,
|
||||||
|
{
|
||||||
|
self.stock_1_1: self.stock_1_1.quantity,
|
||||||
|
self.stock_1_2: 10,
|
||||||
|
self.stock_2_1: 30,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
# Allocate tracked parts against each output
|
||||||
|
self.allocate_stock(self.output_1, {self.stock_3_1: 6})
|
||||||
|
self.allocate_stock(self.output_2, {self.stock_3_1: 14})
|
||||||
|
|
||||||
|
# Two independent in-memory copies of the same build
|
||||||
|
build_a = Build.objects.get(pk=self.build.pk)
|
||||||
|
build_b = Build.objects.get(pk=self.build.pk)
|
||||||
|
|
||||||
|
build_a.complete_build_output(self.output_1, None)
|
||||||
|
build_b.complete_build_output(self.output_2, None)
|
||||||
|
|
||||||
|
# Both completions must be counted
|
||||||
|
self.build.refresh_from_db()
|
||||||
|
self.assertEqual(self.build.completed, 10)
|
||||||
|
|
||||||
|
def test_complete_allocation_stale_build_line(self):
|
||||||
|
"""The 'consumed' count is incremented atomically at the database level.
|
||||||
|
|
||||||
|
Simulates two concurrent workers completing different allocations
|
||||||
|
against the same BuildLine, each holding its own (stale) copy of the line.
|
||||||
|
"""
|
||||||
|
self.build.issue_build()
|
||||||
|
|
||||||
|
self.allocate_stock(None, {self.stock_1_1: 3, self.stock_1_2: 5})
|
||||||
|
|
||||||
|
alloc_a, alloc_b = BuildItem.objects.filter(build_line=self.line_1).order_by(
|
||||||
|
'pk'
|
||||||
|
)
|
||||||
|
|
||||||
|
# Cache a separate copy of the BuildLine on each allocation
|
||||||
|
self.assertEqual(alloc_a.build_line.consumed, 0)
|
||||||
|
self.assertEqual(alloc_b.build_line.consumed, 0)
|
||||||
|
|
||||||
|
alloc_a.complete_allocation(user=self.user)
|
||||||
|
alloc_b.complete_allocation(user=self.user)
|
||||||
|
|
||||||
|
# Both consumed quantities must be counted
|
||||||
|
self.line_1.refresh_from_db()
|
||||||
|
self.assertEqual(self.line_1.consumed, 8)
|
||||||
|
|
||||||
def test_complete_with_required_tests(self):
|
def test_complete_with_required_tests(self):
|
||||||
"""Test the prevention completion when a required test is missing feature."""
|
"""Test the prevention completion when a required test is missing feature."""
|
||||||
# with required tests incompleted the save should fail
|
# with required tests incompleted the save should fail
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ from typing import cast
|
|||||||
from django.conf import settings
|
from django.conf import settings
|
||||||
from django.contrib.auth import authenticate, login
|
from django.contrib.auth import authenticate, login
|
||||||
from django.contrib.auth.models import User
|
from django.contrib.auth.models import User
|
||||||
|
from django.db import transaction
|
||||||
from django.db.models import F, Q
|
from django.db.models import F, Q
|
||||||
from django.http.response import JsonResponse
|
from django.http.response import JsonResponse
|
||||||
from django.urls import include, path, re_path
|
from django.urls import include, path, re_path
|
||||||
@@ -684,12 +685,20 @@ class PurchaseOrderLineItemList(
|
|||||||
# possibly merge duplicate items
|
# possibly merge duplicate items
|
||||||
line_item = None
|
line_item = None
|
||||||
if data.get('merge_items', True):
|
if data.get('merge_items', True):
|
||||||
other_line = models.PurchaseOrderLineItem.objects.filter(
|
with transaction.atomic():
|
||||||
|
# Lock the matching row, so concurrent line creations cannot
|
||||||
|
# both read the same starting quantity (lost update)
|
||||||
|
other_line = (
|
||||||
|
models.PurchaseOrderLineItem.objects
|
||||||
|
.select_for_update()
|
||||||
|
.filter(
|
||||||
part=data.get('part'),
|
part=data.get('part'),
|
||||||
order=data.get('order'),
|
order=data.get('order'),
|
||||||
target_date=data.get('target_date'),
|
target_date=data.get('target_date'),
|
||||||
destination=data.get('destination'),
|
destination=data.get('destination'),
|
||||||
).first()
|
)
|
||||||
|
.first()
|
||||||
|
)
|
||||||
|
|
||||||
if other_line is not None:
|
if other_line is not None:
|
||||||
other_line.quantity += Decimal(data.get('quantity', 0))
|
other_line.quantity += Decimal(data.get('quantity', 0))
|
||||||
|
|||||||
@@ -804,7 +804,9 @@ class PurchaseOrder(TotalPriceMixin, Order):
|
|||||||
|
|
||||||
if group:
|
if group:
|
||||||
# Check if there is already a matching line item (for this PurchaseOrder)
|
# Check if there is already a matching line item (for this PurchaseOrder)
|
||||||
matches = self.lines.filter(part=supplier_part)
|
# Lock the matching row, so concurrent additions cannot both read
|
||||||
|
# the same starting quantity (lost update)
|
||||||
|
matches = self.lines.select_for_update().filter(part=supplier_part)
|
||||||
|
|
||||||
if matches.count() > 0:
|
if matches.count() > 0:
|
||||||
line = matches.first()
|
line = matches.first()
|
||||||
@@ -1063,9 +1065,14 @@ class PurchaseOrder(TotalPriceMixin, Order):
|
|||||||
# Cache the custom status options for the StockItem model
|
# Cache the custom status options for the StockItem model
|
||||||
custom_stock_status_values = stock.models.StockItem.STATUS_CLASS.custom_values()
|
custom_stock_status_values = stock.models.StockItem.STATUS_CLASS.custom_values()
|
||||||
|
|
||||||
line_items = PurchaseOrderLineItem.objects.filter(
|
# Lock the line item rows, so that concurrent receipts against the same
|
||||||
pk__in=line_items_ids
|
# lines cannot both read the same 'received' value (lost update)
|
||||||
).prefetch_related('part', 'part__part', 'order')
|
line_items = (
|
||||||
|
PurchaseOrderLineItem.objects
|
||||||
|
.select_for_update()
|
||||||
|
.filter(pk__in=line_items_ids)
|
||||||
|
.prefetch_related('part', 'part__part', 'order')
|
||||||
|
)
|
||||||
|
|
||||||
# Map order line items to their corresponding stock items
|
# Map order line items to their corresponding stock items
|
||||||
line_item_map = {line.pk: line for line in line_items}
|
line_item_map = {line.pk: line for line in line_items}
|
||||||
@@ -1196,8 +1203,10 @@ class PurchaseOrder(TotalPriceMixin, Order):
|
|||||||
stock_data['is_building'] = False
|
stock_data['is_building'] = False
|
||||||
|
|
||||||
# Increase the 'completed' quantity for the build order
|
# Increase the 'completed' quantity for the build order
|
||||||
build_order.completed += stock_quantity
|
# Increment at the database level to prevent lost updates
|
||||||
build_order.save()
|
build_order.completed = F('completed') + stock_quantity
|
||||||
|
build_order.save(update_fields=['completed'])
|
||||||
|
build_order.refresh_from_db(fields=['completed'])
|
||||||
elif build_order.status == BuildStatus.CANCELLED:
|
elif build_order.status == BuildStatus.CANCELLED:
|
||||||
# A 'cancelled' build order is ignored
|
# A 'cancelled' build order is ignored
|
||||||
pass
|
pass
|
||||||
@@ -2911,8 +2920,10 @@ class SalesOrderAllocation(models.Model):
|
|||||||
)
|
)
|
||||||
|
|
||||||
# Update the 'shipped' quantity
|
# Update the 'shipped' quantity
|
||||||
self.line.shipped += self.quantity
|
# Increment at the database level to prevent lost updates
|
||||||
self.line.save()
|
self.line.shipped = F('shipped') + self.quantity
|
||||||
|
self.line.save(update_fields=['shipped'])
|
||||||
|
self.line.refresh_from_db(fields=['shipped'])
|
||||||
|
|
||||||
# Update our own reference to the StockItem
|
# Update our own reference to the StockItem
|
||||||
# (It may have changed if the stock was split)
|
# (It may have changed if the stock was split)
|
||||||
@@ -4021,8 +4032,10 @@ class TransferOrderAllocation(models.Model):
|
|||||||
|
|
||||||
# Update the transferred qty
|
# Update the transferred qty
|
||||||
# Note: use the quantity which was *actually* transferred
|
# Note: use the quantity which was *actually* transferred
|
||||||
self.line.transferred += transfer_quantity
|
# Increment at the database level to prevent lost updates
|
||||||
self.line.save()
|
self.line.transferred = F('transferred') + transfer_quantity
|
||||||
|
self.line.save(update_fields=['transferred'])
|
||||||
|
self.line.refresh_from_db(fields=['transferred'])
|
||||||
|
|
||||||
|
|
||||||
def _touch_order_updated_at(instance):
|
def _touch_order_updated_at(instance):
|
||||||
|
|||||||
@@ -220,6 +220,29 @@ class SalesOrderTest(InvenTreeTestCase):
|
|||||||
self.assertTrue(self.line.is_fully_allocated())
|
self.assertTrue(self.line.is_fully_allocated())
|
||||||
self.assertEqual(self.line.allocated_quantity(), 50)
|
self.assertEqual(self.line.allocated_quantity(), 50)
|
||||||
|
|
||||||
|
def test_complete_allocation_stale_line_instance(self):
|
||||||
|
"""The 'shipped' count is incremented atomically at the database level.
|
||||||
|
|
||||||
|
Simulates two concurrent workers completing different allocations
|
||||||
|
against the same order line, each holding its own (stale) copy of the line.
|
||||||
|
"""
|
||||||
|
self.allocate_stock(True)
|
||||||
|
|
||||||
|
alloc_a, alloc_b = SalesOrderAllocation.objects.filter(line=self.line).order_by(
|
||||||
|
'pk'
|
||||||
|
)
|
||||||
|
|
||||||
|
# Cache a separate copy of the line on each allocation
|
||||||
|
self.assertEqual(alloc_a.line.shipped, 0)
|
||||||
|
self.assertEqual(alloc_b.line.shipped, 0)
|
||||||
|
|
||||||
|
alloc_a.complete_allocation(None)
|
||||||
|
alloc_b.complete_allocation(None)
|
||||||
|
|
||||||
|
# Both shipped quantities must be counted
|
||||||
|
self.line.refresh_from_db()
|
||||||
|
self.assertEqual(self.line.shipped, 50)
|
||||||
|
|
||||||
def test_allocate_variant(self):
|
def test_allocate_variant(self):
|
||||||
"""Allocate a variant of the designated item."""
|
"""Allocate a variant of the designated item."""
|
||||||
SalesOrderAllocation.objects.create(
|
SalesOrderAllocation.objects.create(
|
||||||
|
|||||||
Reference in New Issue
Block a user