Merge pull request #22459 from netbox-community/21355-denormalize
#21355 - Handle updates to denormalized data via PostgreSQL triggers
This commit is contained in:
bctiemann
GitHub
commit
cfc5414922
|
|
@ -16,10 +16,6 @@ A dictionary mapping of models to foreign keys with which cached counter fields
|
|||
|
||||
A dictionary mapping data backend types to their respective classes. These are used to interact with [remote data sources](../models/core/datasource.md).
|
||||
|
||||
### `denormalized_fields`
|
||||
|
||||
Stores registration made using `netbox.denormalized.register()`. For each model, a list of related models and their field mappings is maintained to facilitate automatic updates.
|
||||
|
||||
### `filtersets`
|
||||
|
||||
A dictionary mapping each model (identified by its app and label) to its filterset class, if one has been registered for it. Filtersets are registered using the `@register_filterset` decorator.
|
||||
|
|
|
|||
|
|
@ -1,7 +1,5 @@
|
|||
from django.apps import AppConfig
|
||||
|
||||
from netbox import denormalized
|
||||
|
||||
|
||||
class CircuitsConfig(AppConfig):
|
||||
name = "circuits"
|
||||
|
|
@ -11,16 +9,6 @@ class CircuitsConfig(AppConfig):
|
|||
from netbox.models.features import register_models
|
||||
|
||||
from . import search, signals # noqa: F401
|
||||
from .models import CircuitTermination
|
||||
|
||||
# Register models
|
||||
register_models(*self.get_models())
|
||||
|
||||
denormalized.register(CircuitTermination, '_site', {
|
||||
'_region': 'region',
|
||||
'_site_group': 'group',
|
||||
})
|
||||
|
||||
denormalized.register(CircuitTermination, '_location', {
|
||||
'_site': 'site',
|
||||
})
|
||||
|
|
|
|||
|
|
@ -0,0 +1,18 @@
|
|||
"""
|
||||
Maintain CircuitTermination's denormalized site/region/site-group columns via PostgreSQL triggers instead
|
||||
of the Python `post_save` handler formerly registered in netbox.denormalized.
|
||||
"""
|
||||
from django.db import migrations
|
||||
|
||||
from utilities.migration import cached_scope_triggers
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('circuits', '0057_default_ordering_indexes'),
|
||||
# Source tables (dcim_site, dcim_location) must already exist.
|
||||
('dcim', '0238_ltree_paths'),
|
||||
]
|
||||
|
||||
operations = cached_scope_triggers('circuits_circuittermination')
|
||||
|
|
@ -3,7 +3,7 @@ from django.core.exceptions import NON_FIELD_ERRORS, ValidationError
|
|||
from django.test import TestCase
|
||||
|
||||
from circuits.models import Circuit, CircuitTermination, CircuitType, Provider, ProviderNetwork
|
||||
from dcim.models import Site
|
||||
from dcim.models import Location, Region, Site, SiteGroup
|
||||
|
||||
|
||||
class CircuitTerminationTestCase(TestCase):
|
||||
|
|
@ -166,3 +166,85 @@ class CircuitTerminationTestCase(TestCase):
|
|||
self.assertIn(NON_FIELD_ERRORS, errors)
|
||||
self.assertIn('Please select a Provider Network.', errors[NON_FIELD_ERRORS])
|
||||
self.assertNotIn('termination_id', errors)
|
||||
|
||||
|
||||
class CircuitTerminationDenormalizationTriggerTestCase(TestCase):
|
||||
"""
|
||||
Verify the PostgreSQL triggers (installed by circuits migration 0058) that keep a
|
||||
CircuitTermination's denormalized scope columns in sync with its Site/Location.
|
||||
|
||||
These replace the former Python `post_save` handler in netbox.denormalized. Unlike that
|
||||
handler, the triggers also fire for bulk QuerySet.update() writes (exercised below).
|
||||
"""
|
||||
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
provider = Provider.objects.create(name='Provider 1', slug='provider-1')
|
||||
circuit_type = CircuitType.objects.create(name='Circuit Type 1', slug='circuit-type-1')
|
||||
cls.circuit = Circuit.objects.create(cid='Circuit 1', provider=provider, type=circuit_type)
|
||||
|
||||
def test_site_region_group_change_propagates_to_termination(self):
|
||||
region_a = Region.objects.create(name='Region A', slug='region-a')
|
||||
region_b = Region.objects.create(name='Region B', slug='region-b')
|
||||
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', region=region_a, group=group_a)
|
||||
|
||||
termination = CircuitTermination.objects.create(
|
||||
circuit=self.circuit, term_side='A', termination=site,
|
||||
)
|
||||
self.assertEqual(termination._region, region_a)
|
||||
self.assertEqual(termination._site_group, group_a)
|
||||
|
||||
# Reassign the Site's region/group; the trigger should update the termination.
|
||||
site.region = region_b
|
||||
site.group = group_b
|
||||
site.save()
|
||||
|
||||
termination.refresh_from_db()
|
||||
self.assertEqual(termination._region, region_b)
|
||||
self.assertEqual(termination._site_group, group_b)
|
||||
|
||||
def test_location_site_change_propagates_to_termination(self):
|
||||
region_a = Region.objects.create(name='Region A', slug='region-a')
|
||||
region_b = Region.objects.create(name='Region B', slug='region-b')
|
||||
group_a = SiteGroup.objects.create(name='Group A', slug='group-a')
|
||||
group_b = SiteGroup.objects.create(name='Group B', slug='group-b')
|
||||
site_a = Site.objects.create(name='Site A', slug='site-a', region=region_a, group=group_a)
|
||||
site_b = Site.objects.create(name='Site B', slug='site-b', region=region_b, group=group_b)
|
||||
location = Location.objects.create(name='Loc', slug='loc', site=site_a)
|
||||
|
||||
termination = CircuitTermination.objects.create(
|
||||
circuit=self.circuit, term_side='A', termination=location,
|
||||
)
|
||||
self.assertEqual(termination._site, site_a)
|
||||
self.assertEqual(termination._location, location)
|
||||
|
||||
# Move the Location to a different Site; the trigger updates _site and pulls the new
|
||||
# site's region/group through in the same statement.
|
||||
location.site = site_b
|
||||
location.save()
|
||||
|
||||
termination.refresh_from_db()
|
||||
self.assertEqual(termination._site, site_b)
|
||||
self.assertEqual(termination._region, region_b)
|
||||
self.assertEqual(termination._site_group, group_b)
|
||||
|
||||
def test_bulk_update_of_site_propagates_to_termination(self):
|
||||
"""
|
||||
A QuerySet.update() bypasses post_save (the old handler never fired for it); the
|
||||
DB trigger fires regardless, which is the behavior this change introduces.
|
||||
"""
|
||||
region_a = Region.objects.create(name='Region A', slug='region-a')
|
||||
region_b = Region.objects.create(name='Region B', slug='region-b')
|
||||
site = Site.objects.create(name='Site', slug='site', region=region_a)
|
||||
|
||||
termination = CircuitTermination.objects.create(
|
||||
circuit=self.circuit, term_side='A', termination=site,
|
||||
)
|
||||
self.assertEqual(termination._region, region_a)
|
||||
|
||||
Site.objects.filter(pk=site.pk).update(region=region_b)
|
||||
|
||||
termination.refresh_from_db()
|
||||
self.assertEqual(termination._region, region_b)
|
||||
|
|
|
|||
|
|
@ -1,7 +1,5 @@
|
|||
from django.apps import AppConfig
|
||||
|
||||
from netbox import denormalized
|
||||
|
||||
|
||||
class DCIMConfig(AppConfig):
|
||||
name = "dcim"
|
||||
|
|
@ -12,24 +10,10 @@ class DCIMConfig(AppConfig):
|
|||
from utilities.counters import connect_counters
|
||||
|
||||
from . import search, signals # noqa: F401
|
||||
from .models import CableTermination, Device, DeviceType, ModuleType, RackType, VirtualChassis
|
||||
from .models import Device, DeviceType, ModuleType, RackType, VirtualChassis
|
||||
|
||||
# Register models
|
||||
register_models(*self.get_models())
|
||||
|
||||
# Register denormalized fields
|
||||
denormalized.register(CableTermination, '_device', {
|
||||
'_rack': 'rack',
|
||||
'_location': 'location',
|
||||
'_site': 'site',
|
||||
})
|
||||
denormalized.register(CableTermination, '_rack', {
|
||||
'_location': 'location',
|
||||
'_site': 'site',
|
||||
})
|
||||
denormalized.register(CableTermination, '_location', {
|
||||
'_site': 'site',
|
||||
})
|
||||
|
||||
# Register counters
|
||||
connect_counters(Device, DeviceType, ModuleType, RackType, VirtualChassis)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,70 @@
|
|||
"""
|
||||
Maintain denormalized device/rack/location/site columns via PostgreSQL triggers instead of Python
|
||||
`post_save` handlers:
|
||||
|
||||
- CableTermination's _device/_rack/_location/_site (formerly netbox.denormalized).
|
||||
- Each device component's _site/_location/_rack (formerly dcim.signals.handle_device_site_change /
|
||||
handle_rack_site_change / handle_location_site_change). These are derived solely from the parent
|
||||
Device, so a single Device-sourced trigger per component table covers direct device edits as well as
|
||||
the Rack/Location cascades (which write Device.site/location and thus fire this trigger).
|
||||
"""
|
||||
from django.db import migrations
|
||||
|
||||
from utilities.migration import InstallDenormalizationTrigger
|
||||
|
||||
# Device component tables carrying _site/_location/_rack denormalized from their parent Device.
|
||||
COMPONENT_TABLES = (
|
||||
'dcim_consoleport',
|
||||
'dcim_consoleserverport',
|
||||
'dcim_devicebay',
|
||||
'dcim_frontport',
|
||||
'dcim_interface',
|
||||
'dcim_inventoryitem',
|
||||
'dcim_modulebay',
|
||||
'dcim_poweroutlet',
|
||||
'dcim_powerport',
|
||||
'dcim_rearport',
|
||||
)
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('dcim', '0238_ltree_paths'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
# When a Device's rack/location/site changes, propagate to its cable terminations.
|
||||
InstallDenormalizationTrigger(
|
||||
dependent_table='dcim_cabletermination',
|
||||
source_table='dcim_device',
|
||||
fk_column='_device_id',
|
||||
mappings={'_rack_id': 'rack_id', '_location_id': 'location_id', '_site_id': 'site_id'},
|
||||
),
|
||||
# When a Rack's location/site changes, propagate to cable terminations assigned to it.
|
||||
InstallDenormalizationTrigger(
|
||||
dependent_table='dcim_cabletermination',
|
||||
source_table='dcim_rack',
|
||||
fk_column='_rack_id',
|
||||
mappings={'_location_id': 'location_id', '_site_id': 'site_id'},
|
||||
),
|
||||
# When a Location's site changes, propagate to cable terminations assigned to it.
|
||||
InstallDenormalizationTrigger(
|
||||
dependent_table='dcim_cabletermination',
|
||||
source_table='dcim_location',
|
||||
fk_column='_location_id',
|
||||
mappings={'_site_id': 'site_id'},
|
||||
),
|
||||
# Device components: mirror the parent Device's site/location/rack onto each component.
|
||||
# The Rack/Location → Device cascades (dcim.signals) write Device.site/location, which fires
|
||||
# this same trigger, so no separate Rack/Location-sourced component triggers are needed.
|
||||
*[
|
||||
InstallDenormalizationTrigger(
|
||||
dependent_table=table,
|
||||
source_table='dcim_device',
|
||||
fk_column='device_id',
|
||||
mappings={'_site_id': 'site_id', '_location_id': 'location_id', '_rack_id': 'rack_id'},
|
||||
)
|
||||
for table in COMPONENT_TABLES
|
||||
],
|
||||
]
|
||||
|
|
@ -5,59 +5,36 @@ from django.db.models.signals import post_delete, post_save
|
|||
from django.dispatch import receiver
|
||||
|
||||
from dcim.choices import CableEndChoices, LinkStatusChoices
|
||||
from ipam.models import Prefix
|
||||
from virtualization.models import Cluster, VMInterface
|
||||
from wireless.models import WirelessLAN
|
||||
from virtualization.models import VMInterface
|
||||
|
||||
from .models import (
|
||||
Cable,
|
||||
CablePath,
|
||||
CableTermination,
|
||||
ConsolePort,
|
||||
ConsoleServerPort,
|
||||
Device,
|
||||
DeviceBay,
|
||||
FrontPort,
|
||||
Interface,
|
||||
InventoryItem,
|
||||
Location,
|
||||
ModuleBay,
|
||||
PathEndpoint,
|
||||
PortMapping,
|
||||
PowerOutlet,
|
||||
PowerPanel,
|
||||
PowerPort,
|
||||
Rack,
|
||||
RearPort,
|
||||
Site,
|
||||
VirtualChassis,
|
||||
)
|
||||
from .models.cables import trace_paths
|
||||
from .utils import create_cablepaths, rebuild_paths
|
||||
|
||||
COMPONENT_MODELS = (
|
||||
ConsolePort,
|
||||
ConsoleServerPort,
|
||||
DeviceBay,
|
||||
FrontPort,
|
||||
Interface,
|
||||
InventoryItem,
|
||||
ModuleBay,
|
||||
PowerOutlet,
|
||||
PowerPort,
|
||||
RearPort,
|
||||
)
|
||||
|
||||
|
||||
#
|
||||
# Location/rack/device assignment
|
||||
#
|
||||
|
||||
|
||||
@receiver(post_save, sender=Location)
|
||||
def handle_location_site_change(instance, created, **kwargs):
|
||||
"""
|
||||
Update child objects if Site assignment has changed. We intentionally recurse through each child
|
||||
object instead of calling update() on the QuerySet to ensure the proper change records get created for each.
|
||||
Cascade a Location's Site assignment down to the Racks, Devices, and PowerPanels it contains
|
||||
(and to descendant Locations). The denormalized cache columns on cable terminations and device
|
||||
components are maintained by PostgreSQL triggers, which fire on these Site/Location/Rack/Device
|
||||
column writes.
|
||||
"""
|
||||
if not created:
|
||||
instance.get_descendants().update(site=instance.site)
|
||||
|
|
@ -65,39 +42,16 @@ def handle_location_site_change(instance, created, **kwargs):
|
|||
Rack.objects.filter(location__in=locations).update(site=instance.site)
|
||||
Device.objects.filter(location__in=locations).update(site=instance.site)
|
||||
PowerPanel.objects.filter(location__in=locations).update(site=instance.site)
|
||||
CableTermination.objects.filter(_location__in=locations).update(_site=instance.site)
|
||||
# Update component models for devices in these locations
|
||||
for model in COMPONENT_MODELS:
|
||||
model.objects.filter(device__location__in=locations).update(_site=instance.site)
|
||||
|
||||
|
||||
@receiver(post_save, sender=Rack)
|
||||
def handle_rack_site_change(instance, created, **kwargs):
|
||||
"""
|
||||
Update child Devices if Site or Location assignment has changed.
|
||||
Cascade a Rack's Site/Location assignment down to the Devices it contains. The denormalized cache
|
||||
columns on those devices' components are maintained by PostgreSQL triggers.
|
||||
"""
|
||||
if not created:
|
||||
Device.objects.filter(rack=instance).update(site=instance.site, location=instance.location)
|
||||
# Update component models for devices in this rack
|
||||
for model in COMPONENT_MODELS:
|
||||
model.objects.filter(device__rack=instance).update(
|
||||
_site=instance.site,
|
||||
_location=instance.location,
|
||||
)
|
||||
|
||||
|
||||
@receiver(post_save, sender=Device)
|
||||
def handle_device_site_change(instance, created, **kwargs):
|
||||
"""
|
||||
Update child components to update the parent Site, Location, and Rack when a Device is saved.
|
||||
"""
|
||||
if not created:
|
||||
for model in COMPONENT_MODELS:
|
||||
model.objects.filter(device=instance).update(
|
||||
_site=instance.site,
|
||||
_location=instance.location,
|
||||
_rack=instance.rack,
|
||||
)
|
||||
|
||||
|
||||
#
|
||||
|
|
@ -210,44 +164,3 @@ def update_mac_address_interface(instance, created, raw, **kwargs):
|
|||
if created and not raw and instance.primary_mac_address:
|
||||
instance.primary_mac_address.assigned_object = instance
|
||||
instance.primary_mac_address.save()
|
||||
|
||||
|
||||
@receiver(post_save, sender=Location)
|
||||
@receiver(post_save, sender=Site)
|
||||
def sync_cached_scope_fields(instance, created, **kwargs):
|
||||
"""
|
||||
Rebuild cached scope fields for all CachedScopeMixin-based models
|
||||
affected by a change in a Region, SiteGroup, Site, or Location.
|
||||
|
||||
This method is safe to run for objects created in the past and does
|
||||
not rely on incremental updates. Cached fields are recomputed from
|
||||
authoritative relationships.
|
||||
"""
|
||||
if created:
|
||||
return
|
||||
|
||||
if isinstance(instance, Location):
|
||||
filters = {'_location': instance}
|
||||
elif isinstance(instance, Site):
|
||||
filters = {'_site': instance}
|
||||
else:
|
||||
return
|
||||
|
||||
# These models are explicitly listed because they all subclass CachedScopeMixin
|
||||
# and therefore require their cached scope fields to be recomputed.
|
||||
for model in (Prefix, Cluster, WirelessLAN):
|
||||
qs = model.objects.filter(**filters)
|
||||
|
||||
# Bulk update cached fields to avoid O(N) performance issues with large datasets.
|
||||
# This does not trigger post_save signals, avoiding spurious change log entries.
|
||||
objects_to_update = []
|
||||
for obj in qs:
|
||||
# Recompute cache using the same logic as save()
|
||||
obj.cache_related_objects()
|
||||
objects_to_update.append(obj)
|
||||
|
||||
if objects_to_update:
|
||||
model.objects.bulk_update(
|
||||
objects_to_update,
|
||||
['_location', '_site', '_site_group', '_region']
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1,7 +1,9 @@
|
|||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from django.apps import apps
|
||||
from django.contrib.contenttypes.models import ContentType
|
||||
from django.db import connection
|
||||
from django.test import SimpleTestCase, TestCase
|
||||
|
||||
from dcim import signals
|
||||
|
|
@ -9,6 +11,7 @@ from dcim.choices import CableEndChoices, LinkStatusChoices
|
|||
from dcim.models import (
|
||||
Cable,
|
||||
CablePath,
|
||||
CableTermination,
|
||||
Device,
|
||||
DeviceRole,
|
||||
DeviceType,
|
||||
|
|
@ -25,7 +28,10 @@ from dcim.models import (
|
|||
SiteGroup,
|
||||
VirtualChassis,
|
||||
)
|
||||
from dcim.models.device_components import ComponentModel
|
||||
from dcim.models.mixins import CachedScopeMixin
|
||||
from ipam.models import Prefix
|
||||
from netbox.plugins import PluginConfig
|
||||
from virtualization.models import Cluster, ClusterType
|
||||
from wireless.models import WirelessLAN
|
||||
|
||||
|
|
@ -116,10 +122,11 @@ class RackSiteChangeSignalTestCase(TestCase):
|
|||
self.assertEqual(interface._location, self.location_b)
|
||||
|
||||
|
||||
class DeviceSiteChangeSignalTestCase(TestCase):
|
||||
class DeviceComponentScopeTriggerTestCase(TestCase):
|
||||
"""
|
||||
Verify dcim.signals.handle_device_site_change propagates a Device's site/location/rack
|
||||
to its components on save.
|
||||
Verify the PostgreSQL trigger (dcim migration 0239) that propagates a Device's site/location/rack
|
||||
onto its components' denormalized _site/_location/_rack columns. This replaces the former
|
||||
dcim.signals.handle_device_site_change handler.
|
||||
"""
|
||||
|
||||
@classmethod
|
||||
|
|
@ -146,6 +153,25 @@ class DeviceSiteChangeSignalTestCase(TestCase):
|
|||
interface.refresh_from_db()
|
||||
self.assertEqual(interface._site, self.site_b)
|
||||
|
||||
def test_bulk_update_of_device_updates_components_cached_scope(self):
|
||||
"""
|
||||
A bulk QuerySet.update() bypasses post_save (the old handler never fired for it); the DB
|
||||
trigger fires regardless. This is also the path the Rack/Location cascades take.
|
||||
"""
|
||||
device = Device.objects.create(
|
||||
name='Device',
|
||||
site=self.site_a,
|
||||
device_type=self.device_type,
|
||||
role=self.device_role,
|
||||
)
|
||||
interface = Interface.objects.create(device=device, name='Interface 1')
|
||||
self.assertEqual(interface._site, self.site_a)
|
||||
|
||||
Device.objects.filter(pk=device.pk).update(site=self.site_b)
|
||||
|
||||
interface.refresh_from_db()
|
||||
self.assertEqual(interface._site, self.site_b)
|
||||
|
||||
|
||||
class VirtualChassisMasterSignalTestCase(TestCase):
|
||||
"""
|
||||
|
|
@ -385,10 +411,12 @@ class MACAddressInterfaceSignalTestCase(TestCase):
|
|||
self.assertIsNone(mac.assigned_object)
|
||||
|
||||
|
||||
class SyncCachedScopeFieldsSignalTestCase(TestCase):
|
||||
class CachedScopeFieldTriggerTestCase(TestCase):
|
||||
"""
|
||||
Verify dcim.signals.sync_cached_scope_fields recomputes cached scope fields on
|
||||
Prefix, Cluster, and WirelessLAN when a Site or Location is modified.
|
||||
Verify the PostgreSQL triggers (ipam/virtualization/wireless denormalization migrations) that keep
|
||||
the CachedScopeMixin scope columns (_site/_location/_region/_site_group) on Prefix, Cluster, and
|
||||
WirelessLAN in sync when a scoped Site or Location is modified. These replace the former
|
||||
dcim.signals.sync_cached_scope_fields handler.
|
||||
"""
|
||||
|
||||
def test_site_group_change_updates_prefix_cached_scope(self):
|
||||
|
|
@ -428,11 +456,9 @@ class SyncCachedScopeFieldsSignalTestCase(TestCase):
|
|||
self.assertEqual(prefix._location, location)
|
||||
self.assertEqual(prefix._site, site_b)
|
||||
|
||||
def test_signal_updates_cluster_and_wirelesslan_cached_scope(self):
|
||||
# Lock down the explicit (Prefix, Cluster, WirelessLAN) tuple in the
|
||||
# signal by exercising Cluster and WirelessLAN alongside Prefix. If a
|
||||
# future change drops Cluster or WirelessLAN from that tuple, this test
|
||||
# will catch it.
|
||||
def test_triggers_update_cluster_and_wirelesslan_cached_scope(self):
|
||||
# Cluster and WirelessLAN each carry their own Site/Location triggers (installed by the
|
||||
# virtualization and wireless denormalization migrations); exercise both alongside Prefix.
|
||||
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)
|
||||
|
|
@ -486,3 +512,113 @@ class CableSignalDirectHandlerTestCase(SimpleTestCase):
|
|||
signals.update_mac_address_interface(instance=interface, created=True, raw=True)
|
||||
|
||||
primary_mac.save.assert_not_called()
|
||||
|
||||
|
||||
class CableTerminationDenormalizationTriggerTestCase(TestCase):
|
||||
"""
|
||||
Verify the PostgreSQL triggers (installed by dcim migration 0239) that keep a
|
||||
CableTermination's denormalized _device/_rack/_location/_site columns in sync with the
|
||||
parent Device/Rack/Location.
|
||||
|
||||
These replace the former Python `post_save` handler in netbox.denormalized. Crucially,
|
||||
the triggers also fire for bulk QuerySet.update() writes — which the handler (a post_save
|
||||
receiver) never saw — so this exercises that path explicitly.
|
||||
"""
|
||||
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
cls.site_a = Site.objects.create(name='Site A', slug='site-a')
|
||||
cls.site_b = Site.objects.create(name='Site B', slug='site-b')
|
||||
cls.location_b = Location.objects.create(name='Loc B', slug='loc-b', site=cls.site_b)
|
||||
cls.rack_b = Rack.objects.create(name='Rack B', site=cls.site_b, location=cls.location_b)
|
||||
manufacturer = Manufacturer.objects.create(name='Manufacturer', slug='manufacturer')
|
||||
cls.device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Device Type')
|
||||
cls.device_role = DeviceRole.objects.create(name='Device Role', slug='device-role')
|
||||
|
||||
def _connected_termination(self):
|
||||
device = Device.objects.create(
|
||||
name='Device', site=self.site_a, device_type=self.device_type, role=self.device_role,
|
||||
)
|
||||
interface_a = Interface.objects.create(device=device, name='Interface A')
|
||||
interface_b = Interface.objects.create(device=device, name='Interface B')
|
||||
cable = Cable(a_terminations=[interface_a], b_terminations=[interface_b])
|
||||
cable.save()
|
||||
termination = CableTermination.objects.filter(_device=device).first()
|
||||
self.assertIsNotNone(termination)
|
||||
self.assertEqual(termination._site, self.site_a)
|
||||
return device, termination
|
||||
|
||||
def test_device_move_propagates_to_cable_termination(self):
|
||||
device, termination = self._connected_termination()
|
||||
|
||||
device.site = self.site_b
|
||||
device.location = self.location_b
|
||||
device.rack = self.rack_b
|
||||
device.save()
|
||||
|
||||
termination.refresh_from_db()
|
||||
self.assertEqual(termination._site, self.site_b)
|
||||
self.assertEqual(termination._location, self.location_b)
|
||||
self.assertEqual(termination._rack, self.rack_b)
|
||||
|
||||
def test_bulk_update_of_device_propagates_to_cable_termination(self):
|
||||
"""
|
||||
A bulk QuerySet.update() bypasses post_save (the old handler never fired for it);
|
||||
the DB trigger fires regardless.
|
||||
"""
|
||||
device, termination = self._connected_termination()
|
||||
|
||||
Device.objects.filter(pk=device.pk).update(site=self.site_b)
|
||||
|
||||
termination.refresh_from_db()
|
||||
self.assertEqual(termination._site, self.site_b)
|
||||
|
||||
|
||||
def _concrete_subclasses(base):
|
||||
"""
|
||||
Yield every non-abstract, non-plugin model descending from an abstract base model. Plugin-contributed
|
||||
models are skipped: a plugin that adds a ComponentModel/CachedScopeMixin subclass is responsible for
|
||||
its own trigger migration, and must not fail core's coverage check just by being installed.
|
||||
"""
|
||||
for subclass in base.__subclasses__():
|
||||
if subclass._meta.abstract:
|
||||
yield from _concrete_subclasses(subclass)
|
||||
elif not isinstance(apps.get_app_config(subclass._meta.app_label), PluginConfig):
|
||||
yield subclass
|
||||
|
||||
|
||||
def _installed_triggers():
|
||||
with connection.cursor() as cursor:
|
||||
cursor.execute('SELECT tgname FROM pg_trigger WHERE NOT tgisinternal')
|
||||
return {row[0] for row in cursor.fetchall()}
|
||||
|
||||
|
||||
class DenormalizationTriggerCoverageTestCase(TestCase):
|
||||
"""
|
||||
Guard against a new core model silently shipping without its denormalization triggers. The set of
|
||||
device-component tables and CachedScopeMixin dependents is hand-listed in migrations; this test
|
||||
derives those sets from the model layer and asserts the expected triggers are installed, so adding
|
||||
a new component / scoped model without a matching trigger migration fails CI. Plugin-contributed
|
||||
models are excluded (see _concrete_subclasses).
|
||||
"""
|
||||
|
||||
def test_device_components_have_device_trigger(self):
|
||||
triggers = _installed_triggers()
|
||||
for model in _concrete_subclasses(ComponentModel):
|
||||
table = model._meta.db_table
|
||||
self.assertIn(
|
||||
f'{table}_denorm_from_dcim_device', triggers,
|
||||
msg=f'{model.__name__} has no dcim_device denormalization trigger (add it to '
|
||||
f'dcim migration 0239 COMPONENT_TABLES)',
|
||||
)
|
||||
|
||||
def test_cached_scope_models_have_site_and_location_triggers(self):
|
||||
triggers = _installed_triggers()
|
||||
for model in _concrete_subclasses(CachedScopeMixin):
|
||||
table = model._meta.db_table
|
||||
for source in ('dcim_site', 'dcim_location'):
|
||||
self.assertIn(
|
||||
f'{table}_denorm_from_{source}', triggers,
|
||||
msg=f'{model.__name__} (CachedScopeMixin) has no {source} denormalization trigger; '
|
||||
f'add cached_scope_triggers({table!r}) in a migration for its app',
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1,7 +1,5 @@
|
|||
from django.apps import AppConfig
|
||||
|
||||
from netbox import denormalized
|
||||
|
||||
|
||||
class IPAMConfig(AppConfig):
|
||||
name = "ipam"
|
||||
|
|
@ -11,16 +9,6 @@ class IPAMConfig(AppConfig):
|
|||
from netbox.models.features import register_models
|
||||
|
||||
from . import search, signals # noqa: F401
|
||||
from .models import Prefix
|
||||
|
||||
# Register models
|
||||
register_models(*self.get_models())
|
||||
|
||||
# Register denormalized fields
|
||||
denormalized.register(Prefix, '_site', {
|
||||
'_region': 'region',
|
||||
'_site_group': 'group',
|
||||
})
|
||||
denormalized.register(Prefix, '_location', {
|
||||
'_site': 'site',
|
||||
})
|
||||
|
|
|
|||
|
|
@ -0,0 +1,18 @@
|
|||
"""
|
||||
Maintain Prefix's denormalized site/region/site-group columns via PostgreSQL triggers instead of the
|
||||
Python `post_save` handler formerly registered in netbox.denormalized (and dcim.signals.sync_cached_scope_fields).
|
||||
"""
|
||||
from django.db import migrations
|
||||
|
||||
from utilities.migration import cached_scope_triggers
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('ipam', '0090_vlangroup_recompute_total_vlan_ids'),
|
||||
# Source tables (dcim_site, dcim_location) must already exist.
|
||||
('dcim', '0238_ltree_paths'),
|
||||
]
|
||||
|
||||
operations = cached_scope_triggers('ipam_prefix')
|
||||
|
|
@ -5,6 +5,7 @@ from django.test import RequestFactory, TestCase
|
|||
|
||||
from core.choices import ObjectChangeActionChoices
|
||||
from core.models import ObjectChange
|
||||
from dcim.models import Location, Region, Site, SiteGroup
|
||||
from ipam.models import IPAddress, Prefix
|
||||
from netbox.context_managers import event_tracking
|
||||
from users.models import User
|
||||
|
|
@ -229,3 +230,80 @@ class ClearOOBIPSignalTestCase(TestCase):
|
|||
action=ObjectChangeActionChoices.ACTION_UPDATE,
|
||||
).exists()
|
||||
)
|
||||
|
||||
|
||||
class PrefixDenormalizationTriggerTestCase(TestCase):
|
||||
"""
|
||||
Verify the PostgreSQL triggers (installed by ipam migration 0091) that keep a Prefix's
|
||||
denormalized scope columns in sync with its Site/Location.
|
||||
|
||||
These replace the former Python `post_save` handler in netbox.denormalized. Unlike that
|
||||
handler, the triggers also fire for bulk QuerySet.update() writes (exercised below).
|
||||
"""
|
||||
|
||||
def test_site_region_group_change_propagates_to_prefix(self):
|
||||
region_a = Region.objects.create(name='Region A', slug='region-a')
|
||||
region_b = Region.objects.create(name='Region B', slug='region-b')
|
||||
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', region=region_a, group=group_a)
|
||||
prefix = Prefix.objects.create(
|
||||
prefix='10.0.0.0/24',
|
||||
scope_type=ContentType.objects.get_for_model(Site),
|
||||
scope_id=site.pk,
|
||||
)
|
||||
self.assertEqual(prefix._region, region_a)
|
||||
self.assertEqual(prefix._site_group, group_a)
|
||||
|
||||
site.region = region_b
|
||||
site.group = group_b
|
||||
site.save()
|
||||
|
||||
prefix.refresh_from_db()
|
||||
self.assertEqual(prefix._region, region_b)
|
||||
self.assertEqual(prefix._site_group, group_b)
|
||||
|
||||
def test_location_site_change_propagates_to_prefix(self):
|
||||
region_a = Region.objects.create(name='Region A', slug='region-a')
|
||||
region_b = Region.objects.create(name='Region B', slug='region-b')
|
||||
group_a = SiteGroup.objects.create(name='Group A', slug='group-a')
|
||||
group_b = SiteGroup.objects.create(name='Group B', slug='group-b')
|
||||
site_a = Site.objects.create(name='Site A', slug='site-a', region=region_a, group=group_a)
|
||||
site_b = Site.objects.create(name='Site B', slug='site-b', region=region_b, group=group_b)
|
||||
location = Location.objects.create(name='Loc', slug='loc', site=site_a)
|
||||
prefix = Prefix.objects.create(
|
||||
prefix='10.0.0.0/24',
|
||||
scope_type=ContentType.objects.get_for_model(Location),
|
||||
scope_id=location.pk,
|
||||
)
|
||||
self.assertEqual(prefix._site, site_a)
|
||||
|
||||
# Move the Location to a different Site; the trigger updates _site and pulls the new
|
||||
# site's region/group through in the same statement.
|
||||
location.site = site_b
|
||||
location.save()
|
||||
|
||||
prefix.refresh_from_db()
|
||||
self.assertEqual(prefix._site, site_b)
|
||||
self.assertEqual(prefix._region, region_b)
|
||||
self.assertEqual(prefix._site_group, group_b)
|
||||
|
||||
def test_bulk_update_of_site_propagates_to_prefix(self):
|
||||
"""
|
||||
A bulk QuerySet.update() bypasses post_save (the old handler never fired for it);
|
||||
the DB trigger fires regardless.
|
||||
"""
|
||||
region_a = Region.objects.create(name='Region A', slug='region-a')
|
||||
region_b = Region.objects.create(name='Region B', slug='region-b')
|
||||
site = Site.objects.create(name='Site', slug='site', region=region_a)
|
||||
prefix = Prefix.objects.create(
|
||||
prefix='10.0.0.0/24',
|
||||
scope_type=ContentType.objects.get_for_model(Site),
|
||||
scope_id=site.pk,
|
||||
)
|
||||
self.assertEqual(prefix._region, region_a)
|
||||
|
||||
Site.objects.filter(pk=site.pk).update(region=region_b)
|
||||
|
||||
prefix.refresh_from_db()
|
||||
self.assertEqual(prefix._region, region_b)
|
||||
|
|
|
|||
|
|
@ -1,57 +0,0 @@
|
|||
import logging
|
||||
|
||||
from django.db.models.signals import post_save
|
||||
from django.dispatch import receiver
|
||||
|
||||
from netbox.registry import registry
|
||||
|
||||
logger = logging.getLogger('netbox.denormalized')
|
||||
|
||||
|
||||
def register(model, field_name, mappings):
|
||||
"""
|
||||
Register a denormalized model field to ensure that it is kept up-to-date with the related object.
|
||||
|
||||
Args:
|
||||
model: The class being updated
|
||||
field_name: The name of the field related to the triggering instance
|
||||
mappings: Dictionary mapping of local to remote fields
|
||||
"""
|
||||
logger.debug(f'Registering denormalized field {model}.{field_name}')
|
||||
|
||||
field = model._meta.get_field(field_name)
|
||||
rel_model = field.related_model
|
||||
|
||||
registry['denormalized_fields'][rel_model].append(
|
||||
(model, field_name, mappings)
|
||||
)
|
||||
|
||||
|
||||
@receiver(post_save)
|
||||
def update_denormalized_fields(sender, instance, created, raw, **kwargs):
|
||||
"""
|
||||
Check if the sender has denormalized fields registered, and update them as necessary.
|
||||
"""
|
||||
def _get_field_value(instance, field_name):
|
||||
field = instance._meta.get_field(field_name)
|
||||
return field.value_from_object(instance)
|
||||
|
||||
# Skip for new objects or those being populated from raw data
|
||||
if created or raw:
|
||||
return
|
||||
|
||||
# Look up any denormalized fields referencing this model from the application registry
|
||||
for model, field_name, mappings in registry['denormalized_fields'].get(sender, []):
|
||||
logger.debug(f'Updating denormalized values for {model}.{field_name}')
|
||||
filter_params = {
|
||||
field_name: instance.pk,
|
||||
}
|
||||
update_params = {
|
||||
# Map the denormalized field names to the instance's values
|
||||
denorm: _get_field_value(instance, origin) for denorm, origin in mappings.items()
|
||||
}
|
||||
|
||||
# TODO: Improve efficiency here by placing conditions on the query?
|
||||
# Update all the denormalized fields with the triggering object's new values
|
||||
count = model.objects.filter(**filter_params).update(**update_params)
|
||||
logger.debug(f'Updated {count} rows')
|
||||
|
|
@ -25,7 +25,6 @@ class Registry(dict):
|
|||
registry = Registry({
|
||||
'counter_fields': collections.defaultdict(dict),
|
||||
'data_backends': dict(),
|
||||
'denormalized_fields': collections.defaultdict(list),
|
||||
'event_types': dict(),
|
||||
'filtersets': dict(),
|
||||
'model_actions': collections.defaultdict(set),
|
||||
|
|
|
|||
|
|
@ -1,8 +1,10 @@
|
|||
from django.db import models
|
||||
from django.db import migrations, models
|
||||
|
||||
from netbox.config import ConfigItem
|
||||
|
||||
__all__ = (
|
||||
'InstallDenormalizationTrigger',
|
||||
'cached_scope_triggers',
|
||||
'custom_deconstruct',
|
||||
)
|
||||
|
||||
|
|
@ -32,3 +34,146 @@ def custom_deconstruct(field):
|
|||
}
|
||||
|
||||
return name, path, args, kwargs
|
||||
|
||||
|
||||
class InstallDenormalizationTrigger(migrations.operations.base.Operation):
|
||||
"""
|
||||
Install a PostgreSQL trigger that keeps denormalized columns on a dependent table in sync with their
|
||||
source object.
|
||||
|
||||
When a row in `source_table` is updated, the trigger copies the values of the mapped source columns into
|
||||
the corresponding denormalized columns on every `dependent_table` row that references it via `fk_column`.
|
||||
This replaces the Python `post_save` handlers formerly defined in `netbox.denormalized` and `dcim.signals`.
|
||||
|
||||
Args:
|
||||
dependent_table: The table carrying the denormalized columns (e.g. 'ipam_prefix').
|
||||
source_table: The table whose changes are propagated (e.g. 'dcim_site').
|
||||
fk_column: The column on `dependent_table` referencing `source_table` (e.g. '_site_id').
|
||||
mappings: A mapping of {dependent_column: source_column}, using actual database column names
|
||||
(e.g. {'_region_id': 'region_id', '_site_group_id': 'group_id'}). Each is copied directly:
|
||||
`dependent_column = NEW.source_column`.
|
||||
related_mappings: An optional iterable of related-table lookups for columns that live one hop
|
||||
beyond `source_table`. Each entry is a dict with keys `table` (the related table), `source_fk`
|
||||
(a column on `source_table` referencing `related_table.id`), and `mappings`
|
||||
({dependent_column: related_column}). Each is resolved with a single multi-column subquery
|
||||
(`(cols) = (SELECT cols FROM table WHERE id = NEW.source_fk)`), so the related row is read once.
|
||||
This closes the chain gap when a denormalized column is derived through an intermediate object
|
||||
(e.g. a Location's Site change must refresh the dependent's region/site-group, not just its site).
|
||||
If `source_fk` is NULL the subquery returns no row and all its target columns are set to NULL,
|
||||
which is the correct result (the source object has no related object); current callers use a
|
||||
non-nullable `source_fk` (Location.site), so this does not arise in practice.
|
||||
|
||||
The trigger fires AFTER UPDATE of the watched source columns (the direct `mappings` sources plus each
|
||||
related `source_fk`), and only when at least one of them actually changed. It does not fire on INSERT (a
|
||||
newly created source row has no dependents yet) and it does not recurse: the dependent tables carry no
|
||||
triggers of their own.
|
||||
|
||||
Note: this is a row-level trigger, so a bulk source update of N rows fires it N times. A statement-level
|
||||
trigger with transition tables would batch this, but PostgreSQL forbids transition tables on a trigger
|
||||
with an `UPDATE OF <columns>` list, and dropping that column list would fire the trigger on every source
|
||||
update (including unrelated columns) — a worse trade on hot-write tables like dcim_device.
|
||||
"""
|
||||
reversible = True
|
||||
|
||||
def __init__(self, dependent_table, source_table, fk_column, mappings, related_mappings=()):
|
||||
self.dependent_table = dependent_table
|
||||
self.source_table = source_table
|
||||
self.fk_column = fk_column
|
||||
self.mappings = mappings
|
||||
self.related_mappings = list(related_mappings)
|
||||
|
||||
@property
|
||||
def function_name(self):
|
||||
return f'{self.dependent_table}_denorm_from_{self.source_table}_fn'
|
||||
|
||||
@property
|
||||
def trigger_name(self):
|
||||
return f'{self.dependent_table}_denorm_from_{self.source_table}'
|
||||
|
||||
def state_forwards(self, app_label, state):
|
||||
# Triggers are not part of Django's model state.
|
||||
pass
|
||||
|
||||
def database_forwards(self, app_label, schema_editor, from_state, to_state):
|
||||
# Direct column copies from the changed source row.
|
||||
set_parts = [f'"{dest}" = NEW."{src}"' for dest, src in self.mappings.items()]
|
||||
watched = list(self.mappings.values())
|
||||
# One-hop lookups: a single multi-column subquery per related table reads its row only once.
|
||||
for rel in self.related_mappings:
|
||||
dests = ', '.join(f'"{d}"' for d in rel['mappings'].keys())
|
||||
cols = ', '.join(f'"{c}"' for c in rel['mappings'].values())
|
||||
set_parts.append(f'({dests}) = (SELECT {cols} FROM "{rel["table"]}" WHERE id = NEW."{rel["source_fk"]}")')
|
||||
watched.append(rel['source_fk'])
|
||||
|
||||
# Deduplicate watched columns while preserving order (a direct mapping and a related lookup may
|
||||
# both key off the same source column, e.g. site_id).
|
||||
watched_columns = list(dict.fromkeys(watched))
|
||||
|
||||
set_clause = ', '.join(set_parts)
|
||||
update_of = ', '.join(f'"{col}"' for col in watched_columns)
|
||||
when_clause = ' OR '.join(f'OLD."{col}" IS DISTINCT FROM NEW."{col}"' for col in watched_columns)
|
||||
|
||||
schema_editor.execute(f'''
|
||||
CREATE OR REPLACE FUNCTION "{self.function_name}"() RETURNS TRIGGER AS $$
|
||||
BEGIN
|
||||
UPDATE "{self.dependent_table}"
|
||||
SET {set_clause}
|
||||
WHERE "{self.fk_column}" = NEW.id;
|
||||
RETURN NULL;
|
||||
END
|
||||
$$ LANGUAGE plpgsql;
|
||||
''')
|
||||
# Drop first so the operation is idempotent (re-run / partially-applied migration, or a
|
||||
# trigger pre-installed during testing); CREATE TRIGGER alone errors if one already exists.
|
||||
schema_editor.execute(f'DROP TRIGGER IF EXISTS "{self.trigger_name}" ON "{self.source_table}";')
|
||||
schema_editor.execute(f'''
|
||||
CREATE TRIGGER "{self.trigger_name}"
|
||||
AFTER UPDATE OF {update_of} ON "{self.source_table}"
|
||||
FOR EACH ROW WHEN ({when_clause})
|
||||
EXECUTE FUNCTION "{self.function_name}"();
|
||||
''')
|
||||
|
||||
def database_backwards(self, app_label, schema_editor, from_state, to_state):
|
||||
schema_editor.execute(f'DROP TRIGGER IF EXISTS "{self.trigger_name}" ON "{self.source_table}";')
|
||||
schema_editor.execute(f'DROP FUNCTION IF EXISTS "{self.function_name}"();')
|
||||
|
||||
def describe(self):
|
||||
return f'Install denormalization trigger on {self.source_table} updating {self.dependent_table}'
|
||||
|
||||
|
||||
# Site/region/site-group lookup shared by every CachedScopeMixin-style dependent (see cached_scope_triggers).
|
||||
SITE_SCOPE_RELATED_MAPPINGS = (
|
||||
{
|
||||
'table': 'dcim_site',
|
||||
'source_fk': 'site_id',
|
||||
'mappings': {'_region_id': 'region_id', '_site_group_id': 'group_id'},
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
def cached_scope_triggers(dependent_table):
|
||||
"""
|
||||
Return the Site + Location `InstallDenormalizationTrigger` pair for a dependent table carrying the
|
||||
standard cached-scope columns (_site/_location/_region/_site_group) — i.e. any CachedScopeMixin model
|
||||
(Prefix, Cluster, WirelessLAN) plus CircuitTermination, which share the same denormalization shape.
|
||||
|
||||
Region- and SiteGroup-scoped rows need no trigger: their cached FK is the scoped object itself and
|
||||
never changes underneath them. So two triggers fully cover the cache:
|
||||
- dcim_site: region/group changed -> refresh _region/_site_group on rows scoped to that site
|
||||
- dcim_location: site changed -> refresh _site (and the new site's region/group)
|
||||
"""
|
||||
return [
|
||||
InstallDenormalizationTrigger(
|
||||
dependent_table=dependent_table,
|
||||
source_table='dcim_site',
|
||||
fk_column='_site_id',
|
||||
mappings={'_region_id': 'region_id', '_site_group_id': 'group_id'},
|
||||
),
|
||||
InstallDenormalizationTrigger(
|
||||
dependent_table=dependent_table,
|
||||
source_table='dcim_location',
|
||||
fk_column='_location_id',
|
||||
mappings={'_site_id': 'site_id'},
|
||||
related_mappings=SITE_SCOPE_RELATED_MAPPINGS,
|
||||
),
|
||||
]
|
||||
|
|
|
|||
|
|
@ -0,0 +1,18 @@
|
|||
"""
|
||||
Maintain Cluster's denormalized scope columns (CachedScopeMixin: _site/_location/_region/_site_group)
|
||||
via PostgreSQL triggers instead of the Python `dcim.signals.sync_cached_scope_fields` handler.
|
||||
"""
|
||||
from django.db import migrations
|
||||
|
||||
from utilities.migration import cached_scope_triggers
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('virtualization', '0056_virtualmachine_render_config_permission'),
|
||||
# Source tables (dcim_site, dcim_location) must already exist.
|
||||
('dcim', '0238_ltree_paths'),
|
||||
]
|
||||
|
||||
operations = cached_scope_triggers('virtualization_cluster')
|
||||
|
|
@ -0,0 +1,19 @@
|
|||
"""
|
||||
Maintain WirelessLAN's denormalized scope columns (CachedScopeMixin: _site/_location/_region/
|
||||
_site_group) via PostgreSQL triggers instead of the Python `dcim.signals.sync_cached_scope_fields`
|
||||
handler.
|
||||
"""
|
||||
from django.db import migrations
|
||||
|
||||
from utilities.migration import cached_scope_triggers
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('wireless', '0020_ltree_paths'),
|
||||
# Source tables (dcim_site, dcim_location) must already exist.
|
||||
('dcim', '0238_ltree_paths'),
|
||||
]
|
||||
|
||||
operations = cached_scope_triggers('wireless_wirelesslan')
|
||||
Loading…
Reference in New Issue