Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(influxdb2/client): Send Content-Type headers #30

Merged
merged 2 commits into from
Mar 6, 2020
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
### Bugs
1. [#22](https://github.com/influxdata/influxdb-client-ruby/pull/22): Fixed batch write
1. [#28](https://github.com/influxdata/influxdb-client-ruby/pull/28): Correctly parse CSV where multiple results include multiple tables
1. [#30](https://github.com/influxdata/influxdb-client-ruby/pull/30): Send Content-Type headers

## 1.1.0 [2020-02-14]

Expand Down
22 changes: 20 additions & 2 deletions lib/influxdb2/client/default_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class DefaultApi
DEFAULT_TIMEOUT = 10
DEFAULT_REDIRECT_COUNT = 10

HEADER_CONTENT_TYPE = 'Content-Type'.freeze

Choose a reason for hiding this comment

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

nice 😎


# @param [Hash] options The options to be used by the client.
def initialize(options:)
@options = options
Expand All @@ -32,7 +34,17 @@ def initialize(options:)

private

def _post(payload, uri, limit = @max_redirect_count)
def _post_json(payload, uri, headers: {})
_check_arg_type(:headers, headers, Hash)
_post(payload, uri, headers: headers.merge({HEADER_CONTENT_TYPE => 'application/json'}))
end

def _post_text(payload, uri, headers: {})
_check_arg_type(:headers, headers, Hash)
_post(payload, uri, headers: headers.merge({HEADER_CONTENT_TYPE => 'text/plain'}))
end

def _post(payload, uri, limit: @max_redirect_count, headers: {})

Choose a reason for hiding this comment

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

what was your rationale behind creating two separate methods, _post_json and _post_text which are identical aside from content-type? keeping the original _post method and exposing a content_type named argument w/ a reasonable default (e.g. application/json) might also be worth considering? (would also provide increased flexibility in allowing _post consumers to provide different content-type headers alongside their payloads if that's ever deemed beneficial)

(i guess, on second thought, not introducing another argument dependency in favor of encapsulating the content-type-setting logic in this class is probably more robust but still curious to hear your thoughts)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The underlying _post method is HTTP-centric so it warranted a higher-level headers concept if we are to be introducing any additional headers. However, this introduced a minor inconvenience at the JSON-based call sites so I just added some helpers.

raise InfluxError.from_message("Too many HTTP redirects. Exceeded limit: #{@max_redirect_count}") if limit.zero?

http = Net::HTTP.new(uri.host, uri.port)
Expand All @@ -44,6 +56,8 @@ def _post(payload, uri, limit = @max_redirect_count)
request = Net::HTTP::Post.new(uri.request_uri)
request['Authorization'] = "Token #{@options[:token]}"
request['User-Agent'] = "influxdb-client-ruby/#{InfluxDB2::VERSION}"
headers.each { |k,v| request[k] = v }

request.body = payload

begin
Expand All @@ -53,7 +67,7 @@ def _post(payload, uri, limit = @max_redirect_count)
response
when Net::HTTPRedirection then
location = response['location']
_post(payload, URI.parse(location), limit - 1)
_post(payload, URI.parse(location), limit: limit - 1, headers: headers)
else
raise InfluxError.from_response(response)
end
Expand All @@ -62,6 +76,10 @@ def _post(payload, uri, limit = @max_redirect_count)
end
end

def _check_arg_type(name, value, klass)
raise TypeError, "expected a #{klass.name} for #{name}; got #{value.class.name}" unless value.is_a?(klass)
end

def _check(key, value)
raise ArgumentError, "The '#{key}' should be defined as argument or default option: #{@options}" if value.nil?
end
Expand Down
2 changes: 1 addition & 1 deletion lib/influxdb2/client/delete_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def _delete(delete_request, bucket: nil, org: nil)
uri = URI.parse(File.join(@options[:url], '/api/v2/delete'))
uri.query = URI.encode_www_form(org: org_param, bucket: bucket_param)

_post(delete_request.to_body.to_json, uri)
_post_json(delete_request.to_body.to_json, uri)
end

def _to_rfc3339(time)
Expand Down
2 changes: 1 addition & 1 deletion lib/influxdb2/client/query_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def _post_query(query: nil, org: nil, dialect: DEFAULT_DIALECT)
uri = URI.parse(File.join(@options[:url], '/api/v2/query'))
uri.query = URI.encode_www_form(org: org_param)

_post(payload.to_body.to_json, uri)
_post_json(payload.to_body.to_json, uri)
end

def _generate_payload(query, dialect)
Expand Down
2 changes: 1 addition & 1 deletion lib/influxdb2/client/write_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def write_raw(payload, precision: nil, bucket: nil, org: nil)
uri = URI.parse(File.join(@options[:url], '/api/v2/write'))
uri.query = URI.encode_www_form(bucket: bucket_param, org: org_param, precision: precision_param.to_s)

_post(payload, uri)
_post_text(payload, uri)
end

# Item for batching queue
Expand Down
3 changes: 2 additions & 1 deletion test/influxdb/delete_api_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ def test_user_agent_header
body = '{"start":"2019-02-03T04:05:06+07:00","stop":"2019-04-03T04:05:06+07:00"}'
headers = {
'Authorization' => 'Token my-token',
'User-Agent' => "influxdb-client-ruby/#{InfluxDB2::VERSION}"
'User-Agent' => "influxdb-client-ruby/#{InfluxDB2::VERSION}",
'Content-Type' => 'application/json'
}
assert_requested(:post, 'http://localhost:9999/api/v2/delete?bucket=my-bucket&org=my-org',
times: 1, body: body, headers: headers)
Expand Down
5 changes: 3 additions & 2 deletions test/influxdb/query_api_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def test_query
assert_equal 'free', record1.field
end

def test_user_agent_header
def test_headers
stub_request(:post, 'http://localhost:9999/api/v2/query?org=my-org')
.to_return(body: SUCCESS_DATA)

Expand All @@ -87,7 +87,8 @@ def test_user_agent_header

headers = {
'Authorization' => 'Token my-token',
'User-Agent' => "influxdb-client-ruby/#{InfluxDB2::VERSION}"
'User-Agent' => "influxdb-client-ruby/#{InfluxDB2::VERSION}",
'Content-Type' => 'application/json'
}
assert_requested(:post, 'http://localhost:9999/api/v2/query?org=my-org',
times: 1, headers: headers)
Expand Down
5 changes: 3 additions & 2 deletions test/influxdb/write_api_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ def test_write_precision_constant
assert_equal 'The time precision not_supported is not supported.', error.message
end

def test_user_agent_header
def test_headers
stub_request(:any, 'http://localhost:9999/api/v2/write?bucket=my-bucket&org=my-org&precision=ns')
.to_return(status: 204)

Expand All @@ -266,7 +266,8 @@ def test_user_agent_header

headers = {
'Authorization' => 'Token my-token',
'User-Agent' => "influxdb-client-ruby/#{InfluxDB2::VERSION}"
'User-Agent' => "influxdb-client-ruby/#{InfluxDB2::VERSION}",
'Content-Type' => 'text/plain'
}
assert_requested(:post, 'http://localhost:9999/api/v2/write?bucket=my-bucket&org=my-org&precision=ns',
times: 1, body: 'h2o,location=west value=33i 15', headers: headers)
Expand Down