[change] REST API cleanup and consistency #1015

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

nemesifier merged 11 commits into master from api-cleanup

May 2, 2025

openwisp_controller/config/api/serializers.py

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -1,15 +1,13 @@
  
    from copy import deepcopy

    from django.core.exceptions import ValidationError

    from django.db import transaction

    from django.db.models import Q

    from django.utils.translation import gettext_lazy as _

    from rest_framework import serializers

    from swapper import load_model

    from openwisp_users.api.mixins import FilterSerializerByOrgManaged

    from openwisp_utils.api.serializers import ValidatedModelSerializer

    from ...serializers import BaseSerializer

    from .. import settings as app_settings

    Template = load_model('config', 'Template')

    @@ -24,10 +22,6 @@ class BaseMeta:
  
        read_only_fields = ['created', 'modified']

    class BaseSerializer(FilterSerializerByOrgManaged, ValidatedModelSerializer):

        pass

    class TemplateSerializer(BaseSerializer):

        config = serializers.JSONField(initial={}, required=False)

        tags = serializers.StringRelatedField(many=True, read_only=True)

    @@ -110,7 +104,12 @@ def get_queryset(self):
  
            return queryset

    class BaseConfigSerializer(serializers.ModelSerializer):

    class BaseConfigSerializer(ValidatedModelSerializer):

        # The device object is excluded from validation

        # because this serializer focuses on validating

        # config objects.

        exclude_validation = ['device']

        class Meta:

            model = Config

            fields = ['status', 'error_reason', 'backend', 'templates', 'context', 'config']

    @@ -119,8 +118,26 @@ class Meta:
  
                'error_reason': {'read_only': True},

            }

        def validate(self, data):

            """

            The validation here is a bit tricky:

            For existing devices, we have to perform the

            model validation of the existing config object,

            because if we simulate the validation on a new

            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).

            """

            device = self.context.get('device')

            if not self.instance and device:

                # Existing device with existing config

                if device._has_config():

                    self.instance = device.config

            return super().validate(data)

    class DeviceConfigMixin(object):

    class DeviceConfigSerializer(BaseSerializer):

        def _get_config_templates(self, config_data):

            return [template.pk for template in config_data.pop('templates', [])]

    @@ -131,11 +148,29 @@ def _prepare_config(self, device, config_data):
  
            config.full_clean()

            return config

        def _is_config_data_relevant(self, config_data):

            """

            Returns True if ``config_data`` does not equal

            the default values and hence the config is useful.

            """

            return not (

                config_data.get('backend') == app_settings.DEFAULT_BACKEND

                and not config_data.get('templates')

                and not config_data.get('context')

                and not config_data.get('config')

            )

        @transaction.atomic

        def _create_config(self, device, config_data):

            config_templates = self._get_config_templates(config_data)

            try:

                if not device._has_config():

                    # if the user hasn't set any useful config data, skip

                    if (

                        not self._is_config_data_relevant(config_data)

                        and not config_templates

                    ):

                        return

                    config = Config(device=device, **config_data)

                    config.full_clean()

                    config.save()

    @@ -151,15 +186,10 @@ def _create_config(self, device, config_data):
  
                raise serializers.ValidationError({'config': error.messages})

        def _update_config(self, device, config_data):

            if (

                config_data.get('backend') == app_settings.DEFAULT_BACKEND

                and not config_data.get('templates')

                and not config_data.get('context')

                and not config_data.get('config')

            ):

                # Do not create Config object if config_data only

                # contains the default value.

                # See https://github.com/openwisp/openwisp-controller/issues/699

            # Do not create Config object if config_data only

            # contains the default values.

            # See https://github.com/openwisp/openwisp-controller/issues/699

            if not self._is_config_data_relevant(config_data):

                return

            if not device._has_config():

                return self._create_config(device, config_data)

    @@ -190,9 +220,7 @@ class DeviceListConfigSerializer(BaseConfigSerializer):
  
        templates = FilterTemplatesByOrganization(many=True, write_only=True)

    class DeviceListSerializer(

        DeviceConfigMixin, FilterSerializerByOrgManaged, serializers.ModelSerializer

    ):

    class DeviceListSerializer(DeviceConfigSerializer):

        config = DeviceListConfigSerializer(required=False)

        class Meta(BaseMeta):

    @@ -219,14 +247,13 @@ class Meta(BaseMeta):
  
                'management_ip': {'allow_blank': True},

            }

        def validate(self, attrs):

            device_data = deepcopy(attrs)

        def validate(self, data):

            # Validation of "config" is performed after

            # device object is created in the "create" method.

            device_data.pop('config', None)

            device = self.instance or self.Meta.model(**device_data)

            device.full_clean()

            return attrs

            config_data = data.pop('config', None)

            data = super().validate(data)

            data['config'] = config_data

            return data

        def create(self, validated_data):

            config_data = validated_data.pop('config', None)

    @@ -247,7 +274,7 @@ class DeviceDetailConfigSerializer(BaseConfigSerializer):
  
        templates = FilterTemplatesByOrganization(many=True)

    class DeviceDetailSerializer(DeviceConfigMixin, BaseSerializer):

    class DeviceDetailSerializer(DeviceConfigSerializer):

        config = DeviceDetailConfigSerializer(allow_null=True)

        is_deactivated = serializers.BooleanField(read_only=True)

    @@ -280,7 +307,7 @@ def update(self, instance, validated_data):
  
            if config_data:

                self._update_config(instance, config_data)

            elif hasattr(instance, 'config') and validated_data.get('organization'):

            elif instance._has_config() and validated_data.get('organization'):

                if instance.organization != validated_data.get('organization'):

                    # config.device.organization is used for validating

                    # the organization of templates. It is also used for adding

