[bug] Unique constraint for serial numbers (#12483)

* Unique constraint for serial numbers

* Revert unrelated changes

* Updated tests

src/backend/InvenTree/stock/migrations/0126_serial_number_concurrency_guard.py

@@ -0,0 +1,34 @@
+# Generated by Django 5.2.16 on 2026-07-27 08:58
+
+from django.conf import settings
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("build", "0059_build_tags"),
+        ("company", "0080_company_tags"),
+        ("order", "0121_add_line_item_discount"),
+        ("part", "0152_alter_partpricing_currency"),
+        ("stock", "0125_remove_mptt_fields"),
+        (
+            "taggit",
+            "0006_rename_taggeditem_content_type_object_id_taggit_tagg_content_8fc721_idx",
+        ),
+        ("users", "0015_alter_userprofile_type"),
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+    ]
+
+    operations = [
+        migrations.AddConstraint(
+            model_name="stockitem",
+            constraint=models.UniqueConstraint(
+                condition=models.Q(
+                    ("serial__isnull", False), models.Q(("serial", ""), _negated=True)
+                ),
+                fields=("part", "serial"),
+                name="stock_item_unique_part_serial",
+            ),
+        ),
+    ]

src/backend/InvenTree/stock/models.py

@@ -13,7 +13,7 @@ from django.contrib.auth.models import User
 from django.core.exceptions import ValidationError
 from django.core.validators import MinValueValidator
 from django.db import models, transaction
-from django.db.models import Q, QuerySet, Sum
+from django.db.models import Q, QuerySet, Sum, UniqueConstraint
 from django.db.models.functions import Coalesce
 from django.db.models.signals import post_delete, post_save
 from django.db.utils import IntegrityError, OperationalError
