Browser code factoring

lib/common/browser.rb

@@ -169,6 +169,12 @@ class Browser
   end
 
   def merge_request_params(params = {})
+    params = Browser.append_params_header_field(
+      params,
+      'User-Agent',
+      self.user_agent
+    )
+
     if @proxy
       params = params.merge(proxy: @proxy)
 
@@ -178,25 +184,11 @@ class Browser
     end
 
     if @basic_auth
-      if !params.has_key?(:headers)
+      params = Browser.append_params_header_field(
-        params = params.merge(:headers => {'Authorization' => @basic_auth})
+        params,
-      elsif !params[:headers].has_key?('Authorization')
+        'Authorization',
-        params[:headers]['Authorization'] = @basic_auth
+        @basic_auth
-      end
+      )
-    end
-
-    #unless params.has_key?(:ssl_verifyhost)
-    #  params = params.merge(ssl_verifyhost: 0)
-    #end
-
-    #unless params.has_key?(:ssl_verifypeer)
-    #  params = params.merge(ssl_verifypeer: 0)
-    #end
-
-    if !params.has_key?(:headers)
-      params = params.merge(:headers => {'User-Agent' => self.user_agent})
-    elsif !params[:headers].has_key?('User-Agent')
-      params[:headers]['User-Agent'] = self.user_agent
     end
 
     # Used to enable the cache system if :cache_ttl > 0
@@ -209,6 +201,16 @@ class Browser
 
   private
 
+  # return Array
+  def self.append_params_header_field(params = {}, field, field_value)
+    if !params.has_key?(:headers)
+      params = params.merge(:headers => { field => field_value })
+    elsif !params[:headers].has_key?(field)
+      params[:headers][field] = field_value
+    end
+    params
+  end
+
   # return the response
   def run_request(request)
     @hydra.queue request

spec/lib/common/browser_spec.rb

@@ -236,94 +236,98 @@ describe Browser do
     end
   end
 
-  describe '#merge_request_params without proxy' do
+  describe '#append_params_header_field' do
-    it 'should return the default params' do
+    after :each do
-      expected_params = {
+      Browser.append_params_header_field(
-        #disable_ssl_host_verification: true,
+        @params,
-        #disable_ssl_peer_verification: true,
+        @field,
-        headers: { 'User-Agent' => @browser.user_agent },
+        @field_value
-        cache_ttl: @json_config_without_proxy['cache_ttl']
+      ).should === @expected
-      }
-
-      @browser.merge_request_params().should == expected_params
     end
 
-    it 'should return the default params with some values overriden' do
+    context 'when there is no headers' do
-      expected_params = {
+      it 'create the header and set the field' do
-        #disable_ssl_host_verification: false,
+        @params      = { somekey: 'somevalue' }
-        #disable_ssl_peer_verification: true,
+        @field       = 'User-Agent'
-        headers: { 'User-Agent' => 'Fake IE' },
+        @field_value = 'FakeOne'
-        cache_ttl: 0
+        @expected    = { somekey: 'somevalue', headers: { 'User-Agent' => 'FakeOne' } }
-      }
+      end
-
-      @browser.merge_request_params(
-        #disable_ssl_host_verification: false,
-        headers: { 'User-Agent' => 'Fake IE' },
-        cache_ttl: 0
-      ).should == expected_params
     end
 
-    it 'should return the defaul params with :headers:accept = \'text/html\' (should not override :headers:User-Agent)' do
+    context 'when there are headers' do
-      expected_params = {
+      context 'when the field already exists' do
-        #disable_ssl_host_verification: true,
+        it 'does not replace it' do
-        #disable_ssl_peer_verification: true,
+          @params      = { somekey: 'somevalue', headers: { 'Location' => 'SomeLocation' } }
-        headers: { 'User-Agent' => @browser.user_agent, 'accept' => 'text/html' },
+          @field       = 'Location'
-        cache_ttl: @json_config_without_proxy['cache_ttl']
+          @field_value = 'AnotherLocation'
-      }
+          @expected    = @params
+        end
+      end
 
