Просмотр исходного кода

Merge pull request #23524 from netbox-community/23505-distinct-on-name-filters

Fixes #23505: Avoid redundant predicates when filtering by non-unique names
bctiemann 9 часов назад
Родитель
Сommit
fd71a73513

+ 14 - 13
netbox/dcim/filtersets.py

@@ -33,6 +33,7 @@ from utilities.filters import (
     MultiValueMACAddressFilter,
     MultiValueTimeZoneFilter,
     MultiValueWWNFilter,
+    NonUniqueModelMultipleChoiceFilter,
     NumericArrayFilter,
     TreeNodeMultipleChoiceFilter,
 )
@@ -286,7 +287,7 @@ class LocationFilterSet(TenancyFilterSet, ContactModelFilterSet, NestedGroupMode
         distinct=False,
         label=_('Parent location (ID)'),
     )
-    parent = django_filters.ModelMultipleChoiceFilter(
+    parent = NonUniqueModelMultipleChoiceFilter(
         field_name='parent__slug',
         queryset=Location.objects.all(),
         distinct=False,
@@ -1846,7 +1847,7 @@ class ModuleFilterSet(PrimaryModelFilterSet):
         distinct=False,
         label=_('Location (ID)'),
     )
-    location = django_filters.ModelMultipleChoiceFilter(
+    location = NonUniqueModelMultipleChoiceFilter(
         field_name='device__location__slug',
         queryset=Location.objects.all(),
         distinct=False,
@@ -1859,7 +1860,7 @@ class ModuleFilterSet(PrimaryModelFilterSet):
         distinct=False,
         label=_('Rack (ID)'),
     )
-    rack = django_filters.ModelMultipleChoiceFilter(
+    rack = NonUniqueModelMultipleChoiceFilter(
         field_name='device__rack__name',
         queryset=Rack.objects.all(),
         distinct=False,
@@ -1871,7 +1872,7 @@ class ModuleFilterSet(PrimaryModelFilterSet):
         distinct=False,
         label=_('Device (ID)'),
     )
-    device = django_filters.ModelMultipleChoiceFilter(
+    device = NonUniqueModelMultipleChoiceFilter(
         field_name='device__name',
         queryset=Device.objects.all(),
         distinct=False,
@@ -1953,7 +1954,7 @@ class DeviceComponentFilterSet(OwnerFilterMixin, NetBoxModelFilterSet):
         distinct=False,
         label=_('Location (ID)'),
     )
-    location = django_filters.ModelMultipleChoiceFilter(
+    location = NonUniqueModelMultipleChoiceFilter(
         field_name='_location__slug',
         queryset=Location.objects.all(),
         distinct=False,
@@ -1966,7 +1967,7 @@ class DeviceComponentFilterSet(OwnerFilterMixin, NetBoxModelFilterSet):
         distinct=False,
         label=_('Rack (ID)'),
     )
-    rack = django_filters.ModelMultipleChoiceFilter(
+    rack = NonUniqueModelMultipleChoiceFilter(
         field_name='_rack__name',
         queryset=Rack.objects.all(),
         distinct=False,
@@ -1978,7 +1979,7 @@ class DeviceComponentFilterSet(OwnerFilterMixin, NetBoxModelFilterSet):
         distinct=False,
         label=_('Device (ID)'),
     )
-    device = django_filters.ModelMultipleChoiceFilter(
+    device = NonUniqueModelMultipleChoiceFilter(
         field_name='device__name',
         queryset=Device.objects.all(),
         distinct=False,
@@ -2255,7 +2256,7 @@ class MACAddressFilterSet(PrimaryModelFilterSet):
         field_name='pk',
         label=_('Virtual machine (ID)'),
     )
-    interface = django_filters.ModelMultipleChoiceFilter(
+    interface = NonUniqueModelMultipleChoiceFilter(
         field_name='interface__name',
         queryset=Interface.objects.all(),
         to_field_name='name',
@@ -2266,7 +2267,7 @@ class MACAddressFilterSet(PrimaryModelFilterSet):
         queryset=Interface.objects.all(),
         label=_('Interface (ID)'),
     )
-    vminterface = django_filters.ModelMultipleChoiceFilter(
+    vminterface = NonUniqueModelMultipleChoiceFilter(
         field_name='vminterface__name',
         queryset=VMInterface.objects.all(),
         to_field_name='name',
@@ -2516,13 +2517,13 @@ class InterfaceFilterSet(
         queryset=VirtualDeviceContext.objects.all(),
         label=_('Virtual Device Context')
     )
-    vdc_identifier = django_filters.ModelMultipleChoiceFilter(
+    vdc_identifier = NonUniqueModelMultipleChoiceFilter(
         field_name='vdcs__identifier',
         queryset=VirtualDeviceContext.objects.all(),
         to_field_name='identifier',
         label=_('Virtual Device Context (Identifier)')
     )
-    vdc = django_filters.ModelMultipleChoiceFilter(
+    vdc = NonUniqueModelMultipleChoiceFilter(
         field_name='vdcs__name',
         queryset=VirtualDeviceContext.objects.all(),
         to_field_name='name',
@@ -2674,7 +2675,7 @@ class DeviceBayFilterSet(DeviceComponentFilterSet):
         distinct=False,
         label=_('Installed device (ID)'),
     )
-    installed_device = django_filters.ModelMultipleChoiceFilter(
+    installed_device = NonUniqueModelMultipleChoiceFilter(
         field_name='installed_device__name',
         queryset=Device.objects.all(),
         distinct=False,
@@ -2761,7 +2762,7 @@ class VirtualChassisFilterSet(PrimaryModelFilterSet):
         distinct=False,
         label=_('Master (ID)'),
     )
-    master = django_filters.ModelMultipleChoiceFilter(
+    master = NonUniqueModelMultipleChoiceFilter(
         field_name='master__name',
         queryset=Device.objects.all(),
         distinct=False,

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

@@ -61,6 +61,26 @@ class DeviceComponentFilterSetTestMixin:
         params = {'tenant': [tenants[0].slug, tenants[1].slug]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_shared_names(self):
+        # A name or slug shared by several objects should resolve to a single filter value which still matches
+        # components on each of those objects
+        component = self.queryset.filter(_rack__isnull=False, _location__isnull=False).first()
+        device, rack, location = component.device, component._rack, component._location
+        params = (('device', device.name), ('rack', rack.name), ('location', location.slug))
+        counts = {param: self.filterset({param: [value]}, self.queryset).qs.count() for param, value in params}
+
+        # Move the component to a same-named device, rack, and location in another site
+        site = Site.objects.create(name='Site 99', slug='site-99')
+        self.queryset.filter(pk=component.pk).update(
+            device=Device.objects.create(site=site, name=device.name, device_type=device.device_type, role=device.role),
+            _rack=Rack.objects.create(site=site, name=rack.name),
+            _location=Location.objects.create(site=site, name=location.name, slug=location.slug),
+        )
+        for param, value in params:
+            filterset = self.filterset({param: [value]}, self.queryset)
+            self.assertEqual(filterset.qs.count(), counts[param])
+            self.assertEqual(len(filterset.form.cleaned_data[param]), 1)
+
 
 class DeviceComponentTemplateFilterSetTestMixin:
 
@@ -572,6 +592,15 @@ class LocationTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'parent': [locations[0].slug, locations[1].slug]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_parent_shared_slug(self):
+        # A location slug shared by several sites should resolve to a single filter value
+        location = Location.objects.first()
+        site = Site.objects.create(name='Site 99', slug='site-99')
+        Location.objects.create(site=site, name=location.name, slug=location.slug)
+        filterset = self.filterset({'parent': [location.slug]}, self.queryset)
+        self.assertTrue(filterset.is_valid())
+        self.assertEqual(len(filterset.form.cleaned_data['parent']), 1)
+
     def test_ancestor(self):
         locations = Location.objects.filter(parent__isnull=True)[:2]
         params = {'ancestor_id': [locations[0].pk, locations[1].pk]}
@@ -3886,6 +3915,18 @@ class ModuleTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'rack': [racks[0].name, racks[1].name]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 6)
 
+    def test_shared_names(self):
+        # A name or slug shared by several objects should resolve to a single filter value
+        location, rack, device = Location.objects.first(), Rack.objects.first(), Device.objects.first()
+        site = Site.objects.create(name='Site 99', slug='site-99')
+        Location.objects.create(site=site, name=location.name, slug=location.slug)
+        Rack.objects.create(site=site, name=rack.name)
+        Device.objects.create(site=site, name=device.name, device_type=device.device_type, role=device.role)
+        for param, value in (('location', location.slug), ('rack', rack.name), ('device', device.name)):
+            filterset = self.filterset({param: [value]}, self.queryset)
+            self.assertTrue(filterset.is_valid())
+            self.assertEqual(len(filterset.form.cleaned_data[param]), 1)
+
 
 class ConsolePortTestCase(TestCase, DeviceComponentFilterSetTestMixin, ChangeLoggedFilterSetTestMixin):
     queryset = ConsolePort.objects.all()
@@ -6212,6 +6253,20 @@ class InterfaceTestCase(TestCase, DeviceComponentFilterSetTestMixin, ChangeLogge
         params = {'vdc_identifier': vdc.values_list('identifier', flat=True)}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
 
+    def test_vdc_shared_name(self):
+        # A VDC name or identifier shared by several devices should resolve to a single filter value
+        vdc = VirtualDeviceContext.objects.first()
+        VirtualDeviceContext.objects.create(
+            device=Device.objects.exclude(pk=vdc.device_id).first(),
+            name=vdc.name,
+            identifier=vdc.identifier,
+            status=vdc.status,
+        )
+        for param, value in (('vdc', vdc.name), ('vdc_identifier', vdc.identifier)):
+            filterset = self.filterset({param: [value]}, self.queryset)
+            self.assertTrue(filterset.is_valid())
+            self.assertEqual(len(filterset.form.cleaned_data[param]), 1)
+
     def test_vlan(self):
         vlan = VLAN.objects.filter(qinq_role=VLANQinQRoleChoices.ROLE_SERVICE).first()
         params = {'vlan_id': vlan.pk}
@@ -7267,6 +7322,15 @@ class DeviceBayTestCase(TestCase, DeviceComponentFilterSetTestMixin, ChangeLogge
         params = {'device': [devices[0].name, devices[1].name]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_installed_device_shared_name(self):
+        # A device name shared by several sites should resolve to a single filter value
+        device = Device.objects.first()
+        site = Site.objects.create(name='Site 99', slug='site-99')
+        Device.objects.create(site=site, name=device.name, device_type=device.device_type, role=device.role)
+        filterset = self.filterset({'installed_device': [device.name]}, self.queryset)
+        self.assertTrue(filterset.is_valid())
+        self.assertEqual(len(filterset.form.cleaned_data['installed_device']), 1)
+
 
 class InventoryItemTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
     queryset = InventoryItem.objects.all()
@@ -7663,6 +7727,15 @@ class VirtualChassisTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'master': [masters[0].name, masters[2].name]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_master_shared_name(self):
+        # A device name shared by several sites should resolve to a single filter value
+        device = Device.objects.first()
+        site = Site.objects.create(name='Site 99', slug='site-99')
+        Device.objects.create(site=site, name=device.name, device_type=device.device_type, role=device.role)
+        filterset = self.filterset({'master': [device.name]}, self.queryset)
+        self.assertTrue(filterset.is_valid())
+        self.assertEqual(len(filterset.form.cleaned_data['master']), 1)
+
     def test_name(self):
         params = {'name': ['VC 1', 'VC 2']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
@@ -9067,6 +9140,21 @@ 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 = Interface.objects.create(
+            device=Device.objects.get(name='Device 2'), name='Interface 1', type=InterfaceTypeChoices.TYPE_1GE_FIXED
+        )
+        vm_interface = VMInterface.objects.create(
+            virtual_machine=VirtualMachine.objects.get(name='Virtual Machine 2'), name='Interface 1'
+        )
+        MACAddress.objects.create(mac_address='00-00-00-01-01-99', assigned_object=interface)
+        MACAddress.objects.create(mac_address='00-00-00-04-01-99', assigned_object=vm_interface)
+        for param in ('interface', 'vminterface'):
+            filterset = self.filterset({param: ['Interface 1']}, self.queryset)
+            self.assertEqual(filterset.qs.count(), 2)
+            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)

+ 2 - 1
netbox/extras/filtersets.py

@@ -15,6 +15,7 @@ from utilities.filters import (
     MultiValueCharFilter,
     MultiValueContentTypeFilter,
     MultiValueNumberFilter,
+    NonUniqueModelMultipleChoiceFilter,
 )
 from utilities.filtersets import register_filterset
 from virtualization.models import Cluster, ClusterGroup, ClusterType
@@ -760,7 +761,7 @@ class ConfigContextFilterSet(OwnerFilterMixin, ChangeLoggedModelFilterSet):
         queryset=Location.objects.all(),
         label=_('Location'),
     )
-    location = django_filters.ModelMultipleChoiceFilter(
+    location = NonUniqueModelMultipleChoiceFilter(
         field_name='locations__slug',
         queryset=Location.objects.all(),
         to_field_name='slug',

+ 9 - 0
netbox/extras/tests/test_filtersets.py

@@ -1294,6 +1294,15 @@ class ConfigContextTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'location': [locations[0].slug, locations[1].slug]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_location_shared_slug(self):
+        # A location slug shared by several sites should resolve to a single filter value
+        location = Location.objects.first()
+        site = Site.objects.create(name='Site 99', slug='site-99')
+        Location.objects.create(site=site, name=location.name, slug=location.slug)
+        filterset = self.filterset({'location': [location.slug]}, self.queryset)
+        self.assertTrue(filterset.is_valid())
+        self.assertEqual(len(filterset.form.cleaned_data['location']), 1)
+
     def test_device_type(self):
         device_types = DeviceType.objects.all()[:2]
         params = {'device_type_id': [device_types[0].pk, device_types[1].pk]}

+ 5 - 4
netbox/ipam/filtersets.py

@@ -22,6 +22,7 @@ from utilities.filters import (
     MultiValueCharFilter,
     MultiValueContentTypeFilter,
     MultiValueNumberFilter,
+    NonUniqueModelMultipleChoiceFilter,
     TreeNodeMultipleChoiceFilter,
 )
 from utilities.filtersets import register_filterset
@@ -397,7 +398,7 @@ class PrefixFilterSet(PrimaryModelFilterSet, ScopedFilterSet, TenancyFilterSet,
         to_field_name='id',
         label=_('VLAN Group (ID)'),
     )
-    vlan_group = django_filters.ModelMultipleChoiceFilter(
+    vlan_group = NonUniqueModelMultipleChoiceFilter(
         field_name='vlan__group__slug',
         queryset=VLANGroup.objects.all(),
         distinct=False,
@@ -675,7 +676,7 @@ class IPAddressFilterSet(PrimaryModelFilterSet, TenancyFilterSet, ContactModelFi
         field_name='pk',
         label=_('Virtual machine (ID)'),
     )
-    interface = django_filters.ModelMultipleChoiceFilter(
+    interface = NonUniqueModelMultipleChoiceFilter(
         field_name='interface__name',
         queryset=Interface.objects.all(),
         to_field_name='name',
@@ -686,7 +687,7 @@ class IPAddressFilterSet(PrimaryModelFilterSet, TenancyFilterSet, ContactModelFi
         queryset=Interface.objects.all(),
         label=_('Interface (ID)'),
     )
-    vminterface = django_filters.ModelMultipleChoiceFilter(
+    vminterface = NonUniqueModelMultipleChoiceFilter(
         field_name='vminterface__name',
         queryset=VMInterface.objects.all(),
         to_field_name='name',
@@ -1046,7 +1047,7 @@ class VLANFilterSet(PrimaryModelFilterSet, TenancyFilterSet):
         distinct=False,
         label=_('Group (ID)'),
     )
-    group = django_filters.ModelMultipleChoiceFilter(
+    group = NonUniqueModelMultipleChoiceFilter(
         field_name='group__slug',
         queryset=VLANGroup.objects.all(),
         distinct=False,

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

@@ -890,6 +890,15 @@ class PrefixTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'vlan_group': [vlan_groups[0].slug, vlan_groups[1].slug]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
 
+    def test_vlan_group_shared_slug(self):
+        # A VLAN group slug shared by several scopes should resolve to a single filter value
+        group = VLANGroup.objects.first()
+        site = Site.objects.create(name='Site 99', slug='site-99')
+        VLANGroup.objects.create(scope=site, name=group.name, slug=group.slug)
+        filterset = self.filterset({'vlan_group': [group.slug]}, self.queryset)
+        self.assertTrue(filterset.is_valid())
+        self.assertEqual(len(filterset.form.cleaned_data['vlan_group']), 1)
+
     def test_vlan(self):
         vlans = VLAN.objects.all()[:2]
         params = {'vlan_id': [vlans[0].pk, vlans[1].pk]}
@@ -1467,6 +1476,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]}
@@ -2394,6 +2416,15 @@ class VLANTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'group': [groups[0].slug, groups[1].slug]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
 
+    def test_group_shared_slug(self):
+        # A VLAN group slug shared by several scopes should resolve to a single filter value
+        group = VLANGroup.objects.first()
+        site = Site.objects.create(name='Site 99', slug='site-99')
+        VLANGroup.objects.create(scope=site, name=group.name, slug=group.slug)
+        filterset = self.filterset({'group': [group.slug]}, self.queryset)
+        self.assertTrue(filterset.is_valid())
+        self.assertEqual(len(filterset.form.cleaned_data['group']), 1)
+
     def test_description(self):
         params = {'description': ['foobar1', 'foobar2']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)

+ 15 - 0
netbox/utilities/filters.py

@@ -25,6 +25,7 @@ __all__ = (
     'MultiValueTimeFilter',
     'MultiValueTimeZoneFilter',
     'MultiValueWWNFilter',
+    'NonUniqueModelMultipleChoiceFilter',
     'NullableCharFieldFilter',
     'NumericArrayFilter',
     'TreeNodeMultipleChoiceFilter',
@@ -177,6 +178,20 @@ class TreeNodeMultipleChoiceFilter(django_filters.ModelMultipleChoiceFilter):
         return super().filter(qs, value)
 
 
+class NonUniqueModelMultipleChoiceFilter(django_filters.ModelMultipleChoiceFilter):
+    """
+    A ModelMultipleChoiceFilter for a non-unique to_field_name, which resolves each value to a single object.
+    """
+    def __init__(self, *args, **kwargs):
+        if not kwargs.get('to_field_name'):
+            raise ValueError(f"{self.__class__.__name__} requires to_field_name")
+        super().__init__(*args, **kwargs)
+
+    def get_queryset(self, request):
+        to_field_name = self.extra['to_field_name']
+        return super().get_queryset(request).order_by(to_field_name).distinct(to_field_name)
+
+
 class NullableCharFieldFilter(django_filters.CharFilter):
     """
     Allow matching on null field values by passing a special string used to signify NULL.

+ 8 - 0
netbox/utilities/tests/test_filters.py

@@ -37,6 +37,7 @@ from utilities.filters import (
     MultiValueMACAddressFilter,
     MultiValueNumberFilter,
     MultiValueTimeFilter,
+    NonUniqueModelMultipleChoiceFilter,
     TreeNodeMultipleChoiceFilter,
 )
 from wireless.choices import WirelessRoleChoices
@@ -101,6 +102,13 @@ class TreeNodeMultipleChoiceFilterTestCase(TestCase):
         self.assertEqual(qs[1], self.site3)
 
 
+class NonUniqueModelMultipleChoiceFilterTestCase(TestCase):
+
+    def test_to_field_name_required(self):
+        with self.assertRaises(ValueError):
+            NonUniqueModelMultipleChoiceFilter(field_name='device__name', queryset=Device.objects.all())
+
+
 class TagFilterTestCase(TestCase):
     """
     Verify the AND (default), OR (`any`), and NOR (`n`) semantics of TagFilter/TagIDFilter.

+ 9 - 4
netbox/virtualization/filtersets.py

@@ -13,7 +13,12 @@ from ipam.filtersets import PrimaryIPFilterSet
 from netbox.filtersets import NetBoxModelFilterSet, OrganizationalModelFilterSet, PrimaryModelFilterSet
 from tenancy.filtersets import ContactModelFilterSet, TenancyFilterSet
 from users.filterset_mixins import OwnerFilterMixin
-from utilities.filters import MultiValueCharFilter, MultiValueMACAddressFilter, TreeNodeMultipleChoiceFilter
+from utilities.filters import (
+    MultiValueCharFilter,
+    MultiValueMACAddressFilter,
+    NonUniqueModelMultipleChoiceFilter,
+    TreeNodeMultipleChoiceFilter,
+)
 from utilities.filtersets import register_filterset
 
 from .choices import *
@@ -204,7 +209,7 @@ class VirtualMachineFilterSet(
         distinct=False,
         label=_('Device (ID)'),
     )
-    device = django_filters.ModelMultipleChoiceFilter(
+    device = NonUniqueModelMultipleChoiceFilter(
         field_name='device__name',
         queryset=Device.objects.all(),
         distinct=False,
@@ -346,7 +351,7 @@ class VMInterfaceFilterSet(CommonInterfaceFilterSet, OwnerFilterMixin, NetBoxMod
         distinct=False,
         label=_('Virtual machine (ID)'),
     )
-    virtual_machine = django_filters.ModelMultipleChoiceFilter(
+    virtual_machine = NonUniqueModelMultipleChoiceFilter(
         field_name='virtual_machine__name',
         queryset=VirtualMachine.objects.all(),
         distinct=False,
@@ -404,7 +409,7 @@ class VirtualDiskFilterSet(OwnerFilterMixin, NetBoxModelFilterSet):
         distinct=False,
         label=_('Virtual machine (ID)'),
     )
-    virtual_machine = django_filters.ModelMultipleChoiceFilter(
+    virtual_machine = NonUniqueModelMultipleChoiceFilter(
         field_name='virtual_machine__name',
         queryset=VirtualMachine.objects.all(),
         distinct=False,

+ 27 - 0
netbox/virtualization/tests/test_filtersets.py

@@ -601,6 +601,15 @@ class VirtualMachineTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'device': [devices[0].name, devices[1].name]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_device_shared_name(self):
+        # A device name shared by several sites should resolve to a single filter value
+        device = Device.objects.first()
+        site = Site.objects.create(name='Site 99', slug='site-99')
+        Device.objects.create(site=site, name=device.name, device_type=device.device_type, role=device.role)
+        filterset = self.filterset({'device': [device.name]}, self.queryset)
+        self.assertTrue(filterset.is_valid())
+        self.assertEqual(len(filterset.form.cleaned_data['device']), 1)
+
     def test_region(self):
         regions = Region.objects.all()[:2]
         params = {'region_id': [regions[0].pk, regions[1].pk]}
@@ -846,6 +855,15 @@ class VMInterfaceTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'virtual_machine': [vms[0].name, vms[1].name]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_virtual_machine_shared_name(self):
+        # A virtual machine name shared by several clusters should resolve to a single filter value
+        vm = VirtualMachine.objects.first()
+        cluster = Cluster.objects.create(name='Cluster 99', type=vm.cluster.type)
+        VirtualMachine.objects.create(cluster=cluster, name=vm.name)
+        filterset = self.filterset({'virtual_machine': [vm.name]}, self.queryset)
+        self.assertTrue(filterset.is_valid())
+        self.assertEqual(len(filterset.form.cleaned_data['virtual_machine']), 1)
+
     def test_mac_address(self):
         params = {'mac_address': ['00-00-00-00-00-01', '00-00-00-00-00-02']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
@@ -932,6 +950,15 @@ class VirtualDiskTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'virtual_machine': [vms[0].name, vms[1].name]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_virtual_machine_shared_name(self):
+        # A virtual machine name shared by several clusters should resolve to a single filter value
+        vm = VirtualMachine.objects.first()
+        cluster = Cluster.objects.create(name='Cluster 99', type=vm.cluster.type)
+        VirtualMachine.objects.create(cluster=cluster, name=vm.name)
+        filterset = self.filterset({'virtual_machine': [vm.name]}, self.queryset)
+        self.assertTrue(filterset.is_valid())
+        self.assertEqual(len(filterset.form.cleaned_data['virtual_machine']), 1)
+
     def test_name(self):
         params = {'name': ['Disk 1', 'Disk 2']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)

+ 13 - 8
netbox/vpn/filtersets.py

@@ -7,7 +7,12 @@ from dcim.models import Device, Interface
 from ipam.models import VLAN, IPAddress, RouteTarget
 from netbox.filtersets import NetBoxModelFilterSet, OrganizationalModelFilterSet, PrimaryModelFilterSet
 from tenancy.filtersets import ContactModelFilterSet, TenancyFilterSet
-from utilities.filters import MultiValueBigNumberFilter, MultiValueCharFilter, MultiValueContentTypeFilter
+from utilities.filters import (
+    MultiValueBigNumberFilter,
+    MultiValueCharFilter,
+    MultiValueContentTypeFilter,
+    NonUniqueModelMultipleChoiceFilter,
+)
 from utilities.filtersets import register_filterset
 from virtualization.models import VirtualMachine, VMInterface
 
@@ -105,7 +110,7 @@ class TunnelTerminationFilterSet(NetBoxModelFilterSet):
         distinct=False,
     )
     termination_type = MultiValueContentTypeFilter()
-    interface = django_filters.ModelMultipleChoiceFilter(
+    interface = NonUniqueModelMultipleChoiceFilter(
         field_name='interface__name',
         queryset=Interface.objects.all(),
         to_field_name='name',
@@ -116,7 +121,7 @@ class TunnelTerminationFilterSet(NetBoxModelFilterSet):
         queryset=Interface.objects.all(),
         label=_('Interface (ID)'),
     )
-    vminterface = django_filters.ModelMultipleChoiceFilter(
+    vminterface = NonUniqueModelMultipleChoiceFilter(
         field_name='vminterface__name',
         queryset=VMInterface.objects.all(),
         to_field_name='name',
@@ -416,7 +421,7 @@ class L2VPNTerminationFilterSet(NetBoxModelFilterSet):
         field_name='pk',
         label=_('Site (ID)'),
     )
-    device = django_filters.ModelMultipleChoiceFilter(
+    device = NonUniqueModelMultipleChoiceFilter(
         field_name='interface__device__name',
         queryset=Device.objects.all(),
         to_field_name='name',
@@ -427,7 +432,7 @@ class L2VPNTerminationFilterSet(NetBoxModelFilterSet):
         queryset=Device.objects.all(),
         label=_('Device (ID)'),
     )
-    virtual_machine = django_filters.ModelMultipleChoiceFilter(
+    virtual_machine = NonUniqueModelMultipleChoiceFilter(
         field_name='vminterface__virtual_machine__name',
         queryset=VirtualMachine.objects.all(),
         to_field_name='name',
@@ -438,7 +443,7 @@ class L2VPNTerminationFilterSet(NetBoxModelFilterSet):
         queryset=VirtualMachine.objects.all(),
         label=_('Virtual machine (ID)'),
     )
-    interface = django_filters.ModelMultipleChoiceFilter(
+    interface = NonUniqueModelMultipleChoiceFilter(
         field_name='interface__name',
         queryset=Interface.objects.all(),
         to_field_name='name',
@@ -449,7 +454,7 @@ class L2VPNTerminationFilterSet(NetBoxModelFilterSet):
         queryset=Interface.objects.all(),
         label=_('Interface (ID)'),
     )
-    vminterface = django_filters.ModelMultipleChoiceFilter(
+    vminterface = NonUniqueModelMultipleChoiceFilter(
         field_name='vminterface__name',
         queryset=VMInterface.objects.all(),
         to_field_name='name',
@@ -460,7 +465,7 @@ class L2VPNTerminationFilterSet(NetBoxModelFilterSet):
         queryset=VMInterface.objects.all(),
         label=_('VM Interface (ID)'),
     )
-    vlan = django_filters.ModelMultipleChoiceFilter(
+    vlan = NonUniqueModelMultipleChoiceFilter(
         field_name='vlan__name',
         queryset=VLAN.objects.all(),
         to_field_name='name',

+ 53 - 1
netbox/vpn/tests/test_filtersets.py

@@ -4,7 +4,7 @@ from dcim.choices import InterfaceTypeChoices
 from dcim.models import Device, Interface, Site
 from ipam.models import VLAN, IPAddress, RouteTarget
 from utilities.testing import ChangeLoggedFilterSetTestMixin, create_test_device, create_test_virtualmachine
-from virtualization.models import VirtualMachine, VMInterface
+from virtualization.models import Cluster, VirtualMachine, VMInterface
 from vpn.choices import *
 from vpn.filtersets import *
 from vpn.models import *
@@ -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]}
@@ -928,6 +954,14 @@ class L2VPNTerminationTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'vlan': ['VLAN 1', 'VLAN 2']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_vlan_shared_name(self):
+        # A VLAN name shared by several groups should resolve to a single filter value
+        vlan = VLAN.objects.first()
+        VLAN.objects.create(vid=vlan.vid, name=vlan.name)
+        filterset = self.filterset({'vlan': [vlan.name]}, self.queryset)
+        self.assertTrue(filterset.is_valid())
+        self.assertEqual(len(filterset.form.cleaned_data['vlan']), 1)
+
     def test_site(self):
         site = Site.objects.all().first()
         params = {'site_id': [site.pk]}
@@ -942,9 +976,27 @@ class L2VPNTerminationTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'device': ['Device 1']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
 
+    def test_device_shared_name(self):
+        # A device name shared by several sites should resolve to a single filter value
+        device = Device.objects.first()
+        site = Site.objects.create(name='Site 99', slug='site-99')
+        Device.objects.create(site=site, name=device.name, device_type=device.device_type, role=device.role)
+        filterset = self.filterset({'device': [device.name]}, self.queryset)
+        self.assertTrue(filterset.is_valid())
+        self.assertEqual(len(filterset.form.cleaned_data['device']), 1)
+
     def test_virtual_machine(self):
         virtual_machine = VirtualMachine.objects.all().first()
         params = {'virtual_machine_id': [virtual_machine.pk]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
         params = {'virtual_machine': ['Virtual Machine 1']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
+
+    def test_virtual_machine_shared_name(self):
+        # A virtual machine name shared by several clusters should resolve to a single filter value
+        vm = VirtualMachine.objects.first()
+        cluster = Cluster.objects.create(name='Cluster 99', type=vm.cluster.type)
+        VirtualMachine.objects.create(cluster=cluster, name=vm.name)
+        filterset = self.filterset({'virtual_machine': [vm.name]}, self.queryset)
+        self.assertTrue(filterset.is_valid())
+        self.assertEqual(len(filterset.form.cleaned_data['virtual_machine']), 1)