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.

Sign up for GitHub

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

Centralize more Nuget calls #8849

Merged
merged 9 commits into from
Jan 22, 2024
Merged

Centralize more Nuget calls #8849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0d3561a
Clean up Nuget usage
ryanbrandenburg Jan 19, 2024
bff2111
Fix imported methods
ryanbrandenburg Jan 19, 2024
c9aff84
Cleanup local runs and fix tests
ryanbrandenburg Jan 19, 2024
9bd19d7
Update sorbet
ryanbrandenburg Jan 19, 2024
8a0e79c
Cleanup
ryanbrandenburg Jan 19, 2024
cd49e30
Merge branch 'main' into dev/rybrande/NugetJsonParse
abdulapopoola Jan 22, 2024
dcaf77a
Merge branch 'main' into dev/rybrande/NugetJsonParse
abdulapopoola Jan 22, 2024
881b20f
Merge branch 'main' into dev/rybrande/NugetJsonParse
abdulapopoola Jan 22, 2024
9af741d
Merge branch 'main' into dev/rybrande/NugetJsonParse
abdulapopoola Jan 22, 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
5 changes: 5 additions & 0 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,17 @@ gem "tapioca", "0.11.14", require: false, group: :development
common_gemspec = File.expand_path("common/dependabot-common.gemspec", __dir__)