-      @browser.merge_request_params(headers: { 'accept' => 'text/html' }).should == expected_params
+      context 'when the field is not present' do
+        it 'sets the field' do
+          @params      = { somekey: 'somevalue', headers: { 'Auth' => 'user:pass' } }
+          @field       = 'UA'
+          @field_value = 'FF'
+          @expected    = { somekey: 'somevalue', headers: { 'Auth' => 'user:pass', 'UA' => 'FF' } }
+        end
+      end
     end
 
-    it 'should merge the basic-auth' do
-      @browser.basic_auth = 'Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ=='
-      expected_params = {
-        #disable_ssl_host_verification: true,
-        #disable_ssl_peer_verification: true,
-        cache_ttl: @json_config_without_proxy['cache_ttl'],
-        headers: {
-          'Authorization' => 'Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==',
-          'User-Agent'    => @browser.user_agent
-        }
-      }
-
-      @browser.merge_request_params().should == expected_params
-
-      expected_params[:headers].merge!('User-Agent' => 'Fake FF')
-      @browser.merge_request_params(headers: { 'User-Agent' => 'Fake FF' }).should == expected_params
-    end
   end
 
-  describe '#merge_request_params with proxy' do
+  describe '#merge_request_params' do
-    it 'should return the default params' do
+    let(:params) { {} }
-      Browser.reset
+    let(:default_expectation) { { cache_ttl: 250, headers: { 'User-Agent' => 'SomeUA' } } }
-      browser = Browser.instance(config_file: CONFIG_FILE_WITH_PROXY)
 
-      expected_params = {
+    after :each do
-        proxy: @json_config_with_proxy['proxy'],
+      @browser.stub(user_agent: 'SomeUA')
-        #disable_ssl_host_verification: true,
+      @browser.cache_ttl = 250
-        #disable_ssl_peer_verification: true,
-        headers: { 'User-Agent' => @json_config_with_proxy['user_agent'] },
-        cache_ttl: @json_config_with_proxy['cache_ttl']
-      }
 
-      browser.merge_request_params().should == expected_params
+      @browser.merge_request_params(params).should == @expected
     end
 
-    it 'should return the default params (proxy_auth set)' do
+    it 'sets the User-Agent header field and cache_ttl' do
-      Browser.reset
+      @expected = default_expectation
-      browser = Browser.instance(config_file: CONFIG_FILE_WITH_PROXY_AND_AUTH)
-
-      expected_params = {
-        proxy: @json_config_with_proxy['proxy'],
-        proxyauth: 'user:pass',
-        #disable_ssl_host_verification: true,
-        #disable_ssl_peer_verification: true,
-        headers: { 'User-Agent' => @json_config_with_proxy['user_agent'] },
-        cache_ttl: @json_config_with_proxy['cache_ttl']
-      }
-
-      browser.merge_request_params().should == expected_params
     end
+
+
+    context 'when @proxy' do
+      let(:proxy) { '127.0.0.1:9050' }
+      let(:proxy_expectation) { default_expectation.merge(proxy: proxy) }
+
+      it 'merges the proxy' do
+        @browser.proxy = proxy
+        @expected      = proxy_expectation
+      end
+
+      context 'when @proxy_auth' do
+        it 'sets the proxy_auth' do
+          @browser.proxy      = proxy
+          @browser.proxy_auth = 'user:pass'
+          @expected           = proxy_expectation.merge(proxyauth: 'user:pass')
+        end
+      end
+    end
+
+    context 'when @basic_auth' do
+      it 'appends the basic_auth' do
+        @browser.basic_auth = 'basic-auth'
+        @expected           = default_expectation.merge(
+          headers: default_expectation[:headers].merge('Authorization' => 'basic-auth')
+        )
+      end
+
+    end
+
+    context 'when the cache_ttl is alreday set' do
+      let(:params) { { cache_ttl: 500 } }
+
+      it 'does not override it' do
+        @expected = default_expectation.merge(params)
+      end
+    end
+
   end
 
   # TODO