Improve filtering cables by termination device/rack/site

netbox/dcim/filtersets.py

@@ -1537,27 +1537,35 @@ class CableFilterSet(TenancyFilterSet, NetBoxModelFilterSet):
         choices=ColorChoices
     )
     device_id = MultiValueNumberFilter(
-        method='filter_device'
+        method='filter_by_termination'
     )
     device = MultiValueCharFilter(
-        method='filter_device',
+        method='filter_by_termination',
         field_name='device__name'
     )
     rack_id = MultiValueNumberFilter(
-        method='filter_device',
-        field_name='device__rack_id'
+        method='filter_by_termination',
+        field_name='rack_id'
     )
     rack = MultiValueCharFilter(
-        method='filter_device',
-        field_name='device__rack__name'
+        method='filter_by_termination',
+        field_name='rack__name'
+    )
+    location_id = MultiValueNumberFilter(
+        method='filter_by_termination',
+        field_name='location_id'
+    )
+    location = MultiValueCharFilter(
+        method='filter_by_termination',
+        field_name='location__name'
     )
     site_id = MultiValueNumberFilter(
-        method='filter_device',
-        field_name='device__site_id'
+        method='filter_by_termination',
+        field_name='site_id'
     )
     site = MultiValueCharFilter(
-        method='filter_device',
-        field_name='device__site__slug'
+        method='filter_by_termination',
+        field_name='site__slug'
     )
 
     class Meta:
@@ -1569,12 +1577,10 @@ class CableFilterSet(TenancyFilterSet, NetBoxModelFilterSet):
             return queryset
         return queryset.filter(label__icontains=value)
 
-    def filter_device(self, queryset, name, value):
-        queryset = queryset.filter(
-            Q(**{f'_termination_a_{name}__in': value}) |
-            Q(**{f'_termination_b_{name}__in': value})
-        )
-        return queryset
+    def filter_by_termination(self, queryset, name, value):
+        # Filter by a related object cached on CableTermination. Note the underscore preceding the field name.
+        # Supported objects: device, rack, location, site
+        return queryset.filter(**{f'terminations___{name}__in': value}).distinct()
 
 
 class CableTerminationFilterSet(BaseFilterSet):

netbox/dcim/forms/filtersets.py

@@ -730,7 +730,7 @@ class CableFilterForm(TenancyFilterForm, NetBoxModelFilterSetForm):
     model = Cable
     fieldsets = (
         (None, ('q', 'tag')),
-        ('Location', ('site_id', 'rack_id', 'device_id')),
+        ('Location', ('site_id', 'location_id', 'rack_id', 'device_id')),
         ('Attributes', ('type', 'status', 'color', 'length', 'length_unit')),
         ('Tenant', ('tenant_group_id', 'tenant_id')),
     )
@@ -747,13 +747,23 @@ class CableFilterForm(TenancyFilterForm, NetBoxModelFilterSetForm):
         },
         label=_('Site')
     )
+    location_id = DynamicModelMultipleChoiceField(
+        queryset=Location.objects.all(),
+        required=False,
+        label=_('Location'),
+        null_option='None',
+        query_params={
+            'site_id': '$site_id'
+        }
+    )
     rack_id = DynamicModelMultipleChoiceField(
         queryset=Rack.objects.all(),
         required=False,
         label=_('Rack'),
         null_option='None',
         query_params={
-            'site_id': '$site_id'
+            'site_id': '$site_id',
+            'location_id': '$location_id',
         }
     )
     device_id = DynamicModelMultipleChoiceField(
@@ -761,8 +771,9 @@ class CableFilterForm(TenancyFilterForm, NetBoxModelFilterSetForm):
         required=False,
         query_params={
             'site_id': '$site_id',
-            'tenant_id': '$tenant_id',
+            'location_id': '$location_id',
             'rack_id': '$rack_id',
+            'tenant_id': '$tenant_id',
         },
         label=_('Device')
     )

netbox/dcim/migrations/0157_new_cabling_models.py

