Преглед изворни кода

Fixes #23505: Avoid redundant predicates when filtering by a non-unique interface name

Use DISTINCT ON (name) querysets for the interface and VM interface name filters, so that form validation resolves each name to a single object and the filter emits one predicate per name rather than one per matching interface.
Jeremy Stretch пре 5 дана
родитељ
комит
fd22e1edf1

+ 3 - 2
netbox/dcim/filtersets.py

@@ -2255,9 +2255,10 @@ class MACAddressFilterSet(PrimaryModelFilterSet):
         field_name='pk',
         label=_('Virtual machine (ID)'),
     )
+    # Interface names aren't unique, so match on distinct names to avoid redundant predicates
     interface = django_filters.ModelMultipleChoiceFilter(
         field_name='interface__name',
-        queryset=Interface.objects.all(),
+        queryset=Interface.objects.order_by('name').distinct('name'),
         to_field_name='name',
         label=_('Interface (name)'),
     )
@@ -2268,7 +2269,7 @@ class MACAddressFilterSet(PrimaryModelFilterSet):
     )
     vminterface = django_filters.ModelMultipleChoiceFilter(
         field_name='vminterface__name',
-        queryset=VMInterface.objects.all(),
+        queryset=VMInterface.objects.order_by('name').distinct('name'),
         to_field_name='name',
         label=_('VM interface (name)'),
     )

+ 13 - 0
netbox/dcim/tests/test_filtersets.py

@@ -9067,6 +9067,19 @@ class MACAddressTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'vminterface': [vm_interfaces[0].name, vm_interfaces[1].name]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_interface_shared_name(self):
+        # A name shared by several interfaces should resolve to a single filter value
+        Interface.objects.create(
+            device=Device.objects.get(name='Device 2'), name='Interface 1', type=InterfaceTypeChoices.TYPE_1GE_FIXED
+        )
+        VMInterface.objects.create(
+            virtual_machine=VirtualMachine.objects.get(name='Virtual Machine 2'), name='Interface 1'
+        )
+        for param in ('interface', 'vminterface'):
+            filterset = self.filterset({param: ['Interface 1']}, self.queryset)
+            self.assertEqual(filterset.qs.count(), 1)
+            self.assertEqual(len(filterset.form.cleaned_data[param]), 1)
+
     def test_assigned(self):
         params = {'assigned': True}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 8)

+ 3 - 2
netbox/ipam/filtersets.py

@@ -675,9 +675,10 @@ class IPAddressFilterSet(PrimaryModelFilterSet, TenancyFilterSet, ContactModelFi
         field_name='pk',
         label=_('Virtual machine (ID)'),
     )
+    # Interface names aren't unique, so match on distinct names to avoid redundant predicates
     interface = django_filters.ModelMultipleChoiceFilter(
         field_name='interface__name',
-        queryset=Interface.objects.all(),
+        queryset=Interface.objects.order_by('name').distinct('name'),
         to_field_name='name',
         label=_('Interface (name)'),
     )
@@ -688,7 +689,7 @@ class IPAddressFilterSet(PrimaryModelFilterSet, TenancyFilterSet, ContactModelFi
     )
     vminterface = django_filters.ModelMultipleChoiceFilter(
         field_name='vminterface__name',
-        queryset=VMInterface.objects.all(),
+        queryset=VMInterface.objects.order_by('name').distinct('name'),
         to_field_name='name',
         label=_('VM interface (name)'),
     )

+ 13 - 0
netbox/ipam/tests/test_filtersets.py

@@ -1467,6 +1467,19 @@ class IPAddressTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'vminterface': ['Interface 1', 'Interface 2']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_interface_shared_name(self):
+        # A name shared by several interfaces should resolve to a single filter value
+        Interface.objects.create(
+            device=Device.objects.get(name='Device 2'), name='Interface 1', type=InterfaceTypeChoices.TYPE_1GE_FIXED
+        )
+        VMInterface.objects.create(
+            virtual_machine=VirtualMachine.objects.get(name='Virtual Machine 2'), name='Interface 1'
+        )
+        for param in ('interface', 'vminterface'):
+            filterset = self.filterset({param: ['Interface 1']}, self.queryset)
+            self.assertEqual(filterset.qs.count(), 1)
+            self.assertEqual(len(filterset.form.cleaned_data[param]), 1)
+
     def test_fhrpgroup(self):
         fhrp_groups = FHRPGroup.objects.all()[:2]
         params = {'fhrpgroup_id': [fhrp_groups[0].pk, fhrp_groups[1].pk]}

