Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NuGet efficiency #8679

Merged
merged 84 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
84 commits
Select commit Hold shift + click to select a range
a0276bf
Resilience
ryanbrandenburg Dec 28, 2023
1302377
PR cleanup
ryanbrandenburg Dec 29, 2023
8b9a614
Lint rules
ryanbrandenburg Dec 30, 2023
00ccb43
Lint rules
ryanbrandenburg Dec 30, 2023
b470486
Lint rules
ryanbrandenburg Dec 30, 2023
fe651b8
Lint cleanup
ryanbrandenburg Jan 2, 2024
ed8a1f9
Lint cleanup
ryanbrandenburg Jan 2, 2024
b7fd729
Fix nil assignment
ryanbrandenburg Jan 2, 2024
e3576b3
Lint fix
ryanbrandenburg Jan 2, 2024
946df05
Merge branch 'main' of https://github.com/dependabot/dependabot-core …
ryanbrandenburg Jan 2, 2024
df81998
adjust spec
ryanbrandenburg Jan 2, 2024
c130032
Repo details
ryanbrandenburg Jan 2, 2024
c10bc75
Lint
ryanbrandenburg Jan 2, 2024
3c54a4b
Cleanup
ryanbrandenburg Jan 2, 2024
7e6bf5e
Cleanup
ryanbrandenburg Jan 2, 2024
bac5898
Cleanup
ryanbrandenburg Jan 2, 2024
8db360b
Cleanup extra cache items
ryanbrandenburg Jan 3, 2024
665119f
Fix tests
ryanbrandenburg Jan 3, 2024
d4440eb
Merge branch 'main' of https://github.com/dependabot/dependabot-core …
ryanbrandenburg Jan 3, 2024
87c7bc9
Lint
ryanbrandenburg Jan 3, 2024
bd37d3f
Lint fixes
ryanbrandenburg Jan 3, 2024
44e95fc
Fix tests
ryanbrandenburg Jan 3, 2024
bb8b2d1
registration test
ryanbrandenburg Jan 3, 2024
4b00f86
PR feedback
ryanbrandenburg Jan 3, 2024
cb4e47d
registration test
ryanbrandenburg Jan 3, 2024
5ef3f21
Testing
ryanbrandenburg Jan 3, 2024
afaf2d2
Reigstrations
ryanbrandenburg Jan 3, 2024
e6061cc
Expand registration url
ryanbrandenburg Jan 3, 2024
8c6915d
Lint fix
ryanbrandenburg Jan 3, 2024
f3ac081
Fix registration
ryanbrandenburg Jan 3, 2024
228133a
Fix registration
ryanbrandenburg Jan 3, 2024
b13c192
Lint
ryanbrandenburg Jan 3, 2024
d0c8878
Registration fix
ryanbrandenburg Jan 3, 2024
966fc03
fix paged version
ryanbrandenburg Jan 3, 2024
c976527
Fix test
ryanbrandenburg Jan 3, 2024
af5f684
Fix expectations
ryanbrandenburg Jan 3, 2024
24f366d
Fix expectations
ryanbrandenburg Jan 3, 2024
8f9f61c
Fix versions
ryanbrandenburg Jan 3, 2024
0188de5
Fix tests
ryanbrandenburg Jan 3, 2024
78c6635
Fix tests
ryanbrandenburg Jan 3, 2024
540ea34
temp versions
ryanbrandenburg Jan 3, 2024
bc3d689
Fixture cleanup
ryanbrandenburg Jan 4, 2024
f94ada4
Cleanup
ryanbrandenburg Jan 4, 2024
8ec5f33
Tweek results
ryanbrandenburg Jan 4, 2024
c462cb3
Tweet results
ryanbrandenburg Jan 4, 2024
e06a931
Tweet results
ryanbrandenburg Jan 4, 2024
e0e96ce
Update zero-width
ryanbrandenburg Jan 4, 2024
a907cb6
Merge branch 'main' of https://github.com/dependabot/dependabot-core …
ryanbrandenburg Jan 4, 2024
00031d7
Add caching back for file_fetcher
ryanbrandenburg Jan 4, 2024
68c1655
Lint
ryanbrandenburg Jan 4, 2024
b2f2b48
Lint
ryanbrandenburg Jan 4, 2024
7eb8262
Lint
ryanbrandenburg Jan 4, 2024
1ff67a9
Lint
ryanbrandenburg Jan 4, 2024
fe4bc98
Lint
ryanbrandenburg Jan 4, 2024
caae36c
use CacheManager
ryanbrandenburg Jan 4, 2024
06758b5
Merge branch 'main' of https://github.com/dependabot/dependabot-core …
ryanbrandenburg Jan 4, 2024
59ac615
Don't use CacheManager
ryanbrandenburg Jan 4, 2024
c26f674
Merge branch 'main' into dev/rybrande/NugetJsonParse
jakecoffman Jan 5, 2024
f6424ba
Refactor Nuget APIs
ryanbrandenburg Jan 5, 2024
bd970af
Fix requires and lint
ryanbrandenburg Jan 5, 2024
0ae58b5
Fix Sorbet complaints
ryanbrandenburg Jan 5, 2024
243f8cc
Fix Sorbet complaints
ryanbrandenburg Jan 5, 2024
5bc51da
Fix file load issues
ryanbrandenburg Jan 5, 2024
322e490
Merge branch 'main' of https://github.com/dependabot/dependabot-core …
ryanbrandenburg Jan 5, 2024
9c0fe42
Cleanup specs
ryanbrandenburg Jan 5, 2024
18e6e17
Handle 404 requests gracefully
ryanbrandenburg Jan 5, 2024
eff9637
Nil handling
ryanbrandenburg Jan 5, 2024
8be7546
PR feedback
ryanbrandenburg Jan 5, 2024
28319fc
Lint cleanup
ryanbrandenburg Jan 5, 2024
a7608ee
Lint cleanup
ryanbrandenburg Jan 5, 2024
f19e73c
Fix tests
ryanbrandenburg Jan 5, 2024
cb456f2
Guard clause
ryanbrandenburg Jan 5, 2024
4e35f4a
Fix tests
ryanbrandenburg Jan 5, 2024
b63e8ce
Lint length
ryanbrandenburg Jan 5, 2024
10f251b
Non-standard API
ryanbrandenburg Jan 6, 2024
e65660e
No results
ryanbrandenburg Jan 6, 2024
2c4351c
Adjust with-results index.json
ryanbrandenburg Jan 6, 2024
ccb3dcb
Stub next request
ryanbrandenburg Jan 6, 2024
b50a764
Merge branch 'main' of https://github.com/dependabot/dependabot-core …
ryanbrandenburg Jan 8, 2024
80e9898
PR feedback
ryanbrandenburg Jan 9, 2024
d48cf85
Lint cleanup
ryanbrandenburg Jan 9, 2024
f2dc11a
Merge branch 'main' of https://github.com/dependabot/dependabot-core …
ryanbrandenburg Jan 9, 2024
379eedf
Merge branch 'main' into dev/rybrande/NugetJsonParse
bdragon Jan 9, 2024
d28427b
Merge branch 'main' into dev/rybrande/NugetJsonParse
bdragon Jan 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion bin/dry-run.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,15 @@
}
end

