From 52e0f898f1a41f6cf8b213929d7be7a6b39cdcf3 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Tue, 15 Aug 2017 13:54:04 -0400 Subject: [PATCH] Fixes #1421: Improved model validation logic for API serializers --- netbox/circuits/api/serializers.py | 6 ++-- netbox/dcim/api/serializers.py | 44 +++++++++++++++--------------- netbox/extras/api/customfields.py | 13 ++------- netbox/extras/api/serializers.py | 4 +-- netbox/ipam/api/serializers.py | 8 +++--- netbox/secrets/api/serializers.py | 4 +-- netbox/tenancy/api/serializers.py | 4 +-- netbox/utilities/api.py | 28 +++++++++++-------- 8 files changed, 53 insertions(+), 58 deletions(-) diff --git a/netbox/circuits/api/serializers.py b/netbox/circuits/api/serializers.py index cdab3427a37..d2432374f3d 100644 --- a/netbox/circuits/api/serializers.py +++ b/netbox/circuits/api/serializers.py @@ -6,7 +6,7 @@ from dcim.api.serializers import NestedSiteSerializer, InterfaceSerializer from extras.api.customfields import CustomFieldModelSerializer from tenancy.api.serializers import NestedTenantSerializer -from utilities.api import ModelValidationMixin +from utilities.api import ValidatedModelSerializer # @@ -45,7 +45,7 @@ class Meta: # Circuit types # -class CircuitTypeSerializer(ModelValidationMixin, serializers.ModelSerializer): +class CircuitTypeSerializer(ValidatedModelSerializer): class Meta: model = CircuitType @@ -111,7 +111,7 @@ class Meta: ] -class WritableCircuitTerminationSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableCircuitTerminationSerializer(ValidatedModelSerializer): class Meta: model = CircuitTermination diff --git a/netbox/dcim/api/serializers.py b/netbox/dcim/api/serializers.py index 50bf756e3ee..ebfb781e01d 100644 --- a/netbox/dcim/api/serializers.py +++ b/netbox/dcim/api/serializers.py @@ -15,7 +15,7 @@ ) from extras.api.customfields import CustomFieldModelSerializer from tenancy.api.serializers import NestedTenantSerializer -from utilities.api import ChoiceFieldSerializer, ModelValidationMixin +from utilities.api import ChoiceFieldSerializer, ValidatedModelSerializer # @@ -38,7 +38,7 @@ class Meta: fields = ['id', 'name', 'slug', 'parent'] -class WritableRegionSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableRegionSerializer(ValidatedModelSerializer): class Meta: model = Region @@ -100,7 +100,7 @@ class Meta: fields = ['id', 'url', 'name', 'slug'] -class WritableRackGroupSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableRackGroupSerializer(ValidatedModelSerializer): class Meta: model = RackGroup @@ -111,7 +111,7 @@ class Meta: # Rack roles # -class RackRoleSerializer(ModelValidationMixin, serializers.ModelSerializer): +class RackRoleSerializer(ValidatedModelSerializer): class Meta: model = RackRole @@ -216,7 +216,7 @@ class Meta: fields = ['id', 'rack', 'units', 'created', 'user', 'description'] -class WritableRackReservationSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableRackReservationSerializer(ValidatedModelSerializer): class Meta: model = RackReservation @@ -227,7 +227,7 @@ class Meta: # Manufacturers # -class ManufacturerSerializer(ModelValidationMixin, serializers.ModelSerializer): +class ManufacturerSerializer(ValidatedModelSerializer): class Meta: model = Manufacturer @@ -292,7 +292,7 @@ class Meta: fields = ['id', 'device_type', 'name'] -class WritableConsolePortTemplateSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableConsolePortTemplateSerializer(ValidatedModelSerializer): class Meta: model = ConsolePortTemplate @@ -311,7 +311,7 @@ class Meta: fields = ['id', 'device_type', 'name'] -class WritableConsoleServerPortTemplateSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableConsoleServerPortTemplateSerializer(ValidatedModelSerializer): class Meta: model = ConsoleServerPortTemplate @@ -330,7 +330,7 @@ class Meta: fields = ['id', 'device_type', 'name'] -class WritablePowerPortTemplateSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritablePowerPortTemplateSerializer(ValidatedModelSerializer): class Meta: model = PowerPortTemplate @@ -349,7 +349,7 @@ class Meta: fields = ['id', 'device_type', 'name'] -class WritablePowerOutletTemplateSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritablePowerOutletTemplateSerializer(ValidatedModelSerializer): class Meta: model = PowerOutletTemplate @@ -369,7 +369,7 @@ class Meta: fields = ['id', 'device_type', 'name', 'form_factor', 'mgmt_only'] -class WritableInterfaceTemplateSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableInterfaceTemplateSerializer(ValidatedModelSerializer): class Meta: model = InterfaceTemplate @@ -388,7 +388,7 @@ class Meta: fields = ['id', 'device_type', 'name'] -class WritableDeviceBayTemplateSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableDeviceBayTemplateSerializer(ValidatedModelSerializer): class Meta: model = DeviceBayTemplate @@ -399,7 +399,7 @@ class Meta: # Device roles # -class DeviceRoleSerializer(ModelValidationMixin, serializers.ModelSerializer): +class DeviceRoleSerializer(ValidatedModelSerializer): class Meta: model = DeviceRole @@ -418,7 +418,7 @@ class Meta: # Platforms # -class PlatformSerializer(ModelValidationMixin, serializers.ModelSerializer): +class PlatformSerializer(ValidatedModelSerializer): class Meta: model = Platform @@ -516,7 +516,7 @@ class Meta: read_only_fields = ['connected_console'] -class WritableConsoleServerPortSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableConsoleServerPortSerializer(ValidatedModelSerializer): class Meta: model = ConsoleServerPort @@ -536,7 +536,7 @@ class Meta: fields = ['id', 'device', 'name', 'cs_port', 'connection_status'] -class WritableConsolePortSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableConsolePortSerializer(ValidatedModelSerializer): class Meta: model = ConsolePort @@ -556,7 +556,7 @@ class Meta: read_only_fields = ['connected_port'] -class WritablePowerOutletSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritablePowerOutletSerializer(ValidatedModelSerializer): class Meta: model = PowerOutlet @@ -576,7 +576,7 @@ class Meta: fields = ['id', 'device', 'name', 'power_outlet', 'connection_status'] -class WritablePowerPortSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritablePowerPortSerializer(ValidatedModelSerializer): class Meta: model = PowerPort @@ -664,7 +664,7 @@ class Meta: ] -class WritableInterfaceSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableInterfaceSerializer(ValidatedModelSerializer): class Meta: model = Interface @@ -694,7 +694,7 @@ class Meta: fields = ['id', 'url', 'name'] -class WritableDeviceBaySerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableDeviceBaySerializer(ValidatedModelSerializer): class Meta: model = DeviceBay @@ -717,7 +717,7 @@ class Meta: ] -class WritableInventoryItemSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableInventoryItemSerializer(ValidatedModelSerializer): class Meta: model = InventoryItem @@ -749,7 +749,7 @@ class Meta: fields = ['id', 'url', 'connection_status'] -class WritableInterfaceConnectionSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableInterfaceConnectionSerializer(ValidatedModelSerializer): class Meta: model = InterfaceConnection diff --git a/netbox/extras/api/customfields.py b/netbox/extras/api/customfields.py index 52f127a7d6b..fc83b33e575 100644 --- a/netbox/extras/api/customfields.py +++ b/netbox/extras/api/customfields.py @@ -10,6 +10,7 @@ from extras.models import ( CF_TYPE_BOOLEAN, CF_TYPE_DATE, CF_TYPE_SELECT, CustomField, CustomFieldChoice, CustomFieldValue, ) +from utilities.api import ValidatedModelSerializer # @@ -68,7 +69,7 @@ def to_internal_value(self, data): return data -class CustomFieldModelSerializer(serializers.ModelSerializer): +class CustomFieldModelSerializer(ValidatedModelSerializer): """ Extends ModelSerializer to render any CustomFields and their values associated with an object. """ @@ -111,16 +112,6 @@ def _save_custom_fields(self, instance, custom_fields): defaults={'serialized_value': custom_field.serialize_value(value)}, ) - def validate(self, data): - """ - Enforce model validation (see utilities.api.ModelValidationMixin) - """ - model_data = data.copy() - model_data.pop('custom_fields', None) - instance = self.Meta.model(**model_data) - instance.clean() - return data - def create(self, validated_data): custom_fields = validated_data.pop('custom_fields', None) diff --git a/netbox/extras/api/serializers.py b/netbox/extras/api/serializers.py index 39ce63524c0..0eeab49ecc6 100644 --- a/netbox/extras/api/serializers.py +++ b/netbox/extras/api/serializers.py @@ -10,7 +10,7 @@ ACTION_CHOICES, ExportTemplate, Graph, GRAPH_TYPE_CHOICES, ImageAttachment, TopologyMap, UserAction, ) from users.api.serializers import NestedUserSerializer -from utilities.api import ChoiceFieldSerializer, ContentTypeFieldSerializer, ModelValidationMixin +from utilities.api import ChoiceFieldSerializer, ContentTypeFieldSerializer, ValidatedModelSerializer # @@ -104,7 +104,7 @@ def get_parent(self, obj): return serializer(obj.parent, context={'request': self.context['request']}).data -class WritableImageAttachmentSerializer(ModelValidationMixin, serializers.ModelSerializer): +class WritableImageAttachmentSerializer(ValidatedModelSerializer): content_type = ContentTypeFieldSerializer() class Meta: diff --git a/netbox/ipam/api/serializers.py b/netbox/ipam/api/serializers.py index 1374d355275..3ef152ebe0d 100644 --- a/netbox/ipam/api/serializers.py +++ b/netbox/ipam/api/serializers.py @@ -11,7 +11,7 @@ PREFIX_STATUS_CHOICES, RIR, Role, Service, VLAN, VLAN_STATUS_CHOICES, VLANGroup, VRF, ) from tenancy.api.serializers import NestedTenantSerializer -from utilities.api import ChoiceFieldSerializer, ModelValidationMixin +from utilities.api import ChoiceFieldSerializer, ValidatedModelSerializer # @@ -45,7 +45,7 @@ class Meta: # Roles # -class RoleSerializer(ModelValidationMixin, serializers.ModelSerializer): +class RoleSerializer(ValidatedModelSerializer): class Meta: model = Role @@ -64,7 +64,7 @@ class Meta: # RIRs # -class RIRSerializer(ModelValidationMixin, serializers.ModelSerializer): +class RIRSerializer(ValidatedModelSerializer): class Meta: model = RIR @@ -303,7 +303,7 @@ class Meta: fields = ['id', 'device', 'name', 'port', 'protocol', 'ipaddresses', 'description'] -# TODO: Figure out how to use ModelValidationMixin with ManyToManyFields. Calling clean() yields a ValueError. +# TODO: Figure out how to use model validation with ManyToManyFields. Calling clean() yields a ValueError. class WritableServiceSerializer(serializers.ModelSerializer): class Meta: diff --git a/netbox/secrets/api/serializers.py b/netbox/secrets/api/serializers.py index ff2eb1dfa55..b7c4bac9a49 100644 --- a/netbox/secrets/api/serializers.py +++ b/netbox/secrets/api/serializers.py @@ -5,14 +5,14 @@ from dcim.api.serializers import NestedDeviceSerializer from secrets.models import Secret, SecretRole -from utilities.api import ModelValidationMixin +from utilities.api import ValidatedModelSerializer # # SecretRoles # -class SecretRoleSerializer(ModelValidationMixin, serializers.ModelSerializer): +class SecretRoleSerializer(ValidatedModelSerializer): class Meta: model = SecretRole diff --git a/netbox/tenancy/api/serializers.py b/netbox/tenancy/api/serializers.py index ef5b15a169c..a52ac2c6015 100644 --- a/netbox/tenancy/api/serializers.py +++ b/netbox/tenancy/api/serializers.py @@ -4,14 +4,14 @@ from extras.api.customfields import CustomFieldModelSerializer from tenancy.models import Tenant, TenantGroup -from utilities.api import ModelValidationMixin +from utilities.api import ValidatedModelSerializer # # Tenant groups # -class TenantGroupSerializer(ModelValidationMixin, serializers.ModelSerializer): +class TenantGroupSerializer(ValidatedModelSerializer): class Meta: model = TenantGroup diff --git a/netbox/utilities/api.py b/netbox/utilities/api.py index 6a515b21d68..2e827d503bf 100644 --- a/netbox/utilities/api.py +++ b/netbox/utilities/api.py @@ -8,7 +8,7 @@ from rest_framework.exceptions import APIException from rest_framework.pagination import LimitOffsetPagination from rest_framework.permissions import BasePermission, DjangoModelPermissions, SAFE_METHODS -from rest_framework.serializers import Field, ValidationError +from rest_framework.serializers import Field, ModelSerializer, ValidationError from users.models import Token @@ -80,6 +80,21 @@ def has_permission(self, request, view): # Serializers # +class ValidatedModelSerializer(ModelSerializer): + """ + Extends the built-in ModelSerializer to enforce calling clean() on the associated model during validation. + """ + def validate(self, attrs): + if self.instance is None: + instance = self.Meta.model(**attrs) + else: + instance = self.instance + for k, v in attrs.items(): + setattr(instance, k, v) + instance.clean() + return attrs + + class ChoiceFieldSerializer(Field): """ Represent a ChoiceField as {'value': , 'label': }. @@ -121,17 +136,6 @@ def to_internal_value(self, data): # Mixins # -class ModelValidationMixin(object): - """ - Enforce a model's validation through clean() when validating serializer data. This is necessary to ensure we're - employing the same validation logic via both forms and the API. - """ - def validate(self, attrs): - instance = self.Meta.model(**attrs) - instance.clean() - return attrs - - class WritableSerializerMixin(object): """ Allow for the use of an alternate, writable serializer class for write operations (e.g. POST, PUT).