From 126a5e5e4e7a74c34f63872bc5500dc5dccadbd6 Mon Sep 17 00:00:00 2001 From: Joey Wilhelm Date: Thu, 21 Sep 2017 10:33:34 -0700 Subject: [PATCH 1/2] Fix order_naturally with unbalanced names and use RawSQL instead of extra --- netbox/dcim/models.py | 48 +++++++++++++++-------- netbox/dcim/tests/test_models.py | 66 ++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 16 deletions(-) diff --git a/netbox/dcim/models.py b/netbox/dcim/models.py index a189d9f29..52527cc19 100644 --- a/netbox/dcim/models.py +++ b/netbox/dcim/models.py @@ -13,6 +13,7 @@ from django.core.exceptions import ValidationError from django.core.validators import MaxValueValidator, MinValueValidator from django.db import models from django.db.models import Count, Q, ObjectDoesNotExist +from django.db.models.expressions import RawSQL from django.urls import reverse from django.utils.encoding import python_2_unicode_compatible @@ -642,15 +643,16 @@ class InterfaceQuerySet(models.QuerySet): 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}:{channel}.{vc} + {type}{slot}/{subslot}/{position}/{subposition}:{channel}.{vc} - Components absent from the interface name are ignored. For example, an interface named GigabitEthernet0/1 would - be parsed as follows: + Components absent from the interface name are ignored. For example, an interface named GigabitEthernet1/2/3 + would be parsed as follows: name = 'GigabitEthernet' - slot = None - subslot = 0 - position = 1 + slot = 1 + subslot = 2 + position = 3 + subposition = 0 channel = None vc = 0 @@ -659,17 +661,31 @@ class InterfaceQuerySet(models.QuerySet): """ sql_col = '{}.name'.format(self.model._meta.db_table) ordering = { - IFACE_ORDERING_POSITION: ('_slot', '_subslot', '_position', '_channel', '_vc', '_type', 'name'), - IFACE_ORDERING_NAME: ('_type', '_slot', '_subslot', '_position', '_channel', '_vc', 'name'), + IFACE_ORDERING_POSITION: ('_slot', '_subslot', '_position', '_subposition', '_channel', '_vc', '_type', 'name'), + IFACE_ORDERING_NAME: ('_type', '_slot', '_subslot', '_position', '_subposition', '_channel', '_vc', 'name'), }[method] - return self.extra(select={ - '_type': "SUBSTRING({} FROM '^([^0-9]+)')".format(sql_col), - '_slot': "CAST(SUBSTRING({} FROM '([0-9]+)\/[0-9]+\/[0-9]+(:[0-9]+)?(\.[0-9]+)?$') AS integer)".format(sql_col), - '_subslot': "CAST(SUBSTRING({} FROM '([0-9]+)\/[0-9]+(:[0-9]+)?(\.[0-9]+)?$') AS integer)".format(sql_col), - '_position': "CAST(SUBSTRING({} FROM '([0-9]+)(:[0-9]+)?(\.[0-9]+)?$') AS integer)".format(sql_col), - '_channel': "COALESCE(CAST(SUBSTRING({} FROM ':([0-9]+)(\.[0-9]+)?$') AS integer), 0)".format(sql_col), - '_vc': "COALESCE(CAST(SUBSTRING({} FROM '\.([0-9]+)$') AS integer), 0)".format(sql_col), - }).order_by(*ordering) + + fields = { + '_type': RawSQL(r"SUBSTRING({} FROM '^([^0-9]+)')".format(sql_col), []), + '_slot': RawSQL(r"CAST(SUBSTRING({} FROM '^(?:[^0-9]+)([0-9]+)') AS integer)".format(sql_col), []), + '_subslot': RawSQL( + r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)(?:[0-9]+)\/([0-9]+)') AS integer), 0)".format( + sql_col), [] + ), + '_position': RawSQL( + r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)(?:[0-9]+\/){{2}}([0-9]+)') AS integer), 0)".format( + sql_col), [] + ), + '_subposition': RawSQL( + r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)(?:[0-9]+\/){{3}}([0-9]+)') AS integer), 0)".format( + sql_col), [] + ), + '_channel': RawSQL( + r"COALESCE(CAST(SUBSTRING({} FROM ':([0-9]+)(\.[0-9]+)?$') AS integer), 0)".format(sql_col), []), + '_vc': RawSQL(r"COALESCE(CAST(SUBSTRING({} FROM '\.([0-9]+)$') AS integer), 0)".format(sql_col), []), + } + + return self.annotate(**fields).order_by(*ordering) def connectable(self): """ diff --git a/netbox/dcim/tests/test_models.py b/netbox/dcim/tests/test_models.py index 340c58092..9acbe3901 100644 --- a/netbox/dcim/tests/test_models.py +++ b/netbox/dcim/tests/test_models.py @@ -98,3 +98,69 @@ 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_device_port_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' + ) + + self.assertEqual( + list(Interface.objects.all().order_naturally()), + [interface1, interface5, interface4, interface3, interface2] + ) From 89194c067b89287eb5b912a01772f1aedb308ca0 Mon Sep 17 00:00:00 2001 From: Joey Wilhelm Date: Thu, 21 Sep 2017 17:20:21 -0700 Subject: [PATCH 2/2] Another test case to ensure subinterface ordering --- netbox/dcim/tests/test_models.py | 41 +++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/netbox/dcim/tests/test_models.py b/netbox/dcim/tests/test_models.py index 9acbe3901..00a4de28c 100644 --- a/netbox/dcim/tests/test_models.py +++ b/netbox/dcim/tests/test_models.py @@ -129,7 +129,7 @@ class InterfaceTestCase(TestCase): slug='switch', ) - def test_device_port_order_natural(self): + def test_interface_order_natural(self): device1 = Device.objects.create( name='TestSwitch1', device_type=self.device_type, @@ -164,3 +164,42 @@ class InterfaceTestCase(TestCase): list(Interface.objects.all().order_naturally()), [interface1, interface5, interface4, interface3, interface2] ) + + 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()), + [interface6, interface4, interface3, interface5, interface2, interface1] + )