@@ -20,6 +20,10 @@ class Migration(migrations.Migration):
                 ('termination_id', models.PositiveBigIntegerField()),
                 ('cable', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='terminations', to='dcim.cable')),
                 ('termination_type', models.ForeignKey(limit_choices_to=models.Q(models.Q(models.Q(('app_label', 'circuits'), ('model__in', ('circuittermination',))), models.Q(('app_label', 'dcim'), ('model__in', ('consoleport', 'consoleserverport', 'frontport', 'interface', 'powerfeed', 'poweroutlet', 'powerport', 'rearport'))), _connector='OR')), on_delete=django.db.models.deletion.PROTECT, related_name='+', to='contenttypes.contenttype')),
+                ('_device', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='dcim.device')),
+                ('_rack', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='dcim.rack')),
+                ('_location', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='dcim.location')),
+                ('_site', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='dcim.site')),
             ],
             options={
                 'ordering': ('cable', 'cable_end', 'pk'),

netbox/dcim/migrations/0158_populate_cable_terminations.py

@@ -1,10 +1,37 @@
 from django.db import migrations
 
 
+def cache_related_objects(termination):
+    """
+    Replicate caching logic from CableTermination.cache_related_objects()
+    """
+    attrs = {}
+
+    # Device components
+    if getattr(termination, 'device', None):
+        attrs['_device'] = termination.device
+        attrs['_rack'] = termination.device.rack
+        attrs['_location'] = termination.device.location
+        attrs['_site'] = termination.device.site
+
+    # Power feeds
+    elif getattr(termination, 'rack', None):
+        attrs['_rack'] = termination.rack
+        attrs['_location'] = termination.rack.location
+        attrs['_site'] = termination.rack.site
+
+    # Circuit terminations
+    elif getattr(termination, 'site', None):
+        attrs['_site'] = termination.site
+
+    return attrs
+
+
 def populate_cable_terminations(apps, schema_editor):
     """
     Replicate terminations from the Cable model into CableTermination instances.
     """
+    ContentType = apps.get_model('contenttypes', 'ContentType')
     Cable = apps.get_model('dcim', 'Cable')
     CableTermination = apps.get_model('dcim', 'CableTermination')
 
@@ -16,22 +43,20 @@ def populate_cable_terminations(apps, schema_editor):
     # Queue CableTerminations to be created
     cable_terminations = []
     for i, cable in enumerate(cables, start=1):
-        cable_terminations.append(
-            CableTermination(
-                cable_id=cable['id'],
-                cable_end='A',
-                termination_type_id=cable['termination_a_type'],
-                termination_id=cable['termination_a_id']
-            )
-        )
-        cable_terminations.append(
-            CableTermination(
+        for cable_end in ('a', 'b'):
+            # We must manually instantiate the termination object, because GFK fields are not
+            # supported within migrations.
+            termination_ct = ContentType.objects.get(pk=cable[f'termination_{cable_end}_type'])
+            termination_model = apps.get_model(termination_ct.app_label, termination_ct.model)
+            termination = termination_model.objects.get(pk=cable[f'termination_{cable_end}_id'])
+
+            cable_terminations.append(CableTermination(
                 cable_id=cable['id'],
-                cable_end='B',
-                termination_type_id=cable['termination_b_type'],
-                termination_id=cable['termination_b_id']
-            )
-        )
+                cable_end=cable_end.upper(),
+                termination_type_id=cable[f'termination_{cable_end}_type'],
+                termination_id=cable[f'termination_{cable_end}_id'],
+                **cache_related_objects(termination)
+            ))
 
     # Bulk create the termination objects
     CableTermination.objects.bulk_create(cable_terminations, batch_size=100)

netbox/dcim/migrations/0161_cabling_cleanup.py

@@ -34,6 +34,14 @@ class Migration(migrations.Migration):
             model_name='cable',
             name='termination_b_type',
         ),
