From f3f3993963bac7104edc51bd777887462e7dd5af Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 12 Nov 2020 14:23:08 -0500 Subject: [PATCH] Enforce custom field validation on the model --- netbox/extras/api/customfields.py | 61 --------------------- netbox/extras/models/customfields.py | 70 +++++++++++++++++++++--- netbox/extras/tests/test_customfields.py | 23 +++++++- 3 files changed, 83 insertions(+), 71 deletions(-) diff --git a/netbox/extras/api/customfields.py b/netbox/extras/api/customfields.py index 8cb7dd5ba..229efa3b2 100644 --- a/netbox/extras/api/customfields.py +++ b/netbox/extras/api/customfields.py @@ -1,8 +1,4 @@ -import re -from datetime import datetime - from django.contrib.contenttypes.models import ContentType -from rest_framework.exceptions import ValidationError from rest_framework.fields import CreateOnlyDefault, Field from extras.choices import * @@ -66,63 +62,6 @@ class CustomFieldsDataField(Field): if self.parent.instance: data = {**self.parent.instance.custom_field_data, **data} - custom_fields = {field.name: field for field in self._get_custom_fields()} - - for field_name, value in data.items(): - - try: - cf = custom_fields[field_name] - except KeyError: - raise ValidationError(f"Invalid custom field name: {field_name}") - - # Data validation - if value not in [None, '']: - - # Validate text field - if cf.type == CustomFieldTypeChoices.TYPE_TEXT and cf.validation_regex: - if not re.match(cf.validation_regex, value): - raise ValidationError(f"{field_name}: Value must match regex {cf.validation_regex}") - - # Validate integer - if cf.type == CustomFieldTypeChoices.TYPE_INTEGER: - try: - int(value) - except ValueError: - raise ValidationError(f"Invalid value for integer field {field_name}: {value}") - if cf.validation_minimum is not None and value < cf.validation_minimum: - raise ValidationError(f"{field_name}: Value must be at least {cf.validation_minimum}") - if cf.validation_maximum is not None and value > cf.validation_maximum: - raise ValidationError(f"{field_name}: Value must not exceed {cf.validation_maximum}") - - # Validate boolean - if cf.type == CustomFieldTypeChoices.TYPE_BOOLEAN and value not in [True, False, 1, 0]: - raise ValidationError(f"Invalid value for boolean field {field_name}: {value}") - - # Validate date - if cf.type == CustomFieldTypeChoices.TYPE_DATE: - try: - datetime.strptime(value, '%Y-%m-%d') - except ValueError: - raise ValidationError( - f"Invalid date for field {field_name}: {value}. (Required format is YYYY-MM-DD.)" - ) - - # Validate selected choice - if cf.type == CustomFieldTypeChoices.TYPE_SELECT: - if value not in cf.choices: - raise ValidationError(f"Invalid choice for field {field_name}: {value}") - - elif cf.required: - raise ValidationError(f"Required field {field_name} cannot be empty.") - - # Check for missing required fields - missing_fields = [] - for field_name, field in custom_fields.items(): - if field.required and field_name not in data: - missing_fields.append(field_name) - if missing_fields: - raise ValidationError("Missing required fields: {}".format(u", ".join(missing_fields))) - return data diff --git a/netbox/extras/models/customfields.py b/netbox/extras/models/customfields.py index fa4f55fa6..504de0953 100644 --- a/netbox/extras/models/customfields.py +++ b/netbox/extras/models/customfields.py @@ -1,4 +1,6 @@ +import re from collections import OrderedDict +from datetime import datetime from django import forms from django.contrib.contenttypes.models import ContentType @@ -8,10 +10,10 @@ from django.core.validators import RegexValidator, ValidationError from django.db import models from django.utils.safestring import mark_safe -from utilities.forms import CSVChoiceField, DatePicker, LaxURLField, StaticSelect2, add_blank_choice -from utilities.validators import validate_regex from extras.choices import * from extras.utils import FeatureQuery +from utilities.forms import CSVChoiceField, DatePicker, LaxURLField, StaticSelect2, add_blank_choice +from utilities.validators import validate_regex class CustomFieldModel(models.Model): @@ -44,14 +46,21 @@ class CustomFieldModel(models.Model): ]) def clean(self): + custom_fields = {cf.name: cf for cf in CustomField.objects.get_for_model(self)} - # Validate custom field data - custom_field_names = CustomField.objects.get_for_model(self).values_list('name', flat=True) - for field_name in self.custom_field_data: - if field_name not in custom_field_names: - raise ValidationError({ - 'custom_field_data': f'Unknown custom field: {field_name}' - }) + # Validate all field values + for field_name, value in self.custom_field_data.items(): + if field_name not in custom_fields: + raise ValidationError(f"Unknown field name '{field_name}' in custom field data.") + try: + custom_fields[field_name].validate(value) + except ValidationError as e: + raise ValidationError(f"Invalid value for custom field '{field_name}': {e.message}") + + # Check for missing required values + for cf in custom_fields.values(): + if cf.required and cf.name not in self.custom_field_data: + raise ValidationError(f"Missing required custom field '{cf.name}'.") class CustomFieldManager(models.Manager): @@ -270,3 +279,46 @@ class CustomField(models.Model): field.help_text = self.description return field + + def validate(self, value): + """ + Validate a value according to the field's type validation rules. + """ + if value not in [None, '']: + + # Validate text field + if self.type == CustomFieldTypeChoices.TYPE_TEXT and self.validation_regex: + if not re.match(self.validation_regex, value): + raise ValidationError(f"Value must match regex '{self.validation_regex}'") + + # Validate integer + if self.type == CustomFieldTypeChoices.TYPE_INTEGER: + try: + int(value) + except ValueError: + raise ValidationError("Value must be an integer.") + if self.validation_minimum is not None and value < self.validation_minimum: + raise ValidationError(f"Value must be at least {self.validation_minimum}") + if self.validation_maximum is not None and value > self.validation_maximum: + raise ValidationError(f"Value must not exceed {self.validation_maximum}") + + # Validate boolean + if self.type == CustomFieldTypeChoices.TYPE_BOOLEAN and value not in [True, False, 1, 0]: + raise ValidationError("Value must be true or false.") + + # Validate date + if self.type == CustomFieldTypeChoices.TYPE_DATE: + try: + datetime.strptime(value, '%Y-%m-%d') + except ValueError: + raise ValidationError("Date values must be in the format YYYY-MM-DD.") + + # Validate selected choice + if self.type == CustomFieldTypeChoices.TYPE_SELECT: + if value not in self.choices: + raise ValidationError( + f"Invalid choice ({value}). Available choices are: {', '.join(self.choices)}" + ) + + elif self.required: + raise ValidationError("Required field cannot be empty.") diff --git a/netbox/extras/tests/test_customfields.py b/netbox/extras/tests/test_customfields.py index 9286acf1e..fe56027dc 100644 --- a/netbox/extras/tests/test_customfields.py +++ b/netbox/extras/tests/test_customfields.py @@ -550,6 +550,10 @@ class CustomFieldModelTest(TestCase): cf2.content_types.set([ContentType.objects.get_for_model(Rack)]) def test_cf_data(self): + """ + Check that custom field data is present on the instance immediately after being set and after being fetched + from the database. + """ site = Site(name='Test Site', slug='test-site') # Check custom field data on new instance @@ -570,9 +574,26 @@ class CustomFieldModelTest(TestCase): # Set custom field data site.cf['foo'] = 'abc' site.cf['bar'] = 'def' - with self.assertRaises(ValidationError): site.clean() del(site.cf['bar']) site.clean() + + def test_missing_required_field(self): + """ + Check that a ValidationError is raised if any required custom fields are not present. + """ + cf3 = CustomField(type=CustomFieldTypeChoices.TYPE_TEXT, name='baz', required=True) + cf3.save() + cf3.content_types.set([ContentType.objects.get_for_model(Site)]) + + site = Site(name='Test Site', slug='test-site') + + # Set custom field data with a required field omitted + site.cf['foo'] = 'abc' + with self.assertRaises(ValidationError): + site.clean() + + site.cf['baz'] = 'def' + site.clean()