From ab05234faf340a03cade4ae5145c20b11991403e Mon Sep 17 00:00:00 2001 From: Arthur Date: Fri, 4 Sep 2026 11:15:00 -0700 Subject: [PATCH] fixes --- netbox/circuits/models/circuits.py | 13 +++++----- netbox/circuits/tests/test_models.py | 38 ++++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/netbox/circuits/models/circuits.py b/netbox/circuits/models/circuits.py index e13eeb955..2128fc81a 100644 --- a/netbox/circuits/models/circuits.py +++ b/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. + so consumers which replay ObjectChange records 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. + 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: diff --git a/netbox/circuits/tests/test_models.py b/netbox/circuits/tests/test_models.py index a0c70e3e2..c319d0c48 100644 --- a/netbox/circuits/tests/test_models.py +++ b/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],