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

Closes #23286: Avoid loading related objects for Interface counts in the REST API (#23404)

Martin Hauser 4 дней назад
Родитель
Сommit
646391c359

+ 12 - 3
netbox/dcim/api/serializers_/device_components.py

@@ -25,7 +25,7 @@ from dcim.models import (
 from ipam.api.serializers_.vlans import VLANSerializer, VLANTranslationPolicySerializer
 from ipam.api.serializers_.vlans import VLANSerializer, VLANTranslationPolicySerializer
 from ipam.api.serializers_.vrfs import VRFSerializer
 from ipam.api.serializers_.vrfs import VRFSerializer
 from ipam.models import VLAN
 from ipam.models import VLAN
-from netbox.api.fields import ChoiceField, ContentTypeField, SerializedPKRelatedField
+from netbox.api.fields import ChoiceField, ContentTypeField, RelatedObjectCountField, SerializedPKRelatedField
 from netbox.api.gfk_fields import GFKSerializerField
 from netbox.api.gfk_fields import GFKSerializerField
 from netbox.api.serializers import NetBoxModelSerializer
 from netbox.api.serializers import NetBoxModelSerializer
 from netbox.choices import DiameterUnitChoices, FlowRateUnitChoices
 from netbox.choices import DiameterUnitChoices, FlowRateUnitChoices
@@ -334,8 +334,17 @@ class InterfaceSerializer(
         required=False,
         required=False,
         many=True
         many=True
     )
     )
-    count_ipaddresses = serializers.IntegerField(read_only=True)
-    count_fhrp_groups = serializers.IntegerField(read_only=True)
+    # Annotated under private names, since the count properties they fall back to are read-only
+    count_ipaddresses = RelatedObjectCountField(
+        'ip_addresses',
+        source='_count_ipaddresses',
+        fallback_source='count_ipaddresses'
+    )
+    count_fhrp_groups = RelatedObjectCountField(
+        'fhrp_group_assignments',
+        source='_count_fhrp_groups',
+        fallback_source='count_fhrp_groups'
+    )
     # Maintains backward compatibility with NetBox <v4.2; also accepts a MAC string on write to
     # Maintains backward compatibility with NetBox <v4.2; also accepts a MAC string on write to
     # create/update the primary MAC address in a single request.
     # create/update the primary MAC address in a single request.
     mac_address = serializers.CharField(allow_null=True, required=False)
     mac_address = serializers.CharField(allow_null=True, required=False)

+ 1 - 0
netbox/dcim/api/views.py

@@ -506,6 +506,7 @@ class InterfaceViewSet(PathEndpointMixin, NetBoxModelViewSet):
     queryset = Interface.objects.all()
     queryset = Interface.objects.all()
     serializer_class = serializers.InterfaceSerializer
     serializer_class = serializers.InterfaceSerializer
     filterset_class = filtersets.InterfaceFilterSet
     filterset_class = filtersets.InterfaceFilterSet
