diff --git a/lib/stripe.rb b/lib/stripe.rb index 759269a64..06e6f5b70 100644 --- a/lib/stripe.rb +++ b/lib/stripe.rb @@ -62,6 +62,8 @@ module Stripe class << self extend Forwardable + attr_reader :configuration + # User configurable options def_delegators :@configuration, :api_key, :api_key= def_delegators :@configuration, :api_version, :api_version= diff --git a/lib/stripe/connection_manager.rb b/lib/stripe/connection_manager.rb index e3ecc4bcc..0058cdc7b 100644 --- a/lib/stripe/connection_manager.rb +++ b/lib/stripe/connection_manager.rb @@ -15,8 +15,10 @@ class ConnectionManager # by `StripeClient` to determine whether a connection manager should be # garbage collected or not. attr_reader :last_used + attr_reader :config - def initialize + def initialize(config = Stripe.configuration) + @config = config @active_connections = {} @last_used = Util.monotonic_time @@ -117,17 +119,17 @@ def execute_request(method, uri, body: nil, headers: nil, query: nil) # reused Go's default for `DefaultTransport`. connection.keep_alive_timeout = 30 - connection.open_timeout = Stripe.open_timeout - connection.read_timeout = Stripe.read_timeout + connection.open_timeout = config.open_timeout + connection.read_timeout = config.read_timeout if connection.respond_to?(:write_timeout=) - connection.write_timeout = Stripe.write_timeout + connection.write_timeout = config.write_timeout end connection.use_ssl = uri.scheme == "https" - if Stripe.verify_ssl_certs + if config.verify_ssl_certs connection.verify_mode = OpenSSL::SSL::VERIFY_PEER - connection.cert_store = Stripe.ca_store + connection.cert_store = config.ca_store else connection.verify_mode = OpenSSL::SSL::VERIFY_NONE warn_ssl_verify_none @@ -141,10 +143,10 @@ def execute_request(method, uri, body: nil, headers: nil, query: nil) # out those pieces to make passing them into a new connection a little less # ugly. private def proxy_parts - if Stripe.proxy.nil? + if config.proxy.nil? [nil, nil, nil, nil] else - u = URI.parse(Stripe.proxy) + u = URI.parse(config.proxy) [u.host, u.port, u.user, u.password] end end diff --git a/lib/stripe/oauth.rb b/lib/stripe/oauth.rb index eab8fd12b..580b424c8 100644 --- a/lib/stripe/oauth.rb +++ b/lib/stripe/oauth.rb @@ -7,8 +7,8 @@ module OAuthOperations def self.execute_resource_request(method, url, params, opts) opts = Util.normalize_opts(opts) - opts[:client] ||= StripeClient.active_client - opts[:api_base] ||= Stripe.connect_base + opts[:client] ||= opts[:client] || StripeClient.active_client + opts[:api_base] ||= opts[:client].config.connect_base super(method, url, params, opts) end @@ -29,7 +29,8 @@ def self.get_client_id(params = {}) end def self.authorize_url(params = {}, opts = {}) - base = opts[:connect_base] || Stripe.connect_base + client = opts[:client] || StripeClient.active_client + base = opts[:connect_base] || client.config.connect_base path = "/oauth/authorize" path = "/express" + path if opts[:express] diff --git a/lib/stripe/resources/account.rb b/lib/stripe/resources/account.rb index c4dbb36bc..53e96d0df 100644 --- a/lib/stripe/resources/account.rb +++ b/lib/stripe/resources/account.rb @@ -45,12 +45,8 @@ def resource_url end # @override To make id optional - def self.retrieve(id = ARGUMENT_NOT_PROVIDED, opts = {}) - id = if id.equal?(ARGUMENT_NOT_PROVIDED) - nil - else - Util.check_string_argument!(id) - end + def self.retrieve(id = nil, opts = {}) + Util.check_string_argument!(id) if id # Account used to be a singleton, where this method's signature was # `(opts={})`. For the sake of not breaking folks who pass in an OAuth @@ -136,11 +132,10 @@ def deauthorize(client_id = nil, opts = {}) client_id: client_id, stripe_user_id: id, } + opts = @opts.merge(Util.normalize_opts(opts)) OAuth.deauthorize(params, opts) end - ARGUMENT_NOT_PROVIDED = Object.new - private def serialize_additional_owners(legal_entity, additional_owners) original_value = legal_entity diff --git a/lib/stripe/resources/file.rb b/lib/stripe/resources/file.rb index d5d816a82..2f2ab3391 100644 --- a/lib/stripe/resources/file.rb +++ b/lib/stripe/resources/file.rb @@ -25,8 +25,9 @@ def self.create(params = {}, opts = {}) end end + config = opts[:client]&.config || Stripe.configuration opts = { - api_base: Stripe.uploads_base, + api_base: config.uploads_base, content_type: MultipartEncoder::MULTIPART_FORM_DATA, }.merge(Util.normalize_opts(opts)) super diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 48ee67fcd..5b8316f57 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -13,15 +13,31 @@ class StripeClient @thread_contexts_with_connection_managers_mutex = Mutex.new @last_connection_manager_gc = Util.monotonic_time - # Initializes a new `StripeClient`. - # - # Takes a connection manager object for backwards compatibility only, and - # that use is DEPRECATED. - def initialize(_connection_manager = nil) + # Initializes a new StripeClient + def initialize(config_overrides = {}) @system_profiler = SystemProfiler.new @last_request_metrics = nil + + # Supports accepting a connection manager object for backwards + # compatibility only, and that use is DEPRECATED. + @config_overrides = case config_overrides + when Stripe::ConnectionManager + {} + when String + { api_key: config_overrides } + else + config_overrides + end + end + + # Always base config off the global Stripe configuration to ensure the + # client picks up any changes to the config. + def config + Stripe.configuration.reverse_duplicate_merge(@config_overrides) end + attr_reader :options + # Gets a currently active `StripeClient`. Set for the current thread when # `StripeClient#request` is being run so that API operations being executed # inside of that block can find the currently active client. It's reset to @@ -51,8 +67,8 @@ def self.clear_all_connection_managers # its connection manager and remove our reference to it. If it ever # makes a new request we'll give it a new connection manager and # it'll go back into `@thread_contexts_with_connection_managers`. - thread_context.default_connection_manager.clear - thread_context.default_connection_manager = nil + thread_context.default_connection_managers.map { |_, cm| cm.clear } + thread_context.reset_connection_managers end @thread_contexts_with_connection_managers.clear end @@ -63,10 +79,11 @@ def self.default_client current_thread_context.default_client ||= StripeClient.new end - # A default connection manager for the current thread. - def self.default_connection_manager - current_thread_context.default_connection_manager ||= begin - connection_manager = ConnectionManager.new + # A default connection manager for the current thread scoped to the + # configuration object that may be provided. + def self.default_connection_manager(config = Stripe.configuration) + current_thread_context.default_connection_managers[config.key] ||= begin + connection_manager = ConnectionManager.new(config) @thread_contexts_with_connection_managers_mutex.synchronize do maybe_gc_connection_managers @@ -80,8 +97,9 @@ def self.default_connection_manager # Checks if an error is a problem that we should retry on. This includes # both socket errors that may represent an intermittent problem and some # special HTTP statuses. - def self.should_retry?(error, method:, num_retries:) - return false if num_retries >= Stripe.max_network_retries + def self.should_retry?(error, + method:, num_retries:, config: Stripe.configuration) + return false if num_retries >= config.max_network_retries case error when Net::OpenTimeout, Net::ReadTimeout @@ -127,13 +145,13 @@ def self.should_retry?(error, method:, num_retries:) end end - def self.sleep_time(num_retries) + def self.sleep_time(num_retries, config: Stripe.configuration) # Apply exponential backoff with initial_network_retry_delay on the # number of num_retries so far as inputs. Do not allow the number to # exceed max_network_retry_delay. sleep_seconds = [ - Stripe.initial_network_retry_delay * (2**(num_retries - 1)), - Stripe.max_network_retry_delay, + config.initial_network_retry_delay * (2**(num_retries - 1)), + config.max_network_retry_delay, ].min # Apply some jitter by randomizing the value in the range of @@ -141,9 +159,7 @@ def self.sleep_time(num_retries) sleep_seconds *= (0.5 * (1 + rand)) # But never sleep less than the base sleep seconds. - sleep_seconds = [Stripe.initial_network_retry_delay, sleep_seconds].max - - sleep_seconds + [config.initial_network_retry_delay, sleep_seconds].max end # Gets the connection manager in use for the current `StripeClient`. @@ -187,8 +203,8 @@ def execute_request(method, path, raise ArgumentError, "path should be a string" \ unless path.is_a?(String) - api_base ||= Stripe.api_base - api_key ||= Stripe.api_key + api_base ||= config.api_base + api_key ||= config.api_key params = Util.objects_to_ids(params) check_api_key!(api_key) @@ -221,20 +237,22 @@ def execute_request(method, path, # stores information on the request we're about to make so that we don't # have to pass as many parameters around for logging. context = RequestLogContext.new - context.account = headers["Stripe-Account"] - context.api_key = api_key - context.api_version = headers["Stripe-Version"] - context.body = body_log + context.account = headers["Stripe-Account"] + context.api_key = api_key + context.api_version = headers["Stripe-Version"] + context.body = body_log context.idempotency_key = headers["Idempotency-Key"] - context.method = method - context.path = path - context.query = query + context.method = method + context.path = path + context.query = query http_resp = execute_request_with_rescues(method, api_base, context) do - self.class.default_connection_manager.execute_request(method, url, - body: body, - headers: headers, - query: query) + self.class + .default_connection_manager(config) + .execute_request(method, url, + body: body, + headers: headers, + query: query) end begin @@ -246,13 +264,21 @@ def execute_request(method, path, # If being called from `StripeClient#request`, put the last response in # thread-local memory so that it can be returned to the user. Don't store # anything otherwise so that we don't leak memory. - if self.class.current_thread_context.last_responses&.key?(object_id) - self.class.current_thread_context.last_responses[object_id] = resp - end + store_last_response(object_id, resp) [resp, api_key] end + def store_last_response(object_id, resp) + return unless last_response_has_key?(object_id) + + self.class.current_thread_context.last_responses[object_id] = resp + end + + def last_response_has_key?(object_id) + self.class.current_thread_context.last_responses&.key?(object_id) + end + # # private # @@ -328,11 +354,6 @@ class ThreadContext # the user hasn't specified their own. attr_accessor :default_client - # A default `ConnectionManager` for the thread. Normally shared between - # all `StripeClient` objects on a particular thread, and created so as to - # minimize the number of open connections that an application needs. - attr_accessor :default_connection_manager - # A temporary map of object IDs to responses from last executed API # calls. Used to return a responses from calls to `StripeClient#request`. # @@ -345,6 +366,17 @@ class ThreadContext # because that's wrapped in an `ensure` block, they should never leave # garbage in `Thread.current`. attr_accessor :last_responses + + # A default `ConnectionManager` for the thread. Normally shared between + # all `StripeClient` objects on a particular thread, and created so as to + # minimize the number of open connections that an application needs. + def default_connection_managers + @default_connection_managers ||= {} + end + + def reset_connection_managers + @default_connection_managers = {} + end end # Access data stored for `StripeClient` within the thread's current @@ -382,12 +414,15 @@ def self.maybe_gc_connection_managers pruned_thread_contexts = [] @thread_contexts_with_connection_managers.each do |thread_context| - connection_manager = thread_context.default_connection_manager - next if connection_manager.last_used > last_used_threshold - - connection_manager.clear - thread_context.default_connection_manager = nil - pruned_thread_contexts << thread_context + thread_context + .default_connection_managers + .each do |_, connection_manager| + next if connection_manager.last_used > last_used_threshold + + connection_manager.clear + thread_context.reset_connection_managers + pruned_thread_contexts << thread_context + end end @thread_contexts_with_connection_managers -= pruned_thread_contexts @@ -397,7 +432,7 @@ def self.maybe_gc_connection_managers end private def api_url(url = "", api_base = nil) - (api_base || Stripe.api_base) + url + (api_base || config.api_base) + url end private def check_api_key!(api_key) @@ -409,7 +444,7 @@ def self.maybe_gc_connection_managers "support@stripe.com if you have any questions." end - return unless api_key =~ /\s/ + return unless /\s/.match?(api_key) raise AuthenticationError, "Your API key is invalid, as it contains " \ "whitespace. (HINT: You can double-check your API key from the " \ @@ -471,7 +506,7 @@ def self.maybe_gc_connection_managers notify_request_end(context, request_duration, http_status, num_retries, user_data) - if Stripe.enable_telemetry? && context.request_id + if config.enable_telemetry? && context.request_id request_duration_ms = (request_duration * 1000).to_i @last_request_metrics = StripeRequestMetrics.new(context.request_id, request_duration_ms) @@ -498,9 +533,12 @@ def self.maybe_gc_connection_managers notify_request_end(context, request_duration, http_status, num_retries, user_data) - if self.class.should_retry?(e, method: method, num_retries: num_retries) + if self.class.should_retry?(e, + method: method, + num_retries: num_retries, + config: config) num_retries += 1 - sleep self.class.sleep_time(num_retries) + sleep self.class.sleep_time(num_retries, config: config) retry end @@ -622,7 +660,8 @@ def self.maybe_gc_connection_managers error_param: error_data[:param], error_type: error_data[:type], idempotency_key: context.idempotency_key, - request_id: context.request_id) + request_id: context.request_id, + config: config) # The standard set of arguments that can be used to initialize most of # the exceptions. @@ -671,7 +710,8 @@ def self.maybe_gc_connection_managers error_code: error_code, error_description: description, idempotency_key: context.idempotency_key, - request_id: context.request_id) + request_id: context.request_id, + config: config) args = { http_status: resp.http_status, http_body: resp.http_body, @@ -703,7 +743,8 @@ def self.maybe_gc_connection_managers Util.log_error("Stripe network error", error_message: error.message, idempotency_key: context.idempotency_key, - request_id: context.request_id) + request_id: context.request_id, + config: config) errors, message = NETWORK_ERROR_MESSAGES_MAP.detect do |(e, _)| error.is_a?(e) @@ -714,7 +755,7 @@ def self.maybe_gc_connection_managers "with Stripe. Please let us know at support@stripe.com." end - api_base ||= Stripe.api_base + api_base ||= config.api_base message = message % api_base message += " Request was retried #{num_retries} times." if num_retries > 0 @@ -735,7 +776,7 @@ def self.maybe_gc_connection_managers "Content-Type" => "application/x-www-form-urlencoded", } - if Stripe.enable_telemetry? && !@last_request_metrics.nil? + if config.enable_telemetry? && !@last_request_metrics.nil? headers["X-Stripe-Client-Telemetry"] = JSON.generate( last_request_metrics: @last_request_metrics.payload ) @@ -743,12 +784,12 @@ def self.maybe_gc_connection_managers # It is only safe to retry network failures on post and delete # requests if we add an Idempotency-Key header - if %i[post delete].include?(method) && Stripe.max_network_retries > 0 + if %i[post delete].include?(method) && config.max_network_retries > 0 headers["Idempotency-Key"] ||= SecureRandom.uuid end - headers["Stripe-Version"] = Stripe.api_version if Stripe.api_version - headers["Stripe-Account"] = Stripe.stripe_account if Stripe.stripe_account + headers["Stripe-Version"] = config.api_version if config.api_version + headers["Stripe-Account"] = config.stripe_account if config.stripe_account user_agent = @system_profiler.user_agent begin @@ -772,11 +813,13 @@ def self.maybe_gc_connection_managers idempotency_key: context.idempotency_key, method: context.method, num_retries: num_retries, - path: context.path) + path: context.path, + config: config) Util.log_debug("Request details", body: context.body, idempotency_key: context.idempotency_key, - query: context.query) + query: context.query, + config: config) end private def log_response(context, request_start, status, body) @@ -788,11 +831,13 @@ def self.maybe_gc_connection_managers method: context.method, path: context.path, request_id: context.request_id, - status: status) + status: status, + config: config) Util.log_debug("Response details", body: body, idempotency_key: context.idempotency_key, - request_id: context.request_id) + request_id: context.request_id, + config: config) return unless context.request_id @@ -800,7 +845,8 @@ def self.maybe_gc_connection_managers idempotency_key: context.idempotency_key, request_id: context.request_id, url: Util.request_id_dashboard_url(context.request_id, - context.api_key)) + context.api_key), + config: config) end private def log_response_error(context, request_start, error) @@ -810,7 +856,8 @@ def self.maybe_gc_connection_managers error_message: error.message, idempotency_key: context.idempotency_key, method: context.method, - path: context.path) + path: context.path, + config: config) end # RequestLogContext stores information about a request that's begin made so diff --git a/lib/stripe/stripe_configuration.rb b/lib/stripe/stripe_configuration.rb index 814d3e650..097854974 100644 --- a/lib/stripe/stripe_configuration.rb +++ b/lib/stripe/stripe_configuration.rb @@ -101,6 +101,14 @@ def max_network_retries=(val) @max_network_retries = val.to_i end + def max_network_retry_delay=(val) + @max_network_retry_delay = val.to_i + end + + def initial_network_retry_delay=(val) + @initial_network_retry_delay = val.to_i + end + def open_timeout=(open_timeout) @open_timeout = open_timeout StripeClient.clear_all_connection_managers @@ -174,5 +182,15 @@ def ca_store def enable_telemetry? enable_telemetry end + + # Generates a deterministic key to identify configuration objects with + # identical configuration values. + def key + @key ||= begin + instance_variables + .collect { |variable| instance_variable_get(variable) } + .join + end + end end end diff --git a/lib/stripe/util.rb b/lib/stripe/util.rb index dfb4146ac..b944471c2 100644 --- a/lib/stripe/util.rb +++ b/lib/stripe/util.rb @@ -76,24 +76,30 @@ def self.convert_to_stripe_object(data, opts = {}) end def self.log_error(message, data = {}) - if !Stripe.logger.nil? || - !Stripe.log_level.nil? && Stripe.log_level <= Stripe::LEVEL_ERROR + config = data.delete(:config) || Stripe.configuration + logger = config.logger || Stripe.logger + if !logger.nil? || + !config.log_level.nil? && config.log_level <= Stripe::LEVEL_ERROR log_internal(message, data, color: :cyan, level: Stripe::LEVEL_ERROR, logger: Stripe.logger, out: $stderr) end end def self.log_info(message, data = {}) - if !Stripe.logger.nil? || - !Stripe.log_level.nil? && Stripe.log_level <= Stripe::LEVEL_INFO + config = data.delete(:config) || Stripe.configuration + logger = config.logger || Stripe.logger + if !logger.nil? || + !config.log_level.nil? && config.log_level <= Stripe::LEVEL_INFO log_internal(message, data, color: :cyan, level: Stripe::LEVEL_INFO, logger: Stripe.logger, out: $stdout) end end def self.log_debug(message, data = {}) - if !Stripe.logger.nil? || - !Stripe.log_level.nil? && Stripe.log_level <= Stripe::LEVEL_DEBUG + config = data.delete(:config) || Stripe.configuration + logger = config.logger || Stripe.logger + if !logger.nil? || + !config.log_level.nil? && config.log_level <= Stripe::LEVEL_DEBUG log_internal(message, data, color: :blue, level: Stripe::LEVEL_DEBUG, logger: Stripe.logger, out: $stdout) end @@ -105,10 +111,10 @@ def self.symbolize_names(object) new_hash = {} object.each do |key, value| key = (begin - key.to_sym + key.to_sym rescue StandardError key - end) || key + end) || key new_hash[key] = symbolize_names(value) end new_hash @@ -289,7 +295,7 @@ def self.level_name(level) case level when LEVEL_DEBUG then "debug" when LEVEL_ERROR then "error" - when LEVEL_INFO then "info" + when LEVEL_INFO then "info" else level end end diff --git a/test/stripe/connection_manager_test.rb b/test/stripe/connection_manager_test.rb index f73a56eac..c5efcb3b0 100644 --- a/test/stripe/connection_manager_test.rb +++ b/test/stripe/connection_manager_test.rb @@ -79,6 +79,49 @@ class ConnectionManagerTest < Test::Unit::TestCase end end + context "when a StripeClient has different configurations" do + should "correctly initialize a connection" do + old_proxy = Stripe.proxy + + old_open_timeout = Stripe.open_timeout + old_read_timeout = Stripe.read_timeout + + begin + client = StripeClient.new( + proxy: "http://other:pass@localhost:8080", + open_timeout: 400, + read_timeout: 500, + verify_ssl_certs: true + ) + conn = Stripe::ConnectionManager.new(client.config) + .connection_for("https://stripe.com") + + # Host/port + assert_equal "stripe.com", conn.address + assert_equal 443, conn.port + + # Proxy + assert_equal "localhost", conn.proxy_address + assert_equal 8080, conn.proxy_port + assert_equal "other", conn.proxy_user + assert_equal "pass", conn.proxy_pass + + # Timeouts + assert_equal 400, conn.open_timeout + assert_equal 500, conn.read_timeout + + assert_equal true, conn.use_ssl? + assert_equal OpenSSL::SSL::VERIFY_PEER, conn.verify_mode + assert_equal Stripe.ca_store, conn.cert_store + ensure + Stripe.proxy = old_proxy + + Stripe.open_timeout = old_open_timeout + Stripe.read_timeout = old_read_timeout + end + end + end + should "produce the same connection multiple times" do conn1 = @manager.connection_for("https://stripe.com") conn2 = @manager.connection_for("https://stripe.com") diff --git a/test/stripe/oauth_test.rb b/test/stripe/oauth_test.rb index c8ac13f2e..64ac4a8ae 100644 --- a/test/stripe/oauth_test.rb +++ b/test/stripe/oauth_test.rb @@ -28,7 +28,6 @@ class OAuthTest < Test::Unit::TestCase assert_equal("https", uri.scheme) assert_equal("connect.stripe.com", uri.host) assert_equal("/oauth/authorize", uri.path) - assert_equal(["ca_test"], params["client_id"]) assert_equal(["read_write"], params["scope"]) assert_equal(["test@example.com"], params["stripe_user[email]"]) @@ -44,6 +43,14 @@ class OAuthTest < Test::Unit::TestCase assert_equal("connect.stripe.com", uri.host) assert_equal("/express/oauth/authorize", uri.path) end + + should "override the api base path when a StripeClient is provided" do + client = Stripe::StripeClient.new(connect_base: "https://other.stripe.com") + uri_str = OAuth.authorize_url({}, client: client) + + uri = URI.parse(uri_str) + assert_equal("other.stripe.com", uri.host) + end end context ".token" do @@ -83,6 +90,29 @@ class OAuthTest < Test::Unit::TestCase code: "this_is_an_authorization_code") assert_equal("another_access_token", resp.access_token) end + + should "override the api base path when a StripeClient is provided" do + stub_request(:post, "https://other.stripe.com/oauth/token") + .with(body: { + "grant_type" => "authorization_code", + "code" => "this_is_an_authorization_code", + }) + .to_return(body: JSON.generate(access_token: "sk_access_token", + scope: "read_only", + livemode: false, + token_type: "bearer", + refresh_token: "sk_refresh_token", + stripe_user_id: "acct_test", + stripe_publishable_key: "pk_test")) + + client = Stripe::StripeClient.new(connect_base: "https://other.stripe.com") + resp = OAuth.token( + { grant_type: "authorization_code", code: "this_is_an_authorization_code" }, + client: client + ) + + assert_equal("sk_access_token", resp.access_token) + end end context ".deauthorize" do @@ -99,6 +129,20 @@ class OAuthTest < Test::Unit::TestCase resp = OAuth.deauthorize(stripe_user_id: "acct_test_deauth") assert_equal("acct_test_deauth", resp.stripe_user_id) end + + should "override the api base path when a StripeClient is provided" do + stub_request(:post, "https://other.stripe.com/oauth/deauthorize") + .with(body: { + "client_id" => "ca_test", + "stripe_user_id" => "acct_test_deauth", + }) + .to_return(body: JSON.generate(stripe_user_id: "acct_test_deauth")) + + client = Stripe::StripeClient.new(connect_base: "https://other.stripe.com") + resp = OAuth.deauthorize({ stripe_user_id: "acct_test_deauth" }, client: client) + + assert_equal("acct_test_deauth", resp.stripe_user_id) + end end end end diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index c00ed510d..e872a7b1b 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -4,6 +4,24 @@ module Stripe class StripeClientTest < Test::Unit::TestCase + context "initializing a StripeClient" do + should "allow a String to be passed in order to set the api key" do + assert_equal StripeClient.new("test_123").config.api_key, "test_123" + end + + should "allow for overrides via a Hash" do + config = { api_key: "test_123", open_timeout: 100 } + client = StripeClient.new(config) + + assert_equal client.config.api_key, "test_123" + assert_equal client.config.open_timeout, 100 + end + + should "support deprecated ConnectionManager objects" do + assert StripeClient.new(Stripe::ConnectionManager.new).config.is_a?(Stripe::StripeConfiguration) + end + end + context ".active_client" do should "be .default_client outside of #request" do assert_equal StripeClient.default_client, StripeClient.active_client @@ -82,8 +100,8 @@ class StripeClientTest < Test::Unit::TestCase assert_equal 1, StripeClient.maybe_gc_connection_managers # And as an additional check, the connection manager of the current - # thread context should have been set to `nil` as it was GCed. - assert_nil StripeClient.current_thread_context.default_connection_manager + # thread context should have been removed as it was GCed. + assert_equal({}, StripeClient.current_thread_context.default_connection_managers) end end @@ -160,11 +178,26 @@ class StripeClientTest < Test::Unit::TestCase thread.join refute_equal StripeClient.default_connection_manager, other_thread_manager end + + should "create a separate connection manager per configuration" do + config = Stripe::StripeConfiguration.setup { |c| c.open_timeout = 100 } + connection_manager_one = StripeClient.default_connection_manager + connection_manager_two = StripeClient.default_connection_manager(config) + + assert_equal connection_manager_one.config.open_timeout, 30 + assert_equal connection_manager_two.config.open_timeout, 100 + end + + should "create a single connection manager for identitical configurations" do + 2.times { StripeClient.default_connection_manager(Stripe::StripeConfiguration.setup) } + + assert_equal 1, StripeClient.instance_variable_get(:@thread_contexts_with_connection_managers).first.default_connection_managers.size + end end context ".should_retry?" do setup do - Stripe.stubs(:max_network_retries).returns(2) + Stripe::StripeConfiguration.any_instance.stubs(:max_network_retries).returns(2) end should "retry on Errno::ECONNREFUSED" do @@ -275,7 +308,7 @@ class StripeClientTest < Test::Unit::TestCase context ".sleep_time" do should "should grow exponentially" do StripeClient.stubs(:rand).returns(1) - Stripe.stubs(:max_network_retry_delay).returns(999) + Stripe.configuration.stubs(:max_network_retry_delay).returns(999) assert_equal(Stripe.initial_network_retry_delay, StripeClient.sleep_time(1)) assert_equal(Stripe.initial_network_retry_delay * 2, StripeClient.sleep_time(2)) assert_equal(Stripe.initial_network_retry_delay * 4, StripeClient.sleep_time(3)) @@ -284,8 +317,8 @@ class StripeClientTest < Test::Unit::TestCase should "enforce the max_network_retry_delay" do StripeClient.stubs(:rand).returns(1) - Stripe.stubs(:initial_network_retry_delay).returns(1) - Stripe.stubs(:max_network_retry_delay).returns(2) + Stripe.configuration.stubs(:initial_network_retry_delay).returns(1) + Stripe.configuration.stubs(:max_network_retry_delay).returns(2) assert_equal(1, StripeClient.sleep_time(1)) assert_equal(2, StripeClient.sleep_time(2)) assert_equal(2, StripeClient.sleep_time(3)) @@ -295,8 +328,8 @@ class StripeClientTest < Test::Unit::TestCase should "add some randomness" do random_value = 0.8 StripeClient.stubs(:rand).returns(random_value) - Stripe.stubs(:initial_network_retry_delay).returns(1) - Stripe.stubs(:max_network_retry_delay).returns(8) + Stripe.configuration.stubs(:initial_network_retry_delay).returns(1) + Stripe.configuration.stubs(:max_network_retry_delay).returns(8) base_value = Stripe.initial_network_retry_delay * (0.5 * (1 + random_value)) @@ -309,6 +342,23 @@ class StripeClientTest < Test::Unit::TestCase assert_equal(base_value * 4, StripeClient.sleep_time(3)) assert_equal(base_value * 8, StripeClient.sleep_time(4)) end + + should "permit passing in a configuration object" do + StripeClient.stubs(:rand).returns(1) + config = Stripe::StripeConfiguration.setup do |c| + c.initial_network_retry_delay = 1 + c.max_network_retry_delay = 2 + end + + # Set the global configuration to be different than the client + Stripe.configuration.stubs(:initial_network_retry_delay).returns(100) + Stripe.configuration.stubs(:max_network_retry_delay).returns(200) + + assert_equal(1, StripeClient.sleep_time(1, config: config)) + assert_equal(2, StripeClient.sleep_time(2, config: config)) + assert_equal(2, StripeClient.sleep_time(3, config: config)) + assert_equal(2, StripeClient.sleep_time(4, config: config)) + end end context "#execute_request" do @@ -342,6 +392,10 @@ class StripeClientTest < Test::Unit::TestCase # switch over to rspec-mocks at some point, we can probably remove # this. Util.stubs(:monotonic_time).returns(0.0) + + # Stub the Stripe configuration so that mocha matchers will succeed. Currently, + # mocha does not support using param matchers within hashes. + StripeClient.any_instance.stubs(:config).returns(Stripe.configuration) end should "produce appropriate logging" do @@ -353,11 +407,13 @@ class StripeClientTest < Test::Unit::TestCase idempotency_key: "abc", method: :post, num_retries: 0, - path: "/v1/account") + path: "/v1/account", + config: Stripe.configuration) Util.expects(:log_debug).with("Request details", body: "", idempotency_key: "abc", - query: nil) + query: nil, + config: Stripe.configuration) Util.expects(:log_info).with("Response from Stripe API", account: "acct_123", @@ -367,15 +423,18 @@ class StripeClientTest < Test::Unit::TestCase method: :post, path: "/v1/account", request_id: "req_123", - status: 200) + status: 200, + config: Stripe.configuration) Util.expects(:log_debug).with("Response details", body: body, idempotency_key: "abc", - request_id: "req_123") + request_id: "req_123", + config: Stripe.configuration) Util.expects(:log_debug).with("Dashboard link for request", idempotency_key: "abc", request_id: "req_123", - url: Util.request_id_dashboard_url("req_123", Stripe.api_key)) + url: Util.request_id_dashboard_url("req_123", Stripe.api_key), + config: Stripe.configuration) stub_request(:post, "#{Stripe.api_base}/v1/account") .to_return( @@ -404,7 +463,8 @@ class StripeClientTest < Test::Unit::TestCase idempotency_key: nil, method: :post, num_retries: 0, - path: "/v1/account") + path: "/v1/account", + config: Stripe.configuration) Util.expects(:log_info).with("Response from Stripe API", account: nil, api_version: nil, @@ -413,7 +473,8 @@ class StripeClientTest < Test::Unit::TestCase method: :post, path: "/v1/account", request_id: nil, - status: 500) + status: 500, + config: Stripe.configuration) error = { code: "code", @@ -428,7 +489,8 @@ class StripeClientTest < Test::Unit::TestCase error_param: error[:param], error_type: error[:type], idempotency_key: nil, - request_id: nil) + request_id: nil, + config: Stripe.configuration) stub_request(:post, "#{Stripe.api_base}/v1/account") .to_return( @@ -449,7 +511,8 @@ class StripeClientTest < Test::Unit::TestCase idempotency_key: nil, method: :post, num_retries: 0, - path: "/oauth/token") + path: "/oauth/token", + config: Stripe.configuration) Util.expects(:log_info).with("Response from Stripe API", account: nil, api_version: nil, @@ -458,14 +521,16 @@ class StripeClientTest < Test::Unit::TestCase method: :post, path: "/oauth/token", request_id: nil, - status: 400) + status: 400, + config: Stripe.configuration) Util.expects(:log_error).with("Stripe OAuth error", status: 400, error_code: "invalid_request", error_description: "No grant type specified", idempotency_key: nil, - request_id: nil) + request_id: nil, + config: Stripe.configuration) stub_request(:post, "#{Stripe.connect_base}/oauth/token") .to_return(body: JSON.generate(error: "invalid_request", @@ -788,7 +853,7 @@ class StripeClientTest < Test::Unit::TestCase context "idempotency keys" do setup do - Stripe.stubs(:max_network_retries).returns(2) + Stripe::StripeConfiguration.any_instance.stubs(:max_network_retries).returns(2) end should "not add an idempotency key to GET requests" do @@ -838,7 +903,7 @@ class StripeClientTest < Test::Unit::TestCase context "retry logic" do setup do - Stripe.stubs(:max_network_retries).returns(2) + Stripe::StripeConfiguration.any_instance.stubs(:max_network_retries).returns(2) end should "retry failed requests and raise if error persists" do @@ -870,6 +935,21 @@ class StripeClientTest < Test::Unit::TestCase client = StripeClient.new client.execute_request(:post, "/v1/charges") end + + should "pass the client configuration when retrying" do + StripeClient.expects(:sleep_time) + .with(any_of(1, 2), + has_entry(:config, kind_of(Stripe::StripeConfiguration))) + .at_least_once.returns(0) + + stub_request(:post, "#{Stripe.api_base}/v1/charges") + .to_raise(Errno::ECONNREFUSED.new) + + client = StripeClient.new + assert_raises Stripe::APIConnectionError do + client.execute_request(:post, "/v1/charges") + end + end end context "params serialization" do @@ -1079,7 +1159,7 @@ class StripeClientTest < Test::Unit::TestCase context "#proxy" do should "run the request through the proxy" do begin - StripeClient.current_thread_context.default_connection_manager = nil + StripeClient.clear_all_connection_managers Stripe.proxy = "http://user:pass@localhost:8080" @@ -1095,7 +1175,7 @@ class StripeClientTest < Test::Unit::TestCase ensure Stripe.proxy = nil - StripeClient.current_thread_context.default_connection_manager = nil + StripeClient.clear_all_connection_managers end end end diff --git a/test/stripe/stripe_configuration_test.rb b/test/stripe/stripe_configuration_test.rb index 1630aa3d0..9ad743122 100644 --- a/test/stripe/stripe_configuration_test.rb +++ b/test/stripe/stripe_configuration_test.rb @@ -20,6 +20,7 @@ class StripeConfigurationTest < Test::Unit::TestCase assert_equal "https://api.stripe.com", config.api_base assert_equal "https://connect.stripe.com", config.connect_base assert_equal "https://files.stripe.com", config.uploads_base + assert_equal nil, config.api_version end should "allow for overrides when a block is passed" do @@ -41,7 +42,7 @@ class StripeConfigurationTest < Test::Unit::TestCase c.open_timeout = 100 end - duped_config = config.reverse_duplicate_merge(read_timeout: 500) + duped_config = config.reverse_duplicate_merge(read_timeout: 500, api_version: "2018-08-02") assert_equal config.open_timeout, duped_config.open_timeout assert_equal 500, duped_config.read_timeout @@ -57,6 +58,24 @@ class StripeConfigurationTest < Test::Unit::TestCase end end + context "#max_network_retry_delay=" do + should "coerce the option into an integer" do + config = Stripe::StripeConfiguration.setup + + config.max_network_retry_delay = "10" + assert_equal 10, config.max_network_retry_delay + end + end + + context "#initial_network_retry_delay=" do + should "coerce the option into an integer" do + config = Stripe::StripeConfiguration.setup + + config.initial_network_retry_delay = "10" + assert_equal 10, config.initial_network_retry_delay + end + end + context "#log_level=" do should "be backwards compatible with old values" do config = Stripe::StripeConfiguration.setup @@ -127,5 +146,14 @@ class StripeConfigurationTest < Test::Unit::TestCase config.verify_ssl_certs = false end end + + context "#key" do + should "generate the same key when values are identicial" do + assert_equal StripeConfiguration.setup.key, StripeConfiguration.setup.key + + custom_config = StripeConfiguration.setup { |c| c.open_timeout = 1000 } + refute_equal StripeConfiguration.setup.key, custom_config.key + end + end end end diff --git a/test/stripe_test.rb b/test/stripe_test.rb index 53e9d009f..c0c3bd817 100644 --- a/test/stripe_test.rb +++ b/test/stripe_test.rb @@ -114,6 +114,11 @@ class StripeTest < Test::Unit::TestCase assert_equal "https://other.stripe.com", Stripe.api_base end + should "allow api_version to be configured" do + Stripe.api_version = "2018-02-28" + assert_equal "2018-02-28", Stripe.api_version + end + should "allow connect_base to be configured" do Stripe.connect_base = "https://other.stripe.com" assert_equal "https://other.stripe.com", Stripe.connect_base