Closes #2165: Re-implemented natural ordering for interfaces

netbox/dcim/filters.py

@@ -764,10 +764,9 @@ class InterfaceFilter(django_filters.FilterSet):
 
     def filter_device(self, queryset, name, value):
         try:
-            device = Device.objects.select_related('device_type').get(**{name: value})
+            device = Device.objects.get(**{name: value})
             vc_interface_ids = [i['id'] for i in device.vc_interfaces.values('id')]
-            ordering = device.device_type.interface_ordering
+            return queryset.filter(pk__in=vc_interface_ids)
-            return queryset.filter(pk__in=vc_interface_ids).order_naturally(ordering)
         except Device.DoesNotExist:
             return queryset.none()
 

netbox/dcim/forms.py

@@ -1472,12 +1472,12 @@ class InterfaceForm(BootstrapMixin, forms.ModelForm):
         # Limit LAG choices to interfaces belonging to this device (or VC master)
         if self.is_bound:
             device = Device.objects.get(pk=self.data['device'])
-            self.fields['lag'].queryset = Interface.objects.order_naturally().filter(
+            self.fields['lag'].queryset = Interface.objects.filter(
                 device__in=[device, device.get_vc_master()], form_factor=IFACE_FF_LAG
             )
         else:
             device = self.instance.device
-            self.fields['lag'].queryset = Interface.objects.order_naturally().filter(
+            self.fields['lag'].queryset = Interface.objects.filter(
                 device__in=[self.instance.device, self.instance.device.get_vc_master()], form_factor=IFACE_FF_LAG
             )
 
@@ -1608,7 +1608,7 @@ class InterfaceCreateForm(ComponentForm, forms.Form):
 
         # Limit LAG choices to interfaces belonging to this device (or its VC master)
         if self.parent is not None:
-            self.fields['lag'].queryset = Interface.objects.order_naturally().filter(
+            self.fields['lag'].queryset = Interface.objects.filter(
                 device__in=[self.parent, self.parent.get_vc_master()], form_factor=IFACE_FF_LAG
             )
         else:
@@ -1634,9 +1634,9 @@ class InterfaceBulkEditForm(BootstrapMixin, AddRemoveTagsForm, BulkEditForm):
         # Limit LAG choices to interfaces which belong to the parent device (or VC master)
         device = self.parent_obj
         if device is not None:
-            interface_ordering = device.device_type.interface_ordering
+            self.fields['lag'].queryset = Interface.objects.filter(
-            self.fields['lag'].queryset = Interface.objects.order_naturally(method=interface_ordering).filter(
+                device__in=[device, device.get_vc_master()],
-                device__in=[device, device.get_vc_master()], form_factor=IFACE_FF_LAG
+                form_factor=IFACE_FF_LAG
             )
         else:
             self.fields['lag'].choices = []

netbox/dcim/models.py

@@ -22,7 +22,7 @@ from utilities.models import ChangeLoggedModel
 from utilities.utils import serialize_object, to_meters
 from .constants import *
 from .fields import ASNField, MACAddressField
-from .querysets import InterfaceQuerySet
+from .querysets import InterfaceManager
 
 
 class ComponentTemplateModel(models.Model):
@@ -1063,7 +1063,7 @@ class InterfaceTemplate(ComponentTemplateModel):
         verbose_name='Management only'
     )
 
-    objects = InterfaceQuerySet.as_manager()
+    objects = InterfaceManager
 
     class Meta:
         ordering = ['device_type', 'name']
@@ -1954,7 +1954,7 @@ class Interface(CableTermination, ComponentModel):
         verbose_name='Tagged VLANs'
     )
 
-    objects = InterfaceQuerySet.as_manager()
+    objects = InterfaceManager()
     tags = TaggableManager()
 
     class Meta:

netbox/dcim/querysets.py

@@ -1,54 +1,58 @@
-from django.contrib.contenttypes.models import ContentType
+from django.db.models import Manager, QuerySet
-from django.db.models import Q, QuerySet
 from django.db.models.expressions import RawSQL
 
-from .constants import IFACE_ORDERING_NAME, IFACE_ORDERING_POSITION, NONCONNECTABLE_IFACE_TYPES
+from .constants import NONCONNECTABLE_IFACE_TYPES
+
+TYPE_RE = r"SUBSTRING({} FROM '^([^0-9\.:]+)')"
+SLOT_RE = r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)?(\d{{1,9}})/') AS integer), NULL)"
+SUBSLOT_RE = r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9\.:]+)?\d{{1,9}}/(\d{{1,9}})') AS integer), NULL)"
+POSITION_RE = r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)?(?:\d{{1,9}}/){{2}}(\d{{1,9}})') AS integer), NULL)"
+SUBPOSITION_RE = r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)?(?:\d{{1,9}}/){{3}}(\d{{1,9}})') AS integer), NULL)"
+ID_RE = r"CAST(SUBSTRING({} FROM '^(?:[^0-9\.:]+)?(\d{{1,9}})([^/]|$)') AS integer)"
+CHANNEL_RE = r"COALESCE(CAST(SUBSTRING({} FROM '^.*:(\d{{1,9}})(\.\d{{1,9}})?$') AS integer), 0)"
+VC_RE = r"COALESCE(CAST(SUBSTRING({} FROM '^.*\.(\d{{1,9}})$') AS integer), 0)"
 
 
 class InterfaceQuerySet(QuerySet):
 