unless ENV["LOCAL_AZURE_ACCESS_TOKEN"].to_s.strip.empty?
$options[:credentials] << {
"type" => "nuget_feed",
"host" => "pkgs.dev.azure.com",
"url" => ENV.fetch("LOCAL_AZURE_FEED_URL", nil),
"token" => ":#{ENV.fetch('LOCAL_AZURE_ACCESS_TOKEN', nil)}"
}
end

unless ENV["LOCAL_CONFIG_VARIABLES"].to_s.strip.empty?
# For example:
# "[{\"type\":\"npm_registry\",\"registry\":\
Expand Down Expand Up @@ -387,8 +396,8 @@ def fetch_files(fetcher)
else
puts "=> cloning into #{$repo_contents_path}"
FileUtils.rm_rf($repo_contents_path)
fetcher.clone_repo_contents
end
fetcher.clone_repo_contents
if $options[:commit]
Dir.chdir($repo_contents_path) do
puts "=> checking out commit #{$options[:commit]}"
Expand Down
1 change: 1 addition & 0 deletions nuget/lib/dependabot/nuget/file_fetcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

require "dependabot/file_fetchers"
require "dependabot/file_fetchers/base"
require "dependabot/nuget/cache_manager"
require "set"
require "sorbet-runtime"

Expand Down
11 changes: 5 additions & 6 deletions nuget/lib/dependabot/nuget/file_parser/project_file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -298,25 +298,24 @@ def dependency_has_search_results?(dependency)
def dependency_url_has_matching_result?(dependency_name, dependency_url)
repository_type = dependency_url.fetch(:repository_type)
if repository_type == "v3"
dependency_url_has_matching_result_v3?(dependency_name, dependency_url)
dependency_url_has_matching_result_v3?(dependency_url)
elsif repository_type == "v2"
dependency_url_has_matching_result_v2?(dependency_name, dependency_url)
else
raise "Unknown repository type: #{repository_type}"
end
end

