Răsfoiți Sursa

Fixes #23166: Apply zero-valued numeric bounds to profile attribute form fields

Jason Novinger 8 ore în urmă
părinte
comite
e15b3d9080

+ 27 - 0
netbox/dcim/tests/test_forms.py

@@ -229,6 +229,33 @@ class ModuleTypeFormTestCase(TestCase):
             module_type = form.save()
             self.assertEqual(module_type.attribute_data, {'media': ['copper', 'qsfp28']})
 
+    def test_zero_bound_attribute_is_enforced_by_the_form(self):
+        profile = ModuleTypeProfile.objects.create(
+            name='Module Type Profile 2',
+            schema={
+                'properties': {
+                    'offset': {
+                        'title': 'Offset',
+                        'type': 'number',
+                        'minimum': 0,
+                        'maximum': 0,
+                    },
+                },
+            },
+        )
+        form = ModuleTypeForm(data={
+            'manufacturer': self.manufacturer.pk,
+            'model': 'Module Type 2',
+            'profile': profile.pk,
+            'attr_offset': -5,
+        })
+
+        self.assertEqual(form.fields['attr_offset'].min_value, 0)
+        self.assertEqual(form.fields['attr_offset'].max_value, 0)
+        with patch('utilities.forms.fields.dynamic.get_action_url', return_value='/'):
+            self.assertFalse(form.is_valid())
+        self.assertIn('attr_offset', form.errors)
+
 
 class ModuleTypeProfileDescriptionRenderingTestCase(TestCase):
     """

+ 15 - 9
netbox/utilities/jsonschema.py

@@ -87,6 +87,7 @@ class JSONSchemaProperty:
         """
         Instantiate and return a Django form field suitable for editing the property's value.
         """
+        field_class = self.field_class
         field_kwargs = {
             'label': self.title or title(name),
             'help_text': render_markdown(self.description),
@@ -111,10 +112,14 @@ class JSONSchemaProperty:
 
         # String validation
         if self.type == PropertyTypeEnum.STRING.value:
-            if self.minLength is not None:
-                field_kwargs['min_length'] = self.minLength
-            if self.maxLength is not None:
-                field_kwargs['max_length'] = self.maxLength
+            # Checking against CharField is safe because the other CharField-derived fields are
+            # ruled out by the "is a string" check above. UUIDField is the exception: it cleans to
+            # a uuid.UUID, which the length validators can't call len() on.
+            if issubclass(field_class, forms.CharField) and not issubclass(field_class, forms.UUIDField):
+                if self.minLength is not None:
+                    field_kwargs['min_length'] = self.minLength
+                if self.maxLength is not None:
+                    field_kwargs['max_length'] = self.maxLength
             if self.pattern is not None:
                 field_kwargs['validators'] = [
                     RegexValidator(regex=self.pattern)
@@ -122,11 +127,12 @@ class JSONSchemaProperty:
 
         # Integer/number validation
         elif self.type in (PropertyTypeEnum.INTEGER.value, PropertyTypeEnum.NUMBER.value):
-            field_kwargs['widget'] = forms.NumberInput(attrs={'step': 'any'})
-            if self.minimum:
-                field_kwargs['min_value'] = self.minimum
-            if self.maximum:
-                field_kwargs['max_value'] = self.maximum
+            if issubclass(field_class, forms.IntegerField):
+                field_kwargs['widget'] = forms.NumberInput(attrs={'step': 'any'})
+                if self.minimum is not None:
+                    field_kwargs['min_value'] = self.minimum
+                if self.maximum is not None:
+                    field_kwargs['max_value'] = self.maximum
             if self.multipleOf:
                 field_kwargs['validators'] = [
                     MultipleOfValidator(multiple=self.multipleOf)

+ 207 - 0
netbox/utilities/tests/test_jsonschema.py

@@ -1,8 +1,13 @@
+from uuid import UUID
+
 from django import forms
 from django.contrib.postgres.forms import SimpleArrayField
+from django.core.exceptions import ValidationError
+from django.core.validators import RegexValidator
 from django.test import TestCase
 
 from utilities.jsonschema import JSONSchemaProperty
+from utilities.validators import MultipleOfValidator
 
 
 class JSONSchemaPropertyTestCase(TestCase):
@@ -45,6 +50,208 @@ class JSONSchemaPropertyTestCase(TestCase):
         self.assertIsInstance(field.base_field, forms.CharField)
         self.assertEqual(field.clean('ge-0/0/0,ge-0/0/1'), ['ge-0/0/0', 'ge-0/0/1'])
 
+    def test_zero_minimum_is_applied_to_form_field(self):
+        prop = JSONSchemaProperty(type='number', title='Offset', minimum=0)
+
+        field = prop.to_form_field('offset')
+
+        self.assertEqual(field.min_value, 0)
+        with self.assertRaises(ValidationError):
+            field.clean(-5)
+        self.assertEqual(field.clean(0), 0)
+
+    def test_zero_maximum_is_applied_to_form_field(self):
+        prop = JSONSchemaProperty(type='number', title='Offset', maximum=0)
+
+        field = prop.to_form_field('offset')
+
+        self.assertEqual(field.max_value, 0)
+        with self.assertRaises(ValidationError):
+            field.clean(5)
+        self.assertEqual(field.clean(0), 0)
+
+    def test_zero_bounds_are_applied_to_integer_form_field(self):
+        prop = JSONSchemaProperty(type='integer', title='Slots', minimum=0, maximum=0)
+
+        field = prop.to_form_field('slots')
+
+        self.assertEqual(field.min_value, 0)
+        self.assertEqual(field.max_value, 0)
+        with self.assertRaises(ValidationError):
+            field.clean(-1)
+        with self.assertRaises(ValidationError):
+            field.clean(1)
+        self.assertEqual(field.clean(0), 0)
+
+    def test_nonzero_bounds_are_applied_to_form_field(self):
+        prop = JSONSchemaProperty(type='number', title='Offset', minimum=1, maximum=10)
+
+        field = prop.to_form_field('offset')
+
+        self.assertEqual(field.min_value, 1)
+        self.assertEqual(field.max_value, 10)
+        with self.assertRaises(ValidationError):
+            field.clean(0)
+        with self.assertRaises(ValidationError):
+            field.clean(11)
+
+    def test_omitted_bounds_are_not_applied_to_form_field(self):
+        prop = JSONSchemaProperty(type='number', title='Offset')
+
+        field = prop.to_form_field('offset')
+
+        self.assertIsNone(field.min_value)
+        self.assertIsNone(field.max_value)
+        self.assertEqual(field.clean(-100), -100)
+
+    def test_numeric_enum_with_zero_bound_builds_choice_field(self):
+        """A numeric property carrying both an enum and a zero bound resolves to a ChoiceField.
+
+        ChoiceField accepts neither min_value nor max_value, so the numeric bounds must not be
+        passed through when an enum is present.
+        """
+        prop = JSONSchemaProperty(type='integer', title='Slots', enum=[0, 1, 2], minimum=0)
+
+        field = prop.to_form_field('slots')
+
+        self.assertIsInstance(field, forms.ChoiceField)
+        self.assertEqual(list(field.choices), [(None, ''), (0, 0), (1, 1), (2, 2)])
+
+    def test_numeric_enum_with_nonzero_bound_builds_choice_field(self):
+        prop = JSONSchemaProperty(type='integer', title='Slots', enum=[1, 2], minimum=1, maximum=2)
+
+        field = prop.to_form_field('slots')
+
+        self.assertIsInstance(field, forms.ChoiceField)
+        self.assertEqual(list(field.choices), [(None, ''), (1, 1), (2, 2)])
+
+    def test_numeric_enum_with_multiple_of_builds_choice_field(self):
+        """An enum suppresses the numeric bounds but retains the multipleOf validator.
+
+        Field.__init__() accepts validators, so a MultipleOfValidator remains applicable to a
+        ChoiceField even though min_value and max_value are not.
+        """
+        prop = JSONSchemaProperty(type='integer', title='Slots', enum=[2, 4], multipleOf=2)
+
+        field = prop.to_form_field('slots')
+
+        self.assertIsInstance(field, forms.ChoiceField)
+        self.assertEqual(list(field.choices), [(None, ''), (2, 2), (4, 4)])
+        self.assertEqual(len(field.validators), 1)
+        self.assertIsInstance(field.validators[0], MultipleOfValidator)
+
+    def test_string_enum_with_min_length_builds_choice_field(self):
+        """A string property carrying both an enum and a length bound resolves to a ChoiceField.
+
+        ChoiceField accepts neither min_length nor max_length, so the length bounds must not be
+        passed through when an enum is present.
+        """
+        prop = JSONSchemaProperty(type='string', title='Media', enum=['a', 'bb'], minLength=1)
+
+        field = prop.to_form_field('media')
+
+        self.assertIsInstance(field, forms.ChoiceField)
+        self.assertEqual(list(field.choices), [(None, ''), ('a', 'a'), ('bb', 'bb')])
+
+    def test_string_enum_with_max_length_builds_choice_field(self):
+        prop = JSONSchemaProperty(type='string', title='Media', enum=['a', 'bb'], maxLength=2)
+
+        field = prop.to_form_field('media')
+
+        self.assertIsInstance(field, forms.ChoiceField)
+        self.assertEqual(list(field.choices), [(None, ''), ('a', 'a'), ('bb', 'bb')])
+
+    def test_string_enum_retains_pattern_validator(self):
+        """Dropping the length bounds for an enum must not also drop the pattern validator.
+
+        Field.__init__() accepts validators, so a RegexValidator remains applicable to a
+        ChoiceField even though min_length and max_length are not.
+        """
+        prop = JSONSchemaProperty(
+            type='string', title='Media', enum=['a', 'bb'], minLength=1, pattern='^[ab]+$'
+        )
+
+        field = prop.to_form_field('media')
+
+        self.assertIsInstance(field, forms.ChoiceField)
+        self.assertEqual(len(field.validators), 1)
+        self.assertIsInstance(field.validators[0], RegexValidator)
+        self.assertEqual(field.validators[0].regex.pattern, '^[ab]+$')
+
+    def test_string_bounds_are_applied_without_an_enum(self):
+        prop = JSONSchemaProperty(type='string', title='Media', minLength=1, maxLength=4)
+
+        field = prop.to_form_field('media')
+
+        self.assertIsInstance(field, forms.CharField)
+        self.assertEqual(field.min_length, 1)
+        self.assertEqual(field.max_length, 4)
+        with self.assertRaises(ValidationError):
+            field.clean('toolong')
+
+    def test_string_format_with_length_bound_builds_format_field(self):
+        """A string format resolves to a field class which accepts no length bounds.
+
+        DateField, TimeField and DateTimeField do not subclass CharField, so passing minLength
+        or maxLength to one raises TypeError.
+        """
+        for string_format, expected_class in (
+            ('date', forms.DateField),
+            ('time', forms.TimeField),
+            ('datetime', forms.DateTimeField),
+        ):
+            with self.subTest(format=string_format):
+                prop = JSONSchemaProperty(
+                    type='string', title='Timestamp', format=string_format, minLength=10, maxLength=30
+                )
+
+                field = prop.to_form_field('timestamp')
+
+                self.assertIsInstance(field, expected_class)
+
+    def test_string_format_retains_pattern_validator(self):
+        prop = JSONSchemaProperty(type='string', title='Timestamp', format='date', pattern='^x$')
+
+        field = prop.to_form_field('timestamp')
+
+        self.assertIsInstance(field, forms.DateField)
+        self.assertEqual(len(field.validators), 1)
+        self.assertIsInstance(field.validators[0], RegexValidator)
+
+    def test_charfield_derived_format_retains_length_bounds(self):
+        """EmailField and URLField clean to a string, so the length bounds apply to them."""
+        for string_format, expected_class, value in (
+            ('email', forms.EmailField, 'user@example.com'),
+            ('uri', forms.URLField, 'https://example.com/x'),
+        ):
+            with self.subTest(format=string_format):
+                prop = JSONSchemaProperty(
+                    type='string', title='Contact', format=string_format, minLength=5, maxLength=40
+                )
+
+                field = prop.to_form_field('contact')
+
+                self.assertIsInstance(field, expected_class)
+                self.assertEqual(field.min_length, 5)
+                self.assertEqual(field.max_length, 40)
+                self.assertEqual(field.clean(value), value)
+
+    def test_uuid_format_omits_length_bounds(self):
+        """UUIDField subclasses CharField but cleans to a uuid.UUID, which has no length.
+
+        CharField.__init__() installs a MinLengthValidator and MaxLengthValidator for the bounds,
+        and those call len() on the cleaned value, so a UUID raises TypeError at clean time.
+        """
+        value = '12345678-1234-5678-1234-567812345678'
+        prop = JSONSchemaProperty(type='string', title='Serial', format='uuid', minLength=5, maxLength=40)
+
+        field = prop.to_form_field('serial')
+
+        self.assertIsInstance(field, forms.UUIDField)
+        self.assertIsNone(field.min_length)
+        self.assertIsNone(field.max_length)
+        self.assertEqual(field.clean(value), UUID(value))
+
 
 class JSONSchemaPropertyDescriptionSanitizationTestCase(TestCase):
     """