Skip to content

Commit 6bf7d15

Browse files
author
Sasha Romijn
committed
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
1 parent 084978e commit 6bf7d15

2 files changed

Lines changed: 14 additions & 2 deletions

File tree

ietf/review/policies.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,13 @@ def reviewer_at_index(i):
9292

9393
# Loop through the list until finding the first person with skip_next=0,
9494
# who is not the current assignee. Anyone with skip_next>0 encountered before
95-
# has their skip_next decreased.
95+
# has their skip_next decreased. There is a cap on the number of loops, which can
96+
# be hit e.g. if there is only a single reviewer, and the current assignee is excluded
97+
# from being set as NextReviewerInTeam.
9698
current_idx = 0
99+
max_loops = sum([self._reviewer_settings_for(r).skip_next for r in rotation_list]) + len(rotation_list)
97100
if in_order_assignment:
98-
while True:
101+
while current_idx <= max_loops:
99102
current_idx_person = reviewer_at_index(current_idx)
100103
settings = self._reviewer_settings_for(current_idx_person)
101104
if settings.skip_next > 0:

ietf/review/test_policies.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,15 @@ def get_skip_next(person):
198198
self.assertEqual(get_skip_next(reviewers[3]), 1)
199199
self.assertEqual(get_skip_next(reviewers[4]), 0)
200200

201+
# Leave only a single reviewer remaining, which should not trigger an infinite loop.
202+
# The deletion also causes NextReviewerInTeam to be deleted.
203+
[reviewer.delete() for reviewer in reviewers[1:]]
204+
self.assertEqual([reviewers[0]], policy.default_reviewer_rotation_list())
205+
policy.update_policy_state_for_assignment(assignee_person=reviewers[0], add_skip=False)
206+
# No NextReviewerInTeam should be created, the only possible next is the excluded assignee.
207+
self.assertFalse(NextReviewerInTeam.objects.filter(team=team))
208+
self.assertEqual([reviewers[0]], policy.default_reviewer_rotation_list())
209+
201210
def test_return_reviewer_to_top_rotation(self):
202211
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team",
203212
list_email="rotationteam@ietf.org",

0 commit comments

Comments
 (0)