Sort interface templates of device type

Use the new hook to re-sort the interface template list. It is only done
when required to preserve the original order and prevent any unneeded
computation. But even when sorting, it tries its best to preserve as the
original order as much as possible.

The sorting is still done before any validation of the individual
elements, so needs to be able to work with invalid/incomplete/malformed
data. The only guarantee is that we get a list (of something).

It builds a simple dependency graph, uses Kahn's algorithm to create a
topological sorting of it, and applies that to the interface template
list.

Author: m-hau

netbox/dcim/tests/test_views.py

@@ -1063,11 +1063,32 @@ def test_import_interfacebridge(self):
 slug: test-4000
 u_height: 1
 interfaces:
-  - name: Bridge
+  - name: Standalone 1
+    type: 1000base-t
+  - name: Bridge Interface 2
     type: 1000base-t
+    bridge: Bridge
   - name: Bridge Interface 1
     type: 1000base-t
     bridge: Bridge
+  - name: Standalone 2
+    type: 1000base-t
+  - name: Sub-Bridge Interface 2
+    type: 1000base-t
+    bridge: Sub-Bridge
+  - name: Bridge
+    type: 1000base-t
+  - name: Sub-Bridge
+    type: 1000base-t
+    bridge: Bridge
+  - name: Bridge Interface 3
+    type: 1000base-t
+    bridge: Bridge
+  - name: Sub-Bridge Interface 1
+    type: 1000base-t
+    bridge: Sub-Bridge
+  - name: Standalone 3
+    type: 1000base-t
 """
 
         # Add all required permissions to the test user
@@ -1096,13 +1117,74 @@ def test_import_interfacebridge(self):
         self.assertContains(response, "Imported 1 device types")
 
         device_type = DeviceType.objects.get(model='TEST-4000')
-        self.assertEqual(device_type.interfacetemplates.count(), 2)
+        self.assertEqual(device_type.interfacetemplates.count(), 10)
 
         interfaces = InterfaceTemplate.objects.all().order_by('id')
-        self.assertEqual(interfaces[0].name, 'Bridge')
+        self.assertEqual(interfaces[0].name, 'Standalone 1')
         self.assertIsNone(interfaces[0].bridge)
-        self.assertEqual(interfaces[1].name, 'Bridge Interface 1')
-        self.assertEqual(interfaces[1].bridge.name, "Bridge")
+        self.assertEqual(interfaces[1].name, 'Standalone 2')
+        self.assertIsNone(interfaces[1].bridge)
+        self.assertEqual(interfaces[2].name, 'Bridge')
+        self.assertIsNone(interfaces[2].bridge)
+        self.assertEqual(interfaces[3].name, 'Standalone 3')
+        self.assertIsNone(interfaces[3].bridge)
+        self.assertEqual(interfaces[4].name, 'Bridge Interface 2')
+        self.assertEqual(interfaces[4].bridge.name, "Bridge")
+        self.assertEqual(interfaces[5].name, 'Bridge Interface 1')
+        self.assertEqual(interfaces[5].bridge.name, "Bridge")
+        self.assertEqual(interfaces[6].name, 'Sub-Bridge')
+        self.assertEqual(interfaces[6].bridge.name, "Bridge")
+        self.assertEqual(interfaces[7].name, 'Bridge Interface 3')
+        self.assertEqual(interfaces[7].bridge.name, "Bridge")
+        self.assertEqual(interfaces[8].name, 'Sub-Bridge Interface 2')
+        self.assertEqual(interfaces[8].bridge.name, "Sub-Bridge")
+        self.assertEqual(interfaces[9].name, 'Sub-Bridge Interface 1')
+        self.assertEqual(interfaces[9].bridge.name, "Sub-Bridge")
+
+    @override_settings(EXEMPT_VIEW_PERMISSIONS=['*'])
+    def test_import_interfacebridge_cycle(self):
+        IMPORT_DATA = """
+manufacturer: Manufacturer 1
+model: TEST-5000
+slug: test-5000
+u_height: 1
+interfaces:
+  - name: Interface 1
+    type: 1000base-t
+    bridge: Interface 2
+  - name: Interface 2
+    type: 1000base-t
+    bridge: Interface 3
+  - name: Interface 3
+    type: 1000base-t
+    bridge: Interface 1
+    """
+
+        # Add all required permissions to the test user
+        self.add_permissions(
+            'dcim.view_devicetype',
+            'dcim.add_devicetype',
+            'dcim.add_consoleporttemplate',
+            'dcim.add_consoleserverporttemplate',
+            'dcim.add_powerporttemplate',
+            'dcim.add_poweroutlettemplate',
+            'dcim.add_interfacetemplate',
+            'dcim.add_frontporttemplate',
+            'dcim.add_rearporttemplate',
+            'dcim.add_modulebaytemplate',
+            'dcim.add_devicebaytemplate',
+            'dcim.add_inventoryitemtemplate',
+        )
+
+        form_data = {
+            'data': IMPORT_DATA,
+            'format': 'yaml'
+        }
+
+        response = self.client.post(reverse('dcim:devicetype_bulk_import'), data=form_data, follow=True)
+        self.assertHttpStatus(response, 200)
+        self.assertContains(response, "interfaces: Dependency cycle detected in subset "
+                                      "[Interface 1, Interface 2, Interface 3]")
 
     def test_export_objects(self):
         url = reverse('dcim:devicetype_list')

netbox/dcim/views.py

@@ -1,9 +1,11 @@
+from collections import deque
+
 from django.contrib import messages
 from django.contrib.contenttypes.models import ContentType
 from django.core.paginator import EmptyPage, PageNotAnInteger
 from django.db import router, transaction
 from django.db.models import Prefetch
-from django.forms import ModelMultipleChoiceField, MultipleHiddenInput, modelformset_factory
+from django.forms import ModelMultipleChoiceField, MultipleHiddenInput, ValidationError, modelformset_factory
 from django.shortcuts import get_object_or_404, redirect, render
 from django.urls import reverse
 from django.utils.html import escape
@@ -1283,6 +1285,79 @@ class DeviceTypeImportView(generic.BulkImportView):
         'inventory-items': forms.InventoryItemTemplateImportForm,
     }
 
+    def _sort_interfaces(self, interfaces):
+        """Sort the given enumerated interface list to satisfy any dependencies."""
+        if not interfaces:
+            return
+
+        # build the dependency graph
+        all_interface_names = set()
+        sorting_required = False
+        required_by = dict()  # ifname to list of dependant subinterfaces  # TODO replace list with ordered set
+        requires = dict()  # ifname to set of depended-on superinterfaces
+        for idx, interface in interfaces:
+            if not isinstance(interface, dict):  # TODO isinstance(MutableMapping)?
+                # not a dict, will fail validation anyway
+                # but still sort if required, to prevent false "bridge is invalid" errors on any other valid interfaces
+                continue
+            if not interface.get('name'):
+                # no interface name, will fail validation anyway
+                continue
+
+            ifname = interface['name']
+            all_interface_names.add(ifname)
+
+            requirements = set()
+            if bridge := interface.get('bridge'):
+                requirements.add(bridge)
+
+            requires[ifname] = requirements
+            for requirement in list(requirements):
+                required_by.setdefault(requirement, list()).append(ifname)
+
+            # if we haven't seen all requirements yet, sorting is needed
+            sorting_required |= not requirements.issubset(all_interface_names)
+
+        if not sorting_required:
+            return
+
+        # use Kahn's algorithm to build a topological sorting
+        workqueue = deque(ifname for ifname, requirements in requires.items() if not requirements)
+        ifname_ordering = list()
+        while workqueue:
+            ifname = workqueue.popleft()
+            ifname_ordering.append(ifname)
+
+            for dependant in list(required_by.get(ifname, list())):
+                requires[dependant].remove(ifname)
+                if not requires[dependant]:
+                    workqueue.append(dependant)
+
+        if len(ifname_ordering) != len(set(ifname_ordering)):
+            # should never happen
+            raise ValueError("Interface ordering contains duplicates")
+
+        unsatisfied_requirements = list(ifname for ifname, deps in requires.items() if deps)
+        if unsatisfied_requirements:
+            raise ValidationError(
+                _("Dependency cycle detected in subset [%(interfaces)s]"),
+                params={"interfaces": ", ".join(unsatisfied_requirements)},
+            )
+
+        # apply the topological sorting to the actual list
+        def get_sort_key(interface):
+            try:
+                return ifname_ordering.index(interface['name'])
+            except Exception:
+                # Everything broken/invalid goes to the beginning, so validation fails fast
+                return -1
+
+        interfaces.sort(key=lambda entry_tuple: get_sort_key(entry_tuple[1]))
+
+    def prep_related_object_list(self, field_name, enumerated_list):
+        if field_name == 'interfaces':
+            self._sort_interfaces(enumerated_list)
+
     def prep_related_object_data(self, parent, data):
         data.update({'device_type': parent})
         return data