diff --git a/netbox/core/api/schema.py b/netbox/core/api/schema.py index 4c63296d3..eac66b62e 100644 --- a/netbox/core/api/schema.py +++ b/netbox/core/api/schema.py @@ -2,6 +2,7 @@ import re import typing from collections import OrderedDict +from django.utils.translation import gettext_lazy as _ from drf_spectacular.contrib.django_filters import DjangoFilterExtension from drf_spectacular.extensions import OpenApiSerializerExtension, OpenApiSerializerFieldExtension, _SchemaType from drf_spectacular.openapi import AutoSchema @@ -14,10 +15,10 @@ from drf_spectacular.plumbing import ( get_doc, ) from drf_spectacular.types import OpenApiTypes -from drf_spectacular.utils import Direction, OpenApiParameter +from drf_spectacular.utils import Direction, OpenApiParameter, OpenApiResponse from netbox.api.fields import ChoiceField -from netbox.api.serializers import WritableNestedSerializer +from netbox.api.serializers import BulkOperationErrorSerializer, WritableNestedSerializer from netbox.api.viewsets import NetBoxModelViewSet # see netbox.api.routers.NetBoxRouter @@ -182,6 +183,82 @@ class NetBoxAutoSchema(AutoSchema): return response_serializers + def _get_bulk_error_responses(self, direction) -> typing.Any: + """ + Return the error responses of the current bulk write action, keyed by status code, or an + empty dict if this action is not a bulk write. + + A failed bulk write returns a structured body correlating each failure with the object (or, + where no object could be identified, the request position) responsible for it. This is a + documented part of the API contract, but drf-spectacular cannot infer it: responses are + derived from the request/response serializer alone, which describes only the success case. + """ + action = getattr(self.view, 'action', None) + + if action in ('bulk_update', 'bulk_partial_update'): + return { + '400': OpenApiResponse( + response=BulkOperationErrorSerializer, + description=_( + "One or more of the objects specified could not be updated. No objects were " + "modified: a bulk update is an all-or-none operation." + ), + ), + } + + if action == 'bulk_destroy': + return { + '400': OpenApiResponse( + response=BulkOperationErrorSerializer, + description=_( + "The request was malformed, or one or more of the objects specified could " + "not be found. No objects were deleted." + ), + ), + '409': OpenApiResponse( + response=BulkOperationErrorSerializer, + description=_( + "One or more of the objects specified could not be deleted, because a " + "dependent object or a protection rule prevents it. No objects were " + "deleted: a bulk deletion is an all-or-none operation." + ), + ), + } + + if action == 'create' and viewset_handles_bulk_create(self.view): + # A POST to a list endpoint accepts either a single object or a list of them (see + # _get_request_for_media_type()), so its error body takes one of two shapes + # accordingly: field-keyed errors for a single object, or the bulk envelope for a list. + component = self.resolve_serializer(BulkOperationErrorSerializer, direction) + return { + '400': OpenApiResponse( + response={ + 'oneOf': [ + build_basic_type(OpenApiTypes.OBJECT), + component.ref if component else build_basic_type(OpenApiTypes.OBJECT), + ], + }, + description=_( + "The object could not be created. Where a list was submitted, no objects " + "were created: a bulk creation is an all-or-none operation." + ), + ), + } + + return {} + + def _get_response_bodies(self, direction='response') -> typing.Any: + responses = super()._get_response_bodies(direction=direction) + + # Document the error responses of the bulk write actions, which cannot be inferred (see + # _get_bulk_error_responses). A status code already present -- for instance one declared + # via @extend_schema on a custom action -- is left as it is. + for code, response in self._get_bulk_error_responses(direction).items(): + if code not in responses: + responses[code] = self._get_response_for_code(response, code, direction=direction) + + return responses + def _get_request_for_media_type(self, serializer, direction='request'): """ Override to generate oneOf schema for serializers that support both diff --git a/netbox/core/tests/test_openapi_schema.py b/netbox/core/tests/test_openapi_schema.py index 8158b87ec..521557279 100644 --- a/netbox/core/tests/test_openapi_schema.py +++ b/netbox/core/tests/test_openapi_schema.py @@ -107,3 +107,98 @@ 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 _get_response_schema(self, path, method, code): + """Return the JSON response schema documented for the given operation and status code.""" + responses = self.schema['paths'][path][method]['responses'] + self.assertIn(code, responses, f"{method.upper()} {path} should document a {code} response") + return responses[code]['content']['application/json']['schema'] + + def test_bulk_error_component_is_defined(self): + """ + The structured error body returned by a failed bulk operation should be a named component, + so that generated clients have a type for it. + + Refs: #20054 + """ + components = self.schema['components']['schemas'] + + self.assertIn('BulkOperationError', components) + envelope = components['BulkOperationError'] + self.assertEqual(sorted(envelope['properties']), ['detail', 'errors']) + # `errors` is absent where the request could not be attributed to individual entries + self.assertEqual(envelope['required'], ['detail']) + self.assertEqual( + envelope['properties']['errors']['items']['$ref'], + '#/components/schemas/BulkOperationEntryError', + ) + + self.assertIn('BulkOperationEntryError', components) + entry = components['BulkOperationEntryError'] + # An entry is correlated by `id` or by `index`, so neither is required; `errors` always is + self.assertEqual(sorted(entry['properties']), ['errors', 'id', 'index']) + self.assertEqual(entry['required'], ['errors']) + + def test_bulk_update_documents_error_response(self): + """ + Bulk update operations should document the structured 400 response. + + Refs: #20054 + """ + ref = {'$ref': '#/components/schemas/BulkOperationError'} + + for path in ('/api/dcim/sites/', '/api/ipam/prefixes/', '/api/users/users/'): + for method in ('put', 'patch'): + with self.subTest(path=path, method=method): + self.assertEqual(self._get_response_schema(path, method, '400'), ref) + + def test_bulk_delete_documents_error_responses(self): + """ + Bulk delete operations should document both the 400 (unresolvable request) and the 409 + (dependency or protection rule) responses. + + Refs: #20054 + """ + ref = {'$ref': '#/components/schemas/BulkOperationError'} + + for path in ('/api/dcim/sites/', '/api/ipam/prefixes/', '/api/users/users/'): + with self.subTest(path=path): + self.assertEqual(self._get_response_schema(path, 'delete', '400'), ref) + self.assertEqual(self._get_response_schema(path, 'delete', '409'), ref) + + def test_create_documents_error_response_for_either_shape(self): + """ + A POST to a list endpoint accepts either a single object or a list, so its 400 response + should document both the field-keyed and the bulk error shapes. + + Refs: #20054 + """ + for path in ('/api/dcim/sites/', '/api/ipam/prefixes/', '/api/users/users/'): + with self.subTest(path=path): + schema = self._get_response_schema(path, 'post', '400') + self.assertEqual( + schema['oneOf'], + [ + {'type': 'object', 'additionalProperties': {}}, + {'$ref': '#/components/schemas/BulkOperationError'}, + ], + ) + + def test_detail_operations_omit_bulk_error_response(self): + """ + The bulk error body applies only to list endpoints; detail endpoints must not advertise it. + + Refs: #20054 + """ + path = '/api/dcim/sites/{id}/' + + for method in ('get', 'put', 'patch', 'delete'): + with self.subTest(method=method): + responses = self.schema['paths'][path][method]['responses'] + self.assertNotIn('409', responses) + for code, response in responses.items(): + schema = response.get('content', {}).get('application/json', {}).get('schema', {}) + self.assertNotEqual( + schema.get('$ref'), '#/components/schemas/BulkOperationError', + f"{method.upper()} {path} ({code}) should not reference the bulk error body" + ) diff --git a/netbox/netbox/api/serializers/bulk.py b/netbox/netbox/api/serializers/bulk.py index fba4961b0..57c2a27a1 100644 --- a/netbox/netbox/api/serializers/bulk.py +++ b/netbox/netbox/api/serializers/bulk.py @@ -1,11 +1,14 @@ import copy import functools +from django.utils.translation import gettext_lazy as _ from rest_framework import serializers from .features import ChangeLogMessageSerializer __all__ = ( + 'BulkOperationEntryErrorSerializer', + 'BulkOperationErrorSerializer', 'BulkOperationSerializer', 'BulkPartialUpdateSchemaMixin', 'BulkUpdateSchemaMixin', @@ -17,6 +20,60 @@ class BulkOperationSerializer(ChangeLogMessageSerializer): id = serializers.IntegerField() +# The two serializers below are schema-only: they are never used to validate or render data. The +# bulk actions in netbox.api.viewsets.mixins assemble these payloads directly; these exist so that +# their error responses are a documented part of the OpenAPI schema rather than an untyped body. +# Note that a class docstring becomes the component's description in the published schema, so keep +# it user-facing. +class BulkOperationEntryErrorSerializer(serializers.Serializer): + """ + The failure of a single object within a bulk operation. + """ + id = serializers.IntegerField( + required=False, + help_text=_( + "The ID of the object which failed. Present once the entry has been matched to an " + "object; mutually exclusive with `index`." + ) + ) + index = serializers.IntegerField( + required=False, + help_text=_( + "The zero-based position of the entry within the submitted list. Used where no object " + "has been identified for the entry: always for creations, and for updates and deletions " + "where the entry itself could not be interpreted (e.g. a missing or non-numeric `id`). " + "Mutually exclusive with `id`." + ) + ) + errors = serializers.DictField( + help_text=_( + "The errors for this entry, keyed by field name. Values are ordinarily arrays of " + "messages. Errors which do not pertain to a specific field appear under `__all__` " + "(model validation, protection rules, restricted tags) or `non_field_errors` (errors " + "concerning the shape of the entry itself)." + ) + ) + + +class BulkOperationErrorSerializer(serializers.Serializer): + """ + The body returned when a bulk operation fails, correlating each failure with the object + responsible for it. + """ + detail = serializers.CharField( + help_text=_('A summary of the failure, e.g. "1 of 3 objects failed validation."') + ) + errors = BulkOperationEntryErrorSerializer( + many=True, + required=False, + help_text=_( + "One entry per object which failed; objects which would have succeeded are omitted, as " + "a bulk operation is all-or-none. Absent where the request could not be attributed to " + "individual entries at all (e.g. a request body which is not a list)." + ) + ) + + class BulkUpdateSchemaMixin: def get_fields(self): fields = super().get_fields()