-    def order_naturally(self, method=IFACE_ORDERING_POSITION):
+    def connectable(self):
         """
-        Naturally order interfaces by their type and numeric position. The sort method must be one of the defined
+        Return only physical interfaces which are capable of being connected to other interfaces (i.e. not virtual or
-        IFACE_ORDERING_CHOICES (typically indicated by a parent Device's DeviceType).
+        wireless).
+        """
+        return self.exclude(form_factor__in=NONCONNECTABLE_IFACE_TYPES)
 
-        To order interfaces naturally, the `name` field is split into six distinct components: leading text (type),
-        slot, subslot, position, channel, and virtual circuit:
 
-            {type}{slot}/{subslot}/{position}/{subposition}:{channel}.{vc}
+class InterfaceManager(Manager):
 
-        Components absent from the interface name are ignored. For example, an interface named GigabitEthernet1/2/3
+    def get_queryset(self):
-        would be parsed as follows:
+        """
+        Naturally order interfaces by their type and numeric position. To order interfaces naturally, the `name` field
+        is split into eight distinct components: leading text (type), slot, subslot, position, subposition, ID, channel,
+        and virtual circuit:
 
-            name = 'GigabitEthernet'
+            {type}{slot or ID}/{subslot}/{position}/{subposition}:{channel}.{vc}
+
+        Components absent from the interface name are coalesced to zero or null. For example, an interface named
+        GigabitEthernet1/2/3 would be parsed as follows:
+
+            type = 'GigabitEthernet'
             slot =  1
             subslot = 2
             position = 3
-            subposition = 0
+            subposition = None
-            channel = None
+            id = None
+            channel = 0
             vc = 0
 
-        The original `name` field is taken as a whole to serve as a fallback in the event interfaces do not match any of
+        The original `name` field is considered in its entirety to serve as a fallback in the event interfaces do not
-        the prescribed fields.
+        match any of the prescribed fields.
         """
-        sql_col = '{}.name'.format(self.model._meta.db_table)
-        ordering = {
-            IFACE_ORDERING_POSITION: (
-                '_slot', '_subslot', '_position', '_subposition', '_channel', '_type', '_vc', '_id', 'name',
-            ),
-            IFACE_ORDERING_NAME: (
-                '_type', '_slot', '_subslot', '_position', '_subposition', '_channel', '_vc', '_id', 'name',
-            ),
-        }[method]
 