@@ -468,6 +468,23 @@ class StockItem(
         """Model meta options."""
 
         verbose_name = _('Stock Item')
+        constraints = [
+            # Baseline, database-enforced duplicate-serial guard: a given Part
+            # can never have two StockItem rows sharing the same serial number.
+            # This alone does not cover the "unique across a Part variant tree"
+            # (or globally-unique) semantics controlled by the
+            # SERIAL_NUMBER_GLOBALLY_UNIQUE setting - see
+            # StockItem._lock_serial_numbers() for that; a UniqueConstraint
+            # cannot reference the joined Part.tree_id field.
+            UniqueConstraint(
+                fields=['part', 'serial'],
+                # Non-serialized StockItems may store serial as either NULL or
+                # '' depending on the creation path - exclude both, since
+                # "no serial" is not a value this constraint should govern
+                condition=Q(serial__isnull=False) & ~Q(serial=''),
+                name='stock_item_unique_part_serial',
+            )
+        ]
 
     class MPTTMeta:
         """MPTT metaclass options."""
@@ -655,7 +672,68 @@ class StockItem(
             & Q(expiry_date__lt=InvenTree.helpers.current_date())
         )
 
+    @staticmethod
+    def _lock_serial_numbers(part: PartModels.Part, serials: list) -> None:
+        """Serialize concurrent serial number creation, and re-validate under the lock.
+
+        Must be called from within an atomic transaction. Serial number
+        uniqueness is scoped by the SERIAL_NUMBER_GLOBALLY_UNIQUE setting -
+        either across an entire Part variant tree (the default), or globally.
+        That scope depends on Part.tree_id, which is not a field on
+        StockItem, so it cannot be expressed as a database-level
+        UniqueConstraint on StockItem directly (a UniqueConstraint cannot
+        reference a joined field). Instead, this select_for_update()s
+        existing rows that already represent the relevant scope - every Part
+        in the tree, or (for the globally-unique case, where there is no
+        single tree to lock) the SERIAL_NUMBER_GLOBALLY_UNIQUE setting's own
+        row - so concurrent creation attempts within the same scope
+        serialize against each other. It then re-checks for conflicts
+        against StockItem while holding that lock. This closes the race
+        where two concurrent requests both read "no conflict" before either
+        has committed its creation.
+
+        Raises:
+            ValidationError: If any of the provided serial numbers now conflict
+        """
+        if get_global_setting('SERIAL_NUMBER_GLOBALLY_UNIQUE', False):
+            # There is no single Part tree covering "globally unique" - lock
+            # the setting's own row instead, so all concurrent global-scope
+            # creation attempts serialize against each other regardless of
+            # which part is involved. This row is guaranteed to already
+            # exist: reaching this branch means the setting is currently
+            # True, which can only happen if it was explicitly set (and
+            # therefore persisted) at some point.
+            setting, _created = common.models.InvenTreeSetting.objects.get_or_create(
+                key='SERIAL_NUMBER_GLOBALLY_UNIQUE', defaults={'value': str(True)}
+            )
+            common.models.InvenTreeSetting.objects.select_for_update().get(
+                pk=setting.pk
+            )
+        else:
+            # Lock every Part in this variant tree (this always includes
+            # 'part' itself), so concurrent creation attempts for any part in
+            # the same tree serialize against each other
+            list(
+                PartModels.Part.objects
+                .select_for_update()
+                .filter(tree_id=part.tree_id)
+                .order_by('pk')
+            )
+
+        # Re-validate for conflicts now that the lock is held - any
+        # concurrent request for the same scope has either already committed
+        # (and will now show up here) or is blocked behind this lock (and
+        # will see this request's result once it releases)
+        conflicts = part.find_conflicting_serial_numbers(serials)
+
+        if conflicts:
+            msg = _('The following serial numbers already exist or are invalid')
+            msg += ' : '
+            msg += ','.join(str(x) for x in conflicts)
+            raise ValidationError({'serial_numbers': msg})
+
     @classmethod
+    @transaction.atomic
     def _create_serial_numbers(cls, serials: list, **kwargs) -> QuerySet:
         """Create multiple stock items with the provided serial numbers.
 
@@ -672,8 +750,11 @@ class StockItem(
         This method uses bulk_create to create multiple StockItem objects in a single query,
         which is much more efficient than creating them one-by-one.
 
-        However, it does not perform any validation checks on the provided serial numbers,
+        Concurrent calls for an overlapping set of serial numbers are
-        and also does not generate any "stock tracking entries".
+        serialized against each other (see _lock_serial_numbers()), so unlike
+        other validation checks, this method's duplicate-serial protection is
+        safe even when the caller's own pre-check raced against another
+        request. It does not generate any "stock tracking entries".
 
         Note: This is an 'internal' function and should not be used by external code / plugins.
         """
@@ -722,6 +803,10 @@ class StockItem(
         if 'part' not in data:
             raise ValidationError({'part': _('Part must be specified')})
 
+        # Serialize against any other concurrent request creating an
+        # overlapping set of serial numbers, and re-validate under that lock
+        cls._lock_serial_numbers(data['part'], serials)
+
         parent = kwargs.pop('parent', None) or data.get('parent')
 
         data['parent'] = parent
@@ -741,7 +826,13 @@ class StockItem(
             items.append(StockItem(**data))
 
         # Create the StockItem objects in bulk
-        items = bulk_create_and_fetch(StockItem, items)
+        # (the IntegrityError catch is a defense-in-depth backstop against
+        # the database-level UniqueConstraint - _lock_serial_numbers() above
+        # should already have ruled out any conflict)
+        try:
+            items = bulk_create_and_fetch(StockItem, items)
+        except IntegrityError as exc:
+            raise ValidationError({'serial_numbers': str(exc)}) from exc
 
         # Trigger a 'created' event for the new items
         # Note that instead of a single event for each item,

src/backend/InvenTree/stock/test_migrations.py

@@ -189,7 +189,7 @@ class TestTestResultMigration(MigratorTestCase):
             for jj in range(4):
                 si = StockItem.objects.create(
                     part=child,
-                    serial=str(1 + ii * jj),
+                    serial=str(1 + jj),
                     quantity=1,
                     tree_id=0,
                     level=0,

src/backend/InvenTree/stock/tests.py

@@ -1,14 +1,19 @@
 """Tests for stock app."""
 
 import datetime
+import threading
+from unittest import mock
 
+from django.contrib.auth import get_user_model
 from django.core.exceptions import ValidationError
-from django.db import transaction
+from django.db import connection, transaction
 from django.db.models import Sum
-from django.test import override_settings
+from django.test import TransactionTestCase, override_settings, skipUnlessDBFeature
+from django.urls import reverse
 
 from django_q.models import OrmQ
 from djmoney.money import Money
+from rest_framework.test import APIClient
 
 from build.models import Build
 from common.models import InvenTreeSetting
@@ -1966,3 +1971,385 @@ class AdminTest(AdminTestCase):
     def test_admin(self):
         """Test the admin URL."""
         self.helper(model=StockLocationType)
+
+
+@skipUnlessDBFeature('has_select_for_update')
+class StockItemSerialConcurrencyTest(TransactionTestCase):
+    """Genuine cross-transaction regression test for StockItem._create_serial_numbers().
+
+    Uses two real threads (each with its own database connection) to reproduce
+    the reported race: two concurrent requests to create a StockItem with the
+    *same* serial number for the same Part could both check for conflicts
+    before either had committed its bulk_create - producing two StockItem rows
+    sharing one (part, serial) pair.
+
+    _create_serial_numbers() now locks (select_for_update, on the Part rows
+    covering the relevant scope - see StockItem._lock_serial_numbers()) and
+    re-validates for conflicts under that lock before creating anything, and
+    a database-level UniqueConstraint on (part, serial) backstops it, so only
+    one of two concurrent requests for the same serial number may succeed.
+    """
+
+    fixtures = ['users']
+
+    def setUp(self):
+        """Create a single trackable Part to create serialized stock against."""
+        super().setUp()
+
+        self.part = Part.objects.create(
+            name='Concurrency serial part',
+            description='Part for serial creation concurrency test',
+            trackable=True,
+        )
+
+    def test_concurrent_creation_does_not_duplicate_serial(self):
+        """Two concurrent requests for the same serial number must not both succeed."""
+        start_barrier = threading.Barrier(2, timeout=5)
+        errors = []
+        results = []
+
+        # Wrap StockItem._lock_serial_numbers() so both threads reach the
+        # (real, database-level) row lock at the same time - one wins the
+        # lock and proceeds, the other blocks until the winner's transaction
+        # completes.
+        original_lock_serial_numbers = StockItem._lock_serial_numbers
+
+        def synced_lock_serial_numbers(part, serials):
+            start_barrier.wait(timeout=5)
+            return original_lock_serial_numbers(part, serials)
+
+        def create():
+            try:
+                StockItem._create_serial_numbers(['SN-RACE'], part=self.part)
+                results.append('ok')
+            except ValidationError:
+                results.append('rejected')
+            except Exception as exc:  # pragma: no cover - surfaced via errors list
+                errors.append(exc)
+            finally:
+                connection.close()
+
+        thread_a = threading.Thread(target=create)
+        thread_b = threading.Thread(target=create)
+
+        with mock.patch.object(
+            StockItem, '_lock_serial_numbers', synced_lock_serial_numbers
+        ):
+            thread_a.start()
+            thread_b.start()
+
+            thread_a.join(timeout=5)
+            thread_b.join(timeout=5)
+
+        self.assertFalse(thread_a.is_alive())
+        self.assertFalse(thread_b.is_alive())
+        self.assertEqual(errors, [])
+
+        # Exactly one request must have been rejected as a duplicate
+        self.assertEqual(sorted(results), ['ok', 'rejected'])
+
+        # The serial number must only have been created once
+        self.assertEqual(
+            StockItem.objects.filter(part=self.part, serial='SN-RACE').count(), 1
+        )
+
+
+@skipUnlessDBFeature('has_select_for_update')
+class StockItemSerialBatchConcurrencyTest(TransactionTestCase):
+    """Genuine cross-transaction regression test for overlapping batch serial creation.
+
+    Extends the single-serial race in StockItemSerialConcurrencyTest to the
+    more realistic case of *bulk* serial number creation: three concurrent
+    requests, each creating 10 serial numbers, with the requested ranges
+    overlapping pairwise (but not all three sharing any single serial):
+
+        A: 1-10
+        B: 6-15   (overlaps A on 6-10, and C on 11-15)
+        C: 11-20
+
+    Since _create_serial_numbers() creates its whole batch atomically (a
+    conflict anywhere in the batch aborts the batch entirely), and A and C do
+    not share any serial with each other, exactly one of the following must
+    occur:
+
+        - B is rejected, and A and C both succeed in full, or
+        - B succeeds in full, and both A and C are rejected
+
+    In neither case can two of the three batches both succeed, and in neither
+    case can any serial number be created more than once. Since all three
+    batches target the same Part, StockItem._lock_serial_numbers() locks the
+    same Part rows for each of them (rather than locking anything specific to
+    the requested serials), so the three requests are fully serialized
+    against each other regardless of which serials they overlap on.
+    """
+
+    fixtures = ['users']
+
+    def setUp(self):
+        """Create a single trackable Part to create serialized stock against."""
+        super().setUp()
+
+        self.part = Part.objects.create(
+            name='Concurrency batch serial part',
+            description='Part for batch serial creation concurrency test',
+            trackable=True,
+        )
+
+    def test_concurrent_overlapping_batches_do_not_duplicate_serials(self):
+        """Three concurrent, partially-overlapping batch creation requests must stay consistent."""
+        start_barrier = threading.Barrier(3, timeout=10)
+        errors = []
+        results = {}
+        results_lock = threading.Lock()
+
+        # Wrap StockItem._lock_serial_numbers() so all three threads reach the
+        # (real, database-level) row locks at the same time, maximising
+        # contention across the overlapping sets of serial numbers.
+        original_lock_serial_numbers = StockItem._lock_serial_numbers
+
+        def synced_lock_serial_numbers(part, serials):
+            start_barrier.wait(timeout=10)
+            return original_lock_serial_numbers(part, serials)
+
+        batches = {
+            'a': [str(i) for i in range(1, 11)],  # 1-10
+            'b': [str(i) for i in range(6, 16)],  # 6-15
+            'c': [str(i) for i in range(11, 21)],  # 11-20
+        }
+
+        def create(name, serials):
+            try:
+                StockItem._create_serial_numbers(serials, part=self.part)
+                with results_lock:
+                    results[name] = 'ok'
+            except ValidationError:
+                with results_lock:
+                    results[name] = 'rejected'
+            except Exception as exc:  # pragma: no cover - surfaced via errors list
+                with results_lock:
+                    errors.append(exc)
+            finally:
+                connection.close()
+
+        threads = [
+            threading.Thread(target=create, args=(name, serials))
+            for name, serials in batches.items()
+        ]
+
+        with mock.patch.object(
+            StockItem, '_lock_serial_numbers', synced_lock_serial_numbers
+        ):
+            for thread in threads:
+                thread.start()
+
+            for thread in threads:
+                thread.join(timeout=10)
+
+        for thread in threads:
+            self.assertFalse(thread.is_alive())
+        self.assertEqual(errors, [])
+        self.assertEqual(set(results.keys()), {'a', 'b', 'c'})
+
+        # Batch 'b' overlaps both 'a' and 'c', which do not overlap each
+        # other - so either 'b' alone wins, or both 'a' and 'c' win, never
+        # any other combination
+        ok = {name for name, outcome in results.items() if outcome == 'ok'}
+        self.assertIn(ok, [{'b'}, {'a', 'c'}])
+
+        # No serial number was created more than once
+        created_serials = list(
+            StockItem.objects.filter(part=self.part).values_list('serial', flat=True)
+        )
+        self.assertEqual(len(created_serials), len(set(created_serials)))
+
+        # The created serials are exactly the union of the winning batches -
+        # each successful batch's serials all exist, and (since a rejected
+        # batch creates nothing at all) nothing beyond that union exists
+        expected_serials = set()
+        for name in ok:
+            expected_serials.update(batches[name])
+
+        self.assertEqual(set(created_serials), expected_serials)
+
+
+@skipUnlessDBFeature('has_select_for_update')
+class StockItemSerialGloballyUniqueConcurrencyTest(TransactionTestCase):
+    """Genuine cross-transaction regression test for the globally-unique locking path.
+
+    Reproduces the same race as StockItemSerialConcurrencyTest, but with
+    SERIAL_NUMBER_GLOBALLY_UNIQUE enabled and the two concurrent requests
+    targeting *different* parts (in different variant trees). There is no
+    single Part tree covering this scope, so StockItem._lock_serial_numbers()
+    instead locks the SERIAL_NUMBER_GLOBALLY_UNIQUE setting's own row - this
+    verifies that still serializes the two requests correctly.
+    """
+
+    fixtures = ['users']
+
+    def setUp(self):
+        """Create two unrelated trackable Parts (in different variant trees)."""
+        super().setUp()
+
+        InvenTreeSetting.set_setting('SERIAL_NUMBER_GLOBALLY_UNIQUE', True, None)
+        self.addCleanup(
+            InvenTreeSetting.set_setting, 'SERIAL_NUMBER_GLOBALLY_UNIQUE', False, None
+        )
+
+        self.part_a = Part.objects.create(
+            name='Globally-unique concurrency part A',
+            description='Part A for globally-unique serial concurrency test',
+            trackable=True,
+        )
+        self.part_b = Part.objects.create(
+            name='Globally-unique concurrency part B',
+            description='Part B for globally-unique serial concurrency test',
+            trackable=True,
+        )
+
+    def test_concurrent_creation_across_parts_does_not_duplicate_serial(self):
+        """Two concurrent requests for different parts must not both succeed."""
+        start_barrier = threading.Barrier(2, timeout=5)
+        errors = []
+        results = {}
+        results_lock = threading.Lock()
+
+        original_lock_serial_numbers = StockItem._lock_serial_numbers
+
+        def synced_lock_serial_numbers(part, serials):
+            start_barrier.wait(timeout=5)
+            return original_lock_serial_numbers(part, serials)
+
+        def create(name, part):
+            try:
+                StockItem._create_serial_numbers(['SN-RACE'], part=part)
+                with results_lock:
+                    results[name] = 'ok'
+            except ValidationError:
+                with results_lock:
+                    results[name] = 'rejected'
+            except Exception as exc:  # pragma: no cover - surfaced via errors list
+                with results_lock:
+                    errors.append(exc)
+            finally:
+                connection.close()
+
+        thread_a = threading.Thread(target=create, args=('a', self.part_a))
+        thread_b = threading.Thread(target=create, args=('b', self.part_b))
+
+        with mock.patch.object(
+            StockItem, '_lock_serial_numbers', synced_lock_serial_numbers
+        ):
+            thread_a.start()
+            thread_b.start()
+
+            thread_a.join(timeout=5)
+            thread_b.join(timeout=5)
+
+        self.assertFalse(thread_a.is_alive())
+        self.assertFalse(thread_b.is_alive())
+        self.assertEqual(errors, [])
+
+        # Exactly one request must have been rejected as a duplicate, even
+        # though the two parts are unrelated
+        self.assertEqual(sorted(results.values()), ['ok', 'rejected'])
+
+        # The serial number must only have been created once, across both parts
+        self.assertEqual(
+            StockItem.objects.filter(
+                part__in=[self.part_a, self.part_b], serial='SN-RACE'
+            ).count(),
+            1,
+        )
+
+
+@skipUnlessDBFeature('has_select_for_update')
+class StockItemSerialAPIConcurrencyTest(TransactionTestCase):
+    """Genuine cross-transaction regression test driven through the real StockItem creation API.
+
+    Same race as StockItemSerialConcurrencyTest, but issued as five
+    concurrent HTTP POST requests against the 'api-stock-list' endpoint
+    (stock.api.StockList.create()) rather than calling
+    StockItem._create_serial_numbers() directly - this exercises the full
+    view/serializer stack (permission checks, serial number extraction,
+    pre-validation) under concurrency, not just the locking primitive itself.
+    """
+
+    fixtures = ['users']
+
+    def setUp(self):
+        """Create a superuser and a single trackable Part to create serialized stock against."""
+        super().setUp()
+
+        self.user = get_user_model().objects.get(username='sue_the_superuser')
+
+        self.part = Part.objects.create(
+            name='API concurrency serial part',
+            description='Part for API serial creation concurrency test',
+            trackable=True,
+        )
+
+    def test_concurrent_api_creation_does_not_duplicate_serial(self):
+        """Five concurrent API requests for the same serial number must not all succeed."""
+        n_threads = 5
+        start_barrier = threading.Barrier(n_threads, timeout=10)
+        errors = []
+        results = []
+        results_lock = threading.Lock()
+
+        url = reverse('api-stock-list')
+
+        # Wrap StockItem._lock_serial_numbers() so all five threads reach the
+        # (real, database-level) row lock at the same time, regardless of how
+        # long each request takes to reach that point.
+        original_lock_serial_numbers = StockItem._lock_serial_numbers
+
+        def synced_lock_serial_numbers(part, serials):
+            start_barrier.wait(timeout=10)
+            return original_lock_serial_numbers(part, serials)
+
+        def create():
+            client = APIClient()
+            client.force_authenticate(user=self.user)
+
+            try:
+                response = client.post(
+                    url,
+                    {
+                        'part': self.part.pk,
+                        'quantity': 1,
+                        'serial_numbers': 'SN-API-RACE',
+                    },
+                    format='json',
+                )
+                with results_lock:
+                    results.append(response.status_code)
+            except Exception as exc:  # pragma: no cover - surfaced via errors list
+                with results_lock:
+                    errors.append(exc)
+            finally:
+                connection.close()
+
+        threads = [threading.Thread(target=create) for _ in range(n_threads)]
+
+        with mock.patch.object(
+            StockItem, '_lock_serial_numbers', synced_lock_serial_numbers
+        ):
+            for thread in threads:
+                thread.start()
+
+            for thread in threads:
+                thread.join(timeout=15)
+
+        for thread in threads:
+            self.assertFalse(thread.is_alive())
+        self.assertEqual(errors, [])
+
+        # Exactly one request must have succeeded; the rest must have been
+        # rejected as duplicates
+        self.assertEqual(results.count(201), 1)
+        self.assertEqual(results.count(400), n_threads - 1)
+
+        # The serial number must only have been created once
+        self.assertEqual(
+            StockItem.objects.filter(part=self.part, serial='SN-API-RACE').count(), 1
+        )