Skip to content

fix(osvmatcher): dedupe bulk queries#2808

Merged
another-rex merged 5 commits into
google:mainfrom
rohan-patnaik:fix/dedupe-bulk-queries
May 28, 2026
Merged

fix(osvmatcher): dedupe bulk queries#2808
another-rex merged 5 commits into
google:mainfrom
rohan-patnaik:fix/dedupe-bulk-queries

Conversation

@rohan-patnaik

@rohan-patnaik rohan-patnaik commented May 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Overview

This fixes duplicate package queries being sent in the same OSV bulk request.

Fixes #2654

Details

When the scan input contains the same package more than once, OSVMatcher currently sends that same package/version query multiple times to OSV. That does not change the final result, but it does add unnecessary API work and makes larger scans do more work than needed.

This PR deduplicates equivalent OSV queries before calling BatchQueryPaging, then expands the hydrated results back to the original package order. That keeps the scanner output behavior the same while reducing duplicate requests.

I also updated the cached matcher query collection to use the same query key, so it does not rely on pointer identity when collecting missing package queries.

Testing

  • Added a regression test that passes duplicate package inputs and verifies only unique queries are sent while all original result slots are preserved.
  • Ran go test ./internal/clients/clientimpl/osvmatcher.
  • Ran go test ./internal/clients/clientimpl/....

Checklist

  • I have signed the Contributor License Agreement.
  • I have run the linter using ./scripts/run_lints.sh.
  • I have run the unit tests using go test ./internal/clients/clientimpl/....
  • I have made my commits and PR title follow the Conventional Commits specification.

@rohan-patnaik
rohan-patnaik force-pushed the fix/dedupe-bulk-queries branch from 98690df to fdfc494 Compare May 22, 2026 09:10
@rohan-patnaik

rohan-patnaik commented May 22, 2026

Copy link
Copy Markdown
Contributor Author

Rebased this on current main and refreshed the affected VCR cassettes/snapshots after resolving the cassette conflicts.\n\nFocused checks run locally:\n- go test ./internal/clients/clientimpl/osvmatcher -run 'TestOSVMatcher_MatchVulnerabilitiesDeduplicatesBulkQueries|TestOSVMatcher_MatchVulnerabilities' -count=1\n- TEST_VCR_MODE=ReplayOnly go test ./cmd/osv-scanner/scan/source -run '^TestCommand$' -count=1

@another-rex
Merge remote-tracking branch 'upstream/main' into fix/dedupe-bulk-que…
62826d1 
…ries

# Conflicts:
#	cmd/osv-scanner/scan/image/testdata/cassettes/TestCommand_OCIImage.yaml
#	cmd/osv-scanner/scan/image/testdata/cassettes/TestCommand_OCIImage_JSONFormat.yaml
#	cmd/osv-scanner/scan/source/__snapshots__/command_test.snap
#	cmd/osv-scanner/scan/source/testdata/cassettes/TestCommand.yaml
#	cmd/osv-scanner/scan/source/testdata/cassettes/TestCommand_Config_UnusedIgnores.yaml
another-rex
another-rex approved these changes May 27, 2026
View reviewed changes

@another-rex another-rex left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM. Going to fix the merge issues and we should be good to go.

@another-rex
another-rex enabled auto-merge (squash) May 27, 2026 04:36
@codecov-commenter

codecov-commenter commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.48718% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.20%. Comparing base (8550b3d) to head (b347a62).

Files with missing lines Patch % Lines
.../clients/clientimpl/osvmatcher/cachedosvmatcher.go 0.00% 6 Missing ⚠️
...ternal/clients/clientimpl/osvmatcher/osvmatcher.go 93.93% 1 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2808      +/-   ##
==========================================
+ Coverage   79.16%   79.20%   +0.03%     
==========================================
  Files         122      122              
  Lines        8222     8250      +28     
==========================================
+ Hits         6509     6534      +25     
- Misses       1329     1332       +3     
  Partials      384      384

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

auto-merge was automatically disabled May 27, 2026 05:17

Head branch was pushed to by a user without write access

@rohan-patnaik

rohan-patnaik commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

Pushed a small follow-up commit to fix the nlreturn lint failure in osvmatcher.go.

Local checks run on macOS arm64 with Go 1.26.3:

  • go test ./internal/clients/clientimpl/osvmatcher -count=1
  • ./scripts/run_lints.sh
  • TEST_ACCEPTANCE=true go test ./cmd/osv-scanner/scan/source -run 'TestCommand/(fix_non-interactive_json_override_pom.xml|fix_non-interactive_override_pom.xml|fix_non-interactive_in-place_package-lock.json|fix_non-interactive_json_in-place_package-lock.json|fix_non-interactive_json_relax_package.json|fix_non-interactive_relax_package.json)$' -count=1
  • TEST_ACCEPTANCE=true go test ./cmd/osv-scanner/update -run 'TestCommand/update_pom_with_in_place_changes_using_deps_dev_data_source$' -count=1

The exact subtests that failed in the macOS CI log pass locally for me. The CI failure still looks like a VCR/deps.dev cassette mismatch after the cassette refresh, e.g. rpc error: code = Unavailable desc = service unavailable followed by VCR: requested interaction not found in cmd/osv-scanner/scan/source, plus the update snapshot mismatch. If this needs maintainer-side cassette regeneration, please let me know.

@another-rex
another-rex merged commit 2ac093a into google:main May 28, 2026
21 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Jun 18, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@another-rex another-rex another-rex approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bulkquery requests don't deduplicate packages

3 participants

@rohan-patnaik @codecov-commenter @another-rex