-        TYPE_RE = r"SUBSTRING({} FROM '^([^0-9]+)')"
+        sql_col = '{}.name'.format(self.model._meta.db_table)
-        ID_RE = r"CAST(SUBSTRING({} FROM '^(?:[^0-9]+)(\d{{1,9}})$') AS integer)"
+        ordering = [
-        SLOT_RE = r"CAST(SUBSTRING({} FROM '^(?:[^0-9]+)?(\d{{1,9}})\/') AS integer)"
+            '_slot', '_subslot', '_position', '_subposition', '_type', '_id', '_channel', '_vc', 'name',
-        SUBSLOT_RE = r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)?(?:\d{{1,9}}\/)(\d{{1,9}})') AS integer), 0)"
+        ]
-        POSITION_RE = r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)?(?:\d{{1,9}}\/){{2}}(\d{{1,9}})') AS integer), 0)"
-        SUBPOSITION_RE = r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)?(?:\d{{1,9}}\/){{3}}(\d{{1,9}})') AS integer), 0)"
-        CHANNEL_RE = r"COALESCE(CAST(SUBSTRING({} FROM ':(\d{{1,9}})(\.\d{{1,9}})?$') AS integer), 0)"
-        VC_RE = r"COALESCE(CAST(SUBSTRING({} FROM '\.(\d{{1,9}})$') AS integer), 0)"
 
         fields = {
             '_type': RawSQL(TYPE_RE.format(sql_col), []),
@@ -61,11 +65,4 @@ class InterfaceQuerySet(QuerySet):
             '_vc': RawSQL(VC_RE.format(sql_col), []),
         }
 
-        return self.annotate(**fields).order_by(*ordering)
+        return InterfaceQuerySet(self.model, using=self._db).annotate(**fields).order_by(*ordering)
-
-    def connectable(self):
-        """
-        Return only physical interfaces which are capable of being connected to other interfaces (i.e. not virtual or
-        wireless).
-        """
-        return self.exclude(form_factor__in=NONCONNECTABLE_IFACE_TYPES)

netbox/dcim/tests/test_models.py

@@ -149,112 +149,3 @@ class RackTestCase(TestCase):
             face=None,
         )
         self.assertTrue(pdu)
-
-
-class InterfaceTestCase(TestCase):
-
-    def setUp(self):
-
-        self.site = Site.objects.create(
-            name='TestSite1',
-            slug='my-test-site'
-        )
-        self.rack = Rack.objects.create(
-            name='TestRack1',
-            facility_id='A101',
-            site=self.site,
-            u_height=42
-        )
-        self.manufacturer = Manufacturer.objects.create(
-            name='Acme',
-            slug='acme'
-        )
-
-        self.device_type = DeviceType.objects.create(
-            manufacturer=self.manufacturer,
-            model='FrameForwarder 2048',
-            slug='ff2048'
-        )
-        self.role = DeviceRole.objects.create(
-            name='Switch',
-            slug='switch',
-        )
-
-    def test_interface_order_natural(self):
-        device1 = Device.objects.create(
-            name='TestSwitch1',
-            device_type=self.device_type,
-            device_role=self.role,
-            site=self.site,
-            rack=self.rack,
-            position=10,
-            face=RACK_FACE_REAR,
-        )
-        interface1 = Interface.objects.create(
-            device=device1,
-            name='Ethernet1/3/1'
-        )
-        interface2 = Interface.objects.create(
-            device=device1,
-            name='Ethernet1/5/1'
-        )
-        interface3 = Interface.objects.create(
-            device=device1,
-            name='Ethernet1/4'
-        )
-        interface4 = Interface.objects.create(
-            device=device1,
-            name='Ethernet1/3/2/4'
-        )
-        interface5 = Interface.objects.create(
-            device=device1,
-            name='Ethernet1/3/2/1'
-        )
-        interface6 = Interface.objects.create(
-            device=device1,
-            name='Loopback1'
-        )
-
-        self.assertEqual(
-            list(Interface.objects.all().order_naturally()),
-            [interface1, interface5, interface4, interface3, interface2, interface6]
-        )
-
-    def test_interface_order_natural_subinterfaces(self):
-        device1 = Device.objects.create(
-            name='TestSwitch1',
-            device_type=self.device_type,
-            device_role=self.role,
-            site=self.site,
-            rack=self.rack,
-            position=10,
-            face=RACK_FACE_REAR,
-        )
-        interface1 = Interface.objects.create(
-            device=device1,
-            name='GigabitEthernet0/0/3'
-        )
-        interface2 = Interface.objects.create(
-            device=device1,
-            name='GigabitEthernet0/0/2.2'
-        )
-        interface3 = Interface.objects.create(
-            device=device1,
-            name='GigabitEthernet0/0/0.120'
-        )
-        interface4 = Interface.objects.create(
-            device=device1,
-            name='GigabitEthernet0/0/0'
-        )
-        interface5 = Interface.objects.create(
-            device=device1,
-            name='GigabitEthernet0/0/1.117'
-        )
-        interface6 = Interface.objects.create(
-            device=device1,
-            name='GigabitEthernet0'
-        )
-        self.assertEqual(
-            list(Interface.objects.all().order_naturally()),
-            [interface4, interface3, interface5, interface2, interface1, interface6]
-        )

