Skip to content

[Maven] Fix some malformed memoizations #5132

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

Merged
brrygrdn merged 2 commits into from
May 13, 2022
Merged

[Maven] Fix some malformed memoizations #5132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f97a204
Readability pass on memoization
brrygrdn May 11, 2022
3a53e44
Ensure we memoize 'false' values
brrygrdn May 11, 2022
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
3 changes: 2 additions & 1 deletion maven/lib/dependabot/maven/update_checker/property_updater.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ def initialize(dependency:, dependency_files:, credentials:,

def update_possible?
return false unless target_version
return @update_possible if defined?(@update_possible)

@update_possible ||=
@update_possible =
dependencies_using_property.all? do |dep|
next false if includes_property_reference?(updated_version(dep))

Expand Down
40 changes: 22 additions & 18 deletions maven/lib/dependabot/maven/update_checker/version_finder.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def initialize(dependency:, dependency_files:, credentials:,
@raise_on_ignored = raise_on_ignored
@security_advisories = security_advisories
@forbidden_urls = []
@dependency_metadata = {}
end

def latest_version_details
Expand Down Expand Up @@ -154,25 +155,28 @@ def released?(version)
end

def dependency_metadata(repository_details)
@dependency_metadata ||= {}
@dependency_metadata[repository_details.hash] ||=
Comment on lines 157 to 158
Copy link
Copy Markdown
Contributor Author

@brrygrdn brrygrdn May 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This memoization pattern works, but I found it hard to read so this is an opinionated change just to improve the readability.

It does make it a little easier to maintain in the event that the fetch_dependency_metadata code was ever to return a falsy value rather than a blank string.

begin
response = Excon.get(
dependency_metadata_url(repository_details.fetch("url")),
idempotent: true,
**Dependabot::SharedHelpers.excon_defaults(headers: repository_details.fetch("auth_headers"))
)
check_response(response, repository_details.fetch("url"))
repository_key = repository_details.hash
return @dependency_metadata[repository_key] if @dependency_metadata.key?(repository_key)

Nokogiri::XML(response.body)
rescue URI::InvalidURIError
Nokogiri::XML("")
rescue Excon::Error::Socket, Excon::Error::Timeout,
Excon::Error::TooManyRedirects
raise if central_repo_urls.include?(repository_details["url"])
@dependency_metadata[repository_key] = fetch_dependency_metadata(repository_details)
end

Nokogiri::XML("")
end
def fetch_dependency_metadata(repository_details)
response = Excon.get(
dependency_metadata_url(repository_details.fetch("url")),
idempotent: true,
**Dependabot::SharedHelpers.excon_defaults(headers: repository_details.fetch("auth_headers"))
)
check_response(response, repository_details.fetch("url"))

Nokogiri::XML(response.body)
rescue URI::InvalidURIError
Nokogiri::XML("")
rescue Excon::Error::Socket, Excon::Error::Timeout,
Excon::Error::TooManyRedirects
raise if central_repo_urls.include?(repository_details["url"])

Nokogiri::XML("")
end

def check_response(response, repository_url)
Expand All @@ -184,7 +188,7 @@ def check_response(response, repository_url)
end

def repositories
return @repositories if @repositories
return @repositories if defined?(@repositories)

details = pom_repository_details + credentials_repository_details

Expand Down