[chores] Clean up

Author: nemesifier

.github/workflows/ci.yml

@@ -76,7 +76,6 @@ jobs:
         pip install -U -r requirements-test.txt
         pip install -U -e .
         pip install ${{ matrix.django-version }}
-        pip install -U --force-reinstall --no-deps https://github.com/openwisp/openwisp-utils/tarball/exclude_validation
 
     - name: Start postgres and redis
       if: ${{ !cancelled() && steps.deps.conclusion == 'success' }}

openwisp_controller/config/api/serializers.py

@@ -1,5 +1,3 @@
-from copy import deepcopy
-
 from django.core.exceptions import ValidationError
 from django.db import transaction
 from django.db.models import Q
@@ -9,7 +7,7 @@
 
 from openwisp_utils.api.serializers import ValidatedModelSerializer
 
-from ...serializers import BaseSerializer, ValidatedDeviceIdSerializer
+from ...serializers import BaseSerializer
 from .. import settings as app_settings
 
 Template = load_model('config', 'Template')
@@ -106,7 +104,10 @@ def get_queryset(self):
         return queryset
 
 
-class BaseConfigSerializer(ValidatedDeviceIdSerializer):
+class BaseConfigSerializer(ValidatedModelSerializer):
+    # The device object is excluded from validation
+    # because this serializer focuses on validating
+    # config objects.
     exclude_validation = ['device']
 
     class Meta:
@@ -127,22 +128,12 @@ def validate(self, data):
         config object pointing to an existing device,
         the validation will fail because a config object
         for this device already exists (due to one-to-one relationship).
