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

Closes #23564: Batch cable termination lookups in cable and port REST API lists

Jeremy Stretch 16 часов назад
Родитель
Сommit
62b49d6f0c
3 измененных файлов с 120 добавлено и 22 удалено
  1. 23 14
      netbox/dcim/api/views.py
  2. 2 2
      netbox/dcim/tests/query_counts.json
  3. 95 6
      netbox/dcim/tests/test_api.py

+ 23 - 14
netbox/dcim/api/views.py

@@ -1,4 +1,5 @@
 from django.contrib.contenttypes.prefetch import GenericPrefetch
+from django.db.models import FETCH_PEERS, Prefetch
 from django.http import Http404, HttpResponse
 from django.shortcuts import get_object_or_404
 from drf_spectacular.types import OpenApiTypes
@@ -458,33 +459,38 @@ class ModuleViewSet(NetBoxModelViewSet):
 # Device components
 #
 
+def cable_terminations_prefetch(lookup):
+    # Objects loaded by a to-many prefetch don't inherit the parent queryset's fetch mode
+    return Prefetch(lookup, queryset=CableTermination.objects.fetch_mode(FETCH_PEERS))
+
+
 class ConsolePortViewSet(PathEndpointMixin, NetBoxModelViewSet):
-    queryset = ConsolePort.objects.prefetch_related(
-        '_path', 'cable__terminations',
+    queryset = ConsolePort.objects.fetch_mode(FETCH_PEERS).prefetch_related(
+        '_path', cable_terminations_prefetch('cable__terminations'),
     )
     serializer_class = serializers.ConsolePortSerializer
     filterset_class = filtersets.ConsolePortFilterSet
 
 
 class ConsoleServerPortViewSet(PathEndpointMixin, NetBoxModelViewSet):
-    queryset = ConsoleServerPort.objects.prefetch_related(
-        '_path', 'cable__terminations',
+    queryset = ConsoleServerPort.objects.fetch_mode(FETCH_PEERS).prefetch_related(
+        '_path', cable_terminations_prefetch('cable__terminations'),
     )
     serializer_class = serializers.ConsoleServerPortSerializer
     filterset_class = filtersets.ConsoleServerPortFilterSet
 
 
 class PowerPortViewSet(PathEndpointMixin, NetBoxModelViewSet):
-    queryset = PowerPort.objects.prefetch_related(
-        '_path', 'cable__terminations',
+    queryset = PowerPort.objects.fetch_mode(FETCH_PEERS).prefetch_related(
+        '_path', cable_terminations_prefetch('cable__terminations'),
     )
     serializer_class = serializers.PowerPortSerializer
     filterset_class = filtersets.PowerPortFilterSet
 
 
 class PowerOutletViewSet(PathEndpointMixin, NetBoxModelViewSet):
-    queryset = PowerOutlet.objects.prefetch_related(
-        '_path', 'cable__terminations',
+    queryset = PowerOutlet.objects.fetch_mode(FETCH_PEERS).prefetch_related(
+        '_path', cable_terminations_prefetch('cable__terminations'),
     )
     serializer_class = serializers.PowerOutletSerializer
     filterset_class = filtersets.PowerOutletFilterSet
@@ -527,16 +533,16 @@ class InterfaceViewSet(PathEndpointMixin, NetBoxModelViewSet):
 
 
 class FrontPortViewSet(PassThroughPortMixin, NetBoxModelViewSet):
-    queryset = FrontPort.objects.prefetch_related(
-        'cable__terminations',
+    queryset = FrontPort.objects.fetch_mode(FETCH_PEERS).prefetch_related(
+        cable_terminations_prefetch('cable__terminations'),
     )
     serializer_class = serializers.FrontPortSerializer
     filterset_class = filtersets.FrontPortFilterSet
 
 
 class RearPortViewSet(PassThroughPortMixin, NetBoxModelViewSet):
-    queryset = RearPort.objects.prefetch_related(
-        'cable__terminations',
+    queryset = RearPort.objects.fetch_mode(FETCH_PEERS).prefetch_related(
+        cable_terminations_prefetch('cable__terminations'),
     )
     serializer_class = serializers.RearPortSerializer
     filterset_class = filtersets.RearPortFilterSet
@@ -588,14 +594,17 @@ class MACAddressViewSet(NetBoxModelViewSet):
 #
 
 class CableViewSet(NetBoxModelViewSet):
-    queryset = Cable.objects.prefetch_related('terminations__termination')
+    queryset = Cable.objects.prefetch_related(
+        cable_terminations_prefetch('terminations'),
+        'terminations__termination',
+    )
     serializer_class = serializers.CableSerializer
     filterset_class = filtersets.CableFilterSet
 
 
 class CableTerminationViewSet(NetBoxReadOnlyModelViewSet):
     metadata_class = ContentTypeMetadata
-    queryset = CableTermination.objects.all()
+    queryset = CableTermination.objects.fetch_mode(FETCH_PEERS)
     serializer_class = serializers.CableTerminationSerializer
     filterset_class = filtersets.CableTerminationFilterSet
 

+ 2 - 2
netbox/dcim/tests/query_counts.json

@@ -1,9 +1,9 @@
 {
-  "cable:api_list_objects": 23,
+  "cable:api_list_objects": 17,
   "cable:list_objects_with_permission": 20,
   "cablebundle:api_list_objects": 12,
   "cablebundle:list_objects_with_permission": 16,
-  "cabletermination:api_list_objects": 16,
+  "cabletermination:api_list_objects": 14,
   "consoleconnection:list_objects_with_permission": 28,
   "consoleport:api_list_objects": 13,
   "consoleport:list_objects_with_permission": 17,

+ 95 - 6
netbox/dcim/tests/test_api.py

@@ -99,6 +99,46 @@ class Mixins:
             self.assertEqual(segment1[1]['label'], cable.label)
             self.assertEqual(segment1[2][0]['name'], peer_obj.name)
 
+    class LinkPeersMixin(APITestCase):
+        peer_termination_type = None
+
+        def test_list_objects_link_peers_query_count(self):
+            """
+            Listing cabled components loads their link peers, and the peers' devices and cables, with the same
+            number of queries however many components are listed.
+            """
+            self.add_permissions(f'dcim.view_{self.model._meta.model_name}')
+            objects = list(self.model.objects.order_by('pk')[:3])
+            peer_device = Device.objects.create(
+                site=Site.objects.first(),
+                device_type=DeviceType.objects.first(),
+                role=DeviceRole.objects.first(),
+                name='Peer Device'
+            )
+            peer_names = {}
+            for index, obj in enumerate(objects):
+                peer = self.peer_termination_type.objects.create(device=peer_device, name=f'Peer {index}')
+                Cable(a_terminations=[obj], b_terminations=[peer]).save()
+                peer_names[obj.pk] = peer.name
+            url = self._get_list_url()
+
+            def get_query_count(objs):
+                params = {'id': [obj.pk for obj in objs], 'fields': 'id,link_peers'}
+                with CaptureQueriesContext(connection) as queries:
+                    response = self.client.get(url, params, **self.header)
+                self.assertHttpStatus(response, status.HTTP_200_OK)
+                link_peers = {
+                    result['id']: [peer['name'] for peer in result['link_peers']]
+                    for result in response.data['results']
+                }
+                self.assertEqual(link_peers, {obj.pk: [peer_names[obj.pk]] for obj in objs})
+                # Token authentication refreshes last_used once a minute
+                return sum('"users_token"' not in query['sql'] for query in queries.captured_queries)
+
+            # Warm per-process caches (e.g. ContentType)
+            get_query_count(objects)
+            self.assertEqual(get_query_count(objects[:1]), get_query_count(objects))
+
     class FieldPrefetchMixin(APITestCase):
         viewset = None
         # Narrowed requests compared in addition to the default, brief and single-field ones
@@ -4036,7 +4076,7 @@ class ModuleTestCase(APIViewTestCases.APIViewTestCase):
         self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
 
 
-class ConsolePortTestCase(Mixins.ComponentTraceMixin, APIViewTestCases.APIViewTestCase):
+class ConsolePortTestCase(Mixins.ComponentTraceMixin, Mixins.LinkPeersMixin, APIViewTestCases.APIViewTestCase):
     model = ConsolePort
     brief_fields = ['_occupied', 'cable', 'description', 'device', 'display', 'id', 'name', 'url']
     bulk_update_data = {
@@ -4082,7 +4122,7 @@ class ConsolePortTestCase(Mixins.ComponentTraceMixin, APIViewTestCases.APIViewTe
         ]
 
 
-class ConsoleServerPortTestCase(Mixins.ComponentTraceMixin, APIViewTestCases.APIViewTestCase):
+class ConsoleServerPortTestCase(Mixins.ComponentTraceMixin, Mixins.LinkPeersMixin, APIViewTestCases.APIViewTestCase):
     model = ConsoleServerPort
     brief_fields = ['_occupied', 'cable', 'description', 'device', 'display', 'id', 'name', 'url']
     bulk_update_data = {
@@ -4128,7 +4168,7 @@ class ConsoleServerPortTestCase(Mixins.ComponentTraceMixin, APIViewTestCases.API
         ]
 
 
-class PowerPortTestCase(Mixins.ComponentTraceMixin, APIViewTestCases.APIViewTestCase):
+class PowerPortTestCase(Mixins.ComponentTraceMixin, Mixins.LinkPeersMixin, APIViewTestCases.APIViewTestCase):
     model = PowerPort
     brief_fields = ['_occupied', 'cable', 'description', 'device', 'display', 'id', 'name', 'url']
     bulk_update_data = {
@@ -4171,7 +4211,7 @@ class PowerPortTestCase(Mixins.ComponentTraceMixin, APIViewTestCases.APIViewTest
         ]
 
 
-class PowerOutletTestCase(Mixins.ComponentTraceMixin, APIViewTestCases.APIViewTestCase):
+class PowerOutletTestCase(Mixins.ComponentTraceMixin, Mixins.LinkPeersMixin, APIViewTestCases.APIViewTestCase):
     model = PowerOutlet
     brief_fields = ['_occupied', 'cable', 'description', 'device', 'display', 'id', 'name', 'url']
     bulk_update_data = {
@@ -5180,7 +5220,7 @@ class InterfaceTestCase(Mixins.FieldPrefetchMixin, Mixins.ComponentTraceMixin, A
         self.assertNotIn('"ipam_fhrpgroupassignment"', count_sql)
 
 
-class FrontPortTestCase(APIViewTestCases.APIViewTestCase):
+class FrontPortTestCase(Mixins.LinkPeersMixin, APIViewTestCases.APIViewTestCase):
     model = FrontPort
     brief_fields = ['_occupied', 'cable', 'description', 'device', 'display', 'id', 'name', 'url']
     bulk_update_data = {
@@ -5302,7 +5342,7 @@ class FrontPortTestCase(APIViewTestCases.APIViewTestCase):
         self.assertHttpStatus(response, status.HTTP_200_OK)
 
 
-class RearPortTestCase(APIViewTestCases.APIViewTestCase):
+class RearPortTestCase(Mixins.LinkPeersMixin, APIViewTestCases.APIViewTestCase):
     model = RearPort
     brief_fields = ['_occupied', 'cable', 'description', 'device', 'display', 'id', 'name', 'url']
     bulk_update_data = {
@@ -5950,6 +5990,31 @@ class CableTestCase(APIViewTestCases.APIViewTestCase):
             },
         ]
 
+    def test_list_objects_terminations_query_count(self):
+        """
+        Listing cables loads their terminating objects, and those objects' devices and cables, with the same number
+        of queries however many cables are listed.
+        """
+        self.add_permissions('dcim.view_cable')
+        cables = list(Cable.objects.order_by('pk')[:3])
+        url = self._get_list_url()
+
+        def get_query_count(cables):
+            with CaptureQueriesContext(connection) as queries:
+                response = self.client.get(url, {'id': [cable.pk for cable in cables]}, **self.header)
+            self.assertHttpStatus(response, status.HTTP_200_OK)
+            self.assertEqual(len(response.data['results']), len(cables))
+            for result in response.data['results']:
+                for termination in (*result['a_terminations'], *result['b_terminations']):
+                    self.assertEqual(termination['object']['cable']['id'], result['id'])
+                    self.assertIn(termination['object']['device']['name'], ('Device 1', 'Device 2'))
+            # Token authentication refreshes last_used once a minute
+            return sum('"users_token"' not in query['sql'] for query in queries.captured_queries)
+
+        # Warm per-process caches (e.g. ContentType)
+        get_query_count(cables)
+        self.assertEqual(get_query_count(cables[:1]), get_query_count(cables))
+
     def test_repeated_put_does_not_accumulate_paths(self):
         """
         Repeating an identical PUT must leave the cable with the two paths its terminations trace.
@@ -6324,6 +6389,30 @@ class CableTerminationTestCase(
         for cable in cables:
             cable.save()
 
+    def test_list_objects_query_count(self):
+        """
+        Listing cable terminations loads their terminating objects, and those objects' devices and cables, with the
+        same number of queries however many terminations are listed.
+        """
+        self.add_permissions('dcim.view_cabletermination')
+        cables = list(Cable.objects.order_by('pk')[:3])
+        url = self._get_list_url()
+
+        def get_query_count(cables):
+            with CaptureQueriesContext(connection) as queries:
+                response = self.client.get(url, {'cable_id': [cable.pk for cable in cables]}, **self.header)
+            self.assertHttpStatus(response, status.HTTP_200_OK)
+            self.assertEqual(len(response.data['results']), 2 * len(cables))
+            for result in response.data['results']:
+                self.assertEqual(result['termination']['cable']['id'], result['cable'])
+                self.assertIn(result['termination']['device']['name'], ('Device 1', 'Device 2'))
+            # Token authentication refreshes last_used once a minute
+            return sum('"users_token"' not in query['sql'] for query in queries.captured_queries)
+
+        # Warm per-process caches (e.g. ContentType)
+        get_query_count(cables)
+        self.assertEqual(get_query_count(cables[:1]), get_query_count(cables))
+
 
 class ConnectedDeviceTestCase(APITestCase):