Fix tests

kobo/apps/hook/models/hook_log.py

@@ -45,15 +45,15 @@ def can_retry(self) -> bool:
         Notice: even if False is returned, `self.retry()` can be triggered.
         """
         if self.hook.active:
-            if self.tries >= constance.config.HOOK_MAX_RETRIES:
-                # 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
-                )
+            # 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
 

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,6 +12,7 @@
 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
 
 
@@ -94,26 +96,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 +147,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):

kobo/apps/hook/tests/test_api_hook.py

@@ -1,6 +1,7 @@
 # coding: utf-8
 import json
 
+import pytest
 import responses
 from constance.test import override_config
 from django.urls import reverse
@@ -22,6 +23,7 @@
 )
 from kpi.utils.datetime import several_minutes_from_now
 from .hook_test_case import HookTestCase, MockSSRFProtect
+from ..exceptions import HookRemoteServerDownError
 
 
 class ApiHookTestCase(HookTestCase):
@@ -169,36 +171,70 @@ 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=MockSSRFProtect._get_ip_address
+    )
     @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
         response = self.client.patch(retry_url, format=SUBMISSION_FORMAT_TYPE_JSON)
         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)
+        self.assertEqual(response.data.get('tries'), 2)
 
-    @patch('ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address',
-           new=MockSSRFProtect._get_ip_address)
+    @patch(
+        'ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address',
+        new=MockSSRFProtect._get_ip_address
+    )
+    @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'), 1)
+
+    @patch(
+        'ssrf_protect.ssrf_protect.SSRFProtect._get_ip_address',
+        new=MockSSRFProtect._get_ip_address
+    )
     @responses.activate
     def test_payload_template(self):
 
@@ -321,21 +357,26 @@ def test_hook_log_filter_success(self):
     @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()
         submissions = self.asset.deployment.get_submissions(self.asset.owner)
         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={
@@ -344,18 +385,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={

kobo/apps/hook/tests/test_ssrf.py

@@ -1,12 +1,13 @@
-# coding: utf-8
 
+import pytest
 import responses
 from constance.test import override_config
 from mock import patch
 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
@@ -34,9 +35,10 @@ 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)
+        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)

kobo/apps/hook/views/v2/hook_log.py

@@ -114,15 +114,18 @@ def retry(self, request, uid=None, *args, **kwargs):
             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)