+ 6 - 4
netbox/vpn/filtersets.py

@@ -105,9 +105,10 @@ class TunnelTerminationFilterSet(NetBoxModelFilterSet):
         distinct=False,
     )
     termination_type = MultiValueContentTypeFilter()
+    # Interface names aren't unique, so match on distinct names to avoid redundant predicates
     interface = django_filters.ModelMultipleChoiceFilter(
         field_name='interface__name',
-        queryset=Interface.objects.all(),
+        queryset=Interface.objects.order_by('name').distinct('name'),
         to_field_name='name',
         label=_('Interface (name)'),
     )
@@ -118,7 +119,7 @@ class TunnelTerminationFilterSet(NetBoxModelFilterSet):
     )
     vminterface = django_filters.ModelMultipleChoiceFilter(
         field_name='vminterface__name',
-        queryset=VMInterface.objects.all(),
+        queryset=VMInterface.objects.order_by('name').distinct('name'),
         to_field_name='name',
         label=_('VM interface (name)'),
     )
@@ -438,9 +439,10 @@ class L2VPNTerminationFilterSet(NetBoxModelFilterSet):
         queryset=VirtualMachine.objects.all(),
         label=_('Virtual machine (ID)'),
     )
+    # Interface names aren't unique, so match on distinct names to avoid redundant predicates
     interface = django_filters.ModelMultipleChoiceFilter(
         field_name='interface__name',
-        queryset=Interface.objects.all(),
+        queryset=Interface.objects.order_by('name').distinct('name'),
         to_field_name='name',
         label=_('Interface (name)'),
     )
@@ -451,7 +453,7 @@ class L2VPNTerminationFilterSet(NetBoxModelFilterSet):
     )
     vminterface = django_filters.ModelMultipleChoiceFilter(
         field_name='vminterface__name',
-        queryset=VMInterface.objects.all(),
+        queryset=VMInterface.objects.order_by('name').distinct('name'),
         to_field_name='name',
         label=_('VM interface (name)'),
     )

+ 26 - 0
netbox/vpn/tests/test_filtersets.py

@@ -293,6 +293,19 @@ class TunnelTerminationTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'vminterface': [vm_interfaces[0].name, vm_interfaces[1].name]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_interface_shared_name(self):
+        # A name shared by several interfaces should resolve to a single filter value
+        Interface.objects.create(
+            device=create_test_device('Device 2'), name='Interface 1', type=InterfaceTypeChoices.TYPE_VIRTUAL
+        )
+        VMInterface.objects.create(
+            virtual_machine=create_test_virtualmachine('Virtual Machine 2'), name='Interface 1'
+        )
+        for param in ('interface', 'vminterface'):
+            filterset = self.filterset({param: ['Interface 1']}, self.queryset)
+            self.assertEqual(filterset.qs.count(), 1)
+            self.assertEqual(len(filterset.form.cleaned_data[param]), 1)
+
     def test_outside_ip(self):
         ip_addresses = IPAddress.objects.all()[:2]
         params = {'outside_ip_id': [ip_addresses[0].pk, ip_addresses[1].pk]}
@@ -921,6 +934,19 @@ class L2VPNTerminationTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'vminterface_id': [vminterfaces[0].pk, vminterfaces[1].pk]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_interface_shared_name(self):
+        # A name shared by several interfaces should resolve to a single filter value
+        Interface.objects.create(
+            device=create_test_device('Device 2'), name='Interface 1', type=InterfaceTypeChoices.TYPE_1GE_FIXED
+        )
+        VMInterface.objects.create(
+            virtual_machine=create_test_virtualmachine('Virtual Machine 2'), name='Interface 1'
+        )
+        for param in ('interface', 'vminterface'):
+            filterset = self.filterset({param: ['Interface 1']}, self.queryset)
+            self.assertEqual(filterset.qs.count(), 1)
+            self.assertEqual(len(filterset.form.cleaned_data[param]), 1)
+
     def test_vlan(self):
         vlans = VLAN.objects.all()[:2]
         params = {'vlan_id': [vlans[0].pk, vlans[1].pk]}