Avoid duplicated content-type headers#2962
Conversation
Adds tests for Manticore, depends on changes in transport: elastic/elastic-transport-ruby#115 Addresses #2961
picandocodigo
force-pushed
the
content_type
branch
from
5256354 to
0616633
Compare
picandocodigo
left a comment
picandocodigo
left a comment
There was a problem hiding this comment.
The tests for JRuby will pass after we release elastic/elastic-transport-ruby#115
mashhurs
left a comment
mashhurs
left a comment
There was a problem hiding this comment.
Main logics LGTM!
Left a suggestion to fix the unit test. Another out of scope suggestion, it looks like we can create a shared test and use for each duplicate header check specs. It is up to you to make it now or in the future.
| 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 } |
| expect(connection_headers['content-type']).to be nil | ||
| expect(connection_headers.fetch('Content-Type')).to eq 'application/text' |
There was a problem hiding this comment.
It looks like this should be reverse since we are setting content-type
| 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' |
There was a problem hiding this comment.
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.
2 participants
Adds tests for Manticore, depends on changes in transport: elastic/elastic-transport-ruby#115
Addresses #2961