From 739f0a2354a5926321494913918fd5825938b6c2 Mon Sep 17 00:00:00 2001 From: AbhigyaShridhar Date: Tue, 26 Apr 2022 20:36:05 +0530 Subject: [PATCH] [admin] fixed FileNotFoundError while deleting a FirmwareImage #140 Added a try/catch block in the change_view of `BuildAdmin` class. If a `FileNotFoundError` is caught, the relavant file/directory will be temperorily re-created just before recursively calling change_view again. Fixes #140 --- openwisp_firmware_upgrader/admin.py | 62 +++++-------------- .../tests/test_models.py | 6 ++ 2 files changed, 23 insertions(+), 45 deletions(-) diff --git a/openwisp_firmware_upgrader/admin.py b/openwisp_firmware_upgrader/admin.py index 3cc464c7..116fcebe 100644 --- a/openwisp_firmware_upgrader/admin.py +++ b/openwisp_firmware_upgrader/admin.py @@ -8,7 +8,7 @@ from django.conf import settings from django.contrib import admin, messages from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME -from django.shortcuts import HttpResponseRedirect, redirect +from django.shortcuts import redirect from django.template.response import TemplateResponse from django.urls import resolve, reverse from django.utils.safestring import mark_safe @@ -80,37 +80,6 @@ def has_change_permission(self, request, obj=None): return False return True - def get_parent_object(self, request): - """ - method to get the instance from model' s parent class - in context with the ForeignKey relation - """ - resolved = resolve(request.path_info) - if resolved.kwargs: - return self.parent_model.objects.get(pk=resolved.kwargs["object_id"]) - return None - - def get_queryset(self, request): - """ - overriding queryset to remove any FirmwareImage instances, - where the image file has been manually deleted by the user - from the file system - """ - qs = super(FirmwareImageInline, self).get_queryset(request) - build = self.get_parent_object(request) - qs = qs.filter(build=build) - for imageObject in qs: - if imageObject.file is not None: - path = imageObject.file.path - if not os.path.isfile(path): - try: - type = imageObject.type - imageObject.delete() - logger.warning(f"Image object {type} was removed") - except Exception: - pass - return qs - class CategoryFilter(MultitenantOrgFilter): multitenant_lookup = 'organization_id__in' @@ -211,21 +180,24 @@ def change_view(self, request, object_id, form_url='', extra_context=None): extra_context = extra_context or {} upgrade_url = f'{app_label}_build_changelist' extra_context.update({'upgrade_url': upgrade_url}) - # preventing change_view to throw an error/exception + # preventing change_view to throw an error try: return super().change_view(request, object_id, form_url, extra_context) - except Exception as e: - if type(e).__name__ == "FileNotFoundError": - self.message_user( - request, "Please reload the page", level=logging.ERROR - ) - else: - self.message_user( - request, - "Image objects have been removed or form data didn't validate", - level=logging.ERROR, - ) - return HttpResponseRedirect(request.path) + except FileNotFoundError as e: + path = e.filename + directories = path.split('/') + n = len(directories) + dirPath = "" + for i in range(0, n - 1): + if i > 0: + dirPath = dirPath + '/' + dirPath = dirPath + directories[i] + if not os.path.isdir(dirPath): + os.mkdir(dirPath) + f = open(e.filename, "w+") + f.write("To be deleted") + f.close() + return self.change_view(request, object_id, form_url, extra_context) class UpgradeOperationForm(forms.ModelForm): diff --git a/openwisp_firmware_upgrader/tests/test_models.py b/openwisp_firmware_upgrader/tests/test_models.py index 723c4b16..c66c21ba 100644 --- a/openwisp_firmware_upgrader/tests/test_models.py +++ b/openwisp_firmware_upgrader/tests/test_models.py @@ -132,6 +132,12 @@ def test_device_fw_image_changed(self, *args): self.assertEqual(UpgradeOperation.objects.count(), 1) self.assertEqual(BatchUpgradeOperation.objects.count(), 0) + def test_device_fw_image_deleted(self, *args): + with mock.patch( + f'{self.app_label}.models.UpgradeOperation.upgrade', return_value=None + ): + pass + def test_device_fw_created(self, *args): with mock.patch( f'{self.app_label}.models.UpgradeOperation.upgrade', return_value=None