+        migrations.RemoveField(
+            model_name='cable',
+            name='_termination_a_device',
+        ),
+        migrations.RemoveField(
+            model_name='cable',
+            name='_termination_b_device',
+        ),
 
         # Remove old fields from CablePath
         migrations.AlterUniqueTogether(

netbox/dcim/models/cables.py

@@ -19,7 +19,6 @@ from utilities.querysets import RestrictedQuerySet
 from utilities.utils import to_meters
 from wireless.models import WirelessLink
 from .device_components import FrontPort, RearPort
-from .devices import Device
 
 __all__ = (
     'Cable',
@@ -81,22 +80,6 @@ class Cable(NetBoxModel):
         blank=True,
         null=True
     )
-    # Cache the associated device (where applicable) for the A and B terminations. This enables filtering of Cables by
-    # their associated Devices.
-    _termination_a_device = models.ForeignKey(
-        to=Device,
-        on_delete=models.CASCADE,
-        related_name='+',
-        blank=True,
-        null=True
-    )
-    _termination_b_device = models.ForeignKey(
-        to=Device,
-        on_delete=models.CASCADE,
-        related_name='+',
-        blank=True,
-        null=True
-    )
 
     class Meta:
         ordering = ('pk',)
@@ -167,13 +150,6 @@ class Cable(NetBoxModel):
         else:
             self._abs_length = None
 
-        # TODO: Need to come with a proper solution for filtering by termination parent
-        # Store the parent Device for the A and B terminations (if applicable) to enable filtering
-        if hasattr(self, 'a_terminations'):
-            self._termination_a_device = getattr(self.a_terminations[0], 'device', None)
-        if hasattr(self, 'b_terminations'):
-            self._termination_b_device = getattr(self.b_terminations[0], 'device', None)
-
         super().save(*args, **kwargs)
 
         # Update the private pk used in __str__ in case this is a new object (i.e. just got its pk)
@@ -247,6 +223,32 @@ class CableTermination(models.Model):
         fk_field='termination_id'
     )
 
+    # Cached associations to enable efficient filtering
+    _device = models.ForeignKey(
+        to='dcim.Device',
+        on_delete=models.CASCADE,
+        blank=True,
+        null=True
+    )
+    _rack = models.ForeignKey(
+        to='dcim.Rack',
+        on_delete=models.CASCADE,
+        blank=True,
+        null=True
+    )
+    _location = models.ForeignKey(
+        to='dcim.Location',
+        on_delete=models.CASCADE,
+        blank=True,
+        null=True
+    )
+    _site = models.ForeignKey(
+        to='dcim.Site',
+        on_delete=models.CASCADE,
+        blank=True,
+        null=True
+    )
+
     objects = RestrictedQuerySet.as_manager()
 
     class Meta:
@@ -277,6 +279,10 @@ class CableTermination(models.Model):
             })
 
     def save(self, *args, **kwargs):
+
+        # Cache objects associated with the terminating object (for filtering)
+        self.cache_related_objects()
+
         super().save(*args, **kwargs)
 
         # Set the cable on the terminating object
@@ -297,6 +303,30 @@ class CableTermination(models.Model):
 
         super().delete(*args, **kwargs)
 
