From 74370fe604d8e3cc72dd98be0d8a91b571bf5c32 Mon Sep 17 00:00:00 2001
From: Sam Ford <1584702+samford@users.noreply.github.com>
Date: Thu, 2 Mar 2023 15:20:33 -0500
Subject: [PATCH] ElectronBuilder: Update to use Yaml#find_versions

This updates `ElectronBuilder` to use `Yaml#find_versions`, as the
only code unique to that strategy is to restrict regex usage and
use default version-finding logic when a `strategy` block isn't
provided. This is similar to how we have various strategies that
use `PageMatch#find_versions` internally.

This allows us to remove `ElectronBuilder#versions_from_content`
entirely, along with the related tests. I've added support for
`provided_content` to `ElectronBuilder#find_versions` as a way of
adding tests and maintaining code coverage.
---
 .../livecheck/strategy/electron_builder.rb    |  58 +++-------
 .../strategy/electron_builder_spec.rb         | 109 +++++++++---------
 2 files changed, 73 insertions(+), 94 deletions(-)

diff --git a/Library/Homebrew/livecheck/strategy/electron_builder.rb b/Library/Homebrew/livecheck/strategy/electron_builder.rb
index 4be6c35f3e..fc4ff4f92c 100644
--- a/Library/Homebrew/livecheck/strategy/electron_builder.rb
+++ b/Library/Homebrew/livecheck/strategy/electron_builder.rb
@@ -32,61 +32,35 @@ module Homebrew
           URL_MATCH_REGEX.match?(url)
         end
 
-        # Parses YAML text and identifies versions in it.
-        #
-        # @param content [String] the YAML text to parse and check
-        # @param regex [Regexp, nil] a regex for use in a strategy block
-        # @return [Array]
-        sig {
-          params(
-            content: String,
-            regex:   T.nilable(Regexp),
-            block:   T.untyped,
-          ).returns(T::Array[String])
-        }
-        def self.versions_from_content(content, regex = nil, &block)
-          require "yaml"
-
-          yaml = YAML.safe_load(content, permitted_classes: [Date, Time])
-          return [] if yaml.blank?
-
-          if block
-            block_return_value = regex.present? ? yield(yaml, regex) : yield(yaml)
-            return Strategy.handle_block_return(block_return_value)
-          end
-
-          version = yaml["version"]
-          version.present? ? [version] : []
-        end
-
         # Checks the YAML content at the URL for new versions.
         #
         # @param url [String] the URL of the content to check
+        # @param regex [Regexp, nil] a regex used for matching versions
+        # @param provided_content [String, nil] content to use in place of
+        #   fetching via `Strategy#page_content`
         # @return [Hash]
         sig {
           params(
-            url:     String,
-            regex:   T.nilable(Regexp),
-            _unused: T.nilable(T::Hash[Symbol, T.untyped]),
-            block:   T.untyped,
+            url:              String,
+            regex:            T.nilable(Regexp),
+            provided_content: T.nilable(String),
+            unused:           T.nilable(T::Hash[Symbol, T.untyped]),
+            block:            T.untyped,
           ).returns(T::Hash[Symbol, T.untyped])
         }
-        def self.find_versions(url:, regex: nil, **_unused, &block)
+        def self.find_versions(url:, regex: nil, provided_content: nil, **unused, &block)
           if regex.present? && block.blank?
             raise ArgumentError,
                   "#{Utils.demodulize(T.must(name))} only supports a regex when using a `strategy` block"
           end
 
-          match_data = { matches: {}, regex: regex, url: url }
-
-          match_data.merge!(Strategy.page_content(url))
-          content = match_data.delete(:content)
-
-          versions_from_content(content, regex, &block).each do |version_text|
-            match_data[:matches][version_text] = Version.new(version_text)
-          end
-
-          match_data
+          T.unsafe(Yaml).find_versions(
+            url:              url,
+            regex:            regex,
+            provided_content: provided_content,
+            **unused,
+            &block || proc { |yaml| yaml["version"] }
+          )
         end
       end
     end
diff --git a/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb b/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb
index cd27271b69..e82ee5270e 100644
--- a/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb
+++ b/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb
@@ -6,10 +6,12 @@ require "livecheck/strategy"
 describe Homebrew::Livecheck::Strategy::ElectronBuilder do
   subject(:electron_builder) { described_class }
 
-  let(:yaml_url) { "https://www.example.com/example/latest-mac.yml" }
-  let(:non_yaml_url) { "https://brew.sh/test" }
+  let(:http_url) { "https://www.example.com/example/latest-mac.yml" }
+  let(:non_http_url) { "ftp://brew.sh/" }
 
-  let(:electron_builder_yaml) {
+  let(:regex) { /Example[._-]v?(\d+(?:\.\d+)+)[._-]mac\.zip/i }
+
+  let(:content) {
     <<~EOS
       version: 1.2.3
       files:
@@ -26,77 +28,80 @@ describe Homebrew::Livecheck::Strategy::ElectronBuilder do
     EOS
   }
 
-  let(:electron_builder_yaml_with_timestamp) {
+  let(:content_timestamp) {
     # An electron-builder YAML file may use a timestamp instead of an explicit
     # string value (with quotes) for `releaseDate`, so we need to make sure that
     # `ElectronBuilder#versions_from_content` won't encounter an error in this
     # scenario (e.g. `Tried to load unspecified class: Time`).
-    electron_builder_yaml.sub(/releaseDate:\s*'([^']+)'/, 'releaseDate: \1')
+    content.sub(/releaseDate:\s*'([^']+)'/, 'releaseDate: \1')
   }
 