openwisp_controller/config/api/urls.py

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -19,12 +19,12 @@ def get_api_urls(api_views):
  
                    name='template_list',

                ),

                path(

                    'controller/template/<str:pk>/',

                    'controller/template/<uuid:pk>/',

                    api_views.template_detail,

                    name='template_detail',

                ),

                path(

                    'controller/template/<str:pk>/configuration/',

                    'controller/template/<uuid:pk>/configuration/',

                    api_download_views.download_template_config,

                    name='download_template_config',

                ),

    @@ -34,12 +34,12 @@ def get_api_urls(api_views):
  
                    name='vpn_list',

                ),

                path(

                    'controller/vpn/<str:pk>/',

                    'controller/vpn/<uuid:pk>/',

                    api_views.vpn_detail,

                    name='vpn_detail',

                ),

                path(

                    'controller/vpn/<str:pk>/configuration/',

                    'controller/vpn/<uuid:pk>/configuration/',

                    api_download_views.download_vpn_config,

                    name='download_vpn_config',

                ),

    @@ -49,17 +49,17 @@ def get_api_urls(api_views):
  
                    name='device_list',

                ),

                path(

                    'controller/device/<str:pk>/',

                    'controller/device/<uuid:pk>/',

                    api_views.device_detail,

                    name='device_detail',

                ),

                path(

                    'controller/device/<str:pk>/activate/',

                    'controller/device/<uuid:pk>/activate/',

                    api_views.device_activate,

                    name='device_activate',

                ),

                path(

                    'controller/device/<str:pk>/deactivate/',

                    'controller/device/<uuid:pk>/deactivate/',

                    api_views.device_deactivate,

                    name='device_deactivate',

                ),

    @@ -69,7 +69,7 @@ def get_api_urls(api_views):
  
                    name='devicegroup_list',

                ),

                path(

                    'controller/group/<str:pk>/',

                    'controller/group/<uuid:pk>/',

                    api_views.devicegroup_detail,

                    name='devicegroup_detail',

                ),

    @@ -79,7 +79,7 @@ def get_api_urls(api_views):
  
                    name='devicegroup_x509_commonname',

                ),

                path(

                    'controller/device/<str:pk>/configuration/',

                    'controller/device/<uuid:pk>/configuration/',

                    api_download_views.download_device_config,

                    name='download_device_config',

                ),

openwisp_controller/config/api/views.py

-Original file line number
+Diff line change
@@ Expand Up / @@ -111,6 +111,18 @@ def perform_destroy(self, instance): @@
             force_deletion = self.request.query_params.get('force', None) == 'true'
             instance.delete(check_deactivated=(not force_deletion))
+        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):
         serializer_class = serializers.Serializer
@@ Expand Down @@

openwisp_controller/config/base/config.py

-Original file line number
+Diff line change
@@ Expand Up / @@ -469,13 +469,13 @@ def clean(self): @@
             * modifies status if key attributes of the configuration
               have changed (queries the database)
             """
-            super().clean()
             if not self.context:
                 self.context = {}
             if not isinstance(self.context, dict):
                 raise ValidationError(
                     {'context': _('the supplied value is not a JSON object')}
                 )
+            super().clean()
         def save(self, *args, **kwargs):
             created = self._state.adding
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[change] REST API cleanup and consistency #1015

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!