#21355 - Handle updates to denormalized data via PostgreSQL triggers

docs/development/application-registry.md

@@ -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.

netbox/circuits/apps.py

@@ -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',
-        })

netbox/circuits/migrations/0058_denormalization_triggers.py

@@ -0,0 +1,33 @@
+"""
+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 InstallDenormalizationTrigger
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('circuits', '0057_default_ordering_indexes'),
+        # Source tables (dcim_site, dcim_location) must already exist.
+        ('dcim', '0238_ltree_paths'),
+    ]
+
+    operations = [
+        # When a Site's region/group changes, propagate to terminations assigned to it.
+        InstallDenormalizationTrigger(
+            dependent_table='circuits_circuittermination',
+            source_table='dcim_site',
+            fk_column='_site_id',
+            mappings={'_region_id': 'region_id', '_site_group_id': 'group_id'},
+        ),
+        # When a Location's site changes, propagate to terminations assigned to it.
+        InstallDenormalizationTrigger(
+            dependent_table='circuits_circuittermination',
+            source_table='dcim_location',
+            fk_column='_location_id',
+            mappings={'_site_id': 'site_id'},
+        ),
+    ]

netbox/circuits/tests/test_models.py

@@ -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,78 @@ 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):
+        site_a = Site.objects.create(name='Site A', slug='site-a')
+        site_b = Site.objects.create(name='Site B', slug='site-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 should update _site.
+        location.site = site_b
+        location.save()
+
+        termination.refresh_from_db()
+        self.assertEqual(termination._site, site_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)

netbox/dcim/apps.py

@@ -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)

netbox/dcim/migrations/0239_denormalization_triggers.py

@@ -0,0 +1,38 @@
+"""
+Maintain CableTermination's denormalized device/rack/location/site 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 InstallDenormalizationTrigger
+
+
+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'},
+        ),
+    ]

netbox/dcim/tests/test_signals.py

@@ -9,6 +9,7 @@ from dcim.choices import CableEndChoices, LinkStatusChoices
 from dcim.models import (
     Cable,
     CablePath,
+    CableTermination,
     Device,
     DeviceRole,
     DeviceType,
@@ -486,3 +487,63 @@ 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)

netbox/ipam/apps.py

@@ -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',
-        })

netbox/ipam/migrations/0091_denormalization_triggers.py

@@ -0,0 +1,33 @@
+"""
+Maintain Prefix'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 InstallDenormalizationTrigger
+
+
+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 = [
+        # When a Site's region/group changes, propagate to prefixes assigned to it.
+        InstallDenormalizationTrigger(
+            dependent_table='ipam_prefix',
+            source_table='dcim_site',
+            fk_column='_site_id',
+            mappings={'_region_id': 'region_id', '_site_group_id': 'group_id'},
+        ),
+        # When a Location's site changes, propagate to prefixes assigned to it.
+        InstallDenormalizationTrigger(
+            dependent_table='ipam_prefix',
+            source_table='dcim_location',
+            fk_column='_location_id',
+            mappings={'_site_id': 'site_id'},
+        ),
+    ]

netbox/ipam/tests/test_signals.py

@@ -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,72 @@ 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):
+        site_a = Site.objects.create(name='Site A', slug='site-a')
+        site_b = Site.objects.create(name='Site B', slug='site-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)
+
+        location.site = site_b
+        location.save()
+
+        prefix.refresh_from_db()
+        self.assertEqual(prefix._site, site_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)

netbox/netbox/denormalized.py

@@ -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')

netbox/netbox/registry.py

@@ -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),

netbox/utilities/migration.py

@@ -1,8 +1,9 @@
-from django.db import models
+from django.db import migrations, models
 
 from netbox.config import ConfigItem
 
 __all__ = (
+    'InstallDenormalizationTrigger',
     'custom_deconstruct',
 )
 
@@ -32,3 +33,75 @@ 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` handler formerly defined in `netbox.denormalized`.
+
+    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'}).
+
+    The trigger fires AFTER UPDATE of the source columns, and only when at least one of them actually changed.
+    Like the handler it replaces, it does not fire on INSERT (a newly created source row has no dependents
+    yet) and it does not cascade: updating the denormalized columns does not itself trigger further
+    denormalization.
+    """
+    reversible = True
+
+    def __init__(self, dependent_table, source_table, fk_column, mappings):
+        self.dependent_table = dependent_table
+        self.source_table = source_table
+        self.fk_column = fk_column
+        self.mappings = 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):
+        source_columns = list(self.mappings.values())
+        set_clause = ', '.join(f'"{dest}" = NEW."{src}"' for dest, src in self.mappings.items())
+        update_of = ', '.join(f'"{col}"' for col in source_columns)
+        when_clause = ' OR '.join(f'OLD."{col}" IS DISTINCT FROM NEW."{col}"' for col in source_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;
+        ''')
+        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}'