Allow download strategies to preprocess download options

CHANGELOG.md

@@ -10,7 +10,12 @@
   [Sylvain Guillopé](https://github.com/sguillope)
   [#55](https://github.com/CocoaPods/cocoapods-downloader/issues/55)
   [CocoaPods#5318](https://github.com/CocoaPods/CocoaPods/issues/5318)
-
+
+* Allow download strategies to preprocess download options. This is used by
+  `git` strategy to resolve branches into commits directly.  
+  [Juan Civile](https://github.com/champo)
+  [CocoaPods#5386](https://github.com/CocoaPods/CocoaPods/pull/5386)
+
 
 ##### Bug Fixes
 

README.markdown

@@ -19,6 +19,7 @@ require 'cocoapods-downloader'
 
 target_path = './Downloads/MyDownload'
 options = { :git => 'example.com' }
+options = Pod::Downloader.preprocess_options(options)
 downloader = Pod::Downloader.for_target(target_path, options)
 downloader.cache_root = '~/Library/Caches/APPNAME'
 downloader.max_cache_size = 500

lib/cocoapods-downloader.rb

@@ -47,12 +47,42 @@ def self.strategy_from_options(options)
     #         options.
     #
     def self.for_target(target_path, options)
-      options = Hash[options.map { |k, v| [k.to_sym, v] }]
+      options = options_to_sym(options)
 
       if target_path.nil?
         raise DownloaderError, 'No target path provided.'
       end
 
+      strategy, klass = class_for_options(options)
+
+      url = options[strategy]
+      sub_options = options.dup
+      sub_options.delete(strategy)
+
+      klass.new(target_path, url, sub_options)
+    end
+
+    # Have the concrete strategy preprocess options
+    #
+    # @param [Hash<Symbol,String>] options
+    #        The request options to preprocess
+    #
+    # @return [Hash<Symbol,String>] the new options
+    #
+    def self.preprocess_options(options)
+      options = options_to_sym(options)
+
+      _, klass = class_for_options(options)
+      klass.preprocess_options(options)
+    end
+
+    private_class_method
+
+    def self.options_to_sym(options)
+      Hash[options.map { |k, v| [k.to_sym, v] }]
+    end
+
+    def self.class_for_options(options)
       if options.nil? || options.empty?
         raise DownloaderError, 'No source URL provided.'
       end
@@ -63,11 +93,8 @@ def self.for_target(target_path, options)
           "`#{options.inspect}`."
       end
 
-      url = options[strategy]
-      sub_options = options.dup
-      sub_options.delete(strategy)
-      klass = downloader_class_by_key[strategy]
-      klass.new(target_path, url, sub_options)
+      # Explicit return for multiple params, rubocop thinks it's useless but it's not
+      return strategy, downloader_class_by_key[strategy] # rubocop:disable Style/RedundantReturn
     end
   end
 end

lib/cocoapods-downloader/api_exposable.rb

@@ -12,6 +12,9 @@ def expose_api(mod = nil, &block)
           raise 'Only a module *or* is required, not both.'
         end
         include mod
+        # TODO: Try to find a nicer way to do this
+        # See https://github.com/CocoaPods/cocoapods-downloader/pull/57
+        extend mod
       end
 
       alias override_api expose_api

lib/cocoapods-downloader/base.rb

@@ -154,6 +154,20 @@ def self.executable(name)
           execute_command(name.to_s, command.flatten, true)
         end
       end
+
+      # preprocess download options
+      #
+      # Usage of this method is optional. concrete strategies should not
+      # assume options are preprocessed for correct execution.
+      #
+      # @param  [Hash<Symbol,String>] options
+      #         The request options to preprocess
+      #
+      # @return [Hash<Symbol,String>] the new options
+      #
+      def self.preprocess_options(options)
+        options
+      end
     end
   end
 end

lib/cocoapods-downloader/git.rb

@@ -22,6 +22,23 @@ def checkout_options
         end
       end
 
+      def self.preprocess_options(options)
+        return options unless options[:branch]
+
+        command = ['ls-remote',
+                   options[:git],
+                   options[:branch]]
+        output = Git.execute_command('git', command)
+        match = /^([a-z0-9]*)\t.*/.match(output)
+
+        return options if match.nil?
+
+        options[:commit] = match[1]
+        options.delete(:branch)
+
+        options
+      end
+
       private
 
       # @!group Base class hooks

spec/base_spec.rb

@@ -8,6 +8,12 @@ module Downloader
         e = lambda { Base.new('path', 'url', options) }.should.raise DownloaderError
         e.message.should.match /Unrecognized options/
       end
+
+      it 'has no preprocessing' do
+        options = { :symbol => 'aaaaaa' }
+        new_options = Base.preprocess_options(options)
+        new_options.should == options
+      end
     end
   end
 end

spec/bazaar_spec.rb

@@ -77,6 +77,12 @@ module Downloader
         downloader = Downloader.for_target(tmp_folder, options)
         lambda { downloader.download }.should.raise DownloaderError
       end
+
+      it 'has no preprocessing' do
+        options = { :bzr => fixture('bazaar-repo'), :revision => '1' }
+        new_options = Downloader.preprocess_options(options)
+        new_options.should == options
+      end
     end
   end
 end

spec/git_spec.rb

@@ -203,6 +203,26 @@ def ensure_only_one_ref(folder)
           should.not.raise { downloader.download }
         end
       end
+
+      describe '::preprocess_options' do
+        it 'skips non-branch requests' do
+          options = { :git => fixture_url('git-repo'), :commit => 'aaaa' }
+          new_options = Downloader.preprocess_options(options)
+          options.should == new_options
+        end
+
+        it 'resolves master to a commit' do
+          options = { :git => fixture_url('git-repo'), :branch => 'master' }
+          new_options = Downloader.preprocess_options(options)
+          new_options[:commit].should == '98cbf14201a78b56c6b7290f6cac840a7597a1c2'
+        end
+
+        it 'ignores invalid branches' do
+          options = { :git => fixture_url('git-repo'), :branch => 'aaaa' }
+          new_options = Downloader.preprocess_options(options)
+          new_options[:branch].should == 'aaaa'
+        end
+      end
     end
   end
 end

spec/http_spec.rb

@@ -219,6 +219,12 @@ module Downloader
           downloader.send(:type).should == :zip
         end
       end
+
+      it 'has no preprocessing' do
+        options = { :http => 'https://host/file', :type => 'zip' }
+        new_options = Downloader.preprocess_options(options)
+        new_options.should == options
+      end
     end
   end
 end

spec/mercurial_spec.rb

@@ -99,6 +99,12 @@ module Downloader
           :revision => '61118fa8988c2b2eae826f48abd1e3340dae0c6b',
         }
       end
+
+      it 'has no preprocessing' do
+        options = { :hg => fixture('mercurial-repo'), :tag => '1.0.0' }
+        new_options = Downloader.preprocess_options(options)
+        new_options.should == options
+      end
     end
   end
 end

spec/subversion_spec.rb

@@ -104,6 +104,16 @@ module Downloader
         tmp_folder('README').read.strip.should == 'first commit'
         tmp_folder('.svn').should.exist
       end
+
+      it 'has no preprocessing' do
+        options = {
+          :svn => "file://#{fixture('subversion-repo')}",
+          :revision => '1',
+          :checkout => true,
+        }
+        new_options = Downloader.preprocess_options(options)
+        new_options.should == options
+      end
     end
   end
 end