[CLIENT] Added correct Accept headers to the Faraday and Curb adapters

karmi · karmi · commit c22ec891b0be · 2016-07-27T11:01:03.000+02:00
Also fixed failing tests on master, where a request header `Content-Type` has
been ignored when requesting a YAML formatted response.
diff --git a/elasticsearch-transport/lib/elasticsearch/transport/transport/http/curb.rb b/elasticsearch-transport/lib/elasticsearch/transport/transport/http/curb.rb
@@ -26,14 +26,17 @@ def perform_request(method, path, params={}, body=nil)
                 else raise ArgumentError, "Unsupported HTTP method: #{method}"
               end
 
+              request_headers = connection.connection.headers
+              connection.connection.headers = request_headers.merge('Accept' => 'application/json') unless request_headers['Accept']
+
               connection.connection.http(method.to_sym)
 
-              headers = {}
-              headers['content-type'] = 'application/json' if connection.connection.header_str =~ /\/json/
+              response_headers = {}
+              response_headers['content-type'] = 'application/json' if connection.connection.header_str =~ /\/json/
 
               Response.new connection.connection.response_code,
                            connection.connection.body_str,
-                           headers
+                           response_headers
             end
           end
 
diff --git a/elasticsearch-transport/lib/elasticsearch/transport/transport/http/faraday.rb b/elasticsearch-transport/lib/elasticsearch/transport/transport/http/faraday.rb
@@ -18,11 +18,16 @@ class Faraday
           #
           def perform_request(method, path, params={}, body=nil)
             super do |connection, url|
+              headers = connection.connection.headers
+              headers['Content-Type'] ||= 'application/json'
+              headers['Accept']       ||= 'application/json'
+
               response = connection.connection.run_request \
                 method.downcase.to_sym,
                 url,
                 ( body ? __convert_to_json(body) : nil ),
-                { 'Content-Type' => 'application/json' }
+                headers
+
               Response.new response.status, response.body, response.headers
             end
           end
diff --git a/elasticsearch-transport/test/integration/client_test.rb b/elasticsearch-transport/test/integration/client_test.rb
@@ -57,23 +57,26 @@ class Elasticsearch::Transport::ClientIntegrationTest < Elasticsearch::Test::Int
       @client = Elasticsearch::Client.new \
         host: "localhost:#{@port}",
         logger: (ENV['QUIET'] ? nil : @logger),
-        transport_options: { headers: { content_type: 'application/yaml' } }
+        transport_options: { headers: { accept: 'application/yaml', content_type: 'application/yaml' } }
 
       response = @client.perform_request 'GET', '_cluster/health'
-      assert_match /---\ncluster_name:/, response.body.to_s
+
+      assert response.body.to_s.start_with?("---\n"), "Response body should be YAML: #{response.body.inspect}"
+      assert_equal 'application/yaml', response.headers['content-type']
     end
 
     should "pass options to the Faraday::Connection with a block" do
       @client = Elasticsearch::Client.new(
         host: "localhost:#{@port}",
         logger: (ENV['QUIET'] ? nil : @logger)
       ) do |client|
-        client.headers['Content-Type'] = 'application/yaml'
+        client.headers['Content-Type'] = 'application/yaml' # For ES 2.x
+        client.headers['Accept'] = 'application/yaml'       # For ES 5.x
       end
 
       response = @client.perform_request 'GET', '_cluster/health'
 
-      assert response.body.start_with?("---\n"), "Response body should be YAML: #{response.body.inspect}"
+      assert response.body.to_s.start_with?("---\n"), "Response body should be YAML: #{response.body.inspect}"
       assert_equal 'application/yaml', response.headers['content-type']
     end
 
diff --git a/elasticsearch-transport/test/unit/transport_curb_test.rb b/elasticsearch-transport/test/unit/transport_curb_test.rb
@@ -55,7 +55,7 @@ class Elasticsearch::Transport::Transport::HTTP::FaradayTest < Test::Unit::TestC
         @transport.perform_request 'POST', '/', {}, '{"foo":"bar"}'
       end
 
-      should "set application/json header" do
+      should "set application/json response header" do
         @transport.connections.first.connection.expects(:http).with(:GET).returns(stub_everything)
         @transport.connections.first.connection.expects(:body_str).returns('{"foo":"bar"}')
         @transport.connections.first.connection.expects(:header_str).returns('HTTP/1.1 200 OK\r\nContent-Type: application/json; charset=UTF-8\r\nContent-Length: 311\r\n\r\n')
@@ -65,6 +65,19 @@ class Elasticsearch::Transport::Transport::HTTP::FaradayTest < Test::Unit::TestC
         assert_equal 'application/json', response.headers['content-type']
       end
 
+      should "set application/json request header" do
+        @transport.connections.first.connection.expects(:http).with(:GET).returns(stub_everything)
+        @transport.connections.first.connection.expects(:body_str).returns(stub_everything)
+        @transport.connections.first.connection.expects(:header_str).returns(stub_everything)
+
+        @transport.connections.first.connection.expects(:headers=).with do |headers|
+          assert_equal 'application/json', headers['Accept']
+          true
+        end
+
+        response = @transport.perform_request 'GET', '/'
+      end
+
       should "handle HTTP methods" do
         @transport.connections.first.connection.expects(:http).with(:HEAD).returns(stub_everything)
         @transport.connections.first.connection.expects(:http).with(:GET).returns(stub_everything)
diff --git a/elasticsearch-transport/test/unit/transport_faraday_test.rb b/elasticsearch-transport/test/unit/transport_faraday_test.rb
@@ -36,6 +36,7 @@ class Elasticsearch::Transport::Transport::HTTP::FaradayTest < Test::Unit::TestC
         assert_equal :post, method
         assert_equal '{"foo":"bar"}', body
         assert_equal 'application/json', headers['Content-Type']
+        assert_equal 'application/json', headers['Accept']
         true
       end.returns(stub_everything)
       @transport.perform_request 'POST', '/', {}, {:foo => 'bar'}