+    pagination_class = StripCountAnnotationsPaginator
     field_prefetches = {
     field_prefetches = {
         ('link_peers', 'link_peers_type'): (
         ('link_peers', 'link_peers_type'): (
             GenericPrefetch('cable__terminations__termination', [Interface.objects.select_related('device', 'cable')]),
             GenericPrefetch('cable__terminations__termination', [Interface.objects.select_related('device', 'cable')]),

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

@@ -33,7 +33,7 @@
   "frontport:api_list_objects": 14,
   "frontport:api_list_objects": 14,
   "frontport:list_objects_with_permission": 21,
   "frontport:list_objects_with_permission": 21,
   "frontporttemplate:api_list_objects": 12,
   "frontporttemplate:api_list_objects": 12,
-  "interface:api_list_objects": 23,
+  "interface:api_list_objects": 21,
   "interface:list_objects_with_permission": 17,
   "interface:list_objects_with_permission": 17,
   "interfaceconnection:list_objects_with_permission": 40,
   "interfaceconnection:list_objects_with_permission": 40,
   "interfacetemplate:api_list_objects": 11,
   "interfacetemplate:api_list_objects": 11,

+ 183 - 25
netbox/dcim/tests/test_api.py

@@ -42,10 +42,11 @@ from utilities.testing import (
     create_test_device,
     create_test_device,
     create_test_image,
     create_test_image,
     create_test_nat_ip_pair,
     create_test_nat_ip_pair,
+    create_test_virtualmachine,
     disable_logging,
     disable_logging,
     disable_warnings,
     disable_warnings,
 )
 )
-from virtualization.models import Cluster, ClusterType
+from virtualization.models import Cluster, ClusterType, VMInterface
 from vpn.choices import L2VPNTypeChoices
 from vpn.choices import L2VPNTypeChoices
 from vpn.models import L2VPN, L2VPNTermination
 from vpn.models import L2VPN, L2VPNTermination
 from wireless.choices import WirelessChannelChoices
 from wireless.choices import WirelessChannelChoices
@@ -4088,8 +4089,6 @@ class InterfaceTestCase(Mixins.FieldPrefetchMixin, Mixins.ComponentTraceMixin, A
         GenericPrefetch('_path__path_objects', [Interface.objects.select_related('device', 'cable')]),
         GenericPrefetch('_path__path_objects', [Interface.objects.select_related('device', 'cable')]),
         'virtual_circuit_termination',
         'virtual_circuit_termination',
         'l2vpn_terminations',
         'l2vpn_terminations',
-        'ip_addresses',
-        'fhrp_group_assignments',
     )
     )
     brief_fields = ['_occupied', 'cable', 'description', 'device', 'display', 'id', 'name', 'url']
     brief_fields = ['_occupied', 'cable', 'description', 'device', 'display', 'id', 'name', 'url']
     bulk_update_data = {
     bulk_update_data = {
@@ -4732,37 +4731,44 @@ class InterfaceTestCase(Mixins.FieldPrefetchMixin, Mixins.ComponentTraceMixin, A
         self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
         self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
 
 
     def test_field_prefetches_follow_requested_fields(self):
     def test_field_prefetches_follow_requested_fields(self):
-        """List and detail responses carry the declared prefetches of exactly the fields they include."""
+        """List and detail responses prefetch the declarations of included fields, counts annotated instead."""
         self.add_permissions('dcim.view_interface')
         self.add_permissions('dcim.view_interface')
         declared = InterfaceViewSet.field_prefetches
         declared = InterfaceViewSet.field_prefetches
+        declared_before = dict(declared)
         peers = list(declared[('link_peers', 'link_peers_type')])
         peers = list(declared[('link_peers', 'link_peers_type')])
         endpoints = list(declared[('connected_endpoints', 'connected_endpoints_type')])
         endpoints = list(declared[('connected_endpoints', 'connected_endpoints_type')])
         path = list(declared[('connected_endpoints_reachable',)])
         path = list(declared[('connected_endpoints_reachable',)])
         l2vpn = list(declared[('l2vpn_termination',)])
         l2vpn = list(declared[('l2vpn_termination',)])
-        ips, fhrp = list(declared[('count_ipaddresses',)]), list(declared[('count_fhrp_groups',)])
+        counts = {'_count_ipaddresses', '_count_fhrp_groups'}
         # fields takes precedence over omit, and both over brief
         # fields takes precedence over omit, and both over brief
         cases = {
         cases = {
-            '': peers + endpoints + path + l2vpn + ips + fhrp,
-            'brief=1': [],
-            'fields=id,link_peers_type': peers,
-            'fields=id,connected_endpoints_type': endpoints,
-            'fields=id,connected_endpoints_reachable': path,
-            'fields=id,l2vpn_termination': l2vpn,
-            'fields=id,count_ipaddresses': ips,
-            'fields=id,count_fhrp_groups': fhrp,
-            'omit=link_peers,link_peers_type': endpoints + path + l2vpn + ips + fhrp,
-            'fields=id,link_peers&omit=link_peers': peers,
-            'brief=1&omit=l2vpn_termination': peers + endpoints + path + ips + fhrp,
-            'brief=1&fields=id': [],
+            '': (peers + endpoints + path + l2vpn, counts),
+            'brief=1': ([], set()),
+            'fields=id,link_peers_type': (peers, set()),
+            'fields=id,connected_endpoints_type': (endpoints, set()),
+            'fields=id,connected_endpoints_reachable': (path, set()),
+            'fields=id,l2vpn_termination': (l2vpn, set()),
+            'fields=id,count_ipaddresses': ([], {'_count_ipaddresses'}),
+            'fields=id,count_fhrp_groups': ([], {'_count_fhrp_groups'}),
+            'fields=count_ipaddresses': ([], {'_count_ipaddresses'}),
+            'omit=link_peers,link_peers_type': (endpoints + path + l2vpn, counts),
+            'omit=count_ipaddresses': (peers + endpoints + path + l2vpn, {'_count_fhrp_groups'}),
+            'fields=id,link_peers&omit=link_peers': (peers, set()),
+            'brief=1&omit=l2vpn_termination': (peers + endpoints + path, counts),
+            'brief=1&fields=id': ([], set()),
         }
         }
         for url in (self._get_list_url(), self._get_detail_url(Interface.objects.get(name='Interface 1'))):
         for url in (self._get_list_url(), self._get_detail_url(Interface.objects.get(name='Interface 1'))):
-            for params, expected in cases.items():
+            for params, (expected, annotations) in cases.items():
                 with self.subTest(url=url, params=params):
                 with self.subTest(url=url, params=params):
                     with self.capture_querysets(InterfaceViewSet) as querysets:
                     with self.capture_querysets(InterfaceViewSet) as querysets:
                         response = self.client.get(f'{url}?{params}', **self.header)
                         response = self.client.get(f'{url}?{params}', **self.header)
                     self.assertHttpStatus(response, status.HTTP_200_OK)
                     self.assertHttpStatus(response, status.HTTP_200_OK)
                     self.assert_lookups(querysets, expected)
                     self.assert_lookups(querysets, expected)
+                    for queryset in querysets:
+                        self.assertEqual(set(queryset.query.annotations), annotations)
         self.assertEqual(InterfaceViewSet.queryset._prefetch_related_lookups, ())
         self.assertEqual(InterfaceViewSet.queryset._prefetch_related_lookups, ())
+        # Reads narrow their own field set, never the class declarations
+        self.assertEqual(InterfaceViewSet.field_prefetches, declared_before)
 
 
     def test_field_prefetches_keep_class_lookups(self):
     def test_field_prefetches_keep_class_lookups(self):
         """Lookups on the class queryset stay ahead of the declared ones, and a subclass inherits the declarations."""
         """Lookups on the class queryset stay ahead of the declared ones, and a subclass inherits the declarations."""
@@ -4779,6 +4785,18 @@ class InterfaceTestCase(Mixins.FieldPrefetchMixin, Mixins.ComponentTraceMixin, A
         self.assertHttpStatus(response, status.HTTP_200_OK)
         self.assertHttpStatus(response, status.HTTP_200_OK)
         self.assert_lookups(querysets, [*queryset._prefetch_related_lookups, *peers])
         self.assert_lookups(querysets, [*queryset._prefetch_related_lookups, *peers])
 
 
+        # A class lookup on a counted relation stays when its count is annotated
+        queryset = InterfaceViewSet.queryset.prefetch_related('ip_addresses')
+        with (
+            patch.object(InterfaceViewSet, 'queryset', queryset),
+            self.capture_querysets(InterfaceViewSet) as querysets,
+        ):
+            response = self.client.get(f'{url}?fields=id,count_ipaddresses', **self.header)
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        self.assert_lookups(querysets, list(queryset._prefetch_related_lookups))
+        for captured in querysets:
+            self.assertEqual(set(captured.query.annotations), {'_count_ipaddresses'})
+
         class PluginInterfaceViewSet(InterfaceViewSet):
         class PluginInterfaceViewSet(InterfaceViewSet):
             pass
             pass
 
 
@@ -4789,8 +4807,25 @@ class InterfaceTestCase(Mixins.FieldPrefetchMixin, Mixins.ComponentTraceMixin, A
         self.assertHttpStatus(response, status.HTTP_200_OK)
         self.assertHttpStatus(response, status.HTTP_200_OK)
         self.assert_lookups(querysets, peers)
         self.assert_lookups(querysets, peers)
 
 
+    def test_field_prefetches_keep_shared_declarations(self):
+        """A declaration a count shares with another requested field stays when the count is annotated."""
+        self.add_permissions('dcim.view_interface')
+        shared = ('ip_addresses',)
+        field_prefetches = {**InterfaceViewSet.field_prefetches, ('count_ipaddresses', 'description'): shared}
+        with (
+            patch.object(InterfaceViewSet, 'field_prefetches', field_prefetches),
+            self.capture_querysets(InterfaceViewSet) as querysets,
+        ):
+            response = self.client.get(
+                f'{self._get_list_url()}?fields=id,description,count_ipaddresses', **self.header
+            )
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        self.assert_lookups(querysets, list(shared))
+        for queryset in querysets:
+            self.assertEqual(set(queryset.query.annotations), {'_count_ipaddresses'})
+
     def test_field_prefetches_for_other_reads(self):
     def test_field_prefetches_for_other_reads(self):
-        """Exports and writes carry every declaration, the trace action its path nodes."""
+        """Exports and writes carry every declaration and annotate no count, the trace action its path nodes."""
         self.add_permissions('dcim.view_interface', 'dcim.change_interface', 'extras.view_exporttemplate')
         self.add_permissions('dcim.view_interface', 'dcim.change_interface', 'extras.view_exporttemplate')
         declared = [lookup for lookups in InterfaceViewSet.field_prefetches.values() for lookup in lookups]
         declared = [lookup for lookups in InterfaceViewSet.field_prefetches.values() for lookup in lookups]
         interface = Interface.objects.get(name='Interface 1')
         interface = Interface.objects.get(name='Interface 1')
@@ -4801,13 +4836,16 @@ class InterfaceTestCase(Mixins.FieldPrefetchMixin, Mixins.ComponentTraceMixin, A
 
 
         # Exports and writes load every declaration, whatever fields the response selects
         # Exports and writes load every declaration, whatever fields the response selects
         with self.capture_querysets(InterfaceViewSet) as querysets:
         with self.capture_querysets(InterfaceViewSet) as querysets:
-            response = self.client.get(f'{self._get_list_url()}?fields=id&export=Interfaces', **self.header)
-            self.assertHttpStatus(response, status.HTTP_200_OK)
-            response = self.client.patch(
-                f'{self._get_detail_url(interface)}?fields=id', {'description': 'New'}, format='json', **self.header
-            )
-            self.assertHttpStatus(response, status.HTTP_200_OK)
+            for params in ('fields=id&export=Interfaces', 'export=Interfaces'):
+                response = self.client.get(f'{self._get_list_url()}?{params}', **self.header)
+                self.assertHttpStatus(response, status.HTTP_200_OK)
+            for params in ('fields=id', 'fields=id,count_ipaddresses'):
+                response = self.client.patch(
+                    f'{self._get_detail_url(interface)}?{params}', {'description': 'New'}, format='json', **self.header
+                )
+                self.assertHttpStatus(response, status.HTTP_200_OK)
         self.assert_lookups(querysets, declared)
         self.assert_lookups(querysets, declared)
+        self.assertFalse([queryset.query.annotations for queryset in querysets if queryset.query.annotations])
 
 
         # The trace action loads the path nodes as the connected endpoint fields do, whatever fields are selected
         # The trace action loads the path nodes as the connected endpoint fields do, whatever fields are selected
         peer = Interface.objects.create(device=interface.device, name='Peer', type=InterfaceTypeChoices.TYPE_1GE_FIXED)
         peer = Interface.objects.create(device=interface.device, name='Peer', type=InterfaceTypeChoices.TYPE_1GE_FIXED)
@@ -4836,6 +4874,126 @@ class InterfaceTestCase(Mixins.FieldPrefetchMixin, Mixins.ComponentTraceMixin, A
                 self.assertLessEqual(count, eager_count)
                 self.assertLessEqual(count, eager_count)
                 self.assertLess(count, unprefetched_count)
                 self.assertLess(count, unprefetched_count)
 
 
+    def test_count_ipaddresses_and_fhrp_groups(self):
+        """Counts come from annotations of the interface's object type, never from loaded related objects."""
+        self.add_permissions('dcim.view_interface')
+        interface = Interface.objects.get(name='Interface 1')
+        group = FHRPGroup.objects.create(
+            pk=interface.pk, protocol=FHRPGroupProtocolChoices.PROTOCOL_VRRP2, group_id=1
+        )
+        other_group = FHRPGroup.objects.create(
+            pk=interface.pk + 1, protocol=FHRPGroupProtocolChoices.PROTOCOL_VRRP2, group_id=2
+        )
+        vm_interface = VMInterface.objects.create(
+            pk=interface.pk, virtual_machine=create_test_virtualmachine('Virtual Machine 1'), name='eth0'
+        )
+        IPAddress.objects.bulk_create((
+            IPAddress(address='192.0.2.1/24', assigned_object=interface),
+            IPAddress(address='2001:db8::1/64', assigned_object=interface),
+            IPAddress(address='192.0.2.2/24', assigned_object=vm_interface),
+            IPAddress(address='192.0.2.254/24', assigned_object=group),
+        ))
+        FHRPGroupAssignment.objects.bulk_create((
+            FHRPGroupAssignment(group=group, interface=interface, priority=100),
+            # Two on the VM interface, so a count of the wrong object type cannot match
+            FHRPGroupAssignment(group=group, interface=vm_interface, priority=100),
+            FHRPGroupAssignment(group=other_group, interface=vm_interface, priority=100),
+        ))
+
+        with CaptureQueriesContext(connection) as queries:
+            response = self.client.get(self._get_list_url(), **self.header)
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        counts = {
+            row['name']: (row['count_ipaddresses'], row['count_fhrp_groups']) for row in response.data['results']
+        }
+        self.assertEqual(counts, {'Interface 1': (2, 1), 'Interface 2': (0, 0), 'Interface 3': (0, 0)})
+        # Only the interface statement reads the related tables, through its two count subqueries
+        statements = [
+            query['sql'] for query in queries.captured_queries
+            if '"ipam_ipaddress"' in query['sql'] or '"ipam_fhrpgroupassignment"' in query['sql']
+        ]
+        self.assertEqual(len(statements), 1)
+        self.assertTrue(statements[0].startswith('SELECT "dcim_interface"'))
+        # Unannotated instances count through the relations
+        self.assertEqual((interface.count_ipaddresses, interface.count_fhrp_groups), (2, 1))
+
+    def test_count_event_payloads_use_write_prefetches(self):
+        """Write and delete event payloads take both counts from the write prefetches, whatever fields are selected."""
+        self.add_permissions('dcim.view_interface', 'dcim.change_interface', 'dcim.delete_interface')
+        interfaces = Interface.objects.filter(name__in=('Interface 1', 'Interface 2')).order_by('name')
+        interface1, interface2 = interfaces
+        group = FHRPGroup.objects.create(protocol=FHRPGroupProtocolChoices.PROTOCOL_VRRP2, group_id=1)
+        IPAddress.objects.bulk_create((
+            IPAddress(address='192.0.2.1/24', assigned_object=interface1),
+            IPAddress(address='192.0.2.2/24', assigned_object=interface1),
+            IPAddress(address='192.0.2.3/24', assigned_object=interface2),
+        ))
+        FHRPGroupAssignment.objects.create(group=group, interface=interface1, priority=100)
+
+        def payload_counts(events):
+            return {
+                event['data']['name']: (event['data']['count_ipaddresses'], event['data']['count_fhrp_groups'])
+                for event in events if isinstance(event['object'], Interface)
+            }
+
+        def fallback_counts(queries):
+            # The count properties' per-interface fallback
+            return [
+                query['sql'] for query in queries.captured_queries
+                if query['sql'].startswith('SELECT COUNT(*)') and (
+                    '"ipam_ipaddress"."assigned_object_id"' in query['sql']
+                    or '"ipam_fhrpgroupassignment"."interface_id"' in query['sql']
+                )
+            ]
+
+        writes = (
+            ('single', self._get_detail_url(interface1), {'description': 'New'}, {'Interface 1': (2, 1)}),
+            (
+                'bulk', self._get_list_url(),
+                [{'id': interface.pk, 'description': 'Bulk'} for interface in interfaces],
+                {'Interface 1': (2, 1), 'Interface 2': (1, 0)},
+            ),
+        )
+        for label, url, data, expected in writes:
+            with self.subTest(write=label):
+                with (
+                    patch('netbox.context_managers.flush_events') as flush_events,
+                    CaptureQueriesContext(connection) as queries,
+                ):
+                    response = self.client.patch(f'{url}?fields=id,description', data, format='json', **self.header)
+                    self.assertHttpStatus(response, status.HTTP_200_OK)
+                    (events,) = flush_events.call_args.args
+                    # Update payloads serialize on first access
+                    counts = payload_counts(events)
+                self.assertEqual(counts, expected)
+                self.assertEqual(fallback_counts(queries), [])
+
+        # Delete payloads serialize before the rows go, within the request
+        with (
+            patch('netbox.context_managers.flush_events') as flush_events,
+            CaptureQueriesContext(connection) as queries,
+        ):
+            response = self.client.delete(f'{self._get_detail_url(interface1)}?fields=id', **self.header)
+        self.assertHttpStatus(response, status.HTTP_204_NO_CONTENT)
+        (events,) = flush_events.call_args.args
+        self.assertEqual(payload_counts(events), {'Interface 1': (2, 1)})
+        self.assertEqual(fallback_counts(queries), [])
+
+    def test_pagination_count_excludes_count_subqueries(self):
+        """The pagination count of a DISTINCT-filtered list leaves out the count subqueries."""
+        self.add_permissions('dcim.view_interface')
+        with CaptureQueriesContext(connection) as queries:
+            response = self.client.get(self._get_list_url(), {'name': 'Interface 1'}, **self.header)
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        self.assertEqual(response.data['count'], 1)
+        (count_sql,) = [
+            query['sql'] for query in queries.captured_queries if query['sql'].startswith('SELECT COUNT(*)')
+        ]
+        # The name filter must make the list DISTINCT for this test to mean anything
+        self.assertIn('DISTINCT', count_sql)
+        self.assertNotIn('"ipam_ipaddress"', count_sql)
+        self.assertNotIn('"ipam_fhrpgroupassignment"', count_sql)
+
 
 
 class FrontPortTestCase(APIViewTestCases.APIViewTestCase):
 class FrontPortTestCase(APIViewTestCases.APIViewTestCase):
     model = FrontPort
     model = FrontPort

+ 14 - 1
netbox/netbox/api/fields.py

@@ -8,6 +8,7 @@ from drf_spectacular.utils import extend_schema_field
 from netaddr import IPNetwork
 from netaddr import IPNetwork
 from rest_framework import serializers
 from rest_framework import serializers
 from rest_framework.exceptions import ValidationError
 from rest_framework.exceptions import ValidationError
+from rest_framework.fields import get_attribute
 from rest_framework.relations import PrimaryKeyRelatedField, RelatedField
 from rest_framework.relations import PrimaryKeyRelatedField, RelatedField
 
 
 from utilities.data import get_inclusive_integer_range_bounds
 from utilities.data import get_inclusive_integer_range_bounds
@@ -162,12 +163,24 @@ class RelatedObjectCountField(serializers.ReadOnlyField):
     Represents a read-only integer count of related objects (e.g. the number of racks assigned to a site). This field
     Represents a read-only integer count of related objects (e.g. the number of racks assigned to a site). This field
     is detected by get_annotations_for_serializer() when determining the annotations to be added to a queryset
     is detected by get_annotations_for_serializer() when determining the annotations to be added to a queryset
     depending on the serializer fields selected for inclusion in the response.
     depending on the serializer fields selected for inclusion in the response.
+
+    :param relation: A reverse relation or a GenericRelation
+    :param fallback_source: Attribute read when unannotated, as on exports and writes, which need field_prefetches
     """
     """
-    def __init__(self, relation, **kwargs):
+    def __init__(self, relation, *, fallback_source=None, **kwargs):
         self.relation = relation
         self.relation = relation
+        self.fallback_source = fallback_source
 
 
         super().__init__(**kwargs)
         super().__init__(**kwargs)
 
 
+    def get_attribute(self, instance):
+        if not self.fallback_source:
+            return super().get_attribute(instance)
+        try:
+            return get_attribute(instance, self.source_attrs)
+        except (AttributeError, KeyError):
+            return get_attribute(instance, self.fallback_source.split('.'))
+
 
 
 class IntegerRangeSerializer(serializers.Serializer):
 class IntegerRangeSerializer(serializers.Serializer):
     """
     """

+ 3 - 3
netbox/netbox/api/serializers/base.py

@@ -74,11 +74,11 @@ class BaseModelSerializer(serializers.ModelSerializer):
 
 
         # Related object counts are populated by annotations applied to the viewset's queryset, but these
         # Related object counts are populated by annotations applied to the viewset's queryset, but these
         # annotations are not applied when the object is represented as a nested (brief) related object. Omit
         # annotations are not applied when the object is represented as a nested (brief) related object. Omit
-        # these fields when serializing a nested object to avoid advertising fields that will never be populated
-        # (and which would otherwise be declared as required in the generated OpenAPI schema). See #22154.
+        # fields without a fallback when serializing a nested object to avoid advertising fields that will never be
+        # populated (and which would otherwise be declared as required in the generated OpenAPI schema). See #22154.
         if self.nested:
         if self.nested:
             for field_name, field in list(fields.items()):
             for field_name, field in list(fields.items()):
-                if isinstance(field, RelatedObjectCountField):
+                if isinstance(field, RelatedObjectCountField) and not field.fallback_source:
                     fields.pop(field_name)
                     fields.pop(field_name)
 
 
         return fields
         return fields

+ 19 - 3
netbox/netbox/api/viewsets/__init__.py

@@ -12,6 +12,7 @@ from rest_framework.permissions import SAFE_METHODS
 from rest_framework.response import Response
 from rest_framework.response import Response
 from rest_framework.viewsets import GenericViewSet
 from rest_framework.viewsets import GenericViewSet
 
 
+from netbox.api.fields import RelatedObjectCountField
 from netbox.api.serializers.features import ChangeLogMessageSerializer
 from netbox.api.serializers.features import ChangeLogMessageSerializer
 from utilities.api import get_annotations_for_serializer, get_fields_for_serializer, get_prefetches_for_serializer
 from utilities.api import get_annotations_for_serializer, get_fields_for_serializer, get_prefetches_for_serializer
 from utilities.exceptions import AbortRequest, PreconditionFailed
 from utilities.exceptions import AbortRequest, PreconditionFailed
@@ -123,7 +124,8 @@ class BaseViewSet(GenericViewSet):
             qs = qs.prefetch_related(*prefetch)
             qs = qs.prefetch_related(*prefetch)
 
 
         # Dynamically resolve annotations for RelatedObjectCountFields on the serializer and attach them to the queryset
         # Dynamically resolve annotations for RelatedObjectCountFields on the serializer and attach them to the queryset
-        if annotations := get_annotations_for_serializer(serializer_class, **self.field_kwargs):
+        exclude = self.fallback_count_fields if self.renders_whole_objects else ()
+        if annotations := get_annotations_for_serializer(serializer_class, **self.field_kwargs, exclude=exclude):
             qs = qs.annotate(**annotations)
             qs = qs.annotate(**annotations)
 
 
         return qs
         return qs
@@ -134,10 +136,11 @@ class BaseViewSet(GenericViewSet):
             return []
             return []
         if fields is None:
         if fields is None:
             # Exports and writes render whole objects (templates, event payloads), so they load every declaration
             # Exports and writes render whole objects (templates, event payloads), so they load every declaration
-            if 'export' in self.request.query_params or self.request.method not in SAFE_METHODS:
+            if self.renders_whole_objects:
                 fields = set().union(*self.field_prefetches)
                 fields = set().union(*self.field_prefetches)
             else:
             else:
-                fields = self.response_fields
+                # Reads annotate the counts with a fallback in place of their declarations
+                fields = self.response_fields - self.fallback_count_fields
         return [
         return [
             lookup
             lookup
             for field_names, lookups in self.field_prefetches.items()
             for field_names, lookups in self.field_prefetches.items()
@@ -174,6 +177,19 @@ class BaseViewSet(GenericViewSet):
         """Return the serializer field names rendered for this request, as selected by field_kwargs."""
         """Return the serializer field names rendered for this request, as selected by field_kwargs."""
         return frozenset(get_fields_for_serializer(self.get_serializer_class(), **self.field_kwargs))
         return frozenset(get_fields_for_serializer(self.get_serializer_class(), **self.field_kwargs))
 
 
+    @cached_property
+    def renders_whole_objects(self):
+        """Return True for exports and writes, whatever fields the response selects."""
+        return 'export' in self.request.query_params or self.request.method not in SAFE_METHODS
+
+    @cached_property
+    def fallback_count_fields(self):
+        """Return the names of the count fields annotated for reads only."""
+        return frozenset(
+            name for name, field in self.get_serializer_class()._declared_fields.items()
+            if type(field) is RelatedObjectCountField and field.fallback_source
+        )
+
 
 
 class NetBoxReadOnlyModelViewSet(
 class NetBoxReadOnlyModelViewSet(
     ETagMixin,
     ETagMixin,

+ 14 - 4
netbox/netbox/tests/test_api.py

@@ -15,8 +15,8 @@ from rest_framework.settings import api_settings
 from rest_framework.test import APIClient
 from rest_framework.test import APIClient
 
 
 from core.models import DataFile, DataSource, ObjectType
 from core.models import DataFile, DataSource, ObjectType
-from dcim.api.serializers import RackSerializer
-from dcim.models import Device, Site
+from dcim.api.serializers import InterfaceSerializer, RackSerializer
+from dcim.models import Device, Rack, Site
 from extras.models import ExportTemplate
 from extras.models import ExportTemplate
 from ipam.api.serializers import RouteTargetSerializer, VRFSerializer
 from ipam.api.serializers import RouteTargetSerializer, VRFSerializer
 from ipam.models import VRF, RouteTarget
 from ipam.models import VRF, RouteTarget
@@ -108,8 +108,8 @@ class RelatedObjectCountFieldTestCase(TestCase):
     """
     """
     RelatedObjectCountFields are populated by annotations applied to a viewset's queryset, which are only
     RelatedObjectCountFields are populated by annotations applied to a viewset's queryset, which are only
     added when serializing an object via its own endpoint (including ?brief=1). They are never annotated when
     added when serializing an object via its own endpoint (including ?brief=1). They are never annotated when
-    the object is rendered as a nested related object, so they must be omitted from nested representations to
-    keep the generated OpenAPI schema honest. See #22154.
+    the object is rendered as a nested related object, so those without a fallback must be omitted from nested
+    representations to keep the generated OpenAPI schema honest. See #22154.
     """
     """
     def test_count_field_omitted_when_nested(self):
     def test_count_field_omitted_when_nested(self):
         """A nested serializer must drop RelatedObjectCountFields (e.g. RackSerializer.device_count)."""
         """A nested serializer must drop RelatedObjectCountFields (e.g. RackSerializer.device_count)."""
@@ -126,6 +126,16 @@ class RelatedObjectCountFieldTestCase(TestCase):
         self.assertIn('device_count', serializer.fields)
         self.assertIn('device_count', serializer.fields)
         self.assertIsInstance(serializer.fields['device_count'], RelatedObjectCountField)
         self.assertIsInstance(serializer.fields['device_count'], RelatedObjectCountField)
 
 
+    def test_count_field_with_fallback_retained_when_nested(self):
+        """A nested serializer keeps a requested count field that has a fallback."""
+        serializer = InterfaceSerializer(nested=True, fields=['id', 'count_ipaddresses'])
+        self.assertEqual(set(serializer.fields), {'id', 'count_ipaddresses'})
+
+    def test_count_field_without_fallback_omitted_when_unannotated(self):
+        """A count field without a fallback is left out of an unannotated object's representation."""
+        serializer = RackSerializer(Rack(pk=1), fields=['id', 'device_count'])
+        self.assertEqual(serializer.data, {'id': 1})
+
 
 
 class NetBoxPaginationTestCase(TestCase):
 class NetBoxPaginationTestCase(TestCase):
 
 

+ 13 - 5
netbox/utilities/api.py

@@ -1,6 +1,6 @@
 import logging
 import logging
 
 
-from django.contrib.contenttypes.fields import GenericForeignKey
+from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation
 from django.core.exceptions import (
 from django.core.exceptions import (
     FieldDoesNotExist,
     FieldDoesNotExist,
     FieldError,
     FieldError,
@@ -242,18 +242,26 @@ def get_prefetches_for_serializer(serializer_class, fields=None, omit=None, _ser
     return prefetch_fields
     return prefetch_fields
 
 
 
 
-def get_annotations_for_serializer(serializer_class, fields=None, omit=None):
+def get_annotations_for_serializer(serializer_class, fields=None, omit=None, exclude=()):
     """
     """
-    Return a mapping of field names to annotations to be applied to the queryset for a serializer.
+    Return a mapping of attribute names to annotations to be applied to the queryset for a serializer.
     """
     """
     effective_fields = get_fields_for_serializer(serializer_class, fields, omit)
     effective_fields = get_fields_for_serializer(serializer_class, fields, omit)
     model = serializer_class.Meta.model
     model = serializer_class.Meta.model
 
 
     annotations = {}
     annotations = {}
     for field_name, field in serializer_class._declared_fields.items():
     for field_name, field in serializer_class._declared_fields.items():
-        if field_name in effective_fields and type(field) is RelatedObjectCountField:
+        if field_name in effective_fields and field_name not in exclude and type(field) is RelatedObjectCountField:
             related_field = getattr(model, field.relation).field
             related_field = getattr(model, field.relation).field
-            annotations[field_name] = count_related(related_field.model, related_field.name)
+            if isinstance(related_field, GenericRelation):
+                count = count_related(
+                    related_field.related_model,
+                    related_field.object_id_field_name,
+                    **{related_field.content_type_field_name: related_field.get_content_type()}
+                )
+            else:
+                count = count_related(related_field.model, related_field.name)
+            annotations[field.source or field_name] = count
 
 
     return annotations
     return annotations
 
 

+ 2 - 2
netbox/utilities/query.py

@@ -8,13 +8,13 @@ __all__ = (
 )
 )
 
 
 
 
-def count_related(model, field):
+def count_related(model, field, **filters):
     """
     """
     Return a Subquery suitable for annotating a child object count.
     Return a Subquery suitable for annotating a child object count.
     """
     """
     subquery = Subquery(
     subquery = Subquery(
         model.objects.filter(
         model.objects.filter(
-            **{field: OuterRef('pk')}
+            **{field: OuterRef('pk')}, **filters
         ).order_by().values(
         ).order_by().values(
             field
             field
         ).annotate(
         ).annotate(

+ 12 - 3
netbox/virtualization/api/serializers_/virtualmachines.py

@@ -14,7 +14,7 @@ from ipam.api.serializers_.ip import IPAddressSerializer
 from ipam.api.serializers_.vlans import VLANSerializer, VLANTranslationPolicySerializer
 from ipam.api.serializers_.vlans import VLANSerializer, VLANTranslationPolicySerializer
 from ipam.api.serializers_.vrfs import VRFSerializer
 from ipam.api.serializers_.vrfs import VRFSerializer
 from ipam.models import VLAN
 from ipam.models import VLAN
-from netbox.api.fields import ChoiceField, SerializedPKRelatedField
+from netbox.api.fields import ChoiceField, RelatedObjectCountField, SerializedPKRelatedField
 from netbox.api.serializers import NetBoxModelSerializer, PrimaryModelSerializer
 from netbox.api.serializers import NetBoxModelSerializer, PrimaryModelSerializer
 from tenancy.api.serializers_.tenants import TenantSerializer
 from tenancy.api.serializers_.tenants import TenantSerializer
 from users.api.serializers_.mixins import OwnerMixin
 from users.api.serializers_.mixins import OwnerMixin
@@ -121,8 +121,17 @@ class VMInterfaceSerializer(MACAddressShortcutMixin, OwnerMixin, NetBoxModelSeri
     vlan_translation_policy = VLANTranslationPolicySerializer(nested=True, required=False, allow_null=True)
     vlan_translation_policy = VLANTranslationPolicySerializer(nested=True, required=False, allow_null=True)
     vrf = VRFSerializer(nested=True, required=False, allow_null=True)
     vrf = VRFSerializer(nested=True, required=False, allow_null=True)
     l2vpn_termination = L2VPNTerminationSerializer(nested=True, read_only=True, allow_null=True)
     l2vpn_termination = L2VPNTerminationSerializer(nested=True, read_only=True, allow_null=True)
-    count_ipaddresses = serializers.IntegerField(read_only=True)
-    count_fhrp_groups = serializers.IntegerField(read_only=True)
+    # Annotated under private names, since the count properties they fall back to are read-only
+    count_ipaddresses = RelatedObjectCountField(
+        'ip_addresses',
+        source='_count_ipaddresses',
+        fallback_source='count_ipaddresses'
+    )
+    count_fhrp_groups = RelatedObjectCountField(
+        'fhrp_group_assignments',
+        source='_count_fhrp_groups',
+        fallback_source='count_fhrp_groups'
+    )
     # Maintains backward compatibility with NetBox <v4.2; also accepts a MAC string on write to
     # Maintains backward compatibility with NetBox <v4.2; also accepts a MAC string on write to
     # create/update the primary MAC address in a single request.
     # create/update the primary MAC address in a single request.
     mac_address = serializers.CharField(allow_null=True, required=False)
     mac_address = serializers.CharField(allow_null=True, required=False)

+ 6 - 2
netbox/virtualization/api/views.py

@@ -2,6 +2,7 @@ from django.db.models import Sum
 from rest_framework.routers import APIRootView
 from rest_framework.routers import APIRootView
 
 
 from extras.api.mixins import ConfigContextQuerySetMixin, RenderConfigMixin
 from extras.api.mixins import ConfigContextQuerySetMixin, RenderConfigMixin
+from netbox.api.pagination import StripCountAnnotationsPaginator
 from netbox.api.viewsets import NetBoxModelViewSet
 from netbox.api.viewsets import NetBoxModelViewSet
 from utilities.query_functions import CollateAsChar
 from utilities.query_functions import CollateAsChar
 from virtualization import filtersets
 from virtualization import filtersets
@@ -71,11 +72,14 @@ class VirtualMachineViewSet(ConfigContextQuerySetMixin, RenderConfigMixin, NetBo
 class VMInterfaceViewSet(NetBoxModelViewSet):
 class VMInterfaceViewSet(NetBoxModelViewSet):
     queryset = VMInterface.objects.prefetch_related(
     queryset = VMInterface.objects.prefetch_related(
         'l2vpn_terminations',  # Referenced by VMInterfaceSerializer.l2vpn_termination
         'l2vpn_terminations',  # Referenced by VMInterfaceSerializer.l2vpn_termination
-        'ip_addresses',  # Referenced by VMInterface.count_ipaddresses()
-        'fhrp_group_assignments',  # Referenced by VMInterface.count_fhrp_groups()
     )
     )
     serializer_class = serializers.VMInterfaceSerializer
     serializer_class = serializers.VMInterfaceSerializer
     filterset_class = filtersets.VMInterfaceFilterSet
     filterset_class = filtersets.VMInterfaceFilterSet
+    pagination_class = StripCountAnnotationsPaginator
+    field_prefetches = {
+        ('count_ipaddresses',): ('ip_addresses',),
+        ('count_fhrp_groups',): ('fhrp_group_assignments',),
+    }
 
 
     def get_bulk_destroy_queryset(self):
     def get_bulk_destroy_queryset(self):
         # Ensure child interfaces are deleted prior to their parents
         # Ensure child interfaces are deleted prior to their parents

+ 1 - 1
netbox/virtualization/tests/query_counts.json

@@ -11,6 +11,6 @@
   "virtualmachine:list_objects_with_permission": 20,
   "virtualmachine:list_objects_with_permission": 20,
   "virtualmachinetype:api_list_objects": 13,
   "virtualmachinetype:api_list_objects": 13,
   "virtualmachinetype:list_objects_with_permission": 17,
   "virtualmachinetype:list_objects_with_permission": 17,
-  "vminterface:api_list_objects": 18,
+  "vminterface:api_list_objects": 16,
   "vminterface:list_objects_with_permission": 17
   "vminterface:list_objects_with_permission": 17
 }
 }

+ 71 - 4
netbox/virtualization/tests/test_api.py

@@ -13,11 +13,11 @@ from rest_framework import status
 
 
 from core.models import ObjectType
 from core.models import ObjectType
 from dcim.choices import InterfaceModeChoices
 from dcim.choices import InterfaceModeChoices
-from dcim.models import Platform, Site
+from dcim.models import Interface, Platform, Site
 from extras.choices import CustomFieldTypeChoices
 from extras.choices import CustomFieldTypeChoices
-from extras.models import ConfigContext, ConfigTemplate, CustomField
-from ipam.choices import VLANQinQRoleChoices
-from ipam.models import VLAN, VRF, IPAddress, Prefix
+from extras.models import ConfigContext, ConfigTemplate, CustomField, ExportTemplate
+from ipam.choices import FHRPGroupProtocolChoices, VLANQinQRoleChoices
+from ipam.models import VLAN, VRF, FHRPGroup, FHRPGroupAssignment, IPAddress, Prefix
 from users.constants import TOKEN_PREFIX
 from users.constants import TOKEN_PREFIX
 from users.models import Token
 from users.models import Token
 from utilities.testing import (
 from utilities.testing import (
@@ -1062,6 +1062,73 @@ class VMInterfaceTestCase(APIViewTestCases.APIViewTestCase):
         response = self.client.post(self._get_list_url(), data, format='json', **self.header)
         response = self.client.post(self._get_list_url(), data, format='json', **self.header)
         self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
         self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
 
 
+    def test_count_ipaddresses_and_fhrp_groups(self):
+        """Counts come from annotations of the VM interface's object type, never from loaded related objects."""
+        self.add_permissions('virtualization.view_vminterface')
+        vm_interface = VMInterface.objects.get(name='Interface 1')
+        group = FHRPGroup.objects.create(
+            pk=vm_interface.pk, protocol=FHRPGroupProtocolChoices.PROTOCOL_VRRP2, group_id=1
+        )
+        other_group = FHRPGroup.objects.create(
+            pk=vm_interface.pk + 1, protocol=FHRPGroupProtocolChoices.PROTOCOL_VRRP2, group_id=2
+        )
+        interface = Interface.objects.create(
+            pk=vm_interface.pk, device=create_test_device('Device 1'), name='eth0', type='1000base-t'
+        )
+        IPAddress.objects.bulk_create((
+            IPAddress(address='192.0.2.1/24', assigned_object=vm_interface),
+            IPAddress(address='2001:db8::1/64', assigned_object=vm_interface),
+            IPAddress(address='192.0.2.2/24', assigned_object=interface),
+            IPAddress(address='192.0.2.254/24', assigned_object=group),
+        ))
+        FHRPGroupAssignment.objects.bulk_create((
+            FHRPGroupAssignment(group=group, interface=vm_interface, priority=100),
+            # Two on the dcim interface, so a count of the wrong object type cannot match
+            FHRPGroupAssignment(group=group, interface=interface, priority=100),
+            FHRPGroupAssignment(group=other_group, interface=interface, priority=100),
+        ))
+
+        with CaptureQueriesContext(connection) as queries:
+            response = self.client.get(self._get_list_url(), **self.header)
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        counts = {
+            row['name']: (row['count_ipaddresses'], row['count_fhrp_groups']) for row in response.data['results']
+        }
+        self.assertEqual(counts, {'Interface 1': (2, 1), 'Interface 2': (0, 0), 'Interface 3': (0, 0)})
+        # Only the VM interface statement reads the related tables, through its two count subqueries
+        statements = [
+            query['sql'] for query in queries.captured_queries
+            if '"ipam_ipaddress"' in query['sql'] or '"ipam_fhrpgroupassignment"' in query['sql']
+        ]
+        self.assertEqual(len(statements), 1)
+        self.assertTrue(statements[0].startswith('SELECT "virtualization_vminterface"'))
+        # Unannotated instances count through the relations
+        self.assertEqual((vm_interface.count_ipaddresses, vm_interface.count_fhrp_groups), (2, 1))
+
+    def test_export_template_prefetches_related_objects(self):
+        """An API export renders model instances, so it prefetches each counted relation once."""
+        self.add_permissions('virtualization.view_vminterface', 'extras.view_exporttemplate')
+        vm_interface = VMInterface.objects.get(name='Interface 1')
+        IPAddress.objects.create(address='192.0.2.1/24', assigned_object=vm_interface)
+        group = FHRPGroup.objects.create(protocol=FHRPGroupProtocolChoices.PROTOCOL_VRRP2, group_id=1)
+        FHRPGroupAssignment.objects.create(group=group, interface=vm_interface, priority=100)
+        export_template = ExportTemplate.objects.create(
+            name='VM Interface Relations',
+            template_code=(
+                '{% for interface in queryset %}{{ interface.name }}={{ interface.ip_addresses.all()|length }}/'
+                '{{ interface.fhrp_group_assignments.all()|length }},{% endfor %}'
+            ),
+        )
+        export_template.object_types.set([ObjectType.objects.get_for_model(VMInterface)])
+
+        with CaptureQueriesContext(connection) as queries:
+            response = self.client.get(self._get_list_url(), {'export': export_template.name}, **self.header)
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        self.assertEqual(response.content.decode(), 'Interface 1=1/1,Interface 2=0/0,Interface 3=0/0,')
+        for table in ('ipam_ipaddress', 'ipam_fhrpgroupassignment'):
+            statements = [query['sql'] for query in queries.captured_queries if f'FROM "{table}"' in query['sql']]
+            self.assertEqual(len(statements), 1, table)
+
 
 
 class VirtualDiskTestCase(APIViewTestCases.APIViewTestCase):
 class VirtualDiskTestCase(APIViewTestCases.APIViewTestCase):
     model = VirtualDisk
     model = VirtualDisk