diff --git a/kobo/apps/hook/constants.py b/kobo/apps/hook/constants.py index 00998fc333..7b84fa03bb 100644 --- a/kobo/apps/hook/constants.py +++ b/kobo/apps/hook/constants.py @@ -1,5 +1,6 @@ # coding: utf-8 from enum import Enum +from rest_framework import status HOOK_LOG_FAILED = 0 @@ -16,3 +17,11 @@ class HookLogStatus(Enum): KOBO_INTERNAL_ERROR_STATUS_CODE = None SUBMISSION_PLACEHOLDER = '%SUBMISSION%' + +# Status codes that trigger a retry +RETRIABLE_STATUS_CODES = [ + # status.HTTP_429_TOO_MANY_REQUESTS, + status.HTTP_502_BAD_GATEWAY, + status.HTTP_503_SERVICE_UNAVAILABLE, + status.HTTP_504_GATEWAY_TIMEOUT, +] diff --git a/kobo/apps/hook/exceptions.py b/kobo/apps/hook/exceptions.py new file mode 100644 index 0000000000..1997f47c2e --- /dev/null +++ b/kobo/apps/hook/exceptions.py @@ -0,0 +1,3 @@ + +class HookRemoteServerDownError(Exception): + pass diff --git a/kobo/apps/hook/models/hook_log.py b/kobo/apps/hook/models/hook_log.py index 51c1a8a2f9..00ecd429e0 100644 --- a/kobo/apps/hook/models/hook_log.py +++ b/kobo/apps/hook/models/hook_log.py @@ -1,4 +1,3 @@ -# coding: utf-8 from datetime import timedelta import constance @@ -17,38 +16,43 @@ class HookLog(models.Model): - hook = models.ForeignKey("Hook", related_name="logs", on_delete=models.CASCADE) + hook = models.ForeignKey( + "Hook", related_name="logs", on_delete=models.CASCADE + ) uid = KpiUidField(uid_prefix="hl") - submission_id = models.IntegerField(default=0, db_index=True) # `KoBoCAT.logger.Instance.id` + submission_id = models.IntegerField( # `KoboCAT.logger.Instance.id` + default=0, db_index=True + ) tries = models.PositiveSmallIntegerField(default=0) status = models.PositiveSmallIntegerField( choices=[[e.value, e.name.title()] for e in HookLogStatus], - default=HookLogStatus.PENDING.value + default=HookLogStatus.PENDING.value, ) # Could use status_code, but will speed-up queries - status_code = models.IntegerField(default=KOBO_INTERNAL_ERROR_STATUS_CODE, null=True, blank=True) + status_code = models.IntegerField( + default=KOBO_INTERNAL_ERROR_STATUS_CODE, null=True, blank=True + ) message = models.TextField(default="") date_created = models.DateTimeField(auto_now_add=True) date_modified = models.DateTimeField(auto_now_add=True) class Meta: - ordering = ["-date_created"] + ordering = ['-date_created'] + @property def can_retry(self) -> bool: """ Return whether instance can be resent to external endpoint. Notice: even if False is returned, `self.retry()` can be triggered. """ if self.hook.active: - seconds = HookLog.get_elapsed_seconds( - constance.config.HOOK_MAX_RETRIES - ) - threshold = timezone.now() - timedelta(seconds=seconds) - # We can retry only if system has already tried 3 times. - # If log is still pending after 3 times, there was an issue, - # we allow the retry - return ( - self.status == HOOK_LOG_FAILED - or (self.date_modified < threshold and self.status == HOOK_LOG_PENDING) + # If log is still pending after `constance.config.HOOK_MAX_RETRIES` + # times, there was an issue, we allow the retry. + threshold = timezone.now() - timedelta(seconds=120) + + return self.status == HOOK_LOG_FAILED or ( + self.date_modified < threshold + and self.status == HOOK_LOG_PENDING + and self.tries >= constance.config.HOOK_MAX_RETRIES ) return False @@ -66,29 +70,6 @@ def change_status( self.save(reset_status=True) - @staticmethod - def get_elapsed_seconds(retries_count: int) -> int: - """ - Calculate number of elapsed seconds since first try. - Return the number of seconds. - """ - # We need to sum all seconds between each retry - seconds = 0 - for retries_count in range(retries_count): - # Range is zero-indexed - seconds += HookLog.get_remaining_seconds(retries_count) - - return seconds - - @staticmethod - def get_remaining_seconds(retries_count): - """ - Calculate number of remaining seconds before next retry - :param retries_count: int. - :return: int. Number of seconds - """ - return 60 * (10 ** retries_count) - def retry(self): """ Retries to send data to external service @@ -100,7 +81,7 @@ def retry(self): service_definition.send() self.refresh_from_db() except Exception as e: - logging.error("HookLog.retry - {}".format(str(e)), exc_info=True) + logging.error('HookLog.retry - {}'.format(str(e)), exc_info=True) self.change_status(HOOK_LOG_FAILED) return False @@ -110,7 +91,7 @@ def save(self, *args, **kwargs): # Update date_modified each time object is saved self.date_modified = timezone.now() # We don't want to alter tries when we only change the status - if kwargs.pop("reset_status", False) is False: + if kwargs.pop('reset_status', False) is False: self.tries += 1 self.hook.reset_totals() super().save(*args, **kwargs) diff --git a/kobo/apps/hook/models/service_definition_interface.py b/kobo/apps/hook/models/service_definition_interface.py index e721bf45b7..9b2bd1a095 100644 --- a/kobo/apps/hook/models/service_definition_interface.py +++ b/kobo/apps/hook/models/service_definition_interface.py @@ -15,7 +15,9 @@ HOOK_LOG_SUCCESS, HOOK_LOG_FAILED, KOBO_INTERNAL_ERROR_STATUS_CODE, + RETRIABLE_STATUS_CODES, ) +from ..exceptions import HookRemoteServerDownError class ServiceDefinitionInterface(metaclass=ABCMeta): @@ -41,7 +43,8 @@ def _get_data(self): 'service_json.ServiceDefinition._get_data: ' f'Hook #{self._hook.uid} - Data #{self._submission_id} - ' f'{str(e)}', - exc_info=True) + exc_info=True, + ) return None @abstractmethod @@ -71,106 +74,141 @@ def _prepare_request_kwargs(self): """ pass - def send(self): + def send(self) -> bool: """ - Sends data to external endpoint - :return: bool + Sends data to external endpoint. + + Raise an exception if something is wrong. Retries are only allowed + when `HookRemoteServerDownError` is raised. """ - success = False + if not self._data: + self.save_log( + KOBO_INTERNAL_ERROR_STATUS_CODE, 'Submission has been deleted', allow_retries=False + ) + return False + # Need to declare response before requests.post assignment in case of # RequestException response = None - if self._data: - try: - request_kwargs = self._prepare_request_kwargs() - - # Add custom headers - request_kwargs.get("headers").update( - self._hook.settings.get("custom_headers", {})) - - # Add user agent - public_domain = "- {} ".format(os.getenv("PUBLIC_DOMAIN_NAME")) \ - if os.getenv("PUBLIC_DOMAIN_NAME") else "" - request_kwargs.get("headers").update({ - "User-Agent": "KoboToolbox external service {}#{}".format( - public_domain, - self._hook.uid) - }) - - # If the request needs basic authentication with username and - # password, let's provide them - if self._hook.auth_level == Hook.BASIC_AUTH: - request_kwargs.update({ - "auth": (self._hook.settings.get("username"), - self._hook.settings.get("password")) - }) - - ssrf_protect_options = {} - if constance.config.SSRF_ALLOWED_IP_ADDRESS.strip(): - ssrf_protect_options['allowed_ip_addresses'] = constance.\ - config.SSRF_ALLOWED_IP_ADDRESS.strip().split('\r\n') - - if constance.config.SSRF_DENIED_IP_ADDRESS.strip(): - ssrf_protect_options['denied_ip_addresses'] = constance.\ - config.SSRF_DENIED_IP_ADDRESS.strip().split('\r\n') - - SSRFProtect.validate(self._hook.endpoint, - options=ssrf_protect_options) - - response = requests.post(self._hook.endpoint, timeout=30, - **request_kwargs) - response.raise_for_status() - self.save_log(response.status_code, response.text, True) - success = True - except requests.exceptions.RequestException as e: - # If request fails to communicate with remote server. - # Exception is raised before request.post can return something. - # Thus, response equals None - status_code = KOBO_INTERNAL_ERROR_STATUS_CODE - text = str(e) - if response is not None: - text = response.text - status_code = response.status_code - self.save_log(status_code, text) - except SSRFProtectException as e: - logging.error( - 'service_json.ServiceDefinition.send: ' - f'Hook #{self._hook.uid} - ' - f'Data #{self._submission_id} - ' - f'{str(e)}', - exc_info=True) - self.save_log( - KOBO_INTERNAL_ERROR_STATUS_CODE, - f'{self._hook.endpoint} is not allowed') - except Exception as e: - logging.error( - 'service_json.ServiceDefinition.send: ' - f'Hook #{self._hook.uid} - ' - f'Data #{self._submission_id} - ' - f'{str(e)}', - exc_info=True) - self.save_log( - KOBO_INTERNAL_ERROR_STATUS_CODE, - "An error occurred when sending data to external endpoint") - else: - self.save_log( - KOBO_INTERNAL_ERROR_STATUS_CODE, - 'Submission has been deleted' + try: + request_kwargs = self._prepare_request_kwargs() + + # Add custom headers + request_kwargs.get('headers').update( + self._hook.settings.get('custom_headers', {}) ) - return success + # Add user agent + public_domain = ( + '- {} '.format(os.getenv('PUBLIC_DOMAIN_NAME')) + if os.getenv('PUBLIC_DOMAIN_NAME') + else '' + ) + request_kwargs.get('headers').update( + { + 'User-Agent': 'KoboToolbox external service {}#{}'.format( + public_domain, self._hook.uid + ) + } + ) - def save_log(self, status_code: int, message: str, success: bool = False): + # If the request needs basic authentication with username and + # password, let's provide them + if self._hook.auth_level == Hook.BASIC_AUTH: + request_kwargs.update( + { + 'auth': ( + self._hook.settings.get('username'), + self._hook.settings.get('password'), + ) + } + ) + + ssrf_protect_options = {} + if constance.config.SSRF_ALLOWED_IP_ADDRESS.strip(): + ssrf_protect_options[ + 'allowed_ip_addresses' + ] = constance.config.SSRF_ALLOWED_IP_ADDRESS.strip().split( + '\r\n' + ) + + if constance.config.SSRF_DENIED_IP_ADDRESS.strip(): + ssrf_protect_options[ + 'denied_ip_addresses' + ] = constance.config.SSRF_DENIED_IP_ADDRESS.strip().split( + '\r\n' + ) + + SSRFProtect.validate( + self._hook.endpoint, options=ssrf_protect_options + ) + + response = requests.post( + self._hook.endpoint, timeout=30, **request_kwargs + ) + response.raise_for_status() + self.save_log(response.status_code, response.text, success=True) + + return True + + except requests.exceptions.RequestException as e: + # If request fails to communicate with remote server. + # Exception is raised before request.post can return something. + # Thus, response equals None + status_code = KOBO_INTERNAL_ERROR_STATUS_CODE + text = str(e) + if response is not None: + text = response.text + status_code = response.status_code + + if status_code in RETRIABLE_STATUS_CODES: + self.save_log(status_code, text, allow_retries=True) + raise HookRemoteServerDownError + + self.save_log(status_code, text) + raise + except SSRFProtectException as e: + logging.error( + 'service_json.ServiceDefinition.send: ' + f'Hook #{self._hook.uid} - ' + f'Data #{self._submission_id} - ' + f'{str(e)}', + exc_info=True, + ) + self.save_log( + KOBO_INTERNAL_ERROR_STATUS_CODE, + f'{self._hook.endpoint} is not allowed' + ) + raise + except Exception as e: + logging.error( + 'service_json.ServiceDefinition.send: ' + f'Hook #{self._hook.uid} - ' + f'Data #{self._submission_id} - ' + f'{str(e)}', + exc_info=True, + ) + self.save_log( + KOBO_INTERNAL_ERROR_STATUS_CODE, + 'An error occurred when sending ' + f'data to external endpoint: {str(e)}', + ) + raise + + def save_log( + self, + status_code: int, + message: str, + success: bool = False, + allow_retries: bool = False, + ): """ Updates/creates log entry with: - `status_code` as the HTTP status code of the remote server response - `message` as the content of the remote server response """ - fields = { - 'hook': self._hook, - 'submission_id': self._submission_id - } + fields = {'hook': self._hook, 'submission_id': self._submission_id} try: # Try to load the log with a multiple field FK because # we don't know the log `uid` in this context, but we do know @@ -181,7 +219,7 @@ def save_log(self, status_code: int, message: str, success: bool = False): if success: log.status = HOOK_LOG_SUCCESS - elif log.tries >= constance.config.HOOK_MAX_RETRIES: + elif not allow_retries or log.tries >= constance.config.HOOK_MAX_RETRIES: log.status = HOOK_LOG_FAILED log.status_code = status_code diff --git a/kobo/apps/hook/tasks.py b/kobo/apps/hook/tasks.py index 6ff659961f..c87cd21076 100644 --- a/kobo/apps/hook/tasks.py +++ b/kobo/apps/hook/tasks.py @@ -9,44 +9,37 @@ from django.utils import translation, timezone from django_celery_beat.models import PeriodicTask +from kobo.celery import celery_app from kpi.utils.log import logging from .constants import HOOK_LOG_FAILED +from .exceptions import HookRemoteServerDownError from .models import Hook, HookLog - - -@shared_task(bind=True) -def service_definition_task(self, hook_id, submission_id): +from .utils.lazy import LazyMaxRetriesInt + + +@celery_app.task( + autoretry_for=(HookRemoteServerDownError,), + retry_backoff=60, + retry_backoff_max=1200, + max_retries=LazyMaxRetriesInt(), + retry_jitter=True, + queue='kpi_low_priority_queue', +) +def service_definition_task(hook_id: int, submission_id: int) -> bool: """ Tries to send data to the endpoint of the hook It retries n times (n = `constance.config.HOOK_MAX_RETRIES`) - - - after 1 minutes, - - after 10 minutes, - - after 100 minutes - etc ... - - :param self: Celery.Task. - :param hook_id: int. Hook PK - :param submission_id: int. Instance PK """ hook = Hook.objects.get(id=hook_id) # Use camelcase (even if it's not PEP-8 compliant) # because variable represents the class, not the instance. - ServiceDefinition = hook.get_service_definition() + ServiceDefinition = hook.get_service_definition() # noqa service_definition = ServiceDefinition(hook, submission_id) - if not service_definition.send(): - # Countdown is in seconds - countdown = HookLog.get_remaining_seconds(self.request.retries) - raise self.retry(countdown=countdown, max_retries=constance.config.HOOK_MAX_RETRIES) - - return True + return service_definition.send() @shared_task -def retry_all_task(hooklogs_ids): - """ - :param list: . - """ +def retry_all_task(hooklogs_ids: int): hook_logs = HookLog.objects.filter(id__in=hooklogs_ids) for hook_log in hook_logs: hook_log.retry() @@ -71,22 +64,24 @@ def failures_reports(): if failures_reports_period_task: last_run_at = failures_reports_period_task.last_run_at - queryset = HookLog.objects.filter(hook__email_notification=True, - status=HOOK_LOG_FAILED) + queryset = HookLog.objects.filter( + hook__email_notification=True, status=HOOK_LOG_FAILED + ) if last_run_at: queryset = queryset.filter(date_modified__gte=last_run_at) - queryset = queryset.order_by('hook__asset__name', - 'hook__uid', - '-date_modified') + queryset = queryset.order_by( + 'hook__asset__name', 'hook__uid', '-date_modified' + ) # PeriodicTask are updated every 3 minutes (default). # It means, if this task interval is less than 3 minutes, some data can be duplicated in emails. # Setting `beat-sync-every` to 1, makes PeriodicTask to be updated before running the task. # So, we need to update it manually. # see: http://docs.celeryproject.org/en/latest/userguide/configuration.html#beat-sync-every - PeriodicTask.objects.filter(task=beat_schedule.get("task")). \ - update(last_run_at=timezone.now()) + PeriodicTask.objects.filter(task=beat_schedule.get('task')).update( + last_run_at=timezone.now() + ) records = {} max_length = 0 @@ -147,9 +142,12 @@ def failures_reports(): text_content = plain_text_template.render(variables) html_content = html_template.render(variables) - msg = EmailMultiAlternatives(translation.gettext('REST Services Failure Report'), text_content, - constance.config.SUPPORT_EMAIL, - [record.get('email')]) + msg = EmailMultiAlternatives( + translation.gettext('REST Services Failure Report'), + text_content, + constance.config.SUPPORT_EMAIL, + [record.get('email')], + ) msg.attach_alternative(html_content, 'text/html') email_messages.append(msg) diff --git a/kobo/apps/hook/tests/hook_test_case.py b/kobo/apps/hook/tests/hook_test_case.py index b4ea20658e..bbf8799806 100644 --- a/kobo/apps/hook/tests/hook_test_case.py +++ b/kobo/apps/hook/tests/hook_test_case.py @@ -1,6 +1,7 @@ # coding: utf-8 import json +import pytest import responses from django.conf import settings from django.urls import reverse @@ -11,16 +12,10 @@ from kpi.exceptions import BadFormatException from kpi.tests.kpi_test_case import KpiTestCase from ..constants import HOOK_LOG_FAILED +from ..exceptions import HookRemoteServerDownError from ..models import HookLog, Hook -class MockSSRFProtect: - - @staticmethod - def _get_ip_address(url): - return ip_address('1.2.3.4') - - class HookTestCase(KpiTestCase): def setUp(self): @@ -94,26 +89,45 @@ def _send_and_fail(self): :return: dict """ + first_hooklog_response = self._send_and_wait_for_retry() + + # Fakes Celery n retries by forcing status to `failed` + # (where n is `settings.HOOKLOG_MAX_RETRIES`) + first_hooklog = HookLog.objects.get( + uid=first_hooklog_response.get('uid') + ) + first_hooklog.change_status(HOOK_LOG_FAILED) + + return first_hooklog_response + + def _send_and_wait_for_retry(self): self.hook = self._create_hook() ServiceDefinition = self.hook.get_service_definition() submissions = self.asset.deployment.get_submissions(self.asset.owner) submission_id = submissions[0]['_id'] service_definition = ServiceDefinition(self.hook, submission_id) - first_mock_response = {'error': 'not found'} + first_mock_response = {'error': 'gateway timeout'} # Mock first request's try - responses.add(responses.POST, self.hook.endpoint, - json=first_mock_response, status=status.HTTP_404_NOT_FOUND) + responses.add( + responses.POST, + self.hook.endpoint, + json=first_mock_response, + status=status.HTTP_504_GATEWAY_TIMEOUT, + ) # Mock next requests' tries - responses.add(responses.POST, self.hook.endpoint, - status=status.HTTP_200_OK, - content_type='application/json') + responses.add( + responses.POST, + self.hook.endpoint, + status=status.HTTP_200_OK, + content_type='application/json', + ) # Try to send data to external endpoint - success = service_definition.send() - self.assertFalse(success) + with pytest.raises(HookRemoteServerDownError): + service_definition.send() # Retrieve the corresponding log url = reverse('hook-log-list', kwargs={ @@ -126,20 +140,13 @@ def _send_and_fail(self): # Result should match first try self.assertEqual( - first_hooklog_response.get('status_code'), status.HTTP_404_NOT_FOUND + first_hooklog_response.get('status_code'), + status.HTTP_504_GATEWAY_TIMEOUT, ) self.assertEqual( json.loads(first_hooklog_response.get('message')), first_mock_response, ) - - # Fakes Celery n retries by forcing status to `failed` - # (where n is `settings.HOOKLOG_MAX_RETRIES`) - first_hooklog = HookLog.objects.get( - uid=first_hooklog_response.get('uid') - ) - first_hooklog.change_status(HOOK_LOG_FAILED) - return first_hooklog_response def __prepare_submission(self): diff --git a/kobo/apps/hook/tests/test_api_hook.py b/kobo/apps/hook/tests/test_api_hook.py index 511d6486f2..c6368f568d 100644 --- a/kobo/apps/hook/tests/test_api_hook.py +++ b/kobo/apps/hook/tests/test_api_hook.py @@ -1,12 +1,15 @@ # coding: utf-8 import json +import pytest import responses from constance.test import override_config from django.urls import reverse -from mock import patch +from ipaddress import ip_address +from mock import patch, MagicMock from rest_framework import status + from kobo.apps.hook.constants import ( HOOK_LOG_FAILED, HOOK_LOG_PENDING, @@ -21,7 +24,8 @@ PERM_CHANGE_ASSET ) from kpi.utils.datetime import several_minutes_from_now -from .hook_test_case import HookTestCase, MockSSRFProtect +from .hook_test_case import HookTestCase +from ..exceptions import HookRemoteServerDownError class ApiHookTestCase(HookTestCase): @@ -56,42 +60,6 @@ def test_anonymous_access(self): def test_create_hook(self): self._create_hook() - @patch('ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', - new=MockSSRFProtect._get_ip_address) - @responses.activate - def test_data_submission(self): - # Create first hook - first_hook = self._create_hook(name="dummy external service", - endpoint="http://dummy.service.local/", - settings={}) - responses.add(responses.POST, first_hook.endpoint, - status=status.HTTP_200_OK, - content_type="application/json") - hook_signal_url = reverse("hook-signal-list", kwargs={"parent_lookup_asset": self.asset.uid}) - - submissions = self.asset.deployment.get_submissions(self.asset.owner) - data = {'submission_id': submissions[0]['_id']} - response = self.client.post(hook_signal_url, data=data, format='json') - self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) - - # Create second hook - second_hook = self._create_hook(name="other dummy external service", - endpoint="http://otherdummy.service.local/", - settings={}) - responses.add(responses.POST, second_hook.endpoint, - status=status.HTTP_200_OK, - content_type="application/json") - - response = self.client.post(hook_signal_url, data=data, format='json') - self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) - - response = self.client.post(hook_signal_url, data=data, format='json') - self.assertEqual(response.status_code, status.HTTP_409_CONFLICT) - - data = {'submission_id': 4} # Instance doesn't belong to `self.asset` - response = self.client.post(hook_signal_url, data=data, format='json') - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_editor_access(self): hook = self._create_hook() @@ -205,18 +173,20 @@ def test_partial_update_hook(self): self.assertFalse(hook.active) self.assertEqual(hook.name, "some disabled external service") - @patch('ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', - new=MockSSRFProtect._get_ip_address) + @patch( + 'ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', + new=MagicMock(return_value=ip_address('1.2.3.4')) + ) @responses.activate def test_send_and_retry(self): first_log_response = self._send_and_fail() # Let's retry through API call - retry_url = reverse("hook-log-retry", kwargs={ - "parent_lookup_asset": self.asset.uid, - "parent_lookup_hook": self.hook.uid, - "uid": first_log_response.get("uid") + retry_url = reverse('hook-log-retry', kwargs={ + 'parent_lookup_asset': self.asset.uid, + 'parent_lookup_hook': self.hook.uid, + 'uid': first_log_response.get('uid') }) # It should be a success @@ -224,17 +194,49 @@ def test_send_and_retry(self): self.assertEqual(response.status_code, status.HTTP_200_OK) # Let's check if logs has 2 tries - detail_url = reverse("hook-log-detail", kwargs={ - "parent_lookup_asset": self.asset.uid, - "parent_lookup_hook": self.hook.uid, - "uid": first_log_response.get("uid") + detail_url = reverse('hook-log-detail', kwargs={ + 'parent_lookup_asset': self.asset.uid, + 'parent_lookup_hook': self.hook.uid, + 'uid': first_log_response.get('uid') + }) + + response = self.client.get(detail_url, format=SUBMISSION_FORMAT_TYPE_JSON) + self.assertEqual(response.data.get('tries'), 2) + + @patch( + 'ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', + new=MagicMock(return_value=ip_address('1.2.3.4')) + ) + @responses.activate + def test_send_and_cannot_retry(self): + + first_log_response = self._send_and_wait_for_retry() + + # Let's retry through API call + retry_url = reverse('hook-log-retry', kwargs={ + 'parent_lookup_asset': self.asset.uid, + 'parent_lookup_hook': self.hook.uid, + 'uid': first_log_response.get('uid') + }) + + # It should be a failure. The hook log is going to be retried + response = self.client.patch(retry_url, format=SUBMISSION_FORMAT_TYPE_JSON) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + # Let's check if logs has 2 tries + detail_url = reverse('hook-log-detail', kwargs={ + 'parent_lookup_asset': self.asset.uid, + 'parent_lookup_hook': self.hook.uid, + 'uid': first_log_response.get('uid') }) response = self.client.get(detail_url, format=SUBMISSION_FORMAT_TYPE_JSON) - self.assertEqual(response.data.get("tries"), 2) + self.assertEqual(response.data.get('tries'), 1) - @patch('ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', - new=MockSSRFProtect._get_ip_address) + @patch( + 'ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', + new=MagicMock(return_value=ip_address('1.2.3.4')) + ) @responses.activate def test_payload_template(self): @@ -317,8 +319,10 @@ def test_payload_template_validation(self): } self.assertEqual(response.data, expected_response) - @patch('ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', - new=MockSSRFProtect._get_ip_address) + @patch( + 'ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', + new=MagicMock(return_value=ip_address('1.2.3.4')) + ) @responses.activate def test_hook_log_filter_success(self): # Create success hook @@ -352,17 +356,24 @@ def test_hook_log_filter_success(self): response = self.client.get(f'{hook_log_url}?status={HOOK_LOG_FAILED}', format='json') self.assertEqual(response.data.get('count'), 0) - @patch('ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', - new=MockSSRFProtect._get_ip_address) + @patch( + 'ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', + new=MagicMock(return_value=ip_address('1.2.3.4')) + ) @responses.activate def test_hook_log_filter_failure(self): # Create failing hook - hook = self._create_hook(name="failing hook", - endpoint="http://failing.service.local/", - settings={}) - responses.add(responses.POST, hook.endpoint, - status=status.HTTP_500_INTERNAL_SERVER_ERROR, - content_type="application/json") + hook = self._create_hook( + name='failing hook', + endpoint='http://failing.service.local/', + settings={}, + ) + responses.add( + responses.POST, + hook.endpoint, + status=status.HTTP_504_GATEWAY_TIMEOUT, + content_type="application/json", + ) # simulate a submission ServiceDefinition = hook.get_service_definition() @@ -370,8 +381,8 @@ def test_hook_log_filter_failure(self): submission_id = submissions[0]['_id'] service_definition = ServiceDefinition(hook, submission_id) - success = service_definition.send() - self.assertFalse(success) + with pytest.raises(HookRemoteServerDownError): + service_definition.send() # Get log for the failing hook hook_log_url = reverse('hook-log-list', kwargs={ @@ -380,18 +391,24 @@ def test_hook_log_filter_failure(self): }) # There should be no success log for the failing hook - response = self.client.get(f'{hook_log_url}?status={HOOK_LOG_SUCCESS}', format='json') + response = self.client.get( + f'{hook_log_url}?status={HOOK_LOG_SUCCESS}', format='json' + ) self.assertEqual(response.data.get('count'), 0) # There should be a pending log for the failing hook - response = self.client.get(f'{hook_log_url}?status={HOOK_LOG_PENDING}', format='json') + response = self.client.get( + f'{hook_log_url}?status={HOOK_LOG_PENDING}', format='json' + ) self.assertEqual(response.data.get('count'), 1) def test_hook_log_filter_validation(self): # Create hook - hook = self._create_hook(name="success hook", - endpoint="http://hook.service.local/", - settings={}) + hook = self._create_hook( + name='success hook', + endpoint='http://hook.service.local/', + settings={}, + ) # Get log for the success hook hook_log_url = reverse('hook-log-list', kwargs={ @@ -403,14 +420,16 @@ def test_hook_log_filter_validation(self): response = self.client.get(f'{hook_log_url}?status=abc', format='json') self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - @patch('ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', - new=MockSSRFProtect._get_ip_address) + @patch( + 'ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', + new=MagicMock(return_value=ip_address('1.2.3.4')) + ) @responses.activate def test_hook_log_filter_date(self): # Create success hook - hook = self._create_hook(name="date hook", - endpoint="http://date.service.local/", - settings={}) + hook = self._create_hook( + name="date hook", endpoint="http://date.service.local/", settings={} + ) responses.add(responses.POST, hook.endpoint, status=status.HTTP_200_OK, content_type="application/json") diff --git a/kobo/apps/hook/tests/test_email.py b/kobo/apps/hook/tests/test_email.py index 2b1f7d0fdf..2a587340cb 100644 --- a/kobo/apps/hook/tests/test_email.py +++ b/kobo/apps/hook/tests/test_email.py @@ -5,9 +5,10 @@ from django.template.loader import get_template from django.utils import translation, dateparse from django_celery_beat.models import PeriodicTask, CrontabSchedule -from mock import patch +from ipaddress import ip_address +from mock import patch, MagicMock -from .hook_test_case import HookTestCase, MockSSRFProtect +from .hook_test_case import HookTestCase from ..tasks import failures_reports @@ -28,8 +29,10 @@ def _create_periodic_task(self): return periodic_task - @patch('ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', - new=MockSSRFProtect._get_ip_address) + @patch( + 'ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', + new=MagicMock(return_value=ip_address('1.2.3.4')) + ) @responses.activate def test_notifications(self): self._create_periodic_task() diff --git a/kobo/apps/hook/tests/test_ssrf.py b/kobo/apps/hook/tests/test_ssrf.py index 89a71f4d4c..df3ebd193f 100644 --- a/kobo/apps/hook/tests/test_ssrf.py +++ b/kobo/apps/hook/tests/test_ssrf.py @@ -1,21 +1,24 @@ -# coding: utf-8 - +import pytest import responses from constance.test import override_config -from mock import patch +from ipaddress import ip_address +from mock import patch, MagicMock from rest_framework import status +from ssrf_protect.exceptions import SSRFProtectException from kobo.apps.hook.constants import ( - HOOK_LOG_PENDING, + HOOK_LOG_FAILED, KOBO_INTERNAL_ERROR_STATUS_CODE ) -from .hook_test_case import HookTestCase, MockSSRFProtect +from .hook_test_case import HookTestCase class SSRFHookTestCase(HookTestCase): - @patch('ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', - new=MockSSRFProtect._get_ip_address) + @patch( + 'ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', + new=MagicMock(return_value=ip_address('1.2.3.4')) + ) @override_config(SSRF_DENIED_IP_ADDRESS='1.2.3.4') @responses.activate def test_send_with_ssrf_options(self): @@ -34,9 +37,12 @@ def test_send_with_ssrf_options(self): content_type='application/json') # Try to send data to external endpoint - success = service_definition.send() - self.assertFalse(success) + # Note: it should failed because we explicitly deny 1.2.3.4 and + # SSRFProtect._get_ip_address is mocked to return 1.2.3.4 + with pytest.raises(SSRFProtectException): + service_definition.send() + hook_log = hook.logs.all()[0] self.assertEqual(hook_log.status_code, KOBO_INTERNAL_ERROR_STATUS_CODE) - self.assertEqual(hook_log.status, HOOK_LOG_PENDING) + self.assertEqual(hook_log.status, HOOK_LOG_FAILED) self.assertTrue('is not allowed' in hook_log.message) diff --git a/kobo/apps/hook/tests/test_utils.py b/kobo/apps/hook/tests/test_utils.py new file mode 100644 index 0000000000..931f66ce1e --- /dev/null +++ b/kobo/apps/hook/tests/test_utils.py @@ -0,0 +1,53 @@ +import responses +from ipaddress import ip_address +from mock import patch, MagicMock +from rest_framework import status + +from .hook_test_case import HookTestCase +from ..utils.services import call_services + + +class HookUtilsTestCase(HookTestCase): + + @patch( + 'ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address', + new=MagicMock(return_value=ip_address('1.2.3.4')) + ) + @responses.activate + def test_data_submission(self): + # Create first hook + first_hook = self._create_hook( + name='dummy external service', + endpoint='http://dummy.service.local/', + settings={}, + ) + responses.add( + responses.POST, + first_hook.endpoint, + status=status.HTTP_200_OK, + content_type='application/json', + ) + + submissions = self.asset.deployment.get_submissions(self.asset.owner) + submission_id = submissions[0]['_id'] + assert call_services(self.asset.uid, submission_id) is True + + # Create second hook + second_hook = self._create_hook( + name='other dummy external service', + endpoint='http://otherdummy.service.local/', + settings={}, + ) + responses.add( + responses.POST, + second_hook.endpoint, + status=status.HTTP_200_OK, + content_type='application/json', + ) + # Since second hook hasn't received the submission, `call_services` + # should still return True + assert call_services(self.asset.uid, submission_id) is True + + # But if we try again, it should return False (we cannot send the same + # submission twice to the same external endpoint). + assert call_services(self.asset.uid, submission_id) is False diff --git a/kobo/apps/hook/utils.py b/kobo/apps/hook/utils.py deleted file mode 100644 index 55fe5760e0..0000000000 --- a/kobo/apps/hook/utils.py +++ /dev/null @@ -1,32 +0,0 @@ -# coding: utf-8 -from .models.hook_log import HookLog -from .tasks import service_definition_task - - -class HookUtils: - - @staticmethod - def call_services(asset: 'kpi.models.asset.Asset', submission_id: int): - """ - Delegates to Celery data submission to remote servers - """ - # Retrieve `Hook` ids, to send data to their respective endpoint. - hooks_ids = ( - asset.hooks.filter(active=True) - .values_list('id', flat=True) - .distinct() - ) - # At least, one of the hooks must not have a log that corresponds to - # `submission_id` - # to make success equal True - success = False - for hook_id in hooks_ids: - if not HookLog.objects.filter( - submission_id=submission_id, hook_id=hook_id - ).exists(): - success = True - service_definition_task.apply_async( - queue='kpi_low_priority_queue', args=(hook_id, submission_id) - ) - - return success diff --git a/kobo/apps/hook/utils/__init__.py b/kobo/apps/hook/utils/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/kobo/apps/hook/utils/lazy.py b/kobo/apps/hook/utils/lazy.py new file mode 100644 index 0000000000..58a64c9d1a --- /dev/null +++ b/kobo/apps/hook/utils/lazy.py @@ -0,0 +1,44 @@ +import constance + + +class LazyMaxRetriesInt: + """ + constance settings cannot be used as default parameters of a function. + This wrapper helps to return the value of `constance.config.HOOK_MAX_RETRIES` + on demand. + """ + def __call__(self, *args, **kwargs): + return constance.config.HOOK_MAX_RETRIES + + def __repr__(self): + return str(constance.config.HOOK_MAX_RETRIES) + + def __eq__(self, other): + if isinstance(other, int): + return self() == other + return NotImplemented + + def __ne__(self, other): + if isinstance(other, int): + return self() != other + return NotImplemented + + def __lt__(self, other): + if isinstance(other, int): + return self() < other + return NotImplemented + + def __le__(self, other): + if isinstance(other, int): + return self() <= other + return NotImplemented + + def __gt__(self, other): + if isinstance(other, int): + return self() > other + return NotImplemented + + def __ge__(self, other): + if isinstance(other, int): + return self() >= other + return NotImplemented diff --git a/kobo/apps/hook/utils/services.py b/kobo/apps/hook/utils/services.py new file mode 100644 index 0000000000..f0d05c7ad1 --- /dev/null +++ b/kobo/apps/hook/utils/services.py @@ -0,0 +1,27 @@ +from ..models.hook import Hook +from ..models.hook_log import HookLog +from ..tasks import service_definition_task + + +def call_services(asset_uid: str, submission_id: int) -> bool: + """ + Delegates to Celery data submission to remote servers + """ + # Retrieve `Hook` ids, to send data to their respective endpoint. + hooks_ids = ( + Hook.objects.filter(asset__uid=asset_uid, active=True) + .values_list('id', flat=True) + .distinct() + ) + # At least, one of the hooks must not have a log that corresponds to + # `submission_id` + # to make success equal True + success = False + + for hook_id in hooks_ids: + if not HookLog.objects.filter( + submission_id=submission_id, hook_id=hook_id + ).exists(): + success = True + service_definition_task.delay(hook_id, submission_id) + return success diff --git a/kobo/apps/hook/views/v1/__init__.py b/kobo/apps/hook/views/v1/__init__.py index 66a9504388..c3bb54f968 100644 --- a/kobo/apps/hook/views/v1/__init__.py +++ b/kobo/apps/hook/views/v1/__init__.py @@ -1,4 +1,3 @@ # coding: utf-8 from .hook import HookViewSet from .hook_log import HookLogViewSet -from .hook_signal import HookSignalViewSet diff --git a/kobo/apps/hook/views/v1/hook_signal.py b/kobo/apps/hook/views/v1/hook_signal.py deleted file mode 100644 index 37b0a5c5b3..0000000000 --- a/kobo/apps/hook/views/v1/hook_signal.py +++ /dev/null @@ -1,33 +0,0 @@ -# coding: utf-8 -from kobo.apps.hook.views.v2.hook_signal import HookSignalViewSet as HookSignalViewSetV2 - - -class HookSignalViewSet(HookSignalViewSetV2): - """ - ## This document is for a deprecated version of kpi's API. - - **Please upgrade to latest release `/api/v2/assets/hook-signal/`** - - - This endpoint is only used to trigger asset's hooks if any. - - Tells the hooks to post an instance to external servers. -
-    POST /api/v2/assets/{uid}/hook-signal/
-    
- - - > Example - > - > curl -X POST https://[kpi-url]/assets/aSAvYreNzVEkrWg5Gdcvg/hook-signal/ - - - > **Expected payload** - > - > { - > "submission_id": {integer} - > } - - """ - - pass diff --git a/kobo/apps/hook/views/v2/__init__.py b/kobo/apps/hook/views/v2/__init__.py index 66a9504388..c3bb54f968 100644 --- a/kobo/apps/hook/views/v2/__init__.py +++ b/kobo/apps/hook/views/v2/__init__.py @@ -1,4 +1,3 @@ # coding: utf-8 from .hook import HookViewSet from .hook_log import HookLogViewSet -from .hook_signal import HookSignalViewSet diff --git a/kobo/apps/hook/views/v2/hook.py b/kobo/apps/hook/views/v2/hook.py index 9c4f795b0d..1e2a975868 100644 --- a/kobo/apps/hook/views/v2/hook.py +++ b/kobo/apps/hook/views/v2/hook.py @@ -174,13 +174,20 @@ def retry(self, request, uid=None, *args, **kwargs): response = {"detail": t("Task successfully scheduled")} status_code = status.HTTP_200_OK if hook.active: - seconds = HookLog.get_elapsed_seconds(constance.config.HOOK_MAX_RETRIES) - threshold = timezone.now() - timedelta(seconds=seconds) - - records = hook.logs.filter(Q(date_modified__lte=threshold, - status=HOOK_LOG_PENDING) | - Q(status=HOOK_LOG_FAILED)). \ - values_list("id", "uid").distinct() + threshold = timezone.now() - timedelta(seconds=120) + + records = ( + hook.logs.filter( + Q( + date_modified__lte=threshold, + status=HOOK_LOG_PENDING, + tries__gte=constance.config.HOOK_MAX_RETRIES, + ) + | Q(status=HOOK_LOG_FAILED) + ) + .values_list('id', 'uid') + .distinct() + ) # Prepare lists of ids hooklogs_ids = [] hooklogs_uids = [] @@ -190,7 +197,9 @@ def retry(self, request, uid=None, *args, **kwargs): if len(records) > 0: # Mark all logs as PENDING - HookLog.objects.filter(id__in=hooklogs_ids).update(status=HOOK_LOG_PENDING) + HookLog.objects.filter(id__in=hooklogs_ids).update( + status=HOOK_LOG_PENDING + ) # Delegate to Celery retry_all_task.apply_async( queue='kpi_low_priority_queue', args=(hooklogs_ids,) diff --git a/kobo/apps/hook/views/v2/hook_log.py b/kobo/apps/hook/views/v2/hook_log.py index ab6e1e29c9..6b5a8874c1 100644 --- a/kobo/apps/hook/views/v2/hook_log.py +++ b/kobo/apps/hook/views/v2/hook_log.py @@ -108,21 +108,24 @@ def retry(self, request, uid=None, *args, **kwargs): status_code = status.HTTP_200_OK hook_log = self.get_object() - if hook_log.can_retry(): + if hook_log.can_retry: hook_log.change_status() success = hook_log.retry() if success: # Return status_code of remote server too. # `response["status_code"]` is not the same as `status_code` - response["detail"] = hook_log.message - response["status_code"] = hook_log.status_code + response['detail'] = hook_log.message + response['status_code'] = hook_log.status_code else: - response["detail"] = t( - "An error has occurred when sending the data. Please try again later.") + response['detail'] = t( + 'An error has occurred when sending the data. ' + 'Please try again later.' + ) status_code = status.HTTP_500_INTERNAL_SERVER_ERROR else: - response["detail"] = t( - "Data is being or has already been processed") + response['detail'] = t( + 'Data is being or has already been processed' + ) status_code = status.HTTP_400_BAD_REQUEST return Response(response, status=status_code) diff --git a/kobo/apps/hook/views/v2/hook_signal.py b/kobo/apps/hook/views/v2/hook_signal.py deleted file mode 100644 index 5bab71ef46..0000000000 --- a/kobo/apps/hook/views/v2/hook_signal.py +++ /dev/null @@ -1,84 +0,0 @@ -# coding: utf-8 -from django.http import Http404 -from django.utils.translation import gettext_lazy as t -from rest_framework import status, viewsets, serializers -from rest_framework.response import Response -from rest_framework.pagination import _positive_int as positive_int -from rest_framework_extensions.mixins import NestedViewSetMixin - - -from kobo.apps.hook.utils import HookUtils -from kpi.models import Asset -from kpi.utils.viewset_mixins import AssetNestedObjectViewsetMixin - - -class HookSignalViewSet(AssetNestedObjectViewsetMixin, NestedViewSetMixin, - viewsets.ViewSet): - """ - ## - This endpoint is only used to trigger asset's hooks if any. - - Tells the hooks to post an instance to external servers. -
-    POST /api/v2/assets/{uid}/hook-signal/
-    
- - - > Example - > - > curl -X POST https://[kpi-url]/api/v2/assets/aSAvYreNzVEkrWg5Gdcvg/hook-signal/ - - - > **Expected payload** - > - > { - > "submission_id": {integer} - > } - - """ - - parent_model = Asset - - def create(self, request, *args, **kwargs): - """ - It's only used to trigger hook services of the Asset (so far). - - :param request: - :return: - """ - try: - submission_id = positive_int( - request.data.get('submission_id'), strict=True) - except ValueError: - raise serializers.ValidationError( - {'submission_id': t('A positive integer is required.')}) - - # Check if instance really belongs to Asset. - try: - submission = self.asset.deployment.get_submission(submission_id, - request.user) - except ValueError: - raise Http404 - - if not (submission and int(submission['_id']) == submission_id): - raise Http404 - - if HookUtils.call_services(self.asset, submission_id): - # Follow Open Rosa responses by default - response_status_code = status.HTTP_202_ACCEPTED - response = { - "detail": t( - "We got and saved your data, but may not have " - "fully processed it. You should not try to resubmit.") - } - else: - # call_services() refused to launch any task because this - # instance already has a `HookLog` - response_status_code = status.HTTP_409_CONFLICT - response = { - "detail": t( - "Your data for instance {} has been already " - "submitted.".format(submission_id)) - } - - return Response(response, status=response_status_code) diff --git a/kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_viewset.py b/kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_viewset.py index 2831e70fb3..cd79a93c39 100644 --- a/kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_viewset.py +++ b/kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_viewset.py @@ -477,7 +477,6 @@ def test_xform_serializer_none(self): 'instances_with_geopoints': False, 'num_of_submissions': 0, 'attachment_storage_bytes': 0, - 'has_kpi_hooks': False, 'kpi_asset_uid': '', } self.assertEqual(data, XFormSerializer(None).data) diff --git a/kobo/apps/openrosa/apps/logger/migrations/0035_remove_xform_has_kpi_hooks_and_instance_posted_to_kpi.py b/kobo/apps/openrosa/apps/logger/migrations/0035_remove_xform_has_kpi_hooks_and_instance_posted_to_kpi.py new file mode 100644 index 0000000000..b6e48241c8 --- /dev/null +++ b/kobo/apps/openrosa/apps/logger/migrations/0035_remove_xform_has_kpi_hooks_and_instance_posted_to_kpi.py @@ -0,0 +1,27 @@ +# Generated by Django 4.2.11 on 2024-07-31 15:59 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import kobo.apps.openrosa.apps.logger.models.attachment +import kobo.apps.openrosa.apps.logger.models.xform +import kpi.deployment_backends.kc_access.storage + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('logger', '0034_set_require_auth_at_project_level'), + ] + + operations = [ + migrations.RemoveField( + model_name='xform', + name='has_kpi_hooks', + ), + migrations.RemoveField( + model_name='instance', + name='posted_to_kpi', + ), + ] diff --git a/kobo/apps/openrosa/apps/logger/models/instance.py b/kobo/apps/openrosa/apps/logger/models/instance.py index 86a52500f0..7b685d261b 100644 --- a/kobo/apps/openrosa/apps/logger/models/instance.py +++ b/kobo/apps/openrosa/apps/logger/models/instance.py @@ -106,10 +106,6 @@ class Instance(models.Model): # TODO Don't forget to update all records with command `update_is_sync_with_mongo`. is_synced_with_mongo = LazyDefaultBooleanField(default=False) - # If XForm.has_kpi_hooks` is True, this field should be True either. - # It tells whether the instance has been successfully sent to KPI. - posted_to_kpi = LazyDefaultBooleanField(default=False) - class Meta: app_label = 'logger' diff --git a/kobo/apps/openrosa/apps/logger/models/xform.py b/kobo/apps/openrosa/apps/logger/models/xform.py index c37aae1666..6e34279294 100644 --- a/kobo/apps/openrosa/apps/logger/models/xform.py +++ b/kobo/apps/openrosa/apps/logger/models/xform.py @@ -96,7 +96,6 @@ class XForm(BaseModel): tags = TaggableManager() - has_kpi_hooks = LazyDefaultBooleanField(default=False) kpi_asset_uid = models.CharField(max_length=32, null=True) pending_delete = models.BooleanField(default=False) @@ -173,14 +172,6 @@ def data_dictionary(self, use_cache: bool = False): def has_instances_with_geopoints(self): return self.instances_with_geopoints - @property - def kpi_hook_service(self): - """ - Returns kpi hook service if it exists. XForm should have only one occurrence in any case. - :return: RestService - """ - return self.restservices.filter(name="kpi_hook").first() - def _set_id_string(self): matches = self.instance_id_regex.findall(self.xml) if len(matches) != 1: diff --git a/kobo/apps/openrosa/apps/main/migrations/0015_drop_old_restservice_tables.py b/kobo/apps/openrosa/apps/main/migrations/0015_drop_old_restservice_tables.py new file mode 100644 index 0000000000..1b58d492d7 --- /dev/null +++ b/kobo/apps/openrosa/apps/main/migrations/0015_drop_old_restservice_tables.py @@ -0,0 +1,77 @@ +# Generated by Django 4.2.11 on 2024-07-31 15:59 + +from django.db import migrations, connections +from django.conf import settings + + +KC_REST_SERVICES_TABLES = [ + 'restservice_restservice', +] + + +def get_operations(): + if settings.TESTING or settings.SKIP_HEAVY_MIGRATIONS: + # Skip this migration if running in test environment or because we want + # to voluntarily skip it. + return [] + + tables = KC_REST_SERVICES_TABLES + operations = [] + + sql = """ + SELECT con.conname + FROM pg_catalog.pg_constraint con + INNER JOIN pg_catalog.pg_class rel + ON rel.oid = con.conrelid + INNER JOIN pg_catalog.pg_namespace nsp + ON nsp.oid = connamespace + WHERE nsp.nspname = 'public' + AND rel.relname = %s; + """ + with connections[settings.OPENROSA_DB_ALIAS].cursor() as cursor: + drop_table_queries = [] + for table in tables: + cursor.execute(sql, [table]) + drop_index_queries = [] + for row in cursor.fetchall(): + if not row[0].endswith('_pkey'): + drop_index_queries.append( + f'ALTER TABLE public.{table} DROP CONSTRAINT {row[0]};' + ) + drop_table_queries.append(f'DROP TABLE IF EXISTS {table};') + operations.append( + migrations.RunSQL( + sql=''.join(drop_index_queries), + reverse_sql=migrations.RunSQL.noop, + ) + ) + + operations.append( + migrations.RunSQL( + sql=''.join(drop_table_queries), + reverse_sql=migrations.RunSQL.noop, + ) + ) + + return operations + + +def print_migration_warning(apps, schema_editor): + if settings.TESTING or settings.SKIP_HEAVY_MIGRATIONS: + return + print( + """ + This migration might take a while. If it is too slow, you may want to + re-run migrations with SKIP_HEAVY_MIGRATIONS=True and apply this one + manually from the django shell. + """ + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0014_drop_old_formdisclaimer_tables'), + ] + + operations = [migrations.RunPython(print_migration_warning), *get_operations()] diff --git a/kobo/apps/openrosa/apps/restservice/RestServiceInterface.py b/kobo/apps/openrosa/apps/restservice/RestServiceInterface.py deleted file mode 100644 index 28495d5a5c..0000000000 --- a/kobo/apps/openrosa/apps/restservice/RestServiceInterface.py +++ /dev/null @@ -1,4 +0,0 @@ -# coding: utf-8 -class RestServiceInterface: - def send(self, url, data=None): - raise NotImplementedError diff --git a/kobo/apps/openrosa/apps/restservice/__init__.py b/kobo/apps/openrosa/apps/restservice/__init__.py deleted file mode 100644 index 7fe25636ee..0000000000 --- a/kobo/apps/openrosa/apps/restservice/__init__.py +++ /dev/null @@ -1,9 +0,0 @@ -# coding: utf-8 -SERVICE_KPI_HOOK = ("kpi_hook", "KPI Hook POST") - -SERVICE_CHOICES = ( - SERVICE_KPI_HOOK, -) - - -default_app_config = "kobo.apps.openrosa.apps.restservice.app.RestServiceConfig" diff --git a/kobo/apps/openrosa/apps/restservice/app.py b/kobo/apps/openrosa/apps/restservice/app.py deleted file mode 100644 index 32c379ee84..0000000000 --- a/kobo/apps/openrosa/apps/restservice/app.py +++ /dev/null @@ -1,12 +0,0 @@ -# coding: utf-8 -from django.apps import AppConfig - - -class RestServiceConfig(AppConfig): - name = 'kobo.apps.openrosa.apps.restservice' - verbose_name = 'restservice' - - def ready(self): - # Register RestService signals - from . import signals - super().ready() diff --git a/kobo/apps/openrosa/apps/restservice/management/__init__.py b/kobo/apps/openrosa/apps/restservice/management/__init__.py deleted file mode 100644 index 57d631c3f0..0000000000 --- a/kobo/apps/openrosa/apps/restservice/management/__init__.py +++ /dev/null @@ -1 +0,0 @@ -# coding: utf-8 diff --git a/kobo/apps/openrosa/apps/restservice/management/commands/__init__.py b/kobo/apps/openrosa/apps/restservice/management/commands/__init__.py deleted file mode 100644 index 57d631c3f0..0000000000 --- a/kobo/apps/openrosa/apps/restservice/management/commands/__init__.py +++ /dev/null @@ -1 +0,0 @@ -# coding: utf-8 diff --git a/kobo/apps/openrosa/apps/restservice/management/commands/update_kpi_hooks_endpoint.py b/kobo/apps/openrosa/apps/restservice/management/commands/update_kpi_hooks_endpoint.py deleted file mode 100644 index 64a3a79053..0000000000 --- a/kobo/apps/openrosa/apps/restservice/management/commands/update_kpi_hooks_endpoint.py +++ /dev/null @@ -1,41 +0,0 @@ -# coding: utf-8 -from django.core.management.base import BaseCommand - -from kobo.apps.openrosa.apps.restservice.models import RestService - - -class Command(BaseCommand): - """ - A faster method is available with PostgreSQL: - UPDATE restservice_restservice - SET service_url = REGEXP_REPLACE( - service_url, - '/assets/([^/]*)/submissions/', - '/api/v2/assets/\1/hook-signal/' - ) - WHERE service_url LIKE '/assets/%'; - """ - - help = 'Updates KPI rest service endpoint' - - def handle(self, *args, **kwargs): - - rest_services = RestService.objects.filter(name='kpi_hook').all() - for rest_service in rest_services: - service_url = rest_service.service_url - do_save = False - if service_url.endswith('/submissions/'): - service_url = service_url.replace('/submissions/', '/hook-signal/') - rest_service.service_url = service_url - do_save = True - rest_service.save(update_fields=["service_url"]) - - if service_url.startswith('/assets/'): - service_url = service_url.replace('/assets/', '/api/v2/assets/') - rest_service.service_url = service_url - do_save = True - - if do_save: - rest_service.save(update_fields=["service_url"]) - - print('Done!') diff --git a/kobo/apps/openrosa/apps/restservice/migrations/0001_initial.py b/kobo/apps/openrosa/apps/restservice/migrations/0001_initial.py deleted file mode 100644 index 0d68804e6d..0000000000 --- a/kobo/apps/openrosa/apps/restservice/migrations/0001_initial.py +++ /dev/null @@ -1,25 +0,0 @@ -# coding: utf-8 -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('logger', '0001_initial'), - ] - - operations = [ - migrations.CreateModel( - name='RestService', - fields=[ - ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), - ('service_url', models.URLField(verbose_name='Service URL')), - ('name', models.CharField(max_length=50, choices=[('f2dhis2', 'f2dhis2'), ('generic_json', 'JSON POST'), ('generic_xml', 'XML POST'), ('bamboo', 'bamboo')])), - ('xform', models.ForeignKey(to='logger.XForm', on_delete=models.CASCADE)), - ], - ), - migrations.AlterUniqueTogether( - name='restservice', - unique_together=set([('service_url', 'xform', 'name')]), - ), - ] diff --git a/kobo/apps/openrosa/apps/restservice/migrations/0002_add_related_name_with_delete_on_cascade.py b/kobo/apps/openrosa/apps/restservice/migrations/0002_add_related_name_with_delete_on_cascade.py deleted file mode 100644 index c2d6cf46c3..0000000000 --- a/kobo/apps/openrosa/apps/restservice/migrations/0002_add_related_name_with_delete_on_cascade.py +++ /dev/null @@ -1,22 +0,0 @@ -# coding: utf-8 -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('restservice', '0001_initial'), - ] - - operations = [ - migrations.AlterField( - model_name='restservice', - name='name', - field=models.CharField(max_length=50, choices=[('f2dhis2', 'f2dhis2'), ('generic_json', 'JSON POST'), ('generic_xml', 'XML POST'), ('bamboo', 'bamboo'), ('kpi_hook', 'KPI Hook POST')]), - ), - migrations.AlterField( - model_name='restservice', - name='xform', - field=models.ForeignKey(related_name='restservices', to='logger.XForm', on_delete=models.CASCADE), - ), - ] diff --git a/kobo/apps/openrosa/apps/restservice/migrations/0003_remove_deprecated_services.py b/kobo/apps/openrosa/apps/restservice/migrations/0003_remove_deprecated_services.py deleted file mode 100644 index 306e80da8f..0000000000 --- a/kobo/apps/openrosa/apps/restservice/migrations/0003_remove_deprecated_services.py +++ /dev/null @@ -1,17 +0,0 @@ -# coding: utf-8 -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('restservice', '0002_add_related_name_with_delete_on_cascade'), - ] - - operations = [ - migrations.AlterField( - model_name='restservice', - name='name', - field=models.CharField(max_length=50, choices=[('kpi_hook', 'KPI Hook POST')]), - ), - ] diff --git a/kobo/apps/openrosa/apps/restservice/migrations/__init__.py b/kobo/apps/openrosa/apps/restservice/migrations/__init__.py deleted file mode 100644 index 57d631c3f0..0000000000 --- a/kobo/apps/openrosa/apps/restservice/migrations/__init__.py +++ /dev/null @@ -1 +0,0 @@ -# coding: utf-8 diff --git a/kobo/apps/openrosa/apps/restservice/models.py b/kobo/apps/openrosa/apps/restservice/models.py deleted file mode 100644 index f93945a267..0000000000 --- a/kobo/apps/openrosa/apps/restservice/models.py +++ /dev/null @@ -1,31 +0,0 @@ -# coding: utf-8 -from django.db import models -from django.utils.translation import gettext_lazy - -from kobo.apps.openrosa.apps.logger.models.xform import XForm -from kobo.apps.openrosa.apps.restservice import SERVICE_CHOICES - - -class RestService(models.Model): - - class Meta: - app_label = 'restservice' - unique_together = ('service_url', 'xform', 'name') - - service_url = models.URLField(gettext_lazy("Service URL")) - xform = models.ForeignKey(XForm, related_name="restservices", on_delete=models.CASCADE) - name = models.CharField(max_length=50, choices=SERVICE_CHOICES) - - def __str__(self): - return "%s:%s - %s" % (self.xform, self.long_name, self.service_url) - - def get_service_definition(self): - m = __import__(''.join(['kobo.apps.openrosa.apps.restservice.services.', - self.name]), - globals(), locals(), ['ServiceDefinition']) - return m.ServiceDefinition - - @property - def long_name(self): - sv = self.get_service_definition() - return sv.verbose_name diff --git a/kobo/apps/openrosa/apps/restservice/services/__init__.py b/kobo/apps/openrosa/apps/restservice/services/__init__.py deleted file mode 100644 index f6b69c77ea..0000000000 --- a/kobo/apps/openrosa/apps/restservice/services/__init__.py +++ /dev/null @@ -1,2 +0,0 @@ -# coding: utf-8 -__all__ = ('kpi_hook') diff --git a/kobo/apps/openrosa/apps/restservice/services/kpi_hook.py b/kobo/apps/openrosa/apps/restservice/services/kpi_hook.py deleted file mode 100644 index 4d0f7127fe..0000000000 --- a/kobo/apps/openrosa/apps/restservice/services/kpi_hook.py +++ /dev/null @@ -1,43 +0,0 @@ -# coding: utf-8 -import logging -import re - -import requests -from django.conf import settings -from kobo.apps.openrosa.apps.restservice.RestServiceInterface import RestServiceInterface -from kobo.apps.openrosa.apps.logger.models import Instance - - -class ServiceDefinition(RestServiceInterface): - id = 'kpi_hook' - verbose_name = 'KPI Hook POST' - - def send(self, endpoint, data): - - # Will be used internally by KPI to fetch data with KoBoCatBackend - post_data = { - 'submission_id': data.get('instance_id') - } - headers = {'Content-Type': 'application/json'} - - # Verify if endpoint starts with `/assets/` before sending - # the request to KPI - pattern = r'{}'.format(settings.KPI_HOOK_ENDPOINT_PATTERN.replace( - '{asset_uid}', '[^/]*')) - - # Match v2 and v1 endpoints. - if re.match(pattern, endpoint) or re.match(pattern[7:], endpoint): - # Build the url in the service to avoid saving hardcoded - # domain name in the DB - url = f'{settings.KOBOFORM_INTERNAL_URL}{endpoint}' - response = requests.post(url, headers=headers, json=post_data) - response.raise_for_status() - - # Save successful - Instance.objects.filter(pk=data.get('instance_id')).update( - posted_to_kpi=True - ) - else: - logging.warning( - f'This endpoint: `{endpoint}` is not valid for `KPI Hook`' - ) diff --git a/kobo/apps/openrosa/apps/restservice/signals.py b/kobo/apps/openrosa/apps/restservice/signals.py deleted file mode 100644 index 80ae3b874c..0000000000 --- a/kobo/apps/openrosa/apps/restservice/signals.py +++ /dev/null @@ -1,36 +0,0 @@ -# coding: utf-8 -from django.conf import settings -from django.db.models.signals import post_save -from django.dispatch import receiver - -from kobo.apps.openrosa.apps.restservice import SERVICE_KPI_HOOK -from kobo.apps.openrosa.apps.logger.models import XForm -from kobo.apps.openrosa.apps.restservice.models import RestService - - -@receiver(post_save, sender=XForm) -def save_kpi_hook_service(sender, instance, **kwargs): - """ - Creates/Deletes Kpi hook Rest service related to XForm instance - :param sender: XForm class - :param instance: XForm instance - :param kwargs: dict - """ - kpi_hook_service = instance.kpi_hook_service - if instance.has_kpi_hooks: - # Only register the service if it hasn't been created yet. - if kpi_hook_service is None: - # For retro-compatibility, if `asset_uid` is null, fallback on - # `id_string` - asset_uid = instance.kpi_asset_uid if instance.kpi_asset_uid \ - else instance.id_string - kpi_hook_service = RestService( - service_url=settings.KPI_HOOK_ENDPOINT_PATTERN.format( - asset_uid=asset_uid), - xform=instance, - name=SERVICE_KPI_HOOK[0] - ) - kpi_hook_service.save() - elif kpi_hook_service is not None: - # Only delete the service if it already exists. - kpi_hook_service.delete() diff --git a/kobo/apps/openrosa/apps/restservice/tasks.py b/kobo/apps/openrosa/apps/restservice/tasks.py deleted file mode 100644 index ce67dfd5e2..0000000000 --- a/kobo/apps/openrosa/apps/restservice/tasks.py +++ /dev/null @@ -1,35 +0,0 @@ -# coding: utf-8 -import logging - -from celery import shared_task -from django.conf import settings - -from kobo.apps.openrosa.apps.restservice.models import RestService - - -@shared_task(bind=True) -def service_definition_task(self, rest_service_id, data): - """ - Tries to send data to the endpoint of the hook - It retries 3 times maximum. - - after 2 minutes, - - after 20 minutes, - - after 200 minutes - - :param self: Celery.Task. - :param rest_service_id: RestService primary key. - :param data: dict. - """ - try: - rest_service = RestService.objects.get(pk=rest_service_id) - service = rest_service.get_service_definition()() - service.send(rest_service.service_url, data) - except Exception as e: - logger = logging.getLogger("console_logger") - logger.error("service_definition_task - {}".format(str(e)), exc_info=True) - # Countdown is in seconds - countdown = 120 * (10 ** self.request.retries) - # Max retries is 3 by default. - raise self.retry(countdown=countdown, max_retries=settings.REST_SERVICE_MAX_RETRIES) - - return True diff --git a/kobo/apps/openrosa/apps/restservice/templates/add-service.html b/kobo/apps/openrosa/apps/restservice/templates/add-service.html deleted file mode 100644 index 717082f50e..0000000000 --- a/kobo/apps/openrosa/apps/restservice/templates/add-service.html +++ /dev/null @@ -1,11 +0,0 @@ -{% load i18n %} -{% block content %} -
-

