diff --git a/CHANGELOG.md b/CHANGELOG.md index 4025b42e..6f175681 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/engine/apps/schedules/constants.py b/engine/apps/schedules/constants.py index 818ddeb1..880cf50d 100644 --- a/engine/apps/schedules/constants.py +++ b/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 diff --git a/engine/apps/schedules/ical_utils.py b/engine/apps/schedules/ical_utils.py index 9dede8fc..eb9840d5 100644 --- a/engine/apps/schedules/ical_utils.py +++ b/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 diff --git a/engine/apps/schedules/tasks/refresh_ical_files.py b/engine/apps/schedules/tasks/refresh_ical_files.py index d0ae3a4a..a022d464 100644 --- a/engine/apps/schedules/tasks/refresh_ical_files.py +++ b/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,)) diff --git a/engine/apps/schedules/tests/tasks/test_refresh_ical_files.py b/engine/apps/schedules/tests/tasks/test_refresh_ical_files.py index c6c8ced7..fdbbec36 100644 --- a/engine/apps/schedules/tests/tasks/test_refresh_ical_files.py +++ b/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]