diff --git a/netbox/netbox/api/serializers/bulk.py b/netbox/netbox/api/serializers/bulk.py index ad0cb5660..10d4eaafc 100644 --- a/netbox/netbox/api/serializers/bulk.py +++ b/netbox/netbox/api/serializers/bulk.py @@ -48,9 +48,9 @@ class BulkOperationEntryErrorSerializer(serializers.Serializer): 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)." + "messages. Errors which pertain to no particular field -- model validation, protection " + "rules, restricted tags, object-level permissions, or the shape of the entry itself -- " + "all appear under the single key `__all__`." ) ) diff --git a/netbox/netbox/api/viewsets/mixins.py b/netbox/netbox/api/viewsets/mixins.py index 362f2b0f9..f1c57c025 100644 --- a/netbox/netbox/api/viewsets/mixins.py +++ b/netbox/netbox/api/viewsets/mixins.py @@ -438,7 +438,10 @@ class BulkCreateModelMixin: for i, item in enumerate(data): if not isinstance(item, dict): # Checked explicitly because get_serializer() infers many=True from a list, so a - # nested list would otherwise be validated as a batch of its own + # nested list would otherwise be validated as a batch of its own. This mirrors + # the message REST framework itself reports for a non-dictionary item, hence its + # key rather than a literal -- which NON_FIELD_ERRORS_KEY is configured to match + # anyway, so that the API has a single key for non-field errors (see settings). errors.append({ 'index': i, 'errors': { diff --git a/netbox/netbox/settings.py b/netbox/netbox/settings.py index 07787427e..3d7f1bbfd 100644 --- a/netbox/netbox/settings.py +++ b/netbox/netbox/settings.py @@ -805,6 +805,12 @@ REST_FRAMEWORK = { 'DEFAULT_SCHEMA_CLASS': 'core.api.schema.NetBoxAutoSchema', 'DEFAULT_VERSION': REST_FRAMEWORK_VERSION, 'DEFAULT_VERSIONING_CLASS': 'rest_framework.versioning.AcceptHeaderVersioning', + # Align REST framework's key for errors which pertain to no particular field with Django's + # (django.core.exceptions.NON_FIELD_ERRORS), so that the API reports such an error under one key + # rather than two. Model validation errors reach a response by way of full_clean(), and so are + # keyed by Django; errors raised by a serializer or field are keyed by REST framework. Without + # this, which of the two a client must read depends on the layer which rejected the request. + 'NON_FIELD_ERRORS_KEY': '__all__', 'SCHEMA_COERCE_METHOD_NAMES': { # Default mappings 'retrieve': 'read', diff --git a/netbox/netbox/tests/test_api.py b/netbox/netbox/tests/test_api.py index 29d002a5e..7293f13c0 100644 --- a/netbox/netbox/tests/test_api.py +++ b/netbox/netbox/tests/test_api.py @@ -1,11 +1,13 @@ import uuid from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import NON_FIELD_ERRORS from django.db.backends.postgresql.psycopg_any import NumericRange from django.test import RequestFactory, TestCase from django.urls import reverse from rest_framework.exceptions import ValidationError from rest_framework.request import Request +from rest_framework.settings import api_settings from dcim.api.serializers import RackSerializer from dcim.models import Device, Site @@ -51,6 +53,44 @@ class AppTestCase(APITestCase): self.assertEqual(response.data['id'], self.user.pk) +class NonFieldErrorKeyTestCase(APITestCase): + """ + REST framework's key for errors which pertain to no particular field is configured to match + Django's, so that the API reports such an error under `__all__` regardless of which layer + rejected the request (see REST_FRAMEWORK['NON_FIELD_ERRORS_KEY'] in settings). Model validation + errors are keyed by Django, having reached the response by way of full_clean(); errors raised by + a serializer or field are keyed by REST framework. + """ + def setUp(self): + super().setUp() + self.add_permissions('dcim.add_site', 'dcim.view_site', 'dcim.change_site') + self.url = reverse('dcim-api:site-list') + + def test_setting_matches_django(self): + self.assertEqual(api_settings.NON_FIELD_ERRORS_KEY, NON_FIELD_ERRORS) + + def test_serializer_error_uses_all_key(self): + """An error from REST framework's own machinery (here, a non-dictionary item).""" + response = self.client.post(self.url, ['not an object'], format='json', **self.header) + + self.assertEqual(response.status_code, 400) + self.assertIn(NON_FIELD_ERRORS, response.data['errors'][0]['errors']) + + def test_model_validation_error_uses_all_key(self): + """An error from Django's full_clean(), which uses this key of its own accord.""" + site = Site.objects.create(name='Site 1', slug='site-1') + # A Location's name must be unique within its Site, enforced by a model constraint + location_url = reverse('dcim-api:location-list') + self.add_permissions('dcim.add_location', 'dcim.view_location') + data = {'name': 'Location 1', 'slug': 'location-1', 'site': site.pk} + self.assertEqual(self.client.post(location_url, data, format='json', **self.header).status_code, 201) + + response = self.client.post(location_url, data, format='json', **self.header) + + self.assertEqual(response.status_code, 400) + self.assertIn(NON_FIELD_ERRORS, response.data) + + class RelatedObjectCountFieldTestCase(TestCase): """ RelatedObjectCountFields are populated by annotations applied to a viewset's queryset, which are only