Closes #22989: Reference brief components for nested SerializedPKRelatedField

[jeremystretch]

Closes: #22989

Problem

SerializedPKRelatedField.to_representation() renders related objects using the field's nested setting:

return self.serializer(value, nested=self.nested, context={'request': self.context['request']}).data

The drf-spectacular extension in core/api/schema.py instead handed the serializer class to resolve_serializer(). drf-spectacular calls force_instance() on it, instantiating with no kwargs, so nested fell back to False. As a result both NetBoxAutoSchema._get_serializer_name() (which applies the Brief prefix) and _map_serializer() (which prunes to Meta.brief_fields) saw a non-nested serializer, and the schema advertised fields the API never returns.

For example, Site.asns documented #/components/schemas/ASN (18 fields) while the API returns the brief representation (5 fields). 31 fields across the schema were affected.

Fix

FixSerializedPKRelatedField.map_serializer_field() now resolves a serializer instance carrying the field's nested setting, mirroring what to_representation() does:

serializer = self.target.serializer(nested=self.target.nested)
component = auto_schema.resolve_serializer(serializer, direction)

The request branch is untouched, so request bodies continue to accept an array of integer primary keys.

Notes:

• many=True needs no special handling: drf-spectacular checks the extension registry before unwrapping ManyRelatedField, and RelatedField.many_init() passes nested through to the child_relation, so self.target is the SerializedPKRelatedField itself with .nested intact.
• Fields declared without nested (e.g. VRF.import_targets) instantiate with nested=False, which is identical to what force_instance() did before — no change for them.
• A serializer that doesn't accept a nested kwarg is deliberately not defended against here. to_representation() passes nested= unconditionally, so such a serializer already raises TypeError on every read of the field; catching it in the schema layer would only document a component for a field the API cannot serve.

Avoiding gratuitous component renames

Three serializers — ASNSiteSerializer, NestedGroupSerializer, NestedUserSerializer — are used only in a nested context, and their brief and complete field sets are identical (the two Nested* serializers declare no brief_fields at all). Prefixing them with Brief would therefore have renamed an existing component to no purpose and dropped ASNSite, NestedGroup and NestedUser from the schema entirely.

_get_serializer_name() now exempts serializers which declare an explicit Meta.ref_name from the prefix, and those three names are pinned. drf-spectacular already honours Meta.ref_name; NetBox's own get_serializer_ref_name() is unrelated (it only feeds the Writable prefix). A serializer that legitimately needs both a complete and a brief form must not declare ref_name.

Schema impact

No components are removed and none are renamed. Nine new Brief* components are added, which is purely additive:

BriefASN, BriefContactGroup, BriefGroup, BriefIKEProposal, BriefIPSecProposal, BriefObjectPermission, BriefRouteTarget, BriefVirtualDeviceContext, BriefWirelessLAN

31 fields change their $ref to the brief component they actually return — ConfigContext (12 fields), Interface.tagged_vlans / vdcs / wireless_lans, Site.asns, Provider.asns, User.groups / permissions, L2VPN.import_targets / export_targets, and others. This is the fix itself and cannot be avoided: a client generated from the old schema carried a type there which the API never actually produced. SDKs regenerated after this change will differ in those fields, so it still warrants a note when the release notes are assembled.

contrib/openapi.json is not regenerated here, as it is refreshed as part of the release process. Note that scripts/verify-openapi.sh will therefore report a mismatch if run locally before then; it is not wired into any CI workflow.

Testing

core/tests/test_openapi_schema.py gains coverage at two levels.

Against the generated schema (OpenAPISchemaTestCase):

• Nested fields reference the brief component — Site.asns → BriefASN, ConfigContext.sites → BriefSite, Interface.tagged_vlans → BriefVLAN — and BriefASN advertises only ASNSerializer.Meta.brief_fields.
• Serializers pinned with Meta.ref_name keep their name: ASN.sites → ASNSite, ObjectPermission.groups → NestedGroup, ObjectPermission.users → NestedUser, with no Brief* counterpart generated.
• Non-nested control: VRF.import_targets / export_targets still reference the complete RouteTarget.
• Request schemas still document an array of integer PKs (SiteRequest.asns, ConfigContextRequest.sites, ASNRequest.sites).

Against the extension directly (SerializedPKRelatedFieldSchemaTestCase, a SimpleTestCase — it touches no database):

• The serializer is instantiated with the field's nested setting, for both True and False, and the resolved component's ref is what gets returned.
• The request direction returns an integer type without resolving a serializer at all.

All of the schema-level assertions were verified to fail against the unpatched extension.

This class also now generates the OpenAPI schema once in setUpClass() rather than once per test method — it is one of the more expensive operations in the suite, and its output is immutable across these tests. Measured on OpenAPISchemaTestCase alone: 0.700s → 0.277s.

🤖 Generated with Claude Code

[jeremystretch]

@claude /review Only report on issues of potential concern; we don't need commentary on anything that doesn't require further consideration.

[github-actions]

Claude finished @jeremystretch's task in 4m 24s —— View job

---

Review: #23071 — Reference brief components for nested SerializedPKRelatedField

• Gather PR diff and context
• Review netbox/core/api/schema.py
• Review added tests
• Post review

