From 289392866217b23c6a857ad6d73778756ff9e621 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 28 Aug 2026 14:56:23 -0400 Subject: [PATCH 1/3] Closes #22989: Reference brief components for nested SerializedPKRelatedField FixSerializedPKRelatedField passed the serializer class to resolve_serializer(), which instantiates it with no arguments. The field's nested setting was therefore lost, and the generated response schema referenced the complete component (with the complete field set) even where the field renders a brief representation. Resolve an instance carrying the field's nested setting instead. Request schemas are unaffected and continue to accept integer primary keys. Co-Authored-By: Claude Opus 5 --- netbox/core/api/schema.py | 15 ++- netbox/core/tests/test_openapi_schema.py | 114 +++++++++++++++++++++++ 2 files changed, 127 insertions(+), 2 deletions(-) diff --git a/netbox/core/api/schema.py b/netbox/core/api/schema.py index 4c63296d3..90d8fb6c1 100644 --- a/netbox/core/api/schema.py +++ b/netbox/core/api/schema.py @@ -1,3 +1,4 @@ +import inspect import re import typing from collections import OrderedDict @@ -17,7 +18,7 @@ from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import Direction, OpenApiParameter from netbox.api.fields import ChoiceField -from netbox.api.serializers import WritableNestedSerializer +from netbox.api.serializers import BaseModelSerializer, WritableNestedSerializer from netbox.api.viewsets import NetBoxModelViewSet # see netbox.api.routers.NetBoxRouter @@ -389,7 +390,17 @@ class FixSerializedPKRelatedField(OpenApiSerializerFieldExtension): def map_serializer_field(self, auto_schema, direction): if direction == "response": - component = auto_schema.resolve_serializer(self.target.serializer, direction) + # Resolve an instance of the serializer carrying the field's nested setting, so that the brief + # component is referenced wherever the field renders a brief representation. Serializers which + # don't derive from BaseModelSerializer (e.g. a plain ModelSerializer employed by a plugin) don't + # accept the nested kwarg, so instantiate those without it. + serializer = self.target.serializer + if inspect.isclass(serializer): + if issubclass(serializer, BaseModelSerializer): + serializer = serializer(nested=self.target.nested) + else: + serializer = serializer() + component = auto_schema.resolve_serializer(serializer, direction) return component.ref if component else None return build_basic_type(OpenApiTypes.INT) diff --git a/netbox/core/tests/test_openapi_schema.py b/netbox/core/tests/test_openapi_schema.py index 8158b87ec..0d0f78b18 100644 --- a/netbox/core/tests/test_openapi_schema.py +++ b/netbox/core/tests/test_openapi_schema.py @@ -6,6 +6,12 @@ Refs: #20638 import json from django.test import TestCase +from rest_framework import serializers + +from core.api.schema import FixSerializedPKRelatedField +from dcim.api.serializers import SiteSerializer +from dcim.models import Site +from netbox.api.fields import SerializedPKRelatedField class OpenAPISchemaTestCase(TestCase): @@ -107,3 +113,111 @@ class OpenAPISchemaTestCase(TestCase): self.assertNotIn('oneOf', request_schema, "DELETE should NOT have oneOf") self.assertEqual(request_schema['type'], 'array', "DELETE should require array") self.assertIn('items', request_schema, "DELETE array should have items") + + def test_nested_related_fields_reference_brief_components(self): + """ + A SerializedPKRelatedField declared with nested=True must reference the brief component in + response schemas, as that is what the API returns. + + Refs: #22989 + """ + components = self.schema['components']['schemas'] + + for component, field, ref in ( + ('Site', 'asns', 'BriefASN'), + ('ConfigContext', 'sites', 'BriefSite'), + ('ASN', 'sites', 'BriefASNSite'), + ): + with self.subTest(component=component, field=field): + self.assertEqual( + components[component]['properties'][field]['items']['$ref'], + f'#/components/schemas/{ref}' + ) + + # The brief component must advertise only the serializer's brief fields + self.assertEqual( + set(components['BriefASN']['properties']), + {'id', 'url', 'display', 'asn', 'description'} + ) + + def test_non_nested_related_fields_reference_full_components(self): + """ + A SerializedPKRelatedField declared without nested=True must continue to reference the + complete component. + + Refs: #22989 + """ + components = self.schema['components']['schemas'] + + for field in ('import_targets', 'export_targets'): + with self.subTest(field=field): + self.assertEqual( + components['VRF']['properties'][field]['items']['$ref'], + '#/components/schemas/RouteTarget' + ) + + def test_nested_related_fields_accept_pks_on_write(self): + """ + Request schemas for a SerializedPKRelatedField must continue to accept an array of integer + primary keys. + + Refs: #22989 + """ + components = self.schema['components']['schemas'] + + for component, field in ( + ('SiteRequest', 'asns'), + ('ConfigContextRequest', 'sites'), + ('ASNRequest', 'sites'), + ): + with self.subTest(component=component, field=field): + self.assertEqual(components[component]['properties'][field]['items']['type'], 'integer') + + +class SerializedPKRelatedFieldSchemaTestCase(TestCase): + """Tests for the schema extension which maps SerializedPKRelatedField.""" + + class PlainSerializer(serializers.ModelSerializer): + """A serializer which does not derive from BaseModelSerializer, as a plugin might employ.""" + + class Meta: + model = Site + fields = ('id', 'name') + + def resolve_serializer(self, field): + """Invoke the schema extension for a field, returning the serializer instance it resolved.""" + resolved = [] + + class DummyAutoSchema: + def resolve_serializer(self, serializer, direction): + resolved.append(serializer) + + FixSerializedPKRelatedField(field).map_serializer_field(DummyAutoSchema(), 'response') + return resolved[0] + + def test_nested_flag_is_passed_to_netbox_serializers(self): + """ + A serializer derived from BaseModelSerializer must be instantiated with the field's nested setting. + + Refs: #22989 + """ + for nested in (True, False): + with self.subTest(nested=nested): + field = SerializedPKRelatedField( + serializer=SiteSerializer, + queryset=Site.objects.all(), + nested=nested + ) + serializer = self.resolve_serializer(field) + self.assertIsInstance(serializer, SiteSerializer) + self.assertEqual(serializer.nested, nested) + + def test_serializer_without_nested_support(self): + """ + A serializer which does not accept the nested kwarg must still resolve, rather than breaking + generation of the entire schema. + + Refs: #22989 + """ + field = SerializedPKRelatedField(serializer=self.PlainSerializer, queryset=Site.objects.all()) + self.assertIsInstance(self.resolve_serializer(field), self.PlainSerializer) From 9a37694d2264abd6f34568a5ab060f59864ac477 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 28 Aug 2026 15:36:42 -0400 Subject: [PATCH 2/3] Address review feedback on #22989 * Drop the non-BaseModelSerializer fallback in FixSerializedPKRelatedField. SerializedPKRelatedField.to_representation() passes nested unconditionally, so a serializer which doesn't accept it raises TypeError on every read; the branch documented a component for a configuration the API cannot serve. * Generate the OpenAPI schema once per class rather than once per test method. * Exercise the component.ref and request-schema return paths, and use SimpleTestCase for the tests which don't touch the database. Co-Authored-By: Claude Opus 5 --- netbox/core/api/schema.py | 15 ++--- netbox/core/tests/test_openapi_schema.py | 70 ++++++++++++++---------- 2 files changed, 44 insertions(+), 41 deletions(-) diff --git a/netbox/core/api/schema.py b/netbox/core/api/schema.py index 90d8fb6c1..57dc842b7 100644 --- a/netbox/core/api/schema.py +++ b/netbox/core/api/schema.py @@ -1,4 +1,3 @@ -import inspect import re import typing from collections import OrderedDict @@ -18,7 +17,7 @@ from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import Direction, OpenApiParameter from netbox.api.fields import ChoiceField -from netbox.api.serializers import BaseModelSerializer, WritableNestedSerializer +from netbox.api.serializers import WritableNestedSerializer from netbox.api.viewsets import NetBoxModelViewSet # see netbox.api.routers.NetBoxRouter @@ -391,15 +390,9 @@ class FixSerializedPKRelatedField(OpenApiSerializerFieldExtension): def map_serializer_field(self, auto_schema, direction): if direction == "response": # Resolve an instance of the serializer carrying the field's nested setting, so that the brief - # component is referenced wherever the field renders a brief representation. Serializers which - # don't derive from BaseModelSerializer (e.g. a plain ModelSerializer employed by a plugin) don't - # accept the nested kwarg, so instantiate those without it. - serializer = self.target.serializer - if inspect.isclass(serializer): - if issubclass(serializer, BaseModelSerializer): - serializer = serializer(nested=self.target.nested) - else: - serializer = serializer() + # component is referenced wherever the field renders a brief representation. (The field's + # to_representation() passes nested in the same manner.) See #22989. + serializer = self.target.serializer(nested=self.target.nested) component = auto_schema.resolve_serializer(serializer, direction) return component.ref if component else None return build_basic_type(OpenApiTypes.INT) diff --git a/netbox/core/tests/test_openapi_schema.py b/netbox/core/tests/test_openapi_schema.py index 0d0f78b18..00da4ae3c 100644 --- a/netbox/core/tests/test_openapi_schema.py +++ b/netbox/core/tests/test_openapi_schema.py @@ -5,8 +5,7 @@ Refs: #20638 """ import json -from django.test import TestCase -from rest_framework import serializers +from django.test import SimpleTestCase, TestCase from core.api.schema import FixSerializedPKRelatedField from dcim.api.serializers import SiteSerializer @@ -17,11 +16,17 @@ from netbox.api.fields import SerializedPKRelatedField class OpenAPISchemaTestCase(TestCase): """Tests for OpenAPI schema generation.""" - def setUp(self): - """Fetch schema via API endpoint.""" - response = self.client.get('/api/schema/', {'format': 'json'}) - self.assertEqual(response.status_code, 200) - self.schema = json.loads(response.content) + @classmethod + def setUpClass(cls): + """ + Fetch the schema via the API endpoint. Schema generation is expensive and its output is + immutable across these tests, so do this once for the class rather than per test method. + """ + super().setUpClass() + + response = cls.client_class().get('/api/schema/', {'format': 'json'}) + assert response.status_code == 200, f'Failed to generate OpenAPI schema (HTTP {response.status_code})' + cls.schema = json.loads(response.content) def test_post_operation_documents_single_or_array(self): """ @@ -174,30 +179,26 @@ class OpenAPISchemaTestCase(TestCase): self.assertEqual(components[component]['properties'][field]['items']['type'], 'integer') -class SerializedPKRelatedFieldSchemaTestCase(TestCase): +class SerializedPKRelatedFieldSchemaTestCase(SimpleTestCase): """Tests for the schema extension which maps SerializedPKRelatedField.""" - class PlainSerializer(serializers.ModelSerializer): - """A serializer which does not derive from BaseModelSerializer, as a plugin might employ.""" + class DummyComponent: + ref = {'$ref': '#/components/schemas/Dummy'} - class Meta: - model = Site - fields = ('id', 'name') + class DummyAutoSchema: + """Records the serializer resolved by the extension, in place of generating a component.""" - def resolve_serializer(self, field): - """Invoke the schema extension for a field, returning the serializer instance it resolved.""" - resolved = [] + def __init__(self): + self.resolved = [] - class DummyAutoSchema: - def resolve_serializer(self, serializer, direction): - resolved.append(serializer) + def resolve_serializer(self, serializer, direction): + self.resolved.append(serializer) + return SerializedPKRelatedFieldSchemaTestCase.DummyComponent - FixSerializedPKRelatedField(field).map_serializer_field(DummyAutoSchema(), 'response') - return resolved[0] - - def test_nested_flag_is_passed_to_netbox_serializers(self): + def test_nested_flag_is_passed_to_serializer(self): """ - A serializer derived from BaseModelSerializer must be instantiated with the field's nested setting. + The field's serializer must be instantiated with the field's nested setting, so that the + component matching the rendered representation is referenced. Refs: #22989 """ @@ -208,16 +209,25 @@ class SerializedPKRelatedFieldSchemaTestCase(TestCase): queryset=Site.objects.all(), nested=nested ) - serializer = self.resolve_serializer(field) + auto_schema = self.DummyAutoSchema() + + schema = FixSerializedPKRelatedField(field).map_serializer_field(auto_schema, 'response') + + serializer = auto_schema.resolved[0] self.assertIsInstance(serializer, SiteSerializer) self.assertEqual(serializer.nested, nested) + self.assertEqual(schema, self.DummyComponent.ref) - def test_serializer_without_nested_support(self): + def test_request_schema_is_an_integer(self): """ - A serializer which does not accept the nested kwarg must still resolve, rather than breaking - generation of the entire schema. + Request schemas must document an integer primary key, regardless of the nested setting. Refs: #22989 """ - field = SerializedPKRelatedField(serializer=self.PlainSerializer, queryset=Site.objects.all()) - self.assertIsInstance(self.resolve_serializer(field), self.PlainSerializer) + field = SerializedPKRelatedField(serializer=SiteSerializer, queryset=Site.objects.all(), nested=True) + auto_schema = self.DummyAutoSchema() + + schema = FixSerializedPKRelatedField(field).map_serializer_field(auto_schema, 'request') + + self.assertEqual(schema['type'], 'integer') + self.assertEqual(auto_schema.resolved, []) From 443a22706f4ff81c218ff8ef0363e914a8cfa2fd Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 28 Aug 2026 15:55:51 -0400 Subject: [PATCH 3/3] Avoid renaming existing schema components Serializers used only in a nested context have no complete form in the schema, so prefixing them with "Brief" renamed an existing component to no purpose and dropped the old name entirely. Exempt serializers declaring an explicit Meta.ref_name from the prefix, and pin the three affected names. This narrows the schema diff to the fields the bug actually affected: no components are removed, and the nine which are added are purely additive. Co-Authored-By: Claude Opus 5 --- netbox/core/api/schema.py | 6 ++++-- netbox/core/tests/test_openapi_schema.py | 24 +++++++++++++++++++++++- netbox/ipam/api/serializers_/asns.py | 1 + netbox/users/api/serializers_/nested.py | 2 ++ 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/netbox/core/api/schema.py b/netbox/core/api/schema.py index 57dc842b7..939868e40 100644 --- a/netbox/core/api/schema.py +++ b/netbox/core/api/schema.py @@ -215,8 +215,10 @@ class NetBoxAutoSchema(AutoSchema): def _get_serializer_name(self, serializer, direction, bypass_extensions=False) -> str: name = super()._get_serializer_name(serializer, direction, bypass_extensions) - # If this serializer is nested, prepend its name with "Brief" - if getattr(serializer, 'nested', False): + # If this serializer is nested, prepend its name with "Brief". Serializers which declare an explicit + # Meta.ref_name are exempt: those are brief by design and have no complete form in the schema, so the + # prefix would only rename an existing component to no purpose. See #22989. + if getattr(serializer, 'nested', False) and not getattr(getattr(serializer, 'Meta', None), 'ref_name', None): name = f'Brief{name}' return name diff --git a/netbox/core/tests/test_openapi_schema.py b/netbox/core/tests/test_openapi_schema.py index 00da4ae3c..50ac6c970 100644 --- a/netbox/core/tests/test_openapi_schema.py +++ b/netbox/core/tests/test_openapi_schema.py @@ -131,7 +131,7 @@ class OpenAPISchemaTestCase(TestCase): for component, field, ref in ( ('Site', 'asns', 'BriefASN'), ('ConfigContext', 'sites', 'BriefSite'), - ('ASN', 'sites', 'BriefASNSite'), + ('Interface', 'tagged_vlans', 'BriefVLAN'), ): with self.subTest(component=component, field=field): self.assertEqual( @@ -145,6 +145,28 @@ class OpenAPISchemaTestCase(TestCase): {'id', 'url', 'display', 'asn', 'description'} ) + def test_ref_name_exempts_serializer_from_brief_prefix(self): + """ + A serializer which declares an explicit Meta.ref_name keeps that name when nested, rather than + acquiring a Brief prefix. These serializers are brief by design and have no complete form in the + schema, so prefixing them would rename an existing component to no purpose. + + Refs: #22989 + """ + components = self.schema['components']['schemas'] + + for component, field, ref in ( + ('ASN', 'sites', 'ASNSite'), + ('ObjectPermission', 'groups', 'NestedGroup'), + ('ObjectPermission', 'users', 'NestedUser'), + ): + with self.subTest(component=component, field=field): + self.assertEqual( + components[component]['properties'][field]['items']['$ref'], + f'#/components/schemas/{ref}' + ) + self.assertNotIn(f'Brief{ref}', components) + def test_non_nested_related_fields_reference_full_components(self): """ A SerializedPKRelatedField declared without nested=True must continue to reference the diff --git a/netbox/ipam/api/serializers_/asns.py b/netbox/ipam/api/serializers_/asns.py index c5c95bf19..fe7153cf3 100644 --- a/netbox/ipam/api/serializers_/asns.py +++ b/netbox/ipam/api/serializers_/asns.py @@ -54,6 +54,7 @@ class ASNSiteSerializer(PrimaryModelSerializer): model = Site fields = ('id', 'url', 'display', 'name', 'description', 'slug') brief_fields = ('id', 'url', 'display', 'name', 'description', 'slug') + ref_name = 'ASNSite' class ASNSerializer(PrimaryModelSerializer): diff --git a/netbox/users/api/serializers_/nested.py b/netbox/users/api/serializers_/nested.py index b268776b5..a3e9ffaa2 100644 --- a/netbox/users/api/serializers_/nested.py +++ b/netbox/users/api/serializers_/nested.py @@ -15,6 +15,7 @@ class NestedGroupSerializer(WritableNestedSerializer): class Meta: model = models.Group fields = ['id', 'url', 'display_url', 'display', 'name'] + ref_name = 'NestedGroup' class NestedUserSerializer(WritableNestedSerializer): @@ -22,6 +23,7 @@ class NestedUserSerializer(WritableNestedSerializer): class Meta: model = models.User fields = ['id', 'url', 'display_url', 'display', 'username'] + ref_name = 'NestedUser' @extend_schema_field(OpenApiTypes.STR) def get_display(self, obj):