Update schedule on-call cache on scheduled refresh tasks (#3699)

Related to https://github.com/grafana/oncall/issues/3673
Keep cache up to date on every schedule refresh task run (which should
keep cache populated every time), helping on any call using cached
information (particularly the direct paging slack dialog building).

CHANGELOG.md

@@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed Webhooks UI not allowing simple webhooks to be created ([#3691](https://github.com/grafana/oncall/pull/3691))
 - Fix posting Slack message when route is deleted by @vadimkerr ([#3702](https://github.com/grafana/oncall/pull/3702))
 
+### Changed
+
+- Update schedules on-call users cache on every scheduled schedule refresh task ([#3699](https://github.com/grafana/oncall/pull/3699)).
+
 ## v1.3.88 (2024-01-16)
 
 ### Fixed

engine/apps/schedules/constants.py

@@ -26,3 +26,6 @@ CALENDAR_TYPE_FINAL = "final"
 
 EXPORT_WINDOW_DAYS_AFTER = 180
 EXPORT_WINDOW_DAYS_BEFORE = 15
+
+SCHEDULE_ONCALL_CACHE_KEY_PREFIX = "schedule_oncall_users_"
+SCHEDULE_ONCALL_CACHE_TTL = 15 * 60  # 15 minutes in seconds

engine/apps/schedules/ical_utils.py

@@ -33,6 +33,8 @@ from apps.schedules.constants import (
     RE_EVENT_UID_V1,
     RE_EVENT_UID_V2,
     RE_PRIORITY,
+    SCHEDULE_ONCALL_CACHE_KEY_PREFIX,
+    SCHEDULE_ONCALL_CACHE_TTL,
 )
 from apps.schedules.ical_events import ical_events
 from common.cache import ensure_cache_key_allocates_to_the_same_hash_slot
@@ -387,6 +389,22 @@ def get_oncall_users_for_multiple_schedules(
     return oncall_users
 
 
+def _generate_cache_key_for_schedule_oncall_users(schedule: "OnCallSchedule") -> str:
+    return ensure_cache_key_allocates_to_the_same_hash_slot(
+        f"{SCHEDULE_ONCALL_CACHE_KEY_PREFIX}{schedule.public_primary_key}", SCHEDULE_ONCALL_CACHE_KEY_PREFIX
+    )
+
+
+def update_cached_oncall_users_for_schedule(schedule: "OnCallSchedule"):
+    oncall_users = get_oncall_users_for_multiple_schedules([schedule])
+    users = oncall_users.get(schedule, [])
+    cache.set(
+        _generate_cache_key_for_schedule_oncall_users(schedule),
+        [user.public_primary_key for user in users],
+        timeout=SCHEDULE_ONCALL_CACHE_TTL,
+    )
+
+
 def get_cached_oncall_users_for_multiple_schedules(schedules: typing.List["OnCallSchedule"]) -> SchedulesOnCallUsers:
     """
     More "performant" version of `apps.schedules.ical_utils.get_oncall_users_for_multiple_schedules`
@@ -404,22 +422,13 @@ def get_cached_oncall_users_for_multiple_schedules(schedules: typing.List["OnCal
     from apps.schedules.models import OnCallSchedule
     from apps.user_management.models import User
 
-    CACHE_KEY_PREFIX = "schedule_oncall_users_"
-
-    def _generate_cache_key_for_schedule_oncall_users(schedule: "OnCallSchedule") -> str:
-        return ensure_cache_key_allocates_to_the_same_hash_slot(
-            f"{CACHE_KEY_PREFIX}{schedule.public_primary_key}", CACHE_KEY_PREFIX
-        )
-
     def _get_schedule_public_primary_key_from_schedule_oncall_users_cache_key(cache_key: str) -> str:
         """
         remove any brackets that might be included in the cache key (when redis cluster is active).
         See `_generate_cache_key_for_schedule_oncall_users` just above
         """
         cache_key = cache_key.replace("{", "").replace("}", "")
-        return cache_key.replace(CACHE_KEY_PREFIX, "")
-
-    CACHE_TTL = 15 * 60  # 15 minutes in seconds
+        return cache_key.replace(SCHEDULE_ONCALL_CACHE_KEY_PREFIX, "")
 
     cache_keys = [_generate_cache_key_for_schedule_oncall_users(schedule) for schedule in schedules]
 
@@ -464,7 +473,7 @@ def get_cached_oncall_users_for_multiple_schedules(schedules: typing.List["OnCal
             _generate_cache_key_for_schedule_oncall_users(schedule)
         ] = oncall_user_public_primary_keys
 
-    cache.set_many(new_results_to_update_in_cache, timeout=CACHE_TTL)
+    cache.set_many(new_results_to_update_in_cache, timeout=SCHEDULE_ONCALL_CACHE_TTL)
 
     # make two queries to the database, one to fetch the schedule objects we need and the other to fetch
     # the user objects we need

engine/apps/schedules/tasks/refresh_ical_files.py

@@ -1,7 +1,7 @@
 from celery.utils.log import get_task_logger
 
 from apps.alerts.tasks import notify_ical_schedule_shift  # type: ignore[no-redef]
-from apps.schedules.ical_utils import is_icals_equal
+from apps.schedules.ical_utils import is_icals_equal, update_cached_oncall_users_for_schedule
 from apps.schedules.tasks import notify_about_empty_shifts_in_schedule_task, notify_about_gaps_in_schedule_task
 from apps.slack.tasks import start_update_slack_user_group_for_schedules
 from common.custom_celery_tasks import shared_dedicated_queue_retry_task
@@ -81,6 +81,9 @@ def refresh_ical_file(schedule_pk):
             task_logger.info(f"run_task_overrides {schedule_pk} {run_task_primary} icals not equal")
     run_task = run_task_primary or run_task_overrides
 
+    # update cached schedule on-call users
+    update_cached_oncall_users_for_schedule(schedule)
+
     if run_task:
         notify_about_empty_shifts_in_schedule_task.apply_async((schedule_pk,))
         notify_about_gaps_in_schedule_task.apply_async((schedule_pk,))

engine/apps/schedules/tests/tasks/test_refresh_ical_files.py

@@ -2,8 +2,10 @@ import datetime
 from unittest.mock import patch
 
 import pytest
+from django.core.cache import cache
+from django.utils import timezone
 
-from apps.schedules.models import OnCallScheduleICal, OnCallScheduleWeb
+from apps.schedules.models import CustomOnCallShift, OnCallScheduleICal, OnCallScheduleWeb
 from apps.schedules.tasks.refresh_ical_files import refresh_ical_file, start_refresh_ical_files
 
 
@@ -79,3 +81,48 @@ def test_refresh_ical_files_filter_orgs(
         assert len(called_args) == 1
         assert schedule.id in called_args[0].args[0]
         assert schedule_from_deleted_org.id not in called_args[0].args[0]
+
+
+@pytest.mark.django_db
+def test_refresh_ical_updates_oncall_cache(
+    make_organization,
+    make_user_for_organization,
+    make_schedule,
+    make_on_call_shift,
+):
+    organization = make_organization()
+    users = [make_user_for_organization(organization) for _ in range(2)]
+
+    today = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0)
+    schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb)
+    shift_start_time = today - timezone.timedelta(hours=1)
+    on_call_shift = make_on_call_shift(
+        organization=organization,
+        shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT,
+        start=shift_start_time,
+        rotation_start=shift_start_time,
+        duration=timezone.timedelta(seconds=(24 * 60 * 60)),
+        priority_level=1,
+        frequency=CustomOnCallShift.FREQUENCY_DAILY,
+        schedule=schedule,
+    )
+    on_call_shift.add_rolling_users([users])
+
+    def _generate_cache_key(schedule):
+        return f"schedule_oncall_users_{schedule.public_primary_key}"
+
+    # start with empty cache
+    cache.clear()
+
+    # patch ical comparison to string compare
+    with patch("apps.schedules.tasks.refresh_ical_files.is_icals_equal", side_effect=lambda a, b: a == b):
+        # patch schedule refresh to avoid changing schedule status (keep as defined above)
+        with patch("apps.schedules.models.OnCallSchedule.refresh_ical_file", return_value=None):
+            # do not trigger tasks for real
+            with patch("apps.schedules.tasks.refresh_ical_files.notify_ical_schedule_shift"):
+                with patch("apps.schedules.tasks.refresh_ical_files.notify_about_empty_shifts_in_schedule_task"):
+                    with patch("apps.schedules.tasks.refresh_ical_files.notify_about_gaps_in_schedule_task"):
+                        refresh_ical_file(schedule.pk)
+
+    cached_data = cache.get(_generate_cache_key(schedule))
+    assert cached_data == [u.public_primary_key for u in users]