From 127a84bc1a099848af0187812d038d97c0b0d649 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Tue, 22 Feb 2022 09:47:17 -0500 Subject: [PATCH 1/5] Remove attachment files from storage on Instance objects deletion --- onadata/apps/logger/__init__.py | 12 ++++++++++++ onadata/apps/logger/signals.py | 19 +++++++++++++++++++ onadata/apps/logger/xform_instance_parser.py | 1 - onadata/libs/utils/logger_tools.py | 4 +++- onadata/settings/base.py | 2 +- 5 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 onadata/apps/logger/signals.py diff --git a/onadata/apps/logger/__init__.py b/onadata/apps/logger/__init__.py index 57d631c3f..be6e6dcef 100644 --- a/onadata/apps/logger/__init__.py +++ b/onadata/apps/logger/__init__.py @@ -1 +1,13 @@ # coding: utf-8 +from django.apps import AppConfig + + +class LoggerAppConfig(AppConfig): + + name = 'onadata.apps.logger' + + def ready(self): + # Makes sure all signal handlers are connected + # Uncomment the lines below if you need signals + from onadata.apps.logger import signals + super().ready() diff --git a/onadata/apps/logger/signals.py b/onadata/apps/logger/signals.py new file mode 100644 index 000000000..f00eeef8c --- /dev/null +++ b/onadata/apps/logger/signals.py @@ -0,0 +1,19 @@ +# coding: utf-8 +import logging +from django.db.models.signals import pre_delete +from django.dispatch import receiver + +from onadata.apps.logger.models.attachment import Attachment + + +@receiver(pre_delete, sender=Attachment) +def post_delete_asset(instance, **kwargs): + # Unfortunately, it seems that Django does not call Model.delete() on + # delete CASCADE. But this signal is called though. + # We want to delete all files when an Instance (or Attachment) object is + # deleted. + try: + instance.media_file.delete() + except Exception as e: + logger = logging.getLogger('console_logger') + logger.error(str(e), exc_info=True) diff --git a/onadata/apps/logger/xform_instance_parser.py b/onadata/apps/logger/xform_instance_parser.py index 246c9fcd3..46abccf88 100644 --- a/onadata/apps/logger/xform_instance_parser.py +++ b/onadata/apps/logger/xform_instance_parser.py @@ -332,7 +332,6 @@ def _set_attributes(self): logger = logging.getLogger("console_logger") logger.debug("Skipping duplicate attribute: %s" " with value %s" % (key, value)) - logger.debug(str(all_attributes)) else: self._attributes[key] = value diff --git a/onadata/libs/utils/logger_tools.py b/onadata/libs/utils/logger_tools.py index 1e44af288..6ef03663d 100644 --- a/onadata/libs/utils/logger_tools.py +++ b/onadata/libs/utils/logger_tools.py @@ -4,6 +4,7 @@ import sys import traceback from datetime import date, datetime +from xml.etree import ElementTree as ET from xml.parsers.expat import ExpatError import pytz @@ -53,7 +54,8 @@ clean_and_parse_xml, get_uuid_from_xml, get_deprecated_uuid_from_xml, - get_submission_date_from_xml) + get_submission_date_from_xml, +) from onadata.apps.main.models import UserProfile from onadata.apps.viewer.models.data_dictionary import DataDictionary from onadata.apps.viewer.models.parsed_instance import ParsedInstance diff --git a/onadata/settings/base.py b/onadata/settings/base.py index e4290da80..8baa88a1e 100644 --- a/onadata/settings/base.py +++ b/onadata/settings/base.py @@ -211,7 +211,7 @@ def skip_suspicious_operations(record): 'rest_framework.authtoken', 'taggit', 'readonly', - 'onadata.apps.logger', + 'onadata.apps.logger.LoggerAppConfig', 'onadata.apps.viewer', 'onadata.apps.main', 'onadata.apps.restservice', From 15cea3d683aa8f6e350ca717baa6291eab5c9311 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Fri, 18 Feb 2022 18:22:35 -0500 Subject: [PATCH 2/5] Remove full DEBUG output in console in dev mode --- onadata/settings/dev.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/onadata/settings/dev.py b/onadata/settings/dev.py index 154a64276..8db111d6c 100644 --- a/onadata/settings/dev.py +++ b/onadata/settings/dev.py @@ -6,11 +6,6 @@ # Django Framework settings # ################################ -LOGGING['root'] = { - 'handlers': ['console'], - 'level': 'DEBUG' -} - SESSION_ENGINE = "redis_sessions.session" SESSION_REDIS = RedisHelper.config(default="redis://redis_cache:6380/2") From 1c24a2658ed8727412fbadd1dfbd796744b6913c Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Tue, 22 Feb 2022 09:51:22 -0500 Subject: [PATCH 3/5] Remove unused imports --- onadata/libs/utils/logger_tools.py | 1 - 1 file changed, 1 deletion(-) diff --git a/onadata/libs/utils/logger_tools.py b/onadata/libs/utils/logger_tools.py index 6ef03663d..7b671ec6d 100644 --- a/onadata/libs/utils/logger_tools.py +++ b/onadata/libs/utils/logger_tools.py @@ -4,7 +4,6 @@ import sys import traceback from datetime import date, datetime -from xml.etree import ElementTree as ET from xml.parsers.expat import ExpatError import pytz From f1351544d8e0eb29662c25cb8524df35bc1abad8 Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Tue, 22 Feb 2022 11:31:25 -0500 Subject: [PATCH 4/5] Make changes per review of #791 --- onadata/apps/logger/__init__.py | 1 - onadata/apps/logger/signals.py | 12 ++++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/onadata/apps/logger/__init__.py b/onadata/apps/logger/__init__.py index be6e6dcef..f63bf156a 100644 --- a/onadata/apps/logger/__init__.py +++ b/onadata/apps/logger/__init__.py @@ -8,6 +8,5 @@ class LoggerAppConfig(AppConfig): def ready(self): # Makes sure all signal handlers are connected - # Uncomment the lines below if you need signals from onadata.apps.logger import signals super().ready() diff --git a/onadata/apps/logger/signals.py b/onadata/apps/logger/signals.py index f00eeef8c..e346361f2 100644 --- a/onadata/apps/logger/signals.py +++ b/onadata/apps/logger/signals.py @@ -7,13 +7,13 @@ @receiver(pre_delete, sender=Attachment) -def post_delete_asset(instance, **kwargs): - # Unfortunately, it seems that Django does not call Model.delete() on - # delete CASCADE. But this signal is called though. +def pre_delete_attachment(attachment, **kwargs): + # "Model.delete() isn’t called on related models, but the pre_delete and + # post_delete signals are sent for all deleted objects." See + # https://docs.djangoproject.com/en/2.2/ref/models/fields/#django.db.models.CASCADE # We want to delete all files when an Instance (or Attachment) object is # deleted. try: - instance.media_file.delete() + attachment.media_file.delete() except Exception as e: - logger = logging.getLogger('console_logger') - logger.error(str(e), exc_info=True) + logging.error('Failed to delete attachment: ' + str(e), exc_info=True) From 9979941838c65dbec3da76db641c8c85de86c6ba Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Tue, 22 Feb 2022 12:11:13 -0500 Subject: [PATCH 5/5] =?UTF-8?q?Supplicate=20before=20the=20gods=20of=20Dja?= =?UTF-8?q?ngo=20and=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit do not dare to change the name of the `instance` parameter of signal handlers --- onadata/apps/logger/signals.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/onadata/apps/logger/signals.py b/onadata/apps/logger/signals.py index e346361f2..04a1ad066 100644 --- a/onadata/apps/logger/signals.py +++ b/onadata/apps/logger/signals.py @@ -7,12 +7,16 @@ @receiver(pre_delete, sender=Attachment) -def pre_delete_attachment(attachment, **kwargs): +def pre_delete_attachment(instance, **kwargs): # "Model.delete() isn’t called on related models, but the pre_delete and # post_delete signals are sent for all deleted objects." See # https://docs.djangoproject.com/en/2.2/ref/models/fields/#django.db.models.CASCADE # We want to delete all files when an Instance (or Attachment) object is # deleted. + + # `instance` here means "model instance", and no, it is not allowed to + # change the name of the parameter + attachment = instance try: attachment.media_file.delete() except Exception as e: