From 6bd5053ae8f77183fd96dfbea05cc1985e257ecf Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Thu, 2 Apr 2020 16:00:21 -0400 Subject: [PATCH] remove the limitation on (very) old DTSTART values for schedules --- awx/api/serializers.py | 2 + awx/main/models/schedules.py | 14 +++++-- .../tests/functional/api/test_schedules.py | 37 ++++++++++++++++++- .../tests/functional/models/test_schedule.py | 7 +++- 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 47cf3dffa8..51062c7885 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -4539,6 +4539,8 @@ class SchedulePreviewSerializer(BaseSerializer): try: Schedule.rrulestr(rrule_value) except Exception as e: + import traceback + logger.error(traceback.format_exc()) raise serializers.ValidationError(_("rrule parsing failed validation: {}").format(e)) return value diff --git a/awx/main/models/schedules.py b/awx/main/models/schedules.py index 58aee91d96..a3dd32d6b0 100644 --- a/awx/main/models/schedules.py +++ b/awx/main/models/schedules.py @@ -191,7 +191,7 @@ class Schedule(PrimordialModel, LaunchTimeConfig): return rrule @classmethod - def rrulestr(cls, rrule, **kwargs): + def rrulestr(cls, rrule, fast_forward=True, **kwargs): """ Apply our own custom rrule parsing requirements """ @@ -205,11 +205,17 @@ class Schedule(PrimordialModel, LaunchTimeConfig): 'A valid TZID must be provided (e.g., America/New_York)' ) - if 'MINUTELY' in rrule or 'HOURLY' in rrule: + if fast_forward and ('MINUTELY' in rrule or 'HOURLY' in rrule): try: first_event = x[0] - if first_event < now() - datetime.timedelta(days=365 * 5): - raise ValueError('RRULE values with more than 1000 events are not allowed.') + if first_event < now(): + # hourly/minutely rrules with far-past DTSTART values + # are *really* slow to precompute + # start *from* one week ago to speed things up drastically + dtstart = x._rrule[0]._dtstart.strftime(':%Y%m%dT') + new_start = (now() - datetime.timedelta(days=7)).strftime(':%Y%m%dT') + new_rrule = rrule.replace(dtstart, new_start) + return Schedule.rrulestr(new_rrule, fast_forward=False) except IndexError: pass return x diff --git a/awx/main/tests/functional/api/test_schedules.py b/awx/main/tests/functional/api/test_schedules.py index 996c0d4721..7b93c2804b 100644 --- a/awx/main/tests/functional/api/test_schedules.py +++ b/awx/main/tests/functional/api/test_schedules.py @@ -1,6 +1,8 @@ +import datetime import pytest from django.utils.encoding import smart_str +from django.utils.timezone import now from awx.api.versioning import reverse from awx.main.models import JobTemplate, Schedule @@ -140,7 +142,6 @@ def test_encrypted_survey_answer(post, patch, admin_user, project, inventory, su ("DTSTART:20030925T104941Z RRULE:FREQ=DAILY;INTERVAL=10;COUNT=500;UNTIL=20040925T104941Z", "RRULE may not contain both COUNT and UNTIL"), # noqa ("DTSTART;TZID=America/New_York:20300308T050000Z RRULE:FREQ=DAILY;INTERVAL=1", "rrule parsing failed validation"), ("DTSTART:20300308T050000 RRULE:FREQ=DAILY;INTERVAL=1", "DTSTART cannot be a naive datetime"), - ("DTSTART:19700101T000000Z RRULE:FREQ=MINUTELY;INTERVAL=1", "more than 1000 events are not allowed"), # noqa ]) def test_invalid_rrules(post, admin_user, project, inventory, rrule, error): job_template = JobTemplate.objects.create( @@ -342,6 +343,40 @@ def test_months_with_31_days(post, admin_user): ] +@pytest.mark.django_db +@pytest.mark.timeout(3) +@pytest.mark.parametrize('freq, delta, total_seconds', ( + ('MINUTELY', 1, 60), + ('MINUTELY', 15, 15 * 60), + ('HOURLY', 1, 3600), + ('HOURLY', 4, 3600 * 4), +)) +def test_really_old_dtstart(post, admin_user, freq, delta, total_seconds): + url = reverse('api:schedule_rrule') + # every , at the :30 second mark + rrule = f'DTSTART;TZID=America/New_York:20051231T000030 RRULE:FREQ={freq};INTERVAL={delta}' + start = now() + next_ten = post(url, {'rrule': rrule}, admin_user, expect=200).data['utc'] + + assert len(next_ten) == 10 + + # the first date is *in the future* + assert next_ten[0] >= start + + # ...but *no more than* into the future + assert now() + datetime.timedelta(**{ + 'minutes' if freq == 'MINUTELY' else 'hours': delta + }) + + # every date in the list is greater than the last + for i, x in enumerate(next_ten): + if i == 0: + continue + assert x.second == 30 + delta = (x - next_ten[i - 1]) + assert delta.total_seconds() == total_seconds + + def test_dst_rollback_duplicates(post, admin_user): # From Nov 2 -> Nov 3, 2030, daylight savings ends and we "roll back" an hour. # Make sure we don't "double count" duplicate times in the "rolled back" diff --git a/awx/main/tests/functional/models/test_schedule.py b/awx/main/tests/functional/models/test_schedule.py index 3521e9a95d..5575921ff0 100644 --- a/awx/main/tests/functional/models/test_schedule.py +++ b/awx/main/tests/functional/models/test_schedule.py @@ -325,16 +325,19 @@ def test_dst_phantom_hour(job_template): @pytest.mark.django_db +@pytest.mark.timeout(3) def test_beginning_of_time(job_template): # ensure that really large generators don't have performance issues + start = now() rrule = 'DTSTART:19700101T000000Z RRULE:FREQ=MINUTELY;INTERVAL=1' s = Schedule( name='Some Schedule', rrule=rrule, unified_job_template=job_template ) - with pytest.raises(ValueError): - s.save() + s.save() + assert s.next_run > start + assert (s.next_run - start).total_seconds() < 60 @pytest.mark.django_db