Refactor download strategy detection

Signed-off-by: Jack Nagel <jacknagel@gmail.com>

Library/Homebrew/cmd/create.rb

@@ -88,7 +88,7 @@ class FormulaCreator
     end
 
     unless ARGV.include? "--no-fetch" and version
-      strategy = detect_download_strategy url
+      strategy = DownloadStrategyDetector.new(url).detect
       @sha1 = strategy.new(url, name, version, nil).fetch.sha1 if strategy == CurlDownloadStrategy
     end
 

Library/Homebrew/download_strategy.rb

@@ -562,27 +562,61 @@ class FossilDownloadStrategy < AbstractDownloadStrategy
   end
 end
 
-def detect_download_strategy url
-  case url
-    # We use a special URL pattern for cvs
-  when %r[^cvs://] then CVSDownloadStrategy
-    # Standard URLs
-  when %r[^bzr://] then BazaarDownloadStrategy
-  when %r[^git://] then GitDownloadStrategy
-  when %r[^https?://.+\.git$] then GitDownloadStrategy
-  when %r[^hg://] then MercurialDownloadStrategy
-  when %r[^svn://] then SubversionDownloadStrategy
-  when %r[^svn\+http://] then SubversionDownloadStrategy
-  when %r[^fossil://] then FossilDownloadStrategy
-    # Some well-known source hosts
-  when %r[^https?://(.+?\.)?googlecode\.com/hg] then MercurialDownloadStrategy
-  when %r[^https?://(.+?\.)?googlecode\.com/svn] then SubversionDownloadStrategy
-  when %r[^https?://(.+?\.)?sourceforge\.net/svnroot/] then SubversionDownloadStrategy
-  when %r[^http://svn.apache.org/repos/] then SubversionDownloadStrategy
-  when %r[^http://www.apache.org/dyn/closer.cgi] then CurlApacheMirrorDownloadStrategy
-    # Common URL patterns
-  when %r[^https?://svn\.] then SubversionDownloadStrategy
-    # Otherwise just try to download
-  else CurlDownloadStrategy
+class DownloadStrategyDetector
+  def initialize url, strategy=nil
+    @url = url
+    @strategy = strategy
+  end
+
+  def detect
+    case @strategy
+    when AbstractDownloadStrategy then @strategy
+    when Symbol then detect_from_symbol
+    else detect_from_url
+    end
+  end
+
+  private
+
+  def detect_from_url
+    case @url
+      # We use a special URL pattern for cvs
+    when %r[^cvs://] then CVSDownloadStrategy
+      # Standard URLs
+    when %r[^bzr://] then BazaarDownloadStrategy
+    when %r[^git://] then GitDownloadStrategy
+    when %r[^https?://.+\.git$] then GitDownloadStrategy
+    when %r[^hg://] then MercurialDownloadStrategy
+    when %r[^svn://] then SubversionDownloadStrategy
+    when %r[^svn\+http://] then SubversionDownloadStrategy
+    when %r[^fossil://] then FossilDownloadStrategy
+      # Some well-known source hosts
+    when %r[^https?://(.+?\.)?googlecode\.com/hg] then MercurialDownloadStrategy
+    when %r[^https?://(.+?\.)?googlecode\.com/svn] then SubversionDownloadStrategy
+    when %r[^https?://(.+?\.)?sourceforge\.net/svnroot/] then SubversionDownloadStrategy
+    when %r[^http://svn.apache.org/repos/] then SubversionDownloadStrategy
+    when %r[^http://www.apache.org/dyn/closer.cgi] then CurlApacheMirrorDownloadStrategy
+      # Common URL patterns
+    when %r[^https?://svn\.] then SubversionDownloadStrategy
+    when bottle_native_regex, bottle_regex, old_bottle_regex
+      CurlBottleDownloadStrategy
+      # Otherwise just try to download
+    else CurlDownloadStrategy
+    end
+  end
+
+  def detect_from_symbol
+    case @strategy
+    when :bzr then BazaarDownloadStrategy
+    when :curl then CurlDownloadStrategy
+    when :cvs then CVSDownloadStrategy
+    when :git then GitDownloadStrategy
+    when :hg then MercurialDownloadStrategy
+    when :nounzip then NoUnzipCurlDownloadStrategy
+    when :post then CurlPostDownloadStrategy
+    when :svn then SubversionDownloadStrategy
+    else
+      raise "Unknown download strategy #{@strategy} was requested."
+    end
   end
 end

Library/Homebrew/formula_support.rb

@@ -2,35 +2,15 @@ require 'download_strategy'
 require 'checksums'
 
 class SoftwareSpec
-  attr_reader :checksum, :mirrors, :specs, :strategy
-
-  VCS_SYMBOLS = {
-    :bzr     => BazaarDownloadStrategy,
-    :curl    => CurlDownloadStrategy,
-    :cvs     => CVSDownloadStrategy,
-    :git     => GitDownloadStrategy,
-    :hg      => MercurialDownloadStrategy,
-    :nounzip => NoUnzipCurlDownloadStrategy,
-    :post    => CurlPostDownloadStrategy,
-    :svn     => SubversionDownloadStrategy,
-  }
+  attr_reader :checksum, :mirrors, :specs
 
   # Was the version defined in the DSL, or detected from the URL?
   def explicit_version?
     @explicit_version || false
   end
 
-  # Returns a suitable DownloadStrategy class that can be
-  # used to retrieve this software package.
   def download_strategy
-    return detect_download_strategy(@url) if @strategy.nil?
-
-    # If a class is passed, assume it is a download strategy
-    return @strategy if @strategy.kind_of? Class
-
-    detected = VCS_SYMBOLS[@strategy]
-    raise "Unknown strategy #{@strategy} was requested." unless detected
-    return detected
+    @download_strategy ||= DownloadStrategyDetector.new(@url, @using).detect
   end
 
   def verify_download_integrity fn
@@ -64,9 +44,9 @@ class SoftwareSpec
     return @url if val.nil?
     @url = val
     if specs.nil?
-      @strategy = nil
+      @using = nil
     else
-      @strategy = specs.delete :using
+      @using = specs.delete :using
       @specs = specs
     end
   end
@@ -104,7 +84,6 @@ class Bottle < SoftwareSpec
   def initialize
     super
     @revision = 0
-    @strategy = CurlBottleDownloadStrategy
   end
 
   # Checksum methods in the DSL's bottle block optionally take