Arthur 1 день назад
Родитель
Сommit
ab05234faf
2 измененных файлов с 41 добавлено и 12 удалено
  1. 8 7
      netbox/circuits/models/circuits.py
  2. 33 5
      netbox/circuits/tests/test_models.py

+ 8 - 7
netbox/circuits/models/circuits.py

@@ -423,13 +423,14 @@ class CircuitTermination(
 
         This is written via snapshot() + save() rather than a queryset update() so that the write
         passes through post_save and is recorded in the changelog. A raw update() emits no signal,
-        so consumers which replay ObjectChange records -- notably the branching plugin, which
-        applies a CREATE via a raw save that never runs this method -- have no record of the write
-        and silently drop the association.
-
-        The Circuit is always re-fetched rather than reusing a cached `self.circuit`: creating the
-        A and Z terminations in sequence would otherwise snapshot a Circuit loaded before the A
-        pointer was set, recording a prechange value that no longer matches the database.
+        so consumers which replay ObjectChange records have no record of the write and silently
+        drop the association.
+
+        The Circuit is re-fetched rather than reusing a cached `self.circuit` so that saving the A
+        and Z terminations in sequence does not snapshot a Circuit loaded before the A pointer was
+        written. That only holds within a single sequential flow: under READ COMMITTED, concurrent
+        writers can each snapshot a Circuit which does not yet reflect the other's uncommitted
+        write. The row itself is safe, as update_fields limits each write to one column.
         """
         circuit = Circuit.objects.filter(pk=circuit_id).first()
         if circuit is None or getattr(circuit, f'{field_name}_id') == value:

+ 33 - 5
netbox/circuits/tests/test_models.py

@@ -282,7 +282,7 @@ class CircuitTerminationChangeLoggingTestCase(TestCase):
     """
     The Circuit.termination_a/termination_z pointers are maintained by CircuitTermination.save().
     They were previously written with a queryset update(), which emits no post_save and therefore
-    no ObjectChange, so consumers which replay the changelog never saw the association. (#22651)
+    no ObjectChange, so consumers which replay the changelog never saw the association. (#23134)
     """
     @classmethod
     def setUpTestData(cls):
@@ -314,7 +314,7 @@ class CircuitTerminationChangeLoggingTestCase(TestCase):
             action=ObjectChangeActionChoices.ACTION_UPDATE,
         ).order_by('pk')
 
-    @tag('regression')  # Ref: #22651
+    @tag('regression')  # Ref: #23134
     def test_creation_records_circuit_update(self):
         termination = self._tracked(lambda: CircuitTermination.objects.create(
             circuit=self.circuits[0], term_side='A', termination=self.sites[0],
@@ -325,7 +325,7 @@ class CircuitTerminationChangeLoggingTestCase(TestCase):
         self.assertIsNone(changes[0].prechange_data['termination_a'])
         self.assertEqual(changes[0].postchange_data['termination_a'], termination.pk)
 
-    @tag('regression')  # Ref: #22651
+    @tag('regression')  # Ref: #23134
     def test_second_termination_snapshots_current_state(self):
         # The A pointer is already committed when the Z termination is created; its prechange
         # snapshot must reflect that rather than a Circuit cached before the A write.
@@ -345,7 +345,7 @@ class CircuitTerminationChangeLoggingTestCase(TestCase):
         self.assertEqual(changes[0].postchange_data['termination_a'], termination_a.pk)
         self.assertEqual(changes[0].postchange_data['termination_z'], termination_z.pk)
 
-    @tag('regression')  # Ref: #22651
+    @tag('regression')  # Ref: #23134
     def test_circuit_change_records_both_circuits(self):
         termination = self._tracked(lambda: CircuitTermination.objects.create(
             circuit=self.circuits[0], term_side='A', termination=self.sites[0],
@@ -370,7 +370,7 @@ class CircuitTerminationChangeLoggingTestCase(TestCase):
         self.assertIsNone(new_changes[0].prechange_data['termination_a'])
         self.assertEqual(new_changes[0].postchange_data['termination_a'], termination.pk)
 
-    @tag('regression')  # Ref: #22651
+    @tag('regression')  # Ref: #23134
     def test_term_side_change_records_single_circuit_update(self):
         termination = self._tracked(lambda: CircuitTermination.objects.create(
             circuit=self.circuits[0], term_side='A', termination=self.sites[0],
@@ -389,6 +389,34 @@ class CircuitTerminationChangeLoggingTestCase(TestCase):
         self.assertIsNone(changes[0].postchange_data['termination_a'])
         self.assertEqual(changes[1].postchange_data['termination_z'], termination.pk)
 
+    @tag('regression')  # Ref: #23134
+    def test_pointer_already_set_records_no_circuit_update(self):
+        # bulk_create() bypasses save(), so the circuit's pointer is never written. Moving the
+        # termination afterwards reaches the clear path with the pointer already null.
+        CircuitTermination.objects.bulk_create([
+            CircuitTermination(circuit=self.circuits[0], term_side='A', termination=self.sites[0]),
+        ])
+        termination = CircuitTermination.objects.get(circuit=self.circuits[0], term_side='A')
+
+        def _move():
+            termination.circuit = self.circuits[1]
+            termination.save()
+
+        old_circuit_last_updated = Circuit.objects.get(pk=self.circuits[0].pk).last_updated
+
+        self._tracked(_move)
+
+        # The old circuit's pointer was already null, so it is not written to at all...
+        self.assertFalse(self._circuit_changes(self.circuits[0]).exists())
+        self.assertEqual(
+            Circuit.objects.get(pk=self.circuits[0].pk).last_updated, old_circuit_last_updated
+        )
+
+        # ...while the new circuit's pointer is set as usual.
+        new_changes = self._circuit_changes(self.circuits[1])
+        self.assertEqual(new_changes.count(), 1)
+        self.assertEqual(new_changes[0].postchange_data['termination_a'], termination.pk)
+
     def test_noop_resave_records_no_circuit_update(self):
         termination = self._tracked(lambda: CircuitTermination.objects.create(
             circuit=self.circuits[0], term_side='A', termination=self.sites[0],