-
-        For new devices, the device hasn't been created yet,
-        so we have to exclude the `device` field from validation.
         """
-        # Existing device
         device = self.context.get('device')
-        # data.pop('device', None)
-        # import ipdb; ipdb.set_trace()
         if not self.instance and device:
             # Existing device with existing config
-            # Or it's an exsiting device with a new config
             if device._has_config():
                 self.instance = device.config
-            # return super().validate(data)
-        # New device
-        # self.exclude_validation = ['device']
         return super().validate(data)
 
 
@@ -216,10 +207,6 @@ def _update_config(self, device, config_data):
         except ValidationError as error:
             raise serializers.ValidationError({'config': error.messages})
 
-    # def run_validation(self, *args, **kwargs):
-    #     import ipdb; ipdb.set_trace()
-    #     return super().run_validation(*args, **kwargs)
-
 
 class DeviceListConfigSerializer(BaseConfigSerializer):
     config = serializers.JSONField(
@@ -286,10 +273,6 @@ class DeviceDetailConfigSerializer(BaseConfigSerializer):
     )
     templates = FilterTemplatesByOrganization(many=True)
 
-    # def validate(self, *args, **kwargs):
-    #     import ipdb; ipdb.set_trace()
-    #     return super().validate(*args, **kwargs)
-
 
 class DeviceDetailSerializer(DeviceConfigSerializer):
     config = DeviceDetailConfigSerializer(allow_null=True)
@@ -316,10 +299,6 @@ class Meta(BaseMeta):
             'modified',
         ]
 
-    # def to_internal_value(self, *args, **kwargs):
-    #     import ipdb; ipdb.set_trace()
-    #     return super().to_internal_value(*args, **kwargs)
-
     def update(self, instance, validated_data):
         config_data = validated_data.pop('config', {})
         raw_data_for_signal_handlers = {

openwisp_controller/config/api/views.py

@@ -114,20 +114,17 @@ def perform_destroy(self, instance):
         force_deletion = self.request.query_params.get('force', None) == 'true'
         instance.delete(check_deactivated=(not force_deletion))
 
-    # def update(self, request, *args, **kwargs):
-    #     import ipdb; ipdb.set_trace()
-    #     partial = kwargs.pop('partial', False)
-    #     instance = self.get_object()
-    #     serializer = self.get_serializer(instance, data=request.data, partial=partial)
-    #     serializer.is_valid(raise_exception=True)
-    #     self.perform_update(serializer)
-    #
-    #     if getattr(instance, '_prefetched_objects_cache', None):
-    #         # If 'prefetch_related' has been applied to a queryset, we need to
-    #         # forcibly invalidate the prefetch cache on the instance.
-    #         instance._prefetched_objects_cache = {}
-    #
-    #     return Response(serializer.data)
+    def get_object(self):
+        """Set device property for serializer context."""
+        obj = super().get_object()
+        self.device = obj
+        return obj
+
+    def get_serializer_context(self):
+        """Add device to serializer context for validation purposes."""
+        context = super().get_serializer_context()
+        context['device'] = self.device
+        return context
 
 
 class DeviceActivateView(ProtectedAPIMixin, GenericAPIView):

openwisp_controller/connection/api/serializers.py

@@ -1,19 +1,25 @@
 from rest_framework import serializers
 from swapper import load_model
 
-from ...serializers import (
-    BaseDeviceIdSerializer,
-    BaseSerializer,
-    ValidatedDeviceIdSerializer,
-)
+from openwisp_users.api.mixins import FilterSerializerByOrgManaged
+from openwisp_utils.api.serializers import ValidatedModelSerializer
+
+from ...serializers import BaseSerializer
 
 Command = load_model('connection', 'Command')
 DeviceConnection = load_model('connection', 'DeviceConnection')
 Credentials = load_model('connection', 'Credentials')
 Device = load_model('config', 'Device')
 
 
-class CommandSerializer(ValidatedDeviceIdSerializer):
+class ValidatedDeviceFieldSerializer(ValidatedModelSerializer):
+    def validate(self, data):
+        # Add "device_id" to the data for validation
+        data['device_id'] = self.context['device_id']
+        return super().validate(data)
+
+
+class CommandSerializer(ValidatedDeviceFieldSerializer):
     input = serializers.JSONField(allow_null=True)
     device = serializers.PrimaryKeyRelatedField(
         read_only=True, pk_field=serializers.UUIDField(format='hex_verbose')
@@ -61,7 +67,9 @@ class Meta:
         read_only_fields = ('created', 'modified')
 
 
-class DeviceConnectionSerializer(BaseDeviceIdSerializer):
+class DeviceConnectionSerializer(
+    FilterSerializerByOrgManaged, ValidatedDeviceFieldSerializer
+):
     class Meta:
         model = DeviceConnection
         fields = (
@@ -81,9 +89,3 @@ class Meta:
             'is_working': {'read_only': True},
         }
         read_only_fields = ('created', 'modified')
-
-    # def validate(self, data):
-    #     data['device'] = Device.objects.get(pk=self.context['device_id'])
-    #     instance = self.instance or self.Meta.model(**data)
-    #     instance.full_clean()
-    #     return data

openwisp_controller/connection/base/models.py

@@ -458,6 +458,18 @@ def __str__(self):
         return f'«{command}» {sent} {created.strftime("%d %b %Y at %I:%M %p")}'
 
     def clean(self):
+        self._verify_command_type_allowed()
+        try:
+            jsonschema.Draft4Validator(self._schema).validate(self.input)
+        except SchemaError as e:
+            raise ValidationError({'input': e.message})
+
+    def _verify_command_type_allowed(self):
+        """Raises validation error if command type is not allowed."""
+        # if device is not set, skip to avoid uncaught exception
+        # (standard model validation will kick in)
+        if not hasattr(self, 'device'):
+            return
         if self.type not in self.get_org_choices(
             organization_id=self.device.organization_id
         ):
@@ -471,10 +483,6 @@ def clean(self):
                     )
                 }
             )
-        try:
-            jsonschema.Draft4Validator(self._schema).validate(self.input)
-        except SchemaError as e:
-            raise ValidationError({'input': e.message})
 
     @property
     def is_custom(self):

openwisp_controller/connection/tests/test_api.py

@@ -503,7 +503,7 @@ def test_put_devceconnection_detail(self):
             'enabled': False,
             'failure_reason': '',
         }
-        with self.assertNumQueries(15):
+        with self.assertNumQueries(14):
             response = self.client.put(path, data, content_type='application/json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(
@@ -517,7 +517,7 @@ def test_patch_deviceconnectoin_detail(self):
         path = reverse('connection_api:deviceconnection_detail', args=(d1, dc.pk))
         self.assertEqual(dc.update_strategy, app_settings.UPDATE_STRATEGIES[0][0])
         data = {'update_strategy': app_settings.UPDATE_STRATEGIES[1][0]}
-        with self.assertNumQueries(14):
+        with self.assertNumQueries(13):
             response = self.client.patch(path, data, content_type='application/json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(

openwisp_controller/serializers.py

@@ -2,47 +2,11 @@
 from openwisp_utils.api.serializers import ValidatedModelSerializer
 
 
-class ValidatedDeviceIdSerializer(ValidatedModelSerializer):
-    pass
-    # def validate(self, data):
-    #     """Adds "device" to the data dictionary.
-    #
-    #     Used to satisfy validation needs."""
-    #     # for key in ['device', 'device_id']:
-    #     #     if key in self.context:
-    #     #         data[key] = self.context[key]
-    #     if 'device' in self.context:
-    #         data['device'] = self.context['device']
-    #     data = super().validate(data)
-    #     data.pop('device', None)
-    #     return data
-
-
 class BaseSerializer(FilterSerializerByOrgManaged, ValidatedModelSerializer):
-    """
-    TODO
-    """
-
-    pass
+    """BaseSerializer for most API endpoints.
 
-
-class BaseDeviceIdSerializer(FilterSerializerByOrgManaged, ValidatedDeviceIdSerializer):
-    """
-    TODO
+    - FilterSerializerByOrgManaged: for multi-tenancy
+    - ValidatedModelSerializer: for model validation
     """
 
     pass
-
-
-class DeviceContextMixin:
-    """Adds the device object to the serializer context."""
-
-    def get_object(self):
-        obj = super().get_object()
-        self.object = obj
-        return obj
-
-    def get_serializer_context(self):
-        context = super().get_serializer_context()
-        context['device'] = self.object
-        return context