def dependency_url_has_matching_result_v3?(dependency_name, dependency_url)
url = dependency_url.fetch(:search_url)
def dependency_url_has_matching_result_v3?(dependency_url)
url = dependency_url.fetch(:versions_url)
auth_header = dependency_url.fetch(:auth_header)
response = execute_search_for_dependency_url(url, auth_header)
return false unless response.status == 200

body = JSON.parse(response.body)
data = body["data"]
return false unless data.length.positive?
versions = body["versions"]

data.any? { |result| result["id"].casecmp?(dependency_name) }
versions != nil
end

def dependency_url_has_matching_result_v2?(dependency_name, dependency_url)
Expand Down
36 changes: 31 additions & 5 deletions nuget/lib/dependabot/nuget/update_checker/repository_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ def find_dependency_urls
end.compact.uniq
end

# rubocop:disable Metrics/AbcSize
def build_url_for_details(repo_details)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not for this PR, but I think this method would is a good candidate for some cleanup. Doing a bit more than building a url. Would we want to encapsulate parsing the repo metadata response somewhere else?

response = get_repo_metadata(repo_details)
check_repo_response(response, repo_details)
Expand All @@ -62,7 +63,9 @@ def build_url_for_details(repo_details)
body = remove_wrapping_zero_width_chars(response.body)
base_url = base_url_from_v3_metadata(JSON.parse(body))
resolved_base_url = base_url || repo_details.fetch(:url).gsub("/index.json", "-flatcontainer")
search_url = search_url_from_v3_metadata(JSON.parse(body))
parsed_json = JSON.parse(body)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should move this up so we can use it when getting the base_url from the response body.

search_url = search_url_from_v3_metadata(parsed_json)
registration_url = registration_url_from_v3_metadata(parsed_json)

