Merge pull request #4607 from reitermarkus/merge-cask-download-strategies

Use normal download strategies for casks.

Library/Homebrew/cask/lib/hbc/download.rb

@@ -22,13 +22,9 @@ module Hbc
     attr_accessor :downloaded_path
 
     def downloader
-      @downloader ||= case cask.url.using
-      when :svn
-        SubversionDownloadStrategy.new(cask)
-      when :post
-        CurlPostDownloadStrategy.new(cask)
-      else
-        CurlDownloadStrategy.new(cask)
+      @downloader ||= begin
+        strategy = DownloadStrategyDetector.detect(cask.url.to_s, cask.url.using)
+        strategy.new(cask.url.to_s, cask.token, cask.version, cache: Cache.path, **cask.url.specs)
       end
     end
 
@@ -37,7 +33,8 @@ module Hbc
     end
 
     def fetch
-      self.downloaded_path = downloader.fetch
+      downloader.fetch
+      @downloaded_path = downloader.cached_location
     rescue StandardError => e
       raise CaskError, "Download failed on Cask '#{cask}' with message: #{e}"
     end

Library/Homebrew/cask/lib/hbc/url.rb

@@ -6,13 +6,13 @@ class URL
     :data
   ].freeze
 
-  attr_reader :uri
+  attr_reader :uri, :specs
   attr_reader(*ATTRIBUTES)
 
   extend Forwardable
   def_delegators :uri, :path, :scheme, :to_s
 
-  def initialize(uri, options = {})
+  def initialize(uri, **options)
     @uri        = URI(uri)
     @user_agent = :default
 
@@ -20,5 +20,7 @@ class URL
       next unless options.key?(attribute)
       instance_variable_set("@#{attribute}", options[attribute])
     end
+
+    @specs = options
   end
 end

Library/Homebrew/test/cask/cli/fetch_spec.rb

@@ -13,34 +13,44 @@ describe Hbc::CLI::Fetch, :cask do
   it_behaves_like "a command that requires a Cask token"
   it_behaves_like "a command that handles invalid options"
 
-  it "allows download the installer of a Cask" do
+  it "allows downloading the installer of a Cask" do
+    transmission_location = CurlDownloadStrategy.new(
+      local_transmission.url.to_s, local_transmission.token, local_transmission.version,
+      cache: Hbc::Cache.path, **local_transmission.url.specs
+    ).cached_location
+    caffeine_location = CurlDownloadStrategy.new(
+      local_caffeine.url.to_s, local_caffeine.token, local_caffeine.version,
+      cache: Hbc::Cache.path, **local_caffeine.url.specs
+    ).cached_location
+
+    expect(transmission_location).not_to exist
+    expect(caffeine_location).not_to exist
+
     described_class.run("local-transmission", "local-caffeine")
-    expect(Hbc::CurlDownloadStrategy.new(local_transmission).cached_location).to exist
-    expect(Hbc::CurlDownloadStrategy.new(local_caffeine).cached_location).to exist
+
+    expect(transmission_location).to exist
+    expect(caffeine_location).to exist
   end
 
   it "prevents double fetch (without nuking existing installation)" do
-    download_stategy = Hbc::CurlDownloadStrategy.new(local_transmission)
+    cached_location = Hbc::Download.new(local_transmission).perform
 
-    Hbc::Download.new(local_transmission).perform
-    old_ctime = File.stat(download_stategy.cached_location).ctime
+    old_ctime = File.stat(cached_location).ctime
 
     described_class.run("local-transmission")
-    new_ctime = File.stat(download_stategy.cached_location).ctime
+    new_ctime = File.stat(cached_location).ctime
 
     expect(old_ctime.to_i).to eq(new_ctime.to_i)
   end
 
   it "allows double fetch with --force" do
-    Hbc::Download.new(local_transmission).perform
+    cached_location = Hbc::Download.new(local_transmission).perform
 
-    download_stategy = Hbc::CurlDownloadStrategy.new(local_transmission)
-    old_ctime = File.stat(download_stategy.cached_location).ctime
+    old_ctime = File.stat(cached_location).ctime
     sleep(1)
 
     described_class.run("local-transmission", "--force")
-    download_stategy = Hbc::CurlDownloadStrategy.new(local_transmission)
-    new_ctime = File.stat(download_stategy.cached_location).ctime
+    new_ctime = File.stat(cached_location).ctime
 
     expect(new_ctime.to_i).to be > old_ctime.to_i
   end