Skip to content

Commit

Permalink
[admin] fixed FileNotFoundError while deleting a FirmwareImage openwi…
Browse files Browse the repository at this point in the history
…sp#140

I couldn't reproduce this issue with Django's standard `models.FileField`
so I figured that the problem is with the way `private_storage` is being
used. Strangely, there's no problem when delete method is called on an
instance of the model. The error is always thrown when the delete method
is called on a "modelForm" for an instance, the image file for which has
been deleted.

So the issue really is with the `private_storage` module. To get around
the error though, what I have done is:
 - `try` to return change_view
 - `catch` the `FileNotFoundError` if it occurs
 - Get the `path` from the error itself
 - check if the specified directory exists, if not create one
 - create a temporary file
 - return `change_view` again (recursion)
 - This time the file is there to be deleted and the model instance is
   deleted as well

This seems like a "brute force" solution, but since the origin of the issue
is an external module, this is the only possible workaround (As we don't want
to automatically delete any model instances because it would be strange for the
user)

Fixes openwisp#140
  • Loading branch information
AbhigyaShridhar committed Apr 26, 2022
1 parent c19f1cd commit a3e1263
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 45 deletions.
62 changes: 17 additions & 45 deletions openwisp_firmware_upgrader/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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):
Expand Down
6 changes: 6 additions & 0 deletions openwisp_firmware_upgrader/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit a3e1263

Please sign in to comment.