From d101bf3e0e0108d86730dd0ae60f748eeb7a8b50 Mon Sep 17 00:00:00 2001 From: Jared Lewis Date: Mon, 24 Mar 2025 17:02:53 -0400 Subject: [PATCH 1/6] Handle no-op handlers --- .../0006_rrule_time_last_handled.py | 18 ++++ ambition_utils/rrule/models.py | 10 +++ ambition_utils/rrule/tests/model_tests.py | 84 +++++++++++++++++++ ambition_utils/rrule/tests/models.py | 6 ++ 4 files changed, 118 insertions(+) create mode 100644 ambition_utils/rrule/migrations/0006_rrule_time_last_handled.py diff --git a/ambition_utils/rrule/migrations/0006_rrule_time_last_handled.py b/ambition_utils/rrule/migrations/0006_rrule_time_last_handled.py new file mode 100644 index 0000000..769dabb --- /dev/null +++ b/ambition_utils/rrule/migrations/0006_rrule_time_last_handled.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.20 on 2025-03-24 20:56 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('rrule', '0005_auto_20230802_1548'), + ] + + operations = [ + migrations.AddField( + model_name='rrule', + name='time_last_handled', + field=models.DateTimeField(default=None, null=True), + ), + ] diff --git a/ambition_utils/rrule/models.py b/ambition_utils/rrule/models.py index 41f3096..9f7339c 100644 --- a/ambition_utils/rrule/models.py +++ b/ambition_utils/rrule/models.py @@ -78,6 +78,7 @@ def process_related_model_handlers(self, limit=None, filters=None): related_object_id__isnull=False, **(filters or {}) ).order_by( + F('time_last_handled').asc(nulls_first=True), 'next_occurrence', 'id' ).prefetch_related( @@ -86,6 +87,8 @@ def process_related_model_handlers(self, limit=None, filters=None): if limit: rrule_objects = rrule_objects[:limit] + rrule_objects = list(rrule_objects) + rrule_objects_ids = [rrule_object.id for rrule_object in rrule_objects] rrules_to_advance = [] for rrule_object in rrule_objects: if hasattr(rrule_object.related_object, rrule_object.related_object_handler_name): @@ -98,6 +101,10 @@ def process_related_model_handlers(self, limit=None, filters=None): # Bulk update the next occurrences rrules_to_advance = RRule.objects.update_next_occurrences(rrule_objects=rrules_to_advance) + # Update the last handled time + RRule.objects.filter(id__in=rrule_objects_ids).update(time_last_handled=datetime.utcnow()) + + # Return the rules we advanced return rrules_to_advance def overdue_handler_class_instances(self, limit=None, **kwargs): @@ -171,6 +178,9 @@ class RRule(models.Model): # An optional number of days to offset occurrences by day_offset = models.SmallIntegerField(blank=True, null=True) + # The last time we handled this rrule for overdue occurrences + time_last_handled = models.DateTimeField(null=True, default=None) + # Custom object manager objects = RRuleManager() diff --git a/ambition_utils/rrule/tests/model_tests.py b/ambition_utils/rrule/tests/model_tests.py index 08d6a47..41f6ae1 100644 --- a/ambition_utils/rrule/tests/model_tests.py +++ b/ambition_utils/rrule/tests/model_tests.py @@ -421,6 +421,90 @@ def test_related_object_handlers_with_limit(self): self.assertEqual(program2.start_recurrence.next_occurrence, datetime.datetime(2022, 6, 2, 9)) self.assertEqual(program2.end_recurrence.next_occurrence, datetime.datetime(2022, 6, 2, 17)) + def test_related_object_handlers_no_op(self): + """ + Test a scenario where the handler of the object is no-op and does not progress the rule + """ + + no_op_program1 = Program.objects.create(name='Program 1') + rrule1 = RRule.objects.create( + rrule_params={ + 'freq': rrule.DAILY, + 'interval': 1, + 'dtstart': datetime.datetime(2022, 6, 1, 9), + 'byhour': 9, + }, + related_object=no_op_program1, + related_object_handler_name='handle_no_op', + ) + no_op_program1.start_recurrence = rrule1 + no_op_program1.save() + + no_op_program2 = Program.objects.create(name='Program 1') + rrule2 = RRule.objects.create( + rrule_params={ + 'freq': rrule.DAILY, + 'interval': 1, + 'dtstart': datetime.datetime(2022, 6, 1, 9), + 'byhour': 9, + }, + related_object=no_op_program1, + related_object_handler_name='handle_no_op', + ) + no_op_program2.start_recurrence = rrule2 + no_op_program2.save() + + program = Program.objects.create(name='Program 1') + rrule3 = RRule.objects.create( + rrule_params={ + 'freq': rrule.DAILY, + 'interval': 1, + 'dtstart': datetime.datetime(2022, 6, 1, 9), + 'byhour': 9, + }, + related_object=program, + related_object_handler_name='handle_start_recurrence', + ) + program.start_recurrence = rrule3 + program.save() + + # Run with a limit of 1, we should handle the no-op program + with freeze_time(datetime.datetime(2022, 6, 1, 9)): + RRule.objects.handle_overdue(related_model_handler_limit=1) + + # Make sure only the no-op program is handled + no_op_program1.refresh_from_db() + no_op_program2.refresh_from_db() + program.refresh_from_db() + self.assertEqual(no_op_program1.start_recurrence.time_last_handled, datetime.datetime(2022, 6, 1, 9)) + self.assertEqual(no_op_program2.start_recurrence.time_last_handled, None) + self.assertEqual(program.start_recurrence.time_last_handled, None) + + # Run with a limit of 1 again , we should handle the no-op program2 + with freeze_time(datetime.datetime(2022, 6, 1, 9)): + RRule.objects.handle_overdue(related_model_handler_limit=1) + + # Make sure only the no-op program is handled + no_op_program1.refresh_from_db() + no_op_program2.refresh_from_db() + program.refresh_from_db() + self.assertEqual(no_op_program1.start_recurrence.time_last_handled, datetime.datetime(2022, 6, 1, 9)) + self.assertEqual(no_op_program2.start_recurrence.time_last_handled, datetime.datetime(2022, 6, 1, 9)) + self.assertEqual(program.start_recurrence.time_last_handled, None) + + # Run with a limit of 1 again , we should handle the program + with freeze_time(datetime.datetime(2022, 6, 1, 9)): + RRule.objects.handle_overdue(related_model_handler_limit=1) + + # Make sure only the program is handled + no_op_program1.refresh_from_db() + no_op_program2.refresh_from_db() + program.refresh_from_db() + self.assertEqual(no_op_program1.start_recurrence.time_last_handled, datetime.datetime(2022, 6, 1, 9)) + self.assertEqual(no_op_program2.start_recurrence.time_last_handled, datetime.datetime(2022, 6, 1, 9)) + self.assertEqual(program.start_recurrence.time_last_handled, datetime.datetime(2022, 6, 1, 9)) + + def test_related_object_handlers_invalid_handler(self): """ Hits the else block when the handler path is not valid diff --git a/ambition_utils/rrule/tests/models.py b/ambition_utils/rrule/tests/models.py index ce35a1a..8c533f7 100644 --- a/ambition_utils/rrule/tests/models.py +++ b/ambition_utils/rrule/tests/models.py @@ -24,3 +24,9 @@ def handle_end_recurrence(self, rrule): self.end_called += 1 self.save() return rrule + + def handle_no_op(self, rrule): + """ + A no op handler that does nothing + """ + return None From 918a9b198efd48525d0b740bb74024be8b413ed3 Mon Sep 17 00:00:00 2001 From: Jared Lewis Date: Mon, 24 Mar 2025 17:14:13 -0400 Subject: [PATCH 2/6] no op for other handler --- ambition_utils/rrule/models.py | 14 ++++- ambition_utils/rrule/tests/model_tests.py | 64 +++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/ambition_utils/rrule/models.py b/ambition_utils/rrule/models.py index 9f7339c..cfc108e 100644 --- a/ambition_utils/rrule/models.py +++ b/ambition_utils/rrule/models.py @@ -65,11 +65,22 @@ def process_occurrence_handler_paths(self, limit=None, **kwargs): # Build a list of rrules that get returned from the handler rrules = [] for instance in instances: - rrules.extend(instance.handle()) + handler_rrules = instance.handle() + if handler_rrules: + rrules.extend(handler_rrules) # Bulk update the next occurrences RRule.objects.update_next_occurrences(rrule_objects=rrules) + # Update the last handled time + handler_paths = [ + f"{instance.__class__.__module__}.{instance.__class__.__name__}" + for instance in instances + ] + RRule.objects.filter( + occurrence_handler_path__in=handler_paths + ).update(time_last_handled=datetime.utcnow()) + def process_related_model_handlers(self, limit=None, filters=None): # Get the rrule objects that are overdue and need to be handled rrule_objects = self.get_queryset().filter( @@ -125,6 +136,7 @@ def overdue_handler_class_instances(self, limit=None, **kwargs): ).filter( row_num=1 ).order_by( + F('time_last_handled').asc(nulls_first=True), 'next_occurrence', 'occurrence_handler_path' ) diff --git a/ambition_utils/rrule/tests/model_tests.py b/ambition_utils/rrule/tests/model_tests.py index 41f6ae1..5dd4d4f 100644 --- a/ambition_utils/rrule/tests/model_tests.py +++ b/ambition_utils/rrule/tests/model_tests.py @@ -50,6 +50,12 @@ def handle(self): ).order_by('id') +class HandlerNoOp(OccurrenceHandler): + + def handle(self): + return None + + class RRuleManagerTest(TestCase): def add_test_rules(self): # Make a program with HandlerOne that is not overdue @@ -266,6 +272,64 @@ def test_run_with_limit(self): self.assertEqual(recurrences[2].next_occurrence, datetime.datetime(2017, 1, 4)) self.assertEqual(recurrences[3].next_occurrence, datetime.datetime(2017, 1, 4)) + def test_run_with_no_op(self): + """ + Test running with a no-op handler + """ + rrule_params = { + 'freq': rrule.DAILY, + 'interval': 1, + 'dtstart': datetime.datetime(2017, 1, 2), + } + no_op_rrule1 = G( + RRule, + rrule_params=rrule_params, + occurrence_handler_path='ambition_utils.rrule.tests.model_tests.HandlerNoOp' + ) + no_op_rrule2 = G( + RRule, + rrule_params=rrule_params, + occurrence_handler_path='ambition_utils.rrule.tests.model_tests.HandlerNoOp' + ) + rrule1 = G( + RRule, + rrule_params=rrule_params, + occurrence_handler_path='ambition_utils.rrule.tests.model_tests.HandlerOne' + ) + + # Process with a limit of 1, we should handle the no-op rrules + with freeze_time(datetime.datetime(2017, 1, 2)): + RRule.objects.handle_overdue(occurrence_handler_limit=1) + + no_op_rrule1.refresh_from_db() + no_op_rrule2.refresh_from_db() + rrule1.refresh_from_db() + self.assertEqual(no_op_rrule1.time_last_handled, datetime.datetime(2017, 1, 2)) + self.assertEqual(no_op_rrule2.time_last_handled, datetime.datetime(2017, 1, 2)) + self.assertEqual(rrule1.time_last_handled, None) + + # Process with a limit of 1, we should handle handler one + with freeze_time(datetime.datetime(2017, 1, 3)): + RRule.objects.handle_overdue(occurrence_handler_limit=1) + + no_op_rrule1.refresh_from_db() + no_op_rrule2.refresh_from_db() + rrule1.refresh_from_db() + self.assertEqual(no_op_rrule1.time_last_handled, datetime.datetime(2017, 1, 2)) + self.assertEqual(no_op_rrule2.time_last_handled, datetime.datetime(2017, 1, 2)) + self.assertEqual(rrule1.time_last_handled, datetime.datetime(2017, 1, 3)) + + # Process again and we should go back to the no op handler + with freeze_time(datetime.datetime(2017, 1, 4)): + RRule.objects.handle_overdue(occurrence_handler_limit=1) + + no_op_rrule1.refresh_from_db() + no_op_rrule2.refresh_from_db() + rrule1.refresh_from_db() + self.assertEqual(no_op_rrule1.time_last_handled, datetime.datetime(2017, 1, 4)) + self.assertEqual(no_op_rrule2.time_last_handled, datetime.datetime(2017, 1, 4)) + self.assertEqual(rrule1.time_last_handled, datetime.datetime(2017, 1, 3)) + class RRuleTest(TestCase): From f951eb6975696cd706c8be5457ca67ca07cb50b4 Mon Sep 17 00:00:00 2001 From: Jared Lewis Date: Mon, 24 Mar 2025 17:15:38 -0400 Subject: [PATCH 3/6] index --- .../rrule/migrations/0006_rrule_time_last_handled.py | 4 ++-- ambition_utils/rrule/models.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ambition_utils/rrule/migrations/0006_rrule_time_last_handled.py b/ambition_utils/rrule/migrations/0006_rrule_time_last_handled.py index 769dabb..8e57ecf 100644 --- a/ambition_utils/rrule/migrations/0006_rrule_time_last_handled.py +++ b/ambition_utils/rrule/migrations/0006_rrule_time_last_handled.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.20 on 2025-03-24 20:56 +# Generated by Django 4.2.20 on 2025-03-24 21:15 from django.db import migrations, models @@ -13,6 +13,6 @@ class Migration(migrations.Migration): migrations.AddField( model_name='rrule', name='time_last_handled', - field=models.DateTimeField(default=None, null=True), + field=models.DateTimeField(db_index=True, default=None, null=True), ), ] diff --git a/ambition_utils/rrule/models.py b/ambition_utils/rrule/models.py index cfc108e..6e1fed2 100644 --- a/ambition_utils/rrule/models.py +++ b/ambition_utils/rrule/models.py @@ -191,7 +191,7 @@ class RRule(models.Model): day_offset = models.SmallIntegerField(blank=True, null=True) # The last time we handled this rrule for overdue occurrences - time_last_handled = models.DateTimeField(null=True, default=None) + time_last_handled = models.DateTimeField(null=True, default=None, db_index=True) # Custom object manager objects = RRuleManager() From 6d8b26bb61dc2e0664bbb79807e1f742522b2045 Mon Sep 17 00:00:00 2001 From: Jared Lewis Date: Mon, 24 Mar 2025 17:17:44 -0400 Subject: [PATCH 4/6] bump version --- ambition_utils/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ambition_utils/version.py b/ambition_utils/version.py index b50da94..29e4a94 100644 --- a/ambition_utils/version.py +++ b/ambition_utils/version.py @@ -1 +1 @@ -__version__ = '3.2.1' +__version__ = '3.2.2' From 65da25d292d05899322393ffd29d6499527a4603 Mon Sep 17 00:00:00 2001 From: Jared Lewis Date: Mon, 24 Mar 2025 17:30:21 -0400 Subject: [PATCH 5/6] fix test --- ambition_utils/rrule/tests/model_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ambition_utils/rrule/tests/model_tests.py b/ambition_utils/rrule/tests/model_tests.py index 5dd4d4f..5825c50 100644 --- a/ambition_utils/rrule/tests/model_tests.py +++ b/ambition_utils/rrule/tests/model_tests.py @@ -248,8 +248,8 @@ def test_run_with_limit(self): # except for handler two which should occur three times expected_classes = [ {HandlerTwo}, - {HandlerOne}, {HandlerThree}, + {HandlerOne}, {HandlerTwo}, {HandlerOne}, {HandlerThree}, From 62772adb2fd76c4f3e55ff9ae8404e59d040db4a Mon Sep 17 00:00:00 2001 From: Jared Lewis Date: Mon, 24 Mar 2025 17:47:47 -0400 Subject: [PATCH 6/6] flake8 --- ambition_utils/rrule/tests/model_tests.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ambition_utils/rrule/tests/model_tests.py b/ambition_utils/rrule/tests/model_tests.py index 5825c50..f82713b 100644 --- a/ambition_utils/rrule/tests/model_tests.py +++ b/ambition_utils/rrule/tests/model_tests.py @@ -325,7 +325,7 @@ def test_run_with_no_op(self): no_op_rrule1.refresh_from_db() no_op_rrule2.refresh_from_db() - rrule1.refresh_from_db() + rrule1.refresh_from_db() self.assertEqual(no_op_rrule1.time_last_handled, datetime.datetime(2017, 1, 4)) self.assertEqual(no_op_rrule2.time_last_handled, datetime.datetime(2017, 1, 4)) self.assertEqual(rrule1.time_last_handled, datetime.datetime(2017, 1, 3)) @@ -503,7 +503,7 @@ def test_related_object_handlers_no_op(self): ) no_op_program1.start_recurrence = rrule1 no_op_program1.save() - + no_op_program2 = Program.objects.create(name='Program 1') rrule2 = RRule.objects.create( rrule_params={ @@ -568,7 +568,6 @@ def test_related_object_handlers_no_op(self): self.assertEqual(no_op_program2.start_recurrence.time_last_handled, datetime.datetime(2022, 6, 1, 9)) self.assertEqual(program.start_recurrence.time_last_handled, datetime.datetime(2022, 6, 1, 9)) - def test_related_object_handlers_invalid_handler(self): """ Hits the else block when the handler path is not valid