details = {
base_url: resolved_base_url,
Expand All @@ -78,18 +81,30 @@ def build_url_for_details(repo_details)
details[:search_url] =
search_url + "?q=#{dependency.name.downcase}&prerelease=true&semVerLevel=2.0.0"
end

details[:registration_url] = registration_url + dependency.name.downcase if registration_url

details
rescue JSON::ParserError
build_v2_url(response, repo_details)
rescue Excon::Error::Timeout, Excon::Error::Socket
handle_timeout(repo_metadata_url: repo_details.fetch(:url))
end
# rubocop:enable Metrics/AbcSize

def get_repo_metadata(repo_details)
Dependabot::RegistryClient.get(
url: repo_details.fetch(:url),
headers: auth_header_for_token(repo_details.fetch(:token))
)
url = repo_details.fetch(:url)
cache = CacheManager.cache("repo_finder_metadatacache")
if !CacheManager.caching_disabled? && cache[url]
cache[url]
else
result = Dependabot::RegistryClient.get(
url: url,
headers: auth_header_for_token(repo_details.fetch(:token))
)
cache[url] = result
result
end
end

def base_url_from_v3_metadata(metadata)
Expand All @@ -99,6 +114,17 @@ def base_url_from_v3_metadata(metadata)
&.fetch("@id")
end

def registration_url_from_v3_metadata(metadata)
allowed_registration_types = %w(
RegistrationsBaseUrl/3.0.0-beta
RegistrationsBaseUrl
)
ryanbrandenburg marked this conversation as resolved.
Show resolved Hide resolved
metadata
.fetch("resources", [])
.find { |r| allowed_registration_types.find { |s| r.fetch("@type") == s } }
&.fetch("@id")
end

def search_url_from_v3_metadata(metadata)
# allowable values from here: https://learn.microsoft.com/en-us/nuget/api/search-query-service-resource#versioning
allowed_search_types = %w(
Expand Down
63 changes: 62 additions & 1 deletion nuget/lib/dependabot/nuget/update_checker/version_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,10 @@ def fetch_v2_next_link_href(xml_body)
def versions_for_v3_repository(repository_details)
# If we have a search URL that returns results we use it
# (since it will exclude unlisted versions)
if repository_details[:search_url]

if repository_details[:registration_url]
get_nuget_versions_from_registration(repository_details)
elsif repository_details[:search_url]
fetch_versions_from_search_url(repository_details)
# Otherwise, use the versions URL
elsif repository_details[:versions_url]
Expand All @@ -311,6 +314,64 @@ def versions_for_v3_repository(repository_details)
end
end

def get_nuget_versions_from_registration(repository_details)
response = Dependabot::RegistryClient.get(
url: repository_details[:registration_url],
headers: repository_details[:auth_header]
)
return unless response.status == 200

body = remove_wrapping_zero_width_chars(response.body)
pages = JSON.parse(body).fetch("items")
versions = Set.new
pages.each do |page|
items = page["items"]
if items
# inlined entries
items.each do |item|
catalog_entry = item["catalogEntry"]
if catalog_entry["listed"] == true
vers = catalog_entry["version"]
versions << vers
end
end
else
# paged entries
page_url = page["@id"]
page_versions = get_nuget_versions_from_registration_page(repository_details, page_url)
versions.merge(page_versions)
end
end

versions
rescue Excon::Error::Timeout, Excon::Error::Socket
repo_url = repository_details[:repository_url]
raise if repo_url == RepositoryFinder::DEFAULT_REPOSITORY_URL

raise PrivateSourceTimedOut, repo_url
end

def get_nuget_versions_from_registration_page(repository_details, page_url)
response = Dependabot::RegistryClient.get(
url: page_url,
headers: repository_details[:auth_header]
)
return unless response.status == 200

body = remove_wrapping_zero_width_chars(response.body)
items = JSON.parse(body).fetch("items")
versions = Set.new
items.each do |item|
catalog_entry = item.fetch("catalogEntry")
versions << item.fetch("version") if catalog_entry["listed"] == true
end
rescue Excon::Error::Timeout, Excon::Error::Socket
repo_url = repository_details[:repository_url]
raise if repo_url == RepositoryFinder::DEFAULT_REPOSITORY_URL

raise PrivateSourceTimedOut, repo_url
end

def fetch_versions_from_search_url(repository_details)
response = Dependabot::RegistryClient.get(
url: repository_details[:search_url],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,58 @@

module NuGetSearchStubs
def stub_no_search_results(name)
stub_request(:get, "https://azuresearch-usnc.nuget.org/query?prerelease=true&q=#{name}&semVerLevel=2.0.0")
.to_return(status: 200, body: fixture("nuget_responses", "search_no_data.json"))
stub_request(:get, "https://api.nuget.org/v3-flatcontainer/#{name}/index.json")
.to_return(status: 404, body: "")
stub_request(:get, "https://api.nuget.org/v3/registration5-semver1/#{name}/index.json")
.to_return(status: 404, body: "")
end

def stub_registry_v3(name, versions)
registration_json = registration_results(name, versions)
stub_request(:get, "https://api.nuget.org/v3/registration5-semver1/#{name}/index.json")
.to_return(status: 200, body: registration_json)
end

def stub_search_results_with_versions_v3(name, versions)
json = search_results_with_versions_v3(name, versions)
stub_request(:get, "https://azuresearch-usnc.nuget.org/query?prerelease=true&q=#{name}&semVerLevel=2.0.0")
.to_return(status: 200, body: json)
stub_registry_v3(name, versions)
stub_versions_v3(name, versions)
end

def stub_versions_v3(name, versions)
versions_json = version_results_v3(versions)
stub_request(:get, "https://api.nuget.org/v3-flatcontainer/#{name}/index.json")
.to_return(status: 200, body: versions_json)
end

def version_results_v3(versions)
{
"versions" => versions
}.to_json
end

def registration_results(name, versions)
page = {
"@id": "https://api.nuget.org/v3/registration5-semver1/#{name}/index.json#page/PAGE1",
"@type": "catalog:CatalogPage",
"count" => versions.count,
"items" => versions.map do |version|
{
"catalogEntry" => {
"@type": "PackageDetails",
"id" => "#{name}",
"listed" => true,
"version" => version
}
}
end
}
pages = [page]
response = {
"@id": "https://api.nuget.org/v3/registration5-gz-semver1/#{name}/index.json",
"count" => versions.count,
"items" => pages
}
response.to_json
end

def search_results_with_versions_v3(name, versions)
Expand Down Expand Up @@ -189,7 +233,9 @@ def search_results_with_versions_v2(name, versions)
end
let(:parser) { described_class.new(dependency_files: files, credentials: credentials) }
let(:dependencies) { dependency_set.dependencies }
subject(:transitive_dependencies) { dependencies.reject(&:top_level?) }
subject(:transitive_dependencies) do
dependencies.reject(&:top_level?)
end

its(:length) { is_expected.to eq(20) }

Expand Down Expand Up @@ -687,13 +733,18 @@ def dependencies_from(dep_info)
end
end

# This is a bit of a noop now that we're moved off the query Nuget API,
# But we're keeping the test for completeness.
describe "the dependency name is a partial, but not perfect match" do
let(:file_body) do
fixture("csproj", "dependency_with_name_that_does_not_exist.csproj")
end

before do
stub_request(:get, "https://azuresearch-usnc.nuget.org/query?prerelease=true&q=this.dependency.does.not.exist&semVerLevel=2.0.0")
stub_request(:get, "https://api.nuget.org/v3-flatcontainer/this.dependency.does.not.exist/index.json")
.to_return(status: 404, body: "")

stub_request(:get, "https://api.nuget.org/v3-flatcontainer/this.dependency.does.not.exist_but.this.one.does")
.to_return(status: 200, body: search_results_with_versions_v3(
"this.dependency.does.not.exist_but.this.one.does", ["1.0.0"]
))
Expand Down Expand Up @@ -744,19 +795,18 @@ def dependencies_from(dep_info)
stub_request(:get, "https://no-results.api.example.com/v3/index.json")
.to_return(status: 200, body: fixture("nuget_responses", "index.json",
"no-results.api.example.com.index.json"))
stub_request(:get, "https://no-results.api.example.com/query?prerelease=true&q=microsoft.extensions.dependencymodel&semVerLevel=2.0.0")
.to_return(status: 200, body: fixture("nuget_responses", "search_no_data.json"))
stub_request(:get, "https://no-results.api.example.com/query?prerelease=true&q=this.dependency.does.not.exist&semVerLevel=2.0.0")
.to_return(status: 200, body: fixture("nuget_responses", "search_no_data.json"))
stub_request(:get, "https://no-results.api.example.com/v3-flatcontainer/microsoft.extensions.dependencymodel/index.json")
.to_return(status: 404, body: "")
stub_request(:get, "https://no-results.api.example.com/v3-flatcontainer/this.dependency.does.not.exist/index.json")
.to_return(status: 404, body: "")
# with results
stub_request(:get, "https://with-results.api.example.com/v3/index.json")
.to_return(status: 200, body: fixture("nuget_responses", "index.json",
"with-results.api.example.com.index.json"))
stub_request(:get, "https://with-results.api.example.com/query?prerelease=true&q=microsoft.extensions.dependencymodel&semVerLevel=2.0.0")
.to_return(status: 200, body: search_results_with_versions_v3("microsoft.extensions.dependencymodel",
["1.1.1", "1.1.0"]))
stub_request(:get, "https://with-results.api.example.com/query?prerelease=true&q=this.dependency.does.not.exist&semVerLevel=2.0.0")
.to_return(status: 200, body: fixture("nuget_responses", "search_no_data.json"))
stub_request(:get, "https://with-results.api.example.com/v3-flatcontainer/microsoft.extensions.dependencymodel/index.json")
.to_return(status: 200, body: version_results_v3(["1.1.1", "1.1.0"]))
stub_request(:get, "https://with-results.api.example.com/v3-flatcontainer/this.dependency.does.not.exist/index.json")
.to_return(status: 404, body: "")
end

it "has the right details" do
Expand Down Expand Up @@ -870,8 +920,9 @@ def dependencies_from(dep_info)
end

it "has the right details" do
query_stub = stub_search_results_with_versions_v3("microsoft.extensions.dependencymodel_cached",
["1.1.1", "1.1.0"])
versions_stub = stub_versions_v3("microsoft.extensions.dependencymodel_cached", ["1.1.1", "1.1.0"])
stub_registry_v3("microsoft.extensions.dependencymodel_cached", ["1.1.1", "1.1.0"])
ryanbrandenburg marked this conversation as resolved.
Show resolved Hide resolved

expect(top_level_dependencies.count).to eq(1)
expect(top_level_dependencies.first).to be_a(Dependabot::Dependency)
expect(top_level_dependencies.first.name).to eq("Microsoft.Extensions.DependencyModel_cached")
Expand All @@ -884,7 +935,7 @@ def dependencies_from(dep_info)
source: nil
}]
)
expect(WebMock::RequestRegistry.instance.times_executed(query_stub.request_pattern)).to eq(1)
expect(WebMock::RequestRegistry.instance.times_executed(versions_stub.request_pattern)).to eq(1)
end
end
end
Expand Down
Loading
Loading