Implement rudimentary rate limiting (#4653)

Author: pgrace-google

src/clusterfuzz/_internal/cron/external_testcase_reader.py

@@ -96,9 +96,9 @@ def close_issue_if_invalid(upload_request, attachment_info, description,
   return invalid
 
 
-def close_issue_if_not_reproducible(issue):
-  if issue.status == ISSUETRACKER_ACCEPTED_STATE and filed_one_day_ago(
-      issue.created_time):
+def close_issue_if_not_reproducible(issue, config):
+  if issue.status == ISSUETRACKER_ACCEPTED_STATE and filed_n_days_ago(
+      issue.created_time, config):
     comment_message = ('Clusterfuzz failed to reproduce - '
                        'please check testcase details for more info.')
     issue.status = ISSUETRACKER_WONTFIX_STATE
@@ -107,10 +107,11 @@ def close_issue_if_not_reproducible(issue):
   return False
 
 
-def filed_one_day_ago(issue_created_time_string):
+def filed_n_days_ago(issue_created_time_string, config):
   created_time = datetime.datetime.strptime(issue_created_time_string,
                                             '%Y-%m-%dT%H:%M:%S.%fZ')
-  return datetime.datetime.now() - created_time > datetime.timedelta(days=1)
+  return datetime.datetime.now() - created_time > datetime.timedelta(
+      days=config.get('submitted-buffer-days'))
 
 
 def submit_testcase(issue_id, file, filename, filetype, cmds):
@@ -152,31 +153,41 @@ def submit_testcase(issue_id, file, filename, filetype, cmds):
 
 
 def handle_testcases(tracker, config):
-  """Fetches and submits testcases from bugs or closes unnecssary bugs."""
-  # TODO(pgrace) remove ID filter once done testing.
-  issues = tracker.find_issues_with_filters(
+  """Fetches and submits testcases from bugs or closes unnecessary bugs."""
+
+  # Handle bugs that were already submitted and still open.
+  older_issues = tracker.find_issues_with_filters(
       keywords=[],
-      query_filters=['componentid:1600865', 'id:373893311'],
+      query_filters=['componentid:1600865', 'status:accepted'],
       only_open=True)
+  for issue in older_issues:
+    # Close out older bugs that may have failed to reproduce.
+    if close_issue_if_not_reproducible(issue, config):
+      helpers.log('Closing issue {issue_id} as it failed to reproduce',
+                  issue.id)
 
+  # Handle new bugs that may need to be submitted.
+  issues = tracker.find_issues_with_filters(
+      keywords=[],
+      query_filters=['componentid:1600865', 'status:new'],
+      only_open=True)
   if len(issues) == 0:
     return
 
   # TODO(pgrace) Cache in redis.
   vrp_uploaders = get_vrp_uploaders(config)
 
-  # TODO(pgrace) Implement rudimentary rate limiting.
+  # Rudimentary rate limiting -
+  # Process only a certain number of bugs per reporter for each job run.
+  reporters_map = {}
 
   for issue in issues:
-    # Close out older bugs that may have failed to reproduce.
-    if close_issue_if_not_reproducible(issue):
-      helpers.log('Closing issue {issue_id} as it failed to reproduce',
-                  issue.id)
-      continue
-
-    # Close out invalid bugs.
     attachment_metadata = tracker.get_attachment_metadata(issue.id)
     commandline_flags = tracker.get_description(issue.id)
+    if reporters_map.get(issue.reporter,
+                         0) > config.get('max-report-count-per-run'):
+      continue
+    reporters_map[issue.reporter] = reporters_map.get(issue.reporter, 1) + 1
     if close_issue_if_invalid(issue, attachment_metadata, commandline_flags,
                               vrp_uploaders):
       helpers.log('Closing issue {issue_id} as it is invalid', issue.id)

src/clusterfuzz/_internal/tests/appengine/handlers/cron/external_testcase_reader_test.py

@@ -32,7 +32,9 @@
 }
 BASIC_CONFIG = {
     'vrp-uploaders-bucket': 'bucket-name',
-    'vrp-uploaders-blob': 'blob-name'
+    'vrp-uploaders-blob': 'blob-name',
+    'max-report-count-per-run': 5,
+    'submitted-buffer-days': 1,
 }
 
 
@@ -53,8 +55,8 @@ def test_handle_testcases(self, mock_close_issue_if_invalid,
     mock_close_issue_if_invalid.return_value = False
     mock_it = mock.create_autospec(issue_tracker.IssueTracker)
     basic_issue = mock.MagicMock()
-    basic_issue.reporter.return_value = '[email protected]'
-    mock_it.find_issues_with_filters.return_value = [basic_issue]
+    basic_issue.reporter = '[email protected]'
+    mock_it.find_issues_with_filters.side_effect = [[], [basic_issue]]
 
     external_testcase_reader.handle_testcases(mock_it, BASIC_CONFIG)
 
@@ -68,15 +70,33 @@ def test_handle_testcases_invalid(self, mock_close_issue_if_invalid,
     mock_close_issue_if_invalid.return_value = True
     mock_it = mock.create_autospec(issue_tracker.IssueTracker)
     basic_issue = mock.MagicMock()
-    basic_issue.reporter.return_value = '[email protected]'
-    mock_it.find_issues_with_filters.return_value = [basic_issue]
+    basic_issue.reporter = '[email protected]'
+    mock_it.find_issues_with_filters.side_effect = [[], [basic_issue]]
 
     external_testcase_reader.handle_testcases(mock_it, BASIC_CONFIG)
 
     mock_close_issue_if_invalid.assert_called_once()
     mock_it.get_attachment.assert_not_called()
     mock_submit_testcase.assert_not_called()
 
+  def test_handle_testcases_rate_limit(self, mock_close_issue_if_invalid,
+                                       mock_submit_testcase, _):
+    """Test a handle_testcases where one reporter hits the rate limit."""
+    mock_close_issue_if_invalid.return_value = False
+    mock_it = mock.create_autospec(issue_tracker.IssueTracker)
+    basic_issue = mock.MagicMock()
+    basic_issue.reporter = '[email protected]'
+    mock_it.find_issues_with_filters.side_effect = [[], [
+        basic_issue, basic_issue, basic_issue, basic_issue, basic_issue,
+        basic_issue
+    ]]
+
+    external_testcase_reader.handle_testcases(mock_it, BASIC_CONFIG)
+
+    mock_close_issue_if_invalid.assert_called()
+    self.assertEqual(mock_it.get_attachment.call_count, 5)
+    self.assertEqual(mock_submit_testcase.call_count, 5)
+
   @mock.patch.object(
       external_testcase_reader,
       'close_issue_if_not_reproducible',
@@ -88,7 +108,7 @@ def test_handle_testcases_not_reproducible(
     mock_it = mock.create_autospec(issue_tracker.IssueTracker)
     basic_issue = mock.MagicMock()
     basic_issue.reporter.return_value = '[email protected]'
-    mock_it.find_issues_with_filters.return_value = [basic_issue]
+    mock_it.find_issues_with_filters.side_effect = [[basic_issue], []]
 
     external_testcase_reader.handle_testcases(mock_it, BASIC_CONFIG)