diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 83b64dbab..77d0f92c1 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -14,28 +14,29 @@ class StripeClient @last_connection_manager_gc = Util.monotonic_time # Initializes a new StripeClient - def initialize(config_overrides = {}) + def initialize(config_arg = {}) @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 - + @config = case config_arg + when Hash + Stripe.configuration.reverse_duplicate_merge(config_arg) + when Stripe::ConnectionManager + # Supports accepting a connection manager object for backwards + # compatibility only, and that use is DEPRECATED. + Stripe.configuration.dup + when Stripe::StripeConfiguration + config_arg + when String + Stripe.configuration.reverse_duplicate_merge( + { api_key: config_arg } + ) + else + raise ArgumentError, "Can't handle argument: #{config_arg}" + end + end + + attr_reader :config attr_reader :options # Gets a currently active `StripeClient`. Set for the current thread when @@ -53,30 +54,45 @@ def self.active_client # clears them from internal tracking in all connection managers across all # threads. # + # If passed a `config` object, only clear connection managers for that + # particular configuration. + # # For internal use only. Does not provide a stable API and may be broken # with future non-major changes. - def self.clear_all_connection_managers + def self.clear_all_connection_managers(config: nil) # Just a quick path for when configuration is being set for the first # time before any connections have been opened. There is technically some # potential for thread raciness here, but not in a practical sense. return if @thread_contexts_with_connection_managers.empty? @thread_contexts_with_connection_managers_mutex.synchronize do + pruned_contexts = Set.new + @thread_contexts_with_connection_managers.each do |thread_context| # Note that the thread context itself is not destroyed, but we clear # 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_managers.map { |_, cm| cm.clear } - thread_context.reset_connection_managers + thread_context.default_connection_managers.reject! do |cm_config, cm| + if config.nil? || config.key == cm_config + cm.clear + true + end + end + + if thread_context.default_connection_managers.empty? + pruned_contexts << thread_context + end end - @thread_contexts_with_connection_managers.clear + + @thread_contexts_with_connection_managers.subtract(pruned_contexts) end end # A default client for the current thread. def self.default_client - current_thread_context.default_client ||= StripeClient.new + current_thread_context.default_client ||= + StripeClient.new(Stripe.configuration) end # A default connection manager for the current thread scoped to the @@ -412,7 +428,7 @@ def self.maybe_gc_connection_managers last_used_threshold = Util.monotonic_time - CONNECTION_MANAGER_GC_LAST_USED_EXPIRY - pruned_thread_contexts = [] + pruned_contexts = [] @thread_contexts_with_connection_managers.each do |thread_context| thread_context .default_connection_managers @@ -427,13 +443,13 @@ def self.maybe_gc_connection_managers @thread_contexts_with_connection_managers.each do |thread_context| next unless thread_context.default_connection_managers.empty? - pruned_thread_contexts << thread_context + pruned_contexts << thread_context end - @thread_contexts_with_connection_managers -= pruned_thread_contexts + @thread_contexts_with_connection_managers -= pruned_contexts @last_connection_manager_gc = Util.monotonic_time - pruned_thread_contexts.count + pruned_contexts.count end private def api_url(url = "", api_base = nil) diff --git a/lib/stripe/stripe_configuration.rb b/lib/stripe/stripe_configuration.rb index cb12b4e25..1fd94df05 100644 --- a/lib/stripe/stripe_configuration.rb +++ b/lib/stripe/stripe_configuration.rb @@ -111,12 +111,12 @@ def initial_network_retry_delay=(val) def open_timeout=(open_timeout) @open_timeout = open_timeout - StripeClient.clear_all_connection_managers + StripeClient.clear_all_connection_managers(config: self) end def read_timeout=(read_timeout) @read_timeout = read_timeout - StripeClient.clear_all_connection_managers + StripeClient.clear_all_connection_managers(config: self) end def write_timeout=(write_timeout) @@ -125,32 +125,32 @@ def write_timeout=(write_timeout) end @write_timeout = write_timeout - StripeClient.clear_all_connection_managers + StripeClient.clear_all_connection_managers(config: self) end def proxy=(proxy) @proxy = proxy - StripeClient.clear_all_connection_managers + StripeClient.clear_all_connection_managers(config: self) end def verify_ssl_certs=(verify_ssl_certs) @verify_ssl_certs = verify_ssl_certs - StripeClient.clear_all_connection_managers + StripeClient.clear_all_connection_managers(config: self) end def uploads_base=(uploads_base) @uploads_base = uploads_base - StripeClient.clear_all_connection_managers + StripeClient.clear_all_connection_managers(config: self) end def connect_base=(connect_base) @connect_base = connect_base - StripeClient.clear_all_connection_managers + StripeClient.clear_all_connection_managers(config: self) end def api_base=(api_base) @api_base = api_base - StripeClient.clear_all_connection_managers + StripeClient.clear_all_connection_managers(config: self) end def ca_bundle_path=(path) @@ -159,7 +159,7 @@ def ca_bundle_path=(path) # empty this field so a new store is initialized @ca_store = nil - StripeClient.clear_all_connection_managers + StripeClient.clear_all_connection_managers(config: self) end # A certificate store initialized from the the bundle in #ca_bundle_path and diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index 9ea451dd3..2cb1d0766 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -20,6 +20,12 @@ class StripeClientTest < Test::Unit::TestCase should "support deprecated ConnectionManager objects" do assert StripeClient.new(Stripe::ConnectionManager.new).config.is_a?(Stripe::StripeConfiguration) end + + should "support passing a full configuration object" do + config = Stripe.configuration.reverse_duplicate_merge({ api_key: "test_123", open_timeout: 100 }) + client = StripeClient.new(config) + assert_equal config, client.config + end end context ".active_client" do @@ -203,6 +209,27 @@ class StripeClientTest < Test::Unit::TestCase # And finally, give all threads time to perform their check. threads.each(&:join) end + + should "clear only connection managers with a given configuration" do + StripeClient.clear_all_connection_managers + + client1 = StripeClient.new(read_timeout: 5.0) + StripeClient.default_connection_manager(client1.config) + client2 = StripeClient.new(read_timeout: 2.0) + StripeClient.default_connection_manager(client2.config) + + thread_contexts = StripeClient.instance_variable_get(:@thread_contexts_with_connection_managers) + assert_equal 1, thread_contexts.count + thread_context = thread_contexts.first + + refute_nil thread_context.default_connection_managers[client1.config.key] + refute_nil thread_context.default_connection_managers[client2.config.key] + + StripeClient.clear_all_connection_managers(config: client1.config) + + assert_nil thread_context.default_connection_managers[client1.config.key] + refute_nil thread_context.default_connection_managers[client2.config.key] + end end context ".default_client" do @@ -244,7 +271,9 @@ class StripeClientTest < Test::Unit::TestCase assert_equal connection_manager_two.config.open_timeout, 100 end - should "create a single connection manager for identitical configurations" do + should "create a single connection manager for identical configurations" do + StripeClient.clear_all_connection_managers + 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 diff --git a/test/stripe/stripe_configuration_test.rb b/test/stripe/stripe_configuration_test.rb index 9ad743122..9f34293b3 100644 --- a/test/stripe/stripe_configuration_test.rb +++ b/test/stripe/stripe_configuration_test.rb @@ -100,49 +100,49 @@ class StripeConfigurationTest < Test::Unit::TestCase should "clear when setting allow ca_bundle_path" do config = Stripe::StripeConfiguration.setup - StripeClient.expects(:clear_all_connection_managers) + StripeClient.expects(:clear_all_connection_managers).with(config: config) config.ca_bundle_path = "/path/to/ca/bundle" end should "clear when setting open timeout" do config = Stripe::StripeConfiguration.setup - StripeClient.expects(:clear_all_connection_managers) + StripeClient.expects(:clear_all_connection_managers).with(config: config) config.open_timeout = 10 end should "clear when setting read timeout" do config = Stripe::StripeConfiguration.setup - StripeClient.expects(:clear_all_connection_managers) + StripeClient.expects(:clear_all_connection_managers).with(config: config) config.read_timeout = 10 end should "clear when setting uploads_base" do config = Stripe::StripeConfiguration.setup - StripeClient.expects(:clear_all_connection_managers) + StripeClient.expects(:clear_all_connection_managers).with(config: config) config.uploads_base = "https://other.stripe.com" end - should "clearn when setting api_base to be configured" do + should "clear when setting api_base to be configured" do config = Stripe::StripeConfiguration.setup - StripeClient.expects(:clear_all_connection_managers) + StripeClient.expects(:clear_all_connection_managers).with(config: config) config.api_base = "https://other.stripe.com" end should "clear when setting connect_base" do config = Stripe::StripeConfiguration.setup - StripeClient.expects(:clear_all_connection_managers) + StripeClient.expects(:clear_all_connection_managers).with(config: config) config.connect_base = "https://other.stripe.com" end should "clear when setting verify_ssl_certs" do config = Stripe::StripeConfiguration.setup - StripeClient.expects(:clear_all_connection_managers) + StripeClient.expects(:clear_all_connection_managers).with(config: config) config.verify_ssl_certs = false end end