Skip to content

Commit eb29faa

Browse files
feat(worker): remove stale import findings for successful record (#2945)
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
1 parent 6822678 commit eb29faa

File tree

2 files changed

+31
-0
lines changed

2 files changed

+31
-0
lines changed

gcp/workers/worker/worker.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,7 @@ def _do_update(self, source_repo, repo, vulnerability, relative_path,
542542

543543
osv.update_affected_commits(bug.key.id(), result.commits, bug.public)
544544
self._notify_ecosystem_bridge(vulnerability)
545+
self._maybe_remove_import_findings(bug)
545546

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

565+
def _maybe_remove_import_findings(self, vulnerability: osv.Bug):
566+
"""Remove any stale import findings for a successfully processed Bug,"""
567+
568+
finding = osv.ImportFinding.get_by_id(vulnerability.id())
569+
if finding:
570+
logging.info('Removing stale import finding for %s', vulnerability.id())
571+
finding.key.delete()
572+
564573
def _do_process_task(self, subscriber, subscription, ack_id, message,
565574
done_event):
566575
"""Process task with timeout."""

gcp/workers/worker/worker_test.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1734,6 +1734,28 @@ def test_analysis_crash_handling(self):
17341734

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

1737+
def test_update_clears_stale_import_finding(self):
1738+
"""A subsequent successful update removes the now stale import finding."""
1739+
1740+
# Add a pre-existing record import finding.
1741+
1742+
osv.ImportFinding(
1743+
bug_id='OSV-123',
1744+
source='source',
1745+
findings=[osv.ImportFindings.INVALID_JSON],
1746+
first_seen=osv.utcnow(),
1747+
last_attempt=osv.utcnow()).put()
1748+
1749+
# Simulate a successful record update.
1750+
1751+
self.test_update()
1752+
1753+
# Check the pre-existing finding is no longer present.
1754+
1755+
self.assertIsNone(
1756+
osv.ImportFinding.get_by_id('OSV-123'),
1757+
'Stale import finding still present after successful record processing')
1758+
17371759

17381760
if __name__ == '__main__':
17391761
ds_emulator = tests.start_datastore_emulator()

0 commit comments

Comments
 (0)