+    def cache_related_objects(self):
+        """
+        Cache objects related to the termination (e.g. device, rack, site) directly on the object to
+        enable efficient filtering.
+        """
+        assert self.termination is not None
+
+        # Device components
+        if getattr(self.termination, 'device', None):
+            self._device = self.termination.device
+            self._rack = self.termination.device.rack
+            self._location = self.termination.device.location
+            self._site = self.termination.device.site
+
+        # Power feeds
+        elif getattr(self.termination, 'rack', None):
+            self._rack = self.termination.rack
+            self._location = self.termination.rack.location
+            self._site = self.termination.rack.site
+
+        # Circuit terminations
+        elif getattr(self.termination, 'site', None):
+            self._site = self.termination.site
+
 
 class CablePath(models.Model):
     """

netbox/dcim/models/device_components.py

@@ -126,6 +126,13 @@ class CabledObjectModel(models.Model):
         help_text="Treat as if a cable is connected"
     )
 
+    cable_terminations = GenericRelation(
+        to='dcim.CableTermination',
+        content_type_field='termination_type',
+        object_id_field='termination_id',
+        related_query_name='%(class)s',
+    )
+
     class Meta:
         abstract = True
 

netbox/dcim/tests/test_filtersets.py

@@ -3663,6 +3663,21 @@ class CableTestCase(TestCase, ChangeLoggedFilterSetTests):
         )
         Site.objects.bulk_create(sites)
 
+        locations = (
+            Location(name='Location 1', site=sites[0], slug='location-1'),
+            Location(name='Location 2', site=sites[1], slug='location-1'),
+            Location(name='Location 3', site=sites[2], slug='location-1'),
+        )
+        for location in locations:
+            location.save()
+
+        racks = (
+            Rack(name='Rack 1', site=sites[0], location=locations[0]),
+            Rack(name='Rack 2', site=sites[1], location=locations[1]),
+            Rack(name='Rack 3', site=sites[2], location=locations[2]),
+        )
+        Rack.objects.bulk_create(racks)
+
         tenants = (
             Tenant(name='Tenant 1', slug='tenant-1'),
             Tenant(name='Tenant 2', slug='tenant-2'),
@@ -3670,24 +3685,17 @@ class CableTestCase(TestCase, ChangeLoggedFilterSetTests):
         )
         Tenant.objects.bulk_create(tenants)
 
-        racks = (
-            Rack(name='Rack 1', site=sites[0]),
-            Rack(name='Rack 2', site=sites[1]),
-            Rack(name='Rack 3', site=sites[2]),
-        )
-        Rack.objects.bulk_create(racks)
-
         manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
         device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Model 1', slug='model-1')
         device_role = DeviceRole.objects.create(name='Device Role 1', slug='device-role-1')
 
         devices = (
-            Device(name='Device 1', device_type=device_type, device_role=device_role, site=sites[0], rack=racks[0], position=1),
-            Device(name='Device 2', device_type=device_type, device_role=device_role, site=sites[0], rack=racks[0], position=2),
-            Device(name='Device 3', device_type=device_type, device_role=device_role, site=sites[1], rack=racks[1], position=1),
-            Device(name='Device 4', device_type=device_type, device_role=device_role, site=sites[1], rack=racks[1], position=2),
-            Device(name='Device 5', device_type=device_type, device_role=device_role, site=sites[2], rack=racks[2], position=1),
-            Device(name='Device 6', device_type=device_type, device_role=device_role, site=sites[2], rack=racks[2], position=2),
+            Device(name='Device 1', device_type=device_type, device_role=device_role, site=sites[0], rack=racks[0], location=locations[0], position=1),
+            Device(name='Device 2', device_type=device_type, device_role=device_role, site=sites[0], rack=racks[0], location=locations[0], position=2),
+            Device(name='Device 3', device_type=device_type, device_role=device_role, site=sites[1], rack=racks[1], location=locations[1], position=1),
+            Device(name='Device 4', device_type=device_type, device_role=device_role, site=sites[1], rack=racks[1], location=locations[1], position=2),
+            Device(name='Device 5', device_type=device_type, device_role=device_role, site=sites[2], rack=racks[2], location=locations[2], position=1),
+            Device(name='Device 6', device_type=device_type, device_role=device_role, site=sites[2], rack=racks[2], location=locations[2], position=2),
         )
         Device.objects.bulk_create(devices)
 
@@ -3759,6 +3767,13 @@ class CableTestCase(TestCase, ChangeLoggedFilterSetTests):
         params = {'rack': [racks[0].name, racks[1].name]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 6)
 
+    def test_location(self):
+        locations = Location.objects.all()[:2]
+        params = {'location_id': [locations[0].pk, locations[1].pk]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 6)
+        params = {'location': [locations[0].name, locations[1].name]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 6)
+
     def test_site(self):
         site = Site.objects.all()[:2]
         params = {'site_id': [site[0].pk, site[1].pk]}