diff --git a/netbox/ipam/graphql/types.py b/netbox/ipam/graphql/types.py index f17c9c72e..b684022da 100644 --- a/netbox/ipam/graphql/types.py +++ b/netbox/ipam/graphql/types.py @@ -51,23 +51,16 @@ __all__ = ( @strawberry.type class IPAddressFamilyType: + """ + 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. + """ value: int label: str -@strawberry.type -class BaseIPAddressFamilyType: - """ - Base type for models that need to expose their IPAddress family type. - """ - - @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}') - - @strawberry_django.type( models.ASN, fields='__all__', @@ -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')] diff --git a/netbox/netbox/tests/test_graphql.py b/netbox/netbox/tests/test_graphql.py index a0641798d..5fe5b8468 100644 --- a/netbox/netbox/tests/test_graphql.py +++ b/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