Generalize GitHubReleaseDownloadStrategy in order to support archive URL

Library/Homebrew/download_strategy.rb

@@ -532,42 +532,78 @@ class S3DownloadStrategy < CurlDownloadStrategy
   end
 end
 
-# GitHubReleaseDownloadStrategy downloads tarballs from GitHub Release assets.
-# To use it, add ":using => GitHubReleaseDownloadStrategy" to the URL section
-# of your formula. This download strategy uses GitHub access tokens (in the
-# environment variables HOMEBREW_GITHUB_API_TOKEN) to sign the request.
-# This strategy is suitable for corporate use just like S3DownloadStrategy,
-# because it lets you use a private GttHub repository for internal distribution.
-# It works with public one, but in that case simply use CurlDownloadStrategy.
-class GitHubReleaseDownloadStrategy < CurlDownloadStrategy
-  require "utils/formatter"
-  require 'utils/github'
-
+# GitHubPrivateRepositoryDownloadStrategy downloads contents from GitHub
+# Private Repository. To use it, add
+# ":using => GitHubPrivateRepositoryDownloadStrategy" to the URL section of
+# your formula. This download strategy uses GitHub access tokens (in the
+# environment variables HOMEBREW_GITHUB_API_TOKEN) to sign the request.  This
+# strategy is suitable for corporate use just like S3DownloadStrategy, because
+# it lets you use a private GttHub repository for internal distribution.  It
+# works with public one, but in that case simply use CurlDownloadStrategy.
+class GitHubPrivateRepositoryDownloadStrategy < CurlDownloadStrategy
   def initialize(name, resource)
     super
+    set_github_token
+    parse_url_pattern
+  end
 
-    @github_token = ENV["HOMEBREW_GITHUB_API_TOKEN"]
-    raise CurlDownloadStrategyError, "Environmental variable HOMEBREW_GITHUB_API_TOKEN is required." unless @github_token
+  def parse_url_pattern
+    url_pattern = %r|https://github.com/([^/]+)/([^/]+)/(\S+)|
+    unless @url =~ url_pattern
+      raise CurlDownloadStrategyError, "Invalid url pattern for GitHub Repository."
+    end
 
-    url_pattern = %r|https://github.com/(\S+)/(\S+)/releases/download/(\S+)/(\S+)|
-    raise CurlDownloadStrategyError, "Invalid url pattern for GitHub Release." unless @url =~ url_pattern
+    _, @owner, @repo, @filepath = *(@url.match(url_pattern))
+  end
 
-    _, @owner, @repo, @tag, @filename = *(@url.match(url_pattern))
+  def download_url
+    "https://#{@github_token}@github.com/#{@owner}/#{@repo}/#{@filepath}"
   end
 
   def _fetch
-    puts "Download asset_id: #{asset_id}"
-    # HTTP request header `Accept: application/octet-stream` is required.
-    # Without this, the GitHub API will respond with metadata, not binary.
-    curl asset_url, "-C", downloaded_size, "-o", temporary_path, "-H", 'Accept: application/octet-stream'
+    curl download_url, "-C", downloaded_size, "-o", temporary_path
   end
 
   private
 
-  def asset_url
+  def set_github_token
+    @github_token = ENV["HOMEBREW_GITHUB_API_TOKEN"]
+    unless @github_token
+      raise CurlDownloadStrategyError, "Environmental variable HOMEBREW_GITHUB_API_TOKEN is required."
+    end
+  end
+end
+
+# GitHubPrivateRepositoryReleaseDownloadStrategy downloads tarballs from GitHub
+# Release assets. To use it, add
+# ":using => GitHubPrivateRepositoryReleaseDownloadStrategy" to the URL section
+# of your formula. This download strategy uses GitHub access tokens (in the
+# environment variables HOMEBREW_GITHUB_API_TOKEN) to sign the request.
+class GitHubPrivateRepositoryReleaseDownloadStrategy < GitHubPrivateRepositoryDownloadStrategy
+  require "utils/formatter"
+  require 'utils/github'
+
+  def parse_url_pattern
+    url_pattern = %r|https://github.com/([^/]+)/([^/]+)/releases/download/([^/]+)/(\S+)|
+    unless @url =~ url_pattern
+      raise CurlDownloadStrategyError, "Invalid url pattern for GitHub Release."
+    end
+
+    _, @owner, @repo, @tag, @filename = *(@url.match(url_pattern))
+  end
+
+  def download_url
     "https://#{@github_token}@api.github.com/repos/#{@owner}/#{@repo}/releases/assets/#{asset_id}"
   end
 