deps_shared_with_common = %w(
debug
gpgme
rake
rspec-its
rspec-sorbet
rubocop
rubocop-performance
rubocop-sorbet
stackprof
turbo_tests
vcr
Comment on lines +32 to +42
Copy link
Member

Choose a reason for hiding this comment

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

It's not quite clear to me why these changes are needed, I would expect these to already be pulled in via the gemspec?

Copy link
Contributor

Choose a reason for hiding this comment

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

They are unfortunately not. Running bundle exec turbo_tests in any directory currently gives

bundler: failed to load command: turbo_tests (/home/jamie/.asdf/installs/ruby/3.1.4/bin/turbo_tests)
/home/jamie/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/bundler-2.5.3/lib/bundler/rubygems_integration.rb:265:in `block in replace_bin_path': can't find executable turbo_tests for gem turbo_tests. turbo_tests is not currently included in the bundle, perhaps you meant to add it to your Gemfile? (Gem::Exception)

I think it might actually be easier to get rid of deps_shared_with_common and just load the same development dependencies below

Dir.chdir(File.dirname(common_gemspec)) do
  Gem::Specification.load(common_gemspec).development_dependencies.each do |dep|
-     next unless deps_shared_with_common.include?(dep.name)
-
    gem dep.name, *dep.requirement.as_list
  end
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I believe this is a bug in Bundler. If that's the case, upgrading to 2.5.5 should fix it!

webmock
webrick
)
Expand Down
41 changes: 41 additions & 0 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ GEM
commonmarker (0.23.10)
crack (0.4.5)
rexml
debug (1.8.0)
irb (>= 1.5.0)
reline (>= 0.3.1)
diff-lcs (1.5.0)
docker_registry2 (1.18.0)
rest-client (>= 1.8.0)
domain_name (0.6.20231109)
Expand All @@ -181,6 +185,10 @@ GEM
httparty (0.21.0)
mini_mime (>= 1.0.0)
multi_xml (>= 0.5.2)
io-console (0.7.2)
irb (1.11.1)
rdoc
reline (>= 0.4.2)
jmespath (1.6.2)
json (2.6.3)
language_server-protocol (3.17.0.3)
Expand Down Expand Up @@ -213,6 +221,8 @@ GEM
opentelemetry-semantic_conventions (1.10.0)
opentelemetry-api (~> 1.0)
parallel (1.24.0)
parallel_tests (4.4.0)
parallel
parseconfig (1.0.8)
parser (3.3.0.1)
ast (~> 2.4.1)
Expand All @@ -228,13 +238,35 @@ GEM
rbi (0.1.6)
prism (>= 0.18.0, < 0.20)
sorbet-runtime (>= 0.5.9204)
rdoc (6.6.2)
psych (>= 4.0.0)
regexp_parser (2.8.3)
reline (0.4.2)
io-console (~> 0.5)
rest-client (2.1.0)
http-accept (>= 1.7.0, < 2.0)
http-cookie (>= 1.0.2, < 2.0)
mime-types (>= 1.16, < 4.0)
netrc (~> 0.8)
rexml (3.2.6)
rspec (3.12.0)
rspec-core (~> 3.12.0)
rspec-expectations (~> 3.12.0)
rspec-mocks (~> 3.12.0)
rspec-core (3.12.2)
rspec-support (~> 3.12.0)
rspec-expectations (3.12.3)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.12.0)
rspec-its (1.3.0)
rspec-core (>= 3.0.0)
rspec-expectations (>= 3.0.0)
rspec-mocks (3.12.6)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.12.0)
rspec-sorbet (1.9.2)
sorbet-runtime
rspec-support (3.12.1)
rubocop (1.58.0)
json (~> 2.3)
language_server-protocol (>= 3.17.0)
Expand Down Expand Up @@ -290,7 +322,11 @@ GEM
thor (1.3.0)
toml-rb (2.2.0)
citrus (~> 3.0, > 3.0)
turbo_tests (2.2.0)
parallel_tests (>= 3.3.0, < 5)
rspec (>= 3.10)
unicode-display_width (2.5.0)
vcr (6.2.0)
webmock (3.19.1)
addressable (>= 2.8.0)
crack (>= 0.3.2)
Expand All @@ -308,6 +344,7 @@ PLATFORMS
x86_64-linux

DEPENDENCIES
debug (~> 1.8.0)
dependabot-bundler!
dependabot-cargo!
dependabot-common!
Expand All @@ -329,12 +366,16 @@ DEPENDENCIES
dependabot-terraform!
gpgme (~> 2.0)
rake (~> 13)
rspec-its (~> 1.3)
rspec-sorbet (~> 1.9.2)
rubocop (~> 1.58.0)
rubocop-performance (~> 1.19.0)
rubocop-sorbet (~> 0.7.3)
sorbet (= 0.5.11178)
stackprof (~> 0.2.16)
tapioca (= 0.11.14)
turbo_tests (~> 2.2.0)
vcr (~> 6.1)
webmock (~> 3.18)
webrick (>= 1.7)

Expand Down
43 changes: 2 additions & 41 deletions nuget/lib/dependabot/nuget/file_parser/project_file_parser.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,47 +293,8 @@ def dependency_has_search_results?(dependency)
end

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)
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)
versions = NugetClient.get_package_versions_v3(dependency_name, dependency_url)

versions != nil
end

def dependency_url_has_matching_result_v2?(dependency_name, 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

doc = Nokogiri::XML(response.body)
doc.remove_namespaces!
id_nodes = doc.xpath("/feed/entry/properties/Id")
found_matching_result = id_nodes.any? do |id_node|
return false unless id_node.text

id_node.text.casecmp?(dependency_name)
end
found_matching_result
end

def execute_search_for_dependency_url(url, auth_header)
cache = ProjectFileParser.dependency_url_search_cache
cache[url] ||= Dependabot::RegistryClient.get(
url: url,
headers: auth_header
)

cache[url]
versions = NugetClient.get_package_versions(dependency_name, dependency_url)
versions&.any?
end

def dependency_name(dependency_node, project_file)
Expand Down
85 changes: 74 additions & 11 deletions nuget/lib/dependabot/nuget/nuget_client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,18 @@
module Dependabot
module Nuget
class NugetClient
def self.get_package_versions_v3(dependency_name, repository_details)
def self.get_package_versions(dependency_name, repository_details)
repository_type = repository_details.fetch(:repository_type)
if repository_type == "v3"
get_package_versions_v3(dependency_name, repository_details)
elsif repository_type == "v2"
get_package_versions_v2(dependency_name, repository_details)
else
raise "Unknown repository type: #{repository_type}"
end
end

private_class_method def self.get_package_versions_v3(dependency_name, repository_details)
# Use the registration URL if possible because it is fast and correct
if repository_details[:registration_url]
get_versions_from_registration_v3(repository_details)
Expand All @@ -20,14 +31,32 @@ def self.get_package_versions_v3(dependency_name, repository_details)
end
end

private_class_method def self.get_package_versions_v2(dependency_name, repository_details)
doc = execute_xml_nuget_request(repository_details.fetch(:versions_url), repository_details)
return unless doc

id_nodes = doc.xpath("/feed/entry/properties/Id")
matching_versions = Set.new
id_nodes.each do |id_node|
return nil unless id_node.text

next unless id_node.text.casecmp?(dependency_name)

version_node = id_node.parent.xpath("Version")
matching_versions << version_node.text if version_node && version_node.text
end

matching_versions
end

private_class_method def self.get_versions_from_versions_url_v3(repository_details)
body = execute_search_for_dependency_url(repository_details[:versions_url], repository_details)
body = execute_json_nuget_request(repository_details[:versions_url], repository_details)
body&.fetch("versions")
end

private_class_method def self.get_versions_from_registration_v3(repository_details)
url = repository_details[:registration_url]
body = execute_search_for_dependency_url(url, repository_details)
body = execute_json_nuget_request(url, repository_details)

return unless body

Expand All @@ -47,7 +76,7 @@ def self.get_package_versions_v3(dependency_name, repository_details)
else
# paged entries
page_url = page["@id"]
page_body = execute_search_for_dependency_url(page_url, repository_details)
page_body = execute_json_nuget_request(page_url, repository_details)
items = page_body.fetch("items")
items.each do |item|
catalog_entry = item.fetch("catalogEntry")
Expand All @@ -61,29 +90,63 @@ def self.get_package_versions_v3(dependency_name, repository_details)

private_class_method def self.get_versions_from_search_url_v3(repository_details, dependency_name)
search_url = repository_details[:search_url]
body = execute_search_for_dependency_url(search_url, repository_details)
body = execute_json_nuget_request(search_url, repository_details)

body&.fetch("data")
&.find { |d| d.fetch("id").casecmp(dependency_name.downcase).zero? }
&.fetch("versions")
&.map { |d| d.fetch("version") }
end

private_class_method def self.execute_search_for_dependency_url(url, repository_details)
cache = CacheManager.cache("dependency_url_search_cache")
cache[url] ||= Dependabot::RegistryClient.get(
private_class_method def self.execute_xml_nuget_request(url, repository_details)
response = execute_nuget_request_internal(
url: url,
headers: repository_details[:auth_header]
auth_header: repository_details[:auth_header],
repository_url: repository_details[:repository_url]
)
return unless response.status == 200

response = cache[url]
doc = Nokogiri::XML(response.body)
doc.remove_namespaces!
doc
end

private_class_method def self.execute_json_nuget_request(url, repository_details)
response = execute_nuget_request_internal(
url: url,
auth_header: repository_details[:auth_header],
repository_url: repository_details[:repository_url]
)
return unless response.status == 200

body = remove_wrapping_zero_width_chars(response.body)
JSON.parse(body)
end

private_class_method def self.execute_nuget_request_internal(
url: String,
auth_header: String,
repository_url: String
)
cache = CacheManager.cache("dependency_url_search_cache")
if cache[url].nil?
response = Dependabot::RegistryClient.get(
url: url,
headers: auth_header
)

if [401, 402, 403].include?(response.status)
raise Dependabot::PrivateSourceAuthenticationFailure, repository_url
end

cache[url] = response if !CacheManager.caching_disabled? && response.status == 200
else
response = cache[url]
end

response
rescue Excon::Error::Timeout, Excon::Error::Socket
repo_url = repository_details[:repository_url]
repo_url = repository_url
raise if repo_url == Dependabot::Nuget::UpdateChecker::RepositoryFinder::DEFAULT_REPOSITORY_URL

raise PrivateSourceTimedOut, repo_url
Expand Down
6 changes: 1 addition & 5 deletions nuget/lib/dependabot/nuget/update_checker/version_finder.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ def v3_nuget_listings
dependency_urls
.select { |details| details.fetch(:repository_type) == "v3" }
.filter_map do |url_details|
versions = versions_for_v3_repository(url_details)
versions = NugetClient.get_package_versions(dependency.name, url_details)
next unless versions

{ "versions" => versions, "listing_details" => url_details }
Expand Down Expand Up @@ -294,10 +294,6 @@ def fetch_v2_next_link_href(xml_body)
nil
end

def versions_for_v3_repository(repository_details)
NugetClient.get_package_versions_v3(dependency.name, repository_details)
end

def dependency_urls
@dependency_urls ||=
RepositoryFinder.new(
Expand Down
8 changes: 7 additions & 1 deletion nuget/spec/dependabot/nuget/file_parser/project_file_parser_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require "spec_helper"
require "dependabot/dependency_file"
require "dependabot/source"
require "dependabot/nuget/cache_manager"
require "dependabot/nuget/file_parser/project_file_parser"

module NuGetSearchStubs
Expand Down Expand Up @@ -903,6 +904,7 @@ def dependencies_from(dep_info)

before do
stub_no_search_results("this.dependency.does.not.exist")
ENV["DEPENDABOT_NUGET_CACHE_DISABLED"] = "false"
end

it "has the right details" do
Expand All @@ -927,6 +929,10 @@ def dependencies_from(dep_info)
ENV["DEPENDABOT_NUGET_CACHE_DISABLED"] = "true"
Dependabot::Nuget::CacheManager.instance_variable_set(:@cache, nil)
end

after do
ENV["DEPENDABOT_NUGET_CACHE_DISABLED"] = "true"
end
end
end
end
Expand All @@ -935,7 +941,7 @@ def dependencies_from(dep_info)
let(:file_body) { fixture("csproj", "basic.nuproj") }

before do
stub_search_results_with_versions_v3("nanoframework.coreextra", [])
stub_search_results_with_versions_v3("nanoframework.coreextra", ["1.0.0"])
end

it "gets the right number of dependencies" do
Expand Down
Loading
Loading