From e5f12109c5b0cf235fd636e2df0b803905144e6f Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 5 Nov 2018 11:51:38 -0500 Subject: [PATCH] Closes #2165: Re-implemented natural ordering for interfaces --- netbox/dcim/filters.py | 5 +- netbox/dcim/forms.py | 12 +- netbox/dcim/models.py | 6 +- netbox/dcim/querysets.py | 79 +++++------ netbox/dcim/tests/test_models.py | 109 -------------- netbox/dcim/tests/test_natural_ordering.py | 157 +++++++++++++++++++++ netbox/dcim/views.py | 14 +- 7 files changed, 210 insertions(+), 172 deletions(-) create mode 100644 netbox/dcim/tests/test_natural_ordering.py diff --git a/netbox/dcim/filters.py b/netbox/dcim/filters.py index 710ebb47d..7ad91979d 100644 --- a/netbox/dcim/filters.py +++ b/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).order_naturally(ordering) + return queryset.filter(pk__in=vc_interface_ids) except Device.DoesNotExist: return queryset.none() diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index f230604f2..52f04ccff 100644 --- a/netbox/dcim/forms.py +++ b/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.order_naturally(method=interface_ordering).filter( - device__in=[device, device.get_vc_master()], form_factor=IFACE_FF_LAG + self.fields['lag'].queryset = Interface.objects.filter( + device__in=[device, device.get_vc_master()], + form_factor=IFACE_FF_LAG ) else: self.fields['lag'].choices = [] diff --git a/netbox/dcim/models.py b/netbox/dcim/models.py index 6f0bdace7..95f17fb37 100644 --- a/netbox/dcim/models.py +++ b/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: diff --git a/netbox/dcim/querysets.py b/netbox/dcim/querysets.py index 6b3c95e9c..84d309719 100644 --- a/netbox/dcim/querysets.py +++ b/netbox/dcim/querysets.py @@ -1,54 +1,58 @@ -from django.contrib.contenttypes.models import ContentType -from django.db.models import Q, QuerySet +from django.db.models import Manager, 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 - IFACE_ORDERING_CHOICES (typically indicated by a parent Device's DeviceType). + 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) - 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 - would be parsed as follows: + def get_queryset(self): + """ + 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 - channel = None + subposition = 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 prescribed fields. + The original `name` field is considered in its entirety to serve as a fallback in the event interfaces do not + 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]+)')" - ID_RE = r"CAST(SUBSTRING({} FROM '^(?:[^0-9]+)(\d{{1,9}})$') AS integer)" - SLOT_RE = r"CAST(SUBSTRING({} FROM '^(?:[^0-9]+)?(\d{{1,9}})\/') AS integer)" - 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)" + sql_col = '{}.name'.format(self.model._meta.db_table) + ordering = [ + '_slot', '_subslot', '_position', '_subposition', '_type', '_id', '_channel', '_vc', 'name', + ] 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) - - 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) + return InterfaceQuerySet(self.model, using=self._db).annotate(**fields).order_by(*ordering) diff --git a/netbox/dcim/tests/test_models.py b/netbox/dcim/tests/test_models.py index ec59b59b0..012c7d121 100644 --- a/netbox/dcim/tests/test_models.py +++ b/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] - ) diff --git a/netbox/dcim/tests/test_natural_ordering.py b/netbox/dcim/tests/test_natural_ordering.py new file mode 100644 index 000000000..d4dca43d7 --- /dev/null +++ b/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) diff --git a/netbox/dcim/views.py b/netbox/dcim/views.py index 0367be3c9..31787ec6d 100644 --- a/netbox/dcim/views.py +++ b/netbox/dcim/views.py @@ -551,9 +551,7 @@ class DeviceTypeView(View): orderable=False ) interface_table = tables.InterfaceTemplateTable( - list(InterfaceTemplate.objects.order_naturally( - devicetype.interface_ordering - ).filter(device_type=devicetype)), + list(InterfaceTemplate.objects.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( - device.device_type.interface_ordering - ).select_related( + interfaces = device.vc_interfaces.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( - device.device_type.interface_ordering - ).connectable().select_related( + interfaces = device.vc_interfaces.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