[IMP] maintenance_service_http_monitoring : modify logic to avoid false-positive http alerts

maintenance_service_http_monitoring/tests/test_http_monitoring.py

@@ -11,9 +11,9 @@ EQUIPMENT_HTTP_REQUESTS = (
     "odoo.addons.maintenance_service_http_monitoring"
     ".models.maintenance_equipment.http_requests"
 )
-SERVICE_INSTANCE_SLEEP = (
-    "odoo.addons.maintenance_service_http_monitoring.models.service_instance.time.sleep"
-)
+
+# Comfortably past HTTP_KO_CONFIRMATION_DELAY (currently 10 minutes).
+PAST_CONFIRMATION_DELAY = timedelta(minutes=20)
 
 
 def _mock_response(status_code):
@@ -41,6 +41,13 @@ class TestHttpMonitoring(TransactionCase):
             }
         )
 
+    def _backdate_first_ko(self, *service_instances):
+        """Simulate that the current KO streak started long enough ago to be confirmed."""
+        for service_instance in service_instances:
+            service_instance.write(
+                {"http_first_ko_at": fields.Datetime.now() - PAST_CONFIRMATION_DELAY}
+            )
+
     # ------------------------------------------------------------------
     # Test 1 -- HTTP 200 -> service marked OK
     # ------------------------------------------------------------------
@@ -55,19 +62,26 @@ class TestHttpMonitoring(TransactionCase):
         self.assertIsNotNone(self.service_instance.last_http_check_date)
 
     # ------------------------------------------------------------------
-    # Test 2 -- Two KO passes -> maintenance.request created on the service
+    # Test 2 -- KO confirmed only once continuously KO for HTTP_KO_CONFIRMATION_DELAY,
+    # not on the first observed failure
     # ------------------------------------------------------------------
-    def test_http_500_creates_maintenance_request(self):
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
-        ):
+    def test_http_500_creates_request_after_confirmation_delay(self):
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
             mock_requests.get.return_value = _mock_response(500)
             mock_requests.exceptions.RequestException = Exception
             self.env["service.instance"].cron_check_http_services()
 
         self.assertFalse(self.service_instance.http_status_ok)
         self.assertEqual(self.service_instance.last_http_status_code, 500)
+        self.assertTrue(self.service_instance.http_first_ko_at)
+        self.assertFalse(self.service_instance.http_maintenance_request)
+
+        self._backdate_first_ko(self.service_instance)
+
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(500)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()
 
         request = self.service_instance.http_maintenance_request
         self.assertTrue(request)
@@ -90,27 +104,29 @@ class TestHttpMonitoring(TransactionCase):
         self.assertEqual(self.service_instance.last_http_status_code, -1)
 
     # ------------------------------------------------------------------
-    # Test 4 -- Two consecutive cron runs KO -> no duplicate request
+    # Test 4 -- Repeated failure after confirmation -> a single request, no duplicate
     # ------------------------------------------------------------------
     def test_no_duplicate_request_on_repeated_failure(self):
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
-        ):
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
             mock_requests.get.return_value = _mock_response(500)
             mock_requests.exceptions.RequestException = Exception
-            self.env["service.instance"].cron_check_http_services()
+            self.env["service.instance"].cron_check_http_services()  # first_ko_at set
+
+        self.assertFalse(self.service_instance.http_maintenance_request)
+        self._backdate_first_ko(self.service_instance)
+
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(500)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()  # confirmed, request created
 
         request_1 = self.service_instance.http_maintenance_request
         self.assertTrue(request_1)
 
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
-        ):
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
             mock_requests.get.return_value = _mock_response(500)
             mock_requests.exceptions.RequestException = Exception
-            self.env["service.instance"].cron_check_http_services()
+            self.env["service.instance"].cron_check_http_services()  # still KO
 
         self.assertEqual(self.service_instance.http_maintenance_request, request_1)
         self.assertEqual(
@@ -181,7 +197,8 @@ class TestHttpMonitoring(TransactionCase):
         self.assertEqual(self.service_instance.last_http_status_code, 404)
 
     # ------------------------------------------------------------------
-    # Test 9 -- Webhook called when a new maintenance.request is created
+    # Test 9 -- Webhook called only once the request is actually created
+    # (after confirmation delay), not on the first observed failure
     # ------------------------------------------------------------------
     def test_webhook_called_on_new_request(self):
         self.env["ir.config_parameter"].sudo().set_param(
@@ -190,7 +207,17 @@ class TestHttpMonitoring(TransactionCase):
         )
         with (
             patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
+            patch(EQUIPMENT_HTTP_REQUESTS) as mock_http,
+        ):
+            mock_requests.get.return_value = _mock_response(500)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()
+
+        mock_http.post.assert_not_called()
+        self._backdate_first_ko(self.service_instance)
+
+        with (
+            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
             patch(EQUIPMENT_HTTP_REQUESTS) as mock_http,
         ):
             mock_requests.get.return_value = _mock_response(500)
@@ -209,15 +236,22 @@ class TestHttpMonitoring(TransactionCase):
         self.env["ir.config_parameter"].sudo().set_param(
             "maintenance_service_http_monitoring.webhook_url", ""
         )
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(500)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()
+
+        self._backdate_first_ko(self.service_instance)
+
         with (
             patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
             patch(EQUIPMENT_HTTP_REQUESTS) as mock_http,
         ):
             mock_requests.get.return_value = _mock_response(500)
             mock_requests.exceptions.RequestException = Exception
             self.env["service.instance"].cron_check_http_services()
 
+        self.assertTrue(self.service_instance.http_maintenance_request)
         mock_http.post.assert_not_called()
 
     # ------------------------------------------------------------------
@@ -235,22 +269,25 @@ class TestHttpMonitoring(TransactionCase):
         self.assertFalse(self.service_instance.last_http_check_date)
 
     # ------------------------------------------------------------------
-    # Test 12 -- Transient failure (KO pass 1, OK pass 2) -> no request created
+    # Test 12 -- Transient failure (KO, then back OK before confirmation) ->
+    # no request created, and the KO streak is reset
     # ------------------------------------------------------------------
     def test_transient_failure_no_request_created(self):
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
-        ):
-            mock_requests.get.side_effect = [
-                _mock_response(500),  # pass 1: KO
-                _mock_response(200),  # pass 2 (retry): OK
-            ]
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(500)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()
+
+        self.assertTrue(self.service_instance.http_first_ko_at)
+
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(200)
             mock_requests.exceptions.RequestException = Exception
             self.env["service.instance"].cron_check_http_services()
 
         self.assertTrue(self.service_instance.http_status_ok)
         self.assertEqual(self.service_instance.last_http_status_code, 200)