netbox/dcim/tests/test_natural_ordering.py

@@ -0,0 +1,157 @@
+from django.test import TestCase
+
+from dcim.models import Device, DeviceRole, DeviceType, Interface, Manufacturer, Site
+
+
+class NaturalOrderingTestCase(TestCase):
+
+    def setUp(self):
+
+        site = Site.objects.create(name='Test Site 1', slug='test-site-1')
+        manufacturer = Manufacturer.objects.create(name='Test Manufacturer 1', slug='test-manufacturer-1')
+        devicetype = DeviceType.objects.create(
+            manufacturer=manufacturer, model='Test Device Type 1', slug='test-device-type-1'
+        )
+        devicerole = DeviceRole.objects.create(
+            name='Test Device Role 1', slug='test-device-role-1', color='ff0000'
+        )
+        self.device = Device.objects.create(
+            device_type=devicetype, device_role=devicerole, name='Test Device 1', site=site
+        )
+
+    def _compare_names(self, queryset, names):
+
+        for i, obj in enumerate(queryset):
+            self.assertEqual(obj.name, names[i])
+
+    def test_interface_ordering_numeric(self):
+
+        INTERFACES = (
+            '0',
+            '0.1',
+            '0.2',
+            '0.10',
+            '0.100',
+            '0:1',
+            '0:1.1',
+            '0:1.2',
+            '0:1.10',
+            '0:2',
+            '0:2.1',
+            '0:2.2',
+            '0:2.10',
+            '1',
+            '1.1',
+            '1.2',
+            '1.10',
+            '1.100',
+            '1:1',
+            '1:1.1',
+            '1:1.2',
+            '1:1.10',
+            '1:2',
+            '1:2.1',
+            '1:2.2',
+            '1:2.10',
+        )
+
+        for name in INTERFACES:
+            iface = Interface(device=self.device, name=name)
+            iface.save()
+
+        self._compare_names(Interface.objects.filter(device=self.device), INTERFACES)
+
+    def test_interface_ordering_linux(self):
+
+        INTERFACES = (
+            'eth0',
+            'eth0.1',
+            'eth0.2',
+            'eth0.10',
+            'eth0.100',
+            'eth1',
+            'eth1.1',
+            'eth1.2',
+            'eth1.100',
+            'lo0',
+        )
+
+        for name in INTERFACES:
+            iface = Interface(device=self.device, name=name)
+            iface.save()
+
+        self._compare_names(Interface.objects.filter(device=self.device), INTERFACES)
+
+    def test_interface_ordering_junos(self):
+
+        INTERFACES = (
+            'xe-0/0/0',
+            'xe-0/0/1',
+            'xe-0/0/2',
+            'xe-0/0/3',
+            'xe-0/1/0',
+            'xe-0/1/1',
+            'xe-0/1/2',
+            'xe-0/1/3',
+            'xe-1/0/0',
+            'xe-1/0/1',
+            'xe-1/0/2',
+            'xe-1/0/3',
+            'xe-1/1/0',
+            'xe-1/1/1',
+            'xe-1/1/2',
+            'xe-1/1/3',
+            'xe-2/0/0.1',
+            'xe-2/0/0.2',
+            'xe-2/0/0.10',
+            'xe-2/0/0.11',
+            'xe-2/0/0.100',
+            'xe-3/0/0:1',
+            'xe-3/0/0:2',
+            'xe-3/0/0:10',
+            'xe-3/0/0:11',
+            'xe-3/0/0:100',
+            'xe-10/1/0',
+            'xe-10/1/1',
+            'xe-10/1/2',
+            'xe-10/1/3',
+            'ae1',
+            'ae2',
+            'ae10.1',
+            'ae10.10',
+            'irb.1',
+            'irb.2',
+            'irb.10',
+            'irb.100',
+            'lo0',
+        )
+
+        for name in INTERFACES:
+            iface = Interface(device=self.device, name=name)
+            iface.save()
+
+        self._compare_names(Interface.objects.filter(device=self.device), INTERFACES)
+
+    def test_interface_ordering_ios(self):
+
+        INTERFACES = (
+            'GigabitEthernet0/1',
+            'GigabitEthernet0/2',
+            'GigabitEthernet0/10',
+            'TenGigabitEthernet0/20',
+            'TenGigabitEthernet0/21',
+            'GigabitEthernet1/1',
+            'GigabitEthernet1/2',
+            'GigabitEthernet1/10',
+            'TenGigabitEthernet1/20',
+            'TenGigabitEthernet1/21',
+            'FastEthernet1',
+            'FastEthernet2',
+            'FastEthernet10',
+        )
+
+        for name in INTERFACES:
+            iface = Interface(device=self.device, name=name)
+            iface.save()
+
+        self._compare_names(Interface.objects.filter(device=self.device), INTERFACES)

