Преглед изворни кода

Closes #22823: Avoid extraneous DB queries when fetching IP/prefix family via GraphQL API

Jeremy Stretch пре 1 дан
родитељ
комит
aefe938c63
2 измењених фајлова са 87 додато и 17 уклоњено
  1. 24 17
      netbox/ipam/graphql/types.py
  2. 63 0
      netbox/netbox/tests/test_graphql.py

+ 24 - 17
netbox/ipam/graphql/types.py

@@ -51,21 +51,14 @@ __all__ = (
 
 @strawberry.type
 class IPAddressFamilyType:
-    value: int
-    label: str
-
-
-@strawberry.type
-class BaseIPAddressFamilyType:
     """
-    Base type for models that need to expose their IPAddress family type.
+    The address family (4 or 6) of a model's IP address or prefix column. Each type exposing this
+    declares its own `family` resolver, hinted with the column backing it so that the query optimizer
+    does not defer that column. `value` is non-null because those columns are: the models' `family`
+    properties return None only for an unsaved instance with no address assigned.
     """
-
-    @strawberry.field
-    def family(self) -> IPAddressFamilyType:
-        # Note that self, is an instance of models.IPAddress
-        # thus resolves to the address family value.
-        return IPAddressFamilyType(value=self.family, label=f'IPv{self.family}')
+    value: int
+    label: str
 
 
 @strawberry_django.type(
@@ -103,11 +96,16 @@ class ASNRangeType(OrganizationalObjectType):
     filters=AggregateFilter,
     pagination=True
 )
-class AggregateType(ContactsMixin, BaseIPAddressFamilyType, PrimaryObjectType):
+class AggregateType(ContactsMixin, PrimaryObjectType):
     prefix: str
     rir: Annotated["RIRType", strawberry.lazy('ipam.graphql.types')] | None
     tenant: Annotated["TenantType", strawberry.lazy('tenancy.graphql.types')] | None
 
+    # Note that self is the Django model instance, so self.family resolves to the model's family property
+    @strawberry_django.field(only=['prefix'])
+    def family(self) -> IPAddressFamilyType:
+        return IPAddressFamilyType(value=self.family, label=f'IPv{self.family}')
+
 
 @strawberry_django.type(
     models.FHRPGroup,
@@ -143,16 +141,20 @@ class FHRPGroupAssignmentType(BaseObjectType):
     filters=IPAddressFilter,
     pagination=True
 )
-class IPAddressType(ContactsMixin, BaseIPAddressFamilyType, PrimaryObjectType):
+class IPAddressType(ContactsMixin, PrimaryObjectType):
     address: str
     vrf: Annotated['VRFType', strawberry.lazy('ipam.graphql.types')] | None
     tenant: Annotated['TenantType', strawberry.lazy('tenancy.graphql.types')] | None
     nat_inside: Annotated['IPAddressType', strawberry.lazy('ipam.graphql.types')] | None
-
     nat_outside: list[Annotated['IPAddressType', strawberry.lazy('ipam.graphql.types')]]
     tunnel_terminations: list[Annotated['TunnelTerminationType', strawberry.lazy('vpn.graphql.types')]]
     services: list[Annotated['ServiceType', strawberry.lazy('ipam.graphql.types')]]
 
+    # Note that self is the Django model instance, so self.family resolves to the model's family property
+    @strawberry_django.field(only=['address'])
+    def family(self) -> IPAddressFamilyType:
+        return IPAddressFamilyType(value=self.family, label=f'IPv{self.family}')
+
     @strawberry_django.field(prefetch_related='assigned_object')
     def assigned_object(self) -> Annotated[
         Annotated['InterfaceType', strawberry.lazy('dcim.graphql.types')]
@@ -183,13 +185,18 @@ class IPRangeType(ContactsMixin, PrimaryObjectType):
     filters=PrefixFilter,
     pagination=True
 )
-class PrefixType(ContactsMixin, BaseIPAddressFamilyType, PrimaryObjectType):
+class PrefixType(ContactsMixin, PrimaryObjectType):
     prefix: str
     vrf: Annotated['VRFType', strawberry.lazy('ipam.graphql.types')] | None
     tenant: Annotated['TenantType', strawberry.lazy('tenancy.graphql.types')] | None
     vlan: Annotated['VLANType', strawberry.lazy('ipam.graphql.types')] | None
     role: Annotated['RoleType', strawberry.lazy('ipam.graphql.types')] | None
 
+    # Note that self is the Django model instance, so self.family resolves to the model's family property
+    @strawberry_django.field(only=['prefix'])
+    def family(self) -> IPAddressFamilyType:
+        return IPAddressFamilyType(value=self.family, label=f'IPv{self.family}')
+
     @strawberry_django.field(prefetch_related='scope')
     def scope(self) -> Annotated[
         Annotated['LocationType', strawberry.lazy('dcim.graphql.types')]

+ 63 - 0
netbox/netbox/tests/test_graphql.py

@@ -27,6 +27,7 @@ from dcim.models import (
 )
 from extras.choices import CustomFieldTypeChoices
 from extras.models import CustomField, TableConfig, Tag
+from ipam.models import RIR, Aggregate, IPAddress, Prefix
 from netbox.graphql.scalars import BigInt, BigIntScalar
 from netbox.graphql.schema import Query, get_schema_extensions, schema
 from users.models import Token, User
@@ -694,6 +695,17 @@ class GraphQLDeferredColumnTestCase(APITestCase):
             reservation.pk: len(reservation.units)
             for reservation in RackReservation.objects.bulk_create(reservations)
         }
+        # IPAM objects, for the `family` field of each type which exposes one
+        IPAddress.objects.bulk_create([
+            IPAddress(address=f'10.0.0.{i + 1}/24') for i in range(cls.OBJECT_COUNT)
+        ])
+        Prefix.objects.bulk_create([
+            Prefix(prefix=f'10.{i}.0.0/16') for i in range(cls.OBJECT_COUNT)
+        ])
+        rir = RIR.objects.create(name='RIR 1', slug='rir-1')
+        Aggregate.objects.bulk_create([
+            Aggregate(prefix=f'{i + 20}.0.0.0/8', rir=rir) for i in range(cls.OBJECT_COUNT)
+        ])
 
     def _execute(self, query):
         url = reverse('graphql')
@@ -782,6 +794,57 @@ class GraphQLDeferredColumnTestCase(APITestCase):
 
         self.assertNoDeferredColumnReloads(query, 'rack_reservation_list', 'dcim_rackreservation', validate)
 
+    def test_ip_address_family(self):
+        """
+        Regression test for #22823: IPAddressType.family must not defer `address`.
+        """
+        self.add_permissions('ipam.view_ipaddress')
+        query = """
+        {
+            ip_address_list(pagination: {limit: %(limit)s}) {
+                id
+                family { value label }
+            }
+        }
+        """
+        self.assertNoDeferredColumnReloads(
+            query, 'ip_address_list', 'ipam_ipaddress', self._validate_ipv4_family
+        )
+
+    def test_prefix_family(self):
+        """
+        Regression test for #22823: PrefixType.family must not defer `prefix`.
+        """
+        self.add_permissions('ipam.view_prefix')
+        query = """
+        {
+            prefix_list(pagination: {limit: %(limit)s}) {
+                id
+                family { value label }
+            }
+        }
+        """
+        self.assertNoDeferredColumnReloads(query, 'prefix_list', 'ipam_prefix', self._validate_ipv4_family)
+
+    def test_aggregate_family(self):
+        """
+        Regression test for #22823: AggregateType.family must not defer `prefix`.
+        """
+        self.add_permissions('ipam.view_aggregate')
+        query = """
+        {
+            aggregate_list(pagination: {limit: %(limit)s}) {
+                id
+                family { value label }
+            }
+        }
+        """
+        self.assertNoDeferredColumnReloads(query, 'aggregate_list', 'ipam_aggregate', self._validate_ipv4_family)
+
+    def _validate_ipv4_family(self, objects):
+        for obj in objects:
+            self.assertEqual(obj['family'], {'value': 4, 'label': 'IPv4'})
+
 
 class GraphQLSchemaCoverageTestCase(APIViewTestCases.GraphQLSchemaCoverageTestCase):
     pass