From a75387cb215a3b3749a5a3745300df0c1b2749c1 Mon Sep 17 00:00:00 2001 From: Hanne Moa Date: Fri, 15 Nov 2024 12:02:43 +0100 Subject: [PATCH] Fix wrong end_time /.. also, attempt to clean up/document event types, and event type validation in the API. --- src/argus/incident/models.py | 79 +++++++++++++++++++++++++++++++-- src/argus/incident/views.py | 53 ++++++++++++---------- tests/incident/test_incident.py | 32 +++++++++++++ 3 files changed, 138 insertions(+), 26 deletions(-) diff --git a/src/argus/incident/models.py b/src/argus/incident/models.py index c1dabd66d..f7225c652 100644 --- a/src/argus/incident/models.py +++ b/src/argus/incident/models.py @@ -4,6 +4,7 @@ import logging from operator import and_ from random import randint, choice +from typing import Optional from urllib.parse import urljoin from django.contrib.auth import get_user_model @@ -215,7 +216,28 @@ class Type(models.TextChoices): Type.INCIDENT_CHANGE, Type.STATELESS, } - ALLOWED_TYPES_FOR_END_USERS = {Type.CLOSE, Type.REOPEN, Type.ACKNOWLEDGE, Type.OTHER} + ALLOWED_TYPES_FOR_END_USERS = { + Type.ACKNOWLEDGE, + Type.OTHER, + Type.CLOSE, + Type.REOPEN, + } + CLOSING_TYPES = { + Type.INCIDENT_END, + Type.CLOSE, + } + OPENING_TYPES = { + Type.INCIDENT_START, + Type.REOPEN, + } + STATE_TYPES = OPENING_TYPES | CLOSING_TYPES + SHARED_TYPES = { + Type.ACKNOWLEDGE, + Type.OTHER, + Type.INCIDENT_CHANGE, + } + STATELESS_TYPES = SHARED_TYPES | {Type.STATELESS} + STATEFUL_TYPES = SHARED_TYPES | STATE_TYPES incident = models.ForeignKey(to="Incident", on_delete=models.PROTECT, related_name="events") actor = models.ForeignKey(to=User, on_delete=models.PROTECT, related_name="caused_events") @@ -334,8 +356,9 @@ def create_events(self, actor: User, event_type: Event.Type, timestamp=None, des def close(self, actor: User, timestamp=None, description=""): "Close incidents correctly and create the needed events" + timestamp = timestamp or timezone.now() qs = self.open() - qs.update(end_time=timestamp or timezone.now()) + qs.update(end_time=timestamp) qs = self.all() # Reload changes from database event_type = Event.Type.CLOSE events = qs.create_events(actor, event_type, timestamp, description) @@ -439,17 +462,36 @@ def tags(self): def incident_relations(self): return IncidentRelation.objects.filter(Q(incident1=self) | Q(incident2=self)) + def all_opening_events(self): + open_events = (Event.Type.INCIDENT_START, Event.Type.REOPEN) + return self.events.filter(type__in=open_events).order_by("timestamp") + + def all_reopen_events(self): + return self.events.filter(typen=Event.Type.REOPEN).order_by("timestamp") + + def all_closing_events(self): + close_events = (Event.Type.CLOSE, Event.Type.INCIDENT_END) + return self.events.filter(type__in=close_events).order_by("timestamp") + @property def start_event(self): return self.events.filter(type=Event.Type.INCIDENT_START).order_by("timestamp").first() + @property + def reopen_event(self): + return self.all_reopen_events.last() + @property def end_event(self): return self.events.filter(type=Event.Type.INCIDENT_END).order_by("timestamp").first() + @property + def close_event(self): + return self.events.filter(type=Event.Type.CLOSE).order_by("timestamp").first() + @property def last_close_or_end_event(self): - return self.events.filter(type__in=(Event.Type.CLOSE, Event.Type.INCIDENT_END)).order_by("timestamp").last() + return self.all_close_events().last() @property def latest_change_event(self): @@ -475,6 +517,37 @@ def acked(self): return self.events.filter((acks_query & acks_not_expired_query) | ack_is_just_being_created).exists() + def event_already_exists(self, event_type): + return self.events.filter(type=event_type).exists() + + def repair_end_time(self) -> Optional[bool]: + if not self.stateful: + return + + if self.stateless_event: + # Weird, stateless event without stateless end_time, fix + self.end_time = None + self.save() + LOG.warn("Mismatch between self %s end_time and event type: set stateless", self.pk) + return True + + close_events = self.all_close_events() + if not self.open and close_events.exists(): + # end_time is already set closed (golden path) + return False + + reopen_event = self.reopen_event + last_close_event = close_events.last() + if not reopen_event or reopen_event.timestamp < last_close_event.timestamp: + # end_time was not set when making closing event, fix + self.end_time = close_events.first().timestamp + self.save() + LOG.warn("Mismatch between self %s end_time and event type: set end_time to less than infinity", self.pk) + return True + + # a reopen event correctly exists and the incident is correctly open + return False + def is_acked_by(self, group: str) -> bool: return group in self.acks.active().group_names() diff --git a/src/argus/incident/views.py b/src/argus/incident/views.py index fcb26c6e6..e24f963d0 100644 --- a/src/argus/incident/views.py +++ b/src/argus/incident/views.py @@ -480,37 +480,44 @@ def perform_create(self, serializer: EventSerializer): def validate_event_type_for_user(self, event_type: str, user: User): if user.is_source_system: if event_type not in Event.ALLOWED_TYPES_FOR_SOURCE_SYSTEMS: - self._raise_type_validation_error(f"A source system cannot post events of type '{event_type}'.") + self._abort_due_to_type_validation_error(f"A source system cannot post events of type '{event_type}'.") else: if event_type not in Event.ALLOWED_TYPES_FOR_END_USERS: - self._raise_type_validation_error(f"An end user cannot post events of type '{event_type}'.") + self._abort_due_to_type_validation_error(f"An end user cannot post events of type '{event_type}'.") def validate_event_type_for_incident(self, event_type: str, incident: Incident): - def validate_incident_has_no_relation_to_event_type(): - if incident.events.filter(type=event_type).exists(): - self._raise_type_validation_error(f"The incident already has a related event of type '{event_type}'.") - - if incident.stateful: - if event_type in {Event.Type.INCIDENT_START, Event.Type.INCIDENT_END}: - validate_incident_has_no_relation_to_event_type() - if event_type in {Event.Type.INCIDENT_END, Event.Type.CLOSE} and not incident.open: - self._raise_type_validation_error("The incident is already closed.") - elif event_type == Event.Type.REOPEN and incident.open: - self._raise_type_validation_error("The incident is already open.") - else: - if event_type == Event.Type.STATELESS: - validate_incident_has_no_relation_to_event_type() - elif event_type == Event.Type.INCIDENT_START: - self._raise_type_validation_error("Stateless incident cannot have an INCIDENT_START event.") - elif event_type in {Event.Type.INCIDENT_END, Event.Type.CLOSE, Event.Type.REOPEN}: - self._raise_type_validation_error("Cannot change the state of a stateless incident.") + def abort_due_to_too_many_events(incident, event_type): + error_msg = f"Incident #{incident.pk} can only have one event of type '{event_type}'." + LOG.warn(error_msg) + self._abort_due_to_type_validation_error(error_msg) if event_type == Event.Type.ACKNOWLEDGE: acks_endpoint = reverse("incident:incident-acks", args=[incident.pk], request=self.request) - self._raise_type_validation_error( - f"Acknowledgements of this incidents should be posted through {acks_endpoint}." + self._abort_due_to_type_validation_error( + f"Acknowledgement of an incident should be posted through {acks_endpoint}." ) + if incident.stateful: + if incident.event_already_exists(event_type): + if event_type == Event.Type.INCIDENT_START: + abort_due_to_too_many_events(incident, event_type) + if event_type == Event.Type.INCIDENT_END: + incident.repair_end_time() + abort_due_to_too_many_events(incident, event_type) + if event_type in Event.CLOSING_TYPES and not incident.open: + self._abort_due_to_type_validation_error("The incident is already closed.") + if event_type == Event.Type.REOPEN and incident.open: + self._abort_due_to_type_validation_error("The incident is already open.") + + # type ok for stateful + return + + # stateless from here + if event_type == Event.Type.STATELESS and incident.event_already_exists(event_type): + abort_due_to_too_many_events(incident, event_type) + if event_type in Event.STATE_TYPES: + self._abort_due_to_type_validation_error("Cannot change the state of a stateless incident.") + def update_incident(self, validated_data: dict, incident: Incident): timestamp = validated_data["timestamp"] event_type = validated_data["type"] @@ -522,7 +529,7 @@ def update_incident(self, validated_data: dict, incident: Incident): incident.save() @staticmethod - def _raise_type_validation_error(message: str): + def _abort_due_to_type_validation_error(message: str): raise serializers.ValidationError({"type": message}) diff --git a/tests/incident/test_incident.py b/tests/incident/test_incident.py index 6dddffb11..90ac27333 100644 --- a/tests/incident/test_incident.py +++ b/tests/incident/test_incident.py @@ -11,6 +11,7 @@ ) from argus.incident.models import Event from argus.incident.factories import SourceSystemFactory, SourceUserFactory +from argus.util.datetime_utils import INFINITY_REPR from argus.util.testing import disconnect_signals, connect_signals @@ -89,3 +90,34 @@ def tearDown(self): def test_level_str_returns_name_of_level(self): incident = StatefulIncidentFactory(level=1) self.assertEqual(incident.pp_level(), "Critical") + + +class IncidentRepairEndTimeTests(TestCase): + def setup(self): + disconnect_signals() + + def tearDown(self): + connect_signals() + + def test_golden_path(self): + incident = StatefulIncidentFactory(level=1) + end_time = incident.end_time + self.assertFalse(incident.repair_end_time()) + self.assertEqual(end_time, incident.end_time) + + def test_stateless_event_on_stateful_incident_should_fix_end_time(self): + incident = StatefulIncidentFactory(level=1) + end_time = incident.end_time + EventFactory(incident=incident, type=Event.Type.STATELESS) + self.asserTrue(incident.repair_end_time()) + self.assertNotEqual(end_time, incident.end_time) + self.assertEqual(incident.end_time, None) + + def test_closing_event_without_finite_end_time_and_reopen_event_should_fix_endt_time(self): + incident = StatefulIncidentFactory(level=1) + end_time = incident.end_time + self.assertEqual(incident.end_time, INFINITY_REPR) + EventFactory(incident=incident, type=Event.Type.INCIDENT_END) + self.asserTrue(incident.repair_end_time()) + self.assertNotEqual(end_time, incident.end_time) + self.assertLessThan(incident.end_time, INFINITY_REPR)