diff --git a/README.md b/README.md index baab5ec..a2544ed 100644 --- a/README.md +++ b/README.md @@ -337,6 +337,21 @@ client = Contentful::Client.new( nil Password for proxy authentication. + + timeout_read + nil + Number of seconds the request waits to read from the server before timing out. + + + timeout_write + nil + Number of seconds the request waits when writing to the server before timing out. + + + timeout_connect + nil + Number of seconds the request waits to connect to the server before timing out. + logger nil diff --git a/lib/contentful/client.rb b/lib/contentful/client.rb index 0acebf9..7652e26 100644 --- a/lib/contentful/client.rb +++ b/lib/contentful/client.rb @@ -35,6 +35,9 @@ class Client proxy_username: nil, proxy_password: nil, proxy_port: nil, + timeout_connect: nil, + timeout_read: nil, + timeout_write: nil, max_rate_limit_retries: 1, max_rate_limit_wait: 60, max_include_resolution_depth: 20, @@ -49,11 +52,13 @@ class Client # Wraps the actual HTTP request via proxy # @private - def self.get_http(url, query, headers = {}, proxy = {}) + def self.get_http(url, query, headers = {}, proxy = {}, timeout = {}) + http = HTTP[headers] + http = http.timeout(timeout) if timeout.any? if proxy[:host] - HTTP[headers].via(proxy[:host], proxy[:port], proxy[:username], proxy[:password]).get(url, params: query) + http.via(proxy[:host], proxy[:port], proxy[:username], proxy[:password]).get(url, params: query) else - HTTP[headers].get(url, params: query) + http.get(url, params: query) end end @@ -68,6 +73,9 @@ def self.get_http(url, query, headers = {}, proxy = {}) # @option given_configuration [String] :proxy_username # @option given_configuration [String] :proxy_password # @option given_configuration [Number] :proxy_port + # @option given_configuration [Number] :timeout_read + # @option given_configuration [Number] :timeout_write + # @option given_configuration [Number] :timeout_connect # @option given_configuration [Number] :max_rate_limit_retries # @option given_configuration [Number] :max_rate_limit_wait # @option given_configuration [Number] :max_include_resolution_depth @@ -110,6 +118,15 @@ def proxy_params } end + # @private + def timeout_params + { + connect: configuration[:timeout_connect], + read: configuration[:timeout_read], + write: configuration[:timeout_write] + }.reject { |_, value| value.nil? } + end + # Returns the default configuration # @private def default_configuration @@ -355,7 +372,8 @@ def run_request(request) url, request_query(request.query), request_headers, - proxy_params + proxy_params, + timeout_params ), request ) end @@ -431,6 +449,7 @@ def validate_configuration! fail ArgumentError, 'The :dynamic_entries mode must be :auto or :manual' unless %i[auto manual].include?( configuration[:dynamic_entries] ) + fail ArgumentError, 'Timeout parameters must be all omitted or all present' unless timeout_params.empty? || timeout_params.length == 3 end end end diff --git a/spec/client_class_spec.rb b/spec/client_class_spec.rb index 4c97f09..343a5fb 100644 --- a/spec/client_class_spec.rb +++ b/spec/client_class_spec.rb @@ -2,10 +2,11 @@ describe Contentful::Client do describe '#get' do - let(:client) { create_client } + let(:client) { create_client() } let(:proxy_client) { create_client(proxy_host: '183.207.232.194', proxy_port: 8080, secure: false) } + let(:timeout_client) { create_client(timeout_connect: 1, timeout_read: 2, timeout_write: 3) } let(:request) { Contentful::Request.new(nil, client.environment_url('/content_types'), nil, 'cat') } it 'uses #base_url' do @@ -32,17 +33,37 @@ end it 'calls #get_http' do - expect(client.class).to receive(:get_http).with(client.base_url + request.url, request.query, client.request_headers, client.proxy_params) { raw_fixture('content_type') } + expect(client.class).to receive(:get_http).with(client.base_url + request.url, request.query, client.request_headers, client.proxy_params, client.timeout_params) { raw_fixture('content_type') } client.get(request) end it 'calls #get_http via proxy' do - expect(proxy_client.class).to receive(:get_http).with(proxy_client.base_url + request.url, request.query, proxy_client.request_headers, proxy_client.proxy_params) { raw_fixture('content_type') } + expect(proxy_client.class).to receive(:get_http).with(proxy_client.base_url + request.url, request.query, proxy_client.request_headers, proxy_client.proxy_params, client.timeout_params) { raw_fixture('content_type') } proxy_client.get(request) expect(proxy_client.proxy_params[:host]).to eq '183.207.232.194' expect(proxy_client.proxy_params[:port]).to eq 8080 end + describe 'timeout params' do + context 'with timeouts configured' do + it 'calls #get_http with timeouts' do + expect(timeout_client.class).to receive(:get_http).with(timeout_client.base_url + request.url, request.query, timeout_client.request_headers, timeout_client.proxy_params, timeout_client.timeout_params) { raw_fixture('content_type') } + timeout_client.get(request) + expect(timeout_client.timeout_params[:connect]).to eq 1 + expect(timeout_client.timeout_params[:read]).to eq 2 + expect(timeout_client.timeout_params[:write]).to eq 3 + end + end + + context 'without timeouts' do + it 'calls #get_http with timeouts' do + expect(client.class).to receive(:get_http).with(client.base_url + request.url, request.query, client.request_headers, client.proxy_params, client.timeout_params) { raw_fixture('content_type') } + client.get(request) + expect(client.timeout_params).to eq({}) + end + end + end + describe 'build_resources parameter' do it 'returns Contentful::Resource object if second parameter is true [default]' do res = vcr('content_type') { client.get(request) } diff --git a/spec/client_configuration_spec.rb b/spec/client_configuration_spec.rb index 41c133c..d4d4fbb 100644 --- a/spec/client_configuration_spec.rb +++ b/spec/client_configuration_spec.rb @@ -347,4 +347,31 @@ class Cat < Contentful::Entry expect(client.request_headers['X-Contentful-User-Agent']).to eq client.contentful_user_agent end end + + describe 'timeout options' do + let(:full_options) { { timeout_connect: 1, timeout_read: 2, timeout_write: 3 } } + + it 'allows the three options to be present together' do + expect do + create_client(full_options) + end.not_to raise_error + end + + it 'allows the three options to be omitted' do + expect do + create_client() + end.not_to raise_error + end + + it 'does not allow only some options to be set' do + # Test that any combination of 1 or 2 keys is rejected + 1.upto(2) do |options_count| + full_options.keys.combination(options_count).each do |option_keys| + expect do + create_client(full_options.select { |key, _| option_keys.include?(key) }) + end.to raise_error(ArgumentError) + end + end + end + end end