- Please manage REST Services within the new user interface. Go to the - Project Dashboard and navigate to - - Your Project > Settings > REST Services. - -

-{% endblock %} diff --git a/kobo/apps/openrosa/apps/restservice/tests/__init__.py b/kobo/apps/openrosa/apps/restservice/tests/__init__.py deleted file mode 100644 index 57d631c3f0..0000000000 --- a/kobo/apps/openrosa/apps/restservice/tests/__init__.py +++ /dev/null @@ -1 +0,0 @@ -# coding: utf-8 diff --git a/kobo/apps/openrosa/apps/restservice/tests/fixtures/dhisform.xls b/kobo/apps/openrosa/apps/restservice/tests/fixtures/dhisform.xls deleted file mode 100755 index d0b29d74b7..0000000000 Binary files a/kobo/apps/openrosa/apps/restservice/tests/fixtures/dhisform.xls and /dev/null differ diff --git a/kobo/apps/openrosa/apps/restservice/tests/test_restservice.py b/kobo/apps/openrosa/apps/restservice/tests/test_restservice.py deleted file mode 100644 index f27414601e..0000000000 --- a/kobo/apps/openrosa/apps/restservice/tests/test_restservice.py +++ /dev/null @@ -1,40 +0,0 @@ -# coding: utf-8 -import os - -from django.conf import settings - -from kobo.apps.openrosa.apps.main.tests.test_base import TestBase -from kobo.apps.openrosa.apps.logger.models.xform import XForm -from kobo.apps.openrosa.apps.restservice.RestServiceInterface import RestServiceInterface -from kobo.apps.openrosa.apps.restservice.models import RestService - - -class RestServiceTest(TestBase): - - def setUp(self): - super().setUp() - xls_file_path = os.path.join( - os.path.dirname(os.path.abspath(__file__)), - 'fixtures', - 'dhisform.xls' - ) - self._publish_xls_file_and_set_xform(xls_file_path) - - def _create_rest_service(self): - rs = RestService( - service_url=settings.KPI_HOOK_ENDPOINT_PATTERN.format( - asset_uid='aAAAAAAAAAAA'), - xform=XForm.objects.all().reverse()[0], - name='kpi_hook') - rs.save() - self.restservice = rs - - def test_create_rest_service(self): - count = RestService.objects.all().count() - self._create_rest_service() - self.assertEqual(RestService.objects.all().count(), count + 1) - - def test_service_definition(self): - self._create_rest_service() - sv = self.restservice.get_service_definition()() - self.assertEqual(isinstance(sv, RestServiceInterface), True) diff --git a/kobo/apps/openrosa/apps/restservice/utils.py b/kobo/apps/openrosa/apps/restservice/utils.py deleted file mode 100644 index 5e3cbbb5a0..0000000000 --- a/kobo/apps/openrosa/apps/restservice/utils.py +++ /dev/null @@ -1,24 +0,0 @@ -# coding: utf-8 -from kobo.apps.openrosa.apps.restservice.models import RestService -from kobo.apps.openrosa.apps.restservice.tasks import service_definition_task - - -def call_service(parsed_instance): - # lookup service - instance = parsed_instance.instance - rest_services = RestService.objects.filter(xform=instance.xform) - # call service send with url and data parameters - for rest_service in rest_services: - # Celery can't pickle ParsedInstance object, - # let's use build a serializable object instead - # We don't really need `xform_id`, `xform_id_string`, `instance_uuid` - # We use them only for retro compatibility with all services (even if they are deprecated) - data = { - "xform_id": instance.xform.id, - "xform_id_string": instance.xform.id_string, - "instance_uuid": instance.uuid, - "instance_id": instance.id, - "xml": parsed_instance.instance.xml, - "json": parsed_instance.to_dict_for_mongo() - } - service_definition_task.delay(rest_service.pk, data) diff --git a/kobo/apps/openrosa/apps/viewer/models/parsed_instance.py b/kobo/apps/openrosa/apps/viewer/models/parsed_instance.py index 469fae597a..4afd88a3f3 100644 --- a/kobo/apps/openrosa/apps/viewer/models/parsed_instance.py +++ b/kobo/apps/openrosa/apps/viewer/models/parsed_instance.py @@ -12,7 +12,6 @@ from kobo.apps.openrosa.apps.api.mongo_helper import MongoHelper from kobo.apps.openrosa.apps.logger.models import Instance from kobo.apps.openrosa.apps.logger.models import Note -from kobo.apps.openrosa.apps.restservice.utils import call_service from kobo.apps.openrosa.libs.utils.common_tags import ( ID, UUID, @@ -27,6 +26,8 @@ ) from kobo.apps.openrosa.libs.utils.decorators import apply_form_field_names from kobo.apps.openrosa.libs.utils.model_tools import queryset_iterator +from kobo.apps.hook.utils.services import call_services +from kpi.utils.log import logging # this is Mongo Collection where we will store the parsed submissions xform_instances = settings.MONGO_DB.instances @@ -371,7 +372,16 @@ def save(self, asynchronous=False, *args, **kwargs): # Rest Services were called before data was saved in DB. success = self.update_mongo(asynchronous) if success and created: - call_service(self) + records = ParsedInstance.objects.filter( + instance_id=self.instance_id + ).values_list('instance__xform__kpi_asset_uid', flat=True) + if not (asset_uid := records[0]): + logging.warning( + f'ParsedInstance #: {self.pk} - XForm is not linked with Asset' + ) + else: + call_services(asset_uid, self.instance_id) + return success def add_note(self, note): diff --git a/kobo/apps/openrosa/libs/constants.py b/kobo/apps/openrosa/libs/constants.py index 761d96edd5..9e1b580763 100644 --- a/kobo/apps/openrosa/libs/constants.py +++ b/kobo/apps/openrosa/libs/constants.py @@ -39,6 +39,5 @@ 'logger', 'viewer', 'main', - 'restservice', 'guardian', ] diff --git a/kobo/apps/openrosa/libs/serializers/xform_serializer.py b/kobo/apps/openrosa/libs/serializers/xform_serializer.py index cc910c1e41..f9ad5c0685 100644 --- a/kobo/apps/openrosa/libs/serializers/xform_serializer.py +++ b/kobo/apps/openrosa/libs/serializers/xform_serializer.py @@ -27,7 +27,6 @@ class XFormSerializer(serializers.HyperlinkedModelSerializer): lookup_field='pk') users = serializers.SerializerMethodField('get_xform_permissions') hash = serializers.SerializerMethodField() - has_kpi_hooks = serializers.BooleanField() class Meta: model = XForm diff --git a/kobo/settings/base.py b/kobo/settings/base.py index 19b1644a35..715d2badc3 100644 --- a/kobo/settings/base.py +++ b/kobo/settings/base.py @@ -138,7 +138,6 @@ 'kobo.apps.openrosa.apps.logger.app.LoggerAppConfig', 'kobo.apps.openrosa.apps.viewer.app.ViewerConfig', 'kobo.apps.openrosa.apps.main.app.MainConfig', - 'kobo.apps.openrosa.apps.restservice.app.RestServiceConfig', 'kobo.apps.openrosa.apps.api', 'guardian', 'kobo.apps.openrosa.libs', @@ -1199,7 +1198,7 @@ def dj_stripe_request_callback_method(): # http://docs.celeryproject.org/en/latest/getting-started/brokers/redis.html#redis-visibility-timeout # TODO figure out how to pass `Constance.HOOK_MAX_RETRIES` or `HookLog.get_remaining_seconds() # Otherwise hardcode `HOOK_MAX_RETRIES` in Settings - "visibility_timeout": 60 * (10 ** 3) # Longest ETA for RestService (seconds) + "visibility_timeout": 60 * (10 ** 2) # Longest ETA for RestService (seconds) } CELERY_TASK_DEFAULT_QUEUE = "kpi_queue" diff --git a/kpi/deployment_backends/base_backend.py b/kpi/deployment_backends/base_backend.py index d2e6d88ac2..f7860fdf71 100644 --- a/kpi/deployment_backends/base_backend.py +++ b/kpi/deployment_backends/base_backend.py @@ -446,10 +446,6 @@ def set_enketo_open_rosa_server( ): pass - @abc.abstractmethod - def set_has_kpi_hooks(self): - pass - def set_status(self, status): self.save_to_db({'status': status}) diff --git a/kpi/deployment_backends/mock_backend.py b/kpi/deployment_backends/mock_backend.py index 1efbd5e650..296e8fbb25 100644 --- a/kpi/deployment_backends/mock_backend.py +++ b/kpi/deployment_backends/mock_backend.py @@ -91,7 +91,6 @@ def generate_uuid_for_form(): 'active': active, 'backend_response': { 'downloadable': active, - 'has_kpi_hook': self.asset.has_active_hooks, 'kpi_asset_uid': self.asset.uid, 'uuid': generate_uuid_for_form(), # TODO use XForm object and get its primary key @@ -525,19 +524,6 @@ def set_enketo_open_rosa_server( ): pass - def set_has_kpi_hooks(self): - """ - Store a boolean which indicates that KPI has active hooks (or not) - and, if it is the case, it should receive notifications when new data - comes in - """ - has_active_hooks = self.asset.has_active_hooks - self.store_data( - { - 'has_kpi_hooks': has_active_hooks, - } - ) - def set_namespace(self, namespace): self.store_data( { diff --git a/kpi/deployment_backends/openrosa_backend.py b/kpi/deployment_backends/openrosa_backend.py index a17b72114b..1d89afdb63 100644 --- a/kpi/deployment_backends/openrosa_backend.py +++ b/kpi/deployment_backends/openrosa_backend.py @@ -150,10 +150,9 @@ def connect(self, active=False): with kc_transaction_atomic(): self._xform = publish_xls_form(xlsx_file, self.asset.owner) self._xform.downloadable = active - self._xform.has_kpi_hooks = self.asset.has_active_hooks self._xform.kpi_asset_uid = self.asset.uid self._xform.save( - update_fields=['downloadable', 'has_kpi_hooks', 'kpi_asset_uid'] + update_fields=['downloadable', 'kpi_asset_uid'] ) self.store_data( @@ -842,11 +841,9 @@ def redeploy(self, active=None): XForm.objects.filter(pk=self.xform.id).update( downloadable=active, title=self.asset.name, - has_kpi_hooks=self.asset.has_active_hooks, ) self.xform.downloadable = active self.xform.title = self.asset.name - self.xform.has_kpi_hooks = self.asset.has_active_hooks publish_xls_form(xlsx_file, self.asset.owner, self.xform.id_string) @@ -1024,27 +1021,6 @@ def set_enketo_open_rosa_server( server_url, ) - def set_has_kpi_hooks(self): - """ - `PATCH` `has_kpi_hooks` boolean of related KoBoCAT XForm. - It lets KoBoCAT know whether it needs to notify KPI - each time a submission comes in. - - Store results in deployment data - """ - # Use `queryset.update()` over `model.save()` because we don't need to - # run the logic of the `model.save()` method and we don't need signals - # to be called. - XForm.objects.filter(pk=self.xform_id).update( - kpi_asset_uid=self.asset.uid, - has_kpi_hooks=self.asset.has_active_hooks - ) - self.xform.kpi_asset_uid = self.asset.uid - self.xform.has_active_hooks = self.asset.has_active_hooks - - self.backend_response['kpi_asset_uid'] = self.asset.uid - self.store_data({'backend_response': self.backend_response}) - def set_validation_status( self, submission_id: int, diff --git a/kpi/signals.py b/kpi/signals.py index 90245bbee6..89be23937d 100644 --- a/kpi/signals.py +++ b/kpi/signals.py @@ -74,16 +74,6 @@ def tag_uid_post_save(sender, instance, created, raw, **kwargs): TagUid.objects.get_or_create(tag=instance) -@receiver(post_save, sender=Hook) -def update_kc_xform_has_kpi_hooks(sender, instance, **kwargs): - """ - Updates KoboCAT XForm instance as soon as Asset.Hook list is updated. - """ - asset = instance.asset - if asset.has_deployment: - asset.deployment.set_has_kpi_hooks() - - @receiver(post_delete, sender=Asset) def post_delete_asset(sender, instance, **kwargs): # Update parent's languages if this object is a child of another asset. diff --git a/kpi/tests/api/v2/test_api_invalid_password_access.py b/kpi/tests/api/v2/test_api_invalid_password_access.py index 9468156e6e..fc2f2918ef 100644 --- a/kpi/tests/api/v2/test_api_invalid_password_access.py +++ b/kpi/tests/api/v2/test_api_invalid_password_access.py @@ -185,20 +185,3 @@ def _access_endpoints(self, access_granted: bool, headers: dict = {}): # `/environment` response = self.client.get(reverse('environment'), **headers) assert response.status_code == status.HTTP_200_OK - - # Hook signal is a particular case but should not return a 403 - data = {'submission_id': submission_id} - # # `/api/v2/assets//hook-signal/` - response = self.client.post( - reverse( - self._get_endpoint('hook-signal-list'), - kwargs={ - 'format': 'json', - 'parent_lookup_asset': self.asset.uid, - }, - ), - data=data, - **headers, - ) - # return a 202 first time but 409 other attempts. - assert response.status_code != status.HTTP_403_FORBIDDEN diff --git a/kpi/urls/router_api_v1.py b/kpi/urls/router_api_v1.py index 4e53c7ae8c..368101a9e3 100644 --- a/kpi/urls/router_api_v1.py +++ b/kpi/urls/router_api_v1.py @@ -3,7 +3,6 @@ from kobo.apps.hook.views.v1.hook import HookViewSet from kobo.apps.hook.views.v1.hook_log import HookLogViewSet -from kobo.apps.hook.views.v1.hook_signal import HookSignalViewSet from kobo.apps.reports.views import ReportsViewSet from kpi.views.v1 import ( @@ -29,11 +28,6 @@ basename='asset-version', parents_query_lookups=['asset'], ) -asset_routes.register(r'hook-signal', - HookSignalViewSet, - basename='hook-signal', - parents_query_lookups=['asset'], - ) asset_routes.register(r'submissions', SubmissionViewSet, basename='submission', diff --git a/kpi/urls/router_api_v2.py b/kpi/urls/router_api_v2.py index 9883884fc4..7c7dbd2575 100644 --- a/kpi/urls/router_api_v2.py +++ b/kpi/urls/router_api_v2.py @@ -5,7 +5,6 @@ from kobo.apps.audit_log.urls import router as audit_log_router from kobo.apps.hook.views.v2.hook import HookViewSet from kobo.apps.hook.views.v2.hook_log import HookLogViewSet -from kobo.apps.hook.views.v2.hook_signal import HookSignalViewSet from kobo.apps.languages.urls import router as language_router from kobo.apps.organizations.views import OrganizationViewSet from kobo.apps.project_ownership.urls import router as project_ownership_router @@ -104,12 +103,6 @@ def get_urls(self, *args, **kwargs): parents_query_lookups=['asset'], ) -asset_routes.register(r'hook-signal', - HookSignalViewSet, - basename='hook-signal', - parents_query_lookups=['asset'], - ) - asset_routes.register(r'paired-data', PairedDataViewset, basename='paired-data',