Merge pull request #16234 from samford/livecheck/xml-add-element_text…

…-method

Xml: Add #element_text method

Library/Homebrew/livecheck/strategy/sparkle.rb

@@ -95,16 +95,16 @@ def self.items_from_content(content)
  os = enclosure["os"].presence
  end
 
- title = item.elements["title"]&.text&.strip&.presence
- link = item.elements["link"]&.text&.strip&.presence
+ title = Xml.element_text(item, "title")
+ link = Xml.element_text(item, "link")
  url ||= link
- channel = item.elements["channel"]&.text&.strip&.presence
- release_notes_link = item.elements["releaseNotesLink"]&.text&.strip&.presence
- short_version ||= item.elements["shortVersionString"]&.text&.strip&.presence
- version ||= item.elements["version"]&.text&.strip&.presence
+ channel = Xml.element_text(item, "channel")
+ release_notes_link = Xml.element_text(item, "releaseNotesLink")
+ short_version ||= Xml.element_text(item, "shortVersionString")
+ version ||= Xml.element_text(item, "version")
 
  minimum_system_version_text =
- item.elements["minimumSystemVersion"]&.text&.strip&.gsub(/\A\D+|\D+\z/, "")&.presence
+ Xml.element_text(item, "minimumSystemVersion")&.gsub(/\A\D+|\D+\z/, "")
  if minimum_system_version_text.present?
  minimum_system_version = begin
  MacOSVersion.new(minimum_system_version_text)
@@ -113,7 +113,7 @@ def self.items_from_content(content)
  end
  end
 
- pub_date = item.elements["pubDate"]&.text&.strip&.presence&.then do |date_string|
+ pub_date = Xml.element_text(item, "pubDate")&.then do |date_string|
  Time.parse(date_string)
  rescue ArgumentError
  # Omit unparsable strings (e.g. non-English dates)

Library/Homebrew/livecheck/strategy/xml.rb

@@ -72,6 +72,30 @@ def self.parse_xml(content)
  end
  end
 
+ # Retrieves the stripped inner text of an `REXML` element. Returns
+ # `nil` if the optional child element doesn't exist or the text is
+ # blank.
+ # @param element [REXML::Element] an `REXML` element to retrieve text
+ # from, either directly or from a child element
+ # @param child_path [String, nil] the XPath of a child element to
+ # retrieve text from
+ # @return [String, nil]
+ sig {
+ params(
+ element: REXML::Element,
+ child_path: T.nilable(String),
+ ).returns(T.nilable(String))
+ }
+ def self.element_text(element, child_path = nil)
+ element = element.get_elements(child_path).first if child_path.present?
+ return if element.nil?
+
+ text = element.text
+ return if text.blank?
+
+ text.strip
+ end
+
  # Parses XML text and identifies versions using a `strategy` block.
  # If a regex is provided, it will be passed as the second argument to
  # the `strategy` block (after the parsed XML data).

Library/Homebrew/test/livecheck/strategy/xml_spec.rb

@@ -79,6 +79,24 @@
  EOS
  end
 
+ let(:parent_child_text) { { parent: "1.2.3", child: "4.5.6" } }
+ let(:content_parent_child) do
+ # This XML deliberately includes unnecessary whitespace, to ensure that
+ # Xml#element_text properly strips the retrieved text.
+ <<~EOS
+ <?xml version="1.0" encoding="utf-8"?>
+ <elements>
+ <parent>
+ #{parent_child_text[:parent]}
+ <child> #{parent_child_text[:child]} </child>
+ </parent>
+ <blank-parent>
+ <blank-child></blank-child>
+ </blank-parent>
+ </elements>
+ EOS
+ end
+
  let(:content_matches) { ["1.1.2", "1.1.1", "1.1.0", "1.0.3", "1.0.2", "1.0.1", "1.0.0"] }
  let(:content_simple_matches) { ["1.2.3"] }
 
@@ -123,6 +141,29 @@
  end
  end
 
+ describe "::element_text" do
+ let(:parent_child_doc) { xml.parse_xml(content_parent_child) }
+ let(:parent) { parent_child_doc.get_elements("/elements/parent").first }
+ let(:blank_parent) { parent_child_doc.get_elements("/elements/blank-parent").first }
+
+ it "returns the element text if child_name is not provided" do
+ expect(xml.element_text(parent)).to eq(parent_child_text[:parent])
+ end
+
+ it "returns the child element text if child_name is provided" do
+ expect(xml.element_text(parent, "child")).to eq(parent_child_text[:child])
+ end
+
+ it "returns `nil` if the provided child element does not exist" do
+ expect(xml.element_text(parent, "nonexistent")).to be_nil
+ end
+
+ it "returns `nil` if the retrieved text is blank" do
+ expect(xml.element_text(blank_parent)).to be_nil
+ expect(xml.element_text(blank_parent, "blank-child")).to be_nil
+ end
+ end
+
  describe "::versions_from_content" do
  it "returns an empty array when given a block but content is blank" do
  expect(xml.versions_from_content("", regex) { "1.2.3" }).to eq([])
@@ -142,11 +183,11 @@
 
  # Returning an array of strings from block
  expect(xml.versions_from_content(content_version_text, regex) do |xml, regex|
- xml.get_elements("versions//version").map { |item| item.text[regex, 1] }
+ xml.get_elements("/versions/version").map { |item| item.text[regex, 1] }
  end).to eq(content_matches)
 
  expect(xml.versions_from_content(content_version_attr, regex) do |xml, regex|
- xml.get_elements("items//item").map do |item|
+ xml.get_elements("/items/item").map do |item|
  version = item["version"]
  next if version.blank?
 
@@ -173,7 +214,7 @@
  describe "::find_versions?" do
  it "finds versions in provided_content using a block" do
  expect(xml.find_versions(url: http_url, regex: regex, provided_content: content_version_text) do |xml, regex|
- xml.get_elements("versions//version").map { |item| item.text[regex, 1] }
+ xml.get_elements("/versions/version").map { |item| item.text[regex, 1] }
  end).to eq(find_versions_cached_return_hash)
 
  # NOTE: A regex should be provided using the `#regex` method in a
@@ -182,7 +223,7 @@
  # regex isn't provided.
  expect(xml.find_versions(url: http_url, provided_content: content_version_text) do |xml|
  regex = /^v?(\d+(?:\.\d+)+)$/i.freeze
- xml.get_elements("versions//version").map { |item| item.text[regex, 1] }
+ xml.get_elements("/versions/version").map { |item| item.text[regex, 1] }
  end).to eq(find_versions_cached_return_hash.merge({ regex: nil }))
  end