Merge ed9069449f into 2f79d987a8
This commit is contained in:
Arthur Hanson
GitHub
commit
2382d7e64d
|
|
@ -241,6 +241,16 @@ class CircuitGroupAssignment(CustomFieldsMixin, ExportTemplatesMixin, TagsMixin,
|
|||
return reverse('circuits:circuitgroupassignment', args=[self.pk])
|
||||
|
||||
|
||||
def _set_circuit_terminations(circuit, fields):
|
||||
"""
|
||||
Set or clear a Circuit's cached `termination_a`/`termination_z` fields, recording the change.
|
||||
"""
|
||||
circuit.snapshot()
|
||||
for field_name, value in fields.items():
|
||||
setattr(circuit, field_name, value)
|
||||
circuit.save(update_fields=[*fields, 'last_updated'])
|
||||
|
||||
|
||||
class CircuitTermination(
|
||||
CustomFieldsMixin,
|
||||
CustomLinksMixin,
|
||||
|
|
@ -403,18 +413,39 @@ class CircuitTermination(
|
|||
# Clear the old termination reference if circuit or term_side changed
|
||||
if circuit_changed or term_side_changed:
|
||||
old_termination_name = f'termination_{self._orig_term_side.lower()}'
|
||||
Circuit.objects.filter(pk=self._orig_circuit_id).update(**{old_termination_name: None})
|
||||
circuit = Circuit.objects.filter(
|
||||
pk=self._orig_circuit_id, **{old_termination_name: self.pk}
|
||||
).first()
|
||||
if circuit is not None:
|
||||
_set_circuit_terminations(circuit, {old_termination_name: None})
|
||||
|
||||
# Update the cache if this is a new termination or circuit/term_side changed
|
||||
if is_new or circuit_changed or term_side_changed:
|
||||
# Update the new circuit's termination reference
|
||||
termination_name = f'termination_{self.term_side.lower()}'
|
||||
Circuit.objects.filter(pk=self.circuit_id).update(**{termination_name: self.pk})
|
||||
_set_circuit_terminations(Circuit.objects.get(pk=self.circuit_id), {termination_name: self})
|
||||
|
||||
# Update cached values for subsequent saves
|
||||
self._orig_circuit_id = self.circuit_id
|
||||
self._orig_term_side = self.term_side
|
||||
|
||||
@classmethod
|
||||
def clear_cached_references(cls, instances, collector):
|
||||
# Called by CustomCollector ahead of the DELETE, for explicit and cascaded deletions alike
|
||||
pks = {instance.pk for instance in instances}
|
||||
doomed_circuits = {circuit.pk for circuit in collector.data.get(Circuit, ())}
|
||||
|
||||
circuits = Circuit.objects.filter(
|
||||
models.Q(termination_a__in=pks) | models.Q(termination_z__in=pks)
|
||||
).exclude(pk__in=doomed_circuits)
|
||||
|
||||
for circuit in circuits:
|
||||
_set_circuit_terminations(circuit, {
|
||||
name: None
|
||||
for name in ('termination_a', 'termination_z')
|
||||
if getattr(circuit, f'{name}_id') in pks
|
||||
})
|
||||
|
||||
def cache_related_objects(self):
|
||||
self._provider_network = self._region = self._site_group = self._site = self._location = None
|
||||
if self.termination_type:
|
||||
|
|
|
|||
|
|
@ -1,9 +1,15 @@
|
|||
import uuid
|
||||
|
||||
from django.contrib.contenttypes.models import ContentType
|
||||
from django.core.exceptions import NON_FIELD_ERRORS, ValidationError
|
||||
from django.test import TestCase
|
||||
from django.test import RequestFactory, TestCase, tag
|
||||
|
||||
from circuits.models import Circuit, CircuitTermination, CircuitType, Provider, ProviderNetwork
|
||||
from core.choices import ObjectChangeActionChoices
|
||||
from core.models import ObjectChange
|
||||
from dcim.models import Location, Region, Site, SiteGroup
|
||||
from netbox.context_managers import event_tracking
|
||||
from users.models import User
|
||||
|
||||
|
||||
class CircuitTerminationTestCase(TestCase):
|
||||
|
|
@ -270,3 +276,300 @@ class CircuitTerminationDenormalizationTriggerTestCase(TestCase):
|
|||
|
||||
termination.refresh_from_db()
|
||||
self.assertEqual(termination._region, region_b)
|
||||
|
||||
|
||||
class CircuitTerminationChangeLoggingTestCase(TestCase):
|
||||
"""
|
||||
Circuit.termination_a/termination_z are maintained by CircuitTermination.save(). Writing them
|
||||
with a queryset update() emitted no post_save, and so no ObjectChange. (#23134)
|
||||
"""
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
cls.user = User.objects.create_user(username='testuser', password='pw')
|
||||
|
||||
provider = Provider.objects.create(name='Provider 1', slug='provider-1')
|
||||
circuit_type = CircuitType.objects.create(name='Circuit Type 1', slug='circuit-type-1')
|
||||
|
||||
cls.sites = (
|
||||
Site.objects.create(name='Site 1', slug='site-1'),
|
||||
Site.objects.create(name='Site 2', slug='site-2'),
|
||||
)
|
||||
cls.circuits = (
|
||||
Circuit.objects.create(cid='Circuit 1', provider=provider, type=circuit_type),
|
||||
Circuit.objects.create(cid='Circuit 2', provider=provider, type=circuit_type),
|
||||
)
|
||||
|
||||
def _tracked(self, func):
|
||||
request = RequestFactory().get('/')
|
||||
request.id = uuid.uuid4()
|
||||
request.user = self.user
|
||||
with event_tracking(request):
|
||||
return func()
|
||||
|
||||
def _termination_change(self, termination_pk, action):
|
||||
return ObjectChange.objects.get(
|
||||
changed_object_type=ContentType.objects.get_for_model(CircuitTermination),
|
||||
changed_object_id=termination_pk,
|
||||
action=action,
|
||||
)
|
||||
|
||||
def _circuit_changes(self, circuit):
|
||||
return ObjectChange.objects.filter(
|
||||
changed_object_type=ContentType.objects.get_for_model(Circuit),
|
||||
changed_object_id=circuit.pk,
|
||||
action=ObjectChangeActionChoices.ACTION_UPDATE,
|
||||
).order_by('pk')
|
||||
|
||||
@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],
|
||||
))
|
||||
|
||||
changes = self._circuit_changes(self.circuits[0])
|
||||
self.assertEqual(changes.count(), 1)
|
||||
self.assertIsNone(changes[0].prechange_data['termination_a'])
|
||||
self.assertEqual(changes[0].postchange_data['termination_a'], termination.pk)
|
||||
|
||||
# The pointer references the termination's PK, so the create must be recorded first
|
||||
termination_create = self._termination_change(
|
||||
termination.pk, ObjectChangeActionChoices.ACTION_CREATE
|
||||
)
|
||||
self.assertLess(termination_create.pk, changes[0].pk)
|
||||
|
||||
@tag('regression') # Ref: #23134
|
||||
def test_second_termination_snapshots_current_state(self):
|
||||
# The A pointer is already committed when the Z termination is created
|
||||
termination_a = self._tracked(lambda: CircuitTermination.objects.create(
|
||||
circuit=self.circuits[0], term_side='A', termination=self.sites[0],
|
||||
))
|
||||
ObjectChange.objects.all().delete()
|
||||
|
||||
termination_z = self._tracked(lambda: CircuitTermination.objects.create(
|
||||
circuit=self.circuits[0], term_side='Z', termination=self.sites[1],
|
||||
))
|
||||
|
||||
changes = self._circuit_changes(self.circuits[0])
|
||||
self.assertEqual(changes.count(), 1)
|
||||
self.assertEqual(changes[0].prechange_data['termination_a'], termination_a.pk)
|
||||
self.assertIsNone(changes[0].prechange_data['termination_z'])
|
||||
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: #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],
|
||||
))
|
||||
ObjectChange.objects.all().delete()
|
||||
|
||||
def _move():
|
||||
termination.circuit = self.circuits[1]
|
||||
termination.save()
|
||||
|
||||
self._tracked(_move)
|
||||
|
||||
# The old circuit's pointer is cleared
|
||||
old_changes = self._circuit_changes(self.circuits[0])
|
||||
self.assertEqual(old_changes.count(), 1)
|
||||
self.assertEqual(old_changes[0].prechange_data['termination_a'], termination.pk)
|
||||
self.assertIsNone(old_changes[0].postchange_data['termination_a'])
|
||||
|
||||
# The new circuit's pointer is set
|
||||
new_changes = self._circuit_changes(self.circuits[1])
|
||||
self.assertEqual(new_changes.count(), 1)
|
||||
self.assertIsNone(new_changes[0].prechange_data['termination_a'])
|
||||
self.assertEqual(new_changes[0].postchange_data['termination_a'], termination.pk)
|
||||
|
||||
@tag('regression') # Ref: #23134
|
||||
def test_term_side_change_records_circuit_updates(self):
|
||||
termination = self._tracked(lambda: CircuitTermination.objects.create(
|
||||
circuit=self.circuits[0], term_side='A', termination=self.sites[0],
|
||||
))
|
||||
ObjectChange.objects.all().delete()
|
||||
|
||||
def _flip():
|
||||
termination.term_side = 'Z'
|
||||
termination.save()
|
||||
|
||||
self._tracked(_flip)
|
||||
|
||||
# The old pointer is cleared, then the new one is set
|
||||
changes = self._circuit_changes(self.circuits[0])
|
||||
self.assertEqual(changes.count(), 2)
|
||||
self.assertEqual(changes[0].prechange_data['termination_a'], termination.pk)
|
||||
self.assertIsNone(changes[0].postchange_data['termination_a'])
|
||||
self.assertIsNone(changes[1].prechange_data['termination_z'])
|
||||
self.assertEqual(changes[1].postchange_data['termination_z'], termination.pk)
|
||||
|
||||
@tag('regression') # Ref: #23134
|
||||
def test_creation_leaves_another_terminations_pointer_alone(self):
|
||||
# A pointer referencing a different termination must never be cleared
|
||||
termination_a = self._tracked(lambda: CircuitTermination.objects.create(
|
||||
circuit=self.circuits[0], term_side='A', termination=self.sites[0],
|
||||
))
|
||||
ObjectChange.objects.all().delete()
|
||||
|
||||
def _create():
|
||||
termination = CircuitTermination(
|
||||
circuit=self.circuits[0], term_side='A', termination=self.sites[1],
|
||||
)
|
||||
termination.term_side = 'Z'
|
||||
termination.save()
|
||||
return termination
|
||||
|
||||
termination_z = self._tracked(_create)
|
||||
|
||||
self.circuits[0].refresh_from_db()
|
||||
self.assertEqual(self.circuits[0].termination_a_id, termination_a.pk)
|
||||
self.assertEqual(self.circuits[0].termination_z_id, termination_z.pk)
|
||||
|
||||
changes = self._circuit_changes(self.circuits[0])
|
||||
self.assertEqual(changes.count(), 1)
|
||||
self.assertEqual(changes[0].postchange_data['termination_a'], termination_a.pk)
|
||||
|
||||
@tag('regression') # Ref: #23134
|
||||
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],
|
||||
))
|
||||
ObjectChange.objects.all().delete()
|
||||
|
||||
self._tracked(termination.save)
|
||||
|
||||
self.assertFalse(self._circuit_changes(self.circuits[0]).exists())
|
||||
|
||||
@tag('regression') # Ref: #23134
|
||||
def test_circuit_change_via_update_fields_records_circuit_update(self):
|
||||
# save(update_fields=...) takes its own branch when deciding what is being persisted
|
||||
termination = self._tracked(lambda: CircuitTermination.objects.create(
|
||||
circuit=self.circuits[0], term_side='A', termination=self.sites[0],
|
||||
))
|
||||
ObjectChange.objects.all().delete()
|
||||
|
||||
def _move():
|
||||
termination.circuit = self.circuits[1]
|
||||
termination.save(update_fields=('circuit',))
|
||||
|
||||
self._tracked(_move)
|
||||
|
||||
old_changes = self._circuit_changes(self.circuits[0])
|
||||
self.assertEqual(old_changes.count(), 1)
|
||||
self.assertIsNone(old_changes[0].postchange_data['termination_a'])
|
||||
|
||||
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)
|
||||
|
||||
@tag('regression') # Ref: #23134
|
||||
def test_deletion_records_circuit_update(self):
|
||||
termination = self._tracked(lambda: CircuitTermination.objects.create(
|
||||
circuit=self.circuits[0], term_side='A', termination=self.sites[0],
|
||||
))
|
||||
ObjectChange.objects.all().delete()
|
||||
termination_pk = termination.pk
|
||||
|
||||
self._tracked(termination.delete)
|
||||
|
||||
self.circuits[0].refresh_from_db()
|
||||
self.assertIsNone(self.circuits[0].termination_a_id)
|
||||
|
||||
changes = self._circuit_changes(self.circuits[0])
|
||||
self.assertEqual(changes.count(), 1)
|
||||
self.assertEqual(changes[0].prechange_data['termination_a'], termination_pk)
|
||||
self.assertIsNone(changes[0].postchange_data['termination_a'])
|
||||
|
||||
# The pointer clear must precede the DELETE, so that a consumer replaying in reverse
|
||||
# restores the termination before the record which references it
|
||||
termination_delete = self._termination_change(
|
||||
termination_pk, ObjectChangeActionChoices.ACTION_DELETE
|
||||
)
|
||||
self.assertLess(changes[0].pk, termination_delete.pk)
|
||||
|
||||
@tag('regression') # Ref: #23134
|
||||
def test_bulk_deletion_records_circuit_update(self):
|
||||
# NetBox's bulk delete views iterate obj.delete() rather than calling queryset.delete()
|
||||
termination = self._tracked(lambda: CircuitTermination.objects.create(
|
||||
circuit=self.circuits[0], term_side='A', termination=self.sites[0],
|
||||
))
|
||||
ObjectChange.objects.all().delete()
|
||||
termination_pk = termination.pk
|
||||
|
||||
def _bulk_delete():
|
||||
for obj in CircuitTermination.objects.filter(pk=termination_pk):
|
||||
obj.delete()
|
||||
|
||||
self._tracked(_bulk_delete)
|
||||
|
||||
changes = self._circuit_changes(self.circuits[0])
|
||||
self.assertEqual(changes.count(), 1)
|
||||
self.assertEqual(changes[0].prechange_data['termination_a'], termination_pk)
|
||||
self.assertIsNone(changes[0].postchange_data['termination_a'])
|
||||
|
||||
@tag('regression') # Ref: #23134
|
||||
def test_deletion_resolves_the_pointer_from_the_database(self):
|
||||
# The pointer cleared is the one which references this termination, not the one named by
|
||||
# a stale in-memory term_side
|
||||
termination_a = self._tracked(lambda: CircuitTermination.objects.create(
|
||||
circuit=self.circuits[0], term_side='A', termination=self.sites[0],
|
||||
))
|
||||
termination_z = self._tracked(lambda: CircuitTermination.objects.create(
|
||||
circuit=self.circuits[0], term_side='Z', termination=self.sites[1],
|
||||
))
|
||||
ObjectChange.objects.all().delete()
|
||||
termination_z_pk = termination_z.pk
|
||||
|
||||
termination_z.term_side = 'A'
|
||||
self._tracked(termination_z.delete)
|
||||
|
||||
self.circuits[0].refresh_from_db()
|
||||
self.assertEqual(self.circuits[0].termination_a_id, termination_a.pk)
|
||||
self.assertIsNone(self.circuits[0].termination_z_id)
|
||||
|
||||
changes = self._circuit_changes(self.circuits[0])
|
||||
self.assertEqual(changes.count(), 1)
|
||||
self.assertEqual(changes[0].prechange_data['termination_z'], termination_z_pk)
|
||||
self.assertIsNone(changes[0].postchange_data['termination_z'])
|
||||
self.assertEqual(changes[0].postchange_data['termination_a'], termination_a.pk)
|
||||
|
||||
@tag('regression') # Ref: #23134
|
||||
def test_cascade_deletion_records_circuit_update(self):
|
||||
# Deleting the terminating Site reaches the termination through the collector, which does
|
||||
# not call delete(). Both sides go in one record.
|
||||
termination_a = self._tracked(lambda: CircuitTermination.objects.create(
|
||||
circuit=self.circuits[0], term_side='A', termination=self.sites[0],
|
||||
))
|
||||
termination_z = self._tracked(lambda: CircuitTermination.objects.create(
|
||||
circuit=self.circuits[0], term_side='Z', termination=self.sites[0],
|
||||
))
|
||||
ObjectChange.objects.all().delete()
|
||||
termination_a_pk, termination_z_pk = termination_a.pk, termination_z.pk
|
||||
|
||||
self._tracked(self.sites[0].delete)
|
||||
|
||||
self.circuits[0].refresh_from_db()
|
||||
self.assertIsNone(self.circuits[0].termination_a_id)
|
||||
self.assertIsNone(self.circuits[0].termination_z_id)
|
||||
|
||||
changes = self._circuit_changes(self.circuits[0])
|
||||
self.assertEqual(changes.count(), 1)
|
||||
self.assertEqual(changes[0].prechange_data['termination_a'], termination_a_pk)
|
||||
self.assertEqual(changes[0].prechange_data['termination_z'], termination_z_pk)
|
||||
self.assertIsNone(changes[0].postchange_data['termination_a'])
|
||||
self.assertIsNone(changes[0].postchange_data['termination_z'])
|
||||
|
||||
# The clear must precede the DELETEs which the cascade emits for the terminations
|
||||
termination_delete = self._termination_change(
|
||||
termination_a_pk, ObjectChangeActionChoices.ACTION_DELETE
|
||||
)
|
||||
self.assertLess(changes[0].pk, termination_delete.pk)
|
||||
|
||||
def test_circuit_deletion_records_no_pointer_update(self):
|
||||
self._tracked(lambda: CircuitTermination.objects.create(
|
||||
circuit=self.circuits[0], term_side='A', termination=self.sites[0],
|
||||
))
|
||||
ObjectChange.objects.all().delete()
|
||||
|
||||
self._tracked(self.circuits[0].delete)
|
||||
|
||||
self.assertFalse(self._circuit_changes(self.circuits[0]).exists())
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import logging
|
||||
|
||||
from django.contrib.contenttypes.fields import GenericRelation
|
||||
from django.db import router
|
||||
from django.db import router, transaction
|
||||
from django.db.models.deletion import CASCADE, Collector
|
||||
from django.utils.translation import gettext as _
|
||||
|
||||
|
|
@ -126,6 +126,18 @@ class CustomCollector(Collector):
|
|||
# Add the model that the generic relation points to as a dependency
|
||||
self.add_dependency(field.related_model, instance, reverse_dependency=True)
|
||||
|
||||
def delete(self):
|
||||
# Clear any cached references to the objects being deleted first, so that each clear is
|
||||
# recorded and precedes the DELETE. Django nulls a SET_NULL column with a bulk UPDATE,
|
||||
# which emits no post_save and so is never change-logged. Models opt in by defining
|
||||
# clear_cached_references(); cascaded objects reach this the same as explicit deletions.
|
||||
with transaction.atomic(using=self.using):
|
||||
for model, instances in self.data.items():
|
||||
if clear_references := getattr(model, 'clear_cached_references', None):
|
||||
clear_references(instances, self)
|
||||
|
||||
return super().delete()
|
||||
|
||||
|
||||
class DeleteMixin:
|
||||
"""
|
||||
|
|
|
|||
Loading…
Reference in New Issue