-  let(:mac_regex) { /Example[._-]v?(\d+(?:\.\d+)+)[._-]mac\.zip/i }
+  let(:content_matches) { ["1.2.3"] }
 
-  let(:versions) { ["1.2.3"] }
+  let(:find_versions_return_hash) {
+    {
+      matches: {
+        "1.2.3" => Version.new("1.2.3"),
+      },
+      regex:   nil,
+      url:     http_url,
+    }
+  }
+
+  let(:find_versions_cached_return_hash) {
+    find_versions_return_hash.merge({ cached: true })
+  }
 
   describe "::match?" do
     it "returns true for a YAML file URL" do
-      expect(electron_builder.match?(yaml_url)).to be true
+      expect(electron_builder.match?(http_url)).to be true
     end
 
     it "returns false for non-YAML URL" do
-      expect(electron_builder.match?(non_yaml_url)).to be false
+      expect(electron_builder.match?(non_http_url)).to be false
     end
   end
 
-  describe "::versions_from_content" do
-    it "returns an empty array if content is blank" do
-      expect(electron_builder.versions_from_content("")).to eq([])
+  describe "::find_versions?" do
+    it "finds versions in provided_content using a block" do
+      expect(electron_builder.find_versions(url: http_url, provided_content: content))
+        .to eq(find_versions_cached_return_hash)
+
+      expect(electron_builder.find_versions(url: http_url, regex: regex, provided_content: content) do |yaml, regex|
+        yaml["path"][regex, 1]
+      end).to eq(find_versions_cached_return_hash.merge({ regex: regex }))
+
+      expect(electron_builder.find_versions(
+        url:              http_url,
+        regex:            regex,
+        provided_content: content_timestamp,
+      ) do |yaml, regex|
+        yaml["path"][regex, 1]
+      end).to eq(find_versions_cached_return_hash.merge({ regex: regex }))
+
+      # NOTE: A regex should be provided using the `#regex` method in a
+      # `livecheck` block but we're using a regex literal in the `strategy`
+      # block here simply to ensure this method works as expected when a
+      # regex isn't provided.
+      expect(electron_builder.find_versions(url: http_url, provided_content: content) do |yaml|
+        regex = /^v?(\d+(?:\.\d+)+)$/i.freeze
+        yaml["version"][regex, 1]
+      end).to eq(find_versions_cached_return_hash)
     end
 
-    it "returns an array of version strings when given YAML text" do
-      expect(electron_builder.versions_from_content(electron_builder_yaml)).to eq(versions)
-      expect(electron_builder.versions_from_content(electron_builder_yaml_with_timestamp)).to eq(versions)
+    it "errors if a block is not provided" do
+      expect { electron_builder.find_versions(url: http_url, regex: regex, provided_content: content) }
+        .to raise_error(ArgumentError, "ElectronBuilder only supports a regex when using a `strategy` block")
     end
 
-    it "returns an array of version strings when given YAML text and a block" do
-      # Returning a string from block
-      expect(
-        electron_builder.versions_from_content(electron_builder_yaml) do |yaml|
-          yaml["version"].sub("3", "4")
-        end,
-      ).to eq(["1.2.4"])
-
-      # Returning an array of strings from block
-      expect(electron_builder.versions_from_content(electron_builder_yaml) { versions }).to eq(versions)
+    it "returns default match_data when url is blank" do
+      expect(electron_builder.find_versions(url: "") { "1.2.3" })
+        .to eq({ matches: {}, regex: nil, url: "" })
     end
 
-    it "returns an array of version strings when given YAML text, a regex, and a block" do
-      # Returning a string from block
-      expect(
-        electron_builder.versions_from_content(electron_builder_yaml, mac_regex) do |yaml, regex|
-          yaml["path"][regex, 1]
-        end,
-      ).to eq(versions)
-
-      # Returning an array of strings from block
-      expect(
-        electron_builder.versions_from_content(electron_builder_yaml, mac_regex) do |yaml, regex|
-          yaml["files"]&.map do |file|
-            next if file["url"].blank?
-
-            file["url"][regex, 1]
-          end
-        end,
-      ).to eq(versions)
-    end
-
-    it "allows a nil return from a block" do
-      expect(electron_builder.versions_from_content(electron_builder_yaml) { next }).to eq([])
-    end
-
-    it "errors on an invalid return type from a block" do
-      expect { electron_builder.versions_from_content(electron_builder_yaml) { 123 } }
-        .to raise_error(TypeError, Homebrew::Livecheck::Strategy::INVALID_BLOCK_RETURN_VALUE_MSG)
+    it "returns default match_data when content is blank" do
+      expect(electron_builder.find_versions(url: http_url, provided_content: "") { "1.2.3" })
+        .to eq({ matches: {}, regex: nil, url: http_url, cached: true })
     end
   end
 end