From ffb86de085392bbdeaff3360bcefee7b95b2f956 Mon Sep 17 00:00:00 2001 From: dee077 Date: Tue, 9 Jun 2026 18:28:35 +0530 Subject: [PATCH 1/8] [feature] Added BatchCommand model for mass command execution #1344 Introduced AbstractBatchCommand model with calculate_and_update_status() and launch() methods to support batch command execution on multiple devices, following the pattern of BatchUpgradeOperation in openwisp-firmware-upgrader. Added batch_command FK to the existing Command model to link individual commands to their parent batch. Closes #1344 --- openwisp_controller/connection/base/models.py | 174 ++++++++++++++++++ openwisp_controller/connection/models.py | 13 +- openwisp_controller/connection/tasks.py | 10 + 3 files changed, 196 insertions(+), 1 deletion(-) diff --git a/openwisp_controller/connection/base/models.py b/openwisp_controller/connection/base/models.py index 17a06f7bb..04f86ad41 100644 --- a/openwisp_controller/connection/base/models.py +++ b/openwisp_controller/connection/base/models.py @@ -467,6 +467,12 @@ class AbstractCommand(TimeStampedEditableModel): encoder=DjangoJSONEncoder, ) output = models.TextField(blank=True) + batch_command = models.ForeignKey( + get_model_name("connection", "BatchCommand"), + on_delete=models.SET_NULL, + blank=True, + null=True, + ) class Meta: verbose_name = _("Command") @@ -719,3 +725,171 @@ def _enforce_not_custom(self): f"arguments property is not applicable in " f'command instance of type "{self.type}"' ) + + +class AbstractBatchCommand(TimeStampedEditableModel): + STATUS_CHOICES = ( + ("idle", _("idle")), + ("in-progress", _("in progress")), + ("success", _("completed successfully")), + ("failed", _("completed with some failures")), + ("cancelled", _("completed with some cancellations")), + ) + organization = models.ForeignKey( + get_model_name("openwisp_users", "Organization"), + on_delete=models.CASCADE, + ) + status = models.CharField( + max_length=12, choices=STATUS_CHOICES, default=STATUS_CHOICES[0][0] + ) + command_type = models.CharField( + max_length=16, + choices=(COMMAND_CHOICES if django.VERSION < (5, 0) else get_command_choices), + ) + command_input = JSONField(blank=True, null=True, encoder=DjangoJSONEncoder) + group = models.ForeignKey( + get_model_name("config", "DeviceGroup"), + on_delete=models.SET_NULL, + blank=True, + null=True, + verbose_name=_("device group"), + ) + location = models.ForeignKey( + get_model_name("geo", "Location"), + on_delete=models.SET_NULL, + blank=True, + null=True, + verbose_name=_("location"), + ) + include_all_devices = models.BooleanField(default=False) + total_devices = models.PositiveIntegerField(default=0) + successful = models.PositiveIntegerField(default=0) + failed = models.PositiveIntegerField(default=0) + cancelled = models.PositiveIntegerField(default=0) + + class Meta: + abstract = True + verbose_name = _("Batch command operation") + verbose_name_plural = _("Batch command operations") + + def clean(self): + super().clean() + if self.group and self.group.organization != self.organization: + raise ValidationError( + { + "group": _( + "The organization of the group doesn't match " + "the organization of the batch command operation" + ) + } + ) + if self.location and self.location.organization != self.organization: + raise ValidationError( + { + "location": _( + "The organization of the location doesn't match " + "the organization of the batch command operation" + ) + } + ) + allowed = dict( + AbstractCommand.get_org_allowed_commands( + organization_id=self.organization_id + ) + ) + if self.command_type not in allowed: + raise ValidationError( + { + "command_type": _( + '"{command}" command is not available ' "for this organization" + ).format(command=self.command_type) + } + ) + try: + jsonschema.Draft4Validator(get_command_schema(self.command_type)).validate( + self.command_input + ) + except SchemaError as e: + raise ValidationError({"command_input": e.message}) + + def resolve_devices(self): + Device = load_model("config", "Device") + qs = Device.objects.filter(organization=self.organization) + if self.group: + qs = qs.filter(group=self.group) + if self.location: + qs = qs.filter(location=self.location) + if not self.include_all_devices and not self.group and not self.location: + qs = qs.none() + return qs + + def launch(self): + self.status = "in-progress" + self.save() + devices = self.resolve_devices() + Command = load_model("connection", "Command") + count = 0 + for device in devices.iterator(): + cmd = Command( + device=device, + type=self.command_type, + input=self.command_input, + batch_command=self, + ) + cmd.full_clean() + cmd.save() + count += 1 + self.total_devices = count + self.save(update_fields=["total_devices"]) + self.calculate_and_update_status() + + def launch_async(self): + self.status = "in-progress" + self.save(update_fields=["status"]) + from ..tasks import launch_batch_command + + transaction.on_commit(lambda: launch_batch_command.delay(self.pk)) + + def calculate_and_update_status(self): + Command = load_model("connection", "Command") + operations = Command.objects.filter(batch_command=self) + stats = operations.aggregate( + total_operations=models.Count("id"), + in_progress=models.Count( + models.Case( + models.When(status="in-progress", then=1), + output_field=models.IntegerField(), + ) + ), + completed=models.Count( + models.Case( + models.When(~models.Q(status="in-progress"), then=1), + output_field=models.IntegerField(), + ) + ), + successful=models.Count( + models.Case( + models.When(status="success", then=1), + output_field=models.IntegerField(), + ) + ), + failed=models.Count( + models.Case( + models.When(status="failed", then=1), + output_field=models.IntegerField(), + ) + ), + ) + self.successful = stats["successful"] + self.failed = stats["failed"] + if stats["total_operations"] == 0: + self.status = "idle" + elif stats["in_progress"] > 0: + self.status = "in-progress" + elif stats["failed"] > 0: + self.status = "failed" + elif ( + stats["successful"] > 0 and stats["completed"] == stats["total_operations"] + ): + self.status = "success" + self.save(update_fields=["status", "successful", "failed"]) diff --git a/openwisp_controller/connection/models.py b/openwisp_controller/connection/models.py index 39d485b57..533ad6c4d 100644 --- a/openwisp_controller/connection/models.py +++ b/openwisp_controller/connection/models.py @@ -1,6 +1,11 @@ import swapper -from .base.models import AbstractCommand, AbstractCredentials, AbstractDeviceConnection +from .base.models import ( + AbstractBatchCommand, + AbstractCommand, + AbstractCredentials, + AbstractDeviceConnection, +) class Credentials(AbstractCredentials): @@ -19,3 +24,9 @@ class Command(AbstractCommand): class Meta(AbstractCommand.Meta): abstract = False swappable = swapper.swappable_setting("connection", "Command") + + +class BatchCommand(AbstractBatchCommand): + class Meta(AbstractBatchCommand.Meta): + abstract = False + swappable = swapper.swappable_setting("connection", "BatchCommand") diff --git a/openwisp_controller/connection/tasks.py b/openwisp_controller/connection/tasks.py index d6ea2828b..7d65a7038 100644 --- a/openwisp_controller/connection/tasks.py +++ b/openwisp_controller/connection/tasks.py @@ -98,6 +98,16 @@ def launch_command(command_id): command._save_without_resurrecting() +@shared_task(bind=True, soft_time_limit=3600) +def launch_batch_command(self, batch_id): + BatchCommand = load_model("connection", "BatchCommand") + try: + batch = BatchCommand.objects.get(pk=batch_id) + batch.launch() + except ObjectDoesNotExist: + logger.warning(f"The BatchCommand object with id {batch_id} has been deleted") + + @shared_task(soft_time_limit=3600) def auto_add_credentials_to_devices(credential_id, organization_id): Credentials = load_model("connection", "Credentials") From 76982cd66f803036f947b93051da1d9b2fa14be9 Mon Sep 17 00:00:00 2001 From: dee077 Date: Sat, 13 Jun 2026 01:42:46 +0530 Subject: [PATCH 2/8] [feature] Proof of concept need some code refinement --- .../connection/api/serializers.py | 56 +++++++ openwisp_controller/connection/api/urls.py | 5 + openwisp_controller/connection/api/views.py | 34 +++++ openwisp_controller/connection/base/models.py | 70 ++++++--- ...0011_batchcommand_command_batch_command.py | 142 ++++++++++++++++++ openwisp_controller/connection/tasks.py | 10 +- 6 files changed, 293 insertions(+), 24 deletions(-) create mode 100644 openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py diff --git a/openwisp_controller/connection/api/serializers.py b/openwisp_controller/connection/api/serializers.py index 142c1c30a..32af819b0 100644 --- a/openwisp_controller/connection/api/serializers.py +++ b/openwisp_controller/connection/api/serializers.py @@ -12,6 +12,7 @@ DeviceConnection = load_model("connection", "DeviceConnection") Credentials = load_model("connection", "Credentials") Device = load_model("config", "Device") +BatchCommand = load_model("connection", "BatchCommand") class ValidatedDeviceFieldSerializer(ValidatedModelSerializer): @@ -43,6 +44,10 @@ class CommandSerializer(ValidatedDeviceFieldSerializer): required=False, pk_field=serializers.UUIDField(format="hex_verbose"), ) + batch_command = serializers.PrimaryKeyRelatedField( + read_only=True, + pk_field=serializers.UUIDField(format="hex_verbose"), + ) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -115,3 +120,54 @@ class Meta: "is_working": {"read_only": True}, } read_only_fields = ("created", "modified") + + +class BatchCommandExecuteSerializer( + FilterSerializerByOrgManaged, serializers.ModelSerializer +): + type = serializers.CharField(source="command_type") + input = serializers.JSONField( + source="command_input", allow_null=True, required=False + ) + devices = serializers.PrimaryKeyRelatedField( + many=True, + queryset=Device.objects.all(), + required=False, + allow_empty=True, + pk_field=serializers.UUIDField(format="hex_verbose"), + ) + + class Meta: + model = BatchCommand + fields = ( + "organization", + "type", + "input", + "devices", + "group", + "location", + ) + extra_kwargs = { + "organization": {"required": False, "allow_null": True}, + } + + def validate(self, data): + if ( + not data.get("organization") + and not self.context["request"].user.is_superuser + ): + raise serializers.ValidationError( + _("Only superusers can execute batch commands without an organization.") + ) + if devices := data.get("devices"): + org = data.get("organization") + for device in devices: + if org and device.organization_id != org.id: + raise serializers.ValidationError( + { + "devices": _( + "All devices must belong to the same organization." + ) + } + ) + return data diff --git a/openwisp_controller/connection/api/urls.py b/openwisp_controller/connection/api/urls.py index 4ec3e70ab..76a30c64a 100644 --- a/openwisp_controller/connection/api/urls.py +++ b/openwisp_controller/connection/api/urls.py @@ -40,6 +40,11 @@ def get_api_urls(api_views): api_views.deviceconnection_detail_view, name="deviceconnection_detail", ), + path( + "api/v1/controller/batch-command/execute/", + api_views.batch_command_execute_view, + name="batch_command_execute", + ), ] diff --git a/openwisp_controller/connection/api/views.py b/openwisp_controller/connection/api/views.py index 6af1270c7..ca88355d0 100644 --- a/openwisp_controller/connection/api/views.py +++ b/openwisp_controller/connection/api/views.py @@ -1,12 +1,15 @@ from django.utils.translation import gettext_lazy as _ from drf_yasg import openapi from drf_yasg.utils import swagger_auto_schema +from rest_framework import status from rest_framework.generics import ( + GenericAPIView, ListCreateAPIView, RetrieveAPIView, RetrieveUpdateDestroyAPIView, get_object_or_404, ) +from rest_framework.response import Response from swapper import load_model from openwisp_utils.api.pagination import OpenWispPagination @@ -17,6 +20,7 @@ RelatedDeviceProtectedAPIMixin, ) from .serializers import ( + BatchCommandExecuteSerializer, CommandSerializer, CredentialSerializer, DeviceConnectionSerializer, @@ -26,6 +30,7 @@ Device = load_model("config", "Device") Credentials = load_model("connection", "Credentials") DeviceConnection = load_model("connection", "DeviceConnection") +BatchCommand = load_model("connection", "BatchCommand") class BaseCommandView(RelatedDeviceProtectedAPIMixin): @@ -138,6 +143,33 @@ class DeviceConnectionListCreateView(BaseDeviceConnection, ListCreateAPIView): DeviceConnenctionListCreateView = DeviceConnectionListCreateView +class BatchCommandExecuteView(ProtectedAPIMixin, GenericAPIView): + model = BatchCommand + queryset = BatchCommand.objects.all() + serializer_class = BatchCommandExecuteSerializer + + def post(self, request): + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + batch = serializer.save() + batch.launch_async() + return Response({"batch": str(batch.pk)}, status=status.HTTP_201_CREATED) + + def get(self, request): + serializer = self.get_serializer(data=request.query_params) + serializer.is_valid(raise_exception=True) + data = serializer.validated_data + device_pks = [] + devices_list = data.pop("devices", None) + if devices_list: + device_pks = [str(d.pk) for d in devices_list] + batch = BatchCommand(**data) + if not device_pks: + resolved = batch.resolve_devices() + device_pks = [str(d.pk) for d in resolved] + return Response({"devices": device_pks}) + + class DeviceConnectionDetailView(BaseDeviceConnection, RetrieveUpdateDestroyAPIView): def get_object(self): queryset = self.filter_queryset(self.get_queryset()) @@ -158,3 +190,5 @@ def get_object(self): # TODO: remove in version 1.4 deviceconnection_details_view = deviceconnection_detail_view + +batch_command_execute_view = BatchCommandExecuteView.as_view() diff --git a/openwisp_controller/connection/base/models.py b/openwisp_controller/connection/base/models.py index 04f86ad41..a6c48c8c8 100644 --- a/openwisp_controller/connection/base/models.py +++ b/openwisp_controller/connection/base/models.py @@ -563,6 +563,8 @@ def save(self, *args, **kwargs): output = super().save(*args, **kwargs) if adding: self._schedule_command() + if self.batch_command_id and self.status != "in-progress": + self.batch_command.calculate_and_update_status() return output def _save_without_resurrecting(self): @@ -738,6 +740,8 @@ class AbstractBatchCommand(TimeStampedEditableModel): organization = models.ForeignKey( get_model_name("openwisp_users", "Organization"), on_delete=models.CASCADE, + blank=True, + null=True, ) status = models.CharField( max_length=12, choices=STATUS_CHOICES, default=STATUS_CHOICES[0][0] @@ -761,7 +765,11 @@ class AbstractBatchCommand(TimeStampedEditableModel): null=True, verbose_name=_("location"), ) - include_all_devices = models.BooleanField(default=False) + devices = models.ManyToManyField( + get_model_name("config", "Device"), + blank=True, + verbose_name=_("devices"), + ) total_devices = models.PositiveIntegerField(default=0) successful = models.PositiveIntegerField(default=0) failed = models.PositiveIntegerField(default=0) @@ -769,29 +777,43 @@ class AbstractBatchCommand(TimeStampedEditableModel): class Meta: abstract = True - verbose_name = _("Batch command operation") - verbose_name_plural = _("Batch command operations") + verbose_name = _("Batch command") + verbose_name_plural = _("Batch commands") def clean(self): super().clean() - if self.group and self.group.organization != self.organization: - raise ValidationError( - { - "group": _( - "The organization of the group doesn't match " - "the organization of the batch command operation" - ) - } - ) - if self.location and self.location.organization != self.organization: - raise ValidationError( - { - "location": _( - "The organization of the location doesn't match " - "the organization of the batch command operation" + if self.organization_id: + if self.group and self.group.organization != self.organization: + raise ValidationError( + { + "group": _( + "The organization of the group doesn't match " + "the organization of the batch command operation" + ) + } + ) + if self.location and self.location.organization != self.organization: + raise ValidationError( + { + "location": _( + "The organization of the location doesn't match " + "the organization of the batch command operation" + ) + } + ) + if self.pk and self.devices.exists(): + org_mismatch = self.devices.exclude( + organization=self.organization + ).exists() + if org_mismatch: + raise ValidationError( + { + "devices": _( + "All devices must belong to the same " + "organization as the batch command." + ) + } ) - } - ) allowed = dict( AbstractCommand.get_org_allowed_commands( organization_id=self.organization_id @@ -813,14 +835,16 @@ def clean(self): raise ValidationError({"command_input": e.message}) def resolve_devices(self): + if self.pk and self.devices.exists(): + return self.devices.all() Device = load_model("config", "Device") - qs = Device.objects.filter(organization=self.organization) + qs = Device.objects.all() + if self.organization_id: + qs = qs.filter(organization=self.organization) if self.group: qs = qs.filter(group=self.group) if self.location: qs = qs.filter(location=self.location) - if not self.include_all_devices and not self.group and not self.location: - qs = qs.none() return qs def launch(self): diff --git a/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py b/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py new file mode 100644 index 000000000..a0dad8b92 --- /dev/null +++ b/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py @@ -0,0 +1,142 @@ +# Generated by Django 5.2.15 on 2026-06-09 22:36 + +import uuid + +import django.core.serializers.json +import django.db.models.deletion +import django.utils.timezone +import model_utils.fields +from django.conf import settings +from django.db import migrations, models + +import openwisp_controller.connection.commands + + +class Migration(migrations.Migration): + + dependencies = [ + ("connection", "0010_replace_jsonfield_with_django_builtin"), + ("openwisp_users", "0022_user_expiration_date"), + migrations.swappable_dependency(settings.CONFIG_DEVICEGROUP_MODEL), + migrations.swappable_dependency(settings.CONFIG_DEVICE_MODEL), + migrations.swappable_dependency(settings.GEO_LOCATION_MODEL), + ] + + operations = [ + migrations.CreateModel( + name="BatchCommand", + fields=[ + ( + "id", + models.UUIDField( + default=uuid.uuid4, + editable=False, + primary_key=True, + serialize=False, + ), + ), + ( + "created", + model_utils.fields.AutoCreatedField( + default=django.utils.timezone.now, + editable=False, + verbose_name="created", + ), + ), + ( + "modified", + model_utils.fields.AutoLastModifiedField( + default=django.utils.timezone.now, + editable=False, + verbose_name="modified", + ), + ), + ( + "status", + models.CharField( + choices=[ + ("idle", "idle"), + ("in-progress", "in progress"), + ("success", "completed successfully"), + ("failed", "completed with some failures"), + ("cancelled", "completed with some cancellations"), + ], + default="idle", + max_length=12, + ), + ), + ( + "command_type", + models.CharField( + choices=openwisp_controller.connection.commands.get_command_choices, + max_length=16, + ), + ), + ( + "command_input", + models.JSONField( + blank=True, + encoder=django.core.serializers.json.DjangoJSONEncoder, + null=True, + ), + ), + ("total_devices", models.PositiveIntegerField(default=0)), + ("successful", models.PositiveIntegerField(default=0)), + ("failed", models.PositiveIntegerField(default=0)), + ("cancelled", models.PositiveIntegerField(default=0)), + ( + "devices", + models.ManyToManyField( + blank=True, + to=settings.CONFIG_DEVICE_MODEL, + verbose_name="devices", + ), + ), + ( + "group", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to=settings.CONFIG_DEVICEGROUP_MODEL, + verbose_name="device group", + ), + ), + ( + "location", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to=settings.GEO_LOCATION_MODEL, + verbose_name="location", + ), + ), + ( + "organization", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="openwisp_users.organization", + ), + ), + ], + options={ + "verbose_name": "Batch command operation", + "verbose_name_plural": "Batch command operations", + "abstract": False, + "swappable": "CONNECTION_BATCHCOMMAND_MODEL", + }, + ), + migrations.AddField( + model_name="command", + name="batch_command", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to=settings.CONNECTION_BATCHCOMMAND_MODEL, + ), + ), + ] diff --git a/openwisp_controller/connection/tasks.py b/openwisp_controller/connection/tasks.py index 7d65a7038..b3b169c40 100644 --- a/openwisp_controller/connection/tasks.py +++ b/openwisp_controller/connection/tasks.py @@ -81,6 +81,14 @@ def launch_command(command_id): return try: command.execute() + # Todo: Remove once demo is completed + if command.batch_command_id: + print("****************************") + print(f"Device: {command.device.name}") + print(f"Status: {command.status}") + print("") + print(command.output) + print("****************************") except SoftTimeLimitExceeded: command.status = "failed" command._add_output(_("Background task time limit exceeded.")) @@ -105,7 +113,7 @@ def launch_batch_command(self, batch_id): batch = BatchCommand.objects.get(pk=batch_id) batch.launch() except ObjectDoesNotExist: - logger.warning(f"The BatchCommand object with id {batch_id} has been deleted") + logger.warning(f"The BatchCommand object with id {batch_id} not foound") @shared_task(soft_time_limit=3600) From 82f104ca42b82ac258f7cc2156ef20dc7c9da97e Mon Sep 17 00:00:00 2001 From: dee077 Date: Mon, 15 Jun 2026 00:06:51 +0530 Subject: [PATCH 3/8] [feature] Refactored BatchCommand model, API, and task for mass command execution - Removed counter DB fields - Added computed properties via aggregation - Added execute_all boolean field - Renamed launch() to create_commands() - Added execute() and dry_run() classmethods - Updated calculate_and_update_status() - Made organization FK nullable - Updated views with execute/dry_run - Updated serializer with execute_all and type/input aliases - Updated migration and celery task --- .../connection/api/serializers.py | 22 +++-- openwisp_controller/connection/api/views.py | 29 +++--- openwisp_controller/connection/base/models.py | 93 +++++++++++++------ ...0011_batchcommand_command_batch_command.py | 18 ++-- openwisp_controller/connection/tasks.py | 2 +- 5 files changed, 102 insertions(+), 62 deletions(-) diff --git a/openwisp_controller/connection/api/serializers.py b/openwisp_controller/connection/api/serializers.py index 32af819b0..77f9a5968 100644 --- a/openwisp_controller/connection/api/serializers.py +++ b/openwisp_controller/connection/api/serializers.py @@ -136,6 +136,7 @@ class BatchCommandExecuteSerializer( allow_empty=True, pk_field=serializers.UUIDField(format="hex_verbose"), ) + execute_all = serializers.BooleanField(required=False, default=False) class Meta: model = BatchCommand @@ -146,21 +147,30 @@ class Meta: "devices", "group", "location", + "execute_all", ) extra_kwargs = { "organization": {"required": False, "allow_null": True}, } def validate(self, data): - if ( - not data.get("organization") - and not self.context["request"].user.is_superuser - ): + org = data.get("organization") + execute_all = data.get("execute_all", False) + devices = data.get("devices") + group = data.get("group") + location = data.get("location") + if not org and not self.context["request"].user.is_superuser: raise serializers.ValidationError( _("Only superusers can execute batch commands without an organization.") ) - if devices := data.get("devices"): - org = data.get("organization") + if not execute_all and not org and not devices and not group and not location: + raise serializers.ValidationError( + _( + "Specify at least one targeting option " + "or set execute_all to true." + ) + ) + if devices: for device in devices: if org and device.organization_id != org.id: raise serializers.ValidationError( diff --git a/openwisp_controller/connection/api/views.py b/openwisp_controller/connection/api/views.py index ca88355d0..8628eeb4d 100644 --- a/openwisp_controller/connection/api/views.py +++ b/openwisp_controller/connection/api/views.py @@ -1,3 +1,4 @@ +from django.core.exceptions import ValidationError from django.utils.translation import gettext_lazy as _ from drf_yasg import openapi from drf_yasg.utils import swagger_auto_schema @@ -151,23 +152,25 @@ class BatchCommandExecuteView(ProtectedAPIMixin, GenericAPIView): def post(self, request): serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) - batch = serializer.save() - batch.launch_async() - return Response({"batch": str(batch.pk)}, status=status.HTTP_201_CREATED) + try: + batch = BatchCommand.execute(**serializer.validated_data) + except ValidationError as e: + return Response( + {"error": str(e.messages[0])}, status=status.HTTP_400_BAD_REQUEST + ) + return Response({"batch": str(batch.pk)}, status=201) def get(self, request): serializer = self.get_serializer(data=request.query_params) serializer.is_valid(raise_exception=True) - data = serializer.validated_data - device_pks = [] - devices_list = data.pop("devices", None) - if devices_list: - device_pks = [str(d.pk) for d in devices_list] - batch = BatchCommand(**data) - if not device_pks: - resolved = batch.resolve_devices() - device_pks = [str(d.pk) for d in resolved] - return Response({"devices": device_pks}) + try: + data = BatchCommand.dry_run(**serializer.validated_data) + except ValidationError as e: + return Response( + {"error": str(e.messages[0])}, status=status.HTTP_400_BAD_REQUEST + ) + data["devices"] = [str(d.pk) for d in data["devices"]] + return Response(data) class DeviceConnectionDetailView(BaseDeviceConnection, RetrieveUpdateDestroyAPIView): diff --git a/openwisp_controller/connection/base/models.py b/openwisp_controller/connection/base/models.py index a6c48c8c8..2ce23f676 100644 --- a/openwisp_controller/connection/base/models.py +++ b/openwisp_controller/connection/base/models.py @@ -30,7 +30,11 @@ ) from ..exceptions import NoWorkingDeviceConnectionError from ..signals import is_working_changed -from ..tasks import auto_add_credentials_to_devices, launch_command +from ..tasks import ( + auto_add_credentials_to_devices, + launch_batch_command, + launch_command, +) logger = logging.getLogger(__name__) @@ -735,8 +739,8 @@ class AbstractBatchCommand(TimeStampedEditableModel): ("in-progress", _("in progress")), ("success", _("completed successfully")), ("failed", _("completed with some failures")), - ("cancelled", _("completed with some cancellations")), ) + organization = models.ForeignKey( get_model_name("openwisp_users", "Organization"), on_delete=models.CASCADE, @@ -770,16 +774,28 @@ class AbstractBatchCommand(TimeStampedEditableModel): blank=True, verbose_name=_("devices"), ) - total_devices = models.PositiveIntegerField(default=0) - successful = models.PositiveIntegerField(default=0) - failed = models.PositiveIntegerField(default=0) - cancelled = models.PositiveIntegerField(default=0) + execute_all = models.BooleanField(default=False) class Meta: abstract = True verbose_name = _("Batch command") verbose_name_plural = _("Batch commands") + @cached_property + def total_devices(self): + Command = load_model("connection", "Command") + return Command.objects.filter(batch_command=self).count() + + @property + def successful(self): + Command = load_model("connection", "Command") + return Command.objects.filter(batch_command=self, status="success").count() + + @property + def failed(self): + Command = load_model("connection", "Command") + return Command.objects.filter(batch_command=self, status="failed").count() + def clean(self): super().clean() if self.organization_id: @@ -841,39 +857,54 @@ def resolve_devices(self): qs = Device.objects.all() if self.organization_id: qs = qs.filter(organization=self.organization) + if self.execute_all: + return qs if self.group: qs = qs.filter(group=self.group) if self.location: qs = qs.filter(location=self.location) return qs - def launch(self): + @classmethod + def execute(cls, **kwargs): + devices_list = kwargs.pop("devices", None) + batch = cls(**kwargs) + batch.full_clean() + batch.save() + if devices_list: + batch.devices.set(devices_list) + batch.status = "in-progress" + batch.save(update_fields=["status"]) + transaction.on_commit(lambda: launch_batch_command.delay(batch.pk)) + return batch + + @classmethod + def dry_run(cls, **kwargs): + devices_list = kwargs.pop("devices", None) + batch = cls(**kwargs) + batch.full_clean() + if devices_list: + return {"devices": list(devices_list)} + return {"devices": list(batch.resolve_devices())} + + def create_commands(self): self.status = "in-progress" self.save() - devices = self.resolve_devices() Command = load_model("connection", "Command") - count = 0 - for device in devices.iterator(): - cmd = Command( + for device in self.resolve_devices().iterator(): + command = Command( device=device, type=self.command_type, input=self.command_input, batch_command=self, ) - cmd.full_clean() - cmd.save() - count += 1 - self.total_devices = count - self.save(update_fields=["total_devices"]) + try: + command.full_clean() + command.save() + except ValidationError as e: + logger.warning(f"Skipping device {device.pk} for batch {self.pk}: {e}") self.calculate_and_update_status() - def launch_async(self): - self.status = "in-progress" - self.save(update_fields=["status"]) - from ..tasks import launch_batch_command - - transaction.on_commit(lambda: launch_batch_command.delay(self.pk)) - def calculate_and_update_status(self): Command = load_model("connection", "Command") operations = Command.objects.filter(batch_command=self) @@ -904,16 +935,18 @@ def calculate_and_update_status(self): ) ), ) - self.successful = stats["successful"] - self.failed = stats["failed"] if stats["total_operations"] == 0: - self.status = "idle" + new_status = "idle" elif stats["in_progress"] > 0: - self.status = "in-progress" + new_status = "in-progress" elif stats["failed"] > 0: - self.status = "failed" + new_status = "failed" elif ( stats["successful"] > 0 and stats["completed"] == stats["total_operations"] ): - self.status = "success" - self.save(update_fields=["status", "successful", "failed"]) + new_status = "success" + else: + new_status = self.status + if self.status != new_status: + self.status = new_status + self.save(update_fields=["status"]) diff --git a/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py b/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py index a0dad8b92..8b6e02d47 100644 --- a/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py +++ b/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py @@ -1,16 +1,14 @@ -# Generated by Django 5.2.15 on 2026-06-09 22:36 - -import uuid +# Generated by Django 5.2.15 on 2026-06-14 18:00 import django.core.serializers.json import django.db.models.deletion import django.utils.timezone import model_utils.fields +import openwisp_controller.connection.commands +import uuid from django.conf import settings from django.db import migrations, models -import openwisp_controller.connection.commands - class Migration(migrations.Migration): @@ -59,7 +57,6 @@ class Migration(migrations.Migration): ("in-progress", "in progress"), ("success", "completed successfully"), ("failed", "completed with some failures"), - ("cancelled", "completed with some cancellations"), ], default="idle", max_length=12, @@ -80,10 +77,7 @@ class Migration(migrations.Migration): null=True, ), ), - ("total_devices", models.PositiveIntegerField(default=0)), - ("successful", models.PositiveIntegerField(default=0)), - ("failed", models.PositiveIntegerField(default=0)), - ("cancelled", models.PositiveIntegerField(default=0)), + ("execute_all", models.BooleanField(default=False)), ( "devices", models.ManyToManyField( @@ -123,8 +117,8 @@ class Migration(migrations.Migration): ), ], options={ - "verbose_name": "Batch command operation", - "verbose_name_plural": "Batch command operations", + "verbose_name": "Batch command", + "verbose_name_plural": "Batch commands", "abstract": False, "swappable": "CONNECTION_BATCHCOMMAND_MODEL", }, diff --git a/openwisp_controller/connection/tasks.py b/openwisp_controller/connection/tasks.py index b3b169c40..e90ba2edb 100644 --- a/openwisp_controller/connection/tasks.py +++ b/openwisp_controller/connection/tasks.py @@ -111,7 +111,7 @@ def launch_batch_command(self, batch_id): BatchCommand = load_model("connection", "BatchCommand") try: batch = BatchCommand.objects.get(pk=batch_id) - batch.launch() + batch.create_commands() except ObjectDoesNotExist: logger.warning(f"The BatchCommand object with id {batch_id} not foound") From b0ef6f497769809f421e500cfd18e63ec2f86d66 Mon Sep 17 00:00:00 2001 From: dee077 Date: Mon, 15 Jun 2026 20:22:14 +0530 Subject: [PATCH 4/8] [feature] Added model-level validation, failed Command records, and idempotency guard - Added full_clean() to dry_run() for model-level validation - Create failed Command records instead of skipping on validation error - Added idempotency guard to create_commands() via Command existence check - Narrowed ObjectDoesNotExist handler in launch_batch_command task - Return full message_dict instead of first message on ValidationError --- openwisp_controller/connection/api/views.py | 6 ++++-- openwisp_controller/connection/base/models.py | 9 +++++++-- .../0011_batchcommand_command_batch_command.py | 6 ++++-- openwisp_controller/connection/tasks.py | 7 ++++--- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/openwisp_controller/connection/api/views.py b/openwisp_controller/connection/api/views.py index 8628eeb4d..df87f9cce 100644 --- a/openwisp_controller/connection/api/views.py +++ b/openwisp_controller/connection/api/views.py @@ -156,7 +156,8 @@ def post(self, request): batch = BatchCommand.execute(**serializer.validated_data) except ValidationError as e: return Response( - {"error": str(e.messages[0])}, status=status.HTTP_400_BAD_REQUEST + getattr(e, "message_dict", e.messages), + status=status.HTTP_400_BAD_REQUEST, ) return Response({"batch": str(batch.pk)}, status=201) @@ -167,7 +168,8 @@ def get(self, request): data = BatchCommand.dry_run(**serializer.validated_data) except ValidationError as e: return Response( - {"error": str(e.messages[0])}, status=status.HTTP_400_BAD_REQUEST + getattr(e, "message_dict", e.messages), + status=status.HTTP_400_BAD_REQUEST, ) data["devices"] = [str(d.pk) for d in data["devices"]] return Response(data) diff --git a/openwisp_controller/connection/base/models.py b/openwisp_controller/connection/base/models.py index 2ce23f676..510c04027 100644 --- a/openwisp_controller/connection/base/models.py +++ b/openwisp_controller/connection/base/models.py @@ -888,9 +888,11 @@ def dry_run(cls, **kwargs): return {"devices": list(batch.resolve_devices())} def create_commands(self): + Command = load_model("connection", "Command") + if Command.objects.filter(batch_command=self).exists(): + return self.status = "in-progress" self.save() - Command = load_model("connection", "Command") for device in self.resolve_devices().iterator(): command = Command( device=device, @@ -902,7 +904,10 @@ def create_commands(self): command.full_clean() command.save() except ValidationError as e: - logger.warning(f"Skipping device {device.pk} for batch {self.pk}: {e}") + command.status = "failed" + command.output = str(e) + models.Model.save(command) + logger.warning(f"Device {device.pk} failed for batch {self.pk}: {e}") self.calculate_and_update_status() def calculate_and_update_status(self): diff --git a/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py b/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py index 8b6e02d47..c91b62d35 100644 --- a/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py +++ b/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py @@ -1,14 +1,16 @@ # Generated by Django 5.2.15 on 2026-06-14 18:00 +import uuid + import django.core.serializers.json import django.db.models.deletion import django.utils.timezone import model_utils.fields -import openwisp_controller.connection.commands -import uuid from django.conf import settings from django.db import migrations, models +import openwisp_controller.connection.commands + class Migration(migrations.Migration): diff --git a/openwisp_controller/connection/tasks.py b/openwisp_controller/connection/tasks.py index e90ba2edb..53f39e1d8 100644 --- a/openwisp_controller/connection/tasks.py +++ b/openwisp_controller/connection/tasks.py @@ -111,9 +111,10 @@ def launch_batch_command(self, batch_id): BatchCommand = load_model("connection", "BatchCommand") try: batch = BatchCommand.objects.get(pk=batch_id) - batch.create_commands() - except ObjectDoesNotExist: - logger.warning(f"The BatchCommand object with id {batch_id} not foound") + except BatchCommand.DoesNotExist: + logger.warning(f"The BatchCommand object with id {batch_id} has been deleted") + return + batch.create_commands() @shared_task(soft_time_limit=3600) From 0d7aa65dcc8cbe5f04f0dd3b8d8b2ebc9c78e21a Mon Sep 17 00:00:00 2001 From: dee077 Date: Tue, 16 Jun 2026 01:34:53 +0530 Subject: [PATCH 5/8] [feature] Finalized BatchCommand migration and QA fixes - Fixed migration swappable dependency for config/geo apps - Log only field names (not values) in create_commands error handler - Added batch_command field to expected websocket response - Added sample_connection BatchCommand model, migration, view, and settings - Updated geo test query count assertions --- openwisp_controller/connection/base/models.py | 8 +- ...0011_batchcommand_command_batch_command.py | 26 ++-- .../connection/tests/pytest.py | 1 + .../geo/estimated_location/tests/tests.py | 2 +- openwisp_controller/geo/tests/test_api.py | 2 +- .../openwisp2/sample_connection/api/views.py | 8 + ...0005_batchcommand_command_batch_command.py | 138 ++++++++++++++++++ tests/openwisp2/sample_connection/models.py | 6 + tests/openwisp2/settings.py | 1 + 9 files changed, 178 insertions(+), 14 deletions(-) create mode 100644 tests/openwisp2/sample_connection/migrations/0005_batchcommand_command_batch_command.py diff --git a/openwisp_controller/connection/base/models.py b/openwisp_controller/connection/base/models.py index 510c04027..0d095bf87 100644 --- a/openwisp_controller/connection/base/models.py +++ b/openwisp_controller/connection/base/models.py @@ -907,7 +907,13 @@ def create_commands(self): command.status = "failed" command.output = str(e) models.Model.save(command) - logger.warning(f"Device {device.pk} failed for batch {self.pk}: {e}") + fields = list(getattr(e, "message_dict", {}).keys()) or ["__all__"] + logger.warning( + "Command validation failed for batch=%s device=%s fields=%s", + self.pk, + device.pk, + ",".join(fields), + ) self.calculate_and_update_status() def calculate_and_update_status(self): diff --git a/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py b/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py index c91b62d35..62fe4eef7 100644 --- a/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py +++ b/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py @@ -2,14 +2,15 @@ import uuid +import django import django.core.serializers.json import django.db.models.deletion import django.utils.timezone import model_utils.fields -from django.conf import settings +import swapper from django.db import migrations, models -import openwisp_controller.connection.commands +from openwisp_controller import connection as connection_config class Migration(migrations.Migration): @@ -17,9 +18,8 @@ class Migration(migrations.Migration): dependencies = [ ("connection", "0010_replace_jsonfield_with_django_builtin"), ("openwisp_users", "0022_user_expiration_date"), - migrations.swappable_dependency(settings.CONFIG_DEVICEGROUP_MODEL), - migrations.swappable_dependency(settings.CONFIG_DEVICE_MODEL), - migrations.swappable_dependency(settings.GEO_LOCATION_MODEL), + ("config", "0063_replace_jsonfield_with_django_builtin"), + ("geo", "0006_create_geo_settings_for_existing_orgs"), ] operations = [ @@ -67,8 +67,12 @@ class Migration(migrations.Migration): ( "command_type", models.CharField( - choices=openwisp_controller.connection.commands.get_command_choices, max_length=16, + choices=( + connection_config.commands.COMMAND_CHOICES + if django.VERSION < (5, 0) + else connection_config.commands.get_command_choices + ), ), ), ( @@ -84,7 +88,7 @@ class Migration(migrations.Migration): "devices", models.ManyToManyField( blank=True, - to=settings.CONFIG_DEVICE_MODEL, + to=swapper.get_model_name("config", "Device"), verbose_name="devices", ), ), @@ -94,7 +98,7 @@ class Migration(migrations.Migration): blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, - to=settings.CONFIG_DEVICEGROUP_MODEL, + to=swapper.get_model_name("config", "DeviceGroup"), verbose_name="device group", ), ), @@ -104,7 +108,7 @@ class Migration(migrations.Migration): blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, - to=settings.GEO_LOCATION_MODEL, + to=swapper.get_model_name("geo", "Location"), verbose_name="location", ), ), @@ -114,7 +118,7 @@ class Migration(migrations.Migration): blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, - to="openwisp_users.organization", + to=swapper.get_model_name("openwisp_users", "Organization"), ), ), ], @@ -132,7 +136,7 @@ class Migration(migrations.Migration): blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, - to=settings.CONNECTION_BATCHCOMMAND_MODEL, + to=swapper.get_model_name("connection", "BatchCommand"), ), ), ] diff --git a/openwisp_controller/connection/tests/pytest.py b/openwisp_controller/connection/tests/pytest.py index 220604cef..d7f7f5659 100644 --- a/openwisp_controller/connection/tests/pytest.py +++ b/openwisp_controller/connection/tests/pytest.py @@ -65,6 +65,7 @@ def _get_expected_response(self, command): "output": command.output, "device": str(command.device_id), "connection": str(command.connection_id), + "batch_command": None, }, } diff --git a/openwisp_controller/geo/estimated_location/tests/tests.py b/openwisp_controller/geo/estimated_location/tests/tests.py index c037106dd..fee63ae94 100644 --- a/openwisp_controller/geo/estimated_location/tests/tests.py +++ b/openwisp_controller/geo/estimated_location/tests/tests.py @@ -566,7 +566,7 @@ def _verify_location_details(device, mocked_response): device2.save() # 3 queries related to notifications cleanup device2.refresh_from_db() - with self.assertNumQueries(15): + with self.assertNumQueries(16): manage_estimated_locations(device2.pk, device2.last_ip) mock_info.assert_called_once_with( f"Estimated location saved successfully for {device2.pk}" diff --git a/openwisp_controller/geo/tests/test_api.py b/openwisp_controller/geo/tests/test_api.py index 156550692..df46460cf 100644 --- a/openwisp_controller/geo/tests/test_api.py +++ b/openwisp_controller/geo/tests/test_api.py @@ -694,7 +694,7 @@ def test_change_location_type_to_outdoor_api(self): def test_delete_location_detail(self): l1 = self._create_location() path = reverse("geo_api:detail_location", args=[l1.pk]) - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.delete(path) self.assertEqual(response.status_code, 204) diff --git a/tests/openwisp2/sample_connection/api/views.py b/tests/openwisp2/sample_connection/api/views.py index fd2207cbc..66510f22a 100644 --- a/tests/openwisp2/sample_connection/api/views.py +++ b/tests/openwisp2/sample_connection/api/views.py @@ -1,3 +1,6 @@ +from openwisp_controller.connection.api.views import ( + BatchCommandExecuteView as BaseBatchCommandExecuteView, +) from openwisp_controller.connection.api.views import ( CommandDetailsView as BaseCommandDetailsView, ) @@ -42,9 +45,14 @@ class DeviceConnectionDetailView(BaseDeviceConnectionDetailView): pass +class BatchCommandExecuteView(BaseBatchCommandExecuteView): + pass + + command_list_create_view = CommandListCreateView.as_view() command_details_view = CommandDetailsView.as_view() credential_list_create_view = CredentialListCreateView.as_view() credential_detail_view = CredentialDetailView.as_view() deviceconnection_list_create_view = DeviceConnectionListCreateView.as_view() deviceconnection_detail_view = DeviceConnectionDetailView.as_view() +batch_command_execute_view = BatchCommandExecuteView.as_view() diff --git a/tests/openwisp2/sample_connection/migrations/0005_batchcommand_command_batch_command.py b/tests/openwisp2/sample_connection/migrations/0005_batchcommand_command_batch_command.py new file mode 100644 index 000000000..593a9f6b2 --- /dev/null +++ b/tests/openwisp2/sample_connection/migrations/0005_batchcommand_command_batch_command.py @@ -0,0 +1,138 @@ +# Generated by Django 5.2.15 on 2026-06-15 18:15 + +import uuid + +import django +import django.core.serializers.json +import django.db.models.deletion +import django.utils.timezone +import model_utils.fields +from django.db import migrations, models + +from openwisp_controller import connection as connection_config + + +class Migration(migrations.Migration): + + dependencies = [ + ("sample_config", "0009_replace_jsonfield_with_django_builtin"), + ("sample_connection", "0004_replace_jsonfield_with_django_builtin"), + ("sample_geo", "0005_organizationgeosettings"), + ("sample_users", "0005_user_expiration_date_user_user_active_expiry_idx"), + ] + + operations = [ + migrations.CreateModel( + name="BatchCommand", + fields=[ + ( + "id", + models.UUIDField( + default=uuid.uuid4, + editable=False, + primary_key=True, + serialize=False, + ), + ), + ( + "created", + model_utils.fields.AutoCreatedField( + default=django.utils.timezone.now, + editable=False, + verbose_name="created", + ), + ), + ( + "modified", + model_utils.fields.AutoLastModifiedField( + default=django.utils.timezone.now, + editable=False, + verbose_name="modified", + ), + ), + ( + "status", + models.CharField( + choices=[ + ("idle", "idle"), + ("in-progress", "in progress"), + ("success", "completed successfully"), + ("failed", "completed with some failures"), + ], + default="idle", + max_length=12, + ), + ), + ( + "command_type", + models.CharField( + max_length=16, + choices=( + connection_config.commands.COMMAND_CHOICES + if django.VERSION < (5, 0) + else connection_config.commands.get_command_choices + ), + ), + ), + ( + "command_input", + models.JSONField( + blank=True, + encoder=django.core.serializers.json.DjangoJSONEncoder, + null=True, + ), + ), + ("execute_all", models.BooleanField(default=False)), + ( + "devices", + models.ManyToManyField( + blank=True, to="sample_config.device", verbose_name="devices" + ), + ), + ( + "group", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="sample_config.devicegroup", + verbose_name="device group", + ), + ), + ( + "location", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="sample_geo.location", + verbose_name="location", + ), + ), + ( + "organization", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="sample_users.organization", + ), + ), + ], + options={ + "verbose_name": "Batch command", + "verbose_name_plural": "Batch commands", + "abstract": False, + }, + ), + migrations.AddField( + model_name="command", + name="batch_command", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="sample_connection.batchcommand", + ), + ), + ] diff --git a/tests/openwisp2/sample_connection/models.py b/tests/openwisp2/sample_connection/models.py index 7964c5471..4e96065e8 100644 --- a/tests/openwisp2/sample_connection/models.py +++ b/tests/openwisp2/sample_connection/models.py @@ -1,6 +1,7 @@ from django.db import models from openwisp_controller.connection.base.models import ( + AbstractBatchCommand, AbstractCommand, AbstractCredentials, AbstractDeviceConnection, @@ -27,3 +28,8 @@ class Meta(AbstractDeviceConnection.Meta): class Command(AbstractCommand): class Meta(AbstractCommand.Meta): abstract = False + + +class BatchCommand(AbstractBatchCommand): + class Meta(AbstractBatchCommand.Meta): + abstract = False diff --git a/tests/openwisp2/settings.py b/tests/openwisp2/settings.py index c45eb5537..0e27adfde 100644 --- a/tests/openwisp2/settings.py +++ b/tests/openwisp2/settings.py @@ -293,6 +293,7 @@ CONNECTION_CREDENTIALS_MODEL = "sample_connection.Credentials" CONNECTION_DEVICECONNECTION_MODEL = "sample_connection.DeviceConnection" CONNECTION_COMMAND_MODEL = "sample_connection.Command" + CONNECTION_BATCHCOMMAND_MODEL = "sample_connection.BatchCommand" SUBNET_DIVISION_SUBNETDIVISIONRULE_MODEL = ( "sample_subnet_division.SubnetDivisionRule" ) From 2ff725821f72b852345d84956b300804570e6325 Mon Sep 17 00:00:00 2001 From: dee077 Date: Tue, 16 Jun 2026 20:16:23 +0530 Subject: [PATCH 6/8] [fix] Made GET dry-run work without type param, default execute_all to true - Make type optional on GET requests via serializer __init__ - Skip full_clean() in dry_run when command_type is not provided - Default execute_all to True for both GET and POST --- openwisp_controller/connection/api/serializers.py | 8 +++++++- openwisp_controller/connection/base/models.py | 5 ++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/openwisp_controller/connection/api/serializers.py b/openwisp_controller/connection/api/serializers.py index 77f9a5968..c25f27812 100644 --- a/openwisp_controller/connection/api/serializers.py +++ b/openwisp_controller/connection/api/serializers.py @@ -136,7 +136,13 @@ class BatchCommandExecuteSerializer( allow_empty=True, pk_field=serializers.UUIDField(format="hex_verbose"), ) - execute_all = serializers.BooleanField(required=False, default=False) + execute_all = serializers.BooleanField(required=False, default=True) + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + request = self.context.get("request") + if request and request.method == "GET": + self.fields["type"].required = False class Meta: model = BatchCommand diff --git a/openwisp_controller/connection/base/models.py b/openwisp_controller/connection/base/models.py index 0d095bf87..592835070 100644 --- a/openwisp_controller/connection/base/models.py +++ b/openwisp_controller/connection/base/models.py @@ -881,8 +881,11 @@ def execute(cls, **kwargs): @classmethod def dry_run(cls, **kwargs): devices_list = kwargs.pop("devices", None) + command_type = kwargs.pop("command_type", None) batch = cls(**kwargs) - batch.full_clean() + if command_type: + batch.command_type = command_type + batch.full_clean() if devices_list: return {"devices": list(devices_list)} return {"devices": list(batch.resolve_devices())} From 65f73d05ed6d776aa3246b7db11be3aef54120de Mon Sep 17 00:00:00 2001 From: dee077 Date: Fri, 19 Jun 2026 06:32:46 +0530 Subject: [PATCH 7/8] [fix] Add docstrings and minor fixes --- openwisp_controller/connection/base/models.py | 156 +++++++++++------- ...0011_batchcommand_command_batch_command.py | 2 +- openwisp_controller/connection/tasks.py | 10 +- ...0005_batchcommand_command_batch_command.py | 2 +- 4 files changed, 101 insertions(+), 69 deletions(-) diff --git a/openwisp_controller/connection/base/models.py b/openwisp_controller/connection/base/models.py index 592835070..0564c3dea 100644 --- a/openwisp_controller/connection/base/models.py +++ b/openwisp_controller/connection/base/models.py @@ -476,6 +476,7 @@ class AbstractCommand(TimeStampedEditableModel): on_delete=models.SET_NULL, blank=True, null=True, + related_name="batch_commands", ) class Meta: @@ -774,7 +775,6 @@ class AbstractBatchCommand(TimeStampedEditableModel): blank=True, verbose_name=_("devices"), ) - execute_all = models.BooleanField(default=False) class Meta: abstract = True @@ -783,63 +783,61 @@ class Meta: @cached_property def total_devices(self): - Command = load_model("connection", "Command") - return Command.objects.filter(batch_command=self).count() + return self.batch_commands.count() @property def successful(self): - Command = load_model("connection", "Command") - return Command.objects.filter(batch_command=self, status="success").count() + return self.batch_commands.filter(status="success").count() @property def failed(self): - Command = load_model("connection", "Command") - return Command.objects.filter(batch_command=self, status="failed").count() + return self.batch_commands.filter(status="failed").count() - def clean(self): - super().clean() - if self.organization_id: - if self.group and self.group.organization != self.organization: - raise ValidationError( - { - "group": _( - "The organization of the group doesn't match " - "the organization of the batch command operation" - ) - } - ) - if self.location and self.location.organization != self.organization: + def _validate_org_relations(self): + if not self.organization_id: + return + if self.group and self.group.organization != self.organization: + raise ValidationError( + { + "group": _( + "The organization of the group doesn't match " + "the organization of the batch command operation" + ) + } + ) + if self.location and self.location.organization != self.organization: + raise ValidationError( + { + "location": _( + "The organization of the location doesn't match " + "the organization of the batch command operation" + ) + } + ) + if self.pk and self.devices.exists(): + org_mismatch = self.devices.exclude(organization=self.organization).exists() + if org_mismatch: raise ValidationError( { - "location": _( - "The organization of the location doesn't match " - "the organization of the batch command operation" + "devices": _( + "All devices must belong to the same " + "organization as the batch command." ) } ) - if self.pk and self.devices.exists(): - org_mismatch = self.devices.exclude( - organization=self.organization - ).exists() - if org_mismatch: - raise ValidationError( - { - "devices": _( - "All devices must belong to the same " - "organization as the batch command." - ) - } - ) + + def clean(self): + super().clean() + self._validate_org_relations() + Command = load_model("connection", "Command") allowed = dict( - AbstractCommand.get_org_allowed_commands( - organization_id=self.organization_id - ) + Command.get_org_allowed_commands(organization_id=self.organization_id) ) if self.command_type not in allowed: raise ValidationError( { "command_type": _( - '"{command}" command is not available ' "for this organization" + '"{command}" command is not available for this organization' ).format(command=self.command_type) } ) @@ -851,28 +849,51 @@ def clean(self): raise ValidationError({"command_input": e.message}) def resolve_devices(self): + """ + Returns the queryset of devices targeted by this batch command. + - Devices explicitly set via M2M relation: returns those directly. + - No explicit devices: resolves via organization, group, and location filters. + - No filters set: returns all devices (superuser targeting all orgs). + """ if self.pk and self.devices.exists(): - return self.devices.all() + return self.devices.iterator() Device = load_model("config", "Device") qs = Device.objects.all() if self.organization_id: qs = qs.filter(organization=self.organization) - if self.execute_all: - return qs if self.group: qs = qs.filter(group=self.group) if self.location: - qs = qs.filter(location=self.location) + qs = qs.filter(devicelocation__location=self.location) return qs @classmethod def execute(cls, **kwargs): + """ + Creates, validates, and persists the batch command, then schedules + execution via a background task. + - No devices match the specified criteria: batch is deleted and + ValidationError is raised to avoid orphan records. + - Explicit device list provided: uses device PKs directly instead + of resolving via organization/group/location filters. + - Superuser with no organization: targets devices across all orgs. + """ devices_list = kwargs.pop("devices", None) batch = cls(**kwargs) batch.full_clean() batch.save() if devices_list: batch.devices.set(devices_list) + if not batch.devices.exists(): + batch.delete() + raise ValidationError( + _("No devices match the specified criteria."), + ) + elif not batch.resolve_devices().exists(): + batch.delete() + raise ValidationError( + _("No devices match the specified criteria."), + ) batch.status = "in-progress" batch.save(update_fields=["status"]) transaction.on_commit(lambda: launch_batch_command.delay(batch.pk)) @@ -880,23 +901,40 @@ def execute(cls, **kwargs): @classmethod def dry_run(cls, **kwargs): + """ + Returns the devices that would be targeted by this batch command without + executing it. + - Command type not provided (GET request): skips full clean, only + validates organization relations. + - Explicit device list provided: returns it directly without resolving + via organization/group/location filters. + """ devices_list = kwargs.pop("devices", None) command_type = kwargs.pop("command_type", None) batch = cls(**kwargs) if command_type: batch.command_type = command_type batch.full_clean() + else: + batch._validate_org_relations() if devices_list: return {"devices": list(devices_list)} return {"devices": list(batch.resolve_devices())} def create_commands(self): - Command = load_model("connection", "Command") - if Command.objects.filter(batch_command=self).exists(): + """ + Creates individual Command instances for each device targeted by this batch + command. + - Commands already exist for this batch: returns early (idempotent guard). + - Device validation fails (deactivated, no credentials, wrong org): + device is skipped and logged — no Command record is created. + """ + if self.batch_commands.exists(): return + Command = load_model("connection", "Command") self.status = "in-progress" self.save() - for device in self.resolve_devices().iterator(): + for device in self.resolve_devices(): command = Command( device=device, type=self.command_type, @@ -904,25 +942,27 @@ def create_commands(self): batch_command=self, ) try: - command.full_clean() command.save() except ValidationError as e: - command.status = "failed" - command.output = str(e) - models.Model.save(command) - fields = list(getattr(e, "message_dict", {}).keys()) or ["__all__"] logger.warning( - "Command validation failed for batch=%s device=%s fields=%s", - self.pk, + "Skipping device %s for batch %s: %s", device.pk, - ",".join(fields), + self.pk, + e, ) self.calculate_and_update_status() def calculate_and_update_status(self): - Command = load_model("connection", "Command") - operations = Command.objects.filter(batch_command=self) - stats = operations.aggregate( + """ + Calculate batch status based on individual command statuses and update if + changed. + - No commands exist: status set to "idle". + - Commands still running: status set to "in-progress". + - Some commands failed: status set to "failed". + - All commands completed successfully: status set to "success". + - Status unchanged: no database write performed. + """ + stats = self.batch_commands.aggregate( total_operations=models.Count("id"), in_progress=models.Count( models.Case( diff --git a/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py b/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py index 62fe4eef7..9ade089f7 100644 --- a/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py +++ b/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py @@ -83,7 +83,6 @@ class Migration(migrations.Migration): null=True, ), ), - ("execute_all", models.BooleanField(default=False)), ( "devices", models.ManyToManyField( @@ -136,6 +135,7 @@ class Migration(migrations.Migration): blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, + related_name="batch_commands", to=swapper.get_model_name("connection", "BatchCommand"), ), ), diff --git a/openwisp_controller/connection/tasks.py b/openwisp_controller/connection/tasks.py index 53f39e1d8..2ad4b153a 100644 --- a/openwisp_controller/connection/tasks.py +++ b/openwisp_controller/connection/tasks.py @@ -81,14 +81,6 @@ def launch_command(command_id): return try: command.execute() - # Todo: Remove once demo is completed - if command.batch_command_id: - print("****************************") - print(f"Device: {command.device.name}") - print(f"Status: {command.status}") - print("") - print(command.output) - print("****************************") except SoftTimeLimitExceeded: command.status = "failed" command._add_output(_("Background task time limit exceeded.")) @@ -106,7 +98,7 @@ def launch_command(command_id): command._save_without_resurrecting() -@shared_task(bind=True, soft_time_limit=3600) +@shared_task(bind=True, soft_time_limit=app_settings.SSH_COMMAND_TIMEOUT * 1.2) def launch_batch_command(self, batch_id): BatchCommand = load_model("connection", "BatchCommand") try: diff --git a/tests/openwisp2/sample_connection/migrations/0005_batchcommand_command_batch_command.py b/tests/openwisp2/sample_connection/migrations/0005_batchcommand_command_batch_command.py index 593a9f6b2..5f361d5e5 100644 --- a/tests/openwisp2/sample_connection/migrations/0005_batchcommand_command_batch_command.py +++ b/tests/openwisp2/sample_connection/migrations/0005_batchcommand_command_batch_command.py @@ -82,7 +82,6 @@ class Migration(migrations.Migration): null=True, ), ), - ("execute_all", models.BooleanField(default=False)), ( "devices", models.ManyToManyField( @@ -132,6 +131,7 @@ class Migration(migrations.Migration): blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, + related_name="batch_commands", to="sample_connection.batchcommand", ), ), From e9bf89f7cc3185deedaa585500771ade64100204 Mon Sep 17 00:00:00 2001 From: dee077 Date: Sun, 21 Jun 2026 04:26:42 +0530 Subject: [PATCH 8/8] [fix] Restructure BatchCommand fields and refine test fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Renamed BatchCommand fields across model, serializer, migrations, and tests - Moved test fixtures from setUp into individual test methods - _create_batch_command now requires organization as positional arg - Added test_batch_command_execute_queries with assertNumQueries(13) - Added test_batch_command_cross_org_restrictions for org-level command restrictions - Added device ID assertions in execute tests - Renamed tests: list_filter_org→list_organization_scoped, execute_no_devices→execute_org_has_no_devices --- .../connection/api/serializers.py | 40 ++- openwisp_controller/connection/api/urls.py | 10 + openwisp_controller/connection/api/views.py | 19 +- openwisp_controller/connection/base/models.py | 67 ++--- ...0011_batchcommand_command_batch_command.py | 8 +- .../connection/tests/test_api.py | 264 ++++++++++++++++++ .../openwisp2/sample_connection/api/views.py | 16 ++ ...0005_batchcommand_command_batch_command.py | 8 +- 8 files changed, 381 insertions(+), 51 deletions(-) diff --git a/openwisp_controller/connection/api/serializers.py b/openwisp_controller/connection/api/serializers.py index c25f27812..5d2a6c681 100644 --- a/openwisp_controller/connection/api/serializers.py +++ b/openwisp_controller/connection/api/serializers.py @@ -125,10 +125,8 @@ class Meta: class BatchCommandExecuteSerializer( FilterSerializerByOrgManaged, serializers.ModelSerializer ): - type = serializers.CharField(source="command_type") - input = serializers.JSONField( - source="command_input", allow_null=True, required=False - ) + type = serializers.CharField() + input = serializers.JSONField(allow_null=True, required=False) devices = serializers.PrimaryKeyRelatedField( many=True, queryset=Device.objects.all(), @@ -187,3 +185,37 @@ def validate(self, data): } ) return data + + +class BatchCommandSerializer(BaseSerializer): + device_count = serializers.IntegerField(source="devices.count", read_only=True) + + class Meta: + model = BatchCommand + fields = ( + "id", + "organization", + "status", + "type", + "input", + "group", + "location", + "device_count", + "created", + "modified", + ) + read_only_fields = ( + "created", + "modified", + ) + + +class BatchCommandDetailSerializer(BatchCommandSerializer): + devices = serializers.PrimaryKeyRelatedField( + many=True, + read_only=True, + pk_field=serializers.UUIDField(format="hex_verbose"), + ) + + class Meta(BatchCommandSerializer.Meta): + fields = BatchCommandSerializer.Meta.fields + ("devices",) diff --git a/openwisp_controller/connection/api/urls.py b/openwisp_controller/connection/api/urls.py index 76a30c64a..4a94d171d 100644 --- a/openwisp_controller/connection/api/urls.py +++ b/openwisp_controller/connection/api/urls.py @@ -40,6 +40,16 @@ def get_api_urls(api_views): api_views.deviceconnection_detail_view, name="deviceconnection_detail", ), + path( + "api/v1/controller/batch-command/", + api_views.batch_command_list_view, + name="batch_command_list", + ), + path( + "api/v1/controller/batch-command//", + api_views.batch_command_detail_view, + name="batch_command_detail", + ), path( "api/v1/controller/batch-command/execute/", api_views.batch_command_execute_view, diff --git a/openwisp_controller/connection/api/views.py b/openwisp_controller/connection/api/views.py index df87f9cce..0a4912d76 100644 --- a/openwisp_controller/connection/api/views.py +++ b/openwisp_controller/connection/api/views.py @@ -5,6 +5,7 @@ from rest_framework import status from rest_framework.generics import ( GenericAPIView, + ListAPIView, ListCreateAPIView, RetrieveAPIView, RetrieveUpdateDestroyAPIView, @@ -21,7 +22,9 @@ RelatedDeviceProtectedAPIMixin, ) from .serializers import ( + BatchCommandDetailSerializer, BatchCommandExecuteSerializer, + BatchCommandSerializer, CommandSerializer, CredentialSerializer, DeviceConnectionSerializer, @@ -152,6 +155,7 @@ class BatchCommandExecuteView(ProtectedAPIMixin, GenericAPIView): def post(self, request): serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) + serializer.validated_data.pop("execute_all", None) try: batch = BatchCommand.execute(**serializer.validated_data) except ValidationError as e: @@ -164,6 +168,7 @@ def post(self, request): def get(self, request): serializer = self.get_serializer(data=request.query_params) serializer.is_valid(raise_exception=True) + serializer.validated_data.pop("execute_all", None) try: data = BatchCommand.dry_run(**serializer.validated_data) except ValidationError as e: @@ -175,6 +180,17 @@ def get(self, request): return Response(data) +class BatchCommandListView(ProtectedAPIMixin, ListAPIView): + queryset = BatchCommand.objects.all().order_by("-created") + serializer_class = BatchCommandSerializer + pagination_class = OpenWispPagination + + +class BatchCommandDetailView(ProtectedAPIMixin, RetrieveAPIView): + queryset = BatchCommand.objects.all() + serializer_class = BatchCommandDetailSerializer + + class DeviceConnectionDetailView(BaseDeviceConnection, RetrieveUpdateDestroyAPIView): def get_object(self): queryset = self.filter_queryset(self.get_queryset()) @@ -195,5 +211,6 @@ def get_object(self): # TODO: remove in version 1.4 deviceconnection_details_view = deviceconnection_detail_view - batch_command_execute_view = BatchCommandExecuteView.as_view() +batch_command_list_view = BatchCommandListView.as_view() +batch_command_detail_view = BatchCommandDetailView.as_view() diff --git a/openwisp_controller/connection/base/models.py b/openwisp_controller/connection/base/models.py index 0564c3dea..e92888474 100644 --- a/openwisp_controller/connection/base/models.py +++ b/openwisp_controller/connection/base/models.py @@ -738,8 +738,8 @@ class AbstractBatchCommand(TimeStampedEditableModel): STATUS_CHOICES = ( ("idle", _("idle")), ("in-progress", _("in progress")), - ("success", _("completed successfully")), - ("failed", _("completed with some failures")), + ("success", _("success")), + ("failed", _("failed")), ) organization = models.ForeignKey( @@ -751,11 +751,11 @@ class AbstractBatchCommand(TimeStampedEditableModel): status = models.CharField( max_length=12, choices=STATUS_CHOICES, default=STATUS_CHOICES[0][0] ) - command_type = models.CharField( + type = models.CharField( max_length=16, choices=(COMMAND_CHOICES if django.VERSION < (5, 0) else get_command_choices), ) - command_input = JSONField(blank=True, null=True, encoder=DjangoJSONEncoder) + input = JSONField(blank=True, null=True, encoder=DjangoJSONEncoder) group = models.ForeignKey( get_model_name("config", "DeviceGroup"), on_delete=models.SET_NULL, @@ -833,27 +833,26 @@ def clean(self): allowed = dict( Command.get_org_allowed_commands(organization_id=self.organization_id) ) - if self.command_type not in allowed: + if self.type not in allowed: raise ValidationError( { - "command_type": _( + "type": _( '"{command}" command is not available for this organization' - ).format(command=self.command_type) + ).format(command=self.type) } ) try: - jsonschema.Draft4Validator(get_command_schema(self.command_type)).validate( - self.command_input + jsonschema.Draft4Validator(get_command_schema(self.type)).validate( + self.input ) except SchemaError as e: - raise ValidationError({"command_input": e.message}) + raise ValidationError({"input": e.message}) def resolve_devices(self): """ - Returns the queryset of devices targeted by this batch command. - - Devices explicitly set via M2M relation: returns those directly. - - No explicit devices: resolves via organization, group, and location filters. - - No filters set: returns all devices (superuser targeting all orgs). + Returns an iterator of devices targeted by this batch command, + resolved from explicit M2M devices or filtered by organization, + group, and location. Returns an empty iterator if no devices match. """ if self.pk and self.devices.exists(): return self.devices.iterator() @@ -865,18 +864,14 @@ def resolve_devices(self): qs = qs.filter(group=self.group) if self.location: qs = qs.filter(devicelocation__location=self.location) - return qs + return qs.iterator() @classmethod def execute(cls, **kwargs): """ Creates, validates, and persists the batch command, then schedules - execution via a background task. - - No devices match the specified criteria: batch is deleted and - ValidationError is raised to avoid orphan records. - - Explicit device list provided: uses device PKs directly instead - of resolving via organization/group/location filters. - - Superuser with no organization: targets devices across all orgs. + execution via a background task. Raises ValidationError and deletes + the batch if no devices match the criteria. """ devices_list = kwargs.pop("devices", None) batch = cls(**kwargs) @@ -889,7 +884,7 @@ def execute(cls, **kwargs): raise ValidationError( _("No devices match the specified criteria."), ) - elif not batch.resolve_devices().exists(): + elif not any(batch.resolve_devices()): batch.delete() raise ValidationError( _("No devices match the specified criteria."), @@ -902,18 +897,15 @@ def execute(cls, **kwargs): @classmethod def dry_run(cls, **kwargs): """ - Returns the devices that would be targeted by this batch command without - executing it. - - Command type not provided (GET request): skips full clean, only - validates organization relations. - - Explicit device list provided: returns it directly without resolving - via organization/group/location filters. + Returns the devices that would be targeted by this batch command + without executing it. Skips full validation when command type is + not provided case for GET request. """ devices_list = kwargs.pop("devices", None) - command_type = kwargs.pop("command_type", None) + cmd_type = kwargs.pop("type", None) batch = cls(**kwargs) - if command_type: - batch.command_type = command_type + if cmd_type: + batch.type = cmd_type batch.full_clean() else: batch._validate_org_relations() @@ -923,11 +915,10 @@ def dry_run(cls, **kwargs): def create_commands(self): """ - Creates individual Command instances for each device targeted by this batch - command. - - Commands already exist for this batch: returns early (idempotent guard). - - Device validation fails (deactivated, no credentials, wrong org): - device is skipped and logged — no Command record is created. + Creates individual Command instances for each device targeted by + this batch command. Returns early if commands already exist + (idempotent guard). Devices that fail validation are silently + skipped. """ if self.batch_commands.exists(): return @@ -937,8 +928,8 @@ def create_commands(self): for device in self.resolve_devices(): command = Command( device=device, - type=self.command_type, - input=self.command_input, + type=self.type, + input=self.input, batch_command=self, ) try: diff --git a/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py b/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py index 9ade089f7..5f5b7f590 100644 --- a/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py +++ b/openwisp_controller/connection/migrations/0011_batchcommand_command_batch_command.py @@ -57,15 +57,15 @@ class Migration(migrations.Migration): choices=[ ("idle", "idle"), ("in-progress", "in progress"), - ("success", "completed successfully"), - ("failed", "completed with some failures"), + ("success", "success"), + ("failed", "failed"), ], default="idle", max_length=12, ), ), ( - "command_type", + "type", models.CharField( max_length=16, choices=( @@ -76,7 +76,7 @@ class Migration(migrations.Migration): ), ), ( - "command_input", + "input", models.JSONField( blank=True, encoder=django.core.serializers.json.DjangoJSONEncoder, diff --git a/openwisp_controller/connection/tests/test_api.py b/openwisp_controller/connection/tests/test_api.py index 6e3a827ab..8ab0bb5d0 100644 --- a/openwisp_controller/connection/tests/test_api.py +++ b/openwisp_controller/connection/tests/test_api.py @@ -21,6 +21,7 @@ Command = load_model("connection", "Command") DeviceConnection = load_model("connection", "DeviceConnection") +BatchCommand = load_model("connection", "BatchCommand") command_qs = Command.objects.order_by("-created") OrganizationUser = load_model("openwisp_users", "OrganizationUser") Group = load_model("openwisp_users", "Group") @@ -860,3 +861,266 @@ def test_deviceconnection_unauthenticated_user(self): "delete": 401, }, ) + + +class TestBatchCommandsAPI( + TestAdminMixin, AuthenticationMixin, TestCase, CreateConnectionsMixin +): + url_namespace = "connection_api" + + def setUp(self): + super().setUp() + self._login() + + def _create_batch_command(self, organization, **kwargs): + opts = dict( + organization=organization, + type="custom", + input={"command": "echo test"}, + ) + devices = kwargs.pop("devices", None) + opts.update(kwargs) + batch = BatchCommand(**opts) + batch.full_clean() + batch.save() + if devices is not None: + if not isinstance(devices, (list, tuple)): + devices = [devices] + batch.devices.set(devices) + return batch + + def test_batch_command_list(self): + org = self._get_org() + url = reverse("connection_api:batch_command_list") + for _ in range(3): + self._create_batch_command(organization=org) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data["count"], 3) + self.assertEqual(len(response.data["results"]), 3) + created_list = [cmd["created"] for cmd in response.data["results"]] + sorted_created = sorted(created_list, reverse=True) + self.assertEqual(created_list, sorted_created) + result = response.data["results"][0] + self.assertIn("id", result) + self.assertIn("status", result) + self.assertIn("type", result) + self.assertIn("input", result) + self.assertIn("device_count", result) + self.assertIn("created", result) + self.assertEqual(result["device_count"], 0) + + def test_batch_command_detail(self): + org = self._get_org() + device = self._create_device(organization=org) + self._create_config(device=device) + batch = self._create_batch_command(organization=org, devices=[device]) + url = reverse("connection_api:batch_command_detail", args=[batch.pk]) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data["id"], str(batch.pk)) + self.assertEqual(response.data["status"], batch.status) + self.assertEqual(response.data["type"], batch.type) + self.assertEqual(response.data["input"], batch.input) + self.assertIn("devices", response.data) + self.assertEqual(response.data["devices"], [str(device.pk)]) + self.assertEqual(response.data["device_count"], 1) + + def test_batch_command_execute(self): + org = self._get_org() + device = self._create_device(organization=org) + self._create_config(device=device) + self._create_device_connection(device=device) + payload = { + "organization": str(org.pk), + "type": "custom", + "input": {"command": "echo test"}, + "devices": [str(device.pk)], + } + response = self.client.post( + reverse("connection_api:batch_command_execute"), + data=json.dumps(payload), + content_type="application/json", + ) + self.assertEqual(response.status_code, 201) + self.assertIn("batch", response.data) + batch = BatchCommand.objects.get(pk=response.data["batch"]) + # transaction.on_commit doesn't fire in TestCase; trigger manually + batch.create_commands() + command = Command.objects.get(batch_command=batch) + self.assertEqual(command.device.pk, device.pk) + + def test_batch_command_execute_queries(self): + org = self._get_org() + devices = [] + for i in range(3): + d = self._create_device( + name=f"q-dev-{i}", + mac_address=f"00:11:22:33:44:{i:02x}", + organization=org, + ) + self._create_config(device=d) + devices.append(d) + payload = { + "organization": str(org.pk), + "type": "custom", + "input": {"command": "echo test"}, + "devices": [str(d.pk) for d in devices], + } + with self.assertNumQueries(13): + response = self.client.post( + reverse("connection_api:batch_command_execute"), + data=json.dumps(payload), + content_type="application/json", + ) + self.assertEqual(response.status_code, 201) + self.assertIn("batch", response.data) + batch = BatchCommand.objects.get(pk=response.data["batch"]) + self.assertEqual(batch.devices.count(), 3) + self.assertCountEqual( + batch.devices.values_list("pk", flat=True), + [d.pk for d in devices], + ) + + def test_batch_command_execute_org_has_no_devices(self): + org = self._get_org() + payload = { + "organization": str(org.pk), + "type": "custom", + "input": {"command": "echo test"}, + "execute_all": True, + } + response = self.client.post( + reverse("connection_api:batch_command_execute"), + data=json.dumps(payload), + content_type="application/json", + ) + self.assertEqual(response.status_code, 400) + + def test_batch_command_execute_no_org(self): + org = self._get_org() + self.client.logout() + operator = self._create_operator(organizations=[org]) + add_perm = Permission.objects.get(codename="add_batchcommand") + operator.user_permissions.add(add_perm) + self.client.force_login(operator) + payload = { + "type": "custom", + "input": {"command": "echo test"}, + "execute_all": True, + } + response = self.client.post( + reverse("connection_api:batch_command_execute"), + data=json.dumps(payload), + content_type="application/json", + ) + self.assertEqual(response.status_code, 400) + self.assertIn( + "Only superusers", + str(response.data), + ) + + def test_batch_command_dry_run(self): + org = self._get_org() + device = self._create_device(organization=org) + self._create_config(device=device) + url = "{0}?organization={1}".format( + reverse("connection_api:batch_command_execute"), str(org.pk) + ) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertIn("devices", response.data) + self.assertIn(str(device.pk), response.data["devices"]) + + def test_batch_command_list_organization_scoped(self): + org = self._get_org() + org2 = self._create_org(name="org2", slug="org2") + self._create_batch_command(organization=org) + self._create_batch_command(organization=org2) + self.client.logout() + operator = self._create_operator(organizations=[org]) + view_perm = Permission.objects.get(codename="view_batchcommand") + operator.user_permissions.add(view_perm) + self.client.force_login(operator) + response = self.client.get(reverse("connection_api:batch_command_list")) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data["count"], 1) + + def test_batch_command_unauthorized(self): + self.client.logout() + with self.subTest("List"): + response = self.client.get(reverse("connection_api:batch_command_list")) + self.assertEqual(response.status_code, 401) + + with self.subTest("Detail"): + response = self.client.get( + reverse( + "connection_api:batch_command_detail", + args=[uuid.uuid4()], + ) + ) + self.assertEqual(response.status_code, 401) + + with self.subTest("Dry run"): + response = self.client.get(reverse("connection_api:batch_command_execute")) + self.assertEqual(response.status_code, 401) + + with self.subTest("Execute"): + response = self.client.post( + reverse("connection_api:batch_command_execute"), + data=json.dumps({"type": "custom"}), + content_type="application/json", + ) + self.assertEqual(response.status_code, 401) + + def test_batch_command_cross_org_restrictions(self): + org = self._get_org() + org2 = self._create_org(name="org2", slug="org2") + device_a = self._create_device( + name="device-a", + mac_address="00:11:22:33:44:aa", + organization=org, + ) + self._create_config(device=device_a) + self._create_device_connection(device=device_a) + device_b = self._create_device( + name="device-b", + mac_address="00:11:22:33:44:bb", + organization=org2, + ) + self._create_config(device=device_b) + self._create_device_connection(device=device_b) + + with patch.dict( + ORGANIZATION_ENABLED_COMMANDS, + {str(org2.pk): ("reboot",)}, + ): + payload = { + "type": "custom", + "input": {"command": "echo test"}, + "devices": [str(device_a.pk), str(device_b.pk)], + } + response = self.client.post( + reverse("connection_api:batch_command_execute"), + data=json.dumps(payload), + content_type="application/json", + ) + self.assertEqual(response.status_code, 201) + batch = BatchCommand.objects.get(pk=response.data["batch"]) + # transaction.on_commit doesn't fire in TestCase; + # trigger create_commands() manually + batch.create_commands() + batch.refresh_from_db() + command_qs = Command.objects.filter(batch_command=batch) + self.assertTrue(command_qs.filter(device=device_a).exists()) + self.assertFalse(command_qs.filter(device=device_b).exists()) + # Verify rendering works for created commands + url = reverse( + "connection_api:device_command_list", + args=[device_a.pk], + ) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + cmd_data = response.data["results"][0] + self.assertIn("type", cmd_data) + self.assertIn("input", cmd_data) diff --git a/tests/openwisp2/sample_connection/api/views.py b/tests/openwisp2/sample_connection/api/views.py index 66510f22a..22d04a924 100644 --- a/tests/openwisp2/sample_connection/api/views.py +++ b/tests/openwisp2/sample_connection/api/views.py @@ -1,6 +1,12 @@ +from openwisp_controller.connection.api.views import ( + BatchCommandDetailView as BaseBatchCommandDetailView, +) from openwisp_controller.connection.api.views import ( BatchCommandExecuteView as BaseBatchCommandExecuteView, ) +from openwisp_controller.connection.api.views import ( + BatchCommandListView as BaseBatchCommandListView, +) from openwisp_controller.connection.api.views import ( CommandDetailsView as BaseCommandDetailsView, ) @@ -49,6 +55,14 @@ class BatchCommandExecuteView(BaseBatchCommandExecuteView): pass +class BatchCommandListView(BaseBatchCommandListView): + pass + + +class BatchCommandDetailView(BaseBatchCommandDetailView): + pass + + command_list_create_view = CommandListCreateView.as_view() command_details_view = CommandDetailsView.as_view() credential_list_create_view = CredentialListCreateView.as_view() @@ -56,3 +70,5 @@ class BatchCommandExecuteView(BaseBatchCommandExecuteView): deviceconnection_list_create_view = DeviceConnectionListCreateView.as_view() deviceconnection_detail_view = DeviceConnectionDetailView.as_view() batch_command_execute_view = BatchCommandExecuteView.as_view() +batch_command_list_view = BatchCommandListView.as_view() +batch_command_detail_view = BatchCommandDetailView.as_view() diff --git a/tests/openwisp2/sample_connection/migrations/0005_batchcommand_command_batch_command.py b/tests/openwisp2/sample_connection/migrations/0005_batchcommand_command_batch_command.py index 5f361d5e5..de9613fc0 100644 --- a/tests/openwisp2/sample_connection/migrations/0005_batchcommand_command_batch_command.py +++ b/tests/openwisp2/sample_connection/migrations/0005_batchcommand_command_batch_command.py @@ -56,15 +56,15 @@ class Migration(migrations.Migration): choices=[ ("idle", "idle"), ("in-progress", "in progress"), - ("success", "completed successfully"), - ("failed", "completed with some failures"), + ("success", "success"), + ("failed", "failed"), ], default="idle", max_length=12, ), ), ( - "command_type", + "type", models.CharField( max_length=16, choices=( @@ -75,7 +75,7 @@ class Migration(migrations.Migration): ), ), ( - "command_input", + "input", models.JSONField( blank=True, encoder=django.core.serializers.json.DjangoJSONEncoder,