Skip to content

Commit

Permalink
feat(worker): remove stale import findings for successful record (#2945)
Browse files Browse the repository at this point in the history
This adds functionality to the worker to clear any existing import
findings for a record that previously failed to import, but now does.
This avoids confusing historical findings existing for records that are
successfully importing now.

Part of #2189 and #2188
  • Loading branch information
andrewpollock authored Feb 3, 2025
1 parent 6822678 commit eb29faa
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 0 deletions.
9 changes: 9 additions & 0 deletions gcp/workers/worker/worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,7 @@ def _do_update(self, source_repo, repo, vulnerability, relative_path,

osv.update_affected_commits(bug.key.id(), result.commits, bug.public)
self._notify_ecosystem_bridge(vulnerability)
self._maybe_remove_import_findings(bug)

def _notify_ecosystem_bridge(self, vulnerability):
"""Notify ecosystem bridges."""
Expand All @@ -561,6 +562,14 @@ def _notify_ecosystem_bridge(self, vulnerability):
push_topic,
data=json.dumps(osv.vulnerability_to_dict(vulnerability)).encode())

def _maybe_remove_import_findings(self, vulnerability: osv.Bug):
"""Remove any stale import findings for a successfully processed Bug,"""

finding = osv.ImportFinding.get_by_id(vulnerability.id())
if finding:
logging.info('Removing stale import finding for %s', vulnerability.id())
finding.key.delete()

def _do_process_task(self, subscriber, subscription, ack_id, message,
done_event):
"""Process task with timeout."""
Expand Down
22 changes: 22 additions & 0 deletions gcp/workers/worker/worker_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1734,6 +1734,28 @@ def test_analysis_crash_handling(self):

self.expect_dict_equal('analysis_crash_handling', bug._to_dict())

def test_update_clears_stale_import_finding(self):
"""A subsequent successful update removes the now stale import finding."""

# Add a pre-existing record import finding.

osv.ImportFinding(
bug_id='OSV-123',
source='source',
findings=[osv.ImportFindings.INVALID_JSON],
first_seen=osv.utcnow(),
last_attempt=osv.utcnow()).put()

# Simulate a successful record update.

self.test_update()

# Check the pre-existing finding is no longer present.

self.assertIsNone(
osv.ImportFinding.get_by_id('OSV-123'),
'Stale import finding still present after successful record processing')


if __name__ == '__main__':
ds_emulator = tests.start_datastore_emulator()
Expand Down

0 comments on commit eb29faa

Please sign in to comment.