+        self.assertFalse(self.service_instance.http_first_ko_at)
         self.assertFalse(self.service_instance.http_maintenance_request)
         self.assertEqual(
             self.env["maintenance.request"].search_count(
@@ -260,25 +297,33 @@ class TestHttpMonitoring(TransactionCase):
         )
 
     # ------------------------------------------------------------------
-    # Test 13 -- Confirmed failure (KO pass 1 and 2) -> request created
+    # Test 13 -- Confirmed failure (KO continuously past the confirmation delay)
+    # -> request created
     # ------------------------------------------------------------------
     def test_confirmed_failure_creates_request(self):
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP) as mock_sleep,
-        ):
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
             mock_requests.get.return_value = _mock_response(503)
             mock_requests.exceptions.RequestException = Exception
             self.env["service.instance"].cron_check_http_services()
 
-        mock_sleep.assert_called_once_with(2)
-        self.assertEqual(mock_requests.get.call_count, 2)
+        self.assertEqual(mock_requests.get.call_count, 1)
+        self.assertFalse(self.service_instance.http_maintenance_request)
+
+        self._backdate_first_ko(self.service_instance)
+
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(503)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()
+
+        self.assertEqual(mock_requests.get.call_count, 1)
         self.assertFalse(self.service_instance.http_status_ok)
         self.assertEqual(self.service_instance.last_http_status_code, 503)
         self.assertTrue(self.service_instance.http_maintenance_request)
 
     # ------------------------------------------------------------------
     # Test 14 -- 2 KO services on same equipment -> 2 distinct requests
+    # once both reach the confirmation delay
     # ------------------------------------------------------------------
     def test_two_ko_services_same_equipment_create_two_requests(self):
         service2 = self.env["service"].create({"name": "Test Service 2"})
@@ -290,10 +335,14 @@ class TestHttpMonitoring(TransactionCase):
             }
         )
 
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
-        ):
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(500)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()
+
+        self._backdate_first_ko(self.service_instance, service_instance2)
+
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
             mock_requests.get.return_value = _mock_response(500)
             mock_requests.exceptions.RequestException = Exception
             self.env["service.instance"].cron_check_http_services()
@@ -317,11 +366,16 @@ class TestHttpMonitoring(TransactionCase):
     # Test 15 -- Service recovery closes the open request and posts a note
     # ------------------------------------------------------------------
     def test_service_recovery_closes_request(self):
-        # First cron run: service is KO -> request created
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
-        ):
+        # First cron run: service is KO, streak just started -> no request yet
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(500)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()
+
+        self._backdate_first_ko(self.service_instance)
+
+        # Second cron run: streak confirmed -> request created
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
             mock_requests.get.return_value = _mock_response(500)
             mock_requests.exceptions.RequestException = Exception
             self.env["service.instance"].cron_check_http_services()
@@ -330,11 +384,8 @@ class TestHttpMonitoring(TransactionCase):
         self.assertTrue(request)
         self.assertFalse(request.stage_id.done)
 
-        # Second cron run: service is back OK -> request auto-closed
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
-        ):
+        # Next cron run: service is back OK -> request auto-closed
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
             mock_requests.get.return_value = _mock_response(200)
             mock_requests.exceptions.RequestException = Exception
             self.env["service.instance"].cron_check_http_services()
@@ -343,6 +394,8 @@ class TestHttpMonitoring(TransactionCase):
         self.assertTrue(request.stage_id.done)
         # http_maintenance_request must be cleared on the service instance
         self.assertFalse(self.service_instance.http_maintenance_request)
+        # KO streak must be reset
+        self.assertFalse(self.service_instance.http_first_ko_at)
         # A chatter note must have been posted mentioning the service URL
         notes = request.message_ids.filtered(
             lambda m: self.service_instance.service_url in (m.body or "")