netbox/dcim/views.py

@@ -551,9 +551,7 @@ class DeviceTypeView(View):
             orderable=False
         )
         interface_table = tables.InterfaceTemplateTable(
-            list(InterfaceTemplate.objects.order_naturally(
+            list(InterfaceTemplate.objects.filter(device_type=devicetype)),
-                devicetype.interface_ordering
-            ).filter(device_type=devicetype)),
             orderable=False
         )
         front_port_table = tables.FrontPortTemplateTable(
@@ -900,9 +898,7 @@ class DeviceView(View):
         poweroutlets = device.poweroutlets.select_related('connected_endpoint__device', 'cable')
 
         # Interfaces
-        interfaces = device.vc_interfaces.order_naturally(
+        interfaces = device.vc_interfaces.select_related(
-            device.device_type.interface_ordering
-        ).select_related(
             'lag', '_connected_interface__device', '_connected_circuittermination__circuit', 'cable'
         ).prefetch_related(
             'cable__termination_a', 'cable__termination_b', 'ip_addresses'
@@ -995,9 +991,7 @@ class DeviceLLDPNeighborsView(PermissionRequiredMixin, View):
     def get(self, request, pk):
 
         device = get_object_or_404(Device, pk=pk)
-        interfaces = device.vc_interfaces.order_naturally(
+        interfaces = device.vc_interfaces.connectable().select_related(
-            device.device_type.interface_ordering
-        ).connectable().select_related(
             '_connected_interface__device'
         )
 
@@ -1341,7 +1335,7 @@ class InterfaceBulkEditView(PermissionRequiredMixin, BulkEditView):
 
 class InterfaceBulkRenameView(PermissionRequiredMixin, BulkRenameView):
     permission_required = 'dcim.change_interface'
-    queryset = Interface.objects.order_naturally()
+    queryset = Interface.objects.all()
     form = forms.InterfaceBulkRenameForm