Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions elasticsearch/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,6 @@ if ENV['TRANSPORT_VERSION'] == 'main'
end

gem 'opentelemetry-sdk', require: false if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3.0')
if defined?(JRUBY_VERSION)
gem 'manticore', platform: :jruby
end
4 changes: 2 additions & 2 deletions elasticsearch/lib/elasticsearch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -189,10 +189,10 @@ def set_user_agent!(arguments)
def set_content_type!(arguments)
headers = {}
user_headers = arguments&.[](:transport_options)&.[](:headers)
unless user_headers&.keys&.detect { |h| h =~ /content-?_?type/ }
unless user_headers&.keys&.detect { |h| h =~ /content-?_?type/i }
headers['content-type'] = 'application/vnd.elasticsearch+json; compatible-with=9'
end
unless user_headers&.keys&.detect { |h| h =~ /accept/ }
unless user_headers&.keys&.detect { |h| h =~ /accept/i }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

headers['accept'] = 'application/vnd.elasticsearch+json; compatible-with=9'
end
set_header(headers, arguments) unless headers.empty?
Expand Down
89 changes: 82 additions & 7 deletions elasticsearch/spec/unit/headers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,18 @@

expect_any_instance_of(Faraday::Connection)
.to receive(:run_request)
.with(:get, 'http://localhost:9200/_search', nil, connection_headers) { OpenStruct.new(body: '') }
.with(:get, 'http://localhost:9200/_search', nil, connection_headers) { OpenStruct.new(body: '') }
client.search
end
end

context 'when content-type header is changed' do
let!(:client) do
described_class.new(
host: 'http://localhost:9200',
transport_options: { headers: instance_headers }
).tap do |client|
described_class.new(transport_options: { headers: instance_headers }).tap do |client|
client.instance_variable_set('@verified', true)
end
end

let(:instance_headers) do
{ content_type: 'application/json' }
end
Expand All @@ -98,11 +96,88 @@

expect_any_instance_of(Faraday::Connection)
.to receive(:run_request)
.with(:get, 'http://localhost:9200/_search', nil, connection_headers) { OpenStruct.new(body: '') }
.with(:get, 'http://localhost:9200/_search', nil, connection_headers) { OpenStruct.new(body: '') }
client.search
end
end

context 'when Content-Type header is used' do
let(:client) do
described_class.new(transport_options: { headers: instance_headers }).tap do |client|
client.instance_variable_set('@verified', true)
end
end

let(:instance_headers) do
{ 'Content-Type' => 'application/text' }
end

it 'performs the request with the header' do
connection_headers = client.transport.connections.connections.first.connection.headers
expect(connection_headers['Accept']).to eq 'application/vnd.elasticsearch+json; compatible-with=9'
expect(connection_headers['Content-Type']).to eq 'application/text'

expect_any_instance_of(Faraday::Connection)
.to receive(:run_request)
.with(:get, 'http://localhost:9200/_search', nil, connection_headers) { OpenStruct.new(body: '') }
client.search
end
end

if defined?(JRUBY_VERSION)
context 'Using Manticore with JRuby' do
let(:client) do
require 'elastic/transport/transport/http/manticore'
described_class.new(
transport_class: Elastic::Transport::Transport::HTTP::Manticore,
transport_options: { headers: instance_headers }
).tap do |client|
client.instance_variable_set('@verified', true)
end
end

context 'when Content-Type header is used' do
let(:instance_headers) do
{ 'Content-Type' => 'application/text' }
end

it 'does not duplicate content-type the header' do
connection_headers = client.transport.connections.connections.first.connection.instance_variable_get('@options')[:headers]
expect(connection_headers['accept']).to eq 'application/vnd.elasticsearch+json; compatible-with=9'
expect(connection_headers['content-type']).to be nil
expect(connection_headers.fetch('Content-Type')).to eq 'application/text'

expect_any_instance_of(Manticore::Client)
.to receive(:get).with(
'http://localhost:9200/_search',
{ headers: connection_headers }
) { OpenStruct.new(body: '') }
client.search
end

context 'when content-type header is used' do
let(:instance_headers) do
{ 'content-type' => 'application/text' }
end

it 'does not duplicate Content-Type the header' do
connection_headers = client.transport.connections.connections.first.connection.instance_variable_get('@options')[:headers]
expect(connection_headers['accept']).to eq 'application/vnd.elasticsearch+json; compatible-with=9'
expect(connection_headers['content-type']).to be nil
expect(connection_headers.fetch('Content-Type')).to eq 'application/text'
Comment on lines +166 to +167
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this should be reverse since we are setting content-type

Suggested change
expect(connection_headers['content-type']).to be nil
expect(connection_headers.fetch('Content-Type')).to eq 'application/text'
expect(connection_headers['Content-Type']).to be nil
expect(connection_headers.fetch('content-type')).to eq 'application/text'

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is being set at the transport level in manticore.rb. On initialize of the base class, it runs build_connections which calls apply_headers (the code I updated in the other PR).

The code eventually sets headers[CONTENT_TYPE_STRING], and CONTENT_TYPE_STR is 'Content-Type'. So the final key that will be sent to Elasticsearch will be Content-Type, no matter how it's set on the elasticsearch-ruby side. I wrote tests for setting content-type and Content-Type and making sure only Content-Type is used in the request with Manticore. I will also add tests on the transport side.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you for explanation.


expect_any_instance_of(Manticore::Client)
.to receive(:get).with(
'http://localhost:9200/_search',
{ headers: connection_headers }
) { OpenStruct.new(body: '') }
client.search
end
end
end
end
end

context 'when no header is set, uses v9 content-type and accept' do
let!(:client) do
described_class.new(host: 'http://localhost:9200').tap do |client|
Expand All @@ -117,7 +192,7 @@

expect_any_instance_of(Faraday::Connection)
.to receive(:run_request)
.with(:get, 'http://localhost:9200/_search', nil, expected_headers) { OpenStruct.new(body: '') }
.with(:get, 'http://localhost:9200/_search', nil, expected_headers) { OpenStruct.new(body: '') }
client.search
end
end
Expand Down
Loading