The core fix is correct. RelatedField.many_init() does pass nested through to the child_relation, and drf-spectacular's _map_serializer_field() checks the extension registry before unwrapping ManyRelatedField, so self.target is the SerializedPKRelatedField with .nested intact — the many=True reasoning in the description holds. I spot-checked the fixtures the new tests rely on (ASNSerializer.sites/ASNSiteSerializer are nested=True, VRFSerializer.import_targets/export_targets are not, ASNSerializer.Meta.brief_fields == ('id', 'url', 'display', 'asn', 'description')) and they match the assertions.

I was not able to execute the test suite in this environment (no Postgres/Redis, manage.py test not permitted), so the findings below are from static analysis only.

---

Issues of potential concern

1. The non-BaseModelSerializer fallback documents a configuration that cannot work at runtime — netbox/core/api/schema.py:399-401

else:
    serializer = serializer()

The comment justifies this as supporting "a plain ModelSerializer employed by a plugin," but SerializedPKRelatedField.to_representation() (netbox/netbox/api/fields.py:149) passes nested= unconditionally:

return self.serializer(value, nested=self.nested, context={'request': self.context['request']}).data

A plugin using a serializer that doesn't accept nested gets a TypeError on every read of that field — it never reaches a working response. So this branch (and test_serializer_without_nested_support, which locks it in) describes a scenario the API cannot serve; the schema would advertise a full component for a field that 500s. Either drop the branch and let map_serializer_field() surface the same failure the runtime has, or, if the intent is to genuinely support such plugins, the fix belongs in to_representation() too.

2. issubclass(..., BaseModelSerializer) is a narrower gate than "accepts nested" — netbox/core/api/schema.py:397

A third-party serializer that isn't a BaseModelSerializer but does accept nested (and honours it in get_fields()) falls into the else branch and is instantiated non-nested — the schema then advertises the full field set while the API returns the brief one, which is exactly the bug this PR fixes, just relocated. Since inspect is already imported, keying on the constructor signature would cover both cases in one branch:

if inspect.isclass(serializer):
    if 'nested' in inspect.signature(serializer).parameters:
        serializer = serializer(nested=self.target.nested)
    else:
        serializer = serializer()

Fix this →

3. OpenAPISchemaTestCase.setUp() regenerates the entire OpenAPI schema per test method

setUp() issues GET /api/schema/, which is one of the most expensive operations in the suite. This PR takes that class from 3 tests to 6, doubling the cost. The schema is immutable across these tests, so it should be generated once in setUpClass() and stored on the class. This is pre-existing, but the PR is what makes it expensive enough to matter.

Fix this →

4. contrib/openapi.json is now out of sync with generated output

scripts/verify-openapi.sh exists specifically to assert those two match and will fail after this merges. It isn't wired into any workflow in .github/workflows/, so CI won't break — but anyone running it locally will hit a false alarm until the release-time regeneration in docs/development/release-checklist.md:179. Worth confirming that's the intended trade-off rather than an oversight.

5. Dropped components are a breaking change for generated clients

ASNSite, NestedGroup, and NestedUser vanish from components/schemas, and NestedUser/NestedGroup effectively rename to BriefNestedUser/BriefNestedGroup. Consumers generating SDKs from the published schema will break on upgrade. The PR body covers this, but it should get an explicit breaking-change note when release notes are assembled — not something a reader of the diff would infer.

---

Minor

• SerializedPKRelatedFieldSchemaTestCase extends django.test.TestCase, incurring a per-test transaction, but nothing in it touches the database (Site.objects.all() is never evaluated, and ModelSerializer field construction reads metadata only). SimpleTestCase would suffice.
• DummyAutoSchema.resolve_serializer() returns None, so map_serializer_field() returns None in both new unit tests. That's fine for asserting which instance was resolved, but it means the component.ref return path is never exercised — a regression there would only be caught by the integration-level assertions in OpenAPISchemaTestCase.
  • Branch: 22989-nested-schema-components

[jeremystretch]

1 — Non-BaseModelSerializer fallback (dropped). The review is right: SerializedPKRelatedField.to_representation() (netbox/netbox/api/fields.py:149) passes nested= unconditionally, so a serializer that can't accept it raises TypeError on every read of that field. The else branch was documenting a component for a configuration the API can never serve. Removed it, along with test_serializer_without_nested_support and its PlainSerializer fixture. map_serializer_field() is back to the plain form:

serializer = self.target.serializer(nested=self.target.nested)

That also removes the need for the import inspect and BaseModelSerializer imports this branch had added — schema.py now matches main on both import lines. This moots item 2, since there's no longer a gate to widen.

3 — Per-test schema generation. Moved into setUpClass() (not setUpTestData(), which wraps class attributes in a deepcopy-per-test descriptor — expensive for a structure this size). Measured on OpenAPISchemaTestCase alone: 0.700s → 0.277s for the 6 tests.

5 — Breaking change. Renamed the PR section to "Schema churn (breaking change for generated clients)" with a bolded call-out that it warrants an explicit breaking-change note when release notes are assembled. I also folded in the item-4 detail as a factual note (scripts/verify-openapi.sh will report a mismatch locally until release-time regeneration; it's not wired into CI) — you said to ignore that item, so it's recorded rather than acted on.

[bctiemann]

Some reservations about the Brief-prefixed serializer names being programmatically derived, since that makes them more opaque and more difficult to debug; but probably not really a concern under modern circumstances.