+  def _fetch
+    # HTTP request header `Accept: application/octet-stream` is required.
+    # Without this, the GitHub API will respond with metadata, not binary.
+    curl download_url, "-C", downloaded_size, "-o", temporary_path, "-H", 'Accept: application/octet-stream'
+  end
+
+  private
+
   def asset_id
     @asset_id ||= resolve_asset_id
   end
@@ -580,11 +616,8 @@ class GitHubReleaseDownloadStrategy < CurlDownloadStrategy
     return assets.first["id"]
   end
 
-  def release_url
-    "https://api.github.com/repos/#{@owner}/#{@repo}/releases/tags/#{@tag}"
-  end
-
   def fetch_release_metadata
+    release_url = "https://api.github.com/repos/#{@owner}/#{@repo}/releases/tags/#{@tag}"
     GitHub.open(release_url)
   end
 end

Library/Homebrew/test/download_strategies_test.rb

@@ -61,25 +61,47 @@ class VCSDownloadStrategyTests < Homebrew::TestCase
   end
 end
 
-class GitHubReleaseDownloadStrategyTests < Homebrew::TestCase
+class GitHubPrivateRepositoryDownloadStrategyTests < Homebrew::TestCase
+  def setup
+    resource = ResourceDouble.new("https://github.com/owner/repo/archive/1.1.5.tar.gz")
+    ENV["HOMEBREW_GITHUB_API_TOKEN"] = "token"
+    @strategy = GitHubPrivateRepositoryDownloadStrategy.new("foo", resource)
+  end
+
+  def test_set_github_token
+    assert_equal "token", @strategy.instance_variable_get(:@github_token)
+  end
+
+  def test_parse_url_pattern
+    assert_equal "owner", @strategy.instance_variable_get(:@owner)
+    assert_equal "repo", @strategy.instance_variable_get(:@repo)
+    assert_equal "archive/1.1.5.tar.gz", @strategy.instance_variable_get(:@filepath)
+  end
+
+  def test_download_url
+    expected = "https://token@github.com/owner/repo/archive/1.1.5.tar.gz"
+    assert_equal expected, @strategy.download_url
+  end
+end
+
+class GitHubPrivateRepositoryReleaseDownloadStrategyTests < Homebrew::TestCase
   def setup
     resource = ResourceDouble.new("https://github.com/owner/repo/releases/download/tag/foo_v0.1.0_darwin_amd64.tar.gz")
     ENV["HOMEBREW_GITHUB_API_TOKEN"] = "token"
-    @strategy = GitHubReleaseDownloadStrategy.new("foo", resource)
+    @strategy = GitHubPrivateRepositoryReleaseDownloadStrategy.new("foo", resource)
   end
 
-  def test_initialize
-    assert_equal "token", @strategy.instance_variable_get(:@github_token)
+  def test_parse_url_pattern
     assert_equal "owner", @strategy.instance_variable_get(:@owner)
     assert_equal "repo", @strategy.instance_variable_get(:@repo)
     assert_equal "tag", @strategy.instance_variable_get(:@tag)
     assert_equal "foo_v0.1.0_darwin_amd64.tar.gz", @strategy.instance_variable_get(:@filename)
   end
 
-  def test_asset_url
+  def test_download_url
     @strategy.stubs(:resolve_asset_id).returns(456)
     expected = "https://token@api.github.com/repos/owner/repo/releases/assets/456"
-    assert_equal expected, @strategy.send(:asset_url)
+    assert_equal expected, @strategy.download_url
   end
 
   def test_resolve_asset_id
@@ -99,9 +121,14 @@ class GitHubReleaseDownloadStrategyTests < Homebrew::TestCase
     assert_equal 456, @strategy.send(:resolve_asset_id)
   end
 
-  def test_release_url
-    expected = "https://api.github.com/repos/owner/repo/releases/tags/tag"
-    assert_equal expected, @strategy.send(:release_url)
+  def test_fetch_release_metadata
+    expected_release_url = "https://api.github.com/repos/owner/repo/releases/tags/tag"
+    github_mock = MiniTest::Mock.new
+    github_mock.expect :call, {}, [expected_release_url]
+    GitHub.stub :open, github_mock do
+      @strategy.send(:fetch_release_metadata)
+    end
+    github_mock.verify
   end
 end