Skip descendant and component denormalization updates when a Location, Rack, or Device is saved without changing its scope assignments. Handle partial saves safely to avoid propagating stale in-memory values.
This commit is contained in:
Jeremy Stretch
GitHub
parent
43fe81f9e8
commit
05e35e9264
|
|
@ -53,13 +53,21 @@ COMPONENT_MODELS = (
|
|||
RearPort,
|
||||
)
|
||||
|
||||
# The scope-relevant fields stashed before each model's save by cache_presave_scope_fields(),
|
||||
# so that the post_save handlers can tell whether the save actually changed any of them and
|
||||
# skip their work when it did not.
|
||||
STASHED_SCOPE_FIELDS = {
|
||||
Site: ('region_id', 'group_id'),
|
||||
Location: ('site_id',),
|
||||
Rack: ('site_id', 'location_id'),
|
||||
Device: ('site_id', 'location_id', 'rack_id'),
|
||||
}
|
||||
|
||||
|
||||
#
|
||||
# Location/rack/device assignment
|
||||
#
|
||||
|
||||
@receiver(pre_save, sender=Location)
|
||||
@receiver(pre_save, sender=Site)
|
||||
def cache_presave_scope_fields(instance, raw=False, using=None, **kwargs):
|
||||
"""
|
||||
Stash the scope-relevant field values currently in the database so that the post_save
|
||||
|
|
@ -67,31 +75,108 @@ def cache_presave_scope_fields(instance, raw=False, using=None, **kwargs):
|
|||
locks the row, so overlapping saves of the same object serialize here and the
|
||||
comparison always runs against the final committed state.
|
||||
|
||||
Outside of a transaction no stash is taken (and any stash left by a previous
|
||||
transactional save of the same instance is cleared): in autocommit, this read and the
|
||||
subsequent UPDATE would run in separate transactions, so the comparison could race a
|
||||
concurrent save. The post_save handlers treat a missing stash as "the values may have
|
||||
changed" and rebuild or repair unconditionally.
|
||||
No stash is taken for a raw save, for a new instance, or outside a transaction: in
|
||||
autocommit, this read and the subsequent UPDATE would run in separate transactions, so
|
||||
the comparison could race a concurrent save. In each of those cases any stash left by a
|
||||
previous save of the same instance is cleared, as it no longer reflects the current
|
||||
database state. The post_save handlers treat a missing stash as "the values may have
|
||||
changed" and rebuild or repair unconditionally — except on a raw save, which they skip
|
||||
before consulting the stash at all, making the clearing there purely defensive.
|
||||
"""
|
||||
if raw or instance.pk is None:
|
||||
return
|
||||
if not transaction.get_connection(using).in_atomic_block:
|
||||
if raw or instance.pk is None or not transaction.get_connection(using).in_atomic_block:
|
||||
# Clear any stash left by a previous save of this instance.
|
||||
instance._presave_scope_fields = None
|
||||
return
|
||||
fields = ('region_id', 'group_id') if isinstance(instance, Site) else ('site_id',)
|
||||
fields = STASHED_SCOPE_FIELDS[instance.__class__]
|
||||
instance._presave_scope_fields = (
|
||||
instance.__class__.objects.using(using)
|
||||
.filter(pk=instance.pk)
|
||||
# no_key: serializes overlapping saves of this object without blocking foreign
|
||||
# key inserts that reference it
|
||||
.select_for_update(no_key=True)
|
||||
.order_by() # Clear default ordering to avoid JOINs
|
||||
.select_for_update(no_key=True) # no_key: Avoid blocking foreign key inserts that reference this object
|
||||
.values(*fields)
|
||||
.first()
|
||||
)
|
||||
|
||||
|
||||
for _model in STASHED_SCOPE_FIELDS:
|
||||
pre_save.connect(cache_presave_scope_fields, sender=_model)
|
||||
|
||||
|
||||
# update_fields may name a foreign key by either its name ('site') or its attname
|
||||
# ('site_id') — Django accepts both — so deciding whether a save wrote a stashed field has
|
||||
# to test both forms. Derived from each model's own meta rather than written out, so the two
|
||||
# spellings cannot disagree.
|
||||
STASHED_FIELD_ALIASES = {
|
||||
model: {
|
||||
field.attname: frozenset((field.attname, field.name))
|
||||
for field in model._meta.concrete_fields
|
||||
if field.attname in fields
|
||||
}
|
||||
for model, fields in STASHED_SCOPE_FIELDS.items()
|
||||
}
|
||||
|
||||
|
||||
def _unwritten_scope_fields(instance, update_fields):
|
||||
"""
|
||||
Return the scope-relevant fields listed for the instance's model which this save did
|
||||
not write.
|
||||
"""
|
||||
if update_fields is None:
|
||||
return frozenset()
|
||||
aliases = STASHED_FIELD_ALIASES[instance.__class__]
|
||||
return frozenset(field for field, names in aliases.items() if names.isdisjoint(update_fields))
|
||||
|
||||
|
||||
def _scope_fields_unchanged(instance, update_fields=None):
|
||||
"""
|
||||
Return True when the values stashed immediately before this save show that it changed
|
||||
none of the scope-relevant fields listed for the instance's model, meaning the caller's
|
||||
propagation or rebuild can be skipped in its entirety.
|
||||
"""
|
||||
prev = getattr(instance, '_presave_scope_fields', None)
|
||||
if prev is None:
|
||||
return False
|
||||
unwritten = _unwritten_scope_fields(instance, update_fields)
|
||||
return all(value == getattr(instance, field) for field, value in prev.items() if field not in unwritten)
|
||||
|
||||
|
||||
def _scope_values(instance, update_fields, using):
|
||||
"""
|
||||
Return the values the scope-relevant fields hold in the database once this save has
|
||||
been applied, keyed by field name, for the propagation handlers to push down.
|
||||
|
||||
Must be called inside the transaction the propagation runs in: the fallback read below
|
||||
locks the row for the remainder of it, so that no concurrent write can move the object
|
||||
out from under the values being propagated.
|
||||
|
||||
Returns None when the row cannot be read at all, leaving the caller nothing to
|
||||
propagate.
|
||||
"""
|
||||
values = {field: getattr(instance, field) for field in STASHED_SCOPE_FIELDS[instance.__class__]}
|
||||
unwritten = _unwritten_scope_fields(instance, update_fields)
|
||||
if not unwritten:
|
||||
return values
|
||||
stashed = getattr(instance, '_presave_scope_fields', None)
|
||||
if stashed is None:
|
||||
stashed = (
|
||||
instance.__class__.objects.using(using)
|
||||
.filter(pk=instance.pk)
|
||||
# Cleared for the same reason as in cache_presave_scope_fields().
|
||||
.order_by()
|
||||
.select_for_update(no_key=True)
|
||||
.values(*unwritten)
|
||||
.first()
|
||||
)
|
||||
# No row to read: it was deleted after this save committed, or was never inserted
|
||||
# (an instance with a pre-assigned primary key).
|
||||
if stashed is None:
|
||||
return None
|
||||
values.update({field: stashed[field] for field in unwritten})
|
||||
return values
|
||||
|
||||
|
||||
@receiver(post_save, sender=Location)
|
||||
def handle_location_site_change(instance, created, using=None, **kwargs):
|
||||
def handle_location_site_change(instance, created, raw=False, using=None, update_fields=None, **kwargs):
|
||||
"""
|
||||
Update child objects when a Location is saved. All updates are queryset update() calls,
|
||||
which fire no signals and generate no change records for the affected objects.
|
||||
|
|
@ -102,107 +187,127 @@ def handle_location_site_change(instance, created, using=None, **kwargs):
|
|||
transaction opened here. For the same reason the new Site is assigned by ID: reading
|
||||
instance.site would fetch the related object over a router-selected connection whenever
|
||||
the save left it uncached (a rename, say).
|
||||
|
||||
When the values read from the database immediately before this save show that the Site
|
||||
assignment is unchanged, the propagation is skipped: every value written below is
|
||||
derived from it, so there is nothing for the descendants to pick up. A raw save is
|
||||
skipped outright.
|
||||
"""
|
||||
if created:
|
||||
if created or raw:
|
||||
return
|
||||
|
||||
# Skip the propagation when this save left the Site assignment untouched.
|
||||
if _scope_fields_unchanged(instance, update_fields):
|
||||
return
|
||||
|
||||
with transaction.atomic(using=using, savepoint=False):
|
||||
instance.get_descendants().using(using).update(site_id=instance.site_id)
|
||||
scope = _scope_values(instance, update_fields, using)
|
||||
if scope is None:
|
||||
return
|
||||
site_id = scope['site_id']
|
||||
instance.get_descendants().using(using).update(site_id=site_id)
|
||||
# Materialized once so every statement below sees the same membership, even if a
|
||||
# concurrent commit renumbers the tree mid-handler.
|
||||
locations = list(instance.get_descendants(include_self=True).using(using).values_list('pk', flat=True))
|
||||
Rack.objects.using(using).filter(location__in=locations).update(site_id=instance.site_id)
|
||||
Device.objects.using(using).filter(location__in=locations).update(site_id=instance.site_id)
|
||||
PowerPanel.objects.using(using).filter(location__in=locations).update(site_id=instance.site_id)
|
||||
CableTermination.objects.using(using).filter(_location__in=locations).update(_site_id=instance.site_id)
|
||||
Rack.objects.using(using).filter(location__in=locations).update(site_id=site_id)
|
||||
Device.objects.using(using).filter(location__in=locations).update(site_id=site_id)
|
||||
PowerPanel.objects.using(using).filter(location__in=locations).update(site_id=site_id)
|
||||
CableTermination.objects.using(using).filter(_location__in=locations).update(_site_id=site_id)
|
||||
# Update component models for devices in these locations
|
||||
for model in COMPONENT_MODELS:
|
||||
model.objects.using(using).filter(device__location__in=locations).update(_site_id=instance.site_id)
|
||||
model.objects.using(using).filter(device__location__in=locations).update(_site_id=site_id)
|
||||
|
||||
# Objects scoped to descendant Locations receive no post_save of their own from the
|
||||
# queryset updates above, so their cached scope fields are updated here whenever the
|
||||
# Site assignment has actually changed. (Objects scoped to this Location itself are
|
||||
# recomputed by sync_cached_scope_fields on this same save.) Values are read fresh
|
||||
# from the database rather than taken from the saved instance, whose cached site
|
||||
# relation may be stale.
|
||||
prev = getattr(instance, '_presave_scope_fields', None)
|
||||
if prev is None or prev['site_id'] != instance.site_id:
|
||||
# Lock the destination Site (without blocking FK inserts that reference it) so
|
||||
# a concurrent scope change on that Site serializes against this move; an
|
||||
# unlocked read could stamp region/group values from before that change.
|
||||
site = (
|
||||
Site.objects.using(using)
|
||||
.filter(pk=instance.site_id)
|
||||
.select_for_update(no_key=True)
|
||||
.values('region_id', 'group_id')
|
||||
.first()
|
||||
)
|
||||
if site is not None:
|
||||
# Select rows through the authoritative scope rather than the cached
|
||||
# _location, which may itself be stale; scope_id doubles as the correct
|
||||
# _location value for Location-scoped rows.
|
||||
# The content type is read on the saving connection as well, since its ID is
|
||||
# fed straight into the pinned filter below; a router-selected read could
|
||||
# return an ID which means something else on that connection.
|
||||
location_ct = ContentType.objects.db_manager(using).get_for_model(Location)
|
||||
for model in (Prefix, Cluster, WirelessLAN):
|
||||
model.objects.using(using).filter(scope_type=location_ct, scope_id__in=locations).update(
|
||||
_location_id=F('scope_id'),
|
||||
_site_id=instance.site_id,
|
||||
_region_id=site['region_id'],
|
||||
_site_group_id=site['group_id'],
|
||||
)
|
||||
|
||||
# CircuitTermination caches the same ancestry under its own generic
|
||||
# termination field rather than CachedScopeMixin.scope, so it is invisible to
|
||||
# both the loop above and sync_cached_scope_fields(). Its own rows are
|
||||
# refreshed by the denormalized-field registry, but only their _site: _region
|
||||
# and _site_group are mapped off the separate _site registration, which fires
|
||||
# on a Site save. Rows scoped to descendant Locations get nothing at all, as
|
||||
# the get_descendants() update above fires no post_save — which is why the
|
||||
# include_self=True membership is load-bearing here.
|
||||
CircuitTermination.objects.using(using).filter(
|
||||
termination_type=location_ct, termination_id__in=locations
|
||||
).update(
|
||||
_location_id=F('termination_id'),
|
||||
_site_id=instance.site_id,
|
||||
# queryset updates above, so their cached scope fields are updated here.
|
||||
site = (
|
||||
Site.objects.using(using)
|
||||
.filter(pk=site_id)
|
||||
.select_for_update(no_key=True) # Lock the destination Site (without blocking FK inserts that reference it)
|
||||
.values('region_id', 'group_id')
|
||||
.first()
|
||||
)
|
||||
if site is not None:
|
||||
location_ct = ContentType.objects.db_manager(using).get_for_model(Location)
|
||||
for model in (Prefix, Cluster, WirelessLAN):
|
||||
model.objects.using(using).filter(scope_type=location_ct, scope_id__in=locations).update(
|
||||
_location_id=F('scope_id'),
|
||||
_site_id=site_id,
|
||||
_region_id=site['region_id'],
|
||||
_site_group_id=site['group_id'],
|
||||
)
|
||||
|
||||
# CircuitTermination caches the same ancestry under its own generic
|
||||
# termination field rather than CachedScopeMixin.scope, so it is invisible to
|
||||
# both the loop above and sync_cached_scope_fields().
|
||||
CircuitTermination.objects.using(using).filter(
|
||||
termination_type=location_ct, termination_id__in=locations
|
||||
).update(
|
||||
_location_id=F('termination_id'),
|
||||
_site_id=site_id,
|
||||
_region_id=site['region_id'],
|
||||
_site_group_id=site['group_id'],
|
||||
)
|
||||
|
||||
|
||||
@receiver(post_save, sender=Rack)
|
||||
def handle_rack_site_change(instance, created, using=None, **kwargs):
|
||||
def handle_rack_site_change(instance, created, raw=False, using=None, update_fields=None, **kwargs):
|
||||
"""
|
||||
Update child Devices if Site or Location assignment has changed. Queries are pinned to
|
||||
the connection the Rack was saved on, and the new values are assigned by ID so that no
|
||||
related object is fetched over a router-selected connection.
|
||||
|
||||
A save which changed neither assignment propagates nothing and is skipped, as does a
|
||||
raw save.
|
||||
"""
|
||||
if not created:
|
||||
if created or raw:
|
||||
return
|
||||
|
||||
# Skip the propagation when this save left the Site and Location assignments untouched.
|
||||
if _scope_fields_unchanged(instance, update_fields):
|
||||
return
|
||||
|
||||
with transaction.atomic(using=using, savepoint=False):
|
||||
scope = _scope_values(instance, update_fields, using)
|
||||
if scope is None:
|
||||
return
|
||||
Device.objects.using(using).filter(rack=instance).update(
|
||||
site_id=instance.site_id,
|
||||
location_id=instance.location_id,
|
||||
site_id=scope['site_id'],
|
||||
location_id=scope['location_id'],
|
||||
)
|
||||
# Update component models for devices in this rack
|
||||
for model in COMPONENT_MODELS:
|
||||
model.objects.using(using).filter(device__rack=instance).update(
|
||||
_site_id=instance.site_id,
|
||||
_location_id=instance.location_id,
|
||||
_site_id=scope['site_id'],
|
||||
_location_id=scope['location_id'],
|
||||
)
|
||||
|
||||
|
||||
@receiver(post_save, sender=Device)
|
||||
def handle_device_site_change(instance, created, using=None, **kwargs):
|
||||
def handle_device_site_change(instance, created, raw=False, using=None, update_fields=None, **kwargs):
|
||||
"""
|
||||
Update child components to update the parent Site, Location, and Rack when a Device is saved.
|
||||
Queries are pinned to the connection the Device was saved on, and the new values are
|
||||
assigned by ID so that no related object is fetched over a router-selected connection.
|
||||
|
||||
A save which changed none of the three assignments propagates nothing and is skipped,
|
||||
as does a raw save.
|
||||
"""
|
||||
if not created:
|
||||
if created or raw:
|
||||
return
|
||||
|
||||
# Skip the propagation when this save left the Site, Location, and Rack assignments untouched.
|
||||
if _scope_fields_unchanged(instance, update_fields):
|
||||
return
|
||||
|
||||
with transaction.atomic(using=using, savepoint=False):
|
||||
scope = _scope_values(instance, update_fields, using)
|
||||
if scope is None:
|
||||
return
|
||||
for model in COMPONENT_MODELS:
|
||||
model.objects.using(using).filter(device=instance).update(
|
||||
_site_id=instance.site_id,
|
||||
_location_id=instance.location_id,
|
||||
_rack_id=instance.rack_id,
|
||||
_site_id=scope['site_id'],
|
||||
_location_id=scope['location_id'],
|
||||
_rack_id=scope['rack_id'],
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -364,18 +469,20 @@ def _get_scope_object(scope_type_id, scope_id, using):
|
|||
|
||||
@receiver(post_save, sender=Location)
|
||||
@receiver(post_save, sender=Site)
|
||||
def sync_cached_scope_fields(instance, created, using=None, **kwargs):
|
||||
def sync_cached_scope_fields(instance, created, raw=False, using=None, update_fields=None, **kwargs):
|
||||
"""
|
||||
Rebuild cached scope fields for all CachedScopeMixin-based models
|
||||
affected by a change to a Site or Location.
|
||||
|
||||
When the values read from the database immediately before this save
|
||||
show that no scope-relevant field has changed, the rebuild is
|
||||
skipped. Otherwise, cached fields are recomputed from each object's
|
||||
authoritative scope relationships — never copied from the saved
|
||||
instance — so rows holding stale cached values are also repaired.
|
||||
skipped, as is a raw save. Otherwise, cached fields are recomputed
|
||||
from each object's authoritative scope relationships — never copied
|
||||
from the saved instance — so rows holding stale cached values are
|
||||
also repaired. A partial save is judged on the fields it actually
|
||||
wrote.
|
||||
"""
|
||||
if created:
|
||||
if created or raw:
|
||||
return
|
||||
|
||||
if isinstance(instance, Location):
|
||||
|
|
@ -385,19 +492,10 @@ def sync_cached_scope_fields(instance, created, using=None, **kwargs):
|
|||
else:
|
||||
return
|
||||
|
||||
# Skip the rebuild when this save changed no scope-relevant field. The pre-save values
|
||||
# are read from the database by cache_presave_scope_fields() immediately before the
|
||||
# write, with the row locked, so the comparison holds even when overlapping saves race
|
||||
# on the same object. The stash exists only for saves made inside a transaction; when
|
||||
# it's absent (autocommit saves), rebuild unconditionally.
|
||||
prev = getattr(instance, '_presave_scope_fields', None)
|
||||
if prev is not None:
|
||||
if isinstance(instance, Site):
|
||||
if prev['region_id'] == instance.region_id and prev['group_id'] == instance.group_id:
|
||||
return
|
||||
# The dispatch above ensures the instance can only be a Location here
|
||||
elif prev['site_id'] == instance.site_id:
|
||||
return
|
||||
# Skip the rebuild when this save changed no scope-relevant field. The rebuild reads
|
||||
# each row's own scope rather than the instance, so it needs no _scope_values() here.
|
||||
if _scope_fields_unchanged(instance, update_fields):
|
||||
return
|
||||
|
||||
# These models are explicitly listed because they all subclass CachedScopeMixin
|
||||
# and therefore require their cached scope fields to be recomputed.
|
||||
|
|
|
|||
|
|
@ -35,13 +35,44 @@ from utilities.testing import PinnedConnectionRouter
|
|||
from virtualization.models import Cluster, ClusterType
|
||||
from wireless.models import WirelessLAN
|
||||
|
||||
COMPONENT_TABLES = frozenset(model._meta.db_table for model in signals.COMPONENT_MODELS)
|
||||
|
||||
class LocationSiteChangeSignalTestCase(TestCase):
|
||||
|
||||
class ScopePropagationCaptureMixin:
|
||||
"""
|
||||
Helper for asserting whether a save propagated to the tables its post_save handler
|
||||
rewrites.
|
||||
|
||||
dcim_cabletermination is never among them: the denormalized-field registry
|
||||
(netbox.denormalized) rewrites it on Location, Rack, and Device saves alike, so it
|
||||
cannot distinguish a propagation from a plain save. Neither is the saved object's own
|
||||
table, which carries the save's own UPDATE.
|
||||
"""
|
||||
propagation_tables = frozenset()
|
||||
|
||||
def capture_propagation_updates(self, obj, raw=False, update_fields=None):
|
||||
with CaptureQueriesContext(connection) as ctx:
|
||||
if raw:
|
||||
obj.save_base(raw=True)
|
||||
elif update_fields is not None:
|
||||
obj.save(update_fields=update_fields)
|
||||
else:
|
||||
obj.save()
|
||||
|
||||
return {
|
||||
table for table in self.propagation_tables
|
||||
for q in ctx.captured_queries
|
||||
if q['sql'].startswith(f'UPDATE "{table}"')
|
||||
}
|
||||
|
||||
|
||||
class LocationSiteChangeSignalTestCase(ScopePropagationCaptureMixin, TestCase):
|
||||
"""
|
||||
Verify dcim.signals.handle_location_site_change propagates a Location's new Site to
|
||||
every descendant Location, Rack, Device, PowerPanel, and component when the parent
|
||||
Location's site assignment changes.
|
||||
"""
|
||||
propagation_tables = COMPONENT_TABLES | {'dcim_rack', 'dcim_device', 'dcim_powerpanel'}
|
||||
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
|
|
@ -125,12 +156,157 @@ class LocationSiteChangeSignalTestCase(TestCase):
|
|||
# Should not raise — newly-created locations have no descendants.
|
||||
Location.objects.create(name='New', slug='new', site=self.site_a)
|
||||
|
||||
def _seed_location_with_children(self):
|
||||
location = Location.objects.create(name='Parent', slug='parent', site=self.site_a)
|
||||
device = Device.objects.create(
|
||||
name='Device',
|
||||
site=self.site_a,
|
||||
location=location,
|
||||
device_type=self.device_type,
|
||||
role=self.device_role,
|
||||
)
|
||||
Interface.objects.create(device=device, name='Interface 1')
|
||||
Rack.objects.create(name='Rack', site=self.site_a, location=location)
|
||||
PowerPanel.objects.create(name='Panel', site=self.site_a, location=location)
|
||||
return location
|
||||
|
||||
class RackSiteChangeSignalTestCase(TestCase):
|
||||
def test_unchanged_site_skips_propagation(self):
|
||||
# Every value the handler writes is derived from the Location's site assignment, so a
|
||||
# save which leaves it alone has nothing to propagate and must not rewrite a single
|
||||
# descendant row. Rewriting them is not merely wasted work: PostgreSQL writes a new
|
||||
# tuple version for every row an UPDATE matches, and holds a row lock on each for the
|
||||
# remainder of the transaction.
|
||||
location = self._seed_location_with_children()
|
||||
location.description = 'updated'
|
||||
|
||||
self.assertEqual(self.capture_propagation_updates(location), set())
|
||||
|
||||
def test_changed_site_propagates(self):
|
||||
# Counterpart to the test above, which would pass vacuously if these UPDATEs stopped
|
||||
# being issued (or their tables were renamed) rather than merely being skipped.
|
||||
location = self._seed_location_with_children()
|
||||
location.site = self.site_b
|
||||
|
||||
self.assertEqual(self.capture_propagation_updates(location), self.propagation_tables)
|
||||
|
||||
def test_raw_save_skips_propagation(self):
|
||||
# raw=True is set only by Django's loaddata pathway, whose fixture already carries the
|
||||
# denormalized values for every object it loads, so the propagation would rewrite each
|
||||
# matched row with what it already holds. netbox.denormalized.update_denormalized_fields()
|
||||
# returns early on raw for the same reason.
|
||||
location = self._seed_location_with_children()
|
||||
location.site = self.site_b
|
||||
|
||||
self.assertEqual(self.capture_propagation_updates(location, raw=True), set())
|
||||
|
||||
def test_stale_partial_save_does_not_propagate_an_unwritten_site(self):
|
||||
# A save passing update_fields writes only the fields it names, so an omitted field
|
||||
# keeps whatever the database holds no matter what the instance carries. This instance
|
||||
# was loaded before the move below, so its in-memory site is one the database no longer
|
||||
# holds and this save does not write: propagating it would push every descendant back
|
||||
# to a site the Location itself has left.
|
||||
location = self._seed_location_with_children()
|
||||
stale = Location.objects.get(pk=location.pk)
|
||||
self.assertEqual(stale.site, self.site_a)
|
||||
|
||||
location.site = self.site_b
|
||||
location.save()
|
||||
|
||||
stale.description = 'updated'
|
||||
self.assertEqual(
|
||||
self.capture_propagation_updates(stale, update_fields=['description']), set()
|
||||
)
|
||||
|
||||
# Nothing beneath the Location was dragged back to site_a.
|
||||
self.assertEqual(Rack.objects.get(location=location).site, self.site_b)
|
||||
device = Device.objects.get(location=location)
|
||||
self.assertEqual(device.site, self.site_b)
|
||||
self.assertEqual(Interface.objects.get(device=device)._site, self.site_b)
|
||||
|
||||
def test_partial_save_naming_the_field_still_propagates(self):
|
||||
# The converse of the test above: a save which really did write the site must still
|
||||
# propagate. update_fields may name a foreign key by its field name...
|
||||
location = self._seed_location_with_children()
|
||||
location.site = self.site_b
|
||||
|
||||
self.assertEqual(
|
||||
self.capture_propagation_updates(location, update_fields=['site']),
|
||||
self.propagation_tables,
|
||||
)
|
||||
|
||||
def test_partial_save_naming_the_attname_still_propagates(self):
|
||||
# ...or by its attname, which Django accepts equally. Deciding whether a guarded field
|
||||
# was written has to recognise both spellings, or a real move named this way would be
|
||||
# mistaken for an unwritten field and silently skipped.
|
||||
location = self._seed_location_with_children()
|
||||
location.site = self.site_b
|
||||
|
||||
self.assertEqual(
|
||||
self.capture_propagation_updates(location, update_fields=['site_id']),
|
||||
self.propagation_tables,
|
||||
)
|
||||
|
||||
def test_raw_save_does_not_reuse_a_previous_saves_stash(self):
|
||||
# A raw save takes no stash of its own, so it must clear the one left by the previous
|
||||
# save of the same instance: comparing against a snapshot of the database as it stood
|
||||
# before an earlier write can report the propagated fields as unchanged when they are
|
||||
# not. The raw guard above means no handler consults the stash on this save, making the
|
||||
# clearing defensive — but it keeps the invariant that a stash never outlives its save,
|
||||
# so a later reader cannot be handed a stale one.
|
||||
location = self._seed_location_with_children()
|
||||
location.save()
|
||||
self.assertIsNotNone(location._presave_scope_fields)
|
||||
|
||||
location.save_base(raw=True)
|
||||
|
||||
self.assertIsNone(location._presave_scope_fields)
|
||||
|
||||
|
||||
class LocationSiteChangeAutocommitTestCase(TransactionTestCase):
|
||||
"""
|
||||
Exercise the autocommit save path, which TestCase cannot reach (it wraps every test in a
|
||||
transaction). Outside an atomic block the pre-save read and the save's UPDATE run in
|
||||
separate transactions, so the skip guard is disabled there: the stash is cleared and the
|
||||
propagation runs unconditionally.
|
||||
|
||||
Note: TransactionTestCase teardown flushes all tables, which removes rows seeded by data
|
||||
migrations from a --keepdb database (e.g. the dcim.0206 ModuleTypeProfiles). A fresh test
|
||||
database restores them.
|
||||
"""
|
||||
|
||||
def test_autocommit_noop_save_always_propagates(self):
|
||||
site = Site.objects.create(name='Site', slug='site')
|
||||
other_site = Site.objects.create(name='Other Site', slug='other-site')
|
||||
manufacturer = Manufacturer.objects.create(name='Manufacturer', slug='manufacturer')
|
||||
device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Device Type')
|
||||
device_role = DeviceRole.objects.create(name='Device Role', slug='device-role')
|
||||
location = Location.objects.create(name='Loc', slug='loc', site=site)
|
||||
device = Device.objects.create(
|
||||
name='Device', site=site, location=location, device_type=device_type, role=device_role
|
||||
)
|
||||
interface = Interface.objects.create(device=device, name='Interface 1')
|
||||
|
||||
# A transactional save first, so the instance carries a stash. The subsequent
|
||||
# autocommit save must clear it rather than compare against a previous save's values.
|
||||
with transaction.atomic():
|
||||
location.save()
|
||||
|
||||
# Poison a cached column via a signal-less update; an unconditional propagation
|
||||
# repairs it.
|
||||
Interface.objects.filter(pk=interface.pk).update(_site=other_site)
|
||||
|
||||
location.save() # Autocommit: no stash, unconditional propagation
|
||||
|
||||
interface.refresh_from_db()
|
||||
self.assertEqual(interface._site, site)
|
||||
|
||||
|
||||
class RackSiteChangeSignalTestCase(ScopePropagationCaptureMixin, TestCase):
|
||||
"""
|
||||
Verify dcim.signals.handle_rack_site_change propagates a Rack's site/location to its
|
||||
Devices and their components when the Rack is moved.
|
||||
Devices and their components when the Rack is moved, and only then.
|
||||
"""
|
||||
propagation_tables = COMPONENT_TABLES | {'dcim_device'}
|
||||
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
|
|
@ -163,6 +339,133 @@ class RackSiteChangeSignalTestCase(TestCase):
|
|||
self.assertEqual(interface._site, self.site_b)
|
||||
self.assertEqual(interface._location, self.location_b)
|
||||
|
||||
def _seed_rack_with_devices(self):
|
||||
rack = Rack.objects.create(name='Rack', site=self.site_a)
|
||||
device = Device.objects.create(
|
||||
name='Device',
|
||||
site=self.site_a,
|
||||
rack=rack,
|
||||
device_type=self.device_type,
|
||||
role=self.device_role,
|
||||
)
|
||||
Interface.objects.create(device=device, name='Interface 1')
|
||||
return rack
|
||||
|
||||
def test_unchanged_scope_skips_propagation(self):
|
||||
# Both values the handler writes are derived from the Rack's site and location
|
||||
# assignments, so a save which leaves both alone must not rewrite a single device or
|
||||
# component row.
|
||||
rack = self._seed_rack_with_devices()
|
||||
rack.description = 'updated'
|
||||
|
||||
self.assertEqual(self.capture_propagation_updates(rack), set())
|
||||
|
||||
def test_changed_site_propagates(self):
|
||||
# Counterpart to the test above, which would pass vacuously if these UPDATEs stopped
|
||||
# being issued (or their tables were renamed) rather than merely being skipped.
|
||||
rack = self._seed_rack_with_devices()
|
||||
rack.site = self.site_b
|
||||
|
||||
self.assertEqual(self.capture_propagation_updates(rack), self.propagation_tables)
|
||||
|
||||
def test_changed_location_propagates(self):
|
||||
# Location moves within the same Site must propagate too: the guard covers both
|
||||
# fields, not just the Site.
|
||||
rack = self._seed_rack_with_devices()
|
||||
rack.site = self.site_b
|
||||
rack.save()
|
||||
rack.location = self.location_b
|
||||
|
||||
self.assertEqual(self.capture_propagation_updates(rack), self.propagation_tables)
|
||||
|
||||
def test_raw_save_skips_propagation(self):
|
||||
# raw=True is set only by Django's loaddata pathway, whose fixture already carries the
|
||||
# denormalized values for every object it loads, so the propagation would rewrite each
|
||||
# matched row with what it already holds.
|
||||
rack = self._seed_rack_with_devices()
|
||||
rack.site = self.site_b
|
||||
|
||||
self.assertEqual(self.capture_propagation_updates(rack, raw=True), set())
|
||||
|
||||
def test_stale_partial_save_does_not_propagate_an_unwritten_scope(self):
|
||||
# As for Location: this instance was loaded before the move below, so neither of its
|
||||
# in-memory scope values is one this save writes, and neither may be propagated.
|
||||
rack = self._seed_rack_with_devices()
|
||||
stale = Rack.objects.get(pk=rack.pk)
|
||||
|
||||
rack.site = self.site_b
|
||||
rack.location = self.location_b
|
||||
rack.save()
|
||||
|
||||
stale.description = 'updated'
|
||||
self.assertEqual(
|
||||
self.capture_propagation_updates(stale, update_fields=['description']), set()
|
||||
)
|
||||
|
||||
device = Device.objects.get(rack=rack)
|
||||
self.assertEqual(device.site, self.site_b)
|
||||
self.assertEqual(device.location, self.location_b)
|
||||
interface = Interface.objects.get(device=device)
|
||||
self.assertEqual(interface._site, self.site_b)
|
||||
self.assertEqual(interface._location, self.location_b)
|
||||
|
||||
|
||||
class StashedScopeFieldsRegistrationTestCase(TestCase):
|
||||
"""
|
||||
Verify cache_presave_scope_fields() is connected for every model in
|
||||
signals.STASHED_SCOPE_FIELDS, and that each entry's fields resolve. An entry whose
|
||||
receiver was never connected would leave the post_save handlers reading its stash
|
||||
finding none, and doing their work unconditionally on every save.
|
||||
"""
|
||||
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
cls.instances = {}
|
||||
site = Site.objects.create(name='Site', slug='site')
|
||||
location = Location.objects.create(name='Location', slug='location', site=site)
|
||||
rack = Rack.objects.create(name='Rack', site=site, location=location)
|
||||
manufacturer = Manufacturer.objects.create(name='Manufacturer', slug='manufacturer')
|
||||
cls.instances = {
|
||||
Site: site,
|
||||
Location: location,
|
||||
Rack: rack,
|
||||
Device: Device.objects.create(
|
||||
name='Device',
|
||||
site=site,
|
||||
location=location,
|
||||
rack=rack,
|
||||
device_type=DeviceType.objects.create(manufacturer=manufacturer, model='Device Type'),
|
||||
role=DeviceRole.objects.create(name='Device Role', slug='device-role'),
|
||||
),
|
||||
}
|
||||
|
||||
def test_every_mapped_model_stashes_its_fields_on_save(self):
|
||||
# TestCase wraps each test in a transaction, so every save below takes a stash.
|
||||
self.assertEqual(set(self.instances), set(signals.STASHED_SCOPE_FIELDS))
|
||||
|
||||
for model, fields in signals.STASHED_SCOPE_FIELDS.items():
|
||||
with self.subTest(model=model.__name__):
|
||||
instance = self.instances[model]
|
||||
instance.save()
|
||||
|
||||
self.assertEqual(instance._presave_scope_fields.keys(), set(fields))
|
||||
|
||||
def test_every_mapped_field_resolves_to_both_spellings(self):
|
||||
# STASHED_FIELD_ALIASES is derived from the model meta, so a field name which stopped
|
||||
# resolving would drop out of it silently — and a field missing from it is one that
|
||||
# update_fields can never mark as written, permanently skipping its propagation.
|
||||
self.assertEqual(set(signals.STASHED_FIELD_ALIASES), set(signals.STASHED_SCOPE_FIELDS))
|
||||
|
||||
for model, fields in signals.STASHED_SCOPE_FIELDS.items():
|
||||
with self.subTest(model=model.__name__):
|
||||
aliases = signals.STASHED_FIELD_ALIASES[model]
|
||||
self.assertEqual(set(aliases), set(fields))
|
||||
for attname, names in aliases.items():
|
||||
# Both the field name and its attname, which update_fields may use
|
||||
# interchangeably.
|
||||
field = model._meta.get_field(attname.removesuffix('_id'))
|
||||
self.assertEqual(names, frozenset((field.name, field.attname)))
|
||||
|
||||
|
||||
class ScopeSignalConnectionTestCase(TestCase):
|
||||
"""
|
||||
|
|
@ -285,11 +588,12 @@ class ScopeSignalConnectionTestCase(TestCase):
|
|||
self.assertEqual(cluster._region, region)
|
||||
|
||||
|
||||
class DeviceSiteChangeSignalTestCase(TestCase):
|
||||
class DeviceSiteChangeSignalTestCase(ScopePropagationCaptureMixin, TestCase):
|
||||
"""
|
||||
Verify dcim.signals.handle_device_site_change propagates a Device's site/location/rack
|
||||
to its components on save.
|
||||
to its components on save, and only then.
|
||||
"""
|
||||
propagation_tables = COMPONENT_TABLES
|
||||
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
|
|
@ -315,6 +619,99 @@ class DeviceSiteChangeSignalTestCase(TestCase):
|
|||
interface.refresh_from_db()
|
||||
self.assertEqual(interface._site, self.site_b)
|
||||
|
||||
def _seed_device_with_components(self):
|
||||
device = Device.objects.create(
|
||||
name='Device',
|
||||
site=self.site_a,
|
||||
device_type=self.device_type,
|
||||
role=self.device_role,
|
||||
)
|
||||
Interface.objects.create(device=device, name='Interface 1')
|
||||
return device
|
||||
|
||||
def test_unchanged_scope_skips_propagation(self):
|
||||
# Components repopulate _site/_location/_rack from their Device on their own save
|
||||
# (see ComponentModel.save), so a Device save which moved the Device nowhere has
|
||||
# nothing to push down and must not rewrite a single component row.
|
||||
device = self._seed_device_with_components()
|
||||
device.description = 'updated'
|
||||
|
||||
self.assertEqual(self.capture_propagation_updates(device), set())
|
||||
|
||||
def test_changed_site_propagates(self):
|
||||
# Counterpart to the test above, which would pass vacuously if these UPDATEs stopped
|
||||
# being issued (or their tables were renamed) rather than merely being skipped.
|
||||
device = self._seed_device_with_components()
|
||||
device.site = self.site_b
|
||||
|
||||
self.assertEqual(self.capture_propagation_updates(device), self.propagation_tables)
|
||||
|
||||
def test_changed_rack_propagates(self):
|
||||
# A Rack assignment is the third guarded field, and the only one changed here: the
|
||||
# Rack is deliberately left without a Location, so Device.save() does not inherit one
|
||||
# and neither site nor location moves.
|
||||
device = self._seed_device_with_components()
|
||||
rack = Rack.objects.create(name='Rack', site=self.site_a)
|
||||
self.assertIsNone(rack.location)
|
||||
device.rack = rack
|
||||
|
||||
self.assertEqual(self.capture_propagation_updates(device), self.propagation_tables)
|
||||
|
||||
def test_raw_save_skips_propagation(self):
|
||||
# raw=True is set only by Django's loaddata pathway, whose fixture already carries the
|
||||
# denormalized values for every object it loads, so the propagation would rewrite each
|
||||
# matched row with what it already holds.
|
||||
device = self._seed_device_with_components()
|
||||
device.site = self.site_b
|
||||
|
||||
self.assertEqual(self.capture_propagation_updates(device, raw=True), set())
|
||||
|
||||
def test_stale_partial_save_does_not_propagate_an_unwritten_scope(self):
|
||||
# As for Location and Rack: this instance was loaded before the move below, so its
|
||||
# in-memory site is not one this save writes and must not reach the components.
|
||||
device = self._seed_device_with_components()
|
||||
stale = Device.objects.get(pk=device.pk)
|
||||
|
||||
device.site = self.site_b
|
||||
device.save()
|
||||
|
||||
stale.description = 'updated'
|
||||
self.assertEqual(
|
||||
self.capture_propagation_updates(stale, update_fields=['description']), set()
|
||||
)
|
||||
|
||||
self.assertEqual(Interface.objects.get(device=device)._site, self.site_b)
|
||||
|
||||
def test_stale_partial_save_propagates_written_field_with_database_values(self):
|
||||
# The mixed case, which the skip cannot cover: one guarded field is written, so the
|
||||
# propagation must run — and the two fields the save did not write have to be taken
|
||||
# from the database, not from the stale instance. Assigning the rack alone leaves the
|
||||
# site and location columns untouched, so the components must end up at site_b (where
|
||||
# the device actually is) rather than site_a (which the instance still carries).
|
||||
device = self._seed_device_with_components()
|
||||
stale = Device.objects.get(pk=device.pk)
|
||||
|
||||
device.site = self.site_b
|
||||
device.save()
|
||||
|
||||
# A rack in site_b with no location, so Device.save() inherits no location from it.
|
||||
rack = Rack.objects.create(name='Rack', site=self.site_b)
|
||||
self.assertIsNone(rack.location)
|
||||
stale.rack = rack
|
||||
|
||||
self.assertEqual(
|
||||
self.capture_propagation_updates(stale, update_fields=['rack']),
|
||||
self.propagation_tables,
|
||||
)
|
||||
|
||||
interface = Interface.objects.get(device=device)
|
||||
self.assertEqual(interface._site, self.site_b)
|
||||
self.assertEqual(interface._rack, rack)
|
||||
self.assertIsNone(interface._location)
|
||||
# The device's own site column was never rewritten by the partial save either.
|
||||
device.refresh_from_db()
|
||||
self.assertEqual(device.site, self.site_b)
|
||||
|
||||
|
||||
class VirtualChassisMasterSignalTestCase(TestCase):
|
||||
"""
|
||||
|
|
@ -964,6 +1361,50 @@ class SyncCachedScopeFieldsSignalTestCase(TestCase):
|
|||
]
|
||||
self.assertEqual(len(cluster_updates), 1)
|
||||
|
||||
def test_stale_partial_save_skips_resync(self):
|
||||
# A save passing update_fields writes only the fields it names, so an omitted scope
|
||||
# field cannot have changed and the rebuild has nothing to recompute.
|
||||
group_a = SiteGroup.objects.create(name='Group A', slug='group-a')
|
||||
group_b = SiteGroup.objects.create(name='Group B', slug='group-b')
|
||||
site = Site.objects.create(name='Site', slug='site', group=group_a)
|
||||
cluster_type = ClusterType.objects.create(name='CT', slug='ct')
|
||||
cluster = Cluster.objects.create(name='Cluster', type=cluster_type, scope=site)
|
||||
|
||||
stale = Site.objects.get(pk=site.pk)
|
||||
site.group = group_b
|
||||
site.save()
|
||||
|
||||
stale.description = 'updated'
|
||||
with CaptureQueriesContext(connection) as ctx:
|
||||
stale.save(update_fields=['description'])
|
||||
|
||||
self.assertEqual(
|
||||
[q for q in ctx.captured_queries if q['sql'].startswith('UPDATE "virtualization_cluster"')],
|
||||
[],
|
||||
)
|
||||
cluster.refresh_from_db()
|
||||
self.assertEqual(cluster._site_group, group_b)
|
||||
|
||||
def test_raw_save_skips_resync(self):
|
||||
# raw=True is set only by Django's loaddata pathway, whose fixture already carries the
|
||||
# cached scope fields for every object it loads, so the rebuild would recompute the
|
||||
# values the rows already hold.
|
||||
group_a = SiteGroup.objects.create(name='Group A', slug='group-a')
|
||||
group_b = SiteGroup.objects.create(name='Group B', slug='group-b')
|
||||
site = Site.objects.create(name='Site', slug='site', group=group_a)
|
||||
cluster_type = ClusterType.objects.create(name='CT', slug='ct')
|
||||
Cluster.objects.create(name='Cluster', type=cluster_type, scope=site)
|
||||
|
||||
site.group = group_b # A real scope change, which a non-raw save would resync
|
||||
|
||||
with CaptureQueriesContext(connection) as ctx:
|
||||
site.save_base(raw=True)
|
||||
|
||||
self.assertEqual(
|
||||
[q for q in ctx.captured_queries if q['sql'].startswith('UPDATE "virtualization_cluster"')],
|
||||
[],
|
||||
)
|
||||
|
||||
|
||||
class SyncCachedScopeFieldsAutocommitTestCase(TransactionTestCase):
|
||||
"""
|
||||
|
|
|
|||
Loading…
Reference in New Issue