From 6bf7d15b70e3861a1527441910520a3093391689 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Thu, 21 Nov 2019 12:37:02 +0000 Subject: [PATCH] Add a limit to the update_policy_state_for_assignment loop to prevent infinite loops, e.g. when a team has only a single reviewer. - Legacy-Id: 17087 --- ietf/review/policies.py | 7 +++++-- ietf/review/test_policies.py | 9 +++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/ietf/review/policies.py b/ietf/review/policies.py index d4e3dc6ab..2e123e5b1 100644 --- a/ietf/review/policies.py +++ b/ietf/review/policies.py @@ -92,10 +92,13 @@ class AbstractReviewerQueuePolicy: # Loop through the list until finding the first person with skip_next=0, # who is not the current assignee. Anyone with skip_next>0 encountered before - # has their skip_next decreased. + # has their skip_next decreased. There is a cap on the number of loops, which can + # be hit e.g. if there is only a single reviewer, and the current assignee is excluded + # from being set as NextReviewerInTeam. current_idx = 0 + max_loops = sum([self._reviewer_settings_for(r).skip_next for r in rotation_list]) + len(rotation_list) if in_order_assignment: - while True: + while current_idx <= max_loops: current_idx_person = reviewer_at_index(current_idx) settings = self._reviewer_settings_for(current_idx_person) if settings.skip_next > 0: diff --git a/ietf/review/test_policies.py b/ietf/review/test_policies.py index 76176fa7a..1ca299208 100644 --- a/ietf/review/test_policies.py +++ b/ietf/review/test_policies.py @@ -198,6 +198,15 @@ class RotateAlphabeticallyReviewerAndGenericQueuePolicyTest(TestCase): self.assertEqual(get_skip_next(reviewers[3]), 1) self.assertEqual(get_skip_next(reviewers[4]), 0) + # Leave only a single reviewer remaining, which should not trigger an infinite loop. + # The deletion also causes NextReviewerInTeam to be deleted. + [reviewer.delete() for reviewer in reviewers[1:]] + self.assertEqual([reviewers[0]], policy.default_reviewer_rotation_list()) + policy.update_policy_state_for_assignment(assignee_person=reviewers[0], add_skip=False) + # No NextReviewerInTeam should be created, the only possible next is the excluded assignee. + self.assertFalse(NextReviewerInTeam.objects.filter(team=team)) + self.assertEqual([reviewers[0]], policy.default_reviewer_rotation_list()) + def test_return_reviewer_to_top_rotation(self): team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org",